Skip to content

Commit 425e5df

Browse files
rossross
committed
Rate-limit setting updated to be per-profile
1 parent e8624e8 commit 425e5df

27 files changed

+275
-39
lines changed

‎.changeset/curvy-masks-scream.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"roo-cline": minor
3+
---
4+
5+
Rate-limit setting updated to be per-profile

‎src/core/config/ProviderSettingsManager.ts‎

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@ export const providerProfilesSchema = z.object({
1313
currentApiConfigName: z.string(),
1414
apiConfigs: z.record(z.string(), providerSettingsWithIdSchema),
1515
modeApiConfigs: z.record(z.string(), z.string()).optional(),
16+
migrations: z
17+
.object({
18+
rateLimitSecondsMigrated: z.boolean().optional(),
19+
})
20+
.optional(),
1621
})
1722

1823
export type ProviderProfiles = z.infer<typeof providerProfilesSchema>
@@ -27,8 +32,16 @@ export class ProviderSettingsManager {
2732

2833
private readonly defaultProviderProfiles: ProviderProfiles = {
2934
currentApiConfigName: "default",
30-
apiConfigs: { default: { id: this.defaultConfigId } },
35+
apiConfigs: {
36+
default: {
37+
id: this.defaultConfigId,
38+
rateLimitSeconds: 0,
39+
},
40+
},
3141
modeApiConfigs: this.defaultModeApiConfigs,
42+
migrations: {
43+
rateLimitSecondsMigrated: true, // Mark as migrated on fresh installs
44+
},
3245
}
3346

3447
private readonly context: ExtensionContext
@@ -53,7 +66,7 @@ export class ProviderSettingsManager {
5366
}
5467

5568
/**
56-
* Initialize config if it doesn't exist.
69+
* Initialize config if it doesn't exist and run migrations.
5770
*/
5871
public async initialize() {
5972
try {
@@ -75,6 +88,18 @@ export class ProviderSettingsManager {
7588
}
7689
}
7790

91+
// Ensure migrations field exists
92+
if (!providerProfiles.migrations) {
93+
providerProfiles.migrations = { rateLimitSecondsMigrated: false } // Initialize with default values
94+
isDirty = true
95+
}
96+
97+
if (!providerProfiles.migrations.rateLimitSecondsMigrated) {
98+
await this.migrateRateLimitSeconds(providerProfiles)
99+
providerProfiles.migrations.rateLimitSecondsMigrated = true
100+
isDirty = true
101+
}
102+
78103
if (isDirty) {
79104
await this.store(providerProfiles)
80105
}
@@ -84,6 +109,36 @@ export class ProviderSettingsManager {
84109
}
85110
}
86111

112+
private async migrateRateLimitSeconds(providerProfiles: ProviderProfiles) {
113+
try {
114+
let rateLimitSeconds: number | undefined
115+
116+
try {
117+
rateLimitSeconds = await this.context.globalState.get<number>("rateLimitSeconds")
118+
} catch (error) {
119+
console.error("[MigrateRateLimitSeconds] Error getting global rate limit:", error)
120+
}
121+
122+
if (rateLimitSeconds === undefined) {
123+
// Failed to get the existing value, use the default
124+
rateLimitSeconds = 0
125+
}
126+
127+
for (const [name, apiConfig] of Object.entries(providerProfiles.apiConfigs)) {
128+
if (apiConfig.rateLimitSeconds === undefined) {
129+
console.log(
130+
`[MigrateRateLimitSeconds] Applying rate limit ${rateLimitSeconds}s to profile: ${name}`,
131+
)
132+
apiConfig.rateLimitSeconds = rateLimitSeconds
133+
}
134+
}
135+
136+
console.log(`[MigrateRateLimitSeconds] migration complete`)
137+
} catch (error) {
138+
console.error(`[MigrateRateLimitSeconds] Failed to migrate rate limit settings:`, error)
139+
}
140+
}
141+
87142
/**
88143
* List all available configs with metadata.
89144
*/

‎src/core/config/__tests__/ProviderSettingsManager.test.ts‎

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,14 @@ const mockSecrets = {
1212
delete: jest.fn(),
1313
}
1414

15+
const mockGlobalState = {
16+
get: jest.fn(),
17+
update: jest.fn(),
18+
}
19+
1520
const mockContext = {
1621
secrets: mockSecrets,
22+
globalState: mockGlobalState,
1723
} as unknown as ExtensionContext
1824

1925
describe("ProviderSettingsManager", () => {
@@ -45,6 +51,9 @@ describe("ProviderSettingsManager", () => {
4551
id: "default",
4652
},
4753
},
54+
migrations: {
55+
rateLimitSecondsMigrated: true,
56+
},
4857
}),
4958
)
5059

@@ -78,6 +87,43 @@ describe("ProviderSettingsManager", () => {
7887
expect(storedConfig.apiConfigs.test.id).toBeTruthy()
7988
})
8089

