Skip to content

chat : add the MiniCPM5-2B chat template - #28595

Closed
cyxu0401 wants to merge 1 commit into
ggml-org:masterfrom
cyxu0401:minicpm5-2b-chat-template
Closed

cyxu0401 wants to merge 1 commit into
ggml-org:masterfrom
cyxu0401:minicpm5-2b-chat-template

Conversation

@cyxu0401

@cyxu0401 cyxu0401 commented Sep 8, 2026

Copy link
Copy Markdown

Follow-up to #23384 (MiniCPM5-1B), now that
openbmb/MiniCPM5-2B is public.

Adds models/templates/openbmb-MiniCPM5-2B.jinja (fetched verbatim from the
HF repo) and covers it in tests/test-chat.cpp.

2B's chat template is not the same as 1B's. The tool-call and reasoning
syntax is identical, so 2B is already picked up correctly by the existing
detection in common/chat.cpp and resolves to the same chat format. What
differs is how past assistant turns are rendered:

  • 1B drops the reasoning content of previous assistant turns entirely;
  • 2B keeps it, and when a past turn carries no reasoning it emits an empty
    <think>\n\n</think> block instead.

Nothing covered either behaviour. Since the format is selected by substring
matching on the template text, it is easy to break by accident, so the tests
assert both halves: that 2B still resolves to the shared format, and that the
generation prompt diverges from 1B in the way described above. A negative
assertion was added on the 1B side so the two cannot silently converge.

This is test-only coverage plus a fixture — no runtime code is touched, and
models/templates/ is not read at runtime.

Independent of the tokenizer-note PR; either can land first.

Testing

test-chat passes. I checked the new assertions actually execute by
temporarily inverting one and confirming it fails rather than passing
vacuously.

openbmb/MiniCPM5-2B ships a different chat template from MiniCPM5-1B: it
renders the reasoning of past assistant turns instead of dropping them,
and falls back to an empty <think> block when a past turn carries no
reasoning. Both models keep the same generation prompt, tool-call syntax
and reasoning tags, so 2B already resolves to the MiniCPM5 chat format and
needs no parser change.

Add the template and cover the rendering difference from both sides, so
neither model can drift into the other's behaviour unnoticed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@cyxu0401
cyxu0401 requested a review from pwilkin as a code owner September 8, 2026 06:53
@ggml-gh-bot

ggml-gh-bot Bot commented Sep 8, 2026

Copy link
Copy Markdown

Hi @cyxu0401, thanks for your contribution!

Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:

  • PR Template not respected: Please respect the template when creating a new pull request. Make sure to fill out all required sections.

  • Multiple open PRs from a new contributor: We limit new contributors (those without a previously merged PR) to 1 open PR at a time. You currently have 2 open PRs.

  • AI-generated content: While code is allowed to be generated by AI, please write the PR description and commit messages on your own without the help of AI.


Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below.

@ggml-gh-bot ggml-gh-bot Bot added the draft PR will be changed to draft by github-actions bot label Sep 8, 2026
@github-actions github-actions Bot added the testing Everything test related label Sep 8, 2026
@github-actions
github-actions Bot marked this pull request as draft September 8, 2026 07:00
@github-actions github-actions Bot removed the draft PR will be changed to draft by github-actions bot label Sep 8, 2026
@cyxu0401

cyxu0401 commented Sep 8, 2026

Copy link
Copy Markdown
Author

Closing this. MiniCPM5-2B works with llama.cpp as-is: its tokenizer is byte-identical to 1B, and its chat template ships inside the GGUF, so no upstream change is required. This was only regression coverage, not a fix, and it isn't worth maintainer time right now.

@cyxu0401 cyxu0401 closed this Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant