Skip to content

[BugFix] Fix Jamba tool parser deleting all occurrences of the streamed prefix on tool transition#48709

Open
chuenchen309 wants to merge 1 commit into
vllm-project:mainfrom
chuenchen309:fix/jamba-tool-flush-replace-count
Open

[BugFix] Fix Jamba tool parser deleting all occurrences of the streamed prefix on tool transition#48709
chuenchen309 wants to merge 1 commit into
vllm-project:mainfrom
chuenchen309:fix/jamba-tool-flush-replace-count

Conversation

@chuenchen309

Copy link
Copy Markdown

Why is this not duplicating an existing PR?

gh pr list --repo vllm-project/vllm --state open --search "jamba" returns
#46617 (Rust frontend, only touches rust/src/**, unrelated) and #48295.
#48295 ("Stream tool calls that arrive whole in a single delta in
llama3_json, jamba and ernie45 parsers") does touch this file and this same
"starting a new tool" branch, but for a different bug — it adds an early-exit
for the case where a whole message arrives in the first delta. Confirmed via
gh pr diff 48295 that the .replace() call this PR fixes is not present
as an added/removed line anywhere in that diff — only as unchanged
surrounding context. No logical conflict; a straightforward rebase either
way if #48295 merges first.

Bug

When a new tool call starts in the streamed array,
extract_tool_calls_streaming() flushes whatever wasn't yet sent from the
previous tool's arguments:

diff = json.dumps(diff, ensure_ascii=False).replace(
    self.streamed_args_for_tool[self.current_tool_id], ""
)

str.replace() with no count removes every occurrence of the
already-streamed prefix, not just the leading one it's meant to strip. If
that prefix's text recurs elsewhere in the completed arguments — e.g. a
nested object that repeats a top-level key/value, a realistic pattern for
find/replace-style tool arguments — the recurrence is silently deleted from
the flushed diff too, corrupting the JSON sent to the client.

Repro (isolated to this one code path, also the added regression test)

Tool 0's arguments are {"find": "cat", "options": {"find": "cat", "replace": "dog"}}; by the time tool 1 starts, streamed_args_for_tool[0]
is already '{"find": "cat"' (a normal partial-streaming state). Before
this fix, the flushed diff is ', "options": , "replace": "dog"}}' — the
nested "find": "cat" is gone entirely, and the reconstructed JSON fails to
parse (json.decoder.JSONDecodeError: Expecting value). This isn't a
narrow edge case; any tool whose argument schema can legitimately repeat a
key/value pair in a nested structure hits it.

Fix

Pass count=1 to .replace() so only the leading occurrence — the one
that's actually the already-streamed prefix — is removed.

Testing

  • Independently reproduced against the real installed vllm package by
    directly driving JambaToolParser.extract_tool_calls_streaming() with
    hand-constructed internal state, isolating this one code path from the
    rest of the streaming pipeline. Confirmed red
    (json.decoder.JSONDecodeError: Expecting value) before the fix, green
    (valid, correctly-nested JSON) after.
  • Added test_flush_on_new_tool_replaces_only_leading_occurrence to
    tests/tool_parsers/test_jamba_tool_parser.py. Confirmed red→green:
    reverting only the source change reproduces the exact JSONDecodeError;
    reapplying passes. Existing 8 tests in this file still pass (9 total now).
  • pre-commit run ruff-check, ruff-format, mypy-3.12 (--files scoped
    to the 2 changed files): pass.

Model-eval N/A — this only changes streaming-argument diff bookkeeping
logic, not model output/accuracy.

AI-Generated disclosure

Found via an AI-assisted code review pass (Claude Code) over
vllm/tool_parsers/. I independently reproduced the corruption, traced the
root cause, verified the fix, checked it against the overlapping open PR
#48295 line-by-line, and ran the tests above myself before submitting.

…ed prefix on tool transition, not just the leading one

When a new tool call starts in the streamed array, extract_tool_calls_streaming()
flushes whatever wasn't yet sent from the PREVIOUS tool's arguments:

    diff = json.dumps(diff, ensure_ascii=False).replace(
        self.streamed_args_for_tool[self.current_tool_id], ""
    )

str.replace() with no count removes every occurrence of the already-streamed
prefix, not just the leading one it's meant to strip. If that prefix's text
recurs elsewhere in the completed arguments -- e.g. a nested object that
repeats a top-level key/value, a realistic pattern for find/replace-style
tool arguments -- the recurrence is silently deleted from the flushed diff
too, corrupting the JSON sent to the client.

Repro (isolated to this one code path, also added as a regression test):
tool 0's arguments are {"find": "cat", "options": {"find": "cat", "replace":
"dog"}}; by the time tool 1 starts, streamed_args_for_tool[0] is already
'{"find": "cat"' (a normal partial-streaming state). Before this fix, the
flushed diff is ', "options": , "replace": "dog"}}' -- the nested `"find":
"cat"` is gone entirely, and the reconstructed JSON fails to parse
(`json.decoder.JSONDecodeError: Expecting value`). This isn't a narrow edge
case; any tool whose argument schema can legitimately repeat a key/value
pair in a nested structure hits it.

Fix: pass count=1 to .replace() so only the leading occurrence -- the one
that's actually the already-streamed prefix -- is removed.

Verified independently: reproduced against the real installed vllm package
by directly driving JambaToolParser.extract_tool_calls_streaming() with
hand-constructed internal state (isolating this one code path from the
rest of the streaming pipeline), confirmed red (JSONDecodeError) before the
fix and green (valid, correctly-nested JSON) after. Added
test_flush_on_new_tool_replaces_only_leading_occurrence to
tests/tool_parsers/test_jamba_tool_parser.py. Existing 8 tests in this file
still pass (9 total now). ruff check / ruff format / mypy (pre-commit) all
pass.

Duplicate-work check (gh pr list --search "jamba", gh api search/issues):
open PR vllm-project#48295 ("Stream tool calls that arrive whole in a single delta in
llama3_json, jamba and ernie45 parsers") also touches this file and this
same "starting a new tool" branch, but restructures the surrounding control
flow for a different bug (whole-message-in-one-delta) without touching this
.replace() call at all (confirmed via `gh pr diff 48295` -- the line isn't
present as an added/removed line in that diff, only as unchanged context).
No logical conflict; a straightforward rebase either way if vllm-project#48295 merges
first. vllm-project#46617 ("[Rust Frontend] Add Jamba tool parser") only touches
rust/src/**, unrelated.

AI-assisted: found via an AI-assisted code review pass over
vllm/tool_parsers/, independently reproduced, fixed, and verified by me
before submitting.

Signed-off-by: Andrew Chen <48723787+chuenchen309@users.noreply.github.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Andrew Chen <48723787+chuenchen309@users.noreply.github.com>

@claude claude Bot 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.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@github-actions

Copy link
Copy Markdown

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

PRs do not trigger a full CI run by default. Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging.

To run CI, PR reviewers can either: Add ready label to the PR or enable auto-merge.

If you have any questions, please reach out to us on Slack at https://slack.vllm.ai.

Agent Guidelines

IMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban.

🚀

@mergify mergify Bot added tool-calling bug Something isn't working labels Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working tool-calling

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant