feat(tool_parser,reasoning_parser): add MiniMax-M3 tool and reasoning parsers#1815
feat(tool_parser,reasoning_parser): add MiniMax-M3 tool and reasoning parsers#1815slin1237 wants to merge 1 commit into
Conversation
… parsers Tool parser handles M3's namespaced framing (`]<]minimax[>[` prefix before each structural tag), recursive parameter elements (objects/arrays), schema-type coercion, XML entity decoding, and streaming. Reasoning parser uses the `<mm:think>`/`</mm:think>` delimiters. Both register M3 model patterns ahead of the broad `minimax` (M2) match so M3 IDs are not captured by the M2 parser. Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
github-actions
Bot
added
tests
📝 WalkthroughWalkthroughAdds ChangesMiniMax M3 Parser Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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: f02fc04d58
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| registry.map_model("minimax*", "minimax_m2"); | ||
| registry.map_model("MiniMax*", "minimax_m2"); | ||
| // M3 IDs route to the M3 parser via longer (more specific) patterns. | ||
| registry.map_model("minimax-m3*", "minimax_m3"); |
There was a problem hiding this comment.
When the model ID uses the mm-m3 alias, the tool parser factory has no matching prefix and it also does not start with the broader minimax*/MiniMax* patterns, so auto-detection falls back to passthrough and M3 tool-call blocks are never parsed. The reasoning factory in this same change routes mm-m3 to minimax_m3, so add an mm-m3* mapping alongside these M3 tool mappings.
Useful? React with 👍 / 👎.
|
|
||
| let mut map: Map<String, Value> = Map::new(); | ||
| for (name, child) in children { | ||
| let child_json = Self::value_to_json(child, None); |
There was a problem hiding this comment.
For nested object/array parameters, child values are always converted with declared_type set to None here, so nested schema information is discarded. In a tool schema such as shipping.zip declared as a string, <zip>00123</zip> will be inferred as number 123 instead of preserving the string; array item fields have the same issue in the array branch above. Pass the nested child/item schema through the recursion to avoid silently corrupting nested tool arguments.
Useful? React with 👍 / 👎.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // M3 IDs route to the M3 parser via longer (more specific) patterns. | ||
| registry.map_model("minimax-m3*", "minimax_m3"); | ||
| registry.map_model("MiniMax-M3*", "minimax_m3"); | ||
| registry.map_model("MiniMaxAI/MiniMax-M3*", "minimax_m3"); |
There was a problem hiding this comment.
🟡 Nit: The reasoning parser registers mm-m3 as a pattern (line 204 of reasoning_parser/src/factory.rs), but there's no corresponding mm-m3* glob here. A model ID like "mm-m3-chat" would get M3 reasoning parsing but fall through to the default (passthrough) tool parser, since none of minimax-m3* / MiniMax-M3* / MiniMaxAI/MiniMax-M3* match the mm- prefix.
This mirrors an existing gap with M2 (mm-m2 in reasoning, no mm-m2* in tool), but the PR could close it for M3:
| registry.map_model("MiniMaxAI/MiniMax-M3*", "minimax_m3"); | |
| registry.map_model("MiniMaxAI/MiniMax-M3*", "minimax_m3"); | |
| registry.map_model("mm-m3*", "minimax_m3"); |
| // M3 IDs route to the M3 parser via longer (more specific) patterns. | ||
| registry.map_model("minimax-m3*", "minimax_m3"); |
There was a problem hiding this comment.
🟡 Nit: The reasoning parser factory has test_minimax_m3_model verifying that M3 model IDs (e.g. "MiniMaxAI/MiniMax-M3", "mm-m3-chat") route to the M3 parser and not the broad M2 one. Consider adding an equivalent test for the tool parser factory — the mechanism is different here (longest-glob-wins vs first-substring-wins), so a test would guard against regressions if the patterns are ever reordered.
There was a problem hiding this comment.
Clean, well-structured PR with thorough test coverage (30 cases for the tool parser, 7 for reasoning). The critical pattern-ordering concern is handled correctly in both factories — reasoning uses first-substring-match with M3 before M2, tool uses longest-glob-wins. Two minor nits posted inline (missing mm-m3* tool parser glob, no factory routing test for tool parser). No bugs found.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for the MiniMax M3 model by implementing specific reasoning and tool parsers. In reasoning_parser, a new MinimaxM3Parser is added to handle reasoning blocks enclosed in <mm:think> tags. In tool_parser, a corresponding MinimaxM3Parser is introduced to parse tool calls framed with the ]<]minimax[>[ namespace marker, supporting nested parameters and multiple invokes. Model routing and factory registrations are updated in both crates to correctly map M3 model IDs, and comprehensive unit and integration tests are added to verify the new parsers. There are no review comments to address, and I have no additional 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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/tool_parser/src/factory.rs`:
- Around line 406-412: The `mm-m3*` model pattern is missing from the model
mappings in the registry, which causes mm-m3 IDs to be misrouted away from the
minimax_m3 parser. Add a new registry.map_model call that maps the "mm-m3*"
pattern to "minimax_m3", placing it alongside the existing minimax-m3,
MiniMax-M3, and MiniMaxAI/MiniMax-M3 mappings that are already routed to the M3
parser.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d796d76d-17e5-4dbf-ba8f-596e95b108d6
📒 Files selected for processing (9)
crates/reasoning_parser/src/factory.rscrates/reasoning_parser/src/lib.rscrates/reasoning_parser/src/parsers/minimax_m3.rscrates/reasoning_parser/src/parsers/mod.rscrates/tool_parser/src/factory.rscrates/tool_parser/src/lib.rscrates/tool_parser/src/parsers/minimax_m3.rscrates/tool_parser/src/parsers/mod.rscrates/tool_parser/tests/tool_parser_minimax_m3.rs
| // General MiniMax IDs default to the M2 parser. | ||
| registry.map_model("minimax*", "minimax_m2"); | ||
| registry.map_model("MiniMax*", "minimax_m2"); | ||
| // M3 IDs route to the M3 parser via longer (more specific) patterns. | ||
| registry.map_model("minimax-m3*", "minimax_m3"); | ||
| registry.map_model("MiniMax-M3*", "minimax_m3"); | ||
| registry.map_model("MiniMaxAI/MiniMax-M3*", "minimax_m3"); |
There was a problem hiding this comment.
Add missing mm-m3* model mapping to minimax_m3.
Line 410-Line 412 add M3 routes, but mm-m3* is still not mapped despite the PR contract. That can misroute mm-m3 IDs away from the M3 parser.
Proposed fix
// M3 IDs route to the M3 parser via longer (more specific) patterns.
registry.map_model("minimax-m3*", "minimax_m3");
+ registry.map_model("mm-m3*", "minimax_m3");
registry.map_model("MiniMax-M3*", "minimax_m3");
registry.map_model("MiniMaxAI/MiniMax-M3*", "minimax_m3");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // General MiniMax IDs default to the M2 parser. | |
| registry.map_model("minimax*", "minimax_m2"); | |
| registry.map_model("MiniMax*", "minimax_m2"); | |
| // M3 IDs route to the M3 parser via longer (more specific) patterns. | |
| registry.map_model("minimax-m3*", "minimax_m3"); | |
| registry.map_model("MiniMax-M3*", "minimax_m3"); | |
| registry.map_model("MiniMaxAI/MiniMax-M3*", "minimax_m3"); | |
| // General MiniMax IDs default to the M2 parser. | |
| registry.map_model("minimax*", "minimax_m2"); | |
| registry.map_model("MiniMax*", "minimax_m2"); | |
| // M3 IDs route to the M3 parser via longer (more specific) patterns. | |
| registry.map_model("minimax-m3*", "minimax_m3"); | |
| registry.map_model("mm-m3*", "minimax_m3"); | |
| registry.map_model("MiniMax-M3*", "minimax_m3"); | |
| registry.map_model("MiniMaxAI/MiniMax-M3*", "minimax_m3"); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/tool_parser/src/factory.rs` around lines 406 - 412, The `mm-m3*` model
pattern is missing from the model mappings in the registry, which causes mm-m3
IDs to be misrouted away from the minimax_m3 parser. Add a new
registry.map_model call that maps the "mm-m3*" pattern to "minimax_m3", placing
it alongside the existing minimax-m3, MiniMax-M3, and MiniMaxAI/MiniMax-M3
mappings that are already routed to the M3 parser.
|
This pull request has been automatically marked as stale because it has not had any activity within 14 days. It will be automatically closed if no further activity occurs within 16 days. Leave a comment if you feel this pull request should remain open. Thank you! |
1 participant
Description
Problem
SMG has no parser for MiniMax-M3, so M3 tool calls and
<mm:think>reasoning content are not extracted.Solution
Add MiniMax-M3 tool and reasoning parsers and register them so M3 model IDs route to them — ordered ahead of the broad MiniMax/M2 patterns so M3 IDs aren't captured by the M2 parser.
Changes
crates/tool_parser/src/parsers/minimax_m3.rs): M3 namespaced framing (]<]minimax[>[before each structural tag), recursive parameter elements (nested objects/arrays), schema-type coercion with JSON-type inference fallback, XML entity decoding, mixed-text preservation, and streaming (block-level invoke emission with partial-token hold-back vialongest_partial_suffix).crates/reasoning_parser/src/parsers/minimax_m3.rs):<mm:think>/</mm:think>delimiters viaBaseReasoningParser(always_in_reasoning=false).minimax_m3and routeminimax-m3/MiniMax-M3*/mm-m3to it, before the broadminimax(M2) match.Test Plan
cargo +nightly fmt --all— cleancargo clippy -p tool-parser -p reasoning-parser --all-targets -- -D warnings— clean (default features)cargo test -p tool-parser -p reasoning-parser— all pass, incl. the newtool_parser_minimax_m3suite (30 cases: single/multiple/parallel calls, streaming at chunk and char boundaries, nested objects/arrays, schema coercion, XML entities, malformed/incomplete input, reset) and the reasoning-parser unit tests.Checklist
cargo +nightly fmtpassescargo clippy --all-targets -- -D warningspasses (parser crates; default features — workspace--all-featurespulls opencv, unrelated)Summary by CodeRabbit
New Features
Tests