UI Suggestions for Account Types in Setup Modal + Stats-Based Inactive Handling (#368)

* - Add tests for `Simplefin::AccountTypeMapper` and `AccountSimplefinCreation`
- Implement `Simplefin::AccountTypeMapper` for account type inference with fallback-only logic
- Enhance inactive state handling for `SimplefinItem::Importer`
- Improve subtype selection handling in views with confidence-based inference

* Remove unnecessary `.presence` check for `openai_uri_base` in hostings settings

* Refine zero balance detection logic in `SimplefinItem::Importer` and add regression test for missing balances scenario

* Enhance account type and subtype inference logic with explicit investment subtype mapping, improved regex handling, and institution-based credit card detection

* Refine retirement subtype mapping in `AccountTypeMapper` tests with explicit case-based assertions

* Expand `AccountTypeMapper` investment subtype mapping to include `403b` and `tsp` with updated regex definitions

* Remove unused `retirement_hint?` method in `AccountTypeMapper` to simplify codebase

---------

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

app/controllers/simplefin_items_controller.rb

@@ -165,6 +165,28 @@ class SimplefinItemsController < ApplicationController
       [ "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"]
+      institution_name = nil
+      begin
+        od = sfa.org_data
+        institution_name = od["name"] if od.is_a?(Hash)
+      rescue
+        institution_name = nil
+      end
+      inf = Simplefin::AccountTypeMapper.infer(
+        name: sfa.name,
+        holdings: holdings,
+        extra: sfa.extra,
+        balance: sfa.current_balance,
+        available_balance: sfa.available_balance,
+        institution: institution_name
+      )
+      @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 @@ class Account < ApplicationRecord
 
 
     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/investment.rb

@@ -7,6 +7,8 @@ class Investment < ApplicationRecord
     "retirement" => { short: "Retirement", long: "Retirement" },
     "401k" => { short: "401(k)", long: "401(k)" },
     "roth_401k" => { short: "Roth 401(k)", long: "Roth 401(k)" },
+    "403b" => { short: "403(b)", long: "403(b)" },
+    "tsp" => { short: "TSP", long: "Thrift Savings Plan" },
     "529_plan" => { short: "529 Plan", long: "529 Plan" },
     "hsa" => { short: "HSA", long: "Health Savings Account" },
     "mutual_fund" => { short: "Mutual Fund", long: "Mutual Fund" },

app/models/simplefin/account_type_mapper.rb

@@ -0,0 +1,90 @@
+# 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|apple card|freedom unlimited|quicksilver)\b/i.freeze
+    LOAN_KEYWORDS = /\b(loan|mortgage|heloc|line of credit|loc)\b/i.freeze
+
+    # Explicit investment subtype tokens mapped to known SUBTYPES keys
+    EXPLICIT_INVESTMENT_TOKENS = {
+      /\btraditional\s+ira\b/i => "ira",
+      /\broth\s+ira\b/i => "roth_ira",
+      /\broth\s+401\(k\)\b|\broth\s*401k\b/i => "roth_401k",
+      /\b401\(k\)\b|\b401k\b/i => "401k",
+      /\b529\s*plan\b|\b529\b/i => "529_plan",
+      /\bhsa\b|\bhealth\s+savings\s+account\b/i => "hsa",
+      /\bpension\b/i => "pension",
+      /\bmutual\s+fund\b/i => "mutual_fund",
+      /\b403b\b|\b403\(b\)\b/i => "403b",
+      /\btsp\b/i => "tsp"
+    }.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, institution: nil)
+      nm_raw = name.to_s
+      nm = nm_raw
+      # Normalized form to catch variants like RothIRA, Traditional-IRA, 401(k)
+      nm_norm = nm_raw.downcase.gsub(/[^a-z0-9]+/, " ").squeeze(" ").strip
+      inst = institution.to_s
+      holdings_present = holdings.is_a?(Array) && holdings.any?
+      bal = (balance.to_d rescue nil)
+      avail = (available_balance.to_d rescue nil)
+
+      # 0) Explicit retirement/plan tokens → Investment with explicit subtype (match against normalized name)
+      if (explicit_sub = EXPLICIT_INVESTMENT_TOKENS.find { |rx, _| nm_norm.match?(rx) }&.last)
+        if defined?(Investment::SUBTYPES) && Investment::SUBTYPES.key?(explicit_sub)
+          return Inference.new(accountable_type: "Investment", subtype: explicit_sub, confidence: :high)
+        else
+          return Inference.new(accountable_type: "Investment", subtype: nil, confidence: :high)
+        end
+      end
+
+      # 1) Holdings present => Investment (high confidence)
+      if holdings_present
+        # Do not guess generic retirement; explicit tokens handled above
+        return Inference.new(accountable_type: "Investment", subtype: nil, 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)
+      # - Card brands (Visa/Mastercard/Amex/Discover/Apple Card) → high
+      # - Or negative balance with available-balance present (medium)
+      if CREDIT_NAME_KEYWORDS.match?(nm) || CREDIT_BRAND_KEYWORDS.match?(nm) || CREDIT_BRAND_KEYWORDS.match?(inst)
+        return Inference.new(accountable_type: "CreditCard", confidence: :high)
+      end
+      # Strong combined signal for credit card: negative balance and positive available-balance
+      if bal && bal < 0 && avail && avail > 0
+        return Inference.new(accountable_type: "CreditCard", confidence: :high)
+      end
+
+      # 4) Retirement keywords without holdings still point to Investment (retirement)
+      if RETIREMENT_KEYWORDS.match?(nm)
+        # If the name contains 'brokerage', avoid forcing retirement subtype
+        subtype = BROKERAGE_KEYWORD.match?(nm) ? nil : "retirement"
+        return Inference.new(accountable_type: "Investment", subtype: subtype, confidence: :high)
+      end
+
+      # 5) Default
+      Inference.new(accountable_type: "Depository", confidence: :low)
+    end
+  end
+end

