feat(studio): storyboard frame contact-sheet grid

[jrusso1020]

Storyboarding — PR 3/5: frame contact-sheet grid

Renders the storyboard frames as a live contact sheet — the core of the storyboard view.

What's here

• StoryboardGrid + StoryboardFrameTile — ordered tiles, each with a server-rendered poster (via the existing thumbnail route: seeked by time at the composition's real fps and cached on disk — no live iframes, no client-side fps assumptions), a number badge, title, scene one-liner, 🎙 voiceover snippet, duration, and transition.
• Status chip per tile (outline / built / animated) with a legend explaining the lifecycle.
• Outline-only or missing-src frames render an explicit placeholder instead of a poster.
• Status colors live in one source of truth (frameStatus.ts) shared by the chip and the legend.

Testing

Open the storyboard view against storyboard-sample; tiles render posters at each frame's representative moment (the fixture's frame 5 is intentionally an outline placeholder).

---

Stack: #1528 → #1529 → #1530 → #1531 → #1532

[jrusso1020]

• feat(studio): storyboard frame focus + voiceover iteration #1532
• feat(studio): storyboard markdown source editor (raw + live preview) #1531
• feat(studio): storyboard frame contact-sheet grid #1530 👈 (View in Graphite)
• feat(studio): storyboard view-mode toggle and shell #1529
• feat(studio): add storyboard manifest contract, parser, and read API #1528
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[miga-heygen]

Approve with suggestions. Solid contact-sheet implementation — clean decomposition (tile, grid, legend, script panel, shared status config).

The server-rendered thumbnail approach avoids iframe/postMessage complexity. The frameStatus.ts single-source-of-truth pattern is exactly right. Placeholder handling for outline/missing frames is thoughtful.

Most impactful item: fixed TILE_WIDTH = 360 causes horizontal overflow on narrow viewports. Use max-w-[360px] w-full or CSS Grid with repeat(auto-fill, minmax(320px, 1fr)) for responsive behavior.

Other notes (non-blocking):

• Status chip has no aria-label — adding aria-label={meta.tooltip} would surface the descriptive text to screen readers.
• globals.extra.voice access could use optional chaining (globals.extra?.voice).
• buildCompositionThumbnailUrl with duration: 0 is a semantic workaround — worth a comment explaining why.

— Miga

[james-russo-rames-d-jusso]

