diff --git a/app/models/account/current_balance_manager.rb b/app/models/account/current_balance_manager.rb index f1c5928a5..b01c2f26a 100644 --- a/app/models/account/current_balance_manager.rb +++ b/app/models/account/current_balance_manager.rb @@ -92,22 +92,55 @@ def adjust_opening_balance_with_delta(new_balance:, old_balance:) # Linked accounts manage "current balance" via the special `current_anchor` valuation. # This is NOT a user-facing feature, and is primarily used in "processors" while syncing # linked account data (e.g. via Plaid) + # + # Before overwriting a stale (previous-day) current_anchor, we convert it to a + # reconciliation valuation. This preserves the API-reported balance as a historical + # waypoint that the ReverseCalculator uses for more accurate balance history. def set_current_balance_for_linked_account(balance) - if current_anchor_valuation - changes_made = update_current_anchor(balance) - Result.new(success?: true, changes_made?: changes_made, error: nil) - else - create_current_anchor(balance) - Result.new(success?: true, changes_made?: true, error: nil) + changes_made = false + + ActiveRecord::Base.transaction do + # If an anchor exists from a previous day, preserve it as a reconciliation + # before replacing it with today's fresh anchor. + preserve_anchor_as_reconciliation_if_stale if current_anchor_valuation + + # Re-check: the memoized value was cleared if the anchor was converted + if current_anchor_valuation + changes_made = update_current_anchor(balance) + else + create_current_anchor(balance) + changes_made = true + end end + + Result.new(success?: true, changes_made?: changes_made, error: nil) end def current_anchor_valuation @current_anchor_valuation ||= account.valuations.current_anchor.includes(:entry).first end + # If the existing current_anchor is from a previous day, convert it to a + # reconciliation before overwriting. This accumulates a chain of API-reported + # balance waypoints over time without creating extra entries per sync. + # + # Same-day updates are left in place (no extra reconciliations on repeated syncs). + def preserve_anchor_as_reconciliation_if_stale + entry = current_anchor_valuation.entry + return if entry.date == Date.current # Same-day update — nothing to preserve + + current_anchor_valuation.update!(kind: "reconciliation") + entry.update!(name: Valuation.build_reconciliation_name(account.accountable_type)) + Rails.logger.info("[AnchorRotation] Converted current_anchor to reconciliation for account #{account.id}, date=#{entry.date}, entry_id=#{entry.id}") + + # Clear memoized value so the next check creates a fresh current_anchor. + # The chained scope (.current_anchor.first) always issues a fresh SQL query, + # so we don't need to reload the full association. + @current_anchor_valuation = nil + end + def create_current_anchor(balance) - entry = account.entries.create!( + account.entries.create!( date: Date.current, name: Valuation.build_current_anchor_name(account.accountable_type), amount: balance, @@ -115,31 +148,28 @@ def create_current_anchor(balance) entryable: Valuation.new(kind: "current_anchor") ) - # Reload associations and clear memoized value so it gets the new anchor - account.valuations.reload + # Clear memoized value so it picks up the new anchor on next access. @current_anchor_valuation = nil end def update_current_anchor(balance) changes_made = false - ActiveRecord::Base.transaction do - # Update associated entry attributes - entry = current_anchor_valuation.entry - - if entry.amount != balance - entry.amount = balance - changes_made = true - end + # Update associated entry attributes + entry = current_anchor_valuation.entry - if entry.date != Date.current - entry.date = Date.current - changes_made = true - end + if entry.amount != balance + entry.amount = balance + changes_made = true + end - entry.save! if entry.changed? + if entry.date != Date.current + entry.date = Date.current + changes_made = true end + entry.save! if entry.changed? + changes_made end end diff --git a/app/models/balance/reverse_calculator.rb b/app/models/balance/reverse_calculator.rb index 35a774452..39b530178 100644 --- a/app/models/balance/reverse_calculator.rb +++ b/app/models/balance/reverse_calculator.rb @@ -12,6 +12,7 @@ def calculate # Calculates in reverse-chronological order (End of day -> Start of day) account.current_anchor_date.downto(account.opening_anchor_date).map do |date| flows = flows_for_date(date) + valuation = sync_cache.get_valuation(date) if use_opening_anchor_for_date?(date) end_cash_balance = derive_cash_balance_on_date_from_total( @@ -20,6 +21,21 @@ def calculate ) end_non_cash_balance = account.opening_anchor_balance - end_cash_balance + start_cash_balance = end_cash_balance + start_non_cash_balance = end_non_cash_balance + market_value_change = 0 + elsif valuation && valuation.entryable.reconciliation? + # Reconciliation waypoint: reset to the known API-reported balance. + # These waypoints are created by CurrentBalanceManager when it preserves + # a stale current_anchor as a reconciliation before replacing it. + # We derive both cash and non-cash from the total to ensure the split + # reflects the account's cash ratio on that date. + end_cash_balance = derive_cash_balance_on_date_from_total( + total_balance: valuation.amount, + date: date + ) + end_non_cash_balance = valuation.amount - end_cash_balance + start_cash_balance = end_cash_balance start_non_cash_balance = end_non_cash_balance market_value_change = 0 @@ -73,9 +89,9 @@ def derive_start_non_cash_balance(end_non_cash_balance:, date:) derive_non_cash_balance(end_non_cash_balance, date, direction: :reverse) end - # Reverse syncs are a bit different than forward syncs because we do not allow "reconciliation" valuations - # to be used at all. This is primarily to keep the code and the UI easy to understand. For a more detailed - # explanation, see the test suite. + # Checks if this date should use the opening anchor balance instead of deriving it. + # Only the opening_anchor_date itself gets this treatment — reconciliation waypoints + # are handled separately in the calculate loop above. def use_opening_anchor_for_date?(date) account.has_opening_anchor? && date == account.opening_anchor_date end diff --git a/test/models/account/current_balance_manager_test.rb b/test/models/account/current_balance_manager_test.rb index 539a4d3c9..b3499718a 100644 --- a/test/models/account/current_balance_manager_test.rb +++ b/test/models/account/current_balance_manager_test.rb @@ -208,7 +208,7 @@ class Account::CurrentBalanceManagerTest < ActiveSupport::TestCase assert_equal 1000, @linked_account.balance end - test "updates existing anchor for linked account" do + test "preserves previous day anchor as reconciliation when updating linked account balance" do # First create a current anchor manager = Account::CurrentBalanceManager.new(@linked_account) result = manager.set_current_balance(1000) @@ -216,69 +216,35 @@ class Account::CurrentBalanceManagerTest < ActiveSupport::TestCase current_anchor = @linked_account.valuations.current_anchor.first original_id = current_anchor.id - original_entry_id = current_anchor.entry.id # Travel to tomorrow to ensure date change travel_to Date.current + 1.day do # Now update it - assert_no_difference -> { @linked_account.entries.count } do - assert_no_difference -> { @linked_account.valuations.count } do - result = manager.set_current_balance(2000) - assert result.success? - assert result.changes_made? - end + assert_difference -> { @linked_account.entries.count } => 1, + -> { @linked_account.valuations.count } => 1 do + result = manager.set_current_balance(2000) + assert result.success? + assert result.changes_made? end - current_anchor.reload - assert_equal original_id, current_anchor.id # Same valuation record - assert_equal original_entry_id, current_anchor.entry.id # Same entry record - assert_equal 2000, current_anchor.entry.amount - assert_equal Date.current, current_anchor.entry.date # Should be updated to current date + # The old anchor should now be a reconciliation + preserved_valuation = Valuation.find(original_id) + assert_equal "reconciliation", preserved_valuation.kind + assert_equal 1000, preserved_valuation.entry.amount + assert_equal Valuation.build_reconciliation_name(@linked_account.accountable_type), preserved_valuation.entry.name + assert_equal Date.yesterday, preserved_valuation.entry.date + + # A new current anchor should exist for today + new_anchor = @linked_account.valuations.current_anchor.first + assert_not_equal original_id, new_anchor.id + assert_equal 2000, new_anchor.entry.amount + assert_equal Date.current, new_anchor.entry.date end assert_equal 2000, @linked_account.balance end - test "when no changes made, returns success with no changes made" do - # First create a current anchor - manager = Account::CurrentBalanceManager.new(@linked_account) - result = manager.set_current_balance(1000) - assert result.success? - assert result.changes_made? - - # Try to set the same value on the same date - result = manager.set_current_balance(1000) - - assert result.success? - assert_not result.changes_made? - assert_nil result.error - - assert_equal 1000, @linked_account.balance - end - - test "updates only amount when balance changes" do - manager = Account::CurrentBalanceManager.new(@linked_account) - - # Create initial anchor - result = manager.set_current_balance(1000) - assert result.success? - - current_anchor = @linked_account.valuations.current_anchor.first - original_date = current_anchor.entry.date - - # Update only the balance - result = manager.set_current_balance(1500) - assert result.success? - assert result.changes_made? - - current_anchor.reload - assert_equal 1500, current_anchor.entry.amount - assert_equal original_date, current_anchor.entry.date # Date should remain the same if on same day - - assert_equal 1500, @linked_account.balance - end - - test "updates date when called on different day" do + test "does not preserve same-day anchor as reconciliation" do manager = Account::CurrentBalanceManager.new(@linked_account) # Create initial anchor @@ -286,20 +252,31 @@ class Account::CurrentBalanceManagerTest < ActiveSupport::TestCase assert result.success? current_anchor = @linked_account.valuations.current_anchor.first - original_amount = current_anchor.entry.amount + original_id = current_anchor.id - # Travel to tomorrow and update with same balance - travel_to Date.current + 1.day do + # Try to set the same value on the same date + assert_no_difference -> { @linked_account.entries.count } do result = manager.set_current_balance(1000) assert result.success? - assert result.changes_made? # Should be true because date changed + assert_not result.changes_made? + end - current_anchor.reload - assert_equal original_amount, current_anchor.entry.amount - assert_equal Date.current, current_anchor.entry.date # Should be updated to new current date + # Update with different value on the same day + assert_no_difference -> { @linked_account.entries.count } do + assert_no_difference -> { @linked_account.valuations.count } do + result = manager.set_current_balance(1500) + assert result.success? + assert result.changes_made? + end end - assert_equal 1000, @linked_account.balance + current_anchor.reload + assert_equal original_id, current_anchor.id + assert_equal 1500, current_anchor.entry.amount + assert_equal "current_anchor", current_anchor.kind + assert_equal Date.current, current_anchor.entry.date + + assert_equal 1500, @linked_account.balance end test "current_balance returns balance from current anchor" do diff --git a/test/models/balance/reverse_calculator_test.rb b/test/models/balance/reverse_calculator_test.rb index c3ba12ba4..cee497bc8 100644 --- a/test/models/balance/reverse_calculator_test.rb +++ b/test/models/balance/reverse_calculator_test.rb @@ -28,29 +28,21 @@ class Balance::ReverseCalculatorTest < ActiveSupport::TestCase ) end - # An artificial constraint we put on the reverse sync because it's confusing in both the code and the UI - # to think about how an absolute "Valuation" affects balances when syncing backwards. Furthermore, since - # this is typically a Plaid sync, we expect Plaid to provide us the history. - # Note: while "reconciliation" valuations don't affect balance, `current_anchor` and `opening_anchor` do. - test "reconciliation valuations do not affect balance for reverse syncs" do + # Reconciliation valuations act as waypoints during reverse syncs. This ensures that + # historical balances accurately reflect the API-reported values, even if the transaction + # history is incomplete or missing. + test "reconciliation valuations act as waypoints that reset balance for reverse syncs" 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: "reconciliation", date: 1.day.ago, balance: 17000 }, # Ignored - { type: "reconciliation", date: 2.days.ago, balance: 17000 }, # Ignored + { type: "reconciliation", date: 2.days.ago, balance: 17000 }, # Waypoint! { type: "opening_anchor", date: 4.days.ago, balance: 15000 } ] ) calculated = Balance::ReverseCalculator.new(account).calculate - # The "opening anchor" works slightly differently than most would expect. Since it's an artificial - # value provided by the user to set the date/balance of the start of the account, we must assume - # that there are "missing" entries following it. Because of this, we cannot "carry forward" this value - # like we do for a "forward sync". We simply sync backwards normally, then set the balance on opening - # date equal to this anchor. This is not "ideal", but is a constraint put on us since we cannot guarantee - # a 100% full entries history. assert_calculated_ledger_balances( calculated_data: calculated, expected_data: [ @@ -67,18 +59,164 @@ class Balance::ReverseCalculatorTest < ActiveSupport::TestCase balances: { start: 20000, start_cash: 20000, start_non_cash: 0, end_cash: 20000, end_non_cash: 0, end: 20000 }, flows: 0, adjustments: 0 - }, + }, # Derived from Current anchor { date: 2.days.ago, + 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 + }, # Reset by Reconciliation waypoint + { + date: 3.days.ago, + 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: { cash_adjustments: 0, non_cash_adjustments: 0 } + }, # Derived from Reconciliation waypoint + { + date: 4.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 + ] + ) + end + + # Steady-state production scenario: after several consecutive daily syncs, a chain of + # reconciliation waypoints accumulates. Each waypoint must independently reset the + # balance without interactions leaking between them or toward the opening_anchor. + # Non-monotonic balance values (up and down) confirm each reset is truly independent. + test "multiple consecutive reconciliation waypoints thread through independently" do + account = create_account_with_ledger( + account: { type: Depository, balance: 25000, cash_balance: 25000, currency: "USD" }, + entries: [ + { type: "current_anchor", date: Date.current, balance: 25000 }, + { type: "reconciliation", date: 1.day.ago, balance: 23000 }, # Waypoint 1 (down) + { type: "reconciliation", date: 2.days.ago, balance: 24500 }, # Waypoint 2 (up — non-monotonic) + { type: "reconciliation", date: 3.days.ago, balance: 21000 }, # Waypoint 3 (down) + { type: "reconciliation", date: 4.days.ago, balance: 22000 }, # Waypoint 4 (up — non-monotonic) + { type: "transaction", date: 5.days.ago, amount: 500 }, # Expense in the gap + { type: "transaction", date: 6.days.ago, amount: -300 }, # Income in the gap + { type: "opening_anchor", date: 7.days.ago, balance: 18000 } + ] + ) + + calculated = Balance::ReverseCalculator.new(account).calculate + + assert_calculated_ledger_balances( + calculated_data: calculated, + expected_data: [ + { + date: Date.current, + legacy_balances: { balance: 25000, cash_balance: 25000 }, + balances: { start: 25000, start_cash: 25000, start_non_cash: 0, end_cash: 25000, end_non_cash: 0, end: 25000 }, + flows: 0, + adjustments: 0 + }, # Current anchor + { + date: 1.day.ago, + legacy_balances: { balance: 23000, cash_balance: 23000 }, + balances: { start: 23000, start_cash: 23000, start_non_cash: 0, end_cash: 23000, end_non_cash: 0, end: 23000 }, + flows: 0, + adjustments: 0 + }, # Waypoint 1: reset to 23000 (independent of current_anchor 25000) + { + date: 2.days.ago, + legacy_balances: { balance: 24500, cash_balance: 24500 }, + balances: { start: 24500, start_cash: 24500, start_non_cash: 0, end_cash: 24500, end_non_cash: 0, end: 24500 }, + flows: 0, + adjustments: 0 + }, # Waypoint 2: reset UP to 24500 (proves no carry-forward from waypoint 1's 23000) + { + date: 3.days.ago, + legacy_balances: { balance: 21000, cash_balance: 21000 }, + balances: { start: 21000, start_cash: 21000, start_non_cash: 0, end_cash: 21000, end_non_cash: 0, end: 21000 }, + flows: 0, + adjustments: 0 + }, # Waypoint 3: reset down to 21000 + { + date: 4.days.ago, + legacy_balances: { balance: 22000, cash_balance: 22000 }, + balances: { start: 22000, start_cash: 22000, start_non_cash: 0, end_cash: 22000, end_non_cash: 0, end: 22000 }, + flows: 0, + adjustments: 0 + }, # Waypoint 4: reset up to 22000 (last waypoint before the gap) + { + date: 5.days.ago, + legacy_balances: { balance: 22000, cash_balance: 22000 }, + balances: { start: 22500, start_cash: 22500, start_non_cash: 0, end_cash: 22000, end_non_cash: 0, end: 22000 }, + flows: { cash_inflows: 0, cash_outflows: 500 }, + adjustments: 0 + }, # Expense derived from waypoint 4's 22000 + { + date: 6.days.ago, + legacy_balances: { balance: 22500, cash_balance: 22500 }, + balances: { start: 22200, start_cash: 22200, start_non_cash: 0, end_cash: 22500, end_non_cash: 0, end: 22500 }, + flows: { cash_inflows: 300, cash_outflows: 0 }, + adjustments: { cash_adjustments: 0, non_cash_adjustments: 0 } + }, # Income derived further back, right before opening_anchor + { + date: 7.days.ago, + legacy_balances: { balance: 18000, cash_balance: 18000 }, + balances: { start: 18000, start_cash: 18000, start_non_cash: 0, end_cash: 18000, end_non_cash: 0, end: 18000 }, + flows: 0, + adjustments: 0 + } # Opening anchor + ] + ) + end + + test "same-day flows are not ignored when processing current_anchor valuation" do + # This regression test ensures that the ReverseCalculator does not mistake the + # current_anchor valuation (on Date.current) for a reconciliation waypoint, + # which would cause it to reset the balance and ignore same-day transactions. + account = create_account_with_ledger( + account: { type: Depository, balance: 20000, cash_balance: 20000, currency: "USD" }, + entries: [ + { type: "current_anchor", date: Date.current, balance: 20000 }, + # A transaction on the same day as the current anchor! + # If we incorrectly "reset" at Date.current, this flow is ignored. + { type: "transaction", date: Date.current, amount: -500 }, + { type: "reconciliation", date: 2.days.ago, balance: 18000 }, + { type: "opening_anchor", date: 4.days.ago, balance: 15000 } + ] + ) + + calculated = Balance::ReverseCalculator.new(account).calculate + + 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 }, + # Since there was a $500 income (amount: -500) today, the START balance + # of today must be 19500 to end at 20000. + balances: { start: 19500, start_cash: 19500, start_non_cash: 0, end_cash: 20000, end_non_cash: 0, end: 20000 }, + flows: { cash_inflows: 500, cash_outflows: 0 }, + adjustments: 0 + }, + { + date: 1.day.ago, + legacy_balances: { balance: 19500, cash_balance: 19500 }, + balances: { start: 19500, start_cash: 19500, start_non_cash: 0, end_cash: 19500, end_non_cash: 0, end: 19500 }, flows: 0, adjustments: 0 }, + { + date: 2.days.ago, + legacy_balances: { balance: 18000, cash_balance: 18000 }, + balances: { start: 18000, start_cash: 18000, start_non_cash: 0, end_cash: 18000, end_non_cash: 0, end: 18000 }, + flows: 0, + adjustments: 0 + }, # Reset by Reconciliation waypoint { date: 3.days.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 }, + legacy_balances: { balance: 18000, cash_balance: 18000 }, + balances: { start: 18000, start_cash: 18000, start_non_cash: 0, end_cash: 18000, end_non_cash: 0, end: 18000 }, flows: 0, adjustments: { cash_adjustments: 0, non_cash_adjustments: 0 } }, @@ -93,6 +231,7 @@ class Balance::ReverseCalculatorTest < ActiveSupport::TestCase ) end + # Investment account balances are made of two components: cash and holdings. test "anchors on investment accounts calculate cash balance dynamically based on holdings value" do account = create_account_with_ledger(