mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Python: Fix MCP tool result serialization for list[TextContent] (#2523)
* Fix MCP tool result serialization for list[TextContent] When MCP tools return results containing list[TextContent], they were incorrectly serialized to object repr strings like: '[<agent_framework._types.TextContent object at 0x...>]' This fix properly extracts text content from list items by: 1. Checking if items have a 'text' attribute (TextContent) 2. Using model_dump() for items that support it 3. Falling back to str() for other types 4. Joining single items as plain text, multiple items as JSON array Fixes #2509 * Address PR review feedback for MCP tool result serialization - Extract serialize_content_result() to shared _utils.py - Fix logic: use texts[0] instead of join for single item - Add type annotation: texts: list[str] = [] - Return empty string for empty list instead of '[]' - Move import json to file top level - Add comprehensive unit tests for serialization * Address PR review feedback: fix type checking and double serialization - Add isinstance(item.text, str) check to ensure text attribute is a string - Fix double-serialization issue by keeping model_dump results as dicts until final json.dumps (removes escaped JSON strings in arrays) - Improve docstring with detailed return value documentation - Add test for non-string text attribute handling - Add tests for list type tool results in _events.py path * Simplify PR: minimal changes to fix MCP tool result serialization Addresses reviewer feedback about excessive refactoring: - Reset _events.py to original structure - Only add import and use serialize_content_result in one location - All review comments addressed in serialize_content_result(): - Added isinstance(item.text, str) check - Use model_dump(mode="json") to avoid double-serialization - Improved docstring with explicit return value documentation - Empty list returns "" instead of "[]" * Refactor: Move MCP TextContent serialization to core prepare_function_call_results Per reviewer feedback, moved the TextContent serialization logic from ag-ui's serialize_content_result to the core package's prepare_function_call_results function. Changes: - Added handling for objects with 'text' attribute (like MCP TextContent) in _prepare_function_call_results_as_dumpable - Removed serialize_content_result from ag-ui/_utils.py - Updated _events.py and _message_adapters.py to use prepare_function_call_results from core package - Updated tests to match the core function's behavior * Fix failing tests for prepare_function_call_results behavior - test_tool_result_with_none: Update expected value to 'null' (JSON serialization of None) - test_tool_result_with_model_dump_objects: Use Pydantic BaseModel instead of plain class * Fix B903 linter error: Convert MockTextContent to dataclass The ruff linter was reporting B903 (class could be dataclass or namedtuple) for the MockTextContent test helper classes. This commit converts them to dataclasses to satisfy the linter check.
This commit is contained in:
committed by
GitHub
Unverified
parent
f49e537721
commit
db283cd396
@@ -31,6 +31,7 @@ from agent_framework import (
|
||||
FunctionCallContent,
|
||||
FunctionResultContent,
|
||||
TextContent,
|
||||
prepare_function_call_results,
|
||||
)
|
||||
|
||||
from ._utils import generate_event_id
|
||||
@@ -391,12 +392,7 @@ class AgentFrameworkEventBridge:
|
||||
self.state_delta_count = 0
|
||||
|
||||
result_message_id = generate_event_id()
|
||||
if isinstance(content.result, dict):
|
||||
result_content = json.dumps(content.result) # type: ignore[arg-type]
|
||||
elif content.result is not None:
|
||||
result_content = str(content.result)
|
||||
else:
|
||||
result_content = ""
|
||||
result_content = prepare_function_call_results(content.result)
|
||||
|
||||
result_event = ToolCallResultEvent(
|
||||
message_id=result_message_id,
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
"""Message format conversion between AG-UI and Agent Framework."""
|
||||
|
||||
import json
|
||||
from typing import Any, cast
|
||||
|
||||
from agent_framework import (
|
||||
@@ -11,6 +12,7 @@ from agent_framework import (
|
||||
FunctionResultContent,
|
||||
Role,
|
||||
TextContent,
|
||||
prepare_function_call_results,
|
||||
)
|
||||
|
||||
# Role mapping constants
|
||||
@@ -59,10 +61,8 @@ def agui_messages_to_agent_framework(messages: list[dict[str, Any]]) -> list[Cha
|
||||
# Distinguish approval payloads from actual tool results
|
||||
is_approval = False
|
||||
if isinstance(result_content, str) and result_content:
|
||||
import json as _json
|
||||
|
||||
try:
|
||||
parsed = _json.loads(result_content)
|
||||
parsed = json.loads(result_content)
|
||||
is_approval = isinstance(parsed, dict) and "accepted" in parsed
|
||||
except Exception:
|
||||
is_approval = False
|
||||
@@ -237,13 +237,8 @@ def agent_framework_messages_to_agui(messages: list[ChatMessage] | list[dict[str
|
||||
elif isinstance(content, FunctionResultContent):
|
||||
# Tool result content - extract call_id and result
|
||||
tool_result_call_id = content.call_id
|
||||
# Serialize result to string
|
||||
if isinstance(content.result, dict):
|
||||
import json
|
||||
|
||||
content_text = json.dumps(content.result) # type: ignore
|
||||
elif content.result is not None:
|
||||
content_text = str(content.result)
|
||||
# Serialize result to string using core utility
|
||||
content_text = prepare_function_call_results(content.result)
|
||||
|
||||
agui_msg: dict[str, Any] = {
|
||||
"id": msg.message_id if msg.message_id else generate_event_id(), # Always include id
|
||||
|
||||
@@ -201,7 +201,8 @@ async def test_tool_result_with_none():
|
||||
assert len(events) == 2
|
||||
assert events[0].type == "TOOL_CALL_END"
|
||||
assert events[1].type == "TOOL_CALL_RESULT"
|
||||
assert events[1].content == ""
|
||||
# prepare_function_call_results serializes None as JSON "null"
|
||||
assert events[1].content == "null"
|
||||
|
||||
|
||||
async def test_multiple_tool_results_in_sequence():
|
||||
@@ -688,3 +689,97 @@ async def test_state_delta_count_logging():
|
||||
|
||||
# State delta count should have incremented (one per unique state update)
|
||||
assert bridge.state_delta_count >= 1
|
||||
|
||||
|
||||
# Tests for list type tool results (MCP tool serialization)
|
||||
|
||||
|
||||
async def test_tool_result_with_empty_list():
|
||||
"""Test FunctionResultContent with empty list result."""
|
||||
from agent_framework_ag_ui._events import AgentFrameworkEventBridge
|
||||
|
||||
bridge = AgentFrameworkEventBridge(run_id="test_run", thread_id="test_thread")
|
||||
|
||||
update = AgentRunResponseUpdate(contents=[FunctionResultContent(call_id="call_123", result=[])])
|
||||
events = await bridge.from_agent_run_update(update)
|
||||
|
||||
assert len(events) == 2
|
||||
assert events[0].type == "TOOL_CALL_END"
|
||||
assert events[1].type == "TOOL_CALL_RESULT"
|
||||
# Empty list serializes as JSON empty array
|
||||
assert events[1].content == "[]"
|
||||
|
||||
|
||||
async def test_tool_result_with_single_text_content():
|
||||
"""Test FunctionResultContent with single TextContent-like item (MCP tool result)."""
|
||||
from dataclasses import dataclass
|
||||
|
||||
from agent_framework_ag_ui._events import AgentFrameworkEventBridge
|
||||
|
||||
@dataclass
|
||||
class MockTextContent:
|
||||
text: str
|
||||
|
||||
bridge = AgentFrameworkEventBridge(run_id="test_run", thread_id="test_thread")
|
||||
|
||||
update = AgentRunResponseUpdate(
|
||||
contents=[FunctionResultContent(call_id="call_123", result=[MockTextContent("Hello from MCP tool!")])]
|
||||
)
|
||||
events = await bridge.from_agent_run_update(update)
|
||||
|
||||
assert len(events) == 2
|
||||
assert events[0].type == "TOOL_CALL_END"
|
||||
assert events[1].type == "TOOL_CALL_RESULT"
|
||||
# TextContent text is extracted and serialized as JSON array
|
||||
assert events[1].content == '["Hello from MCP tool!"]'
|
||||
|
||||
|
||||
async def test_tool_result_with_multiple_text_contents():
|
||||
"""Test FunctionResultContent with multiple TextContent-like items (MCP tool result)."""
|
||||
from dataclasses import dataclass
|
||||
|
||||
from agent_framework_ag_ui._events import AgentFrameworkEventBridge
|
||||
|
||||
@dataclass
|
||||
class MockTextContent:
|
||||
text: str
|
||||
|
||||
bridge = AgentFrameworkEventBridge(run_id="test_run", thread_id="test_thread")
|
||||
|
||||
update = AgentRunResponseUpdate(
|
||||
contents=[
|
||||
FunctionResultContent(
|
||||
call_id="call_123",
|
||||
result=[MockTextContent("First result"), MockTextContent("Second result")],
|
||||
)
|
||||
]
|
||||
)
|
||||
events = await bridge.from_agent_run_update(update)
|
||||
|
||||
assert len(events) == 2
|
||||
assert events[0].type == "TOOL_CALL_END"
|
||||
assert events[1].type == "TOOL_CALL_RESULT"
|
||||
# Multiple TextContent items should return JSON array
|
||||
assert events[1].content == '["First result", "Second result"]'
|
||||
|
||||
|
||||
async def test_tool_result_with_model_dump_objects():
|
||||
"""Test FunctionResultContent with Pydantic BaseModel objects."""
|
||||
from pydantic import BaseModel
|
||||
|
||||
from agent_framework_ag_ui._events import AgentFrameworkEventBridge
|
||||
|
||||
class MockModel(BaseModel):
|
||||
value: int
|
||||
|
||||
bridge = AgentFrameworkEventBridge(run_id="test_run", thread_id="test_thread")
|
||||
|
||||
update = AgentRunResponseUpdate(
|
||||
contents=[FunctionResultContent(call_id="call_123", result=[MockModel(value=1), MockModel(value=2)])]
|
||||
)
|
||||
events = await bridge.from_agent_run_update(update)
|
||||
|
||||
assert len(events) == 2
|
||||
assert events[1].type == "TOOL_CALL_RESULT"
|
||||
# Should be properly serialized JSON array without double escaping
|
||||
assert events[1].content == '[{"value": 1}, {"value": 2}]'
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
"""Tests for message adapters."""
|
||||
|
||||
import pytest
|
||||
from agent_framework import ChatMessage, FunctionCallContent, Role, TextContent
|
||||
from agent_framework import ChatMessage, FunctionCallContent, FunctionResultContent, Role, TextContent
|
||||
|
||||
from agent_framework_ag_ui._message_adapters import (
|
||||
agent_framework_messages_to_agui,
|
||||
@@ -278,3 +278,119 @@ def test_extract_text_from_custom_contents():
|
||||
result = extract_text_from_contents(contents)
|
||||
|
||||
assert result == "Custom Mixed"
|
||||
|
||||
|
||||
# Tests for FunctionResultContent serialization in agent_framework_messages_to_agui
|
||||
|
||||
|
||||
def test_agent_framework_to_agui_function_result_dict():
|
||||
"""Test converting FunctionResultContent with dict result to AG-UI."""
|
||||
msg = ChatMessage(
|
||||
role=Role.TOOL,
|
||||
contents=[FunctionResultContent(call_id="call-123", result={"key": "value", "count": 42})],
|
||||
message_id="msg-789",
|
||||
)
|
||||
|
||||
messages = agent_framework_messages_to_agui([msg])
|
||||
|
||||
assert len(messages) == 1
|
||||
agui_msg = messages[0]
|
||||
assert agui_msg["role"] == "tool"
|
||||
assert agui_msg["toolCallId"] == "call-123"
|
||||
assert agui_msg["content"] == '{"key": "value", "count": 42}'
|
||||
|
||||
|
||||
def test_agent_framework_to_agui_function_result_none():
|
||||
"""Test converting FunctionResultContent with None result to AG-UI."""
|
||||
msg = ChatMessage(
|
||||
role=Role.TOOL,
|
||||
contents=[FunctionResultContent(call_id="call-123", result=None)],
|
||||
message_id="msg-789",
|
||||
)
|
||||
|
||||
messages = agent_framework_messages_to_agui([msg])
|
||||
|
||||
assert len(messages) == 1
|
||||
agui_msg = messages[0]
|
||||
# None serializes as JSON null
|
||||
assert agui_msg["content"] == "null"
|
||||
|
||||
|
||||
def test_agent_framework_to_agui_function_result_string():
|
||||
"""Test converting FunctionResultContent with string result to AG-UI."""
|
||||
msg = ChatMessage(
|
||||
role=Role.TOOL,
|
||||
contents=[FunctionResultContent(call_id="call-123", result="plain text result")],
|
||||
message_id="msg-789",
|
||||
)
|
||||
|
||||
messages = agent_framework_messages_to_agui([msg])
|
||||
|
||||
assert len(messages) == 1
|
||||
agui_msg = messages[0]
|
||||
assert agui_msg["content"] == "plain text result"
|
||||
|
||||
|
||||
def test_agent_framework_to_agui_function_result_empty_list():
|
||||
"""Test converting FunctionResultContent with empty list result to AG-UI."""
|
||||
msg = ChatMessage(
|
||||
role=Role.TOOL,
|
||||
contents=[FunctionResultContent(call_id="call-123", result=[])],
|
||||
message_id="msg-789",
|
||||
)
|
||||
|
||||
messages = agent_framework_messages_to_agui([msg])
|
||||
|
||||
assert len(messages) == 1
|
||||
agui_msg = messages[0]
|
||||
# Empty list serializes as JSON empty array
|
||||
assert agui_msg["content"] == "[]"
|
||||
|
||||
|
||||
def test_agent_framework_to_agui_function_result_single_text_content():
|
||||
"""Test converting FunctionResultContent with single TextContent-like item."""
|
||||
from dataclasses import dataclass
|
||||
|
||||
@dataclass
|
||||
class MockTextContent:
|
||||
text: str
|
||||
|
||||
msg = ChatMessage(
|
||||
role=Role.TOOL,
|
||||
contents=[FunctionResultContent(call_id="call-123", result=[MockTextContent("Hello from MCP!")])],
|
||||
message_id="msg-789",
|
||||
)
|
||||
|
||||
messages = agent_framework_messages_to_agui([msg])
|
||||
|
||||
assert len(messages) == 1
|
||||
agui_msg = messages[0]
|
||||
# TextContent text is extracted and serialized as JSON array
|
||||
assert agui_msg["content"] == '["Hello from MCP!"]'
|
||||
|
||||
|
||||
def test_agent_framework_to_agui_function_result_multiple_text_contents():
|
||||
"""Test converting FunctionResultContent with multiple TextContent-like items."""
|
||||
from dataclasses import dataclass
|
||||
|
||||
@dataclass
|
||||
class MockTextContent:
|
||||
text: str
|
||||
|
||||
msg = ChatMessage(
|
||||
role=Role.TOOL,
|
||||
contents=[
|
||||
FunctionResultContent(
|
||||
call_id="call-123",
|
||||
result=[MockTextContent("First result"), MockTextContent("Second result")],
|
||||
)
|
||||
],
|
||||
message_id="msg-789",
|
||||
)
|
||||
|
||||
messages = agent_framework_messages_to_agui([msg])
|
||||
|
||||
assert len(messages) == 1
|
||||
agui_msg = messages[0]
|
||||
# Multiple items should return JSON array
|
||||
assert agui_msg["content"] == '["First result", "Second result"]'
|
||||
|
||||
@@ -5,7 +5,11 @@
|
||||
from dataclasses import dataclass
|
||||
from datetime import date, datetime
|
||||
|
||||
from agent_framework_ag_ui._utils import generate_event_id, make_json_safe, merge_state
|
||||
from agent_framework_ag_ui._utils import (
|
||||
generate_event_id,
|
||||
make_json_safe,
|
||||
merge_state,
|
||||
)
|
||||
|
||||
|
||||
def test_generate_event_id():
|
||||
|
||||
@@ -1869,6 +1869,9 @@ def _prepare_function_call_results_as_dumpable(content: Contents | Any | list[Co
|
||||
return content.model_dump()
|
||||
if hasattr(content, "to_dict"):
|
||||
return content.to_dict(exclude={"raw_representation", "additional_properties"})
|
||||
# Handle objects with text attribute (e.g., MCP TextContent)
|
||||
if hasattr(content, "text") and isinstance(content.text, str):
|
||||
return content.text
|
||||
return content
|
||||
|
||||
|
||||
|
||||
@@ -2133,3 +2133,55 @@ def test_prepare_function_call_results_nested_pydantic_model():
|
||||
assert "Seattle" in json_result
|
||||
assert "rainy" in json_result
|
||||
assert "18.0" in json_result or "18" in json_result
|
||||
|
||||
|
||||
# region prepare_function_call_results with MCP TextContent-like objects
|
||||
|
||||
|
||||
def test_prepare_function_call_results_text_content_single():
|
||||
"""Test that objects with text attribute (like MCP TextContent) are properly handled."""
|
||||
from dataclasses import dataclass
|
||||
|
||||
@dataclass
|
||||
class MockTextContent:
|
||||
text: str
|
||||
|
||||
result = [MockTextContent("Hello from MCP tool!")]
|
||||
json_result = prepare_function_call_results(result)
|
||||
|
||||
# Should extract text and serialize as JSON array of strings
|
||||
assert isinstance(json_result, str)
|
||||
assert json_result == '["Hello from MCP tool!"]'
|
||||
|
||||
|
||||
def test_prepare_function_call_results_text_content_multiple():
|
||||
"""Test that multiple TextContent-like objects are serialized correctly."""
|
||||
from dataclasses import dataclass
|
||||
|
||||
@dataclass
|
||||
class MockTextContent:
|
||||
text: str
|
||||
|
||||
result = [MockTextContent("First result"), MockTextContent("Second result")]
|
||||
json_result = prepare_function_call_results(result)
|
||||
|
||||
# Should extract text from each and serialize as JSON array
|
||||
assert isinstance(json_result, str)
|
||||
assert json_result == '["First result", "Second result"]'
|
||||
|
||||
|
||||
def test_prepare_function_call_results_text_content_with_non_string_text():
|
||||
"""Test that objects with non-string text attribute are not treated as TextContent."""
|
||||
|
||||
class BadTextContent:
|
||||
def __init__(self):
|
||||
self.text = 12345 # Not a string!
|
||||
|
||||
result = [BadTextContent()]
|
||||
json_result = prepare_function_call_results(result)
|
||||
|
||||
# Should not extract text since it's not a string, will serialize the object
|
||||
assert isinstance(json_result, str)
|
||||
|
||||
|
||||
# endregion
|
||||
|
||||
Reference in New Issue
Block a user