[he-yufeng]

Overview

Fixes #60341. In streaming mode Internlm2ToolParser stops emitting anything once <|action_start|> appears and waits for the full <|action_start|><|plugin|> marker. When the action turns out to be a non-plugin block (<|interpreter|>), or the generation ends right after <|action_start|>, the rest of the output is never streamed, while the non-streaming path returns the same text verbatim as content.

Claims

• Streaming now holds text back only while the tail after <|action_start|> is still a prefix of <|plugin|>; anything else streams through as content, matching extract_tool_calls.
• finalize_generation gives a legacy tool parser that defines finish_streaming() one chance to release held text at end of stream (opt-in via getattr, same shape as the reasoning fallback right above it). This also wires up the existing K2HorizonToolParser.finish_streaming(), which was previously only called from its own test.
• Plugin-call streaming is unchanged: while the tail can still grow into <|plugin|> the parser waits exactly as before, and the name/argument partial-JSON path is untouched.

Validation

Issue reproducer (an <|interpreter|> block, and a stream ending right after <|action_start|>), run against current main and against this branch:

• before: streaming returned only the prefix "Let me compute that. " in both cases
• after: concatenated streaming content equals the non-streaming content in both cases

Tests (CPU, .venv/bin/python -m pytest):

• tests/tool_parsers/test_internlm2_tool_parser.py: 14 passed, 8 xfailed. New tests cover non-plugin release, hold-then-release across a chunk boundary inside <|plugin|>, end-of-stream flush, and that a truncated plugin call is not re-emitted as content.
• tests/parser/test_streaming.py tests/tool_parsers/test_k2_horizon_tool_parser.py: all passed, including a new test driving parse_delta(..., finished=True) through DelegatingParser for the finalize wiring.
• ruff check / ruff format --check clean on the touched files.

Details

Root cause: the streaming guard required the combined marker <|action_start|><|plugin|> in the unstreamed tail and returned None otherwise, so any non-plugin action wedged the parser for the rest of the stream. There was also no end-of-stream hook on the tool-parser side, so even a legitimately held ambiguous window (a strict prefix of <|plugin|>) was dropped when generation stopped inside it.

vllm/parser/abstract_parser.py is touched on purpose, and the vllm/tool_parsers guidance says to call that out: releasing held text at end of stream is missing for every legacy tool parser with a hold-back, not just InternLM2, so the flush lives in finalize_generation next to the reasoning fallback. It is opt-in: parsers without finish_streaming() see no behavior change, engine-based parsers keep flushing through _flush_engine_parsers, and the hook name/signature follows the convention already used by the reasoning parsers and K2HorizonToolParser.

Tradeoff to be aware of: K2HorizonToolParser streams that end mid-parse now emit their unresolved tail as final content instead of dropping it. That is what its finish_streaming() was written for, and its test suite passes unchanged.

AI assistance was used in preparing this PR (Kimi Code); the change was validated locally as described above. On duplication: no open PR references #60341, and the other active InternLM2 streaming PR (#60363) fixes a different bug (argument characters dropped as partial JSON grows), so the two do not overlap.

---

Pull Request Checklist

• I used vLLM's /pr-checklist skill. (Mandatory for agents, optional for humans).

• AI assistance was used during the creation of this PR.

• Design Fit: Minimizes impact on core components, reuses existing functionality, and justifies added complexity.

• Testing and Validation: Validates the change and ensures any added tests are meaningful and reliable, with CI coverage or documented CI resource constraints and validation performed outside CI.

• Code Quality and Style: Keeps code and comments clear and concise, and updates relevant documentation and examples.

• Pull Request Contents: Includes a brief summary and relevant links, supports claims with evidence, explains root causes and implementation trade-offs, and follows the contributing guide.

[claude]

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]

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

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

[he-yufeng]

Rebased onto current main. The conflict was test-file adjacency with #60709, which landed this morning; both test sets are kept and the parser hunk from that fix sits in the argument-diff region, disjoint from the marker handling changed here. test_internlm2_tool_parser.py (16 passed) and tests/parser/test_streaming.py (27 passed) are green at the rebased head, pre-commit clean.

