Skip to content

[Bugfix][ToolParser] Fix ValueError on second action block in InternLM2 streaming - #58346

Open
AdaAibaby wants to merge 1 commit into
vllm-project:mainfrom
AdaAibaby:fix/internlm2-streaming-two-action-blocks
Open

AdaAibaby wants to merge 1 commit into
vllm-project:mainfrom
AdaAibaby:fix/internlm2-streaming-two-action-blocks

Conversation

@AdaAibaby

Copy link
Copy Markdown
Contributor

Purpose

Internlm2ToolParser.extract_tool_calls_streaming splits the accumulated text on the tool-call marker with an unbounded str.split and unpacks the result into exactly two values:

new_delta = current_text[last_pos:]
text, action = new_delta.split("<|action_start|><|plugin|>")

self.position is only advanced past text that precedes the first marker, and self.current_tool_id never exceeds 0, so the if self.current_tool_id > 0 early-return never fires on the tool-call path. Once the model streams a second <|action_start|><|plugin|> block, current_text[last_pos:] contains two markers, split returns three parts, and the unpack raises:

ValueError: too many values to unpack (expected 2)

This line runs before the method's try/except (which starts a few lines later), so the exception propagates and aborts the streaming request rather than being swallowed.

Trigger

A model emitting two consecutive tool-call blocks, e.g. accumulated text:

<|action_start|><|plugin|>{"name": "f", "parameters": {}}<|action_end|><|action_start|><|plugin|>{"name": "g", "parameters": {}}<|action_end|>

InternLM2 is documented as not supporting parallel tool calls, but that is a limitation, not a guard — nothing constrains the decoder from emitting a second block, and a misprompted/hallucinating model realistically can.

Fix

Split only on the first marker with str.partition:

text, _, action = new_delta.partition("<|action_start|><|plugin|>")

The subsequent action.split("<|action_end|>")[0] already isolates the first tool call, so single-tool-call behavior is unchanged. Verified that for single-block input the extracted (text, action) is identical to the old code.

Test

Added test_streaming_two_action_blocks_no_unpack_error in tests/tool_parsers/test_internlm2_tool_parser.py, which streams two action blocks and asserts the extractor does not raise. It fails on main (ValueError) and passes with this change.

pytest tests/tool_parsers/test_internlm2_tool_parser.py -k two_action_blocks

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

@mergify

mergify Bot commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @AdaAibaby.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Oct 23, 2026
@github-project-automation github-project-automation Bot moved this to Unsorted queue in Tool Calling Oct 23, 2026
@github-project-automation github-project-automation Bot moved this to Unsorted queue in Tool Calling (arpera) Oct 23, 2026
@AdaAibaby
AdaAibaby force-pushed the fix/internlm2-streaming-two-action-blocks branch from 325ea68 to 3a4a669 Compare October 26, 2026 03:57
…nlm2 streaming

Internlm2ToolParser.extract_tool_calls_streaming splits the accumulated
delta on "<|action_start|><|plugin|>" with an unbounded str.split and
unpacks the result into exactly two names:

    text, action = new_delta.split("<|action_start|><|plugin|>")

self.position is only advanced past text that precedes the first marker,
and self.current_tool_id never exceeds 0, so the `current_tool_id > 0`
early-return never fires on the tool-call path. Once the model streams a
second "<|action_start|><|plugin|>" block, current_text[last_pos:]
contains two markers, split returns three parts, and the unpack raises
`ValueError: too many values to unpack (expected 2)`. This runs before
the method's try/except (which starts later), so it propagates and aborts
the stream.

Use str.partition to split only on the first marker. The following
`action.split("<|action_end|>")[0]` already isolates the first tool call,
so single-tool behavior is unchanged.

Signed-off-by: shaolila <shaolila@buaa.edu.cn>
@AdaAibaby
AdaAibaby force-pushed the fix/internlm2-streaming-two-action-blocks branch from 3a4a669 to b1d572a Compare October 26, 2026 04:06
@mergify mergify Bot removed the needs-rebase label Oct 26, 2026

This branch has not been deployed

No deployments
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: Unsorted queue

Development

Successfully merging this pull request may close these issues.

2 participants