Skip to content

Commit 98488ab

Browse files
committed
fix: address PR feedback for minimal reasoning support
- OpenRouter now returns undefined for "minimal" reasoning effort - Removed duplicate ReasoningEffort type, using ReasoningEffortWithMinimal consistently - Fixed test expectations to match new behavior - Extracted UI logic into shouldShowMinimalOption helper function - Translation key for "minimal" already exists in settings.json
1 parent 8c5eeeb commit 98488ab

File tree

3 files changed

+31
-15
lines changed

3 files changed

+31
-15
lines changed

src/api/transform/__tests__/reasoning.spec.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,6 @@ describe("reasoning.ts", () => {
165165
const modelWithEffort: ModelInfo = {
166166
...baseModel,
167167
supportsReasoningEffort: true,
168-
reasoningEffort: effort === "minimal" ? undefined : effort,
169168
}
170169

171170
const settingsWithEffort: ProviderSettings = {
@@ -179,7 +178,12 @@ describe("reasoning.ts", () => {
179178
reasoningEffort: effort,
180179
}
181180
const result = getOpenRouterReasoning(options)
182-
expect(result).toEqual({ effort })
181+
// "minimal" should return undefined for OpenRouter
182+
if (effort === "minimal") {
183+
expect(result).toBeUndefined()
184+
} else {
185+
expect(result).toEqual({ effort })
186+
}
183187
})
184188
})
185189

@@ -202,7 +206,8 @@ describe("reasoning.ts", () => {
202206

203207
const result = getOpenRouterReasoning(options)
204208

205-
expect(result).toEqual({ effort: "minimal" })
209+
// "minimal" should return undefined for OpenRouter
210+
expect(result).toBeUndefined()
206211
})
207212

208213
it("should handle minimal reasoning effort from settings", () => {
@@ -224,7 +229,8 @@ describe("reasoning.ts", () => {
224229

225230
const result = getOpenRouterReasoning(options)
226231

227-
expect(result).toEqual({ effort: "minimal" })
232+
// "minimal" should return undefined for OpenRouter
233+
expect(result).toBeUndefined()
228234
})
229235

230236
it("should handle zero reasoningBudget", () => {

src/api/transform/reasoning.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,8 @@ import type { ModelInfo, ProviderSettings, ReasoningEffortWithMinimal } from "@r
66

77
import { shouldUseReasoningBudget, shouldUseReasoningEffort } from "../../shared/api"
88

9-
type ReasoningEffort = "minimal" | "low" | "medium" | "high"
10-
119
export type OpenRouterReasoningParams = {
12-
effort?: ReasoningEffort
10+
effort?: ReasoningEffortWithMinimal
1311
max_tokens?: number
1412
exclude?: boolean
1513
}
@@ -36,7 +34,7 @@ export const getOpenRouterReasoning = ({
3634
shouldUseReasoningBudget({ model, settings })
3735
? { max_tokens: reasoningBudget }
3836
: shouldUseReasoningEffort({ model, settings })
39-
? reasoningEffort
37+
? reasoningEffort && reasoningEffort !== "minimal"
4038
? { effort: reasoningEffort }
4139
: undefined
4240
: undefined

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

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,17 @@ interface ThinkingBudgetProps {
2424
modelInfo?: ModelInfo
2525
}
2626

27+
// Helper function to determine if minimal option should be shown
28+
const shouldShowMinimalOption = (
29+
provider: string | undefined,
30+
modelId: string | undefined,
31+
supportsEffort: boolean | undefined,
32+
): boolean => {
33+
const isGpt5Model = provider === "openai-native" && modelId?.startsWith("gpt-5")
34+
const isOpenRouterWithEffort = provider === "openrouter" && supportsEffort === true
35+
return !!(isGpt5Model || isOpenRouterWithEffort)
36+
}
37+
2738
export const ThinkingBudget = ({ apiConfiguration, setApiConfigurationField, modelInfo }: ThinkingBudgetProps) => {
2839
const { t } = useAppTranslation()
2940
const { id: selectedModelId } = useSelectedModel(apiConfiguration)
@@ -37,14 +48,15 @@ export const ThinkingBudget = ({ apiConfiguration, setApiConfigurationField, mod
3748
const isReasoningBudgetRequired = !!modelInfo && modelInfo.requiredReasoningBudget
3849
const isReasoningEffortSupported = !!modelInfo && modelInfo.supportsReasoningEffort
3950

40-
// Check if this is a GPT-5 model or OpenRouter to show "minimal" option
41-
const isOpenAiNativeProvider = apiConfiguration.apiProvider === "openai-native"
42-
const isOpenRouterProvider = apiConfiguration.apiProvider === "openrouter"
43-
const isGpt5Model = isOpenAiNativeProvider && selectedModelId && selectedModelId.startsWith("gpt-5")
44-
// Add "minimal" option for GPT-5 models and OpenRouter models that support reasoning effort
45-
// Spread to convert readonly tuple into a mutable array, then expose as readonly for safety
51+
// Determine if minimal option should be shown
52+
const showMinimalOption = shouldShowMinimalOption(
53+
apiConfiguration.apiProvider,
54+
selectedModelId,
55+
isReasoningEffortSupported,
56+
)
57+
58+
// Build available reasoning efforts list
4659
const baseEfforts = [...reasoningEfforts] as ReasoningEffortWithMinimal[]
47-
const showMinimalOption = isGpt5Model || (isOpenRouterProvider && isReasoningEffortSupported)
4860
const availableReasoningEfforts: ReadonlyArray<ReasoningEffortWithMinimal> = showMinimalOption
4961
? (["minimal", ...baseEfforts] as ReasoningEffortWithMinimal[])
5062
: baseEfforts

0 commit comments

Comments
 (0)