From 39e9cf4e3d1f088381a96bb694cf7b8517ce1d4d Mon Sep 17 00:00:00 2001 From: Rick Gray Date: Tue, 21 Jul 2026 16:29:58 -0500 Subject: [PATCH] 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 --- app/models/snaptrade_account.rb | 48 +++++- app/models/snaptrade_account/data_helpers.rb | 8 + .../snaptrade_account/holdings_processor.rb | 6 + app/models/snaptrade_account/processor.rb | 68 ++++++-- .../snaptrade_account_processor_test.rb | 149 ++++++++++++++++++ 5 files changed, 258 insertions(+), 21 deletions(-) diff --git a/app/models/snaptrade_account.rb b/app/models/snaptrade_account.rb index 54d6f2822..a57d9323b 100644 --- a/app/models/snaptrade_account.rb +++ b/app/models/snaptrade_account.rb @@ -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" } || diff --git a/app/models/snaptrade_account/data_helpers.rb b/app/models/snaptrade_account/data_helpers.rb index c0f1d4c23..c542e5308 100644 --- a/app/models/snaptrade_account/data_helpers.rb +++ b/app/models/snaptrade_account/data_helpers.rb @@ -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"] diff --git a/app/models/snaptrade_account/holdings_processor.rb b/app/models/snaptrade_account/holdings_processor.rb index cb59932a0..f036a2007 100644 --- a/app/models/snaptrade_account/holdings_processor.rb +++ b/app/models/snaptrade_account/holdings_processor.rb @@ -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" diff --git a/app/models/snaptrade_account/processor.rb b/app/models/snaptrade_account/processor.rb index 5b9b578fd..27e88e77a 100644 --- a/app/models/snaptrade_account/processor.rb +++ b/app/models/snaptrade_account/processor.rb @@ -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 diff --git a/test/models/snaptrade_account_processor_test.rb b/test/models/snaptrade_account_processor_test.rb index d6926d250..820d6d54e 100644 --- a/test/models/snaptrade_account_processor_test.rb +++ b/test/models/snaptrade_account_processor_test.rb @@ -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",