Skip to content

fix(tool-parser): preserve whitespace in GLM string arguments - #2744

Merged
slin1237 merged 1 commit into
mainfrom
ai-jz/glm-string-whitespace
Oct 3, 2026
Merged

slin1237 merged 1 commit into
mainfrom
ai-jz/glm-string-whitespace

Conversation

@ai-jz

@ai-jz ai-jz commented Oct 3, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Problem

The GLM parser removes leading indentation and trailing newlines from tool argument strings. A model can produce the correct code, but an editing tool receives different text: " return 1\n" becomes "return 1". That can break Python indentation or prevent an exact-text edit from matching its target. This is a parser data-loss issue, independent of the verifier's expected answer.

Both GLM-4.5/4.6 and GLM-4.7/5 use this shared Rust parser. This completes the schema-aware string preservation introduced by #1841. The MRE below reproduces the data loss without a model server.

Solution

Pass the text inside <arg_value> to the existing schema-aware conversion before trimming it. Declared strings retain their whitespace; unknown-type inference still receives trimmed text. Numeric conversion and JSON-quoted string decoding retain their existing behavior.

Changes

  • Preserve raw argument text for declared strings in the shared GLM parser.
  • Add one focused regression covering code indentation, exact-text matching, whitespace-only strings, and conversion controls in both dialects and complete/streaming parsing.

Test Plan

Minimal reproducible example — CPU only

From an SMG checkout, save the following as crates/tool_parser/examples/repro_glm_whitespace.rs (create the examples directory if needed), then run cargo run -p tool-parser --example repro_glm_whitespace. No model endpoint or GPU is required.

use openai_protocol::common::Tool;
use serde_json::{json, Value};
use tool_parser::{Glm4MoeParser, ToolParser};

#[tokio::main]
async fn main() {
    let tools: Vec<Tool> = serde_json::from_value(json!([{
        "type": "function",
        "function": {
            "name": "edit",
            "parameters": {
                "type": "object",
                "properties": {"code": {"type": "string"}}
            }
        }
    }]))
    .unwrap();
    let code = "    return 1\n";
    let text = format!(
        "<tool_call>edit<arg_key>code</arg_key><arg_value>{code}</arg_value></tool_call>"
    );
    let (_, calls) = Glm4MoeParser::glm47()
        .parse_complete_with_tools(&text, &tools)
        .await
        .unwrap();
    let args: Value = serde_json::from_str(&calls[0].function.arguments).unwrap();
    println!("returned code: {:?}", args["code"].as_str().unwrap());
    assert_eq!(args["code"], code);
}

Remove the temporary example before running repository lint checks.

Check Before After
MRE code argument "return 1"; assertion fails " return 1\n"; assertion passes
Exact-text edit argument "total = 1" " total = 1\n"
Integer conversion and unknown-type inference Existing behavior Same results
JSON-quoted string decoding Existing behavior Same decoded string

Tested on an independent patch against c0d3efa4ff829694164d901bbe7a837923604fc1, with Rust 1.98.0: the MRE fails before and passes after the fix. Both GLM dialects pass complete and streaming checks; all 551 tool-parser tests pass with no skips. Parser Clippy and workspace formatting pass.

cargo test -p tool-parser test_declared_strings_preserve_whitespace
cargo test -p tool-parser

This validation exercises the native parser directly. It does not claim a complete CodeAgent session or GPU serving run. Nullable/union string schemas retain the existing inference and are outside this fix.

Selected command output from the independent patch
Finished `dev` profile [unoptimized] target(s) in 2.57s
test result: ok. 118 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.02s

The test line is the library test binary; the total across all tool-parser test binaries is reported above.

Repository checks

Workspace checks also passed with this patch and the two companion parser fixes (#2743 and #2745) applied together. The parser checks above tested this PR independently.

Command Result
cargo +nightly fmt --all -- --check PASS, exit 0
cargo clippy --locked --all-targets --all-features -- -D warnings PASS, exit 0
cargo test --locked PASS, exit 0; 44 existing ignored tests

WASM test fixtures were built. Fixture-dependent vision cases were not exercised because the CI vision-golden generation step was not run. Ignored external-service/campaign tests and GPU serving were not run; this is not a full hosted-CI result.

Hosted CI

Both workflows completed successfully for PR head 43873eb8077343b2f11eed8414d9a38318ca8f5e:

Workflow Result
PR Test (SMG) PASS, including Rust unit tests and the repository's serving E2E matrix
Benchmark - Tool Parser PASS

Claude Respond and go-bindings-benchmark were skipped by the workflow; they are not counted as passing tests.

Checklist
  • cargo +nightly fmt --all -- --check passes
  • Workspace Clippy passes with all three fixes applied
  • Default workspace cargo test passes with all three fixes applied; limits listed above

Signed-off-by: ai-jz <ai-jz@users.noreply.github.com>
@github-actions github-actions Bot added the tool-parser Tool/function call parser changes label Oct 3, 2026
@coderabbitai

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 22c25186-5156-474c-a972-fc9d892d9f77
📥 Commits

Reviewing files that changed from the base of the PR and between c0d3efa and 43873eb.

📒 Files selected for processing (1)
  • crates/tool_parser/src/parsers/glm4_moe.rs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.


📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • GLM-4.5 and GLM-4.7 parsing now preserves surrounding whitespace in schema-declared string arguments. Values with unknown types continue to be trimmed before type inference.

Walkthrough

GLM-4 MoE argument parsing now preserves captured whitespace for schema coercion and trims values before fallback inference. Tests cover both complete parsing and chunked streaming for GLM-4.5 and GLM-4.7.

Changes

GLM-4 MoE argument parsing

Layer / File(s) Summary
Schema coercion and fallback inference
crates/tool_parser/src/parsers/glm4_moe.rs
parse_arguments passes raw captured values to schema coercion. If coercion does not handle a value, parsing trims it before calling infer_value. Tests cover declared strings, integers, and unknown-type values in complete and chunked parsing.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 43873

No actionable merge risk remains in the supplied evidence; the change is ready for normal merge checks.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files.
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.
Title check ✅ Passed The title clearly summarizes the main change: preserving whitespace in GLM string arguments.
Description check ✅ Passed The description explains the whitespace-loss problem, the parser change, regression coverage, and test results.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Comment thread crates/tool_parser/src/parsers/glm4_moe.rs
@slin1237
slin1237 merged commit 0f9f219 into main Oct 3, 2026
67 checks passed
@slin1237
slin1237 deleted the ai-jz/glm-string-whitespace branch October 3, 2026 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tool-parser Tool/function call parser changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants