Python: Fix reasoning content parsing in OpenAIChatCompletionClient#7028
Python: Fix reasoning content parsing in OpenAIChatCompletionClient#7028giles17 wants to merge 3 commits into
Conversation
Fix two issues with reasoning content handling in the Chat Completions client: 1. (microsoft#6979) reasoning_details plaintext buried as encrypted data: The client dumped the entire reasoning_details array into Content.protected_data without setting Content.text, causing AG-UI to emit ReasoningEncryptedValueEvent instead of visible ReasoningMessageContentEvent for plaintext reasoning providers (e.g. OpenRouter). Now extracts readable text from reasoning_details entries into Content.text while preserving protected_data for round-trip fidelity. 2. (microsoft#6978) Mistral list content causes crash: Mistral reasoning models return content as a list of typed chunks ([{"type": "thinking", ...}, {"type": "text", ...}]) instead of a plain string. _parse_text_from_openai assumed content was always a string, causing a Pydantic ValidationError downstream. Now detects list content and parses thinking chunks as Content.from_text_reasoning and text chunks as Content.from_text. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull request overview
This PR fixes parsing of “reasoning” outputs in the Python OpenAIChatCompletionClient so downstream consumers (notably AG-UI) can display plaintext reasoning correctly and avoid crashes when providers return structured chunked content.
Changes:
- Add
_extract_reasoning_text()and use it to populateContent.textfor plaintextreasoning_detailswhile preserving full payload round-tripped inprotected_data. - Refactor
_parse_text_from_openai()to returnlist[Content]and introduce_parse_chunked_content()to handle Mistral-stylecontent: [...]chunks (thinking+text). - Add regression tests covering plaintext
reasoning_detailsextraction and chunked list-content parsing in both streaming and non-streaming paths.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| python/packages/openai/agent_framework_openai/_chat_completion_client.py | Adds plaintext reasoning extraction + chunked content parsing; updates response parsing to emit correct Content items. |
| python/packages/openai/tests/openai/test_openai_chat_completion_client.py | Adds tests for plaintext reasoning extraction and Mistral chunked content in both streaming and non-streaming modes. |
- Use cast() for proper type narrowing in _extract_reasoning_text and
_parse_chunked_content to satisfy pyright strict mode
- Handle {"content": "..."} string shape in _extract_reasoning_text
(addresses review comment about missing format coverage)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
model_construct bypasses Pydantic runtime validation but mypy still checks declared types. Use cast(Any, ...) for the list content args. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| 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))) | ||
| contents.append(Content.from_text_reasoning( |
There was a problem hiding this comment.
Should visible reasoning normalization cover the other documented OpenRouter plaintext forms here? reasoning.summary entries put the displayable value under summary, while raw-reasoning models return message.reasoning (or reasoning_content) without reasoning_details; on this head the first becomes text=None and the second is discarded entirely. Could we normalize those fields in both streaming and non-streaming paths so the fix covers the provider's supported response shapes?
| else: | ||
| text = "" | ||
| if text: | ||
| results.append(Content.from_text_reasoning(text=text, raw_representation=choice)) |
There was a problem hiding this comment.
Should we preserve the original structured assistant content for the next turn? Converting the ThinkChunk here and the text chunk below into separate framework contents makes _prepare_message_for_openai() replay them as two plain assistant messages, so the original [{"type":"thinking", ...}, {"type":"text", ...}] is lost. Mistral requires the complete assistant message, including the ThinkChunk, for multi-turn reasoning and tool continuation; could we retain or reconstruct that list so it round-trips as one message?
Motivation and Context
Fixes #6978 and #6979.
Two related bugs in
OpenAIChatCompletionClientprevent reasoning content from being displayed correctly:Python: [Bug]: OpenAI Chat Completions client buries plaintext
reasoning_detailsas encrypted data #6979 - Plaintext reasoning buried as encrypted data: The client dumps the entirereasoning_detailsarray intoContent.protected_datawithout settingContent.text. Downstream, AG-UI emitsReasoningEncryptedValueEvent(opaque) instead of visibleReasoningMessageContentEventfor providers like OpenRouter that send plaintext reasoning.Python: [Bug]: OpenAI Chat Completions client crashes on (and drops) Mistral reasoning content chunks #6978 - Mistral list content causes crash: Mistral reasoning models return
contentas a list of typed chunks ([{"type": "thinking", ...}, {"type": "text", ...}]) instead of a plain string._parse_text_from_openaiassumed content was always a string, causing a PydanticValidationErrorcrash in AG-UI.Changes
_chat_completion_client.py_extract_reasoning_text()helper (module-level): Extracts readable text from variousreasoning_detailsformats (list of dicts withtextkeys, dict withcontentlist, plain strings). ReturnsNonefor genuinely encrypted/opaque data.reasoning_detailshandling (lines 753-757, 799-803): Now passes extracted plaintext asContent.textwhile preserving the full JSON inprotected_datafor round-trip fidelity._parse_text_from_openai(): Returnslist[Content]instead ofContent | None. Detects whenmessage.contentis a list and delegates to_parse_chunked_content()._parse_chunked_content()static method: Parses Mistral-stylethinkingchunks →Content.from_text_reasoningandtextchunks →Content.from_text.Tests
test_parse_reasoning_details_extracts_plaintext— verifies plaintext extraction from OpenRouter-style reasoning_detailstest_parse_reasoning_details_extracts_plaintext_streaming— same for streaming pathtest_parse_mistral_chunked_content_from_response— verifies Mistral list content parsing (non-streaming)test_parse_mistral_chunked_content_streaming— verifies Mistral list content parsing (streaming)test_parse_plain_string_content_still_works— regression test for normal string contentValidation
test_openai_chat_completion_client.pypass (including 6 new tests)_chat_client.py) which already handles reasoning correctly