Skip to content

[Frontend] Improve error message in tool_choice validation #19239

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Jun 12, 2025
Merged

[Frontend] Improve error message in tool_choice validation #19239

Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
057de82
Simplify and improve error message in tool_choice validation
22quinn Jun 5, 2025
18b15bd
fix
22quinn Jun 5, 2025
4d93bdd
Simplify and improve error message in tool_choice validation
22quinn Jun 5, 2025
82acd45
fix
22quinn Jun 5, 2025
4777d07
Merge branch 'main' into specified_function
22quinn Jun 7, 2025
ece4d39
Merge branch 'main' into specified_function
22quinn Jun 11, 2025
569104c
address comments
22quinn Jun 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 8 additions & 12 deletions vllm/entrypoints/openai/protocol.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,20 +700,16 @@ def check_tool_usage(cls, data):
# it matches a valid tool
if isinstance(data["tool_choice"], dict):
valid_tool = False
specified_function = data["tool_choice"].get("function")
if not specified_function:
function = data["tool_choice"].get("function")
if not isinstance(function, dict) or (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There two types of error: 1) function is not dict, 2) function_name is not str, can distinguish them?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not seem to add more value to show different error messages to users for these two cases, as the key is to show the correct usage to users. Feels original code is duplicated so consolidating them.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same sentiment with Lu Fang here. I think we should be very explicit about the error.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aarnphm ok updated, thanks for review

(function_name := function.get("name"))
and not isinstance(function_name, str)):
raise ValueError(
"Expected field `function` in `tool_choice`."
" Correct usage: `{\"type\": \"function\","
" \"function\": {\"name\": \"my_function\"}}`")
specified_function_name = specified_function.get("name")
if not specified_function_name:
raise ValueError(
"Expected field `name` in `function` in `tool_choice`."
"Correct usage: `{\"type\": \"function\", "
"\"function\": {\"name\": \"my_function\"}}`")
f'Invalid value for `function`: `{function}` in '
'`tool_choice`! Correct usage: `{"type": "function",'
' "function": {"name": "my_function"}}`')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This condition checks if function is not a dictionary or if function_name exists but is not a string. It could be simplified by extracting the condition into a separate variable for better readability and maintainability.

Also, consider adding a check to ensure that the function_name is not empty to prevent unexpected behavior if the name is an empty string.

function_name_valid = isinstance(function, dict) and \
                           isinstance(function.get("name"), str)
                if not function_name_valid:
                    raise ValueError(
                        f'Invalid value for `function`: `{function}` in '
                        '`tool_choice`! Correct usage: `{"type": "function",'
                        ' "function": {"name": "my_function"}}`')

for tool in data["tools"]:
if tool["function"]["name"] == specified_function_name:
if tool["function"]["name"] == function_name:
valid_tool = True
break
if not valid_tool:
Expand Down