fix: allow refreshes from the same source for cost basis updates (#917)

* fix: allow refreshes from the same source for cost basis updates

* test: update cost basis priority expectations

app/models/holding.rb

@@ -112,7 +112,9 @@ class Holding < ApplicationRecord
       return new_source == "calculated"
     end
 
-    new_priority > cost_basis_source_priority
+    # Allow refreshes from the same source (e.g., new trades change calculated cost basis,
+    # or providers send updated cost basis).
+    new_priority >= cost_basis_source_priority
   end
 
   # Set cost_basis from user input (locks the value)

app/models/holding/cost_basis_reconciler.rb

@@ -40,6 +40,15 @@ class Holding::CostBasisReconciler
     # Check priority - can the incoming source replace the existing?
     if existing_holding.cost_basis_replaceable_by?(incoming_source)
       if incoming_cost_basis.present?
+        # Avoid writes when nothing would change (common when re-materializing)
+        if existing_holding.cost_basis_source == incoming_source && existing_holding.cost_basis == incoming_cost_basis
+          return {
+            cost_basis: existing_holding.cost_basis,
+            cost_basis_source: existing_holding.cost_basis_source,
+            should_update: false
+          }
+        end
+
         return {
           cost_basis: incoming_cost_basis,
           cost_basis_source: incoming_source,

test/models/holding/materializer_test.rb

@@ -75,4 +75,22 @@ class Holding::MaterializerTest < ActiveSupport::TestCase
     assert_equal BigDecimal("180.00"), holding.cost_basis,
       "Trade-derived cost_basis should override provider cost_basis when available"
   end
+
+  test "recalculates calculated cost_basis when new trades are added" do
+    date = Date.current
+
+    create_trade(@aapl, account: @account, qty: 1, price: 3000, date: date)
+    Holding::Materializer.new(@account, strategy: :forward).materialize_holdings
+
+    holding = @account.holdings.find_by!(security: @aapl, date: date, currency: "USD")
+    assert_equal "calculated", holding.cost_basis_source
+    assert_equal BigDecimal("3000.0"), holding.cost_basis
+
+    create_trade(@aapl, account: @account, qty: 1, price: 2500, date: date)
+    Holding::Materializer.new(@account, strategy: :forward).materialize_holdings
+
+    holding.reload
+    assert_equal "calculated", holding.cost_basis_source
+    assert_equal BigDecimal("2750.0"), holding.cost_basis
+  end
 end

test/models/holding_test.rb

@@ -129,17 +129,18 @@ class HoldingTest < ActiveSupport::TestCase
     assert_not @amzn.cost_basis_replaceable_by?("manual")
   end
 
-  test "cost_basis_replaceable_by? respects priority hierarchy" do
-    # Provider data can be replaced by calculated or manual
+  test "cost_basis_replaceable_by? respects priority hierarchy and allows refreshes" do
+    # Provider data can be replaced by higher-priority sources (calculated/manual)
+    # and can be refreshed by provider again.
     @amzn.update!(cost_basis: 200, cost_basis_source: "provider", cost_basis_locked: false)
     assert @amzn.cost_basis_replaceable_by?("calculated")
     assert @amzn.cost_basis_replaceable_by?("manual")
-    assert_not @amzn.cost_basis_replaceable_by?("provider")
+    assert @amzn.cost_basis_replaceable_by?("provider")
 
-    # Calculated data can be replaced by manual only
+    # Calculated data can be replaced by manual and can be refreshed by calculated again.
     @amzn.update!(cost_basis: 200, cost_basis_source: "calculated", cost_basis_locked: false)
     assert @amzn.cost_basis_replaceable_by?("manual")
-    assert_not @amzn.cost_basis_replaceable_by?("calculated")
+    assert @amzn.cost_basis_replaceable_by?("calculated")
     assert_not @amzn.cost_basis_replaceable_by?("provider")
 
     # Manual data when LOCKED cannot be replaced by anything