From 27aa48bca1b59c26e8f6c7e49f50a6861c90a208 Mon Sep 17 00:00:00 2001 From: Zetian Li - ikun <68205293+fatday@users.noreply.github.com> Date: Sun, 23 Aug 2026 03:09:43 -0700 Subject: [PATCH] =?UTF-8?q?[Fix]=20lfm2=20detector:=20recover=20tool=20cal?= =?UTF-8?q?ls=20dropped=20by=20common=20model-outpu=E2=80=A6=20(#34237)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Claude Co-authored-by: Xinyuan Tong --- .../sglang/srt/function_call/lfm2_detector.py | 500 +++++++++++++++++- .../srt/function_call/pythonic_detector.py | 32 +- .../test_function_call_parser.py | 260 +++++++++ 3 files changed, 774 insertions(+), 18 deletions(-) diff --git a/python/sglang/srt/function_call/lfm2_detector.py b/python/sglang/srt/function_call/lfm2_detector.py index e649cf866..63ed6deff 100644 --- a/python/sglang/srt/function_call/lfm2_detector.py +++ b/python/sglang/srt/function_call/lfm2_detector.py @@ -19,6 +19,7 @@ Also supports JSON format: import ast import json +import keyword as _python_keyword import logging import re from typing import Any, Dict, List, Optional, Tuple @@ -46,6 +47,387 @@ _PYTHONIC_NAME_LITERALS = { "null": None, } +_QUOTE_FOLLOWERS = {",", ")", "]", "}", ":"} +_RESERVED_KW_SUFFIX = "_pyreservedkw_" + + +def _rename_reserved_kwargs(text: str) -> Tuple[str, bool]: + """Rename Python-keyword parameter names so the text parses. + + Tools legitimately name parameters ``from``/``in``/``class``, but + ``memory_get(from=1)`` is a Python ``SyntaxError``. Rename ``from=`` to + ``from_pyreservedkw_=`` (outside string literals, keyword-argument + position only), parse, then restore via + :func:`_restore_reserved_kwarg_names`. Returns (rewritten_text, changed). + """ + out: List[str] = [] + quote: Optional[str] = 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: + """Exact inverse of :func:`_rename_reserved_kwargs` on a decoded dict.""" + 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 _is_escaped(text: str, index: int) -> bool: + """Whether the char at ``index`` follows an odd run of backslashes.""" + backslashes = 0 + j = index - 1 + while j >= 0 and text[j] == "\\": + backslashes += 1 + j -= 1 + return backslashes % 2 == 1 + + +def _escape_nested_quotes_in_strings(text: str) -> Tuple[str, bool]: + """Close a broken string literal at the only closing quote that works. + + Shell commands nest unescaped same-style quotes inside a string argument + (``command='sed -n '360,450p' f.py'`` or a quoted ``python3 -c`` payload), + which Python reads as juxtaposed garbage, so the call is dropped even + though the intent is unambiguous. A string is 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 kept verbatim — and + the result validated with ``ast.parse``. Exactly one parsing candidate + means recovery; zero or several means genuine ambiguity and the text is + returned unchanged rather than guessed at. + + Returns (rewritten_text, changed). + """ + + 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 + + # A late-closing reading can swallow a whole sibling call into the + # string value (``f(a='x 'y'), g(...)`` parsing as one call with + # ``g(...)`` inside ``a``) — worse than dropping it, since the tool then + # runs with corrupted arguments. Counting brackets is immune to the + # broken quote, so the block's call count is the invariant. + expected_calls = len(_split_top_level_calls(text, respect_strings=False)) + 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]): + 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 :]] + ) + try: + module = safe_ast_parse(_escape_ctrl_chars_in_strings(candidate)) + except (SyntaxError, ValueError): + continue + if expected_calls > 1 and _top_level_call_count(module) < expected_calls: + continue + winners.append(candidate) + if len(winners) == 1: + return winners[0], True + return text, False + return text, False + + +def _escape_ctrl_chars_in_strings(text: str) -> str: + """Escape raw control chars inside string literals of pythonic text. + + Models frequently place raw newlines inside a string argument (multi-line + shell commands), which is invalid Python, and a NUL byte anywhere makes + ``ast.parse`` raise ``ValueError``. Escaping ``\\n``/``\\r``/``\\t``/ + ``\\x00`` only inside string literals makes the text parseable while the + escape sequences evaluate back to the exact original value. + """ + out: List[str] = [] + quote: Optional[str] = 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) + + +def _normalize_leading_zero_ints(text: str) -> str: + """Strip leading zeros from decimal int literals (``month=07`` -> ``7``). + + Zero-padded integers are a ``SyntaxError`` no other rewrite recovers. + Only rewrites outside string literals; tokens that are already valid + Python (``0x``/``0o``/``0b``, floats, exponents, all-zero literals, + fractional parts like ``1.07``) are left untouched. + """ + out: List[str] = [] + quote: Optional[str] = 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) + + +def _recovery_candidates(content: str) -> List[Tuple[str, bool]]: + """Progressive rewrites for content that failed to parse. + + Each rewrite is a no-op on already-valid text; the first candidate whose + result parses wins. Nested-quote recovery is re-escaped, since requoting + can move raw newlines inside the string. The flag marks candidates that + went through reserved-keyword renaming; only their decoded arguments get + the original parameter names restored. + """ + escaped = _escape_ctrl_chars_in_strings(_normalize_leading_zero_ints(content)) + candidates: List[Tuple[str, bool]] = [(escaped, False)] + requoted, requote_changed = _escape_nested_quotes_in_strings(escaped) + if requote_changed: + candidates.append((_escape_ctrl_chars_in_strings(requoted), False)) + for text, _ in list(candidates): + renamed, kw_renamed = _rename_reserved_kwargs(text) + if kw_renamed: + candidates.append((renamed, True)) + # A call can stack both quirks; renaming first lets requote + # validate candidates the keyword SyntaxError otherwise blocks. + requoted_after, requote_after_changed = _escape_nested_quotes_in_strings( + renamed + ) + if requote_after_changed: + candidates.append((_escape_ctrl_chars_in_strings(requoted_after), True)) + return candidates + + +def _split_top_level_calls(text: str, *, respect_strings: bool = True) -> List[str]: + """Split a pythonic call block into top-level call segments. + + ``[a(x=1), b(y=2)]`` becomes ``["a(x=1)", "b(y=2)"]``: one enclosing + bracket pair is stripped and only commas at bracket depth 0 separate + segments. With ``respect_strings=False`` only brackets are counted, + which a broken quote cannot desynchronize; string arguments always sit + at depth >= 1, so their commas still never split. + """ + text = text.strip() + if text.startswith("[") and text.endswith("]"): + text = text[1:-1] + segments: List[str] = [] + start = 0 + depth = 0 + quote: Optional[str] = None + index = 0 + while index < len(text): + char = text[index] + if respect_strings and quote is not None: + if char == "\\": + index += 2 + continue + if char == quote: + quote = None + index += 1 + continue + if respect_strings and char in {"'", '"'}: + quote = char + elif char in "([{": + depth += 1 + elif char in ")]}": + depth -= 1 + elif char == "," and depth == 0: + segments.append(text[start:index]) + start = index + 1 + index += 1 + segments.append(text[start:]) + return [segment.strip() for segment in segments if segment.strip()] + + +def _top_level_call_count(module: ast.Module) -> int: + """Number of calls in a parsed ``[a(...), b(...)]`` block.""" + if not module.body: + return 0 + value = getattr(module.body[0], "value", None) + if isinstance(value, ast.List): + return sum(1 for element in value.elts if isinstance(element, ast.Call)) + return 1 if isinstance(value, ast.Call) else 0 + + +def _salvage_calls_from_unparsable_block(text: str) -> List[Tuple[ast.Call, bool]]: + """Recover individual calls from a block ``ast.parse`` cannot handle. + + When the block as a whole is a SyntaxError no rewrite recovers, there + is no call list at all and one bad call drops every parseable sibling, + leaving an agent loop with no tool result. Split with both scanning + strategies and parse each segment on its own through the rewrite + ladder. A wrongly split segment simply fails to parse and is dropped, + so this can only under-recover, never attribute arguments to the wrong + call. Each call carries the reserved-keyword flag of the candidate it + parsed from. + """ + best: List[Tuple[ast.Call, bool]] = [] + for respect_strings in (True, False): + segments = _split_top_level_calls(text, respect_strings=respect_strings) + if len(segments) < 2: + continue + calls: List[Tuple[ast.Call, bool]] = [] + for segment in segments: + for candidate, kw_renamed in [(segment, False)] + _recovery_candidates( + segment + ): + try: + module = safe_ast_parse(candidate) + except (SyntaxError, ValueError): + continue + parsed = getattr(module.body[0], "value", None) if module.body else None + if isinstance(parsed, ast.Call): + calls.append((parsed, kw_renamed)) + break + if len(calls) > len(best): + best = calls + return best + class Lfm2Detector(BaseFormatDetector): """ @@ -85,7 +467,14 @@ class Lfm2Detector(BaseFormatDetector): Reuses pattern from PythonicDetector. """ 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 have no JSON form; raising ValueError + # here lets the per-call handler skip this call instead of a + # TypeError inside json.dumps dropping every sibling call. + raise ValueError( + f"Constant has no JSON representation: {type(val.value).__name__}" + ) elif isinstance(val, ast.Dict): return { self._get_parameter_value(k): self._get_parameter_value(v) @@ -96,6 +485,16 @@ class Lfm2Detector(BaseFormatDetector): return [self._get_parameter_value(v) for v in val.elts] elif isinstance(val, ast.Tuple): return tuple(self._get_parameter_value(v) for v in val.elts) + elif isinstance(val, ast.Set): + # JSON has no set type; decode as a list in source order. + return [self._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 + ): + # A placeholder-free f-string (f'hello') is a plain string + # constant, but Python parses it as JoinedStr; f-strings with + # real placeholders still fall through to the raise below. + return "".join(str(part.value) for part in val.values) elif isinstance(val, ast.Name): # Python True/False/None are ast.Constant on modern Python, but # accept their legacy node shape plus LFM2's JSON-literal spellings. @@ -103,12 +502,12 @@ class Lfm2Detector(BaseFormatDetector): return _PYTHONIC_NAME_LITERALS[val.id] except KeyError: raise ValueError(f"Unsupported name reference: {val.id}") from None - elif isinstance(val, ast.UnaryOp) and isinstance(val.op, ast.USub): - # Handle negative numbers like -5 + elif isinstance(val, ast.UnaryOp) and isinstance(val.op, (ast.USub, ast.UAdd)): + # Handle signed numbers like -5 and +5 inner = self._get_parameter_value(val.operand) - if isinstance(inner, (int, float)): - return -inner - raise ValueError(f"Cannot negate non-numeric value: {inner}") + if isinstance(inner, (int, float)) and not isinstance(inner, bool): + return -inner if isinstance(val.op, ast.USub) else inner + raise ValueError(f"Cannot apply sign to non-numeric value: {inner}") else: raise ValueError( f"Tool call arguments must be literals, got: {type(val).__name__}" @@ -128,7 +527,12 @@ class Lfm2Detector(BaseFormatDetector): return ".".join(reversed(parts)) def _parse_pythonic_call( - self, call: ast.Call, call_index: int, tool_indices: Dict[str, int] + self, + call: ast.Call, + call_index: int, + tool_indices: Dict[str, int], + *, + restore_reserved_kwarg: bool = False, ) -> Optional[ToolCallItem]: """ Parse a single AST Call node into a ToolCallItem. @@ -137,6 +541,8 @@ class Lfm2Detector(BaseFormatDetector): call: AST Call node representing a function call call_index: Index of this call in the list of calls tool_indices: Mapping of tool names to their indices + restore_reserved_kwarg: Whether the parsed text went through + reserved-keyword renaming Returns: ToolCallItem if successful, None if the call should be skipped @@ -156,12 +562,31 @@ class Lfm2Detector(BaseFormatDetector): if not envs.SGLANG_FORWARD_UNKNOWN_TOOLS.get(): return None # Skip unknown tools (default legacy behavior) + if call.args: + # Only keyword arguments carry parameter names; positional + # values used to be dropped silently, emitting a + # successful-looking call with arguments missing. Reject + # instead (parseable sibling calls are kept). + logger.warning(f"Tool call {function_name} has positional arguments") + return None + # Parse arguments arguments = {} for keyword in call.keywords: if keyword.arg is None: - # **kwargs unpacking - skip for now - logger.warning("Tool call with **kwargs unpacking is not supported") + # **-unpacking is ast.keyword(arg=None); the kwargs used to + # be skipped silently, emitting the call with arguments + # missing. Merge dict literals with Python's + # later-binding-wins semantics and reject anything else. + try: + unpacked = self._get_parameter_value(keyword.value) + except ValueError as e: + logger.warning(f"Failed to parse **-unpacked arguments: {e}") + return None + if not isinstance(unpacked, dict): + logger.warning("**-unpacked arguments must be a dict literal") + return None + arguments.update(unpacked) continue try: arguments[keyword.arg] = self._get_parameter_value(keyword.value) @@ -169,10 +594,24 @@ class Lfm2Detector(BaseFormatDetector): logger.warning(f"Failed to parse argument {keyword.arg}: {e}") return None + if restore_reserved_kwarg: + # Unconditional restore would rewrite a parameter literally + # named e.g. ``in_pyreservedkw_`` to ``in`` on the normal path. + arguments = _restore_reserved_kwarg_names(arguments) + + try: + # allow_nan=False: a non-finite float (e.g. the literal 1e999 + # overflowing to inf) would otherwise serialize as Infinity, + # which is not valid JSON for downstream clients. + parameters = json.dumps(arguments, ensure_ascii=False, allow_nan=False) + except (ValueError, TypeError) as e: + logger.warning(f"Arguments of {function_name} are not valid JSON: {e}") + return None + return ToolCallItem( tool_index=call_index, # Use the call index in the response, not tool position name=function_name, - parameters=json.dumps(arguments, ensure_ascii=False), + parameters=parameters, ) def _parse_pythonic_content( @@ -192,7 +631,37 @@ class Lfm2Detector(BaseFormatDetector): tool_indices = self._get_tool_indices(tools) try: - module = safe_ast_parse(content) + kw_renamed = False + try: + module = safe_ast_parse(content) + except (SyntaxError, ValueError): + # Recoverable model quirks are rewritten value-preservingly; + # the first rewrite that parses wins. Unrecoverable text + # re-raises the original error. + for candidate, kw_renamed in _recovery_candidates(content): + try: + module = safe_ast_parse(candidate) + break + except (SyntaxError, ValueError): + continue + else: + # The block as a whole is unrecoverable. Split it into + # top-level segments and parse each on its own so one bad + # call does not drop every parseable sibling. + salvaged = _salvage_calls_from_unparsable_block(content) + if not salvaged: + raise + calls = [] + for call_index, (call, segment_kw_renamed) in enumerate(salvaged): + item = self._parse_pythonic_call( + call, + call_index, + tool_indices, + restore_reserved_kwarg=segment_kw_renamed, + ) + if item is not None: + calls.append(item) + return calls, "" parsed = getattr(module.body[0], "value", None) if module.body else None if parsed is None: @@ -215,13 +684,18 @@ class Lfm2Detector(BaseFormatDetector): calls = [] for call_index, call in enumerate(call_nodes): - item = self._parse_pythonic_call(call, call_index, tool_indices) + item = self._parse_pythonic_call( + call, + call_index, + tool_indices, + restore_reserved_kwarg=kw_renamed, + ) if item is not None: calls.append(item) return calls, "" - except SyntaxError as e: + except (SyntaxError, ValueError) as e: return [], f"Python syntax error: {e}" except Exception as e: logger.exception("Unexpected error in pythonic tool call parsing") diff --git a/python/sglang/srt/function_call/pythonic_detector.py b/python/sglang/srt/function_call/pythonic_detector.py index 83f5c3dd2..c17c44343 100644 --- a/python/sglang/srt/function_call/pythonic_detector.py +++ b/python/sglang/srt/function_call/pythonic_detector.py @@ -95,14 +95,29 @@ class PythonicDetector(BaseFormatDetector): if not envs.SGLANG_FORWARD_UNKNOWN_TOOLS.get(): continue # Skip unknown tools (default legacy behavior) - arguments = {} - for keyword in call.keywords: - arguments[keyword.arg] = self._get_parameter_value(keyword.value) + # Convert each call on its own: an unconvertible argument used + # to escape to the outer handler and drop every parseable + # sibling call in the block. + try: + arguments = {} + for keyword in call.keywords: + arguments[keyword.arg] = self._get_parameter_value( + keyword.value + ) + # allow_nan=False: a non-finite float (e.g. the literal + # 1e999 overflowing to inf) would otherwise serialize as + # Infinity, which is not valid JSON for downstream clients. + parameters = json.dumps( + arguments, ensure_ascii=False, allow_nan=False + ) + except (ValueError, TypeError) as e: + logger.warning(f"Skipping tool call {function_name}: {e}") + continue calls.append( ToolCallItem( tool_index=call_index, # Use the call index in the response, not tool position name=function_name, - parameters=json.dumps(arguments, ensure_ascii=False), + parameters=parameters, ) ) @@ -207,7 +222,14 @@ class PythonicDetector(BaseFormatDetector): def _get_parameter_value(self, val): 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 have no JSON form; raising here lets the + # per-call handler skip this call instead of a TypeError inside + # json.dumps dropping every sibling call in the block. + raise ValueError( + f"Constant has no JSON representation: {type(val.value).__name__}" + ) elif isinstance(val, ast.Dict): return { k.value: self._get_parameter_value(v) diff --git a/test/registered/unit/function_call/test_function_call_parser.py b/test/registered/unit/function_call/test_function_call_parser.py index f6054039a..6202f7aef 100644 --- a/test/registered/unit/function_call/test_function_call_parser.py +++ b/test/registered/unit/function_call/test_function_call_parser.py @@ -769,6 +769,27 @@ class TestPythonicDetector(unittest.TestCase): self.assertEqual(params["location"], "Mars") self.assertEqual(params["unit"], "celsius") + def test_non_finite_argument_never_emits_invalid_json(self): + """A 1e999 literal overflows to inf and json.dumps rendered it as + Infinity — parameters no JSON parser accepts, delivered as a + successful call. The call is skipped instead.""" + text = "[get_weather(location='Tokyo', unit=1e999)]" + result = self.detector.detect_and_parse(text, self.tools) + + for call in result.calls: + json.loads(call.parameters) + self.assertEqual(result.calls, []) + + def test_unconvertible_argument_skips_only_that_call(self): + """A bytes argument is an ast.Constant, so it passed value + extraction and only failed later inside json.dumps, escaping to the + block-level handler and dropping every parseable sibling call.""" + text = "[get_weather(location='Tokyo'), search(query=b'raw')]" + result = self.detector.detect_and_parse(text, self.tools) + + self.assertEqual([c.name for c in result.calls], ["get_weather"]) + self.assertEqual(json.loads(result.calls[0].parameters), {"location": "Tokyo"}) + class TestMistralDetector(unittest.TestCase): def setUp(self): @@ -4052,6 +4073,245 @@ class TestLfm2Detector(unittest.TestCase): self.assertEqual(result.calls[0].name, "get_weather") self.assertEqual(result.calls[1].name, "search") + # ==================== recovery tests (dropped-call regressions) ==================== + + def test_multiline_string_argument_recovered(self): + """A raw newline inside a string argument (multi-line shell command) + is invalid Python, so ast.parse failed and the whole call was + dropped. The value must round-trip with the newline intact.""" + text = ( + "<|tool_call_start|>[search(query='line one\nline two')]<|tool_call_end|>" + ) + result = self.detector.detect_and_parse(text, self.tools) + + self.assertEqual(len(result.calls), 1) + params = json.loads(result.calls[0].parameters) + self.assertEqual(params["query"], "line one\nline two") + + def test_nul_byte_in_string_argument_recovered(self): + """A NUL byte anywhere makes ast.parse raise ValueError (not + SyntaxError), so the call was dropped with no recovery path.""" + text = "<|tool_call_start|>[search(query='printf a\x00b')]<|tool_call_end|>" + result = self.detector.detect_and_parse(text, self.tools) + + self.assertEqual(len(result.calls), 1) + params = json.loads(result.calls[0].parameters) + self.assertEqual(params["query"], "printf a\x00b") + + def test_nested_quotes_recovered(self): + """Unescaped same-style quotes nested in a shell command + (sed -n '360,450p') read as string/number juxtaposition, a + SyntaxError, so the call was dropped even though only one closing + quote yields parseable text.""" + text = ( + "<|tool_call_start|>[search(query='sed -n '360,450p' f.py')]" + "<|tool_call_end|>" + ) + result = self.detector.detect_and_parse(text, self.tools) + + self.assertEqual(len(result.calls), 1) + params = json.loads(result.calls[0].parameters) + self.assertEqual(params["query"], "sed -n '360,450p' f.py") + + def test_ambiguous_nested_quotes_not_guessed(self): + """When a later string argument's closing quote is also a plausible + closer, the nesting is genuinely ambiguous; recovery must NOT guess + a reading (guards the recovery predicate degrading to greedy).""" + text = ( + "<|tool_call_start|>[get_weather(city='echo 'hi', unit='celsius')]" + "<|tool_call_end|>" + ) + result = self.detector.detect_and_parse(text, self.tools) + + self.assertEqual(result.calls, []) + + def test_reserved_keyword_parameter_recovered(self): + """A parameter named after a Python keyword (from=1) is a + SyntaxError; the call was dropped. The original parameter name must + be restored in the decoded arguments.""" + text = "<|tool_call_start|>[search(query='M.md', from=1)]<|tool_call_end|>" + result = self.detector.detect_and_parse(text, self.tools) + + self.assertEqual(len(result.calls), 1) + params = json.loads(result.calls[0].parameters) + self.assertEqual(params, {"query": "M.md", "from": 1}) + + def test_zero_padded_int_recovered(self): + """Zero-padded ints (day=07) are a SyntaxError ("leading zeros in + decimal integer literals"); the call was dropped.""" + text = "<|tool_call_start|>[get_weather(city='NYC', day=07)]<|tool_call_end|>" + result = self.detector.detect_and_parse(text, self.tools) + + self.assertEqual(len(result.calls), 1) + params = json.loads(result.calls[0].parameters) + self.assertEqual(params["day"], 7) + + def test_explicit_positive_number(self): + """An explicitly signed positive number (+7) is UnaryOp(UAdd), which + only had a USub branch, so the call was dropped.""" + text = "<|tool_call_start|>[search(query='x', limit=+7)]<|tool_call_end|>" + result = self.detector.detect_and_parse(text, self.tools) + + self.assertEqual(len(result.calls), 1) + params = json.loads(result.calls[0].parameters) + self.assertEqual(params["limit"], 7) + + def test_set_argument_decoded_as_list(self): + """A set argument ({'a', 'b'}) raised in _get_parameter_value and + dropped the call; JSON has no set type so it decodes as a list.""" + text = "<|tool_call_start|>[search(query={'a', 'b'})]<|tool_call_end|>" + result = self.detector.detect_and_parse(text, self.tools) + + self.assertEqual(len(result.calls), 1) + params = json.loads(result.calls[0].parameters) + self.assertEqual(params["query"], ["a", "b"]) + + def test_constant_fstring_argument(self): + """A placeholder-free f-string (f'hello') parses as JoinedStr, not + Constant, and dropped the call although it is a plain string.""" + text = "<|tool_call_start|>[search(query=f'hello')]<|tool_call_end|>" + result = self.detector.detect_and_parse(text, self.tools) + + self.assertEqual(len(result.calls), 1) + params = json.loads(result.calls[0].parameters) + self.assertEqual(params["query"], "hello") + + def test_bytes_argument_skips_only_that_call(self): + """A bytes argument passed _get_parameter_value (it is an + ast.Constant) and only failed later as TypeError inside json.dumps, + which escaped the per-call handler and dropped every sibling call in + the block.""" + text = ( + "<|tool_call_start|>[get_weather(city='SF'), search(query=b'z')]" + "<|tool_call_end|>" + ) + result = self.detector.detect_and_parse(text, self.tools) + + self.assertEqual(len(result.calls), 1) + self.assertEqual(result.calls[0].name, "get_weather") + + def test_non_finite_number_never_emits_invalid_json(self): + """The literal 1e999 overflows to float inf, and json.dumps rendered + it as Infinity — parameters that no JSON parser accepts. The call + must be skipped instead; every emitted parameters string must be + valid JSON.""" + text = "<|tool_call_start|>[search(query='x', limit=1e999)]<|tool_call_end|>" + result = self.detector.detect_and_parse(text, self.tools) + + for call in result.calls: + json.loads(call.parameters) + self.assertEqual(result.calls, []) + + def test_kwargs_unpack_merges_dict(self): + """**-unpacked kwargs were skipped silently, emitting the call with + arguments missing; a dict literal merges with later-binding-wins + semantics instead, and non-dict operands reject the call.""" + text = "<|tool_call_start|>[search(**{'query': 'x'}, limit=2)]<|tool_call_end|>" + result = self.detector.detect_and_parse(text, self.tools) + + self.assertEqual(len(result.calls), 1) + params = json.loads(result.calls[0].parameters) + self.assertEqual(params, {"query": "x", "limit": 2}) + + bad = "<|tool_call_start|>[search(**[1, 2])]<|tool_call_end|>" + self.assertEqual(self.detector.detect_and_parse(bad, self.tools).calls, []) + + def test_positional_argument_call_not_silently_corrupted(self): + """get_weather('Paris', unit='celsius') used to silently drop + 'Paris' and emit a successful call with only {"unit": "celsius"} — + a wrong execution instead of a visible failure. The call is + rejected; a keyword-only sibling still comes through.""" + text = ( + "<|tool_call_start|>[search(query='x'), " + "get_weather('Paris', unit='celsius')]<|tool_call_end|>" + ) + result = self.detector.detect_and_parse(text, self.tools) + + self.assertEqual(len(result.calls), 1) + self.assertEqual(result.calls[0].name, "search") + + def test_good_call_survives_unparsable_block(self): + """A genuinely ambiguous nested quote makes the whole block a + SyntaxError, so no call list exists and the parseable sibling died + with the block, leaving the agent loop with no tool result.""" + text = ( + "<|tool_call_start|>[search(query='ok'), " + "get_weather(city='x 'y', unit='c')]<|tool_call_end|>" + ) + result = self.detector.detect_and_parse(text, self.tools) + + self.assertEqual(len(result.calls), 1) + self.assertEqual(result.calls[0].name, "search") + self.assertEqual(json.loads(result.calls[0].parameters), {"query": "ok"}) + + def test_swallowing_reading_rejected(self): + """Closing the broken string late makes the text parse by absorbing + the sibling call into the argument value, so the tool would run with + corrupted arguments. Rejecting readings that lose calls leaves the + correct early close and recovers both calls.""" + text = ( + "<|tool_call_start|>[search(query='x 'y'), " + "get_weather(city='p 'q')]<|tool_call_end|>" + ) + result = self.detector.detect_and_parse(text, self.tools) + + self.assertEqual([c.name for c in result.calls], ["search", "get_weather"]) + self.assertEqual(json.loads(result.calls[0].parameters), {"query": "x 'y"}) + self.assertEqual(json.loads(result.calls[1].parameters), {"city": "p 'q"}) + + def test_unrecoverable_block_reports_no_calls(self): + """Splitting must not fabricate calls: when no segment parses, the + block yields no tool calls at all.""" + text = ( + "<|tool_call_start|>[search(query='x 'y' 'z), " + "get_weather(city='p 'q' 'r)]<|tool_call_end|>" + ) + result = self.detector.detect_and_parse(text, self.tools) + + self.assertEqual(result.calls, []) + + def test_streaming_recovers_multiline(self): + """Streaming buffers the block and delegates to detect_and_parse; + an incremental rewrite of the streaming path would bypass the + recovery rewrites and re-drop multi-line commands.""" + text = ( + "<|tool_call_start|>[search(query='line one\nline two')]<|tool_call_end|>" + ) + detector = Lfm2Detector() + calls = [] + for i in range(0, len(text), 7): + result = detector.parse_streaming_increment(text[i : i + 7], self.tools) + calls.extend(result.calls) + + self.assertEqual(len(calls), 1) + params = json.loads(calls[0].parameters) + self.assertEqual(params["query"], "line one\nline two") + + def test_reserved_kwarg_suffix_parameter_not_rewritten(self): + """A parameter literally named in_pyreservedkw_ must survive the + normal parse path; only recovery-renamed kwargs get restored.""" + text = ( + "<|tool_call_start|>[search(query='x', in_pyreservedkw_=5)]" + "<|tool_call_end|>" + ) + result = self.detector.detect_and_parse(text, self.tools) + + self.assertEqual(len(result.calls), 1) + params = json.loads(result.calls[0].parameters) + self.assertEqual(params, {"query": "x", "in_pyreservedkw_": 5}) + + def test_reserved_kwarg_with_nested_quote_recovered(self): + """A keyword-named parameter holding a nested-quote command needs + the rename and requote rewrites to compose.""" + text = ( + "<|tool_call_start|>[search(from='sed -n '1,5p' f.py')]" "<|tool_call_end|>" + ) + result = self.detector.detect_and_parse(text, self.tools) + + self.assertEqual(len(result.calls), 1) + params = json.loads(result.calls[0].parameters) + self.assertEqual(params, {"from": "sed -n '1,5p' f.py"}) + # ==================== structure_info tests ==================== def test_supports_structural_tag(self):