feat(scenes): ambient first-load preview#42
Merged
Conversation
A scene sequence paused at time 0 renders every scene in its
pre-entrance state, so the default first-load frame is a blank stage
behind a play button. Fill it with the lesson itself instead of a poster
asset: `createScenesProvider` and `ScenesPlayer` take an optional
`preview: { endSeconds, cycles? }`, and with it the stage plays its
opening window muted on load, loops it (twice by default), then holds
the opening scene's settled final frame.
Throughout the preview MediaState keeps reporting a paused player at
time 0, carrying the viewer's own volume and muted values. Every chrome
surface and every consumer already keys off that idle shape, so the idle
overlay, control bar and captions behave exactly as they do without a
preview and nothing downstream mistakes the loop for playback. Only
buffered and readyState flow through, since loading progress is not
clock state and cross-lesson autoplay arms on the readyState edge.
The first transport action hands the clock back: the host is paused,
seeked to where the viewer wants it, and given back their audio settings
and rate, after which snapshots flow normally and nothing
preview-related runs again. play() hands back at the top, so the preview
is a tease of the opening rather than progress through it. setRate is
the one exception, since the idle speed chips set a rate and then play.
The loop always runs at 1x whatever speed the viewer watches at: their
rate is how fast they want to learn, not how fast a teaser should move,
and a saved 2x or 2.5x turns the loop frantic.
Two ordering guards, both against snapshots the host queued before a
seek landed: one stops a boundary snapshot burning a second loop cycle,
the other drops a stale snapshot that would flash the preview clock into
the scrubber on hand-back.
endSeconds comes from the caller, which is the side that has the
manifest; the opening scene's boundary is the natural window, since one
that ends mid-motion also restarts mid-motion. The preview is skipped
for prefers-reduced-motion, for saveData, and when autoPlay is set. It
waits for the player to intersect the viewport before starting so a
lesson below the fold does not spend its loops unwatched, and if muted
autoplay is refused (iOS low power mode) it settles on the held frame,
which is still a real frame of the lesson.
No protocol change and no host change, so this needs no runtime
republish. Omit `preview` and nothing changes.
📝 WalkthroughWalkthroughThe scenes provider adds an optional ambient first-load preview that loops a muted opening segment, holds its final frame, and hands control back on viewer interaction. Preview behavior includes suppression, viewport deferral, autoplay handling, state filtering, teardown, and comprehensive tests. ChangesAmbient first-load preview
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Host as kino host
participant Provider as createScenesProvider
participant Viewer as Viewer
Host->>Provider: kino:ready
Provider->>Host: mute, seek, and play preview
Host->>Provider: preview state ticks
Provider->>Host: loop or settle opening frame
Viewer->>Provider: first transport action
Provider->>Host: restore viewer state and resume normal playback
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
kino | 3756463 | Commit Preview URL Branch Preview URL |
Jul 25 2026, 09:45 PM |
![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
🧹 Nitpick comments (1)
src/scenes/provider.test.ts (1)
1060-1088: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest-environment mutations are undone after the assertions, so any failure leaks into the rest of the suite. All four of these tests install a global (fake timers,
window.matchMedia,navigator.connection) and restore it as the final statement; a throwingexpectskips the restore and every later test inherits fake timers or a suppressed preview.
src/scenes/provider.test.ts#L1060-L1088: drop the trailingvi.useRealTimers()calls and restore timers from anafterEach.src/scenes/provider.test.ts#L1116-L1136: replace the manualwindow.matchMediaassignment andObject.defineProperty(navigator, "connection", …)withvi.stubGlobalplus anafterEachteardown.🤖 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 `@src/scenes/provider.test.ts` around lines 1060 - 1088, Ensure test-environment cleanup runs even when assertions fail: for src/scenes/provider.test.ts lines 1060-1088, remove the trailing vi.useRealTimers() calls and restore timers from an afterEach; for lines 1116-1136, replace direct window.matchMedia and navigator.connection mutations with vi.stubGlobal and restore both stubs in afterEach. Keep each test’s existing assertions and setup behavior unchanged.
🤖 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 `@src/scenes/provider.ts`:
- Around line 522-529: Update the preview audio-command handling alongside
setRate so setMuted and setVolume do not forward changes while
previewHoldsClock() is true, keeping the ambient loop silent; retain the
existing patch/state bookkeeping so releasePreview restores state.muted and
state.volume, and add coverage next to the setRate test.
- Around line 410-419: Bound the preview hand-back guard in the provider’s
kino:state handling so missed or clamped seek snapshots cannot suppress
MediaState indefinitely; track the number of dropped in-flight snapshots with
previewResumeDrops and clear previewResumeAt after the allowed limit while
preserving the epsilon match behavior. Update releasePreview to reset
previewResumeDrops to zero whenever it sets previewResumeAt.
- Around line 358-360: Update the preview lifecycle around armPreview and
releasePreview to add an explicit retired latch that distinguishes “never armed”
from “released.” Set the latch when releasePreview retires the preview, and have
armPreview return early when retired so subsequent kino:ready events cannot
re-arm the muted loop or pin MediaState.
---
Nitpick comments:
In `@src/scenes/provider.test.ts`:
- Around line 1060-1088: Ensure test-environment cleanup runs even when
assertions fail: for src/scenes/provider.test.ts lines 1060-1088, remove the
trailing vi.useRealTimers() calls and restore timers from an afterEach; for
lines 1116-1136, replace direct window.matchMedia and navigator.connection
mutations with vi.stubGlobal and restore both stubs in afterEach. Keep each
test’s existing assertions and setup behavior unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b9f44624-27c7-4baa-bdad-825c583a234c
📒 Files selected for processing (4)
.changeset/silly-donuts-shave.mdsrc/scenes.tssrc/scenes/provider.test.tssrc/scenes/provider.ts
Comment on lines
+358
to
+360
| const armPreview = () => { | ||
| if (!preview || previewPhase !== "off") return | ||
| if (previewSuppressed()) return |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
armPreview cannot tell "never armed" from "retired", so a second kino:ready re-arms the preview mid-session.
releasePreview parks previewPhase back at "off" — the same value it starts at — so if the host document announces ready again (reload/recovery in the same iframe) after hand-back, the muted loop restarts, previewHoldsClock() goes true again, and MediaState is pinned at paused/time 0 while the host actually plays. Latch the retirement instead of inferring it from the phase.
🛠️ Proposed fix: explicit retired latch
+ // Set by the hand-back: the preview is retired for the life of this
+ // provider, even though the phase parks back at "off". A host that
+ // re-announces ready must not re-arm it under the viewer.
+ let previewRetired = false
const previewHoldsClock = () =>
previewPhase === "running" || previewPhase === "settled" const releasePreview = (time: number) => {
if (previewPhase === "off") return
const disturbed = previewHoldsClock()
previewPhase = "off"
+ previewRetired = true const armPreview = () => {
- if (!preview || previewPhase !== "off") return
+ if (!preview || previewRetired || previewPhase !== "off") return
if (previewSuppressed()) return📝 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 armPreview = () => { | |
| if (!preview || previewPhase !== "off") return | |
| if (previewSuppressed()) return | |
| const armPreview = () => { | |
| if (!preview || previewRetired || previewPhase !== "off") return | |
| if (previewSuppressed()) return |
🤖 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 `@src/scenes/provider.ts` around lines 358 - 360, Update the preview lifecycle
around armPreview and releasePreview to add an explicit retired latch that
distinguishes “never armed” from “released.” Set the latch when releasePreview
retires the preview, and have armPreview return early when retired so subsequent
kino:ready events cannot re-arm the muted loop or pin MediaState.
Comment on lines
+410
to
+419
| // A snapshot the host queued before the hand-back seek landed still | ||
| // carries the preview clock; drop it rather than flash it. | ||
| if (previewResumeAt !== null) { | ||
| if ( | ||
| Math.abs(msg.state.currentTime - previewResumeAt) > | ||
| PREVIEW_RESUME_EPSILON_S | ||
| ) | ||
| break | ||
| previewResumeAt = null | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
The hand-back guard is unbounded: one missed tick window freezes MediaState permanently.
previewResumeAt only clears on a snapshot within ±0.5s of the seek target, but after a play() hand-back the host clock walks away from that target monotonically. If ~5 consecutive 10Hz ticks are missed (backgrounded/throttled tab, slow resume after the seek) or the host clamps the seek target, no snapshot ever lands inside the epsilon and every subsequent kino:state is dropped — the scrubber and paused stay frozen at the hand-back value for the life of the provider while audio plays. Bound the guard so it can only swallow the in-flight snapshots.
🛠️ Proposed fix: expire the guard instead of waiting indefinitely
+// A queued snapshot is at most a tick or two old; past that the host has
+// simply moved on and its clock is authoritative again, epsilon or not.
+const PREVIEW_RESUME_MAX_DROPS = 3 let previewResumeAt: number | null = null
+ let previewResumeDrops = 0 if (previewResumeAt !== null) {
if (
Math.abs(msg.state.currentTime - previewResumeAt) >
- PREVIEW_RESUME_EPSILON_S
- )
- break
+ PREVIEW_RESUME_EPSILON_S &&
+ previewResumeDrops < PREVIEW_RESUME_MAX_DROPS
+ ) {
+ previewResumeDrops += 1
+ break
+ }
previewResumeAt = null
+ previewResumeDrops = 0
}releasePreview should reset previewResumeDrops = 0 alongside setting previewResumeAt.
🤖 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 `@src/scenes/provider.ts` around lines 410 - 419, Bound the preview hand-back
guard in the provider’s kino:state handling so missed or clamped seek snapshots
cannot suppress MediaState indefinitely; track the number of dropped in-flight
snapshots with previewResumeDrops and clear previewResumeAt after the allowed
limit while preserving the epsilon match behavior. Update releasePreview to
reset previewResumeDrops to zero whenever it sets previewResumeAt.
Comment on lines
+522
to
529
| patch({ rate: r }) | ||
| // Deliberately not a hand-back: the idle speed chips set a rate and then | ||
| // play, so a rate change alone must leave the preview alone. Speeding up | ||
| // the muted loop under the viewer would only look like a glitch; the | ||
| // chosen rate rides the hand-back instead. | ||
| if (previewHoldsClock()) return | ||
| send({ type: "kino:setRate", rate: r }) | ||
| sendMirror({ type: "kino:setRate", rate: r }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
setRate holds the preview, but setVolume/setMuted do not — the ambient loop can be unmuted.
The preview's contract is a silent loop, yet MediaState reports the idle player, so the chrome's mute/volume controls are live while it runs. setMuted(false) (or setVolume) forwards straight to the host and makes the loop audible at the loop's own position — audio the viewer never started. State bookkeeping is already right (releasePreview restores state.muted/state.volume), so these need the same hold setRate just got. Worth a test alongside the setRate one at Line 1046.
🛠️ Proposed fix: hold audio commands too
setVolume: (v) => {
const vol = Math.min(1, Math.max(0, v))
- send({ type: "kino:setVolume", volume: vol })
patch({ volume: vol })
+ // Same rule as setRate: the loop is silent by contract and the chosen
+ // volume rides the hand-back.
+ if (previewHoldsClock()) return
+ send({ type: "kino:setVolume", volume: vol })
},
setMuted: (m) => {
- send({ type: "kino:setMuted", muted: m })
patch({ muted: m })
+ if (previewHoldsClock()) return
+ send({ type: "kino:setMuted", muted: m })
},🤖 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 `@src/scenes/provider.ts` around lines 522 - 529, Update the preview
audio-command handling alongside setRate so setMuted and setVolume do not
forward changes while previewHoldsClock() is true, keeping the ambient loop
silent; retain the existing patch/state bookkeeping so releasePreview restores
state.muted and state.volume, and add coverage next to the setRate test.
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.
Why
A scene sequence paused at time 0 renders every scene in its pre-entrance state (opacity 0, off-position), so the default first-load frame is a blank stage behind a play button. That is karnstack#150.
The issue proposed a poster raster. Nothing writes
manifest.postertoday, so that would mean a new capture step at ship time plus a republish of every lesson bundle, and an asset to keep in sync forever. Filling the frame with the lesson itself costs no new assets, no republish, and no protocol change.What
createScenesProviderandScenesPlayertake an optionalpreview:With it, the stage plays its opening window muted on load, loops it, then holds the opening scene's settled final frame.
The provider lies to
MediaState. For the whole preview it keeps reporting a paused player at time 0, carrying the viewer's own volume and muted values. Every chrome surface and every consumer already keys off that idle shape, soIdleOverlay,ControlBar,Captions,GestureLayerand downstream consumers (resume position, cross-lesson autoplay, end watcher) need zero changes and none of them can mistake the loop for playback. OnlybufferedandreadyStateflow through, since loading progress is not clock state and cross-lesson autoplay arms on thereadyStateedge.The first transport action hands the clock back: pause, seek to where the viewer wants it, restore their muted/volume/rate, then snapshots flow normally and nothing preview-related runs again.
play()hands back at the top, so the preview is a tease of the opening rather than progress through it.setRateis the deliberate exception, since the idle speed chips set a rate and then play.The loop always runs at 1x whatever speed the viewer watches at. Their rate is how fast they want to learn, not how fast a teaser should move; a saved 2x or 2.5x turns the loop frantic. The real rate rides the hand-back.
Skipped when
prefers-reduced-motion: reducenavigator.connection.saveDataautoPlayis set (already starting the real thing)Two ordering guards
Both against snapshots the host queued before a seek landed: one stops a boundary snapshot burning a second loop cycle, the other drops a stale snapshot that would flash the preview clock into the scrubber on hand-back.
Scope
Provider-side only. No wire protocol change and no
host.tsxchange, so embedders need no runtime republish. Omitpreviewand nothing changes.Testing
232 pass (20 new in
src/scenes/provider.test.ts), plus typecheck, lint, format and build.New coverage: handshake starts the loop muted and mutes before it plays; the loop never reaches
MediaState; no caption fires; the window loopscyclestimes then settles; a queued snapshot cannot burn a cycle;play()hands back at the top with the viewer's audio;play()after settling still starts from the top; a stale snapshot after hand-back is dropped;seek()hands back at the requested time;setRateholds the preview; refused autoplay settles after the grace; a rolling loop is never settled by the grace timer;autoPlaywins; a too-short window is no preview; each suppression path; deferred start on intersection; teardown of a preview that never started.Also driven live in a browser against a real lesson, recorded off the wire: two full
[0, 25]cycles, loop-back at cycle 1, settle at 24.85 paused,MediaStateidle throughout, play button and speed chips up, scrubber never appeared, and the hand-back seek + unmute on play.Consumer
karnstackapps/webderives the window from the manifest it already fetches (min(scenes[0].end, 25)) and skips the preview when a resume position exists, so a returning viewer still lands on their own frame. That change is ready and waits on this release.Summary by CodeRabbit