review(1198 follow-up): non-blocking ASR unload on the normal completion path too

[debpalash]

CodeRabbit's finding covered both unload sites; the harvest fixed only
gen()'s finally. The success path still blocked the event loop for the
gc/CUDA-cache drop on every completed transcription.

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

Summary by CodeRabbit

• Bug Fixes

  • Improved transcription stream responsiveness by offloading speech-recognition model cleanup from the active streaming path.
  • Ensured cleanup failures behave consistently and prevent repeated unload attempts.

• Chores / CI

  • Enhanced the Windows smoke-test dependency setup by installing ffmpeg with a retry loop and incremental backoff, including clearer logs for failed attempts.

[coderabbitai]

Caution

Review failed

The pull request is closed.

Warning

.coderabbit.yaml has a parsing error

The CodeRabbit configuration file in this repository has a parsing error and default settings were used instead. Please fix the error(s) in the configuration file. You can initialize chat with CodeRabbit to get help with the configuration file.

💥 Parsing errors (1)

Validation error: Too big: expected string to have <=250 characters at "tone_instructions"

⚙️ Configuration instructions

• Please see the configuration documentation for more information.
• You can also validate your configuration using the online YAML validator.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 29fcaa40-5a52-43b1-b59d-477a16d3ee3d

📥 Commits

Reviewing files that changed from the base of the PR and between 68a479f and 62afc20.

📒 Files selected for processing (1)

• .github/workflows/ci.yml

---

📝 Walkthrough

Walkthrough

The SSE transcription cleanup now unloads the ASR backend asynchronously through the GPU executor. Windows smoke tests now retry Chocolatey ffmpeg installation up to three times with incremental backoff.

Changes

ASR cleanup flow

Layer / File(s)	Summary
GPU-pool ASR unloading backend/api/routers/dub_core.py	Post-transcription cleanup awaits _asr_backend.unload() through loop.run_in_executor(_gpu_pool, ...) while retaining error handling and backend reference clearing.

Windows smoke-test dependency installation

Layer / File(s)	Summary
ffmpeg installation retries .github/workflows/ci.yml	The Windows system-deps step retries Chocolatey ffmpeg installation up to three times, logging failures and applying incremental delays.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• debpalash/OmniVoice-Studio#75: Introduces and extends ASR backend unload() implementations used by the asynchronous cleanup change.
• debpalash/OmniVoice-Studio#1197: Modifies the same transcription stream’s ASR backend lifecycle.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description is too brief and does not follow the required template sections for summary, changes, type, testing, and checklist.	Add the required Summary, Changes, Type, Testing, and Checklist sections and fill them with concrete details about the unload-path fix.
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the main change: making ASR unload non-blocking on the normal completion path.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/1198-followup-unload-normal-path

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

This follow-up to #1198 offloads _asr_backend.unload() to _gpu_pool on the normal-completion path in _gen_body, matching the fix already applied to gen()'s finally block; _loaded_asr["backend"] is cleared unconditionally after the executor call so the finally-block guard still prevents a double-unload. The CI change wraps the single Chocolatey ffmpeg install in a 3-attempt retry loop with linear backoff.

• dub_core.py: await loop.run_in_executor(_gpu_pool, _asr_backend.unload) replaces the direct blocking call; the post-try _loaded_asr["backend"] = None clear keeps the gen() finally idempotent on every exit path.
• ci.yml: The retry loop exits on success (&& break) and falls through to ffmpeg -version on total failure, which acts as the validation step and fails the job correctly.

Important Files Changed

Filename	Overview
backend/api/routers/dub_core.py	Normal-completion ASR unload moved off the event loop via run_in_executor; _loaded_asr["backend"] cleared unconditionally to keep the gen() finally guard correct — no issues found.
.github/workflows/ci.yml	Three-attempt retry loop with 30/60/90s backoff for Chocolatey ffmpeg install; ffmpeg -version after the loop acts as the failure gate — correct.

_{Reviews (2): Last reviewed commit: "ci: retry chocolatey ffmpeg install — co..." | Re-trigger Greptile}