app/models/simplefin_item/importer.rb

@@ -109,6 +109,51 @@ class SimplefinItem::Importer
       @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]
+      amounts = [ balance, avail ].compact
+      zeroish_balance = amounts.any? && amounts.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 +502,13 @@ class SimplefinItem::Importer
       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 +526,10 @@ class SimplefinItem::Importer
         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,23 @@
   <% 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 = "" %>
+      <% if n =~ /\bchecking\b|\bchequing\b|\bck\b|demand\s+deposit/ %>
+        <% selected_value = "checking" %>
+      <% elsif n =~ /\bsavings\b|\bsv\b/ %>
+        <% selected_value = "savings" %>
+      <% elsif n =~ /money\s+market|\bmm\b/ %>
+        <% selected_value = "money_market" %>
+      <% end %>
+    <% elsif account_type == "Investment" %>
+      <% inferred = @inferred_map&.dig(simplefin_account.id) || {} %>
+      <% if inferred[:confidence] == :high && inferred[:type] == "Investment" && inferred[:subtype].present? %>
+        <% selected_value = inferred[:subtype] %>
+      <% end %>
+    <% 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,40 @@
+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 "explicit retirement tokens map to exact subtypes" do
+    cases = {
+      "My Roth IRA" => "roth_ira",
+      "401k Fidelity" => "401k"
+    }
+    cases.each do |name, expected_subtype|
+      inf = Simplefin::AccountTypeMapper.infer(name: name, holdings: [ { symbol: "VTI" } ])
+      assert_equal "Investment", inf.accountable_type
+      assert_equal expected_subtype, 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,62 @@
+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
+
+
+# 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
+
+    importer.send(:import_account, account_data)
+    stats = @sync.reload.sync_stats
+
+    assert_equal 0, stats.dig("zero_runs", "a4").to_i
+    assert_equal false, stats.dig("inactive", "a4")
+  end
+end