mirror of
https://github.com/we-promise/sure.git
synced 2026-05-10 14:15:01 +00:00
c294cbf54b
* Performance improvements in holding calculation pipeline
Investment accounts with large histories were pegging CPU at 100% during
sync. Root cause was a cluster of quadratic and superlinear algorithms in
the inner holding calculation loop. All are replaced with O(1) hash lookups
built from single-pass indexes over the already-loaded data.
Holding::PortfolioCache - load_prices:
Three O(SxN) patterns inside the per-security loop:
1. DB prices: `security.prices.where(...)` fired one SQL query per
security (N+1). Replaced with a single bulk query before the loop:
Security::Price.where(security_id: ..., date: ...).group_by(&:security_id)
70 securities -> 70 queries becomes 1.
2. Trade prices: `trades.select { |t| t.entryable.security_id == id }`
scanned the full trades array for every security - O(SxT). Replaced
with trades_by_security_id, pre-indexed once from the loaded array.
3. Holding prices: `holdings.select { |h| h.security_id == id }` - same
O(SxH) pattern. Replaced with holdings_by_security_id.
Prices are now indexed into prices_by_date and prices_by_date_and_source
hashes during load_prices, making get_price O(1) instead of scanning the
flat prices array on every lookup.
Holding::PortfolioCache - get_trades / get_price:
- get_trades(date:): `trades.select { |t| t.date == date }` (O(T) scan)
replaced with trades_by_date hash (O(1)).
- get_price: two `prices.select { p.date == date ... }.min_by` linear
scans replaced with direct hash lookups into prices_by_date and
prices_by_date_and_source.
Holding::PortfolioCache - collect_unique_securities:
`holdings.map(&:security)` traversed the security association on every
holding record (N+1 if not preloaded). Replaced with a pluck of
security_ids followed by a single Security.where(id: ...) batch load.
Holding::ForwardCalculator / ReverseCalculator:
`holdings += build_holdings(...)` allocated a new array copy on every
iteration - O(N) per day x thousands of days = O(D^2) total allocations.
Replaced with holdings.concat(...) which appends in place, O(1).
Holding::ReverseCalculator - precompute_cost_basis:
Old: walked every date from account.start_date to Date.current (O(D)),
writing a cost_basis entry for every security on every date. For an
account with 2 trades over 9,250 days this wrote ~18,500 hash entries
and consumed the full date range in the outer loop regardless of trade
density.
New: walks only buy trades (O(T)), appending one [date, avg_cost]
snapshot per trade. cost_basis_for binary-searches the sparse snapshot
array - O(log T) per lookup. Memory drops from O(DxS) to O(T).
Holding::Gapfillable:
`security_holdings.find { |h| h.date == date }` was called on every
date in the gapfill range - O(H) per date, O(HxD) total. Replaced with
security_holdings.index_by(:date) built once before the loop, making
each date lookup O(1).
Holding::Materializer - purge_stale_holdings:
`account.entries.trades.map { |entry| entry.entryable.security_id }.uniq`
loaded all trade entry records into Ruby then traversed the entryable
association on each (N+1). Replaced with account.trades.pluck(:security_id).uniq
(single SQL query returning only the IDs).
In testing, these changes were able to reduce sync time of an account with
25 years of history and 70 securities from about 90 minutes down to under
3 minutes.
* Lint fix
* Lint fix
* addressing the open review nits I agreed with:
* return dup'd arrays from PortfolioCache#get_trades so callers can't mutate memoized cache state
* use the precomputed security-id indexes in collect_unique_securities
* keep security-id dedupe in SQL via distinct.pluck(:security_id)
* tighten the DB price preload to select only needed columns
* harden cost-basis assertions with assert_in_delta
* Back out unnecessary AI slop
* Add back dup to trades array returned from memoized hash
trades_by_date[date] returns a live reference into the memoized hash.
Any caller that mutates the result would silently corrupt the cache for
subsequent calls on the same date within the same sync run. Add .dup to
return a shallow copy, matching the safety of the original select path.
319 lines
14 KiB
Ruby
319 lines
14 KiB
Ruby
require "test_helper"
|
|
|
|
class Holding::ReverseCalculatorTest < ActiveSupport::TestCase
|
|
include EntriesTestHelper
|
|
|
|
setup do
|
|
@account = families(:empty).accounts.create!(
|
|
name: "Test",
|
|
balance: 20000,
|
|
cash_balance: 20000,
|
|
currency: "USD",
|
|
accountable: Investment.new
|
|
)
|
|
end
|
|
|
|
test "no holdings" do
|
|
empty_snapshot = OpenStruct.new(to_h: {})
|
|
calculated = Holding::ReverseCalculator.new(@account, portfolio_snapshot: empty_snapshot).calculate
|
|
assert_equal [], calculated
|
|
end
|
|
|
|
test "holding generation respects user timezone and last generated date is current user date" do
|
|
# Simulate user in EST timezone
|
|
Time.use_zone("America/New_York") do
|
|
# Set current time to 1am UTC on Jan 5, 2025
|
|
# This would be 8pm EST on Jan 4, 2025 (user's time, and the last date we should generate holdings for)
|
|
travel_to Time.utc(2025, 01, 05, 1, 0, 0)
|
|
|
|
voo = Security.create!(ticker: "VOO", name: "Vanguard S&P 500 ETF")
|
|
Security::Price.create!(security: voo, date: "2025-01-02", price: 500)
|
|
Security::Price.create!(security: voo, date: "2025-01-03", price: 500)
|
|
Security::Price.create!(security: voo, date: "2025-01-04", price: 500)
|
|
|
|
# Today's holdings (provided)
|
|
@account.holdings.create!(security: voo, date: "2025-01-04", qty: 10, price: 500, amount: 5000, currency: "USD")
|
|
|
|
create_trade(voo, qty: 10, date: "2025-01-03", price: 500, account: @account)
|
|
|
|
expected = [ [ "2025-01-02", 0 ], [ "2025-01-03", 5000 ], [ "2025-01-04", 5000 ] ]
|
|
# Mock snapshot with the holdings we created
|
|
snapshot = OpenStruct.new(to_h: { voo.id => 10 })
|
|
calculated = Holding::ReverseCalculator.new(@account, portfolio_snapshot: snapshot).calculate
|
|
|
|
assert_equal expected, calculated.sort_by(&:date).map { |b| [ b.date.to_s, b.amount ] }
|
|
end
|
|
end
|
|
|
|
# Should be able to handle this case, although we should not be reverse-syncing an account without provided current day holdings
|
|
test "reverse portfolio with trades but without current day holdings" do
|
|
voo = Security.create!(ticker: "VOO", name: "Vanguard S&P 500 ETF")
|
|
Security::Price.create!(security: voo, date: Date.current, price: 470)
|
|
Security::Price.create!(security: voo, date: 1.day.ago.to_date, price: 470)
|
|
|
|
create_trade(voo, qty: -10, date: Date.current, price: 470, account: @account)
|
|
|
|
# Mock empty portfolio since no current day holdings
|
|
snapshot = OpenStruct.new(to_h: { voo.id => 0 })
|
|
calculated = Holding::ReverseCalculator.new(@account, portfolio_snapshot: snapshot).calculate
|
|
assert_equal 2, calculated.length
|
|
end
|
|
|
|
test "reverse portfolio calculation" do
|
|
load_today_portfolio
|
|
|
|
# Build up to 10 shares of VOO (current value $5000)
|
|
create_trade(@voo, qty: 20, date: 3.days.ago.to_date, price: 470, account: @account)
|
|
create_trade(@voo, qty: -15, date: 2.days.ago.to_date, price: 480, account: @account)
|
|
create_trade(@voo, qty: 5, date: 1.day.ago.to_date, price: 490, account: @account)
|
|
|
|
# Amazon won't exist in current holdings because qty is zero, but should show up in historical portfolio
|
|
create_trade(@amzn, qty: 1, date: 2.days.ago.to_date, price: 200, account: @account)
|
|
create_trade(@amzn, qty: -1, date: 1.day.ago.to_date, price: 200, account: @account)
|
|
|
|
# Build up to 100 shares of WMT (current value $10000)
|
|
create_trade(@wmt, qty: 100, date: 1.day.ago.to_date, price: 100, account: @account)
|
|
|
|
expected = [
|
|
# 4 days ago
|
|
Holding.new(security: @voo, date: 4.days.ago.to_date, qty: 0, price: 460, amount: 0),
|
|
Holding.new(security: @wmt, date: 4.days.ago.to_date, qty: 0, price: 100, amount: 0),
|
|
Holding.new(security: @amzn, date: 4.days.ago.to_date, qty: 0, price: 200, amount: 0),
|
|
|
|
# 3 days ago
|
|
Holding.new(security: @voo, date: 3.days.ago.to_date, qty: 20, price: 470, amount: 9400),
|
|
Holding.new(security: @wmt, date: 3.days.ago.to_date, qty: 0, price: 100, amount: 0),
|
|
Holding.new(security: @amzn, date: 3.days.ago.to_date, qty: 0, price: 200, amount: 0),
|
|
|
|
# 2 days ago
|
|
Holding.new(security: @voo, date: 2.days.ago.to_date, qty: 5, price: 480, amount: 2400),
|
|
Holding.new(security: @wmt, date: 2.days.ago.to_date, qty: 0, price: 100, amount: 0),
|
|
Holding.new(security: @amzn, date: 2.days.ago.to_date, qty: 1, price: 200, amount: 200),
|
|
|
|
# 1 day ago
|
|
Holding.new(security: @voo, date: 1.day.ago.to_date, qty: 10, price: 490, amount: 4900),
|
|
Holding.new(security: @wmt, date: 1.day.ago.to_date, qty: 100, price: 100, amount: 10000),
|
|
Holding.new(security: @amzn, date: 1.day.ago.to_date, qty: 0, price: 200, amount: 0),
|
|
|
|
# Today
|
|
Holding.new(security: @voo, date: Date.current, qty: 10, price: 500, amount: 5000),
|
|
Holding.new(security: @wmt, date: Date.current, qty: 100, price: 100, amount: 10000),
|
|
Holding.new(security: @amzn, date: Date.current, qty: 0, price: 200, amount: 0)
|
|
]
|
|
|
|
# Mock snapshot with today's portfolio from load_today_portfolio
|
|
snapshot = OpenStruct.new(to_h: { @voo.id => 10, @wmt.id => 100, @amzn.id => 0 })
|
|
calculated = Holding::ReverseCalculator.new(@account, portfolio_snapshot: snapshot).calculate
|
|
|
|
assert_equal expected.length, calculated.length
|
|
|
|
expected.each do |expected_entry|
|
|
calculated_entry = calculated.find { |c| c.security == expected_entry.security && c.date == expected_entry.date }
|
|
|
|
assert_equal expected_entry.qty, calculated_entry.qty, "Qty mismatch for #{expected_entry.security.ticker} on #{expected_entry.date}"
|
|
assert_equal expected_entry.price, calculated_entry.price, "Price mismatch for #{expected_entry.security.ticker} on #{expected_entry.date}"
|
|
assert_equal expected_entry.amount, calculated_entry.amount, "Amount mismatch for #{expected_entry.security.ticker} on #{expected_entry.date}"
|
|
end
|
|
end
|
|
|
|
# For a reverse sync, Plaid will provide today's holdings + prices. We need to match those exactly so balances match in net worth rollups.
|
|
test "current day holdings always match provided holdings and prices" do
|
|
# Provider gives us total value of $10,000 ($5,000 cash, $5,000 in holdings)
|
|
@account.update!(balance: 10000, cash_balance: 5000)
|
|
|
|
wmt = Security.create!(ticker: "WMT", name: "Walmart Inc.")
|
|
create_trade(wmt, qty: 50, date: 1.day.ago.to_date, price: 98, account: @account)
|
|
|
|
@account.holdings.create!(
|
|
date: Date.current,
|
|
price: 100,
|
|
qty: 50,
|
|
amount: 5000,
|
|
currency: "USD",
|
|
security: wmt
|
|
)
|
|
|
|
Security::Price.create!(security: wmt, date: Date.current, price: 102) # This price should be ignored on current day
|
|
Security::Price.create!(security: wmt, date: 1.day.ago, price: 98) # This price will be used for historical holding calculation
|
|
Security::Price.create!(security: wmt, date: 2.days.ago, price: 95) # This price will be used for historical holding calculation
|
|
|
|
expected = [
|
|
Holding.new(security: wmt, date: 2.days.ago.to_date, qty: 0, price: 95, amount: 0), # Uses market price, empty holding
|
|
Holding.new(security: wmt, date: 1.day.ago.to_date, qty: 50, price: 98, amount: 4900), # Uses market price
|
|
Holding.new(security: wmt, date: Date.current, qty: 50, price: 100, amount: 5000) # Uses holding price, not market price
|
|
]
|
|
|
|
# Mock snapshot with WMT holding from the test setup
|
|
snapshot = OpenStruct.new(to_h: { wmt.id => 50 })
|
|
calculated = Holding::ReverseCalculator.new(@account, portfolio_snapshot: snapshot).calculate
|
|
|
|
assert_equal expected.length, calculated.length
|
|
|
|
expected.each do |expected_entry|
|
|
calculated_entry = calculated.find { |c| c.security == expected_entry.security && c.date == expected_entry.date }
|
|
|
|
assert_equal expected_entry.qty, calculated_entry.qty, "Qty mismatch for #{expected_entry.security.ticker} on #{expected_entry.date}"
|
|
assert_equal expected_entry.price, calculated_entry.price, "Price mismatch for #{expected_entry.security.ticker} on #{expected_entry.date}"
|
|
assert_equal expected_entry.amount, calculated_entry.amount, "Amount mismatch for #{expected_entry.security.ticker} on #{expected_entry.date}"
|
|
end
|
|
end
|
|
|
|
# cost_basis_for
|
|
|
|
test "cost_basis_for returns nil when there are no buy trades" do
|
|
security = Security.create!(ticker: "TST", name: "Test")
|
|
calc = calculator_with_trades(security)
|
|
|
|
assert_nil cost_basis_for(calc, security, Date.current)
|
|
end
|
|
|
|
test "cost_basis_for returns nil for dates before the first buy" do
|
|
security = Security.create!(ticker: "TST", name: "Test")
|
|
buy_date = 5.days.ago.to_date
|
|
|
|
calc = calculator_with_trades(security) do
|
|
create_trade(security, account: @account, qty: 10, price: 100, date: buy_date)
|
|
end
|
|
|
|
assert_nil cost_basis_for(calc, security, buy_date - 1)
|
|
end
|
|
|
|
test "cost_basis_for returns weighted average cost on buy date" do
|
|
security = Security.create!(ticker: "TST", name: "Test")
|
|
buy_date = 5.days.ago.to_date
|
|
|
|
calc = calculator_with_trades(security) do
|
|
create_trade(security, account: @account, qty: 10, price: 100, date: buy_date)
|
|
end
|
|
|
|
assert_in_delta 100.0, cost_basis_for(calc, security, buy_date).to_f, 1e-6
|
|
end
|
|
|
|
test "cost_basis_for carries forward to dates between buys" do
|
|
security = Security.create!(ticker: "TST", name: "Test")
|
|
first_buy = 10.days.ago.to_date
|
|
second_buy = 3.days.ago.to_date
|
|
|
|
calc = calculator_with_trades(security) do
|
|
create_trade(security, account: @account, qty: 10, price: 100, date: first_buy)
|
|
create_trade(security, account: @account, qty: 5, price: 130, date: second_buy)
|
|
end
|
|
|
|
# Between the two buys, cost basis is from the first buy only
|
|
assert_in_delta 100.0, cost_basis_for(calc, security, first_buy + 1).to_f, 1e-6
|
|
assert_in_delta 100.0, cost_basis_for(calc, security, second_buy - 1).to_f, 1e-6
|
|
|
|
# After second buy: WAC = (10*100 + 5*130) / 15 = 110.0
|
|
assert_in_delta 110.0, cost_basis_for(calc, security, second_buy).to_f, 1e-6
|
|
assert_in_delta 110.0, cost_basis_for(calc, security, Date.current).to_f, 1e-6
|
|
end
|
|
|
|
test "cost_basis_for accumulates multiple buys on the same date" do
|
|
security = Security.create!(ticker: "TST", name: "Test")
|
|
buy_date = 5.days.ago.to_date
|
|
|
|
calc = calculator_with_trades(security) do
|
|
create_trade(security, account: @account, qty: 10, price: 100, date: buy_date)
|
|
create_trade(security, account: @account, qty: 5, price: 130, date: buy_date)
|
|
end
|
|
|
|
# WAC = (10*100 + 5*130) / 15 = 110.0 — not the intermediate value after only the first trade
|
|
assert_in_delta 110.0, cost_basis_for(calc, security, buy_date).to_f, 1e-6
|
|
end
|
|
|
|
test "cost_basis_for ignores sell trades" do
|
|
security = Security.create!(ticker: "TST", name: "Test")
|
|
buy_date = 10.days.ago.to_date
|
|
sell_date = 5.days.ago.to_date
|
|
|
|
calc = calculator_with_trades(security) do
|
|
create_trade(security, account: @account, qty: 10, price: 100, date: buy_date)
|
|
create_trade(security, account: @account, qty: -5, price: 120, date: sell_date)
|
|
end
|
|
|
|
# Sell does not change cost basis
|
|
assert_in_delta 100.0, cost_basis_for(calc, security, sell_date).to_f, 1e-6
|
|
assert_in_delta 100.0, cost_basis_for(calc, security, Date.current).to_f, 1e-6
|
|
end
|
|
|
|
private
|
|
def assert_holdings(expected, calculated)
|
|
expected.each do |expected_entry|
|
|
calculated_entry = calculated.find { |c| c.security == expected_entry.security && c.date == expected_entry.date }
|
|
|
|
assert_equal expected_entry.qty, calculated_entry.qty, "Qty mismatch for #{expected_entry.security.ticker} on #{expected_entry.date}"
|
|
assert_equal expected_entry.price, calculated_entry.price, "Price mismatch for #{expected_entry.security.ticker} on #{expected_entry.date}"
|
|
assert_equal expected_entry.amount, calculated_entry.amount, "Amount mismatch for #{expected_entry.security.ticker} on #{expected_entry.date}"
|
|
end
|
|
end
|
|
|
|
def load_prices
|
|
@voo = Security.create!(ticker: "VOO", name: "Vanguard S&P 500 ETF")
|
|
Security::Price.create!(security: @voo, date: 4.days.ago.to_date, price: 460)
|
|
Security::Price.create!(security: @voo, date: 3.days.ago.to_date, price: 470)
|
|
Security::Price.create!(security: @voo, date: 2.days.ago.to_date, price: 480)
|
|
Security::Price.create!(security: @voo, date: 1.day.ago.to_date, price: 490)
|
|
Security::Price.create!(security: @voo, date: Date.current, price: 500)
|
|
|
|
@wmt = Security.create!(ticker: "WMT", name: "Walmart Inc.")
|
|
Security::Price.create!(security: @wmt, date: 4.days.ago.to_date, price: 100)
|
|
Security::Price.create!(security: @wmt, date: 3.days.ago.to_date, price: 100)
|
|
Security::Price.create!(security: @wmt, date: 2.days.ago.to_date, price: 100)
|
|
Security::Price.create!(security: @wmt, date: 1.day.ago.to_date, price: 100)
|
|
Security::Price.create!(security: @wmt, date: Date.current, price: 100)
|
|
|
|
@amzn = Security.create!(ticker: "AMZN", name: "Amazon.com Inc.")
|
|
Security::Price.create!(security: @amzn, date: 4.days.ago.to_date, price: 200)
|
|
Security::Price.create!(security: @amzn, date: 3.days.ago.to_date, price: 200)
|
|
Security::Price.create!(security: @amzn, date: 2.days.ago.to_date, price: 200)
|
|
Security::Price.create!(security: @amzn, date: 1.day.ago.to_date, price: 200)
|
|
Security::Price.create!(security: @amzn, date: Date.current, price: 200)
|
|
end
|
|
|
|
# Portfolio holdings:
|
|
# +--------+-----+--------+---------+
|
|
# | Ticker | Qty | Price | Amount |
|
|
# +--------+-----+--------+---------+
|
|
# | VOO | 10 | $500 | $5,000 |
|
|
# | WMT | 100 | $100 | $10,000 |
|
|
# +--------+-----+--------+---------+
|
|
# Brokerage Cash: $5,000
|
|
# Holdings Value: $15,000
|
|
# Total Balance: $20,000
|
|
def calculator_with_trades(security)
|
|
yield if block_given?
|
|
snapshot = OpenStruct.new(to_h: { security.id => 10 })
|
|
calc = Holding::ReverseCalculator.new(@account, portfolio_snapshot: snapshot)
|
|
calc.send(:precompute_cost_basis)
|
|
calc
|
|
end
|
|
|
|
def cost_basis_for(calc, security, date)
|
|
calc.send(:cost_basis_for, security.id, date)
|
|
end
|
|
|
|
def load_today_portfolio
|
|
@account.update!(cash_balance: 5000)
|
|
|
|
load_prices
|
|
|
|
@account.holdings.create!(
|
|
date: Date.current,
|
|
price: 500,
|
|
qty: 10,
|
|
amount: 5000,
|
|
currency: "USD",
|
|
security: @voo
|
|
)
|
|
|
|
@account.holdings.create!(
|
|
date: Date.current,
|
|
price: 100,
|
|
qty: 100,
|
|
amount: 10000,
|
|
currency: "USD",
|
|
security: @wmt
|
|
)
|
|
end
|
|
end
|