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)
This commit is contained in:
+46
-14
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user