fix(tool-parser): honor additionalProperties parameter types

[ai-jz]

Description

Problem

A tool can accept arbitrary dictionary keys but require their values to be strings with additionalProperties: {"type":"string"}. SMG ignores that declaration and turns the model's text 42 into a number, or null into JSON null. A tool expecting strings can then reject the arguments.

The same text is handled correctly when its type is declared directly in properties. This fills a gap in the schema-based string preservation introduced by SMG's #1841. The expected type comes from the tool's JSON Schema, rather than another provider's response.

Solution

Use the explicit property's type first. For names absent from properties, also read the single declared type from additionalProperties before applying the existing conversion. GLM, Qwen XML, and MiniMax M2 share this lookup. Only these three parsers are changed; MiniMax M3 and HY v4 use separate lookups and remain unchanged.

Behavior change: these additional arguments follow their declared type. Explicit properties, including those without a type, retain precedence. Boolean, union, and unknown schemas retain the existing inference. When patternProperties is nonempty, this change conservatively leaves unlisted names on the existing path; it does not add pattern matching or $ref resolution.

Changes

• Add an internal schema type lookup; preserve the existing public helper's signature and behavior.
• Use it in the three XML parsers and add focused complete/streaming regressions plus precedence and fallback controls.

Test Plan

Minimal reproducible example — CPU only

From an SMG checkout, create crates/tool_parser/examples, save the following as repro_additional_properties.rs inside it, and run cargo run -p tool-parser --example repro_additional_properties. No model endpoint or GPU is required. Before the fix, the first assertion passes and the second fails; after the fix, both pass.

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

#[tokio::main]
async fn main() {
    let text = "<tool_call>save_config<arg_key>code</arg_key><arg_value>42</arg_value></tool_call>";
    for schema in [
        json!({"type":"object","properties":{"code":{"type":"string"}}}),
        json!({"type":"object","additionalProperties":{"type":"string"}}),
    ] {
        let tools: Vec<Tool> = serde_json::from_value(json!([{
            "type":"function","function":{"name":"save_config","parameters":schema}
        }])).unwrap();
        let parser = Glm4MoeParser::glm47();
        let (_, calls) = parser.parse_complete_with_tools(text, &tools).await.unwrap();
        assert_eq!(calls.len(), 1);
        let arguments: Value = serde_json::from_str(&calls[0].function.arguments).unwrap();
        println!("schema={schema}\narguments={arguments}");
        assert_eq!(arguments, json!({"code":"42"}));
    }
}

Remove the temporary example before running repository lint checks.

Check	Before	After
Direct-property string control	{"code":"42"}	Same; assertion passes
Same type through additionalProperties	{"code":42}; assertion fails	{"code":"42"}; assertion passes
GLM, Qwen XML, MiniMax M2 complete and streaming	Lookup ignores the declaration	All 6 complete/streaming combinations preserve "42" and "null"

Tested on an independent patch against c0d3efa4ff829694164d901bbe7a837923604fc1, with Rust 1.98.0: the MRE passes, all 553 tool-parser tests pass with no skips, and parser Clippy and workspace formatting pass. The two lookup regressions also check explicit-property precedence, existing integer types, and unchanged fallback for boolean, union, and patterned schemas.

cargo test -p tool-parser --test tool_parser_additional_properties
cargo test -p tool-parser

This checks the native Rust parser. It does not claim HTTP serving or model inference validation.

Selected command output from the independent patch

Finished `dev` profile [unoptimized] target(s) in 1.40s
test result: ok. 119 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 (#2744 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 228630c809a633d4d45c192f74e093405c0c9be3:

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
• Independent tool-parser tests and combined workspace results recorded

[coderabbitai]

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: 480bb8f4-17d3-46e2-9349-033227e2d07d

📥 Commits

Reviewing files that changed from the base of the PR and between c0d3efa and 228630c.

📒 Files selected for processing (5)

• crates/tool_parser/src/parsers/glm4_moe.rs
• crates/tool_parser/src/parsers/helpers.rs
• crates/tool_parser/src/parsers/minimax_m2.rs
• crates/tool_parser/src/parsers/qwen_xml.rs
• crates/tool_parser/tests/tool_parser_additional_properties.rs

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

---

📝 Summary

Summary by CodeRabbit

• Bug Fixes
  • Improved tool-argument parsing for GLM4-MoE, Qwen XML, and Minimax M2 when schemas specify types for additional properties. Arguments are now coerced according to the declared schema type in both complete and streaming responses.
  • Preserved fallback parsing for values without a recognized schema type, including cases where pattern-based properties make the type unclear.

Walkthrough

The change adds ParamTypes for reading scalar parameter types from function schemas. GLM4-MoE, Minimax M2, and Qwen XML use it during complete and incremental parsing. New tests cover schema lookup and additionalProperties arguments.

Changes

Parameter type lookup

Layer / File(s)	Summary
Schema lookup behavior crates/tool_parser/src/parsers/helpers.rs	Adds ParamTypes to resolve scalar types from explicit properties or additionalProperties. Nonempty patternProperties prevents fallback for unlisted names. Unit tests cover these cases and unsupported schemas.
Parser integration and validation crates/tool_parser/src/parsers/glm4_moe.rs, crates/tool_parser/src/parsers/minimax_m2.rs, crates/tool_parser/src/parsers/qwen_xml.rs, crates/tool_parser/tests/tool_parser_additional_properties.rs	Updates complete and incremental parsing to use ParamTypes for schema-based coercion. Adds tests for additionalProperties string arguments in all three formats.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 22863

The additional-properties change is mergeable after normal checks; support for single-element type arrays can be considered separately.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 22863

The change is limited to argument typing and preserves the inspected parsing and cleanup behavior. No introduced security concern was established, but downstream authorization and argument validation were not verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The established change affects argument values returned by three parser dialects, not tool selection or invocation authority. The effective downstream asset, tenant, privilege, and execution scope remains unverified.

Trust Boundaries and Controls

• observed — Generated text supplies function names, argument keys, and values; the caller supplies schemas. Streaming GLM and Qwen paths retain checks against supplied tool names. Those checks are not evidence of final authorization, and complete parsing remains capable of returning names without a matching schema, as before.

Resilience and Maintainability Implications

• inferred — Transition inspection found no new persistent schema state, error path, or cleanup obligation. Lookup remains transient and function-scoped; partial buffering, completion, tool indexing, recovery, and reset retain their existing control flow. This supports unchanged parser-level failure containment, not end-to-end execution safety.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 92.31% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 5 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: honoring parameter types declared through additionalProperties.
Description check	✅ Passed	The description explains the problem, solution, affected parsers, behavior, and tests. It is directly related to the changeset.

✨ 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.}