fix(core): stop forcing DeepSeek temperature - #9879
Conversation
Let DeepSeek reasoning models use their model default temperature so deterministic sampling does not amplify repeated thinking loops. Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR!
Moving on to code review. 🔍 中文说明感谢贡献！
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewMy independent take before reading the diff: make the DeepSeek provider stop forcing a temperature — return the same empty default config the base provider and DashScope already return — and update the one test that pins the old value. That is exactly what this PR does, and there's nothing extra in it. What I verified in the code:
No blockers, no convention violations. The diff is the minimal change for the stated goal. The load-bearing part of this change is where the temperature value comes from at request-build time: sequenceDiagram
participant P1 as User samplingParams config
participant P2 as Per-request config
participant P3 as DeepSeek provider default
participant P4 as buildGenerateContentConfig
participant P5 as Wire request body
P4->>P1: read temperature
P4->>P2: read temperature
P4->>P3: read temperature
Note over P4: first defined wins - user config, then request, then default
P4->>P5: emit temperature only when one is defined
Files changed (2)
Testing evidenceThis is an unattended CI run — I did not build or execute any PR code. Evidence below is the PR's own CI, fetched via the API at the reviewed commit. Currently green: Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Static review settles the merge priority (deterministic code path), and the updated unit test pins the new default, but the wire-level consequence — DeepSeek request bodies no longer carrying 中文说明代码审查：我在看 diff 之前的独立想法就是——让 DeepSeek provider 不再强制 temperature，返回与基类和 DashScope 相同的空默认配置，并更新锁定旧值的测试。PR 正是这么做的，没有任何多余改动。 已在代码中核实：
无阻塞项，无规范违规。diff 是达成目标的最小改动。 测试证据：这是无人值守 CI 运行，未构建或执行任何 PR 代码，以上证据来自 API 拉取的 PR 自身 CI。目前安全类检查（Secret scan、依赖 CVE 审计）、Desktop Shell（ubuntu/windows）、precheck 均绿；单测套件 沙箱验证可坐实 wire 层结论： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, minimal fix for an observed bug with the right test pin; holding back the last point only because the unit suite hasn't landed green on this commit yet and the loop-fix half of the claim rests on the reporter's account. Stepping back: this PR does exactly what I would have done — stop overriding the model's sampling default, match what the base provider and DashScope already do, and fix the one test that pinned the old value. Nothing extra, nothing missing. The problem is real and observed (#9765's cross-client comparison is decent evidence, and greedy decoding on reasoning models looping is a well-known failure mode), and the direction matches where the repo is already heading — the reasoning-effort design doc explicitly wants sampling params off DeepSeek thinking requests. One thing a maintainer should see before merge, stated plainly: the author has ~20 open PRs, nearly all created in a single burst today. That doesn't taint this PR — judged on its own merit it's a legitimate, well-scoped fix — but it's exactly the situation where each PR needs to stand on its own evidence rather than momentum, so I'm keeping the bar where it belongs. Verdict: approve — but the unit suite 中文说明置信度：4/5 —— 对一个已观测到的 bug 的干净、最小修复，测试锚点正确；保留最后 1 分仅因为该 commit 的单测套件尚未跑绿，且"循环消失"这半结论依赖报告者的陈述。 整体来看：这个 PR 的做法与我的独立方案完全一致——不再覆盖模型采样默认值，与基类 provider 和 DashScope 保持一致，并修正唯一锁定旧值的测试。没有多余改动，也没有遗漏。问题是真实且已观测的（#9765 的跨客户端对比是不错的证据，贪心采样导致 reasoning 模型循环也是已知的失效模式），方向也与仓库现有路线一致——reasoning-effort 设计文档明确希望 DeepSeek thinking 请求不携带采样参数。 一点需要 maintainer 在合并前看到的情况，直说：该作者约有 20 个 open PR，几乎全部在今天的同一波提交中创建。这并不影响本 PR——按自身价值评判，它是一个合理、范围清晰的修复——但这正是每个 PR 都必须靠自身证据站住脚、而不是靠数量势能过关的场景，因此评审标准保持在应有高度。 结论：批准——但该 commit 的单测 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: reverse audit — stopped before round 1 by the review time budget.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：反向审计——评审时间预算不足，未能开始第 1 轮。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| override getDefaultGenerationConfig(): GenerateContentConfig { | ||
| return { | ||
| temperature: 0, | ||
| }; | ||
| return {}; | ||
| } |
There was a problem hiding this comment.
[Suggestion] This override now returns {} — byte-for-byte identical to what the base class DefaultOpenAICompatibleProvider.getDefaultGenerationConfig() already returns (provider/default.ts:219), so inheritance yields identical behavior and the override — along with its dedicated test, which now asserts the base behavior as if it were DeepSeek-specific — is redundant; five of the seven subclasses already inherit {} without overriding. The cost is maintenance, not runtime: the override and test read as if DeepSeek carried a distinct default generation config, and if the base default ever gains a real value, DeepSeek silently opts out here and diverges from every other subclass with no visible diff — re-introducing the exact bug class this PR fixes. Either delete the override, its test block, and the now-unused GenerateContentConfig import, or — if the override deliberately pins {} as a regression guard for #9765 — say that why in a one-line comment, which is currently invisible:
override getDefaultGenerationConfig(): GenerateContentConfig {
// Deliberately empty: forcing temperature 0 caused repetitive thinking
// loops on DeepSeek reasoning models (#9765).
return {};
}中文说明
[Suggestion] 这个 override 现在返回 {}——与基类 DefaultOpenAICompatibleProvider.getDefaultGenerationConfig() 已经返回的内容（provider/default.ts:219）逐字节完全相同，因此继承即可得到完全一致的行为；该 override 连同它的专属测试（现在把基类行为当作 DeepSeek 特有行为来断言）都是冗余的——七个子类中已有五个不做 override、直接继承 {}。代价在维护层面而非运行时：这个 override 和测试读起来像是 DeepSeek 拥有独立的默认 generation config；如果基类默认值将来加入了真实的值，DeepSeek 会通过这里悄悄退出继承、在没有任何可见 diff 的情况下与其他所有子类产生差异——重新引入本 PR 所修复的那类 bug。可以删除该 override、对应的测试代码块以及因此不再使用的 GenerateContentConfig import；或者——如果这个 override 是有意将 {} 固定下来、作为 #9765 的回归防护——请用一行注释说明这个原因，目前这一点完全看不出来。
— qwen3.8-max via Qwen Code /review (v0.22.0)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
Maintainer verification — built and run locally, real stackI built the merge tree ( The A/B is a single-variable swap inside one tree: flip the four-line hunk in Verdict: the PR's claim holds exactly as written, and I'd merge it. One consequence is bigger than the PR body suggests, and it's a conscious call rather than a defect — see Blast radius below. 1. Core claim — verified at the wireDiff of the two actual "stream_options": {
"include_usage": true
},
- "temperature": 0,
"tools": "<27 tool schemas elided>"
}
Three things this pins that the unit test alone cannot:
2. The migration note in the PR body is trueAn explicit user temperature still reaches the wire under HEAD, including the deterministic
3. Tests, mutation, lint
4. Blast radius — worth one conscious decision before merge
I walked every What that means in numbers: DeepSeek documents its API default as I think that trade is still worth taking (it restores parity with every other provider, and a looping model is worse than a warmer one), but it's the kind of thing that should be merged knowingly rather than discovered later. If a maintainer disagrees, #9765's option 2 — keep 5. What I could not verify
6. Real session, end to endAn interactive TUI turn against the
Environment & how to reproduce
# 1. capture server: records every request body verbatim, answers with a valid SSE stream
node wire-capture-server.mjs # 127.0.0.1:8879, writes capture.jsonl
# 2. drive the bundled CLI at it — DeepSeek provider is selected on the model name
env -i PATH="$PATH" HOME="$ISOLATED_HOME" QWEN_HOME="$ISOLATED_HOME/.qwen" \
OPENAI_API_KEY=sk-test OPENAI_BASE_URL=http://127.0.0.1:8879/v1 \
OPENAI_MODEL=deepseek-v4-flash \
node dist/cli.js --yolo -p "Say the word ready and nothing else."
# 3. read the temperature off the recorded bytes
python3 -c "import json;[print(json.loads(json.loads(l)['raw']).get('temperature','ABSENT')) for l in open('capture.jsonl')]"
# 4. genuine hostname leg
docker run --rm --add-host api.deepseek.com:127.0.0.1 -v "$PWD/dist:/app/dist:ro" ... node:24-bookworm
# with OPENAI_BASE_URL=http://api.deepseek.com/v1中文说明维护者验证 —— 本地真实环境构建并运行我构建了合并树（ A/B 是同一棵树内的单变量切换：翻转 结论：PR 的论断完全成立，我支持合入。 有一处影响面比 PR 描述里写的要大，那属于需要明确拍板的取舍，不是缺陷 —— 见第 4 节。 1. 核心论断 —— 链路层已证实两次真实
这三点是单测本身锁不住的：
2. PR 描述里的迁移说明属实HEAD 下用户显式配置的 temperature 仍然会上链路，包括老用户想恢复确定性时会设的 3. 测试、变异、lint
4. 影响面 —— 合并前值得明确拍一次板
我把所有 换算成具体数字：DeepSeek 官方文档写明 API 默认 我认为这个交换仍然值得（它让 DeepSeek 与其它所有 provider 对齐，而且模型死循环比采样偏热更糟），但这属于应当知情合入、而不是事后才发现的事。若维护者不认同，#9765 的方案二 —— thinking 关闭时保留 5. 我没有验证到的部分
6. 端到端真实会话HEAD bundle 下针对 环境： macOS 26.6.2 arm64 / Node v24.18.1 / npm 11.16.0；容器侧 |
3 participants
What this PR does
Removes the DeepSeek OpenAI-compatible provider's hard-coded default
temperature: 0so DeepSeek models can use their model/server default sampling behavior unless the user explicitly configures a temperature.Why it's needed
Issue #9765 reports that forcing greedy sampling by default can make DeepSeek reasoning models such as
deepseek-v4-flashfall into repetitive thinking loops. The provider should not override the model default for all DeepSeek requests; explicit user overrides via generation config remain supported by the normal request-building path.Reviewer Test Plan
How to verify
Run the targeted DeepSeek provider tests, Prettier on the touched files, and the core package typecheck.
Evidence (Before & After)
Before:
DeepSeekOpenAICompatibleProvider.getDefaultGenerationConfig()returned{ temperature: 0 }and a regression test expecting no forced temperature failed.After: the provider returns
{}, leaving default sampling to the model/server unless the user explicitly sets a temperature.Tested on
Environment (optional)
Local Node/npm workspace on macOS.
Risk & Scope
generationConfig.samplingParams.temperatureexplicitly.Linked Issues
Fixes #9765.
中文说明
What this PR does
移除 DeepSeek OpenAI-compatible provider 中硬编码的默认
temperature: 0，让 DeepSeek 模型在用户没有显式配置 temperature 时使用模型/服务端默认采样行为。Why it's needed
#9765 报告了 DeepSeek reasoning 模型（例如
deepseek-v4-flash）在默认贪心采样下容易进入重复 thinking loop。provider 不应该对所有 DeepSeek 请求强制覆盖模型默认值；用户显式配置的 temperature 仍会通过现有请求构造路径生效。Reviewer Test Plan
How to verify
运行 DeepSeek provider 目标测试、改动文件 Prettier 检查、core 包 typecheck。
Evidence (Before & After)
Before：
DeepSeekOpenAICompatibleProvider.getDefaultGenerationConfig()返回{ temperature: 0 }，期望不强制 temperature 的回归测试失败。After：provider 返回
{}，默认采样交给模型/服务端，除非用户显式设置 temperature。Tested on
Environment (optional)
macOS 本地 Node/npm 工作区。
Risk & Scope
generationConfig.samplingParams.temperature。Linked Issues
Fixes #9765.