## Summary
Re-architects the studio motion panel to persist GSAP motion data directly in HTML element attributes instead of a `.hyperframes/studio-motion.json` JSON sidecar file. Same pattern as position/resize/rotation edits.
### Before
```
MotionPanel → commitStudioMotionManifestOptimistically()
→ writes .hyperframes/studio-motion.json
→ applyStudioMotionManifest(doc, manifest)
```
### After
```html
<div id="hero" data-hf-studio-motion='{"start":0.5,"duration":1,"ease":"power3.out","from":{"opacity":0,"y":40},"to":{"opacity":1,"y":0}}'>
```
```
MotionPanel → writeStudioMotionToElement(element, motion)
→ buildMotionPatches(element)
→ commitPositionPatchToHtml(selection, patches)
```
## What changed
- **studioMotionOps.ts** — Added `readStudioMotionFromElement()`, `writeStudioMotionToElement()`, `clearStudioMotionFromElement()` for attribute-based CRUD
- **studioMotion.ts** — Added `applyStudioMotionFromDom()` that reads motion from DOM attributes and builds GSAP timeline (kept `applyStudioMotionManifest` for render script compat)
- **manualEditsDom.ts** — Added `buildMotionPatches()` / `buildClearMotionPatches()`, integrated motion into `reapplyPositionEditsAfterSeek()`
- **useDomEditCommits.ts** — Rewrote `handleDomMotionCommit` / `handleDomMotionClear` to use HTML patching instead of manifest persistence
- **useManifestPersistence.ts** — Removed all motion manifest state (~200 lines): `studioMotionManifestRef`, `commitStudioMotionManifestOptimistically`, `applyStudioMotionToPreview`, motion SSE handler
- **App.tsx** — Reads motion from element attribute (`readStudioMotionFromElement`) instead of manifest ref
- **manualEditsRenderScript.ts** — Extended `studioPositionSeekReapplyRuntime` to rebuild GSAP motion timeline from `data-hf-studio-motion` attributes after each seek, including CustomEase support
- **htmlCompiler.ts** — Trigger seek-reapply script injection on `data-hf-studio-motion=` attributes
## Benefits
- No sidecar file — motion survives git, copy-paste, and manual HTML editing
- Undo/redo works via HTML source history (same as position edits)
- Renders correctly via CLI — seek-reapply script handles motion timeline rebuild
- Simpler architecture — one persistence path for all studio edits
## Test plan
- [x] `bun run build` passes
- [x] Pre-commit hooks pass (lint, format, typecheck)
- [ ] Set motion on element in Studio → `data-hf-studio-motion` attribute appears in HTML source
- [ ] Reload page → motion persists and plays correctly
- [ ] Clear motion → attribute removed, element returns to original state
- [ ] Undo/redo motion changes
- [ ] Render via CLI → motion visible in rendered video
- [ ] Seek animation → motion timeline re-syncs correctly
Add --no-open boolean flag to both commands via citty's built-in
boolean negation (--no-open sets args.open to false).
- preview.ts: guard all 4 open() calls with args.open check
- play.ts: guard the open() call with args.open check
- Default is true (open browser), preserving existing behavior
Closes#1
Co-authored-by: AnoKno <122017492+AnoKno@users.noreply.github.com>
* fix(engine): preserve video frame replacement geometry
* test(producer): cover video overlay stretch regression
* fix(engine): always pass clip to Page.captureScreenshot
Without an explicit clip, Chrome can resolve replaced-element sizing
differently at dpr=1 when full-bleed absolute videos interact with
overlay layers — producing anisotropic frame stretching on some
compositor paths. Always passing clip with scale=dpr (including 1)
ensures geometry is locked to the measured viewport dimensions.
Credit: brian-t-allen (#837)
* test(producer): regenerate style-9-prod baseline for always-clip capture path
The always-clip change in screenshotService.ts routes Chrome through a
different compositor capture path at dpr=1, producing different video
frame compression artifacts. Regenerated inside Dockerfile.test to match
CI environment.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* feat(studio): add pasteboard background to preview viewport
Adds bg-neutral-800 to the preview viewport so the area outside the
canvas is visually distinct from the composition content — consistent
with professional video editors (Premiere, DaVinci, Figma).
* feat(studio): pasteboard background and canvas outline around preview
- NLEPreview: viewport gets bg-neutral-700 (#404040) as the pasteboard
color surrounding the canvas — distinct from the app chrome (#0a0a0a)
- Player wrapper: drop bg-black so the pasteboard shows around the canvas
(loading overlays still cover the area with bg-black during load)
- Player: set host background to transparent via inline style (overrides
:host { background: #000 } in shadow DOM), and inject a style rule into
the open shadow root so .hfp-container has overflow:visible and the
canvas iframe gets a thin white ring + soft drop-shadow — making the
canvas boundary legible against the pasteboard
* feat(studio): disable manual positioning JSON by default, add toggle
Manual edits were always stored in `.hyperframes/studio-manual-edits.json`,
making it hard to share source without the sidecar file and easy to
accidentally reposition elements via drag.
Changes:
- `enabled` field added to `StudioManualEditManifest` (defaults to `false`
when absent — existing projects are unaffected until they opt in)
- Drag handles, resize, and rotation handles are hidden when disabled
- Layout X/Y/W/H/R fields in the Design panel are read-only when disabled
- "Manual positioning" toggle added at the bottom of the Design panel,
visible whether or not an element is selected
- Toggle state is persisted to `.hyperframes/studio-manual-edits.json`
so each project can opt in independently
- `STUDIO_PREVIEW_MANUAL_EDITING_ENABLED` env flag still acts as a hard
cap (env off → feature off regardless of project setting)
* feat(studio): enable manual positioning by default (opt-out)
* feat(studio): allow absolute elements to drag without toggle; gate JSON-backed drag behind toggle
* feat(studio): persist positions directly to HTML; remove JSON sidecar and manual positioning toggle
Replace the `.hyperframes/studio-manual-edits.json` sidecar with inline-style
persistence baked directly into the HTML source. Drag/resize/rotation values
are written as CSS custom properties (`--hf-studio-offset-x/y`, `--hf-studio-width/height`,
`--hf-studio-rotation`) plus `translate`/`width`/`height`/`rotate` inline styles via
`persistDomEditOperations` — no re-apply step needed on load.
Key changes:
- `sourcePatcher`: add `value: string | null` to `PatchOperation` — null removes the
property/attribute from the HTML tag instead of setting it
- `manualEditsDom`: add `build*Patches` / `buildClear*Patches` helpers that capture live
element state into `PatchOperation[]` for HTML source writes; add
`reapplyPositionEditsAfterSeek` (DOM-query-based seek hook, queries data-attribute markers)
- `manualEdits.ts`: remove `applyStudioManualEditManifest` and all manifest target
resolution; export `reapplyPositionEditsAfterSeek`; keep seek/play wrap infrastructure
- `useManifestPersistence`: remove all JSON I/O — no disk read on load, no manifest
state, no toggle state; `applyCurrentStudioManualEditsToPreview` now only installs
seek hooks via `reapplyPositionEditsAfterSeek`
- `useDomEditCommits`: replace `commitStudioManualEditManifestOptimistically` calls with
direct DOM apply + `commitPositionPatchToHtml` (queued HTML patch write, skipRefresh)
- `DomEditOverlay`: remove `manualEditsEnabled` prop; revert all `canMove || manualEditsEnabled`
gates to just `canApplyManualOffset` — every draggable element is always draggable
- `PropertyPanel`: remove `ManualPositioningToggle` component and all toggle props
- `manualEditsParsing/manualEditsTypes`: remove manifest types, upsert functions, and
`STUDIO_MANUAL_EDITS_PATH`; keep `finiteNumber`, `readStudioFileChangePath`,
`roundRotationAngle`, and snapshot/CSS-property types
* fix(studio): sync keyboard shortcut handler with main; fix keepPlaying seek assertions in test
* fix(studio): strip GSAP-cached translate from transform on path offset apply
* fix(studio): remove Reset edits button from design panel
* feat(studio): wire reloadPreview into manifest persistence; drop stale group-selection refresh
- Pass `reloadPreview` into `useManifestPersistence` so undo/redo reloads
via the refresh-key path instead of directly touching the iframe.
- Remove `refreshDomEditGroupSelectionsFromPreview` from commit handlers;
HTML is now the source of truth so no stale-ref refresh is needed.
- Add `manualEditsRenderScript` helper; export via studio-api and apply
it in `htmlCompiler` during HTML compilation.
* fix(studio): prevent root composition from being selected; correct overlay drift on resize
- Guard `getDomLayerPatchTarget` against elements with `data-composition-id`
so the root composition div is never returned as a visual selection target.
- Apply the same guard to the raw `elementFromPoint` fallback in
`getPreviewTargetFromPointer`, which was the actual escape path.
- Thread `iframeRef` into gesture handler opts; after applying draft
dimensions during resize, re-read the element BCR via `toOverlayRect`
and update the overlay box position to compensate for visual drift on
elements with centered transform-origin (e.g. GSAP scale tweens).
* fix(studio): correct resize overlay for scaled elements; block invisible element selection
- Resize: use BCR from `toOverlayRect` for both position and size after
applying draft dimensions — GSAP scale makes visual size diverge from
raw CSS size, BCR is the only accurate source during a gesture.
- Click selection: add `isElementComputedVisible` guard to the
`elementFromPoint` fallback so opacity-0 / autoAlpha-hidden elements
cannot be selected even though the browser hit-test returns them.
* fix(studio): reload preview on external file changes via SSE/HMR
Share the app-level domEditSaveTimestampRef with useManifestPersistence
so the SSE/HMR handler can suppress echoes from all studio saves (code
tab, timeline, DOM edits), then call reloadPreview() for non-motion
external changes that aren't echoes of our own saves.
* fix(studio): suppress post-resize click to keep selection on resized element
* fix(studio): serve registry blocks without index.html in preview
Blocks ship as {id}.html + assets/ with no index.html. The preview
route hard-coded index.html so these projects returned 404 and their
assets (e.g. korea-map.png, map-nyc-paris.png) were never served.
Add resolveProjectMainHtml() that falls back to {id}.html, thread the
resolved compositionPath through transformPreviewHtml and
injectStudioPreviewAugmentations, and update listProjects() in the
vite adapter to surface block directories in the project list.
* fix(render): preserve studio drag/resize/rotation offsets in rendered video
Three issues caused studio-edited positions to be lost during rendering:
1. The seek-reapply script used setInterval to wrap window.__hf.seek, but
Puppeteer's page.evaluate() calls don't yield the event loop for
macrotasks — the interval never fired, so reapplyAll() never ran after
GSAP seeks. Fix: use Object.defineProperty to trap writes to the seek
property, wrapping it synchronously the instant the bridge assigns it.
2. MEDIA_VISUAL_STYLE_PROPERTIES (copied from <video> to proxy <img>
during render) included "transform" but not "translate", "rotate", or
"scale" — the CSS Transforms Level 2 individual properties used by
studio drag/resize/rotation. The proxy was positioned at offsetLeft/
offsetTop without the translate offset.
3. getViewportMatrix (HDR compositor) only read cs.transform, missing
individual transform properties entirely. Added composeIndividualTransforms
to build the translate × rotate × scale matrix and compose it before
the legacy transform matrix.
* fix(studio): select elements with pointer-events: none in preview
Compositions often set pointer-events: none on scenes, avatar wrappers,
and decorative layers. elementsFromPoint() skips these elements entirely,
making them unselectable in the Studio. Fix: temporarily inject a
* { pointer-events: auto !important } stylesheet during hit-testing, then
remove it immediately after.
Also adds a pointer_events_none lint rule (info severity, visible with
--verbose) so authors know which selectors may affect Studio selection.
Setting an in or out point now turns on loopEnabled so the playhead
respects the marker instead of running past the out-point. Closes the
last open sub-bug of #834.
Background: PR #811 wired the work-area RAF loop to read inPoint/outPoint
but kept the loop branch gated behind loopEnabled. Default for that flag
is false, so users who set markers without first toggling the loop button
saw playback sail past the out-point (or, with the L shuttle, overshoot
by a few frames before pausing). The original spec for the feature in
issue #807 described markers as logic that "constrains the playback
engine"; the actual UX did not match that until the toggle was on.
Fix: setInPoint and setOutPoint flip loopEnabled to true when given a
non-null value. This sits next to the existing "smart setter" behavior
already in the store (setting one marker past the other nullifies the
counterpart). Clearing a marker with null preserves the current
loopEnabled, so a user who manually toggles the loop button stays in
control after that point.
Tests: full coverage for setInPoint and setOutPoint (none existed
before), including overlap nullification, non-finite rejection,
auto-enable on set, and preserve-on-clear in both directions.
Closes#834
Co-authored-by: Carlos Alcaraz <193642530+calcarazgre646@users.noreply.github.com>
Address @vanceingalls and @miguel-heygen review findings on #852:
1. Asymmetric soft-skip — only the N=1 plan+render+assemble call was
wrapped in the host-Chrome-failure catch; an SwiftShader / cold-Chrome
flake on the N=4 call would hard-fail instead of soft-skip. Factor a
local runRender() helper and wrap both calls.
2. Vacuously-passing length assertion — 'expect(framesOne.length).toBe(
framesFour.length)' passes when both runs produce 0 frames. Pin the
absolute count (EXPECTED_FRAME_COUNT = 60) so a regression that
identically truncates both renders shows red.
3. CDN version drift — anime-boundary loaded gsap@3.14.2 from jsdelivr
while every other boundary fixture loaded 3.12.2 from cdnjs. Unify on
cdnjs@3.12.2 so the next reader doesn't have to wonder why one fixture
diverges. (gsap is an empty duration-driver in all six fixtures so
the version was never load-bearing — but the divergence reads as
intentional and isn't.)
4. VIDEO_EXT type narrowing — the lookup is Record<"mp4"|"mov"|"webm">
but outputFormat includes "png-sequence". The isPngSequence ternary
short-circuits before png-sequence can reach the indexing site, but TS
can't narrow through that. Add an explicit cast at the indexing site
(not the lookup definition — over-widening to include "png-sequence":
undefined would defeat the existence guarantee).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Address findings from a three-agent code-review pass over the Phase 4 stack:
- regression-harness: hoist `readdirSync` out of the per-checkpoint
failure-extraction loop (was running 20 redundant syscalls on every
failing png-sequence test). Drop redundant `existsSync` guards before
`mkdirSync(recursive: true)` and `rmSync(force: true)`. Replace the
three-deep ternary that built the output filename suffix with a
single `Record<format, ext>` lookup.
- regression-harness-distributed: flatten the `format === "mp4" ? {...} : {...}`
branching in the `plan()` call into a single config object with a
conditional spread. `plan()` already accepts `codec: undefined` for
non-mp4 formats, so the duplicate object was unnecessary.
- chunkBoundary.test: rename the stale "byte-identical mp4" test title
to "byte-identical frames" (the test now uses png-sequence). Trim the
10-line comment justifying `rejectOnSystemFonts: false` to the
essential WHY.
- renderChunk / plan.test / regression-harness: drop trailing-edge
comment phrases that pinned the prose to the PR's calendar context
("today", "v1.5", "pre-codec-knob output", section-numbered cross-
references to the planning doc).
No behavior change. All 49 distributed unit tests pass. Smoke + four
distributed format fixtures pass in --mode=distributed-simulated.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Address @vanceingalls review on #851:
1. validateMetadata's codec/format check read 'rc.codec !== undefined &&
rc.format !== undefined && rc.format !== "mp4"'. The behavior was
correct (omitted format defaults to mp4 downstream so codec is legal)
but relied on the reader knowing that default. Normalize 'effectiveFormat
= rc.format ?? "mp4"' before the comparison so the intent reads
directly.
2. The mp4-h264-sdr sibling carries inline rationale for the no-audio
choice (AAC frame quantization extends container.duration past
nb_frames/fps and trips the harness PSNR sampler) and the chunk-seam
mapping (crossfade window 0.9-1.1s straddles frame 30). mp4-h265-sdr
stripped both. Carry them back so the two fixtures stay parallel.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
The generic parameter constraint exceeded oxfmt's line width, so the
formatter wraps the type-param list onto its own line. Applies the same
formatting locally that CI's 'Format' job would have produced via
'bun run format:check' — no behavior change.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Address @vanceingalls review on #850:
1. Unknown codec strings (typos like 'H265', future additions like 'av1')
silently fell through to libx264 in resolveEncoderTriple. Add an
explicit throw symmetric to the non-mp4-format branch already there.
A JS caller building config from JSON who passes 'codec: "h266"'
now gets a clear error at plan time instead of unflagged h264 output.
2. The preset.codec override in renderChunk had no fast unit coverage —
only the heavyweight Docker fixture in #851 would catch a regression
if someone refactored the spread (e.g. moved it into getEncoderPreset
itself). Extract resolvePresetForLockedEncoder() and add 4 fast unit
tests pinning the four encoder shapes (libx265/libx264/prores/png-seq).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
## What
Two correctness fixes to `packages/cli/src/browser/manager.ts` so the CLI picks the right Chrome binary for any perf path that depends on `chrome-headless-shell`:
1. **Also scan the puppeteer-managed cache.** `findFromCache` now reads from both `~/.cache/hyperframes/chrome` (the CLI's own managed cache) and `~/.cache/puppeteer/chrome-headless-shell/<version>/<platform-dir>/chrome-headless-shell` (the path layout that the engine's `resolveHeadlessShellPath` already reads from). When `chrome-headless-shell` is present in either cache, it now wins over system Chrome.
2. **Warn when falling through to a non-`chrome-headless-shell` system binary on Linux.** A single one-time `console.warn` explains the perf consequence and points the user at `npx @puppeteer/browsers install chrome-headless-shell`. Linux-scoped because the BeginFrame perf path is Linux-only.
## Why
Discovered in a recent spike on the BeginFrame perf path. On a clean install, the CLI's hyperframes-managed cache (`~/.cache/hyperframes/chrome`) is empty, so `findFromCache` returns `undefined`. The CLI then falls through to `findFromSystem()` and picks `/usr/bin/google-chrome`, exporting it to the engine via `PRODUCER_HEADLESS_SHELL_PATH` in `render.ts`. The engine receives that path, sees it's already set, and skips its own correct `~/.cache/puppeteer/chrome-headless-shell` scan.
Regular Chrome (147+) has dropped `HeadlessExperimental.enable`. The engine's BeginFrame probe correctly catches this and silently falls back to screenshot mode — but the operator sees no signal, so any user who installed `chrome-headless-shell` via `npx @puppeteer/browsers install` (the standard puppeteer flow) silently loses the perf path.
This is a "two codepaths know about 'the chrome we ship' but look in different places" bug. The fix collapses them.
## How
- `findFromCache` now consults both caches. Hyperframes-managed cache wins when both contain a binary (preserves existing behavior).
- New `findFromPuppeteerCache` mirrors `resolveHeadlessShellPath` from `packages/engine/src/services/browserManager.ts` — same path layout, same newest-first version sort. A comment in both files notes they need to move together if puppeteer ever changes the on-disk layout.
- `warnSystemFallbackOnce` is gated on `process.platform === "linux"` and on the binary name (`basename` of the path being `chrome-headless-shell`/`.exe`). One-shot latch so a long-running `hyperframes studio` process isn't spammed. Exported test-reset helper `_resetSystemFallbackWarnForTests` for the unit tests.
No public-API changes. `findBrowser`, `ensureBrowser`, `clearBrowser`, `setBrowserPath` all keep the same signatures.
## Test plan
- [x] Unit tests added (`packages/cli/src/browser/manager.test.ts`, 7 tests):
- cache hit on hyperframes dir
- cache hit on puppeteer dir (the new path)
- newest-version preference when multiple versions are cached
- system fallback + Linux warning emitted
- no warning when the resolved path is itself `chrome-headless-shell` (e.g. `HYPERFRAMES_BROWSER_PATH` override)
- no warning on macOS (Linux-only perf path)
- one-time warning idempotency across repeated `findBrowser()` calls
- [x] `bun run --filter @hyperframes/cli typecheck` — passes
- [x] `bun run --filter @hyperframes/cli test` — 305/305 passing
- [x] `bun run --filter @hyperframes/cli build` — passes
- [ ] Manual smoke on a Linux host with chrome-headless-shell in the puppeteer cache (skipped — sandbox already has both binaries and the unit tests cover the resolution logic deterministically; reviewers welcome to verify)
— Vai
Address @vanceingalls review on #848: the mp4-h264-sdr sibling fixture
explains the crossfade-straddles-frame-30 and continuous-rotation
chunk-seam design choices inline; mov-prores didn't. Add the parallel
comment so the next contributor reading either fixture finds the same
context. Notes specifically that ProRes is intra-only and therefore
exercises the QuickTime atom / -c copy contract rather than frame-level
state continuity.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Address @vanceingalls review on #847: the maxFrameFailures=0 byte-identity
threshold will fail when Chromium's CDP screenshot bytes or libpng's
deflate output shifts on a Docker image bump. Pin the recovery procedure
in the fixture's description so a future on-call sees 'regenerate
baselines' rather than spending time investigating a non-regression.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Address @vanceingalls review on #845: the new discoverTestSuites
dispatch was silently allowing a future tests/distributed/<x>/ fixture
to collide with an existing tests/<x>/ fixture of the same name. Both
would push under the same suite.id and stomp each other's failures/
output, baseline lookup, and CLI --filter match.
Detect the collision at discovery time and throw with both source dirs
named, so the conflict is fixable at author time. Easier to enforce now
(one fixture in the new namespace) than after the rest of the Phase 4
fixtures land.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
## Summary
- **Direct-timeline path** (GSAP compositions with `window.__timelines`): The player drives these via `DirectTimelineAdapter`, bypassing postMessage entirely. Rate changes sent `set-playback-rate` to the iframe but had no receiver — GSAP's `timeScale()` was never called. Fix: add optional `timeScale?` to `DirectTimelineAdapter` and call `this._directTimelineAdapter?.timeScale?.(rate)` in `attributeChangedCallback`. GSAP timelines expose `timeScale` natively, no composition changes required.
- **Audio-clock path** (compositions with audio): Three bugs caused `TransportClock` to always run at 1x when an audio element or WebAudio context drove the clock:
1. `schedulePlayback` was called without the `playbackRate` arg (defaulted to 1).
2. `onSetPlaybackRate` and `player.setPlaybackRate` didn't call `webAudio.setRate()`.
3. `TransportClock.attachAudioSource` divided by `this._rate` instead of `el.playbackRate`, cancelling the rate multiplier.
- Adds 2 regression tests to `clock.test.ts` covering the corrected audio-clock formula.
## Test plan
- [ ] Unit tests: `bun run --cwd packages/core test` — 861/861 pass
- [ ] Browser verification (Playwright headless, GSAP direct-timeline composition):
- 1x speed → ratio 0.972 ✓
- 2x speed → ratio 1.965 ✓
- 0.5x speed → ratio 0.490 ✓
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Two surgical changes, both isolated to the catalog-previews flow:
1. `packages/studio/src/player/hooks/usePlaybackKeyboard.test.ts`
PR #842 changed `seek()` to take `(time, { keepPlaying: true })` for the
A/E shortcuts. The keyboard-layout tests added by #839 still asserted the
single-arg form. Both landed on main without cross-checking, so `main`
itself has been failing Test/Windows since. Update the two assertions
to match the new signature. Same fix Miguel already authored on
`feat/studio-preview-pasteboard-bg`.
2. `registry/components/vignette/demo.html`
The original demo captured a frame where the vignette was at its
weakest point — the effect was nearly invisible in the static preview
used by docs. Reworked the demo so:
- The backdrop is a layered "cinematic still" (warm key + teal rim +
dark falloff) and includes a centered subject ("moon"), so the
vignette has a focal point to frame.
- Vignette starts soft (size 70%, alpha 0.35) and ramps to a dramatic
cinematic vignette (size 26%, alpha 0.92) over 1.6s.
- Peak intensity holds across t≈3.0s, which is exactly where the
catalog script samples the thumbnail (`Math.min(3.0, duration*0.6)`
with duration=5).
- Breathing motion in t=3.4–5.2s gives the video loop visible life
without disturbing the still frame.
When the user has the timeline playing and presses A (Jump to in-point)
or E (Jump to out-point), the seek seeks to the marker as expected but
also pauses the playback. The reporter (and the natural UX) expects
playback to keep going from the marker.
Root cause sits in two layers:
1. The `seek` callback in `useTimelinePlayer.ts` unconditionally calls
`setIsPlaying(false)` and `stopRAFLoop()` whenever the store reports
playing. That path is shared with timeline clicks, LayersPanel
navigation, and frame stepping — flipping the default would change
behavior the rest of the app expects.
2. `wrapTimeline` (the GSAP-timeline-backed adapter) calls `tl.pause()`
before `tl.seek(t)`, so even if the callback above stopped pausing,
GSAP-driven compositions would still get paused inside the adapter.
The fix is opt-in at both layers:
- Extend `PlaybackAdapter.seek` with `options?: { keepPlaying?: boolean }`.
Default is omitted/false, preserving existing behavior for every
caller that doesn't pass the option.
- `wrapTimeline.seek` skips the implicit `tl.pause()` when keepPlaying
is set. `createStaticSeekPlaybackAdapter` accepts the new signature
but is a no-op for the flag (it never paused internally).
- `useTimelinePlayer` seek callback grows the same option and forwards
it to adapter.seek(time, options). The reset block (stopRAFLoop,
setIsPlaying(false), shuttle refs) is gated behind !options.keepPlaying.
- Reverse shuttle is always stopped on seek (the RAF reverse tick
cannot survive a seek), so keepPlaying is overridden when the
shuttle was running backward. Documented with an inline comment.
- usePlaybackKeyboard updates its seek param type to match and passes
{ keepPlaying: true } on the A and E handlers only. Frame stepping
(Arrow keys, J/L with K held) keeps the default.
Tests (happy-dom):
- useTimelinePlayer.seek.test.ts covers the callback in three cases:
default seek clears isPlaying, seek with keepPlaying preserves
isPlaying=true, and the option from paused state stays paused.
- playbackAdapter.test.ts (new) covers wrapTimeline: default seek
pauses the GSAP timeline, keepPlaying: true skips the pause,
keepPlaying: false is the explicit default.
Closes part of #834 (sub-bug #2). Sub-bug #1 (playhead should loop
to in-point when exceeding out-point) lives in the RAF tick and is
left for a follow-up PR.
Co-authored-by: Carlos Alcaraz <193642530+calcarazgre646@users.noreply.github.com>
Miguel (approved) and Vai (commented) both flagged the same
PSNR-threshold doc/code mismatch; Vai additionally flagged a
path-anchoring bug in the projectDir-copy filter and a dishonest type
cast. Addressed all five findings:
PSNR threshold doc/code mismatch (important):
- Module docstring, `resolveMinPsnrForMode` JSDoc, and tests/README.md all
claimed distributed-simulated tightens to ≥50 dB. The actual code uses
`max(fixture.minPsnr, 10)` — 10 dB is a pathology floor, the per-test
gate is the fixture's authored `minPsnr`. Updated all three doc sites
to describe what the code does. The 50 dB target in §5.1 is a per-
render distributed-vs-in-process contract; against the frozen baseline
it's unreachable for either mode (shared encoder/JPEG jitter), so it
can't be a per-fixture gate.
`PLAN_PROJECT_DIR_COPY_SKIP` regex matched absolute paths (important):
- `cpSync` calls the filter with the absolute source path, so a
`projectDir` whose absolute path happens to contain a blocklisted
segment (`/home/user/work/output/comp/`, `~/projects/dist/foo/`, etc.)
caused the filter to return false for every descendant — empty
compiled directory, broken render. Now matches relative-to-projectDir
segments via `path.relative()` + `split(sep)`. Switched from a regex
to a Set for clarity. Harness fixtures don't hit this because they
live under `tests/<name>/src/`, but adapters call `plan()` with
caller-supplied paths.
Dishonest type cast in regression-harness.ts (important):
- `as "mp4" | "mov" | "png-sequence"` claimed reachability for formats
that `validateMetadata` doesn't accept (the schema is `"mp4" | "webm"`,
and webm is rejected by `checkDistributedSupport`). Narrowed to
hardcoded `format: "mp4"` with a comment naming the metadata-schema
invariant that lets us do that.
Renamed `chunkVideoInjectorFactory` (nit):
- The variable was invoked once and never used again — "factory" implied
repeated calls. Inlined as a plain `videoInjector: BeforeCaptureHook | null`
ternary.
Replaced tautology test (nit):
- `expect(DISTRIBUTED_SIMULATED_MIN_PSNR_DB).toBe(10)` was a value-pin
over an exported constant. The invariant the JSDoc actually asserts is
"10 dB is below any real fixture's authored minPsnr"; if someone lands
a permissive fixture (minPsnr: 5), the value-pin doesn't catch it.
Replaced with a test that walks `tests/*/meta.json` and asserts every
authored `minPsnr` is ≥ the floor.
Validated in `docker:test --mode=distributed-simulated`:
font-variant-numeric, many-cuts, gsap-letters-render-compat,
style-1-prod, sub-composition-video — all PASSED.
Unit tests: 15/15 pass (new fixture-scan test included).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code reuse:
- Move `PlanVideosJson` interface + `meta/videos.json` path constant into
`services/distributed/shared.ts`; plan.ts and renderChunk.ts import from
there instead of redeclaring the same shape with the "duplicated here so
renderChunk doesn't import from plan.ts" comment.
- Replace hand-rolled `framePattern.slice(lastIndexOf("."))` with
`extname()` from `node:path` in `rebuildExtractedFramesFromPlanDir`.
Efficiency:
- Hoist `rebuildExtractedFramesFromPlanDir` + `createFrameLookupTable` +
`createVideoFrameInjector` out of the per-chunk closure in renderChunk.
Computed once per chunk now, not once per `createRenderVideoFrameInjector`
callsite (which `runCaptureStage` may invoke multiple times).
- Add a regex filter to `cpSync(projectDir → planDir/compiled/)` so
`node_modules`, `.git`, `output/`, `failures/`, `dist/`, etc. are not
copied. Real projects can have hundreds of MB in those directories;
shipping them to S3/Lambda /tmp on every render bloats cost and time.
- Drop redundant `if (!existsSync(metaDir)) mkdirSync(metaDir, {recursive:true})`
guards; `mkdirSync({recursive:true})` is already idempotent.
Quality:
- Strip narrative comments that told the story of debugging:
- renderChunk's 30-line "Two failure modes made the call actively
harmful" block → 4-line invariant on why `discardWarmupCapture` is
omitted.
- plan.ts's "DO NOT call cleanup()" block → 3 lines naming the
invariant.
- plan.ts's pre-seed-projectDir block → 7 lines on the file-server
invariant.
- renderChunk.ts top docstring's discardWarmupCapture paragraph.
- regression-harness-distributed.ts's PSNR-drift table (belongs in
DISTRIBUTED-RENDERING-PLAN.md, not the source).
- test file's docstring about which tests live where.
- Drop the unreachable IIFE-throw on `format === "webm"` in the harness
(the support check above rejected webm); replace with a plain
`as` cast.
- Replace dynamic `await import("node:fs")` with a top-level import in
`regression-harness-distributed.ts`.
All 54 distributed unit tests still pass in Docker. Full fixture sweep
in `docker:test --mode=distributed-simulated` (font-variant-numeric,
many-cuts, gsap-letters-render-compat, style-1-prod, sub-composition-video)
all PASSED.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The chunk worker passed `createRenderVideoFrameInjector: () => null` to
`runCaptureStage`, leaving the page's `<video>` elements to decode the
source mp4 against the virtual clock. Chrome's native video pipeline
seeks ±1 frame off what the in-process renderer captures (which uses
pre-extracted frames injected as images via createVideoFrameInjector).
That ±1 frame drift produced the PSNR gap on sub-composition-video and
style-1-prod against the in-process baselines.
Two pieces:
1. `plan()` now persists the engine's `VideoElement[]` (composition.videos)
and a serialized form of `extractionResult.extracted` (videoId,
srcPath, framePattern, fps, totalFrames, metadata — paths omitted) to
`<planDir>/meta/videos.json`. This is the data renderChunk needs to
reconstruct a `FrameLookupTable` without re-running the extract stage.
2. `plan()` no longer calls `frameLookup.cleanup()` after extraction.
That cleanup was rm-rf-ing each video's outputDir, which for the
in-process orchestrator is a scratch tree the renderer owns — but for
plan() that "scratch" IS `compiledDir/__hyperframes_video_frames/<videoId>/`,
the source material that the subsequent rename moves into
`planDir/video-frames/`. Cleaning it up before the rename left
planDir/video-frames/ with only the `_downloads/` subdirectory and no
actual frame files. Both `style-1-prod` and `sub-composition-video`
reproduced this on every distributed-simulated run; both pass after
the cleanup is dropped.
3. `renderChunk` reads `meta/videos.json`, rebuilds `ExtractedFrames[]`
by re-listing `planDir/video-frames/<videoId>/` for each video, calls
`createFrameLookupTable(videos, extracted)`, and wraps the result in
`createVideoFrameInjector` — the same hook the in-process renderer
uses. The rebuilt entries set `ownedByLookup: false` so any later
cleanup() call from the engine doesn't rm the planDir bytes another
worker may still be reading.
Validated in `docker:test --mode=distributed-simulated`:
font-variant-numeric: PASSED
many-cuts: PASSED
gsap-letters-render-compat: PASSED
style-1-prod: PASSED (was: 15 frames at 26-29 dB)
sub-composition-video: PASSED (was: most frames at 21-25 dB)
In-process unchanged; 54 distributed unit tests still pass in Docker.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Validating the harness against a multi-chunk render (chunkSize=50 on
many-cuts, N=4 chunks) revealed that the previous "discard at startFrame-1
for chunk N>0" fix had a second deadlock mode: the discard's
frameTimeTicks (base + 49*interval) ended up LARGER than the captureStage
first-call's frameTimeTicks (base + 0). Chrome's compositor wedges when
asked to go backward in time as predictably as it wedges on a same-time
duplicate.
Both attempted fixes were trying to work around a problem that doesn't
exist: lastFrameCache is only consulted when Chrome returns
hasDamage=false, and every chunk frame seeks fresh DOM via __hf.seek()
before the screenshot, so hasDamage is always true and the cache is
never read. The priming step is unnecessary.
Validated:
- many-cuts at chunkSize=50 (N=4 chunks): distributed-simulated PASSED
- many-cuts at default chunkSize (N=1): distributed-simulated PASSED
- font-variant-numeric (N=1): distributed-simulated PASSED
- 39 unit tests across distributed/ : PASSED in Docker
- in-process mode unchanged: font-variant-numeric + many-cuts PASSED
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The 7 letter shortcuts (J/K/L/I/O/A/E) in usePlaybackKeyboard were gated
on `e.code === "Key*"`, which is the physical key position on a US-QWERTY
layout. On AZERTY (and other layouts) the physical "KeyA" slot produces
e.key="q", so "Jump to in-point" and the rest of the letter shortcuts
either fired on the wrong character or not at all.
Switch the 7 letter shortcuts to compare `e.key.toLowerCase()` and rename
`pressedCodesRef` → `pressedKeysRef` so the K-hold combo (K+J / K+L for
frame stepping) is also keyed off the typed character. `Space` and
`Arrow*` keep using `e.code` since those codes are layout-independent.
Adds a happy-dom test covering QWERTY happy path, AZERTY (physical KeyQ
produces e.key="a" → in-point seek fires), AZERTY contrapositive (physical
KeyA producing e.key="q" no longer triggers in-point), Shift+I clears
in-point, K-hold combo for frame stepping, K release returning the set
to clean state, and Space passthrough.
Addresses bug #3 in #834. Bugs #1 (loop at out-point) and #2 (Jump to
in-point forcing pause) live outside this hook (player loop and adapter
`seek` respectively) and are left for follow-up PRs.
Empirical investigation of --mode=distributed-simulated against many-cuts
revealed that the BeginFrame "hang" attributed earlier to a Chrome 148
SwiftShader compositor wedge was actually a renderChunk bug:
discardWarmupCapture was called with frameIndex=slice.startFrame, then
captureStage immediately captured frame 0 (relative) of the chunk's range.
For chunk 0 (slice.startFrame=0) these two calls produced the same
frameTimeTicks. Chrome's HeadlessExperimental.beginFrame deadlocks when
called twice in a row with the same frameTimeTicks — the compositor has no
new damage to advance for, and the second call hangs until the Puppeteer
protocolTimeout fires.
Tracing the chunk worker confirmed:
warmup call 1 t=0 -> ok
warmup call 60 t=1947 -> ok (loop exited)
beginFrame call #1 t=2333.33 -> returned, hasData=true, hasDamage=true
beginFrame call #2 t=2333.33 -> HANG
Fix: discardWarmupCapture skips chunk 0 (no prior frame to prime, and the
in-process renderer also has an empty cache at frame 0) and uses
slice.startFrame - 1 for chunk N>0 (the actual previous absolute frame,
which more accurately matches what the in-process renderer's cache holds
at the start of frame N).
The engine probe complications I added earlier — multi-step screenshot
test, inline data:URL pre-navigation, rastered-bytes assertion — were
chasing a phantom and are reverted to the original simple form.
chrome-headless-shell @stable on Linux with --use-angle=swiftshader
renders BeginFrame screenshots correctly after the warmup loop; what
looked like "wedged compositor" was the same frameTimeTicks deadlock
masquerading as a Chrome regression.
Also lowers the harness's distributed-simulated PSNR floor from 45 dB to
10 dB and switches to using the fixture's own minPsnr for both modes. The
45 dB floor was set against font-variant-numeric's static-content
baseline drift (~48 dB), but dynamic compositions like many-cuts produce
34-44 dB baseline drift even in-process — both renderers share the same
encoder/JPEG jitter floor, so requiring distributed to clear a tighter
threshold than in-process catches no real regression. 10 dB remains as an
absolute-pathology guard for fixtures with a permissive authored
threshold.
Validated end-to-end in `docker:test --mode=distributed-simulated`:
font-variant-numeric: PASSED (PSNR ~48 dB, audio correlation 1.000)
many-cuts: PASSED (PSNR 37-44 dB across rapid transitions)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three Phase 3 regressions surfaced when validating --mode=distributed-simulated:
engine: probeBeginFrameSupport approved chrome-headless-shell 148 even when
its SwiftShader compositor was wedged. The existing noDisplayUpdates:true
probe returns instantly on 148 and the screenshot variant returned empty
data without erroring. The real capture loop then hung on first frame with
"HeadlessExperimental.beginFrame timed out". Probe now navigates to a small
inline page (matching the real capture's compositor state, not about:blank)
and asserts that 3 back-to-back beginFrame calls each return non-empty
screenshotData. Catches the 148 soft-failure mode; falls back to
Page.captureScreenshot.
producer/plan: plan() didn't copy local assets (style.css, script.js, etc.
referenced by relative URL) into planDir/compiled/. The in-process file
server serves these from projectDir, but the distributed chunk worker's
file server only sees compiledDir. Result: every composition with external
local files rendered as unstyled HTML. Now plan() pre-seeds compiledDir
with cpSync(projectDir, ..., {dereference:true}) before compileStage
overwrites the entry HTML, so the planDir is the self-contained bundle
the docstring claims.
producer/renderChunk: force forceScreenshot:true in the chunk worker's
EngineConfig. Chrome 148's BeginFrame screenshot wedge is content-dependent
— the engine probe (now improved) catches it for some pages but not all,
and the real capture loop hangs on composition-shaped content the probe
can't simulate. Page.captureScreenshot works on every chrome-headless-shell
build we've tested, and executeRenderJob already takes this path for
multi-worker mp4, so the distributed pipeline inherits the proven Linux
reliability profile.
Also lowers the harness's distributed-simulated PSNR floor to 45 dB.
The plan's 50 dB target was written for per-render comparison; against
the frozen baseline file, the in-process renderer itself drifts ~2 dB
due to libx264/JPEG-capture jitter, so 50 dB is empirically unreachable
for either mode. 45 dB tracks the observed ~47-48 dB floor and stays
well above the 30 dB fixture threshold.
Validated:
- font-variant-numeric in distributed-simulated: PASSED (PSNR ~48 dB
across 100 checkpoints, audio correlation 1.000).
- many-cuts surfaces a fourth Phase 3 issue: timing drift on compositions
with external script src= files. First ~5 frames render the
pre-script-execution state and later variants come in ~200 ms late vs
baseline. Tracking separately — the harness mode is correctly detecting
it as a regression, which is the point.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Miguel Ángel