feat(vamana): add optional two-pass graph build - #634
Open
luoxiaojian wants to merge 1 commit into
Open
Conversation
luoxiaojian
requested review from
Cuiyus,
chinaux,
iaojnh,
richyreachy and
zhourrr
as code owners
July 30, 2026 09:23
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds an optional “two-pass” Vamana graph build (initial alpha=1.0 pass + full-graph refine pass at configured alpha), wiring it through params (proto/JSON/Python), engine initialization, and merge/persistence finalization.
Changes:
- Introduces
two_pass_buildin Vamana params across C++/proto/JSON/Python and propagates it into the Vamana streamer. - Adds a
finalize_build()hook to streamers and invokes it duringIndex::Merge, with idempotent finalize behavior for Vamana. - Implements full-graph refine pass (
refine_graph) in the Vamana algorithm and adds tests validating merge finalization + serialization.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/db/index/common/db_proto_converter_test.cc | Adds proto <-> params conversion coverage for two_pass_build. |
| tests/core/interface/index_interface_test.cc | Adds merge/finalize behavior test and JSON serialization assertions for two_pass_build. |
| src/include/zvec/db/index_params.h | Extends VamanaIndexParams with two_pass_build, cloning/equality/string output, and an overload for ordering. |
| src/include/zvec/core/interface/index_param_builders.h | Adds builder support WithTwoPassBuild. |
| src/include/zvec/core/interface/index_param.h | Adds two_pass_build field to core Vamana index params. |
| src/include/zvec/core/framework/index_streamer.h | Adds finalize_build() virtual hook for streamers. |
| src/db/proto/zvec.proto | Adds two_pass_build to VamanaIndexParams protobuf. |
| src/db/index/common/proto_converter.cc | Serializes/deserializes two_pass_build via protobuf converter. |
| src/db/index/column/vector_column/engine_helper.hpp | Propagates DB params into engine builder for two_pass_build. |
| src/core/interface/indexes/vamana_index.cc | Passes two_pass_build into streamer params. |
| src/core/interface/index_param.cc | Adds JSON serialize/deserialize for two_pass_build. |
| src/core/interface/index.cc | Calls streamer_->finalize_build() during merge. |
| src/core/algorithm/vamana/vamana_streamer.h | Declares finalize hook and finalize helpers for two-pass build. |
| src/core/algorithm/vamana/vamana_streamer.cc | Implements idempotent finalize pass, dump-time finalize, and stats attributes. |
| src/core/algorithm/vamana/vamana_params.h | Adds streamer param key for two-pass build enable. |
| src/core/algorithm/vamana/vamana_algorithm.h | Adds refine_graph() and refine_node() API. |
| src/core/algorithm/vamana/vamana_algorithm.cc | Implements full-graph refine pass and candidate merge behavior. |
| src/binding/python/model/param/python_param.cc | Exposes two_pass_build in Python bindings, dict/repr/pickle. |
| python/zvec/model/param/init.pyi | Updates Python type stubs for new param. |
| python/tests/test_vamana.py | Extends Python tests to cover defaults/custom/to_dict/repr/pickle/schema with two_pass_build. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+47
to
+51
| //! Finalize a completed build before persistence. Streamers that need a | ||
| //! whole-index post-build phase can override this hook. | ||
| virtual int finalize_build(void) { | ||
| return 0; | ||
| } |
Comment on lines
+452
to
+476
| ctx->clear(); | ||
| ctx->topk_heap().clear(); | ||
| ctx->topk_heap().limit(entity_.search_list_size()); | ||
| ctx->dist_calculator().clear_compare_cnt(); | ||
| ctx->reset_query(query_vec); | ||
|
|
||
| greedy_search(entry_point, ctx, /*use_pool=*/false); | ||
|
|
||
| TopkHeap &candidates = ctx->topk_heap(); | ||
| const Neighbors current_neighbors = entity_.get_neighbors(id); | ||
| candidates.limit(candidates.size() + current_neighbors.size() + 1); | ||
|
|
||
| const dist_t *cached_dists = entity_.get_neighbor_dists(id); | ||
| for (uint32_t i = 0; i < current_neighbors.size(); ++i) { | ||
| node_id_t neighbor = current_neighbors[i]; | ||
| if (neighbor == id) continue; | ||
| const void *neighbor_vec = entity_.get_vector(neighbor); | ||
| if (neighbor_vec == nullptr) continue; | ||
| dist_t dist = cached_dists | ||
| ? cached_dists[i] | ||
| : ctx->dist_calculator().dist(query_vec, neighbor_vec); | ||
| heap_emplace_unique(candidates, neighbor, dist); | ||
| } | ||
|
|
||
| robust_prune(id, candidates, alpha, entity_.max_degree(), ctx); |
Comment on lines
+24
to
+35
| inline bool heap_contains(const TopkHeap &heap, node_id_t id) { | ||
| for (const auto &item : heap) { | ||
| if (item.first == id) return true; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| inline void heap_emplace_unique(TopkHeap &heap, node_id_t id, dist_t dist) { | ||
| if (!heap_contains(heap, id)) { | ||
| heap.emplace(id, dist); | ||
| } | ||
| } |
Comment on lines
+943
to
+947
| VectorData query{DenseVector{vectors[7].data()}}; | ||
| SearchResult result; | ||
| ASSERT_EQ(0, reopened->Search(query, query_param, &result)); | ||
| ASSERT_FALSE(result.doc_list_.empty()); | ||
| EXPECT_EQ(7U, result.doc_list_[0].key()); |
Comment on lines
739
to
744
| two_pass_build (bool): If True, build the initial graph with alpha=1.0, | ||
| then run one full-graph pass with the configured alpha. Default is | ||
| False. | ||
| quantize_type (QuantizeType): Optional quantization type for vector | ||
| compression (e.g., FP16, INT8). Default is ``QuantizeType.UNDEFINED`` | ||
| to disable quantization. |
Comment on lines
758
to
759
| int search_list_size, float alpha, bool saturate_graph, | ||
| bool use_contiguous_memory, bool use_id_map, |
| bool use_contiguous_memory, bool use_id_map, | ||
| QuantizeType quantize_type, | ||
| QuantizerParam quantizer_param) { | ||
| QuantizerParam quantizer_param, bool two_pass_build) { |
luoxiaojian
force-pushed
the
codex/vamana-two-pass-build
branch
from
July 31, 2026 02:30
01dade9 to
0f17219
Compare
Collaborator
|
CAPI是否也要透出two pass build参数 |
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.
No description provided.