Fix reasoning_effort tier mapping and context-gate misreading - #704
Open
Flor1an-B wants to merge 2 commits into
Open
Fix reasoning_effort tier mapping and context-gate misreading#704Flor1an-B wants to merge 2 commits into
Flor1an-B wants to merge 2 commits into
Conversation
Two independent bugs in the same two functions:
1. DS4_THINK_HIGH never rendered a prefix at all (only DS4_THINK_MAX did,
using text that is actually the official "high" tier prefix per
deepseek-ai/DeepSeek-V4-Flash-0731's encoding/README.md). Every
reasoning_effort other than "max" silently rendered as official "low".
The real "max" prefix ("Beyond maximum...") was absent from the codebase
entirely. Fixed in both independent prompt-rendering paths: ds4.c's
chat_push_think_prefix() (CLI/engine) and ds4_server.c's
render_deepseek_chat_prompt_text() (server).
2. ds4_think_mode_for_context() downgraded "max"->"high" below a 384K
context threshold, citing DeepSeek's guidance as an input-context
requirement. The model card's actual text: "For the high and max
reasoning effort levels, we recommend a maximum output length of
384K tokens" -- an output-budget guideline, not a context-size
precondition. Turned into a documented no-op rather than removed,
since four ds4_server.c call sites use it and
ds4_think_max_min_context() may still be useful for client-side
output-length advice.
Adds test_render_think_high_prompt_prefix() (there was previously zero
coverage of the "high" tier ever rendering anything), fixes the now-wrong
context-gate assertion, and adds a mutual-exclusivity check to the
existing "max" prompt test.
make ds4_test passes clean against the full suite plus these changes.
Issue antirez#660: a user setting reasoning_effort to "low" saw the model reason for 9k tokens on a simple task and never converge -- "it became qwen3.5 all of a sudden." Community workaround suggestions (try "max" instead) only underscored that "low" was doing something wrong, not that "max" was doing something right. Root cause, confirmed against DeepSeek-V4's own encoding spec (encoding/README.md, "Reasoning Effort" section): the spec defines exactly three levels -- "low" (default, prompt prefix "none"), "high", and "max". DS4's parse_reasoning_effort_name() collapsed "low" (and "medium"/"minimal") into DS4_THINK_HIGH, the same enum value as an explicit "high" request, with an explicit comment acknowledging the simplification: "DS4 only exposes HIGH and MAX above zero, so minimal collapses to the smallest non-zero level (HIGH)." So a client asking for "low" got the *most* elaborate non-max prompt DS4 has ("You MUST be very thorough... rigorously stress-testing your logic against all potential paths, edge cases, and adversarial scenarios...") instead of the model's own default behavior with no extra prompting at all -- a plausible direct cause of runaway reasoning on tasks that don't need it. Fix: add DS4_THINK_LOW as a genuine fourth ds4_think_mode value -- "thinking enabled, no prompt prefix" -- distinct from DS4_THINK_NONE (thinking disabled entirely). Wired through every switch/branch over the enum (grep confirmed a small, complete list): ds4_think_mode_enabled(), ds4_think_mode_name(), ds4_glm_reasoning_effort_text(), chat_push_think_prefix() and its ds4_server.c equivalent in render_deepseek_chat_prompt_text() (both already no-op for an unmatched mode, so they needed no code change, just a comment explaining why), think_mode_from_enabled() (now a straight pass-through instead of collapsing non-NONE/non-MAX to HIGH), and parse_reasoning_effort_name() ("low" and "minimal" now map to LOW; "medium"/"xhigh" -- not official DeepSeek values -- still map to HIGH absent more specific guidance). The default reasoning_effort when a client sends none at all is untouched (still HIGH); this only changes what an *explicit* "low"/"minimal" request actually does. Tested: - Full existing ds4_test suite passes (compiler catches any missed switch case over the enum; there were none beyond the ones listed above). - Updated test_reasoning_effort_mapping for the new low/minimal -> LOW mapping, plus direct checks of ds4_think_mode_enabled()/ think_mode_from_enabled() on the new value. - New test_render_think_low_prompt_prefix mirrors the existing high/max prompt-prefix regression tests: no prefix text, but the rendered prompt still ends in the thinking-enabled "<think>" (not "</think>"). - Live A/B on the production Flash quant, same prompt, temp=0: reasoning_effort "low" and "high" now produce genuinely different reasoning_content (were byte-identical prompts before this fix, so would have produced identical output). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Follows up on #635 (@kernelzeroday, @zom-2018) and #653 (@rell666) — this is the patch discussed there, posted as a comment first (#635 (comment)) and opened as a PR per @darkbasic's question.
Two independent bugs, same two functions
DS4_THINK_HIGHnever rendered a prefix at all. OnlyDS4_THINK_MAXdid, using text that is actually the official "high" tier prefix perdeepseek-ai/DeepSeek-V4-Flash-0731'sencoding/README.md(verified verbatim against the primary source). Everyreasoning_effortother than"max"silently rendered as official "low". The real"max"prefix ("Beyond maximum — exhaustive, relentless, and uncompromising...", note the em dash U+2014) was absent from the codebase entirely — unreachable via the API regardless of setting. Fixed in both independent prompt-rendering paths that had this bug:ds4.c'schat_push_think_prefix()(CLI/engine) andds4_server.c'srender_deepseek_chat_prompt_text()(server).ds4_think_mode_for_context()downgraded"max"→"high"below a 384K context threshold, citing DeepSeek's guidance as an input-context requirement. The model card's actual text: "For thehighandmaxreasoning effort levels, we recommend a maximum output length of 384K tokens." That's an output-budget guideline, not a context-size precondition. Turned into a documented no-op rather than removed outright, since fourds4_server.ccall sites use it andds4_think_max_min_context()may still be useful for client-side output-length advice.Testing
test_render_think_high_prompt_prefix()— there was previously zero test coverage of the "high" tier ever rendering anything.test_reasoning_effort_mapping()."max"prompt test.make ds4_testpasses clean against the full existing suite plus these changes.reasoning_effort: highand: maxnow produce visibly distinct prompts;: maxno longer collapses to the "high" text below 384K context.Requirements
Update: a real DS4_THINK_LOW tier (issue #660)
Found while looking for other reasoning_effort-related community reports to help with: issue #660 ("New 0731 iq2 thinks forever!") describes
reasoning_effort: "low"producing 9k tokens of runaway reasoning.Root cause, again verified against DeepSeek-V4's own encoding spec (
encoding/README.md, "Reasoning Effort" section): it defines exactly three levels —"low"(the default, prompt prefix "none"),"high", and"max". DS4'sparse_reasoning_effort_name()collapsed"low"(and"medium"/"minimal") into the sameDS4_THINK_HIGHenum value as an explicit"high"request — the code even had a comment acknowledging this: "DS4 only exposes HIGH and MAX above zero, so minimal collapses to the smallest non-zero level (HIGH)." So a client asking for"low"got the most elaborate non-max prompt DS4 has, instead of the model's own default behavior with no extra prompting — a very plausible direct cause of runaway reasoning on tasks that don't need it.Added
DS4_THINK_LOWas a genuine fourthds4_think_modevalue ("thinking enabled, no prompt prefix", distinct fromDS4_THINK_NONEwhich disables thinking entirely) and wired it through every switch/branch over the enum. The defaultreasoning_effortwhen a client sends none at all is untouched (still high); this only fixes what an explicit"low"/"minimal"request does.Tested the same way as the rest of this PR: full suite passes, new regression test (
test_render_think_low_prompt_prefix) plus updatedtest_reasoning_effort_mapping, and a live A/B on the production Flash quant confirmingreasoning_effort: "low"and"high"now produce genuinely differentreasoning_content(they were byte-identical prompts before this fix).