Skip to content

Commit 698cc5b

Browse files
committed
refactor: update MaxOutputTokensControl to use ProviderSettings and adjust min tokens
1 parent 55a4115 commit 698cc5b

File tree

4 files changed

+362
-247
lines changed

4 files changed

+362
-247
lines changed

webview-ui/src/components/settings/MaxOutputTokensControl.test.tsx

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
import React from "react"
22
import { render, screen } from "@testing-library/react"
3+
4+
import type { ProviderSettings, ModelInfo } from "@roo-code/types"
5+
36
import { MaxOutputTokensControl } from "./MaxOutputTokensControl"
4-
import { ApiConfiguration, ModelInfo } from "@roo/shared/api"
7+
import { Slider } from "@src/components/ui"
58

69
// Mock ResizeObserver globally
710
global.ResizeObserver = jest.fn().mockImplementation(() => ({
@@ -11,33 +14,28 @@ global.ResizeObserver = jest.fn().mockImplementation(() => ({
1114
}))
1215
// Static import of Slider removed again to avoid TS error for non-existent module
1316

14-
// Mock i18n
15-
jest.mock("react-i18next", () => ({
16-
useTranslation: () => ({
17+
// Mock i18n translation context
18+
jest.mock("@src/i18n/TranslationContext", () => ({
19+
useAppTranslation: () => ({
1720
t: (key: string) => key,
1821
}),
1922
}))
2023

21-
// Use jest.doMock with { virtual: true } to mock a non-existent module.
22-
// This mock factory will be used when '../Slider' is imported.
23-
jest.mock("../ui/slider", () => ({
24-
__esModule: true,
24+
// Mock UI components
25+
jest.mock("@src/components/ui", () => ({
2526
Slider: jest.fn((props) => <div data-testid="mock-slider" {...props} />),
2627
}))
2728

2829
// Import the mocked Slider *after* jest.mock
2930
// We need to refer to it for assertions and clearing.
30-
// eslint-disable-next-line @typescript-eslint/no-var-requires
31-
const MockedSlider = require("../ui/slider").Slider as jest.Mock
31+
const MockedSlider = jest.mocked(Slider)
3232

3333
describe("MaxOutputTokensControl", () => {
3434
const mockSetApiConfigurationField = jest.fn()
35-
const mockApiConfiguration: ApiConfiguration = {
35+
const mockApiConfiguration: ProviderSettings = {
3636
apiProvider: "openai",
37-
openAiModelId: "test-model",
38-
modelTemperature: 0.5,
37+
apiModelId: "test-model",
3938
modelMaxTokens: undefined,
40-
modelMaxThinkingTokens: undefined,
4139
}
4240

4341
beforeEach(() => {
@@ -66,7 +64,7 @@ describe("MaxOutputTokensControl", () => {
6664
const modelInfo: Partial<ModelInfo> = {
6765
maxTokens: 16000,
6866
}
69-
const apiConfigurationWithOverride: ApiConfiguration = {
67+
const apiConfigurationWithOverride: ProviderSettings = {
7068
...mockApiConfiguration,
7169
modelMaxTokens: 10000, // Override the default maxTokens
7270
}
@@ -86,11 +84,11 @@ describe("MaxOutputTokensControl", () => {
8684
expect(screen.getByText(expectedValue.toString())).toBeInTheDocument()
8785

8886
// Assert that the label is displayed (using the mocked translation key)
89-
expect(screen.getByText("providers.customModel.maxTokens.label")).toBeInTheDocument()
87+
expect(screen.getByText("settings:thinkingBudget.maxTokens")).toBeInTheDocument()
9088
})
9189

9290
// Make the test async to use dynamic import
93-
it("should pass min={8192} to the Slider component when rendered", async () => {
91+
it("should pass min={2048} to the Slider component when rendered", async () => {
9492
// Dynamically import Slider; it will be the mocked version due to jest.doMock
9593
// Note: For this to work, the test function must be async and you await the import.
9694
// However, for prop checking on a mock function, we can directly use the
@@ -117,7 +115,7 @@ describe("MaxOutputTokensControl", () => {
117115
// This test is expected to fail because Slider is not yet used or not with this prop.
118116
expect(MockedSlider).toHaveBeenCalledWith(
119117
expect.objectContaining({
120-
min: 8192,
118+
min: 2048,
121119
}),
122120
expect.anything(), // Context for React components
123121
)
@@ -178,7 +176,7 @@ describe("MaxOutputTokensControl", () => {
178176
const modelInfo: Partial<ModelInfo> = {
179177
maxTokens: 32000, // This should be ignored
180178
}
181-
const apiConfigurationWithOverride: ApiConfiguration = {
179+
const apiConfigurationWithOverride: ProviderSettings = {
182180
...mockApiConfiguration,
183181
modelMaxTokens: 10000,
184182
}
@@ -203,7 +201,7 @@ describe("MaxOutputTokensControl", () => {
203201
const modelInfo: Partial<ModelInfo> = {
204202
maxTokens: 16000, // This should be used
205203
}
206-
const apiConfigurationWithoutOverride: ApiConfiguration = {
204+
const apiConfigurationWithoutOverride: ProviderSettings = {
207205
...mockApiConfiguration,
208206
modelMaxTokens: undefined,
209207
}
@@ -242,7 +240,7 @@ describe("MaxOutputTokensControl", () => {
242240
// MockedSlider.mock.calls[0][0] gives us the props of the first render call.
243241
const sliderProps = MockedSlider.mock.calls[0][0]
244242
const newValue = [20000]
245-
sliderProps.onValueChange(newValue)
243+
sliderProps.onValueChange?.(newValue)
246244

247245
// Assert that setApiConfigurationField was called with the correct arguments
248246
expect(mockSetApiConfigurationField).toHaveBeenCalledWith("modelMaxTokens", newValue[0])

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

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,25 @@
11
import React from "react"
2-
import { ApiConfiguration, ModelInfo } from "@roo/shared/api"
3-
import { Slider } from "../ui/slider"
4-
import { useTranslation } from "react-i18next"
2+
3+
import { type ProviderSettings, type ModelInfo } from "@roo-code/types"
4+
5+
import { useAppTranslation } from "@src/i18n/TranslationContext"
6+
import { Slider } from "@src/components/ui"
57

68
interface MaxOutputTokensControlProps {
7-
apiConfiguration: ApiConfiguration
8-
setApiConfigurationField: <K extends keyof ApiConfiguration>(field: K, value: ApiConfiguration[K]) => void
9+
apiConfiguration: ProviderSettings
10+
setApiConfigurationField: <K extends keyof ProviderSettings>(field: K, value: ProviderSettings[K]) => void
911
modelInfo?: ModelInfo
1012
}
1113

12-
const MIN_OUTPUT_TOKENS = 8192
14+
const MIN_OUTPUT_TOKENS = 2048
1315
const STEP_OUTPUT_TOKENS = 1024
1416

1517
export const MaxOutputTokensControl: React.FC<MaxOutputTokensControlProps> = ({
1618
apiConfiguration,
1719
setApiConfigurationField,
1820
modelInfo,
1921
}) => {
20-
const { t } = useTranslation()
22+
const { t } = useAppTranslation()
2123
const shouldRender = modelInfo && typeof modelInfo.maxTokens === "number" && modelInfo.maxTokens > 0
2224

2325
if (!shouldRender) {
@@ -26,20 +28,18 @@ export const MaxOutputTokensControl: React.FC<MaxOutputTokensControlProps> = ({
2628

2729
const currentMaxOutputTokens = apiConfiguration.modelMaxTokens ?? modelInfo.maxTokens!
2830

29-
// Basic structure for now, will be expanded in later tasks.
3031
return (
31-
<div data-testid="max-output-tokens-control">
32-
<label className="block mb-2">{t("providers.customModel.maxTokens.label")}</label>
33-
<div className="flex items-center space-x-2">
32+
<div className="flex flex-col gap-1" data-testid="max-output-tokens-control">
33+
<div className="font-medium">{t("settings:thinkingBudget.maxTokens")}</div>
34+
<div className="flex items-center gap-1">
3435
<Slider
35-
className="flex-grow"
3636
min={MIN_OUTPUT_TOKENS}
3737
max={modelInfo.maxTokens!}
3838
step={STEP_OUTPUT_TOKENS}
3939
value={[currentMaxOutputTokens]}
4040
onValueChange={([value]) => setApiConfigurationField("modelMaxTokens", value)}
4141
/>
42-
<div className="w-12 text-sm text-right">{currentMaxOutputTokens}</div>
42+
<div className="w-12 text-sm text-center">{currentMaxOutputTokens}</div>
4343
</div>
4444
</div>
4545
)

0 commit comments

Comments
 (0)