Skip to content

Conversation

@olweraltuve
Copy link
Contributor

@olweraltuve olweraltuve commented Mar 17, 2025

This commit introduces a new method to migrate the global rate limit setting to individual Profiles configurations. The migrateRateLimitToProfiles method updates all existing configurations with the global rate limit value and removes the global setting. Additionally, the rate limit is now part of the Profile configuration, allowing for more granular control.

  • Added migration logic in ConfigManager.
  • Updated ClineProvider to handle rate limit updates.
  • Adjusted related components to accommodate the new rate limit structure.

Tests have been added to ensure the migration works correctly and handles edge cases.


Important

Migrates global rate limit to profile-specific settings, updating configuration management and UI components accordingly.

  • Behavior:
    • Migrates global rate limit to profile-specific settings using migrateRateLimitToProfiles() in ConfigManager.
    • Removes global rate limit setting from globalState.
    • Updates ClineProvider to handle rate limit per profile.
  • UI:
    • Updates AdvancedSettings.tsx to use rateLimitSeconds from profile settings.
    • Adjusts SettingsView.tsx to handle rateLimitSeconds changes.
  • Tests:
    • Adds tests for migrateRateLimitToProfiles() in ConfigManager.test.ts to ensure correct migration and error handling.

This description was created by Ellipsis for b4a1506. It will automatically update as commits are pushed.

This commit introduces a new method to migrate the global rate limit setting to individual API configurations. The `migrateRateLimitToProfiles` method updates all existing configurations with the global rate limit value and removes the global setting. Additionally, the rate limit is now part of the API configuration, allowing for more granular control.

- Added migration logic in `ConfigManager`.
- Updated `ClineProvider` to handle rate limit updates.
- Adjusted related components to accommodate the new rate limit structure.

Tests have been added to ensure the migration works correctly and handles edge cases.
@changeset-bot
Copy link

changeset-bot bot commented Mar 17, 2025

⚠️ No Changeset found

Latest commit: 700db7b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels Mar 17, 2025
alwaysApproveResubmit,
requestDelaySeconds,
rateLimitSeconds,
// rateLimitSeconds is now part of apiConfiguration
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// rateLimitSeconds is now part of apiConfiguration

alwaysApproveResubmit: alwaysApproveResubmit ?? false,
requestDelaySeconds: requestDelaySeconds ?? 10,
rateLimitSeconds: rateLimitSeconds ?? 0,
rateLimitSeconds: apiConfiguration.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.

Do we still need this?

alwaysApproveResubmit: stateValues.alwaysApproveResubmit ?? false,
requestDelaySeconds: Math.max(5, stateValues.requestDelaySeconds ?? 10),
rateLimitSeconds: stateValues.rateLimitSeconds ?? 0,
// rateLimitSeconds is now part of the API configuration
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// rateLimitSeconds is now part of the API configuration

export type ApiConfiguration = ApiHandlerOptions & {
apiProvider?: ApiProvider
id?: string // stable unique identifier
rateLimitSeconds?: number // Rate limit in seconds between API requests
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we need to add this here


type AdvancedSettingsProps = HTMLAttributes<HTMLDivElement> & {
rateLimitSeconds: number
rateLimitSeconds?: number
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should move this out of the advanced settings and move it up near the model temperature so it's clear that it's linked to the profile

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I'm going to place it below the temperature

Copy link
Collaborator

Choose a reason for hiding this comment

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

Great, thanks!

@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Mar 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants