Skip to content

Commit cf5e7de

Browse files
luckyPipewrenchJosh Waldrep
andauthored
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>
1 parent eb4b978 commit cf5e7de

10 files changed

Lines changed: 336 additions & 4 deletions

File tree

app/controllers/simplefin_items_controller.rb

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,28 @@ def setup_accounts
165165
[ "Other Asset", "OtherAsset" ]
166166
]
167167

168+
# Compute UI-only suggestions (preselect only when high confidence)
169+
@inferred_map = {}
170+
@simplefin_accounts.each do |sfa|
171+
holdings = sfa.raw_holdings_payload.presence || sfa.raw_payload.to_h["holdings"]
172+
institution_name = nil
173+
begin
174+
od = sfa.org_data
175+
institution_name = od["name"] if od.is_a?(Hash)
176+
rescue
177+
institution_name = nil
178+
end
179+
inf = Simplefin::AccountTypeMapper.infer(
180+
name: sfa.name,
181+
holdings: holdings,
182+
extra: sfa.extra,
183+
balance: sfa.current_balance,
184+
available_balance: sfa.available_balance,
185+
institution: institution_name
186+
)
187+
@inferred_map[sfa.id] = { type: inf.accountable_type, subtype: inf.subtype, confidence: inf.confidence }
188+
end
189+
168190
# Subtype options for each account type
169191
@subtype_options = {
170192
"Depository" => {

app/models/account.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,12 @@ def create_and_sync(attributes)
8787

8888

8989
def create_from_simplefin_account(simplefin_account, account_type, subtype = nil)
90+
# Respect user choice when provided; otherwise infer a sensible default
91+
# Require an explicit account_type; do not infer on the backend
92+
if account_type.blank? || account_type.to_s == "unknown"
93+
raise ArgumentError, "account_type is required when creating an account from SimpleFIN"
94+
end
95+
9096
# Get the balance from SimpleFin
9197
balance = simplefin_account.current_balance || simplefin_account.available_balance || 0
9298

app/models/investment.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ class Investment < ApplicationRecord
77
"retirement" => { short: "Retirement", long: "Retirement" },
88
"401k" => { short: "401(k)", long: "401(k)" },
99
"roth_401k" => { short: "Roth 401(k)", long: "Roth 401(k)" },
10+
"403b" => { short: "403(b)", long: "403(b)" },
11+
"tsp" => { short: "TSP", long: "Thrift Savings Plan" },
1012
"529_plan" => { short: "529 Plan", long: "529 Plan" },
1113
"hsa" => { short: "HSA", long: "Health Savings Account" },
1214
"mutual_fund" => { short: "Mutual Fund", long: "Mutual Fund" },
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
# frozen_string_literal: true
2+
3+
# Fallback-only inference for SimpleFIN-provided accounts.
4+
# Conservative, used only to suggest a default type during setup/creation.
5+
# Never overrides a user-selected type.
6+
module Simplefin
7+
class AccountTypeMapper
8+
Inference = Struct.new(:accountable_type, :subtype, :confidence, keyword_init: true)
9+
10+
RETIREMENT_KEYWORDS = /\b(401k|401\(k\)|403b|403\(b\)|tsp|ira|roth|retirement)\b/i.freeze
11+
BROKERAGE_KEYWORD = /\bbrokerage\b/i.freeze
12+
CREDIT_NAME_KEYWORDS = /\b(credit|card)\b/i.freeze
13+
CREDIT_BRAND_KEYWORDS = /\b(visa|mastercard|amex|american express|discover|apple card|freedom unlimited|quicksilver)\b/i.freeze
14+
LOAN_KEYWORDS = /\b(loan|mortgage|heloc|line of credit|loc)\b/i.freeze
15+
16+
# Explicit investment subtype tokens mapped to known SUBTYPES keys
17+
EXPLICIT_INVESTMENT_TOKENS = {
18+
/\btraditional\s+ira\b/i => "ira",
19+
/\broth\s+ira\b/i => "roth_ira",
20+
/\broth\s+401\(k\)\b|\broth\s*401k\b/i => "roth_401k",
21+
/\b401\(k\)\b|\b401k\b/i => "401k",
22+
/\b529\s*plan\b|\b529\b/i => "529_plan",
23+
/\bhsa\b|\bhealth\s+savings\s+account\b/i => "hsa",
24+
/\bpension\b/i => "pension",
25+
/\bmutual\s+fund\b/i => "mutual_fund",
26+
/\b403b\b|\b403\(b\)\b/i => "403b",
27+
/\btsp\b/i => "tsp"
28+
}.freeze
29+
30+
# Public API
31+
# @param name [String, nil]
32+
# @param holdings [Array<Hash>, nil]
33+
# @param extra [Hash, nil] - provider extras when present
34+
# @param balance [Numeric, String, nil]
35+
# @param available_balance [Numeric, String, nil]
36+
# @return [Inference] e.g. Inference.new(accountable_type: "Investment", subtype: "retirement", confidence: :high)
37+
def self.infer(name:, holdings: nil, extra: nil, balance: nil, available_balance: nil, institution: nil)
38+
nm_raw = name.to_s
39+
nm = nm_raw
40+
# Normalized form to catch variants like RothIRA, Traditional-IRA, 401(k)
41+
nm_norm = nm_raw.downcase.gsub(/[^a-z0-9]+/, " ").squeeze(" ").strip
42+
inst = institution.to_s
43+
holdings_present = holdings.is_a?(Array) && holdings.any?
44+
bal = (balance.to_d rescue nil)
45+
avail = (available_balance.to_d rescue nil)
46+
47+
# 0) Explicit retirement/plan tokens → Investment with explicit subtype (match against normalized name)
48+
if (explicit_sub = EXPLICIT_INVESTMENT_TOKENS.find { |rx, _| nm_norm.match?(rx) }&.last)
49+
if defined?(Investment::SUBTYPES) && Investment::SUBTYPES.key?(explicit_sub)
50+
return Inference.new(accountable_type: "Investment", subtype: explicit_sub, confidence: :high)
51+
else
52+
return Inference.new(accountable_type: "Investment", subtype: nil, confidence: :high)
53+
end
54+
end
55+
56+
# 1) Holdings present => Investment (high confidence)
57+
if holdings_present
58+
# Do not guess generic retirement; explicit tokens handled above
59+
return Inference.new(accountable_type: "Investment", subtype: nil, confidence: :high)
60+
end
61+
62+
# 2) Name suggests LOAN (high confidence)
63+
if LOAN_KEYWORDS.match?(nm)
64+
return Inference.new(accountable_type: "Loan", confidence: :high)
65+
end
66+
67+
# 3) Credit card signals
68+
# - Name contains credit/card (medium to high)
69+
# - Card brands (Visa/Mastercard/Amex/Discover/Apple Card) → high
70+
# - Or negative balance with available-balance present (medium)
71+
if CREDIT_NAME_KEYWORDS.match?(nm) || CREDIT_BRAND_KEYWORDS.match?(nm) || CREDIT_BRAND_KEYWORDS.match?(inst)
72+
return Inference.new(accountable_type: "CreditCard", confidence: :high)
73+
end
74+
# Strong combined signal for credit card: negative balance and positive available-balance
75+
if bal && bal < 0 && avail && avail > 0
76+
return Inference.new(accountable_type: "CreditCard", confidence: :high)
77+
end
78+
79+
# 4) Retirement keywords without holdings still point to Investment (retirement)
80+
if RETIREMENT_KEYWORDS.match?(nm)
81+
# If the name contains 'brokerage', avoid forcing retirement subtype
82+
subtype = BROKERAGE_KEYWORD.match?(nm) ? nil : "retirement"
83+
return Inference.new(accountable_type: "Investment", subtype: subtype, confidence: :high)
84+
end
85+
86+
# 5) Default
87+
Inference.new(accountable_type: "Depository", confidence: :low)
88+
end
89+
end
90+
end

app/models/simplefin_item/importer.rb

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,51 @@ def stats
109109
@stats ||= {}
110110
end
111111

112+
# Heuristics to set a SimpleFIN account inactive when upstream indicates closure/hidden
113+
# or when we repeatedly observe zero balances and zero holdings. This should not block
114+
# import and only sets a flag and suggestion via sync stats.
115+
def update_inactive_state(simplefin_account, account_data)
116+
payload = (account_data || {}).with_indifferent_access
117+
raw = (simplefin_account.raw_payload || {}).with_indifferent_access
118+
119+
# Flags from payloads
120+
closed = [ payload[:closed], payload[:hidden], payload.dig(:extra, :closed), raw[:closed], raw[:hidden] ].compact.any? { |v| v == true || v.to_s == "true" }
121+
122+
balance = payload[:balance]
123+
avail = payload[:"available-balance"]
124+
holdings = payload[:holdings]
125+
amounts = [ balance, avail ].compact
126+
zeroish_balance = amounts.any? && amounts.all? { |x| x.to_d.zero? rescue false }
127+
no_holdings = !(holdings.is_a?(Array) && holdings.any?)
128+
129+
stats["zero_runs"] ||= {}
130+
stats["inactive"] ||= {}
131+
key = simplefin_account.account_id.presence || simplefin_account.id
132+
key = key.to_s
133+
# Ensure key exists and defaults to false (so tests don't read nil)
134+
stats["inactive"][key] = false unless stats["inactive"].key?(key)
135+
136+
if closed
137+
stats["inactive"][key] = true
138+
stats["hints"] = Array(stats["hints"]) + [ "Some accounts appear closed/hidden upstream. You can relink or hide them." ]
139+
return
140+
end
141+
142+
if zeroish_balance && no_holdings
143+
stats["zero_runs"][key] = stats["zero_runs"][key].to_i + 1
144+
# Cap to avoid unbounded growth
145+
stats["zero_runs"][key] = [ stats["zero_runs"][key], 10 ].min
146+
else
147+
stats["zero_runs"][key] = 0
148+
stats["inactive"][key] = false
149+
end
150+
151+
if stats["zero_runs"][key].to_i >= 3
152+
stats["inactive"][key] = true
153+
stats["hints"] = Array(stats["hints"]) + [ "One or more accounts show no balance/holdings for multiple syncs — consider relinking or marking inactive." ]
154+
end
155+
end
156+
112157
# Track seen error fingerprints during a single importer run to avoid double counting
113158
def seen_errors
114159
@seen_errors ||= Set.new
@@ -457,6 +502,13 @@ def import_account(account_data)
457502
end
458503
simplefin_account.assign_attributes(attrs)
459504

505+
# Inactive detection/toggling (non-blocking)
506+
begin
507+
update_inactive_state(simplefin_account, account_data)
508+
rescue => e
509+
Rails.logger.warn("SimpleFin: inactive-state evaluation failed for sfa=#{simplefin_account.id || account_id}: #{e.class} - #{e.message}")
510+
end
511+
460512
# Final validation before save to prevent duplicates
461513
if simplefin_account.account_id.blank?
462514
simplefin_account.account_id = account_id
@@ -474,6 +526,10 @@ def import_account(account_data)
474526
register_error(message: msg, category: "other", account_id: account_id, name: account_data[:name])
475527
persist_stats!
476528
nil
529+
ensure
530+
# Ensure stats like zero_runs/inactive are persisted even when no errors occur,
531+
# particularly helpful for focused unit tests that call import_account directly.
532+
persist_stats!
477533
end
478534
end
479535

app/views/simplefin_items/_subtype_select.html.erb

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,23 @@
22
<% if subtype_config[:options].present? %>
33
<%= label_tag "account_subtypes[#{simplefin_account.id}]", subtype_config[:label],
44
class: "block text-sm font-medium text-primary mb-2" %>
5-
<% selected_value = account_type == "Depository" ?
6-
(simplefin_account.name.downcase.include?("checking") ? "checking" :
7-
simplefin_account.name.downcase.include?("savings") ? "savings" : "") : "" %>
5+
<% selected_value = "" %>
6+
<% if account_type == "Depository" %>
7+
<% n = simplefin_account.name.to_s.downcase %>
8+
<% selected_value = "" %>
9+
<% if n =~ /\bchecking\b|\bchequing\b|\bck\b|demand\s+deposit/ %>
10+
<% selected_value = "checking" %>
11+
<% elsif n =~ /\bsavings\b|\bsv\b/ %>
12+
<% selected_value = "savings" %>
13+
<% elsif n =~ /money\s+market|\bmm\b/ %>
14+
<% selected_value = "money_market" %>
15+
<% end %>
16+
<% elsif account_type == "Investment" %>
17+
<% inferred = @inferred_map&.dig(simplefin_account.id) || {} %>
18+
<% if inferred[:confidence] == :high && inferred[:type] == "Investment" && inferred[:subtype].present? %>
19+
<% selected_value = inferred[:subtype] %>
20+
<% end %>
21+
<% end %>
822
<%= select_tag "account_subtypes[#{simplefin_account.id}]",
923
options_for_select([["Select #{account_type == 'Depository' ? 'subtype' : 'type'}", ""]] + subtype_config[:options], selected_value),
1024
{ 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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,10 @@
7878
<div>
7979
<%= label_tag "account_types[#{simplefin_account.id}]", "Account Type:",
8080
class: "block text-sm font-medium text-primary mb-2" %>
81+
<% inferred = @inferred_map[simplefin_account.id] || {} %>
82+
<% selected_type = inferred[:confidence] == :high ? inferred[:type] : "" %>
8183
<%= select_tag "account_types[#{simplefin_account.id}]",
82-
options_for_select(@account_type_options),
84+
options_for_select(@account_type_options, selected_type),
8385
{ 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",
8486
data: {
8587
action: "change->account-type-selector#updateSubtype"
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
require "test_helper"
2+
3+
class AccountSimplefinCreationTest < ActiveSupport::TestCase
4+
setup do
5+
@family = families(:dylan_family)
6+
@item = SimplefinItem.create!(family: @family, name: "SF Conn", access_url: "https://example.com/access")
7+
end
8+
9+
test "requires explicit account_type at creation" do
10+
sfa = SimplefinAccount.create!(
11+
simplefin_item: @item,
12+
name: "Brokerage",
13+
account_id: "acct_1",
14+
currency: "USD",
15+
account_type: "investment",
16+
current_balance: 1000
17+
)
18+
19+
assert_raises(ArgumentError) do
20+
Account.create_from_simplefin_account(sfa, nil)
21+
end
22+
end
23+
24+
test "uses provided account_type without inference" do
25+
sfa = SimplefinAccount.create!(
26+
simplefin_item: @item,
27+
name: "My Loan",
28+
account_id: "acct_2",
29+
currency: "USD",
30+
account_type: "loan",
31+
current_balance: -5000
32+
)
33+
34+
account = Account.create_from_simplefin_account(sfa, "Loan")
35+
36+
assert_equal "Loan", account.accountable_type
37+
end
38+
end
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
require "test_helper"
2+
3+
class Simplefin::AccountTypeMapperTest < ActiveSupport::TestCase
4+
test "holdings present implies Investment" do
5+
inf = Simplefin::AccountTypeMapper.infer(name: "Vanguard Brokerage", holdings: [ { symbol: "VTI" } ])
6+
assert_equal "Investment", inf.accountable_type
7+
assert_nil inf.subtype
8+
end
9+
10+
test "explicit retirement tokens map to exact subtypes" do
11+
cases = {
12+
"My Roth IRA" => "roth_ira",
13+
"401k Fidelity" => "401k"
14+
}
15+
cases.each do |name, expected_subtype|
16+
inf = Simplefin::AccountTypeMapper.infer(name: name, holdings: [ { symbol: "VTI" } ])
17+
assert_equal "Investment", inf.accountable_type
18+
assert_equal expected_subtype, inf.subtype
19+
end
20+
end
21+
22+
test "credit card names map to CreditCard" do
23+
[ "Chase Credit Card", "VISA Card", "CREDIT" ] .each do |name|
24+
inf = Simplefin::AccountTypeMapper.infer(name: name)
25+
assert_equal "CreditCard", inf.accountable_type
26+
end
27+
end
28+
29+
test "loan-like names map to Loan" do
30+
[ "Mortgage", "Student Loan", "HELOC", "Line of Credit" ].each do |name|
31+
inf = Simplefin::AccountTypeMapper.infer(name: name)
32+
assert_equal "Loan", inf.accountable_type
33+
end
34+
end
35+
36+
test "default is Depository" do
37+
inf = Simplefin::AccountTypeMapper.infer(name: "Everyday Checking")
38+
assert_equal "Depository", inf.accountable_type
39+
end
40+
end

0 commit comments

Comments
 (0)