Skip to content

[Frontend]: minimax_m2 supports structural_tag#32232

Draft
chaunceyjiang wants to merge 3 commits intovllm-project:mainfrom
chaunceyjiang:minimax_structural_tag
Draft

[Frontend]: minimax_m2 supports structural_tag#32232
chaunceyjiang wants to merge 3 commits intovllm-project:mainfrom
chaunceyjiang:minimax_structural_tag

Conversation

@chaunceyjiang
Copy link
Copy Markdown
Collaborator

@chaunceyjiang chaunceyjiang commented Jan 13, 2026

Purpose

follow up #32142

Test Plan

Test Result


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
@mergify
Copy link
Copy Markdown

mergify bot commented Jan 13, 2026

Documentation preview: https://vllm--32232.org.readthedocs.build/en/32232/

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds support for structural_tag in the minimax_m2 tool parser. This is a nice enhancement, enabling more robust tool calling for MiniMax M2 models by leveraging structured outputs. The changes involve implementing the prepare_structured_tags method to dynamically construct the necessary schema and updating an example file for testing.

My review has identified several instances of leftover debug print statements across different files. These should be removed to maintain clean logs. More critically, I've found an issue in vllm/tool_parsers/minimax_m2_tool_parser.py where the extract_tool_calls method for non-streaming responses has been effectively disabled, which could break existing functionality. Please address these points.

Comment on lines +390 to +392
return ExtractedToolCallInformation(
tools_called=False, tool_calls=[], content=model_output
)
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.

critical

This early return effectively disables the entire extract_tool_calls method, causing non-streaming tool call extraction to fail. The method will always return an empty list of tool calls, regardless of the model output. This appears to be a critical issue that breaks existing functionality.

Comment on lines +816 to +817
print("Structured outputs params:")
print(self.structured_outputs)
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.

high

These appear to be debug print statements. They should be removed before merging to avoid polluting production logs.

request.response_format = StructuralTagResponseFormat(
type="structural_tag", format=structured_tags["format"]
)
print(structured_tags)
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.

high

This print statement seems to be for debugging purposes. Please remove it before merging to keep the logs clean.

Comment on lines +116 to +117
print("1*1" * 20)
print(s_tag)
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.

high

These print statements appear to be for debugging. Please remove them to avoid unnecessary output in the logs.

Comment on lines +365 to +366
print("*" * 20)
print(s_tag)
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.

high

These print statements look like they were added for debugging. They should be removed before this pull request is merged.

Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
Signed-off-by: chaunceyjiang <chaunceyjiang@gmail.com>
# "type": "const_string",
# "value": "...",
# }, # debug
"content": {"type": "any_text"},
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@mergify
Copy link
Copy Markdown

mergify bot commented Jan 13, 2026

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @chaunceyjiang.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify bot added the needs-rebase label Jan 13, 2026
scottgl9 added a commit to scottgl9/vllm that referenced this pull request Mar 2, 2026
Mark PR vllm-project#33303 as applied. Add additional MiniMax-specific PRs:
- vllm-project#34863: compressed-tensors FP8 scale propagation
- vllm-project#32232: structural_tag support
- vllm-project#35358: reasoning-end detection fix
scottgl9 added a commit to scottgl9/vllm that referenced this pull request Mar 4, 2026
Mark PR vllm-project#33303 as applied. Add additional MiniMax-specific PRs:
- vllm-project#34863: compressed-tensors FP8 scale propagation
- vllm-project#32232: structural_tag support
- vllm-project#35358: reasoning-end detection fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status
Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant