From 06822f236cab6384f3e6ef5ff014e381bb29f7df Mon Sep 17 00:00:00 2001 From: Xi Zhang Date: Sun, 19 Apr 2026 23:15:20 +0100 Subject: [PATCH] feat: enhance tool result handling with tool_call_id for concurrent execution --- EvoScientist/cli/widgets/subagent_widget.py | 5 ++- EvoScientist/stream/emitter.py | 7 +++- EvoScientist/stream/events.py | 3 +- EvoScientist/stream/state.py | 26 +++++++++++-- tests/test_stream_emitter.py | 7 ++++ tests/test_stream_state.py | 16 ++++++++ tests/test_tui_widgets.py | 43 +++++++++++++++++++++ 7 files changed, 99 insertions(+), 8 deletions(-) diff --git a/EvoScientist/cli/widgets/subagent_widget.py b/EvoScientist/cli/widgets/subagent_widget.py index 393085e..acc542b 100644 --- a/EvoScientist/cli/widgets/subagent_widget.py +++ b/EvoScientist/cli/widgets/subagent_widget.py @@ -208,10 +208,11 @@ class SubAgentWidget(Vertical): widget.set_success(content) else: widget.set_error(content) - # Move from running to completed + # Move from running to completed (dedup guards against repeat + # deliveries of the same tool result inflating the collapse summary). if matched_key and matched_key in self._running_ids: self._running_ids.remove(matched_key) - if matched_key: + if matched_key and matched_key not in self._completed_ids: self._completed_ids.append(matched_key) self._update_visibility() diff --git a/EvoScientist/stream/emitter.py b/EvoScientist/stream/emitter.py index aee2b98..1198ca3 100644 --- a/EvoScientist/stream/emitter.py +++ b/EvoScientist/stream/emitter.py @@ -82,7 +82,11 @@ class StreamEventEmitter: @staticmethod def subagent_tool_result( - subagent: str, name: str, content: str, success: bool = True + subagent: str, + name: str, + content: str, + success: bool = True, + tool_call_id: str = "", ) -> StreamEvent: """Tool result from inside a sub-agent.""" return StreamEvent( @@ -93,6 +97,7 @@ class StreamEventEmitter: "name": name, "content": content, "success": success, + "id": tool_call_id, }, ) diff --git a/EvoScientist/stream/events.py b/EvoScientist/stream/events.py index 83fc9fb..dc5eae9 100644 --- a/EvoScientist/stream/events.py +++ b/EvoScientist/stream/events.py @@ -801,11 +801,12 @@ async def stream_agent_events( info.id, ).data name = getattr(msg, "name", "unknown") + tool_call_id = getattr(msg, "tool_call_id", "") or "" raw_content, _is_img = _extract_tool_content(msg) content = raw_content[: DisplayLimits.TOOL_RESULT_MAX] success = is_success(content) yield emitter.subagent_tool_result( - subagent, name, content, success + subagent, name, content, success, tool_call_id ).data else: for ev in _process_tool_result(msg, emitter, main_tracker): diff --git a/EvoScientist/stream/state.py b/EvoScientist/stream/state.py index 4115699..3290176 100644 --- a/EvoScientist/stream/state.py +++ b/EvoScientist/stream/state.py @@ -42,17 +42,34 @@ class SubAgentState: return self.tool_calls.append(tc_data) - def add_tool_result(self, name: str, content: str, success: bool = True): - result = {"name": name, "content": content, "success": success} + def add_tool_result( + self, + name: str, + content: str, + success: bool = True, + tool_call_id: str = "", + ): + result = { + "name": name, + "content": content, + "success": success, + "tool_call_id": tool_call_id, + } self.tool_results.append(result) - # Try to match result to the first unmatched tool call with same name + # Preferred: exact id match (correct under concurrent same-name tools). + if tool_call_id: + for tc in self.tool_calls: + if tc.get("id") == tool_call_id: + self._result_map[tool_call_id] = result + return + # Fallback: first unmatched tool call with same name. for tc in self.tool_calls: tc_id = tc.get("id", "") tc_name = tc.get("name", "") if tc_id and tc_id not in self._result_map and tc_name == name: self._result_map[tc_id] = result return - # Fallback: match first unmatched tool call + # Last resort: first unmatched tool call regardless of name. for tc in self.tool_calls: tc_id = tc.get("id", "") if tc_id and tc_id not in self._result_map: @@ -257,6 +274,7 @@ class StreamState: event.get("name", "unknown"), event.get("content", ""), event.get("success", True), + event.get("id", ""), ) elif event_type == "subagent_end": diff --git a/tests/test_stream_emitter.py b/tests/test_stream_emitter.py index 34ab026..60722e9 100644 --- a/tests/test_stream_emitter.py +++ b/tests/test_stream_emitter.py @@ -54,6 +54,13 @@ class TestStreamEventEmitter: ) assert ev.type == "subagent_tool_result" assert ev.data["subagent"] == "research-agent" + assert ev.data["id"] == "" # default when caller omits tool_call_id + + def test_subagent_tool_result_with_id(self): + ev = StreamEventEmitter.subagent_tool_result( + "research-agent", "execute", "ok", True, tool_call_id="tc_xyz" + ) + assert ev.data["id"] == "tc_xyz" def test_subagent_end(self): ev = StreamEventEmitter.subagent_end("research-agent") diff --git a/tests/test_stream_state.py b/tests/test_stream_state.py index dd41425..e092876 100644 --- a/tests/test_stream_state.py +++ b/tests/test_stream_state.py @@ -61,6 +61,20 @@ class TestSubAgentState: assert result is not None assert result["content"] == "output" + def test_add_tool_result_matched_by_id(self): + """Concurrent same-name tools must be paired by tool_call_id, not name order.""" + sa = SubAgentState("agent") + sa.add_tool_call("execute", {"cmd": "a"}, "tc1") + sa.add_tool_call("execute", {"cmd": "b"}, "tc2") + # Result for tc2 arrives first (out-of-order). + sa.add_tool_result("execute", "out2", True, tool_call_id="tc2") + assert sa.get_result_for(sa.tool_calls[1])["content"] == "out2" + assert sa.get_result_for(sa.tool_calls[0]) is None + # Then tc1's result arrives. + sa.add_tool_result("execute", "out1", True, tool_call_id="tc1") + assert sa.get_result_for(sa.tool_calls[0])["content"] == "out1" + assert sa.get_result_for(sa.tool_calls[1])["content"] == "out2" + def test_get_result_for_no_match(self): sa = SubAgentState("agent") tc = {"id": "tc_missing", "name": "x", "args": {}} @@ -188,10 +202,12 @@ class TestStreamState: "name": "execute", "content": "output", "success": True, + "id": "tc1", } ) sa = state.subagents[0] assert len(sa.tool_results) == 1 + assert sa.get_result_for(sa.tool_calls[0])["content"] == "output" def test_handle_subagent_end(self): state = StreamState() diff --git a/tests/test_tui_widgets.py b/tests/test_tui_widgets.py index eff270a..989fc7c 100644 --- a/tests/test_tui_widgets.py +++ b/tests/test_tui_widgets.py @@ -338,6 +338,49 @@ class TestSubAgentWidget(unittest.TestCase): assert "id-1" in sa._tool_widgets assert sa._tool_widgets["id-1"] is tw + def test_complete_tool_routes_by_id_and_dedups(self): + """Result delivery must use tool_call_id so concurrent same-name tools + don't leave orphans that get marked ``interrupted`` at finalize time. + Also, repeat deliveries must not inflate ``_completed_ids``. + """ + from EvoScientist.cli.widgets.subagent_widget import SubAgentWidget + from EvoScientist.cli.widgets.tool_call_widget import ToolCallWidget + + class _FakeToolWidget(ToolCallWidget): + def set_success(self, content): + self._status = "success" + self._result_content = content + + def set_error(self, content): + self._status = "error" + self._result_content = content + + sa = SubAgentWidget("code-agent") + sa._update_visibility = lambda: None # skip mount-dependent render + + tw_a = _FakeToolWidget("execute", {"cmd": "a"}, "id_A") + tw_b = _FakeToolWidget("execute", {"cmd": "b"}, "id_B") + sa._tool_widgets["id_A"] = tw_a + sa._tool_widgets["id_B"] = tw_b + sa._running_ids = ["id_A", "id_B"] + + # Out-of-order delivery: result for B arrives first. + sa.complete_tool("execute", "out B", True, tool_id="id_B") + assert tw_b._status == "success" + assert tw_a._status == "running" + assert sa._completed_ids == ["id_B"] + assert sa._running_ids == ["id_A"] + + # Duplicate delivery for id_B must not inflate counts. + sa.complete_tool("execute", "out B dup", True, tool_id="id_B") + assert sa._completed_ids == ["id_B"] + + # Now id_A finishes normally. + sa.complete_tool("execute", "out A", True, tool_id="id_A") + assert tw_a._status == "success" + assert sa._completed_ids == ["id_B", "id_A"] + assert sa._running_ids == [] + @unittest.skipUnless(_has_textual, "textual not installed") class TestTodoWidget(unittest.TestCase):