## Summary
Adds the Nigerian Naira (NGN, ₦) as a selectable currency.
Conversion support required no code change: rates come from the
fawazahmed0 currency-api CDN, which already serves NGN (verified live,
e.g. `EUR→NGN ≈ 1566.8`). `ExchangeRateService` /
`CurrencyConversionService` are currency-agnostic, so NGN converts as
soon as it's a valid option.
## Changes
- `config/currencies.php` — add NGN entry (allowed as both primary and
account currency).
- `resources/js/utils/currency.ts` — add `₦` to the symbol map.
- `lang/es.json` — Spanish translation for the currency name.
- `tests/Feature/CurrencyOptionsTest.php` — assert NGN is exposed as a
primary and account currency.
## Testing
- `php artisan test tests/Feature/CurrencyOptionsTest.php
tests/Feature/CurrencyConversionServiceTest.php
tests/Feature/LocalizationTest.php` — pass
- `vendor/bin/pint`, `bun run format`, `bun run lint`, `vitest run
currency.test.ts` — pass
## Summary
Wave 1 hardening: privacy/security quick wins on the Inertia boundary, a
CI safety net, and stricter test isolation. Four focused changes plus
two review fixes, each in its own commit. No dependency or
product-behavior changes.
## Changes (by commit)
1. **Hide sensitive User fields from serialization** — `User::$hidden`
only covered password / 2FA / remember_token, leaving Cashier billing
columns (`stripe_id`, `pm_type`, `pm_last_four`, `trial_ends_at`) and
the legacy `encryption_salt` exposed in every serialized User, including
the Inertia-shared `auth.user` prop. None are read by the frontend and
Cashier keeps reading them server-side, so hiding them is invisible to
the UI and billing.
2. **Advisory frontend type-check in CI + duplicate `currency_code`
fix** — the CI linter job never ran `tsc`, so type errors accumulated
unseen. Adds a `Type Check Frontend` step running `bun run types`. It is
`continue-on-error: true` on purpose: the codebase already carries ~157
pre-existing `tsc --noEmit` errors, so gating on it now would turn CI
red on unrelated code. It surfaces type output today and should be
flipped to blocking once the backlog is cleared. Also removes a
duplicate `currency_code` member on the `User` TS interface (declared
twice, `CurrencyCode` and `string | null`); the TS language server flags
it, `tsc` masks it under `skipLibCheck`.
3. **Move residual-encryption cleanup out of Inertia `share()` into a
queued job** — `share()` is a shared-data provider and must be
read-only, but it ran a `DELETE`+`UPDATE` against the user on every
non-API web GET to purge the leftover encryption salt /
`EncryptedMessage` once a user had no encrypted data left. The existing
`encryption:*` commands do not cover this case (both target users who
*still* have encrypted data; this finalizes users who *finished*
decrypting). The work now goes to a new idempotent
`PurgeResidualEncryptionArtifactsJob` dispatched from `share()`,
preserving the eventual-cleanup semantics without writing during the
render.
4. **Block stray HTTP in the Feature test suite** — adds
`Http::preventStrayRequests()` in a Feature-scoped `beforeEach` so any
unfaked outbound request fails loudly instead of hitting the network. A
representative HTTP-touching subset (open banking,
exchange-rate/currency, AI categorization + AI/stats reports, analytics,
Discord, Stripe, bank logos) was run with the guard active; no test
relied on a real request, so no fakes had to be added.
### Review fixes (after the two-reviewer pass)
5. **Purge check uses `name_iv` source of truth, not the stale
`encrypted` flag** — the destructive purge decided "no encrypted
accounts" from `accounts.encrypted`, a flag the codebase already treats
as unreliable (see the
`align_accounts_encrypted_flag_with_plaintext_names` migration and
`FindsUsersWithLegacyEncryption`, which key off `name_iv`). An account
with an encrypted name but a stale `encrypted=false` flag could have its
key material destroyed. The job's guard now matches the canonical `*_iv`
predicate exactly; the loose flag gate in `share()` is kept only as a
cheap dispatch filter, and the job re-verifies with the safe predicate
before touching anything.
6. **Deduplicate purge dispatches with `ShouldBeUnique`** — `share()`
runs on every web GET, so an affected user re-enqueued the job on each
page load until a worker cleared the salt. The job is now
`ShouldBeUnique` keyed by user id, collapsing repeat dispatches into one
pending job.
## Test plan
- New/updated tests, all green:
- `InertiaSharedDataTest`: `auth.user` omits all sensitive fields; a web
GET no longer mutates the user inline and instead queues the cleanup
(and does not queue it when there is no salt).
- `PurgeResidualEncryptionArtifactsJobTest`: clears salt + message when
no `*_iv` data remains; keeps them when an encrypted transaction, an
encrypted account name, or a stale-flag-but-encrypted-name account
exists; no-op when salt already null.
- `StrayHttpRequestGuardTest`: an unfaked request throws
`StrayRequestException`; a matched fake still resolves.
- Green locally: `vendor/bin/pint --test`, `bun run lint` (0 errors),
`bun run format:check`, `bun run test` (254 frontend tests), targeted
backend
`--filter=InertiaSharedData|PurgeResidualEncryptionArtifactsJob|StrayHttpRequestGuard|Encryption`
(24 tests). Ran ~700 HTTP-touching Feature tests with the stray-request
guard active with no guard-induced failures.
- `bun run types` still reports the ~157 pre-existing errors (unchanged
set; this PR adds none) — that is exactly why the CI step is advisory
for now.
## Reviewer findings — addressed vs deferred
**Addressed**
- 🟠 Destructive purge keyed off the stale `accounts.encrypted` flag
instead of the `name_iv` source of truth → fixed in commit 5 (job now
mirrors `FindsUsersWithLegacyEncryption`; added a regression test for
the stale-flag case).
- 🟡 Per-request dispatch amplification with no dedup → fixed in commit 6
via `ShouldBeUnique`.
**Deferred (with rationale)**
- 🟠 "Three divergent copies of the legacy-encryption query" —
substantively resolved: the job now matches
`FindsUsersWithLegacyEncryption` exactly. The only remaining
`encrypted`-flag use is the `hasEncryptedAccounts` **UI prop** in
`share()`, which is a separate, pre-existing frontend concern; changing
it would alter which accounts the UI treats as encrypted and is out of
scope here. A full extraction into one shared scope would require
restructuring the trait (it builds a `User` query, not a per-model
boolean) and is not a Wave 1 quick win.
- 🟢 Redundant `->fresh()` / null guard in the job — kept deliberately:
it is the idempotency guard that makes the re-check read committed state
on the sync path (the second reviewer credited it as what makes repeat
dispatches safe).
- 🟢 `continue-on-error` shows the type-check step green — acknowledged;
flipping to blocking (or failing on an increase over a committed
baseline) is the follow-up once the ~157-error backlog is cleared.
- 🟢 (Product review) Theoretical one-request SSR/client
`hasEncryptionSetup` diff from async salt clearing — invisible in
practice (the lock button gates on `hasEncryptedAccounts ||
hasEncryptedTransactions`, false in both SSR and client), and `ssr.tsx`
is untouched. No action.
Product-bug reviewer verdict: no user-facing regressions. Hiding the 5
fields does not affect Cashier (raw attribute access),
`EncryptionController`, notifications, or any API/JSON path; the
`currency_code` dedup is runtime-identical; the cleanup timing change is
client-absorbed.
## Why
The drip email family was near copy-paste. Each of the eight mailables
repeated the same constructor, `$tries`/`$backoff`, `drip_from`
envelope, `markdown` + `userName` content, and `RateLimited` middleware
— differing only in subject and template (plus a reply-to on one, an
extra view var on another). Each of the eight jobs repeated the
`canReceiveEmails` / `hasReceivedEmail` guards, the `Mail::send` call,
and the `UserMailLog::create` block — differing only in the email type,
the mailable, and a handful of per-email eligibility checks.
## What
Two abstract bases, so each subclass declares only what varies:
- **`DripMail`** — owns the shared mailable shape. Subclasses declare
`dripSubject()` and `template()`; optional `contentData()` (PromoCode's
`FOUNDER` var) and `repliesToSender()` (PaywallFollowUp) hooks. The
hooks are named `dripSubject()`/`template()` deliberately to avoid
colliding with `Illuminate\Mail\Mailable`'s existing public
`subject()`/`markdown()` methods.
- **`SendDripEmailJob`** — owns the shared `handle()` flow (shared
guards → send → `UserMailLog`). Subclasses declare `emailType()` and
`buildMail()`; an optional `shouldSend()` hook carries the per-email
eligibility rules (pro-plan, onboarding, transactions, bank connections,
AI consent).
Each mailable drops from ~68 to ~13 lines and each job from ~48 to ~18.
Net **−459 lines**, with the send/log logic and envelope construction
now in one place.
## Behavior
No functional change. Subjects, `from`/`reply-to`, queue name, rate
limiting, templates, view data, and every per-job eligibility guard are
preserved. Notably, passing `replyTo: []` for the seven non-reply mails
is identical to the previous omission — `Envelope`'s `replyTo` defaults
to `[]`. Verified: `pest --exclude-testsuite=Browser` **1872 passing, 0
failing**; `pint --test`, `phpstan` (level 5), `format`, `lint` clean.
## Review-driven follow-up commits
Two review agents (architecture/duplication/coverage, and
behavior/regressions) ran against the first commit. The behavior agent
verified all eight jobs' guard logic invert correctly (order +
negations) and that `replyTo: []` ≡ omitted — no regressions. The
coverage agent found the two hook-driven behaviors were untested;
applied, each as its own commit:
- Extended the drip-sender parity test to all eight mailables and
asserted `PaywallFollowUpEmail`'s reply-to (and that a default drip mail
sets none).
- Asserted `PromoCodeEmail` injects the `FOUNDER` promo code into its
view data.
## Deliberately out of scope
- Non-drip mailables (`Waitlist*`, `Banking*`, `UpdateEmail`, …) share
the same `RateLimited('emails')->releaseAfter(1)` middleware and could
later share a queued-mail base too — separate concern.
- **Pre-existing** (not introduced here, noted by review): the job sends
the mail before writing `UserMailLog`, so a failure between the two can
re-enqueue a duplicate; there is no lock/unique constraint on `(user_id,
email_type)`. Worth a separate hardening pass.
## Why
The five API-key OpenBanking "connect" controllers — Binance, Bitpanda,
Coinbase, Indexa Capital and Interactive Brokers — each carried a
~60-line `store()` that was near-identical: subscription gate →
credential validation → `Bank::firstOrCreate` → create the
`BankingConnection` (Pending) → update it to `AwaitingMapping` with
pending accounts → auto-map during onboarding or redirect to mapping.
Any change to that flow, or a bug in it, had to be made five times.
## What
Extract the shared flow into a small class hierarchy, so each controller
declares only what actually varies per provider.
- **`OpenBankingConnectController`** (abstract) owns the whole flow in a
`connect()` template method, with per-provider extension points:
`provider()`, `providerName()`, `bankLogo()`, `aspspCountry()`,
`fetchProviderData()`, `credentialErrorMessage()`,
`buildPendingAccounts()`, and an optional `emptyProviderDataMessage()`
guard.
- **`CryptoPortfolioConnectController`** (abstract, extends the above)
implements the two hooks the crypto providers share: `aspspCountry()`
(from the request) and the single "Crypto Portfolio"
`buildPendingAccounts()` (uid derived from the provider enum value, so
the generated uids are unchanged).
- The five controllers shrink to their genuine differences (client,
names, logo, error copy, and — for IB — the Flex error mapping and
empty-statement guard).
Net **−110 lines** across the five `store()` methods, and the connection
lifecycle now lives in one place.
## Behavior
No functional change. The credential-failure warning log is now a single
structured message with a `provider` key instead of five free-text
variants (the only observable difference; HTTP responses are
byte-for-byte identical). Verified by the full suite: **1869 passing, 0
failing** (`--exclude-testsuite=Browser`); `pint --test`, `format` and
`lint` clean.
## Review-driven follow-up commits
Two review agents (architecture/duplication/coverage, and
behavior/regressions) ran against the first commit. Applied, each as its
own commit:
- Finished the crypto dedup via the intermediate base (the reviewers
flagged the still-duplicated crypto hooks).
- Dropped the over-engineered per-provider
`validationFailureLogMessage()` hook in favor of a structured base log.
- Added the missing test for the Interactive Brokers empty-statement
guard (the only conditional hook, previously unexercised): asserts 422 +
NAV-section message, no connection row, and no warning logged.
## Deliberately out of scope
- **Wise** is left as-is: it builds pending accounts mid-flow (needs the
client), creates the connection in one step, and has its own
empty-portfolio guard — it does not fit this template.
- **Pre-existing** (not introduced here, noted by review):
`Bank::firstOrCreate` returns an existing bank without refreshing its
logo, so `aspsp_logo` can copy a stale/null value. Worth a separate fix.
Adds the `laravel/ai` (v0) package and the `ai-sdk-development` skill to
the Laravel Boost guidelines section of CLAUDE.md, reflecting the
current installed ecosystem.
## Problem
The Discord **Plan changed** notification only showed the *new* plan
value in the `Changed` field, so it was impossible to tell what actually
changed:
```
Changed
Plan: €3.99 / month
```
Was it a price change? An interval change? From which plan? No way to
know.
## Fix
Compute the previous plan from Stripe's `previous_attributes` and render
it as `old → new`, matching the existing status-change behaviour:
```
Changed
Plan: €9.99 / month → €3.99 / month
```
`planLabel()` was generalised to read both the modern
`items.data[].price` shape and the legacy top-level `plan` shape Stripe
still includes in the diff, so the same helper works on both the current
object and the `previous_attributes` diff. Falls back to the old `Plan:
X` output when no previous value is detectable.
## Test
Added a feature test asserting the `old → new` output for a
`customer.subscription.updated` plan change. All 11 tests in the file
pass.
## Summary
- Add a new landing-page testimonial from Francisco Montes, sourced from
user feedback received by email.
- Add the matching Spanish translation in `lang/es.json` (required by
the translation CI check).
The quote was edited for clarity while keeping the user's own words and
intent. It sits just before the co-owner note, which stays last.
## Testing
- `python3 -c "import json; json.load(open('lang/es.json'))"` (valid
JSON)
- Full test suite not run locally: this worktree has no `vendor/`
installed. CI will run `./vendor/bin/pest` including the translation-key
check.
## Why
Two issues surfaced from a real mis-categorization corrected via the
**Edit Transaction** modal (not the table quick-edit).
### 1. The modal never told the user the AI learned
Both the table quick-edit and the edit modal hit the same backend
(`PATCH /transactions/{id}` → `CategoryOverrideHandler` →
`AiRuleLearner`), so both learn a forward rule from a correction. But
the modal **discarded** the update response and never read
`learned_rule`:
- No *"Learned: similar transactions will be categorized automatically"*
confirmation.
- No **Undo**.
- It still showed the generic *"Automatize"* prompt — offering to create
a rule that already exists.
The modal now reads `learned_rule` and mirrors the table's toast + undo,
skipping the automatize prompt when a rule was learned. Same i18n
strings as the table (no new keys).
### 2. The learner could key a rule on a single weak token
A correction on `"Pago en MADRID SUC 04 MADRID ES"` produced a rule
keyed on `suc` alone. The tokenizer drops noise by **per-user** document
frequency, so `madrid` (20% of this user's descriptions) is correctly
dropped — but `suc` (0.14%) survives, even though it is a generic
banking abbreviation that matches broadly as a substring. The per-user
filter can't see that `suc` is generic in general.
`AiRuleLearner::descriptionClause()` now refuses a single distinctive
token shorter than 5 characters. It learns only when there are **two or
more** distinctive tokens, or a single token of **at least 5
characters**. Merchant-keyed rules are unaffected.
## Tests
- `does not learn a description rule from a single short token`
- `learns a description rule from a single sufficiently long token`
- `learns a description rule from two short tokens`
All `AiRuleLearnerTest` pass (13/13).
## Not in this PR
The one already-learned prod rule still carries the stale `suc`
clause/title (low impact, matches ~2 txns). Cleaning that data is a
separate manual step.
## What
Two artisan commands to retire the remaining legacy client-side
encrypted data in production (transactions with
`description_iv`/`notes_iv`, accounts with `name_iv`).
- **`encryption:notify-removal`** — queues a re-engagement/deletion
warning to every user who still has an encrypted transaction or account.
Reuses `SendUpdateEmailJob` (queue `emails`, dedup via `UserMailLog`,
50/day pacing). The email is framed as an **inactivity** cleanup (no
mention of encryption) and pitches recent additions (AI insights, bank
integrations, etc.).
- **`encryption:delete-accounts`** — soft-deletes and anonymizes
(`User::markAsDeleted()`) those users **only** if they have not signed
in within the grace window (`--days`, default 7, via `last_active_at`).
Anyone who came back is spared, and the demo account is always excluded.
Both support `--dry-run` and `--force`, and share the target query via
the `FindsUsersWithLegacyEncryption` concern so the warning and the
deletion always target the same set.
## Intended manual run
`notify-removal --dry-run` → run for real → wait 7 days →
`delete-accounts --dry-run` → run for real.
## Notes / scope
- Deletion is a **soft-delete + email anonymization** (accounts become
unusable); it does not hard-purge rows, and does not cancel Stripe
subscriptions or revoke bank connections (unlike `user:delete`). Legacy
inactive accounts are unlikely to have either; handle separately if
needed.
- Email copy is plain English (matches the existing `mail/updates/*`
convention), no new `__()` keys.
## Tests
`tests/Feature/Console/NotifyEncryptedDataRemovalCommandTest.php` and
`DeleteEncryptedDataAccountsCommandTest.php` — 6 passing (targeting,
queue, dry-run, grace window, demo exclusion).
## Problem
Since #632 the account show page loads its transaction ledger via a
**deferred** Inertia prop. On first load of a transactional account the
user saw **two different loading skeletons back-to-back**, then the
table — a visible shape jump:
1. During the deferred network round-trip, `<Deferred>` rendered an
8-plain-bar fallback.
2. Once data arrived, `<TransactionList>` mounted with `isLoading`
initialized to `true` and rendered its **own, differently-shaped**
internal table skeleton for ~1 frame.
3. Then the real table.
Sequence: `plain-bars skeleton → table-shaped skeleton (~1 frame) →
table`.
## Fix
- Extract the internal table skeleton into a shared, exported
`TransactionListSkeleton`.
- Use it for **both** the `<Deferred>` fallback (`Accounts/Show.tsx`)
**and** `TransactionList`'s own `isLoading` branch — one consistent
skeleton, zero duplication.
- Initialize `isLoading` from `providedTransactions === undefined`, so
the redundant loading frame is dropped when data is already present at
mount.
### Before → After
| Phase | Before | After |
|-------|--------|-------|
| Deferred fetch | 8 plain bars | table-shaped skeleton |
| TransactionList mount | table-shaped skeleton (~1 frame) | (none —
data already present) |
| Loaded | table | table |
Result: a single, consistent, table-shaped loading state — no shape
jump.
## Shared-component safety
`TransactionList` is also used by the **budgets** show page
(`budgets/show.tsx`), which always passes a resolved `transactions`
array behind its own loading gate — so the `isLoading` init change keeps
that path rendering the table immediately. The transactions **index**
page has its own table and does not use `TransactionList`. Accounts with
zero transactions still render an empty table (unchanged).
## Tests
- New `transaction-list.test.tsx` asserts `TransactionListSkeleton`
renders the table-shaped skeleton (bordered container + many
placeholders), i.e. not the old plain-bars shape.
- `Accounts/Show.test.tsx` mock updated to export
`TransactionListSkeleton`; existing tests stay green.
- `bun run format`, `bun run lint` clean. No new TypeScript errors
introduced (pre-existing count unchanged).
## Problem
`AccountController@show` (added in #631) loaded the **full transaction
ledger** for an account into the initial Inertia payload on every visit
— `transactions()->with(['category','labels'])->get()`. For accounts
with years of movements that's several MB, all on the critical render
path, blocking first paint. The `ponytail:` comment flagged the
deferred/cursor move as a follow-up; this PR does it.
## Fix
Wrap the `transactions` prop in `Inertia::defer(...)` so the page shell
(header, chart, actions) paints immediately and the ledger arrives in a
follow-up request behind a skeleton. Mirrors how `index()` already
defers `accountMetrics` and how `dashboard.tsx` consumes `<Deferred>`.
The ledger **stays the whole set on purpose** — search and filtering run
client-side over *decrypted* rows (privacy-first design), so the client
needs every row. Cursor/offset pagination isn't viable without breaking
search over encrypted fields, so deferred is the right lever: it moves
the cost off the critical path rather than reducing bytes.
Non-transactional account types
(`loan`/`investment`/`retirement`/`real_estate`, i.e.
`!hasTransactionLedger()`) return a plain `[]` and never render
`<Deferred>`, so they take no extra round-trip.
## Testing
- `AccountControllerTest`: the "includes transactions" case migrated
from `->has('transactions', 2)` to
`->missing('transactions')->loadDeferredProps(...)`, proving the prop is
deferred yet resolves.
- `Show.test.tsx`: added coverage that creating a transaction reloads
**only** the deferred prop (`router.reload({ only: ['transactions'] })`)
— this refresh became load-bearing on deferred-prop semantics once the
remount `key` was dropped in #631.
- `php artisan test` (AccountController + PageQueryCount): 45 passed.
Vitest Show: 3 passed. Pint + ESLint clean.
## Reviewer notes
- **Known minor (not fixed):** brief loading-skeleton shape change on
first load — the `<Deferred>` fallback (plain bars) differs from
`TransactionList`'s own internal table skeleton, so there's a ~1-frame
visual jump. Aligning them would mean either duplicating the table
skeleton or adding a prop to the shared component; judged not worth it
for a sub-frame cosmetic nit.
- **Out of scope (pre-existing, from #631, flagged during review):**
since #631 removed the remount `key`, an active filter/search now
persists across a create-reload (a new non-matching row can look "not
created"); and in-memory search/sort currently operates on
still-encrypted rows pending the plaintext migration. Neither is
introduced or worsened here.
## Summary
Follow-up to #623 — addresses security findings **not covered** by the
maintainer's draft PR #627. The original scope was larger; out-of-scope
changes were reverted (see `revert: drop out-of-scope changes from
security round 2`) and are **no longer part of this PR** (details under
_Reverted / not included_).
## Changes
### Rate limiting
- `routes/api.php`: authenticated API group throttled at `300,1` (per
user). An SPA with offline sync + multi-widget dashboards fans out many
requests per interaction, so a tighter cap throttled legitimate bursts.
- `routes/web.php`: unauthenticated open-banking callback throttled at
`30,1` (per IP), enough headroom for browser retries and the iOS PWA →
Safari hand-off that can fire the callback twice.
- `use-decrypt-transactions.ts`: the legacy decrypt migration loops
without backoff (~3 requests per 100 encrypted transactions), so a large
account could hit the API throttle and abort the whole migration
silently. It now honours Laravel's `Retry-After` header on 429 and
retries the same request instead of bailing.
### State token persistence on failure
- `AuthorizationController::callback`: clears `state_token` on the
connection when EnableBanking session creation fails, preventing a stale
token from being replayed.
### Trust proxies hardening
- `bootstrap/app.php`: replaced `trustProxies(at: '*')` with an
env-based `TRUSTED_PROXIES` config. When unset, no proxy is trusted
(Laravel default) instead of trusting every IP.
- ⚠️ **Deploy requirement:** production runs behind a reverse proxy /
TLS terminator (`Dockerfile.production` + nginx serve HTTP on :80). With
`TRUSTED_PROXIES` unset, Laravel stops trusting
`X-Forwarded-For`/`X-Forwarded-Proto` → client IPs collapse to the proxy
IP (contaminating logs and per-IP throttling) and HTTPS detection
breaks. `TRUSTED_PROXIES` **must** be set in the production environment
before merge, and should be added to `.env.production.example`.
### XSS-safe Blade-to-JS injection
- `app.blade.php`: replaced `{{ }}` Blade syntax inside `<script>` with
`@json()` to prevent JS injection via variable content.
### TOCTOU defense-in-depth
- `Api/TransactionController::bulkUpdate`: added `->where('user_id',
$userId)` to the per-row UPDATE as belt-and-suspenders behind the
existing `abort(403)` ownership pre-check.
## Reverted / not included
The following were in the original description but were reverted and are
**not** in the current diff:
- `block-demo` middleware on additional route groups (already present
upstream on `routes/settings.php`).
- `->limit(500)` / `has_more` / `since` validation on
`TransactionSyncController`.
## Notes
- `vite.config.ts` appears in the diff only because of an older
merge-base; it is identical to `main` and nets to no change (will vanish
on rebase).
- No tests yet for the `state_token` clearing on `createSession` failure
or for the throttles — worth adding before merge (the callback throttle
is the most exposed surface).
---------
Co-authored-by: Víctor Falcón <victoor89@gmail.com>
## Summary
Migrates the last two flows off the deprecated client-side
IndexedDB/Dexie "offline-first" model so they read from the backend
instead of the local database. The Dexie cache was already being
dismantled and is no longer the source of truth for the transaction
list; these two consumers were the last ones still tied to it, and the
local read could silently return incomplete data.
**Duplicate detection on import** and **the account detail transaction
list** now both come from the server.
### What changed
- **Import duplicate detection** → new `POST
/api/transactions/check-duplicates`. Matches on day + amount (integer
cents) + normalized description server-side, replacing
`sync()`-then-read-Dexie, which silently missed duplicates that fell
outside the local cache window.
- **Account detail transaction list** → served as an Inertia prop from
`AccountController@show` (eager-loading `category` + `labels`) instead
of fetching every transaction from `/api/sync/transactions` and
filtering client-side (which could show an incomplete list for accounts
with long history).
- **Import preview "existing transactions" panel** → reads from `GET
/api/transactions?account_id=…&per_page=10` (the index gains an
`account_id` filter + clamped `per_page`) instead of Dexie.
- `TransactionList` now renders only from the provided prop; the
`/api/sync` fetch path, the Dexie-backed search, and the `refreshKey`
refetch are removed (search is in-memory; create triggers a scoped
`router.reload({ only: ['transactions'] })`).
### Commits
1. `feat` — the backend migration (endpoints, controller prop, frontend
rewiring, tests).
2. `refactor` — centralize the "has transaction ledger" rule in
`AccountType::hasTransactionLedger()` (a single named concept the
frontend mirrors, documented as distinct from `isNonTransactional()`),
and add an `id` tiebreak for deterministic pagination.
3. `fix` — drop rows reassigned to another account from the account list
(they lingered now that the list no longer refetches).
4. `fix` — match Unicode whitespace (e.g. non-breaking spaces) in
server-side duplicate detection, matching the old JS behavior; hardens
endpoint tests.
## Review notes
Two review agents (architecture/quality and bugs/correctness) reviewed
the change. Applied: the `AccountType` single-source-of-truth, stable
pagination sort, the account-reassignment stale-row fix, the
Unicode-whitespace parity fix, and extra edge-case tests (validation,
date-range boundary, cross-user scoping).
**Deferred (follow-up):** `AccountController@show` still loads the
account's full transaction history into the Inertia payload. It's
acceptable now (single account, strictly better than the old
load-all-accounts approach) and marked with a `ponytail:` comment, but
accounts with very long history should move to a deferred +
cursor-paginated prop (the new `?account_id=` endpoint can back it).
Also, the deprecated Dexie/sync-manager stack (`sync()`, `getAll`,
`getByAccountId`, `/api/sync/transactions`) now has fewer callers and
can be retired in a dedicated cleanup.
## Test plan
- `tests/Feature/Api/TransactionDuplicateCheckTest.php` — matching,
normalization (incl. NBSP), date-range boundary, account scoping, IDOR,
validation.
- `tests/Feature/Api/TransactionIndexTest.php` — `account_id` filter +
newest-first order, `per_page` cap and lower clamp, cross-user scoping.
- `tests/Feature/AccountControllerTest.php` — the account-show
`transactions` prop (own vs other accounts, non-transactional accounts).
- Existing `DecryptTransactionsTest` (the other `/api/transactions`
consumer) unaffected.
- Frontend vitest suites for the touched areas pass.
Draft while the deferred payload/cleanup items are discussed.
## What
Migrate the Vite source map upload from the decommissioned Bugsink
instance to Sentry. Bugsink is no longer used error tracking is fully on
Sentry.
- Point `sentryVitePlugin` at the Sentry EU region
(`https://de.sentry.io/`), org `whisper-money`, project `php-laravel`.
- Read the auth token from `process.env.SENTRY_AUTH_TOKEN` instead of
hardcoding it.
- Remove the hardcoded Bugsink URL, org, project, and auth token.
Source maps go to `php-laravel` (not `frontend`) because frontend JS
errors are captured there app.tsx initializes Sentry with
`SENTRY_LARAVEL_DSN`, and the live JS issues (e.g. `PHP-LARAVEL-2Y`,
platform `javascript`, `assets/app-*.js` frames) confirm it.
## Source map leak fix
A single flag (`SENTRY_AUTH_TOKEN` present) now gates **both** map
generation and upload, so the "maps generated but never deleted" leak
into `public/build/assets/` can no longer happen:
- **With token** (production CI): `sourcemap: 'hidden'` → maps are
emitted, uploaded, then deleted. `'hidden'` omits the `sourceMappingURL`
comment, so browsers never fetch them even in the window before
deletion.
- **Without token** (local/dev/CI): `sourcemap: false` → no maps
emitted. Nothing to leak, nothing to upload.
## Follow-ups (outside this file)
- [ ] **Rotate the old Bugsink token** (`b0f46d1b…a2e8a`) it is removed
here but remains in git history.
- [ ] **Provide `SENTRY_AUTH_TOKEN` to the production build** so uploads
actually run. `Dockerfile.production` runs `bun run build:ssr` without
it today; inject it via a BuildKit `--mount=type=secret` (not an `ARG`,
which persists in image history). Until then uploads are skipped safely,
but frontend stacks stay minified.
## Summary
Starts from a security finding on the background-job status endpoints
and folds in the most important issues surfaced by a follow-up review of
the same feature area (categorization backfill, bulk rule re-evaluation,
automation-rule apply).
Each change is its own commit.
## Changes
### 1. `fix(security)` — scope job-status cache keys to the owning user
The categorization, bulk re-evaluation, and apply status endpoints
looked jobs up by a bare job UUID with no user scoping. Any
authenticated user who obtained another user's job id could poll its
progress payload. Cache keys now include the owning user's id, so a
status request keyed by the polling user's id resolves only that user's
own jobs — a mismatched owner falls through to the existing 404. No
ownership store or extra lookup. Cross-user isolation tests added for
all three endpoints.
### 2. `fix(automation-rules)` — re-check `only_uncategorized` at apply
time
The apply flow cached a match snapshot for up to 15 min (keyed only by
`rule.updated_at`) and applied the rule's category to every id in it
without re-checking eligibility. A transaction categorized *after* the
snapshot (by the user, a sibling rule, or a concurrent AI backfill) was
silently re-categorized and stamped `category_source=Rule`. Now
re-filtered through `shouldSkipForOnlyUncategorized()` at apply time;
skipped rows are no longer counted as changed.
### 3. `perf(automation-rules)` — memoize rule set per user in
`applyRules()`
Bulk re-evaluation calls `applyRules()` once per transaction, and each
call re-queried the user's whole rule set + labels — an N+1 scaling with
transaction count (the sibling apply job already loaded rules once).
Memoized per user for the service instance's lifetime (resolved fresh
per job, so rules created mid-run are intentionally not seen).
Query-count test asserts one `automation_rules` query regardless of
transaction count.
### 4. `fix(transactions)` — stop polling when the job never starts
The re-evaluate and apply pollers rescheduled on any non-terminal
status, including `pending`. If the queue worker is down the job never
runs, `failed()` never fires, and the client polled for the full
hour-long TTL with a stuck spinner. Now gives up after 30 consecutive
`pending` ticks (~30s), mirroring the guard the AI-categorization poller
already has. Long `processing` runs are unaffected.
### 5. `test(automation-rules)` — cover apply job execution and failure
branches
`ApplySingleAutomationRuleJob` was only asserted to be *pushed*; its
`handle()` body and `failed()` branch had no coverage, and
`ReEvaluateTransactionRulesJob::failed()` was untested too. Added direct
`handle()`/`failed()` tests pinning the progress-cache payloads.
## Deferred follow-ups (surfaced by review, not in this PR)
- **Consolidate the three frontend pollers onto `usePollJobStatus`** —
they hand-roll `setTimeout` loops and don't tear down on unmount (can
`setState`/`onClose` after the dialog closes). The hook already exists
and is unmount-safe; routing all three through it would also make the
`pending` cap unit-testable. The cap in change #4 currently mirrors the
already-shipped AI poller and is not separately unit-tested.
- **`noteAlreadyPresent()` uses a substring match**
(`AutomationRuleService`) — a rule note that is a substring of an
existing note is silently not appended. Should compare note lines
exactly.
- **No dedup/lock on concurrent apply/re-evaluate jobs** — note appends
are a non-atomic read-then-write, so two concurrent jobs can duplicate a
note. Category/label writes are already idempotent.
- **Transient 404 → false "failed"** — a mid-run cache
eviction/TTL-expiry (or the deploy window of change #1, where an
in-flight job's pre-deploy key is orphaned) makes the poller surface a
false failure. Retry a few times on transient errors before giving up.
- **`matches()` pagination** advances `next_offset` by fetched rows, not
page-window size — deleted ids can stall infinite scroll before reaching
`total`.
- **"Apply to N" count / encrypted-skip counts** can be stale or
under-report vs what's actually processed.
- **Job-trait convention drift** between the three jobs (modern
`Queueable` vs legacy trait stack).
## Testing
- `vendor/bin/pint --test` — pass
- `bun run format` / `bun run lint` — pass (one pre-existing unrelated
warning in `chart.tsx`)
- Affected Pest suites (apply, re-evaluate, evaluation, rule,
categorization, apply-rule-suggestions) — 94 tests pass
## What
`demo:reset` now bails out early with an info message when
`config('app.demo.enabled')` is falsy, instead of rebuilding the demo
account regardless of the `DEMO_ENABLED` flag.
## Why
The `DEMO_ENABLED` config existed but the command ignored it, so a
scheduled reset would recreate demo data on environments where the demo
account is turned off.
## Problem
`SendAiConsentFollowUpEmailsCommandTest > it treats consent exactly
three days after signup as onboarding` fails intermittently on `main`
(more likely under parallel load):
```
The unexpected [App\Jobs\Drip\SendAiConsentFollowUpEmailJob] job was dispatched.
```
## Root cause
The `userWithConsent()` helper builds two timestamps from separate
`now()` calls:
```php
User::factory()->create(['created_at' => now()->subDays($signedUpDaysAgo)]); // now() at T1
$user->aiConsents()->create(['accepted_at' => now()->subDays($acceptedDaysAgo)]); // now() at T2 > T1
```
The command treats consent given more than `ONBOARDING_GRACE_DAYS` (3)
after signup as a post-onboarding opt-in:
```php
if ($consent->accepted_at->lessThanOrEqualTo($user->created_at->copy()->addDays(3))) {
continue; // skip
}
```
For the boundary case (`signedUpDaysAgo: 5, acceptedDaysAgo: 2`),
`created_at + 3 days = T1 - 2 days` and `accepted_at = T2 - 2 days`.
Because `T2 > T1` by microseconds, `accepted_at` lands just *after* the
grace boundary, the `<=` returns `false`, and the job is dispatched —
failing the assertion.
## Fix
Freeze time for the test file so all day-relative timestamps land on
exact boundaries. The command logic is correct and unchanged.
## Testing
```
php artisan test tests/Feature/SendAiConsentFollowUpEmailsCommandTest.php
# 9 passed
```
> **Draft on purpose — a partial fix + a design for the full one.**
These commits are safe, behavior-preserving reductions of the N+1, but
they do **not** fully resolve PHP-LARAVEL-40. The complete fix is a
batch-aware refactor of the AI rule-learning path, which is delicate
(getting it wrong silently mis-categorizes users' transactions). I've
written the design below for a human to implement and diff against the
current per-transaction behavior. Current user impact is low (~170 ms
request, 1 event), so there's no urgency to rush the risky part.
## What
Sentry **PHP-LARAVEL-40** — N+1 in `TransactionController@bulkUpdate`
(PATCH `/transactions/bulk`). A bulk category update calls
`CategoryOverrideHandler::record()` once per transaction, and each call
runs the full AI rule-learning pipeline
(`AiRuleLearner::forgetFromAiRules()` + `learnFromCorrection()`):
loading all the user's AI rules, plucking every description, running
matcher `count(*)` probes, and inserting a `category_corrections` row.
For a 112-transaction batch that's ~112× each. The offending span is the
per-transaction `category_corrections` insert.
## What this branch does (safe, partial)
Two behavior-preserving reductions, each validated by the existing
learning tests:
1. `perf(transactions): resolve the override handler once for bulk
updates` — `bulkUpdate()` re-resolved `CategoryOverrideHandler` from the
container every iteration. Resolve it once (also required for #2 to take
effect).
2. `perf(ai): memoize the description corpus per user in AiRuleLearner`
— `learnFromCorrection` reloaded and re-tokenized every one of the
user's descriptions on each transaction. The corpus is immutable while
only categories change, so memoize the document-frequency map + count
per user. Removes one `SELECT` (and its tokenization) per transaction.
3. `test(ai): assert batch corrections learn correctly through the
memoized corpus` — proves the second, memoized-corpus learning still
yields a correct, distinct clause (not just that the query count
dropped).
4. `test(ai): guard AiRuleLearner against a singleton binding` — the
cache has no invalidation and is only safe while the learner is resolved
fresh per request; a test now fails if it is ever bound singleton.
## What it does NOT do
It does **not** resolve the flagged N+1. Still per-transaction: the
`category_corrections` insert, `forgetFromAiRules`' reload of all AI
rules, the matcher `total()`/`countMatchingAll()` overbroad probes,
`releaseClauseFromOtherCorrectionRules`, and `existingCorrectionRule`. A
learnable transaction still costs ~6–10 queries. Treat PHP-LARAVEL-40 as
**reduced, not closed** (hence no `Fixes` keyword).
## Reviewed by two independent agents (architecture +
product/correctness)
Both verified the two perf commits are **behavior-preserving and safe**:
the memo cannot go stale (nothing in the correction path mutates
descriptions; the bulk `update()` only writes
`category_id`/`category_source`/`ai_confidence`/`categorized_by_rule_id`,
and runs after the loop), no cross-user leak (keyed by user_id; learner
is transient, one user per request, no Octane), and the handler is
stateless. The existing 34 learning tests exercise the corpus→rule
computation on fresh loads.
## Proposed full fix (for a human to implement)
Exploit that a bulk update sends **all transactions to the same category
for one user.** Add `CategoryOverrideHandler::recordBulk(Collection,
?string)` backed by `AiRuleLearner::learnFromCorrections(array,
string)`; load user-level data once, mutate in memory, write once:
1. **Batch-resolve** `categorized_by_rule_id` via one `whereIn` instead
of per-txn `find()`; apply the **identical** per-txn learnable test
against the map.
2. **Batch-insert** corrections: snapshot each AI-driven txn's *old*
`from_category_id`/`source`/`confidence` during the loop, collect rows,
one insert. (Note: raw `insert()` skips model events/UUID/timestamps —
verify `CategoryCorrection` has no `creating` hook, else chunked
`create` in one transaction.)
3. **Forget once**: union all learnable txns' merchant tokens, load AI
rules once, apply the **whole union** to each rule *before* the
delete-vs-save decision, save/delete each once.
4. **Learn once**: compute each clause (merchant or memoized-corpus
tokens), dedupe within the batch and against the target rule, run
`releaseClauseFromOtherCorrectionRules` for the union once, load/create
the single target correction rule once, append all, save once.
5. Wrap 2–4 in one `DB::transaction`, and route the single-txn path
through the same method (one-element collection) so the two can't drift.
**Correctness invariants to preserve (call these out in review):**
- Apply-all-then-decide for both `forget` and `releaseClause` deletes —
an incremental forget/save/forget would delete a rule a later txn's
clause should have kept.
- Corrections must read each txn's **old** category/source/confidence —
run before the bulk `update()`, as today.
- Clause dedup must match `appendClause`'s loose `==` array comparison.
- Validate with a golden-set test diffing learned/forgotten rules
before/after against the current per-txn path over a mixed batch
(merchant txns, description txns, encrypted/no-merchant skips,
re-corrections that move a key between categories).
## Testing
- `tests/Feature/Ai/` + `BulkUpdateTransactionsTest` + IDOR/decrypt:
153/153 green. `pint` clean.
Refs PHP-LARAVEL-40
## What
Addresses Sentry **PHP-LARAVEL-3X** — a slow DB query in
`App\Jobs\SendDailyBankTransactionsSyncedEmailJob::handle()`.
The daily "transactions synced" email filters transactions by:
```php
Transaction::query()
->where('user_id', $this->user->id)
->where('source', TransactionSource::EnableBanking)
->when($lastSentMailLog?->sent_at, fn ($q, $at) => $q->where('created_at', '>', $at))
->whereHas('account.bankingConnection', ...) // EXISTS on PKs
->get();
```
The only `user_id`-leading index is `idx_transactions_budget_lookup
(user_id, transaction_date, category_id)` — its second column is
`transaction_date`, **not** `created_at`, so it can't serve
`source`/`created_at`.
## How
Add a composite index matching the predicate — two equalities then the
range:
```
idx_transactions_user_source_created (user_id, source, created_at)
```
The `whereHas` compiles to correlated `EXISTS` subqueries that join on
primary keys (`accounts.id`, `banking_connections.id`), so no extra
index on those tables is needed — the `transactions` index alone gives
the optimizer the selective driving path it currently lacks.
## Production verification (via read-only prod queries)
Confirmed the diagnosis and de-risked the deploy against the live
database:
- **The query is genuinely slow.** `EXPLAIN ANALYZE` for the heaviest
user (~6.1k EnableBanking transactions) runs in **~6 s**. The optimizer,
lacking a selective path, drives from a **full table scan of all ~2,500
accounts** and examines ~108k transaction rows (334 account loops × ~323
rows), filtering `user_id`/`source` only afterwards.
- **The index fixes it.** `(user_id, source, created_at)` lets the
optimizer drive from transactions — a seek to `(user_id,
'enablebanking')` (~6.1k rows) plus primary-key semi-joins — far cheaper
than the current ~108k-row plan, so it will be chosen.
- **The deploy is low-risk.** `transactions` holds **~297k rows** (not
millions). Adding a secondary index on InnoDB/MySQL 8 is online
(`ALGORITHM=INPLACE, LOCK=NONE`, no table rebuild), so at this size the
build is seconds and does not block the sync write path.
## Commits
1. `perf(db): index transactions for the daily synced-email query` — the
migration + a test asserting the index columns/order.
2. `test(db): assert the daily-email index name, not just its columns` —
lock the explicit index name (review feedback).
## Reviewed by two independent agents (architecture +
product/operational risk)
Both rated the change correct and shippable: column order right
(equalities before the range), migration reversible, index non-redundant
(an existing index can't be widened without breaking budget queries),
and no behavior/result change (a secondary btree index never alters
result sets; the job has no `ORDER BY`).
## Impact / risk
- **User impact:** indirect — a background email job (Sentry reports 0
interactive users), but the ~6 s query wastes queue-worker time and IO
on every run.
- **Complexity:** low — one additive, reversible index migration; no
application logic changed.
- **Write path:** one more index maintained per transaction insert (10th
on the table). Marginal, consistent with the existing UUID-leading index
cost profile.
Fixes PHP-LARAVEL-3X
## What
Fixes the N+1 flagged by Sentry **PHP-LARAVEL-3Y** in
`App\Jobs\SyncBankingConnectionJob`.
`TransactionSyncService::importTransaction()` ran one `exists()` probe
**per incoming bank transaction** to detect duplicates:
```sql
select exists(
select * from transactions
where account_id = ?
and (dedup_fingerprint = ? or external_transaction_id = ?)
) as exists
```
A sync importing N transactions issued N dedup `SELECT`s. As transaction
volume per sync grows, so does the query count.
## How
Preload the account's existing dedup keys **once** at the start of
`sync()` and match against in-memory sets:
- `loadExistingDedupKeys()` streams (`cursor()`) the two dedup columns
for the account — including soft-deleted rows (`withTrashed()`) — into
two keyed sets. One query instead of N.
- `importTransaction()` checks membership in memory (`isset()`), an
exact mirror of the old `fingerprint OR external_id` predicate.
- Keys from successful inserts are folded back into the sets, so
duplicates **within the same sync** (e.g. the same transaction repeated
across pages) are still caught in memory.
- The `(account_id, dedup_fingerprint)` unique index +
`UniqueConstraintViolationException` catch still backstop concurrent
syncs (`SyncBankingConnectionJob` is already `ShouldBeUnique` per
connection).
## Commits
1. `perf(banking): preload dedup keys to kill per-transaction N+1` — the
core fix + a query-count regression test.
2. `perf(banking): stream dedup key preload with a cursor` — avoids
buffering every historical row into a Collection on top of the sets
(memory guard for very large accounts).
3. `fix(banking): match external ids case-insensitively in dedup` — the
old query compared `external_transaction_id` under the production
`utf8mb4_unicode_ci` collation; the in-memory `isset()` is byte-exact.
With no unique index on `external_transaction_id`, a legacy id stored as
`ABC` vs an incoming `abc` would have silently double-imported.
Normalized with `mb_strtolower()` on both sides + a test.
4. `test(banking): cover intra-run cross-page dedup` — new coverage for
the same transaction appearing on two pages of one sync.
## Testing
- 303/303 in `tests/Feature/OpenBanking/` green; full backend suite
green except one **pre-existing, unrelated** failure on `main`
(`DashboardTest` returns 409 locally — an Inertia asset-version artifact
that resolves once `bun run build` runs in CI).
- New tests: dedup runs as a single preload SELECT regardless of batch
size; case-insensitive external-id dedup; cross-page intra-run dedup.
- `pint` clean; `larastan` clean on the changed file.
## Reviewed by two independent agents (architecture +
product/correctness)
Both rated the change behavior-preserving and shippable. Follow-ups they
surfaced, intentionally **not** bundled here to keep this fix focused
and low-risk:
- **`WiseTransactionSyncService` has the identical latent N+1**
(`:149-158`). It is not currently firing in Sentry (lower volume). Worth
a separate PR — possibly extracting a shared dedup-set helper once both
call sites need it.
- **Residual collation divergence**: accent/width folding from
`utf8mb4_unicode_ci` is not replicated in PHP. Irrelevant for real bank
reference ids (ASCII), accepted.
- The test comment referencing a "cleanup command" that repoints orphan
fingerprinted rows describes a command that does not exist in the repo —
pre-existing, worth correcting separately.
Fixes PHP-LARAVEL-3Y
## What & why
Fixes the Sentry N+1 **PHP-LARAVEL-3V** — `N+1 insert into jobs` on
`POST /transactions`.
### Root cause
Laravel's **event discovery is on by default** (the base
`EventServiceProvider` sets `$shouldDiscoverEvents = true`, and this app
has no subclass to turn it off), so every listener in `app/Listeners` is
already wired to its event by its `handle()` type-hint.
`AppServiceProvider::boot()` was **also** registering six of them with
`Event::listen(...)`, so each ended up registered **twice** — every
queued listener published its job twice per event.
Concretely, creating a transaction dispatched
`AssignTransactionToBudget` and `CategorizeTransactionWithAi` twice each
→ repeated `insert into jobs` in one request. (The same bug made
`PostStripeEventToDiscord` post to Discord twice per webhook,
`ApplyAutomationRules` run twice, etc.)
## Changes
1. **`AppServiceProvider`** — drop the redundant explicit
`Event::listen` calls and rely on discovery, which already wires every
listener exactly once (`ScheduleDripEmailsListener` and
`SyncUserToResendListener` were discovery-only already). Verified with
`php artisan event:list`: each event now has a single listener, and the
synchronous `ApplyAutomationRules` still runs first on
`TransactionCreated`.
2. **`CategorizeTransactionWithAi`** — add `shouldQueue()` so the job
isn't even published when it would immediately no-op (already
categorized by a rule, encrypted, or user not AI-eligible — the default,
since AI is opt-in). The same guard still runs in `handle()` because
state can change while the job waits.
Together these remove the second `insert into jobs` on transaction
creation for the common (non-AI) path.
## Tests
- New `tests/Feature/Ai/CategorizeTransactionWithAiListenerTest.php`:
the AI job is queued **once** for an eligible uncategorized transaction,
and **not at all** when the user has no AI consent or the transaction is
already categorized.
- Full suite: **1828 passing**. The one failure (`DashboardTest` → 409)
is **pre-existing on `main`** (Inertia asset-version mismatch from a
local build manifest) and reproduces with these changes reverted —
unrelated to this PR.
Fixes PHP-LARAVEL-3V
## What
Removes the `AiConsentSettings` Laravel Pennant feature flag. The AI
consent management UI — the toggle in **Billing settings** and the
banner in **Transactions** — is now available to all users
automatically, no longer gated behind the flag.
The underlying AI consent functionality (model, controller, routes,
`User` consent methods) is unchanged; only the gate that hid its UI was
removed.
## Changes
- Delete `app/Features/AiConsentSettings.php`.
- `HandleInertiaRequests`: drop the `aiConsentSettings` shared prop and
its resolution.
- Frontend: render `AiConsentSection` and the transactions consent
banner unconditionally; drop the now-unused `aiConsentSettings` type and
`features` destructuring.
- Tests: remove the two flag-specific assertions in
`AiConsentSettingsTest` (consent-state coverage kept), update
`InertiaSharedDataTest`, and switch `FeatureEnableCommandTest` to
`CalculateBalancesOnImport` as its example feature.
## Testing
- `php artisan test` on the affected suites — 13 passed.
- `vendor/bin/pint`, `bun run format`, `bun run lint` — clean.
## Why
During onboarding we prompt for AI suggestions only because it's a
**paid feature** — enabling it forces the user to pick a plan at the end
of onboarding. A user who has **already connected a bank** is committed
to a paid plan regardless, so the prompt gives them nothing new. For
them we should just turn AI on.
## What
- **Connected bank → activate AI directly.** The consent prompt is
skipped and consent is recorded automatically (reuses the existing
`/ai/consent` endpoint, so the categorization backfill still runs).
`setBusy` batches with the state update so the consent screen never
flashes.
- **No bank → unchanged opt-in.** Free users still explicitly accept.
The notice is reworded to make the paid framing clear: *"AI suggestions
are a paid feature. Enable them and you'll choose a plan at the end of
the onboarding."* (translated in `es.json`).
- No backend change needed:
`RuleSuggestionController::requiresUpgrade()` already treats
bank-connected users as not needing an upgrade, and
`SubscriptionController` already gates the free plan on `!hasBank &&
!hasConsent`.
## Tests
Browser tests in `OnboardingFlowTest`:
- Connected bank → consent prompt skipped and `hasActiveAiConsent()`
becomes true.
- No bank → prompt (with the paid notice) shown; consent recorded only
after accepting.
- `/subscribe` forces a plan (no "Continue for free") when a bank is
connected or AI consent is active.
Vitest specs for `StepAiSuggestions` updated (new prop + reworded
notice) plus a new case asserting bank-connected users auto-activate.
All green: 5/5 browser tests (24 assertions), 6/6 vitest.
## What
The transactions AI consent banner now lets users decline, tells them
the outcome, and stops appearing once they've decided.
- **Dismiss option**: an X button before "Enable AI" permanently hides
the banner without granting consent.
- **Show only until the first decision**: the choice is persisted on
`users.ai_consent_prompt_dismissed_at` (set on both accept and dismiss).
The banner shows only while the user hasn't responded and never
reappears afterwards — even if they later revoke consent from settings.
- **Clear feedback**: accepting or dismissing shows a toast stating
whether AI was enabled and that the choice can be changed anytime in
**Settings > Manage Plan**.
- **Full-width layout on desktop**: the banner content spans the full
available width (button pushed to the right, text on a single line). The
narrow stacked layout is kept for mobile only.
## How
- New `POST ai/consent/dismiss` endpoint
(`AiConsentController::dismiss`).
- `User::dismissAiConsentPrompt()` (idempotent) +
`hasDismissedAiConsentPrompt()`; `store` now also marks the prompt
dismissed on accept.
- Migration adds the nullable `ai_consent_prompt_dismissed_at`
timestamp.
- `TransactionController` passes `aiConsentPromptDismissed` to the page.
## Tests
- `AiConsentTest`: idempotent dismissal, dismissal without consent, and
accept marking the prompt dismissed.
- Full consent + backfill suite and the `create a transaction` browser
test pass; pint / lint / format clean.
## Why
Feedback from onboarding users: in the categorizer step people often
**don't realize they need to categorize at least 5 transactions** before
*Continue* unlocks, and can't tell why the button is disabled. Worse,
the old counter switched to **"N remaining"** after 5 (showing *every*
uncategorized transaction, e.g. "195 remaining"), which made several
users think they had to categorize **all** of them.
## What changed
### Categorizer clarity (`step-categorize-transactions.tsx`)
- **New full-width progress banner** above the transaction card,
replacing the cramped corner counter that was getting truncated on
mobile:
- explicit instruction: *"To continue, you need to categorize at least 5
transactions."* (reuses the existing intro string)
- a **5-segment progress bar** that fills as you go
- the live **X/5** count
- a reassurance line: *"You do not need to categorize all of them."*
- When the minimum is reached (or the list is exhausted), the banner
turns **green** — *"Done! You can continue now, or keep categorizing if
you want."* — and the **Continue** button gets a ring so it's obvious
the step is unlocked.
- **Removed the misleading "N remaining"** display.
### Onboarding layout & toasts
- **Mobile top spacing** (`onboarding-layout.tsx`): reduced the large
top padding on the step content on mobile so it sits closer to the
progress dots (unchanged on desktop).
- **Toasts during onboarding** (`app.tsx`): onboarding has no bottom
navigation bar, so toasts now render **bottom-center, flush to the
bottom** instead of being lifted 110px to clear the (absent) mobile tab
bar. Behavior elsewhere in the app is unchanged.
Two new strings added to `lang/es.json`.
## Notes
- The `canContinue` / `minimumRequired` gating logic is unchanged.
- Verified: `lint` clean, `es.json` valid, `LocalizationTest` passes,
full CI green.
- Recorded mobile + desktop walkthroughs locally.
## What
Adjust the highest-index chart color variables so adjacent segments stay
distinguishable:
- **Light mode:** `--chart-9`/`--chart-10` use darker zinc shades
(`zinc-700`/`zinc-600`) instead of the near-white `zinc-100`/`zinc-50`,
which were invisible against the light background.
- **Dark mode:** `--chart-7`/`--chart-8` use lighter zinc shades
(`zinc-200`/`zinc-300`) instead of the near-black `zinc-800`/`zinc-900`.
## Why
Charts with many categories were rendering low-index and high-index
segments at nearly the same value as the background, making them
unreadable.
## Summary
Landing page copy and social proof around AI, made consistent with our
privacy stance.
- **Clarify AI as a privacy-respecting feature.** Name AI explicitly
where transactions are auto-categorized, replace the contradictory "no
AI snooping" line with an "our own AI" framing, and add an FAQ stating
the AI serves you and never trains on, sells, or shares your data.
- **Add six testimonials**, four of which highlight AI and automation
(AI categorization that learns from corrections, automatic bank sync,
the privacy framing of our own AI, and CSV auto-mapping). The other two
cover the all-in-one-place and clean-UI experience.
## Notes
- All new strings have matching `lang/es.json` translations;
`LocalizationTest` passes.
- New testimonials use placeholder gravatar hashes that 404 and fall
back to the generated `Facehash` avatar (no real photo needed).
## Test plan
- [x] `php artisan test tests/Feature/LocalizationTest.php`
- [x] Prettier formatting
## Why
On the **analysis** drawer, income/expense were derived from each
transaction's amount sign alone, ignoring the category type.
Transactions in `transfer` / `savings` / `investment` categories
therefore leaked into income/expense totals and every breakdown — e.g.
moving money between your own accounts inflated both sides. The
**cashflow** screen already keys off the category type; analysis now
does the same.
## What changed
Three commits:
1. **fix(analysis): exclude transfer, savings and investment categories
from income and expense**
Only `expense` categories (or uncategorized outflows) count as expenses,
and only `income` categories (or uncategorized inflows) count as income.
`transfer` / `savings` / `investment` are internal movements and sit on
neither side — identical to how cashflow computes income/expense/net.
2. **refactor(analytics): de-duplicate currency conversion and
category-type helpers**
`convertTransactionAmount()` / `preloadExchangeRates()` were
byte-identical in three controllers, and `categoryType()` in two.
Extracted the currency helpers into a shared
`Api\Concerns\ConvertsTransactionCurrency` trait (following the existing
`OpenBanking/Concerns` pattern) and moved `categoryType()` onto the
`Transaction` model. No behavior change.
3. **fix(analysis): net refunds within a category so totals reconcile
with cashflow**
Classification keyed off each transaction's own sign dropped contra-sign
rows entirely, so a refund booked to an expense category disappeared
instead of netting the spend down — and analysis disagreed with cashflow
on the same data (a `-10000` charge + `3000` refund read as `10000`
spent, not `7000`). A transaction's side is now decided by its category
type and signed amounts are summed before clamping, mirroring cashflow's
reconciliation across summary, category, payee, account, tag and
over-time. The largest-expenses list still shows only genuine outflows.
## Tests
Added analysis coverage for: transfer/savings/investment exclusion
(summary, breakdowns, largest, over-time), refund netting (asserts
parity with cashflow's `7000`), income-category reversals,
foreign-currency conversion, and uncategorized inflows. Full non-browser
suite green (1823 passed); `pint --test`, `bun run format`, `bun run
lint` all clean.
## Follow-up for product (not in this PR)
On **cashflow**, savings/investment outflows are excluded from expense
but re-surfaced in a dedicated "Saved & Invested" card. The **analysis**
drawer has no equivalent surface, so money categorized as
savings/investment is now correctly excluded from spending but isn't
shown anywhere. If we want analysis to fully account for it, we should
add a small "set aside" summary there. Flagged for a product decision
rather than bundled into this bugfix.
## Problem
Users keep correcting the same transactions over and over. The AI
mislabels a merchant (e.g. supermarket → fuel), the user fixes it, and
the next near-identical transaction from that merchant gets mislabeled
the same way again. Today a correction is logged and the offending ai
rule is self-healed, but the system only *forgets* its mistake — it
never *remembers* the user's fix.
## Approach
A correction now becomes a deterministic, forward-looking
`AutomationRule` (new `RuleOrigin::Correction`). The next matching
transaction is categorized by that rule **before the model ever runs**
(`ApplyAutomationRules` is synchronous and runs ahead of AI
categorization), ending the loop. Zero model cost, instant, reuses the
existing rule engine.
**Matching key** (in order):
1. **Merchant** (`creditor_name`/`debtor_name`, exact `==`) when present
— stable even as the description varies.
2. Otherwise the **description's distinctive tokens** (`in` /
AND-of-`in`), extracted by the shared `DescriptionTokenizer` (noise
tokens dropped by document frequency, language-agnostic), **guarded**
against over-broad rules that could silently mis-file en masse. If
guarded out → nothing is learned, silently.
## Deliberate decisions (from a design walkthrough)
- **Forward-only**: never retroactively re-categorizes existing
transactions.
- **Learn only from system categorizations** (AI label, ai rule, or a
prior correction rule) — never from one-off manual filing, bank
categories, or the user's own hand-authored rules.
- **A key lives in exactly one correction rule**, so changing your mind
moves it to the new category. Correcting a transaction a prior
correction rule categorized is also learnable, so correction rules stay
fixable in-flow.
- Correcting to *uncategorized* learns nothing but still self-heals the
ai rule.
- **Safety net**: correction rules are visible/editable in
`settings/automation-rules` (marked with the AI sparkle, tooltip
"Learned from your correction"); the transactions table shows a toast
with an instant **Undo**.
## Review pass (two independent agents + live QA)
- **HIGH fix** (`67fc4293`): an ai rule could out-rank a freshly learned
correction and re-apply the wrong category (when the corrected
transaction was a *direct* model label with no rule id). Now every ai
rule holding the merchant is swept on correction. Regression test added.
- **Refactor** (`ca743fde`): collapsed duplicated clause-append logic;
removed a speculative unused enum helper.
- **Coverage** (`12ceb0f7`): debtor_name path, single-token description
clause, encrypted-description fail-safe.
- **Toast conflict fix** (`382c8169`, found in live QA): correcting an
AI transaction fired both the new "Learned …" toast and the pre-existing
"Transaction categorized → Automatize" prompt, which invited the user to
manually create the rule the correction had just created. Made them
mutually exclusive. Adds a browser test for the inline-correction flow.
- **Settings icon** (`f3f882b6`): correction rules now show the AI
sparkle in settings, like ai rules.
## Verified end-to-end (against a running instance)
Drove the real UI with a browser: correcting an AI-mislabeled
transaction creates the correction rule, shows the "Learned · Undo"
toast (no competing Automatize prompt), and Undo deletes the rule while
keeping the correction. Confirmed for **merchant** keys and the
**description-only** path — including that a later "practically
identical" description (different surrounding text, no merchant) is
caught by the rule, while a near-miss sharing only one distinctive token
is correctly **not** caught.
## Open question for reviewers
**`debtor_name` (P2P) as a rule key.** For incoming transfers the
merchant key falls back to the sender's name, so correcting one can
create a rule keyed on a person's name (useful for recurring transfers
from a roommate, but a possible privacy surprise; the name appears in
the rule title in settings). This matches the existing tier-2 learner's
behaviour. Keep as-is, or restrict correction rules to `creditor_name`
only? Happy to change.
## Testing
- `tests/Feature/Ai/CategoryOverrideHandlerTest.php`: merchant +
description learning, next-transaction match, over-broad rejection,
change-of-mind move, correct-to-null self-heal, the HIGH regression,
debtor_name, single-token, encrypted fail-safe.
- `tests/Browser/CategoryCorrectionLearningTest.php`: inline correction
→ toast → learned rule → undo.
- `automation-rule-title.test.tsx`: the AI sparkle shows for `ai` and
`correction`, not `user`.
- Full AI suite green (106 tests); transaction/bulk-update suites green
(62). Pint + Prettier + ESLint clean.
No new dependency, no migration (the `origin` column is a free-text
string). The feature is implicitly gated by AI categorization — with no
AI categorization there is nothing to correct and nothing is learned.
## What
The "new since last visit" highlight (#609) stored its marker in
`localStorage` (`transactions-last-visit`), so it was **per-device**:
each device kept its own last-visit timestamp and re-flagged the same
rows. This moves the marker to a per-user server column so a single
"last visit" is shared across all your devices.
## How
- New nullable `transactions_last_visited_at` column on `users` (same
pattern as `last_active_at` / `paywall_seen_at`).
- `TransactionController@index` reads the stored marker, renders the
list with the **old** value (`lastVisitAt` prop), then advances it
forward to the newest `created_at` it served.
- Frontend drops the `localStorage` read/write and just freezes the
`lastVisitAt` prop at mount; `isNewSince` per-row logic is unchanged.
## Why newest-served `created_at` and not `now()`
Advancing to `now()` would mark a back-dated synced row (old
`transaction_date`, lands on a later page that wasn't served) as seen,
so it would never be highlighted — the exact "hiding" failure the
per-row design avoids. Advancing only to what was actually served keeps
the feature's "err on showing, never hiding" stance. The marker only
ever moves forward.
## Notes
- Same filter caveat as before: opening the list with a filter applied
advances the marker using the filtered payload. Carried over from the
original `localStorage` behavior; not a regression.
- Removed the now-dead `loadLastVisit` / `saveLastVisit` /
`newestCreatedAt` helpers and their tests.
## Tests
- `NewTransactionsMarkerTest` (feature): null marker on first visit +
stores newest served; later visit sees previous marker + advances
forward; marker never moves backward.
- `new-transactions.test.ts` (`isNewSince`), Pint, ESLint, Prettier all
pass.
## What
Between visits, highlight the transactions that arrived since the user
last opened the list.
- The newest `created_at` from the initial page load is stored in
`localStorage` (`transactions-last-visit`), frozen at mount.
- Every transaction whose `created_at` is newer than that marker gets a
subtle accent on the row (left border + faint tint).
## Why per-row instead of a single divider line
The first cut drew one "Last visit" divider with new rows above it. A
code review found this is structurally wrong: the list is sorted by
`transaction_date` (when the money moved), but "new" is defined by
`created_at` (when the row was inserted). The two don't correlate — a
bank sync that imports a batch dated across several days scatters the
new rows throughout the list, so a single boundary lands at the first
older row and **hides every new row below it**, the exact miss the
feature was meant to prevent.
Per-row marking is correct regardless of sort order: no new transaction
can be hidden.
## Notes / limitations
- The marker is written **once at mount** from the initial payload. Rows
that arrive mid-visit (load-more, a sync/refresh, AI categorization) do
not advance it, so they are never recorded as "seen" before the user has
seen them. The trade-off is they may re-appear as new next visit (errs
on showing, never hiding).
- Purely client-side (localStorage), so the marker is per-device.
## Tests
- Unit tests for `isNewSince` and `newestCreatedAt`.
- `LocalizationTest`, ESLint, Prettier all pass.
## What
Remove the fixed `max-w-[90px]` cap (and `pr-1`) on the transaction date
cell so the date column sizes to its content instead of wrapping
awkwardly.
## Why
The 90px cap forced the formatted date to wrap onto two lines in the
transactions table.
## What
Follow-up to the `--no-discord` flag added to `stats:experiment-funnel`.
Extends the same flag to the **other stats report commands that post to
Discord**, so any of them can be run on demand (e.g. inside the prod
container) and just printed to the console without posting:
```bash
php artisan stats:subscription-funnel --no-discord
php artisan stats:ai-cohort-report --no-discord
php artisan stats:stuck-cohort-report --no-discord
php artisan stats:daily-report --no-discord
```
## How
- The two **funnel** commands (`subscription-funnel`) already printed
their table to the console, so `--no-discord` just short-circuits the
`->send()`.
- The **cohort/daily** commands only ever built a Discord embed. A small
`App\Console\Commands\Concerns\RendersReportToConsole` trait renders an
embed (title / description / fields) as plain text, so `--no-discord`
prints a readable report to the console instead of posting.
- Default behaviour is unchanged: without the flag (and on the scheduled
weekly/daily runs) they post to Discord exactly as before.
## Tests
A `--no-discord` test added to each command (`Http::assertNothingSent()`
+ the command succeeds). Pint + Larastan + all four command suites
green.
> Note: the commands query the default DB connection, so for
**production** numbers run them inside the prod container (Coolify
terminal).
## What
Adds a `--no-discord` flag to `stats:experiment-funnel` so the
per-variant report can be run **on demand (e.g. in production) and only
printed to the console**, without posting to the Discord channel.
```bash
php artisan stats:experiment-funnel --no-discord
```
## Why
Handy for ad-hoc checks of the live experiment without spamming the
ops/Stripe Discord channel. The scheduled weekly run (and any run
without the flag) still posts to Discord exactly as before — default
behaviour is unchanged.
## Notes
- The console table already prints before the Discord step; the flag
just short-circuits the `->send()` and prints `Skipped Discord
(--no-discord).`
- Test added: with `--no-discord`, the command succeeds, prints the
table, and `Http::assertNothingSent()`.
- Pint + Larastan + the command's Pest suite green.
> Tip: the command queries the default DB connection, so to see
**production** numbers run it inside the prod container (e.g. via the
Coolify terminal).
## What
Follow-up to #600. Reframes the **pay_now** paywall message so the
emphasis is on *trying it risk-free*, not on being charged today.
**Before**
> You'll be charged €3.99 today
> Changed your mind? Get a full refund yourself from Settings within the
first 3 days — no questions asked.
**After**
> **Try it for 3 days**
> Don't like it? Get a full refund with one tap from Settings — no
questions asked.
> _You pay €3.99 today, refunded in full if you cancel within 3 days._
(muted footnote)
## Why
The upfront-charge headline reads as friction. Leading with "try it /
one-tap refund" frames pay_now as risk-free while the charge stays
**honestly disclosed** as a muted footnote — so it's never a surprise at
refund time (and avoids chargeback risk). Amount and day counts stay
dynamic per plan/variant.
## Notes
- Only the pay_now branch of `TrialTerms` changes; control/reduced
trials untouched.
- Spanish + French translations updated; the two orphaned old keys
removed.
- Lint, tsc (paywall), and `LocalizationTest` pass; no test asserted the
literal copy.
## What
Adds an **Edit** button (icon-only, top-right) to the dashboard accounts
grid, next to a new **Accounts** section title. It opens a dialog for
managing accounts:
```
Accounts [edit]
[ acc 1 ] [ acc 2 ]
[ ] [ ]
```
Each dialog row shows:
- the bank logo + account name
- an **eye toggle** (open/closed) to hide or show the account on the
dashboard grid
- a **drag handle** to reorder
## Behavior
- **Visibility**: hidden accounts disappear from the grid but stay in
the net worth chart/totals and remain in the dialog so they can be
re-enabled. Backed by a new `hidden_on_dashboard` column.
- **Reorder**: moved entirely into the dialog (over *all* accounts,
including hidden) instead of dragging the cards. This keeps a single
source of truth for `position` — dragging visible-only cards would have
left hidden accounts' positions inconsistent. The card-level drag handle
was removed.
- Both actions use optimistic UI, mirroring the existing reorder flow.
## Changes
- Migration: `hidden_on_dashboard` boolean on `accounts` (hidden from
default serialization like `position`, surfaced explicitly in the net
worth evolution payload).
- `AccountController::updateVisibility` +
`UpdateAccountVisibilityRequest` (owner-authorized) and the
`accounts.visibility` route.
- New `AccountsManagerDialog` component; dashboard header + plain grid;
removed the now-unused `dragHandle` prop from `AccountBalanceCard`.
- New `es.json` keys.
## Tests
Added feature tests for the visibility endpoint (toggle, validation,
ownership). Full `AccountControllerTest` and `DashboardAnalyticsTest`
suites pass; lint and types are clean on the touched files.
## Why
Production log dashboards were flooded with paired `Banking sync failed`
+ `EnableBanking API error` warnings for a handful of users, recurring
on every 6-hour `banking:sync` cycle for days. Investigation (prod
`BankingSyncLog` + connection state) showed the affected connections are
**healthy and syncing daily** (`last_synced_at` = today,
`consecutive_sync_failures = 0`): the first attempt hits a transient
`ASPSP_ERROR` / `429`, and the job's retry recovers it.
So the warnings are **noise, not breakage** — but
`SyncBankingConnectionJob` logged `Banking sync failed` inside the catch
block on *every* attempt, including non-final ones that later succeed.
That's one warning per cycle for connections that sync fine, which reads
as a failure and causes alert fatigue.
## What
Gate the `Log::log(...)` call so it only fires when the connection
actually gives up:
- the **final attempt** (`attempts() >= tries`), or
- a **permanent auth error** (`isAuthError`).
Transient errors recovered by the retry are no longer logged.
Per-attempt traceability is unchanged: `BankingSyncLog` still records
every attempt (Success / Failed) in the database.
## Tests
`tests/Feature/OpenBanking/SyncRetryAndLoggingTest.php`:
- non-final attempt → `Log::log` is **not** called
- final attempt → `Log::log('error', 'Banking sync failed', ...)` **is**
called once
Full file: 24/24 passing.
## Why
Prod warning logs were flooded with `Exchange rate not found, returning
unconverted amount` (≈500 lines from a single dashboard load). The
`ExchangeRateService` was behaving correctly — the source currency was
the ISO 4217 placeholder **`XXX`** ("no currency"), which has no
exchange rate, so conversions silently fell back to an **unconverted
(wrong) amount**.
Root cause: when importing accounts from a banking connection (Enable
Banking can report `currency: "XXX"`), the code did
`$accountData['currency'] ?? 'EUR'`. The `??` only catches
`null`/missing, not the literal `"XXX"`, so the placeholder was
persisted as `currency_code`.
Prod data confirmed `XXX` is the **only** account currency not covered
by the rates table (342 currencies, incl. BTC/VES/exotics, are all
present): 18 accounts (16 from bank links, 2 manual) across 14 users,
plus **5 users** whose base currency had itself become `XXX` (a
first-account `XXX` propagates to the user via `syncFromFirstAccount`).
## What
1. **Fix the source** —
`AccountUserCurrencyService::resolveImportedCurrency()` resolves a
bank-reported currency to: bank value → user base currency → app default
(`cashier.currency`), treating `XXX`/empty/missing as "no currency".
Wired into both creation paths (`CreatesAccountsFromPending`,
`AccountMappingController`); also covers Interactive Brokers imports.
2. **Backfill** — migration fixes existing rows in order: `XXX` users →
app default first, then `XXX` accounts → their owner's currency.
## Tests
- `AccountUserCurrencyServiceTest` — resolver chain (valid / uppercasing
/ XXX·empty·null → user / both missing → default).
- `AccountMappingTest` — mapping flow falls back to the user currency
when the bank reports `XXX`.
- `BackfillXxxAccountCurrenciesTest` — migration resolves both XXX
owners and XXX accounts end-to-end.
23 tests green; Pint and Larastan clean.
## What
A 3-way experiment on how the paid plan is offered, plus per-variant
measurement. New signups (on/after `SUBSCRIPTION_EXPERIMENT_STARTED_AT`)
are split evenly into:
- **control** — current 15-day trial.
- **reduced_trial** — shorter trial: 3 days monthly, 7 days yearly.
- **pay_now** — charged immediately (no trial), with a self-service
money-back guarantee for the first 3 days.
Earlier users stay **legacy** and keep the 15-day trial. **While
`started_at` is null the experiment is off and everyone behaves like
control — inert until activated via env.**
## How it works
- **Assignment** — `App\Features\SubscriptionExperiment` (Pennant),
deterministic even split by a stable hash of the user id. QA can force a
variant with `feature:enable`.
- **Offer policy** — `ExperimentOffer` is the single source of truth for
trial days per plan, the pay-now flag, the refund window and refund
eligibility; shared by checkout, paywall and billing.
- **Checkout** — trial length comes from the variant (`trialDays(0)` for
pay_now → immediate charge).
- **Onboarding clarity** — the paywall states the exact terms above the
CTA: trial length for the selected plan, or "charged €X today + 3-day
money-back guarantee" for pay_now.
- **Self-service refund (pay_now)** — Settings → Billing, within the
window: refunds the upfront charge, `cancelNow`, revokes bank
connections keeping imported data. `refunded_at` records it and blocks a
second refund. Crash-safe ordering: the refund is stamped before
cancel/disconnect, which run best-effort in a try/catch.
## Measurement
`stats:experiment-funnel` (weekly → Discord): per-variant funnel
(assigned, subscribed, status breakdown, refunds) with a **net-active
rate** gated by each variant's decision window (control 15d / reduced 7d
/ pay_now 3d) so cohorts are read at equal age. Attribution reads the
variant Pennant actually served each user, so the report can't drift
from what users experienced. It also reports **MRR** (monthly run-rate
of mature net-active subs, yearly normalised ÷12) and **ARPU** (MRR ÷
assigned) per variant — ARPU is the revenue metric for the winner
decision. Plus a winner can be pinned org-wide with
`SUBSCRIPTION_EXPERIMENT_FORCE_VARIANT` (env, no deploy).
## Config (env)
- `SUBSCRIPTION_EXPERIMENT_STARTED_AT` — activates the experiment
(launch date). Null = off.
- `SUBSCRIPTION_EXPERIMENT_REDUCED_TRIAL_MONTHLY` (3), `..._YEARLY` (7),
`..._REFUND_WINDOW_DAYS` (3)
## Tests
- **Feature/unit:** assignment, offer policy, checkout wiring, refund
eligibility, the refund action incl. idempotency + crash-safe ordering
(Stripe mocked), and the funnel collector/command. ES + FR translations.
- **Browser** (`tests/Browser/SubscriptionRefundTest.php`): the
self-service refund UX end to end — card visibility + deadline, two-step
confirm, back-out, the refund control disappearing after confirming, and
gating (window passed / non-pay_now hidden). The `RefundSelfServe`
action is doubled so it never hits Stripe but applies the same DB
effect. Screenshots: `refund-card-visible`, `refund-confirm-step`,
`refund-completed`.
- Full non-browser suite green (the one failing `DashboardTest` is
pre-existing on `main` — Inertia 409 from the unbuilt local manifest).
Pint + ESLint + tsc (changed files) clean.
## Two independent reviews — acted on
**Fixed:** refund atomicity/idempotency (major) · funnel attribution now
reads Pennant's served value instead of recomputing, killing
report-vs-runtime drift (major) · pay_now copy shows the exact amount
charged · throttle + block-demo on the refund route · `resolve(?User)`
nullable · French translations.
**Reviewer notes (deferred, low value):**
- `refunded_at` is not cast to Carbon on Cashier's `Subscription` (safe
today — only null-compared; would need a custom Cashier model).
- `ExperimentFunnelCollector` walks users in PHP via `chunkById`; fine
at current volume, can move to grouped SQL if it grows.
## Confidence: 85 / 100
The critical money path is now **verified live against the Stripe
sandbox** (see below), which removes the earlier cap. All gates are
green and the acceptance criteria are met. Held at 85 (not higher)
because the browser UI test runs in CI rather than locally, and the
pay_now *hosted-checkout + webhook* leg reuses the standard Cashier
checkout already proven by the control flow (only `trialDays(0)`
differs) but wasn't re-driven through the hosted page. Given it moves
money + disconnects accounts, a human glance is still warranted before
enabling.
## Sandbox verification (live Stripe test mode)
`php artisan stripe:verify-refund` creates a real immediately-charged
subscription with a test card, runs the actual `RefundSelfServe`, and
checks the Stripe API. Result:
```
PASS subscription active after immediate charge (pay_now, no trial)
PASS canSelfRefund is true before refund
PASS latestPayment() resolves a payment intent
PASS refunded_at is stamped
PASS subscription is canceled
PASS canSelfRefund is false after refund
PASS Stripe charge shows a full refund (refunded=true)
```
The command is committed and guarded to Stripe test keys /
non-production, so it can be re-run before each launch toggle.
## Launch checklist
1. Stripe-sandbox smoke: `php artisan stripe:verify-refund` (done —
passing). Optionally also drive the hosted pay_now checkout once for
monthly + yearly to confirm the webhook leg.
2. Set `SUBSCRIPTION_EXPERIMENT_STARTED_AT` to the launch date (set
once; don't backdate).
3. Watch `stats:experiment-funnel`; a clean cohort baseline lands once
each variant's window matures.
## Summary
- **Fix:** `integration-requests:review` crashed with `Attempt to read
property "email" on null` when a request's author no longer exists. The
command now renders a dash instead of assuming the `user` relation is
present.
- **Feature:** new terminal `done` status that marks an integration
request as shipped.
## The `done` status
Modeled after `not_doable` (a frozen, terminal state):
- Shown on the board, sinking to the bottom regardless of votes.
- Can no longer be voted on or unvoted (`removeVote` returns 404).
- Drops its public comment when set, so the board shows no stale note.
- Available in both `integration-requests:review` flows (pending review
and `--all`).
Frontend gets a `Done` badge and an `isFrozen()` helper that replaces
the scattered `=== 'not_doable'` checks gating the vote/unvote buttons.
## Tests
- 6 new feature tests: visibility without comment, bottom ordering,
vote/unvote blocked, command sets `done` and clears the comment, and the
command tolerating an orphaned author.
- `php artisan test tests/Feature/IntegrationRequestTest.php` → 34
passed.
- Board vitest suite, Pint, ESLint and Prettier all green.
## What
Adds a weekly **registration → subscription → paid** funnel so we can
baseline subscription conversion and, later, measure A/B tests against
it.
- `stats:subscription-funnel` artisan command — prints the cohort table
and posts it to Discord (same webhook as the other weekly reports).
- `SubscriptionFunnelCollector` — builds the age-normalized weekly
cohort series.
- Scheduled weekly, Mondays 09:15, next to the existing cohort reports.
## How the funnel is defined
Every stage is measured from each user's **own signup**, so weekly
cohorts are compared at the same age regardless of the calendar.
- **Registered** — signups that week.
- **Subscribed** — started a `default` plan ≤30d after signup (entered
checkout/trial).
- **Paid** — that plan billed past the 15d trial: currently `active`, or
`canceled` only after outliving the trial. `trial_ends_at` is cleared on
cancel, so subscription lifespan is the reliable "did it ever bill"
signal.
Both stages are bounded by the same subscription-creation window, which
guarantees the funnel invariant **registered ≥ subscribed ≥ paid** (a
test enforces it — I caught and fixed a window bug where `paid` could
exceed `subscribed`). Cohorts too young to score show `pend`; signup
surges (launch/marketing waves) are flagged with ⚡.
## Current baseline (prod, 2026-06-27)
| Stage | Count | Rate |
|---|---|---|
| Registered | 1,289 | — |
| Subscribed | 158 | 12.3% of registered |
| Trial resolved | 128 | conversion denominator |
| Paid after trial | 91 | **71%** of resolved · 7.1% of registered |
| Currently active | 75 | 82% of payers retained |
⚠️ ~940 of the 1,289 signups arrived in the last ~5 weeks (a
late-May/June surge); their trials haven't fully resolved, so
cohort-level conversion for that wave isn't measurable yet. The 71% is
status-based — a clean cohort baseline lands in ~4–6 weeks.
## Tests
5 Pest tests covering cohort counting, the attribution window, the
funnel invariant, maturity gating, and Discord delivery. All green, Pint
clean.
## What
Updates the transaction filters popover:
- **Reordered** the filter sections to: **Date, Amount, Category,
Labels, Accounts, AI, Counterparties**.
- **Accounts** now use a searchable dropdown (the same pattern as
Categories/Labels) instead of toggle badges. Each option shows the
**bank logo** as its icon (with a `Building2` fallback for accounts
without a logo/bank).
## Why
The badge list didn't scale well with many accounts and was visually
inconsistent with the other multi-select filters. A dropdown keeps the
popover compact and adds search.
## Notes
- Fixed `handleAccountToggle` parameter type (`number` → `UUID`) to
match `accountIds`.
- Each account `CommandItem` uses a unique `value` (`name + id`) so
accounts that share a name aren't collapsed by cmdk's default filtering.
- Added Spanish translations for the new keys (`Select accounts...`,
`Search accounts...`).
## Tests
- New `transaction-filters.test.tsx` covering select/deselect of an
account and the empty state.
- Full front-end suite green (244 tests), ESLint/Prettier clean,
localization test passes.
## Why
Sentry issue **PHP-LARAVEL-3S** (`ProviderOverloadedException: AI
provider [gemini] is overloaded`) recurs whenever Gemini returns 503/429
under high demand — 7 events in 3h during the last surge, 0 users
impacted.
Two problems behind it:
1. **Sentry noise.** `CategorizeTransactions::resolveChunkWithRetry`
retries, `laravel/ai` fails over providers, and a still-failing chunk is
deliberately dropped so the rest of the backfill proceeds. But the catch
reported **every** dropped chunk via `report()`, so an expected,
self-healing transient condition floods Sentry and buries real bugs.
2. **Silently lost work.** A dropped chunk leaves those transactions
uncategorized (`category_id NULL`) with nothing to re-trigger them: the
backfill jobs are one-shot (`tries = 1`), there is no scheduled
backfill, and the real-time listener only handles *new* transactions.
They stay uncategorized until someone manually re-runs a backfill.
## What
**1. Stop reporting transient failures.** Catch `FailoverableException`
(the marker interface for `ProviderOverloadedException` /
`RateLimitedException`) separately and log a warning instead of
reporting it. Everything else still goes to `report()` unchanged, so
real failures (malformed responses, insufficient credits, …) keep
surfacing.
**2. Retry the dropped work.** On a transient failure, schedule a
deferred, per-user `RetryTransientAiCategorizationJob` that re-reads the
user's still-pending transactions once the provider has had time to
recover (`ai_categorization.retry_delay`, default 10 min).
- `ShouldBeUnique` per user collapses a surge of dropped chunks into a
**single** retry.
- The unique lock is held through processing, so a retry that overloads
again **cannot chain another** — exactly one deferred attempt per
failure, no infinite loop. Failed 503s aren't billed.
- Wired in `resolve()`, so it covers every entry point (backfill,
onboarding, real-time listener, admin command).
- Model cost is negligible (per config), so the retry re-runs the
existing backfill rather than tracking which exact chunks failed.
## Tests
- Transient overload → chunk dropped, nothing reported, retry scheduled
for the user.
- Unexpected failure → reported, **no** retry scheduled.
- Retry job → categorizes still-pending transactions for a consenting
user; no-op without consent (agent never prompted).
Full `tests/Feature/Ai` + listeners suite green (111 tests); Pint clean.
## Existing prod backlog
The transactions already dropped before this ships stay `pending`; an
`ai:categorize-backfill <user>` recovers them on demand.
Fixes PHP-LARAVEL-3S
## What
Three tweaks to the **Add Transaction** form:
1. **Account selector moved to the top** of the create form, above the
date field.
2. **"Update account balance" checkbox hidden for connected accounts**
(accounts with a `banking_connection_id`). Their balance is kept in sync
by the banking connection, so the submit also skips the balance update
for them.
3. **Amount placeholder** now shows a negative example `-25.00` instead
of `0.00`.
## Testing
- Added a unit test covering that the checkbox is hidden for a connected
account.
- `bun run test edit-transaction-dialog` — 5 passed.
- `bun run format` / `bun run lint` clean (only a pre-existing warning
in `chart.tsx`).
## What
Adds a lifecycle email sent **2 days after a user records AI consent**,
but only when that consent was given **outside of onboarding** — i.e.
more than 3 days after sign-up. This targets users who deliberately
opted into AI later (e.g. via the billing toggle / transactions banner
gated by the `AiConsentSettings` flag), not those who consented during
initial setup.
## How
- **`email:ai-consent-follow-up`** scheduled command (daily, 10:15
Europe/Madrid) selects active consents accepted exactly 2 days ago,
skips onboarding-era ones (`accepted_at <= created_at + 3 days`), and
queues a job per user.
- **`SendAiConsentFollowUpEmailJob`** sends the mailable and records a
`UserMailLog`. Dedup, locale and the queued-email plumbing all reuse the
existing drip system — **no migration**.
- Email is localised automatically via the user's `HasLocalePreference`,
like every other drip email.
## Design notes
- **Scheduled command, not a 2-day delayed job** — survives worker
restarts and respects consent revoked in the meantime.
- **Exact "2 days ago" window**, matching the existing paywall drip. If
the scheduler misses a day that cohort isn't retried; kept consistent
with the other drips on purpose.
- Copy is a first draft — open to wording tweaks.
## Tests
`tests/Feature/SendAiConsentFollowUpEmailsCommandTest.php` — 9 tests
covering the happy path, onboarding exclusion (incl. the exact-3-days
boundary), timing window, revoked consent, the disabled-drip gate,
single-send + mail log, no-resend, and locale rendering. All green, Pint
clean.
## Why
We pay for AI transaction categorization on Gemini, but we couldn't tell
which model produced any given suggestion. The `gemini-flash-latest`
alias used by categorization is **floating** — it silently drifted from
the `flash-lite` tier up to `gemini-3.5-flash`, which is now the
dominant cost on the Gemini dashboard. Before swapping categorization to
a cheaper pinned model we need to measure accuracy per model, and for
that we have to know which model labelled each row.
We already store the AI-suggested category and its confidence on
`transactions`; this adds the missing third axis: the model.
## What
- New nullable `ai_model` column on `transactions`, written by
`CategorizeTransactions::recordOutcome` from
`config('ai_categorization.model')` whenever the model produces a
suggestion (above or below the label bar).
- The migration backfills existing AI-categorized rows
(`ai_suggested_category_at` not null) with `gemini-3.5-flash` — the
model `gemini-flash-latest` resolved to when those suggestions were
made. Done as a raw update so it doesn't bump every row's `updated_at`.
- `ai_model` added to `$fillable` and to `$hidden` (pure ops/measurement
metadata, no need to ship it to the frontend).
## Testing
- Extended `CategorizeTransactionsTest` to assert `ai_model` is stored
on both the auto-applied and below-bar paths.
- `php artisan test tests/Feature/Ai/CategorizeTransactionsTest.php` → 5
passed.
## Follow-ups (not in this PR)
- Pin `AI_CATEGORIZATION_MODEL` to an explicit cheaper tier (mirror the
existing `AI_SUGGESTIONS_MODEL=gemini-2.5-flash-lite`) and kill the
floating alias in the config defaults.
- Capture token usage from the SDK response for cost tracking.
## What
Removes the `InteractiveBrokers` Pennant feature flag. The integration
is now always available to every user.
## Changes
- Delete the `App\Features\InteractiveBrokers` flag class.
- Drop the per-user `abort_unless(...->active(...))` gate from
`InteractiveBrokersController`.
- Stop sharing the `interactiveBrokers` flag in Inertia props
(`HandleInertiaRequests`) and remove it from the `Features` type.
- Remove the now-unused `feature` gating mechanism from the
connect-provider registry and `useConnectFlow` — Interactive Brokers was
its only consumer.
- Update tests: drop the "blocked when flag off" case and all flag
activations; the connect dialog now asserts IB is always offered.
The Interactive Brokers integration itself (provider enum, client,
syncer) is untouched.
## Testing
- `php artisan test InteractiveBrokersControllerTest
InertiaSharedDataTest` — 17 passed
- `vitest connect-account-dialog` — 7 passed
- `bun run lint` / `bun run format` — clean
## What
`feature:enable <feature> <target>` now accepts a percentage like `25%`
as the target, in addition to a user email or `all`.
```bash
php artisan feature:enable AiConsentSettings 25%
```
This activates the feature for a random `ceil(total * N/100)` sample of
the **current** users, persisted in Pennant's `features` table — no need
to edit the feature class and redeploy for a percentage-based rollout.
## Notes
- It targets the current user base. New users keep the feature's default
(`false`) unless the command is run again.
- Percentage is validated to be between 1 and 100.
- `feature:disable` was left unchanged — disabling for a random subset
has no clear use case.
## Test plan
- [x] `feature:enable AiConsentSettings 40%` enables the feature for
exactly 4 of 10 users
- [x] An out-of-range percentage (`0%`) fails
Víctor Falcón
Anshul Nitin
