Private
Public Access
docs: scrub gemini_cli references from 12 docs (provider count 8->7)
Cleaned: docs/guide_ai_client.md, docs/guide_architecture.md, docs/guide_models.md, docs/guide_simulations.md, docs/guide_context_aggregation.md, docs/guide_tools.md, docs/Readme.md, conductor/tech-stack.md, conductor/product.md, conductor/product-guidelines.md, conductor/workflow.md, conductor/code_styleguides/error_handling.md. Provider list citations updated to 7 (gemini, anthropic, deepseek, minimax, qwen, grok, llama). guide_meta_boundary.md intentionally retained (its gemini_cli references are the meta-tooling GEMINI_CLI_HOOK_CONTEXT env var, NOT the provider; per spec GAP-A12).
This commit is contained in:
@@ -167,7 +167,7 @@ Need to represent "missing or failed"?
|
||||
| +-- Use raise (only for programmer errors like KeyError on a known dict)
|
||||
|
|
||||
+-- Does the SDK raise an exception you can't avoid?
|
||||
+-- Catch at the boundary; convert to ErrorInfo inside a Result
|
||||
+-- Catch at the boundary; convert to ErrorInfo inside a Result
|
||||
```
|
||||
|
||||
---
|
||||
@@ -177,17 +177,17 @@ Need to represent "missing or failed"?
|
||||
**DON'T do these things:**
|
||||
|
||||
1. **DON'T** use `Optional[X]` for "this might fail at runtime". Use
|
||||
`Result[X]` instead.
|
||||
`Result[X]` instead.
|
||||
2. **DON'T** use `None` as a sentinel for "no result". Use a nil-sentinel
|
||||
dataclass.
|
||||
dataclass.
|
||||
3. **DON'T** raise a custom exception class for runtime failures. Catch SDK
|
||||
exceptions and return `ErrorInfo`.
|
||||
exceptions and return `ErrorInfo`.
|
||||
4. **DON'T** use `Union[T, E]` (sum type). Use a struct with parallel fields
|
||||
(AND over OR).
|
||||
(AND over OR).
|
||||
5. **DON'T** have `if x is None: handle; else: use_x` patterns in production
|
||||
code. The nil-sentinel makes them unnecessary.
|
||||
code. The nil-sentinel makes them unnecessary.
|
||||
6. **DON'T** catch `except Exception` and silently swallow. Convert to
|
||||
`ErrorInfo` and return in the `Result`.
|
||||
`ErrorInfo` and return in the `Result`.
|
||||
|
||||
---
|
||||
|
||||
@@ -196,16 +196,16 @@ Need to represent "missing or failed"?
|
||||
The 3 refactored subsystems demonstrate each pattern in context:
|
||||
|
||||
- **`src/mcp_client.py:205-294`** — `read_file`, `list_directory`,
|
||||
`search_files` return `Result[str]`; `(p, err)` tuples become
|
||||
`Result[Path]`; the 30+ `assert p is not None` chain (lines 304-794) is
|
||||
removed.
|
||||
`search_files` return `Result[str]`; `(p, err)` tuples become
|
||||
`Result[Path]`; the 30+ `assert p is not None` chain (lines 304-794) is
|
||||
removed.
|
||||
- **`src/ai_client.py`** — `_send_<vendor>_result()` returns `Result[str]`
|
||||
(8 vendors: gemini, anthropic, deepseek, minimax, gemini_cli, qwen, llama,
|
||||
grok); `send(...) -> Result[str, ErrorInfo]` is the public API.
|
||||
(8 vendors: gemini, anthropic, deepseek, minimax, qwen, llama,
|
||||
grok); `send(...) -> Result[str, ErrorInfo]` is the public API.
|
||||
- **`src/rag_engine.py:100-180`** — `_init_vector_store_result`,
|
||||
`_validate_collection_dim_result`, `is_empty_result`, `add_documents_result`
|
||||
return `Result[None]` or `Result[T]`; broad `except Exception` blocks
|
||||
become `ErrorInfo` entries.
|
||||
`_validate_collection_dim_result`, `is_empty_result`, `add_documents_result`
|
||||
return `Result[None]` or `Result[T]`; broad `except Exception` blocks
|
||||
become `ErrorInfo` entries.
|
||||
|
||||
---
|
||||
|
||||
@@ -219,23 +219,23 @@ from `_in_3_files.py` per the contradictions report) covers all
|
||||
remaining work to bring the 14 migration-target files into compliance.
|
||||
|
||||
- **`Optional[T]` return types are FORBIDDEN** in all `src/*.py`. Use
|
||||
`Result[T]` (with `NIL_T` singleton if needed) instead. Rationale:
|
||||
`Optional[T]` is the sum type `Union[T, None]` that Fleury's framework
|
||||
replaces. Mixing the two patterns reintroduces the bifurcation the
|
||||
convention is designed to remove.
|
||||
- Argument types that may be `None` (e.g., `rag_engine: Optional[Any] = None`)
|
||||
remain allowed; they describe a caller choice, not a runtime failure
|
||||
of this function. Only `Optional[T]` *return* types are banned.
|
||||
`Result[T]` (with `NIL_T` singleton if needed) instead. Rationale:
|
||||
`Optional[T]` is the sum type `Union[T, None]` that Fleury's framework
|
||||
replaces. Mixing the two patterns reintroduces the bifurcation the
|
||||
convention is designed to remove.
|
||||
- Argument types that may be `None` (e.g., `rag_engine: Optional[Any] = None`)
|
||||
remain allowed; they describe a caller choice, not a runtime failure
|
||||
of this function. Only `Optional[T]` *return* types are banned.
|
||||
- **Function return types must be `Result[T]` for any function that can fail
|
||||
at runtime.** A function that can't fail (e.g., `get_name() -> str`)
|
||||
doesn't need a `Result`. The classification is "can this return a different
|
||||
value under different runtime conditions?" If yes, `Result`. If no, plain
|
||||
return type.
|
||||
at runtime.** A function that can't fail (e.g., `get_name() -> str`)
|
||||
doesn't need a `Result`. The classification is "can this return a different
|
||||
value under different runtime conditions?" If yes, `Result`. If no, plain
|
||||
return type.
|
||||
- **Catch SDK exceptions at the boundary only.** Inside the 3 refactored
|
||||
files, the only place an exception is caught is at the SDK call site
|
||||
(e.g., `_send_<vendor>_result()` wrapping the SDK call). Internal
|
||||
`try/except` is reserved for converting `OSError`, `PermissionError`, and
|
||||
similar I/O exceptions to `ErrorInfo` at the mcp_client tool boundary.
|
||||
files, the only place an exception is caught is at the SDK call site
|
||||
(e.g., `_send_<vendor>_result()` wrapping the SDK call). Internal
|
||||
`try/except` is reserved for converting `OSError`, `PermissionError`, and
|
||||
similar I/O exceptions to `ErrorInfo` at the mcp_client tool boundary.
|
||||
|
||||
The verification script `scripts/audit_optional_returns.py` enforces the
|
||||
`Optional[X]` rule by failing CI if any new `Optional[X]` return type
|
||||
@@ -266,18 +266,18 @@ warnings use `warnings.warn(..., stacklevel=2)` which is thread-safe.
|
||||
**Use it for:**
|
||||
|
||||
- New public APIs (any function that can fail at runtime and the caller
|
||||
might care).
|
||||
might care).
|
||||
- New internal functions where the caller benefits from knowing the failure
|
||||
(vs. just propagating `None`).
|
||||
(vs. just propagating `None`).
|
||||
|
||||
**Don't use it for:**
|
||||
|
||||
- Constructors (`__init__`) that fail with programmer errors (use `assert` or
|
||||
`raise` for these). See "Constructors Can Raise" below for the full rule.
|
||||
`raise` for these). See "Constructors Can Raise" below for the full rule.
|
||||
- Trivial getters that can't fail (`get_name() -> str` doesn't need a
|
||||
`Result`).
|
||||
`Result`).
|
||||
- Performance-critical hot paths where the overhead of the dataclass
|
||||
allocation is measurable (rare; benchmark first).
|
||||
allocation is measurable (rare; benchmark first).
|
||||
|
||||
---
|
||||
|
||||
@@ -333,16 +333,16 @@ into internal code; it's the framework contract.
|
||||
# Compliant: FastAPI boundary in _api_* handler
|
||||
async def _api_get_key(controller, header_key: str) -> str:
|
||||
if not _is_valid_key(header_key):
|
||||
raise HTTPException(status_code=403, detail="Could not validate API Key")
|
||||
raise HTTPException(status_code=403, detail="Could not validate API Key")
|
||||
return header_key
|
||||
|
||||
# Compliant: broad catch + HTTPException at the FastAPI boundary
|
||||
async def _api_generate(controller, payload):
|
||||
try:
|
||||
result = ai_client.send(...)
|
||||
return result.data
|
||||
result = ai_client.send(...)
|
||||
return result.data
|
||||
except Exception as e:
|
||||
raise HTTPException(status_code=500, detail=f"AI call failed: {e}")
|
||||
raise HTTPException(status_code=500, detail=f"AI call failed: {e}")
|
||||
```
|
||||
|
||||
The catch-all `except Exception` is acceptable here **because the
|
||||
@@ -353,13 +353,13 @@ HTTP status code is the framework contract.
|
||||
### What is NOT a boundary
|
||||
|
||||
- Internal business logic: `try/except` around a `for` loop in a
|
||||
controller method is internal, not boundary.
|
||||
controller method is internal, not boundary.
|
||||
- Cross-method calls within `src/`: calling a method in
|
||||
`app_controller.py` from a method in `app_controller.py` is internal,
|
||||
not boundary.
|
||||
`app_controller.py` from a method in `app_controller.py` is internal,
|
||||
not boundary.
|
||||
- stdlib I/O that the user controls directly: opening a file the user
|
||||
passed via `--config` is internal; converting the failure should be
|
||||
Result-based, not exception-based.
|
||||
passed via `--config` is internal; converting the failure should be
|
||||
Result-based, not exception-based.
|
||||
|
||||
---
|
||||
|
||||
@@ -391,10 +391,10 @@ where the caller of the drain point does NOT need to receive a
|
||||
```python
|
||||
# COMPLIANT: drain point. The HTTP status code IS the error response.
|
||||
async def _api_get_track(controller, track_id: str) -> dict:
|
||||
result = controller.get_track_result(track_id)
|
||||
if not result.ok:
|
||||
raise HTTPException(status_code=404, detail=result.errors[0].ui_message())
|
||||
return {"track": result.data}
|
||||
result = controller.get_track_result(track_id)
|
||||
if not result.ok:
|
||||
raise HTTPException(status_code=404, detail=result.errors[0].ui_message())
|
||||
return {"track": result.data}
|
||||
```
|
||||
|
||||
The caller (the HTTP client) receives an HTTP 4xx/5xx response. The
|
||||
@@ -407,10 +407,10 @@ the error.
|
||||
```python
|
||||
# COMPLIANT: drain point. The user sees the error in the modal.
|
||||
def _show_track_load_failure(controller, track_id: str) -> None:
|
||||
result = controller.get_track_result(track_id)
|
||||
if not result.ok:
|
||||
imgui.open_popup("Track Load Error")
|
||||
# popup body reads result.errors[0].ui_message() and displays it
|
||||
result = controller.get_track_result(track_id)
|
||||
if not result.ok:
|
||||
imgui.open_popup("Track Load Error")
|
||||
# popup body reads result.errors[0].ui_message() and displays it
|
||||
```
|
||||
|
||||
The user sees the error. The caller (`_show_track_load_failure`)
|
||||
@@ -421,10 +421,10 @@ returns `None` — it is the end of the propagation chain.
|
||||
```python
|
||||
# COMPLIANT: drain point. The app shuts down intentionally.
|
||||
def _shutdown_on_critical_failure(controller) -> None:
|
||||
result = controller._init_session_db_result()
|
||||
if not result.ok:
|
||||
sys.stderr.write(f"FATAL: {result.errors[0].ui_message()}\n")
|
||||
sys.exit(1)
|
||||
result = controller._init_session_db_result()
|
||||
if not result.ok:
|
||||
sys.stderr.write(f"FATAL: {result.errors[0].ui_message()}\n")
|
||||
sys.exit(1)
|
||||
```
|
||||
|
||||
The error is propagated to the OS via `sys.exit(1)`. The drain point
|
||||
@@ -435,12 +435,12 @@ is the process termination itself.
|
||||
```python
|
||||
# COMPLIANT: drain point. The error is sent to monitoring.
|
||||
def _report_failure_to_telemetry(controller, op_name: str, result: Result[T]) -> None:
|
||||
if not result.ok:
|
||||
telemetry.emit_error(
|
||||
operation=op_name,
|
||||
kind=result.errors[0].kind.value,
|
||||
message=result.errors[0].message,
|
||||
)
|
||||
if not result.ok:
|
||||
telemetry.emit_error(
|
||||
operation=op_name,
|
||||
kind=result.errors[0].kind.value,
|
||||
message=result.errors[0].message,
|
||||
)
|
||||
```
|
||||
|
||||
The error reaches the telemetry system. The caller of the drain point
|
||||
@@ -452,12 +452,12 @@ receives `None`.
|
||||
# COMPLIANT: drain point. The retry is bounded and the final failure
|
||||
# is reported back to the user (which is itself a drain point).
|
||||
def _load_track_with_retry(controller, track_id: str) -> Track | None:
|
||||
for attempt in range(MAX_RETRIES):
|
||||
result = controller.get_track_result(track_id)
|
||||
if result.ok:
|
||||
return result.data
|
||||
time.sleep(BACKOFF_SECONDS * (attempt + 1))
|
||||
return None # Caller will display "failed after N attempts"
|
||||
for attempt in range(MAX_RETRIES):
|
||||
result = controller.get_track_result(track_id)
|
||||
if result.ok:
|
||||
return result.data
|
||||
time.sleep(BACKOFF_SECONDS * (attempt + 1))
|
||||
return None # Caller will display "failed after N attempts"
|
||||
```
|
||||
|
||||
The retry loop is a drain point: the function returns `Track | None`
|
||||
@@ -471,15 +471,15 @@ The following are **NOT** drain points. They are silent-fallback
|
||||
violations that lose data:
|
||||
|
||||
- **`sys.stderr.write(...)` alone** (without visible user feedback or
|
||||
app-level decision): the data is lost; the user sees nothing.
|
||||
Logging is NOT a drain.
|
||||
app-level decision): the data is lost; the user sees nothing.
|
||||
Logging is NOT a drain.
|
||||
- **`logging.error(...)` / `logger.exception(...)` alone**: same as
|
||||
above. The log is recorded, but the error is invisible to the user.
|
||||
above. The log is recorded, but the error is invisible to the user.
|
||||
- **`return default_value`** after a `try/except`: the original error
|
||||
context is lost; the caller cannot distinguish success from failure.
|
||||
context is lost; the caller cannot distinguish success from failure.
|
||||
- **`pass`**: silent. The data is lost.
|
||||
- **`traceback.print_exc(...)` alone**: similar to logging — visible in
|
||||
the console but invisible to the user.
|
||||
the console but invisible to the user.
|
||||
|
||||
**The key distinction:** a drain point **terminates the propagation**
|
||||
with a visible, intentional action. A log call or silent fallback
|
||||
@@ -490,13 +490,13 @@ with a visible, intentional action. A log call or silent fallback
|
||||
The two concepts are complementary:
|
||||
|
||||
- **Boundary types** (Section: "Boundary Types") describe WHERE
|
||||
exceptions originate or are converted (third-party SDK calls, stdlib
|
||||
I/O, FastAPI handlers). The catch site at a boundary converts the
|
||||
exception to `ErrorInfo` and returns it in `Result`.
|
||||
exceptions originate or are converted (third-party SDK calls, stdlib
|
||||
I/O, FastAPI handlers). The catch site at a boundary converts the
|
||||
exception to `ErrorInfo` and returns it in `Result`.
|
||||
- **Drain points** describe WHERE the `Result[T]` propagation
|
||||
terminates (HTTP error response, GUI display, app termination,
|
||||
telemetry, bounded retry). The function at a drain point returns
|
||||
`None` or raises into a framework; it does NOT return `Result[T]`.
|
||||
terminates (HTTP error response, GUI display, app termination,
|
||||
telemetry, bounded retry). The function at a drain point returns
|
||||
`None` or raises into a framework; it does NOT return `Result[T]`.
|
||||
|
||||
A function can be BOTH a boundary AND a drain point. The
|
||||
`_api_*` FastAPI handler is a boundary (catches SDK exceptions) and a
|
||||
@@ -510,12 +510,12 @@ Heuristic D that recognizes drain-point patterns as `INTERNAL_COMPLIANT`.
|
||||
The patterns are:
|
||||
|
||||
1. `except (SomeError): self.send_response(status); ...` (HTTP
|
||||
response in a `BaseHTTPRequestHandler` subclass)
|
||||
response in a `BaseHTTPRequestHandler` subclass)
|
||||
2. `except (SomeError): imgui.open_popup(...)` (GUI error display)
|
||||
3. `except (SomeError): sys.exit(...)` (intentional termination)
|
||||
4. `except (SomeError): telemetry.emit_*(...)` (telemetry)
|
||||
5. `except (SomeError): for attempt in range(N): ...; return None`
|
||||
(bounded retry; followed by `return None` or similar end-of-propagation)
|
||||
(bounded retry; followed by `return None` or similar end-of-propagation)
|
||||
|
||||
A site matching any of these is classified `INTERNAL_COMPLIANT`, with a
|
||||
note that the pattern is a drain point.
|
||||
@@ -554,18 +554,18 @@ calls):
|
||||
```python
|
||||
def _validate_collection_dim_result(self) -> Result[None]:
|
||||
if self.collection is None or self.collection == "mock":
|
||||
return Result(data=None)
|
||||
return Result(data=None)
|
||||
try:
|
||||
res = self.collection.get(limit=1, include=["embeddings"])
|
||||
# ... validation logic ...
|
||||
return Result(data=None)
|
||||
res = self.collection.get(limit=1, include=["embeddings"])
|
||||
# ... validation logic ...
|
||||
return Result(data=None)
|
||||
except Exception as e:
|
||||
return Result(data=None, errors=[
|
||||
ErrorInfo(kind=ErrorKind.INTERNAL,
|
||||
message=f"Failed to validate collection dim: {e}",
|
||||
source="rag._validate_collection_dim",
|
||||
original=e)
|
||||
])
|
||||
return Result(data=None, errors=[
|
||||
ErrorInfo(kind=ErrorKind.INTERNAL,
|
||||
message=f"Failed to validate collection dim: {e}",
|
||||
source="rag._validate_collection_dim",
|
||||
original=e)
|
||||
])
|
||||
```
|
||||
|
||||
This `except Exception` is **compliant** because the catch + ErrorInfo
|
||||
@@ -602,11 +602,11 @@ elaborates.
|
||||
```python
|
||||
class MyClass:
|
||||
def __init__(self, config: Config):
|
||||
if config is None:
|
||||
raise ValueError("MyClass requires a non-None Config")
|
||||
if not config.api_key:
|
||||
raise ValueError("MyClass requires a non-empty api_key")
|
||||
self._config = config
|
||||
if config is None:
|
||||
raise ValueError("MyClass requires a non-None Config")
|
||||
if not config.api_key:
|
||||
raise ValueError("MyClass requires a non-empty api_key")
|
||||
self._config = config
|
||||
```
|
||||
|
||||
**Compliant assert (for impossible states):**
|
||||
@@ -674,7 +674,7 @@ try:
|
||||
do_something(resource)
|
||||
finally:
|
||||
release(resource) # `finally` is cleaner; `except+raise` is for when
|
||||
# you also need to log or convert
|
||||
# you also need to log or convert
|
||||
```
|
||||
|
||||
Use `try/finally` for the pure cleanup case (no logging/conversion).
|
||||
@@ -759,15 +759,15 @@ script's output):
|
||||
Files scanned: 65
|
||||
Files with findings: 42
|
||||
Total sites: 348
|
||||
Compliant sites: 80
|
||||
Suspicious sites: 25
|
||||
Violation sites: 211
|
||||
Unclear (review): 32
|
||||
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
|
||||
Sites: 112, violations: 77
|
||||
--- Migration target (all other src/ files) ---
|
||||
Sites: 236, violations: 134
|
||||
Sites: 236, violations: 134
|
||||
```
|
||||
|
||||
The **baseline** is the 3 fully-refactored files (the convention reference).
|
||||
@@ -788,15 +788,15 @@ When converting existing code:
|
||||
|
||||
1. Identify the `Optional[X]` return type or the `raise` statement.
|
||||
2. Define a `Result` dataclass (or use the existing one) with `data: X` and
|
||||
`errors: list[ErrorInfo]`.
|
||||
`errors: list[ErrorInfo]`.
|
||||
3. Replace `None` returns with `Result(data=NIL_X, errors=[...])` or
|
||||
`Result(data=zero_value, errors=[...])`.
|
||||
`Result(data=zero_value, errors=[...])`.
|
||||
4. Replace `raise X` with
|
||||
`return Result(data=zero_value, errors=[ErrorInfo(kind=..., message=...)])`.
|
||||
`return Result(data=zero_value, errors=[ErrorInfo(kind=..., message=...)])`.
|
||||
5. Update the caller to check `result.errors` instead of `is None` /
|
||||
`try/except`.
|
||||
`try/except`.
|
||||
6. Add a test that verifies both the success and failure paths return the
|
||||
right `Result`.
|
||||
right `Result`.
|
||||
|
||||
---
|
||||
|
||||
@@ -816,13 +816,13 @@ When a function is migrated from `Optional[T]` / `raise` to `Result[T]`:
|
||||
```python
|
||||
# BEFORE (the legacy):
|
||||
def do_thing() -> Optional[str]:
|
||||
result = do_thing_result()
|
||||
if not result.ok: return None
|
||||
return result.data
|
||||
result = do_thing_result()
|
||||
if not result.ok: return None
|
||||
return result.data
|
||||
|
||||
# AFTER (the new):
|
||||
def do_thing_result() -> Result[str]:
|
||||
...
|
||||
...
|
||||
```
|
||||
|
||||
The `do_thing` function must be **deleted**, not kept as a wrapper. Keep only one entry point: `do_thing_result()`.
|
||||
@@ -832,7 +832,7 @@ The `do_thing` function must be **deleted**, not kept as a wrapper. Keep only on
|
||||
```python
|
||||
# After OBLITERATE: only do_thing_result exists
|
||||
def do_thing_result() -> Result[str]:
|
||||
...
|
||||
...
|
||||
```
|
||||
|
||||
Callers are rewritten:
|
||||
@@ -881,18 +881,18 @@ checklist before claiming a task is done.**
|
||||
**Before writing or modifying ANY `try/except` code, you MUST:**
|
||||
|
||||
1. **READ `conductor/code_styleguides/error_handling.md` end-to-end.**
|
||||
The 7 sections are: (1) The 5 Patterns, (2) Decision Tree,
|
||||
(3) Anti-Patterns, (4) Hard Rules, (5) Boundary Types, (6) The
|
||||
Broad-Except Distinction, (7) AI Agent Checklist (this section).
|
||||
The 7 sections are: (1) The 5 Patterns, (2) Decision Tree,
|
||||
(3) Anti-Patterns, (4) Hard Rules, (5) Boundary Types, (6) The
|
||||
Broad-Except Distinction, (7) AI Agent Checklist (this section).
|
||||
|
||||
2. **Acknowledge the read in the commit message.** Format: "TIER-2
|
||||
READ conductor/code_styleguides/error_handling.md before
|
||||
<phase/task>."
|
||||
READ conductor/code_styleguides/error_handling.md before
|
||||
<phase/task>."
|
||||
|
||||
3. **The styleguide is the source of truth.** Your training data is
|
||||
the OPPOSITE of this convention. Idiomatic Python (`try/except` +
|
||||
`Optional[T]` + `raise Exception`) is what the convention is
|
||||
designed to REPLACE.
|
||||
the OPPOSITE of this convention. Idiomatic Python (`try/except` +
|
||||
`Optional[T]` + `raise Exception`) is what the convention is
|
||||
designed to REPLACE.
|
||||
|
||||
**Why:** the previous round (Phase 10) added 5 LAUNDERING HEURISTICS to
|
||||
the audit script that classified narrowing as compliant, which is the
|
||||
@@ -906,75 +906,75 @@ re-introducing laundering heuristics.**
|
||||
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.
|
||||
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.
|
||||
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.
|
||||
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.
|
||||
`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.
|
||||
"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/`). Use `Result[T]` instead. CI fails if you add a new
|
||||
`Optional[T]` return type to any `src/*.py` (enforced by
|
||||
`scripts/audit_optional_in_baseline_files.py --strict`,
|
||||
which scans all `src/*.py` as of 2026-06-27).
|
||||
`src/`). Use `Result[T]` instead. CI fails if you add a new
|
||||
`Optional[T]` return type to any `src/*.py` (enforced by
|
||||
`scripts/audit_optional_in_baseline_files.py --strict`,
|
||||
which scans all `src/*.py` as of 2026-06-27).
|
||||
|
||||
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.
|
||||
`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.
|
||||
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`.
|
||||
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.
|
||||
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`.
|
||||
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`.
|
||||
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)
|
||||
|
||||
@@ -982,20 +982,20 @@ 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`.
|
||||
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`.
|
||||
`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.
|
||||
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
|
||||
|
||||
@@ -1038,14 +1038,14 @@ automated check; the checklist is the manual one.
|
||||
---
|
||||
|
||||
- `conductor/tracks/data_oriented_error_handling_20260606/spec.md` — the spec
|
||||
that established this convention.
|
||||
that established this convention.
|
||||
- `docs/guide_ai_client.md` "Data-Oriented Error Handling (Fleury Pattern)"
|
||||
— the in-context guide for the provider layer.
|
||||
— the in-context guide for the provider layer.
|
||||
- `docs/guide_mcp_client.md` "Data-Oriented Error Handling (Fleury Pattern)"
|
||||
— the in-context guide for the MCP tool layer.
|
||||
— the in-context guide for the MCP tool layer.
|
||||
- `conductor/code_styleguides/data_oriented_design.md` (added 2026-06-12) — the canonical Data-Oriented Design (DOD) reference; this track is the canonical application of DOD to error handling ("errors are data, not control flow").
|
||||
- `conductor/code_styleguides/agent_memory_dimensions.md` (added 2026-06-12) — the 4-dim memory model; the knowledge harvest TDD protocol in `workflow.md` uses this track's `Result` pattern.
|
||||
- `docs/guide_rag.md` "Data-Oriented Error Handling (Fleury Pattern)" — the
|
||||
in-context guide for the RAG engine.
|
||||
in-context guide for the RAG engine.
|
||||
- Ryan Fleury's [original article](https://www.dgtlgrove.com/p/the-easiest-way-to-handle-errors)
|
||||
— the philosophical foundation.
|
||||
— the philosophical foundation.
|
||||
|
||||
Reference in New Issue
Block a user