fix(search): stop double-counting fail_count so DeepSearch terminates… - #17
Open
QingCheng24 wants to merge 1 commit into
Open
fix(search): stop double-counting fail_count so DeepSearch terminates…#17QingCheng24 wants to merge 1 commit into
QingCheng24 wants to merge 1 commit into
Conversation
|
|
|
Hi @QingCheng24, thanks for your contribution! This pull request is currently waiting for the CLA Assistant check to pass before the CI process can continue. Please follow the signing link shown by CLA Assistant on this PR. If you have already signed the CLA but the check is still failing, please make sure that all email addresses used in your commits are added to and verified on your GitHub account. After updating your email settings or signing the CLA, please click Recheck in the CLA Assistant comment/check so the status can be refreshed. Once the CLA Assistant check passes, CI will continue automatically. |
… on time Each state_creation sub-workflow was invoked with the parent's cumulative fail_count (workflow.py) and increments it by 1 on a failed action (algorithm/search_nodes/utils.py). The parent then added that already- cumulative value back with `+=`, re-adding the base every iteration and compounding under parallel workers. With fail_limit reached far too early, the agent returned FAIL_LIMIT with a missing or low-quality answer. Fix by making the accounting a per-action delta: - pass fail_count=0 into each sub-workflow, so it reports only its own 0/1 increment rather than echoing back the running total; - accumulate that delta with a default of 0 (the previous default of self.fail_count doubled the counter whenever the key was absent). Adds an integration regression test that mirrors the real sub-workflow contract; with fail_limit=3 it now requires 3 failed actions to terminate (the bug terminated after 2).
QingCheng24
force-pushed
the
fix/search-fail-count-double-count
branch
from
August 7, 2026 12:36
15a9a64 to
ea449ef
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Paired: GitHub #17 ↔ GitCode !324
What type of PR is this?
/kind bug
What does this PR do / why do we need it:
state_creationsub-workflow was invoked with the parent's cumulativefail_count(workflow.py), and the sub-workflow increments it by 1 on a failed action (algorithm/search_nodes/utils.py). The parent then added that already-cumulative value back with+=, re-adding the base every iteration and compounding under parallel workers. As a resultself.fail_count >= fail_limittripped much earlier than intended, and the agent returnedFAIL_LIMITwith a missing or low-quality answer.fail_count=0into each sub-workflow, so it reports only its own 0/1 increment rather than echoing back the running total;0(the previous default ofself.fail_countdoubled the counter whenever the key was absent).fail_limitstill means "terminate after N failed actions"; this PR makes the counter actually honor it.Which issue(s) this PR fixes:
N/A (no tracked issue)
What scenarios were tested, and what were the verification results (Function, performance, reliability, etc.):
test_fail_count_accumulates_one_per_failed_actionintests/search_agent/test_integration_search_loop.py. It mirrors the real sub-workflow contract (returns the passed-infail_count + 1insideconfig) and asserts that withfail_limit=3the loop needs exactly 3 failed actions to terminate, thatagent.fail_count == 3, and that each sub-workflow is handed a delta base of0.configshowsfail_countjumping 1 -> 2 -> limit). With the fix in place it passes.pytest tests/search_agent/test_integration_search_loop.py-> 5 passed.tests/search_agentrun -> passing (1 pre-existing failuretest_simple_react_telemetry_uses_built_tool_map, confirmed failing on cleanmain, unrelated to this PR).python -m compileall openjiuwen_deepsearch server-> clean.Self-checklist:
fail_limitsemantics are unchanged; this restores the intended behavior.)