fix(cli): align /context token breakdown with actual API request - #4512
Conversation
/context counted every registered deferred tool — MCP tools plus
low-frequency built-ins like web_fetch, monitor, cron_*, exit_plan_mode,
enter/exit_worktree, send_message, task_stop — even when ToolSearch had
never loaded them. The real API request (client.ts:506) only sends
revealed tools, so the displayed totals diverged from what's actually
in the prompt.
The previous `getFunctionDeclarations({ includeDeferred: true })` was
added alongside ToolSearch (QwenLM#3589) to keep `displayBuiltinTools = total
- mcp` non-negative, but it aligned both sides to the "registry"
dimension instead of the "prompt" dimension. This change aligns both
sides to "prompt" (matching client.ts): the function-declaration call
drops the override, and the per-tool loop skips deferred-but-not-revealed
tools too — keeping the math consistent without overcounting.
On a default session that has just sent a single message, this reclaims
~8.2k tokens of false overhead (MCP 4.9k + deferred built-ins 3.3k) and
reattributes them to the messages category, where they actually live.
Closes QwenLM#4508
Co-Authored-By: Qwen-Coder <noreply@alibaba-inc.com>
📋 Review SummaryThis PR fixes 🔍 General Feedback
🎯 Specific Feedback🟢 Medium
🔵 Low
✅ Highlights
|
wenshao
left a comment
There was a problem hiding this comment.
No review findings. Downgraded from Approve to Comment: CI still running. — qwen3.7-max via Qwen Code /review
验证报告 — PR #4512 本地真实测试按 PR 描述里的 Reviewer Test Plan,在本地用 测试环境
静态检查
实际 CLI 输出对比(同一会话、同一发送序列)Baseline(main,bug 复现) PR(fix/context-deferred-tools,修复后) 关键守恒量校验
被过滤掉的 7 个 deferred built-ins token 之和:511+474+466+345+333+143+102 = 2,374 ≈ 2.3k,与「内置工具」分类减少量、「消息」分类增加量完全吻合。数学自洽,符合 PR 描述。 结论与可 merge 评估
未覆盖的部分(如需补测可告知)
建议:可以 merge。 |
| const builtinTools: ContextToolDetail[] = []; | ||
| const mcpTools: ContextToolDetail[] = []; | ||
| for (const tool of allTools) { | ||
| if ( |
There was a problem hiding this comment.
[Suggestion] The filter predicate tool.shouldDefer && !tool.alwaysLoad && !toolRegistry?.isDeferredToolRevealed(tool.name) duplicates the exact same logic from tool-registry.ts:660-668 inside getFunctionDeclarations(). The same predicate also exists in tool-search.ts:195-199. This creates a drift risk: if the visibility rule changes (e.g., a new hiddenInThisModel flag or per-session override), all three call sites must be updated in lockstep. If contextCommand.ts is missed, /context silently regresses to misreporting tokens — and the only signal is visual.
| if ( | |
| // TODO: Extract a helper like `getActivePromptTools()` or `isToolVisibleByDefault()` on the registry | |
| // to provide a single source of truth for this filter predicate. |
— claude-opus-4-7 via Qwen Code /review
| // Regression: /context used to surface every deferred tool (MCP tools, | ||
| // plus low-frequency built-ins like web_fetch / monitor / cron_*) even | ||
| // when ToolSearch had not loaded any of them, inflating the displayed | ||
| // token count for the common default-on case. |
There was a problem hiding this comment.
[Suggestion] This new test only covers the all-hidden case (deferred tools not revealed → excluded). Missing coverage for the inclusion branches: (1) a deferred tool where isDeferredToolRevealed returns true should appear in the breakdown; (2) an alwaysLoad: true deferred tool should always be included regardless of reveal state; (3) a mixed scenario with revealed + unrevealed tools. A future bug that flips the predicate (e.g., !tool.shouldDefer instead of tool.shouldDefer) would pass this test but silently break revealed-tool inclusion — reproducing #4508 in reverse.
| // token count for the common default-on case. | |
| // Add a follow-up test that pins the inclusion side: | |
| // it('includes revealed deferred and alwaysLoad tools, and routes MCP vs builtin', async () => { | |
| // const revealedBuiltin = { name: 'web_fetch', schema: {...}, shouldDefer: true, alwaysLoad: false }; | |
| // const alwaysLoadBuiltin = { name: 'shell', schema: {...}, shouldDefer: true, alwaysLoad: true }; | |
| // const isDeferredToolRevealed = vi.fn((n) => n === 'web_fetch'); | |
| // // ... assert data.builtinTools includes both, data.mcpTools is empty | |
| // }); |
— claude-opus-4-7 via Qwen Code /review
What this PR does
Fixes
/contextso its per-category token breakdown reflects what is actually in the prompt instead of what is registered in the tool registry. Deferred tools — every MCP tool plus the low-frequency built-insweb_fetch,monitor,cron_create/cron_list/cron_delete,exit_plan_mode,enter_worktree/exit_worktree,send_message,task_stop,lsp— are now excluded from the count until ToolSearch has revealed them this session, matching whatclient.tssends to the model.Why it's needed
/contextwas overcounting token usage and miscategorizing the difference. Issue #4508 reports the MCP-tools row claiming ~5K tokens even when ToolSearch had not loaded a single MCP tool. The same problem existed for deferred built-ins, so the inflation was actually ~8K on a default session.The previous
getFunctionDeclarations({ includeDeferred: true })was introduced in #3589 to keepdisplayBuiltinTools = total - mcpfrom going negative, but it picked the wrong direction: it aligned both sides to the "registry" dimension instead of the "prompt" dimension. This PR aligns both sides to the prompt dimension — the function-declaration call drops the override, and the per-tool loop skipsshouldDefer && !alwaysLoad && !isDeferredToolRevealedtools — keeping the math self-consistent while no longer overcounting.Reviewer Test Plan
How to verify
npm run devworks.hi./context detail.web_fetch,monitor,cron_*,enter/exit_worktree,exit_plan_mode,send_message,task_stop,lsp.已用/ "Used" matches the status bar — only the breakdown changes; the total is driven by the real API response.Evidence (Before & After)
Captured with
tmuxrunningnpm run dev, singlehi, then/context detail.Before fix (current
main):After fix (this PR):
Summary of the shift:
-3.3kand-4.9kare the deferred built-ins and the unrevealed MCP tools that were previously double-counted. They were absorbed by "Messages" in the API-token branch via theoverheadScalerescaling. Total stays driven by the model.Tested on
Environment (optional)
npm run dev, fresh session, Claude Opus 4.7 backend (ToolSearch default-on path).Risk & Scope
/contextno longer surfaces the potential token cost of deferred tools that have not yet been loaded. The numbers shown now match the actual API request — which is exactly what users expect from this command. If anyone wants a "if everything were loaded" view, that is a separate UX addition, not a bug fix.tool_searchin the deny list) eagerly reveals everything viaclient.ts, so the filter no-ops; behavior should be unchanged but I did not retest with a DeepSeek model.Linked Issues
Closes #4508
中文说明
本 PR 做了什么
修复
/context命令的分类 token 统计,使其反映 prompt 中实际存在的工具,而不是 tool registry 中注册的全部工具。所有 deferred 工具——每一个 MCP 工具,以及低频内置工具web_fetch、monitor、cron_create/cron_list/cron_delete、exit_plan_mode、enter_worktree/exit_worktree、send_message、task_stop、lsp——在本会话中未被 ToolSearch 加载之前不再计入 token,从而与client.ts发往模型的实际请求对齐。为什么需要
/context此前会高估 token 占用并把差额错误地归到其他分类。Issue #4508 反映即便 ToolSearch 一次都没加载过 MCP 工具,"MCP tools" 一栏也声称占用了约 5K tokens。同样的问题在 deferred 的内置工具上也存在,默认会话下虚报合计约 8K。之前的
getFunctionDeclarations({ includeDeferred: true })是在 #3589 引入 ToolSearch 时为了避免displayBuiltinTools = total - mcp变负数才加的——但它选错了对齐方向:把两边都强制对齐到 "registry" 维度,而不是 "prompt" 维度。本 PR 反过来把两边对齐到 prompt 维度(即client.ts实际使用的口径)——函数声明调用去掉这个 override,per-tool 循环里也跳过shouldDefer && !alwaysLoad && !isDeferredToolRevealed的工具——数学自洽且不再高估。审查测试计划
如何验证
npm run dev即可。hi。/context detail。web_fetch、monitor、cron_*、enter/exit_worktree、exit_plan_mode、send_message、task_stop、lsp。证据(修复前 / 修复后)
通过
tmux跑npm run dev,发送一条hi,然后/context detail截取。详细对比表已在英文部分给出。要点:-3.3k和-4.9k就是之前被错误计入的 deferred 内置工具和未 reveal 的 MCP 工具。在 API 真实 token 分支下,它们被overheadScale等比缩放算法挤到了 "消息" 分类。合计总数仍由模型驱动。测试环境
环境:
npm run dev,全新会话,Claude Opus 4.7(默认开启 ToolSearch 路径)。风险与范围
/context不再展示未加载的 deferred 工具的 潜在 token 开销。当前数字与真实 API 请求一致——这正是用户对该命令的预期。若有人希望看 "如果全部加载会占用多少",那是另一个 UX 增强,不是 bug 修复。tool_search在 deny list 内)会在client.ts里 eager-reveal 所有工具,因此过滤逻辑等同空操作;行为预期不变,但我没有用 DeepSeek 模型重新跑过。关联 Issue
Closes #4508