90+
it("should call migrateRateLimitSeconds if it has not done so already", async () => {
91+
mockGlobalState.get.mockResolvedValue(42)
92+
93+
mockSecrets.get.mockResolvedValue(
94+
JSON.stringify({
95+
currentApiConfigName: "default",
96+
apiConfigs: {
97+
default: {
98+
config: {},
99+
id: "default",
100+
rateLimitSeconds: undefined,
101+
},
102+
test: {
103+
apiProvider: "anthropic",
104+
rateLimitSeconds: undefined,
105+
},
106+
existing: {
107+
apiProvider: "anthropic",
108+
// this should not really be possible, unless someone has loaded a hand edited config,
109+
// but we don't overwrite so we'll check that
110+
rateLimitSeconds: 43,
111+
},
112+
},
113+
migrations: {
114+
rateLimitSecondsMigrate: false,
115+
},
116+
}),
117+
)
118+
119+
await providerSettingsManager.initialize()
120+
121+
const storedConfig = JSON.parse(mockSecrets.store.mock.calls[1][1])
122+
expect(storedConfig.apiConfigs.default.rateLimitSeconds).toEqual(42)
123+
expect(storedConfig.apiConfigs.test.rateLimitSeconds).toEqual(42)
124+
expect(storedConfig.apiConfigs.existing.rateLimitSeconds).toEqual(43)
125+
})
126+
81127
it("should throw error if secrets storage fails", async () => {
82128
mockSecrets.get.mockRejectedValue(new Error("Storage failed"))
83129

@@ -105,6 +151,9 @@ describe("ProviderSettingsManager", () => {
105151
architect: "default",
106152
ask: "default",
107153
},
154+
migrations: {
155+
rateLimitSecondsMigrated: false,
156+
},
108157
}
109158

110159
mockSecrets.get.mockResolvedValue(JSON.stringify(existingConfig))
@@ -125,6 +174,9 @@ describe("ProviderSettingsManager", () => {
125174
architect: "default",
126175
ask: "default",
127176
},
177+
migrations: {
178+
rateLimitSecondsMigrated: false,
179+
},
128180
}
129181

130182
mockSecrets.get.mockResolvedValue(JSON.stringify(emptyConfig))
@@ -201,6 +253,9 @@ describe("ProviderSettingsManager", () => {
201253
id: "test-id",
202254
},
203255
},
256+
migrations: {
257+
rateLimitSecondsMigrated: false,
258+
},
204259
}
205260

206261
mockSecrets.get.mockResolvedValue(JSON.stringify(existingConfig))
@@ -221,6 +276,9 @@ describe("ProviderSettingsManager", () => {
221276
id: "test-id",
222277
},
223278
},
279+
migrations: {
280+
rateLimitSecondsMigrated: false,
281+
},
224282
}
225283

226284
expect(mockSecrets.store).toHaveBeenCalledWith(
@@ -257,6 +315,9 @@ describe("ProviderSettingsManager", () => {
257315
id: "test-id",
258316
},
259317
},
318+
migrations: {
319+
rateLimitSecondsMigrated: false,
320+
},
260321
}
261322

262323
mockSecrets.get.mockResolvedValue(JSON.stringify(existingConfig))
@@ -312,8 +373,12 @@ describe("ProviderSettingsManager", () => {
312373
id: "test-id",
313374
},
314375
},
376+
migrations: {
377+
rateLimitSecondsMigrated: false,
378+
},
315379
}
316380

381+
mockGlobalState.get.mockResolvedValue(42)
317382
mockSecrets.get.mockResolvedValue(JSON.stringify(existingConfig))
318383

319384
const config = await providerSettingsManager.loadConfig("test")
@@ -325,7 +390,7 @@ describe("ProviderSettingsManager", () => {
325390
})
326391

327392
// Get the stored config to check the structure
328-
const storedConfig = JSON.parse(mockSecrets.store.mock.calls[0][1])
393+
const storedConfig = JSON.parse(mockSecrets.store.mock.calls[1][1])
329394
expect(storedConfig.currentApiConfigName).toBe("test")
330395
expect(storedConfig.apiConfigs.test).toEqual({
331396
apiProvider: "anthropic",
@@ -409,6 +474,9 @@ describe("ProviderSettingsManager", () => {
409474
id: "test-id",
410475
},
411476
},
477+
migrations: {
478+
rateLimitSecondsMigrated: false,
479+
},
412480
}
413481

414482
mockSecrets.get.mockResolvedValue(JSON.stringify(existingConfig))

‎src/core/config/__tests__/importExport.test.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ describe("importExport", () => {
121121
},
122122
},
123123
}
124+
124125
mockProviderSettingsManager.export.mockResolvedValue(previousProviderProfiles)
125126

126127
// Mock listConfig
@@ -294,6 +295,9 @@ describe("importExport", () => {
294295
id: "test-id",
295296
},
296297
},
298+
migrations: {
299+
rateLimitSecondsMigrated: false,
300+
},
297301
}
298302
mockProviderSettingsManager.export.mockResolvedValue(mockProviderProfiles)
299303

@@ -345,6 +349,9 @@ describe("importExport", () => {
345349
id: "test-id",
346350
},
347351
},
352+
migrations: {
353+
rateLimitSecondsMigrated: false,
354+
},
348355
})
349356

