diff --git a/CLAUDE.md b/CLAUDE.md index 2ab2536..74080e6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -25,7 +25,7 @@ For detailed information about this project, refer to the documentation files in **Key Guidelines:** - **JavaScript Only**: No TypeScript files (.ts, .tsx) -- **Function Parameters**: Use individual parameters, not object destructuring +- **Function Parameters**: Use individual parameters, not object destructuring, in library functions, scripts and route helpers. React components take a single props object and destructure it, as every component here does - **File Modifications**: Only modify source code, `/docs`, and `/plans` - **Generated Data**: `src/data/` is build output; the panorama index lives in Neon Postgres, seeded from gitignored `data-build/panos/` artifacts. Change diff --git a/docs/development.md b/docs/development.md index fcb9b66..1a75056 100644 --- a/docs/development.md +++ b/docs/development.md @@ -23,9 +23,55 @@ - All components and utilities should be .js or .jsx files ### Function Parameters -- All functions should use **individual parameters** instead of object destructuring -- Use `function(param1, param2)` instead of `function({param1, param2})` -- This applies to React components, utility functions, and API handlers +- Library functions, scripts and route helpers use **individual parameters** + instead of object destructuring: `function(param1, param2)`, not + `function({param1, param2})` +- React components are the exception: they take one props object and + destructure it, which is what every component here does and what React + expects + +### Conventions the code follows +Each of these is enforced by a test or a lint rule where one exists; the rest +are the shape the code has and new code should keep. + +- **Server-only modules say so** in their header comment, and the import-graph + test in `tests/regions.test.js` keeps `src/lib/regions.js` from reaching + `pano-index.js`, `pano-db.js` or `daily.js`. Anything that touches exact + panorama coordinates is server-only. +- **The route decides what is best-effort; the library never swallows.** A + library function throws on failure. A route wraps the writes it can afford + to lose (recent-location history, distance records, statistics) in a small + `...OrIgnore` / `...OrNone` helper with a comment saying why, and lets the + load-bearing write (the session, the score) fail loudly. +- **Failure kinds are classes, not message prefixes**: `DryPoolError` and + `UpstreamError` in `src/lib/errors.js`. A route maps them to statuses (404 + for a dry pool, 502 for an upstream failure); nothing string-matches an + error message. +- **Claim before write.** A session is consumed with an atomic `DEL` before any + board is written, and the fan-out after it settles per level rather than + all-or-nothing, because nothing after the claim can be retried. +- **One module per browser-storage concern**, all reading through + `src/lib/storage.js` (never throws, notifies watchers) and rendered through + `useStoredValue` in `src/lib/use-stored-value.js`. No component seeds state + from storage in an effect; the `react-hooks/set-state-in-effect` rule is an + error. +- **Callback props reach imperative handlers through `useEffectEvent`**, not + refs written during render; the `react-hooks/refs` rule is an error. The one + data ref written in render (`CoverageMap.js`) carries an inline disable and + its reason. +- **Logical Redis keys are unprefixed** and the adapter in `src/lib/upstash.js` + applies `KEY_PREFIX`. A new Redis command means a new adapter function and a + matching method on `tests/fake-upstash-redis.js`. +- **Three region levels, fixed**: country, province, district. Codes are + uppercase inside the app and lowercase in URLs (`regionSlug`); players see + `regionName()`, the accented form. +- **`URLSearchParams.get` is read with `||`, not `??`**: `?region=` yields + `''`, which must mean "absent". +- **API responses are `{ success, ... }` on success and + `{ success: false, error, reason? }` on failure**, with a real HTTP status. + `/api/guess` returns `gameResult` alone; the e2e stubs in + `tests/e2e/helpers.js` must carry every key a route emits, and + `tests/e2e-stub-contract.test.js` checks that they do. ### File Modification Policy - **Only modify source code files**, documentation (/docs), and plans (/plans) @@ -97,13 +143,15 @@ Restore with Tests live in `tests/` and cover the logic in `src/lib/` (scoring and distance, the Upstash key adapter, game sessions, the region tree, the panorama indexes -and the leaderboards), the API routes, and the leaderboard migration. Run +and the leaderboards), the API routes, the district-assignment and env-loading +script helpers, and the e2e stubs' agreement with the routes. Run `npm test` after changing anything under `src/lib/` or `src/data/`. The suite runs against two backing stores, from one set of test files: - `npm test` uses `tests/fake-upstash-redis.js`, an in-memory stand-in mocked in - at the `@upstash/redis` boundary. No service, no Docker, well under a second. + at the `@upstash/redis` boundary, and PGlite for Postgres. No service, no + Docker; about 25 seconds for the whole suite, most of it PGlite start-up. This is the default. - `npm run test:integration` runs the same files against a real Redis. Two of them skip: one asserts a response shape only an older SDK produces, and one diff --git a/docs/features.md b/docs/features.md index a5ad506..2e1527b 100644 --- a/docs/features.md +++ b/docs/features.md @@ -23,7 +23,10 @@ from `/images?bbox=` search, which returns HTTP 500 in exactly the dense districts the game wants to play. See the header of `src/lib/mapillary.js` - **Runtime cost is one lookup**: `fetchPanoramaById` resolves a chosen id in - ~230ms; a couple of alternates are tried in case an image was deleted upstream + ~230ms; a couple of alternates are tried in case an image was deleted + upstream, within an eight-second budget for the whole draw. A region with + nothing left to show is a 404 with the coverage message; Mapillary not + answering is a 502, never reported as missing coverage - **Country draws pick a province first**, uniformly, so Vietnam rounds are not 97% Ha Noi and Ho Chi Minh by panorama count - **No immediate repeats**: the last 50 panoramas a player was shown are @@ -86,8 +89,9 @@ - **Single-use sessions**: the session is claimed with an atomic `DEL` before any score is written, so a replayed or concurrent submit scores exactly once. After the claim the score fan-out settles per level: the levels that wrote - are returned, `partial: true` marks a level that did not, and only a round - where no level wrote is reported as unsaved. The failure carries a `reason` (`session-expired`, `session-consumed`, + are returned in `gameResult.levels`, `gameResult.partial` marks a level that + did not, and only a round where no level wrote is reported as unsaved. The + response is `gameResult` alone. The failure carries a `reason` (`session-expired`, `session-consumed`, `invalid-guess`, `invalid-username`, `invalid-request`) and the result dialog words each one differently, so an expired round is not reported as a failed write diff --git a/docs/game-flow.md b/docs/game-flow.md index b2db000..ab83372 100644 --- a/docs/game-flow.md +++ b/docs/game-flow.md @@ -139,5 +139,5 @@ points added at every level are the same number. ### 10. Continue or Exit - Option to start next round with new session and location in the same region - Option to return to the region picker and choose somewhere else -- Option to view full leaderboard with pagination +- Option to view the leaderboards (top 200 per board) - Redis session cleanup ensures fresh start for each round \ No newline at end of file diff --git a/docs/project-structure.md b/docs/project-structure.md index 8362a5c..62199b1 100644 --- a/docs/project-structure.md +++ b/docs/project-structure.md @@ -63,7 +63,10 @@ Next.js 16 App Router structure: #### React Components (`src/app/components/`) - `AppBackground.js` - The key art (`public/bg.png`) on one fixed layer under every page, served through `next/image` -- `GameClient.js` - Main game client component +- `GameClient.js` - The round lifecycle: fetch, epochs, prefetch, submit, + daily replay. Layout lives in the components it renders +- `GameHeader.js` - The game screen's app bar, presentational +- `PlaceName.js` - A Vietnamese place name marked `lang="vi"` - `LeafletMap.js` - Interactive map for guess placement - `PanoramaViewer.js` - 360 degree street view display; owns the Mapillary attribution and a `topBarSlot` for host chrome sharing that row @@ -100,7 +103,6 @@ the shadcn CLI when a screen needs them, rather than keeping unused ones around. - `label.jsx` - Form labels - `select.jsx` - Grouped select (region picker) - `skeleton.jsx` - Loading skeletons -- `tabs.jsx` - Tab navigation ### Generated Data (`src/data/`) Both directories are build output. Do not hand-edit; see *Rebuilding the @@ -150,6 +152,13 @@ Neon Postgres, which is what the app queries at runtime. - `region-locate.js` - **Server-side only.** Which region a map point falls in, from the generated boundaries; feeds the result dialog's region-hit line - `debug-access.js` - The production gate on `/api/debug/*` +- `errors.js` - `DryPoolError` and `UpstreamError`, the two ways a draw fails +- `storage.js` - The one place localStorage is touched: guarded read, write, + and change notification across this tab and others +- `use-stored-value.js` - **Client-side only.** Renders a stored value via + useSyncExternalStore +- `first-round-hint.js` - Whether the how-to-play hint has been seen +- `geo-search.js` - Client-safe region and street search for the guess map - `share.js` - Client-safe share text for a round and the share-sheet call - `upstash.js` - Upstash Redis REST client adapter with multi-tenant key prefix - `theme.js`, `use-count-up.js` - Theme persistence and a count-up hook @@ -170,6 +179,10 @@ Each carries a header comment with its flags and its cost. (`npm run leaderboard:export`; run weekly by a GitHub Actions workflow) - `lib/assign-districts.mjs` - District assignment shared by the two pano scripts - `lib/pano-schema.mjs` - Panorama table DDL shared by the seed and the tests +- `lib/env.mjs` - The `.env` parser and loader every script shares +- `lib/region-config.mjs` - The hand-edited region configuration the boundary + builder reads (input data, distinct from the generated tree) +- `lib/barrel.mjs`, `lib/paths.mjs` - Barrel writer and output paths ## Tests (`tests/`) Vitest, mostly one file per `src/lib/` module, plus a route test for diff --git a/docs/tech-stack.md b/docs/tech-stack.md index 6572fa6..88283f7 100644 --- a/docs/tech-stack.md +++ b/docs/tech-stack.md @@ -58,7 +58,7 @@ ## UI Components & Styling - **shadcn/ui**: Complete component library with "new-york" style -- **Radix UI**: Headless component primitives -- dialog, label, slot, tabs, plus +- **Radix UI**: Headless component primitives -- dialog, label, slot, plus accordion (province expansion) and select (region picker) - **Lucide React**: Icon library - **class-variance-authority**: Component variant management @@ -66,7 +66,7 @@ ## Testing - **Vitest**: Test runner for the logic in `src/lib/`, the API routes, the - generated region data, and the leaderboard migration + generated region data, and the pipeline script helpers - **In-memory Upstash fake**: Default Redis backing store, no service required - **PGlite**: In-process Postgres (WASM) mocked in at the `@neondatabase/serverless` boundary, so the panorama queries run against real diff --git a/package-lock.json b/package-lock.json index 961b201..0a4b931 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15,7 +15,6 @@ "@radix-ui/react-label": "^2.1.11", "@radix-ui/react-select": "^2.3.7", "@radix-ui/react-slot": "^1.3.0", - "@radix-ui/react-tabs": "^1.1.17", "@turf/turf": "^7.4.0", "@upstash/redis": "^1.38.3", "@vercel/analytics": "^1.5.0", @@ -1935,39 +1934,6 @@ } } }, - "node_modules/@radix-ui/react-roving-focus": { - "version": "1.1.19", - "resolved": "https://registry.npmjs.org/@radix-ui/react-roving-focus/-/react-roving-focus-1.1.19.tgz", - "integrity": "sha512-V9jI6hDjT7l3jsCQD9bLNvDLM3tH/gdbOTp7Tefp3hbbgCGQoK7tUvrWiRlcoBHIZ809ElXwNQwVo0B98LuTXQ==", - "license": "MIT", - "dependencies": { - "@radix-ui/primitive": "1.1.7", - "@radix-ui/react-collection": "1.1.15", - "@radix-ui/react-compose-refs": "1.1.5", - "@radix-ui/react-context": "1.2.2", - "@radix-ui/react-direction": "1.1.4", - "@radix-ui/react-id": "1.1.4", - "@radix-ui/react-primitive": "2.1.10", - "@radix-ui/react-use-callback-ref": "1.1.4", - "@radix-ui/react-use-controllable-state": "1.2.6", - "@radix-ui/react-use-is-hydrated": "0.1.3", - "@radix-ui/react-use-layout-effect": "1.1.4" - }, - "peerDependencies": { - "@types/react": "*", - "@types/react-dom": "*", - "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", - "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" - }, - "peerDependenciesMeta": { - "@types/react": { - "optional": true - }, - "@types/react-dom": { - "optional": true - } - } - }, "node_modules/@radix-ui/react-select": { "version": "2.3.7", "resolved": "https://registry.npmjs.org/@radix-ui/react-select/-/react-select-2.3.7.tgz", @@ -2030,36 +1996,6 @@ } } }, - "node_modules/@radix-ui/react-tabs": { - "version": "1.1.21", - "resolved": "https://registry.npmjs.org/@radix-ui/react-tabs/-/react-tabs-1.1.21.tgz", - "integrity": "sha512-UKxJlZid7FVtsk/WTxj4i4uSEgj2Au+KBbS7SQyTlzMhhn+86Cz3tISZdTa87bfEfcuvZezf2ZsxD4xuEKtkog==", - "license": "MIT", - "dependencies": { - "@radix-ui/primitive": "1.1.7", - "@radix-ui/react-context": "1.2.2", - "@radix-ui/react-direction": "1.1.4", - "@radix-ui/react-id": "1.1.4", - "@radix-ui/react-presence": "1.1.10", - "@radix-ui/react-primitive": "2.1.10", - "@radix-ui/react-roving-focus": "1.1.19", - "@radix-ui/react-use-controllable-state": "1.2.6" - }, - "peerDependencies": { - "@types/react": "*", - "@types/react-dom": "*", - "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", - "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" - }, - "peerDependenciesMeta": { - "@types/react": { - "optional": true - }, - "@types/react-dom": { - "optional": true - } - } - }, "node_modules/@radix-ui/react-use-callback-ref": { "version": "1.1.4", "resolved": "https://registry.npmjs.org/@radix-ui/react-use-callback-ref/-/react-use-callback-ref-1.1.4.tgz", @@ -2113,21 +2049,6 @@ } } }, - "node_modules/@radix-ui/react-use-is-hydrated": { - "version": "0.1.3", - "resolved": "https://registry.npmjs.org/@radix-ui/react-use-is-hydrated/-/react-use-is-hydrated-0.1.3.tgz", - "integrity": "sha512-umO/aJ+82CpOnhDZUTbILCQf7kU/g0iv+oGs/Q8jw7IkhWBzaEP4sA268PhFAJTFetbwp3ICc6ktpI4TqtxcIw==", - "license": "MIT", - "peerDependencies": { - "@types/react": "*", - "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" - }, - "peerDependenciesMeta": { - "@types/react": { - "optional": true - } - } - }, "node_modules/@radix-ui/react-use-layout-effect": { "version": "1.1.4", "resolved": "https://registry.npmjs.org/@radix-ui/react-use-layout-effect/-/react-use-layout-effect-1.1.4.tgz", diff --git a/package.json b/package.json index 8a4d73a..086e227 100644 --- a/package.json +++ b/package.json @@ -32,7 +32,6 @@ "@radix-ui/react-label": "^2.1.11", "@radix-ui/react-select": "^2.3.7", "@radix-ui/react-slot": "^1.3.0", - "@radix-ui/react-tabs": "^1.1.17", "@turf/turf": "^7.4.0", "@upstash/redis": "^1.38.3", "@vercel/analytics": "^1.5.0", diff --git a/plans/reports/synthesis-260921-1547-whole-project-review.md b/plans/reports/synthesis-260921-1547-whole-project-review.md index 1dc44ac..7c681f2 100644 --- a/plans/reports/synthesis-260921-1547-whole-project-review.md +++ b/plans/reports/synthesis-260921-1547-whole-project-review.md @@ -108,3 +108,49 @@ modules; `development.md` says tests run "well under a second" (24.5s measured); `game-flow.md` describes leaderboard pagination that does not exist; `tech-stack.md` two stale lines. All listed with line numbers in the architecture report. + +## Resolution (same day) + +Applied on `dev` in five commits, one per area, all gates green throughout +(401 tests, lint 0 errors and 0 warnings, production compile). + +- **Bugs 1–8**: `partial` reaches the dialog; new-game answers 404 for a dry + pool and 502 for an upstream failure, its error handler tolerates a + non-Error, and it no longer logs the answer's district; the theme toggle + pair reads one store; every localStorage access goes through a guarded + module; the map centre is derived; the e2e stubs carry `hit`, `partial`, + `guessedRegion` and a `/api/daily` stub, and `tests/e2e-stub-contract.test.js` + asserts each stub is a superset of the real route; the scripts share one + `.env` loader with the unquoting. +- **Dead surface**: aliases, legacy ranks, duplicated envelope, `submitScore`, + unread leaderboard fields, the new-game debug POST, `zScore`, `isDay`, + `indexedProvinces`, `@radix-ui/react-tabs` — all gone. `leaderboard.test.js` + awards points through `submitRoundScore`. +- **Item 2** (envelope): `/api/guess` returns `gameResult` alone; + `/api/leaderboard` returns the rows alone. +- **Item 3** (error seam): `src/lib/errors.js`; eight string-match sites + replaced; an eight-second draw budget and a fifteen-second client timeout. +- **Item 4** (storage): `src/lib/storage.js` and `useStoredValue`; 20 lint + warnings fixed with useSyncExternalStore, useEffectEvent, render-time + adjustment and derived values; the one data ref carries its reason; both + react-hooks rules are now errors. +- **Item 5** (GameClient): GameHeader extracted (713 → 644 lines); the daily + replay derives from the stored record rather than six setState calls. The + `use-round` hook is deferred until a multi-round feature needs it. +- **Item 6** (scripts): `scripts/lib/env.mjs`, `scripts/lib/region-config.mjs`, + `data:refresh`. `drawFromProvinces` not extracted: `pickPanoBySeed` keeps its + own walk because its skip-empty-province rule differs from the random draw. +- **Item 7**: 28 tests for `assign-districts.mjs`, 14 for the env loader. +- **Item 8**: PlaceName with `lang="vi"` at the pure-name sites, the expanded + guess map is a dialog, MapSearchBox loads with the map, debug hub is a + server component, header buttons use the default touch size. Not done: + `cardRowVariants`, `safe-x` utility, the reveal-sequence class — cosmetic + and each touches several files for no behaviour change. +- **Item 9**: conventions codified in `docs/development.md`; the parameter + rule amended in `CLAUDE.md`; doc drift fixed. + +Decisions taken (defaults, override freely): upstream outage is a 502, not a +200; the lint warnings are fixed rather than the comment updated; +`submitScore` is deleted (no backfill procedure references it); the +`use-round` hook waits for a multi-round feature; the home-page username +interception stays. diff --git a/tests/regions.test.js b/tests/regions.test.js index 15d17be..1ee0d20 100644 --- a/tests/regions.test.js +++ b/tests/regions.test.js @@ -282,6 +282,10 @@ describe('client safety', () => { 'pano-db', 'pano-history', 'data/boundaries', + // The daily pick passes the day's answer through; a client bundle that + // reached it would hold every player's round. + 'lib/daily.js', + 'region-locate', ]; const collectSourceFiles = (dir, out) => {