archived internal review draft (b) - #2
Closed
valeriyischenko wants to merge 2 commits into
Closed
Conversation
When `thinking_token_budget` is exhausted, the budget forces `reasoning_end_str`. That assumes the marker which ends reasoning is also a complete instruction to start answering, which is true of `</think>` but not of a model that closes a reasoning message and then has to *open* an answer channel: forcing the closing marker alone leaves such a model free to open another reasoning message and keep thinking, so the budget never takes effect. Add `forced_reasoning_end_str`, defaulting to None so `ReasoningConfig` falls back to `reasoning_end_str` and every existing parser is unchanged. `ReasoningConfig` now forces that string while `reasoning_end_str` keeps detecting the natural end, which is also the shorter of the two. Document both, and document that either string is matched against generated token ids, so it has to be spelled the way the model generates it -- including leading whitespace the chat template leaves to the first generated token. Signed-off-by: Valerii Ishchenko <valeriy@ischenko.me>
`ReasoningConfig.initialize_token_ids` returns early unless it can resolve both boundary strings, so `reasoning_config.enabled` stayed False for MuseGlimmer and any request carrying a `thinking_token_budget` was rejected with "reasoning_config is not configured. Please set --reasoning-parser" -- which the user had already done. The parser declared no boundary strings because MuseGlimmer frames reasoning as a channel rather than delimiting it with a `<think>`/`</think>` pair. Declare the three strings that framing implies: ` to=self<|message|>` opens reasoning, `<|eom|>` ends it naturally, and `<|eom|><|start|>assistant to=user<|message|>` is the forced end, which closes the reasoning message and opens the answer channel in one string so the model cannot respond to the injection by thinking further. The start string keeps its leading space deliberately. The chat template renders an assistant header as `<|start|>assistant to=self<|message|>` and ends the generation prompt after `<|start|>assistant`, so the space belongs to the first generated token and ` to` is not the token `to`. The budget matches token ids by exact slice, so the other spelling would silently never fire. Also implement `count_reasoning_tokens`, which reported 0 reasoning tokens for every MuseGlimmer response. There is no start token to key on, so the few tokens ahead of each `<|message|>` are decoded to recover the recipient; channel bodies are never decoded. Signed-off-by: Valerii Ishchenko <valeriy@ischenko.me>
Owner
Author
|
Closing — superseded by the 2026-08-30 submission plan. This PR shrinks before upstream submission: keep the two string declarations (budget enablement/enforcement), drop count_reasoning_tokens (ceded to vllm-project#54238) and the forced_reasoning_end_str base property (motivating example removed by the tool-routing A/B; Muse keeps forced=None). Branch is kept for reference. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
thinking_token_budgetis unusable with Muse-Glimmer: the server rejects anyrequest that sets it. The chain, all on current
main:MuseGlimmerReasoningParseroverrides neitherreasoning_start_strnorreasoning_end_str, so both inherit the baseNone(
abs_reasoning_parsers.py).ReasoningConfig.initialize_token_idstherefore reachesif not reasoning_start_str or not reasoning_end_str: returnand leaves_enabled = False(vllm/config/reasoning.py).InputProcessorthen raises for every request carrying a budget(
vllm/v1/engine/input_processor.py):which surfaces as HTTP 400 — and the advice in the message is already
satisfied: the server was started with
--reasoning-parser muse_glimmer.Confirmed on the serving path, not just by reading: with the stock parser a
thinking_token_budgetrequest returns 400, while aqwen3server on the samebuild accepts it.
count_reasoning_tokens()also returns 0, so usage accountingreports no reasoning tokens even when no budget is requested.
Declaring the strings is not enough, because Muse needs two different end
strings:
<|eom|>closes the reasoning message; short, so it matchesreliably;
<|eom|>alone leaves the model free to open anotherto=selfblock, so exhausting the budget must also open the answer channel:
<|eom|><|start|>assistant to=user<|message|>.ReasoningConfigalready distinguishes forced (reasoning_end_token_ids) fromnatural (
natural_reasoning_end_token_ids), but a parser can only declare onestring; the forced one is a config-only override. This PR adds the missing hook.
Why this is not a duplicate
natural/forced split now in
main). This PR is the other half: it lets theparser declare a forced string that differs from its natural one, which no open
PR does.
reasoning_end_strto break repetition loops. It consumes the samefield and benefits from this change; it does not provide it.
thinking_budget_state.py; disjoint.Design
Two commits: the generic property first, then the Muse-Glimmer change that uses it.
ReasoningParser.forced_reasoning_end_str— new property, defaults toNone,documented as "override only when closing reasoning does not commit the model to
answering". The default keeps every existing parser unchanged.
ReasoningConfig.initialize_token_idsprefersparser.forced_reasoning_end_str or parser.reasoning_end_strfor the forced ids,and keeps
parser.reasoning_end_stras the natural ids.reasoning_start_str = " to=self<|message|>",reasoning_end_str = "<|eom|>",forced_reasoning_end_str = "<|eom|><|start|>assistant to=user<|message|>".The leading space is load-bearing: the chat template ends the generation
prompt after
<|start|>assistant, so the model emitsto, a different tokenfrom
to, and the budget matches ids by exact slice — the wrong spelling failssilently.
count_reasoning_tokenscounts tokens insideto=selfmessages by finding<|message|>ids and decoding a short window backwards for the recipient(channel framing has no start token to key on), returning 0 if the framing
markers are not single tokens rather than guessing. Channel bodies are never
decoded.
docs/features/reasoning_outputs.mdon the id-matched-spellingtrap and on channel-framed models needing a separate forced string.
Files:
vllm/reasoning/abs_reasoning_parsers.py,vllm/config/reasoning.py,vllm/reasoning/muse_glimmer_reasoning_parser.py,tests/reasoning/test_muse_glimmer_reasoning_parser.py,docs/features/reasoning_outputs.md.Where the strings come from
chat_template.jinjain the checkpoint, read directly:'<|start|>assistant'with no trailing space, and achannel is rendered as
'<|start|>assistant' + ' to=' + recipient + '<|message|>', so the first thing the model generates isto=self<|message|>—the leading space is the template's, not a guess;
'<|start|>assistant to=self<|message|>' + reasoning + '<|eom|>', and the answer that follows usesrecipient = message.get('recipient') or 'user', i.e. always a recipient. Sothe transition the model was trained to emit is exactly
<|eom|><|start|>assistant to=user<|message|>, the forced string here;<|start|>,<|message|>,<|eom|>,<|eot|>are all declared special tokens,so each is a single id and
count_reasoning_tokenscan key onvocab["<|message|>"].Tests
Seven new tests over a stub tokenizer: the three boundary strings;
ReasoningConfigending up
enabledwith forced ≠ natural ids; andcount_reasoning_tokensover asingle span, several spans, an unterminated span, a tool channel, and a vocabulary
without the framing tokens.
End-to-end enforcement is measured: with the gate open, the chat endpoint returns
exactly 64 and exactly 256 reasoning tokens for those budgets, with an
answer present and no overshoot, against HTTP 400 in the stock state. Combined with
PR 1, the forced answer channel is also schema-valid and ends on
<|eot|>atbudgets 256 and 512 — which is the strongest argument for this PR: on a
reasoning-heavy prompt the schema is unusable without a budget, because the model
reasons past
max_tokens(4,093 reasoning tokens,finish_reason: length) andreturns no answer at all.
Known limitations, stated rather than inherited silently
The legacy
vllm/v1/sample/thinking_budget_state.pypath usesreasoning_end_token_ids(the forced string) for both forcing and natural-enddetection;
natural_reasoning_end_token_idsis consumed only by the MRv2 GPU path(
vllm/v1/worker/gpu/sample/thinking_budget.py, added in vllm-project#46727). For a normalMuse answer turn that still works, because the model emits the whole forced string
verbatim — detection just lags by the header tokens. It breaks for a tool turn,
where the model leaves reasoning with
<|eom|><|start|>assistant to=<tool>.<fn><|message|>: that is not the forcedstring, so the legacy path sees no natural end and can force the answer header into
the middle of a tool call. This PR supplies the natural ids that fix it on MRv2;
wiring the legacy path to them is a separate change and pre-existing behaviour.
That failure mode is not hypothetical: open issue vllm-project#44676 reports exactly it for
Qwen3.5+ — the tool parser treats
<tool_call>as an implicit reasoning end,ThinkingBudgetStateHolderdoes not, so the budget expires mid-JSON and the forcedend marker lands inside the tool-call arguments. It is the reason a parser must be
able to declare a natural end distinct from the string it wants forced; the Muse
channel protocol makes that distinction structural rather than model-specific.
Second: enabling the budget for Muse also opts these requests into the marker-scan
cost that open PR vllm-project#52106 fixes.
ThinkingBudgetState'sscan_offsetonly advanceswhen a thinking section exits, so a single long reasoning span rescans the whole
growing output every decode step — 1.2 ms/step at 16k tokens in pure Python, ~20 s
cumulative per request. Orthogonal to this PR and pre-existing for every budget
user, but fair warning for anyone enabling a budget on long reasoning.
AI assistance
Written with AI assistance (Claude Code), reviewed by submitter.