diff --git a/app/models/account/syncer.rb b/app/models/account/syncer.rb index ab198a958..3b6a4c9b0 100644 --- a/app/models/account/syncer.rb +++ b/app/models/account/syncer.rb @@ -8,7 +8,7 @@ class Account::Syncer def perform_sync(sync) Rails.logger.info("Processing balances (#{account.linked? ? 'reverse' : 'forward'})") import_market_data - materialize_balances + materialize_balances(window_start_date: sync.window_start_date) end def perform_post_sync @@ -16,9 +16,9 @@ class Account::Syncer end private - def materialize_balances + def materialize_balances(window_start_date: nil) strategy = account.linked? ? :reverse : :forward - Balance::Materializer.new(account, strategy: strategy).materialize_balances + Balance::Materializer.new(account, strategy: strategy, window_start_date: window_start_date).materialize_balances end # Syncs all the exchange rates + security prices this account needs to display historical chart data diff --git a/app/models/balance/forward_calculator.rb b/app/models/balance/forward_calculator.rb index af092a2a3..3e9956f60 100644 --- a/app/models/balance/forward_calculator.rb +++ b/app/models/balance/forward_calculator.rb @@ -1,11 +1,22 @@ class Balance::ForwardCalculator < Balance::BaseCalculator + def initialize(account, window_start_date: nil) + super(account) + @window_start_date = window_start_date + @fell_back = nil # unknown until calculate is called + end + + # True only when we are actually running in incremental mode (i.e. window_start_date + # was provided and we successfully found a valid prior balance to seed from). + # + # Must not be called before calculate — @fell_back is nil until resolve_starting_balances runs. + def incremental? + raise "incremental? must not be called before calculate" if @window_start_date.present? && @fell_back.nil? + @window_start_date.present? && @fell_back == false + end + def calculate Rails.logger.tagged("Balance::ForwardCalculator") do - start_cash_balance = derive_cash_balance_on_date_from_total( - total_balance: account.opening_anchor_balance, - date: account.opening_anchor_date - ) - start_non_cash_balance = account.opening_anchor_balance - start_cash_balance + start_cash_balance, start_non_cash_balance = resolve_starting_balances calc_start_date.upto(calc_end_date).map do |date| valuation = sync_cache.get_valuation(date) @@ -52,8 +63,67 @@ class Balance::ForwardCalculator < Balance::BaseCalculator end private + # Returns [start_cash_balance, start_non_cash_balance] for the first iteration. + # + # In incremental mode: load the persisted end-of-day balance for window_start_date - 1 + # from the DB and use that as the seed. Falls back to full recalculation when: + # - No prior balance record exists in the DB, or + # - The prior balance has a non-zero non-cash component (e.g. investment holdings) + # because Holding::Materializer always does a full recalc, which could make the + # persisted non-cash seed stale relative to freshly-computed holding prices. + def resolve_starting_balances + if @window_start_date.present? + if multi_currency_account? + Rails.logger.info("Account has multi-currency entries or is foreign, falling back to full recalculation") + @fell_back = true + return opening_starting_balances + end + + prior = prior_balance + + if prior && (prior.end_non_cash_balance || 0).zero? + Rails.logger.info("Incremental sync from #{@window_start_date}, seeding from persisted balance on #{prior.date}") + @fell_back = false + return [ prior.end_cash_balance, prior.end_non_cash_balance ] + elsif prior + Rails.logger.info("Prior balance has non-cash component, falling back to full recalculation") + else + Rails.logger.info("No persisted balance found for #{@window_start_date - 1}, falling back to full recalculation") + end + + @fell_back = true + end + + opening_starting_balances + end + + # Returns true when the account has entries in currencies other than the + # account currency, or when the account currency differs from the family + # currency. In either case, balance calculations depend on exchange rates + # that may have been missing (fallback_rate: 1) on a prior sync and later + # imported — so we must do a full recalculation to pick them up. + def multi_currency_account? + account.entries.where.not(currency: account.currency).exists? || + account.currency != account.family.currency + end + + def opening_starting_balances + cash = derive_cash_balance_on_date_from_total( + total_balance: account.opening_anchor_balance, + date: account.opening_anchor_date + ) + [ cash, account.opening_anchor_balance - cash ] + end + + # The balance record for the day immediately before the incremental window. + def prior_balance + account.balances + .where(currency: account.currency) + .find_by(date: @window_start_date - 1) + end + def calc_start_date - account.opening_anchor_date + incremental? ? @window_start_date : account.opening_anchor_date end def calc_end_date diff --git a/app/models/balance/materializer.rb b/app/models/balance/materializer.rb index e4715c021..4a2066454 100644 --- a/app/models/balance/materializer.rb +++ b/app/models/balance/materializer.rb @@ -1,10 +1,11 @@ class Balance::Materializer attr_reader :account, :strategy, :security_ids - def initialize(account, strategy:, security_ids: nil) + def initialize(account, strategy:, security_ids: nil, window_start_date: nil) @account = account @strategy = strategy @security_ids = security_ids + @window_start_date = window_start_date end def materialize_balances @@ -74,17 +75,44 @@ class Balance::Materializer def purge_stale_balances sorted_balances = @balances.sort_by(&:date) - oldest_calculated_balance_date = sorted_balances.first&.date - newest_calculated_balance_date = sorted_balances.last&.date - deleted_count = account.balances.delete_by("date < ? OR date > ?", oldest_calculated_balance_date, newest_calculated_balance_date) + + if sorted_balances.empty? + # In incremental forward-sync, even when no balances were calculated for the window + # (e.g. window_start_date is beyond the last entry), purge stale tail records that + # now fall beyond the prior-balance boundary so orphaned future rows are cleaned up. + if strategy == :forward && calculator.incremental? && account.opening_anchor_date <= @window_start_date - 1 + deleted_count = account.balances.delete_by( + "date < ? OR date > ?", + account.opening_anchor_date, + @window_start_date - 1 + ) + Rails.logger.info("Purged #{deleted_count} stale balances") if deleted_count > 0 + end + return + end + + newest_calculated_balance_date = sorted_balances.last.date + + # In incremental forward-sync mode the calculator only recalculates from + # window_start_date onward, so balances before that date are still valid. + # Use opening_anchor_date as the lower purge bound to preserve them. + # We ask the calculator whether it actually ran incrementally — it may have + # fallen back to a full recalculation, in which case we use the normal bound. + oldest_valid_date = if strategy == :forward && calculator.incremental? + account.opening_anchor_date + else + sorted_balances.first.date + end + + deleted_count = account.balances.delete_by("date < ? OR date > ?", oldest_valid_date, newest_calculated_balance_date) Rails.logger.info("Purged #{deleted_count} stale balances") if deleted_count > 0 end def calculator - if strategy == :reverse + @calculator ||= if strategy == :reverse Balance::ReverseCalculator.new(account) else - Balance::ForwardCalculator.new(account) + Balance::ForwardCalculator.new(account, window_start_date: @window_start_date) end end end diff --git a/test/models/balance/forward_calculator_test.rb b/test/models/balance/forward_calculator_test.rb index b2462cb58..3d2c9fa26 100644 --- a/test/models/balance/forward_calculator_test.rb +++ b/test/models/balance/forward_calculator_test.rb @@ -581,6 +581,180 @@ class Balance::ForwardCalculatorTest < ActiveSupport::TestCase ) end + # ------------------------------------------------------------------------------------------------ + # Incremental calculation (window_start_date) + # ------------------------------------------------------------------------------------------------ + + test "incremental sync produces same results as full sync for the recalculated window" do + account = create_account_with_ledger( + account: { type: Depository, currency: "USD" }, + entries: [ + { type: "opening_anchor", date: 5.days.ago.to_date, balance: 20000 }, + { type: "transaction", date: 4.days.ago.to_date, amount: -500 }, # income → 20500 + { type: "transaction", date: 2.days.ago.to_date, amount: 100 } # expense → 20400 + ] + ) + + # Persist full balances via the materializer (same path as production). + Balance::Materializer.new(account, strategy: :forward).materialize_balances + + # Incremental from 3.days.ago: seeds from persisted balance on 4.days.ago (20500). + incremental = Balance::ForwardCalculator.new(account, window_start_date: 3.days.ago.to_date).calculate + + assert_equal [ 3.days.ago.to_date, 2.days.ago.to_date ], incremental.map(&:date).sort + + assert_calculated_ledger_balances( + calculated_data: incremental, + expected_data: [ + { + date: 3.days.ago.to_date, + legacy_balances: { balance: 20500, cash_balance: 20500 }, + balances: { start: 20500, start_cash: 20500, start_non_cash: 0, end_cash: 20500, end_non_cash: 0, end: 20500 }, + flows: 0, + adjustments: 0 + }, + { + date: 2.days.ago.to_date, + legacy_balances: { balance: 20400, cash_balance: 20400 }, + balances: { start: 20500, start_cash: 20500, start_non_cash: 0, end_cash: 20400, end_non_cash: 0, end: 20400 }, + flows: { cash_inflows: 0, cash_outflows: 100 }, + adjustments: 0 + } + ] + ) + end + + test "falls back to full recalculation when prior balance has a non-cash component" do + account = create_account_with_ledger( + account: { type: Depository, currency: "USD" }, + entries: [ + { type: "opening_anchor", date: 3.days.ago.to_date, balance: 20000 }, + { type: "transaction", date: 2.days.ago.to_date, amount: -500 } + ] + ) + + # Persist a prior balance (window_start_date - 1 = 3.days.ago) with a non-zero + # non-cash component. This simulates an investment account where holdings were + # fully recalculated, making the stored non-cash seed potentially stale. + account.balances.create!( + date: 3.days.ago.to_date, + balance: 20000, + cash_balance: 15000, + currency: "USD", + start_cash_balance: 15000, + start_non_cash_balance: 5000, + cash_inflows: 0, cash_outflows: 0, + non_cash_inflows: 0, non_cash_outflows: 0, + net_market_flows: 0, cash_adjustments: 0, non_cash_adjustments: 0, + flows_factor: 1 + ) + + result = Balance::ForwardCalculator.new(account, window_start_date: 2.days.ago.to_date).calculate + + # Fell back: full range from opening_anchor_date, not just the window. + assert_includes result.map(&:date), 3.days.ago.to_date + assert_includes result.map(&:date), 2.days.ago.to_date + end + + test "falls back to full recalculation when no prior balance exists in DB" do + account = create_account_with_ledger( + account: { type: Depository, currency: "USD" }, + entries: [ + { type: "opening_anchor", date: 3.days.ago.to_date, balance: 20000 }, + { type: "transaction", date: 2.days.ago.to_date, amount: -500 } + ] + ) + + # No persisted balances — prior_balance will be nil, so fall back to full sync. + result = Balance::ForwardCalculator.new(account, window_start_date: 2.days.ago.to_date).calculate + + # Full range returned (opening_anchor_date to last entry date). + assert_equal [ 3.days.ago.to_date, 2.days.ago.to_date ], result.map(&:date).sort + + assert_calculated_ledger_balances( + calculated_data: result, + expected_data: [ + { + date: 3.days.ago.to_date, + legacy_balances: { balance: 20000, cash_balance: 20000 }, + balances: { start: 20000, start_cash: 20000, start_non_cash: 0, end_cash: 20000, end_non_cash: 0, end: 20000 }, + flows: 0, + adjustments: 0 + }, + { + date: 2.days.ago.to_date, + legacy_balances: { balance: 20500, cash_balance: 20500 }, + balances: { start: 20000, start_cash: 20000, start_non_cash: 0, end_cash: 20500, end_non_cash: 0, end: 20500 }, + flows: { cash_inflows: 500, cash_outflows: 0 }, + adjustments: 0 + } + ] + ) + end + + test "multi-currency account falls back to full recalc so late exchange rate imports are picked up" do + # Step 1: Create account with a EUR entry but NO exchange rate yet. + # SyncCache will use fallback_rate: 1, so the €500 entry is treated as $500. + account = create_account_with_ledger( + account: { type: Depository, currency: "USD" }, + entries: [ + { type: "opening_anchor", date: 4.days.ago.to_date, balance: 100 }, + { type: "transaction", date: 3.days.ago.to_date, amount: -100 }, + { type: "transaction", date: 2.days.ago.to_date, amount: -500, currency: "EUR" } + ] + ) + + # First full sync — balances computed with fallback rate (1:1 EUR→USD). + Balance::Materializer.new(account, strategy: :forward).materialize_balances + stale_balance = account.balances.find_by(date: 2.days.ago.to_date) + assert stale_balance, "Balance should exist after full sync" + + # Step 2: Exchange rate arrives later (e.g. daily cron imports it). + ExchangeRate.create!(date: 2.days.ago.to_date, from_currency: "EUR", to_currency: "USD", rate: 1.2) + + # Step 3: Next sync requests incremental from today — but the guard should + # force a full recalc because the account has multi-currency entries. + calculator = Balance::ForwardCalculator.new(account, window_start_date: 1.day.ago.to_date) + result = calculator.calculate + + assert_not calculator.incremental?, "Should not be incremental for multi-currency accounts" + + # Full range returned — includes dates before the window. + assert_includes result.map(&:date), 4.days.ago.to_date + + # The EUR entry on 2.days.ago is now converted at 1.2, so the balance + # picks up the corrected rate: opening 100 + $100 txn + €500*1.2 = $800 + # (without the guard, incremental mode would have seeded from the stale + # $700 balance computed with fallback_rate 1, and never corrected it). + corrected = result.find { |b| b.date == 2.days.ago.to_date } + assert corrected + assert_equal 800, corrected.balance, + "Balance should reflect the corrected EUR→USD rate (€500 * 1.2 = $600, not $500)" + end + + test "falls back to full recalculation for foreign accounts (account currency != family currency)" do + account = create_account_with_ledger( + account: { type: Depository, currency: "EUR" }, + entries: [ + { type: "opening_anchor", date: 3.days.ago.to_date, balance: 1000 }, + { type: "transaction", date: 2.days.ago.to_date, amount: -100 } + ] + ) + + # Precondition: account currency must differ from family currency for this test. + assert_not_equal account.currency, account.family.currency, + "Test requires account currency (#{account.currency}) to differ from family currency (#{account.family.currency})" + + # Persist balances via full materializer. + Balance::Materializer.new(account, strategy: :forward).materialize_balances + calculator = Balance::ForwardCalculator.new(account, window_start_date: 2.days.ago.to_date) + result = calculator.calculate + + # Full range returned. + assert_includes result.map(&:date), 3.days.ago.to_date + assert_not calculator.incremental?, "Should not be incremental for foreign currency accounts" + end + private def assert_balances(calculated_data:, expected_balances:) # Sort calculated data by date to ensure consistent ordering diff --git a/test/models/balance/materializer_test.rb b/test/models/balance/materializer_test.rb index 01d347694..472f5fbd5 100644 --- a/test/models/balance/materializer_test.rb +++ b/test/models/balance/materializer_test.rb @@ -61,6 +61,100 @@ class Balance::MaterializerTest < ActiveSupport::TestCase assert_balance_fields_persisted(expected_balances) end + test "incremental sync preserves balances before window_start_date and purges only beyond calc_end_date" do + # Add an opening anchor so opening_anchor_date is well in the past. + @account.entries.create!( + name: "Opening Balance", + date: 10.days.ago.to_date, + amount: 5000, + currency: "USD", + entryable: Valuation.new(kind: "opening_anchor") + ) + + preserved_old = create_balance(account: @account, date: 5.days.ago.to_date, balance: 10000) + preserved_mid = create_balance(account: @account, date: 3.days.ago.to_date, balance: 12000) + stale_future = create_balance(account: @account, date: 5.days.from_now.to_date, balance: 99000) + + # Calculator returns only the window being recalculated (2.days.ago). + recalculated = [ + Balance.new( + date: 2.days.ago.to_date, + balance: 15000, + cash_balance: 15000, + currency: "USD", + start_cash_balance: 12000, + start_non_cash_balance: 0, + cash_inflows: 3000, + cash_outflows: 0, + non_cash_inflows: 0, + non_cash_outflows: 0, + net_market_flows: 0, + cash_adjustments: 0, + non_cash_adjustments: 0, + flows_factor: 1 + ) + ] + + Balance::ForwardCalculator.any_instance.expects(:calculate).returns(recalculated) + Balance::ForwardCalculator.any_instance.stubs(:incremental?).returns(true) + Holding::Materializer.any_instance.expects(:materialize_holdings).returns([]).once + + Balance::Materializer.new(@account, strategy: :forward, window_start_date: 2.days.ago.to_date).materialize_balances + + # Balances before window_start_date must be preserved. + assert_not_nil @account.balances.find_by(id: preserved_old.id), + "Balance at 5.days.ago should be preserved (before window_start_date)" + assert_not_nil @account.balances.find_by(id: preserved_mid.id), + "Balance at 3.days.ago should be preserved (before window_start_date)" + + # Balance after calc_end_date must be purged. + assert_nil @account.balances.find_by(id: stale_future.id), + "Balance at 5.days.from_now should be purged (after calc_end_date)" + + # Recalculated balance must be present. + assert_not_nil @account.balances.find_by(date: 2.days.ago.to_date), + "Recalculated balance for 2.days.ago should be persisted" + end + + test "falls back to full recalculation when window_start_date is given but no prior balance exists" do + @account.entries.create!( + name: "Opening Balance", + date: 5.days.ago.to_date, + amount: 20000, + currency: "USD", + entryable: Valuation.new(kind: "opening_anchor") + ) + @account.entries.create!( + name: "Test transaction", + date: 3.days.ago.to_date, + amount: -1000, + currency: "USD", + entryable: Transaction.new + ) + + # A stale pre-window balance with a wrong value. + # In successful incremental mode this would be preserved as-is; + # in fallback (no prior balance) the full recalc must overwrite it. + wrong_pre_window = create_balance(account: @account, date: 4.days.ago.to_date, balance: 99999) + + # A stale balance before opening_anchor_date — must be purged in both modes. + stale_before_anchor = create_balance(account: @account, date: 8.days.ago.to_date, balance: 99999) + + Holding::Materializer.any_instance.stubs(:materialize_holdings).returns([]) + + # No prior balance exists for window_start_date - 1 (3.days.ago) → calculator falls back to full recalc. + Balance::Materializer.new(@account, strategy: :forward, window_start_date: 2.days.ago.to_date).materialize_balances + + # After fallback the pre-window balance must be recalculated with the correct value, not preserved. + recalculated = @account.balances.find_by(date: wrong_pre_window.date) + assert_not_nil recalculated, "Balance at 4.days.ago should exist after full recalculation" + assert_equal 20000, recalculated.balance, "Balance should reflect full recalculation, not the stale value (99999)" + + # Stale balance before opening_anchor_date should be purged. + assert_nil @account.balances.find_by(id: stale_before_anchor.id), + "Balance before opening_anchor_date should be purged" + end + test "purges stale balances outside calculated range" do # Create existing balances that will be stale stale_old = create_balance(account: @account, date: 5.days.ago.to_date, balance: 5000)