Skip to content

fix(engine): a missing frame file skips its self-verify sample instead of aborting the render - #7334

Merged
miguel-heygen merged 1 commit into
mainfrom
test/engine-disk-sample-check
Nov 6, 2026
Merged

miguel-heygen merged 1 commit into
mainfrom
test/engine-disk-sample-check

Conversation

@miguel-heygen

Copy link
Copy Markdown
Collaborator

What

A missing frame file now skips its drawElement self-verify sample on the disk path, as the function's contract says, instead of aborting the render with "ffmpeg or the psnr filter is unavailable". This PR also adds the first tests that run the real disk compare and the real sample arming.

Why

psnrForDiskSample read the file and ran the compare inside one try. A read that failed with ENOENT reached isFfmpegInfrastructureFailure, which reads any ENOENT as a missing ffmpeg binary, so a frame file that was not there failed the render and blamed ffmpeg.

The new tests found that. Before them, nothing ran verifyDiskDrawElementSamples: the tests that mention it mock it out. Making the compare never throw, or arming zero samples in captureDeVerificationFrames, left every test green. Sequential disk capture and parallel workers rely on this check before shipping drawElement frames.

Related work

Refs #7324, whose review exposed the gap. #7255 covers the separate segmented and streaming compare, which already has tests.

How

  • parallelCoordinator.ts: reads the file in its own try. A read failure skips the sample; compare failures keep their existing skip or abort classification.
  • frameCapture.ts: exports captureDeVerificationFrames so a test can call it.
  • parallelCoordinator.verifyDisk.test.ts: real files on disk, psnrDb stubbed, since scoring is ffmpeg's job and the check's logic is what is under test.
  • frameCapture-verifyArming.browser.test.ts: real Chrome and a real truth page.

Test plan

  • Unit and browser tests added. Fails without the fix: with the read back inside the compare's try, "skips a sample whose file cannot be read, and still checks the next" fails (1 failed, 16 passed).
  • Each break is caught, one at a time, then restored: the disk compare never throws (3 failed); arming zero samples (3 failed); comparing against the wrong sample's truth (2 failed).
  • New tests: 17 passed, three runs in a row on Linux, with Chrome really running (not skipped). parallelCoordinator.test.ts, frameCapture-verifySampleFractions.test.ts, frameCapture.test.ts: 122 passed.
  • Engine typecheck, oxlint and oxfmt on the changed files.
  • Manual testing performed: no real drawElement render. It needs a macOS or Windows GPU host.
  • Comments follow CONTRIBUTING.md "Comments".

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 267 (base branch 267), smooth 114 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (8)

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approved at 57e09a5d. No blockers.

  • Fix: moving readFile into its own try stops a frame-file ENOENT from reaching isFfmpegInfrastructureFailure, which reads any ENOENT as a missing binary. The skip path now matches the function's documented contract (readFile races are per-sample noise). Compare failures keep their skip/abort classification, and both abort cases are still tested.
  • Fails without the fix, checked locally: I restored parallelCoordinator.ts/frameCapture.ts from the merge-base and ran parallelCoordinator.verifyDisk.test.ts. Result: 1 failed / 11 passed, and the failure was the named test "skips a sample whose file cannot be read, and still checks the next". With the PR head: 12/12 passed.
  • Tests: the disk tests run against real files with only psnrDb stubbed, which fits since the logic under test is the check, not ffmpeg scoring. The browser arming test is skipped when Chrome is missing. I did not run it locally. CI is green on this head; Windows render verification is still running.

SanHsien added a commit to SanHsien/hyperframes that referenced this pull request Nov 6, 2026
…, record duplicate rule

Review PR heygen-com#7304-heygen-com#7335: 10 merged already adopted or deferred; 10 open or draft PRs
pinned (heygen-com#7304, heygen-com#7308, heygen-com#7311, heygen-com#7315, heygen-com#7321, heygen-com#7330-heygen-com#7334); open duplicates of
pinned heygen-com#7281/heygen-com#7285/heygen-com#7286/heygen-com#7296 are tracked by those pins. strict exit 0;
monitored 110 -> 120.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@miguel-heygen
miguel-heygen added this pull request to the merge queue Nov 6, 2026
Merged via the queue into main with commit cfa0d0c Nov 6, 2026
91 checks passed
@miguel-heygen
miguel-heygen deleted the test/engine-disk-sample-check branch November 6, 2026 21:00
SanHsien added a commit to SanHsien/hyperframes that referenced this pull request Nov 7, 2026
…-com#7336-heygen-com#7358

Adopt heygen-com#7304 (without CLI renderDiagnostic, needs heygen-com#6725), heygen-com#7331, heygen-com#7332, heygen-com#7334,
heygen-com#7337, heygen-com#7336, heygen-com#7339, heygen-com#7289, heygen-com#5594, heygen-com#6900, heygen-com#7322, heygen-com#7342, heygen-com#7343, heygen-com#7350 (onProgress
only), heygen-com#7352, heygen-com#7348, heygen-com#7344, heygen-com#7353, heygen-com#7356 and heygen-com#7355. Defer heygen-com#5708 (needs heygen-com#6911) and
heygen-com#7345 (READ_VERSION 4 vs 7); heygen-com#7307 not applicable. Re-read 14 moved pins, pin 6
new PRs (monitored 126). Make checkReportFile module-private and suppress the
cross-package closeTestChrome duplicate for fallow.

Co-authored-by: Miguel Ángel <miguel.sierra@heygen.com>
Co-authored-by: SophiaHuangCc <129631224+SophiaHuangCc@users.noreply.github.com>
Co-authored-by: Dante <duanjl.china@gmail.com>
Co-authored-by: Muzzy5150 <muzzy5150yt@gmail.com>
Co-authored-by: ump45nose <52391318+ump45nose@users.noreply.github.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

1 participant