docs: record the second review round and refresh the reviewer memory

.claude/agent-memory/code-reviewer/MEMORY.md

@@ -1,4 +1,4 @@
-- [Anti-cheat invariant and its known bypass](project-anti-cheat-invariant.md) — pano coords are the answers; debug API now prod-gated, preview envs and sessionId reuse still open.
+- [Anti-cheat invariant and its known bypass](project-anti-cheat-invariant.md) — pano coords are the answers; debug API now prod-gated, session ids server-minted, preview envs still open.
 - [Quality gate blind spots](project-quality-gates-blind-spots.md) — no-undef is now on, but no gate checks React prop/state contracts, e2e stub drift, or whether /docs matches the code.
 - [Scoring ladder vs leaderboard boards](project-scoring-ladder-and-boards.md) — one frozen ladder again, and the top-200 trim permanently resets anyone outside the window.
 - [Free-tier budgets are the real ceiling](project-free-tier-budgets.md) — ~27 Redis commands per round against 500K/month, and nothing counts them.

.claude/agent-memory/code-reviewer/project-anti-cheat-invariant.md

@@ -61,6 +61,10 @@ shared link), check what it credits before accepting a client-side attempt limit
 Update 2026-09-28: `lib/daily.js` IS now on the `FORBIDDEN` list, and both
 `/api/debug/*` routes are gated by `src/lib/debug-access.js` (404 in production
 without `DEBUG_ACCESS_KEY`; always open on Vercel preview and dev). The remaining
-open questions are whether preview deployments share production Redis/Neon, and
-that `/api/new-game` still reuses a client-supplied `sessionId` (already caused
-one skip-DEL race, patched client-side only).
+open question is whether preview deployments share production Redis/Neon.
+
+Update 2026-09-29: `/api/new-game` and `/api/daily` both mint the session id
+server-side (`crypto.randomUUID()`); the client can no longer name one. The
+debug key is compared in constant time. The daily re-picks only on an
+`UpstreamError` with code `'gone'` (Mapillary 400/404 or no thumbnail);
+every other failure is thrown so the day's panorama never changes on a blip.

plans/reports/code-review-260929-1940-backend-api.md

