Fix V2 sampler gate to enforce thinking_token_budget - #357
Open
ozskywalker wants to merge 1 commit into
Open
ozskywalker wants to merge 1 commit into
ozskywalker wants to merge 1 commit into
Conversation
Sampler._requires_logits_processing checks every per-request sampling feature (logit bias, penalties, bad words, non-default temperature/ min_p/top_k/top_p) except thinking_token_budget. A request that sets only a thinking budget, with otherwise-default sampling params (e.g. temperature=0, no penalties), makes the gate return False, so apply_sampling_params returns the raw logits and the thinking-budget kernel never runs. ThinkingBudgetState is otherwise fully wired (add_request/apply_staged_writes/apply) and ReasoningConfig correctly resolves single-token <think>/</think> markers even when reasoning_start_str/reasoning_end_str are left empty; only this gate check was missing. Confirmed live against DeepSeek-V4-Flash-0731 on the b12x image: thinking_token_budget=64 with temperature=0 and max_tokens=128 spent all 128 tokens on reasoning and returned content: null, finish_reason: length. Not B12X-specific; affects any model/recipe using thinking_token_budget under the V2 GPU model runner.
ozskywalker
marked this pull request as ready for review
August 23, 2026 19:11
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.
Summary
thinking_token_budgetis supposed to cap how many tokens a reasoning model spends "thinking" before it has to give a real answer. On the V2 GPU model runner, it silently doesn't work: if a request sets a thinking budget but doesn't also set some other special sampling option (custom temperature, penalties, etc.), the sampler skips the code path that would enforce the cap. The model then keeps reasoning until it runs out ofmax_tokensand returns an empty answer.This isn't specific to any one model or recipe — it affects any request that relies on
thinking_token_budgetunder the V2 model runner with otherwise-default sampling settings (e.g.temperature=0, which is common for tool calls and deterministic answers).Symptom
A
deepseek_v4request withthinking_token_budget=64, max_tokens=128, temperature=0burns all 128 tokens on reasoning and comes back withcontent: null, finish_reason: "length". In production this showed up as broken tool calls and missed long-context answers — same root cause, just triggered by reasoning running long in both cases.Fix
One missing check in
Sampler._requires_logits_processing(
vllm/v1/worker/gpu/sample/sampler.py):Everything else needed to make budgets work (
ThinkingBudgetState.add_request/apply_staged_writes/apply, andReasoningConfig's fallback to the parser's own<think>/</think>markers) was already correct — this one gate check was the only thing missing.Shipped as
docker/patch_vllm_thinking_budget_gate.py, matching the existingpatch_vllm_*.pyconvention (idempotent, no-ops on builds without this V2 sampler code).Testing
Unit tests (
tests/test_vllm_thinking_budget_gate_patch.py) plus on-hardware validation: restarted a live 2x DGX Spark production server on the patched image and re-ran the same checks that had just failed on the unpatched one.thinking_token_budget=64probecontent: null,finish_reason: lengthcontent: "Dawn"(correct),finish_reason: stopfinish_reason: length, invalid argsfinish_reason: tool_calls, valid args