feat(acp): support selecting thinking effort levels - #1992
Conversation
The ACP thinking config option was a binary on/off toggle mapped to the model's default effort, so ACP clients (e.g. Zed) could not pick a concrete level even though the engine and the SDK fully support support_efforts granularity. - Advertise one select row per declared effort level (off + levels); boolean models keep the legacy off/on rows, always-thinking models drop the off row. - Track the session thinking state as an effort string and reconcile it with the engine-normalized value read back from session status after model/thinking changes. - Validate incoming levels against the current model's catalog and reject unknown ones with invalid_params before any SDK call; keep the legacy 'on' value as an alias for the model default effort.
🦋 Changeset detectedLatest commit: 344f296 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 |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 77c13d990e
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const currentValue = | ||
| !alwaysThinking && currentEffort === 'off' | ||
| ? 'off' | ||
| : efforts.includes(currentEffort) | ||
| ? currentEffort | ||
| : defaultEffort; |
There was a problem hiding this comment.
When a model declares default_effort that is not present in support_efforts (the schema allows this, and agent-core's defaultThinkingEffortFor falls back to a declared effort), this fallback makes the ACP select's currentValue become that invalid default even though options only contains off plus supportEfforts. In that configuration, a fresh snapshot with current effort on/unknown can render a value the client cannot select; validate defaultEffort against efforts and fall back to the middle declared effort instead.
Useful? React with 👍 / 👎.
1 participant
Related Issue
No linked issue — the problem is explained below (user feedback).
Problem
Users reported that ACP clients (e.g. Zed) cannot select a thinking effort level — the
thinkingconfig option was a binaryoff/ontoggle, andonwas silently mapped to the model's default effort. The engine (resolveThinkingEffort) and the SDK (Session.setThinking(effort)) already fully supportsupport_effortsgranularity; the adapter deliberately collapsed it to a boolean, so levels likelow/medium/highwere unreachable over ACP even though the protocol'sthought_levelselect carries arbitrary rows.What changed
Scoped to
packages/acp-adapter(engine / SDK / protocol untouched):thinkingconfig option now emitsoffplus one row per declaredsupport_effortslevel of the current model. Boolean models (nosupport_efforts) keep the legacyoff/onrows;always_thinkingmodels drop theoffrow. The model catalog helper now exposessupportEffortsalongsidedefaultThinkingEffort.session/set_config_option({configId: 'thinking', value})takesoff, the legacyon(alias for the model default effort, kept for compatibility), or a declared level; unknown levels are rejected withinvalid_paramsbefore any SDK call.Session.getStatus()after model/thinking changes, so engine-side clamping (e.g.always_thinkingrejectingoff, or a level the newly selected model does not declare) is what the next snapshot renders.This fits Kimi Code because the adapter stays a thin projection over the engine's single source of truth (
support_efforts/default_effort/resolveThinkingEffort) — no parallel effort logic is introduced.Verification: 324 unit tests green (new cases for per-level rows, default-effort mapping, invalid-level rejection, status reconciliation); typecheck clean; plus a real-process smoke test driving
kimi acpover stdio with a real ACP client through initialize → session/new → level selection → invalid-level rejection → model switching (16/16 assertions passed).Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.