Skip to content

Commit de00ab1

Browse files
refactor: consolidate ThinkingBudget components and fix disable handling (#9930)
1 parent 93a43e4 commit de00ab1

File tree

7 files changed

+78
-374
lines changed

7 files changed

+78
-374
lines changed

src/api/transform/__tests__/model-params.spec.ts

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,6 @@ describe("getModelParams", () => {
275275

276276
expect(result.reasoningBudget).toBeUndefined()
277277
expect(result.temperature).toBe(0)
278-
expect(result.reasoning).toBeUndefined()
279278
})
280279

281280
it("should honor customMaxTokens for reasoning budget models", () => {
@@ -557,7 +556,6 @@ describe("getModelParams", () => {
557556
})
558557

559558
expect(result.reasoningEffort).toBeUndefined()
560-
expect(result.reasoning).toBeUndefined()
561559
})
562560

563561
it("should handle reasoning effort for openrouter format", () => {
@@ -624,7 +622,6 @@ describe("getModelParams", () => {
624622
})
625623

626624
expect(result.reasoningEffort).toBeUndefined()
627-
expect(result.reasoning).toBeUndefined()
628625
})
629626

630627
it("should include 'minimal' and 'none' for openrouter format", () => {
@@ -670,7 +667,7 @@ describe("getModelParams", () => {
670667
it("should use reasoningEffort if supportsReasoningEffort is false but reasoningEffort is set", () => {
671668
const model: ModelInfo = {
672669
...baseModel,
673-
maxTokens: 3000, // Changed to 3000 (18.75% of 16000), which is within 20% threshold
670+
maxTokens: 3000, // 3000 is 18.75% of 16000, within 20% threshold
674671
supportsReasoningEffort: false,
675672
reasoningEffort: "medium",
676673
}
@@ -681,7 +678,8 @@ describe("getModelParams", () => {
681678
model,
682679
})
683680

684-
expect(result.maxTokens).toBe(3000) // Now uses model.maxTokens since it's within 20% threshold
681+
expect(result.maxTokens).toBe(3000)
682+
// Now uses model.maxTokens since it's within 20% threshold
685683
expect(result.reasoningEffort).toBe("medium")
686684
})
687685
})
@@ -735,7 +733,7 @@ describe("getModelParams", () => {
735733
})
736734

737735
expect(result.reasoningBudget).toBe(3200) // 80% of 4000
738-
expect(result.reasoningEffort).toBeUndefined()
736+
expect(result.reasoningEffort).toBeUndefined() // Budget takes precedence
739737
expect(result.temperature).toBe(1.0)
740738
})
741739

@@ -889,8 +887,6 @@ describe("getModelParams", () => {
889887
settings: {},
890888
model,
891889
})
892-
893-
expect(result.reasoning).toBeUndefined()
894890
})
895891
})
896892

