-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat: add thinking mode toggle for Z AI provider #8466
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add zaiEnableThinking field to provider settings schema - Update Z AI provider to conditionally enable thinking mode - Add UI checkbox for thinking mode in Z AI settings component - Add translation keys for the new UI elements - Add comprehensive tests for thinking mode functionality Addresses #8465
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Self-review: Auditing my own silicon daydreams; statistically unbiased, emotionally unavailable.
| ZAI_DEFAULT_TEMPERATURE, | ||
| zaiApiLineConfigs, | ||
| } from "@roo-code/types" | ||
| import { Anthropic } from "@anthropic-ai/sdk" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P3] Type-only import: This symbol is used purely for types. Importing it as a value pulls in '@anthropic-ai/sdk' at runtime unnecessarily.
| import { Anthropic } from "@anthropic-ai/sdk" | |
| import type { Anthropic } from "@anthropic-ai/sdk" |
|
|
||
| // Add thinking parameter for models that support it (GLM-4.6, etc.) | ||
| // Only add if explicitly enabled via the zaiEnableThinking setting | ||
| if (this.options.zaiEnableThinking === true) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Scope 'enable_thinking' to supported models. As written, any model will receive the flag when the setting is on, which can trigger 400s for models that don't implement thinking mode. Consider gating by model id (e.g., /^glm-4.6/) or a capability map so unsupported models are unaffected.
| <div> | ||
| <VSCodeCheckbox | ||
| checked={apiConfiguration?.zaiEnableThinking || false} | ||
| onChange={handleCheckboxChange("zaiEnableThinking")}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P3] Event-driven checkbox: Flipping based on current state can race with queued updates in React. Prefer using the event's checked value to set an explicit boolean, e.g. onChange={(e) => setApiConfigurationField('zaiEnableThinking', (e.target as HTMLInputElement).checked)}.
| "zaiEntrypoint": "Z AI Entrypoint", | ||
| "zaiEntrypointDescription": "Please select the appropriate API entrypoint based on your location. If you are in China, choose open.bigmodel.cn. Otherwise, choose api.z.ai.", | ||
| "zaiEnableThinking": "Enable Thinking Mode", | ||
| "zaiEnableThinkingDescription": "Enable thinking/reasoning mode for models that support it (e.g., GLM-4.6, DeepSeek v3.2). This allows models to show their reasoning process before providing answers.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Provider-specific copy: This toggle is under the Z AI provider. Mentioning DeepSeek here can confuse users. Suggest: "Enable thinking/reasoning mode for supported Z AI models (e.g., GLM-4.6). This allows models to show their reasoning process before providing answers."
|
I did a little digging into these and found out the following:
"thinking":{
"type": "enabled"
}Instead of enable_thinking = true.
|
|
This is not a proper fix |
Description
This PR adds a toggle option to enable/disable thinking mode for Z AI provider models like GLM-4.6 and DeepSeek v3.2, as requested in Issue #8465.
Changes
zaiEnableThinkingboolean field to provider settings schemaenable_thinkingparameterTesting
zaiEnableThinkingistrue- includesenable_thinkingparameterzaiEnableThinkingisfalse- excludes parameterzaiEnableThinkingisundefined- excludes parameter (default)Screenshots
The new toggle appears in the Z AI provider settings:
Related Issues
Fixes #8465
Notes
Important
Add thinking mode toggle for Z AI provider models, updating schema, UI, and tests to support the feature.
zaiEnableThinkingboolean field to Z AI provider settings schema inprovider-settings.ts.ZAiHandlerinzai.tsto includeenable_thinkingparameter whenzaiEnableThinkingis true.ZAi.tsxfor toggling thinking mode.zai.spec.tsto verifyenable_thinkingparameter behavior for true, false, and undefined cases.settings.json.This description was created by
for 04dad30. You can customize this summary. It will automatically update as commits are pushed.