[data] fix MiniMax tool_extractor to return content when no tool call parses#10587
[data] fix MiniMax tool_extractor to return content when no tool call parses#10587he-yufeng wants to merge 1 commit into
Conversation
… parses MiniMaxM1/M2 tool_extractor returned an empty list when the wrapper tag was present but no valid call parsed (M1 skips bad-JSON lines, M2 finds no <invoke>), dropping the original content. Return content in that case, matching SeedToolUtils (hiyouga#10408), Qwen35 and LFM2. Signed-off-by: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
This pull request updates the tool extractors for MiniMaxM1 and MiniMaxM2 in src/llamafactory/data/tool_utils.py to return the original content instead of an empty list when no valid tool calls are parsed. Additionally, corresponding unit tests have been added in tests/data/test_tool_utils.py to verify both successful parsing and fallback behavior. No review comments were provided, and the changes look correct and well-tested.
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.
|
Closing this in favor of #10599, which is the same fix (return the raw content when the wrapper tag is present but nothing parses inside it) on a fresh branch and is the one currently under review. No point keeping two identical PRs open. Thanks! |
What does this PR do?
MiniMaxM1ToolUtils.tool_extractorandMiniMaxM2ToolUtils.tool_extractorreturn an empty list when the tool-call wrapper tag is present but no valid call parses out of it. M1 skips lines that failjson.loads(viacontinue), and M2 finds no<invoke>block, soresultsends up empty. An empty list reads downstream as "a tool call with zero functions" rather than "plain content", so the original assistant text is dropped.This returns the original
contentin that case, matchingSeedToolUtils(fixed in #10408),Qwen35ToolUtilsandLFM2ToolUtils, which already usereturn results if results else content.DefaultToolUtils,QwenToolUtilsandGemma4ToolUtilsalready handle it (they return content on a parse failure, or append one call per regex match), so only the two MiniMax extractors needed the change.Added
tests/data/test_tool_utils.pycovering both the empty case (tag present, nothing parses, returns content) and the valid case (a real call still returns theFunctionCalllist).Before submitting