feat(directives): harvest 2 directives from error_handling.md (Result pattern + nil-sentinel)
This commit is contained in:
@@ -0,0 +1,33 @@
|
||||
# nil_sentinel_pattern — v1
|
||||
|
||||
**Why this iteration:** Lifted verbatim from `conductor/code_styleguides/error_handling.md` §"The 5 Patterns" #1 (lines 23-47).
|
||||
This is the baseline encoding — the convention-style description with code example currently in production.
|
||||
Future variants will test alternative encodings (rationale-first, before/after, tabular) against this baseline.
|
||||
|
||||
**Source:** `conductor/code_styleguides/error_handling.md:23-47`
|
||||
|
||||
---
|
||||
|
||||
### 1. Nil-Sentinel Dataclasses (replaces `None`)
|
||||
|
||||
When a function would "return None" in conventional Python, return a
|
||||
nil-sentinel dataclass instead. The sentinel has all default values
|
||||
(zero-initialized) and is safe to read from.
|
||||
|
||||
```python
|
||||
from dataclasses import dataclass, field
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class NilPath:
|
||||
exists: bool = False
|
||||
read_text: str = ""
|
||||
errors: list[ErrorInfo] = field(default_factory=list)
|
||||
|
||||
NIL_PATH = NilPath() # module-level singleton
|
||||
```
|
||||
|
||||
Callers don't need `if x is None:` checks; they can call `x.read_text` and
|
||||
get `""` on the nil path.
|
||||
|
||||
**Convention:** `NIL_*` (uppercase) is the module-level singleton. `Nil*`
|
||||
(PascalCase) is the class. Frozen dataclass prevents runtime mutation.
|
||||
@@ -0,0 +1,155 @@
|
||||
# result_error_pattern — v1
|
||||
|
||||
**Why this iteration:** Lifted verbatim from `conductor/code_styleguides/error_handling.md` §"The 5 Patterns" (lines 21-131) + §"Hard Rules" (lines 213-242).
|
||||
This is the baseline encoding — the prose+code-example style currently in production.
|
||||
Future variants will test alternative encodings (rationale-first, tabular) against this baseline.
|
||||
|
||||
**Source:** `conductor/code_styleguides/error_handling.md:21-131 + 213-242`
|
||||
|
||||
---
|
||||
|
||||
## The 5 Patterns
|
||||
|
||||
### 1. Nil-Sentinel Dataclasses (replaces `None`)
|
||||
|
||||
When a function would "return None" in conventional Python, return a
|
||||
nil-sentinel dataclass instead. The sentinel has all default values
|
||||
(zero-initialized) and is safe to read from.
|
||||
|
||||
```python
|
||||
from dataclasses import dataclass, field
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class NilPath:
|
||||
exists: bool = False
|
||||
read_text: str = ""
|
||||
errors: list[ErrorInfo] = field(default_factory=list)
|
||||
|
||||
NIL_PATH = NilPath() # module-level singleton
|
||||
```
|
||||
|
||||
Callers don't need `if x is None:` checks; they can call `x.read_text` and
|
||||
get `""` on the nil path.
|
||||
|
||||
**Convention:** `NIL_*` (uppercase) is the module-level singleton. `Nil*`
|
||||
(PascalCase) is the class. Frozen dataclass prevents runtime mutation.
|
||||
|
||||
### 2. Zero-Initialization (via `@dataclass` defaults)
|
||||
|
||||
Fresh memory from the OS is zero-initialized. In Python, `@dataclass` with
|
||||
field defaults achieves the same: the data is in a valid "empty" state
|
||||
without any explicit constructor logic.
|
||||
|
||||
```python
|
||||
@dataclass(frozen=True)
|
||||
class String8:
|
||||
text: str = ""
|
||||
size: int = 0
|
||||
```
|
||||
|
||||
Code that consumes `String8` (e.g., a for-loop bounded by `size`) works
|
||||
correctly with the zero-initialized instance.
|
||||
|
||||
**Convention:** Mutable defaults use `field(default_factory=list)` (NOT `= []`,
|
||||
which is shared across instances).
|
||||
|
||||
### 3. Fail Early (push validation to shallow stack frames)
|
||||
|
||||
Don't defer error checks to deep in the call stack. Push them to the entry
|
||||
point so the user knows ASAP if the operation cannot succeed.
|
||||
|
||||
```python
|
||||
def do_thing(path: Path) -> Result[str]:
|
||||
resolved = _resolve_path(path) # validation happens HERE, not deeper
|
||||
if not resolved.ok:
|
||||
return Result(data="", errors=resolved.errors)
|
||||
...
|
||||
```
|
||||
|
||||
**Convention:** `assert` at entry points for invariants. Early `return` for
|
||||
user-facing errors. `try/finally` (Python's analog to `goto defer`) for
|
||||
cleanup.
|
||||
|
||||
### 4. AND over OR (Result with side-channel errors; no sum types)
|
||||
|
||||
Instead of `Union[T, E]` or `Result<T, E>`, return a struct with BOTH data
|
||||
and errors as parallel fields:
|
||||
|
||||
```python
|
||||
@dataclass(frozen=True)
|
||||
class Result(Generic[T]):
|
||||
data: T # the happy-path result (zero-initialized on failure)
|
||||
errors: list[ErrorInfo] = field(default_factory=list) # side-channel; empty = success
|
||||
```
|
||||
|
||||
Callers:
|
||||
|
||||
```python
|
||||
r = do_thing(path)
|
||||
if r.errors:
|
||||
for err in r.errors: log(err.ui_message())
|
||||
# use r.data regardless (it's the zero-initialized value on failure)
|
||||
```
|
||||
|
||||
**Convention:** `Result` is generic over `T` (the success data) but NOT over
|
||||
the error type. Errors are always `list[ErrorInfo]` (a side-channel list, not
|
||||
a tagged sum). This collapses the bifurcated `if r.ok: ... else: ...`
|
||||
codepaths into a single flat codepath.
|
||||
|
||||
### 5. Error Info as Side-Channel (not as exception)
|
||||
|
||||
Errors flow as DATA in the `Result` struct, not as exceptions. SDK
|
||||
boundaries (which must catch vendor exceptions) convert them to `ErrorInfo`:
|
||||
|
||||
```python
|
||||
@dataclass(frozen=True)
|
||||
class ErrorInfo:
|
||||
kind: ErrorKind
|
||||
message: str
|
||||
source: str = ""
|
||||
original: BaseException | None = None
|
||||
def ui_message(self) -> str:
|
||||
src = f"[{self.source}] " if self.source else ""
|
||||
return f"{src}{self.kind.value}: {self.message}"
|
||||
```
|
||||
|
||||
**Convention:** `ErrorInfo` is the canonical error type. The legacy
|
||||
`ai_client.ProviderError` exception class is removed; SDK helpers
|
||||
(`_classify_<vendor>_error()`) RETURN `ErrorInfo` instead of raising.
|
||||
|
||||
---
|
||||
|
||||
## Hard Rules (enforced in all `src/*.py` as of 2026-06-27)
|
||||
|
||||
These are non-negotiable in all `src/*.py` files. The migration-target
|
||||
files (14 of them) were historically not enforced; as of 2026-06-27 the
|
||||
`scripts/audit_optional_in_baseline_files.py --strict` audit (renamed
|
||||
from `_in_3_files.py` per the contradictions report) covers all
|
||||
`src/*.py`, and the `cruft_elimination_20260627` track documents the
|
||||
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.
|
||||
- **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.
|
||||
- **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.
|
||||
|
||||
The verification script `scripts/audit_optional_returns.py` enforces the
|
||||
`Optional[X]` rule by failing CI if any new `Optional[X]` return type
|
||||
appears in any `src/*.py` file. (As of 2026-06-27 this is the successor to
|
||||
`scripts/audit_optional_in_3_files.py`, which covered only 4 baseline files;
|
||||
the new script scans all `src/*.py` per the cruft_elimination_20260627
|
||||
expansion of the ban.)
|
||||
Reference in New Issue
Block a user