[Bugfix] Fix lfm2 tool parser dropping calls with brackets or newline… (#48171)
Signed-off-by: Zetian Li <804561096@qq.com> Co-authored-by: Claude (Anthropic) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude
parent
61c1dd0966
commit
7303c66f68
@@ -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 = [
|
||||
|
||||
@@ -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 <b>fine</b>'\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,
|
||||
}
|
||||
|
||||
@@ -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):
|
||||
|
||||
+404
-17
@@ -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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user