Simplefin liabilities recording fix (#410)

* Add tests and logic for Simplefin account balance normalization

- Introduced `SimplefinAccountProcessorTest` to verify balance normalization logic.
- Updated `SimplefinAccount::Processor` to invert negative balances for liability accounts (credit cards and loans) while keeping asset balances unchanged.
- Added comments to clarify balance conventions and sign normalization rules.

* Refactor balances-only sync logic and improve tests for edge cases

- Updated `SimplefinItem::Importer` and `SimplefinItem::Syncer` to ensure `last_synced_at` remains nil during balances-only runs, preserving chunked-history behavior for full syncs.
- Introduced additional comments to clarify balances-only implications and syncing logic.
- Added test case in `SimplefinAccountProcessorTest` to verify correct handling of overpayment for credit card liabilities.
- Refined balance normalization in `SimplefinAccount::Processor` to always invert liability balances for consistency.

---------

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

app/jobs/simplefin_item/balances_only_job.rb

@@ -19,12 +19,9 @@ class SimplefinItem::BalancesOnlyJob < ApplicationJob
       Rails.logger.warn("SimpleFin BalancesOnlyJob import failed: #{e.class} - #{e.message}")
     end
 
-    # Best-effort freshness update
-    begin
-      item.update!(last_synced_at: Time.current) if item.has_attribute?(:last_synced_at)
-    rescue => e
-      Rails.logger.warn("SimpleFin BalancesOnlyJob last_synced_at update failed: #{e.class} - #{e.message}")
-    end
+    # IMPORTANT: Do NOT update last_synced_at during balances-only discovery.
+    # Leaving last_synced_at nil ensures the next full sync uses the
+    # chunked-history path to fetch historical transactions.
 
     # Refresh the SimpleFin card on Providers/Accounts pages so badges and statuses update without a full reload
     begin

app/models/simplefin_account/processor.rb

@@ -41,10 +41,14 @@ class SimplefinAccount::Processor
       account = simplefin_account.current_account
       balance = simplefin_account.current_balance || simplefin_account.available_balance || 0
 
-      # SimpleFIN balance convention matches our app convention:
-      # - Positive balance = debt (you owe money)
-      # - Negative balance = credit balance (bank owes you, e.g., overpayment)
-      # No sign conversion needed - pass through as-is (same as Plaid)
+      # Normalize balances for liabilities (SimpleFIN typically uses opposite sign)
+      # App convention:
+      # - Liabilities: positive => you owe; negative => provider owes you (overpayment/credit)
+      # Since providers often send the opposite sign, ALWAYS invert for liabilities so
+      # that both debt and overpayment cases are represented correctly.
+      if [ "CreditCard", "Loan" ].include?(account.accountable_type)
+        balance = -balance
+      end
 
       # Calculate cash balance correctly for investment accounts
       cash_balance = if account.accountable_type == "Investment"

app/models/simplefin_item/importer.rb

@@ -16,8 +16,14 @@ class SimplefinItem::Importer
     Rails.logger.info "SimplefinItem::Importer - sync_start_date: #{simplefin_item.sync_start_date.inspect}"
 
     begin
-      if simplefin_item.last_synced_at.nil?
-        # First sync - use chunked approach to get full history
+      # Defensive guard: If last_synced_at is set but there are linked accounts
+      # with no transactions captured yet (typical after a balances-only run),
+      # force the first full run to use chunked history to backfill.
+      linked_accounts = simplefin_item.simplefin_accounts.joins(:account)
+      no_txns_yet = linked_accounts.any? && linked_accounts.all? { |sfa| sfa.raw_transactions_payload.blank? }
+
+      if simplefin_item.last_synced_at.nil? || no_txns_yet
+        # First sync (or balances-only pre-run) — use chunked approach to get full history
         Rails.logger.info "SimplefinItem::Importer - Using chunked history import"
         import_with_chunked_history
       else
@@ -211,9 +217,15 @@ class SimplefinItem::Importer
       max_requests = 22
       current_end_date = Time.current
 
-      # Use user-selected sync_start_date if available, otherwise use default lookback
+      # Decide how far back to walk:
+      # - If the user set a custom sync_start_date, honor it
+      # - Else, for first-time chunked history, walk back up to the provider-safe
+      #   limit implied by chunking so we actually import meaningful history.
+      #   We do NOT use the small initial lookback (7 days) here, because that
+      #   would clip the very first chunk to ~1 week and prevent further history.
       user_start_date = simplefin_item.sync_start_date
-      default_start_date = initial_sync_lookback_period.days.ago
+      implied_max_lookback_days = chunk_size_days * max_requests
+      default_start_date = implied_max_lookback_days.days.ago
       target_start_date = user_start_date ? user_start_date.beginning_of_day : default_start_date
 
       # Enforce maximum 3-year lookback to respect SimpleFin's actual 60-day limit per request
@@ -698,7 +710,9 @@ class SimplefinItem::Importer
     end
 
     def initial_sync_lookback_period
-      # Default to 7 days for initial sync to avoid API limits
+      # Default to 7 days for initial sync. Providers that support deeper
+      # history will supply it via chunked fetches, and users can optionally
+      # set a custom `sync_start_date` to go further back.
       7
     end
 

app/models/simplefin_item/syncer.rb

@@ -6,16 +6,38 @@ class SimplefinItem::Syncer
   end
 
   def perform_sync(sync)
+    # If no accounts are linked yet, run a balances-only discovery pass so the user
+    # can review and manually link accounts first. This mirrors the historical flow
+    # users expect: initial 7-day balances snapshot, then full chunked history after linking.
+    begin
+      if simplefin_item.simplefin_accounts.joins(:account).count == 0
+        sync.update!(status_text: "Discovering accounts (balances only)...") if sync.respond_to?(:status_text)
+        # Pre-mark the sync as balances_only so downstream completion code does not
+        # bump last_synced_at. The importer also sets this flag, but setting it here
+        # guarantees the guard is present even if the importer exits early.
+        if sync.respond_to?(:sync_stats)
+          existing = (sync.sync_stats || {})
+          sync.update_columns(sync_stats: existing.merge("balances_only" => true))
+        end
+        SimplefinItem::Importer.new(simplefin_item, simplefin_provider: simplefin_item.simplefin_provider, sync: sync).import_balances_only
+        finalize_setup_counts(sync)
+        mark_completed(sync)
+        return
+      end
+    rescue => e
+      # If discovery-only path errors, fall back to regular logic below so we don't block syncs entirely
+      Rails.logger.warn("SimplefinItem::Syncer auto balances-only path failed: #{e.class} - #{e.message}")
+    end
+
     # Balances-only fast path
     if sync.respond_to?(:sync_stats) && (sync.sync_stats || {})["balances_only"]
       sync.update!(status_text: "Refreshing balances only...") if sync.respond_to?(:status_text)
       begin
         # Use the Importer to run balances-only path
         SimplefinItem::Importer.new(simplefin_item, simplefin_provider: simplefin_item.simplefin_provider, sync: sync).import_balances_only
-        # Update last_synced_at for UI freshness if the column exists
-        if simplefin_item.has_attribute?(:last_synced_at)
-          simplefin_item.update!(last_synced_at: Time.current)
-        end
+        # IMPORTANT: Do NOT update last_synced_at during balances-only runs.
+        # Leaving last_synced_at nil ensures the next full sync uses the
+        # chunked-history path to fetch full historical transactions.
         finalize_setup_counts(sync)
         mark_completed(sync)
       rescue => e

test/models/simplefin_account_processor_test.rb

@@ -0,0 +1,84 @@
+require "test_helper"
+
+class SimplefinAccountProcessorTest < ActiveSupport::TestCase
+  setup do
+    @family = families(:dylan_family)
+    @item = SimplefinItem.create!(
+      family: @family,
+      name: "SimpleFIN",
+      access_url: "https://example.com/token"
+    )
+  end
+
+  test "inverts negative balance for credit card liabilities" do
+    sfin_acct = SimplefinAccount.create!(
+      simplefin_item: @item,
+      name: "Chase Credit",
+      account_id: "cc_1",
+      currency: "USD",
+      account_type: "credit",
+      current_balance: BigDecimal("-123.45")
+    )
+
+    acct = accounts(:credit_card)
+    acct.update!(simplefin_account: sfin_acct)
+
+    SimplefinAccount::Processor.new(sfin_acct).send(:process_account!)
+
+    assert_equal BigDecimal("123.45"), acct.reload.balance
+  end
+
+  test "does not invert balance for asset accounts (depository)" do
+    sfin_acct = SimplefinAccount.create!(
+      simplefin_item: @item,
+      name: "Checking",
+      account_id: "dep_1",
+      currency: "USD",
+      account_type: "checking",
+      current_balance: BigDecimal("1000.00")
+    )
+
+    acct = accounts(:depository)
+    acct.update!(simplefin_account: sfin_acct)
+
+    SimplefinAccount::Processor.new(sfin_acct).send(:process_account!)
+
+    assert_equal BigDecimal("1000.00"), acct.reload.balance
+  end
+
+  test "inverts negative balance for loan liabilities" do
+    sfin_acct = SimplefinAccount.create!(
+      simplefin_item: @item,
+      name: "Mortgage",
+      account_id: "loan_1",
+      currency: "USD",
+      account_type: "mortgage",
+      current_balance: BigDecimal("-50000")
+    )
+
+    acct = accounts(:loan)
+    acct.update!(simplefin_account: sfin_acct)
+
+    SimplefinAccount::Processor.new(sfin_acct).send(:process_account!)
+
+    assert_equal BigDecimal("50000"), acct.reload.balance
+  end
+
+  test "positive provider balance (overpayment) becomes negative for credit card liabilities" do
+    sfin_acct = SimplefinAccount.create!(
+      simplefin_item: @item,
+      name: "Chase Credit",
+      account_id: "cc_overpay",
+      currency: "USD",
+      account_type: "credit",
+      current_balance: BigDecimal("75.00") # provider sends positive for overpayment
+    )
+
+    acct = accounts(:credit_card)
+    acct.update!(simplefin_account: sfin_acct)
+
+    SimplefinAccount::Processor.new(sfin_acct).send(:process_account!)
+
+    assert_equal BigDecimal("-75.00"), acct.reload.balance
+  end
+end