[alexcastgonz (pcbeingused333)]

Fixes #3700

Description

VideoHelper._score_frames_async ran frame extraction with a bare asyncio.to_thread(self._extract_frames, video_path). Cancelling the caller ends that await but not the worker thread, which keeps writing PNGs created with delete=False. The cancelled caller never receives the paths, so the files are never scored, registered in memory or deleted. Both VideoTrueFalseScorer and VideoFloatScaleScorer use this helper.

The audio extraction in the same helper already handles this: it runs through run_legacy_sync_async (which waits for the worker before propagating cancellation) and removes its result on CancelledError.

Changes

• pyrit/score/video_scorer.py: new _extract_video_frames_async, mirroring _extract_video_audio_async. It runs _extract_frames through run_legacy_sync_async, and on cancellation removes every frame path the worker produced before re-raising. _score_frames_async calls it instead of asyncio.to_thread.
• tests/unit/score/test_video_scorer.py: test_video_frame_scorer_cancellation_during_extraction_cleans_up_frames_async (cancelled once and twice while the worker is blocked; asserts the frame files are gone and no frame is scored). It fails on main and passes with the change.

Tests and Documentation

• python -m pytest tests/unit/score/test_video_scorer.py passes (15 passed, 19 skipped because OpenCV is not installed in the test environment); ruff check and ruff format --check are clean on the touched files.
• No documentation change needed.

---

This change was prepared with the assistance of an AI coding agent (Claude).

[Roman Lutz (romanlutz)]

Rereviewed the complete updated diff at 21fbf66 after addressing the partial-extraction cleanup finding in 2a914ed. I found no remaining issue in the scoped change. Extraction owns its temporary files until it succeeds, exceptions clean up partial results, failed encodes create no file, and normal scoring keeps its existing retained-frame behavior.

The 106 targeted video, model and HTTP-target tests pass. All 11 original isolated-video review probes now pass, including the two previously failing partial-extraction cases. Changed-line coverage is 93.5%, and lint, formatting, type checks and the repository commit hooks pass. The original review thread has been answered and resolved. Required exact-head CI must pass before merge.

[Copilot]

🟢 Approval recommended

No active review issues block the cancellation-related temporary-frame cleanup changes.

No open issues 🎉

What changed in this PR

Fixes #3700 by preserving ownership of temporary video frames during cancellation so the shared video-scoring helper can clean them up before propagating cancellation.

Changes:

• Waits for frame extraction to finish and removes its outputs when scoring is cancelled.
• Cleans up partially extracted frames on failure and checks for unsuccessful image writes.
• Adds regression tests for single and repeated cancellation, partial failures, and preservation of unrelated files.

File	Description
tests/​unit/​score/​test_video_scorer.py	Adds cancellation and extraction-failure cleanup tests.
pyrit/​score/​video_scorer.py	Adds cancellation-safe extraction and temporary-frame cleanup on failure.

_{Review effort: Balanced}

[Roman Lutz (romanlutz)]

I compared the current heads of #3756 and #3806 after this PR merged. This PR contains the cancellation-safe extraction behavior, and it also covers partial extraction failure, failed encoding/writes, and single or repeated cancellation.

One useful assertion in #3806 is not explicit here: its cancellation test verifies that the source video still exists afterward. This implementation only removes the returned temporary frame paths, so the source should already be preserved. That explicit assertion could be added in a small follow-up test if we want to lock the behavior down.