remove budget level calendar alignemnt by akshaydeo · Pull Request #3434 · maximhq/bifrost

akshaydeo · 2026-05-12T13:37:26Z

Summary

The CalendarAligned field has been moved from TableBudget to TableVirtualKey. Previously, each budget row stored its own calendar_aligned flag, but this caused redundancy since calendar alignment is a property of the virtual key, not individual budgets. Budget reset logic now looks up the virtual key associated with a budget to determine whether calendar alignment applies.

Changes

Removed the CalendarAligned field from TableBudget and its associated database column.
Updated ResetExpiredBudgetsInMemory to resolve calendar alignment by loading the virtual key linked to the budget and reading CalendarAligned from there.
Removed CalendarAligned from budget struct literals in createTeam and updateTeam handlers, since it is no longer a budget-level property.
Simplified the updateTeam calendar-alignment transition logic: the previous false → true guard is no longer needed since the flag is no longer stored on the budget itself.
Bumped github.com/jackc/pgx/v5 from v5.9.1 to v5.9.2 in the prompts plugin.

Type of change

Affected areas

How to test

go test ./...

Create a virtual key with calendar_aligned: true.
Attach a budget to that virtual key.
Trigger ResetExpiredBudgetsInMemory and confirm the budget resets at the correct calendar boundary.
Confirm budgets attached to virtual keys without calendar_aligned reset on a rolling interval as before.

Breaking changes

Yes
No

The calendar_aligned column is removed from the budgets table. A database migration dropping this column is required. Any existing data in that column will be lost; ensure virtual keys are configured with the correct CalendarAligned value before migrating.

Related issues

Security considerations

None.

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

akshaydeo · 2026-05-12T13:37:41Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

coderabbitai · 2026-05-12T13:38:04Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b135d196-10af-4de6-baf1-65b75c89aef3

📥 Commits

Reviewing files that changed from the base of the PR and between 256fc09 and 0f30480.

📒 Files selected for processing (4)

framework/configstore/tables/budget.go
framework/configstore/tables/team.go
plugins/governance/store.go
transports/bifrost-http/handlers/governance.go

💤 Files with no reviewable changes (1)

framework/configstore/tables/budget.go

✅ Files skipped from review due to trivial changes (1)

framework/configstore/tables/team.go

🚧 Files skipped from review as they are similar to previous changes (2)

plugins/governance/store.go
transports/bifrost-http/handlers/governance.go

📝 Walkthrough

Summary by CodeRabbit

New Features
- Team objects now include a non-persistent calendar_aligned flag in API responses.
Behavior Changes
- Budget creation and updates use the incoming calendar_aligned signal to compute LastReset and may snap LastReset and zero usage.
- Budget records no longer persist a calendar_aligned attribute; alignment is treated as transient.
- Reset logic now derives calendar alignment from the associated virtual key; missing keys fall back to duration-based resets.
Refactor
- Calendar-aligned reset handling reorganized for consistency.

Walkthrough

Calendar-aligned reset moved off persisted budgets: TableBudget.CalendarAligned removed; TableTeam.CalendarAligned added as a non-persisted JSON field. Handlers compute LastReset from incoming calendar_aligned and stores derive alignment from the linked TableVirtualKey during reset logic.

Changes

Calendar-aligned budget refactoring

Layer / File(s)	Summary
Storage: remove persisted flag and derive alignment from virtual key `framework/configstore/tables/budget.go`, `plugins/governance/store.go`	`TableBudget.CalendarAligned` removed; `ResetExpiredBudgetsInMemory` looks up `TableVirtualKey` via `budget.VirtualKeyID` and uses `virtualKey.CalendarAligned`, defaulting to false if missing.
HTTP handlers and team struct: compute LastReset, do not persist flag `framework/configstore/tables/team.go`, `transports/bifrost-http/handlers/governance.go`	`TableTeam` exposes a non-persisted `calendar_aligned` JSON field. Handlers compute `LastReset` via `budgetLastReset(b.CalendarAligned, b.ResetDuration)`, set `CurrentUsage=0` and `TeamID`, and apply snap-and-zero when incoming `calendar_aligned=true` without persisting `CalendarAligned` to budget rows.

Sequence Diagram

sequenceDiagram
  participant Client
  participant Handler as transports/bifrost-http/handlers/governance.go
  participant Store as plugins/governance/store.go
  participant VirtualKey as TableVirtualKey

  Client->>Handler: create/update team (payload includes calendar_aligned)
  Handler->>Store: persist/modify budgets (compute LastReset from payload)
  Store->>VirtualKey: lookup by VirtualKeyID during ResetExpiredBudgetsInMemory
  VirtualKey-->>Store: return CalendarAligned value used for reset decision

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I am a rabbit in a code-lined glen,
I hop through structs and jump again.
A flag hopped out, the key now knows,
Handlers set times, the reset wind blows. 🐇✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly describes the main change: removing the CalendarAligned field from the budget table level.
Description check	✅ Passed	The description comprehensively covers the purpose, changes, type, affected areas, testing steps, and breaking changes with clear explanations.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch 05-12-remove_budget_level_calendar_alignemnt

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 @coderabbitai help to get the list of available commands and usage tips.}

CLAassistant · 2026-05-12T13:40:03Z

All committers have signed the CLA.

greptile-apps · 2026-05-12T13:41:10Z

Confidence Score: 2/5

Not safe to merge: fresh installs will fail on startup due to the broken migration, existing deployments will accumulate schema drift, and team/provider-config budgets will silently stop honouring calendar boundaries after the change goes live.

Multiple distinct defects affect core paths: the existing migrateCalendarAlignedToBudgetsAndRateLimitsTable migration aborts on fresh installs, no DropColumn migration is included for existing installs, team-owned and provider-config-owned budgets permanently lose calendar alignment in the periodic reset loop, and the unconditional snap in updateTeam discards in-flight spend data on routine updates.

All four changed files need attention: budget.go and the migration layer for schema correctness, store.go for the incomplete alignment lookup, and governance.go for the unconditional reset and stale comment in updateTeam.

Important Files Changed

Filename	Overview
framework/configstore/tables/budget.go	Removes `CalendarAligned` from `TableBudget`; breaks the existing `migrateCalendarAlignedToBudgetsAndRateLimitsTable` migration on fresh installs (GORM can no longer resolve the field) and leaves the `calendar_aligned` DB column in place on existing installs without a `DropColumn` migration.
framework/configstore/tables/team.go	Adds a `gorm:"-"` `CalendarAligned` field to `TableTeam`; the field is never populated anywhere in this PR, so every GET-team response will always return `"calendar_aligned": false`.
plugins/governance/store.go	Calendar-alignment is now only resolved via the linked VK, so team-owned budgets and provider-config-owned budgets (both having `VirtualKeyID == nil`) silently fall back to rolling-duration resets and permanently drift off calendar boundaries.
transports/bifrost-http/handlers/governance.go	Removes `wasCalendarAligned` guard in `updateTeam`; the snap-and-zero now fires unconditionally on every update that sends `calendar_aligned: true`, silently discarding in-flight spending on routine `MaxLimit` bumps. The stale comment still describes the removed `false→true` transition semantics.

_{Reviews (7): Last reviewed commit: "remove budget level calendar alignemnt" | Re-trigger Greptile}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

transports/bifrost-http/handlers/governance.go (1)

1687-1694: 💤 Low value

Stale comment describes old behavior.

The comment references "on the false → true transition" but the code was changed to trigger whenever b.CalendarAligned is true, regardless of previous state. Since TableBudget no longer has a CalendarAligned field, the transition check is no longer possible. Update the comment to accurately reflect the new behavior.

📝 Suggested comment update

-					// Match the UI's calendar-alignment confirmation promise: on the
-					// false → true transition, snap LastReset to the current period
-					// start and zero out CurrentUsage now, instead of lazily waiting
-					// for the next period boundary in ResetExpiredBudgetsInMemory.
+					// When calendar_aligned is true in the request, snap LastReset to
+					// the current calendar period start and zero out CurrentUsage now,
+					// instead of waiting for the next period boundary in
+					// ResetExpiredBudgetsInMemory.
					if b.CalendarAligned {

🤖 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.go` around lines 1687 - 1694,
Update the stale comment to reflect that the code now unconditionally applies
the calendar-aligned reset when b.CalendarAligned is true (rather than only on a
false→true transition); mention that this block sets existing.LastReset via
configstoreTables.GetCalendarPeriodStart(b.ResetDuration, time.Now()) and zeroes
existing.CurrentUsage immediately, and remove the reference to the removed
TableBudget.CalendarAligned transition and to ResetExpiredBudgetsInMemory
waiting behavior so the comment matches the current logic.

🤖 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 1378-1383: The lookup uses the VK ID but gs.virtualKeys is keyed
by the virtual key value, so replace the Load key to use the budget's
virtual-key value (not VirtualKeyID) when calling gs.virtualKeys.Load; after
loading cast to configstoreTables.TableVirtualKey and read CalendarAligned into
calendarAligned (same as current cast), ensuring the calendar-aligned branch can
run. Refer to gs.virtualKeys.Load, budget.VirtualKeyID (replace), the budget's
virtual-key value field, and configstoreTables.TableVirtualKey.CalendarAligned
when making the change.

---

Nitpick comments:
In `@transports/bifrost-http/handlers/governance.go`:
- Around line 1687-1694: Update the stale comment to reflect that the code now
unconditionally applies the calendar-aligned reset when b.CalendarAligned is
true (rather than only on a false→true transition); mention that this block sets
existing.LastReset via configstoreTables.GetCalendarPeriodStart(b.ResetDuration,
time.Now()) and zeroes existing.CurrentUsage immediately, and remove the
reference to the removed TableBudget.CalendarAligned transition and to
ResetExpiredBudgetsInMemory waiting behavior so the comment matches the current
logic.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 705dc2f4-db1c-4f2c-aa0e-a4a1e81af3df

📥 Commits

Reviewing files that changed from the base of the PR and between cee9005 and f5ee0c2.

⛔ Files ignored due to path filters (1)

plugins/prompts/go.sum is excluded by !**/*.sum

📒 Files selected for processing (4)

framework/configstore/tables/budget.go
plugins/governance/store.go
plugins/prompts/go.mod
transports/bifrost-http/handlers/governance.go

💤 Files with no reviewable changes (1)

framework/configstore/tables/budget.go

coderabbitai

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`:
- Around line 1691-1698: The current code unconditionally snaps LastReset and
zeros CurrentUsage whenever b.CalendarAligned is true, causing unintended resets
on updates; modify the logic in the update path to compute desiredStart :=
configstoreTables.GetCalendarPeriodStart(b.ResetDuration, time.Now()) and only
set existing.LastReset = desiredStart and existing.CurrentUsage = 0 if
existing.LastReset does not equal desiredStart (or if existing.LastReset is
zero/uninitialized), thereby detecting the false→true transition (or initial
create) instead of always snapping on every update; keep the same symbols
b.CalendarAligned, existing.LastReset, existing.CurrentUsage,
configstoreTables.GetCalendarPeriodStart, b.ResetDuration and time.Now() when
implementing this check.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 987818fd-d81b-4545-9c02-75ed58c0cbeb

📥 Commits

Reviewing files that changed from the base of the PR and between 23d3ccd and 256fc09.

📒 Files selected for processing (4)

framework/configstore/tables/budget.go
framework/configstore/tables/team.go
plugins/governance/store.go
transports/bifrost-http/handlers/governance.go

💤 Files with no reviewable changes (1)

framework/configstore/tables/budget.go

🚧 Files skipped from review as they are similar to previous changes (1)

framework/configstore/tables/team.go

The merge-base changed after approval.

akshaydeo · 2026-05-15T15:20:21Z

Merge activity

May 15, 3:20 PM UTC: A user started a stack merge that includes this pull request via Graphite.
May 15, 3:20 PM UTC: @akshaydeo merged this pull request with Graphite.

## Summary The `CalendarAligned` field has been moved from `TableBudget` to `TableVirtualKey`. Previously, each budget row stored its own `calendar_aligned` flag, but this caused redundancy since calendar alignment is a property of the virtual key, not individual budgets. Budget reset logic now looks up the virtual key associated with a budget to determine whether calendar alignment applies. ## Changes - Removed the `CalendarAligned` field from `TableBudget` and its associated database column. - Updated `ResetExpiredBudgetsInMemory` to resolve calendar alignment by loading the virtual key linked to the budget and reading `CalendarAligned` from there. - Removed `CalendarAligned` from budget struct literals in `createTeam` and `updateTeam` handlers, since it is no longer a budget-level property. - Simplified the `updateTeam` calendar-alignment transition logic: the previous `false → true` guard is no longer needed since the flag is no longer stored on the budget itself. - Bumped `github.com/jackc/pgx/v5` from `v5.9.1` to `v5.9.2` in the prompts plugin. ## Type of change - [ ] Bug fix - [x] Refactor - [ ] Feature - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./... ``` 1. Create a virtual key with `calendar_aligned: true`. 2. Attach a budget to that virtual key. 3. Trigger `ResetExpiredBudgetsInMemory` and confirm the budget resets at the correct calendar boundary. 4. Confirm budgets attached to virtual keys without `calendar_aligned` reset on a rolling interval as before. ## Breaking changes - [x] Yes - [ ] No The `calendar_aligned` column is removed from the `budgets` table. A database migration dropping this column is required. Any existing data in that column will be lost; ensure virtual keys are configured with the correct `CalendarAligned` value before migrating. ## Related issues ## Security considerations None. ## 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

akshaydeo marked this pull request as ready for review May 12, 2026 13:37

coderabbitai Bot requested changes May 12, 2026

View reviewed changes

Comment thread plugins/governance/store.go

greptile-apps Bot reviewed May 12, 2026

View reviewed changes

Comment thread transports/bifrost-http/handlers/governance.go

Comment thread plugins/governance/store.go

Comment thread transports/bifrost-http/handlers/governance.go

Comment thread plugins/prompts/go.sum

akshaydeo force-pushed the 05-12-remove_budget_level_calendar_alignemnt branch from f5ee0c2 to 63802ac Compare May 12, 2026 14:37

greptile-apps Bot reviewed May 12, 2026

View reviewed changes

Comment thread framework/configstore/tables/team.go

roroghost17 mentioned this pull request May 13, 2026

fix: fixes calendar_aligned migration and UI for VK #3452

Merged

18 tasks

akshaydeo force-pushed the dev branch 2 times, most recently from 47a6eb6 to 59c5f71 Compare May 13, 2026 10:59

roroghost17 force-pushed the 05-12-remove_budget_level_calendar_alignemnt branch from 63802ac to b9d05f4 Compare May 13, 2026 18:05

roroghost17 requested a review from a team as a code owner May 13, 2026 18:05

roroghost17 mentioned this pull request May 13, 2026

feat: implements calendar align feature at team level #3476

Merged

18 tasks

roroghost17 force-pushed the 05-12-remove_budget_level_calendar_alignemnt branch from b9d05f4 to d68f3c9 Compare May 13, 2026 18:07

coderabbitai Bot requested review from danpiths and roroghost17 May 13, 2026 18:07

akshaydeo force-pushed the dev branch from 89f0bbb to 8be492f Compare May 14, 2026 12:51

akshaydeo commented May 14, 2026

View reviewed changes

Comment thread framework/logstore/migrations.go

roroghost17 force-pushed the 05-12-remove_budget_level_calendar_alignemnt branch from d68f3c9 to f67bf9d Compare May 14, 2026 14:10

coderabbitai Bot approved these changes May 14, 2026

View reviewed changes

greptile-apps Bot reviewed May 14, 2026

View reviewed changes

Comment thread plugins/governance/store.go

roroghost17 force-pushed the 05-12-remove_budget_level_calendar_alignemnt branch 3 times, most recently from 23d3ccd to 256fc09 Compare May 15, 2026 05:44

coderabbitai Bot requested changes May 15, 2026

View reviewed changes

Comment thread transports/bifrost-http/handlers/governance.go

coderabbitai Bot previously approved these changes May 15, 2026

View reviewed changes

akshaydeo force-pushed the dev branch from 0914e31 to 6b78c0a Compare May 15, 2026 05:59

remove budget level calendar alignemnt

0f30480

roroghost17 force-pushed the 05-12-remove_budget_level_calendar_alignemnt branch from 256fc09 to 0f30480 Compare May 15, 2026 11:34

coderabbitai Bot approved these changes May 15, 2026

View reviewed changes

akshaydeo merged commit 685aed8 into dev May 15, 2026
14 of 15 checks passed

akshaydeo deleted the 05-12-remove_budget_level_calendar_alignemnt branch May 15, 2026 15:20

This was referenced May 17, 2026

Budget & rate-limit state is per-replica (sync.Map); multi-replica deployments silently leak past caps #3547

Closed

migration fix for calendar aligned #3553

Merged

fix: migration fix for drop columns for calendar aligned #3614

Merged

This was referenced May 20, 2026

fixing test cases #3639

Merged

feat: add instance-level global budget and rate limit governance tier evaluated before all other tiers #3657

Closed

This was referenced Jul 3, 2026

[Feature Request] Budget & Utilization Alignment: Timezone Support, Advanced Date Scheduling (UI/API), Manual Reset, and Others Currency #4892

Open

feat: add budget reset endpoint and functionality #4921

Open

Conversation

akshaydeo commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Type of change

Affected areas

How to test

Breaking changes

Related issues

Security considerations

Checklist

Uh oh!

akshaydeo commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Sequence Diagram

Estimated code review effort

Poem

Uh oh!

CLAassistant commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps Bot commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 2/5

Important Files Changed

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

akshaydeo commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

akshaydeo commented May 12, 2026 •

edited

Loading

akshaydeo commented May 12, 2026 •

edited

Loading

coderabbitai Bot commented May 12, 2026 •

edited

Loading

CLAassistant commented May 12, 2026 •

edited

Loading

greptile-apps Bot commented May 12, 2026 •

edited

Loading

akshaydeo commented May 15, 2026 •

edited

Loading