fix(cli): enable ToolSearch by default for DeepSeek - #8331
Conversation
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterFull-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
|
Thanks for the PR! Template looks good ✓ Problem: Real and well-evidenced. #4069 added a DeepSeek-specific ToolSearch disable because the old reveal path rewrote the provider tool list and invalidated prefix caches. #8276 fixes that root cause by keeping deferred schemas in conversation content via a stable proxy declaration — the A/B attached there measured 0.00% → 96.97% post-reveal cache hit rate across 30 paired runs. Once #8276 lands, the model-specific workaround here is genuinely dead weight. Direction: Aligned — removing model-specific defaults simplifies the config path and lets DeepSeek users benefit from deferred tool discovery. One important note: #8276 is still open. This PR explicitly depends on it and must not merge first. The code is correct given that dependency, but merging order matters here. Size: 21 production logic lines (17 config.ts, 2 settingsSchema.ts, 2 i18n.tsx), 48 test lines, 4 docs/schema lines. Well under any threshold. Core paths touched ( Approach: Minimal and focused. The diff does exactly one thing: removes the Risk: No elevated risk signals — no high-risk paths matched. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题: 真实且有充分证据。#4069 为 DeepSeek 添加了特定的 ToolSearch 禁用逻辑,因为旧的 reveal 路径会重写 provider 工具列表并使前缀缓存失效。#8276 通过稳定代理声明修复了根本原因——其 A/B 测试在 30 次配对运行中测得 reveal 后缓存命中率从 0.00% 提升至 96.97%。#8276 落地后,此处的模型特定规避方案确实不再需要。 方向: 对齐——移除模型特定默认值简化了配置路径,让 DeepSeek 用户受益于延迟工具发现。重要提示:#8276 仍未合并。 此 PR 明确依赖它且不能先于它合并。代码在满足该依赖的前提下是正确的,但合并顺序很重要。 规模: 21 行生产逻辑(config.ts 17 行、settingsSchema.ts 2 行、i18n.tsx 2 行),48 行测试,4 行文档/schema。远低于任何阈值。触及核心路径( 方案: 最小且聚焦。diff 只做一件事:移除 风险: 无升级风险信号——未匹配高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Independent proposal: Given the goal (remove the DeepSeek-specific ToolSearch disable after #8276 fixes the cache invalidation), I would: (1) delete the Code review: Clean. The 16-line removal in Testing evidence (CI):
Ubuntu unit tests and web-shell E2E pass. Windows/macOS tests and integration tests are skipped (typical for fork PRs). No failures. The unit tests pin the config behavior (tool_search no longer auto-denied for any model). The cache-safety claim — that removing the disable won't regress DeepSeek prefix caching — rests entirely on #8276's stable-proxy mechanism, which is still open. Sandboxed verification ( 中文说明独立方案: 给定目标(在 #8276 修复缓存失效后移除 DeepSeek 特定的 ToolSearch 禁用),我会:(1) 删除 代码审查: 干净。 测试证据(CI): Ubuntu 单元测试和 web-shell E2E 通过。Windows/macOS 测试和集成测试被跳过(fork PR 常见)。无失败。 单元测试固定了配置行为(tool_search 不再对任何模型自动拒绝)。缓存安全性声明——移除禁用不会导致 DeepSeek 前缀缓存回退——完全依赖于 #8276 的稳定代理机制,而该 PR 仍未合并。沙箱验证( — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 3/5 — clean, minimal code that does exactly what it says; deferred because the dependency #8276 hasn't landed and approving now would greenlight a merge that regresses DeepSeek prefix caching. The code itself is about as clean as a removal PR gets. The regex, the comment block, the two-branch conditional — all gone, replaced by one guard. Tests collapse from five to one parametrized case. Descriptions updated consistently across four surfaces. My independent proposal matched the PR's approach exactly; I didn't find a simpler path it missed. The reason this isn't a 5/5 is not the code — it's the merge dependency. #8276 ("preserve prompt cache across deferred tool discovery") is still open. This PR removes the DeepSeek-specific cache protection because #8276 replaces it with a provider-agnostic mechanism. Merge this first and DeepSeek users lose prefix-cache stability with no replacement. The PR body says this plainly ("must not merge first"), but GitHub doesn't enforce merge ordering — an approval here is a green light. Deferring to a maintainer to coordinate the merge order with #8276. Once #8276 lands, this is ready to go. 中文说明置信度:3/5 — 代码干净、最小化,完全做到了它所说的;推迟是因为依赖的 #8276 尚未合并,现在批准会放行一个导致 DeepSeek 前缀缓存回退的合并。 代码本身几乎是一个移除 PR 的典范。正则、注释块、双分支条件——全部移除,替换为一个守卫。测试从五个合并为一个参数化用例。描述在四个界面一致更新。我的独立方案与 PR 的方案完全一致;没有找到它遗漏的更简路径。 不是 5/5 的原因不在于代码——而在于合并依赖。#8276("跨延迟工具发现保持提示词缓存")仍未合并。此 PR 移除 DeepSeek 特定的缓存保护,因为 #8276 用 provider 无关的机制替代了它。先合并此 PR 会让 DeepSeek 用户失去前缀缓存稳定性且没有替代方案。PR 正文对此说明清楚("不能先于它合并"),但 GitHub 不强制合并顺序——此处的批准就是绿灯。 — Qwen Code · qwen3.8-max-preview Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max-preview via Qwen Code /review
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 332 passed · 0 failed · 332 total 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:332 通过 · 0 失败 · 332 总计 Verification reportPR #8331 Deep Verification ReportVerdict: 中文摘要结论: A/B 结论:中心声明已证实。在 BASE ( Findings:无阻塞性问题。一个注意事项:PR 声明依赖 #8276(稳定代理声明机制),当前 base 中未包含该机制(见 Not covered)。 未覆盖范围:#8276 依赖的合并顺序无法在此环境验证;真实 DeepSeek API 下的工具选择质量未测试(无凭证);threshold 行为未单独 A/B(代码路径未被本 PR 修改)。 Central claim + A/B tableCentral claim: Model identity no longer changes the ToolSearch default — DeepSeek models get ToolSearch enabled by default, same as all other models. Secondary claims:
A/B load-bearing proofThe PR's own
HEAD: 5/5 pass. BASE: 3/5 fail — exactly the 3 DeepSeek models flip from auto-denied to not-denied. The Evidence: Vacuity checkReverted the key hunk in Targeted gateFull Description consistencyAll four description locations were updated consistently:
The EN i18n section has no Residual "KV cach" / "prefix" matches in the changed files are all in the unchanged
FindingsNo blocking findings. Not covered
MethodologyEnvironment: A/B: base worktree created at Vacuity: key hunk reverted in-place via Schema regeneration: Raw logs: vitest output captured via Evidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
@qwen-code /review |
| _Qwen Code review request accepted. Review is queued in [workflow run](https://github.com/QwenLM/qwen-code/actions/runs/30729640117)._ |
Local real-environment verification (maintainer)Verified this PR end-to-end on a local build with a real DeepSeek API session ( Environment: macOS 26.6, Node v24.18.1 · PR head Unit tests
Results
Case A ≠ Case C with identical settings and identical model is exactly this PR's change: model identity no longer flips the ToolSearch default. Case B confirms the explicit opt-out still produces the old eager-declaration behavior. Wire-level evidence (proxy log of provider requests)Case A (PR, default enabled, Case A′ (PR, all defaults) — 10% preload budget fires, deferred tools declared upfront and Case B (PR, End-to-end on-demand discovery (real model)Asked the real Screenshots: /tools listing per case (model-echoed case labels on screen)Case A — PR build, default settings: Case B — PR build, Case C — baseline main, same default settings as Case A: Caveats for merging
Functionally this PR does exactly what it says. LGTM once #8276 is merged. 中文版本(Chinese version)本地真实环境验证(维护者)在本地构建上用真实 DeepSeek API 会话( 环境: macOS 26.6,Node v24.18.1 · PR head 单元测试
结果
用例 A 与用例 C 设置相同、模型相同而结果不同,正是本 PR 的改动:模型身份不再改变 ToolSearch 默认值。用例 B 确认显式关闭仍会回到旧的全量声明行为。 Wire 层证据(代理记录的 provider 请求)用例 A(PR,默认启用, 用例 A′(PR,全默认)—— 10% 预载预算生效,延迟工具预先声明,同时 用例 B(PR, 端到端按需发现(真实模型)让真实的 合并注意事项
功能上本 PR 与描述完全一致。待 #8276 合并后 LGTM。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
Follow-up: measured cache-hit impact on the current PR head (old reveal path)Supplementing the verification above with real Mid-session reveal does invalidate the prefix from the insertion point (old path)One interactive session, four turns,
The reveal rewrites the provider tool array, so everything after the insertion point is invalidated. Scope of the risk is narrow
One more observation: a brand-new session's first request already showed ConclusionAs long as #8276 merges first (as the PR description requires), this change does not degrade cache hit rate — and defaults-path users see no behavioral difference at all. If this PR were merged alone, DeepSeek users with over-budget tool catalogs would pay a one-time prefix invalidation (~5k+ uncached tokens in this measurement, potentially more depending on insertion point) per newly discovered tool. Recommend a merge-order guard: keep this in draft or blocked until #8276 lands. 中文版本(Chinese version)补充:当前 PR head(旧 reveal 路径)上实测的缓存命中影响用同一日志代理抓取 DeepSeek API 真实 旧路径下 mid-session reveal 确实会从插入点起作废前缀单个交互式会话四个回合,
reveal 会重写 provider 工具数组,插入点之后的前缀全部作废。本次 风险范围很窄
另一个观察:全新会话首个请求即有 结论只要按 PR 描述的要求让 #8276 先合并,本改动不会降低缓存命中率,默认配置用户行为完全无变化。若本 PR 被单独合并,超预算工具目录的 DeepSeek 用户会为每个新发现的工具付一次前缀失效代价(本次实测约 5k+ 未命中 tokens,视插入点可能更多)。建议保持 draft 或 blocked 状态直到 #8276 落地,作为合并顺序护栏。 |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 343 passed · 0 failed · 343 total 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:343 通过 · 0 失败 · 343 总计 Verification reportPR #8331 Deep Verification Report (follow-up round)Verdict: This is a follow-up round on an unchanged PR head. The verified head 中文摘要结论: A/B 结论:中心声明再次证实(见 A/B table)。在 BASE 上 3 个 DeepSeek 模型( Findings:无阻塞性问题。 未覆盖范围 / 携带项:#8276 依赖仍未并入 base(重新实测确认,见下表第 1 行)——合并顺序须由 maintainer 强制;真实 DeepSeek API 工具选择质量未测(无凭证);threshold 路径未被本 PR 修改,未单独 A/B。 Previous-finding status tableRound 1 reported no blocking findings; its open items were all in Not covered. Their status at the (unchanged) head, re-measured this round:
No round-1 item was Central claim + A/B tableCentral claim: Model identity no longer changes the ToolSearch default — DeepSeek models get ToolSearch enabled by default, same as every other model. Secondary claims: (1) explicit The PR's own
HEAD: 5/5 green. BASE: exactly the 3 DeepSeek cells red, Vacuity checkReverted the key hunk in HEAD's Targeted gateFull Description consistency (re-verified)All four user-facing locations updated; DeepSeek appears 0 times in each (
Residual CorrectionsNone. Round 1's descriptions of the code (mechanism, ARIA-irrelevant here, cause attribution) were accurate; re-reading the diff and the core sources this round found nothing to correct. FindingsNo blocking findings. The single carried-forward note (#8276 not yet in base) is a maintainer merge-ordering action, not a defect in this PR's code — the PR explicitly declares the dependency and "must not merge first." Not covered
MethodologyEnvironment: A/B: base worktree at Vacuity: key hunk reverted in-place by Schema: Assertion composition ( Evidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
⏸️ Deferring to @wenshao — the code review is clean (3/5, no blockers), but this PR explicitly depends on #8276 which is still open. Approving now would greenlight a merge that removes DeepSeek's prefix-cache protection before the replacement mechanism lands. Needs a maintainer to coordinate merge ordering: #8276 first, then this PR. |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
@qwen-code /reivew |
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed at 9ad1571. The diff is correct and I have no objection to the change itself — I am requesting changes purely to put a mechanical block behind the merge-order constraint this PR states in prose.
The dependency is unmerged, and nothing currently prevents this from landing first
The body says "This PR depends on #8276 and must not merge first." I verified the current state rather than taking the note at face value:
- #8276 is
OPEN, not merged. - Its stable-proxy mechanism is not in
main:packages/core/src/core/deferred-tool-call-normalization.ts— a file #8276 adds — is absent fromorigin/main, andmain'sclient.tsstill carries the old path verbatim: "when ToolSearch is not registered … every deferred tool is eagerly revealed here so it lands in the declaration list." - Meanwhile this PR is
isDraft=false,mergeable=MERGEABLE, and already carries a maintainer approval.
So the only thing standing between the repo and an out-of-order merge is the sentence in the description. If this lands first, DeepSeek models get ToolSearch enabled while the reveal path still rewrites the provider tool declarations — which is exactly the prefix-cache invalidation #4069 introduced the DeepSeek default-off for. On models where cached tokens are discounted up to 1/120, that is the 0.00% post-reveal hit rate your own A/B measured on the old path, silently restored for every DeepSeek user who has not set tools.toolSearch.enabled explicitly.
A CHANGES_REQUESTED flips reviewDecision and keeps the merge button unavailable until it is dismissed, which is the enforcement the constraint deserves. Dismiss this the moment #8276 lands (or convert this PR to draft and I will withdraw it instead) — I am not asking for a code change. Adding a do-not-merge label would work equally well if the project prefers that; I went with the review because it is the only signal that actually blocks.
The change itself checks out
The removal is minimal and correct: the whole model-sniffing block collapses to if (settings.tools?.toolSearch?.enabled === false), so the explicit opt-out survives intact and only the implicit DeepSeek default is dropped — no ^-anchor regex left behind, no dead branch. The setting descriptions in all four surfaces (CLI schema, vscode JSON schema, settings.md, web-shell i18n) are updated consistently and no longer advise disabling ToolSearch for prefix-cache stability, which would be stale advice after #8276.
Test coverage is a fair trade rather than a net loss: the five should auto-disable tool_search for deepseek-* cases are replaced by one parameterised should enable tool_search by default for %s over deepseek-v4-flash, deepseek-v3, openrouter/deepseek/deepseek-chat, and qwen-max — the same model shapes including the provider-prefixed one, asserting the inverted expectation. The explicit enabled: false deny-list case and the threshold tests (default 10, safe/bare-mode forcing to 0) are untouched, so the surviving behaviour stays pinned.
Ran both touched test files locally at this HEAD: 359/359. CI green.
Process note: this touches packages/cli/src/config/**, which AGENTS.md counts as core, from a non-maintainer author. At +1/-16 of production logic it is firmly Tier-2 small-scope, the behaviour change is a single collapsed conditional whose downstream consumers I can name in full (the mergedDeny push and the four description surfaces), and a maintainer has already approved — so the gate is satisfied. Flagging only for awareness.
|
Re the merge-order block: this PR is now The dependency is still unmet: #8276 is open and its stable-proxy path is not in |
# Conflicts: # docs/users/configuration/settings.md










What this PR does
This draft enables ToolSearch by default for DeepSeek models, while preserving the existing explicit opt-out and the 10% deferred-tool preload threshold. It also updates the user-facing setting descriptions so they no longer recommend disabling ToolSearch for prefix-cache stability.
This PR depends on #8276 and must not merge first. Once #8276 lands, revealing a deferred schema through ToolSearch no longer changes the provider-facing tool declarations, so the DeepSeek-specific default-off workaround is no longer necessary.
Why it's needed
#4069 intentionally disabled ToolSearch for DeepSeek because the old reveal path rewrote the provider tool list and invalidated prefix-based prompt caches. #8276 removes that failure mode by keeping deferred schemas in conversation content and executing them through a stable proxy declaration. Keeping the model-specific disable after that change would prevent DeepSeek users from benefiting from deferred tool discovery when their catalog exceeds the existing preload budget.
The prompt-cache A/B attached to #8276 measured 30 paired runs across five deferred tools. The post-reveal weighted cache hit rate increased from 0.00% on the old path to 96.97% on the stable-proxy path: #8276 (comment)
Reviewer Test Plan
How to verify
Start Qwen Code with a DeepSeek v3, v4, or provider-prefixed DeepSeek model and no explicit ToolSearch setting. Confirm ToolSearch remains available and the existing 10% threshold still decides whether deferred schemas are preloaded or discovered on demand. Then set
tools.toolSearch.enabledtofalse, restart, and confirm ToolSearch is disabled and deferred tools are declared eagerly.Evidence (Before & After)
Before: DeepSeek model names automatically placed ToolSearch in the deny list even when the user had not disabled it.
After: Model identity no longer changes the ToolSearch default. Explicit
enabled: falseremains authoritative, and threshold behavior is unchanged.Tested on
Environment (optional)
Node.js 24.18.0, local macOS worktree without sandboxing.
Risk & Scope
tools.toolSearch.enabledtofalse; no settings migration is required.Linked Issues
Depends on #8276. Follow-up to #4069 and discussion #4065.
中文说明
本 PR 的改动
此 Draft PR 默认对 DeepSeek 模型启用 ToolSearch,同时保留现有的显式关闭能力和 10% 延迟工具预加载阈值。它还会更新面向用户的设置说明,不再建议为了保持前缀缓存稳定而关闭 ToolSearch。
此 PR 依赖 #8276,不能先于它合并。#8276 落地后,通过 ToolSearch 展示延迟工具 schema 不再改变面向 provider 的工具声明,因此不再需要针对 DeepSeek 默认关闭的临时规避方案。
为什么需要此改动
#4069 有意为 DeepSeek 关闭 ToolSearch,因为旧的 reveal 路径会重写 provider 工具列表并使基于前缀的提示词缓存失效。#8276 通过将延迟 schema 放入 conversation,并使用稳定的代理声明执行工具,消除了这个问题。继续保留模型特定的关闭逻辑,会使 DeepSeek 用户在工具目录超过现有预加载预算时无法受益于延迟工具发现。
#8276 中附带的提示词缓存 A/B 覆盖五种延迟工具的 30 次配对运行。schema reveal 后的加权缓存命中率从旧路径的 0.00% 提升至稳定代理路径的 96.97%:#8276 (comment)
Reviewer 测试计划
验证方式
在没有显式 ToolSearch 设置的情况下,分别使用 DeepSeek v3、v4 或带 provider 前缀的 DeepSeek 模型启动 Qwen Code。确认 ToolSearch 默认可用,并且现有的 10% 阈值仍决定延迟 schema 是预加载还是按需发现。随后将
tools.toolSearch.enabled设为false,重启并确认 ToolSearch 被禁用且延迟工具会被预先声明。前后对比证据
改动前:即使用户没有关闭 ToolSearch,DeepSeek 模型名也会自动将其加入 deny list。
改动后:模型身份不再改变 ToolSearch 的默认值。显式
enabled: false仍然生效,threshold 行为保持不变。测试环境
环境(可选)
Node.js 24.18.0,macOS 本地 worktree,未启用 sandbox。
风险与范围
tools.toolSearch.enabled为false,无需迁移设置。关联事项
依赖 #8276;是 #4069 和 discussion #4065 的后续改动。