From c9b085ff65c649aca55a7b8fe108a1c27feb7e75 Mon Sep 17 00:00:00 2001 From: Ed_ Date: Thu, 11 Jun 2026 23:39:24 -0400 Subject: [PATCH] docs(rag): document new Result return types + NilRAGState sentinel --- docs/guide_rag.md | 134 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 126 insertions(+), 8 deletions(-) diff --git a/docs/guide_rag.md b/docs/guide_rag.md index cba14601..600a0381 100644 --- a/docs/guide_rag.md +++ b/docs/guide_rag.md @@ -258,22 +258,46 @@ The injection point is **before** the system prompt construction. This means the ### Public Methods +> **As of 2026-06-11:** The signatures below document the **post-refactor** +> `Result[T]` returns applied by the `data_oriented_error_handling_20260606` +> track. The pre-refactor methods raised `ImportError` / `ValueError` or +> silently set `self.collection = None` on failure. See the new +> [Data-Oriented Error Handling (Fleury Pattern)](#data-oriented-error-handling-fleury-pattern) +> section below for the full convention. + ```python # Index a single file -rag_engine.index_file(path: str) -> None +rag_engine.index_file(path: str) -> Result[None] +# data=None on both success and failure; check result.errors # Search the index -rag_engine.search(query: str, top_k: int = 5) -> List[Dict[str, Any]] -# Returns: [{"text": str, "metadata": dict, "distance": float}, ...] +rag_engine.search(query: str, top_k: int = 5) -> Result[list[dict[str, Any]]] +# data is the list of {"text", "metadata", "distance"} hits; [] on failure +# Result[None] in the unconfigured case (data=NIL_RAG_STATE) # Index management -rag_engine.add_documents(ids: List[str], texts: List[str], metadatas: Optional[List[dict]] = None) -> None -rag_engine.delete_documents(ids: List[str]) -> None -rag_engine.delete_documents_by_path(path: str) -> None -rag_engine.get_all_indexed_paths() -> List[str] -rag_engine.is_empty() -> bool +rag_engine.add_documents( + ids: List[str], + texts: List[str], + metadatas: Optional[List[dict]] = None, +) -> Result[None] +rag_engine.delete_documents(ids: List[str]) -> Result[None] +rag_engine.delete_documents_by_path(path: str) -> Result[None] +rag_engine.get_all_indexed_paths() -> Result[list[str]] +rag_engine.is_empty() -> Result[bool] +# All return Result; on error, data is the zero value and result.errors is populated ``` +The `RAGEngine._init_vector_store_result()` and +`RAGEngine._validate_collection_dim_result()` methods are the new +internal entry points that produce `Result[None]`. They replace the +old `_init_vector_store()` (which raised `ImportError` on missing +chromadb, or `ValueError` on unknown vector-store provider) and the +old `_validate_collection_dim()` (which caught `Exception` and silently +corrupted the collection). Post-refactor, every failure path produces a +typed `ErrorInfo` entry; the application can react instead of crashing +on an unhandled exception. + --- ## Configuration @@ -413,7 +437,101 @@ def test_rag_augmented_send(live_gui): For unit tests that don't need real embedding models, the `BaseEmbeddingProvider` is mocked to return deterministic vectors (e.g., based on the hash of the input text). --- +## Data-Oriented Error Handling (Fleury Pattern) +The RAG engine follows the "errors are just cases" framework +(Ryan Fleury). The canonical reference is +[`conductor/code_styleguides/error_handling.md`](../conductor/code_styleguides/error_handling.md). + +### Result-Based Returns + +RAG methods that previously raised `ImportError`, `ValueError`, or +silently mutated `self.collection = None` on failure now return +`Result[T]` with side-channel `ErrorInfo` entries: + +| Method | Pre-refactor | Post-refactor | +|---|---|---| +| `_init_vector_store()` | `raise ImportError` (no chromadb) or `raise ValueError` (unknown provider) | `_init_vector_store_result() -> Result[None]` | +| `_validate_collection_dim()` | `except Exception: pass` (silent corruption) | `_validate_collection_dim_result() -> Result[None]` | +| `is_empty()` | `bool` (or `None` if collection failed) | `Result[bool]` (data is `False` on failure) | +| `add_documents()` | `raise` on chromadb error | `Result[None]` (errors as `ErrorInfo`) | +| `search()` | `List[Dict]` (or `[]` on failure) | `Result[list[dict]]` (data is `[]` on failure) | +| `index_file()` | `raise` on missing file or chromadb error | `Result[None]` (errors as `ErrorInfo`) | + +### Nil-Sentinel Pattern + +The `NIL_RAG_STATE` dataclass is the "RAG engine in unconfigured/failed- +to-init state" — it has all default values and is safe to read from: + +```python +@dataclass(frozen=True) +class NilRAGState: + enabled: bool = False + is_empty_result: bool = True + errors: list[ErrorInfo] = field(default_factory=list) + +NIL_RAG_STATE = NilRAGState() # module-level singleton +``` + +When the RAG engine is in this state (e.g., chromadb isn't installed, +or the configured provider is unknown), methods that would have raised +now return `Result` with `data=NIL_RAG_STATE` and the error in +`.errors`. Callers can check `if isinstance(result.data, NilRAGState): + handle_as_disabled()` — but most callers just need to know +"should I render the RAG panel as enabled?" and +`NIL_RAG_STATE.enabled == False` is fine. + +### Constructor Behavior + +`RAGEngine.__init__` still raises for "config missing" (fail early at +init — that's a programmer error). "Config invalid" (e.g., bad +embedding provider, bad chromadb collection) defers to +`_init_vector_store_result()` and is called explicitly or lazily. The +constructor itself returns a "best-effort" instance with +`self.collection = NIL_COLLECTION` if init fails; the first call to +`search()` / `add_documents()` etc. will surface the deferred error +in its `Result.errors`. + +### Example + +```python +from src import rag_engine +from src.result_types import ErrorKind + +result = rag_engine.search("user query", top_k=5) +if result.errors: + for err in result.errors: + if err.kind == ErrorKind.NOT_READY: + log.info("RAG not yet warmed: %s", err.message) + elif err.kind == ErrorKind.CONFIG: + log.warning("RAG misconfigured: %s", err.message) + else: + log.error(err.ui_message()) +# use result.data regardless (it's the zero-initialized [] on failure) +for hit in result.data: + process(hit) +``` + +### Dimension Mismatch Protection (Recovers via `ErrorInfo`) + +The 2026-06-06 collection-dim-mismatch bug fix +(commit `16412ad5`) lives inside `_validate_collection_dim_result()` +post-refactor. When the on-disk collection's dim doesn't match the +current embedding provider's dim, the method returns +`Result[None]` with a single `ErrorInfo(kind=ErrorKind.CONFIG, ...)` +instead of raising `InvalidDimensionError` deep in chromadb. The +caller (`_init_vector_store_result()`) sees the error in the +`.errors` list and can recreate the collection. This is the canonical +"SDK boundary catches, convert to ErrorInfo" pattern in action. + +### See Also (in-doc) + +- [`conductor/code_styleguides/error_handling.md`](../conductor/code_styleguides/error_handling.md) — canonical styleguide (5 patterns, data model, decision tree, anti-patterns) +- [`conductor/tracks/data_oriented_error_handling_20260606/spec.md`](../conductor/tracks/data_oriented_error_handling_20260606/spec.md) — the spec that introduced this pattern +- [`docs/guide_ai_client.md`](guide_ai_client.md#data-oriented-error-handling-fleury-pattern) — same pattern in the provider layer +- [`docs/guide_mcp_client.md`](guide_mcp_client.md#data-oriented-error-handling-fleury-pattern) — same pattern in the MCP tool layer + +--- ## Edge Cases & Limitations 1. **Empty Index**: If the index has no documents, `search()` returns `[]` and no context is injected. The AI call proceeds normally with just the explicit file context.