Skip to content

[Bugfix][Parser] Parse closed GLM-4.7 tool arguments once while streaming - #64535

Merged
sfeng33 merged 2 commits into
vllm-project:mainfrom
vMaroon:glm47-resume-closed-args
Oct 29, 2026
Merged

sfeng33 merged 2 commits into
vllm-project:mainfrom
vMaroon:glm47-resume-closed-args

Conversation

@vMaroon

@vMaroon vMaroon commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Overview

A GLM tool call that repeats closed argument pairs makes Glm47MoeParser re-parse every pair on each streaming conversion, so the API server's cost per token grows with the call until the event loop saturates and liveness fails. This parses each closed pair once. Follow-up to #61253, which covers a single long open value.

Claims

  • Streaming a call with N closed pairs matches the argument regex N times in total. On main every conversion matches all closed pairs again: 1,345,003 matches for N = 401 in the new test.
  • Streamed deltas and non-streaming results are unchanged.
  • No new config, flag or API. Scope is the glm47 tool parser; Ling3Parser gets the > gate only (see Details).

Validation

Check Command main 24e80c2614 This PR
New cost test pytest tests/parser/engine/test_glm47_moe.py::TestStreamedToolArguments::test_closed_pairs_are_parsed_once AssertionError: assert 1345003 == 401 passed
GLM parser tests pytest tests/parser/engine/test_glm47_moe.py 22 passed, 1 failed (the new test) 23 passed
Parser suite pytest tests/parser --ignore=tests/parser/mistral/test_tool_calls.py --ignore=tests/parser/test_harmony.py 6660 passed, 3 failed 6661 passed, 2 failed
Looping stream replay (below) GLM-5.3 tokenizer, 200 tok/s, liveness 30 s / 5 s / 3 liveness kill at 275 s no kill in 1200 s, loop 0-7% busy at 240K tokens

