fix: align local WebGPU capture behavior - #2907
Conversation
vanceingalls
left a comment
There was a problem hiding this comment.
R1 review @ 8976543
Verdict: APPROVE. All four focus areas are cleanly resolved with test-locks on each. The GPU-policy centralization is the right shape (one gpuPolicy.ts module, threaded through every local capture surface), the hf-seek completion contract matches the WebPlatform ExtendableEvent.waitUntil idiom, the paused-presentation heartbeat correctly emits at the same seek time without advancing simulation state, and the checker's WebGPU-failure elevation + dedup is scoped exactly to genuine validation/OOM/internal/destroyed-resource conditions.
Grade: A · CORRECT
1. Cross-surface GPU override behavior. New packages/cli/src/browser/gpuPolicy.ts centralizes the precedence chain:
- Explicit
--browser-gpu/--no-browser-gpu→ hardware/software PRODUCER_BROWSER_GPU_MODE=hardware|software|auto→ env- Neither →
"auto", then resolved via the engine's cached probe (resolveBrowserGpuMode)
Threaded through every local capture surface: snapshot (captureSnapshots.browserGpuMode new option), check (--browser-gpu → CheckOptions.browserGpuMode), validate (resolveCliChromeGpuMode now returns BrowserGpuMode and validates via engine probe), layout (auto probe + buildChromeArgs), preview (sets env for downstream Studio server), studioServer.getThumbnailBrowser (caches {requested, resolved} on the browser instance, reuses for assertWebGpuRequirement at each generateThumbnail), and checkBrowser.runBrowserCheck (options.browserGpuMode ?? resolveCliChromeGpuMode() — CLI override wins).
The key bug-fix at the shim level: resolveCliChromeGpuMode used to return "software" | "hardware" and forced auto/"" to "hardware". It now returns BrowserGpuMode ("auto" | "hardware" | "software") and correctly returns "auto" for both. Test-locked at captureCompositionFrame.test.ts (resolveCliChromeGpuMode("auto") === "auto", resolveCliChromeGpuMode("") === "auto").
Distributed / Docker render paths are untouched — the gpuPolicy module lives only in the CLI package. The body's "no distributed-render or Docker GPU defaults were changed" claim holds by construction because no import reaches from producer/, aws-lambda/, or gcp-cloud-run/ into the CLI helper.
assertWebGpuRequirement triggers only on requestedMode === "auto" && resolvedMode === "software" && compositionRequiresWebGpu(html) — never on explicit --no-browser-gpu (author's intent to test the software fallback) or explicit --browser-gpu (already hardware). Diagnostic message names the marker (data-requires-webgpu) and the concrete escape hatches. Correct semantics.
2. hf-seek waitUntil(...) completion contract. The contract chain is end-to-end:
seek-dispatch.ts— dispatch synchronously with a freshHfSeekEventDetail,pending: PromiseLike<unknown>[]closure.waitUntilpushes intopendingwhileaccepting === true; after the sync dispatch,accepting = falseand_pendingCompletion = Promise.all(pending).then(() => undefined). Any post-listenerwaitUntilcall throws ("hf-seek waitUntil() must be called synchronously from the event listener") — this is the correct WebPlatform-style enforcement.runtime/init.ts— publisheswindow.__hfWaitForSeekCompletion = waitForSeekCompletionat bootstrap AND registers a cleanup callback that deletes it on runtime shutdown (guarded by identity checkif (window.__hfWaitForSeekCompletion === waitForSeekCompletion)so a replacement won't be clobbered). Nice defensive pattern.frameCapture.ts— newwaitForPendingSeekCompletion(page)exported; called fromprepareFrameForCaptureaftersession.onBeforeCapture(page, quantizedTime)and before the screenshot function. The gpu-completion test uses a static-source-ordering assertion (readFileSync + indexOf) to lock the call order — brittle to unrelated reorderings in that file, but explicit and correct as a defensive lock.captureCompositionFrame.ts— the CLI-sideseekCompositionTimelineawaitswindow.__hfWaitForSeekCompletionbetween the seek execution and the animation-frame settle. Test atcaptureCompositionFrame.test.ts:168-198verifies the promise-chain: registers a hanginggpuWorkpromise, callsseekCompositionTimeline, assertssettled === falsebeforecompleteGpu(), thensettled === trueafter. Directly locks the "GPU work blocks capture" invariant.
The TypeGPU adapter docs update at typegpu.ts shows callers the new pattern (e.detail.waitUntil(device.queue.onSubmittedWorkDone())), replacing the old "call await onSubmittedWorkDone() after render(time)" advice — which was correct in principle but the framework had no way to observe it. New contract makes the wait explicit and framework-observable.
The _pendingCompletion state is module-level per runtime instance. Each browser page has its own module scope, so concurrent capture sessions (e.g. two Studio thumbnails at once) don't share the completion tracker. Race-safe because dispatch replaces the promise atomically after the synchronous listener chain settles.
3. Paused WebGPU presentation heartbeat. TYPEGPU_PRESENT_HEARTBEAT_MS = 250 interval driven by startPresentHeartbeat / stopPresentHeartbeat:
- Started on
seek(), stopped onplay()andrevert()— matches the "paused only" intent. - Gated on
document.querySelector("[data-composition-id][data-requires-webgpu]")— only paying the 250ms interval cost for compositions that actually declared WebGPU. WebGL / DOM-only compositions are unaffected. - Fires
forceDispatchSeekEvent(forcedTime)with the SAMEforcedTime, so simulation state doesn't advance — listeners re-render the same frame, and any GPU work registered viawaitUntilgets awaited on the next capture.
Test at typegpu.test.ts:91-113 uses vi.useFakeTimers to advance exactly TYPEGPU_PRESENT_HEARTBEAT_MS, asserts times === [1.25, 1.25] (same time both dispatches — no advancement), then calls play() and advances 2× more heartbeat — verifies no further dispatch, so the interval is torn down cleanly. Tight lock on both the same-time invariant AND the stop semantics.
4. Deduplicated WebGPU validation failures. checkBrowser.ts promotion + dedup:
WEBGPU_RUNTIME_FAILUREregex catchesGPUValidationError,GPUOutOfMemoryError,GPUInternalError,WebGPU uncaptured error, and the bidirectionaldestroyed ... submitphrase (both orderings). Word-boundary\bon the type names avoids false-positives in user text.- On the
warnconsole branch:webGpuFailure = isWebGpuRuntimeFailure(text)→ code becomeswebgpu_runtime_error, severityerror; otherwise staysconsole_warning/warning. Ordinary warnings unaffected. pushRuntimeDraftscopes dedup tocode === "webgpu_runtime_error"only: matches by(code, message, url, line)tuple; increments existingcount, otherwise pushes withcount: 1. Console errors, page errors, and media-proxy findings still get one row per event (no dedup) — correct, since those need per-occurrence timelines.runtimeFindingmessage gets"(repeated N times)"appended only whencount > 1. Consumers should key oncode(stable) rather than message.
Test at checkBrowser.test.ts:436-479 locks the split cleanly: two identical WebGPU validation warns → one finding with code: "webgpu_runtime_error", severity: "error", message: "... (repeated 2 times)"; an ordinary warn passes through as code: "console_warning", severity: "warning". Precise assertion, both branches covered in one test.
Findings.
- P1: none.
- P2: none.
- Non-blocker (missing negative-case test on heartbeat DOM gate): the typegpu adapter guards
startPresentHeartbeatondocument.querySelector("[data-composition-id][data-requires-webgpu]"), but there's no test verifying the heartbeat doesn't fire for a composition WITHOUTdata-requires-webgpu. A DOM-only composition regression could re-introduce a 250ms interval on every seek without tripping the existing positive test. Small add: same fixture, remove the marker, asserttimesremains empty (or just the initial1.25). - Non-blocker (static source-code ordering test):
frameCapture-gpuCompletion.test.ts:34-42doesreadFileSync + source.indexOfto lock the call ordering inframeCapture.ts. Brittle to unrelated reorderings in that file (e.g. movingcaptureFrameCoreaboveprepareFrameForCapturefor readability would break it). Alternative would be instrumentation-based ordering assertions on the mockpage.evaluate— more test code, more resilient. Fine as-is; the intent (guard the order at the source level) is clear enough that a reader who breaks it will understand why. - Non-blocker (WebGPU regex false-positive risk): the
WEBGPU_RUNTIME_FAILUREregex would elevate a warn like"Learn more about GPUValidationError"(educational/debug content) toerrorseverity. Very unlikely in practice but possible on compositions that intentionally log WebGPU-related warnings. Non-issue for shipping this PR.
CI: fully green at head across everything I can see — Detect changes (5), Preflight (4), Typecheck, Lint, Format, Fallow, File size, Studio smoke, Perf (drift/fps/load/scrub), Test: skills, Test: runtime contract, SDK, Producer unit + integration, CLI npx shim (ubuntu/macos/windows), Codex plugin package, Preview parity, preview-regression. PR is marked isDraft: true — presumably promoted to non-draft before merge.
Ship it (once un-drafted). The four focus areas are each individually well-designed and the surface-level threading is uniform.
— Via
miga-heygen
left a comment
There was a problem hiding this comment.
PR #2907 Review: Fix local WebGPU capture policy and frame completion
Overview
+560/−93 across core, engine, CLI, and studio. Four subsystems: (1) a shared CLI GPU-policy module that resolves auto through the engine's cached probe, (2) an hf-seek waitUntil() completion contract for GPU frame capture, (3) a paused-WebGPU presentation heartbeat, and (4) deduplicated WebGPU validation errors in the checker.
All four are clean. The completion contract mirrors Service Worker's ExtendableEvent.waitUntil() — a well-understood pattern — and the GPU policy correctly threads every local capture surface without touching distributed/Docker paths.
CI note: Semantic PR title check fails — the title needs a conventional commit prefix (e.g. fix(cli,engine,core):).
1. Cross-surface GPU override behavior — COMPLETE
Every local capture surface now threads the resolved GPU mode:
| Surface | Resolution path | WebGPU guard | --browser-gpu flag |
|---|---|---|---|
snapshot |
resolveLocalBrowserGpuMode(flag) → openSettledCompositionPage → resolveCaptureBrowserGpuMode |
✓ | ✓ Added |
check |
resolveLocalBrowserGpuMode(flag) → CheckOptions.browserGpuMode → checkBrowser.ts |
✓ | ✓ Added |
validate |
resolveCliChromeGpuMode() → resolveCaptureBrowserGpuMode directly |
✓ | env only |
layout |
resolveLocalBrowserGpuMode() → resolveCaptureBrowserGpuMode directly |
✓ | env only |
preview / Studio |
resolveLocalBrowserGpuMode() at browser launch, stored with session |
✓ (per thumbnail) | ✓ via env mutation |
| render (producer) | Engine's own resolveBrowserGpuForCli → config |
Engine handles | Already existed |
Override precedence: CLI flag (highest) → PRODUCER_BROWSER_GPU_MODE env var → "auto" (default). Consistent across all surfaces.
Docker/distributed paths: Untouched. Engine config defaults to "software". Correct — deterministic rendering must stay on SwiftShader.
data-requires-webgpu guard: assertWebGpuRequirement() throws only when auto → software AND the composition declares the marker. Explicit --no-browser-gpu is deliberately allowed ("intentionally testing the fallback"). Clean error message with actionable suggestions.
One minor asymmetry (non-blocking): validate and layout lack the --browser-gpu CLI flag that check and snapshot gain. Users must use the env var for those commands. Not a bug — the env path works — but worth considering for a follow-up to complete the flag surface.
2. Completion ordering (waitUntil) — SOUND
The new hf-seek detail exposes waitUntil(promise):
- Registration: Synchronous only — an
acceptingflag in afinallyblock closes the window afterdispatchEventreturns. Async calls throw with a clear error. This prevents the race that would occur if listeners could register work after the handler returns. - Aggregation:
Promise.all(pending).then(() => undefined)stored as_pendingCompletion. Multiple listeners callingwaitUntilworks correctly — all promises are collected. - Consumption: Two consumers, both correct:
- Engine's
waitForPendingSeekCompletion(page)inprepareFrameForCapture— AFTER video injection, BEFORE screenshot. Correct ordering. - CLI's
seekCompositionTimeline— AFTER seek evaluate, BEFORE animation-frame settle.
- Engine's
- Backward compat: Old compositions that don't call
waitUntil→ emptypendingarray →Promise.all([])resolves immediately → no hang. Ifwindow.__hfWaitForSeekCompletionis undefined (old runtime), optional chaining short-circuits. - No explicit timeout: Intentional —
device.queue.onSubmittedWorkDone()is sub-ms; a timeout risks capturing partial frames. Higher-level navigation/render timeouts provide the safety net. - Cleanup: Identity-checked deletion in
runtimeCleanupCallbacksprevents hot-reload from removing a re-registered function.
The source-order test at frameCapture-gpuCompletion.test.ts is a nice touch — it reads the actual .ts source to verify the call ordering (onBeforeCapture → waitForPendingSeekCompletion → captureFrameCore) instead of relying on a mock that could become stale.
3. Paused WebGPU presentation heartbeat — CORRECT
- Trigger:
startPresentHeartbeat()only fires ifdocument.querySelector("[data-composition-id][data-requires-webgpu]")matches. Non-GPU compositions never get it. - Behavior:
forceDispatchSeekEvent(forcedTime)dispatches the SAME time — bypassing the dedup guard without advancing simulation state. The test confirmstimes === [1.25, 1.25]after one heartbeat tick. - Lifecycle:
pause()starts →play()stops →revert()stops. Idempotent start guard (if (presentHeartbeat !== null) return). No stacking risk. - Cleanup: Browser page teardown implicitly clears intervals. Runtime cleanup calls
revert()→stopPresentHeartbeat(). No leak path. - No heartbeat/waitUntil race: The heartbeat fires every 250ms — well after GPU work completes (~sub-ms).
_pendingCompletionis overwritten per dispatch, so no accumulation.
4. Checker classification — CLEAN
- Regex:
WEBGPU_RUNTIME_FAILUREmatchesGPUValidationError,GPUOutOfMemoryError,GPUInternalError,WebGPU uncaptured error, and destroyed resource patterns. Covers the standard WebGPU error taxonomy. - Promotion: Warning-level console messages matching the regex are promoted to
webgpu_runtime_errorwith severityerror. Ordinary warnings pass through asconsole_warningunchanged. - Deduplication:
pushRuntimeDraft()matches by code + message + url + line. Duplicates incrementcountinstead of adding entries. Display:"(repeated N times)"suffix. - Test coverage: The test fires the same validation failure twice plus one ordinary warning, then asserts the failure appears once with
(repeated 2 times)and the ordinary warning appears separately. Clean.
5. Studio server unification — nice win
The Studio thumbnail capture replaces an inline seek implementation (manual __player.seek / __timelines.pause / gsap.ticker.tick) with the shared seekCompositionTimeline(), which includes the waitUntil completion contract. This means Studio thumbnails now correctly await GPU work before capture. The GPU modes are stored alongside the browser session (_thumbnailBrowserModes) and the WebGPU requirement is checked per-thumbnail.
Verdict
Approve (pending semantic PR title fix). The GPU policy correctly threads every local capture surface while leaving distributed/Docker paths deterministic. The waitUntil completion contract is well-designed (synchronous registration, backward-compatible, no hang risk). The heartbeat is scoped to data-requires-webgpu compositions and cannot advance state. The checker classification promotes genuine GPU failures without suppressing ordinary warnings. 142 focused tests pass.
— Miga
miguel-heygen
left a comment
There was a problem hiding this comment.
Review at exact head 8976543d429369a90bb70bbe99802f134df29caf.
The main GPU-policy and capture-ordering work is well shaped, but I found three reachable gaps that block approval:
-
packages/cli/src/commands/motionShot.ts:228,254-264still bypasses both halves of the new contract. It hard-codes SwiftShader/--disable-gpu, and it emits legacynew CustomEvent("hf-seek", { detail: { time } }). A TypeGPU composition following the new documentede.detail.waitUntil(...)pattern therefore receives nowaitUntil, while motion-shot continues capturing without awaiting GPU completion. Please route this surface through the shared GPU policy/requirement guard and the completion-aware seek path, with a regression covering a WebGPU-marked composition. -
packages/core/src/runtime/adapters/seek-dispatch.ts:48replaces_pendingCompletionon every dispatch. A second force-dispatch or paused heartbeat can therefore makewaitForSeekCompletion()stop waiting for still-pending work registered by an earlier seek generation. Please retain all unconsumed completion generations (and their failures) until the capture wait observes them, with an overlapping-seek test. -
packages/cli/src/commands/preview.ts:208-209,387-429applies--browser-gpuonly to the current process environment, but embedded/background preview may reuse an already-running Studio server. In that path the requested override never reaches the server or its cached thumbnail browser. Please make reuse conditional on a matching GPU policy, or start/refuse reuse when an explicit override differs, and test both foreground and background reuse.
Two smaller follow-ups should be handled in the same pass:
- The shared
data-requires-webgpuerror recommends--browser-gpu, butlayoutandvalidatedo not accept that flag. Either add the flag to those commands or give an actionable command-specific/env-var remediation. - In
studioServer.ts, the old browser'sdisconnectedcallback clears_thumbnailBrowserModesand_thumbnailBrowserInitializingoutside the lease-identity guard. A close/reacquire race can clear state belonging to the newer lease; guard all generation-owned state by the acquired lease identity.
The semantic title check is also still failing because the PR title is not conventional, and the PR remains draft.
miguel-heygen
left a comment
There was a problem hiding this comment.
Re-reviewed exact head 0c8ee3fd955fbe77f70a36d3a51996356a8b37ff.
The runtime-tsconfig typing failure from the previous head is fixed. One capture-correctness blocker remains:
- [P1] Drain seek completions that arrive while the barrier is already waiting —
waitForSeekCompletion()snapshots_pendingCompletionsonce and immediately replaces the queue. If the paused TypeGPU heartbeat (or another forced seek) dispatches after that snapshot whilePromise.all(observed)is still pending, the new generation lands in the replacement queue. Resolving the observed generation lets the current capture continue to screenshot while the newer GPU work is still pending; a rejection from that generation is deferred until a later capture. I reproduced this on this exact head with two forced seeks: start the wait after the first, enqueue the second while the first is unresolved, resolve only the first, and the wait resolves with the second promise still pending. Please drain arrivals to a quiescent/generation boundary and add a regression that dispatches both a success and rejection afterwaitForSeekCompletion()has started.
Two related non-blocking hardening items worth fixing in the same state machine:
- The 250 ms paused TypeGPU heartbeat appends a completion record on every dispatch, including empty/already-fulfilled generations, so a paused Studio session with no capture grows
_pendingCompletionswithout bound. - In MotionShot,
runtimeSeekedis set before invokingrenderSeek/seek/__hfReseekGpu; if the hook throws andtryCallswallows it, the standalonehf-seekfallback is skipped and stale output can be sampled.
No merge or deployment performed.
miguel-heygen
left a comment
There was a problem hiding this comment.
Re-reviewed exact head a0351c3911267474c1bd72b4d9162be0b83768c1.
The prior late-generation escape is fixed: the drain now observes finite generations added while it is waiting, fulfilled/empty generations no longer accumulate, and the immediate-throw MotionShot fallback works. One liveness blocker remains:
- [P1] Use a finite capture fence instead of waiting for global quiescence.
waitForSeekCompletion()now loops until the global pending set is empty. A paused WebGPU composition continues to callforceDispatchSeekEvent()every 250 ms. If each heartbeat registers slow-but-successful GPU work taking at least one interval, there is always another promise in the set and capture never returns—even though every individual generation completes. I reproduced this on the exact head with a continuous 5 ms heartbeat and 20 ms completions: the barrier stayed pending for the full observation window and only completed after the heartbeat stopped. Please suspend/coalesce presentation heartbeats while a capture barrier is active, or await a finite generation/fence that cannot be extended forever, and add fake-timer coverage where completion latency exceeds heartbeat cadence.
Two related P2 contract gaps remain:
- Two concurrent barrier callers observing one rejected generation produce different outcomes because the first caller clears
_pendingFailure; one rejects and the other resolves. - If a MotionShot runtime hook dispatches tracked work and then throws, the fallback awaits only its local work and does not drain the partially registered runtime generation. The new test covers throw-before-dispatch only.
Focused core seek/TypeGPU tests pass 21/21 locally. Exact-head Build, Typecheck, Test, runtime contract, Lint, and Format are green; remaining regression/Windows jobs were still running at review time. No merge or deployment performed.
miguel-heygen
left a comment
There was a problem hiding this comment.
Approved exact head fa621e3631b178b9d2633b6d7160bb3a4376011d.
The three prior blockers are resolved:
- paused TypeGPU heartbeats are suppressed while any capture completion barrier is active, so slow successful GPU work cannot extend the barrier forever;
- concurrent barriers retain and observe the same rejected generation before it is consumed;
- MotionShot now awaits both locally registered fallback work and the runtime completion hook, including dispatch-then-throw fallback.
Focused verification passed locally: core seek/TypeGPU 23/23, MotionShot 5/5, core+CLI typechecks, diff check, formatting, and lint. No unresolved review threads remain. Exact-head CI has no failures but several regression/Windows/smoke jobs are still running; merge remains gated on required terminal-green checks. No merge performed.
miguel-heygen
left a comment
There was a problem hiding this comment.
Approved fresh exact head e83100f3ad400883c952a9a07eb5c29fc9b2ed15.
The follow-up is correctly scoped: renderSeek now enters the same adapter-paused presentation state as ordinary deterministic seeks, which activates the existing TypeGPU same-time heartbeat only for data-requires-webgpu compositions. The heartbeat remains idempotent, stops on play/revert/teardown, and stays suppressed while a seek-completion barrier is active, so this does not reopen the starvation and concurrent-failure bugs fixed in the prior rounds. The regression exercises the real runtime entrypoint and proves the same-time presentation dispatch after the configured heartbeat interval.
Focused local verification passed: runtime init + seek-dispatch + TypeGPU, 92/92 tests; changed-file formatting/lint and git diff --check are clean. Exact-head Build, Typecheck, Test, runtime contract, Windows render, CLI smoke, and other completed checks are green; several regression shards and Windows tests are still running, so merge remains gated on terminal-green required CI. No unresolved review threads. No merge performed.

What
Align local visual-audit and capture surfaces around one browser GPU policy and make WebGPU capture frame-complete.
autothrough the engine's existing cached GPU probe forsnapshot,check, local browser validation/layout capture, and Studio thumbnails/current-frame capture.PRODUCER_BROWSER_GPU_MODEoverrides while leaving distributed and Docker rendering on deterministic software GPU paths.data-requires-webgpuas the minimal composition capability marker and fail with an actionable diagnostic when auto-detection resolves to software.hf-seekevent.detail.waitUntil(...)completion contract and await registered GPU work before screenshots/frame capture.hyperframes checkwhile preserving ordinary warnings.Why
Local capture paths had diverged:
snapshotand Studio thumbnail capture omittedbrowserGpuModeand silently fell through to SwiftShader, whilecheckforced hardware instead of using the engine's auto probe. WebGPU compositions could therefore capture their no-GPU fallback even on capable hosts.Separately,
hf-seeklisteners could submit asynchronous GPU work but HyperFrames had no way to observe queue completion before capture. Paused WebGPU swapchains also needed re-presentation, and GPU validation failures emitted asconsole.warndid not fail the checker.How
A small CLI GPU-policy module delegates auto resolution to
@hyperframes/engineand threads the concrete mode into every local capture browser. The runtime's backward-compatiblehf-seekdetail now exposeswaitUntil, with completion surfaced through a runtime hook consumed by CLI and engine capture paths.The TypeGPU adapter emits same-time presentation heartbeats only for compositions marked
data-requires-webgpu. Checker console handling recognizes WebGPU validation/out-of-memory/internal/destroyed-resource failures and deduplicates identical findings with a repeat count.No distributed-render or Docker GPU defaults were changed.
Test plan
bun run --cwd packages/core test -- src/runtime/adapters/seek-dispatch.test.ts src/runtime/adapters/typegpu.test.ts— 16 testsbun run --cwd packages/engine test -- src/services/browserManager.test.ts src/services/frameCapture-gpuCompletion.test.ts— 46 testsbun run --cwd packages/cli test -- src/browser/gpuPolicy.test.ts src/capture/captureCompositionFrame.test.ts src/commands/snapshot.test.ts src/server/studioServer.test.ts src/utils/checkBrowser.test.ts— 80 testsbun run --filter '@hyperframes/{core,engine,cli}' typecheckbun run lintBuilt
@hyperframes/core,@hyperframes/engine,@hyperframes/studio, and@hyperframes/cliPre-commit tracked-artifact, formatting, Fallow, typecheck, and commitlint gates
Unit tests added/updated
Manual testing performed
Documentation updated (if applicable)