From 4c0b19b4db461a484a7c0c4b448d1c30032b0d3d Mon Sep 17 00:00:00 2001 From: Ed_ Date: Tue, 16 Jun 2026 10:15:46 -0400 Subject: [PATCH] conductor(track): spec/plan/metadata for result_migration_20260616 (5 sub-tracks, NO day estimates) --- .../result_migration_20260616/metadata.json | 165 ++++++++ .../tracks/result_migration_20260616/plan.md | 205 +++++++++ .../tracks/result_migration_20260616/spec.md | 398 ++++++++++++++++++ 3 files changed, 768 insertions(+) create mode 100644 conductor/tracks/result_migration_20260616/metadata.json create mode 100644 conductor/tracks/result_migration_20260616/plan.md create mode 100644 conductor/tracks/result_migration_20260616/spec.md diff --git a/conductor/tracks/result_migration_20260616/metadata.json b/conductor/tracks/result_migration_20260616/metadata.json new file mode 100644 index 00000000..7527aafa --- /dev/null +++ b/conductor/tracks/result_migration_20260616/metadata.json @@ -0,0 +1,165 @@ +{ + "track_id": "result_migration_20260616", + "name": "Result Migration (Phase 2 - eliminate all bad exception handling)", + "initialized": "2026-06-16", + "completed_at": "2026-06-16 (umbrella planned; sub-tracks execute over time)", + "owner": "tier2-tech-lead", + "priority": "A", + "status": "active", + "type": "refactor (5 sub-tracks; each a separate TDD execution)", + "scope": { + "new_files": [ + "conductor/tracks/result_migration_20260616/spec.md", + "conductor/tracks/result_migration_20260616/plan.md", + "conductor/tracks/result_migration_20260616/metadata.json" + ], + "sub_tracks_planned": [ + "result_migration_review_pass_", + "result_migration_small_files_", + "result_migration_app_controller_", + "result_migration_gui_2_", + "result_migration_baseline_cleanup_" + ], + "modified_files": [], + "deleted_files": [] + }, + "blocked_by": [ + "exception_handling_audit_20260616 (shipped 2026-06-16; produced the 268-site inventory)" + ], + "blocks": [ + "data_structure_strengthening_20260606 (parallel; uses the cleaner Result API from this phase)", + "user_stated_intent: send_result -> send mass rename (user manual refactor; post-this-phase)" + ], + "estimated_phases": 5, + "spec": "spec.md", + "plan": "plan.md", + + "sub_tracks": [ + { + "id": "1: result_migration_review_pass", + "scope": "32 UNCLEAR + 25 INTERNAL_RETHROW = 57 sites across 15 files", + "tshirt_size": "S", + "why_first": "The UNCLEAR sites are ambiguous; human review + audit script heuristic updates feed into all later sub-tracks", + "files": "All 15 files with UNCLEAR or INTERNAL_RETHROW sites" + }, + { + "id": "2: result_migration_small_files", + "scope": "37 files (35 SMALL + 2 MEDIUM); 72 V+S sites", + "tshirt_size": "L", + "why_second": "Quick wins; doesn't depend on the orchestrator or GUI; can run in parallel with sub-tracks 3-4", + "files": "api_hooks.py, project_manager.py, aggregate.py, multi_agent_conductor.py, summary_cache.py, commands.py, external_editor.py, models.py, outline_tool.py, file_cache.py, gemini_cli_adapter.py, log_registry.py, markdown_helper.py, orchestrator_pm.py, presets.py, shell_runner.py, command_palette.py, context_presets.py, diff_viewer.py, hot_reloader.py, startup_profiler.py, summarize.py, theme_2.py, tool_presets.py, workspace_manager.py, theme_models.py, paths.py, rag_data_models.py, performance_monitor.py, plus 6 more, plus session_logger.py, warmup.py" + }, + { + "id": "3: result_migration_app_controller", + "scope": "src/app_controller.py (166KB); 56 sites (35 V + 3 S + 2 ? + 16 C)", + "tshirt_size": "XL", + "why_dedicated": "The orchestrator touches every subsystem; high coordination with Hook API + MMA + RAG", + "files": "src/app_controller.py" + }, + { + "id": "4: result_migration_gui_2", + "scope": "src/gui_2.py (260KB); 54 sites (37 V + 2 S + 13 ? + 2 C)", + "tshirt_size": "XL", + "why_dedicated": "Largest file in the codebase; immediate-mode UI; depends on sub-track 3 for clean API", + "files": "src/gui_2.py" + }, + { + "id": "5: result_migration_baseline_cleanup", + "scope": "3 refactored files; 112 sites (77 V + 10 S + 6 ? + 19 C)", + "tshirt_size": "L", + "why_last": "The baseline files ARE the convention reference; the remaining 77 violations are gaps to close (parent's Path C deferred work)", + "files": "src/mcp_client.py, src/ai_client.py, src/rag_engine.py" + } + ], + + "regressions_and_pre_existing_failures": [], + "pre_existing_failures_fixed_by_this_track": [], + "pre_existing_failures_remaining": [], + "incidental_fixes_from_parent_track": [], + "deferred_to_followup_tracks": [ + { + "id": "send_result_to_send_rename", + "title": "send_result -> send Mass Rename (user's stated intent)", + "description": "The user has stated intent to do a mass rename of send_result to send. The rename is mechanical (Result[T] return type is stable; only the function name changes). The user will do this manually after this phase ships.", + "track_status": "user_manual_refactor" + }, + { + "id": "data_structure_strengthening_20260606", + "title": "Data Structure Strengthening (Type Aliases + NamedTuples)", + "description": "Introduce 6 TypeAlias definitions in src/type_aliases.py; replace 370+ anonymous dict[str, Any] sites in 6 high-traffic files. Spec already exists; plan pending. Blocked by this phase (cleaner Result API usage makes type-alias replacement easier).", + "track_status": "ready to start; blocked by this phase" + }, + { + "id": "live_gui_mock_injection_20260615", + "title": "Live GUI Mock Injection Infrastructure", + "description": "Infrastructure for mock injection into the live_gui subprocess. Unblocks proper end-to-end live_gui + AI client tests.", + "track_status": "recommended; not yet specced" + }, + { + "id": "audit_optional_in_3_files_track", + "title": "Wire 4 audit scripts into CI as --strict gates", + "description": "After this phase ships, scripts/audit_exception_handling.py --strict returns 0. Wire the 4 enforcement audit scripts (audit_weak_types, audit_optional_in_3_files [referenced by error_handling.md but not yet committed], audit_main_thread_imports, audit_exception_handling) into CI as --strict gates.", + "track_status": "recommended; not yet specced" + } + ], + + "verification_criteria": { + "g1_review_pass_complete": "32 UNCLEAR sites reviewed; per-site decision (compliant-or-migration) recorded", + "g2_rethrow_pass_complete": "25 INTERNAL_RETHROW sites classified; per-site decision (one of 3 patterns or migration) recorded", + "g3_audit_heuristics_updated": "scripts/audit_exception_handling.py updated with heuristics for the most common compliant patterns", + "g4_updated_audit_runs": "Re-running the audit with the updated heuristics shows the UNCLEAR count is ~0", + "g5_per_subtrack_scope_updated": "The umbrella spec's per-sub-track plan is updated to reflect the post-review scope", + "g6_review_pass_report_exists": "docs/reports/RESULT_MIGRATION_REVIEW_PASS_.md exists with the per-site decision table", + "g7_no_test_regressions": "Full test suite: 1288 + 4 + 0 (unchanged; the review pass is informational)", + "g8_atomic_commits_per_subtrack": "Each sub-track is committed in 5+ atomic commits (spec, plan, metadata, code, docs)", + "g9_per_commit_git_notes": "All commits have git notes", + "nf1_no_production_code_change_in_review_pass": "Sub-track 1 (review pass) is informational; no production code change", + "nf2_atomic_commits": "Per-task atomic commits across the 5 sub-tracks", + "nf3_per_commit_git_notes": "All commits have git notes summarizing the work" + }, + + "estimated_effort": { + "method": "Scope + T-shirt size (per conductor/workflow.md §Tier 1 Track Initialization Rules). NO day estimates. The user / Tier 2 agent decides the actual pacing.", + "sub_track_1_review_pass": { "scope": "57 sites across 15 files", "tshirt_size": "S" }, + "sub_track_2_small_files": { "scope": "72 V+S sites across 37 files", "tshirt_size": "L" }, + "sub_track_3_app_controller": { "scope": "56 sites in 1 file (166KB)", "tshirt_size": "XL" }, + "sub_track_4_gui_2": { "scope": "54 sites in 1 file (260KB)", "tshirt_size": "XL" }, + "sub_track_5_baseline_cleanup": { "scope": "112 sites across 3 files", "tshirt_size": "L" }, + "total": "5 sub-tracks, 268 sites across 42 files" + }, + + "risk_register": { + "R1_takes_longer_than_expected": { + "likelihood": "medium", + "impact": "high", + "mitigation": "Track 5 (baseline cleanup) is the biggest risk; the 30+ tool functions in mcp_client.py may be bigger than expected. The plan acknowledges scope can grow; the user decides whether to split sub-tracks further." + }, + "R2_hot_reload_breaks": { + "likelihood": "medium", + "impact": "high", + "mitigation": "Sub-track 4 uses the hot-reload mechanism for visual verification. The migration is done incrementally; the user verifies each change visually." + }, + "R3_hook_api_breaks": { + "likelihood": "low", + "impact": "high", + "mitigation": "Sub-track 3 includes before/after verification of the Hook API (via live_gui tests). The convention's Result type is structurally compatible with the existing str/None return types if needed." + }, + "R4_review_pass_grows_scope": { + "likelihood": "medium", + "impact": "medium", + "mitigation": "The review pass updates the audit's heuristics; the migration scope for sub-tracks 2-4 may grow. The plan documents the scope changes in Phase 5." + }, + "R5_user_reorders_subtracks": { + "likelihood": "low", + "impact": "low", + "mitigation": "The plan recommends a sequence but the user can reorder. The sub-tracks are independent enough to swap." + } + }, + + "milestone_context": { + "pre_track_state": "First fully green baseline (1288 + 4 + 0). The convention is applied to 3 of 65 src/ files (mcp_client, ai_client, rag_engine). 211 violations + 25 suspicious + 32 unclear = 268 'bad' sites across 42 files, per the exception_handling_audit_20260616 report.", + "post_track_target": "All 268 sites migrated. The convention is applied to all 65 src/ files. The 4 enforcement audit scripts can be wired into CI as --strict gates. Test pass count: 1288 + 4 + 0 (unchanged; the migration is behavior-preserving).", + "historical_context": "This is the migration phase that completes the data_oriented_error_handling_20260606 track (shipped 2026-06-12). The parent track established the convention; this phase applies it to the remaining 62 src/ files and closes the gaps in the 3 refactored files.", + "user_intent_after_this_track": "User decides: send_result -> send mass rename (manual) or data_structure_strengthening_20260606 (parallel track; uses the cleaner Result API from this phase)." + } +} diff --git a/conductor/tracks/result_migration_20260616/plan.md b/conductor/tracks/result_migration_20260616/plan.md new file mode 100644 index 00000000..8ffbec72 --- /dev/null +++ b/conductor/tracks/result_migration_20260616/plan.md @@ -0,0 +1,205 @@ +# Plan: Result Migration — Sub-Track 1 (Review Pass) + +**Sub-track:** `result_migration_review_pass_20260616` (first of 5 sub-tracks) +**Umbrella:** `result_migration_20260616` +**Date:** 2026-06-16 +**Owner:** Tier 2 Tech Lead +**Base commit:** `4521a7df` (feat(scripts): add --summary and --by-size modes) + +--- + +## Phase 1: Setup + +- [x] **Task 1.1: Create sub-track folder** + - WHERE: `conductor/tracks/result_migration_review_pass_20260616/` + - WHAT: spec.md, plan.md, metadata.json + - HOW: Copy the umbrella spec as the starting point; customize for + the review pass scope + - COMMIT: `conductor(track): spec for result_migration_review_pass (sub-track 1 of 5)` + - GIT NOTE: Summary of sub-track 1 scope + dependency on the umbrella + +- [x] **Task 1.2: Update `conductor/tracks.md`** + - WHERE: `conductor/tracks.md` (after the umbrella row 6c) + - WHAT: Add a row for the sub-track + - HOW: Same pattern as the umbrella + - COMMIT: `conductor: register result_migration_review_pass in tracks.md` + - GIT NOTE: 1-sentence note + +--- + +## Phase 2: Review the 32 UNCLEAR sites + +For each UNCLEAR site, a human (the Tier 2 implementer with a human +review from the user on disputed cases) reads the snippet + 2-3 lines +of context and decides: +- **Compliant** (it's a boundary the script doesn't recognize; document + the pattern; add a heuristic to the script) +- **Migration-target** (it should be converted to Result-based; record + the line + file + decision in the report) + +The 32 UNCLEAR sites are in (per the audit): +- `src/gui_2.py`: 13 sites +- `src/mcp_client.py`: 4 sites (baseline) +- `src/ai_client.py`: 2 sites (baseline) +- `src/app_controller.py`: 2 sites +- `src/models.py`: 2 sites +- `src/outline_tool.py`, `src/summarize.py`, `src/shell_runner.py`, + `src/log_registry.py`, `src/summary_cache.py` (other small files): + ~9 sites total + +- [x] **Task 2.1: Review `src/gui_2.py` UNCLEAR sites (13)** + - WHERE: `src/gui_2.py` + - WHAT: For each of the 13 sites, decide compliant-or-migration + - HOW: Read the snippet; check the context; classify + +- [x] **Task 2.2: Review `src/mcp_client.py` UNCLEAR sites (4, baseline)** + - WHERE: `src/mcp_client.py` + - WHAT: Same as 2.1 + +- [x] **Task 2.3: Review `src/ai_client.py` UNCLEAR sites (2, baseline)** + - WHERE: `src/ai_client.py` + - WHAT: Same as 2.1 + +- [x] **Task 2.4: Review `src/app_controller.py` UNCLEAR sites (2)** + - WHERE: `src/app_controller.py` + - WHAT: Same as 2.1 + +- [x] **Task 2.5: Review the 11 small-file UNCLEAR sites** + - WHERE: 11 small files + - WHAT: Same as 2.1 + +--- + +## Phase 3: Classify the 25 INTERNAL_RETHROW sites + +For each INTERNAL_RETHROW site, classify as one of: +- **PATTERN 1 (catch + convert + raise as different type)**: legitimate +- **PATTERN 2 (catch + log + re-raise)**: legitimate +- **PATTERN 3 (catch + cleanup + re-raise)**: legitimate +- **SUSPICIOUS (catch + re-raise the same exception)**: migration-target + (remove the try/except or convert to Result-based) + +The 25 INTERNAL_RETHROW sites are in: +- `src/ai_client.py`: 6 sites (baseline) +- `src/rag_engine.py`: 4 sites (baseline) +- `src/app_controller.py`: 3 sites +- `src/gui_2.py`: 2 sites +- `src/warmup.py`, `src/api_hooks.py`, `src/models.py`, + `src/outline_tool.py`, `src/shell_runner.py`, `src/gemini_cli_adapter.py`, + `src/theme_models.py`: ~10 sites total + +- [x] **Task 3.1: Review `src/ai_client.py` INTERNAL_RETHROW sites (6)** + - WHERE: `src/ai_client.py` + - WHAT: Apply the 4 classifications + - HOW: Read the snippet; match against the patterns + +- [x] **Task 3.2: Review `src/rag_engine.py` INTERNAL_RETHROW sites (4)** + - WHERE: `src/rag_engine.py` + - WHAT: Same as 3.1 + +- [x] **Task 3.3: Review `src/app_controller.py` INTERNAL_RETHROW sites (3)** + - WHERE: `src/app_controller.py` + - WHAT: Same as 3.1 + +- [x] **Task 3.4: Review the 12 small-file INTERNAL_RETHROW sites** + - WHERE: 12 small files + - WHAT: Same as 3.1 + +--- + +## Phase 4: Update the audit script's heuristics + +For each site that turned out to be compliant (a common pattern the +script doesn't recognize), add a heuristic to the classification logic. + +- [x] **Task 4.1: Add heuristics for the 5-10 most common compliant patterns** + - WHERE: `scripts/audit_exception_handling.py` + - WHAT: Add new classification logic for the patterns the review pass + found to be compliant + - HOW: Update `_classify_except` and `_classify_raise`; add new + constants if needed + - SAFETY: The script is a static analyzer; the changes don't affect + runtime behavior + +- [x] **Task 4.2: Verify the updated classification** + - WHERE: `scripts/audit_exception_handling.py` + - WHAT: Re-run the audit; the UNCLEAR count should drop to 0 (or + close to it); the INTERNAL_RETHROW count should drop to whatever + the 3 legitimate patterns don't cover + - HOW: `uv run python scripts/audit_exception_handling.py --by-size` + +--- + +## Phase 5: Report + +- [x] **Task 5.1: Write the review pass report** + - WHERE: `docs/reports/RESULT_MIGRATION_REVIEW_PASS_.md` + - WHAT: Per-site decision table; updated migration scope for the + later sub-tracks; updated audit script heuristics; per-sub-track + site-count adjustments + - HOW: Use the format of the `EXCEPTION_HANDLING_AUDIT_20260616.md` + report + - COMMIT: `docs(report): add result_migration_review_pass report` + - GIT NOTE: Summary of the review pass + updated migration scope + +- [x] **Task 5.2: Update the umbrella spec's per-sub-track plan** + - WHERE: `conductor/tracks/result_migration_20260616/spec.md` (the + per-sub-track plan section) + - WHAT: Reflect the updated migration scope (some UNCLEAR sites may + be compliant; the site count per sub-track changes) + - HOW: Edit the spec; commit as a docs update + - COMMIT: `docs(track): update result_migration_20260616 with post-review scope` + - GIT NOTE: 1-sentence note about the scope change + +--- + +## Phase 6: Verification + +- [x] **Task 6.1: Verify the updated audit script** + - WHERE: `scripts/audit_exception_handling.py` + - WHAT: Re-run with `--by-size`; verify the UNCLEAR count is now + ~0; verify the per-bucket totals reflect the updated scope + - HOW: `uv run python scripts/audit_exception_handling.py --by-size` + +- [x] **Task 6.2: Verify the test pass count is unchanged** + - WHERE: `tests/` + - WHAT: This sub-track is informational; the test pass count should + stay at 1288 + 4 + 0 + - HOW: `uv run pytest tests/ --timeout=120 -p no:cacheprovider -q` (this + takes a while; consider running the batched version instead) + +- [x] **Task 6.3: Mark the sub-track as completed** + - WHERE: `conductor/tracks/result_migration_review_pass_/metadata.json`, `conductor/tracks.md` + - WHAT: Update `status: active → completed`; `completed_at: 2026-06-16` + - HOW: Edit the files; commit + - COMMIT: `conductor(track): mark result_migration_review_pass as completed` + - GIT NOTE: 1-sentence note + +--- + +## Notes for the Tier 2 Implementer + +- **This is a research task, not a refactor.** No production code + changes (only the audit script and the docs). The Tier 2 implementer's + job is to look at each of the 57 sites and make a decision. +- **The decisions feed into the migration scope** of sub-tracks 2-4. + Some sites that are UNCLEAR now may turn out to be compliant (the + script's heuristics are imperfect). Some INTERNAL_RETHROW sites may + turn out to be one of the 3 legitimate re-raise patterns. +- **The audit script updates are optional but encouraged.** If a + pattern turns out to be commonly compliant, add a heuristic. This + helps future audits. +- **The user is the final arbiter** on disputed cases. If a site's + classification is unclear after human review, ask the user. +- **The review pass is bounded by site count, not time.** 57 sites to + review; the audit script updates + report writing follow. The + Tier 2 implementer should not block on review for disputed cases. +## Risks at the Plan Level + +| Risk | Mitigation | +|---|---| +| The review pass reveals more UNCLEAR sites than expected (the heuristics miss patterns) | The plan includes a "Task 4.2: Verify the updated classification" step; the user re-runs the audit and confirms the UNCLEAR count is ~0 | +| The user disagrees with a classification on a disputed case | The plan asks the user for input on disputed cases; the user is the final arbiter | +| The user disagrees with a classification | The plan asks the user for input on disputed cases; the user is the final arbiter | +| The audit script updates introduce regressions | Run the updated audit after each heuristic change; compare before/after counts | +| The post-review scope changes invalidate the umbrella spec's per-sub-track plan | The plan includes a Task 5.2 to update the umbrella spec with the new scope | diff --git a/conductor/tracks/result_migration_20260616/spec.md b/conductor/tracks/result_migration_20260616/spec.md new file mode 100644 index 00000000..6e862590 --- /dev/null +++ b/conductor/tracks/result_migration_20260616/spec.md @@ -0,0 +1,398 @@ +# Track Specification: Result Migration (Phase 2 — eliminate all bad exception handling) + +**Track ID:** `result_migration_20260616` (umbrella for the 5 sub-tracks below) +**Status:** Active (spec approved 2026-06-16) +**Priority:** A (foundational; the 3 refactored baseline files + 5 migration sub-tracks complete the data-oriented error handling convention) +**Owner:** Tier 2 Tech Lead +**Type:** refactor (5 sub-tracks, each a separate TDD execution) +**Scope:** 268 sites across 42 files (per the `exception_handling_audit_20260616` audit) +**Parent tracks:** `data_oriented_error_handling_20260606` (shipped 2026-06-12), `exception_handling_audit_20260616` (shipped 2026-06-16) +**Sibling tracks:** `data_structure_strengthening_20260606` (planned, parallel; uses the cleaner Result API from this phase) + +> **Note on effort estimates:** per the Tier 1 rules (see `conductor/workflow.md` +> §"Tier 1 Track Initialization Rules"), this spec does NOT include day +> estimates. Effort is measured by scope (N files, M sites) and T-shirt +> size (S/M/L/XL) per sub-track. The user / Tier 2 agent decides the +> actual pacing. + +--- + +## 0. TL;DR + +This is the **migration phase** that completes the data-oriented error +handling convention. The 2026-06-12 parent track established the +convention; this umbrella track plans 5 sub-tracks that eliminate the +remaining 211 violations + 25 suspicious + 32 unclear = **268 "bad" +sites** across the codebase. + +**Per-file baseline (per `exception_handling_audit_20260616`):** + +| Bucket | Files | V+S sites | What | +|---|---|---|---| +| **LARGE** | 2 (gui_2, app_controller) | 77 | Dedicated track per file (T-shirt: XL) | +| **MEDIUM** | 2 (session_logger, warmup) | 15 | Folds into the small-files track | +| **SMALL** | 35 | 57 | Batched in one track (T-shirt: L) | +| **BASELINE** | 3 (mcp_client, ai_client, rag_engine) | 87 | Closes the gaps in the convention reference (T-shirt: L) | + +**5 sub-tracks with consistent `result_migration_*` prefix:** + +1. `result_migration_review_pass` (T-shirt: S) — 57 sites (32 UNCLEAR + 25 INTERNAL_RETHROW); updates the audit's heuristics +2. `result_migration_small_files` (T-shirt: L) — 37 files (35 SMALL + 2 MEDIUM; 72 V+S sites) +3. `result_migration_app_controller` (T-shirt: XL) — 56 sites (35 V + 3 S + 2 ? + 16 C; 13 FastAPI boundary stay as-is) +4. `result_migration_gui_2` (T-shirt: XL) — 54 sites (37 V + 2 S + 13 ? + 2 C) +5. `result_migration_baseline_cleanup` (T-shirt: L) — 112 sites (77 V + 10 S + 6 ? + 19 C in the 3 refactored files) + +**Total: 5 sub-tracks, 268 sites migrated, ~2100 lines changed across ~42 files.** + +--- + +## 1. Overview + +### 1.1 The State Before This Phase (as of 2026-06-16) + +Per `exception_handling_audit_20260616`: + +- **Convention is applied to 3 of 65 `src/` files** (mcp_client.py, ai_client.py, rag_engine.py — the "baseline"). +- **62 `src/` files are in the migration-target state** — they still use idiomatic Python (`try/except`, `Optional[T]`, broad `except Exception`). +- **211 violations + 25 suspicious + 32 unclear = 268 "bad" sites** across 42 files. +- **Test pass count: 1288 + 4 + 0** (the codebase works correctly; the audit identifies refactor opportunities, not bugs). + +### 1.2 The Goal + +Migrate **all 268 "bad" sites** in the 42 affected files to the +data-oriented error handling convention. After this phase, the +codebase will have: + +- Zero `INTERNAL_SILENT_SWALLOW` (except ...: pass / log-only). +- Zero `INTERNAL_BROAD_CATCH` (except Exception without ErrorInfo conversion, in non-`*_result` code). +- Zero `INTERNAL_OPTIONAL_RETURN` (try/except + return None/Optional[T]). +- Zero `INTERNAL_RETHROW` (try/except + raise without ErrorInfo conversion) — except where the new "Re-Raise Patterns" section allows. +- Zero `UNCLEAR` (manual review confirms each is compliant or gets migrated). + +The 5 sub-tracks collectively achieve this. The convention's "delete to +turn off" audit script (`scripts/audit_exception_handling.py`) becomes +useful as a **CI gate** in `--strict` mode after this phase: any new +violation introduced by future code will fail CI. + +### 1.3 The 5 Sub-Tracks (consistent `result_migration_*` prefix) + +All 5 sub-tracks follow the naming pattern `result_migration__`. +The umbrella spec uses placeholders; each sub-track gets its own date +when it starts. The umbrella commit names (this spec) use `20260616`. + +#### Sub-track 1: `result_migration_review_pass_` + +**Scope:** 32 UNCLEAR + 25 INTERNAL_RETHROW = 57 sites across 15 files. +**T-shirt size:** S (smallest sub-track; mostly research + audit-script edits). + +**Why first:** the UNCLEAR sites are ambiguous; a human review pass +turns them into definite decisions (compliant or migration-target). The +INTERNAL_RETHROW sites need the 3 legitimate re-raise patterns from +`conductor/code_styleguides/error_handling.md` (added 2026-06-16) to be +applied. Both feed into all later sub-tracks. + +**What it does:** +- For each of the 32 UNCLEAR sites, a human looks at the site and decides + compliant-or-migration. Updates the audit's heuristics for sites + that turn out to be a common pattern. +- For each of the 25 INTERNAL_RETHROW sites, classify as one of the 3 + legitimate re-raise patterns (convert, log+raise, cleanup+raise) or + mark for migration. +- Output: a doc with the per-site decision (added as an appendix to + this umbrella spec when the sub-track ships). + +**Dependency:** none (it's the first sub-track). + +#### Sub-track 2: `result_migration_small_files_` + +**Scope:** 37 files (the 35 SMALL + 2 MEDIUM from the `--by-size` bucket); +72 V+S sites. +**T-shirt size:** L (batched; ~700 lines changed across 37 files; mechanical). + +**Why second:** the small files are quick wins; they don't depend on +the orchestrator (app_controller) or the GUI. Some of them DO depend on +sub-track 1's review pass (so the UNCLEAR sites are classified first). + +**What it does:** +- Migrates each of the 37 files to the convention. +- Each file's migration is a small `Result[T]` introduction + an + `except as e: return Result(data=NIL_T, errors=[ErrorInfo(...)])` + replacement. +- The 2 MEDIUM files (session_logger, warmup) get dedicated commits; the + 35 SMALL files get batched commits (5-7 files per commit). + +**Dependency:** sub-track 1 (for the UNCLEAR classification). + +#### Sub-track 3: `result_migration_app_controller_` + +**Scope:** `src/app_controller.py` (166KB); 56 sites (35 V + 3 S + 2 ? + 16 C). +**T-shirt size:** XL (the orchestrator; high coordination with Hook API + MMA + RAG; ~700 lines changed in 1 file). + +**Why dedicated:** the controller is the orchestrator; it touches every +subsystem. Changes here require careful coordination with the +`_predefined_callbacks` and `_gettable_fields` Hook API registries, the +MMA conductor, and the RAG engine. + +**What it does:** +- Migrates the 22 migration-target sites (35 V - 13 FastAPI boundary = 22). +- The 13 FastAPI boundary sites (per the new "Boundary Types" section in + `conductor/code_styleguides/error_handling.md`) stay as-is. +- The 16 compliant sites stay as-is. +- Uses the 5-file-commit pattern from the parent track's + `doeh_test_thinking_cleanup_20260615` (not 11 separate test mocks). +- Adds tests for the new Result-based API (similar to + `test_ai_client_result.py`). + +**Dependency:** sub-track 1 (for the 2 UNCLEAR sites at lines 1842 and 1668). + +#### Sub-track 4: `result_migration_gui_2_` + +**Scope:** `src/gui_2.py` (260KB); 54 sites (37 V + 2 S + 13 ? + 2 C). +**T-shirt size:** XL (the largest file; immediate-mode UI; ~700 lines changed in 1 file). + +**Why dedicated:** the largest file in the codebase. The immediate-mode +UI means changes here affect every render frame. The migration should +be done incrementally with the hot-reload mechanism (`Ctrl+Alt+R`) so +the user can verify each change visually. + +**What it does:** +- Migrates the 37 V + 2 S + 13 ? = 52 migration-target sites. +- The 2 compliant sites stay as-is. +- The 13 UNCLEAR sites are the trickiest (per sub-track 1's review pass). +- Uses the hot-reload mechanism for visual verification. + +**Dependency:** sub-track 1 (for the 13 UNCLEAR sites); sub-track 3 +(strong coordination, since app_controller calls gui_2 methods; the +controller should be migrated first to give the GUI a clean API). + +#### Sub-track 5: `result_migration_baseline_cleanup_` + +**Scope:** the 3 refactored files (mcp_client.py, ai_client.py, +rag_engine.py); 112 sites (77 V + 10 S + 6 ? + 19 C). +**T-shirt size:** L (parent's Path C deferred work; ~600 lines changed across 3 files). + +**Why last:** the baseline files ARE the convention reference. The +remaining 77 violations are gaps in the reference (mostly the parent's +"deferred" work — the 30+ tool functions in mcp_client.py, the +SDK-exception-classification helpers in ai_client.py, the non-`*_result` +methods in rag_engine.py). Closing these makes the convention reference +**pure** — no migration-target sites in the baseline. + +**What it does:** +- Migrates the 30+ tool functions in mcp_client.py (the parent's Path C + deferred work). +- Migrates the broad-catches in the SDK-exception-classification helpers + in ai_client.py (catch `anthropic.APIError` + convert to ErrorInfo). +- Migrates the non-`*_result` methods in rag_engine.py. +- Result: the 3 refactored files become 100% convention-compliant. + +**Dependency:** none (independent of the other 4 sub-tracks; can run in +parallel with sub-tracks 2-4 if the Tier 2 agents coordinate). + +### 1.4 Out of Scope (Explicit) + +- **`send_result` → `send` mass rename** (user's stated manual refactor; + separate work after this phase ships). +- **`data_structure_strengthening_20260606`** (parallel track; uses the + cleaner Result API from this phase). +- **`live_gui_mock_injection_20260615`** (separate infrastructure track). +- **Removing the `send()` deprecation** (followup; once the rename ships). +- **Migrating `tests/` files** (the `public_api_migration_20260606` track + already migrated 22 test files to `send_result()`; the remaining tests + are out of scope for this phase). +- **Adding new `Result` patterns to areas that don't have any** (this + phase migrates EXISTING `try/except` sites, not adds new ones). + +--- + +## 2. Recommended Sequence + +``` +[Track 1: review pass] (S; informational; can run in parallel with 2-5) + ↓ +[Track 2: small files] (L; 37 files) + ↓ +[Track 3: app_controller] (XL; high coordination) + ↓ +[Track 4: gui_2] (XL; depends on 3 for clean API) + ↓ +[Track 5: baseline cleanup] (L; can run in parallel with 3-4) +``` + +**Parallelization options:** +- Tracks 2 + 5 can run in parallel (different files). +- Tracks 3 + 5 can run in parallel (different files; both touch + app_controller's interface but Track 5 only touches the convention + reference files). +- Track 4 depends on Track 3 (the GUI calls controller methods). +- Track 1 is independent (informational; can run any time). + +--- + +## 3. Architecture Reference + +### 3.1 The Convention + +- `conductor/code_styleguides/error_handling.md` — the canonical + styleguide (5 patterns + 5 doc-clarification sections added 2026-06-16) +- `docs/AGENTS.md` §"The 4 memory dimensions" — the cross-cutting lens +- `docs/guide_ai_client.md` "Data-Oriented Error Handling (Fleury + Pattern)" — the in-context guide for the provider layer +- `docs/guide_mcp_client.md` "Data-Oriented Error Handling (Fleury + Pattern)" — the in-context guide for the MCP tool layer +- `docs/guide_rag.md` "Data-Oriented Error Handling (Fleury Pattern)" + — the in-context guide for the RAG engine +- `conductor/code_styleguides/data_oriented_design.md` — the canonical + DOD reference + +### 3.2 The Audit Script + +- `scripts/audit_exception_handling.py` — the static analyzer + (10-category classification; `--json`, `--top`, `--verbose`, `--strict`, + `--summary`, `--by-size` modes) +- `docs/reports/EXCEPTION_HANDLING_AUDIT_20260616.md` — the audit + report (the 268-site inventory; the per-file + per-category breakdown) +- `docs/guide_app_controller.md` "Exception Handling" — the + app_controller-specific guide (the 13 FastAPI boundary sites; the 22 + migration-target sites) + +### 3.3 The 4 Enforcement Audit Scripts (CI gates) + +This phase's goal is to make `--strict` mode of +`scripts/audit_exception_handling.py` a viable CI gate. The other 3 +enforcement scripts are: + +- `scripts/audit_weak_types.py` — the `dict[str, Any]` / `list[dict[...]]` + type-strengthening audit +- `scripts/audit_optional_in_3_files.py` — the `Optional[T]` return type + ban in the 3 refactored files (referenced by `error_handling.md` but + not yet committed; should be created in `data_structure_strengthening_20260606` + per its spec §12.2) +- `scripts/audit_main_thread_imports.py` — the main-thread import + graph purity invariant + +After this phase ships, all 4 scripts should be wired into CI as +`--strict` mode gates. + +--- + +## 4. Per-Sub-Track Plan (just sub-track 1; the rest are detailed when each sub-track starts) + +Sub-track 1 (`result_migration_review_pass`) is the only one with a +detailed plan; the other 4 are detailed when each starts. The reason: +the audit's UNCLEAR + INTERNAL_RETHROW classification may change the +migration scope of the later sub-tracks (some UNCLEAR sites may turn +out to be compliant, reducing the migration work). + +### Phase 1: Setup (Sub-track 1) + +- [x] **Task 1.1: Initialize the sub-track folder** + - WHERE: `conductor/tracks/result_migration_review_pass_/` + - WHAT: spec.md, plan.md, metadata.json + - HOW: Copy this umbrella spec as the starting point; customize for the review pass + +- [x] **Task 1.2: Update `conductor/tracks.md`** + - WHERE: `conductor/tracks.md` (new row for the sub-track) + - WHAT: Add the sub-track under the umbrella row + - HOW: Same pattern as the previous tracks + +### Phase 2: Review (Sub-track 1) + +- [x] **Task 2.1: Review the 32 UNCLEAR sites** + - WHERE: All `src/` files + - WHAT: For each site, decide compliant-or-migration; record the + decision in a doc + - HOW: Use the audit's JSON output; for each site, read the snippet + + context + 2-3 lines around it; classify + +- [x] **Task 2.2: Classify the 25 INTERNAL_RETHROW sites** + - WHERE: All `src/` files + - WHAT: For each site, apply the 3 legitimate re-raise patterns from + the new styleguide section; record the decision + - HOW: Same as 2.1; the decisions feed into the migration scope of + sub-tracks 2-4 + +- [x] **Task 2.3: Update the audit script's heuristics** + - WHERE: `scripts/audit_exception_handling.py` + - WHAT: For sites that turned out to be compliant (a common pattern + the script doesn't recognize), add a heuristic to the + classification logic + - HOW: Add to the `_classify_except` / `_classify_raise` functions + +### Phase 3: Report (Sub-track 1) + +- [x] **Task 3.1: Write the review pass report** + - WHERE: `docs/reports/RESULT_MIGRATION_REVIEW_PASS_.md` + - WHAT: Per-site decision table; updated migration scope for the + later sub-tracks; updated audit script heuristics + - HOW: Use the format of the `EXCEPTION_HANDLING_AUDIT_20260616.md` + report + +### Phase 4: Verification (Sub-track 1) + +- [x] **Task 4.1: Verify the updated audit script** + - WHERE: `scripts/audit_exception_handling.py` + - WHAT: Re-run the audit; the UNCLEAR count should drop to 0; the + INTERNAL_RETHROW count should drop to whatever the 3 legitimate + patterns don't cover + - HOW: `uv run python scripts/audit_exception_handling.py --by-size` + +- [x] **Task 4.2: Document the updated migration scope** + - WHERE: This umbrella spec (the per-sub-track plan section) + - WHAT: The sub-track 2-4 scope may change after the review pass; + document the changes + +--- + +## 5. Verification Criteria (per sub-track) + +Each sub-track has its own verification criteria. The umbrella's criteria +are that **all 5 sub-tracks pass their criteria**; the umbrella is +"complete" when: + +- 268 sites migrated (or marked as legitimate via the review pass). +- `--strict` mode of the audit script returns 0 (no violations). +- Full test suite: 1288 + 4 + 0 (unchanged; the migration is + behavior-preserving). +- The convention is now fully applied to all 65 `src/` files. +- The 4 enforcement audit scripts can be wired into CI as `--strict` + gates. + +--- + +## 6. Risks & Mitigations + +| ID | Risk | Likelihood | Impact | Mitigation | +|---|---|---|---|---| +| R1 | The 5 sub-tracks are larger than expected (the parent's Path C deferred work is bigger than estimated) | Medium | High | Track 5 (baseline cleanup) is the biggest risk — the 30+ tool functions in mcp_client.py may be bigger than expected. The plan acknowledges scope can grow; the user decides whether to split sub-tracks further. | +| R2 | The migration breaks the Hot Reload mechanism (changes to gui_2.py don't hot-reload correctly) | Medium | High | Sub-track 4 uses the hot-reload mechanism for visual verification. The migration should be done incrementally; the user can verify each change visually. | +| R3 | The migration breaks the Hook API (changes to app_controller.py break the `_predefined_callbacks` / `_gettable_fields` registries) | Low | High | Sub-track 3 includes a "before/after" verification of the Hook API (via `live_gui` tests). The convention's `Result` type is structurally compatible with the existing str/None return types if needed. | +| R4 | The review pass (sub-track 1) reveals that more sites are violations than the audit's heuristics suggest | Medium | Medium | The review pass updates the audit's heuristics; the migration scope for sub-tracks 2-4 may grow. The plan documents the scope changes in Phase 4. | +| R5 | The user wants a different sub-track ordering (e.g., the orchestrator first) | Low | Low | The plan recommends a sequence but the user can reorder. The sub-tracks are independent enough to swap. | + +--- + +## 7. Commits (the umbrella + 5 sub-tracks, in order) + +The umbrella is 1 commit. Each sub-track is 5+ commits (spec, plan, metadata, code, docs). +Total: 1 + 5*5 = 26 commits across the 5 sub-tracks. + +--- + +## 8. See Also + +- `conductor/code_styleguides/error_handling.md` — the canonical convention + (5 patterns + 5 doc-clarification sections) +- `conductor/code_styleguides/data_oriented_design.md` — the canonical + DOD reference +- `docs/reports/EXCEPTION_HANDLING_AUDIT_20260616.md` — the audit + report (the 268-site inventory) +- `scripts/audit_exception_handling.py` — the static analyzer (with + `--summary` and `--by-size` modes) +- `conductor/tracks/exception_handling_audit_20260616/spec.md` — the + audit track's spec +- `conductor/tracks/data_oriented_error_handling_20260606/spec.md` + §12.2 — the parent's prioritized list of future migration tracks + (this umbrella replaces that list) +- `conductor/tracks/data_structure_strengthening_20260606/spec.md` — + the parallel track (uses the cleaner Result API from this phase)