Skip to content

perf: transcript Gemini fallback cleans up downloaded video on the event loop #1244

Description

@groupthinking

Problem

TranscriptActionWorkflow._fallback_transcript_with_gemini cleans up the downloaded
video in a finally block that runs directly on the event loop.

Line Call Blocking work
transcript_action_workflow.py:852 video_path.exists() stat() syscall
transcript_action_workflow.py:854 video_path.unlink() unlink of a full downloaded video
transcript_action_workflow.py:857 temp_root.exists() stat() syscall
transcript_action_workflow.py:858 shutil.rmtree(temp_root, ignore_errors=True) recursive walk + unlink of the whole temp tree

_download_video_file requests
best[ext=mp4]/bestvideo[ext=mp4]+bestaudio[ext=m4a]/best with
merge_output_format: mp4, so the temp tree can hold the merged output plus
unmerged .fNNN fragments — potentially hundreds of MB across several files.
Each inode removal happens while the loop is stalled, so every other in-flight
request served by that worker is frozen for the duration.

The same file already does its download off-loop: _download_video_file builds a
synchronous _download() closure and returns await asyncio.to_thread(_download)
(line 1058). Only the cleanup half was left on the loop.

Reachability evidence

Reached from two live HTTP endpoints (router mounted at main.py:192):

POST /api/v1/transcript-action    router.py:456 -> run_transcript_action:462
POST /api/v1/videos/process       router.py:1489 -> _run_video_job:1515
  -> TranscriptActionWorkflow.run                   router.py:500 / router.py:1537
  -> TranscriptActionWorkflow.run                   transcript_action_workflow.py:99
  -> _extract_transcript                            :132 -> :307
  -> _fallback_transcript_with_gemini               :335 -> :708
  -> finally: exists()/unlink()/exists()/rmtree()   :852-858   <-- runs on the loop

Import-closure analysis from youtube_extension.main confirms the module is in the
production closure:
main -> backend.api.v1.router -> services.workflows.transcript_action_workflow.

Acceptance criteria

  • No filesystem call in the finally block executes on the event-loop thread.
  • Cleanup still removes the downloaded file and the temporary tree.
  • Cleanup still runs to completion even when the caller is cancelled. The current
    synchronous code is uncancellable, so that property must not regress.
  • Existing TestFallbackTranscriptWithGemini tests keep passing unchanged.
  • Regression tests assert cleanup runs on a non-loop thread and that the loop stays
    responsive while cleanup is in flight.

Proposed fix

Move the four calls into a synchronous closure and await it via asyncio.to_thread,
mirroring _download_video_file. Preserve the predicates (exists() guards, the
OSError swallow, and ignore_errors=True) exactly — this change is about where the
work runs, not what it does.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions