From 3700cc2a167c832801d1cc09a518d6d77c0fc9ce Mon Sep 17 00:00:00 2001
From: Lirian Su <liriansu@gmail.com>
Date: Fri, 22 May 2026 15:11:27 +0800
Subject: [PATCH] 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");
