feat: add per-model budgets/rate-limits to virtual key provider configs - #5703
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughPer-model budgets and rate limits are added to virtual-key provider configurations. Backend reconciliation and hydration support model-specific governance. The virtual-key UI accepts, normalizes, submits, and displays these settings. ChangesVirtual-key model governance
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant VirtualKeySheet
participant GovernanceAPI
participant syncVKGovernanceToModelConfigs
participant ConfigStore
participant VirtualKeyDetailsSheet
VirtualKeySheet->>GovernanceAPI: Submit model_budgets
GovernanceAPI->>syncVKGovernanceToModelConfigs: Reconcile provider and model tiers
syncVKGovernanceToModelConfigs->>ConfigStore: Store model-config governance
VirtualKeyDetailsSheet->>GovernanceAPI: Request virtual-key details
GovernanceAPI->>ConfigStore: Load scoped model configs
ConfigStore-->>GovernanceAPI: Return model budgets and rate limits
GovernanceAPI-->>VirtualKeyDetailsSheet: Return hydrated model_budgets
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx (1)
1167-1197: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftPer-model budget edits bypass the budget-reset prompt and over-limit warning.
hasBudgetResetRelevantChangesandgetBudgetUsageWarning(Lines 691-760) only walkconfig.budgets. Now thatmodel_budgetsare editable here, changing a model's limit or reset frequency saves without the "Reset budget usage?" dialog, and existing usage that already exceeds the new per-model limit is never surfaced — inconsistent with provider- and VK-level behavior.Extending
budgetSignature/findBudgetUsageWarningover each config'smodel_budgets(scope label${providerLabel} · ${model_name}) would close the gap.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx` around lines 1167 - 1197, Extend hasBudgetResetRelevantChanges and getBudgetUsageWarning to include each provider config’s model_budgets alongside config.budgets. Reuse budgetSignature and findBudgetUsageWarning for per-model budget entries, labeling their scope as `${providerLabel} · ${model_name}`, so edits trigger the reset dialog and existing usage over the new limit produces a warning.
🧹 Nitpick comments (2)
transports/bifrost-http/handlers/governance_test.go (1)
116-131: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the new per-model governance behavior.
The mock/signature updates keep existing tests compiling, but the new logic is untested:
validateVKModelBudgetNames(limit, empty, wildcard, duplicate),buildVKModelBudgetsIndexgrouping/sorting, and — most importantly — the prune-vs-preserve branch insyncVKGovernanceToModelConfigs(model_budgetsomitted leaves existing per-model configs untouched; supplied set prunes absent models). That last branch is the one most likely to silently delete a customer's per-model budgets.As per coding guidelines, apply "table-driven coverage for behavior changes".
Want me to draft these table-driven cases?
Also applies to: 3183-3183, 3218-3218
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@transports/bifrost-http/handlers/governance_test.go` around lines 116 - 131, Add table-driven tests covering validateVKModelBudgetNames for limit, empty, wildcard, and duplicate inputs; buildVKModelBudgetsIndex grouping and sorting; and both syncVKGovernanceToModelConfigs branches, preserving existing per-model configs when model_budgets is omitted and pruning absent models when it is supplied. Use the existing mockRotateConfigStore and governance test helpers, and assert the resulting configurations explicitly.Source: Coding guidelines
ui/lib/types/governance.ts (1)
165-193: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider an update-specific model-budget request type.
The backend update payload uses
UpdateRateLimitRequestsemantics formodel_budgets[].rate_limit(nulls /{}to clear), whileVirtualKeyProviderConfigUpdateRequest.model_budgetsreusesVirtualKeyModelBudgetRequestwithCreateRateLimitRequest.virtualKeySheet.tsxalready emits the removal shape, so the type no longer describes the wire payload (it's only hidden because normalization returnsany[]).As per coding guidelines, "Avoid
any; prefer strict typing, inference, and reusable shared types."♻️ Suggested split
+export interface VirtualKeyModelBudgetUpdateRequest { + model_name: string; + budgets?: CreateBudgetRequest[]; + rate_limit?: UpdateRateLimitRequest; +}export interface VirtualKeyProviderConfigUpdateRequest { ... - model_budgets?: VirtualKeyModelBudgetRequest[]; // Full desired per-model set when provider_configs is supplied + model_budgets?: VirtualKeyModelBudgetUpdateRequest[]; // Full desired per-model set when provider_configs is supplied }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/lib/types/governance.ts` around lines 165 - 193, Introduce an update-specific model-budget request type alongside VirtualKeyModelBudgetRequest, using UpdateRateLimitRequest for rate_limit while preserving the shared model_name and budgets fields. Change VirtualKeyProviderConfigUpdateRequest.model_budgets to use the new update type, keeping the create request on VirtualKeyModelBudgetRequest and ensuring the type represents null or empty-object removal payloads emitted by virtualKeySheet.tsx.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsx`:
- Around line 418-490: Add BudgetOverrideDialog to each model budget row in the
config.model_budgets.map rendering, using the budget’s identifier and
virtual-key/model scope required by the override endpoint. Reuse the existing
provider or virtual-key budget dialog integration and effective-limit/override
data so model budgets support viewing and editing overrides consistently.
In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx`:
- Around line 100-124: Update the model_name field in the model_budgets schema
to reject empty or whitespace-only values and provide a meaningful inline
validation message, while preserving valid model names and the existing
normalization flow in normalizeProviderConfigs.
---
Outside diff comments:
In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx`:
- Around line 1167-1197: Extend hasBudgetResetRelevantChanges and
getBudgetUsageWarning to include each provider config’s model_budgets alongside
config.budgets. Reuse budgetSignature and findBudgetUsageWarning for per-model
budget entries, labeling their scope as `${providerLabel} · ${model_name}`, so
edits trigger the reset dialog and existing usage over the new limit produces a
warning.
---
Nitpick comments:
In `@transports/bifrost-http/handlers/governance_test.go`:
- Around line 116-131: Add table-driven tests covering
validateVKModelBudgetNames for limit, empty, wildcard, and duplicate inputs;
buildVKModelBudgetsIndex grouping and sorting; and both
syncVKGovernanceToModelConfigs branches, preserving existing per-model configs
when model_budgets is omitted and pruning absent models when it is supplied. Use
the existing mockRotateConfigStore and governance test helpers, and assert the
resulting configurations explicitly.
In `@ui/lib/types/governance.ts`:
- Around line 165-193: Introduce an update-specific model-budget request type
alongside VirtualKeyModelBudgetRequest, using UpdateRateLimitRequest for
rate_limit while preserving the shared model_name and budgets fields. Change
VirtualKeyProviderConfigUpdateRequest.model_budgets to use the new update type,
keeping the create request on VirtualKeyModelBudgetRequest and ensuring the type
represents null or empty-object removal payloads emitted by virtualKeySheet.tsx.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3b394f8d-d7e8-45e7-924c-2b52a26e72cf
📒 Files selected for processing (6)
framework/configstore/tables/virtualkey.gotransports/bifrost-http/handlers/governance.gotransports/bifrost-http/handlers/governance_test.goui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsxui/app/workspace/virtual-keys/views/virtualKeySheet.tsxui/lib/types/governance.ts
bb71488 to
4eb1124
Compare
4eb1124 to
aa5e5ee
Compare
The merge-base changed after approval.
aa5e5ee to
2ddf2ba
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
2ddf2ba to
e1f1b8b
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
e1f1b8b to
3ef24fc
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
The merge-base changed after approval.
3ef24fc to
40c75ff
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
40c75ff to
b2209df
Compare
c01a0a2 to
41c3a10
Compare
The base branch was changed.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@transports/bifrost-http/handlers/governance.go`:
- Line 880: In reconcileVKModelConfig, declare modelName by calling
d.modelNameOrAll() before the query that uses it, ensuring the existing
references compile correctly.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f1100786-815c-4f8a-aae9-80e4f032a3a9
📒 Files selected for processing (2)
transports/bifrost-http/handlers/governance.goui/app/workspace/virtual-keys/views/virtualKeySheet.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx
b2209df to
ad9a9ea
Compare
Merge activity
|
… configs (maximhq#5703) ## Summary Adds support for per-model budgets and rate limits scoped to a specific provider config on a Virtual Key. Previously, budgets and rate limits could only be set at the VK top-level or per-provider tier (the `"*"` all-models wildcard). This change allows operators to define finer-grained spending and request caps for individual models within a provider config. ## Changes - Introduced `VKProviderModelBudget` as a serialization-only struct on `TableVirtualKeyProviderConfig`, hydrated from VK-scoped model configs at read time rather than stored directly in the provider config table. - Added `model_budgets` to `CreateVirtualKeyRequest` and `UpdateVirtualKeyRequest` provider config entries, with dedicated `vkModelBudgetRequest` and `vkModelBudgetUpdateRequest` types. - Extended `vkModelConfigDesired` with a `modelName` field and a `reconcileModelBudgets` flag. When `model_budgets` is supplied for a provider, the backend treats the list as the full desired set and prunes any per-model configs absent from it; when omitted, existing per-model configs are left untouched. - Refactored `syncVKGovernanceToModelConfigs` to key the `keep` map by `provider + "\x00" + modelTier`, enabling both the `"*"` provider tier and concrete model tiers to coexist and be independently reconciled. - Added `buildVKCreateModelBudgets` and `buildVKUpdateModelBudgets` helpers with validation (`validateVKModelBudgetNames`) enforcing uniqueness, non-empty names, non-wildcard model names, and a cap of 100 per-model groups per provider. - Added `buildVKModelBudgetsIndex` to group per-model VK-scoped model configs by provider for bulk hydration, and updated `applyVKGovernanceFromModelConfigs`, `hydrateVKGovernance`, `hydrateVKListGovernance`, `getVirtualKeys`, `getVirtualKey`, and `collectVKModelUsage` to pass and consume this index. - Replaced the per-provider N+1 `GetModelConfig` calls in `hydrateVKGovernance` with a single `GetModelConfigsByScopeAndScopeIDs` bulk load. - Updated the UI type definitions (`VirtualKeyModelBudget`, `VirtualKeyModelBudgetRequest`) and provider config request interfaces to include `model_budgets`. - Extended the VK create/edit sheet (`virtualKeySheet.tsx`) to pass `model_budgets` through form state, normalization, and the `ProviderConfigEditor` component (via a new `showModelBudgets` prop). - Updated the VK details sheet (`virtualKeyDetailsSheet.tsx`) to render per-model budgets, token limits, and request limits for each provider config. - Updated tests and the mock config store to implement `GetModelConfigsByScopeAndScopeIDs` and pass the new `perModelByKey` argument to `applyVKGovernanceFromModelConfigs`. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go test ./... # UI cd ui pnpm i pnpm build ``` 1. Create a Virtual Key with a provider config that includes `model_budgets`, e.g.: ```json { "provider_configs": [{ "provider": "openai", "model_budgets": [ { "model_name": "gpt-4o", "budgets": [{ "max_limit": 10.0, "reset_duration": "30d" }] }, { "model_name": "gpt-4o-mini", "rate_limit": { "request_max_limit": 100, "request_reset_duration": "1h" } } ] }] } ``` 2. Fetch the VK and confirm `model_budgets` is populated on the provider config in the response. 3. Update the VK omitting `model_budgets` for a provider and confirm existing per-model configs are preserved. 4. Update the VK supplying an empty `model_budgets: []` for a provider and confirm existing per-model configs are pruned. 5. Verify the VK details sheet renders per-model budget and rate-limit rows under the provider config section. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations Per-model budget validation rejects the `"*"` wildcard as a model name to prevent accidental shadowing of the provider-level tier. Model name uniqueness is enforced per provider config to prevent ambiguous budget application. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
… configs (maximhq#5703) ## Summary Adds support for per-model budgets and rate limits scoped to a specific provider config on a Virtual Key. Previously, budgets and rate limits could only be set at the VK top-level or per-provider tier (the `"*"` all-models wildcard). This change allows operators to define finer-grained spending and request caps for individual models within a provider config. ## Changes - Introduced `VKProviderModelBudget` as a serialization-only struct on `TableVirtualKeyProviderConfig`, hydrated from VK-scoped model configs at read time rather than stored directly in the provider config table. - Added `model_budgets` to `CreateVirtualKeyRequest` and `UpdateVirtualKeyRequest` provider config entries, with dedicated `vkModelBudgetRequest` and `vkModelBudgetUpdateRequest` types. - Extended `vkModelConfigDesired` with a `modelName` field and a `reconcileModelBudgets` flag. When `model_budgets` is supplied for a provider, the backend treats the list as the full desired set and prunes any per-model configs absent from it; when omitted, existing per-model configs are left untouched. - Refactored `syncVKGovernanceToModelConfigs` to key the `keep` map by `provider + "\x00" + modelTier`, enabling both the `"*"` provider tier and concrete model tiers to coexist and be independently reconciled. - Added `buildVKCreateModelBudgets` and `buildVKUpdateModelBudgets` helpers with validation (`validateVKModelBudgetNames`) enforcing uniqueness, non-empty names, non-wildcard model names, and a cap of 100 per-model groups per provider. - Added `buildVKModelBudgetsIndex` to group per-model VK-scoped model configs by provider for bulk hydration, and updated `applyVKGovernanceFromModelConfigs`, `hydrateVKGovernance`, `hydrateVKListGovernance`, `getVirtualKeys`, `getVirtualKey`, and `collectVKModelUsage` to pass and consume this index. - Replaced the per-provider N+1 `GetModelConfig` calls in `hydrateVKGovernance` with a single `GetModelConfigsByScopeAndScopeIDs` bulk load. - Updated the UI type definitions (`VirtualKeyModelBudget`, `VirtualKeyModelBudgetRequest`) and provider config request interfaces to include `model_budgets`. - Extended the VK create/edit sheet (`virtualKeySheet.tsx`) to pass `model_budgets` through form state, normalization, and the `ProviderConfigEditor` component (via a new `showModelBudgets` prop). - Updated the VK details sheet (`virtualKeyDetailsSheet.tsx`) to render per-model budgets, token limits, and request limits for each provider config. - Updated tests and the mock config store to implement `GetModelConfigsByScopeAndScopeIDs` and pass the new `perModelByKey` argument to `applyVKGovernanceFromModelConfigs`. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go test ./... # UI cd ui pnpm i pnpm build ``` 1. Create a Virtual Key with a provider config that includes `model_budgets`, e.g.: ```json { "provider_configs": [{ "provider": "openai", "model_budgets": [ { "model_name": "gpt-4o", "budgets": [{ "max_limit": 10.0, "reset_duration": "30d" }] }, { "model_name": "gpt-4o-mini", "rate_limit": { "request_max_limit": 100, "request_reset_duration": "1h" } } ] }] } ``` 2. Fetch the VK and confirm `model_budgets` is populated on the provider config in the response. 3. Update the VK omitting `model_budgets` for a provider and confirm existing per-model configs are preserved. 4. Update the VK supplying an empty `model_budgets: []` for a provider and confirm existing per-model configs are pruned. 5. Verify the VK details sheet renders per-model budget and rate-limit rows under the provider config section. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations Per-model budget validation rejects the `"*"` wildcard as a model name to prevent accidental shadowing of the provider-level tier. Model name uniqueness is enforced per provider config to prevent ambiguous budget application. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable

Summary
Adds support for per-model budgets and rate limits scoped to a specific provider config on a Virtual Key. Previously, budgets and rate limits could only be set at the VK top-level or per-provider tier (the
"*"all-models wildcard). This change allows operators to define finer-grained spending and request caps for individual models within a provider config.Changes
VKProviderModelBudgetas a serialization-only struct onTableVirtualKeyProviderConfig, hydrated from VK-scoped model configs at read time rather than stored directly in the provider config table.model_budgetstoCreateVirtualKeyRequestandUpdateVirtualKeyRequestprovider config entries, with dedicatedvkModelBudgetRequestandvkModelBudgetUpdateRequesttypes.vkModelConfigDesiredwith amodelNamefield and areconcileModelBudgetsflag. Whenmodel_budgetsis supplied for a provider, the backend treats the list as the full desired set and prunes any per-model configs absent from it; when omitted, existing per-model configs are left untouched.syncVKGovernanceToModelConfigsto key thekeepmap byprovider + "\x00" + modelTier, enabling both the"*"provider tier and concrete model tiers to coexist and be independently reconciled.buildVKCreateModelBudgetsandbuildVKUpdateModelBudgetshelpers with validation (validateVKModelBudgetNames) enforcing uniqueness, non-empty names, non-wildcard model names, and a cap of 100 per-model groups per provider.buildVKModelBudgetsIndexto group per-model VK-scoped model configs by provider for bulk hydration, and updatedapplyVKGovernanceFromModelConfigs,hydrateVKGovernance,hydrateVKListGovernance,getVirtualKeys,getVirtualKey, andcollectVKModelUsageto pass and consume this index.GetModelConfigcalls inhydrateVKGovernancewith a singleGetModelConfigsByScopeAndScopeIDsbulk load.VirtualKeyModelBudget,VirtualKeyModelBudgetRequest) and provider config request interfaces to includemodel_budgets.virtualKeySheet.tsx) to passmodel_budgetsthrough form state, normalization, and theProviderConfigEditorcomponent (via a newshowModelBudgetsprop).virtualKeyDetailsSheet.tsx) to render per-model budgets, token limits, and request limits for each provider config.GetModelConfigsByScopeAndScopeIDsand pass the newperModelByKeyargument toapplyVKGovernanceFromModelConfigs.Type of change
Affected areas
How to test
model_budgets, e.g.:{ "provider_configs": [{ "provider": "openai", "model_budgets": [ { "model_name": "gpt-4o", "budgets": [{ "max_limit": 10.0, "reset_duration": "30d" }] }, { "model_name": "gpt-4o-mini", "rate_limit": { "request_max_limit": 100, "request_reset_duration": "1h" } } ] }] }model_budgetsis populated on the provider config in the response.model_budgetsfor a provider and confirm existing per-model configs are preserved.model_budgets: []for a provider and confirm existing per-model configs are pruned.Breaking changes
Related issues
Security considerations
Per-model budget validation rejects the
"*"wildcard as a model name to prevent accidental shadowing of the provider-level tier. Model name uniqueness is enforced per provider config to prevent ambiguous budget application.Checklist
docs/contributing/README.mdand followed the guidelines