fix(common): tolerate dangling Gemma 4 channel prefix after tool responses#25100
fix(common): tolerate dangling Gemma 4 channel prefix after tool responses#25100rodboev wants to merge 1 commit into
Conversation
aldehir
left a comment
There was a problem hiding this comment.
Thanks for the PR, but we do not indiscriminately apply fixes to code that impacts all models. Gemma 4 has a dedicated parser which should encode this edge case, iff it exists with a well-behaved client.
First and foremost, due diligence should be done to ensure the reporter is using a proper client that sends back reasoning content. The absence of reasoning content from tool calls leads to model hallucinations and is the source of many parsing problems.
|
That makes sense. I see you already asked, so we we should have more details about the tool responses soon. In the meantime I'm moving the fix into the dedicated Gemma 4 grammar rather than the shared parser path. |
f409532 to
3106c25
Compare
|
This is probably from a bug in the chat template, and it should be noted that there's a slew of upcoming fixes in https://huggingface.co/google/gemma-4-31B-it/discussions/118. I believe "fix: restore model turn + thinking cue after tool responses" is the relevant fix to this particular issue. |
Overview
Gemma 4 tool-calling turns can end on a bare
<|channel>prefix with optional trailing whitespace. On currentmaster, that fragment is treated as invalidpeg-gemma4output, sollama-serverreturns a 500 with:This change keeps the fix in the shared Gemma 4 parse path. It tolerates only the exact final dangling
<|channel>prefix with trailing whitespace, preserves any already parsed assistant state, keeps the existing tolerated trailing marker cases, and still rejects malformed non-whitespace tails after parsed prefixes.Closes #25072
Additional information
The live issue log shows the final unparsed fragment as:
The non-tool Gemma 4 path already has narrow tolerance for trailing channel-marker noise. The tool-enabled path did not have an equivalent final-parse escape hatch, which is why the shared parser escalated this exact fragment into a 500.
Focused validation for the implemented diff stayed on the parser surfaces:
Requirements