fix(episode): stage frames in unique tempdir and publish atomically (#634)

[shobhitagnihotri69]

Summary

Prevents concurrency collisions and orphaned staging directories in Episode.frames() and Episode.frames_at_indices() when multiple callers or threads share an explicit workdir.

Why

Fixes #634.

When multiple Episode callers share one workdir, concurrent extractions previously collided on the fixed .tmp staging directory name or unlinked each other's in-progress files. Furthermore, frames_at_indices invoked shutil.rmtree(output_directory) on cache misses, which could blow away an in-progress or completed cache directory created by a concurrent caller.

Following maintainer guidance on #635:

• Stages frame extraction into a unique per-call directory (tempfile.mkdtemp(dir=self.workdir, prefix=...)) rather than a fixed .tmp name.
• Publishes completed frames with an atomic rename. If the output directory already exists (another caller finished first), the local staging directory is removed in finally and the winner's cache is used.
• Drops shutil.rmtree(output_directory) in frames_at_indices() since with atomic publishing the output directory is either completely published or absent.
• Avoids external file-locking dependencies or complex coordination.

Validation

Ran linting and type checks:

uv run ruff check
uv run ruff format --check
uv run ty check

[greptile-apps]

Confidence Score: 3/5

[Medium risk] Changes frame extraction staging to use unique temp directories.

The PR should not merge until rename failures are surfaced correctly and indexed-cache reads handle missing files.

Findings

1.  Incomplete cache returns missing frames ▶
2.  Rename errors discard extracted frames ▶
3.  Tests may miss publication race ▶

Summary

The PR gives each frame extraction its own staging directory, publishes completed frames by rename, and adds shared-workdir concurrency tests.

• The publication error handling can discard extracted frames without reporting the rename failure.
• Indexed cache reads no longer repair missing frame files, and the tests do not guarantee that a race occurs.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Frame request] --> B{Final cache exists?}
  B -- Yes --> E[Return cached frame paths]
  B -- No --> C[Extract into unique staging directory]
  C --> D[Rename staging to final cache]
  D --> F[Remove remaining staging directory]
  F --> E

Loading

_{Reviews (1) · Last reviewed commit: "fix(episode): stage frames in unique tem..."}

[greptile-apps]

Incomplete cache returns missing frames

If a JPEG is missing from an existing cache directory in a reused workdir, this check skips extraction and returns a path to the missing file. A caller that reads the frame then gets FileNotFoundError. The previous check verified every expected JPEG and rebuilt an incomplete cache.

Knowledge Base Used: Episode storage and identity

[greptile-apps]

Rename errors discard extracted frames

If the rename fails for a reason other than another caller publishing first, suppress(OSError) hides the failure and finally deletes the successfully extracted frames. frames() can then return an empty list; the same pattern in frames_at_indices() can return paths to files that do not exist. Check that a complete destination was published before treating a rename error as a cache hit.

[greptile-apps]

Tests may miss publication race

These workers are not synchronized, so the first extraction may finish before the second checks the cache. In that case the test passes through a cache hit without exercising concurrent staging or publication. The indexed-frame test has the same gap; synchronize both workers after they pass the cache-miss check so the tests reliably cover the race.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!