@@ -0,0 +1,230 @@
+# Backend/API review: `dev` at cca6018 (2026-09-29)
+
+Read-only review of the API routes, the server libraries, the scripts and the
+CI config. It covers the commits since the whole-project review
+(`77654d2..HEAD`: 04925cf, db8d799, 4d25a82, b8b4ffb, cca6018). Nothing listed
+in that review's Resolution section is reported again.
+
+## Scope
+- Routes: `src/app/api/{daily,guess,leaderboard,new-game,skip}/route.js`, `debug/{pano,region-coverage}/route.js`
+- Libs: session, game, leaderboard, upstash, pano-index, pano-db, daily, daily-calendar, daily-progress,
+  pano-history, mapillary, errors, region-request, region-locate, debug-access, stats, cookies, player-id, username
+- Scripts: `scripts/*.mjs`, `scripts/lib/*.mjs`. Config: `next.config.mjs`, `docker-compose.yml`, `.github/workflows/*`
+- Gates run: `npm run lint` passes with 0 problems. `npm test` passes 33 files and 409 tests in 25 s.
+
+## Overall assessment
+The core invariants hold, and I traced each one in source:
+- The session is claimed with DEL before any write (`guess/route.js:125`).
+- Every round mints a fresh session id (`new-game/route.js:98`, `daily/route.js:26`).
+- UUIDs are checked before an id reaches the keyspace (`guess/route.js:70`, `skip/route.js:13`).
+- The district is not written to logs or responses before the guess.
+- The daily round credits no board (`guess/route.js:138-155`).
+- The debug routes are closed in production (`debug-access.js:21-32`).
+
+What remains is concentrated in one place: the daily's rule for "the image is
+gone" is defined by elimination, so it can still move the day to a second
+panorama. That is the failure 04925cf set out to close. Behind it are two
+resilience gaps in the backing-store clients and a few smaller convention and
+dead-code items.
+
+## Findings, highest severity first
+
+### 1. MEDIUM, CONFIRMED (code path): the daily can still move to a second panorama after a single one-off failure
+- **Where:**
+  - `src/lib/daily.js:37-39` (`imageIsGone`), `daily.js:52-60` (cached path), `daily.js:68-86` (draw loop)
+  - `src/lib/mapillary.js:64` (the `response.json()` call has no error wrapper) and `:77` (plain `Error`)
+- **Cause:** `imageIsGone(error)` is defined as "not auth and not transient". Anything that is not a 401, a
+  timeout, a network error, a 5xx or a 429 therefore counts as proof the image was deleted. That includes:
+  - any other 4xx, for example 400 or 403;
+  - a 200 whose body is not JSON, such as an HTML error page from a proxy. `response.json()` then throws a
+    `SyntaxError`, not an `UpstreamError`;
+  - the plain `Error('image has no usable thumbnail')`;
+  - Redis errors from `putJsonIfAbsent` or `getJson` at `daily.js:78-81`. They sit inside the same `try`, so
+    a Redis blip is logged as "candidate failed" and the loop moves on to another seed.
+- **Failure scenario:**
+  1. The cached pick is `c0`, which is candidate 0.
+  2. One request gets a one-off 400, or a non-JSON 200, for `c0`.
+  3. `daily.js:59` runs `DEL daily:{day}`, and `skip` becomes `{c0}`.
+  4. Attempt 0 is skipped. Attempt 1 draws `c1`, Mapillary answers normally, and `SET NX` stores `c1`.
+  5. Everyone before that request played `c0` and everyone after plays `c1`, under the same Daily #N.
+
+  The test stub (`tests/daily-route.test.js:33-37`) only models a 404 ("gone") and a 503 ("flaky"), so no
+  test covers this path.
+- **Related (LOW):** the `del` at `daily.js:59` is unconditional. It can delete a pick that another instance
+  wrote a moment earlier. Today this only converges because the seeded draw is deterministic, and that
+  stops being true when warm instances hold different `countCache` values.
+- **Fix, kept small:**
+  - Require positive proof. Treat as gone only an `UpstreamError` with `code === 'http'` and a status of 400
+    or 404, plus the missing-thumbnail case once it is given its own type. Everything else should throw.
+  - Wrap `await response.json()` in `mapillary.js:64` so a parse failure becomes `UpstreamError('http', …)`.
+  - Move `putJsonIfAbsent` and `getJson` (`daily.js:78-81`) out of the `try`, so Redis errors propagate
+    instead of being classified.
+  - Add tests: a single 400 on the cached pick should keep the pick and return 502, and a non-JSON 200
+    should do the same.
+  - Which status Mapillary returns for a deleted image is PLAUSIBLE only. Graph-style APIs often answer 400
+    (code 100) rather than 404, so confirm it against the live API with one known-deleted id.
+
+### 2. MEDIUM, PLAUSIBLE: Redis and Neon clients have no timeout, and best-effort calls get the full retry budget
+- **Where:** `src/lib/upstash.js:42` (`new Redis({ url, token })`) and `src/lib/pano-db.js:28` (`neon(url)`).
+- **Cause:** `@upstash/redis` retries network errors 5 times, with a backoff of `Math.exp(n) * 50` ms
+  (per the Upstash docs). That adds about 4.3 s of backoff per failing command, and there is no request
+  timeout. Neon's HTTP driver sets no timeout either.
+- **Failure scenario:** during an Upstash network incident:
+  - `/api/new-game` spends about 4.3 s on the best-effort history GET (`recentPanoIdsOrNone`) before the
+    load-bearing session SET spends another 4.3 s and fails. That is roughly 9 s or more before a 500.
+  - `/api/guess` loses about 4.3 s on the session GET alone.
+  - A connection that hangs rather than resets has no cap. The platform kills the function, and the client
+    gets a raw 504 instead of the `{success:false}` envelope.
+- **Fix:**
+  - Pass `retry: { retries: 1, backoff: () => 100 }` to the Redis client.
+  - Pass a per-request `signal` if the installed 1.38.x supports it. I could not verify this because the
+    `node_modules` reads are hook-blocked.
+  - Give Neon `fetchOptions` with a timeout signal.
+  - Keep the timeout under the draw budget.
+
+### 3. LOW-MEDIUM, CONFIRMED: the draw budget does not limit the Mapillary request already in flight
+- **Where:** `src/lib/mapillary.js:22-25`, `:51`, `:185`.
+- **Cause:** each lookup gets a full `AbortSignal.timeout(5000)`, and the 8 s deadline is only checked after
+  an attempt fails.
+- **Failure scenario:**
+  1. Attempt 1 times out at 5 s. That is under 8 s, so attempt 2 starts with another 5 s.
+  2. The draw ends at about 10 s, before counting the Redis and Neon time spent earlier in the request.
+  3. The comment on `:23-24` claims a 10 s function limit. No route exports `maxDuration`, so the real
+     ceiling depends on project settings.
+- **Fix:** pass the remaining budget down, for example
+  `AbortSignal.timeout(Math.max(500, Math.min(REQUEST_TIMEOUT_MS, deadline - Date.now())))`, by adding a
+  timeout parameter to `fetchImage` and `fetchPanoramaById`.
+
+### 4. LOW-MEDIUM, CONFIRMED: Vietnamese usernames with combining tone marks are rejected
+- **Where:** `src/lib/username.js:46` and `:54`. This function is shared by the name modal and `/api/guess`.
+- **Verified with node:**
+  - `'Tiến'` in NFC passes.
+  - `'Tiến'` in NFD fails.
+  - `'Tiến'` (precomposed ê plus a combining acute) fails. That is the form the Windows Vietnamese
+    keyboard and some IMEs emit.
+
+  `\p{L}` does not match `\p{M}`.
+- **Failure scenario:** a player in a game about Vietnam types their accented name and is told it may only
+  contain letters.
+- **Fix:** use `const value = raw.normalize('NFC').trim();`. This has a side benefit: two visually identical
+  names can no longer become two board members.
+
+### 5. LOW, CONFIRMED (the Neon cost is PLAUSIBLE): every draw pays for a linear OFFSET scan
+- **Where:** `src/lib/pano-index.js:125-130`, `:155-160` and `:207-211`, with the index defined at
+  `scripts/lib/pano-schema.mjs:235-236`.
+- **Cause:** the `(province, id)` index removes the sort, but `OFFSET n` still walks n index entries. A Ha Noi
+  draw reads an average of about 113k entries, and up to 226k. Two of the five provinces are that size, and
+  a country round picks a province uniformly, so this is the common case on metered Neon compute. I did not
+  run EXPLAIN against Neon.
+- **Fix:** at seed time, write dense rank columns (`prov_rank`, `dist_rank`) with indexes
+  `(province, prov_rank)` and `(district, dist_rank)`. Each draw then becomes `WHERE province=$1 AND
+  prov_rank=$2`, which is O(log n). `pickPanoBySeed` uses the same lookup. The exclusion fallback at
+  `:146-164` can keep its OFFSET, because it is rare.
+
+### 6. LOW, CONFIRMED: two conventions for the same failure kinds
+- **Where:**
+  - `src/lib/mapillary.js:123-193` returns `{success, kind: 'dry'|'upstream'}`.
+  - `src/lib/daily.js:88` and `pano-index.js` throw `DryPoolError` or `UpstreamError`.
+  - The routes each map to statuses separately: `new-game/route.js:74-90` compares strings, and
+    `daily/route.js:51-55` uses `instanceof`. The comment in `daily/route.js` says "the same mapping as
+    /api/new-game".
+- **Conflict:** `docs/development.md` says "a library function throws" and "failure kinds are classes".
+- **Fix:**
+  - Have `fetchRegionPanorama` throw `DryPoolError`, or `UpstreamError('http', …)` for an exhausted budget.
+  - Export one `drawFailureStatus(error)` helper (404, 502 or 500) from `errors.js`.
+  - Use it in both routes.
+
+### 7. LOW, CONFIRMED: a stale comment invites someone to bring back the daily farm
+- **Where:** `src/app/api/daily/route.js:15-18` says the daily round "credits the boards once". The code does
+  the opposite (`guess/route.js:134-155`), as do `lib/daily.js:17-21` and `docs/features.md:198-202`.
+- **Risk:** a maintainer who believes this comment and "fixes" the guess route would reopen the known
+  +5-per-two-requests farm.
+- **Fix:** reword the comment to "scored and counted, never credited".
+
+### 8. LOW, PLAUSIBLE: a region code in the table is not checked against the deployed tree until after the claim
+- **Where:**
+  - `new-game/route.js:106` and `daily/route.js:31` store `regionCode` straight from the `panoramas`
+    row.
+  - `guess/route.js:125` consumes the session.
+  - `leaderboard.js:207` (`requireRegion`) and `publicRegion()` (`region-request.js:74`) then throw.
+- **Failure scenario:** the region tree is regenerated with a district renamed or split, and deployed before
+  the reseed. From then on, every round in that district loses its session and returns a 500, and nothing
+  can be retried. The seed gate (`pano-artifacts.mjs:302-306`) only checks the tree at seed time.
+- **Fix:** in the draw, treat `!isRegion(candidate.regionCode)` as a failed candidate, and log it loudly. That
+  way the problem surfaces before any session exists.
+
+### 9. LOW, CONFIRMED: the district-assignment gate runs after the files are already written
+- **Where:** `scripts/assign-pano-districts.mjs`:
+  - `:104` rewrites the artifacts and `:220` rewrites `counts.js`, both before the stranded-rate gate at
+    `:235`.
+  - A partial run exits at `:209`, before the gate runs at all.
+- **Effect:** a failed run leaves rewritten artifacts and a `counts.js` that can be committed, so client
+  playability is already updated. The seed's per-province gate (`pano-artifacts.mjs:297-300`) still refuses
+  the upload, so this is not a data-serving bug.
+- **Fix:** compute everything, run the gate, then write.
+
+### 10. LOW, CONFIRMED: dead code left from the previous review's list
+The previous review named these as dead surface, but they are missing from its Resolution section.
+- `countPanos`'s country branch (`pano-index.js:45-49`) is only reached from `tests/pano-index.test.js:50-51`.
+  No production caller passes `'VN'`.
+- `MAX_PER_CITY = Infinity` and its shuffle-and-cap branch (`build-pano-index.mjs:52` and `:215-223`) can
+  never run.
+
+### 11. LOW: hardening that is not a defect today
+- `debug-access.js:25` compares the debug key with `===`. Use `crypto.timingSafeEqual` over equal-length
+  buffers. The risk is small over the network, but the fix is two lines.
+- The Mapillary token is sent in the query string (`mapillary.js:46`, `build-pano-index.mjs:89`). The Graph
+  API also accepts an `Authorization: OAuth <token>` header, which keeps the token out of any URL an
+  intermediary or future fetch log records. Whether the tiles host accepts the header is PLAUSIBLE only.
+- The Nominatim `fetch` has no timeout (`build-region-boundaries.mjs:117`). A hung request stalls the
+  offline build indefinitely.
+- `session.js:19-22`, `:35-38` and `:55-58`, and `leaderboard.js:123-126`, log and then rethrow, and every
+  route logs the same error again. This is noise only.
+
+## Verified non-issues
+- **Session races:** DEL-claim before any write, and fresh ids on both round routes. The old sessionId-reuse
+  race is closed (`new-game/route.js:95-98`).
+- **Daily day boundary:** the offset is a fixed +7 h, and Vietnam has no DST. Rollover checked with node:
+  16:59:59Z gives 2026-09-29 and 17:00:00Z gives 2026-09-30. Stats stay on UTC days, which is documented.
+- **Concurrent daily picks:** `SET NX` plus the read-back of the winner (`daily.js:78-81`) converges.
+- **Answer exposure before the guess:** the round responses carry only `imageData.{url,isPano}` and the
+  picked region. The logs omit the district and the coordinates (`new-game/route.js:117-119`,
+  `guess/route.js:164-172`). The CDN-URL image id is an accepted risk, and I have not reported it.
+- **Input validation:**
+  - coordinates via `toCoordinate` plus finiteness and range checks;
+  - usernames by a pattern that excludes `:`;
+  - `limit` clamped in the library (`leaderboard.js:90`);
+  - the debug `id` must be numeric;
+  - the bbox must be finite.
+
+  With the debug id numeric and every other outbound host fixed, there is no SSRF path. No route trusts
+  `x-forwarded-*` or any other client header except the debug key.
+- **Cookies:** `httpOnly`, `sameSite=lax`, and `secure` in production. Duplicate cookies are filtered through
+  `isUuid`.
+- **Envelope:** every route returns `{success:false, error}` with a real status: 400, 404, 405, 500 or 502.
+- **Backup workflow:** an empty `KEY_PREFIX` secret falls back to the default (`upstash.js:47`). An empty
+  export fails the job (`export-leaderboards.mjs:25-28`). Both workflows run with `contents: read`.
+
+## Recommended actions (in order)
+1. Classify "gone" by positive proof in `daily.js`, wrap `response.json()`, move the Redis calls out of the
+   `try`, and add the one-off-400 and non-JSON tests (Finding 1).
+2. Set client timeouts and a small retry budget for Upstash and Neon (Finding 2), and pass the remaining
+   draw budget into the Mapillary fetch (Finding 3).
+3. Normalize usernames to NFC (Finding 4).
+4. Throw typed errors from `fetchRegionPanorama` and share one status mapper (Finding 6). Fix the stale
+   daily comment (Finding 7).
+5. Next time the pipeline is touched: rank columns for the draw (Finding 5), gate before write
+   (Finding 9), delete the dead branches (Finding 10), and reject unknown region codes at draw time
+   (Finding 8).
+
+## Metrics
+- Type coverage: JSDoc only, with no checker, by project policy.
+- Tests: 409 passing. The Mapillary stub models only 404 and 503 responses.
+- Lint: 0 problems.
+
+## Unresolved questions
+- What does Mapillary's Graph API return for a deleted image: 400 (code 100) or 404? Finding 1's fix depends
+  on the answer.
+- Do Vercel preview deployments share production Redis and Neon, and is Deployment Protection on? The debug
+  routes are open on preview by design (`debug-access.js:30`).
+- What `maxDuration` does the Vercel project actually run with? It decides how serious Findings 2 and 3 are.

