Exclude pending transactions from balances

[atlas-maxjb]

Summary

• exclude provider-marked pending transaction entries from balance materialization
• ignore pending entries when determining calculation and linked-investment analytics history boundaries
• retain posted transactions, trades, valuations, and other non-transaction entries

Tests

• Added focused regressions for balance sync caching, forward balance calculation, and linked-investment series history
• git diff --check (pass)
• Rails tests could not run locally because the workspace does not include the required Ruby 3.4.9 executable; CI will run the suite

Summary by CodeRabbit

• Bug Fixes

  • Pending transactions are now excluded from balance calculations, cash inflows, investment history dates, and converted entry results.
  • Balance periods and currency detection now reflect posted activity only.
  • Posted transactions and eligible trade entries continue to be included correctly.

• Tests

  • Added regression coverage for pending transaction handling across balances, investment histories, and cached entries.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1a96e551-19bf-45bf-9a36-72c93948d359

📥 Commits

Reviewing files that changed from the base of the PR and between 659b3d4 and 48c336d.

📒 Files selected for processing (1)

• test/models/balance/forward_calculator_test.rb

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

• test/models/balance/forward_calculator_test.rb

---

📝 Walkthrough

Walkthrough

Pending entries are excluded from balance date and currency calculations, investment history start dates, and converted-entry caching. Tests verify that posted transactions and trade entries remain included.

Changes

Pending balance entry handling

Layer / File(s)	Summary
Pending-entry balance calculations app/models/balance/base_calculator.rb, app/models/balance/forward_calculator.rb, app/models/balance/linked_investment_series_normalizer.rb, test/models/balance/forward_calculator_test.rb, test/models/balance/linked_investment_series_normalizer_test.rb	Balance calculators and investment history normalization exclude pending entries from date and currency calculations. Tests cover calculation windows, balances, cash inflows, foreign-currency entries, and history start dates.
Pending-entry cache filtering app/models/balance/sync_cache.rb, test/models/balance/sync_cache_test.rb	Converted-entry caching excludes pending transactions while retaining posted transactions and trade entries.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• we-promise/sure#1709: Handles pending transaction state during balance processing and duplicate merging.
• we-promise/sure#1937: Modifies balance and transaction handling for pending entries.
• we-promise/sure#2434: Updates calculation_start_date and related balance date boundaries.

Suggested reviewers: jjmata

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the primary change: excluding pending transactions from balance-related calculations.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: af426047b8

ℹ️ 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 investment contributions in reports breakdown

With the default preference (treat_investment_contributions_as_transfers? false), IncomeStatement now keeps the matched brokerage outflow via cash_flow_transfer_sql, so the report summary totals include investment contributions. This scope unconditionally uses without_matched_transfer, and the export helpers copy the same filter, so a confirmed transfer to an investment account is dropped from the transactions breakdown/CSV while the expense total still includes it; use the cash-flow transfer filter unless the user has opted out.

Useful? React with 👍 / 👎.

[jjmata]

Overview

This PR extends the existing Entry.excluding_pending / Transaction.excluding_pending scopes (already used by income_statement.rb, goal.rb, family.rb, transaction/search.rb) into the balance materialization pipeline, so provider-marked pending transactions no longer contribute to materialized Balance rows, multi-currency detection, calc boundaries, or linked-investment history trimming.

Files touched:

• Balance::BaseCalculator#calculation_start_date — bounds the earliest materialized date on posted entries only
• Balance::ForwardCalculator#multi_currency_account? / #calc_end_date — ignores pending entries for the full-recalc trigger and the end-of-window date
• Balance::LinkedInvestmentSeriesNormalizer — trims linked-investment history start dates using posted activity only
• Balance::SyncCache#converted_entries — the central per-date entry cache used by both ForwardCalculator and ReverseCalculator (via flows_for_date/get_entries) now excludes pending entries

Code quality / correctness

• Good central fix: routing the actual flow/balance math through Balance::SyncCache#converted_entries means both Balance::ForwardCalculator and Balance::ReverseCalculator pick up the exclusion "for free" via sync_cache.get_entries/get_valuation, since both inherit flows_for_date/derive_cash_balance/derive_non_cash_balance from Balance::BaseCalculator. The remaining call sites (calculation_start_date, multi_currency_account?, calc_end_date, the linked-investment normalizer) are boundary-detection queries that hit account.entries directly and correctly needed the same fix independently — nice catch that these weren't just relying on the cache.
• Reuses existing provider-agnostic infra: no new pending-detection logic was introduced; it all flows through Transaction::PENDING_PROVIDERS/PENDING_CHECK_SQL, which already covers simplefin/plaid/lunchflow/enable_banking/akahu/up/mercury/redbark. This keeps balance exclusion consistent with the Transaction#pending? UI badge and avoids the risk of drift between "what counts as pending" in different places.
• entryable_type != 'Transaction' guard preserved: excluding_pending (Entry-level) correctly leaves Trades, Valuations, etc. untouched — confirmed via the new sync_cache_test.rb case that a Trade entry survives the filter.
• No N+1 / correctness risk: all changed call sites are single aggregate/EXISTS SQL queries (.minimum, .maximum, .exists?, or a subquery-filtered WHERE), not per-record loads, so this doesn't introduce N+1s. The calc_end_date switch from .order(:date).last&.date to .maximum(:date) (and the same for holdings) is a nice incidental efficiency win (aggregate instead of full-row fetch + sort), though it's an unrelated optimization riding along with the pending-exclusion fix on the same line — worth calling out separately in the commit message/description for future git-blame clarity, but not blocking.
• Convention adherence looks solid: logic lives in app/models/balance/* (no new service objects), no current_user/current_family usage needed here (no controller changes), no new dependencies, and no user-facing strings requiring i18n updates.

Test coverage

Tests are minimal, focused, and use Minitest + the existing create_account_with_ledger/fixture helpers (no FactoryBot/RSpec), consistent with repo conventions:

• forward_calculator_test.rb: verifies a pending transaction is excluded from both balance and cash_inflows, and its date doesn't appear in the output series.
• linked_investment_series_normalizer_test.rb (new): verifies common_supported_history_start_date skips a pending Plaid transaction in favor of the posted one.
• sync_cache_test.rb: verifies converted_entries drops a pending transaction while keeping a posted transaction and a Trade.

Two gaps worth considering (non-blocking, but would tighten coverage):

1. No direct test for Balance::BaseCalculator#calculation_start_date in the scenario where the oldest entry is pending (i.e., confirming it correctly falls back to opening_anchor_date/next posted entry rather than the pending entry's date).
2. No test for ForwardCalculator#multi_currency_account? confirming a pending foreign-currency entry no longer forces a full recalculation on an otherwise single-currency account — this is a meaningful behavior change (avoids unnecessary full recalcs) that isn't directly asserted.

Risks / things to double check before merge

• PR description notes tests couldn't be run locally (missing Ruby 3.4.9); please confirm CI is green, especially given Balance::Materializer#purge_stale_balances depends on calculation_start_date/calc_end_date for its purge bounds — worth watching CI for any interaction with balance purging on accounts that have only pending entries near the boundary.
• Balance::ReverseCalculator wasn't directly touched or tested, but appears correctly covered transitively through Balance::BaseCalculator + Balance::SyncCache. Worth a quick manual sanity check (or a follow-up test) since reverse sync is a distinct code path from forward.

Overall this is a well-scoped, correctly-targeted fix that reuses existing provider-agnostic pending infrastructure rather than reinventing it, and the tests exercise the actual regression being fixed. Nice work.

---

Generated by Claude Code