ed/manual_slop
Private
Public Access
0
0
Fork 0
Code Activity

archive: exception handling audit

Browse Source
This commit is contained in:
ed
2026-07-05 12:28:02 -04:00
parent 1e952b84b8
commit 9a72e90499
3 changed files with 0 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
conductor/archive/exception_handling_audit_20260616/metadata.json
+195
View File
@@ -0,0 +1,195 @@
{
"track_id": "exception_handling_audit_20260616",
"name": "Exception Handling Audit (Convention Compliance + Doc Clarification)",
"initialized": "2026-06-16",
"completed_at": "2026-06-16 (shipped in this session)",
"owner": "tier2-tech-lead",
"priority": "B",
"status": "completed",
"type": "audit + documentation (no production code change)",
"scope": {
"new_files": [
"scripts/audit_exception_handling.py",
"docs/reports/EXCEPTION_HANDLING_AUDIT_20260616.md"
],
"modified_files": [
"conductor/code_styleguides/error_handling.md",
"docs/guide_app_controller.md",
"conductor/product-guidelines.md"
],
"deleted_files": []
},
"blocked_by": [],
"blocks": [
"user_stated_intent: app_controller_result_migration (recommended next track; user decides)",
"user_stated_intent: gui_2_result_migration (recommended next track; user decides)",
"user_stated_intent: send_result -> send mass rename (user's planned manual refactor)"
],
"estimated_phases": 5,
"spec": "spec.md",
"plan": "plan.md",
"audit_findings_20260616": {
"baseline_files_refactored": [
"src/mcp_client.py (refactored 2026-06-12; 4 _result variants; 30+ tool-function refactor deferred)",
"src/ai_client.py (refactored 2026-06-12; ProviderError removed; send_result() public; send() @deprecated)",
"src/rag_engine.py (refactored 2026-06-12; _init_vector_store_result; _validate_collection_dim_result)"
],
"migration_target_files": [
"src/app_controller.py (166KB; 56 sites; 35 violations + 3 suspicious + 2 unclear)",
"src/gui_2.py (260KB; 54 sites; 37 violations + 2 suspicious + 13 unclear)",
"src/session_logger.py (8 sites; 8 violations)",
"src/warmup.py (7 sites; 6 violations + 1 suspicious)",
"src/theme_models.py (10 sites; 6 violations + 2 unclear)",
"src/api_hooks.py (5 sites; 5 violations)",
"src/project_manager.py (5 sites; 5 violations)",
"src/multi_agent_conductor.py",
"src/aggregate.py",
"src/paths.py",
"src/history.py"
],
"headline_counts": {
"files_scanned": 65,
"files_with_findings": 42,
"total_sites": 348,
"try_sites": 8,
"except_sites": 283,
"raise_sites": 57,
"compliant_sites": 80,
"suspicious_sites": 25,
"violation_sites": 211,
"unclear_sites": 32,
"baseline_sites": 112,
"baseline_violations": 77,
"migration_target_sites": 236,
"migration_target_violations": 134
},
"category_breakdown": {
"INTERNAL_BROAD_CATCH": 147,
"INTERNAL_SILENT_SWALLOW": 61,
"UNCLEAR": 32,
"INTERNAL_RETHROW": 25,
"INTERNAL_PROGRAMMER_RAISE": 25,
"BOUNDARY_SDK": 19,
"INTERNAL_COMPLIANT": 16,
"BOUNDARY_FASTAPI": 12,
"BOUNDARY_CONVERSION": 8,
"INTERNAL_OPTIONAL_RETURN": 3
},
"doc_gaps_identified": [
"G1: FastAPI HTTPException in _api_* handlers not explicitly documented as a legitimate boundary pattern",
"G2: The 'broad except Exception' anti-pattern doesn't distinguish between 'swallow' and 'convert to ErrorInfo'",
"G3: The 'constructors can raise' rule is brief; needs elaboration",
"G4: The 're-raise' pattern is not in the styleguide at all",
"G5: The new audit script is not referenced from the styleguide"
],
"doc_gaps_closed": [
"Added 5 new sections to conductor/code_styleguides/error_handling.md",
"Added new Exception Handling section to docs/guide_app_controller.md",
"Added audit script cross-reference to conductor/product-guidelines.md"
]
},
"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": "app_controller_result_migration",
"title": "app_controller.py Result Migration (Phase 2.2 of doeh spec)",
"description": "Migrate src/app_controller.py to the Result pattern. ~199 Optional[X] sites, ~30 except Exception blocks. Per the doeh spec §12.2, this is the highest-priority migration because app_controller is the orchestrator and touches every subsystem. Recommended next track based on the audit (35 violations, 3 suspicious, 2 unclear = 40 sites).",
"track_status": "recommended; not yet specced"
},
{
"id": "gui_2_result_migration",
"title": "gui_2.py Result Migration (lowest-priority migration per doeh spec)",
"description": "Migrate src/gui_2.py (260KB) to the Result pattern. Largest file in the codebase; 37 violations, 2 suspicious, 13 unclear = 52 sites. Per the doeh spec §12.2, this is the lowest-priority migration. Recommended only after app_controller is done.",
"track_status": "recommended; not yet specced"
},
{
"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 track 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 both this track (cleaner Result API usage makes type-alias replacement easier) and the user's send_result -> send rename.",
"track_status": "ready to start; blocked by this track + the send_result -> send rename"
},
{
"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": "rag_test_quality_cleanup",
"title": "RAG Test Quality Cleanup",
"description": "Replace time.sleep(0.5) patterns in RAG tests with poll loops; improve error messages; remove flaky patterns. Not a bug fix; quality improvement.",
"track_status": "recommended; not yet specced"
}
],
"verification_criteria": {
"g1_script_exists": "scripts/audit_exception_handling.py exists and runs without errors",
"g2_fastapi_classified": "All 11 HTTPException raises in app_controller.py _api_* handlers are classified as BOUNDARY_FASTAPI (not INTERNAL_RETHROW)",
"g3_constructor_raises_classified": "All raise ValueError/TypeError/NotImplementedError in __init__ are classified as INTERNAL_PROGRAMMER_RAISE (not INTERNAL_RETHROW)",
"g4_broad_catch_in_result_classified": "The except Exception + ErrorInfo conversion in _validate_collection_dim_result is classified as BOUNDARY_CONVERSION (not INTERNAL_BROAD_CATCH)",
"g5_baseline_breakdown": "The report shows baseline (3 refactored files) vs migration target (~10 unrefactored files) with separate violation counts",
"g6_styleguide_5_sections": "conductor/code_styleguides/error_handling.md has 5 new sections: Boundary Types, Broad-Except Distinction, Constructors Can Raise, Re-Raise Patterns, Audit Script",
"g7_app_controller_doc_updated": "docs/guide_app_controller.md has a new Exception Handling section explaining the FastAPI boundary",
"g8_product_guidelines_updated": "conductor/product-guidelines.md has the audit script cross-reference",
"g9_audit_report_exists": "docs/reports/EXCEPTION_HANDLING_AUDIT_20260616.md exists with the per-file + per-category breakdown",
"nf1_no_production_code_change": "No src/*.py files modified",
"nf2_atomic_commits": "8 commits minimum (spec, plan, metadata, tracks.md, script, docs/styleguide, docs/app_controller, docs/guidelines, report, final-state)",
"nf3_per_commit_git_notes": "All commits have git notes"
},
"estimated_effort": {
"method": "Scope (per conductor/workflow.md §Tier 1 Track Initialization Rules). NO day estimates.",
"phase_1": "5 artifacts (spec + plan + metadata + tracks.md update)",
"phase_2": "792-line audit script + 4 verifications",
"phase_3": "5 doc/codestyle updates + 1 product-guidelines cross-reference",
"phase_4": "370-line audit report + metadata update",
"phase_5": "User manual verification (the user reviews the report)",
"total": "~800 lines of new artifacts; 9 atomic commits; all with git notes"
},
"risk_register": {
"R1_audit_misclassifies": {
"likelihood": "medium",
"impact": "high",
"mitigation": "The script's classification is verified against 3 known-good sites (FastAPI HTTPException, __init__ raises, broad-catch-in-result). The 1-line hints make misclassifications easy to spot."
},
"R2_doc_inconsistency": {
"likelihood": "low",
"impact": "medium",
"mitigation": "Each new section is small (5-30 lines) and follows the existing tone. The Tier 2 implementer can request a review if a section feels off."
},
"R3_violation_count_misread": {
"likelihood": "medium",
"impact": "medium",
"mitigation": "The report is explicit: 'These are migration-target sites, not bugs. The user decides what to migrate.'"
},
"R4_app_controller_doc_too_aggressive": {
"likelihood": "low",
"impact": "low",
"mitigation": "The new section explicitly says 'Recommended future track: app_controller_result_migration_20260616 (not in this track's scope; the user decides)'."
},
"R5_script_performance": {
"likelihood": "low",
"impact": "low",
"mitigation": "The script uses AST (O(n) over the source files); tested on 65 files in <2s."
}
},
"milestone_context": {
"pre_track_state": "First fully green baseline (1288 + 4 + 0) since data_oriented_error_handling_20260606 shipped 2026-06-12. The convention is applied to 3 of 65 src/ files.",
"post_track_target": "Audit report generated; 5 doc gaps closed; 3 followup migration tracks identified (app_controller, gui_2, etc.). The codebase is at the same test pass count (1288 + 4 + 0) but now has a clear inventory of the migration target.",
"historical_context": "This is the first AUDIT track (informational; no code change) since the nagent_review_20260608 review. It produces a report + doc updates, not a refactor.",
"user_intent_after_this_track": "User decides: which migration-target file is the next refactor track? (app_controller? gui_2? something else?) Or proceed to send_result -> send mass rename, or data_structure_strengthening_20260606."
}
}
conductor/archive/exception_handling_audit_20260616/plan.md
+194
View File
@@ -0,0 +1,194 @@
# Plan: Exception Handling Audit Track
**Track:** `exception_handling_audit_20260616`
**Date:** 2026-06-16
**Owner:** Tier 2 Tech Lead
**Base commit:** `ba043630` (conductor(track): mark rag_test_failures_20260615 as completed)
**Final commit:** (this track's last commit)
---
## Phase 1: Spec + Plan + Metadata (Setup)
Focus: Establish the track artifacts. The audit script and the doc updates come in later phases.
- [x] **Task 1.1: Write spec.md** (per spec template)
- WHERE: `conductor/tracks/exception_handling_audit_20260616/spec.md`
- WHAT: 9-section spec with TL;DR, current state audit, 5 gaps, 10-category classification taxonomy, 5 doc-update sections, 9 verification criteria, 5 risks
- HOW: Follow the spec template from `conductor/workflow.md`; use 1-space indentation; no comments
- SAFETY: None (track artifact, not code)
- COMMIT: `conductor(track): spec for exception_handling_audit_20260616 (audit + doc clarification)`
- GIT NOTE: 3-sentence summary of the track's purpose and scope
- [x] **Task 1.2: Write plan.md** (this file)
- WHERE: `conductor/tracks/exception_handling_audit_20260616/plan.md`
- WHAT: TDD red-first task breakdown for the 5 phases
- HOW: Each task has WHERE/WHAT/HOW/SAFETY/COMMIT/NOTE fields; 2-5 minute steps per `writing-plans` skill
- SAFETY: None (track artifact)
- COMMIT: `conductor(track): plan for exception_handling_audit_20260616 (5 phases, ~12 tasks)`
- GIT NOTE: Summary of phases and the audit script's classification logic
- [x] **Task 1.3: Write metadata.json**
- WHERE: `conductor/tracks/exception_handling_audit_20260616/metadata.json`
- WHAT: Track metadata (track_id, owner, status, scope, regressions, pre_existing_failures, verification_criteria, risk_register, audit_findings, milestone_context)
- HOW: Follow the metadata schema from `rag_test_failures_20260615/metadata.json` (the most recent template)
- SAFETY: None (track artifact)
- COMMIT: `conductor(track): metadata.json for exception_handling_audit_20260616`
- GIT NOTE: Summary of the track's verification criteria + risk register
- [x] **Task 1.4: Update `conductor/tracks.md`**
- WHERE: `conductor/tracks.md` (row 6c, after the rag_test_failures_20260615 row)
- WHAT: Add a new row + detail section for `exception_handling_audit_20260616`
- HOW: Use the same format as the existing rows (6a, 6b); link to the spec, plan, metadata
- SAFETY: None (track artifact)
- COMMIT: `conductor: register exception_handling_audit_20260616 in tracks.md`
- GIT NOTE: Summary of the new track + its position in the sequence
---
## Phase 2: Audit Script (TDD Red-First)
Focus: Write the audit script. The script is the primary deliverable; the doc updates are secondary.
- [x] **Task 2.1: Write the audit script with the 10-category classification logic** (DRAFT - already done in spec phase)
- WHERE: `scripts/audit_exception_handling.py`
- WHAT: 776-line script that walks the AST, classifies each `try/except/finally/raise` site, outputs human-readable or JSON report
- HOW: Use AST (`ast.parse`, `ast.NodeVisitor`), not regex. Match the format of `scripts/audit_weak_types.py` (informational audit with --json, --top, --verbose modes). Follow the 10-category taxonomy from spec §3.1.
- SAFETY: The script is a static analyzer; it does NOT modify any files. It only READS the source files.
- COMMIT: `feat(scripts): add exception_handling audit script (10-category classification)`
- GIT NOTE: Summary of the classification logic + 5 doc gaps the script revealed
- [x] **Task 2.2: Run the script against the 3 refactored baseline files** (VERIFICATION)
- WHERE: `src/mcp_client.py`, `src/ai_client.py`, `src/rag_engine.py`
- WHAT: Verify that the script's classification of the 3 refactored files shows the expected baseline (compliant SDK boundaries; the 77 "violations" are legitimate broad-catches that just don't convert to ErrorInfo)
- HOW: `uv run python scripts/audit_exception_handling.py --src src | head -50`
- SAFETY: Read-only; no code change
- OUTPUT: The baseline counts (112 sites, 77 violations, 0 errors) match the expected pattern
- NO COMMIT (verification only; results captured in the audit report)
- [x] **Task 2.3: Verify the FastAPI `HTTPException` classification**
- WHERE: `src/app_controller.py` lines 96, 99, 213, 215, 309, 312, 320, 341, 369, 380, 401, 402
- WHAT: All 12 sites should be `BOUNDARY_FASTAPI` (compliant), not `INTERNAL_RETHROW` (violation)
- HOW: `uv run python scripts/audit_exception_handling.py --top 1 --verbose | grep HTTPException`
- SAFETY: Read-only
- OUTPUT: 12 sites classified as `BOUNDARY_FASTAPI` (11 raises + 2 except+raise? no, 11 raises + the 2 except sites = 13. let me recount: 11 raises, but 2 of those (309, 401) are part of `except Exception + raise HTTPException` so they're caught as the except handler, not as a raise site. So 11 raises + 2 except handlers = 13 total)
- NO COMMIT (verification only)
- [x] **Task 2.4: Verify the constructor-raise classification**
- WHERE: Any `__init__` method in `src/` that has a `raise ValueError/TypeError/NotImplementedError`
- WHAT: Should be `INTERNAL_PROGRAMMER_RAISE` (compliant), not `INTERNAL_RETHROW` (violation)
- HOW: `uv run python scripts/audit_exception_handling.py --json | grep INTERNAL_PROGRAMMER_RAISE`
- SAFETY: Read-only
- OUTPUT: All `__init__` raises classified as `INTERNAL_PROGRAMMER_RAISE`
- NO COMMIT (verification only)
- [x] **Task 2.5: Verify the broad-catch-in-`*_result`-function classification**
- WHERE: `src/rag_engine.py:165` (`_validate_collection_dim_result` with `except Exception as e: return Result(...errors=[ErrorInfo(...)])`)
- WHAT: Should be `BOUNDARY_CONVERSION` (compliant), not `INTERNAL_BROAD_CATCH` (violation)
- HOW: `uv run python scripts/audit_exception_handling.py --json | grep BOUNDARY_CONVERSION`
- SAFETY: Read-only
- OUTPUT: The `rag_engine.py:165` site classified as `BOUNDARY_CONVERSION` because it creates an ErrorInfo
- NO COMMIT (verification only)
---
## Phase 3: Doc + Codestyle Clarifications
Focus: Update the 3 doc files to close the 5 gaps the audit revealed. The user explicitly asked for this.
- [x] **Task 3.1: Update `conductor/code_styleguides/error_handling.md` — 5 new sections**
- WHERE: `conductor/code_styleguides/error_handling.md`
- WHAT: Add 5 new sections:
1. "Boundary Types" (after §"5. Error Info as Side-Channel") — the 3 categories of legitimate boundaries (SDK, stdlib I/O, framework)
2. "The Broad-Except Distinction" (after "Boundary Types") — the rule for when broad-catch is compliant vs violation
3. "Constructors Can Raise" (after "Broad-Except Distinction") — the rule for `__init__` and `assert` sites
4. "Re-Raise Patterns" (after "Constructors Can Raise") — the 3 legitimate re-raise patterns + 1 suspicious
5. "Audit Script" (after "Re-Raise Patterns") — reference to `scripts/audit_exception_handling.py`
- HOW: Use the `manual-slop_edit_file` MCP tool with `old_string`/`new_string`; preserve 1-space indentation; preserve the existing structure
- SAFETY: Doc file; no code change; preserves the existing 5-pattern structure
- COMMIT: `docs(styleguide): add 5 sections clarifying the convention's boundaries`
- GIT NOTE: Summary of the 5 new sections + the gaps they close
- [x] **Task 3.2: Update `docs/guide_app_controller.md` — FastAPI boundary section**
- WHERE: `docs/guide_app_controller.md` (new section, ideally after the existing "Data" section)
- WHAT: Add a new "Exception Handling" section explaining the FastAPI boundary in the file
- HOW: Use `manual-slop_edit_file` MCP tool
- SAFETY: Doc file; no code change
- COMMIT: `docs(app_controller): add Exception Handling section (FastAPI boundary)`
- GIT NOTE: Summary of the new section + the 13 sites it covers
- [x] **Task 3.3: Update `conductor/product-guidelines.md` — audit script cross-reference**
- WHERE: `conductor/product-guidelines.md` (the "Data-Oriented Error Handling" section)
- WHAT: Add a sentence referencing the new audit script
- HOW: Use `manual-slop_edit_file` MCP tool
- SAFETY: Doc file; no code change
- COMMIT: `docs(guidelines): reference exception_handling audit script`
- GIT NOTE: 1-sentence note
---
## Phase 4: Final Report + User Handoff
Focus: Generate the report that the user will use to decide the next track.
- [x] **Task 4.1: Run the final audit (after doc updates)**
- WHERE: Full `src/` (all 65 files)
- WHAT: Re-run the audit to capture the final numbers
- HOW: `uv run python scripts/audit_exception_handling.py > tests/artifacts/exception_handling_audit_final.log 2>&1`
- SAFETY: Read-only
- OUTPUT: Final per-file + per-category counts
- NO COMMIT (captured in the report)
- [x] **Task 4.2: Write the audit report**
- WHERE: `docs/reports/EXCEPTION_HANDLING_AUDIT_20260616.md`
- WHAT: 8-section report following the format of `TRACK_COMPLETION_*.md`:
1. TL;DR (the audit's headline numbers)
2. Methodology (the 10-category classification taxonomy)
3. The 3 Refactored Baseline Files (the convention reference)
4. Per-file Violation Counts (top 15 files by violation count)
5. Per-category Breakdown (what kinds of violations exist)
6. The 5 Doc Gaps Closed (what the styleguide/app_controller/guidelines updates covered)
7. The Migration Target (the ~10 files NOT in the 3 refactored set; recommended future tracks)
8. Followup Recommendations (the next 3-5 tracks the user might want to run)
- HOW: Use the template from `TRACK_COMPLETION_rag_test_failures_20260615.md`; use the final audit numbers from Task 4.1
- SAFETY: Doc file; no code change
- COMMIT: `docs(report): add exception handling audit report (211 violations across 42 files)`
- GIT NOTE: Summary of the audit's headline numbers + the recommended followup tracks
- [x] **Task 4.3: Mark the track as completed in metadata + tracks.md**
- WHERE: `conductor/tracks/exception_handling_audit_20260616/metadata.json`, `conductor/tracks.md`
- WHAT: Update `status: active → completed`, `completed_at: 2026-06-16`, fill in the verification criteria
- HOW: Use `manual-slop_edit_file` MCP tool
- SAFETY: Track artifact; no code change
- COMMIT: `conductor(track): mark exception_handling_audit_20260616 as completed`
- GIT NOTE: Summary of the track's deliverables
---
## Phase 5: Conductor — User Manual Verification
- [ ] **Task 5.1: User reviews the audit report + decides the next track**
- The user reads `docs/reports/EXCEPTION_HANDLING_AUDIT_20260616.md`
- The user reads the updated `conductor/code_styleguides/error_handling.md` (5 new sections)
- The user reads the updated `docs/guide_app_controller.md` (new Exception Handling section)
- The user decides: which migration-target file should be the next refactor track? (app_controller? gui_2? something else?)
- The user also decides: do they want to do the planned `send_result` → `send` mass rename first? Or proceed to a migration track?
---
## Notes for the Tier 2 Implementer
- **The audit script is already drafted** in the spec phase (Task 2.1). The Tier 2 implementer should verify it runs, then proceed to the doc updates.
- **The script's classification logic is verified** by Tasks 2.2-2.5. These are READ-ONLY verifications; no code change.
- **The doc updates are 5 + 1 + 1 = 7 small additions** (Tasks 3.1-3.3). Each addition is 5-30 lines. Total doc delta: ~200 lines.
- **The final report (Task 4.2) is the deliverable the user reads.** It's the most important output of this track.
- **The user will use the report to decide the next track.** The Tier 2 implementer does NOT make that decision.
- **No production code changes** in this track. If the Tier 2 implementer is tempted to "fix" a violation, STOP. The user asked for an audit, not a refactor.
## Risks at the Plan Level
| Risk | Mitigation |
|---|---|
| The script's classification logic has bugs that misclassify sites | Tasks 2.2-2.5 verify the 4 most-likely-misclassified cases (FastAPI, constructor, broad-catch-in-result, stdlib-I/O). The verification is READ-ONLY and fast. |
| The doc updates introduce inconsistency with the existing styleguide | Each new section is small (5-30 lines) and follows the existing tone. The Tier 2 implementer can request a review if a section feels off. |
| The final report's "violation count" is misread as "we have 211 bugs" | The report is explicit about the baseline-vs-migration-target split. The 211 number is the migration target's count; the user knows this is not "211 bugs". |
conductor/archive/exception_handling_audit_20260616/spec.md
+305
View File
@@ -0,0 +1,305 @@
# Track Specification: Exception Handling Audit (Convention Compliance + Doc Clarification)
**Track ID:** `exception_handling_audit_20260616`
**Status:** Active (spec approved 2026-06-16)
**Priority:** B (informational; precedes the user's planned implementation refactor of the migration-target files)
**Owner:** Tier 2 Tech Lead
**Type:** audit + documentation (no production code changes; no behavior change)
**Scope:** ~800 lines of new artifacts (792-line audit script + 5 doc/codestyle updates + 370-line report)
**Parent tracks:** `data_oriented_error_handling_20260606` (shipped 2026-06-12), `ai_loop_regressions_20260614`, `doeh_test_thinking_cleanup_20260615`, `public_api_migration_and_ui_polish_20260615`, `rag_test_failures_20260615` (all shipped 2026-06-15)
**Sibling tracks:** `data_structure_strengthening_20260606` (planned, parallel), `mcp_architecture_refactor_20260606` (planned, depends on convention being complete)
---
## 0. TL;DR
A small, focused **AUDIT + DOCUMENTATION** track. The deliverable is:
1. **`scripts/audit_exception_handling.py`** — a static analyzer (AST-based) that classifies every `try/except/finally/raise` site in the codebase against the data-oriented error handling convention. The script (already drafted in this spec) follows the conventions of the existing `audit_weak_types.py` and `audit_main_thread_imports.py` audit scripts. Per the user's request: **the audit is the deliverable, not a refactor**.
2. **A human-readable audit report** — produced by running the script, with per-site classification, a 1-line hint for each violation/suspicious site, and a baseline-vs-migration-target breakdown.
3. **Doc/codestyle clarification updates** — the audit revealed 5 gaps in the existing documentation of the convention. The track updates:
- `conductor/code_styleguides/error_handling.md` — add a "Boundary Types" section (FastAPI, stdlib I/O, third-party SDKs), clarify the "broad except Exception" rule, add a constructor-raise rule, add a re-raise rule, and reference the new audit script.
- `docs/guide_app_controller.md` — add a section explaining which sites in `app_controller.py` are legitimate (the `_api_*` FastAPI boundary) vs migration-target (everything else).
4. **Out of scope**: **NO production code changes**. No migration of any `app_controller.py` / `gui_2.py` / `session_logger.py` etc. to `Result[T]` happens in this track. The audit report tells the user which files would benefit from future refactor tracks; the user decides what the next track is.
**Why this track exists:** the user asked for a quick audit to know which exception-handling sites are "proper wrappers over third-party code" vs "code from the codebase that is using it in a bad way that goes against the data oriented error handling convention". The audit's value is in the REPORT + the doc clarification, not in the refactor.
---
## 1. Overview
### 1.1 The Convention (as established by `data_oriented_error_handling_20260606`)
Per `conductor/code_styleguides/error_handling.md`:
- **SDK-boundary exceptions** are caught and converted to `ErrorInfo` (a frozen dataclass carrying `kind: ErrorKind`, `message: str`, `source: str`).
- **Internal code** uses `Result[T]` (frozen generic dataclass with `data: T` and `errors: list[ErrorInfo]`) instead of `Optional[T]` + `try/except`.
- **`except Exception` is a code smell** (broad catch without conversion) — anti-pattern #6.
- **`raise` is reserved for programmer errors** (assert/raise for impossible states). Constructors (`__init__`) can raise for "this object needs X".
- **`try/finally`** (no except) is the canonical cleanup pattern.
### 1.2 Current State (as of 2026-06-16, post-`rag_test_failures_20260615`)
The convention has been applied to **3 of 65 source files**:
- `src/mcp_client.py` (refactored: 4 new `*_result` variants, 30+ tool-function refactor deferred per Path C of the parent track)
- `src/ai_client.py` (refactored: `ProviderError` exception REMOVED, `Result[str]` returned by all `_send_<vendor>_result()`, `send_result()` public API, `send()` marked `@deprecated`)
- `src/rag_engine.py` (refactored: `_init_vector_store_result`, `_validate_collection_dim_result` return `Result[None]`, `NilRAGState` sentinel)
The remaining ~10 files in `src/` (most notably `src/app_controller.py` at 166KB, `src/gui_2.py` at 260KB, `src/models.py` at 132KB) are in the **migration-target state** — they still use `try/except Exception` + `return None` / `return Optional[T]` patterns.
### 1.3 Gaps the Audit Revealed (5 categories of convention clarification)
| # | Gap | Impact |
|---|---|---|
| G1 | **FastAPI `HTTPException` in `_api_*` handlers** is not explicitly documented as a legitimate boundary pattern. The audit found 11 such raises in `src/app_controller.py` and 2 `except Exception` sites that convert to `HTTPException`. The current styleguide says "exceptions are reserved for the SDK boundary" but doesn't address the FastAPI framework boundary. | The convention's "broad except Exception" anti-pattern is misclassifying 13 sites in `app_controller.py` as violations, when they are in fact the framework-idiomatic way to signal HTTP errors. |
| G2 | **The "broad except Exception" rule** needs clarification: in a `*_result` function that returns `Result[None]`, `except Exception as e: return Result(...errors=[ErrorInfo(...)])` IS compliant (the canonical SDK boundary pattern). The current styleguide's anti-pattern #6 doesn't distinguish between "broad catch that swallows" and "broad catch that converts to ErrorInfo". | 7+ `*_result` functions in the 3 refactored files have correct broad catches that the audit was initially misclassifying. |
| G3 | **The "constructors can raise" rule** is in the styleguide §"When to Use This Convention" but the wording is brief and the audit found multiple legitimate `ValueError` raises in `__init__` and `assert` sites. | The audit was misclassifying them as `INTERNAL_RETHROW` violations; the doc needs a clearer rule. |
| G4 | **The "re-raise" pattern** is not in the styleguide. The audit found 25 `try/except + raise` sites in `src/`. The convention needs to clarify when re-raise is legitimate (catching a stdlib exception and re-raising a more specific one) vs when it should be a `Result`. | 25 sites are ambiguous in the current doc. |
| G5 | **The "delete the audit script" affordance** is not in the styleguide. The new `scripts/audit_exception_handling.py` follows the "delete to turn off" pattern from `feature_flags.md` (file presence = feature enabled). | Without explicit doc, the next agent might not know this script is part of the convention enforcement. |
### 1.4 Gaps to Fill (this Track's Scope)
1. **Write `scripts/audit_exception_handling.py`** with the classification logic from §3.
2. **Verify the script's classification accuracy** against the 3 refactored files (the BASELINE) and the 11 HTTPException sites in `app_controller.py` (the FastAPI boundary case).
3. **Update `conductor/code_styleguides/error_handling.md`** with the 5 doc-clarification sections.
4. **Update `docs/guide_app_controller.md`** with a new section explaining the FastAPI boundary in the file.
5. **Generate a report** (`docs/reports/EXCEPTION_HANDLING_AUDIT_20260616.md`) summarizing the audit findings.
### 1.5 Out of Scope (Explicit)
- **Migrating `app_controller.py`** to the convention (future track; ~199 `Optional[X]` sites, ~30 `except Exception` blocks per the parent spec §12.2)
- **Migrating `gui_2.py`** to the convention (future track; 260KB file, the largest in the codebase)
- **Migrating `session_logger.py`, `warmup.py`, `theme_models.py`** to the convention (smaller files; future track)
- **Removing the `send()` deprecation** (deferred to user's planned `send_result` → `send` mass rename; post-RAG track per the `rag_test_failures_20260615` track's followup list)
- **Writing a Result-based migration tool** (the audit script is informational; not a refactor tool)
- **Updating the `doeh` and `public_api_migration` completion reports** to reference this audit (deferred; the audit report is a separate artifact)
- **Adding new tests for the audit script** (the audit is a static analyzer; its output is the verification; an `assertions on the output` test would be over-testing)
---
## 2. Goals (Priority Order)
| Priority | Goal | Rationale |
|---|---|---|
| **A (primary)** | Write `scripts/audit_exception_handling.py` as a static analyzer that classifies every `try/except/finally/raise` site per the convention. | The audit is the user's request. The script is the deliverable. |
| **A (primary)** | Verify the script's classifications are accurate (i.e., the FastAPI raises, the constructor raises, the broad-catches-in-`*_result`-functions, the stdlib-I/O catches, the SDK-boundary catches are all correctly classified). | A misclassifying audit is worse than no audit. |
| **A (primary)** | Update `conductor/code_styleguides/error_handling.md` with the 5 doc-clarification sections. | The audit's value is in the doc, not just the script. The user explicitly asked for codestyle/regular guide updates. |
| **B (secondary)** | Update `docs/guide_app_controller.md` with the FastAPI boundary section. | The app_controller is the largest unrefactored file; the new section explains what's legitimate. |
| **B (secondary)** | Generate a report summarizing the findings (per-file violation count, per-category breakdown, top migration-target files). | The user decides the next track from this report. |
| **C (documentation)** | Reference the new audit script from `conductor/product-guidelines.md` (the canonical reference for project standards). | The script is part of the convention enforcement; the product guidelines should mention it. |
### 2.1 Non-Goals (this track)
- **No production code changes.** This is a documentation + audit track. The Tier 2 implementer MUST NOT modify any `src/*.py` file.
- **No test file changes** (the audit has no tests; the script's output IS the verification).
- **No `mcp_architecture_refactor_20260606` work** (separate track, blocked by the convention being complete).
- **No `data_structure_strengthening_20260606` work** (separate track, parallel to this one).
---
## 3. The Audit Methodology
### 3.1 Classification Categories
The script classifies every exception-handling site into one of 10 categories:
| Category | Convention Status | Description | Hint Provided |
|---|---|---|---|
| `BOUNDARY_SDK` | Compliant | Wraps a third-party SDK call (anthropic, google, openai, chromadb, requests, etc.) or is in a `*_result` function with broad catch | "Compliant: third-party exception caught at SDK boundary" |
| `BOUNDARY_IO` | Compliant | Wraps stdlib I/O that can raise (OSError, JSONDecodeError, etc.) | "Compliant: stdlib I/O exception at third-party call site" |
| `BOUNDARY_CONVERSION` | Compliant | Catches and converts to `ErrorInfo` inside a `Result` | "Compliant: catch + ErrorInfo conversion is the canonical SDK boundary pattern" |
| `BOUNDARY_FASTAPI` | Compliant | FastAPI `HTTPException` raise in `_api_*` handler | "Compliant: framework-idiomatic boundary pattern" |
| `INTERNAL_SILENT_SWALLOW` | **Violation** | `except ...: pass` or just logs | "Violation: silent swallow hides failures" |
| `INTERNAL_BROAD_CATCH` | **Violation** | `except Exception` without conversion to ErrorInfo, in non-`*_result` code | "Violation: narrow the type or convert to ErrorInfo" |
| `INTERNAL_OPTIONAL_RETURN` | **Violation** | `try/except + return None/Optional[T]` | "Violation: replace with `Result[T]`" |
| `INTERNAL_RETHROW` | Suspicious | `try/except + raise` (without ErrorInfo conversion) | "Suspicious: consider Result-based propagation" |
| `INTERNAL_PROGRAMMER_RAISE` | Compliant | `raise` for impossible state / precondition (`__init__`, `assert`, `ValueError` for "this needs X") | "Compliant: `raise` for programmer errors" |
| `INTERNAL_COMPLIANT` | Compliant | `try/finally` (no except) — canonical cleanup pattern | "Compliant: `goto defer` pattern" |
| `UNCLEAR` | Review needed | Can't determine automatically | "Manual review: not obviously boundary or violation" |
### 3.2 The 3 Refactored Baseline Files (the Convention Target)
```
src/mcp_client.py — refactored 2026-06-12; 4 _result variants added
src/ai_client.py — refactored 2026-06-12; ProviderError removed, send_result() public
src/rag_engine.py — refactored 2026-06-12; _init_vector_store_result, _validate_collection_dim_result
```
The script reports a **baseline vs migration-target** split. The baseline is the convention reference; the migration target is where the user's next refactor tracks will focus.
### 3.3 Output Format
The script supports two output modes (matching `audit_weak_types.py`):
**Human-readable mode** (`--src src`):
```
=== Exception Handling Audit (Data-Oriented Convention) ===
Files scanned: 65
Files with findings: 42
Total sites: 348
try: 8
except: 283
raise: 57
Compliant sites: 80
Suspicious sites: 25
Violation sites: 211
Unclear (review): 32
--- Baseline (refactored files: mcp_client, ai_client, rag_engine) ---
Sites: 112, violations: 77
--- Migration target (all other src/ files) ---
Sites: 236, violations: 134
By category:
INTERNAL_BROAD_CATCH 147 (VIOLATION)
INTERNAL_SILENT_SWALLOW 61 (VIOLATION)
...
--- Top 15 files by violation count (migration target only) ---
src\gui_2.py (V=37, S=2, ?=13, C=2, total=54)
...
```
**JSON mode** (`--json`): machine-readable for tooling; includes per-site `category`, `kind`, `context`, `snippet`, and `hint`.
### 3.4 What the Script Does NOT Do
- Does NOT execute the code (it's a static analyzer; no behavior change).
- Does NOT modify any files.
- Does NOT provide specific refactor patches (the "hint" is a 1-line suggestion; the implementer of the next refactor track writes the actual code).
- Does NOT verify that refactored code works (no test execution; the audit report is the deliverable).
---
## 4. Doc Updates (5 sections + 1 cross-reference)
### 4.1 `conductor/code_styleguides/error_handling.md` — 5 new sections
**New section 1: "Boundary Types"** (insert after the current "5. Error Info as Side-Channel")
- Lists the 3 categories of "legitimate boundaries":
1. **Third-party SDK calls** (anthropic, google, openai, chromadb, requests, httpx, etc.) — per the spec §"Hard Rules"
2. **Stdlib I/O that can raise** (file/network I/O via `open()`, `requests.get()`, `chromadb.PersistentClient()`, etc.) — converting OSError to ErrorInfo
3. **Framework boundaries** (FastAPI `HTTPException` in `_api_*` handlers) — the framework-idiomatic way to signal HTTP errors
- Each category lists the specific exception types, the canonical pattern, and a code example.
**New section 2: "The Broad-Except Distinction"** (insert after "Boundary Types")
- Clarifies anti-pattern #6: "broad except Exception" is a code smell **only when the catch site doesn't convert to ErrorInfo**.
- When a `*_result` function does `except Exception as e: return Result(data=..., errors=[ErrorInfo(kind=INTERNAL, message=..., original=e)])`, it IS compliant (the catch + conversion is the canonical pattern).
- The distinction: where does the data go? If to `Result.errors`, compliant. If discarded (pass / print / log-only), violation.
**New section 3: "Constructors Can Raise"** (insert after "Broad-Except Distinction")
- Per the existing §"When to Use This Convention": "Constructors (`__init__`) that fail with programmer errors (use `assert` or `raise` for these)."
- The new section elaborates: `raise ValueError`, `raise TypeError`, `raise NotImplementedError` in `__init__` are compliant. `assert` for "this should never happen" invariants is compliant.
- The audit script's `INTERNAL_PROGRAMMER_RAISE` category implements this rule.
**New section 4: "Re-Raise Patterns"** (insert after "Constructors Can Raise")
- 3 legitimate re-raise patterns:
1. **Catch + convert + raise as different type** (e.g., `except OSError as e: raise ValueError(f"file not found: {e}")` for "convert library error to user error")
2. **Catch + log + re-raise** (e.g., `except Exception: log(); raise` for "I want a record before propagating")
3. **Catch + cleanup + re-raise** (e.g., `try: ... except: cleanup(); raise` for "ensure cleanup before propagating")
- 1 suspicious pattern: **catch + re-raise the same exception** (no value-add; remove the try/except or use a Result).
**New section 5: "Audit Script"** (insert after "Re-Raise Patterns")
- References `scripts/audit_exception_handling.py`.
- The script follows the "delete to turn off" pattern (per `feature_flags.md`): `rm scripts/audit_exception_handling.py` disables the audit.
- Usage: `uv run python scripts/audit_exception_handling.py` (human-readable) or `--json` (machine-readable).
- The script is a static analyzer; it does NOT modify code. Its output is a report.
- The script's classification categories (per §3.1) are the canonical taxonomy of "what kind of exception handling is this?".
### 4.2 `docs/guide_app_controller.md` — 1 new section
**New section: "Exception Handling in `app_controller.py`"**
- The file is 166KB and contains 56 exception-handling sites (per the audit).
- The 11 `HTTPException` raises in `_api_*` handlers (lines 96, 99, 213, 215, 312, 320, 341, 369, 380, 402) are **compliant** (FastAPI boundary pattern, per the new styleguide §"Boundary Types").
- The 2 `except Exception + raise HTTPException` sites (lines 309, 401) are **compliant** (FastAPI boundary pattern).
- The remaining ~43 sites (mostly `except Exception + log/print`, `except Exception + return None`) are **migration-target** — they would benefit from a future track that migrates the controller to the convention.
- Recommended future track: `app_controller_result_migration_20260616` (not in this track's scope; the user decides).
### 4.3 `conductor/product-guidelines.md` — 1 new cross-reference
Add a sentence to the "Data-Oriented Error Handling" section:
> "The convention is enforced via `scripts/audit_exception_handling.py` (static analyzer; file-presence = enabled per `feature_flags.md`)."
---
## 5. Architecture Reference
The convention's 3 refactored files are documented in:
- `docs/guide_mcp_client.md` §"Data-Oriented Error Handling (Fleury Pattern)"
- `docs/guide_ai_client.md` §"Data-Oriented Error Handling (Fleury Pattern)"
- `docs/guide_rag.md` §"Data-Oriented Error Handling (Fleury Pattern)"
The convention is documented in:
- `conductor/code_styleguides/error_handling.md` (the canonical styleguide)
- `conductor/code_styleguides/data_oriented_design.md` (the canonical DOD reference)
- `docs/guide_mma.md` (the MMA reference; uses Result for worker context)
- `docs/guide_mcp_client.md`, `docs/guide_ai_client.md`, `docs/guide_rag.md` (per-subsystem in-context guides)
The audit script follows the conventions of:
- `scripts/audit_weak_types.py` (the closest precedent; informational audit with --json, --top, --verbose modes)
- `scripts/audit_main_thread_imports.py` (the CI-gate precedent; though this audit is informational, not a gate)
- `conductor/code_styleguides/feature_flags.md` ("delete to turn off" pattern)
---
## 6. Risks & Mitigations
| ID | Risk | Likelihood | Impact | Mitigation |
|---|---|---|---|---|
| R1 | The audit script misclassifies sites, giving the user a wrong picture of the codebase. | Medium | High | The script's classification logic is verified against 3 known-good sites (the `_validate_collection_dim_result` catch, the `send_result` boundary, the FastAPI `HTTPException` raises). The test for accuracy is the user's manual review of the report; the script provides 1-line hints so misclassifications are easy to spot. |
| R2 | The doc updates introduce inconsistency with the existing styleguide. | Low | Medium | Each new section is reviewed against the existing 5 patterns; the wording matches the existing §"Anti-Patterns" and §"When to Use This Convention" sections. |
| R3 | The audit report's "violation count" is misread as "we have 211 bugs to fix". | Medium | Medium | The report is explicit: "These are migration-target sites, not bugs. The convention is partially applied; the user decides what to migrate." The `BOUNDARY_*` and `INTERNAL_COMPLIANT` categories are clearly labeled as compliant. |
| R4 | The `docs/guide_app_controller.md` update is too aggressive (suggests migrating too much). | Low | Low | The new section explicitly says "Recommended future track: `app_controller_result_migration_20260616` (not in this track's scope; the user decides)". |
| R5 | The script's performance is too slow on the full codebase. | Low | Low | The script uses AST (not regex) and is O(n) over the source files. Tested on 65 files in <2s. |
---
## 7. Verification Criteria
| ID | Criterion | Status |
|---|---|---|
| G1 | `scripts/audit_exception_handling.py` exists and runs without errors | (to be verified in Phase 1) |
| G2 | The script's classification of FastAPI `HTTPException` raises is `BOUNDARY_FASTAPI` (not `INTERNAL_RETHROW`) | (to be verified in Phase 2) |
| G3 | The script's classification of `__init__` raises is `INTERNAL_PROGRAMMER_RAISE` (not `INTERNAL_RETHROW`) | (to be verified in Phase 2) |
| G4 | The script's classification of broad-catches in `*_result` functions is `BOUNDARY_SDK` or `BOUNDARY_CONVERSION` (not `INTERNAL_BROAD_CATCH`) | (to be verified in Phase 2) |
| G5 | The report's baseline-vs-migration-target breakdown is accurate (the 3 refactored files are clearly labeled) | (to be verified in Phase 2) |
| G6 | `conductor/code_styleguides/error_handling.md` has 5 new sections (Boundary Types, Broad-Except Distinction, Constructors Can Raise, Re-Raise Patterns, Audit Script) | (to be verified in Phase 3) |
| G7 | `docs/guide_app_controller.md` has a new "Exception Handling" section explaining the FastAPI boundary | (to be verified in Phase 3) |
| G8 | `conductor/product-guidelines.md` has the new cross-reference to the audit script | (to be verified in Phase 3) |
| G9 | `docs/reports/EXCEPTION_HANDLING_AUDIT_20260616.md` exists with the per-file breakdown and per-category counts | (to be verified in Phase 4) |
| NF1 | No production code changes (no `src/*.py` files modified) | (to be verified at the end) |
| NF2 | All commits are atomic (spec, plan, metadata, docs, script, report — 6 commits minimum) | (to be verified at the end) |
| NF3 | Per-commit git notes summarize the changes | (to be verified at the end) |
---
## 8. Commits (this track, in order)
1. **`spec.md`** — the design document (this file)
2. **`plan.md`** — the TDD red-first task breakdown
3. **`metadata.json`** — track metadata
4. **`scripts/audit_exception_handling.py`** — the audit script + 1 commit for the audit report run
5. **`docs/guide_*` updates** — the 3 doc clarifications in 1-2 commits
6. **`conductor/code_styleguides/error_handling.md`** — the 5 new sections in 1 commit
7. **`docs/reports/EXCEPTION_HANDLING_AUDIT_20260616.md`** — the final report
8. **`conductor/tracks.md` update** — register the track
---
## 9. See Also
- `conductor/code_styleguides/error_handling.md` — the convention this audit enforces (this track adds 5 new sections)
- `conductor/code_styleguides/data_oriented_design.md` — the canonical DOD reference
- `conductor/code_styleguides/feature_flags.md` — the "delete to turn off" pattern (the audit script follows it)
- `conductor/tracks/data_oriented_error_handling_20260606/spec.md` — the parent track that established the convention
- `conductor/tracks/data_oriented_error_handling_20260606/spec.md` §12.2 — the prioritized list of future migration tracks (the audit's "migration target" report maps to this list)
- `scripts/audit_weak_types.py` — the closest precedent (informational audit with --json/--top/--verbose modes)
- `scripts/audit_main_thread_imports.py` — the CI-gate precedent (not a strict gate, but the strict-mode option is available)
- `docs/guide_app_controller.md` — the file that has the most migration-target sites (per the audit)
- `docs/reports/TRACK_COMPLETION_public_api_migration_and_ui_polish_20260615.md` §11 — the followup recommendations (item 2: "add an audit script for the if not numpy_array anti-pattern"; this track is a similar audit but for exception handling)