Fix avg_cost to return per-share cost basis (#1692)

* Fix avg_cost to return per-share cost basis

* Revert "Fix avg_cost to return per-share cost basis"

This reverts commit 38c438a614.

* Normalize SimpleFIN holding cost basis

* Track SimpleFIN cost basis source field

* Add SimpleFIN cost basis normalization tests

* Update SimpleFIN value cost basis expectation

* Handle missing SimpleFIN quantity for cost basis

* Ignore missing SimpleFIN cost basis fields

* Fix SimpleFIN holdings processor test setup

app/models/simplefin_account/investments/holdings_processor.rb

@@ -47,7 +47,8 @@ class SimplefinAccount::Investments::HoldingsProcessor
         # which would cause the system to display average cost as current price. (GH #1182)
         qty = parse_decimal(any_of(simplefin_holding, %w[shares quantity qty units]))
         market_value = parse_decimal(any_of(simplefin_holding, %w[market_value current_value]))
-        cost_basis = parse_decimal(any_of(simplefin_holding, %w[cost_basis basis total_cost value]))
+        raw_cost_basis, cost_basis_source_key = cost_basis_from(simplefin_holding)
+        cost_basis = normalize_cost_basis(raw_cost_basis, qty, cost_basis_source_key)
 
         # Derive price from market_value when possible; otherwise fall back to any price field
         fallback_price = parse_decimal(any_of(simplefin_holding, %w[purchase_price price unit_price average_cost avg_cost]))
@@ -112,6 +113,32 @@ class SimplefinAccount::Investments::HoldingsProcessor
       simplefin_account.raw_holdings_payload || []
     end
 
+    def cost_basis_from(simplefin_holding)
+      %w[cost_basis basis total_cost value].each do |key|
+        raw = simplefin_holding[key]
+        next if raw.nil? || raw.to_s.strip.empty?
+
+        return [ parse_decimal(raw), key ]
+      end
+
+      [ nil, nil ]
+    end
+
+    # Sure stores holding cost_basis as per-share average cost. Some SimpleFIN
+    # providers expose total position basis via total_cost/value, so normalize only
+    # when the selected provider field is known to represent total position basis.
+    def normalize_cost_basis(raw_cost_basis, qty, source_key)
+      return nil if raw_cost_basis.nil?
+
+      if %w[total_cost value].include?(source_key)
+        return nil unless qty.to_d.positive?
+
+        raw_cost_basis / qty
+      else
+        raw_cost_basis
+      end
+    end
+
     def resolve_security(symbol, description)
       # Normalize crypto tickers to a distinct namespace so they don't collide with equities
       sym = symbol.to_s.upcase

test/jobs/simplefin_holdings_apply_job_test.rb

@@ -124,7 +124,7 @@ class SimplefinHoldingsApplyJobTest < ActiveSupport::TestCase
     # Price derived from market_value
     assert_in_delta 500.0, holding.price.to_f, 0.01
 
-    # cost_basis should fall back to "value" field (45000)
-    assert_in_delta 45000.0, holding.cost_basis.to_f, 0.01
+    # cost_basis should fall back to "value" field and normalize total basis to per-share basis
+    assert_in_delta 450.0, holding.cost_basis.to_f, 0.01
   end
 end

test/models/simplefin_account/investments/holdings_processor_test.rb

@@ -0,0 +1,95 @@
+require "test_helper"
+
+class SimplefinAccount::Investments::HoldingsProcessorTest < ActiveSupport::TestCase
+  setup do
+    @processor = SimplefinAccount::Investments::HoldingsProcessor.new(nil)
+  end
+
+  test "cost_basis source is used unchanged as per share basis" do
+    payload = {
+      "cost_basis" => "16.61",
+      "total_cost" => "9588.61",
+      "value" => "10108.16"
+    }
+
+    raw_cost_basis, source_key = @processor.send(:cost_basis_from, payload)
+    cost_basis = @processor.send(:normalize_cost_basis, raw_cost_basis, BigDecimal("577.279"), source_key)
+
+    assert_equal BigDecimal("16.61"), cost_basis
+    assert_equal "cost_basis", source_key
+  end
+
+  test "basis source is used unchanged as per share basis" do
+    payload = {
+      "basis" => "16.61",
+      "total_cost" => "9588.61"
+    }
+
+    raw_cost_basis, source_key = @processor.send(:cost_basis_from, payload)
+    cost_basis = @processor.send(:normalize_cost_basis, raw_cost_basis, BigDecimal("577.279"), source_key)
+
+    assert_equal BigDecimal("16.61"), cost_basis
+    assert_equal "basis", source_key
+  end
+
+  test "total_cost source is normalized to per share basis" do
+    payload = {
+      "total_cost" => "9588.61"
+    }
+
+    raw_cost_basis, source_key = @processor.send(:cost_basis_from, payload)
+    cost_basis = @processor.send(:normalize_cost_basis, raw_cost_basis, BigDecimal("577.279"), source_key)
+
+    assert_equal BigDecimal("9588.61") / BigDecimal("577.279"), cost_basis
+    assert_equal "total_cost", source_key
+  end
+
+  test "value source is normalized to per share basis" do
+    payload = {
+      "value" => "9588.61"
+    }
+
+    raw_cost_basis, source_key = @processor.send(:cost_basis_from, payload)
+    cost_basis = @processor.send(:normalize_cost_basis, raw_cost_basis, BigDecimal("577.279"), source_key)
+
+    assert_equal BigDecimal("9588.61") / BigDecimal("577.279"), cost_basis
+    assert_equal "value", source_key
+  end
+
+  test "total cost source with zero quantity returns nil" do
+    payload = {
+      "total_cost" => "9588.61"
+    }
+
+    raw_cost_basis, source_key = @processor.send(:cost_basis_from, payload)
+    cost_basis = @processor.send(:normalize_cost_basis, raw_cost_basis, BigDecimal("0"), source_key)
+
+    assert_nil cost_basis
+    assert_equal "total_cost", source_key
+  end
+
+  test "total cost source with nil quantity returns nil" do
+    payload = {
+      "total_cost" => "9588.61"
+    }
+
+    raw_cost_basis, source_key = @processor.send(:cost_basis_from, payload)
+    cost_basis = @processor.send(:normalize_cost_basis, raw_cost_basis, nil, source_key)
+
+    assert_nil cost_basis
+    assert_equal "total_cost", source_key
+  end
+
+  test "missing cost basis fields return nil" do
+    payload = {
+      "market_value" => "10108.16"
+    }
+
+    raw_cost_basis, source_key = @processor.send(:cost_basis_from, payload)
+    cost_basis = @processor.send(:normalize_cost_basis, raw_cost_basis, BigDecimal("577.279"), source_key)
+
+    assert_nil raw_cost_basis
+    assert_nil source_key
+    assert_nil cost_basis
+  end
+end