TreeDB: route RaBitQ search via HNSW pack - #2603
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR removes stats-mode normalization from the RaBitQ prepared HNSW search path. Previously, minimal stats requests were automatically upgraded to full diagnostics; now the caller's ChangesStats Mode Pass-Through
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@codex review |
|
@copilot review |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 328e39a1e0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if err != nil && searchStats.QuantizedScorerActive == 0 && searchStats.QuantizedScoreCalls == 0 { | ||
| p.routeStats.apply(&response.Stats) | ||
| } else { | ||
| vectorIndexSearchRouteStatsForHNSWSearchPackRoute(reader.hnswSearchPack.routeStats(reader.hnswSearchPackStatus, reader.hnswSearchPackOpenNanos)).apply(&response.Stats) |
There was a problem hiding this comment.
Preserve benchmark route contract for RaBitQ
When SearchBenchmarkVector calls Collection.SearchVectorIndexWithBuffer for a rabitq_1bit quantized query, this new branch now returns SearchRouteHNSWSearchPack=1 here; for rerank it also reports NormBytesRead=0 because exact reads come from the pack. The existing validateBenchmarkQuantizedVectorSearchRoute in TreeDB/documentservice/service.go still rejects any quantized response that touches HNSW pack counters and requires nonzero norm reads for rerank, so RaBitQ benchmark requests that previously passed now fail closed with CodeIndexUnavailable. Please either keep the service path on the old column-graph route or update the benchmark validator/diagnostics for the new RaBitQ pack route.
Useful? React with 👍 / 👎.
|
Latest head is 8342f02 (test expectation update only after CI failure); CI is running again. @coderabbitai review |
|
✅ Action performedReview finished.
|
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53bba87cfc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| scorer, err := r.prepareRabitQQuantizedScorer(queryMode, opts.QuantizedIndexName, query, scratch) | ||
| if err != nil { | ||
| recordColumnVectorGraphQuantizedAssetErrorStats(&stats, err) | ||
| return nil, stats, err |
There was a problem hiding this comment.
Preserve query-validation errors before preparing RaBitQ scorer
When the new RaBitQ pack route is selected, malformed queries such as zero/NaN vectors reach prepareRabitQQuantizedScorer, whose EncodeQuery failure is wrapped as ErrVectorIndexSearchUnavailable and then recorded as a quantized asset error here. The previous SearchCosine path validated the query norm first and returned the query-norm error without marking the index asset invalid; in Collection.SearchVectorIndexWithBuffer this new unavailable error also causes the prepared quantized cache to be invalidated for a bad user query. Please validate the query norm before scorer preparation (as the legacy path does) so invalid input does not look like index corruption/unavailability.
Useful? React with 👍 / 👎.
|
Latest head is 08b9e8a; PR body is updated and CI status check rollup is green. @coderabbitai review |
|
✅ Action performedReview finished.
|
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
# Conflicts: # TreeDB/collections/column_vector_graph_rabitq_quantized_asset_test.go
|
Latest head is 21f5766 after merging origin/main; PR body is updated with the current small delta and CI status check rollup is green. @coderabbitai review |
|
✅ Action performedReview finished.
|
|
Codex Review: Didn't find any major issues. You're on a roll. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Objective
Fixes #2587. Route eligible TreeDB
rabitq_1bitquantized_onlyandquantized_reranksearches through the preparedhnsw_search_pack_v1traversal seam with a RaBitQ score plane.Context
RaBitQ was still using the quantized column-graph traversal path after the pack traversal seam landed. This PR reuses that seam for RaBitQ so traversal/scoring counters and exact-read guardrails line up with the prepared pack route while preserving existing RaBitQ v1 scorer semantics.
Current delta against
origin/mainorigin/mainmoved during review and now contains the parallel RaBitQ pack-route implementation from #2606. After merging latest main (ae23f683e), this PR's remaining diff is intentionally small:StatsMode: opts.StatsMode);VectorIndexSearchStatsModeFullDiagnostics;The design/evidence below documents the full #2587 route work and the guardrails this PR preserves.
Non-goals
Design
column_hnsw_rabitq_prepared_search.go.rabitq_1bitv1, stats mode is supported, and thehnsw_search_pack_v1asset is direct/heap healthy.quantized_onlyuses pack traversal with the RaBitQ quantized score plane and keeps exact vector/norm reads at zero.quantized_reranktraverses the full configuredefSearchbreadth with the RaBitQ score plane, trims only the retained shortlist, then exact-reranks that shortlist from prepared pack FP32 vectors.Hard invariant statement: this PR does not intentionally change the RaBitQ v1 durable layout, LSB-first bit order, padding semantics, weighted sign-dot score formula, codec name/version/config identity, durable asset identity, or fail-closed behavior for invalid/unavailable RaBitQ assets.
Correctness / tests
Latest pushed head:
21f57662d(merges latestorigin/mainae23f683e; PR functional product code remains the2cb1baee0RaBitQ route plus the Windows heap-fallback test expectation fix).Local checks on
21f57662d:GitHub status check rollup is green on
21f57662dafter merging latestorigin/main.Coverage added/updated:
SearchCosinescorer.SearchRouteHNSWSearchPack=1/HNSWSearchPackActive=1and avoids column-graph fallback counters.quantized_onlyexact vector/norm reads remain zero.quantized_rerankexact calls/read bytes are limited to the retained shortlist; pack rerank may reportNormBytesRead=0because norms come from prepared pack vectors.Benchmark evidence
Baseline for collected route benchmarks:
origin/main7f6689046. Candidate functional code:2cb1baee0; after #2606 reachedmain, the latest PR diff is the production-stats/test correction described above, so these benchmark rows are retained as route evidence rather than rerun for the now-small delta.Clean hot-cache matrix (
-benchtime=100000x -count=5, same Apple M3 host) showed RaBitQ pack-route counters active and hot rows at0 B/op,0 allocs/op:SearchWithBuffer:quantized_onlyc=1/c=8 andquantized_rerankc=1 were statistically neutral;quantized_rerankc=8 improved6.388µ -> 3.631µ(-43.16%).SearchVectorIndexWithBuffer:quantized_onlyimproved c=126.46µ -> 13.45µ(-49.16%) and c=87.251µ -> 3.989µ(-44.99%);quantized_rerankc=1/c=8 were statistically neutral in that run.hnsw_search_pack_active/search=1,hnsw_search_pack_fallbacks/search=0,quantized_onlynorm_B/search=0, and rerank exact score calls stayed at the configured shortlist.Production-gate matrix (
BenchmarkCollectionVectorQuantizedProductionGate2591,-benchtime=10000x -count=5) also kept hot RaBitQ rows at0 B/op,0 allocs/op; production timings were mixed/noisy after restoring fullefSearchtraversal for rerank correctness, with collectionquantized_rerankc=1 still improving189.8µ -> 91.3µ(-51.9%). Exact-FP32 code is unchanged; exact rows are treated as guardrail/CI coverage rather than promoted as a speedup.Risk notes
Summary by CodeRabbit
Bug Fixes
Tests