Skip to content

refactor(hgraph): centralize regular search routing #2606

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
LHT129 wants to merge 1 commit into
base: main
Choose a base branch
from
+531 −238
Open

refactor(hgraph): centralize regular search routing #2606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions src/algorithm/hgraph/hgraph.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ namespace vsag {
class FlattenOptimizedBuildInterface;
class HGraphOptimizedBuildSession;
class IteratorFilterContext;
class ReasoningContext;

/**
* @brief HGraph: hierarchical navigable graph index.
Expand Down Expand Up @@ -805,6 +806,56 @@ class HGraph : public InnerIndexInterface {
bool used_precise_float_csr{false};
};

[[nodiscard]] QueryContext
create_query_context(const SearchRequest& request,
const HGraphSearchParameters& params,
int64_t k,
Comment thread
LHT129 marked this conversation as resolved.
bool use_custom_distance,
SearchStatistics* stats,
std::shared_ptr<ReasoningContext>& reasoning_ctx) const;

void
search_route_graphs(const SearchRequest& request,
const HGraphSearchParameters& params,
InnerIdType entry_point,

@LHT129 LHT129 Aug 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[suggestion] search_route_graphs declares params and use_custom_distance as parameters but neither is used in the function body. This will trigger -Wunused-parameter warnings on strict builds.

Consider removing them, or if they are kept for future use / interface consistency, comment out the parameter names.

bool use_custom_distance,
const void* query,
const VisitedListPtr& visited_list,
QueryContext* ctx,
InnerSearchParam& search_param) const;

static void
configure_bottom_graph_search(const SearchRequest& request,
const HGraphSearchParameters& params,

@LHT129 LHT129 Aug 19, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[note] The QueryContext* ctx parameter in configure_bottom_graph_search is declared but never used in the function body (see hgraph_search.cpp implementation). If it was added for future extensibility, consider adding a comment explaining its intended purpose. Otherwise, it can be removed to keep the interface minimal.

bool is_range,
Comment thread
LHT129 marked this conversation as resolved.
int64_t k,
bool use_custom_distance,
const FilterPtr& filter,
const std::optional<float>& threshold,
QueryContext* ctx,
InnerSearchParam& search_param);

[[nodiscard]] DatasetPtr

@LHT129 LHT129 Aug 20, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[note] configure_bottom_graph_search is declared static in the header (hgraph.h:838) and defined as a static member function, but it mutates ctx->stats->is_timeout (line 576 in the .cpp). While technically valid since ctx is passed as a parameter, the static qualifier is semantically misleading — it suggests the method does not depend on or mutate instance state, yet the stats object it modifies is typically owned by the calling SearchWithRequest instance flow.

Consider removing static and making it a regular const member function. This would make the dependency on ctx (which carries instance-derived state) more explicit and consistent with other helper methods like search_route_graphs and create_query_context which are non-static const members.

pack_search_result(const SearchRequest& request,
int64_t k,
DistHeapPtr search_result,
const QueryContext& ctx,

@LHT129 LHT129 Aug 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[note] The configure_bottom_graph_search function takes a const std::optional<float>& threshold parameter but only uses it for the KNN distance_threshold assignment. The range search branch still reads request.radius_ directly from the request parameter.

This is functionally correct, but the parameter name threshold is slightly misleading since it suggests it applies to both search modes. Consider renaming to distance_threshold to clarify it is KNN-specific.

const MCIHybridSearchResult& mci_result,
const SearchStatistics& stats,
const std::shared_ptr<ReasoningContext>& reasoning_ctx) const;

[[nodiscard]] HGraphSearchParameters
parse_and_validate_search_params(const SearchRequest& request,
bool is_range,
int64_t k,
bool use_custom_distance) const;

[[nodiscard]] std::shared_ptr<ReasoningContext>
initialize_reasoning_context(const SearchRequest& request,
int64_t k,
bool use_custom_distance,
QueryContext* ctx) const;

[[nodiscard]] MCIHybridSearchResult
try_mci_search(const SearchRequest& request,
const HGraphSearchParameters& params,
Expand Down
50 changes: 50 additions & 0 deletions src/algorithm/hgraph/hgraph_mci_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <cmath>
#include <cstdlib>
#include <future>
#include <limits>
#include <numeric>
#include <set>
#include <sstream>
Expand Down Expand Up @@ -83,6 +84,18 @@ class CountingValidIdsFilter : public HalfRatioAllValidFilter {
mutable std::atomic<uint64_t> check_count_{0};
};

class NaNRatioAllValidFilter : public HalfRatioAllValidFilter {
public:
explicit NaNRatioAllValidFilter(const std::vector<int64_t>& ids)
: HalfRatioAllValidFilter(ids) {
}

float
ValidRatio() const override {
return std::numeric_limits<float>::quiet_NaN();
}
};

class CallbackOnlyFilter : public vsag::Filter {
public:
bool
Expand Down Expand Up @@ -281,6 +294,43 @@ TEST_CASE("HGraph companion MCI incrementally updates cliques after Add", "[ut][
REQUIRE(std::stoull(result.value()->GetStatistics({"mci_seed_count"})[0]) ==
expected_seed_count);

result = index.value()->KnnSearch(
query,
3,
R"({"hgraph":{"ef_search":16,"use_mci":true,"mci_seed_ratio":0.5,)"
R"("hgraph_valid_ratio_threshold":1.0,"brute_force_threshold":0.3}})",
filter);
REQUIRE(result.has_value());
REQUIRE(result.value()->GetStatistics({"mci_hybrid_route"})[0] == R"("mci")");

auto nan_ratio_filter = std::make_shared<NaNRatioAllValidFilter>(ids);
result =
index.value()->KnnSearch(query,
3,
R"({"hgraph":{"ef_search":16,"use_mci":true,"mci_seed_ratio":0.5,)"
R"("hgraph_valid_ratio_threshold":1.0}})",
nan_ratio_filter);
REQUIRE(result.has_value());
REQUIRE(result.value()->GetStatistics({"mci_hybrid_route"})[0] == R"("mci")");

result = index.value()->KnnSearch(
query,
3,
R"({"hgraph":{"ef_search":16,"use_mci":true,"mci_seed_ratio":0.5,)"
R"("hgraph_valid_ratio_threshold":1.0,"brute_force_threshold":0.5}})",
filter);
REQUIRE(result.has_value());
REQUIRE(result.value()->GetStatistics({"mci_hybrid_route"})[0] == R"("brute_force")");

result = index.value()->RangeSearch(
Comment thread
LHT129 marked this conversation as resolved.
query,
std::numeric_limits<float>::max(),
R"({"hgraph":{"ef_search":16,"use_mci":true,"mci_seed_ratio":0.5,)"
R"("hgraph_valid_ratio_threshold":1.0}})",
filter);
REQUIRE(result.has_value());
REQUIRE(result.value()->GetStatistics({"mci_hybrid_route"})[0] == R"("hgraph")");

result =
index.value()->KnnSearch(query,
3,
Expand Down
Loading
Loading