[Bugfix] ensure tool_choice is popped when tool_choice:null is passed in json payload

[gcalmettes]

When request is made with "tool_choice":null,"tools":[] in a json payload, vllm returns a 500 Internal Server Error.

The minimalist example below triggers the error:

curl -X POST  -H "Content-Type: application/json" --url "http://localhost:8003/v1/chat/completions"  --data
 '{"model":"llama33", "messages":[{"role":"user","content":[{"type":"text","text":"tell me a short joke"}]}],"stream":t
rue,"temperature":1.0,"tool_choice":null, tools:[]}'

INFO:     172.17.0.1:38432 - "POST /v1/chat/completions HTTP/1.1" 500 Internal Server Error

This is due to the fact that the tool_choice field is then present on the request, and deserialized as None, but this case is not handled, in the current check_tool_usage routine.

This PR fixes the issue.

[github-actions]

👋 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 fastcheck CI which starts running only a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of those by going to your fastcheck build on Buildkite UI (linked in the PR checks section) and unblock them. If you do not have permission to unblock, ping simon-mo or khluu to add you in our Buildkite org.

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 ready label to the PR or enable auto-merge.

🚀

[gemini-code-assist]

Summary of Changes

Hello @gcalmettes, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a bug in the chat completions endpoint where providing "tool_choice": null in the request body resulted in a server error. The change modifies the input validation to gracefully handle this specific case, preventing the error and allowing the request to proceed as if tool_choice was not specified.

Highlights

• Bugfix: Fixes a 500 Internal Server Error that occurred when the tool_choice field was explicitly set to null in the JSON payload for chat completions.
• Input Handling: Adds specific handling in the check_tool_usage validation logic to correctly process requests where tool_choice is provided with a null value.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configureGemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This PR fixes a bug where a 500 Internal Server Error occurs when tool_choice is set to null in the request payload. The change removes tool_choice from the request data if it's None. A suggestion has been made to use data.get("tool_choice") instead of direct dictionary access to prevent a KeyError if tool_choice is missing.

[gcalmettes]

@aarnphm any chance you could look at this small fix ?

[aarnphm]

fyi: #17177

[gcalmettes]

@aarnphm it seems that PR #17177 deals with the case when the valid tool_choice option "none" is passed.
The fix in the current PR handles the case when tool_choice is set to null (valid json) in the payload, which is a bit different.

Currently, the below request will results in a 500 Internal Error because the tool_choice field is present in the request but its value is None (python null, not "none" the valid tool_choice option)

curl -s -k -vv -X POST  -H "Content-Type: application/json" --url "http://localhost:8003/v1/chat/completions"  --data '{"model":"llama3", "messages":[{"role":"user","content":[{"type":"text","text":"tell me a short joke"}]}],"stream":true,"temperature":1.0,"tool_choice":null,"tools":[]}'

[aarnphm]

What is the behaviour if you just don't specify tool_choice?

[gcalmettes]

if tool_choice is not present in the payload, then the model will respond without error since tool_choice will not be in data.

The problem is that some external integration are always passing the tool_choice parameters and set it to null if no tools are present, we got the problem when trying to use the ZED ai integration with a model served by vLLM

[aarnphm]

The problem is that some external integration are always passing the tool_choice parameters and set it to null if no tools are present, we got the problem when trying to use the ZED ai integration with a model served by vLLM

Right, then it seems like these tools aren't respecting the openai spec https://platform.openai.com/docs/api-reference/responses/create#responses-create-tool_choice 🤦

fwiw I think we ought to update

vllm/vllm/entrypoints/openai/protocol.py

Line 690 in 6f68c49

if "tool_choice" in data and data["tool_choice"] == "none":

to check for None and none...

I'm not against merging this, just that this is prone to getting removed, as I plan to clean this up a bit later, so probably some inline docs would also be helpful. If possible, can you also open a ticket from the zed-editor side to mention about this?

[gcalmettes]

fwiw I think we ought to update
if "tool_choice" in data and data["tool_choice"] == "none":
to check for None and none...

This is what I had originally done, but then decided to make it a separate block, as not specifying tool_choice (it being null in the json) has a very different signification than explicitly saying that we don't want tools (tool_choice="none").

In fact, based on your input, I know think that the best solution would be to validate that tool_choice is not None here. This would also be more future-proof whatever the outcome of #17177 is.

On a second note, I also realized the 500 Internal Server Error was coming from the NotImplementedError here, we should probably change it to a ValueError so a proper error message is sent back to the client instead of returning a 500 (added to this PR).

I have amended the PR, let me think if you think this is more reasonable.

[aarnphm]

Yep, I think this makes more sense. Thanks for this.

[gcalmettes]

any chance you have the permission to restart the failed tests ? It seems that they are due to errors unrelated to the changes introduced in this PR ? Or should I push an empty commit to trigger the ci again ?

[gcalmettes]

re-up on this one @aarnphm 🙏