refactor(algorithm): extract common search helpers to InnerIndexInterface base class#2139
Hidden character warning
Conversation
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 Require kind labelWonderful, this rule succeeded.
🟢 Require version labelWonderful, this rule succeeded.
|
There was a problem hiding this comment.
Code Review
This pull request refactors search, validation, and serialization logic across several algorithms (including BruteForce, HGraph, IVF, Pyramid, SINDI, and WARP) by consolidating common helper methods into the base class InnerIndexInterface. This significantly reduces code duplication for tasks like filter creation, result packing, argument validation, and index footer handling. The review feedback highlights several critical issues where the return value of read_index_footer is not checked in pyramid.cpp, sindi.cpp, and warp.cpp, which could lead to crashes if a footer is missing. Additionally, a defensive null check is recommended for this->extra_infos_ in inner_index_interface.cpp to prevent potential null pointer dereferences.
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.
There was a problem hiding this comment.
Pull request overview
This PR refactors common search and serialization boilerplate from multiple InnerIndexInterface subclasses into shared protected helper methods on InnerIndexInterface, reducing duplication across algorithms while aiming to keep behavior consistent.
Changes:
- Added reusable helpers to
InnerIndexInterfacefor query validation, filter composition, heap→dataset packing, empty results, and footer read/write. - Updated multiple index implementations (BruteForce, HGraph, IVF, Pyramid, SINDI, WARP) to call the new helpers instead of duplicating logic.
- Standardized result packing and footer serialization/deserialization call sites.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/algorithm/inner_index_interface.h | Declares new protected helper methods for search/filter/result/serialization. |
| src/algorithm/inner_index_interface.cpp | Implements shared helper logic (filters, packing, footer IO, validation). |
| src/algorithm/bruteforce/bruteforce.cpp | Uses shared filter/result packing helpers and footer helpers; centralizes empty-result handling. |
| src/algorithm/hgraph/hgraph_search.cpp | Replaces local validation/filter/result packing with shared helpers (incl. extra-info packing). |
| src/algorithm/ivf/ivf.cpp | Uses shared filter creation and heap→dataset packing in multiple paths. |
| src/algorithm/pyramid/pyramid.cpp | Uses shared filter creation and footer helpers for serialization/deserialization. |
| src/algorithm/sindi/sindi.cpp | Uses shared filter creation, empty-result helper, and footer helpers. |
| src/algorithm/warp/warp.cpp | Uses shared filter creation, heap→dataset packing, and footer helpers. |
Comments suppressed due to low confidence (1)
src/algorithm/sindi/sindi.cpp:508
read_index_footer()returns a bool but its return value is ignored. When a footer is absent,jsonify_basic_info[INDEX_PARAM].GetString()will throw a nlohmann::json exception (not aVsagException). Since this path is gated bydeserialize_without_footer_, it should explicitly require a footer and fail with a controlledVsagException(or skip the compatibility check whenhas_footer == false).
if (not deserialize_without_footer_) {
JsonType jsonify_basic_info;
this->read_index_footer(reader, jsonify_basic_info);
// Check if the index parameter is compatible
{
auto param = jsonify_basic_info[INDEX_PARAM].GetString();
SINDIParameterPtr index_param = std::make_shared<SINDIParameter>();
393da39 to
2837e09
Compare
2837e09 to
e2ea28b
Compare
…face base class Extract repetitive search validation, filter composition, result packing, and serialization footer logic from individual index implementations into protected helper methods in InnerIndexInterface: - validate_knn_args / validate_range_args / validate_search_query: common query dimension, k, radius, and num_elements checks - create_search_filter: composing DeletedIdsFilter with user filter (supports both InnerIdWrapperFilter and ExtraInfoWrapperFilter) - pack_knn_result / pack_knn_result_with_extra_info: DistHeap to Dataset - make_empty_result: empty dataset creation - write_index_footer / read_index_footer: Footer serialization helpers Applied to BruteForce, HGraph, IVF, Pyramid, SINDI, and WARP, eliminating ~254 lines of duplicated boilerplate across 6 index implementations. Signed-off-by: LHT129 <tianlan.lht@antgroup.com>
e2ea28b to
ef51f83
Compare
What problem does this PR solve?
Resolves #2138
Extracts repetitive search validation, filter composition, result packing, and serialization footer logic from individual index implementations into protected helper methods in
InnerIndexInterface.What is changed and how does it work?
New protected helper methods in
InnerIndexInterface:validate_search_query()validate_knn_args()validate_range_args()create_search_filter()pack_knn_result()pack_knn_result_with_extra_info()make_empty_result()write_index_footer()read_index_footer()Applied to 6 index implementations:
Impact:
Related issues
Closes #2138