diff --git a/tests/tool_parsers/test_lfm2_tool_parser.py b/tests/tool_parsers/test_lfm2_tool_parser.py index 9cb5b195f1..0cb865fef5 100644 --- a/tests/tool_parsers/test_lfm2_tool_parser.py +++ b/tests/tool_parsers/test_lfm2_tool_parser.py @@ -54,6 +54,34 @@ EMPTY_LIST_FUNCTION_CALL = FunctionCall( name="do_something_cool", arguments='{"steps": []}', ) +SET_ARG_FUNCTION_OUTPUT = "label(tags={'urgent', 'bug'})" +SET_ARG_FUNCTION_CALL = FunctionCall( + name="label", + arguments='{"tags": ["urgent", "bug"]}', +) +LEADING_ZERO_FUNCTION_OUTPUT = "set_date(month=07, day=05)" +LEADING_ZERO_FUNCTION_CALL = FunctionCall( + name="set_date", + arguments='{"month": 7, "day": 5}', +) +NESTED_QUOTE_FUNCTION_OUTPUT = ( + "bash(command='sed -n '360,450p' /testbed/sympy/matrices/common.py')" +) +NESTED_QUOTE_FUNCTION_CALL = FunctionCall( + name="bash", + arguments=('{"command": "sed -n \'360,450p\' /testbed/sympy/matrices/common.py"}'), +) +NESTED_PYTHON_C_FUNCTION_OUTPUT = ( + "bash(command='cd /testbed && python3 -c \"from sympy import latex\n" + "print(latex(3*x, mul_symbol='\\,'))\"')" +) +NESTED_PYTHON_C_FUNCTION_CALL = FunctionCall( + name="bash", + arguments=( + '{"command": "cd /testbed && python3 -c \\"from sympy import latex\\n' + "print(latex(3*x, mul_symbol='\\\\,'))\\\"\"}" + ), +) ESCAPED_STRING_FUNCTION_OUTPUT = ( r"get_weather(city='Martha\'s Vineyard', metric='\"cool units\"')" ) @@ -74,6 +102,29 @@ DOTTED_NAME_FUNCTION_CALL = FunctionCall( '"deliveryAddress": "845 Willow Lane, Springfield, IL 62704"}' ), ) +# Agentic shell commands: a bracket inside a string argument must not corrupt +# the streaming bracket tracker. +BRACKET_IN_STRING_FUNCTION_OUTPUT = "exec(command='grep -F \"]\" log.txt')" +BRACKET_IN_STRING_FUNCTION_CALL = FunctionCall( + name="exec", + arguments='{"command": "grep -F \\"]\\" log.txt"}', +) +# A raw newline inside a string argument (multi-line shell command) is invalid +# Python; the parser must recover the full value instead of dropping the call. +MULTILINE_FUNCTION_OUTPUT = ( + "exec(command='cat > f.py << EOF\nimport csv\nprint(1)\nEOF')" +) +MULTILINE_FUNCTION_CALL = FunctionCall( + name="exec", + arguments='{"command": "cat > f.py << EOF\\nimport csv\\nprint(1)\\nEOF"}', +) +# A NUL byte inside a string argument makes ast.parse raise ValueError (not +# SyntaxError) for the whole source; the escape path must recover the call. +NUL_BYTE_FUNCTION_OUTPUT = "exec(command='printf a\x00b')" +NUL_BYTE_FUNCTION_CALL = FunctionCall( + name="exec", + arguments='{"command": "printf a\\u0000b"}', +) @pytest.fixture(scope="module") @@ -227,6 +278,112 @@ TEST_CASES = [ None, id="dotted_name_nonstreaming", ), + # Messy agentic shell commands: literal bracket inside a string argument + pytest.param( + True, + _wrap(BRACKET_IN_STRING_FUNCTION_OUTPUT), + [BRACKET_IN_STRING_FUNCTION_CALL], + None, + id="bracket_in_string_streaming", + ), + pytest.param( + False, + _wrap(BRACKET_IN_STRING_FUNCTION_OUTPUT), + [BRACKET_IN_STRING_FUNCTION_CALL], + None, + id="bracket_in_string_nonstreaming", + ), + # Multi-line string argument (raw newlines in a shell command) + pytest.param( + True, + _wrap(MULTILINE_FUNCTION_OUTPUT), + [MULTILINE_FUNCTION_CALL], + None, + id="multiline_string_arg_streaming", + ), + pytest.param( + False, + _wrap(MULTILINE_FUNCTION_OUTPUT), + [MULTILINE_FUNCTION_CALL], + None, + id="multiline_string_arg_nonstreaming", + ), + # Set argument decoded as a list (JSON has no set type) + pytest.param( + True, + _wrap(SET_ARG_FUNCTION_OUTPUT), + [SET_ARG_FUNCTION_CALL], + None, + id="set_arg_streaming", + ), + pytest.param( + False, + _wrap(SET_ARG_FUNCTION_OUTPUT), + [SET_ARG_FUNCTION_CALL], + None, + id="set_arg_nonstreaming", + ), + # Unescaped same-style quotes nested in a shell command argument + pytest.param( + True, + _wrap(NESTED_QUOTE_FUNCTION_OUTPUT), + [NESTED_QUOTE_FUNCTION_CALL], + None, + id="nested_quote_streaming", + ), + pytest.param( + False, + _wrap(NESTED_QUOTE_FUNCTION_OUTPUT), + [NESTED_QUOTE_FUNCTION_CALL], + None, + id="nested_quote_nonstreaming", + ), + # Doubly nested: multi-line python -c payload with its own inner quotes + pytest.param( + True, + _wrap(NESTED_PYTHON_C_FUNCTION_OUTPUT), + [NESTED_PYTHON_C_FUNCTION_CALL], + None, + id="nested_python_c_streaming", + ), + pytest.param( + False, + _wrap(NESTED_PYTHON_C_FUNCTION_OUTPUT), + [NESTED_PYTHON_C_FUNCTION_CALL], + None, + id="nested_python_c_nonstreaming", + ), + # Zero-padded integer arguments (SyntaxError: leading zeros) + pytest.param( + True, + _wrap(LEADING_ZERO_FUNCTION_OUTPUT), + [LEADING_ZERO_FUNCTION_CALL], + None, + id="leading_zero_int_streaming", + ), + pytest.param( + False, + _wrap(LEADING_ZERO_FUNCTION_OUTPUT), + [LEADING_ZERO_FUNCTION_CALL], + None, + id="leading_zero_int_nonstreaming", + ), + # NUL byte in a string argument (ValueError from ast.parse, not + # SyntaxError) + pytest.param( + True, + _wrap(NUL_BYTE_FUNCTION_OUTPUT), + [NUL_BYTE_FUNCTION_CALL], + None, + id="nul_byte_string_arg_streaming", + ), + pytest.param( + False, + _wrap(NUL_BYTE_FUNCTION_OUTPUT), + [NUL_BYTE_FUNCTION_CALL], + None, + id="nul_byte_string_arg_nonstreaming", + ), ] @@ -255,6 +412,47 @@ def test_tool_call( assert actual.function == expected +@pytest.mark.parametrize("body", ["", " "]) +def test_empty_tool_call_block(body: str, lfm2_tokenizer: TokenizerLike): + """An empty block ([] or [ ]) means the model called no tools; it must + not report tools_called=True with zero tool calls (the invariant + run_tool_extraction asserts).""" + cls = ToolParserManager.get_tool_parser("lfm2") + model_output = f"{TOOL_CALL_START}[{body}]{TOOL_CALL_END}" + + content, tool_calls = run_tool_extraction( + cls(lfm2_tokenizer), model_output, streaming=False + ) + assert len(tool_calls) == 0 + + reconstructor = run_tool_extraction_streaming( + cls(lfm2_tokenizer), [model_output], assert_one_tool_per_delta=False + ) + assert len(reconstructor.tool_calls) == 0 + + +def test_whitespace_after_start_token(lfm2_tokenizer: TokenizerLike): + """Whitespace between <|tool_call_start|> and the opening bracket must + not break parsing. The streaming path used to feed the indented text to + ast.parse verbatim, so every completion candidate raised + IndentationError and the call was silently dropped (non-streaming + stripped it and succeeded).""" + cls = ToolParserManager.get_tool_parser("lfm2") + model_output = f"{TOOL_CALL_START} [{SIMPLE_FUNCTION_OUTPUT}]{TOOL_CALL_END}" + + content, tool_calls = run_tool_extraction( + cls(lfm2_tokenizer), model_output, streaming=False + ) + assert len(tool_calls) == 1 + assert tool_calls[0].function == SIMPLE_FUNCTION_CALL + + reconstructor = run_tool_extraction_streaming( + cls(lfm2_tokenizer), [model_output], assert_one_tool_per_delta=False + ) + assert len(reconstructor.tool_calls) == 1 + assert reconstructor.tool_calls[0].function == SIMPLE_FUNCTION_CALL + + def test_streaming_tool_call_with_large_steps(lfm2_tokenizer: TokenizerLike): tool_parser: ToolParser = ToolParserManager.get_tool_parser("lfm2")(lfm2_tokenizer) model_output_deltas = [ diff --git a/tests/tool_parsers/test_utils.py b/tests/tool_parsers/test_utils.py index 3276fa9ddd..f8890c2d35 100644 --- a/tests/tool_parsers/test_utils.py +++ b/tests/tool_parsers/test_utils.py @@ -1,13 +1,24 @@ # SPDX-License-Identifier: Apache-2.0 # SPDX-FileCopyrightText: Copyright contributors to the vLLM project +import ast import json import pytest from vllm.tool_parsers.utils import ( + UnexpectedAstError, coerce_to_schema_type, + contains_broken_string_literal, + escape_ctrl_chars_in_strings, + escape_nested_quotes_in_strings, extract_types_from_schema, + get_parameter_value, + handle_single_tool, + make_valid_python, + normalize_leading_zero_ints, + rename_reserved_kwargs, + restore_reserved_kwarg_names, ) @@ -279,3 +290,477 @@ class TestExtractTypesFromSchema: } result = set(extract_types_from_schema(schema)) assert result == {"integer", "null", "string"} + + +def _value_of(expr: str): + """Parse a single Python expression and run get_parameter_value on it.""" + return get_parameter_value(ast.parse(expr, mode="eval").body) + + +def _first_call(text: str) -> ast.Call: + """Parse ``[foo(...)]`` and return the single ast.Call node.""" + statement = ast.parse(text).body[0] + assert isinstance(statement, ast.Expr) + assert isinstance(statement.value, ast.List) + call = statement.value.elts[0] + assert isinstance(call, ast.Call) + return call + + +def _bare_call(text: str) -> ast.Call: + """Parse ``foo(...)`` (no list wrapper) and return the ast.Call node.""" + statement = ast.parse(text).body[0] + assert isinstance(statement, ast.Expr) + assert isinstance(statement.value, ast.Call) + return statement.value + + +def _kwarg_constant(call: ast.Call, index: int = 0): + """Return the constant value of the call's ``index``-th keyword arg.""" + value = call.keywords[index].value + assert isinstance(value, ast.Constant) + return value.value + + +class TestMakeValidPythonStringLiterals: + def test_bracket_inside_string_is_literal(self): + # A bracket inside a string argument must not be counted as a + # structural bracket. Regression: `]` inside the string popped the + # bracket stack and the whole call raised as mismatched. + text = "[exec(command='grep -F \"]\" log.txt')]" + assert make_valid_python(text) == (text, "") + + def test_open_bracket_inside_string_is_literal(self): + # An unclosed `[` inside a string must not leave a phantom open + # bracket on the stack. + text = "[exec(command='grep [abc log.txt')]" + assert make_valid_python(text) == (text, "") + + def test_partial_string_with_bracket_completes(self): + # Streaming prefix ending mid-string after a literal bracket closes + # with quote + paren + bracket. + result = make_valid_python('[exec(command=\'grep -F "]" lo') + assert result is not None + completed, added = result + assert added == "')]" + assert completed == "[exec(command='grep -F \"]\" lo')]" + + def test_real_mismatched_bracket_still_raises(self): + with pytest.raises(UnexpectedAstError): + make_valid_python("[exec(command=data])") + + def test_multiline_string_argument_recovers_after_escape(self): + # A raw newline inside a string argument is invalid Python, so + # make_valid_python alone returns None; callers pre-escape control + # chars (as the lfm2 parser does) and the escaped value must evaluate + # back to the original. + text = "[exec(command='line1\nline2')]" + assert make_valid_python(text) is None + result = make_valid_python(escape_ctrl_chars_in_strings(text)) + assert result is not None + completed, added = result + assert added == "" + assert _kwarg_constant(_first_call(completed)) == "line1\nline2" + + def test_value_ending_in_backslash_recovers(self): + # A string value ending in a literal backslash: the closing quote follows + # an escaped backslash (an *even* run), so it closes the string. Checking + # only the single preceding char misread it as an escaped quote, left the + # string open, and make_valid_python returned None — dropping calls whose + # last argument ends in a backslash (common in regex like r'\b'). + text = "[write(path='x', content='pattern \\\\')]" + assert make_valid_python(text) == (text, "") + + def test_escaped_quote_odd_backslashes_stays_open(self): + # An escaped quote (an *odd* backslash run) must NOT close the string; + # only the final unescaped quote does. Value round-trips to it's fine. + text = "[say(msg='it\\'s fine')]" + assert make_valid_python(text) == (text, "") + assert _kwarg_constant(_first_call(text)) == "it's fine" + + +class TestEscapeCtrlCharsInStrings: + def test_newline_inside_string_escaped(self): + assert escape_ctrl_chars_in_strings("f(cmd='a\nb')") == "f(cmd='a\\nb')" + + def test_ctrl_chars_outside_strings_untouched(self): + assert escape_ctrl_chars_in_strings("f(a=1,\nb=2)") == "f(a=1,\nb=2)" + + def test_existing_escapes_pass_through(self): + text = "f(cmd='a\\nb')" + assert escape_ctrl_chars_in_strings(text) == text + + def test_escaped_quote_does_not_close_string(self): + assert escape_ctrl_chars_in_strings("f(cmd='a\\'\nb')") == "f(cmd='a\\'\\nb')" + + def test_value_preserved_through_ast(self): + # The escaped text parses and evaluates back to the original value. + raw = "cat > f.py << EOF\nimport csv\nEOF\techo done" + escaped = escape_ctrl_chars_in_strings(f"f(cmd='{raw}')") + assert _kwarg_constant(_bare_call(escaped)) == raw + + def test_nul_byte_inside_string_escaped(self): + # ast.parse raises ValueError (not SyntaxError) on NUL anywhere in + # the source, so an unescaped NUL in a string arg is unrecoverable. + raw = "printf a\x00b" + escaped = escape_ctrl_chars_in_strings(f"f(cmd='{raw}')") + assert "\x00" not in escaped + assert _kwarg_constant(_bare_call(escaped)) == raw + + def test_nul_byte_outside_strings_untouched(self): + text = "f(a=1,\x00b=2)" + assert escape_ctrl_chars_in_strings(text) == text + + +class TestGetParameterValueNegativeNumbers: + # A negative number is parsed by Python as UnaryOp(USub, Constant(n)), not + # a plain Constant. Without explicit handling the entire tool call is + # dropped. Negative longitudes/deltas/offsets are extremely common tool + # arguments (e.g. every Western-hemisphere coordinate). + def test_negative_int(self): + assert _value_of("-1") == -1 + + def test_negative_float(self): + assert _value_of("-3.5") == -3.5 + + def test_explicit_positive_int(self): + assert _value_of("+7") == 7 + + def test_negative_longitude(self): + assert _value_of("-74.0046539") == -74.0046539 + + def test_negative_in_list(self): + assert _value_of("[-1, 2, -3]") == [-1, 2, -3] + + def test_negative_in_dict(self): + assert _value_of('{"min": -5, "max": 5}') == {"min": -5, "max": 5} + + def test_nested_negative(self): + assert _value_of('{"bbox": [-74.0, 40.7, -73.9]}') == { + "bbox": [-74.0, 40.7, -73.9] + } + + def test_non_numeric_unary_still_raises(self): + # ``not x`` / ``~x`` are not literals and must still be rejected. + with pytest.raises(UnexpectedAstError): + _value_of("~5") + with pytest.raises(UnexpectedAstError): + _value_of("not True") + + +class TestHandleSingleToolNegativeNumbers: + def test_negative_arg_end_to_end(self): + call = _first_call("[searchWeather(latitude=40.84, longitude=-74.0046539)]") + tool = handle_single_tool(call) + assert tool.function.name == "searchWeather" + assert json.loads(tool.function.arguments) == { + "latitude": 40.84, + "longitude": -74.0046539, + } + + def test_negative_delta_end_to_end(self): + call = _first_call("[updateInventory(quantity_delta=-20)]") + tool = handle_single_tool(call) + assert json.loads(tool.function.arguments) == {"quantity_delta": -20} + + +class TestGetParameterValueSet: + # JSON has no set type; a set argument is decoded as a list (preserving + # source order) instead of dropping the whole call, mirroring the tuple + # handling. + def test_set_becomes_list(self): + assert _value_of("{'a', 'b'}") == ["a", "b"] + + def test_set_of_numbers(self): + assert _value_of("{1, 2, 3}") == [1, 2, 3] + + def test_set_nested_in_dict(self): + assert _value_of("{'tags': {'x', 'y'}}") == {"tags": ["x", "y"]} + + def test_set_end_to_end(self): + call = _first_call("[label(tags={'urgent', 'bug'})]") + tool = handle_single_tool(call) + assert json.loads(tool.function.arguments) == {"tags": ["urgent", "bug"]} + + +class TestMakeValidPythonSets: + def test_complete_set_in_model_text_accepted(self): + # A set the model wrote and closed itself is a genuine argument. + text = "[label(tags={'urgent', 'bug'})]" + assert make_valid_python(text) == (text, "") + + def test_truncated_dict_completed_to_set_still_rejected(self): + # `{"k` closes to `{"k"}` — a truncated dict, not a set the model + # wrote. The completion added the `}`, so it must keep waiting. + assert make_valid_python('[f(x={"k') is None + + +class TestGetParameterValueFString: + # A placeholder-free f-string is a plain string constant, but ast parses + # it as JoinedStr; it must not drop the call. Real placeholders are not + # literals and must still be rejected. + def test_constant_fstring(self): + assert _value_of("f'hello world'") == "hello world" + + def test_empty_fstring(self): + assert _value_of("f''") == "" + + def test_constant_fstring_in_list(self): + assert _value_of("[f'a', 'b']") == ["a", "b"] + + def test_fstring_with_placeholder_still_raises(self): + with pytest.raises(UnexpectedAstError): + _value_of("f'{x}'") + + def test_constant_fstring_end_to_end(self): + call = _first_call("[send(msg=f'hello')]") + tool = handle_single_tool(call) + assert json.loads(tool.function.arguments) == {"msg": "hello"} + + +class TestGetParameterValueTuple: + # JSON has no tuple type, so a tuple argument is decoded as a list rather + # than dropping the whole call. + def test_tuple_becomes_list(self): + assert _value_of("(800, 600)") == [800, 600] + + def test_nested_tuple(self): + assert _value_of("[(1, 2), (3, 4)]") == [[1, 2], [3, 4]] + + def test_tuple_with_negative(self): + assert _value_of("(-74.0, 40.7)") == [-74.0, 40.7] + + def test_tuple_end_to_end(self): + call = _first_call("[resize(size=(800, 600))]") + tool = handle_single_tool(call) + assert json.loads(tool.function.arguments) == {"size": [800, 600]} + + +class TestEscapeNestedQuotesInStrings: + # Unescaped same-style quotes nested in a string argument (shell + # commands like sed -n '1,9p') are Python juxtaposition errors. When + # exactly one quote can syntactically close the string, close there and + # escape the interior quotes; anything ambiguous is left unchanged. + def test_sed_command_recovers_exact_value(self): + text = "[bash(command='sed -n '360,450p' /testbed/common.py')]" + rewritten, changed = escape_nested_quotes_in_strings(text) + assert changed + assert _kwarg_constant(_first_call(rewritten)) == ( + "sed -n '360,450p' /testbed/common.py" + ) + + def test_double_quote_variant(self): + text = '[bash(command="grep "foo" log.txt")]' + rewritten, changed = escape_nested_quotes_in_strings(text) + assert changed + assert _kwarg_constant(_first_call(rewritten)) == 'grep "foo" log.txt' + + def test_mixed_with_already_escaped_quote(self): + text = "[bash(command='it\\'s 'x' end')]" + rewritten, changed = escape_nested_quotes_in_strings(text) + assert changed + assert _kwarg_constant(_first_call(rewritten)) == "it's 'x' end" + + def test_doubly_nested_python_c_payload(self): + # command='python3 -c "...latex(x, mul_symbol='\,')..."' — two false + # closers both followed by ')', but only the real one yields text + # that parses; ast-validation disambiguates. + text = "[bash(command='python3 -c \"print(latex(x, mul_symbol='\\,'))\"')]" + rewritten, changed = escape_nested_quotes_in_strings(text) + assert changed + assert _kwarg_constant(_first_call(rewritten)) == ( + "python3 -c \"print(latex(x, mul_symbol='\\,'))\"" + ) + + def test_multiline_python_c_with_inner_string(self): + # Raw newlines beyond the phantom close plus a single-quoted inner + # string; recovery must survive both (caller re-escapes ctrl chars). + inner = "python3 -c \"\ntest_str = 'is fine'\nprint(test_str)\n\"" + text = f"[bash(command='{inner}')]" + rewritten, changed = escape_nested_quotes_in_strings(text) + assert changed + recovered = escape_ctrl_chars_in_strings(rewritten) + assert _kwarg_constant(_first_call(recovered)) == inner + + def test_nested_quotes_before_non_string_argument(self): + # A later NON-string argument adds no candidate closer, so the + # nested string is still unambiguous. + text = "[run(cmd='awk '{print}' f', count=2)]" + rewritten, changed = escape_nested_quotes_in_strings(text) + assert changed + call = _first_call(rewritten) + assert _kwarg_constant(call, 0) == "awk '{print}' f" + + @pytest.mark.parametrize( + "text", + [ + "[f(a='x')]", + "[f(a='x', b='y')]", + "[f(a={'k': 'v'})]", + "[f(a='it\\'s fine')]", + "[a(x='p'), b(y='q')]", + ], + ) + def test_valid_text_unchanged(self, text): + rewritten, changed = escape_nested_quotes_in_strings(text) + assert not changed + assert rewritten == text + + @pytest.mark.parametrize( + "text", + [ + "[f(a='echo 'hi', b='x')]", + "[run(cmd='awk '{print}' f', mode='fast')]", + ], + ) + def test_ambiguous_nesting_left_unchanged(self, text): + # A later string argument's closing quote is itself a plausible + # closer, making the nesting formally ambiguous; no rewrite is + # attempted rather than guessing. + rewritten, changed = escape_nested_quotes_in_strings(text) + assert not changed + assert rewritten == text + + +class TestContainsBrokenStringLiteral: + @pytest.mark.parametrize( + "text, broken", + [ + ("[bash(command='sed -n '360,450p' /x')]", True), + ("[f(a='echo 'hi', b='x')]", True), + # Closing quote at the very end of partial text: the follower + # is unknown until the next chunk arrives, so hold. + ("[f(a='x'", True), + ("[f(a='x', b='y')]", False), + # Mid-string (no closing quote yet) is normal streaming. + ('[f(a=\'grep -F "]" log', False), + ("[f(a={'k': 'v'})]", False), + ("[f(a='it\\'s fine')]", False), + ], + ) + def test_detection(self, text, broken): + assert contains_broken_string_literal(text) is broken + + +class TestGetParameterValueNonJsonConstants: + # bytes/Ellipsis/complex are ast.Constant but have no JSON form; they + # must raise UnexpectedAstError (like other unsupported nodes) instead + # of surfacing later as a TypeError inside json.dumps. + @pytest.mark.parametrize("expr", ["b'abc'", "...", "1j"]) + def test_non_json_constant_raises(self, expr): + with pytest.raises(UnexpectedAstError): + _value_of(expr) + + def test_handle_single_tool_raises_ast_error_not_type_error(self): + call = _first_call("[f(x=b'abc')]") + with pytest.raises(UnexpectedAstError): + handle_single_tool(call) + + @pytest.mark.parametrize( + "expr, expected", + [("'s'", "s"), ("1", 1), ("1.5", 1.5), ("True", True), ("None", None)], + ) + def test_json_constants_still_pass(self, expr, expected): + assert _value_of(expr) == expected + + +class TestNormalizeLeadingZeroInts: + # Zero-padded ints (month=07) are a SyntaxError no other recovery path + # handles; the rewrite must strip the padding without touching tokens + # that are already valid Python. + @pytest.mark.parametrize( + "text, expected", + [ + ("[f(month=07)]", "[f(month=7)]"), + ("[f(x=007, y=05)]", "[f(x=7, y=5)]"), + ("[f(x=0_7)]", "[f(x=7)]"), + ("[f(x=-07)]", "[f(x=-7)]"), + ], + ) + def test_leading_zeros_stripped(self, text, expected): + assert normalize_leading_zero_ints(text) == expected + assert ast.parse(expected) + + @pytest.mark.parametrize( + "text", + [ + "[f(x=0)]", + "[f(x=00)]", + "[f(x=0.5)]", + "[f(x=07.5)]", + "[f(x=1.07)]", + "[f(x=1e07)]", + "[f(x=0x1F)]", + "[f(x=0o17)]", + "[f(x=0b101)]", + "[f(s='id 007')]", + '[f(s="v0.07")]', + ], + ) + def test_valid_tokens_and_strings_untouched(self, text): + assert normalize_leading_zero_ints(text) == text + + def test_end_to_end(self): + normalized = normalize_leading_zero_ints("[set_date(month=07, day=05)]") + tool = handle_single_tool(_first_call(normalized)) + assert json.loads(tool.function.arguments) == {"month": 7, "day": 5} + + +class TestRenameReservedKwargs: + # A parameter named after a Python keyword (`from=1`) is a SyntaxError + # that no escape/retry can recover; rename_reserved_kwargs rewrites it to + # a parseable name and restore_reserved_kwarg_names is its exact inverse. + def test_reserved_kwarg_renamed(self): + text, changed = rename_reserved_kwargs("[memory_get(from=1)]") + assert changed + assert text == "[memory_get(from_pyreservedkw_=1)]" + assert ast.parse(text) + + def test_round_trip_restores_original_name(self): + renamed, _ = rename_reserved_kwargs("[memory_get(path='M.md', from=1)]") + tool = handle_single_tool(_first_call(renamed)) + restored = restore_reserved_kwarg_names(json.loads(tool.function.arguments)) + assert restored == {"path": "M.md", "from": 1} + + def test_multiple_reserved_kwargs(self): + text, changed = rename_reserved_kwargs('[search(in="docs/", from=0)]') + assert changed + args = json.loads(handle_single_tool(_first_call(text)).function.arguments) + assert restore_reserved_kwarg_names(args) == {"in": "docs/", "from": 0} + + def test_keyword_inside_string_untouched(self): + text, changed = rename_reserved_kwargs('[f(cmd="import x from y")]') + assert not changed + assert text == '[f(cmd="import x from y")]' + + def test_keyword_with_from_eq_inside_string_untouched(self): + text, changed = rename_reserved_kwargs('[f(cmd="SELECT from=1")]') + assert not changed + + def test_keyword_value_untouched(self): + # `x=True` has a keyword as *value*, not parameter name. + text, changed = rename_reserved_kwargs("[f(x=True, y=None)]") + assert not changed + + def test_double_equals_untouched(self): + text, changed = rename_reserved_kwargs('[f(expr="a", cond=1)]') + assert not changed + # `in ==` comparison-like text is not kwarg position anyway, but the + # `==` guard also protects string-free edge text. + text, changed = rename_reserved_kwargs("[f(x=1)]") + assert not changed + + def test_non_keyword_names_untouched(self): + text, changed = rename_reserved_kwargs("[f(fromage=1, classic=2)]") + assert not changed + + def test_spaces_around_equals(self): + text, changed = rename_reserved_kwargs("[f( from = 1 )]") + assert changed + assert ast.parse(text) + + def test_restore_leaves_normal_names_alone(self): + assert restore_reserved_kwarg_names({"path": "x", "from": 1}) == { + "path": "x", + "from": 1, + } diff --git a/vllm/tool_parsers/lfm2_tool_parser.py b/vllm/tool_parsers/lfm2_tool_parser.py index ee92d060fb..3e4c15d994 100644 --- a/vllm/tool_parsers/lfm2_tool_parser.py +++ b/vllm/tool_parsers/lfm2_tool_parser.py @@ -2,6 +2,7 @@ # SPDX-FileCopyrightText: Copyright contributors to the vLLM project import ast +import json from collections.abc import Sequence import regex as re @@ -24,8 +25,14 @@ from vllm.tool_parsers.abstract_tool_parser import ( from vllm.tool_parsers.utils import ( UnexpectedAstError, compute_tool_delta, + contains_broken_string_literal, + escape_ctrl_chars_in_strings, + escape_nested_quotes_in_strings, handle_single_tool, make_valid_python, + normalize_leading_zero_ints, + rename_reserved_kwargs, + restore_reserved_kwarg_names, ) logger = init_logger(__name__) @@ -94,6 +101,15 @@ class Lfm2ToolParser(ToolParser): def current_tool_index(self, value: int) -> None: self.current_tool_id = value + @staticmethod + def _restore_reserved(tool_call): + """Restore parameter names renamed by ``rename_reserved_kwargs``.""" + arguments = json.loads(tool_call.function.arguments) + restored = restore_reserved_kwarg_names(arguments) + if restored != arguments: + tool_call.function.arguments = json.dumps(restored, ensure_ascii=False) + return tool_call + @staticmethod def _strip_echo(raw_after: str) -> str: """Drop any orphan <|tool_call_end|> (and the preceding text) from @@ -171,17 +187,53 @@ class Lfm2ToolParser(ToolParser): ) try: - module = ast.parse(tool_text) + kw_renamed = False + try: + module = ast.parse(tool_text) + except (SyntaxError, ValueError): + # Progressive rewrites, each a no-op on already-valid text: + # escape raw control chars / NUL bytes inside string literals, + # strip leading zeros from int literals (month=07), close + # unambiguous nested quotes (command='sed -n '1,9p' f.py'), + # and rename reserved-keyword parameters (from=1; restored + # below). The first rewrite whose result parses wins. + escaped = escape_ctrl_chars_in_strings( + normalize_leading_zero_ints(tool_text) + ) + candidates = [escaped] + requoted, requote_changed = escape_nested_quotes_in_strings(escaped) + if requote_changed: + # Requoting can move raw control chars (newlines beyond + # the phantom close) inside the string; escape again. + candidates.append(escape_ctrl_chars_in_strings(requoted)) + renamed, kw_renamed = rename_reserved_kwargs(candidates[-1]) + if kw_renamed: + candidates.append(renamed) + for candidate in candidates: + try: + module = ast.parse(candidate) + break + except (SyntaxError, ValueError): + continue + else: + raise parsed = getattr(module.body[0], "value", None) - if isinstance(parsed, ast.List) and all( - isinstance(e, ast.Call) for e in parsed.elts + # An empty block ([]) must not report tools_called=True with zero + # calls, so require at least one element. + if ( + isinstance(parsed, ast.List) + and parsed.elts + and all(isinstance(e, ast.Call) for e in parsed.elts) ): + tool_calls = [ + handle_single_tool(e) # type: ignore + for e in parsed.elts + ] + if kw_renamed: + tool_calls = [self._restore_reserved(tc) for tc in tool_calls] return ExtractedToolCallInformation( tools_called=True, - tool_calls=[ - handle_single_tool(e) # type: ignore - for e in parsed.elts - ], + tool_calls=tool_calls, content=content, ) else: @@ -261,6 +313,9 @@ class Lfm2ToolParser(ToolParser): # Strip the end token if present (entire call arrived at once). if TOOL_CALL_END in tool_text: tool_text = tool_text.split(TOOL_CALL_END, 1)[0] + # Leading whitespace after the start token would make every completion + # candidate an IndentationError (the non-streaming path strips it too). + tool_text = tool_text.lstrip() def _content_only_or_none() -> DeltaMessage | None: """Return a content-only delta if any content arrived in this @@ -271,6 +326,41 @@ class Lfm2ToolParser(ToolParser): return DeltaMessage(content=combined) if combined else None try: + # A raw control char inside a string argument would make every + # completion candidate a SyntaxError; escape them here rather than + # inside make_valid_python so the shared helper keeps its upstream + # behavior for the other pythonic parsers. Leading zeros in int + # literals (month=07) and parameters named after Python keywords + # (`from=1`) can never parse; rewrite those too. All rewrites are + # deterministic, so successive chunks stay consistent; keyword + # names are restored after decoding. + tool_text = escape_ctrl_chars_in_strings( + normalize_leading_zero_ints(tool_text) + ) + if has_end_in_current: + # Nested-quote recovery needs the final text (which quote + # closes a partial string is not stable across chunks) and + # must never touch text that already parses. + try: + ast.parse(tool_text) + except (SyntaxError, ValueError): + requoted_text, requote_changed = escape_nested_quotes_in_strings( + tool_text + ) + if requote_changed: + tool_text = escape_ctrl_chars_in_strings(requoted_text) + renamed_tool_text, kw_renamed = rename_reserved_kwargs(tool_text) + if kw_renamed: + tool_text = renamed_tool_text + + # A broken string (its first closing quote cannot close it — + # nested-quote text mid-arrival) makes every completion-based + # partial parse an implicit-concatenation misreading whose + # streamed prefix could never be retracted. Withhold deltas + # until the requote recovery above has produced sane text. + if contains_broken_string_literal(tool_text): + return _content_only_or_none() + valid_and_added_text = make_valid_python(tool_text) if valid_and_added_text is None: return _content_only_or_none() @@ -286,6 +376,8 @@ class Lfm2ToolParser(ToolParser): handle_single_tool(e) # type: ignore for e in parsed.elts ] + if kw_renamed: + tool_calls = [self._restore_reserved(tc) for tc in tool_calls] tool_deltas = [] for index, new_call in enumerate(tool_calls): diff --git a/vllm/tool_parsers/utils.py b/vllm/tool_parsers/utils.py index a11d4a9eec..98ea9cc195 100644 --- a/vllm/tool_parsers/utils.py +++ b/vllm/tool_parsers/utils.py @@ -3,6 +3,7 @@ import ast import json +import keyword as _python_keyword import math import warnings from dataclasses import dataclass @@ -451,7 +452,16 @@ def get_parameter_value(val: ast.expr) -> Any: UnexpectedAstError: If the AST node is not a supported literal type. """ if isinstance(val, ast.Constant): - return val.value + if val.value is None or isinstance(val.value, (str, int, float)): + return val.value + # bytes/Ellipsis/complex constants have no JSON representation and + # would otherwise surface as a TypeError deep inside json.dumps; + # reject them explicitly like other unsupported nodes. + logger.warning( + "Non-JSON-representable constant in tool call arguments: %s", + ast.dump(val), + ) + raise UnexpectedAstError("Tool call arguments must be JSON values") elif isinstance(val, ast.Dict): if not all(isinstance(k, ast.Constant) for k in val.keys): logger.warning( @@ -465,8 +475,44 @@ def get_parameter_value(val: ast.expr) -> Any: } elif isinstance(val, ast.List): return [get_parameter_value(v) for v in val.elts] + elif isinstance(val, ast.Tuple): + # JSON has no tuple type; a tuple argument (e.g. ``size=(800, 600)``) + # is treated as a list so it round-trips through ``json.dumps``. + # Without this the whole call is dropped. + return [get_parameter_value(v) for v in val.elts] + elif isinstance(val, ast.Set): + # JSON has no set type either; a set argument (e.g. + # ``tags={'a', 'b'}``) is treated as a list, preserving source order. + return [get_parameter_value(v) for v in val.elts] + elif isinstance(val, ast.JoinedStr) and all( + isinstance(part, ast.Constant) for part in val.values + ): + # An f-string without placeholders (``f'hello'``) is a plain string + # constant, but Python parses it as JoinedStr rather than Constant; + # without this branch the whole call is dropped. F-strings with real + # placeholders still fall through to the raise below. + return "".join( + str(part.value) # type: ignore + for part in val.values + ) elif isinstance(val, ast.Name) and val.id in _JSON_NAME_LITERALS: return _JSON_NAME_LITERALS[val.id] + elif isinstance(val, ast.UnaryOp) and isinstance(val.op, (ast.USub, ast.UAdd)): + # A negative (or explicitly positive) number is parsed by Python as a + # unary operation over a numeric constant, e.g. ``-1`` becomes + # ``UnaryOp(USub, Constant(1))`` rather than a plain ``Constant(-1)``. + # These are extremely common tool arguments (negative longitudes, + # offsets, deltas); without this branch the whole call is dropped. + # Restrict to numeric operands so ``not``/``~`` and other expressions + # still raise below. + operand = get_parameter_value(val.operand) + if isinstance(operand, (int, float)) and not isinstance(operand, bool): + return -operand if isinstance(val.op, ast.USub) else operand + logger.warning( + "Unsupported unary operand in tool call arguments: %s", + ast.dump(val), + ) + raise UnexpectedAstError("Tool call arguments must be literals") else: logger.warning( "Unsupported AST node type in tool call arguments: %s", @@ -524,6 +570,343 @@ def handle_single_tool(call: ast.Call) -> ToolCall: ) +def escape_ctrl_chars_in_strings(text: str) -> str: + """Escape literal control chars inside string literals of pythonic text. + + Models emitting pythonic tool calls frequently place raw newlines inside a + string argument (e.g. ``exec(command='line1\\nline2')`` written with a real + line break). That is invalid Python — ``ast.parse`` fails with "unterminated + string literal" — so the call would be dropped even though the intent is + unambiguous. A NUL byte is worse: ``ast.parse`` rejects it anywhere in the + source with ``ValueError``. Escaping ``\\n``/``\\r``/``\\t``/``\\x00`` only + *inside* string literals makes the text parseable while preserving the + argument value exactly (the escape sequences evaluate back to the original + control chars). + + Text outside string literals is returned unchanged. + """ + out: list[str] = [] + quote: str | None = None + index, length = 0, len(text) + while index < length: + char = text[index] + if quote is None: + if char in {"'", '"'}: + quote = char + out.append(char) + elif char == "\\" and index + 1 < length: + out.append(char) + out.append(text[index + 1]) + index += 2 + continue + elif char == quote: + quote = None + out.append(char) + elif char == "\n": + out.append("\\n") + elif char == "\r": + out.append("\\r") + elif char == "\t": + out.append("\\t") + elif char == "\x00": + out.append("\\x00") + else: + out.append(char) + index += 1 + return "".join(out) + + +_RESERVED_KW_SUFFIX = "_pyreservedkw_" + + +def rename_reserved_kwargs(text: str) -> tuple[str, bool]: + """Rename Python-keyword parameter names so pythonic tool text parses. + + Tools legitimately name parameters ``from``, ``in``, ``class`` — but + ``memory_get(from=1)`` is a Python ``SyntaxError``, so the whole call + would be dropped. Rename ``from=`` to ``from_pyreservedkw_=`` (outside + string literals only, and only in keyword-argument position: preceded by + ``(`` or ``,`` and followed by a single ``=``), parse, then restore the + original name with :func:`restore_reserved_kwarg_names`. + + Returns (rewritten_text, changed). Keyword *values* (``x=True``) and + keywords inside string arguments are never touched. + """ + out: list[str] = [] + quote: str | None = None + changed = False + last_sig = "" + index, length = 0, len(text) + while index < length: + char = text[index] + if quote is not None: + out.append(char) + if char == "\\" and index + 1 < length: + out.append(text[index + 1]) + index += 2 + continue + if char == quote: + quote = None + index += 1 + continue + if char in {"'", '"'}: + quote = char + out.append(char) + last_sig = char + index += 1 + continue + if char.isalpha() or char == "_": + end = index + while end < length and (text[end].isalnum() or text[end] == "_"): + end += 1 + name = text[index:end] + look = end + while look < length and text[look].isspace(): + look += 1 + if ( + _python_keyword.iskeyword(name) + and look < length + and text[look] == "=" + and (look + 1 >= length or text[look + 1] != "=") + and last_sig in {"(", ","} + ): + out.append(name + _RESERVED_KW_SUFFIX) + changed = True + else: + out.append(name) + last_sig = name[-1] + index = end + continue + out.append(char) + if not char.isspace(): + last_sig = char + index += 1 + return "".join(out), changed + + +def restore_reserved_kwarg_names(arguments: dict) -> dict: + """Undo :func:`rename_reserved_kwargs` on a decoded arguments dict. + + Only keys that carry the rename suffix *and* whose stem is a Python + keyword are restored, making this an exact inverse of the rename. + """ + restored = {} + for key, value in arguments.items(): + if ( + isinstance(key, str) + and key.endswith(_RESERVED_KW_SUFFIX) + and _python_keyword.iskeyword(key[: -len(_RESERVED_KW_SUFFIX)]) + ): + restored[key[: -len(_RESERVED_KW_SUFFIX)]] = value + else: + restored[key] = value + return restored + + +def normalize_leading_zero_ints(text: str) -> str: + """Strip leading zeros from decimal integer literals so the text parses. + + Models emit zero-padded integers (``month=07``), which Python rejects + ("leading zeros in decimal integer literals are not permitted"), so the + whole call would be dropped. Rewrite ``07`` to ``7`` outside string + literals only. Tokens that are already valid Python are left alone: + all-zero literals (``00``), floats and fractional parts (``07.5``, + ``1.07``), exponents (``1e07``, consumed as a name run), and ``0x``/ + ``0o``/``0b`` prefixes (the digit run stops at the prefix letter). + """ + out: list[str] = [] + quote: str | None = None + index, length = 0, len(text) + while index < length: + char = text[index] + if quote is not None: + out.append(char) + if char == "\\" and index + 1 < length: + out.append(text[index + 1]) + index += 2 + continue + if char == quote: + quote = None + index += 1 + continue + if char in {"'", '"'}: + quote = char + out.append(char) + index += 1 + continue + if char.isalpha() or char == "_": + end = index + while end < length and (text[end].isalnum() or text[end] == "_"): + end += 1 + out.append(text[index:end]) + index = end + continue + if char.isdigit(): + end = index + while end < length and (text[end].isdigit() or text[end] == "_"): + end += 1 + token = text[index:end] + digits = token.replace("_", "") + follower = text[end] if end < length else "" + preceded_by_dot = index > 0 and text[index - 1] == "." + if ( + digits[0] == "0" + and digits.strip("0") + and not preceded_by_dot + and follower not in {".", "e", "E", "j", "J"} + ): + out.append(str(int(digits))) + else: + out.append(token) + index = end + continue + out.append(char) + index += 1 + return "".join(out) + + +_QUOTE_FOLLOWERS = {",", ")", "]", "}", ":"} + + +def escape_nested_quotes_in_strings(text: str) -> tuple[str, bool]: + """Close a broken string literal at the only closing quote that works. + + Models emitting shell commands frequently nest unescaped same-style + quotes inside a string argument — ``command='sed -n '360,450p' f.py'``, + or a quoted ``python3 -c`` payload that itself contains quoted strings — + which Python reads as juxtaposed garbage, so the call is dropped even + though the intent is unambiguous. A string is treated as broken when + its first unescaped quote cannot syntactically close it (what follows + is none of ``,``, ``)``, ``]``, ``}``, ``:``). For a broken string, + every syntactically plausible closing quote is tried: interior quotes + escaped, the rest of the text kept verbatim, and the result (with + control chars escaped) validated with ``ast.parse``. Exactly one + candidate parsing means recovery — the decoded value is exactly the + text the model wrote. Zero or several parsing candidates means the + nesting is genuinely ambiguous and the text is returned unchanged + rather than guessed at. + + Returns (rewritten_text, changed); run the result through + escape_ctrl_chars_in_strings before parsing — quotes chosen here can + move raw control chars inside the string. + """ + + def unescaped_quotes(start: int, quote: str) -> list[int]: + positions = [] + j = start + while j < len(text): + if text[j] == "\\": + j += 2 + continue + if text[j] == quote: + positions.append(j) + j += 1 + return positions + + def is_closer(pos: int) -> bool: + k = pos + 1 + while k < len(text) and text[k].isspace(): + k += 1 + return k < len(text) and text[k] in _QUOTE_FOLLOWERS + + prefix: list[str] = [] + index = 0 + while index < len(text): + char = text[index] + if char not in {"'", '"'}: + prefix.append(char) + index += 1 + continue + quotes = unescaped_quotes(index + 1, char) + if not quotes: + return text, False + if is_closer(quotes[0]): + # The normal reading closes this string; move past it. + prefix.append(text[index : quotes[0] + 1]) + index = quotes[0] + 1 + continue + winners = [] + for close in (j for j in quotes if is_closer(j)): + interior: list[str] = [] + for j in range(index + 1, close): + if text[j] == char and not _is_escaped(text, j): + interior.append("\\") + interior.append(text[j]) + candidate = "".join( + ["".join(prefix), char, "".join(interior), char, text[close + 1 :]] + ) + with warnings.catch_warnings(): + warnings.simplefilter("ignore", SyntaxWarning) + try: + ast.parse(escape_ctrl_chars_in_strings(candidate)) + except (SyntaxError, ValueError): + continue + winners.append(candidate) + if len(winners) == 1: + return winners[0], True + return text, False + return text, False + + +def contains_broken_string_literal(text: str) -> bool: + """Whether some string literal's first closing quote cannot close it. + + Streaming guard companion to escape_nested_quotes_in_strings: + completion-based partial parses of nested-quote text read the string as + implicit concatenation and stream argument prefixes that can never be + retracted. Callers should withhold tool deltas while this returns True + and let the requote recovery run on the final text instead. A string + whose closing quote has not arrived yet is NOT broken (normal + streaming); a closing quote at the very end of the text counts as + broken only because its follower is still unknown. + """ + index = 0 + while index < len(text): + char = text[index] + if char == "\\": + index += 2 + continue + if char not in {"'", '"'}: + index += 1 + continue + j = index + 1 + close = -1 + while j < len(text): + if text[j] == "\\": + j += 2 + continue + if text[j] == char: + close = j + break + j += 1 + if close == -1: + return False + k = close + 1 + while k < len(text) and text[k].isspace(): + k += 1 + if k >= len(text) or text[k] not in _QUOTE_FOLLOWERS: + return True + index = close + 1 + return False + + +def _is_escaped(text: str, index: int) -> bool: + """Whether the character at ``index`` is backslash-escaped. + + A character is escaped iff it is preceded by an *odd* number of consecutive + backslashes. Checking only the single preceding character is wrong for even + runs: in ``'ab\\'`` the closing quote follows an escaped backslash (``\\\\``) + and is therefore NOT escaped — it closes the string. Common in regex/code + arguments such as ``r'\\\\b'``. + """ + backslashes = 0 + j = index - 1 + while j >= 0 and text[j] == "\\": + backslashes += 1 + j -= 1 + return backslashes % 2 == 1 + + def make_valid_python(text: str) -> tuple[str, str] | None: """Attempt to close all open brackets/quotes to make partial Python valid. @@ -541,6 +924,14 @@ def make_valid_python(text: str) -> tuple[str, str] | None: """ bracket_stack: list[str] = [] for index, char in enumerate(text): + # Inside a string literal only an unescaped matching quote is + # significant; brackets are literal text. Without this guard a bracket + # in a string argument (e.g. `cmd='grep -F "]"'`) corrupts the bracket + # stack and the whole tool call is rejected as mismatched. + if bracket_stack and bracket_stack[-1] in {"'", '"'}: + if char == bracket_stack[-1] and not _is_escaped(text, index): + bracket_stack.pop() + continue if char in {"[", "(", "{"}: bracket_stack.append(char) elif char == "]": @@ -553,15 +944,7 @@ def make_valid_python(text: str) -> tuple[str, str] | None: if not bracket_stack or bracket_stack.pop() != "{": raise UnexpectedAstError("Mismatched curly braces") elif char in {"'", '"'}: - if bracket_stack and bracket_stack[-1] == char: - if index > 0 and text[index - 1] == "\\": - pass - else: - bracket_stack.pop() - elif bracket_stack and bracket_stack[-1] in {"'", '"'}: - pass - else: - bracket_stack.append(char) + bracket_stack.append(char) text = text.rstrip() if text.endswith("=") or text.endswith(":"): @@ -600,19 +983,23 @@ def make_valid_python(text: str) -> tuple[str, str] | None: # 1. Mid-key inside a dict (`..., "k`) closes to `..., "k"}` — a # syntactically invalid mixed dict/set. # 2. A bare string inside a dict (`{"k`) closes to `{"k"}` — valid - # Python but a *set* literal, which downstream tool-call AST - # handling rejects. - # Validate the candidate parses, has a body, and contains no Set - # nodes (pythonic tool calls always use dicts for `{...}`). + # Python but a *set* literal that is really a truncated dict. + # Validate the candidate parses and has a body, and treat Set nodes as + # incomplete — but only when this completion added a `}` itself; a set + # already closed in the model text is a genuine set argument, not an + # artifact. Callers whose models emit raw control chars inside string + # arguments must escape them (see escape_ctrl_chars_in_strings) before + # calling. try: module = ast.parse(candidate) except SyntaxError: return None if not module.body: return None - for node in ast.walk(module): - if isinstance(node, ast.Set): - return None + if "}" in added_text: + for node in ast.walk(module): + if isinstance(node, ast.Set): + return None return candidate, added_text