Tests ran on CPU in vllm-ci-postmerge-repo:074c83a09a0b872cec92e244d91adf2b3c55e41b-arm64-cpu with the branch tree on PYTHONPATH. The 2 parser-suite failures left are in tests/parser/mistral/test_reasoning.py and fail the same on main: the image ships mistral-common 1.11.6 and main needs 1.13.0 (#64169). The two ignored files fail collection in that image for the same reason and for a harmony download. ruff 0.14.0 and the repo's mypy hook (1.20.2, Python 3.11) are clean on both files.

Also checked on glm47_moe.py:

  • The resumed parse equals a fresh parse on 541,418 random argument texts grown piece by piece (3 seeds), with tag fragments, repeated keys, and a second call reusing the cache.
  • Streamed output with the real GLM-5.3 tokenizer through ParserManager.get_parser("glm47", "glm45"): 11 streams (edits, long writes, open values, typed and array arguments, repeated keys, two calls, plain content) at chunk sizes 1, 3, 4, 7 and 64, plus non-streaming. Content, reasoning, names, arguments and argument-delta counts are identical to the unpatched parser (vLLM d6394cf758, same glm47_moe.py and argument path as main) in 66 of 66 cases.

Not measured: an end-to-end server run with a GLM checkpoint.

Looping stream replay

The replay feeds one GLM-5.3 output through the streaming parser on a virtual clock. The engine emits 200 tokens/s in 4-token steps (MTP at full acceptance). Each event-loop pass hands every token emitted since the previous pass to one parse_delta call. Measured parser time is doubled for a slower server CPU, and kubelet liveness (30 s period, 5 s timeout, 3 failures) is replayed on those passes. The output is </think><tool_call>Edit, a file_path pair, then old_string / new_string pairs repeated.

Stream time main This PR
27 s, 5.4K tokens loop 24% busy 0%
114 s, 22.8K tokens 99% 1%
210-270 s liveness fails 3 of 3 (25-58 s waits), kill at 275 s 1%
1200 s, 240K tokens, 1.46M chars 7%

Once the loop saturates, each pass merges more tokens and each merged event converts again, so passes keep getting longer. The last pass on main merged 6,608 tokens, ran 7,267 conversions and took 62.9 s.

Details

Root cause. ParserEngine._compute_arg_delta converts the whole raw argument text whenever the open-value extender from #61253 is inactive: between arguments, and while the open key repeats an earlier key (the extender needs the open value to be the last JSON value). GLM set no arg_structural_chars, so every argument chunk event converted, and _glm47_parse_args re-ran _ARG_RE.finditer over every closed pair plus _ARG_RE.sub over the whole text. Merging deltas under load does not bound this, because conversions scale with the events in the merged text.

We hit this serving GLM-5.3 on vLLM 0.32.0, which includes #61253. Decode API servers climbed from 0.3 to 1 core in 3-4 minutes and failed liveness, five times in 24 hours. In three of them the logs show a single request left on the pod, generating at 99-100% MTP draft acceptance (the model repeating itself) while the stats line slipped from 10 s to 15-40 s apart. Stream content was not captured in production; the replay reproduces the same failure through the parser.

Change.

  • arg_structural_chars=frozenset(">"), as DeepSeek V3.2 and V4 use. A delta without > cannot close a key or value, so it cannot produce a streamable change: an open last value goes through the extender, and an open repeated key sits mid-JSON where the prefix check withholds it. The next conversion or the final flush converts the full text. The gate alone only delays the kill in the replay (305 s against 215 s, measured on d6394cf758), because each conversion still re-scans.
  • _glm47_parse_args takes an optional _ClosedArgs cache, and Glm47MoeParser binds one per instance into its converter and extender, the same way DeepSeek V4 and MiniMax M3 replace _arg_converter after super().__init__. A text that extends the cached text resumes after its last closed pair. Any other text parses from the start, and the cache moves forward whenever a scan closes a new pair. Appending text cannot change a pair that already closed, since each match ends at the first </arg_value> after its key, so resuming is exact.

Cost. What stays linear in the text per conversion is C-level work: startswith against the cached text, the slice when a pair closes, and the join of text outside the pairs for the open-key search. That is the 7% at 1.46M chars above.

Compatibility and limits.

  • A parser built with a custom arg_converter keeps the uncached path (identity check in __init__), so tests that wrap the converter still see every call.
  • Ling3Parser calls ParserEngine.__init__ directly, so it gets the gate from the shared config and no cache.
  • A looping call still generates until max_tokens; this keeps the server responsive while it does.
  • Unchanged here and the same on main: when a repeated key closes with a different value than before, the final JSON no longer extends what was streamed and the flush withholds it, so the client gets truncated arguments. I can file that separately.

Not a duplicate. #61253 (merged) covers one long open value. #58418 (open) edits _glm47_parse_args for an unclosed final value; the overlap is textual only. #51141 and #64196 address Inkling and DeepSeek DSML. No open PR covers re-parsing closed GLM pairs.

AI assistance. Claude Code was used for the root-cause analysis, the fix, the tests and this description.


Pull Request Checklist
  • I used vLLM's /pr-checklist skill. (Mandatory for agents, optional for humans).

  • AI assistance was used during the creation of this PR.

  • Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.

  • Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.

  • Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.

  • Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.

…ming

A GLM tool call that repeats closed argument pairs re-parsed every pair on
each conversion, so the API server's cost per token grew with the call. A
looping model saturates the event loop within minutes and fails liveness.

Skip conversions for deltas without '>' and resume each conversion after
the last closed pair from a per-parser cache.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Maroon Ayoub <mayoub@redhat.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@sfeng33 sfeng33 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the work, I tested it end to end on a live server:
Setup: RedHatAI/GLM-5.3-MXFP4xFP8_BLOCK, TP4 on H200, --tool-call-parser glm47 --reasoning-parser glm47. I ran this PR (fef48163f6) and its merge-base (24e80c2614) side by side with the same flags.

GLM-5.3 reproduced the loop on its own: asked for a MultiEdit call, it repeated the closed file_path pair about 1,600 times until max_tokens.

Looping requests API server CPU, main API server CPU, this PR
1 35% of a core 3%
8 86% 6%
16 93% 10%

@sfeng33 sfeng33 added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 29, 2026
@sfeng33

sfeng33 commented Oct 29, 2026

Copy link
Copy Markdown
Member

/ci run

@github-actions

Copy link
Copy Markdown

✅ @vMaroon, CI is now available for this PR.

  • /ci run starts upstream CI; /amd-ci run starts AMD CI only.
  • Run commands test your branch with every commit currently on its upstream target branch. If it is behind, the bot merges the target branch into it first (pull before pushing again). Append --allow-stale to a run command to test an outdated branch as is, at your own risk.
  • /ci retry retries failed jobs in the CI build for the current PR head. If the current head has no CI build, it starts a new CI build for the current head containing only jobs that failed in the latest earlier CI build for this PR.
  • /amd-ci retry retries failed jobs in AMD CI for the current PR head. Use /amd-ci run when the current head has no AMD CI build.
  • /ci cancel cancels scheduled or running CI builds for this PR branch; /amd-ci cancel does the same for AMD CI only.

@sfeng33
sfeng33 enabled auto-merge (squash) October 29, 2026 18:16
@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #98409 for commit aab61e135754.

@sfeng33
sfeng33 merged commit 255431b into vllm-project:main Oct 29, 2026
71 checks passed
bbrowning added a commit to bbrowning/vllm that referenced this pull request Oct 30, 2026
Add ParserEngineConfig.slot_arg_converter, a factory the engine calls once
per ToolCallSlot. The object it returns owns both the converter and the
open-value extender for that call and only ever sees that slot's growing
argument text, so it can resume from the arguments it has already parsed
instead of re-converting from byte 0 on every streamed event.

Adopt it for the GLM-4.7 config: Glm47ArgConverter parses each closed
<arg_key>/<arg_value> pair once, and the ">" structural gate skips deltas
that cannot close a tag. Ling3 shares the config and inherits both.
Streamed and non-streaming arguments are unchanged against main (240
chunked scenarios, 90K fuzzed resumed parses). A 2000-pair looping call
streams in 2.9 s instead of 300 s.

Prototype of the engine-level alternative to vllm-project#64535.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Ben Browning <56071+bbrowning@users.noreply.github.com>
bbrowning added a commit to bbrowning/vllm that referenced this pull request Oct 30, 2026
Profiling the looping GLM call showed the remaining cost was not in the
parser: the engine re-joined the slot's whole argument text for every
conversion (1.4 s of 3.6 s at 2000 pairs), which no converter can avoid
while it receives raw_args. The SlotArgConverter protocol now takes
feed(delta) / convert(partial) / extender(), and the stateless
arg_converter and arg_value_extender config hooks are wrapped in one
per-slot adapter, so the engine has a single conversion path and no
fallback assert.

Glm47ArgConverter keeps only the text after the last closed value. The
retained inter-pair text is gone: a closed match consumes every earlier
<arg_key>, so the open pair can only start after the last closed value,
and the partial search no longer rescans the gaps. The stateless wrapper
functions had no callers left and are removed.

2000 looping pairs at 4-char chunks: main 317 s, vllm-project#64535 3.4 s, this
1.1 s; output identical to main on 240 chunked scenarios and 183K fuzzed
parses.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Ben Browning <56071+bbrowning@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working glm ready ONLY add when PR is ready to merge/full CI is needed tool-calling

Projects

Status: Unsorted queue

Development

Successfully merging this pull request may close these issues.

2 participants