From d2e97760e2a46ccedbdc3284128062a37c21879c Mon Sep 17 00:00:00 2001 From: lbbniu Date: Tue, 9 Dec 2025 11:30:05 +0800 Subject: [PATCH 1/2] Python: Fix duplicate tool result messages in handoff workflow (#2711) When using HandoffBuilder, the _HandoffCoordinator._append_tool_acknowledgement method was creating duplicate tool result messages in the conversation history. This occurred because the method didn't check if a tool result with the same call_id already existed before appending a new one. The duplication happened when: 1. _AutoHandoffMiddleware creates a synthetic tool result 2. ChatAgent appends this result to the conversation 3. _HandoffCoordinator._append_tool_acknowledgement creates another result with the same call_id but different author_name This led to: - Polluted conversation history - Unnecessary token usage - Increased checkpoint storage - Confusing debugging experience Changes: - Add _has_tool_result_for_call() module-level helper function to check if a tool result with the given call_id already exists in the conversation - Modify _append_tool_acknowledgement() to skip adding tool result if one already exists with the same call_id - Add debug logging when skipping duplicate tool acknowledgement Fixes #2711 --- .../agent_framework/_workflows/_handoff.py | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/python/packages/core/agent_framework/_workflows/_handoff.py b/python/packages/core/agent_framework/_workflows/_handoff.py index 0a3608d2e6..ff10cf90a9 100644 --- a/python/packages/core/agent_framework/_workflows/_handoff.py +++ b/python/packages/core/agent_framework/_workflows/_handoff.py @@ -281,6 +281,28 @@ def _target_from_tool_name(name: str | None) -> str | None: return None +def _has_tool_result_for_call(conversation: list[ChatMessage], call_id: str) -> bool: + """Check if a tool result message exists for the given call_id. + + Args: + conversation: The conversation history to check + call_id: The function call ID to look for + + Returns: + True if a tool result with matching call_id exists, False otherwise + """ + for msg in conversation: + if msg.role != Role.TOOL: + continue + + for content in msg.contents: + if isinstance(content, FunctionResultContent): + if content.call_id == call_id: + return True + + return False + + class _HandoffCoordinator(BaseGroupChatOrchestrator): """Coordinates agent-to-agent transfers and user turn requests.""" @@ -482,6 +504,14 @@ def _append_tool_acknowledgement( if not call_id: return + # Skip if tool result already exists to avoid duplicates from _AutoHandoffMiddleware + if _has_tool_result_for_call(conversation, call_id): + logger.debug( + f"Tool result for call_id '{call_id}' already exists, " + f"skipping duplicate for handoff to '{resolved_id}'" + ) + return + result_payload: Any = {"handoff_to": resolved_id} result_content = FunctionResultContent(call_id=call_id, result=result_payload) tool_message = ChatMessage( From 5b9252af384a753c5d75eb95afa2cb5a88c83fc8 Mon Sep 17 00:00:00 2001 From: lbbniu Date: Tue, 9 Dec 2025 12:05:09 +0800 Subject: [PATCH 2/2] Python: Add test coverage for duplicate tool result detection in handoff workflow This commit addresses Copilot's review comment on PR #2711 by adding comprehensive test coverage for the duplicate detection logic introduced in commit d2e97760. Test coverage includes: 1. Verification that _has_tool_result_for_call() correctly detects existing tool results 2. Verification that _has_tool_result_for_call() returns False for non-existent call_ids 3. Verification that _append_tool_acknowledgement() skips adding duplicates 4. Verification that debug log messages are emitted when duplicates are detected 5. Verification that tool results are added when no duplicate exists The test ensures that the fix for issue #2711 (preventing duplicate tool result messages in the conversation history) remains effective and prevents regression. --- .../core/tests/workflow/test_handoff.py | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/python/packages/core/tests/workflow/test_handoff.py b/python/packages/core/tests/workflow/test_handoff.py index 722885a6cb..07e5dc5ac7 100644 --- a/python/packages/core/tests/workflow/test_handoff.py +++ b/python/packages/core/tests/workflow/test_handoff.py @@ -727,3 +727,101 @@ async def test_return_to_previous_state_serialization(): # Verify current_agent_id was restored assert coordinator2._current_agent_id == "specialist_a", "Current agent should be restored from checkpoint" # type: ignore[reportPrivateUsage] + + +def test_duplicate_tool_result_detection(caplog): + """Test that _append_tool_acknowledgement skips duplicates and logs debug message.""" + from agent_framework import FunctionResultContent + from agent_framework._workflows._handoff import ( # type: ignore[reportPrivateUsage] + _HandoffCoordinator, + _has_tool_result_for_call, + ) + + # Test _has_tool_result_for_call helper function + call_id_1 = "call-123" + call_id_2 = "call-456" + + # Create a conversation with an existing tool result + existing_tool_result = FunctionResultContent(call_id=call_id_1, result={"handoff_to": "specialist_a"}) + existing_tool_msg = ChatMessage(role=Role.TOOL, contents=[existing_tool_result], author_name="handoff_tool") + + conversation_with_result = [ + ChatMessage(role=Role.USER, text="Help me"), + ChatMessage(role=Role.ASSISTANT, text="Routing to specialist"), + existing_tool_msg, + ] + + # Should find the existing tool result + assert _has_tool_result_for_call(conversation_with_result, call_id_1) is True, ( + "Should detect existing tool result" + ) + + # Should not find a tool result with different call_id + assert _has_tool_result_for_call(conversation_with_result, call_id_2) is False, ( + "Should not find tool result with different call_id" + ) + + # Test on conversation without tool results + conversation_without_result = [ + ChatMessage(role=Role.USER, text="Help me"), + ChatMessage(role=Role.ASSISTANT, text="Routing to specialist"), + ] + assert _has_tool_result_for_call(conversation_without_result, call_id_1) is False, ( + "Should not find tool result in conversation without tool messages" + ) + + # Test _append_tool_acknowledgement skips duplicates + coordinator = _HandoffCoordinator( + starting_agent_id="triage", + specialist_ids={"specialist_a": "specialist_a"}, + input_gateway_id="gateway", + termination_condition=lambda conv: False, + id="test-coordinator", + ) + + # Create a function call for handoff + handoff_call = FunctionCallContent(call_id=call_id_1, name="handoff_to_specialist_a", arguments="{}") + + # Conversation already has a tool result for this call_id (simulating _AutoHandoffMiddleware behavior) + conversation_with_duplicate = list(conversation_with_result) + + # Try to append tool acknowledgement - should skip due to duplicate + # Set caplog to capture DEBUG level logs from the handoff module + import logging + + caplog.set_level(logging.DEBUG, logger="agent_framework._workflows._handoff") + coordinator._append_tool_acknowledgement( # type: ignore[reportPrivateUsage] + conversation_with_duplicate, handoff_call, "specialist_a" + ) + + # Verify conversation wasn't modified (no duplicate added) + assert len(conversation_with_duplicate) == 3, "Conversation should not have duplicate tool result added" + + # Verify debug log was emitted + assert any( + "Tool result for call_id 'call-123' already exists" in record.message for record in caplog.records + ), "Should log debug message when skipping duplicate" + + assert any( + "skipping duplicate for handoff to 'specialist_a'" in record.message for record in caplog.records + ), "Debug message should mention the target specialist" + + # Test that tool acknowledgement IS added when no duplicate exists + conversation_no_duplicate = [ + ChatMessage(role=Role.USER, text="Help me"), + ChatMessage(role=Role.ASSISTANT, text="Routing to specialist"), + ] + new_call = FunctionCallContent(call_id="call-new", name="handoff_to_specialist_a", arguments="{}") + + caplog.clear() + coordinator._append_tool_acknowledgement(conversation_no_duplicate, new_call, "specialist_a") # type: ignore[reportPrivateUsage] + + # Should have added the tool result + assert len(conversation_no_duplicate) == 3, "Tool result should be added when no duplicate exists" + last_msg = conversation_no_duplicate[-1] + assert last_msg.role == Role.TOOL, "Last message should be a tool result" + + # Should NOT log the debug message (no duplicate detected) + assert not any( + "already exists" in record.message for record in caplog.records + ), "Should not log duplicate message when adding new tool result"