feat: expose acp thinking variants #9064
Conversation
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
The following comment was made by an LLM, it may be inaccurate: No duplicate PRs found |
| "description": "AI-powered development tool", | ||
| "private": true, | ||
| "type": "module", | ||
| "packageManager": "[email protected].5", |
There was a problem hiding this comment.
Can we revert this? There is a reason we havent upgraded bun yet :)
There was a problem hiding this comment.
For sure! And I'll resolve other style-related problems too, working on it
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| directory: string, | ||
| sessionId: string, | ||
| ): Promise<{ availableModes: ModeOption[]; currentModeId?: string }> { | ||
| const availableModes = await this.loadAvailableModes(directory) |
There was a problem hiding this comment.
Style guide suggestion: This let statement could potentially be refactored to avoid mutation. Consider using an IIFE or ternary if the logic can be simplified. However, given the async defaultAgent() call inside the conditional, this may be acceptable as-is since the complexity of alternatives might outweigh the benefit.
|
|
||
| const providers = await this.sdk.config.providers({ directory }).then((x) => x.data!.providers) | ||
| const entries = sortProvidersByName(providers) | ||
| const availableVariants = modelVariantsFromProviders(entries, model) |
There was a problem hiding this comment.
Style guide note: Destructuring like const { availableModes, currentModeId } = ... goes against the style guide recommendation to avoid unnecessary destructuring. You could use const modeState = await this.resolveModeState(...) and access modeState.availableModes and modeState.currentModeId to preserve context. This is just a suggestion - feel free to keep as-is if you prefer the readability here.
| async unstable_setSessionModel(params: SetSessionModelRequest) { | ||
| const session = this.sessionManager.get(params.sessionId) | ||
| const providers = await this.sdk.config | ||
| .providers({ directory: session.cwd }, { throwOnError: true }) |
There was a problem hiding this comment.
Style guide note: Same as above - destructuring const { model, variant } = ... could be const selection = parseModelSelection(...) with selection.model and selection.variant. Just a suggestion for consistency with style guide.
|
thanks! |
2 participants
What does this PR do?
Fixes #8973 by exposing thinking variants in ACP model selection:
claude-sonnet-4/high) tomodels.availableModelssession/set_modelselections and keeps session variant in sync_meta(modelId,variant,availableVariants) instead of empty object@agentclientprotocol/sdkto 0.13.0How did you verify your code works?
bun run typechecksession/set_modelresponses include correct_meta.opencodewith variant infoScreenshot