Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/curvy-masks-scream.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"roo-cline": minor
---

Rate-limit setting updated to be per-profile
59 changes: 57 additions & 2 deletions src/core/config/ProviderSettingsManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ export const providerProfilesSchema = z.object({
currentApiConfigName: z.string(),
apiConfigs: z.record(z.string(), providerSettingsWithIdSchema),
modeApiConfigs: z.record(z.string(), z.string()).optional(),
migrations: z
.object({
rateLimitSecondsMigrated: z.boolean().optional(),
})
.optional(),
})

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

private readonly defaultProviderProfiles: ProviderProfiles = {
currentApiConfigName: "default",
apiConfigs: { default: { id: this.defaultConfigId } },
apiConfigs: {
default: {
id: this.defaultConfigId,
rateLimitSeconds: 0,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary to add?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'd have to ask gemini 😁. Actually I wasn't 100% sure when I saw it and forgot to come back to it by the time I'd gotten other things happy. Will remove it and make sure that things are cleanly dealing w/not having a value for rateLimitSeconds in the default.

},
},
modeApiConfigs: this.defaultModeApiConfigs,
migrations: {
rateLimitSecondsMigrated: true, // Mark as migrated on fresh installs
},
}

private readonly context: ExtensionContext
Expand All @@ -53,7 +66,7 @@ export class ProviderSettingsManager {
}

/**
* Initialize config if it doesn't exist.
* Initialize config if it doesn't exist and run migrations.
*/
public async initialize() {
try {
Expand All @@ -75,6 +88,18 @@ export class ProviderSettingsManager {
}
}

// Ensure migrations field exists
if (!providerProfiles.migrations) {
providerProfiles.migrations = { rateLimitSecondsMigrated: false } // Initialize with default values
isDirty = true
}

if (!providerProfiles.migrations.rateLimitSecondsMigrated) {
await this.migrateRateLimitSeconds(providerProfiles)
providerProfiles.migrations.rateLimitSecondsMigrated = true
isDirty = true
}

if (isDirty) {
await this.store(providerProfiles)
}
Expand All @@ -84,6 +109,36 @@ export class ProviderSettingsManager {
}
}

private async migrateRateLimitSeconds(providerProfiles: ProviderProfiles) {
try {
let rateLimitSeconds: number | undefined

try {
rateLimitSeconds = await this.context.globalState.get<number>("rateLimitSeconds")
} catch (error) {
console.error("[MigrateRateLimitSeconds] Error getting global rate limit:", error)
}

if (rateLimitSeconds === undefined) {
// Failed to get the existing value, use the default
rateLimitSeconds = 0
}

for (const [name, apiConfig] of Object.entries(providerProfiles.apiConfigs)) {
if (apiConfig.rateLimitSeconds === undefined) {
console.log(
`[MigrateRateLimitSeconds] Applying rate limit ${rateLimitSeconds}s to profile: ${name}`,
)
apiConfig.rateLimitSeconds = rateLimitSeconds
}
}

console.log(`[MigrateRateLimitSeconds] migration complete`)
} catch (error) {
console.error(`[MigrateRateLimitSeconds] Failed to migrate rate limit settings:`, error)
}
}

/**
* List all available configs with metadata.
*/
Expand Down
70 changes: 69 additions & 1 deletion src/core/config/__tests__/ProviderSettingsManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,14 @@ const mockSecrets = {
delete: jest.fn(),
}

const mockGlobalState = {
get: jest.fn(),
update: jest.fn(),
}

const mockContext = {
secrets: mockSecrets,
globalState: mockGlobalState,
} as unknown as ExtensionContext

describe("ProviderSettingsManager", () => {
Expand Down Expand Up @@ -45,6 +51,9 @@ describe("ProviderSettingsManager", () => {
id: "default",
},
},
migrations: {
rateLimitSecondsMigrated: true,
},
}),
)

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

