feat(acp-adapter): expose thinking effort levels in config options - #1988
feat(acp-adapter): expose thinking effort levels in config options#1988cnctem wants to merge 3 commits into
Conversation
🦋 Changeset detectedLatest commit: b60a97d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d31ea1588
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| name: 'Thinking', | ||
| category: 'thought_level', | ||
| currentValue: 'on', | ||
| options: [{ value: 'on', name: 'Thinking On' }], | ||
| options: [{ value: 'on', name: 'On' }], |
There was a problem hiding this comment.
Show effort choices for always-on effort models
When a catalog row has both alwaysThinking and supportEfforts, this early return runs before the effort-dropdown branch, so ACP exposes only a single on choice. Such rows are real: effectiveModelAlias assigns always_thinking for profiles with canDisableThinking === false while still copying profile.efforts (packages/agent-core/src/config/model.ts:41-56), and managed/open-platform models can also be supports_thinking_type='only' with efforts. In those environments the new ACP surface still hides the low/medium/high/... selector for effort-capable models; the always-on case should omit only off, not collapse all efforts to on.
Useful? React with 👍 / 👎.
| const normalizedEffort = | ||
| currentEffort !== 'off' && efforts.includes(currentEffort) ? currentEffort : 'off'; |
There was a problem hiding this comment.
Don't render carried thinking effort as off
When currentEffort is any non-off value that is not declared by the currently selected model, this reports currentValue: 'off' without actually disabling thinking. For example, after selecting max on one model and switching to a Kimi model whose supportEfforts are only ['low','high'], AcpSession.setModel preserves the non-off adapter state while agent-core normalizes unsupported Kimi efforts to the target model default (packages/agent-core/src/agent/config/thinking.ts:79-81); the ACP snapshot then tells the client thinking is off even though the session will run with thinking on. Prefer the target/default effort for non-off carried values, or refresh the adapter state from the session after the model switch.
Useful? React with 👍 / 👎.
4d31ea1 to
135341a
Compare
|
针对 Review 的两点修改如下: 1. always-thinking + supportEfforts 现在展示 effort 选项文件: // 修改前
if (alwaysThinking) {
return { currentValue: 'on', options: [{ value: 'on', name: 'On' }] };
}
// 修改后
if (alwaysThinking) {
if (efforts.length > 0) {
// 有 effort 列表时展示全部 effort,只去掉 off
return {
currentValue: normalizedEffort,
options: efforts.map((effort) => ({ value: effort, name: label(effort) })),
};
}
// 没有 effort 列表时才展示单个 On
return { currentValue: 'on', options: [{ value: 'on', name: 'On' }] };
}匹配 2. 模型切换后遗留的 effort 映射到默认 effort,不再显示为 off文件: const efforts = currentModelEntry?.supportEfforts;
let normalizedEffort = currentThinkingEffort;
if (
normalizedEffort === 'off' ||
(efforts !== undefined &&
efforts.length > 0 &&
!efforts.includes(normalizedEffort))
) {
normalizedEffort =
normalizedEffort === 'off'
? 'off'
: (currentModelEntry?.defaultThinkingEffort ?? 'on');
}
if (alwaysThinking && normalizedEffort === 'off') {
normalizedEffort = currentModelEntry?.defaultThinkingEffort ?? 'on';
}逻辑:
对应新增测试文件:
|
bc112c0 to
86c4a59
Compare
86c4a59 to
7d31a96
Compare
…cnctem/jimi-code into feat/acp-thinking-effort-levels
|
#1992 已经实现该功能,故关闭 |
Related Issue
#1947
Problem
The ACP
thinkingconfig option currently exposes only a binaryoff/ontoggle, even for models that support multiple thinking effort levels (low,medium,high, etc.). The VS Code extension'sThinkingButtoncomponent already supports per-model effort selection, but the ACP adapter does not, leaving editor integrations like Zed with a coarser control than the first-party UI.What changed
This PR aligns the ACP adapter's thinking control with
apps/vscode/webview-ui/src/components/ThinkingButton.tsx:supportEfforts, thethinkingoption now renders['off', ...efforts](e.g.off / low / medium / high), matchingThinkingButton's effort mode.off / onselect, equivalent toThinkingButton's switch mode.alwaysThinkingmodels collapse to a singleonentry, equivalent toThinkingButton's always-on mode.ThinkingButton(Off,On,Low,Medium, …).Implementation details:
packages/acp-adapter/src/model-catalog.ts: exposesupportEffortsonAcpModelEntry, sourced from the model alias config or Anthropic model profile inference.packages/acp-adapter/src/config-options.ts:buildThinkingOptionnow renders an effort dropdown whensupportEffortsis present, falling back to binaryoff/onotherwise.packages/acp-adapter/src/session.ts: adapter-side thinking state is now tracked as an effort string instead of a boolean, andsetThinkingaccepts effort strings with'on'normalized to the model default.packages/acp-adapter/src/server.ts:resolveCurrentThinkingEffortreturns the actual effort string;setSessionConfigOption({ configId: 'thinking', value })passes the effort through.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.