Found while verifying PR #271 (merged as fd92603).
#271 fixed a real silent-wrong class: --skip-upload skips configure(), and four engines initialised search-critical state there, so the reuse path measured under different settings than the upload path. The fix re-derives that state at the top of search():
| engine |
field |
consequence before the fix |
| pgvector |
distance_op |
empty operator → hard SQL error (fail-loud) |
| milvus |
metric_type |
wrong metric in the request body |
| turbopuffer |
distance_metric |
silently wrong — defaults to cosine_distance in new(), so an L2 dataset is measured as cosine with no error (measured: recall 1.000 → 0.775 on a shipped dataset) |
| chroma |
collection_id |
Collection ID is not a valid UUIDv4 |
The gap: none of the four has an automated test asserting the re-derivation matches what configure() computes. Integration tests referencing skip_upload exist only for the redis-wire engines, mongodb and qdrant.
Correctness today rests on the two expressions being textually identical — verified by hand during review (turbopuffer's is byte-identical; pgvector's and milvus's differ only by an inlined temporary). Nothing stops them drifting apart in a later edit, and turbopuffer's drift would be silent.
Why this is cheap: all four are pure functions of the Dataset. A unit test needs no server, no container, and no network — construct the config, call both derivations, assert equality. Something like:
#[test]
fn search_derives_the_same_metric_configure_does() {
for distance in ["cosine", "l2", "dot", "COSINE", "L2"] {
let ds = dataset_declaring(distance);
assert_eq!(configure_side_derivation(&ds), search_side_derivation(&ds), "{distance}");
}
}This is the repo's documented silent-config-drop class ([[silent-config-drop-class]] in the review notes; see also #212, #219, #239): a value that parses cleanly, is then computed twice, and the two copies quietly disagree.
Adjacent, pre-existing, worth fixing in the same change: to_turbopuffer_metric matches lowercase literals with a catch-all _ => "cosine_distance". Unlike the milvus and pgvector mappers it neither lowercases its input nor errors on an unknown value, so a dataset declaring "L2" silently gets cosine. That is identical on both paths today — #271 introduced no divergence — but it is the same failure mode one level down, and the parameterised test above would catch it if the case list includes "L2".
Found while verifying PR #271 (merged as
fd92603).#271 fixed a real silent-wrong class:
--skip-uploadskipsconfigure(), and four engines initialised search-critical state there, so the reuse path measured under different settings than the upload path. The fix re-derives that state at the top ofsearch():distance_opmetric_typedistance_metriccosine_distanceinnew(), so an L2 dataset is measured as cosine with no error (measured: recall 1.000 → 0.775 on a shipped dataset)collection_idCollection ID is not a valid UUIDv4The gap: none of the four has an automated test asserting the re-derivation matches what
configure()computes. Integration tests referencingskip_uploadexist only for the redis-wire engines, mongodb and qdrant.Correctness today rests on the two expressions being textually identical — verified by hand during review (turbopuffer's is byte-identical; pgvector's and milvus's differ only by an inlined temporary). Nothing stops them drifting apart in a later edit, and turbopuffer's drift would be silent.
Why this is cheap: all four are pure functions of the
Dataset. A unit test needs no server, no container, and no network — construct the config, call both derivations, assert equality. Something like:This is the repo's documented silent-config-drop class ([[silent-config-drop-class]] in the review notes; see also #212, #219, #239): a value that parses cleanly, is then computed twice, and the two copies quietly disagree.
Adjacent, pre-existing, worth fixing in the same change:
to_turbopuffer_metricmatches lowercase literals with a catch-all_ => "cosine_distance". Unlike the milvus and pgvector mappers it neither lowercases its input nor errors on an unknown value, so a dataset declaring"L2"silently gets cosine. That is identical on both paths today — #271 introduced no divergence — but it is the same failure mode one level down, and the parameterised test above would catch it if the case list includes"L2".