Skip to content

adds reset budget honor flow - #6002

Merged
akshaydeo merged 1 commit into
mainfrom
honour-reset-budget-usage
Aug 10, 2026
Merged

adds reset budget honor flow#6002
akshaydeo merged 1 commit into
mainfrom
honour-reset-budget-usage

Conversation

@akshaydeo

@akshaydeo akshaydeo commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds an operator-triggered budget usage reset that clears a virtual key's spend without advancing its reset boundary. Previously, there was no way to zero a budget's CurrentUsage outside of a natural window expiry — the config write path deliberately preserves live accounting values, and ReloadVirtualKey carries cached usage forward. This PR threads a ResetBudgetUsage flag from the update request through DB persistence, in-memory governance store clearing, and (in enterprise) cluster-wide propagation.

Changes

  • ResetBudgetUsageInMemory on GovernanceStore: New method on LocalGovernanceStore that calls RebaseBudget with zero usage and simultaneously zeroes the LastDBUsages baseline. The baseline must be cleared alongside usage — the dump path writes the delta between in-memory usage and that baseline, so a stale baseline would immediately re-add the spend that was just cleared.
  • budgetUsageReset struct: Carries the operator's reset intent through budget reconciliation (reconcileModelConfigBudgets, reconcileVKModelConfig, syncVKGovernanceToModelConfigs) and collects the IDs of budgets that were actually reset, so the in-memory store can be cleared after the transaction commits. A nil pointer means no reset was requested, keeping all existing call sites unaffected.
  • UpdateVirtualKey handler: Reads req.ResetBudgetUsage, constructs a budgetUsageReset, passes it through reconciliation, and after ReloadVirtualKey completes calls ResetBudgetUsageInMemory on the governance manager. The ordering is deliberate — the reload must happen first because it carries cached usage forward, which would otherwise undo the reset.
  • BifrostHTTPServer.ResetBudgetUsageInMemory: Implements the ServerCallbacks and GovernanceManager interfaces, iterating the collected budget IDs and calling the store method for each. Missing budgets are logged and skipped rather than treated as errors, since a budget can be deleted in the same request that requested the reset.
  • LastReset is intentionally left untouched: Every persistence path guards LastReset forward-only, and the operator reset targets a window that has not yet closed. Moving the boundary is explicitly ruled out.

Type of change

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

go test ./plugins/governance/... -run TestResetBudgetUsageInMemory
go test ./plugins/governance/... -run TestResetBudgetUsageInMemoryMissingBudget
go test ./transports/bifrost-http/...
go test ./...
  1. Create a virtual key with a monthly budget and generate enough traffic to accumulate spend.
  2. Call the update virtual key endpoint with "reset_budget_usage": true.
  3. Verify CurrentUsage is zero in the database and that subsequent requests are not blocked by the previous spend.
  4. Verify LastReset has not changed.
  5. Verify that a subsequent config dump does not restore the old usage value.

Breaking changes

  • Yes
  • No

Related issues

Security considerations

The reset is operator-gated through the existing virtual key update endpoint. No new authentication surface is introduced. The reset does not expose usage history or allow usage to be set to an arbitrary value — only zeroed.

Checklist

  • I read docs/contributing/README.md and followed the guidelines
  • I added/updated tests where appropriate
  • I updated documentation where needed
  • I verified builds succeed (Go and UI)
  • I verified the CI pipeline passes locally if applicable

@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added support for resetting virtual-key budget usage while preserving budget reset boundaries.
    • Reset operations now clear current and persisted usage and propagate updates across the system.
    • Empty reset requests are safely treated as no-ops.
  • Bug Fixes

    • Prevented stale usage snapshots from restoring usage after a reset.
    • Missing budgets are logged without causing reset operations to fail.
    • Added coverage for reset and persistence behavior.

Walkthrough

This change adds explicit virtual-key budget usage reset support. It resets persisted and in-memory usage, preserves LastReset, prevents stale dumps from restoring cleared usage, and adds coverage for existing and missing budgets.

Changes

Budget usage reset

Layer / File(s) Summary
Governance store reset and dump protection
plugins/governance/store.go, plugins/governance/budgetcycle_test.go
The store tracks reset generations, atomically clears usage, clears the persisted baseline, preserves LastReset, and filters stale dump rows. Tests cover existing budgets, missing budgets, and interleaved dump/reset operations.
Server reset callback
transports/bifrost-http/server/server.go, transports/bifrost-http/handlers/pricing_override_test.go
The server callback resets each requested budget through the governance plugin, skips empty input, logs missing budgets, and supports the test governance manager contract.
Virtual-key reset reconciliation
transports/bifrost-http/handlers/governance.go
Virtual-key updates carry reset state through reconciliation, persist zero usage, reload the virtual key, and clear recorded in-memory usage. Other update paths pass no reset request.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant VirtualKeyUpdate
  participant GovernanceManager
  participant BifrostHTTPServer
  participant GovernanceStore
  participant BudgetDatabase
  VirtualKeyUpdate->>GovernanceManager: process reset_budget_usage
  GovernanceManager->>BudgetDatabase: persist zero usage
  GovernanceManager->>BifrostHTTPServer: reset recorded budgets
  BifrostHTTPServer->>GovernanceStore: clear in-memory usage
  GovernanceStore->>GovernanceStore: increment reset generation
  GovernanceStore-->>BifrostHTTPServer: return reset result
Loading

Possibly related PRs

  • maximhq/bifrost#6004: Extends this in-memory reset mechanism and virtual-key reset flow to additional governance owners and request paths.

