[Bugfix][Frontend] Malformed $defs in a tool schema should be a 400, not a 500 (#54850)
Signed-off-by: lzhan011 <35493221+lzhan011@users.noreply.github.com> Signed-off-by: Flora Feng <4florafeng@gmail.com> Co-authored-by: Flora Feng <4florafeng@gmail.com>
This commit is contained in:
@@ -410,6 +410,101 @@ class TestNonFunctionToolsSkipped:
|
||||
get_json_schema_from_tools(tools=[WEB_SEARCH_TOOL], tool_choice="required")
|
||||
|
||||
|
||||
class TestMalformedToolSchemaDefs:
|
||||
"""Malformed `$defs` in caller-supplied tool parameters is a 400, not a 500."""
|
||||
|
||||
@staticmethod
|
||||
def _tool(params: dict) -> ChatCompletionToolsParam:
|
||||
return TypeAdapter(ChatCompletionToolsParam).validate_python(
|
||||
{
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "get_weather",
|
||||
"description": "Get the weather",
|
||||
"parameters": params,
|
||||
},
|
||||
}
|
||||
)
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"defs",
|
||||
[
|
||||
pytest.param(None, id="null"),
|
||||
pytest.param([], id="list"),
|
||||
pytest.param("nope", id="string"),
|
||||
pytest.param(1, id="int"),
|
||||
],
|
||||
)
|
||||
def test_non_object_defs_is_a_client_error(self, defs):
|
||||
tools = [
|
||||
self._tool(
|
||||
{
|
||||
"type": "object",
|
||||
"properties": {"a": {"type": "string"}},
|
||||
"$defs": defs,
|
||||
}
|
||||
)
|
||||
]
|
||||
|
||||
with pytest.raises(VLLMValidationError) as exc_info:
|
||||
get_json_schema_from_tools(tools=tools, tool_choice="required")
|
||||
|
||||
assert exc_info.value.parameter == "tools"
|
||||
assert "$defs" in str(exc_info.value)
|
||||
assert "get_weather" in str(exc_info.value)
|
||||
|
||||
def test_duplicate_def_names_is_a_client_error(self):
|
||||
"""Also caller-caused, and it used to raise a plain `ValueError`."""
|
||||
tools = [
|
||||
self._tool(
|
||||
{
|
||||
"type": "object",
|
||||
"properties": {"a": {"$ref": "#/$defs/D"}},
|
||||
"$defs": {"D": {"type": "string"}},
|
||||
}
|
||||
),
|
||||
self._tool(
|
||||
{
|
||||
"type": "object",
|
||||
"properties": {"b": {"$ref": "#/$defs/D"}},
|
||||
"$defs": {"D": {"type": "integer"}},
|
||||
}
|
||||
),
|
||||
]
|
||||
|
||||
with pytest.raises(VLLMValidationError) as exc_info:
|
||||
get_json_schema_from_tools(tools=tools, tool_choice="required")
|
||||
|
||||
assert exc_info.value.parameter == "tools"
|
||||
assert "multiple schemas" in str(exc_info.value)
|
||||
|
||||
def test_well_formed_defs_still_hoisted(self):
|
||||
"""The type check must not reject the shape it is guarding."""
|
||||
tools = [
|
||||
self._tool(
|
||||
{
|
||||
"type": "object",
|
||||
"properties": {"a": {"$ref": "#/$defs/D"}},
|
||||
"$defs": {"D": {"type": "string"}},
|
||||
}
|
||||
)
|
||||
]
|
||||
|
||||
schema = get_json_schema_from_tools(tools=tools, tool_choice="required")
|
||||
|
||||
assert isinstance(schema, dict)
|
||||
assert schema["$defs"] == {"D": {"type": "string"}}
|
||||
|
||||
def test_absent_defs_still_works(self):
|
||||
tools = [
|
||||
self._tool({"type": "object", "properties": {"a": {"type": "string"}}})
|
||||
]
|
||||
|
||||
schema = get_json_schema_from_tools(tools=tools, tool_choice="required")
|
||||
|
||||
assert isinstance(schema, dict)
|
||||
|
||||
|
||||
class TestParallelToolCallsConstraint:
|
||||
"""`parallel_tool_calls=false` must be enforced by the decoding grammar.
|
||||
|
||||
|
||||
@@ -368,15 +368,22 @@ def _get_tool_schema_defs(
|
||||
) -> dict:
|
||||
all_defs: dict[str, dict[str, Any]] = {}
|
||||
for tool in tools:
|
||||
_, params = _extract_tool_info(tool)
|
||||
name, params = _extract_tool_info(tool)
|
||||
if params is None:
|
||||
continue
|
||||
defs = params.pop("$defs", {})
|
||||
if not isinstance(defs, dict):
|
||||
raise VLLMValidationError(
|
||||
f"`$defs` in the parameters of tool '{name}' must be an "
|
||||
f"object, got {type(defs).__name__}.",
|
||||
parameter="tools",
|
||||
)
|
||||
for def_name, def_schema in defs.items():
|
||||
if def_name in all_defs and all_defs[def_name] != def_schema:
|
||||
raise ValueError(
|
||||
raise VLLMValidationError(
|
||||
f"Tool definition '{def_name}' has multiple schemas, "
|
||||
"which is not supported."
|
||||
"which is not supported.",
|
||||
parameter="tools",
|
||||
)
|
||||
all_defs[def_name] = def_schema
|
||||
return all_defs
|
||||
|
||||
Reference in New Issue
Block a user