Skip to content

Commit b21617c

Browse files
docs(qdrant): fix multivector README distance claims, round 4
Round 4 found the round-3 README fix was wrong again, in a new way -- third consecutive round the same paragraph has been wrong: - The sentence claimed cosine/angular/euclid all "hard-error at load" via the normalization guard. False for euclid specifically: needs_normalization() only matches cosine/angular, so euclid never trips that guard at all. The only thing that rejects euclid is map_qdrant_distance, called solely from Engine::configure()'s create-collection paths -- which --skip-upload skips entirely. So a euclid-registered multivector dataset run with --skip-upload against an existing collection silently passes both checks, contrary to what the README claimed. Rewrote to separate the two mechanisms: the normalization guard (cosine/angular/omitted, fires on every read path including --skip-upload) from collection-creation-time rejection of any other unrecognized string (configure()-only, bypassed by --skip-upload). - A round-3 fix to the test doc comment replaced a unit error ("docs" vs "KB") with a magnitude error: the fixture is actually ~56 KB (150 docs x mean 6 tokens x 16 dims x 4 bytes), not "well under a KB" -- and ~350x under the 20000 KB threshold, not "several orders of magnitude". Corrected with the actual computed size. - Also: reworded write_multivector_project_with_distance's doc comment, which claimed a test purpose (exercising the guard against cosine) that the actual test doesn't use it for; and added the dot-only-ground-truth caveat directly to the README paragraph, not just to generate_multivector's doc comment, so a reader hits it before registering an l2 dataset. All via 9-way agent consensus, verified live (text-only changes; reran both multivector integration tests to confirm no regression). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 10db083 commit b21617c

3 files changed

Lines changed: 26 additions & 15 deletions

File tree

README.md

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -918,14 +918,21 @@ neighbours.jsonl # ground truth: one JSON array of ids per query line
918918

919919
Register it in `datasets/datasets.json` with `"type": "multivector"` and the
920920
per-token `vector_size`/`distance`. **Only `dot`/`l2` are accepted** (`ip`/
921-
`euclidean` also work, as synonyms) — `cosine`/`angular`/`euclid` (and an
922-
*omitted* `distance`, which defaults to `cosine`) hard-error at load, since
921+
`euclidean` also work, as synonyms). `cosine`/`angular` (and an *omitted*
922+
`distance`, which defaults to `cosine`) are rejected by a normalization guard
923+
that runs on every read path — including under `--skip-upload` — since
923924
neither the reader nor the generator apply per-token normalization yet; this
924925
is the most likely first-attempt failure, since omitting `distance` is common
925-
elsewhere in this file. Ground truth for a multivector dataset MUST
926-
be a genuine brute-force MaxSim ranking, not a heuristic — see
927-
`generate_multivector`'s doc comment in `src/synthetic.rs` for why the hybrid
928-
generator's "planted" shortcut does not carry over. The end-to-end path
926+
elsewhere in this file. Any other string (e.g. a typo'd `euclid`) is rejected
927+
only by Qdrant's collection creation, which `--skip-upload` bypasses entirely
928+
— such a value silently passes through unrejected in that mode. Ground truth
929+
for a multivector dataset MUST be a genuine brute-force MaxSim ranking, not a
930+
heuristic — see `generate_multivector`'s doc comment in `src/synthetic.rs` for
931+
why the hybrid generator's "planted" shortcut does not carry over. That ground
932+
truth is always scored with raw dot-product MaxSim regardless of the declared
933+
`distance` — correct for `dot` (what this repo ships) but silently wrong for
934+
`l2`, which would need its own Euclid-MaxSim ground truth that
935+
`generate_multivector` does not produce. The end-to-end path
929936
(collection creation, ragged upload, and a `query_points` search using the
930937
`"colbert"` vector) is covered by
931938
`tests/integration_qdrant.rs::test_binary_qdrant_multivector`, which also

tests/common/mod.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1734,9 +1734,12 @@ pub fn write_multivector_project(
17341734
}
17351735

17361736
/// Same fixture as [`write_multivector_project`], but with a caller-chosen
1737-
/// `distance` — used to exercise the normalization guard against a
1738-
/// cosine/angular (or omitted) multivector dataset, which this repo does not
1739-
/// yet support (#316 review round 2).
1737+
/// `distance`. NOTE: as of #316, this parameter is only ever exercised with
1738+
/// `"dot"` (via [`write_multivector_project`]) — the live cosine-guard
1739+
/// integration test needs to re-register an ALREADY-UPLOADED corpus under a
1740+
/// different distance mid-test, which this function can't do (it always
1741+
/// allocates a fresh corpus/tempdir), so that test hand-writes its own
1742+
/// second `datasets.json` instead of calling this with `"cosine"`.
17401743
pub fn write_multivector_project_with_distance(
17411744
dataset_name: &str,
17421745
engine_configs_json: &str,

tests/integration_qdrant.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -643,12 +643,13 @@ fn test_binary_qdrant_sparse() {
643643
/// real engine (a "colbert" named vector with `multivector_config`/`MaxSim`,
644644
/// upsert of ragged per-doc token vectors, and a `query_points` search using
645645
/// that named vector), then assert recall against the brute-force MaxSim
646-
/// ranking. `indexing_threshold` is a KB-of-vector-data size, not a doc count —
647-
/// at this fixture's size (150 docs of a handful of 16-dim token vectors each,
648-
/// well under a KB total) Qdrant's collection stays several orders of
649-
/// magnitude under the default HNSW `indexing_threshold` (20000 KB), so no
650-
/// segment is ever indexed and the search is an exact full scan, not ANN —
651-
/// recall must therefore be exact, not merely above a tolerant floor.
646+
/// ranking. `indexing_threshold` is a KB-of-vector-data size, not a doc count
647+
/// — at this fixture's size (150 docs, mean 6 tokens each of 16-dim f32
648+
/// vectors, ≈56 KB of vector data total) Qdrant's collection stays roughly
649+
/// 350x (~2.5 orders of magnitude) under the default HNSW `indexing_threshold`
650+
/// (20000 KB), so no segment is ever indexed and the search is an exact full
651+
/// scan, not ANN — recall must therefore be exact, not merely above a
652+
/// tolerant floor.
652653
#[test]
653654
fn test_binary_qdrant_multivector() {
654655
wait_for_qdrant();

0 commit comments

Comments
 (0)