From 42095232282d873d8930e0b308c0e7dc00582f16 Mon Sep 17 00:00:00 2001 From: Ed_ Date: Tue, 16 Jun 2026 09:07:24 -0400 Subject: [PATCH] docs(app_controller+guidelines): add Exception Handling section + audit script cross-reference --- conductor/product-guidelines.md | 10 ++++ docs/guide_app_controller.md | 84 +++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/conductor/product-guidelines.md b/conductor/product-guidelines.md index 6a055b6b..848c0305 100644 --- a/conductor/product-guidelines.md +++ b/conductor/product-guidelines.md @@ -71,6 +71,16 @@ tracks will apply it to the remaining `src/` files see `conductor/tracks/data_oriented_error_handling_20260606/spec.md` ยง12.2 for the prioritized list). +**Audit:** the convention is enforced via +[`scripts/audit_exception_handling.py`](../../scripts/audit_exception_handling.py) +(static analyzer; file-presence = enabled per +[`feature_flags.md`](code_styleguides/feature_flags.md)). Run +`uv run python scripts/audit_exception_handling.py` for a human-readable +report or `--json` for machine-readable output. The audit classifies each +`try/except/finally/raise` site against 10 categories (5 compliant + 3 +violation + 1 suspicious + 1 unclear); see the styleguide's "Audit Script" +section for the full taxonomy. + ### `Optional[T]` ban (return types only) In the 3 refactored files (`src/mcp_client.py`, `src/ai_client.py`, diff --git a/docs/guide_app_controller.md b/docs/guide_app_controller.md index d5e8fcf3..1bb5ac96 100644 --- a/docs/guide_app_controller.md +++ b/docs/guide_app_controller.md @@ -391,6 +391,90 @@ def test_apply_persona(live_gui): --- +## Exception Handling in `app_controller.py` + +Per the data-oriented error handling convention +(`conductor/code_styleguides/error_handling.md`), `app_controller.py` is a +**migration-target file**: it has not been fully refactored to the +`Result[T]` pattern. The convention is applied to 3 of 65 source files +(`src/mcp_client.py`, `src/ai_client.py`, `src/rag_engine.py`); the rest of +`src/` is in the migration-target state. + +The exception-handling audit (`scripts/audit_exception_handling.py`) +classifies 56 exception-handling sites in this file (per the 2026-06-16 +audit run): + +| Category | Count | Convention status | +|---|---|---| +| `BOUNDARY_FASTAPI` | 13 | Compliant | +| `INTERNAL_BROAD_CATCH` | 28 | **Violation** (migration target) | +| `INTERNAL_SILENT_SWALLOW` | 6 | **Violation** (migration target) | +| `INTERNAL_RETHROW` | 3 | Suspicious | +| `UNCLEAR` | 2 | Manual review needed | +| `INTERNAL_OPTIONAL_RETURN` | 1 | **Violation** (migration target) | + +### The 13 FastAPI boundary sites (compliant) + +`app_controller.py` is the FastAPI handler layer (`--enable-test-hooks` +mode). The 11 `HTTPException` raises in `_api_*` handlers (lines 96, 99, +213, 215, 312, 320, 341, 369, 380, 402) and the 2 `except Exception + +raise HTTPException` sites (lines 309, 401) are the **framework boundary** +โ€” `HTTPException` is the FastAPI-idiomatic way to signal HTTP errors; +FastAPI converts it to a JSON response at the framework level. These +sites are compliant per the convention's "Framework boundaries (FastAPI)" +section. + +Example (line 96): +```python +if not target_key: + raise HTTPException(status_code=403, detail="API Key not configured on server") +``` + +Example (line 309): +```python +try: + result = ai_client.send_result(...) + return result.data +except Exception as e: + raise HTTPException(status_code=500, detail=f"AI call failed: {e}") +``` + +### The 40 migration-target sites (violations) + +The remaining ~40 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 scope of the audit track; the user decides). The work would be: +1. Convert `Optional[T]` return types to `Result[T]`. +2. Convert `except Exception + log/print` to `except Exception + + return Result(data=NIL_T, errors=[ErrorInfo(...)])`. +3. Convert `except Exception + return None` to `except Exception + + return Result(data=NIL_T, errors=[ErrorInfo(...)])`. +4. Keep the 13 FastAPI boundary sites as-is (they're the framework contract). +5. Add tests that verify the success and failure paths of each migrated + function return the right `Result`. + +This is a 2-3 day Tier 2 effort. The audit's per-site hints (in the JSON +output via `--json`) tell the implementer what the fix should look like +for each site. + +### Quick check + +To see the current state of the file's exception handling: +```bash +uv run python scripts/audit_exception_handling.py --top 1 --verbose | head -50 +``` + +To see only the violations (not the compliant FastAPI boundary sites): +```bash +uv run python scripts/audit_exception_handling.py --json | \ + python -c "import json,sys; r=json.load(sys.stdin); print([f for f in r['files'][0]['findings'] if f['category'] in ('INTERNAL_BROAD_CATCH', 'INTERNAL_SILENT_SWALLOW', 'INTERNAL_OPTIONAL_RETURN')])" +``` + +--- + ## See Also - **[guide_architecture.md](guide_architecture.md)** โ€” Threading and event flow