mirror of
https://github.com/we-promise/sure.git
synced 2026-05-08 05:04:59 +00:00
7c14c80444
* Fix SimpleFIN inverting Loan account balances
SimplefinAccount::Processor#process_account! routes every liability
through OverpaymentAnalyzer + normalize_liability_balance. That path
is built around credit-like liabilities, where transaction history
distinguishes debt vs. credit. For a Loan account with only the
opening anchor (no payment history), the analyzer returns :unknown
and the fallback negates the observed value:
def normalize_liability_balance(observed, bal, avail)
...
-observed
end
That's wrong for loans: the bank reports the principal outstanding
as a positive number from its own books. Negating it stores the loan
balance as negative, so BalanceSheet#net_worth = assets - liabilities
ends up _adding_ the loan instead of subtracting it (off by 2× the
loan amount). Example with a hypothetical mortgage:
raw_balance = 100000.00 (positive — bank's own report)
Sure stored = -100000.00 (negated by the fallback)
Net worth shown = inflated by 2 × 100000
Short-circuit Loan accountables straight to observed.abs and skip the
analyzer/fallback entirely. Loans don't have credit-vs-debt
ambiguity — if the loan is paid off the balance is 0, not negative.
Credit cards still go through the existing heuristic.
* Add observability for the SimpleFIN loan sign branch
Mirrors the logging + Sentry breadcrumb the credit-card branches emit
when the OverpaymentAnalyzer classifies as :credit / :debt, so the
loan short-circuit shows up in production traces too. Per CodeRabbit
review on #1574.
* Test that positive bank-reported loan balances are preserved
The existing "inverts negative balance for loan liabilities" test only
covers a bank that reports the loan as negative — both the old (buggy)
fallback and the new short-circuit produce the same +50000 there, which
is why the inversion bug went undetected. Add a sibling test where the
bank reports +50000 (the common mortgage convention); under the old
code that became -50000 and inflated net worth.
* Redact monetary amounts from SimpleFIN liability info logs
Move raw observed/stored amounts and metric totals from `Rails.logger.info`
and `Sentry.add_breadcrumb` payloads to a `Rails.logger.debug` line.
The info-level message and breadcrumb data now carry only identifiers
(`sfa_id`) plus the classification (`loan` / `credit` / `debt` /
`unknown`) and `tx_count`, so log aggregators and Sentry no longer
receive raw monetary values for any of the four liability branches.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
247 lines
7.6 KiB
Ruby
247 lines
7.6 KiB
Ruby
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
|
|
|
|
# Many banks (and most mortgage feeds) report the loan principal
|
|
# outstanding as a positive number from the bank's books. The old
|
|
# liability path ran every Loan through OverpaymentAnalyzer +
|
|
# normalize_liability_balance, and the latter's fallback for
|
|
# `:unknown` classifications returned `-observed`, flipping a
|
|
# bank-reported `+50000` into `-50000`. That bug made loans _add_ to
|
|
# net worth instead of subtracting, so this test pins the corrected
|
|
# behaviour.
|
|
test "preserves positive provider balance for loan liabilities" do
|
|
sfin_acct = SimplefinAccount.create!(
|
|
simplefin_item: @item,
|
|
name: "Mortgage",
|
|
account_id: "loan_2",
|
|
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
|
|
|
|
test "liability debt with both fields negative becomes positive (you owe)" do
|
|
sfin_acct = SimplefinAccount.create!(
|
|
simplefin_item: @item,
|
|
name: "BofA Visa",
|
|
account_id: "cc_bofa_1",
|
|
currency: "USD",
|
|
account_type: "credit",
|
|
current_balance: BigDecimal("-1200"),
|
|
available_balance: BigDecimal("-5000")
|
|
)
|
|
|
|
acct = accounts(:credit_card)
|
|
acct.update!(simplefin_account: sfin_acct)
|
|
|
|
SimplefinAccount::Processor.new(sfin_acct).send(:process_account!)
|
|
|
|
assert_equal BigDecimal("1200"), acct.reload.balance
|
|
end
|
|
|
|
test "liability overpayment with both fields positive becomes negative (credit)" do
|
|
sfin_acct = SimplefinAccount.create!(
|
|
simplefin_item: @item,
|
|
name: "BofA Visa",
|
|
account_id: "cc_bofa_2",
|
|
currency: "USD",
|
|
account_type: "credit",
|
|
current_balance: BigDecimal("75"),
|
|
available_balance: BigDecimal("5000")
|
|
)
|
|
|
|
acct = accounts(:credit_card)
|
|
acct.update!(simplefin_account: sfin_acct)
|
|
|
|
SimplefinAccount::Processor.new(sfin_acct).send(:process_account!)
|
|
|
|
assert_equal BigDecimal("-75"), acct.reload.balance
|
|
end
|
|
|
|
test "mixed signs falls back to invert observed (balance positive, avail negative => negative)" do
|
|
sfin_acct = SimplefinAccount.create!(
|
|
simplefin_item: @item,
|
|
name: "Chase Freedom",
|
|
account_id: "cc_chase_1",
|
|
currency: "USD",
|
|
account_type: "credit",
|
|
current_balance: BigDecimal("50"),
|
|
available_balance: BigDecimal("-5000")
|
|
)
|
|
|
|
acct = accounts(:credit_card)
|
|
acct.update!(simplefin_account: sfin_acct)
|
|
|
|
SimplefinAccount::Processor.new(sfin_acct).send(:process_account!)
|
|
|
|
assert_equal BigDecimal("-50"), acct.reload.balance
|
|
end
|
|
|
|
test "only available-balance present positive → negative (credit) for liability" do
|
|
sfin_acct = SimplefinAccount.create!(
|
|
simplefin_item: @item,
|
|
name: "Chase Visa",
|
|
account_id: "cc_chase_2",
|
|
currency: "USD",
|
|
account_type: "credit",
|
|
current_balance: nil,
|
|
available_balance: BigDecimal("25")
|
|
)
|
|
|
|
acct = accounts(:credit_card)
|
|
acct.update!(simplefin_account: sfin_acct)
|
|
|
|
SimplefinAccount::Processor.new(sfin_acct).send(:process_account!)
|
|
|
|
assert_equal BigDecimal("-25"), acct.reload.balance
|
|
end
|
|
|
|
test "linked depository account type takes precedence over mapper-inferred liability" do
|
|
sfin_acct = SimplefinAccount.create!(
|
|
simplefin_item: @item,
|
|
name: "Visa Signature",
|
|
account_id: "cc_mislinked_asset",
|
|
currency: "USD",
|
|
account_type: "credit",
|
|
current_balance: BigDecimal("100.00"),
|
|
available_balance: BigDecimal("5000.00")
|
|
)
|
|
|
|
acct = accounts(:depository)
|
|
acct.update!(simplefin_account: sfin_acct)
|
|
|
|
SimplefinAccount::Processor.new(sfin_acct).send(:process_account!)
|
|
|
|
# Manual selection as depository; final should be the same
|
|
assert_equal BigDecimal("100.00"), acct.reload.balance
|
|
end
|
|
|
|
test "linked credit card account type takes precedence over mapper-inferred liability" do
|
|
sfin_acct = SimplefinAccount.create!(
|
|
simplefin_item: @item,
|
|
name: "Visa Signature",
|
|
account_id: "cc_mislinked_liability",
|
|
currency: "USD",
|
|
account_type: "credit",
|
|
current_balance: BigDecimal("100.00"),
|
|
available_balance: BigDecimal("5000.00")
|
|
)
|
|
|
|
acct = accounts(:credit_card)
|
|
acct.update!(simplefin_account: sfin_acct)
|
|
|
|
SimplefinAccount::Processor.new(sfin_acct).send(:process_account!)
|
|
|
|
# Liability has flipped sign; final should be negative
|
|
assert_equal BigDecimal("-100.00"), acct.reload.balance
|
|
end
|
|
|
|
test "dormant credit card with zero balance and negative available-balance shows zero debt" do
|
|
sfin_acct = SimplefinAccount.create!(
|
|
simplefin_item: @item,
|
|
name: "Discover Card",
|
|
account_id: "cc_dormant",
|
|
currency: "USD",
|
|
account_type: "credit",
|
|
current_balance: BigDecimal("0"),
|
|
available_balance: BigDecimal("-3800") # credit limit reported as negative
|
|
)
|
|
|
|
acct = accounts(:credit_card)
|
|
acct.update!(simplefin_account: sfin_acct)
|
|
|
|
SimplefinAccount::Processor.new(sfin_acct).send(:process_account!)
|
|
|
|
# Should use explicit zero balance, not negative available_balance
|
|
assert_equal BigDecimal("0"), acct.reload.balance
|
|
end
|
|
end
|