From 89c3ecc7623c1077010fe0bd2d7aa38542439f4f Mon Sep 17 00:00:00 2001 From: lbbniu Date: Tue, 9 Dec 2025 11:30:05 +0800 Subject: [PATCH 1/3] 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 33c533c5e5..c83c4034ba 100644 --- a/python/packages/core/agent_framework/_workflows/_handoff.py +++ b/python/packages/core/agent_framework/_workflows/_handoff.py @@ -322,6 +322,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.""" @@ -565,6 +587,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 32a7a8ac17f6a317d018e931daa08b58f7dde606 Mon Sep 17 00:00:00 2001 From: lbbniu Date: Tue, 9 Dec 2025 12:05:09 +0800 Subject: [PATCH 2/3] 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. # Conflicts: # python/packages/core/tests/workflow/test_handoff.py --- .../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 d0d5092323..c1afadc49e 100644 --- a/python/packages/core/tests/workflow/test_handoff.py +++ b/python/packages/core/tests/workflow/test_handoff.py @@ -1670,3 +1670,101 @@ async def test_coordinator_handle_user_input_multiple_consecutive_user_messages( assert len(coordinator._conversation) == 2 assert coordinator._conversation[0].text == "New user message 1" assert coordinator._conversation[1].text == "New user message 2" + + +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" From d9134116ee62ce44d749a31ccffe903471a1c634 Mon Sep 17 00:00:00 2001 From: lbbniu Date: Fri, 19 Dec 2025 11:43:39 +0800 Subject: [PATCH 3/3] Python: Refactor nested if statements in handoff workflow Simplify code by combining nested if statements into a single condition using 'and' operator. This addresses SIM102 linting issue while maintaining the same logic for checking tool result call IDs. --- python/packages/core/agent_framework/_workflows/_handoff.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/python/packages/core/agent_framework/_workflows/_handoff.py b/python/packages/core/agent_framework/_workflows/_handoff.py index c83c4034ba..4a6d855bb7 100644 --- a/python/packages/core/agent_framework/_workflows/_handoff.py +++ b/python/packages/core/agent_framework/_workflows/_handoff.py @@ -337,9 +337,8 @@ def _has_tool_result_for_call(conversation: list[ChatMessage], call_id: str) -> continue for content in msg.contents: - if isinstance(content, FunctionResultContent): - if content.call_id == call_id: - return True + if isinstance(content, FunctionResultContent) and content.call_id == call_id: + return True return False