fix(hgraph): prevent stall in low-selectivity filtered KNN (#2974) by thlurte · Pull Request #2983 · antgroup/vsag

thlurte · 2026-09-20T13:51:19Z

What problem does this PR solve?

When a KNN search uses a low-selectivity filter where the total valid candidates in the dataset or reachable component are fewer than ef_search, the searcher loop condition:

if ((-current_first_node_pair.first) > lower_bound && top_candidates->Size() == ef) {
    break;
}

is never met because top_candidates->Size() cannot reach ef.

Moreover, candidate insertion logic (top_candidates->Size() < ef || lower_bound > dist) continually pushed neighbors into candidate_set regardless of distance. This caused the searcher to traverse the entire graph (millions of nodes/edges), stalling queries for minutes at 100% CPU.

What is changed and how it works?

Unrewarded Hops Safeguard:
- Tracks consecutive hops where the candidate distance exceeds lower_bound without adding or improving top_candidates.
- Capped at max_unrewarded_hops = std::max<uint32_t>(ef, 500).
- Resets to 0 whenever a valid candidate is pushed to top_candidates.
Empty Traversal Safeguard:
- When no valid candidates exist yet (top_candidates->Empty()), caps empty exploration at max_empty_hops = std::max<uint32_t>(10000, 10 * ef).
Searcher Coverage:
- Applied across all 3 search_impl overloads in BasicSearcher as well as ParallelSearcher.
Regression Tests:
- Added unit tests in basic_searcher_test.cpp verifying bounded traversal when valid nodes < ef, zero valid nodes, and recall preservation across non-valid intermediate bridge nodes.

Check List

Relevant tests added or updated
All unit & regression tests passing locally
Code style checked with clang-format
DCO Signed-off

…up#2974) Signed-off-by: Ahmed <thlurte@gmail.com>

vsag-bot · 2026-09-20T13:51:24Z

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

mergify · 2026-09-20T13:51:55Z

Merge Protections

🔴 1 of 2 protections blocking · waiting on 🙋 you

	Protection	Waiting on
🔴	Require kind label	🙋 you
🟢	Require version label	—

🔴 Require kind label

Waiting for

label~=^kind/

This rule is failing.

label~=^kind/

Show 1 satisfied protection

🟢 Require version label

label~=^version/

LHT129 · 2026-09-20T13:54:31Z

@@ -253,6 +279,7 @@ BasicSearcher::search_impl(const GraphInterfacePtr& graph,
                if (check_func(id) and


[suggestion] The empty_hops counter is never reset to 0, unlike unrewarded_hops which is reset whenever a valid candidate is pushed to top_candidates. While this is safe in practice because top_candidates is monotonic (only grows, never shrinks during the main loop), it creates a latent fragility: if future changes ever remove entries from top_candidates mid-loop, empty_hops could carry stale state. Consider resetting empty_hops = 0 alongside unrewarded_hops = 0 for consistency and defense-in-depth.

This applies to all 3 search_impl overloads in basic_searcher.cpp and the one in parallel_searcher.cpp.

Addressed in 7c8e6d0: reset empty_hops = 0 alongside unrewarded_hops = 0 across all BasicSearcher overloads and ParallelSearcher.

LHT129 · 2026-09-20T13:54:43Z

+                }
+            } else if (-candidate_set->Top().first > lower_bound) {
+                if (top_candidates->Size() >= ef) {
+                    if (reasoning != nullptr) {


[note] When the empty-hops safeguard triggers (empty_hops >= max_empty_hops), the termination reason is recorded as kTerminationHopsLimitReached, which is the same reason used when hops >= inner_search_param.hops_limit. These are semantically different conditions: one means "no valid candidates found after extensive exploration", the other means "the absolute hop budget was exhausted". If downstream consumers of the reasoning report need to distinguish these cases, consider introducing a distinct termination reason (e.g. kTerminationNoValidCandidates).

This applies to all 3 search_impl overloads in basic_searcher.cpp.

LHT129 · 2026-09-20T13:54:53Z

@@ -258,8 +262,21 @@ ParallelSearcher::search_impl(const GraphInterfacePtr& graph,
        node_pair[0] = current_first_node_pair;


[note] Unlike the basic_searcher.cpp overloads, the parallel_searcher.cpp safeguard paths do not record a termination reason via reasoning->SetTermination(...). This appears to be because ParallelSearcher::search_impl does not have a reasoning variable wired up from QueryContext. If reasoning support is added to the parallel searcher in the future, these new termination paths should also record their reason.

This applies to the empty_hops >= max_empty_hops and unrewarded_hops >= max_unrewarded_hops break paths.

LHT129 · 2026-09-20T13:54:54Z

+    uint32_t unrewarded_hops = 0;
+    uint32_t empty_hops = 0;
+    const uint32_t max_unrewarded_hops = std::max<uint32_t>(ef, 500);
+    const uint32_t max_empty_hops = std::max<uint32_t>(10000, 10 * ef);


[suggestion] The new safeguard logic in ParallelSearcher::search_impl does not report termination reasons via ReasoningContext, unlike the equivalent changes in BasicSearcher::search_impl.

In basic_searcher.cpp, both the max_empty_hops and max_unrewarded_hops break paths call reasoning->SetTermination(...) before breaking. The ParallelSearcher lacks a reasoning variable entirely in this scope, so these termination paths silently break without recording the reason.

This creates an inconsistency in the reasoning/debugging output between the basic and parallel searchers. Consider either:

Adding a reasoning variable (derived from ctx) and calling SetTermination in the break paths, or

At minimum, documenting why the parallel searcher intentionally omits this.

The reasoning variable can be obtained the same way as in BasicSearcher:

auto* reasoning = ctx == nullptr ? nullptr : ctx->reasoning_ctx;

LHT129 · 2026-09-20T13:55:03Z

-                break;
+            if (top_candidates->Empty()) {
+                ++empty_hops;
+                if (empty_hops >= max_empty_hops) {


[note] The max_empty_hops safeguard uses kTerminationHopsLimitReached as the termination reason, but this is semantically different from the actual hops_limit check at the top of the loop.

When empty_hops >= max_empty_hops triggers, the traversal was not terminated by the user-configured hops_limit — it was terminated by the new empty-traversal safeguard. Using the same reason string for both makes it harder to distinguish between these two termination paths in reasoning/debugging output.

Consider adding a dedicated constant (e.g., kTerminationEmptyTraversalLimitReached) or a more specific reason string to differentiate this case from a genuine hops-limit termination.

thlurte · 2026-09-20T13:57:20Z

/label kind/bug

Reset empty_hops alongside unrewarded_hops whenever a valid candidate is pushed to top_candidates across BasicSearcher and ParallelSearcher for defense-in-depth. Signed-off-by: Ahmed <thlurte@gmail.com>

Add kTerminationEmptyTraversalLimitReached constant to differentiate between the user-configured hops_limit and empty traversal safeguard termination in reasoning reports. Signed-off-by: Ahmed <thlurte@gmail.com>

Derive reasoning from QueryContext in ParallelSearcher and record termination reasons for empty traversal and lower bound early breaks. Signed-off-by: Ahmed <thlurte@gmail.com>

LHT129

Reviewed the updated commits (e54b5e4). All issues raised in the previous review round have been addressed:

empty_hops reset: empty_hops = 0 is now consistently reset alongside unrewarded_hops = 0 across all BasicSearcher overloads and ParallelSearcher.
Distinct termination reason: The empty-traversal safeguard now uses the dedicated kTerminationEmptyTraversalLimitReached constant instead of reusing kTerminationHopsLimitReached.
Parallel searcher reasoning: ParallelSearcher::search_impl now derives reasoning from QueryContext and records termination reasons in both safeguard break paths, consistent with BasicSearcher.

The core logic — unrewarded hops safeguard and empty traversal safeguard — is correctly implemented across all 3 BasicSearcher::search_impl overloads and ParallelSearcher::search_impl. The three new test cases provide good coverage for the low-selectivity, zero-candidate, and bridge-node scenarios.

No new issues found.

LHT129

Reviewed the latest commit e54b5e4. All issues from the previous review round have been addressed:

empty_hops is now reset to 0 alongside unrewarded_hops in all code paths across all 3 BasicSearcher overloads and ParallelSearcher
A dedicated kTerminationEmptyTraversalLimitReached constant was added to ReasoningContext, replacing the previous reuse of kTerminationHopsLimitReached for the empty-traversal safeguard
ParallelSearcher now derives reasoning from QueryContext and records termination reasons in both the max_empty_hops and max_unrewarded_hops break paths

The safeguard logic is consistently applied, the test coverage is thorough (bounded traversal, zero candidates, recall across bridge nodes), and I see no remaining correctness or style issues.

LHT129

Reviewed commit e54b5e4. All issues from the previous review round have been addressed:

empty_hops is consistently reset to 0 alongside unrewarded_hops across all BasicSearcher overloads and ParallelSearcher
The dedicated kTerminationEmptyTraversalLimitReached constant is used for the empty-traversal safeguard
ParallelSearcher now derives reasoning from QueryContext and records termination reasons in both safeguard break paths

The safeguard logic (unrewarded hops + empty traversal) is correctly and consistently applied across all 3 BasicSearcher::search_impl overloads and ParallelSearcher::search_impl. The three new test cases provide good coverage for the low-selectivity, zero-candidate, and bridge-node scenarios.

No new issues found.

thlurte · 2026-10-03T17:36:49Z

Hi @LHT129 @wxyucs, friendly ping to see if you could help assign the necessary labels / trigger the CI workflow for this PR when you have a moment. Thank you!

LHT129

Reviewed the latest version (e54b5e4). All issues raised in the previous review round have been addressed:

empty_hops reset: Now consistently reset to 0 alongside unrewarded_hops across all BasicSearcher overloads and ParallelSearcher.
Dedicated termination reason: kTerminationEmptyTraversalLimitReached has been added to ReasoningContext and is used in all empty-traversal break paths.
ParallelSearcher reasoning: reasoning is now wired up from QueryContext and SetTermination is called in both safeguard break paths.

The safeguard logic is consistently applied across all three BasicSearcher::search_impl overloads and ParallelSearcher::search_impl. The regression tests cover the key scenarios: valid candidates < ef, zero valid candidates, and recall preservation across non-valid bridge nodes. No new issues found.

LHT129

Review Summary

I have reviewed the changes in commit e54b5e43 across all 4 files. This PR adds stall-prevention safeguards for low-selectivity filtered KNN searches in HGraph.

What was checked

Correctness: The two-tier safeguard logic (unrewarded hops + empty traversal) is sound. The termination conditions are correctly placed before the expensive graph traversal work in each loop iteration. The unrewarded_hops reset in the else branch (based on distance promise) combined with the reset-on-push (based on actual filter pass) provides a reasonable balance between exploration and early termination.
Consistency: All 3 BasicSearcher::search_impl overloads and ParallelSearcher::search_impl apply the same safeguard pattern with the same threshold constants. The reasoning context is properly wired up in all paths.
Previous comments addressed: All 5 review comments from LHT129 (dated 2026-09-20) have been resolved — empty_hops is now reset, a dedicated kTerminationEmptyTraversalLimitReached constant is used, and parallel_searcher now records termination reasons via ReasoningContext.
Test coverage: 3 new test cases cover: valid candidates < ef (unrewarded hops bound), zero valid candidates (empty traversal bound), and recall preservation across non-valid bridge nodes. Tests verify both result correctness and traversal boundedness.
Performance: The safeguard counters are plain uint32_t increments with no heap allocations in the hot path. Threshold computation (std::max) is done once before the loop.
Code style: Follows VSAG conventions (4-space indent, uint32_t, .cpp suffix, vsag namespace).

Verdict

No blocking issues found. The implementation is clean, consistent, and well-tested. The previous review feedback has been fully incorporated.

LHT129

I have reviewed the latest version of this PR (commit e54b5e4). The changes from the previous review round (7c8e6d0) have addressed the earlier feedback:

empty_hops is now consistently reset to 0 alongside unrewarded_hops across all searcher overloads.
A dedicated kTerminationEmptyTraversalLimitReached termination reason has been introduced, resolving the semantic ambiguity with kTerminationHopsLimitReached.
ParallelSearcher now derives reasoning from QueryContext and records termination reasons in the safeguard break paths.

The core logic is sound: the two-tier safeguard (empty-traversal cap + unrewarded-hops cap) correctly prevents the searcher from stalling when filter selectivity is too low to fill ef candidates. The tests cover the key scenarios: fewer valid candidates than ef, zero valid candidates, and recall preservation across non-valid bridge nodes.

No new blocking issues found in the current diff.

LHT129

Review Summary

This PR addresses the stall issue (#2974) in low-selectivity filtered KNN search by introducing two safeguards:

Unrewarded Hops Safeguard (max_unrewarded_hops = max(ef, 500)): Caps traversal when consecutive hops fail to improve top_candidates despite candidate distances exceeding lower_bound.
Empty Traversal Safeguard (max_empty_hops = max(10000, 10 * ef)): Caps exploration when no valid candidates have been found yet.

What was checked

Correctness: The loop termination logic is sound across all 3 BasicSearcher::search_impl overloads and ParallelSearcher::search_impl. The unrewarded_hops counter is correctly reset to 0 whenever a valid candidate is pushed to top_candidates. The empty_hops counter is also reset alongside it (addressing a prior review comment).
Termination reasons: A dedicated kTerminationEmptyTraversalLimitReached constant was added to ReasoningContext to distinguish empty-traversal termination from the hops-limit termination, addressing a prior review comment.
ParallelSearcher: The reasoning variable is now wired up from QueryContext, and all new break paths correctly call SetTermination().
Tests: Three well-structured regression tests cover: valid candidates < ef, zero valid candidates, and recall preservation across non-valid bridge nodes. The tests verify both result correctness and traversal bounding via visited list assertions.
Code style: Consistent with VSAG conventions (4-space indent, uint32_t types, vsag namespace, .cpp suffix).

Conclusion

The changes are focused, correct, and well-tested. All issues raised in the previous review round have been addressed. No blocking concerns.

LHT129

Thanks for this fix. The approach of adding safeguard counters (unrewarded_hops and empty_hops) to prevent unbounded traversal in low-selectivity filtered KNN searches is sound and well-implemented.

Summary of changes:

Added unrewarded_hops counter (capped at max(ef, 500)) to prevent infinite traversal when valid candidates < ef
Added empty_hops counter (capped at max(10000, 10*ef)) to prevent infinite traversal when zero valid candidates exist
Applied consistently across all 3 BasicSearcher::search_impl overloads and ParallelSearcher::search_impl
Added kTerminationEmptyTraversalLimitReached reasoning constant for the empty-traversal case
Wired up ReasoningContext in ParallelSearcher (was previously missing)
Fixed Size() == ef → Size() >= ef in ParallelSearcher for consistency with BasicSearcher
Added 3 well-structured unit tests covering: valid candidates < ef, zero valid candidates, and recall across non-valid bridge nodes

Previous review comments from LHT129 have all been addressed:

empty_hops is now reset alongside unrewarded_hops at all top_candidates->Push sites
A distinct kTerminationEmptyTraversalLimitReached constant was added
ReasoningContext support was added to ParallelSearcher

No blocking issues found.

LHT129 · 2026-10-12T15:09:37Z


    float dist = 0.0F;
    auto lower_bound = std::numeric_limits<float>::max();
+    auto* reasoning = ctx == nullptr ? nullptr : ctx->reasoning_ctx;


[note] The ParallelSearcher::search_impl now correctly wires up reasoning from ctx and records termination reasons in the new safeguard paths, which is a good improvement over the initial version. However, unlike BasicSearcher::search_impl, the parallel searcher does not record per-hop reasoning details (visits, evictions, filter rejects) via ReasoningContext. This is pre-existing behavior and not introduced by this PR, but if full reasoning/debugging support is desired for the parallel searcher in the future, those calls should be added as well.

LHT129 · 2026-10-12T18:35:58Z

+    uint32_t unrewarded_hops = 0;
+    uint32_t empty_hops = 0;
+    const uint32_t max_unrewarded_hops = std::max<uint32_t>(ef, 500);
+    const uint32_t max_empty_hops = std::max<uint32_t>(10000, 10 * ef);


[note] std::max<uint32_t>(10000, 10 * ef) involves an implicit narrowing conversion: ef is uint64_t (see InnerSearchParam::ef), so 10 * ef is also uint64_t, but std::max<uint32_t> expects uint32_t arguments. While ef is unlikely to exceed 2^32 / 10 ≈ 4.3e8 in practice, compilers with -Wconversion may warn.

Consider computing with uint64_t and explicitly casting the result:

const uint32_t max_unrewarded_hops = static_cast<uint32_t>(std::max<uint64_t>(ef, 500)); const uint32_t max_empty_hops = static_cast<uint32_t>(std::max<uint64_t>(10000, 10 * ef));

This applies to all 3 search_impl overloads in basic_searcher.cpp and the one in parallel_searcher.cpp.

LHT129

Reviewed the latest version (e54b5e4) of this PR.

The fix correctly addresses the stall issue described in #2974 by introducing two safeguard mechanisms:

Unrewarded hops limit (max_unrewarded_hops = max(ef, 500)) — prevents infinite traversal when top_candidates cannot reach ef due to low-selectivity filters.
Empty traversal limit (max_empty_hops = max(10000, 10 * ef)) — caps exploration when no valid candidates exist at all.

The implementation is consistent across all 3 BasicSearcher::search_impl overloads and ParallelSearcher::search_impl. The new kTerminationEmptyTraversalLimitReached reasoning constant properly distinguishes this termination path.

The test coverage is good, covering three key scenarios: valid candidates < ef, zero valid candidates, and recall preservation across non-valid bridge nodes.

Previous review comments from @LHT129 regarding empty_hops reset consistency, termination reason naming, and ParallelSearcher reasoning wiring have all been addressed in the current version. The remaining [note] about std::max<uint32_t>(ef, 500) narrowing conversion (ef is uint64_t) is a minor style consideration that does not affect correctness.

No new issues found.

…l-2974 # Conflicts: # src/impl/reasoning/reasoning_context.h # src/impl/searcher/basic_searcher.cpp # src/impl/searcher/basic_searcher_test.cpp

LHT129 · 2026-10-14T11:40:18Z

+                if (empty_hops >= max_empty_hops) {
+                    if (reasoning != nullptr) {
+                        reasoning->SetTermination(
+                            ReasoningContext::kTerminationEmptyTraversalLimitReached);


[critical] ReasoningContext::kTerminationEmptyTraversalLimitReached does not exist in the ReasoningContext class (see src/impl/reasoning/reasoning_context.h). The correct identifier is ReasoningTermination::kEmptyTraversalLimitReached.

The SetTermination method accepts a ReasoningTermination enum value, and the enum kEmptyTraversalLimitReached is defined in reasoning_types.h. Using ReasoningContext::kTerminationEmptyTraversalLimitReached will cause a compilation error.

This applies to the first search_impl overload (DistanceProviderForGraph). The other two overloads correctly use ReasoningTermination::kEmptyTraversalLimitReached.

// Current (broken): reasoning->SetTermination( ReasoningContext::kTerminationEmptyTraversalLimitReached); // Fix: reasoning->SetTermination( ReasoningTermination::kEmptyTraversalLimitReached);

LHT129 · 2026-10-14T11:40:19Z

+            } else if (-candidate_set->Top().first > lower_bound) {
+                if (top_candidates->Size() >= ef) {
+                    if (reasoning != nullptr) {
+                        reasoning->SetTermination(ReasoningContext::kTerminationLowerBoundReached);


[critical] Same issue as above: ReasoningContext::kTerminationLowerBoundReached does not exist in the ReasoningContext class. The correct identifier is ReasoningTermination::kLowerBoundReached.

This appears in two places within the first search_impl overload (DistanceProviderForGraph):

The top_candidates->Size() >= ef early-exit path

The unrewarded_hops >= max_unrewarded_hops safeguard path

Both should use ReasoningTermination::kLowerBoundReached (as the other two overloads correctly do).

// Fix both occurrences: reasoning->SetTermination(ReasoningTermination::kLowerBoundReached);

LHT129 · 2026-10-14T11:40:36Z

-                break;
+            if (top_candidates->Empty()) {
+                ++empty_hops;
+                if (empty_hops >= max_empty_hops) {


[critical] ReasoningContext::kTerminationEmptyTraversalLimitReached and ReasoningContext::kTerminationLowerBoundReached do not exist in the ReasoningContext class. ReasoningContext::SetTermination() accepts a ReasoningTermination enum value, not a ReasoningContext member.

The correct constants are:

ReasoningTermination::kEmptyTraversalLimitReached (defined in reasoning_types.h)

ReasoningTermination::kLowerBoundReached (defined in reasoning_types.h)

This affects the first search_impl overload in basic_searcher.cpp (this location) and the parallel_searcher.cpp safeguard paths. The second and third search_impl overloads in basic_searcher.cpp correctly use ReasoningTermination::k*.

Using the non-existent ReasoningContext::kTermination* names will cause a compilation failure.

LHT129 · 2026-10-14T11:40:58Z

+                    break;
+                }
+            } else if ((-current_first_node_pair.first) > lower_bound) {
+                if (top_candidates->Size() >= ef) {


[critical] Same issue as in basic_searcher.cpp: ReasoningContext::kTerminationEmptyTraversalLimitReached and ReasoningContext::kTerminationLowerBoundReached do not exist. Use ReasoningTermination::kEmptyTraversalLimitReached and ReasoningTermination::kLowerBoundReached instead.

LHT129 · 2026-10-14T11:41:18Z

@@ -147,10 +148,15 @@ ParallelSearcher::search_impl(const GraphInterfacePtr& graph,



[critical] #include "impl/reasoning/search_reasoning.h" references a file that does not exist in the repository. The ReasoningContext class is declared in reasoning_context.h (already included transitively or directly elsewhere). The reasoning variable in this file uses ReasoningContext which is declared in impl/reasoning/reasoning_context.h. This include should be removed or corrected to #include "impl/reasoning/reasoning_context.h".

LHT129

This commit (6b57438) addresses all previously raised review concerns:

Fixed enum references: ReasoningContext::kTermination* → ReasoningTermination::k* across all BasicSearcher overloads and ParallelSearcher.
Fixed missing include: parallel_searcher.cpp now correctly includes reasoning_context.h and reasoning_types.h instead of the non-existent search_reasoning.h.
Added empty_hops reset: empty_hops = 0 is now reset alongside unrewarded_hops = 0 in all push paths across all searcher overloads.
Added reasoning support to ParallelSearcher: The reasoning variable is now wired from QueryContext and SetTermination is called in both safeguard break paths.
Fixed Size() == ef → Size() >= ef: The ParallelSearcher break condition now uses >= (consistent with BasicSearcher), preventing a stall when top_candidates exceeds ef.

The safeguard logic is consistently applied across all 3 BasicSearcher::search_impl overloads and ParallelSearcher::search_impl. The new ReasoningTermination::kEmptyTraversalLimitReached enum value is properly defined and wired through ToString. Tests cover the three key scenarios: valid candidates < ef, zero valid candidates, and recall preservation across non-valid bridge nodes.

No new issues identified.

…nversions Signed-off-by: Ahmed <thlurte@gmail.com>

LHT129

This review covers commit 04cae0d0. All previously flagged critical issues have been addressed:

Wrong constant names — ReasoningContext::kTermination* has been corrected to ReasoningTermination::k* across all search_impl overloads and parallel_searcher.cpp.
Missing empty_hops reset — empty_hops is now reset to 0 alongside unrewarded_hops in all push-to-top_candidates paths.
Missing termination reason for empty traversal — kEmptyTraversalLimitReached has been added to the ReasoningTermination enum and ToString mapping.
Parallel searcher reasoning support — parallel_searcher.cpp now properly obtains reasoning from QueryContext and records termination reasons in all safeguard break paths.
Non-existent include — parallel_searcher.cpp now includes reasoning_context.h and reasoning_types.h.

The implementation is consistent across all 3 BasicSearcher::search_impl overloads and ParallelSearcher::search_impl. The safeguard thresholds (max_unrewarded_hops = max(ef, 500), max_empty_hops = max(10000, 10*ef)) are reasonable defaults. The tests cover the key scenarios: fewer valid candidates than ef, zero valid candidates, and recall preservation across non-valid bridge nodes.

No new issues found. LGTM.

fix(hgraph): bound traversal for low-selectivity filtered KNN (antgro…

0cdc46c

…up#2974) Signed-off-by: Ahmed <thlurte@gmail.com>

thlurte requested review from LHT129, inabao, jiaweizone and wxyucs as code owners September 20, 2026 13:51

pull-request-size Bot added the size/L 100-499 changed lines label Sep 20, 2026

vsag-bot added the version/1.1 label Sep 20, 2026

LHT129 reviewed Sep 20, 2026

View reviewed changes

thlurte added 3 commits September 20, 2026 19:28

fix(searcher): reset empty_hops on valid candidate discovery

7c8e6d0

Reset empty_hops alongside unrewarded_hops whenever a valid candidate is pushed to top_candidates across BasicSearcher and ParallelSearcher for defense-in-depth. Signed-off-by: Ahmed <thlurte@gmail.com>

fix(reasoning): distinguish empty traversal safeguard termination reason

e13efe8

Add kTerminationEmptyTraversalLimitReached constant to differentiate between the user-configured hops_limit and empty traversal safeguard termination in reasoning reports. Signed-off-by: Ahmed <thlurte@gmail.com>

fix(searcher): wire up reasoning context in ParallelSearcher

e54b5e4

Derive reasoning from QueryContext in ParallelSearcher and record termination reasons for empty traversal and lower bound early breaks. Signed-off-by: Ahmed <thlurte@gmail.com>

LHT129 reviewed Sep 20, 2026

View reviewed changes

LHT129 reviewed Oct 3, 2026

View reviewed changes

LHT129 reviewed Oct 4, 2026

View reviewed changes

LHT129 reviewed Oct 7, 2026

View reviewed changes

LHT129 reviewed Oct 12, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into fix-filtered-knn-stal…

af70bc7

…l-2974 # Conflicts: # src/impl/reasoning/reasoning_context.h # src/impl/searcher/basic_searcher.cpp # src/impl/searcher/basic_searcher_test.cpp

LHT129 reviewed Oct 14, 2026

View reviewed changes

fix(searcher): use ReasoningTermination enum and resolve narrowing co…

04cae0d

…nversions Signed-off-by: Ahmed <thlurte@gmail.com>

thlurte force-pushed the fix-filtered-knn-stall-2974 branch from 6b57438 to 04cae0d Compare October 14, 2026 13:27

LHT129 reviewed Oct 14, 2026

View reviewed changes

		@@ -253,6 +279,7 @@ BasicSearcher::search_impl(const GraphInterfacePtr& graph,
		if (check_func(id) and

		@@ -258,8 +262,21 @@ ParallelSearcher::search_impl(const GraphInterfacePtr& graph,
		node_pair[0] = current_first_node_pair;

		@@ -147,10 +148,15 @@ ParallelSearcher::search_impl(const GraphInterfacePtr& graph,

Conversation

thlurte commented Sep 20, 2026

What problem does this PR solve?

What is changed and how it works?

Check List

Uh oh!

vsag-bot commented Sep 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mergify Bot commented Sep 20, 2026

Merge Protections

🔴 Require kind label

🟢 Require version label

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thlurte commented Sep 20, 2026

Uh oh!

LHT129 left a comment

Choose a reason for hiding this comment

Uh oh!

LHT129 left a comment

Choose a reason for hiding this comment

Uh oh!

LHT129 left a comment

Choose a reason for hiding this comment

Uh oh!

thlurte commented Oct 3, 2026

Uh oh!

LHT129 left a comment

Choose a reason for hiding this comment

Uh oh!

LHT129 left a comment

Choose a reason for hiding this comment

Review Summary

What was checked

Verdict

Uh oh!

LHT129 left a comment

Choose a reason for hiding this comment

Uh oh!

LHT129 left a comment

Choose a reason for hiding this comment

Review Summary

What was checked

Conclusion

Uh oh!

LHT129 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LHT129 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LHT129 left a comment

Choose a reason for hiding this comment

Uh oh!

vsag-bot commented Sep 20, 2026 •

edited

Loading