mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Python: Fix IndexError when reasoning models produce reasoning-only messages in Magentic-One workflow (#4413)
* Fix IndexError when reasoning models return no text content (#4384) In _prepare_message_for_openai(), the text_reasoning case unconditionally accessed all_messages[-1] to attach reasoning_details. When a reasoning model (e.g. gpt-5-mini) returns reasoning_details without text content, all_messages is empty, causing an IndexError. Guard the access by initializing all_messages with the current args dict when it is empty, so reasoning_details can be safely attached. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address review: buffer reasoning details for valid message payloads (#4384) - Buffer pending reasoning details and attach to the next message with content/tool_calls, avoiding standalone reasoning-only messages. - When reasoning is the only content, emit a message with empty content to satisfy Chat Completions schema requirements. - Strengthen test assertions to verify text+reasoning co-location and that all messages with reasoning_details also have content or tool_calls. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix text_reasoning handling: always buffer and tighten tests (#4384) - Always buffer reasoning into pending_reasoning instead of conditionally attaching to the previous message via fragile all_messages emptiness check - Attach buffered reasoning to last message at end-of-loop when no subsequent content consumed it - Assert exact content values (content == '' not in ('', None)) - Assert exact list lengths (== 1 not >= 1) for stronger regression guards - Add test for reasoning before FunctionCallContent Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Apply pre-commit auto-fixes --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
Unverified
parent
c37f74f898
commit
dae3caa719
@@ -548,6 +548,7 @@ class RawOpenAIChatClient( # type: ignore[misc]
|
||||
return []
|
||||
|
||||
all_messages: list[dict[str, Any]] = []
|
||||
pending_reasoning: Any = None
|
||||
for content in message.contents:
|
||||
# Skip approval content - it's internal framework state, not for the LLM
|
||||
if content.type in ("function_approval_request", "function_approval_response"):
|
||||
@@ -575,15 +576,33 @@ class RawOpenAIChatClient( # type: ignore[misc]
|
||||
# Functions returning None should still have a tool result message
|
||||
args["content"] = content.result if content.result is not None else ""
|
||||
case "text_reasoning" if (protected_data := content.protected_data) is not None:
|
||||
all_messages[-1]["reasoning_details"] = json.loads(protected_data)
|
||||
# Buffer reasoning to attach to the next message with content/tool_calls
|
||||
pending_reasoning = json.loads(protected_data)
|
||||
case _:
|
||||
if "content" not in args:
|
||||
args["content"] = []
|
||||
# this is a list to allow multi-modal content
|
||||
args["content"].append(self._prepare_content_for_openai(content)) # type: ignore
|
||||
if "content" in args or "tool_calls" in args:
|
||||
if pending_reasoning is not None:
|
||||
args["reasoning_details"] = pending_reasoning
|
||||
pending_reasoning = None
|
||||
all_messages.append(args)
|
||||
|
||||
# If reasoning was the only content, emit a valid message with empty content
|
||||
if pending_reasoning is not None:
|
||||
if all_messages:
|
||||
all_messages[-1]["reasoning_details"] = pending_reasoning
|
||||
else:
|
||||
pending_args: dict[str, Any] = {
|
||||
"role": message.role,
|
||||
"content": "",
|
||||
"reasoning_details": pending_reasoning,
|
||||
}
|
||||
if message.author_name and message.role != "tool":
|
||||
pending_args["name"] = message.author_name
|
||||
all_messages.append(pending_args)
|
||||
|
||||
# Flatten text-only content lists to plain strings for broader
|
||||
# compatibility with OpenAI-like endpoints (e.g. Foundry Local).
|
||||
# See https://github.com/microsoft/agent-framework/issues/4084
|
||||
|
||||
@@ -643,6 +643,110 @@ def test_prepare_message_with_text_reasoning_content(openai_unit_test_env: dict[
|
||||
assert prepared[0]["content"] == "The answer is 42."
|
||||
|
||||
|
||||
def test_prepare_message_with_only_text_reasoning_content(openai_unit_test_env: dict[str, str]) -> None:
|
||||
"""Test that a message with only text_reasoning content does not raise IndexError.
|
||||
|
||||
Regression test for https://github.com/microsoft/agent-framework/issues/4384
|
||||
Reasoning models (e.g. gpt-5-mini) may produce reasoning_details without text content,
|
||||
which previously caused an IndexError when preparing messages.
|
||||
"""
|
||||
client = OpenAIChatClient()
|
||||
|
||||
mock_reasoning_data = {
|
||||
"effort": "high",
|
||||
"summary": "Deep analysis of the problem",
|
||||
}
|
||||
|
||||
reasoning_content = Content.from_text_reasoning(text=None, protected_data=json.dumps(mock_reasoning_data))
|
||||
|
||||
# Message with only reasoning content and no text
|
||||
message = Message(
|
||||
role="assistant",
|
||||
contents=[reasoning_content],
|
||||
)
|
||||
|
||||
prepared = client._prepare_message_for_openai(message)
|
||||
|
||||
# Should have one message with reasoning_details
|
||||
assert len(prepared) == 1
|
||||
assert prepared[0]["role"] == "assistant"
|
||||
assert "reasoning_details" in prepared[0]
|
||||
assert prepared[0]["reasoning_details"] == mock_reasoning_data
|
||||
# Message should also include a content field to be a valid Chat Completions payload
|
||||
assert "content" in prepared[0]
|
||||
assert prepared[0]["content"] == ""
|
||||
|
||||
|
||||
def test_prepare_message_with_text_reasoning_before_text(openai_unit_test_env: dict[str, str]) -> None:
|
||||
"""Test that text_reasoning content appearing before text content is handled correctly.
|
||||
|
||||
Regression test for https://github.com/microsoft/agent-framework/issues/4384
|
||||
"""
|
||||
client = OpenAIChatClient()
|
||||
|
||||
mock_reasoning_data = {
|
||||
"effort": "medium",
|
||||
"summary": "Quick analysis",
|
||||
}
|
||||
|
||||
reasoning_content = Content.from_text_reasoning(text=None, protected_data=json.dumps(mock_reasoning_data))
|
||||
|
||||
# Reasoning appears before text content
|
||||
message = Message(
|
||||
role="assistant",
|
||||
contents=[
|
||||
reasoning_content,
|
||||
Content.from_text(text="The answer is 42."),
|
||||
],
|
||||
)
|
||||
|
||||
prepared = client._prepare_message_for_openai(message)
|
||||
|
||||
# Should produce exactly one message without raising IndexError
|
||||
assert len(prepared) == 1
|
||||
|
||||
# Reasoning details should be present on the message
|
||||
assert "reasoning_details" in prepared[0]
|
||||
assert prepared[0]["reasoning_details"] == mock_reasoning_data
|
||||
assert prepared[0]["content"] == "The answer is 42."
|
||||
|
||||
|
||||
def test_prepare_message_with_text_reasoning_before_function_call(openai_unit_test_env: dict[str, str]) -> None:
|
||||
"""Test that text_reasoning content appearing before a function call is handled correctly.
|
||||
|
||||
Regression test for https://github.com/microsoft/agent-framework/issues/4384
|
||||
"""
|
||||
client = OpenAIChatClient()
|
||||
|
||||
mock_reasoning_data = {
|
||||
"effort": "medium",
|
||||
"summary": "Deciding to call a function",
|
||||
}
|
||||
|
||||
reasoning_content = Content.from_text_reasoning(text=None, protected_data=json.dumps(mock_reasoning_data))
|
||||
|
||||
# Reasoning appears before function call content
|
||||
message = Message(
|
||||
role="assistant",
|
||||
contents=[
|
||||
reasoning_content,
|
||||
Content.from_function_call(call_id="call_abc", name="get_weather", arguments='{"city": "Seattle"}'),
|
||||
],
|
||||
)
|
||||
|
||||
prepared = client._prepare_message_for_openai(message)
|
||||
|
||||
# Should produce exactly one message
|
||||
assert len(prepared) == 1
|
||||
|
||||
# The message should carry the reasoning details and tool_calls
|
||||
assert "reasoning_details" in prepared[0]
|
||||
assert prepared[0]["reasoning_details"] == mock_reasoning_data
|
||||
assert "tool_calls" in prepared[0]
|
||||
assert prepared[0]["tool_calls"][0]["function"]["name"] == "get_weather"
|
||||
assert prepared[0]["role"] == "assistant"
|
||||
|
||||
|
||||
def test_function_approval_content_is_skipped_in_preparation(openai_unit_test_env: dict[str, str]) -> None:
|
||||
"""Test that function approval request and response content are skipped."""
|
||||
client = OpenAIChatClient()
|
||||
|
||||
Reference in New Issue
Block a user