TIER-2 READ conductor/code_styleguides/error_handling.md end-to-end before Phase 10: refactor(gui_2): migrate L7271 render_task_dag_panel cycle_check to Result[T] (Phase 10 site 12)
Extracted _dag_cycle_check_result(app) -> Result[bool] helper above the call site in render_task_dag_panel. ANTI-SLIMING: full Result[T] propagation (NO except+pass). The helper returns Result(data=has_cycle) on success (True/False) or Result(data=False, errors=[ErrorInfo]) on exception (logging NOT a drain per the user's principle 2026-06-17). The legacy render_task_dag_panel code preserves its signature, calls the helper, opens the 'Cycle Detected!' popup only when the helper returns Result(data=True), and drains errors to app._last_request_errors. Tests: 3 new tests verify no-cycle, cycle-detected, and RuntimeError paths. Audit: L7271 reclassified from INTERNAL_SILENT_SWALLOW (1 site remaining, was 2). New helper L7271 is INTERNAL_COMPLIANT.
This commit is contained in:
+32
-9
@@ -7267,15 +7267,38 @@ def render_task_dag_panel(app: App) -> None: # 4. Task DAG Visualizer
|
||||
ed.end_delete()
|
||||
|
||||
# Validate DAG after any changes
|
||||
#TODO(Ed): Exception(Review)
|
||||
try:
|
||||
from src.dag_engine import TrackDAG #TODO(Ed) Reivew local import
|
||||
ticket_dicts = [{'id': str(t.get('id', '')), 'depends_on': t.get('depends_on', [])} for t in app.active_tickets]
|
||||
temp_dag = TrackDAG(ticket_dicts)
|
||||
if temp_dag.has_cycle():
|
||||
imgui.open_popup("Cycle Detected!")
|
||||
except Exception:
|
||||
pass
|
||||
cycle_result = _dag_cycle_check_result(app)
|
||||
if not cycle_result.ok:
|
||||
if not hasattr(app, '_last_request_errors'): app._last_request_errors = []
|
||||
app._last_request_errors.append(("render_task_dag_panel.cycle_check", cycle_result.errors[0]))
|
||||
elif cycle_result.data:
|
||||
imgui.open_popup("Cycle Detected!")
|
||||
|
||||
def _dag_cycle_check_result(app: "App") -> Result[bool]:
|
||||
"""Drain-aware variant of render_task_dag_panel DAG cycle check (L7271 INTERNAL_SILENT_SWALLOW).
|
||||
|
||||
Extracts the TrackDAG construction + has_cycle check try/except from
|
||||
render_task_dag_panel into a Result-returning helper. On a valid DAG,
|
||||
returns Result(data=False). On a cyclic DAG, returns Result(data=True).
|
||||
On exception (bad ticket dict, dag engine failure), converts to
|
||||
ErrorInfo (logging NOT a drain per the user's principle 2026-06-17).
|
||||
The caller drains to app._last_request_errors.
|
||||
|
||||
[C: src/gui_2.py:render_task_dag_panel (L7271 caller)]
|
||||
"""
|
||||
from src.dag_engine import TrackDAG
|
||||
try:
|
||||
ticket_dicts = [{'id': str(t.get('id', '')), 'depends_on': t.get('depends_on', [])} for t in app.active_tickets]
|
||||
temp_dag = TrackDAG(ticket_dicts)
|
||||
has_cycle = temp_dag.has_cycle()
|
||||
return Result(data=has_cycle)
|
||||
except Exception as e:
|
||||
return Result(data=False, errors=[ErrorInfo(
|
||||
kind=ErrorKind.INTERNAL,
|
||||
message=f"DAG cycle check failed: {e}",
|
||||
source="gui_2._dag_cycle_check_result",
|
||||
original=e,
|
||||
)])
|
||||
|
||||
ed.end()
|
||||
# 5. Add Ticket Form
|
||||
|
||||
@@ -2202,4 +2202,69 @@ def test_phase_10_l6908_tier_stream_scroll_sync_result_failure():
|
||||
assert "missing key" in err.message
|
||||
|
||||
|
||||
def test_phase_10_l7271_dag_cycle_check_result_no_cycle():
|
||||
"""
|
||||
L7271 _dag_cycle_check_result returns Result(data=False) when no cycle is found.
|
||||
|
||||
The helper extracts the TrackDAG() cycle check try/except from
|
||||
render_task_dag_panel into a Result-returning helper. On a valid DAG
|
||||
(no cycle), returns Result(data=False). The caller continues without
|
||||
opening the "Cycle Detected!" popup.
|
||||
"""
|
||||
from unittest.mock import MagicMock, patch
|
||||
import src.gui_2 as gui2_mod
|
||||
app = MagicMock()
|
||||
app.active_tickets = [{"id": "T-001", "depends_on": []}]
|
||||
mock_dag = MagicMock()
|
||||
mock_dag.has_cycle.return_value = False
|
||||
with patch("src.dag_engine.TrackDAG", return_value=mock_dag):
|
||||
result = gui2_mod._dag_cycle_check_result(app)
|
||||
assert result.ok, f"Expected ok=True on success, got errors: {result.errors}"
|
||||
assert result.data is False
|
||||
|
||||
|
||||
def test_phase_10_l7271_dag_cycle_check_result_cycle_detected():
|
||||
"""
|
||||
L7271 _dag_cycle_check_result returns Result(data=True) when a cycle IS found.
|
||||
|
||||
The helper detects cycles via TrackDAG.has_cycle(). On a cyclic DAG,
|
||||
returns Result(data=True). The caller opens the "Cycle Detected!" popup.
|
||||
"""
|
||||
from unittest.mock import MagicMock, patch
|
||||
import src.gui_2 as gui2_mod
|
||||
app = MagicMock()
|
||||
app.active_tickets = [
|
||||
{"id": "T-001", "depends_on": ["T-002"]},
|
||||
{"id": "T-002", "depends_on": ["T-001"]},
|
||||
]
|
||||
mock_dag = MagicMock()
|
||||
mock_dag.has_cycle.return_value = True
|
||||
with patch("src.dag_engine.TrackDAG", return_value=mock_dag):
|
||||
result = gui2_mod._dag_cycle_check_result(app)
|
||||
assert result.ok, f"Expected ok=True on success, got errors: {result.errors}"
|
||||
assert result.data is True
|
||||
|
||||
|
||||
def test_phase_10_l7271_dag_cycle_check_result_failure():
|
||||
"""
|
||||
L7271 _dag_cycle_check_result returns Result(data=False, errors=[ErrorInfo]) on failure.
|
||||
|
||||
When TrackDAG() construction or has_cycle() raises (bad ticket dict,
|
||||
dag engine error), the helper converts to ErrorInfo. The caller
|
||||
drains to app._last_request_errors.
|
||||
"""
|
||||
from unittest.mock import MagicMock, patch
|
||||
import src.gui_2 as gui2_mod
|
||||
app = MagicMock()
|
||||
app.active_tickets = []
|
||||
with patch("src.dag_engine.TrackDAG", side_effect=RuntimeError("dag engine failure")):
|
||||
result = gui2_mod._dag_cycle_check_result(app)
|
||||
assert not result.ok, f"Expected ok=False on failure, got data: {result.data}"
|
||||
assert result.data is False
|
||||
assert result.errors, "Expected at least one error on failure"
|
||||
err = result.errors[0]
|
||||
assert err.source == "gui_2._dag_cycle_check_result"
|
||||
assert "dag engine failure" in err.message
|
||||
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user