kayg/calternal

Watch

Fork 0

Money: a delayed read replaces the acknowledged month snapshot #826

New issue

Open

opened 2026-10-02 13:20:39 +00:00 by kayg · 1 comment

kayg commented

2026-10-02 13:20:39 +00:00

Owner

Independent round 7b review for #427. Branch job/reload-423, head 2399db841c. Non-blocking: stale display and browser snapshot; no lost server write was shown.

Context: the branch keeps month reports in memory and per-User browser storage. A quiet read can be in flight when a User saves a change.

Evidence:

apps/web/src/lib/money/store.svelte.ts:125-141: refreshMonth captures only the session generation and publishes the response through rememberMonth.
store.svelte.ts:146-157: rememberMonth stores the acknowledged report but does not fence an earlier read.
store.svelte.ts:245-257: refresh(budget) removes saved reports but neither advances their generation nor invalidates pending month requests.
apps/web/src/routes/money/[budget]/[month]/+page.svelte:121-126: a successful save calls refresh and rememberMonth with the new report. The quiet read at lines 81-83 can still publish its older response to the mounted view.

Reproduction: hold a read with an old report; acknowledge a write; run the route's refresh and rememberMonth steps with the new report; release the old read. The older response replaces both memory and browser storage. This was checked with the branch's store implementation, mocked transport and storage, and identity stubs for Svelte state runes. No real User data or live server was used.

Expected: fence responses issued before a committed mutation, per report or Budget. Keep the acknowledged report in both the mounted view and stored snapshot.

Regression test idea: delay a report GET, commit an edit, return its new report, then release the old GET. Assert that the mounted report, cachedMonth and stored snapshot retain the new revision.

Duplicate check: searched all issue titles and snapshot/cache terms, including #423 and #673. They describe reload retention and future shared contracts, not this late-response mutation race.

Independent round 7b review for #427. Branch job/reload-423, head 2399db841cf16ade3fbf47fcfab50578a3abe364. Non-blocking: stale display and browser snapshot; no lost server write was shown. Context: the branch keeps month reports in memory and per-User browser storage. A quiet read can be in flight when a User saves a change. Evidence: - apps/web/src/lib/money/store.svelte.ts:125-141: refreshMonth captures only the session generation and publishes the response through rememberMonth. - store.svelte.ts:146-157: rememberMonth stores the acknowledged report but does not fence an earlier read. - store.svelte.ts:245-257: refresh(budget) removes saved reports but neither advances their generation nor invalidates pending month requests. - apps/web/src/routes/money/[budget]/[month]/+page.svelte:121-126: a successful save calls refresh and rememberMonth with the new report. The quiet read at lines 81-83 can still publish its older response to the mounted view. Reproduction: hold a read with an old report; acknowledge a write; run the route's refresh and rememberMonth steps with the new report; release the old read. The older response replaces both memory and browser storage. This was checked with the branch's store implementation, mocked transport and storage, and identity stubs for Svelte state runes. No real User data or live server was used. Expected: fence responses issued before a committed mutation, per report or Budget. Keep the acknowledged report in both the mounted view and stored snapshot. Regression test idea: delay a report GET, commit an edit, return its new report, then release the old GET. Assert that the mounted report, cachedMonth and stored snapshot retain the new revision. Duplicate check: searched all issue titles and snapshot/cache terms, including #423 and #673. They describe reload retention and future shared contracts, not this late-response mutation race.

kayg referenced this issue

2026-10-02 13:22:08 +00:00

Log attachments: deleted file still shown, attachments not live, batch send not live, Log order, raw camera names #427

kayg referenced this issue

2026-10-02 13:36:18 +00:00

Log attachments: deleted file still shown, attachments not live, batch send not live, Log order, raw camera names #427

kayg referenced this issue

2026-10-02 13:40:44 +00:00

BLOCKER: retrying a Composer linked-body failure creates duplicate acknowledged Logs #844

kayg referenced this issue

2026-10-02 14:17:45 +00:00

BLOCKER: retrying a Composer linked-body failure creates duplicate acknowledged Logs #844

