Repository navigation
fix(parser): keep GLM tool delimiters inside argument values - #639
lukealonso merged 6 commits into
Conversation
GLM tokenizes a literal </tool_call> or </arg_value> inside an argument value to the same token ID as the structural tag, so the parser can only tell them apart by position. Cover the converter, the non-streaming path, and streaming at both tag and character granularity. Co-Authored-By: Claude Code <noreply@anthropic.com>
A literal </tool_call> or </arg_value> inside a GLM argument value carries the same token ID as the structural tag, so the parser has to use position. Split the argument states: </tool_call> now closes a call only from TOOL_NAME, TOOL_BETWEEN, or TOOL_ARG_END_PENDING, and </arg_value> parks in TOOL_ARG_END_PENDING until the next non-whitespace text decides whether it was structural or data. The new non_whitespace_transitions config field drives that decision and defaults to empty, so other parsers are unaffected. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe GLM-4.7 MoE parser preserves literal tool delimiters in argument values. The parser engine adds pending argument-value handling and non-whitespace transitions. Tests cover streaming, non-streaming, chunked, partial, whitespace, and speculative-decoding inputs. ChangesGLM-4.7 MoE parser updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to GLM tool parsing now preserves literal delimiter text inside argument values while continuing to recognize structural delimiters. The added coverage includes the missing argument-terminator case, with no remaining merge-blocking risk identified. Sequence Diagram(s)sequenceDiagram
participant Tokenizer
participant StreamingParserEngine
participant Glm47MoeParser
participant ToolCallResult
Tokenizer->>StreamingParserEngine: Deliver tag and text chunks
StreamingParserEngine->>Glm47MoeParser: Apply parser state transitions
Glm47MoeParser->>StreamingParserEngine: Preserve literal delimiter as argument content
StreamingParserEngine->>ToolCallResult: Emit parsed tool call
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
4c3c961
into
local-inference-lab:dev/jovian-judgement
Problem
GLM-4.7 and GLM-5.3 tokenize a literal
</tool_call>,</arg_value>, or<tool_call>inside an argument value to the same token ID as the structural tag. When the model writes those strings as data, such as while editing parser or template code, the parser can close the tool call at the first</tool_call>and leak the rest of the value into content.Fix
Position is the only remaining signal, so the parser applies two rules:
</tool_call>is structural only outside an argument value: before arguments, between arguments, or immediately after a candidate</arg_value>. The(TOOL_ARGS, TOOL_END)transition is removed from the GLM transition table.</arg_value>is structural only when the next non-whitespace text is<arg_key>or</tool_call>. A newTOOL_ARG_END_PENDINGstate holds that decision. Any other non-whitespace text returns the engine toTOOL_ARGS.The second rule uses a small parser-engine hook,
ParserEngineConfig.non_whitespace_transitions. Its default is empty, so existing parser configurations do not change._emit_for_stateapplies the configured transition when plain content contains a non-whitespace character.The argument converter uses
_ARG_REwith a lookahead for the next<arg_key>or the end of the argument buffer, plus_TAIL_ARG_REfor the open final argument. In partial mode, a final</arg_value>is held until later text proves whether it is structural or data. This keeps streamed argument prefixes monotonic.Compatibility and failure behavior
glm47_moe_configis also used by the GLM-4.5 aliases and Ling3. Their existing parser tests pass.non_whitespace_transitionsis opt-in and default-inert for every other parser configuration.</arg_value><arg_key>and</arg_value></tool_call>are necessarily structural. The token stream contains no information that could distinguish those boundaries from identical literal data.</arg_value>, a later</tool_call>is treated as argument data and following text remains in the argument untilfinish(). This is the deliberate tradeoff for preserving literal tool delimiters inside values.</arg_value>candidate is used as the malformed fallback and later bytes are discarded.Related work and duplicate check
This is not a duplicate of vllm-project/vllm#54971. That PR prevents stray
arg_keytags from corrupting argument keys; this PR disambiguates delimiters inside argument values. Both edit the GLM argument regexes, so a future upstream rebase must compose its_ARG_KEYprefix with this PR's lookahead patterns.This PR replaces the GLM delimiter portion of #540. The companion scanner correction is #640. Other parser-wide experiments from #540 are intentionally not included.
Open-PR searches found no other upstream or fork PR implementing this argument-value boundary behavior.
Tests
Pytest validation used Python 3.12 in
/opt/venvinside the existingrtx-station/glm53-jovian:lp15image. Source and temporary test dependencies were mounted read-only. Lint validation used the repository-pinned Ruff version throughuvxon the host.Results:
tests/parser/engine: 3,800 passed.git diff --check: passed.Model and serving validation
The cached GLM-5.3 tokenizer maps the relevant delimiters to IDs 154842, 154843, 154844, 154847, 154848, 154849, and 154850, with observation stop ID 154829. An independent token-by-token replay using the production incremental detokenizer passed with literal delimiters, Unicode data, and a stripped final observation token text.
No live model-generation or serving re-probe was run for this revision. The tests validate parser behavior for recorded and constructed token streams, not the model's likelihood of generating them.
Out of scope
</think>. That token is byte-identical to the structural one, so mitigation belongs in the prompt or a logits processor.TokenIDScannercase where a trailing stop token ID has no text in the delta. That is handled by fix(parser): keep anchors when a stop token's text is stripped #640.AI assistance and human review
Claude Code assisted with implementation and tests. OpenAI Codex performed an independent line-by-line diff review and validation. The human submitter reviewed and understands every changed line.