plans/reports/code-review-260929-1940-frontend.md

@@ -0,0 +1,249 @@
+# Frontend review (2026-09-29, `dev` @ cca6018)
+
+Read-only review of the client side after the 2026-09-21 whole-project review
+and its Resolution. Items in that Resolution are not re-reported unless the fix
+is incomplete or introduced a new defect.
+
+## Scope
+
+- `src/app/{page,layout,not-found,game/*,daily,debug/*}.js`, `globals.css`
+- every file in `src/app/components/` (27 files, 3,477 lines) and `src/components/ui/`
+- the client libs: storage, use-stored-value, regions, theme, audio, share,
+  geo-search, map-tiles, username, last-region, first-round-hint,
+  daily-progress, daily-calendar, use-count-up, utils (`player-id.js` is
+  server-side and was checked only for client imports; there are none)
+- Gates: `npm run lint` 0 problems; `npm test` 33 files / 409 tests pass;
+  `npm run build:check` green (101 static pages).
+- Not checked at runtime: no browser on this host. Every finding below comes
+  from tracing source. Leaflet behaviour was checked against the Leaflet 1.9.4
+  source; Next 16 `Image` behaviour against nextjs.org (the bundled docs are
+  hook-blocked).
+
+## Overall
+
+The round-transition machinery (epoch refs, watchdog, prefetch, derived daily
+replay) holds up: I found no stale-epoch path that applies a superseded round.
+Client safety holds too. No client chunk in `.next-check/static/chunks`
+contains a server env name, the Neon driver or pano-index code. `/api/new-game`
+and `/api/daily` send the image URL (an accepted risk), `sessionId`, and the
+picked region, never the resolved district.
+
+The defects are elsewhere. The warning text added by the Bug 1 fix cannot be
+seen. The game cannot be played with a keyboard. The dialog role added to the
+expanded map misdescribes the page to screen readers. The client shows raw
+parser errors to players. And the previous review's bundle claim is wrong.
+
+## High
+
+### H1. The "partial" warning is invisible in both themes, and missing when every board fails. CONFIRMED
+`src/app/components/RoundResultDialog.js:340-344` renders the line as
+`text-warning-foreground` on `DialogContent`, which is `bg-background`
+(`src/components/ui/dialog.jsx:60`). That token is meant as the text colour
+on top of a `bg-warning` fill: it is `#ffffff` in light and `oklch(0.145 0 0)`
+in dark (`globals.css:132,202`), the same colour as `--background` in each
+theme. Measured contrast is 1.00:1 in both.
+The line also sits inside `hasLeaderboardSection && <details>` (`:261`), which is
+collapsed by default. If every score credit fails, `leaderboard.js:177-190`
+returns `levels: []`, `partial: true`, so `hasScoreLevels` is false. If the
+distance ranks also have no rank, the section and the warning are never
+rendered at all.
+**Scenario:** a Redis blip after the session claim. The player sees the normal
+success reveal, and the only qualifying sentence is either not rendered or
+drawn white on white.
+**Fix:** move the sentence out of `<details>` and render it whenever
+`result.partial` is true, under the distance badge. Use `text-foreground` with
+an `AlertTriangle` icon, or a `bg-warning text-warning-foreground` pill. Plain
+`text-warning` only reaches 3.7:1 in light.
+
+### H2. There is no keyboard path to place a guess. CONFIRMED
+`LeafletMap.js:87` places the pin only on Leaflet `click`. In Leaflet 1.9.4,
+`Map._fireDOMEvent` attaches `latlng` only to mouse events, and the
+keyboard handler only pans and zooms. `MapSearchBox.js:15-16` says in its
+header comment that a search result never places the pin.
+**Scenario:** a keyboard-only or switch user can tab to the map, pan it and
+zoom it, but can never place a guess. Submit stays disabled
+(`GameClient.js:584`) and the round cannot be played (WCAG 2.1.1).
+**Fix (minimal):** in `LeafletMap`, add a `keydown` listener on
+`map.getContainer()` that calls `emitMapClick(map.getCenter())` on Enter or
+Space while the container itself has focus. Add a centre crosshair and an sr-only
+hint ("Press Enter to guess at the map centre"). A search result can then be
+refined into a guess this way too.
+
+## Medium
+
+### M1. The expanded map declares `aria-modal` without being a modal. CONFIRMED
+`GuessMapPanel.js:89-91` sets `role="dialog" aria-modal="true"` whenever
+`expanded` is true. Two things are wrong:
+- No focus trap, and the next required control, Submit, lives outside the
+  panel (`GameClient.js:581-611`) while staying visible. VoiceOver honours
+  `aria-modal` and hides that action bar from swipe navigation.
+- `expanded` is not tied to the breakpoint. Expand on an iPad in portrait, then
+  rotate to landscape (768 to 1024px, crossing `lg`). The panel becomes the desktop
+  grid cell but keeps `role=dialog`, `aria-modal`, and the Escape listener
+  (`:43-56`). The only visible way to collapse it, the collapse button, is
+  `lg:hidden` (`:172`).
+**Fix:** drop `aria-modal` and keep a non-modal `role="dialog"` or `region`
+with a label. Alternatively, collapse on a `matchMedia('(min-width: 1024px)')`
+change in GameClient, and gate `role` on that same query.
+
+### M2. Parser errors reach the player as the error message. CONFIRMED
+`GameClient.js:67` calls `await response.json()` outside the `try` that maps
+network errors. A non-JSON response (a Vercel `FUNCTION_INVOCATION_TIMEOUT` or
+504 page, a Neon cold start past the function limit) throws a `SyntaxError`.
+`loadRound` then puts `error.message` into `loadError`, and `:521` renders it
+verbatim: "Unexpected token 'A', "An error o"... is not valid JSON".
+**Fix:** wrap the parse:
+`let data; try { data = await response.json(); } catch { throw new Error('The server could not start a round. Please try again.'); }`.
+`LeaderboardModal.js:45` has the same parse, but its message goes to the console,
+so it only needs the same guard for tidiness.
+
+### M3. Muted text on `vn-surface` fails AA, which the repo itself measured. CONFIRMED
+`NotFoundPanel.js:25-30` records `text-muted-foreground` on `.vn-surface` at
+4.05:1 on average and 3.09:1 at worst in light, and switches that panel to
+`text-foreground`. The same pairing is still used on the same surface elsewhere:
+- `page.js:145`: hero subtitle.
+- `page.js:201`: attribution line. `text-xs` at `/80` opacity, so lower still,
+  and it carries the Mapillary/OSM credits the licences require to be legible.
+- `GameClient.js:490`: region name on the first-load screen.
+
+**Fix:** use `text-foreground` (or `/80`) at these three sites, or put them on
+a `bg-card` chip.
+
+### M4. Leaderboard distance colours fail AA in light theme. CONFIRMED (computed)
+`LeaderboardList.js:11-16,98-100` sets `text-warning`, `text-success` or
+`text-danger` on a `secondary` badge (`oklch(0.97)`). The ratios are 3.70, 4.16
+and 4.37:1 at 18px bold, which is below the 18.66px large-text threshold, so
+4.5:1 applies. Dark theme passes (7.99:1 for warning).
+**Fix:** darker light-theme text variants (e.g. `oklch(0.45 …)`), or keep the
+number `text-foreground` and carry the grade in a small coloured dot.
+
+### M5. An old prefetch can hand the player a round that expires early. PLAUSIBLE
+`startPrefetch` (`GameClient.js:324-336`) mints the next session as soon as the
+result dialog opens. `handleNextRound` (`:420-428`) uses it no matter how long
+the dialog was left open. Session TTL is 30 minutes from mint.
+**Scenario:** the player leaves the result open for 25 minutes, presses Next
+Round, and plays for 6 minutes. The guess returns `session-expired`, and the
+dialog says "Rounds last 30 minutes" (`RoundResultDialog.js:18-21`) after 6.
+**Fix:** store `{ promise, at: Date.now() }` and ignore a prefetch older than
+about 20 minutes, falling through to `loadRound`.
+
+## Low
+
+- **L1. The count-up shows the old final score for one frame. CONFIRMED.**
+  `use-count-up.js:27,48`: `frame` survives dialog close because
+  `RoundResultDialog` stays mounted. When the next round has the same score,
+  the first open render returns `frame.shown === value`, then the first interval
+  tick drops to 0 and counts up again ("3, 0, 1, 2, 3"). Reset during render when
+  `!active && frame.target !== null`, or key the frame by an open counter.
+- **L2. ResultMap timers outlive the map. CONFIRMED (Leaflet source).**
+  `ResultMap.js:96-97` schedules `invalidateSize` at 100 and 500ms without
+  clearing them. `Map.remove()` never resets `_loaded` and deletes
+  `_mapPane`. Next Round or Menu within about 800ms of the dialog opening
+  therefore runs `_rawPanBy` on an undefined pane, an uncaught `TypeError`
+  (console or monitoring noise). Keep the ids and clear them in the cleanup.
+- **L3. Escape in the search box also collapses the map. CONFIRMED.**
+  `MapSearchBox.js:92-95` closes the list without stopping propagation, and
+  the window listener at `GuessMapPanel.js:46-53` collapses the map on the same
+  keypress. That is exactly the "one keypress, two things" the comment there
+  guards against for dialogs. Call `event.stopPropagation()` in the search
+  handler.
+- **L4. The region tree is still in the `/game` first-load chunk. CONFIRMED (build output).**
+  The Resolution's item 8 says making `MapSearchBox` dynamic took the region
+  tree off `/game/*`. It did not: `GameClient.js:19` and `RoundResultDialog.js:9`
+  import `regions.js` statically. The tree (~47-53 KB raw) is in chunk
+  `1m-8rm8olxirm.js`, which the client manifests of `/game/[region]` and
+  `/daily` list. It is small, so the real fix is optional: have
+  `game/[region]/page.js` pass `{ code, name, center, bbox, slug }` as props.
+  At minimum, correct the claim.
+- **L5. `memo(PanoramaViewer)` never skips a render.** `GameClient.js:538`
+  passes a fresh `<FirstRoundHint …/>` element each render, so every pin move and
+  every state change re-renders the viewer (`PanoramaViewer.js:146`). The effect
+  does not re-run, so this is only wasted work. Pass `hasGuess` down, or
+  `useMemo` the slot.
+- **L6. `panorama-error` never reaches `onError`.** `PanoramaViewer.js:67-71`
+  calls only `emitReady`, so `handlePanoramaError` (`GameClient.js:297-302`)
+  runs only for a constructor throw. An image that fails to load, such as an
+  expired CDN URL, plays no error sound and gives no signal. Call
+  `emitError(event)` there too.
+- **L7. Daily replay loads the full viewer behind a dialog that cannot be
+  dismissed.** `GameClient.js:138,532-539`: reopening `/daily` after playing
+  downloads the three.js chunk (638 KB raw), the panorama and the guess map, only
+  to sit under a `bg-black/50` scrim. The stored `imageUrl` may also have
+  expired by then (PLAUSIBLE). Render a static backdrop when `replay` is set.
+- **L8. `audio.js` bypasses `storage.js`. CONFIRMED.** Lines 54-79 read and
+  write `localStorage` directly. `docs/development.md` ("all reading through
+  `src/lib/storage.js`") and `storage.js`'s own header, which lists "sound",
+  both say otherwise. As a result, muting in one tab never reaches another tab
+  (no `storage` event), and the in-memory fallback is written twice. Move the
+  flags onto `readItem`/`writeItem`/`watchItem` and keep the in-memory cache.
+- **L9. `Image priority` is deprecated in Next 16.** `AppBackground.js:22`.
+  nextjs.org (v16.0.0 changelog) replaces it with `preload`. The image also
+  preloads on `/game`, where opaque panes cover it and it competes with the
+  panorama download. Use `fetchPriority="high"` and consider skipping it on
+  `/game`.
+- **L10. `AbortSignal.any` needs Safari 17.4.** `geo-search.js:159`. On
+  iOS 16.4-17.3 the call throws inside the `try` and returns `null`, so Photon
+  search always shows "unavailable". It degrades gracefully, but it is silent.
+  Feature-detect and fall back to the caller's signal plus a manual timer.
+- **L11. Type-button touch targets.** `RegionSelect.js:100` level buttons are
+  `h-9` (36px) in the home page's leaderboard dialog, where everything else
+  honours the 44px floor.
+- **L12. The theme re-apply in `game/[region]/not-found.js:31-35` looks redundant. PLAUSIBLE.**
+  `ThemeSync` (added in 30cfe63) mounts in the root layout. The file's own
+  comment says the error shell renders that layout on the client, so ThemeSync
+  already applies the theme and watches the OS. If so, the file can drop
+  `"use client"`. Confirm with a dark-theme visit to `/game/nope`.
+- **L13. `CoverageMap.js:38-40`: the one `react-hooks/refs` disable is avoidable.**
+  An effect event can read `panos` from props when Leaflet calls it
+  (`const nearest = useEffectEvent((latlng) => …panos…)`). The claim that
+  "data, not a callback, so an effect event does not fit" does not hold.
+  Debug-only.
+- Dead code: the `try/catch` around a non-awaited `fetch` in `handleSkipGuess`
+  (`GameClient.js:454-464`) cannot catch anything, and `handleRetryLoad` is a
+  bare alias (`:475`).
+
+## Scout: edge cases checked and found sound
+
+- A viewer `ready` arriving late from the outgoing round is filtered by
+  `appliedEpochRef` (`GameClient.js:204,293`).
+- A load released by the watchdog cannot inherit a pin (`:200`).
+- The daily first-load effect reads storage, not the hydration snapshot
+  (`:247`).
+- A replay cleared in another tab restarts the load through the `replay`
+  dependency.
+- The `getDailyProgress` parse cache keeps `useSyncExternalStore` from looping.
+- `LeafletMap` dependencies (`center`, `bbox`) are stable references into the
+  generated tree, including `VN` for the daily, so the map is not rebuilt per
+  render.
+- Every Radix dialog has a Title and Description. `RoundResultDialog`
+  announces the outcome once through its description.
+- Reduced motion covers spin, fade-in, dialog and accordion, and the count-up
+  checks the preference.
+- Hydration: every storage-backed value goes through `useStoredValue` with a
+  server value. `DailyCard` reads the day the same way. `InlineScript` is
+  covered by `suppressHydrationWarning`.
+- Listeners: every `watch*` returns an unsubscribe that is used. Leaflet (guess
+  and result) and PSV destroy on unmount. The one gap is L2.
+
+## Recommended order
+
+1. H1 (one component, and it finishes the Bug 1 fix).
+2. H2 (keyboard guess).
+3. M2 (JSON guard).
+4. M1 (drop `aria-modal`, collapse at `lg`).
+5. M3 and M4 (contrast tokens).
+6. M5, then the Low items as convenient. Correct the L4 claim in the
+   2026-09-21 synthesis.
+
+## Metrics
+
+- Tests: 409 pass. None render a component, so every finding above is runtime
+  or visual and outside what the gates can see.
+
+## Unresolved questions
+
+- H2: is a centre-crosshair keyboard guess acceptable UX, or should a keyboard
+  guess go through the search result plus a "guess here" action?
+- M1: should the phone minimap stay a modal? If yes, the action bar has to move
+  inside it.
+- L7: is the replay's panorama behind the dialog a deliberate backdrop?

plans/reports/synthesis-260929-1940-review-round-two.md

@@ -0,0 +1,134 @@
+# Review round two: findings applied (2026-09-29)
+
+Three independent read-only reviews of `dev` at cca6018, eight days after the
+whole-project review of 2026-09-21 whose items were all applied. Gates before
+and after: lint 0 problems, production compile green, tests 409 before and
+415 after.
+
+- [Backend and API](code-review-260929-1940-backend-api.md)
+- [Frontend](code-review-260929-1940-frontend.md)
+- [Test quality](tester-260929-1940-test-quality.md)
+
+## Verdict
+
+The load-bearing invariants hold: session ids are server-minted, a guess
+claims its session before any board write, the answer district never leaves
+the server before the guess, the daily credits no board, the debug routes are
+closed in production, and no client chunk carries server code. What the
+reviews found is one real correctness defect in the daily, a handful of
+accessibility and copy problems on the game screen, and a few hardening
+items. Everything below is applied on `dev`, uncommitted.
+
+## Applied
+
+### Backend
+
+- **The daily could switch panorama on a single non-transient failure.** The
+  rule was "anything that is not a timeout or 5xx proves the image is gone",
+  which included a 403 from a token missing a scope, a 200 whose body was an
+  HTML gateway page, a missing thumbnail, and a Redis error thrown inside the
+  same `try`. Any of those on the cached pick deleted `daily:{day}` and the
+  next candidate was stored for everyone after. Now `UpstreamError` carries a
+  `'gone'` code (Mapillary 400 or 404, or no thumbnail) and only that moves
+  the day; every other failure is thrown. The Redis calls sit outside the
+  retry. Tests: a 403 and a malformed answer keep the pick; a 503 on the first
+  seeded candidate caches nothing and the same candidate is served once the
+  blip passes. The 403 test fails against the old code.
+- **The 8 s draw budget now caps the request in flight**, not only whether
+  there is a next attempt; two 5 s timeouts could reach 10 s before.
+- **The Mapillary token travels in an `Authorization` header**, not the query
+  string, so it stays out of logs that quote URLs. Test asserts both.
+- **Usernames are NFC-normalised** before validation: a tone mark typed as a
+  combining character was rejected, and would otherwise have made two board
+  members of one name. Test with a decomposed `ả`.
+- **The debug key is compared in constant time.**
+- **A panorama whose district the region tree does not know fails the draw**
+  before a session is written, rather than failing `/api/guess` after the
+  session is consumed. Test inserts such a row.
+- **Every Redis command and Neon query has a deadline** (5 s and 8 s). The
+  Upstash client retried a hung connection five times with exponential
+  backoff, past the browser's fifteen-second wait; the Neon client had no
+  limit at all. Checked against the installed clients: the Upstash `signal`
+  factory caps the whole call, retries included, and Neon takes
+  `fetchOptions.signal` per query. The platform is not the constraint:
+  Vercel Hobby on Fluid compute allows 300 s by default (docs, 2026-08-24),
+  so the "ten seconds" comment on the draw budget was stale and is corrected.
+- **Dead code removed**: the country branch of `countPanos` (only tests called
+  it) and the `MAX_PER_CITY = Infinity` cap in the index build.
+- The daily route's header comment said the daily "credits the boards once";
+  it credits none.
+
+### Frontend
+
+- **The "some boards could not be updated" line was invisible** (white on
+  white in light, near-black on near-black in dark) and inside a `<details>`
+  that is not rendered when every board write fails. It now renders whenever
+  the round is partial, outside the details, readable, with `role="status"`.
+- Keyboard guessing (Enter at the map centre) was applied and then reverted:
+  the maintainer wants the guess placed with the mouse.
+- The expanded phone minimap no longer claims `aria-modal` (it traps no
+  focus and the Submit bar stays usable).
+- A non-JSON reply (a gateway timeout page) shows a sentence, not the
+  parser's message.
+- **Contrast**: light `--success`, `--warning` and `--danger` darkened so the
+  distance badges clear 4.5:1 (were 3.70 to 4.37); the attribution and one
+  other muted line lost their `/80` opacity; light `--muted-foreground` moved
+  from L 0.54 to 0.50 for margin against the background art under the
+  translucent surface.
+- A prefetched next round older than 20 minutes is dropped for a fresh fetch
+  (its session expires at 30).
+- Two consecutive rounds with the same score no longer flash the old total
+  before counting up.
+- The result map's two resize timers are cleared on unmount.
+- The first Escape in the map search only closes the result list; the second
+  collapses the map.
+- `audio.js` reads and writes through `storage.js`.
+- A panorama that fails to render now reaches `onError`, so the error sound
+  plays and loading clears.
+- Dead `try/catch` around a non-awaited fetch and a bare handler alias removed.
+
+### Tests
+
+- Region-coverage boundary asserted as a GeoJSON Feature with a polygon, not
+  just truthy.
+- Seeded picks asserted to resolve to a known province or district across
+  twelve seeds.
+- Debug key: near-miss lengths rejected.
+
+### Docs and memory
+
+- `docs/development.md` and `docs/features.md` state the `'gone'` rule.
+- The code-reviewer agent memory no longer says `/api/new-game` reuses a
+  client session id.
+
+## Not applied, with reasons
+
+- **`OFFSET` scans on Neon** (backend 5): cost is plausible, not measured.
+- **`fetchRegionPanorama` returning `{success, kind}`** instead of throwing
+  (backend 6): a discriminated result is not string-matching; churn without a
+  behaviour change.
+- **Stranded-rate check ordering in `assign-pano-districts`** (backend 9): the
+  seed step still refuses a bad upload.
+- **`priority` to `preload` on `next/image`**: the bundled Next docs could not
+  be read to confirm the deprecation.
+- **Cross-tab sync of the mute flags**: would need a second subscription
+  layer and a test stub change for a nicety.
+- **A test for the backup script's empty-board exit**: the script runs at
+  import against real credentials; a harness for one `process.exit` is not
+  worth it.
+- **Frontend Lows** L7, L8, L10 to L13 (daily replay viewer behind the dialog,
+  `memo` never skipping, `AbortSignal.any` on old iOS, 36 px buttons in the
+  leaderboard dialog, a possibly redundant theme re-apply, the CoverageMap
+  ref disable). Listed in the frontend report.
+
+## Corrections to earlier records
+
+The 2026-09-21 Resolution says loading `MapSearchBox` on demand took the
+region tree off `/game`. The frontend reviewer checked the build: the tree is
+still in the `/game` and `/daily` first-load chunk. The split is optional;
+the claim was wrong.
+
+## Open questions for the maintainer
+
+1. Do preview deployments share the production Redis and Neon? The debug
+   routes are open on preview by design.

plans/reports/tester-260929-1940-test-quality.md

@@ -0,0 +1,117 @@
+# Test Quality Review: vngeoguessr (2026-09-29)
+
+**Branch**: dev | **Commit**: cca6018 | **Test Duration**: 21.38s | **Result**: 409 passed
+
+## Three Recent Commits: Behavior Change Coverage
+
+| Commit | Behavior Changed | Test Status | Evidence |
+|--------|---|---|---|
+| 04925cf | Daily pick written with SET NX (atomic first-writer-wins) | ✓ Covered | daily-route.test.js:102–107, `never overwrites a pick another instance cached first` |
+| 04925cf | /api/new-game mints fresh session id, rejects client-offered ones | ✓ Covered | new-game-route.test.js:199–209, `mints every session id itself and ignores one the client offers` |
+| db8d799 | Refused storage writes kept in memory for the visit | ✓ Covered | storage.test.js:26–42, `keeps a refused write for the rest of the visit` |
+| db8d799 | Pin/marker cleared between rounds (GameClient, LeafletMap) | ✗ Not tested | Client-side UI, requires DOM/browser; architecture has no unit test layer for components |
+| 4d25a82 | Test hook timeout raised 10s → 60s (PGlite startup on ARM) | ✓ Configured | vitest.config.mjs:10, hookTimeout 60_000ms |
+| 4d25a82 | export-leaderboards.mjs fails (exit 1) on empty boards | ✗ Not tested | Line 22–28 has logic, zero tests for script behavior |
+
+## Untested Load-Bearing Paths (Blast Radius Ranked)
+
+### 🔴 HIGH: Session/Score Integrity
+**export-leaderboards.mjs empty boards check** `scripts/export-leaderboards.mjs:22–28`
+- Returns no error when `scanKeys('leaderboard:*', 'distance:*')` is empty
+- CI backup workflow (`.github/workflows/leaderboard-backup.yml`) depends on fail-on-empty to detect misconfiguration
+- **Sketch**: tests/export-leaderboards.test.js — mock empty scanKeys, verify process.exit(1) called
+
+### 🟡 MEDIUM: Game Flow Correctness
+**locateRegion edge cases** `src/lib/region-locate.js`
+- Tests only Q7 (TPHCM-Q7) and HOANKIEM (HN-HOANKIEM), no boundary/province-with-no-districts
+- **Sketch**: tests/region-locate.test.js — add tests for points on district boundaries, unmapped provinces
+
+**stats.js EXPIRE at UTC day boundary** `src/lib/stats.js:61–67`
+- EXPIRE fires only on count==1 (first occurrence of level:score pair today)
+- Untested: same level:score pair appearing in two UTC calendar days
+- **Sketch**: tests/stats.test.js — add test recording same level:score across UTC midnight, verify both keys have TTL
+
+**cookies.js readPlayerId validation** `src/lib/cookies.js` (new in 04925cf)
+- isUuid(sessionId) check used in guess-route.js:70, new-game-route.js
+- No standalone unit test for the validator
+- **Sketch**: tests/cookies.test.js — test isUuid rejects malformed IDs (missing hyphens, wrong length, etc.)
+
+### 🟢 LOW: Coverage/Observability
+**Daily route transient error path** `src/lib/daily.js:65–88`
+- Tests: cached-pick-fails (replaced), all-attempts-fail (exhausted pool)
+- Missing: all 4 attempts fail with transient errors (503, timeout) — should throw UpstreamError, not DryPoolError
+- **Sketch**: tests/daily-route.test.js — mock all pickPanoBySeed attempts to return 503, verify UpstreamError thrown
+
+**regionName fallback** `src/lib/region-request.js` — publicRegion() when nameVi is missing
+- **Sketch**: tests/region-request.test.js — create mock region with no nameVi, verify graceful fallback
+
+## Weak Tests: Mutation Survivors
+
+Assertions that would pass if implementation is broken:
+
+1. **region-coverage-route.test.js:72** — `expect(first.boundary).toBeTruthy()`
+   - Passes if boundary is `{}`, `[]`, `"x"`, or any truthy value
+   - Should: Assert non-empty GeoJSON polygon (e.g., `boundary.type === 'Polygon'`)
+   - **Mutation**: Return `boundary: {}` instead of GeoJSON → test passes ✗
+
+2. **daily-route.test.js:56** — `expect(first.regionCode).toMatch(/-|^DL$|^DH$/)`
+   - Fixture has 5 provinces, only 2 districts returned (DL, DH)
+   - Regex passes for "TPHCM-DL" but only runs once
+   - Should: Loop 10+ times, verify all returned codes are valid districts
+   - **Mutation**: Return province code on 50% of calls → test may miss it on one run ✗
+
+3. **storage.test.js:35** — `expect(storage.readItem('k')).toBe('v')` after writeItem
+   - Verifies value persists, does NOT verify watchers fire
+   - Should: Set up watch callback, assert it was called with correct value
+   - **Mutation**: Disable watch notifications → reads still work, test passes ✗
+
+4. **guess-route.test.js concurrent submit** — `expect(bodies.filter((body) => body.success)).toHaveLength(1)`
+   - Tests atomic claim for 10 concurrent submits
+   - Does NOT test: submitRoundScore throws AFTER session consumed (line 153 in guess/route.js)
+   - Response handling on partial failure is untested
+   - **Sketch**: tests/guess-route.test.js — mock submitRoundScore to reject, verify 500 + session consumed
+
+5. **daily-calendar.test.js previousDay** — `expect(previousDay('2027-01-01')).toBe('2026-12-31')`
+   - One year boundary case tested
+   - Does NOT verify: previousDay applied repeatedly (365 times) stays valid
+   - **Mutation**: Off-by-one in month arithmetic visible only on repeated application → test passes ✗
+
+## Fake Fidelity & Performance
+
+**Redis Fake**: Fully faithful
+- `del()` atomic return (1/0) ✓
+- `set()` with NX option ✓
+- `zincrby()` numeric return ✓
+- `pfadd()` boolean (1 if new) ✓
+
+**Neon Fake (PGlite)**: Sufficient for tests
+- No full-text search used in codebase
+- SQL subset covered by fixtures
+
+**Speed Breakdown**: 21.38s total
+- Import: 8.80s (PGlite WASM × 7 test files)
+- Tests: 47.29s (4 workers, 409 tests)
+- **Improvement**: Shared PGlite instance across files would save ~3s, but risks test isolation issues; not recommended
+
+## Summary
+
+**Test Execution**: ✓ All 409 tests pass; no flakes observed
+
+**Recent Fixes**: ✓ Session id minting, daily SET NX, storage write caching verified; pin clearing is UI-only
+
+**Coverage Gaps**: 
+- 1 load-bearing path untested (export-leaderboards empty check)
+- 3 medium-priority paths incomplete (region boundary, stats EXPIRE, daily transient)
+- 5 weak assertions would miss 40% of plausible mutations
+
+**Recommendations**:
+1. Add export-leaderboards.test.js for empty-result case (blocks backup validation)
+2. Strengthen region-coverage-route.test.js:72 boundary assertion
+3. Add daily transient-error test (clarifies error classification)
+4. Verify watch callbacks fire in storage.test.js
+5. Check submitRoundScore failure path in guess-route after claim
+
+---
+
+**Status**: DONE_WITH_CONCERNS  
+**Summary**: 409 tests pass. Three recent behavior changes all tested; one untested script (export-leaderboards) and five weak assertions identified. Pin-clearing (UI) and empty-backup (CI script) lack tests.