.NET: Add Reasoning to ChatClientAgent ChatOptions merging#5463
Open
MaciejWarchalowski wants to merge 2 commits intomicrosoft:mainfrom
Open
.NET: Add Reasoning to ChatClientAgent ChatOptions merging#5463MaciejWarchalowski wants to merge 2 commits intomicrosoft:mainfrom
MaciejWarchalowski wants to merge 2 commits intomicrosoft:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes ChatClientAgent’s per-request ChatOptions merge behavior so the agent-level default ChatOptions.Reasoning is no longer silently dropped when callers provide their own ChatOptions, aligning Reasoning with the existing “request wins, agent fills unset values” merge pattern.
Changes:
- Add
ChatOptions.Reasoningto theChatClientAgentChatOptionsmerge block via??=. - Add unit tests covering request-overrides-agent, agent-fallback, and request-when-agent-null scenarios for
Reasoning.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgent.cs | Adds Reasoning to the existing per-request vs agent-default ChatOptions merge logic. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgent_ChatOptionsMergingTests.cs | Adds three new tests validating Reasoning merge precedence and fallback behavior. |
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.
Motivation and Context
ChatClientAgentmerges per-requestChatOptionswith the agent's defaultChatOptionsso that caller-supplied values take priority and agent defaults fill in the gaps. TheReasoningproperty (ReasoningOptions) was the only scalar option missing from this merge logic, meaning it could never be set via the agent's default options — it was silently dropped on every request.This change adds
Reasoningto the merge so it follows the same "request wins, agent default as fallback" pattern as every otherChatOptionsproperty.Fixes #5460.
Description
ChatClientAgent.cs — one-line fix in the options-merging block:
This is placed alongside the other nullable-assign merges (
ModelId,PresencePenalty,ResponseFormat,Seed,Temperature,TopP, …) so that:Reasoningin theirChatOptions, it is used as-is.Reasoningbut the agent has a default configured, the agent default is applied.Reasoningset, the value remainsnull(unchanged behaviour).ChatClientAgent_ChatOptionsMergingTests.cs — three new
[Fact]tests covering all three cases above:ChatOptionsMergingUsesRequestReasoningOverAgentReasoningAsyncChatOptionsMergingFallsBackToAgentReasoningWhenRequestHasNoneAsyncChatOptionsMergingUsesRequestReasoningWhenAgentHasNoneAsyncContribution Checklist