diff --git a/app/models/holding/materializer.rb b/app/models/holding/materializer.rb index 5832e2033..689a43ad1 100644 --- a/app/models/holding/materializer.rb +++ b/app/models/holding/materializer.rb @@ -88,8 +88,27 @@ class Holding::Materializer "cost_basis_source" => reconciled[:cost_basis_source] ) else - # No cost_basis to set, or existing is better - don't touch cost_basis fields - holdings_to_upsert_without_cost << base_attrs + # No new calculated value — fall back to the most recent provider + # cost_basis for this security on or before the holding date. + # Calculated/manual values outrank a provider carry-forward. + existing_source = existing&.cost_basis_source + preserve_existing = existing&.cost_basis.present? && %w[calculated manual].include?(existing_source) + + if preserve_existing + holdings_to_upsert_without_cost << base_attrs + else + carried = carry_forward_provider_cost_basis(holding) + + if carried && (existing&.cost_basis != carried || existing_source != "provider") + holdings_to_upsert_with_cost << base_attrs.merge( + "cost_basis" => carried, + "cost_basis_source" => "provider" + ) + else + # No cost_basis to set, or existing is better - don't touch cost_basis fields + holdings_to_upsert_without_cost << base_attrs + end + end end end @@ -165,6 +184,50 @@ class Holding::Materializer [ holding.account_id || account.id, holding.security_id, holding.date, holding.currency ] end + # Returns the most recent provider-supplied cost_basis for the given holding's + # security on or before its date, converted to the holding's currency. + # Used to backfill calculated rows past the provider's last snapshot so + # reports keep showing trend data. + # + # Provider and calculated rows can be denominated in different currencies + # (e.g., IBKR reports USD holdings while the reverse calculator converts to + # the account's base currency). When they differ, the cost_basis is converted + # at the snapshot date — the same convention ReverseCalculator uses for trade + # prices — so the result is consistent with trade-derived cost_basis values. + def carry_forward_provider_cost_basis(holding) + snapshots = provider_cost_basis_snapshots[holding.security_id] + return nil if snapshots.blank? + + result = nil + snapshots.each do |snap_date, cost_basis, snap_currency| + break if snap_date > holding.date + result = [ cost_basis, snap_currency, snap_date ] + end + return nil unless result + + cost_basis, snap_currency, snap_date = result + return cost_basis if snap_currency == holding.currency + + Money.new(cost_basis, snap_currency).exchange_to(holding.currency, date: snap_date).amount + rescue Money::ConversionError + nil + end + + def provider_cost_basis_snapshots + @provider_cost_basis_snapshots ||= begin + ids = @holdings.map(&:security_id).uniq + account.holdings + .where.not(account_provider_id: nil) + .where.not(cost_basis: nil) + .where(security_id: ids) + .order(:date) # ascending required: carry_forward_provider_cost_basis scans and breaks on snap_date > holding.date + .pluck(:security_id, :currency, :date, :cost_basis) + .each_with_object(Hash.new { |h, k| h[k] = [] }) do |(security_id, currency, date, cost_basis), memo| + memo[security_id] << [ date, cost_basis, currency ] + end + end + end + def purge_stale_holdings portfolio_security_ids = account.trades.distinct.pluck(:security_id) diff --git a/test/models/holding/materializer_test.rb b/test/models/holding/materializer_test.rb index 605f00478..e975de3cd 100644 --- a/test/models/holding/materializer_test.rb +++ b/test/models/holding/materializer_test.rb @@ -155,6 +155,203 @@ class Holding::MaterializerTest < ActiveSupport::TestCase assert_equal [ account_provider.id ], today_holdings.pluck(:account_provider_id) end + test "carries forward provider cost_basis to calculated rows past the provider snapshot date" do + coinstats_item = @family.coinstats_items.create!(name: "CoinStats", api_key: "test-key") + coinstats_account = coinstats_item.coinstats_accounts.create!(name: "Brokerage", currency: "USD") + account_provider = AccountProvider.create!(account: @account, provider: coinstats_account) + + # Provider snapshot two days ago with known cost basis, but no trades. + # This mirrors IBKR Flex where the export ends on Friday but today is Sunday. + Holding.create!( + account: @account, + security: @aapl, + qty: 10, + price: 200, + amount: 2000, + currency: "USD", + date: 2.days.ago.to_date, + account_provider: account_provider, + cost_basis: BigDecimal("125.50"), + cost_basis_source: "provider" + ) + + Holding::Materializer.new(@account, strategy: :reverse).materialize_holdings + + today_holding = @account.holdings.find_by!(security: @aapl, date: Date.current, currency: "USD") + assert_nil today_holding.account_provider_id, + "Today's row is calculated, not a provider snapshot" + assert_equal BigDecimal("125.50"), today_holding.cost_basis, + "Today's calculated row should inherit the provider's cost_basis so trend/return calcs work" + assert_equal "provider", today_holding.cost_basis_source + end + + test "does not overwrite an existing calculated cost_basis with provider carry-forward" do + coinstats_item = @family.coinstats_items.create!(name: "CoinStats", api_key: "test-key") + coinstats_account = coinstats_item.coinstats_accounts.create!(name: "Brokerage", currency: "USD") + account_provider = AccountProvider.create!(account: @account, provider: coinstats_account) + + Holding.create!( + account: @account, + security: @aapl, + qty: 10, + price: 200, + amount: 2000, + currency: "USD", + date: 2.days.ago.to_date, + account_provider: account_provider, + cost_basis: BigDecimal("125.50"), + cost_basis_source: "provider" + ) + + # Pre-existing calculated row for today (e.g., from a prior trade-derived run) + Holding.create!( + account: @account, + security: @aapl, + qty: 10, + price: 200, + amount: 2000, + currency: "USD", + date: Date.current, + cost_basis: BigDecimal("180.00"), + cost_basis_source: "calculated" + ) + + Holding::Materializer.new(@account, strategy: :reverse).materialize_holdings + + today_holding = @account.holdings.find_by!(security: @aapl, date: Date.current, currency: "USD") + assert_equal BigDecimal("180.00"), today_holding.cost_basis, + "Existing calculated cost_basis must beat provider carry-forward" + assert_equal "calculated", today_holding.cost_basis_source + end + + test "refreshes stale provider carry-forward when a newer provider snapshot arrives" do + coinstats_item = @family.coinstats_items.create!(name: "CoinStats", api_key: "test-key") + coinstats_account = coinstats_item.coinstats_accounts.create!(name: "Brokerage", currency: "USD") + account_provider = AccountProvider.create!(account: @account, provider: coinstats_account) + + # With no entries, start_date = yesterday, so materializer only descends to + # yesterday. Use an older date so the second snapshot doesn't land on a date + # the materializer already owns. + Holding.create!( + account: @account, security: @aapl, qty: 10, price: 200, amount: 2000, + currency: "USD", date: 5.days.ago.to_date, + account_provider: account_provider, + cost_basis: BigDecimal("100.00"), cost_basis_source: "provider" + ) + + Holding::Materializer.new(@account, strategy: :reverse).materialize_holdings + + today_holding = @account.holdings.find_by!(security: @aapl, date: Date.current, currency: "USD") + assert_equal BigDecimal("100.00"), today_holding.cost_basis + + # Provider publishes a newer snapshot with an updated cost_basis on a date + # that falls outside the materializer's window (older than start_date). + Holding.create!( + account: @account, security: @aapl, qty: 10, price: 210, amount: 2100, + currency: "USD", date: 3.days.ago.to_date, + account_provider: account_provider, + cost_basis: BigDecimal("150.00"), cost_basis_source: "provider" + ) + + Holding::Materializer.new(@account, strategy: :reverse).materialize_holdings + + today_holding.reload + assert_equal BigDecimal("150.00"), today_holding.cost_basis, + "Carry-forward should update to the newer provider snapshot value" + assert_equal "provider", today_holding.cost_basis_source + end + + test "carry-forward is a no-op for forward-strategy accounts without provider holdings" do + create_trade(@aapl, account: @account, qty: 5, price: 200, date: Date.current) + + assert_nothing_raised do + Holding::Materializer.new(@account, strategy: :forward).materialize_holdings + end + + today_holding = @account.holdings.find_by!(security: @aapl, date: Date.current, currency: "USD") + assert_equal "calculated", today_holding.cost_basis_source + assert_equal BigDecimal("200.00"), today_holding.cost_basis, + "Forward strategy with no provider rows should compute cost_basis from trades normally" + end + + test "does not overwrite a zero-valued manual cost_basis with provider carry-forward" do + coinstats_item = @family.coinstats_items.create!(name: "CoinStats", api_key: "test-key") + coinstats_account = coinstats_item.coinstats_accounts.create!(name: "Brokerage", currency: "USD") + account_provider = AccountProvider.create!(account: @account, provider: coinstats_account) + + Holding.create!( + account: @account, security: @aapl, + qty: 10, price: 200, amount: 2000, currency: "USD", + date: 2.days.ago.to_date, + account_provider: account_provider, + cost_basis: BigDecimal("125.50"), cost_basis_source: "provider" + ) + + # Free shares: legitimate zero-cost basis recorded manually + Holding.create!( + account: @account, security: @aapl, + qty: 10, price: 200, amount: 2000, currency: "USD", + date: Date.current, + cost_basis: BigDecimal("0"), cost_basis_source: "manual" + ) + + Holding::Materializer.new(@account, strategy: :reverse).materialize_holdings + + today_holding = @account.holdings.find_by!(security: @aapl, date: Date.current, currency: "USD") + assert_equal BigDecimal("0"), today_holding.cost_basis, + "Zero-valued manual cost_basis (e.g., free shares) must not be overwritten by provider carry-forward" + assert_equal "manual", today_holding.cost_basis_source + end + + test "carry-forward converts provider cost_basis currency when provider and calculated currencies differ" do + snap_date = 2.days.ago.to_date + ExchangeRate.create!(from_currency: "EUR", to_currency: "USD", date: snap_date, rate: 1.2) + + coinstats_item = @family.coinstats_items.create!(name: "CoinStats", api_key: "test-key") + coinstats_account = coinstats_item.coinstats_accounts.create!(name: "Brokerage", currency: "EUR") + account_provider = AccountProvider.create!(account: @account, provider: coinstats_account) + + Holding.create!( + account: @account, security: @aapl, + qty: 10, price: 200, amount: 2000, currency: "EUR", + date: snap_date, + account_provider: account_provider, + cost_basis: BigDecimal("100.00"), cost_basis_source: "provider" + ) + + Holding::Materializer.new(@account, strategy: :reverse).materialize_holdings + + today_holding = @account.holdings.find_by!(security: @aapl, date: Date.current, currency: "USD") + assert_in_delta BigDecimal("120.00"), today_holding.cost_basis, BigDecimal("0.01"), + "Provider cost_basis in EUR should be converted to USD at the snapshot-date exchange rate" + assert_equal "provider", today_holding.cost_basis_source + end + + test "carry-forward skips provider cost_basis when FX conversion raises Money::ConversionError" do + snap_date = 2.days.ago.to_date + # No ExchangeRate created — EUR→USD conversion will raise Money::ConversionError + + coinstats_item = @family.coinstats_items.create!(name: "CoinStats", api_key: "test-key") + coinstats_account = coinstats_item.coinstats_accounts.create!(name: "Brokerage", currency: "EUR") + account_provider = AccountProvider.create!(account: @account, provider: coinstats_account) + + Holding.create!( + account: @account, security: @aapl, + qty: 10, price: 200, amount: 2000, currency: "EUR", + date: snap_date, + account_provider: account_provider, + cost_basis: BigDecimal("100.00"), cost_basis_source: "provider" + ) + + assert_nothing_raised do + Holding::Materializer.new(@account, strategy: :reverse).materialize_holdings + end + + today_holding = @account.holdings.find_by!(security: @aapl, date: Date.current, currency: "USD") + assert_nil today_holding.cost_basis, + "Carry-forward should be skipped gracefully when currency conversion fails" + end + test "preserves same-day non-provider holdings for securities absent from the provider snapshot" do ExchangeRate.create!(from_currency: "EUR", to_currency: "USD", date: Date.current, rate: 1.2)