Skip to content

fix: validate GLM tool names against the provided tools list - #114

Merged
mikasenghaas merged 1 commit into
mainfrom
fix/glm45-vllm-024-tool-name-validation
Jul 29, 2026
Merged

fix: validate GLM tool names against the provided tools list#114
mikasenghaas merged 1 commit into
mainfrom
fix/glm45-vllm-024-tool-name-validation

Conversation

@mikasenghaas

@mikasenghaas mikasenghaas commented Jul 29, 2026

Copy link
Copy Markdown
Member

Summary

  • vLLM 0.24 re-aliased the glm45 tool parser from the lenient Glm4MoeModelToolParser to Glm47MoeModelToolParser, whose engine runs with validate_tool_names=True (vllm/parser/glm47_moe.py:171): a completed <tool_call> block whose name isn't in the request's tool list is silently dropped — no tool call is emitted, tools_called=False, 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 same engine.
  • parse_glm now mirrors the ≥ 0.24 semantics when tools is passed: a parsed call whose name isn't declared gets the new ToolCallParseStatus.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 (renderers/client.py) no longer fires — so both sides agree on "no tool was called". This also covers the missing-<arg_key> shape (<tool_call>bash\n<arg_value>…</arg_value></tool_call>): both parsers resolve the whole block as the name, which then fails validation.
  • Without tools, no validation happens — matching vLLM (ParserEngine._is_valid_tool_name returns True on tool-less requests) and keeping all existing tools-less parse_response callers untouched.
  • Applies to both GLM45Renderer and GLM5Renderer (shared parse_glm; vLLM's glm45 and glm47 aliases both resolve to the strict parser in ≥ 0.24).
  • New tests/test_glm_tool_name_validation.py pins the semantics for GLM-4.5-Air and GLM-5.

Verification

Cross-validated end-to-end against the real vLLM v0.23.0 and v0.24.0 parser sources (vendored from the release tags, serving-layer imports stubbed) on identical completions — token ids to the renderer, decoded text to vLLM, as in production. Comparison is at the harness-visible level: dispatched (name, args) pairs plus whether the episode continues (finish_reason == "tool_calls").

Before (renderer tracks 0.23, diverges from 0.24):

case                    vllm-0.23                       vllm-0.24   renderer (OK calls)
valid_call              bash{command,timeout}  continue  same        same                            MATCH
unknown_tool_name       read{path}             continue  dropped     no OK call                      MATCH
missing_arg_key         bash{}                 continue  dropped     OK "bash\n<arg_value>pwd..."    DIVERGE
unknown_tool_json_args  read{lines:10}         continue  dropped     OK read{lines:10}               DIVERGE
unknown_tool_no_args    submit{}               continue  dropped     OK submit{}                     DIVERGE
plain_stop              —                      stop      stop        stop                            MATCH

After: all six cases MATCH vLLM 0.24 (unknown-name attempts surface as unknown_tool, valid calls still parse with schema coercion, plain stops unchanged).

Full suite: 2573 passed, 169 skipped, 1 xfailed. ruff + ty clean.

Breaking

  • parse_glm / GLM45Renderer.parse_response / GLM5Renderer.parse_response: when tools is passed, calls naming an undeclared tool now return status=UNKNOWN_TOOL instead of OK. Downstream OK-filters (including the renderer client's finish-reason promotion) stop treating them as tool turns — this is the intended parity with vLLM ≥ 0.24. Migration: callers that want the old lenient behavior can omit tools at parse time or additionally accept UNKNOWN_TOOL when filtering.
  • ToolCallParseStatus gains a member (UNKNOWN_TOOL = "unknown_tool"); exhaustive matches over the enum need a new arm.

🤖 Generated with Claude Code


Note

Medium Risk
Behavior change when parse_response(..., tools=...) is used: undeclared names no longer count as OK, affecting finish-reason promotion and any exhaustive ToolCallParseStatus matches; tools-less parsing is unchanged.

Overview
Aligns GLM train-side parsing with vLLM ≥ 0.24, where undeclared tool names are no longer treated as successful calls.

Adds ToolCallParseStatus.UNKNOWN_TOOL and _extract_tool_names so parse_glm (GLM-4.5 / GLM-5) checks function names against the passed tools list when it is non-empty. Unknown names get UNKNOWN_TOOL instead of OK, so renderers/client.py stop→tool_calls finish-reason promotion matches an OpenAI client on the same engine, while the parse attempt stays on parsed.tool_calls for verifiers/RL. No tools ⇒ no validation (unchanged). OpenAI function envelopes and parameter-less tools count as known names.

New tests/test_glm_tool_name_validation.py covers known/unknown/mixed calls, missing-<arg_key> shapes, and tools-less backward compatibility.

Reviewed by Cursor Bugbot for commit 6629992. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Validate GLM tool call names against the provided tools list

  • Adds UNKNOWN_TOOL to the ToolCallParseStatus enum in renderers/base.py to represent parsed calls whose name is not in the declared tools list.
  • Introduces _extract_tool_names helper in renderers/parsing.py to derive the set of valid tool names from the tools list, supporting both flat ToolSpec dicts and OpenAI-style {'type': 'function', 'function': {...}} envelopes.
  • When a tools list is provided, _parse_glm_tool_calls now assigns status=UNKNOWN_TOOL (instead of OK) for calls whose names are not declared; name and arguments are still populated. When no tools list is provided, behavior is unchanged.
  • Behavioral Change: callers that check ParsedToolCall.status will now see UNKNOWN_TOOL for unrecognized tool names when a tools list is passed to the GLM parser.

Macroscope summarized 6629992.

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 <tool_call> 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-<arg_key> 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 <noreply@anthropic.com>
@macroscopeapp

macroscopeapp Bot commented Jul 29, 2026

Copy link
Copy Markdown

Approvability

Verdict: Needs human review

This PR introduces a runtime behavior change to GLM parsing: tool calls with unrecognized names will now receive UNKNOWN_TOOL status instead of OK when tools are provided. While well-scoped and tested, this changes how downstream consumers interpret parsing results and warrants human review.

You can customize Macroscope's approvability policy. Learn more.

@mikasenghaas
mikasenghaas requested a review from hallerite July 29, 2026 19:16
@mikasenghaas
mikasenghaas merged commit 5fdf78c into main Jul 29, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants