fix(deepseek): align V4 thinking behavior with the official API - #1962
fix(deepseek): align V4 thinking behavior with the official API#1962junliu-mde wants to merge 6 commits into
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
github-actions
Bot
added
tokenizer
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds typed DeepSeek thinking configuration, DeepSeek V4 parser and tokenizer defaults, reasoning-effort normalization, HTTP control translation, and request-based gateway precedence handling. Tests cover validation, parser selection, prompt rendering, routing translation, and reasoning-state resolution. ChangesDeepSeek thinking flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Router
participant ChatGateway
participant HuggingFaceTokenizer
participant ReasoningParser
Client->>Router: submit DeepSeek V4 thinking controls
Router->>ChatGateway: forward translated request
ChatGateway->>HuggingFaceTokenizer: render thinking-aware prompt
ChatGateway->>ReasoningParser: select and start reasoning parsing
ReasoningParser-->>Client: return separated reasoning and response
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hi @junliu-mde, the DCO sign-off check has failed. All commits must include a To fix existing commits: # Sign off the last N commits (replace N with the number of unsigned commits)
git rebase HEAD~N --signoff
git push --force-with-leaseTo sign off future commits automatically:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@model_gateway/src/routers/grpc/utils/parsers.rs`:
- Around line 50-74: Update resolve_user_thinking to reuse
ChatCompletionRequest::thinking_preference() for the protocol and
reasoning_effort fallback instead of passing request.thinking and
request.reasoning_effort through resolve_thinking_pref; preserve
template_thinking as the highest-priority value. Adjust
resolve_thinking_pref_explicit_kwarg_wins to exercise the simplified
resolve_user_thinking with an appropriate request fixture, or remove it in favor
of coverage through resolve_user_thinking.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3b22415c-ced2-41e5-beff-96187f88f248
📒 Files selected for processing (10)
crates/protocols/src/chat.rscrates/reasoning_parser/src/factory.rscrates/tokenizer/src/encoders/deepseek_v4.rscrates/tokenizer/src/huggingface.rscrates/tokenizer/tests/deepseek_renderer_detection.rsmodel_gateway/src/routers/grpc/regular/processor.rsmodel_gateway/src/routers/grpc/regular/stages/chat/request_building.rsmodel_gateway/src/routers/grpc/regular/streaming.rsmodel_gateway/src/routers/grpc/utils/chat_utils.rsmodel_gateway/src/routers/grpc/utils/parsers.rs
0441fa6 to
586a5ba
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 586a5ba918
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let owned = inject_tools_into_messages(messages, params.tools); | ||
| let msgs: &[serde_json::Value] = owned.as_deref().unwrap_or(messages); | ||
| let thinking_mode = derive_thinking_mode(params); | ||
| let thinking_mode = derive_thinking_mode(params, true); |
There was a problem hiding this comment.
When this default makes DeepSeek V4 prompts enter thinking mode for Anthropic Messages requests with no thinking config, the response paths still only enable separate_reasoning for explicit Enabled/Adaptive configs (process_non_streaming_messages_response and the Messages streaming setup). In that omitted-config case the registered V4 parser is never run, so output like reasoning</think>answer is emitted as a text block/SSE text instead of a thinking block plus answer, leaking/corrupting reasoning content for the Messages API.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
fixed in 486a618.
Both Messages response paths (non-streaming process_non_streaming_messages_response and the streaming setup) now compute separate_reasoning as: explicit Enabled/Adaptive OR should_mark_reasoning_started(user_thinking, tokenizer), so a V4 request with an omitted thinking config (template default on) runs the reasoning parser and emits a proper thinking block instead of leaking reasoning</think> into text content. Explicit Disabled still opts out.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/tokenizer/src/huggingface.rs`:
- Around line 581-589: Update the changelog or release notes to explicitly
document that DeepSeek V4 now defaults thinking mode to on when neither thinking
nor reasoning_effort is specified, causing responses to include
<think>...</think> reasoning content. Reference the DeepSeek V4 default behavior
introduced by thinking_toggle and the related request mapping changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ad216246-e226-4e9c-a0de-bd49e2ae0264
📒 Files selected for processing (10)
crates/protocols/src/chat.rscrates/reasoning_parser/src/factory.rscrates/tokenizer/src/encoders/deepseek_v4.rscrates/tokenizer/src/huggingface.rscrates/tokenizer/tests/deepseek_renderer_detection.rsmodel_gateway/src/routers/grpc/regular/processor.rsmodel_gateway/src/routers/grpc/regular/stages/chat/request_building.rsmodel_gateway/src/routers/grpc/regular/streaming.rsmodel_gateway/src/routers/grpc/utils/chat_utils.rsmodel_gateway/src/routers/grpc/utils/parsers.rs
586a5ba to
afc6e8a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afc6e8a26b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| registry.register_pattern("deepseek-r1", "deepseek_r1"); | ||
| registry.register_pattern("deepseek-v3.1", "deepseek_v31"); | ||
| registry.register_pattern("deepseek-v3-1", "deepseek_v31"); | ||
| registry.register_pattern("deepseek-v4", "deepseek_v4"); |
There was a problem hiding this comment.
When a DeepSeek V4 tokenizer is registered under an alias such as a label/namespace model id that does not contain deepseek-v4, the native renderer still selects V4 from config.json::architectures and now opens <think> by default, but response processing asks check_reasoning_parser_availability(..., &chat_request.model) and this registration only matches names containing deepseek-v4. In that alias case separate_reasoning has no parser and the model's reasoning is returned as normal content; tie parser selection to the V4 renderer or register/use the parser for the effective tokenizer instead of only this name pattern.
Useful? React with 👍 / 👎.
|
@key4ng please also take a look, thanks |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@model_gateway/src/routers/http/pd_router.rs`:
- Around line 219-280: Gate the entire thinking-translation logic in
apply_deepseek_v4_http_compat on is_deepseek_v4, returning before inspecting or
writing chat_template_kwargs.thinking for other models. Preserve the existing
DeepSeek V4 reasoning_effort remapping and translation behavior, and add a
regression test proving non-DeepSeek-V4 requests with reasoning_effort "none" or
explicit thinking remain unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0c1b5bb2-1952-49eb-be8b-1b1ea611231d
📒 Files selected for processing (12)
bindings/golang/src/utils.rscrates/protocols/src/chat.rscrates/reasoning_parser/src/factory.rscrates/tokenizer/src/encoders/deepseek_v4.rscrates/tokenizer/src/huggingface.rscrates/tokenizer/tests/deepseek_renderer_detection.rsmodel_gateway/src/routers/grpc/regular/processor.rsmodel_gateway/src/routers/grpc/regular/stages/chat/request_building.rsmodel_gateway/src/routers/grpc/regular/streaming.rsmodel_gateway/src/routers/grpc/utils/chat_utils.rsmodel_gateway/src/routers/grpc/utils/parsers.rsmodel_gateway/src/routers/http/pd_router.rs
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 486a61845f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@model_gateway/src/routers/http/deepseek_compat.rs`:
- Around line 102-115: Extend
deepseek_v4_http_compat_preserves_internal_override_and_maps_effort to assert
all documented V4 reasoning_effort normalizations: low, medium, and high must
each become high, while preserving the existing xhigh-to-max and non-V4
assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5c0f2b38-35f6-4835-9eed-2fca1edfb024
📒 Files selected for processing (4)
model_gateway/src/routers/http/deepseek_compat.rsmodel_gateway/src/routers/http/mod.rsmodel_gateway/src/routers/http/pd_router.rsmodel_gateway/src/routers/http/router.rs
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d386b6df0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
github-actions
Bot
added
the
documentation
df090a8 to
ff4df69
Compare
github-actions
Bot
removed
the
documentation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff4df69543
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let explicit_thinking = request | ||
| .get("thinking") |
There was a problem hiding this comment.
For DeepSeek V4 HTTP requests that use the new official thinking object, this shim reads it and adds chat_template_kwargs.thinking, but the original top-level thinking field is still forwarded to the SGLang HTTP worker. The file-level contract says this translates public controls into SGLang worker controls before forwarding, and the code already consumes unsupported off-signal reasoning_effort values for the same reason; leaving thinking in the payload can make otherwise valid DeepSeek requests fail or be misinterpreted by workers that only understand the translated kwarg. Remove it after reading it, including before the chat_template_kwargs.thinking override return path.
Useful? React with 👍 / 👎.
|
Hi @junliu-mde, this PR has merge conflicts that must be resolved before it can be merged. Please rebase your branch: git fetch origin main
git rebase origin/main
# resolve any conflicts, then:
git push --force-with-lease |
mergify
Bot
added
the
needs-rebase
Signed-off-by: Jun Liu <jun.c.liu@rakuten.com>
Signed-off-by: Jun Liu <jun.c.liu@rakuten.com>
…soning in messages - Gate all of apply_deepseek_v4_http_compat on the model check so other models' requests never receive a DeepSeek-only chat_template_kwargs thinking key or a remapped reasoning_effort. - Messages responses (non-streaming and streaming) now also separate reasoning when the template default turns thinking on (DeepSeek V4) with no explicit thinking config, instead of emitting the reasoning as text content. Signed-off-by: Jun Liu <jun.c.liu@rakuten.com>
Move the V4 translation out of PDRouter into a shared http/deepseek_compat module and call it from the regular Router's send_typed_request for /v1/chat/completions, so non-PD HTTP workers also receive the thinking kwarg projection and effort remap. Signed-off-by: Jun Liu <jun.c.liu@rakuten.com>
…HTTP workers none/minimal are the gateway's thinking-off signal, not official V4 levels; the worker only understands absent/high/max, so remove the field from the forwarded JSON once consumed. Signed-off-by: Jun Liu <jun.c.liu@rakuten.com>
… HTTP workers The shim translates it into chat_template_kwargs.thinking; leaving the original in the payload lets a worker that natively understands the official field contradict the translation (e.g. an explicit kwarg override). Remove it on every path, matching the off-signal reasoning_effort handling. Signed-off-by: Jun Liu <jun.c.liu@rakuten.com>
7d6e757 to
7df8476
Compare
mergify
Bot
removed
the
needs-rebase
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7df84764a9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| thinking: openai_protocol::chat::thinking_from_reasoning_effort( | ||
| request.reasoning_effort.as_deref(), | ||
| ), | ||
| thinking: request.thinking_preference(), |
There was a problem hiding this comment.
When a gRPC chat request targets another toggle-capable model such as Qwen or GLM and includes the new DeepSeek-specific thinking object, this unconditional preference is projected by ChatTemplateState::apply onto that model's own enable_thinking key, unexpectedly enabling or disabling its reasoning and changing a field that was previously ignored. The HTTP compatibility path explicitly no-ops for non-V4 models, so gate this protocol field by the effective DeepSeek V4 renderer/model while retaining the generic reasoning_effort mapping.
Useful? React with 👍 / 👎.
3 participants
Description
Problem
The gateway's DeepSeek V4 handling diverges from DeepSeek's official API in three ways:
enabled", and the V4 encoder'sthinking_modeis a required argument with the default supplied at the API layer. We reused the V3.2DefaultOffcontract, so every request without an explicit toggle got a chat-mode prompt (…<|Assistant|></think>) — the served model silently behaves unlikeapi.deepseek.comfor identical requests.thinkingrequest field. The official switch isthinking: {"type": "enabled" | "disabled"}(Create Chat Completion). Clients migrating from the official API had it swallowed into theotherflatten and ignored — a request explicitly disabling thinking still paid for reasoning tokens.reasoning_effortmapping was incomplete. Official semantics: real levels arehighandmax;low/mediummap tohigh,xhighmaps tomax. We only recognizedmaxandhigh, soxhigh(sent by e.g. Claude Code) silently lost the max-effort prompt preamble.Solution
ChatThinkingConfigonChatCompletionRequest: the official{"type": "enabled" | "disabled"}schema, strict (deny_unknown_fields) so malformed variants fail loudly instead of being ignored.ThinkingToggle::DefaultOnfor the V4 renderer (V3.2 keepsDefaultOff);derive_thinking_modetakes the renderer's official default as its fallback.chat_template_kwargstoggle >thinkingfield >reasoning_effortcompatibility mapping > renderer default. Both the prompt-building path (process_chat_messages_with_placeholders) and the reasoning-parser arming path (resolve_user_thinking, now taking the request) resolve identically, so the parser state always agrees with what the prefill actually contains.reasoning_effortnormalization in the V4 shim:xhigh → Max,low|medium|high → High, verbatim per the official mapping.deepseek_v4reasoning parser registration +deepseek-v4model-name pattern, so CLI/parser selection matches the served model instead of falling through.Behavior notes
thinkingschema rejects shapes the official API also rejects (booleans, Anthropic-stylebudget_tokens) that were previously silently ignored — now a 400.ThinkingConfig; an omitted config on a V4 model now inherits the model's official default-on rather than Anthropic's default-off.thinkingconfig. This covers DeepSeek V4 and also fixes the same<think>-tag leak for other default-on templates (Qwen3, GLM) on the Messages endpoint, matching the chat endpoint's long-standing arming semantics. An explicitDisabledconfig still opts out.maxeffort for agent traffic (Claude Code, OpenCode) — that's origin-side heuristics, out of scope for a passthrough gateway.Changes
crates/protocols/chat.rs:ChatThinkingConfig,thinkingfield,thinking_preference()accessor.crates/tokenizer/huggingface.rs: per-renderer thinking default (V4 → DefaultOn),xhigh/low/mediumeffort normalization;encoders/deepseek_v4.rssource comment now points at the DeepSeek-V4-Pro upstream.crates/reasoning_parser/factory.rs:deepseek_v4parser + pattern (shared helper withdeepseek_v31).model_gatewaygrpc:resolve_user_thinking(&request, tokenizer)signature; chat prompt building usesthinking_preference().Test Plan
thinkingdeserialization round-trip + rejection cases,thinking-vs-reasoning_effortprecedence, V4 factory/parser arming, per-renderer toggle defaults, effort normalization (incl.xhigh→ "Absolute maximum" preamble), end-to-end prompt assertions throughprocess_chat_messageswith a real V4-detected tokenizer dir.cargo test -p openai-protocol(83+126),-p reasoning-parser(75),-p llm-tokenizer(146 lib + integration incl.deepseek_renderer_detection),-p smg --lib routers::grpc::utils(34): all green.Summary by CodeRabbit
New Features
{type: enabled|disabled}with clear precedence over reasoning-effort inputs.xhighhandling (“absolute maximum” marker).Bug Fixes
Tests
/v1/chat/completionsHTTP compatibility behavior.