350357
// Mock global settings
@@ -384,6 +391,9 @@ describe("importExport", () => {
384391
id: "test-id",
385392
},
386393
},
394+
migrations: {
395+
rateLimitSecondsMigrated: false,
396+
},
387397
})
388398

389399
// Mock global settings

‎src/exports/roo-code.d.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@ type ProviderSettings = {
177177
modelMaxTokens?: number | undefined
178178
modelMaxThinkingTokens?: number | undefined
179179
includeMaxTokens?: boolean | undefined
180+
rateLimitSeconds?: number | undefined
180181
fakeAi?: unknown | undefined
181182
}
182183

‎src/exports/types.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ type ProviderSettings = {
178178
modelMaxTokens?: number | undefined
179179
modelMaxThinkingTokens?: number | undefined
180180
includeMaxTokens?: boolean | undefined
181+
rateLimitSeconds?: number | undefined
181182
fakeAi?: unknown | undefined
182183
}
183184

‎src/schemas/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,7 @@ export const providerSettingsSchema = z.object({
386386
modelMaxThinkingTokens: z.number().optional(),
387387
// Generic
388388
includeMaxTokens: z.boolean().optional(),
389+
rateLimitSeconds: z.number().optional(),
389390
// Fake AI
390391
fakeAi: z.unknown().optional(),
391392
})
@@ -470,6 +471,7 @@ const providerSettingsRecord: ProviderSettingsRecord = {
470471
modelMaxThinkingTokens: undefined,
471472
// Generic
472473
includeMaxTokens: undefined,
474+
rateLimitSeconds: undefined,
473475
// Fake AI
474476
fakeAi: undefined,
475477
}

‎webview-ui/src/components/settings/AdvancedSettings.tsx‎

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,11 @@ import { SectionHeader } from "./SectionHeader"
1111
import { Section } from "./Section"
1212

1313
type AdvancedSettingsProps = HTMLAttributes<HTMLDivElement> & {
14-
rateLimitSeconds: number
1514
diffEnabled?: boolean
1615
fuzzyMatchThreshold?: number
17-
setCachedStateField: SetCachedStateField<"rateLimitSeconds" | "diffEnabled" | "fuzzyMatchThreshold">
16+
setCachedStateField: SetCachedStateField<"diffEnabled" | "fuzzyMatchThreshold">
1817
}
1918
export const AdvancedSettings = ({
20-
rateLimitSeconds,
2119
diffEnabled,
2220
fuzzyMatchThreshold,
2321
setCachedStateField,
@@ -36,25 +34,6 @@ export const AdvancedSettings = ({
3634
</SectionHeader>
3735

3836
<Section>
39-
<div>
40-
<div className="flex flex-col gap-2">
41-
<span className="font-medium">{t("settings:advanced.rateLimit.label")}</span>
42-
<div className="flex items-center gap-2">
43-
<Slider
44-
min={0}
45-
max={60}
46-
step={1}
47-
value={[rateLimitSeconds]}
48-
onValueChange={([value]) => setCachedStateField("rateLimitSeconds", value)}
49-
/>
50-
<span className="w-10">{rateLimitSeconds}s</span>
51-
</div>
52-
</div>
53-
<div className="text-vscode-descriptionForeground text-sm mt-1">
54-
{t("settings:advanced.rateLimit.description")}
55-
</div>
56-
</div>
57-
5837
<div>
5938
<VSCodeCheckbox
6039
checked={diffEnabled}

‎webview-ui/src/components/settings/ApiOptions.tsx‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ import { VSCodeButtonLink } from "../common/VSCodeButtonLink"
5252
import { ModelInfoView } from "./ModelInfoView"
5353
import { ModelPicker } from "./ModelPicker"
5454
import { TemperatureControl } from "./TemperatureControl"
55+
import { RateLimitSecondsControl } from "./RateLimitSecondsControl"
5556
import { ApiErrorMessage } from "./ApiErrorMessage"
5657
import { ThinkingBudget } from "./ThinkingBudget"
5758
import { R1FormatSetting } from "./R1FormatSetting"
@@ -1623,11 +1624,17 @@ const ApiOptions = ({
16231624
)}
16241625

16251626
{!fromWelcomeView && (
1626-
<TemperatureControl
1627-
value={apiConfiguration?.modelTemperature}
1628-
onChange={handleInputChange("modelTemperature", noTransform)}
1629-
maxValue={2}
1630-
/>
1627+
<>
1628+
<TemperatureControl
1629+
value={apiConfiguration?.modelTemperature}
1630+
onChange={handleInputChange("modelTemperature", noTransform)}
1631+
maxValue={2}
1632+
/>
1633+
<RateLimitSecondsControl
1634+
value={apiConfiguration.rateLimitSeconds || 0}
1635+
onChange={(value) => setApiConfigurationField("rateLimitSeconds", value)}
1636+
/>
1637+
</>
16311638
)}
16321639
</div>
16331640
)

0 commit comments

Comments
 (0)