From 6629992aa352877f1bc453957b99dd39007c0f2a Mon Sep 17 00:00:00 2001 From: Mika Senghaas Date: Wed, 29 Jul 2026 12:09:06 -0700 Subject: [PATCH] fix(parsing): validate GLM tool names against the provided tools list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit vLLM 0.24 re-aliased "glm45" from the lenient Glm4MoeModelToolParser to Glm47MoeModelToolParser, whose engine runs with validate_tool_names=True (vllm/parser/glm47_moe.py:171): a completed block whose name isn't in the request's tool list is silently dropped — no tool call is emitted and the response reads as a voluntary stop. parse_glm stayed lenient, so train-side parsing (renderer client) disagreed with what an OpenAI chat-completions eval client sees from the engine: unknown-name and missing- blocks kept episodes alive in train but killed them in eval. Mirror the >=0.24 semantics: when tools is passed, a parsed call whose name isn't declared gets the new status UNKNOWN_TOOL instead of OK. The attempt stays visible on parsed.tool_calls (unlike vLLM's silent drop) for verifier / RL-loss consumers, but the client-side stop->tool_calls finish-reason promotion no longer fires, matching the engine. Without tools there is no validation, exactly like vLLM's ParserEngine._is_valid_tool_name on tool-less requests. Cross-validated end-to-end against the real vLLM v0.23.0 and v0.24.0 parsers (vendored from the release tags) on identical completions: before, the renderer matched 0.23's leniency and diverged from 0.24 on unknown-name and missing-arg_key blocks; after, all cases match 0.24. Co-Authored-By: Claude Fable 5 --- renderers/base.py | 12 +- renderers/parsing.py | 45 +++++- tests/test_glm_tool_name_validation.py | 183 +++++++++++++++++++++++++ 3 files changed, 237 insertions(+), 3 deletions(-) create mode 100644 tests/test_glm_tool_name_validation.py diff --git a/renderers/base.py b/renderers/base.py index 304d6cc..e8c9f22 100644 --- a/renderers/base.py +++ b/renderers/base.py @@ -558,8 +558,15 @@ class ToolCallParseStatus(str, enum.Enum): """Per-attempt outcome of parsing a single ```` block. The renderer parser's job is JSON-syntax → ``dict`` (the parser-level - contract). Schema validation — required fields, argument types, tool - name lookup — is the *tool*'s job and is intentionally not done here. + contract). Schema validation — required fields, argument types — is + the *tool*'s job and is intentionally not done here. Tool-*name* + lookup is the one exception, and only where the reference inference + parser does it: vLLM ≥ 0.24 aliases ``glm45``/``glm47`` to a parser + with ``validate_tool_names=True`` that silently drops any call whose + name isn't in the request's tool list. ``parse_glm`` mirrors that as + ``UNKNOWN_TOOL`` (when ``tools`` is passed) so train-side parsing + agrees with what an eval client sees from the engine — but keeps the + attempt visible instead of swallowing it. See ``ParsedToolCall.status`` for what each value means. Diverges from vLLM/SGLang on purpose. Both engines collapse parse @@ -576,6 +583,7 @@ class ToolCallParseStatus(str, enum.Enum): UNCLOSED_BLOCK = "unclosed_block" # opening delim hit EOS / stop MISSING_NAME = "missing_name" # parsed structurally, but no function name MALFORMED_STRUCTURE = "malformed_structure" # format-specific shape error + UNKNOWN_TOOL = "unknown_tool" # name not in the provided tools list @dataclass diff --git a/renderers/parsing.py b/renderers/parsing.py index 0b9d12a..cbc7504 100644 --- a/renderers/parsing.py +++ b/renderers/parsing.py @@ -59,6 +59,26 @@ def _build_param_type_index( return index +def _extract_tool_names(tools: list[ToolSpec] | None) -> set[str] | None: + """Set of declared tool names, or ``None`` when ``tools`` is empty. + + ``None`` disables name validation — mirroring vLLM's + ``ParserEngine._is_valid_tool_name``, which returns ``True`` whenever + the request carries no tools. Accepts both flat ``ToolSpec`` and the + OpenAI ``{"type": "function", "function": {...}}`` envelope, like + ``_build_param_type_index`` (but independent of it: a tool with no + ``parameters.properties`` still counts as a known name). + """ + if not tools: + return None + names: set[str] = set() + for tool in tools: + spec = tool.get("function", tool) if isinstance(tool, dict) else None + if isinstance(spec, dict) and isinstance(spec.get("name"), str): + names.add(spec["name"]) + return names + + def _coerce_arg_value( text: str, param_schema: dict[str, Any] | None ) -> tuple[Any, bool]: @@ -434,7 +454,23 @@ def parse_glm( arg_value_end_id: int, tools: list[ToolSpec] | None = None, ) -> ParsedResponse: - """Parse GLM completion tokens. Token-level thinking + arg_key/arg_value tool calls.""" + """Parse GLM completion tokens. Token-level thinking + arg_key/arg_value tool calls. + + When ``tools`` is passed, tool names are validated against it: a call + whose name isn't declared gets ``status=UNKNOWN_TOOL`` instead of + ``OK``. This mirrors vLLM ≥ 0.24, where the ``glm45``/``glm47`` tool + parsers run with ``validate_tool_names=True`` and silently drop + unknown-name calls (``vllm/parser/glm47_moe.py``, + ``ParserEngine._is_valid_tool_name``) — so a completion that yields no + tool call from the engine also yields no ``OK`` call here, and + downstream finish-reason promotion (``renderers/client.py``) agrees + with an OpenAI chat-completions client talking to the same engine. + Notably this covers the missing-```` shape + (``bash\\n...``): both + this parser and vLLM's resolve the whole block as the name, which then + fails validation. Without ``tools``, no validation happens (vLLM + behaves the same when the request carries no tools). + """ ids = _strip_stop_tokens(token_ids, stop_ids) reasoning = None @@ -468,6 +504,7 @@ def parse_glm( arg_value_end_id, section_offset=parse_offset + tc_start, param_index=_build_param_type_index(tools), + known_names=_extract_tool_names(tools), ) else: content_text = _decode(tokenizer, ids).strip() @@ -491,6 +528,7 @@ def _parse_glm_tool_calls( *, section_offset: int, param_index: dict[str, dict[str, dict[str, Any]]], + known_names: set[str] | None = None, ) -> list[ParsedToolCall]: """Parse GLM-style tool calls: name + arg_key/arg_value pairs, all by token ID.""" tool_calls: list[ParsedToolCall] = [] @@ -548,6 +586,11 @@ def _parse_glm_tool_calls( j += 1 if not name: status = ToolCallParseStatus.MISSING_NAME + elif known_names is not None and name not in known_names: + # vLLM ≥ 0.24 drops the call entirely here; we keep the + # attempt visible but deny it OK so consumers agree with + # the engine on "no tool was called." + status = ToolCallParseStatus.UNKNOWN_TOOL elif structure_broke: status = ToolCallParseStatus.MALFORMED_STRUCTURE elif any_json_fallback: diff --git a/tests/test_glm_tool_name_validation.py b/tests/test_glm_tool_name_validation.py new file mode 100644 index 0000000..e16f51b --- /dev/null +++ b/tests/test_glm_tool_name_validation.py @@ -0,0 +1,183 @@ +"""GLM tool-name validation against the provided tools list. + +vLLM 0.24 re-aliased ``glm45`` from the lenient ``Glm4MoeModelToolParser`` +to ``Glm47MoeModelToolParser``, whose engine config sets +``validate_tool_names=True`` (``vllm/parser/glm47_moe.py``): a completed +```` block whose name isn't in the request's tool list is +silently dropped — no tool call is emitted and the response reads as a +voluntary stop. Before this, unknown-name calls were surfaced to the +harness, which answered with a recoverable "unknown tool" error. + +``parse_glm`` mirrors the ≥ 0.24 behavior when ``tools`` is passed: +unknown names get ``status=UNKNOWN_TOOL`` — never ``OK`` — so the +client-side stop→tool_calls finish-reason promotion +(``renderers/client.py``) agrees with what an OpenAI chat-completions +client sees from the engine, while the attempt itself stays visible for +verifier / RL-loss code. Without ``tools`` there is no validation, also +matching vLLM (``ParserEngine._is_valid_tool_name`` returns ``True`` +when the request has no tools). + +Cross-validated end-to-end against the real vLLM v0.23.0 and v0.24.0 +parsers (vendored from the release tags) on identical completions. +""" + +from __future__ import annotations + +from functools import lru_cache + +from renderers.base import ToolCallParseStatus + +# Both GLM renderers share ``parse_glm`` and are served by the same +# strict vLLM parser (the ``glm45`` and ``glm47`` aliases both resolve +# to ``Glm47MoeModelToolParser`` in vLLM ≥ 0.24). +_MODELS = [ + ("THUDM/GLM-4.5-Air", "auto"), + ("zai-org/GLM-5", "auto"), +] + +_TOOLS = [ + { + "name": "bash", + "description": "Run a shell command.", + "parameters": { + "type": "object", + "properties": { + "command": {"type": "string"}, + "timeout": {"type": "integer"}, + }, + "required": ["command"], + }, + }, + # A declared tool with no parameters must still count as a known + # name (name lookup is independent of the param-type index). + {"name": "submit", "description": "Finish the episode."}, +] + + +@lru_cache(maxsize=None) +def _load(model: str, renderer_name: str): + from renderers import config_from_name, create_renderer + from renderers.base import load_tokenizer + + tok = load_tokenizer(model) + return tok, create_renderer(tok, config_from_name(renderer_name)) + + +def pytest_generate_tests(metafunc): + if "model" in metafunc.fixturenames: + metafunc.parametrize( + "model,renderer_name", + _MODELS, + ids=[m for m, _ in _MODELS], + ) + + +def _parse(model: str, renderer_name: str, text: str, tools): + tok, renderer = _load(model, renderer_name) + ids = tok.encode(text, add_special_tokens=False) + return renderer.parse_response(ids, tools=tools) + + +def _statuses(parsed): + return [tc.status for tc in parsed.tool_calls] + + +def test_known_name_is_ok(model, renderer_name): + parsed = _parse( + model, + renderer_name, + "bash\n" + "command\npwd\n" + "", + _TOOLS, + ) + assert _statuses(parsed) == [ToolCallParseStatus.OK] + assert parsed.tool_calls[0].name == "bash" + assert parsed.tool_calls[0].arguments == {"command": "pwd"} + + +def test_known_name_without_parameters_is_ok(model, renderer_name): + parsed = _parse(model, renderer_name, "submit\n", _TOOLS) + assert _statuses(parsed) == [ToolCallParseStatus.OK] + assert parsed.tool_calls[0].name == "submit" + + +def test_unknown_name_is_flagged(model, renderer_name): + parsed = _parse( + model, + renderer_name, + "read\n" + "lines\n10\n" + "", + _TOOLS, + ) + assert _statuses(parsed) == [ToolCallParseStatus.UNKNOWN_TOOL] + # The attempt stays visible (unlike vLLM's silent drop): name and + # parsed arguments are preserved for verifier / RL-loss consumers. + assert parsed.tool_calls[0].name == "read" + assert parsed.tool_calls[0].arguments == {"lines": 10} + + +def test_unknown_name_without_args_is_flagged(model, renderer_name): + parsed = _parse(model, renderer_name, "finish\n", _TOOLS) + assert _statuses(parsed) == [ToolCallParseStatus.UNKNOWN_TOOL] + + +def test_missing_arg_key_block_is_flagged(model, renderer_name): + # No token ⇒ the whole block resolves as the name — both + # here and in vLLM's engine (unmatched terminals in TOOL_NAME state + # accumulate into the name) — and then fails validation. + parsed = _parse( + model, + renderer_name, + "bash\npwd\n", + _TOOLS, + ) + assert _statuses(parsed) == [ToolCallParseStatus.UNKNOWN_TOOL] + assert "bash" in (parsed.tool_calls[0].name or "") + + +def test_mixed_calls_flag_only_unknown(model, renderer_name): + parsed = _parse( + model, + renderer_name, + "bash\n" + "command\nls\n" + "" + "read\n" + "path\nx\n" + "", + _TOOLS, + ) + assert _statuses(parsed) == [ + ToolCallParseStatus.OK, + ToolCallParseStatus.UNKNOWN_TOOL, + ] + + +def test_no_tools_means_no_validation(model, renderer_name): + # vLLM skips name validation when the request carries no tools; so + # do we — this also keeps tools-less parse_response calls (the + # common test / SFT path) byte-for-byte backward compatible. + parsed = _parse( + model, + renderer_name, + "read\n" + "lines\n10\n" + "", + None, + ) + assert _statuses(parsed) == [ToolCallParseStatus.OK] + + +def test_openai_envelope_tools_are_recognized(model, renderer_name): + wrapped = [{"type": "function", "function": t} for t in _TOOLS] + parsed = _parse( + model, + renderer_name, + "bash\n" + "command\npwd\n" + "", + wrapped, + ) + assert _statuses(parsed) == [ToolCallParseStatus.OK]