Skip to content

feat: support host filtering and streaming across SINDI modes - #2701

Merged
wxyucs merged 6 commits into
antgroup:mainfrom
Roxanne0321:feat/sindi-host-filter
Sep 2, 2026
Merged

feat: support host filtering and streaming across SINDI modes#2701
wxyucs merged 6 commits into
antgroup:mainfrom
Roxanne0321:feat/sindi-host-filter

Conversation

@Roxanne0321

@Roxanne0321 Roxanne0321 commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR adds host-aware filtering to SINDI and SINDI_V2 across mutable and immutable modes, including mutable Add and the new streaming serialization lifecycle.

Host filtering

  • Accepts dense uint32 host metadata during host-aware Build and Add.
  • Uses host_id = 0 for documents without a host ID; it is indexed and queried like any other host bucket.
  • Has no independent fixed unique-host cap; distinct hosts are naturally bounded by indexed documents, while host-aware document capacity remains uint32_t.
  • Supports host-aware Build and query for mutable and immutable SINDI and SINDI_V2.
  • Supports host-aware Add for mutable SINDI and SINDI_V2, including multiple non-contiguous ranges for the same host.
  • Rejects host-aware Add without complete metadata and prevents switching a host-unaware index to host-aware mode through Add.
  • Keeps queries without a host ID unfiltered.
  • Routes every host-filtered query through inverted-list window scans with exact host-range filtering.
  • Removes host_filter_threshold and the low-cardinality direct or brute-force path without compatibility handling for unpublished threshold-based host indexes.

Streaming serialization

  • Supports SerializeStreaming, DeserializeStreaming, and Index::Load for mutable and immutable SINDI and SINDI_V2.
  • Serializes and restores host metadata.
  • Restores mutable indexes as mutable so they continue to accept Add.
  • Adds a SINDI_V2 term-layout streaming block required to reconstruct mutable and immutable term data cells.
  • Validates required blocks, duplicate blocks, block versions, element counts, host ranges, and manifest consistency.
  • Leaves legacy Serialize and Deserialize unchanged; legacy serialization does not persist host metadata.

Compatibility impact

  • Extends the public Dataset interface with UInt32Metadata and GetUInt32Metadata. Downstream Dataset implementations must add these methods, so this is a source and ABI compatibility change.
  • Updates the in-repository DatasetImpl, mocks, and tests for the new interface.
  • Does not preserve unpublished host_filter_threshold configuration or threshold-based host serialization.
  • Keeps non-host index behavior and legacy index serialization unchanged.

Performance impact

  • Host directory storage scales with the number of unique hosts and ranges rather than the maximum host ID.
  • Host-aware Build and each host-aware Add batch perform an O(N log N) deterministic sort by host ID to create contiguous ranges. This avoids O(max host ID) memory for sparse uint32 host IDs.
  • Mutable Add may append or merge ranges; repeated non-contiguous host batches add one compact range each.
  • Host-filtered queries skip windows in gaps between disjoint mutable ranges, scan only intersecting inverted-list windows, and apply the exact range filter. There is no separate brute-force allocation or scoring path.

Documentation

Updates the English and Chinese SINDI, SINDI_V2, and new-serialization pages. The documentation describes missing-host ID 0, mutable multi-range Add, posting-only host filtering, supported streaming modes, and legacy serialization behavior.

Validation

  • Built the unittests target.
  • Host-filter tests: 356 assertions in 13 test cases.
  • SparseTermComputer tests: 223 assertions in 2 test cases.
  • SINDI and SINDI_V2 parameter tests: 152 assertions in 24 test cases.
  • Clang-format 15 dry run passed for all changed C++ files.
  • Clang-tidy 15 passed for the changed core implementation files.
  • git diff --check passed.

Closes: #2700

Copilot AI lite review requested due to automatic review settings August 17, 2026 04:10
@vsag-bot

vsag-bot commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

/label status/waiting-for-review
/waiting-on reviewer
/request-review @jiaweizone
/request-review @inabao

