Skip to content

[MOD-18624] Cover tiered get_vector and read uint8 vectors back - #1049

Merged
dor-forer merged 1 commit into
mainfrom
dor-forer-fix-tiered-get-vector-mod-18624
Sep 15, 2026
Merged

dor-forer merged 1 commit into
mainfrom
dor-forer-fix-tiered-get-vector-mod-18624

Conversation

@dor-forer

@dor-forer dor-forer commented Sep 15, 2026 •

Copy link
Copy Markdown
Collaborator

What

Two independent gaps around the Python get_vector(), both on uncompressed indexes.

1. Tiered get_vector() has no test coverage. The dispatcher used to cast every non brute force index to HNSWIndex, so a tiered index failed the cast and the null result was dereferenced (SIGSEGV). #1045 replaced that cast with a tiered-first one, which fixed the crash but left the behavior untested: every tiered read in the flow tests runs after wait_for_index(), so nothing exercises a read while the vector is still in the flat buffer.

This adds test_get_vector and test_get_vector_multi to tests/flow/test_hnsw_tiered.py. Both read a label back before and after ingestion drains, and assert an absent label is reported as no vectors. The multi case asserts a count range before draining: an ingest job inserts into the backend before removing from the buffer, so a vector caught inside that window is reported by both tiers.

2. getVector() had no uint8 branch. Reading a uint8 vector back raised RuntimeError: Invalid vector data type, even though uint8 is supported everywhere else in the bindings (knn_query, range_query, index construction). Adds the branch, plus a read-back test for int8 and uint8 in tests/flow/test_hnsw.py.

Verification

Built the Python bindings and ran the flow tests on a Linux x86_64 box:

  • Without the uint8 branch: TestUINT8::test_get_vector fails with RuntimeError: Invalid vector data type; TestINT8::test_get_vector passes.
  • With it: both pass.
  • Full tests/flow/test_hnsw_tiered.py: 21 passed.
  • The reported reproducer (tiered get_vector after add_vector + wait_for_index) returns (1, 64) with the inserted values, no SIGSEGV.
  • check-format.sh clean.

Not included

SQ8 tiered coverage. HNSWParams::quantType is not exposed to Python on main, so the SQ8 half of MOD-18624 belongs with #1048, which adds that binding. MOD-18624 stays open until that test lands there.

🤖 Generated with Claude Code


Note

Low Risk
Small binding branch plus test-only coverage; no auth or core query-path logic changes beyond aligning UINT8 with existing INT8 read-back behavior.

Overview
Fixes Python get_vector() for UINT8 indexes and adds flow tests for get_vector on plain HNSW and tiered HNSW.

In bindings.cpp, getVector now handles VecSimType_UINT8 (same pattern as INT8: uint8_t storage returned as float numpy), so UINT8 read-back no longer raises Invalid vector data type.

test_hnsw.py adds a shared get_vector check on the cached L2 index (exact round-trip for int types, empty (0, dim) for missing labels) and wires test_get_vector into TestINT8 and TestUINT8.

test_hnsw_tiered.py adds test_get_vector and test_get_vector_multi, reading labels before wait_for_index() (buffer vs backend) and after drain; the multi test allows up to 2 * per_label rows mid-ingest when both tiers briefly report the same vector.

Reviewed by Cursor Bugbot for commit 2c7378c. Bugbot is set up for automated code reviews on this repo. Configure here.

The Python get_vector() dispatcher used to cast every non brute force index
to HNSWIndex, so a tiered index failed the cast and the null result was
dereferenced. That cast was replaced in #1045, which leaves the behaviour
uncovered by a test: the tiered reads in the flow tests all run after
ingestion has drained, so nothing exercises a read while the vector is still
in the flat buffer.

Add tiered get_vector tests for the single and multi cases that read a label
back both before and after ingestion drains, and assert an absent label is
reported as no vectors. The multi case asserts a count range before draining,
because an ingest job inserts into the backend before removing from the
buffer and a vector caught inside that window is reported by both tiers.

getVector() also had no branch for uint8, so reading a uint8 vector back
raised "Invalid vector data type" even though the type is otherwise
supported. Add the branch, and a read back test for int8 and uint8.

SQ8 tiered coverage is not included: quantType is not exposed to Python on
main, so those tests belong with the bindings that expose it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dor-forer
dor-forer marked this pull request as ready for review September 15, 2026 07:13
@codecov

codecov Bot commented Sep 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.49%. Comparing base (26ffef5) to head (2c7378c).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1049      +/-   ##
==========================================
- Coverage   97.50%   97.49%   -0.02%     
==========================================
  Files         141      141              
  Lines        8851     8851              
==========================================
- Hits         8630     8629       -1     
- Misses        221      222       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dor-forer
dor-forer added this pull request to the merge queue Sep 15, 2026
Merged via the queue into main with commit ec835be Sep 15, 2026
22 checks passed
@dor-forer
dor-forer deleted the dor-forer-fix-tiered-get-vector-mod-18624 branch September 15, 2026 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants