feat: enhance tool result handling with tool_call_id for concurrent execution
This commit is contained in:
@@ -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()
|
||||
|
||||
|
||||
@@ -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,
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
@@ -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):
|
||||
|
||||
@@ -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":
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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):
|
||||
|
||||
Reference in New Issue
Block a user