@Roxanne0321 Roxanne0321 added kind/feature Brand-new functionality or capabilities 引入全新的功能、新特性或新能力 version/1.0 labels Aug 17, 2026 — with ChatGPT Codex Connector
@mergify mergify Bot added module/docs module/api Public C++ API and headers 公共 C++ API 与头文件 labels Aug 17, 2026
@mergify

mergify Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Merge Protections

🟢 All 3 merge protections satisfied — ready to merge.

Show 3 satisfied protections

🟢 Require kind label

  • label~=^kind/

🟢 Require version label

  • label~=^version/

🟢 Require linked issue for feature/bug PRs

  • body~=(?im)(?:^|[\s\-\*])(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?)\s*:?\s+(?:#\d+|[\w.\-]+/[\w.\-]+#\d+|https?://github\.com/[\w.\-]+/[\w.\-]+/issues/\d+)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Warning

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 host-aware routing for immutable DMQ-based SINDI by introducing per-element uint32_t dataset metadata (host_id), persisting it through (legacy + streaming) serialization, and using it to restrict search to host-relevant immutable windows (or direct-DMQ score small hosts).

Changes:

  • Extend Dataset with named uint32_t metadata, including deep-copy/append semantics and ownership-safe destruction.
  • Add host_filter_threshold parameter and implement immutable host-aware search routing (direct scoring for small hosts; window-overlap routing for larger hosts).
  • Add versioned host metadata to legacy + streaming SINDI serialization and document the new behavior (EN/ZH), with new unit tests.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/storage/serialization_tags.h Adds a new streaming block tag for SINDI host metadata.
include/vsag/dataset.h Public API: adds named uint32_t metadata setters/getters to Dataset.
src/dataset_impl.h / src/dataset_impl.cpp Implements storage, deep copy, append, and owner-safe cleanup for uint32_t metadata.
src/dataset_impl_test.cpp Adds unit tests for uint32_t metadata deep copy, append, and missing-metadata errors.
src/algorithm/sindi/sindi_parameter.{h,cpp} Adds host_filter_threshold param with JSON parsing/validation and compatibility rules.
src/algorithm/sindi/sindi_parameter_test.cpp Adds parameter default/compat/boundary tests for host filter threshold.
src/algorithm/sindi/sindi.h / src/algorithm/sindi/sindi.cpp Implements host grouping at build, host-range routing at query, and host metadata (legacy + streaming) serialization.
src/algorithm/sindi/sindi_dmq_test.cpp Adds host-filter routing + serialization tests.
docs/docs/{en,zh}/src/indexes/sindi.md Documents host filtering API/behavior and streaming support for immutable runtime.
docs/docs/{en,zh}/src/advanced/new_serialization.md Documents the new sindi_host_metadata streaming block and immutable support.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/algorithm/sindi/sindi.cpp Outdated
Comment thread src/algorithm/sindi/sindi_parameter.cpp Outdated
Comment thread src/algorithm/sindi/sindi.cpp Outdated
Comment thread src/algorithm/sindi/sindi.cpp Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/algorithm/sindi/sindi.cpp:1995

  • deserialize_host_metadata only checks host_offsets_.back() <= cur_element_count_, which would allow a corrupted/partial host-offset table that fails to cover all inserted documents. Since host metadata implies every successfully inserted doc is assigned to exactly one host range, the final offset should match the element count exactly; otherwise host-scoped queries can silently miss documents.
    CHECK_ARGUMENT(host_offsets_.front() == 0, "serialized SINDI host offsets must start at zero");
    CHECK_ARGUMENT(std::is_sorted(host_offsets_.begin(), host_offsets_.end()),
                   "serialized SINDI host offsets must be ordered");
    CHECK_ARGUMENT(host_offsets_.back() <= static_cast<uint64_t>(cur_element_count_.load()),
                   "serialized SINDI host offsets exceed the element count");

@LHT129 LHT129 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this well-structured PR. The host-aware filtering feature for immutable SINDI is implemented cleanly with comprehensive test coverage and documentation. All previous Copilot review comments have been properly addressed.

Summary of changes reviewed:

  • Host-aware filtering: Documents are grouped by host_id uint32 metadata, with coordinate-compressed host ID mapping for O(log H) query routing. Two search paths: direct DMQ scoring for small hosts (≤ threshold) and window-based search with InnerIdRangeFilter for large hosts.
  • Dataset uint32 metadata: Clean implementation with ownership-safe deep copy, append validation, and proper destructor cleanup.
  • Serialization: Both legacy and streaming paths support host metadata with format versioning and validation.
  • Tests: 3 focused test cases covering direct DMQ path, window path, filter+tombstone, invalid host_ids, serialization round-trip (legacy + streaming), compatibility, and cross-window host spanning.
  • Documentation: English and Chinese docs updated for both the index feature and streaming serialization.

Observations (non-blocking):

  1. host_filter_threshold is always serialized in ToJson() (line 222 of sindi_parameter.cpp), while dmq_shared_codebook_threshold is only serialized when rerank_type == dmq8. The unconditional serialization is harmless (default value on deserialization ensures backward compatibility), but the inconsistency with the existing pattern is worth noting.

  2. The host_ids_.front() != 0 check in deserialize_host_metadata (line 1981 of sindi.cpp) is logically redundant with the subsequent strict-ordering check, though it provides a clearer error message for this specific case.

No critical issues found. The implementation is correct, well-tested, and follows the existing codebase patterns.

@Roxanne0321
Roxanne0321 force-pushed the feat/sindi-host-filter branch from 653fb1c to 5c66d00 Compare August 17, 2026 09:52
Copilot AI review requested due to automatic review settings August 17, 2026 09:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/dataset_impl.cpp:241

  • This destructor path allocates (and may rehash) an std::unordered_set. If that allocation throws, the destructor will call std::terminate (destructors are implicitly noexcept in most cases). Consider avoiding heap allocations in the destructor by using a small inlined container + linear dedup (metadata key count is typically small), or by tracking owned uint32_t* allocations in a member structure during mutation so destruction becomes a simple non-allocating iteration.
        const auto* vector_counts = DatasetImpl::GetVectorCounts();
        allocator_->Deallocate(void_ptr(vector_counts));
        std::unordered_set<const uint32_t*> released_uint32_arrays;
        if (vector_counts != nullptr) {
            released_uint32_arrays.insert(vector_counts);
        }
        for (const auto& [key, value] : this->data_) {
            if (IsUInt32MetadataKey(key)) {
                const auto* values = std::get<const uint32_t*>(value);
                if (values != nullptr && released_uint32_arrays.insert(values).second) {
                    allocator_->Deallocate(void_ptr(values));
                }
            }
        }

Comment thread src/algorithm/sindi/sindi.cpp Outdated

@LHT129 LHT129 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

This PR adds host-aware filtering to immutable SINDI with a clean design:

Architecture

  • Coordinate-compressed host IDs with binary search lookup at query time — memory scales with unique hosts, not max host ID
  • Two-path routing: direct forward-store scoring for small hosts (≤threshold), window-based search for large hosts
  • InnerIdRangeFilter composes host range with user filter cleanly
  • Shared route_host_search used by both KnnSearch and SearchWithRequest

Correctness

  • Proper handling of missing host metadata, invalid host IDs, empty hosts, and tombstone deletions
  • Streaming and legacy serialization both covered with format versioning
  • Host metadata compatibility checked during deserialization (not parameter comparison), avoiding false load failures for non-host indexes

Tests

  • 61 assertions across 3 focused test cases covering direct routing, window routing, filter composition, tombstones, serialization roundtrips, and edge cases

All prior review feedback has been addressed in commit 653fb1ca. No substantive issues found.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Copy Markdown
Collaborator Author

Maintenance update for 04e526b4:

  • Fixed the remaining inline review issue by keeping the immutable-build position loop in int64_t; the host path still narrows only reordered IDs after the existing capacity guard.
  • Diagnosed the prior Test X86 Functests failure as an ASan heap-use-after-free during process-exit destruction: the new metadata-prefix function-local std::string had already been destroyed. Replaced it with non-owning constexpr storage.
  • Replaced the newly added allocating unordered_set metadata dedup in DatasetImpl::~DatasetImpl() with allocation-free linear alias detection, and added aliased metadata ownership coverage.

Local verification: incremental unittests build; [dataset] (99,727 assertions), [SINDIParameter] (86), and [host_filter] (75); clang-format 15; focused clang-tidy 15 for dataset_impl.cpp and sindi.cpp; git diff --check. The new CI run is currently in progress.

Comment thread src/algorithm/sindi/sindi.cpp Outdated
Comment thread src/algorithm/sindi/sindi.cpp Outdated
Comment thread src/algorithm/sindi/sindi.cpp Outdated
Comment thread src/algorithm/sindi_host_filter.cpp Outdated
Comment thread src/algorithm/sindi_host_filter.cpp
Comment thread src/algorithm/sindi_host_filter.cpp
Release host directory storage on reset, improve malformed metadata diagnostics, remove the fixed unique-host cap, skip empty windows between disjoint host ranges, and cover repeated mutable additions and range-aware routing. Keep host search behavior scoped to posting-window filtering, preserve wrapped filter valid-ID bounds, clear stale host state after legacy loads, avoid allocations while releasing Dataset metadata, and keep host-filter tests independent of DMQ-specific coverage.

Signed-off-by: Roxanne0321 <liruoxvan020321@qq.com>
Assisted-by: Codex:gpt-5

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.

Comment thread src/algorithm/sindi_host_filter.cpp

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.

@LHT129 LHT129 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] In SindiHostFilter::Classify (sindi_host_filter.cpp), query_host_id[0] is accessed without verifying that the query dataset has at least one element. If a caller passes a query with NumElements(0) and a non-null host_id metadata pointer (pointing to an empty or undersized buffer), this is undefined behavior. Consider adding query->GetNumElements() == 0 to the early-return guard.

@LHT129 LHT129 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review for PR #2701 (commit 69519e6).

@LHT129 LHT129 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review for PR #2701.

Comment thread src/algorithm/sindi_host_filter.cpp
Comment thread src/algorithm/sindi_host_filter.cpp
Comment thread src/algorithm/sindi_host_filter.cpp
Comment thread src/algorithm/sindi_host_filter.cpp
Comment thread src/algorithm/sindi_host_filter.cpp
Comment thread src/algorithm/sindi_host_filter.cpp
Comment thread src/algorithm/sindi_host_filter.cpp
Comment thread src/algorithm/sindi_host_filter.cpp

@LHT129 LHT129 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

This PR adds host-aware filtering and streaming serialization to both SINDI and SINDI_V2 indexes. The implementation is well-structured with clear separation of concerns: SindiHostFilter handles the host metadata lifecycle, SindiHostBuildPlan manages the build-time reordering, and InnerIdHostFilter provides runtime inner-ID range filtering.

Architecture & Design

The host filter design is sound:

  • Sort-based grouping in PrepareBuild groups documents by host_id before insertion, enabling contiguous inner-ID ranges per host
  • Merge-based CommitBuild correctly handles incremental additions with contiguous range merging
  • Three-route classification (UNFILTERED/EMPTY/WINDOW) cleanly separates the no-filter, no-results, and filtered search paths
  • Window-level optimization via ApplyWindowRoute/NextMatchingWindow/RequiresFullTermScan avoids scanning irrelevant windows and correctly disables term pruning at window boundaries

