Repository navigation
feat(query): a ranked read reports its ranking - #1130
Conversation
A ranked read's JSON envelope gains two additive arrays derived from the accepted plan: retrievals (one per ranked scan, per rrf() fusion and per fusion arm, ids in plan preorder with a fusion before its arms and the arms in argument order) and metrics (one per returned column whose value is a score a retrieval of the plan writes, a return distinct group key included). The planner derives them; execution attaches them to the query result before the plan runs, so every read door and a replay report the same ids for one plan. No parameter value is included, and a stored property named like a score column on an older schema is no metric.
RecommendationDo not merge this head yet. The read-descriptor behavior matches RFC 0047. Test Workspace on this commit fails one test, This review is a comment. It is not an approval. What this change doesA ranked read now reports the retrievals that produced its order, and the returned columns that hold a score. The planner derives that report from the accepted physical plan (
Legal queries keep the same rows. This change does not write Lance data, and it does not change index use. Lance ANN search stays approximate. A flat scan stays an execution choice. The declared recall follows the kind, so a client does not depend on a guarantee that disappears when an index is built. Blocking
Add one inventory row per occurrence in
The Graph Vocabulary Guard job is skipped on pull requests, and Optional
Tradeoff and liabilityNearest recall stays approximate on the exact overfetch pass. A client must not treat a flat pass as a promise. That promise would disappear when an index is built. This matches RFC 0047 and the Lance alignment fence. The report is one derived view of the plan. There is no second stored description. Query semantics stay in the plan. Index layout stays derived state. The new cost is a stable wire surface. Each later envelope array should use the same door. Derive it from the plan, attach it in A future score carrier that is not a bare property access would omit a metric. A cast, or an alias of a group key, is that shape. It is not a legal return today. T9 refuses a rank expression beside an aggregate. T56 refuses Other open workThis head conflicts with current
On Open PR #1012 also edits ValidationReviewed head: Local, checkout
Result: 1 passed, Local, same checkout,
Result: Exact-commit CI, run 39746533611, Test Workspace job
GQT ordinary, GQT dst, GQT served, and the DST pinned suite passed on this commit. rustfmt and clippy passed. RustFS S3, default and failpoints, passed. I did not finish a local rerun of the engine, server, or CLI integration tests. Those passed on CI for this commit. I did not measure performance. This change does not alter storage traffic, so the DST cost golden is unchanged. I did not run S3 or Azure locally. Regression proofNo GQT before-and-after pair applies. This is a new envelope contract. GQT records rows, counts, result column types, and errors. It has no expectation for Inline:
|
# Conflicts: # openapi.json
RecommendationDo not merge this head yet. The read-descriptor behavior still matches RFC 0047. One OpenAPI naming rule from current main fails on this commit. Rename the four string-enum schemas, regenerate the spec, move the inventory rows, then merge. This review is a comment. It is not an approval. What this change doesA ranked read reports the retrievals that produced its order, and the returned columns that hold a score. The planner derives that report from the accepted physical plan (
Legal queries keep the same rows. This change does not write Lance data. Lance ANN search stays approximate. A scan without an index can score every vector, and this report still says Earlier findings
Blocking
Give each string enum a schema name with no transport suffix. Regenerate The JSON values stay Object schemas keep Merge with mainThe second parent is current
No conflict markers remain. Other open pull requests#1137 and #1139 merge cleanly onto this head. Both use #1012 still conflicts with this head in Tradeoff and liabilityThe report stays one derived view of the accepted plan. There is no second stored description. Query semantics stay in the plan. Index layout stays derived state. Nearest recall stays approximate on a full scan, so a client does not depend on a guarantee that disappears when an index is built. The new cost is a stable wire surface plus the OpenAPI naming rule main now enforces. Each later envelope array should use the same door: derive it from the plan, attach it in This merge closes the vocabulary gap and the two coverage gaps from the previous review. It leaves the enum-name rule that main added after the feature was written. Setting the names before merge avoids a later client rename. Shipping the ValidationReviewed head: Exact-commit CI, run 40101672643:
Local, checkout
Result:
Result: 1 passed. I did not finish a second local run of the api-types, engine, server, or CLI tests. Those descriptor tests passed on CI for this commit. I did not measure performance. This change does not alter storage traffic, so the DST cost golden is unchanged. I did not run S3 or Azure locally. I did not rerun Regression proofNo GQT before-and-after pair applies. This is a new envelope contract. GQT records rows, counts, result column types, errors, and plan facts. It has no expectation for Inline:
|
The four string enums of the read descriptors become RetrievalKind, Recall, MetricKind and CoverageUnknownReason, as the OpenAPI conventions require; their JSON values are unchanged. The vocabulary inventory rows for their descriptions move to the new schema paths.
RecommendationMerge this head. The OpenAPI suffix blocker is resolved. This review finds no new blocker. This review is a comment. It is not a GitHub approval. What this change doesA ranked read reports the retrievals that set its order. It also reports each returned column that holds a score. The planner derives both arrays from the accepted plan. This commit only renames four string-enum schemas. The names are Earlier findings
BlockingNone. OptionalNone. Other open pull requests#1137 and #1139 merge cleanly onto this head. #1139 returns the query-route JSON from the #1150 merges cleanly. It changes the description of #1151 merges cleanly. The merged lock keeps #1012 still conflicts in Tradeoff and liabilityThe report stays one derived view of the accepted plan. This commit adds no second description. It sets the string-enum names before HTTP contract 0.14 ships, so a later client does not rename these schemas. The JSON contract is unchanged. Nearest recall stays Five similar arrays would add wire fields, schema names, and inventory rows. They add no second source of truth while they stay derived from the plan. Each new string enum needs a name with no transport suffix. ValidationReviewed head: The delta is four files: Local, checkout
Result:
Result: 106 passed, 0 failed. This includes A direct read of Exact-commit CI, run 40123494256:
I did not rerun the engine, route, or CLI descriptor tests locally. They passed in the Test Workspace report for this commit. I did not measure performance. This commit does not change storage traffic, so the DST cost golden is unchanged. I did not run S3 or Azure locally. Those CI jobs passed. Regression proofNo GQT before-and-after pair applies. This commit renames schemas. It does not change rows, counts, result column types, errors, or JSON values. GQT records those facts and plan facts. It has no expectation for |
Replace the read descriptors (`retrievals` and `metrics`, with plan-local producer ids, arm references, a per-kind recall label and an always-unknown embedding coverage) with one `ranking` object: the search function the order names, the direction its score ranks, whether this run's search was exhaustive, and the returned columns holding the score. `exhaustive` is a per-run fact. A `nearest` scan records whether its last attempt ranked exactly: proven empty, flat, every partition of the IVF_FLAT index searched, or every admitted row returned, none unreached. `bm25` scores every match. A source guard pins IVF_FLAT as the only vector index the engine builds. RFC 0047 is amended to match.
ragnorc
changed the title
The checkpoint counted itself entered before it marked itself waiting, so a test that waited for `entered()` and then asserted `is_paused()` could see the gap on a busy runner. The first body now claims the pause, marks it waiting and only then entered.
RecommendationReady to merge. This review finds no blocking defect. This review is a comment. It is not a GitHub approval. Why, what, how, impactA caller of a top-k search cannot tell, from the rows alone, whether a nearer or more relevant row was left out, or which returned columns hold the score. This head adds one The planner reads the accepted plan and derives the search, the score columns, and the nearest scans. The run sets
Every vector index this engine builds is IVF_FLAT. Lance scores every vector inside a searched IVF_FLAT partition. Earlier reviewThe review of BlockingNone. Optional
The pause change
Tradeoff and liabilityThe object removes a client-side guess. One response states the search, the better score direction, the exactness of this run, and the score columns. It adds a public field that a client may trust. Five more facts of this kind should extend this one object. The plan derives the shape. The run supplies the evidence. The API renders it. A second object, or a copy stored beside the plan, would drift. Open pull requests that meet this change: #1198 adds a TOON format and conflicts in the read formatter, so that format must carry ValidationReviewed head: GQT cannot express an envelope field. The owner is the existing capped-nearest test. I added one temporary assertion to Base,
Result: FAILED, 1 failed. The rows were Head, I then removed the temporary assertion. The clean test, which already asserts On the head only, I also asserted that Other local commands at the head, each ok with 1 passed:
Commands were CI on this commit, run 40769972614: Test Workspace pass (11m1s), clippy pass, rustfmt pass, GQT ordinary, served, and dst pass, DST pinned pass, RustFS and Azurite pass, LimitationsI did not measure latency or memory. The added value is one small JSON object on a ranked read. I did not run the full workspace suite, S3, or Azure locally. Those jobs passed on this commit in CI. I read Lance 13.0.0 for the partition counters and for exact scores inside a searched IVF_FLAT partition. I did not run a later Lance version. |
Summary
RFC 0047 rollout step 6: a ranked read reports how it ordered its rows.
The read envelope (
POST /query, stored-query calls,query --json, and the--format jsonlmetadata line) gains one additive object, absent when the read's order is no search:by: the search function the order names,nearest,bm25orrrf. Acceptance admits one ranking per read (the leading order key;T33keeps anrrf()arm's own score out ofreturn), so one object describes it.order:ascendingfor anearestdistance,descendingfor abm25or fused score, so a client knows which value is better without knowing each function.exhaustive: what this run did, not a per-function label.truemeans no row the search admitted and did not return outranks a returned one.bm25scores every match, so it is always exhaustive. Anearestscan records whether its last attempt ranked exactly: proven empty, flat (the ladder's choice, the exact overfetch pass, or no index covering the column, which Lance reports by omitting both partition counters), every partition of the index searched, or every admitted row returned, none left unreached at+inf.rrfis exhaustive when itsnearestarms are. A scan the run never reached makes the ranking inexact.score_columns: the returned columns that hold the ranking's own score, in return order. Two columns returning one score both appear; a computed column (bm25(...) > 0.0 as hit) and a legacy stored property named_scoredo not.Why "every partition searched" is exact. Every vector index the engine has ever built is IVF_FLAT (back to the first public commit), which scores every row of a partition it searches exactly. A new source guard,
forbidden_apis.rs::vector_indexes_are_ivf_flat, pins the constructor, so a quantized or HNSW index cannot quietly makeexhaustiveclaim too much.Where it is computed.
omnigraph_planner::planned_rankingderivesby,score_columnsand the rankednearestscans from the accepted plan.engine::executeaddsexhaustiveafter the run from the reports of the pass whose rows it returns, so the ordinary, inspected and replay doors report the same ranking. Replay repeats the run's probe decisions, and the replay test requires the same ranking.This replaces the earlier version of this PR, which followed the RFC's original
retrievals/metricsarrays (plan-local producer ids, arm references, a per-kindrecalllabel, an always-unknownembedding_coverage). The RFC body is amended to match; its decision log names each superseded sentence and gives the reasons: producer ids named nothing a client could address separately, a per-function recall label repeated the function, and coverage was never known.The RFC's rollout table also records steps 3 (#1066) and 4 (#1069) as merged.
Tests
search.rs, on the four-partition IVF_FLAT fixture:issue_567_unfiltered_nearest_keeps_the_probe_cap(a cap of one leaves three partitions unsearched, so the answer is not exhaustive),issue_567_unfiltered_nearest_climbs_the_ladder_to_the_whole_corpus(the last rung searched every partition, so it is),issue_567_ladder_stops_when_the_prefilter_admits_fewer_rows_than_k(a flat scan is exhaustive).engine_v2_plan_replay.rs::a_read_reports_its_ranking: twobm25arms with two columns returning one fused score, anearestwith its distance projected, a mixed fusion, areturn distinctscore key, a computed score, an unranked read; the inspected, ordinary and replayed runs agree. The sharedreplayedhelper requires every replay to report its run's ranking.forbidden_apis.rs::vector_indexes_are_ivf_flat.bound_plan.rs(a fused plan read back reports the same ranking; itsnearestarm is the only scan deciding exhaustiveness; a broken arm is an internal error); in-sourceranking.rs(a stored_scoreproperty beside anearestorder is no score column, andexhaustivefollows the named scan).data_routes.rs::query_envelope_reports_its_ranking(exact wire object, no parameter values, absence on an unranked read); the remotenearesttest checks its ranking;stored_queries.rs::invoke_stored_read_returns_rowscovers the stored-query door.cli_data.rs::ranked_read_reports_its_ranking_in_json_and_jsonl;parity_matrix.rs::parity_ranked_query_ranking; the in-source jsonl test covers presence and absence.read_ranking_renders_and_round_trips(wire shape per function, decode/encode identity for the remote CLI, absence).openapi.jsonregenerated; the vocabulary inventory classifies the new occurrences.Docs
Search guide "What a ranked read reports", the CLI reference's machine-readable positions, the execution guide, the Lance compatibility table's KNN probe budget row (the partition counters now also decide
exhaustive), the RFC 0047 amendment, and a changelog note. The MCPqueryand stored-read tool descriptions now nameranking, which MCP already passes through with the rest of the envelope.