[sfeng33]

Please test with live model, unit test is not sufficient.

[he-yufeng]

@sfeng33 Live model run, as asked. This branch built from source on the macOS CPU backend (there are no prebuilt arm64 wheels), serving internlm/internlm2_5-1_8b-chat in fp16 with --enable-auto-tool-choice --tool-call-parser internlm. A dummy get_weather tool is registered so the parser path is active, and both runs below are streamed chat completions at temperature 0.

Case 1, the non-plugin action from #60341. A computation prompt puts the model into its trained interpreter format, so the generated stream starts with <|action_start|><|interpreter|>:

• with the two parser files checked out from the parent commit (same build; this PR touches no C++, so the extension is identical): content='', finish_reason=stop. The whole answer is swallowed.
• with this branch: content arrives across 21 deltas and concatenates to <|action_start|><|interpreter|>\n```python\nresult = 123456 * 789\nresult\n```<|action_end|>\n, byte-identical to what the non-streaming path returns for the same text.

Case 2, stream ends immediately after <|action_start|> (max_tokens=1, finish_reason=length):

• pre-fix: content='', the held marker is lost.
• this branch: content='<|action_start|>', released by the new end-of-stream flush.

The plugin-call path is untouched by this change. I could not coax the 1.8B model into emitting its trained plugin format on demand, so that path is asserted by the parser unit tests at this head (16 passed, 8 xfailed) rather than by a server transcript here.

[he-yufeng]

Rebased onto current main; no semantic conflicts. The parser and streaming suites pass locally on this head (43 passed, 8 xfailed, CPU).

[he-yufeng]

Rebased onto current main (3b51898) to pick up #62593 — the earlier pre-commit failure here was the Step5 sparse_attention.py mypy breakage inherited from main, not something in this change. Clean rebase, diff unchanged.

[smiyamoto3]

One problem in the end-of-stream hook, inline. I confirmed the new tests catch the InternLM2 case.

[smiyamoto3]

The engine tool adapters already define finish_streaming (adapters.py), so this call drains the engine flush before _flush_engine_parsers runs, and only .content is kept. Held-back tool args get dropped: TestAdapterFinishOnStreamEnd::test_args_buffer_flushed_on_finished passes on main and fails here, and a Qwen3 stream cut off by max_tokens right after </parameter> now ends with {"city": "Tokyo instead of {"city": "Tokyo"}.

Skipping engine parsers here, like _flush_engine_parsers does, fixes both and tests/parser still passes. Renaming the hook would work too.

Suggested change

	if tool_finish is not None:
	if tool_finish is not None and not getattr(
	self._tool_parser, "engine_based_streaming", False
	):

[he-yufeng]

Confirmed and fixed in f519b89, taking your suggestion verbatim. I reproduced the failure first: TestAdapterFinishOnStreamEnd::test_args_buffer_flushed_on_finished fails on the previous head and passes with the guard, and the InternLM2/streaming/parser-engine suites plus the full tests/tool_parsers run stay green locally (1211 passed; the only errors are the llama3 tokenizer fixture needing HF auth for a gated repo, unrelated to this change).

You were right about the mechanism: the engine adapters' finish_streaming drains the engine flush early and only .content would survive in this hook, dropping held-back args. Skipping engine parsers here matches what _flush_engine_parsers already does, so I kept your naming over renaming the hook.

[smiyamoto3]

Thanks for the fix, LGTM. One small thing: mypy in pre-commit fails on the four new tests (Cannot assign to a method). Switching them to monkeypatch.setattr(...) like the rest of the file (since #62142) fixes it.

[mergify]

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

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

[he-yufeng]

Rebased onto current main to clear the conflict. #62142 rewrote the shared fixture to monkeypatch.setattr, so my additions now follow the same pattern and the # type: ignore[method-assign] comments are gone entirely.

Verified on the pushed head (7e6bbd6): tests/tool_parsers/test_internlm2_tool_parser.py and tests/parser/test_streaming.py pass on CPU (43 passed, 8 xfailed).

[smiyamoto3]

Thanks for the update. The monkeypatch version looks good to me.