Repository navigation
feat: add Hamming distance support for binary vectors - #3262
DoubleCore wants to merge 15 commits into
Conversation
Signed-off-by: DoubleCore <851678303@qq.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Reviewer | Review completeAutomated pull request review completed. Model: No actionable inline findings were found. |
vsag-bot
added
kind/feature
|
Jev-driven classification Applied: Option probabilities
Model: |
Merger | Merge ReadinessHead: Updated: 2026-11-06 17:04:25 UTC Status: BLOCKED Merger decision: HOLD (shadow assessment; no merge executed).
Next owner: Author. Required next step: Resolve the GitHub merge conditions, then reassess. Merge reliability: 0.00 / 1.00 (WITHHELD)
Evidence subtotal: 0.67; applicable ceiling: 0.00. Final score = min(subtotal, ceiling). Policy:
AI review: COMPLETE. Static review: patch is correct; CI is reported separately. Review findings: 0; P0/P1 findings in that review: 0; coverage: 44/44 changed files. Findings are not a count of currently unresolved threads. CI summary: 1 neutral, 5 success. Check source: Next action: Merge commit freshness: Waiting for GitHub to refresh the merge commit for the current head and base. Only current head checks are shown.; PR CI Gate: Waiting for required result from the expected source; Human review: GitHub: REVIEW_REQUIRED; approvals >= 1; Code Owner approval required; stale approvals dismissed; Conversations: 60 unresolved; resolution required; GitHub merge state: BLOCKED CI check details
This is an uncalibrated evidence score, not a probability, correctness guarantee or approval. Zero means the assessment is withheld or lacks positive evidence, not a proven-bad patch. Automatic merging is disabled. New commits or base changes invalidate earlier evidence; never authorize a merge from this score alone. |
Merge Protections🟢 All 3 merge protections satisfied — ready to merge. Show 3 satisfied protections🟢 Require kind label
🟢 Require version label
🟢 Require linked issue for feature/bug PRs
|
mergify
Bot
added
area/examples
vsag-lgtm
left a comment
vsag-lgtm
left a comment
There was a problem hiding this comment.
Automated review @ a81e25c8 — 4 inline suggestion(s).
| BinaryQuantizer::BinaryQuantizer(int dim, Allocator* allocator) | ||
| : Quantizer<BinaryQuantizer>(dim, allocator) { | ||
| this->code_size_ = (static_cast<uint64_t>(dim) + 7U) / 8U; | ||
| this->query_code_size_ = this->code_size_; |
There was a problem hiding this comment.
[critical] References undefined symbols and is not wired into the build.
MetricType::METRIC_TYPE_HAMMING, HammingDistance, and QUANTIZATION_TYPE_VALUE_BINARY are used here but are not declared anywhere in the repository. src/quantization/binary_quantizer.cpp and src/quantization/binary_quantizer_parameter.cpp are also missing from src/quantization/CMakeLists.txt, so they will not be compiled. Before this PR can build, the MetricType enum/value, the HammingDistance implementation, the binary quantization-type constant, the source file registration, and the metric-to-datacell dispatch all need to be added.
| auto base = vsag::Dataset::Make(); | ||
| base->NumElements(num_vectors) | ||
| ->Dim(dim) | ||
| ->Ids(ids.data()) |
There was a problem hiding this comment.
[critical] Calls a public Dataset::BinaryVectors API that does not exist.
include/vsag/dataset.h does not declare a BinaryVectors accessor, so this example cannot compile. Add the public binary-vector dataset interface or remove the example until the interface exists.
| return vsag::Dataset::Make() | ||
| ->NumElements(static_cast<int64_t>(ids.size())) | ||
| ->Dim(dim) | ||
| ->Ids(ids.data()) |
There was a problem hiding this comment.
[critical] Relies on a missing public binary-vector dataset API.
vsag::Dataset::Make()->...->BinaryVectors(...) is not declared in the public Dataset interface. This test will not compile unless the binary-vector API is added.
| ['git', 'status'], | ||
| capture_output=True, | ||
| text=True, | ||
| cwd='C:\\Users\\AORUS\\work\\oss-camp\\vsag-wt-2872' |
There was a problem hiding this comment.
[warning] Accidental local debugging file with an absolute Windows path.
run_git.py, check_git.py, gitstatus.py, run_git.bat, and git_status.txt look like personal worktree helpers and should be removed from the PR.
vsag-bot
left a comment
vsag-bot
left a comment
There was a problem hiding this comment.
Reviewer | Code review
Automated pull request review completed.
Model: gpt-6.1-sol; reasoning effort: high.
Review scope: high (1205 changed lines across 14 files).
Submitted 6 inline comments.
Review coverage checks completed; this does not prove absence of defects.
Static review only; compilation and runtime tests are outside this review's scope.
File coverage: 14/14; observed passing test runs: 0.
Reviewed commit a81e25c.
| FlattenInterfacePtr | ||
| make_flatten_data_cell_hamming(const FlattenInterfaceParamPtr& param, | ||
| const IndexCommonParam& common_param) { | ||
| return MakeFlattenDataCellForMetric<MetricType::METRIC_TYPE_HAMMING>(param, common_param); |
There was a problem hiding this comment.
[P1] [P1] Include the prerequisite Hamming API definitions
src/datacell/CMakeLists.txt automatically includes this translation unit, but MetricType defines only L2SQR, IP, and COSINE. This reference therefore breaks ordinary library builds, including builds with tests disabled. Other required companion definitions are also absent: QUANTIZATION_TYPE_VALUE_BINARY, HammingDistance, and Dataset::BinaryVectors used by the new tests. Include these supporting API and kernel changes in this PR so the added sources have valid definitions.
|
|
||
| namespace vsag { | ||
|
|
||
| BinaryQuantizer::BinaryQuantizer(int dim, Allocator* allocator) |
There was a problem hiding this comment.
[P1] [P1] Register both binary implementation files in the quantizer target
src/quantization/CMakeLists.txt uses an explicit QUANTIZER_SRC list, which includes neither binary_quantizer.cpp nor binary_quantizer_parameter.cpp. Consequently the library contains none of their out-of-line implementations. The new unit test is discovered automatically and references both classes, so after resolving the declaration errors it still cannot link. Add both implementation files to the quantizer target.
| namespace vsag { | ||
|
|
||
| FlattenInterfacePtr | ||
| make_flatten_data_cell_hamming(const FlattenInterfaceParamPtr& param, |
There was a problem hiding this comment.
[P1] [P1] Connect binary support to public index construction
This function has no caller: MakeFlattenDataCell still dispatches only L2, IP, and cosine, and its quantizer factory never selects BinaryQuantizer. Earlier in construction, IndexCommonParam rejects both "binary" and "hamming", while QuantizerParameter rejects "binary". The default mappings and vector-access paths also remain float-oriented. Thus the configurations introduced by the example and functional tests cannot create or use a binary index even after build errors are fixed. Wire the new type through construction, defaults, and packed-vector access.
| base = make_binary_data(num_elements, dim, 11) | ||
| ids = np.arange(num_elements, dtype=np.int64) | ||
| index = pyvsag.Index("brute_force", index_params("brute_force", dim)) | ||
| index.build(vectors=base, ids=ids, num_elements=num_elements, dim=dim) |
There was a problem hiding this comment.
[P2] [P2] Add the packed-binary path to the Python bindings
These tests introduce a Python API contract without implementing it. In python_bindings/index_binding.cpp, parse_dense_vector_kind classifies binary as UNSUPPORTED, Build/Add validate num_elements * dim scalar elements, and searches use the byte-array length as the logical dimension. CalcDistancesById additionally force-casts queries to float32. Even after C++ index creation is enabled, valid uint8 packed inputs remain unusable. Implement binary dtype handling, byte-length validation, and logical-bit dimensions in the bindings.
| bool | ||
| BinaryQuantizer::DecodeOneImpl(const uint8_t* codes, float* data) { | ||
| for (uint64_t d = 0; d < this->dim_; ++d) { | ||
| data[d] = static_cast<float>((codes[d / 8U] >> (d % 8U)) & 1U); |
There was a problem hiding this comment.
[P2] [P2] Preserve the packed representation when decoding
EncodeOne interprets its float pointer as packed bytes, but DecodeOne expands those bytes into dim floating-point values. Re-encoding a decoded vector therefore changes its bits; for example, on little-endian systems an eight-bit code 0x01 becomes 0x00. Existing callers rely on decode/reinsert compatibility: BruteForce::Remove(FORCE_REMOVE) decodes the last vector and reinserts that buffer into the vacated slot. Preserve packed bytes in DecodeOne/DecodeBatch, as the native INT8 quantizer does, or explicitly adapt every caller.
| used_tail_bits = dim % 8 | ||
| if used_tail_bits: | ||
| mask = (1 << used_tail_bits) - 1 | ||
| tail = POPCOUNT[(q[:, None, -1] ^ b[None, :, -1]) & ~mask & 0xFF].sum(axis=2) |
There was a problem hiding this comment.
[P2] [P2] Remove the nonexistent axis from the tail-bit oracle
q[:, None, -1] and b[None, :, -1] broadcast to a two-dimensional (query_count, base_count) array. POPCOUNT indexing preserves that shape, so sum(axis=2) requests a nonexistent axis whenever dim is not divisible by eight. This prevents the 137-bit test and non-aligned parameter cases from checking distances even with a correct implementation. Subtract the per-pair tail popcounts directly.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a81e25c8ae
ℹ️ 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".
| "dtype": "binary", | ||
| "metric_type": "hamming", |
There was a problem hiding this comment.
Any Factory::CreateIndex call using this new configuration is rejected before reaching the binary implementation. Engine::CreateIndex calls IndexCommonParam::CheckAndCreate, but fill_datatype and fill_metrictype in src/index_common_param.cpp still reject "binary" and "hamming", respectively. Consequently, the example and all newly added public-API tests receive INVALID_ARGUMENT instead of an index. Add the new dtype and metric to common-parameter parsing alongside this feature.
Useful? React with 👍 / 👎.
| FlattenInterfacePtr | ||
| make_flatten_data_cell_hamming(const FlattenInterfaceParamPtr& param, | ||
| const IndexCommonParam& common_param) { | ||
| return MakeFlattenDataCellForMetric<MetricType::METRIC_TYPE_HAMMING>(param, common_param); |
There was a problem hiding this comment.
This new helper has no callers in a repository-wide reference search. MakeFlattenDataCell in src/datacell/flatten_datacell_factory.cpp still dispatches only L2/IP/cosine and returns nullptr otherwise; MakeFlattenDataCellInstance in flatten_datacell_factory_impl.h has no binary-quantizer branch, and QuantizerParameter::GetQuantizerParameterByJson still rejects "binary". Therefore, even after common-parameter parsing accepts the new configuration, neither public index can instantiate BinaryQuantizer. Register the new backend in these existing dispatch paths rather than leaving it accessible only through direct construction in unit tests.
Useful? React with 👍 / 👎.
| base = make_binary_data(num_elements, dim, 11) | ||
| ids = np.arange(num_elements, dtype=np.int64) | ||
| index = pyvsag.Index("brute_force", index_params("brute_force", dim)) | ||
| index.build(vectors=base, ids=ids, num_elements=num_elements, dim=dim) |
There was a problem hiding this comment.
The packed np.uint8 inputs introduced here have no corresponding binding implementation. In python_bindings/index_binding.cpp, parse_dense_vector_kind maps "binary" to UNSUPPORTED, and build/add/search reject that kind; their buffer validation also expects dim elements per vector rather than ceil(dim / 8) bytes. Thus fixing C++ index construction alone still leaves every new Python success case unusable. Add a binary binding path that validates packed-byte shapes and passes the bytes through without floating-point conversion, including the distance-by-ID operation.
Useful? React with 👍 / 👎.
| used_tail_bits = dim % 8 | ||
| if used_tail_bits: | ||
| mask = (1 << used_tail_bits) - 1 | ||
| tail = POPCOUNT[(q[:, None, -1] ^ b[None, :, -1]) & ~mask & 0xFF].sum(axis=2) |
There was a problem hiding this comment.
For non-byte-aligned dimensions such as 137 or 9, indexing the final byte makes q[:, None, -1] ^ b[None, :, -1] a two-dimensional (num_queries, num_base_vectors) array. The popcount lookup preserves that shape, so .sum(axis=2) addresses a nonexistent axis and raises AxisError if evaluation reaches the reduction. This prevents the non-aligned-dimension tests from calculating their reference distances even with a working index. Subtract the per-pair tail popcount directly instead of reducing a third dimension.
Useful? React with 👍 / 👎.
|
本轮评审已处理，新提交 已修
未处理（如实说明）
|
vsag-bot
added
area/bindings-python
mergify
Bot
added
the
module/simd
vsag-bot
left a comment
vsag-bot
left a comment
There was a problem hiding this comment.
Reviewer | Code review
Automated pull request review completed.
Review decision: Comment only. No approval was submitted.
Model: gpt-6.1-sol; reasoning effort: high.
Review scope: high (1454 changed lines across 27 files).
Submitted 8 inline comments.
Review coverage checks completed; this does not prove absence of defects.
Static review only; compilation and runtime tests are outside this review's scope.
File coverage: 27/27; observed passing test runs: 0.
Reviewed commit ec53951.
| if (common_param.data_type_ == DataTypes::DATA_TYPE_BINARY) { | ||
| if (not external_param.Contains(HGRAPH_BASE_QUANTIZATION_TYPE)) { | ||
| inner_json[BASE_CODES_KEY][QUANTIZATION_PARAMS_KEY][TYPE_KEY].SetString( | ||
| QUANTIZATION_TYPE_VALUE_BINARY); |
There was a problem hiding this comment.
[P1] [P1] Bypass float-only training sampling for binary HGraph
A binary Build or initial Add exceeding 65,536 rows with default parameters reaches prepare_add_context -> sample_train_dataset -> sample_train_data. That sampler reads GetFloat32Vectors(), which is null for a binary-only dataset, and copies from it, causing undefined behavior before BinaryQuantizer::TrainImpl is reached. A 600-row batch with train_sample_count=512 also triggers this. Skip sampling for this training-free quantizer or make the sampler preserve packed binary storage.
|
|
||
| bool | ||
| BinaryQuantizer::DecodeOneImpl(const uint8_t* codes, float* data) { | ||
| std::memcpy(static_cast<void*>(data), codes, this->code_size_); |
There was a problem hiding this comment.
[P1] [P1] Adapt public vector retrieval to packed binary decoding
Public retrieval still allocates dim floats per row and exposes Float32Vectors, while this decoder initializes only ceil(dim/8) bytes. For example, GetDataByIds on a default 64-bit binary BruteForce index returns a 256-byte float row with only 8 bytes initialized, exposing allocator contents and no BinaryVectors field. HGraph GetRawVectorByIds with store_raw_vector=true has the same mismatch. Adapt retrieval allocation, stride, and dataset field to packed binary output, or reject unsupported retrieval.
| throw VsagException(ErrorType::INVALID_ARGUMENT, | ||
| "Hamming distance requires binary dtype and binary quantization"); | ||
| } | ||
| return MakeFlattenDataCellInstance<BinaryQuantizer, IOTmpl>(param, common_param); |
There was a problem hiding this comment.
[P2] [P2] Handle binary strides in scattered batch insertion
This instantiates FlattenDataCell for binary input, but its BatchInsertVector scattered-ID branch has no DATA_TYPE_BINARY case and retains source_stride=dim*sizeof(float). With dim=64, two packed rows and destination IDs {0,2}, the second row is read at byte 256 instead of byte 8, outside the 16-byte input. Add the packed-byte stride to the inherited batch insertion path.
| HGraph::CheckAndMappingExternalParam(const JsonType& external_param, | ||
| const IndexCommonParam& common_param) { | ||
| auto inner_json = map_hgraph_param(external_param); | ||
| if (common_param.data_type_ == DataTypes::DATA_TYPE_BINARY) { |
There was a problem hiding this comment.
[P2] [P2] Wire binary HGraph updates through native helpers
Binary HGraph advertises SUPPORT_UPDATE_VECTOR_CONCURRENT, but every UpdateVector call reaches get_vectors in utils/util_functions.cpp, which has no binary branch and throws 'no support for this type: 5', including force_update=true. The non-forced path also uses float-only distance overloads. Route binary updates through native extraction/distance paths so a valid packed replacement can actually update the index.
| } else if (datatype == DATATYPE_INT8) { | ||
| result.data_type_ = DataTypes::DATA_TYPE_INT8; | ||
| } else if (datatype == DATATYPE_BINARY) { | ||
| result.data_type_ = DataTypes::DATA_TYPE_BINARY; |
There was a problem hiding this comment.
[P2] [P2] Reject binary configuration for unsupported WARP indexes
CreateIndex("warp", {"dtype":"binary","metric_type":"hamming","dim":64}) now passes common validation and WARP mapping, but make_multi_vector_data_cell_for_io has no Hamming branch and returns nullptr. BruteForce::InitFeatures then dereferences inner_codes_, crashing during index creation. Reject this unsupported combination before construction, or provide a compatible multi-vector datacell.
| if (query->GetInt8Vectors() != nullptr) { | ||
| sub->Int8Vectors(query->GetInt8Vectors() + q * query->GetDim()); | ||
| } | ||
| if (query->GetBinaryVectors() != nullptr) { |
There was a problem hiding this comment.
[P2] [P2] Exclude binary datasets from float-only distance dispatch
The earlier is_float_query predicate excludes int8, float16, sparse, and multi-vector fields but omits GetBinaryVectors(). A dataset containing both BinaryVectors and auxiliary Float32Vectors therefore bypasses this new branch and uses the float-pointer overload. BruteForce computes Hamming distance from the float buffer's bytes, while HGraph rejects the query. Include binary storage in the exclusion or dispatch according to the index datatype.
|
|
||
| auto build_dataset = MakeDataset(base_data, ids, dim); | ||
|
|
||
| auto brute_made = vsag::Factory::CreateIndex("brute_force", params); |
There was a problem hiding this comment.
[P2] [P2] Use valid brute-force parameters for the reference index
The shared params contains index_param.support_remove, which BruteForce::CheckAndMappingExternalParam does not accept. CreateIndex therefore returns INVALID_ARGUMENT and the following REQUIRE fails before any HGraph behavior is exercised. Give the reference index separate parameters without the HGraph-only support_remove key.
| dim = 64 | ||
| index = pyvsag.Index("brute_force", index_params("brute_force", dim)) | ||
| vectors = np.zeros((10, dim // 8), dtype=np.float32) | ||
| with pytest.raises(RuntimeError): |
There was a problem hiding this comment.
[P2] [P2] Expect ValueError from binding input validation
validate_dense_array throws std::invalid_argument for the wrong dtype, shape, length, and contiguity cases used here. Pybind translates that exception to ValueError, so this assertion and the other RuntimeError rejection assertions in this file fail when validation works correctly. Expect ValueError, consistent with the existing dense-input validation tests.
vsag-lgtm
left a comment
vsag-lgtm
left a comment
There was a problem hiding this comment.
Automated review @ ec539511 — 3 inline suggestion(s).
| dim = 64 | ||
| index = pyvsag.Index("brute_force", index_params("brute_force", dim)) | ||
| vectors = np.zeros((10, dim // 8), dtype=np.float32) | ||
| with pytest.raises(RuntimeError): |
There was a problem hiding this comment.
[critical] These reject tests can never pass as written: the binding raises ValueError, not RuntimeError.
validate_dense_array in python_bindings/index_binding.cpp throws std::invalid_argument for dtype/shape/size/contiguity violations, and no custom exception translator is registered in python_bindings/, so pybind11's default translation surfaces it as Python ValueError. The repo's own pre-existing test (tests/python/test_distance_by_id.py:87, merged in #3246) already asserts ValueError for the same validator's errors, which confirms the actually raised type. Consequently all five pytest.raises(RuntimeError) assertions (lines 151, 164, 179, 181, 193) fail with a raised ValueError.
| with pytest.raises(RuntimeError): | |
| with pytest.raises(ValueError): |
The same one-line change applies at lines 164, 179, 181, and 193. Alternatively, pytest.raises((RuntimeError, ValueError)) if the intent is to stay tolerant of error-type mapping.
| for (uint64_t i = 0; i < num_byte; ++i) { | ||
| auto diff = static_cast<uint8_t>(x[i] ^ y[i]); | ||
| while (diff != 0) { | ||
| diff = static_cast<uint8_t>(diff & (diff - 1U)); | ||
| ++distance; | ||
| } | ||
| } |
There was a problem hiding this comment.
[suggestion] This is the hot kernel for every binary Hamming distance computation, but the per-byte bit-clearing loop is scalar and data-dependent (up to 8 iterations per byte even when hardware popcount is available). The codebase already relies on unguarded __builtin_popcountll in src/simd/generic.cpp:1155 and src/simd/fast_bitset.cpp:65, and there is no MSVC path in CI, so the builtin is safe to use here as well:
| for (uint64_t i = 0; i < num_byte; ++i) { | |
| auto diff = static_cast<uint8_t>(x[i] ^ y[i]); | |
| while (diff != 0) { | |
| diff = static_cast<uint8_t>(diff & (diff - 1U)); | |
| ++distance; | |
| } | |
| } | |
| for (uint64_t i = 0; i < num_byte; ++i) { | |
| distance += static_cast<uint64_t>(__builtin_popcount(x[i] ^ y[i])); | |
| } |
For a bigger win, go word-wise: accumulate __builtin_popcountll over 8-byte words loaded via std::memcpy and fall back to per-byte counting for the tail. That is roughly an order of magnitude faster with hardware popcount and follows the pattern generic.cpp already established.
| std::unique_lock global_lock(this->global_mutex_); | ||
| InnerIdType inner_id = this->label_table_->GetIdByLabel(id); | ||
| return this->inner_codes_->UpdateVector(new_base->GetFloat32Vectors(), inner_id); | ||
| return this->inner_codes_->UpdateVector(this->get_data(new_base, 0), inner_id); |
There was a problem hiding this comment.
[suggestion] Now that packed binary bytes are wired through the punned path, UpdateVector and the FORCE_REMOVE decode-reinsert path work, but neither has end-to-end coverage: tests/test_binary_hamming_hgraph.cpp only exercises Remove in the default MARK_REMOVE mode, and no binary test calls UpdateVector. Since correctness of both paths depends on DecodeOneImpl copying raw bytes out and EncodeOneImpl re-masking the tail, a regression test would guard that round-trip: update a binary vector and re-search to verify results reflect the new vector, and/or force-remove an element and verify the reinserted neighbor still matches.
|
Addressed the packed-scratch review in 070d480.
Validation: clang-format 20.1.8 --dry-run --Werror and git diff --check passed. 本地未编译，仅静态核对 (per the local environment restriction). |
vsag-bot
left a comment
vsag-bot
left a comment
There was a problem hiding this comment.
Reviewer | Code review
Automated pull request review completed.
Review decision: Comment only. No approval was submitted.
Model: gpt-6.1-sol; reasoning effort: high.
Review scope: high (2213 changed lines across 37 files).
Submitted 2 inline comments.
Review coverage checks completed; this does not prove absence of defects.
Static review only; compilation and runtime tests are outside this review's scope.
File coverage: 37/37; observed passing test runs: 0.
Reviewed commit 070d480.
| SerializeImpl(StreamWriter& writer) {}; | ||
|
|
||
| void | ||
| DeserializeImpl(StreamReader& reader) {}; |
There was a problem hiding this comment.
[P2] [P2] Restore the query buffer size during deserialization
Quantizer::Deserialize restores dim_ and code_size_, but this empty hook leaves query_code_size_ at its constructor value. Loading a 128-bit binary index into an otherwise compatible index constructed with dim=64 is accepted and restores dim=128; ProcessQueryImpl then allocates only 8 bytes while ComputeDistImpl reads 16 bytes from that buffer. The reverse dimension change overreads the input query during memcpy. Recompute query_code_size_ from the restored code size before processing queries.
There was a problem hiding this comment.
Fixed in 90c468d. BinaryQuantizer::DeserializeImpl now recomputes query_code_size_ from the code_size_ restored by Quantizer::Deserialize, so both dimension growth and shrinkage use the restored query allocation/copy size. The quantizer regression test now starts with a different constructor dimension, checks GetQueryCodeSize(), exercises SetQuery/ComputeDist, and also covers shrinking back to 64 bits. Local C++ compilation was intentionally not run; static symbol checks and targeted clang-format 20.1.8 checks passed.
| auto query = vsag::Dataset::Make(); | ||
| query->NumElements(1)->Dim(to_int64(static_cast<uint64_t>(buf.shape[0])))->Owner(false); | ||
| const uint64_t query_dim = dense_vector_kind_ == DenseVectorKind::BINARY | ||
| ? dim_ |
There was a problem hiding this comment.
[P2] [P2] Refresh the cached binary dimension after loading
Load() lets C++ restore the serialized dimension but never updates the new dim_ cache. For example, construct a binary HGraph with dim=9 and load a compatible dim=10 index: both use two packed bytes, but every query is still tagged Dim(9) here and fails the index's dimension check. Range searches, distance calls, and add validation also use the stale cache. Refresh the cached dimension after successful deserialization.
There was a problem hiding this comment.
Fixed in 90c468d. Load() now checks the deserialization result and, only after success, reads the existing serialized basic_info.dim through the existing Footer::Parse/Metadata APIs and refreshes the binary dim_ cache. The added Python regression loads a 10-bit HGraph into a wrapper constructed with dim=9 and covers KNN, range search, distance-by-ID, and add validation. No new public API or JSON key was introduced. The test file passed py_compile; runtime pytest was not run because no rebuilt pyvsag wheel is available and local C++ compilation is prohibited for this task.
vsag-lgtm
left a comment
vsag-lgtm
left a comment
There was a problem hiding this comment.
Automated review @ 070d480f — 0 inline comments; incremental diff 3fcb3167..070d480f (1 commit, 2 files) has no blocking findings. Approval withheld: earlier review threads remain unresolved.
Signed-off-by: DoubleCore <851678303@qq.com> Assisted-by: OpenCode:ark-code-latest
|
本地验证记录（90c468d333b5532731ee616399a21b1441383f2e）：未运行 CMake、未编译 C++；已完成基于 |
vsag-bot
left a comment
vsag-bot
left a comment
There was a problem hiding this comment.
Reviewer | Code review
Automated pull request review completed.
Review decision: Comment only. No approval was submitted.
Model: gpt-6.1-sol; reasoning effort: high.
Review scope: high (2298 changed lines across 37 files).
Submitted 3 inline comments.
Review coverage checks completed; this does not prove absence of defects.
Static review only; compilation and runtime tests are outside this review's scope.
File coverage: 37/37; observed passing test runs: 0.
Reviewed commit 90c468d.
| return this->inner_codes_->FactoryComputer(&query_multi_vectors[0]); | ||
| } | ||
| return this->inner_codes_->FactoryComputer(query->GetFloat32Vectors()); | ||
| return this->inner_codes_->FactoryComputer(this->get_data(query, 0)); |
There was a problem hiding this comment.
[P2] [P2] Validate binary queries before constructing the computer
For a nonempty 65-bit binary index, a query with Dim(8) and a one-byte buffer reaches BinaryQuantizer::ProcessQueryImpl, which copies nine bytes, before SearchWithRequest calls validate_knn_args. A correctly dimensioned dataset without BinaryVectors similarly passes nullptr to memcpy. Validate the query dimension and native storage before FactoryComputer so malformed C++ queries return INVALID_ARGUMENT instead of causing invalid memory access.
There was a problem hiding this comment.
Fixed in 9f4e047. BruteForce::make_search_computer now rejects a null query, calls validate_search_query for dimension/single-element checks, and requires GetBinaryVectors() before FactoryComputer. Added nonempty 65-bit regression cases for a Dim(8) one-byte query and a correctly dimensioned float-only query; both must return INVALID_ARGUMENT. Local C++ was not compiled; this was checked statically and with clang-format 20.1.8.
| } else if (datatype == DATATYPE_INT8) { | ||
| result.data_type_ = DataTypes::DATA_TYPE_INT8; | ||
| } else if (datatype == DATATYPE_BINARY) { | ||
| result.data_type_ = DataTypes::DATA_TYPE_BINARY; |
There was a problem hiding this comment.
[P2] [P2] Reject binary configurations in unsupported index backends
Accepting binary here also makes CreateIndex("sindi", {"dtype":"binary","metric_type":"hamming","dim":64}) succeed: SINDI's default configuration has no dtype/metric guard. Building it with the newly documented BinaryVectors dataset then dereferences the null GetSparseVectors() result in SINDI::add. Gate binary support in unsupported index creators so these configurations are rejected before construction/build.
There was a problem hiding this comment.
Fixed in 9f4e047. create_registered_index now rejects DATA_TYPE_BINARY for every registered backend except brute_force and hgraph before invoking the creator, so SINDI and other unsupported backends return INVALID_ARGUMENT before construction/build. The regression covers IVF, LazyHGraph, Pyramid, SINDI, SINDI V2, SIMQ, and WARP. No JSON keys were added. Local C++ was not compiled; static and format checks only.
| ids_are_contiguous = idx_vec[i] == idx_vec[0] + i; | ||
| } | ||
| if (not ids_are_contiguous) { | ||
| const auto packed_dim = static_cast<uint64_t>((this->common_param_.dim_ + 7) / 8); |
There was a problem hiding this comment.
[P2] [P2] Derive scattered row strides from the restored quantizer
FlattenDataCell::Deserialize restores the quantizer dimension but leaves common_param_.dim_ at its constructor value. After restoring a 65-bit cell into one constructed for 128 bits, inserting two packed rows with idx_vec={0,2} uses a 16-byte source stride instead of nine bytes; the second insertion reads beyond the 18-byte input and stores incorrect data. Use the restored quantizer's code size, as the binary branch in the base BatchInsertVector already does.
There was a problem hiding this comment.
Fixed in 9f4e047. BinaryFlattenDataCell scattered insertion now derives packed_dim from quantizer_->GetCodeSize(), which reflects the deserialized quantizer, instead of common_param_.dim_. Added a regression that restores a 65-bit stream into a cell constructed for 128 bits, inserts two rows with ids {0,2}, and verifies both packed rows byte-for-byte. Local C++ was not compiled; static and clang-format 20.1.8 checks only.
Signed-off-by: DoubleCore <851678303@qq.com> Assisted-by: OpenCode:ark-code-latest
|
Validation for 9f4e047: per the local environment constraint, I did not run CMake or compile C++. Static symbol checks, git diff --check, and clang-format 20.1.8 checks on the changed regions passed. Python testing was attempted, but tools/dataset_profiler does not exist in this branch and tests/python/test_binary_hamming.py could not collect because pyvsag is not installed. I therefore do not claim a local Python test pass or green CI. The remote DCO check is successful for this head. |
vsag-lgtm
left a comment
vsag-lgtm
left a comment
There was a problem hiding this comment.
Automated incremental review of 070d480f..9f4e0472: both commits' fixes and tests verified correct; 1 inline suggestion.
|
|
||
| void | ||
| DeserializeImpl(StreamReader& reader) { | ||
| this->query_code_size_ = this->code_size_; |
There was a problem hiding this comment.
[suggestion] Consider applying this same query_code_size_ restoration to the sibling quantizers that share this pattern.
This fix is correct and timing-safe: the base Quantizer::Deserialize (src/quantization/quantizer.h:170-177) reads dim_/metric_/code_size_/is_trained_ from the stream before dispatching to DeserializeImpl, and query_code_size_ is never serialized, so re-deriving it from the already-restored code_size_ is exactly right. The new cross-dimension tests (src/quantization/binary_quantizer_test.cpp:246-263) lock this in for BinaryQuantizer.
Two siblings with the same pre-fix pattern are not touched by this PR: their constructors set query_code_size_ = code_size_ (src/quantization/fp32_quantizer.cpp:28; src/quantization/int8_quantizer.cpp:40), but their DeserializeImpl bodies are empty (src/quantization/fp32_quantizer.h:70; src/quantization/int8_quantizer.h:74), so a cross-dimension Deserialize() leaves query_code_size_ at the constructor's value:
FP32Quantizeris the risky one:ProcessQueryImplallocatesquery_code_size_bytes and then writescode_size_/dim_bytes (src/quantization/fp32_quantizer.cpp:148-160), so deserializing onto an instance constructed with a smaller dim produces a heap-buffer overflow — the same bug class fixed here, but an out-of-bounds write rather than a stale-sized buffer.INT8Quantizerusescode_size_/dim_directly at query time (src/quantization/int8_quantizer.cpp:143,149), so it is functionally safe, but the publicGetQueryCodeSize()(src/quantization/quantizer.h:332) keeps reporting the stale value.
Since this PR already establishes "deserialized state must be self-consistent" in several places (the binding Load() dim refresh, the GetCodeSize() scattered-insert stride), adding the same one-line restoration to those two DeserializeImpls — or re-deriving it centrally in the base class — would close the remaining gaps. Fine to defer to a follow-up issue if you want to keep this PR scoped to the binary paths.
There was a problem hiding this comment.
Implemented in 5154c03. FP32Quantizer::DeserializeImpl and INT8Quantizer::DeserializeImpl now restore query_code_size_ from the already-deserialized code_size_. Their serialization tests now deserialize into instances constructed with dim / 2 and assert that GetQueryCodeSize() matches the source; the existing FP32 query-processing coverage therefore exercises the former small-allocation/large-write path. I kept the fix local to these two siblings rather than changing the base class because other quantizers can legitimately have different query and stored-code sizes.
Validation: local C++ compilation and C++ tests were not run (per task constraint). Static symbol checks, clang-format 20.1.8 --dry-run --Werror on all four changed files, and git diff --check passed. This change does not involve Python or tools/dataset_profiler, so Python tests were not run. This reply makes no claim that CI is green.
Restore FP32 and INT8 query_code_size_ from deserialized code_size_ and exercise cross-dimension restoration in their serialization tests. Signed-off-by: DoubleCore <851678303@qq.com> Assisted-by: OpenCode:ark-code-latest
vsag-bot
left a comment
vsag-bot
left a comment
There was a problem hiding this comment.
Reviewer | Code review
Automated pull request review completed.
Review decision: Comment only. No approval was submitted.
Model: gpt-6.1-sol; reasoning effort: high.
Review scope: high (2408 changed lines across 43 files).
Submitted 1 inline comment.
Review coverage checks completed; this does not prove absence of defects.
Static review only; compilation and runtime tests are outside this review's scope.
File coverage: 43/43; observed passing test runs: 0.
Reviewed commit 5154c03.
| const bool packed_binary_rebuild = data_type_ == DataTypes::DATA_TYPE_BINARY and | ||
| binary_tune_source and is_binary_code(new_code); | ||
| const uint64_t insert_buffer_bytes = packed_binary_rebuild | ||
| ? tune_source->GetQuantizerCodeSize() |
There was a problem hiding this comment.
[P1] [P1] Use the restored dimension when rebuilding binary cells
Deserialize a 65-bit HGraph into an instance constructed with dim=128, then call Tune with use_reorder=true. ExportCommonParam still contains the constructor dimension, so Tune creates a 128-bit precise quantizer, while this branch allocates only 12 scratch bytes (the loaded 9-byte code rounded to floats). InsertVector then performs a 16-byte memcpy from that buffer, reading out of bounds; subsequent precise queries also consume too many bytes. Synchronize the common parameters with the restored dim_ before constructing rebuild cells and enforce matching packed widths.
There was a problem hiding this comment.
Fixed in 19ee7fc. Tune now overwrites the exported common_param.dim_ with the restored dim_ before constructing any rebuild cells. Packed binary rebuilds also require both source and destination code widths to equal ceil(dim_ / 8), and the scratch buffer uses the verified destination width. Added a regression that serializes a 65-bit HGraph, restores it into an instance constructed with dim=128, enables reorder, and verifies a precise self-distance. Validation: local C++ was not compiled, per task constraints; I performed static symbol checks, clang-format 20.1.8, and git diff --check only. This does not imply CI is green.
Synchronize Tune common parameters with deserialized dimensions, validate packed source and destination widths, and cover cross-dimension restoration with precise reordering. Signed-off-by: DoubleCore <851678303@qq.com> Assisted-by: OpenCode:ark-code-latest
vsag-bot
left a comment
vsag-bot
left a comment
There was a problem hiding this comment.
Reviewer | Code review
Automated pull request review completed.
Review decision: Comment only. No approval was submitted.
Model: gpt-6.1-sol; reasoning effort: high.
Review scope: high (2463 changed lines across 43 files).
Submitted 3 inline comments.
Review coverage checks completed; this does not prove absence of defects.
Static review only; compilation and runtime tests are outside this review's scope.
File coverage: 43/43; observed passing test runs: 0.
Reviewed commit 19ee7fc.
| if (this->query_code_size_ == 0) { | ||
| return; | ||
| } | ||
| std::memcpy(computer.buf_, static_cast<const void*>(query), this->query_code_size_); |
There was a problem hiding this comment.
[P2] [P2] Reject missing binary query storage before copying
On a nonempty binary HGraph, RangeSearch with NumElements(1), the correct Dim, and only Float32Vectors passes validate_range_args. get_data then returns nullptr, which reaches this memcpy through QueryComputerPool and crashes. Unlike KNN, the range path does not validate matching vector storage. Reject the missing BinaryVectors representation before preprocessing so this returns INVALID_ARGUMENT.
There was a problem hiding this comment.
Fixed in 0d641da. HGraph::SearchWithRequest now checks get_data(query) != nullptr right after validate_range_args on the range path, so a float32-only Dataset on a nonempty binary HGraph is rejected with INVALID_ARGUMENT before preprocessing, using the same "query vector storage must match index data type" message as the KNN path. A regression case builds a dim=65 binary HGraph and asserts RangeSearch returns INVALID_ARGUMENT instead of crashing. Validation: local C++ was not compiled, per task constraints; I performed static symbol checks, clang-format 20.1.8, and git diff --check only. This does not imply CI is green.
| const bool is_float_query = | ||
| query->GetFloat32Vectors() != nullptr && !is_sparse && | ||
| query->GetMultiVectors() == nullptr && query->GetInt8Vectors() == nullptr && | ||
| query->GetBinaryVectors() == nullptr && query->GetFloat16Vectors() == nullptr; |
There was a problem hiding this comment.
[P2] [P2] Validate binary dtype before selecting the float batch path
For a binary BruteForce index, a float32-only Dataset still satisfies is_float_query. CalcDistancesById consequently bypasses the new native Dataset validation and calls BruteForce's float-pointer overload, which interprets the first ceil(dim/8) bytes of the floats as packed bits and returns bogus distances. The equivalent single-ID Dataset call correctly rejects this input. Validate the configured binary representation before taking this fast path.
There was a problem hiding this comment.
Fixed in 0d641da. InnerIndexInterface::CalcDistancesById(const DatasetPtr&...) now validates the configured binary representation before selecting the float batch fast path: a float32-only Dataset against a binary index is rejected with INVALID_ARGUMENT with the same "query vector storage must match index data type" message as the single-ID Dataset path. The guard covers both the topk-allocated fast path and the per-row float path, and the dim=65 binary BruteForce regression asserts both topk=-1 and topk=2 calls return INVALID_ARGUMENT instead of bogus Hamming scores. Validation: local C++ was not compiled, per task constraints; I performed static symbol checks, clang-format 20.1.8, and git diff --check only. This does not imply CI is green.
| CalcDistancesById(const py::array& query, | ||
| const py::array_t<int64_t, py::array::c_style | py::array::forcecast>& ids) { | ||
| auto buf_query = query.request(); | ||
| validate_dense_index_kind(dense_vector_kind_, "calc_distances_by_id"); |
There was a problem hiding this comment.
[P3] [P3] Preserve the existing unsupported-distance exception contract
For a sparse SINDI index, this new guard throws std::invalid_argument, exposed as ValueError, before the existing C++ failure-to-RuntimeError translation. Both distance method names previously raised RuntimeError for this call, as explicitly required by tests/python/test_distance_by_id.py::test_unsupported_dense_representation_is_not_missing. Preserve that exception behavior when adding binary validation.
There was a problem hiding this comment.
Fixed in 0d641da. The binding-level guard on calc_distances_by_id no longer throws std::invalid_argument for an UNSUPPORTED index kind; the allow_unsupported flag lets a sparse SINDI call fall through to the C++ rejection, which the binding translates to RuntimeError with its existing message ("query sparse vectors must not be null"), so both calc_distances_by_id and cal_distance_by_id preserve the exception contract required by tests/python/test_distance_by_id.py::test_unsupported_dense_representation_is_not_missing. The existing tests are untouched. Validation: local C++ was not compiled, per task constraints; I performed static symbol checks, clang-format 20.1.8, and git diff --check only. This does not imply CI is green.
vsag-lgtm
left a comment
vsag-lgtm
left a comment
There was a problem hiding this comment.
Automated review @ 19ee7fc3 — 2 inline comments on the increment 9f4e0472..19ee7fc3 (1 suggestion, 1 note).
| // init new_basic_code obj | ||
| auto common_param = this->basic_flatten_codes_->ExportCommonParam(); | ||
| // Deserialize restores dim_ but leaves the datacell's constructor-time common parameter stale. | ||
| common_param.dim_ = this->dim_; |
There was a problem hiding this comment.
[suggestion] Only dim_ is re-derived from the restored state; metric_ and data_type_ are restored from the payload as well and stay stale here.
deserialize_basic_info overwrites dim_ (hgraph_serialize.cpp:255), metric_ (:258) and data_type_ (:262) from the stream, while ExportCommonParam() returns the construction-time snapshot (flatten_datacell.h:251), so all three fields can disagree with the restored index. In this scope the exported param drives the datacell dispatch (MakeFlattenDataCell selects on common_param.metric_, flatten_datacell_factory.cpp:44) and the quantizer construction, whereas the branch above at line 243 tests this->data_type_. Restoring a binary/Hamming payload into an index created with a different metric or dtype would therefore build the new datacells from the constructor-time values while the rest of Tune follows the payload. Please confirm whether only dim is meant to be allowed to differ; if the other fields can differ too, re-deriving all three keeps the two views consistent.
| common_param.dim_ = this->dim_; | |
| common_param.dim_ = this->dim_; | |
| common_param.metric_ = this->metric_; | |
| common_param.data_type_ = this->data_type_; |
| void | ||
| DeserializeImpl(StreamReader& reader) {}; | ||
| DeserializeImpl(StreamReader& reader) { | ||
| this->query_code_size_ = this->code_size_; |
There was a problem hiding this comment.
[note] For COSINE + hold_molds, query_code_size_ now depends on the construction path for the same logical quantizer.
The parameter constructor sets query_code_size_ to dim * sizeof(float) (fp32_quantizer.cpp:28) before hold_molds adds sizeof(float) to code_size_ (:38), while the restored path now takes the full code_size_. This is safe today: ProcessQueryImpl only writes dim floats for cosine (:157) and ComputeDistImpl reads dim (:172-178), so the extra 4 bytes are just a small over-allocation. Recording it only because GetQueryCodeSize() is now larger after deserialize than after construction; this->dim_ * sizeof(float) would match the constructor exactly. No change required if the over-allocation is intended.
Range search on a binary HGraph accepted a float32-only Dataset and dereferenced a null vector storage during preprocessing; reject the mismatched vector storage with INVALID_ARGUMENT like the KNN path does. Batch CalcDistancesById on an index configured for packed binary vectors misread a float32-only Dataset as raw bits through the native float overload; validate the configured binary representation before selecting the float fast path, matching the single-ID Dataset rejection. The calc_distances_by_id binding no longer short-circuits unsupported (sparse) index kinds with ValueError; the call falls through to C++ so the existing RuntimeError contract is preserved. Regression covers the dense-only range and batch distance rejections. Signed-off-by: DoubleCore <851678303@qq.com> Assisted-by: OpenCode:glm-5.3-flash-free
vsag-lgtm
left a comment
vsag-lgtm
left a comment
There was a problem hiding this comment.
[note] Automated review of increment 19ee7fc3..0d641da4: 1 suggestion, 1 note, no blocking findings.
| // Range search remains single-query only (validate_range_args enforces NumElements==1). | ||
| if (not use_custom_distance) { | ||
| this->validate_range_args(query, request.radius_, request.limited_size_); | ||
| CHECK_ARGUMENT(get_data(query) != nullptr, |
There was a problem hiding this comment.
[note] This storage check runs after a call that already dereferences query.
validate_range_args -> validate_search_query (src/algorithm/inner_index_interface.cpp:1349-1357) evaluates query->GetDim() unconditionally, so a null DatasetPtr is dereferenced before line 723 is reached and the caller gets a crash instead of INVALID_ARGUMENT. Every sibling branch guards the null dataset first: the KNN branch at line 734 (CHECK_ARGUMENT(query != nullptr, "query dataset cannot be null")) and the custom-distance range branch at line 726. This is pre-existing behaviour, not a regression from this commit, but since this block is being hardened it is worth ordering the checks as null dataset -> validate_range_args -> storage, so all range-search callers see a deterministic INVALID_ARGUMENT rather than a segfault.
| REQUIRE_FALSE(range_result.has_value()); | ||
| REQUIRE(range_result.error().type == vsag::ErrorType::INVALID_ARGUMENT); | ||
|
|
||
| auto batch_result = brute->CalcDistancesById(dense_query, ids.data(), count, true, -1); |
There was a problem hiding this comment.
[suggestion] The case name claims HGraph batch-distance coverage, but both batch assertions target brute_force.
CalcDistancesById(const DatasetPtr&, ...) is not overridden by HGraph (only the const float* overload is overridden, src/algorithm/hgraph/hgraph.h:137), so graph->CalcDistancesById(...) does route through the new guard at src/algorithm/inner_index_interface.cpp:535-538. However, this case only exercises the guard through the brute_force handle and through graph->RangeSearch, so the "HGraph ... Batch" half of the title is not actually verified. Adding the same two assertions on graph (identical expectations, topk = -1 and topk = 2) closes that gap, or the title can be narrowed to the paths that are covered.
Check the range-search dataset before validating its fields, and cover null queries plus both HGraph batch-distance guard paths. Signed-off-by: DoubleCore <doublecore@users.noreply.github.com> Assisted-by: OpenCode:ark-code-latest
|
Addressed both vsag-lgtm comments from 2026-11-05T07:37:12Z in de672b7.
Validation on the committed source: clang-format 20.1.8 --dry-run --Werror passed for both changed files; git diff --check HEAD^ HEAD passed; static symbol inspection confirmed the HGraph DatasetPtr batch call routes through InnerIndexInterface's binary guard. Local C++ compilation and test execution were not run, per task constraints. This does not claim CI is green. |
vsag-bot
left a comment
vsag-bot
left a comment
There was a problem hiding this comment.
Reviewer | Code review
Automated pull request review completed.
Review decision: Comment only. No approval was submitted.
Model: gpt-6.1-sol; reasoning effort: high.
Review scope: high (2556 changed lines across 44 files).
Submitted 1 inline comment.
Review coverage checks completed; this does not prove absence of defects.
Static review only; compilation and runtime tests are outside this review's scope.
File coverage: 44/44; observed passing test runs: 0.
Reviewed commit de672b7.
| if (not force_update) { | ||
| std::shared_lock label_lock(this->label_lookup_mutex_); | ||
| auto new_base_query = Dataset::Make(); | ||
| set_dataset(data_type_, dim_, new_base_query, new_base_vec, 1); |
There was a problem hiding this comment.
[P2] [P2] Validate binary replacement dimensions before constructing the query
For a 65-bit binary HGraph, UpdateVector accepts a one-row Dim(8) dataset backed by one packed byte. This call changes its declared dimension to 65, so CalcDistanceById passes validation and BinaryQuantizer::ProcessQueryImpl copies nine bytes from that one-byte buffer. Forced updates likewise reach encoding without checking the original dimension or binary pointer. Validate new_base->GetDim() and GetBinaryVectors() before either update path and return INVALID_ARGUMENT for malformed replacements.
Signed-off-by: DoubleCore <doublecore@users.noreply.github.com> Assisted-by: OpenCode:ark-code-latest
|
Addressed discussion_r4478562734 in commit 562f3a2.
|
vsag-bot
left a comment
vsag-bot
left a comment
There was a problem hiding this comment.
Reviewer | Code review
Automated pull request review completed.
Review decision: Approved. No actionable findings were identified in this complete static review. CI and merge requirements remain separate.
Model: gpt-6.1-sol; reasoning effort: high.
Review scope: high (2627 changed lines across 44 files).
No actionable inline findings were found.
Review coverage checks completed; this does not prove absence of defects.
Static review only; compilation and runtime tests are outside this review's scope.
File coverage: 44/44; observed passing test runs: 0.
Reviewed commit 562f3a2.
vsag-lgtm
left a comment
vsag-lgtm
left a comment
There was a problem hiding this comment.
Automated review @ 562f3a21 — 0 inline findings on the increment 0d641da4..562f3a21 (2 commits, 3 files); both previously raised items are addressed. Approval withheld: earlier review threads remain unresolved.
认领登记：DoubleCore(@DoubleCore)
认领
已在云燕平台完成认领登记。本 issue 的二进制向量 Hamming 距离支持已实现并提交 PR。
已提交 PR
PR: feat: add Hamming distance support for binary vectors
请维护者们评审，谢谢！
Fixes #2872