From ea449efc9340fca4369170def2b9e15a8e092e02 Mon Sep 17 00:00:00 2001 From: QingCheng24 Date: Thu, 23 Jul 2026 15:23:10 +0200 Subject: [PATCH] fix(search): stop double-counting fail_count so DeepSearch terminates 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). --- .../framework/openjiuwen/agent/workflow.py | 7 +- .../test_integration_search_loop.py | 72 +++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/deepsearch/openjiuwen_deepsearch/framework/openjiuwen/agent/workflow.py b/deepsearch/openjiuwen_deepsearch/framework/openjiuwen/agent/workflow.py index 7d246db0..2a02913d 100644 --- a/deepsearch/openjiuwen_deepsearch/framework/openjiuwen/agent/workflow.py +++ b/deepsearch/openjiuwen_deepsearch/framework/openjiuwen/agent/workflow.py @@ -1398,7 +1398,12 @@ async def run_state_creation_workflow( "total_input_tokens": run_context.total_input_tokens, "total_output_tokens": run_context.total_output_tokens, "log_dir": run_context.log_dir, - "fail_count": run_context.fail_count, + # Pass 0 so the sub-workflow reports a per-action delta (0/1), + # not the running cumulative total. The parent accumulates it + # at the completion site; passing run_context.fail_count here + # caused the total to be double-counted (and inflated under + # parallelism). + "fail_count": 0, }, ) diff --git a/deepsearch/tests/search_agent/test_integration_search_loop.py b/deepsearch/tests/search_agent/test_integration_search_loop.py index 8fc183cb..714ab624 100644 --- a/deepsearch/tests/search_agent/test_integration_search_loop.py +++ b/deepsearch/tests/search_agent/test_integration_search_loop.py @@ -21,6 +21,13 @@ pytestmark = pytest.mark.integration +class ExposedDeepSearchAgent(DeepSearchAgent): + """用于测试的类，公开受保护的方法以遵循 G.CLS.11 规则""" + + async def run_internal(self, *args, **kwargs): + return await super()._run_internal(*args, **kwargs) + + def _make_agent(tmp_log_dir: Path, **pqp_updates: Any): agent = DeepSearchAgent() agent_config = AgentConfig() @@ -244,6 +251,71 @@ async def _fake_state_creation(*args: Any, **kwargs: Any) -> SimpleNamespace: assert final.prediction == "Lyon" +@pytest.mark.asyncio +async def test_fail_count_accumulates_one_per_failed_action( + monkeypatch: pytest.MonkeyPatch, tmp_log_dir: Path, base_action, base_state +) -> None: + """Each failed action must raise the global fail_count by exactly 1. + + Regression for the double-counting bug: the parent used to pass its + cumulative ``fail_count`` into every state_creation sub-workflow AND add the + (already-cumulative) returned value back with ``+=``. With ``fail_limit=3`` + that tripped termination after 2 failed actions instead of 3. Here the mock + reproduces the real sub-workflow contract -- it returns the passed-in + ``fail_count`` incremented by 1 -- so the bug would surface as an early + termination. + """ + _, run_context = _make_agent(tmp_log_dir, fail_limit=3, max_workers=1) + agent = ExposedDeepSearchAgent() + actions = [ + base_action.model_copy( + update={"id": f"action-{i}", "proposal": ActionProposal(direction=f"d{i}", score=0.5)} + ) + for i in range(5) + ] + state_creation_calls: list[int] = [] + + async def _fake_run_workflow(*, workflow: str, inputs: dict) -> SimpleNamespace: + if workflow == "init_state_1": + return SimpleNamespace( + result={"init_state": base_state, "total_input_tokens": 0, "total_output_tokens": 0} + ) + if workflow == "find_action_1": + return SimpleNamespace( + result={"actions": actions, "total_input_tokens": 0, "total_output_tokens": 0} + ) + if workflow == "state_creation_1": + # Mirror algorithm/search_nodes/utils.py: the sub-workflow increments + # the fail_count it was handed and echoes it back inside ``config``. + passed_in = inputs.get("fail_count", 0) + state_creation_calls.append(passed_in) + return SimpleNamespace( + result={ + "result": None, + "config": {"fail_count": passed_in + 1}, + "total_input_tokens": 0, + "total_output_tokens": 0, + } + ) + raise AssertionError(workflow) + + monkeypatch.setattr( + "openjiuwen_deepsearch.framework.openjiuwen.agent.workflow.Runner.run_workflow", + _fake_run_workflow, + ) + + final = await agent.run_internal(run_context) + + assert final.termination == "fail_limit" + # Exactly 3 failed actions are needed to reach fail_limit=3 (one increment + # each). The double-counting bug would terminate after only 2. + assert len(state_creation_calls) == 3 + assert run_context.fail_count == 3 + # The parent must hand a per-action delta base of 0 to each sub-workflow, + # not its running cumulative total. + assert state_creation_calls == [0, 0, 0] + + @pytest.mark.asyncio async def test_answer_writes_final_result_json( monkeypatch: pytest.MonkeyPatch, tmp_log_dir: Path, base_action, base_state