refactor: consolidate ThinkingBudget components and fix disable handling (#9930)

This commit is contained in:
Hannes Rudolph 2025-12-08 18:23:49 -07:00 committed by GitHub
parent 93a43e427e
commit de00ab10e2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 78 additions and 374 deletions

View file

@ -275,7 +275,6 @@ describe("getModelParams", () => {
expect(result.reasoningBudget).toBeUndefined()
expect(result.temperature).toBe(0)
expect(result.reasoning).toBeUndefined()
})
it("should honor customMaxTokens for reasoning budget models", () => {
@ -557,7 +556,6 @@ describe("getModelParams", () => {
})
expect(result.reasoningEffort).toBeUndefined()
expect(result.reasoning).toBeUndefined()
})
it("should handle reasoning effort for openrouter format", () => {
@ -624,7 +622,6 @@ describe("getModelParams", () => {
})
expect(result.reasoningEffort).toBeUndefined()
expect(result.reasoning).toBeUndefined()
})
it("should include 'minimal' and 'none' for openrouter format", () => {
@ -670,7 +667,7 @@ describe("getModelParams", () => {
it("should use reasoningEffort if supportsReasoningEffort is false but reasoningEffort is set", () => {
const model: ModelInfo = {
...baseModel,
maxTokens: 3000, // Changed to 3000 (18.75% of 16000), which is within 20% threshold
maxTokens: 3000, // 3000 is 18.75% of 16000, within 20% threshold
supportsReasoningEffort: false,
reasoningEffort: "medium",
}
@ -681,7 +678,8 @@ describe("getModelParams", () => {
model,
})
expect(result.maxTokens).toBe(3000) // Now uses model.maxTokens since it's within 20% threshold
expect(result.maxTokens).toBe(3000)
// Now uses model.maxTokens since it's within 20% threshold
expect(result.reasoningEffort).toBe("medium")
})
})
@ -735,7 +733,7 @@ describe("getModelParams", () => {
})
expect(result.reasoningBudget).toBe(3200) // 80% of 4000
expect(result.reasoningEffort).toBeUndefined()
expect(result.reasoningEffort).toBeUndefined() // Budget takes precedence
expect(result.temperature).toBe(1.0)
})
@ -889,8 +887,6 @@ describe("getModelParams", () => {
settings: {},
model,
})
expect(result.reasoning).toBeUndefined()
})
})

View file

@ -130,10 +130,12 @@ export function getModelParams({
temperature = 1.0
} else if (shouldUseReasoningEffort({ model, settings })) {
// "Traditional" reasoning models use the `reasoningEffort` parameter.
const effort = (customReasoningEffort ?? model.reasoningEffort) as
| ReasoningEffortExtended
| "disable"
| undefined
// Only fallback to model default if user hasn't explicitly set a value.
// If customReasoningEffort is "disable", don't fallback to model default.
const effort =
customReasoningEffort !== undefined
? customReasoningEffort
: (model.reasoningEffort as ReasoningEffortExtended | "disable" | undefined)
// Capability and settings checks are handled by shouldUseReasoningEffort.
// Here we simply propagate the resolved effort into the params, while
// still treating "disable" as an omission.

View file

@ -379,7 +379,6 @@ describe("shouldUseReasoningEffort", () => {
reasoningEffort: "medium",
}
// Should return true regardless of settings (unless explicitly disabled)
expect(shouldUseReasoningEffort({ model })).toBe(true)
expect(shouldUseReasoningEffort({ model, settings: {} })).toBe(true)
expect(shouldUseReasoningEffort({ model, settings: { reasoningEffort: undefined } })).toBe(true)
@ -444,7 +443,7 @@ describe("shouldUseReasoningEffort", () => {
expect(shouldUseReasoningEffort({ model })).toBe(false)
})
test("should return false when model doesn't support reasoning effort", () => {
test("should return false when model doesn't support reasoning effort and has no default", () => {
const model: ModelInfo = {
contextWindow: 200_000,
supportsPromptCache: true,

View file

@ -109,7 +109,6 @@ import { inputEventTransform, noTransform } from "./transforms"
import { ModelInfoView } from "./ModelInfoView"
import { ApiErrorMessage } from "./ApiErrorMessage"
import { ThinkingBudget } from "./ThinkingBudget"
import { SimpleThinkingBudget } from "./SimpleThinkingBudget"
import { Verbosity } from "./Verbosity"
import { DiffSettingsControl } from "./DiffSettingsControl"
import { TodoListSettingsControl } from "./TodoListSettingsControl"
@ -855,22 +854,14 @@ const ApiOptions = ({
</>
)}
{!fromWelcomeView &&
(selectedProvider === "roo" ? (
<SimpleThinkingBudget
key={`${selectedProvider}-${selectedModelId}`}
apiConfiguration={apiConfiguration}
setApiConfigurationField={setApiConfigurationField}
modelInfo={selectedModelInfo}
/>
) : (
<ThinkingBudget
key={`${selectedProvider}-${selectedModelId}`}
apiConfiguration={apiConfiguration}
setApiConfigurationField={setApiConfigurationField}
modelInfo={selectedModelInfo}
/>
))}
{!fromWelcomeView && (
<ThinkingBudget
key={`${selectedProvider}-${selectedModelId}`}
apiConfiguration={apiConfiguration}
setApiConfigurationField={setApiConfigurationField}
modelInfo={selectedModelInfo}
/>
)}
{/* Gate Verbosity UI by capability flag */}
{!fromWelcomeView && selectedModelInfo?.supportsVerbosity && (

View file

@ -1,120 +0,0 @@
import { useEffect } from "react"
import { type ProviderSettings, type ModelInfo, type ReasoningEffort, reasoningEfforts } from "@roo-code/types"
import { useAppTranslation } from "@src/i18n/TranslationContext"
import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@src/components/ui"
interface SimpleThinkingBudgetProps {
apiConfiguration: ProviderSettings
setApiConfigurationField: <K extends keyof ProviderSettings>(
field: K,
value: ProviderSettings[K],
isUserAction?: boolean,
) => void
modelInfo?: ModelInfo
}
// Extended type to include "none" option
type ReasoningEffortWithNone = ReasoningEffort | "none"
export const SimpleThinkingBudget = ({
apiConfiguration,
setApiConfigurationField,
modelInfo,
}: SimpleThinkingBudgetProps) => {
const { t } = useAppTranslation()
// Check model capabilities
const isReasoningEffortSupported = !!modelInfo && modelInfo.supportsReasoningEffort
const isReasoningEffortRequired = !!modelInfo && modelInfo.requiredReasoningEffort
// Build available reasoning efforts list
// Include "none" option unless reasoning effort is required
const baseEfforts = [...reasoningEfforts] as ReasoningEffort[]
const availableReasoningEfforts: ReadonlyArray<ReasoningEffortWithNone> = isReasoningEffortRequired
? baseEfforts
: (["none", ...baseEfforts] as ReasoningEffortWithNone[])
// Default reasoning effort - use model's default if available, otherwise "medium"
const modelDefaultReasoningEffort = modelInfo?.reasoningEffort as ReasoningEffort | undefined
const defaultReasoningEffort: ReasoningEffortWithNone = isReasoningEffortRequired
? modelDefaultReasoningEffort || "medium"
: "none"
// Current reasoning effort - treat undefined/null as "none"
const currentReasoningEffort: ReasoningEffortWithNone =
(apiConfiguration.reasoningEffort as ReasoningEffort | undefined) || defaultReasoningEffort
// Set default reasoning effort when model supports it and no value is set
useEffect(() => {
if (isReasoningEffortSupported && !apiConfiguration.reasoningEffort) {
// Only set a default if reasoning is required, otherwise leave as undefined (which maps to "none")
if (isReasoningEffortRequired && defaultReasoningEffort !== "none") {
setApiConfigurationField("reasoningEffort", defaultReasoningEffort as ReasoningEffort, false)
}
}
}, [
isReasoningEffortSupported,
isReasoningEffortRequired,
apiConfiguration.reasoningEffort,
defaultReasoningEffort,
setApiConfigurationField,
])
useEffect(() => {
if (!isReasoningEffortSupported) return
const shouldEnable = isReasoningEffortRequired || currentReasoningEffort !== "none"
if (shouldEnable && apiConfiguration.enableReasoningEffort !== true) {
setApiConfigurationField("enableReasoningEffort", true, false)
}
}, [
isReasoningEffortSupported,
isReasoningEffortRequired,
currentReasoningEffort,
apiConfiguration.enableReasoningEffort,
setApiConfigurationField,
])
if (!modelInfo || !isReasoningEffortSupported) {
return null
}
return (
<div className="flex flex-col gap-1" data-testid="simple-reasoning-effort">
<div className="flex justify-between items-center">
<label className="block font-medium mb-1">{t("settings:providers.reasoningEffort.label")}</label>
</div>
<Select
value={currentReasoningEffort}
onValueChange={(value: ReasoningEffortWithNone) => {
// If "none" is selected, clear the reasoningEffort field
if (value === "none") {
setApiConfigurationField("reasoningEffort", undefined)
} else {
setApiConfigurationField("reasoningEffort", value as ReasoningEffort)
}
}}>
<SelectTrigger className="w-full">
<SelectValue
placeholder={
currentReasoningEffort
? currentReasoningEffort === "none"
? t("settings:providers.reasoningEffort.none")
: t(`settings:providers.reasoningEffort.${currentReasoningEffort}`)
: t("settings:common.select")
}
/>
</SelectTrigger>
<SelectContent>
{availableReasoningEfforts.map((value) => (
<SelectItem key={value} value={value}>
{value === "none"
? t("settings:providers.reasoningEffort.none")
: t(`settings:providers.reasoningEffort.${value}`)}
</SelectItem>
))}
</SelectContent>
</Select>
</div>
)
}

View file

@ -78,26 +78,62 @@ export const ThinkingBudget = ({ apiConfiguration, setApiConfigurationField, mod
// Build available reasoning efforts list from capability
const supports = modelInfo?.supportsReasoningEffort
const availableOptions: ReadonlyArray<ReasoningEffortWithMinimal> =
const baseAvailableOptions: ReadonlyArray<ReasoningEffortWithMinimal> =
supports === true
? (reasoningEfforts as readonly ReasoningEffortWithMinimal[])
: Array.isArray(supports)
? (supports as ReadonlyArray<ReasoningEffortWithMinimal>)
: (reasoningEfforts as readonly ReasoningEffortWithMinimal[])
// "disable" turns off reasoning entirely; "none" is a valid reasoning level.
// Both display as "None" in the UI but behave differently.
// Add "disable" option if reasoning effort is not required.
type ReasoningEffortOption = ReasoningEffortWithMinimal | "none" | "disable"
const availableOptions: ReadonlyArray<ReasoningEffortOption> = modelInfo?.requiredReasoningEffort
? (baseAvailableOptions as ReadonlyArray<ReasoningEffortOption>)
: (["disable", ...baseAvailableOptions] as ReasoningEffortOption[])
// Default reasoning effort - use model's default if available
// GPT-5 models have "medium" as their default in the model configuration
const modelDefaultReasoningEffort = modelInfo?.reasoningEffort as ReasoningEffortWithMinimal | undefined
const defaultReasoningEffort: ReasoningEffortWithMinimal = modelDefaultReasoningEffort || "medium"
const currentReasoningEffort: ReasoningEffortWithMinimal =
(apiConfiguration.reasoningEffort as ReasoningEffortWithMinimal | undefined) || defaultReasoningEffort
const defaultReasoningEffort: ReasoningEffortOption = modelInfo?.requiredReasoningEffort
? modelDefaultReasoningEffort || "medium"
: "disable"
// Current reasoning effort from settings, or fall back to default
const storedReasoningEffort = apiConfiguration.reasoningEffort as ReasoningEffortOption | undefined
const currentReasoningEffort: ReasoningEffortOption = storedReasoningEffort || defaultReasoningEffort
// Set default reasoning effort when model supports it and no value is set
useEffect(() => {
if (isReasoningEffortSupported && !apiConfiguration.reasoningEffort && defaultReasoningEffort) {
setApiConfigurationField("reasoningEffort", defaultReasoningEffort, false)
if (isReasoningEffortSupported && !apiConfiguration.reasoningEffort) {
// Only set a default if reasoning is required, otherwise leave as undefined (which maps to "disable")
if (modelInfo?.requiredReasoningEffort && defaultReasoningEffort !== "disable") {
setApiConfigurationField("reasoningEffort", defaultReasoningEffort as ReasoningEffortWithMinimal, false)
}
}
}, [isReasoningEffortSupported, apiConfiguration.reasoningEffort, defaultReasoningEffort, setApiConfigurationField])
}, [
isReasoningEffortSupported,
apiConfiguration.reasoningEffort,
defaultReasoningEffort,
modelInfo?.requiredReasoningEffort,
setApiConfigurationField,
])
// Sync enableReasoningEffort based on selection
// "disable" turns off reasoning; "none" is a valid level (reasoning enabled)
useEffect(() => {
if (!isReasoningEffortSupported) return
const shouldEnable = modelInfo?.requiredReasoningEffort || currentReasoningEffort !== "disable"
if (shouldEnable && apiConfiguration.enableReasoningEffort !== true) {
setApiConfigurationField("enableReasoningEffort", true, false)
}
}, [
isReasoningEffortSupported,
modelInfo?.requiredReasoningEffort,
currentReasoningEffort,
apiConfiguration.enableReasoningEffort,
setApiConfigurationField,
])
const enableReasoningEffort = apiConfiguration.enableReasoningEffort
const customMaxOutputTokens = apiConfiguration.modelMaxTokens || DEFAULT_HYBRID_REASONING_MODEL_MAX_TOKENS
@ -193,14 +229,24 @@ export const ThinkingBudget = ({ apiConfiguration, setApiConfigurationField, mod
</div>
<Select
value={currentReasoningEffort}
onValueChange={(value: ReasoningEffortWithMinimal) => {
setApiConfigurationField("reasoningEffort", value)
onValueChange={(value: ReasoningEffortOption) => {
// "disable" turns off reasoning entirely; "none" is a valid reasoning level
if (value === "disable") {
setApiConfigurationField("enableReasoningEffort", false)
setApiConfigurationField("reasoningEffort", "disable")
} else {
// "none", "minimal", "low", "medium", "high" all enable reasoning
setApiConfigurationField("enableReasoningEffort", true)
setApiConfigurationField("reasoningEffort", value as ReasoningEffortWithMinimal)
}
}}>
<SelectTrigger className="w-full">
<SelectValue
placeholder={
currentReasoningEffort
? t(`settings:providers.reasoningEffort.${currentReasoningEffort}`)
? currentReasoningEffort === "none" || currentReasoningEffort === "disable"
? t("settings:providers.reasoningEffort.none")
: t(`settings:providers.reasoningEffort.${currentReasoningEffort}`)
: t("settings:common.select")
}
/>
@ -208,7 +254,9 @@ export const ThinkingBudget = ({ apiConfiguration, setApiConfigurationField, mod
<SelectContent>
{availableOptions.map((value) => (
<SelectItem key={value} value={value}>
{t(`settings:providers.reasoningEffort.${value}`)}
{value === "none" || value === "disable"
? t("settings:providers.reasoningEffort.none")
: t(`settings:providers.reasoningEffort.${value}`)}
</SelectItem>
))}
</SelectContent>

View file

@ -1,212 +0,0 @@
import { describe, it, expect, vi, beforeEach } from "vitest"
import { render, screen } from "@testing-library/react"
import { SimpleThinkingBudget } from "../SimpleThinkingBudget"
import type { ProviderSettings, ModelInfo } from "@roo-code/types"
// Mock the translation hook
vi.mock("@src/i18n/TranslationContext", () => ({
useAppTranslation: () => ({
t: (key: string) => {
const translations: Record<string, string> = {
"settings:providers.reasoningEffort.label": "Model Reasoning Effort",
"settings:providers.reasoningEffort.none": "None",
"settings:providers.reasoningEffort.low": "Low",
"settings:providers.reasoningEffort.medium": "Medium",
"settings:providers.reasoningEffort.high": "High",
"settings:common.select": "Select",
}
return translations[key] || key
},
}),
}))
// Mock the useSelectedModel hook
vi.mock("@src/components/ui/hooks/useSelectedModel", () => ({
useSelectedModel: () => ({ id: "test-model" }),
}))
describe("SimpleThinkingBudget", () => {
const mockSetApiConfigurationField = vi.fn()
const baseApiConfiguration: ProviderSettings = {
apiProvider: "roo",
}
const modelWithReasoningEffort: ModelInfo = {
maxTokens: 8192,
contextWindow: 128000,
supportsImages: false,
supportsPromptCache: true,
supportsReasoningEffort: true,
inputPrice: 0,
outputPrice: 0,
}
beforeEach(() => {
vi.clearAllMocks()
})
it("should not render when model does not support reasoning effort", () => {
const modelWithoutReasoningEffort: ModelInfo = {
...modelWithReasoningEffort,
supportsReasoningEffort: false,
}
const { container } = render(
<SimpleThinkingBudget
apiConfiguration={baseApiConfiguration}
setApiConfigurationField={mockSetApiConfigurationField}
modelInfo={modelWithoutReasoningEffort}
/>,
)
expect(container.firstChild).toBeNull()
})
it("should not render when modelInfo is undefined", () => {
const { container } = render(
<SimpleThinkingBudget
apiConfiguration={baseApiConfiguration}
setApiConfigurationField={mockSetApiConfigurationField}
modelInfo={undefined}
/>,
)
expect(container.firstChild).toBeNull()
})
it("should render with None option when reasoning effort is not required", () => {
render(
<SimpleThinkingBudget
apiConfiguration={baseApiConfiguration}
setApiConfigurationField={mockSetApiConfigurationField}
modelInfo={modelWithReasoningEffort}
/>,
)
expect(screen.getByTestId("simple-reasoning-effort")).toBeInTheDocument()
expect(screen.getByText("Model Reasoning Effort")).toBeInTheDocument()
})
it("should not render None option when reasoning effort is required", () => {
const modelWithRequiredReasoningEffort: ModelInfo = {
...modelWithReasoningEffort,
requiredReasoningEffort: true,
}
render(
<SimpleThinkingBudget
apiConfiguration={baseApiConfiguration}
setApiConfigurationField={mockSetApiConfigurationField}
modelInfo={modelWithRequiredReasoningEffort}
/>,
)
expect(screen.getByTestId("simple-reasoning-effort")).toBeInTheDocument()
})
it("should set default reasoning effort when required and no value is set", () => {
const modelWithRequiredReasoningEffort: ModelInfo = {
...modelWithReasoningEffort,
requiredReasoningEffort: true,
reasoningEffort: "high",
}
render(
<SimpleThinkingBudget
apiConfiguration={baseApiConfiguration}
setApiConfigurationField={mockSetApiConfigurationField}
modelInfo={modelWithRequiredReasoningEffort}
/>,
)
// Should set default reasoning effort
expect(mockSetApiConfigurationField).toHaveBeenCalledWith("reasoningEffort", "high", false)
})
it("should not set default reasoning effort when not required", () => {
render(
<SimpleThinkingBudget
apiConfiguration={baseApiConfiguration}
setApiConfigurationField={mockSetApiConfigurationField}
modelInfo={modelWithReasoningEffort}
/>,
)
// Should not set any default value
expect(mockSetApiConfigurationField).not.toHaveBeenCalled()
})
it("should include None option in available efforts when not required", () => {
render(
<SimpleThinkingBudget
apiConfiguration={baseApiConfiguration}
setApiConfigurationField={mockSetApiConfigurationField}
modelInfo={modelWithReasoningEffort}
/>,
)
// Component should render with the select
expect(screen.getByRole("combobox")).toBeInTheDocument()
})
it("should exclude None option when reasoning effort is required", () => {
const modelWithRequiredReasoningEffort: ModelInfo = {
...modelWithReasoningEffort,
requiredReasoningEffort: true,
}
render(
<SimpleThinkingBudget
apiConfiguration={baseApiConfiguration}
setApiConfigurationField={mockSetApiConfigurationField}
modelInfo={modelWithRequiredReasoningEffort}
/>,
)
// Component should render with the select
expect(screen.getByRole("combobox")).toBeInTheDocument()
})
it("should display current reasoning effort value", () => {
render(
<SimpleThinkingBudget
apiConfiguration={{ ...baseApiConfiguration, reasoningEffort: "low" }}
setApiConfigurationField={mockSetApiConfigurationField}
modelInfo={modelWithReasoningEffort}
/>,
)
expect(screen.getByText("Low")).toBeInTheDocument()
})
it("should display None when no reasoning effort is set", () => {
render(
<SimpleThinkingBudget
apiConfiguration={baseApiConfiguration}
setApiConfigurationField={mockSetApiConfigurationField}
modelInfo={modelWithReasoningEffort}
/>,
)
expect(screen.getByText("None")).toBeInTheDocument()
})
it("should use model default reasoning effort when required and available", () => {
const modelWithDefaultEffort: ModelInfo = {
...modelWithReasoningEffort,
requiredReasoningEffort: true,
reasoningEffort: "medium",
}
render(
<SimpleThinkingBudget
apiConfiguration={baseApiConfiguration}
setApiConfigurationField={mockSetApiConfigurationField}
modelInfo={modelWithDefaultEffort}
/>,
)
expect(mockSetApiConfigurationField).toHaveBeenCalledWith("reasoningEffort", "medium", false)
})
})