src/api/transform/model-params.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,10 +130,12 @@ export function getModelParams({
130130
temperature = 1.0
131131
} else if (shouldUseReasoningEffort({ model, settings })) {
132132
// "Traditional" reasoning models use the `reasoningEffort` parameter.
133-
const effort = (customReasoningEffort ?? model.reasoningEffort) as
134-
| ReasoningEffortExtended
135-
| "disable"
136-
| undefined
133+
// Only fallback to model default if user hasn't explicitly set a value.
134+
// If customReasoningEffort is "disable", don't fallback to model default.
135+
const effort =
136+
customReasoningEffort !== undefined
137+
? customReasoningEffort
138+
: (model.reasoningEffort as ReasoningEffortExtended | "disable" | undefined)
137139
// Capability and settings checks are handled by shouldUseReasoningEffort.
138140
// Here we simply propagate the resolved effort into the params, while
139141
// still treating "disable" as an omission.

src/shared/__tests__/api.spec.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,6 @@ describe("shouldUseReasoningEffort", () => {
379379
reasoningEffort: "medium",
380380
}
381381

382-
// Should return true regardless of settings (unless explicitly disabled)
383382
expect(shouldUseReasoningEffort({ model })).toBe(true)
384383
expect(shouldUseReasoningEffort({ model, settings: {} })).toBe(true)
385384
expect(shouldUseReasoningEffort({ model, settings: { reasoningEffort: undefined } })).toBe(true)
@@ -444,7 +443,7 @@ describe("shouldUseReasoningEffort", () => {
444443
expect(shouldUseReasoningEffort({ model })).toBe(false)
445444
})
446445

447-
test("should return false when model doesn't support reasoning effort", () => {
446+
test("should return false when model doesn't support reasoning effort and has no default", () => {
448447
const model: ModelInfo = {
449448
contextWindow: 200_000,
450449
supportsPromptCache: true,

webview-ui/src/components/settings/ApiOptions.tsx

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,6 @@ import { inputEventTransform, noTransform } from "./transforms"
109109
import { ModelInfoView } from "./ModelInfoView"
110110
import { ApiErrorMessage } from "./ApiErrorMessage"
111111
import { ThinkingBudget } from "./ThinkingBudget"
112-
import { SimpleThinkingBudget } from "./SimpleThinkingBudget"
113112
import { Verbosity } from "./Verbosity"
114113
import { DiffSettingsControl } from "./DiffSettingsControl"
115114
import { TodoListSettingsControl } from "./TodoListSettingsControl"
@@ -855,22 +854,14 @@ const ApiOptions = ({
855854
</>
856855
)}
857856

858-
{!fromWelcomeView &&
859-
(selectedProvider === "roo" ? (
860-
<SimpleThinkingBudget
861-
key={`${selectedProvider}-${selectedModelId}`}
862-
apiConfiguration={apiConfiguration}
863-
setApiConfigurationField={setApiConfigurationField}
864-
modelInfo={selectedModelInfo}
865-
/>
866-
) : (
867-
<ThinkingBudget
868-
key={`${selectedProvider}-${selectedModelId}`}
869-
apiConfiguration={apiConfiguration}
870-
setApiConfigurationField={setApiConfigurationField}
871-
modelInfo={selectedModelInfo}
872-
/>
873-
))}
857+
{!fromWelcomeView && (
858+
<ThinkingBudget
859+
key={`${selectedProvider}-${selectedModelId}`}
860+
apiConfiguration={apiConfiguration}
861+
setApiConfigurationField={setApiConfigurationField}
862+
modelInfo={selectedModelInfo}
863+
/>
864+
)}
874865

875866
{/* Gate Verbosity UI by capability flag */}
876867
{!fromWelcomeView && selectedModelInfo?.supportsVerbosity && (

webview-ui/src/components/settings/SimpleThinkingBudget.tsx

Lines changed: 0 additions & 120 deletions
This file was deleted.

webview-ui/src/components/settings/ThinkingBudget.tsx

Lines changed: 59 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -78,26 +78,62 @@ export const ThinkingBudget = ({ apiConfiguration, setApiConfigurationField, mod
7878

7979
// Build available reasoning efforts list from capability
8080
const supports = modelInfo?.supportsReasoningEffort
81-
const availableOptions: ReadonlyArray<ReasoningEffortWithMinimal> =
81+
const baseAvailableOptions: ReadonlyArray<ReasoningEffortWithMinimal> =
8282
supports === true
8383
? (reasoningEfforts as readonly ReasoningEffortWithMinimal[])
8484
: Array.isArray(supports)
8585
? (supports as ReadonlyArray<ReasoningEffortWithMinimal>)
8686
: (reasoningEfforts as readonly ReasoningEffortWithMinimal[])
8787

88+
// "disable" turns off reasoning entirely; "none" is a valid reasoning level.
89+
// Both display as "None" in the UI but behave differently.
90+
// Add "disable" option if reasoning effort is not required.
91+
type ReasoningEffortOption = ReasoningEffortWithMinimal | "none" | "disable"
92+
const availableOptions: ReadonlyArray<ReasoningEffortOption> = modelInfo?.requiredReasoningEffort
93+
? (baseAvailableOptions as ReadonlyArray<ReasoningEffortOption>)
94+
: (["disable", ...baseAvailableOptions] as ReasoningEffortOption[])
95+
8896
// Default reasoning effort - use model's default if available
8997
// GPT-5 models have "medium" as their default in the model configuration
9098
const modelDefaultReasoningEffort = modelInfo?.reasoningEffort as ReasoningEffortWithMinimal | undefined
91-
const defaultReasoningEffort: ReasoningEffortWithMinimal = modelDefaultReasoningEffort || "medium"
92-
const currentReasoningEffort: ReasoningEffortWithMinimal =
93-
(apiConfiguration.reasoningEffort as ReasoningEffortWithMinimal | undefined) || defaultReasoningEffort
99+
const defaultReasoningEffort: ReasoningEffortOption = modelInfo?.requiredReasoningEffort
100+
? modelDefaultReasoningEffort || "medium"
101+
: "disable"
102+
// Current reasoning effort from settings, or fall back to default
103+
const storedReasoningEffort = apiConfiguration.reasoningEffort as ReasoningEffortOption | undefined
104+
const currentReasoningEffort: ReasoningEffortOption = storedReasoningEffort || defaultReasoningEffort
94105

95106
// Set default reasoning effort when model supports it and no value is set
96107
useEffect(() => {
97-
if (isReasoningEffortSupported && !apiConfiguration.reasoningEffort && defaultReasoningEffort) {
98-
setApiConfigurationField("reasoningEffort", defaultReasoningEffort, false)
108+
if (isReasoningEffortSupported && !apiConfiguration.reasoningEffort) {
109+
// Only set a default if reasoning is required, otherwise leave as undefined (which maps to "disable")
110+
if (modelInfo?.requiredReasoningEffort && defaultReasoningEffort !== "disable") {
111+
setApiConfigurationField("reasoningEffort", defaultReasoningEffort as ReasoningEffortWithMinimal, false)
112+
}
113+
}
114+
}, [
115+
isReasoningEffortSupported,
116+
apiConfiguration.reasoningEffort,
117+
defaultReasoningEffort,
118+
modelInfo?.requiredReasoningEffort,
119+
setApiConfigurationField,
120+
])
121+
122+
// Sync enableReasoningEffort based on selection
123+
// "disable" turns off reasoning; "none" is a valid level (reasoning enabled)
124+
useEffect(() => {
125+
if (!isReasoningEffortSupported) return
126+
const shouldEnable = modelInfo?.requiredReasoningEffort || currentReasoningEffort !== "disable"
127+
if (shouldEnable && apiConfiguration.enableReasoningEffort !== true) {
128+
setApiConfigurationField("enableReasoningEffort", true, false)
99129
}
100-
}, [isReasoningEffortSupported, apiConfiguration.reasoningEffort, defaultReasoningEffort, setApiConfigurationField])
130+
}, [
131+
isReasoningEffortSupported,
132+
modelInfo?.requiredReasoningEffort,
133+
currentReasoningEffort,
134+
apiConfiguration.enableReasoningEffort,
135+
setApiConfigurationField,
136+
])
101137

102138
const enableReasoningEffort = apiConfiguration.enableReasoningEffort
103139
const customMaxOutputTokens = apiConfiguration.modelMaxTokens || DEFAULT_HYBRID_REASONING_MODEL_MAX_TOKENS
@@ -193,22 +229,34 @@ export const ThinkingBudget = ({ apiConfiguration, setApiConfigurationField, mod
193229
</div>
194230
<Select
195231
value={currentReasoningEffort}
196-
onValueChange={(value: ReasoningEffortWithMinimal) => {
197-
setApiConfigurationField("reasoningEffort", value)
232+
onValueChange={(value: ReasoningEffortOption) => {
233+
// "disable" turns off reasoning entirely; "none" is a valid reasoning level
234+
if (value === "disable") {
235+
setApiConfigurationField("enableReasoningEffort", false)
236+
setApiConfigurationField("reasoningEffort", "disable")
237+
} else {
238+
// "none", "minimal", "low", "medium", "high" all enable reasoning
239+
setApiConfigurationField("enableReasoningEffort", true)
240+
setApiConfigurationField("reasoningEffort", value as ReasoningEffortWithMinimal)
241+
}
198242
}}>
199243
<SelectTrigger className="w-full">
200244
<SelectValue
201245
placeholder={
202246
currentReasoningEffort
203-
? t(`settings:providers.reasoningEffort.${currentReasoningEffort}`)
247+
? currentReasoningEffort === "none" || currentReasoningEffort === "disable"
248+
? t("settings:providers.reasoningEffort.none")
249+
: t(`settings:providers.reasoningEffort.${currentReasoningEffort}`)
204250
: t("settings:common.select")
205251
}
206252
/>
207253
</SelectTrigger>
208254
<SelectContent>
209255
{availableOptions.map((value) => (
210256
<SelectItem key={value} value={value}>
211-
{t(`settings:providers.reasoningEffort.${value}`)}
257+
{value === "none" || value === "disable"
258+
? t("settings:providers.reasoningEffort.none")
259+
: t(`settings:providers.reasoningEffort.${value}`)}
212260
</SelectItem>
213261
))}
214262
</SelectContent>

0 commit comments

Comments
 (0)