Skip to content

fix(engine): isolate drawElement verification from capture - #5452

Open
sulmatajb wants to merge 3 commits into
heygen-com:mainfrom
sulmatajb:fix/isolate-drawelement-verification
Open

sulmatajb wants to merge 3 commits into
heygen-com:mainfrom
sulmatajb:fix/isolate-drawelement-verification

Conversation

@sulmatajb

@sulmatajb sulmatajb commented Oct 11, 2026 •

Copy link
Copy Markdown

What

Preserve late-starting lower thirds during default drawElement capture. A 100-second composition with the unchanged lt-side-rule block mounted at 0.3 seconds currently exports without its title; with this change, the title appears while drawElement capture and its verification remain enabled.

Refs #5415. This addresses the missing graphic; the report's separate observation about the debug-artifact directory is outside this patch.

Why

captureDeVerificationFrames seeks the live capture page through late sample times and then rewinds it to zero. In this reproduction, the block's delayed visibility tween captures inherited visibility:hidden during those seeks. The live page retains that state, so the subsequent render loses the title. The eight verification samples are all after the 0.3–5.1 second title window and still pass.

Disabling only HF_DE_VERIFY restores the title. Inspecting the real page at 2 seconds shows the inner #lt element hidden before the change and visible afterward.

Related work

How

Capture drawElement ground-truth samples on createStaticVerificationPage(session), then close that page in finally. Preserve the existing sample selection, video-frame injection, double screenshots, and verification results. The live capture page is used only for the existing read-only eligibility checks.

Regression tests run the actual browser callbacks in separate mocked page contexts. They assert that verification never seeks or injects into the live page, that the isolated page supplies the expected frames, and that screenshot/injection failures propagate while closing it. Disabled verification allocates no page.

Test plan

  • Regression first: 3 new tests fail on the original implementation; 1 disabled-verification case passes. Patched new and existing verification-page suites: 8 pass.
  • Related capture suites: 341 pass, 3 existing browser-placeholder tests skipped, across 31 files.
  • All-workspace build; engine, core, Studio and scripts typechecks; changed-file oxlint/oxfmt; Fallow audit; tracked-artifact and large-file checks.
  • Comment citations and comment-share checks.
  • Real 100-second, 1080p/30fps, 3,000-frame exports on macOS arm64, Apple M6, Node 24.21.0, Chrome headless shell 152.0.7977.30, FFmpeg 6.0. Reproduced with the canonical block and both a solid-color and moving synthetic video background.
  • Fixed default exports retain the title and pass all eight drawElement verification samples. Compared the title region over seconds 1–4 against screenshot-capture references: 55.30 dB PSNR for the solid background, 50.38 dB for moving video. These are region-specific checks, not whole-render equivalence claims.
  • Composition lint/browser checks pass. The moving fixture is darkened for readable contrast; the canonical title block is unchanged.
  • Full repository CI: eight fork workflows await maintainer approval; the two completed status checks pass.

The broader bun run test completed with failures outside this change. After hydrating the exact LFS fixture and correcting the local tool environment, all ten failing files were rerun against both the untouched base and the PR: the same six tests still fail (two FFmpeg color/timestamp cases, one macOS watcher case, and three cancellation cases), and the same font-suite setup is blocked by the protected user home. The remaining 392 tests in those comparison files pass in each checkout. The isolated test environment also denies ps process inspection, which the cancellation ancestry check requires. Full logs and comparison results are retained; this is not a claim that the full repository suite is green.

Reproduction: follow #5415's init/add lt-side-rule setup, mount the block at data-start="0.3" for 4.8 seconds, render the full 100 seconds with default capture, and extract the frame at 2 seconds. For a screenshot reference use PRODUCER_FORCE_SCREENSHOT=true; for a diagnostic on the old implementation use HF_DE_VERIFY=0. The fix does not disable verification or force screenshot capture.

Prepared with Codex. The contribution is limited to the engine function and its regression tests; diagnostic harnesses and generated media are not included in the source diff.

Collect drawElement ground-truth frames on the existing disposable verification page. This prevents verifier seeks from caching hidden state in late-starting GSAP clips on the live capture page. Close the verification page on success and failure.

Addresses heygen-com#5415. Prepared with Codex; regression tests and real 3,000-frame exports are documented in the pull request.
Verify that ground-truth capture never seeks or injects into the live page, closes its disposable page on success and failure, and allocates no page when disabled.

Three symptom tests fail before the engine change and pass afterward. Prepared with Codex.

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracking this down. The root cause in #5415 is right:

  • Verification seeked the live capture page out of order, so the tl.set(wrap, {visibility:"hidden"}, 4.75) render recorded hidden as the start value.
  • Rewinding to 0 then kept the title hidden for the whole render, and every sample fell after its window, so verification still passed.
  • Moving verification onto its own page is the right fix, and it matches what static dedup already does for the same reason.

There's one blocker the separate page introduces, plus two should-fixes.

Blocker: a video can be captured blank after verification

  • session.onBeforeCapture is the video injector closure. It remembers the last frame it injected for each video in lastInjectedFrameByVideo (videoFrameInjector.ts:189), and skips a video whose frame index hasn't changed (:223).
  • That memory is per session, not per page. Verification now runs the same hook against the throwaway page (frameCapture.ts, verification loop), then closes the page, so the memory describes a page that no longer exists.
  • On the first real capture frame, a video whose frame index equals the one from the last verification sample is skipped. The capture page never received that frame, so the video area renders empty, with no warning.
  • Example: a short looping background video whose frame index at the 95% sample matches t=0. A scratch test against this head reproduced it: 0 injections on the capture page where 1 was expected.
  • Fix: key that memory by page (e.g. a WeakMap<Page, Map>) or reset it after verification. Please add a regression test.

Should fix

  1. Deduped media ids aren't injected on the verification page. It doesn't install installMediaRenderIdBridge, which the capture page does (frameCapture.ts:1440). A video with an id like clip__hf2 therefore never gets injected into the reference frames. A sample on it then fails verification and re-renders the whole job by screenshot, about 2× the wall time. Installing the bridge in createStaticVerificationPage fixes it.
  2. If the second page fails to load, the whole render fails. A navigation or readiness timeout throws an error the orchestrator doesn't fall back on. Static dedup catches the same failure and carries on (frameCapture.ts:3654-3663). This path should fall back to screenshots the same way.

Worth knowing: this adds one extra full page load per drawElement session, which means per worker, at init. Comps eligible for static dedup pay two. It's fine for the fix, but the PR doesn't measure it.

Nit: the deleted comment explaining "seek 0 first, then ascending" still applies on the new page; a trimmed version is worth keeping.

Tests: the new tests pass 4/4, and 3 of them fail against main's frameCapture.ts. The related frameCapture*, drawElementService* and videoFrameInjector* suites pass (323). Merges cleanly with main. I couldn't run a real #5415 repro here because this box has no GPU that supports drawElement.

— Rames

drawElement verification now runs the session's video frame injector on a throwaway page, but the injector remembered the last frame index per video for the whole session, so the first real capture frame could skip injecting a video the dead verification page had already received, leaving its area blank. The injector now keeps that memory per page. The verification page also installs the media render-id bridge the capture page has, so deduped media ids get injected into reference frames, and a verification page that fails to load keeps the session on its launch capture mode instead of failing the render or shipping drawElement unverified.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@jrusso1020

Copy link
Copy Markdown
Collaborator

@sulmatajb I wrote up a fix for the review points so this can land. It's one signed commit on top of your head f4417e2d: 03da53ff (on branch review/5452-verify-page-injection). It makes three changes:

  1. Blank-video blocker. The video injector keeps its "last injected frame" memory per page, in a WeakMap<Page, Map>, so the verification page can no longer make the capture page skip a frame. There's a regression test that fails on your head.
  2. The verification page now installs the media render-id bridge, so deduped ids like clip__hf2 are injected into the reference frames.
  3. If the verification page fails to load, the session stays on its launch capture mode (deFallbackTrigger = "verify_page_failed", with the transparent background set up as in the other fallbacks) instead of failing the render.

Engine tests (314), tsc, oxfmt/oxlint and the comment ratchet pass, and it merges cleanly with main.

Could you either:

  • cherry-pick it: git fetch https://github.com/heygen-com/hyperframes.git review/5452-verify-page-injection && git cherry-pick 03da53ff, or
  • tick "Allow edits by maintainers" on this PR, and we'll push it for you.

I'll approve as soon as it's in. Thanks for finding the root cause of #5415!

— Rames

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants