[Frontend] Add structured_outputs field to ResponsesRequest#33249
[Frontend] Add structured_outputs field to ResponsesRequest#33249MarcinGodniak wants to merge 2 commits intovllm-project:mainfrom
Conversation
Signed-off-by: Marcin Godniak <marcingodniak@gmail.com> Signed-off-by: Marcin Godniak <mgodniak@amazon.com>
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run You ask your reviewers to trigger select CI tests on top of Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add If you have any questions, please reach out to us on Slack at https://slack.vllm.ai. 🚀 |
There was a problem hiding this comment.
Code Review
This pull request adds the structured_outputs field to ResponsesRequest, aligning it with the ChatCompletionRequest and allowing for more flexible structured output configurations. The implementation in to_sampling_params correctly uses this new field. However, it introduces a side effect by mutating the request object. I've provided a suggestion to refactor this to avoid mutation, which improves code clarity and maintainability by making the method's behavior more predictable.
| # TODO: consider supporting non harmony messages as well | ||
| previous_input_messages: list[OpenAIHarmonyMessage | dict] | None = None | ||
|
|
||
| structured_outputs: StructuredOutputsParams | None = Field( |
There was a problem hiding this comment.
Do you need to use this field?
There was a problem hiding this comment.
My ultimate goal is to support providing grammar via the tool parser.
For chat/completions requests, this field is used to analyze the request and, when necessary, populate the StructuredOutputsParams.grammar field, which is later consumed by to_sampling_params. I already use this field in chat/completions, and it seemed natural to replicate the same approach here.
I’m open to other approaches if there’s a better way to achieve this.
There was a problem hiding this comment.
For structured outputs, the Responses API has its own dedicated fields.
|
This pull request has merge conflicts that must be resolved before it can be |
Purpose
Expose structured_outputs as a field on ResponsesRequest to allow internal components (e.g., tool_parser) to modify structured output parameters during request processing.
Previously, structured_outputs was a local variable in to_sampling_params(), making it inaccessible to other parts of the request pipeline. This change promotes it to an instance field so it can be set/modified by tool parsers before sampling params are generated.
This aligns the Responses API with the existing behavior in /chat/completions, where structured_outputs is already available as a field.
Test Plan
pytest tests/entrypoints/openai/responses -v
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.