The SINDI_V2 streaming serialization is a substantial new feature (300+ lines) that follows the same pattern as the existing SINDI streaming code. The block-based format with version checking and critical/non-critical tagging is well-designed for forward compatibility.

Test Coverage

The test suite is thorough:

  • Host filter routing tests for both SINDI and SINDI_V2 (immutable mode)
  • Mutable/immutable/reorder mode combinations for SINDI_V2
  • Window boundary term prune preservation tests
  • Legacy deserialization compatibility (host metadata cleared)
  • Full streaming round-trip tests with block manipulation (erase block, version bump)
  • Dataset UInt32Metadata tests (aliasing, deep copy, append, slice, null)

Issues Raised

I have left 5 inline comments on the PR:

  1. RecordSuccess ordering assumption (line 85)
  2. CommitBuild merge invariant documentation (line 167)
  3. RequiresFullTermScan three-case clarity (line 275)
  4. RecordSuccess bounds check when enabled but empty (line 92)
  5. NextMatchingWindow mid-window range edge case (line 290)

All are suggestions for improving clarity or defensive checks — none are correctness issues.

Overall Assessment

The implementation is correct and well-tested. The host filter integration is cleanly done across both SINDI and SINDI_V2 with minimal code duplication. The streaming serialization for SINDI_V2 is comprehensive and follows established patterns. Ready to merge after addressing the minor suggestions.

