Python: Fix Foundry reasoning MCP compaction (#6907)
* Fix Foundry reasoning MCP compaction * Address reasoning MCP review feedback --------- Co-authored-by: godququ5-code <256881196+godququ5-code@users.noreply.github.com> Co-authored-by: Eduard van Valkenburg <eavanvalkenburg@users.noreply.github.com>
This commit is contained in:
@@ -921,6 +921,9 @@ class RawAnthropicClient(
|
||||
):
|
||||
a_content[-1]["signature"] = content.protected_data
|
||||
continue
|
||||
if content.id and not content.protected_data:
|
||||
a_content.append({"type": "text", "text": content.text})
|
||||
continue
|
||||
thinking_block: dict[str, Any] = {"type": "thinking", "thinking": content.text}
|
||||
if content.protected_data:
|
||||
thinking_block["signature"] = content.protected_data
|
||||
|
||||
@@ -496,6 +496,20 @@ def test_prepare_message_for_anthropic_text_reasoning_with_signature(
|
||||
assert result["content"][0]["signature"] == "sig_abc123"
|
||||
|
||||
|
||||
def test_prepare_message_for_anthropic_provider_reasoning_without_signature_is_text(
|
||||
mock_anthropic_client: MagicMock,
|
||||
) -> None:
|
||||
client = create_test_anthropic_client(mock_anthropic_client)
|
||||
message = Message(
|
||||
role="assistant",
|
||||
contents=[Content.from_text_reasoning(id="rs_abc123", text="Foundry summary")],
|
||||
)
|
||||
|
||||
result = client._prepare_message_for_anthropic(message)
|
||||
|
||||
assert result["content"] == [{"type": "text", "text": "Foundry summary"}]
|
||||
|
||||
|
||||
def test_prepare_message_for_anthropic_attaches_signature_only_reasoning(
|
||||
mock_anthropic_client: MagicMock,
|
||||
) -> None:
|
||||
|
||||
@@ -12,6 +12,7 @@ from typing import (
|
||||
Literal,
|
||||
Protocol,
|
||||
TypeAlias,
|
||||
cast,
|
||||
runtime_checkable,
|
||||
)
|
||||
|
||||
@@ -37,6 +38,14 @@ SUMMARIZED_BY_SUMMARY_ID_KEY = "_summarized_by_summary_id"
|
||||
|
||||
logger = logging.getLogger("agent_framework")
|
||||
|
||||
_TOOL_CALL_CONTENT_TYPES: Final[set[str]] = {
|
||||
"function_call",
|
||||
"mcp_server_tool_call",
|
||||
"code_interpreter_tool_call",
|
||||
"shell_tool_call",
|
||||
"image_generation_tool_call",
|
||||
}
|
||||
|
||||
|
||||
@runtime_checkable
|
||||
class TokenizerProtocol(Protocol):
|
||||
@@ -74,8 +83,8 @@ def _has_content_type(message: Message, content_type: str) -> bool:
|
||||
return any(content.type == content_type for content in message.contents)
|
||||
|
||||
|
||||
def _has_function_call(message: Message) -> bool:
|
||||
return _has_content_type(message, "function_call")
|
||||
def _has_tool_call(message: Message) -> bool:
|
||||
return any(content.type in _TOOL_CALL_CONTENT_TYPES for content in message.contents)
|
||||
|
||||
|
||||
def _has_reasoning(message: Message) -> bool:
|
||||
@@ -83,7 +92,7 @@ def _has_reasoning(message: Message) -> bool:
|
||||
|
||||
|
||||
def _is_tool_call_assistant(message: Message) -> bool:
|
||||
return message.role == "assistant" and _has_function_call(message)
|
||||
return message.role == "assistant" and _has_tool_call(message)
|
||||
|
||||
|
||||
def _is_reasoning_only_assistant(message: Message) -> bool:
|
||||
@@ -876,6 +885,8 @@ class ToolResultCompactionStrategy:
|
||||
for content in msg.contents:
|
||||
if content.type == "function_call" and content.call_id and content.name:
|
||||
call_id_to_name[content.call_id] = content.name
|
||||
elif content.type == "mcp_server_tool_call" and content.call_id and content.tool_name:
|
||||
call_id_to_name[content.call_id] = content.tool_name
|
||||
# Collect tool results with the function name for context.
|
||||
tool_results: list[str] = []
|
||||
for msg in group_msgs:
|
||||
@@ -885,6 +896,11 @@ class ToolResultCompactionStrategy:
|
||||
func_name = call_id_to_name.get(content.call_id or "", "")
|
||||
label = f"{func_name}: {result_text}" if func_name else result_text
|
||||
tool_results.append(label.strip())
|
||||
elif content.type == "mcp_server_tool_result":
|
||||
result_text = _tool_result_text(content.output)
|
||||
tool_name = call_id_to_name.get(content.call_id or "", "")
|
||||
label = f"{tool_name}: {result_text}" if tool_name else result_text
|
||||
tool_results.append(label.strip())
|
||||
summary_label = "; ".join(tool_results) if tool_results else "no results"
|
||||
summary_text = f"[Tool results: {summary_label}]"
|
||||
|
||||
@@ -918,6 +934,26 @@ class ToolResultCompactionStrategy:
|
||||
return changed
|
||||
|
||||
|
||||
def _tool_result_text(value: Any) -> str:
|
||||
if isinstance(value, str):
|
||||
return value
|
||||
if isinstance(value, Sequence) and not isinstance(value, (bytes, bytearray, str)):
|
||||
text_parts: list[str] = []
|
||||
for item in cast(Sequence[object], value):
|
||||
if isinstance(item, Content) and item.type == "text" and item.text is not None:
|
||||
text_parts.append(item.text)
|
||||
elif isinstance(item, Mapping):
|
||||
item_mapping = cast(Mapping[str, object], item)
|
||||
text = item_mapping.get("text")
|
||||
if item_mapping.get("type") == "text" and isinstance(text, str):
|
||||
text_parts.append(text)
|
||||
if text_parts:
|
||||
return "\n".join(text_parts)
|
||||
if isinstance(value, Mapping):
|
||||
return json.dumps(cast(Mapping[str, object], value), ensure_ascii=False)
|
||||
return str(cast(object, value))
|
||||
|
||||
|
||||
def _format_messages_for_summary(messages: list[Message]) -> str:
|
||||
lines: list[str] = []
|
||||
for index, message in enumerate(messages, start=1):
|
||||
|
||||
@@ -5,6 +5,8 @@ from __future__ import annotations
|
||||
import logging
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
|
||||
from agent_framework import (
|
||||
EXCLUDED_KEY,
|
||||
GROUP_ANNOTATION_KEY,
|
||||
@@ -45,6 +47,35 @@ def _assistant_function_call(call_id: str) -> Message:
|
||||
)
|
||||
|
||||
|
||||
def _assistant_mcp_call(call_id: str) -> Message:
|
||||
return Message(
|
||||
role="assistant",
|
||||
contents=[
|
||||
Content.from_mcp_server_tool_call(
|
||||
call_id=call_id,
|
||||
tool_name="search",
|
||||
server_name="test_server",
|
||||
arguments='{"query":"x"}',
|
||||
)
|
||||
],
|
||||
)
|
||||
|
||||
|
||||
def _assistant_mcp_call_with_result(call_id: str, output: str) -> Message:
|
||||
return Message(
|
||||
role="assistant",
|
||||
contents=[
|
||||
Content.from_mcp_server_tool_call(
|
||||
call_id=call_id,
|
||||
tool_name="search",
|
||||
server_name="test_server",
|
||||
arguments='{"query":"x"}',
|
||||
),
|
||||
Content.from_mcp_server_tool_result(call_id=call_id, output=[Content.from_text(output)]),
|
||||
],
|
||||
)
|
||||
|
||||
|
||||
def _assistant_reasoning_and_function_calls(*call_ids: str) -> Message:
|
||||
contents: list[Content] = [Content.from_text_reasoning(text="thinking")]
|
||||
for call_id in call_ids:
|
||||
@@ -154,6 +185,45 @@ def test_group_annotations_handle_same_message_reasoning_and_function_calls() ->
|
||||
assert _group_has_reasoning(messages[1]) is True
|
||||
|
||||
|
||||
async def test_sliding_window_keeps_reasoning_and_mcp_call_atomic() -> None:
|
||||
messages = [
|
||||
Message(role="system", contents=["system"]),
|
||||
Message(role="assistant", contents=[Content.from_text_reasoning(id="rs_1", text="thinking")]),
|
||||
_assistant_mcp_call("mcp_1"),
|
||||
Message(role="assistant", contents=["answer"]),
|
||||
Message(role="user", contents=["follow up"]),
|
||||
]
|
||||
annotate_message_groups(messages)
|
||||
|
||||
await SlidingWindowStrategy(keep_last_groups=3)(messages)
|
||||
|
||||
assert messages[1].additional_properties[EXCLUDED_KEY] is False
|
||||
assert messages[2].additional_properties[EXCLUDED_KEY] is False
|
||||
assert _group_id(messages[1]) == _group_id(messages[2])
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"tool_call",
|
||||
[
|
||||
Content.from_code_interpreter_tool_call(call_id="ci_1"),
|
||||
Content.from_shell_tool_call(call_id="sh_1", commands=["echo hi"]),
|
||||
Content.from_image_generation_tool_call(image_id="img_1"),
|
||||
],
|
||||
)
|
||||
def test_group_annotations_keep_reasoning_with_hosted_tool_calls(tool_call: Content) -> None:
|
||||
messages = [
|
||||
Message(role="assistant", contents=[Content.from_text_reasoning(id="rs_1", text="thinking")]),
|
||||
Message(role="assistant", contents=[tool_call]),
|
||||
Message(role="assistant", contents=["answer"]),
|
||||
]
|
||||
|
||||
annotate_message_groups(messages)
|
||||
|
||||
assert _group_id(messages[0]) == _group_id(messages[1])
|
||||
assert _group_kind(messages[0]) == "tool_call"
|
||||
assert _group_has_reasoning(messages[0]) is True
|
||||
|
||||
|
||||
def test_annotate_message_groups_with_tokenizer_adds_token_counts() -> None:
|
||||
messages = [
|
||||
Message(role="user", contents=["hello"]),
|
||||
@@ -711,6 +781,24 @@ async def test_tool_result_compaction_summary_has_full_annotations() -> None:
|
||||
assert summary.additional_properties.get(EXCLUDED_KEY) is False
|
||||
|
||||
|
||||
async def test_tool_result_compaction_summarizes_mcp_tool_results() -> None:
|
||||
messages = [
|
||||
Message(role="user", contents=["hello"]),
|
||||
_assistant_mcp_call_with_result("mcp_1", "found 10 cats"),
|
||||
Message(role="assistant", contents=["I found cats."]),
|
||||
_assistant_function_call("c1"),
|
||||
_tool_result("c1", "new result"),
|
||||
]
|
||||
annotate_message_groups(messages)
|
||||
|
||||
changed = await ToolResultCompactionStrategy(keep_last_tool_call_groups=1)(messages)
|
||||
|
||||
assert changed is True
|
||||
projected = included_messages(messages)
|
||||
summary = next(m for m in projected if (m.text or "").startswith("[Tool results:"))
|
||||
assert summary.text == "[Tool results: search: found 10 cats]"
|
||||
|
||||
|
||||
async def test_summarization_strategy_summary_has_full_annotations() -> None:
|
||||
"""Summary messages inserted by SummarizationStrategy must have all compaction annotations."""
|
||||
messages = [
|
||||
|
||||
@@ -880,13 +880,13 @@ class _OutputItemTracker:
|
||||
if self._text_content is not None:
|
||||
yield self._text_content.emit_delta(content.text)
|
||||
|
||||
elif content.type == "text_reasoning" and content.text is not None:
|
||||
elif content.type == "text_reasoning":
|
||||
if self._active_type != "text_reasoning":
|
||||
yield from self._close()
|
||||
yield from self._open_reasoning()
|
||||
self._accumulated.append(content.text)
|
||||
self._accumulated.append(content.text or "")
|
||||
if self._summary_part is not None:
|
||||
yield self._summary_part.emit_text_delta(content.text)
|
||||
yield self._summary_part.emit_text_delta(content.text or "")
|
||||
|
||||
elif content.type == "function_call" and content.call_id is not None:
|
||||
if self._active_type != "function_call" or self._active_id != content.call_id:
|
||||
@@ -1116,7 +1116,9 @@ async def _item_to_message(item: Item, *, approval_storage: ApprovalStorage | No
|
||||
reason_contents: list[Content] = []
|
||||
if reasoning.summary:
|
||||
for summary in reasoning.summary:
|
||||
reason_contents.append(Content.from_text(summary.text))
|
||||
reason_contents.append(Content.from_text_reasoning(id=reasoning.id, text=summary.text))
|
||||
else:
|
||||
reason_contents.append(Content.from_text_reasoning(id=reasoning.id))
|
||||
return Message(role="assistant", contents=reason_contents)
|
||||
|
||||
if item.type == "mcp_call":
|
||||
@@ -1405,7 +1407,9 @@ async def _output_item_to_message(item: OutputItem, *, approval_storage: Approva
|
||||
contents: list[Content] = []
|
||||
if reasoning.summary:
|
||||
for summary in reasoning.summary:
|
||||
contents.append(Content.from_text(summary.text))
|
||||
contents.append(Content.from_text_reasoning(id=reasoning.id, text=summary.text))
|
||||
else:
|
||||
contents.append(Content.from_text_reasoning(id=reasoning.id))
|
||||
return Message(role="assistant", contents=contents)
|
||||
|
||||
if item.type == "mcp_call":
|
||||
@@ -1786,8 +1790,8 @@ async def _to_outputs(
|
||||
if content.type == "text" and content.text is not None:
|
||||
async for event in stream.aoutput_item_message(content.text):
|
||||
yield event
|
||||
elif content.type == "text_reasoning" and content.text is not None:
|
||||
async for event in stream.aoutput_item_reasoning_item(content.text):
|
||||
elif content.type == "text_reasoning":
|
||||
async for event in stream.aoutput_item_reasoning_item(content.text or ""):
|
||||
yield event
|
||||
elif content.type == "function_call":
|
||||
async for event in stream.aoutput_item_function_call(
|
||||
|
||||
@@ -814,6 +814,8 @@ class TestOutputItemToMessage:
|
||||
msg = await _output_item_to_message(item)
|
||||
assert msg.role == "assistant"
|
||||
assert len(msg.contents) == 1
|
||||
assert msg.contents[0].type == "text_reasoning"
|
||||
assert msg.contents[0].id == "r-1"
|
||||
assert msg.contents[0].text == "thinking hard"
|
||||
|
||||
async def test_reasoning_no_summary(self) -> None:
|
||||
@@ -822,7 +824,10 @@ class TestOutputItemToMessage:
|
||||
item = OutputItemReasoningItem({"type": "reasoning", "id": "r-2"})
|
||||
msg = await _output_item_to_message(item)
|
||||
assert msg.role == "assistant"
|
||||
assert msg.contents == []
|
||||
assert len(msg.contents) == 1
|
||||
assert msg.contents[0].type == "text_reasoning"
|
||||
assert msg.contents[0].id == "r-2"
|
||||
assert msg.contents[0].text is None
|
||||
|
||||
async def test_mcp_call(self) -> None:
|
||||
from azure.ai.agentserver.responses.models import OutputItemMcpToolCall
|
||||
@@ -1305,6 +1310,8 @@ class TestItemToMessage:
|
||||
assert msg is not None
|
||||
assert msg.role == "assistant"
|
||||
assert len(msg.contents) == 1
|
||||
assert msg.contents[0].type == "text_reasoning"
|
||||
assert msg.contents[0].id == "r-1"
|
||||
assert msg.contents[0].text == "thinking hard"
|
||||
|
||||
async def test_reasoning_no_summary(self) -> None:
|
||||
@@ -1314,7 +1321,10 @@ class TestItemToMessage:
|
||||
msg = await _item_to_message(item)
|
||||
assert msg is not None
|
||||
assert msg.role == "assistant"
|
||||
assert msg.contents == []
|
||||
assert len(msg.contents) == 1
|
||||
assert msg.contents[0].type == "text_reasoning"
|
||||
assert msg.contents[0].id == "r-2"
|
||||
assert msg.contents[0].text is None
|
||||
|
||||
async def test_mcp_call(self) -> None:
|
||||
from azure.ai.agentserver.responses.models import ItemMcpToolCall
|
||||
|
||||
@@ -133,6 +133,34 @@ _AZURE_AI_SEARCH_OUTPUT_EVENT_PREFIX = "response.azure_ai_search_call_output."
|
||||
_AF_MCP_PENDING_OUTPUT_KEY = "__af_pending_mcp_result__"
|
||||
|
||||
|
||||
def _mcp_call_ids_paired_with_reasoning(messages: Sequence[Message]) -> set[str]:
|
||||
paired_call_ids: set[str] = set()
|
||||
pending_reasoning_prefix = False
|
||||
|
||||
for message in messages:
|
||||
has_reasoning = any(content.type == "text_reasoning" for content in message.contents)
|
||||
has_mcp_call = False
|
||||
for content in message.contents:
|
||||
if content.type != "mcp_server_tool_call":
|
||||
continue
|
||||
has_mcp_call = True
|
||||
if (has_reasoning or pending_reasoning_prefix) and content.call_id:
|
||||
paired_call_ids.add(content.call_id)
|
||||
|
||||
if has_mcp_call:
|
||||
pending_reasoning_prefix = False
|
||||
elif (
|
||||
message.role == "assistant"
|
||||
and message.contents
|
||||
and all(content.type == "text_reasoning" for content in message.contents)
|
||||
):
|
||||
pending_reasoning_prefix = True
|
||||
else:
|
||||
pending_reasoning_prefix = False
|
||||
|
||||
return paired_call_ids
|
||||
|
||||
|
||||
class OpenAIContinuationToken(ContinuationToken):
|
||||
"""Continuation token for OpenAI Responses API background operations."""
|
||||
|
||||
@@ -1482,10 +1510,7 @@ class RawOpenAIChatClient(
|
||||
)
|
||||
drop_mcp_call_ids: set[str] = set()
|
||||
if drops_reasoning_without_storage:
|
||||
for message in chat_messages:
|
||||
for content in message.contents:
|
||||
if content.type == "mcp_server_tool_call" and content.call_id:
|
||||
drop_mcp_call_ids.add(content.call_id)
|
||||
drop_mcp_call_ids = _mcp_call_ids_paired_with_reasoning(chat_messages)
|
||||
|
||||
list_of_list = [
|
||||
self._prepare_message_for_openai(
|
||||
|
||||
@@ -926,6 +926,10 @@ class RawOpenAIChatCompletionClient(
|
||||
case "text_reasoning" if (protected_data := content.protected_data) is not None:
|
||||
# Buffer reasoning to attach to the next message with content/tool_calls
|
||||
pending_reasoning = json.loads(protected_data)
|
||||
case "text_reasoning":
|
||||
if content.text is None:
|
||||
continue
|
||||
args["content"] = [{"type": "text", "text": content.text}]
|
||||
case _:
|
||||
if "content" not in args:
|
||||
args["content"] = []
|
||||
|
||||
@@ -6574,6 +6574,63 @@ def test_prepare_messages_for_openai_drops_mcp_call_across_reasoning_messages()
|
||||
assert "function_call_output" not in types
|
||||
|
||||
|
||||
def test_prepare_messages_for_openai_keeps_unpaired_mcp_when_reasoning_is_stripped() -> None:
|
||||
client = OpenAIChatClient(model="test-model", api_key="test-key")
|
||||
|
||||
messages = [
|
||||
Message(
|
||||
role="assistant",
|
||||
contents=[
|
||||
Content.from_mcp_server_tool_call(
|
||||
call_id="mcp_keep",
|
||||
tool_name="search",
|
||||
server_name="api_specs",
|
||||
arguments='{"q": "dogs"}',
|
||||
)
|
||||
],
|
||||
),
|
||||
Message(
|
||||
role="tool",
|
||||
contents=[
|
||||
Content.from_mcp_server_tool_result(
|
||||
call_id="mcp_keep",
|
||||
output=[Content.from_text(text="found 5 dogs")],
|
||||
)
|
||||
],
|
||||
),
|
||||
Message(
|
||||
role="assistant",
|
||||
contents=[Content.from_text_reasoning(id="rs_abc123", text="Need a tool call.")],
|
||||
),
|
||||
Message(
|
||||
role="assistant",
|
||||
contents=[
|
||||
Content.from_mcp_server_tool_call(
|
||||
call_id="mcp_drop",
|
||||
tool_name="search",
|
||||
server_name="api_specs",
|
||||
arguments='{"q": "cats"}',
|
||||
)
|
||||
],
|
||||
),
|
||||
Message(
|
||||
role="tool",
|
||||
contents=[
|
||||
Content.from_mcp_server_tool_result(
|
||||
call_id="mcp_drop",
|
||||
output=[Content.from_text(text="found 10 cats")],
|
||||
)
|
||||
],
|
||||
),
|
||||
]
|
||||
|
||||
result = client._prepare_messages_for_openai(messages, request_uses_service_side_storage=False)
|
||||
|
||||
mcp_items = [item for item in result if isinstance(item, dict) and item.get("type") == "mcp_call"]
|
||||
assert [item["id"] for item in mcp_items] == ["mcp_keep"]
|
||||
assert mcp_items[0]["output"] == "found 5 dogs"
|
||||
|
||||
|
||||
def test_prepare_messages_for_openai_drops_orphan_mcp_server_tool_result() -> None:
|
||||
"""When an mcp_server_tool_result has no matching mcp_server_tool_call in
|
||||
the message list, it must be dropped, NOT serialized as a
|
||||
|
||||
@@ -918,6 +918,20 @@ def test_prepare_message_with_text_reasoning_content(
|
||||
assert prepared[0]["content"] == "The answer is 42."
|
||||
|
||||
|
||||
def test_prepare_message_with_unprotected_text_reasoning_content(
|
||||
openai_unit_test_env: dict[str, str],
|
||||
) -> None:
|
||||
client = OpenAIChatCompletionClient()
|
||||
message = Message(
|
||||
role="assistant",
|
||||
contents=[Content.from_text_reasoning(id="rs_abc123", text="Foundry summary")],
|
||||
)
|
||||
|
||||
prepared = client._prepare_message_for_openai(message)
|
||||
|
||||
assert prepared == [{"role": "assistant", "content": "Foundry summary"}]
|
||||
|
||||
|
||||
def test_prepare_message_with_only_text_reasoning_content(
|
||||
openai_unit_test_env: dict[str, str],
|
||||
) -> None:
|
||||
|
||||
Reference in New Issue
Block a user