-
-
Notifications
You must be signed in to change notification settings - Fork 10k
[Frontend] Align tool_choice="required" behavior with OpenAI when tools is empty #21052
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
[Frontend] Align tool_choice="required" behavior with OpenAI when tools is empty #21052
Conversation
👋 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 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 🚀 |
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.
Code Review
This pull request introduces an explicit validation check for tool_choice
in the OpenAI server implementation. Specifically, it ensures that if tool_choice
is set to "required"
, the tools
list cannot be empty. This change prevents a downstream error from the guided decoding backend and provides a much clearer error message to the user, which is a significant improvement, especially for streaming responses. The implementation is correct, robust, and placed appropriately within the Pydantic model validator. I find no issues with the proposed change.
50f3403
to
7fb607c
Compare
Signed-off-by: Sungyoon Jeong <[email protected]>
7fb607c
to
c4e552b
Compare
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.
Could you help verify how the OpenAI online service handles this situation?
@chaunceyjiang I checked, and it turns out that the OpenAI API behaves as if Request payload: {
"model": "gpt-4o",
"messages": [
{
"role": "user",
"content": "How is the weather today in Seoul?"
}
],
"stream": false,
"tool_choice": "required",
"tools": []
} Response body: "choices": [
{
"index": 0,
"message": {
"role": "assistant",
"content": "I currently don't have real-time weather data. You might want to check a reliable weather website or use a weather app for the most current information in Seoul.",
"refusal": null,
"annotations": []
},
"logprobs": null,
"finish_reason": "stop"
}
], To align with this behavior, I think the simplest approach would be to override |
Sounds good. |
Signed-off-by: Sungyoon Jeong <[email protected]>
@chaunceyjiang Applied in 781573d. Also changed the title of this PR. Updated test result:
|
Hi @chaunceyjiang, just a friendly reminder to take a look at this when you have a chance. Thanks in advance. |
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.
…ls is empty (vllm-project#21052) Signed-off-by: Sungyoon Jeong <[email protected]> Signed-off-by: shuw <[email protected]>
…ls is empty (vllm-project#21052) Signed-off-by: Sungyoon Jeong <[email protected]> Signed-off-by: shuw <[email protected]>
…ls is empty (vllm-project#21052) Signed-off-by: Sungyoon Jeong <[email protected]>
…ls is empty (vllm-project#21052) Signed-off-by: Sungyoon Jeong <[email protected]>
…ls is empty (vllm-project#21052) Signed-off-by: Sungyoon Jeong <[email protected]> Signed-off-by: x22x22 <[email protected]>
…ls is empty (vllm-project#21052) Signed-off-by: Sungyoon Jeong <[email protected]> Signed-off-by: x22x22 <[email protected]>
…ls is empty (vllm-project#21052) Signed-off-by: Sungyoon Jeong <[email protected]>
…ls is empty (vllm-project#21052) Signed-off-by: Sungyoon Jeong <[email protected]> Signed-off-by: jingyu <[email protected]>
…ls is empty (vllm-project#21052) Signed-off-by: Sungyoon Jeong <[email protected]> Signed-off-by: Jinzhen Lin <[email protected]>
…ls is empty (vllm-project#21052) Signed-off-by: Sungyoon Jeong <[email protected]> Signed-off-by: Noam Gat <[email protected]>
…ls is empty (vllm-project#21052) Signed-off-by: Sungyoon Jeong <[email protected]> Signed-off-by: Paul Pak <[email protected]>
…ls is empty (vllm-project#21052) Signed-off-by: Sungyoon Jeong <[email protected]>
…ls is empty (vllm-project#21052) Signed-off-by: Sungyoon Jeong <[email protected]> Signed-off-by: Boyuan Feng <[email protected]>
…ls is empty (vllm-project#21052) Signed-off-by: Sungyoon Jeong <[email protected]> Signed-off-by: Diego-Castan <[email protected]>
…ls is empty (vllm-project#21052) Signed-off-by: Sungyoon Jeong <[email protected]>
…ls is empty (vllm-project#21052) Signed-off-by: Sungyoon Jeong <[email protected]>
…ls is empty (vllm-project#21052) Signed-off-by: Sungyoon Jeong <[email protected]>
Purpose
When a user specifies
tool_choice="required"
with an emptytools
, the OpenAI-compatible server does not currently handle this scenario in the most appropriate way. The response returned is as follows:This
BadRequestError
is not raised by the model validator but by the guided decoding backend, because the request generates an unsatisfiable JSON schema:Although this behavior technically blocks invalid requests, it is not the most user-friendly. The issue becomes more problematic when
stream=True
: in this case, the request is not validated before upgrading to SSE. Instead, the error surfaces after the model has begun generating results:This PR adds explicit validation of tool length when
tool_choice="required"
.Test Plan
Manually ran
vllm serve meta-llama/Llama-3.1-8B-Instruct --enable-auto-tool-choice --tool-call-parser llama3_json
and executed the curl command above.Test Result