Commit Graph
6 Commits
Author SHA1 Message Date
Lirian Su 9a7b3efa94 fix(cli): align snapshot's local InjectFn return type with engine
`injectVideoFramesBatch` now returns `Promise<string[]>` so the caller can
filter cache entries to videos the page actually painted. The cli-side
snapshot command does not use the return value, but its local `InjectFn`
declared `Promise<void>` which made the `as { injectVideoFramesBatch:
InjectFn }` cast on the dynamic engine import fail typecheck under TS's
"sufficiently overlapping types" rule. Match the engine's actual export
shape.
2026-05-26 00:37:13 -04:00
Lirian Su f89c17fd81 fix(engine): harden ancestor-hidden video skip against mask + caller cache
Two follow-ups to the ancestor-visibility skip in `injectVideoFramesBatch`
and `syncVideoFrameVisibility`.

1. **Mask defence.** Both ancestor-hidden branches previously wrote a plain
   `img.style.visibility = "hidden"`. `applyDomLayerMask` writes the
   stylesheet rule `#${showId} *{visibility:visible !important}`, and CSS
   cascade puts important stylesheet author above non-important inline
   author — so a sub-comp host landing in the active layer's `show` set
   would revive a stale `__render_frame__` and let it bleed onto the
   layer composite. Write the hide via
   `style.setProperty("visibility", "hidden", "important")` instead;
   important inline beats important stylesheet.

2. **Caller cache hygiene.** `createVideoFrameInjector` unconditionally
   wrote `lastInjectedFrameByVideo.set(id, frameIndex)` after calling
   `injectVideoFramesBatch`, even for videos the page silently skipped due
   to a hidden visual ancestor. On the next call at the same frameIndex —
   common with source-fps < output-fps, paused source frames, or
   non-frame-aligned host starts — the cache short-circuited the second
   inject and the host's first visible frame painted blank because the
   replacement `<img>` was never created.

   Make `injectVideoFramesBatch` return `string[]` (the subset of ids it
   actually painted) and have the caller cache only those. The cli-side
   `snapshot.ts` consumer is unaffected: its local `InjectFn` types the
   return as `Promise<void>`, which is structurally compatible with
   `Promise<string[]>` under TS void-return assignment rules.

Tests: linkedom doesn't preserve `!important` in cssText, so the two new
mask-defence cases spy on the live `<img>`'s `style.setProperty` and assert
the 3-arg call shape. The cache-hygiene case stubs the page-side primitives
via `vi.mock`, drives the hook twice at the same frameIndex with a stubbed
"injected nothing" first response, and verifies the second call still
issues an inject. A counter-test pins the happy-path cache hit so a future
refactor can't trade the skip bug for a never-cache regression.
2026-05-26 00:37:13 -04:00
Lirian Su f3bb6dc125 fix(engine): narrow visibility:hidden ancestor skip to sub-comp hosts
`isVisualAncestorHidden` was treating any `visibility: hidden` ancestor as a
signal to skip injecting the replacement frame. That's too broad — for plain
`[data-start]` containers, the replacement `<img>`'s explicit
`visibility: visible` correctly overrides the ancestor per CSS spec, and
consumers rely on that to hold the final GSAP-driven frame when an authored
`data-duration` outlives the composition's GSAP timeline (e.g.
`style-9-prod`, where the runtime truncates the host to `visibility: hidden`
after the timeline ends and the replacement frame must paint through).

Restrict the `visibility: hidden` skip to ancestors that carry
`data-composition-src` or `data-composition-file` — the actual sub-composition
hosts this guard was added for. `display: none` keeps the broad behavior:
it takes the whole subtree out of layout and a child override cannot escape.

Update the existing regression suite to mark the host as a sub-composition,
and add two new cases pinning the plain-`[data-start]` behavior: both
`injectVideoFramesBatch` and `syncVideoFrameVisibility` must still produce a
visible replacement `<img>` when the host is `visibility: hidden` but does
not carry a sub-composition attribute.
2026-05-26 00:37:13 -04:00
Lirian SuandClaude Opus 4.7 68ade6609f chore(engine): drop stale fork-branch reference from test comment
The screenshotService.test.ts regression-suite comment pointed at the
author's fork branch as backstory. Strip the line so upstream code
doesn't carry a fork-relative reference; the surrounding paragraph
already explains the bug end-to-end without it.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-26 00:37:13 -04:00
Lirian SuandClaude Opus 4.7 3700cc2a16 fix(engine): skip video frame injection when a visual ancestor is hidden
`injectVideoFramesBatch` and `syncVideoFrameVisibility` iterate every
`video[data-start]` whose raw time window covers the current seek.
Inner `<video>` elements inside `[data-composition-src]`
sub-compositions get `data-start="0"` auto-injected by
`compileTimingAttrs` and probed-duration cover the entire timeline,
so they look "active" even when their host has not yet started.

When the runtime then hides the host with `visibility: hidden` (its
out-of-window lifecycle), the inner video inherits hidden via the CSS
cascade — but our injector responded by painting a replacement
`<img class="__render_frame__" style="visibility: visible">` next to
the video. `visibility: visible` on the descendant defeats the parent
`visibility: hidden` cascade, and because the host has not been
morphed by GSAP yet the video's bounding box is its CSS default
(usually full-bleed). The result is one full-bleed frame per inactive
sub-comp painted over whichever moment is *actually* visible — the
overlay symptom the upstream agentic-finecut project saw.

Walk ancestors in both functions; if any has `display: none` or
`visibility: hidden`, skip the inject and hide any stale
`__render_frame__` sibling. The render is now correctly empty for
hidden hosts, which is what the surrounding CSS cascade already
intends.

Tests:
- `screenshotService.test.ts`: cover the new guard for both
  visibility:hidden and display:none hosts, both for the fresh-img and
  the stale-img paths, plus `syncVideoFrameVisibility` for the case
  where the time window calls a video "active" but a hidden ancestor
  still requires its frame to stay hidden. Each test fails against
  pre-fix `screenshotService.ts`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-26 00:37:13 -04:00
Lirian SuandClaude Opus 4.7 69b7965446 fix(core): preserve replace-pattern characters in bundled runtime IIFE
`injectInterceptor` used `String.prototype.replace(target, replacement)`
to inject the runtime `<script>` before `</head>`. The replacement
string is a substitution template — `$&` expands to the matched
substring, and the minified runtime IIFE contains legitimate `$&`
sequences (e.g. `if(te&&$&!y.hasAttribute(...))`), so every `$&` in
the body was silently rewritten to `</head>`, producing
`Unexpected token '<'` SyntaxErrors and breaking every timeline in
the bundle.

Switch to the function-replacer form so the runtime body is passed
through verbatim. Add a regression test that diffs the bundled
runtime body against `getHyperframeRuntimeScript()` and asserts only
one `</head>` survives in the document — the test exercises the
`<head>`-present injection path (the only branch that uses the
substitution template; the no-`<head>` fallback uses slice+concat
and was unaffected).

Only the bundler is affected — `producer/fileServer.ts` already uses
the function form via `injectScriptsIntoHtml` in
`htmlDocument.ts`, so render output was correct. Snapshot, preview,
studio, layout, and validate all consume `bundleToSingleHtml` and
were broken before this fix.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-22 15:06:11 +08:00