Repository navigation
Conversation
ragnorc
left a comment
There was a problem hiding this comment.
Recommendation: request changes for one contract inconsistency, detailed inline. Reviewed exact head 5e6e82e102d04b2ad82beaca728fcec33e15c56d. This is a draft RFC review, not a claim that the proposed options already work.
This RFC makes search choices explicit. It separates candidate targets from returned rows, adds weighted fusion and arm metrics, and lets callers request exact vector results. It also makes vector distance and embedding recipes part of the accepted schema. The aim is useful: an agent should know which population a query searched, what its count means, and which approximations can affect the answer. The proposal uses existing ranking calls and typed plan nodes, so it avoids a second query syntax for the same operations.
One decision remains inconsistent. Law 3 says a downstream limit never sizes an upstream window. The proposed default does exactly that above 100. A deterministic example using the RFC's fusion formula changes the winner when only limit changes from 100 to 101. A constant-window control preserves the result prefix. This matters when an investigation requests more evidence from the same snapshot. Larger output requests should have the contract the RFC promises. Either make the default independent or state the exception and its consequences.
The main factual premises match this checkout:
- The planner derives a vector arm's fetch from
limitand leaves a BM25 arm uncapped.search_nodealso passeslimitinto leading nearest retrieval. - The grammar accepts two RRF arms and an optional positional
kargument. The compiler requires a limit and restricts projected rank expressions throughT33. It rejects alias ordering beside standalone nearest throughT18. - Vector indexes use IVF_FLAT with L2. Generated embeddings pass finite-value, dimension, and nonzero-norm checks before normalization. The pinned Lance dot implementation agrees with the proposed
1 - dotvalue.
The tradeoffs need to remain explicit:
- For small investigative result sets, a 100-target vector window spends more work than today's window to improve fusion recall. It does not establish a recall guarantee. No workload measurement here establishes 100 as the best default.
- Exact vector selection requires the eligible population's distances and stable membership at boundary ties. This can cost much more than index probes, especially with remote storage.
use_index(false)supplies a flat route. The implementation must still enforce the RFC's target-ID tie rule before discarding candidates. - Up to 16 arms adds scoring work and nullable metric columns. Uncapped BM25 arms can still be large. Candidate bounds do not replace the shared memory protocol or prove a fixed request cost.
- Persisted distances and recipe identities add migration, export, and compatibility obligations. Changing an index metric and changing an embedding space require different repair work. The RFC correctly keeps those identities separate.
The liability balance is promising but conditional. These 600 added lines are design text, not runtime complexity. Reusing calls, one typed option mechanism, and one validator can remove ambiguity without adding execution frameworks. Schema identity and additional ranking options still create long-term obligations. Five similar extensions should compose through the same population, target, cut, and ownership rules. Contradictory defaults would instead force callers and tests to learn exceptions. Resolve that decision before implementation makes it a compatibility cost.
Validation and limits:
- Local exact head: documentation checks, AGENTS links, and spelling checks passed. All six existing compiler tests selected by
rrfpassed. - An independent arithmetic oracle reproduced the winner change and checked the fixed-window control. This evaluates the proposed contract, not an implementation of named options.
- Code inspection covered compiler admission, planner fetch values, fusion ranks, the scan adapter, embedding normalization, and index construction. I used the complete relevant upstream search/index guides and the exact pinned Lance 11 source.
- Documentation CI passed on merge commit
0a937f239aefb738a62aad7290b15bf7047b988d, which contains this head. The workspace test job was skipped. CI is not implementation evidence for these proposed features. - I did not measure production latency, rerun cloud suites, or independently verify the cited private agent-competence results. I made no source edits. The checkout is clean.
| - **A `nearest` arm of `rrf()`** always has a window. The default is the | ||
| query's `limit` or 100, whichever is larger, so the arm can fill the limit | ||
| on its own; today it fetches exactly `limit` targets. RFC 0047 keeps that | ||
| cap distinct from the final row cut. |
There was a problem hiding this comment.
[P2] Resolve the conflict between the default window and law 3
max(limit, 100) still lets a downstream row limit change the upstream ranking population. Law 3 forbids this, and the Alternatives section rejects that coupling.
The difference can change the winner. Use k = 60, unit weights, and fixed arm rankings. X has lexical rank 1 and vector rank 101. A has lexical rank 2 and vector rank 100. The first 99 vector targets have no lexical match. With limit 100, A scores 1/62 + 1/160, while X scores only 1/61, so A wins. With limit 101, X gains 1/161 and becomes the winner. The snapshot, eligibility, and arm rankings did not change.
Please choose one contract before acceptance. An independent bounded default preserves law 3. If the coupling is intentional, state that exception in the law and explain that increasing limit can replace earlier results. Apply the same decision to the retained implicit window for leading nearest. Also define the default for limit > 10000, where this formula exceeds the table's candidate bound. Add these boundary cases to the proposed oracle.
What this is
RFC 0048 states the contract search keeps as it grows beyond what RFC 0047 fixes, and adds only the options that contract needs. The options are named arguments on the ranking calls that already exist (
nearest,bm25,rrf). It adds no clause and no stage.It replaces the RFC 0048 draft and the GQ composition draft in #606. Those drafts proposed a staged query syntax (
rank … yield, named sources,metric(),limit … of … per). The shared expression model keeps ranking as the leadingorderkey instead, and engine v2's planner already builds typed retrieval nodes, so the staged syntax is withdrawn. This version is written againstmainb14c22c5.The problems (checked in code)
rrf()vector arm fetches exactlylimittargets while a BM25 arm fetches every match. Under an aggregate, a leadingnearestreads as many rows as the query returns groups.rrf()takes exactly two positional arms. Arms cannot be weighted, and an arm's own score cannot be returned.Vector(n)declares no distance, every vector index is built with L2, and@embed(…, model=…)records only a model label.The proposal
limit, one target identity, one snapshot, exact answers independent of index state.candidates:windows. Anearestarm ofrrf()always has one (default: the larger oflimitand 100). Abm25arm keeps its full matching population, as RFC 0047 specifies, unless the query caps it. A leading ranking may declare one. A leadingnearestin an aggregate must declare one.rrf()with 2 to 16 arms,weight:per arm and a namedk:. Each arm's score can be returned, and it is null where that arm did not select the target.nearest(…, exact: true)for exact results under every index state.Vector(n, distance="l2" | "cosine" | "dot"), where an omitted distance meansl2(today's meaning), plus embedding recipes whose identity is more than a model label.Deferred, each needing its own decision: per-group selection, a second ranking after a traversal, fusion over different bindings, discovery across node types, scoring features, learned rerankers, and ranked pagination.
Related
terms,match_terms,bm25_v1): rfc: analyzed lexical search on the shared expression model #792Checks:
python3 scripts/check-docs.py,bash scripts/check-agents-md.sh,typos.Update (2026-10-01)
Aligned with RFC 0047 as merged (#791): a
bm25arm stays uncapped by default;exact: truebreaks a tie at the window boundary by target id; arm metrics use RFC 0047's retrieval ids; RFC 0047's validator checks this RFC's options as invariants. Every factual claim was checked again againstmain92ea5449, which corrected several: an arm's score is refused today byT33, notT37; the aggregate example needsT18lifted; properties accept@descriptiononly; these declarations add a SchemaIR feature name (RFC 0040), not a version. The RFC now also names its relaxation ofT33and its extension of RFC 0047'srecallrule.