feat(gallery): The Curator's Walk — fullscreen museum walk with turn-to-face - #82
Merged
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… dead test branch, named tuning constants
Root cause: three.js draws the entire opaque render list before the transparent list, and renderOrder only sorts within each list. The inspected case's poster/frame are opaque, so the transparent dimQuad (renderOrder 90, depthTest: false, opacity 0.78) painted over them every frame despite their renderOrder 95 — the hero poster read at ~22% brightness. Fix: while inspected, the poster material flips transparent and the frame strips swap to a transparent twin of the shared frame material, so the case joins the transparent list where renderOrder 95 actually beats dimQuad (90) and backdrop (91). Regression guard: scripts/gallery-smoke (vite + puppeteer-core against system Chrome) inspects a procedural-fallback fixture poster and asserts max center-region luminance > 0.35. Before: 0.173 FAIL. After: 0.778 PASS. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… panel Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ts the Walk Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e Shelf patterns Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
📝 WalkthroughWalkthroughThe gallery now uses single-file stations with walk-turn camera behavior, fullscreen portal presentation, improved inspect rendering, and a Puppeteer smoke harness covering navigation, luminance, Escape handling, screenshots, and console errors. ChangesGallery walk-turn and engine integration
Fullscreen gallery presentation
Smoke validation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant RankingAppPage
participant GalleryOverlay
participant GalleryView
participant GalleryEngine
participant SmokeRunner
RankingAppPage->>GalleryOverlay: render fullscreen gallery
GalleryOverlay->>GalleryView: mount canvas and exit controls
GalleryView->>GalleryEngine: initialize and set items
SmokeRunner->>GalleryView: drive keyboard and gallery actions
GalleryView->>GalleryEngine: walk, inspect, and render
GalleryEngine-->>SmokeRunner: mode, camera, geometry, and luminance data
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/gallery/galleryLayout.ts`:
- Around line 95-97: Update the wall-side calculation in the tier item mapping
to use the cumulative position, `flatIndex + i`, instead of the tier-local index
`i`, so alternation continues across tier boundaries. Add a regression test
covering consecutive tiers where the first tier has an odd number of items and
verify the next tier starts on the opposite wall.
In `@docs/plans/2026-07-28-curators-walk.md`:
- Around line 198-222: Update the plan’s smoke-harness references in both
affected sections to use the existing scripts/gallery-smoke/ directory,
specifically run.mjs and smoke.ts, instead of the nonexistent
scripts/gallerySmoke.mjs path. Adjust the commands, file list, and any related
harness wording consistently while preserving the documented smoke-test steps.
- Line 15: Add an H2 parent heading, such as “Implementation tasks,” between the
document’s H1 and the existing “### Task 0” through “### Task 6” headings in the
plan document, preserving all task content.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 95b66a3e-8ea8-4cee-8a7e-fc54bcc5fb34
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (17)
.gitignorecomponents/gallery/GalleryEngine.tscomponents/gallery/GalleryOverlay.tsxcomponents/gallery/GalleryView.tsxcomponents/gallery/__tests__/fixtures.tscomponents/gallery/__tests__/galleryLayout.test.tscomponents/gallery/__tests__/walkTurn.test.tscomponents/gallery/galleryLayout.tscomponents/gallery/walkTurn.tsdocs/plans/2026-07-28-curators-walk-design.mddocs/plans/2026-07-28-curators-walk.mdpackage.jsonpages/RankingAppPage.tsxscripts/gallery-smoke/run.mjsscripts/gallery-smoke/smoke.htmlscripts/gallery-smoke/smoke.tsscripts/gallery-smoke/vite.config.ts
Comment on lines
95
to
+97
| const roomSlots: CaseSlot[] = tierItems.map((item, i) => { | ||
| const side: 'left' | 'right' = i % 2 === 0 ? 'left' : 'right'; | ||
| const pair = Math.floor(i / 2); | ||
| const z = startZ - ROOM_LEAD - pair * CASE_SPACING; | ||
| const z = startZ - ROOM_LEAD - i * CASE_SPACING; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Continue wall alternation across tier boundaries.
i resets for every tier, so an odd-sized tier causes the next tier’s first case to repeat the previous wall. Derive parity from flatIndex + i and add a cross-tier regression test.
Proposed fix
const roomSlots: CaseSlot[] = tierItems.map((item, i) => {
- const side: 'left' | 'right' = i % 2 === 0 ? 'left' : 'right';
+ const slotIndex = flatIndex + i;
+ const side: 'left' | 'right' =
+ slotIndex % 2 === 0 ? 'left' : 'right';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const roomSlots: CaseSlot[] = tierItems.map((item, i) => { | |
| const side: 'left' | 'right' = i % 2 === 0 ? 'left' : 'right'; | |
| const pair = Math.floor(i / 2); | |
| const z = startZ - ROOM_LEAD - pair * CASE_SPACING; | |
| const z = startZ - ROOM_LEAD - i * CASE_SPACING; | |
| const roomSlots: CaseSlot[] = tierItems.map((item, i) => { | |
| const slotIndex = flatIndex + i; | |
| const side: 'left' | 'right' = | |
| slotIndex % 2 === 0 ? 'left' : 'right'; | |
| const z = startZ - ROOM_LEAD - i * CASE_SPACING; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/gallery/galleryLayout.ts` around lines 95 - 97, Update the
wall-side calculation in the tier item mapping to use the cumulative position,
`flatIndex + i`, instead of the tier-local index `i`, so alternation continues
across tier boundaries. Add a regression test covering consecutive tiers where
the first tier has an odd number of items and verify the next tier starts on the
opposite wall.
|
|
||
| --- | ||
|
|
||
| ### Task 0: Branch |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add an H2 parent for the task headings.
### Task 0 follows the document’s H1 directly, triggering markdownlint MD001. Add an H2 such as ## Implementation tasks before the Task 0–6 subsections.
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 15-15: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/plans/2026-07-28-curators-walk.md` at line 15, Add an H2 parent heading,
such as “Implementation tasks,” between the document’s H1 and the existing “###
Task 0” through “### Task 6” headings in the plan document, preserving all task
content.
Source: Linters/SAST tools
Comment on lines
+198
to
+222
| **Step 1: Reproduce headlessly.** Run the existing smoke flow (see PR #81 body; if no | ||
| script exists, create `scripts/gallerySmoke.mjs` from Task 5's harness) and capture a | ||
| screenshot ~800ms after `beginInspect`. Sample the center pixel of the inspected | ||
| poster region. | ||
|
|
||
| **Step 2: Root-cause.** Known suspects, in order: | ||
| 1. `dimQuad` renders in front of the inspected case (renderOrder/z fight) — check | ||
| `dimQuad.position.z` vs the case's final flight z and both `renderOrder`s. | ||
| 2. The flight target sits behind the backdrop plane's dim material. | ||
| 3. `renderer.toneMappingExposure` or a material `color.multiplyScalar` applied to ALL | ||
| cases including the inspected one during inspect (grep `exposure`, `multiplyScalar`). | ||
|
|
||
| **Step 3: Fix minimally.** The invariant: the inspected case must render *in front of* | ||
| the dim layer at full material brightness (`color 0xffffff`, `toneMapped` true, | ||
| no opacity dim). World dimming happens ONLY via `dimQuad` + backdrop material. | ||
|
|
||
| **Step 4: Guard.** Add to the smoke script an assertion: sampled center-pixel | ||
| luminance of the inspected poster > 0.35 (0–1 scale) for a known-bright fixture | ||
| poster. Expected: FAIL before fix, PASS after. | ||
|
|
||
| **Step 5: Commit** | ||
|
|
||
| ```bash | ||
| git add components/gallery/GalleryEngine.ts scripts/gallerySmoke.mjs | ||
| git commit -m "fix(gallery): inspected poster renders bright — dim only the world layer" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the documented smoke-harness path.
The plan still references scripts/gallerySmoke.mjs, but this PR’s harness lives under scripts/gallery-smoke/ with run.mjs and smoke.ts. The current commands and file list point readers to a nonexistent path.
Also applies to: 351-377
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/plans/2026-07-28-curators-walk.md` around lines 198 - 222, Update the
plan’s smoke-harness references in both affected sections to use the existing
scripts/gallery-smoke/ directory, specifically run.mjs and smoke.ts, instead of
the nonexistent scripts/gallerySmoke.mjs path. Adjust the commands, file list,
and any related harness wording consistently while preserving the documented
smoke-test steps.
BIBOYANG425
added a commit
that referenced
this pull request
Jul 29, 2026
…live-pass tuning (#83) * feat(gallery): human-scale posters on a square-on turn The owner walked PR #82 on prod and reported dollhouse-sized posters, oblique cases that never focus, and half-clipped frames. Two coupled root causes: 1. STOP_LEAD (2.4) was uncoupled from CASE_FACE_BIAS: the turned eye stood far down-corridor and viewed every case obliquely ("does not focus / half the poster"). STOP_LEAD is now derived — (WALL_X + DRIFT_X)·tan(bias) — so the eye stands exactly on the case's normal line and the 90° turn lands perpendicular, square-on. DRIFT_X moved to galleryLayout (re-exported from walkTurn) so the layout can derive the lead without importing the camera module. 2. Scale: CASE_H 0.93 → 2.8 (~1.6× an adult, the owner's ask), CASE_W 1:1.5, CASE_Y 1.7 (bottom hangs 0.3 above the floor), EYE_Y kept at human 1.42. Corridor rescaled coherently: WALL_X 2.9, DRIFT_X 1.45, CASE_SPACING 3.8 (≈1.35×CASE_H), room leads/tails/arch, wall height 5.4, fog/clip, glow/frame/label sizes, inspect distance and scale. A WALL_STANDOFF (0.35) stands the side walls behind the case so the now-wide poster's bias-yawed far edge no longer clips through the wall (the occluded-right-half bug). Bias bumped 0.22 → 0.30 for a readable three-quarter face while walking past; the derived stop guarantees the frontal moment. At a stop the poster now fills ~80% of frame height, dead-centered and frontal. Smoke harness: seed the converged camera pose and add a deterministic pump()/snapshot() so the throttled-rAF headless screenshots are faithful; phase 3 now asserts square-on (equal edge heights, centered) from projected poster corners. galleryLayout length-bounds test updated for the new spacing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(gallery): inspect backdrop reads as a dark museum wall The owner saw a bright TMDB backdrop still filling the inspect screen. The backdrop plane sat above the dim quad in render order and animated to 0.9 opacity, so the world-dim never touched it — a movie-still wash. Cap the backdrop material at 0.15 opacity (was 0.9) and darken its offscreen bake harder (brightness 0.28, blur 18px). The dimQuad's darkness now dominates and inspect reads as art on a dark museum wall, while the inspected poster (renderOrder 95) still draws bright over it — the Task-2 transparent-list luminance guard keeps passing (max 0.778). Verified with a deliberately bright injected backdrop: it stays a faint warm tint, not a wash. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rebuilds Gallery mode as The Curator's Walk: a fullscreen night-museum walk where the camera turns to face each poster at its walk stop, with a bright pull-forward inspect. Design doc:
docs/plans/2026-07-28-curators-walk-design.md; implementation plan:docs/plans/2026-07-28-curators-walk.md.What changed
CASE_SPACINGretuned; cases hang flatter (CASE_FACE_BIAS0.42 → 0.22) since the camera now does the facing.components/gallery/walkTurn.ts(unit-tested): dominant-station weighting drifts the camera toward the opposite wall and blends the gaze onto the case; the head turns before the feet plant.round(targetWalk)(the destination), never the current station. The old current-position snap fought in-flight travel: arrow-key nav stalled andtravelToTierlanded mid-corridor. Guarded by smoke phases 2 and 6 (phase 6 drives the realtravelToTier).renderOrderonly sorts within a list; the transparent dim-quad therefore painted over the opaque poster no matter its renderOrder. During inspect the case's meshes now join the transparent list (postertransparent = true; frames swap to a dedicated twin material, reverted on restore). Guarded by a luminance assertion (0.173 before → 0.778 after).GalleryOverlay(createPortal, body-scroll lock,inertbackground, focus capture/restore, Esc/✕ exit). Re-rank deliberately exits the Walk before opening the ceremony modal. Grid stays mounted underneath for instant exit/fallback.npm run smoke:gallery: 6 phases (hall → travel → turn → inspect → Esc-return → tier fast-travel) with per-phase screenshots, strict zero-console-error guard, luminance guard,SMOKE_CHROMEoverride. Local-only (not wired into CI).GalleryEngine.tsheader.Verification
npx vitest run: 770/770 (includes new walkTurn + layout tests, shared fixtures)npx tsc --noEmit: only the 8 pre-existing errors (untouched files)npm run smoke:gallery: 6/6 phases, zero console errorsnpm run build: gallery stays a lazy chunk (554.7 kB / 141.8 kB gzip); main bundle unchangedHonest gaps vs the design doc (follow-ups)
№ 7) yet; rank shows on the inspect placard only.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes