Add overpayment detection for SimpleFIN liabilities (default ON) with heuristic-based classification and robust fallbacks (#412)

* Add liability balance normalization logic with comprehensive tests

- Updated `SimplefinAccount::Processor` to normalize liability balances based on observed values, ensuring correct handling of debts and overpayments.
- Enhanced `SimplefinItem::Importer` to apply similar normalization rules during imports, improving consistency.
- Added multiple test cases in `SimplefinAccountProcessorTest` to validate edge cases for liabilities and mixed-sign scenarios.
- Introduced helper methods (`to_decimal`, `same_sign?`) to simplify numeric operations in normalization logic.

* Add overpayment detection for liabilities with heuristic-based classification

- Introduced `SimplefinAccount::Liabilities::OverpaymentAnalyzer` to classify liability balances as credit, debt, or unknown using transaction history.
- Updated `SimplefinAccount::Processor` and `SimplefinItem::Importer` to integrate heuristic-based balance normalization with fallback logic for ambiguous cases.
- Added comprehensive unit tests in `OverpaymentAnalyzerTest` to validate classification logic and edge cases.
- Enhanced logging and observability around classification results and fallback scenarios.

* Refactor liability handling for better fallback consistency

- Updated `sticky_key` method in `OverpaymentAnalyzer` to handle missing `@sfa.id` with a default value.
- Enhanced `SimplefinAccount::Processor` to use `with_indifferent_access` for `raw_payload` and `org_data`, improving robustness in liability type inference.

* Extract numeric helper methods into `SimplefinNumericHelpers` concern and apply across models

- Moved `to_decimal` and `same_sign?` methods into a new `SimplefinNumericHelpers` concern for reuse.
- Updated `OverpaymentAnalyzer`, `Processor`, and `Importer` to include the concern and remove redundant method definitions.
- Added empty fixtures for `simplefin_accounts` and `simplefin_items` to ensure test isolation.
- Refactored `OverpaymentAnalyzerTest` to reduce fixture dependencies and ensure cleanup of created records.

* Refactor overpayment detection logic for clarity and fallback consistency

- Simplified `enabled?` method in `OverpaymentAnalyzer` for clearer precedence order (Setting > ENV > default).
- Added `parse_bool` helper to streamline boolean parsing.
- Enhanced error handling with detailed logging for transaction gathering failures.
- Improved `sticky_key` method to use a temporary object ID fallback when `@sfa.id` is missing.

---------

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

test/fixtures/simplefin_accounts.yml

@@ -0,0 +1,2 @@
+# Empty fixture to ensure the simplefin_accounts table is truncated during tests.
+# Tests create SimplefinAccount records explicitly in setup.

test/fixtures/simplefin_items.yml

@@ -0,0 +1,2 @@
+# Empty fixture to ensure the simplefin_items table is truncated during tests.
+# Tests create SimplefinItem records explicitly in setup.

test/models/simplefin_account/liabilities/overpayment_analyzer_test.rb

@@ -0,0 +1,99 @@
+require "test_helper"
+
+class SimplefinAccount::Liabilities::OverpaymentAnalyzerTest < ActiveSupport::TestCase
+  # Limit fixtures to only what's required to avoid FK validation on unrelated tables
+  fixtures :families
+  setup do
+    @family = families(:dylan_family)
+    @item = SimplefinItem.create!(family: @family, name: "SimpleFIN", access_url: "https://example.com/token")
+    @sfa = SimplefinAccount.create!(
+      simplefin_item: @item,
+      name: "Test Credit Card",
+      account_id: "cc_txn_window_1",
+      currency: "USD",
+      account_type: "credit",
+      current_balance: BigDecimal("-22.72")
+    )
+
+    # Avoid cross‑suite fixture dependency by creating a fresh credit card account
+    @acct = Account.create!(
+      family: @family,
+      name: "Test CC",
+      balance: 0,
+      cash_balance: 0,
+      currency: "USD",
+      accountable: CreditCard.new
+    )
+    # Create explicit provider link to ensure FK validity in isolation
+    AccountProvider.create!(account: @acct, provider: @sfa)
+
+    # Enable heuristic
+    Setting["simplefin_cc_overpayment_detection"] = "true"
+    # Loosen thresholds for focused unit tests
+    Setting["simplefin_cc_overpayment_min_txns"] = "1"
+    Setting["simplefin_cc_overpayment_min_payments"] = "1"
+    Setting["simplefin_cc_overpayment_statement_guard_days"] = "0"
+  end
+
+  teardown do
+    # Disable heuristic to avoid bleeding into other tests
+    Setting["simplefin_cc_overpayment_detection"] = nil
+    Setting["simplefin_cc_overpayment_min_txns"] = nil
+    Setting["simplefin_cc_overpayment_min_payments"] = nil
+    Setting["simplefin_cc_overpayment_statement_guard_days"] = nil
+    begin
+      Rails.cache.delete_matched("simplefin:sfa:#{@sfa.id}:liability_sign_hint") if @sfa&.id
+    rescue
+      # ignore cache backends without delete_matched
+    end
+    # Ensure created records are removed to avoid FK validation across examples in single-file runs
+    AccountProvider.where(account_id: @acct.id).destroy_all rescue nil
+    @acct.destroy! rescue nil
+    @sfa.destroy! rescue nil
+    @item.destroy! rescue nil
+  end
+
+  test "classifies credit when payments exceed charges roughly by observed amount" do
+    # Create transactions in Maybe convention for liabilities:
+    # charges/spend: positive; payments: negative
+    # Observed abs is 22.72; make payments exceed charges by ~22.72
+    @acct.entries.delete_all
+    @acct.entries.create!(date: 10.days.ago.to_date, name: "Store A", amount: 50, currency: "USD", entryable: Transaction.new)
+    # Ensure payments exceed charges by at least observed.abs (~22.72)
+    @acct.entries.create!(date: 8.days.ago.to_date,  name: "Payment", amount: -75, currency: "USD", entryable: Transaction.new)
+
+    result = SimplefinAccount::Liabilities::OverpaymentAnalyzer.new(@sfa, observed_balance: @sfa.current_balance).call
+    assert_equal :credit, result.classification, "expected classification to be credit"
+  end
+
+  test "classifies debt when charges exceed payments" do
+    @acct.entries.delete_all
+    @acct.entries.create!(date: 12.days.ago.to_date, name: "Groceries", amount: 120, currency: "USD", entryable: Transaction.new)
+    @acct.entries.create!(date: 11.days.ago.to_date, name: "Coffee", amount: 10, currency: "USD", entryable: Transaction.new)
+    @acct.entries.create!(date: 9.days.ago.to_date,  name: "Payment", amount: -50, currency: "USD", entryable: Transaction.new)
+
+    result = SimplefinAccount::Liabilities::OverpaymentAnalyzer.new(@sfa, observed_balance: BigDecimal("-80")).call
+    assert_equal :debt, result.classification, "expected classification to be debt"
+  end
+
+  test "returns unknown when insufficient transactions" do
+    @acct.entries.delete_all
+    @acct.entries.create!(date: 5.days.ago.to_date, name: "Small", amount: 1, currency: "USD", entryable: Transaction.new)
+
+    result = SimplefinAccount::Liabilities::OverpaymentAnalyzer.new(@sfa, observed_balance: BigDecimal("-5")).call
+    assert_equal :unknown, result.classification
+  end
+
+  test "fallback to raw payload when no entries present" do
+    @acct.entries.delete_all
+    # Provide raw transactions in provider convention (expenses negative, income positive)
+    # We must negate in analyzer to convert to Maybe convention.
+    @sfa.update!(raw_transactions_payload: [
+      { id: "t1", amount: -100, posted: (10.days.ago.to_date.to_s) }, # charge (-> +100)
+      { id: "t2", amount: 150,  posted: (8.days.ago.to_date.to_s) }   # payment (-> -150)
+    ])
+
+    result = SimplefinAccount::Liabilities::OverpaymentAnalyzer.new(@sfa, observed_balance: BigDecimal("-50")).call
+    assert_equal :credit, result.classification
+  end
+end

test/models/simplefin_account_processor_test.rb

@@ -81,4 +81,101 @@ class SimplefinAccountProcessorTest < ActiveSupport::TestCase
 
     assert_equal BigDecimal("-75.00"), acct.reload.balance
   end
+
+  test "liability debt with both fields negative becomes positive (you owe)" do
+    sfin_acct = SimplefinAccount.create!(
+      simplefin_item: @item,
+      name: "BofA Visa",
+      account_id: "cc_bofa_1",
+      currency: "USD",
+      account_type: "credit",
+      current_balance: BigDecimal("-1200"),
+      available_balance: BigDecimal("-5000")
+    )
+
+    acct = accounts(:credit_card)
+    acct.update!(simplefin_account: sfin_acct)
+
+    SimplefinAccount::Processor.new(sfin_acct).send(:process_account!)
+
+    assert_equal BigDecimal("1200"), acct.reload.balance
+  end
+
+  test "liability overpayment with both fields positive becomes negative (credit)" do
+    sfin_acct = SimplefinAccount.create!(
+      simplefin_item: @item,
+      name: "BofA Visa",
+      account_id: "cc_bofa_2",
+      currency: "USD",
+      account_type: "credit",
+      current_balance: BigDecimal("75"),
+      available_balance: BigDecimal("5000")
+    )
+
+    acct = accounts(:credit_card)
+    acct.update!(simplefin_account: sfin_acct)
+
+    SimplefinAccount::Processor.new(sfin_acct).send(:process_account!)
+
+    assert_equal BigDecimal("-75"), acct.reload.balance
+  end
+
+  test "mixed signs falls back to invert observed (balance positive, avail negative => negative)" do
+    sfin_acct = SimplefinAccount.create!(
+      simplefin_item: @item,
+      name: "Chase Freedom",
+      account_id: "cc_chase_1",
+      currency: "USD",
+      account_type: "credit",
+      current_balance: BigDecimal("50"),
+      available_balance: BigDecimal("-5000")
+    )
+
+    acct = accounts(:credit_card)
+    acct.update!(simplefin_account: sfin_acct)
+
+    SimplefinAccount::Processor.new(sfin_acct).send(:process_account!)
+
+    assert_equal BigDecimal("-50"), acct.reload.balance
+  end
+
+  test "only available-balance present positive → negative (credit) for liability" do
+    sfin_acct = SimplefinAccount.create!(
+      simplefin_item: @item,
+      name: "Chase Visa",
+      account_id: "cc_chase_2",
+      currency: "USD",
+      account_type: "credit",
+      current_balance: nil,
+      available_balance: BigDecimal("25")
+    )
+
+    acct = accounts(:credit_card)
+    acct.update!(simplefin_account: sfin_acct)
+
+    SimplefinAccount::Processor.new(sfin_acct).send(:process_account!)
+
+    assert_equal BigDecimal("-25"), acct.reload.balance
+  end
+
+  test "mislinked as asset but mapper infers credit → normalize as liability" do
+    sfin_acct = SimplefinAccount.create!(
+      simplefin_item: @item,
+      name: "Visa Signature",
+      account_id: "cc_mislinked",
+      currency: "USD",
+      account_type: "credit",
+      current_balance: BigDecimal("100.00"),
+      available_balance: BigDecimal("5000.00")
+    )
+
+    # Link to an asset account intentionally
+    acct = accounts(:depository)
+    acct.update!(simplefin_account: sfin_acct)
+
+    SimplefinAccount::Processor.new(sfin_acct).send(:process_account!)
+
+    # Mapper should infer liability from name; final should be negative
+    assert_equal BigDecimal("-100.00"), acct.reload.balance
+  end
 end