Repository navigation
fix(parsers): re-jail a complete start marker in trailing content - #408
Merged
keivenchang merged 1 commit intoOct 15, 2026
Merged
Conversation
When a multi-token delta closes one tool call and also carries the full start marker of the next, handle_trailing_content only handled a partial marker match. A complete match fell through to the gemma4 call-prefix split, which emitted the start marker as content and held only the "ca" tail, so the marker leaked into message.content and, depending on the chunking, the next call was lost. Handle MatchResult::Complete the way process_content does: emit any prose before the marker and re-jail from the marker. Refs ai-dynamo/dynamo#16383 Signed-off-by: GokayAI <[email protected]>
Contributor
|
/ok to test e4f3f2f |
keivenchang
reviewed
Oct 15, 2026
keivenchang
left a comment
Contributor
There was a problem hiding this comment.
hey Gokay — no confirmed P0/P1 findings at e4f3f2f. Local validation passed 974 library tests and 83 jail tests; a separate Gemma4 sweep passed all 9,316 three-chunk partitions with exact arguments, call indices, call order, and empty content. An independent source audit found no additional defects. Current-head Rust and conformance CI are running; full lifecycle/logprob qualification remains incomplete.
|
📊 Conformance matrix rendered — view in CI summary |
keivenchang
approved these changes
Oct 15, 2026
keivenchang
left a comment
Contributor
There was a problem hiding this comment.
approved — thanks Gokay, this fixes Gemma4 call re-entry across chunk boundaries. nice work.
keivenchang
added a commit
that referenced
this pull request
Oct 23, 2026
Signed-off-by: Keiven Chang <[email protected]>
keivenchang
added a commit
that referenced
this pull request
Oct 23, 2026
Signed-off-by: Keiven Chang <[email protected]> Co-authored-by: Keiven Chang <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes ai-dynamo/dynamo#16383 (for the gemma4 tool-call jail; see Scope below).
Problem
With parallel tool calls and multi-token deltas (for example under speculative decoding), one delta can close call A and carry the full start marker of call B:
After call A completes, the rest of that delta (
<|tool_call>ca) goes tohandle_trailing_content. That function only handledMatchResult::Partial. AMatchResult::Completefell through tosplit_partial_tool_call_start, and the gemma4 call-prefix split tookcaas a partialcall:prefix. It emitted<|tool_call>as content and held onlyca. Onmainthe stream above returns both calls butcontentis"<|tool_call>". With other chunkings, a whole call ends up incontentand is missing fromtool_calls. For example,["<", "|tool_call>call:f{…}<tool_call|><|tool_call>c", "all:g{…}<tool_call|><|tool_call>call:h{…}<tool_call|>"]returns onlyfandh.Fix
handle_trailing_contentnow handlesMatchResult::Completethe same wayprocess_contentdoes. Prose before the marker is emitted as trailing content, unless it carries harmony protocol, in which case it is jailed with the marker, as inprocess_content. The jail then restarts from the marker. ThePartialandNonepaths are unchanged.Tests
test_gemma4_next_call_start_marker_in_trailing_content_is_jailedinparsers/v1/tests/jail.rs. It runs the exact delta sequence from the issue, and then every 3-chunk split of a 3-call gemma4 stream where the middle chunk spans the end of callgand the start of callh. Each run must returnf,g,hwith empty content. It fails onmain(left: "<|tool_call>") and passes with the fix.cargo test -p dynamo-parsers: all suites pass (974 lib tests, 84 injail).cargo clippy -p dynamo-parsers --all-targets -- -D warningsandcargo fmt --all -- --checkare clean.I also ran a local sweep over every 3-chunk split of that 3-call gemma4 stream (8,911 chunkings), comparing each against the single-chunk result. 1,263 differed on
mainand 0 differ with this change. That sweep is not in the PR because it is slow.Scope
The issue also lists
llama3_jsonandpythonic. The same sweep shows chunking-dependent differences for those parsers, but this change doesn't affect them: the counts are the same before and after. They come from other paths (the;separator between llama3_json calls and back-to-back pythonic bracket groups), so I left them out of this PR rather than guess at the intended output.This needs a
dynamo-parsersrelease and a bump in ai-dynamo/dynamo to reach the frontend.