Skip to content

Commit 21ece1c

Browse files
fix(ThinkingBudget): gate max-output slider behind reasoning toggle
In the binary-reasoning path the max-output-tokens slider rendered even with reasoning off, but getModelMaxOutputTokens then ignores the value (returns the model default), so the control was interactive yet silently ignored. Guard it with enableReasoningEffort to match the budget path, and add tests covering both states. Addresses PR #332 review (edelauna).
1 parent 0801635 commit 21ece1c

2 files changed

Lines changed: 43 additions & 1 deletion

File tree

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,10 @@ export const ThinkingBudget = ({ apiConfiguration, setApiConfigurationField, mod
196196
}>
197197
{t("settings:providers.useReasoning")}
198198
</Checkbox>
199-
{maxOutputTokensControl}
199+
{/* Only show the max-output-tokens slider when reasoning is enabled: with it off,
200+
getModelMaxOutputTokens returns the model default regardless of the slider, so an
201+
interactive-but-ignored control would mislead. Matches the budget path's guard. */}
202+
{enableReasoningEffort && maxOutputTokensControl}
200203
</div>
201204
)
202205
}

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,45 @@ describe("ThinkingBudget", () => {
111111
expect(screen.queryByTestId("reasoning-effort")).not.toBeInTheDocument()
112112
})
113113

114+
const binaryReasoningModel: ModelInfo = {
115+
...mockModelInfo,
116+
supportsReasoningBinary: true,
117+
supportsReasoningBudget: false,
118+
supportsReasoningEffort: false,
119+
}
120+
121+
it("should hide the max-output-tokens slider in binary reasoning when reasoning is off", () => {
122+
// With reasoning disabled, getModelMaxOutputTokens ignores the slider value, so an
123+
// interactive control would be misleading — it must not render (PR #332 review).
124+
render(
125+
<ThinkingBudget
126+
{...defaultProps}
127+
apiConfiguration={{ enableReasoningEffort: false }}
128+
modelInfo={binaryReasoningModel}
129+
/>,
130+
)
131+
132+
expect(screen.getByText("settings:providers.useReasoning")).toBeInTheDocument()
133+
expect(screen.queryByTestId("slider")).not.toBeInTheDocument()
134+
})
135+
136+
it("should show and wire the max-output-tokens slider in binary reasoning when reasoning is on", () => {
137+
const setApiConfigurationField = vi.fn()
138+
render(
139+
<ThinkingBudget
140+
{...defaultProps}
141+
apiConfiguration={{ enableReasoningEffort: true }}
142+
setApiConfigurationField={setApiConfigurationField}
143+
modelInfo={binaryReasoningModel}
144+
/>,
145+
)
146+
147+
const slider = screen.getByTestId("slider")
148+
expect(slider).toBeInTheDocument()
149+
fireEvent.change(slider, { target: { value: "12288" } })
150+
expect(setApiConfigurationField).toHaveBeenCalledWith("modelMaxTokens", 12288)
151+
})
152+
114153
it("should render sliders when model supports thinking", () => {
115154
render(<ThinkingBudget {...defaultProps} />)
116155

0 commit comments

Comments
 (0)