Repository navigation
fix(tool-parser): close MiniMax M2 streamed argument objects - #2745
Conversation
Signed-off-by: ai-jz <ai-jz@users.noreply.github.com>
|
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
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review. 📝 SummarySummary by CodeRabbit
WalkthroughWhen a MiniMax M2 invocation ends after streaming parameters, the parser now emits and stores a closing brace unconditionally. New tests cover nested object parameters, chunked input, and successive parser runs. ChangesMiniMax M2 streaming
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The MiniMax M2 streaming fix is mergeable after normal checks; no remaining defect is established. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Description
Problem
SMG's MiniMax M2 tool parser can return a valid tool call in non-streaming mode but malformed JSON in streaming mode. For example, a tool expects
{"data":{"host":"db.example"}}, but the assembled stream is{"data": {"host":"db.example"}: the outer arguments object never closes, so the client cannot decode it or execute the tool. This was found while checking SMG's native Rust parser; it does not depend on a verifier's expected answer.This fixes
minimax_m2; MiniMax M3 uses the separateminimax_m3parser, which this PR does not change.The completion check treats a value's final
}as the end of the outer object. Counting braces also treats braces inside strings as JSON structure.Solution
When
</invoke>arrives, close the outer object that the parser opened. Parameter fragments never close that object themselves. Preserve the{}output for empty calls from #2718.This follows the existing Qwen XML fix in #2490: close the outer object at the tool-call boundary rather than counting braces inside values.
Changes
Test Plan
Minimal reproducible example — CPU only, no model required
From an SMG checkout, create the example directory with
mkdir -p crates/tool_parser/examples. Save the following ascrates/tool_parser/examples/repro_minimax_closure.rs, then runcargo run -p tool-parser --example repro_minimax_closure.Remove the temporary example before running repository lint checks.
{"data":{"host":"db.example"}}}; JSON decoding failsTested on an independent patch against
c0d3efa4ff829694164d901bbe7a837923604fc1, with Rust 1.98.0: the MRE fails before and passes after the fix, and all 552 tool-parser tests pass with no skips. Parser Clippy and workspace formatting pass.Focused regression command:
cargo test -p tool-parser --test tool_parser_minimax_m2The nested-object test uses both a whole input and seven-byte chunks. The second regression checks a string containing
}, followed by an empty invoke in the same wrapper, and repeats afterreset(). The existing scalar, empty-call, parallel-call, and partial-tag tests remain applicable. Non-streaming parsing and parameter conversion are unchanged. A separate, pre-existing issue can mix arguments when multiple invokes arrive in one chunk; invocation scoping is outside this JSON-closure fix.Selected command output from the independent patch
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 #2744) applied together. The parser checks above tested this PR independently.
cargo +nightly fmt --all -- --checkcargo clippy --locked --all-targets --all-features -- -D warningscargo test --lockedWASM 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
19f33a5495e51e89618bfbb13ce6f8570c393fbd:Claude Respondandgo-bindings-benchmarkwere skipped by the workflow; they are not counted as passing tests.Checklist
cargo +nightly fmt --all -- --checkpasses