Skip to content

fix(vllm): tell the reasoning parser whether thinking was enabled - #11791

Open
pos-ei-don wants to merge 1 commit into
mudler:masterfrom
pos-ei-don:vllm-reasoning-thinking-state
Open

fix(vllm): tell the reasoning parser whether thinking was enabled#11791
pos-ei-don wants to merge 1 commit into
mudler:masterfrom
pos-ei-don:vllm-reasoning-thinking-state

Conversation

@pos-ei-don

Copy link
Copy Markdown
Contributor

Description

vLLM's engine-based reasoning parsers derive their initial state from the chat template
kwargs. Qwen3Parser:

chat_kwargs = kwargs.get("chat_template_kwargs", {}) or {}
self.thinking_enabled = chat_kwargs.get("enable_thinking", True)

Constructed as ReasoningParser(tokenizer) the flag defaults to True, so the parser starts
in the REASONING state. A completion produced with thinking disabled contains no tags at all,
and every reasoning parser shape then reports the whole answer as reasoning:

  • engine-based parsers classify it by initial state;
  • BaseThinkingReasoningParser hits its documented "may not generate start token" fallback
    and returns (model_output, None).

Either way content = c if c is not None else generated_text turns that into a duplicate: a
Qwen3 model answering 391 with thinking off comes back as reasoning_content="391" and
content="391".

Measured against Qwen3.5-MoE on vLLM 0.28, non-streaming:

before   thinking on   reasoning=202   content="391"
         thinking off  reasoning="391" content="391"   <- duplicated
after    thinking on   reasoning=192   content="391"
         thinking off  reasoning=""    content="391"

The fix forwards the kwargs the prompt was rendered with, which is what vLLM's own OpenAI
server does; parsers that do not accept the argument keep the plain constructor.

Notes for Reviewers

The parser class matters here. On vLLM 0.28 the qwen3 name resolves to
Qwen3ParserReasoningAdapter, an engine-based adapter with no start_token — not to
BaseThinkingReasoningParser. A fix written against the latter is inert against the shipped
parser, which is why this one sets the state through the constructor rather than around it.

_split_reasoning() covers the older parser shape, which has no initial state to set. It
only reclassifies when the parser exposes a start/end token pair and neither the
completion nor the prompt ever opened a reasoning block. Truncated reasoning (block open, end
token never arrived) stays reasoning, and parsers without that token pair are left untouched.

Worth noting for anyone reading this alongside reasoning_effort: vLLM forwards the value
into template kwargs but filters it out when the template does not declare it. For a template
that only knows enable_thinking, thinking is binary and reasoning_effort has no effect
beyond on/off — this PR does not change that.

Signed commits

  • Yes, I signed my commits.
  • Documentation updated (docs/content/) for user-facing changes, or not applicable

vLLM's engine-based reasoning parsers derive their initial state from the
chat template kwargs. Qwen3Parser:

    chat_kwargs = kwargs.get("chat_template_kwargs", {}) or {}
    self.thinking_enabled = chat_kwargs.get("enable_thinking", True)

Constructed as ReasoningParser(tokenizer) the flag defaults to True, so the
parser starts in the REASONING state. A completion produced with thinking
disabled contains no tags at all, and every reasoning parser shape then
reports the whole answer as reasoning:

  - engine-based parsers classify it by initial state;
  - BaseThinkingReasoningParser hits its documented "may not generate start
    token" fallback and returns (model_output, None).

Either way `content = c if c is not None else generated_text` turns that
into a duplicate: a Qwen3 model answering "391" with thinking off comes back
as reasoning_content="391" AND content="391".

Measured against Qwen3.5-MoE on vLLM 0.28, non-streaming:

    before   thinking on   reasoning=202  content="391"
             thinking off  reasoning="391" content="391"   <- duplicated
    after    thinking on   reasoning=192  content="391"
             thinking off  reasoning=""    content="391"

Forward the kwargs the prompt was rendered with, which is what vLLM's own
OpenAI server does; parsers that do not accept the argument keep the plain
constructor.

_split_reasoning() covers the older parser shape, which has no initial state
to set. It only reclassifies when the parser exposes a start/end token pair
and neither the completion nor the prompt ever opened a reasoning block.
Truncated reasoning (block open, end token never arrived) stays reasoning,
and parsers without that token pair are left untouched.

Signed-off-by: pos-ei-don <1822533+pos-ei-don@users.noreply.github.com>

@localai-org-maint-bot localai-org-maint-bot 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.

Good to merge from my review. The parser now receives the same template kwargs used to render the prompt, while the fallback preserves older parser compatibility; the split logic also distinguishes plain answers from truncated reasoning. Regression coverage, Python syntax checks, and diff checks look clean. @mudler

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants