Repository navigation
fix(engine): retain fresh frames when replacing an empty cache - #5351
jrusso1020 merged 1 commit into
Conversation
jrusso1020
left a comment
There was a problem hiding this comment.
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
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
-vsyncoption; both fail identically with the change reverted. The dedicated recovery test uses current FFmpeg options and runs successfully.