diff --git a/app/controllers/simplefin_items_controller.rb b/app/controllers/simplefin_items_controller.rb index 271740cbcc9..4d78b1c5a03 100644 --- a/app/controllers/simplefin_items_controller.rb +++ b/app/controllers/simplefin_items_controller.rb @@ -165,6 +165,28 @@ 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"] + 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" => { diff --git a/app/models/account.rb b/app/models/account.rb index d82872f6b94..1b9b5e23a3f 100644 --- a/app/models/account.rb +++ b/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 diff --git a/app/models/investment.rb b/app/models/investment.rb index 4e4c25c86b7..c7a4898e5db 100644 --- a/app/models/investment.rb +++ b/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" }, diff --git a/app/models/simplefin/account_type_mapper.rb b/app/models/simplefin/account_type_mapper.rb new file mode 100644 index 00000000000..e01f4adb09d --- /dev/null +++ b/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, 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 diff --git a/app/models/simplefin_item/importer.rb b/app/models/simplefin_item/importer.rb index 19038da8af5..95b5fc307e0 100644 --- a/app/models/simplefin_item/importer.rb +++ b/app/models/simplefin_item/importer.rb @@ -109,6 +109,51 @@ 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] + 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 @@ 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 +526,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 diff --git a/app/views/simplefin_items/_subtype_select.html.erb b/app/views/simplefin_items/_subtype_select.html.erb index f8c5378af23..f5c30548f4b 100644 --- a/app/views/simplefin_items/_subtype_select.html.erb +++ b/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" } %> diff --git a/app/views/simplefin_items/setup_accounts.html.erb b/app/views/simplefin_items/setup_accounts.html.erb index 44ca829823f..1570bb471c0 100644 --- a/app/views/simplefin_items/setup_accounts.html.erb +++ b/app/views/simplefin_items/setup_accounts.html.erb @@ -78,8 +78,10 @@
<%= 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" diff --git a/test/models/account_simplefin_creation_test.rb b/test/models/account_simplefin_creation_test.rb new file mode 100644 index 00000000000..6cb3cc7b54f --- /dev/null +++ b/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 diff --git a/test/models/simplefin/account_type_mapper_test.rb b/test/models/simplefin/account_type_mapper_test.rb new file mode 100644 index 00000000000..6e86382ecf2 --- /dev/null +++ b/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 diff --git a/test/models/simplefin_item/importer_inactive_test.rb b/test/models/simplefin_item/importer_inactive_test.rb new file mode 100644 index 00000000000..4ae83db6f86 --- /dev/null +++ b/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