From 4d89366debf25611bd0746754d55c968ef712c71 Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Sun, 10 May 2026 00:36:54 +0700 Subject: [PATCH] chore(plans): sweep shipped plan folders and orphaned reports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../phase-01-build-autocountdown.md | 110 ------ .../phase-02-integrate-and-verify.md | 130 ------- .../260430-1919-auto-call-countdown/plan.md | 36 -- .../phase-01-lift-master-state.md | 121 ------ .../phase-02-player-via-store.md | 157 -------- .../phase-03-replay-flows.md | 182 --------- .../phase-04-tests-and-verify.md | 110 ------ .../plan.md | 62 --- .../phase-01-wire-gh-pages-build-into-ci.md | 88 ----- .../phase-02-remove-cloudflare-artifacts.md | 87 ----- .../phase-03-update-docs-and-todo.md | 99 ----- .../plan.md | 41 -- ...nstorm-260430-2024-both-mode-edge-cases.md | 331 ---------------- ...ainstorm-260430-2114-voice-and-multitab.md | 140 ------- ...torm-260430-2131-waiting-cell-indicator.md | 183 --------- .../code-review-260426-1919-loto-project.md | 213 ----------- .../code-reviewer-260427-1036-three-mode.md | 68 ---- .../code-reviewer-260427-1151-full-project.md | 77 ---- .../code-reviewer-260427-2030-polish-pwa.md | 63 ---- .../code-reviewer-260427-2047-pass2-full.md | 83 ---- ...ode-reviewer-260430-1925-auto-countdown.md | 124 ------ ...iewer-260430-2024-both-mode-consistency.md | 182 --------- ...reviewer-260430-2103-both-mode-refactor.md | 354 ------------------ .../researcher-260430-2131-waiting-cell-ux.md | 34 -- .../security-260427-1151-full-audit.md | 67 ---- .../security-260427-2047-pass2-full.md | 73 ---- .../ui-ux-designer-260427-1151-full-audit.md | 118 ------ .../ui-ux-designer-260427-2047-pass2-full.md | 125 ------- web/plans/todo.md | 5 +- 29 files changed, 3 insertions(+), 3460 deletions(-) delete mode 100644 web/plans/260430-1919-auto-call-countdown/phase-01-build-autocountdown.md delete mode 100644 web/plans/260430-1919-auto-call-countdown/phase-02-integrate-and-verify.md delete mode 100644 web/plans/260430-1919-auto-call-countdown/plan.md delete mode 100644 web/plans/260430-2050-both-mode-state-consistency/phase-01-lift-master-state.md delete mode 100644 web/plans/260430-2050-both-mode-state-consistency/phase-02-player-via-store.md delete mode 100644 web/plans/260430-2050-both-mode-state-consistency/phase-03-replay-flows.md delete mode 100644 web/plans/260430-2050-both-mode-state-consistency/phase-04-tests-and-verify.md delete mode 100644 web/plans/260430-2050-both-mode-state-consistency/plan.md delete mode 100644 web/plans/260509-2247-switch-deploy-to-github-pages/phase-01-wire-gh-pages-build-into-ci.md delete mode 100644 web/plans/260509-2247-switch-deploy-to-github-pages/phase-02-remove-cloudflare-artifacts.md delete mode 100644 web/plans/260509-2247-switch-deploy-to-github-pages/phase-03-update-docs-and-todo.md delete mode 100644 web/plans/260509-2247-switch-deploy-to-github-pages/plan.md delete mode 100644 web/plans/reports/brainstorm-260430-2024-both-mode-edge-cases.md delete mode 100644 web/plans/reports/brainstorm-260430-2114-voice-and-multitab.md delete mode 100644 web/plans/reports/brainstorm-260430-2131-waiting-cell-indicator.md delete mode 100644 web/plans/reports/code-review-260426-1919-loto-project.md delete mode 100644 web/plans/reports/code-reviewer-260427-1036-three-mode.md delete mode 100644 web/plans/reports/code-reviewer-260427-1151-full-project.md delete mode 100644 web/plans/reports/code-reviewer-260427-2030-polish-pwa.md delete mode 100644 web/plans/reports/code-reviewer-260427-2047-pass2-full.md delete mode 100644 web/plans/reports/code-reviewer-260430-1925-auto-countdown.md delete mode 100644 web/plans/reports/code-reviewer-260430-2024-both-mode-consistency.md delete mode 100644 web/plans/reports/code-reviewer-260430-2103-both-mode-refactor.md delete mode 100644 web/plans/reports/researcher-260430-2131-waiting-cell-ux.md delete mode 100644 web/plans/reports/security-260427-1151-full-audit.md delete mode 100644 web/plans/reports/security-260427-2047-pass2-full.md delete mode 100644 web/plans/reports/ui-ux-designer-260427-1151-full-audit.md delete mode 100644 web/plans/reports/ui-ux-designer-260427-2047-pass2-full.md diff --git a/web/plans/260430-1919-auto-call-countdown/phase-01-build-autocountdown.md b/web/plans/260430-1919-auto-call-countdown/phase-01-build-autocountdown.md deleted file mode 100644 index acffaa7..0000000 --- a/web/plans/260430-1919-auto-call-countdown/phase-01-build-autocountdown.md +++ /dev/null @@ -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 `` 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 `
- -
- {/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 diff --git a/web/plans/260430-1919-auto-call-countdown/plan.md b/web/plans/260430-1919-auto-call-countdown/plan.md deleted file mode 100644 index 360dad1..0000000 --- a/web/plans/260430-1919-auto-call-countdown/plan.md +++ /dev/null @@ -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) diff --git a/web/plans/260430-2050-both-mode-state-consistency/phase-01-lift-master-state.md b/web/plans/260430-2050-both-mode-state-consistency/phase-01-lift-master-state.md deleted file mode 100644 index b3b4f54..0000000 --- a/web/plans/260430-2050-both-mode-state-consistency/phase-01-lift-master-state.md +++ /dev/null @@ -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. diff --git a/web/plans/260430-2050-both-mode-state-consistency/phase-02-player-via-store.md b/web/plans/260430-2050-both-mode-state-consistency/phase-02-player-via-store.md deleted file mode 100644 index 2346cd8..0000000 --- a/web/plans/260430-2050-both-mode-state-consistency/phase-02-player-via-store.md +++ /dev/null @@ -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} 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` 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. diff --git a/web/plans/260430-2050-both-mode-state-consistency/phase-03-replay-flows.md b/web/plans/260430-2050-both-mode-state-consistency/phase-03-replay-flows.md deleted file mode 100644 index 9dbfd25..0000000 --- a/web/plans/260430-2050-both-mode-state-consistency/phase-03-replay-flows.md +++ /dev/null @@ -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. diff --git a/web/plans/260430-2050-both-mode-state-consistency/phase-04-tests-and-verify.md b/web/plans/260430-2050-both-mode-state-consistency/phase-04-tests-and-verify.md deleted file mode 100644 index 178e4df..0000000 --- a/web/plans/260430-2050-both-mode-state-consistency/phase-04-tests-and-verify.md +++ /dev/null @@ -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 diff --git a/web/plans/260430-2050-both-mode-state-consistency/plan.md b/web/plans/260430-2050-both-mode-state-consistency/plan.md deleted file mode 100644 index 017b610..0000000 --- a/web/plans/260430-2050-both-mode-state-consistency/plan.md +++ /dev/null @@ -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 diff --git a/web/plans/260509-2247-switch-deploy-to-github-pages/phase-01-wire-gh-pages-build-into-ci.md b/web/plans/260509-2247-switch-deploy-to-github-pages/phase-01-wire-gh-pages-build-into-ci.md deleted file mode 100644 index 80a73a3..0000000 --- a/web/plans/260509-2247-switch-deploy-to-github-pages/phase-01-wire-gh-pages-build-into-ci.md +++ /dev/null @@ -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. diff --git a/web/plans/260509-2247-switch-deploy-to-github-pages/phase-02-remove-cloudflare-artifacts.md b/web/plans/260509-2247-switch-deploy-to-github-pages/phase-02-remove-cloudflare-artifacts.md deleted file mode 100644 index cbb915c..0000000 --- a/web/plans/260509-2247-switch-deploy-to-github-pages/phase-02-remove-cloudflare-artifacts.md +++ /dev/null @@ -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. diff --git a/web/plans/260509-2247-switch-deploy-to-github-pages/phase-03-update-docs-and-todo.md b/web/plans/260509-2247-switch-deploy-to-github-pages/phase-03-update-docs-and-todo.md deleted file mode 100644 index 93f4fb4..0000000 --- a/web/plans/260509-2247-switch-deploy-to-github-pages/phase-03-update-docs-and-todo.md +++ /dev/null @@ -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. diff --git a/web/plans/260509-2247-switch-deploy-to-github-pages/plan.md b/web/plans/260509-2247-switch-deploy-to-github-pages/plan.md deleted file mode 100644 index eaf717e..0000000 --- a/web/plans/260509-2247-switch-deploy-to-github-pages/plan.md +++ /dev/null @@ -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). diff --git a/web/plans/reports/brainstorm-260430-2024-both-mode-edge-cases.md b/web/plans/reports/brainstorm-260430-2024-both-mode-edge-cases.md deleted file mode 100644 index 29635a1..0000000 --- a/web/plans/reports/brainstorm-260430-2024-both-mode-edge-cases.md +++ /dev/null @@ -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) diff --git a/web/plans/reports/brainstorm-260430-2114-voice-and-multitab.md b/web/plans/reports/brainstorm-260430-2114-voice-and-multitab.md deleted file mode 100644 index 9b24e18..0000000 --- a/web/plans/reports/brainstorm-260430-2114-voice-and-multitab.md +++ /dev/null @@ -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"} - -{/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. - diff --git a/web/plans/reports/brainstorm-260430-2131-waiting-cell-indicator.md b/web/plans/reports/brainstorm-260430-2131-waiting-cell-indicator.md deleted file mode 100644 index c3bb50c..0000000 --- a/web/plans/reports/brainstorm-260430-2131-waiting-cell-indicator.md +++ /dev/null @@ -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 - -``` - -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. diff --git a/web/plans/reports/code-review-260426-1919-loto-project.md b/web/plans/reports/code-review-260426-1919-loto-project.md deleted file mode 100644 index c5cdfab..0000000 --- a/web/plans/reports/code-review-260426-1919-loto-project.md +++ /dev/null @@ -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 `
` only — not focusable, not announced). -5. Split `master/page.tsx` (244 lines) into `use-master-state` hook + `` + `` 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(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 -- `
` 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 `` 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 `