fix(reasoning): support DeepSeek V4 Flash 0731 effort encoding - #2025
fix(reasoning): support DeepSeek V4 Flash 0731 effort encoding#2025key4ng wants to merge 6 commits into
Conversation
Signed-off-by: key4ng <rukeyang@gmail.com>
Signed-off-by: key4ng <rukeyang@gmail.com>
Signed-off-by: key4ng <rukeyang@gmail.com>
github-actions
Bot
added
tokenizer
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe changes standardize reasoning-effort handling across protocols, tokenizers, parsers, and gRPC routes. They add DeepSeek V4 support, preserve native template precedence, update reasoning propagation, and consolidate modern Responses usage serialization. ChangesReasoning effort and DeepSeek V4
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ResponsesConversion
participant ChatTemplateParams
participant HuggingFaceTokenizer
participant DeepSeekV4Encoder
Client->>ResponsesConversion: submit reasoning_effort
ResponsesConversion->>ChatTemplateParams: forward public and native effort
ChatTemplateParams->>HuggingFaceTokenizer: apply chat template
HuggingFaceTokenizer->>DeepSeekV4Encoder: resolve effort precedence
DeepSeekV4Encoder-->>HuggingFaceTokenizer: select reasoning prefix
HuggingFaceTokenizer-->>Client: render encoded prompt
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thorough review of all 17 changed files. No issues found — approving.
What changed:
thinking_from_reasoning_effortnow explicitly enables thinking for all known non-noneOpenAI levels (previously onlynone/minimalwere handled, others returnedNone).ResponseReasoningParam.effortno longer defaults toMedium— an empty reasoning object leaves effort unspecified.- DeepSeek V4 renderer gains 0731 effort encoding with three native buckets (
low/high/max) mapped from the six OpenAI levels. resolve_deepseek_v4_reasoning_effortcleanly separates three effort sources with correct precedence: explicit native (template_reasoning_effort) > merged kwargs (template_kwargs) when no public effort > public OpenAI effort viafrom_openai.response_completed_usageunifies the streaming/non-streaming/tool-loop wire format to always includeinput_tokens_detailsandoutput_tokens_details.responses_to_chatnow setsseparate_reasoning: trueandstream_reasoning: true, fixing reasoning content bleeding into output text.- Harmony builder maps
xhigh/maxto its highest supported effort, consistent across Chat and Responses paths (verified bychat_and_responses_reasoning_efforts_build_the_same_system_message).
Verified:
ReasoningEffortserde:XHighcorrectly renames toxhigh(notx_highfromsnake_case), confirmed by round-trip test.from_openai("none")→Lowis harmless sincethinking_from_reasoning_effort("none")→Some(false)disables thinking, so the effort prefix is never emitted.factory.rschange fromfrom_filetofrom_file_with_chat_template(_, None)is safe: auto-detect is a fallback for non-standard extensions, embedded templates intokenizer_config.jsonstill load, and V4 uses a native renderer that bypasses Jinja entirely.- All behavioral changes are intentional and covered by updated/new tests.
0 🔴 Important · 0 🟡 Nit · 0 🟣 Pre-existing
![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 `@crates/tokenizer/src/encoders/deepseek_v4.rs`:
- Around line 35-42: Update resolve_deepseek_v4_reasoning_effort to distinguish
an omitted effort from an explicitly supplied unsupported value: preserve the
existing from_openai mapping for recognized names, but return a validation error
instead of treating None as omitted when a value such as "turbo" is provided.
Adjust the corresponding test to expect rejection rather than successful
rendering without an effort prefix.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 675c2244-ed02-4482-a498-cb92dfcb1698
📒 Files selected for processing (17)
crates/protocols/src/chat.rscrates/protocols/src/responses.rscrates/reasoning_parser/src/factory.rscrates/tokenizer/src/chat_template.rscrates/tokenizer/src/encoders/deepseek_v4.rscrates/tokenizer/src/factory.rscrates/tokenizer/src/huggingface.rscrates/tokenizer/tests/deepseek_renderer_detection.rsmodel_gateway/src/routers/grpc/common/responses/mod.rsmodel_gateway/src/routers/grpc/common/responses/streaming.rsmodel_gateway/src/routers/grpc/harmony/builder.rsmodel_gateway/src/routers/grpc/regular/responses/common.rsmodel_gateway/src/routers/grpc/regular/responses/conversions.rsmodel_gateway/src/routers/grpc/regular/responses/streaming.rsmodel_gateway/src/routers/grpc/utils/chat_utils.rsmodel_gateway/src/routers/grpc/utils/parsers.rsmodel_gateway/src/workflow/tokenizer_registration.rs
| /// Map OpenAI-compatible effort names into the three native 0731 buckets. | ||
| pub fn from_openai(value: &str) -> Option<Self> { | ||
| match value { | ||
| "none" | "minimal" | "low" => Some(Self::Low), | ||
| "medium" | "high" => Some(Self::High), | ||
| "xhigh" | "max" => Some(Self::Max), | ||
| _ => None, | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🔴 Important: Reject unsupported public effort values.
Line 36 returns None for an unsupported public value such as "turbo". resolve_deepseek_v4_reasoning_effort then treats that result as omitted effort, so the renderer emits no effort prefix instead of rejecting the request. Keep from_openai as a mapper if needed, but return an error when a supplied public value does not map to a DeepSeek V4 bucket. Update the test that expects "turbo" to succeed.
As per coding guidelines, do not silently fall back to None or a default when configuration validation should fail loudly.
🤖 Prompt for 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.
In `@crates/tokenizer/src/encoders/deepseek_v4.rs` around lines 35 - 42, Update
resolve_deepseek_v4_reasoning_effort to distinguish an omitted effort from an
explicitly supplied unsupported value: preserve the existing from_openai mapping
for recognized names, but return a validation error instead of treating None as
omitted when a value such as "turbo" is provided. Adjust the corresponding test
to expect rejection rather than successful rendering without an effort prefix.
Source: Coding guidelines
Signed-off-by: key4ng <rukeyang@gmail.com>
Signed-off-by: key4ng <rukeyang@gmail.com>
| ) -> Option<bool> { | ||
| resolve_thinking_pref( | ||
| extract_thinking_from_kwargs(kwargs, tokenizer), | ||
| extract_template_effort_thinking(kwargs, tokenizer), |
There was a problem hiding this comment.
🟡 Nit: The doc comment on resolve_user_thinking (lines 75-77) is now stale — it describes a two-tier precedence (explicit kwarg → reasoning_effort) but the implementation now has three tiers with this new middle layer. The internal resolve_thinking_pref doc was updated but the public-facing one was not.
Consider updating lines 75-77 to:
/// Resolve the user's effective thinking preference, honoring an explicit
/// template thinking kwarg first, then a native template effort for renderers
/// that support it, then the protocol-level OpenAI `reasoning_effort` mapping.
|
Live B300 parity validation completed against the original Compared Chat Completions requests using: {"reasoning_effort":"low|max"}and: {"chat_template_kwargs":{"reasoning_effort":"low|max"}}Results with identical prompts and deterministic decoding:
For each effort, the two request shapes produced identical SHA-256 hashes for both The live test caught and fixed two separate gaps:
Both paths now use the same precedence: explicit Focused verification: 19/19 DeepSeek renderer tests, 10/10 DeepSeek V4 encoder tests, 51/51 Responses tests, parser-helper tests, rustfmt, diff check, and tokenizer-library Clippy all pass. |
Signed-off-by: key4ng <rukeyang@gmail.com>
1 participant
Description
Problem
DeepSeek V4 Flash 0731 defines a three-level
low/high/maxreasoning-effort encoding, but SMG's native Rust renderer did not consistently receive the OpenAI-compatible Chat Completions and Responses API effort fields. DeepSeek V4 models were also not automatically selecting the appropriate reasoning parser, and regular Responses conversion could merge reasoning into output text or lose modern usage details on some completion paths.Solution
Map the official OpenAI request fields into the native DeepSeek V4 renderer while keeping an explicit native
chat_template_kwargs.reasoning_effortoverride. Use the 0731 effort encoding for the DeepSeek V4 renderer, keep thinking disabled when no effort is requested, and automatically register the DeepSeek V4 reasoning parser. Preserve separated reasoning and modern Responses API usage details in non-streaming, streaming, and tool-loop completion paths.Changes
reasoning_effortand Responses APIreasoning.effort.low,high, andmaxbuckets.chat_template_kwargs.reasoning_effortvalue and reject invalid native buckets.Test Plan
cargo test -p smg responses --lib— 51 passed.cargo test --workspace -- --skip dsml_one_shot_and_incremental_decode_match --skip test_router_with_tracing— passed, including unit, integration, and doc tests.cargo +nightly fmt --all -- --check— passed.pre-commit run --all-files— every hook passed except Clippy, which could not start because the local macOS environment has nopkg-config/OpenCV installation.dsml_one_shot_and_incremental_decode_matchis a pre-existing untracked debug probe requiringDSV4_TOKENIZER.test_router_with_tracingwas run twice independently and consistently received zero OTLP spans; this branch does not modify tracing or observability code.Live B300 validation used the original
deepseek-ai/DeepSeek-V4-Flashweights with TP2 on GPUs 4–5. Because the deployed SMG 1.9.0 image predates this patch, the benchmark injected the exact official 0731 prefixes that the new Rust unit tests verify:The original Flash weights clearly respond to the 0731 effort prompts. Reasoning length is task-dependent rather than strictly monotonic, so these prefixes should be understood as instructions, not hard token budgets.
Checklist
cargo +nightly fmtpassescargo clippy --all-targets --all-features -- -D warningspasses (blocked locally by missingpkg-config/OpenCV)