feat(gateway): forward reasoning_effort to vllm - #1613
Open
qdanik wants to merge 1 commit into
Open
Conversation
…t, defaulting to max instead of inheriting the engine's high
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
reasoning_effortwas enum-validated and then stripped on every route, andmax— the one value DeepSeek-V4 defines beyond the OpenAI set — was rejected outright, so the field could not reachdeepseek-ai/DeepSeek-V4-Flash-0731, the routed model that actually reads it.Forward the field on that route only; every other route keeps the strip.
Fill in
maxwhen the caller omits the field. An explicit level always survives, including levels weaker than the default.Add
maxtoReasoningEffortValidator, matching the vLLM wire enum.Record
reasoning: {"enabled": false}asreasoning_effort: "none"instead of dropping it silently.Add
DefaultLiteralParameter— the write-only-if-absent counterpart to the existingForceLiteralParameter, which can only overwrite.Document the route in
docs/chat-api/deepseek-v4-flash-0731.md, including the value mapping the engine applies:noneminimal/low/mediumlowhigh/xhighhighmaxmaxWhy
vLLM's
build_chat_paramsmergesreasoning_effortintochat_template_kwargsitself and separately derivesenable_thinkingfrom it, so the top-level field is sufficient on its own and needs no mirroring the way Kimi'sthinkingdoes. That same derivation is why the other routes keep the strip: forwarding the field to a template that declaresenable_thinkingwould silently flip thinking on or off while the effort level itself went nowhere.The levels are prompt prefixes rather than sampling knobs —
REASONING_EFFORT_PROMPTSmaps each to a literal instruction block injected once at message index 0. An omitted field resolves tohigh, which is the level the model's own discussion thread names in its reasoning-loop reports, so the route defaults to the strongest prefix the encoder defines rather than inheriting that fallback. One consequence worth knowing when reading the table above:minimal,lowandmediumall render an empty prefix, so sending them asks for less reasoning than sending nothing.reasoning: {"enabled": false}had to stop being dropped. It deleted the wrapper and wrote nothing, leaving the request indistinguishable from one that never mentioned reasoning — which the new default reads as permission to fill the gap. Without this, a client explicitly disabling reasoning would have received maximum reasoning.No
max_tokensguard was added, deliberately. Reasoning tokens do count againstmax_tokensin vLLM (vllm-project/vllm#28266) and this route has no budget knob — the tokenizer defines none,thinking_token_budgetis stripped off-Kimi, and it is a logits processor that vLLM discards under the--speculative-configthis model's card recommends. The Kimi-style floor was still the wrong thing to copy: its256threshold sits next to a budget that can be clamped, which does not exist here, so the equivalent action would be disabling reasoning outright. Epoch 364 gives no reason to: empty-stream share is 2% on DeepSeek against 2% on MiniMax-M2.7 and 3% on Kimi-K2.6, measured while the engine was already defaulting this route tohigh. If that share moves after rollout there will be a measured threshold to fit instead of a borrowed one.On the prompt side the prefix is not new — the engine was already injecting the
highblock while we stripped the field, so the change adds 50 characters (~12 tokens) against a 400k-tokens configured window. Admission is unaffected either way:ContextTotalHintis parsed out of the host's own rejection message, so the gateway reads the real prompt size from the engine rather than predicting it.Tests
TestNormalizeChatRequestForwardsReasoningEffortToDeepSeek— all seven wire values survive on the DeepSeek route, andchat_template_kwargsis not written (vLLM performs that merge; mirroring would write the key twice).TestNormalizeChatRequestStripsReasoningEffortOffTheReasoningRoute— every value, includingmax, is stripped on Kimi, MiniMax and the empty route.TestNormalizeChatRequestDefaultsDeepSeekReasoningEffortToMax— an omitted field arrives asmax.TestNormalizeChatRequestKeepsAnExplicitDeepSeekReasoningEffort— the default never overrules a caller's level, weaker levels included.TestNormalizeChatRequestReasoningDisabledSurvivesTheDeepSeekDefault—reasoning: {"enabled": false}lands asnone, notmax.TestNormalizeChatRequestDoesNotDefaultReasoningEffortOffTheReasoningRoute— no default leaks onto routes that strip the field.TestReasoningValidatorEnabledFalseRecordsTheRefusal— the refusal is written asnonerather than dropped.TestReasoningEffortValidatorAccepts/TestNormalizeChatRequestRejectsInvalidReasoningEffort—maxaccepted, unknown strings still rejected.Mutation-tested: reverting the default, weakening it to
high, letting it overwrite an explicit value, restoring the silent loss ofenabled:false, moving the model scope to Kimi, reverting to the universal strip, and removingmaxfrom the enum are each killed by the above.Test plan
go build ./...cleango test ./cmd/devshardctl/... -count=1green across all four packagesgofmtclean on every touched filedocs/chat-api/references.md