Private
Public Access
conductor(plan): add Phase 6 to result_migration_app_controller_20260618
After Tier 2's Phase 3 commit 7fcce652 'migrate 8 INTERNAL_SILENT_SWALLOW
sites', the audit still shows 28 INTERNAL_SILENT_SWALLOW sites in
src/app_controller.py. The 8 sites were renamed with narrower exception
types and given logging.debug bodies — but logging.debug is NOT a drain
point per conductor/code_styleguides/error_handling.md:530:
'narrow except + log (sys.stderr.write / logging.*) only' |
INTERNAL_SILENT_SWALLOW | VIOLATION — logging is NOT a drain
Phase 6 fixes all 28 sites with proper Result[T] propagation:
Sub-phase 6.1: 2 signal handler sites (Pattern 3 drain: os._exit)
Sub-phase 6.2: 2 timeline-event sinks (stderr carry + instance state)
Sub-phase 6.3: 3 GUI state/property setters (Result helper sibling)
Sub-phase 6.4: 1 SDK boundary (_fetch_models.do_fetch)
Sub-phase 6.5: 10 background worker sites (_report_worker_error)
Sub-phase 6.6: 3 per-event handler sites (per-request error list)
Sub-phase 6.7: 6 helper/utility sites (Result propagates upward)
Sub-phase 6.8: audit --strict gate + 28 site tests + report rewrite
Audit gate: uv run python scripts/audit_exception_handling.py --src
src/app_controller.py --strict must exit 0. No logging.debug in
except bodies (verified by grep). Every except body returns
Result(data=..., errors=[ErrorInfo(original=e)]) or reaches a real
drain point (os._exit, stderr carry, instance state for sub-track 4).
Per user reply 2026-06-18: stderr/sys.stderr logging is acceptable
terminal drain until sub-track 4 lands the GUI error display.
Spec.md §12-§21 (addendum); plan.md Phase 6 (8 sub-phases);
state.toml adds 18 t6_* tasks; metadata.json adds 4 verification
criteria + 4 risk_register entries; tracks.md row updated.
Refs:
- docs/reports/TRACK_COMPLETION_result_migration_app_controller_20260618.md
(the Phase 5 report this addendum supersedes)
- conductor/tracks/result_migration_20260616/spec.md (umbrella)
This commit is contained in:
+1
-1
@@ -756,7 +756,7 @@ Lightweight chronology; full spec/plan/state per track is in the linked folder.
|
||||
|---|---|---|---|---|
|
||||
| 1 | `result_migration_review_pass` | S | 57 sites (32 UNCLEAR + 25 INTERNAL_RETHROW) across 15 files | First: human review + audit script heuristic updates inform all later sub-tracks |
|
||||
| 2 | `result_migration_small_files` | L | 37 files (35 SMALL + 2 MEDIUM from `--by-size`); 72 V+S sites | Second: quick wins; doesn't depend on the orchestrator or GUI; can run in parallel with 3-4 |
|
||||
| 3 | `result_migration_app_controller` | XL | 56 sites in `src/app_controller.py` (166KB; 13 FastAPI boundary stay as-is) | Third: high coordination with Hook API + MMA + RAG; gates the GUI migration |
|
||||
| 3 | `result_migration_app_controller` | XL | 56 sites in `src/app_controller.py` (166KB; 13 FastAPI boundary stay as-is) — **Phase 6 added 2026-06-18** to fix the 28 silent-swallow sites that Phase 3's `logging.debug` migration didn't actually migrate (audit gate: `--strict` exits 0) | Third: high coordination with Hook API + MMA + RAG; gates the GUI migration |
|
||||
| 4 | `result_migration_gui_2` | XL | **55 sites** in `src/gui_2.py` (260KB; 14 ? includes the +1 site `src/gui_2.py:1349` from the review pass) | Fourth: depends on 3 for clean API; the largest file |
|
||||
| 5 | `result_migration_baseline_cleanup` | L | 112 sites in 3 refactored files (mcp_client.py, ai_client.py, rag_engine.py) | Fifth: closes the gaps in the convention reference; parent's Path C deferred work |
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
"id": "result_migration_app_controller_20260618",
|
||||
"name": "Result Migration - Sub-Track 3 (App Controller)",
|
||||
"date": "2026-06-18",
|
||||
"phase_6_added": "2026-06-18",
|
||||
"type": "refactor",
|
||||
"priority": "A",
|
||||
"spec": "conductor/tracks/result_migration_app_controller_20260618/spec.md",
|
||||
@@ -26,26 +27,28 @@
|
||||
"conductor/tracks/result_migration_app_controller_20260618/state.toml",
|
||||
"conductor/tracks/result_migration_app_controller_20260618/metadata.json",
|
||||
"conductor/tracks/result_migration_app_controller_20260618/plan.md",
|
||||
"conductor/tracks/result_migration_app_controller_20260618/spec.md",
|
||||
"conductor/tracks/result_migration_20260616/spec.md"
|
||||
],
|
||||
"deleted_files": []
|
||||
},
|
||||
"verification_criteria": [
|
||||
"src/app_controller.py has zero INTERNAL_BROAD_CATCH sites (32 migrated)",
|
||||
"src/app_controller.py has zero INTERNAL_SILENT_SWALLOW sites (8 migrated with logging.debug per Heuristic #19)",
|
||||
"src/app_controller.py has zero INTERNAL_RETHROW sites (4 classified: legitimate patterns stay; SUSPICIOUS migrated)",
|
||||
"src/app_controller.py has zero INTERNAL_OPTIONAL_RETURN sites (1 migrated to Result[int] or nil-sentinel)",
|
||||
"src/app_controller.py has zero INTERNAL_BROAD_CATCH sites (32 migrated in Phase 2)",
|
||||
"src/app_controller.py has zero INTERNAL_SILENT_SWALLOW sites (28 properly migrated in Phase 6 with Result[T] propagation; no logging.debug anti-pattern per error_handling.md:530)",
|
||||
"src/app_controller.py has zero INTERNAL_RETHROW sites (4 classified in Phase 4 as legitimate Pattern 1/3; stay as-is)",
|
||||
"src/app_controller.py has zero INTERNAL_OPTIONAL_RETURN sites (1 migrated to Result[float] in Phase 4)",
|
||||
"src/app_controller.py preserves 15 BOUNDARY_FASTAPI sites (unchanged, per styleguide Boundary Types section)",
|
||||
"src/app_controller.py preserves 2 BOUNDARY_SDK sites (unchanged, per styleguide Boundary Types section)",
|
||||
"src/app_controller.py preserves 4 INTERNAL_COMPLIANT sites (unchanged, already compliant)",
|
||||
"src/app_controller.py preserves 1 INTERNAL_PROGRAMMER_RAISE site (unchanged, per Fail Early pattern)",
|
||||
"tests/test_app_controller_result.py exists with 5+ tests, all pass",
|
||||
"tests/test_app_controller_offloading.py has 2 new unwrap-path tests, all pass",
|
||||
"tests/test_tool_presets_execution::test_tool_ask_approval passes (Regression 1 fixed)",
|
||||
"tests/test_extended_sims::test_execution_sim_live passes (Regression 2 fixed)",
|
||||
"uv run python scripts/audit_exception_handling.py --by-size shows 0 INTERNAL_BROAD_CATCH and 0 INTERNAL_SILENT_SWALLOW for src/app_controller.py",
|
||||
"uv run python scripts/run_tests_batched.py shows no new regressions (882 passed / 17 skipped / 2 xfailed, with the 2 previously-failing tests now passing)",
|
||||
"docs/reports/TRACK_COMPLETION_result_migration_app_controller_20260618.md exists with the 7 standard sections"
|
||||
"tests/test_app_controller_result.py exists with 5+ tests, all pass (extended with 28 Phase 6 site tests)",
|
||||
"tests/test_app_controller_offloading.py has 2 unwrap-path tests, all pass",
|
||||
"tests/test_tool_presets_execution::test_tool_ask_approval passes (Regression 1 fixed in Phase 1)",
|
||||
"tests/test_extended_sims::test_execution_sim_live passes (Regression 2 fixed in Phase 1 + verified environmentally dependent)",
|
||||
"uv run python scripts/audit_exception_handling.py --src src/app_controller.py --strict exits 0 (Phase 6 hard gate)",
|
||||
"uv run python scripts/audit_exception_handling.py --src src/app_controller.py --json shows 0 sites in INTERNAL_SILENT_SWALLOW category",
|
||||
"uv run python scripts/run_tests_batched.py shows no new regressions (890 passed / 17 skipped / 2 xfailed, matching Tier 2's pre-Phase-6 baseline)",
|
||||
"Every migrated except body contains Result(data=..., errors=[ErrorInfo(original=e)]) (verified by grep - no debug-log-only except bodies)",
|
||||
"docs/reports/TRACK_COMPLETION_result_migration_app_controller_20260618.md rewritten with full Phase 1-6 coverage; the misleading '8 silent swallow migrated' claim from Phase 5 is superseded"
|
||||
],
|
||||
"regressions_and_pre_existing_failures": [
|
||||
{
|
||||
@@ -103,6 +106,26 @@
|
||||
"risk": "The 2 known regressions (test_tool_ask_approval, test_execution_sim_live) may have additional root causes beyond the log_tool_call half-migration",
|
||||
"likelihood": "low",
|
||||
"mitigation": "Phase 1 task 1.3 is the regression fix; if the tests still fail after the fix, the implementation investigates before Phase 2 begins (do not loop; read code, predict, fix once, report)"
|
||||
},
|
||||
{
|
||||
"risk": "Phase 6: Tier 2 may repeat the Phase 3 deferral pattern (using logging.debug as a 'migration' that the audit still flags as silent swallow)",
|
||||
"likelihood": "medium",
|
||||
"mitigation": "The audit gate in FR12 (--strict exits 1 on any violation) is the hard verification. If FR12 fails, the track is not complete regardless of how many sites are touched."
|
||||
},
|
||||
{
|
||||
"risk": "Phase 6: Some sites may need their callers updated to receive Result[T] instead of T (e.g., _update_inject_preview)",
|
||||
"likelihood": "medium",
|
||||
"mitigation": "Each task identifies its caller chain via py_find_usages and updates all callers in the same commit. For property setters (which can't return values), the migration uses a sibling _result helper pattern."
|
||||
},
|
||||
{
|
||||
"risk": "Phase 6: The 20 nested sites introduced by Phase 2 may have been overwritten by Phase 3's logging.debug add",
|
||||
"likelihood": "medium",
|
||||
"mitigation": "The migration must remove the logging.debug AND replace with Result return (not add a Result on top of the logging). The audit --strict gate catches any leftover logging-only bodies."
|
||||
},
|
||||
{
|
||||
"risk": "Phase 6: Scope (28 sites) is large; Phase 6 may itself need a follow-up Phase 7 if any site resists migration",
|
||||
"likelihood": "low",
|
||||
"mitigation": "Phase 6 is bounded by 8 sub-phases with concrete drain-point patterns. If a site resists migration (e.g., a function with side effects that cannot return Result), the user explicitly carves it out; no Tier 2-initiated 'follow-up' deferrals are allowed."
|
||||
}
|
||||
]
|
||||
}
|
||||
|
||||
@@ -273,6 +273,189 @@ Focus: confirm all 45 migration-target sites are migrated; re-run batched suite;
|
||||
|
||||
---
|
||||
|
||||
## End-of-Track Report (added 2026-06-17 convention)
|
||||
## Phase 6 Addendum: Proper `Result[T]` migration of the 28 INTERNAL_SILENT_SWALLOW sites
|
||||
|
||||
On Phase 5 completion, write `docs/reports/TRACK_COMPLETION_result_migration_app_controller_20260618.md` and update `conductor/tracks/result_migration_app_controller_20260618/state.toml` to `status = "completed"`.
|
||||
Focus: replace every `except ...: logging.debug(...); <local side effect>` body with proper `Result[T]` propagation. The 8 sites that Phase 3 "migrated" with `logging.debug` did not satisfy the convention (per `error_handling.md:530` — logging is NOT a drain). Phase 6 fixes all 28 sites with real `Result` propagation + real drain points.
|
||||
|
||||
**Audit gate:** `uv run python scripts/audit_exception_handling.py --src src/app_controller.py --strict` exits 0.
|
||||
|
||||
**Pattern reference (per `error_handling.md:530`):** A `logging.*` call in an except body is `INTERNAL_SILENT_SWALLOW` (a violation). The only acceptable patterns are:
|
||||
1. Return `Result(data=..., errors=[ErrorInfo(original=e)])` from the function
|
||||
2. Reach a real drain point: HTTPException (Pattern 1), GUI display (Pattern 2), os._exit (Pattern 3), telemetry emission (Pattern 4), bounded retry (Pattern 5)
|
||||
|
||||
### Sub-phase 6.1: Signal handlers (Pattern 3 drain: os._exit) — 2 sites
|
||||
|
||||
**Task 6.1.1: Migrate `_on_sigint` (L772) and `_install_sigint_exit_handler` (L777)**
|
||||
- **WHERE:** `src/app_controller.py:769-778`
|
||||
- **WHAT:** Extract `_shutdown_io_pool_result(self) -> Result[None]` helper. The inner signal handler `_on_sigint` calls the helper and:
|
||||
```python
|
||||
def _on_sigint(signum, frame):
|
||||
result = controller._shutdown_io_pool_result()
|
||||
if not result.ok:
|
||||
sys.stderr.write(f"FATAL: {result.errors[0].ui_message()}\n")
|
||||
sys.stderr.flush()
|
||||
os._exit(0) # Pattern 3 drain: intentional termination
|
||||
```
|
||||
The outer `_install_sigint_exit_handler` becomes `_install_signal_handler_result(self) -> Result[None]`; the function call site at `AppController.__init__` (L828) stores `self._signal_handler_error = result.errors[0] if not result.ok else None`.
|
||||
- **SAFETY:** Signal handlers cannot return values to callers; the `os._exit(0)` is the terminal drain. The stderr write before exit is part of the termination pattern (Heuristic D match for Pattern 3).
|
||||
- **VERIFY:** New tests `test_on_sigint_writes_stderr_on_io_pool_failure` + `test_install_signal_handler_stores_error_on_failure`. Run audit.
|
||||
- **COMMIT:** `refactor(app_controller): migrate 2 signal handler sites to Result (Pattern 3 drain via os._exit)`
|
||||
- **GIT NOTE:** Replaces Phase 3's `logging.debug` add at L772/L777 with proper Result propagation.
|
||||
|
||||
### Sub-phase 6.2: Event sinks / one-shot best-effort logging — 2 sites
|
||||
|
||||
**Task 6.2.1: Migrate `mark_first_frame_rendered` (L1315) and `_on_warmup_complete_for_timeline` (L1411)**
|
||||
- **WHERE:** `src/app_controller.py:1294-1316` and `:1396-1412`
|
||||
- **WHAT:** Extract `_log_startup_timeline_event_result(self, event_kind: str) -> Result[None]` helper. Both functions call the helper instead of inline `sys.stderr.write + logging.debug`. The errors are appended to `self._startup_timeline_errors: list[ErrorInfo]` for sub-track 4's GUI display; the helper itself writes to stderr (user-confirmed acceptable terminal drain until sub-track 4).
|
||||
- **SAFETY:** These are event sinks (called once per app lifecycle event). The helper preserves the original stderr output for humans tailing the logs.
|
||||
- **VERIFY:** New tests `test_mark_first_frame_carries_error_in_state` + `test_warmup_complete_carries_error_in_state`. Run audit.
|
||||
- **COMMIT:** `refactor(app_controller): migrate 2 timeline-event sites to Result (event sink with stderr carry)`
|
||||
- **GIT NOTE:** Replaces Phase 3's `logging.debug` add at L1315/L1411 with proper Result propagation + instance state carry.
|
||||
|
||||
### Sub-phase 6.3: GUI state setters / property setters — 3 sites
|
||||
|
||||
**Task 6.3.1: Migrate `_update_inject_preview` (L1456)**
|
||||
- **WHERE:** `src/app_controller.py:1430-1458`
|
||||
- **WHAT:** Function becomes `_update_inject_preview_result(self) -> Result[str]`. Caller (gui_2.py render fn, deferred to sub-track 4) checks `.ok`. Until then, the immediate caller in `gui_2.py` (find via `py_find_usages src.app_controller.AppController._update_inject_preview`) writes `result.errors[0].ui_message()` to stderr. In `app_controller.py`, add a thin wrapper `_update_inject_preview(self) -> None` that calls `_update_inject_preview_result` and stores `self._inject_preview_error: ErrorInfo | None`; the legacy call sites still work.
|
||||
- **VERIFY:** New test `test_update_inject_preview_returns_result_with_error_on_read_failure`.
|
||||
- **COMMIT:** `refactor(app_controller): _update_inject_preview returns Result[str] (silent swallow site 1)`
|
||||
|
||||
**Task 6.3.2: Migrate `mcp_config_json` setter (L1604)**
|
||||
- **WHERE:** `src/app_controller.py:1599-1606`
|
||||
- **WHAT:** Add sibling `_set_mcp_config_json_result(self, value: str) -> Result[None]`. The property setter becomes:
|
||||
```python
|
||||
@mcp_config_json.setter
|
||||
def mcp_config_json(self, value: str) -> None:
|
||||
result = self._set_mcp_config_json_result(value)
|
||||
if not result.ok:
|
||||
self._mcp_config_parse_error = result.errors[0]
|
||||
sys.stderr.write(f"mcp_config parse failed: {result.errors[0].ui_message()}\n")
|
||||
sys.stderr.flush()
|
||||
```
|
||||
- **VERIFY:** New test `test_mcp_config_setter_stores_error_on_parse_failure`.
|
||||
- **COMMIT:** `refactor(app_controller): mcp_config_json setter returns Result via sibling helper (silent swallow site 2)`
|
||||
|
||||
**Task 6.3.3: Migrate `_save_active_project` (L3024)**
|
||||
- **WHERE:** `src/app_controller.py:3016-3027`
|
||||
- **WHAT:** Function becomes `_save_active_project_result(self) -> Result[None]`. The wrapper `_save_active_project(self) -> None` calls the result variant; on failure, stores `self._save_project_error: ErrorInfo | None` and writes to stderr.
|
||||
- **VERIFY:** New test `test_save_active_project_returns_result_with_error_on_io_failure`.
|
||||
- **COMMIT:** `refactor(app_controller): _save_active_project returns Result[None] (silent swallow site 3)`
|
||||
|
||||
### Sub-phase 6.4: SDK boundary — 1 site
|
||||
|
||||
**Task 6.4.1: Migrate `_fetch_models.do_fetch` (L3173)**
|
||||
- **WHERE:** `src/app_controller.py:3168-3190`
|
||||
- **WHAT:** Add `_list_models_for_provider_result(self, p: str) -> Result[list]` helper that wraps `ai_client.list_models(p)` and converts SDK exceptions to `ErrorInfo(kind=ErrorKind.NETWORK/PERMISSION/AUTH, ...)`. The `do_fetch` function accumulates per-provider results in `self._model_fetch_errors: dict[str, ErrorInfo]` and returns `Result[None]` with the aggregated errors. Per-provider failures don't block the overall fetch (the user can still see models from providers that worked).
|
||||
- **SAFETY:** SDK boundary (the `ai_client.list_models()` call) is the right place to catch and convert per `error_handling.md` §"Boundary Types".
|
||||
- **VERIFY:** New test `test_fetch_models_aggregates_per_provider_errors_into_result`.
|
||||
- **COMMIT:** `refactor(app_controller): _fetch_models.do_fetch accumulates per-provider Result (SDK boundary)`
|
||||
|
||||
### Sub-phase 6.5: Background workers / threads — 10 sites
|
||||
|
||||
**Task 6.5.1: Migrate `_handle_compress_discussion.worker` (L3532) and the 2 other `worker` closures (L3570, L3642)**
|
||||
- **WHERE:** `src/app_controller.py:3471-3535`, `:3535-3542`, `:3542-3570` (or wherever the 3 `worker` keyword closures live)
|
||||
- **WHAT:** Each `worker` closure returns `Result[None]`. The outer function that calls `self.submit_io(worker)` wraps with a completion handler that checks `result.ok`; on failure, calls `_report_worker_error(op_name, result)` which writes to stderr and appends to `self._worker_errors: list[tuple[str, ErrorInfo]]` (Pattern 4 telemetry drain — `self._worker_errors` is the in-process telemetry buffer; sub-track 4 forwards to GUI).
|
||||
- **SAFETY:** Background threads; the worker closures cannot mutate shared state without locks. The `_report_worker_error` helper uses `self._worker_errors_lock` (new lock) for append.
|
||||
- **VERIFY:** New tests `test_worker_reports_error_via_result_on_failure` (one per worker site, parameterized).
|
||||
- **COMMIT:** `refactor(app_controller): 3 worker closures return Result and report errors via _report_worker_error (sub-batch 1)`
|
||||
|
||||
**Task 6.5.2: Migrate `_bg_task` (L4175, L4204, L4207)**
|
||||
- **WHERE:** `src/app_controller.py:4175, 4204, 4207`
|
||||
- **WHAT:** Same pattern as 6.5.1. The 3 sites in `_bg_task` each become `Result[None]`-returning sub-tasks; the wrapper calls `_report_worker_error` on each failure.
|
||||
- **VERIFY:** New test `test_bg_task_reports_error_via_result_on_failure` (parameterized over the 3 sites).
|
||||
- **COMMIT:** `refactor(app_controller): _bg_task 3 sites return Result (sub-batch 2)`
|
||||
|
||||
**Task 6.5.3: Migrate `_start_track_logic` (L4300, L4346)**
|
||||
- **WHERE:** `src/app_controller.py:4300, 4346`
|
||||
- **WHAT:** Same pattern. The function returns `Result[None]`; on per-step failure, the error is appended to `self._track_logic_errors` (Pattern 4 telemetry).
|
||||
- **VERIFY:** New test `test_start_track_logic_returns_result_with_error_on_topological_sort_failure`.
|
||||
- **COMMIT:** `refactor(app_controller): _start_track_logic returns Result (sub-batch 3)`
|
||||
|
||||
**Task 6.5.4: Migrate `_cb_run_conductor_setup` (L4459) and `_cb_load_track` (L4557)**
|
||||
- **WHERE:** `src/app_controller.py:4459, 4557`
|
||||
- **WHAT:** Same pattern. Each callback returns `Result[None]`; errors reported via `_report_worker_error`.
|
||||
- **VERIFY:** New tests for both.
|
||||
- **COMMIT:** `refactor(app_controller): _cb_run_conductor_setup + _cb_load_track return Result (sub-batch 4)`
|
||||
|
||||
### Sub-phase 6.6: Per-event handlers — 3 sites
|
||||
|
||||
**Task 6.6.1: Migrate `_handle_request_event` RAG + symbol resolution (L3736, L3750)**
|
||||
- **WHERE:** `src/app_controller.py:3736, 3750`
|
||||
- **WHAT:** Add `_rag_search_result(self, query: str) -> Result[str]` and `_symbol_resolution_result(self, user_msg: str, file_paths: list) -> Result[str]` helpers. The handler accumulates errors into `self._last_request_errors: list[ErrorInfo]` (drained at end of handler via stderr write + instance state carry for sub-track 4).
|
||||
- **VERIFY:** New tests `test_handle_request_event_carries_rag_error_in_state` + `test_handle_request_event_carries_symbol_error_in_state`.
|
||||
- **COMMIT:** `refactor(app_controller): _handle_request_event RAG + symbol sites return Result (event handler)`
|
||||
|
||||
**Task 6.6.2: Migrate `_process_pending_gui_tasks` per-task try (L1707)**
|
||||
- **WHERE:** `src/app_controller.py:1695-1710`
|
||||
- **WHAT:** The per-task execution becomes a `_execute_gui_task_result(self, task) -> Result[None]` helper. The loop accumulates per-task errors into `self._gui_task_errors: list[tuple[dict, ErrorInfo]]` (one entry per failed task). At end of processing, stderr summary + instance state carry.
|
||||
- **VERIFY:** New test `test_process_pending_gui_tasks_carries_per_task_errors_in_state`.
|
||||
- **COMMIT:** `refactor(app_controller): _process_pending_gui_tasks per-task try returns Result`
|
||||
|
||||
### Sub-phase 6.7: Helpers / utilities (Result propagates upward) — 6 sites
|
||||
|
||||
**Task 6.7.1: Migrate `replace_ref` (L1986)**
|
||||
- **WHERE:** `src/app_controller.py:1986`
|
||||
- **WHAT:** Function becomes `replace_ref_result(content: str, ref: str, replacement: str) -> Result[str]`. Caller (the next-level utility) checks `.ok` and propagates.
|
||||
- **VERIFY:** New test `test_replace_ref_returns_result_with_error_on_string_failure`.
|
||||
- **COMMIT:** `refactor(app_controller): replace_ref returns Result[str] (helper) `
|
||||
|
||||
**Task 6.7.2: Migrate `cb_load_prior_log.token_history` (L2128)**
|
||||
- **WHERE:** `src/app_controller.py:2128`
|
||||
- **WHAT:** The try block becomes `_parse_token_history_ts_result(item: dict) -> Result[float]`. The `cb_load_prior_log` wrapper (which already returns `Result[None]`) checks `.ok` and merges errors via `.with_errors([...])`.
|
||||
- **VERIFY:** New test `test_cb_load_prior_log_propagates_token_history_parse_error`.
|
||||
- **COMMIT:** `refactor(app_controller): cb_load_prior_log token_history site returns Result`
|
||||
|
||||
**Task 6.7.3: Migrate `_load_active_project` primary + fallback (L2195, L2210)**
|
||||
- **WHERE:** `src/app_controller.py:2195, 2210`
|
||||
- **WHAT:** The inner try blocks become `_load_project_from_path_result(path: str) -> Result[Project]`. The outer `_load_active_project` (already returns `Result[None]`) iterates, collects `Result` from each, and merges via `.with_errors([...])` so the caller knows there was a partial failure (the fallback worked, but the primary didn't).
|
||||
- **VERIFY:** New tests `test_load_active_project_carries_partial_failure_error` + `test_load_active_project_fallback_loop_returns_result`.
|
||||
- **COMMIT:** `refactor(app_controller): _load_active_project primary + fallback return Result (helpers)`
|
||||
|
||||
**Task 6.7.4: Migrate `queue_fallback` (L2454)**
|
||||
- **WHERE:** `src/app_controller.py:2448-2457`
|
||||
- **WHAT:** The inner try becomes `_run_pending_tasks_once_result(self) -> Result[None]`. The `queue_fallback` outer loop checks `.ok`; on failure, logs to stderr and continues the loop (the fallback IS the bounded retry Pattern 5 drain).
|
||||
- **VERIFY:** New test `test_queue_fallback_returns_result_on_per_iteration_failure`.
|
||||
- **COMMIT:** `refactor(app_controller): queue_fallback per-iteration try returns Result (bounded retry drain)`
|
||||
|
||||
**Task 6.7.5: Migrate `_refresh_from_project.active_track` (L2969)**
|
||||
- **WHERE:** `src/app_controller.py:2969`
|
||||
- **WHAT:** The try block becomes `_deserialize_active_track_result(at_data: dict) -> Result[Track]`. The outer `_refresh_from_project` (already returns `Result[None]`) merges errors via `.with_errors([...])`.
|
||||
- **VERIFY:** New test `test_refresh_from_project_propagates_active_track_deserialize_error`.
|
||||
- **COMMIT:** `refactor(app_controller): _refresh_from_project active_track deserialize returns Result`
|
||||
|
||||
### Sub-phase 6.8: Tests + verification — 1 audit gate
|
||||
|
||||
**Task 6.8.1: Run audit and verify zero INTERNAL_SILENT_SWALLOW sites**
|
||||
- **COMMAND:** `uv run python scripts/audit_exception_handling.py --src src/app_controller.py --strict`
|
||||
- **EXPECT:** Exit 0; output shows 0 sites in INTERNAL_SILENT_SWALLOW category.
|
||||
- **COMMIT:** No commit (verification only).
|
||||
- **NOTE:** If exit 1, identify the leftover sites and add tasks to Phase 6 (do not declare Phase 6 complete).
|
||||
|
||||
**Task 6.8.2: Run full batched suite**
|
||||
- **COMMAND:** `uv run python scripts/run_tests_batched.py`
|
||||
- **EXPECT:** Same 890 passed / 17 skipped / 2 xfailed as Tier 2's pre-Phase-6 baseline.
|
||||
- **COMMIT:** No commit (verification only).
|
||||
- **NOTE:** If new failures appear, fix forward (do not loop; read code, predict, fix once, report).
|
||||
|
||||
**Task 6.8.3: Add audit-heuristic test for the strict gate**
|
||||
- **WHERE:** `tests/test_audit_exception_handling_heuristics.py` (extend existing)
|
||||
- **WHAT:** Add `test_app_controller_post_phase6_has_zero_silent_swallow` — asserts the audit's per-category count for `src/app_controller.py` is 0 for INTERNAL_SILENT_SWALLOW.
|
||||
- **VERIFY:** The new test passes.
|
||||
- **COMMIT:** `test(audit): add post-Phase-6 silent-swallow assertion for app_controller`
|
||||
- **GIT NOTE:** Locks in the Phase 6 invariant.
|
||||
|
||||
**Task 6.8.4: Phase 6 checkpoint commit**
|
||||
- **COMMIT:** `conductor(plan): mark Phase 6 complete (28 silent swallow sites properly migrated)`
|
||||
- **GIT NOTE:** Phase 6 = 28 silent swallow sites migrated with proper Result[T] propagation; audit shows 0 INTERNAL_SILENT_SWALLOW for `src/app_controller.py`.
|
||||
|
||||
### Task 6.8.5: Rewrite the end-of-track report
|
||||
- **WHERE:** `docs/reports/TRACK_COMPLETION_result_migration_app_controller_20260618.md` (overwrite — the old report was misleading)
|
||||
- **WHAT:** Full rewrite covering ALL 6 phases (1-6), the audit deltas (45 → 0 migration sites; 28 silent swallows now properly propagated), the 2 regressions fixed (Phase 1), the 4 INTERNAL_RETHROW classified (Phase 4), the cold_start_ts migration (Phase 4), and the 28 silent swallow rewrites (Phase 6). 7 sections (Header, Tasks completed, Audit results, Last 3 failures, Files modified, Git state, Recommendation).
|
||||
- **COMMIT:** `docs(reports): TRACK_COMPLETION_result_migration_app_controller_20260618 (full rewrite; covers Phase 6)`
|
||||
- **GIT NOTE:** End-of-track report rewritten to reflect Phase 6's corrections; the previous report's claims about "8 silent swallows migrated" are superseded.
|
||||
|
||||
---
|
||||
|
||||
## End-of-Track Report (added 2026-06-17 convention; rewritten per Phase 6)
|
||||
|
||||
On Phase 6 completion, rewrite `docs/reports/TRACK_COMPLETION_result_migration_app_controller_20260618.md` to cover all 6 phases. Update `conductor/tracks/result_migration_app_controller_20260618/state.toml` to `status = "completed"`, `current_phase = 6`.
|
||||
|
||||
@@ -308,3 +308,171 @@ The post-track batched suite: same 882 passed / 17 skipped / 2 xfailed (the 1 cu
|
||||
- **R3:** The 1 `INTERNAL_OPTIONAL_RETURN` site (L1358 `cold_start_ts`) has multiple call sites. The shape (nil-sentinel vs Result) depends on how the call sites use the value. Tier 2 reads the call sites and picks the right shape.
|
||||
- **R4:** The `log_tool_call` call site in `_offload_entry_payload` (FR5) is the regression that's blocking the batched suite. It's also the FIRST thing Tier 2 should fix (in Phase 1 Task 1.x) to unblock the regression check.
|
||||
- **R5:** Scope is larger than the umbrella estimated (45 vs 22 migration). Mitigation: the umbrella spec is updated post-track to reflect the actual count; the audit's per-category output is the source of truth, not the umbrella's T-shirt-size estimate.
|
||||
|
||||
---
|
||||
|
||||
# Phase 6 Addendum (added 2026-06-18 — post Tier 2 commit b7d3d9a4)
|
||||
|
||||
## 12. Why Phase 6 exists
|
||||
|
||||
After Tier 2's commit `7fcce652` (Phase 3 "8 silent swallow sites migrated"), the audit still shows **28 INTERNAL_SILENT_SWALLOW sites** in `src/app_controller.py`. The 8 "spec-estimated" sites were renamed with narrower exception types and given `logging.debug(...)` bodies — but the audit correctly classifies them as `INTERNAL_SILENT_SWALLOW` because:
|
||||
|
||||
> `narrow except + log (sys.stderr.write / logging.*) only` | `INTERNAL_SILENT_SWALLOW` | **Violation** — **logging is NOT a drain**. The user's principle (2026-06-17) explicitly states: `sys.stderr.write` / `logging.error` / `logger.exception` / `traceback.print_exc` alone is NOT a drain point. Use `Result[T]` propagation to a true drain point. (per `error_handling.md:530`, audit hint matches `result_migration_small_files_20260617` Phase 12.1)
|
||||
|
||||
The additional 20 nested sites were introduced by Phase 2's bulk migrations (some try blocks have multiple except clauses; the outer one was migrated to `Result`, the inner ones are `except: pass` or `except: log`). Per the convention, all 28 sites need proper `Result[T]` propagation with `ErrorInfo(original=e)` carrying the swallowed exception to a real drain point.
|
||||
|
||||
## 13. Current state of `src/app_controller.py` (post-Phase-5, audit baseline for Phase 6)
|
||||
|
||||
```
|
||||
src\app_controller.py (V=28, S=4, ?=0, C=36, total=68)
|
||||
INTERNAL_SILENT_SWALLOW 28 <-- Phase 6 target
|
||||
INTERNAL_COMPLIANT 17
|
||||
BOUNDARY_FASTAPI 15 (boundary; stays)
|
||||
INTERNAL_RETHROW 4 (Phase 4 classified as Pattern 1/3 legitimate; stays)
|
||||
BOUNDARY_SDK 2 (boundary; stays)
|
||||
BOUNDARY_CONVERSION 1 (Phase 1's _offload_entry_payload fix; stays)
|
||||
INTERNAL_PROGRAMMER_RAISE 1 (programmer error; stays)
|
||||
```
|
||||
|
||||
**Note:** Phase 6 does NOT regress the 4 INTERNAL_RETHROW sites (they're legitimate per Phase 4) or the 1 INTERNAL_OPTIONAL_RETURN site (`cold_start_ts` was migrated to `Result[float]` in Phase 4; the audit now classifies it as INTERNAL_COMPLIANT).
|
||||
|
||||
## 14. The 28 Phase 6 sites grouped by drain-point pattern
|
||||
|
||||
Per `error_handling.md` §"The 5 drain point patterns" and §"Boundary types vs. drain points", each site is migrated with its drain point identified. The user has confirmed (per session reply 2026-06-18): stderr/sys.stderr logging is an acceptable terminal drain until sub-track 4 (`result_migration_gui_2`) lands the GUI-side error display.
|
||||
|
||||
### Group 6.1 — Signal handlers (drain: `os._exit` Pattern 3)
|
||||
- `src/app_controller.py:772` `_on_sigint` (inner closure)
|
||||
- `src/app_controller.py:777` `_install_sigint_exit_handler` (outer)
|
||||
|
||||
**Migration:** Extract `_shutdown_io_pool_result() -> Result[None]` and `_install_signal_handler_result() -> Result[None]` helpers. The signal handler calls the helper; if `not result.ok`, writes `result.errors[0].ui_message()` to `sys.stderr`; then `os._exit(0)`. The `os._exit(0)` IS the drain point (Pattern 3 — intentional app termination). The stderr write is part of the termination pattern (Heuristic D match).
|
||||
|
||||
### Group 6.2 — Event sinks / one-shot best-effort logging (drain: stderr + carry in instance state)
|
||||
- `src/app_controller.py:1315` `mark_first_frame_rendered`
|
||||
- `src/app_controller.py:1411` `_on_warmup_complete_for_timeline`
|
||||
|
||||
**Migration:** Replace `logging.debug` with `_log_startup_timeline_result() -> Result[None]`. The caller (event sink) carries errors in `self._startup_timeline_errors: list[ErrorInfo]`; stderr logs each error (user-confirmed acceptable terminal sink until sub-track 4). The instance state is the data plane; the stderr write is the visible-but-incomplete drain (full drain = GUI display in sub-track 4).
|
||||
|
||||
### Group 6.3 — GUI state setters / property setters (drain: stderr + carry in instance state)
|
||||
- `src/app_controller.py:1456` `_update_inject_preview`
|
||||
- `src/app_controller.py:1604` `mcp_config_json` setter
|
||||
- `src/app_controller.py:3024` `_save_active_project`
|
||||
|
||||
**Migration:** Function returns `Result[T]`. Caller (`gui_2.py` render fns) checks `.ok` and opens an error modal — BUT until sub-track 4, the caller writes the error to `sys.stderr` and stores the error on instance state for sub-track 4 to consume. For `mcp_config_json` property setter (Python property setters cannot return values), add a sibling `_set_mcp_config_result(value) -> Result[None]` that stores `self._mcp_config_parse_error: ErrorInfo | None`. The setter is a thin wrapper: `result = self._set_mcp_config_result(value); if not result.ok: self._mcp_config_parse_error = result.errors[0]; sys.stderr.write(result.errors[0].ui_message())`.
|
||||
|
||||
### Group 6.4 — SDK boundary (drain: stderr + instance state)
|
||||
- `src/app_controller.py:3173` `_fetch_models.do_fetch`
|
||||
|
||||
**Migration:** Wrap `ai_client.list_models()` calls in `_list_models_for_provider_result(p) -> Result[list]`. The per-provider failures are accumulated in `self._model_fetch_errors: dict[str, ErrorInfo]`. The overall function returns `Result[None]` carrying the aggregated errors. Caller writes to stderr + stores in instance state for sub-track 4.
|
||||
|
||||
### Group 6.5 — Background workers / threads (drain: stderr + telemetry state)
|
||||
- `src/app_controller.py:3532` `_handle_compress_discussion.worker` (the inner `try/except`)
|
||||
- `src/app_controller.py:3570` (next `worker` closure; per `worker` keyword)
|
||||
- `src/app_controller.py:3642` (next `worker` closure)
|
||||
- `src/app_controller.py:4175, 4204, 4207` `_bg_task`
|
||||
- `src/app_controller.py:4300, 4346` `_start_track_logic`
|
||||
- `src/app_controller.py:4459` `_cb_run_conductor_setup`
|
||||
- `src/app_controller.py:4557` `_cb_load_track`
|
||||
|
||||
**Migration:** The worker function returns `Result[None]`. The `self.submit_io(worker)` caller wraps with a completion handler that checks `result.ok`; on failure, calls `_report_worker_error(op_name, result)` which writes to `sys.stderr` (user-confirmed terminal sink) and appends to `self._worker_errors: list[tuple[str, ErrorInfo]]` for telemetry (Pattern 4 drain — telemetry emission is a real drain per `error_handling.md:421`).
|
||||
|
||||
### Group 6.6 — Per-event handlers (drain: stderr + per-request state)
|
||||
- `src/app_controller.py:3736, 3750` `_handle_request_event` (RAG search + symbol resolution)
|
||||
- `src/app_controller.py:1707` `_process_pending_gui_tasks` (per-task try)
|
||||
|
||||
**Migration:** Each sub-operation gets a `_result` helper. Handler accumulates errors into a per-request list. At end of handler, if errors, calls `_drain_request_errors(errors)` which writes to stderr + stores in `self._last_request_errors: list[ErrorInfo]` for the GUI to display in the next render frame (sub-track 4 surfaces it).
|
||||
|
||||
### Group 6.7 — Helpers / utilities (drain: Result propagates upward)
|
||||
- `src/app_controller.py:1986` `replace_ref`
|
||||
- `src/app_controller.py:2128` `cb_load_prior_log.token_history`
|
||||
- `src/app_controller.py:2195` `_load_active_project.primary`
|
||||
- `src/app_controller.py:2210` `_load_active_project.fallback_loop`
|
||||
- `src/app_controller.py:2454` `queue_fallback`
|
||||
- `src/app_controller.py:2969` `_refresh_from_project.active_track`
|
||||
|
||||
**Migration:** Function returns `Result[T]`. Caller (already a `Result`-returning function in most cases — `_load_active_project`, `cb_load_prior_log`, `_refresh_from_project` already return `Result[None]`) checks `.ok` and either propagates the error or merges errors into the existing `Result.errors` via `.with_errors([...])`. For `replace_ref` and `queue_fallback`, the caller is the next-level utility — same pattern.
|
||||
|
||||
## 15. Goals for Phase 6
|
||||
|
||||
1. **Zero `INTERNAL_SILENT_SWALLOW` sites in `src/app_controller.py` after Phase 6.** Audit re-run shows 28 → 0 for the silent swallow category; no category reverts.
|
||||
2. **Every migrated site carries `ErrorInfo(original=e)`** so the swallowed exception's traceback is preserved (the convention's "AND over OR" + "Error Info as Side-Channel" patterns).
|
||||
3. **No `logging.debug` in except bodies** (per `error_handling.md:530` — logging is NOT a drain). Every except body either returns `Result(data=..., errors=[ErrorInfo(...)])` OR falls through to a real drain point (os._exit, stderr for terminal sinks, instance state for deferred drains).
|
||||
4. **All Phase 1-5 invariants preserved:** 0 INTERNAL_BROAD_CATCH, 0 INTERNAL_OPTIONAL_RETURN, 0 SUSPICIOUS INTERNAL_RETHROW.
|
||||
5. **No new test regressions.** Batched suite must still show the same pass count (890 passed / 17 skipped / 2 xfailed as of Tier 2's last run).
|
||||
|
||||
## 16. Functional Requirements
|
||||
|
||||
**FR9. Replace every `logging.debug(..., extra={"source": ...})` in an except body with `Result[T]` return.**
|
||||
|
||||
Each except body becomes:
|
||||
```python
|
||||
except (SpecificException1, SpecificException2) as e:
|
||||
return Result(data=default_value, errors=[ErrorInfo(
|
||||
kind=ErrorKind.INTERNAL,
|
||||
message=str(e),
|
||||
source="app_controller.<function_name>",
|
||||
original=e,
|
||||
)])
|
||||
```
|
||||
|
||||
For void functions, use `Result[None]` with `OK` for success. For non-void functions, the return type changes to `Result[T]` and the caller checks `.ok` and `.errors`.
|
||||
|
||||
**FR10. For functions where the caller can't easily receive a Result** (property setters, signal handlers, event sinks), use the pattern:
|
||||
- Property setter: add a sibling `_set_<thing>_result(value) -> Result[None]` method; the `@<prop>.setter` is a thin wrapper that calls the sibling and stores the error in `self._<thing>_error: ErrorInfo | None` for downstream consumers.
|
||||
- Signal handler: drain point IS `os._exit(0)` (Pattern 3); the handler writes the ErrorInfo to stderr right before exit.
|
||||
- Event sink: caller accumulates errors in instance state (`self._<event>_errors: list[ErrorInfo]`); stderr logs each one (user-confirmed acceptable until sub-track 4).
|
||||
|
||||
**FR11. Every migration site has a test.**
|
||||
|
||||
For each of the 28 sites, add at least 1 test (or extend an existing test) verifying:
|
||||
- Success path returns `Result(data=success_value)` with `.ok = True`
|
||||
- Failure path returns `Result(data=zero_value, errors=[ErrorInfo(original=expected_exception)])` with `.ok = False`
|
||||
- The error's `kind` and `source` match the spec
|
||||
|
||||
Tests are organized by group in `tests/test_app_controller_result.py` (extend the existing file; do not create a new one).
|
||||
|
||||
**FR12. Audit gate.**
|
||||
|
||||
`uv run python scripts/audit_exception_handling.py --src src/app_controller.py --strict` must exit 0 (no violations). Per-site count for INTERNAL_SILENT_SWALLOW must be 0.
|
||||
|
||||
**FR13. NO deferrals, NO "follow-up" carve-outs.**
|
||||
|
||||
Unlike Phase 3's deferral pattern (which left 20 nested sites as "follow-up"), Phase 6 must migrate ALL 28 sites in this phase. If a site is genuinely best-effort and should stay as-is (e.g., the `os._exit(0)` drain point sites), the migration must use the drain-point pattern with stderr write + Result propagation — not silent fall-through.
|
||||
|
||||
## 17. Non-Functional Requirements (Phase 6 additions)
|
||||
|
||||
- **No new dependencies.** `Result`, `ErrorInfo`, `ErrorKind`, `OK` are all in `src/result_types.py` (already imported).
|
||||
- **Thread safety preserved.** Background workers (`_bg_task`, `worker` closures) and signal handlers already use thread-local state; the migration uses the same thread-local conventions.
|
||||
- **No behavior change visible to the user** (until sub-track 4 ships the GUI display). The user sees the same stdout/stderr they saw before; the difference is the data shape (Result carrying the errors to instance state instead of being lost).
|
||||
- **Per-task atomic commits.** Each site is its own commit (28 sites = 28 commits) plus 8 test commits plus 1 audit-gate commit plus 1 end-of-phase checkpoint commit = ~38 commits.
|
||||
|
||||
## 18. Architecture Reference (Phase 6 additions)
|
||||
|
||||
- `conductor/code_styleguides/error_handling.md` §"The 5 drain point patterns" — defines Pattern 3 (intentional termination) used by Group 6.1.
|
||||
- `conductor/code_styleguides/error_handling.md` §"Boundary types vs. drain points" — defines when a function is BOTH a boundary and a drain point (the Group 6.4 SDK boundary sites).
|
||||
- `conductor/code_styleguides/error_handling.md` §"The Broad-Except Distinction" — explicit table that says `narrow except + log only` is `INTERNAL_SILENT_SWALLOW` (a violation). This is the rule Tier 2's Phase 3 commit violated.
|
||||
- `conductor/code_styleguides/error_handling.md` §"Re-Raise Patterns" — Pattern 1/2/3 for the 4 INTERNAL_RETHROW sites (already classified in Phase 4).
|
||||
- `src/result_types.py:91-105` — the `Result[T]` dataclass and its `ok` property; the migration target.
|
||||
|
||||
## 19. Out of Scope (Phase 6)
|
||||
|
||||
- GUI-side error display (modals, toasts, error panels in `gui_2.py`) — sub-track 4 (`result_migration_gui_2`). The user has confirmed that stderr + instance state is acceptable until sub-track 4.
|
||||
- The 4 INTERNAL_RETHROW sites — already classified as legitimate Patterns 1/3 in Phase 4; not Phase 6 targets.
|
||||
- The 1 INTERNAL_OPTIONAL_RETURN site (`cold_start_ts`) — already migrated to `Result[float]` in Phase 4; audit now classifies it INTERNAL_COMPLIANT.
|
||||
- The 15 BOUNDARY_FASTAPI + 2 BOUNDARY_SDK + 4 INTERNAL_COMPLIANT + 1 INTERNAL_PROGRAMMER_RAISE = 22 stay sites — not Phase 6 targets.
|
||||
- Sub-track 4 (`gui_2.py`) — separate track.
|
||||
|
||||
## 20. Verification Criteria (Phase 6)
|
||||
|
||||
- `uv run python scripts/audit_exception_handling.py --src src/app_controller.py --strict` exits 0.
|
||||
- `uv run python scripts/audit_exception_handling.py --src src/app_controller.py --json | python -c "..."` shows 0 sites with category `INTERNAL_SILENT_SWALLOW`.
|
||||
- `uv run python -m pytest tests/test_app_controller_result.py -v` passes all tests.
|
||||
- `uv run python scripts/run_tests_batched.py` shows the same pass count as the pre-Phase-6 baseline (890 passed / 17 skipped / 2 xfailed). No new failures.
|
||||
- Every migrated except body contains `Result(data=..., errors=[ErrorInfo(original=e)])` (or equivalent Pattern 3 drain for signal handlers) — verified by `grep -n 'logging.getLogger.*\.debug' src/app_controller.py | grep -v '#'` showing no debug-log-only except bodies.
|
||||
|
||||
## 21. Risk Register (Phase 6 additions)
|
||||
|
||||
- **R6 (Phase 6):** Tier 2 may repeat the Phase 3 deferral pattern (using `logging.debug` as a "migration" that the audit still flags as silent swallow). Mitigation: the audit gate in FR12 (`--strict` exits 1 on any violation) is the hard verification. If FR12 fails, the track is not complete regardless of how many sites are touched.
|
||||
- **R7 (Phase 6):** Some sites may need their callers updated to receive `Result[T]` instead of `T`. For example, `_update_inject_preview` currently returns `None` and sets `self._inject_preview`; changing to `Result[str]` requires the caller to check `.ok` and propagate. Mitigation: each task identifies its caller chain via `py_find_usages` and updates all callers in the same commit.
|
||||
- **R8 (Phase 6):** The 20 nested sites introduced by Phase 2 may have been overwritten by Phase 3's `logging.debug` add. The migration must remove the `logging.debug` AND replace with `Result` return (not add a Result on top of the logging).
|
||||
- **R9 (Phase 6):** Scope (28 sites) is large but bounded. Mitigation: 8 groups with clear drain patterns; each group is a sub-batch (3-5 commits per group). If a group takes too many commits, the group can be split further.
|
||||
|
||||
|
||||
@@ -4,11 +4,12 @@
|
||||
[meta]
|
||||
track_id = "result_migration_app_controller_20260618"
|
||||
name = "Result Migration - Sub-Track 3 (App Controller)"
|
||||
status = "completed"
|
||||
current_phase = 5
|
||||
status = "active"
|
||||
current_phase = 6
|
||||
last_updated = "2026-06-18"
|
||||
umbrella = "result_migration_20260616"
|
||||
sub_track_index = 3
|
||||
phase_6_added = "2026-06-18 — supersedes Phase 3's logging.debug 'migration' with proper Result[T] propagation; audit gate via --strict"
|
||||
|
||||
[blocked_by]
|
||||
result_migration_small_files_20260617 = "shipped 2026-06-17"
|
||||
@@ -19,9 +20,10 @@ result_migration_gui_2_<YYYYMMDD> = "blocked by this track; will be planned afte
|
||||
[phases]
|
||||
phase_1 = { status = "completed", checkpointsha = "75a11fb0", name = "Setup + Fix the regression (test_tool_ask_approval + test_execution_sim_live)" }
|
||||
phase_2 = { status = "completed", checkpointsha = "ddd600f4", name = "Migrate the 32 INTERNAL_BROAD_CATCH sites (4 bulk batches)" }
|
||||
phase_3 = { status = "completed", checkpointsha = "7fcce652", name = "Migrate the 8 INTERNAL_SILENT_SWALLOW sites (with logging.debug per Heuristic #19) - audit shows 28; spec estimated 8" }
|
||||
phase_3 = { status = "completed", checkpointsha = "7fcce652", name = "Migrate the 8 INTERNAL_SILENT_SWALLOW sites (with logging.debug per Heuristic #19) - SUPERSEDED by Phase 6; logging.debug is NOT a drain per error_handling.md:530" }
|
||||
phase_4 = { status = "completed", checkpointsha = "cc2448fb", name = "Classify 4 INTERNAL_RETHROW + migrate 1 INTERNAL_OPTIONAL_RETURN" }
|
||||
phase_5 = { status = "completed", checkpointsha = "9e061276", name = "Verify, document, end-of-track report" }
|
||||
phase_5 = { status = "completed", checkpointsha = "9e061276", name = "Verify, document, end-of-track report - SUPERSEDED by Phase 6; report rewritten" }
|
||||
phase_6 = { status = "pending", checkpointsha = "", name = "Proper Result[T] migration of the 28 INTERNAL_SILENT_SWALLOW sites (no logging.debug; real drain points; audit --strict gate)" }
|
||||
|
||||
[tasks]
|
||||
# Phase 1: Setup + Fix the regression
|
||||
@@ -59,12 +61,55 @@ t5_4 = { status = "pending", commit_sha = "", description = "Add 2 post-migratio
|
||||
t5_5 = { status = "pending", commit_sha = "", description = "Write docs/reports/TRACK_COMPLETION_result_migration_app_controller_20260618.md" }
|
||||
t5_6 = { status = "pending", commit_sha = "", description = "Mark state.toml complete; update umbrella spec count to reflect actual scope (45 migration + 22 stay = 67 total)" }
|
||||
|
||||
# Phase 6: Proper Result[T] migration of the 28 INTERNAL_SILENT_SWALLOW sites
|
||||
# Audit gate: uv run python scripts/audit_exception_handling.py --src src/app_controller.py --strict exits 0
|
||||
|
||||
# Sub-phase 6.1: Signal handlers (Pattern 3 drain: os._exit) - 2 sites
|
||||
t6_1_1 = { status = "pending", commit_sha = "", description = "Migrate _on_sigint (L772) + _install_sigint_exit_handler (L777) via _shutdown_io_pool_result + _install_signal_handler_result helpers; os._exit(0) is the drain" }
|
||||
|
||||
# Sub-phase 6.2: Event sinks / one-shot best-effort logging - 2 sites
|
||||
t6_2_1 = { status = "pending", commit_sha = "", description = "Migrate mark_first_frame_rendered (L1315) + _on_warmup_complete_for_timeline (L1411) via _log_startup_timeline_event_result helper; stderr carry acceptable until sub-track 4" }
|
||||
|
||||
# Sub-phase 6.3: GUI state setters / property setters - 3 sites
|
||||
t6_3_1 = { status = "pending", commit_sha = "", description = "Migrate _update_inject_preview (L1456) - function returns Result[str]; legacy wrapper stores _inject_preview_error for sub-track 4" }
|
||||
t6_3_2 = { status = "pending", commit_sha = "", description = "Migrate mcp_config_json setter (L1604) via _set_mcp_config_json_result sibling helper; setter stores _mcp_config_parse_error" }
|
||||
t6_3_3 = { status = "pending", commit_sha = "", description = "Migrate _save_active_project (L3024) - function returns Result[None]; legacy wrapper stores _save_project_error" }
|
||||
|
||||
# Sub-phase 6.4: SDK boundary - 1 site
|
||||
t6_4_1 = { status = "pending", commit_sha = "", description = "Migrate _fetch_models.do_fetch (L3173) - per-provider _list_models_for_provider_result helpers; aggregated errors in _model_fetch_errors dict" }
|
||||
|
||||
# Sub-phase 6.5: Background workers / threads - 10 sites
|
||||
t6_5_1 = { status = "pending", commit_sha = "", description = "Migrate 3 worker closures (L3532 _handle_compress, L3570 _handle_generate, L3642 _handle_md_only) - each worker returns Result[None]; _report_worker_error helper for stderr + telemetry" }
|
||||
t6_5_2 = { status = "pending", commit_sha = "", description = "Migrate _bg_task 3 sites (L4175, L4204, L4207) via _report_worker_error helper" }
|
||||
t6_5_3 = { status = "pending", commit_sha = "", description = "Migrate _start_track_logic 2 sites (L4300, L4346) via _report_worker_error helper" }
|
||||
t6_5_4 = { status = "pending", commit_sha = "", description = "Migrate _cb_run_conductor_setup (L4459) + _cb_load_track (L4557) via _report_worker_error helper" }
|
||||
|
||||
# Sub-phase 6.6: Per-event handlers - 3 sites
|
||||
t6_6_1 = { status = "pending", commit_sha = "", description = "Migrate _handle_request_event RAG (L3736) + symbol resolution (L3750) via _rag_search_result + _symbol_resolution_result helpers; errors accumulated in _last_request_errors" }
|
||||
t6_6_2 = { status = "pending", commit_sha = "", description = "Migrate _process_pending_gui_tasks per-task try (L1707) via _execute_gui_task_result helper; per-task errors in _gui_task_errors" }
|
||||
|
||||
# Sub-phase 6.7: Helpers / utilities - 6 sites
|
||||
t6_7_1 = { status = "pending", commit_sha = "", description = "Migrate replace_ref (L1986) - returns Result[str]; caller (next-level utility) checks .ok" }
|
||||
t6_7_2 = { status = "pending", commit_sha = "", description = "Migrate cb_load_prior_log token_history site (L2128) via _parse_token_history_ts_result helper; outer cb_load_prior_log merges errors via .with_errors()" }
|
||||
t6_7_3 = { status = "pending", commit_sha = "", description = "Migrate _load_active_project primary (L2195) + fallback_loop (L2210) via _load_project_from_path_result helper; outer function merges via .with_errors()" }
|
||||
t6_7_4 = { status = "pending", commit_sha = "", description = "Migrate queue_fallback per-iteration try (L2454) via _run_pending_tasks_once_result helper; bounded retry Pattern 5 drain" }
|
||||
t6_7_5 = { status = "pending", commit_sha = "", description = "Migrate _refresh_from_project active_track deserialize (L2969) via _deserialize_active_track_result helper; outer function merges via .with_errors()" }
|
||||
|
||||
# Sub-phase 6.8: Tests + verification
|
||||
t6_8_1 = { status = "pending", commit_sha = "", description = "Run audit_exception_handling.py --src src/app_controller.py --strict; confirm exit 0 and 0 INTERNAL_SILENT_SWALLOW sites" }
|
||||
t6_8_2 = { status = "pending", commit_sha = "", description = "Run full batched suite; confirm 890 passed / 17 skipped / 2 xfailed (no new regressions vs pre-Phase-6 baseline)" }
|
||||
t6_8_3 = { status = "pending", commit_sha = "", description = "Add test_app_controller_post_phase6_has_zero_silent_swallow invariant test" }
|
||||
t6_8_4 = { status = "pending", commit_sha = "", description = "Phase 6 checkpoint commit (conductor(plan): mark Phase 6 complete)" }
|
||||
t6_8_5 = { status = "pending", commit_sha = "", description = "Rewrite docs/reports/TRACK_COMPLETION_result_migration_app_controller_20260618.md to cover all 6 phases; supersede the misleading '8 silent swallow migrated' claim" }
|
||||
|
||||
[verification]
|
||||
phase_1_complete = true
|
||||
phase_2_complete = true
|
||||
phase_3_complete = true
|
||||
phase_4_complete = true
|
||||
phase_5_complete = true
|
||||
phase_6_complete = false
|
||||
regression_1_fixed = true
|
||||
regression_2_fixed = false
|
||||
batched_suite_no_new_regressions = true
|
||||
audit_silent_swallow_zero = false
|
||||
|
||||
Reference in New Issue
Block a user