-
Couldn't load subscription status.
- Fork 610
.NET [Breaking] Simplify and Refactor ChatclientAgentOptions Ctor + Instructions #1517
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
.NET [Breaking] Simplify and Refactor ChatclientAgentOptions Ctor + Instructions #1517
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Refactors ChatClientAgentOptions to unify Instructions storage within ChatOptions and updates usages across tests and samples.
- Removes the parameterized constructor and introduces an Instructions proxy to ChatOptions.
- Adjusts instruction merging logic in ChatClientAgent and updates related tests and samples.
- Updates tests but introduces some mismatches between test names, assertions, and missing coverage of new instruction propagation behavior.
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgentOptions.cs | Refactors options: removes ctor, proxies Instructions to internal ChatOptions, adds cloning and conditional preservation logic. |
| dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgent.cs | Adjusts request instruction merging, reorders initialization assignments. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgentOptionsTests.cs | Rewrites tests to use object initializers; adds new scenarios but leaves some assertions incomplete and one test name inconsistent with its setup. |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgentTests.cs | Updates expectation: ChatOptions now always present when instructions provided. |
| dotnet/samples/*/Program.cs | Replaces removed constructor overload usages with object initializer pattern for Instructions and ChatOptions. |
| dotnet/agent-framework-dotnet.slnx | Removes SemanticKernelMigration folder reference from solution. |
dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgentOptions.cs
Outdated
Show resolved
Hide resolved
dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgentOptions.cs
Outdated
Show resolved
Hide resolved
dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgentOptionsTests.cs
Outdated
Show resolved
Hide resolved
dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgentOptionsTests.cs
Outdated
Show resolved
Hide resolved
dotnet/tests/Microsoft.Agents.AI.UnitTests/ChatClient/ChatClientAgentOptionsTests.cs
Show resolved
Hide resolved
dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgentOptions.cs
Outdated
Show resolved
Hide resolved
dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgentOptions.cs
Outdated
Show resolved
Hide resolved
dotnet/src/Microsoft.Agents.AI/ChatClient/ChatClientAgentOptions.cs
Outdated
Show resolved
Hide resolved
|
any chance this PR can help having custom options so that we can use gemini with the openai compatibility API and use custom options like you see in the Thinking section of their guide where it is even possible to disable thinking at all or even obtain thinking tokens? |
|
@MithrilMan, any options that are supported by the OpenAI SDK can be provided. ChatOptions allows you to provide the options for the underlying service via the See here for example where it is providing reasoning options for responses via See e.g. for responses, this is the options object, and it does have some reasoning settings: |
|
@westey-m Yeah I saw that but the problem is that google offers more control over thinking that OpenAI doesn't. Also in the documentation I linked, they show how you can set specific thinking budget (something OpenAI doesn't) and how to receive thinking tokens to see the internal thinking process of the LLM. I understand that you can't add explicit value on OpenAI options that the OpenAI doesn't handle, but you could allow custom values that the user can set manually Of course would be better to have a specific gemini client with its option |
@MithrilMan, definitely open an issue in the OpenAI SDK repo with your requirement: https://github.com/openai/openai-dotnet. |
|
@westey-m I've the feeling it would be something they don't have the incentive in doing it, since it would mean supporting potentially competitors. Anyway does your reply means you are not gonna support gemini as first citizen, with dedicated client api classes? |
dotnet/samples/GettingStarted/Agents/Agent_Step05_StructuredOutput/Program.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: westey <[email protected]>
For Gemini you can reuse the Semantic Kernel We are working to push the IChatClient implementations into existing client SDK's or M.E.AI. |
Motivation and Context
Duplicated "Instructions" options
Context
Before
Microsoft.Extensions.AI.ChatOptionshad theInstructionsproperty we added this property into theAgentOptions.InstructionsIn the past months the
chatOptions.Instructionsproperty was addedAnd now we have
agentOptions.Instructionsserving the same purpose ofagentOptions.ChatOptions.Instructionswhen configuring theChatClientAgentwhich is not ideal.As a first step to simplify, the problem above this PR updates
options.Instructionsto pointing to a single reference: e.g:agentOptions.Instructions => agentOptions.ChatOptions.Instructions.Note
We may consider the full removal of the
options.Instructionsin favor of theoptions.ChatOptions.Instructionsas a next step, happy to get some input from community.