perf(classroom): speed up classroom loading (media hydration, sidebar thumbnails, media range requests) - #1276
Conversation
… path Entering a classroom awaited the full mediaFiles restore — one object URL per restored image/video blob — before the loading gate opened. For video-heavy courses that is hundreds of MB of IndexedDB materialization blocking first paint. Split the restore into two phases: - The awaited phase now builds metadata-complete task entries but only creates object URLs for failed rows (none needed) and for media referenced by the scene the classroom opens on (persisted cursor, else the first scene). buildRestoredMediaTasks gains an optional shouldHydrateBlob predicate, defaulting to eager hydration so existing callers are unchanged. - Remaining blob-backed records hydrate in the background, chunked over requestIdleCallback (setTimeout fallback). Each deferred task stays 'done' — so generation resume never re-runs it — but carries no objectUrl yet, which the media resolution state machine already renders as a pending skeleton until the URL lands. Background hydration is guarded per record: a task that was replaced (classroom switch, regeneration, retry) or that belongs to another stage is skipped and its freshly minted URLs are revoked immediately.
The playback scene sidebar mounted a full SlideCanvas for every slide scene the moment the classroom opened (and every off-screen video element opened a preload="metadata" fetch), because SlideThumbnail's existing visible prop was never passed. Extract the editor nav rail's near-viewport IntersectionObserver hook to lib/hooks/use-near-viewport.ts and gate the sidebar's slide thumbnails through it: only scenes within 200px of the viewport render the live canvas; the rest show SlideThumbnail's existing placeholder until scrolled near. The placeholder keeps the same box size, so gating never shifts layout. The shared hook now starts hidden instead of eager: with the previous eager-initial state, opening a long deck still mounted every canvas for a frame before the observer could flip off-screen items off. The observer's guaranteed initial delivery flips near-viewport items within a frame; the no-IntersectionObserver fallback (e.g. jsdom) defers to a microtask so the effect never synchronously re-renders.
… media Renderer: BaseImageElement's <img> now carries loading="lazy" and decoding="async", so thumbnail-heavy surfaces (playback sidebar, editor nav rail, course cards) no longer fetch and decode every slide image up front. In-viewport images are unaffected — the browser fetches them immediately. slideToPng forces eager loading inside its permanently off-screen snapshot tree, where lazy images would otherwise never fetch and exports would capture blank slides. Server: GET /api/classroom-media/[classroomId]/[...path] now answers single byte-range requests with 206 Partial Content (Content-Range, Accept-Ranges, correct Content-Length), enabling progressive playback and seeking for hosted video/audio instead of downloading whole files. Suffix ranges are supported; unsatisfiable ranges get 416 with the full size; unsupported units or multi-range sets fall back to the plain 200 full-body response, which is always a legal answer. Existing caching headers are kept on every response shape.
…bound idle waits A deferred record is only ever 'done' without an objectUrl; a task that regeneration or retry restarted passes through pending/generating, so skip those instead of attaching stale persisted bytes. Also give the idle scheduling a timeout so a busy main thread cannot starve hydration.
A restore epoch captured at apply time now gates each idle chunk: loading another classroom or reloading the same one invalidates any older hydration loop, so it neither keeps scheduling work for an abandoned classroom nor attaches bytes read by an older load to a newer load's tasks.
…s priority A 416 with public immutable caching can poison the media URL for later valid requests, so range errors now send Cache-Control: no-store. Priority classification also collects the opening scene's media element ids, since task lookup binds records keyed stage:<elementId> even when the slide slot carries a different opaque ref.
… token The restore epoch only advanced when the next load reached apply, so an abandoned classroom kept hydrating during the next load's storage/network phase. Compose the epoch with the load's isCurrent (load token plus effect cleanup) so navigation stops the loop at the next idle boundary.
Legacy singleton video recovery assigns a placeholderRef only at the end of the task build, so a record keyed by an allocated id without placeholderRef was deferred even when it backs the opening scene's gen_vid_* element. Run a metadata-only build first to learn each record's effective ref and classify against it, keeping the first visible page's legacy video eager.
Cross-review summaryRan 6 codex review rounds over the full diff; converged at zero actionable
CI green on the final head (lint/typecheck/unit, E2E, render service, storage |
LING-6150
left a comment
LING-6150
left a comment
There was a problem hiding this comment.
@wyuc Overall, this looks good to me. I found one edge case around deferred video hydration that may leave play_video stuck:
Deferred videos can miss play_video before their URL is hydrated
This introduces a restored task state where status === 'done' but objectUrl is still undefined. The renderer correctly treats this state as pending, but ActionEngine.executePlayVideo() still treats status === 'done' as immediately playable.
For an imported or legacy classroom, a video on a non-opening scene can still be deferred when the user reaches that scene. In that case, play_video sets playingVideoElementId while the renderer is showing a skeleton and no <video> exists. When hydration later supplies the URL, the component is preserved and its playback effect does not depend on the resolved source, so video.play() is not retriggered. Playback can then remain stuck on this action until the user intervenes or the five-minute timeout expires, after which the video is skipped.
Could we make the readiness checks in executePlayVideo() use the same renderability contract as the renderer? In particular, the initial check, subscription exit condition, and post-subscription recheck should not treat this new done-without-URL state as ready. It would also be useful to cover this with an engine-level regression test: start with done and no objectUrl, verify playback has not started, then add the URL and verify that it starts.
executePlayVideo treated status done as immediately playable, but a deferred restore is done without an objectUrl: the renderer shows a skeleton, no <video> exists, and the later hydration never retriggers play, leaving the action stuck until the safety timeout. Readiness now follows the renderer's contract (only done-with-bytes is playable) at the initial check, the subscription exit, and the post-subscription recheck; the failed skip applies whether or not a wait happened.
|
Good catch — fixed in 5b6064c.
Added the engine-level regression test in the shape you suggested ( |
The stage-level whiteboard stays open across standalone classroom switches, so its media can be visible before any scene is. Classifying it as deferred left visible whiteboard media pending behind idle hydration chunks.
LING-6150
left a comment
LING-6150
left a comment
There was a problem hiding this comment.
Re-verified at afbdfba. The deferred video playback issue I raised is resolved: ActionEngine now waits for a completed task to have its playable URL before starting play_video, including the initial check, subscription condition, and race recheck.
I also reviewed the subsequent stage-whiteboard priority update and did not find any additional actionable issues. Thanks for the quick fixes!
| loading="lazy" | ||
| decoding="async" | ||
| style={{ | ||
| position: 'absolute', |
There was a problem hiding this comment.
3 participants
Summary
Entering a media-heavy classroom currently blocks on hydrating every restored
media blob and eagerly renders a full slide canvas for every scene in the
sidebar. This PR moves the expensive work off the load path.
synchronously and only materializes object URLs for the scene it opens on
(persisted cursor, else first scene). All other restored blobs hydrate in
idle-time chunks afterwards. Consumers resolve a known-but-not-yet-hydrated
task as pending and re-render when the URL lands, so nothing is regenerated
or broken. (
lib/classroom/load-classroom.ts)SlideThumbnailrendering with the near-viewport IntersectionObserveralready used by the editor nav rail (extracted to
lib/hooks/use-near-viewport.ts). Off-screen scenes render the cheapplaceholder and skip their
<video preload="metadata">fetches.loading="lazy" decoding="async"(the off-screenslideToPngsnapshot treeforces eager to keep exports correct), and the classroom media route answers
single byte-range requests with proper 206/416 semantics so hosted
audio/video can stream progressively and seek.
Verification
tsc --noEmit,eslint,prettier --checkcleanvitest run: new tests for deferred media restore, near-viewport hook,range parsing, and the media route; full suite green except pre-existing
environmental timeouts that reproduce identically on the base commit
No user-facing behavior changes: same content, same persistence semantics,
just less work before first paint.