feat: implement the M4 review gate #67

Merged

TimCane merged 15 commits from feat/m4-review-gate into main

2026-07-06 10:30:18 +01:00

TimCane commented

2026-07-06 09:48:10 +01:00

(Migrated from github.com)

Summary

Item CRUD + bill endpoints, host-only and Review-only, with ISO 4217 currency validation (#30)
Host-only receipt image endpoint, 404 once the object is deleted at open (#31)
POST /open: mint the short code before the CAS, delete the image, return shortCode + joinUrl (#32)
Anonymous, rate-limited GET /codes/{shortCode} resolve (#33)
Review screen: item list, edit/add sheet, receipt sheet, self-rename, extras/total sheet, open-confirm (#34)
Checksum banner variants + dismissible parser warnings; warnings plumbed through the snapshot + docs (#35)
Share sheet with client-side QR, short code, copy-link, reopenable (#36)
/join code entry, hub-live Processing screen, non-host holding card (#37)

Test plan

dotnet build clean; BillSplitter.Tests 59 pass
Integration suites (review/receipt/open/code-resolve) run in CI (Docker/Testcontainers)
pnpm typecheck, pnpm lint, pnpm test (12) and pnpm build all green

Closes #30, closes #31, closes #32, closes #33, closes #34, closes #35, closes #36, closes #37, closes #29.

## Summary - Item CRUD + bill endpoints, host-only and Review-only, with ISO 4217 currency validation (#30) - Host-only receipt image endpoint, 404 once the object is deleted at open (#31) - `POST /open`: mint the short code before the CAS, delete the image, return `shortCode` + `joinUrl` (#32) - Anonymous, rate-limited `GET /codes/{shortCode}` resolve (#33) - Review screen: item list, edit/add sheet, receipt sheet, self-rename, extras/total sheet, open-confirm (#34) - Checksum banner variants + dismissible parser warnings; warnings plumbed through the snapshot + docs (#35) - Share sheet with client-side QR, short code, copy-link, reopenable (#36) - `/join` code entry, hub-live Processing screen, non-host holding card (#37) ## Test plan - `dotnet build` clean; `BillSplitter.Tests` 59 pass - Integration suites (review/receipt/open/code-resolve) run in CI (Docker/Testcontainers) - `pnpm typecheck`, `pnpm lint`, `pnpm test` (12) and `pnpm build` all green Closes #30, closes #31, closes #32, closes #33, closes #34, closes #35, closes #36, closes #37, closes #29.

TimCane commented

2026-07-06 10:24:36 +01:00

(Migrated from github.com)

Code review fixes

Ran a review over this branch and pushed fixes for the issues it surfaced (5 commits).

Correctness

Money scaled per-currency, but the server stores hundredths. money.ts/moneyInput.ts used each currency's ISO fraction digits, while the backend (ReceiptParser.ToMinor) always multiplies by 100. Any non-2-decimal currency a host selects (e.g. JPY, in the seed set) displayed and parsed 100x wrong, and switching currency in the bill sheet silently rescaled every amount. Now fixed at 100 to match the server; also caches one Intl formatter per currency.
Orphan short code on a rejected open. OpenAsync minted and persisted the code before Session.Open's state/host guards ran, so a double-tap/retry or wrong-state open left a second key that still resolved to the session for its whole TTL. The mint is now rolled back if the transition throws.
A receipt-delete hiccup stranded the host. In POST /open the transition committed, then a failed storage.DeleteAsync 500'd the request with no SnapshotUpdated push - and the retry 409'd, leaving the host stuck on Review though the split was open. Delete is now best-effort (the MinIO lifecycle rule expires the object within a day).
Host-only OCR detail leaked to joiners. warnings and failureReason are marked host-only but ride the shared snapshot that joining participants read once the split is open. They're now cleared at the open gate, matching how the receipt image is dropped.

UX

Join-by-code reported a 429 as a wrong code. A throttled resolve told the user their (possibly correct) code didn't match, so they kept retrying into the limiter. Now shows rate-limit copy on a 429.

Verification

Frontend: tsc + vitest green (added a 429 test).
Backend: dotnet build clean, 59 domain unit tests pass. Integration tests need Docker (Testcontainers), unavailable in this environment - not exercised.

## Code review fixes Ran a review over this branch and pushed fixes for the issues it surfaced (5 commits). ### Correctness - **Money scaled per-currency, but the server stores hundredths.** `money.ts`/`moneyInput.ts` used each currency's ISO fraction digits, while the backend (`ReceiptParser.ToMinor`) always multiplies by 100. Any non-2-decimal currency a host selects (e.g. JPY, in the seed set) displayed and parsed 100x wrong, and switching currency in the bill sheet silently rescaled every amount. Now fixed at 100 to match the server; also caches one `Intl` formatter per currency. - **Orphan short code on a rejected open.** `OpenAsync` minted and persisted the code before `Session.Open`'s state/host guards ran, so a double-tap/retry or wrong-state open left a second key that still resolved to the session for its whole TTL. The mint is now rolled back if the transition throws. - **A receipt-delete hiccup stranded the host.** In `POST /open` the transition committed, then a failed `storage.DeleteAsync` 500'd the request with no `SnapshotUpdated` push - and the retry 409'd, leaving the host stuck on Review though the split was open. Delete is now best-effort (the MinIO lifecycle rule expires the object within a day). - **Host-only OCR detail leaked to joiners.** `warnings` and `failureReason` are marked host-only but ride the shared snapshot that joining participants read once the split is open. They're now cleared at the open gate, matching how the receipt image is dropped. ### UX - **Join-by-code reported a 429 as a wrong code.** A throttled resolve told the user their (possibly correct) code didn't match, so they kept retrying into the limiter. Now shows rate-limit copy on a 429. ### Verification - Frontend: `tsc` + `vitest` green (added a 429 test). - Backend: `dotnet build` clean, 59 domain unit tests pass. Integration tests need Docker (Testcontainers), unavailable in this environment - not exercised.