Muse Glimmer: fix detection of tool calls after EOM - #26879
Conversation
Muse Glimmer routinely answers the user and calls a tool in a single
generation. The template terminates a message with <|eom|> when more
messages follow in the same turn and <|eot|> only at the end of the turn,
so the answer is closed by <|eom|> and the call opens a fresh header:
<prose><|eom|><|start|>assistant to=<tool><|message|><atem:function_calls>...
The final-message rule read content with until("<|eot|>"), which assumed the
user-facing message is always last. There is no <|eot|> before the call, so
content ran to the end of the turn, absorbed the markup, and no tool_calls
were emitted - the tool never ran. On a tau2-bench telecom run this hit 43
turns across 19 of 114 tasks.
Stop the answer at <|eom|> and parse what follows as tool calls.
Adds models/templates/muse-glimmer.jinja and four parser tests: a plain
answer, the <|eom|> junction, markup quoted in an answer staying content,
and tool markup inside the to=self channel staying reasoning.
|
Hi @ruanslv, thanks for your contribution! Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:
Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below. |
|
The only thing missing is a trigger for the malformed tool call that omits the header/recipient. Adding |
|
@ruanslv got internet at the hotel right now so can take a peek :) |
| {{- '</atem:invoke>\n</atem:function_calls>' -}} | ||
| {%- endmacro -%} | ||
| {%- macro render_reasoning() -%} | ||
| {%- set rs = reasoning_strength if reasoning_strength is defined and reasoning_strength else 'high' -%} |
There was a problem hiding this comment.
OAI uses reasoning_effort - wondering if that would be a better name? also for consistency with other templates e.g. deepseek and gpt-oss-120b
There was a problem hiding this comment.
The model has been trained explicitly with "reasoning strength", not "reasoning effort". If people actually say reasoning effort the behavior is undefined, I wanted to keep as aligned with training as possible
There was a problem hiding this comment.
I don't see how it would be harmful to set rs to reasoning_strength if that is defined, and if not, then set it to reasoning_effort if that is defined. It would certainly improve the consistency and ease of use with other models, and the model never sees the name of the chat template variables.
The real problem (in my opinion) is that llama.cpp still has no native concept of reasoning levels or how to map them to what the model is expecting. I don't think users should have to control this with a raw chat template variable. The usability problem is getting worse all the time as more models release with various reasoning levels.
But, my opinion on this stuff doesn't really matter.
There was a problem hiding this comment.
@coder543 this is my idea:
the jinja template for each model would be responsible for mapping OAI resoning_effort to the model's accepted values / equivalent concept.
There was a problem hiding this comment.
the jinja template for each model would be responsible for mapping OAI resoning_effort to the model's accepted values / equivalent concept
Just to confirm, you mean the jinja template that lives in HuggingFace, right? That is usually a standalone model artifact, that is not really aware of llama.cpp innerworks at all.
I think expecting model templates to even be aware of some OpenAI flag compatibility doesn't sound great. Isn't it better if this translation layer lives in llama.cpp directly? If llama.cpp wants a unified "reasoning_effort" flag that's fine, but then there's some code that maps that to each individual template (so in my case reasoning_effort->reasoning_strength) internally before reaching jinja.
Otherwise you would be forcing jinja files in HF to somehow have to work around serving logic that is not model specific
Let me know if Im missing anything
There was a problem hiding this comment.
I thought we were talking about a jinja file that lives in this repo, not huggingface? And it's not really OpenAI compatibility. Other models like DeepSeek V4, Kimi K3, and Hy3 use the same config variable name of reasoning_effort. reasoning_effort has been the standard variable name for models with named reasoning levels across all open weight models in the industry that I'm aware of until this model introduced reasoning_strength.
I see it as the same thing as standard role names like assistant and user. There's technically nothing that requires models to use those in the jinja template, but breaking compatibility there would make integration with standard inference runtimes harder. Unless there is a good reason for changing reasoning_effort to reasoning_strength, then I wish companies wouldn't. It only makes things harder for people looking to integrate with this model. There are surely tons of clients that already hard code reasoning_effort which will have to be changed unless llama.cpp (and vLLM, and SGLang, and every other runtime) adds a compatibility shim just for this model.
But again, my opinion doesn't matter here. I'm just sharing some of my perspective as someone who has closely tracked open weight models for years. The addition of reasoning levels in this model is awesome, and I appreciate that, although I haven't found any benchmarks that show how much the reasoning levels help.
There was a problem hiding this comment.
The template should be the one from the HF repository. The only exceptions are models that don't ship with a template.
I thought it would be better to keep this out. Two reasons:
|
The parser explicitly handles |
pwilkin
left a comment
There was a problem hiding this comment.
I'd say remove the p.optional from the start marker and this should be gtg.
| return p.zero_or_more(start + analysis) + start + tool_calls; | ||
| } | ||
| return p.zero_or_more(start + analysis) + start + (tool_calls | final_msg); | ||
| auto trailing_calls = p.optional(p.literal("<|eom|>") + p.optional(start) + tool_calls); |
There was a problem hiding this comment.
I believe what @aldehir means is that this should be:
| auto trailing_calls = p.optional(p.literal("<|eom|>") + p.optional(start) + tool_calls); | |
| auto trailing_calls = p.optional(p.literal("<|eom|>") + start + tool_calls); |
instead, since otherwise you allow a tool call without the start marker in the trailing tool call section (which is what you're saying you want to avoid, so I'd just make the start obligatory here since otherwise you are enabling the parsing of calls of the shape:
[... prefix here ...]<|eom|>to=fun<|message|>...There was a problem hiding this comment.
Done, tks! EOM was already required but missed start, thanks for the suggestion
|
I apologize, I was under the impression we were still trying to handle case 2 optional semantics from the original PR #26849, given the trailing parser definition. If we're not, then the PR LGTM. Thank you for the contribution. |
…6879) * chat : fix muse-glimmer swallowing a trailing tool call into content Muse Glimmer routinely answers the user and calls a tool in a single generation. The template terminates a message with <|eom|> when more messages follow in the same turn and <|eot|> only at the end of the turn, so the answer is closed by <|eom|> and the call opens a fresh header: <prose><|eom|><|start|>assistant to=<tool><|message|><atem:function_calls>... The final-message rule read content with until("<|eot|>"), which assumed the user-facing message is always last. There is no <|eot|> before the call, so content ran to the end of the turn, absorbed the markup, and no tool_calls were emitted - the tool never ran. On a tau2-bench telecom run this hit 43 turns across 19 of 114 tasks. Stop the answer at <|eom|> and parse what follows as tool calls. Adds models/templates/muse-glimmer.jinja and four parser tests: a plain answer, the <|eom|> junction, markup quoted in an answer staying content, and tool markup inside the to=self channel staying reasoning. * address comment (cherry picked from commit 0b1bad1)
Signed-off-by: Gabe Goodhart <ghart@us.ibm.com> * origin/master: (383 commits) cmake : introduce semantic versioning (ggml-org#26839) gguf : harden loader against malformed tensor dims and metadata types (ggml-org#25596) kleidiai: Add runtime feature detection mechanism for aarch64/kleidiai (ggml-org#26076) model : disallow integer dflash sliding_window_pattern (ggml-org#26900) sync : ggml cmake : add config version support (ggml/1582) server : support slot save/restore with media inputs (ggml-org#26640) ui: add read_media tool (ggml-org#25877) opencl: default FA c8 cluster width to 16 on X1E (ggml-org#26433) tests : update speculative params (ggml-org#26925) vulkan: add TQ2_0 (ternary) support (ggml-org#25850) wavtokenizer-dec : bound posnet/convnext block_count against n_layer_all (ggml-org#26892) convert : handle per_layer_config in Gemma4 (transformers 5.15) (ggml-org#26882) opencl: use flat mv q5_k when weight exceeds image1d_buffer_t limit (ggml-org#26880) chat : fix muse-glimmer detection of tool calls after EOM (ggml-org#26879) ci : add missing release check (ggml-org#26923) CUDA: only disable CUDA graphs when mul_mat_id actually needs a stream sync (ggml-org#26802) cuda : add warp-per-row wkv7 kernel for single-token decode (ggml-org#26111) spec : update speculative-simple (ggml-org#26904) chat : tighten bare function parsing for Qwen models (ggml-org#26793) ...
…6879) * chat : fix muse-glimmer swallowing a trailing tool call into content Muse Glimmer routinely answers the user and calls a tool in a single generation. The template terminates a message with <|eom|> when more messages follow in the same turn and <|eot|> only at the end of the turn, so the answer is closed by <|eom|> and the call opens a fresh header: <prose><|eom|><|start|>assistant to=<tool><|message|><atem:function_calls>... The final-message rule read content with until("<|eot|>"), which assumed the user-facing message is always last. There is no <|eot|> before the call, so content ran to the end of the turn, absorbed the markup, and no tool_calls were emitted - the tool never ran. On a tau2-bench telecom run this hit 43 turns across 19 of 114 tasks. Stop the answer at <|eom|> and parse what follows as tool calls. Adds models/templates/muse-glimmer.jinja and four parser tests: a plain answer, the <|eom|> junction, markup quoted in an answer staying content, and tool markup inside the to=self channel staying reasoning. * address comment
…6879) * chat : fix muse-glimmer swallowing a trailing tool call into content Muse Glimmer routinely answers the user and calls a tool in a single generation. The template terminates a message with <|eom|> when more messages follow in the same turn and <|eot|> only at the end of the turn, so the answer is closed by <|eom|> and the call opens a fresh header: <prose><|eom|><|start|>assistant to=<tool><|message|><atem:function_calls>... The final-message rule read content with until("<|eot|>"), which assumed the user-facing message is always last. There is no <|eot|> before the call, so content ran to the end of the turn, absorbed the markup, and no tool_calls were emitted - the tool never ran. On a tau2-bench telecom run this hit 43 turns across 19 of 114 tasks. Stop the answer at <|eom|> and parse what follows as tool calls. Adds models/templates/muse-glimmer.jinja and four parser tests: a plain answer, the <|eom|> junction, markup quoted in an answer staying content, and tool markup inside the to=self channel staying reasoning. * address comment
Overview
Muse Glimmer sometimes answers the user and calls a tool in a single turn:
The previous parser read content with until("<|eot|>"), which assumed the user-facing message was always last. This caused the tool call to go undetected.
Added some tests based on the latest template
Requirements