diff --git a/python/packages/openai/agent_framework_openai/_chat_completion_client.py b/python/packages/openai/agent_framework_openai/_chat_completion_client.py index 2da59e031e..fa10503e95 100644 --- a/python/packages/openai/agent_framework_openai/_chat_completion_client.py +++ b/python/packages/openai/agent_framework_openai/_chat_completion_client.py @@ -159,6 +159,10 @@ class OpenAIChatCompletionOptions(ChatOptions[ResponseModelT], Generic[ResponseM "max_tokens": "max_completion_tokens", } +REASONING_DETAILS_FIELD = "reasoning_details" +REASONING_CONTENT_FIELD = "reasoning_content" +REASONING_FORMAT_KEY = "openai_reasoning_format" + # region Base Client class RawOpenAIChatCompletionClient( # type: ignore[misc] @@ -687,8 +691,8 @@ def _parse_response_from_openai(self, response: ChatCompletion, options: Mapping contents.append(text_content) if parsed_tool_calls := [tool for tool in self._parse_tool_calls_from_openai(choice)]: contents.extend(parsed_tool_calls) - if reasoning_details := getattr(choice.message, "reasoning_details", None): - contents.append(Content.from_text_reasoning(protected_data=json.dumps(reasoning_details))) + if reasoning_content := self._parse_reasoning_from_choice_message(choice): + contents.append(reasoning_content) messages.append(Message(role="assistant", contents=contents)) return ChatResponse( response_id=response.id, @@ -725,8 +729,8 @@ def _parse_response_update_from_openai( if text_content := self._parse_text_from_openai(choice): contents.append(text_content) - if reasoning_details := getattr(choice.delta, "reasoning_details", None): - contents.append(Content.from_text_reasoning(protected_data=json.dumps(reasoning_details))) + if reasoning_content := self._parse_reasoning_from_choice_delta(choice): + contents.append(reasoning_content) return ChatResponseUpdate( created_at=datetime.fromtimestamp(chunk.created, tz=timezone.utc).strftime("%Y-%m-%dT%H:%M:%S.%fZ"), contents=contents, @@ -770,6 +774,28 @@ def _parse_text_from_openai(self, choice: Choice | ChunkChoice) -> Content | Non return Content.from_text(text=message.refusal, raw_representation=choice) return None + def _parse_reasoning_from_choice_message(self, choice: Choice) -> Content | None: + """Parse reasoning content from a non-streaming chat completion choice.""" + if reasoning_details := getattr(choice.message, REASONING_DETAILS_FIELD, None): + return Content.from_text_reasoning(protected_data=json.dumps(reasoning_details)) + if reasoning_content := getattr(choice.message, REASONING_CONTENT_FIELD, None): + return Content.from_text_reasoning( + protected_data=json.dumps(reasoning_content), + additional_properties={REASONING_FORMAT_KEY: REASONING_CONTENT_FIELD}, + ) + return None + + def _parse_reasoning_from_choice_delta(self, choice: ChunkChoice) -> Content | None: + """Parse reasoning content from a streaming chat completion delta.""" + if reasoning_details := getattr(choice.delta, REASONING_DETAILS_FIELD, None): + return Content.from_text_reasoning(protected_data=json.dumps(reasoning_details)) + if reasoning_content := getattr(choice.delta, REASONING_CONTENT_FIELD, None): + return Content.from_text_reasoning( + text=reasoning_content, + additional_properties={REASONING_FORMAT_KEY: REASONING_CONTENT_FIELD}, + ) + return None + def _get_metadata_from_chat_response(self, response: ChatCompletion) -> dict[str, Any]: """Get metadata from a chat response.""" return { @@ -852,6 +878,7 @@ def _prepare_message_for_openai(self, message: Message) -> list[dict[str, Any]]: all_messages: list[dict[str, Any]] = [] pending_reasoning: Any = None + pending_reasoning_field = REASONING_DETAILS_FIELD 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"): @@ -862,15 +889,22 @@ def _prepare_message_for_openai(self, message: Message) -> list[dict[str, Any]]: } if message.author_name and message.role != "tool": args["name"] = message.author_name - if "reasoning_details" in message.additional_properties and ( - details := message.additional_properties["reasoning_details"] + if ( + reasoning_field := self._reasoning_field_from_additional_properties(message.additional_properties) + ) and ( + details := message.additional_properties.get(reasoning_field) ): - args["reasoning_details"] = details + args[reasoning_field] = details match content.type: case "function_call": if all_messages and "tool_calls" in all_messages[-1]: # If the last message already has tool calls, append to it all_messages[-1]["tool_calls"].append(self._prepare_content_for_openai(content)) + elif self._can_extend_last_assistant_message( + all_messages, + message, + ): + all_messages[-1]["tool_calls"] = [self._prepare_content_for_openai(content)] else: args["tool_calls"] = [self._prepare_content_for_openai(content)] # type: ignore case "function_result": @@ -889,29 +923,50 @@ def _prepare_message_for_openai(self, message: Message) -> list[dict[str, Any]]: args["content"] = content.result if content.result is not None else "" all_messages.append(args) continue - case "text_reasoning" if (protected_data := content.protected_data) is not None: + case "text_reasoning": # Buffer reasoning to attach to the next message with content/tool_calls - pending_reasoning = json.loads(protected_data) + reasoning_field = self._reasoning_field_from_additional_properties(content.additional_properties) + if reasoning_field is None: + reasoning_field = REASONING_DETAILS_FIELD + if content.protected_data is not None: + pending_reasoning = json.loads(content.protected_data) + pending_reasoning_field = reasoning_field + continue + if content.text is not None: + pending_reasoning = content.text + pending_reasoning_field = reasoning_field + continue 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 self._can_extend_last_assistant_message( + all_messages, + message, + ): + last_content = all_messages[-1].setdefault("content", []) + if not isinstance(last_content, list): + last_content = [Content.from_text(text=str(last_content)).to_dict(exclude_none=True)] + all_messages[-1]["content"] = last_content + cast(list[dict[str, Any]], last_content).append(self._prepare_content_for_openai(content)) + else: + 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 + args[pending_reasoning_field] = pending_reasoning pending_reasoning = None + pending_reasoning_field = REASONING_DETAILS_FIELD 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 + all_messages[-1][pending_reasoning_field] = pending_reasoning else: pending_args: dict[str, Any] = { "role": message.role, "content": "", - "reasoning_details": pending_reasoning, + pending_reasoning_field: pending_reasoning, } if message.author_name and message.role != "tool": pending_args["name"] = message.author_name @@ -940,6 +995,37 @@ def _prepare_message_for_openai(self, message: Message) -> list[dict[str, Any]]: return all_messages + def _reasoning_field_from_additional_properties( + self, + additional_properties: Mapping[str, Any] | None, + ) -> str | None: + """Return the outbound reasoning field based on additional properties.""" + if not additional_properties: + return None + if REASONING_CONTENT_FIELD in additional_properties: + return REASONING_CONTENT_FIELD + if REASONING_DETAILS_FIELD in additional_properties: + return REASONING_DETAILS_FIELD + if (reasoning_format := additional_properties.get(REASONING_FORMAT_KEY)) in ( + REASONING_DETAILS_FIELD, + REASONING_CONTENT_FIELD, + ): + return cast(str, reasoning_format) + return None + + def _can_extend_last_assistant_message( + self, + all_messages: list[dict[str, Any]], + message: Message, + ) -> bool: + """Return True when content/tool calls should stay on the current assistant message.""" + return bool( + all_messages + and message.role == "assistant" + and all_messages[-1].get("role") == "assistant" + and "tool_call_id" not in all_messages[-1] + ) + def _prepare_content_for_openai(self, content: Content) -> dict[str, Any]: """Prepare content for OpenAI.""" match content.type: diff --git a/python/packages/openai/tests/openai/test_openai_chat_completion_client.py b/python/packages/openai/tests/openai/test_openai_chat_completion_client.py index c012433b08..24c2026f60 100644 --- a/python/packages/openai/tests/openai/test_openai_chat_completion_client.py +++ b/python/packages/openai/tests/openai/test_openai_chat_completion_client.py @@ -19,9 +19,13 @@ SupportsWebSearchTool, tool, ) +from agent_framework._types import _finalize_response, _process_update from agent_framework.exceptions import ChatClientException, SettingNotFoundError from openai import BadRequestError from openai.types.chat.chat_completion import ChatCompletion, Choice +from openai.types.chat.chat_completion_chunk import ChatCompletionChunk +from openai.types.chat.chat_completion_chunk import Choice as ChunkChoice +from openai.types.chat.chat_completion_chunk import ChoiceDelta as ChunkChoiceDelta from openai.types.chat.chat_completion_message import ChatCompletionMessage from pydantic import BaseModel from pytest import param @@ -823,10 +827,6 @@ def test_parse_text_reasoning_content_from_streaming_chunk( openai_unit_test_env: dict[str, str], ) -> None: """Test that TextReasoningContent is correctly parsed from streaming OpenAI chunk with reasoning_details.""" - from openai.types.chat.chat_completion_chunk import ChatCompletionChunk - from openai.types.chat.chat_completion_chunk import Choice as ChunkChoice - from openai.types.chat.chat_completion_chunk import ChoiceDelta as ChunkChoiceDelta - client = OpenAIChatCompletionClient() # Mock streaming chunk with reasoning_details @@ -869,6 +869,76 @@ def test_parse_text_reasoning_content_from_streaming_chunk( assert parsed_details == mock_reasoning_details +def test_parse_reasoning_content_from_response( + openai_unit_test_env: dict[str, str], +) -> None: + """Test that OpenAI-compatible reasoning_content is preserved from a non-streaming response.""" + client = OpenAIChatCompletionClient() + + mock_response = ChatCompletion( + id="test-response", + object="chat.completion", + created=1234567890, + model="deepseek-reasoner", + choices=[ + Choice( + index=0, + message=ChatCompletionMessage( + role="assistant", + content="The answer is 42.", + reasoning_content="Step-by-step thinking...", + ), + finish_reason="stop", + ) + ], + ) + + response = client._parse_response_from_openai(mock_response, {}) + + assert len(response.messages) == 1 + message = response.messages[0] + assert len(message.contents) == 2 + assert message.contents[0].type == "text" + assert message.contents[1].type == "text_reasoning" + assert message.contents[1].protected_data is not None + assert json.loads(message.contents[1].protected_data) == "Step-by-step thinking..." + assert message.contents[1].additional_properties["openai_reasoning_format"] == "reasoning_content" + + +def test_parse_reasoning_content_from_streaming_chunk( + openai_unit_test_env: dict[str, str], +) -> None: + """Test that OpenAI-compatible reasoning_content is preserved from streaming chunks.""" + client = OpenAIChatCompletionClient() + + mock_chunk = ChatCompletionChunk( + id="test-chunk", + object="chat.completion.chunk", + created=1234567890, + model="deepseek-reasoner", + choices=[ + ChunkChoice( + index=0, + delta=ChunkChoiceDelta( + role="assistant", + content="Partial answer", + reasoning_content="Analyzing the question...", + ), + finish_reason=None, + ) + ], + ) + + update = client._parse_response_update_from_openai(mock_chunk) + + assert len(update.contents) == 2 + assert update.contents[0].type == "text" + assert update.contents[1].type == "text_reasoning" + assert update.contents[1].text == "Analyzing the question..." + assert update.contents[1].protected_data is None + assert update.contents[1].additional_properties["openai_reasoning_format"] == "reasoning_content" + + def test_prepare_message_with_text_reasoning_content( openai_unit_test_env: dict[str, str], ) -> None: @@ -1013,6 +1083,148 @@ def test_prepare_message_with_text_reasoning_before_function_call( assert prepared[0]["role"] == "assistant" +def test_prepare_message_with_reasoning_content_before_function_call( + openai_unit_test_env: dict[str, str], +) -> None: + """Test that reasoning_content is replayed on tool-calling messages for compatible providers.""" + client = OpenAIChatCompletionClient() + + reasoning_content = Content.from_text_reasoning( + text=None, + protected_data=json.dumps("Analyzing before tool call"), + additional_properties={"openai_reasoning_format": "reasoning_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) + + assert len(prepared) == 1 + assert "reasoning_content" in prepared[0] + assert prepared[0]["reasoning_content"] == "Analyzing before tool call" + assert "reasoning_details" not in prepared[0] + assert prepared[0]["tool_calls"][0]["function"]["name"] == "get_weather" + + +def test_prepare_message_with_text_function_call_and_reasoning_details_stays_single_message( + openai_unit_test_env: dict[str, str], +) -> None: + """Test that text plus function_call plus reasoning_details stays one assistant message.""" + client = OpenAIChatCompletionClient() + + message = Message( + role="assistant", + contents=[ + Content.from_text(text="Let me check that."), + Content.from_function_call(call_id="call_abc", name="get_weather", arguments='{"city": "Seattle"}'), + Content.from_text_reasoning( + text=None, + protected_data=json.dumps({"summary": "Deciding to call a function"}), + ), + ], + ) + + prepared = client._prepare_message_for_openai(message) + + assert len(prepared) == 1 + assert prepared[0]["content"] == "Let me check that." + assert prepared[0]["tool_calls"][0]["function"]["name"] == "get_weather" + assert prepared[0]["reasoning_details"] == {"summary": "Deciding to call a function"} + + +def test_prepare_message_with_text_function_call_and_reasoning_content_stays_single_message( + openai_unit_test_env: dict[str, str], +) -> None: + """Test that text plus function_call plus reasoning_content stays one assistant message.""" + client = OpenAIChatCompletionClient() + + message = Message( + role="assistant", + contents=[ + Content.from_text(text="Let me check that."), + Content.from_function_call(call_id="call_abc", name="get_weather", arguments='{"city": "Seattle"}'), + Content.from_text_reasoning( + text=None, + protected_data=json.dumps("Deciding to call a function"), + additional_properties={"openai_reasoning_format": "reasoning_content"}, + ), + ], + ) + + prepared = client._prepare_message_for_openai(message) + + assert len(prepared) == 1 + assert prepared[0]["content"] == "Let me check that." + assert prepared[0]["tool_calls"][0]["function"]["name"] == "get_weather" + assert prepared[0]["reasoning_content"] == "Deciding to call a function" + assert "reasoning_details" not in prepared[0] + + +def test_streaming_reasoning_content_accumulates_and_replays_on_tool_call_message( + openai_unit_test_env: dict[str, str], +) -> None: + """Test that streamed reasoning_content chunks accumulate before replay on a tool-calling message.""" + client = OpenAIChatCompletionClient() + response = ChatResponse(messages=[], response_id="resp-1") + + first_chunk = ChatCompletionChunk( + id="test-chunk", + object="chat.completion.chunk", + created=1234567890, + model="deepseek-reasoner", + choices=[ + ChunkChoice( + index=0, + delta=ChunkChoiceDelta( + role="assistant", + reasoning_content="first ", + ), + finish_reason=None, + ) + ], + ) + second_chunk = ChatCompletionChunk( + id="test-chunk", + object="chat.completion.chunk", + created=1234567890, + model="deepseek-reasoner", + choices=[ + ChunkChoice( + index=0, + delta=ChunkChoiceDelta( + role="assistant", + reasoning_content="second", + ), + finish_reason=None, + ) + ], + ) + + _process_update(response, client._parse_response_update_from_openai(first_chunk)) + _process_update(response, client._parse_response_update_from_openai(second_chunk)) + _finalize_response(response) + + tool_call_message = Message( + role="assistant", + contents=[ + *response.messages[0].contents, + Content.from_function_call(call_id="call_abc", name="get_weather", arguments='{"city": "Seattle"}'), + ], + ) + + prepared = client._prepare_message_for_openai(tool_call_message) + + assert len(prepared) == 1 + assert prepared[0]["reasoning_content"] == "first second" + assert prepared[0]["tool_calls"][0]["function"]["name"] == "get_weather" + + def test_function_approval_content_is_skipped_in_preparation( openai_unit_test_env: dict[str, str], ) -> None: