Repository navigation
Keep null finish reason in streaming conversion - #7866
Open
nurullahturkoglu wants to merge 1 commit into
Open
nurullahturkoglu wants to merge 1 commit into
nurullahturkoglu wants to merge 1 commit into
Conversation
AsOpenAIStreamingChatCompletionUpdatesAsync mapped a missing finish reason to Stop, so every content chunk was written with "finish_reason":"stop". Pass null on the streaming path instead. The non-streaming conversion keeps its Stop default. Fixes dotnet#7863
Collaborator
🎉 Good job! The coverage increased 🎉
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1642632&view=codecoverage-tab |
valkiara
reviewed
Oct 23, 2026
valkiara
left a comment
valkiara
left a comment
Contributor
There was a problem hiding this comment.
Reviewed 555648c. The adapter test project passed on net10.0 with SDK 10.0.401: 396 passed, 163 skipped with live integration tests disabled.
I also ran eight local cases checking serialized finish reasons for stop, length, content_filter, tool_calls and function_call, plus function-call deltas, raw-update passthrough and the completed-response default. Content and trailing usage chunks kept a missing finish reason, terminal chunks retained their explicit reason, and non-streaming responses still defaulted to Stop. All eight passed.
No issues found in the changed conversion path.
This branch has not been deployed
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 #7863
AsOpenAIStreamingChatCompletionUpdatesAsyncpassed every update throughToChatFinishReason, whose default arm mapsnulltoStop. Every content chunk before the last one was written with"finish_reason":"stop", so a client that ends the turn on the first non-null finish reason stops after the first chunk.The streaming path now passes
nullwhen the update has no finish reason.AsOpenAIChatCompletionkeeps itsStopdefault, since a complete response always has a finish reason. The reverse conversion inOpenAIChatClientalready maps a missing finish reason tonull.AsOpenAIStreamingChatCompletionUpdatesAsync_WithMultipleUpdates_ProcessesAllCorrectlyasserted the old behavior for the first chunk, so I changed that assertion to expectnull. It fails without the fix.I ran
Microsoft.Extensions.AI.OpenAI.Testsfor net10.0 on macOS arm64: 396 passed, 163 skipped, 0 failed. I didn't run net8.0 or net9.0 locally. I also serialized the two updates from the issue withModelReaderWriter: the content chunk no longer has afinish_reasonproperty, and the terminal chunk has"length".Microsoft Reviewers: Open in CodeFlow