Suggested reviewers: danpiths, pratham-mishra04, roroghost17

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title identifies the budget reset flow, which is the main change, but its wording is somewhat informal.
Description check ✅ Passed The description covers the purpose, implementation, testing, impact, security, and checklist with only non-critical omissions.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch honour-reset-budget-usage

Comment @coderabbitai help to get the list of available commands.

@akshaydeo
akshaydeo marked this pull request as ready for review August 9, 2026 22:16

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 `@plugins/governance/store.go`:
- Around line 2047-2053: Synchronize ResetBudgetUsageInMemory with DumpBudgets
so a dump cannot persist stale CurrentUsage after the in-memory reset updates
LastDBUsagesBudgets. Use the existing budget/reset synchronization or add a
generation check that invalidates snapshots taken before the reset, while
preserving the reset’s database behavior; add a test covering the interleaving
where DumpBudgets snapshots before ResetBudgetUsageInMemory completes.
🪄 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: 06eabe75-3c2e-4298-bdd0-94a4f0c77ea9

📥 Commits

Reviewing files that changed from the base of the PR and between 40ad078 and c148a5f.

📒 Files selected for processing (5)
  • plugins/governance/budgetcycle_test.go
  • plugins/governance/store.go
  • transports/bifrost-http/handlers/governance.go
  • transports/bifrost-http/handlers/pricing_override_test.go
  • transports/bifrost-http/server/server.go

Comment thread plugins/governance/store.go
coderabbitai[bot]
coderabbitai Bot previously approved these changes Aug 10, 2026
danpiths
danpiths previously approved these changes Aug 10, 2026

akshaydeo commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Merge activity

  • Aug 10, 9:42 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Aug 10, 9:56 PM UTC: Graphite rebased this pull request as part of a merge.
  • Aug 10, 9:57 PM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo
akshaydeo changed the base branch from quarterly-budget-docs to graphite-base/6002 August 10, 2026 21:48
@akshaydeo
akshaydeo changed the base branch from graphite-base/6002 to main August 10, 2026 21:51
@akshaydeo
akshaydeo dismissed stale reviews from danpiths and coderabbitai[bot] August 10, 2026 21:51

The base branch was changed.

@akshaydeo
akshaydeo requested a review from a team as a code owner August 10, 2026 21:51
@akshaydeo
akshaydeo force-pushed the honour-reset-budget-usage branch from 1ef8496 to 26a50f0 Compare August 10, 2026 21:56
@akshaydeo
akshaydeo merged commit 27342e7 into main Aug 10, 2026
15 checks passed
@akshaydeo
akshaydeo deleted the honour-reset-budget-usage branch August 10, 2026 21:57
atharvamhaske pushed a commit to atharvamhaske/bifrost that referenced this pull request Aug 13, 2026
## Summary

Adds an operator-triggered budget usage reset that clears a virtual key's spend without advancing its reset boundary. Previously, there was no way to zero a budget's `CurrentUsage` outside of a natural window expiry — the config write path deliberately preserves live accounting values, and `ReloadVirtualKey` carries cached usage forward. This PR threads a `ResetBudgetUsage` flag from the update request through DB persistence, in-memory governance store clearing, and (in enterprise) cluster-wide propagation.

## Changes

- **`ResetBudgetUsageInMemory` on `GovernanceStore`**: New method on `LocalGovernanceStore` that calls `RebaseBudget` with zero usage and simultaneously zeroes the `LastDBUsages` baseline. The baseline must be cleared alongside usage — the dump path writes the delta between in-memory usage and that baseline, so a stale baseline would immediately re-add the spend that was just cleared.
- **`budgetUsageReset` struct**: Carries the operator's reset intent through budget reconciliation (`reconcileModelConfigBudgets`, `reconcileVKModelConfig`, `syncVKGovernanceToModelConfigs`) and collects the IDs of budgets that were actually reset, so the in-memory store can be cleared after the transaction commits. A nil pointer means no reset was requested, keeping all existing call sites unaffected.
- **`UpdateVirtualKey` handler**: Reads `req.ResetBudgetUsage`, constructs a `budgetUsageReset`, passes it through reconciliation, and after `ReloadVirtualKey` completes calls `ResetBudgetUsageInMemory` on the governance manager. The ordering is deliberate — the reload must happen first because it carries cached usage forward, which would otherwise undo the reset.
- **`BifrostHTTPServer.ResetBudgetUsageInMemory`**: Implements the `ServerCallbacks` and `GovernanceManager` interfaces, iterating the collected budget IDs and calling the store method for each. Missing budgets are logged and skipped rather than treated as errors, since a budget can be deleted in the same request that requested the reset.
- **`LastReset` is intentionally left untouched**: Every persistence path guards `LastReset` forward-only, and the operator reset targets a window that has not yet closed. Moving the boundary is explicitly ruled out.

## Type of change

- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI

## Affected areas

- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [ ] UI (React)
- [ ] Docs

## How to test

```sh
go test ./plugins/governance/... -run TestResetBudgetUsageInMemory
go test ./plugins/governance/... -run TestResetBudgetUsageInMemoryMissingBudget
go test ./transports/bifrost-http/...
go test ./...
```

1. Create a virtual key with a monthly budget and generate enough traffic to accumulate spend.
2. Call the update virtual key endpoint with `"reset_budget_usage": true`.
3. Verify `CurrentUsage` is zero in the database and that subsequent requests are not blocked by the previous spend.
4. Verify `LastReset` has not changed.
5. Verify that a subsequent config dump does not restore the old usage value.

## Breaking changes

- [ ] Yes
- [x] No

## Related issues

## Security considerations

The reset is operator-gated through the existing virtual key update endpoint. No new authentication surface is introduced. The reset does not expose usage history or allow usage to be set to an arbitrary value — only zeroed.

## 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants