feat: adding scope to budget limits table - #3937
Merged
Merged
Conversation
Contributor
|
Caution Review failedFailed to post review comments 📝 WalkthroughWalkthroughAdds per-virtual-key scoping for model configurations: DB schema and migration, scope-aware config queries and in-memory caching, governance checks and usage updates for scoped budgets and rate-limits, HTTP handler validation/enrichment (including from-memory listing), tests, and UI scope selection/display. ChangesVirtual-Key-Scoped Model Governance
Sequence Diagram(s): sequenceDiagram
participant Client
participant HTTPHandler
participant GovernanceManager
participant GovernanceStore
participant RDB
Client->>HTTPHandler: create/get model config (scope, scope_id, model_name, provider)
HTTPHandler->>GovernanceManager: Read/Reload governance data / from_memory request
GovernanceManager->>GovernanceStore: Query scoped model config (scope, scope_id, ...)
GovernanceStore->>RDB: GetModelConfig / migration-backed schema operations
GovernanceStore->>GovernanceStore: rebuildInMemoryStructures (scope-aware keys)
GovernanceStore->>GovernanceStore: CheckVirtualKeyScopedModelRateLimit/Budget
GovernanceStore->>GovernanceStore: UpdateVirtualKeyScopedModel*UsageInMemory
HTTPHandler-->>Client: Response with enriched scope_name
🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
|
|
This was referenced May 31, 2026
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
18 tasks
roroghost17
marked this pull request as ready for review
June 1, 2026 10:07
Contributor
Confidence Score: 4/5Core governance and in-memory scoping logic looks sound; the main concern is the incomplete owned-row cleanup in the DB-side DeleteVirtualKey path and a one-time hash-drift re-sync on first restart. The scoped check and update wiring in the resolver and tracker is well-structured and consistently follows the global model-config patterns. Two issues temper confidence: (1) budgetIDs/rateLimitIDs collected inside DeleteVirtualKey have no corresponding batch-delete statements, so owned budget and rate-limit rows for scoped model configs are orphaned on VK deletion; and (2) GenerateModelConfigHash now includes scope/scope_id, meaning all existing global model configs will hash-mismatch their stored config_hash on the first post-migration restart and trigger a spurious re-sync wave. framework/configstore/rdb.go — DeleteVirtualKey needs batch-delete statements for the collected budgetIDs and rateLimitIDs after the TableModelConfig bulk delete. Important Files Changed
Reviews (3): Last reviewed commit: "feat: adding scope to budget limits tabl..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 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 `@framework/configstore/migrations_test.go`:
- Around line 2409-2412: Add more cases in migrations_test.go around the
governance_model_configs inserts: insert multiple rows (e.g., "mc-existing-1",
"mc-existing-2") to exercise batch backfill, insert rows with both model_name
and provider populated (use the same now and db variables) to ensure provider is
preserved, and add assertions that verify the composite unique constraint
behavior by attempting to insert a duplicate model_name+provider+scope and
expecting an error while inserting the same model_name+provider under a
different scope succeeds; use the same db.Exec pattern and
require.NoError/require.Error checks to validate outcomes against the
migration/backfill logic.
- Around line 2430-2432: The test verifies the backfilled scope but misses
asserting scope_id is NULL for global rows; update the test after querying the
row with id "mc-existing" (using
db.Table("governance_model_configs").Select(...).Where(...)) to also read the
scope_id (e.g., into a sql.NullString or pointer) and assert that scope_id is
NULL/invalid when scope == tables.ModelConfigScopeGlobal so the composite-unique
invariant is enforced.
In `@framework/configstore/migrations.go`:
- Around line 3863-3883: The composite index creation loses global uniqueness
because scope_id remains NULL for backfilled global rows; before creating
idx_model_scope_provider (in the migration around modelConfig,
migrator.CreateIndex), normalize scope_id for global rows (e.g., UPDATE
governance_model_configs SET scope_id = <non-NULL-sentinel> WHERE scope =
tables.ModelConfigScopeGlobal AND scope_id IS NULL) or instead create the unique
index as an expression/partial index that coalesces scope_id (use
COALESCE(scope_id, <sentinel>) or a WHERE clause) so that global rows are
treated consistently; then proceed to CreateIndex and only afterwards
DropIndex("idx_model_provider"). Ensure the sentinel choice matches application
expectations and struct tags if using CreateIndex metadata.
- Around line 3873-3881: The migration currently calls migrator.CreateIndex and
migrator.DropIndex inside the transaction for modelConfig
(idx_model_scope_provider / idx_model_provider), which will block large Postgres
tables; change the migration to build and drop the replacement index
non-transactionally using Postgres CONCURRENTLY semantics: detect Postgres, end
the transaction (or run a non-transactional step) and execute "CREATE UNIQUE
INDEX CONCURRENTLY ... ON governance_model_configs (...)" to create
idx_model_scope_provider if missing, then execute "DROP INDEX CONCURRENTLY
idx_model_provider" if present, ensuring you still check existence
(migrator.HasIndex or an equivalent query) and surface errors; reference the
same modelConfig, idx_model_scope_provider and idx_model_provider symbols so
callers can locate and replace migrator.CreateIndex/migrator.DropIndex with the
non-transactional concurrent SQL path.
- Around line 3886-3905: The rollback in the Rollback function for
TableModelConfig is lossy (it drops scope/scope_id and idx_model_scope_provider
but never recreates the original idx_model_provider or restore scoped rows), so
update the migration to explicitly mark it non-rollbackable: replace or change
the current Rollback implementation to return a clear non-rollbackable error (or
set Rollback to nil with a comment) indicating that downgrades are unsupported
for this migration and referencing the affected symbols (Rollback function,
tables.TableModelConfig, dropped columns "scope" and "scope_id", and indexes
"idx_model_scope_provider" vs original "idx_model_provider"). Ensure the error
message is descriptive so callers know the migration cannot be safely reverted.
In `@framework/configstore/rdb_test.go`:
- Line 656: Remove manual timestamp initialization from the test objects (e.g.,
the TableBudget instantiation named budget and any other test instances) by
deleting explicit time.Now() assignments for LastReset, CreatedAt, and
UpdatedAt; rely on GORM's autoCreateTime/autoUpdateTime tags to populate those
fields on insert/update. Locate the test entries that set LastReset, CreatedAt
or UpdatedAt (e.g., the &tables.TableBudget{... LastReset: time.Now()} construct
and similar initializations at the other noted locations) and remove those field
assignments so the structs omit those timestamp fields when created.
In `@framework/configstore/rdb.go`:
- Around line 2927-2941: The current loop deletes owned
TableBudget/TableRateLimit rows (scopedModelConfigs loop) before deleting their
parent TableModelConfig, which can violate FK constraints; change the order to
first delete the TableModelConfig rows (the txDB.WithContext(...).Where("scope =
? AND scope_id = ?", tables.ModelConfigScopeVirtualKey,
id).Delete(&tables.TableModelConfig{}) call) and only after that iterate
scopedModelConfigs to delete TableBudget and TableRateLimit rows (using
txDB.WithContext(...).Delete(&tables.TableBudget{}, "id = ?", *mc.BudgetID) and
Delete(&tables.TableRateLimit{}, "id = ?", *mc.RateLimitID)), mirroring the
DeleteModelConfig behavior so parent deletions cascade to owned rows without FK
errors.
In `@plugins/governance/store.go`:
- Around line 1012-1044: The code uses configstoreTables.ModelConfigScopeGlobal
and configstoreTables.ModelConfigScopeVirtualKey but those scope constants are
defined in the package that actually exports them (not configstoreTables);
update references in modelConfigStoreKey, nonGlobalModelConfigScopeChain, and
findModelOnlyConfig to use the correct package-qualified constants (e.g.,
replace configstoreTables.ModelConfigScopeGlobal and
configstoreTables.ModelConfigScopeVirtualKey with the package that declares
them) and adjust imports accordingly so the file imports the package that
exports the scope constants.
- Around line 1314-1357: The current logic can produce nondeterministic results
because CheckRateLimit may return on the first violated map entry (map iteration
order varies); update the CheckRateLimit implementation so it does not exit
early: iterate all entries (e.g., over the entityWiseRateLimits passed from
CheckVirtualKeyScopedModelRateLimit), accumulate all token/request violations
across entries, and only compute the final Decision after processing every entry
(narrow to token_limited or request_limited only when exactly one violation type
present, otherwise return rate_limited); apply the same non-early-exit
aggregation approach for related helpers like CheckProviderRateLimit and
CheckVirtualKeyRateLimit so decisions no longer depend on map iteration order.
- Around line 1270-1357: CollectApplicableGovernanceIDs is missing VK-scoped
model budget/rate-limit IDs, so reconciliation can miss the per-VK model
entities; update CollectApplicableGovernanceIDs to traverse
nonGlobalModelConfigScopeChain(vk) (same as in CheckVirtualKeyScopedModelBudget
and CheckVirtualKeyScopedModelRateLimit), and for each scope add the same
model+provider key (using modelConfigStoreKey(scope.name, scope.id, model,
providerStr)) and model-only config key (using the configKey returned by
findScopedModelOnlyConfig) into the collector, and when a scoped
TableModelConfig exists include its BudgetID and RateLimitID (if non-nil) into
the returned applicable IDs so that VK-scoped budget/rate-limit entities are
reported along with global model/provider and VK-hierarchy IDs.
In `@plugins/governance/tracker.go`:
- Around line 127-138: The scoped-model update block only runs when update.Model
!= "" but leaves out the global model updates when update.Provider == "",
causing global counters to be undercounted; modify the logic so that when
update.Model != "" you also invoke the same global model update functions used
above (the ones currently gated by update.Provider != "") even if
update.Provider == "" — i.e., call the global model rate-limit and budget update
methods the same way you call UpdateVirtualKeyScopedModelRateLimitUsageInMemory
and UpdateVirtualKeyScopedModelBudgetUsageInMemory (preserving
shouldUpdateTokens/shouldUpdateRequests and shouldUpdateBudget) so writes
increment both scoped and global counters for provider-less model requests.
In `@transports/bifrost-http/handlers/governance.go`:
- Around line 2667-2668: The comparison vkErr == configstore.ErrNotFound can
miss wrapped errors; update the check around the call to
h.configStore.GetVirtualKey(ctx, *req.ScopeID) to use errors.Is(vkErr,
configstore.ErrNotFound) instead of direct equality (vkErr ==
configstore.ErrNotFound) so wrapped ErrNotFound values are detected and the
handler returns the intended 400 path; adjust the vkErr branch accordingly
wherever ErrNotFound is checked for GetVirtualKey in this handler.
In `@ui/app/workspace/model-limits/views/modelLimitSheet.tsx`:
- Line 72: The virtual keys query useGetVirtualKeysQuery runs unconditionally;
change it to skip fetching unless the form's scope is "virtual_key" by passing
the query options with skip: form.watch("scope") !== "virtual_key" (use the
existing form.watch("scope") call and the useGetVirtualKeysQuery call to locate
places to update). Ensure default data shape remains ({ virtual_keys: [] }) when
skipped so UI logic doesn't break.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 718f8566-5a8a-4989-92c7-7a9df6322a23
📒 Files selected for processing (23)
.gitignoreframework/configstore/clientconfig.goframework/configstore/migrations.goframework/configstore/migrations_test.goframework/configstore/rdb.goframework/configstore/rdb_test.goframework/configstore/store.goframework/configstore/tables/modelconfig.goplugins/governance/modelprovidergovernance_test.goplugins/governance/resolver.goplugins/governance/store.goplugins/governance/test_utils.goplugins/governance/tracker.gotransports/bifrost-http/handlers/governance.gotransports/bifrost-http/lib/config_test.goui/app/workspace/model-limits/views/modelLimitSheet.tsxui/app/workspace/model-limits/views/modelLimitsTable.tsxui/app/workspace/routing-rules/views/routingRuleInfoSheet.tsxui/app/workspace/routing-rules/views/routingRulesTable.tsxui/lib/constants/governance.tsui/lib/types/governance.tsui/lib/utils/labels.tsui/lib/utils/routingRules.ts
💤 Files with no reviewable changes (1)
- ui/lib/utils/routingRules.ts
roroghost17
force-pushed
the
05-26-feat_adding_scope_to_budget_limits_table
branch
from
June 1, 2026 13:30
eb27aa8 to
0b1b2f8
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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 `@framework/configstore/rdb_test.go`:
- Line 7: Remove the unused time import from the test file: edit
framework/configstore/rdb_test.go and delete "time" from the import list (where
imports are declared) so the package builds; locate the import block near the
top of the file (the one showing "time") and remove that entry or the whole
import if it becomes empty.
In `@framework/configstore/rdb.go`:
- Around line 2929-2943: The code collects budgetIDs and rateLimitIDs from
scopedModelConfigs but never removes those owned rows, leaking budgets and
rate-limits; after the Delete on tables.TableModelConfig (using
txDB.WithContext(ctx)), add conditional deletes for the collected IDs: if
len(budgetIDs) > 0 call txDB.WithContext(ctx).Where("id IN (?)",
budgetIDs).Delete(&tables.TableBudget{}) and similarly for rateLimitIDs call
txDB.WithContext(ctx).Where("id IN (?)",
rateLimitIDs).Delete(&tables.TableRateLimit{}), ensuring you run these within
the same transaction/txDB and check for and return any errors from those Delete
calls.
In `@framework/configstore/tables/modelconfig.go`:
- Around line 31-37: The composite unique index idx_model_scope_provider is
being bypassed because BeforeSave sets ScopeID = nil for global scope (and
Provider can be nil), so NULLs allow duplicates; update BeforeSave to set
ScopeID to a non-NULL sentinel (e.g., pointer to a constant like "__global__")
for Scope == "global" and ensure Provider is a non-nil pointer (e.g., pointer to
"" or a sentinel) so the indexed columns are never NULL, or alternatively create
a DB-level partial/COALESCE unique constraint; modify the BeforeSave method and
related handling of ScopeID/Provider (symbols: BeforeSave, ScopeID, Scope,
Provider, idx_model_scope_provider, ModelName) to set and persist the chosen
non-NULL sentinels to enforce uniqueness.
In `@plugins/governance/modelprovidergovernance_test.go`:
- Around line 2129-2199: Add tests that mirror the existing VK-scoped model
budget tests but use provider-scoped model configs: create VK-scoped configs via
buildVKScopedModelConfig with Model set to "gpt-4" and Provider set to
schemas.OpenAI (or combined identifier if your config API expects
"gpt-4:openai"), use buildBudget and buildBudgetWithUsage for within-limit and
exceeded scenarios, and call CheckVirtualKeyScopedModelBudget with
EvaluationRequest specifying different Provider values to assert matching only
when providers match; also add a test where both a model-only VK-scoped config
and a model+provider VK-scoped config coexist to verify the provider-specific
config applies only to its provider while the model-only config still applies to
others (use DecisionAllow/expected errors to assert outcomes).
- Around line 2201-2231: Add a new unit test mirroring
TestStore_CheckVirtualKeyScopedModelRateLimit_TokenLimitExceeded but for request
count: create TestStore_CheckVirtualKeyScopedModelRateLimit_RequestLimitExceeded
that builds a virtual key (buildVirtualKey), a rate limit via
buildRateLimitWithUsage with requests at or above the limit (requests usage
equals max) and tokens well within limit, a VK-scoped model config
(buildVKScopedModelConfig) and a LocalGovernanceStore via
NewLocalGovernanceStore; call store.CheckVirtualKeyScopedModelRateLimit with an
EvaluationRequest for "gpt-4" and assert it returns an error and the
DecisionRequestLimited decision (parallel to DecisionTokenLimited in the token
test).
In `@plugins/governance/store.go`:
- Around line 3137-3153: When updating gs.modelConfigs with the new derived key
(using modelConfigStoreKey and the local clone), remove any previous cache entry
that belonged to the same model config ID but uses a different derived key so
stale entries don't remain; implement this by scanning gs.modelConfigs (or using
its iteration API) before calling Store to find entries whose stored *mc.ID*
matches clone.ID and whose key != newKey and delete them (mirroring
DeleteModelConfigInMemory's matching logic), then proceed to
gs.modelConfigs.Store(key, &clone).
In `@plugins/governance/test_utils.go`:
- Line 270: Replace the direct address operator usage with the repository
pointer helper: change the assignment to mc.ScopeID = bifrost.Ptr(vkID)
(referencing mc.ScopeID and vkID) and ensure the bifrost package is imported in
the test file so the helper is available.
In `@transports/bifrost-http/handlers/governance.go`:
- Around line 2496-2520: The from_memory branch currently returns raw
modelConfigs without applying the same search, limit and offset logic as the
normal listing path; update the from_memory=true branch (where you call
h.governanceManager.GetGovernanceData, build modelConfigs, and call
h.enrichModelConfigScopeNames) to parse query params (search, limit, offset)
from ctx.QueryArgs(), apply the same in-memory filtering by search and then
paginate the filtered slice before calling SendJSON, and set the response
envelope fields ("model_configs", "count", "total_count", "limit", "offset") to
reflect the filtered total_count and the paginated count; preserve the existing
behavior of copying pointers into a value slice
(configstoreTables.TableModelConfig) and keep calling
h.enrichModelConfigScopeNames and SendError/SendJSON as before.
- Around line 395-396: The JSON schema needs to include the new scoped fields so
runtime and validation match: update transports/config.schema.json to add
"scope" (string, default "global", description matching Scope in governance.go)
and "scope_id" (string or nullable, description matching ScopeID in
governance.go) to the properties of governance.model_configs[] items; ensure the
model_configs item schema exposes these fields (and, if desired, add a
conditional/annotation noting scope_id is required when scope != "global") so
schema-validated payloads accept scoped model config objects.
- Around line 2503-2512: The code currently copies only top-level values of
configstoreTables.TableModelConfig which leaves nested pointer fields (e.g.,
Budget, RateLimit and any pointer/slice/map fields inside TableModelConfig)
aliasing live governance state; change this to produce fully detached objects
before calling h.enrichModelConfigScopeNames and serializing: implement and use
a cloneModelConfig (or map-to-response-DTO) that deep-copies all nested
pointers, slices and maps from data.ModelConfigs (skipping nil entries),
populate modelConfigs with these cloned/detached instances, and pass those
clones to h.enrichModelConfigScopeNames so the endpoint marshals race-safe,
non-shared data.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 2aeaf4b9-7493-4d28-aa87-bda46fb64c58
📒 Files selected for processing (23)
.gitignoreframework/configstore/clientconfig.goframework/configstore/migrations.goframework/configstore/migrations_test.goframework/configstore/rdb.goframework/configstore/rdb_test.goframework/configstore/store.goframework/configstore/tables/modelconfig.goplugins/governance/modelprovidergovernance_test.goplugins/governance/resolver.goplugins/governance/store.goplugins/governance/test_utils.goplugins/governance/tracker.gotransports/bifrost-http/handlers/governance.gotransports/bifrost-http/lib/config_test.goui/app/workspace/model-limits/views/modelLimitSheet.tsxui/app/workspace/model-limits/views/modelLimitsTable.tsxui/app/workspace/routing-rules/views/routingRuleInfoSheet.tsxui/app/workspace/routing-rules/views/routingRulesTable.tsxui/lib/constants/governance.tsui/lib/types/governance.tsui/lib/utils/labels.tsui/lib/utils/routingRules.ts
💤 Files with no reviewable changes (1)
- ui/lib/utils/routingRules.ts
18 tasks
18 tasks
roroghost17
force-pushed
the
05-26-feat_adding_scope_to_budget_limits_table
branch
from
June 1, 2026 17:56
0b1b2f8 to
d7b2251
Compare
This was referenced Jun 2, 2026
Contributor
Merge activity
|
akshaydeo
pushed a commit
that referenced
this pull request
Jun 2, 2026
## Summary Model configs previously applied globally to all traffic. This PR introduces a `scope` / `scope_id` system that allows model-level rate limits and budgets to be pinned to a specific virtual key, so per-VK model limits can be enforced independently of (and in addition to) the global model limits. ## Changes - Added `scope` (default `"global"`) and `scope_id` (nullable) columns to `governance_model_configs` via a new idempotent migration. Existing rows are backfilled to `"global"` and the unique index is swapped from `(model_name, provider)` to `(scope, scope_id, model_name, provider)`. - Introduced `ModelConfigScopeGlobal` and `ModelConfigScopeVirtualKey` constants and a `BeforeSave` hook that validates and normalises scope/scope_id on every write. - Extended `GetModelConfig` to accept `scope` and `scopeID` as lookup parameters so the identity check on create is scope-aware. - Added `CheckVirtualKeyScopedModelBudget` / `CheckVirtualKeyScopedModelRateLimit` and their corresponding `UpdateVirtualKeyScopedModel*UsageInMemory` methods to `GovernanceStore`. These are wired into `EvaluateVirtualKeyRequest` (resolver) and `UsageTracker.UpdateUsage` so scoped limits are both enforced pre-request and incremented post-response. - In-memory store keys are namespaced by scope via `modelConfigStoreKey`, preventing global and scoped configs from colliding. `DeleteVirtualKeyInMemory` now evicts scoped model configs (and their owned budgets/rate-limits) when a VK is removed; `DeleteVirtualKey` does the same on the DB side. - `GenerateModelConfigHash` now includes `scope` and `scope_id` so config.json ↔ DB drift detection works correctly for scoped configs. - The `getModelConfigs` HTTP handler gains a `?from_memory=true` shortcut and enriches all responses with a transient `scope_name` field (resolved VK name) for UI display. - `CreateModelConfigRequest` accepts `scope` / `scope_id`; the handler validates the scope value, enforces that `virtual_key` scope references an existing VK, and returns scope-aware conflict messages. - UI: the model-limit sheet gains a Scope selector and a virtual-key combobox (shown only for the `virtual_key` scope). The model-limits table gains a Scope column. `getScopeLabel` is extracted to a shared `ui/lib/utils/labels.ts` and the routing-rules views are updated to import from there. - `.zed/` added to `.gitignore`. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go test ./framework/configstore/... ./plugins/governance/... # UI cd ui pnpm i pnpm build ``` 1. Start the server against a fresh or existing DB — the migration runs automatically and backfills existing model configs to `scope = "global"`. 2. Create a virtual key, then create a model config with `scope = "virtual_key"` and `scope_id = <vk_id>` via the UI or `POST /api/governance/model-configs`. 3. Send requests using that virtual key for the configured model and verify the scoped rate limit / budget is enforced independently of the global model config. 4. Delete the virtual key and confirm the scoped model config and its owned budget/rate-limit rows are removed. ## Breaking changes - [ ] Yes - [x] No The migration is additive and fully backward-compatible. Existing model configs default to `"global"` scope and behave identically to before. ## Related issues ## Security considerations `scope_id` for the `virtual_key` scope is validated against the VK table on creation, preventing configs from being attached to non-existent keys. `scope_id` carries no FK constraint in the DB, so the explicit cascade cleanup on VK deletion is required to avoid orphaned rows — this is implemented both in the DB layer and the in-memory store. ## 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Model limits and rate limits can be scoped to individual virtual keys; UI supports selecting scope and picking a virtual key. API accepts scope/scope_id on create. * **UI** * Model limits table shows a Scope column and friendly scope labels; forms include Scope + Virtual Key selector. * **Behavior / Bug Fixes** * Deleting a virtual key removes its scoped model configs and related owned records. Conflicts now consider scope/scope_id. * **Tests & Migrations** * Added tests and a migration to introduce and backfill model-config scoping. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
akshaydeo
pushed a commit
that referenced
this pull request
Jun 4, 2026
## Summary Model configs previously applied globally to all traffic. This PR introduces a `scope` / `scope_id` system that allows model-level rate limits and budgets to be pinned to a specific virtual key, so per-VK model limits can be enforced independently of (and in addition to) the global model limits. ## Changes - Added `scope` (default `"global"`) and `scope_id` (nullable) columns to `governance_model_configs` via a new idempotent migration. Existing rows are backfilled to `"global"` and the unique index is swapped from `(model_name, provider)` to `(scope, scope_id, model_name, provider)`. - Introduced `ModelConfigScopeGlobal` and `ModelConfigScopeVirtualKey` constants and a `BeforeSave` hook that validates and normalises scope/scope_id on every write. - Extended `GetModelConfig` to accept `scope` and `scopeID` as lookup parameters so the identity check on create is scope-aware. - Added `CheckVirtualKeyScopedModelBudget` / `CheckVirtualKeyScopedModelRateLimit` and their corresponding `UpdateVirtualKeyScopedModel*UsageInMemory` methods to `GovernanceStore`. These are wired into `EvaluateVirtualKeyRequest` (resolver) and `UsageTracker.UpdateUsage` so scoped limits are both enforced pre-request and incremented post-response. - In-memory store keys are namespaced by scope via `modelConfigStoreKey`, preventing global and scoped configs from colliding. `DeleteVirtualKeyInMemory` now evicts scoped model configs (and their owned budgets/rate-limits) when a VK is removed; `DeleteVirtualKey` does the same on the DB side. - `GenerateModelConfigHash` now includes `scope` and `scope_id` so config.json ↔ DB drift detection works correctly for scoped configs. - The `getModelConfigs` HTTP handler gains a `?from_memory=true` shortcut and enriches all responses with a transient `scope_name` field (resolved VK name) for UI display. - `CreateModelConfigRequest` accepts `scope` / `scope_id`; the handler validates the scope value, enforces that `virtual_key` scope references an existing VK, and returns scope-aware conflict messages. - UI: the model-limit sheet gains a Scope selector and a virtual-key combobox (shown only for the `virtual_key` scope). The model-limits table gains a Scope column. `getScopeLabel` is extracted to a shared `ui/lib/utils/labels.ts` and the routing-rules views are updated to import from there. - `.zed/` added to `.gitignore`. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go test ./framework/configstore/... ./plugins/governance/... # UI cd ui pnpm i pnpm build ``` 1. Start the server against a fresh or existing DB — the migration runs automatically and backfills existing model configs to `scope = "global"`. 2. Create a virtual key, then create a model config with `scope = "virtual_key"` and `scope_id = <vk_id>` via the UI or `POST /api/governance/model-configs`. 3. Send requests using that virtual key for the configured model and verify the scoped rate limit / budget is enforced independently of the global model config. 4. Delete the virtual key and confirm the scoped model config and its owned budget/rate-limit rows are removed. ## Breaking changes - [ ] Yes - [x] No The migration is additive and fully backward-compatible. Existing model configs default to `"global"` scope and behave identically to before. ## Related issues ## Security considerations `scope_id` for the `virtual_key` scope is validated against the VK table on creation, preventing configs from being attached to non-existent keys. `scope_id` carries no FK constraint in the DB, so the explicit cascade cleanup on VK deletion is required to avoid orphaned rows — this is implemented both in the DB layer and the in-memory store. ## 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Model limits and rate limits can be scoped to individual virtual keys; UI supports selecting scope and picking a virtual key. API accepts scope/scope_id on create. * **UI** * Model limits table shows a Scope column and friendly scope labels; forms include Scope + Virtual Key selector. * **Behavior / Bug Fixes** * Deleting a virtual key removes its scoped model configs and related owned records. Conflicts now consider scope/scope_id. * **Tests & Migrations** * Added tests and a migration to introduce and backfill model-config scoping. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
18 tasks
akshaydeo
added a commit
that referenced
this pull request
Jun 6, 2026
## Summary This PR bumps the Go toolchain version from `1.26.3` to `1.26.4` across all modules and CI workflows, and cuts a new release (`core` v1.5.17, `framework` v1.3.17, `transports` v1.5.9, `plugins/compat` v0.1.16, `plugins/governance` v1.5.17, and associated plugin versions) incorporating a large batch of features and fixes accumulated since the previous release. ## Changes - **Go 1.26.4** — Updated `go-version` in all GitHub Actions workflows (`e2e-tests`, `helm-release`, `pr-tests`, `release-cli`, `release-pipeline`, `snyk`) and all `go.mod` files (core, framework, transports, cli, all plugins, examples, and test modules). - **Core (v1.5.17)** — OpenAI compaction support, multi-customer logs and usage tracking, multiple team/business unit support, `request_headers` wildcard pattern capture for OTel and Maxim plugins, xAI `x_search` tool, fetch URL validation with SSRF hardening, `file://` pricing URL scheme, virtual key provider fan-out filtering, and a broad set of fixes including Anthropic prompt cache key, empty thinking block stripping, OpenAI stream usage event cleanup, Gemini numeric schema constraints, stale connection retries, Azure Claude diagnostic strip, and passthrough budget handling. - **Framework (v1.3.17)** — Scope-aware budgets and limits wired from model configs, provider-level governance, multiple customer budget support with `calendar_aligned` windows, paginated virtual key fetch, `config.json` source-of-truth flow, FTS index cap reduction, sync worker drift fix, cascade deletes for model configs, and high-scale virtual key flow improvements. - **Transports (v1.5.9)** — Full changelog covering all of the above plus UI improvements (log navigation, customer detail sheet, `BudgetDisplay` component, inline loading shell, materialized view alias), SCIM provisioning fields, Helm/config schema additions (`roles`, `per_user_oauth`), client IP resolution from forwarded headers, and dependency upgrades (`recharts` to 3.8.1, `golang.org/x` CVE remediation). - **Plugins** — `governance` v1.5.17 adds team budget/rate-limit exporters, ghost node reconciliation fix, and VK double usage counting fix; `logging` v1.5.17 adds wildcard header capture and file attachment rendering; `otel` v1.2.17 adds `disable_content_logging` and multiple collectors support; `maxim` v1.6.17 adds `request_headers` wildcard capture; `compat` v0.1.16 fixes `max_tokens` preservation during param filtering. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [x] Providers/Integrations - [x] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Verify Go version go version # should report go1.26.4 # Run core tests cd core && go test ./... # Run framework tests cd framework && go test ./... # Run transports tests cd transports && go test ./... # Run plugin tests cd plugins/governance && go test ./... cd plugins/logging && go test ./... cd plugins/otel && go test ./... # UI cd ui pnpm i pnpm build pnpm test ``` ## Breaking changes - [ ] Yes - [x] No ## Related issues #4053, #4066, #4041, #4012, #3976, #3947, #3991, #4045, #3957, #3938, #3937, #3939, #3981, #3998, #3997, #4092, #4091, #4079, #4080, #4086, #3929, #3994, #4028, #3970, #3919, #3861, #3664, #3999, #4088, #4070, #4051, #4043, #4057, #4023, #3941, #3955, #4024, #3956, #3967, #3925, #3992, #3900 ## Security considerations - Fetch URL validation hardened against SSRF by tightening IP checks for private networks and link-local addresses (#4092, #3947, #3991). - Transitive `golang.org/x` dependencies (crypto, net, sys, text) bumped to address Docker Scout CVEs (#3900). ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * OpenAI compaction, multi-customer/team logstore support, request-header wildcard capture, enhanced governance (provider-level & scope-aware limits), disable-content-logging option, support for multiple OpenTelemetry collectors, SSRF hardening and URL validation. * **Chores** * Bumped Go toolchain across modules and updated component/plugin version releases. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Merged
akshaydeo
pushed a commit
that referenced
this pull request
Jun 7, 2026
## Summary Model configs previously applied globally to all traffic. This PR introduces a `scope` / `scope_id` system that allows model-level rate limits and budgets to be pinned to a specific virtual key, so per-VK model limits can be enforced independently of (and in addition to) the global model limits. ## Changes - Added `scope` (default `"global"`) and `scope_id` (nullable) columns to `governance_model_configs` via a new idempotent migration. Existing rows are backfilled to `"global"` and the unique index is swapped from `(model_name, provider)` to `(scope, scope_id, model_name, provider)`. - Introduced `ModelConfigScopeGlobal` and `ModelConfigScopeVirtualKey` constants and a `BeforeSave` hook that validates and normalises scope/scope_id on every write. - Extended `GetModelConfig` to accept `scope` and `scopeID` as lookup parameters so the identity check on create is scope-aware. - Added `CheckVirtualKeyScopedModelBudget` / `CheckVirtualKeyScopedModelRateLimit` and their corresponding `UpdateVirtualKeyScopedModel*UsageInMemory` methods to `GovernanceStore`. These are wired into `EvaluateVirtualKeyRequest` (resolver) and `UsageTracker.UpdateUsage` so scoped limits are both enforced pre-request and incremented post-response. - In-memory store keys are namespaced by scope via `modelConfigStoreKey`, preventing global and scoped configs from colliding. `DeleteVirtualKeyInMemory` now evicts scoped model configs (and their owned budgets/rate-limits) when a VK is removed; `DeleteVirtualKey` does the same on the DB side. - `GenerateModelConfigHash` now includes `scope` and `scope_id` so config.json ↔ DB drift detection works correctly for scoped configs. - The `getModelConfigs` HTTP handler gains a `?from_memory=true` shortcut and enriches all responses with a transient `scope_name` field (resolved VK name) for UI display. - `CreateModelConfigRequest` accepts `scope` / `scope_id`; the handler validates the scope value, enforces that `virtual_key` scope references an existing VK, and returns scope-aware conflict messages. - UI: the model-limit sheet gains a Scope selector and a virtual-key combobox (shown only for the `virtual_key` scope). The model-limits table gains a Scope column. `getScopeLabel` is extracted to a shared `ui/lib/utils/labels.ts` and the routing-rules views are updated to import from there. - `.zed/` added to `.gitignore`. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go test ./framework/configstore/... ./plugins/governance/... # UI cd ui pnpm i pnpm build ``` 1. Start the server against a fresh or existing DB — the migration runs automatically and backfills existing model configs to `scope = "global"`. 2. Create a virtual key, then create a model config with `scope = "virtual_key"` and `scope_id = <vk_id>` via the UI or `POST /api/governance/model-configs`. 3. Send requests using that virtual key for the configured model and verify the scoped rate limit / budget is enforced independently of the global model config. 4. Delete the virtual key and confirm the scoped model config and its owned budget/rate-limit rows are removed. ## Breaking changes - [ ] Yes - [x] No The migration is additive and fully backward-compatible. Existing model configs default to `"global"` scope and behave identically to before. ## Related issues ## Security considerations `scope_id` for the `virtual_key` scope is validated against the VK table on creation, preventing configs from being attached to non-existent keys. `scope_id` carries no FK constraint in the DB, so the explicit cascade cleanup on VK deletion is required to avoid orphaned rows — this is implemented both in the DB layer and the in-memory store. ## 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 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Model limits and rate limits can be scoped to individual virtual keys; UI supports selecting scope and picking a virtual key. API accepts scope/scope_id on create. * **UI** * Model limits table shows a Scope column and friendly scope labels; forms include Scope + Virtual Key selector. * **Behavior / Bug Fixes** * Deleting a virtual key removes its scoped model configs and related owned records. Conflicts now consider scope/scope_id. * **Tests & Migrations** * Added tests and a migration to introduce and backfill model-config scoping. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
akshaydeo
added a commit
that referenced
this pull request
Jun 7, 2026
## ✨ Features - **OpenAI Compaction** — Added OpenAI conversation compaction support across core, framework, logging, and the API surface (#4053) - **Multi-Customer & Org Hierarchy** — Logs and usage tracking now support multiple customers, teams, and business units, including business unit CRUD, team assignment, and governance endpoints in the OpenAPI spec (#4066, #4041, #4082) - **Provider-Level Governance** — Budgets & limits are now scope-aware and can be applied at the virtual-key top level and per provider, wired from the model configs table, with UI filters for scope and providers (#3938, #3937, #3939, #3981, #3962) - **Customer Budgets** — Customers support multiple budgets and `calendar_aligned` budget windows (#3998, #3997) - **Virtual Key Attribution & Controls** — Added a `created_by` user attribution column and a `blacklisted_models` column for virtual key provider configs (#3672, #3653) - **Request Header Capture** — OTel and Maxim observability plugins capture `request_headers` by pattern, with wildcard support (e.g. `x-custom-*`); logging gained the same wildcard header capture (#4012, #3958) - **OTel Content Controls & Collectors** — New `disable_content_logging` option drops message/tool content from exported spans, plus support for multiple OTel collectors (#4064, #3894) - **xAI x_search** — Added xAI `x_search` tool support (#3976) - **URL Validation** — Added fetch URL validation with private-network configuration and link-local blocking (#3947, #3991) - **File Scheme Pricing URLs** — Pricing source URLs now accept the `file://` scheme for air-gapped and self-hosted deployments (#4045) - **Paginated Virtual Keys** — Virtual key fetching is paginated to handle deployments with very large numbers of keys (#3957) - **Client IP Resolution** — Resolve client IP from `X-Forwarded-For`/`X-Real-IP` headers - **SCIM Provisioning** — Added `attributeType`/`attributeValue` SCIM provisioning fields - **Helm/Config Schema** — Added `roles` RBAC governance config and `per_user_oauth` MCP auth to the Helm chart and config schema (#4004, #4009) - **Log Navigation UI** — Added a "View logs" menu item to customer, team, and virtual key tables, clickable links in log detail views, a customer detail sheet, and a reusable `BudgetDisplay` component (#4073, #4054, #4026, #4055) - **Faster First Paint** — Added an inline loading shell to `#root` before React mounts (#4063) - **Materialized View Alias** — Added an `alias` column to the materialized view with filter support (#4078) ## 🐞 Fixed - **Fetch URL IP Checks** — Hardened fetch URL IP checks against SSRF (#4092) - **Mantle Model Matching** — Broadened Mantle model matching to all `gpt` variants (#4091) - **Empty Thinking Blocks** — Strip thinking blocks when the signature is empty (#4079) - **OpenAI Stream Usage** — Removed usage from the `responses.created` event in the OpenAI stream (#4080) - **Prompt Cache Key** — Set the prompt cache key from the Anthropic integration (#4086) - **Upstream Failure Status** — Map upstream connection failures to 502 instead of 400 (#3929) (thanks [@chris-colinsky](https://github.com/chris-colinsky)!) - **Gemini Schema Constraints** — Accept numeric schema integer constraints for Gemini (#3994) (thanks [@yanhao98](https://github.com/yanhao98)!) - **Files Provider Param** — Accept the `?provider=` query param on `GET /v1/files` (#3971) (thanks [@alexef](https://github.com/alexef)!) - **Optional Batch Model** — Made the `model` field optional on `POST /v1/batches` (#3973) (thanks [@alexef](https://github.com/alexef)!) - **Helm Azure Config** — Added missing `azure_key_config` fields to the Helm schema (#3996) (thanks [@axelray-dev](https://github.com/axelray-dev)!) - **Text Completion Chunk Model** — Added the missing `Model` field to `TextCompletionChunkResponse` (#3970) (thanks [@kuishou68](https://github.com/kuishou68)!) - **MCP Inline stdio Env** — MCP stdio server configs accept inline environment variable assignments (#3861) (thanks [@Shushmitaaaa](https://github.com/Shushmitaaaa)!) - **Orphaned Tool Results** — Orphaned tool results in the OpenAI to Anthropic conversion flow are no longer rejected by the Anthropic API (#3919) - **Node Usage Reconciliation** — Added a monotonic `inc_number` log cursor so node usage reconciliation does not skip late async log writes (#3664) - **Bedrock Output Assessments** — Corrected the type of `outputAssessments` in Bedrock responses (#4028) - **Model Pool Pricing Reloads** — Preserve non-pricing model pool entries across pricing reloads (#3999) - **Ghost Node Reconciliation** — Replicate the VK hierarchy flow for ghost node reconciliation (#4088) - **VK Double Usage Counting** — Fixed double usage counting when creating a virtual key (#4070) - **Model Config Lifecycle** — Cascade deletes for model configs and removal of stale in-memory model configs (#4051, #4043) - **FTS Index Cap** — Reduced the FTS index `left()` cap from 800k to 250k chars to stay within the tsvector limit (#4057) - **Sync Worker Drift** — Reduced the sync worker ticker period to 5m to prevent threshold drift (#4023) - **Passthrough** — Fixed passthrough budgets, gated passthrough models per VK, model extraction for Azure passthrough, and restricted fallbacks/provider selection to the VK boundary (#3941, #3988, #3983, #3924) - **Provider Response Headers** — Strip provider response headers and add a content-type filter (#3955, #4024) - **Stream Handling** — Drain non-SSE stream readers and retry stale connections (#3956, #3967) - **Azure Claude** — Strip Azure diagnostic property for Claude models (#3925) - **Compat max_tokens** — Preserve chat `max_tokens` during param filtering (#3992) - **Raw Request Flag** — Removed the raw request flag from providers that don't support it (#4058) - **UI Fixes** — Standardized page container layout, virtual key model configs UI, and dashboard chart tooltips (#4046, #4052, #4044) ## 🔧 Maintenance - **Dependency Upgrades** — Bumped transitive `golang.org/x` dependencies (crypto, net, sys, text) for Docker Scout CVE remediation and `recharts` to 3.8.1; cascaded version bumps across all modules (#3900, #4003)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Model configs previously applied globally to all traffic. This PR introduces a
scope/scope_idsystem that allows model-level rate limits and budgets to be pinned to a specific virtual key, so per-VK model limits can be enforced independently of (and in addition to) the global model limits.Changes
scope(default"global") andscope_id(nullable) columns togovernance_model_configsvia a new idempotent migration. Existing rows are backfilled to"global"and the unique index is swapped from(model_name, provider)to(scope, scope_id, model_name, provider).ModelConfigScopeGlobalandModelConfigScopeVirtualKeyconstants and aBeforeSavehook that validates and normalises scope/scope_id on every write.GetModelConfigto acceptscopeandscopeIDas lookup parameters so the identity check on create is scope-aware.CheckVirtualKeyScopedModelBudget/CheckVirtualKeyScopedModelRateLimitand their correspondingUpdateVirtualKeyScopedModel*UsageInMemorymethods toGovernanceStore. These are wired intoEvaluateVirtualKeyRequest(resolver) andUsageTracker.UpdateUsageso scoped limits are both enforced pre-request and incremented post-response.modelConfigStoreKey, preventing global and scoped configs from colliding.DeleteVirtualKeyInMemorynow evicts scoped model configs (and their owned budgets/rate-limits) when a VK is removed;DeleteVirtualKeydoes the same on the DB side.GenerateModelConfigHashnow includesscopeandscope_idso config.json ↔ DB drift detection works correctly for scoped configs.getModelConfigsHTTP handler gains a?from_memory=trueshortcut and enriches all responses with a transientscope_namefield (resolved VK name) for UI display.CreateModelConfigRequestacceptsscope/scope_id; the handler validates the scope value, enforces thatvirtual_keyscope references an existing VK, and returns scope-aware conflict messages.virtual_keyscope). The model-limits table gains a Scope column.getScopeLabelis extracted to a sharedui/lib/utils/labels.tsand the routing-rules views are updated to import from there..zed/added to.gitignore.Type of change
Affected areas
How to test
scope = "global".scope = "virtual_key"andscope_id = <vk_id>via the UI orPOST /api/governance/model-configs.Breaking changes
The migration is additive and fully backward-compatible. Existing model configs default to
"global"scope and behave identically to before.Related issues
Security considerations
scope_idfor thevirtual_keyscope is validated against the VK table on creation, preventing configs from being attached to non-existent keys.scope_idcarries no FK constraint in the DB, so the explicit cascade cleanup on VK deletion is required to avoid orphaned rows — this is implemented both in the DB layer and the in-memory store.Checklist
docs/contributing/README.mdand followed the guidelinesSummary by CodeRabbit
New Features
UI
Behavior / Bug Fixes
Tests & Migrations