#3404 added DailyExpenseTotals with scoping SQL that mirrored Totals, and
the same fragments (classification CASE, currency-converted amount,
entries/accounts/exchange-rates joins, budget-excluded kinds, tax-advantaged
and finance-account scoping) were already duplicated in FamilyStats and
CategoryStats. This extracts them into
IncomeStatement::ScopedTransactionsQuery so every income statement number is
computed from one definition of what counts as a reportable transaction.
No behavior change: only whitespace in the generated SQL differs. A new
equivalence test runs each refactored class against a verbatim legacy copy
(test/support/legacy_income_statement_*.rb) over the class's full option
matrix (trade inclusion, account scoping, stats interval) on a dataset that
exercises every scoping rule, and asserts identical rows.
* Add cumulative spending chart dashboard widget
New dashboard section showing the selected month's running spending total
against the previous month's full curve on a shared day-of-month axis,
inspired by Copilot Money's Spending card.
- IncomeStatement#daily_expense_series: per-day expense totals in family
currency, same scoping as the other income statement totals (visible,
posted, budget-included transactions; report-included accounts; daily
exchange-rate conversion)
- PagesController: spending_trend section with month picker (clamped like
money_flow), cumulative series builder, delta vs. previous month
- spending-chart Stimulus controller (D3): previous month in gray, current
month in green with a today marker, gridlines with compact currency
labels, shared tooltip
- i18n (en) and model/controller tests
* Address PR review: locale-safe axis labels, currency/rate-aware cache key
- X-axis tick labels are now rendered server-side (I18n.l), one per axis
day: when the previous month is longer than the selected one it owns the
tail labels, so a tick can no longer roll past the selected month's end
(e.g. day 31 of a February view showed "Mar 3"), and labels follow the
app locale instead of D3's default English time-format locale.
- IncomeStatement#daily_expense_series cache key now includes the family
currency and the latest exchange-rate timestamp, since ExchangeRate::
Importer's upsert_all and currency changes leave entries/accounts
untouched and previously served stale chart data.
* Fix spending trend tests: empty-state month in axis test, dropped start_date key
- Axis-label test picked a month pair with no transactions, so the widget
rendered its empty state and there was no chart payload to parse; seed
spending in both months under test.
- The clamp test still asserted on the payload's removed start_date key;
assert the clamped month via the current series' first point date instead.
* Support Wise Strong Customer Authentication for balance statements
The balance-statement endpoint always 403s because it requires a signed
one-time-token challenge (SCA) that Sure never implemented, so every sync
silently fell back to /v1/transfers — an outgoing-only endpoint — meaning
incoming payments into a Wise balance never synced.
Adds a per-item RSA keypair (private key encrypted at rest) that signs the
SCA challenge and retries the statement request once, plus a settings UI
to generate the keypair and register its public key with Wise.
Fixes#3384
* Backfill incoming statements past legacy transfers; fix review nits
Backfill: once statements start succeeding for an account that already has
legacy /v1/transfers rows, the fetch window was clamped to end the day
before the oldest legacy transfer, so the window where incoming payments
were actually missing (the recent window transfers already "covered" with
outgoing-only data) was never re-fetched. Statement rows in that overlap
are now kept when they're incoming and dropped when outgoing, since the
legacy transfer rows already account for the outgoing side.
Also: replace the inline onclick handler on the SCA public key display with
the existing clipboard Stimulus controller (copy button, matching the API
key reveal pattern), and correct the regenerate-keypair confirmation text,
which implied local regeneration revokes the key with Wise -- it doesn't;
the old public key stays valid there until removed manually.
* Avoid double-booking internal cross-currency conversions on statement backfill
The backfilled statement fetch's outgoing/incoming filter only looked at
sign: a positive (credit) statement row was always kept in the legacy
overlap window. But a legacy transfer row can itself be incoming for this
account when it's the target side of a conversion between two of the
profile's own balances -- Wise already fully captures both legs of those
via /v1/transfers, unlike genuine external payments.
Now an incoming statement row in the overlap window is dropped only when
it matches a known incoming legacy transfer's date and amount, so internal
conversions aren't duplicated while external incoming payments (no legacy
counterpart) still backfill correctly.
* Never drop an incoming statement row on a date/amount heuristic
The previous fix dropped an incoming statement row in the legacy-overlap
window when it matched a known incoming legacy transfer's date and amount,
to avoid double-booking internal cross-currency conversions. But nothing
short of an endpoint-proven correlation id can tell that apart from a
genuine external payment that happens to share the same date and amount --
and silently losing a real transaction is worse than an occasional visible,
user-correctable duplicate. Incoming rows are kept unconditionally again.
Instead, bound the exposure at the source: the /v1/transfers fallback now
stops running for an account as soon as it has a successful statement row,
since statements alone cover both directions from then on. This leaves only
a narrow, one-time window (the initial backfill of historical internal
conversions) where a duplicate can occur, rather than an indefinite one.
* Gate the transfer fallback per-account, not per-item
legacy_transfer_import_needed? decides whether to fetch /v1/transfers at
all, but that decision is profile-wide -- true as soon as any one account
still needs the fallback. store_transfers_per_account then merged those
transfers into every currency-matching account by currency alone, with no
check for whether that specific account had already migrated to
statements. A still-legacy account in one currency was enough to make an
already-migrated account in the same currency re-absorb a movement its own
statements already had, double-booked under a different key.
account_transfers is now cleared for any account that already has
statement rows, regardless of why the profile-wide fetch ran.
* Handle SCA controller errors, corrupted keys, and adapter test coverage
- generate_sca_keypair now rescues like every other mutating action in
this controller, logging and re-rendering the panel with an error
instead of a raw 500 if the update ever raises.
- sca_configured? now depends on sca_public_key actually parsing, not just
sca_private_key being present, so a corrupted/unparsable stored key
(encryption misconfig, manual DB edit) falls back to the "generate a
keypair" UI state instead of rendering a public key box around nothing.
- Added test/models/provider/wise_adapter_test.rb, which had no coverage
at all, to cover build_provider's family/wise_item_id resolution and
that sca_private_key actually reaches the constructed Provider::Wise.
* Add logging to Wise sync
---------
Co-authored-by: Juan José Mata <juanjo.mata@gmail.com>
* fix(ui): make whole list row clickable, not just the name text
Fixes#3366. Transaction, split-parent, trade, and account list rows carry
hover styling that implies the whole row is clickable, but only the name
text actually opened the detail drawer — clicking the avatar, amount, or
whitespace between them did nothing. Add a clickable-row Stimulus
controller that delegates a click anywhere on the row to its primary link,
while leaving real interactive descendants (checkbox, category menu,
account link, quick-edit badge, kebab menu) to handle their own clicks.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(ui): exclude popover/menu panels from row click delegation, preserve modifier clicks
Codex review on #3367 flagged two issues in clickable_row_controller:
- Category/account popovers render as position:fixed but stay DOM
descendants of the row, so clicking their background (padding,
headings, plain text) fell through to the row's link.
- linkTarget.click() discarded Ctrl/Cmd/Shift modifiers, so
modified clicks opened in the current frame instead of a new tab.
* fix(ui): exclude quick-edit dropdown, use window.open for modified clicks
CodeRabbit review on #3367 found two more issues:
- The investment activity quick-edit dropdown is a hand-rolled
absolute-positioned panel (not DS::Popover/DS::Menu), so it wasn't
covered by the earlier popover/menu exclusion and background clicks
inside it fell through to the row link.
- Redispatching a synthetic MouseEvent with modifier flags doesn't
actually open a new tab: browsers only honor Ctrl/Cmd/Shift on
trusted, native click events, so the previous "fix" was cosmetic.
Explicitly call window.open() for modified clicks instead.
* fix(ui): make Dividend/Interest quick-edit badge not swallow row clicks
jjmata found that the activity-label badge renders as a real <button>
even when Dividend/Interest trades have no dropdown/click handler
(income_trade), so clickable-row's "a, button, ..." exclusion still
treats it as an interactive descendant and swallows the click instead
of opening the row — same dead-spot symptom as #3366, relocated to
this one badge. Render it as a <span> in that case so the row click
delegates normally.
* fix(ui): show pointer cursor on delegated Dividend/Interest badge
CodeRabbit caught that the badge kept cursor-default after becoming a
non-interactive <span> that delegates its click to the row link — the
cursor no longer matched the actual (now clickable) behavior.
---------
Co-authored-by: GFR <248542187+gfr-free@users.noreply.github.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* feat(forms): parse pasted formatted amounts in money fields
Number inputs silently reject pasted values like "20,000 " or
"1.234,56", leaving the field blank. Money fields now intercept the
paste, run it through parseLocaleFloat (spaces stripped, thousands and
locale decimal separators handled), and insert the plain number — so
amounts copy cleanly out of statements and spreadsheets.
* fix(forms): reject non-amount pastes and honour currency precision
parseLocaleFloat coerces unparseable text to 0, so the finite check in
pasteAmount never fired. Pasting "$1,234.56" — or any stray text — wrote
0.00 over the field instead of falling through to the browser, which is
worse than the blank field the feature set out to fix. The clipboard text
is now validated first by a new parse_amount_paste util, which also strips
a leading or trailing currency symbol so statement formats parse, and
reads parenthesised amounts as negative rather than dropping the sign.
The precision fallback was hard-coded to 2 while the field renders with
the selected currency's default precision, so pasting into a BTC or KWD
amount lost digits. It is derived from the input's step instead, which
already tracks the live currency selection.
Dispatch change alongside input: auto_submit_form listens for change on
number inputs, so an auto-submitting money field never saved a pasted
value.
* fix(forms): read the sign before stripping the currency symbol
The currency strip ran before the sign was read and accepted any
non-digit run, so "$-500" parsed as +500 and "USD (1,200.00)" as +1200.
Since the handler also dispatches change, an auto-submitting money field
could post a debit as a credit. The same permissive strip turned prose
like "memo 500" into 500.
The text is matched against a strict grammar instead: an optional sign,
an optional currency symbol or code of at most three characters on either
side, and a number. The sign is read first so it survives the strip, and
parentheses still mark a negative with the currency allowed outside them.
Anything that does not fit returns null and the browser handles the paste.
* fix(forms): accept only symbols and ISO codes as currency markers
The currency token matched any one-to-three character non-digit run, so
"fee 500" and "500 tax" parsed as 500 rather than falling through to the
browser. With the change event this handler dispatches, an auto-submit
form could save an amount lifted out of prose.
A marker is now either a letter-free symbol or a three-letter uppercase
ISO code. Lowercase prose no longer matches. The cost is that markers
containing letters, "R$" and "kr" among them, are no longer stripped and
those pastes fall through untouched — the safe direction, since accepting
them means accepting "fee 500" too. Telling them apart would need the
server's currency list, which a paste event cannot wait for.
* fix(forms): strip only currency symbols from config/currencies.yml
Matching a currency marker by shape accepted anything shaped like one:
"TAX" satisfied the three-uppercase-letter branch and "***" satisfied the
symbol branch, so "TAX 500" and "*** 500" both parsed as 500 and could be
written into the field.
The marker is now one of the 29 letter-free symbols this app already
declares in config/currencies.yml, so the allowlist is the supported set
rather than a shape. Lettered markers, "USD" and "kr" and "R$" among them,
are no longer stripped: telling them from prose needs the currency list,
which a paste event cannot wait for, and leaving the field untouched is
the safer of the two failures.
* fix(forms): reject multi-cell pastes and keep step="any" precision
Three parsing gaps, all reachable from a normal paste.
Internal whitespace was allowed anywhere inside the digit run, so two
adjacent spreadsheet cells arrived as one amount: "100\t200" parsed as
100200 and "1,234.56\t500" as 1234.565. Only the spaces locales actually
use to group digits are accepted now (space, no-break space, narrow
no-break space), so "1 234,56" still parses and a tab- or newline-joined
paste falls through to the browser.
#pastePrecision fell back to two decimals whenever the step was not
numeric, and Number("any") is NaN. Four money fields render step="any"
with no precision — the trade amount, price and fee on trades/show and
the fee on trades/_form — so a sub-cent crypto price pasted there was
truncated to "0.00". A step that declares no precision now writes the
parsed value unrounded rather than rounding it to a guess.
The doc comment still used "USD (1,200.00)" as its worked example, which
stopped parsing when currency markers were narrowed to letter-free
symbols.
The cases now import the shipped parser instead of a hand-copied
duplicate, rewriting its importmap specifier to a file URL so Node can
resolve it. That deletes the copy that could drift, and the three new
multi-cell cases fail against the previous parse_amount_paste.js with
"actual: 100200, expected: null".
Verified with node --test test/javascript/. Ruby and lint checks are
unchanged by this commit; biome runs in CI.
* feat(transactions): cascade parent/subcategory checkboxes in the category filter
Checking a parent category in the transaction filter sidebar now auto-checks
its subcategories, and vice versa. Unchecking a single subcategory also
unchecks the parent so the submitted filter never silently includes a
category the user just deselected — Transaction::Search#apply_category_filter
includes every subcategory whenever a parent name is present, with no way to
exclude one individually, so the parent checkbox must reflect exactly what
gets submitted.
Also fixes the "swipe-to-categorize" pill picker (transactions/categorizes/show.html.erb),
which was still a flat alphabetical list with no parent/child indication —
now grouped and labeled consistently with the rest of the app (PR #2845, #3292).
Adds an :indeterminate style for .checkbox--light (only .checkbox--dark had one).
Closes discussion #3149.
This code was written by Claude Code (Anthropic).
* fix(transactions): preserve parent-only category filters on reopen
connect() derived each checkbox's parent/child state independently from
server-rendered checked attributes, so a parent-only filter (e.g. an
incoming link naming only the parent category) rendered the parent
checked with its children unchecked — syncParentState() then read that
as "some children unchecked" and cleared the parent, silently dropping
the filter on the next Apply. Cascade checked parents to their children
before deriving parent state so the picker matches the active query.
Also makes the internal helper methods private per review feedback.
* fix(transactions): eager-load category parent, fix categorize-pill filter text
Addresses PR #3356 review from jjmata:
- Current.family.categories.alphabetically caused an N+1 (SELECT per
parent) via display_name_with_parent in the categorize-wizard pill
loop. Added .includes(:parent) at all three call sites.
- data-filter-name still used the bare category name while the pill
label showed "Parent > Child" for subcategories, so searching by the
visible parent prefix found nothing. Both now share one computed
label.
---------
Co-authored-by: Gerald <248542187+gfr-free@users.noreply.github.com>
Disabled accounts were already excluded from the dashboard and the new
transaction modal, but the new transfer modal's from/to account selects
still listed them. Add the same `.active` filter used by the transaction
form to TransfersController#set_accounts.
Claude-Session: https://claude.ai/code/session_01P9MCGiD5KEZGy9wB6ySuRA
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* fix(i18n): localize recurring occurrence feedback
* Address PR review feedback (#3382)
- Localize every due-label state rendered by the occurrence drawer
- Cover the German due labels with fallback-disabled and rendered UI assertions
---------
Co-authored-by: Johns <19662585+Rowdy@users.noreply.github.com>
* feat: verify AI configuration and provider liveness from worker processes
Closes#3169.
The AI status page (#3145, PR #3155) proves only that the `web` process
resolved a valid-looking configuration and can reach the configured
provider from its own network context. Most AI workloads -- assistant
responses, PDF processing, embeddings, auto-categorization, and merchant
detection -- actually run in Sidekiq `worker` processes, which can differ
from `web` in environment, DNS, proxy rules, network policy, or even
loaded credentials (workload-specific overrides, an updated Secret without
a pod restart, `web` recreated without `worker`). A passing web check says
nothing about whether a worker can do the same.
## What this adds
`WorkerAiHealthCheckJob`, queued on demand from a new "Verify worker
configuration" button on System health -> AI status. It runs the same
bounded, non-destructive probes `AiHealth` already runs, but from inside
whichever Sidekiq worker process dequeues it, and records the result via
`WorkerAiHealth`: process identity (hostname:pid), checked-at time, a
non-secret configuration fingerprint (effective provider, model, redacted
endpoint, vector-store adapter/embedding config), and probe outcomes.
The AI status tab lists every recorded result -- most recent first, kept
for `WorkerAiHealth::RETENTION` (15 minutes) -- each labeled with a status
pill (`Passing` / `Failing` / `Stale`, the last once older than
`STALE_AFTER`) and a configuration pill comparing it against the web
snapshot (`Matches web` / `Differs from web`), with a failure-reason list
reusing the existing failure-code translations when a probe failed.
## Implementation constraints from the issue, addressed directly
- **Cannot reuse a web-cached probe result, or vice versa.** `AiHealth.new`
gained an injectable `probe_cache:` (default `Rails.cache`, matching
today's behavior). The worker job passes a fresh
`ActiveSupport::Cache::NullStore` instead, so every worker check is a
live call that neither reads a web-cached entry nor leaves one behind.
- **A single job only verifies one worker.** Documented on the button
(`coverage_notice`) and in the docs: with multiple replicas, a passing
result names one process, not the fleet. Queuing again samples another.
- **Never persists or displays a raw credential.** `WorkerAiHealth::Snapshot`
only carries redacted endpoints (AiHealth already redacts these before
they reach the job) and provider/model/status fields -- there is no field
for a token to occupy. A structural test asserts this stays true.
- **Failures land in both places an operator already checks.** Same
destinations as `AiHealth::Probe`'s own failures: `Rails.logger` and
`DebugLogEntry` (new `ai_health_worker` category), tagged with the
process identity.
- **Results carry a clear status**, including the `pending` case implicitly
(no result yet renders an explanatory empty state) and `stale` for a
result whose process may no longer reflect current state.
- **DB-backed vs ENV-backed settings are labeled.** A new info block next
to the worker results explains which UI settings propagate automatically
(rails-settings-cached invalidates the shared cache on write) versus
which require restarting/recreating both `web` and `worker`.
## What this deliberately doesn't do
Full-fleet coverage (every process publishing a periodic fingerprint) --
the issue lists this under "Other options to consider," not the acceptance
criteria, and Sidekiq's normal dispatch doesn't target every process
without an explicit per-process coordination mechanism. This PR implements
the on-demand, single-check design the acceptance criteria actually
describes ("An administrator can request an asynchronous worker-side
check", "does not imply full-fleet coverage"); periodic fleet-wide
publishing is a natural follow-up if operators need it.
## Testing
- `WorkerAiHealth`: recording/reading, same-process replacement vs.
cross-process coexistence, MAX_RESULTS bounding, staleness, status
derivation (failure codes / component statuses / function-calling
refusal), `matches_web?` comparison, and the credential-field structural
guard.
- `WorkerAiHealthCheckJob`: records a passing/failing snapshot naming this
process, writes failures to Rails.logger + DebugLogEntry (and only on
failure), never leaks the access token, and -- the defining property --
is proven to construct `AiHealth.new` with an isolated `NullStore`
rather than the shared web-facing cache.
- `Admin::SystemHealthController`: empty state, a rendered result with
matching/mismatched configuration, a failing result's failure reason, a
stale result, the `verify_worker_ai` action enqueuing the job and
redirecting with a flash notice, and that non-super-admins and
unauthenticated requests cannot trigger it.
I could not run the Rails test suite in this environment (no working
Ruby/Bundler toolchain available locally -- Ruby 2.6 system Ruby vs. the
project's required 3.4.9, no way to install without sudo/Docker access).
Every file was checked with `ruby -c`, YAML files with `YAML.load_file`,
and the ERB view with `ERB.new(...).src`, plus careful manual tracing of
each test against the production code paths it exercises, but CI should
be treated as the first real run of this suite per the repository's own
guidance for exactly this situation.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A9494Pxh4LZKnNLTGfFXPw
* fix: correct test setup bugs found by actually running the suite
Set up a working Docker-based Rails environment (this session's shell
had no compatible Ruby/Bundler) and ran the full suite against PR #3298.
Two real bugs surfaced that static checks couldn't have caught:
- WorkerAiHealth::Snapshot.new(**{...}.merge(overrides)) needs the
double-splat -- a bare Hash isn't auto-converted to keyword arguments.
Both test snapshot builders passed a positional Hash instead, which
raised "missing keywords" for every field on every call.
- assert_enqueued_with/assert_no_enqueued_jobs need `include
ActiveJob::TestHelper` explicitly in a plain ActiveSupport::TestCase --
every other model test in this codebase that uses them does the same;
I'd wrongly assumed it was available process-wide.
With both fixed: 7510 runs, 29877 assertions, 0 failures, 0 errors, 30
skips for the full suite; rubocop, erb_lint, and brakeman all clean.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A9494Pxh4LZKnNLTGfFXPw
* fix(worker-ai-health): address feedback on health status and cache handling
- Add checks for 'not_configured' and 'unavailable' states in Snapshot#status
- Include PDF probe failure codes in failure_codes detection
- Fix cache lifetime extension by removing expires_in and filtering expired entries in recent()
Ensures unconfigured workers and missing PDF pipelines are marked as failing,
and stale cache entries don't get indefinite TTL refreshes.
Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A9494Pxh4LZKnNLTGfFXPw
* fix(worker-ai-health): fix stub gaps causing ci/test_unit failure
stub_ai_health in WorkerAiHealthCheckJobTest omitted
pdf_text_extraction_probe/pdf_vision_processing_probe, so
WorkerAiHealthCheckJob#failure_codes raised NoMethodError on nil.
Also fix vector_store_status to :missing (no adapter configured) rather
than :not_configured (adapter configured but unusable) to match the
scenario AiHealth actually returns and Snapshot#status's semantics.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01A9494Pxh4LZKnNLTGfFXPw
* fix(worker-ai-health): compare request timeout, fix raw color class, document dev-mode cache caveat
- Add llm_request_timeout to WorkerAiHealth::Snapshot and compare it in
matches_web? (CodeRabbit) -- a worker with a different effective request
timeout than web (e.g. a workload-specific OPENAI_REQUEST_TIMEOUT override)
previously showed as "Matches web" despite a real configuration
difference, exactly the kind of drift this feature exists to catch.
- Replace border-alpha-black-25 with the border-primary functional token in
the worker result card (CodeRabbit nitpick).
- Document the dev-mode cache_store caveat jjmata flagged: bin/dev runs web
and worker as separate OS processes, and development.rb uses a
process-local memory_store/null_store, so a worker check queued locally
writes to a cache the web process never reads from -- "Verify worker
configuration" can appear to silently do nothing. Added a note to
docs/hosting/ai.md rather than changing behavior, since production's
shared Redis store is unaffected.
- Corrected the retention description in the same doc section (CodeRabbit,
most recent review): only the 5 most recently checked-in distinct
processes are retained (MAX_RESULTS), not "kept for RETENTION" -- a 6th
process checking in can evict an older entry before its own 15-minute
RETENTION window is up.
jjmata's four other findings (unconfigured-worker and missing-PDF-probe
states rendering as "Passing", and the cache-retention/TTL-extension issue)
were already fixed in 1a7b664d, before this pass -- verified against current
code, no changes needed there.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019yyETKmVExx3Q1rYwCynpb
---------
Signed-off-by: Juan José Mata <juanjo.mata@gmail.com>
Co-authored-by: Jonathan Kaiser <jaysbeekay@users.noreply.github.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Juan José Mata <juanjo.mata@gmail.com>
* fix(reports): stop Safari swallowing every click on the reports page
The category rows in the reports breakdown use a stretched link whose
`before:absolute before:inset-0` overlay was anchored to the parent
`<tr class="relative">`.
`position: relative` on a table row is left undefined by CSS 2.1 §9.3.1 and
WebKit does not implement it, so in Safari the row never becomes a containing
block. The overlay resolves against a far larger positioned ancestor instead,
blankets the reports page, and intercepts every click — including clicks on
the period picker popover — navigating to that category's transactions
drill-down. Chrome and Firefox do establish the containing block, which is
why this is invisible outside WebKit.
Anchor the overlay to the flex wrapper inside the cell instead. A `<div>` is
an unambiguous containing block in every engine. The click target narrows
from the whole row to the category cell, which is still the icon, the name
and the entry count.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* test(reports): assert the stretched overlay on the cell wrapper, not the row
The regression test still required `tr.relative`, which the previous commit
removes, so the suite would have failed. Assert instead that the stretched
link sits inside `td div.relative` — the containing block the overlay is now
anchored to.
Verified the selector against the rendered markup: it matches the clickable
row once, does not match a non-clickable row, and the old selector matches
nothing under the new markup.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* fix(transfers): prevent duplicate creation on double-submit
TransfersController#create -> Transfer::Creator had no protection
against a repeated form submission - a double-click, a browser retry,
or two near-simultaneous requests could each create a separate,
identical transfer (and its 2-4 underlying Entry/Transaction rows).
Adds a per-form idempotency key, the same approach already used for
TransactionsController#create: a UUID hidden field generated fresh on
page load, tagging the outflow/inflow (and fee, with a distinguishing
suffix since a fee leg shares its account with its primary leg) entries
via the existing entries(account_id, source, external_id) partial
unique index. A pre-check handles the sequential double-submit case;
rescue ActiveRecord::RecordNotUnique is the authoritative backstop for
genuine concurrent requests - the whole Transfer.transaction block
rolls back cleanly on conflict, so there's no risk of a half-created
transfer.
A same-day duplicate transfer can be legitimate (unlike a duplicate
valuation, see #3339/PR #3340), so this uses the same per-submission
token approach as #3334/PR #3338 rather than a natural-key DB
constraint.
Fixes#3341.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(transfers): store the idempotency key in its own column, isolate the retry with a savepoint
Same two review findings as PR #3338 (transactions) and #3340
(valuations), applied here since this branch shares the same
mechanism:
- Reusing external_id/source for the web-form idempotency token made
every leg of a manually-created transfer satisfy Entry#linked?,
incorrectly making it look provider-synced. Uses the same dedicated
entries.idempotency_key column added in
db/migrate/20260902180400_add_idempotency_key_to_entries.rb (cherry-picked
identically from PR #3338 - this branch depends on that migration;
please merge #3338 first, or merge this after it lands so the
duplicate migration file is a no-op).
- Transfer::Creator now wraps the actual save in
Transfer.transaction(requires_new: true) so a RecordNotUnique only
rolls back to a savepoint rather than aborting any transaction the
caller might already be in, keeping the rescue's retry lookup usable
(mirrors the fix already applied to Account::ReconciliationManager
in PR #3340).
Added a regression test asserting neither leg of a transfer created
via this path is linked? or has external_id/source set.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(transfers): distinct idempotency key per leg, rebuild invalid index on retry
Two more review findings:
- CodeRabbit: the form doesn't prevent selecting the same account as
both source and destination. The outflow and inflow legs shared the
bare idempotency key, so on that same-account path they'd collide
with each other under the same account-scoped unique index (as
would both fee legs, which shared a single "-fee" suffix). Every
leg now gets a distinct, role-specific suffix (outflow stays bare -
that's what find_existing_transfer looks up by - inflow/source_fee/
destination_fee each get their own).
- Codex (same finding already fixed once for entries.idempotency_key's
sibling migration, recurring here since this branch carries an
identical copy): index_exists? alone doesn't distinguish a valid
index from an INVALID one left behind by an interrupted CREATE INDEX
CONCURRENTLY, so a retry after a failed build would short-circuit
and record the migration as applied while the constraint was still
missing. Now checks pg_index.indisvalid directly before deciding to
skip.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(transfers): clear idempotency key on destroy so a retry doesn't 500
Codex flagged that Transfer#destroy! (used by reject!) preserves the
outflow/inflow entries but not the Transfer join row - a retried
create request with the same idempotency_key would find no Transfer
via find_existing_transfer, attempt another insert, hit the stale
entry's unique key, and re-raise RecordNotUnique instead of finding
a match. Clear the key on the surviving entries when a transfer is
destroyed.
Also adds a regression test for the CodeRabbit-flagged per-leg key
collision concern (already fixed by role-specific suffixes in the
prior commit) to lock in that fee legs never share a key with their
primary leg.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(transfers): verify idempotency key matches the request, fix stale doc comment
jjmata review on #3342:
- find_existing_transfer matched on idempotency_key + source_account only,
so a stale key from a cached form could silently return a different,
older transfer instead of creating the one actually requested. Now
verifies destination account, date, and amount before treating a key
match as the same request; a genuine mismatch surfaces as a new
StaleIdempotencyKeyError (422 + message) instead of a false success or
a raw 500.
- Removed a comment claiming parity with a
TransactionsController#new_transaction_idempotency_key method that
doesn't exist in the codebase.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(transfers): preserve from_account_id on error, match fees/exchange rate in idempotency check
coderabbitai review on #3342:
- All three create rescue blocks (exchange rate unavailable, invalid date,
stale idempotency key) failed to set @from_account_id, so the re-rendered
form lost the user's selected source account.
- matches_request? only compared accounts/date/outflow amount, so a retry
with the same key but a different exchange_rate or fee would be reported
as success while silently keeping the old inflow amount and fee entries.
Now recomputes the request's effective inflow amount and compares derived
fee totals too; a mismatch raises StaleIdempotencyKeyError like other
stale-key mismatches instead of silently returning the old transfer.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---------
Signed-off-by: Juan José Mata <juanjo.mata@gmail.com>
Co-authored-by: Gerald <248542187+gfr-free@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Juan José Mata <juanjo.mata@gmail.com>
* fix(transactions): prevent duplicate creation on double-submit
TransactionsController#create had no protection against a repeated
form submission - a double-click, a browser retry, or two
near-simultaneous requests could all create a separate identical
transaction. Adds a per-form idempotency key (a UUID hidden field,
generated fresh on page load) that reuses the existing
entries(account_id, source, external_id) partial unique index, with a
pre-check for the sequential case and a RecordNotUnique rescue as the
authoritative backstop for genuine concurrent requests - the same
pattern already used by mark_as_recurring and the public API's
idempotency-key support.
Fixes#3334.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(transactions): store the idempotency key in its own column, not external_id
Codex review finding: reusing external_id/source for the web-form
idempotency token made every manually-created transaction satisfy
Entry#linked? (external_id.present?), since the form always supplies
a key. That incorrectly made manual entries look provider-synced -
disabling their date/nature/amount/currency fields in the editor
(app/views/transactions/show.html.erb), and hiding them from future
provider dedup matching (which filters to external_id: nil).
Adds a dedicated entries.idempotency_key column with its own partial
unique index scoped by account_id, used only for this de-duplication
and with no meaning anywhere else in the app, so it can't collide with
provider-linkage semantics. TransactionsController now tags/looks up
entries by this column instead of source/external_id.
Added a regression test asserting a transaction created via this path
is not linked? and has no external_id/source set.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(migration): rebuild an invalid index left by an interrupted CONCURRENTLY build
Codex review finding: index_exists? alone doesn't distinguish a valid
index from an INVALID one left behind by an interrupted CREATE INDEX
CONCURRENTLY (e.g. a deploy killed mid-build). A retry after such a
failure would short-circuit on the early-return and record this
migration as applied, while the actual uniqueness constraint stays
missing/broken. Checks pg_index.indisvalid directly before deciding
whether to skip the rebuild.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(transactions): rotate idempotency token on bfcache/Turbo restore, keep index removal concurrent
Codex flagged that a page restored from the browser bfcache or Turbo's
snapshot cache (back button, duplicated tab) keeps the already-consumed
idempotency token in the hidden field. Submitting a different, edited
transaction from that restored page would then match the old committed
entry and silently redirect onto it instead of creating the new one.
transaction_form_controller now rotates the token on turbo:before-cache
so any later restore starts from a fresh, unconsumed value.
Also address CodeRabbit's note that the migration's down block did a
blocking DROP INDEX instead of DROP INDEX CONCURRENTLY.
* fix(transactions): also rotate idempotency token on native bfcache restore
CodeRabbit noted turbo:before-cache only covers Turbo's own snapshot
cache, not the browser's native bfcache (e.g. a full navigation away
and back, not through Turbo drive). Add a persisted-pageshow handler
alongside it, and wire both through declarative data-action bindings
on the form per this repo's Stimulus convention instead of manual
addEventListener/connect/disconnect.
* fix(transactions): fall back to manual UUID when crypto.randomUUID is unavailable
crypto.randomUUID() requires a secure context, but this app's self-hosted
mode is commonly reached over plain HTTP (LAN, reverse proxy without TLS).
On such a deployment, calling it inside the cache-restore rotation handlers
throws, leaving the stale, already-consumed idempotency token in the hidden
field — a later edited resubmission would then silently match the old entry
via find_duplicate_manual_entry and drop the user's edits. Build a v4 UUID
manually from crypto.getRandomValues (which has no secure-context
restriction) when randomUUID is missing.
Also drops a stale comment reference to a MANUAL_FORM_SOURCE constant that
doesn't exist anywhere in the codebase, and corrects a rescue comment that
still described the old (account_id, source, external_id) index instead of
the (account_id, idempotency_key) index actually backing this constraint.
---------
Co-authored-by: Gerald <248542187+gfr-free@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Adds a 'Running Sure on small (512 MB) hosts' section to the Docker
self-hosting guide: what fits in 512 MB (boot, onboarding, daily use,
small syncs/imports, cron - ~352 MB steady state), what does not
(demo-data generator deterministic OOM, very large first imports, AI
flavors), the tuning already baked into the image (jemalloc, YJIT,
Puma 1x3), and the operational steps for loading sample data on a
small host (raise the worker limit, fresh deploy - plan changes do
not apply on restart).
Co-authored-by: Instinct agent <agent@sure.am>
The account activity tab rendered split transaction children as flat,
ungrouped rows, unlike /transactions which collapses them into a
parent row with indented children when "Group split transactions" is
enabled. Wire the same EntriesHelper.group_split_entries logic into
the account activity feed (UI::Account::ActivityDate, the actual
render path since the ViewComponent refactor superseded the old
accounts/show/_activity partial), sourcing split parents via the same
single-query batched lookup pattern already used by
TransactionsController#index.
Also forward view_ctx through entries/_split_group so split children
render correctly regardless of which page renders the group.
Fixeswe-promise/sure#3227
Co-authored-by: Gerald <248542187+gfr-free@users.noreply.github.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* feat(ai): support OPENAI_EXTRA_HEADERS on OpenAI-compatible provider
Adds a fail-closed parser for the OPENAI_EXTRA_HEADERS env var (a JSON
object of header names to values) as a new Provider::Openai.extra_headers
class method. Malformed, non-object, blank, or unset values yield {}
with an error log and never the raw value, so chat keeps working on bad
config. Parsed headers are passed to the ruby-openai client at
construction, attaching them to every request the provider's client
makes (chat and batch flows alike).
ENV-only by design: no Setting fallback or settings-UI entry. Values are
stringified (nested JSON becomes Ruby-inspect strings) and blank values
are dropped.
Adds hosting docs and commented examples in .env.example and
.env.local.example, plus Minitest coverage mirroring the request_timeout
tests, including a docs-consistency test binding the knob to its docs.
* feat(ai): substitute {session_id} in OPENAI_EXTRA_HEADERS per chat request
Header values containing the literal {session_id} are now withheld at
client construction and merged onto the client at request time, with the
placeholder replaced by the chat's UUID. This identifies requests per
conversation rather than per install, for gateways that key sessions
(e.g. OpenCode Zen's x-opencode-session).
A session header is only merged when a session_id is present, so batch
flows (auto-categorize, merchant detection, PDF processing) — which
bypass chat_response — never send it; they receive static headers only.
The merge adds/overwrites without deleting managed headers.
Docs updated to cover both static and session-valued usage.
* fix(ai): keep OPENAI_EXTRA_HEADERS session values request-scoped
client.add_headers persists headers on the shared client in
ruby-openai 8.1.0, so a chat's resolved session header could survive
onto later requests made through the same provider instance. Session
headers are now merged onto a request-scoped dup of the client; the
shared client is never mutated. Batch flows and session-less chats
cannot observe another chat's session id.
Also updates the CodeRabbit-flagged tests to assert the shared client
stays untouched and the scoped copy is what issues the chat request.
* docs(ai): add YARD tags to OPENAI_EXTRA_HEADERS method docs
Converts the comment blocks on the four methods touched by this
feature (extra_headers, initialize, request_timeout, and
with_session_headers) into YARD docstrings with @param/@return tags,
satisfying CodeRabbit's docstring-coverage pre-merge check.
* Add Trade Republic provider integration
Introduce authenticated web and QR login, resilient account synchronization, deterministic financial imports, account discovery, and provider diagnostics. Keep login state encrypted, PINs transient, and incomplete provider responses non-destructive.
* Address Trade Republic review findings
Keep QR-authenticated sessions syncable, preserve historical holding snapshots, correct dividend direction, handle unpriced positions safely, localize repair feedback, and align provider controls with the design system.
* Add Trade Republic translations for supported locales
* Restore German Trade Republic account labels
* Resolve remaining Trade Republic review findings
* Resolve remaining Trade Republic review findings
* Address latest Trade Republic review feedback
* Refactor Trade Republic panel buttons to use DS::Button component and add integration tests
* Fix 100x money inflation and missing positions locale key in TR views
Money.new takes major units, so multiplying by 100 displayed EUR 12.34
as EUR 1234 in the holdings category cards and expense summary. Also
add the pluralized holdings.index.positions key that t(".positions")
resolves to (previously only defined at the unused holdings.positions
root level), across all 18 locales.
* fix(db): repair merge artifacts in schema and migrations
- Remove duplicated icon/progress_basis columns on goals in schema.rb
- Renumber Trade Republic migrations to unique versions (clashed with
main's 20260824120000_add_lifecycle_to_goals)
- Bump schema version to match latest migration
* Address remaining Trade Republic review feedback
* fix(trade-republic): address open PR #3168 review findings\n\n- Reject authenticated sessions without a securities account number so a\n blank account does not mark the item connected on a broken session.\n- Derive a missing trade amount from |quantity| x price, and a missing\n price from the resolved amount, without changing the signed import amount.\n- Regenerate db/schema.rb so the Trade Republic item/account tables and\n indexes are present; a fresh test database was otherwise missing the\n tables even though the migrations were marked up.\n
* fix(trade-republic): localize activity labels in ActivitiesProcessor (i18n)
* fix(trade-republic): localize activity labels in ActivitiesProcessor (i18n)
* fix(trade-republic): add activity labels i18n keys to all locale files
* Fix Trade Republic PR review follow-ups
* fix(trade-republic): i18n-aware category guard and ignore generated graphify cache
- Category matcher skipped core deposit/withdrawal labels; guard now compares
against translated values so German etc skip correctly
- Remove committed graphify-out cache and ignore dir
* Protect holdings from malformed snapshots
* Consolidate Trade Republic migrations
* Address final Trade Republic review comments
* Address final Trade Republic review comments
- Remove hard-coded category matcher (merchant keyword taxonomy) and leave Trade Republic transactions uncategorized when no structured category exists; rely on Sure rules/AI
- Revert shared ProviderImportAdapter# import_trade extra: param; handle Trade Republic trade metadata locally in ActivitiesProcessor via post-import Trade extra merge (preserve existing extra, deep_merge)
- Preserve Trade Republic product distinctions (cash, brokerage/private_markets/interest_products/crypto_wallet via portfolio categories) without collapsing account kinds
---------
Co-authored-by: Aland Baban <snow@iBananaMac.fritz.box>
* fix(hosting): consistent provider-block visibility + fix Twelve Data toggle bug (#3089)
T-Invest was the only provider block always rendered regardless of its
checkbox state; now it follows the same pattern as every other provider
(shown when tinkoff_invest or moex_public is enabled, since T-Invest also
serves as a brand-logo fallback for MOEX-priced securities).
Also fixes a related functional bug: unchecking every securities provider
tried to clear the legacy securities_provider setting by assigning nil,
but rails-settings-cached treats nil as "delete override", which silently
reverted the field to its own default ("twelve_data") — re-enabling Twelve
Data right after the user disabled it. Assigning "" instead persists the
cleared state.
Twelve Data and Yahoo Finance settings blocks can still be shown purely
because they're the selected FX/exchange-rate provider even when unchecked
for securities pricing; added an info notice explaining that instead of
leaving it unexplained.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(hosting): address review feedback on #3333
- Drop the FX-only notice for Twelve Data/Yahoo Finance per feedback —
the block staying visible while unchecked (because it's still the FX
provider) doesn't need extra UI explanation.
- Fix Codex finding: T-Invest settings must stay visible/manageable
whenever a token is already configured, not just when tinkoff_invest
or moex_public is checked. Security::Provided#import_brand_logo calls
the T-Invest provider unconditionally for every non-crypto security
once a token exists, regardless of price provider — hiding the field
in that case would leave an active credential impossible to see,
rotate, or clear through the UI. Reworded the notice to reflect the
real, provider-independent reason instead of the narrower "MOEX only"
framing.
- Fix CodeRabbit finding: setting_test.rb's default-fallback test now
isolates against SECURITIES_PROVIDER(S) env vars, and the
explicit-clear test captures and restores the pre-test values instead
of hardcoding a restore target.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* No overexplaining in code
---------
Co-authored-by: Gerald <248542187+gfr-free@users.noreply.github.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Juan José Mata <jjmata@jjmata.com>
Translate the remaining provider-owned setup and status copy while reusing Accountable subtype labels before legacy English fallbacks. Cover German rendering, error handling, and pluralized English and German status summaries.
Co-authored-by: Johns <19662585+Rowdy@users.noreply.github.com>
* feat(bills): assistant and MCP tools for bills
Last of three chunks carved out of #3083, stacked on the UI bundle. Exposes
bills to the builtin assistant and to MCP clients. Everything here is gated
behind preview features, so the tools are absent from tools/list until a user
opts in.
Seven tools:
- get_bills, get_bill_details and get_paycheck_plan for reads
- get_bill_audit, a deterministic review that surfaces likely duplicates, price
changes, trials about to convert, upcoming renewals and long-overdue bills
- create_bill, update_bill and record_bill_payment for writes
Shared argument parsing, permission checks and error shapes live in
BillsSupport, so every tool answers with the same {error, hint} contract the
existing tools use, and a bad argument never aborts the turn.
The write tools mutate financial records on a model's say-so, so they refuse
rather than guess: a payment cannot exceed what its cycle still owes, a repeated
settle will not quietly close next month, an unrecognized frequency is an error
instead of a silent monthly default, and non-finite or negative amounts are
rejected before they reach the database.
The read tools say what they filtered. An empty result names the statuses that
do hold matches, the paycheck plan discloses the unconfirmed series it excluded
from spending headroom, and history and price-change windows report their real
totals rather than letting a caller sum a truncated list.
A not-found no longer returns the scoped relation's SQL, which handed any MCP
client the access-control schema for the cost of a guessed id.
The in-page AI helpers are not here. Smart fill and smart configuration are
buttons on the bills pages, so they ship with the UI bundle along with the
provider-side suggester they call.
Suite 7,854 runs, 0 failures. Rubocop clean, eager loading verified.
* Address the ready-review round
* Reject an out-of-range audit lookback out loud
* Speak the cycle remainder guard through the allocator locale
* feat(rules): add not-equal, does-not-contain, is-not-empty condition operators
Extend transaction rule conditions beyond "equal to" / "is empty":
- text: add "does not contain" (not_like), "not equal to" (!=), "is not empty" (is_not_null)
- number: add "not equal to" (!=)
- select: add "not equal to" (!=), "is not empty" (is_not_null)
NULL handling is inclusive so the operators match user intent:
- "!=" uses IS DISTINCT FROM, so e.g. "category not equal to X" also matches
uncategorized (NULL) transactions
- "does not contain" also matches rows where the field is NULL
transaction_type keeps its custom operator set, and transaction_details is
pinned to the original operators since its JSONB apply only supports
contains/equals/empty semantics.
The conditions Stimulus controller hides the value field for both valueless
operators (is_null and is_not_null).
* refactor(rules): address PR review feedback on condition operators
- Pass VALUELESS_OPERATORS from Ruby to JS via Stimulus value attribute
instead of duplicating the list as a static class property, so there
is a single source of truth for which operators suppress the value field
- Clarify IS DISTINCT FROM comment to note the NULL-inclusion behaviour
is intentional for select-type fields (merchant_id, category_id) and
not applicable to number fields where NULL is impossible at the DB level
- Add test that exercises the OR IS NULL branch of not_like by using
transaction_notes (entries.notes is nullable, unlike entries.name)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* refactor(rules): localize condition operator labels via i18n
Moves all Rule::ConditionFilter operator labels (including ones that
predate this PR) out of OPERATORS_MAP and into config/locales, so
operators() resolves them through t() per request instead of hardcoded
English strings.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GvxjdTgH34cPoJenQAqnpN
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Juan José Mata