Skip to content

[Bugfix][Parser] Scope reasoning-end detection to the current turn via turn-boundary tokens - #54089

Merged
bbrowning merged 1 commit into
vllm-project:mainfrom
Xarbirus:qwen-reasoning-parser-fix
Aug 27, 2026
Merged

bbrowning merged 1 commit into
vllm-project:mainfrom
Xarbirus:qwen-reasoning-parser-fix

Conversation

@Xarbirus

Copy link
Copy Markdown
Contributor

Purpose

ParserEngine.is_reasoning_end() walks the token sequence backwards and returns True on the first </think> it sees. It is called on the full prompt (the entire rendered conversation) from two places:

  1. DelegatingParser.parse_delta (vllm/parser/abstract_parser.py) - seeds reasoning_ended at stream start. A stale </think> in history routes the new turn's entire <think>…</think> block into content instead of reasoning (and the answer is lost entirely under include_reasoning=false).
  2. StructuredOutputManager.should_fill_bitmask (vllm/v1/structured_output/__init__.py) - seeds the scheduler's reasoning-end gate. A stale </think> applies the grammar bitmask from the first generated token, constraining the model's thinking.

Fix

Add turn_boundary_tokens to ParserEngineConfig; the backward walk now stops at the first turn boundary (<|im_start|>/<|im_end|> for the ChatML family), so markers from earlier turns are no longer visible. Hitting a
boundary falls back to initial_state != REASONING (the same semantics as an empty prompt) which keeps the "template closed the think block in the generation prompt" case working (covered by a test). This generalizes into the
engine what #44551 (Cohere) and poolside_v1 (here) already do per-parser, and follows the model-specific backward-scan direction maintainers requested when closing the generic serving-layer attempts (#46663 review).

Backward compatible: the default boundary set is empty, and boundary tokens missing from a tokenizer's vocab resolve to nothing, so parsers on non-ChatML vocabs (e.g. SeedOssParser) keep the previous behaviour. Both cases are covered by tests.

Why this is not a duplicate

Test Plan

pytest tests/parser/engine/test_qwen3_reasoning.py -v
pytest tests/parser -q
pre-commit run --files <changed files>
pre-commit run mypy-3.12 --all-files --hook-stage manual

E2E: serve Qwen/Qwen3-1.7B with --reasoning-parser qwen3, send streaming requests whose prompt contains a stale </think> (literal tag in user text), plus regression controls, at temperature=0; compare field routing before and after this change.

Test Result

Tested on H100

  • tests/parser/engine/test_qwen3_reasoning.py: 57 passed (incl. 10 new turn-boundary tests)

  • full tests/parser suite: 4086 passed, 0 failures

  • pre-commit on changed files: all hooks passed

  • E2E (Qwen/Qwen3-1.7B, --reasoning-parser qwen3, streaming, temperature=0):

    case main this PR
    control single turn reasoning 1227 ch / content 77 ch identical
    replayed reasoning in history reasoning 875 ch / content 16 ch identical
    literal </think> in user text reasoning 0 ch, whole response incl. <think> tag leaked into content (602 ch) reasoning 571 ch / content 16 ch, no leak

Control cases are byte-identical before/after.


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.

…a turn-boundary tokens

ParserEngine.is_reasoning_end() walks the whole token sequence backwards
and returns True on the first </think>, so a stale marker in conversation
history makes the DelegatingParser stream the new turn's thinking into
`content`

Co-authored-by: Claude Code
Signed-off-by: Mikhail Podvitskii <podvitskiymichael@gmail.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.

@mergify mergify Bot added qwen Related to Qwen models tool-calling bug Something isn't working labels Aug 27, 2026
@bbrowning bbrowning added the verified Run pre-commit for new contributors without triggering other tests label Aug 27, 2026
@bbrowning

Copy link
Copy Markdown
Collaborator

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #85873 for commit 24a3b24415a1.

@bbrowning bbrowning left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Note that the SeedOSS models inherit from the Qwen3 parser here. That happens to be harmless in this case, even though the boundary turn tokens are now wrong for SeedOSS, because the code gracefully degrades when those tokens are not in the model vocab.

To get this fixed for SeedOSS models as well, need to override SeedOssParser.TURN_BOUNDARIES with <seed:bos> and <seed:eos> instead of the inherited ChatML tokens (and confirm those are the right turn boundary tokens for the target model and in that model's vocab) and add a multi-turn test containing a stale </seed:think> before the latest Seed turn boundary.

Given that this is harmless for SeedOSS models today, approving this as is. Thanks for the fix!

@bbrowning
bbrowning merged commit 3a9bfc2 into vllm-project:main Aug 27, 2026
72 of 78 checks passed
@Xarbirus
Xarbirus deleted the qwen-reasoning-parser-fix branch August 28, 2026 10:11
am-cohere pushed a commit to am-cohere/vllm that referenced this pull request Sep 1, 2026
…a turn-boundary tokens (vllm-project#54089)

Signed-off-by: Mikhail Podvitskii <podvitskiymichael@gmail.com>
mylibrar pushed a commit to tanyuqian/vllm that referenced this pull request Sep 3, 2026
…a turn-boundary tokens (vllm-project#54089)

Signed-off-by: Mikhail Podvitskii <podvitskiymichael@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working qwen Related to Qwen models tool-calling verified Run pre-commit for new contributors without triggering other tests

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants