12 KiB
Result Migration Sub-Track 1: Review Pass Report
Track: result_migration_review_pass_20260617
Umbrella: result_migration_20260616
Type: audit + documentation (informational; no production code change)
Status: active
Date: 2026-06-17
0. Executive Summary
This report captures the per-site decisions for the 43 ambiguous exception-handling sites identified by scripts/audit_exception_handling.py --json on 2026-06-17:
- 24 UNCLEAR sites (the script cannot classify from AST alone)
- 19 INTERNAL_RETHROW sites (
try/except + raise; needs the 3 legitimate pattern checks)
Each site was reviewed by reading the snippet + 2-3 lines of context. The decisions flow into the umbrella's sub-tracks 2-4 as their starting migration scope.
1. Pre-Review Audit Snapshot (2026-06-17, base commit b6caca40)
| Bucket | Count | Description |
|---|---|---|
UNCLEAR |
24 | Script could not classify; needs human review |
INTERNAL_RETHROW |
19 | try/except + raise; needs 3-pattern check |
| Total review scope | 43 | 11 files affected |
Other audit findings (unchanged by this review pass):
- 211 violations (broad catch, silent swallow, Optional[T] return) — out of scope here
- 80 compliant sites — out of scope here
- 25 INTERNAL_PROGRAMMER_RAISE (raise in init / assert) — compliant; out of scope
2. Per-Site Decision Table
2.1 src/gui_2.py — UNCLEAR sites (13)
| Line | Context | Snippet | Decision | Pattern / Rationale |
|---|---|---|---|---|
| 65 | _resolve (deferred importer) |
except AttributeError: ... _FiledialogStub() |
compliant | Graceful degradation for missing optional modules (filedialog stub) |
| 69 | _resolve (deferred importer) |
except (ImportError, ModuleNotFoundError): _FiledialogStub() |
compliant | Graceful degradation for missing optional modules (filedialog stub) |
| 684 | run (ImGui main loop) |
except RuntimeError as _immapp_exc: ... log + keep alive |
compliant | Defer-not-catch for native bundle crashes (per workflow.md); logs to _gui_degraded_reason |
| 806 | _get_active_capabilities |
except KeyError: caps = VendorCapabilities(... notes="unregistered") |
compliant | Lookup-miss-with-default for get_capabilities(provider, model) |
| 1349 | _populate_auto_slices |
except Exception: return |
migration-target | Broad except Exception + silent return. Should narrow to (OSError, UnicodeDecodeError) or return Result. Sub-track 4 (gui_2) |
| 2401 | render_rag_panel (vector store provider combo) |
except (ValueError, AttributeError): idx = 0 |
compliant | list.index miss with default; standard Python combo-box idiom |
| 2411 | render_rag_panel (embedding provider combo) |
except (ValueError, AttributeError): idx_e = 0 |
compliant | list.index miss with default; standard Python combo-box idiom |
| 2533 | render_agent_tools_panel (tool preset combo) |
except ValueError: idx = 0 |
compliant | list.index miss with default; standard Python combo-box idiom |
| 2561 | render_agent_tools_panel (filter category combo) |
except ValueError: f_idx = 0 |
compliant | list.index miss with default; standard Python combo-box idiom |
| 2759 | render_persona_selector_panel (load persona context preset) |
except KeyError as e: app.ai_status = f"persona context preset missing: {e}" |
compliant | Lookup-miss-with-user-feedback; defensive but user-visible |
| 4106 | render_context_files_table (view mode combo) |
except ValueError: current_idx = 1; f_item.view_mode = "summary" |
compliant | list.index miss with default + state correction |
| 4159 | render_context_presets (context preset combo) |
except ValueError: idx = 0 |
compliant | list.index miss with default; standard Python combo-box idiom |
| 6830 | render_tier_stream_panel (ImGui child end guard) |
except (TypeError, AttributeError): imgui.end_child() |
compliant | ImGui scope cleanup guard; ensures end_child() is always called |
Subtotals: 12 compliant + 1 migration-target.
New heuristics identified for the audit script (added in Task 4.1):
list.indexwithValueErrorfallback to a default index →INTERNAL_COMPLIANTdict.get/KeyErrorlookup with default value construction →INTERNAL_COMPLIANT- Narrow
except (RuntimeError, OSError, AttributeError, ImportError)+imgui.end_*or stub construction →INTERNAL_COMPLIANT(defer-not-catch for ImGui) - Narrow
except (ImportError, ModuleNotFoundError, AttributeError)+ fallback attribute/stub →INTERNAL_COMPLIANT(graceful degradation)
2.2 src/mcp_client.py — UNCLEAR sites (4, baseline)
| Line | Context | Snippet | Decision | Pattern / Rationale |
|---|---|---|---|---|
| 126 | configure (allowlist setup) |
except (OSError, ValueError): rp = Path(p).resolve() (non-strict fallback) |
compliant | Graceful path resolution: Path.resolve(strict=True) may fail if file missing; fallback to non-strict is a safe degradation |
| 152 | _is_allowed (allowlist check) |
except (OSError, ValueError): rp = path.resolve() (non-strict fallback) |
compliant | Graceful path resolution (same as L126) |
| 177 | _is_allowed (cwd subpath check) |
except ValueError: pass after rp.relative_to(cwd) |
compliant | Path.relative_to raises ValueError when path is not relative to base; this is the canonical "not-a-subpath" check, not an error |
| 987 | py_check_syntax (tool function) |
except SyntaxError: ... then except Exception: return f"ERROR..." |
compliant | Tool-boundary pattern: function returns a string (Result-like); both narrow and broad excepts convert exceptions to user-readable strings. No silent swallow |
Subtotals: 4 compliant + 0 migration-target.
New heuristic candidates:
5. Path.resolve(strict=True) with (OSError, ValueError) fallback to non-strict → INTERNAL_COMPLIANT (graceful path resolution)
6. Path.relative_to with ValueError (not-a-subpath) → INTERNAL_COMPLIANT (canonical subpath check)
7. MCP tool function with except Exception: return f"ERROR..." (string return) → BOUNDARY_TOOL (tool boundary; converts to string Result)
2.3 src/ai_client.py — UNCLEAR sites (2, baseline)
| Line | Context | Snippet | Decision | Pattern / Rationale |
|---|---|---|---|---|
| 828 | run_with_tool_loop (sync/async bridge) |
except RuntimeError: results = asyncio.run(...) after asyncio.get_running_loop() |
compliant | Sync/async bridge: get_running_loop() raises RuntimeError when no loop is running; the fallback to asyncio.run is the canonical pattern |
| 2813 | _get_llama_cost_tracking (vendor capabilities lookup) |
except KeyError: return True after get_capabilities("llama", _model) |
compliant | Lookup-miss-with-default (same as gui_2 L806); default to cost-tracking-on for unknown models |
Subtotals: 2 compliant + 0 migration-target.
New heuristic candidates:
8. asyncio.get_running_loop() with except RuntimeError: asyncio.run(...) → INTERNAL_COMPLIANT (sync/async bridge)
2.4 src/app_controller.py — UNCLEAR sites (2)
| Line | Context | Snippet | Decision | Pattern / Rationale |
|---|---|---|---|---|
| 1842 | init_state (controller initialization) |
except KeyError: caps = None after get_capabilities(...) |
compliant | Lookup-miss-with-None default; same pattern as L806/L2813; downstream check if caps is None or caps.model_discovery |
| 3740 | _on_ai_stream (streaming handler) |
except KeyError: caps = None after get_capabilities(...) |
compliant | Lookup-miss-with-None default; downstream check if caps is None or caps.streaming |
Subtotals: 2 compliant + 0 migration-target.
2.5 src/models.py — UNCLEAR sites (2)
| Line | Context | Snippet | Decision | Pattern / Rationale |
|---|---|---|---|---|
| 452 | from_dict (track-state deserialization) |
except ValueError: created = None after datetime.fromisoformat(created) |
compliant | Lenient deserialization: malformed ISO date in TOML config → None (don't crash the entire load). Canonical pattern for user-edited config |
| 457 | from_dict (track-state deserialization) |
except ValueError: updated = None after datetime.fromisoformat(updated) |
compliant | Lenient deserialization (same as L452) |
Subtotals: 2 compliant + 0 migration-target.
New heuristic candidates:
9. datetime.fromisoformat(s) with except ValueError: <var> = None → INTERNAL_COMPLIANT (lenient TOML deserialization)
2.6 src/multi_agent_conductor.py — UNCLEAR sites (1)
| Line | Context | Snippet | Decision | Pattern / Rationale |
|---|---|---|---|---|
| 236 | parse_json_tickets (CLI-style JSON input) |
except json.JSONDecodeError as e: print(...); except KeyError as e: print(...) |
compliant | CLI-style input parser: print provides user-visible error feedback; the function is -> None so there is no Result to add. The narrow excepts are appropriate for the two distinct failure modes (malformed JSON vs missing required field) |
Subtotals: 1 compliant + 0 migration-target.
New heuristic candidates:
10. try/except (json.JSONDecodeError, KeyError) around JSON parse with print(...) and return (no Result) → INTERNAL_COMPLIANT (CLI-style JSON input parser)
2.7 src/ai_client.py — INTERNAL_RETHROW sites (6, baseline)
| Line | Context | Snippet | Decision | Pattern / Rationale |
|---|---|---|---|---|
| 277 | _load_credentials (file load) |
except FileNotFoundError: raise FileNotFoundError(...) with helpful setup message |
PATTERN_1 | Catch + convert + raise as same type with better message. Provides actionable instructions in the error message. Baseline transition pattern. |
| 801 | _default_send (Result→Exception bridge) |
if not res.ok: ... raise res.errors[0].original |
PATTERN_1 | Result→Exception bridge: re-raise original SDK exception. Legacy callers expect exceptions; the Result layer above provides the structured error info |
| 802 | _default_send (Result→Exception bridge) |
raise RuntimeError(res.errors[0].message if res.errors else "Unknown OpenAI error") |
PATTERN_1 | Result→Exception bridge: convert Result error to RuntimeError. Same as L801 |
| 1234 | _list_anthropic_models (Anthropic SDK) |
except Exception as exc: raise _classify_anthropic_error(exc) from exc |
PATTERN_1 | Catch + convert + raise as different type: convert raw SDK exception to structured ErrorInfo. from exc preserves the traceback |
| 1529 | _list_gemini_models (Gemini SDK) |
except Exception as exc: raise _classify_gemini_error(exc) from exc |
PATTERN_1 | Same as L1234, Gemini SDK |
| 2520 | _dashscope_call (Qwen/DashScope SDK) |
if status_code != 200: raise classify_dashscope_error(...) |
PATTERN_1 | Result→Exception bridge: explicit raise on API non-200 status. Caller (Result-based) catches and converts. No try/except in this function; the raise is the explicit "this is a domain error" path |
Subtotals: 6 PATTERN_1 + 0 PATTERN_2/3 + 0 migration-target.
Note: All 6 baseline ai_client INTERNAL_RETHROW sites are the "Result→Exception bridge" pattern. This is the canonical pattern for the baseline transition: Result-based provider functions still raise on hard failures for legacy callers, but the convention layer above catches and converts to a Result. The 2026-06-12 refactor intentionally preserved this pattern for the boundary.
2.8 src/rag_engine.py — INTERNAL_RETHROW sites (4, baseline)
(filled in Task 3.2)
2.9 src/app_controller.py — INTERNAL_RETHROW sites (3)
(filled in Task 3.3)
2.10 src/gui_2.py — INTERNAL_RETHROW sites (2)
(filled in Task 3.4)
2.11 src/api_hooks.py — INTERNAL_RETHROW sites (2)
(filled in Task 3.5)
2.12 src/models.py — INTERNAL_RETHROW site (1)
(filled in Task 3.6)
2.13 src/warmup.py — INTERNAL_RETHROW site (1)
(filled in Task 3.7)
3. Post-Review Migration Scope
(filled in Task 5.1)
4. Audit Script Heuristic Updates
(filled in Task 4.1)
5. Verification
(filled in Phase 6)