feat(query): sparse query vectors (setSparseVector) (#210) - #244
Merged
Merged
Conversation
There was no way to put a sparse vector into a query. A query built for a sparse field carried a dense payload, upstream converted it into a single 0-index entry, and every document scored 0.0 -- above any -radius threshold, so setRadius() could never exclude anything and reported no error. Silent no-op, and the reason sparse radius queries looked like they were working. setSparseVector(array $indices, array $values) writes a real sparse clause onto the native handle. It clears any dense vector first, because leaving one set is exactly what produced the all-zero scores. The clause is the raw bytes of each array, which is what upstream's own zvec_sub_query_set_sparse_indices/values produce. Verified by calling that upstream C API and dumping the resulting std::string: 12 raw bytes each for three entries, held inline in the string's SSO buffer. Upstream sorts the indices itself and rejects duplicates, so neither is pre-sorted or pre-checked here -- both are asserted instead. Sparse similarity is not normalised the way dense scores are, so a sparse score can be negative and setRadius() compares against it directly. That is what makes the test observable: with scores 0.5 / 0.5 / -0.5 and the IP threshold at -radius, radius 0.25 excludes the negative one. The legacy scalar query() path has no dense vector to work with, so a sparse query delegates to queryVector() rather than failing on an empty float buffer. Two things found while testing, asserted by the test rather than hidden: - the radius threshold DOES leak into later queries on a sparse field within the same process, contradicting the assumption in #210 that HNSW-sparse self-resets. Asserted, not worked around. - resetRadiusThreshold() cannot undo it: it takes a dense vector and fails with "missing query clause" on a sparse field. So the natural reflex when the leak bites does not work here either. Both are follow-ups on #200's thread-local threshold, not on this change, and both are recorded in the test and the changelog. Tests: 214/214, 0 skipped, 0 failed, 2 expected fail.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #210.
Summary
There was no way to put a sparse vector into a query. A query built for a sparse field carried a dense payload, upstream converted it into a single 0-index entry, and every document scored
0.0— above any-radiusthreshold. SosetRadius()on a sparse field was a silent no-op that reported no error, which is why sparse radius queries looked like they were working.setSparseVector()writes a real sparse clause onto the native handle and clears any dense vector first — leaving one set is exactly what produced the all-zero scores.The encoding was determined, not guessed
The clause is the raw bytes of each array, in a
std::stringpair. Upstream's own C API (zvec_sub_query_set_sparse_indices/..._values) does the conversion, and we shiplibzvec_c_api.so, so I verified it rather than guessing:Both strings are held inline in the SSO buffer, confirming raw binary rather than a text encoding.
Upstream sorts the indices itself and rejects duplicates, so neither is pre-sorted or pre-checked here — both are asserted instead.
Why the test is observable
Sparse similarity is not normalised the way dense scores are, so a sparse score can be negative, and
setRadius()compares against it directly. With scores0.5 / 0.5 / -0.5and the IP threshold at-radius, radius0.25excludes the negative one:Before this change the
scores non-zeroandradiuslines are the only ones that could not have passed.Two findings asserted, not hidden
Both contradict assumptions in the issue text, so they are pinned by the test rather than worked around silently:
threshold leaks after radius: yes.resetRadiusThreshold()cannot undo it. It takes a dense vector, so on a sparse field it fails withmissing query clause— the natural reflex when the leak bites does not work either. Asserted asresetRadiusThreshold on sparse: unsupported.Both are follow-ups on #200's thread-local threshold rather than on this change. The test orders its radius-free assertions first and its radius assertions last, precisely so the leak cannot silently change an unrelated expectation.
Also
The legacy scalar
query()path has no dense vector to work with, so a sparse query delegates toqueryVector()rather than failing on an empty float buffer.🤖 Generated with Claude Code