Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/llamafactory/data/tool_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ def tool_extractor(content: str) -> Union[str, list["FunctionCall"]]:
except json.JSONDecodeError:
continue

return results
return results if results else content

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While this fallback improvement is great, MiniMaxM1ToolUtils.tool_extractor is vulnerable to runtime crashes if the content inside <tool_calls> is valid JSON but does not match the expected structure (e.g., it is missing the "name" or "arguments" keys, or is not a dictionary). For example, if the model outputs <tool_calls> {"invalid_key": "value"} </tool_calls>, json.loads will succeed, but accessing tool_call["name"] or tool_call["arguments"] will raise a KeyError (or TypeError if it's a list/string), crashing the extraction. Consider catching (json.JSONDecodeError, KeyError, TypeError) in the try-except block to make the parser more robust.



class MiniMaxM2ToolUtils(ToolUtils):
Expand Down Expand Up @@ -548,7 +548,7 @@ def tool_extractor(content: str) -> Union[str, list["FunctionCall"]]:

results.append(FunctionCall(func_name.strip(), json.dumps(args_dict, ensure_ascii=False)))

return results
return results if results else content


class MistralToolUtils(ToolUtils):
Expand Down
45 changes: 45 additions & 0 deletions tests/data/test_tool_utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# Copyright 2025 the LlamaFactory team.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from llamafactory.data.tool_utils import MiniMaxM1ToolUtils, MiniMaxM2ToolUtils


def test_minimax_m1_parses_valid_tool_call():
content = '<tool_calls>\n{"name": "get_weather", "arguments": {"city": "NYC"}}\n</tool_calls>'
result = MiniMaxM1ToolUtils.tool_extractor(content)
assert isinstance(result, list) and len(result) == 1
assert result[0].name == "get_weather"


def test_minimax_m1_returns_content_when_no_valid_tool_call():
# Wrapper tag is present but its body is not valid JSON, so nothing parses.
# The extractor should return the original content, not an empty list.
content = "<tool_calls>\nthis is not json\n</tool_calls>"
assert MiniMaxM1ToolUtils.tool_extractor(content) == content


def test_minimax_m1_empty_wrapper_returns_content():
content = "<tool_calls>\n\n</tool_calls>"
assert MiniMaxM1ToolUtils.tool_extractor(content) == content


def test_minimax_m2_returns_content_when_no_valid_tool_call():
# Wrapper tag is present but there is no well-formed <invoke> block.
content = "<minimax:tool_call>\nno invoke block here\n</minimax:tool_call>"
assert MiniMaxM2ToolUtils.tool_extractor(content) == content


def test_minimax_m2_empty_wrapper_returns_content():
content = "<minimax:tool_call>\n\n</minimax:tool_call>"
assert MiniMaxM2ToolUtils.tool_extractor(content) == content