it("should call migrateRateLimitSeconds if it has not done so already", async () => {
mockGlobalState.get.mockResolvedValue(42)

mockSecrets.get.mockResolvedValue(
JSON.stringify({
currentApiConfigName: "default",
apiConfigs: {
default: {
config: {},
id: "default",
rateLimitSeconds: undefined,
},
test: {
apiProvider: "anthropic",
rateLimitSeconds: undefined,
},
existing: {
apiProvider: "anthropic",
// this should not really be possible, unless someone has loaded a hand edited config,
// but we don't overwrite so we'll check that
rateLimitSeconds: 43,
},
},
migrations: {
rateLimitSecondsMigrated: false,
},
}),
)

await providerSettingsManager.initialize()

const storedConfig = JSON.parse(mockSecrets.store.mock.calls[1][1])
expect(storedConfig.apiConfigs.default.rateLimitSeconds).toEqual(42)
expect(storedConfig.apiConfigs.test.rateLimitSeconds).toEqual(42)
expect(storedConfig.apiConfigs.existing.rateLimitSeconds).toEqual(43)
})

it("should throw error if secrets storage fails", async () => {
mockSecrets.get.mockRejectedValue(new Error("Storage failed"))

Expand Down Expand Up @@ -105,6 +151,9 @@ describe("ProviderSettingsManager", () => {
architect: "default",
ask: "default",
},
migrations: {
rateLimitSecondsMigrated: false,
},
}

mockSecrets.get.mockResolvedValue(JSON.stringify(existingConfig))
Expand All @@ -125,6 +174,9 @@ describe("ProviderSettingsManager", () => {
architect: "default",
ask: "default",
},
migrations: {
rateLimitSecondsMigrated: false,
},
}

mockSecrets.get.mockResolvedValue(JSON.stringify(emptyConfig))
Expand Down Expand Up @@ -201,6 +253,9 @@ describe("ProviderSettingsManager", () => {
id: "test-id",
},
},
migrations: {
rateLimitSecondsMigrated: false,
},
}

mockSecrets.get.mockResolvedValue(JSON.stringify(existingConfig))
Expand All @@ -221,6 +276,9 @@ describe("ProviderSettingsManager", () => {
id: "test-id",
},
},
migrations: {
rateLimitSecondsMigrated: false,
},
}

expect(mockSecrets.store).toHaveBeenCalledWith(
Expand Down Expand Up @@ -257,6 +315,9 @@ describe("ProviderSettingsManager", () => {
id: "test-id",
},
},
migrations: {
rateLimitSecondsMigrated: false,
},
}

mockSecrets.get.mockResolvedValue(JSON.stringify(existingConfig))
Expand Down Expand Up @@ -312,8 +373,12 @@ describe("ProviderSettingsManager", () => {
id: "test-id",
},
},
migrations: {
rateLimitSecondsMigrated: false,
},
}

mockGlobalState.get.mockResolvedValue(42)
mockSecrets.get.mockResolvedValue(JSON.stringify(existingConfig))

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

// Get the stored config to check the structure
const storedConfig = JSON.parse(mockSecrets.store.mock.calls[0][1])
const storedConfig = JSON.parse(mockSecrets.store.mock.calls[1][1])
expect(storedConfig.currentApiConfigName).toBe("test")
expect(storedConfig.apiConfigs.test).toEqual({
apiProvider: "anthropic",
Expand Down Expand Up @@ -409,6 +474,9 @@ describe("ProviderSettingsManager", () => {
id: "test-id",
},
},
migrations: {
rateLimitSecondsMigrated: false,
},
}

mockSecrets.get.mockResolvedValue(JSON.stringify(existingConfig))
Expand Down
10 changes: 10 additions & 0 deletions src/core/config/__tests__/importExport.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ describe("importExport", () => {
},
},
}

mockProviderSettingsManager.export.mockResolvedValue(previousProviderProfiles)

// Mock listConfig
Expand Down Expand Up @@ -294,6 +295,9 @@ describe("importExport", () => {
id: "test-id",
},
},
migrations: {
rateLimitSecondsMigrated: false,
},
}
mockProviderSettingsManager.export.mockResolvedValue(mockProviderProfiles)

Expand Down Expand Up @@ -345,6 +349,9 @@ describe("importExport", () => {
id: "test-id",
},
},
migrations: {
rateLimitSecondsMigrated: false,
},
})

// Mock global settings
Expand Down Expand Up @@ -384,6 +391,9 @@ describe("importExport", () => {
id: "test-id",
},
},
migrations: {
rateLimitSecondsMigrated: false,
},
})

