diff --git a/conductor/code_styleguides/error_handling.md b/conductor/code_styleguides/error_handling.md index 5d08c322..a35d59a5 100644 --- a/conductor/code_styleguides/error_handling.md +++ b/conductor/code_styleguides/error_handling.md @@ -639,7 +639,149 @@ assert the warning is NOT emitted by `send_result()`. --- -## See Also +## AI Agent Checklist (Added 2026-06-16) + +This section is for AI agents writing code in this codebase. LLMs are +trained on idiomatic Python (`try/except`, `Optional[T]`, `raise +Exception`, etc.) which is the OPPOSITE of this convention. The +checklist below catches the most common LLM mistakes. **Run this +checklist before claiming a task is done.** + +### The 5 MUST-DO rules + +When writing NEW code, you MUST: + +1. **Use `Result[T]` for any function that can fail at runtime.** A + function that returns a different value under different runtime + conditions (success vs. failure) returns `Result[T]`, not + `Optional[T]`, not `T | None`, not a custom exception class. Use the + `Result` dataclass from `src/result_types.py`; populate + `errors: list[ErrorInfo]` on failure. + +2. **Catch SDK exceptions at the boundary, convert to `ErrorInfo`.** If + your code calls `anthropic`, `google.genai`, `openai`, `chromadb`, + `requests`, or any other third-party SDK, the catch site + converts the exception to `ErrorInfo(kind=..., message=...)` and + returns it in `Result.errors`. Do NOT re-raise; do NOT swallow; + do NOT let the exception propagate into internal code. + +3. **Use nil-sentinel dataclasses for "no result".** If a function + would return `None` in idiomatic Python, return a frozen + `NilPath` / `NilRAGState` / etc. singleton from + `src/result_types.py` instead. Callers don't need `if x is None:` + checks; they can call `x.read_text` and get `""` on the nil path. + +4. **Use `try/finally` (no except) for cleanup.** Bare + `try: ...; finally: cleanup()` is the canonical `goto defer` + pattern. Use it for resource cleanup, lock release, file handle + close. Do NOT use `try/except` + pass for cleanup; the cleanup + should run whether or not an exception occurred. + +5. **`raise` is reserved for programmer errors.** `assert` for + "this should never happen" invariants. `raise ValueError`, + `raise NotImplementedError`, `raise KeyError` in `__init__` for + "this object needs X." Do NOT use `raise` for runtime failures + (the network is down, the file doesn't exist, the API rate-limited); + those are `Result` cases. + +### The 7 MUST-NOT-DO rules + +When writing NEW code, you MUST NOT: + +1. **DO NOT use `Optional[T]` as a return type** (in any file in + `src/mcp_client.py`, `src/ai_client.py`, `src/rag_engine.py` โ€” + the 3 refactored files). Use `Result[T]` instead. CI fails if + you add a new `Optional[T]` to those files (enforced by + `scripts/audit_optional_in_3_files.py`). + +2. **DO NOT use `Optional[T]` as a return type** (anywhere else in + `src/`). The convention is migrating to `Result[T]`; new code + should set the pattern, not perpetuate the old one. Argument + types that may be `None` (caller choice) are still OK. + +3. **DO NOT use `None` as a sentinel for "no result".** Use a + nil-sentinel dataclass. The data is zero-initialized; the caller + doesn't need a None check. + +4. **DO NOT raise a custom exception class for runtime failures.** + SDK exceptions caught and converted to `ErrorInfo` is the only + legitimate exception path. Internal code uses `Result`. + +5. **DO NOT use `Union[T, E]` (sum type).** Use `Result[T]` with + side-channel `errors: list[ErrorInfo]`. The result is the data + AND the errors, not a tagged sum. + +6. **DO NOT catch `except Exception` and silently swallow.** Either + narrow the exception type, convert to `ErrorInfo` in a `Result`, + or document the intentional swallow with a comment-free `assert` + for the precondition. The audit script flags this as + `INTERNAL_SILENT_SWALLOW`. + +7. **DO NOT catch `except Exception` in non-`*_result` code without + conversion to `ErrorInfo`.** If you must catch, convert: + `except SomeError as e: return Result(data=NIL_T, errors=[ErrorInfo(kind=INTERNAL, message=..., original=e)])`. + The audit script flags this as `INTERNAL_BROAD_CATCH`. + +### The 3 boundary patterns (where `try/except` IS the right answer) + +These are the 3 categories where `try/except` is legitimate. See the +"Boundary Types" section above for the full discussion. + +1. **Third-party SDK calls.** Wrapping `anthropic.Anthropic().messages.create(...)` + in `try/except anthropic.APIError` is the canonical pattern. + Convert to `ErrorInfo`; return in `Result`. + +2. **Stdlib I/O that can raise.** `open()`, `os.path.*`, + `json.loads()`, `subprocess.run()`, `socket.*`, `sqlite3.*`, + `chromadb.PersistentClient()` can all raise. Catch the specific + exception (`OSError`, `FileNotFoundError`, `json.JSONDecodeError`, + `subprocess.CalledProcessError`, etc.); convert to `ErrorInfo`. + +3. **FastAPI `HTTPException` in `_api_*` handlers.** `raise + HTTPException(status_code=..., detail=...)` in a function named + `_api_*` is the FastAPI-idiomatic way to signal HTTP errors. + FastAPI converts it to a JSON response at the framework level. + This is NOT an exception leak; it's the framework contract. + +### The pre-commit gate + +Before claiming "done," you MUST run: + +```bash +uv run python scripts/audit_exception_handling.py +``` + +If the script reports any `INTERNAL_*` (other than `INTERNAL_COMPLIANT` +and `INTERNAL_PROGRAMMER_RAISE`) or `BOUNDARY_*` (other than +`BOUNDARY_FASTAPI` in `_api_*` handlers), your code violates the +convention. Fix it before committing. For CI use: + +```bash +uv run python scripts/audit_exception_handling.py --strict +``` + +`--strict` exits 1 on any violation; use this in pre-commit hooks and +CI to enforce the convention. The 4 enforcement audit scripts are: + +- `scripts/audit_exception_handling.py --strict` (this one) +- `scripts/audit_weak_types.py --strict` (the type-strengthening audit) +- `scripts/audit_main_thread_imports.py` (always strict; the import graph gate) +- `scripts/audit_no_models_config_io.py` (the config-I/O ownership gate) + +All 4 are part of the convention enforcement. See +`conductor/product-guidelines.md` "Data-Oriented Error Handling" and +`docs/AGENTS.md` ยง"Convention Enforcement" for the project-level rules. + +### Why this checklist exists + +LLMs are trained on idiomatic Python. Without this checklist, an +AI agent writing new code in this codebase will revert to idiomatic +patterns (`try/except`, `Optional[T]`, `raise Exception`) โ€” the +"tech rot with idiomatic Python" the user is preventing. The +checklist is the last line of defense. The audit scripts are the +automated check; the checklist is the manual one. + +--- - `conductor/tracks/data_oriented_error_handling_20260606/spec.md` โ€” the spec that established this convention.