Skip to content

Commit 08487e4

Browse files
committed
fix(settings): address PR review feedback for custom max tokens feature
- Simplify MaxTokensControl component by removing redundant fallback pattern - Update UI to show model's actual max tokens as default instead of hardcoded 8192 - Add validation in getModelMaxOutputTokens to cap user values to model's max - Add test case for validation when user sets higher than model's max - Update existing tests to expect capped values Addresses review feedback from daniel-lxs on PR #5788
1 parent 1829d51 commit 08487e4

File tree

3 files changed

+33
-8
lines changed

3 files changed

+33
-8
lines changed

src/shared/__tests__/api.spec.ts

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,8 @@ describe("getModelMaxOutputTokens", () => {
7373
settings,
7474
})
7575

76-
expect(result).toBe(32000)
76+
// Should cap to model's maxTokens (8192) even though user set 32000
77+
expect(result).toBe(8192)
7778
})
7879

7980
test("should return default of 8192 when maxTokens is undefined", () => {
@@ -166,7 +167,8 @@ describe("getModelMaxOutputTokens", () => {
166167
settings,
167168
})
168169

169-
expect(result).toBe(16000)
170+
// Should cap to model's maxTokens (8192) even though user set 16000
171+
expect(result).toBe(8192)
170172
})
171173

172174
test("should ignore modelMaxTokens when it's 0 or negative", () => {
@@ -216,7 +218,29 @@ describe("getModelMaxOutputTokens", () => {
216218
format: "anthropic",
217219
})
218220

219-
expect(result).toBe(20000)
221+
// Should cap to model's maxTokens (8192) even though user set 20000
222+
expect(result).toBe(8192)
223+
})
224+
225+
test("should cap modelMaxTokens to model's actual max when user sets higher value", () => {
226+
const model: ModelInfo = {
227+
maxTokens: 16000,
228+
contextWindow: 100000,
229+
supportsPromptCache: true,
230+
}
231+
232+
const settings: ProviderSettings = {
233+
modelMaxTokens: 32000, // Higher than model's max
234+
}
235+
236+
const result = getModelMaxOutputTokens({
237+
modelId: "some-model",
238+
model,
239+
settings,
240+
})
241+
242+
// Should cap at model's max
243+
expect(result).toBe(16000)
220244
})
221245
})
222246

src/shared/api.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,10 @@ export const getModelMaxOutputTokens = ({
7070
return settings.claudeCodeMaxOutputTokens || CLAUDE_CODE_DEFAULT_MAX_OUTPUT_TOKENS
7171
}
7272

73-
// Check for user-configured modelMaxTokens
73+
// Check for user-configured modelMaxTokens (takes precedence over all other logic)
7474
if (settings?.modelMaxTokens && settings.modelMaxTokens > 0) {
75-
return settings.modelMaxTokens
75+
const maxAllowed = model?.maxTokens || Number.MAX_SAFE_INTEGER
76+
return Math.min(settings.modelMaxTokens, maxAllowed)
7677
}
7778

7879
// Existing reasoning budget logic

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ export const MaxTokensControl: React.FC<MaxTokensControlProps> = ({
1717
onChange,
1818
modelInfo,
1919
minValue = 1000,
20-
maxValue = 200000,
20+
maxValue,
2121
className,
2222
}) => {
2323
const { t } = useAppTranslation()
@@ -35,8 +35,8 @@ export const MaxTokensControl: React.FC<MaxTokensControlProps> = ({
3535
}
3636
}
3737

38-
const effectiveMaxValue = modelInfo?.maxTokens || maxValue
39-
const displayValue = value ?? 8192
38+
const effectiveMaxValue = modelInfo?.maxTokens || maxValue || 100000
39+
const displayValue = value ?? modelInfo?.maxTokens ?? 8192
4040

4141
const isValueTooHigh = displayValue > effectiveMaxValue
4242
const isValueTooLow = displayValue < minValue

0 commit comments

Comments
 (0)