Skip to content

[Fix] MiniCPM-V 4.6: skip legacy minicpmv conv template, fall back to… #25053

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
tc-mb wants to merge 1 commit into from
Closed

[Fix] MiniCPM-V 4.6: skip legacy minicpmv conv template, fall back to… #25053

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 8 additions & 1 deletion python/sglang/srt/parser/conversation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1150,10 +1150,17 @@ def match_qwen_chat_ml(model_path: str):

@register_conv_template_matching_function
def match_minicpm(model_path: str):
# MiniCPM-V 4.6 (and any future versions) ship a real HF jinja chat
# template that uses <|image_pad|> / <|video_pad|> instead of the legacy
# (<image>./</image>) placeholder. Returning None here lets the
# TemplateManager fall back to the HF tokenizer/processor template,
# which keeps the prompt aligned with the multimodal processor.
model_type = get_model_type(model_path)
if model_type == "minicpmv4_6":
return None
Comment on lines +1153 to +1160
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 12, 2026

Choose a reason for hiding this comment

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

medium

The comment mentions that this logic should apply to "any future versions", but the implementation specifically checks only for minicpmv4_6. If future versions (e.g., minicpmv4_7) or variants like minicpmo4_6 are released with HF jinja templates, they will fall through to the regex match on line 1161 and incorrectly return the legacy template. Consider using a more generic check (e.g., checking for a version prefix or a list of types) to align with the comment's intent and ensure future-proofing.

match = re.search(r"minicpm-(v|o)", model_path, re.IGNORECASE)
if match:
return f"minicpm{match.group(1).lower()}"
model_type = get_model_type(model_path)
return MODEL_TYPE_TO_TEMPLATE.get(model_type)


Expand Down
Loading