Skip to content

Conversation

@ReubenBond
Copy link
Member

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

  • The code builds clean without any errors or warnings
  • The PR follows the Contribution Guidelines
  • All unit tests pass, and I have added new tests where possible
  • Is this a breaking change? If yes, add "[BREAKING]" prefix to the title of the PR.

Copilot AI review requested due to automatic review settings November 7, 2025 14:51
@github-actions github-actions bot changed the title OpenAI Responses: suppress 'model' parameter if it is used for agent .NET: OpenAI Responses: suppress 'model' parameter if it is used for agent Nov 7, 2025
Copy link
Contributor

Copilot AI left a 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 ValidateRequestAsync method in IResponseExecutor and IResponsesService for early request validation
  • Refactors error handling in ResponsesHttpHandler to use validation results and response status codes instead of exception-based error handling
  • Updates agent resolution logic in HostedAgentResponseExecutor to 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

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;
Copy link
Member

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?

Copy link
Member Author

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

Copy link
Member

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.

Copy link
Member Author

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?

Copy link
Member Author

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

@ReubenBond ReubenBond marked this pull request as draft November 7, 2025 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants