Skip to content

feat(studio): show the agent's storyboard in a Storyboard tab beside the preview - #7284

Merged
WaterrrForever merged 11 commits into
mainfrom
miao/studio-storyboard-tab
Nov 6, 2026
Merged

WaterrrForever merged 11 commits into
mainfrom
miao/studio-storyboard-tab

Conversation

@WaterrrForever

@WaterrrForever WaterrrForever commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator

What

Studio shows the agent's storyboard in a Storyboard tab beside the preview, the way HyperFrames Desktop does. The board's bar carries the desktop-app button, because commenting on frames, drawing on them and saying go happen in Desktop.

Why

A teammate ran /style-sampling on OSS and got its storyboard as a file:// page in a browser tab, separate from Studio. The board itself was correct: boards/storyboard.html in the recipe's format, which is the file Desktop's Storyboard tab reads. OSS had nowhere to show it, because the Studio storyboard view was removed in #4140 and the recipe has said "the board is a page the agent writes" since #4150 and #5599. This PR shows that page; it does not bring back the old view.

How

Finding the board. Same rule as Desktop's findDocFile: storyboard.html at the root, else in boards/, case-insensitive. With no board there is no strip at all.

Reading frames. Studio fetches the page and parses it with DOMParser, so the page's scripts never run. It reads the top-level class="frame" boxes by the same rules as Desktop's storyboardOutline.mjs: number and title, time ("a to b"), the act heading before the frame, the first img as the still, and the words. Stills resolve against the page's own folder through /api/projects/:id/preview/*. A src outside the project, on another host or a data: URL is dropped.

Board and frame views.

  • The board is grouped by act.
  • Clicking a card opens one frame large. ←/→ step, Esc goes back to the board, and "Show in preview" returns to the film at that frame's start.
  • A page with no .frame boxes, or one that can't be read, shows as written in an <iframe sandbox=""> with the reason in the bar. "Show page as written" lets the person see it that way on request.

Live updates. From /api/events, as in Desktop:

  • A board that appears while Studio is open brings its tab to the front.
  • A write to the page, anything beside it in boards/, or STORYBOARD.md reloads it in place.
  • A rewrite seen from the Preview tab marks Storyboard with a dot.
  • The film stays mounted under the board, and opening the board pauses it.

Desktop button. The board's bar renders OpenInDesktopButton placement="storyboard". Its funnel events (desktop_cta_eligibility / impression / guide / outcome, and toolbar_action) now carry placement: "header" | "storyboard", so the two buttons can be told apart.

Approve in Studio. A Studio user without the desktop app is never stuck at the board. Studio reaches the person's own agent through their chat, so the bar's Approve copies "The storyboard is approved. Build the video from it." for them to paste there. The toast says so, or spells the words out when the clipboard refuses. The storyboard recipe tells the agent those words are the person's go. Commenting and drawing on frames stay in the desktop app.

Hand-over onto the board.

  • The storyboard's button also posts doc: "storyboard". The preview server passes on only that value, and openInDesktop writes it into .hyperframes/desktop-cta.json beside the page id.
  • Desktop reads it in heygen-com/hyperframes-internal#9010. The project opens on its Storyboard tab, and the board waits for its go there, so Build shows as it does for a board Desktop's own agent wrote.
  • The guide card opened from the storyboard says "Comment on this storyboard in the desktop app" and "Comment on a frame, draw on it, then press Build". Its last step says the app opens on this storyboard. The header's card is unchanged.

Skills. The recipe (hyperframes/references/storyboard.md, where it names the board) and style-sampling's stop 1 now say the board is handed over as the Storyboard tab beside the preview: with no preview running, start one and give its link, never the file's path. Skills manifest regenerated.

File changes. The external-change coordinator stays the only subscriber to the transport: HMR in dev, SSE when built. It announces each decoded change through hooks/fileChangeFanout.ts, and the board listens there, so no second /api/events stream is opened.

Not in this PR (Desktop-only today, or for later):

  • Commenting, drawing and Build on frames.
  • The page's own interactive layout.

Before

hyperframes@0.9.16 preview on a project that has boards/storyboard.html: Studio doesn't show it anywhere.

Before

After

The board appears while Studio is open and its tab comes to the front:

After: board

One frame, stepped with ←/→:

After: one frame

The desktop-app card, opened from the board's bar:

After: desktop card

Review round 1 (0f815f44d)

  • The hand-off intent is written before dispatch. openInDesktop writes desktop-cta.json before the folder is sent. A header press or a plain open drops an earlier storyboard ask, and a failed open puts back the earlier record. Tests cover the record seen at dispatch, a later header or plain open, and the undo.
  • The film stays steady under the board. PreviewStage keeps the film at one tree position with or without a board. The automatic switch to the board goes through the same pause as a click, and any play that starts while the board is up is paused. PreviewStage.dom.test.tsx covers first appearance, removal and reappearance, and a play started while the board is up.
  • Stills outside the board folder now reload. The board reloads when a still it draws changes, including a root board's stills/ and a boards/ page's ../assets.
  • Each new test fails on the old code. Against the old PreviewStage all 3 fail, and against dispatch-before-write 2 fail.

Size budget (0952f4453): the @hyperframes/cli packed budget goes from 7,902,966 to 7,982,002 bytes. The CLI tarball carries Studio's build, and that build now holds the Storyboard tab. Merged with main, the tarball measured 7 bytes past the 1% tolerance.

Review round 2 (91845efc1): while the board covers the film, the stage takes the player's own playLocked hold. It only does so when no other owner holds it, and it releases the hold on Preview. J, Space and playback requests are refused before they start. Pausing after the fact let J's reverse loop schedule a frame first. PreviewStage.player.dom.test.tsx mounts the real useTimelinePlayer, store and keyboard: J under the board leaves the adapter at 10 s with no frame queued, and J works again on Preview. Against the old stage it fails: the film moved to 11.43 s.

Test plan

  • Unit tests: storyboardBoard.test.ts, 14 tests.
    • Board lookup: root wins, case-insensitive, no other folders.
    • Which writes reload the board, still paths (escapes and other hosts refused), reading the recipe's page, nested frames, a page with no frames.
    • Act grouping, frame start, view mode.
  • OpenInDesktopButton.dom.test.tsx: expectations include placement: "header". New tests check that every funnel event from the storyboard's button says storyboard, that it posts doc: "storyboard", and that its card asks to comment on the board, not edit the video. 23 pass.
  • ApproveBoardButton.dom.test.tsx: Approve copies the go and says to paste it to the agent; a refused clipboard spells the words out. 2 pass.
  • desktopApp.test.ts and desktopRoutes.test.ts: the record carries doc only for a storyboard press, and the route passes on only "storyboard". 44 pass.
  • fileChangeFanout.test.ts added. useExternalFileChangeCoordinator, externalFileChangeOwnership and the EditorShell tests pass. scripts/studio-runtime-smoke.mjs passes locally against the Vite dev server.
  • Studio tsc --noEmit is clean. oxfmt and oxlint pass. check:studio-copy-dashes passes. fallow audit reports no new findings in the changed files.
  • lint-skills, check-core-skills and check-skill-mirror pass. Skills manifest regenerated.
  • Manual, headless Chrome against the built CLI, rerun after the fan-out change (node packages/cli/dist/cli.js preview) on a 14-frame science film:
    • No strip without a board.
    • Writing boards/storyboard.html showed the strip and brought Storyboard to the front.
    • Rewriting a frame title reloaded the board in place.
    • Open frame, ←/→ and Esc work.
    • The desktop card opens from the board's bar.
  • Comment ratchet: not run locally, because this box's git lacks check-attr --source. CI runs it.
  • Not checked: dark theme screenshots, and the hand-off on a machine with Desktop installed (this box is Linux without it).

🤖 Generated with Claude Code

https://claude.ai/code/session_01HebQXPiLcYKQfvPWB3Fgxy

@github-actions

github-actions Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 267 (base branch 267), smooth 122 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (8)

Unstable (1)

  • seqfastkeys-none-px-r0-root-z100: tracking 0.06, pressJump 0.05, drop 0, reload 0, render 0.02, renderKey -, undo false, teleport true / tracking 0.06, pressJump 0.05, drop 0, reload 0, render 0.02, renderKey -, undo true, teleport true / tracking 0.06, pressJump 0.05, drop 0, reload 0, render 0.02, renderKey -, undo true, teleport true

@WaterrrForever
WaterrrForever force-pushed the miao/studio-storyboard-tab branch from b4d59ae to 2c292e3 Compare November 6, 2026 07:29
WaterrrForever and others added 6 commits November 6, 2026 10:02
…the preview

The board an agent writes per the storyboard recipe (boards/storyboard.html,
or a root storyboard.html) now shows in Studio the way HyperFrames Desktop
shows it: a Preview | Storyboard strip over the preview panel, only while the
project has a board. Studio reads the page's class="frame" boxes into a board
of stills, with acts, titles, times and words, and one frame at a time with
arrow keys. A page with no frames shows as written in a script-less iframe.

A board written while Studio is open brings its tab to the front, as Desktop
does at the agent's stop. A rewrite of the page, a still beside it or
STORYBOARD.md reloads it in place, and a dot marks a rewrite seen from the
preview. "Show in preview" goes back to the film at the frame's start.

Notes, drawings and the go belong to Desktop's agent, so the board's bar
carries the desktop-app button. Its funnel events now say which button was
pressed (placement: header or storyboard).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HebQXPiLcYKQfvPWB3Fgxy
… smaller parts

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HebQXPiLcYKQfvPWB3Fgxy
…b, not the file

With the board shown beside the preview in Studio as in Desktop, the recipe's
stop and style-sampling's stop 1 tell the person to look in the Storyboard
tab, and to start a preview and give its link when none is running, instead
of handing over boards/storyboard.html to open from disk.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HebQXPiLcYKQfvPWB3Fgxy
…el, and its notes are trimmed

The board opened its own /api/events stream, which the Vite dev server
behind the runtime smoke does not serve. It now subscribes through
subscribeFileChanges, the one channel picker the external-change
coordinator already uses (test adapter, Vite HMR, then SSE), so dev and
built Studio deliver the same changes to both.

The new files' comments are cut to the package's share, and the recipe's
Storyboard-tab line moves to the paragraph that names the board, leaving
the stop step as it was.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HebQXPiLcYKQfvPWB3Fgxy
…t a second subscription

The external-change coordinator stays the one subscriber to the file-change
transport (HMR in dev, SSE when built), as externalFileChangeOwnership
asserts. It announces each decoded change through fileChangeFanout, and the
storyboard listens there. Mounting EditorShell alone, as its tests do, no
longer opens an EventSource, and the coordinator file stays under the line cap.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HebQXPiLcYKQfvPWB3Fgxy
…en on the board, and its card says why

- The storyboard bar's button posts `doc: "storyboard"` with its page id. The preview server passes only that value
  on, and `openInDesktop` writes it into `.hyperframes/desktop-cta.json` beside the page id, so the app can open the
  project on its Storyboard tab with Build in sight (hyperframes-internal reads it).
- The guide card opened from the storyboard says "Approve this storyboard in the desktop app" and "Comment on a frame,
  draw on it, then press Build", and its last step says the app opens on this storyboard. The header's card is
  unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HebQXPiLcYKQfvPWB3Fgxy
@WaterrrForever
WaterrrForever force-pushed the miao/studio-storyboard-tab branch from 2c292e3 to 2eb68a1 Compare November 6, 2026 10:04
@WaterrrForever
WaterrrForever marked this pull request as ready for review November 6, 2026 10:24
WaterrrForever and others added 2 commits November 6, 2026 10:41
…ay in the desktop app

A Studio user without the desktop app had no way to approve the board from the tab, and the bar said approval was
in the desktop app, so the tab read as a dead end. Studio talks to the person's own agent through their chat, so the
bar's Approve copies "The storyboard is approved. Build the video from it." for them to paste to it, and says so in
a toast (or spells the words out when the clipboard refuses). Commenting and drawing on frames stay in the desktop
app. The storyboard recipe tells the agent those pasted words are the person's go.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HebQXPiLcYKQfvPWB3Fgxy
…pproving now happens in Studio

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HebQXPiLcYKQfvPWB3Fgxy

@jerrai-bot-heygen jerrai-bot-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At e73b0e27f2480f6ec33ad63bb0f137ecb783e535, I found two code issues in the OSS Storyboard layer: the Desktop hand-off record is written after the folder is dispatched, creating a first-open race with #9010, and root-level boards do not refresh when a referenced still changes. See inline scenarios. The normal route only forwards literal doc: "storyboard", Approve copies the stated prompt rather than sending it automatically, and the hosted Studio/CLI checks reported success at this head. I did not reproduce a real installed-Desktop handoff. This is a code review, not merge authority; Miguel/Rory own the final feature merge. — Jerrai

Comment thread packages/cli/src/utils/desktopApp.ts Outdated
const opened = (app: string): DesktopOpenResult => {
if (cta) writeRecord(dir, DESKTOP_CTA_RECORD, JSON.stringify({ cta, at: Date.now() }));
if (cta || doc)
writeRecord(dir, DESKTOP_CTA_RECORD, JSON.stringify({ cta, at: Date.now(), doc }));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new doc record is written inside opened(), after open(id, dir)/launch(executable, dir) has already handed the folder to Desktop. With an already-running single-instance Desktop, it can process that folder and read .hyperframes/desktop-cta.json before this write, so #9010 opens the film instead of the requested Storyboard tab. Please make the intent durable before dispatching the folder (with a safe failed-open cleanup/expiry path), and test the hand-off callback reading the record at dispatch time rather than only checking it after openInDesktop returns.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 0f815f4. openInDesktop now writes the record before open/launch. A header or plain open drops an earlier storyboard ask, and a failed open puts the earlier record back. Desktop already ignores a record over 10 minutes old. New tests read the record from inside the open callback, which is dispatch time, and cover a storyboard press followed by a header press and by a plain open. They fail with dispatch moved back before the write.

const path = changed.replace(/\\/g, "/").replace(/^\.\//, "");
if (path === board || path.toLowerCase() === PLAN_FILE) return true;
const folder = folderOf(board);
return folder !== "" && path.startsWith(folder);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lookup deliberately supports a root-level storyboard.html, but for that board folderOf(board) === ''; replacing a referenced stills/frame.png does not match either early return and never bumps the revision. Studio keeps the prior still URL/preview even though the agent rewrote that frame image. Please include referenced stills for root boards (without treating every project write as a board change), and cover root board + changed still in the test.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 0f815f4. The hook keeps the resolved project paths of the stills the board draws, and touchesBoard counts a write to any of them. That covers a root board's stills/… and a boards/ page's ../assets/…, without treating unrelated writes as board changes. The new test covers both, plus a non-drawn still and index.html staying false.

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at e73b0e2

Need: yes, expose the agent's existing board · Size: ok as one feature · Reuse: existing preview route, desktop CTA and change coordinator · Simpler: keep one persistent preview container · Structure: board parsing, fetching and presentation are separated
Risk: medium — player lifecycle and a cross-process handoff change together.

The literal-only doc forwarding (desktopRoutes.ts:61–68) and manual clipboard approval (ApproveBoardButton.tsx:6–23) keep the handoff narrow and leave sending the build instruction to the person.

Important — keep the film mounted and paused across board transitions. packages/studio/src/components/storyboard/PreviewStage.tsx:45–65 switches from a fragment containing the film to a different nested tree when the first board appears. React unmounts/remounts the preview subtree, including its iframe, instead of preserving it. The automatic selection at lines 21–26 also bypasses the pause in show() at line 40. My mounted probe observed two film mounts and zero pause calls on absent → present. Keep the film container at the same parent position and component identity even without a tab strip, and route automatic/manual board entry through a shared pause/hold path. Cover first appearance, removal/reappearance and playback shortcuts while the board is visible. A temporary stable-container + automatic-pause control made the same probe pass; source was restored. Zero pause calls establishes the skipped pause path, not a measured live audio leak.

I independently agree with @jerrai-bot-heygen's two Important findings, with these additional boundaries:

  • packages/cli/src/utils/desktopApp.ts:203–205: the extracted, unchanged openInDesktop function with real record writing and an early-reading open callback saw no record at dispatch, followed by doc: "storyboard" after return. Write the current intent before either OS dispatch; handle failed dispatch safely and test both a storyboard dispatch and a subsequent header dispatch so an old storyboard intent cannot be reused; the latter is a required boundary, not an additional reproduced case.
  • packages/studio/src/components/storyboard/storyboardBoard.ts:19–23: the invalidation mismatch also affects accepted ../assets/a.png stills from a board in boards/. Track referenced resolved still paths as well as the board; test root and parent-directory still creation/replacement without invalidating every unrelated write.

Compatibility: the new record is additive: the old Desktop has no reader/schema rejecting doc; #9010 returns no special landing for old records without it. Either PR can merge independently at the record-format level. This does not cure the dispatch-order bug above. Path lookup accepts only the root/boards filenames; preview serving still enforces its existing lexical containment and deliberately permits explicit project asset symlinks. No new symlink escape established. The fanout unsubscribes and stale fetch results are guarded.

Audited: new board modules, CTA/route/record changes, coordinator integration and immediate handoff, preview-serving and playback consumers; skills changes and tests. Not exercised: installed Desktop handoff, multi-window/duplicate handoff delivery, full browser layout or full package build. The focused React lifecycle probe failed as described. Standard CLI/Studio suites could not collect with borrowed dependencies (missing package exports/Ajv); I am not claiming the author's suite counts locally.

Verdict: REQUEST CHANGES
Reasoning: the new stage remounts the film, and handoff ordering/live-still invalidation can show the wrong or stale content. Fix these cases and pin their transitions in tests.

— Rames (hf-review)

WaterrrForever and others added 2 commits November 6, 2026 15:40
…oard, stills outside the board folder

- openInDesktop writes desktop-cta.json before sending the folder, so a running
  app reads the storyboard ask on arrival; a plain or header open drops an
  earlier storyboard ask; a failed open puts the earlier record back.
- PreviewStage keeps the film at one place in the tree with or without a board,
  so a board appearing never remounts it; the automatic switch goes through the
  same pause as a click, and a play while the board is up is paused.
- The board reloads when a still it draws changes, including a root board's
  stills/ and a boards/ page's ../assets.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HebQXPiLcYKQfvPWB3Fgxy
… record helper's doc

The CLI tarball carries Studio's build, which now holds the Storyboard tab; merged
with main it measures 7,982,002 bytes packed, 7 bytes past the 1% tolerance.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HebQXPiLcYKQfvPWB3Fgxy

@jerrai-bot-heygen jerrai-bot-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The prior hand-off dispatch race and still invalidation findings are fixed at this head. The record is written before OS dispatch, plain/header opens clear prior storyboard intent and failed dispatch restores the earlier record; the new tests inspect the record from inside dispatch. Root and parent-directory referenced stills now invalidate the board, while unrelated writes do not. The film stays in one React tree position across board arrivals, and both automatic selection and playback shortcuts pause it; mounted regressions cover these paths. The successor commit from 0f815f44d changes only the packed CLI size budget (with measured reason in the PR body) and a doc comment. This is a code approval while hosted checks are running; required CI and Miguel/Rory's feature integration remain separate gates. No installed-app handoff was performed by me.

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 0952f44

Need: yes, show the agent's board · Size: ok · Reuse: existing preview routes, CTA and player lock · Simpler: reject hidden-film playback before starting it · Structure: board parsing and display remain separate
Risk: medium — player lifecycle and cross-process opening intent.

The dispatch-order and referenced-still findings are fixed. I independently checked early Mac/Windows dispatch, header/plain intent clearing and failed-dispatch rollback using the actual extracted function and real record helpers. Root and parent-directory still paths now reach invalidation. The persistent film position and automatic pause-request tests also pass; those focused tests do not establish reverse-RAF cancellation. This agrees with @jerrai-bot-heygen's successor review on those cases.

Important — the requested shortcut hold is still incomplete. With Storyboard visible, press J. The new PreviewStage.tsx:38–43 subscriber calls pause synchronously from setIsPlaying(true), but useTimelinePlayer.ts:273–275 schedules the reverse RAF after that pause. Each reverse tick repeats the ordering at lines 269–270. The hidden film therefore keeps seeking backwards and loses its paused position.

I reproduced this with the actual PreviewStage, player hook, store, keyboard handler and fake adapter: after three controlled RAF callbacks the film moved 10 → 9.699 seconds, with one RAF still queued. Normal Preview reverse playback passes; setting the existing playLocked gate while the board is visible also keeps it at 10. The author test replaces requestPlayback with a call counter, so it proves a pause request occurred, not that playback stopped. This is the remaining sibling of my earlier film/shortcut finding, rather than a new issue in unchanged unrelated code.

Fix: one option is to have board visibility participate in the existing playback-lock policy, preserving other lock owners, so shortcuts and direct playback requests are refused before scheduling work. Alternatively make the player cancellation robust to a pause during startup. Add an integration regression using the real player/store: J under Storyboard must leave adapter time unchanged and no reverse RAF queued; returning to Preview must allow J again when no other lock is active, while preserving another owner's lock.

Evidence: focused copies of the author stage/parser tests: 18 passed, with only UI primitives mocked and unchanged frameStart/boardMode extracted to avoid unrelated borrowed-dependency failures. Reverting the stage makes all three stage tests fail; reverting still invalidation makes its new test fail. Real-player boundary probe: 1 failed, 2 controls passed. Dispatch probe passes at this head and fails at the previous head. The CI-only budget delta is +79,036 packed bytes; I checked its rationale and source delta, not a fresh tarball measurement. No installed Desktop handoff, full build, live browser layout or backend walk by me. The record readers/writers remain format-compatible by the source audit from my prior review; I did not exercise either installed-app merge order.

Verdict: REQUEST CHANGES
Reasoning: the reported order, mounting and invalidation cases are repaired, but reverse playback still moves the film while Storyboard covers it.

— Rames (hf-review)

@jerrai-bot-heygen jerrai-bot-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am replacing my earlier approval at this same head: the film-position/shortcut conclusion was too broad. Rames's real-player probe reproduced J moving a hidden film from 10 to 9.699 seconds with another reverse RAF queued (their review); I verified the source ordering. PreviewStage issues a pause when isPlaying changes, but useTimelinePlayer.ts playBackward() calls setIsPlaying(true) before assigning its new reverse requestAnimationFrame, and each tick likewise schedules the next frame after setIsPlaying(true). A pause during that notification cannot cancel a future frame. My previous evidence only proved the pause request and stable mount, not a held playhead. Please prevent reverse playback before scheduling while the board is visible (respecting other lock owners), or make the cancellation path robust, and add a real-player regression. The dispatch-order and still-refresh fixes remain accepted. No merge action. — Jerrai

A pause after the fact let J's reverse loop schedule its next frame, so the
hidden film kept seeking. The stage now takes the player's own hold while the
board is up (and only when no one else holds it), so J, Space and playback
requests are refused before they start; Preview releases it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HebQXPiLcYKQfvPWB3Fgxy
@WaterrrForever

Copy link
Copy Markdown
Collaborator Author

Re: J under Storyboard, fixed in 91845ef. Your probe was right: pausing after setIsPlaying(true) let the reverse loop queue its frame first. The stage now takes the player's own playLocked while the board is up. It takes it only when no other owner holds it, releases it on Preview, and leaves another owner's hold alone. So J, Space and playback requests are refused before they schedule anything. PreviewStage.player.dom.test.tsx mounts the real useTimelinePlayer, store and keyboard. With J under the board, the adapter stays at 10 s and no frame is queued. Back on Preview, J queues frames again. Against the previous stage it fails at 11.43 s.

@jerrai-bot-heygen jerrai-bot-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed 91845efc101dbd4fd88eef41bdd5f2c0980233cf. The new playLocked gate is reached before reverse J schedules its RAF, and the real-player test measures the playhead/queue rather than merely counting pause calls. That repairs the prior reverse-playback finding in isolation. One lock-ownership gap remains: the stage skips taking a hold if one exists, yet its effect does not reacquire when that owner releases; in the reverse order its cleanup can clear a later owner's hold. See inline scenarios. I did not run a local suite or installed-app handoff; hosted checks are separate. No approval or merge action. — Jerrai

if (tab !== "storyboard" || usePlayerStore.getState().playLocked) return;
usePlayerStore.getState().setPlayLocked(true);
return () => usePlayerStore.getState().setPlayLocked(false);
}, [tab]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The real-player J regression fixes the reverse-RAF race, but this boolean is not an owned lease. Two opposite lock-order cases remain: (1) if another owner already has playLocked=true when Storyboard opens, this effect returns without acquiring anything; when that owner releases while the board stays open, J/Space becomes allowed behind the board. (2) if Storyboard acquires first and another owner sets the same boolean true during the board view, this cleanup writes false on Preview and releases that owner's hold. setPlayLocked is a plain boolean setter in readinessSlice.ts; there is no ownership count. The current test covers only an owner held before Storyboard and released after it. Please make lock ownership composable (or otherwise prove the two handoff orders safe) and pin both transition orders in a test, preserving the real-player J case.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Today the stage is the only runtime writer of playLocked in OSS Studio, so both orders are unreachable. If a second owner is added, playLocked should become an owned lease (a holder count or a set) rather than a boolean, and I'll leave that to that change. Thanks for confirming the J regression.

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 91845ef

Need: yes, show the existing agent board · Size: ok · Reuse: player's existing playback lock · Simpler: refuse playback before scheduling · Structure: fine
Risk: medium — player lifecycle and Desktop handoff.

The last Important finding is fixed. Storyboard now holds playLocked before playback can start; the real keyboard/player test leaves the film at 10 seconds with no frame queued, then permits J after returning to Preview. The stage releases its own hold on departure and leaves a pre-existing hold untouched. I traced J/L through the shuttle guard and Space/playback requests through the player's existing guard; the only runtime lock setter in this repository is this stage. This establishes the current lifecycle, not a general multi-owner lock protocol.

Follow-up (Nit): I checked @jerrai-bot-heygen's latest ownership finding and both proposed overlap orders. The boolean cannot compose independent holders. However, the complete repository runtime scan has no second setter/field writer today, so I could not establish a currently reachable overlap failure. I am treating lease/token composition as a follow-up when another owner is added, rather than blocking this sole current owner on a hypothetical second writer. The pre-existing-hold control passes; it is not proof of arbitrary overlapping ownership.

Evidence: vitest run --config adversary.review.config.mjs: 19 passed (author stage/parser tests and the new real-player regression, in focused copies with UI/player barrel mocks and unchanged pure view helpers extracted for borrowed-dependency compatibility). Replacing only PreviewStage with the previous head makes the real-player test fail on a moved playhead; restoring this head returns 19 passed. An independent adversarial pass also ran the three stage tests plus unmount and StrictMode/removal controls: 5 passed. The production code was restored after mutation.

This delta touches only the stage hold and its tests. The previously verified dispatch-before-record-read, header/plain clearing, failed-dispatch rollback, persistent film mount and referenced-still invalidation fixes remain unchanged (prior review). Record-format compatibility remains additive by source inspection. No full build, installed Desktop handoff, live backend or browser-layout walk by me. Hosted CI is still running.

Verdict: APPROVE
Reasoning: the real-player J regression exercises reverse playback and now leaves the hidden film held, and no blocker remains in the reviewed change.

— Rames (hf-review)

@jerrai-bot-heygen jerrai-bot-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approval supersedes my changes-requested review at the same head. The boolean playLocked cannot in general compose independent overlapping owners, but the current OSS Studio runtime has only this stage as a writer; I could not establish either overlap order as reachable in this product today. Rames independently checked the runtime writer census and passed the real-player J regression and adversarial stage lifecycle controls (review); my source review agrees that the lock is acquired before playback starts and that the earlier dispatch/still fixes remain. Treat a multi-owner lease as future hardening if another runtime writer is introduced, not a blocker for this head. Hosted CI and Miguel/Rory's merge decision remain independent; I did not run an installed-app handoff. No merge action.

@WaterrrForever
WaterrrForever added this pull request to the merge queue Nov 6, 2026
Merged via the queue into main with commit 318724f Nov 6, 2026
131 checks passed
@WaterrrForever
WaterrrForever deleted the miao/studio-storyboard-tab branch November 6, 2026 16:30
SanHsien added a commit to SanHsien/hyperframes that referenced this pull request Nov 6, 2026
…view to heygen-com#7265

Adopt heygen-com#7284 (Studio Storyboard tab and `doc: "storyboard"` Desktop handoff,
adapted to the fork's desktopApp without heygen-com#6897) and heygen-com#7324 (drawElement gate:
webgl_unavailable / beginframe_no_self_verify). PR axis heygen-com#7260-heygen-com#7265 carries
earlier verdicts; re-pin heygen-com#6695 and heygen-com#6816, resolve heygen-com#7150 as superseded by heygen-com#5746.
Watermarks: commit 18fc75d, PR heygen-com#7265, issue heygen-com#7223, monitored 99.

Co-authored-by: WaterrrForever <miao.yang@heygen.com>
Co-authored-by: Vance Ingalls <vance@heygen.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants