feat(sindi): add date-window filtering - #2500
Conversation
Signed-off-by: Roxanne0321 <liruoxvan020321@qq.com> Assisted-by: Codex:gpt-5
|
/label status/blocked |
|
Automated pull request review completed. Review effort: Submitted 4 inline comments. |
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
|
Roxanne0321
added
kind/feature
ChatGPT Codex Connector
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces date-window filtering for online queries in the SINDI sparse vector index by adding support for per-element date labels, updating window grouping and metadata management, and implementing date-based pruning during searches. The review feedback identifies a critical performance bottleneck in sindi_analyzer.cpp where a linear search (std::find) is used repeatedly inside a loop, resulting in O(N^2) complexity. The reviewer suggests optimizing this by building a hash map (std::unordered_map) of labels to indices once to achieve O(1) lookups.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| auto base_idx = static_cast<int64_t>(inner_id); | ||
| const auto label = sindi_->label_table_->GetLabelById(inner_id); | ||
| if (base_dataset->GetIds() != nullptr) { | ||
| const auto* begin = base_dataset->GetIds(); | ||
| const auto* end = begin + base_dataset->GetNumElements(); | ||
| const auto* iter = std::find(begin, end, label); | ||
| if (iter == end) { | ||
| return false; | ||
| } | ||
| base_idx = iter - begin; | ||
| } | ||
| copy_sparse_vector(base_dataset->GetSparseVectors()[base_idx], data, allocator); | ||
| return true; |
There was a problem hiding this comment.
Using std::find to perform a linear search over base_dataset->GetIds() introduces an get_original_sparse_vector_by_inner_id. When called repeatedly (e.g., in calculate_ground_truth or get_mean_doc_retained which can perform up to
To optimize this, we should build a hash map (std::unordered_map<int64_t, int64_t>) mapping each label to its corresponding index in base_dataset once, and use it for
| std::vector<InnerIdType> matching_inner_ids; | ||
| matching_inner_ids.reserve(static_cast<uint64_t>(sindi_->cur_element_count_.load())); | ||
| for (uint64_t raw_inner_id = 0; | ||
| raw_inner_id < static_cast<uint64_t>(sindi_->cur_element_count_.load()); | ||
| ++raw_inner_id) { | ||
| const auto inner_id = static_cast<InnerIdType>(raw_inner_id); | ||
| const auto window_id = sindi_->find_window_id(inner_id); | ||
| if (SINDIDateMatches(sindi_->window_metadata_[window_id].date, search_param.date)) { | ||
| matching_inner_ids.push_back(inner_id); | ||
| } | ||
| } | ||
|
|
||
| auto query_count = query_dataset->GetNumElements(); | ||
| auto base_count = base_dataset == nullptr ? sindi_->cur_element_count_.load() | ||
| : base_dataset->GetNumElements(); | ||
| auto effective_topk = std::min<int64_t>(topk, base_count); | ||
| auto effective_topk = std::min<int64_t>(topk, static_cast<int64_t>(matching_inner_ids.size())); | ||
| if (effective_topk <= 0) { | ||
| return nullptr; | ||
| } |
There was a problem hiding this comment.
To avoid the get_original_sparse_vector_by_inner_id during ground truth calculation, we can build a std::unordered_map mapping each label to its corresponding index in base_dataset once at the beginning of calculate_ground_truth.
std::vector<InnerIdType> matching_inner_ids;
matching_inner_ids.reserve(static_cast<uint64_t>(sindi_->cur_element_count_.load()));
for (uint64_t raw_inner_id = 0;
raw_inner_id < static_cast<uint64_t>(sindi_->cur_element_count_.load());
++raw_inner_id) {
const auto inner_id = static_cast<InnerIdType>(raw_inner_id);
const auto window_id = sindi_->find_window_id(inner_id);
if (SINDIDateMatches(sindi_->window_metadata_[window_id].date, search_param.date)) {
matching_inner_ids.push_back(inner_id);
}
}
std::unordered_map<int64_t, int64_t> label_to_base_idx;
if (base_dataset != nullptr && base_dataset->GetIds() != nullptr) {
label_to_base_idx.reserve(base_dataset->GetNumElements());
for (int64_t i = 0; i < base_dataset->GetNumElements(); ++i) {
label_to_base_idx[base_dataset->GetIds()[i]] = i;
}
}
auto query_count = query_dataset->GetNumElements();
auto effective_topk = std::min<int64_t>(topk, static_cast<int64_t>(matching_inner_ids.size()));
if (effective_topk <= 0) {
return nullptr;
}| for (const auto inner_id : matching_inner_ids) { | ||
| SparseVector original{}; | ||
| const SparseVector* base_vector = nullptr; | ||
| if (base_dataset != nullptr && base_dataset->GetSparseVectors() != nullptr) { | ||
| base_vector = base_dataset->GetSparseVectors() + base_idx; | ||
| } else { | ||
| sindi_->GetSparseVectorByInnerId(base_idx, &original, sindi_->allocator_); | ||
| base_vector = &original; | ||
| if (not get_original_sparse_vector_by_inner_id( | ||
| inner_id, &original, sindi_->allocator_, base_dataset)) { | ||
| sindi_->allocator_->Deallocate(ids); | ||
| sindi_->allocator_->Deallocate(distances); | ||
| return nullptr; | ||
| } | ||
|
|
||
| auto label = base_dataset != nullptr && base_dataset->GetIds() != nullptr | ||
| ? base_dataset->GetIds()[base_idx] | ||
| : sindi_->label_table_->GetLabelById(base_idx); | ||
| heap.emplace(get_sparse_distance(query, *base_vector), label); | ||
| auto label = sindi_->label_table_->GetLabelById(inner_id); | ||
| heap.emplace(get_sparse_distance(query, original), label); | ||
| if (static_cast<int64_t>(heap.size()) > effective_topk) { | ||
| heap.pop(); |
There was a problem hiding this comment.
Use the pre-built label_to_base_idx map to retrieve the original sparse vector in
for (const auto inner_id : matching_inner_ids) {
SparseVector original{};
bool success = false;
if (sindi_->use_reorder_) {
sindi_->GetSparseVectorByInnerId(inner_id, &original, sindi_->allocator_);
success = true;
} else if (base_dataset != nullptr && base_dataset->GetSparseVectors() != nullptr) {
auto label = sindi_->label_table_->GetLabelById(inner_id);
int64_t base_idx = inner_id;
if (base_dataset->GetIds() != nullptr) {
auto iter = label_to_base_idx.find(label);
if (iter != label_to_base_idx.end()) {
base_idx = iter->second;
success = true;
}
} else if (inner_id < base_dataset->GetNumElements()) {
success = true;
}
if (success) {
copy_sparse_vector(base_dataset->GetSparseVectors()[base_idx], &original, sindi_->allocator_);
}
}
if (not success) {
sindi_->allocator_->Deallocate(ids);
sindi_->allocator_->Deallocate(distances);
return nullptr;
}
auto label = sindi_->label_table_->GetLabelById(inner_id);
heap.emplace(get_sparse_distance(query, original), label);
if (static_cast<int64_t>(heap.size()) > effective_topk) {
heap.pop();There was a problem hiding this comment.
Pull request overview
Adds optional per-document date labels to Dataset and extends the SINDI sparse index to group documents into date-homogeneous windows, enabling hierarchical (year/month/day) date-window pruning at query time while maintaining legacy serialization compatibility.
Changes:
- Add
Dataset::Dates()/Dataset::GetDates()and propagate date labels through deep copy / append and serialization. - Introduce SINDI window metadata (date, start id, doc count) to support date-grouped windows and date-pruned KNN/range search.
- Update tests, examples, and English/Chinese docs to cover date validation, search semantics, and serialization behavior.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/storage/serialization_tags.h | Adds a new streaming tag for SINDI window metadata blocks. |
| src/inner_string_params.h | Introduces the "date" search parameter key for SINDI. |
| src/dataset_impl.h | Implements Dates() / GetDates() in the concrete dataset implementation. |
| src/dataset_impl.cpp | Ensures date labels are released, deep-copied, and appended with dataset merges. |
| src/dataset_impl_test.cpp | Adds unit tests covering date label set/get, deep copy, and append behavior. |
| src/constants.cpp | Defines the DATASET_DATES internal key string. |
| src/analyzer/sindi_analyzer.h | Updates analyzer ground-truth API to accept search parameters (incl. date selector). |
| src/analyzer/sindi_analyzer.cpp | Applies date-window pruning in analyzer paths and updates ground-truth computation. |
| src/algorithm/sindi/sindi.h | Introduces SINDIWindowMetadata and new metadata/serialization helpers on the index. |
| src/algorithm/sindi/sindi.cpp | Implements date grouping into windows, date-pruned search, and metadata (de)serialization + legacy inference. |
| src/algorithm/sindi/sindi_test.cpp | Adds coverage for date-window search semantics, add behavior, and serialization compatibility. |
| src/algorithm/sindi/sindi_parameter.h | Adds date validation/matching helpers and a date field in SINDISearchParameter. |
| src/algorithm/sindi/sindi_parameter.cpp | Implements date parsing/validation and hierarchical matching; plumbs date through JSON. |
| src/algorithm/sindi/sindi_parameter_test.cpp | Adds tests for date parsing, validation, and hierarchical matching. |
| README.md | Notes SINDI’s new date-window filtering capability at a high level. |
| include/vsag/dataset.h | Adds the public Dataset date-label accessor API. |
| include/vsag/constants.h | Exposes DATASET_DATES as a public constant. |
| examples/cpp/109_index_sindi.cpp | Demonstrates supplying dates on build and using the date search parameter. |
| docs/docs/zh/src/indexes/sindi.md | Documents date-window grouping and query-time date filtering (Chinese). |
| docs/docs/zh/src/api/dataset.md | Documents the new dataset date accessor (Chinese). |
| docs/docs/zh/src/advanced/new_serialization.md | Documents the new optional streaming block and legacy fallback (Chinese). |
| docs/docs/en/src/indexes/sindi.md | Documents date-window grouping and query-time date filtering (English). |
| docs/docs/en/src/api/dataset.md | Documents the new dataset date accessor (English). |
| docs/docs/en/src/advanced/new_serialization.md | Documents the new optional streaming block and legacy fallback (English). |
| if (base_dataset == nullptr || base_dataset->GetSparseVectors() == nullptr || | ||
| inner_id >= base_dataset->GetNumElements()) { | ||
| return false; | ||
| } | ||
| copy_sparse_vector(base_dataset->GetSparseVectors()[inner_id], data, allocator); | ||
| auto base_idx = static_cast<int64_t>(inner_id); |
vsag-bot
left a comment
vsag-bot
left a comment
There was a problem hiding this comment.
Automated inline review completed.
Review effort: high (1253 changed lines across 24 files).
Submitted 4 inline comments.
Reviewed commit 4cc2841.
| * @return DatasetPtr A shared pointer to the dataset with updated date labels. | ||
| */ | ||
| virtual DatasetPtr | ||
| Dates(const std::string* dates) = 0; |
There was a problem hiding this comment.
[P1] Do not insert new virtual slots into the public Dataset ABI
These pure virtual methods are inserted before existing methods such as ExtraInfos, shifting every subsequent vtable slot. An application compiled against the previous header but loading the new library will dispatch those calls to the wrong functions (for example, the old ExtraInfos slot now invokes Dates), while external Dataset subclasses also become source-incompatible. Preserve existing slot ordering, such as by appending an ABI-safe/defaulted extension instead.
| } | ||
|
|
||
| auto inner_id = static_cast<uint16_t>(cur_element_count_ - window_start_id); | ||
| for (const auto& group : date_groups) { |
There was a problem hiding this comment.
[P2] Resolve duplicate labels before regrouping rows by date
Iterating date groups instead of input rows changes the established first-occurrence behavior for duplicate IDs. For rows (B,Z), (A,X), (B,X), the later B/X row is now inserted before the earlier A/X row, so the earlier row is reported as failed and date queries associate X with the wrong date/vector. Determine accepted duplicate rows in original input order before grouping them into windows.
| if (loaded_window_metadata) { | ||
| validate_window_metadata(window_count); | ||
| } else { | ||
| infer_legacy_window_metadata(window_count); |
There was a problem hiding this comment.
[P2] Only infer fixed boundaries for an explicitly legacy stream
A missing or unsupported metadata block is always treated as a legacy stream, even though SINDI_WINDOW_METADATA is marked non-critical and the header does not identify whether variable date windows were written. Removing/skipping this block from a new dated stream therefore loads successfully but reconstructs starts as 0, window_size, ..., causing later postings to be dropped or mapped to wrong labels. Record the window format in the streaming header and require this block for the new format.
| deserialize_window_metadata(reader_ref); | ||
| validate_window_metadata(window_term_list_size); | ||
| } else { | ||
| infer_legacy_window_metadata(window_term_list_size); |
There was a problem hiding this comment.
[P2] Preserve date boundaries in footerless round-trips
With deserialize_without_footer=true, has_window_metadata_format can never be set, so a current index serialized with date windows is always restored through this fixed-size legacy inference instead of reading its appended metadata. Two differently dated documents become short windows at starts 0 and 1, but restoration assigns starts 0 and 10000; date searches return no hits and unfiltered search misses the second document. Either make the body self-describing/read the metadata in this mode or reject footerless loading for date-window storage.
|
/version 1.1 |
Change Type
Linked Issue
What Changed
Dataset::Dates, supportingYYYY,YYYY/MM,and
YYYY/MM/DDcalendar forms.window_size;datasets without date metadata continue to use undated windows.
using AND semantics.
legacy serialized indexes.
reordering, analyzer paths, and public documentation/examples.
Test Evidence
make fmtmake lintmake testmake cov, run tests, and collect coverageTest details:
Compatibility Impact
dateselector issupplied. Missing or empty selectors retain the previous search-all behavior.
Performance and Concurrency Impact
bookkeeping proportional to the document count.
Documentation Impact
README.mdDEVELOPMENT.mdCONTRIBUTING.mdC++ example.
Risk and Rollback
4cc2841ato remove the additive Dataset API and SINDI date-windowbehavior together.
Checklist
kind/bugandkind/feature; see "Linked Issue" above)[skip ci]prefix)