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.
This commit is contained in:
+32
-6
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user