conductor(track): init result_migration_review_pass_20260617 (sub-track 1 of 5)
Sub-track 1 of the 5-sub-track result_migration_20260616 campaign. Audit-driven research task: classify 43 ambiguous exception-handling sites (24 UNCLEAR + 19 INTERNAL_RETHROW across 11 files) and update the audit script's heuristics. No production code change. Scope: 11 files, 43 sites, T-shirt S. The per-site decisions feed sub-tracks 2-4 (small_files, app_controller, gui_2) as their starting migration scope. Files: spec.md, plan.md, metadata.json, state.toml under conductor/tracks/result_migration_review_pass_20260617/. Row added to conductor/tracks.md.
This commit is contained in:
+2
-1
@@ -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) |
|
||||
|
||||
@@ -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_<future_date> (needs the per-site decisions)",
|
||||
"result_migration_app_controller_<future_date> (needs the per-site decisions)",
|
||||
"result_migration_gui_2_<future_date> (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"
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -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)
|
||||
@@ -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.
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user