refactor(gui_2): migrate L1284 _handle_history_logic to Result[T] (Phase 3)
TIER-2 READ conductor/code_styleguides/error_handling.md end-to-end before Phase 3. Adds _handle_history_logic_result(app) -> Result[bool] helper that wraps the snapshot debounce try/except from App._handle_history_logic. The _is_applying_snapshot pre-condition guard stays in the legacy wrapper (not error handling; the original early return has no try/except). App._handle_history_logic becomes a thin wrapper that drains errors to _last_request_errors. The drain failure mode is structurally safe (hasattr check + append) so no outer try/except is required (per the L1123 wrapper decision; avoiding new INTERNAL_SILENT_SWALLOW violations). Audit: BROAD_CATCH count 19 -> 18, COMPLIANT count 18 -> 19. Tests: 2/2 pass.
This commit is contained in:
+69
-50
@@ -1231,57 +1231,11 @@ class App:
|
||||
"""Logic for capturing UI state for undo/redo."""
|
||||
if self._is_applying_snapshot:
|
||||
return
|
||||
|
||||
try:
|
||||
# 2. Debounced snapshotting
|
||||
current = self._take_snapshot()
|
||||
if self._last_ui_snapshot is None:
|
||||
self._last_ui_snapshot = current
|
||||
return
|
||||
|
||||
# Compare only core fields for performance
|
||||
changed = (
|
||||
current.ai_input != self._last_ui_snapshot.ai_input or
|
||||
current.project_system_prompt != self._last_ui_snapshot.project_system_prompt or
|
||||
current.global_system_prompt != self._last_ui_snapshot.global_system_prompt or
|
||||
current.base_system_prompt != self._last_ui_snapshot.base_system_prompt or
|
||||
current.use_default_base_prompt != self._last_ui_snapshot.use_default_base_prompt or
|
||||
abs(current.temperature - self._last_ui_snapshot.temperature) > 1e-5 or
|
||||
abs(current.top_p - self._last_ui_snapshot.top_p) > 1e-5 or
|
||||
current.max_tokens != self._last_ui_snapshot.max_tokens or
|
||||
current.auto_add_history != self._last_ui_snapshot.auto_add_history or
|
||||
len(current.disc_entries) != len(self._last_ui_snapshot.disc_entries) or
|
||||
len(current.files) != len(self._last_ui_snapshot.files) or
|
||||
len(current.context_files) != len(self._last_ui_snapshot.context_files) or
|
||||
len(current.screenshots) != len(self._last_ui_snapshot.screenshots)
|
||||
)
|
||||
|
||||
if not changed and len(current.disc_entries) > 0:
|
||||
if current.disc_entries[-1].get('content') != self._last_ui_snapshot.disc_entries[-1].get('content'):
|
||||
changed = True
|
||||
|
||||
if changed:
|
||||
if not self._pending_snapshot:
|
||||
self._pending_snapshot = True
|
||||
self._snapshot_timer = time.time()
|
||||
# Capture state BEFORE current change
|
||||
self._state_to_push = self._last_ui_snapshot
|
||||
else:
|
||||
# Reset timer for settle debounce
|
||||
self._snapshot_timer = time.time()
|
||||
|
||||
self._last_ui_snapshot = current
|
||||
|
||||
if self._pending_snapshot and (time.time() - self._snapshot_timer > self._snapshot_debounce):
|
||||
if self._state_to_push:
|
||||
self.history.push(self._state_to_push, "UI Update")
|
||||
self._state_to_push = None
|
||||
self._pending_snapshot = False
|
||||
except Exception as e:
|
||||
import sys, traceback
|
||||
sys.stderr.write(f"[DEBUG History] ERROR in _handle_history_logic: {e}\n")
|
||||
traceback.print_exc(file=sys.stderr)
|
||||
sys.stderr.flush()
|
||||
result = _handle_history_logic_result(self)
|
||||
if not result.ok:
|
||||
if not hasattr(self, '_last_request_errors'): self._last_request_errors = []
|
||||
self._last_request_errors.append(("_handle_history_logic", result.errors[0]))
|
||||
|
||||
def cb_load_prior_log(self, path: Optional[str] = None) -> None:
|
||||
if path is None:
|
||||
@@ -7583,6 +7537,71 @@ def _show_menus_is_max_result(app: "App", hwnd) -> Result[bool]:
|
||||
original=e,
|
||||
)])
|
||||
|
||||
def _handle_history_logic_result(app: "App") -> Result[bool]:
|
||||
"""Drain-aware variant of L1284 _handle_history_logic snapshot try/except.
|
||||
|
||||
Extracts the snapshot debounce try/except from App._handle_history_logic
|
||||
into a Result-returning helper. The legacy wrapper keeps the
|
||||
_is_applying_snapshot early return (a pre-condition guard, not error
|
||||
handling) and delegates the rest to this helper.
|
||||
|
||||
On success, returns Result(data=True). On failure (any exception
|
||||
during _take_snapshot or the snapshot diff), returns Result(data=False,
|
||||
errors=[ErrorInfo]).
|
||||
|
||||
[C: src/gui_2.py:App._handle_history_logic (L1284 legacy wrapper)]
|
||||
"""
|
||||
try:
|
||||
current = app._take_snapshot()
|
||||
if app._last_ui_snapshot is None:
|
||||
app._last_ui_snapshot = current
|
||||
return Result(data=True)
|
||||
|
||||
changed = (
|
||||
current.ai_input != app._last_ui_snapshot.ai_input or
|
||||
current.project_system_prompt != app._last_ui_snapshot.project_system_prompt or
|
||||
current.global_system_prompt != app._last_ui_snapshot.global_system_prompt or
|
||||
current.base_system_prompt != app._last_ui_snapshot.base_system_prompt or
|
||||
current.use_default_base_prompt != app._last_ui_snapshot.use_default_base_prompt or
|
||||
abs(current.temperature - app._last_ui_snapshot.temperature) > 1e-5 or
|
||||
abs(current.top_p - app._last_ui_snapshot.top_p) > 1e-5 or
|
||||
current.max_tokens != app._last_ui_snapshot.max_tokens or
|
||||
current.auto_add_history != app._last_ui_snapshot.auto_add_history or
|
||||
len(current.disc_entries) != len(app._last_ui_snapshot.disc_entries) or
|
||||
len(current.files) != len(app._last_ui_snapshot.files) or
|
||||
len(current.context_files) != len(app._last_ui_snapshot.context_files) or
|
||||
len(current.screenshots) != len(app._last_ui_snapshot.screenshots)
|
||||
)
|
||||
|
||||
if not changed and len(current.disc_entries) > 0:
|
||||
if current.disc_entries[-1].get('content') != app._last_ui_snapshot.disc_entries[-1].get('content'):
|
||||
changed = True
|
||||
|
||||
if changed:
|
||||
if not app._pending_snapshot:
|
||||
app._pending_snapshot = True
|
||||
app._snapshot_timer = time.time()
|
||||
app._state_to_push = app._last_ui_snapshot
|
||||
else:
|
||||
app._snapshot_timer = time.time()
|
||||
|
||||
app._last_ui_snapshot = current
|
||||
|
||||
if app._pending_snapshot and (time.time() - app._snapshot_timer > app._snapshot_debounce):
|
||||
if app._state_to_push:
|
||||
app.history.push(app._state_to_push, "UI Update")
|
||||
app._state_to_push = None
|
||||
app._pending_snapshot = False
|
||||
|
||||
return Result(data=True)
|
||||
except Exception as e:
|
||||
return Result(data=False, errors=[ErrorInfo(
|
||||
kind=ErrorKind.INTERNAL,
|
||||
message=f"[DEBUG History] ERROR in _handle_history_logic: {e}",
|
||||
source="gui_2._handle_history_logic_result",
|
||||
original=e,
|
||||
)])
|
||||
|
||||
#endregion: Phase 3 Render-Loop Result Helpers
|
||||
|
||||
#endregion: MMA
|
||||
|
||||
@@ -463,4 +463,49 @@ def test_phase_3_l1222_show_menus_is_max_result_failure():
|
||||
assert result.errors, "Expected at least one error on failure"
|
||||
err = result.errors[0]
|
||||
assert err.source == "gui_2._show_menus_is_max_result"
|
||||
assert result.data is False
|
||||
assert result.data is False
|
||||
|
||||
|
||||
def test_phase_3_l1284_handle_history_logic_result_success():
|
||||
"""
|
||||
L1284 _handle_history_logic_result returns Result.ok=True on success.
|
||||
|
||||
The helper wraps the snapshot try/except in App._handle_history_logic.
|
||||
The simplest success path is when _last_ui_snapshot is None (first
|
||||
snapshot, early return) or when nothing changed (no push needed).
|
||||
"""
|
||||
from src import gui_2
|
||||
from unittest.mock import MagicMock
|
||||
app = MagicMock()
|
||||
app._is_applying_snapshot = False
|
||||
app._last_ui_snapshot = None
|
||||
mock_snapshot = MagicMock(name="mock_snapshot")
|
||||
mock_snapshot.disc_entries = []
|
||||
mock_snapshot.files = []
|
||||
mock_snapshot.context_files = []
|
||||
mock_snapshot.screenshots = []
|
||||
app._take_snapshot.return_value = mock_snapshot
|
||||
result = gui_2._handle_history_logic_result(app)
|
||||
assert result.ok, f"Expected ok=True on success, got errors: {result.errors}"
|
||||
assert result.data is True
|
||||
|
||||
|
||||
def test_phase_3_l1284_handle_history_logic_result_failure():
|
||||
"""
|
||||
L1284 _handle_history_logic_result returns Result.ok=False with ErrorInfo on failure.
|
||||
|
||||
When _take_snapshot raises (or any other code in the try body), the
|
||||
helper returns Result(data=False, errors=[ErrorInfo]).
|
||||
"""
|
||||
from src import gui_2
|
||||
from unittest.mock import MagicMock
|
||||
app = MagicMock()
|
||||
app._is_applying_snapshot = False
|
||||
app._last_ui_snapshot = MagicMock()
|
||||
app._take_snapshot.side_effect = ValueError("snapshot failed")
|
||||
result = gui_2._handle_history_logic_result(app)
|
||||
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._handle_history_logic_result"
|
||||
assert "snapshot failed" in err.message
|
||||
Reference in New Issue
Block a user