Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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: {
rateLimitSecondsMigrate: 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