Skip to content

fix(engine): retain fresh frames when replacing an empty cache - #5351

Merged
jrusso1020 merged 1 commit into
heygen-com:mainfrom
user-github-me:fix/empty-extraction-cache-publish
Oct 24, 2026
Merged

jrusso1020 merged 1 commit into
heygen-com:mainfrom
user-github-me:fix/empty-extraction-cache-publish

Conversation

@user-github-me

@user-github-me user-github-me commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

If an extraction cache loses its frames but retains .hf-complete, lookup correctly reports a miss. After re-extraction, the publisher still adopts the empty directory as a completed concurrent winner and discards the fresh frames. Every subsequent render fails the coverage gate with zero frames.

Require a published winner to have frames as well as the completion sentinel, using the same check as cache lookup. An empty entry then takes the existing stale-directory replacement path. Healthy winners remain reusable; frame-count and coverage checks stay in their existing layers. This completes the empty-cache recovery promised by merged #3434.

Validation: both new tests fail on main; all 45 cache, frame-index, and recovery tests pass with the fix. Six built CLI renders show the cold/recovery/warm sequence before and after: main's two recovery attempts fail at 0/30 frames; the fix produces 30-frame, one-second MP4s and identical extracted-frame bytes on all three runs. Engine/CLI builds, engine/producer/CLI typechecks, repository lint, formatting, comment checks, test reachability, and normal commit hooks pass.

The broader extractor suite cannot complete with this machine's FFmpeg 9 because two existing fixture setups use the removed -vsync option; both fail identically with the change reverted. The dedicated recovery test uses current FFmpeg options and runs successfully.

@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.

Confirmed the bug on current main: lookupCacheEntry requires the sentinel plus hasFrameFiles, but adoptPublishedWinner only checks the sentinel, so after a re-extraction the rename hits ENOTEMPTY on the emptied-but-sentineled dir, the empty dir is adopted as the winner, and the fresh frames are deleted. Adding hasFrameFiles makes the publisher use the same definition of a usable entry as lookup, and an empty entry falls through to the existing rm-and-rename path. The TOCTOU retry calls the same helper, so it gets the fix too. The unit test reproduces the exact sentinel-without-frames state, and the ffmpeg recovery test covers the cold, recovered and warm sequence end to end. Need: yes · Size: ok · Reuse: reuses hasFrameFiles from lookup · Simpler: already minimal.

— Rames

@jrusso1020
jrusso1020 added this pull request to the merge queue Oct 24, 2026
@jrusso1020
jrusso1020 removed this pull request from the merge queue due to a manual request Oct 24, 2026
@jrusso1020
jrusso1020 added this pull request to the merge queue Oct 24, 2026
Merged via the queue into heygen-com:main with commit 6fccefe Oct 24, 2026
233 of 234 checks passed
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