From a89e3224b45d6e7fd72f270c9dc41f04b980dc4b Mon Sep 17 00:00:00 2001
From: Lirian Su <liriansu@gmail.com>
Date: Fri, 22 May 2026 15:11:27 +0800
Subject: [PATCH 1/5] fix(engine): skip video frame injection when a visual
 ancestor is hidden
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`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>
---
 .../src/services/screenshotService.test.ts    | 193 ++++++++++++++++++
 .../engine/src/services/screenshotService.ts  |  51 ++++-
 2 files changed, 240 insertions(+), 4 deletions(-)
diff --git a/packages/engine/src/services/screenshotService.test.ts b/packages/engine/src/services/screenshotService.test.ts
index 62fc4b740..94b244a93 100644
--- a/packages/engine/src/services/screenshotService.test.ts
+++ b/packages/engine/src/services/screenshotService.test.ts
@@ -6,6 +6,7 @@ import {
   pageScreenshotCapture,
   cdpSessionCache,
   injectVideoFramesBatch,
+  syncVideoFrameVisibility,
 } from "./screenshotService.js";
 
 // Stub a Page + CDPSession just enough that pageScreenshotCapture can call
@@ -191,3 +192,195 @@ describe("injectVideoFramesBatch replacement layout", () => {
     expect(img?.style.inset).toBe("auto");
   });
 });
+
+describe("video-frame injection respects ancestor visibility", () => {
+  // Regression guard: the runtime's `[data-start]` lifecycle hides
+  // out-of-window sub-composition hosts with `visibility:hidden`, but the
+  // injector used to ignore that and paint a replacement <img> for every
+  // active `<video data-start>` element. Inner-PIP videos inside *other*
+  // moments still appear active in the raw time-window check (their auto-
+  // injected `data-start="0"` + probed full-source duration cover the
+  // whole timeline), so the bug produced one full-bleed speaker overlay
+  // per inactive sub-comp — covering whichever moment was actually visible.
+  // See: https://github.com/lirian-su-opus/hyperframes branch issue thread.
+
+  type StyleLike = {
+    display?: string;
+    visibility?: string;
+    opacity?: string;
+    objectFit?: string;
+    objectPosition?: string;
+    zIndex?: string;
+  };
+
+  function setupHostHiddenScenario(hostStyle: StyleLike) {
+    const { window, document } = parseHTML(
+      '<html><body><div id="host"><div id="pip-frame"><video id="pip" data-start="0" data-duration="10"></video></div></div></body></html>',
+    );
+
+    Object.defineProperty(window.HTMLImageElement.prototype, "decode", {
+      configurable: true,
+      value: () => Promise.resolve(),
+    });
+
+    const host = document.getElementById("host") as HTMLElement;
+    const pipFrame = document.getElementById("pip-frame") as HTMLElement;
+    const video = document.getElementById("pip") as HTMLVideoElement;
+
+    Object.defineProperties(video, {
+      offsetLeft: { configurable: true, get: () => 0 },
+      offsetTop: { configurable: true, get: () => 0 },
+      offsetWidth: { configurable: true, get: () => 1080 },
+      offsetHeight: { configurable: true, get: () => 1920 },
+    });
+    video.getBoundingClientRect = () =>
+      ({
+        x: 0,
+        y: 0,
+        left: 0,
+        top: 0,
+        right: 1080,
+        bottom: 1920,
+        width: 1080,
+        height: 1920,
+        toJSON: () => ({}),
+      }) as DOMRect;
+
+    const styles = new Map<Element, StyleLike>();
+    styles.set(host, hostStyle);
+    styles.set(pipFrame, {});
+    styles.set(video, { opacity: "1", objectFit: "cover", objectPosition: "center", zIndex: "1" });
+
+    Object.defineProperty(window, "getComputedStyle", {
+      configurable: true,
+      value: (el: Element) => {
+        const declared = styles.get(el) ?? {};
+        return {
+          display: declared.display ?? "block",
+          visibility: declared.visibility ?? "visible",
+          opacity: declared.opacity ?? "1",
+          objectFit: declared.objectFit ?? "fill",
+          objectPosition: declared.objectPosition ?? "50% 50%",
+          zIndex: declared.zIndex ?? "auto",
+          getPropertyValue: (prop: string) => {
+            const camel = prop.replace(/-([a-z])/g, (_, c: string) =>
+              c.toUpperCase(),
+            ) as keyof StyleLike;
+            return declared[camel] ?? "";
+          },
+        };
+      },
+    });
+
+    return { window, document, video, host, pipFrame };
+  }
+
+  function withGlobals<T extends { window: Window; document: Document; video: HTMLVideoElement }>(
+    setup: T,
+  ): { teardown: () => void; setup: T } {
+    const globals = globalThis as unknown as { window?: Window; document?: Document };
+    const previousWindow = globals.window;
+    const previousDocument = globals.document;
+    globals.window = setup.window;
+    globals.document = setup.document;
+    return {
+      setup,
+      teardown: () => {
+        globals.window = previousWindow;
+        globals.document = previousDocument;
+      },
+    };
+  }
+
+  function passthroughPage(): Page {
+    return {
+      evaluate: async (fn: (...args: unknown[]) => unknown, ...args: unknown[]) =>
+        // The implementation is built to run inside the page sandbox via
+        // `page.evaluate`, but linkedom gives us a DOM compatible enough to
+        // execute the function body directly in Node.
+        Promise.resolve((fn as (...a: unknown[]) => unknown)(...args)),
+    } as unknown as Page;
+  }
+
+  it("skips replacement-frame creation when the video's host has visibility:hidden", async () => {
+    const { teardown, setup } = withGlobals(setupHostHiddenScenario({ visibility: "hidden" }));
+    try {
+      await injectVideoFramesBatch(passthroughPage(), [
+        {
+          videoId: "pip",
+          dataUri:
+            "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkAAIAAAoAAv/lxKUAAAAASUVORK5CYII=",
+        },
+      ]);
+    } finally {
+      teardown();
+    }
+
+    // No replacement <img> should be injected next to the video — the host is
+    // currently hidden, so painting a frame over it would bleed onto whichever
+    // sibling host is actually visible on this seek.
+    const sibling = setup.video.nextElementSibling as HTMLElement | null;
+    expect(sibling).toBeNull();
+  });
+
+  it("skips replacement-frame creation when the video's host has display:none", async () => {
+    const { teardown, setup } = withGlobals(setupHostHiddenScenario({ display: "none" }));
+    try {
+      await injectVideoFramesBatch(passthroughPage(), [
+        {
+          videoId: "pip",
+          dataUri:
+            "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkAAIAAAoAAv/lxKUAAAAASUVORK5CYII=",
+        },
+      ]);
+    } finally {
+      teardown();
+    }
+
+    const sibling = setup.video.nextElementSibling as HTMLElement | null;
+    expect(sibling).toBeNull();
+  });
+
+  it("hides an existing replacement <img> when the host becomes visibility:hidden", async () => {
+    // First seed an existing __render_frame__ <img> next to the video (the
+    // state the page is in after a previous seek when the host was visible).
+    const { teardown, setup } = withGlobals(setupHostHiddenScenario({ visibility: "hidden" }));
+    const seededImg = setup.document.createElement("img");
+    seededImg.classList.add("__render_frame__");
+    seededImg.style.visibility = "visible";
+    setup.video.parentNode?.insertBefore(seededImg, setup.video.nextSibling);
+
+    try {
+      await injectVideoFramesBatch(passthroughPage(), [
+        {
+          videoId: "pip",
+          dataUri:
+            "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkAAIAAAoAAv/lxKUAAAAASUVORK5CYII=",
+        },
+      ]);
+    } finally {
+      teardown();
+    }
+
+    expect(seededImg.style.visibility).toBe("hidden");
+  });
+
+  it("syncVideoFrameVisibility hides the replacement <img> for ancestor-hidden actives", async () => {
+    const { teardown, setup } = withGlobals(setupHostHiddenScenario({ visibility: "hidden" }));
+    const seededImg = setup.document.createElement("img");
+    seededImg.classList.add("__render_frame__");
+    seededImg.style.visibility = "visible";
+    setup.video.parentNode?.insertBefore(seededImg, setup.video.nextSibling);
+
+    try {
+      // "pip" IS in the active set (per the raw time-window check) but the
+      // host is hidden. sync must keep the <img> hidden, not flip it to
+      // `visibility: visible`.
+      await syncVideoFrameVisibility(passthroughPage(), ["pip"]);
+    } finally {
+      teardown();
+    }
+
+    expect(seededImg.style.visibility).toBe("hidden");
+  });
+});
diff --git a/packages/engine/src/services/screenshotService.ts b/packages/engine/src/services/screenshotService.ts
index 712563a64..2219c36f6 100644
--- a/packages/engine/src/services/screenshotService.ts
+++ b/packages/engine/src/services/screenshotService.ts
@@ -386,12 +386,39 @@ export async function injectVideoFramesBatch(
         "bottom",
         "inset",
       ]);
+      // Walk ancestors looking for a host that the page has hidden via
+      // `display:none` or `visibility:hidden`. The runtime hides
+      // `[data-composition-src]` and `[data-start]` hosts that fall outside
+      // their time window using exactly these properties; a nested
+      // `<video data-start>` inside such a host still appears "active" in the
+      // raw time-window check (its own `data-start`/`data-end` cover the
+      // whole clip), so without this guard we would paint a full-bleed
+      // replacement frame over a sibling host that *is* visible.
+      const isVisualAncestorHidden = (el: HTMLElement): boolean => {
+        let parent = el.parentElement;
+        while (parent !== null && parent !== document.documentElement) {
+          const computed = window.getComputedStyle(parent);
+          if (computed.display === "none" || computed.visibility === "hidden") return true;
+          parent = parent.parentElement;
+        }
+        return false;
+      };
       for (const item of items) {
         const video = document.getElementById(item.videoId) as HTMLVideoElement | null;
         if (!video) continue;
 
         let img = video.nextElementSibling as HTMLImageElement | null;
-        const isNewImage = !img || !img.classList.contains("__render_frame__");
+        const hasImg = img !== null && img.classList.contains("__render_frame__");
+
+        if (isVisualAncestorHidden(video)) {
+          // Don't paint a frame over a hidden host — if an existing replacement
+          // <img> is still around from when the host was visible, hide it so it
+          // doesn't bleed through a sibling host that *is* visible on this seek.
+          if (hasImg && img) img.style.visibility = "hidden";
+          continue;
+        }
+
+        const isNewImage = !hasImg;
         const computedStyle = window.getComputedStyle(video);
         // Read the GSAP-controlled opacity directly from the native <video>.
         // We hide the <video> below with `visibility: hidden` only (never
@@ -489,12 +516,28 @@ export async function syncVideoFrameVisibility(
   activeVideoIds: string[],
 ): Promise<void> {
   await page.evaluate((ids: string[]) => {
+    // Mirror the ancestor-visibility guard from `injectVideoFramesBatch`: a
+    // video whose host is `display:none` / `visibility:hidden` (e.g., a
+    // sub-composition that the runtime has marked out-of-window) must not
+    // have its replacement <img> reach `visibility:visible` here, otherwise
+    // it would paint through the hidden host onto whichever sibling host is
+    // currently visible.
+    const isVisualAncestorHidden = (el: HTMLElement): boolean => {
+      let parent = el.parentElement;
+      while (parent !== null && parent !== document.documentElement) {
+        const computed = window.getComputedStyle(parent);
+        if (computed.display === "none" || computed.visibility === "hidden") return true;
+        parent = parent.parentElement;
+      }
+      return false;
+    };
     const active = new Set(ids);
     const videos = Array.from(document.querySelectorAll("video[data-start]")) as HTMLVideoElement[];
     for (const video of videos) {
       const img = video.nextElementSibling as HTMLElement | null;
       const hasImg = img && img.classList.contains("__render_frame__");
-      if (active.has(video.id)) {
+      const ancestorHidden = isVisualAncestorHidden(video);
+      if (active.has(video.id) && !ancestorHidden) {
         // Active video: show injected <img>, hide native <video>.
         // Do NOT clobber inline opacity here — GSAP-controlled opacity must
         // survive until injectVideoFramesBatch reads it via getComputedStyle.
@@ -506,8 +549,8 @@ export async function syncVideoFrameVisibility(
           img.style.visibility = "visible";
         }
       } else {
-        // Inactive video: hide both. Use visibility only (never opacity) so we
-        // never clobber GSAP-controlled inline opacity.
+        // Inactive (or ancestor-hidden) video: hide both. Use visibility only
+        // (never opacity) so we never clobber GSAP-controlled inline opacity.
         video.style.removeProperty("display");
         video.style.setProperty("visibility", "hidden", "important");
         video.style.setProperty("pointer-events", "none", "important");

From a6e09e8a433fee071cc1b7cbe35917e8c61efcb6 Mon Sep 17 00:00:00 2001
From: Lirian Su <liriansu@gmail.com>
Date: Fri, 22 May 2026 15:30:15 +0800
Subject: [PATCH 2/5] chore(engine): drop stale fork-branch reference from test
 comment
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 packages/engine/src/services/screenshotService.test.ts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/packages/engine/src/services/screenshotService.test.ts b/packages/engine/src/services/screenshotService.test.ts
index 94b244a93..9fb4a2416 100644
--- a/packages/engine/src/services/screenshotService.test.ts
+++ b/packages/engine/src/services/screenshotService.test.ts
@@ -202,7 +202,6 @@ describe("video-frame injection respects ancestor visibility", () => {
   // injected `data-start="0"` + probed full-source duration cover the
   // whole timeline), so the bug produced one full-bleed speaker overlay
   // per inactive sub-comp — covering whichever moment was actually visible.
-  // See: https://github.com/lirian-su-opus/hyperframes branch issue thread.
 
   type StyleLike = {
     display?: string;

From 1381bfa547959b7f0c48f58907a61aed85460e37 Mon Sep 17 00:00:00 2001
From: Lirian Su <liriansu@gmail.com>
Date: Mon, 25 May 2026 10:45:50 +0800
Subject: [PATCH 3/5] fix(engine): narrow visibility:hidden ancestor skip to
 sub-comp hosts
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`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.
---
 .../src/services/screenshotService.test.ts    | 75 ++++++++++++++++++-
 .../engine/src/services/screenshotService.ts  | 58 ++++++++++----
 2 files changed, 115 insertions(+), 18 deletions(-)

