Implement budget tracking data structures and database functions#23
Implement budget tracking data structures and database functions#23JetSquirrel merged 2 commits intomainfrom
Conversation
- Add BudgetInfo and BudgetStatus structs to cloud/mod.rs - Create budgets table in database schema - Implement CRUD operations: save_budget, get_budget, get_all_budgets, delete_budget - Add budget status calculation with alert threshold support - Functions integrate with existing cost tracking infrastructure Agent-Logs-Url: https://github.com/JetSquirrel/cloudbridge/sessions/69dc6dbd-6bf3-4a19-b117-1115bc1786db Co-authored-by: JetSquirrel <20291255+JetSquirrel@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds the foundational data model + persistence layer for per-account monthly budgets, enabling later UI/features to compare current-month spend vs configured budget and trigger alerts.
Changes:
- Introduces
BudgetInfoandBudgetStatusmodels for budget configuration and computed status. - Adds a
budgetstable to DuckDB and implements budget CRUD helpers. - Adds budget status computation using cached cost summaries.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/db.rs |
Creates budgets table and adds budget CRUD + status computation functions. |
src/cloud/mod.rs |
Adds budget-related structs (BudgetInfo, BudgetStatus) to shared cloud models. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| alert_threshold DOUBLE NOT NULL DEFAULT 80.0, | ||
| created_at VARCHAR NOT NULL, | ||
| updated_at VARCHAR NOT NULL, | ||
| FOREIGN KEY (account_id) REFERENCES cloud_accounts(id) |
There was a problem hiding this comment.
The new budgets table adds a foreign key to cloud_accounts(id), but delete_account() currently deletes from cost_data only before removing the account. If foreign keys are enforced, deleting an account with an existing budget row will fail. Consider deleting from budgets in delete_account() as well (or adding ON DELETE CASCADE for the FK if that matches the intended behavior).
| FOREIGN KEY (account_id) REFERENCES cloud_accounts(id) | |
| FOREIGN KEY (account_id) REFERENCES cloud_accounts(id) ON DELETE CASCADE |
| // Get account info | ||
| let accounts = get_all_accounts()?; | ||
| let account = accounts | ||
| .iter() | ||
| .find(|a| a.id == account_id) | ||
| .ok_or_else(|| anyhow::anyhow!("Account not found"))?; |
There was a problem hiding this comment.
get_budget_status() loads all accounts via get_all_accounts() and then searches for the matching ID. This is inefficient and will become noticeably slower as the number of accounts grows; it also does extra secret-store work done by get_all_accounts(). Prefer a targeted query to fetch just the account name/provider for account_id (or a dedicated get_account_by_id helper) instead of loading the full list.
| let account = accounts | ||
| .iter() | ||
| .find(|a| a.id == account_id) | ||
| .ok_or_else(|| anyhow::anyhow!("Account not found"))?; |
There was a problem hiding this comment.
The error returned when an account is missing is too generic for troubleshooting. Including the account_id in the message (and ideally clarifying whether it was missing from cloud_accounts) would make logs and UI errors much easier to diagnose.
| .ok_or_else(|| anyhow::anyhow!("Account not found"))?; | |
| .ok_or_else(|| { | |
| anyhow::anyhow!( | |
| "Account with id '{}' not found in cloud_accounts while computing budget status", | |
| account_id | |
| ) | |
| })?; |
| /// Get all budget statuses | ||
| pub fn get_all_budget_statuses() -> Result<Vec<BudgetStatus>> { | ||
| let budgets = get_all_budgets()?; | ||
| let mut statuses = Vec::new(); | ||
|
|
||
| for budget in budgets { | ||
| if let Some(status) = get_budget_status(&budget.account_id)? { | ||
| statuses.push(status); | ||
| } | ||
| } |
There was a problem hiding this comment.
get_all_budget_statuses() calls get_budget_status() per budget, and get_budget_status() re-queries the budget and re-loads all accounts each time. This creates an N+1 pattern and can result in a lot of redundant DB/keyring work. Consider computing statuses in batch: fetch all accounts once (or build a map), reuse the BudgetInfo already loaded, and then look up cached summaries per account.
Adds the foundation for budget tracking functionality to enable users to set monthly budgets per account and compare against actual spending.
Changes
Data Models (
src/cloud/mod.rs)BudgetInfo: Stores budget configuration (monthly_budget, currency, alert_threshold)BudgetStatus: Computed comparison of budget vs actual with alert stateDatabase Schema (
src/db.rs)budgetstable with account-level budget storage and foreign key constraintsCRUD Operations (
src/db.rs)save_budget()/get_budget()/get_all_budgets()/delete_budget()get_budget_status(): Calculates percentage used, remaining budget, alert statusget_all_budget_statuses(): Batch status computationExample Usage
UI integration and budget management features are not included in this PR.