mirror of
https://github.com/we-promise/sure.git
synced 2026-07-27 12:12:13 +00:00
fix(snaptrade): don't double count cash-equivalent (money market) positions (#2696)
* fix snaptrade double count cash equivalent * debug logging * fix fallback edge case * comments and no floor
This commit is contained in:
@@ -151,10 +151,7 @@ class SnaptradeAccount < ApplicationRecord
|
||||
# multi-currency cash isn't discarded (issue #1809). Excludes the actual
|
||||
# primary currency — including the USD fallback — to avoid double-counting.
|
||||
def non_primary_cash_entries
|
||||
entries = Array(raw_balances_payload).map do |entry|
|
||||
entry.respond_to?(:with_indifferent_access) ? entry.with_indifferent_access : {}
|
||||
end
|
||||
|
||||
entries = normalized_balance_entries
|
||||
primary_code = primary_cash_entry(entries)&.dig(:currency, :code)
|
||||
|
||||
entries.filter_map do |e|
|
||||
@@ -166,6 +163,40 @@ class SnaptradeAccount < ApplicationRecord
|
||||
end
|
||||
end
|
||||
|
||||
# Currency of the amount stored in cash_balance. upsert_balances! falls back
|
||||
# to a USD (or first) balance entry when there is no entry in the account
|
||||
# currency, so the stored cash is not guaranteed to be denominated in
|
||||
# `currency` — cash adjustments must use this instead.
|
||||
def primary_cash_currency
|
||||
primary_cash_entry(normalized_balance_entries)&.dig(:currency, :code) || currency
|
||||
end
|
||||
|
||||
# Total value of positions SnapTrade flags as `cash_equivalent` (usually
|
||||
# money market / sweep funds such as Fidelity's SPAXX) held in the given
|
||||
# currency. Per SnapTrade's docs, these positions are ALSO included in the
|
||||
# balances endpoint's `cash` figure, so cash figures must have this value
|
||||
# removed to avoid double counting the same money. The positions themselves
|
||||
# are still imported as holdings by SnaptradeAccount::HoldingsProcessor.
|
||||
def cash_equivalent_position_value(currency_code = currency)
|
||||
Array(raw_holdings_payload).sum(BigDecimal("0")) do |holding|
|
||||
data = holding.is_a?(Hash) ? holding.with_indifferent_access : {}
|
||||
next BigDecimal("0") unless data[:cash_equivalent] == true
|
||||
|
||||
# Positions without currency metadata deliberately fall back to the
|
||||
# ACCOUNT currency (not currency_code): that's where HoldingsProcessor
|
||||
# books such a position as a holding, so it must be subtracted from that
|
||||
# same bucket — using currency_code here would subtract the position
|
||||
# from every caller's currency bucket.
|
||||
position_currency = extract_currency(data, extract_symbol_data(data), currency)
|
||||
next BigDecimal("0") unless position_currency == currency_code
|
||||
|
||||
units = parse_decimal(data[:units]) || BigDecimal("0")
|
||||
price = parse_decimal(data[:price]) || BigDecimal("0")
|
||||
|
||||
units * price
|
||||
end
|
||||
end
|
||||
|
||||
# Get the SnapTrade provider instance via the parent item
|
||||
def snaptrade_provider
|
||||
snaptrade_item.snaptrade_provider
|
||||
@@ -178,9 +209,16 @@ class SnaptradeAccount < ApplicationRecord
|
||||
|
||||
private
|
||||
|
||||
def normalized_balance_entries
|
||||
Array(raw_balances_payload).map do |entry|
|
||||
entry.respond_to?(:with_indifferent_access) ? entry.with_indifferent_access : {}
|
||||
end
|
||||
end
|
||||
|
||||
# Selects the primary cash entry from a list of indifferent-access balance
|
||||
# hashes: account currency first, then USD, then the first entry. Shared by
|
||||
# upsert_balances! and non_primary_cash_entries so both stay in sync.
|
||||
# upsert_balances!, non_primary_cash_entries, and primary_cash_currency so
|
||||
# they stay in sync.
|
||||
def primary_cash_entry(entries)
|
||||
entries.find { |b| b.dig(:currency, :code) == currency } ||
|
||||
entries.find { |b| b.dig(:currency, :code) == "USD" } ||
|
||||
|
||||
@@ -129,6 +129,14 @@ module SnaptradeAccount::DataHelpers
|
||||
end
|
||||
end
|
||||
|
||||
# SnapTrade positions nest the security data as holding.symbol.symbol
|
||||
def extract_symbol_data(data)
|
||||
symbol_wrapper = data[:symbol].is_a?(Hash) ? data[:symbol].with_indifferent_access : {}
|
||||
raw_symbol_data = symbol_wrapper[:symbol]
|
||||
|
||||
raw_symbol_data.is_a?(Hash) ? raw_symbol_data.with_indifferent_access : {}
|
||||
end
|
||||
|
||||
def extract_currency(data, symbol_data = {}, fallback_currency = nil)
|
||||
currency_data = data[:currency] || data["currency"] || symbol_data[:currency] || symbol_data["currency"]
|
||||
|
||||
|
||||
@@ -48,6 +48,12 @@ class SnaptradeAccount::HoldingsProcessor
|
||||
amount = parse_decimal(entry[:amount])
|
||||
next if amount.nil?
|
||||
|
||||
# SnapTrade's per-currency cash includes money market / sweep funds
|
||||
# that are also reported as `cash_equivalent: true` positions and
|
||||
# imported as holdings — remove the overlap so the synthetic cash
|
||||
# holding doesn't double count them.
|
||||
amount -= @snaptrade_account.cash_equivalent_position_value(entry[:currency])
|
||||
|
||||
security = Security.cash_for(account, currency: entry[:currency])
|
||||
|
||||
Rails.logger.info "SnaptradeAccount::HoldingsProcessor - Importing #{entry[:currency]} cash holding"
|
||||
|
||||
@@ -57,7 +57,7 @@ class SnaptradeAccount::Processor
|
||||
total_balance = calculate_total_balance
|
||||
cash_balance = calculate_cash_balance
|
||||
|
||||
Rails.logger.info "SnaptradeAccount::Processor - Balance update: total=#{total_balance}, cash=#{cash_balance}"
|
||||
Rails.logger.debug "SnaptradeAccount::Processor - Balance update: total=#{total_balance}, cash=#{cash_balance}"
|
||||
|
||||
# Update the cached fields on the account
|
||||
account.assign_attributes(
|
||||
@@ -81,16 +81,16 @@ class SnaptradeAccount::Processor
|
||||
# Calculate total from holdings + cash for accuracy
|
||||
# SnapTrade's current_balance can sometimes be stale or just the cash value
|
||||
holdings_value = calculate_holdings_value
|
||||
cash_value = snaptrade_account.cash_balance || 0
|
||||
cash_value = calculate_cash_balance
|
||||
|
||||
calculated_total = holdings_value + cash_value
|
||||
|
||||
# Use calculated total if we have holdings, otherwise trust API value
|
||||
if holdings_value > 0
|
||||
Rails.logger.info "SnaptradeAccount::Processor - Using calculated total: holdings=#{holdings_value} + cash=#{cash_value} = #{calculated_total}"
|
||||
Rails.logger.debug "SnaptradeAccount::Processor - Using calculated total: holdings=#{holdings_value} + cash=#{cash_value} = #{calculated_total}"
|
||||
calculated_total
|
||||
elsif snaptrade_account.current_balance.present?
|
||||
Rails.logger.info "SnaptradeAccount::Processor - Using API total: #{snaptrade_account.current_balance}"
|
||||
Rails.logger.debug "SnaptradeAccount::Processor - Using API total: #{snaptrade_account.current_balance}"
|
||||
snaptrade_account.current_balance
|
||||
else
|
||||
calculated_total
|
||||
@@ -98,11 +98,54 @@ class SnaptradeAccount::Processor
|
||||
end
|
||||
|
||||
def calculate_cash_balance
|
||||
# Use SnapTrade's cash_balance directly
|
||||
# Note: Can be negative for margin accounts
|
||||
cash = snaptrade_account.cash_balance
|
||||
Rails.logger.info "SnaptradeAccount::Processor - Cash balance from API: #{cash.inspect}"
|
||||
cash || BigDecimal("0")
|
||||
# Memoized: called for both the total and the cash figure, and the
|
||||
# cash-equivalent exclusion should only be recorded once per sync.
|
||||
@calculate_cash_balance ||= begin
|
||||
# Note: Can be negative for margin accounts
|
||||
cash = snaptrade_account.cash_balance
|
||||
Rails.logger.debug "SnaptradeAccount::Processor - Cash balance from API: #{cash.inspect}"
|
||||
|
||||
if cash.nil?
|
||||
BigDecimal("0")
|
||||
else
|
||||
# SnapTrade includes money market / sweep funds (e.g. Fidelity SPAXX) in
|
||||
# the balances endpoint's `cash` figure while ALSO reporting them as
|
||||
# positions with `cash_equivalent: true`. Those positions are imported as
|
||||
# holdings, so their value must come out of cash here — otherwise the
|
||||
# account total counts the same money twice. Subtract in the currency the
|
||||
# stored cash is actually denominated in (upsert_balances! can fall back
|
||||
# to a USD/first entry), not the account currency.
|
||||
cash_currency = snaptrade_account.primary_cash_currency
|
||||
cash_equivalent_value = snaptrade_account.cash_equivalent_position_value(cash_currency)
|
||||
|
||||
# No floor: negative cash is legitimate for margin accounts, and
|
||||
# clamping would silently skew the calculated total. The before/after
|
||||
# values in the debug entry let support distinguish real margin from
|
||||
# a stale holdings snapshot over-subtracting.
|
||||
adjusted_cash = cash - cash_equivalent_value
|
||||
|
||||
if cash_equivalent_value.nonzero?
|
||||
DebugLogEntry.capture(
|
||||
category: "provider_sync",
|
||||
level: "info",
|
||||
message: "Excluded cash-equivalent (money market) positions already counted in SnapTrade cash balance",
|
||||
source: self.class.name,
|
||||
provider_key: "snaptrade",
|
||||
family: snaptrade_account.snaptrade_item.family,
|
||||
account_provider: snaptrade_account.account_provider,
|
||||
metadata: {
|
||||
snaptrade_account_id: snaptrade_account.id,
|
||||
currency: cash_currency,
|
||||
cash_equivalent_value: cash_equivalent_value.to_s("F"),
|
||||
cash_balance_before: cash.to_s("F"),
|
||||
cash_balance_after: adjusted_cash.to_s("F")
|
||||
}
|
||||
)
|
||||
end
|
||||
|
||||
adjusted_cash
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def calculate_holdings_value
|
||||
@@ -129,11 +172,4 @@ class SnaptradeAccount::Processor
|
||||
extract_currency(data, extract_symbol_data(data), snaptrade_account.currency)
|
||||
end.uniq
|
||||
end
|
||||
|
||||
def extract_symbol_data(data)
|
||||
symbol_wrapper = data[:symbol].is_a?(Hash) ? data[:symbol].with_indifferent_access : {}
|
||||
raw_symbol_data = symbol_wrapper[:symbol]
|
||||
|
||||
raw_symbol_data.is_a?(Hash) ? raw_symbol_data.with_indifferent_access : {}
|
||||
end
|
||||
end
|
||||
|
||||
@@ -380,6 +380,155 @@ class SnaptradeAccountProcessorTest < ActiveSupport::TestCase
|
||||
assert eur_cash, "processor must run the holdings processor so secondary-currency cash is surfaced even with no stock holdings"
|
||||
end
|
||||
|
||||
# === Cash-equivalent positions (money market / sweep funds) ===
|
||||
#
|
||||
# SnapTrade includes money market funds in the balances endpoint's `cash`
|
||||
# figure AND returns them as positions with `cash_equivalent: true`.
|
||||
# Counting both inflates the account total (e.g. Fidelity SPAXX).
|
||||
|
||||
test "processor does not double count cash-equivalent positions included in cash balance" do
|
||||
security = securities(:aapl)
|
||||
Account.any_instance.stubs(:set_current_balance)
|
||||
|
||||
@snaptrade_account.update!(
|
||||
currency: "USD",
|
||||
cash_balance: BigDecimal("5000.00"),
|
||||
raw_holdings_payload: [
|
||||
{
|
||||
"symbol" => {
|
||||
"symbol" => { "symbol" => "SPAXX", "description" => "Fidelity Government Money Market Fund" }
|
||||
},
|
||||
"units" => "4000",
|
||||
"price" => "1.00",
|
||||
"currency" => "USD",
|
||||
"cash_equivalent" => true
|
||||
},
|
||||
{
|
||||
"symbol" => {
|
||||
"symbol" => { "symbol" => security.ticker, "description" => security.name }
|
||||
},
|
||||
"units" => "10",
|
||||
"price" => "150.00",
|
||||
"currency" => "USD"
|
||||
}
|
||||
],
|
||||
raw_activities_payload: []
|
||||
)
|
||||
|
||||
SnaptradeAccount::Processor.new(@snaptrade_account).process
|
||||
|
||||
@account.reload
|
||||
assert_equal BigDecimal("1000.00"), @account.cash_balance, "cash-equivalent position value must be subtracted from cash"
|
||||
assert_equal BigDecimal("6500.00"), @account.balance, "total must count the money market fund only once (1500 stock + 4000 MMF + 1000 cash)"
|
||||
|
||||
spaxx = @account.holdings.joins(:security).where(securities: { ticker: "SPAXX" }).order(date: :desc).first
|
||||
assert_not_nil spaxx, "the cash-equivalent position is still imported as a holding"
|
||||
assert_equal BigDecimal("4000"), spaxx.amount
|
||||
|
||||
debug_entries = DebugLogEntry.where(category: "provider_sync", provider_key: "snaptrade")
|
||||
assert_equal 1, debug_entries.count, "the exclusion is recorded once in /settings/debug"
|
||||
assert_equal "4000.0", debug_entries.first.metadata["cash_equivalent_value"]
|
||||
end
|
||||
|
||||
test "cash balance may go negative when cash-equivalent value exceeds reported cash and is recorded for support" do
|
||||
Account.any_instance.stubs(:set_current_balance)
|
||||
|
||||
# A stale holdings snapshot (or real margin) can make the cash-equivalent
|
||||
# value exceed reported cash. No floor is applied — negative cash is
|
||||
# legitimate for margin — but before/after values are recorded so support
|
||||
# can tell the two apart in /settings/debug.
|
||||
@snaptrade_account.update!(
|
||||
currency: "USD",
|
||||
cash_balance: BigDecimal("100.00"),
|
||||
raw_holdings_payload: [
|
||||
{
|
||||
"symbol" => {
|
||||
"symbol" => { "symbol" => "SPAXX", "description" => "Fidelity Government Money Market Fund" }
|
||||
},
|
||||
"units" => "4000",
|
||||
"price" => "1.00",
|
||||
"currency" => "USD",
|
||||
"cash_equivalent" => true
|
||||
}
|
||||
],
|
||||
raw_activities_payload: []
|
||||
)
|
||||
|
||||
SnaptradeAccount::Processor.new(@snaptrade_account).process
|
||||
|
||||
@account.reload
|
||||
assert_equal BigDecimal("-3900.00"), @account.cash_balance, "negative cash is preserved, not floored"
|
||||
|
||||
entry = DebugLogEntry.where(category: "provider_sync", provider_key: "snaptrade").order(created_at: :desc).first
|
||||
assert_equal "100.0", entry.metadata["cash_balance_before"]
|
||||
assert_equal "-3900.0", entry.metadata["cash_balance_after"]
|
||||
end
|
||||
|
||||
test "cash-equivalent positions are subtracted in the stored cash currency when it falls back to USD" do
|
||||
Account.any_instance.stubs(:set_current_balance)
|
||||
|
||||
# CAD account with no CAD cash entry: upsert_balances!' USD fallback means
|
||||
# cash_balance is denominated in USD, so the USD money market fund must be
|
||||
# subtracted from it even though it doesn't match the account currency.
|
||||
@snaptrade_account.update!(
|
||||
currency: "CAD",
|
||||
current_balance: BigDecimal("10000.00"),
|
||||
cash_balance: BigDecimal("5000.00"),
|
||||
raw_balances_payload: [
|
||||
{ "currency" => { "code" => "USD" }, "cash" => "5000.00" }
|
||||
],
|
||||
raw_holdings_payload: [
|
||||
{
|
||||
"symbol" => {
|
||||
"symbol" => { "symbol" => "SPAXX", "description" => "Fidelity Government Money Market Fund" }
|
||||
},
|
||||
"units" => "4000",
|
||||
"price" => "1.00",
|
||||
"currency" => "USD",
|
||||
"cash_equivalent" => true
|
||||
}
|
||||
],
|
||||
raw_activities_payload: []
|
||||
)
|
||||
|
||||
SnaptradeAccount::Processor.new(@snaptrade_account).process
|
||||
|
||||
@account.reload
|
||||
assert_equal BigDecimal("1000.00"), @account.cash_balance, "USD cash-equivalent position must be subtracted from the USD-denominated cash balance"
|
||||
assert_equal BigDecimal("10000.00"), @account.balance, "multi-currency holdings still use the API total"
|
||||
end
|
||||
|
||||
test "cash-equivalent positions in a non-primary currency reduce that currency's synthetic cash holding" do
|
||||
@snaptrade_account.update!(
|
||||
currency: "USD",
|
||||
cash_balance: BigDecimal("1500.00"),
|
||||
raw_balances_payload: [
|
||||
{ "currency" => { "code" => "USD" }, "cash" => "1500.00" },
|
||||
{ "currency" => { "code" => "EUR" }, "cash" => "800.00" }
|
||||
],
|
||||
raw_holdings_payload: [
|
||||
{
|
||||
"symbol" => {
|
||||
"symbol" => { "symbol" => "EURMM", "description" => "Euro Money Market Fund" }
|
||||
},
|
||||
"units" => "300",
|
||||
"price" => "1.00",
|
||||
"currency" => "EUR",
|
||||
"cash_equivalent" => true
|
||||
}
|
||||
]
|
||||
)
|
||||
|
||||
SnaptradeAccount::HoldingsProcessor.new(@snaptrade_account).process
|
||||
|
||||
eur_cash = @account.holdings.joins(:security).where(securities: { kind: "cash" }, currency: "EUR").order(date: :desc).first
|
||||
assert_not_nil eur_cash
|
||||
assert_equal BigDecimal("500"), eur_cash.qty, "EUR synthetic cash must exclude the EUR cash-equivalent position (800 - 300)"
|
||||
|
||||
eur_mmf = @account.holdings.joins(:security).where(securities: { ticker: "EURMM" }).order(date: :desc).first
|
||||
assert_not_nil eur_mmf, "the EUR cash-equivalent position is still imported as a holding"
|
||||
end
|
||||
|
||||
test "non-primary cash holding is not duplicated across repeated syncs" do
|
||||
@snaptrade_account.update!(
|
||||
currency: "USD",
|
||||
|
||||
Reference in New Issue
Block a user