UI Suggestions for Account Types in Setup Modal + Stats-Based Inactive Handling

app/controllers/settings/hostings_controller.rb

@@ -75,7 +75,7 @@ def update
     end
 
     if hosting_params.key?(:openai_uri_base)
-      Setting.openai_uri_base = hosting_params[:openai_uri_base]
+      Setting.openai_uri_base = hosting_params[:openai_uri_base].presence
     end
 
     if hosting_params.key?(:openai_model)

app/controllers/simplefin_items_controller.rb

@@ -165,6 +165,20 @@ def setup_accounts
       [ "Other Asset", "OtherAsset" ]
     ]
 
+    # Compute UI-only suggestions (preselect only when high confidence)
+    @inferred_map = {}
+    @simplefin_accounts.each do |sfa|
+      holdings = sfa.raw_holdings_payload.presence || sfa.raw_payload.to_h["holdings"]
+      inf = Simplefin::AccountTypeMapper.infer(
+        name: sfa.name,
+        holdings: holdings,
+        extra: sfa.extra,
+        balance: sfa.current_balance,
+        available_balance: sfa.available_balance
+      )
+      @inferred_map[sfa.id] = { type: inf.accountable_type, subtype: inf.subtype, confidence: inf.confidence }
+    end
+
     # Subtype options for each account type
     @subtype_options = {
       "Depository" => {

app/models/account.rb

@@ -87,6 +87,12 @@ def create_and_sync(attributes)
 
 
     def create_from_simplefin_account(simplefin_account, account_type, subtype = nil)
+      # Respect user choice when provided; otherwise infer a sensible default
+      # Require an explicit account_type; do not infer on the backend
+      if account_type.blank? || account_type.to_s == "unknown"
+        raise ArgumentError, "account_type is required when creating an account from SimpleFIN"
+      end
+
       # Get the balance from SimpleFin
       balance = simplefin_account.current_balance || simplefin_account.available_balance || 0
 

app/models/simplefin/account_type_mapper.rb

@@ -0,0 +1,69 @@
+# frozen_string_literal: true
+
+# Fallback-only inference for SimpleFIN-provided accounts.
+# Conservative, used only to suggest a default type during setup/creation.
+# Never overrides a user-selected type.
+module Simplefin
+  class AccountTypeMapper
+    Inference = Struct.new(:accountable_type, :subtype, :confidence, keyword_init: true)
+
+    RETIREMENT_KEYWORDS = /\b(401k|401\(k\)|403b|403\(b\)|tsp|ira|roth|retirement)\b/i.freeze
+    BROKERAGE_KEYWORD = /\bbrokerage\b/i.freeze
+    CREDIT_NAME_KEYWORDS = /\b(credit|card)\b/i.freeze
+    CREDIT_BRAND_KEYWORDS = /\b(visa|mastercard|amex|american express|discover)\b/i.freeze
+    LOAN_KEYWORDS = /\b(loan|mortgage|heloc|line of credit|loc)\b/i.freeze
+
+    # Public API
+    # @param name [String, nil]
+    # @param holdings [Array<Hash>, nil]
+    # @param extra [Hash, nil] - provider extras when present
+    # @param balance [Numeric, String, nil]
+    # @param available_balance [Numeric, String, nil]
+    # @return [Inference] e.g. Inference.new(accountable_type: "Investment", subtype: "retirement", confidence: :high)
+    def self.infer(name:, holdings: nil, extra: nil, balance: nil, available_balance: nil)
+      nm = name.to_s
+      holdings_present = holdings.is_a?(Array) && holdings.any?
+      bal = (balance.to_d rescue nil)
+      avail = (available_balance.to_d rescue nil)
+
+      # 1) Holdings present => Investment (high confidence)
+      if holdings_present
+        subtype = retirement_hint?(nm, extra) ? "retirement" : nil
+        return Inference.new(accountable_type: "Investment", subtype: subtype, confidence: :high)
+      end
+
+      # 2) Name suggests LOAN (high confidence)
+      if LOAN_KEYWORDS.match?(nm)
+        return Inference.new(accountable_type: "Loan", confidence: :high)
+      end
+
+      # 3) Credit card signals
+      # - Name contains credit/card (medium to high)
+      # - Or negative balance with available-balance present (medium)
+      if CREDIT_NAME_KEYWORDS.match?(nm) || CREDIT_BRAND_KEYWORDS.match?(nm)
+        return Inference.new(accountable_type: "CreditCard", confidence: :high)
+      end
+      if bal && bal < 0 && !avail.nil?
+        return Inference.new(accountable_type: "CreditCard", confidence: :medium)
+      end
+
+      # 4) Retirement keywords without holdings still point to Investment (retirement)
+      if RETIREMENT_KEYWORDS.match?(nm)
+        return Inference.new(accountable_type: "Investment", subtype: "retirement", confidence: :high)
+      end
+
+      # 5) Default
+      Inference.new(accountable_type: "Depository", confidence: :low)
+    end
+
+    def self.retirement_hint?(name, extra)
+      return true if RETIREMENT_KEYWORDS.match?(name.to_s)
+
+      # sometimes providers include hints in extra payload
+      x = (extra || {}).with_indifferent_access
+      candidate = [ x[:account_subtype], x[:type], x[:subtype], x[:category] ].compact.join(" ")
+      RETIREMENT_KEYWORDS.match?(candidate)
+    end
+    private_class_method :retirement_hint?
+  end
+end

app/models/simplefin_item/importer.rb

@@ -109,6 +109,50 @@ def stats
       @stats ||= {}
     end
 
+    # Heuristics to set a SimpleFIN account inactive when upstream indicates closure/hidden
+    # or when we repeatedly observe zero balances and zero holdings. This should not block
+    # import and only sets a flag and suggestion via sync stats.
+    def update_inactive_state(simplefin_account, account_data)
+      payload = (account_data || {}).with_indifferent_access
+      raw = (simplefin_account.raw_payload || {}).with_indifferent_access
+
+      # Flags from payloads
+      closed = [ payload[:closed], payload[:hidden], payload.dig(:extra, :closed), raw[:closed], raw[:hidden] ].compact.any? { |v| v == true || v.to_s == "true" }
+
+      balance = payload[:balance]
+      avail = payload[:"available-balance"]
+      holdings = payload[:holdings]
+      zeroish_balance = [ balance, avail ].compact.all? { |x| x.to_d.zero? rescue false }
+      no_holdings = !(holdings.is_a?(Array) && holdings.any?)
+
+      stats["zero_runs"] ||= {}
+      stats["inactive"] ||= {}
+      key = simplefin_account.account_id.presence || simplefin_account.id
+      key = key.to_s
+      # Ensure key exists and defaults to false (so tests don't read nil)
+      stats["inactive"][key] = false unless stats["inactive"].key?(key)
+
+      if closed
+        stats["inactive"][key] = true
+        stats["hints"] = Array(stats["hints"]) + [ "Some accounts appear closed/hidden upstream. You can relink or hide them." ]
+        return
+      end
+
+      if zeroish_balance && no_holdings
+        stats["zero_runs"][key] = stats["zero_runs"][key].to_i + 1
+        # Cap to avoid unbounded growth
+        stats["zero_runs"][key] = [ stats["zero_runs"][key], 10 ].min
+      else
+        stats["zero_runs"][key] = 0
+        stats["inactive"][key] = false
+      end
+
+      if stats["zero_runs"][key].to_i >= 3
+        stats["inactive"][key] = true
+        stats["hints"] = Array(stats["hints"]) + [ "One or more accounts show no balance/holdings for multiple syncs — consider relinking or marking inactive." ]
+      end
+    end
+
     # Track seen error fingerprints during a single importer run to avoid double counting
     def seen_errors
       @seen_errors ||= Set.new
@@ -457,6 +501,13 @@ def import_account(account_data)
       end
       simplefin_account.assign_attributes(attrs)
 
+      # Inactive detection/toggling (non-blocking)
+      begin
+        update_inactive_state(simplefin_account, account_data)
+      rescue => e
+        Rails.logger.warn("SimpleFin: inactive-state evaluation failed for sfa=#{simplefin_account.id || account_id}: #{e.class} - #{e.message}")
+      end
+
       # Final validation before save to prevent duplicates
       if simplefin_account.account_id.blank?
         simplefin_account.account_id = account_id
@@ -474,6 +525,10 @@ def import_account(account_data)
         register_error(message: msg, category: "other", account_id: account_id, name: account_data[:name])
         persist_stats!
         nil
+      ensure
+        # Ensure stats like zero_runs/inactive are persisted even when no errors occur,
+        # particularly helpful for focused unit tests that call import_account directly.
+        persist_stats!
       end
     end
 

app/views/simplefin_items/_subtype_select.html.erb

@@ -2,9 +2,11 @@
   <% if subtype_config[:options].present? %>
     <%= label_tag "account_subtypes[#{simplefin_account.id}]", subtype_config[:label],
                  class: "block text-sm font-medium text-primary mb-2" %>
-    <% selected_value = account_type == "Depository" ?
-         (simplefin_account.name.downcase.include?("checking") ? "checking" :
-          simplefin_account.name.downcase.include?("savings") ? "savings" : "") : "" %>
+    <% selected_value = "" %>
+    <% if account_type == "Depository" %>
+      <% n = simplefin_account.name.to_s.downcase %>
+      <% selected_value = n.include?("checking") ? "checking" : (n.include?("savings") ? "savings" : "") %>
+    <% end %>
     <%= select_tag "account_subtypes[#{simplefin_account.id}]",
                   options_for_select([["Select #{account_type == 'Depository' ? 'subtype' : 'type'}", ""]] + subtype_config[:options], selected_value),
                   { class: "appearance-none bg-container border border-primary rounded-md px-3 py-2 text-sm leading-6 text-primary focus:border-primary focus:ring-1 focus:ring-primary focus:outline-none w-full" } %>

app/views/simplefin_items/setup_accounts.html.erb

@@ -78,8 +78,10 @@
               <div>
                 <%= label_tag "account_types[#{simplefin_account.id}]", "Account Type:",
                              class: "block text-sm font-medium text-primary mb-2" %>
+                <% inferred = @inferred_map[simplefin_account.id] || {} %>
+                <% selected_type = inferred[:confidence] == :high ? inferred[:type] : "" %>
                 <%= select_tag "account_types[#{simplefin_account.id}]",
-                              options_for_select(@account_type_options),
+                              options_for_select(@account_type_options, selected_type),
                               { class: "appearance-none bg-container border border-primary rounded-md px-3 py-2 text-sm leading-6 text-primary focus:border-primary focus:ring-1 focus:ring-primary focus:outline-none w-full",
                                 data: {
                                   action: "change->account-type-selector#updateSubtype"

test/models/account_simplefin_creation_test.rb

@@ -0,0 +1,38 @@
+require "test_helper"
+
+class AccountSimplefinCreationTest < ActiveSupport::TestCase
+  setup do
+    @family = families(:dylan_family)
+    @item = SimplefinItem.create!(family: @family, name: "SF Conn", access_url: "https://example.com/access")
+  end
+
+  test "requires explicit account_type at creation" do
+    sfa = SimplefinAccount.create!(
+      simplefin_item: @item,
+      name: "Brokerage",
+      account_id: "acct_1",
+      currency: "USD",
+      account_type: "investment",
+      current_balance: 1000
+    )
+
+    assert_raises(ArgumentError) do
+      Account.create_from_simplefin_account(sfa, nil)
+    end
+  end
+
+  test "uses provided account_type without inference" do
+    sfa = SimplefinAccount.create!(
+      simplefin_item: @item,
+      name: "My Loan",
+      account_id: "acct_2",
+      currency: "USD",
+      account_type: "loan",
+      current_balance: -5000
+    )
+
+    account = Account.create_from_simplefin_account(sfa, "Loan")
+
+    assert_equal "Loan", account.accountable_type
+  end
+end

test/models/simplefin/account_type_mapper_test.rb

@@ -0,0 +1,36 @@
+require "test_helper"
+
+class Simplefin::AccountTypeMapperTest < ActiveSupport::TestCase
+  test "holdings present implies Investment" do
+    inf = Simplefin::AccountTypeMapper.infer(name: "Vanguard Brokerage", holdings: [ { symbol: "VTI" } ])
+    assert_equal "Investment", inf.accountable_type
+    assert_nil inf.subtype
+  end
+
+  test "retirement inferred when name includes IRA/401k/Roth" do
+    [ "My Roth IRA", "401k Fidelity" ].each do |name|
+      inf = Simplefin::AccountTypeMapper.infer(name: name, holdings: [ { symbol: "VTI" } ])
+      assert_equal "Investment", inf.accountable_type
+      assert_equal "retirement", inf.subtype
+    end
+  end
+
+  test "credit card names map to CreditCard" do
+    [ "Chase Credit Card", "VISA Card", "CREDIT" ] .each do |name|
+      inf = Simplefin::AccountTypeMapper.infer(name: name)
+      assert_equal "CreditCard", inf.accountable_type
+    end
+  end
+
+  test "loan-like names map to Loan" do
+    [ "Mortgage", "Student Loan", "HELOC", "Line of Credit" ].each do |name|
+      inf = Simplefin::AccountTypeMapper.infer(name: name)
+      assert_equal "Loan", inf.accountable_type
+    end
+  end
+
+  test "default is Depository" do
+    inf = Simplefin::AccountTypeMapper.infer(name: "Everyday Checking")
+    assert_equal "Depository", inf.accountable_type
+  end
+end

test/models/simplefin_item/importer_inactive_test.rb

@@ -0,0 +1,48 @@
+require "test_helper"
+
+class SimplefinItem::ImporterInactiveTest < ActiveSupport::TestCase
+  setup do
+    @family = families(:dylan_family)
+    @item = SimplefinItem.create!(family: @family, name: "SF Conn", access_url: "https://example.com/access")
+    @sync = Sync.create!(syncable: @item)
+  end
+
+  def importer
+    @importer ||= SimplefinItem::Importer.new(@item, simplefin_provider: mock(), sync: @sync)
+  end
+
+  test "marks inactive when payload indicates closed or hidden" do
+    account_data = { id: "a1", name: "Old Checking", balance: 0, currency: "USD", closed: true }
+    importer.send(:import_account, account_data)
+
+    stats = @sync.reload.sync_stats
+    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
+    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"
+
+    importer.send(:import_account, account_data)
+    stats = @sync.reload.sync_stats
+    assert_equal true, stats.dig("inactive", "a2"), "should be inactive at threshold"
+  end
+
+  test "resets zero_runs_count 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")
+
+    # Activity returns: non-zero balance or holdings
+    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