From a10d69116cb25c8137eeb3f320add71d4e04fda9 Mon Sep 17 00:00:00 2001 From: Flora Feng <4florafeng@gmail.com> Date: Wed, 20 May 2026 10:21:00 -0400 Subject: [PATCH] [Bugfix] Use shared coerce_to_schema_type in DeepSeekV32 tool parser (#43019) Signed-off-by: sfeng33 <4florafeng@gmail.com> --- .../test_deepseekv32_tool_parser.py | 494 +++++++++--------- vllm/tool_parsers/deepseekv32_tool_parser.py | 61 +-- 2 files changed, 270 insertions(+), 285 deletions(-) diff --git a/tests/tool_parsers/test_deepseekv32_tool_parser.py b/tests/tool_parsers/test_deepseekv32_tool_parser.py index 693cf5caddd..0d37778e406 100644 --- a/tests/tool_parsers/test_deepseekv32_tool_parser.py +++ b/tests/tool_parsers/test_deepseekv32_tool_parser.py @@ -16,7 +16,6 @@ from vllm.entrypoints.openai.chat_completion.protocol import ( ChatCompletionToolsParam, FunctionDefinition, ) -from vllm.tokenizers import get_tokenizer from vllm.tool_parsers.deepseekv32_tool_parser import DeepSeekV32ToolParser # --------------------------------------------------------------------------- @@ -65,58 +64,6 @@ def build_tool_call(func_name: str, params: dict[str, str]) -> str: return f'{FC_START}\n{INV_START}{func_name}">\n{param_strs}\n{INV_END}\n{FC_END}' -# --------------------------------------------------------------------------- -# Tests: DeepSeekV32ToolParser._convert_param_value -# --------------------------------------------------------------------------- - - -class TestConvertParamValue: - @pytest.fixture - def parser(self): - return make_parser() - - def test_null(self, parser): - assert parser._convert_param_value("null", "string") is None - assert parser._convert_param_value("NULL", "integer") is None - - def test_string(self, parser): - assert parser._convert_param_value("hello", "string") == "hello" - - def test_integer_valid(self, parser): - assert parser._convert_param_value("42", "integer") == 42 - - def test_integer_invalid_falls_back_to_str(self, parser): - assert parser._convert_param_value("abc", "int") == "abc" - - def test_number_float(self, parser): - assert parser._convert_param_value("3.14", "number") == pytest.approx(3.14) - - def test_number_whole_returns_int(self, parser): - assert parser._convert_param_value("5.0", "number") == 5 - assert isinstance(parser._convert_param_value("5.0", "number"), int) - - def test_boolean_true(self, parser): - assert parser._convert_param_value("true", "boolean") is True - assert parser._convert_param_value("1", "bool") is True - - def test_boolean_false(self, parser): - assert parser._convert_param_value("false", "boolean") is False - assert parser._convert_param_value("False", "bool") is False - - def test_object_valid_json(self, parser): - assert parser._convert_param_value('{"k": 1}', "object") == {"k": 1} - - def test_object_invalid_json_falls_back(self, parser): - assert parser._convert_param_value("not-json", "object") == "not-json" - - def test_array_valid_json(self, parser): - assert parser._convert_param_value("[1, 2]", "array") == [1, 2] - - def test_unknown_type_tries_json_then_string(self, parser): - assert parser._convert_param_value("123", "unknown") == 123 - assert parser._convert_param_value("hello", "unknown") == "hello" - - # --------------------------------------------------------------------------- # Tests: extract_tool_calls (non-streaming) # --------------------------------------------------------------------------- @@ -331,6 +278,163 @@ class TestExtractToolCalls: args = json.loads(result.tool_calls[0].function.arguments) assert args == {"location": "Beijing"} + def test_object_and_array_params(self): + """Object and array schema types must be JSON-parsed, not left as strings.""" + tool = ChatCompletionToolsParam( + function=FunctionDefinition( + name="update", + parameters={ + "type": "object", + "properties": { + "tags": {"type": "array"}, + "meta": {"type": "object"}, + }, + }, + ), + ) + parser = make_parser(tools=[tool]) + model_output = ( + f"{FC_START}\n" + f'{INV_START}update">\n' + f'{PARAM_START}tags" string="false">["a", "b"]{PARAM_END}\n' + f'{PARAM_START}meta" string="false">{{"k": 1}}{PARAM_END}\n' + f"{INV_END}\n" + f"{FC_END}" + ) + result = parser.extract_tool_calls(model_output, None) + assert result.tools_called + args = json.loads(result.tool_calls[0].function.arguments) + assert args["tags"] == ["a", "b"] + assert isinstance(args["tags"], list) + assert args["meta"] == {"k": 1} + assert isinstance(args["meta"], dict) + + def test_number_param(self): + """Number (float) schema type must be converted correctly.""" + tool = ChatCompletionToolsParam( + function=FunctionDefinition( + name="measure", + parameters={ + "type": "object", + "properties": { + "ratio": {"type": "number"}, + "whole": {"type": "number"}, + }, + }, + ), + ) + parser = make_parser(tools=[tool]) + model_output = ( + f"{FC_START}\n" + f'{INV_START}measure">\n' + f'{PARAM_START}ratio" string="false">3.14{PARAM_END}\n' + f'{PARAM_START}whole" string="false">5.0{PARAM_END}\n' + f"{INV_END}\n" + f"{FC_END}" + ) + result = parser.extract_tool_calls(model_output, None) + args = json.loads(result.tool_calls[0].function.arguments) + assert args["ratio"] == pytest.approx(3.14) + assert args["whole"] == 5 + assert isinstance(args["whole"], int) + + def test_multi_typed_schema(self): + """Schema with type: ["integer", "null"] must handle both cases.""" + tool = ChatCompletionToolsParam( + function=FunctionDefinition( + name="set_val", + parameters={ + "type": "object", + "properties": { + "count": {"type": ["integer", "null"]}, + "label": {"type": ["string", "null"]}, + }, + }, + ), + ) + parser = make_parser(tools=[tool]) + model_output = ( + f"{FC_START}\n" + f'{INV_START}set_val">\n' + f'{PARAM_START}count" string="false">42{PARAM_END}\n' + f'{PARAM_START}label" string="false">hello{PARAM_END}\n' + f"{INV_END}\n" + f"{FC_END}" + ) + result = parser.extract_tool_calls(model_output, None) + args = json.loads(result.tool_calls[0].function.arguments) + assert args["count"] == 42 + assert isinstance(args["count"], int) + assert args["label"] == "hello" + + def test_multi_typed_null_value(self): + """Literal 'null' must become None when the schema includes 'null'.""" + tool = ChatCompletionToolsParam( + function=FunctionDefinition( + name="clear", + parameters={ + "type": "object", + "properties": { + "value": {"type": ["integer", "null"]}, + }, + }, + ), + ) + parser = make_parser(tools=[tool]) + model_output = ( + f"{FC_START}\n" + f'{INV_START}clear">\n' + f'{PARAM_START}value" string="false">null{PARAM_END}\n' + f"{INV_END}\n" + f"{FC_END}" + ) + result = parser.extract_tool_calls(model_output, None) + args = json.loads(result.tool_calls[0].function.arguments) + assert args["value"] is None + + def test_null_not_coerced_without_null_in_schema(self): + """Literal 'null' must stay as a string when the schema is just 'string'.""" + tool = ChatCompletionToolsParam( + function=FunctionDefinition( + name="echo", + parameters={ + "type": "object", + "properties": { + "text": {"type": "string"}, + }, + }, + ), + ) + parser = make_parser(tools=[tool]) + model_output = ( + f"{FC_START}\n" + f'{INV_START}echo">\n' + f'{PARAM_START}text" string="false">null{PARAM_END}\n' + f"{INV_END}\n" + f"{FC_END}" + ) + result = parser.extract_tool_calls(model_output, None) + args = json.loads(result.tool_calls[0].function.arguments) + assert args["text"] == "null" + assert isinstance(args["text"], str) + + def test_no_schema_keeps_strings(self): + """Without a tool schema, all string='false' params default to string.""" + parser = make_parser(tools=None) + model_output = ( + f"{FC_START}\n" + f'{INV_START}unknown_fn">\n' + f'{PARAM_START}count" string="false">42{PARAM_END}\n' + f'{PARAM_START}flag" string="false">true{PARAM_END}\n' + f"{INV_END}\n" + f"{FC_END}" + ) + result = parser.extract_tool_calls(model_output, None) + assert result.tools_called + args = json.loads(result.tool_calls[0].function.arguments) + assert args["count"] == "42" + assert args["flag"] == "true" + # --------------------------------------------------------------------------- # Tests: extract_tool_calls_streaming @@ -713,6 +817,110 @@ class TestExtractToolCallsStreaming: content = "".join(d.content for d in deltas if d.content is not None) assert content == full_text + def test_object_and_array_params_streaming(self): + """Streaming: object/array params must be JSON-parsed.""" + tool = ChatCompletionToolsParam( + function=FunctionDefinition( + name="update", + parameters={ + "type": "object", + "properties": { + "tags": {"type": "array"}, + "meta": {"type": "object"}, + }, + }, + ), + ) + parser = make_parser(tools=[tool]) + full_text = ( + f"{FC_START}\n" + f'{INV_START}update">\n' + f'{PARAM_START}tags" string="false">["a", "b"]{PARAM_END}\n' + f'{PARAM_START}meta" string="false">{{"k": 1}}{PARAM_END}\n' + f"{INV_END}\n" + f"{FC_END}" + ) + deltas = self._stream(parser, full_text) + args = json.loads(self._reconstruct_args(deltas)) + assert args["tags"] == ["a", "b"] + assert args["meta"] == {"k": 1} + + def test_multi_typed_schema_streaming(self): + """Streaming: type: ["integer", "null"] must coerce correctly.""" + tool = ChatCompletionToolsParam( + function=FunctionDefinition( + name="set_val", + parameters={ + "type": "object", + "properties": { + "count": {"type": ["integer", "null"]}, + }, + }, + ), + ) + parser = make_parser(tools=[tool]) + full_text = ( + f"{FC_START}\n" + f'{INV_START}set_val">\n' + f'{PARAM_START}count" string="false">42{PARAM_END}\n' + f"{INV_END}\n" + f"{FC_END}" + ) + deltas = self._stream(parser, full_text) + args = json.loads(self._reconstruct_args(deltas)) + assert args["count"] == 42 + assert isinstance(args["count"], int) + + def test_multi_typed_null_streaming(self): + """Streaming: 'null' with ["integer", "null"] schema must become None.""" + tool = ChatCompletionToolsParam( + function=FunctionDefinition( + name="clear", + parameters={ + "type": "object", + "properties": { + "value": {"type": ["integer", "null"]}, + }, + }, + ), + ) + parser = make_parser(tools=[tool]) + full_text = ( + f"{FC_START}\n" + f'{INV_START}clear">\n' + f'{PARAM_START}value" string="false">null{PARAM_END}\n' + f"{INV_END}\n" + f"{FC_END}" + ) + deltas = self._stream(parser, full_text) + args = json.loads(self._reconstruct_args(deltas)) + assert args["value"] is None + + def test_number_param_streaming(self): + """Streaming: number type must be converted.""" + tool = ChatCompletionToolsParam( + function=FunctionDefinition( + name="measure", + parameters={ + "type": "object", + "properties": { + "ratio": {"type": "number"}, + }, + }, + ), + ) + parser = make_parser(tools=[tool]) + full_text = ( + f"{FC_START}\n" + f'{INV_START}measure">\n' + f'{PARAM_START}ratio" string="false">3.14{PARAM_END}\n' + f"{INV_END}\n" + f"{FC_END}" + ) + deltas = self._stream(parser, full_text) + args = json.loads(self._reconstruct_args(deltas)) + assert args["ratio"] == pytest.approx(3.14) + class TestDelimiterPreservation: """Regression: fast detokenization skipping DSML delimiters (PR #33964).""" @@ -794,183 +1002,3 @@ class TestDelimiterPreservation: assert reconstructor.tool_calls[0].function.name == "search" streamed_args = json.loads(reconstructor.tool_calls[0].function.arguments) assert streamed_args == ns_args - - -@pytest.fixture(scope="module") -def deepseekv32_tokenizer(): - return get_tokenizer(tokenizer_name="deepseek-ai/DeepSeek-V3.2") - - -@pytest.fixture -def parser(deepseekv32_tokenizer): - return DeepSeekV32ToolParser(deepseekv32_tokenizer) - - -def test_convert_param_value_single_types(parser): - """Test _convert_param_value with single type parameters.""" - # Test string type - assert parser._convert_param_value("hello", "string") == "hello" - assert parser._convert_param_value("123", "string") == "123" - - # Test integer type - valid integers - assert parser._convert_param_value("123", "integer") == 123 - assert parser._convert_param_value("456", "int") == 456 - # Invalid integer should return original string (due to exception catch) - assert parser._convert_param_value("abc", "integer") == "abc" - - # Test float/number type - assert parser._convert_param_value("123.45", "float") == 123.45 - assert ( - parser._convert_param_value("123.0", "number") == 123 - ) # Should be int when whole number - assert parser._convert_param_value("123.5", "number") == 123.5 - # Invalid float should return original string - assert parser._convert_param_value("abc", "float") == "abc" - - # Test boolean type - valid boolean values - assert parser._convert_param_value("true", "boolean") is True - assert parser._convert_param_value("false", "bool") is False - assert parser._convert_param_value("1", "boolean") is True - assert parser._convert_param_value("0", "boolean") is False - # Invalid boolean should return original string - assert parser._convert_param_value("yes", "boolean") == "yes" - assert parser._convert_param_value("no", "bool") == "no" - - # Test null value - assert parser._convert_param_value("null", "string") is None - assert parser._convert_param_value("null", "integer") is None - - # Test object/array type (JSON) - assert parser._convert_param_value('{"key": "value"}', "object") == {"key": "value"} - assert parser._convert_param_value("[1, 2, 3]", "array") == [1, 2, 3] - # Invalid JSON should return original string - assert parser._convert_param_value("{invalid}", "object") == "{invalid}" - - # Test fallback for unknown type (tries json.loads, then returns original) - assert parser._convert_param_value('{"key": "value"}', "unknown") == { - "key": "value" - } - assert parser._convert_param_value("plain text", "unknown") == "plain text" - - -def test_convert_param_value_multi_typed_values(parser): - """Test _convert_param_value with multi-typed values (list of types).""" - # Test with list of types where first type succeeds - assert parser._convert_param_value("123", ["integer", "string"]) == 123 - assert parser._convert_param_value("true", ["boolean", "string"]) is True - assert parser._convert_param_value('{"x": 1}', ["object", "string"]) == {"x": 1} - - # Test with list of types where first type fails but second succeeds - # "abc" is not a valid integer, so should try string next - assert parser._convert_param_value("abc", ["integer", "string"]) == "abc" - - # Test with list of types where all fail - should return original value - # "invalid json" is not valid JSON, last type is "object" which will fail JSON parse - result = parser._convert_param_value("invalid json", ["integer", "object"]) - assert result == "invalid json" # Returns original value after all types fail - - # Test with three types - assert parser._convert_param_value("123.5", ["integer", "float", "string"]) == 123.5 - assert parser._convert_param_value("true", ["integer", "boolean", "string"]) is True - - # Test with null in multi-type list - assert parser._convert_param_value("null", ["integer", "string"]) is None - assert parser._convert_param_value("null", ["boolean", "object"]) is None - - # Test nested type conversion - boolean fails, integer succeeds - value = parser._convert_param_value("123", ["boolean", "integer", "string"]) - assert value == 123 # Should be integer, not boolean - - # Test that order matters - assert ( - parser._convert_param_value("123", ["string", "integer"]) == "123" - ) # String first - assert ( - parser._convert_param_value("123", ["integer", "string"]) == 123 - ) # Integer first - - # Test with all types failing - returns original value - assert ( - parser._convert_param_value("not_a_number", ["integer", "float", "boolean"]) - == "not_a_number" - ) - - -def test_convert_param_value_stricter_type_checking(parser): - """Test stricter type checking in the updated implementation.""" - # Boolean now has stricter validation - assert parser._convert_param_value("true", "boolean") is True - assert parser._convert_param_value("false", "boolean") is False - assert parser._convert_param_value("1", "boolean") is True - assert parser._convert_param_value("0", "boolean") is False - - # These should return original string (not valid boolean values) - assert parser._convert_param_value("yes", "boolean") == "yes" - assert parser._convert_param_value("no", "boolean") == "no" - assert parser._convert_param_value("TRUE", "boolean") is True - assert parser._convert_param_value("FALSE", "boolean") is False - - # Integer and float now raise exceptions for invalid values - assert parser._convert_param_value("123abc", "integer") == "123abc" - assert parser._convert_param_value("123.45.67", "float") == "123.45.67" - - # JSON parsing is stricter - invalid JSON returns original - assert parser._convert_param_value("{invalid: json}", "object") == "{invalid: json}" - assert parser._convert_param_value("[1, 2,", "array") == "[1, 2," - - # Test multi-type with stricter checking - # "yes" is not valid boolean, but string would accept it - assert parser._convert_param_value("yes", ["boolean", "string"]) == "yes" - - # "123abc" is not valid integer or float, but string accepts it - assert ( - parser._convert_param_value("123abc", ["integer", "float", "string"]) - == "123abc" - ) - - -def test_convert_param_value_edge_cases(parser): - """Test edge cases for _convert_param_value.""" - # Empty string - assert parser._convert_param_value("", "string") == "" - assert ( - parser._convert_param_value("", "integer") == "" - ) # Invalid int returns original - - # Whitespace - trimmed by conversion functions - assert parser._convert_param_value(" 123 ", "integer") == 123 - assert parser._convert_param_value(" true ", "boolean") is True - - # Numeric strings with special characters - assert parser._convert_param_value("123.45.67", "float") == "123.45.67" - assert parser._convert_param_value("123abc", "integer") == "123abc" - - # JSON with whitespace - should parse correctly - assert parser._convert_param_value(' { "key" : "value" } ', "object") == { - "key": "value" - } - - # Invalid JSON returns original - assert parser._convert_param_value("{invalid}", "object") == "{invalid}" - assert parser._convert_param_value("[1, 2,", "array") == "[1, 2," - - -def test_convert_param_value_checked_helper(parser): - """Test the _convert_param_value_checked helper function indirectly.""" - # This tests the behavior through the main function - # Valid conversions should work - assert parser._convert_param_value("123", "integer") == 123 - assert parser._convert_param_value("123.45", "float") == 123.45 - assert parser._convert_param_value("true", "boolean") is True - assert parser._convert_param_value('{"x": 1}', "object") == {"x": 1} - - # Invalid conversions should return original value (exception caught) - assert parser._convert_param_value("abc", "integer") == "abc" - assert parser._convert_param_value("abc", "float") == "abc" - assert parser._convert_param_value("yes", "boolean") == "yes" - assert parser._convert_param_value("{invalid}", "object") == "{invalid}" - - # Test that null handling works in checked function - assert parser._convert_param_value("null", "integer") is None - assert parser._convert_param_value("null", "boolean") is None - assert parser._convert_param_value("null", "object") is None diff --git a/vllm/tool_parsers/deepseekv32_tool_parser.py b/vllm/tool_parsers/deepseekv32_tool_parser.py index f01f7f92942..511b6e7c359 100644 --- a/vllm/tool_parsers/deepseekv32_tool_parser.py +++ b/vllm/tool_parsers/deepseekv32_tool_parser.py @@ -26,7 +26,12 @@ from vllm.tool_parsers.abstract_tool_parser import ( Tool, ToolParser, ) -from vllm.tool_parsers.utils import partial_tag_overlap +from vllm.tool_parsers.utils import ( + coerce_to_schema_type, + extract_types_from_schema, + find_tool_properties, + partial_tag_overlap, +) logger = init_logger(__name__) @@ -109,41 +114,6 @@ class DeepSeekV32ToolParser(ToolParser): param_dict[param_name] = (param_val, string_attr) return param_dict - def _convert_param_value_checked(self, value: str, param_type: str) -> Any: - """Convert parameter value to the correct type.""" - if value.lower() == "null": - return None - - param_type = param_type.lower() - if param_type in ["string", "str", "text"]: - return value - elif param_type in ["integer", "int"]: - return int(value) - elif param_type in ["number", "float"]: - val = float(value) - return val if val != int(val) else int(val) - elif param_type in ["boolean", "bool"]: - value = value.strip() - if value.lower() not in ["false", "0", "true", "1"]: - raise ValueError("Invalid boolean value") - return value.lower() in ["true", "1"] - elif param_type in ["object", "array"]: - return json.loads(value) - else: - return json.loads(value) - - def _convert_param_value(self, value: str, param_type: str | list[str]) -> Any: - """Convert parameter value to the correct type.""" - if not isinstance(param_type, list): - param_type = [param_type] - for current_type in param_type: - try: - return self._convert_param_value_checked(value, current_type) - except Exception: - continue - # return value as fallback - return value - @staticmethod def _repair_param_dict( param_dict: dict[str, Any], @@ -172,18 +142,7 @@ class DeepSeekV32ToolParser(ToolParser): param_dict: dict[str, tuple[str, str]], ) -> dict[str, Any]: """Convert raw string param values using the tool schema types.""" - param_config: dict = {} - if self.tools: - for tool in self.tools: - if ( - hasattr(tool, "function") - and tool.function.name == function_name - and hasattr(tool.function, "parameters") - ): - schema = tool.function.parameters - if isinstance(schema, dict) and "properties" in schema: - param_config = schema["properties"] - break + param_config = find_tool_properties(self.tools, function_name) converted: dict[str, Any] = {} for name, (value, string_attr) in param_dict.items(): @@ -191,10 +150,8 @@ class DeepSeekV32ToolParser(ToolParser): converted[name] = value continue - param_type = "string" - if name in param_config and isinstance(param_config[name], dict): - param_type = param_config[name].get("type", "string") - converted[name] = self._convert_param_value(value, param_type) + param_types = extract_types_from_schema(param_config.get(name, {})) + converted[name] = coerce_to_schema_type(value, param_types) return self._repair_param_dict(converted, param_config) def extract_tool_calls(