Fix false positive inactive hints for SimpleFin accounts during chunked imports (#573)

* Add tests and logic for zero balance handling and inactivity detection

- Updated `SimplefinItem::ImporterInactiveTest` to include cases for chunked imports, credit cards, and loans.
- Added logic to skip zero balance detection for liability accounts (e.g., credit cards, loans).
- Ensured zero balance runs are counted only once per sync to avoid false positives during chunked imports.

* Add nil safety

---------

Co-authored-by: Josh Waldrep <joshua.waldrep5+github@gmail.com>

app/models/simplefin_item/importer.rb

@@ -158,6 +158,16 @@ class SimplefinItem::Importer
         return
       end
 
+      # Skip zero balance detection for liability accounts (CreditCard, Loan) where
+      # 0 balance with no holdings is normal (paid off card/loan)
+      account_type = simplefin_account.current_account&.accountable_type
+      return if %w[CreditCard Loan].include?(account_type)
+
+      # Only count each account once per sync run to avoid false positives during
+      # chunked imports (which process the same account multiple times)
+      zero_balance_seen_keys << key if zeroish_balance && no_holdings
+      return if zero_balance_seen_keys.count(key) > 1
+
       if zeroish_balance && no_holdings
         stats["zero_runs"][key] = stats["zero_runs"][key].to_i + 1
         # Cap to avoid unbounded growth
@@ -173,6 +183,11 @@ class SimplefinItem::Importer
       end
     end
 
+    # Track accounts that have been flagged for zero balance in this sync run
+    def zero_balance_seen_keys
+      @zero_balance_seen_keys ||= []
+    end
+
     # Track seen error fingerprints during a single importer run to avoid double counting
     def seen_errors
       @seen_errors ||= Set.new

test/models/simplefin_item/importer_inactive_test.rb

@@ -19,37 +19,33 @@ class SimplefinItem::ImporterInactiveTest < ActiveSupport::TestCase
     assert stats.dig("inactive", "a1"), "should be inactive when closed flag present"
   end
 
-  test "marks inactive after three consecutive zero runs with no holdings" do
+  test "counts zero runs once per sync even with multiple imports" do
     account_data = { id: "a2", name: "Dormant", balance: 0, "available-balance": 0, currency: "USD" }
 
-    2.times { importer.send(:import_account, account_data) }
-    stats = @sync.reload.sync_stats
-    assert_equal 2, stats.dig("zero_runs", "a2"), "should count zero runs"
-    assert_equal false, stats.dig("inactive", "a2"), "should not be inactive before threshold"
+    # Multiple imports in the same sync (simulating chunked imports) should only count once
+    5.times { importer.send(:import_account, account_data) }
 
-    importer.send(:import_account, account_data)
     stats = @sync.reload.sync_stats
-    assert_equal true, stats.dig("inactive", "a2"), "should be inactive at threshold"
+    assert_equal 1, stats.dig("zero_runs", "a2"), "should only count once per sync despite multiple imports"
+    assert_equal false, stats.dig("inactive", "a2"), "should not be inactive after single count"
   end
 
-  test "resets zero_runs_count and inactive when activity returns" do
+  test "resets zero_runs and inactive when activity returns" do
     account_data = { id: "a3", name: "Dormant", balance: 0, "available-balance": 0, currency: "USD" }
-    3.times { importer.send(:import_account, account_data) }
-    stats = @sync.reload.sync_stats
-    assert_equal true, stats.dig("inactive", "a3")
+    importer.send(:import_account, account_data)
 
-    # Activity returns: non-zero balance or holdings
+    stats = @sync.reload.sync_stats
+    assert_equal 1, stats.dig("zero_runs", "a3")
+
+    # Activity returns: non-zero balance
     active_data = { id: "a3", name: "Dormant", balance: 10, currency: "USD" }
     importer.send(:import_account, active_data)
+
     stats = @sync.reload.sync_stats
     assert_equal 0, stats.dig("zero_runs", "a3")
     assert_equal false, stats.dig("inactive", "a3")
   end
-end
 
-
-# Additional regression: no balances present should not increment zero_runs or mark inactive
-class SimplefinItem::ImporterInactiveTest < ActiveSupport::TestCase
   test "does not count zero run when both balances are missing and no holdings" do
     account_data = { id: "a4", name: "Unknown", currency: "USD" } # no balance keys, no holdings
 
@@ -59,4 +55,64 @@ class SimplefinItem::ImporterInactiveTest < ActiveSupport::TestCase
     assert_equal 0, stats.dig("zero_runs", "a4").to_i
     assert_equal false, stats.dig("inactive", "a4")
   end
+
+  test "skips zero balance detection for credit cards" do
+    # Create a SimplefinAccount linked to a CreditCard account
+    sfa = SimplefinAccount.create!(
+      simplefin_item: @item,
+      name: "Paid Off Card",
+      account_id: "cc1",
+      account_type: "credit",
+      currency: "USD",
+      current_balance: 0
+    )
+
+    credit_card = CreditCard.create!
+    account = @family.accounts.create!(
+      name: "Paid Off Card",
+      balance: 0,
+      currency: "USD",
+      accountable: credit_card,
+      simplefin_account_id: sfa.id
+    )
+
+    account_data = { id: "cc1", name: "Paid Off Card", balance: 0, "available-balance": 0, currency: "USD" }
+
+    # Even with zero balance and no holdings, credit cards should not trigger the counter
+    importer.send(:import_account, account_data)
+    stats = @sync.reload.sync_stats
+
+    assert_nil stats.dig("zero_runs", "cc1"), "should not count zero runs for credit cards"
+    assert_equal false, stats.dig("inactive", "cc1")
+  end
+
+  test "skips zero balance detection for loans" do
+    # Create a SimplefinAccount linked to a Loan account
+    sfa = SimplefinAccount.create!(
+      simplefin_item: @item,
+      name: "Paid Off Loan",
+      account_id: "loan1",
+      account_type: "loan",
+      currency: "USD",
+      current_balance: 0
+    )
+
+    loan = Loan.create!
+    account = @family.accounts.create!(
+      name: "Paid Off Loan",
+      balance: 0,
+      currency: "USD",
+      accountable: loan,
+      simplefin_account_id: sfa.id
+    )
+
+    account_data = { id: "loan1", name: "Paid Off Loan", balance: 0, "available-balance": 0, currency: "USD" }
+
+    # Even with zero balance and no holdings, loans should not trigger the counter
+    importer.send(:import_account, account_data)
+    stats = @sync.reload.sync_stats
+
+    assert_nil stats.dig("zero_runs", "loan1"), "should not count zero runs for loans"
+    assert_equal false, stats.dig("inactive", "loan1")
+  end
 end