Surface Responses API reasoning item content as TextReasoningContent - #7752
Open
LouisDeconinck wants to merge 3 commits into
Open
LouisDeconinck wants to merge 3 commits into
LouisDeconinck wants to merge 3 commits into
Conversation
OpenAIResponsesChatClient.ToChatMessages only read reasoning text from a ReasoningResponseItem's summary parts. Responses API implementations that return raw reasoning in the item's "content" field instead (e.g. OpenRouter) produced an empty TextReasoningContent, so the reasoning never surfaced in responses or telemetry. The OpenAI client doesn't surface "content" as a property but preserves it in the item's JsonPatch, so fall back to concatenating the text of its reasoning_text parts when the summary is empty, matching the approach used for reasoning_content in OpenAIChatClient. The field already round-trips through the preserved raw item, so no request-side change is needed. Fixes dotnet#7714
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The fallback should only extract text from reasoning_text parts.
Pull request overview
Updates the OpenAI Responses API adapter to surface reasoning text stored in content when summary is empty.
Changes:
- Adds JSON-patch fallback extraction for reasoning content.
- Adds regression coverage for concatenated reasoning text.
File summaries
| File | Description |
|---|---|
test/Libraries/Microsoft.Extensions.AI.OpenAI.Tests/OpenAIResponseClientTests.cs |
Adds non-streaming reasoning-content regression coverage. |
src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIResponsesChatClient.cs |
Implements the content fallback for reasoning text. |
Review details
Suppressed comments (1)
src/Libraries/Microsoft.Extensions.AI.OpenAI/OpenAIResponsesChatClient.cs:971
- This fallback appends the
textproperty from every object incontent, rather than only fromreasoning_textparts. If a response contains another content-part type with a text field, that text will be incorrectly exposed asTextReasoningContent; check the part'stypebefore appending.
if (part.ValueKind == JsonValueKind.Object &&
part.TryGetProperty("text"u8, out JsonElement textElement) &&
textElement.ValueKind == JsonValueKind.String)
{
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
The patch-based fallback appended every content part's text, so a part of another type carrying a text field would leak into TextReasoningContent. Check the part's type before appending, and cover it with a non-reasoning_text part in the regression test.
Collaborator
🎉 Good job! The coverage increased 🎉
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1600596&view=codecoverage-tab |
Contributor
Author
|
Added Note on the coverage report above: the flagged project minimums (e.g. Microsoft.Extensions.AI.OpenAI at ~68.5% line vs 75%) are standing whole-project baselines — the same numbers appear on unrelated PRs — not something introduced by this diff. The new code paths are covered. |
Collaborator
🎉 Good job! The coverage increased 🎉
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1600976&view=codecoverage-tab |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7714
OpenAIResponsesChatClient.ToChatMessagesread reasoning text only from aReasoningResponseItem's summary parts viaGetSummaryText(). Responses API implementations that return raw chain-of-thought in the item'scontentfield instead ofsummary(e.g. OpenRouter with gpt-oss-family models) produced aTextReasoningContentwith emptyText, so the reasoning never surfaced to callers or OTel traces.The OpenAI client doesn't expose
contentas a property (per openai/openai-dotnet#1329 the field exists in the schema but is never returned by the OpenAI service), but it is preserved in the item'sJsonPatch. This change falls back to reading$.contentfrom the patch when the summary is empty, concatenating thetextof itsreasoning_textparts — mirroring how #7295 readsreasoning_contentvia patch inOpenAIChatClient, and how the streaming branch surfacesresponse.reasoning_text.deltapayloads.No request-side change is needed: the reasoning item's
RawRepresentationis preserved and round-trips thecontentfield as-is.Tests
ReasoningContent_NonStreamingtest asserts a reasoning item carryingcontent(with an emptysummary) produces aTextReasoningContentwhoseTextcontains the concatenated reasoning. Verified the test fails before the fix (Textwas empty).Microsoft.Extensions.AI.OpenAI.Tests: 390 passed / 0 failed on net8.0, net9.0, and net10.0.Microsoft Reviewers: Open in CodeFlow