Private
Public Access
conductor(track): spec/plan/metadata for result_migration_20260616 (5 sub-tracks, NO day estimates)
This commit is contained in:
@@ -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_<YYYYMMDD>.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_<YYYYMMDD>/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 |
|
||||
Reference in New Issue
Block a user