TIER-2 READ conductor/code_styleguides/error_handling.md end-to-end before Phase 7: refactor(gui_2): migrate L4321 worker to Result[T] (Phase 7)
Migrate the worker() closure in _check_auto_refresh_context_preview (L4321) to the canonical Result[T] pattern: - Extract _worker_context_preview_result(app) -> Result[None] helper into new Phase 7 Worker/Background Result Helpers region. - The legacy worker() wrapper calls the helper and drains errors to app.controller._worker_errors (with controller._worker_errors_lock acquired on append) per sub-track 3 Phase 6 Group 6.5 telemetry drain. - The try/finally cleanup (setting _is_generating_preview=False and handling _pending_preview_refresh) is preserved verbatim. Tests: - test_phase_7_l4321_worker_context_preview_result_success - test_phase_7_l4321_worker_context_preview_result_failure Audit: INTERNAL_BROAD_CATCH count in src/gui_2.py dropped from 3 to 2 (remaining: L591 _diag_layout_state, L897 _capture_workspace_profile). The lock-protected append ensures thread-safety when multiple worker threads call _report-style drains concurrently. The helper preserves the original fallback behavior (app.context_preview_text = 'Error generating context preview.' on failure) so the user-visible UX is unchanged.
This commit is contained in:
+35
-6
@@ -4315,16 +4315,16 @@ def _check_auto_refresh_context_preview(app: App) -> None:
|
||||
app._is_generating_preview = True
|
||||
def worker():
|
||||
try:
|
||||
app.controller.context_files = app.context_files
|
||||
res = app.controller._do_generate()
|
||||
app.context_preview_text = res[0]
|
||||
except Exception:
|
||||
app.context_preview_text = "Error generating context preview."
|
||||
result = _worker_context_preview_result(app)
|
||||
if not result.ok:
|
||||
if app.controller is not None and hasattr(app.controller, "_worker_errors"):
|
||||
with app.controller._worker_errors_lock:
|
||||
app.controller._worker_errors.append(("_check_auto_refresh_context_preview.worker", result.errors[0]))
|
||||
finally:
|
||||
app._is_generating_preview = False
|
||||
if getattr(app, "_pending_preview_refresh", False):
|
||||
app._pending_preview_refresh = False
|
||||
# This will trigger again on next GUI frame because _last_context_preview_state
|
||||
# This will trigger again on next GUI frame because _last_context_preview_state
|
||||
# will be slightly behind if another change happened during the thread.
|
||||
# Or we just clear the state so it re-triggers.
|
||||
app._last_context_preview_state = None
|
||||
@@ -8073,4 +8073,33 @@ def _render_beads_tab_list_result(app: "App") -> Result[list]:
|
||||
|
||||
#endregion: Phase 5 Event Handler Result Helpers
|
||||
|
||||
#region: Phase 7 Worker/Background Result Helpers (result_migration_gui_2_20260619)
|
||||
|
||||
def _worker_context_preview_result(app: "App") -> Result[None]:
|
||||
"""Drain-aware variant of L4321 worker closure in _check_auto_refresh_context_preview.
|
||||
|
||||
Extracts the try body from the worker() closure into a Result-returning helper.
|
||||
On exception, sets app.context_preview_text to a fallback message and returns
|
||||
Result(data=None, errors=[ErrorInfo]). On success, sets app.context_preview_text
|
||||
to the generated markdown. The legacy worker() wrapper drains errors to
|
||||
app.controller._worker_errors (with the controller lock acquired on append).
|
||||
|
||||
[C: src/gui_2.py:_check_auto_refresh_context_preview (L4321 legacy wrapper)]
|
||||
"""
|
||||
try:
|
||||
app.controller.context_files = app.context_files
|
||||
res = app.controller._do_generate()
|
||||
app.context_preview_text = res[0]
|
||||
return Result(data=None)
|
||||
except Exception as e:
|
||||
app.context_preview_text = "Error generating context preview."
|
||||
return Result(data=None, errors=[ErrorInfo(
|
||||
kind=ErrorKind.INTERNAL,
|
||||
message=f"Worker error generating context preview: {e}",
|
||||
source="gui_2._worker_context_preview_result",
|
||||
original=e,
|
||||
)])
|
||||
|
||||
#endregion: Phase 7 Worker/Background Result Helpers
|
||||
|
||||
#endregion: MMA
|
||||
|
||||
@@ -1398,3 +1398,53 @@ def test_phase_6_invariant_zero_sites_in_phase_6():
|
||||
f"{phase_6_site_tests}. If a Phase 6 site was added, update the "
|
||||
f"inventory and migrate it."
|
||||
)
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# Phase 7 Tests - Migration of 1 INTERNAL_BROAD_CATCH worker site to Result[T]
|
||||
# The helper wraps the try body from the worker() closure in
|
||||
# _check_auto_refresh_context_preview (L4321).
|
||||
# The legacy wrapper drains errors to app.controller._worker_errors (with lock).
|
||||
# =============================================================================
|
||||
|
||||
|
||||
def test_phase_7_l4321_worker_context_preview_result_success():
|
||||
"""
|
||||
L4321 _worker_context_preview_result returns Result(data=None) on success.
|
||||
|
||||
The helper wraps the try body from the worker() closure in
|
||||
_check_auto_refresh_context_preview. On success, sets
|
||||
app.context_preview_text to the generated markdown and returns
|
||||
Result(data=None).
|
||||
"""
|
||||
from src import gui_2
|
||||
from unittest.mock import MagicMock
|
||||
app = MagicMock()
|
||||
app.controller._do_generate.return_value = ("# Generated Preview\n\nContent here", "preview.md")
|
||||
result = gui_2._worker_context_preview_result(app)
|
||||
assert result.ok, f"Expected ok=True on success, got errors: {result.errors}"
|
||||
assert result.data is None
|
||||
assert app.controller.context_files == app.context_files
|
||||
assert app.context_preview_text == "# Generated Preview\n\nContent here"
|
||||
|
||||
|
||||
def test_phase_7_l4321_worker_context_preview_result_failure():
|
||||
"""
|
||||
L4321 _worker_context_preview_result returns Result(data=None, errors=[ErrorInfo]) on failure.
|
||||
|
||||
When the underlying controller._do_generate() call raises, the helper sets
|
||||
app.context_preview_text to a fallback error message and returns Result
|
||||
with ErrorInfo describing the failure.
|
||||
"""
|
||||
from src import gui_2
|
||||
from unittest.mock import MagicMock
|
||||
app = MagicMock()
|
||||
app.controller._do_generate.side_effect = RuntimeError("do_generate blew up")
|
||||
result = gui_2._worker_context_preview_result(app)
|
||||
assert not result.ok, f"Expected ok=False on failure, got data: {result.data}"
|
||||
assert result.errors, "Expected at least one error on failure"
|
||||
err = result.errors[0]
|
||||
assert err.source == "gui_2._worker_context_preview_result"
|
||||
assert "do_generate blew up" in err.message
|
||||
assert app.context_preview_text == "Error generating context preview."
|
||||
|
||||
|
||||
Reference in New Issue
Block a user