@LHT129 LHT129 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

I reviewed the full diff (22 files, +2445/-46) for the host-aware filtering and streaming serialization across SINDI modes. The overall architecture is well-designed: coordinate-compressed host IDs, clean separation of SindiHostFilter/SindiHostBuildPlan/SindiHostSearchRoute, and thorough validation in serialization. All issues from previous review rounds (coordinate compression, host_filter_threshold removal, InnerIdRangeFilterInnerIdHostFilter, search_host_direct removal, uint32_t truncation) have been properly addressed.

Remaining observations

[note] ABI-breaking Dataset interface change

include/vsag/dataset.h: Adding pure virtual methods UInt32Metadata and GetUInt32Metadata breaks ABI compatibility. Any downstream Dataset implementation will fail to compile until it adds stubs for these methods. This was flagged previously and remains present. Consider whether this can be done with a non-virtual helper or free function instead, or document the break clearly in release notes.

[suggestion] CommitBuild merge logic uses double-negative booleans

src/algorithm/sindi_host_filter.cpp lines 786-787: has_existing = not take_added and has_added = not take_existing. When both host_ids are equal, both take_existing and take_added are false, making both has_existing and has_added true — which is correct but the reasoning is indirect. Consider using an explicit equality comparison for the same-host-id case to make the intent clearer.

[note] O(n²) aliasing detection in DatasetImpl destructor

src/dataset_impl.cpp lines 2801-2822: The release_uint32_metadata lambda detects aliased pointers with a nested loop over data_. In practice the number of uint32 metadata keys is very small (typically 1-2), so this is not a real performance concern, but it was flagged in a previous review and remains unchanged.

What looks good

  • Serialization format consistency: WriteVector (uint64_t size + raw data) in Serialize matches the ReadObj + reader.Read pattern in Deserialize. Validation is thorough: strict ordering, disjoint ranges, full element coverage, and bounds checking.
  • Host metadata lifecycle: PrepareBuild correctly rejects host metadata after host-unaware documents and vice versa. Legacy Deserialize clears host metadata in both SINDI and SINDIV2.
  • Streaming block handling: Both SINDI and SINDIV2 properly handle missing/duplicate/unsupported blocks with appropriate error messages. The expects_host_metadata flag from basic_info correctly gates the host metadata block requirement.
  • Term prune control: SetTermPruneEnabled correctly disables pruning for windows that only partially overlap with host ranges, preventing incorrect candidate elimination at window boundaries.
  • Test coverage: 13 host-filter test cases (356 assertions), SINDIV2 streaming roundtrip with mutable/immutable modes, legacy deserialize clears host metadata, streaming validation (missing blocks, version checks), and per-window term prune tests.
  • Documentation: Both English and Chinese docs updated with clear examples and behavioral descriptions.

Comment thread src/algorithm/sindi_host_filter.cpp
Comment thread src/algorithm/sindi_host_filter.h
Comment thread include/vsag/dataset.h
Comment thread src/quantization/sparse_quantization/sparse_term_computer.h
Comment thread src/algorithm/sindi/sindi.cpp
Comment thread src/algorithm/sindi/sindi.cpp
Comment thread src/dataset_impl.cpp
Comment thread src/factory/factory.cpp
Comment thread src/algorithm/sindi_v2/sindi_v2.cpp

@wxyucs wxyucs left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@LHT129 LHT129 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mergify

mergify Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/docs Website and repository documentation 网站与仓库文档 kind/feature Brand-new functionality or capabilities 引入全新的功能、新特性或新能力 module/api Public C++ API and headers 公共 C++ API 与头文件 module/datacell Data cells, vector I/O, and quantization 数据单元、向量 I/O 与量化 module/index Index algorithms and implementations 索引算法与实现 size/XXL version/1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[feat](sindi): add host filtering and streaming support

5 participants