fix(glm53): preserve content across reasoning and tool parsing - #433
Conversation
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_388c0e4e-b0e7-42ae-a0f5-ccf2fa3f7b53) |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟡 Changes recommended
The new GLM parser overlay can retain stale tool definitions across reused parser instances because it only updates _tools when request.tools is truthy, which can break correctness for tool-less/empty-tool requests.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR hardens GLM-5.3 Chat Completions parsing and request normalization to prevent loss of literal tool-call XML / post-tool text / literal think tags, while adding an opt-in “required-first” tool-schema ordering tweak and expanding CPU-only regression and acceptance-style checks.
Changes:
- Add a pinned GLM-only
glm47_moe.pyparser overlay that preserves user content across reasoning/tool parsing and fixes non-streaming / chunk-boundary content collection issues. - Add default-off
glm53_required_firstoption to reorder JSON Schemapropertiessorequiredfields appear first (recursively) without changing schema semantics or request payload. - Expand CPU tests/contract probes and add an opt-in network acceptance probe + pinned official HF template fixture for parity checks.
File summaries
| File | Description |
|---|---|
| tests/test_glm53_tool_acceptance.py | New CPU-only unit tests for acceptance probe stream assembly / verdict checks. |
| tests/test_glm53_chat.py | Adds pinned official template parity test and glm53_required_first regression coverage. |
| tests/fixtures/glm53_official_chat_template.json | Pins upstream HF template revision/URL/SHA metadata. |
| tests/fixtures/glm53_official_chat_template.jinja | Adds pinned upstream HF chat template fixture for parity checks. |
| profiles/README.md | Updates overlay-file/module counts and runtime description to include GLM body preservation. |
| probes/glm53_tool_choice_acceptance.py | New opt-in live-endpoint acceptance probe (schema/tool/SSE finish checks; no tool execution). |
| probes/glm53_chat_contract.py | Exercises real ParserManager dispatch and auto content-format detection in CPU replay. |
| overlay/modules/glm53_runtime/README.md | Documents new parser overlay behavior + required-first option + acceptance probe usage. |
| overlay/modules/glm53_runtime/manifest.tsv | Adds pinned glm47_moe.py overlay entry for the serving image base SHA. |
| overlay/modules/glm53_runtime/glm53_chat.py | Implements glm53_required_first normalization and recursive schema reordering. |
| overlay/modules/glm53_runtime/glm47_moe.py | New content-preserving GLM-4.7 parser overlay built on the declarative parser engine. |
| launchers/start-glm53-nvfp4-tp4.sh | Requires the new parser overlay mount before non-dry-run startup. |
| launchers/check-glm53-chat.sh | Gates the overlay against the image’s stock parser SHA; runs added CPU tests. |
| build/glm53/manifest.tsv | Build manifest updated to include the pinned parser overlay entry. |
| build/glm53/glm53_chat.py | Build output mirrors glm53_chat.py required-first changes. |
| build/glm53/glm47_moe.py | Build output mirrors the new pinned parser overlay. |
Review details
- Files reviewed: 16/16 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df7cf4c291
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_83f3fbdb-f42c-47d8-bde3-b0af8f413643) |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_8867a7b7-224d-4960-b2f6-cfef3410b6f5) |
GLM Chat Completions could delete literal tool-call XML with
tool_choice="none", lose text after/between calls in non-streaming replies, and remove literal think tags or misalign token IDs inside tool arguments. Preserve that data in the GLM parser while keeping initial reasoning separation, typed arguments, tool IDs, and the requested tool-choice mode.ParserManagerglm45/glm47 dispatch, request adjustment/structural tags, and automatic history content-format detection. Retain direct-engine checks and add none/auto/required/named, repeated functions, literal tags, partial delimiters, reasoning visibility, and token/character chunk boundaries.glm53_required_firstrequest ordering before template rendering. It preserves JSON Schema semantics and input objects; it does not downgrade forced calls. Pin the official HF template revision with differential text/media-placeholder/tool-history fixtures.Validation:
glm53:v13-b12x-itimage and local tokenizer, isolated CPU container with no GPU or network: 19 template/option/middleware tests + 5 acceptance-verdict tests + 26 parser scenarios / 3,765 streaming replays and a 3-request parser-reuse sequence passed. vLLM0.1.dev20051+g487ecf187; parser SHAd339da08f143f8316f1cda96e0628c2e5438b9510039c2d262c37d058f8f401d.Runtime limits: this PR does not deploy/restart production or establish NVFP4 generation-quality/forced-decoding convergence. The required-first option stays off pending a same-image/weights A/B. The new network acceptance probe is not invoked by the CPU release gate. HTTP/SSE finish reasons and partial arguments are not repaired or reclassified.
References: none content loss, required-first draft, forced generation report, official template update.
Note
Medium Risk
Changes sit on the hot path for GLM Chat Completions parsing and streaming deltas; mistakes could alter tool-call extraction, reasoning separation, or client-visible content, though scope is limited to the overlay and extensive CPU replay tests were added.
Overview
Adds a GLM-only
glm47_moeparser overlay (pinned to the serving image base SHA) so Chat Completions no longer drops literal tool XML undertool_choice=none, loses text between/after tool calls in non-streaming replies, or strips/mis-handles think tags inside arguments. The overlay skips tool recognition instead of consuming-then-hiding, merges text with tool deltas, preserves whitespace, and tightens reasoning/token boundaries when thinking is off.Chat middleware gains default-off
glm53_required_first: before template render, tool parameter schemas are deep-copied andpropertiesreordered sorequiredfields appear first (nested/$defsincluded) without changing JSON Schema semantics ortool_choice.Release gates now require the parser mount (
start-glm53-nvfp4-tp4.sh), verify image parser SHA and mount the candidate incheck-glm53-chat.sh, and expand CPU probes (glm53_chat_contract,test_glm53_tool_acceptance, pinned official HF template parity). An opt-inglm53_tool_choice_acceptance.pyprobe hits a live endpoint for generation A/B (not in the default CPU gate).Reviewed by Cursor Bugbot for commit 3ad2c65. Bugbot is set up for automated code reviews on this repo. Configure here.