rfc: search contracts and retrieval algebra - #606
Conversation
The public proposal required by the governance reset of PR #595 (size-L): retire the provably inert fuzzy() with a stable T25 diagnostic, reject silently-dropped search/rank targets on traversal-introduced bindings (T26), state retrieval in the lowered plan instead of re-inferring it from order_by[0], make rank metrics projectable with structural identity to the executed retrieval, give every ranked result a total deterministic order (fusion and aggregation included), and add the additive warnings/metrics/retrievals read envelope with exact prefiltered embedding coverage. The RFC follows the template's section set and additionally carries a clearly-marked non-normative agent-context appendix: substrate dependencies and their validated assumptions, cross-PR composition contracts (#587 gate, RFC 0040/0043 adjacency), and the build/CI traps observed while producing the prototype evidence (closed PR #595, branch retained). Registry: adds the 0047 row and advances next-available to 0048 (the stale 0045 reservation clause is retired — 0045 has landed).
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
The decision log and agent appendix referenced an internal design-graph record by identifier. Public history carries the public facts only (the prototype PR, the evidence classes); the larger program is described without internal addressing, per the change-discipline rule against private planning shorthand in public history.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b7ce5a344
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - **Physical acceleration is derived (7):** preserved — the unindexed-column | ||
| condition warns, it does not fail; index absence changes cost and (on the | ||
| flat fallback) analysis behavior, which is exactly what the warning makes | ||
| visible. Recall reporting is contractual, not plan-derived. |
There was a problem hiding this comment.
Prevent missing FTS indexes from changing search answers
When the requested column has no FTS index, this design explicitly permits a case-sensitive fallback whose results differ from indexed execution and only adds a warning. The same query can therefore gain or lose rows after index reconciliation even though an index is derived state; making that discrepancy visible does not restore logical correctness. Use analyzer-equivalent fallback behavior or fail closed instead.
AGENTS.md reference: AGENTS.md:L99-L100
Useful? React with 👍 / 👎.
| - **Coverage.** Ready/pending counts reuse the scan's own structured | ||
| predicate through a sealed, streaming count on the storage boundary — no | ||
| SQL strings, no retained batches, computed only for `@embed`-backed vector | ||
| retrievals. |
There was a problem hiding this comment.
Bound coverage work independently of table size
For an indexed nearest query with a small limit and no selective filter, computing exact ready/pending counts through a streaming predicate still scans the entire source/vector population on every request. Streaming bounds retained memory, but not the O(table-size) I/O and latency added to an otherwise sublinear ANN read; coverage must use bounded metadata, be opt-in, or otherwise avoid an unconditional full-population count.
AGENTS.md reference: AGENTS.md:L104-L105
Useful? React with 👍 / 👎.
| 1. `fuzzy()` is retired with a stable `T25` compile diagnostic — it provably | ||
| never matched under the supported tokenizer, so every use was a confident | ||
| empty answer. |
There was a problem hiding this comment.
Prove fuzzy is universally inert before retiring it
The cited characterization in crates/omnigraph/tests/search.rs:1764-1787 exercises only one capitalized, stem-sensitive typo (Introductio), which cannot establish that every fuzzy() invocation returns empty. Fuzzy matching includes zero-edit matches, so normalized terms such as lower-case deep can still match indexed terms; rejecting every use with T25 would break working stored queries despite the compatibility section calling all affected usage provably broken. Add representative exact/stem/case/max-edit evidence or narrow the retirement to the actually inert shape.
AGENTS.md reference: AGENTS.md:L151-L153
Useful? React with 👍 / 👎.
| A complete prototype exists (closed PR #595, branch | ||
| `search-contracts-p0-p1`, retained as evidence per the closure note): eleven | ||
| staged commits, canonical workspace graph green (2,860 tests), both Clippy | ||
| gates, OpenAPI regenerated, vocabulary-guard inventory classified. Test | ||
| owners extended, not forked: compiler typecheck/lowering suites (T25, T26, |
There was a problem hiding this comment.
Record the exact Lance surfaces reviewed
This Lance-dependent RFC reports only that an unspecified impact analysis and source validation occurred; it does not identify the complete upstream index, FTS, tokenizer, vector, or DataFusion pages reviewed. The RFC process requires the exact version and surveyed surfaces so acceptance can verify that the fallback, tie, and index-lifecycle assumptions were checked against the pinned substrate rather than an unavailable prototype branch.
AGENTS.md reference: AGENTS.md:L13-L17
Useful? React with 👍 / 👎.
The broader search-contracts program as a public draft RFC: three composable search contracts, schema-owned analyzed semantics (@analyzed with substrate-identity fingerprints), schema-bound vector distance and mandatory embedding-space identity, match_terms/knn/ann with a typed oversample dial, N-arm weighted rrf, typed metric domains, and the SchemaIR/format rebuild boundary they require. Blocked on RFC 0047 (its first slice); cross-links added in 0047 and the registry (next available number is now 0049).
0047 and 0048 are allocated by PR #606, so the RFC becomes 0049 and the registry's next number 0050. From review: the ledger restore seam is withdrawn, since its real use is a coherent restore point where the ledger and the graphs come back together; readiness reports counts only and the graph ids stay behind the authenticated GET /graphs; the shutdown watchdog is an operating-system thread armed by a listener installed before graphs open; the authority enum gains `unlocked`; and the compatibility section states the Rust-level breaks instead of calling the change additive. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015cD8PEeUfrzpqYuU1jaZBq
0047 and 0048 are allocated by PR #606, so the RFC becomes 0049 and the registry's next number 0050. From review: the ledger restore seam is withdrawn, since its real use is a coherent restore point where the ledger and the graphs come back together; readiness reports counts only and the graph ids stay behind the authenticated GET /graphs; the shutdown watchdog is an operating-system thread armed by a listener installed before graphs open; the authority enum gains `unlocked`; and the compatibility section states the Rust-level breaks instead of calling the change additive. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015cD8PEeUfrzpqYuU1jaZBq
rfc: accept RFC 0049 Maintainer decision recorded in the decision log; the registry row moves to accepted. Implementation status advances with the two implementation PRs. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015cD8PEeUfrzpqYuU1jaZBq rfc: renumber to 0049, drop the ledger restore seam, record the review 0047 and 0048 are allocated by PR #606, so the RFC becomes 0049 and the registry's next number 0050. From review: the ledger restore seam is withdrawn, since its real use is a coherent restore point where the ledger and the graphs come back together; readiness reports counts only and the graph ids stay behind the authenticated GET /graphs; the shutdown watchdog is an operating-system thread armed by a listener installed before graphs open; the authority enum gains `unlocked`; and the compatibility section states the Rust-level breaks instead of calling the change additive. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015cD8PEeUfrzpqYuU1jaZBq rfc: add RFC 0048, control-plane seams Four small, independently shippable contracts an external control plane needs from the cluster crate and the server: observe-only reads (`plan --observe`, `cluster observe`), a ledger restore (`cluster state restore`), a readiness witness (`GET /readyz`), and a bounded shutdown (`--shutdown-grace-seconds`). Nothing changes a storage format, the ledger, the lock, or the recovery protocol; RFC 0034 and 0035 stay independent. The next number becomes 0049: 0047 is held by an out-of-tree draft. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015cD8PEeUfrzpqYuU1jaZBq
|
Azim, the latest RFC 0048 iteration makes bounded retrieval compose explicitly with ordinary graph queries. This covers the staged retrieval redesign and the follow-up migration, implementation-phase, and default changes through
The main review points are stage/metric grammar and identity through graph fan-out, the schema-default declaration and migration rules, and the remaining scoring, encoding-identity, and NFC/index qualification gates. Both RFCs remain drafts with implementation not started. This revision changes documentation only; documentation checks passed. |
Proposes two public draft RFCs for search planning and language contracts.
RFC 0047 — Search plan truth introduces compiled retrieval plans, scan-rooted target validation (
T26), projectable ranking metrics, deterministic ordering, and response warnings/metadata. It no longer retiresfuzzy()through a standaloneT25stage: existing fuzzy queries can match, and the replacement belongs with the schema-owned lexical contract. Warnings expose existing index-dependent behavior; they do not establish correctness.RFC 0048 — Search contracts and retrieval algebra defines exact value predicates, analyzed lexical membership, and ranked retrieval.
match_terms(field, query, mode: all|any, max_edits: 0|1|2)replacesfuzzy,search, andmatch_textin one pre-stable breaking release, without compatibility aliases or a deprecation window. It defaults to all-term, zero-edit matching and uses the same schema-owned analyzer on every execution path. Exact streaming evaluation is the baseline; native index acceleration requires parity evidence, and expansion overflow must scan within budget or fail explicitly. Fuzzy membership does not implicitly change BM25 scoring.RFC 0048 also covers immutable analyzer/scorer profiles with substrate fingerprints, schema-bound vector distance and embedding-space identity, exact
knn, approximateann(oversample:), explicit N-arm RRF windows/weights, typed metric domains, and the accepted-SchemaIR rebuild boundary. Its separatenearestdeprecation remains specified in the RFC.The evidence distinguishes the historical PR #595 prototype from the revised proposal. Existing-fixture and disposable-graph probes demonstrate working typo matches and index-state divergence; the planned qualification matrix covers normalization, edit-distance monotonicity, index lifecycle, expansion completeness, resource failures, and ranking composition.
A second audit rechecked checksum-matched Lance 11.0.0 source against the full upstream guides. The revised text specifies analyzer settings and Unicode limitations, disables silent token-length filtering in the new profiles, and distinguishes public tokenizer primitives from the new Boolean evaluator and its resource protocol. It also corrects claims about native tie completeness and monotone ANN recall; score parity remains a separate qualification gate. A pinned-library probe passed 73,008 Unicode edit-distance comparisons plus analyzer-boundary and construction-failure assertions. These are primitive-level checks, not a completed engine implementation.
Both RFCs remain drafts with implementation not started. This PR changes proposal documents and their registry entries; acceptance unlocks the implementation stages described in each RFC.
Validation:
python3 scripts/check-docs.py,bash scripts/check-agents-md.sh,typos, andgit diff --checkpass. No runtime code changed.