fix: stabilize dub/diarization UI + production deployment + sonitranslate plumbing - #75
Conversation
📝 WalkthroughWalkthroughThis PR adds two ASR backends (NeMo Parakeet TDT and Moonshine), introduces a SoniTranslate sidecar service and router with dubbing endpoints, improves generation OOM detection and retry, tightens Dub UI layout and styling, adds a test localStorage mock, and adds argostranslate as a dependency. ChangesASR Backend Expansion
SoniTranslate Dubbing Integration
Generation Robustness
Frontend UI Layout and Styling
Test Infrastructure and Dependencies
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/api/routers/dub_generate.py`:
- Around line 166-206: The OOM detection currently treats any "CUDA error" as
OOM and masks real errors on retry; update the is_oom check in the exception
handler around _model.generate to look for torch.cuda.OutOfMemoryError or more
specific substrings like "cuda out of memory" (and keep the existing "out of
memory" lowercase check), not the generic "CUDA error"; then when catching the
retry exception (retry_err) apply the same OOM detection logic and only raise
the GPU-memory RuntimeError message if retry_err is also OOM—otherwise re-raise
the original retry_err to preserve the original error context (refer to is_oom,
torch.cuda.OutOfMemoryError, retry_err, _model.generate, apply_mastering,
normalize_audio).
In `@backend/api/routers/sonitranslate.py`:
- Around line 65-71: The request model DubRequest currently accepts raw
filesystem paths via video_path and output_dir which allows arbitrary host file
access; change the API to accept a server-managed asset identifier (e.g.,
video_id) instead of video_path and restrict output targets to server-controlled
locations (or a strict allowlist) rather than free-form output_dir. Update the
handler that calls soni.dub_video to: 1) resolve the incoming asset ID to a
canonical server path using a lookup function (and reject if not found), 2)
validate any requested output target by mapping a small set of allowed logical
names to server directories and canonicalizing the final path against those
roots, and 3) never pass client-provided raw paths into soni.dub_video (use the
resolved server path and mapped output path). Ensure these changes touch the
DubRequest model and the codepath that invokes soni.dub_video so only validated,
server-controlled paths are used.
- Around line 36-39: Replace direct exposure of internal exception details in
the HTTP responses: for each place that calls logger.exception("SoniTranslate
install failed") (and the other similar exception blocks that currently do raise
HTTPException(status_code=500, detail=str(e))), keep the logger.exception call
to record full details but change the raised HTTPException to return a generic
message (e.g., "Internal server error while processing SoniTranslate request")
or a safe error ID/slug instead of str(e); update the exception handling in the
same exception blocks referenced (the blocks using logger.exception(...)
followed by raise HTTPException(..., detail=str(e))) so only sanitized,
non-sensitive text is sent to the client while internal details remain in logs.
In `@backend/services/asr_backend.py`:
- Around line 651-657: The current try/except only catches ImportError for
moonshine_onnx but not exceptions from moonshine_onnx.transcribe; update the
block around moonshine_onnx.transcribe(audio_path, model=self._model_name) so
any exception raised during import or transcription (e.g., except Exception as
e) triggers the fallback to moonshine_voice.Transcriber; when falling back,
ensure you still set text (and join if list) using the Transcriber result and
preserve audio_path and self._model_name usage so behavior matches the intended
two-step strategy.
In `@backend/services/sonitranslate.py`:
- Around line 94-99: The install() function currently runs subprocesses to
create the venv (using asyncio.create_subprocess_exec with python, "-m", "venv",
SONI_VENV) and to install requirements but doesn't check their exit codes;
modify both spots to capture the process result (await proc.communicate() or
await proc.wait()), check proc.returncode, and if non-zero log the stderr (or
include error text) and raise an exception or return a failing status
immediately so install() fails fast on venv or pip failures; apply this to the
block using SONI_VENV/python and the subsequent pip/requirements install block
so a broken "installed" state cannot be returned.
- Around line 156-167: The startup timeout path currently raises without
cleaning up the started subprocess (_proc); modify the timeout branch in the
loop that waits for is_running() (and the final raise for "SoniTranslate failed
to start within 30s") to terminate the subprocess before raising: call
_proc.terminate() (and if it does not exit within a short grace period call
_proc.kill()), wait for the process to exit (use _proc.wait() or
asyncio.create_subprocess_* await pattern), capture and include the last
stdout/stderr output from _proc in the raised RuntimeError, and ensure any
exceptions from terminating are caught/logged via logger so no orphaned sidecar
remains.
- Around line 209-213: The Gradio Client creation and prediction are being
called synchronously inside the async function (e.g., when creating
Client(SONI_URL) and calling client.predict(...) in
batch_multilingual_media_conversion), which blocks the event loop; change both
calls to run in a thread via asyncio.to_thread (and add an asyncio import) so
you create the client with await asyncio.to_thread(lambda: Client(SONI_URL)) and
invoke prediction with await asyncio.to_thread(lambda: client.predict(...)),
preserving arguments and return values.
- Around line 38-39: The code hardcodes the POSIX "bin" venv path (e.g., the
expression creating pip: SONI_VENV / "bin" / "pip") which breaks Windows venvs
that use "Scripts" and different executable extensions; add a small helper
(e.g., venv_exec(name: str) or get_venv_path(subdir: str)) that selects
"Scripts" when sys.platform startswith "win" (otherwise "bin"), appends the
requested executable name and on Windows also try common extensions (".exe",
".cmd") or use shutil.which against that folder, and replace the direct
constructions at the pip, python and sonic/sonitranslate executable lookups (the
occurrences referenced by the symbols creating pip and the lookups at the other
two sites) to call this helper so the code works on Windows and POSIX.
In `@frontend/src/pages/DubTab.css`:
- Line 216: The .dub-split-2 CSS rule currently forces overflow: hidden which
may clip child content and hide scrollbars; verify intent and either remove or
change it to overflow: auto (or overflow-y: auto / overflow-x: auto as
appropriate) so content can scroll, or scope clipping to a child element if you
only intended to enforce border-radius clipping; locate the .dub-split-2
selector and update the overflow property accordingly and test with oversized
child content and interactive elements to ensure nothing important is being
clipped.
In `@frontend/src/pages/DubTab.jsx`:
- Line 590: The colored dot span that currently renders "●" (inside the same
expression using activeEngineUnavailable) must include an accessible status
label; update that span (the one with style={{ color: activeEngineUnavailable ?
'`#fb4934`' : '`#b8bb26`' }}) to provide an aria-label (e.g.
aria-label={activeEngineUnavailable ? "Translation engine unavailable" :
"Translation engine available"}) and role="img", or alternatively insert a
visually-hidden text node next to the dot conveying the same status; ensure you
use the existing activeEngineUnavailable boolean and keep the visible color
behavior unchanged.
In `@frontend/src/test/setup.js`:
- Around line 6-8: The getItem method incorrectly treats stored empty strings as
missing because it uses `store[key] || null`; update getItem (referencing the
getItem function and the store object) to return the exact stored value when the
key exists (e.g., check `store` for the key using `hasOwnProperty` or `key in
store`) and only return null when the key is truly absent, ensuring an empty
string is returned unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b98f26b3-fbc5-4520-b9c3-53abeb01c9f3
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
.gitignorebackend/api/routers/dub_generate.pybackend/api/routers/sonitranslate.pybackend/api/routers/system.pybackend/config/models.yamlbackend/main.pybackend/services/asr_backend.pybackend/services/sonitranslate.pyfrontend/src/components/DubSegmentRow.cssfrontend/src/components/DubSegmentTable.jsxfrontend/src/pages/DubTab.cssfrontend/src/pages/DubTab.jsxfrontend/src/test/setup.jspyproject.toml
| is_oom = ( | ||
| isinstance(e, torch.cuda.OutOfMemoryError) | ||
| or "out of memory" in str(e).lower() | ||
| or "CUDA error" in str(e) | ||
| ) | ||
| # Always try to reclaim VRAM regardless of error type. | ||
| import gc | ||
| gc.collect() | ||
| if torch.backends.mps.is_available(): | ||
| torch.mps.empty_cache() | ||
| elif torch.cuda.is_available(): | ||
| if torch.cuda.is_available(): | ||
| torch.cuda.empty_cache() | ||
| # User-facing: what happened · why · what to do. | ||
| raise RuntimeError( | ||
| f"Ran out of GPU memory generating this segment. " | ||
| f"Try the Flush button in the header to free VRAM, or switch to CPU in Settings. " | ||
| f"Underlying error: {e}" | ||
| elif hasattr(torch.backends, "mps") and torch.backends.mps.is_available(): | ||
| torch.mps.empty_cache() | ||
|
|
||
| if not is_oom: | ||
| raise # Non-OOM — propagate the real error, don't mask it. | ||
|
|
||
| # OOM recovery: retry once with reduced steps (less VRAM). | ||
| retry_steps = min(nstep, 8) | ||
| logger.warning( | ||
| "OOM on segment (nstep=%d), retrying with %d steps after cache flush", | ||
| nstep, retry_steps, | ||
| ) | ||
| try: | ||
| audios = _model.generate( | ||
| text=text, language=lang if lang != "Auto" else None, | ||
| ref_audio=ref_audio, ref_text=ref_text, | ||
| instruct=instruct_str if instruct_str else None, | ||
| duration=dur_s, num_step=retry_steps, guidance_scale=cfg, | ||
| speed=spd, denoise=True, postprocess_output=True, | ||
| ) | ||
| audio_out = audios[0] | ||
| mastered_audio = apply_mastering(audio_out, sample_rate=_model.sampling_rate if hasattr(_model, 'sampling_rate') else 24000) | ||
| return normalize_audio(mastered_audio, target_dBFS=-2.0) | ||
| except Exception as retry_err: | ||
| raise RuntimeError( | ||
| f"Ran out of GPU memory generating this segment. " | ||
| f"Retried with {retry_steps} steps but still failed. " | ||
| f"Try the Flush button in the header to free VRAM, " | ||
| f"or switch to CPU in Settings. " | ||
| f"Underlying error: {retry_err}" | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if there are any other error handling patterns in the codebase
# that give us context about how CUDA errors are handled
rg -A 5 "OutOfMemoryError|out of memory" backend/api/routers/dub_generate.pyRepository: debpalash/OmniVoice-Studio
Length of output: 408
🏁 Script executed:
# Check the imports to understand the PyTorch version and context
head -50 backend/api/routers/dub_generate.pyRepository: debpalash/OmniVoice-Studio
Length of output: 1862
🌐 Web query:
PyTorch torch.cuda.OutOfMemoryError exception CUDA error messages 2024 2025
💡 Result:
The torch.cuda.OutOfMemoryError in PyTorch is a RuntimeError indicating that the GPU has insufficient memory to fulfill an allocation request [1][2][3]. Because PyTorch uses a caching memory allocator to improve performance, memory reported by tools like nvidia-smi may not reflect the actual memory available to PyTorch [1][4]. Common Causes 1. Large Batch Sizes: The most frequent cause; each batch requires memory for inputs, activations, gradients, and weights [5][2]. 2. Memory Leaks: Accumulating tensors in lists or dictionaries without calling.detach or.cpu retains the computation graph and prevents memory from being freed [1][5]. 3. Gradient Accumulation: Forgetting to call optimizer.zero_grad causes gradients to accumulate across iterations [5][3]. 4. Memory Fragmentation: Even if total free memory is sufficient, it may be fragmented into small, non-contiguous blocks that cannot satisfy a large allocation request [5][2]. 5. Inference without torch.no_grad: By default, PyTorch tracks gradients during inference, consuming unnecessary memory [6][5][3]. Debugging and Monitoring - Memory Summary: Use torch.cuda.memory_summary to get a detailed breakdown of memory allocation, which helps identify the source of OOM errors [6][7]. - Statistics: Functions like torch.cuda.memory_stats and torch.cuda.memory_snapshot provide granular data on active, inactive, and reserved memory blocks [8][9]. - External Tools: Ensure no other processes are hogging GPU memory by checking with nvidia-smi [1][2]. Mitigation Strategies - Reduce Batch Size: The simplest fix; use gradient accumulation to maintain effective batch sizes [6][5][3]. - Mixed Precision Training: Use torch.cuda.amp (Automatic Mixed Precision) to reduce memory footprint [5][2][3]. - Environment Configuration: The PYTORCH_CUDA_ALLOC_CONF environment variable allows tuning the allocator [4][10]. For example, max_split_size_mb can reduce fragmentation by preventing large blocks from being split [4][10]. - Preemptive OOM Handling: Recent updates (as of 2026) include the throw_on_cudamalloc_oom flag, which can be used with per_process_memory_fraction to preemptively reject allocations and raise an OutOfMemoryError instead of triggering a fatal GPU runtime abort [11]. - Explicit Cleanup: Use del to remove unused variables and call torch.cuda.empty_cache to release cached memory back to the system (note that this does not free memory for other processes, only makes it available for PyTorch) [6][5][3]. Always ensure that OOM recovery code is placed outside of the except block to avoid holding references to the stack frame that caused the error [1].
Citations:
- 1: https://pytorch.org/docs/1.10/notes/faq.html
- 2: https://tensorrigs.com/blog/cuda-out-of-memory/
- 3: https://www.geeksforgeeks.org/deep-learning/how-to-avoid-cuda-out-of-memory-in-pytorch/
- 4: https://docs.nvidia.com/dl-cuda-graph/latest/troubleshooting/memory-issues.html
- 5: https://mljourney.com/pytorch-cuda-out-of-memory-causes-solutions-and-best-practices/
- 6: https://stackoverflow.com/questions/59129812/how-to-avoid-cuda-out-of-memory-in-pytorch
- 7: https://docs.pytorch.org/docs/stable/generated/torch.cuda.memory.memory_summary.html
- 8: https://docs.pytorch.org/docs/2.11/generated/torch.cuda.memory.memory_stats.html
- 9: https://docs.pytorch.org/docs/main/generated/torch.cuda.memory.memory_stats_as_nested_dict.html
- 10: https://runebook.dev/en/docs/pytorch/generated/torch.cuda.set_per_process_memory_fraction
- 11: pytorch/pytorch@74b2b19
Narrow the OOM check to prevent false retries and preserve non-OOM error context.
Line 169 matches any "CUDA error" substring, which catches device-side asserts, invalid device state, and other non-OOM CUDA failures that should not be retried with fewer steps. Lines 199–206 then always replace the retry failure with a GPU-memory message, masking the real root cause if the retry fails for a different reason.
Replace the generic "CUDA error" pattern with more specific OOM indicators like "cuda out of memory". Apply the same OOM detection to retry failures, and only wrap with the GPU-memory message if the retry error is also OOM; otherwise, re-raise the original exception to preserve debuggability.
Suggested fix
except Exception as e:
- is_oom = (
- isinstance(e, torch.cuda.OutOfMemoryError)
- or "out of memory" in str(e).lower()
- or "CUDA error" in str(e)
- )
+ err_msg = str(e).lower()
+ is_oom = (
+ isinstance(e, torch.cuda.OutOfMemoryError)
+ or "out of memory" in err_msg
+ or "cuda out of memory" in err_msg
+ )
# Always try to reclaim VRAM regardless of error type.
import gc
gc.collect()
if torch.cuda.is_available():
torch.cuda.empty_cache()
@@
try:
audios = _model.generate(
text=text, language=lang if lang != "Auto" else None,
ref_audio=ref_audio, ref_text=ref_text,
instruct=instruct_str if instruct_str else None,
@@
audio_out = audios[0]
mastered_audio = apply_mastering(audio_out, sample_rate=_model.sampling_rate if hasattr(_model, 'sampling_rate') else 24000)
return normalize_audio(mastered_audio, target_dBFS=-2.0)
except Exception as retry_err:
- raise RuntimeError(
- f"Ran out of GPU memory generating this segment. "
- f"Retried with {retry_steps} steps but still failed. "
- f"Try the Flush button in the header to free VRAM, "
- f"or switch to CPU in Settings. "
- f"Underlying error: {retry_err}"
- )
+ retry_msg = str(retry_err).lower()
+ retry_is_oom = (
+ isinstance(retry_err, torch.cuda.OutOfMemoryError)
+ or "out of memory" in retry_msg
+ or "cuda out of memory" in retry_msg
+ )
+ if not retry_is_oom:
+ raise
+ raise RuntimeError(
+ f"Ran out of GPU memory generating this segment. "
+ f"Retried with {retry_steps} steps but still failed. "
+ f"Try the Flush button in the header to free VRAM, "
+ f"or switch to CPU in Settings."
+ ) from retry_err🧰 Tools
🪛 Ruff (0.15.12)
[warning] 199-199: Do not catch blind exception: Exception
(BLE001)
[warning] 200-206: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/api/routers/dub_generate.py` around lines 166 - 206, The OOM
detection currently treats any "CUDA error" as OOM and masks real errors on
retry; update the is_oom check in the exception handler around _model.generate
to look for torch.cuda.OutOfMemoryError or more specific substrings like "cuda
out of memory" (and keep the existing "out of memory" lowercase check), not the
generic "CUDA error"; then when catching the retry exception (retry_err) apply
the same OOM detection logic and only raise the GPU-memory RuntimeError message
if retry_err is also OOM—otherwise re-raise the original retry_err to preserve
the original error context (refer to is_oom, torch.cuda.OutOfMemoryError,
retry_err, _model.generate, apply_mastering, normalize_audio).
| except Exception as e: | ||
| logger.exception("SoniTranslate install failed") | ||
| raise HTTPException(status_code=500, detail=str(e)) | ||
|
|
There was a problem hiding this comment.
Avoid leaking internal exception details in HTTP 500 responses.
detail=str(e) can expose filesystem paths, subprocess output, and environment-specific internals to clients.
💡 Suggested fix
- except Exception as e:
+ except Exception:
logger.exception("SoniTranslate install failed")
- raise HTTPException(status_code=500, detail=str(e))
+ raise HTTPException(status_code=500, detail="SoniTranslate install failed")
@@
- except Exception as e:
+ except Exception:
logger.exception("SoniTranslate start failed")
- raise HTTPException(status_code=500, detail=str(e))
+ raise HTTPException(status_code=500, detail="SoniTranslate start failed")
@@
- except Exception as e:
+ except Exception:
logger.exception("SoniTranslate dub failed")
- raise HTTPException(status_code=500, detail=str(e))
+ raise HTTPException(status_code=500, detail="SoniTranslate dub failed")Also applies to: 50-53, 91-93
🧰 Tools
🪛 Ruff (0.15.12)
[warning] 38-38: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/api/routers/sonitranslate.py` around lines 36 - 39, Replace direct
exposure of internal exception details in the HTTP responses: for each place
that calls logger.exception("SoniTranslate install failed") (and the other
similar exception blocks that currently do raise HTTPException(status_code=500,
detail=str(e))), keep the logger.exception call to record full details but
change the raised HTTPException to return a generic message (e.g., "Internal
server error while processing SoniTranslate request") or a safe error ID/slug
instead of str(e); update the exception handling in the same exception blocks
referenced (the blocks using logger.exception(...) followed by raise
HTTPException(..., detail=str(e))) so only sanitized, non-sensitive text is sent
to the client while internal details remain in logs.
| try: | ||
| import moonshine_onnx | ||
| text = moonshine_onnx.transcribe(audio_path, model=self._model_name) | ||
| if isinstance(text, list): | ||
| text = " ".join(text) | ||
| except ImportError: | ||
| from moonshine_voice import Transcriber |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the fallback only catches ImportError today.
rg -n -A20 -B5 'moonshine_onnx\.transcribe|except ImportError|moonshine_voice' backend/services/asr_backend.pyRepository: debpalash/OmniVoice-Studio
Length of output: 8017
Moonshine fallback only handles import failures, not runtime failures.
At line 653, if moonshine_onnx.transcribe() raises any exception other than ImportError, the error will propagate instead of falling back to moonshine_voice. The current except ImportError block (line 656) won't catch runtime failures, contradicting the intended two-step strategy noted in the comment at line 650.
Catch both import failures and runtime exceptions, then fall back to moonshine_voice when either occurs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/asr_backend.py` around lines 651 - 657, The current
try/except only catches ImportError for moonshine_onnx but not exceptions from
moonshine_onnx.transcribe; update the block around
moonshine_onnx.transcribe(audio_path, model=self._model_name) so any exception
raised during import or transcription (e.g., except Exception as e) triggers the
fallback to moonshine_voice.Transcriber; when falling back, ensure you still set
text (and join if list) using the Transcriber result and preserve audio_path and
self._model_name usage so behavior matches the intended two-step strategy.
| pip = SONI_VENV / "bin" / "pip" | ||
| return pip.is_file() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify hardcoded POSIX-only venv paths are present and Windows fallback is absent.
rg -n 'SONI_VENV / "bin"' backend/services/sonitranslate.py
rg -n 'Scripts|\.exe' backend/services/sonitranslate.pyRepository: debpalash/OmniVoice-Studio
Length of output: 373
🏁 Script executed:
cat -n backend/services/sonitranslate.py | head -150Repository: debpalash/OmniVoice-Studio
Length of output: 5977
🏁 Script executed:
rg -n 'SONI_VENV|virtualenv|venv' backend/services/sonitranslate.py | head -30Repository: debpalash/OmniVoice-Studio
Length of output: 747
Fix hardcoded POSIX virtualenv paths to support Windows.
The venv creation uses python -m venv (platform-agnostic), but executable lookups hardcode "bin" instead of handling Windows' "Scripts" directory. This breaks the sidecar entirely on Windows (lines 38, 102, 139).
Create a helper function to resolve platform-specific paths:
Suggested fix
+def _venv_executable(name: str) -> Path:
+ scripts_dir = "Scripts" if os.name == "nt" else "bin"
+ ext = ".exe" if os.name == "nt" else ""
+ return SONI_VENV / scripts_dir / f"{name}{ext}"
+
def is_venv_ready() -> bool:
"""Check if the SoniTranslate virtualenv exists with key deps."""
- pip = SONI_VENV / "bin" / "pip"
+ pip = _venv_executable("pip")
return pip.is_file()
@@
- pip = str(SONI_VENV / "bin" / "pip")
+ pip = str(_venv_executable("pip"))
@@
- python = str(SONI_VENV / "bin" / "python") if is_venv_ready() else sys.executable
+ python = str(_venv_executable("python")) if is_venv_ready() else sys.executable🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/sonitranslate.py` around lines 38 - 39, The code hardcodes
the POSIX "bin" venv path (e.g., the expression creating pip: SONI_VENV / "bin"
/ "pip") which breaks Windows venvs that use "Scripts" and different executable
extensions; add a small helper (e.g., venv_exec(name: str) or
get_venv_path(subdir: str)) that selects "Scripts" when sys.platform startswith
"win" (otherwise "bin"), appends the requested executable name and on Windows
also try common extensions (".exe", ".cmd") or use shutil.which against that
folder, and replace the direct constructions at the pip, python and
sonic/sonitranslate executable lookups (the occurrences referenced by the
symbols creating pip and the lookups at the other two sites) to call this helper
so the code works on Windows and POSIX.
| proc = await asyncio.create_subprocess_exec( | ||
| python, "-m", "venv", str(SONI_VENV), | ||
| stdout=asyncio.subprocess.PIPE, | ||
| stderr=asyncio.subprocess.PIPE, | ||
| ) | ||
| await proc.communicate() |
There was a problem hiding this comment.
Fail fast when environment setup commands fail.
install() can continue after failed venv/requirements installs and still return status, leaving a broken but “installed” state.
💡 Suggested fix
proc = await asyncio.create_subprocess_exec(
python, "-m", "venv", str(SONI_VENV),
stdout=asyncio.subprocess.PIPE,
stderr=asyncio.subprocess.PIPE,
)
- await proc.communicate()
+ _, stderr = await proc.communicate()
+ if proc.returncode != 0:
+ raise RuntimeError(f"Virtualenv creation failed: {stderr.decode()[-500:]}")
@@
- stdout, stderr = await proc.communicate()
+ _, stderr = await proc.communicate()
if proc.returncode != 0:
- logger.error("Base requirements failed: %s", stderr.decode()[-500:])
+ raise RuntimeError(f"Base requirements install failed: {stderr.decode()[-500:]}")
@@
proc = await asyncio.create_subprocess_exec(
pip, "install", "-r", str(SONI_DIR / "requirements_extra.txt"),
cwd=str(SONI_DIR),
stdout=asyncio.subprocess.PIPE,
stderr=asyncio.subprocess.PIPE,
)
- await proc.communicate()
+ _, stderr = await proc.communicate()
+ if proc.returncode != 0:
+ raise RuntimeError(f"Extra requirements install failed: {stderr.decode()[-500:]}")Also applies to: 106-125
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/sonitranslate.py` around lines 94 - 99, The install()
function currently runs subprocesses to create the venv (using
asyncio.create_subprocess_exec with python, "-m", "venv", SONI_VENV) and to
install requirements but doesn't check their exit codes; modify both spots to
capture the process result (await proc.communicate() or await proc.wait()),
check proc.returncode, and if non-zero log the stderr (or include error text)
and raise an exception or return a failing status immediately so install() fails
fast on venv or pip failures; apply this to the block using SONI_VENV/python and
the subsequent pip/requirements install block so a broken "installed" state
cannot be returned.
| # Wait up to 30s for it to be ready | ||
| for _ in range(60): | ||
| await asyncio.sleep(0.5) | ||
| if is_running(): | ||
| logger.info("SoniTranslate started successfully") | ||
| return {"started": True, **status()} | ||
| if _proc.poll() is not None: | ||
| out = _proc.stdout.read().decode()[-500:] if _proc.stdout else "" | ||
| raise RuntimeError(f"SoniTranslate exited early: {out}") | ||
|
|
||
| raise RuntimeError("SoniTranslate failed to start within 30s") | ||
|
|
There was a problem hiding this comment.
Clean up the subprocess when startup times out.
Timeout currently raises without terminating _proc, which can leave an orphaned sidecar process.
💡 Suggested fix
- raise RuntimeError("SoniTranslate failed to start within 30s")
+ if _proc is not None and _proc.poll() is None:
+ _proc.terminate()
+ try:
+ _proc.wait(timeout=5)
+ except subprocess.TimeoutExpired:
+ _proc.kill()
+ _proc = None
+ raise RuntimeError("SoniTranslate failed to start within 30s")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/sonitranslate.py` around lines 156 - 167, The startup
timeout path currently raises without cleaning up the started subprocess
(_proc); modify the timeout branch in the loop that waits for is_running() (and
the final raise for "SoniTranslate failed to start within 30s") to terminate the
subprocess before raising: call _proc.terminate() (and if it does not exit
within a short grace period call _proc.kill()), wait for the process to exit
(use _proc.wait() or asyncio.create_subprocess_* await pattern), capture and
include the last stdout/stderr output from _proc in the raised RuntimeError, and
ensure any exceptions from terminating are caught/logged via logger so no
orphaned sidecar remains.
| client = Client(SONI_URL) | ||
|
|
||
| # The main function is `batch_multilingual_media_conversion` | ||
| # which is exposed as the first API endpoint | ||
| result = client.predict( |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify sync Gradio calls inside async dub path.
rg -n -C2 'async def dub_video|Client\(|client\.predict\(' backend/services/sonitranslate.pyRepository: debpalash/OmniVoice-Studio
Length of output: 578
🏁 Script executed:
#!/bin/bash
# Check lines 271-272 mentioned in the review
sed -n '269,274p' backend/services/sonitranslate.pyRepository: debpalash/OmniVoice-Studio
Length of output: 275
🏁 Script executed:
#!/bin/bash
# Check if asyncio is imported and what pattern is used elsewhere in the file
head -20 backend/services/sonitranslate.py && echo "---" && rg -n 'import asyncio|from asyncio|asyncio\.' backend/services/sonitranslate.py | head -10Repository: debpalash/OmniVoice-Studio
Length of output: 1068
🏁 Script executed:
#!/bin/bash
# Find all async function definitions to understand the async context
rg -n 'async def' backend/services/sonitranslate.pyRepository: debpalash/OmniVoice-Studio
Length of output: 210
🌐 Web query:
Gradio Client library async support Python
💡 Result:
The Gradio Python Client library does not provide native, first-class support for Python's async/await syntax (asyncio) for its primary API methods like predict or submit [1][2]. Key details regarding asynchronous operations in the Gradio Python Client: 1. Blocking vs. Non-blocking: The client's predict method is a blocking operation that waits for the remote API call to complete [1][2]. The submit method is the recommended approach for non-blocking operations; it runs the prediction in a background thread and returns a Job object immediately [1][2]. 2. Job Management: While submit is non-blocking, it relies on threading rather than asyncio [1][2]. You can interact with the returned Job object to check status, retrieve results using the blocking result method, or add callbacks to handle completion [1][2]. 3. Limitations: Because the library is designed around thread-based concurrency for background tasks, it does not natively integrate with an asyncio event loop [1][2]. Users attempting to use the client in highly concurrent async environments (e.g., submitting many jobs rapidly) have reported potential race conditions or issues with job completion tracking [3]. If you are working within an asynchronous Python application (such as FastAPI), you may need to wrap the blocking Gradio client calls in run_in_executor to avoid blocking the main event loop.
Citations:
- 1: https://www.gradio.app/main/docs/python-client/client
- 2: https://www.gradio.app/guides/getting-started-with-the-python-client
- 3: Python gradio_client: when submitting asynchronous jobs too rapidly to a batch server, some never report completed gradio-app/gradio#9968
🏁 Script executed:
#!/bin/bash
# Get full context around lines 271-272
sed -n '265,280p' backend/services/sonitranslate.pyRepository: debpalash/OmniVoice-Studio
Length of output: 654
🏁 Script executed:
#!/bin/bash
# Find all Client instantiations and predict calls in the file
rg -n 'Client\(|\.predict\(' backend/services/sonitranslate.pyRepository: debpalash/OmniVoice-Studio
Length of output: 137
🏁 Script executed:
#!/bin/bash
# Get the full dub_video function to understand its scope
sed -n '185,320p' backend/services/sonitranslate.pyRepository: debpalash/OmniVoice-Studio
Length of output: 4698
Wrap blocking Gradio Client calls with asyncio.to_thread().
Client(SONI_URL) and client.predict(...) are synchronous operations that block the event loop when called directly in this async function, stalling other concurrent requests for the full dub duration.
Suggested fix
- client = Client(SONI_URL)
+ client = await asyncio.to_thread(Client, SONI_URL)
# The main function is `batch_multilingual_media_conversion`
# which is exposed as the first API endpoint
- result = client.predict(
+ result = await asyncio.to_thread(client.predict,
handle_file(video_path), # media_file
"", # link_media
@@
api_name="/batch_multilingual_media_conversion",
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| client = Client(SONI_URL) | |
| # The main function is `batch_multilingual_media_conversion` | |
| # which is exposed as the first API endpoint | |
| result = client.predict( | |
| client = await asyncio.to_thread(Client, SONI_URL) | |
| # The main function is `batch_multilingual_media_conversion` | |
| # which is exposed as the first API endpoint | |
| result = await asyncio.to_thread(client.predict, | |
| handle_file(video_path), # media_file | |
| "", # link_media | |
| # ... (remaining arguments) | |
| api_name="/batch_multilingual_media_conversion", | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/sonitranslate.py` around lines 209 - 213, The Gradio Client
creation and prediction are being called synchronously inside the async function
(e.g., when creating Client(SONI_URL) and calling client.predict(...) in
batch_multilingual_media_conversion), which blocks the event loop; change both
calls to run in a thread via asyncio.to_thread (and add an asyncio import) so
you create the client with await asyncio.to_thread(lambda: Client(SONI_URL)) and
invoke prediction with await asyncio.to_thread(lambda: client.predict(...)),
preserving arguments and return values.
| > | ||
| <ChevronDown size={10} /> | ||
| <span><strong>{dubLang}</strong> · {dubLangCode} · {translateQuality} · {translateProvider}</span> | ||
| <span><strong>{dubLang}</strong> · {dubLangCode} · {translateQuality} · <span style={{ color: activeEngineUnavailable ? '#fb4934' : '#b8bb26' }}>●</span> {translateProvider}</span> |
There was a problem hiding this comment.
Add accessible label for the engine availability indicator.
The colored dot uses color alone to convey whether the translation engine is available (red/green), which violates WCAG 2.1 SC 1.4.1 (Use of Color). Screen reader users will hear "black circle" without status context.
Consider adding an aria-label or adjacent text:
♿ Proposed accessibility fix
-<span><strong>{dubLang}</strong> · {dubLangCode} · {translateQuality} · <span style={{ color: activeEngineUnavailable ? '`#fb4934`' : '`#b8bb26`' }}>●</span> {translateProvider}</span>
+<span><strong>{dubLang}</strong> · {dubLangCode} · {translateQuality} · <span style={{ color: activeEngineUnavailable ? '`#fb4934`' : '`#b8bb26`' }} aria-label={activeEngineUnavailable ? 'Engine unavailable' : 'Engine ready'}>●</span> {translateProvider}</span>Alternatively, replace the inline style with a CSS class that includes text alternatives.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <span><strong>{dubLang}</strong> · {dubLangCode} · {translateQuality} · <span style={{ color: activeEngineUnavailable ? '#fb4934' : '#b8bb26' }}>●</span> {translateProvider}</span> | |
| <span><strong>{dubLang}</strong> · {dubLangCode} · {translateQuality} · <span style={{ color: activeEngineUnavailable ? '`#fb4934`' : '`#b8bb26`' }} aria-label={activeEngineUnavailable ? 'Engine unavailable' : 'Engine ready'}>●</span> {translateProvider}</span> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/pages/DubTab.jsx` at line 590, The colored dot span that
currently renders "●" (inside the same expression using activeEngineUnavailable)
must include an accessible status label; update that span (the one with style={{
color: activeEngineUnavailable ? '`#fb4934`' : '`#b8bb26`' }}) to provide an
aria-label (e.g. aria-label={activeEngineUnavailable ? "Translation engine
unavailable" : "Translation engine available"}) and role="img", or alternatively
insert a visually-hidden text node next to the dot conveying the same status;
ensure you use the existing activeEngineUnavailable boolean and keep the visible
color behavior unchanged.
| getItem(key) { | ||
| return store[key] || null; | ||
| }, |
There was a problem hiding this comment.
getItem returns wrong value for empty-string entries.
store[key] || null treats "" as missing. localStorage.getItem() should return the stored string exactly when the key exists.
Suggested fix
getItem(key) {
- return store[key] || null;
+ return Object.prototype.hasOwnProperty.call(store, key)
+ ? store[key]
+ : null;
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| getItem(key) { | |
| return store[key] || null; | |
| }, | |
| getItem(key) { | |
| return Object.prototype.hasOwnProperty.call(store, key) | |
| ? store[key] | |
| : null; | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/test/setup.js` around lines 6 - 8, The getItem method
incorrectly treats stored empty strings as missing because it uses `store[key]
|| null`; update getItem (referencing the getItem function and the store object)
to return the exact stored value when the key exists (e.g., check `store` for
the key using `hasOwnProperty` or `key in store`) and only return null when the
key is truly absent, ensuring an empty string is returned unchanged.
f5af607 to
c33e88d
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
backend/services/asr_backend.py (1)
651-662:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMoonshine fallback only handles import failures, not runtime failures.
At line 653, if
moonshine_onnx.transcribe()raises a runtime exception (e.g., model loading failure, audio format error), the error propagates instead of falling back tomoonshine_voice. Theexcept ImportErrorblock at line 656 only catches import failures.Widen the exception handler to catch runtime failures from
moonshine_onnx.transcribe()as well:Proposed fix
# Try moonshine_onnx first (lighter), then moonshine_voice + text = None try: import moonshine_onnx text = moonshine_onnx.transcribe(audio_path, model=self._model_name) if isinstance(text, list): text = " ".join(text) - except ImportError: + except Exception as e: + logger.debug("moonshine_onnx unavailable or failed (%s), trying moonshine_voice", e) + + if text is None: from moonshine_voice import Transcriber if self._transcriber is None: self._transcriber = Transcriber(model=self._model_name) text = self._transcriber.transcribe_file(audio_path) if isinstance(text, list): text = " ".join(text)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/asr_backend.py` around lines 651 - 662, The current try/except only catches ImportError so runtime exceptions from moonshine_onnx.transcribe(...) will not fall back to moonshine_voice; change the handler so that the block attempting moonshine_onnx.transcribe(audio_path, model=self._model_name) rescues broader exceptions (e.g., Exception) and then performs the same fallback flow: instantiate or reuse self._transcriber = Transcriber(model=self._model_name) from moonshine_voice and call self._transcriber.transcribe_file(audio_path), joining list outputs into a string as currently done; ensure you still only import moonshine_onnx in the try and keep the same variables (audio_path, self._model_name, self._transcriber, moonshine_voice.Transcriber) to locate the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/api/routers/sonitranslate.py`:
- Around line 65-71: The DubRequest model allows invalid max_speakers (0 or
negative) which leads to a 500 from the sidecar; add validation to reject values
<1 before the downstream call by either adding a Pydantic validator on
DubRequest.max_speakers (ensure value >= 1 and raise ValueError) or by checking
the request in the API handler that processes DubRequest and raising
fastapi.HTTPException(status_code=422) if max_speakers < 1, and perform this
check prior to entering the try block that calls the sidecar so invalid inputs
return 422 instead of causing a downstream 500.
In `@backend/config/models.yaml`:
- Around line 107-117: The default repo_id used by MoonshineASRBackend
("moonshine/base") does not match the catalog entries
("UsefulSensors/moonshine-base" and "UsefulSensors/moonshine-small"), so update
MoonshineASRBackend to use the full HuggingFace repo_id format (e.g.,
"UsefulSensors/moonshine-base" or "UsefulSensors/moonshine-small") as the
default, or add clear documentation in the MoonshineASRBackend configuration
explaining that shortened aliases are not supported and the full
"UsefulSensors/..." repo_id must be provided; locate the default in the
MoonshineASRBackend class/constructor and change the default value or its
docstring accordingly.
In `@backend/services/sonitranslate.py`:
- Around line 136-140: The current fallback assigns python = sys.executable when
is_venv_ready() is false, which causes the sidecar to run in the backend
interpreter; instead, change the logic in the startup path that sets the python
executable (the python variable built from SONI_VENV / "bin" / "python") so that
if is_venv_ready() returns False you raise a clear RuntimeError (or similar
install/repair exception) indicating the virtualenv is not ready and instructing
to run the install/repair flow, rather than falling back to sys.executable or
any global interpreter; update references to is_venv_ready(), python, and
SONI_VENV to reflect this behavior so app_rvc.py is never launched outside the
intended venv.
- Around line 130-166: Concurrent calls can race on the shared _proc handle
causing multiple sidecars to be started; introduce a module-level asyncio.Lock
(e.g., _lifecycle_lock = asyncio.Lock()) and acquire it in start(), stop(), and
dub_video() before checking or mutating _proc, then re-check the running state
inside the lock and perform the start/stop/dub actions only if still needed;
release the lock after the transition completes (use async with _lifecycle_lock:
or try/finally) and preserve existing behavior for error handling and logging.
- Around line 157-164: The loop in start() can raise AttributeError because
is_running() may clear the shared _proc before the startup failure check
accesses _proc.poll() or _proc.stdout; fix by capturing a local reference to the
child process at the start of start()'s retry loop (e.g., proc = self._proc) and
use that local proc when calling proc.poll() and reading proc.stdout, or
alternatively change is_running() so it does not mutate/clear _proc during
checks; ensure logger.info("SoniTranslate started successfully") and return
{"started": True, **status()} still use the authoritative state from status()
while the early-exit handling uses the preserved local proc handle to surface
the real subprocess error.
---
Duplicate comments:
In `@backend/services/asr_backend.py`:
- Around line 651-662: The current try/except only catches ImportError so
runtime exceptions from moonshine_onnx.transcribe(...) will not fall back to
moonshine_voice; change the handler so that the block attempting
moonshine_onnx.transcribe(audio_path, model=self._model_name) rescues broader
exceptions (e.g., Exception) and then performs the same fallback flow:
instantiate or reuse self._transcriber = Transcriber(model=self._model_name)
from moonshine_voice and call self._transcriber.transcribe_file(audio_path),
joining list outputs into a string as currently done; ensure you still only
import moonshine_onnx in the try and keep the same variables (audio_path,
self._model_name, self._transcriber, moonshine_voice.Transcriber) to locate the
code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 157448d6-4777-4b9a-a7fd-a931911e9fc4
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
.gitignorebackend/api/routers/dub_generate.pybackend/api/routers/sonitranslate.pybackend/api/routers/system.pybackend/config/models.yamlbackend/main.pybackend/services/asr_backend.pybackend/services/sonitranslate.pyfrontend/src/components/DubSegmentRow.cssfrontend/src/components/DubSegmentTable.jsxfrontend/src/pages/DubTab.cssfrontend/src/pages/DubTab.jsxfrontend/src/test/setup.jspyproject.toml
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- frontend/src/test/setup.js
🚧 Files skipped from review as they are similar to previous changes (7)
- pyproject.toml
- frontend/src/components/DubSegmentTable.jsx
- backend/main.py
- frontend/src/pages/DubTab.jsx
- backend/api/routers/system.py
- frontend/src/components/DubSegmentRow.css
- frontend/src/pages/DubTab.css
| class DubRequest(BaseModel): | ||
| video_path: str | ||
| target_language: str = "Spanish (es)" | ||
| source_language: str = "Automatic detection" | ||
| tts_voice: str = "es-ES-AlvaroNeural-Male" | ||
| max_speakers: int = 1 | ||
| output_dir: Optional[str] = None |
There was a problem hiding this comment.
Reject invalid speaker counts before calling the sidecar.
max_speakers is unvalidated here, but the service always sends min_speakers=1. A request with max_speakers=0 or a negative value becomes an invalid downstream call and then gets wrapped as a 500. Validate it before entering the try block so the API returns a 422 instead.
✅ Minimal fix
`@router.post`("/dub")
async def sonitranslate_dub(body: DubRequest):
"""Run full dubbing pipeline via SoniTranslate.
@@
Transcribes, translates, generates TTS, and mixes audio.
Returns the path to the dubbed output video.
"""
+ if body.max_speakers < 1:
+ raise HTTPException(status_code=422, detail="max_speakers must be >= 1")
+
try:
result = await soni.dub_video(
video_path=body.video_path,
target_language=body.target_language,
source_language=body.source_language,Also applies to: 74-89
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/api/routers/sonitranslate.py` around lines 65 - 71, The DubRequest
model allows invalid max_speakers (0 or negative) which leads to a 500 from the
sidecar; add validation to reject values <1 before the downstream call by either
adding a Pydantic validator on DubRequest.max_speakers (ensure value >= 1 and
raise ValueError) or by checking the request in the API handler that processes
DubRequest and raising fastapi.HTTPException(status_code=422) if max_speakers <
1, and perform this check prior to entering the try block that calls the sidecar
so invalid inputs return 422 instead of causing a downstream 500.
| - repo_id: "UsefulSensors/moonshine-base" | ||
| label: "Moonshine base (edge-optimized, 61M, ONNX)" | ||
| role: ASR | ||
| size_gb: 0.12 | ||
| note: "Variable-length processing, sub-200ms latency. Great for CPU/edge. Requires moonshine-onnx." | ||
|
|
||
| - repo_id: "UsefulSensors/moonshine-small" | ||
| label: "Moonshine small (edge-optimized, 300M, ONNX)" | ||
| role: ASR | ||
| size_gb: 0.6 | ||
| note: "Higher accuracy than base, still fast. Requires moonshine-onnx." |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the correct HuggingFace repo_id for UsefulSensors Moonshine ASR models?
💡 Result:
The Moonshine ASR models released by Useful Sensors are hosted on Hugging Face under the organization account UsefulSensors [1][2][3]. While there is a repository named UsefulSensors/moonshine [4][1], the specific model checkpoints compatible with the Hugging Face Transformers library are typically accessed via individual repositories for each model size [2][3][5]. The primary repositories for these models are: - UsefulSensors/moonshine-base [2][5] - UsefulSensors/moonshine-tiny [3][5] Additionally, newer streaming variants, such as UsefulSensors/moonshine-streaming-medium, are also available under the same organization [6]. Note that while the organization name UsefulSensors is used on Hugging Face, the company has also operated under the name Moonshine AI [7][8]. Documentation and official repositories may sometimes refer to these models collectively or point to the organization's Hugging Face page for access to various checkpoints [8].
Citations:
- 1: https://huggingface.co/UsefulSensors/moonshine
- 2: https://huggingface.co/UsefulSensors/moonshine-base
- 3: https://huggingface.co/UsefulSensors/moonshine-tiny
- 4: https://huggingface.co/UsefulSensors/moonshine/blob/main/README.md
- 5: https://huggingface.co/docs/transformers/v5.3.0/en/model_doc/moonshine
- 6: https://huggingface.co/UsefulSensors/moonshine-streaming-medium
- 7: https://usefulsensors.com/
- 8: https://github.com/moonshine-ai/moonshine
🏁 Script executed:
fd -t f "asr_backend.py" --exec cat -n {} \;Repository: debpalash/OmniVoice-Studio
Length of output: 36100
Moonshine repo_id in backend default does not match catalog entries.
The catalog lists UsefulSensors/moonshine-base and UsefulSensors/moonshine-small (confirmed as the official HuggingFace organization), but MoonshineASRBackend defaults to moonshine/base at line 624. This mismatch could cause issues if the moonshine library doesn't accept the shortened format as an alias for the full HuggingFace repo_id. Align the backend default with the catalog entries or document the expected repo_id format.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/config/models.yaml` around lines 107 - 117, The default repo_id used
by MoonshineASRBackend ("moonshine/base") does not match the catalog entries
("UsefulSensors/moonshine-base" and "UsefulSensors/moonshine-small"), so update
MoonshineASRBackend to use the full HuggingFace repo_id format (e.g.,
"UsefulSensors/moonshine-base" or "UsefulSensors/moonshine-small") as the
default, or add clear documentation in the MoonshineASRBackend configuration
explaining that shortened aliases are not supported and the full
"UsefulSensors/..." repo_id must be provided; locate the default in the
MoonshineASRBackend class/constructor and change the default value or its
docstring accordingly.
| async def start() -> dict: | ||
| """Start the SoniTranslate Gradio server as a subprocess.""" | ||
| global _proc | ||
| if is_running(): | ||
| return {"started": False, "reason": "already_running", **status()} | ||
|
|
||
| if not is_installed(): | ||
| raise RuntimeError("SoniTranslate not installed. Call /engines/sonitranslate/install first.") | ||
|
|
||
| python = str(SONI_VENV / "bin" / "python") if is_venv_ready() else sys.executable | ||
|
|
||
| # Pass HF token from OmniVoice environment | ||
| env = os.environ.copy() | ||
| hf_token = os.environ.get("HF_TOKEN", "") | ||
| if hf_token: | ||
| env["YOUR_HF_TOKEN"] = hf_token | ||
|
|
||
| logger.info("Starting SoniTranslate on port %d...", SONI_PORT) | ||
| _proc = subprocess.Popen( | ||
| [python, "app_rvc.py"], | ||
| cwd=str(SONI_DIR), | ||
| env=env, | ||
| stdout=subprocess.PIPE, | ||
| stderr=subprocess.STDOUT, | ||
| ) | ||
|
|
||
| # Wait up to 30s for it to be ready | ||
| for _ in range(60): | ||
| await asyncio.sleep(0.5) | ||
| if is_running(): | ||
| logger.info("SoniTranslate started successfully") | ||
| return {"started": True, **status()} | ||
| if _proc.poll() is not None: | ||
| out = _proc.stdout.read().decode()[-500:] if _proc.stdout else "" | ||
| raise RuntimeError(f"SoniTranslate exited early: {out}") | ||
|
|
||
| raise RuntimeError("SoniTranslate failed to start within 30s") |
There was a problem hiding this comment.
Serialize sidecar lifecycle transitions.
start(), stop(), and dub_video() all race on the shared _proc handle. Two concurrent /start or /dub requests can both observe “not running” before /info is healthy and launch separate sidecars on the same port, with the later process overwriting _proc. Guard lifecycle transitions with a module-level asyncio.Lock and re-check state inside the lock.
Also applies to: 169-182, 197-199
🧰 Tools
🪛 Ruff (0.15.12)
[error] 148-148: subprocess call: check for execution of untrusted input
(S603)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/sonitranslate.py` around lines 130 - 166, Concurrent calls
can race on the shared _proc handle causing multiple sidecars to be started;
introduce a module-level asyncio.Lock (e.g., _lifecycle_lock = asyncio.Lock())
and acquire it in start(), stop(), and dub_video() before checking or mutating
_proc, then re-check the running state inside the lock and perform the
start/stop/dub actions only if still needed; release the lock after the
transition completes (use async with _lifecycle_lock: or try/finally) and
preserve existing behavior for error handling and logging.
| if not is_installed(): | ||
| raise RuntimeError("SoniTranslate not installed. Call /engines/sonitranslate/install first.") | ||
|
|
||
| python = str(SONI_VENV / "bin" / "python") if is_venv_ready() else sys.executable | ||
|
|
There was a problem hiding this comment.
Do not fall back to the backend interpreter for sidecar startup.
When is_venv_ready() is false, Line 139 runs app_rvc.py with sys.executable. That turns a partial install into “start SoniTranslate inside the OmniVoice backend env”, which defeats the isolation this module advertises and makes failures depend on whatever packages happen to be installed globally. Return a clear repair/install error here instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/sonitranslate.py` around lines 136 - 140, The current
fallback assigns python = sys.executable when is_venv_ready() is false, which
causes the sidecar to run in the backend interpreter; instead, change the logic
in the startup path that sets the python executable (the python variable built
from SONI_VENV / "bin" / "python") so that if is_venv_ready() returns False you
raise a clear RuntimeError (or similar install/repair exception) indicating the
virtualenv is not ready and instructing to run the install/repair flow, rather
than falling back to sys.executable or any global interpreter; update references
to is_venv_ready(), python, and SONI_VENV to reflect this behavior so app_rvc.py
is never launched outside the intended venv.
| for _ in range(60): | ||
| await asyncio.sleep(0.5) | ||
| if is_running(): | ||
| logger.info("SoniTranslate started successfully") | ||
| return {"started": True, **status()} | ||
| if _proc.poll() is not None: | ||
| out = _proc.stdout.read().decode()[-500:] if _proc.stdout else "" | ||
| raise RuntimeError(f"SoniTranslate exited early: {out}") |
There was a problem hiding this comment.
Preserve the child handle while checking for early startup failure.
If the sidecar exits during startup, is_running() clears _proc, and the following _proc.poll() / _proc.stdout access raises AttributeError instead of surfacing the real subprocess failure. Keep a local proc reference inside start() or stop mutating _proc from is_running().
🐛 Minimal fix
async def start() -> dict:
"""Start the SoniTranslate Gradio server as a subprocess."""
global _proc
@@
_proc = subprocess.Popen(
[python, "app_rvc.py"],
cwd=str(SONI_DIR),
env=env,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
)
# Wait up to 30s for it to be ready
+ proc = _proc
for _ in range(60):
await asyncio.sleep(0.5)
if is_running():
logger.info("SoniTranslate started successfully")
return {"started": True, **status()}
- if _proc.poll() is not None:
- out = _proc.stdout.read().decode()[-500:] if _proc.stdout else ""
+ if proc is not None and proc.poll() is not None:
+ out = proc.stdout.read().decode()[-500:] if proc.stdout else ""
+ _proc = None
raise RuntimeError(f"SoniTranslate exited early: {out}")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/services/sonitranslate.py` around lines 157 - 164, The loop in
start() can raise AttributeError because is_running() may clear the shared _proc
before the startup failure check accesses _proc.poll() or _proc.stdout; fix by
capturing a local reference to the child process at the start of start()'s retry
loop (e.g., proc = self._proc) and use that local proc when calling proc.poll()
and reading proc.stdout, or alternatively change is_running() so it does not
mutate/clear _proc during checks; ensure logger.info("SoniTranslate started
successfully") and return {"started": True, **status()} still use the
authoritative state from status() while the early-exit handling uses the
preserved local proc handle to surface the real subprocess error.
1 participant
Summary
Multi-area stability pass touching dub pipeline UI, diarization handling, production deployment, and sonitranslate engine plumbing. Single commit (`f5af607`) from 3 days ago.
Files changed (15)
Backend:
Frontend (dub editor):
Infra:
This is a single 890-line commit across mixed concerns (dub UI + diarization + production deploy + sonitranslate). Reviewer may want to either:
CodeRabbit will surface specific issues per-file.
Test plan
Scope vs milestone
Touches stability areas in v0.3.x scope (dubbing pipeline = Wave 2 stability), but pre-dates the GSD planning. Bundled as a single commit by intent.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Style
Tests
Chores