fix(reports): eliminate N+1 query in trends data calculation

[sentry]

This PR re-applies the fix for the N+1 query issue in ReportsController#index.

The build_trends_data method previously iterated over each month, making individual calls to income_statement.income_totals and income_statement.expense_totals, resulting in multiple database queries (N+1 problem).

This change introduces a new method, monthly_totals_for_range, in IncomeStatement that fetches all monthly income and expense totals within a given date range using a single, grouped SQL query. The build_trends_data method has been refactored to utilize this new bulk fetching method, thereby resolving the N+1 query performance issue.

This addresses a regression caused by the previous reversion of PR #2493.

Fixes SURE-APP-DP

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e487701272

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Keep final trend totals within the selected period

When @end_date falls before the end of its month, as it does for custom and default YTD ranges, this upper bound includes every transaction through month end. The removed loop explicitly capped the final monthly period at @end_date, so a historical custom range now counts later transactions outside the selection, while current YTD can count future-dated entries, making the trend disagree with the summary and transaction breakdown. Pass @end_date as the query's upper bound while continuing to group the returned rows by month.

Useful? React with 👍 / 👎.

[jjmata]

The N+1 problem being fixed here is real (per-month calls to income_totals/expense_totals inside a loop), but the fix reimplements the underlying financial-totals logic as a hand-written raw SQL string in monthly_totals_for_range rather than reusing/parameterizing the existing income_totals/expense_totals code path, and I'd like to flag two things before this merges:

1. No test coverage added. The diff touches app/models/income_statement.rb and app/controllers/reports_controller.rb but adds no spec/test exercising monthly_totals_for_range or the trends endpoint. Given this duplicates business logic that already has an established, presumably-tested implementation elsewhere, a regression here (e.g. a currency, kind-classification, or exclusion rule that drifts out of sync between the two) would be silent. Given repo convention ("write tests as you go, when required") and that this is financial-totals logic, I'd expect at least one test comparing monthly_totals_for_range output against the existing income_totals/expense_totals per-month for a representative period.

2. Possible behavioral drift in the reimplementation, worth double-checking against the original methods:

   • The FX join uses LEFT JOIN exchange_rates ... COALESCE(er.rate, 1) — an exact-date match falling back to a rate of 1 when missing. If income_totals/expense_totals use a different fallback strategy (e.g. nearest available rate) for missing exchange rate rows, multi-currency totals could differ between the Trends chart and the rest of the Reports page.
   • The income/expense classification (at.kind IN ('investment_contribution','loan_payment') THEN 'expense', else sign-based) is duplicated here rather than delegated to whatever Income::Totals/Expense::Totals computes — any future change to that classification would need to be mirrored manually in this SQL string or the two will silently diverge.

Not blocking necessarily if this has already been validated against production data in the sentry issue, but I'd want to see the numeric parity confirmed with a test before merging, since this touches money totals shown to users.

---

Generated by Claude Code