Skip to content

fix(parsers): re-jail a complete start marker in trailing content - #408

Merged
keivenchang merged 1 commit into
ai-dynamo:mainfrom
gokay-ai:fix/jail-trailing-complete-start-marker
Oct 15, 2026
Merged

keivenchang merged 1 commit into
ai-dynamo:mainfrom
gokay-ai:fix/jail-trailing-complete-start-marker

Conversation

@gokay-ai

Copy link
Copy Markdown
Contributor

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:

"<|tool_call>call", ":f{", "a:<|\"|>1<|\"|>",
"}<tool_call|><|tool_call>ca",
"ll:g{", "b:", "<|\"|>2<|\"|>}<tool_call|>"

After call A completes, the rest of that delta (<|tool_call>ca) goes to handle_trailing_content. That function only handled MatchResult::Partial. A MatchResult::Complete fell through to split_partial_tool_call_start, and the gemma4 call-prefix split took ca as a partial call: prefix. It emitted <|tool_call> as content and held only ca. On main the stream above returns both calls but content is "<|tool_call>". With other chunkings, a whole call ends up in content and is missing from tool_calls. For example, ["<", "|tool_call>call:f{…}<tool_call|><|tool_call>c", "all:g{…}<tool_call|><|tool_call>call:h{…}<tool_call|>"] returns only f and h.

Fix

handle_trailing_content now handles MatchResult::Complete the same way process_content does. Prose before the marker is emitted as trailing content, unless it carries harmony protocol, in which case it is jailed with the marker, as in process_content. The jail then restarts from the marker. The Partial and None paths are unchanged.

Tests

  • test_gemma4_next_call_start_marker_in_trailing_content_is_jailed in parsers/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 call g and the start of call h. Each run must return f, g, h with empty content. It fails on main (left: "<|tool_call>") and passes with the fix.
  • cargo test -p dynamo-parsers: all suites pass (974 lib tests, 84 in jail).
  • cargo clippy -p dynamo-parsers --all-targets -- -D warnings and cargo fmt --all -- --check are 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 main and 0 differ with this change. That sweep is not in the PR because it is slow.

Scope

The issue also lists llama3_json and pythonic. 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-parsers release and a bump in ai-dynamo/dynamo to reach the frontend.

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]>
@gokay-ai
gokay-ai requested a review from a team as a code owner October 14, 2026 06:32
Copilot AI balanced review requested due to automatic review settings October 14, 2026 06:32
@copy-pr-bot

copy-pr-bot Bot commented Oct 14, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot added fix external-contribution PR originates from a fork labels Oct 14, 2026
@keivenchang

Copy link
Copy Markdown
Contributor

/ok to test e4f3f2f

@keivenchang keivenchang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

@github-actions

Copy link
Copy Markdown

📊 Conformance matrix rendered — view in CI summary

@keivenchang keivenchang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

approved — thanks Gokay, this fixes Gemma4 call re-entry across chunk boundaries. nice work.

@keivenchang
keivenchang merged commit f9f8e87 into ai-dynamo:main Oct 15, 2026
7 checks passed
keivenchang added a commit that referenced this pull request Oct 23, 2026
keivenchang added a commit that referenced this pull request Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external-contribution PR originates from a fork fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: Streaming tool-call parser leaks a parallel tool call into message.content on multi-token delta boundaries

3 participants