diff --git a/conductor/tracks.md b/conductor/tracks.md index 0dc5af0b..28a83a81 100644 --- a/conductor/tracks.md +++ b/conductor/tracks.md @@ -24,7 +24,8 @@ Tracks that are unblocked and ready to start. Ordered by **dependency** (blocked | 6a | A | [Public API Migration + UI Polish Test Cleanup](#track-public-api-migration--ui-polish-test-cleanup) | spec ✓, plan ✓, shipped 2026-06-15 (13 pre-existing failures fixed; 3 RAG failures deferred to `rag_test_failures_20260615`) | (none — independent; **NEW 2026-06-15**; combined stability track) | | 6b | A | [RAG Test Failures Fix](#track-rag-test-failures-fix-new-2026-06-15) | spec ✓, plan ✓, shipped 2026-06-15 (3 RAG tests fixed; first fully green baseline 1288 + 4 + 0) | (none — independent; **NEW 2026-06-15**; small bug-fix track) | | 6c | B | [Exception Handling Audit (Convention Compliance + Doc Clarification)](#track-exception-handling-audit-convention-compliance--doc-clarification) | spec ✓, plan ✓, shipped 2026-06-16 (211 violations identified across 42 files; 5 doc gaps closed) | (none — independent; **NEW 2026-06-16**; audit + doc track; identifies the migration target for `data_structure_strengthening_20260606` and the user's `send_result` → `send` rename) | -| 6d | A | [Result Migration (5 sub-tracks)](#track-result-migration-5-sub-tracks-new-2026-06-16) | umbrella spec ✓; 5 sub-tracks pending (sub-track 1: `result_migration_review_pass`) | `exception_handling_audit_20260616`; identifies the migration target | (none — independent; **NEW 2026-06-16**; refactor phase; 5 sub-tracks eliminate the 268 "bad" sites per the audit; sub-tracks use the consistent `result_migration_*` prefix) | +| 6d | A | [Result Migration (5 sub-tracks)](#track-result-migration-5-sub-tracks-new-2026-06-16) | umbrella spec ✓; 5 sub-tracks pending (sub-track 1: `result_migration_review_pass_20260617` initialized; 4 remaining) | `exception_handling_audit_20260616`; identifies the migration target | (none — independent; **NEW 2026-06-16**; refactor phase; 5 sub-tracks eliminate the 268 "bad" sites per the audit; sub-tracks use the consistent `result_migration_*` prefix) | +| 6d-1 | A | [Result Migration Sub-Track 1: Review Pass](#track-result-migration-sub-track-1-review-pass-2026-06-17) | spec ✓, plan ✓, metadata ✓, state ✓; **ready for Tier 2** | `result_migration_20260616` (umbrella); `exception_handling_audit_20260616` (shipped 2026-06-16) | (**NEW 2026-06-17**; sub-track 1 of 5; 43 sites to classify: 24 UNCLEAR + 19 INTERNAL_RETHROW across 11 files; no production code change; T-shirt S; per-site decisions feed sub-tracks 2-4) | | 6e | A (meta-tooling) | [Tier 2 Autonomous Sandbox (unattended track execution)](#track-tier-2-autonomous-sandbox-new-2026-06-16) | spec ✓, plan ✓, **shipped 2026-06-16** (9 phases, 24 default-on tests + 4 opt-in tests + 1 smoke e2e) | (none — independent; **NEW 2026-06-16**; meta-tooling; eliminates the `permission: ask` bottleneck for well-regularized tracks via a 3-layer enforcement stack: OpenCode permission system + Windows restricted token + git hooks) | | 7 | — | [UI Polish (Five Issues)](#track-ui-polish-five-issues) | spec ✓, plan ✓, ready to start (Phases 1/4/5 shipped; Phases 2/3 code shipped but tests broken — fixed by track 6a) | (none — independent) | | 7a | B | [SQLite-Granularity Inline Docs for gui_2.py](#track-sqlite-granularity-inline-docs-for-gui_2py) | spec ✓, plan ✓, complete | (none — independent) | diff --git a/conductor/tracks/result_migration_review_pass_20260617/metadata.json b/conductor/tracks/result_migration_review_pass_20260617/metadata.json new file mode 100644 index 00000000..b82d6296 --- /dev/null +++ b/conductor/tracks/result_migration_review_pass_20260617/metadata.json @@ -0,0 +1,80 @@ +{ + "id": "result_migration_review_pass_20260617", + "title": "Result Migration Sub-Track 1 (Review Pass: classify 43 UNCLEAR + INTERNAL_RETHROW sites)", + "type": "audit + documentation (informational; no production code change)", + "status": "active", + "priority": "A", + "created": "2026-06-17", + "owner": "tier2-tech-lead", + "parent_umbrella": "result_migration_20260616", + "sub_track_of_5": 1, + "spec": "conductor/tracks/result_migration_review_pass_20260617/spec.md", + "plan": "conductor/tracks/result_migration_review_pass_20260617/plan.md", + "scope": { + "files_affected": 11, + "sites_to_classify": 43, + "unclear_sites": 24, + "internal_rethrow_sites": 19, + "audit_script_lines_changed": "TBD (heuristics; ~10-50 lines)", + "report_lines": "~200-400 (per-site decision table)", + "umbrella_spec_lines_changed": "~20-50 (per-sub-track plan section update)" + }, + "depends_on": [ + "result_migration_20260616 (umbrella)", + "exception_handling_audit_20260616 (shipped 2026-06-16; produced the original 268-site inventory)" + ], + "blocks": [ + "result_migration_small_files_ (needs the per-site decisions)", + "result_migration_app_controller_ (needs the per-site decisions)", + "result_migration_gui_2_ (needs the per-site decisions)" + ], + "tshirt_size": "S", + "test_summary": { + "new_tests": 0, + "modified_tests": 0, + "test_pass_count_target": "1288 + 4 + 0 (unchanged; informational track)" + }, + "verification_criteria": [ + "docs/reports/RESULT_MIGRATION_REVIEW_PASS_20260617.md exists with per-site decision table for all 43 sites", + "scripts/audit_exception_handling.py has >= 1 new heuristic for commonly-compliant patterns", + "Re-running the audit post-heuristics: UNCLEAR count is 0 (+/- 2 acceptable)", + "conductor/tracks/result_migration_20260616/spec.md section 1.3 is updated with post-review site counts", + "Full test pass count: 1288 + 4 + 0 (unchanged; informational track)", + "Atomic commits per file: spec, plan, metadata, state, 6 UNCLEAR-file review commits, 7 INTERNAL_RETHROW-file review commits, audit script update, report, umbrella update, completion" + ], + "out_of_scope": [ + "Migrating any production code (sub-tracks 2-4 do that)", + "Refactoring the audit script's overall architecture (only _classify_except / _classify_raise are touched)", + "The 211 violations + remaining INTERNAL_RETHROW sites (sub-tracks 2-5)" + ], + "risks": [ + { + "id": "R1", + "description": "Review reveals more sites are violations than the audit's heuristics suggest", + "mitigation": "Per-site decision table records every site; sub-tracks 2-4 absorb the scope growth" + }, + { + "id": "R2", + "description": "User disagrees with a classification on a disputed case", + "mitigation": "User is the final arbiter; no site is left without a decision" + }, + { + "id": "R3", + "description": "Audit script updates introduce regressions (a new heuristic misclassifies a known site)", + "mitigation": "Run the audit before and after each heuristic change; compare counts" + } + ], + "estimated_effort": { + "method": "Scope + T-shirt size (per conductor/workflow.md section Tier 1 Track Initialization Rules). NO day estimates. The user / Tier 2 agent decides the actual pacing.", + "scope": "43 sites across 11 files; ~10-50 lines of audit script changes; ~200-400 lines of report", + "tshirt_size": "S" + }, + "deferred_to_followup_tracks": [ + { + "id": "result_migration_subsequent_subtracks", + "title": "Result Migration Sub-Tracks 2-5", + "description": "After this review pass ships, sub-tracks 2-5 pick up the migration work using the per-site decisions in the report. Sub-track 1 is the prerequisite for all of them.", + "track_status": "blocked by this sub-track" + } + ] +} diff --git a/conductor/tracks/result_migration_review_pass_20260617/plan.md b/conductor/tracks/result_migration_review_pass_20260617/plan.md new file mode 100644 index 00000000..5cb64681 --- /dev/null +++ b/conductor/tracks/result_migration_review_pass_20260617/plan.md @@ -0,0 +1,242 @@ +# Plan: Result Migration — Sub-Track 1 (Review Pass) + +**Sub-track:** `result_migration_review_pass_20260617` +**Umbrella:** [`result_migration_20260616`](../../result_migration_20260616/spec.md) +**Owner:** Tier 2 Tech Lead +**Base commit:** `b6caca40` (test(theme_nerv): align alert test with kwargs call signature) +**Audit-data commit:** see `git log scripts/audit_exception_handling.py` (the audit script's most recent change is the post-report heuristic update; the 24+19 inventory is the live state) + +--- + +## Phase 1: Setup + +- [ ] **Task 1.1: Initialize the sub-track folder** + - WHERE: `conductor/tracks/result_migration_review_pass_20260617/` (already created) + - WHAT: `spec.md`, `plan.md`, `metadata.json`, `state.toml` (this file) + - HOW: Read the umbrella spec; the sub-track spec mirrors the umbrella's sub-track 1 plan + - COMMIT: `conductor(track): spec for result_migration_review_pass (sub-track 1 of 5)` + - GIT NOTE: Sub-track 1 scope (43 sites across 11 files; 24 UNCLEAR + 19 INTERNAL_RETHROW); dependency on the umbrella + +- [ ] **Task 1.2: Update `conductor/tracks.md`** + - WHERE: `conductor/tracks.md` (after the umbrella row 6d) + - WHAT: Add a row for sub-track 1 + - HOW: Same pattern as the umbrella row; reference the umbrella and parent audit + - COMMIT: `conductor: register result_migration_review_pass_20260617 in tracks.md` + - GIT NOTE: 1-sentence note pointing to the sub-track folder + +--- + +## Phase 2: Review the 24 UNCLEAR sites (6 files) + +For each site, the Tier 2 implementer reads the snippet + 2-3 lines of context and decides: +- **Compliant** — the site matches a pattern the audit script SHOULD recognize; document the pattern; add a heuristic +- **Migration-target** — the site should be converted to Result-based in sub-tracks 2-4; record the line + file + decision in the report + +The 24 UNCLEAR sites are in (per the live audit JSON, 2026-06-17): + +- `src/gui_2.py`: 13 sites (lines 65, 69, 684, 806, 1349, 2401, 2411, 2533, 2561, 2759, 4106, 4159, 6830) +- `src/mcp_client.py`: 4 sites (lines 126, 152, 177, 987) — BASELINE +- `src/ai_client.py`: 2 sites (lines 828, 2813) — BASELINE +- `src/app_controller.py`: 2 sites (lines 1842, 3740) +- `src/models.py`: 2 sites (lines 452, 457) +- `src/multi_agent_conductor.py`: 1 site (line 236) + +- [ ] **Task 2.1: Review `src/gui_2.py` UNCLEAR sites (13)** + - WHERE: `src/gui_2.py` + - WHAT: For each of the 13 sites, classify compliant-or-migration + - HOW: `manual-slop_get_file_slice` on each line; read 2-3 lines of context + - COMMIT: `docs(track): result_migration_review_pass decisions for src/gui_2.py UNCLEAR` + - GIT NOTE: Per-site decisions for gui_2 UNCLEAR + +- [ ] **Task 2.2: Review `src/mcp_client.py` UNCLEAR sites (4, baseline)** + - WHERE: `src/mcp_client.py` + - WHAT: Same as 2.1; note the baseline status (refactored 2026-06-12; remaining sites are Path C deferred work) + - HOW: Same as 2.1 + - COMMIT: `docs(track): result_migration_review_pass decisions for src/mcp_client.py UNCLEAR` + - GIT NOTE: Per-site decisions for mcp_client UNCLEAR + +- [ ] **Task 2.3: Review `src/ai_client.py` UNCLEAR sites (2, baseline)** + - WHERE: `src/ai_client.py` + - WHAT: Same as 2.2 + - HOW: Same as 2.1 + - COMMIT: `docs(track): result_migration_review_pass decisions for src/ai_client.py UNCLEAR` + - GIT NOTE: Per-site decisions for ai_client UNCLEAR + +- [ ] **Task 2.4: Review `src/app_controller.py` UNCLEAR sites (2)** + - WHERE: `src/app_controller.py` + - WHAT: Same as 2.1 + - HOW: Same as 2.1 + - COMMIT: `docs(track): result_migration_review_pass decisions for src/app_controller.py UNCLEAR` + - GIT NOTE: Per-site decisions for app_controller UNCLEAR + +- [ ] **Task 2.5: Review `src/models.py` UNCLEAR sites (2)** + - WHERE: `src/models.py` + - WHAT: Same as 2.1 + - HOW: Same as 2.1 + - COMMIT: `docs(track): result_migration_review_pass decisions for src/models.py UNCLEAR` + - GIT NOTE: Per-site decisions for models UNCLEAR + +- [ ] **Task 2.6: Review `src/multi_agent_conductor.py` UNCLEAR sites (1)** + - WHERE: `src/multi_agent_conductor.py` + - WHAT: Same as 2.1 + - HOW: Same as 2.1 + - COMMIT: `docs(track): result_migration_review_pass decisions for src/multi_agent_conductor.py UNCLEAR` + - GIT NOTE: Per-site decisions for multi_agent_conductor UNCLEAR + +--- + +## Phase 3: Classify the 19 INTERNAL_RETHROW sites (7 files) + +For each 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 +- **Migration-target** (catch + re-raise same exception OR no good reason): queue for sub-tracks 2-4 + +See `conductor/code_styleguides/error_handling.md` §"Re-Raise Patterns" for the canonical pattern definitions. + +The 19 INTERNAL_RETHROW sites are in (per the live audit JSON): + +- `src/ai_client.py`: 6 sites (lines 277, 801, 802, 1234, 1529, 2520) — BASELINE, all `RAISE` kind +- `src/rag_engine.py`: 4 sites (lines 29, 36, 57, 75) — BASELINE +- `src/app_controller.py`: 3 sites (lines 1224, 1250, 2982) — all `RAISE` in `__getattr__` + 1 `RAISE` in `load_context_preset` +- `src/gui_2.py`: 2 sites (lines 757, 760) — both `RAISE` in `__getattr__` +- `src/api_hooks.py`: 2 sites (lines 938, 941) — 1 EXCEPT + 1 RAISE in `main` +- `src/models.py`: 1 site (line 268) — `RAISE` in `__getattr__` +- `src/warmup.py`: 1 site (line 85) — `RAISE` in `submit` + +- [ ] **Task 3.1: Review `src/ai_client.py` INTERNAL_RETHROW sites (6, baseline)** + - WHERE: `src/ai_client.py` + - WHAT: Apply the 4 classifications to each of the 6 RAISE sites + - HOW: For each line, read the surrounding 5-10 lines to determine if it's PATTERN 1/2/3 or migration-target + - COMMIT: `docs(track): result_migration_review_pass decisions for src/ai_client.py INTERNAL_RETHROW` + - GIT NOTE: Per-site classifications for ai_client INTERNAL_RETHROW + +- [ ] **Task 3.2: Review `src/rag_engine.py` INTERNAL_RETHROW sites (4, baseline)** + - WHERE: `src/rag_engine.py` + - WHAT: Same as 3.1; lines 29+36 are in `_get_sentence_transformers` (lazy import pattern), lines 57+75 are in `embed` + - HOW: Same as 3.1 + - COMMIT: `docs(track): result_migration_review_pass decisions for src/rag_engine.py INTERNAL_RETHROW` + - GIT NOTE: Per-site classifications for rag_engine INTERNAL_RETHROW + +- [ ] **Task 3.3: Review `src/app_controller.py` INTERNAL_RETHROW sites (3)** + - WHERE: `src/app_controller.py` + - WHAT: Same as 3.1; lines 1224+1250 are in `__getattr__` (defer-not-catch guard) + - HOW: Same as 3.1 + - COMMIT: `docs(track): result_migration_review_pass decisions for src/app_controller.py INTERNAL_RETHROW` + - GIT NOTE: Per-site classifications for app_controller INTERNAL_RETHROW + +- [ ] **Task 3.4: Review `src/gui_2.py` INTERNAL_RETHROW sites (2)** + - WHERE: `src/gui_2.py` + - WHAT: Same as 3.1; lines 757+760 are in `__getattr__` (defer-not-catch guard, likely) + - HOW: Same as 3.1 + - COMMIT: `docs(track): result_migration_review_pass decisions for src/gui_2.py INTERNAL_RETHROW` + - GIT NOTE: Per-site classifications for gui_2 INTERNAL_RETHROW + +- [ ] **Task 3.5: Review `src/api_hooks.py` INTERNAL_RETHROW sites (2)** + - WHERE: `src/api_hooks.py` + - WHAT: Same as 3.1; lines 938+941 in `main` + - HOW: Same as 3.1 + - COMMIT: `docs(track): result_migration_review_pass decisions for src/api_hooks.py INTERNAL_RETHROW` + - GIT NOTE: Per-site classifications for api_hooks INTERNAL_RETHROW + +- [ ] **Task 3.6: Review `src/models.py` INTERNAL_RETHROW site (1)** + - WHERE: `src/models.py` + - WHAT: Same as 3.1; line 268 in `__getattr__` + - HOW: Same as 3.1 + - COMMIT: `docs(track): result_migration_review_pass decisions for src/models.py INTERNAL_RETHROW` + - GIT NOTE: Per-site classifications for models INTERNAL_RETHROW + +- [ ] **Task 3.7: Review `src/warmup.py` INTERNAL_RETHROW site (1)** + - WHERE: `src/warmup.py` + - WHAT: Same as 3.1; line 85 in `submit` + - HOW: Same as 3.1 + - COMMIT: `docs(track): result_migration_review_pass decisions for src/warmup.py INTERNAL_RETHROW` + - GIT NOTE: Per-site classifications for warmup INTERNAL_RETHROW + +--- + +## 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 `_classify_except` or `_classify_raise` in `scripts/audit_exception_handling.py`. + +- [ ] **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: Use the AST inspection patterns the script already has; add to the `_classify_except` / `_classify_raise` functions + - SAFETY: The script is a static analyzer; the changes don't affect runtime behavior. Run the audit before and after each heuristic change to verify the new heuristic doesn't misclassify existing sites. + - COMMIT: `feat(scripts): add heuristics to audit_exception_handling for review pass patterns` + - GIT NOTE: Heuristics added; per-site rationale + +- [ ] **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; ±2 acceptable per the spec); the INTERNAL_RETHROW count should drop to whatever the 3 legitimate patterns don't cover + - HOW: `uv run python scripts/audit_exception_handling.py --json` and compare before/after counts + - SAFETY: If the new heuristic misclassifies a known site, the audit will show a different breakdown — re-check the per-site decisions in the report + - COMMIT: `docs(track): verify audit heuristic update` (only if a doc change is needed; otherwise rolled into 4.1) + +--- + +## Phase 5: Report + +- [ ] **Task 5.1: Write the review pass report** + - WHERE: `docs/reports/RESULT_MIGRATION_REVIEW_PASS_20260617.md` + - WHAT: Per-site decision table (43 rows); 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 + +- [ ] **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 + +- [ ] **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 (±2); verify the per-bucket totals reflect the updated scope + - HOW: `uv run python scripts/audit_exception_handling.py --by-size` + - COMMIT: rolled into 5.1 (the report captures the verification command + output) + +- [ ] **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 python scripts/run_tests_batched.py` (the tier-2 standard, per `conductor/workflow.md` §"Tier 2 Autonomous Sandbox") + - NOTE: The batched runner is the canonical verification for tier-2; isolated `pytest` is forbidden per the Isolated-Pass Verification Fallacy rule + - COMMIT: rolled into 5.1 + +- [ ] **Task 6.3: Mark the sub-track as completed** + - WHERE: `conductor/tracks/result_migration_review_pass_20260617/metadata.json` + `state.toml`, `conductor/tracks.md` + - WHAT: Update `status: active → completed`; `current_phase: "complete"` + - HOW: Edit the files; commit + - COMMIT: `conductor(track): mark result_migration_review_pass_20260617 as completed` + - GIT NOTE: 1-sentence note + +--- + +## Risks at the Plan Level + +| Risk | Mitigation | +|---|---| +| The review pass reveals more UNCLEAR sites than expected (the heuristics miss patterns) | Task 4.2 verifies the post-heuristic UNCLEAR count is ~0; if not, iterate | +| The user disagrees with a classification on a disputed case | The plan defers to the user as the final arbiter (per the spec §"Notes for the Tier 2 Implementer") | +| Audit script updates introduce regressions | Task 4.1 includes a safety step: run the audit before and after each heuristic change; compare counts | +| The post-review scope changes invalidate the umbrella spec's per-sub-track plan | Task 5.2 updates the umbrella spec with the new scope | +| The test pass count drops unexpectedly | Task 6.2 catches this; investigate the test failure per the standard process | + +--- + +## Verification Snapshot (capture in the report) + +After the review pass + heuristic update, capture in `docs/reports/RESULT_MIGRATION_REVIEW_PASS_20260617.md`: + +- `audit_exception_handling.py` count before: 24 UNCLEAR + 19 INTERNAL_RETHROW = 43 +- `audit_exception_handling.py` count after: 0 UNCLEAR (±2) + N INTERNAL_RETHROW (where N = total - 3-pattern-matches) +- Per-site decision table (43 rows) +- Per-file migration-target delta (the change in sub-tracks 2-4 site counts) +- Audit script heuristics added (count + 1-line summary per heuristic) diff --git a/conductor/tracks/result_migration_review_pass_20260617/spec.md b/conductor/tracks/result_migration_review_pass_20260617/spec.md new file mode 100644 index 00000000..3ed40790 --- /dev/null +++ b/conductor/tracks/result_migration_review_pass_20260617/spec.md @@ -0,0 +1,136 @@ +# Track Specification: Result Migration — Sub-Track 1 (Review Pass) + +**Track ID:** `result_migration_review_pass_20260617` +**Parent umbrella:** [`result_migration_20260616`](../../result_migration_20260616/spec.md) (sub-track 1 of 5) +**Type:** audit + documentation (informational; no production code change) +**Priority:** A (foundational; feeds all later sub-tracks) +**T-shirt size:** S +**Status:** ready to start (blocked-by cleared; unblocked) + +--- + +## 0. Overview + +This is sub-track 1 of the 5-sub-track `result_migration_20260616` campaign that eliminates the 268 "bad" exception-handling sites across 42 files (per the `exception_handling_audit_20260616` audit). Sub-track 1 is the **review pass**: it does not migrate any production code. It makes 43 ambiguous audit classifications into 43 definite decisions (compliant or migration-target), updates the audit script's heuristics for the patterns the human review found to be common, and produces the per-site decision table that sub-tracks 2-4 will use as their starting scope. + +## 1. Current State Audit (as of 2026-06-17, base commit `b6caca40`) + +### 1.1 The 348-Site Baseline (per `scripts/audit_exception_handling.py --json`) + +The audit script classifies every `try/except/finally/raise` site into 10 categories. As of 2026-06-17: + +| Category | Count | Status | +|---|---|---| +| Compliant | varies | ok | +| Violations | 211 | migration target | +| Suspicious | 25 | reviewable | +| UNCLEAR | 32 | needs human review | + +**Note:** the audit script's heuristics were updated since the original report (`docs/reports/EXCEPTION_HANDLING_AUDIT_20260616.md`); the current re-run shows **24 UNCLEAR + 19 INTERNAL_RETHROW = 43 sites** across 11 files (down from the report's 32 + 25 = 57 across 15). Some sites have been reclassified as compliant by the new heuristics; the per-site inventory below is the live state. + +### 1.2 The 24 UNCLEAR Sites (per-file inventory) + +| File | Sites | Lines | In baseline? | +|---|---|---|---| +| `src/gui_2.py` | 13 | 65, 69, 684, 806, 1349, 2401, 2411, 2533, 2561, 2759, 4106, 4159, 6830 | no (migration target) | +| `src/mcp_client.py` | 4 | 126, 152, 177, 987 | **yes** (refactored 2026-06-12) | +| `src/ai_client.py` | 2 | 828, 2813 | **yes** (refactored 2026-06-12) | +| `src/app_controller.py` | 2 | 1842, 3740 | no | +| `src/models.py` | 2 | 452, 457 | no | +| `src/multi_agent_conductor.py` | 1 | 236 | no | + +**Total: 24 sites across 6 files.** + +### 1.3 The 19 INTERNAL_RETHROW Sites (per-file inventory) + +| File | Sites | Lines | In baseline? | +|---|---|---|---| +| `src/ai_client.py` | 6 | 277, 801, 802, 1234, 1529, 2520 | **yes** (all `RAISE` kind) | +| `src/rag_engine.py` | 4 | 29, 36, 57, 75 | **yes** | +| `src/app_controller.py` | 3 | 1224, 1250, 2982 | no (all `RAISE`) | +| `src/gui_2.py` | 2 | 757, 760 | no (both `RAISE` in `__getattr__`) | +| `src/api_hooks.py` | 2 | 938, 941 | no (1 EXCEPT + 1 RAISE in `main`) | +| `src/models.py` | 1 | 268 | no (`RAISE` in `__getattr__`) | +| `src/warmup.py` | 1 | 85 | no (`RAISE` in `submit`) | + +**Total: 19 sites across 7 files.** + +### 1.4 The 3 Legitimate Re-Raise Patterns (per `conductor/code_styleguides/error_handling.md` §"Re-Raise Patterns", added 2026-06-16) + +The styleguide defines 3 patterns where `try/except + raise` is legitimate (not a violation): + +1. **PATTERN 1: catch + convert + raise as different type** (e.g., `except IOError as e: raise ProviderError(str(e))` — converts an SDK-boundary exception into a domain exception) +2. **PATTERN 2: catch + log + re-raise** (e.g., `except Exception as e: logger.exception("..."); raise` — preserves the original traceback for debugging) +3. **PATTERN 3: catch + cleanup + re-raise** (e.g., `except Exception: lock.release(); raise` — runs cleanup logic and re-raises the original) + +Sites that don't match any of the 3 patterns are migration-target (remove the try/except or convert to Result-based). + +### 1.5 The Audit Script's Classification Logic (reference) + +The script (`scripts/audit_exception_handling.py`) uses Python's `ast` module to classify each site. The `UNCLEAR` category fires when the script cannot determine the classification from the AST alone (the body of the `except` is too complex, or the surrounding context is ambiguous). The `INTERNAL_RETHROW` category fires on `try/except + raise` patterns without context about WHY the re-raise happens. + +## 2. Goals + +The track has 3 goals, all bounded by scope (not time): + +1. **Per-site decision** for all 24 UNCLEAR sites: `compliant` (with a heuristic update) or `migration-target` (queued for sub-tracks 2-4). +2. **Per-site classification** for all 19 INTERNAL_RETHROW sites: `PATTERN_1`, `PATTERN_2`, `PATTERN_3`, or `migration-target`. +3. **Updated audit script heuristics** for the 5-10 most common compliant patterns the review pass discovered. + +## 3. Functional Requirements + +- **FR1:** A per-site decision table is written to `docs/reports/RESULT_MIGRATION_REVIEW_PASS_20260617.md` covering all 43 sites. +- **FR2:** The audit script's classification logic (`scripts/audit_exception_handling.py`, the `_classify_except` and `_classify_raise` functions) is updated with at least 1 new heuristic for each commonly-compliant pattern. +- **FR3:** Re-running `uv run python scripts/audit_exception_handling.py --json` after the heuristic updates shows the UNCLEAR count is 0 (or close to it; ±2 sites that the user classifies as "ambiguous, leave as UNCLEAR"). +- **FR4:** The umbrella spec's per-sub-track plan section (`conductor/tracks/result_migration_20260616/spec.md`) is updated to reflect the post-review migration scope (some UNCLEAR sites may be compliant; sub-tracks 2-4 site counts change). + +## 4. Non-Functional Requirements + +- **NF1:** No production code change. Only the audit script and documentation are modified. +- **NF2:** Atomic per-task commits. Each review batch is its own commit (e.g., "review `src/gui_2.py` UNCLEAR sites"). +- **NF3:** Per-commit git notes summarizing the per-site decisions. +- **NF4:** Test pass count is unchanged: 1288 + 4 + 0 (the review pass is informational). + +## 5. Architecture Reference + +- `conductor/code_styleguides/error_handling.md` §"Re-Raise Patterns" — the 3 legitimate re-raise patterns to apply to INTERNAL_RETHROW sites +- `docs/AGENTS.md` §"Convention Enforcement" — the 4 enforcement audit scripts (this track updates one of them) +- `docs/reports/EXCEPTION_HANDLING_AUDIT_20260616.md` — the parent audit report (the original 268-site inventory) +- `conductor/tracks/result_migration_20260616/spec.md` — the umbrella spec (the parent) +- `conductor/tracks/exception_handling_audit_20260616/spec.md` — the audit track (the grandparent) +- `scripts/audit_exception_handling.py` — the audit script being updated +- `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 + +## 6. Out of Scope (Explicit) + +- **Migrating any production code.** Sub-track 1 is informational; the migration happens in sub-tracks 2-4. +- **Updating the umbrella spec's recommendation sequence** (sub-tracks 2-4 ordering is unchanged). +- **Adding new `Result` patterns to areas that don't have any** (this track classifies EXISTING sites only). +- **Refactoring the audit script's overall architecture** (only the `_classify_except` and `_classify_raise` functions are touched). +- **The 211 violations + remaining 6 INTERNAL_RETHROW-equivalent sites** (those are sub-tracks 2-5's work). + +## 7. Verification Criteria + +- **G1:** `docs/reports/RESULT_MIGRATION_REVIEW_PASS_20260617.md` exists and contains a per-site decision table for all 43 sites. +- **G2:** `scripts/audit_exception_handling.py` has at least 1 new heuristic for commonly-compliant patterns (count recorded in the report). +- **G3:** Re-running the audit post-heuristics: UNCLEAR count is 0 (±2 acceptable). +- **G4:** `conductor/tracks/result_migration_20260616/spec.md` §1.3 is updated with the post-review site counts. +- **G5:** Full test pass count: 1288 + 4 + 0 (unchanged; informational track). +- **G6:** Atomic commits: spec, plan, metadata + state, per-file review batches, audit script update, umbrella spec update, report, final verification. + +## 8. Risks + +- **R1:** Review reveals more sites are violations than the audit's heuristics suggest → the migration scope for sub-tracks 2-4 grows; mitigated by the per-site decision table that records every site. +- **R2:** User disagrees with a classification on a disputed case → the plan defers to the user as the final arbiter; no site is left without a decision. +- **R3:** Audit script updates introduce regressions (e.g., a new heuristic misclassifies a known site) → mitigated by running the audit before and after each heuristic change and comparing counts. + +## 9. Notes for the Tier 2 Implementer + +- This is a **research task, not a refactor**. Read the code, classify the site, write the decision. No production code edits. +- For each site, read the snippet + 2-3 lines of context. The audit's `context` field gives the enclosing function name; `line` gives the exact line. +- For UNCLEAR sites, the question is: "is this a pattern the audit script SHOULD recognize as compliant?" If yes, mark `compliant` and add a heuristic. If no, mark `migration-target`. +- For INTERNAL_RETHROW sites, the question is: "is this one of the 3 legitimate re-raise patterns?" Check the styleguide's Re-Raise Patterns section. If none, mark `migration-target`. +- 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. 43 sites; ~2-3 hours of focused review. diff --git a/conductor/tracks/result_migration_review_pass_20260617/state.toml b/conductor/tracks/result_migration_review_pass_20260617/state.toml new file mode 100644 index 00000000..5e65a907 --- /dev/null +++ b/conductor/tracks/result_migration_review_pass_20260617/state.toml @@ -0,0 +1,92 @@ +# Track state for result_migration_review_pass_20260617 +# Updated by Tier 2 Tech Lead as tasks complete + +[meta] +track_id = "result_migration_review_pass_20260617" +name = "Result Migration Sub-Track 1 (Review Pass)" +status = "active" +current_phase = 0 # 0 = pre-Phase 1; 1..N = in Phase N; "complete" if all phases done +last_updated = "2026-06-17" + +[parent] +umbrella = "result_migration_20260616" +sub_track_of_5 = 1 + +[blocked_by] +# Per the umbrella's spec section 1.3, sub-track 1 has no dependency (it's the first) +result_migration_20260616 = "umbrella specced; sub-track 1 is independent" +exception_handling_audit_20260616 = "shipped 2026-06-16" + +[blocks] +# Sub-tracks 2-4 depend on this sub-track's per-site decisions +result_migration_small_files = "blocked; needs per-site decisions in the report" +result_migration_app_controller = "blocked; needs per-site decisions in the report" +result_migration_gui_2 = "blocked; needs per-site decisions in the report" + +[phases] +phase_1 = { status = "in_progress", checkpointsha = "", name = "Setup (sub-track folder + tracks.md update)" } +phase_2 = { status = "pending", checkpointsha = "", name = "Review the 24 UNCLEAR sites (6 files)" } +phase_3 = { status = "pending", checkpointsha = "", name = "Classify the 19 INTERNAL_RETHROW sites (7 files)" } +phase_4 = { status = "pending", checkpointsha = "", name = "Update the audit script's heuristics" } +phase_5 = { status = "pending", checkpointsha = "", name = "Report (per-site decision table + umbrella scope update)" } +phase_6 = { status = "pending", checkpointsha = "", name = "Verification (audit re-run + test pass count + mark complete)" } + +[tasks] +# Phase 1: Setup +t1_1 = { status = "in_progress", commit_sha = "", description = "Create the sub-track folder with spec/plan/metadata/state" } +t1_2 = { status = "pending", commit_sha = "", description = "Update conductor/tracks.md with the sub-track row" } + +# Phase 2: Review UNCLEAR (6 files, 24 sites) +t2_1 = { status = "pending", commit_sha = "", description = "Review src/gui_2.py UNCLEAR sites (13)" } +t2_2 = { status = "pending", commit_sha = "", description = "Review src/mcp_client.py UNCLEAR sites (4, baseline)" } +t2_3 = { status = "pending", commit_sha = "", description = "Review src/ai_client.py UNCLEAR sites (2, baseline)" } +t2_4 = { status = "pending", commit_sha = "", description = "Review src/app_controller.py UNCLEAR sites (2)" } +t2_5 = { status = "pending", commit_sha = "", description = "Review src/models.py UNCLEAR sites (2)" } +t2_6 = { status = "pending", commit_sha = "", description = "Review src/multi_agent_conductor.py UNCLEAR sites (1)" } + +# Phase 3: Classify INTERNAL_RETHROW (7 files, 19 sites) +t3_1 = { status = "pending", commit_sha = "", description = "Classify src/ai_client.py INTERNAL_RETHROW sites (6, baseline)" } +t3_2 = { status = "pending", commit_sha = "", description = "Classify src/rag_engine.py INTERNAL_RETHROW sites (4, baseline)" } +t3_3 = { status = "pending", commit_sha = "", description = "Classify src/app_controller.py INTERNAL_RETHROW sites (3)" } +t3_4 = { status = "pending", commit_sha = "", description = "Classify src/gui_2.py INTERNAL_RETHROW sites (2)" } +t3_5 = { status = "pending", commit_sha = "", description = "Classify src/api_hooks.py INTERNAL_RETHROW sites (2)" } +t3_6 = { status = "pending", commit_sha = "", description = "Classify src/models.py INTERNAL_RETHROW sites (1)" } +t3_7 = { status = "pending", commit_sha = "", description = "Classify src/warmup.py INTERNAL_RETHROW sites (1)" } + +# Phase 4: Audit script heuristics +t4_1 = { status = "pending", commit_sha = "", description = "Add heuristics for the 5-10 most common compliant patterns in scripts/audit_exception_handling.py" } +t4_2 = { status = "pending", commit_sha = "", description = "Verify the updated classification (UNCLEAR count drops to ~0)" } + +# Phase 5: Report +t5_1 = { status = "pending", commit_sha = "", description = "Write docs/reports/RESULT_MIGRATION_REVIEW_PASS_20260617.md with per-site decision table" } +t5_2 = { status = "pending", commit_sha = "", description = "Update the umbrella spec's per-sub-track plan with the post-review scope" } + +# Phase 6: Verification +t6_1 = { status = "pending", commit_sha = "", description = "Verify the updated audit script (--by-size, UNCLEAR count)" } +t6_2 = { status = "pending", commit_sha = "", description = "Verify test pass count is unchanged (1288 + 4 + 0)" } +t6_3 = { status = "pending", commit_sha = "", description = "Mark the sub-track as completed (metadata.json + state.toml + tracks.md)" } + +[verification] +phase_1_setup_complete = false +phase_2_unclear_review_complete = false +phase_3_rethrow_classification_complete = false +phase_4_heuristics_updated = false +phase_5_report_written = false +phase_6_verification_complete = false +report_exists = false +umbrella_spec_updated = false +audit_uncleft_count_zero = false +test_pass_count_unchanged = false +metadata_json_status_completed = false + +[scope_metrics] +unclear_sites_target = 24 +unclear_sites_compliant = 0 +unclear_sites_migration_target = 0 +unclear_sites_left_unclear = 0 +rethrow_sites_target = 19 +rethrow_sites_pattern_1 = 0 +rethrow_sites_pattern_2 = 0 +rethrow_sites_pattern_3 = 0 +rethrow_sites_migration_target = 0 +heuristics_added = 0