// Mock global settings
Expand Down
1 change: 1 addition & 0 deletions src/exports/roo-code.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ type ProviderSettings = {
modelMaxTokens?: number | undefined
modelMaxThinkingTokens?: number | undefined
includeMaxTokens?: boolean | undefined
rateLimitSeconds?: number | undefined
fakeAi?: unknown | undefined
}

Expand Down
1 change: 1 addition & 0 deletions src/exports/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ type ProviderSettings = {
modelMaxTokens?: number | undefined
modelMaxThinkingTokens?: number | undefined
includeMaxTokens?: boolean | undefined
rateLimitSeconds?: number | undefined
fakeAi?: unknown | undefined
}

Expand Down
2 changes: 2 additions & 0 deletions src/schemas/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,7 @@ export const providerSettingsSchema = z.object({
modelMaxThinkingTokens: z.number().optional(),
// Generic
includeMaxTokens: z.boolean().optional(),
rateLimitSeconds: z.number().optional(),
// Fake AI
fakeAi: z.unknown().optional(),
})
Expand Down Expand Up @@ -470,6 +471,7 @@ const providerSettingsRecord: ProviderSettingsRecord = {
modelMaxThinkingTokens: undefined,
// Generic
includeMaxTokens: undefined,
rateLimitSeconds: undefined,
// Fake AI
fakeAi: undefined,
}
Expand Down
23 changes: 1 addition & 22 deletions webview-ui/src/components/settings/AdvancedSettings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,11 @@ import { SectionHeader } from "./SectionHeader"
import { Section } from "./Section"

type AdvancedSettingsProps = HTMLAttributes<HTMLDivElement> & {
rateLimitSeconds: number
diffEnabled?: boolean
fuzzyMatchThreshold?: number
setCachedStateField: SetCachedStateField<"rateLimitSeconds" | "diffEnabled" | "fuzzyMatchThreshold">
setCachedStateField: SetCachedStateField<"diffEnabled" | "fuzzyMatchThreshold">
}
export const AdvancedSettings = ({
rateLimitSeconds,
diffEnabled,
fuzzyMatchThreshold,
setCachedStateField,
Expand All @@ -36,25 +34,6 @@ export const AdvancedSettings = ({
</SectionHeader>

<Section>
<div>
<div className="flex flex-col gap-2">
<span className="font-medium">{t("settings:advanced.rateLimit.label")}</span>
<div className="flex items-center gap-2">
<Slider
min={0}
max={60}
step={1}
value={[rateLimitSeconds]}
onValueChange={([value]) => setCachedStateField("rateLimitSeconds", value)}
/>
<span className="w-10">{rateLimitSeconds}s</span>
</div>
</div>
<div className="text-vscode-descriptionForeground text-sm mt-1">
{t("settings:advanced.rateLimit.description")}
</div>
</div>

<div>
<VSCodeCheckbox
checked={diffEnabled}
Expand Down
17 changes: 12 additions & 5 deletions webview-ui/src/components/settings/ApiOptions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import { VSCodeButtonLink } from "../common/VSCodeButtonLink"
import { ModelInfoView } from "./ModelInfoView"
import { ModelPicker } from "./ModelPicker"
import { TemperatureControl } from "./TemperatureControl"
import { RateLimitSecondsControl } from "./RateLimitSecondsControl"
import { ApiErrorMessage } from "./ApiErrorMessage"
import { ThinkingBudget } from "./ThinkingBudget"
import { R1FormatSetting } from "./R1FormatSetting"
Expand Down Expand Up @@ -1623,11 +1624,17 @@ const ApiOptions = ({
)}

{!fromWelcomeView && (
<TemperatureControl
value={apiConfiguration?.modelTemperature}
onChange={handleInputChange("modelTemperature", noTransform)}
maxValue={2}
/>
<>
<TemperatureControl
value={apiConfiguration?.modelTemperature}
onChange={handleInputChange("modelTemperature", noTransform)}
maxValue={2}
/>
<RateLimitSecondsControl
value={apiConfiguration.rateLimitSeconds || 0}
onChange={(value) => setApiConfigurationField("rateLimitSeconds", value)}
/>
</>
)}
</div>
)
Expand Down
Loading
Loading