-
Notifications
You must be signed in to change notification settings - Fork 682
.NET: OpenAI Responses: suppress 'model' parameter if it is used for agent #2003
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?
Conversation
…resolution, improve validation
…resolution, improve validation
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
This pull request refactors error handling for the OpenAI Responses API by introducing a validation layer that executes before response creation. The main goal is to return proper HTTP status codes (BadRequest instead of NotFound) for validation errors and to centralize validation logic.
- Introduces a new
ValidateRequestAsyncmethod inIResponseExecutorandIResponsesServicefor early request validation - Refactors error handling in
ResponsesHttpHandlerto use validation results and response status codes instead of exception-based error handling - Updates agent resolution logic in
HostedAgentResponseExecutorto support the new validation pattern
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| OpenAIResponsesAgentResolutionIntegrationTests.cs | Updates test expectation from NotFound to BadRequest for non-existent agent scenarios |
| ResponsesHttpHandler.cs | Adds pre-validation step and replaces exception-based error handling with status-based error handling |
| InMemoryResponsesService.cs | Implements ValidateRequestAsync method and removes inline validation from execution methods |
| IResponsesService.cs | Adds ValidateRequestAsync method signature to interface |
| IResponseExecutor.cs | Adds ValidateRequestAsync method signature and necessary using statement |
| HostedAgentResponseExecutor.cs | Implements ValidateRequestAsync and refactors agent resolution logic in ExecuteAsync |
| AgentRunResponseExtensions.cs | Removes default value assignment for ServiceTier |
| AIAgentResponseExecutor.cs | Adds no-op implementation of ValidateRequestAsync |
| 08_WriterCriticWorkflow/Program.cs | Removes explicit type parameter from StreamAsync call |
| 07_MixedWorkflowAgentsAndExecutors/Program.cs | Removes explicit type parameter from StreamAsync call |
dotnet/src/Microsoft.Agents.AI.Hosting.OpenAI/Responses/HostedAgentResponseExecutor.cs
Outdated
Show resolved
Hide resolved
dotnet/src/Microsoft.Agents.AI.Hosting.OpenAI/Responses/InMemoryResponsesService.cs
Show resolved
Hide resolved
dotnet/src/Microsoft.Agents.AI.Hosting.OpenAI/Responses/ResponsesHttpHandler.cs
Show resolved
Hide resolved
| this._logger.LogError(ex, "Failed to resolve agent with name '{AgentName}'", agentName); | ||
| throw new InvalidOperationException($"Agent '{agentName}' not found. Ensure the agent is registered with AddAIAgent().", ex); | ||
| } | ||
| return request.Agent?.Name ?? request.Model; |
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.
Can you remind me why DevUI doesn't just always send it in an agent property rather than using model for that?
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.
DevUI uses the official OpenAI SDK, which doesn't have the 'agent' property. I want to introduce a change there to use the 'metadata' dictionary instead of 'model', but I want to un-block @jeffhandley etc asap
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.
DevUI uses the official OpenAI SDK, which doesn't have the 'agent' property
But it does now have JSON Patch support, so you can get/set whatever properties you want to a request.
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.
Do all of the OpenAI SDKs support setting arbitrary properties in requests & responses?
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.
I have an alternate PR here that uses metadata.entity_id instead: #1984
Motivation and Context
The DevUI currently flows the entity id in the 'model' field, but this field has should otherwise be flowed into the implementation. When the 'model' field is used for the entity id, we should instead suppress it from flowing further down the call chain as the model field, since consumers cannot distinguish a model parameter from an entity id.
This PR implements the above logic and enhances request validation.
Contribution Checklist