tiennm99/loto
1
0
Fork 0
mirror of https://github.com/tiennm99/loto.git synced 2026-09-03 18:16:53 +00:00
Code Issues Packages Projects Releases Wiki Activity

chore(plans): sweep shipped plan folders and orphaned reports

Browse Source 
Apply the policy stated in plans/todo.md ("all prior plan folders
have been deleted") that had drifted: remove three completed plan
folders (auto-call-countdown, both-mode-state-consistency,
switch-deploy-to-github-pages) and the reports tied to them.

Also remove pre-shipping audits that referenced the now-deleted
Cloudflare _headers / CSP setup, plus the just-actioned
cloudflare-legacy cleanup audit. Keep the evergreen Lô Tô rules
researcher report.

Refresh todo.md hand-off header to reflect the GitHub Pages target.
This commit is contained in:
tiennm99
2026-05-10 00:36:54 +07:00
parent 777b2dbaec
commit 4d89366deb
29 changed files with 3 additions and 3460 deletions
Download Patch File Download Diff File Expand all files Collapse all files
web/plans/260430-1919-auto-call-countdown/phase-01-build-autocountdown.md
-110
View File
@@ -1,110 +0,0 @@
---
phase: 1
title: "Build AutoCountdown component"
status: completed
priority: P2
effort: "1h"
dependencies: []
---
# Phase 1: Build `AutoCountdown.svelte`
## Overview
Self-contained countdown component: shows seconds-remaining number with a
circular SVG ring that depletes from full → empty over each tick interval.
No knowledge of game state — pure visual driven by props.
## Requirements
**Functional**
- Display integer seconds remaining (e.g. `5 → 4 → 3 → 2 → 1`)
- Render circular progress ring (SVG) that depletes smoothly during the tick
- Reset to full whenever a new tick starts (parent signals via `tickKey` prop change)
- Pause/hide cleanly when `running === false`
**Non-functional**
- Smooth animation via `requestAnimationFrame` — no `setInterval` polling
- Respect `prefers-reduced-motion`: skip ring animation, show only number
- File ≤ 200 lines (KISS — keep visual logic only)
- No localStorage / no settings reads — props-driven only
## Architecture
**Props**
```js
{
running: boolean, // master is auto-calling
duration: number, // seconds per tick (settings.autoCallSpeed, 1..10)
tickKey: number, // changes on each draw — triggers ring reset
}
```
**Internal state**
- `tickStart` (`$state`, ms timestamp): set to `performance.now()` when
`tickKey` changes or `running` flips on
- `now` (`$state`, ms): updated by rAF loop while `running === true`
- `secondsRemaining` (`$derived`): `Math.ceil(duration - (now - tickStart) / 1000)`
clamped to `[0, duration]`
- `progress` (`$derived`): `(now - tickStart) / (duration * 1000)` clamped to `[0, 1]`
**rAF loop**
- Single `$effect` keyed on `running`: starts loop when true, cancels on cleanup
- Loop sets `now = performance.now()`, then `requestAnimationFrame(loop)`
- When `running === false` → no rAF active, render last frame statically
**SVG ring**
- Outer `<svg viewBox="0 0 100 100">` square, sized via wrapper class
- Background track: full circle, light stroke
- Progress arc: same circle, `stroke-dasharray = circumference`,
`stroke-dashoffset = circumference * progress` → arc shrinks as time elapses
- Rotated `-90deg` so depletion starts at 12 o'clock and goes clockwise
**Reduced motion fallback**
- Detect once via `window.matchMedia('(prefers-reduced-motion: reduce)')`
- If set → skip rAF loop; update `now` only on `tickKey` change (one frame)
- Number still updates per tick (jumps from `5 → 4 → 3 ...`); ring stays full
## Related Code Files
- Create: `src/lib/AutoCountdown.svelte`
## Implementation Steps
1. Scaffold `<script>` block with props (`running`, `duration`, `tickKey`)
2. Add `tickStart` / `now` `$state` and derived `secondsRemaining` / `progress`
3. Add `$effect` to (a) reset `tickStart` on `tickKey` or `running` rising edge
and (b) drive rAF loop while `running`
4. Wire reduced-motion check (one-time, module-scope or component-scope const)
5. Render SVG: track circle + progress arc with dynamic `stroke-dashoffset`
6. Center seconds number with `text-3xl font-black tabular-nums`
7. Match token color language: amber-50 background, sky/emerald rings —
pick **one** neutral color (slate or amber) since this isn't a number token
8. Self-test: `console.log` derived values briefly (remove before commit)
## Success Criteria
- [ ] `AutoCountdown.svelte` < 200 lines
- [ ] Renders nothing visually intrusive when `running === false`
- [ ] Ring depletes from full to empty over `duration` seconds
- [ ] Number ticks down: `duration → duration-1 → ... → 1`
- [ ] Resets cleanly when `tickKey` changes
- [ ] No memory leaks — rAF cancelled on `running=false` and component unmount
- [ ] Reduced-motion: ring static, number still updates
## Risk Assessment
- **rAF leak**: forgetting cleanup when `running` flips false → loop keeps
running invisibly. Mitigation: single `$effect` with `cancelAnimationFrame`
in cleanup; verify with DevTools Performance panel.
- **Off-by-one number flash**: `Math.ceil(0)` → 0 right at tick edge before
parent resets `tickKey`. Mitigation: clamp lower bound to 1 while
`running && progress < 1`, allow 0 only when stopped.
- **Drift from setInterval**: parent uses `setInterval`, this uses `rAF` →
microsecond drift over many ticks. Acceptable: ring is visual feedback,
not authoritative timer; parent's interval still fires on schedule.
## Notes
- Keep visual style consistent with `MasterPanel`'s "Số vừa xổ" hero —
reuse `border-[6px]`, `rounded-full`, `tabular-nums`, `font-black`
web/plans/260430-1919-auto-call-countdown/phase-02-integrate-and-verify.md
-130
View File
@@ -1,130 +0,0 @@
---
phase: 2
title: "Integrate into MasterPanel and verify"
status: completed
priority: P2
effort: "30m"
dependencies: [1]
---
# Phase 2: Integrate into MasterPanel and verify
## Overview
Mount `AutoCountdown` inside `MasterPanel.svelte`, drive it from the existing
auto-call `$effect`, and verify behavior in the browser. Replace (or augment)
the static "Tự động: Xs/số" caption with the live countdown.
## Requirements
**Functional**
- Countdown appears only when `settings.autoCallEnabled && autoRunning && state?.remaining.length > 0`
- Resets each time `handleDrawNext()` fires
- Disappears when host clicks "Dừng" or game runs out
**Non-functional**
- No regression to existing `setInterval` timing — countdown is decorative
- No additional re-renders on the master grid (3-cell wide affected area only)
- All existing 53 vitest tests still pass
## Architecture
**Tick key signaling**
- Add `let tickCount = $state(0)` to `MasterPanel`
- `handleDrawNext()` increments `tickCount` after `broadcastDraw`
- Pass `tickKey={tickCount}` to `AutoCountdown`
**Reset on toggle**
- When `toggleAuto()` flips `autoRunning` from false → true, also bump
`tickCount` so countdown starts immediately at full duration
- Cleanest: bump `tickCount` inside the auto-call `$effect` on the rising
edge of `autoRunning`
**Layout**
- Replace lines 220–226 (`{#if settings.autoCallEnabled && state && state.remaining.length > 0}` block)
with a flex row that contains the countdown when running, and falls back
to the static caption when not running
- Or simpler: keep the static caption, add countdown above the "Số vừa xổ"
hero only while `autoRunning` — less rewiring of existing layout
**Recommendation**: keep static caption, render `AutoCountdown` immediately
above the "Số vừa xổ" hero (line 229), gated by `autoRunning && state?.remaining.length > 0`.
Size around `w-20 h-20 sm:w-24 sm:h-24` (smaller than the hero so it doesn't
compete for visual attention).
## Related Code Files
- Modify: `src/lib/MasterPanel.svelte`
- Add import for `AutoCountdown`
- Add `tickCount` state
- Bump `tickCount` in `handleDrawNext` and on `autoRunning` rising edge
- Render `<AutoCountdown>` above the hero block
## Implementation Steps
1. Import `AutoCountdown` from `$lib/AutoCountdown.svelte`
2. Add `let tickCount = $state(0);` near `autoRunning`
3. In `handleDrawNext()`: append `tickCount++;` after `broadcastDraw(next)`
4. In the auto-call `$effect`: on the rising edge of `autoRunning` (i.e. when
the effect re-runs because `autoRunning` flipped to true), also `tickCount++`
so the ring resets immediately rather than waiting for the first interval tick
5. Add markup before line 229's hero block:
```svelte
{#if autoRunning && state && state.remaining.length > 0}
<div class="flex justify-center mb-3">
<AutoCountdown
running={autoRunning}
duration={settings.autoCallSpeed}
tickKey={tickCount}
/>
</div>
{/if}
```
6. Run `npm run lint` — fix any warnings
7. Run `npm test` — ensure all 53 tests still pass
8. Run `npm run dev`, open browser, manually verify:
- Enable master mode + auto-call in settings
- Click "Bắt đầu" → countdown appears, ring depletes, number ticks down
- Each new draw resets the countdown
- Click "Dừng" → countdown disappears
- Change `autoCallSpeed` mid-run → next tick uses new duration cleanly
- Toggle reduced-motion (DevTools → Rendering tab) → ring stays static, number still updates
## Success Criteria
- [ ] Countdown visible only during active auto-call
- [ ] Smooth ring animation on default-motion devices
- [ ] Number resets to `autoCallSpeed` value at each tick
- [ ] No console errors / no leaked rAF after stopping
- [ ] All 53 existing vitest tests pass
- [ ] `npm run lint` clean
- [ ] `npm run build` succeeds
## Risk Assessment
- **Speed change mid-run**: parent's `$effect` tears down + re-arms the
`setInterval` when `settings.autoCallSpeed` changes (already handled,
see line 116). The `AutoCountdown` `duration` prop will also flow
through, so its derived calculations re-base. Need to bump `tickCount`
on speed change too — otherwise ring shows wrong progress until next
natural tick. Mitigation: bump `tickCount++` inside the auto-call
`$effect` body so any re-arm (running, speed, enabled) resets the ring.
- **CSP impact**: SVG inline + Svelte-injected style attrs (e.g.
`stroke-dashoffset`) — already permitted by existing CSP setup
(see `scripts/inject-csp-hashes.mjs`). No new CSP work needed.
- **Build size**: one small SVG component, negligible.
## Verification Checklist (manual)
- [ ] `npm run dev` starts cleanly
- [ ] Settings → enable "Chế độ quản trò" + "Tự động xổ"
- [ ] Click "Bắt đầu", observe countdown
- [ ] Watch ≥3 ticks — verify smooth depletion + reset
- [ ] Change speed slider → ring re-bases without glitch
- [ ] DevTools → Rendering → "prefers-reduced-motion: reduce" → verify static fallback
- [ ] Stop, verify countdown unmounts; rAF count in DevTools idle
## Docs Impact
- Minor: add brief note to `docs/codebase-summary.md` about the new component
and to `docs/system-architecture.md` if it lists key UI components
web/plans/260430-1919-auto-call-countdown/plan.md
-36
View File
@@ -1,36 +0,0 @@
---
title: "Auto-call countdown indicator"
status: completed
created: 2026-04-30
completed: 2026-04-30
slug: auto-call-countdown
---
# Auto-call countdown indicator
Show a visible countdown (number + shrinking circular ring) while the master
panel auto-calls numbers, so the host knows exactly when the next draw fires.
## Why
`MasterPanel.svelte` currently shows only a static "Tự động: Xs/số" line while
auto-call runs. Host has no per-tick feedback — UX feels dead between draws,
especially at slower speeds (5–10s).
## Phases
| # | Phase | Status |
|---|-------|--------|
| 1 | [Build `AutoCountdown.svelte`](phase-01-build-autocountdown.md) | completed |
| 2 | [Integrate into MasterPanel + verify](phase-02-integrate-and-verify.md) | completed |
## Key Files
- Create: `src/lib/AutoCountdown.svelte`
- Modify: `src/lib/MasterPanel.svelte`
## Out of Scope
- Sound/vibration on tick — voice already speaks when number is drawn
- Configurable countdown styling — match existing token visual language
- Player-side countdown (this is master-only)
web/plans/260430-2050-both-mode-state-consistency/phase-01-lift-master-state.md
-121
View File
@@ -1,121 +0,0 @@
---
phase: 1
title: "Lift master state to shared store"
status: completed
priority: P1
effort: "1h"
dependencies: []
---
# Phase 1: Lift master state to shared store
## Overview
Extract the `{called, remaining}` state from `MasterPanel.svelte` into a
new reactive module `master-store.svelte.js`. MasterPanel becomes a
view over the store; nothing else changes UX-wise. Foundation for
phase 2's player auto-cross.
## Requirements
**Functional**
- Same persistence semantics: localStorage `loto_master`, same shape
- Same load-on-mount, save-on-change behavior
- Existing master grid / "Số vừa xổ" / history list unchanged
**Non-functional**
- File ≤ 200 lines
- Validators preserved (16 KB cap, `__proto__` stripping, shape check)
- No behavior change observable from user — pure refactor
## Architecture
**`src/lib/master-store.svelte.js`** (new)
```js
const STORAGE_KEY = "loto_master";
const MAX_STORAGE_BYTES = 16_384;
export const masterState = $state({
/** @type {number[]} */
called: [],
/** @type {number[]} */
remaining: [],
});
export function loadMaster() { /* parse + validate, write into masterState */ }
export function saveMaster() { /* serialize from masterState */ }
export function startNewGame() { /* fill remaining with shuffled 1..90, clear called */ }
export function drawNext() {
/** @returns {number | null} */
if (masterState.remaining.length === 0) return null;
const next = masterState.remaining[0];
masterState.called = [...masterState.called, next];
masterState.remaining = masterState.remaining.slice(1);
return next;
}
export function resetMaster() {
/** Clears both arrays — used by "Ván mới" */
masterState.called = [];
masterState.remaining = [];
}
```
The `lastCalled` derived value lives in `MasterPanel` since it's
display-only:
```js
const lastCalled = $derived(
masterState.called.length ? masterState.called.at(-1) : null,
);
```
**Persistence pattern**: a single $effect in `MasterPanel` (or in the
store module if cleaner) calls `saveMaster()` on `masterState.called`
or `masterState.remaining` change. Keep load gated on first mount so
SSR doesn't try to touch localStorage.
## Related Code Files
- Create: `src/lib/master-store.svelte.js`
- Modify: `src/lib/MasterPanel.svelte`
- Remove `state`, `loadState`, `saveState`, `createFreshState` from
component-level — move to `master-store.svelte.js`
- `handleNewGame` calls `startNewGame()`
- `handleDrawNext` calls `drawNext()`, then `broadcastDraw(next)`
(still using bus until phase 2)
- `lastCalled` becomes `$derived(masterState.called.at(-1))`
- The 11×9 board's `callOrder` map derived from `masterState.called`
## Implementation Steps
1. Create `master-store.svelte.js` with the 5 exports above
2. Move `STORAGE_KEY`, `MAX_STORAGE_BYTES`, `loadState`, `saveState`,
and the shuffle helper from `MasterPanel.svelte` into the store
3. Replace `MasterPanel`'s `let state = $state(...)` with reads against
`masterState` (all six mutations: load, draw, new, called list,
remaining list, lastCalled derive)
4. Wire load on mount: `$effect(() => { loadMaster(); });`
5. Wire save on change: `$effect(() => { saveMaster(); });` —
reading `masterState.called` + `masterState.remaining` inside
6. Smoke-test: refresh tab, master state persists; click "Ván mới",
reset works; click "Xổ số", draws + broadcasts as before
## Success Criteria
- [ ] `master-store.svelte.js` exists, ≤ 100 lines
- [ ] `MasterPanel.svelte` shrinks (no state/storage code inside)
- [ ] Master flow unchanged: load on mount, draw, new game, reload-restore
- [ ] All 123 existing vitest tests still pass
- [ ] `npx svelte-check` clean
- [ ] `npm run build` succeeds
## Risk Assessment
- **Reactivity break**: rune state inside a `.svelte.js` module re-exports
fine via destructuring? Yes — `settings-store.svelte.js` proves the
pattern. Make sure consumers import `masterState` (not destructure
fields) so reactivity threads through.
- **Migration of saved data**: localStorage shape unchanged, no migration
needed. Validators carried over verbatim.
- **Multiple MasterPanel mounts**: not exercised today (mode toggle
unmounts). Store is module-singleton, so two mounts would share — fine.
web/plans/260430-2050-both-mode-state-consistency/phase-02-player-via-store.md
-157
View File
@@ -1,157 +0,0 @@
---
phase: 2
title: "Player auto-cross via shared store"
status: completed
priority: P1
effort: "1.5h"
dependencies: [1]
---
# Phase 2: Player auto-cross via shared store
## Overview
Replace `PlayerBoard`'s bus-driven auto-tick with a $effect that
watches `masterState.called`. New numbers are auto-crossed; the
"already-handled" cursor moves to track length, not timestamp. Kills
the F7 (1ms collision) and F8 (single-slot history loss) classes
because we read the array directly.
## Requirements
**Functional**
- Auto-cross still fires only in `mode === "both"`
- Manual untick stays manual — auto-cross does NOT re-cross numbers the
user explicitly unticked since the last reset
- All previously called numbers still on the board get crossed if the
cursor catches up (covers F1 player-regen replay in phase 3)
**Non-functional**
- No regression in existing 53 player-side test scenarios
- Auto-tick logic unit-testable as a pure helper
## Architecture
**Replace `auto-tick.js` with `player-auto-cross.js`** (new pure helper):
```js
/**
* Decide which cells flip given the master's full called[] history
* and the player's already-applied cursor. Cursor advances strictly,
* even when no cell flips, so manual unticks don't re-fire.
*
* @param {object} args
* @param {number[][] | null} args.grid
* @param {boolean[][]} args.crossed
* @param {number[]} args.called - master's full history
* @param {number} args.lastHandledIndex - index already consumed
* @param {Set<number>} args.manualUnticks - numbers user unticked
* @param {"player" | "master" | "both"} args.mode
* @returns {{ crossed: boolean[][], lastHandledIndex: number, changed: boolean }}
*/
export function applyMasterCalls({ grid, crossed, called, lastHandledIndex, manualUnticks, mode }) {
if (lastHandledIndex >= called.length) return { crossed, lastHandledIndex, changed: false };
if (mode !== "both" || !grid || crossed.length === 0) {
// Advance cursor anyway to keep player→both transitions catching up
// only on FUTURE draws, not the whole back-history.
return { crossed, lastHandledIndex: called.length, changed: false };
}
let next = crossed;
let changed = false;
for (let i = lastHandledIndex; i < called.length; i++) {
const num = called[i];
if (manualUnticks.has(num)) continue;
const target = findUncrossedCell(grid, next, num);
if (!target) continue;
next = next.map((row, ri) =>
ri === target.row ? row.map((v, ci) => (ci === target.col ? true : v)) : row,
);
changed = true;
}
return { crossed: next, lastHandledIndex: called.length, changed };
}
```
**Manual untick tracking**
Add `let manualUnticks = $state(new Set())` at PlayerBoard's top. In the
cell click handler, if the user transitions a cell from `true → false`
on a number that's in `masterState.called`, add it to `manualUnticks`.
`true → false` on an uncalled number doesn't need tracking. `false → true`
removes from the set (re-cross overrides the untick).
**Persistence**: `manualUnticks` persisted as a sorted array under key
`{prefix}_manualUnticks`, loaded on mount.
**Player effect** (replaces old bus-watching effect):
```js
let lastHandledIndex = $state(0);
$effect(() => {
const result = applyMasterCalls({
grid,
crossed,
called: masterState.called,
lastHandledIndex,
manualUnticks,
mode: settings.mode,
});
if (result.lastHandledIndex !== lastHandledIndex) {
lastHandledIndex = result.lastHandledIndex;
}
if (result.changed) crossed = result.crossed;
});
```
The `lastHandledIndex` is in-memory only — phase 3 covers the reload
behavior (reload re-applies all called numbers as a catch-up).
## Related Code Files
- Create: `src/lib/player-auto-cross.js` + tests
- Modify: `src/lib/PlayerBoard.svelte`
- Drop `bus`, `resetBus` import (kept only until phase 3 cleanup)
- Add `masterState` import from `$lib/master-store.svelte.js`
- Replace `lastHandledDrawAt` with `lastHandledIndex`
- Add `manualUnticks` $state + persistence
- Update cell click handler to track `true → false` transitions
- Keep (read-only ref): `findUncrossedCell` from `game-logic.js`
## Implementation Steps
1. Write `player-auto-cross.js` with `applyMasterCalls` (copy `findUncrossedCell` logic OR import it)
2. Write `player-auto-cross.test.js` covering:
- mode mismatch advances cursor without flipping
- mode=both crosses uncrossed cells, skips already-crossed
- manualUnticks numbers skipped
- empty called array → no-op
- cursor at length → no-op
3. Update `PlayerBoard.svelte`:
- Replace bus auto-tick effect with `applyMasterCalls` effect
- Add `manualUnticks` state + load/save helpers in `game-logic.js`
- Track unticks in `handleCellClick` (locate it; add 2-line transition check)
4. Verify mode toggle player→both: cursor advances to `called.length` in
non-both modes, so toggle-on doesn't replay back-history (intentional;
F6 explicitly accepts this trade-off — refresh path covers catch-up)
5. Run `npm test` — fix any auto-tick.test.js fallout (delete old tests if helper retired)
## Success Criteria
- [ ] `player-auto-cross.js` ≤ 100 lines, fully unit-tested
- [ ] PlayerBoard auto-crosses correctly on master draws (manual smoke)
- [ ] Manual untick → next master draw of SAME number does NOT re-cross
- [ ] Manual re-cross clears the untick (next draw of same number works)
- [ ] All vitest tests pass (with old `auto-tick.test.js` removed if helper retired)
## Risk Assessment
- **`manualUnticks` persistence shape**: stored as array of numbers,
reconstructed to `Set<number>` on load. Validate `Number.isInteger`
+ range 1..90 to defend against poisoned localStorage.
- **Cursor-vs-history mismatch on first run**: `lastHandledIndex` defaults
to 0; on mount, $effect runs, applies all `called` history. This is
the desired catch-up behavior for reload (covers F4).
- **Effect re-entry**: writing `crossed` inside an effect that reads
`crossed` — same pattern as today, dedup via `result.changed` makes
it stable. No new exposure.
web/plans/260430-2050-both-mode-state-consistency/phase-03-replay-flows.md
-182
View File
@@ -1,182 +0,0 @@
---
phase: 3
title: "Replay flows + retire bus"
status: completed
priority: P1
effort: "1h"
dependencies: [1, 2]
---
# Phase 3: Replay flows + retire bus
## Overview
Wire up the four cross-panel flows (master "Ván mới", player "Tạo bảng
mới", player "Xoá đánh dấu", master draw → player auto-cross) to the
new shared store. Delete the now-dead `call-bus.svelte.js` and remove
the F3 violation (player handlers calling `resetBus()`).
## Requirements
**Functional**
- **Master "Ván mới"** in both mode → player crossed clears + manualUnticks clears (locked decision #1)
- **Player "Tạo bảng mới"** mid-game → new grid, then auto-cross all current `masterState.called` numbers found on it (covers F1)
- **Player "Xoá đánh dấu"** in both mode → clear crossed + manualUnticks, then immediately re-apply `masterState.called` so all called numbers cross again (locked decision #2)
- **Master draw** → player auto-cross via phase 2's effect, no bus needed
**Non-functional**
- No reference to `call-bus.svelte.js` remains in source
- `resetBus` import in PlayerBoard removed (F3 fixed)
## Architecture
**Master "Ván mới" propagation**
Detection: PlayerBoard runs an `$effect` watching `masterState.called.length`.
When it transitions from `> 0` to `0` AND `settings.mode === "both"`,
clear player crossed + manualUnticks + reset cursor.
```js
let prevCalledLen = $state(0);
$effect(() => {
const len = masterState.called.length;
const wasReset = prevCalledLen > 0 && len === 0;
prevCalledLen = len;
if (wasReset && settings.mode === "both" && grid) {
crossed = grid.map(row => row.map(() => false));
manualUnticks = new Set();
lastHandledIndex = 0;
celebratedRows.clear();
notifiedWaitingRows.clear();
}
});
```
**Player "Tạo bảng mới" replay** (`handleGenerate`)
```js
function handleGenerate() {
if (grid && !confirm("Bạn có muốn tạo lại bảng không?")) return;
cancelPlayback();
const newGrid = generateGrid();
let newCrossed = newGrid.map(row => row.map(() => false));
// Replay master's called[] onto the new grid (no-op outside both mode).
if (settings.mode === "both") {
const result = applyMasterCalls({
grid: newGrid, crossed: newCrossed,
called: masterState.called, lastHandledIndex: 0,
manualUnticks: new Set(), mode: "both",
});
newCrossed = result.crossed;
lastHandledIndex = result.lastHandledIndex;
} else {
lastHandledIndex = masterState.called.length;
}
grid = newGrid;
crossed = newCrossed;
manualUnticks = new Set();
saveGrid(newGrid, STORAGE_PREFIX);
saveCrossedState(newCrossed, STORAGE_PREFIX);
saveManualUnticks(manualUnticks, STORAGE_PREFIX);
celebratedRows.clear();
notifiedWaitingRows.clear();
dismissToast();
showCongrats = false;
// No resetBus — bus is gone.
}
```
**Player "Xoá đánh dấu" replay** (`handleClear`)
```js
function handleClear() {
if (!grid) return;
const hasMarks = crossed.some(row => row.some(Boolean));
if (hasMarks && !confirm("Bạn có muốn xoá tất cả đánh dấu không?")) return;
cancelPlayback();
manualUnticks = new Set();
let cleared = grid.map(row => row.map(() => false));
if (settings.mode === "both") {
const result = applyMasterCalls({
grid, crossed: cleared,
called: masterState.called, lastHandledIndex: 0,
manualUnticks: new Set(), mode: "both",
});
cleared = result.crossed;
lastHandledIndex = result.lastHandledIndex;
} else {
lastHandledIndex = masterState.called.length;
}
crossed = cleared;
saveCrossedState(crossed, STORAGE_PREFIX);
saveManualUnticks(manualUnticks, STORAGE_PREFIX);
celebratedRows.clear();
notifiedWaitingRows.clear();
dismissToast();
showCongrats = false;
}
```
**Bus retirement**
After phase 2 + the above wiring, nothing imports from `call-bus.svelte.js`
or `auto-tick.js`. Delete the source files and their tests:
- `src/lib/call-bus.svelte.js`
- `src/lib/call-bus.test.js`
- `src/lib/auto-tick.js`
- `src/lib/auto-tick.test.js`
Remove `MasterPanel`'s `import { broadcastDraw, resetBus } ...` and the
`broadcastDraw(next)` / `resetBus()` calls — they're no-ops now since
`masterState` itself is the signal.
## Related Code Files
- Modify: `src/lib/PlayerBoard.svelte` (handleGenerate, handleClear, new $effect for master-reset detection)
- Modify: `src/lib/MasterPanel.svelte` (drop bus imports + calls)
- Modify: `src/lib/game-logic.js` (add `saveManualUnticks` / `loadManualUnticks`)
- Delete: `src/lib/call-bus.svelte.js`, `src/lib/call-bus.test.js`,
`src/lib/auto-tick.js`, `src/lib/auto-tick.test.js`
## Implementation Steps
1. Add `saveManualUnticks` / `loadManualUnticks` to `game-logic.js`
(mirror existing patterns; validate ints in [1,90])
2. Update `handleGenerate` per architecture above
3. Update `handleClear` per architecture above
4. Add `prevCalledLen` $state + master-reset $effect
5. Strip bus imports + calls from MasterPanel and PlayerBoard
6. Delete the four files above
7. `npm test` — should still pass (after old auto-tick.test removal)
8. `npm run lint`, `npm run build`, `npx svelte-check`
## Success Criteria
- [ ] No source file imports from `call-bus` or `auto-tick`
- [ ] All four flows behave per locked decisions:
- Master "Ván mới" wipes player crossed in both mode
- Player regen replays master.called onto new grid
- Player "Xoá đánh dấu" in both mode replays immediately
- Master draw auto-crosses on player side
- [ ] Lint, build, svelte-check clean
- [ ] All remaining tests pass
## Risk Assessment
- **Mode-aware reset detection**: a player toggling mode AWAY from
"both" mid-game shouldn't accidentally trigger the reset clear. The
$effect gates on `settings.mode === "both"` at trigger time, so toggling
to "player" then master "Ván mới" in another window won't wipe player
crossed (multi-tab is out of scope, but local mode-toggle is covered).
- **`prevCalledLen` race on mount**: it initializes to 0; first effect run
sees `len = stored.called.length`, transition `0 → N` is NOT a reset.
Only `>0 → 0` triggers, so safe.
- **Replay performance**: replay loop is O(called × grid) ≈ O(90 × 81) =
~7k ops worst case. Trivial.
## Open question
If the user wants the same "force-clear" behavior on mode toggle
player→both (auto-replay back-history), phase 2's cursor logic needs a
tweak. Current locked decisions don't cover this. Flag for sếp post-impl.
web/plans/260430-2050-both-mode-state-consistency/phase-04-tests-and-verify.md
-110
View File
@@ -1,110 +0,0 @@
---
phase: 4
title: "Tests and verify"
status: completed
priority: P1
effort: "45m"
dependencies: [1, 2, 3]
---
# Phase 4: Tests and verify
## Overview
Add coverage for the new helper + flows, run the full suite, and
manually verify the four locked behaviors in the browser.
## Requirements
- New unit tests for `applyMasterCalls`
- New unit tests for `master-store.svelte.js` (load/save/draw/new/reset)
- Manual browser verification of every locked behavior
- All existing tests still pass
## Test Matrix
### `player-auto-cross.test.js` (new)
| Case | Expected |
|------|----------|
| empty called[] | no-op, cursor stays at 0 |
| cursor = called.length | no-op |
| mode="player", called grows | cursor advances to length, no flips |
| mode="both", called=[5], grid has 5 | crosses cell, cursor=1 |
| mode="both", called=[5,5,5] (impossible but test) | first match crosses, second / third no-op (already crossed) |
| mode="both", manualUnticks={5}, called=[5] | no flip, cursor advances |
| mode="both", grid=null | no flip, cursor stays |
### `master-store.test.js` (new)
| Case | Expected |
|------|----------|
| `loadMaster` with empty storage | masterState stays empty |
| `loadMaster` with corrupt JSON | falls back to empty |
| `loadMaster` with > 16 KB | rejected |
| `startNewGame` | called=[], remaining=shuffle(1..90) |
| `drawNext` | called appends, remaining shifts; returns drawn num |
| `drawNext` with empty remaining | returns null, no mutation |
| `resetMaster` | both arrays empty |
### `game-logic.test.js` (extend)
| Case | Expected |
|------|----------|
| `saveManualUnticks` round-trip | Set in → Set out, sorted on disk |
| `loadManualUnticks` with garbage | empty Set |
| `loadManualUnticks` with out-of-range nums | filtered |
### Manual browser verification
Enable both mode + auto-call. Click through:
| # | Action | Expected |
|---|--------|----------|
| 1 | Master "Ván mới" with player marks present | Player crossed wipes |
| 2 | Master draws 3 numbers, then player "Tạo bảng mới" | New grid, those 3 numbers (if on grid) crossed |
| 3 | Master draws 5 numbers, player "Xoá đánh dấu" | All 5 immediately cross again |
| 4 | Player manually unticks #42 after auto-cross, master re-broadcast not possible (each num drawn once) | n/a — verify unticks persist across reload instead |
| 5 | Reload mid-game | Master state restored, player crossed restored, new master draws still auto-cross |
| 6 | Mode toggle both → player → both | New draws auto-cross; back-history NOT replayed (phase 3 open Q) |
| 7 | "Bắt đầu" auto-call → countdown shows + draws every N seconds | (regression check for last task) |
## Implementation Steps
1. Write `player-auto-cross.test.js`
2. Write `master-store.test.js`
3. Extend `game-logic.test.js`
4. `npm test` — expect green (delete old auto-tick.test.js if it broke
per phase 3)
5. `npm run lint`, `npx svelte-check`, `npm run build`
6. `npm run dev`, walk through the 7-case manual matrix
7. Commit per-phase or as one feat commit (sếp's call)
## Success Criteria
- [ ] All new test files green
- [ ] Full suite green (count > 123 minus retired tests + new tests)
- [ ] Lint clean (only pre-existing errors in `verify-build-inline-scripts.mjs`,
`MasterEmptyState.svelte`, `PlayerBoard.svelte` 396 — those are pre-existing,
not from this refactor)
- [ ] All 7 manual cases pass
- [ ] No console errors / warnings during the walkthrough
## Risk Assessment
- **Test fallout**: removing `auto-tick.js` retires 53 tests' worth of
coverage. The replacement helper covers equivalent ground; verify
count parity before declaring done.
- **Manual case 6 (mode toggle replay)**: this is currently OUT of scope
per the open question in phase 3. If sếp wants replay-on-toggle later,
it's a one-line tweak in `applyMasterCalls`'s mode-mismatch branch
(don't advance cursor on mismatch — let the next both-mode pass replay).
## Docs Impact
- Update `docs/codebase-summary.md`:
- Replace "call-bus.svelte.js" entry with "master-store.svelte.js"
- Replace "auto-tick.js" entry with "player-auto-cross.js"
- Update PlayerBoard / MasterPanel descriptions
- Update `docs/system-architecture.md` if it diagrams the bus
- Update `plans/todo.md` carryover items if any
web/plans/260430-2050-both-mode-state-consistency/plan.md
-62
View File
@@ -1,62 +0,0 @@
---
title: "Both-mode state consistency refactor"
status: completed
created: 2026-04-30
completed: 2026-04-30
slug: both-mode-state-consistency
---
# Both-mode state consistency refactor
Fix the cross-panel inconsistencies surfaced in the 2026-04-30 audit
(`plans/reports/code-reviewer-260430-2024-both-mode-consistency.md` and
`plans/reports/brainstorm-260430-2024-both-mode-edge-cases.md`).
Targets findings F1, F2, F4, F6, F7, F8, F10. Out of scope: F9 (voice
collision) and #20 (multi-tab) — separate plans to follow.
## Why
Single-slot `call-bus` carries only the latest draw. Any state event
that happens off-bus (player regen, master "Ván mới", reload, mode
toggle, throttled tab) silently loses history. Symptom the host hit:
fresh player board doesn't replay master's existing draws.
## Product decisions (locked 2026-04-30)
1. Master "Ván mới" → **force-clear** player's crossed in both mode.
2. Player "Xoá đánh dấu" in both mode → **replay all** called numbers
immediately after the clear.
## Approach (surgical, KISS)
Lift master's `called[]` to a shared reactive store. Player auto-cross
becomes a $effect on `masterStore.called` length growth, not a bus
slot. Existing tests keep passing; the bus dies because nothing reads
it. No full crossed-derivation rewrite — keep `crossed` as $state to
preserve manual cross/uncross UX.
## Phases
| # | Phase | Status |
|---|-------|--------|
| 1 | [Lift master state to shared store](phase-01-lift-master-state.md) | completed |
| 2 | [Player auto-cross via shared store](phase-02-player-via-store.md) | completed |
| 3 | [Replay flows + retire bus](phase-03-replay-flows.md) | completed |
| 4 | [Tests + verify](phase-04-tests-and-verify.md) | completed |
## Key Files
- Create: `src/lib/master-store.svelte.js`
- Modify: `src/lib/MasterPanel.svelte`, `src/lib/PlayerBoard.svelte`,
`src/lib/auto-tick.js` (or replace with new helper)
- Delete (after migration): `src/lib/call-bus.svelte.js`,
`src/lib/call-bus.test.js`, `src/lib/auto-tick.js`,
`src/lib/auto-tick.test.js` (if signature changes too much to keep)
## Out of Scope
- F9 voice ownership (master vs player playback collision)
- #20 multi-tab guard (banner / single-master lock)
- Full derived-crossed model (would erase manualUntick UX)
- AutoCountdown — already shipped, untouched here
web/plans/260509-2247-switch-deploy-to-github-pages/phase-01-wire-gh-pages-build-into-ci.md
-88
View File
@@ -1,88 +0,0 @@
---
phase: 1
title: Wire GH Pages build into CI
status: completed
priority: P2
effort: 1h
dependencies: []
---
# Phase 1: Wire GH Pages build into CI
## Overview
Replace the redirect-only `deploy-github-pages.yml` with a real build+deploy
pipeline that runs `npm run build:gh` (basePath `/loto`) and uploads `build/`
as the GH Pages artifact. Site lives at `https://tiennm99.github.io/loto/`.
## Requirements
- Functional: push to `main` builds and deploys the SvelteKit app to GH Pages.
- Non-functional: workflow uses `actions/configure-pages@v5`, `upload-pages-artifact@v3`, `deploy-pages@v4` (already present). Concurrency group `github-pages`. Caches npm.
## Architecture
Single workflow, two jobs (build → deploy). Build job runs Node 20, `npm ci`,
`npm run build:gh`, uploads `build/`. Deploy job consumes the artifact.
`build:gh` already exists in `package.json` and produces basePath `/loto` via
`BUILD_PROFILE=gh` in `svelte.config.js:23`. CSP-hash injection step in that
script (`node scripts/inject-csp-hashes.mjs`) gets removed in Phase 2 — for
this phase we leave it; the script no-ops cleanly if `_headers` is absent
after Phase 2 lands (will be revisited).
Note: Phase 1 + 2 should land in the same PR so the build script and the
files it touches stay consistent.
## Related Code Files
- Modify: `.github/workflows/deploy-github-pages.yml`
- Read for context: `package.json`, `svelte.config.js`, `.github/workflows/verify-build.yml`
## Implementation Steps
1. Rewrite `.github/workflows/deploy-github-pages.yml`:
- Replace the `Generate redirect pages` step block with a real build:
```yaml
- uses: actions/checkout@v4
- uses: actions/setup-node@v4
with:
node-version: 20
cache: npm
- run: npm ci
- run: npm run build:gh
- uses: actions/configure-pages@v5
- uses: actions/upload-pages-artifact@v3
with:
path: build
```
- Keep deploy job as-is (`actions/deploy-pages@v4`, environment
`github-pages`).
- Rename workflow: `name: Deploy redirect to GitHub Pages` →
`name: Deploy to GitHub Pages`.
2. After PR merges and the first run goes green:
- GitHub repo → Settings → Pages → Source: GitHub Actions (should already
be set; confirm).
- Confirm `https://tiennm99.github.io/loto/` loads the app, not the old
redirect HTML.
## Success Criteria
- [ ] `deploy-github-pages.yml` runs `npm run build:gh` and uploads `build/`.
- [ ] First post-merge run on `main` succeeds (both build + deploy jobs green).
- [ ] `https://tiennm99.github.io/loto/` serves the live app.
- [ ] Service worker registers at `/loto/sw.js`; manifest at
`/loto/manifest.webmanifest`; icons at `/loto/icons/...` resolve.
- [ ] Audio clips load from `/loto/audio/{voice}/{n}.mp3`.
## Risk Assessment
- **Risk:** basePath mismatch causes 404s on assets.
**Mitigation:** `build:gh` already wires basePath `/loto`; `import { base } from '$app/paths'` is used internally per `docs/deployment-guide.md:12`. Verify in Phase 1 success-criteria checks.
- **Risk:** Stale CF cache or DNS still points users to old `loto.miti99.com`.
**Mitigation:** Out of scope for this plan; documented as post-merge manual step in `plan.md`.
- **Risk:** Service worker from previous CF deploy lingers in user browsers and
serves stale paths. **Mitigation:** SW uses `registerType: "autoUpdate"`
(`vite.config.js:42`). Users on `loto.miti99.com` won't see the new
deployment anyway since URL changed; users on `tiennm99.github.io/loto` had
only the redirect HTML before, no SW registered.
web/plans/260509-2247-switch-deploy-to-github-pages/phase-02-remove-cloudflare-artifacts.md
-87
View File
@@ -1,87 +0,0 @@
---
phase: 2
title: Remove Cloudflare artifacts
status: completed
priority: P2
effort: 1h
dependencies:
- 1
---
# Phase 2: Remove Cloudflare artifacts
## Overview
Delete CF-only files (`wrangler.toml`, `static/_headers`, `static/_redirects`)
and the CSP-hash machinery that exists solely to patch `_headers`. GH Pages
ignores these files (or wouldn't have them) and the user opted to keep it
simple for a static site.
## Requirements
- Functional: `npm run build` and `npm run build:gh` produce a deployable
`build/` without invoking CSP-hash injection or relying on `_headers` /
`_redirects`.
- Non-functional: no dead scripts in `package.json`; no orphan files in
`static/` or repo root.
## Architecture
The chain `vite build → inject-csp-hashes.mjs → verify-build-inline-scripts.mjs`
exists only because CF Pages reads `static/_headers` and we wanted to ship a
strict CSP without `'unsafe-inline'`. None of that survives the move:
- GH Pages can't set HTTP headers from a `_headers` file.
- User accepted dropping CSP/security-headers machinery.
So the simplification is:
- `npm run build` → `vite build` (no postbuild step).
- `npm run build:gh` → `BUILD_PROFILE=gh vite build` (no postbuild step).
- Delete `verify:build` script entry; CI step that called it gets removed.
## Related Code Files
- Delete: `wrangler.toml`
- Delete: `static/_headers`
- Delete: `static/_redirects`
- Delete: `scripts/inject-csp-hashes.mjs`
- Delete: `scripts/verify-build-inline-scripts.mjs`
- Modify: `package.json` (drop CSP postbuild from `build` and `build:gh`; drop `verify:build` script)
- Modify: `.github/workflows/verify-build.yml` (drop `npm run verify:build` step)
## Implementation Steps
1. Delete `wrangler.toml` (CF-only manifest).
2. Delete `static/_headers` and `static/_redirects` (CF-only routing/headers).
3. Delete `scripts/inject-csp-hashes.mjs` and `scripts/verify-build-inline-scripts.mjs`.
4. Edit `package.json` scripts:
- `"build": "vite build && node scripts/inject-csp-hashes.mjs"` →
`"build": "vite build"`
- `"build:gh": "BUILD_PROFILE=gh vite build && node scripts/inject-csp-hashes.mjs"` →
`"build:gh": "BUILD_PROFILE=gh vite build"`
- Remove the entire `"verify:build": "node scripts/verify-build-inline-scripts.mjs"` line.
5. Edit `.github/workflows/verify-build.yml`:
- Remove the trailing `- run: npm run verify:build` step.
6. Run locally to confirm:
- `npm run build` exits 0, produces `build/index.html` and assets.
- `npm run build:gh` exits 0, produces `build/` with basePath `/loto`
visible in the rendered HTML (`grep -q '/loto/_app/' build/index.html`).
- `npm test` still passes (no test should reference `_headers`/`_redirects`).
## Success Criteria
- [ ] Deleted files no longer present (`git status` shows them as deletions).
- [ ] `npm run build` and `npm run build:gh` both succeed locally.
- [ ] `package.json` has no reference to `inject-csp-hashes` or `verify:build`.
- [ ] `verify-build.yml` does not invoke `npm run verify:build`.
- [ ] CI `Verify build` workflow stays green on PR.
## Risk Assessment
- **Risk:** Some test or doc depends on `static/_headers` content.
**Mitigation:** Phase 3 sweeps docs. Tests under `src/` don't reference
these files; sanity-check with `grep -r '_headers\|_redirects\|wrangler' src/ tests/ 2>/dev/null` before merging.
- **Risk:** `inject-csp-hashes.mjs` referenced from somewhere besides
`package.json` (e.g. a husky hook, a doc snippet someone copy-pastes).
**Mitigation:** `grep -r 'inject-csp-hashes\|verify-build-inline-scripts' .`
before deletion to confirm only `package.json` references them.
web/plans/260509-2247-switch-deploy-to-github-pages/phase-03-update-docs-and-todo.md
-99
View File
@@ -1,99 +0,0 @@
---
phase: 3
title: Update docs and TODO
status: completed
priority: P2
effort: 1h
dependencies:
- 1
- 2
---
# Phase 3: Update docs and TODO
## Overview
Sweep all docs and the residual TODO list to remove CF references and
describe GH Pages as the sole deploy target. Update the README build snippet
since `build:gh` becomes the canonical build (or `build` stays as the GH
build — we keep both scripts for now since they're aliased to the same
output via env).
## Requirements
- Functional: docs accurately describe the new deploy flow.
- Non-functional: no stale `loto.miti99.com` references except where
intentional (e.g. PageFooter's `miti99.com` is the author site, not the
deploy URL — leave alone).
## Related Code Files
- Modify: `README.md`
- Modify: `docs/deployment-guide.md` (heaviest rewrite — currently CF-centric)
- Modify: `docs/codebase-summary.md`
- Modify: `docs/system-architecture.md`
- Modify: `docs/code-standards.md`
- Modify: `docs/development-roadmap.md`
- Modify: `docs/project-overview-pdr.md`
- Modify: `plans/todo.md` (drop CF Lighthouse entries; keep GH Pages ones)
- Read for context: `src/lib/PageFooter.svelte` (no change — `miti99.com` link is unrelated)
## Implementation Steps
1. **`README.md`**
- Replace the Build section's two-script table with a single `npm run build:gh`
line OR keep both but mark `build:gh` as the deployed one.
- Replace `Deployed to Cloudflare Pages from main (set up via the CF
dashboard — see docs/deployment-guide.md).` with: `Deployed to GitHub
Pages from main via .github/workflows/deploy-github-pages.yml — see
docs/deployment-guide.md.`
2. **`docs/deployment-guide.md`** — substantial rewrite:
- Build Profiles table: drop CF row, keep GH Pages row as the only target.
- Replace "Production Deployment — Cloudflare Pages" section with
"Production Deployment — GitHub Pages": describe the workflow, GH repo
Settings → Pages → Source: GitHub Actions, URL `https://tiennm99.github.io/loto/`.
- Delete "GitHub Pages (redirect-only)" subsection.
- Delete "Manual GH Pages Build (still available)" subsection (the build
IS the canonical build now).
- "Build & Output" section: remove mention of `_headers`/`_redirects` and
CSP injection.
- "Environment Variables → Build-Time": `BUILD_PROFILE=gh` is now the
default for the deploy workflow; document it as such.
- "CI/CD Pipeline" section: drop the Cloudflare bullet; keep only GH Pages.
- "Security Considerations": drop CSP/headers bullets that no longer apply;
a one-liner that GH Pages serves HTTPS by default is enough.
- "Troubleshooting" table: drop the `BUILD_PROFILE` row that mentions
Cloudflare; reword the basePath row for `/loto` only.
- Update "Last reviewed" date to 2026-05-09.
3. **`docs/codebase-summary.md`** — find Cloudflare/CF/wrangler/_headers
mentions, replace with GH Pages descriptions or remove.
4. **`docs/system-architecture.md`** — same sweep; if it has a deployment
diagram or section, replace CF box with GH Pages.
5. **`docs/code-standards.md`** — likely just a passing CF mention; replace
or remove. If it references `inject-csp-hashes.mjs`, drop that.
6. **`docs/development-roadmap.md`** — replace CF references with GH Pages.
7. **`docs/project-overview-pdr.md`** — replace CF references with GH Pages.
8. **`plans/todo.md`** — under "PWA install verification":
- Delete "Lighthouse — Cloudflare Pages (root base)" subsection entirely.
- Keep "Lighthouse — GitHub Pages (`/loto/` base)" as the sole production
check.
- Delete the "CSP + headers (production)" subsection (no longer applicable).
- In "Common gotchas", remove the CSP / `_headers` references.
- Drop the "CSP hash brittleness" entry under "Tech debt".
## Success Criteria
- [ ] `grep -ri 'cloudflare\|wrangler\|_headers\|_redirects\|loto\.miti99\.com' docs/ README.md plans/todo.md` returns nothing (or only intentional leftovers documented in this plan).
- [ ] `docs/deployment-guide.md` describes only GH Pages.
- [ ] `plans/todo.md` no longer has CF-specific Lighthouse / CSP entries.
- [ ] `docs/deployment-guide.md` "Last reviewed" updated.
## Risk Assessment
- **Risk:** Doc sweeps miss a reference and downstream readers get confused.
**Mitigation:** The grep success-criterion is the safety net.
- **Risk:** `docs/code-standards.md` or `docs/system-architecture.md` describe
the CSP hash injection as a code-standard. Removing without reading
context could leave a dangling concept (e.g. "we ship strict CSP" claims).
**Mitigation:** Read each doc fully before editing; rewrite affected
paragraphs rather than deleting sentences mid-thought.
web/plans/260509-2247-switch-deploy-to-github-pages/plan.md
-41
View File
@@ -1,41 +0,0 @@
---
title: Switch deploy target from Cloudflare Pages to GitHub Pages
description: >-
Make GitHub Pages the canonical deploy at tiennm99.github.io/loto. Drop CF
Pages, _headers, _redirects, CSP-hash injection. Keep it simple — static site,
no security-headers machinery.
status: completed
priority: P2
created: 2026-05-09T00:00:00.000Z
---
# Switch deploy target from Cloudflare Pages to GitHub Pages
## Overview
Today CF Pages is canonical (`loto.miti99.com`) and GH Pages serves a redirect HTML
to it. Flip that: make GH Pages do a real build of `npm run build:gh` (basePath
`/loto`) and serve the app at `https://tiennm99.github.io/loto/`. Remove CF
artifacts (`wrangler.toml`, `static/_headers`, `static/_redirects`, CSP-hash
injection scripts) since GH Pages can't honor them and the user opted to keep
it simple for a static site.
## Phases
| Phase | Name | Status |
|-------|------|--------|
| 1 | [Wire GH Pages build into CI](./phase-01-wire-gh-pages-build-into-ci.md) | Completed |
| 2 | [Remove Cloudflare artifacts](./phase-02-remove-cloudflare-artifacts.md) | Completed |
| 3 | [Update docs and TODO](./phase-03-update-docs-and-todo.md) | Completed |
## Dependencies
None. Sequential within plan: phase 1 → 2 → 3 (CI must work before docs declare
the new flow). Phase 2 can land in same PR as phase 1 since they touch
different files.
## Post-merge manual step
Disable the Cloudflare Pages project from the CF dashboard so it stops
auto-building from `main`. Optionally remove the `loto.miti99.com` DNS record
or repoint it (out of scope for this plan).
web/plans/reports/brainstorm-260430-2024-both-mode-edge-cases.md
-331
View File
@@ -1,331 +0,0 @@
# Brainstorm — "Both Mode" Edge Cases & Inconsistencies
Date: 2026-04-30
Scope: Hypotheses only. No code verification. Cold-eyes triage list for the team.
Repo: /config/workspace/tiennm99/loto
Mode model recap:
- `mode = "player" | "master" | "both"` — in `both`, MasterPanel + PlayerBoard mount on same page.
- Master draw → `broadcastDraw(num)` writes `{num, at: Date.now()}` to in-memory bus.
- Player `$effect` watches `bus.lastDrawn`; `processAutoTick` dedupes by `at`, advances `lastHandledAt` on every new `at`.
- Master state in `localStorage["loto_master"]`; player state in prefixed keys (`loto_grid`, `loto_crossed`); bus NOT persisted.
- Auto-call interval, voice (master-call + player "Chờ N"/"Kinh"), AutoCountdown component all overlay this.
Legend: critical = data loss / can't recover game state | major = wrong gameplay outcome / user confusion | minor = cosmetic / rare | unknown = needs spike.
---
## 1. Bus history loss on player board regeneration (CONFIRMED)
- Risk: **major**
- Why: Player presses "Tạo bảng mới" mid-game → fresh grid won't auto-cross numbers already called pre-regenerate, because bus only carries `lastDrawn`. Player must manually cross or wait for next draw.
- Verify: `PlayerBoard.svelte` regenerate handler + `auto-tick.js` (no replay of `master.called`).
## 2. Bus is in-memory only → reload silently desyncs both panels
- Risk: **critical**
- Why: After page reload in `both` mode, `loto_master.called` rehydrates but bus.lastDrawn is null. If player grid had uncrossed numbers from before reload, no auto-replay happens — player relies on master's *next* draw to ever fire `$effect`. Player display "last called" pill / "Chờ N" toast goes blank vs master's visible called list.
- Verify: `call-bus.svelte.js` initial state + `+page.svelte` mount order + `PlayerBoard.svelte` `$effect`.
## 3. `lastHandledAt` not persisted → after reload first new draw may double-process or be skipped
- Risk: **major**
- Why: If `lastHandledAt` lives in component state only, post-reload it resets to 0/null. On next master draw the `$effect` will process it, but if `lastDrawn` already exists from a *previous* in-memory state mid-session, behavior is undefined. Worse: auto-tick may treat a stale `at` as fresh.
- Verify: `auto-tick.js` dedup logic + where `lastHandledAt` is held (component vs store).
## 4. Two broadcasts in same millisecond → one dropped
- Risk: **minor** (low likelihood) but **major** when it bites
- Why: `at: Date.now()` has 1ms resolution. Auto-call at high speed or clock with low-res timers (some Windows VMs) → identical `at` → dedup-by-`at` discards second broadcast. Number called by master never reaches player auto-cross.
- Verify: `call-bus.svelte.js` broadcastDraw, `auto-tick.js` dedup; consider monotonic counter instead of `at`.
## 5. System clock backwards jump → `at` stops advancing
- Risk: **minor**
- Why: NTP correction or user changing clock can make new `at` ≤ old `at`. Dedup-by-`at` would treat new draw as old → skipped silently.
- Verify: `call-bus.svelte.js`; recommend `performance.now()` or sequence number.
## 6. Rapid mode toggle player→both→master→both during a draw
- Risk: **major**
- Why: Player `$effect` may re-mount/unmount mid-tick. If `broadcastDraw` fires while PlayerBoard is unmounted (mode=master), player misses it; on toggling back to both, bus.lastDrawn is the missed number but `at` may already be ≤ player's `lastHandledAt` if it was persisted, or it gets re-processed if not.
- Verify: `+page.svelte` conditional rendering + lifecycle of PlayerBoard `$effect`.
## 7. Mode toggle wipes wrong state
- Risk: **major**
- Why: Switching to player mode and back may clear master's auto-call interval but not its `loto_master`, OR may unmount MasterPanel mid-auto-call leaving an orphan `setInterval` that keeps broadcasting. Either way, "both"-mode timing breaks.
- Verify: `MasterPanel.svelte` onMount/onDestroy, AutoCountdown lifecycle, `settings-store.svelte.js` mode transitions.
## 8. Auto-call interval not cleared on mode change / route change
- Risk: **major**
- Why: If MasterPanel registers `setInterval` but only clears in onDestroy, switching to a route that doesn't unmount it (SvelteKit nav can keep layout) leaves it firing. Numbers keep getting called against a hidden master.
- Verify: `MasterPanel.svelte` interval cleanup, `+layout.svelte`.
## 9. Auto-call speed change mid-run
- Risk: **minor**
- Why: Changing `autoCallSpeed` while interval running typically requires clear+set; if implemented as reactive `$effect` watching speed, may double-register, leaving old interval ticking + new one. Player gets bursts.
- Verify: `MasterPanel.svelte` (or wherever interval is owned) + `settings-store.svelte.js`.
## 10. AutoCountdown drift vs actual interval
- Risk: **minor**
- Why: Countdown likely uses `setInterval(1000)` or rAF; setInterval throttled in background tabs to ≥1s, rAF paused. Master's draw interval may also be throttled. Visible countdown can desync from actual draw firing — user sees "0s" but draw fires 5s later.
- Verify: `AutoCountdown.svelte`, visibility handling.
## 11. Tab backgrounded → setInterval throttling
- Risk: **major**
- Why: Backgrounded host tab → master's auto-call throttled to 1Hz min (sometimes paused). Player on same tab is also throttled. When tab refocuses, multiple draws may fire in burst, voice queue overflows, and `lastHandledAt` skips multiple `at`s — but only ONE will be processed (only the latest is in bus).
- Verify: visibilitychange handlers (likely none); `MasterPanel.svelte` interval; `voice.js` queueing.
## 12. Voice queue collisions in "both" mode
- Risk: **major**
- Why: Master speaks "Số N" and player speaks "Chờ N" / "Kinh" from same `speechSynthesis` queue on same page. Either they serialize (lag, "Chờ" announced 5s after the call) or one cancels the other (`speechSynthesis.cancel()` typical pattern). Either is wrong UX.
- Verify: `voice.js` — does it `cancel()` before `speak()`? Single shared queue?
## 13. "Kinh" (bingo) speaks twice — once on auto-cross, once on detection
- Risk: **minor/major**
- Why: If bingo is detected both during `processAutoTick` (auto-cross caused win) and during render `$derived(bingo)`, two voice triggers may fire. Or worse, "Chờ N" speaks for the winning number first, then "Kinh" — confusing.
- Verify: `PlayerBoard.svelte` bingo detection effect, `voice.js`, ordering vs `processAutoTick`.
## 14. "Chờ N" toast/voice fires for stale numbers after Tạo bảng mới
- Risk: **major**
- Why: After regenerate, board has new numbers. If `processAutoTick` re-runs against current bus.lastDrawn (because `lastHandledAt` reset), it announces "Chờ N" for a number that was called minutes ago — misleading the player it's a fresh call.
- Verify: regenerate handler + auto-tick re-run logic.
## 15. Master "Ván mới" doesn't reset player
- Risk: **major**
- Why: Master clears `loto_master.called/remaining/last`. Player still has `loto_grid + loto_crossed` from previous game. Player sees old marks, no new "Chờ" because bus now empty. Need explicit player reset signal — does the bus carry "reset" event?
- Verify: MasterPanel "Ván mới" handler; bus contract; PlayerBoard listening for reset.
## 16. Master "Ván mới" while player mid-bingo
- Risk: **minor**
- Why: Player has bingo state showing, master starts new game, player still announcing "Kinh" while master draws number 1 of new game. Voice collision + player's bingo ribbon stays.
- Verify: bingo state lifecycle, reset propagation.
## 17. Player "Xoá đánh dấu" doesn't replay history either
- Risk: **major** (same root cause as #1)
- Why: Clears `crossed` only. Master's already-called list is still valid but player won't re-cross them automatically. Player must re-cross by hand.
- Verify: PlayerBoard clear handler.
## 18. localStorage shape drift after settings refactor
- Risk: **major**
- Why: Old users with `loto_master` v1 schema (e.g. array vs object, missing `last`) load on new code → JSON.parse succeeds but destructure yields undefined → app crashes or silently breaks (called list shows blank). No version field visible from filename.
- Verify: `settings-store.svelte.js` parse + default-merge; check for `schemaVersion` field; wrap parse in try/catch with reset fallback.
## 19. localStorage quota exceeded / disabled (private mode, Safari)
- Risk: **minor**
- Why: Setting `loto_master` throws `QuotaExceededError`. If unhandled, mode toggling and game state silently fails to persist; reload returns blank board. Master-only writes might succeed while player writes fail (or vice versa) → divergence.
- Verify: try/catch around localStorage.setItem in stores; user-facing error toast?
## 20. Multiple tabs of the host
- Risk: **critical**
- Why: Two tabs both running master = two independent bus instances (in-memory, per-tab). Each writes to same `loto_master` localStorage key, last-write-wins, called numbers from one tab overwrite the other. Player in tab A sees draws from tab A only. No `storage` event listener to reconcile.
- Verify: any `window.addEventListener('storage', ...)`; document this is unsupported or add BroadcastChannel.
## 21. Storage event in another tab triggers $effect cascade
- Risk: **unknown**
- Why: If reactive store does subscribe to storage events, cross-tab edits could rehydrate state mid-draw, replacing `called` array out from under MasterPanel render — visible flicker, possible double-add.
- Verify: settings-store + master state hydration.
## 22. ARIA live regions stack on auto-call
- Risk: **minor**
- Why: Each draw probably writes to `aria-live="assertive"` (called pill, "Chờ N" toast, master's call display). At 2s/draw, screen reader queues 3+ announcements per number → unintelligible.
- Verify: any `aria-live="assertive"` in PlayerBoard / MasterPanel / AutoCountdown; prefer `polite` or single region.
## 23. AutoCountdown announces every second to AT
- Risk: **minor**
- Why: If countdown digit is in an aria-live region, it will read "5… 4… 3…" every tick. Annoying and pre-empts important call announcement.
- Verify: `AutoCountdown.svelte` aria attributes.
## 24. Bingo detection runs on every cross including auto
- Risk: **minor**
- Why: Auto-cross from `processAutoTick` flips a cell → bingo `$derived` recomputes → if it triggers a side-effect (voice "Kinh", confetti) inside an `$effect` that also fires for manual crosses, the path may differ subtly. E.g. on manual cross, master's draw display has updated; on auto-cross it has too — but order of effects between voice "Chờ" and bingo detection is the question.
- Verify: PlayerBoard ordering of `$effect`s.
## 25. processAutoTick advances lastHandledAt even when number not on grid
- Risk: **minor** (intentional) but **major** if combined with regenerate
- Why: Stated behavior: dedup advances on every new `at`, even if no cell flipped. Fine — until player regenerates and now the number IS on grid but `lastHandledAt` has already moved past `at`. No retro-cross happens. Same root as #1.
- Verify: `auto-tick.js`.
## 26. PWA service worker serves stale JS, but localStorage is fresh
- Risk: **major**
- Why: User had v1 app open, we deploy v2 with new bus contract. SW caches v1 assets; localStorage has v2 schema written by another device or refresh-on-other-tab. v1 code reads v2 data → crash or wrong rendering. Or vice versa.
- Verify: `service-worker.js` (if exists), Workbox/SvelteKit PWA config, schemaVersion.
## 27. PWA offline: bus state lost, called list survives
- Risk: **major**
- Why: User goes offline, app keeps running from SW cache. Page reload offline — works. But bus history lost on every reload, divergence becomes more frequent because user reloads more often without connectivity feedback.
- Verify: SW + #2.
## 28. Visibility change: rejoin race
- Risk: **major**
- Why: Tab returns from background. Master's interval was throttled → catches up by firing draws back-to-back. Player `$effect` sees rapid `at` increments but only the LAST `lastDrawn` is in bus → all intermediate numbers silently lost from auto-cross perspective, but they ARE in master's `called` list. Massive divergence.
- Verify: visibilitychange handler; need bus to be a queue or to replay from `master.called`.
## 29. `$effect` re-runs on grid change post-cross
- Risk: **minor**
- Why: If `processAutoTick` is called from an `$effect` that depends on both `bus.lastDrawn` and `grid`, swapping the grid triggers re-run with same `lastDrawn` — but `lastHandledAt` already advanced, so it's a no-op. Confirm dedup is robust to this.
- Verify: PlayerBoard `$effect` deps list.
## 30. Two PlayerBoards on page (future / accidental)
- Risk: **unknown**
- Why: If `both` mode somehow renders both routes' PlayerBoard or component is reused, both subscribe to bus; both write to the same `loto_grid` key → last-write-wins, crossed cells flicker.
- Verify: `+page.svelte` + `+layout.svelte`; current code likely single mount but worth checking.
## 31. Master's "remaining" pool out of sync with "called"
- Risk: **major**
- Why: If "called" array is updated optimistically before "remaining" splice (or vice versa) and a render happens between, draw next could pick a number already called. Especially under React-style batching that Svelte 5 may or may not apply.
- Verify: MasterPanel draw handler; consider single transactional update.
## 32. Manual call entry vs auto-call collide
- Risk: **major**
- Why: If master can manually enter a number while interval is running, two `broadcastDraw` paths exist. They could fire in the same ms (#4) or out of order. Also, manual entry might bypass "remaining" pool update.
- Verify: MasterPanel manual call (if exists) + auto-call interaction.
## 33. Voice "Chờ N" speaks for number not on player's board
- Risk: **minor**
- Why: If "Chờ N" is announced whenever a draw happens regardless of whether it's on grid (vs the intent: announce only when player needs to wait/has it). Spec ambiguity — "Chờ" = "wait for N"? clarify.
- Verify: `voice.js` + PlayerBoard call site.
## 34. Speech synthesis voice not loaded yet
- Risk: **minor**
- Why: `voiceschanged` event fires async. First few calls may speak with default voice (English) instead of Vietnamese. Especially on mobile Safari where voices load on first user gesture.
- Verify: `voice.js` voice selection + fallback.
## 35. iOS audio policy: needs user gesture
- Risk: **major** for iOS users
- Why: Auto-call interval fires draw without user gesture → speechSynthesis silent on iOS Safari. User thinks voice is broken. Especially in `both` mode where master never clicked draw button after enabling auto-call.
- Verify: `voice.js`; consider primer gesture.
## 36. settings-store mode=both with stale prefix
- Risk: **minor**
- Why: `storagePrefix` setting (player keys like `loto_grid`) could be edited while in both mode; old keys remain orphaned in localStorage; new prefix has empty grid; player auto-rehydrates blank.
- Verify: settings-store prefix change handler.
## 37. broadcastDraw called with non-number / 0 / NaN
- Risk: **minor**
- Why: Auto-tick dedup runs but cell match `grid.includes(NaN)` returns false; lastHandledAt advances. Player silently ignores. But voice announces "Số NaN".
- Verify: type guards in `call-bus.svelte.js` + `voice.js`.
## 38. Negative auto-call speed / 0
- Risk: **minor**
- Why: If user sets autoCallSpeed=0 in settings (or via DevTools), `setInterval(fn, 0)` = ~4ms minimum, draws 90 numbers in ~1 sec. Bus only retains last; ALL but final auto-cross lost.
- Verify: settings-store validation min/max.
## 39. Master draws 90 numbers, then 91st click
- Risk: **minor**
- Why: Empty `remaining` array → draw next fails silently or throws. If interval is still on, it fires every Ns hitting empty array — does it auto-stop?
- Verify: MasterPanel handleDrawNext when remaining empty + interval guard.
## 40. processAutoTick runs in master mode (mode=master)
- Risk: **minor**
- Why: Player effect should be guarded by mode!==master. If guard is missing or off-by-one ("both" treated as master), auto-cross runs but no PlayerBoard renders, `lastHandledAt` advances pointlessly. Not data-corrupting but wastes work.
- Verify: `processAutoTick({mode})` mode check.
## 41. lastHandledAt advancement inside test vs production
- Risk: **unknown**
- Why: `auto-tick.test.js` exists — if tests pass with mocked Date.now but production uses real Date.now plus throttling, test coverage may not catch #28 / #11.
- Verify: `auto-tick.test.js`; add throttling/burst test.
## 42. Confetti / celebration replay on reload after bingo
- Risk: **minor**
- Why: Bingo state is `$derived(crossed)` → on reload, crossed rehydrates → bingo true → confetti fires again. Annoying.
- Verify: PlayerBoard bingo effect + flag like "celebrated".
## 43. Both mode disables one panel by mistake
- Risk: **minor**
- Why: A `if (mode === 'master')` instead of `if (mode === 'master' || mode === 'both')` on a master button hides it in `both` mode. Vice versa for player. Small typos with three-way enum.
- Verify: every `mode ===` check across components.
## 44. Settings change persists before save
- Risk: **minor**
- Why: SettingsButton may use 2-way binding directly to store instead of staging — toggling mode in dialog immediately remounts panels behind the dialog. UX confusion + state loss if user cancels.
- Verify: `SettingsButton.svelte` binding model.
## 45. Reload during auto-call
- Risk: **major**
- Why: Auto-call running, user F5. `loto_master` saved up to last draw. On reload, auto-call interval is NOT auto-restarted (probably) — game appears paused without indication. Or auto-restarted from autoplay setting → first draw fires with no UI feedback yet.
- Verify: MasterPanel onMount + `autoCall` setting persistence.
## 46. Network/CDN-cached audio mismatch
- Risk: **minor**
- Why: `audio-manifest.js` may reference numbered MP3s; if some 404 due to cache miss, voice fallback to TTS for some numbers and audio for others — inconsistent UX.
- Verify: `audio-manifest.js` + `voice.js` fallback chain.
## 47. Master's "called" history > UI display window
- Risk: **minor**
- Why: After 50+ calls, called list display may overflow / paginate. If player only sees last N, reload doesn't help — but that's master display only. Just confirm.
- Verify: MasterPanel called list rendering.
## 48. Reactive cycle: $effect → state change → $effect re-runs
- Risk: **major**
- Why: If `processAutoTick` mutates `lastHandledAt` AND the `$effect` reads it, infinite loop possible. Svelte 5 has guards but they're not free — perf hit, console warnings.
- Verify: `auto-tick.js` return contract + how PlayerBoard wires it.
## 49. "Kinh" voice on regen-induced auto-cross sweep (if #1 is fixed)
- Risk: **major** (forward-looking)
- Why: If team fixes #1 by replaying `master.called` on regen, the replay might trigger 5+ auto-crosses, last one a bingo, which speaks "Kinh" instantly when user hits "Tạo bảng mới" — startling and wrong (didn't actually win this round).
- Verify: any future replay logic; suppress voice during replay.
## 50. AutoCountdown shows when auto-call disabled
- Risk: **minor**
- Why: Recently added component — if its mount logic doesn't guard on `autoCall` setting, it shows stale "0s" countdown when manual mode active.
- Verify: `AutoCountdown.svelte` mount conditions.
## 51. processAutoTick assumes `crossed` is mutable Set
- Risk: **unknown**
- Why: If crossed is a `$state` reactive Set, mutating in-place vs replacing affects reactivity. Auto-tick may flip cell but PlayerBoard not re-render.
- Verify: `auto-tick.js` mutation strategy + PlayerBoard.
## 52. localStorage write thrash during auto-call
- Risk: **minor**
- Why: Each draw writes `loto_master`. At 2s cadence x 90 draws = 90 writes. Player on same tab also writes `loto_crossed`. Combined with reactive sync (every state change writes), localStorage is hot. Devices with slow storage hitch every draw.
- Verify: persistence layer in stores; consider debounce.
## 53. Dialog/Modal stealing focus during draw
- Risk: **minor**
- Why: SettingsButton dialog open while auto-call fires — focus trap doesn't know about toast/announcement; SR users may miss draws.
- Verify: SettingsButton focus management.
## 54. Reload during call-bus dispatch (race)
- Risk: **minor**
- Why: User hits F5 between `master.called.push(N)` and `broadcastDraw(N)`. localStorage has N in called, bus never broadcast. After reload bus is empty anyway (#2) so net effect is same divergence — but called list now contains N that was never voiced.
- Verify: MasterPanel handleDrawNext atomicity.
## 55. Settings test coverage (false confidence)
- Risk: **unknown**
- Why: `settings-store.test.js` is 369 lines but tests probably mock localStorage. Real-world quota / disabled storage / private mode untested → #19 lurks.
- Verify: test file scenarios.
## 56. broadcastDraw before player mounts (initial both-mode)
- Risk: **minor**
- Why: First render of `+page.svelte` mounts MasterPanel and PlayerBoard. Order matters: if MasterPanel onMount triggers a draw (e.g. resume autoplay) before PlayerBoard `$effect` registered, player misses #1.
- Verify: mount order; defer master autoplay to next tick.
## 57. `Date.now()` in test environment vs SSR
- Risk: **minor**
- Why: SvelteKit may SSR `+page.svelte`. `Date.now()` differs between server and client → hydration mismatch warnings if `at` is used in rendered output.
- Verify: any direct render of `at` value; SSR config.
---
## Triage Recommendation (top 10 to fix first)
1. **#2 Bus reload desync** + **#1 regenerate replay**: root-cause fix = persist `lastHandledAt` AND replay `master.called` on regenerate/reload. Single shared design.
2. **#28 Visibility burst loss**: bus must become queue or pull from `master.called` since last `at`.
3. **#15 Master "Ván mới" doesn't reset player**: define explicit "session reset" signal on bus.
4. **#20 Multiple host tabs**: at minimum show warning, ideally BroadcastChannel.
5. **#12 Voice queue collisions**: define ownership — only player speaks "Chờ/Kinh" after master finishes "Số N". Or pick one speaker in both mode.
6. **#18 Schema drift**: add `schemaVersion`, parse defensively.
7. **#35 iOS gesture**: prime audio on first user click.
8. **#11 Background throttling**: visibilitychange listener to flush queue / pause auto-call.
9. **#7 Mode-toggle interval orphan**: audit interval/effect cleanup.
10. **#43 Three-way enum typos**: grep all `mode ===` usages, normalize to helper `isHost(mode)` / `isGuest(mode)`.
---
## Unresolved questions
- Q1: Is `lastHandledAt` persisted? (drives #3, #14, #25)
- Q2: Does master's "Ván mới" emit any signal player can react to, or is it localStorage-only? (drives #15)
- Q3: What is the exact spec of "Chờ N" — fired for every draw, or only when N is on grid and uncrossed? (drives #33)
- Q4: Is auto-call resumed on reload? (drives #45)
- Q5: Is there any `addEventListener('storage')` for cross-tab? (drives #20, #21)
- Q6: Does bus have any "reset" / "session" event type, or only number broadcasts? (drives #15, #49)
- Q7: How does `processAutoTick` handle `mode==="master"` — early return or run anyway? (drives #40)
- Q8: Is voice serialized via `cancel()+speak()` or queued? (drives #12, #13)
- Q9: Is there a schemaVersion in any localStorage payload? (drives #18, #26)
- Q10: SSR — does `+page.svelte` actually render dynamic state on server, or fully client-only? (drives #57)
web/plans/reports/brainstorm-260430-2114-voice-and-multitab.md
-140
View File
@@ -1,140 +0,0 @@
# Brainstorm: Voice ownership in both mode + Multi-tab guard
**Date:** 2026-04-30
**Repo:** tiennm99/loto (SvelteKit, client-only, localStorage)
**Scale:** ~8 source files, single page, PWA installable
**Principles:** YAGNI / KISS / DRY
---
## Topic 1 — Voice ownership in both mode (F9)
### Problem recap
In both mode, master `playNumber(N)` and player `playWaiting(M)` share `cancelPlayback()`. Whichever fires last cancels the other mid-syllable. Worst case: master speaks "bốn mươi hai" and 200ms later "Chờ 42" stomps it. User wants `voiceWaitingNumber` suppressed in both mode regardless of setting.
### Options table
| # | Option | LoC | UX change | Failure modes | Test surface | Solves F9? |
|---|--------|-----|-----------|---------------|--------------|------------|
| a | Pure default + explicit gate: in both mode treat `voiceWaitingNumber` as false | ~3 lines (one ternary in PlayerBoard or in `playWaiting`) | Chờ becomes bare word in both mode; settings UI may still show toggle (confusing) | Settings UI lies if toggle stays user-editable in both mode → must hide/disable toggle too (~10 LoC SettingsButton) | 1 unit test (playWaiting in both mode skips number clip) | Yes, partially — collision still possible for "Chờ" alone vs "42" |
| b | Voice-owner enum `voiceOwner: "off"\|"master"\|"player"` | ~50–80 LoC (settings-store rewrite, migration, SettingsButton UI rework, all consumers) | Cleaner mental model; player loses ability to opt out of master voice without going silent everywhere | Migration churn for users with existing settings; product semantics for both mode unclear (master says all, player says all, or some hybrid?) | Settings migration tests + voice consumer tests + UI tests | Indirectly — only if "master" owner suppresses player Chờ entirely (different product) |
| c | Audio queue / serializer `voice-queue.js` | ~40–60 LoC new module + cancel semantics rework + integration in 3 sites | All clips play sequentially; total latency grows (Chờ delayed up to ~1.5s after number) | Queue grows unbounded if calls fire faster than playback; "Ván mới" needs flush; cancel semantics get murky | New module + queue tests + integration tests | Yes, but at cost of timing |
| d | Event-priority guard: master `playNumber` is "high", suppress Chờ/Kinh while master clip in-flight | ~10 LoC in `voice.js` (track activeKind, drop low-priority calls when high active) | Chờ/Kinh dropped silently if number announcement still playing | "Dropped" Chờ never gets a second chance — player misses the audio cue entirely | 2–3 unit tests in voice.test.js | Yes, fully — number wins by design |
| e | Coalescer: drop Chờ N if N was just announced within 1s | ~15 LoC + timestamp tracking | Smartest UX but invisible heuristic | Hard to reason about; window tuning is fiddly; doesn't help when waitingNumber ≠ lastCalled | New tests for window edge cases | Partially — only when called == waitingNumber |
| f | Hybrid (a) + (d) | ~15 LoC total | Bare "Chờ" word + master always wins audio | Combined behaviour but two rules to reason about | Tests for both rules | Yes, fully |
### Recommendation: **Option (a) — pure gate + hide toggle**
**Why (a) over (d)/(f):**
- F9 is about a *specific* product confusion ("Chờ + number" sounding like a second call), not a general collision problem. The collision between bare "Chờ" and a previous number announcement is acceptable (Chờ is the final state cue and it's only 300ms).
- (d) priority guard *silently drops* Chờ — bad for the player who never hears it. The current "cancel last wins" is actually OK if Chờ is short.
- (b) is over-engineering for a one-page app — YAGNI.
- (c) queue introduces 1+ second latency to Chờ which defeats its purpose as a real-time hint.
- (a) is 3 lines + UI hide. Documents the rationale. Reversible.
**Sketch:**
```js
// src/lib/voice.js — playWaiting:
const speakNumber = settings.voiceWaitingNumber && settings.mode !== "both";
// ^^^^ added guard
// src/lib/SettingsButton.svelte — hide the "Chờ + số" toggle when mode === "both"
{#if settings.mode !== "both"}
<label>... voiceWaitingNumber checkbox ...</label>
{/if}
```
**One unresolved nit:** if user enables `voiceWaitingNumber` in solo player mode, then switches to both, the setting persists silently. Acceptable — switching back restores it. Document in code comment.
---
## Topic 2 — Multi-tab guard (#20)
### Problem recap
Two tabs in mode "both" both run auto-call intervals, both write `loto_master`, both speak audio. User wants new tab to silence old tab. Need to clarify: only one **master** tab, or only one tab period?
### Decision: scope of "lock"
The actual damage vectors are:
1. **Auto-call interval double-fire** (mode=both, autoCallEnabled=on) — corrupts state + double audio
2. **Two voice playbacks** — same audio twice
3. **Two writes to `loto_master`** — last-write-wins is benign for `called[]` if both observe the same draws, but if both draw independently → divergence
Pure player mode doesn't write to `loto_master` and doesn't draw — having two viewer tabs is harmless. So the lock should be on **master/both** tabs, not all tabs.
But the user spec says "old tab should do nothing, stop all actions" — simplest interpretation is **only one tab period, regardless of mode**. KISS reading.
### Options table
| # | Option | Browser support | LoC | UX | Failure modes | Solves spec? |
|---|--------|-----------------|-----|-----|----------------|--------------|
| a | BroadcastChannel API | Modern (Safari 15.4+, all Chrome/FF) — safe for Cloudflare Pages target | ~30 LoC: 1 channel, claim/relinquish msgs, banner | Cleanest; near-instant cross-tab signal | iOS Safari <15.4 (~3% global, mostly old iPads) silently no-op; tab crash leaves no relinquish msg (but new tab claim wins anyway) | Yes |
| b | `storage` event listener | Universal (IE9+) | ~40 LoC: write `loto_active_tab` token on focus, listen for changes | Works everywhere; ~5–50ms latency | Same-tab `storage` events don't fire (must update local state manually); two tabs at exact same ms → both write, last-write-wins | Yes |
| c | Web Locks API (`navigator.locks`) | ~95% (Safari 15.4+) | ~25 LoC: request lock with `ifAvailable`, hold for tab lifetime | Native single-writer guarantee | iOS <15.4 no-op; lock auto-releases on tab close (good); doesn't notify old tab proactively (must combine with BC) | Partial — guard but no UX feedback |
| d | Tab id + timestamp watchdog on every write | Universal | ~50 LoC: tab id in every state mutation + check on read | Works everywhere | Adds overhead to every write; very chatty; complexity creeps; race window during simultaneous writes | Yes but ugly |
| e | Doc/UI warning only (`document.hasFocus()` heuristic) | Universal | ~5 LoC | No protection, just a sign | Doesn't actually fix anything — auto-call still double-fires | No |
| f | Hard takeover with confirm dialog on new tab | Modern (any of above) | ~40 LoC | Friendly, reversible | Confirm dialog on every new tab is annoying for the rare honest case | Yes |
| g | Disable specific actions in non-active tab (auto-call + draws) | Modern | ~30 LoC + per-action gates | Allows viewing in non-active tab | Two surfaces to gate (master draws, voice playback) → spreading concern across files | Partial |
### Recommendation: **Option (a) BroadcastChannel + frozen-banner**
**Why (a) over (b)/(c)/(g):**
- BroadcastChannel is purpose-built for this; cleanest API. Chromium/Firefox/Safari 15.4+ all support it.
- iOS Safari ≥15.4 covers nearly all PWA users (PWA on iOS requires ≥16.4 anyway for proper installability). Pre-15.4 fallback: ignore the guard — those users are <3% and the existing race is rare.
- (b) `storage` event also works but lacks the same-origin "active tab" semantics — you'd reinvent BroadcastChannel on top of it.
- (c) Web Locks gives you the lock but doesn't give you the *banner UX* — you still need a side channel.
- (g) per-action gating spreads logic across MasterPanel + voice + state writes — violates KISS.
- The user spec ("old tab should do nothing, stop all actions") aligns with the simple frozen-banner — no per-feature gating.
**Reversibility:** when new tab closes, send a "released" broadcast → old tab unfreezes. Or simpler: old tab also re-claims on `visibilitychange` → focus → if no contender responds in 200ms, take back over.
**Sketch (~30 lines, single new module `src/lib/tab-lock.js`):**
```js
// src/lib/tab-lock.js
const CHANNEL = "loto_tab_lock";
const TAB_ID = crypto.randomUUID();
/** @param {() => void} onFrozen */
export function startTabLock(onFrozen) {
if (typeof BroadcastChannel === "undefined") return () => {};
const bc = new BroadcastChannel(CHANNEL);
// Announce ourselves; any other tab will hear and freeze itself.
bc.postMessage({ type: "claim", id: TAB_ID });
bc.onmessage = (e) => {
if (e.data?.type === "claim" && e.data.id !== TAB_ID) onFrozen();
};
return () => bc.close();
}
```
Mount in `+layout.svelte`:
```svelte
let frozen = $state(false);
$effect(() => startTabLock(() => { frozen = true; }));
```
Render frozen banner when `frozen === true`, replacing the page or overlaying full-screen with "Loto đang mở ở tab khác. Nhấn để kích hoạt lại tab này." → on click, postMessage claim again to take over.
**Side-effect kill:** in the frozen state, no need to clean up auto-call interval — the user can also just close the tab. But if we want a clean stop, add an effect: `$effect(() => { if (frozen) autoRunning = false; })` in MasterPanel (1 line). Voice naturally stops because `cancelPlayback` is called on PlayerBoard unmount, and freezing replaces the layout.
### What this does NOT solve
- **PWA installed on phone:** typically only one window, so no multi-tab scenario at all → guard is silent no-op. Fine.
- **Two devices on same Wi-Fi:** different localStorage origins per device — out of scope (no shared state to corrupt).
- **iOS <15.4 users:** no BroadcastChannel → no guard. Acceptable (rare, and the existing race is rare too).
---
## Combined unresolved questions
1. **Topic 1, settings UI:** when `mode === "both"`, should the "Chờ + số" toggle in SettingsButton be hidden, disabled-with-tooltip, or left alone? Hiding is cleanest but can confuse users who change settings, then change mode and wonder where the toggle went. Disabled-with-tooltip preserves discoverability but adds 5 LoC of tooltip plumbing. Recommend: **hidden** (KISS). Confirm with user.
2. **Topic 1, scope of suppression:** should the `mode === "both"` guard live in `voice.js` (closer to the cancellation root cause) or in `PlayerBoard.svelte` (closer to the behaviour decision)? Recommend: `voice.js` so it can't be bypassed by future call sites.
3. **Topic 2, lock granularity:** confirm whether the lock should fire in **all modes** (player included), or only when the active tab has master capabilities (mode `master` or `both`). User's spec is ambiguous. Recommend: **all modes** (KISS, matches user words "stop all actions").
4. **Topic 2, banner copy:** Vietnamese wording for the freeze banner. Suggest: "Loto đã mở ở tab khác. Tap để chuyển về tab này." — confirm tone (sếp-em vibe?).
5. **Topic 2, takeover behaviour:** if user clicks the banner in old tab to reclaim, should the new tab freeze (handover) or both stay live (race continues)? Recommend: handover via the same `claim` message. New tab's mount listener catches the new claim and freezes itself. Symmetry holds.
6. **Topic 2, fallback for old Safari:** silent no-op (current behaviour preserved) vs. visible warning ("Trình duyệt cũ — có thể xung đột giữa các tab")? Recommend: silent. Edge case noise not worth it.
web/plans/reports/brainstorm-260430-2131-waiting-cell-indicator.md
-183
View File
@@ -1,183 +0,0 @@
# Brainstorm: "Chờ N" Waiting Cell Indicator
**Date:** 2026-04-30 21:31
**Context:** User wants the Chờ N indicator (a) inside the board, (b) translucent so cells stay readable, (c) with an animation on the actual cell holding number N.
---
## Current State Recap
- Toast `Chờ N` floats above grid (`-top-3 sm:-top-4`), amber-500/95, 5s `animate-toast`.
- Section label band already gets a persistent amber inset-ring + 2.4s `section-pulse` when any of its 3 rows is in waiting state.
- Voice "Chờ N" plays when announce flag is on.
- Grid cell holding N has **zero** visual treatment today. Section label only narrows the search to ~27 cells (3 rows × 9 cols).
- Multi-row Chờ is real: up to 9 rows could be in waiting state simultaneously (rare but possible). Toast currently single-slot, replaces previous.
---
## Design Space — 9 Options
### a. Cell-scoped pulse only (drop toast)
- **Clarity:** High — eye is drawn straight to the cell. No translation needed.
- **Read/write:** Zero — pulse is on the cell itself, doesn't cover anything.
- **Cost:** ~10 LoC. One `$derived` Set<`r,c`> of waiting cells, one `.cell-waiting` class with keyframe.
- **Mobile:** Excellent. Nothing to dismiss, nothing covered.
- **Reduced-motion:** Trivial — fall back to static amber ring (no animation).
- **Multi-row:** Scales linearly. 9 amber pulses on a 9×9 = noisy but informative.
- **Risk:** No textual cue → blind/low-vision/cognitive-load users lose Chờ N affordance. Voice + section ring partially mitigate. Toast users may miss the explicit number callout.
### b. Cell pulse + minimal centered chip
- **Clarity:** Good. Chip says number; pulse shows location.
- **Read/write:** Chip in dead-zone (gap between sections, top-right) is a pure overlay; ~30% opacity = readable beneath. Could still smudge cells if mispositioned.
- **Cost:** ~20 LoC. Cell pulse + small absolute-positioned chip with auto-fade.
- **Mobile:** Good. Chip takes ~50px square.
- **Reduced-motion:** Cell pulse → static ring; chip → no animation, just static.
- **Multi-row:** Chip can stack/queue; pulse scales fine.
- **Risk:** Two indicators = mild redundancy. Two CSS knobs to tune.
### c. Centered overlay banner (amber 70% opacity + arrow)
- **Clarity:** High initially, but the arrow is a layout nightmare across 9 possible cell positions and section boundaries.
- **Read/write:** Big overlay blocks taps unless `pointer-events: none`. Even translucent, it visually covers ~9–15 cells.
- **Cost:** ~50 LoC. Arrow geometry math, opacity tuning.
- **Mobile:** Banner crowds small screens.
- **Reduced-motion:** Banner static-only — fine.
- **Multi-row:** Multiple arrows = chaos. Single banner can't pluralize gracefully.
- **Verdict:** Over-engineered. Violates KISS.
### d. Big number ghost (huge faded digit centered over card)
- **Clarity:** Fast pattern-match for the number. No location info — user still has to scan.
- **Read/write:** A 8rem ghost @ 30% opacity over ~half the grid degrades cell legibility (especially for already-crossed red strokes).
- **Cost:** ~15 LoC. One absolute-positioned div with big text.
- **Mobile:** Same digit overlay = visually loud on small screens.
- **Reduced-motion:** No animation needed.
- **Multi-row:** Cannot show 3+ ghost numbers without becoming a soup.
- **Verdict:** Charming but flunks multi-row case + cell legibility.
### e. Cell pulse + smaller toast at bottom-center of card
- **Clarity:** Good. Two reinforcing cues.
- **Read/write:** Toast at bottom = below content, no obstruction.
- **Cost:** ~5 LoC change to existing toast position + add cell pulse.
- **Mobile:** Bottom-toast risks overlapping the next page section / footer.
- **Reduced-motion:** Existing fallback covers.
- **Multi-row:** Same single-toast limitation as today.
- **Verdict:** Closest to "minimum change". Reasonable B-option.
### f. Cell pulse + tooltip on hover
- **Mobile:** Hostile — taps cross cells, not show tooltips. Long-press conflicts with the swipe/tap UX.
- **Verdict:** Reject. Bad for primary use case.
### g. Sweep-light effect
- **Cost:** Heavy (~40 LoC + GPU motion).
- **Reduced-motion:** Must fully disable; users get no fallback indicator at all unless we layer pulse.
- **Multi-row:** 9 simultaneous sweeps = seizure territory.
- **Verdict:** Reject. Overkill, fails accessibility.
### h. Combo (a) + (d) — ghost number + cell pulse
- All of (d)'s cell-legibility issues persist.
- Two animations to coordinate.
- **Verdict:** Worse than either alone.
### i. Floating arrow bouncing at the cell
- Adds a 4th moving element (cells, slashes, section ring already animate).
- Arrow geometry per-cell again.
- **Verdict:** Reject. Visual noise budget exceeded.
---
## Comparison Matrix
| Option | Clarity | Obstruction | Cost | Mobile | RM-friendly | Multi-row |
|--------|---------|-------------|------|--------|-------------|-----------|
| **a. Cell pulse only** | High | None | Low | Best | Yes | Scales |
| b. Cell pulse + chip | High | Tiny | Low-Med | Good | Yes | Good |
| c. Banner + arrow | Med | High | High | Crowded | OK | Bad |
| d. Big ghost number | Med | High | Low | Crowded | OK | Bad |
| e. Pulse + bottom toast | High | None | Lowest | Med | Yes | Same as today |
| f. Hover tooltip | Low | None | Low | **Bad** | OK | OK |
| g. Sweep light | Med | Med | High | OK | **Bad** | **Bad** |
| h. a+d combo | Med | High | Med | Crowded | OK | Bad |
| i. Bouncing arrow | Med | Low | Med | OK | OK | Bad |
---
## Recommendation: **Option (a) — Cell-scoped pulse, drop the toast**
**Why this and not (b)/(e):**
- **YAGNI:** The toast text duplicates info already conveyed by (1) voice "Chờ N", (2) section ring narrowing region, (3) the pulse itself drawing eye to the cell. Three converging cues = textual chip is redundant.
- **KISS:** One mechanism, one CSS keyframe, one `$derived`. No positioning math, no z-stacking, no obstruction debate.
- **DRY:** Mirrors the existing `section-label-waiting` pattern — same amber, same `prefers-reduced-motion` opt-out, same cognitive model. Users already learn "amber = Chờ" from the section ring.
- **Multi-row friendly:** 9 amber pulses degrade gracefully; 9 toasts don't.
- **Brutal truth:** The user asked for 3 things (move inside, opacity, animation). Pulse satisfies all three without an overlay at all — the cell IS in the board, the pulse animates the cell, and there's nothing to fade because nothing covers anything.
**Acknowledged trade-off:** Drops the explicit textual "Chờ N" callout for sighted users who don't enable voice. Mitigation: voice already covers this; for the silent-mode minority, the section ring + cell pulse gives precise location. If user testing reveals the number itself is missed, fall back to **Option (b)** by adding a small chip — but ship (a) first, see if anyone complains. (Add chip later costs ~10 LoC.)
---
## Implementation Sketch (~12 LoC)
**`PlayerBoard.svelte`** — derive a Set of waiting cell coords:
```svelte
<script>
// Set of "row,col" strings for cells holding a row's awaited number.
const waitingCells = $derived.by(() => {
const s = new Set();
if (!grid || !crossed.length) return s;
grid.forEach((row, r) => {
if (rowCompleteness[r]) return;
const n = getWaitingNumber(grid, crossed, r);
if (n === null) return;
const c = row.indexOf(n);
if (c >= 0) s.add(`${r},${c}`);
});
return s;
});
</script>
```
In the cell render, add `cell-waiting` class when `waitingCells.has(`${row},${col}`)`. Drop or keep the toast — recommendation: **delete the toast block** entirely (lines 466-484 + `toast` state + `showToast`/`dismissToast`/`toastTimer`).
**`app.css`** — pulse keyframe, mirrors `section-pulse`:
```css
.cell-waiting {
animation: cell-waiting-pulse 1.6s ease-in-out infinite;
box-shadow: inset 0 0 0 3px rgb(245 158 11 / 0.7);
}
@keyframes cell-waiting-pulse {
0%, 100% { box-shadow: inset 0 0 0 3px rgb(245 158 11 / 0.45); }
50% { box-shadow: inset 0 0 0 3px rgb(245 158 11 / 0.95),
0 0 8px 2px rgb(245 158 11 / 0.5); }
}
@media (prefers-reduced-motion: reduce) {
.cell-waiting { animation: none; box-shadow: inset 0 0 0 3px rgb(245 158 11 / 0.7); }
}
```
**Notes:**
- Inset ring keeps cell footprint stable (no layout shift).
- The outer amber glow at 50% adds extra "look here" without colliding with neighboring cells (8px halo dies at the cell border).
- Compatible with existing red-on-not-yet-crossed and crossed states — `box-shadow` layers above background, doesn't replace it.
- The section-label-waiting ring stays — it's the regional cue; cell pulse is the precise cue. Two-tier hierarchy mirrors how players actually scan: section first, then row, then cell.
---
## Cleanup To Do
If toast is dropped:
- Remove `toast`, `toastTimer`, `showToast`, `dismissToast` from `PlayerBoard.svelte`.
- Remove `showToast(...)` call at line 173.
- Remove `dismissToast()` from `handleGenerate`/`handleClear`/unmount.
- Remove toast HTML block (lines 466-484).
- Remove `@keyframes toast` and `.animate-toast` from `app.css`.
Net diff: ~+12 / −35 LoC. Codebase shrinks.
---
## Unresolved Questions
1. Should the cell pulse stop firing once user-tapped (false-positive scenario: user clicks the awaited number, it crosses, pulse vanishes — already handled by `waitingCells` recompute via `crossed` reactivity)?
2. Color contrast on dark-mode emerald-on-amber overlap if a row is mid-completion: the cell can be amber-pulsing AND already-crossed red (rare; only if we're waiting on a different cell same row — impossible by definition since `getWaitingNumber` returns null if all but one are crossed). Confirmed safe.
3. Should we keep the toast as a fallback behind a setting `settings.showWaitingToast`? Recommendation: **No** — YAGNI. Add it only if a user requests it.
web/plans/reports/code-review-260426-1919-loto-project.md
-213
View File
@@ -1,213 +0,0 @@
# Code Review — loto (dev branch, 260426-1919)
Scope: `app/page.tsx`, `app/master/page.tsx`, `app/loto-player-board.tsx`, `app/loto-game-logic.ts`, `app/globals.css`, `app/layout.tsx`, `next.config.ts`, `package.json`, `.github/workflows/deploy.yml`, `.env.example`, `.gitignore`, `eslint.config.mjs`, `README.md`. ~1.2k LOC. Static-export Next.js 16 SPA, no backend, localStorage persistence only.
---
## Replace-or-keep verdict
**KEEP.** Architecture is sound for the scope: a static SPA with localStorage and no auth/network. No rewrite is justified. Top concrete improvements:
1. Fix the toast/race bug in `loto-player-board.tsx` (multiple eligible rows reset `notifiedWaitingRows` while the toast effect early-returns — see HIGH-1).
2. Validate `JSON.parse` outputs from localStorage (shape + dimension checks) — currently a single hand-edited key crashes the render.
3. Memoize `isRowComplete` per row — currently called 81× per render in `PlayerBoard`.
4. Add ARIA/keyboard support to grid cells (currently `<div onClick>` only — not focusable, not announced).
5. Split `master/page.tsx` (244 lines) into `use-master-state` hook + `<MasterBoard>` + `<CalledHistory>` components.
---
## CRITICAL
None. No data-loss path, no remote auth/security boundary (static export, no server). No dependency on user-supplied URLs.
---
## HIGH
### HIGH-1. Race-y toast / “Chờ X” suppression — `loto-player-board.tsx:72-97`
The detection effect uses `return;` after the *first* completed row or first new waiting row. Consequences:
- If two rows transition to "waiting" simultaneously (one click can do this only when grids overlap, but more importantly **on mount** — see HIGH-2 — multi-row recovery is fine because the seed loop in `useEffect` `46-65` populates the set first; but during *gameplay* with overlapping numbers, the second row never gets a "Chờ X" toast on the click that would have triggered it because `return` exits before the loop reaches it). Next state change re-runs the effect and surfaces it, so it self-heals — but in practice it means the second eligible row stays silent until *another* state change.
- Bigger issue: the “delete” branch at `89-95` runs only for indices the early-return loop *reaches*. Cross a row back from waiting → not-waiting → re-waiting in that order while a *lower-index* row is currently waiting, and the higher-index reset branch is never executed → second waiting toast is suppressed indefinitely.
Fix: split the loop into two passes — first recompute desired sets for all rows, then fire one notification.
### HIGH-2. `isRowComplete` returns `true` for empty/zero-cell row — `loto-game-logic.ts:108-117`
The "any cell with value > 0 must be crossed" check returns `true` if no positive cells exist. `generateGrid` always emits 5 numbers/row so the live path is safe, but:
- Master `BOARD` has rows where `col===0 && row===9` etc. set to 0; they aren't passed to `isRowComplete` today, but anything that imports the helper for a different grid shape could trip it.
- More immediate: if `loadGrid` returns a corrupted grid (e.g. user's localStorage edited to all zeros), the seed loop at `loto-player-board.tsx:56-59` would mark every row as celebrated, and the very first `crossed` change would skip the celebration animation for the real win.
Fix: add `let hasNumber = false;` guard, return `false` when no positive cells.
### HIGH-3. `loadGrid` / `loadCrossedState` accept any JSON shape — `loto-game-logic.ts:83-105`, `app/master/page.tsx:51-59`
`JSON.parse` is wrapped in try/catch, but the parsed value is returned as-is and trusted as `number[][]` / `boolean[][]` / `MasterState`. Hand-edited or stale-from-older-version localStorage will crash the render:
- `crossed[row]?.[col]` (`loto-game-logic.ts:114`) is defensive, but `grid[row][col]` (`isRowComplete` line 114) is not — `grid[row]` could be `undefined` (`grid` shorter than 9 rows), throwing inside render.
- `loadState()` in master could return `{}` and `state.called.length` (`app/master/page.tsx:69`) throws.
- Cross-version risk: if the grid algorithm changes, old saves become wrong-shape but still parse.
Fix: validate shape (`Array.isArray`, lengths, element type) before returning. Drop & log on mismatch.
### HIGH-4. `crossed[][]` / `grid[][]` dimension drift — `loto-player-board.tsx:46-65`
`saveGrid` and `saveCrossedState` are written separately. If `setGrid(newGrid)` succeeds but the next render throws before `saveCrossedState` runs (e.g. browser kills tab), the next session loads a new grid with stale crossed dimensions from a *prior* grid. `crossed[row]?.[col]` masks this for booleans, but a 9×9 grid paired with a 9×5 or 10×9 crossed array silently misreports row completion.
Fix: store both under one key as `{ grid, crossed, version }`, write atomically. Or always re-init `crossed` to all-false on grid load when shapes don't match.
### HIGH-5. `BOARD` mutation hazard via shared module-scope reference — `app/master/page.tsx:61`
`BOARD = buildBoard()` is a module-level mutable nested array. Today nothing mutates it. But any future "click-to-strike" UI that mutates `BOARD[r][c]` would persist across HMR and hot-reload between routes. Freeze with `Object.freeze` on rows or compute inside the component (cheap — 90 ints).
### HIGH-6. Performance: `isRowComplete` called 81× per render — `loto-player-board.tsx:144`
Inside `grid.flat().map(...)`, line 144 calls `isRowComplete(grid, crossed, row)` for every cell. That's 81 calls per render, each scanning 9 cells = 729 reads per render. Trivial today, but on every keystroke/click. Pre-compute `const completedRows = useMemo(() => grid.map((_, i) => isRowComplete(grid, crossed, i)), [grid, crossed])`.
### HIGH-7. `key={idx}` on history pills — `app/master/page.tsx:170`
Acceptable here (history is append-only). However `key={idx}` is also used on grid cells (`loto-player-board.tsx:148`, `master/page.tsx:190`) — for the player grid this *will* break React's reconciliation if `generateGrid` ever returns a different cell ordering between renders (it doesn't, but the contract isn't enforced). Use stable keys derived from `row*9+col` (which equals idx today, so functionally identical — but documents intent).
### HIGH-8. basePath/assetPrefix prod hardcode — `next.config.ts:23`
`basePath = isProd ? "/loto" : ""`. If the GH Pages repo is renamed, or someone deploys to a custom domain (apex), every asset 404s. Pull from `process.env.NEXT_PUBLIC_BASE_PATH` with `/loto` as fallback. Also: `output: "export"` is set unconditionally — `next start` (`package.json:9`) is meaningless for an exported build. Either remove the script or document.
---
## MEDIUM
### MED-1. `randomNumbersInCol` Fisher-Yates is biased — `loto-game-logic.ts:46-49`
`arr.sort(() => 0.5 - Math.random())` is a well-known biased shuffle (V8 sort is not guaranteed pairwise-symmetric). Fine for a casual game, but use a real shuffle (the same one in `master/page.tsx:39-43`) for fairness. DRY: extract one `shuffle<T>(arr: T[]): T[]`.
### MED-2. `confirm()` blocks during render — `loto-player-board.tsx:100`, `master/page.tsx:82`
Native `confirm()` is synchronous and blocked by some browsers (Safari iframe, in-app webviews). Replace with the existing modal pattern (already used for "Kinh!" popup) for consistency and reliability.
### MED-3. `randomARow` mutates caller's `baseWeight` — `loto-game-logic.ts:32-42`
`baseWeight[col]--` mutates the array passed by reference. The caller (`generateGrid:57`) creates a fresh array each call so it's safe today, but the function signature lies. Either rename to `randomARow(baseWeight, mutate=true)` or take/return a copy.
### MED-4. `state.remaining[0]` always drawn — `master/page.tsx:90`
The shuffle is done once at game start, then `remaining` is consumed FIFO. That's deterministic given the initial shuffle. Functionally fine, but "Xổ số" feels less random — consider `Math.floor(Math.random() * remaining.length)` per draw to make each draw visibly random (no algorithmic difference for fairness, just UX perception).
### MED-5. Toast effect dep on `showToast` causes re-runs — `loto-player-board.tsx:97`
`showToast` is `useCallback([dismissToast])` and `dismissToast` is `useCallback([])`, so identity is stable. OK, but adding any future dep would cause double-fires. Document or extract toast logic into a custom hook.
### MED-6. `master/page.tsx` hosts both a `loto_master` (called numbers) and `loto_master_card` (master's own card) key — naming collision risk
A user-side bug where the user navigates to `/master`, generates a master card, then clears storage by clicking "Tạo bảng mới" *only* clears `loto_master_card_*` not `loto_master`. That's correct, but the visual cue (orange palette) doesn't tell the master "the called-number state is independent of your card." Add a small UI hint or rename for clarity.
### MED-7. master/page.tsx is 244 lines — modularization candidate
Per project rules (>200 LOC). Suggested split: `app/master/use-master-state.ts` (state + persistence), `app/master/master-board.tsx` (the 9×10 tracking grid), `app/master/called-history.tsx` (chips). That brings each file under 100.
### MED-8. README is 14 lines, missing dev:codeserver instructions — `README.md`
The new codeserver profile is non-obvious. Add a section explaining `.env.local` setup, `CODESERVER_HOST/PORT`, and the `/absproxy/{port}` URL.
### MED-9. Accessibility — grid is unreachable by keyboard
- `<div onClick>` is not focusable, no `role="button"`, no `aria-pressed={isCrossed}`, no `aria-label="Số 42, đã đánh dấu"`, no `tabIndex={0}`, no Enter/Space handler.
- Congrats modal (`loto-player-board.tsx:192-232`) has no `role="dialog"`, no `aria-modal`, no focus trap, no Escape-to-close.
- Toast has no `aria-live="polite"`.
- Color contrast: `text-slate-400 dark:text-slate-500` (`master/page.tsx:154`, `211`) on `dark:bg-slate-900` likely fails WCAG AA. The diagonal-line cross-out (`globals.css:80-91`) is a single hue (`#ef4444`) — colorblind users may miss it; the bg-color change provides redundancy, OK.
### MED-10. No tests at all
There's no `__tests__/` or `*.test.ts`. `generateGrid`, `isRowComplete`, `getWaitingNumber`, `randomANumberInRow` are pure and trivially testable. Property-based test on `generateGrid`: every row has 5 numbers, every column count ≤ 6, all numbers in range, no duplicates.
### MED-11. CSP / iframe headers not set
Static export + GH Pages → no CSP. App is embedded-friendly which means clickjacking-friendly. Low impact (no auth, no money, no PII), but document or add `<meta http-equiv="Content-Security-Policy">` in `layout.tsx`.
### MED-12. `Math.random()` in `loto-game-logic.ts:24,47` — not cryptographic but called "random"
Fine for a game. Document so a future dev doesn't think it's secure.
---
## LOW
### LOW-1. Dead code: `notifiedWaitingRows` reset branch only triggers when `waitNum===null && notified && !celebrated` — `loto-player-board.tsx:89-95`
A row that completes will never hit this branch because `celebrated.has(i)` blocks it. Add a comment, or refactor: when a row becomes complete, remove from `notifiedWaitingRows` (already done at line 78) and rely on the check.
### LOW-2. `package.json` name is `nextjs-temp` — pre-rename leftover. Rename to `loto`.
### LOW-3. `master/page.tsx:184` `BOARD.flat()` allocates per render. Wrap in `useMemo(() => BOARD.flat(), [])` or compute once at module scope.
### LOW-4. `app/page.tsx:14` "TN1 (2014–2017)" hard-codes copy in the component. If localization is ever added, extract.
### LOW-5. `globals.css:67` typo-prone — `.animate-spin-slow-reverse` reuses `spin-slow` keyframe + `reverse` direction. Works but two classes named almost identically (`spin-slow` vs `spin-slow-reverse`) is brittle.
### LOW-6. `isRowComplete`/`getWaitingNumber` hardcode `col < 9` — `loto-game-logic.ts:113,126`. Use `NUM_COLS` constant for consistency.
### LOW-7. `app/master/page.tsx:79` `const calledSet = new Set(state?.called ?? []);` rebuilt every render. `useMemo` (cheap — 90 elements — so LOW).
### LOW-8. `app/page.tsx:65` `rel="noopener noreferrer"` is good. But `master/page.tsx:233` has the same external link duplicated — extract `<Footer />`.
### LOW-9. `next-env.d.ts` was changed in this branch. That file is auto-regenerated and shouldn't be hand-edited; ensure the change is benign (it imports `./.next/dev/types/routes.d.ts` which is the new Next 16 typegen). Verify the file isn't gitignored on other contributors' machines.
### LOW-10. `eslint.config.mjs:14` ignores `next-env.d.ts` but the file is now actually committed and modified — confirm intent.
---
## Edge cases (adversarial)
| Scenario | Behavior | Severity |
|---|---|---|
| `localStorage` is disabled / quota exceeded | `setItem` throws, uncaught → unhandled exception during state update. | HIGH |
| User edits `loto_grid` to `"hello"` | `JSON.parse` throws, caught, returns `null`. OK. | OK |
| User edits `loto_grid` to `[[1,2]]` (1 row, 2 cols) | `isRowComplete` reads `grid[1][0]` → `undefined.0` → TypeError on render. | HIGH-3 |
| User opens `/` and `/master` in two tabs of same browser | Both tabs write to same `loto_grid` key from `/`, but `/master` uses `loto_master_card_grid` for its own card, so they don't overlap. The user's main card on `/` is shared. NO `storage` event listener — second tab won't update. | MED |
| User opens `/` twice in two tabs | Both write to `loto_grid` — last writer wins, no sync. Mutation in tab A invisible in tab B until refresh. | MED |
| `generateGrid` called on a partially-completed board | `handleGenerate` always overwrites grid + resets crossed. `confirm()` guards. OK. | OK |
| `crossed[][]` shorter than `grid[][]` | `crossed[row]?.[col]` returns undefined → falsy → row never marked complete. Safe but wrong-state. | HIGH-4 |
| `crossed[][]` longer than `grid[][]` | Excess rows ignored. Safe. | OK |
| Click during congrats modal | Cell click works (grid is not aria-hidden, modal is a fixed overlay). User can keep marking. Minor UX: modal blocks clicks via backdrop, but `e.stopPropagation` on inner div allows close. OK. | OK |
| Codeserver host changes mid-session | basePath is baked at build/dev-start. `next dev` must restart. Document. | LOW |
| GH Pages deploy when repo renamed | `/loto` 404. | HIGH-8 |
| Two rows reach "waiting" on the same click | Only first row's "Chờ X" toast shown; second silent until next click. | HIGH-1 |
---
## Security
- **XSS via stored values:** All user-controlled data is rendered as `{num}` (numeric) or `{toast}` (template literal `Chờ ${num}`) — both pass through React's escaping. The "Hàng X đã đầy đủ" message uses `congratsRow` which is a number. No `dangerouslyInnerHTML` anywhere. **Safe.**
- **Prototype pollution via JSON.parse:** `JSON.parse` does not pollute Object.prototype by itself, but `loadCrossedState` returns the raw parsed value. If a malicious actor can control localStorage (i.e. user's own browser — not a real attacker model for a static SPA), they can set `{"__proto__": {...}}` but `JSON.parse` ignores `__proto__` as a data key in modern engines (V8 since long ago). **Safe by current engine behavior**, but a defensive `structuredClone` or shape validator (HIGH-3) makes it explicit.
- **Codeserver dev origin leakage:** `allowedDevOrigins: [host]` with `host` from `.env.local` — no validation. If `CODESERVER_HOST` is set to `*` or a malformed value, Next will accept it. Add a sanity check (must match `/^[a-z0-9.-]+$/i`). Low impact (dev only, never shipped to prod). **Low.**
- **Secrets:** `.env.local` is gitignored. `.env.example` only contains hostnames. `CODESERVER_HOST/PORT` are not `NEXT_PUBLIC_*` so they are server-side only — but `next.config.ts` consumes them at build time and bakes `basePath` into the static bundle. The basePath itself (`/absproxy/3000`) is visible in HTML. Hostname is NOT baked — only used for `allowedDevOrigins`. **Safe.**
- **GH Actions:** `permissions:` is correctly minimal. No third-party actions beyond `actions/*` v4. `npm ci` against committed `package-lock.json` (verify lockfile is committed — not visible in this review; check).
- **No CSRF / no auth / no PII** — N/A.
---
## Positive observations
- Clean separation of pure logic (`loto-game-logic.ts`) from React state.
- `useCallback` and `useRef` used appropriately for stable identity.
- Vietnamese-first UI is consistent.
- `prefix`-based localStorage namespacing is the right call for the new master-card feature — avoids the user-board / master-card collision cleanly.
- Tailwind + CSS keyframes is lighter than dragging in Framer Motion.
- TS strict (assumed from `eslint-config-next/typescript`) and no `any` in app code.
- `output: "export"` matches the deploy target (GH Pages static).
- The new codeserver profile is well-documented inline (`next.config.ts:6-10`) — comment explains *why* `/absproxy` not `/proxy`.
---
## Recommended actions (priority order)
1. **HIGH-1**: Rewrite the row-detection effect as two passes (compute → notify) to fix toast suppression.
2. **HIGH-3 + HIGH-4**: Add a `validateGrid(g): g is number[][]` and `validateCrossed(c, g): c is boolean[][]` helper. Use in `loadGrid`/`loadCrossedState`/`loadState`. On invalid: drop the key and return null.
3. **HIGH-2**: Add empty-row guard to `isRowComplete`.
4. **HIGH-6**: Memoize `completedRows` in `PlayerBoard`.
5. **HIGH-8**: Read `basePath` from `NEXT_PUBLIC_BASE_PATH` env with `/loto` fallback; document in README.
6. **MED-9**: Add ARIA + keyboard handlers to grid cells (role=button, tabIndex, aria-pressed, aria-label, Enter/Space). Add role=dialog + focus trap to congrats modal. Add aria-live=polite to toast.
7. **MED-7**: Split `master/page.tsx` into hook + 2-3 components.
8. **MED-1 + MED-3**: DRY up shuffle into one helper, use it everywhere.
9. **MED-10**: Add Vitest + a property test for `generateGrid` invariants.
10. **LOW-2 + MED-8**: Rename package, expand README.
---
## Metrics
- LOC reviewed: ~1.2k (app code only).
- Type coverage: 100% explicit (no `any`).
- Test coverage: 0% (no tests).
- Lint: not run in this review (ask `tester` agent if needed).
- File-size violations: 1 (`master/page.tsx` 244 LOC > 200 limit).
---
## Unresolved questions
1. Is `package-lock.json` committed? (Required for `npm ci` reproducibility — couldn't see it in the diff stat.)
2. Is the GH Pages deploy guaranteed to be at `/loto`, or is a custom domain planned? Determines whether HIGH-8 is real or theoretical.
3. Does the master "play along" feature (HIGH `loto_master_card`) need to interact with the called-numbers state (e.g. auto-cross master's card when a number is called)? Right now they're fully independent — verify this is intended.
4. Should the user's grid sync across tabs (`storage` event listener)?
5. Is i18n on the roadmap, or is Vietnamese-only acceptable long-term?
web/plans/reports/code-reviewer-260427-1036-three-mode.md
-68
View File
@@ -1,68 +0,0 @@
# Code Review — three-mode + auto-tick + master Chờ/Kinh
## Scope
- Files: settings-store(.svelte.js + .test.js), SettingsButton.svelte, +page.svelte, call-bus(.svelte.js + .test.js), MasterPanel.svelte, PlayerBoard.svelte, voice.js
- Focus: three-mode picker, legacy migration, master→player draw bus, auto-tick gating, master voice for Chờ/Kinh in `both`
- Test/typecheck: 106/106 passing; `svelte-check` 0 errors / 0 warnings
## Overall Assessment
Clean, small, idiomatic Svelte 5. Bus-as-rune is the right primitive — fresh-object trick (`{num,at}`) handles repeats correctly. Set-only auto-tick eliminates manual-vs-auto fights by construction. Migration is pure and one-shot via `saveSettings`. One real test bug found; everything else is minor.
## Critical Issues
None.
## High Priority
### 1. Stale `masterMode` assertion in `settings-store.test.js:189` (vacuous test)
```js
expect(settings.masterMode).toBe(DEFAULT_SETTINGS.masterMode);
```
Both sides are `undefined` after the `mode` rename — the assertion passes for the wrong reason and gives false coverage. Replace with `expect(settings.mode).toBe(DEFAULT_SETTINGS.mode);` or delete (the per-key fallback is already covered elsewhere).
## Medium Priority
### 2. Auto-tick effect early-return collapses dependency tracking
`PlayerBoard.svelte:134` reads `bus.lastDrawn` first (good) but the subsequent `settings.mode !== "both"` early-return means Svelte stops tracking on that path. In practice this works because the *next* draw retriggers via `bus.lastDrawn` and re-reads `settings.mode`. The user-stated intent ("changing mode mid-game tears down/re-arms cleanly") holds for *future* draws, **not retroactively** — a number drawn while in `master` mode and then switching to `both` will not back-fill ticks. If retroactive backfill is desired, tick from `state.called` on mode change. If not (likely correct UX — auto-tick is forward-only), add a one-line comment so a future reader doesn't think it's a bug.
### 3. Master voice cancellation collides with player voice in `both` mode
Both panels share the same `voice.js` singleton (`activeClip`/`activeToken`). Sequence in `both`+both-voices-on: master draws → `playNumber(n)` starts → player auto-tick fires the same render → if the auto-tick completes a row, `playBingo()` cancels the in-flight number clip. End-user effect: "Bingo!" wins, number announcement gets cut off. This is arguably correct (Kinh is more important), but the tradeoff is undocumented. Consider: (a) leave + comment, or (b) queue Kinh after current clip via a small chain. KISS says leave it — but flag it in CHANGELOG so the "voice cuts off sometimes" report doesn't surprise you.
### 4. Voice flag composition repeated as inline expression
`PlayerBoard.svelte:98-99` recomputes `voiceEnabledPlayer || (voiceEnabledMaster && mode === "both")` inline. If a third caller appears, factor a `$derived` on `settings` (stays one place). Skip for YAGNI.
## Low Priority
- `+page.svelte:42` — `transition:slide` on the master section is fine but plays on every mount including initial page load. Acceptable; minor.
- `call-bus.svelte.js` — module-level `$state` is fine for SvelteKit SSR (no client state leak across requests since it's reset on each app instance), but a comment noting this would help future readers who worry about it.
## Edge Cases (scout)
- **Repeat draw**: bus publishes new object → effect re-runs → set-only guard finds cell already true → no-op. Correct.
- **`Ván mới` mid-auto**: `MasterPanel.handleNewGame` calls `cancelPlayback()` + `autoRunning=false` + `resetBus()`. Bus reset to `null`, player effect early-returns. Correct.
- **Mode flip mid-game from `both`→`master`**: PlayerBoard unmounts → `cancelPlayback()` cleanup runs → master clip stops. Master's `playNumber` next draw re-acquires audio. Correct.
- **Migration idempotency**: `saveSettings` writes `mode:"both"` and drops `masterMode` key. Subsequent loads take the `validMode` branch. Correct.
- **localStorage QuotaExceeded mid-save**: silently swallowed (existing behavior). OK.
## Test Coverage Gaps
- No test asserts `bus.lastDrawn` integration with PlayerBoard auto-tick (would require a Svelte component test harness — currently the suite is unit-only). Given KISS, a single component-level integration test is worth it: render PlayerBoard with a fixed grid, set `settings.mode="both"`, call `broadcastDraw(n)`, assert `crossed[r][c]===true`. This is the only un-tested branch of the new feature.
- No regression test for "auto-tick does NOT fire in `master` or `player` mode" — easy to add alongside the above.
## Positive Observations
- Per-key validation pattern preserved on the new `mode` field — adding it didn't break the existing per-key fallback contract.
- Fresh-object publish pattern (`{num, at: Date.now()}`) is the right primitive for "fire on every draw including repeats."
- Set-only auto-tick eliminates an entire class of state-fight bugs without locking.
- `voice.js` token + `activeResolver` cancellation is correct under interleaved cancel/play and was not regressed.
- Migration is one-shot via natural `saveSettings` write — no extra "version" plumbing. KISS.
## Recommended Actions
1. Fix vacuous `masterMode` assertion (test line 189) — high.
2. Add one component-level integration test for the auto-tick path — medium.
3. Add 1-line comment in PlayerBoard auto-tick `$effect` clarifying "forward-only, no backfill" — medium.
4. CHANGELOG note: in `both` mode, completing a row may interrupt the number callout — low.
## Metrics
- Tests: 106/106 pass
- svelte-check: 0 errors, 0 warnings
- New LOC: ~+50 net (call-bus + tests + PlayerBoard effect)
## Unresolved Questions
- Is master-clip-cut-by-bingo intentional or worth a queue? (Q for product owner — UX call, not a bug.)
- Should mode-flip retroactively auto-tick already-called numbers? (Current: no. If "no" is intentional, add comment; otherwise small change to seed from `state.called` once on mode→both.)
web/plans/reports/code-reviewer-260427-1151-full-project.md
-77
View File
@@ -1,77 +0,0 @@
# Code review — full project (260427-1151)
Scope: `src/` SvelteKit + Svelte 5. Skip `node_modules`/`.svelte-kit`/`build`.
Focus: runes correctness, bus/state coordination, voice cancellation, edge cases, JSDoc, tests, anti-patterns.
Overall: code is clean, well-commented, JSDoc-typed throughout. One real auto-tick re-mark bug; rest are minor.
## P0 — blocking
**1. Auto-tick re-marks cells the user cleared / unticked / regenerated** — `src/lib/PlayerBoard.svelte:136-151`.
The auto-tick `$effect` reads `bus.lastDrawn` AND `crossed` AND `grid` reactively. So it re-fires when `crossed` changes (manual untick, "Xoá đánh dấu", "Tạo bảng mới") even though no new draw happened. With `bus.lastDrawn` still set, it scans for that number and re-marks it. Reproductions in "both" mode:
- Master draws 42. User taps 42 to untoggle → effect re-runs, sees `!crossed[r][c]`, re-marks. User cannot ever untick the latest draw.
- User taps "Xoá đánh dấu" → all marks clear → effect re-fires → 42 re-marked instantly.
- User taps "Tạo bảng mới" → new `crossed` matrix → effect re-fires → 42 marked on the new card.
Fixes: track `lastDrawn.at` (or the bus reference) in a non-reactive ref and early-exit when unchanged; OR call `resetBus()` from `handleClear`/`handleGenerate` AND skip the effect on the very first run after generate. Same hazard exists conceptually with the comment "auto-tick can't fight manual taps" — comment is wrong.
## P1 — high
**2. `handleClear` / `handleGenerate` don't reset `bus.lastDrawn`** — same file, lines 153-177. Even after fixing #1, the bus stays sticky across player-card resets. Master `handleNewGame` does reset it (`MasterPanel:139`); player resets should too in "both" mode (or at minimum, snapshot `lastDrawn.at` so post-clear effect knows to ignore stale draws).
**3. `handleClear` doesn't reset `congratsRow`/`celebrationTier` and leaves `showCongrats`** — `PlayerBoard.svelte:167-177` sets `showCongrats = false` but if a celebration was mid-animation the state is fine. However `congratsRow`/`celebrationTier` aren't reset; harmless until next bingo overwrites. P2.
**4. Multiple-row simultaneous bingo silently swallowed** — `PlayerBoard.svelte:102-112`. Pass 1 `break`s after first new completion, but pass 2 won't surface another. Not reachable today (one cell-toggle = one row at most). But auto-tick + cross-row coincidence + future logic changes could expose it. Add a short queue or track "pending celebrations" if you ever batch-mark.
**5. Bingo modal Escape only works when backdrop has focus** — `PlayerBoard.svelte:393-402`. `onkeydown={onModalKeydown}` is on the backdrop `<button>`; opening the modal doesn't auto-focus it, so Escape often doesn't fire. Settings modal solved this with a window listener (`SettingsButton.svelte:107-115`); replicate.
**6. MasterPanel auto-stop branch sets state during effect synchronously** — `MasterPanel.svelte:99-103`. `if (!autoRunning || !settings.autoCallEnabled) { autoRunning = false; return; }` writes the same value when `autoRunning` already false; harmless but produces a redundant effect re-run. Minor correctness — early-return without write when already false.
**7. `loadSettings` is not idempotent for theme listener if called twice without DOM teardown** — `settings-store.svelte.js:88-110`. `applyTheme` defensively clears prior listener via module-scoped `mql/mqlListener`, but module is shared across HMR boundaries. In production fine; in dev hot-reload the previous module's listener can leak. Not a prod concern. P2.
## P2 — medium
- **Smooth-scroll-on-draw ignores `prefers-reduced-motion`** — `MasterPanel.svelte:127-128`. Auto mode at 1s/draw forces page jumps. Gate on `matchMedia('(prefers-reduced-motion: reduce)')`.
- **`navigator.vibrate(10)` on every cell click** — `PlayerBoard.svelte:184-186`. No reduced-motion gate; minor.
- **`toastTimer` not cleared on component unmount** — leaks if PlayerBoard unmounts while toast pending. Wrap in `$effect(() => () => dismissToast())`.
- **`onModalKeydown` only handles Escape** — settings modal uses window listener pattern; bingo modal should match for consistency.
- **`CONFETTI` placement is deterministic** — `(i*8.3 + (i%3)*11) % 100` — fine, but visually clusters. Cosmetic.
- **`generateGrid` uses `Math.random()` based shuffle** — `game-logic.js:31-34` does `arr.sort(() => 0.5 - Math.random())` which is biased. Replace with Fisher-Yates (already used in `pickFilledColsOnce:106-109` and `MasterPanel.svelte:35-38`). Bias is small but real and breaks the "uniform pick" assumption.
- **`scrollOnNextDraw` is a plain `let` in a Svelte 5 component** — works because the gating effect is triggered by `lastCalled`, but reads non-reactively. Document or move into ref-style helper.
- **Settings modal lacks focus trap and focus restoration** — clicking gear, then Escape, returns focus to body, not the gear button. Minor a11y.
- **`storagePrefix` prop on PlayerBoard is unused (always default)** — `PlayerBoard.svelte:22`. YAGNI hook. If keeping, document it; otherwise drop.
## P3 — low / nits
- `celebrationTier` typed `1 | 2` but only `>= 3` switches to 2; type and threshold name mismatch — fine, but could be `'normal' | 'big'`.
- `BOARD_FLAT` is computed at module load in `<script module>` — good, just note it persists across HMR.
- `audio-manifest.js:23` `DEFAULT_VOICE = VOICES[0]?.id ?? "hoai-my"` — fallback string isn't validated against `VOICE_IDS` if manifest is empty; impossible today but fragile.
- `cancelPlayback` resets `currentTime` after `pause()` — order is fine, but the cached element keeps `onended`/`onerror` nulled; next `playClip` reattaches. OK.
## Tests
Coverage strong on `game-logic`, `settings-store`, `vietnamese-number`, `call-bus`. No vacuous assertions seen. Gaps:
- No integration test for the auto-tick bus → PlayerBoard `crossed` flow (would have caught P0 #1).
- No test for `MasterPanel` auto-call interval lifecycle (start/stop/speed-change re-arm).
- No test for `voice.js` token cancellation (pure logic — easy to add).
- `generateGrid` rejection sampling tested for "no triple" but not for the fallback path (return `last` after 200 attempts).
## Positives
- Clean module boundaries; `game-logic` pure.
- `safeParse` pattern for localStorage is solid.
- `playClip` token/resolver dance handles cancellation correctly.
- Theme `auto` listener cleanup is proper.
- JSDoc coverage near 100%; types tight.
- Migration path (`masterMode` → `mode: "both"`) preserved with tests.
## Recommended fix order
1. P0 #1 — track `lastDrawn.at` non-reactively in PlayerBoard; only auto-tick when it changes.
2. P1 #2 — call `resetBus()` from player resets (or rely on #1 fix making it moot).
3. P1 #5 — window-level Escape on bingo modal.
4. P2 — Fisher-Yates in `randomNumbersInCol`; reduced-motion gates.
5. Add integration test covering auto-tick + manual-untick interaction.
## Unresolved questions
- Is `storagePrefix` prop intended for a future feature (e.g. master also tracks their own card) or YAGNI?
- For the auto-tick ergonomics: is "user untoggles latest draw" actually a desired flow, or is the auto-tick supposed to be authoritative? Affects the chosen fix for P0 #1.
- Should `voiceEnabledMaster` driving the player Chờ/Kinh in "both" mode be made a separate setting? Current double-meaning (read in `PlayerBoard.svelte:97-99`) is non-obvious.
**Status:** DONE
**Summary:** One P0 (auto-tick re-marks cleared cells), 6 P1, several P2 cosmetic/a11y. Code quality high overall.
web/plans/reports/code-reviewer-260427-2030-polish-pwa.md
-63
View File
@@ -1,63 +0,0 @@
# Code review — UI polish v2 + PWA (260427-2030)
**Scope:** uncommitted changes (9 modified, 5 new). LOC delta ~280 (excluding lockfile). All 115 tests pass; build clean.
**Verdict:** ship-ready with minor follow-ups. No P0 blockers. A couple of P1s worth fixing pre-deploy.
## P0 (blocking)
None.
## P1 (fix before deploy)
**1. Audio cache `maxEntries: 200` — no headroom for a 3rd voice.**
`vite.config.js:38`. Today: 2 voices × 92 = 184 clips. Cap = 200, only 16-clip headroom. If a 3rd voice ships (`hoai-my`/`nam-minh` already exist; future voice would push to 276) Workbox LRU will evict mid-game. Bump to 400 or `2 * voices.length * 92`. Zero runtime cost — `maxEntries` only affects eviction policy, not memory.
**2. Plan vs. impl divergence — audio strategy silently changed.**
Plan (phase-03 §workbox) called for `precache` of all 184 mp3s. Implementation chose runtime `CacheFirst` (`vite.config.js:31-44`). This is actually a better call (saves ~1.87 MB from initial precache, app-shell-only is 213 entries / ~353 KB now). But: **first offline play of any uncached voice will fail silently** — fairground use case (no signal at venue) won't work for clips the user hasn't played yet. Plan's "killer feature: works offline" assumes precache. Two options: (a) document the "warm-up: play once on Wi-Fi" requirement in UI; (b) precache the default voice only (~92 × 10 KB = 920 KB) and runtime-cache alternates. (b) is the right answer.
**3. CSP — no `connect-src` allowance for SW update fetches.**
`static/_headers:2`. `connect-src 'self'` is present — fine. But Workbox `autoUpdate` fetches `/sw.js` periodically; with the new `Cache-Control: no-cache` header on `/sw.js` (good!) the SW will revalidate. No CSP gap detected. `worker-src 'self'` and `manifest-src 'self'` are correctly added. **No action required** — flagging because the question was asked.
**4. `autoUpdate` mid-game stale-content trap is real.**
`vite.config.js:18`. `registerType: "autoUpdate"` + Workbox default `skipWaiting: false` means the new SW activates only after **all tabs close**. For a fairground host running the app for an hour straight, this is fine. BUT if anyone bumps `skipWaiting` later, mid-game asset reload could swap the JS bundle while audio is buffering → broken playback. Suggest a code comment in `vite.config.js` documenting "do NOT add skipWaiting without a reload-prompt UI" so future-you doesn't silently break it.
## P2 (cleanup, non-blocking)
**5. DRY gap — `MasterEmptyState` and `PlayerBoard` empty branch are near-duplicates.**
`MasterEmptyState.svelte:6-43` vs `PlayerBoard.svelte:343-371`. Both: ghost grid (99 vs 27 cells) → italic prompt → emoji subline. Differences: grid size, role pill (master only), accent color (orange vs rose). Worth extracting shared `<EmptyStateHero {gridCells} {gridCols} {rolePill} {prompt} {subline} {accent} />` if a 3rd empty state ever lands. Today: not worth the indirection. **Skip.**
**6. Maskable icon — content ratio 70% is below Android's recommended 80%.**
`source.svg:14` shows 400/512 = 78% inner content area in the standard icon, but `phase-03` plan §icons used `-resize 70%x70%` for maskable padding (= 70% safe zone). Android adaptive icons crop to a circle of diameter ~80% of the container. 70% is fine — slightly conservative — but the wordmark "Lô tô" with stroke at `font-size:240` may still graze edges after the 70% scale. Risk: text clipping on aggressive shape masks (squircle/teardrop). Quick fix: re-export maskable at 65%. Verify in Chrome DevTools → Application → Manifest → "Show maskable preview".
**7. iOS Safari — `apple-touch-icon` only ships 192px.**
`app.html:11`. Apple recommends 180×180 specifically; 192×192 works (Safari scales) but a dedicated 180px is the convention. Also missing: `apple-touch-startup-image` for splash. Low priority — standalone launch will still work, splash is a minor polish.
**8. `MasterEmptyState.svelte:13` unused destructured var pattern.**
`{#each Array(99) as _, i (i)}` — works but `Array(99)` creates a sparse array; some bundler/linter combos warn on iterating sparse arrays. Use `{#each {length: 99}, i}` if Svelte 5 supports it, else `Array.from({length:99})`. Cosmetic; current code works.
**9. Font swap — confirmed safe.**
`@fontsource/roboto-condensed/700.css` ships `font-display: swap` by default per Fontsource docs. FOUT risk: minimal (28 KB woff2, served from same origin under `font-src 'self' data:`). Fallback stack at `app.css:140-146` (Arial Narrow, Avenir Next Condensed, Liberation Sans Narrow, system-ui) ensures Vietnamese diacritics render before Roboto Condensed loads. **No action.**
## Plan compliance check
| Plan item | Shipped? |
|---|---|
| Phase 1: Vietnamese font + master empty state | ✅ (font weight 900→700 documented in CSS comment) |
| Phase 2: mode picker icons + color picker card + brand subline | ✅ |
| Phase 3: PWA + offline | ⚠️ audio precache → runtime-only (P1 §2 above) |
| Section-label Chờ ring (plan §decisions, marked optional) | ❌ skipped (was flagged optional, fine) |
| `npm test` still passing | ✅ 115/115 |
| Lighthouse PWA audit run | ❓ not in evidence — recommend before deploy |
## Positive observations
- `MasterPanel.svelte:135` correctly cancels playback on unmount — no audio leak when switching modes mid-clip.
- `static/_headers:9-13` adds `no-cache` on both `/sw.js` AND `/manifest.webmanifest` — Cloudflare-aware, correct.
- `manifest.webmanifest` uses relative `start_url: "."` and `scope: "."` — survives the `BUILD_PROFILE=gh` `/loto/` base path automatically. Good call vs. hardcoding `/`.
- Dual `theme-color` meta with media queries (`app.html:9-10`) — proper light/dark splash on iOS 15+.
- Scope freeze on `BOARD` + `BOARD_FLAT` (`MasterPanel.svelte:77-78`) — correct from prior review.
## Unresolved questions
1. Was the audio strategy change (precache → runtime CacheFirst) intentional, or did it slip during impl? If intentional, plan should be updated retroactively. If not, P1 §2 should be applied.
2. Has the GitHub Pages mirror (`BUILD_PROFILE=gh`) been smoke-tested? `@vite-pwa/sveltekit` with `paths.base = "/loto"` usually rewrites SW scope correctly, but the hand-written manifest icon paths (`icons/icon-192.png` relative) need verification under `/loto/`.
3. Lighthouse PWA score — was 100/100 confirmed?
web/plans/reports/code-reviewer-260427-2047-pass2-full.md
-83
View File
@@ -1,83 +0,0 @@
# Code Review — Pass 2 (full project, post-PWA)
**Scope:** `src/**`, `vite.config.js`, `static/_headers`, `static/manifest.webmanifest`, `package.json`.
**Baseline:** prior pass at `f28279b`. Reviewed commits: `ad6291e`, `f7db20c`, `d94294d`.
**Verdict:** clean. Prior P0/P1 findings landed correctly. No new P0. Two P1 around PWA/CSP. A few P2 nits.
---
## Prior fixes — landing check
- Auto-tick re-mark guard: `PlayerBoard.svelte:45,157-171` — `lastHandledDrawAt` compared against `bus.lastDrawn.at`. Manual untick / clear / regen no longer re-fires (re-runs read same `at`, early return). Correct.
- Toast positioning: `PlayerBoard.svelte:329-340` — anchored to grid container with `-top-3 sm:-top-4`, `pointer-events-none` wrapper, button-only `pointer-events-auto`. Correct.
- Modal Escape: window-level `keydown` in `PlayerBoard.svelte:142-150` and `SettingsButton.svelte:113-121`. Correct.
- Master empty state: extracted `MasterEmptyState.svelte`, used at `MasterPanel.svelte:348`. Correct.
- Biased shuffle replaced by Fisher-Yates: `game-logic.js:32-35`, `MasterPanel.svelte:35-38`. Correct.
- Reduced-motion gates: `MasterPanel.svelte:145-153` (scroll), `PlayerBoard.svelte:18-23` (vibrate), `app.css:219-232` (animations). Correct.
- Storage payload caps + `__proto__`/`constructor` reviver: `game-logic.js:166-189`, `MasterPanel.svelte:51-75`, `settings-store.svelte.js:127-132`. Correct.
- `encodeURIComponent` on voice URL: `voice.js:41`. Correct.
- Security headers: `static/_headers` present with strict CSP, COOP-equivalent (`frame-ancestors 'none'`), nosniff, etc.
---
## P1 (action recommended pre-merge)
**1. CSP ↔ inline SW registration race / failure mode** — `static/_headers:2`
CSP has `script-src 'self'` (no `'unsafe-inline'`, no nonce). `@vite-pwa/sveltekit` with `registerType: "autoUpdate"` injects an inline `<script>` registering `/sw.js` into the prerendered `index.html`. On a strict CSP host (Cloudflare Pages honors `_headers`), that inline registration block will be blocked → no PWA install. Two fixes: (a) switch to virtual `import { registerSW } from 'virtual:pwa-register'` from a real module, OR (b) configure plugin `injectRegister: 'script-defer'` with a hashed/external file. Verify post-build that `build/index.html` does NOT contain inline registration; if it does, this is silently broken in production.
**2. `static/_headers` does not cover `/sw.js` MIME** — `static/_headers:9-10`
Only `Cache-Control: no-cache` is set on `/sw.js`. Cloudflare Pages will infer `application/javascript` from extension, but spec-strict registrars reject if `Content-Type` isn't `text/javascript`/`application/javascript`. Low-risk on CF, but add explicit `Content-Type: application/javascript` to be safe alongside the manifest entry already doing so.
---
## P2 (nits / tech debt)
- `vite.config.js:13` — `defaultVoiceId = audioManifest.voices[0]?.id ?? "hoai-my"`. Hardcoded fallback drifts from `audio-manifest.js:23`. Either move to a shared `scripts/audio-default.js` import or assert at config-eval time. Low likelihood of mismatch but easy to lose on a manifest rewrite.
- `vite.config.js:23` — `revision: 'audio-v1-{voice}-{n}'` is a manual cache-buster. Comment ("Bump the prefix when audio is regenerated") relies on humans. Consider hashing file content (`createHash('sha1', readFileSync(path))`) so audio regen invalidates automatically.
- `vite.config.js:46` — `includeAssets: ["icons/*.png", "audio/**/*.mp3"]` lists ALL voice mp3s, but `globPatterns` (line 48) does NOT include `mp3`. Result: `includeAssets` only copies them into the build (already happens via `static/`); the actual precache list is `globPatterns ∪ additionalManifestEntries`. Default voice is precached via `additionalManifestEntries`; alt voices fall through runtime CacheFirst as documented. Behavior is correct — but `includeAssets` is dead config noise, drop it or document its no-op role.
- `manifest.webmanifest:5-6` — `start_url: "."`, `scope: "."`. Under base path `/loto/`, browsers resolve relative to manifest URL, so this works on GH Pages. But Cloudflare and GH share the same file. If you ever add a non-root deploy without rewriting the manifest, scope drift will silently break PWA scope detection. Consider `%sveltekit.assets%`-templated manifest emitted at build time, or explicit `start_url: "/loto/"` + a CF-only override.
- `MasterPanel.svelte:107-109` — `callOrder` rebuilds the whole `Map` on every state change; fine at 90 entries. No action.
- `MasterPanel.svelte:90` — `heroEl` typed `HTMLDivElement | null` but `bind:this` runs at every render. Using `$state` here is correct in Svelte 5; OK.
- `PlayerBoard.svelte:48-52` — `rowCompleteness` derived; uses `grid.map((_, r) => isRowComplete(grid, crossed, r))`. Reads both reactively — fires on every cell toggle (9 calls). Acceptable.
- `MasterPanel.svelte:166-177` `handleDrawNext` does not check `settings.mode` before `broadcastDraw`; player auto-tick effect already gates on `settings.mode === "both"` (`PlayerBoard.svelte:162`), so harmless. But broadcasting in `master`-only mode is wasted work and pollutes the bus across mode flips. Suggest gating, OR documenting why it's intentional (so resuming from "both" → "master" → "both" mid-game works).
- `call-bus.svelte.js:1-22` — JSDoc accurate. `broadcastDraw` lacks `@returns`, `resetBus` lacks docstring; trivial.
- `game-logic.js:277-288` — `findUncrossedCell` JSDoc accurate. Top-down/left-right scan order matches test expectation.
- Stale comment risk: `MasterPanel.svelte:51-53` says "16 KB has 30× headroom" — true after the cap landed; keep.
- Dead/duplicate `isBrowser` check in `voice.js:54` (`cancelPlayback`) — fine, but `if (!isBrowser()) return;` is unreachable in test path; cosmetic.
---
## Test-coverage gaps
- No test for the `PlayerBoard` auto-tick effect (mode flip, dedup-by-`at`, manual-untick re-mark prevention). The behavior is the highest-risk new code path. Add one component test: drive `bus.lastDrawn`, assert `crossed[r][c]` flips once, untick by hand, broadcast same `at` → does NOT re-mark. Recommended.
- No test for `MasterPanel.handleDrawNext` → `broadcastDraw` linkage; bus contract tested in isolation only.
- No SW/Workbox integration test — out of scope for vitest, but a `npm run build:gh && grep -r 'sw.js' build/index.html` smoke check in CI would catch P1 #1.
- `voice.test.js` does not cover the `cho → number` cancel-mid-chain case (cancel between the two awaited `playClip` calls). The token mismatch path resolves cleanly; one test would lock it in.
---
## Security
- CSP unchanged, still strict. PWA SW served same-origin; runtime caching only matches `/audio/*.mp3` regex, no third-party cacheing. `manifest.webmanifest` is plain JSON, no scripts. Icons are local PNGs. New attack surface: SW lifecycle. `registerType: "autoUpdate"` + comment on line 38-39 ("Do NOT add `skipWaiting`") is correct — stale clients keep working until tab close.
- `npm overrides` for `serialize-javascript@^7.0.5` and `cookie@^0.7.2` are dev-only build-chain transitive vulns. Lockfile is the source of truth — confirm `npm ls serialize-javascript cookie` shows resolved 7.x/0.7.x post-`npm install`.
- No PII, no telemetry, no remote endpoints.
---
## Positive
- `lastHandledDrawAt` design (closed-over plain ref, not `$state`) is exactly right — avoids the auto-tick effect re-triggering itself.
- Two-pass `$effect` on bingo + waiting in `PlayerBoard.svelte:97-131` reads cleanly; comment on "at most one bingo popup per render" matches code.
- JSDoc + JSDoc-via-`/** @type */` casts give meaningful type narrowing without TS toolchain.
- Test file naming and `// @vitest-environment happy-dom` annotation per file is consistent.
---
## Unresolved questions
1. Is the PWA install actually working in production (Cloudflare Pages) under the strict CSP, or has nobody tested install + reload offline? See P1 #1.
2. Is `BUILD_PROFILE=gh` (`/loto/` base) deployed anywhere live? If not, `manifest.webmanifest`'s relative `start_url` is untested at non-root scope.
3. Should `MasterPanel.handleDrawNext` skip `broadcastDraw` when `settings.mode !== "both"`, or is the cross-mode bus intentional for future "Cả hai" toggling mid-game?
**Status:** DONE
**Summary:** Prior P0/P1 fixes landed cleanly. Two P1 items around PWA SW registration vs strict CSP need a build-output check before next deploy. P2s are minor.
web/plans/reports/code-reviewer-260430-1925-auto-countdown.md
-124
View File
@@ -1,124 +0,0 @@
---
title: Code review — AutoCountdown + MasterPanel integration
reviewer: code-reviewer
date: 2026-04-30
slug: auto-countdown
scope:
- src/lib/AutoCountdown.svelte (new)
- src/lib/MasterPanel.svelte (modified)
plan: plans/260430-1919-auto-call-countdown/
---
# Code Review — Auto-call countdown
## Summary
Implementation matches the plan. rAF cleanup, off-by-one clamp, reactivity, and
race conditions all look sound under Svelte 5 runes semantics. Two **minor**
items worth a follow-up; otherwise good to ship.
## Findings
### Critical
None.
### Major
None.
### Minor
**M1. Hidden coupling: `AutoCountdown` reset effect doesn't depend on `duration`.**
File: `src/lib/AutoCountdown.svelte:26-32`
```js
$effect(() => {
tickKey; // subscribe
if (running) {
tickStart = performance.now();
now = tickStart;
}
});
```
Reset only re-bases on `tickKey` or `running` rising edge. If a parent ever
changes `duration` without also bumping `tickKey`, the ring's progress jumps
mid-tick (because `totalMs` recomputes while `elapsedMs` is unchanged).
Today this is safe: `MasterPanel`'s auto-call `$effect` reads
`settings.autoCallSpeed`, so any speed change tears down + re-arms the effect
which bumps `tickCount` (line 129). The component contract is implicit, not
enforced.
Fix (cheap, robust): include `duration` in the reset effect:
```js
$effect(() => {
tickKey;
duration; // also re-baseline if duration changes without a tick bump
if (running) {
tickStart = performance.now();
now = tickStart;
}
});
```
**M2. `reduceMotion` keeps rAF loop alive needlessly.**
File: `src/lib/AutoCountdown.svelte:36-43`
When `reduceMotion === true`, `dashOffset` is forced to 0 (static ring), but
the rAF loop still runs at ~60Hz updating `now` purely to drive
`secondsRemaining`. A 1Hz `setInterval` (or skipping the loop and updating
`now` only on `tickKey` change as the plan suggested) would be cheaper and
match the plan spec verbatim. Not user-visible; pure CPU hygiene.
### Nits
**N1. `let tickStart = $state(performance.now())` at module top-level.**
Runs at component instantiation, not module import (Svelte 5 compiles `<script>`
into the component constructor), so there's no SSR concern despite `ssr: false`
in `+layout.js`. No action needed; flagged only because module-scope
`performance.now()` looks scary at a glance.
**N2. `tickCount++` inside `$effect` body (`MasterPanel.svelte:129`).**
Safe today because the effect doesn't *read* `tickCount`, so writing it can't
loop. This is a fragile invariant — if anyone later reads `tickCount` inside
that same effect (e.g. for logging/diagnostics), it becomes an infinite
self-trigger. A one-line comment ("not read in this effect — safe to write")
above line 129 would harden the intent.
## Concern Verification
| Concern raised | Verdict | Notes |
|---|---|---|
| rAF leak on `running=false` / unmount / rapid `tickKey` | Safe | Single `$effect` keyed on `running`; cleanup `cancelAnimationFrame(raf)` closes over the latest `raf` id (re-assigned each frame). `tickKey` change doesn't tear down rAF effect (not in deps), only re-bases `tickStart` via the other effect — correct, no churn. |
| Off-by-one number flash at tick edge | Safe | `Math.max(1, Math.ceil(duration - elapsedMs/1000))` clamps to ≥1 while `running`, and falls to `duration` (not 0) when stopped. Cannot render `0`. |
| Reactivity / runes correctness | Correct | Bare `tickKey;` reads the prop and registers as a dep (Svelte 5 tracks property reads inside effect bodies). `running` read via `if (running)` also registered. No infinite loop because no effect both reads and writes the same state. |
| Race: parent effect `tickCount++` vs `handleDrawNext` `tickCount++` | Safe | Parent's auto-call effect doesn't read `tickCount`, so its own write doesn't re-trigger it. Only `autoRunning`, `settings.autoCallEnabled`, `settings.autoCallSpeed` cause re-runs. Each re-arm bumps once; each draw bumps once; child sees a strictly increasing `tickKey`. |
| `currentColor` + `text-amber-500` SVG pattern | Idiomatic | Matches `SettingsButton.svelte:138` and Tailwind's recommended pattern. Per-`<circle>` `text-*` class sets `color`, which `stroke="currentColor"` resolves on that element. |
| `role="timer"` + `aria-live="off"` | Correct | A timer ticking once per second with `aria-live="polite"` would spam screen readers. `off` is the right call; the `aria-label` still exposes current value on focus/inspection. |
| Code style (kebab-case, JSDoc, comment density) | Matches | `@typedef Props`, JSDoc on props, comment style consistent with `MasterPanel.svelte` and `PlayerBoard.svelte`. File is 99 lines (well under 200 LOC limit). |
## Behavioral Checklist
- [x] Concurrency: no shared mutable state across components; only `tickCount` flows parent→child
- [x] Error boundaries: no exceptions thrown; `matchMedia` optional-chained for older clients
- [x] API contracts: `Props` JSDoc matches usage; `tickKey` semantics documented
- [x] Backwards compatibility: no exported interface change; component is purely additive
- [x] Input validation: not applicable — component is render-only, props are internally controlled
- [x] Auth/authz: not applicable — visual UI only
- [x] N+1 / query efficiency: not applicable — no I/O
- [x] Data leaks: not applicable — no PII surface
- [x] Fact-checked: paths and line numbers grep-verified against actual files
## Recommended Actions
1. (Minor) Add `duration` to the reset `$effect` deps in `AutoCountdown.svelte`
to make the reset contract explicit rather than relying on parent discipline.
2. (Minor) Drop the rAF loop on `reduceMotion` — replace with `setInterval(.., 1000)`
or update `now` only on `tickKey` change. Aligns code with phase-01 plan.
3. (Nit) One-line comment at `MasterPanel.svelte:129` documenting why
`tickCount++` inside the effect is loop-safe.
## Unresolved Questions
None.
web/plans/reports/code-reviewer-260430-2024-both-mode-consistency.md
-182
View File
@@ -1,182 +0,0 @@
# Both-mode consistency review
## Summary
13 findings: 4 critical, 6 major, 3 minor.
Theme: master's `called[]` is the source of truth, but the bus only carries `lastDrawn`. Any time the player's `crossed` is rebuilt or remounted while master mid-game, prior history is lost. `resetBus()` clears the slot but never resets the consumer's `lastHandledDrawAt`, so cross-side reset semantics are subtly broken.
## Findings
### F1: Player regen mid-game loses all prior master draws (CRITICAL)
**Where:** `src/lib/PlayerBoard.svelte:193-207` (`handleGenerate`)
**Symptom:** Master has called e.g. 30 numbers. Player taps "Tạo bảng mới" to reroll the card. New grid intersects with called numbers, but no cells are pre-crossed. Player must wait for the NEXT master draw (which will only mark that one number) — the other ~29 hits are silently lost forever.
**Cause:** Two reasons compounding:
1. `handleGenerate` calls `resetBus()` (line 206) which only nulls `bus.lastDrawn`. Master's `state.called[]` is still the truth, but no replay path exists.
2. The `processAutoTick` effect (line 181) is bus-driven, never history-driven. It can't "catch up" because there's nowhere to read history from.
Worse: `resetBus()` here punishes the master too. If the master is currently auto-running and the player hits regen, the next master draw will broadcast normally, BUT every other PlayerBoard mount (if `+page.svelte` ever rendered two) loses its bus slot too. The reset reaches across the trust boundary.
**Repro:**
1. Mode = both, master "Ván mới", draw 10 numbers.
2. Player "Tạo bảng mới" → confirm.
3. Inspect: zero crossed cells on new grid even when 3-4 numbers from `called[]` exist on it.
4. Master "Xổ số" once → only the just-drawn number gets crossed (if on grid). The historical 10 are gone.
**Fix idea:**
Make master's called list the authority. Either (a) export `getCalledNumbers()` from a shared store and have player's regen replay-cross all of them via `findUncrossedCell` in a loop, or (b) when `mode === "both"` skip the `resetBus()` call AND, on regen, walk `state.called[]` from MasterPanel (lift to a shared store) to pre-cross the new grid. Also drop `resetBus()` from `handleGenerate` — regenerating a card has nothing to do with the master's broadcast slot.
---
### F2: Master "Ván mới" leaves player's crossed marks stale (CRITICAL)
**Where:** `src/lib/MasterPanel.svelte:165-172` (`handleNewGame`)
**Symptom:** Master ends a game (everyone Kinh!), starts a new one. Player's grid is still the OLD card with OLD crossed marks. The new game's first `Xổ số` triggers an auto-tick that may flip a cell on the stale board. Player sees "Chờ X" toasts and even possibly a fake "Kinh!" celebration for a row that was already complete from the prior game.
**Cause:** `handleNewGame` resets the master's own `state` and calls `resetBus()`, but never signals the player to clear `crossed`. Player's `loto_crossed` localStorage entry is untouched. The `processAutoTick` effect's `lastHandledDrawAt` is never reset (it's a `let` $state in PlayerBoard), but `resetBus()` sets `bus.lastDrawn = null` which the effect ignores via the `!lastDraw` early-return — so the dedup cursor stays at the OLD game's last `at`. Then the new game's first broadcast (`{ num, at: Date.now() }`) has a fresh `at > old at`, so it ticks against the stale grid.
**Repro:**
1. Mode = both, master draws until player completes row 1 (Kinh modal shows).
2. Master "Ván mới" → confirm.
3. Master "Xổ số" → if the new number happens to be on the stale grid, player sees an auto-cross on what is visually still the old card.
**Fix idea:** Either (a) on master "Ván mới", clear player storage (`loto_crossed` only, keep grid) and broadcast a `gameReset` signal — extend the bus to `{ lastDrawn, gameId }`; player effect resets `crossed` when gameId changes; or (b) keep cards/marks across master games and only require player to manually "Xoá đánh dấu" — but document this and at minimum reset `lastHandledDrawAt` to 0 on a `gameReset` signal so timing math doesn't drift.
---
### F3: `resetBus()` is fired by the player but only resets a single shared slot (CRITICAL)
**Where:** `src/lib/PlayerBoard.svelte:206, 219` and `src/lib/call-bus.svelte.js:20-22`
**Symptom:** Player tapping "Tạo bảng mới" or "Xoá đánh dấu" wipes `bus.lastDrawn` for the master too. If two PlayerBoards were ever mounted (or the master's own logic ever depended on the last-drawn marker — currently it has its own `lastCalled`, but that coupling is fragile), this is a cross-component side effect.
More concretely: after player calls `resetBus()`, master's NEXT `Xổ số` broadcasts a fresh `{num, at}`, so player auto-tick fires again — but the player just chose to reset, expecting silence. With the dedup cursor still at the OLD `at`, master's new broadcast (`Date.now()` > old) does fire. So `resetBus` doesn't even achieve "ignore future master draws on this fresh card", it just creates a one-broadcast lull.
**Cause:** The bus has cross-component write semantics but no scoping. `resetBus` is a sledgehammer — the player can't tell "I want my local cursor to reset" from "I want to wipe the master's broadcast slot". Combining (a) the bus state (master broadcasts) with (b) the consumer cursor (player's `lastHandledDrawAt`) is the structural error.
**Repro:** Player taps "Xoá đánh dấu" (line 219) right after master's draw. Now `bus.lastDrawn` is null even though master drew. Immediately switching mode `both → player → both` (which doesn't republish) leaves player with no marker of what was last drawn. Combined with F11, the player's $effect can re-fire on `crossed` change and silently advance state.
**Fix idea:** Drop the `resetBus()` call from BOTH player handlers. Replace with a local-only cursor reset: `lastHandledDrawAt = bus.lastDrawn?.at ?? 0` so the player consumes the current slot without acting. The master owns the bus.
---
### F4: Page reload mid-game leaves bus null but `lastHandledDrawAt = 0`; first new master draw rewrites a fully-restored crossed grid (CRITICAL)
**Where:** `src/lib/PlayerBoard.svelte:47, 86-103, 181-191` + `src/lib/call-bus.svelte.js:10`
**Symptom:** Master has drawn 20, player has 20 cells crossed (auto-ticked + persisted to `loto_crossed`). User reloads the page. Both panels rehydrate from localStorage. Bus is module-state — fresh, `lastDrawn = null`. Player's `lastHandledDrawAt = 0` (its $state init). On the master's NEXT draw (say number 21), the bus publishes `{ num: 21, at: T }`. Player's effect sees `lastDraw.at !== lastHandledAt`, advances, ticks number 21. So far OK. But because the bus IS null on mount, if the player double-clicks "Tạo bảng mới"+"Xoá đánh dấu" quickly, the regen effect runs → `crossed` becomes empty, persisted. Now master's history is gone AND there's no in-bus draw to replay. Same as F1 but post-reload, harder to recover from because the user thinks the reload preserved state.
**Cause:** Bus is in-memory only (`call-bus.svelte.js`), but `crossed` is persisted. State coupling broken across reload.
**Repro:**
1. Mode = both, draw 20, player's grid is half-crossed and persisted.
2. Reload page.
3. Player taps "Tạo bảng mới" → ALL prior auto-ticks gone with no recourse.
4. Or: if master never draws again (game ended), player has no record at all of what was called — only that some cells were once crossed.
**Fix idea:** Persist the bus alongside master state, or expose master's `called[]` as the canonical source on mount and have player effect run an initial replay-cross pass when it detects `lastHandledDrawAt === 0` AND `state.called.length > 0`. Tied to F1 — same root fix.
---
### F5: `lastHandledDrawAt` is never persisted, so crossed-state and dedup cursor diverge (MAJOR)
**Where:** `src/lib/PlayerBoard.svelte:47`
**Symptom:** Across reloads, `crossed` is restored from localStorage but `lastHandledDrawAt = 0`. If `bus.lastDrawn` happens to be non-null (it never is on cold reload, but could be after HMR or if you ever persist the bus), the player would re-tick the latest already-crossed number. With current code: harmless because bus is null on reload. With any future change to persist the bus, this becomes a re-tick bug.
**Cause:** `let lastHandledDrawAt = 0` is in-memory only, while the state it dedupes against (the bus) and the state it gates (`crossed`) both have persistence stories.
**Repro:** Force-set `bus.lastDrawn` to `{ num: 5, at: 1 }` in a dev tool right after mount (simulating a future persisted bus). Effect fires, double-flips cell containing 5 if it was the only call.
**Fix idea:** Persist `lastHandledDrawAt` as part of `loto_crossed` payload (bump shape to `{ crossed, lastHandledAt }`) or derive it from master state on mount: `lastHandledDrawAt = bus.lastDrawn?.at ?? 0` immediately after the initial-load $effect.
---
### F6: Mode toggle player→both mid-game replays the LAST draw only (MAJOR, partial bug)
**Where:** `src/lib/PlayerBoard.svelte:181-191` + `src/lib/auto-tick.js:35-40`
**Symptom:** User starts in `mode = player` (solo). Master friend joins, host flips to `mode = both` after master has drawn 10 numbers. Player's effect re-fires (mode is reactive in `processAutoTick` args), sees `lastDraw.at !== lastHandledAt` (cursor was 0 in solo), `mode === "both"`, finds an uncrossed cell holding `bus.lastDrawn.num` → ticks ONE cell (the most recent draw). The other 9 historical draws are lost.
This is the same flavour as F1, just triggered by mode toggle. The auto-tick.test.js explicitly documents the "advance lastHandledAt even when mode mismatch" invariant, calling it intentional. The test-comment justification ("solo player switching to 'both' mid-game shouldn't replay a stale draw") explicitly bakes in the partial-replay bug.
**Cause:** `processAutoTick` advances `lastHandledAt` even when `mode !== "both"`. So during solo play, every master broadcast silently consumed the cursor. Toggling to both then has no history to replay.
**Repro:**
1. Mode = player. Master mode toggled off.
2. Set `mode = "both"` first to bind player effect, then back to "player". Master draws 5 numbers (broadcasts still fire). Player effect runs each time, advances `lastHandledDrawAt` to the latest `at`, but mode mismatches so no tick.
3. Switch to "both". Effect re-runs but `lastDraw.at === lastHandledAt` → no-op. Player has zero crosses for 5 already-called numbers.
**Fix idea:** Don't advance `lastHandledAt` when mode isn't "both" — let it sit at 0 until the first "both"-mode tick. Then on the mode flip, do a one-shot replay over master's `called[]`. Requires exposing `called[]` outside MasterPanel (lift to a shared `master-state.svelte.js`).
---
### F7: `Date.now()` collision swallows the second broadcast within the same ms (MAJOR)
**Where:** `src/lib/call-bus.svelte.js:17` + `src/lib/auto-tick.js:35-37`
**Symptom:** Two master draws within the same millisecond produce `{at: T}` twice with identical `at`. The second is treated as a re-fire and silently skipped by `processAutoTick`'s `lastDraw.at === lastHandledAt` check.
Realistic? Manual button mashing on mobile likely produces ≥2-3ms gaps, but: (a) auto-call interval ≥1s so safe there, (b) the master's `handleDrawNext` is synchronous and could be invoked twice in a microtask boundary if called from a synthetic test, (c) future code (e.g. "skip a number" UX) could draw twice in one tick. Bus assumes monotonic strictly-increasing `at`; `Date.now()` doesn't.
**Cause:** `Date.now()` has 1ms resolution; consumer compares with `===` not `>=`.
**Repro:** Synthetic test: `broadcastDraw(1); broadcastDraw(2);` in same tick; mock `Date.now()` to return `1000` for both. Player's effect runs once with `lastDrawn.num = 2`, ticks 2, never sees 1.
**Fix idea:** Use a monotonic counter instead of `Date.now()`: `let seq = 0; broadcastDraw = n => { bus.lastDrawn = { num: n, seq: ++seq } }`. Update `processAutoTick` to compare `seq`. Also guarantees ordering across clock skew (browser tab throttling can move clock).
---
### F8: `called[]` is the source of truth but only the latest leaks via the bus; effect throw drops history forever (MAJOR)
**Where:** `src/lib/MasterPanel.svelte:88-90, 174-186` + `src/lib/PlayerBoard.svelte:181-191`
**Symptom:** If the player's auto-tick `$effect` ever throws (e.g. `findUncrossedCell` is fed corrupted state, immutable map throws on a frozen sub-array, future feature adds a new code path with a bug), Svelte may continue but the cell flip is lost. There's no retry. `lastHandledDrawAt` may or may not have advanced depending on where in the function the throw happened. Subsequent master draws keep pushing forward, and the "missed" number is permanently lost — the bus only carries the latest.
**Cause:** Single-slot bus + no master-side authority for replay.
**Repro:** Hard to repro deliberately, but consider: `crossed.map(...)` allocates O(81) per draw. On a memory-constrained device, an OOM could throw. Or, more realistically, a future refactor introducing async into the effect breaks ordering.
**Fix idea:** Promote `called[]` to a shared `$state` store (lift from MasterPanel into `master-state.svelte.js`). Player effect derives "what should be crossed" from `called` + grid via a pure projection, not a per-event flip. The grid+called → crossed function is idempotent and immune to throw losses.
---
### F9: voiceEnabledMaster + voiceEnabledPlayer simultaneous → waiting/Kinh cancels number announcement (MAJOR)
**Where:** `src/lib/voice.js:53-95` (single `activeClip`/`activeToken` slot) + `PlayerBoard.svelte:140, 152` + `MasterPanel.svelte:185`
**Symptom:** Mode = both, both voice flags on. Master draws, calls `playNumber(n)` → audio "bốn mươi hai" begins. Same draw triggers `processAutoTick` → `crossed` updates → second $effect re-runs → if a row is now waiting OR complete, `playWaiting` or `playBingo` is called, which immediately `cancelPlayback()`s the master's "bốn mươi hai" mid-syllable, then plays "chờ" or "kinh!".
The host hears "bốn—chờ" or "bốn—kinh!" — the number itself is cut. Players around the host don't hear what was called, only the reaction.
**Cause:** `voice.js` uses a single global activeClip slot with `cancelPlayback()` at the top of every `playX`. There's no priority queue; last writer wins. Master and player publishers race during the same draw → render → effect chain.
**Repro:**
1. Mode = both, both voice toggles on, voiceWaitingNumber off.
2. Manually set up a player grid where one row needs exactly one number.
3. Master "Xổ số" → that exact number is the next call. Listen: number cut off mid-pronunciation by "Kinh!".
**Fix idea:** Introduce a small queue: `enqueue(clip, priority)`. Number takes precedence over Chờ; Kinh is highest. Or sequence them: number → 250ms gap → chờ/kinh. Or — simplest — when both flags are on and mode is both, suppress the player-side announcement (master is the announcer; player flag becomes redundant). The current effect at PlayerBoard.svelte:117-118 already partially routes around this for solo player, but doesn't suppress the duplication when both flags are on.
---
### F10: Player "Xoá đánh dấu" loses already-called numbers permanently (MAJOR)
**Where:** `src/lib/PlayerBoard.svelte:209-220` (`handleClear`)
**Symptom:** Player accidentally taps "Xoá đánh dấu" (or genuinely wants to clear). All marks gone. Master is mid-game with 30 called numbers. The player is now at zero crosses with no replay path. Next master draw will re-cross only that one number.
**Cause:** Same root as F1: master's history is unreachable from the player.
**Repro:**
1. Mode = both, master at 30 calls, player has ~17 crosses.
2. Player "Xoá đánh dấu" → grid blank.
3. Master keeps drawing — only new draws cross. The 17 historical hits never come back unless that exact number is re-broadcast (which it won't, it's been consumed from `remaining`).
**Fix idea:** On clear, in `mode === "both"`, replay-cross master's `called[]` against the (existing) grid before marking complete. Keep the manual single-cell untick separate from a wholesale clear. Additionally, prompt the user "This will clear and re-apply called numbers" when `mode === "both"` so the action is informed.
---
### F11: Reactive effect re-fires on `crossed`/`grid` change but is correctly gated — verify under Svelte 5 fine-grained reactivity (MINOR/uncertain)
**Where:** `src/lib/PlayerBoard.svelte:181-191`
**Symptom:** The auto-tick `$effect` reads `bus.lastDrawn`, `lastHandledDrawAt`, `grid`, `crossed`, `settings.mode`. Any of these changing re-fires it. The dedup-by-`at` invariant claims to prevent re-runs from causing a re-flip — and the `auto-tick.test.js` tests verify the pure function. But the effect WRITES `crossed` (line 190) which is one of its dependencies. Svelte 5 runes do detect cycles; usually batches and short-circuits.
However: if Svelte ever schedules the rerun BEFORE the assignment to `lastHandledDrawAt` is committed (line 189 fires a $state write), there's a brief window where `lastHandledAt` is the OLD value and `lastDraw.at` is the new one — re-firing would `findUncrossedCell` on the now-already-crossed cell, return null, no harm. So this is theoretically safe BUT the safety hangs entirely on the `findUncrossedCell` fallthrough.
**Cause:** Effect both reads and writes `crossed` and `lastHandledDrawAt` in the same execution. Standard Svelte 5 should serialize this, but there's no test covering "what if the reactive system schedules a re-run between line 189 and 190".
**Repro:** Hard. Theoretical. Code currently passes tests.
**Fix idea:** Use `untrack()` for the writes, or restructure: compute the result, then in a `flushSync`-style microtask write both. Or accept current semantics and add a comment + test for "re-entrant scheduling cannot double-tick".
---
### F12: `autoCallEnabled` toggle off mid-run leaves player partially up-to-date (MINOR)
**Where:** `src/lib/MasterPanel.svelte:120-139` (master auto-call effect)
**Symptom:** Master is auto-running. Host flips `autoCallEnabled` off in settings. Master effect tears down the interval (line 122-125 sets `autoRunning = false`), so no more auto-broadcasts. Player auto-tick effect doesn't care — it just stops receiving new draws. So far OK.
But: if host then flips `autoCallEnabled` back on, `autoRunning` is now false (it was reset on disable), so the master would have to manually press "Bắt đầu" again. Meanwhile the player has been quietly accruing nothing. No bug per se, just a UX cliff.
**Cause:** `autoRunning` and `autoCallEnabled` are two pieces of state with overlapping semantics. The disable path resets `autoRunning` to false (correct), but there's no "remembered intent" to restart on re-enable.
**Repro:**
1. Mode = both, autoCallEnabled = true, autoRunning = true. Master is calling every 5s.
2. Open settings, toggle autoCallEnabled off, then on.
3. Auto-call doesn't resume; manual "Bắt đầu" required.
**Fix idea:** Either document this in the settings UI ("Toggling off stops auto-run; tap Bắt đầu to resume") or persist `autoRunning` across the toggle. Probably the former — it's user-initiated.
---
### F13: Multiple PlayerBoard mounts share `lastHandledDrawAt = 0` initial state but each instance has its own copy — tested behavior unclear (MINOR)
**Where:** `src/lib/PlayerBoard.svelte:47` + `src/routes/+page.svelte:33-35`
**Symptom:** `+page.svelte` only mounts ONE PlayerBoard. But the architecture suggests "two cards on one device" might be a future ask (verified by F-search of the repo). If two PlayerBoards mounted, both share the same `bus.lastDrawn`, `STORAGE_PREFIX = "loto"` (so SAME localStorage key for grid and crossed — they'd overwrite each other). Each has its own `lastHandledDrawAt` $state, so each correctly handles its own dedup. But the localStorage collision means one's persistence eats the other's.
**Cause:** `STORAGE_PREFIX` is hardcoded; bus is single-slot global.
**Repro:** Mount two `<PlayerBoard />` instances in `+page.svelte`. Tap "Tạo bảng mới" on instance A → instance B's localStorage is overwritten on next persist effect run. Both grids end up showing the same card.
**Fix idea:** Accept `prefix` as a prop with default `"loto"`. Caller mounts `<PlayerBoard prefix="loto-1" />` and `<PlayerBoard prefix="loto-2" />`. Bus remains global (correct — both should auto-tick on master draw). Safe even if not used now.
---
## Prioritized fix path
1. **Lift `called[]` to a shared store** (`src/lib/master-state.svelte.js`): `{ called: $state([]), remaining: $state([]) }` with `drawNext()`, `newGame()`, `reset()`. MasterPanel imports this, so does PlayerBoard.
2. **Replace bus dedup with seq counter** (F7).
3. **Player auto-tick becomes `called`-derived**: `crossed` is a $derived projection of `(grid, called)` via a pure idempotent function, with manual untick handled by an "exclude" set. Eliminates F1, F2, F4, F6, F8, F10 in one structural change.
4. **Drop `resetBus()` calls from PlayerBoard handlers** (F3) — they don't belong there.
5. **Voice queue** (F9): trivially solved by suppressing player-side announcements when `mode === "both" && voiceEnabledMaster`.
## Unresolved questions
- Is "card persists across master Ván mới" the intended UX, or should new master game force-clear player marks? (Affects F2 fix shape.)
- Should "Xoá đánh dấu" in mode=both behave as "clear AND replay-cross called" or "true clear, ignore called"? Need product call. (Affects F10.)
- Future ask: multiple PlayerBoards on one device — is that on the roadmap? (Affects whether F13 is worth fixing now.)
- Manual untick behavior: today, a re-broadcast of the same number doesn't happen (master never repeats). But if it did (testing-only `replay` button), would re-cross be desired? Current pure function says yes.
web/plans/reports/code-reviewer-260430-2103-both-mode-refactor.md
-354
View File
@@ -1,354 +0,0 @@
# Code review: both-mode state consistency refactor
Plan: `plans/260430-2050-both-mode-state-consistency/`
Reviewer scope: static review of `master-store.svelte.js`,
`player-auto-cross.js`, `MasterPanel.svelte`, `PlayerBoard.svelte`,
`game-logic.js` (manualUnticks helpers), the three new/extended test
files, and `+page.svelte` to confirm mode mounting.
## Severity counts
- **Critical:** 0
- **High:** 1
- **Medium:** 4
- **Low:** 4
- **Nits / informational:** 3
Net: refactor is solid. The shared store is small, deletes more code
than it adds, and the bus is fully retired (`grep call-bus|auto-tick|
broadcastDraw|resetBus` only finds a stale doc comment in
`game-logic.js` line 309). The four product flows are all wired per
locked decisions. One real cross-component bug + a few subtle traps
documented below.
---
## High
### H1 — `MasterPanel` unmount in mode=player drops live master state silently
`+page.svelte` lines 33–50 conditionally renders MasterPanel only when
`settings.mode !== "player"`. Persistence and `loadMaster()` live in
`MasterPanel`'s `$effect`, so:
- In **player** mode the master panel never mounts → `loadMaster()`
never runs → `masterState.called` stays `[]` for the entire session.
That's fine in solo player mode.
- BUT the moment the host toggles **player → both**, the panel mounts
fresh, `loadMaster()` reads persisted state, and `masterState.called`
jumps `0 → N` reactively. This trips PlayerBoard's master-reset
detection effect *in reverse*: it sees `prevCalledLen=0, len=N`, no
reset fires (correct), and `applyMasterCalls` sees a backlog of N
calls with `lastHandledIndex=N` (initialized that way at PlayerBoard
mount). Result: **the player board does NOT replay master's persisted
history when switching to both mode** — phase 3's "open question"
behavior, but undocumented in code.
Phase 3 doc says this is intentional ("cursor was advanced past it"),
but the load order is fragile: PlayerBoard's load $effect runs *first*
because PlayerBoard always mounts (it lives outside the
`mode !== "master"` gate? — actually it's inside `mode !== "master"`,
so it unmounts in master-only mode but mounts in both mode). When mode
flips player→both:
- PlayerBoard already mounted with `lastHandledIndex = 0` (first mount
in player mode read `masterState.called.length === 0`).
- Then MasterPanel mounts → `loadMaster()` → `masterState.called` becomes
the persisted N entries.
- Reactivity fires the auto-cross effect. `mode === "both"` now, grid
exists, `lastHandledIndex (0) < called.length (N)` → it WILL replay
the entire back-history at once.
So the documented "no replay on toggle" property is **only true if
mode toggle happens after MasterPanel was already mounted at least
once**. If the user lands the page in `mode=player` and toggles to
`both` for the first time, every persisted master draw will auto-cross
the player board in one shot — surprising, possibly desirable, but not
what phase 3's "open question" claims.
**Fix options:**
1. Accept the behavior, update phase 3's "open question" to "we do
replay on first mount in both/master modes — known and OK".
2. Move `loadMaster()` to module scope (or `+page.svelte`), so master
state is always primed. Then PlayerBoard's mount-time
`lastHandledIndex = masterState.called.length` line correctly
captures the persisted backlog as "already in sync with persisted
crossed", honoring the docstring.
Option 2 is cleaner and matches the docstring "Treat reload as already
in sync with master's full history". Recommend this.
---
## Medium
### M1 — Effect self-trigger: `applyMasterCalls` $effect reads `crossed`, writes `crossed`
PlayerBoard.svelte:205–218.
The effect reads `grid`, `crossed`, `masterState.called`,
`lastHandledIndex`, `manualUnticks`, `settings.mode`, then conditionally
writes `lastHandledIndex` and `crossed`. Svelte 5 effects re-run on
ANY tracked-dep change, including the very ones they wrote.
- When `applyMasterCalls` returns `changed: true`, the effect writes a
new `crossed`. That triggers a re-run.
- Re-run: `lastHandledIndex` was bumped to `called.length` on the same
pass, so guard `lastHandledIndex >= called.length` short-circuits to
`{changed: false, lastHandledIndex unchanged}`. No write, no further
re-run. **Safe.**
- When `manualUnticks` changes (user untick): the effect re-runs with
the same `lastHandledIndex === called.length` → short-circuit. **Safe.**
- When `crossed` changes via manual click: same — short-circuit. **Safe.**
Verdict: not a bug, but the safety hinges on `applyMasterCalls`'s early
return at line 33–35. Add a comment in PlayerBoard pointing at the
guard, otherwise a future "always recompute crossed from called[]" pure
refactor would silently introduce an infinite loop.
### M2 — Master-reset detection effect — first-run semantics
PlayerBoard.svelte:223–234.
```js
$effect(() => {
const len = masterState.called.length;
const wasReset = prevCalledLen > 0 && len === 0;
prevCalledLen = len;
if (wasReset && settings.mode === "both" && grid) { ... }
});
```
The effect declares a dep on `masterState.called.length` (read) and
writes `prevCalledLen` (written). It does not read `prevCalledLen` —
wait, it DOES read it (`prevCalledLen > 0`). So it reads + writes
`prevCalledLen` and reads `masterState.called.length`. Self-trigger
risk is real **but** the write happens unconditionally to the current
length, and once they're equal the read+write pair is idempotent: write
the same value → Svelte's proxy short-circuits identical assignments to
the underlying signal? In Svelte 5 runes, `$state` writes that produce
the same value DO NOT trigger reactivity (proxy short-circuit on
primitive equality). So no infinite loop.
Mount semantics: `prevCalledLen` is initialized to `0` in `$state`, then
the load effect at line 119 sets it to `masterState.called.length`.
Effect ordering between the load effect and the reset-detect effect is
NOT guaranteed by Svelte. If the reset-detect effect runs first on
mount with `prevCalledLen = 0` and persisted `called.length > 0`, then
`wasReset = (0 > 0 && N === 0)` = false. **Safe by accident** — only
the `>0 → 0` shape qualifies as reset, so first-mount transitions
`0 → N` and `N → N` are both no-ops.
Suggest hardening with a one-line comment: `// prevCalledLen=0 + len>0
is NOT a reset — only >0 → 0 qualifies, so init order vs load $effect
doesn't matter.`
### M3 — `manualUnticks` correctness: pre-call manual cross then user untick
`handleCellClick` lines 318–323:
```js
if (num > 0 && masterState.called.includes(num)) {
...
}
```
Walk-through: user manually crosses cell holding `42` BEFORE master
draws 42. `crossed[r][c] = true`, `manualUnticks` unchanged (because
`called.includes(42) === false`). Master then draws 42; auto-cross
runs `findUncrossedCell(grid, crossed, 42)` which returns null (already
crossed) → no-op, `lastHandledIndex` advances. So far so good.
Now user clicks the cell again (untick). `wasCrossed = true`,
`willBeCrossed = false`. `called.includes(42)` is now true →
`next.add(42)`. `manualUnticks = {42}`. Cell becomes false. ✓ Replay
flows skip 42 thereafter. Correct.
Edge case: user manually crosses 42 pre-draw, master draws, user does
NOT untick, then user clicks "Xoá đánh dấu". `manualUnticks` was empty
at handleClear time → it's reset to `new Set()`, then `applyMasterCalls`
re-crosses 42 (because `findUncrossedCell` finds it on the cleared
grid). Correct outcome — the manual cross was indistinguishable from
auto, replay redoes it.
Edge case: `includes()` is O(n) and runs on every cell click. With max
90 calls it's trivial — fine. (Could be a Set on `masterState`, but
YAGNI.)
**Verdict:** logic is correct. Documented `O(called)` cost is
acceptable.
### M4 — `lastCalled` re-derive over the whole array
MasterPanel.svelte:59–63 reads `masterState.called[masterState.called.length-1]`.
Fine. But line 77 `callOrder` rebuilds a `Map` on every change to
`masterState.called`. Since `called` is replaced (not mutated) on every
draw, this is unavoidable cost — O(n) per draw, n ≤ 90. Trivial. Just
flagging that `derived` does not memoize per-element diffs.
---
## Low
### L1 — `applyMasterCalls` builds a new outer array per call (allocation churn)
Line 49–53 does `next = next.map(...)` once per matched call. Replaying
90 back-history hits: 90 outer-array allocations of length 9. Negligible
(<10 µs total). Could pre-clone once and mutate, but that breaks the
pure-function contract documented at the top. Leave as is — KISS.
### L2 — Deep-equal short-circuit absent in `applyMasterCalls` no-flip path
Line 33–35 short-circuits on cursor-at-end. Line 39–41 short-circuits
on mode mismatch. But line 44–55 walks all calls even if every single
one is in `manualUnticks` or off-board, returning the same `next`
reference. The test `returns same crossed reference when no flip
happens` (line 122) confirms this works for off-board nums. Not a bug,
just verifying the test asserts it correctly. ✓
### L3 — `saveMaster` / persistence effect runs on mount with empty arrays
MasterPanel.svelte:69–74 — second $effect reads `called` + `remaining`
and calls `saveMaster()`. On mount BEFORE the load effect runs, both
are empty → `saveMaster()` writes `{"called":[],"remaining":[]}` to
storage, **clobbering any persisted state**. Then the load effect runs
and reads … the just-clobbered empty state. Game state lost on every
mount.
Wait — let me re-read. The two `$effect`s are sibling effects.
Per Svelte 5, on initial mount they run in declaration order: load
($effect at 65) runs first, populates `masterState`, THEN save effect
at 69 runs and persists what was just loaded. **Safe by declaration
order.**
But this depends on declaration order. Move them out of order and you
silently corrupt storage. Add a comment: `// IMPORTANT: load effect
must remain declared before save effect, or save will clobber storage
on mount.`
Edge: if `loadMaster()` is a no-op (no key in localStorage), save
effect writes `{[],[]}`. That's fine — persists "no game" cleanly.
### L4 — `prevCalledLen` exposed as `$state` but only used internally
It doesn't need to be `$state` for the current logic — a plain `let`
read+written in the same effect works. Making it `$state` adds a
reactive dep that the effect both reads and writes (see M2). Switching
to plain `let` would remove the self-trigger concern entirely. Same for
`lastHandledIndex` — it's only read inside `applyMasterCalls`, and
written in a single effect; tagging it `$state` opts it into Svelte's
reactivity graph for no UI consumption. Consider non-reactive `let` for
both. Cosmetic only.
---
## Nits / informational
### N1 — Stale doc comment in `game-logic.js`
Line 309 still says `Used by the master→player auto-tick path.` The
"auto-tick" name dies with this refactor (replaced by "auto-cross").
Cosmetic — update to `auto-cross` for greppability.
### N2 — `resetMaster` vs `startNewGame` from player POV
PlayerBoard's reset-detect effect fires on `called.length: >0 → 0`.
- `startNewGame`: sets `called = []`, then `remaining = shuffled1to90()`.
Two writes, but Svelte batches sibling reactive writes within a tick.
Effect re-runs once with `called.length === 0` → triggers reset.
Correct.
- `resetMaster`: sets both to `[]`. Effect sees `called.length === 0` →
triggers reset. Same outcome.
Player POV is identical. ✓ matches the question in the brief.
### N3 — Test coverage gaps
- No PlayerBoard.svelte component test. The four product flows
(master Ván mới wipes, player regen replays, player Xoá đánh dấu
replays, master draw auto-crosses) are tested at the helper level
only. A tiny `vitest-svelte` mount + `flushSync` harness for one
flow would catch effect-ordering regressions like H1 / L3.
- No test for `prevCalledLen` mount-edge (load order vs effect order).
- No test for "manual cross before draw → master draws → manual untick
populates manualUnticks correctly" (M3 walkthrough).
Not blocking; flag for follow-up.
---
## Persistence-ordering audit
Multiple effects write to localStorage on the same reactive change:
- `MasterPanel`: 1 save effect on `masterState`. ✓
- `PlayerBoard`: 3 save effects — `manualUnticks`, `crossed`, plus
`saveGrid` inline in `handleGenerate`. They all touch different keys
(`loto_master`, `loto_manualUnticks`, `loto_crossed`, `loto_grid`).
No key collision → no stale-write race. Svelte batches state
mutations within a tick, so a single user action triggers each save
effect at most once per tick.
**Verdict:** no race. Keys are disjoint, writes are last-write-wins per
key, and there's only one writer per key.
---
## Mode-toggle behavior matrix (verified)
| Initial mode | Toggle to | Master state behavior | Player crossed behavior |
|---|---|---|---|
| both | player | MasterPanel unmounts; cancelPlayback fires | PlayerBoard stays mounted; auto-cross effect re-runs with `mode=player` → advances cursor, no flips ✓ |
| both | master | PlayerBoard unmounts | masterState retained; on remount PlayerBoard reloads, sets `lastHandledIndex = called.length` (sync) ✓ |
| player | both | MasterPanel mounts, loads persisted master state → `called.length` jumps `0 → N` | PlayerBoard's auto-cross effect sees `lastHandledIndex=0 < N` → **replays full back-history** (see H1) |
| master | both | PlayerBoard mounts fresh, `lastHandledIndex = called.length` = N | No replay (in sync with persisted crossed) ✓ |
Row 3 is the H1 surprise. Row 1 is correctly handled by the cursor
advance in `applyMasterCalls` non-both branch.
---
## Recommended actions (in priority order)
1. **H1**: decide policy for player→both toggle (replay all or skip).
If "replay all" is desired, hoist `loadMaster()` to module-init or
`+page.svelte`. If "skip", add a guard that bumps `lastHandledIndex
= masterState.called.length` whenever `settings.mode` transitions
into `both`.
2. **M1, M2 comments**: short pointer comments locking in the
self-trigger safety conditions so a future refactor doesn't break
them silently.
3. **L3 comment**: declaration-order dependency in MasterPanel.
4. **N1**: rename "auto-tick" stale comment.
5. **N3**: one component test for the master Ván mới reset flow would
add real value; leave the rest if quota is tight.
---
## Positive observations
- Pure helper extraction (`applyMasterCalls`) is clean — easy to test,
easy to reason about, no Svelte coupling.
- `findUncrossedCell` reused (DRY) for both auto-cross and the regen
replay path.
- All persistence helpers validate input (`isValidNumberArray`,
`isUnticksArray`, size cap, `__proto__` reviver). Defense-in-depth
pattern is consistent across the new `loto_master` key and the
pre-existing `loto_grid`/`loto_crossed` paths.
- Bus is fully retired. Only stale reference is a doc comment.
- The locked product decisions (master Ván mới wipes; player Xoá đánh
dấu replays) are implemented exactly as specified.
- `manualUnticks` solves the "regen wipes my manual untick" problem
cleanly — minimal state, mirrors a single user intent.
---
## Unresolved questions
1. H1: is the player→both first-toggle replay surprising or expected?
Phase 3 doc claims "no replay on toggle" but actual behavior depends
on whether MasterPanel mounted earlier.
2. Should `prevCalledLen` and `lastHandledIndex` drop the `$state`
wrapper since they're not consumed by templates? (perf nit, not
correctness)
3. Multi-tab scenario explicitly out of scope (#20) — confirmed in
plan.md, no action.
web/plans/reports/researcher-260430-2131-waiting-cell-ux.md
-34
View File
@@ -1,34 +0,0 @@
---
title: UI/UX Patterns for "Waiting Cell" Highlights in Lô Tô Grid
date: 2026-04-30
---
## Animation Patterns for "One-Away" Cell Highlighting
**Recommended approach:** Pulse with 2s cycle duration (most common in UI — combines attention without fatigue). [Material 3 Expressive emphasizes "guiding eyes without forcing attention"](https://supercharge.design/blog/material-3-expressive) via soft, physics-based pulses. Ring/glow variants work but consume more visual weight on a dense 9×9 grid. **Scale-bounce** risks overloading the card; **gradient sweep** adds motion fatigue over long dwell times.
## Overlay Opacity for Prominence + Readability
Material Design opacity system: **87% for primary emphasis, 60% for secondary, 38% for tertiary**. For a cell indicator layered over card content, use **70–85% opacity** on the highlight color to maintain readability of underlying numbers while achieving prominence. [MDN confirms static color at reduced opacity suffices for visual hierarchy](https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/At-rules/@media/prefers-reduced-motion) without motion dependency.
## Reduced-Motion Compliance
**Fallback:** Static color change (no motion required). Replace pulse animation with persistent 75–80% opacity highlight when `prefers-reduced-motion: reduce` is detected. [W3C guidance](https://www.w3.org/WAI/WCAG22/Techniques/css/C39) confirms static styling communicates state without barriers. **No motion is acceptable; animations are a refinement, not a requirement.**
## Loop Duration & Fatigue Prevention
Standard pulse: **2-second cycle** (common in Material Design and CSS animation libraries). **WCAG 2.2.2** requires any animation >5s to be pausable or auto-stop. For "waiting" state (indefinite), implement: pulse runs **2s cycles but auto-pauses after 3–5 cycles** (6–10s total), then re-engages on user interaction or row status change. Avoids vestibular/cognitive fatigue; [confirmed by accessibility research](https://usability.yale.edu/digital-accessibility/accessibility-resources/accessibility-articles/animated-content-and-timing).
## Color Choice: Amber Context
Vietnamese lotto culture traditionally uses **red/gold** as primary lucky colors (prosperity / Tết connotations). **Amber-500 is a safe choice** — it reads as warm/attention-grabbing without clashing with traditional reds. No specific cultural taboo against amber in lotto/gaming context; amber sits between red (luck) and yellow (wealth). **Recommendation:** Keep amber-500 if already established in your toast pattern; adds visual consistency between notification and card highlight.
---
**Sources:**
- [Material 3 Expressive Design](https://supercharge.design/blog/material-3-expressive)
- [MDN Web Docs: prefers-reduced-motion](https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/At-rules/@media/prefers-reduced-motion)
- [W3C WCAG Techniques: CSS prefers-reduced-motion](https://www.w3.org/WAI/WCAG22/Techniques/css/C39)
- [Yale Digital Accessibility: Animation & Timing](https://usability.yale.edu/digital-accessibility/accessibility-resources/accessibility-articles/animated-content-and-timing)
- [Material Design States](https://m3.material.io/foundations/interaction/states/state-layers)
- [CSS-Tricks: Accessible Web Animation & WCAG](https://css-tricks.com/accessible-web-animation-the-wcag-on-animation-explained/)
web/plans/reports/security-260427-1151-full-audit.md
-67
View File
@@ -1,67 +0,0 @@
# Security & Reliability Audit — Lô tô
Date: 2026-04-27 | Scope: SvelteKit static export, no backend.
Method: STRIDE + OWASP Top 10, manual review.
Files scanned: 11 (`src/**`), build/deploy configs, `npm audit`.
## Summary
- 0 Critical, 0 High, 3 Medium, 4 Low, 5 Informational.
- No outbound network calls; no `{@html}`, `innerHTML`, `eval`, `fetch`, `XHR`, `WebSocket`, `sendBeacon` anywhere in `src/`. Privacy-clean.
- All user input flows through validators; static export served with redirects only.
## Findings
### Medium
**M1 — No CSP / security headers configured.**
`static/_redirects` only handles SPA fallback; no `_headers` file for Cloudflare Pages, no headers in GitHub Pages action. Missing `Content-Security-Policy`, `X-Content-Type-Options: nosniff`, `Referrer-Policy`, `Permissions-Policy`, `X-Frame-Options`. With no CSP, defense-in-depth against future XSS or third-party-origin embedding is absent. Recommend adding `static/_headers` with restrictive CSP (`default-src 'self'; img-src 'self' data:; media-src 'self'; style-src 'self' 'unsafe-inline'; script-src 'self'; connect-src 'self'; frame-ancestors 'none'`). `'unsafe-inline'` for style is needed because Svelte emits inline `style:` attributes. (file: `static/_redirects`)
**M2 — `loadSettings` does not reject `__proto__` / `constructor` keys before destructuring.**
`settings-store.svelte.js:125` does `JSON.parse(raw) ?? {}` then reads named properties. JSON.parse of `{"__proto__":{"polluted":1}}` does NOT pollute `Object.prototype` (modern JS treats `__proto__` from JSON as an own property), so this is theoretically safe today, but the validators read named props directly off `parsed` without `Object.hasOwn` checks. If future code ever uses spread or `Object.assign(target, parsed)` the surface grows. Same pattern in `game-logic.js:171` (`safeParse`). Defense-in-depth: parse with `JSON.parse(raw, (k,v) => k === "__proto__" ? undefined : v)` or check `Object.hasOwn`. (files: `src/lib/settings-store.svelte.js:121-153`, `src/lib/game-logic.js:168-176`)
**M3 — Auto-call interval lower bound borderline; no oversize-payload guard on localStorage reads.**
`autoCallSpeed` validated 1-10s — fine. But `loadGrid` / `loadCrossedState` accept arbitrarily large strings up to localStorage's per-origin quota (~5-10MB) before `JSON.parse`. A poisoned origin (browser extension, shared device) could store a megabyte-scale string; parse stalls UI on mount. Add a length cap (`raw.length > 50_000` reject) before parse. Same applies to `loto_settings` and `loto_master`. (files: `src/lib/game-logic.js:222-228, 246-252`, `src/lib/settings-store.svelte.js:123`, `src/lib/MasterPanel.svelte:51-59`)
### Low
**L1 — `clipUrl` builds path from `settings.voice` only; relies entirely on `validVoiceId` allowlist.**
`voice.js:38-40` interpolates `settings.voice` into a URL. `validVoiceId` (settings-store.svelte.js:67) checks `VOICE_IDS.has(v)` — Set of strings from the bundled manifest. Path-traversal blocked because allowlist values are static slugs. If `settings.voice` is ever set without going through validation (e.g. direct mutation in a future feature), `..%2F` could escape. Add a defensive `encodeURIComponent` on `name` and `settings.voice` in `clipUrl` for resilience. Clip `name` is currently always internal (`String(n)`, `"cho"`, `"kinh"`) — safe. (file: `src/lib/voice.js:38-40`)
**L2 — Audio cache unbounded growth.**
`voice.js:11` keeps every constructed `<audio>` in a `Map` keyed by URL. Per voice = 92 entries (1-90 + cho + kinh). `clearAudioCache` only fires on voice change. With 2 voices today this is fine; if voices grow past ~10 the cache holds ~1000 audio elements. Add LRU cap or drop cache when document hides for >N min. (file: `src/lib/voice.js:11-49`)
**L3 — `crypto.getRandomValues` not used for grid / call shuffles.**
`game-logic.js:31, 104, 107` and `MasterPanel.svelte:35-37` use `Math.random` for shuffle and combination selection. Game has no stake / no fairness guarantees so practical impact is zero, but `Math.random()` is V8's xorshift128+, not cryptographically random; `arr.sort(() => 0.5 - Math.random())` in `randomNumbersInCol` is also a known-biased shuffle (use Fisher-Yates throughout). Information only — replace with Fisher-Yates for bias correctness, not security. (files: `src/lib/game-logic.js:31`, `src/lib/MasterPanel.svelte:35-39`)
**L4 — `npm audit` reports 3 Low vulns (transitive `cookie` <0.7.0 via @sveltejs/kit).**
Static export — runtime never parses cookies, so the OOB-character issue (CWE-74) is not exploitable here. Bump SvelteKit to a release shipping `cookie` ≥0.7.0 next dependency sweep.
### Informational
**I1 — `confirm()` race vs auto-call.**
`MasterPanel.svelte:134` shows native `confirm()` for "Ván mới". `setInterval` callback runs while modal is open (browser-dependent — Chrome pauses, Safari may not). On rapid double-click of "Ván mới" while autoRunning, state could mutate mid-confirmation. Low practical impact. (file: `src/lib/MasterPanel.svelte:99-113, 133-140`)
**I2 — `bus.lastDrawn` never cleared after `bus.lastDrawn` set, only on `resetBus()`.**
If master mode toggles off/on with `mode === "both"` after a draw, `PlayerBoard`'s auto-tick effect re-fires on remount with the stale draw and may double-mark. Effect already has the `!crossed[r][c]` guard so it's idempotent — but a stricter design clears `bus` on mode flip. (file: `src/lib/call-bus.svelte.js`, `src/lib/PlayerBoard.svelte:136-151`)
**I3 — `<a href="https://miti99.com" target="_blank" rel="noopener noreferrer">` is correctly hardened.** Both occurrences (`PlayerBoard.svelte:295`, `PageFooter.svelte`) include `noopener noreferrer`. No reverse-tabnabbing risk.
**I4 — `.env.local` committed to repo via tracked file but only contains non-secret hostnames.**
`.env.local` (in working tree, excluded by `.gitignore` for future) holds `CODESERVER_HOST=codeserver.sg.miti99.com`. Not a credential, but reveals dev host. Confirm gitignore excludes it (it does — `.env*.local`). No remediation needed if not in git history. Verify with `git log --all -- .env.local`.
**I5 — GitHub Pages action interpolates `DEST` into HTML via shell heredoc.**
`.github/workflows/deploy-github-pages.yml:24-50` builds redirect HTML. `DEST` is hardcoded; if it ever becomes a workflow input, the heredoc would shell-interpolate without escaping. Currently safe.
## Trust-boundary verification
- `--empty-cell-bg` injection (M2 from spec): regex `/^#[0-9a-fA-F]{6}$/` is **tight** — anchored, length-fixed, no whitespace. Cannot escape into `;color:red;` or url() injection. PASS.
- localStorage poisoning: type validators present for every key; falls back to default on mismatch. The remaining gap is M2 above (proto keys / payload size). PASS with caveat.
- Voice path traversal: allowlist enforced; clip names internal-only. PASS.
- Resource exhaustion: auto-call clamped 1-10s, `setInterval` cleared on dependency change. Audio cache addressed in L2.
- Privacy: `grep -rn "fetch\|XMLHttpRequest\|sendBeacon\|WebSocket\|EventSource" src/` returns 0 hits. PASS.
- Build/deploy secrets: none in `wrangler.toml`, `svelte.config.js`, `.env.example`. `.env.local` in tree but only non-secret hostnames.
## Unresolved questions
1. Is `'unsafe-inline'` style acceptable for the M1 CSP, or should Svelte be configured to extract `style:` directives into hashed inline blocks?
2. Will the voice list grow beyond 2 entries (affects L2 priority)?
3. Is `.env.local` in any historical git commit, or only working tree? (run `git log --all --oneline -- .env.local`)
4. Cloudflare Pages serves `_headers` — is GitHub Pages mirror also expected to enforce the same headers, or is the GitHub deploy redirect-only (so headers don't matter for that origin)?
web/plans/reports/security-260427-2047-pass2-full.md
-73
View File
@@ -1,73 +0,0 @@
# Security & Reliability Audit — Lô tô (Pass 2)
Date: 2026-04-27 | Scope: SvelteKit static export + new PWA layer (since `f28279b`).
Method: STRIDE + OWASP Top 10, manual.
Files scanned: `src/**`, `static/_headers`, `static/manifest.webmanifest`, `vite.config.js`, `package.json`, `npm audit`.
## Summary
- 0 Critical, 0 High, **2 Medium, 4 Low, 5 Informational**.
- All 4 pass-1 mediums (M1 CSP, M2 proto-pollution, M3 payload cap, L1 encodeURIComponent) **resolved**.
- `npm audit` clean (overrides verified: `cookie@0.7.2`, `serialize-javascript@7.0.5` — no API breakage; both transitive-only at build time).
- No outbound network calls; no `{@html}`, `eval`, `fetch`, `XHR`, `WebSocket` anywhere in `src/`.
## Findings
### Medium
**M1 — CSP gap: SW-update / workbox / manifest fetches not explicitly covered, and `style-src 'unsafe-inline'` likely still required.**
`static/_headers:2`. `connect-src 'self'` covers SW `update()` GETs, OK. But: (a) workbox's generated `sw.js` registers via `navigator.serviceWorker.register('/sw.js')` — covered by `worker-src 'self'`. (b) The plugin emits an inline `<script>` registration block in `app.html` injected at build; check post-build `index.html` for inline `<script>` — if present, current `script-src 'self'` will block it. `script-src` lacks `'unsafe-inline'` AND no nonce/hash declared (header docstring says "nonce for scripts" but no `'nonce-…'` token in the policy string). Verify built output. (c) `'unsafe-inline'` for style: confirmed needed — `style:` directives at `MasterPanel.svelte:309`, `PlayerBoard.svelte:282,356,384-386`, `SettingsButton.svelte:418`, `MasterEmptyState.svelte:18` all compile to inline `style=` attributes. Nonces don't help inline attributes; CSP3 `'unsafe-hashes'` does but isn't widely supported. Keep `'unsafe-inline'` for style. (file: `static/_headers:2`)
**M2 — Service-worker precache integrity not protected by SRI; CacheFirst stores opaque (status 0) responses.**
`vite.config.js:54-67`. `cacheableResponse: { statuses: [0, 200] }` is **required** for CDN-served audio if range requests strip CORS, but it also means a poisoned CDN response is cached as opaque and served forever (CacheFirst). Risk vector: hijacked Cloudflare edge OR rogue CF Pages preview deploy serves a malicious mp3; SW caches and replays for the 30-day TTL. Mitigations available: (a) drop status `0` (audio is same-origin via `${base}/audio/...`, so 200 is sufficient — no opaque needed); (b) bump cache name on each redeploy via revision (already done for precache via `audio-v1-...`, but **not for runtime cache**). Recommend `cacheableResponse: { statuses: [200] }`. Same-origin `<audio>` does not need opaque mode. (file: `vite.config.js:65`)
### Low
**L1 — Manifest `start_url: "."` resolves relative; subpath deploys could mislead PWA install scope.**
`static/manifest.webmanifest:5-6`. Both GH-Pages (`/loto/`) and CF root use `"."` which resolves at the manifest URL. Acceptable, but if `_headers` ever serves `manifest.webmanifest` from a non-root path with redirect, scope can drift. Defense-in-depth: pin `start_url` to absolute path per build profile, or `"./"` (trailing slash) for clarity. Cache-Control `no-cache` (line 13 of `_headers`) is correct. (file: `static/manifest.webmanifest:5`)
**L2 — Audio runtime cache `maxAgeSeconds: 30 days` with no integrity revision.**
`vite.config.js:60-64`. Precache entries get `revision: audio-v1-...`. Runtime cache (alternate voices) has no revision → if a voice clip is regenerated, clients hold the stale 30-day copy until natural eviction. Functional issue (stale audio), low security impact. Bump prefix → also bump runtime `cacheName` in same release to force purge. (file: `vite.config.js:59`)
**L3 — Reactive bus is module-level singleton — fine for same-tab single user, but BroadcastChannel-style leakage if SW ever cross-posts.**
`src/lib/call-bus.svelte.js:10-13`. `bus` lives in JS module memory, scoped per tab/window — no cross-tab leak. SW does not import it (SW runs in separate context). PASS for current arch. Risk would only appear if a future feature uses `BroadcastChannel`/`postMessage` to mirror draws; document that bus is intentionally tab-local. (file: `src/lib/call-bus.svelte.js`)
**L4 — `MasterPanel.loadState` minimal validator accepts any array shape.**
`MasterPanel.svelte:55-75`. Reviver strips `__proto__`/`constructor`, length cap 16 KB, `Array.isArray` on both halves — but elements are not type-checked. Poisoned origin could store `{called: ["💀"], remaining: [{}]}` → `state.called[i]` rendered into DOM at line 288 as `{num}` (Svelte text-interpolation auto-escapes, so no XSS), but `callOrder.get(num)` and number comparisons silently produce NaN/`undefined`. Ugly UI, no security breach. Add `n => typeof n === "number" && n >= 1 && n <= 90` per element if hardening further. (file: `src/lib/MasterPanel.svelte:64-69`)
### Informational
**I1 — `'unsafe-inline'` for style is unavoidable today.** Svelte's `style:` compiles to inline attributes. Hash-based or nonce-based style CSP would require Svelte-side opt-out + extracting all dynamic styles to CSS variables (already done for `--empty-cell-bg`; not done for confetti, master cell bg toggle).
**I2 — `frame-ancestors 'none' + X-Frame-Options: DENY`**: belt-and-braces, OK.
**I3 — `manifest-src 'self'` correctly added.** Required by Chrome since 2020. PASS.
**I4 — Self-hosted font (`@fontsource/roboto-condensed`)** removes Google Fonts CDN dependency → `font-src 'self' data:` is sufficient and tight. PASS.
**I5 — npm overrides verified non-breaking.** `npm ls cookie serialize-javascript` resolves cleanly to `0.7.2` and `7.0.5`; no peer-dep warnings; build artifacts unchanged. Both packages are build-time only (kit dev internals + workbox-build via @rollup/plugin-terser) — runtime never executes them. PASS.
## Trust-boundary verification (re-confirmed)
- localStorage payload caps applied to **all 4 keys**: `loto_settings` (8KB, `settings-store.svelte.js:17,127`), `loto_grid` + `loto_crossed` (32KB, `game-logic.js:169,180`), `loto_master` (16KB, `MasterPanel.svelte:53,58`). PASS.
- `__proto__`/`constructor` reviver applied in **all 3 parse sites**: `settings-store.svelte.js:130`, `game-logic.js:182`, `MasterPanel.svelte:59`. PASS.
- `clipUrl` `encodeURIComponent` belt-and-braces on both `voice` + `name` (`voice.js:41`). PASS.
- Voice allowlist `VOICE_IDS.has(v)` (`settings-store.svelte.js:71`). PASS.
- CSP `frame-ancestors 'none'` blocks clickjacking. PASS.
- SW: `registerType: "autoUpdate"` without `skipWaiting` (`vite.config.js:40`) — explicit comment confirms intent. PASS.
- `noopener noreferrer` on all external `<a target="_blank">`. PASS.
## Pass-1 follow-ups status
| ID | Item | Status |
|----|------|--------|
| M1 (pass1) | CSP added | RESOLVED |
| M2 (pass1) | proto/constructor stripping | RESOLVED (3 sites) |
| M3 (pass1) | localStorage payload caps | RESOLVED (4 keys) |
| L1 (pass1) | encodeURIComponent on clipUrl | RESOLVED |
| L2 (pass1) | Audio cache LRU | DEFERRED — acceptable while voice count ≤ ~5; SW cache now also caps at 400 entries |
| L3 (pass1) | crypto.getRandomValues | DEFERRED — not security-relevant |
| L4 (pass1) | cookie <0.7.0 | RESOLVED via override |
## Unresolved questions
1. Inspect post-build `build/index.html` — does workbox/SvelteKit-PWA inject any inline `<script>` for SW registration? If yes, `script-src 'self'` blocks it (M1c). Run `npm run build && grep -c "<script>" build/index.html`.
2. Is `cacheableResponse.statuses: [0]` actually needed for same-origin `/audio/*.mp3`, or can it be tightened to `[200]` only? (M2)
3. GitHub Pages mirror — does it serve `_headers`? (GH Pages ignores `_headers`; CSP only enforced on CF.) Acceptable since GH is mirror-only?
4. Should runtime `cacheName: "loto-audio"` be versioned (e.g. `loto-audio-v1`) so a future audio regen forces purge? (L2)
web/plans/reports/ui-ux-designer-260427-1151-full-audit.md
-118
View File
@@ -1,118 +0,0 @@
# Lô tô — Full UI/UX Audit (260427)
Stack: SvelteKit + Tailwind 4. Mobile-first, single-page. Audit scope: app.css, +page.svelte, PlayerBoard, MasterPanel, SettingsButton, PageFooter.
Severity: P0 = breaks UX/a11y, P1 = noticeable polish/clarity, P2 = nice-to-have.
---
## P0
### 1. Master-only mode looks abandoned on first load
**Where:** `+page.svelte:37-52`, `MasterPanel.svelte:323-327`.
Mode=master + no game started = page shows only "Ván mới" button + "Nhấn 'Ván mới' để bắt đầu" text. No header context, no preview, no hero. Feels like a broken page, not "the master tool".
**Fix sketch:** add empty-state hero (decorative 11x9 ghost board mock like PlayerBoard's preview, or a "Chuẩn bị xổ số" illustration). Surface a "Đang ở chế độ Quản trò" pill near top so the role is explicit when no player board sits above.
### 2. `cell-crossed` red diagonal — contrast on green winning row
**Where:** `app.css:184-196`, `PlayerBoard.svelte:281`.
Winning row uses `bg-emerald-100`/`text-emerald-700` + `#ef4444` diagonal. Red-on-mint is OK, but the `text-emerald-300` dark variant + red slash + dim emerald-900/40 bg = ~2.5:1 text contrast in dark. Below WCAG AA.
**Fix sketch:** for dark winning cells, lift text to `emerald-200` and bump bg to `emerald-900/60`; or use a lighter slash color (rose-300) on dark.
### 3. `tan-tan-num` font stack omits Vietnamese-safe fallback
**Where:** `app.css:131-142`. "Arial Narrow", "Avenir Next Condensed", "Roboto Condensed" — none are guaranteed to ship Vietnamese diacritics consistently on Android (esp. older WebView). Player numbers won't render diacritics, but the same class is reused for fallback rendering and any future label. Still, Roboto Condensed isn't loaded — relies on system. On Linux/Android without it, you get default sans, defeating the "tall + tight" aesthetic.
**Fix sketch:** load a self-hosted Vietnamese-supporting condensed display face (e.g. Bebas Neue Vie, Oswald subset latin-ext+vietnamese) via @font-face; keep current as fallback.
### 4. Toast overlays board content — blocks the cell underneath
**Where:** `PlayerBoard.svelte:321-336`. Toast is `absolute inset-0` flex-centered, pointer-events-none on wrapper but the button itself eats clicks for ~5s. Lands smack on the middle row.
**Fix sketch:** anchor toast to top-edge or below grid (translate-y above the grid) — never centered over playable cells.
---
## P1
### 5. Three-mode picker has no icon/visual cue
**Where:** `SettingsButton.svelte:241-256`. "Người chơi / Quản trò / Cả hai" are text-only pills. Discoverability OK once modal opens but doesn't communicate role at a glance. "Cả hai" is ambiguous (both *what*?).
**Fix sketch:** add a tiny glyph above each label (player card icon / megaphone / split). Add a 1-line description under the active selection ("Hiển thị bảng người chơi" etc.) so users know *what changes*.
### 6. Light-mode `--section-band-bg` (`#e3f2fd 70%`) over amber radial = muddy edge
**Where:** `app.css:24, 13-17`. Cool blue band on warm amber glow at top → visible color clash on the topmost section label.
**Fix sketch:** either warm the band tint (cream or rose tinge) or darken the section accent slightly so band reads on either glow. Test against amber and indigo variants.
### 7. Master hero number circle — fixed `w-40 h-40` clips on 320px screens with border-[8px]
**Where:** `MasterPanel.svelte:220-237`. 160px circle + 16px ring + page padding (`px-2`=8) + `flex flex-col items-center` = fits, but the called-history scrolls awkwardly because hero never collapses. On scroll-into-view, header gets pushed off, host loses orientation.
**Fix sketch:** sticky top mini-pill of last-called when hero is scrolled offscreen. Or shrink hero to `w-32 h-32` ≤375px.
### 8. Auto-call sub-fieldset density unclear
**Where:** `SettingsButton.svelte:260-294`. Slider sits inside the same fieldset as the toggle, no visual separation, label "Tốc độ" floats. When toggle off, fieldset shrinks abruptly with no transition — jarring.
**Fix sketch:** indent slider with left border (mirror voice-waiting nesting at line 319). Add slide transition. Add tick labels at 1s/5s/10s.
### 9. Voice nesting reads: "Quản trò đọc số" + sub-text "Đọc số đã xổ + báo Chờ/Kinh khi ở Cả hai"
**Where:** `SettingsButton.svelte:307-314`. The hint text only renders when mode≠player. So in mode=player, "Quản trò đọc số" toggle is shown with no explanation — confusing because there is no master visible.
**Fix sketch:** hide the master voice toggle entirely in mode=player (it has no effect there) or add a hint that explains it's for "Cả hai" mode.
### 10. Color-picker: presets are 5×2 grid, hex code is tiny + no swatch label
**Where:** `SettingsButton.svelte:373-389`. 10 color squares with no name tooltip. Selected state is a 110% scale + indigo ring — but the *current custom* hex shown next to the native picker doesn't update its visual when a preset is clicked (it does, but the `<input type="color">` and `<code>` block are visually disconnected from the preset row).
**Fix sketch:** wrap picker + hex + presets in one card with a "Tuỳ chỉnh" / "Mẫu sẵn" sub-divide. Show hex *inside* the selected swatch overlay.
### 11. Settings modal: "Mặc định" button is barely visible (slate-600 text, no border)
**Where:** `SettingsButton.svelte:392-401`. Reset is a destructive-ish action. Currently looks like a footer link, easily missed or accidentally hit.
**Fix sketch:** ghost-button style with subtle border, or move under a divider line with smaller "Đặt lại tất cả" text + confirm dialog.
### 12. Header brand "Lô tô" + sub "Hội chợ TN1" — sub is muted slate, italic, all-caps tracking-[0.28em]
**Where:** `+page.svelte:26-30`. Reads more "fintech subhead" than fairground. Off-brand for festive.
**Fix sketch:** swap to a hand-drawn or display Vietnamese script font for sub; or wrap in decorative dashes/dots ("· Hội chợ TN1 ·"). Lower tracking (.18em) and add a tiny string-light or paper-lantern emoji.
### 13. "Kinh!" celebration modal — purple/pink gradient, generic
**Where:** `PlayerBoard.svelte:389-439`. Confetti overlay only triggers on tier 2 (3+ bingos). First bingo modal is plain-ish for a fairground game. No row number callout in an oversized way.
**Fix sketch:** for first bingo use a single confetti burst (not the rain), and make `Hàng X` huge (text-5xl) — that's the actual win info.
---
## P2
### 14. Footer dual cross-hatch label "Made by miti99" — visually heavy near grid bottom
**Where:** `PlayerBoard.svelte:291-314`. Reuses `.section-label` (which has flanking ✚✚✚). Then `PageFooter.svelte` adds a *second* "Made by miti99" line. Duplicate attribution.
**Fix sketch:** drop the in-card credit (it duplicates the footer); keep just the cross-hatch decorative band with no text, or replace with "TN1 · Lô tô" branding.
### 15. Confetti emoji set "🎊✨🎉🥳" — same chunkiness, all sit at 2rem
**Where:** `PlayerBoard.svelte:34, 175-182`. Visual variety low; no rotation in fall start.
**Fix sketch:** add Vietnamese-flavored bits: 🥢 🎋 🏮 (lantern!) and randomize size 1.5–2.5rem.
### 16. Master called-history pills — pink/green border with cream fill, OK contrast but identical shape to hero
**Where:** `MasterPanel.svelte:253-268`. Mini repeats hero styling. `tabular-nums` good. But `border-[3px]` on 36px pill = ~31px usable. Numbers like 88 cramped.
**Fix sketch:** drop border to 2px on pills; reserve 3px-ring treatment for hero only.
### 17. `cell-crossed` slash uses fixed `#ef4444` red — same on green winning bg as on red losing bg
**Where:** `app.css:184-196`. Green winning row deserves a green or gold slash to communicate "complete" not "marked".
**Fix sketch:** add a `.cell-crossed-win` modifier with emerald slash, applied when `rowComplete`.
### 18. `aria-live="assertive"` on hero number
**Where:** `MasterPanel.svelte:218`. SR users get every draw spammed assertively. Should be polite — assertive is reserved for warnings.
**Fix sketch:** `aria-live="polite"` is sufficient; lastCalled change is informational not urgent.
### 19. Reduced-motion not respected
**Where:** `app.css:144-182`. confetti-fall, bounce-slow, spin-slow, pop-in, toast — none gated by `@media (prefers-reduced-motion: reduce)`.
**Fix sketch:** wrap animation declarations in a media query that disables transforms/opacity loops, falling back to instant fade-in.
### 20. Empty cells use a single solid color across light & dark
**Where:** `app.css:20`, `PlayerBoard.svelte:259-266`. Default `#7030A0` purple over near-black `#050813` is fine but on light theme the same purple looks office-clipart-ish next to amber glow. The `bg-black/15` overlay in dark is a band-aid.
**Fix sketch:** allow per-theme cell color or auto-shift the saved color toward a more festive Tân Tân red/blue when theme=light and color=default.
### 21. No reset/clear keyboard shortcut, no swipe gestures on mobile
**Where:** general. Power user pain — tapping "Xổ số" 90 times.
**Fix sketch:** spacebar to draw next; long-press hero to undo last call. Nice-to-have only.
---
## Unresolved questions
1. Is the rose lower-glow in dark mode visible on tall pages? It's `at 50% 110%` but `background-attachment: fixed` — confirm intent: glow should track the viewport bottom, not the document.
2. Default mode is `"player"` — is that final? "Cả hai" might be the natural at-a-table default. Check telemetry/intent.
3. Is the in-card "Made by miti99" supposed to live there alongside footer attribution, or was it left over from before PageFooter existed?
4. Tier-2 confetti threshold = 3+ bingos per session. Is this per-card or per-page? Code says `celebratedRows.size` on a 9-row card — only triggers if 3 of 9 rows complete, which is rare in a single round.
5. Is there appetite for a per-row column highlight when "Chờ" toast fires (point at the row that's waiting)?
---
**Status:** DONE
**Summary:** 21 findings — 4 P0 (master empty state, dark winning-cell contrast, font Vietnamese fallback, toast blocks cells), 9 P1 (mode picker discoverability, modal density, brand mood, hero scaling), 8 P2.
web/plans/reports/ui-ux-designer-260427-2047-pass2-full.md
-125
View File
@@ -1,125 +0,0 @@
# Lô tô — UI/UX Audit Pass 2 (260427-2047)
Scope: re-audit since `f28279b` against current `main`. Read `app.css`, `app.html`, `manifest.webmanifest`, `+page.svelte`, `PlayerBoard`, `MasterPanel`, `MasterEmptyState`, `SettingsButton`, `PageFooter`. Severity P0/P1/P2.
Mostly clean follow-up. Most pass-1 items addressed well. Findings below are new or partial-fix regressions.
---
## P0
### 1. PWA splash + tab theme color hardcoded saturated blue, mismatches light page background
**Where:** `app.html:9` (`#1565c0` light) and `manifest.webmanifest:9-10` (`theme_color #1565c0`, `background_color #0a0f1f`).
Light app bg is `#f8fafc` (near-white) but Safari tab strip / Chrome top bar paints `#1565c0`. Hard color jump where the bg should bleed into the chrome. PWA splash on iOS uses `background_color` only — `#0a0f1f` (deep navy) → light-mode users get a dark-navy splash that flashes into a near-white app. Jarring on cold launch.
**Fix sketch:** light theme-color `#f3e9d7` or `#fff7ec` (warm off-white that echoes the amber top-glow). Manifest `background_color` to a neutral midpoint or fork by media: `background_color: #f8fafc`. iOS doesn't honor light/dark manifest yet, so pick the value matching the *more common* launch theme — `auto` defaults to user OS, so neutral cream is safer than near-black.
### 2. Tab title still bare "Lô tô" — PWA install card name mismatch
**Where:** `app.html:6` `<title>Lô tô</title>` vs `manifest.name "Lô tô — Hội chợ TN1"`.
Installed app shows full name; browser tab + history show only "Lô tô". Cold-share link previews lose the "Hội chợ TN1" context entirely. Also no Open Graph tags.
**Fix sketch:** title `Lô tô — Hội chợ TN1`; add `<meta property="og:title">`, `og:description`, `og:image` (use `icon-512.png`). One-time copy, immediate brand lift on share.
---
## P1
### 3. PlayerBoard empty-state and MasterEmptyState are near-clones with conflicting prompts
**Where:** `PlayerBoard.svelte:343-371` ghost grid + `MasterEmptyState.svelte:6-43` ghost grid.
Both render same opacity-30 monochrome ghost-grid pattern. Master mode in `both` shows player ghost AND master ghost stacked when no game started. Visual repetition; the page reads "two empty boxes" not "two distinct roles". Also: master grid shows 99 cells but real master board is 11×9=99 with last row = single cell at col 8 — ghost should mirror the actual silhouette.
**Fix sketch:** vary the ghosts visually — player ghost shows a subtle row-of-numbers stripe; master ghost shows scattered "called dots" pattern. Or hide the player ghost entirely in `both` mode pre-game (the master-mode hero CTA carries the call-to-action).
### 4. Mode picker glyphs read at first glance only for "player"; "master" megaphone is ambiguous, "both" looks like a stacked-window icon
**Where:** `SettingsButton.svelte:259-275`.
Player rect-with-grid-lines reads instantly = "card with rows". Master path `M3 11l14-6v14L3 13z` + arc is a megaphone but at 24×24 stroke 1.8 looks like an abstract triangle pointing right; not enough silhouette weight at 28px tall. "Both" stacked rectangles read as "two cards" not "player + master roles". Hint line below mitigates but the glyph itself doesn't sell.
**Fix sketch:** master = filled megaphone with sound waves (use stroke-width 2.2 + fill-on-active). "Both" = player-card-glyph layered with mini-megaphone badge in corner — composes the two prior glyphs. Keeps semantic continuity ("both = the two things above stacked").
### 5. "Đặt lại" reset chip — bordered now, but still no confirm dialog
**Where:** `SettingsButton.svelte:431-440`.
Pass-1 fix turned reset into a chip-with-border (good). But it still resets all settings (theme, mode, color, voice, auto-call) on a single tap. Easy mis-tap on mobile next to "Xong". No undo.
**Fix sketch:** `if (confirm("Đặt lại tất cả tuỳ chỉnh?"))` guard; or convert to two-step ("Tap to reset" → "Confirm reset" inline state for 3s). Native `confirm()` is fine here, low frequency.
### 6. Settings modal scroll on small viewports — sticky header/footer absent
**Where:** `SettingsButton.svelte:166-167`. `max-h-[90vh] overflow-y-auto` whole-modal scroll.
On 375×667 (iPhone SE) with both auto-call AND voice-waiting expanded, modal hits ~700px. User scrolls past "Cài đặt" title; "Đặt lại / Xong" footer scrolls off too — must scroll back to dismiss. Title and primary CTAs should be persistent.
**Fix sketch:** sticky title row (`sticky top-0 bg-white dark:bg-slate-800 -mx-6 px-6 pt-6 pb-3 z-10`), sticky footer row similarly. Inner content gets the scroll. Saves a scroll-trip per session.
### 7. Auto-call slider — still no tick labels at 1s/5s/10s
**Where:** `SettingsButton.svelte:306-316`.
Pass-1 noted this. Fixed: nesting + left-border indent (good). Not fixed: tick labels. Slider value floats free, user has no anchor for "what is fast vs slow".
**Fix sketch:** below slider, add `<div class="flex justify-between text-[10px] text-slate-400 mt-1"><span>1s</span><span>5s</span><span>10s</span></div>` aligned to track. Trivial.
### 8. Voice "Quản trò đọc số" hint copy still confusing in `both` mode
**Where:** `SettingsButton.svelte:336-338`. Pass-1 note: hide in player mode — done. But: hint says `Đọc số đã xổ + báo Chờ/Kinh khi ở "Cả hai".` In `master` mode the second clause ("báo Chờ/Kinh") is wrong — there's no player board to call Chờ/Kinh from in solo master.
**Fix sketch:** branch the hint by mode: master → `Đọc số đã xổ.`; both → `Đọc số đã xổ và báo Chờ/Kinh thay người chơi.`
### 9. Header subline `🏮 Hội chợ TN1` — lantern emoji renders monochrome on Windows/Linux, color on Apple/Android
**Where:** `+page.svelte:31`.
Cross-platform lantern inconsistency. On a Windows browser the lantern is line-art outlined, breaking the festive intent. Dashes-flank treatment is good though.
**Fix sketch:** ship a tiny SVG lantern inline (12×16, color: rose-500) in place of the emoji. Same byte cost, consistent across OS, theme-tintable.
### 10. Master "Số vừa xổ" hero — w-32 mobile is good (pass-1 fixed), but border-[6px] eats interior
**Where:** `MasterPanel.svelte:244-252`.
128px circle - 12px border (×2) = 104px interior for an 8xl number. Number renders fine but the ring feels chunky at this size; reads as "thick outlined badge", not "called number". Aspect feels token-y not announcer-y.
**Fix sketch:** `border-[4px] sm:border-[10px]`. Keep desktop chunk; trim mobile.
---
## P2
### 11. Section-divider hatch repeats under the bottom decorative band but with no label slot
**Where:** `PlayerBoard.svelte:316`. `<div class="section-label" aria-hidden="true"></div>` — empty label = just the cross-hatch flanks ::before/::after with `flex:1` and 0 gap. Visually thinner than divider above section-label rows.
**Fix sketch:** swap to `<div class="section-divider"></div>` — semantic match, consistent thickness. Tested: same color path via `--section-accent`.
### 12. `aria-live="assertive"` on hero number — still assertive in pass-1 had it as P2; updated to "polite" in code (good). No regression.
### 13. Confetti emoji set still `["🎊", "✨", "🎉", "🥳"]` — pass-1 P2 note re. lantern + Vietnamese-flavored set not addressed
**Where:** `PlayerBoard.svelte:35`.
Add `🏮 🎋 🥢` for fairground feel. Match the lantern in the header for cohesion.
### 14. `apple-touch-icon` only 192px (no 180px specifically; no `apple-touch-startup-image`)
**Where:** `app.html:11`. iOS scales 192→180 fine but loses sharpness. Missing splash image = bare-color flash on cold launch.
**Fix sketch:** generate `icon-180.png` and `apple-splash-{2048x2732,1668x2388,1170x2532}.png` from `source.svg`. Wire `<link rel="apple-touch-startup-image" media="...">`. Optional polish.
### 15. `MasterEmptyState` ghost grid uses `i % 11 < 9 && i % 7 === 0` — generates non-deterministic-looking sparse fill that doesn't mirror the master grid silhouette (col 8 row 10 only has 90)
**Where:** `MasterEmptyState.svelte:14`. Cosmetic — looks like a noise-ghost rather than a master-board ghost. Fine as decoration but pass on opportunity to communicate "tracking grid" affordance.
**Fix sketch:** mirror real `BOARD` shape: row 0 cols 1-8, rows 1-9 all cols, row 10 col 8. Then ghost-fill ~15-20% with a deterministic mod pattern.
### 16. Toast above grid (pass-1 fixed) — but on `both` mode the toast renders inside PlayerBoard's `relative` wrapper while master panel below pushes content; toast `-top-3` goes negative into the page-padding zone, can clip on very narrow screens (≤320px) where parent has only `px-2` (8px).
**Where:** `+page.svelte:11`, `PlayerBoard.svelte:330`.
Edge case (≤320 = older Android, rare). Toast still readable but center text near the screen edge.
**Fix sketch:** add `mx-2` on the toast button so it can compress safely; or move toast to `top-1` (positive offset, sits inside the rounded board chrome).
---
## Pass-1 fix verification
| Pass-1 item | Verdict |
|---|---|
| 1. Master empty state | ✅ MasterEmptyState added, role pill shown |
| 2. Dark winning row contrast | ✅ `bg-emerald-900/60 text-emerald-200` |
| 3. Vietnamese font fallback | ✅ self-hosted Roboto Condensed 700 |
| 4. Toast over cells | ✅ moved to `-top-3` (see P2 §16 edge case) |
| 5. Mode picker glyphs | ⚠️ added but readability mixed (P1 §4) |
| 7. Master hero w-40 mobile clip | ✅ w-32 sm:w-56 |
| 8. Auto-call slider density | ⚠️ partial — nesting fixed, no tick labels (P1 §7) |
| 9. Voice nesting wording | ⚠️ partial — hidden in player but copy still off in master (P1 §8) |
| 10. Color picker grouped | ✅ bordered card + sub-headers |
| 11. Reset button visibility | ⚠️ chip-bordered (good) but no confirm (P1 §5) |
| 12. Header subline brand mood | ✅ dash-flanked, lantern emoji (see P1 §9 cross-OS) |
| 13. Bingo modal row-number size | ✅ text-5xl/6xl |
| 18. aria-live polite on hero | ✅ |
| 19. Reduced-motion gating | ✅ media query added |
| 14. Footer dual attribution | ✅ in-card credit removed |
---
## Unresolved questions
1. Manifest `background_color: #0a0f1f` was chosen for dark; should we accept the light-mode PWA splash dark-flash, or pick a neutral cream? (P0 §1)
2. iOS Safari standalone — has it actually been tested on a device, or only DevTools simulated? Apple-status-bar `black-translucent` interacts with `safe-area-inset-top`; nothing in the layout reserves that inset.
3. Is there appetite to drop the `🏮` emoji entirely if the SVG-inline lantern is rejected? Plain dashes alone read fine and ship-stable. (P1 §9)
4. Auto-call max 10s — is that the right ceiling? Real-life Lô tô callers often pause 15-20s for call-and-response. Out of scope but data point.
---
**Status:** DONE
**Summary:** 16 findings — 2 P0 (PWA splash/theme color mismatch, tab title brand), 8 P1 (empty-state duplication, glyph readability, reset-confirm, sticky modal chrome, slider ticks, voice hint copy, lantern cross-OS, mobile hero ring), 6 P2. Pass-1 mostly addressed; partial-fixes on items 5/8/9/11.
web/plans/todo.md
+3 -2
View File
@@ -1,7 +1,8 @@
# Next-session TODO
Hand-off list as of 2026-04-28 (commit `9f24b6d`). All prior plan
folders have been deleted; residual / new items live here directly.
Hand-off list as of 2026-05-10. Deploy target is GitHub Pages
(`/loto/` base). All prior plan folders + their reports have been
swept after shipping; residual / new items live here directly.
## Highest leverage (start here)