From a213677cf084af8787cbfadfa14e952936b08bb0 Mon Sep 17 00:00:00 2001 From: Ed_ Date: Fri, 19 Jun 2026 22:52:32 -0400 Subject: [PATCH] TIER-2 READ conductor/code_styleguides/error_handling.md end-to-end before Phase 4: refactor(gui_2): migrate L3740 render_ast_inspector_modal file_content to Result[T] (Phase 4) Adds _render_ast_inspector_file_content_result(app, f_path) -> Result[str | None] helper that wraps the mcp_client.read_file try/except in render_ast_inspector_modal. On success, returns the file content string. On failure, returns Result(data=None, errors=[ErrorInfo]). The legacy wrapper handles the side effects (sets app._cached_ast_file_lines + app.text_viewer_content) and drains errors to app._last_request_errors (per FR-BC-3 modal pattern; data plane attribute). Audit: BROAD_CATCH count 15 -> 14, COMPLIANT count 22 -> 23. Migration target count drops by 1. All 3 Phase 4 sites migrated. Tests: 2/2 pass. --- src/gui_2.py | 38 +++++++++++++++++++++++++++++++------ tests/test_gui_2_result.py | 39 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 6 deletions(-) diff --git a/src/gui_2.py b/src/gui_2.py index db7718dd..4d5953cc 100644 --- a/src/gui_2.py +++ b/src/gui_2.py @@ -3733,13 +3733,15 @@ def render_ast_inspector_modal(app: App) -> None: 'start_line': int(start_ln), 'end_line': int(end_ln) }) - try: - content = mcp_client.read_file(f_path) - app._cached_ast_file_lines = content.splitlines() - app.text_viewer_content = content - except Exception: + content_result = _render_ast_inspector_file_content_result(app, f_path) + if not content_result.ok: + if not hasattr(app, '_last_request_errors'): app._last_request_errors = [] + app._last_request_errors.append(("render_ast_inspector.file_content", content_result.errors[0])) app._cached_ast_file_lines = ["Error loading file content."] app.text_viewer_content = "Error loading file content." + else: + app._cached_ast_file_lines = content_result.data.splitlines() + app.text_viewer_content = content_result.data app._cached_ast_file_path = f_path imgui.text(f"Editing Structure: {f_path}") @@ -7700,7 +7702,31 @@ def _render_ast_inspector_outline_result(app: "App", f_path: str) -> Result[str] original=e, )]) -#endregion: Phase 4 Modal/Dialog Result Helpers +def _render_ast_inspector_file_content_result(app: "App", f_path: str) -> Result[str | None]: + """Drain-aware variant of L3740 render_ast_inspector_modal file read try/except. + + Extracts the mcp_client.read_file try/except from render_ast_inspector_modal + into a Result-returning helper. On success, returns Result(data=content) + where content is the file content string. On failure (read_file raises), + returns Result(data=None, errors=[ErrorInfo]). + + The legacy wrapper handles the side effects: + - On success: app._cached_ast_file_lines = content.splitlines(); app.text_viewer_content = content + - On failure: app._cached_ast_file_lines = ["Error loading file content."]; app.text_viewer_content = "Error loading file content." + And drains errors to app._last_request_errors (per FR-BC-3 modal pattern). + + [C: src/gui_2.py:render_ast_inspector_modal (L3740 legacy wrapper)] + """ + try: + content = mcp_client.read_file(f_path) + return Result(data=content) + except Exception as e: + return Result(data=None, errors=[ErrorInfo( + kind=ErrorKind.INTERNAL, + message=f"AST file content read failed for {f_path}: {e}", + source="gui_2._render_ast_inspector_file_content_result", + original=e, + )]) #endregion: Phase 4 Modal/Dialog Result Helpers diff --git a/tests/test_gui_2_result.py b/tests/test_gui_2_result.py index 07fc058b..5bfafec0 100644 --- a/tests/test_gui_2_result.py +++ b/tests/test_gui_2_result.py @@ -698,3 +698,42 @@ def test_phase_4_l3718_render_ast_inspector_outline_result_failure(): err = result.errors[0] assert err.source == "gui_2._render_ast_inspector_outline_result" assert "Error fetching outline" in result.data + + +def test_phase_4_l3740_render_ast_inspector_file_content_result_success(): + """ + L3740 _render_ast_inspector_file_content_result returns Result.ok=True on success. + + The helper wraps the mcp_client.read_file try/except in + render_ast_inspector_modal. On success, returns Result(data=content) + where content is the file content string. The caller sets + app._cached_ast_file_lines and app.text_viewer_content from result.data. + """ + from src import gui_2 + from unittest.mock import MagicMock, patch + app = MagicMock() + with patch.object(gui_2.mcp_client, "read_file", return_value="line1\nline2\nline3") as _rf: + result = gui_2._render_ast_inspector_file_content_result(app, "/proj/foo/src/bar.py") + assert result.ok, f"Expected ok=True on success, got errors: {result.errors}" + assert result.data == "line1\nline2\nline3" + + +def test_phase_4_l3740_render_ast_inspector_file_content_result_failure(): + """ + L3740 _render_ast_inspector_file_content_result returns Result.ok=False on failure. + + When mcp_client.read_file raises, the helper returns Result(data=None, + errors=[ErrorInfo]). The legacy wrapper drains to app._last_request_errors + (per FR-BC-3 modal pattern) and sets app._cached_ast_file_lines to the + fallback error message. + """ + from src import gui_2 + from unittest.mock import MagicMock, patch + app = MagicMock() + with patch.object(gui_2.mcp_client, "read_file", side_effect=RuntimeError("read failed")): + result = gui_2._render_ast_inspector_file_content_result(app, "/proj/foo/src/bar.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._render_ast_inspector_file_content_result" + assert result.data is None