kayg referenced this issue

2026-10-02 17:25:47 +00:00

BLOCKER: retrying a Composer linked-body failure creates duplicate acknowledged Logs #844

kayg commented

2026-10-03 06:53:24 +00:00

Author

Owner

Money break-the-numbers review: job/datafix2 @ `ff8e857c2` (#826)

Verdict: PASS (non-blocking findings below). No lost write, no wrong server number, no crash, no cross-User leak found.

Scope note

git diff origin/dev...job/datafix2 -- crates/calternal-money crates/plugins/money does not touch the #826 fix. calternal-money is unchanged. The plugin diff is the #491 MCP event producer (c47bf108c, 18861a42c): events.rs (bill_due scanner) and event emission in create_transaction. The #826 fence lives in apps/web/src/lib/money/store.svelte.ts, apps/web/src/lib/api/revision-cache.ts and the month route (9bb19d8d1, eb063850d). I reviewed both.

Gates run (detached worktree, no branch changes)

cargo test -p calternal-money -p calternal-plugin-money: test result: ok. 15 passed, ok. 11 passed, ok. 2 passed, ok. 24 passed, EXIT 0.
vitest run src/lib/money/acknowledged-month.svelte.test.ts + my probe review-826-probe.svelte.test.ts: Tests 2 failed | 8 passed (10). The 2 failures are probes A2 and A3 below (they show the defects). All branch tests pass.

Attacks

#	Attack	Expected	Actual	Result
1	Late GET for the same month after an acknowledged assign (the #826 repro)	Acknowledged report stays in return value, memory, storage	Fenced: `RevisionCacheSupersededError`; property test over every safe minor-unit value passes	OK
A1	Back-dated assign in Aug while a Sep GET is in flight and Sep is stored	Sep GET fenced, Sep snapshot dropped (Sep totals derive from Aug)	`refresh(budget)` calls `invalidateVariant('money:month')` and removes all of the Budget's stored months; Sep GET rejects as superseded (probe A1 passes)	OK
2	Late 404/403 after an acknowledged write	Must not erase the acknowledged copy	Fenced (branch test)	OK
3	Transaction write on the accounts page (transfers, tracking accounts, splits)	In-flight month GETs fenced	Accounts page calls `moneyStore.refresh`, which fences every month read	OK
4	Session/access/plugin change during a GET	Abort + clear	Store listener clears before the route's `onAccess` reloads (module listener registered first)	OK
A2	Category create while a background revalidate of the same month is in flight	Re-read after the commit returns the new report	`refreshMonth` uses a fixed revalidate key `'refresh'`; `RevisionCache.revalidate` returns the existing pending promise, so `load(…, true)` after `AddCategoryForm.oncreated` joins the pre-write GET and gets the old report (no new category), then stores it. Probe: `{old:1, forced:1, calls:1}`, expected `{forced:99, calls:2}`. The second `oncreated` (line 252) does a non-forced `load`, which paints the cached pre-create report and then does `goto` to the new category anchor that does not exist yet. Category create also never calls `moneyStore.refresh(budget)`, so other months of the Budget keep the pre-create snapshot until they revalidate.	Defect, non-blocking (stale display, self-heals on next read; same class as #826)
A3	Two assigns in different cells; server commits 1 then 2; responses arrive 2 then 1	Keep the report of write 2	`rememberMonth` uses a random UUID revision. The last arrival wins: report 1 replaces report 2 in the mounted view, memory and storage. Probe: got 1, expected 2. `AssignedCell` serializes saves per cell only. This was already true of the mounted view before the branch; the branch now also persists it.	Defect, non-blocking (stale until next revalidate; needs a server month revision/ETag to order acks)
5	Ready to Assign / overspend carry-over / currency rounding / negative zero	Server math unchanged	`calternal-money` not touched; amounts stay `i64` minor units (no negative zero); event amounts are `amount.get()` exact minor units	OK
6	`money.bill_due` with hostile `days` (overflow panic in `Duration::days` / `NaiveDate + Duration`)	Rejected	`valid_arguments` bounds `days` to `u64 <= 365` before an interest exists	OK
7	Bill scanner timezone month edges	Due date in the User's zone	Uses `resolve_zone` + `today(zone)`; target month derived from the target date, so Dec 31 + 1 day reads January	OK
8	Bill scanner repeats every 30 s	No duplicate deliveries	Stable `event_id` (budget, bill block, date, days); server dedupes on `sub.seen`	OK
9	Overspend event on create	Emit once on crossing >= 0 to < 0, after the checked write	Before/after use the pre-write `ledger` and the validated `check` projection, emitted after `store_month`. Notes: (a) for a back-dated transaction it measures the transaction month, not the current month, so it can announce an overspend in a closed month that carry-over already reset, or miss a crossing in the current month; (b) only `create_transaction` emits, edits/deletes that cross into overspend do not; (c) two extra full replays per create (perf, non-gating); (d) no test covers `money.transaction_logged` or `money.category_overspent` emission.	Non-blocking, file follow-ups
10	Concurrent edits during a rebuild	Server rejects stale writes	Unchanged `replace_if` hash path (409); scanner holds the per-User read lock	OK

Suggested fixes (non-blocking)

A2: in both AddCategoryForm.oncreated handlers call moneyStore.refresh(data.budget) before load(…, true) (this fences the in-flight GET), and use the forced load in the second handler so the anchor exists before goto. Regression test: probe A2 above.
A3: return a server month revision (hash of the month files) on GET and PUT, and let rememberMonth/refreshMonth drop a report older than the current one.
Event 9a/9b/9d: issue against #491.

Probe file (review worktree only, not on the branch): apps/web/src/lib/money/review-826-probe.svelte.test.ts in /home/kayg/Developer/calternal-wt/rev-datafix2.

## Money break-the-numbers review: job/datafix2 @ ff8e857c2 (#826) **Verdict: PASS (non-blocking findings below).** No lost write, no wrong server number, no crash, no cross-User leak found. ### Scope note `git diff origin/dev...job/datafix2 -- crates/calternal-money crates/plugins/money` does **not** touch the #826 fix. `calternal-money` is unchanged. The plugin diff is the #491 MCP event producer (c47bf108c, 18861a42c): `events.rs` (bill_due scanner) and event emission in `create_transaction`. The #826 fence lives in `apps/web/src/lib/money/store.svelte.ts`, `apps/web/src/lib/api/revision-cache.ts` and the month route (9bb19d8d1, eb063850d). I reviewed both. ### Gates run (detached worktree, no branch changes) - `cargo test -p calternal-money -p calternal-plugin-money`: `test result: ok. 15 passed`, `ok. 11 passed`, `ok. 2 passed`, `ok. 24 passed`, `EXIT 0`. - `vitest run src/lib/money/acknowledged-month.svelte.test.ts` + my probe `review-826-probe.svelte.test.ts`: `Tests 2 failed | 8 passed (10)`. The 2 failures are probes A2 and A3 below (they show the defects). All branch tests pass. ### Attacks | # | Attack | Expected | Actual | Result | |---|---|---|---|---| | 1 | Late GET for the same month after an acknowledged assign (the #826 repro) | Acknowledged report stays in return value, memory, storage | Fenced: `RevisionCacheSupersededError`; property test over every safe minor-unit value passes | OK | | A1 | Back-dated assign in Aug while a Sep GET is in flight and Sep is stored | Sep GET fenced, Sep snapshot dropped (Sep totals derive from Aug) | `refresh(budget)` calls `invalidateVariant('money:month')` and removes all of the Budget's stored months; Sep GET rejects as superseded (probe A1 passes) | OK | | 2 | Late 404/403 after an acknowledged write | Must not erase the acknowledged copy | Fenced (branch test) | OK | | 3 | Transaction write on the accounts page (transfers, tracking accounts, splits) | In-flight month GETs fenced | Accounts page calls `moneyStore.refresh`, which fences every month read | OK | | 4 | Session/access/plugin change during a GET | Abort + clear | Store listener clears before the route's `onAccess` reloads (module listener registered first) | OK | | A2 | **Category create while a background revalidate of the same month is in flight** | Re-read after the commit returns the new report | `refreshMonth` uses a fixed revalidate key `'refresh'`; `RevisionCache.revalidate` returns the existing pending promise, so `load(…, true)` after `AddCategoryForm.oncreated` **joins the pre-write GET** and gets the old report (no new category), then stores it. Probe: `{old:1, forced:1, calls:1}`, expected `{forced:99, calls:2}`. The second `oncreated` (line 252) does a non-forced `load`, which paints the cached pre-create report and then does `goto` to the new category anchor that does not exist yet. Category create also never calls `moneyStore.refresh(budget)`, so other months of the Budget keep the pre-create snapshot until they revalidate. | **Defect, non-blocking** (stale display, self-heals on next read; same class as #826) | | A3 | Two assigns in different cells; server commits 1 then 2; responses arrive 2 then 1 | Keep the report of write 2 | `rememberMonth` uses a random UUID revision. The last arrival wins: report 1 replaces report 2 in the mounted view, memory and storage. Probe: got 1, expected 2. `AssignedCell` serializes saves per cell only. This was already true of the mounted view before the branch; the branch now also persists it. | **Defect, non-blocking** (stale until next revalidate; needs a server month revision/ETag to order acks) | | 5 | Ready to Assign / overspend carry-over / currency rounding / negative zero | Server math unchanged | `calternal-money` not touched; amounts stay `i64` minor units (no negative zero); event amounts are `amount.get()` exact minor units | OK | | 6 | `money.bill_due` with hostile `days` (overflow panic in `Duration::days` / `NaiveDate + Duration`) | Rejected | `valid_arguments` bounds `days` to `u64 <= 365` before an interest exists | OK | | 7 | Bill scanner timezone month edges | Due date in the User's zone | Uses `resolve_zone` + `today(zone)`; target month derived from the target date, so Dec 31 + 1 day reads January | OK | | 8 | Bill scanner repeats every 30 s | No duplicate deliveries | Stable `event_id` (budget, bill block, date, days); server dedupes on `sub.seen` | OK | | 9 | Overspend event on create | Emit once on crossing >= 0 to < 0, after the checked write | Before/after use the pre-write `ledger` and the validated `check` projection, emitted after `store_month`. Notes: (a) for a **back-dated** transaction it measures the transaction month, not the current month, so it can announce an overspend in a closed month that carry-over already reset, or miss a crossing in the current month; (b) only `create_transaction` emits, edits/deletes that cross into overspend do not; (c) two extra full replays per create (perf, non-gating); (d) no test covers `money.transaction_logged` or `money.category_overspent` emission. | Non-blocking, file follow-ups | | 10 | Concurrent edits during a rebuild | Server rejects stale writes | Unchanged `replace_if` hash path (409); scanner holds the per-User read lock | OK | ### Suggested fixes (non-blocking) - A2: in both `AddCategoryForm.oncreated` handlers call `moneyStore.refresh(data.budget)` before `load(…, true)` (this fences the in-flight GET), and use the forced load in the second handler so the anchor exists before `goto`. Regression test: probe A2 above. - A3: return a server month revision (hash of the month files) on GET and PUT, and let `rememberMonth`/`refreshMonth` drop a report older than the current one. - Event 9a/9b/9d: issue against #491. Probe file (review worktree only, not on the branch): `apps/web/src/lib/money/review-826-probe.svelte.test.ts` in `/home/kayg/Developer/calternal-wt/rev-datafix2`.

kayg referenced this issue

2026-10-03 06:53:55 +00:00

Money: month view ordering after category create and concurrent assigns; overspend event gaps (#826 review) #984

kayg referenced this issue

2026-10-03 15:41:26 +00:00

Money: month view ordering after category create and concurrent assigns; overspend event gaps (#826 review) #984

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

kayg/calternal#826

No description provided.

Rows
Columns