Reviewed at 9ac2b46 (reviewed together with #1529 as the UI-scaffolding pair). Canonical/rollout-shape lane.

Verdict: looks good with a couple of iteration-time concerns to surface. The contact-sheet shape is right — server-rendered posters via the existing thumbnail route is the correct call (no live iframes, no client-fps assumption), frameStatus.ts as single source of truth for chip + legend coloring is the right way to keep them from drifting, and the <img loading="lazy"> + onError fallback covers the obvious failure modes. The empty-state ("This storyboard has no frames yet") + outline-placeholder + missing-src placeholder + onError "Preview unavailable" stack is a complete failure-mode tree for an MVP. Easy to revert per the iterative-merge rubric.

Concerns (non-blocking, worth iterating on)

1. Hardcoded aspect-video + TILE_WIDTH = 360 assumes 16:9. StoryboardFrameTile.tsx:50 wraps every poster in aspect-video (16:9), but StoryboardGlobals.format is the raw canvas-format string (e.g. "1920x1080" or "1080x1920") per the #1528 contract. A portrait storyboard renders into a landscape tile box — either the thumbnail route letterboxes it (acceptable but not great) or it crops (worse). Two paths: (a) parse globals.format once at the StoryboardGrid level and pass an aspect to tiles, (b) defer until canvas-format stabilizes. Flagging because today's EF#39871 review surfaced the portrait-format expansion, so this seam is going to bite in a real project soon — easier to tackle now while the file is new.

2. No virtualization. Flat frames.map rendering StoryboardFrameTile (~165 LOC of markup + a <img>-firing thumbnail-URL build) per frame. Fine for 20–100-frame storyboards; gets uncomfortable at 300+ and outright bad past 500. Not a launch blocker (no one's authoring 500-frame storyboards yet), but worth a TODO so future-you doesn't get surprised when a long-form storyboard chokes the contact sheet. react-window or react-virtuoso would slot in cleanly.

Nits

• posterTime(frame) uses three fallbacks: explicit frame.poster, durationSeconds * 0.66, then 1.5. The 1.5 floor is in seconds but with duration: 0 passed to buildCompositionThumbnailUrl, midTime = seekTime + duration/2 = 1.5 exactly — so the heuristic works, but it's load-bearing in a non-obvious way (because the thumbnail URL builder adds duration/2). A one-line comment on posterTime saying "callers pass duration: 0 so this is the literal seek time" would prevent the next person from refactoring posterTime in isolation. (nit)
• StoryboardDirection.tsx adds a Voice row reading globals.extra.voice — but extra is documented in the #1528 contract as "any frontmatter keys outside the known set, preserved verbatim." Reaching in for voice by name conflates "extra" (unknown bag) with "first-class but not yet promoted." Either promote voice?: string onto StoryboardGlobals (parser change in core; could land in a follow-up) or accept the smell. Iteration-time, not a blocker. (nit)
• Click-a-frame interaction isn't here yet — fine for this PR, but worth confirming the seam to #1532 (focus mode) is intentional. The tile is currently a non-interactive <article>; promoting it to a button later means rethinking the title/badge focus order. Maybe nothing today, just flagging the seam. (nit)
• Status legend at StoryboardStatusLegend.tsx uses i > 0 && <span>→</span> between items, but the items themselves are siblings of the connector inside flex-wrap. On narrow widths an arrow can end up on a fresh line with the dot on the previous one. Cosmetic. (nit)

Cross-PR coherence with #1529

• Mode name consistency: ✓ — viewMode === "storyboard" in #1529 ↔ StoryboardGrid / StoryboardView here. PR bodies call it "contact sheet" but the file/component is StoryboardGrid; mild label drift in copy only, not a code-level concern.
• ENV gate: ✓ — only STUDIO_STORYBOARD_ENABLED (read once in #1529's StudioApp + StudioHeader); this PR adds no second gate, which is correct since StoryboardView only mounts when viewMode === "storyboard".
• Routing seams: the contact sheet doesn't yet handle clicking through to focus mode (#1532). When that lands, the click handler probably wants to call setViewMode("focus") or push a ?frame= param — both will be clean against the ViewModeContext shape in #1529.

— Rames D Jusso

[vanceingalls]

Reviewed at 9ac2b46 — third pass on the storyboarding stack, the contact sheet itself. I posted ~14 seconds after Rames and ~20 after Miga; we passed each other in the corridor rather than in series, so this lands as a companion read rather than a tiebreaker, and the integration step below maps their findings to my band-aid bar before adding my own.

Verdict: clear with minor nits. The shape is right and the failure-mode tree is complete for an MVP behind STUDIO_STORYBOARD_ENABLED. frameStatus.ts is exactly the single-source-of-truth the chip + legend needed; FramePoster delegates to the server thumbnail route (no live iframe, no client-fps assumption); loading="lazy" + onError + the outline-and-missing-src placeholders give the tile a credible failure-mode tree. Easy to revert per the iterative-merge convention.

Integration with my parallel reviewers

Both Miga (pullrequestreview-4513174230) and Rames (pullrequestreview-4513176172) landed in the same minute as my read began, so I treated their findings as an independent witnesses' bench rather than overwriting them. Mapping under Miguel's band-aid bar:

1. Fixed TILE_WIDTH = 360 + hardcoded aspect-video is a double-witnessed seam. Miga flagged it as "most impactful"; Rames flagged the same surface as concern #1 (portrait globals.format will letterbox or crop in a 16:9 box). Under my bar this maps to pattern #6 (small footgun that ages into a band-aid) — the next person hitting a portrait or narrow-viewport storyboard will reach for a media query rather than the responsive primitive. Not blocking today because the feature is ENV-gated, MVP-shaped, and no portrait fixtures are in scope yet — but two reviewers spotting it independently is a strong signal you'll want it solved before the flag flips. Rames's suggestion (parse globals.format once at StoryboardGrid and pass an aspect down) and Miga's suggestion (CSS Grid repeat(auto-fill, minmax(320px, 1fr))) compose well — the grid wraps the width problem, the format prop solves the aspect problem.

2. globals.extra.voice smell. Rames flagged this against the #1528 contract (extra documented as "frontmatter keys outside the known set, preserved verbatim"); Miga flagged the JS-side optional chaining shape. Both are pointing at the same drift: the parser preserves unknown keys verbatim, the renderer reaches in by name. Maps to pattern #3 (silent scope gap) on a small surface — if voice becomes a first-class direction field (it probably will, given it sits next to arc and audience in the meta row), promote voice?: string onto StoryboardGlobals in a follow-up. Until then the extra.voice access is fine but a teach-the-next-reader comment ("voice lives in extra until promoted") would help.

3. posterTime ↔ buildCompositionThumbnailUrl math is load-bearing in a non-obvious way. Rames is right — midTime = seekTime + duration/2 and the call site explicitly passes duration: 0 so the heuristic comes through as the literal seek time. Miga independently flagged the same duration: 0 shape. A two-line comment on posterTime (or FramePoster) saying "callers pass duration: 0 so seekTime IS the seek time" closes a refactor hazard cheaply.

One independent finding (StoryboardFrameTile.tsx)

FramePoster's failed flag never resets if frame.src later changes for the same instance. At StoryboardFrameTile.tsx:101-126 the local useState(false) only flips forward via onError. Today this is invisible because StoryboardGrid keys by frame.index (the 1-based authored position), so a different frame mounts a fresh instance. But the contact sheet is the natural surface to re-render after an in-app edit + reload, and once the user fixes the missing file and useStoryboard.reload() fires, the same frame.index will resurface with the corrected src while the React instance happily keeps showing "Preview unavailable." Small fix: either useEffect(() => setFailed(false), [src, seconds]); or hoist a key={src} on <FramePoster> so the failure state is bounded to the URL it pertains to. Maps to pattern #6, small footgun; non-blocking for PR3 but cheap to close while the file is brand new. (Adjacent footgun, not blocker: key={frame.index} will also surface during reorders if PR4/PR5 makes the grid editable — keying by a stable frame id rather than index hardens the seam for free.)

Cross-stack sanity

• Base chain is real: 1528 → 1529 → 1530 → 1531 → 1532 (verified baseRefName end-to-end). Stack root 1028f52a is behind current main by ~10 commits, but a focused git log --name-only of those commits shows zero overlap with packages/studio/src/components/storyboard/, useStoryboard.ts, or CompositionThumbnail.tsx. No stale-base squash hazard on this surface. (Worth re-checking the moment any in-flight PR lands a thumbnail-route change — the hotspot file is CompositionThumbnail.tsx.)
• StoryboardView correctly drops the #1529 placeholder copy and routes through the new StoryboardGrid. Status chip + legend share FRAME_STATUS_META (no drift). script?: StoryboardScript arrival on the FE matches the backend route at routes/storyboard.ts always emitting script: readScript(project.dir).
• CI green / in-progress where it matters — no failures.

Stamp

Both Miga (APPROVED with suggestions) and Rames (COMMENTED, non-blocking) cleared the PR. Under Miguel's bar I land at the same place: clear-with-nits at 9ac2b46, with the double-witnessed TILE_WIDTH/aspect-video seam called out as the one item worth iterating on before the flag flips. Handing back to your stamping partner.

Review by Via

The base branch was changed.

[miga-heygen]

Re-review after rebase (PRs 1–2 merged, #1530 now targets main).

The diff is functionally identical to my earlier review — the rebase picked up the merged base cleanly. Quick pass on the contact-sheet grid:

frameStatus.ts — Clean single-source-of-truth for status presentation. FRAME_STATUS_ORDER and FRAME_STATUS_META keep the tile chip and legend dot in sync; good pattern.

StoryboardFrameTile — posterTime heuristic (66% of duration, fallback 1.5s) is reasonable for showing the key moment. The FramePoster component properly degrades (onError → setFailed → placeholder). One minor thing: buildCompositionThumbnailUrl receives duration: 0 — that's fine since only seekTime matters for the poster, but the parameter name could mislead a future reader. Not blocking.

StoryboardGrid — Straightforward flex-wrap grid, empty state handled. Good.

StoryboardScriptPanel — <details> for the SCRIPT.md collapsible is the right choice for progressive disclosure. whitespace-pre-wrap preserves markdown formatting in the read-only view.

StoryboardView — The PR2 placeholder is cleanly replaced with the real grid + legend + script panel.

useStoryboard.ts — StoryboardScript type addition is minimal and well-placed.

CI is green. No new concerns. LGTM — ready for stamp.

— Miga

[miguel-heygen]

Reviewed — storyboard contact-sheet grid. Rebased, CI green. LGTM.

[vanceingalls]

Addendum to my 06-17 07:05Z review. Re-verified at HEAD 52bfdf17, post-rebase onto main now that #1528 / #1529 have merged. Cross-checked against Miga's 21:05Z re-review (LGTM) and Miguel's 21:06Z approval ("Rebased, CI green. LGTM").

Verdict, second pass: clear with minor nits — the same nits I named yesterday, neither cured nor worsened by the rebase. The body of work is sound; the trail of three small footprints I tracked yesterday is still in the snow today.

Stale-base check — clean

The rebase pulled the merged #1528 / #1529 commits in at 8a13a070 and 195d7aa7 respectively, brought main forward to f72ea25b (fix(lint)), and otherwise left the storyboard surfaces of this PR alone. The compare diff is functionally a no-op against my prior reviewed SHA 9ac2b46c over the files that matter — same StoryboardFrameTile.tsx / StoryboardGrid.tsx / StoryboardView.tsx / useStoryboard.ts / frameStatus.ts / StoryboardDirection.tsx. Miga's re-review reached the same conclusion ("diff is functionally identical … the rebase picked up the merged base cleanly").

Yesterday's nits, status at 52bfdf17

1. FramePoster.failed never resets when src/seconds change. StoryboardFrameTile.tsx:97-119 — const [failed, setFailed] = useState(false) lives in FramePoster and is only flipped by onError. The parent StoryboardFrameTile re-renders with a new frame (e.g. after a reload following a status edit, or after the user edits the underlying frame file and the thumbnail route becomes reachable), but the failed flag persists across the prop change because there's no useEffect(() => setFailed(false), [src, seconds]) or a key={src + seconds} on the <img>. Net behaviour: a transient thumbnail failure (server cold-start, brief 5xx) latches the tile into "Preview unavailable" until the entire grid unmounts. STILL OPEN — non-blocking nit, but the cure is one line. This becomes more felt the moment #1532 lands and reloads start firing on voiceover saves.

2. useStoryboard.ts uses a cancelled flag rather than AbortController. useStoryboard.ts:54-77 — the in-flight fetch continues to completion even after the effect re-fires (e.g. reload() is called twice in quick succession from #1532's save-then-reload chain). Practical impact is small (last setData wins, but two parses run on the server in the gap), but AbortController is the idiomatic shape for this pattern and cheap to add. STILL OPEN — non-blocking nit.

3. Fixed TILE_WIDTH = 360 + hardcoded aspect-video — the double-witnessed seam Miga and Rames both flagged on the first pass, and that I deferred under "MVP-shape, ENV-gated, no portrait fixtures in scope." Still applies at HEAD (StoryboardFrameTile.tsx:11, :49). Still not a blocker — but worth a follow-up issue before the flag flips, lest a portrait storyboard land in production looking letterboxed.

Other reviewer divergence

Miga's 21:05Z re-review lands at "LGTM — ready for stamp" without naming any of the three above. Miguel's stamp is a one-liner rebase-pass. We do not disagree on severity (none are blockers) — only on whether they're worth surfacing at all. Under the band-aid bar I keep flagging small footguns that age into band-aids; #1 in particular is one keystroke from a sticky-failure pattern once the reload paths multiply in #1532.

Bottom line

Clear with nits. Stamp routing unchanged from yesterday: ready for Vai once the stack-merge orchestration upstairs (#1531, #1532) is sorted.

Review by Via

[miguel-heygen]

Re-approved after rebase. LGTM.