From b20ea145b32cdde781573594f3bba8455245cf04 Mon Sep 17 00:00:00 2001 From: Ed_ Date: Fri, 19 Jun 2026 23:50:33 -0400 Subject: [PATCH] TIER-2 READ conductor/code_styleguides/error_handling.md end-to-end before Phase 5: refactor(gui_2): migrate L1428 request_patch_from_tier4 to Result[T] (Phase 5) Extract request_patch_from_tier4_result helper from the ai_client.run_tier4_patch_generation try/except in App.request_patch_from_tier4. Legacy wrapper drains errors to app._last_request_errors per FR-BC-4 event-handler pattern. [pre-audit] L1428 INTERNAL_BROAD_CATCH [post-audit] V count: 10 -> 9 (L1428 removed) --- src/gui_2.py | 60 +++++++++++++++++++++++++++++--------- tests/test_gui_2_result.py | 43 +++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 14 deletions(-) diff --git a/src/gui_2.py b/src/gui_2.py index a1dc4314..d1f6c4cc 100644 --- a/src/gui_2.py +++ b/src/gui_2.py @@ -1382,20 +1382,10 @@ class App: self._push_mma_state_update() def request_patch_from_tier4(self, error: str, file_context: str) -> None: - try: - from src import ai_client - from src.diff_viewer import parse_diff - patch_text = ai_client.run_tier4_patch_generation(error, file_context) - if patch_text and "---" in patch_text and "+++" in patch_text: - diff_files = parse_diff(patch_text) - file_paths = [df.old_path for df in diff_files] - self._pending_patch_text = patch_text - self._pending_patch_files = file_paths - self._show_patch_modal = True - else: - self._patch_error_message = patch_text or "No patch generated" - except Exception as e: - self._patch_error_message = str(e) + tier4_result = request_patch_from_tier4_result(self, error, file_context) + if not tier4_result.ok: + if not hasattr(self, '_last_request_errors'): self._last_request_errors = [] + self._last_request_errors.append(("request_patch_from_tier4", tier4_result.errors[0])) def bulk_execute(self) -> None: for tid in self.ui_selected_tickets: @@ -7863,6 +7853,48 @@ def _open_patch_in_external_editor_result(app: "App") -> Result[bool]: original=e, )]) + +def request_patch_from_tier4_result(app: "App", error: str, file_context: str) -> Result[bool]: + """Drain-aware variant of L1428 request_patch_from_tier4. + + Extracts the ai_client.run_tier4_patch_generation try/except from + App.request_patch_from_tier4 into a Result-returning helper. On success + (patch_text has --- and +++), sets app._pending_patch_text/files and + app._show_patch_modal=True and returns Result(data=True). On invalid + patch (no --- or no +++) or exception, sets app._patch_error_message and + returns Result(data=False, errors=[ErrorInfo]). + + The legacy wrapper drains errors to app._last_request_errors (per FR-BC-4 + event-handler drain pattern; data plane attribute). + + [C: src/gui_2.py:App.request_patch_from_tier4 (L1428 legacy wrapper)] + """ + from src.diff_viewer import parse_diff + try: + patch_text = ai_client.run_tier4_patch_generation(error, file_context) + if patch_text and "---" in patch_text and "+++" in patch_text: + diff_files = parse_diff(patch_text) + file_paths = [df.old_path for df in diff_files] + app._pending_patch_text = patch_text + app._pending_patch_files = file_paths + app._show_patch_modal = True + return Result(data=True) + else: + app._patch_error_message = patch_text or "No patch generated" + return Result(data=False, errors=[ErrorInfo( + kind=ErrorKind.INTERNAL, + message=f"Tier4 patch invalid or empty: {app._patch_error_message}", + source="gui_2.request_patch_from_tier4_result", + )]) + except Exception as e: + app._patch_error_message = str(e) + return Result(data=False, errors=[ErrorInfo( + kind=ErrorKind.INTERNAL, + message=f"Tier4 patch generation failed: {e}", + source="gui_2.request_patch_from_tier4_result", + original=e, + )]) + #endregion: Phase 5 Event Handler Result Helpers #endregion: MMA diff --git a/tests/test_gui_2_result.py b/tests/test_gui_2_result.py index faa5994e..17f98d4f 100644 --- a/tests/test_gui_2_result.py +++ b/tests/test_gui_2_result.py @@ -975,3 +975,46 @@ def test_phase_5_l1393_open_patch_in_external_editor_result_failure(): err = result.errors[0] assert err.source == "gui_2._open_patch_in_external_editor_result" assert "temp file creation blew up" in err.message + + +def test_phase_5_l1428_request_patch_from_tier4_result_success(): + """ + L1428 request_patch_from_tier4_result returns Result.ok=True on success. + + The helper wraps the ai_client.run_tier4_patch_generation try/except in + App.request_patch_from_tier4. On success (patch_text has --- and +++), + returns Result(data=True) and sets app._pending_patch_text/files and + app._show_patch_modal=True. + """ + from src import gui_2 + from unittest.mock import MagicMock, patch + app = MagicMock() + patch_text = "--- a/foo.py\n+++ b/foo.py\n@@ -1 +1 @@\n-old\n+new\n" + mock_diff_files = [MagicMock(old_path="/proj/foo/foo.py", new_path="/proj/foo/foo.py")] + with patch.object(gui_2.ai_client, "run_tier4_patch_generation", return_value=patch_text), \ + patch("src.diff_viewer.parse_diff", return_value=mock_diff_files): + result = gui_2.request_patch_from_tier4_result(app, "boom", "/proj/foo/foo.py") + assert result.ok, f"Expected ok=True on success, got errors: {result.errors}" + assert result.data is True + assert app._pending_patch_text is patch_text + assert app._pending_patch_files == ["/proj/foo/foo.py"] + assert app._show_patch_modal is True + + +def test_phase_5_l1428_request_patch_from_tier4_result_failure(): + """ + L1428 request_patch_from_tier4_result returns Result.ok=False with ErrorInfo on failure. + + When ai_client.run_tier4_patch_generation raises, the helper converts the + exception to ErrorInfo and returns Result(data=False, errors=[ErrorInfo]). + """ + from src import gui_2 + from unittest.mock import MagicMock, patch + app = MagicMock() + with patch.object(gui_2.ai_client, "run_tier4_patch_generation", side_effect=RuntimeError("tier4 backend down")): + result = gui_2.request_patch_from_tier4_result(app, "boom", "/proj/foo/foo.py") + 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.request_patch_from_tier4_result" + assert "tier4 backend down" in err.message