feat(assistant): add get_budget function for budget tracking#1966
feat(assistant): add get_budget function for budget tracking#1966wolstad wants to merge 5 commits into
Conversation
Exposes the existing Budget / BudgetCategory pacing data to the AI assistant as a `get_budget` function. Supports a target month and an optional `prior_months` window for trend comparison, with the response shape matching the budget UI (totals, income, per-category status, suggested daily spend on the current month). Honors custom month_start_day by matching `Budget.param_to_date` semantics for explicit slug input, so `month` round-trips with the response's `month` field. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
superagent-security
Bot
added
contributor:verified
📝 WalkthroughWalkthroughAdds Assistant::Function::GetBudget and registers it; the function parses month inputs (including custom family month starts), builds month-by-month budget payloads with totals and hierarchical categories, computes category statuses and conditional daily suggestions, centralizes budget period logic, and adds end-to-end tests. ChangesBudget Query Function
Sequence Diagram(s)sequenceDiagram
participant Client
participant Assistant_GetBudget
participant Budget
participant BudgetCategory
Client->>Assistant_GetBudget: call(params month, prior_months)
Assistant_GetBudget->>Budget: period_for(month) / find_or_bootstrap(period)
Budget-->>Assistant_GetBudget: budget + categories
Assistant_GetBudget->>BudgetCategory: compute suggested_daily_spending/status
Assistant_GetBudget-->>Client: structured month-by-month payload
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/models/assistant/function/get_budget_test.rb (1)
94-104: ⚡ Quick winConsider using fixture references instead of hard-coded category names.
Line 96 finds a budget category by comparing
bc.category.name == "Food & Drink". This approach is fragile if category names change due to localization or data updates. A more robust approach would use a fixture reference or a stable identifier.Based on learnings, string comparisons for category identification should be avoided in favor of predicates or identity checks.
♻️ Suggested approach using fixture reference
If a fixture is defined for the Food & Drink category, reference it directly:
- food_bc = budget.budget_categories.find { |bc| bc.category.name == "Food & Drink" } + food_category = categories(:food_and_drink) + food_bc = budget.budget_categories.find { |bc| bc.category == food_category }Alternatively, if a predicate exists:
- food_bc = budget.budget_categories.find { |bc| bc.category.name == "Food & Drink" } + food_bc = budget.budget_categories.find { |bc| bc.category.food_and_drink? }Based on learnings: Do not compare hard-coded strings to identify categories in Ruby code. Prefer model-level predicates or identity checks with canonical categories for maintainability across translations.
🤖 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 `@test/models/assistant/function/get_budget_test.rb` around lines 94 - 104, Replace the fragile string match that finds the "Food & Drink" budget category with a stable reference or predicate: instead of budget.budget_categories.find { |bc| bc.category.name == "Food & Drink" } use a fixture/model identity or predicate (e.g., use the fixture object for the food category or a model predicate like bc.category.food? or compare bc.category.id to the fixture's id) so the test uses Budget, budget.budget_categories, and the same category fixture/identifier used elsewhere; keep the rest of the test (BudgetCategory.any_instance.stubs(:actual_spending) and `@function.call`({})) unchanged.
🤖 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.
Nitpick comments:
In `@test/models/assistant/function/get_budget_test.rb`:
- Around line 94-104: Replace the fragile string match that finds the "Food &
Drink" budget category with a stable reference or predicate: instead of
budget.budget_categories.find { |bc| bc.category.name == "Food & Drink" } use a
fixture/model identity or predicate (e.g., use the fixture object for the food
category or a model predicate like bc.category.food? or compare bc.category.id
to the fixture's id) so the test uses Budget, budget.budget_categories, and the
same category fixture/identifier used elsewhere; keep the rest of the test
(BudgetCategory.any_instance.stubs(:actual_spending) and `@function.call`({}))
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 449bedbf-6ecf-4eeb-8fca-00cb98adf04b
📒 Files selected for processing (3)
app/models/assistant.rbapp/models/assistant/function/get_budget.rbtest/models/assistant/function/get_budget_test.rb
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4326f2e5e7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Replace fragile string match on `bc.category.name == "Food & Drink"` with the `categories(:food_and_drink)` fixture so the test setup isn't sensitive to category-name translations. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
github-project-automation
Bot
moved this to Todo
in Next/future release train (v0.7.2) - June 2026
`Date.strptime` is lenient about trailing characters, so inputs like `"2026-05-01"` or `"may-2026foo"` were parsing successfully and being silently truncated to May 2026. Pre-validate the raw string with anchored regex patterns for the documented YYYY-MM and MMM-YYYY shapes so malformed tool arguments raise Assistant::Error instead. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The helper compared `budget.start_date.month/year` against `Date.current.month/year` and returned nil whenever the current period straddled two calendar months — common for families with `month_start_day != 1` (e.g., May 15–Jun 14 viewed on Jun 1). Replace the calendar-month check with `budget.current?` and compute remaining days from `budget.end_date` so the helper works for both standard and custom periods. This also restores the daily pacing row in the budget UI for custom-month families. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/models/assistant/function/get_budget.rb (1)
168-172: ⚡ Quick winLocalize invalid-month error text via i18n key.
Line 168 and Line 172 hard-code a user-facing message; this should be translated through i18n and keyed by feature.
As per coding guidelines "Organize i18n keys hierarchically by feature... Always use `t()` helper for user-facing strings."Proposed change
- raise Assistant::Error, "Invalid month: #{raw}. Use YYYY-MM or MMM-YYYY." if fmt.nil? + raise Assistant::Error, I18n.t("assistant.functions.get_budget.invalid_month", month: raw) if fmt.nil? Date.strptime(raw, fmt) rescue ArgumentError - raise Assistant::Error, "Invalid month: #{raw}. Use YYYY-MM or MMM-YYYY." + raise Assistant::Error, I18n.t("assistant.functions.get_budget.invalid_month", month: raw) end# config/locales/en.yml en: assistant: functions: get_budget: invalid_month: "Invalid month: %{month}. Use YYYY-MM or MMM-YYYY."🤖 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 `@app/models/assistant/function/get_budget.rb` around lines 168 - 172, Replace the hard-coded error message in the month parsing block with an i18n lookup; where the code currently raises Assistant::Error with "Invalid month: #{raw}. Use YYYY-MM or MMM-YYYY.", change both raise sites (the initial nil-check and the rescued ArgumentError) to call the translation helper (t or I18n.t) using a feature key like "assistant.functions.get_budget.invalid_month" and pass the month as an interpolation variable (month: raw) so the user-facing string is localized.
🤖 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.
Nitpick comments:
In `@app/models/assistant/function/get_budget.rb`:
- Around line 168-172: Replace the hard-coded error message in the month parsing
block with an i18n lookup; where the code currently raises Assistant::Error with
"Invalid month: #{raw}. Use YYYY-MM or MMM-YYYY.", change both raise sites (the
initial nil-check and the rescued ArgumentError) to call the translation helper
(t or I18n.t) using a feature key like
"assistant.functions.get_budget.invalid_month" and pass the month as an
interpolation variable (month: raw) so the user-facing string is localized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2d42e53c-3197-43a7-ac46-f2dd01c37f19
📒 Files selected for processing (4)
app/models/assistant/function/get_budget.rbapp/models/budget_category.rbtest/models/assistant/function/get_budget_test.rbtest/models/budget_category_test.rb
|
The
When
prior = [ params["prior_months"].to_i, 0 ].max
prior = [ prior, MAX_PRIOR_MONTHS ].minThe
def format_money(value)
Money.new(value || 0, family.currency).format
endFor a Missing test: There's a test for the happy path but no test that Generated by Claude Code |
`prior_months: N` was calling `Budget.find_or_bootstrap` for every month, which created empty `Budget` rows (and synced `BudgetCategory` children) as a side effect of an AI query. Only the explicit target month now bootstraps; prior months use `Budget.find_by` and are dropped from the response if they don't exist. The response now includes `months_unavailable: N` so the LLM can phrase a sensible answer when fewer months come back than requested. Extract `Budget.period_for(date, family:)` to share the date-bracket math between `find_or_bootstrap`, `budget_date_valid?`, and the new read-only path in `get_budget`. Adds two tests covering the no-bootstrap behavior for prior months and the `prior_months` clamp at `MAX_PRIOR_MONTHS`. Updates the existing N+1 sorted-months test to seed prior budgets explicitly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@jjmata thanks for the review. Addressed in 37e46bb: #1 (find_or_bootstrap side effect) — fixed. #2 (clamp duplication) — agreed, leaving the Ruby clamp as the safety net since #3 (filter_map silently drops months) — response now includes #4 ( #5 (missing clamp test) — added @coderabbitai on the i18n nitpick ( |
|
Tip For best results, initiate chat on the files or code changes.
Would you like me to open a follow-up GitHub issue to track the Assistant subsystem i18n migration? I can reference this PR and the conversation for context. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/models/assistant/function/get_budget_test.rb (1)
134-134: ⚡ Quick winStub the specific instance instead of all instances.
Using
any_instance.stubsaffects allBudgetCategoryinstances in the test, not justfood_bc. This is broader than necessary and could mask issues if other categories need differentactual_spendingvalues.♻️ Proposed fix to stub only the specific instance
- BudgetCategory.any_instance.stubs(:actual_spending).returns(150) + food_bc.stubs(:actual_spending).returns(150)🤖 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 `@test/models/assistant/function/get_budget_test.rb` at line 134, Replace the broad BudgetCategory.any_instance.stubs(:actual_spending).returns(150) with a stub on the specific test instance (e.g. stub the local variable food_bc) so only that BudgetCategory returns 150 for actual_spending; locate the call to BudgetCategory.any_instance.stubs and change it to stub the specific instance (food_bc.stubs(:actual_spending).returns(150) or equivalent in your test framework).
🤖 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 `@app/models/assistant/function/get_budget.rb`:
- Around line 68-80: The code currently sets requested = month_starts.count {
|start_date| Budget.budget_date_valid?(start_date, family: family) }, which
undercounts requested slots; change requested to represent the total number of
requested month slots (e.g., requested = month_starts.length) so unavailable =
requested - months.length correctly reflects how many requested months were
omitted; keep the month filtering via Budget.budget_date_valid? and
build_month_payload as-is and still set result[:months_unavailable] when
unavailable > 0.
---
Nitpick comments:
In `@test/models/assistant/function/get_budget_test.rb`:
- Line 134: Replace the broad
BudgetCategory.any_instance.stubs(:actual_spending).returns(150) with a stub on
the specific test instance (e.g. stub the local variable food_bc) so only that
BudgetCategory returns 150 for actual_spending; locate the call to
BudgetCategory.any_instance.stubs and change it to stub the specific instance
(food_bc.stubs(:actual_spending).returns(150) or equivalent in your test
framework).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b1c9ad36-6190-4960-915b-6e00cb51fa4e
📒 Files selected for processing (3)
app/models/assistant/function/get_budget.rbapp/models/budget.rbtest/models/assistant/function/get_budget_test.rb
| requested = month_starts.count { |start_date| Budget.budget_date_valid?(start_date, family: family) } | ||
|
|
||
| months = month_starts.filter_map do |start_date| | ||
| next unless Budget.budget_date_valid?(start_date, family: family) | ||
| build_month_payload(start_date, bootstrap: start_date == target_start) | ||
| end | ||
|
|
||
| result = { | ||
| currency: family.currency, | ||
| months: months | ||
| } | ||
| unavailable = requested - months.length | ||
| result[:months_unavailable] = unavailable if unavailable > 0 |
There was a problem hiding this comment.
months_unavailable undercounts when requested months fall outside valid budget range.
Line 68 counts only already-valid months as requested, so asking for more months than the valid window can still report months_unavailable: 0 despite truncation. Count all requested month slots instead.
Suggested fix
- requested = month_starts.count { |start_date| Budget.budget_date_valid?(start_date, family: family) }
+ requested = month_starts.length🤖 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 `@app/models/assistant/function/get_budget.rb` around lines 68 - 80, The code
currently sets requested = month_starts.count { |start_date|
Budget.budget_date_valid?(start_date, family: family) }, which undercounts
requested slots; change requested to represent the total number of requested
month slots (e.g., requested = month_starts.length) so unavailable = requested -
months.length correctly reflects how many requested months were omitted; keep
the month filtering via Budget.budget_date_valid? and build_month_payload as-is
and still set result[:months_unavailable] when unavailable > 0.
2 participants
Summary
Adds an
Assistant::Function::GetBudgetso the AI assistant can answer questions like "how am I tracking against my budget this month?" or "which categories am I over budget on?". The response shape mirrors the budget UI: monthly totals, income, and a parent/subcategory breakdown with per-category status (over_budget,unbudgeted,near_limit,on_track,no_activity) plus suggested daily spend for the current month. Accepts an optionalmonth(YYYY-MM or MMM-YYYY) andprior_monthswindow (0..11) for trend comparison.Honors custom
month_start_dayby matchingBudget.param_to_datefor explicit slugs, so themonthinput round-trips with the response'smonthfield.Test plan
bin/rails test test/models/assistant/function/get_budget_test.rbprior_monthstrendmonth_start_day != 1Summary by CodeRabbit
New Features
Bug Fixes
Tests