From bcfb4887b13af23fce2bfd7bbf694ea6bf3598f9 Mon Sep 17 00:00:00 2001 From: Ed_ Date: Sat, 20 Jun 2026 00:20:52 -0400 Subject: [PATCH] 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. --- src/gui_2.py | 41 ++++++++++++++++++++++++++----- tests/test_gui_2_result.py | 50 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+), 6 deletions(-) diff --git a/src/gui_2.py b/src/gui_2.py index b710984b..21e5f579 100644 --- a/src/gui_2.py +++ b/src/gui_2.py @@ -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 diff --git a/tests/test_gui_2_result.py b/tests/test_gui_2_result.py index 425e9e38..6070acc0 100644 --- a/tests/test_gui_2_result.py +++ b/tests/test_gui_2_result.py @@ -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." +