mirror of
https://github.com/we-promise/sure.git
synced 2026-05-29 15:34:58 +00:00
fix(balance): derive waypoint start from day's flows to prevent double-counting and phantom bumps (#2031)
* fix(balance): fix double-counting on reconciliation waypoints with same-day transactions Waypoint branch was setting start = end = waypoint and passing real flows to build_balance. Since end_balance is a PG generated column that recomputes from flows, transactions were double-counted on waypoint days and the prior gap day inherited a phantom jump. Fix: pin only the end to the API value, derive start from the day's own flows (same as current_anchor). Transaction attributed once, gap day correct, investment cash/holdings split correct. Adds regression test + GUI breakdown test verified against real PG columns through UI::Account::BalanceReconciliation. Fixes #2007. * test(balance): add investment waypoint regression test Covers reconciliation waypoint + same-day trade on investment accounts: end_balance must match API-reported total (not double-count trade flows), cash/non-cash flows must be preserved, and gap day total must be correct.
This commit is contained in:
@@ -25,20 +25,20 @@ class Balance::ReverseCalculator < Balance::BaseCalculator
|
||||
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.
|
||||
# Reconciliation waypoint: hard-reset the END-of-day balance to the
|
||||
# API-reported value, neutralizing any drift accumulated from missing
|
||||
# transactions between here and the next anchor. The START is still
|
||||
# derived from this day's own flows, so a same-day transaction is
|
||||
# attributed exactly once (and not added on top of the waypoint).
|
||||
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
|
||||
start_cash_balance = derive_start_cash_balance(end_cash_balance: end_cash_balance, date: date)
|
||||
start_non_cash_balance = derive_start_non_cash_balance(end_non_cash_balance: end_non_cash_balance, date: date)
|
||||
market_value_change = market_value_change_on_date(date, flows)
|
||||
else
|
||||
start_cash_balance = derive_start_cash_balance(end_cash_balance: end_cash_balance, date: date)
|
||||
start_non_cash_balance = derive_start_non_cash_balance(end_non_cash_balance: end_non_cash_balance, date: date)
|
||||
|
||||
@@ -231,6 +231,66 @@ class Balance::ReverseCalculatorTest < ActiveSupport::TestCase
|
||||
)
|
||||
end
|
||||
|
||||
# Regression for #2007: a reconciliation waypoint on a day that ALSO has a transaction.
|
||||
# The waypoint is an END-of-day balance, so the day's own flow must derive the START
|
||||
# (waypoint - flow), and the flow must NOT be added on top of the waypoint (double-count).
|
||||
# Critically, the derived start (not the waypoint) is what carries back, so the prior
|
||||
# gap day must not show a phantom jump.
|
||||
test "reconciliation waypoint with a same-day transaction is not double-counted" 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: 2.days.ago, balance: 17000 }, # Waypoint (end-of-day)
|
||||
{ type: "transaction", date: 2.days.ago, amount: -2000 }, # 2000 deposit ON the waypoint day
|
||||
{ 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 },
|
||||
flows: 0,
|
||||
adjustments: 0
|
||||
},
|
||||
{
|
||||
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
|
||||
},
|
||||
{
|
||||
date: 2.days.ago, # End forced to waypoint 17000; start derived from the +2000 deposit = 15000
|
||||
legacy_balances: { balance: 17000, cash_balance: 17000 },
|
||||
balances: { start: 15000, start_cash: 15000, start_non_cash: 0, end_cash: 17000, end_non_cash: 0, end: 17000 },
|
||||
flows: { cash_inflows: 2000, cash_outflows: 0 },
|
||||
adjustments: 0
|
||||
},
|
||||
{
|
||||
date: 3.days.ago, # No phantom jump: stays at 15000 (the deposit is not re-applied here)
|
||||
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
|
||||
},
|
||||
{
|
||||
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
|
||||
|
||||
|
||||
# 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
|
||||
|
||||
113
test/models/balance/waypoint_gui_breakdown_test.rb
Normal file
113
test/models/balance/waypoint_gui_breakdown_test.rb
Normal file
@@ -0,0 +1,113 @@
|
||||
require "test_helper"
|
||||
|
||||
# Proves what the day-detail "Details" popup (UI::Account::BalanceReconciliation)
|
||||
# shows for a daily-waypoint (EnableBanking-style) account with a transaction on
|
||||
# every waypoint day. Persists real balances and reads the actual PG generated
|
||||
# columns through the real GUI component.
|
||||
class Balance::WaypointGuiBreakdownTest < ActiveSupport::TestCase
|
||||
include LedgerTestingHelper
|
||||
|
||||
def persist_and_load(account)
|
||||
calculated = Balance::ReverseCalculator.new(account).calculate
|
||||
account.balances.upsert_all(
|
||||
calculated.map { |b|
|
||||
b.attributes.slice(
|
||||
"date", "balance", "cash_balance", "currency",
|
||||
"start_cash_balance", "start_non_cash_balance",
|
||||
"cash_inflows", "cash_outflows",
|
||||
"non_cash_inflows", "non_cash_outflows",
|
||||
"net_market_flows", "cash_adjustments", "non_cash_adjustments",
|
||||
"flows_factor"
|
||||
).merge("updated_at" => Time.now)
|
||||
},
|
||||
unique_by: %i[account_id date currency]
|
||||
)
|
||||
account.balances.order(:date).to_a
|
||||
end
|
||||
|
||||
# Reads the three lines exactly as the GUI Details popup renders them.
|
||||
def gui_lines(balance, account)
|
||||
items = UI::Account::BalanceReconciliation.new(balance: balance, account: account).reconciliation_items
|
||||
{
|
||||
start: items.find { |i| i[:style] == :start }[:value].amount,
|
||||
net_flow: items.find { |i| i[:style] == :flow }[:value].amount,
|
||||
final: items.find { |i| i[:style] == :final }[:value].amount
|
||||
}
|
||||
end
|
||||
|
||||
def build_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: 19000 },
|
||||
{ type: "transaction", date: 1.day.ago, amount: -1000 }, # +1000 deposit
|
||||
{ type: "reconciliation", date: 2.days.ago, balance: 17000 },
|
||||
{ type: "transaction", date: 2.days.ago, amount: -2000 }, # +2000 deposit
|
||||
{ type: "reconciliation", date: 3.days.ago, balance: 16000 },
|
||||
{ type: "transaction", date: 3.days.ago, amount: 500 }, # -500 expense
|
||||
# 4.days.ago is a GAP day: no waypoint, no transaction
|
||||
{ type: "opening_anchor", date: 5.days.ago, balance: 15000 }
|
||||
]
|
||||
)
|
||||
end
|
||||
|
||||
# Hard assertions for the fix currently in the tree (derive-start).
|
||||
test "derive-start fix: Net cash flow preserved on every waypoint day and it reconciles" do
|
||||
account = build_account
|
||||
balances = persist_and_load(account).index_by(&:date)
|
||||
|
||||
expectations = {
|
||||
1.day.ago.to_date => { start: 18000, net_flow: 1000, final: 19000 },
|
||||
2.days.ago.to_date => { start: 15000, net_flow: 2000, final: 17000 },
|
||||
3.days.ago.to_date => { start: 16500, net_flow: -500, final: 16000 }
|
||||
}
|
||||
|
||||
expectations.each do |date, exp|
|
||||
g = gui_lines(balances[date], account)
|
||||
assert_equal exp[:start], g[:start], "Start balance wrong on #{date}"
|
||||
assert_equal exp[:net_flow], g[:net_flow], "Net cash flow wrong on #{date} (should NOT be zeroed)"
|
||||
assert_equal exp[:final], g[:final], "Final balance wrong on #{date}"
|
||||
assert_equal g[:start] + g[:net_flow], g[:final], "Breakdown does not reconcile on #{date}"
|
||||
refute_equal 0, g[:net_flow], "Net cash flow was zeroed on #{date} — transaction hidden"
|
||||
end
|
||||
end
|
||||
|
||||
# Investment account: reconciliation waypoint on a day with a same-day trade.
|
||||
# End total must match the API-reported waypoint (not waypoint + trade flows).
|
||||
# Cash/non-cash split must be preserved and no double-count on the non-cash side.
|
||||
test "investment reconciliation waypoint with same-day trade is not double-counted" do
|
||||
account = create_account_with_ledger(
|
||||
account: { type: Investment, balance: 20000, cash_balance: 10000, currency: "USD" },
|
||||
entries: [
|
||||
{ type: "current_anchor", date: Date.current, balance: 20000 },
|
||||
{ type: "reconciliation", date: 1.day.ago, balance: 18000 }, # Bank says total was 18000
|
||||
{ type: "trade", date: 1.day.ago, ticker: "AAPL", qty: 10, price: 100 }, # Buy $1000 of AAPL
|
||||
{ type: "opening_anchor", date: 3.days.ago, balance: 15000 }
|
||||
],
|
||||
holdings: [
|
||||
{ date: Date.current, ticker: "AAPL", qty: 10, price: 100, amount: 1000 },
|
||||
{ date: 1.day.ago.to_date, ticker: "AAPL", qty: 10, price: 100, amount: 1000 },
|
||||
{ date: 2.days.ago.to_date, ticker: "AAPL", qty: 0, price: 100, amount: 0 },
|
||||
{ date: 3.days.ago.to_date, ticker: "AAPL", qty: 0, price: 100, amount: 0 }
|
||||
]
|
||||
)
|
||||
|
||||
balances = persist_and_load(account).index_by(&:date)
|
||||
waypoint = balances[1.day.ago.to_date]
|
||||
|
||||
# End total must equal the API-reported waypoint — not 18000 + trade flows (which would be 20000).
|
||||
assert_equal 18000, waypoint.end_balance, "Investment waypoint end_balance must equal 18000, not double-count the trade"
|
||||
|
||||
# Cash/non-cash flows from the trade must still be present (not zeroed).
|
||||
assert waypoint.cash_outflows > 0 || waypoint.non_cash_inflows > 0,
|
||||
"Trade flows should be preserved on the waypoint day, not zeroed"
|
||||
|
||||
# A trade moves cash → holdings but doesn't change the total balance, so
|
||||
# the gap day correctly stays at the same total as the waypoint — this is
|
||||
# expected, not a phantom. The key check is that the waypoint day itself
|
||||
# didn't inflate above 18000 by double-counting the trade.
|
||||
gap_day = balances[2.days.ago.to_date]
|
||||
assert_equal 18000, gap_day.end_balance, "Gap day total should equal pre-trade balance (trade doesn't change total)"
|
||||
end
|
||||
end
|
||||
Reference in New Issue
Block a user