diff --git a/packages/engine/src/services/screenshotService.test.ts b/packages/engine/src/services/screenshotService.test.ts
index 9fb4a2416..e6f96d4c6 100644
--- a/packages/engine/src/services/screenshotService.test.ts
+++ b/packages/engine/src/services/screenshotService.test.ts
@@ -202,6 +202,17 @@ describe("video-frame injection respects ancestor visibility", () => {
   // injected `data-start="0"` + probed full-source duration cover the
   // whole timeline), so the bug produced one full-bleed speaker overlay
   // per inactive sub-comp — covering whichever moment was actually visible.
+  //
+  // The skip is intentionally narrow: `visibility:hidden` on a regular
+  // `[data-start]` container must NOT skip injection, because the
+  // replacement <img>'s explicit `visibility:visible` overrides the
+  // ancestor (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. We therefore only treat
+  // `visibility:hidden` as a skip signal on sub-composition hosts
+  // (`[data-composition-src]` / `[data-composition-file]`). `display:none`,
+  // by contrast, takes the whole subtree out of layout regardless of any
+  // child override, so it always triggers the skip.
 
   type StyleLike = {
     display?: string;
@@ -212,9 +223,19 @@ describe("video-frame injection respects ancestor visibility", () => {
     zIndex?: string;
   };
 
-  function setupHostHiddenScenario(hostStyle: StyleLike) {
+  type HostAttribute = "data-composition-src" | "data-composition-file" | "data-start";
+
+  function setupHostHiddenScenario(
+    hostStyle: StyleLike,
+    options: { hostAttribute?: HostAttribute } = {},
+  ) {
+    const hostAttribute = options.hostAttribute ?? "data-composition-src";
+    const hostAttrMarkup =
+      hostAttribute === "data-start"
+        ? 'data-start="0" data-duration="10"'
+        : `${hostAttribute}="sub.html"`;
     const { window, document } = parseHTML(
-      '<html><body><div id="host"><div id="pip-frame"><video id="pip" data-start="0" data-duration="10"></video></div></div></body></html>',
+      `<html><body><div id="host" ${hostAttrMarkup}><div id="pip-frame"><video id="pip" data-start="0" data-duration="10"></video></div></div></body></html>`,
     );
 
     Object.defineProperty(window.HTMLImageElement.prototype, "decode", {
@@ -382,4 +403,54 @@ describe("video-frame injection respects ancestor visibility", () => {
 
     expect(seededImg.style.visibility).toBe("hidden");
   });
+
+  it("still injects when a plain [data-start] host is visibility:hidden (CSS-escapable)", async () => {
+    // Regression guard for the style-9-prod symptom: a regular
+    // `[data-start]` container whose GSAP timeline is shorter than its
+    // authored `data-duration` ends up `visibility: hidden` past the
+    // timeline end. The replacement <img>'s explicit `visibility: visible`
+    // correctly overrides that per CSS spec, so the injector must NOT
+    // short-circuit — it would otherwise drop the final-state frame and
+    // produce blank tail frames.
+    const { teardown, setup } = withGlobals(
+      setupHostHiddenScenario({ visibility: "hidden" }, { hostAttribute: "data-start" }),
+    );
+
+    try {
+      await injectVideoFramesBatch(passthroughPage(), [
+        {
+          videoId: "pip",
+          dataUri:
+            "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkAAIAAAoAAv/lxKUAAAAASUVORK5CYII=",
+        },
+      ]);
+    } finally {
+      teardown();
+    }
+
+    const sibling = setup.video.nextElementSibling as HTMLElement | null;
+    expect(sibling).not.toBeNull();
+    expect(sibling?.classList.contains("__render_frame__")).toBe(true);
+    expect(sibling?.style.visibility).toBe("visible");
+  });
+
+  it("syncVideoFrameVisibility shows the replacement <img> when a plain [data-start] host is visibility:hidden", async () => {
+    const { teardown, setup } = withGlobals(
+      setupHostHiddenScenario({ visibility: "hidden" }, { hostAttribute: "data-start" }),
+    );
+    const seededImg = setup.document.createElement("img");
+    seededImg.classList.add("__render_frame__");
+    seededImg.style.visibility = "hidden";
+    setup.video.parentNode?.insertBefore(seededImg, setup.video.nextSibling);
+
+    try {
+      await syncVideoFrameVisibility(passthroughPage(), ["pip"]);
+    } finally {
+      teardown();
+    }
+
+    // The host's `visibility: hidden` is escapable; sync must flip the
+    // <img> to `visibility: visible` so it overrides the ancestor.
+    expect(seededImg.style.visibility).toBe("visible");
+  });
 });
diff --git a/packages/engine/src/services/screenshotService.ts b/packages/engine/src/services/screenshotService.ts
index 2219c36f6..a73423f87 100644
--- a/packages/engine/src/services/screenshotService.ts
+++ b/packages/engine/src/services/screenshotService.ts
@@ -386,19 +386,40 @@ export async function injectVideoFramesBatch(
         "bottom",
         "inset",
       ]);
-      // Walk ancestors looking for a host that the page has hidden via
-      // `display:none` or `visibility:hidden`. The runtime hides
-      // `[data-composition-src]` and `[data-start]` hosts that fall outside
-      // their time window using exactly these properties; a nested
-      // `<video data-start>` inside such a host still appears "active" in the
-      // raw time-window check (its own `data-start`/`data-end` cover the
-      // whole clip), so without this guard we would paint a full-bleed
-      // replacement frame over a sibling host that *is* visible.
+      // Walk ancestors looking for a host that the page has hidden. The
+      // runtime hides `[data-composition-src]` and `[data-start]` hosts that
+      // fall outside their time window; a nested `<video data-start>` inside
+      // such a host still appears "active" in the raw time-window check (its
+      // own `data-start`/`data-end` cover the whole clip), so without this
+      // guard we would paint a full-bleed replacement frame over a sibling
+      // host that *is* visible.
+      //
+      // `display: none` is always a skip signal — a `display: none` ancestor
+      // takes its whole subtree out of layout, and a child `<img>` cannot
+      // escape that. `visibility: hidden`, by contrast, is escapable: a
+      // descendant with `visibility: visible` overrides an ancestor's
+      // `visibility: hidden` per the CSS spec, and the replacement `<img>`
+      // intentionally sets `visibility: visible`. We therefore only treat
+      // `visibility: hidden` as a skip signal on sub-composition hosts
+      // (`[data-composition-src]` / `[data-composition-file]`), which is the
+      // scenario this guard exists for. Plain `[data-start]` containers may
+      // be hidden with `visibility: hidden` while still wanting their inner
+      // video's final-state frame to paint through (e.g. a GSAP timeline
+      // shorter than the host's authored data-duration, where the runtime
+      // truncates visibility but the replacement <img> must hold its last
+      // frame) — those must NOT be skipped here.
       const isVisualAncestorHidden = (el: HTMLElement): boolean => {
         let parent = el.parentElement;
         while (parent !== null && parent !== document.documentElement) {
           const computed = window.getComputedStyle(parent);
-          if (computed.display === "none" || computed.visibility === "hidden") return true;
+          if (computed.display === "none") return true;
+          if (
+            computed.visibility === "hidden" &&
+            (parent.hasAttribute("data-composition-src") ||
+              parent.hasAttribute("data-composition-file"))
+          ) {
+            return true;
+          }
           parent = parent.parentElement;
         }
         return false;
@@ -516,17 +537,22 @@ export async function syncVideoFrameVisibility(
   activeVideoIds: string[],
 ): Promise<void> {
   await page.evaluate((ids: string[]) => {
-    // Mirror the ancestor-visibility guard from `injectVideoFramesBatch`: a
-    // video whose host is `display:none` / `visibility:hidden` (e.g., a
-    // sub-composition that the runtime has marked out-of-window) must not
-    // have its replacement <img> reach `visibility:visible` here, otherwise
-    // it would paint through the hidden host onto whichever sibling host is
-    // currently visible.
+    // Mirror the ancestor-visibility guard from `injectVideoFramesBatch`.
+    // See that copy for the full rationale on why `visibility: hidden` is
+    // narrowed to sub-composition hosts only — keep these two functions in
+    // sync so the inactive-arm decision matches the inject-time decision.
     const isVisualAncestorHidden = (el: HTMLElement): boolean => {
       let parent = el.parentElement;
       while (parent !== null && parent !== document.documentElement) {
         const computed = window.getComputedStyle(parent);
-        if (computed.display === "none" || computed.visibility === "hidden") return true;
+        if (computed.display === "none") return true;
+        if (
+          computed.visibility === "hidden" &&
+          (parent.hasAttribute("data-composition-src") ||
+            parent.hasAttribute("data-composition-file"))
+        ) {
+          return true;
+        }
         parent = parent.parentElement;
       }
       return false;

From 93ddea41f539b1a9232d1c7a5b9787887ae6039c Mon Sep 17 00:00:00 2001
From: Lirian Su <liriansu@gmail.com>
Date: Mon, 25 May 2026 12:56:01 +0800
Subject: [PATCH 4/5] fix(engine): harden ancestor-hidden video skip against
 mask + caller cache
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 .../src/services/screenshotService.test.ts    |  54 +++++++++
 .../engine/src/services/screenshotService.ts  |  32 ++++-
 .../src/services/videoFrameInjector.test.ts   | 113 +++++++++++++++++-
 .../engine/src/services/videoFrameInjector.ts |  18 ++-
 4 files changed, 206 insertions(+), 11 deletions(-)

diff --git a/packages/engine/src/services/screenshotService.test.ts b/packages/engine/src/services/screenshotService.test.ts
index e6f96d4c6..4332c359d 100644
--- a/packages/engine/src/services/screenshotService.test.ts
+++ b/packages/engine/src/services/screenshotService.test.ts
@@ -453,4 +453,58 @@ describe("video-frame injection respects ancestor visibility", () => {
     // <img> to `visibility: visible` so it overrides the ancestor.
     expect(seededImg.style.visibility).toBe("visible");
   });
+
+  // Regression for the layered/HDR mask path: `applyDomLayerMask` writes an
+  // `!important` stylesheet rule `#${showId} *{visibility:visible !important}`
+  // which, if a sub-comp host id appears in the show set, would revive a
+  // plain (non-important) inline `visibility: hidden` on a descendant
+  // `__render_frame__` — the cascade rule is "important stylesheet author
+  // beats non-important inline author". To stay safe regardless of which
+  // layer ends up in `show`, the ancestor-hidden hide must be written with
+  // `!important` so inline `!important` beats stylesheet `!important`.
+  //
+  // linkedom strips `!important` from `cssText`/`getPropertyPriority`, so we
+  // pin the contract on the API call site instead: a `setProperty(name,
+  // value, "important")` invocation on the live `<img>`'s style.
+  it("injectVideoFramesBatch hides a stale <img> with !important so the layer mask cannot revive it", async () => {
+    const { teardown, setup } = withGlobals(setupHostHiddenScenario({ visibility: "hidden" }));
+    const seededImg = setup.document.createElement("img");
+    seededImg.classList.add("__render_frame__");
+    seededImg.style.visibility = "visible";
+    setup.video.parentNode?.insertBefore(seededImg, setup.video.nextSibling);
+    const setPropertySpy = vi.spyOn(seededImg.style, "setProperty");
+
+    try {
+      await injectVideoFramesBatch(passthroughPage(), [
+        {
+          videoId: "pip",
+          dataUri:
+            "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkAAIAAAoAAv/lxKUAAAAASUVORK5CYII=",
+        },
+      ]);
+    } finally {
+      teardown();
+    }
+
+    expect(seededImg.style.visibility).toBe("hidden");
+    expect(setPropertySpy).toHaveBeenCalledWith("visibility", "hidden", "important");
+  });
+
+  it("syncVideoFrameVisibility hides an existing <img> with !important so the layer mask cannot revive it", async () => {
+    const { teardown, setup } = withGlobals(setupHostHiddenScenario({ visibility: "hidden" }));
+    const seededImg = setup.document.createElement("img");
+    seededImg.classList.add("__render_frame__");
+    seededImg.style.visibility = "visible";
+    setup.video.parentNode?.insertBefore(seededImg, setup.video.nextSibling);
+    const setPropertySpy = vi.spyOn(seededImg.style, "setProperty");
+
+    try {
+      await syncVideoFrameVisibility(passthroughPage(), ["pip"]);
+    } finally {
+      teardown();
+    }
+
+    expect(seededImg.style.visibility).toBe("hidden");
+    expect(setPropertySpy).toHaveBeenCalledWith("visibility", "hidden", "important");
+  });
 });
diff --git a/packages/engine/src/services/screenshotService.ts b/packages/engine/src/services/screenshotService.ts
index a73423f87..7af0481ad 100644
--- a/packages/engine/src/services/screenshotService.ts
+++ b/packages/engine/src/services/screenshotService.ts
@@ -369,13 +369,22 @@ export async function removeDomLayerMask(page: Page, extraHideIds: string[]): Pr
   );
 }
 
+/**
+ * Returns the subset of `updates.videoId`s that were actually painted in
+ * this call. Videos skipped because of a hidden visual ancestor are NOT
+ * included — the caller relies on this to avoid recording a `lastInjected`
+ * cache entry for a frame that never reached the page, which would otherwise
+ * short-circuit the next inject at the same frameIndex and leave the host's
+ * first visible frame blank.
+ */
 export async function injectVideoFramesBatch(
   page: Page,
   updates: Array<{ videoId: string; dataUri: string }>,
-): Promise<void> {
-  if (updates.length === 0) return;
-  await page.evaluate(
+): Promise<string[]> {
+  if (updates.length === 0) return [];
+  return await page.evaluate(
     async (items: Array<{ videoId: string; dataUri: string }>, visualProperties: string[]) => {
+      const injectedIds: string[] = [];
       const pendingDecodes: Array<Promise<void>> = [];
       const replacementLayoutProperties = new Set([
         "width",
@@ -435,7 +444,13 @@ export async function injectVideoFramesBatch(
           // Don't paint a frame over a hidden host — if an existing replacement
           // <img> is still around from when the host was visible, hide it so it
           // doesn't bleed through a sibling host that *is* visible on this seek.
-          if (hasImg && img) img.style.visibility = "hidden";
+          //
+          // Use `!important` so the inline hide survives `applyDomLayerMask`'s
+          // stylesheet `#${showId} *{visibility:visible !important}` when the
+          // sub-comp host happens to land in the active layer's `show` set —
+          // important stylesheet beats non-important inline, but important
+          // inline beats important stylesheet.
+          if (hasImg && img) img.style.setProperty("visibility", "hidden", "important");
           continue;
         }
 
@@ -522,10 +537,12 @@ export async function injectVideoFramesBatch(
         // GSAP-controlled value.
         video.style.setProperty("visibility", "hidden", "important");
         video.style.setProperty("pointer-events", "none", "important");
+        injectedIds.push(item.videoId);
       }
       if (pendingDecodes.length > 0) {
         await Promise.all(pendingDecodes);
       }
+      return injectedIds;
     },
     updates,
     [...MEDIA_VISUAL_STYLE_PROPERTIES],
@@ -577,11 +594,16 @@ export async function syncVideoFrameVisibility(
       } else {
         // Inactive (or ancestor-hidden) video: hide both. Use visibility only
         // (never opacity) so we never clobber GSAP-controlled inline opacity.
+        // Use `!important` on the <img> hide so `applyDomLayerMask`'s
+        // important stylesheet rule (`#${showId} *{visibility:visible !important}`)
+        // cannot revive a stale frame when the sub-comp host lands in the
+        // active layer's `show` set — same mask-defense reasoning as the
+        // `isVisualAncestorHidden` branch in `injectVideoFramesBatch`.
         video.style.removeProperty("display");
         video.style.setProperty("visibility", "hidden", "important");
         video.style.setProperty("pointer-events", "none", "important");
         if (hasImg) {
-          img.style.visibility = "hidden";
+          img.style.setProperty("visibility", "hidden", "important");
         }
       }
     }
diff --git a/packages/engine/src/services/videoFrameInjector.test.ts b/packages/engine/src/services/videoFrameInjector.test.ts
index 28c813641..5a1c793b1 100644
--- a/packages/engine/src/services/videoFrameInjector.test.ts
+++ b/packages/engine/src/services/videoFrameInjector.test.ts
@@ -1,9 +1,30 @@
 // @vitest-environment node
-import { describe, it, expect, beforeEach, afterEach } from "vitest";
+import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
 import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
-import { __testing } from "./videoFrameInjector.js";
+import { type Page } from "puppeteer-core";
+
+// Hoist mocks before importing the module under test so the mock factory wins.
+// The cache-hygiene block exercises createVideoFrameInjector against stubbed
+// page-side primitives so we can assert on Node-side state (cache poisoning)
+// without standing up a real browser.
+const { injectVideoFramesBatchMock, syncVideoFrameVisibilityMock } = vi.hoisted(() => ({
+  injectVideoFramesBatchMock: vi.fn<
+    (page: Page, updates: Array<{ videoId: string; dataUri: string }>) => Promise<string[]>
+  >(async (_page, updates) => updates.map((u) => u.videoId)),
+  syncVideoFrameVisibilityMock: vi.fn<(page: Page, ids: string[]) => Promise<void>>(
+    async () => undefined,
+  ),
+}));
+
+vi.mock("./screenshotService.js", () => ({
+  injectVideoFramesBatch: injectVideoFramesBatchMock,
+  syncVideoFrameVisibility: syncVideoFrameVisibilityMock,
+}));
+
+import { __testing, createVideoFrameInjector } from "./videoFrameInjector.js";
+import { type FrameLookupTable } from "./videoFrameExtractor.js";
 import { DEFAULT_CONFIG } from "../config.js";
 
 const { createFrameSourceCache } = __testing;
@@ -143,3 +164,91 @@ describe("frame source cache eviction", () => {
     expect(cache.stats()).toMatchObject({ ...SHARED_STATS, entries: 0, bytes: 0 });
   });
 });
+
+describe("createVideoFrameInjector cache hygiene against page-side skips", () => {
+  // Build a minimal FrameLookupTable stand-in that returns one fixed payload
+  // for every time so we can drive the hook deterministically. The real
+  // table is exercised exhaustively in videoFrameExtractor.test.ts.
+  function fakeTable(payload: { videoId: string; framePath: string; frameIndex: number }) {
+    return {
+      getActiveFramePayloads: () =>
+        new Map([
+          [payload.videoId, { framePath: payload.framePath, frameIndex: payload.frameIndex }],
+        ]),
+    } as unknown as FrameLookupTable;
+  }
+
+  // Bypass the on-disk frame cache by handing back a synthetic data URI.
+  function inlineResolver(framePath: string): string {
+    return `data:image/png;base64,fake-${framePath}`;
+  }
+
+  beforeEach(() => {
+    injectVideoFramesBatchMock.mockReset();
+    syncVideoFrameVisibilityMock.mockReset();
+    syncVideoFrameVisibilityMock.mockResolvedValue(undefined);
+  });
+
+  it("does not poison the lastInjected cache when the page reports zero ids injected", async () => {
+    // Regression for the agentic-finecut scenario after PR #1028's ancestor
+    // skip: when injectVideoFramesBatch silently drops a video (its sub-comp
+    // host is hidden), the caller used to record `lastInjectedFrame[v] = N`
+    // anyway. On the next frame, if the source frameIndex is unchanged
+    // (low-fps source, multiple output frames per source frame, or
+    // non-frame-aligned host start), the cache short-circuits the second
+    // call and the host's first visible frame paints blank because the
+    // replacement <img> was never created.
+    //
+    // Pin the contract: when the page returns `[]` (no ids actually
+    // injected), the cache must not record those frameIndexes, so a follow-
+    // up call at the same frameIndex still issues an inject.
+    const fakePage = {} as Page;
+    const hook = createVideoFrameInjector(
+      fakeTable({ videoId: "pip", framePath: "/p", frameIndex: 5 }),
+      {
+        frameSrcResolver: inlineResolver,
+      },
+    );
+    expect(hook).not.toBeNull();
+
+    // First call: simulate the ancestor-hidden skip — page-side reports it
+    // injected nothing.
+    injectVideoFramesBatchMock.mockResolvedValueOnce([]);
+    await hook!(fakePage, 0);
+    expect(injectVideoFramesBatchMock).toHaveBeenCalledTimes(1);
+    expect(injectVideoFramesBatchMock).toHaveBeenLastCalledWith(fakePage, [
+      { videoId: "pip", dataUri: "data:image/png;base64,fake-/p" },
+    ]);
+
+    // Second call: same frameIndex, but the previous call did not really
+    // paint. The cache must NOT short-circuit; the inject must run again.
+    injectVideoFramesBatchMock.mockResolvedValueOnce(["pip"]);
+    await hook!(fakePage, 0);
+    expect(injectVideoFramesBatchMock).toHaveBeenCalledTimes(2);
+    expect(injectVideoFramesBatchMock).toHaveBeenLastCalledWith(fakePage, [
+      { videoId: "pip", dataUri: "data:image/png;base64,fake-/p" },
+    ]);
+  });
+
+  it("does cache normally when the page reports the id as injected", async () => {
+    // Counter-test: when injection succeeds for a videoId, the cache must
+    // record it and a second call at the same frameIndex must short-circuit.
+    // This pins the happy path so a future refactor can't trade the skip
+    // bug for a never-cache regression.
+    const fakePage = {} as Page;
+    const hook = createVideoFrameInjector(
+      fakeTable({ videoId: "pip", framePath: "/p", frameIndex: 5 }),
+      {
+        frameSrcResolver: inlineResolver,
+      },
+    );
+
+    injectVideoFramesBatchMock.mockResolvedValueOnce(["pip"]);
+    await hook!(fakePage, 0);
+    expect(injectVideoFramesBatchMock).toHaveBeenCalledTimes(1);
+
+    await hook!(fakePage, 0);
+    // Cache hit — no second inject for the same frameIndex.
+    expect(injectVideoFramesBatchMock).toHaveBeenCalledTimes(1);
+  });
+});
diff --git a/packages/engine/src/services/videoFrameInjector.ts b/packages/engine/src/services/videoFrameInjector.ts
index ddc79722a..d8b9e0e56 100644
--- a/packages/engine/src/services/videoFrameInjector.ts
+++ b/packages/engine/src/services/videoFrameInjector.ts
@@ -197,12 +197,22 @@ export function createVideoFrameInjector(
 
     await syncVideoFrameVisibility(page, Array.from(activeIds));
     if (updates.length > 0) {
-      await injectVideoFramesBatch(
-        page,
-        updates.map((u) => ({ videoId: u.videoId, dataUri: u.dataUri })),
+      // Only record cache entries for videos the page actually painted.
+      // injectVideoFramesBatch skips any video whose visual ancestor is
+      // hidden (sub-comp host out-of-window) and returns the subset of ids
+      // it really wrote — recording the rest would short-circuit the next
+      // call at the same frameIndex and leave the host's first visible
+      // frame blank.
+      const injectedIds = new Set(
+        await injectVideoFramesBatch(
+          page,
+          updates.map((u) => ({ videoId: u.videoId, dataUri: u.dataUri })),
+        ),
       );
       for (const update of updates) {
-        lastInjectedFrameByVideo.set(update.videoId, update.frameIndex);
+        if (injectedIds.has(update.videoId)) {
+          lastInjectedFrameByVideo.set(update.videoId, update.frameIndex);
+        }
       }
     }
   };

From dcd637bb4ac9b13e34913da1710c44f7243a583e Mon Sep 17 00:00:00 2001
From: Lirian Su <liriansu@gmail.com>
Date: Mon, 25 May 2026 13:01:45 +0800
Subject: [PATCH 5/5] 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.
---
 packages/cli/src/commands/snapshot.ts | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/packages/cli/src/commands/snapshot.ts b/packages/cli/src/commands/snapshot.ts
index a05292339..24cbb0470 100644
--- a/packages/cli/src/commands/snapshot.ts
+++ b/packages/cli/src/commands/snapshot.ts
@@ -232,10 +232,16 @@ async function captureSnapshots(
 
       // Chrome-headless ignores programmatic <video>.currentTime writes, so
       // we extract frames via FFmpeg and overlay them as <img> elements.
+      //
+      // The engine's injectVideoFramesBatch returns the subset of videoIds it
+      // actually painted (skipped ancestor-hidden videos are excluded).
+      // Snapshot doesn't use the return value, but the local type must match
+      // the real export — a `Promise<void>` shape rejects the `as` cast on
+      // the dynamic import.
       type InjectFn = (
         page: unknown,
         updates: Array<{ videoId: string; dataUri: string }>,
-      ) => Promise<void>;
+      ) => Promise<string[]>;
       type SyncVisibilityFn = (page: unknown, activeVideoIds: string[]) => Promise<void>;
       type ExtractMediaMetadataFn = (
         filePath: string,
