Skip to content

feat(parser): route GLM-5.x to existing GLM-4.7/4.5 parsers#1833

Merged
key4ng merged 1 commit into
mainfrom
feat/glm5-parser-mapping
Jun 24, 2026
Merged

feat(parser): route GLM-5.x to existing GLM-4.7/4.5 parsers#1833
key4ng merged 1 commit into
mainfrom
feat/glm5-parser-mapping

Conversation

@key4ng

@key4ng key4ng commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Description

Problem

SMG had no routing for GLM-5/5.1/5.2 model IDs. A model ID like glm-5.2-fp8 fell through the tool-parser catch-all glm-* -> json (which cannot parse GLM's <arg_key>/<arg_value> XML, silently breaking tool calls), and matched no reasoning pattern (falling back to the default parser, so reasoning was not extracted).

Solution

GLM-5.x reuse existing formats — no new parser code needed. They use the GLM-4.7 tool-call format (glm47_moe / Glm4MoeParser::glm47()) and the GLM-4.5 <think>/</think> reasoning format (glm45). This mirrors vLLM's published serving command for GLM-5.2: --tool-call-parser glm47 --reasoning-parser glm45. GLM-5.2 swapped enable_thinking for a reasoning_effort field, but the emitted delimiters are unchanged, so the parsers are unaffected.

Changes

  • tool_parser/src/factory.rs: map glm-5* -> glm47_moe (longest-prefix-wins beats the glm-* -> json catch-all).
  • reasoning_parser/src/factory.rs: register substring pattern glm-5 -> glm45.
  • Tests: integration test asserting glm-5{,.1,.2,.2-fp8} route to glm47_moe and extract a tool call; one-line assertion in the reasoning factory test.

Test Plan

  • cargo test -p tool-parser -p reasoning-parser — all suites pass, including the new test_glm5_routes_to_glm47_moe.
  • cargo +nightly fmt --all -- --check — clean.
  • cargo clippy -p tool-parser -p reasoning-parser --all-targets --all-features -- -D warnings — clean.
Checklist
  • cargo +nightly fmt passes
  • cargo clippy --all-targets --all-features -- -D warnings passes
  • (Optional) Documentation updated
  • (Optional) Please join us on Slack #sig-smg to discuss, review, and merge PRs

Summary by CodeRabbit

  • New Features
    • Added support for GLM-5 model variants to use the appropriate existing reasoning and tool-parsing behavior.
  • Tests
    • Added coverage to confirm GLM-5 and GLM-5.x model IDs route to the correct parsers and correctly extract the expected tool call.

@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b8fa2bb4-8458-4953-ae44-2eec539beca8

📥 Commits

Reviewing files that changed from the base of the PR and between 8aff84c and 363fb53.

📒 Files selected for processing (3)
  • crates/reasoning_parser/src/factory.rs
  • crates/tool_parser/src/factory.rs
  • crates/tool_parser/tests/tool_parser_glm47_moe.rs

📝 Walkthrough

Walkthrough

Two factory registrations are added: glm-5 is mapped to the glm45 reasoning parser in reasoning_parser, and glm-5* is mapped to the glm47_moe tool parser in tool_parser. A new async test verifies the tool parser routing across multiple GLM-5 model identifiers, and an existing reasoning parser test gains an assertion for glm-5.2-fp8.

Changes

GLM-5 Parser Routing

Layer / File(s) Summary
GLM-5 pattern registration and routing tests
crates/reasoning_parser/src/factory.rs, crates/tool_parser/src/factory.rs, crates/tool_parser/tests/tool_parser_glm47_moe.rs
Registers glm-5glm45 in the reasoning parser factory and glm-5*glm47_moe in the tool parser factory. Tests assert glm-5.2-fp8 resolves to model_type "glm45" and that multiple glm-5* identifiers extract a get_weather tool call via the MoE parser.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested reviewers

  • slin1237
  • CatherineSue

Poem

🐇 A new model hops down the factory lane,
GLM-5 finds its parser, no need to explain.
"glm45" for reasoning, "moe" for the tools,
Three lines of code following well-trodden rules.
The rabbit checks routing and hops away pleased! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely summarizes the main change: adding routing configuration for GLM-5.x model IDs to existing GLM-4.7/4.5 parsers.
Docstring Coverage ✅ Passed Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/glm5-parser-mapping

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request adds support for GLM-5.x models by routing them to existing parsers. Specifically, GLM-5.x models are configured to reuse the 'glm45' reasoning format in 'reasoning_parser' and the 'glm47_moe' tool parser in 'tool_parser'. Corresponding unit tests have been added to verify correct routing and parsing behavior. There are no review comments, and we have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@github-actions github-actions Bot added tests Test changes tool-parser Tool/function call parser changes reasoning-parser Reasoning parser changes labels Jun 23, 2026

@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.

Clean PR — minimal routing additions with good test coverage. The longest-prefix-wins semantics in the tool parser registry guarantee glm-5* beats the glm-* catch-all, and the reasoning parser substring match on "glm-5" correctly covers all GLM-5.x variants. No issues found.

GLM-5/5.1/5.2 use the same tool-call format as GLM-4.7 (glm47_moe)
and the same <think> reasoning format as GLM-4.5 (glm45), matching
vLLM's --tool-call-parser glm47 --reasoning-parser glm45.

Add model-ID mappings so canonical glm-5.x IDs auto-resolve to those
parsers instead of falling through to the glm-* -> json catch-all.

Signed-off-by: key4ng <rukeyang@gmail.com>
@key4ng key4ng force-pushed the feat/glm5-parser-mapping branch from 8aff84c to 363fb53 Compare June 24, 2026 02:28
@key4ng key4ng merged commit 4d931ae into main Jun 24, 2026
62 of 77 checks passed
@key4ng key4ng deleted the feat/glm5-parser-mapping branch June 24, 2026 04:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reasoning-parser Reasoning parser changes tests Test changes tool-parser Tool/function call parser changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant