Add investment tracking to expenses #381
Conversation
Add new sections to dashboard and reporting around investments.
WalkthroughAdds investment analytics and UI: new InvestmentStatement service and Totals, family accessor, report/dashboard views/partials, category/trade model and DB changes (trade.category), income statement inclusion of trades, and PagesController cashflow sankey signature/logic adjustments. Changes
Sequence Diagram(s)sequenceDiagram
participant Pages as PagesController#dashboard
participant Family as Family
participant InvStmt as InvestmentStatement
participant Totals as InvestmentStatement::Totals
participant Trades as Trades (DB)
participant Accounts as Accounts/Holdings
Pages->>Family: investment_statement (memoized)
Family->>InvStmt: new(family)
Pages->>InvStmt: portfolio_value, top_holdings, unrealized_gains_trend
InvStmt->>Accounts: query investment accounts & holdings
Accounts-->>InvStmt: balances / holdings
rect rgb(220,255,230)
Pages->>InvStmt: totals(period)
InvStmt->>Totals: initialize(trades_scope) & call
Totals->>Trades: SQL aggregate (contributions, withdrawals, trades_count)
Trades-->>Totals: aggregated results
Totals-->>InvStmt: PeriodTotals
end
Pages->>Pages: render _investment_summary partial
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas warranting extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used📓 Path-based instructions (8)**/*.rb📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
test/**/*.rb📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{rb,erb,html.erb}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{erb,html.erb,rb}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{rb,erb}📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
Files:
**/*.{rb,js,erb}📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Files:
**/*.{rb,html.erb}📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Files:
test/**/*_test.rb📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (10)📓 Common learnings📚 Learning: 2025-11-24T16:54:59.185ZApplied to files:
📚 Learning: 2025-11-24T16:55:43.046ZApplied to files:
📚 Learning: 2025-11-24T16:55:43.046ZApplied to files:
📚 Learning: 2025-11-24T16:54:59.185ZApplied to files:
📚 Learning: 2025-11-24T16:56:13.394ZApplied to files:
📚 Learning: 2025-11-24T16:57:02.565ZApplied to files:
📚 Learning: 2025-11-24T16:57:02.565ZApplied to files:
📚 Learning: 2025-11-24T16:57:02.565ZApplied to files:
📚 Learning: 2025-11-24T16:57:34.442ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (1)
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 |
Signed-off-by: soky srm <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (11)
docs/roadmap/investment-integration-assessment.md (1)
1-454: Excellent strategic documentation that grounds the PR in broader architectural context.This roadmap document clearly articulates the current gaps in investment integration, provides a well-structured root-cause analysis, and outlines a pragmatic multi-tiered implementation plan. The technical architecture reference section is particularly valuable for understanding how Holdings, Trades, and Entries fit into the existing balance calculation flow.
The PR implementation aligns well with Tier 1: Quick Wins (dashboard widget, investment categories, cashflow toggle,
InvestmentStatementmodel) and establishes a clear foundation for future Tier 2–4 improvements. This document should serve as the primary reference for future investment-related PRs.Minor enhancement suggestion: Consider adding a "Status by Tier" table at the end to track which items from each tier have been completed and when, making it easier to reference implementation progress in future PRs.
app/models/category.rb (1)
23-24: Investment category wiring is sound; consider reducing name-couplingThe new scopes, constants, defaults, and
investment_category?predicate are internally consistent and match each other. The only downside is the tight coupling to exact name strings:
- Changing any of the four investment category names would silently break
investments,non_investment, andinvestment_category?.INVESTMENT_COLORis defined but the default categories still inline the hex value.If you expect these names to remain stable, this is fine. Otherwise, consider a dedicated boolean column or a classification enum flag for investment categories, or at least reuse
INVESTMENT_COLORindefault_categoriesto avoid duplication.Also applies to: 32-39, 117-122, 148-150
app/views/pages/dashboard.html.erb (1)
35-37: Ensure@investment_statementis always initialized for this viewPassing
period/include_investmentsinto the Sankey partial looks correct, and the investment summary section is well‑placed between outflows and net worth. The only thing to watch is:
@investment_statement.investment_accounts.any?will raise if@investment_statementis ever nil.As long as the dashboard controller unconditionally sets
@investment_statement = Current.family.investment_statementon all paths that render this view, you’re fine; otherwise consider guarding with safe navigation or an early return in the controller.Also applies to: 49-56
app/views/pages/dashboard/_cashflow_sankey.html.erb (1)
1-1: Include‑investments toggle wiring looks good; verify param casting in controllerThe locals signature and the auto‑submit group (checkbox + period select) are wired cleanly. With:
form.check_box :include_investments, { checked: include_investments, data: { "auto-submit-form-target": "auto" } }, "true", "false"Rails will submit
"true"/"false"as strings. Make sure the dashboard controller casts this param explicitly (e.g., viaActiveModel::Type::Boolean.new.cast(params[:include_investments])or a simple string comparison) so@include_investmentsis a real boolean and the Sankey data builder behaves as expected across requests.Also applies to: 9-22
app/controllers/reports_controller.rb (1)
40-42: Investment metrics builder is well-structured and view-friendly
build_investment_metricscleanly encapsulates the investment data shape for the view:
- Early
{ has_investments: false }return keeps the view simple and avoids nil checks.- Uses the same
@periodas the rest of the report, so contributions/withdrawals align with other metrics.- Returns Money objects for monetary fields, matching the app’s money handling.
If you later see performance pressure here, this would be a good spot to layer in caching via
Family#build_cache_key, but what you have now is perfectly reasonable for a first iteration.Also applies to: 356-373
app/views/pages/dashboard/_investment_summary.html.erb (2)
19-22: Consider moving Money object creation to the model layer.Creating
Money.new(trend.value, Current.family.currency)in the view adds domain logic to the template. Theunrealized_gains_trendmethod could return a trend with a pre-formatted money value, similar to howportfolio_value_moneyis exposed. As per coding guidelines, domain logic should be kept out of view templates.
42-47: Add nil guards for security logo and ticker fallback.If
holding.securityis nil orholding.tickeris empty, this code could raise an error. Consider adding defensive checks.- <% if holding.security.logo_url.present? %> - <img src="<%= holding.security.logo_url %>" alt="<%= holding.ticker %>" class="w-8 h-8 rounded-full"> + <% if holding.security&.logo_url.present? %> + <img src="<%= holding.security.logo_url %>" alt="<%= holding.ticker.presence || 'Security' %>" class="w-8 h-8 rounded-full"> <% else %> <div class="w-8 h-8 rounded-full bg-container-inset flex items-center justify-center text-xs font-medium text-secondary"> - <%= holding.ticker[0..1] %> + <%= holding.ticker&.slice(0, 2) || '--' %> </div> <% end %>app/views/reports/_investment_performance.html.erb (2)
27-30: Same Money object creation pattern in view.Similar to the dashboard partial,
Money.new(investment_metrics[:unrealized_trend].value, Current.family.currency)creates domain objects in the view. Consider pre-computing this inbuild_investment_metricsin the controller.
60-60: Add nil guards for optional hash keys.If
investment_metrics[:top_holdings]orinvestment_metrics[:accounts]is nil rather than an empty array, calling.any?will raise aNoMethodError. Consider using safe navigation.- <% if investment_metrics[:top_holdings].any? %> + <% if investment_metrics[:top_holdings]&.any? %>- <% if investment_metrics[:accounts].any? %> + <% if investment_metrics[:accounts]&.any? %>Also applies to: 116-116
app/controllers/pages_controller.rb (1)
118-121: Hardcoded strings should use i18n."Investment Liquidations" and "Investment Contributions" are user-facing strings that should be localized via
t()per coding guidelines.inv_income_idx = add_node.call( "investment_liquidations", - "Investment Liquidations", + t("pages.dashboard.cashflow_sankey.investment_liquidations"), investment_withdrawals,inv_expense_idx = add_node.call( "investment_contributions", - "Investment Contributions", + t("pages.dashboard.cashflow_sankey.investment_contributions"), investment_contributions,Also applies to: 168-171
app/models/investment_statement.rb (1)
149-159: Consider moving Data.define structs to module scope.
PeriodTotalsandHoldingAllocationare returned by public methods but defined in the private section. While this works (constants aren't affected by private), moving them above the public methods would improve discoverability and align with conventional Ruby patterns.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
app/controllers/pages_controller.rb(7 hunks)app/controllers/reports_controller.rb(2 hunks)app/models/category.rb(3 hunks)app/models/family.rb(1 hunks)app/models/investment_statement.rb(1 hunks)app/models/investment_statement/totals.rb(1 hunks)app/views/pages/dashboard.html.erb(2 hunks)app/views/pages/dashboard/_cashflow_sankey.html.erb(1 hunks)app/views/pages/dashboard/_investment_summary.html.erb(1 hunks)app/views/reports/_investment_performance.html.erb(1 hunks)app/views/reports/index.html.erb(1 hunks)config/locales/views/pages/en.yml(2 hunks)config/locales/views/reports/en.yml(1 hunks)docs/roadmap/investment-integration-assessment.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (25)
{app/**/*.{erb,html},app/javascript/**/*.js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Prefer Hotwire-first approach: use Turbo + Stimulus for reactive UI without heavy JavaScript
Files:
app/views/pages/dashboard.html.erbapp/views/reports/_investment_performance.html.erbapp/views/pages/dashboard/_investment_summary.html.erbapp/views/reports/index.html.erbapp/views/pages/dashboard/_cashflow_sankey.html.erb
app/views/**/*.erb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/views/**/*.erb: Always use theiconhelper for icons in views, never uselucide_icondirectly
Use Tailwind design tokens (e.g., text-primary, bg-container, border-secondary) instead of raw color names
Pass data to Stimulus controllers via data-*-value attributes instead of inline JavaScript
Prefer semantic HTML elements (dialog, details, summary) over JavaScript components
Use Turbo Frames for page sections instead of client-side solutions
Use server-side formatting for currencies, numbers, and dates instead of client-sideViews should use ERB and avoid heavy logic; prefer helpers and components instead (ERB checked by
erb-lintper.erb_lint.yml)
Files:
app/views/pages/dashboard.html.erbapp/views/reports/_investment_performance.html.erbapp/views/pages/dashboard/_investment_summary.html.erbapp/views/reports/index.html.erbapp/views/pages/dashboard/_cashflow_sankey.html.erb
**/*.{css,erb}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Tailwind CSS v4.x with custom design system for styling
Files:
app/views/pages/dashboard.html.erbapp/views/reports/_investment_performance.html.erbapp/views/pages/dashboard/_investment_summary.html.erbapp/views/reports/index.html.erbapp/views/pages/dashboard/_cashflow_sankey.html.erb
**/*.{rb,erb,html.erb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{rb,erb,html.erb}: UseCurrent.userfor the current user context in controllers and views. Do NOT usecurrent_user.
UseCurrent.familyfor the current family context. Do NOT usecurrent_family.
Files:
app/views/pages/dashboard.html.erbapp/views/reports/_investment_performance.html.erbapp/controllers/reports_controller.rbapp/models/family.rbapp/views/pages/dashboard/_investment_summary.html.erbapp/views/reports/index.html.erbapp/views/pages/dashboard/_cashflow_sankey.html.erbapp/controllers/pages_controller.rbapp/models/category.rbapp/models/investment_statement.rbapp/models/investment_statement/totals.rb
**/*.{erb,html,vue,jsx,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{erb,html,vue,jsx,tsx}: Use Tailwind CSS v4.x with custom design system defined inapp/assets/tailwind/maybe-design-system.css. Always use functional tokens (e.g.,text-primarynottext-white).
Prefer semantic HTML elements over JavaScript components (e.g., use<dialog>for modals,<details><summary>for disclosures).
Files:
app/views/pages/dashboard.html.erbapp/views/reports/_investment_performance.html.erbapp/views/pages/dashboard/_investment_summary.html.erbapp/views/reports/index.html.erbapp/views/pages/dashboard/_cashflow_sankey.html.erb
**/*.{erb,html.erb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{erb,html.erb}: Useiconhelper for icons, never uselucide_icondirectly.
Leverage Turbo frames for page sections over client-side solutions.
Pass data to Stimulus controllers viadata-*-valueattributes, not inline JavaScript.
**/*.{erb,html.erb}: Use ViewComponents when: element has complex logic or styling patterns, will be reused across multiple views/contexts, needs structured styling with variants/sizes, requires interactive behavior or Stimulus controllers, has configurable slots or complex APIs, or needs accessibility features or ARIA support
Use Partials when: element is primarily static HTML with minimal logic, used in only one or few specific contexts, is simple template content, doesn't need variants/sizes/complex configuration, or is more about content organization than reusable functionality
Prefer components over partials: if a component is available in app/components, use it; if not, look for a partial; if neither exists, decide between component or partial based on complexity and reusability criteria
Keep domain logic out of views: compute values like button classes, conditional logic, and data transformations in the component file, not the template file
Always use the declarative approach when integrating Stimulus controllers in views: the ERB template should declare what happens using data-* attributes, and the Stimulus controller should respond
Component controllers in app/components/ should only be used within their component templates; global controllers in app/javascript/controllers/ can be used across any view
Pass data from Rails to Stimulus using data-*-value attributes, not inline JavaScript
Files:
app/views/pages/dashboard.html.erbapp/views/reports/_investment_performance.html.erbapp/views/pages/dashboard/_investment_summary.html.erbapp/views/reports/index.html.erbapp/views/pages/dashboard/_cashflow_sankey.html.erb
**/*.{erb,html.erb,rb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{erb,html.erb,rb}: All user-facing strings must use localization (i18n) via thet()helper. Update locale files for each new or changed element.
Use server-side formatting for currencies, numbers, and dates. Do not format in JavaScript.
Files:
app/views/pages/dashboard.html.erbapp/views/reports/_investment_performance.html.erbapp/controllers/reports_controller.rbapp/models/family.rbapp/views/pages/dashboard/_investment_summary.html.erbapp/views/reports/index.html.erbapp/views/pages/dashboard/_cashflow_sankey.html.erbapp/controllers/pages_controller.rbapp/models/category.rbapp/models/investment_statement.rbapp/models/investment_statement/totals.rb
{app/components/**/*.html.erb,app/views/**/*.{erb,html.erb}}
📄 CodeRabbit inference engine (CLAUDE.md)
Keep domain logic out of view templates. Logic belongs in component files, not template files.
Files:
app/views/pages/dashboard.html.erbapp/views/reports/_investment_performance.html.erbapp/views/pages/dashboard/_investment_summary.html.erbapp/views/reports/index.html.erbapp/views/pages/dashboard/_cashflow_sankey.html.erb
**/*.erb
📄 CodeRabbit inference engine (CLAUDE.md)
Run
bundle exec erb_lint ./app/**/*.erb -afor ERB linting with auto-correct before pull requests.
Files:
app/views/pages/dashboard.html.erbapp/views/reports/_investment_performance.html.erbapp/views/pages/dashboard/_investment_summary.html.erbapp/views/reports/index.html.erbapp/views/pages/dashboard/_cashflow_sankey.html.erb
**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
**/*.{rb,erb}: UseCurrent.userfor accessing the current user. Do NOT usecurrent_user
UseCurrent.familyfor accessing the current family. Do NOT usecurrent_family
Files:
app/views/pages/dashboard.html.erbapp/views/reports/_investment_performance.html.erbapp/controllers/reports_controller.rbapp/models/family.rbapp/views/pages/dashboard/_investment_summary.html.erbapp/views/reports/index.html.erbapp/views/pages/dashboard/_cashflow_sankey.html.erbapp/controllers/pages_controller.rbapp/models/category.rbapp/models/investment_statement.rbapp/models/investment_statement/totals.rb
**/*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
**/*.html.erb: Prefer native HTML elements over JavaScript-based components. Use semantic HTML: for modals,for disclosures
Leverage Turbo frames to break up pages instead of JavaScript-driven client-side solutions
Use theiconhelper from application_helper.rb for icons. Never uselucide_iconhelper directly
Files:
app/views/pages/dashboard.html.erbapp/views/reports/_investment_performance.html.erbapp/views/pages/dashboard/_investment_summary.html.erbapp/views/reports/index.html.erbapp/views/pages/dashboard/_cashflow_sankey.html.erb
**/*.{rb,js,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Format currencies, numbers, dates, and other values server-side, then pass to Stimulus controllers for display only
Files:
app/views/pages/dashboard.html.erbapp/views/reports/_investment_performance.html.erbapp/controllers/reports_controller.rbapp/models/family.rbapp/views/pages/dashboard/_investment_summary.html.erbapp/views/reports/index.html.erbapp/views/pages/dashboard/_cashflow_sankey.html.erbapp/controllers/pages_controller.rbapp/models/category.rbapp/models/investment_statement.rbapp/models/investment_statement/totals.rb
**/*.{rb,html.erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Use ActiveRecord validations for complex validations and business logic. Simple validations may be mirrored in ActiveRecord for form error handling convenience, but prioritize client-side form validation when possible
Files:
app/views/pages/dashboard.html.erbapp/views/reports/_investment_performance.html.erbapp/controllers/reports_controller.rbapp/models/family.rbapp/views/pages/dashboard/_investment_summary.html.erbapp/views/reports/index.html.erbapp/views/pages/dashboard/_cashflow_sankey.html.erbapp/controllers/pages_controller.rbapp/models/category.rbapp/models/investment_statement.rbapp/models/investment_statement/totals.rb
{app/views/**,app/helpers/**,app/javascript/controllers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
{app/views/**,app/helpers/**,app/javascript/controllers/**}: Reference maybe-design-system.css for base primitives, functional tokens, and component tokens before writing styles
Prefer using functional design system tokens (e.g., text-primary, bg-container, border-primary) from maybe-design-system.css instead of raw Tailwind color values
Files:
app/views/pages/dashboard.html.erbapp/views/reports/_investment_performance.html.erbapp/views/pages/dashboard/_investment_summary.html.erbapp/views/reports/index.html.erbapp/views/pages/dashboard/_cashflow_sankey.html.erb
{app/views/**,app/helpers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Always generate semantic HTML
Files:
app/views/pages/dashboard.html.erbapp/views/reports/_investment_performance.html.erbapp/views/pages/dashboard/_investment_summary.html.erbapp/views/reports/index.html.erbapp/views/pages/dashboard/_cashflow_sankey.html.erb
config/locales/**/*.yml
📄 CodeRabbit inference engine (CLAUDE.md)
Use hierarchical i18n keys by feature:
accounts.index.title,transactions.form.amount_label. Uset()helper for all user-facing strings with interpolation for dynamic content.
Files:
config/locales/views/reports/en.ymlconfig/locales/views/pages/en.yml
config/**/*.{rb,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Configuration files should be placed in
config/directory, with environment examples in.env.local.exampleand.env.test.example
Files:
config/locales/views/reports/en.ymlconfig/locales/views/pages/en.yml
app/views/**/_*.html.erb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use partials only for simple, static HTML with minimal logic in specific contexts
app/views/**/_*.html.erb: Use underscore prefix for partial files (e.g., _trend_change.html.erb, _form_errors.html.erb, _sync_indicator.html.erb)
Place context-specific partials in relevant controller view directory (e.g., accounts/_account_sidebar_tabs.html.erb)
Files:
app/views/reports/_investment_performance.html.erbapp/views/pages/dashboard/_investment_summary.html.erbapp/views/pages/dashboard/_cashflow_sankey.html.erb
app/views/**/_*.{erb,html.erb}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Partials when element is primarily static HTML with minimal logic, used in only one or few contexts, is simple template content, or doesn't need variants/sizes/complex configuration.
Files:
app/views/reports/_investment_performance.html.erbapp/views/pages/dashboard/_investment_summary.html.erbapp/views/pages/dashboard/_cashflow_sankey.html.erb
**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rb: Application supports two modes: 'managed' and 'self_hosted' via Rails.application.config.app_mode
Use Current.user and Current.family instead of current_user / current_family for authentication context
Optimize database queries with proper indexes to prevent N+1 queries using includes/joins
**/*.rb: Use Rails pluralization in i18n:t("transactions.count", count: @transactions.count).
Runbin/rubocop -f github -afor Ruby linting with auto-correct before pull requests.
**/*.rb: Ruby code should use 2-space indent,snake_casefor methods and variables, andCamelCasefor classes and modules, following Rails conventions for folders and file names
Prefer environment variables over hard-coded values for configuration
Files:
app/controllers/reports_controller.rbapp/models/family.rbapp/controllers/pages_controller.rbapp/models/category.rbapp/models/investment_statement.rbapp/models/investment_statement/totals.rb
app/controllers/**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Rails strong parameters and CSRF protection throughout the application
app/controllers/**/*.rb: Use strong parameters and CSRF protection throughout the application.
Use query params for state over localStorage/sessions.
Files:
app/controllers/reports_controller.rbapp/controllers/pages_controller.rb
app/**/*.{rb,js,ts,jsx,tsx,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Code should be organized in
app/directory (Rails MVC, services, jobs, mailers, components), with JS inapp/javascript/, and styles/assets inapp/assets/(Tailwind, images, fonts)
Files:
app/controllers/reports_controller.rbapp/models/family.rbapp/controllers/pages_controller.rbapp/models/category.rbapp/models/investment_statement.rbapp/models/investment_statement/totals.rb
app/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Run
bin/brakemansecurity scan before major PRs to check for static analysis of common Rails issues
Files:
app/controllers/reports_controller.rbapp/models/family.rbapp/controllers/pages_controller.rbapp/models/category.rbapp/models/investment_statement.rbapp/models/investment_statement/totals.rb
app/models/**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/models/**/*.rb: Keep business logic in models using skinny controllers and fat models pattern
Store all monetary values in base currency (user's primary currency) and use Money objects for conversion
Use ActiveRecord validations for form convenience and complex business logic validations
Use Rails concerns and POROs for organizing business logic instead of services folder
app/models/**/*.rb: All monetary values stored in base currency (user's primary currency). UseMoneyobjects for currency conversion and formatting.
Place business logic inapp/models/folder. Avoid creatingapp/services/folder. Use Rails concerns and POROs for organization.
Models should answer questions about themselves using instance methods: useaccount.balance_seriesnotAccountSeries.new(account).call.
Files:
app/models/family.rbapp/models/category.rbapp/models/investment_statement.rbapp/models/investment_statement/totals.rb
{app/models/**/*.rb,db/migrate/**/*.rb}
📄 CodeRabbit inference engine (CLAUDE.md)
Simple validations (null checks, unique indexes) should be in the database. ActiveRecord validations for form convenience (prefer client-side when possible). Complex validations and business logic in ActiveRecord.
Files:
app/models/family.rbapp/models/category.rbapp/models/investment_statement.rbapp/models/investment_statement/totals.rb
🧠 Learnings (13)
📚 Learning: 2025-11-24T16:55:43.046Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.046Z
Learning: Applies to config/locales/**/*.yml : Use hierarchical i18n keys by feature: `accounts.index.title`, `transactions.form.amount_label`. Use `t()` helper for all user-facing strings with interpolation for dynamic content.
Applied to files:
config/locales/views/reports/en.ymlconfig/locales/views/pages/en.yml
📚 Learning: 2025-11-24T16:57:20.307Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-11-24T16:57:20.307Z
Learning: Applies to app/views/**/_*.html.erb : Place context-specific partials in relevant controller view directory (e.g., accounts/_account_sidebar_tabs.html.erb)
Applied to files:
app/views/reports/_investment_performance.html.erbapp/views/pages/dashboard/_investment_summary.html.erb
📚 Learning: 2025-09-23T22:23:22.963Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-09-23T22:23:22.963Z
Learning: Applies to app/views/**/*.html.erb : Use partials when content is primarily static, simple, context-specific, and without variants or complex configuration
Applied to files:
app/views/reports/_investment_performance.html.erb
📚 Learning: 2025-11-24T16:54:59.185Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.185Z
Learning: Applies to app/views/**/_*.html.erb : Use partials only for simple, static HTML with minimal logic in specific contexts
Applied to files:
app/views/reports/_investment_performance.html.erb
📚 Learning: 2025-11-24T16:55:43.046Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.046Z
Learning: Applies to app/views/**/_*.{erb,html.erb} : Use Partials when element is primarily static HTML with minimal logic, used in only one or few contexts, is simple template content, or doesn't need variants/sizes/complex configuration.
Applied to files:
app/views/reports/_investment_performance.html.erb
📚 Learning: 2025-11-24T16:57:20.307Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-11-24T16:57:20.307Z
Learning: Applies to app/views/**/_*.html.erb : Use underscore prefix for partial files (e.g., _trend_change.html.erb, _form_errors.html.erb, _sync_indicator.html.erb)
Applied to files:
app/views/reports/_investment_performance.html.erb
📚 Learning: 2025-11-24T16:57:20.307Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/view_conventions.mdc:0-0
Timestamp: 2025-11-24T16:57:20.307Z
Learning: Applies to **/*.{erb,html.erb} : Use Partials when: element is primarily static HTML with minimal logic, used in only one or few specific contexts, is simple template content, doesn't need variants/sizes/complex configuration, or is more about content organization than reusable functionality
Applied to files:
app/views/reports/_investment_performance.html.erb
📚 Learning: 2025-11-24T16:55:43.046Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.046Z
Learning: Applies to **/*.{erb,html.erb,rb} : All user-facing strings must use localization (i18n) via the `t()` helper. Update locale files for each new or changed element.
Applied to files:
config/locales/views/pages/en.yml
📚 Learning: 2025-11-24T16:56:30.657Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-11-24T16:56:30.657Z
Learning: Account sync must perform transfer auto-matching, calculate daily balance records from start_date to Date.current, calculate holdings, and optionally enrich transaction data
Applied to files:
app/controllers/reports_controller.rbdocs/roadmap/investment-integration-assessment.mdapp/models/investment_statement.rb
📚 Learning: 2025-11-24T16:56:30.657Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-11-24T16:56:30.657Z
Learning: Entry model is a Rails delegated type with three subtypes: Valuation (absolute account value), Transaction (balance modification), and Trade (buy/sell for investment accounts)
Applied to files:
app/controllers/reports_controller.rbapp/views/pages/dashboard/_investment_summary.html.erbdocs/roadmap/investment-integration-assessment.mdapp/models/category.rbapp/models/investment_statement.rbapp/models/investment_statement/totals.rb
📚 Learning: 2025-11-24T16:55:43.046Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.046Z
Learning: Core domain model: User → Accounts → Transactions. Accounts support types: checking, savings, credit cards, investments, crypto, loans, properties. Transactions belong to Categories and may have Tags and Rules.
Applied to files:
docs/roadmap/investment-integration-assessment.md
📚 Learning: 2025-11-14T14:22:37.076Z
Learnt from: jjmata
Repo: we-promise/sure PR: 326
File: app/models/category_import.rb:10-10
Timestamp: 2025-11-14T14:22:37.076Z
Learning: In the CategoryImport model (app/models/category_import.rb), the import process implements a "last write wins" strategy where importing a category.csv file completely overrides existing category structures, including parent relationships. Setting category.parent = nil in the first pass is intentional behavior to ensure the CSV becomes the source of truth.
Applied to files:
app/models/category.rb
📚 Learning: 2025-11-24T16:56:30.657Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-11-24T16:56:30.657Z
Learning: Account model is a Rails delegated type with subtypes: Depository, Investment, Crypto, Property, Vehicle, OtherAsset (assets) and CreditCard, Loan, OtherLiability (liabilities)
Applied to files:
app/models/category.rbapp/models/investment_statement.rb
🧬 Code graph analysis (4)
app/controllers/reports_controller.rb (2)
app/models/family.rb (1)
investment_statement(72-74)app/models/investment_statement.rb (5)
investment_accounts(144-146)totals(15-30)portfolio_value_money(43-45)unrealized_gains_trend(117-125)top_holdings(88-90)
app/controllers/pages_controller.rb (2)
app/models/investment_statement.rb (1)
totals(15-30)app/models/investment_statement/totals.rb (1)
call(7-17)
app/models/investment_statement.rb (4)
app/models/family.rb (1)
entries_cache_version(121-126)app/models/concerns/monetizable.rb (1)
monetize(5-15)app/models/investment_statement/totals.rb (3)
initialize(1-56)initialize(2-5)call(7-17)app/models/period.rb (1)
date_range(133-135)
app/models/investment_statement/totals.rb (2)
app/models/category.rb (1)
initialize(52-55)app/models/investment_statement.rb (1)
initialize(10-12)
🪛 markdownlint-cli2 (0.18.1)
docs/roadmap/investment-integration-assessment.md
43-43: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
163-163: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
169-169: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
398-398: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
430-430: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
444-444: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (13)
config/locales/views/reports/en.yml (1)
109-120: Investment performance i18n block is consistentKey structure and labels under
reports.investment_performancelook coherent and ready for use by the reports UI.app/models/family.rb (1)
72-74: InvestmentStatement accessor matches existing pattern
investment_statementfollows the same memoized pattern asbalance_sheetandincome_statement, which keeps call sites clean and avoids repeated instantiation.app/views/reports/index.html.erb (1)
105-112: Investment performance block is correctly gated by metrics flagConditional rendering based on
@investment_metrics[:has_investments]and passing the full metrics hash into the_investment_performancepartial is a clean integration. Just ensurebuild_investment_metricsalways returns a hash (never nil) so this guard is safe in all cases, which it currently does.config/locales/views/pages/en.yml (1)
25-25: Dashboard investment i18n keys align with new UIThe added
cashflow_sankey.include_investmentslabel and theinvestment_summaryblock are well‑named, scoped underpages.dashboard, and match the expected dashboard investment summary fields.Also applies to: 36-48
app/views/pages/dashboard/_investment_summary.html.erb (1)
1-100: LGTM overall - well-structured partial following conventions.The partial correctly uses i18n via
t(), server-side money formatting, and functional design tokens. The conditional rendering logic is clean and the layout follows the design system.app/views/reports/_investment_performance.html.erb (1)
64-111: Good use of semantic table elements.The top holdings section correctly uses semantic
<table>,<thead>,<tbody>, and proper table row structure, which is better for accessibility than the flexbox approach in the dashboard partial.app/controllers/pages_controller.rb (2)
8-21: LGTM - Clean integration of investment data into dashboard.The investment statement integration follows good patterns: conditional data loading based on params, safe navigation for optional values, and clear separation between regular and investment-inclusive flows.
123-123:Category::INVESTMENT_COLORconstant is properly defined and correctly used.The constant is defined in
app/models/category.rb:32with the value"#0d9488"and is correctly referenced at all specified locations inapp/controllers/pages_controller.rb.app/models/investment_statement/totals.rb (2)
1-56: LGTM - Well-structured SQL aggregation class.The class follows good patterns: single responsibility for aggregation, proper SQL sanitization for parameters, and clear separation of query building. The comments explaining buy vs. sell logic are helpful.
30-47: No security concern identified—trades_scope is safe.Verification confirms
trades_scopealways originates from controlled ActiveRecord queries withinInvestmentStatement(lines 16–18). It's constructed entirely with AR builder methods (joins,where) and filtered only by internal date ranges—never from user input. Whento_sql()is called on an AR relation, Rails maintains SQL parameterization internally, so the interpolation pattern is safe.app/models/investment_statement.rb (3)
66-85: Currency filter may exclude foreign holdings.The query filters holdings by
currency: family.currency(line 74), which would exclude any holdings recorded in foreign currencies. If the portfolio contains international securities, this could underreport holdings. Verify this is intentional or if currency conversion should be applied instead.
93-107: LGTM - Clean allocation calculation.The allocation method correctly computes weight percentages relative to total holdings value and wraps results in a structured
HoldingAllocationobject.
161-167: LGTM - Solid caching strategy.The cache key combines family ID, SQL hash, and entries version, ensuring proper invalidation when data changes. Using MD5 for the SQL hash is appropriate for cache key generation.
| class InvestmentStatement | ||
| include Monetizable | ||
|
|
||
| monetize :total_contributions, :total_dividends, :total_interest, :unrealized_gains |
There was a problem hiding this comment.
🧩 Analysis chain
Verify monetize macro matches actual method names.
The monetize macro references :total_contributions, :total_dividends, :total_interest, :unrealized_gains. However, I only see unrealized_gains defined as a method. The others appear to be missing, which would cause the generated *_money methods to fail.
🏁 Script executed:
#!/bin/bash
# Description: Verify total_contributions, total_dividends, total_interest methods exist
ast-grep --pattern 'def total_contributions'
ast-grep --pattern 'def total_dividends'
ast-grep --pattern 'def total_interest'Length of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Description: View the investment_statement.rb file to understand current state
cat app/models/investment_statement.rb
Length of output: 4637
🏁 Script executed:
#!/bin/bash
# Description: Search for Monetizable concern to understand what monetize does
fd -e rb -path '*/concerns/*' -name '*monetizable*' -o -path '*/concerns/*' | head -20Length of output: 168
🏁 Script executed:
#!/bin/bash
# Description: Search for total_contributions, total_dividends, total_interest usage in the file
rg -n 'total_contributions|total_dividends|total_interest' app/models/investment_statement.rbLength of output: 87
🏁 Script executed:
#!/bin/bash
# Description: Find Monetizable concern definition
rg -n 'module Monetizable' --type rbLength of output: 26
🏁 Script executed:
#!/bin/bash
# Description: Search for monetize macro usage in the codebase to understand patterns
rg -n 'monetize\s+:' --type rb -A2 | head -40Length of output: 26
---
🏁 Script executed:
```shell
#!/bin/bash
# Description: Find Monetizable concern with different search approach
find . -type f -name "*.rb" | xargs grep -l "module Monetizable" | head -5
Length of output: 36
🏁 Script executed:
#!/bin/bash
# Description: Search for monetize macro usage across codebase with proper syntax
rg 'monetize\s+:' -C3 | head -60Length of output: 2758
🏁 Script executed:
#!/bin/bash
# Description: Check if total_contributions, total_dividends, total_interest exist as methods elsewhere
rg 'def (total_contributions|total_dividends|total_interest)' -C2Length of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Description: View Monetizable concern implementation
cat app/models/concerns/monetizable.rb
Length of output: 438
🏁 Script executed:
#!/bin/bash
# Description: View IncomeStatement to compare similar PORO pattern
cat app/models/income_statement.rbLength of output: 4407
Add instance methods for total_contributions, total_dividends, and total_interest.
The monetize macro on line 6 references four methods, but only unrealized_gains is defined. The generated total_contributions_money, total_dividends_money, and total_interest_money methods will fail at runtime with NoMethodError because total_contributions, total_dividends, and total_interest don't exist as instance methods.
Compare with IncomeStatement, which correctly defines median_expense and median_income methods to match its monetize declaration. Add the three missing methods to InvestmentStatement returning numeric values, likely extracting them from the totals() method's PeriodTotals struct or as direct accessor methods.
🤖 Prompt for AI Agents
In app/models/investment_statement.rb around line 6, monetize references
total_contributions, total_dividends, and total_interest but those instance
methods are missing; add three instance methods named total_contributions,
total_dividends, and total_interest that return numeric values (e.g.,
BigDecimal/Integer/Float) so monetize can build the *_money helpers—implement
each to read the corresponding value from the existing totals() result (the
PeriodTotals struct or appropriate accessor) or compute it from available fields
and ensure they return 0 when nil to avoid NoMethodError.
| def unrealized_gains_trend | ||
| holdings = current_holdings.to_a | ||
| return nil if holdings.empty? | ||
|
|
||
| current = holdings.sum(&:amount) | ||
| previous = holdings.sum { |h| h.qty * h.avg_cost.amount } | ||
|
|
||
| Trend.new(current: current, previous: previous) | ||
| end |
There was a problem hiding this comment.
Potential nil access on avg_cost.
Line 122 accesses h.avg_cost.amount without nil checking. If any holding has a nil avg_cost, this will raise NoMethodError.
def unrealized_gains_trend
holdings = current_holdings.to_a
return nil if holdings.empty?
current = holdings.sum(&:amount)
- previous = holdings.sum { |h| h.qty * h.avg_cost.amount }
+ previous = holdings.sum { |h| h.avg_cost ? h.qty * h.avg_cost.amount : 0 }
Trend.new(current: current, previous: previous)
end🤖 Prompt for AI Agents
In app/models/investment_statement.rb around lines 117 to 125, the calculation
for `previous` calls `h.avg_cost.amount` without guarding against `avg_cost`
being nil which can raise NoMethodError; update the sum to handle nil avg_cost
(e.g. skip holdings with nil avg_cost or treat their contribution as zero) by
using a safe check such as `h.avg_cost ? h.qty * h.avg_cost.amount : 0` or
`h.avg_cost&.amount.to_f` so `previous` always gets a numeric value and
`unrealized_gains_trend` won’t raise when any holding has nil `avg_cost`.
| COALESCE(SUM(CASE WHEN t.qty > 0 THEN ABS(ae.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as contributions, | ||
| COALESCE(SUM(CASE WHEN t.qty < 0 THEN ABS(ae.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as withdrawals, |
There was a problem hiding this comment.
Missing exchange rate silently defaults to 1.
When an exchange rate is missing, COALESCE(er.rate, 1) defaults to 1, meaning no currency conversion occurs. This could produce incorrect totals for multi-currency portfolios. Consider logging or flagging when rates are missing for transactions in foreign currencies.
Also applies to: 39-43
🤖 Prompt for AI Agents
In app/models/investment_statement/totals.rb around lines 33-34 (and similarly
lines 39-43), the SQL uses COALESCE(er.rate, 1) which silently treats missing
exchange rates as 1; change the query to detect missing rates instead of
defaulting to 1 — remove the COALESCE fallback and instead multiply by er.rate
only when present (or use a CASE that sets the amount to NULL/0 and sets a flag
when er.rate IS NULL), and add logic to (a) surface a boolean/column indicating
missing_rate for those rows or (b) log/raise an explicit warning when any
transaction uses a foreign currency without an exchange rate so callers can
handle or reject the result.
|
|
||
| The application uses a polymorphic `Entry` model with three entryable types: | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Fix markdown formatting and code block language specifications.
The document has several markdown linting violations:
- Missing code block language specifications (lines 43, 398, 430, 444): Code blocks should include a language identifier (e.g.,
```ruby). - Emphasis used as headings (lines 163, 169): Use markdown heading syntax (
###) instead of bold emphasis for subsection titles.
These violations should be addressed for consistency with markdown best practices.
Apply these fixes:
Line 43: Change ``` to ```ruby
Line 163: Change **Option A: Shadow Transactions** to ### Option A: Shadow Transactions
Line 169: Change **Option B: Parallel Tracking** to ### Option B: Parallel Tracking
Line 398: Change ``` to ```ruby (or add appropriate language)
Line 430: Change ``` to ```ruby (or add appropriate language)
Line 444: Change ``` to ```ruby (or add appropriate language)Alternatively, if these code blocks are intentionally left unlabeled for display purposes, suppress the linting rules with inline comments:
<!-- markdownlint-disable MD040 -->Also applies to: 163-163, 169-169, 398-398, 430-430, 444-444
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
43-43: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
docs/roadmap/investment-integration-assessment.md lines 163,169,398,430,444:
Replace the emphasized subsection titles on lines 163 and 169 with proper
markdown headings by changing "**Option A: Shadow Transactions**" to "### Option
A: Shadow Transactions" and "**Option B: Parallel Tracking**" to "### Option B:
Parallel Tracking"; for the fenced code blocks on lines 43, 398, 430, and 444
add an appropriate language identifier (e.g., change ``` to ```ruby) or, if the
blocks must remain unlabeled for presentation, add an inline markdownlint
disable comment above each block (<!-- markdownlint-disable MD040 -->) to
suppress MD040.
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (9)
app/models/income_statement/totals.rb (1)
48-91: Consider extracting shared SQL to reduce duplication.
transactions_only_query_sqlandtransactions_subquery_sqlare nearly identical (differing only in the count column alias and trailing semicolon). This duplication means changes need to be made in two places.You could extract a shared method with a parameter:
def transactions_sql(count_alias: "transactions_count", include_semicolon: true) <<~SQL SELECT c.id as category_id, c.parent_id as parent_category_id, CASE WHEN ae.amount < 0 THEN 'income' ELSE 'expense' END as classification, ABS(SUM(ae.amount * COALESCE(er.rate, 1))) as total, COUNT(ae.id) as #{count_alias} FROM (#{@transactions_scope.to_sql}) at JOIN entries ae ON ae.entryable_id = at.id AND ae.entryable_type = 'Transaction' LEFT JOIN categories c ON c.id = at.category_id LEFT JOIN exchange_rates er ON ( er.date = ae.date AND er.from_currency = ae.currency AND er.to_currency = :target_currency ) WHERE at.kind NOT IN ('funds_movement', 'one_time', 'cc_payment') AND ae.excluded = false GROUP BY c.id, c.parent_id, CASE WHEN ae.amount < 0 THEN 'income' ELSE 'expense' END#{include_semicolon ? ';' : ''} SQL endapp/models/trade/create_form.rb (1)
57-64: Consider using a constant for the category name lookup.The hardcoded string
"Investment Contributions"makes this lookup fragile—if the default category is renamed, translated, or doesn't exist for a family, the lookup silently fails. Consider defining a constant (e.g.,Category::INVESTMENT_CONTRIBUTIONS_NAME) or using an identifier-based lookup to make this more robust.def investment_category_for(trade_type) return nil unless trade_type == "buy" - account.family.categories.find_by(name: "Investment Contributions") + account.family.categories.find_by(name: Category::INVESTMENT_CONTRIBUTIONS_NAME) enddb/schema.rb (1)
1083-1083: Update migration to addon_delete: :nullifyfor consistency with transactions.The trades category FK uses the default RESTRICT behavior, which prevents deleting categories that have associated trades. Transactions use
on_delete: :nullifyinstead. Align trades to match:- add_reference :trades, :category, null: true, foreign_key: true, type: :uuid + add_reference :trades, :category, null: true, foreign_key: { on_delete: :nullify }, type: :uuidThis ensures both tables handle category deletion consistently: trades become uncategorized rather than blocking the deletion.
app/models/category.rb (2)
24-25: Scopes rely on hardcoded English category names — fragile for renamed or localized categories.These scopes query by
name, which is user-facing and mutable. If a user renames "Dividends" or if localization is introduced, these scopes will silently break. Consider using a dedicatedinvestmentboolean column or aclassificationvalue (e.g.,"investment") instead.If a schema change isn't feasible now, at minimum document that these category names are reserved/system-managed:
+ # Note: These scopes depend on system-managed category names. + # Do not allow users to rename investment categories. scope :investments, -> { where(name: INVESTMENT_CATEGORY_NAMES) } scope :non_investment, -> { where.not(name: INVESTMENT_CATEGORY_NAMES) }
35-40: Hardcoded category names create implicit coupling.
INVESTMENT_CATEGORY_NAMESuses English strings that must exactly match thedefault_categoriesentries. This creates a maintenance burden and potential for drift. Consider extracting these as i18n keys or using a dedicated attribute to mark investment categories.app/models/cashflow_statement.rb (1)
74-79: Summary constant should be public for external consumers.
Summaryis defined in the private section but is returned by the publicsummarymethod. While Ruby allows this, it's unconventional and may confuse consumers who want to type-check or pattern-match against the struct. Consider moving it above theprivatekeyword.+ Summary = Data.define(:operating, :investing, :financing, :net_cash_flow, :total_inflows, :total_outflows) + private - Summary = Data.define(:operating, :investing, :financing, :net_cash_flow, :total_inflows, :total_outflows) def monetizable_currencyapp/models/cashflow_statement/investing_activities.rb (2)
11-14: Inflows represent withdrawals (selling) — semantics are clear but document the sign convention.The comment at line 11 helps, but the mapping of "withdrawals" → "inflows" could confuse maintainers. The
qty < 0convention for sells should be documented or referenced.
41-50: Summary has redundant fields.
contributionsis identical tooutflows_moneyandwithdrawalsis identical toinflows_money. This duplication could lead to drift if one is updated without the other.Consider aliasing instead:
def summary Summary.new( inflows: inflows_money, outflows: outflows_money, net: net_money, - contributions: outflows_money, - withdrawals: inflows_money, + contributions: outflows_money, # alias for clarity in views + withdrawals: inflows_money, # alias for clarity in views trades_count: trades_count ) endOr remove the aliases and use
outflows/inflowsconsistently in views.app/models/cashflow_statement/sankey_builder.rb (1)
186-230: Extract financing color to a constant.The color
"#7c3aed"is repeated 4 times. Extract to a constant for consistency and maintainability, similar toCategory::INVESTMENT_COLOR.+ FINANCING_COLOR = "#7c3aed".freeze + def build_financing_outflows # Loan payments loan_payments = financing.loan_payments.to_f.round(2) if loan_payments.positive? percentage = total_outflows.zero? ? 0 : (loan_payments / total_outflows * 100).round(1) idx = add_node( "loan_payments", "Loan Payments", loan_payments, percentage, - "#7c3aed" # Purple for financing + FINANCING_COLOR )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
app/controllers/pages_controller.rb(3 hunks)app/controllers/trades_controller.rb(1 hunks)app/models/cashflow_statement.rb(1 hunks)app/models/cashflow_statement/financing_activities.rb(1 hunks)app/models/cashflow_statement/investing_activities.rb(1 hunks)app/models/cashflow_statement/operating_activities.rb(1 hunks)app/models/cashflow_statement/sankey_builder.rb(1 hunks)app/models/category.rb(4 hunks)app/models/family.rb(1 hunks)app/models/income_statement/totals.rb(3 hunks)app/models/trade.rb(1 hunks)app/models/trade/create_form.rb(2 hunks)app/views/trades/_trade.html.erb(1 hunks)app/views/trades/show.html.erb(1 hunks)config/locales/views/trades/en.yml(2 hunks)db/migrate/20251125141213_add_category_to_trades.rb(1 hunks)db/schema.rb(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- config/locales/views/trades/en.yml
🧰 Additional context used
📓 Path-based instructions (26)
**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rb: Application supports two modes: 'managed' and 'self_hosted' via Rails.application.config.app_mode
Use Current.user and Current.family instead of current_user / current_family for authentication context
Optimize database queries with proper indexes to prevent N+1 queries using includes/joins
**/*.rb: Use Rails pluralization in i18n:t("transactions.count", count: @transactions.count).
Runbin/rubocop -f github -afor Ruby linting with auto-correct before pull requests.
**/*.rb: Ruby code should use 2-space indent,snake_casefor methods and variables, andCamelCasefor classes and modules, following Rails conventions for folders and file names
Prefer environment variables over hard-coded values for configuration
Files:
app/controllers/trades_controller.rbapp/models/category.rbdb/migrate/20251125141213_add_category_to_trades.rbapp/models/family.rbapp/models/trade.rbapp/controllers/pages_controller.rbapp/models/income_statement/totals.rbapp/models/trade/create_form.rbapp/models/cashflow_statement/investing_activities.rbapp/models/cashflow_statement/operating_activities.rbapp/models/cashflow_statement.rbapp/models/cashflow_statement/financing_activities.rbapp/models/cashflow_statement/sankey_builder.rbdb/schema.rb
app/controllers/**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Rails strong parameters and CSRF protection throughout the application
app/controllers/**/*.rb: Use strong parameters and CSRF protection throughout the application.
Use query params for state over localStorage/sessions.
Files:
app/controllers/trades_controller.rbapp/controllers/pages_controller.rb
**/*.{rb,erb,html.erb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{rb,erb,html.erb}: UseCurrent.userfor the current user context in controllers and views. Do NOT usecurrent_user.
UseCurrent.familyfor the current family context. Do NOT usecurrent_family.
Files:
app/controllers/trades_controller.rbapp/models/category.rbapp/views/trades/_trade.html.erbdb/migrate/20251125141213_add_category_to_trades.rbapp/views/trades/show.html.erbapp/models/family.rbapp/models/trade.rbapp/controllers/pages_controller.rbapp/models/income_statement/totals.rbapp/models/trade/create_form.rbapp/models/cashflow_statement/investing_activities.rbapp/models/cashflow_statement/operating_activities.rbapp/models/cashflow_statement.rbapp/models/cashflow_statement/financing_activities.rbapp/models/cashflow_statement/sankey_builder.rbdb/schema.rb
**/*.{erb,html.erb,rb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{erb,html.erb,rb}: All user-facing strings must use localization (i18n) via thet()helper. Update locale files for each new or changed element.
Use server-side formatting for currencies, numbers, and dates. Do not format in JavaScript.
Files:
app/controllers/trades_controller.rbapp/models/category.rbapp/views/trades/_trade.html.erbdb/migrate/20251125141213_add_category_to_trades.rbapp/views/trades/show.html.erbapp/models/family.rbapp/models/trade.rbapp/controllers/pages_controller.rbapp/models/income_statement/totals.rbapp/models/trade/create_form.rbapp/models/cashflow_statement/investing_activities.rbapp/models/cashflow_statement/operating_activities.rbapp/models/cashflow_statement.rbapp/models/cashflow_statement/financing_activities.rbapp/models/cashflow_statement/sankey_builder.rbdb/schema.rb
**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
**/*.{rb,erb}: UseCurrent.userfor accessing the current user. Do NOT usecurrent_user
UseCurrent.familyfor accessing the current family. Do NOT usecurrent_family
Files:
app/controllers/trades_controller.rbapp/models/category.rbapp/views/trades/_trade.html.erbdb/migrate/20251125141213_add_category_to_trades.rbapp/views/trades/show.html.erbapp/models/family.rbapp/models/trade.rbapp/controllers/pages_controller.rbapp/models/income_statement/totals.rbapp/models/trade/create_form.rbapp/models/cashflow_statement/investing_activities.rbapp/models/cashflow_statement/operating_activities.rbapp/models/cashflow_statement.rbapp/models/cashflow_statement/financing_activities.rbapp/models/cashflow_statement/sankey_builder.rbdb/schema.rb
**/*.{rb,js,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Format currencies, numbers, dates, and other values server-side, then pass to Stimulus controllers for display only
Files:
app/controllers/trades_controller.rbapp/models/category.rbapp/views/trades/_trade.html.erbdb/migrate/20251125141213_add_category_to_trades.rbapp/views/trades/show.html.erbapp/models/family.rbapp/models/trade.rbapp/controllers/pages_controller.rbapp/models/income_statement/totals.rbapp/models/trade/create_form.rbapp/models/cashflow_statement/investing_activities.rbapp/models/cashflow_statement/operating_activities.rbapp/models/cashflow_statement.rbapp/models/cashflow_statement/financing_activities.rbapp/models/cashflow_statement/sankey_builder.rbdb/schema.rb
**/*.{rb,html.erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Use ActiveRecord validations for complex validations and business logic. Simple validations may be mirrored in ActiveRecord for form error handling convenience, but prioritize client-side form validation when possible
Files:
app/controllers/trades_controller.rbapp/models/category.rbapp/views/trades/_trade.html.erbdb/migrate/20251125141213_add_category_to_trades.rbapp/views/trades/show.html.erbapp/models/family.rbapp/models/trade.rbapp/controllers/pages_controller.rbapp/models/income_statement/totals.rbapp/models/trade/create_form.rbapp/models/cashflow_statement/investing_activities.rbapp/models/cashflow_statement/operating_activities.rbapp/models/cashflow_statement.rbapp/models/cashflow_statement/financing_activities.rbapp/models/cashflow_statement/sankey_builder.rbdb/schema.rb
app/**/*.{rb,js,ts,jsx,tsx,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Code should be organized in
app/directory (Rails MVC, services, jobs, mailers, components), with JS inapp/javascript/, and styles/assets inapp/assets/(Tailwind, images, fonts)
Files:
app/controllers/trades_controller.rbapp/models/category.rbapp/models/family.rbapp/models/trade.rbapp/controllers/pages_controller.rbapp/models/income_statement/totals.rbapp/models/trade/create_form.rbapp/models/cashflow_statement/investing_activities.rbapp/models/cashflow_statement/operating_activities.rbapp/models/cashflow_statement.rbapp/models/cashflow_statement/financing_activities.rbapp/models/cashflow_statement/sankey_builder.rb
app/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Run
bin/brakemansecurity scan before major PRs to check for static analysis of common Rails issues
Files:
app/controllers/trades_controller.rbapp/models/category.rbapp/models/family.rbapp/models/trade.rbapp/controllers/pages_controller.rbapp/models/income_statement/totals.rbapp/models/trade/create_form.rbapp/models/cashflow_statement/investing_activities.rbapp/models/cashflow_statement/operating_activities.rbapp/models/cashflow_statement.rbapp/models/cashflow_statement/financing_activities.rbapp/models/cashflow_statement/sankey_builder.rb
app/models/**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/models/**/*.rb: Keep business logic in models using skinny controllers and fat models pattern
Store all monetary values in base currency (user's primary currency) and use Money objects for conversion
Use ActiveRecord validations for form convenience and complex business logic validations
Use Rails concerns and POROs for organizing business logic instead of services folder
app/models/**/*.rb: All monetary values stored in base currency (user's primary currency). UseMoneyobjects for currency conversion and formatting.
Place business logic inapp/models/folder. Avoid creatingapp/services/folder. Use Rails concerns and POROs for organization.
Models should answer questions about themselves using instance methods: useaccount.balance_seriesnotAccountSeries.new(account).call.
Files:
app/models/category.rbapp/models/family.rbapp/models/trade.rbapp/models/income_statement/totals.rbapp/models/trade/create_form.rbapp/models/cashflow_statement/investing_activities.rbapp/models/cashflow_statement/operating_activities.rbapp/models/cashflow_statement.rbapp/models/cashflow_statement/financing_activities.rbapp/models/cashflow_statement/sankey_builder.rb
{app/models/**/*.rb,db/migrate/**/*.rb}
📄 CodeRabbit inference engine (CLAUDE.md)
Simple validations (null checks, unique indexes) should be in the database. ActiveRecord validations for form convenience (prefer client-side when possible). Complex validations and business logic in ActiveRecord.
Files:
app/models/category.rbdb/migrate/20251125141213_add_category_to_trades.rbapp/models/family.rbapp/models/trade.rbapp/models/income_statement/totals.rbapp/models/trade/create_form.rbapp/models/cashflow_statement/investing_activities.rbapp/models/cashflow_statement/operating_activities.rbapp/models/cashflow_statement.rbapp/models/cashflow_statement/financing_activities.rbapp/models/cashflow_statement/sankey_builder.rb
{app/**/*.{erb,html},app/javascript/**/*.js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Prefer Hotwire-first approach: use Turbo + Stimulus for reactive UI without heavy JavaScript
Files:
app/views/trades/_trade.html.erbapp/views/trades/show.html.erb
app/views/**/_*.html.erb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use partials only for simple, static HTML with minimal logic in specific contexts
app/views/**/_*.html.erb: Use underscore prefix for partial files (e.g., _trend_change.html.erb, _form_errors.html.erb, _sync_indicator.html.erb)
Place context-specific partials in relevant controller view directory (e.g., accounts/_account_sidebar_tabs.html.erb)
Files:
app/views/trades/_trade.html.erb
app/views/**/*.erb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
app/views/**/*.erb: Always use theiconhelper for icons in views, never uselucide_icondirectly
Use Tailwind design tokens (e.g., text-primary, bg-container, border-secondary) instead of raw color names
Pass data to Stimulus controllers via data-*-value attributes instead of inline JavaScript
Prefer semantic HTML elements (dialog, details, summary) over JavaScript components
Use Turbo Frames for page sections instead of client-side solutions
Use server-side formatting for currencies, numbers, and dates instead of client-sideViews should use ERB and avoid heavy logic; prefer helpers and components instead (ERB checked by
erb-lintper.erb_lint.yml)
Files:
app/views/trades/_trade.html.erbapp/views/trades/show.html.erb
**/*.{css,erb}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Tailwind CSS v4.x with custom design system for styling
Files:
app/views/trades/_trade.html.erbapp/views/trades/show.html.erb
**/*.{erb,html,vue,jsx,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{erb,html,vue,jsx,tsx}: Use Tailwind CSS v4.x with custom design system defined inapp/assets/tailwind/maybe-design-system.css. Always use functional tokens (e.g.,text-primarynottext-white).
Prefer semantic HTML elements over JavaScript components (e.g., use<dialog>for modals,<details><summary>for disclosures).
Files:
app/views/trades/_trade.html.erbapp/views/trades/show.html.erb
**/*.{erb,html.erb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{erb,html.erb}: Useiconhelper for icons, never uselucide_icondirectly.
Leverage Turbo frames for page sections over client-side solutions.
Pass data to Stimulus controllers viadata-*-valueattributes, not inline JavaScript.
**/*.{erb,html.erb}: Use ViewComponents when: element has complex logic or styling patterns, will be reused across multiple views/contexts, needs structured styling with variants/sizes, requires interactive behavior or Stimulus controllers, has configurable slots or complex APIs, or needs accessibility features or ARIA support
Use Partials when: element is primarily static HTML with minimal logic, used in only one or few specific contexts, is simple template content, doesn't need variants/sizes/complex configuration, or is more about content organization than reusable functionality
Prefer components over partials: if a component is available in app/components, use it; if not, look for a partial; if neither exists, decide between component or partial based on complexity and reusability criteria
Keep domain logic out of views: compute values like button classes, conditional logic, and data transformations in the component file, not the template file
Always use the declarative approach when integrating Stimulus controllers in views: the ERB template should declare what happens using data-* attributes, and the Stimulus controller should respond
Component controllers in app/components/ should only be used within their component templates; global controllers in app/javascript/controllers/ can be used across any view
Pass data from Rails to Stimulus using data-*-value attributes, not inline JavaScript
Files:
app/views/trades/_trade.html.erbapp/views/trades/show.html.erb
app/views/**/_*.{erb,html.erb}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Partials when element is primarily static HTML with minimal logic, used in only one or few contexts, is simple template content, or doesn't need variants/sizes/complex configuration.
Files:
app/views/trades/_trade.html.erb
{app/components/**/*.html.erb,app/views/**/*.{erb,html.erb}}
📄 CodeRabbit inference engine (CLAUDE.md)
Keep domain logic out of view templates. Logic belongs in component files, not template files.
Files:
app/views/trades/_trade.html.erbapp/views/trades/show.html.erb
**/*.erb
📄 CodeRabbit inference engine (CLAUDE.md)
Run
bundle exec erb_lint ./app/**/*.erb -afor ERB linting with auto-correct before pull requests.
Files:
app/views/trades/_trade.html.erbapp/views/trades/show.html.erb
**/*.html.erb
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
**/*.html.erb: Prefer native HTML elements over JavaScript-based components. Use semantic HTML: for modals,for disclosures
Leverage Turbo frames to break up pages instead of JavaScript-driven client-side solutions
Use theiconhelper from application_helper.rb for icons. Never uselucide_iconhelper directly
Files:
app/views/trades/_trade.html.erbapp/views/trades/show.html.erb
{app/views/**,app/helpers/**,app/javascript/controllers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
{app/views/**,app/helpers/**,app/javascript/controllers/**}: Reference maybe-design-system.css for base primitives, functional tokens, and component tokens before writing styles
Prefer using functional design system tokens (e.g., text-primary, bg-container, border-primary) from maybe-design-system.css instead of raw Tailwind color values
Files:
app/views/trades/_trade.html.erbapp/views/trades/show.html.erb
{app/views/**,app/helpers/**}
📄 CodeRabbit inference engine (.cursor/rules/ui-ux-design-guidelines.mdc)
Always generate semantic HTML
Files:
app/views/trades/_trade.html.erbapp/views/trades/show.html.erb
db/migrate/**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use simple validations (null checks, unique indexes) in database layer
Files:
db/migrate/20251125141213_add_category_to_trades.rb
db/migrate/*.rb
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
ActiveRecord migrations must inherit from
ActiveRecord::Migration[7.2]. Do NOT use version 8.0 yetEnforce null checks, unique indexes, and simple validations in the database schema for PostgreSQL
Files:
db/migrate/20251125141213_add_category_to_trades.rb
db/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Data migrations should be placed in
db/directory with test fixtures intest/fixtures/
Files:
db/migrate/20251125141213_add_category_to_trades.rbdb/schema.rb
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-11-24T16:56:30.657Z
Learning: Entry model is a Rails delegated type with three subtypes: Valuation (absolute account value), Transaction (balance modification), and Trade (buy/sell for investment accounts)
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-11-24T16:56:30.657Z
Learning: Account model is a Rails delegated type with subtypes: Depository, Investment, Crypto, Property, Vehicle, OtherAsset (assets) and CreditCard, Loan, OtherLiability (liabilities)
📚 Learning: 2025-11-14T14:22:37.076Z
Learnt from: jjmata
Repo: we-promise/sure PR: 326
File: app/models/category_import.rb:10-10
Timestamp: 2025-11-14T14:22:37.076Z
Learning: In the CategoryImport model (app/models/category_import.rb), the import process implements a "last write wins" strategy where importing a category.csv file completely overrides existing category structures, including parent relationships. Setting category.parent = nil in the first pass is intentional behavior to ensure the CSV becomes the source of truth.
Applied to files:
app/controllers/trades_controller.rbapp/models/category.rbdb/migrate/20251125141213_add_category_to_trades.rbapp/views/trades/show.html.erbapp/models/trade.rbapp/models/trade/create_form.rbdb/schema.rb
📚 Learning: 2025-11-24T16:56:30.657Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-11-24T16:56:30.657Z
Learning: Account model is a Rails delegated type with subtypes: Depository, Investment, Crypto, Property, Vehicle, OtherAsset (assets) and CreditCard, Loan, OtherLiability (liabilities)
Applied to files:
app/models/category.rbapp/models/cashflow_statement/investing_activities.rbapp/models/cashflow_statement/operating_activities.rbapp/models/cashflow_statement/financing_activities.rb
📚 Learning: 2025-11-24T16:56:30.657Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-design.mdc:0-0
Timestamp: 2025-11-24T16:56:30.657Z
Learning: Entry model is a Rails delegated type with three subtypes: Valuation (absolute account value), Transaction (balance modification), and Trade (buy/sell for investment accounts)
Applied to files:
app/models/category.rbapp/models/trade.rbapp/models/income_statement/totals.rbapp/models/trade/create_form.rbapp/models/cashflow_statement/investing_activities.rb
📚 Learning: 2025-11-24T16:54:59.185Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.185Z
Learning: Domain model structure: User → has many Accounts → has many Transactions; Account types include checking, savings, credit cards, investments, crypto, loans, properties
Applied to files:
app/models/category.rb
📚 Learning: 2025-11-24T16:55:59.131Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/general-rules.mdc:0-0
Timestamp: 2025-11-24T16:55:59.131Z
Learning: Applies to db/migrate/*.rb : ActiveRecord migrations must inherit from `ActiveRecord::Migration[7.2]`. Do NOT use version 8.0 yet
Applied to files:
db/migrate/20251125141213_add_category_to_trades.rbdb/schema.rb
📚 Learning: 2025-11-24T16:55:43.046Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.046Z
Learning: Applies to app/models/**/*.rb : Models should answer questions about themselves using instance methods: use `account.balance_series` not `AccountSeries.new(account).call`.
Applied to files:
app/models/family.rb
📚 Learning: 2025-11-24T16:54:59.185Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.185Z
Learning: Applies to **/*.rb : Optimize database queries with proper indexes to prevent N+1 queries using includes/joins
Applied to files:
app/models/income_statement/totals.rbdb/schema.rb
📚 Learning: 2025-09-13T11:36:45.479Z
Learnt from: alessiocappa
Repo: we-promise/sure PR: 141
File: app/models/enable_banking_account/processor.rb:72-79
Timestamp: 2025-09-13T11:36:45.479Z
Learning: The balance_calculator method pattern is consistent across banking integrations in the codebase. Both PlaidAccount::Processor and EnableBankingAccount::Processor use the same approach: `balance = account.current_balance || account.available_balance || 0` followed by `OpenStruct.new(balance: balance, cash_balance: balance)` without explicit requires, and include identical comments about not distinguishing cash vs non-cash balances for non-investment accounts.
Applied to files:
app/models/cashflow_statement/operating_activities.rb
📚 Learning: 2025-09-13T11:36:45.479Z
Learnt from: alessiocappa
Repo: we-promise/sure PR: 141
File: app/models/enable_banking_account/processor.rb:72-79
Timestamp: 2025-09-13T11:36:45.479Z
Learning: The balance_calculator method in EnableBankingAccount::Processor follows the same pattern as the Plaid integration, using OpenStruct without explicit requires and returning balance values as-is without BigDecimal coercion for consistency across banking integrations.
Applied to files:
app/models/cashflow_statement/operating_activities.rb
📚 Learning: 2025-11-24T16:56:13.394Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-11-24T16:56:13.394Z
Learning: Applies to db/migrate/*.rb : Enforce null checks, unique indexes, and simple validations in the database schema for PostgreSQL
Applied to files:
db/schema.rb
📚 Learning: 2025-11-24T16:56:44.793Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/self_improve.mdc:0-0
Timestamp: 2025-11-24T16:56:44.793Z
Learning: Update rules after major refactors
Applied to files:
db/schema.rb
📚 Learning: 2025-11-24T16:54:59.185Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-24T16:54:59.185Z
Learning: Applies to db/migrate/**/*.rb : Use simple validations (null checks, unique indexes) in database layer
Applied to files:
db/schema.rb
🧬 Code graph analysis (4)
app/controllers/pages_controller.rb (4)
app/models/family.rb (2)
investment_statement(72-74)cashflow_statement(76-78)app/models/cashflow_statement.rb (2)
sankey_data(70-72)operating_activities(13-15)app/models/cashflow_statement/financing_activities.rb (1)
outflows(23-25)app/models/cashflow_statement/operating_activities.rb (2)
outflows(22-24)expenses_by_category(43-45)
app/models/trade/create_form.rb (1)
app/models/income_statement.rb (1)
categories(62-64)
app/models/cashflow_statement/investing_activities.rb (2)
app/models/cashflow_statement/operating_activities.rb (9)
include(1-227)inflows(12-14)totals(61-69)inflows_money(16-18)outflows(22-24)outflows_money(26-28)net(30-32)net_money(34-36)summary(47-55)app/models/period.rb (1)
date_range(133-135)
app/models/cashflow_statement/sankey_builder.rb (2)
app/models/cashflow_statement.rb (6)
operating_activities(13-15)investing_activities(18-20)financing_activities(23-25)total_inflows(37-39)total_outflows(46-48)net_cash_flow(28-30)app/models/cashflow_statement/investing_activities.rb (2)
inflows(12-14)outflows(21-23)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: ci / test
🔇 Additional comments (25)
app/models/income_statement/totals.rb (3)
1-6: LGTM!The initialization cleanly adds the
include_tradesflag with a sensible default that aligns with the investment tracking feature.
23-28: LGTM!Clean conditional routing between query paths with proper SQL parameter sanitization.
30-46: LGTM!Good use of
UNION ALLfor combining distinct data sources. The outer aggregation correctly sums totals across both transactions and trades.app/models/cashflow_statement/financing_activities.rb (2)
1-9: LGTM!The class structure follows the existing Monetizable pattern well, with clean separation of concerns and proper initialization.
83-91: Currency conversion fallback silently masks missing exchange rate records across reporting.The
COALESCE(er.rate, 1)pattern is systematic throughout the codebase (9+ files, 15+ locations). When an exchange rate record doesn't exist for a transaction's currency/date combination, the query defaults to 1:1 conversion. While ExchangeRate::Importer uses a gapfill strategy, there's no validation preventing transactions without exchange rates, and tests don't exercise this fallback path.This means cross-currency amounts could be silently misrepresented if:
- Exchange rates haven't been imported for a transaction's currency
- Historical transactions predate rate collection
- Data population gaps occur despite gapfill logic
Recommend either:
- Verify all required exchange rates are guaranteed before transactions are created (add validation if not)
- Add explicit logging/auditing when COALESCE fallback is used to catch data quality issues
- Filter affected transactions with missing rates from reports or raise warnings
This pattern affects:
cashflow_statement/financing_activities.rb,cashflow_statement/operating_activities.rb,cashflow_statement/investing_activities.rb,investment_statement/totals.rb,income_statement/totals.rb,balance/chart_series_builder.rb, and others.app/models/cashflow_statement/operating_activities.rb (2)
123-183: String interpolation in SQL is safe here, but worth documenting.The
#{direction_condition}interpolation (line 181) is safe sincedirection_conditionis derived from the internal:income/:expenseclassification, not user input. However, a brief comment noting this is intentionally non-parameterized could help future maintainers.The same currency conversion fallback concern applies here as in
FinancingActivities.
196-220: LGTM!The OpenStruct usage for category objects is consistent with existing balance_calculator patterns in the codebase. The Uncategorized fallback properly uses the
Category::UNCATEGORIZED_COLORconstant.app/controllers/trades_controller.rb (1)
54-59: LGTM!The addition of
:category_idto permitted entryable_attributes correctly enables category assignment for trades, aligning with the newTrade.belongs_to :categoryassociation.app/views/trades/_trade.html.erb (1)
1-1: The_trade.html.erbpartial appears to be unused dead code.The partial exists at
app/views/trades/_trade.html.erband does reference an undefinedtrade_categoryvariable on line 33. However, the partial is not rendered from any view in the codebase, so the undefined variable doesn't cause runtime errors. Verify whether this partial is:
- Intentionally unused and should be removed, or
- Planned for future use (in which case, fix the
trade_categoryundefined variable issue)If keeping the partial, add
trade_categoryto the locals declaration:-<%# locals: (entry:, balance_trend: nil, **) %> +<%# locals: (entry:, balance_trend: nil, trade_category: nil, **) %>db/migrate/20251125141213_add_category_to_trades.rb (1)
1-5: LGTM!The migration correctly uses
ActiveRecord::Migration[7.2], UUID type for consistency with the schema, and properly setsnull: trueto match the optional association in the model. The foreign key constraint is enforced at the database level as per best practices.app/models/trade.rb (2)
12-19: LGTM! Minor edge case to consider.The predicate methods are clean and follow Ruby conventions. The
validates :qty, presence: trueon line 9 protects against nil values.Note: If
qtyis exactly zero, bothbuy?andsell?return false. This may be intentional for edge cases like cancelled trades, but verify this behavior is expected.
7-7: LGTM!The optional association correctly mirrors the nullable foreign key in the migration and aligns with the new category-tracking feature for trades.
app/models/family.rb (1)
72-78: LGTM!Both accessors follow the pattern established by
balance_sheetandincome_statement. Theinvestment_statementis correctly memoized since it doesn't take parameters. Thecashflow_statementintentionally isn't memoized since it accepts a period parameter—different calls may need different periods.db/schema.rb (1)
739-752: Unrelated changes:rule_runstable appears outside PR scope.The
rule_runstable addition seems unrelated to the investment tracking feature described in the PR objectives. If this is intentional (perhaps bundled changes), no action needed. Otherwise, consider separating into a distinct PR for cleaner history.app/controllers/pages_controller.rb (3)
19-23: LGTM!The sankey data construction properly uses the unified
CashflowStatementmodel with configurable options for including investing activities. Theinclude_financing: truedefault ensures loan/credit card payments are always visible.
8-8: No action required — @investment_statement is used in the dashboard views.The instance variable is referenced in
app/views/pages/dashboard.html.erbat lines 49 and 52, where it's used to check investment accounts and passed to a component for rendering.
67-68: Now I need to check the actual pages_controller.rb file to see the full context of the code in question:Perfect! I now have all the information needed to complete the verification. The web search results confirm that usingrespond_to?to check if an object responds to a particular method is a standard Ruby practice to check if an object can perform an action, which is central to duck typing. Duck typing in Ruby is less concerned with the class of an object and more concerned with what methods can be called on it.From my earlier analysis of the codebase, the
respond_to?(:parent_id)check at line 67 is correct because thecategory_breakdownmethod intentionally creates synthetic "Uncategorized" category objects that don't have theparent_idattribute.Using
respond_to?is the correct approach to determine if an object responds to certain methods, rather than relying on type checking.The review comment is incorrect.
Respond_to? check is correct defensive programming; no issue to fix.
The
respond_to?(:parent_id)check at line 67 is not indicative of a type inconsistency problem. Duck typing in Ruby is concerned with what methods can be called on objects, and this code correctly uses that pattern.The
category_breakdownmethod intentionally creates synthetic "Uncategorized" category objects (created as hashes with limited attributes) alongside real Category objects (from the database with all attributes includingparent_id). Therespond_to?check ensures the code handles both variants safely, which is proper duck typing implementation.app/models/category.rb (2)
3-3: LGTM on the trades association.The
has_many :tradeswithdependent: :nullifycorrectly mirrors the existinghas_many :transactionspattern and aligns with the Trade model's optional category association mentioned in the PR.
149-151: LGTM — consistent with the constant usage.The method correctly delegates to the
INVESTMENT_CATEGORY_NAMESconstant for determining investment status.app/models/cashflow_statement.rb (3)
1-9: Clean initialization and composition pattern.The class follows Rails PORO conventions with proper attribute readers and sensible defaults. The composition of activity types is well-structured.
54-64: LGTM — Summary composition is clean.The
Data.definepattern provides an immutable, named-field structure that's idiomatic for Ruby 3.2+.
27-52: Review comment is incorrect—all activity methods safely return numeric values.All three activity classes (
OperatingActivities,InvestingActivities,FinancingActivities) guarantee numeric returns via|| 0fallback patterns in theirtotalsmethods:
- OperatingActivities:
inflows/outflowsreturnresult["income"]&.to_d || 0andresult["expense"]&.to_d || 0- InvestingActivities:
inflows/outflowsreturnresult["withdrawals"]&.to_d || 0andresult["contributions"]&.to_d || 0- FinancingActivities:
inflowsis hardcoded0;outflowsreturns sum of two decimals with|| 0fallbackEach
netmethod computesinflows - outflows(numeric arithmetic), ensuring numeric output. No nil values propagate toCashflowStatementaggregations. The code is correct.Likely an incorrect or invalid review comment.
app/models/cashflow_statement/sankey_builder.rb (2)
232-260: LGTM — surplus/deficit logic handles edge cases correctly.The positive/negative/zero cash flow handling is clean, with appropriate percentage calculations relative to inflows (surplus) and outflows (deficit).
84-109: No issues found — interface is properly implemented with correct nil handling.The
operating.income_by_categoryreturnsCategoryTotalobjects with a nestedcategoryattribute (an OpenStruct withid,name,color,lucide_icon) and atotalattribute. Uncategorized transactions are properly handled by creating a default category OpenStruct withid: nil,name: "Uncategorized", and a default color. The code inbuild_operating_inflowscorrectly accesses these attributes and handles the nil case for uncategorized transactions using||fallbacks.app/models/cashflow_statement/investing_activities.rb (1)
52-55: The review concern is incorrect — the filters are aligned.The
visiblescope in Account model (line 21) filters bystatus: [ "draft", "active" ], which is identical to thea.status IN ('draft', 'active')condition in the totals query. Both queries apply the same status filter, so there is no misalignment or risk ofhas_investments?returning true while totals are empty due to scope filtering differences.Likely an incorrect or invalid review comment.
| def totals_query_sql | ||
| trades_scope = family.trades | ||
| .joins(:entry) | ||
| .where(entries: { date: period.date_range }) | ||
|
|
||
| ActiveRecord::Base.sanitize_sql_array([ | ||
| <<~SQL, | ||
| SELECT | ||
| COALESCE(SUM(CASE WHEN t.qty > 0 THEN ABS(ae.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as contributions, | ||
| COALESCE(SUM(CASE WHEN t.qty < 0 THEN ABS(ae.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as withdrawals, | ||
| COUNT(t.id) as trades_count | ||
| FROM (#{trades_scope.to_sql}) t | ||
| JOIN entries ae ON ae.entryable_id = t.id AND ae.entryable_type = 'Trade' | ||
| JOIN accounts a ON a.id = ae.account_id | ||
| LEFT JOIN exchange_rates er ON ( | ||
| er.date = ae.date AND | ||
| er.from_currency = ae.currency AND | ||
| er.to_currency = :target_currency | ||
| ) | ||
| WHERE a.family_id = :family_id | ||
| AND a.status IN ('draft', 'active') | ||
| AND ae.excluded = false | ||
| SQL | ||
| { | ||
| family_id: family.id, | ||
| target_currency: family.currency | ||
| } | ||
| ]) | ||
| end |
There was a problem hiding this comment.
SQL query re-joins entries unnecessarily and may miss currency conversion edge cases.
The trades_scope already joins :entry (line 73), but the raw SQL joins entries again on the subquery result. This is redundant and could cause issues.
Additionally, when er.rate is NULL (missing exchange rate), COALESCE(er.rate, 1) assumes a 1:1 conversion which is incorrect for cross-currency families. This could silently produce wrong totals.
Consider using the entry data from the initial scope:
def totals_query_sql
- trades_scope = family.trades
- .joins(:entry)
- .where(entries: { date: period.date_range })
+ trades_scope = family.trades
+ .joins(entry: :account)
+ .where(entries: { date: period.date_range })
+ .where(accounts: { status: %w[draft active] })
+ .where(entries: { excluded: false })
ActiveRecord::Base.sanitize_sql_array([
<<~SQL,
SELECT
COALESCE(SUM(CASE WHEN t.qty > 0 THEN ABS(ae.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as contributions,
COALESCE(SUM(CASE WHEN t.qty < 0 THEN ABS(ae.amount * COALESCE(er.rate, 1)) ELSE 0 END), 0) as withdrawals,
COUNT(t.id) as trades_count
- FROM (#{trades_scope.to_sql}) t
- JOIN entries ae ON ae.entryable_id = t.id AND ae.entryable_type = 'Trade'
- JOIN accounts a ON a.id = ae.account_id
+ FROM trades t
+ JOIN entries ae ON ae.entryable_id = t.id AND ae.entryable_type = 'Trade'Also consider logging or raising when exchange rates are missing for material amounts.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/models/cashflow_statement/investing_activities.rb around lines 71–99, the
SQL rebuild re-joins entries already joined in trades_scope and uses
COALESCE(er.rate, 1) which silently assumes 1:1 conversion when rates are
missing; fix by (1) including the needed entry columns (date, account_id,
currency, amount, excluded, etc.) in the trades_scope subquery so the outer
query can reference them directly and remove the redundant JOIN entries ae ON
...; (2) stop defaulting missing rates to 1 — either require a matching
exchange_rates row (use INNER JOIN) or keep LEFT JOIN but change expressions to
treat NULL rates explicitly (e.g. CASE to mark amounts with missing rates) and
add an application-level hook to log or raise when non-zero amounts have NULL
rates so missing exchange rates are surfaced instead of silently converted.
| def build | ||
| @nodes = [] | ||
| @links = [] | ||
| @node_indices = {} | ||
|
|
||
| build_cash_flow_node | ||
| build_operating_inflows | ||
| build_investing_inflows if include_investing? | ||
| build_operating_outflows | ||
| build_investing_outflows if include_investing? | ||
| build_financing_outflows if include_financing? | ||
| build_surplus_or_deficit | ||
|
|
||
| { nodes: @nodes, links: @links, currency_symbol: currency_symbol } | ||
| end |
There was a problem hiding this comment.
build method is not idempotent — calling it twice corrupts state.
The build method resets @nodes, @links, and @node_indices each call, but the memoized @total_inflows, @total_outflows, @net_cash_flow, and @cash_flow_idx are not reset. Calling build twice on the same instance will produce incorrect results.
Either document single-use or reset all memoized state:
def build
@nodes = []
@links = []
@node_indices = {}
+ @total_inflows = nil
+ @total_outflows = nil
+ @net_cash_flow = nil
+ @cash_flow_idx = nilOr make the builder a single-use object (instantiate per call).
🤖 Prompt for AI Agents
In app/models/cashflow_statement/sankey_builder.rb around lines 16 to 30, the
build method currently clears @nodes, @links, and @node_indices but leaves
memoized state (@total_inflows, @total_outflows, @net_cash_flow, @cash_flow_idx)
intact which makes repeated calls corrupt the result; fix by resetting all
memoized instance variables at the top of build (clear @total_inflows,
@total_outflows, @net_cash_flow, @cash_flow_idx and any other cached fields used
by helper methods) or alternatively document that the builder is single-use and
change callers to instantiate a new builder per build call.
| def build_investing_inflows | ||
| # Investment liquidations (selling investments = cash inflow) | ||
| withdrawals = investing.inflows.to_f.round(2) | ||
| return unless withdrawals.positive? | ||
|
|
||
| percentage = total_inflows.zero? ? 0 : (withdrawals / total_inflows * 100).round(1) | ||
|
|
||
| idx = add_node( | ||
| "investment_liquidations", | ||
| "Investment Liquidations", | ||
| withdrawals, | ||
| percentage, | ||
| Category::INVESTMENT_COLOR | ||
| ) | ||
|
|
||
| @links << { | ||
| source: idx, | ||
| target: cash_flow_idx, | ||
| value: withdrawals, | ||
| color: Category::INVESTMENT_COLOR, | ||
| percentage: percentage | ||
| } | ||
| end |
There was a problem hiding this comment.
"Investment Liquidations" label is not localized.
The hardcoded string "Investment Liquidations" should use i18n. Per coding guidelines, all user-facing strings must use the t() helper.
This applies to other hardcoded labels as well: "Investment Contributions", "Loan Payments", "Credit Card Payments", "Surplus", "Deficit", "Cash Flow".
app/models/category.rb
Outdated
| [ "Savings & Investments", "#059669", "piggy-bank", "expense" ], | ||
| # Investment-specific categories | ||
| [ "Investment Contributions", "#0d9488", "trending-up", "expense" ], | ||
| [ "Dividends", "#16a34a", "banknote", "income" ], | ||
| [ "Capital Gains", "#15803d", "trophy", "income" ], | ||
| [ "Investment Interest", "#22c55e", "percent", "income" ] |
There was a problem hiding this comment.
🧩 Analysis chain
Missing "trending-up" in icon_codes whitelist.
The "Investment Contributions" category uses "trending-up" as its icon, but this icon is not present in the icon_codes method (lines 61-74). This could cause validation or display issues if icon codes are validated against this list.
🏁 Script executed:
#!/bin/bash
# Check if trending-up is used elsewhere or if icon_codes is used for validation
rg -n "trending-up" --type=rb
rg -n "icon_codes" --type=rb -A3Length of output: 53
🏁 Script executed:
#!/bin/bash
# Retry: Search for trending-up icon usage without explicit type flag
rg -n "trending-up"Length of output: 568
🏁 Script executed:
#!/bin/bash
# Search for icon_codes method definition
rg -n "def icon_codes" -A 20Length of output: 1723
🏁 Script executed:
#!/bin/bash
# Search for icon_codes usage to check if it's used for validation
rg -n "icon_codes" -B2 -A2Length of output: 871
🏁 Script executed:
#!/bin/bash
# Search for validations on lucide_icon field
rg -n "lucide_icon" -B2 -A2Length of output: 14901
🏁 Script executed:
#!/bin/bash
# Search for any validation logic that uses icon_codes
rg -n "validates.*icon" -B2 -A2Length of output: 310
🏁 Script executed:
#!/bin/bash
# Check the complete default_categories array to see all icon usages
rg -n "def default_categories" -A 50Length of output: 3409
Add "trending-up" icon to the icon_codes whitelist.
The "Investment Contributions" category uses the "trending-up" icon (line 120), but it's not included in the icon_codes method (lines 61-75). While there's no validation error, this creates a UI inconsistency: users cannot select "trending-up" when editing categories through the form, even though the default category is created with this icon.
🤖 Prompt for AI Agents
In app/models/category.rb (around lines 118-123; icon_codes method is around
lines 61-75), the default "Investment Contributions" category uses the
"trending-up" icon but that token is missing from the icon_codes whitelist,
causing a UI mismatch; add "trending-up" to the icon_codes array (keeping the
list's format/ordering consistent) so the icon is available in the category
edit/create form and persists correctly for the default category.
| def trades_subquery_sql | ||
| # Get trades for the same family and date range as transactions | ||
| # Only include trades that have a category assigned | ||
| <<~SQL | ||
| SELECT | ||
| c.id as category_id, | ||
| c.parent_id as parent_category_id, | ||
| CASE WHEN ae.amount < 0 THEN 'income' ELSE 'expense' END as classification, | ||
| ABS(SUM(ae.amount * COALESCE(er.rate, 1))) as total, | ||
| COUNT(ae.id) as entry_count | ||
| FROM trades t | ||
| JOIN entries ae ON ae.entryable_id = t.id AND ae.entryable_type = 'Trade' | ||
| JOIN accounts a ON a.id = ae.account_id | ||
| LEFT JOIN categories c ON c.id = t.category_id | ||
| LEFT JOIN exchange_rates er ON ( | ||
| er.date = ae.date AND | ||
| er.from_currency = ae.currency AND | ||
| er.to_currency = :target_currency | ||
| ) | ||
| WHERE a.family_id = :family_id | ||
| AND a.status IN ('draft', 'active') | ||
| AND ae.excluded = false | ||
| AND ae.date BETWEEN :start_date AND :end_date | ||
| AND t.category_id IS NOT NULL | ||
| GROUP BY c.id, c.parent_id, CASE WHEN ae.amount < 0 THEN 'income' ELSE 'expense' END | ||
| SQL | ||
| end |
There was a problem hiding this comment.
Inconsistent account status filtering between trades and transactions.
The trades subquery filters accounts by a.status IN ('draft', 'active') (line 113), but the transactions subquery (lines 71-91) has no account status filter. This means transactions from inactive/archived accounts will be included while trades from those same accounts are excluded, leading to inconsistent totals.
Consider adding the same account join and status filter to transactions_subquery_sql:
FROM (#{@transactions_scope.to_sql}) at
JOIN entries ae ON ae.entryable_id = at.id AND ae.entryable_type = 'Transaction'
+ JOIN accounts a ON a.id = ae.account_id
LEFT JOIN categories c ON c.id = at.category_id
LEFT JOIN exchange_rates er ON (
er.date = ae.date AND
er.from_currency = ae.currency AND
er.to_currency = :target_currency
)
- WHERE at.kind NOT IN ('funds_movement', 'one_time', 'cc_payment')
+ WHERE a.status IN ('draft', 'active')
+ AND at.kind NOT IN ('funds_movement', 'one_time', 'cc_payment')
AND ae.excluded = falseCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/models/income_statement/totals.rb around lines 93-119 (and adjust
transactions_subquery_sql at ~lines 71-91), the trades subquery filters by
account family and status but the transactions subquery does not, causing
inconsistent totals; update transactions_subquery_sql to JOIN accounts a ON a.id
= ae.account_id (or the equivalent account join used in trades), add AND
a.family_id = :family_id and AND a.status IN ('draft', 'active') to the WHERE
clause (keep parameter names and aliases consistent with the trades subquery and
ensure no alias collisions).
| def extract_date_range | ||
| # Try to extract date range from the scope's where clauses | ||
| # This is a heuristic - the transactions_scope should have date filters | ||
| scope_sql = @transactions_scope.to_sql | ||
|
|
||
| # Default to current month if we can't extract dates | ||
| start_date = Date.current.beginning_of_month | ||
| end_date = Date.current.end_of_month | ||
|
|
||
| # Try to find date conditions in the SQL | ||
| # Look for patterns like "date >= '2024-01-01'" or "date BETWEEN" | ||
| if scope_sql =~ /entries.*date.*>=.*'(\d{4}-\d{2}-\d{2})'/i | ||
| start_date = Date.parse($1) rescue start_date | ||
| end | ||
|
|
||
| if scope_sql =~ /entries.*date.*<=.*'(\d{4}-\d{2}-\d{2})'/i | ||
| end_date = Date.parse($1) rescue end_date | ||
| end | ||
|
|
||
| { start_date: start_date, end_date: end_date } | ||
| end |
There was a problem hiding this comment.
Fragile regex-based SQL parsing is error-prone and could silently produce incorrect results.
Parsing SQL strings with regex to extract dates is brittle:
- The regex pattern
entries.*date.*>=is greedy and may match unintended fragments - Parameterized dates (
:start_date) orBETWEENclauses won't be extracted - Silent fallback to current month means incorrect data without any warning
- SQL structure changes (e.g., table aliases, quoting styles) will break extraction
Consider passing the date range explicitly to avoid parsing SQL:
- def initialize(family, transactions_scope:, include_trades: true)
+ def initialize(family, transactions_scope:, include_trades: true, date_range: nil)
@family = family
@transactions_scope = transactions_scope
@include_trades = include_trades
+ @date_range = date_range
end- def extract_date_range
- # Try to extract date range from the scope's where clauses
- # This is a heuristic - the transactions_scope should have date filters
- scope_sql = @transactions_scope.to_sql
-
- # Default to current month if we can't extract dates
- start_date = Date.current.beginning_of_month
- end_date = Date.current.end_of_month
-
- # Try to find date conditions in the SQL
- # Look for patterns like "date >= '2024-01-01'" or "date BETWEEN"
- if scope_sql =~ /entries.*date.*>=.*'(\d{4}-\d{2}-\d{2})'/i
- start_date = Date.parse($1) rescue start_date
- end
-
- if scope_sql =~ /entries.*date.*<=.*'(\d{4}-\d{2}-\d{2})'/i
- end_date = Date.parse($1) rescue end_date
- end
-
- { start_date: start_date, end_date: end_date }
- end
+ def date_range
+ @date_range || {
+ start_date: Date.current.beginning_of_month,
+ end_date: Date.current.end_of_month
+ }
+ endThen update callers to pass the date range they already have when constructing the scope.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/models/income_statement/totals.rb around lines 134–154, the method
extract_date_range currently parses SQL with fragile regexes and silently falls
back to the current month; instead change the interface so the date range is
provided explicitly (e.g., accept start_date and end_date or a date_range option
in the initializer or method), remove the regex-based SQL parsing, and make the
fallback explicit by raising or logging an error if no dates are supplied; then
update all callers to pass the existing date range they already have when
constructing or calling this class so no SQL string parsing is necessary.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/controllers/pages_controller.rb(2 hunks)app/models/family.rb(1 hunks)app/views/pages/dashboard.html.erb(1 hunks)config/locales/views/pages/en.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/views/pages/dashboard.html.erb
- app/models/family.rb
🧰 Additional context used
📓 Path-based instructions (11)
**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rb: Application supports two modes: 'managed' and 'self_hosted' via Rails.application.config.app_mode
Use Current.user and Current.family instead of current_user / current_family for authentication context
Optimize database queries with proper indexes to prevent N+1 queries using includes/joins
**/*.rb: Use Rails pluralization in i18n:t("transactions.count", count: @transactions.count).
Runbin/rubocop -f github -afor Ruby linting with auto-correct before pull requests.
**/*.rb: Ruby code should use 2-space indent,snake_casefor methods and variables, andCamelCasefor classes and modules, following Rails conventions for folders and file names
Prefer environment variables over hard-coded values for configuration
Files:
app/controllers/pages_controller.rb
app/controllers/**/*.rb
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Rails strong parameters and CSRF protection throughout the application
app/controllers/**/*.rb: Use strong parameters and CSRF protection throughout the application.
Use query params for state over localStorage/sessions.
Files:
app/controllers/pages_controller.rb
**/*.{rb,erb,html.erb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{rb,erb,html.erb}: UseCurrent.userfor the current user context in controllers and views. Do NOT usecurrent_user.
UseCurrent.familyfor the current family context. Do NOT usecurrent_family.
Files:
app/controllers/pages_controller.rb
**/*.{erb,html.erb,rb}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{erb,html.erb,rb}: All user-facing strings must use localization (i18n) via thet()helper. Update locale files for each new or changed element.
Use server-side formatting for currencies, numbers, and dates. Do not format in JavaScript.
Files:
app/controllers/pages_controller.rb
**/*.{rb,erb}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
**/*.{rb,erb}: UseCurrent.userfor accessing the current user. Do NOT usecurrent_user
UseCurrent.familyfor accessing the current family. Do NOT usecurrent_family
Files:
app/controllers/pages_controller.rb
**/*.{rb,js,erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Format currencies, numbers, dates, and other values server-side, then pass to Stimulus controllers for display only
Files:
app/controllers/pages_controller.rb
**/*.{rb,html.erb}
📄 CodeRabbit inference engine (.cursor/rules/project-conventions.mdc)
Use ActiveRecord validations for complex validations and business logic. Simple validations may be mirrored in ActiveRecord for form error handling convenience, but prioritize client-side form validation when possible
Files:
app/controllers/pages_controller.rb
app/**/*.{rb,js,ts,jsx,tsx,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Code should be organized in
app/directory (Rails MVC, services, jobs, mailers, components), with JS inapp/javascript/, and styles/assets inapp/assets/(Tailwind, images, fonts)
Files:
app/controllers/pages_controller.rb
app/**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Run
bin/brakemansecurity scan before major PRs to check for static analysis of common Rails issues
Files:
app/controllers/pages_controller.rb
config/locales/**/*.yml
📄 CodeRabbit inference engine (CLAUDE.md)
Use hierarchical i18n keys by feature:
accounts.index.title,transactions.form.amount_label. Uset()helper for all user-facing strings with interpolation for dynamic content.
Files:
config/locales/views/pages/en.yml
config/**/*.{rb,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Configuration files should be placed in
config/directory, with environment examples in.env.local.exampleand.env.test.example
Files:
config/locales/views/pages/en.yml
🧠 Learnings (2)
📚 Learning: 2025-11-24T16:56:13.394Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: .cursor/rules/project-conventions.mdc:0-0
Timestamp: 2025-11-24T16:56:13.394Z
Learning: Applies to **/*.{rb,js,erb} : Format currencies, numbers, dates, and other values server-side, then pass to Stimulus controllers for display only
Applied to files:
app/controllers/pages_controller.rb
📚 Learning: 2025-11-24T16:55:43.046Z
Learnt from: CR
Repo: we-promise/sure PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:55:43.046Z
Learning: Applies to config/locales/**/*.yml : Use hierarchical i18n keys by feature: `accounts.index.title`, `transactions.form.amount_label`. Use `t()` helper for all user-facing strings with interpolation for dynamic content.
Applied to files:
config/locales/views/pages/en.yml
🧬 Code graph analysis (1)
app/controllers/pages_controller.rb (3)
app/models/family.rb (1)
investment_statement(72-74)app/models/investment_statement.rb (1)
total_income(154-156)app/models/investment_statement/totals.rb (1)
call(7-17)
🪛 ast-grep (0.40.0)
app/controllers/pages_controller.rb
[warning] 80-80: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: add_node.call("income_#{ct.category.id}", ct.category.name, val, percentage, color)
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures [REFERENCES]
https://cwe.mitre.org/data/definitions/522.html
(hardcoded-secret-rsa-passphrase-ruby)
[warning] 94-94: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: add_node.call("expense_#{ct.category.id}", ct.category.name, val, percentage, color)
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures [REFERENCES]
https://cwe.mitre.org/data/definitions/522.html
(hardcoded-secret-rsa-passphrase-ruby)
🔇 Additional comments (2)
config/locales/views/pages/en.yml (1)
35-47: Locale keys are well-structured and follow hierarchical conventions.The new
investment_summaryblock adheres to the coding guidelines with proper feature-based hierarchy (en.pages.dashboard.investment_summary.*), consistent naming, and correct interpolation syntax for dynamic content. The keys align with their consumption in the investment_summary view partial.app/controllers/pages_controller.rb (1)
6-18: Dashboard investments wiring looks consistent with new modelsUsing
Current.family.investment_statementandCurrent.family.income_statementhere keeps context-scoped and aligns with the new investment analytics objects. The currency is derived fromCurrent.family.currencyand later converted to a symbol server‑side, which matches the guideline of doing formatting on the server before passing data to the frontend. Based on learnings, this is the right place to hook in the investment statement for the dashboard.
| def build_cashflow_sankey_data(income_totals, expense_totals, currency) | ||
| nodes = [] | ||
| links = [] | ||
| node_indices = {} # Memoize node indices by a unique key: "type_categoryid" | ||
| node_indices = {} | ||
|
|
||
| # Helper to add/find node and return its index | ||
| add_node = ->(unique_key, display_name, value, percentage, color) { | ||
| node_indices[unique_key] ||= begin | ||
| nodes << { name: display_name, value: value.to_f.round(2), percentage: percentage.to_f.round(1), color: color } | ||
| nodes.size - 1 | ||
| end | ||
| } | ||
|
|
||
| total_income_val = income_totals.total.to_f.round(2) | ||
| total_expense_val = expense_totals.total.to_f.round(2) | ||
| total_income = income_totals.total.to_f.round(2) | ||
| total_expense = expense_totals.total.to_f.round(2) | ||
|
|
||
| # --- Create Central Cash Flow Node --- | ||
| cash_flow_idx = add_node.call("cash_flow_node", "Cash Flow", total_income_val, 0, "var(--color-success)") | ||
| # Central Cash Flow node | ||
| cash_flow_idx = add_node.call("cash_flow_node", "Cash Flow", total_income, 100.0, "var(--color-success)") | ||
|
|
||
| # --- Process Income Side (Top-level categories only) --- | ||
| # Income side (top-level categories only) | ||
| income_totals.category_totals.each do |ct| | ||
| # Skip subcategories – only include root income categories | ||
| next if ct.category.parent_id.present? | ||
|
|
||
| val = ct.total.to_f.round(2) | ||
| next if val.zero? | ||
|
|
||
| percentage_of_total_income = total_income_val.zero? ? 0 : (val / total_income_val * 100).round(1) | ||
|
|
||
| node_display_name = ct.category.name | ||
| node_color = ct.category.color.presence || Category::COLORS.sample | ||
|
|
||
| current_cat_idx = add_node.call( | ||
| "income_#{ct.category.id}", | ||
| node_display_name, | ||
| val, | ||
| percentage_of_total_income, | ||
| node_color | ||
| ) | ||
|
|
||
| links << { | ||
| source: current_cat_idx, | ||
| target: cash_flow_idx, | ||
| value: val, | ||
| color: node_color, | ||
| percentage: percentage_of_total_income | ||
| } | ||
| percentage = total_income.zero? ? 0 : (val / total_income * 100).round(1) | ||
| color = ct.category.color.presence || Category::COLORS.sample | ||
|
|
||
| idx = add_node.call("income_#{ct.category.id}", ct.category.name, val, percentage, color) | ||
| links << { source: idx, target: cash_flow_idx, value: val, color: color, percentage: percentage } | ||
| end | ||
|
|
||
| # --- Process Expense Side (Top-level categories only) --- | ||
| # Expense side (top-level categories only) | ||
| expense_totals.category_totals.each do |ct| | ||
| # Skip subcategories – only include root expense categories to keep Sankey shallow | ||
| next if ct.category.parent_id.present? | ||
|
|
||
| val = ct.total.to_f.round(2) | ||
| next if val.zero? | ||
|
|
||
| percentage_of_total_expense = total_expense_val.zero? ? 0 : (val / total_expense_val * 100).round(1) | ||
|
|
||
| node_display_name = ct.category.name | ||
| node_color = ct.category.color.presence || Category::UNCATEGORIZED_COLOR | ||
|
|
||
| current_cat_idx = add_node.call( | ||
| "expense_#{ct.category.id}", | ||
| node_display_name, | ||
| val, | ||
| percentage_of_total_expense, | ||
| node_color | ||
| ) | ||
|
|
||
| links << { | ||
| source: cash_flow_idx, | ||
| target: current_cat_idx, | ||
| value: val, | ||
| color: node_color, | ||
| percentage: percentage_of_total_expense | ||
| } | ||
| end | ||
| percentage = total_expense.zero? ? 0 : (val / total_expense * 100).round(1) | ||
| color = ct.category.color.presence || Category::UNCATEGORIZED_COLOR | ||
|
|
||
| # --- Process Surplus --- | ||
| leftover = (total_income_val - total_expense_val).round(2) | ||
| if leftover.positive? | ||
| percentage_of_total_income_for_surplus = total_income_val.zero? ? 0 : (leftover / total_income_val * 100).round(1) | ||
| surplus_idx = add_node.call("surplus_node", "Surplus", leftover, percentage_of_total_income_for_surplus, "var(--color-success)") | ||
| links << { source: cash_flow_idx, target: surplus_idx, value: leftover, color: "var(--color-success)", percentage: percentage_of_total_income_for_surplus } | ||
| idx = add_node.call("expense_#{ct.category.id}", ct.category.name, val, percentage, color) | ||
| links << { source: cash_flow_idx, target: idx, value: val, color: color, percentage: percentage } | ||
| end | ||
|
|
||
| # Update Cash Flow and Income node percentages (relative to total income) | ||
| if node_indices["cash_flow_node"] | ||
| nodes[node_indices["cash_flow_node"]][:percentage] = 100.0 | ||
| # Surplus/Deficit | ||
| net = (total_income - total_expense).round(2) | ||
| if net.positive? | ||
| percentage = total_income.zero? ? 0 : (net / total_income * 100).round(1) | ||
| idx = add_node.call("surplus_node", "Surplus", net, percentage, "var(--color-success)") | ||
| links << { source: cash_flow_idx, target: idx, value: net, color: "var(--color-success)", percentage: percentage } | ||
| end | ||
| # No primary income node anymore, percentages are on individual income cats relative to total_income_val | ||
|
|
||
| { nodes: nodes, links: links, currency_symbol: Money::Currency.new(currency_symbol).symbol } | ||
| { nodes: nodes, links: links, currency_symbol: Money::Currency.new(currency).symbol } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Sankey helper: add i18n for labels and decide how to represent deficits
Two things to tighten up here:
-
i18n for user‑facing labels
"Cash Flow"and"Surplus"are likely rendered directly in the UI. Per guidelines, all user‑facing strings should go throught(). Consider something like:
- cash_flow_idx = add_node.call("cash_flow_node", "Cash Flow", total_income, 100.0, "var(--color-success)")
- cash_flow_idx = add_node.call(
- "cash_flow_node",
- t("dashboard.cashflow.sankey.cash_flow"),
- total_income,
- 100.0,
- "var(--color-success)"
- )
...
- idx = add_node.call("surplus_node", "Surplus", net, percentage, "var(--color-success)")
- idx = add_node.call(
- "surplus_node",
- t("dashboard.cashflow.sankey.surplus"),
- net,
- percentage,
- "var(--color-success)"
- )
…plus corresponding keys in the locale files. This keeps the chart labels localizable. As per coding guidelines, all new user‑facing strings should be localized.
- Deficit case currently not surfaced
The comment says# Surplus/Deficit, but the implementation only adds a node whennet.positive?. When expenses exceed income, the chart will show more flow out of the central node than into it, with no explicit “Deficit” node. If you want symmetry with the surplus case, you could add a branch fornet.negative?to create a “Deficit” node (also localized) and link it appropriately, or update the comment to reflect that only surplus is visualized.
These are behavioral/UX refinements rather than correctness bugs, but worth addressing now while this helper is fresh.
🧰 Tools
🪛 ast-grep (0.40.0)
[warning] 80-80: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: add_node.call("income_#{ct.category.id}", ct.category.name, val, percentage, color)
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures [REFERENCES]
https://cwe.mitre.org/data/definitions/522.html
(hardcoded-secret-rsa-passphrase-ruby)
[warning] 94-94: Found the use of an hardcoded passphrase for RSA. The passphrase can be easily discovered, and therefore should not be stored in source-code. It is recommended to remove the passphrase from source-code, and use system environment variables or a restricted configuration file.
Context: add_node.call("expense_#{ct.category.id}", ct.category.name, val, percentage, color)
Note: [CWE-798]: Use of Hard-coded Credentials [OWASP A07:2021]: Identification and Authentication Failures [REFERENCES]
https://cwe.mitre.org/data/definitions/522.html
(hardcoded-secret-rsa-passphrase-ruby)
2 participants
1.1 Investment Dashboard Widget
2.1 InvestmentStatement Model (Parallel Tracking)
2.2 Enhanced Category System
2.3 Cashflow View Toggle
1.3 Investment Performance in Reports
Summary by CodeRabbit
New Features
Localization
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.