diff --git a/app/models/balance/base_calculator.rb b/app/models/balance/base_calculator.rb index c8b0905479..0e4b5d24a8 100644 --- a/app/models/balance/base_calculator.rb +++ b/app/models/balance/base_calculator.rb @@ -9,6 +9,32 @@ def calculate raise NotImplementedError, "Subclasses must implement this method" end + # The earliest date balances should be materialized for. + # + # Normally this is the opening anchor date, which our system keeps at or + # before the oldest entry. But an entry (most commonly a backfilled + # reconciliation Valuation) can be created with a date EARLIER than the + # opening anchor — the reconciliation path does not move the anchor back. + # When that happens the anchor date alone would clip all pre-anchor entries + # out of the balance series (they'd be stored but never materialized into + # Balance rows, leaving the net-worth chart empty before the anchor). + # + # Bounding on min(opening_anchor_date, oldest_entry_date) ensures those + # earlier entries are included. The opening anchor and any reconciliation + # still reset the absolute balance on their own dates via the + # valuation-override path, so extending the window backward is safe. + # + # Public so Balance::Materializer can use the same lower bound when deciding + # which balances to preserve during an incremental purge. + # + # Memoized: this is read multiple times per sync (calc bound + both purge + # branches) and the underlying MIN(date) is a non-trivial scan on accounts + # with large entry histories. Calculator instances are per-sync, so there is + # no staleness concern. + def calculation_start_date + @calculation_start_date ||= [ account.opening_anchor_date, account.entries.minimum(:date) ].compact.min + end + private def sync_cache @sync_cache ||= Balance::SyncCache.new(account) diff --git a/app/models/balance/forward_calculator.rb b/app/models/balance/forward_calculator.rb index 3e9956f605..29163b7c6e 100644 --- a/app/models/balance/forward_calculator.rb +++ b/app/models/balance/forward_calculator.rb @@ -108,6 +108,14 @@ def multi_currency_account? end def opening_starting_balances + # When the window starts before the opening anchor (a reconciliation or + # other entry was backfilled with a date earlier than the anchor), we have + # no known balance for that earlier start date. Seed flat at zero — the + # earliest reconciliation and the opening anchor each reset the absolute + # balance on their own dates via the valuation-override path, so the seed + # only affects the pre-anchor opening day's adjustment, not later totals. + return [ 0, 0 ] if calculation_start_date < account.opening_anchor_date + cash = derive_cash_balance_on_date_from_total( total_balance: account.opening_anchor_balance, date: account.opening_anchor_date @@ -123,7 +131,7 @@ def prior_balance end def calc_start_date - incremental? ? @window_start_date : account.opening_anchor_date + incremental? ? @window_start_date : calculation_start_date end def calc_end_date diff --git a/app/models/balance/materializer.rb b/app/models/balance/materializer.rb index 4a2066454c..59df779e68 100644 --- a/app/models/balance/materializer.rb +++ b/app/models/balance/materializer.rb @@ -80,10 +80,10 @@ def purge_stale_balances # 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 + if strategy == :forward && calculator.incremental? && calculator.calculation_start_date <= @window_start_date - 1 deleted_count = account.balances.delete_by( "date < ? OR date > ?", - account.opening_anchor_date, + calculator.calculation_start_date, @window_start_date - 1 ) Rails.logger.info("Purged #{deleted_count} stale balances") if deleted_count > 0 @@ -95,11 +95,13 @@ def purge_stale_balances # 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. + # Use calculation_start_date as the lower purge bound to preserve them — + # this is the same lower bound the calculator uses, so pre-anchor balances + # (from entries dated before the opening anchor) are not deleted. # 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 + calculator.calculation_start_date else sorted_balances.first.date end diff --git a/app/models/balance/reverse_calculator.rb b/app/models/balance/reverse_calculator.rb index ba970fdcda..d073beda74 100644 --- a/app/models/balance/reverse_calculator.rb +++ b/app/models/balance/reverse_calculator.rb @@ -9,8 +9,13 @@ def calculate ) end_non_cash_balance = account.current_anchor_balance - end_cash_balance - # Calculates in reverse-chronological order (End of day -> Start of day) - account.current_anchor_date.downto(account.opening_anchor_date).map do |date| + # Calculates in reverse-chronological order (End of day -> Start of day). + # Bound on calculation_start_date (not opening_anchor_date) so entries + # backfilled with a date earlier than the opening anchor are still + # materialized. Reconciliation waypoints below the anchor reset the + # balance on their own dates; use_opening_anchor_for_date? still keys off + # the anchor's real date, so the anchor's own treatment is unchanged. + account.current_anchor_date.downto(calculation_start_date).map do |date| flows = flows_for_date(date) valuation = sync_cache.get_valuation(date) diff --git a/test/models/balance/forward_calculator_test.rb b/test/models/balance/forward_calculator_test.rb index 22c8a5a616..b4f30c7c5b 100644 --- a/test/models/balance/forward_calculator_test.rb +++ b/test/models/balance/forward_calculator_test.rb @@ -757,6 +757,72 @@ class Balance::ForwardCalculatorTest < ActiveSupport::TestCase assert_not calculator.incremental?, "Should not be incremental for foreign currency accounts" end + # Regression: a reconciliation (or any entry) backfilled with a date EARLIER + # than the opening anchor must still be materialized into Balance rows. The + # window is bounded on min(opening_anchor_date, oldest_entry_date), and the + # pre-anchor start seeds from zero — each valuation resets the absolute + # balance on its own date. + test "materializes entries dated before the opening anchor" do + account = create_account_with_ledger( + account: { type: Depository, currency: "USD" }, + entries: [ + { type: "reconciliation", date: 5.days.ago.to_date, balance: 10000 }, # before anchor + { type: "opening_anchor", date: 3.days.ago.to_date, balance: 17000 }, + { type: "reconciliation", date: 1.day.ago.to_date, balance: 20000 } + ] + ) + + calculated = Balance::ForwardCalculator.new(account).calculate + + # Window extends back to the earliest entry, not the (later) opening anchor. + assert_equal 5.days.ago.to_date, calculated.map(&:date).min + + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_data: [ + { + # Pre-anchor reconciliation: seeded from 0, valuation sets the balance. + date: 5.days.ago.to_date, + legacy_balances: { balance: 10000, cash_balance: 10000 }, + balances: { start: 0, start_cash: 0, start_non_cash: 0, end_cash: 10000, end_non_cash: 0, end: 10000 }, + flows: 0, + adjustments: { cash_adjustments: 10000, non_cash_adjustments: 0 } + }, + { + # Carries forward on a day with no entries. + date: 4.days.ago.to_date, + legacy_balances: { balance: 10000, cash_balance: 10000 }, + balances: { start: 10000, start_cash: 10000, start_non_cash: 0, end_cash: 10000, end_non_cash: 0, end: 10000 }, + flows: 0, + adjustments: 0 + }, + { + # Opening anchor resets the balance on its own date. + date: 3.days.ago.to_date, + legacy_balances: { balance: 17000, cash_balance: 17000 }, + balances: { start: 10000, start_cash: 10000, start_non_cash: 0, end_cash: 17000, end_non_cash: 0, end: 17000 }, + flows: 0, + adjustments: { cash_adjustments: 7000, non_cash_adjustments: 0 } + }, + { + # Carries forward on a day with no entries. + date: 2.days.ago.to_date, + legacy_balances: { balance: 17000, cash_balance: 17000 }, + balances: { start: 17000, start_cash: 17000, start_non_cash: 0, end_cash: 17000, end_non_cash: 0, end: 17000 }, + flows: 0, + adjustments: 0 + }, + { + date: 1.day.ago.to_date, + legacy_balances: { balance: 20000, cash_balance: 20000 }, + balances: { start: 17000, start_cash: 17000, start_non_cash: 0, end_cash: 20000, end_non_cash: 0, end: 20000 }, + flows: 0, + adjustments: { cash_adjustments: 3000, non_cash_adjustments: 0 } + } + ] + ) + 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 472f5fbd51..6ee91d99db 100644 --- a/test/models/balance/materializer_test.rb +++ b/test/models/balance/materializer_test.rb @@ -116,6 +116,46 @@ class Balance::MaterializerTest < ActiveSupport::TestCase "Recalculated balance for 2.days.ago should be persisted" end + # Regression: a prior full sync materializes balances for entries dated BEFORE + # the opening anchor; a later incremental sync (window after the anchor) must + # preserve them. The purge lower bound uses calculation_start_date — the same + # bound the calculator uses — not opening_anchor_date. + test "incremental sync preserves balances dated before the opening anchor" do + # Opening anchor at 5.days.ago, but a reconciliation backfilled earlier. + @account.entries.create!( + name: "Opening Balance", date: 5.days.ago.to_date, amount: 5000, + currency: "USD", entryable: Valuation.new(kind: "opening_anchor") + ) + @account.entries.create!( + name: "Backfilled reconciliation", date: 8.days.ago.to_date, amount: 3000, + currency: "USD", entryable: Valuation.new(kind: "reconciliation") + ) + + # Pre-anchor balance materialized by a prior full sync. + pre_anchor = create_balance(account: @account, date: 8.days.ago.to_date, balance: 3000) + preserved_mid = create_balance(account: @account, date: 6.days.ago.to_date, balance: 4000) + + 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 + + assert_not_nil @account.balances.find_by(id: pre_anchor.id), + "Balance before the opening anchor must be preserved during incremental purge" + assert_not_nil @account.balances.find_by(id: preserved_mid.id), + "Balance between anchor and window must be preserved" + 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", diff --git a/test/models/balance/reverse_calculator_test.rb b/test/models/balance/reverse_calculator_test.rb index 11f4463e36..070e1f0ff0 100644 --- a/test/models/balance/reverse_calculator_test.rb +++ b/test/models/balance/reverse_calculator_test.rb @@ -686,4 +686,65 @@ class Balance::ReverseCalculatorTest < ActiveSupport::TestCase ] ) end + + # Regression: a reconciliation waypoint dated EARLIER than the opening anchor + # must still be materialized. The reverse loop is bounded on + # min(opening_anchor_date, oldest_entry_date), so it continues past the anchor + # down to the earliest entry, which resets the balance on its own date. + test "materializes reconciliation waypoints dated before the opening anchor" do + account = create_account_with_ledger( + account: { type: Depository, balance: 20000, cash_balance: 20000, currency: "USD" }, + entries: [ + { type: "current_anchor", date: Date.current, balance: 20000 }, + { type: "opening_anchor", date: 2.days.ago, balance: 15000 }, + { type: "reconciliation", date: 4.days.ago, balance: 9000 } # before the opening anchor + ] + ) + + calculated = Balance::ReverseCalculator.new(account).calculate + + # Loop extends back to the earliest entry, not the (later) opening anchor. + assert_equal 4.days.ago.to_date, calculated.map(&:date).min + + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_data: [ + { + date: Date.current, + 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 + }, # Current anchor + { + date: 1.day.ago, + 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 + }, # Gap above the anchor carries down with no flows + { + date: 2.days.ago, + legacy_balances: { balance: 15000, cash_balance: 15000 }, + balances: { start: 15000, start_cash: 15000, start_non_cash: 0, end_cash: 15000, end_non_cash: 0, end: 15000 }, + flows: 0, + adjustments: 0 + }, # Opening anchor + { + date: 3.days.ago, + legacy_balances: { balance: 15000, cash_balance: 15000 }, + balances: { start: 15000, start_cash: 15000, start_non_cash: 0, end_cash: 15000, end_non_cash: 0, end: 15000 }, + flows: 0, + adjustments: 0 + }, # Gap below the anchor carries down with no flows + { + date: 4.days.ago, + legacy_balances: { balance: 9000, cash_balance: 9000 }, + balances: { start: 9000, start_cash: 9000, start_non_cash: 0, end_cash: 9000, end_non_cash: 0, end: 9000 }, + flows: 0, + adjustments: 0 + } # Pre-anchor reconciliation resets to its own value + ] + ) + end end