QT/sure
1
0
Fork 0
mirror of https://github.com/we-promise/sure.git synced 2026-04-19 03:54:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat(balance): Incremental ForwardCalculator — only recalculate from changed date forward (#1151)

Browse Source 
* feat(balance): incremental ForwardCalculator — only recalculate from changed date forward

When a Sync record carries a window_start_date, ForwardCalculator now
seeds its starting balances from the persisted DB balance for
window_start_date - 1, then iterates only from window_start_date to
calc_end_date.  This avoids recomputing every daily balance on a
long-lived account when a single transaction changes.

Key changes:
- Account::Syncer passes sync.window_start_date to Balance::Materializer
- Balance::Materializer accepts window_start_date and forwards it to
  ForwardCalculator; purge_stale_balances uses opening_anchor_date as the
  lower bound in incremental mode so pre-window balances are not deleted
- Balance::ForwardCalculator accepts window_start_date; resolve_starting_balances
  loads end_cash_balance/end_non_cash_balance from the prior DB record and
  falls back to full recalculation when no prior record exists
- Tests added for incremental correctness, fallback behaviour, and purge safety

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

# Conflicts:
#	app/models/balance/materializer.rb

* Enhance fallback logic on ForwardCalculator and Materializer

* fix(balance): address CodeRabbit review issues on incremental ForwardCalculator

- materializer.rb: handle empty sorted_balances in incremental mode by still
  purging stale tail balances beyond window_start_date - 1, preventing orphaned
  future rows when a transaction is deleted and the recalc window produces no rows

- materializer_test.rb: stub incremental? alongside calculate in the incremental
  sync test so the guard in ForwardCalculator#incremental? doesn't raise when
  @fell_back is nil (never set because calculate was stubbed out)

- materializer_test.rb: correct window_start_date in the fallback test from
  3.days.ago to 2.days.ago so window_start_date - 1 hits a date with no
  persisted balance, correctly triggering full recalculation instead of
  accidentally seeding from the stale wrong_pre_window balance

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat(balance): multi-currency fallback to full recalculation and add corresponding tests

* address coderabbit comment about test

* Make the foreign-currency precondition explicit in the test setup.

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Serge L
2026-03-15 16:29:01 -04:00
committed by GitHub
parent 5aa808e668
commit 5a43f123c2
5 changed files with 381 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

174
test/models/balance/forward_calculator_test.rb
View File

@@ -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

94
test/models/balance/materializer_test.rb
View File

@@ -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)