Repository navigation
fix(engine): a missing frame file skips its self-verify sample instead of aborting the render - #7334
Merged
Merged
Conversation
…d of aborting the render
Contributor
Contributor
There was a problem hiding this comment.
Approved at 57e09a5d. No blockers.
- Fix: moving
readFileinto its owntrystops a frame-fileENOENTfrom reachingisFfmpegInfrastructureFailure, which reads anyENOENTas 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.tsfrom the merge-base and ranparallelCoordinator.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
psnrDbstubbed, 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>
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
psnrfilter is unavailable". This PR also adds the first tests that run the real disk compare and the real sample arming.Why
psnrForDiskSampleread the file and ran the compare inside onetry. A read that failed withENOENTreachedisFfmpegInfrastructureFailure, which reads anyENOENTas 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 incaptureDeVerificationFrames, 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 owntry. A read failure skips the sample; compare failures keep their existing skip or abort classification.frameCapture.ts: exportscaptureDeVerificationFramesso a test can call it.parallelCoordinator.verifyDisk.test.ts: real files on disk,psnrDbstubbed, 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
try, "skips a sample whose file cannot be read, and still checks the next" fails (1 failed, 16 passed).parallelCoordinator.test.ts,frameCapture-verifySampleFractions.test.ts,frameCapture.test.ts: 122 passed.