Commit Graph

880 Commits

Author SHA1 Message Date
Guillem Arias
71ca400f42 fix(goals): catch-up subtracts pending pledges from the demand
UX audit finding. The catch-up alert demanded $X/mo without
accounting for pledges the user had already recorded. The user
recorded a $20k pledge → catch-up still demanded a fresh $20k →
double-counting → stacked yellow CTAs telling them to do the
thing they'd just done.

Goal#catch_up_delta_money now subtracts `open_pledges.sum(amount)`
from the demand:

  delta = max(monthly_target − pace − sum(open_pledges), 0)

Uses the in-memory preloaded `open_pledges` collection (controllers
already eager-load it), so no extra query. The clamp at zero keeps
"$0/mo more" from rendering when pending pledges fully cover the
gap.

Alert branch in show.html.erb now also gates on
`@goal.catch_up_delta_money.amount.positive?` — when the demand
zeroes out via pending pledges, suppress the alert entirely.
Status pill stays `:behind` (because `pace < required`), but the
action surface goes quiet because the user already took it.
2026-05-14 21:54:41 +02:00
Guillem Arias
8e9a697b1f fix(goals): months_remaining uses day-precision
PF audit edge case. Calendar-month math undercounted near the
deadline: May 30 with a June 1 target returned `1` ("save $5k
this month"), then June 1 morning returned `0` (falls through to
`remaining_amount` charged as one-month-required). Users saw a
$5k/mo required rate for a 2-day window, then $5k flat on
deadline day — a cliff that doesn't match reality.

Replace calendar-month delta with `(target_date - Date.current) / 30.0`
so a 2-day-out deadline reports ~0.07 months and `monthly_target_amount`
scales proportionally. `[..., 0.0].max` keeps the past-due case
zero-clamped.
2026-05-14 21:50:54 +02:00
Guillem Arias
51fca464b5 fix(goals): reconciler logs to Sentry + rename :extend route to :renew
Two Ruby idiom audit fixes.

The Reconciler's outer `rescue StandardError` was logging at error
level and moving on. Pipeline-protective (we don't want a Goal
reconcile failure to break the Plaid/SimpleFIN/etc importer it's
hooked into) but invisible — real bugs hid behind a warn log
forever. Add `Sentry.capture_exception(e) if defined?(Sentry)`
alongside the log, matching the pattern in `Account::Syncer`,
`Sync`, `PlaidItem`, and the chart-series rescues this branch
already added. Keep the rescue's protective function.

`member do patch :extend end` shadows `Module#extend` — the
controller action name competes with Ruby's most-common
mixin entry point. `before_action :foo, only: %i[extend destroy]`
reads as "extend this controller with :foo, only: …" to a casual
reader, and stack traces against `def extend` look misleading.
Rename to `:renew` (matches the existing copy: the button says
"Extend 7 days," but the API verb is "renew the watching window"):

  - config/routes.rb: `patch :renew`
  - GoalPledgesController#extend → #renew
  - locale `goal_pledges.extend` → `goal_pledges.renew`
  - banner `extend_goal_pledge_path` → `renew_goal_pledge_path`
  - test refs updated

The user-facing button text is unchanged.
2026-05-14 21:50:01 +02:00
Guillem Arias Fauste
3674f94885 Merge branch 'main' into feat/goals-v2-architecture 2026-05-14 21:42:02 +02:00
CrossDrain
c106aaf10d fix(enable-banking): preserve claimed pending date on subsequent syncs (#1797)
After the first sync claims a pending entry (setting auto_claimed_pending_ids),
subsequent syncs find the entry by booked external_id as an existing record.
pending_match is never entered so pending_entry_date stays nil, causing
`nil || date` to silently overwrite the preserved pending date with the
booked settlement date.

Fix by checking auto_claimed_pending_ids on the existing entry — its presence
signals a prior auto-claim, so entry.date (the original pending date) is kept.

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-14 21:33:22 +02:00
Guillem Arias
26d9ad76bf fix(goals): exclude pending transactions from pace + mobile-stack funding rows
Two audit fixes that pair well.

PF audit B20: pace, family velocity, and the funding widget's
30/90-day totals all summed Entry amounts over the linked accounts
*including provider-pending transactions*. A pending Plaid/SimpleFIN
deposit inflated pace today; the next sync that reversed or dropped
it silently shrunk pace tomorrow, with no signal to the user.
Worse, the reconciler could match a pending transaction and flip
the pledge to "matched" before the underlying entry vanished.

`.merge(Transaction.excluding_pending)` on the three Entry queries
(Goal#pace, Family#savings_inflow_velocity, the funding widget's
`inflow_totals_map`) brings the existing
`Transaction::PENDING_PROVIDERS`-aware scope into play. Single-line
fix across the three call sites.

UX audit: funding-account rows used `grid-cols-[24px_1fr_48px_120px]`
at every breakpoint. On a 375pt iPhone viewport that left ~50px for
the name column after `p-5` padding + container chrome — name
truncated to "Ban…" and the per-row % column squeezed against the
weight/totals stack. The percent number is also already encoded in
the distribution bar above the rows; on mobile it can disappear
without losing signal.

Drop the % column at < sm:
- mobile grid: `grid-cols-[24px_minmax(0,1fr)_auto]` (avatar / name /
  totals)
- sm+: original 4-column layout with the per-row %
- per-row balance subline + accountable label now also drops `.00`
  cents (consistency with the rest of the page).
2026-05-14 21:23:15 +02:00
Guillem Arias
4bda89999b fix(goals/show): strip redundancy + sharpen catch-up framing
The show page repeated the same data multiple times across surfaces
that should each say one thing once. Per-screen counts before this
commit:

  - Account % distribution: 4 places (distribution bar + dot-legend
    strip + 5-bar weight pill + % column)
  - Current balance: 3 places (ring, funding heading total, ring
    "of $X" subline)
  - Target amount: 3 places (header, ring subline, catch-up body)
  - Target date: 3 places (header, catch-up body, chart axis)
  - Pace: 2 places (catch-up body, projection subtitle)
  - ".00" cents: every monetary string

This pass:

- Funding widget drops the dot-legend strip (color/name/% triplet
  redundant with the distribution bar's color + the per-row avatar
  color) and the 5-bar weight pill (rendered as "1-of-5 sliver" for
  low-weight accounts — read as a glitch; the % number next to it
  covered the same fact). Row grid shrinks from 5 to 4 columns.
- Funding section heading drops `· $187,031` — the ring card
  already carries the total balance.
- Catch-up alert reframes:
    Title was "Save $26,621/mo to stay on track" (the *full* required
    rate, with the misleading "stay on track" while the pill says
    "Behind"). Now "Save $20,002/mo more to catch up" using
    `catch_up_delta_money` — the user's actual delta over current
    pace.
    Body collapsed from two with-date / no-date variants to a single
    "Current pace $X/mo · required $Y/mo to hit your target." Drops
    the target date duplication since the header already says it.
    Pledge CTA pre-fills with the *delta*, not the full required —
    so accepting it once funds the gap instead of stacking the full
    required rate on top of existing pace.
    Secondary link "Or adjust your target" → "Adjust target instead"
    (less defeatist framing).
- Projection chart subtitle "At $X/mo you'll miss your target date."
  drops the pace duplication (catch-up above already states pace).
  New: "Falling short at current pace." Diagnostic only.
- All money on the show page uses `format(precision: 0)`. The ".00"
  cents added no information at goal-tracking scale.
- Header `Record pledge` demotes to `outline` variant when status is
  `:behind` — the catch-up alert below owns the primary action.
  One primary action per surface.

Also adjacent fixes:

- Funding widget keys avatar / distribution color off `account.id`,
  not `account.name`. Renaming an account no longer recolors it
  retroactively; two accounts with name-hash collisions no longer
  share a color (Ruby idiom audit finding).
- `Goals::StatusPillComponent`: add `:completed` variant with
  `circle-check-big` icon. `Goal#display_status` now returns
  `:completed` when `goal.completed?` so a manually-completed
  goal (e.g. user stopped at 80%) reads "Completed" rather than
  falling through to `:on_track`/`:behind` and lying on the index.

Locale: drop `body_with_date` (folded into `body`),
`projection.behind` no longer carries interpolation args (caller
doesn't pass them either), `projection.no_pace` plain-language
rewrite ("inflow" → "deposits"), add `status.completed: "Completed"`.
2026-05-14 21:16:59 +02:00
Guillem Arias
5530ff5f06 chore(goals): drop dead V1 hooks + surface chart errors
Loose ends from the V1 → V2 refactor that the architecture commit
didn't sweep.

- Demo generator (B14): the `goal_spec[:contributions]` arrays
  + the `wedding_contribs` / `house_contribs` builders still
  shipped in the file, but the seeding loop that consumed them
  was deleted alongside `GoalContribution`. Dead data. Strip both
  the per-goal arrays and the two locals. Goal balance/pace in
  the demo family now derives from the linked depository
  accounts' own seeded entries elsewhere in the generator.

- Goal stepper controller (B16): the `static targets` declaration
  still listed `initialContributionAmount` and
  `initialContributionAccountSelect`, and `refreshAccountSelect`
  + its two callsites still ran every time a linked-account
  checkbox flipped. The HTML targets disappeared with the V2
  stepper rebuild, so `has*Target` guards short-circuited and the
  method was a no-op — but it was still dispatched on every
  change. Drop the targets, the method, and the two callsites.

- Chart series rescue (B25): `Goal#balance_series_values` and
  `FundingAccountsBreakdownComponent#sparkline_map` both swallowed
  `StandardError` with a `Rails.logger.warn(…)`. The chart then
  degraded to "target line only" silently. Promote the log to
  `error` level and forward to Sentry when present (matching the
  pattern in `Account::Syncer`, `Sync`, `PlaidItem`). Fallback to
  empty result still preserved so the surface degrades instead of
  500-ing.
2026-05-14 19:48:32 +02:00
Guillem Arias
8548703651 refactor(goals/show): move projection_summary + catch_up_delta to model
The show template carried a 17-line `if/elsif` chain computing
`projection_summary` inline, plus a `Money.new([…, 0].max, …)`
expression building the catch-up delta on the fly. CLAUDE.md's
"skinny controllers, fat models" convention pushes both onto Goal.

- `Goal#projection_summary`: returns the localized,
  `html_safe`-aware string for the chart subtitle and the chart's
  `aria-description`. Memoized so the two callsites in show.html.erb
  share one computation.
- `Goal#catch_up_delta_money`: clamped-at-zero monthly delta between
  pace and the required monthly target. Used by the catch-up
  callout body. Previously the view computed
  `Money.new([req - pace, 0].max, currency)` — same math, but
  duplicated inline.

show.html.erb drops both blocks and reads `@goal.projection_summary`
/ `@goal.catch_up_delta_money` directly.

Also: V15 — the celebration card used `bg-green-500/10` directly.
Swap to `bg-success/10` (DS semantic token, same Tailwind-4 alpha
syntax DS::Alert already uses) so the celebration palette tracks
the rest of the success surface.
2026-05-14 19:43:29 +02:00
Guillem Arias
150dc4bdc9 fix(goals): pace counts transfers, family rollup currency-scoped
Two semantic shifts in V2 that drove the worst on-screen confusion.

B3/B4 — `Goal#pace` excluded `Transaction::TRANSFER_KINDS`. When a
user tapped "I just transferred…" and the deposit landed, the linked
account's balance went up but pace did not: pace ignored transfer-
kind entries, so the goal stayed `:behind` against `monthly_target`
and the catch-up callout kept demanding $X/mo even though the user
had just moved the money in. Same root cause hit any long-time saver
whose 90-day net was zero — pace=0, status=:behind, projection says
"At $0.00/mo you'll miss your target date" while the ring sits at
80%.

Drop the transfer-kind exclusion. Pace is now net inflow into linked
accounts over 90 days. Transfers between linked accounts already net
out (both legs land inside the same account set); transfers from
outside (checking → linked savings) net positive, which is exactly
the case the pledge flow records.

B19 — `Family#savings_inflow_velocity` summed entry amounts across
every depository account linked to any goal regardless of currency,
then rendered the result in the family's primary currency. A family
with one USD goal and one EUR goal saw `usd_inflow + eur_inflow`
reported as USD with no FX conversion. Scope the account set to the
family's primary currency until proper FX-conversion lands. Also
let the result go negative (net outflow) — clamping to ≥0 lost
signal; the controller decides how to render the sign.

V20 (controller) — `velocity_30d_sign` was wired off the *delta*
direction, so a $1,234 down-month rendered as "−$1,234 ↓ 27% vs
prior 30d". The minus read as a loss but $1,234 was the (positive)
contribution. Re-wire the headline sign off the headline value
itself; the delta-direction stays on the subline as ↑/↓ N%. With
the family-rollup change above, the headline can now legitimately
be negative — UI now says "−$200 ↓ 50% vs prior 30d" when the
family had net outflow.

B21 — KPI tile `on_track_count` lumped `:reached` goals into "on
track", inflating the numerator while the sort order placed reached
goals at the bottom of the list. Split `reached_count` out and
render it as its own segment in the on-track subline ("1 reached ·
1 behind · 1 paused").

Test: rename the pace=zero test to match its new premise (no
transactions vs. no non-transfer entries). The fixture still has no
entries, so the assertion holds.
2026-05-14 19:17:12 +02:00
Guillem Arias
83c64b9e94 fix(goals): pledge lifecycle + connected-account detection
Behavioural fixes touching Goal, GoalPledge, the reconciler and the
goals controller. No schema change.

B5 — connected-account detection covered only Plaid. SimpleFIN, Brex,
Enable Banking, IBKR, Kraken, SnapTrade and Lunchflow users got
"manual_save" pledges by default; their auto-synced Transactions then
failed to match (reconciler matches Transactions to "transfer" pledges
only). Pledges sat in the yellow banner until expiry. Switch the
detection to !Account#manual?, which mirrors the existing
`Account.manual` scope (no account_providers, no plaid_account_id, no
simplefin_account_id). Add `Account#manual?` so the per-instance and
per-query checks can't drift.

B7 — `extend!` widens `expires_at` but `matches?` was anchored on
`created_at ± 5d`, so an extension that pushed the expiry past day 5
didn't actually buy any match runway. Widen the upper bound to
`max(created_at + 5d, expires_at)`. The lower bound stays at
`created_at − 5d`.

B8 — `Goal#open_pledges` returned `status: open` regardless of expiry.
Between a pledge timing out (day 7) and the 15-min sweep job marking
it `expired`, the show page rendered a ghost yellow banner with
"0 days left" that the reconciler would no longer touch. Add
`expires_at >= NOW` to the scope so the visible state matches the
match-eligible state.

B9 — Double-click on Record pledge produced two identical open
pledges, which then stacked as two yellow banners. Add a create-time
validation rejecting duplicates against (goal_id, account_id, amount,
status=open, expires_at >= NOW).

B10 — The reconciler used `transaction.with_lock` but didn't lock the
pledge. Two concurrent reconcile attempts on different transactions
could both target the same pledge; one would lose to the partial
unique index on `transactions.extra->'goal'->>'pledge_id'` and the
RecordNotUnique was caught by the outer StandardError rescue, which
silently dropped the other transaction's match attempt entirely.
Lock the pledge first, re-check `status_open?` inside the lock, and
catch RecordNotUnique alongside RecordInvalid/NotOpenError in the
reconciler — so on a lost race we fall through to the next candidate
pledge instead of exiting the loop. Extract the Valuation-match path
to `GoalPledge#resolve_with_valuation!` so it goes through the same
locked status-recheck.

B12 — When a goal is destroyed, `dependent: :destroy` reaped pledges
but left `transactions.extra["goal"]["pledge_id"]` pointing at the
now-deleted UUIDs. The partial unique index on that JSON path then
indexed stale references. Add a `before_destroy` on GoalPledge that
clears the matching transaction's `extra` if it still points back to
the pledge.

B6 — `last_matched_pledge_at` used `goal_pledges.maximum(:updated_at)`
on matched rows. Any backfill or sync-resync that touches a matched
pledge bumped `updated_at`, so a single resync set every goal's "Last
saved N days ago" header back to "today". Switch to the entry's
`date` via a join through `matched_transaction_id`, which reflects the
date the money actually moved.

B22 — `scope :chronological` ordered DESC, the opposite of what the
name promises. Rename to `:reverse_chronological` and update the one
caller in `goals#show`. (Other models' `chronological` scopes are
unrelated and ordered correctly.)

Also: preload `account_providers` on `linked_accounts` in the index
and show controllers so `Account#manual?` walks the in-memory
collection instead of triggering N queries.

Tests: add fixture-backed coverage for extend-widens-match-window,
post-extend rejection beyond expiry, and the duplicate-pledge
validation. Existing assertions still hold against the new
`matches?` window math.
2026-05-14 19:12:28 +02:00
Guillem Arias
c92522b149 ux(goals/index): restore prior-30d comparison + multi-part on-track subtitle
The v2 rewrite dropped the velocity_delta_percent / velocity_direction
keys that powered the 'Contributed last 30d' card's '↑ 27.2% vs. prior 30d'
line and the 'Goals on track' multi-part subtitle ('1 behind · 1 paused').
Restore both, sourcing velocity from Family#savings_inflow_velocity with
explicit current-window and prior-window ranges.
2026-05-14 18:14:51 +02:00
Guillem Arias
26e4612748 fix(goals/pledge): drop find_each-incompatible order in reconciler
The explicit .order(created_at: :asc).find_each emitted an AR warning
that broke the strict logger mock in BrexEntry::ProcessorTest.
find_each forces its own primary-key order anyway.
2026-05-14 18:06:00 +02:00
Guillem Arias
eb7ef50eed fix(goals): CI green — schema, brakeman, pledge modal, error class
Regenerate schema.rb after the three v2 migrations so CI's db:schema:load
picks up goal_pledges, the dropped goal_contributions, and the partial
unique pledge_id index.

Brakeman:
- Drop :account_id and :kind from goal_pledge permit; look the account
  up via @goal.linked_accounts.find_by(id:) instead and set kind
  server-side from goal.any_connected_account?.
- Rename goals.show.projection.on_track to .on_track_html so I18n
  marks the result html_safe automatically; drop the unconditional
  .html_safe call in show.html.erb.

Pledge modal: rewrite app/views/goal_pledges/new.html.erb to use
DS::Dialog (the Sure convention for create modals — matches
categories/transfers).

Error handling: replace `raise ActiveRecord::RecordInvalid, "string"`
in GoalPledge#extend!/cancel! with a dedicated GoalPledge::NotOpenError;
the controller rescues that specifically.

Tests: rewrite the "pace is zero" test to create a fresh account with
no entries (the fixture's depository accounts carry transaction history
that produces a non-zero pace). All goal tests now green (73 runs,
157 assertions, 0 failures).
2026-05-14 17:54:08 +02:00
Guillem Arias
88032ce020 feat(goals): v2 architecture — drop ledger, derive balance, add pledge
Reshape the goals feature to live on top of linked-account balances.
A goal's balance is now the live balance of every depository account
linked to it — no parallel ledger, no "log a contribution" step.

The "Add contribution" affordance is replaced by a 7-day GoalPledge
(kind: transfer | manual_save). GoalPledge::Reconciler matches incoming
Transactions (via Account::ProviderImportAdapter) and Valuations (via
Account::ReconciliationManager) against open pledges within ±5 days,
±$0.50, or ±1% — single hook covers every provider (Plaid, SimpleFIN,
Lunchflow, Enable Banking, Brex, IBKR, Kraken, SnapTrade) plus manual
balance edits. A 15-minute Sidekiq cron sweeps expired pledges.

Goal model: balance derived from linked_accounts.sum(&:balance), new
pace (90-day net non-transfer inflow), months_of_runway,
last_matched_pledge_*, pledge_action_label_key (the "I just
transferred…" vs "I just saved…" verb switch).

UI:
- Index gets a 3-card KPI strip (Contributed last 30d / Needs this
  month / On track) plus a pending-pledges callout.
- Show page swaps the "Add contribution" CTA for the pledge modal,
  replaces the contribution list with a pending-pledge banner, and
  rebuilds the funding widget into per-account rows with a 12-bucket
  weekly sparkline and last-30 inflow.
- Projection chart adds a required-line (dashed light from
  today → target) and a translucent pending-pledge bump at today's X.

Schema (3 migrations):
1. goal_pledges table with PG enums (goal_pledge_kind, goal_pledge_status),
   open-by-expiry index, and unique-when-not-null matched_transaction_id.
2. Drop goal_contributions.
3. Partial unique index on
   transactions ((extra -> 'goal' ->> 'pledge_id')) built CONCURRENTLY
   so it doesn't block prod.

After pulling: run bin/rails db:migrate, then commit the schema.rb sync
separately (or let CI regenerate).

Deferred to v1.1: allocation columns, contention/archived banners,
"why is this behind?" diagnostic, reallocate flow, refresh-sync +
Plaid throttle, unallocated-cash chip, joint-account approval,
goal_activities log, polymorphic matched_entry_id/type for manual
pledge audit.
2026-05-14 16:07:14 +02:00
Guillem Arias Fauste
62bc766b0c Merge branch 'main' into feat/savings-goals 2026-05-14 11:53:26 +02:00
CrossDrain
ba3b20627d feat(balance): Preserve historical balances as waypoints for linked accounts (#1663)
* feat(balance): persist daily balance snapshots for linked accounts (SnapTrade, Plaid)

When updating a linked account's balance, the previous day's current_anchor
is now preserved as a reconciliation valuation before being replaced. This
creates a chain of API-reported balance waypoints over time. The
ReverseCalculator has been updated to treat these reconciliation valuations
as reset points during reverse syncs, ensuring historical balances accurately
reflect the known API-reported values even with incomplete transaction history.

* fix(balance): don't treat current_anchor as reconciliation waypoint

The ReverseCalculator was incorrectly treating the current_anchor valuation
(on Date.current) as a reconciliation waypoint, causing it to reset the
balance and ignore same-day transactions. This fix adds a check to ensure
only true reconciliation entries (entryable.reconciliation?) trigger the
reset behavior.

Additionally, set_current_balance_for_linked_account is now wrapped in a
database transaction to ensure atomicity when preserving stale anchors and
creating/updating the current anchor. Logging has been improved to use
debug level for amount details.

A regression test was added to verify that same-day flows are correctly
processed when a current_anchor exists on the current date.

* test(account): ensure preserved valuations use correct historical date

Add validation that valuation entries created during balance
preservation are dated as of yesterday. This prevents future-dated
entries and maintains temporal accuracy in financial snapshots.

* refactor: remove redundant transaction block and unused method comment in current balance manager

* refactor(account): remove redundant valuations reload in CurrentBalanceManager and add regression test for consecutive reconciliation waypoints

* refactor: remove redundant transaction block and update anchor rotation log to include entry ID
2026-05-13 21:27:50 +02:00
ghost
e59235fdc5 feat(statements): add account statement vault (#1753)
* feat(statements): add account statement vault

Add web-only statement uploads, account linking, duplicate detection, and per-account coverage/reconciliation checks without mutating transactions. Extend ActiveStorage authorization and targeted tests for family/account scoping.

* fix(statements): return deleted account statements to inbox

Preserve linked statement records when an account is deleted by moving them back to the unmatched inbox, then expand coverage for upload validation, sanitized parser metadata, unavailable reconciliation, and missing-month coverage.

* fix(statements): harden vault upload review flows

Address review and security findings in the statement vault by preserving sanitized parser metadata, failing closed on orphaned statement blobs, avoiding account_id mass assignment permits, and adding regression coverage for link/delete edge cases.

* fix(statements): harden vault upload and access controls

* fix(statements): address vault hardening review

* fix(statements): address vault review feedback

Prioritize SHA-256 duplicate detection while preserving MD5 fallback for legacy rows.

Remove free-form account notes from statement matching, document direct account-destroy unlinking, and add year-selectable historical coverage with muted out-of-range months.

* fix(statements): harden vault review follow-ups

Clarify legacy MD5 checksum use, whitelist statement balance helper dispatch, and preserve sanitized parser metadata.

Hide statement management controls from read-only viewers while keeping server-side authorization unchanged.

* fix(statements): repair settings system coverage

Allow the changelog provider lookup in the self-hosting settings system test, include Statement Vault in settings navigation coverage, and align the feature title casing. Update the devcontainer so ActiveStorage and parallel system tests can run in the documented environment.

* fix(statements): move vault beside accounts

Place Statement Vault with account settings instead of between Imports and Exports. Keep settings footer ordering and system navigation coverage aligned, including the non-admin visibility guard.

* fix(statements): address vault review cleanup

Resolve CodeRabbit review feedback for statement upload validation, duplicate race handling, account statement matching semantics, metadata detection, ActiveStorage authorization tests, and small UI/style cleanups.

* fix(statements): address vault cleanup review

* fix(statements): deduplicate vault style helpers

* fix(statements): close vault review follow-ups

* fix(statements): refresh schema after upstream rebase

* fix(statements): process vault uploads sequentially

* fix(statements): close vault review follow-ups

* fix(statements): scope vault index to accessible accounts

* fix(statements): harden statement vault readiness

Squash the statement vault migration hardening into the feature migration, tighten Active Storage authorization edge cases, bound CSV metadata detection, and add real PDF fixture coverage for stored statements.

Validation: targeted statement/auth/controller/provider tests, full Rails suite, system tests, RuboCop, Biome, Brakeman, Zeitwerk, importmap audit, npm audit, ERB lint, CodeRabbit, and Codex Security all passed locally.

* fix(statements): close vault review follow-ups

Move statement unlinking to after account destroy commit, keep Kraken account creation on the shared crypto helper, and add statement metadata length limits with DB checks.

Validation: fresh devcontainer with fresh DB via db:prepare, focused account/statement/Kraken/Binance tests, RuboCop, Brakeman, Zeitwerk, git diff --check, CodeRabbit, and Codex Security passed before commit.

* fix(statements): address vault scan follow-ups

Move statement tab data setup out of the ERB partial, harden reconciliation labels and coverage initialization, and tighten statement schema constraints.

Validation: CodeRabbit and Codex Security reviewed the current PR diff; Rails focused tests, full Rails tests, system tests, RuboCop, Brakeman, Zeitwerk, ERB lint, npm lint, importmap audit, npm audit, and git diff --check passed.

* fix(statements): defer vault tab loading

---------

Signed-off-by: Juan José Mata <juanjo.mata@gmail.com>
Co-authored-by: Juan José Mata <juanjo.mata@gmail.com>
2026-05-13 21:05:11 +02:00
ghost
42e7ae677a fix(exports): align CSV roundtrip contracts (#1725)
* fix(exports): align CSV roundtrip contracts

* fix(exports): version CSV export contract

* fix(exports): stabilize CSV export values

* fix(imports): preserve legacy CSV roundtrip contracts

* fix(imports): escape pipe characters in CSV tags

---------

Signed-off-by: Juan José Mata <juanjo.mata@gmail.com>
Co-authored-by: Juan José Mata <juanjo.mata@gmail.com>
2026-05-13 20:07:00 +02:00
Guillem Arias Fauste
b32c378a56 Merge branch 'main' into feat/savings-goals
Signed-off-by: Guillem Arias Fauste <accounts@gariasf.com>
2026-05-13 18:22:55 +02:00
plind
834686cffd fix(simplefin): treat Vanguard/Fidelity cost_basis as total when needed (#1772)
* fix(simplefin): treat Vanguard/Fidelity cost_basis as total when needed

PR #1692 normalized SimpleFIN holdings cost_basis under the assumption
that the `cost_basis` / `basis` keys carry a per-share value (per the
SimpleFIN spec) and only `total_cost` / `value` carry a total position
cost. Vanguard and Fidelity violate the spec — they populate
`cost_basis` with the *total* (see the payload in #1182). After PR
#1692 those holdings get stored with cost_basis = total, and
Holding#calculate_trend then computes previous = qty × avg_cost, so the
"previous" value is inflated by a factor of qty and an entire
investment account renders a phantom return of roughly -(1 − 1/qty),
i.e. -97% to -99%.

Fix: sanity-check raw cost_basis against the holding's market share
price. Let share_price = market_value / qty; the geometric midpoint
between "raw is per-share" (raw ≈ share_price) and "raw is total"
(raw ≈ qty × share_price) is share_price × √qty. If raw is above the
midpoint it is divided by qty; otherwise it is kept as per-share.
Falls back to the pre-fix behaviour (trust the spec) when market_value
or qty is unavailable, so confidently-correct readings are never made
worse.

Verified against the reported Vanguard payload (qty=139, cost_basis=
22004.40, market_value=22626.42): normalize_cost_basis now returns
$158.31/share, matching 22004.40 / 139, and the phantom -99% return
collapses to a realistic ~+2.8%. Per-share readings ($45 cost on a $50
share price) remain untouched.

Closes #1718. Refs #1182, #1692.

* fixup: replace cost_basis heuristic with institution allowlist

Codex and @EdeAbreu23 flagged a real false-positive in the previous
geometric-midpoint heuristic: a legitimate per-share `cost_basis` on a
holding with a large unrealized loss (e.g. 100 shares with $100/share
basis now worth $5/share) trips `share_price × √qty` and gets divided
to $1/share — corrupting any standards-compliant brokerage with a big
loss.

Adopt @EdeAbreu23's safer shape:
- total_cost / value: always divide by qty (unchanged from #1692).
- cost_basis / basis: keep as-is by default.
- Only divide cost_basis / basis when the holding's SimpleFIN account
  is connected to a known-misbehaving institution. Allowlist starts
  with `vanguard` and `fidelity`, matched case-insensitively against
  the account's stored org name and domain. Easy to extend as more
  brokerages turn up.

Trades a small maintenance cost (curated list) for zero risk of
corrupting compliant providers.

Verified against five scenarios (all expected):
  Vanguard total in cost_basis (allowlist) → +2.83%
  Fidelity total in basis (allowlist)      → +33.33%
  Big-loss per-share (Codex case)          → -95.0%  (preserved)
  Honest per-share, small loss             → +11.11% (unchanged)
  total_cost on any institution            → +11.11% (unchanged)

---------

Co-authored-by: plind-junior <plind-junior@users.noreply.github.com>
2026-05-13 18:17:10 +02:00
ghost
95f6451b39 feat(sync): add Brex provider connections (#1752)
* feat(sync): add Brex provider schema

Adds Brex item and account tables with per-family credentials, scoped upstream account uniqueness, encrypted token storage, and sanitized provider payload columns.

* feat(sync): add Brex provider core

Adds Brex item/account models, provider client and adapter support, family connection helpers, and provider enum registration for read-only Brex cash and card data.

* feat(sync): add Brex import pipeline

Adds Brex account discovery, linked-account sync, cash/card balance processors, transaction import, sanitized metadata handling, and idempotent provider entry processing.

* feat(sync): add Brex connection flows

Adds Mercury-style Brex connection management, explicit item-scoped account selection and linking, settings provider UI, account index visibility, localized copy, and per-item cache handling.

* test(sync): cover Brex provider workflows

Adds targeted coverage for Brex provider requests, adapter config, item/account guards, importer behavior, entry processing, and Mercury-style controller flows.

* fix(sync): align Brex API edge cases

Tightens Brex account fetching against the official card-account response shape, sends transaction start filters as RFC3339 date-times, and keeps provider error bodies out of user-facing messages while expanding provider client guard coverage.

* fix(sync): harden Brex provider integration

Restrict Brex API base URLs to official hosts, tighten account-selection UI behavior, and add tests for invalid credentials, cache scoping, and provider setup edge cases.

* test(sync): avoid Brex secret-shaped fixtures

* refactor(sync): extract Brex account flows

* fix(sync): address Brex provider review feedback

* fix(sync): address Brex review follow-ups

Move remaining Brex review cleanup into focused model behavior, tighten link/setup edge cases, localize summaries, and add regression coverage from CodeRabbit feedback.

Also records the security-review pass as no-findings after diff-scoped inspection and Brakeman validation.

* refactor(sync): split Brex account flow controllers

Route Brex account selection and setup actions through small namespaced controllers while keeping existing URLs and helpers stable.

Business flow remains in BrexItem::AccountFlow; the main Brex item controller now only handles connection CRUD, provider-panel rendering, destroy, and sync.

* fix(sync): address Brex CodeRabbit review

* fix(sync): address Brex follow-up review

* fix(sync): address Brex review follow-ups

* fix(sync): address Brex sync review findings

* fix(sync): polish Brex review copy and errors

* fix(sync): register Brex provider health

* fix(sync): polish Brex bank sync presentation

* fix(sync): address Brex review follow-ups

* fix(sync): tighten Brex setup params

* test(api): stabilize usage rate-limit window

* fix(sync): polish Brex setup flow nits

* fix(sync): harden Brex setup params

* fix(sync): finalize Brex review cleanup

---------

Signed-off-by: Juan José Mata <juanjo.mata@gmail.com>
Co-authored-by: Juan José Mata <juanjo.mata@gmail.com>
2026-05-13 18:13:48 +02:00
CrossDrain
7b21a619ec fix(enable-banking): gracefully skip PDNG fetch for ASPSPs that don't support it (#1789)
* fix(enable-banking): gracefully skip PDNG fetch for ASPSPs that don't support it

Some banks reject the PDNG transaction status filter with a 422 validation
error, causing the entire account sync to fail including booked transactions.
Wrap the pending transaction fetch in a rescue block to catch
validation errors from the provider. If the ASPSP does not support
the "PDNG" status, the error is logged and the process continues
without pending transactions instead of failing the entire import.

* fix(enable-banking): gate PDNG fallback on transactionStatus error detail

Tighten the rescue added in the previous commit so it only silences
422s that explicitly mention transactionStatus in the API error body.
Any other validation error (bad date_from, malformed headers, etc.)
re-raises and fails the sync as before, preventing silent data loss.

Tests added for both branches: ASPSP-rejects-PDNG (success) and
unrelated-validation-error (failure).
2026-05-13 17:54:09 +02:00
CrossDrain
406e7217a1 fix(enable-banking): fix pending→posted auto-claim producing badge, duplicate, and wrong date (#1783)
* fix(enable-banking): clear pending flag and prevent stale re-import after auto-claim

When a booked transaction claims a pending entry via the amount/date heuristic
(find_pending_transaction), two bugs caused the entry to remain incorrectly pending
and the old pending transaction to reappear on subsequent syncs.

Bug 1: The extra["enable_banking"]["pending"] flag was never cleared on the claimed
entry. For simple booked transactions with nil extra the deep-merge path is skipped
entirely, so the pending badge persisted forever.

Bug 2: After the claim the old pending external_id (e.g. PDNG_123) stayed in the
stored raw_transactions_payload. The importer's C4 filter only removes pending
entries whose transaction_id matches a BOOK id — Enable Banking issues completely
different ids for pending vs booked transactions — so PDNG_123 was never pruned.
On the next sync find_or_initialize_by(PDNG_123) couldn't find the claimed entry
(now keyed as BOOK_456) and created a fresh pending duplicate with no category.

Fix: on claim, explicitly clear all providers' pending keys from extra in-memory,
and store the displaced pending external_id in extra["auto_claimed_pending_ids"].
The Processor now queries this field alongside manual_merge to build the excluded_ids
set, so the stale pending data is skipped on every future sync.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(enable-banking): preserve pending date when claiming transactions

When a pending transaction is claimed by a booked transaction, the
original pending date is now preserved instead of being overwritten
by the booked transaction's date. This ensures historical accuracy
for transactions that were originally recorded on a different date.

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-13 14:03:37 +02:00
Gian-Reto Tarnutzer
ce5d7dd736 Add Interactive Brokers Provider (#1722)
* Display multi-currency holdings correctly

* Implement IBKR provider

* Fix: Use historical exchange rate for historical prices

* Add brokerage exchange rate for trades

* Sync historical balances from IBKR

* Add logos in activity history

* Fix privacy mode blur in account view

* Improve IBKR XML Flex report parser errors
2026-05-12 23:45:19 +02:00
plind
6402f1dd08 fix(sso): preserve user-edited name across OIDC logins (#1777)
OidcIdentity#sync_user_attributes! runs on every SSO sign-in and
overwrote user.first_name / user.last_name with whatever the IdP sent,
because the precedence was `auth.info.* || user.*` — the IdP always
won when it supplied a value. A user who edited their first name to
"Adam" inside Sure had it reset to the IdP value "Ben" on the next
login, while the last name only "stuck" when the IdP happened not to
return a last_name (#1103).

Swap the precedence to `user.* || auth.info.*` so the IdP fills only
when Sure has nothing on file (first link or admin-blanked field).
Edits inside Sure are then authoritative for every subsequent login.
The audit copy on the OidcIdentity record itself is unchanged, so the
IdP-reported name is still available for debugging.

Closes #1103.

Co-authored-by: plind-junior <plind-junior@users.noreply.github.com>
2026-05-12 21:55:22 +02:00
ghost
0ab3b0b698 feat(exports): add rule operand references (#1726)
* feat(exports): add rule operand references

* fix(exports): preserve rule operand references

* refactor(exports): simplify rule operand branches

* refactor(validation): centralize UUID format checks

* fix(imports): preserve false rule operands
2026-05-12 21:29:29 +02:00
plind
12d799e0b8 fix(binance): support CRYPTO: prefix and USD stablecoins (#1771)
* fix(binance): support CRYPTO: prefix and USD stablecoins

Holdings processors (CoinStats, Coinbase, Kraken, SimpleFIN, Lunchflow,
Binance) store crypto securities with a "CRYPTO:" prefix, but
Provider::BinancePublic#parse_ticker only accepted Binance-search-style
tickers like "BTCUSD". As a result, every fetched price for tickers
like CRYPTO:USDT, CRYPTO:USDC, CRYPTO:SOL, CRYPTO:TRUMP, CRYPTO:KAITO
failed with "Unsupported Binance ticker".

- Strip the CRYPTO: prefix in parse_ticker.
- Short-circuit USD-pegged stablecoins (USDT, USDC, BUSD, DAI, FDUSD,
  TUSD, USDP, PYUSD) to a synthetic flat 1.0 USD price. Binance has no
  self-pair (USDTUSDT is invalid), and the few stablecoin/USDT pairs
  that do exist hover at ~1.0 with sub-cent noise.
- Default prefixed bare base assets (CRYPTO:SOL etc.) to the …USDT
  pair (USD). Only when prefixed, so unprefixed garbage like BTCBNB /
  BTCGBP still returns nil and the existing rejection tests still pass.
- fetch_security_info returns links: nil for stablecoins rather than a
  broken /trade/ URL.

Closes #1441.

* fix(binance): strip CRYPTO: prefix in search_securities

Security::Resolver calls search_provider with the raw holdings-processor
symbol (CRYPTO:SOL, CRYPTO:USDT) before any price fetch. Without prefix
handling here, first-time crypto imports never resolve to an online
Binance security and the new stablecoin/prefix paths in parse_ticker
were unreachable for that flow.

- Strip CRYPTO: from the search query.
- Short-circuit USD stablecoins to a synthetic search result (no
  exchangeInfo call, no Binance self-pair to find).
- Teach parse_ticker the "{stablecoin}USD" form produced by the
  synthetic result so price fetches route to stablecoin_prices.

---------

Co-authored-by: plind-junior <plind-junior@users.noreply.github.com>
2026-05-12 19:41:58 +02:00
Juan José Mata
73b6077ac3 Constrain Lunchflow base URL to trusted endpoint (#1768)
* Constrain Lunchflow base URL to trusted endpoint

Prevent SSRF by ignoring user-provided Lunchflow base_url values unless they match the canonical Lunchflow HTTPS endpoint. Add model tests covering invalid host/scheme and valid canonicalization behavior.

* Linter
2026-05-12 12:18:17 +02:00
Juan José Mata
5ceb55be03 Scope SnapTrade orphan cleanup to current family (#1769)
* Scope SnapTrade orphan cleanup to current family

Restrict orphaned user listing and deletion to SnapTrade user IDs that belong to the current family namespace. Add model tests to prevent cross-family enumeration/deletion regressions.

* Update test/models/snaptrade_item_test.rb

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Signed-off-by: Juan José Mata <jjmata@jjmata.com>

* test: fix snaptrade orphaned users assertion

* style: fix snaptrade test array spacing

---------

Signed-off-by: Juan José Mata <jjmata@jjmata.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: KiloClaw <kiloclaw@openclaw.ai>
2026-05-12 12:17:00 +02:00
Sure Admin (bot)
d943e32b15 fix: correct SnapTrade cash activity signs (#1634)
* fix: correct snaptrade cash activity signs

* test: update snaptrade withdrawal sign expectation

---------

Co-authored-by: SureBot <sure-bot@we-promise.com>
2026-05-12 00:55:46 +02:00
ghost
c1678181f0 fix(imports): import raw balance records (#1724)
* fix(imports): import raw balance records

* fix(imports): preserve partial balance components
2026-05-12 00:41:05 +02:00
Guillem Arias Fauste
7c06fe6296 feat(recurring): allow marking transfers as recurring (#895) (#1589)
Refs #895, discussion #1224.

Adds a "Mark as recurring" entry point on the transfer detail drawer
that creates a `RecurringTransaction` carrying both source and
destination accounts. The recurring index, settings toggle
(`recurring_transactions_disabled`), and projected upcoming feed all
light up automatically once the data shape is there.

Schema:

* `destination_account_id` nullable FK to accounts. `on_delete: :cascade`
  matches #20251030172500's precedent for accounts FKs. The existing
  `account_id` FK is widened to cascade in the same migration so
  Family destruction with a recurring transfer doesn't FK-violate.
* Two predicate-partitioned partial unique indexes per shape:
  non-transfer rows (`destination_account_id IS NULL`, original
  5-column shape preserved) and transfer rows (6-column shape
  including the destination). Postgres treats NULLs as distinct in
  unique indexes, so widening would have broken non-transfer dedupe.
* Two CHECK constraints enforcing transfer invariants in PostgreSQL:
  `chk_recurring_txns_transfer_requires_source` (destination implies
  source) and `chk_recurring_txns_transfer_distinct_accounts`
  (destination cannot equal source). Per CLAUDE.md "Enforce null
  checks, unique indexes, and simple validations in the database
  schema for PostgreSQL".
* `Account` gains an `inbound_recurring_transfers` inverse so the
  destroy chain reaches both ends.

Controller / behaviour:

* `transfers#mark_as_recurring` mirrors `transactions#mark_as_recurring`:
  i18n flashes (4 new keys: transfer_marked_as_recurring,
  transfer_already_exists, transfer_creation_failed,
  transfer_feature_disabled), `respond_to format.html`,
  `redirect_back_or_to transactions_path`, server-side gate on
  `recurring_transactions_disabled?`, and rescue both `RecordInvalid`
  and `RecordNotUnique` for the race window between the dedupe
  `find_by` and `create_from_transfer`. The `StandardError` rescue
  now logs the exception (class, message, transfer/family/user ids)
  before surfacing the generic flash so production failures aren't
  context-less.
* `RecurringTransaction.accessible_by(user)` now requires
  destination_account_id (when present) to be in the user's
  accessible set, so a recurring transfer never leaks to a user
  without access to BOTH endpoints.
* Model validation gains a `destination_account.blank?` branch in
  `transfer_endpoints_consistent` so a dangling
  `destination_account_id` (referenced row destroyed) surfaces as a
  normal validation error instead of an FK exception on save.
* `Identifier` filter for transfer-kind transactions moved into SQL.

UI:

* Recurring index table and projected feed render transfer rows with
  the existing letter-avatar and the row's `name` field
  ("Transfer to {destination}"). No special pill or icon -- every row
  in `/recurring_transactions` is recurring by definition. Amount
  column on transfers uses `text-secondary` (muted-but-live) instead
  of the income/expense colour, since transfers are zero-net for the
  family.

Out of scope (called out in the PR body):

* Auto-creation of future Transfer rows on a schedule
  (discussion #1224's primary ask). Behaviour change vs the
  current projection-only model.
* Auto-identification of recurring transfer pairs in `Identifier`.
* Frequency model richer than `expected_day_of_month`.
* `Cleaner` for recurring transfers (issue #1590 tracks this).

Tests:

* `RecurringTransaction#transfer?` predicate (with / without
  destination).
* `transfer_endpoints_consistent`: rejects same source and
  destination, rejects dangling destination_account_id, rejects
  cross-family destination.
* `RecurringTransaction.create_from_transfer` happy path;
  multi-currency variant stores source-side currency.
* `projected_entry` exposes source / destination on transfer rows.
* `Identifier` skips transfer-kind transactions; creates a pattern
  from expense halves while ignoring co-resident transfer halves.
* Destroying the destination account cascades to inbound recurring
  transfers (FK + AR association).
* Unique partial index still de-duplicates non-transfer rows after
  the destination_account_id widening.
* `transfers#mark_as_recurring` happy path, idempotent on second
  call, rejected when `recurring_transactions_disabled`.

Suite: 3261 / 0 / 0 / 24 on the latest upstream/main. Lint clean.
Brakeman clean.

Signed-off-by: Guillem Arias Fauste <gariasf@proton.me>
2026-05-12 00:37:47 +02:00
ghost
be598aecf0 feat(providers): add Kraken exchange sync (#1759)
* feat(providers): add Kraken exchange sync

Adds family-scoped Kraken API-key connections, read-only balance and trade import, account setup/linking flows, provider status wiring, and focused test coverage.

Closes #1758

* test(providers): avoid Kraken sample secret false positive

* fix(providers): address Kraken review findings

* fix(providers): address Kraken review cleanup

* test(imports): stabilize transaction import ordering
2026-05-12 00:22:37 +02:00
CrossDrain
33bc6b59c8 fix(enable-banking): import transactions missing transaction_id and entry_reference (#1767)
* fix(enable-banking): handle transactions missing transaction_id and entry_reference

Some ASPSPs omit both transaction_id and entry_reference from their transaction payloads, which is valid per the PSD2/Berlin Group spec. Previously, every such transaction raised an ArgumentError and was silently dropped during sync.

compute_external_id now falls back to a deterministic MD5 fingerprint (prefixed enable_banking_content_) derived from date, amount, currency, direction, counterparty, and remittance info. This fingerprint is stable across re-syncs, so duplicate imports are still correctly prevented. An ArgumentError is only raised for truly empty/unidentifiable payloads.

The importer is updated in three places to use compute_external_id
consistently: the pending pre-filter (before combining with booked),
the C4 stored-pending cleanup, and the new_transactions dedup. This means ID-less pending entries are now also removed when their settled booked counterpart arrives.

Tests cover compute_external_id directly (all 5 cases), end-to-end
fingerprint import, idempotency, and importer storage/dedup behaviour for ID-less transactions including the pending→booked settlement path.

* fix(enable-banking): implement dual-strategy matching for transaction settlement

When a stored pending row had only entry_reference (no transaction_id) and
the settled BOOK row arrived with a new transaction_id, compute_external_id
produced different fingerprints for each side (enable_banking_<ref> vs
enable_banking_<txn_id>). The fingerprint-only comparison introduced in the
previous commit never matched, leaving the stale pending entry in
raw_transactions_payload. Both rows were then imported as separate visible
transactions.

Restore a book_entry_refs set alongside book_fingerprints in both the
pending pre-filter and the C4 stored-pending cleanup. A pending entry is
now removed when either its fingerprint or its entry_reference matches a
booked counterpart — covering same-ID settlement, content-fingerprint
settlement, and the entry_reference cross-match settlement path.

Also updates the ArgumentError message in external_id to accurately
reflect that transaction_id, entry_reference, and content fingerprint
are all accepted identifiers, and aligns build_transaction_content_key
to use transaction_date as a fallback (matching compute_external_id).

Adds a regression test that stores a pending-only row and asserts it is removed when the booked counterpart arrives with a new transaction_id.
2026-05-12 00:17:49 +02:00
ghost
1fedc43f68 feat(api): add import preflight validation (#1755)
* feat(api): add import preflight validation

* fix(api): harden import preflight validation
2026-05-12 00:00:49 +02:00
ghost
6b6c3bd343 feat(exports): add attachment manifest (#1728)
* feat(exports): add attachment manifest

* fix(exports): include split parent receipts in manifest
2026-05-11 23:47:36 +02:00
Guillem Arias
cf4e560a4c feat(goals): extract shared color_icon_picker controller; add icon to goals; tinted avatar
User requested replacing the in-house color disclosure with the
categories color+icon popover. Done as a controller extraction so
categories and goals share one Stimulus controller (user's option:
"Extract a shared color_icon_picker_controller.js").

- `git mv` app/javascript/controllers/category_controller.js to
  color_icon_picker_controller.js. Categories form + color_avatar
  partial updated to use the new identifier (data-controller=
  "color-icon-picker", target/action selectors renamed).
- Goal model gains an icon column (migration
  20260511190000_add_icon_to_goals.rb) + ICONS = Category.icon_codes
  + inclusion validation. GoalsController permits :icon in
  goal_params + goal_update_params.
- Goals::AvatarComponent now renders icon when present (falls back to
  first-letter initial), and adopts the Categories tinted-bg + colored
  -content style (bg = `color-mix(in oklab, COLOR 10%, transparent)`,
  text/icon = COLOR). Matches the picker's live preview so what the
  user sees during selection equals the saved state.
- New goals/_color_picker.html.erb mirrors categories/_form's popover:
  avatar + pen overlay summary + popup with color row (+ rainbow
  custom-hex trigger) + icon grid. Pickr / contrast validation / auto-
  adjust all inherited from the shared controller.
- Stepper step 1 layout: drop the inline letter-avatar (data-goal-
  stepper-target="avatarPreview") in favour of the picker avatar next
  to the name input. Step 1's tail no longer renders a separate color
  partial. Edit form passes icons local through.

Verified live: new goal modal renders 11 color radios (10 presets +
custom) + 141 icon radios + pen-summary; categories form still
operational (no console errors) under the renamed controller.
2026-05-11 21:28:23 +02:00
Guillem Arias
9b61e4a41b refactor: rename Savings Goals feature to Goals
User-facing rename + structural rename. Feature is now called just
"Goals" everywhere — page title, sidebar nav, modal headings, flash
messages, AI assistant tool. Code identifiers follow:

- Models: SavingsGoal → Goal, SavingsContribution → GoalContribution,
  SavingsGoalAccount → GoalAccount.
- Tables: savings_goals → goals, savings_contributions → goal_contributions,
  savings_goal_accounts → goal_accounts. FK columns savings_goal_id →
  goal_id. New migration db/migrate/20260511100003_rename_savings_to_goals.rb
  uses rename_table + rename_column; PG handles index renaming and FK
  redirection automatically.
- Controllers: SavingsGoalsController → GoalsController,
  SavingsContributionsController → GoalContributionsController.
- Routes: /savings_goals → /goals, nested /goals/:id/contributions
  (resource name shifts; old route name aliases dropped).
- ViewComponent namespace: Savings::* → Goals::*. Component class
  names drop their redundant "Goal" prefix where the namespace already
  carries it: Savings::GoalCardComponent → Goals::CardComponent,
  Savings::GoalAvatarComponent → Goals::AvatarComponent. Others keep
  their names (Goals::ProgressRingComponent, Goals::StatusPillComponent,
  Goals::AccountStackComponent, Goals::FundingAccountsBreakdownComponent).
- Stimulus controllers: savings_goal_* → goal_*, savings_goals_filter
  → goals_filter. Stimulus identifiers in data-controller / data-*
  attributes follow.
- Locale keys: savings_goals: → goals: (top level), savings_contributions:
  → goal_contributions: (top level). All t() callers updated.
- AI assistant tool: Assistant::Function::CreateSavingsGoal →
  Assistant::Function::CreateGoal, tool name "create_savings_goal" →
  "create_goal", description / response text updated.
- Sidebar nav label "Savings" → "Goals". Goals/show + index page title
  "Savings" → "Goals". Empty goals_section heading/subtitle dropped
  (duplicated the page title post-rename).

Original migrations create_savings_goals / create_savings_goal_accounts /
create_savings_contributions remain untouched so historical replay
still works; the rename migration runs on top.
2026-05-11 20:08:32 +02:00
Guillem Arias
37dfd32628 fix(savings_goals): use display_status for inactive goals; hide pace + projection
- SavingsGoal#display_status returns :archived / :paused before falling
  through to the visualization status. Memoized like #status. The plain
  #status method keeps its meaning (visualization vs. target/pace) so
  callers that genuinely want "is this on track" — KPI sort, goal-card
  ring color, projection_payload — keep working unchanged.
- Savings::StatusPillComponent: status_key uses display_status; new
  :archived variant (bg-surface-inset / text-gray-700 / archive icon).
  Previously an archived goal showed "Behind" on the detail page while
  the archived banner said the goal was archived — conflicting signal.
- show.html.erb: paused/archived goals render a static recap card
  (current saved vs target) instead of the projection chart. Pace stat
  (avg vs required monthly) is also hidden — extrapolating "Behind by
  $X/mo" against a goal that isn't accepting contributions is misleading.
- New locale keys: savings_goals.status.archived,
  savings_goals.show.inactive.{heading_paused, heading_archived, body}.
- Tests cover display_status for archived / paused / active goals.
2026-05-11 19:40:21 +02:00
Guillem Arias
6be5f813a4 perf(savings_goals/show): preload + memoize cut show from 17 to 5 queries
- set_savings_goal: with_current_balance + includes(savings_contributions: :account, linked_accounts: []) so contributions / accounts / current balance don't re-query inside helpers and view partials
- SavingsGoal#status + #average_monthly_contribution: defined?(@ivar) memoization so the 5+ callsites per show (header banner, projection_summary, donut, goal-card pace, stats_for) don't recompute the exists?/MIN/SUM triplet each time
- SavingsGoal#projection_payload: sort loaded contributions in Ruby instead of running a fresh ORDER BY
- SavingsGoalsController#show: replace .chronological re-query with in-memory sort over the preloaded association
- funding_breakdown_for: group_by + transform_values off the loaded collection instead of an extra GROUP BY SQL
- stats_for: contributions_count uses .size to read the loaded cache instead of issuing COUNT(*)
2026-05-11 19:25:08 +02:00
Guillem Arias
6254a02602 fix(savings_goals/show): chart axis includes backdated contributions, legend uses real colors
projection_payload's start_date was created_at, but demo seeds (and
manual imports) can have contributions backdated before created_at —
those points were getting clipped/pushed left of the chart's x-domain
and the saved-series line couldn't render. Use min(created_at,
earliest contribution date) so the axis spans the full history.

Legend "saved" line stroke was var(--text-primary) which doesn't
resolve (Tailwind utility, not CSS var) → invisible swatch. Wrap in
text-primary span + stroke="currentColor".

Legend "projection" line was hardcoded yellow — chart paints green for
on_track goals → mismatch. Pick legend color based on goal status so
it matches what the chart actually draws.
2026-05-11 16:49:47 +02:00
Guillem Arias
ed9759b87b feat(savings_goals): demo variety, breadcrumb naming, ring token, list pattern, header split, tone down behind noise
Demo — extend generate_savings_goals! with three more goals to exercise
status-specific UX: Wedding fund (on_track w/ 6 months of contributions
matching required pace), Sabbatical (paused), Old laptop fund (archived).
House downpayment gains 12 contributions so the scrollable list has real
density. Total now 7 demo goals covering behind / on_track / no_date /
paused / archived / reached.

Breadcrumbs — set @breadcrumbs on index too (it was relying on the
Rails-derived "Savings goals" label). Both views now read "Home →
Savings → ..." consistently, matching the sidebar nav text and H1.

Ring token — goal-card ring stroke switched from var(--color-gray-200)
(a hard light color identical in both themes) to
var(--budget-unallocated-fill) which is gray-50 light / gray-700 dark,
matching the detail page's progress ring.

Contributions list — replace the inline hover-revealed delete-X with
DS::Menu kebab, matching tags/_tag.html.erb and categories/_category.
Each row also gets hover:bg-surface-hover with a px-3 -mx-3 negative
margin to extend the hover area across the card padding. Non-manual
contributions render a 9x9 spacer so the right column stays aligned.

Header sub split — drop the long "·" chain into two lines: primary fact
(target / days left) in text-secondary, recency note in text-subdued
underneath. Less wall-of-text.

Behind noise — pill, ring, catch-up alert and projection chart already
signal "behind". The Monthly-pace combo card's "Behind by $X/mo" delta
no longer renders in text-warning — it switches to text-subdued so the
warning palette doesn't repeat across the page. The catch-up alert stays
loud because it's the primary action; the rest stays informational.

CustomConfirm wired with destructive: true on the contribution delete so
the confirm button gets the outline-destructive treatment.
2026-05-11 16:37:11 +02:00
Guillem Arias
733f7e15b9 feat(savings_goals/new): step 2 initial-contribution amount uses money_field
Add virtual attr_accessors for initial_contribution_amount and
initial_contribution_account_id on SavingsGoal so the form builder can
bind to them without the model needing real columns.

Replace the raw number_field_tag with f.money_field hide_currency: true
so the field shows the currency symbol prefix and step-aware precision,
matching the Target amount field in step 1.
2026-05-11 15:48:08 +02:00
Guillem Arias
f51e38f4fc feat(savings_goals): drop Accounts section from index
The Accounts grid duplicated the sidebar account list. Removing it gives
the Goals section more breathing room and the page a tighter narrative:
header → KPIs → Goals.

Delete Savings::AccountCardComponent, Family#savings_subtype_accounts,
the @savings_accounts / @account_goal_counts controller refs, and the
related locale keys. Sidebar still shows the savings-subtype Depository
accounts under "Cash" — no information is lost.
2026-05-11 14:23:00 +02:00
Guillem Arias
3e05ea8670 feat(savings_goals): goal card pace + status-driven footer
Each card now answers "what's my next move" without clicking into the
detail page. Under the amount/target row, a pace line shows actual avg
contributions vs the monthly target. The footer (previously "$X left")
switches by status:

- behind  → "Save $Y/mo to catch up"
- on_track → "Last contribution Nd ago" (or "today" / "No contributions yet")
- reached / completed → "Goal reached"
- no_target_date → "No deadline set"
- paused → "Paused"

Add SavingsGoal#last_contribution_at and #last_contribution_days_ago.
Both these methods and average_monthly_contribution now respect a loaded
:savings_contributions association so the index page doesn't N+1.
Controller eager-loads :savings_contributions + :linked_accounts.
2026-05-11 14:20:40 +02:00
Guillem Arias
8ba6cbcdc8 feat(savings_goals): replace hero card with KPI strip + differentiate empty states
P1: drop the sparkline + the single mixed hero. Hero became 3 separate
KPI cards (Contributed last 30d, Needs this month, Goals on track),
matching the Transactions page pattern. Each KPI answers a question the
user opens the page asking — saving rate, this-month action, overall
health.

P3: empty state copy + CTA now reflect the reason it is empty. Search
returns 0 → "No goals match X" + Clear search. Chip set to non-all → "No
goals match this filter" + Show all. Both → both reasons + both
buttons.

Drop: total_savings_balance, savings_balance_series,
savings_balance_30d_delta on Family (no other consumers).
Add: Family#contribution_velocity(range:).
2026-05-11 14:14:37 +02:00
Guillem Arias
dad9cf70b6 feat(savings): rebuild index to match Claude Design
- Page header: title "Savings" + "Your savings accounts and the goals
  you're working toward." Removed the top-right New goal button (moves
  into the Goals section).
- Hero card: "Total in savings" with sum-of-savings-subtype balance,
  30-day delta vs last 30 days (Family#savings_balance_30d_delta),
  3-stat sub-row (Accounts / Active goals / Saved toward goals), and a
  D3 sparkline area chart on the right (new
  `savings-sparkline` Stimulus controller, sourced from
  Family#savings_balance_series).
- Accounts section: lists Depository accounts with subtype = "savings"
  as cards (blue avatar, name, subtype, balance, "Funds N goals"). New
  Savings::AccountCardComponent.
- Goals section header: "Goals" + "Save toward what matters." + "New
  goal" button right-aligned to the section (not the page header).
- Removed state-filter pill nav. Active goals render in the main grid;
  Completed goals get a "Completed · N" divider w/ check-circle icon
  and their own grid below.
- Goal card layout reworked: horizontal bar replaced with a 64px donut
  ring on the right side of the card header (ring colour tracks
  goal.status — yellow=behind, primary=on-track, green=reached). Pill
  is inline with the goal name.
- Status pill copy: "Behind pace" → "Behind".
- Filter bar (copied from settings/providers): search input + status
  chips (All / On track / Behind / No date). Hidden when ≤ 6 active
  goals. Powered by `savings-goals-filter` Stimulus controller —
  toggles `.hidden` on cards by goal name + status.
- Family#savings_subtype_accounts, total_savings_balance,
  savings_balance_series, savings_balance_30d_delta helpers; controller
  computes hero payload + account-goal counts for the cards.
2026-05-11 12:18:57 +02:00
Guillem Arias
696fbc0b43 feat(savings): match Claude Design — projection chart, target-icon modal, grouped funding accounts
Brings the savings goals UI closer to the Claude Design reference shared
by the user. Changes:

- Sidebar nav label: "Savings goals" → "Savings".
- Status pill copy: "Behind" → "Behind pace" (matches Pill component
  from GoalsCommon.jsx).
- Empty state rewritten with a large target icon, "No goals yet"
  heading, and the descriptive body copy from the design.

Goal detail page (matches GoalDetail.jsx):
- New "← All goals" back link above the header.
- 2-column hero: ring card on the left (320px column), Projection card
  on the right.
- Projection card uses a new D3 Stimulus controller
  (`savings-goal-projection-chart`) that draws:
    · saved area + line from goal creation → today (solid, primary)
    · dashed projection segment from today → target date (yellow when
      behind, green when on track)
    · horizontal dashed target line with label
    · today marker (vertical dashed line + dot)
  Data shape comes from `SavingsGoal#projection_payload`.
- Card subtitle generates a contextual sentence ("At $X/mo you'll fall
  short. Bump to $Y/mo to hit it on time." / "At your current pace
  you'll reach this goal around Month YYYY." / "Goal reached. Nice
  work.") with a strong tag highlighting the actionable figure.
- Stat row now shows Linked balance (sum across linked accounts) +
  "N accounts" sub-caption instead of duplicate "Target date" stat.

New goal modal (matches the design images 2 + 3):
- DS::Dialog custom header: DS::FilledIcon target glyph + title + step
  subtitle ("Step 1 of 2 · Goal details" / "Step 2 of 2 · Review &
  start") that updates as the user advances.
- Connected stepper at top of body: numbered circles connected by a
  bar, step-1 circle flips to ✓ when complete.
- Step 1 heading "What are you saving for?" + supporting copy.
- Name field paired with a target glyph affordance on its left.
- Target amount + Target date in a 2-col grid.
- Funding accounts list now grouped by account subtype with uppercase
  section headers (CHECKING / SAVINGS / HSA / CD / MONEY MARKET /
  OTHER), each row showing avatar + name + subtype + balance.
- Step 2 heading "Looks good?" + Review card (goal target + funding
  accounts summary + suggested monthly = target/months_remaining), and
  a disclosure for the optional initial contribution.
- Footer: "Cancel" left text-button (closes modal) / "Back" left text
  when on step 2; "Continue →" or "Create goal →" right arrow button.

Demo generator: Depository accounts now set `subtype` ("checking" /
"savings") on the accountable so they group correctly in the modal.

Tests: all green, 35 runs in the savings suite, 92 assertions.
2026-05-11 12:08:47 +02:00
Guillem Arias
39743a9ec4 feat(savings): rebuild UI to match Claude Design + adopt shared donut-chart
Previous savings goals UI looked nothing like the Claude Design output
(see sure-design-context/design/savings-goals/project/goals/*.jsx) and
the hand-rolled ring did not match the segmented D3 donut used at
app/views/budgets/_budget_donut.html.erb. This rewires the surface end
to end.

Donut chart:
- SavingsGoal#to_donut_segments_json returns the same segment shape as
  Budget#to_donut_segments_json: filled portion in goal color, unused
  remainder as `var(--budget-unallocated-fill)`. Visual identity is now
  the same: segmented arc with cornerRadius and gap, courtesy of the
  shared `donut-chart` Stimulus controller and D3.
- ProgressRingComponent renders a `data-controller="donut-chart"` div
  with the same default-content/inner-text pattern as `_budget_donut`.

Index page (matches GoalsIndex.jsx):
- Page header: title + "Save toward what matters." subtitle + "New goal"
  primary CTA right-aligned.
- Summary strip card: total saved / target, overall bar, active goals,
  on-track ratio, behind count.
- State filter rendered as DS::Tabs-style pill nav (`bg-surface-inset
  p-1 rounded-lg`, white-pill active state).
- Cards rebuilt: avatar (44px, rounded-xl, white initial on goal color)
  + name + secondary line ("N days left · by date" / "No target date" /
  "Completed" / "Past due"), status pill with leading dot, big
  $current/$target line + percent, bar in status colour, AccountStack
  (overlapping initials) + "N accounts" + "to go".

Goal detail (matches GoalDetail.jsx):
- Header: 64px avatar + h1 name + status pill + "Target $X by date ·
  N days left" subline + Edit (outline) + Add contribution (primary) +
  kebab (DS::Menu for AASM transitions).
- Donut-chart ring card with stats overlay.
- 4-col stat row (Avg monthly, Total contributions, Target date,
  Started) with mono numerals and "Needs $X/mo" / "Above target pace"
  sub-captions where relevant.
- Two-col bottom: contributions list (avatar + account · date · source
  · green +$amount) and funding accounts breakdown (stacked bar +
  per-account row with $ and % of saved).

New components: Savings::AccountStackComponent (overlapping account
initials with ring-2 ring-container). StatusPillComponent now uses a
leading colored dot instead of an icon. GoalAvatarComponent radii
match Claude Design (rounded-md/lg/xl/2xl) and white initial.

Locale: new keys under savings_goals.{index.subtitle, index.summary.*,
goal_card.{accounts,days_left,completed,past_due,no_target_date},
show.header.*, show.ring.{of,to_go}, show.stats.*, show.funding_balance,
show.of_saved, show.notes}.
2026-05-11 11:52:35 +02:00