feat: onboarding demos, opt-in bug reporting, error-docs deeplinks + issue triage - #133
Merged
Merged
Conversation
…issue triage Working-tree snapshot bundling several in-flight workstreams (v0.3.0): - Onboarding/demo system: DemoPresetGrid, DictationDemo, DubbingDemo components + tests, render scripts (render_demos_omnivoice.py, build_demos.sh, build_dub_demo.sh), personalities preview URLs, alembic 0002 voice-profile demo fields. - Opt-in bug reporting: ReportBugButton (prefilled GitHub-issue URL path). - Error transparency UX: errorDocsMap deeplinks + BootstrapSplash/error wiring. - Dub workspace: DubSegmentRow/Table, WaveformTimeline, dubSlice tweaks. - Issue triage: .planning/issue-clusters/ (plan-01..05 root-cause masters, GH #128-#132). - CLAUDE.md: hard rule — everything ships on v0.3.0, no version bumps. KNOWN GAP (why this is a draft): the generated demo audio assets are NOT in this tree, and backend/assets/samples/demo_voice.wav is deleted. onboarding.py guards the missing file (skips seeding the demo profile with a warning), so no crash — but first-run Launchpad will be empty and /demo_audio/ preview URLs 404 until assets are regenerated via scripts/build_demos.sh. Do not merge before regenerating + committing the demo assets. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThis PR introduces v0.3.0 demo assets and voice design presets, implements dub timing strategies with video stretching, adds pipeline streaming/progress tracking, creates demo walkthroughs for dictation and dubbing, improves installer bootstrap resilience, and expands error diagnostics. Includes backend scripts, schemas, migrations, frontend UI components, styling, tests, and planning documents. Changesv0.3.0 Planning Documents
Demo Audio Asset Build and Serving
Voice Design Demo Presets and UI
Dub Timing Strategy and Video Stretch Export
Dub Pipeline Streaming, Progress, and Prediction
Dub UI Segment Tracking and Controls
Dictation and Dubbing Demo Walkthroughs
Installer Bootstrap Network Resilience
Error Handling, Diagnostics, and Engine Matrix UX
Conventions, Dependencies, System Env, and Tauri Recovery
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Owner
Author
|
@greptileai scan this check for issues in various dimensions |
Contributor
|
| Filename | Overview |
|---|---|
| backend/schemas/requests.py | Adds timing_strategy + overflow_budget_s to DubRequest; TranslateSegment still missing slot_seconds declaration (previously flagged), making the new rate_ratio prediction in dub_translate.py silently inactive. |
| backend/api/routers/dub_generate.py | Adds three-mode timing strategy (concise/stretch_video/strict_slot), pitch-preserving ffmpeg atempo stretch, per-segment fit_status tracking, and video stretch plan persistence. Core logic is sound; the blocking subprocess.run() in _pitch_preserving_stretch (already flagged) remains. |
| backend/api/routers/dub_export.py | Adds _build_video_stretch_filter_graph (per-segment setpts filter chain) and _video_stretch_plan_for for stretch_video export; correctly disables subtitle burn when stretch is active; fixes explicit MIME type for dub media endpoint. |
| backend/services/dub_pipeline.py | Adds run_proc_streaming_stderr, download progress hooks, browser-playability transcoding, and improved yt-dlp subtitle handling. stdout=PIPE opened but never drained (safe for demucs, latent risk for future callers); progress_hooks spread into sub_opts causes subtitle progress noise in the UI bar. |
| backend/api/routers/dub_translate.py | Adds rate_ratio pre-generation prediction in _maybe_cinematic; prediction is always skipped because TranslateSegment.slot_seconds is not declared and Pydantic silently drops the field sent by the frontend. |
| frontend/src/pages/DubTab.jsx | Adds DubbingDemo idle-state widget (dismissible via localStorage), PrepOverlay progress bar with elapsed/speed/ETA, timing strategy segmented control, and compression warning banner. UI logic looks correct. |
| frontend/src/hooks/useDubWorkflow.js | Wires download/demucs progress events to dubPrepProgress state, carries rate_ratio and fit_status from translate/generate responses onto segment records, and passes timingStrategy to generate call. Logic looks correct. |
| frontend/src/components/ReportBugButton.jsx | Implements opt-in prefilled GitHub Issues reporter per CLAUDE.md Capability 2. Strips home paths, captures OS/Python/device/engine info, opens via shell.open. Correct implementation. |
| backend/core/personalities.py | Adds 7 demo personality presets with is_demo:True markers, preview_url paths, and character scripts. Preview URLs will 404 until demo assets are regenerated (acknowledged known gap). |
| backend/core/onboarding.py | Adds _backfill_demo_metadata for v0.2.x→v0.3.0 upgrade path; updates demo profile text and INSERT to include description/is_demo columns. Runs on every startup (previously flagged concern), but is otherwise safe. |
Sequence Diagram
sequenceDiagram
participant FE as Frontend
participant BP as dub_pipeline
participant BG as dub_generate
participant BE as dub_export
FE->>BP: POST /dub/ingest (URL or file)
BP-->>FE: SSE: download_start
BP-->>FE: SSE: download_progress (percent, speed, eta)
Note over BP: yt_download_sync + _ensure_browser_playable_mp4
BP-->>FE: SSE: demucs_start
BP-->>FE: SSE: demucs_progress (percent via tqdm)
BP-->>FE: SSE: ready (job_id)
FE->>BG: POST /dub/translate (segments + slot_seconds)
Note over BG: _maybe_cinematic stamps rate_ratio
BG-->>FE: translated rows + rate_ratio (silently missing)
FE->>BG: "POST /dub/generate/{job_id} (timing_strategy)"
Note over BG: concise / stretch_video / strict_slot mix loop
BG-->>FE: SSE: done (fit_status[], video_stretch_plans)
FE->>BE: "GET /dub/download/{job_id}"
Note over BE: _build_video_stretch_filter_graph if stretch_video
BE-->>FE: dubbed mp4
Reviews (5): Last reviewed commit: "fix(#133): bug-report diagnostics field ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+46
to
+88
|
|
||
| ffmpeg's atempo filter is limited to [0.5, 2.0] per stage. Chaining | ||
| multiple stages multiplies the effective ratio while keeping each | ||
| individual stage inside the well-behaved range. Pitch is preserved | ||
| (WSOLA-style time-domain stretching). ratio > 1 speeds up, < 1 | ||
| slows down. | ||
| """ | ||
| stages: list[str] = [] | ||
| remaining = ratio | ||
| while remaining > 2.0: | ||
| stages.append("atempo=2.0") | ||
| remaining /= 2.0 | ||
| while remaining < 0.5: | ||
| stages.append("atempo=0.5") | ||
| remaining /= 0.5 | ||
| stages.append(f"atempo={remaining:.6f}") | ||
| return ",".join(stages) | ||
|
|
||
|
|
||
| def _pitch_preserving_stretch( | ||
| wav: torch.Tensor, target_samples: int, sr: int, | ||
| ) -> torch.Tensor: | ||
| """Time-stretch a (1, samples) tensor to `target_samples` while | ||
| preserving pitch, by piping the audio through `ffmpeg atempo`. | ||
|
|
||
| Returns a (1, target_samples) tensor on the same device as input. | ||
| Raises RuntimeError when ffmpeg fails — callers should fall back to | ||
| naive linear interpolation, accepting the pitch shift, to ensure the | ||
| output isn't silent. | ||
| """ | ||
| wl = int(wav.shape[-1]) | ||
| if target_samples <= 0 or wl == target_samples: | ||
| return wav | ||
| ratio = wl / target_samples | ||
| filter_str = _atempo_chain(ratio) | ||
|
|
||
| # Mono float32 via stdin → ffmpeg → stdout. One subprocess per | ||
| # segment is ~50-100 ms overhead, dwarfed by TTS generation. | ||
| arr = wav.detach().cpu().to(torch.float32).numpy().reshape(-1).astype(np.float32, copy=False) | ||
| proc = subprocess.run( | ||
| [ | ||
| find_ffmpeg(), "-hide_banner", "-loglevel", "error", "-y", | ||
| "-f", "f32le", "-ar", str(sr), "-ac", "1", "-i", "pipe:0", |
Contributor
There was a problem hiding this comment.
subprocess.run() inside async endpoint
_pitch_preserving_stretch calls subprocess.run() synchronously, which blocks the asyncio event loop for the entire duration of each ffmpeg invocation. Unlike the previous torch.nn.functional.interpolate (a fast C-extension call measured in µs), each subprocess call takes ~50–100 ms per the comment. On a 100-segment dub job that's 5–10 s of event loop freeze, during which health-check polls, status SSE streams, and every other concurrent API request stalls. The fix is to replace subprocess.run with asyncio.create_subprocess_exec + await proc.communicate() (mirrors the pattern already used in run_proc_streaming_stderr in dub_pipeline.py).
Comment on lines
+33
to
+60
| DESIGN_DIR="${SAMPLES_DIR}/voice_design" | ||
| DICT_DIR="${SAMPLES_DIR}/dictation" | ||
|
|
||
| ENGINE="say" | ||
| SKIP_EXISTING=0 | ||
|
|
||
| while [ $# -gt 0 ]; do | ||
| case "$1" in | ||
| --engine) ENGINE="$2"; shift 2 ;; | ||
| --skip-existing) SKIP_EXISTING=1; shift ;; | ||
| --help|-h) | ||
| sed -n '/^#/p' "$0" | head -40 | ||
| exit 0 ;; | ||
| *) echo "Unknown arg: $1" >&2; exit 2 ;; | ||
| esac | ||
| done | ||
|
|
||
| if [ "$ENGINE" = "omnivoice" ]; then | ||
| # Delegate to the Python script that talks to the real engine. | ||
| PY_ARGS="" | ||
| [ "$SKIP_EXISTING" = 1 ] && PY_ARGS="--skip-existing" | ||
| echo "Rendering cloning + voice-design demos via OmniVoice engine…" | ||
| if [ -d "${REPO_ROOT}/.venv" ]; then | ||
| "${REPO_ROOT}/.venv/bin/python" "${REPO_ROOT}/scripts/render_demos_omnivoice.py" $PY_ARGS | ||
| else | ||
| echo "WARN: .venv missing; trying system python3" >&2 | ||
| python3 "${REPO_ROOT}/scripts/render_demos_omnivoice.py" $PY_ARGS | ||
| fi |
Contributor
There was a problem hiding this comment.
say path is macOS-only with no cross-platform fallback
The script defaults to Apple's say command, which doesn't exist on Linux or Windows. Running scripts/build_demos.sh (without --engine omnivoice) on any non-macOS CI/CD runner or contributor machine exits immediately on the first say call because set -e is active. The header comment says "macOS say (default)" but the error is silent on Linux (just command not found). A minimal guard at the top would save confusion: detect which say and either warn and exit cleanly, or auto-delegate to the --engine omnivoice path when say is absent. This matters because the PR block says "Before merge: regenerate via scripts/build_demos.sh" — any CI that runs this on a Linux host will produce zero assets.
debpalash
marked this pull request as ready for review
May 28, 2026 02:43
Owner
Author
|
@greptileai ready for merge? |
Contributor
|
Not yet. There's a hard blocker called out in the PR description itself: Missing demo audio assets —
Checklist before merge:
The code itself is solid; this is purely a missing artifact issue. Once the assets are committed and CI passes, it should be good to go. Tip: You can customize Greptile's behavior for this repo with |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
frontend/src-tauri/src/bootstrap.rs (2)
384-393:⚠️ Potential issue | 🟠 Major | ⚡ Quick winApply the same UV network env defaults in the repair sync path
In
frontend/src-tauri/src/bootstrap.rs(lines 384-393), therepair_cmd(uv sync) removes a few env vars but skipsapply_uv_network_env, so the restricted/flaky-network mirror/timeout/retry hardening added for the primary sync path isn’t applied during repairs.Suggested fix
let mut repair_cmd = Command::new(&uv_path); repair_cmd.env_remove("PYTHONHOME").env_remove("PYTHONPATH").env_remove("LD_LIBRARY_PATH"); + let effective_region = get_effective_region(app); + apply_uv_network_env(&mut repair_cmd, &effective_region); let has_lockfile = project_dir.join("uv.lock").is_file();🤖 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-tauri/src/bootstrap.rs` around lines 384 - 393, The repair path creates repair_cmd but doesn't apply the same network environment hardening; call apply_uv_network_env(&mut repair_cmd) after creating/configuring repair_cmd (before run_streaming) so the UV mirror/timeout/retry settings are applied to the repair sync as they are to the primary sync; update the block that builds repair_cmd (the repair_cmd variable in bootstrap.rs) to invoke apply_uv_network_env prior to run_streaming("installing_deps", &mut repair_cmd).
525-527:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSwitch China mirror routing from
UV_INDEX_URLtoUV_DEFAULT_INDEX(and don’t override user config).
bootstrap.rsstill setsUV_INDEX_URLforeffective_region == "china"(hardcoded to the Aliyun URL), but repo docs instruct usingUV_DEFAULT_INDEXfor China.uvtreatsUV_INDEX_URLas a legacy/deprecated alias for the default index; switching avoids drift and lets users keep their ownUV_DEFAULT_INDEXvalue.Suggested fix
- if effective_region == "china" { - sync_cmd.env("UV_INDEX_URL", "https://mirrors.aliyun.com/pypi/simple/"); - } + if effective_region == "china" && std::env::var_os("UV_DEFAULT_INDEX").is_none() { + sync_cmd.env("UV_DEFAULT_INDEX", "https://pypi.tuna.tsinghua.edu.cn/simple"); + }🤖 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-tauri/src/bootstrap.rs` around lines 525 - 527, The code sets the China PyPI mirror using sync_cmd.env("UV_INDEX_URL", ...) which is deprecated and also unconditionally overrides any user setting; change this to set "UV_DEFAULT_INDEX" instead and only set it when no user-provided UV_DEFAULT_INDEX is present (i.e., check existing env/config before calling sync_cmd.env), keeping the same Aliyun URL value and updating the symbol from UV_INDEX_URL to UV_DEFAULT_INDEX while preserving the conditional on effective_region == "china".scripts/install.sh (1)
324-350:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSet
UV_DEFAULT_INDEXfor China beforeuv sync
scripts/install.shconfiguresUV_PYTHON_INSTALL_MIRRORplus HTTP timeout/retries for restricted networks, but it doesn’t set a PyPI index mirror;uv syncstill uses the default PyPI index unlessUV_DEFAULT_INDEX(orUV_INDEX) is set. This contradicts the project’s own restricted-network guidance indocs/install/linux.md, which instructs users to exportUV_DEFAULT_INDEXwhen PyPI access fails.Suggested fix
case "${OMNIVOICE_REGION:-}" in china|russia|restricted) : "${UV_PYTHON_INSTALL_MIRROR:=https://ghproxy.net/https://github.com/astral-sh/python-build-standalone/releases/download}" export UV_PYTHON_INSTALL_MIRROR note "Using ghproxy.net mirror for Python download (OMNIVOICE_REGION=${OMNIVOICE_REGION})" ;; esac +: "${UV_DEFAULT_INDEX:=}" +if [ "${OMNIVOICE_REGION:-}" = "china" ] && [ -z "$UV_DEFAULT_INDEX" ]; then + UV_DEFAULT_INDEX="https://pypi.tuna.tsinghua.edu.cn/simple" + export UV_DEFAULT_INDEX + note "Using Tsinghua PyPI mirror for dependency sync" +fi : "${UV_HTTP_TIMEOUT:=120}" : "${UV_HTTP_RETRIES:=5}" export UV_HTTP_TIMEOUT UV_HTTP_RETRIESfrontend/src/store/dubSlice.ts (1)
29-29:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix
DubPrepStageto include'cached'
DubPrepStageomits'cached', but the code assignssetDubPrepStage('cached'), so the declared type doesn’t match runtime values.💡 Proposed fix
-export type DubPrepStage = 'download' | 'extract' | 'demucs' | 'scene' | null; +export type DubPrepStage = 'download' | 'extract' | 'demucs' | 'scene' | 'cached' | 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/store/dubSlice.ts` at line 29, The DubPrepStage union type currently excludes the 'cached' state but code calls setDubPrepStage('cached'); update the exported type alias DubPrepStage to include 'cached' (i.e., add 'cached' into the union 'download' | 'extract' | 'demucs' | 'scene' | null) so the type matches runtime usage; then run typechecks to ensure no other locations need adjustments where DubPrepStage is used.
🧹 Nitpick comments (8)
scripts/build_demos.sh (1)
69-77: ⚖️ Poor tradeoffmacOS-only dependency without cross-platform fallback.
The script hard-fails if
sayis missing (Lines 69-73) with a TODO comment about espeak-ng for Linux. This blocks Linux contributors from regenerating demos. Consider adding espeak-ng support or documenting that Linux/Windows contributors must use--engine omnivoiceexclusively.🤖 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 `@scripts/build_demos.sh` around lines 69 - 77, The script currently exits if the macOS "say" command is missing, blocking Linux/Windows contributors; update scripts/build_demos.sh to detect and prefer available TTS engines: check for "say" first, then "espeak-ng" (or "espeak") and use the appropriate command invocation, and only fail if neither TTS engine nor the "--engine omnivoice" flag is provided; also update the error message that currently mentions only macOS to suggest installing espeak-ng on Linux or using "--engine omnivoice", and keep the existing ffmpeg check for "ffmpeg".backend/services/asr_backend.py (1)
163-171: ⚡ Quick winAdd debug breadcrumbs and narrow exception catches in safe-global discovery.
At Line 163 and the similar
except Exception: passblocks in this method, failures are fully swallowed. When a new environment breaks one of these probes, diagnostics are lost and the later unpickle error becomes much harder to root-cause. Please log at debug level and narrow catches (e.g.,ImportError/AttributeError) where possible.♻️ Suggested pattern
- except Exception: - pass + except (ImportError, AttributeError) as e: + logger.debug("safe_globals: skipping omegaconf nodes/base probe: %s", e)- except Exception: - pass + except (ImportError, AttributeError) as e: + logger.debug("safe_globals: skipping numpy helper probe for %s: %s", _modname, e)Also applies to: 180-190, 246-259
🤖 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 163 - 171, The code swallows all exceptions in the safe-global discovery probes (e.g., the try that imports enum and extends allow with enum.Enum/IntEnum/Flag/IntFlag and similar blocks at 180-190 and 246-259); change the bare "except Exception: pass" to catch narrower errors (at minimum ImportError and AttributeError) and log the caught exception at debug level using the module logger (e.g., logger.debug or processLogger.debug) so you record breadcrumb details (include the exception message and context like "failed to probe enum" or the specific probe name) while still allowing the discovery to continue..planning/issue-clusters/05-voice-design-instruct-validator.md (1)
16-22: ⚡ Quick winAdd blank line before table.
Markdown tables should be surrounded by blank lines for better readability and to satisfy linting rules.
📝 Proposed fix
4. Ensure language selection doesn't inject an instruct token the validator rejects. ## Test matrix + | Path | Engine | Required behavior |🤖 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 @.planning/issue-clusters/05-voice-design-instruct-validator.md around lines 16 - 22, The Markdown table under the "Test matrix" header lacks a preceding blank line which breaks linting/readability; insert a single blank line between the "## Test matrix" header and the table (the pipe-delimited block starting with "| Path | Engine | Required behavior |") so the table is surrounded by blank lines..planning/issue-clusters/01-windows-model-storage-hf-cache.md (1)
17-23: ⚡ Quick winAdd blank line before table.
Markdown tables should be surrounded by blank lines for better readability and to satisfy linting rules.
📝 Proposed fix
4. Migration: detect an existing populated cache and reuse it (no re-download for current users). ## Test matrix + | OS | Cache location | Symlink support | Required behavior |🤖 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 @.planning/issue-clusters/01-windows-model-storage-hf-cache.md around lines 17 - 23, Insert a blank line immediately before the markdown table that starts with the header row "| OS | Cache location | Symlink support | Required behavior |" (under the "## Test matrix" heading) so the table is separated from the preceding content; update the markdown in .planning/issue-clusters/01-windows-model-storage-hf-cache.md by adding one empty line above that header row to satisfy linting and improve readability..planning/issue-clusters/04-pipeline-error-transparency.md (1)
15-21: ⚡ Quick winAdd blank line before table.
Markdown tables should be surrounded by blank lines for better readability and to satisfy linting rules.
📝 Proposed fix
4. Prevention net: low-info reports (`#63-style`) should be answerable because the app now emits a copyable diagnostic block. ## Test matrix + | Trigger | Required behavior |🤖 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 @.planning/issue-clusters/04-pipeline-error-transparency.md around lines 15 - 21, The markdown under the "## Test matrix" heading lacks a blank line before the table, which breaks linting/readability; fix by inserting a single empty line between the "## Test matrix" heading and the table start (the line beginning with "| Trigger | Required behavior |") so the table is separated from the heading and passes markdown linters..planning/issue-clusters/02-windows-runtime-integrity.md (1)
17-23: ⚡ Quick winAdd blank line before table.
Markdown tables should be surrounded by blank lines for better readability and to satisfy linting rules.
📝 Proposed fix
4. Add an installer smoke test that imports the ASR/TTS critical path on Windows before the build is published. ## Test matrix + | OS | Accel | Required behavior |🤖 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 @.planning/issue-clusters/02-windows-runtime-integrity.md around lines 17 - 23, Insert a blank line between the "## Test matrix" heading and the Markdown table so the table is preceded by an empty line; update the content around the "## Test matrix" header in .planning/issue-clusters/02-windows-runtime-integrity.md to ensure the table starts on its own paragraph (i.e., add one newline after "## Test matrix")..planning/issue-clusters/03-installer-bootstrap-network.md (1)
18-24: ⚡ Quick winAdd blank line before table.
Markdown tables should be surrounded by blank lines for better readability and to satisfy linting rules.
📝 Proposed fix
5. On total failure, the bootstrap surfaces the exact remediation (install python.org Python + the env vars) instead of a raw uv stack trace. ## Test matrix + | Network | System Python | Required behavior |🤖 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 @.planning/issue-clusters/03-installer-bootstrap-network.md around lines 18 - 24, Insert a single blank line between the "## Test matrix" heading and the Markdown table so the table is separated from the heading (i.e., add an empty line before the table starting with "| Network | System Python | Required behavior |") to satisfy linting and improve readability.backend/api/routers/dub_translate.py (1)
428-433: ⚡ Quick winPre-index segments by ID before rate-ratio stamping.
This lookup is currently quadratic (
next(...)inside loop overtranslated). Precompute a map once to keep large jobs responsive.♻️ Proposed refactor
- for row in translated: - seg_ref = next( - (s for s in req.segments if str(s.id) == str(row["id"])), - None, - ) + seg_by_id = {str(s.id): s for s in req.segments} + for row in translated: + seg_ref = seg_by_id.get(str(row["id"]))🤖 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_translate.py` around lines 428 - 433, The loop over translated is doing a quadratic lookup via next(...) against req.segments; pre-index req.segments once into a dict (e.g. seg_by_id = {str(s.id): s for s in req.segments}) before the for row in translated loop, then replace the next(...) call with seg_ref = seg_by_id.get(str(row["id"])) and keep the existing slot = getattr(seg_ref, "slot_seconds", None) if seg_ref else None; this removes the nested scan and makes rate-ratio stamping scale to large jobs.
🤖 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_export.py`:
- Around line 357-359: Replace the direct dict access job["video_path"] with a
safe lookup (video_path = job.get("video_path")) and explicitly handle the
missing-case before calling os.path.exists: if video_path is None or falsy raise
an appropriate HTTPException (e.g., 404 or 400 with "Media file not found" or
"Missing video_path"), otherwise check os.path.exists(video_path) and raise the
existing 404 if the file is absent; update the block around video_path in the
dub_export route/function to avoid KeyError and ensure controlled HTTP
responses.
In `@backend/api/routers/dub_generate.py`:
- Around line 85-98: The ffmpeg call in _pitch_preserving_stretch uses
subprocess.run without a timeout which can hang; add a timeout argument (e.g.,
timeout=30 or use a configurable constant) to the subprocess.run(...) call and
catch subprocess.TimeoutExpired around that call (referencing
_pitch_preserving_stretch and the subprocess.run invocation) to
terminate/cleanup and raise a clear RuntimeError including timeout context;
ensure you still decode proc.stderr when available or include the timeout
message when TimeoutExpired is raised.
In `@backend/core/personalities.py`:
- Around line 85-86: The new preview_url entries in
backend/core/personalities.py point to demo WAVs that don't exist yet, causing
404s; either regenerate and commit the demo audio bundle using
scripts/build_demos.sh so the files referenced by each preview_url are present,
or temporarily remove/disable the preview_url fields (or set them to None) for
the affected presets and/or add a guard that only sets preview_url when the
corresponding WAV exists on disk. Locate the preview_url attributes in the
preset definitions (and the scripts/build_demos.sh workflow) and apply one of
these fixes consistently for all presets listed (lines referenced in the review:
102-103, 121-122, 140-141, 159-160, 178-179, 197-198, 213-214).
In `@frontend/src/components/DictationDemo.jsx`:
- Around line 87-107: The async listener setup in useEffect may assign
unlistenStart/unlistenStop after unmount, leaving listeners active; introduce a
disposed boolean (e.g., let disposed = false) inside the effect and set it true
in the cleanup, then after each await listen(...) check disposed: if disposed
immediately call the returned unlisten function (or skip assigning to
unlistenStart/unlistenStop) so listeners are removed if the component already
unmounted; keep references to the listen import and use isTauri(),
setHotkeyState, listen, unlistenStart and unlistenStop names so the existing
cleanup logic still works when not disposed.
In `@frontend/src/components/ReportBugButton.jsx`:
- Around line 56-59: The fields from the backend are mis-mapped: replace uses of
j.os and j.torch_device/j.gpu with the backend names j.platform and j.device in
ReportBugButton.jsx so diagnostics are populated; specifically update the lines
that push OS and device info (the statements using j.os, j.torch_device, and
j.gpu) to use j.platform and j.device respectively, keeping stripHome around the
device value if needed and leaving j.python as-is.
In `@frontend/src/hooks/useDubWorkflow.js`:
- Around line 348-350: When merging the new translation `hit` into the existing
state `s`, the current line `rate_error: hit.rate_error || s.rate_error`
preserves old errors; change it so a missing/empty `hit.rate_error` clears the
stale error (e.g., use the same null-check pattern as `rate_ratio`: `rate_error:
hit.rate_error != null ? hit.rate_error : null`) so successful re-translations
remove previous `s.rate_error`.
In `@frontend/src/pages/CloneDesignTab.jsx`:
- Around line 82-89: The readiness check uses enginesData even when the useQuery
failed or is loading, causing anyTtsReady (and later showHearDemo logic) to be
false and hide the normal synth button; update the logic that computes
anyTtsReady (and the showHearDemo gating at the other occurrence) to require the
query succeeded (useQuery's isSuccess) before inspecting enginesData (e.g., only
evaluate (enginesData?.tts?.backends || []).some(...) when isSuccess is true) so
failures/loading do not wrongly flip the UI to the fallback path.
In `@frontend/src/pages/DubTab.jsx`:
- Around line 105-108: The state initializer for demoDismissed in the DubTab
component may throw when accessing localStorage (causing the tab to break);
change the initializer to safely attempt the read in a try/catch (and return
false on any error or when window is undefined), mirroring the write path’s
safety—specifically wrap the
localStorage.getItem('omnivoice.dubbingDemoDismissed') call used to set
demoDismissed (and keep using setDemoDismissed elsewhere) so any exception
returns the safe default instead of bubbling up.
In `@scripts/build_demos.sh`:
- Around line 162-164: The spoken transcript in the demo invocation uses the
literal phrase "renderer dot tsx" (the string passed to render in
scripts/build_demos.sh), but the manifest's expected_transcript uses
"renderer.tsx", causing mismatches; update the manifest's expected_transcript
entries that correspond to this demo (the entry matching the render call with
"Patch the WebGPU shader..." / "${DICT_DIR}/en_technical.wav" 16000 and the
other occurrences noted) so the expected_transcript matches the spoken form
"renderer dot tsx" (or alternatively change the render invocation to speak
"renderer.tsx" if you prefer matching the manifest), ensuring the two strings
are identical.
- Around line 170-172: The script writes to "${SAMPLES_DIR}/demo/manifest.json"
but never ensures the demo subdirectory exists; before the cat >
"${SAMPLES_DIR}/demo/manifest.json" call in build_demos.sh, create the directory
using a safe mkdir -p on "${SAMPLES_DIR}/demo" (or equivalent) so the file write
cannot fail; reference the SAMPLES_DIR variable and the manifest write location
when adding the mkdir -p step.
In `@scripts/build_dub_demo.sh`:
- Around line 20-22: The script currently sets OUT_DIR to
"${REPO_ROOT}/backend/assets/demo/dubbing" which doesn't match the `/demo_audio`
mount used by main.py and DubbingDemo.jsx; update the OUT_DIR assignment so it
writes into the samples mount path (e.g.,
"${REPO_ROOT}/backend/assets/samples/demo/dubbing") and keep the existing mkdir
-p "$OUT_DIR" logic so the directory created matches the frontend's expected
/demo_audio/demo/dubbing/manifest.json location.
In `@scripts/render_demos_omnivoice.py`:
- Around line 195-199: The current async-detection logic is inverted: you call
asyncio.get_running_loop(), then raise RuntimeError which you immediately catch
and call asyncio.run(get_model()), causing a confusing "cannot be called from a
running event loop" when a loop actually exists. Change the control flow so that
you attempt asyncio.get_running_loop() and if it succeeds (loop exists) emit
your clear message and exit (or raise) instead of calling asyncio.run; only when
get_running_loop() raises RuntimeError (no running loop) should you call
asyncio.run(get_model()). Update the block around asyncio.get_running_loop() and
asyncio.run(get_model()) accordingly.
---
Outside diff comments:
In `@frontend/src-tauri/src/bootstrap.rs`:
- Around line 384-393: The repair path creates repair_cmd but doesn't apply the
same network environment hardening; call apply_uv_network_env(&mut repair_cmd)
after creating/configuring repair_cmd (before run_streaming) so the UV
mirror/timeout/retry settings are applied to the repair sync as they are to the
primary sync; update the block that builds repair_cmd (the repair_cmd variable
in bootstrap.rs) to invoke apply_uv_network_env prior to
run_streaming("installing_deps", &mut repair_cmd).
- Around line 525-527: The code sets the China PyPI mirror using
sync_cmd.env("UV_INDEX_URL", ...) which is deprecated and also unconditionally
overrides any user setting; change this to set "UV_DEFAULT_INDEX" instead and
only set it when no user-provided UV_DEFAULT_INDEX is present (i.e., check
existing env/config before calling sync_cmd.env), keeping the same Aliyun URL
value and updating the symbol from UV_INDEX_URL to UV_DEFAULT_INDEX while
preserving the conditional on effective_region == "china".
In `@frontend/src/store/dubSlice.ts`:
- Line 29: The DubPrepStage union type currently excludes the 'cached' state but
code calls setDubPrepStage('cached'); update the exported type alias
DubPrepStage to include 'cached' (i.e., add 'cached' into the union 'download' |
'extract' | 'demucs' | 'scene' | null) so the type matches runtime usage; then
run typechecks to ensure no other locations need adjustments where DubPrepStage
is used.
---
Nitpick comments:
In @.planning/issue-clusters/01-windows-model-storage-hf-cache.md:
- Around line 17-23: Insert a blank line immediately before the markdown table
that starts with the header row "| OS | Cache location | Symlink support |
Required behavior |" (under the "## Test matrix" heading) so the table is
separated from the preceding content; update the markdown in
.planning/issue-clusters/01-windows-model-storage-hf-cache.md by adding one
empty line above that header row to satisfy linting and improve readability.
In @.planning/issue-clusters/02-windows-runtime-integrity.md:
- Around line 17-23: Insert a blank line between the "## Test matrix" heading
and the Markdown table so the table is preceded by an empty line; update the
content around the "## Test matrix" header in
.planning/issue-clusters/02-windows-runtime-integrity.md to ensure the table
starts on its own paragraph (i.e., add one newline after "## Test matrix").
In @.planning/issue-clusters/03-installer-bootstrap-network.md:
- Around line 18-24: Insert a single blank line between the "## Test matrix"
heading and the Markdown table so the table is separated from the heading (i.e.,
add an empty line before the table starting with "| Network | System Python |
Required behavior |") to satisfy linting and improve readability.
In @.planning/issue-clusters/04-pipeline-error-transparency.md:
- Around line 15-21: The markdown under the "## Test matrix" heading lacks a
blank line before the table, which breaks linting/readability; fix by inserting
a single empty line between the "## Test matrix" heading and the table start
(the line beginning with "| Trigger | Required behavior |") so the table is
separated from the heading and passes markdown linters.
In @.planning/issue-clusters/05-voice-design-instruct-validator.md:
- Around line 16-22: The Markdown table under the "Test matrix" header lacks a
preceding blank line which breaks linting/readability; insert a single blank
line between the "## Test matrix" header and the table (the pipe-delimited block
starting with "| Path | Engine | Required behavior |") so the table is
surrounded by blank lines.
In `@backend/api/routers/dub_translate.py`:
- Around line 428-433: The loop over translated is doing a quadratic lookup via
next(...) against req.segments; pre-index req.segments once into a dict (e.g.
seg_by_id = {str(s.id): s for s in req.segments}) before the for row in
translated loop, then replace the next(...) call with seg_ref =
seg_by_id.get(str(row["id"])) and keep the existing slot = getattr(seg_ref,
"slot_seconds", None) if seg_ref else None; this removes the nested scan and
makes rate-ratio stamping scale to large jobs.
In `@backend/services/asr_backend.py`:
- Around line 163-171: The code swallows all exceptions in the safe-global
discovery probes (e.g., the try that imports enum and extends allow with
enum.Enum/IntEnum/Flag/IntFlag and similar blocks at 180-190 and 246-259);
change the bare "except Exception: pass" to catch narrower errors (at minimum
ImportError and AttributeError) and log the caught exception at debug level
using the module logger (e.g., logger.debug or processLogger.debug) so you
record breadcrumb details (include the exception message and context like
"failed to probe enum" or the specific probe name) while still allowing the
discovery to continue.
In `@scripts/build_demos.sh`:
- Around line 69-77: The script currently exits if the macOS "say" command is
missing, blocking Linux/Windows contributors; update scripts/build_demos.sh to
detect and prefer available TTS engines: check for "say" first, then "espeak-ng"
(or "espeak") and use the appropriate command invocation, and only fail if
neither TTS engine nor the "--engine omnivoice" flag is provided; also update
the error message that currently mentions only macOS to suggest installing
espeak-ng on Linux or using "--engine omnivoice", and keep the existing ffmpeg
check for "ffmpeg".
🪄 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: 3b68e038-c7fa-4e0a-b044-0feb0a87304d
⛔ Files ignored due to path filters (2)
backend/assets/samples/demo_voice.wavis excluded by!**/*.wavbun.lockis excluded by!**/*.lock
📒 Files selected for processing (58)
.gitignore.planning/issue-clusters/01-windows-model-storage-hf-cache.md.planning/issue-clusters/02-windows-runtime-integrity.md.planning/issue-clusters/03-installer-bootstrap-network.md.planning/issue-clusters/04-pipeline-error-transparency.md.planning/issue-clusters/05-voice-design-instruct-validator.mdCLAUDE.mdbackend/api/routers/dub_export.pybackend/api/routers/dub_generate.pybackend/api/routers/dub_translate.pybackend/api/routers/system.pybackend/core/db.pybackend/core/onboarding.pybackend/core/personalities.pybackend/main.pybackend/migrations/versions/0002_voice_profile_demo_fields.pybackend/services/asr_backend.pybackend/services/dub_pipeline.pybackend/services/speech_rate.pyfrontend/src-tauri/src/bootstrap.rsfrontend/src/components/BootstrapSplash.jsxfrontend/src/components/DemoPresetGrid.cssfrontend/src/components/DemoPresetGrid.jsxfrontend/src/components/DictationDemo.cssfrontend/src/components/DictationDemo.jsxfrontend/src/components/DubSegmentRow.cssfrontend/src/components/DubSegmentRow.jsxfrontend/src/components/DubSegmentTable.cssfrontend/src/components/DubSegmentTable.jsxfrontend/src/components/DubbingDemo.cssfrontend/src/components/DubbingDemo.jsxfrontend/src/components/EngineCompatibilityMatrix.cssfrontend/src/components/EngineCompatibilityMatrix.jsxfrontend/src/components/ReportBugButton.jsxfrontend/src/components/WaveformErrorBoundary.cssfrontend/src/components/WaveformTimeline.jsxfrontend/src/hooks/useDubWorkflow.jsfrontend/src/hooks/useTTS.jsfrontend/src/i18n/locales/en.jsonfrontend/src/index.cssfrontend/src/pages/CloneDesignTab.cssfrontend/src/pages/CloneDesignTab.jsxfrontend/src/pages/DubTab.cssfrontend/src/pages/DubTab.jsxfrontend/src/pages/Settings.jsxfrontend/src/pages/SetupWizard.jsxfrontend/src/store/dubSlice.tsfrontend/src/test/DemoPresetGrid.test.jsxfrontend/src/test/DictationDemo.test.jsxfrontend/src/test/DubbingDemo.test.jsxfrontend/src/test/EngineCompatibilityMatrix.test.jsxfrontend/src/utils/errorDocsMap.test.tsfrontend/src/utils/errorDocsMap.tspackage.jsonscripts/build_demos.shscripts/build_dub_demo.shscripts/install.shscripts/render_demos_omnivoice.py
Comment on lines
+357
to
359
| video_path = job["video_path"] | ||
| if not os.path.exists(video_path): | ||
| raise HTTPException(status_code=404, detail="Media file not found") |
Contributor
There was a problem hiding this comment.
Handle missing video_path without throwing a 500.
Using job["video_path"] can raise KeyError for incomplete/legacy job rows and bypass your intended HTTP error handling. Prefer .get() and return a controlled 404/400.
Suggested fix
- video_path = job["video_path"]
- if not os.path.exists(video_path):
+ video_path = job.get("video_path")
+ if not video_path or not os.path.exists(video_path):
raise HTTPException(status_code=404, detail="Media file not found")🤖 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_export.py` around lines 357 - 359, Replace the direct
dict access job["video_path"] with a safe lookup (video_path =
job.get("video_path")) and explicitly handle the missing-case before calling
os.path.exists: if video_path is None or falsy raise an appropriate
HTTPException (e.g., 404 or 400 with "Media file not found" or "Missing
video_path"), otherwise check os.path.exists(video_path) and raise the existing
404 if the file is absent; update the block around video_path in the dub_export
route/function to avoid KeyError and ensure controlled HTTP responses.
Comment on lines
+85
to
+98
| proc = subprocess.run( | ||
| [ | ||
| find_ffmpeg(), "-hide_banner", "-loglevel", "error", "-y", | ||
| "-f", "f32le", "-ar", str(sr), "-ac", "1", "-i", "pipe:0", | ||
| "-af", filter_str, | ||
| "-f", "f32le", "-ar", str(sr), "-ac", "1", "pipe:1", | ||
| ], | ||
| input=arr.tobytes(), | ||
| capture_output=True, | ||
| ) | ||
| if proc.returncode != 0 or not proc.stdout: | ||
| raise RuntimeError( | ||
| (proc.stderr.decode(errors="replace") or "atempo failed")[:200] | ||
| ) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify subprocess.run calls in this file and whether timeout is specified.
rg -n "subprocess\.run\(" backend/api/routers/dub_generate.py
rg -n "subprocess\.run\([^)]*timeout=" backend/api/routers/dub_generate.pyRepository: debpalash/OmniVoice-Studio
Length of output: 100
Add a timeout to the ffmpeg stretch subprocess.run call
backend/api/routers/dub_generate.py (_pitch_preserving_stretch, around line 85) invokes subprocess.run(..., capture_output=True) without a timeout=... argument, so a hung ffmpeg process can stall dub generation indefinitely.
💡 Proposed fix
- proc = subprocess.run(
- [
- find_ffmpeg(), "-hide_banner", "-loglevel", "error", "-y",
- "-f", "f32le", "-ar", str(sr), "-ac", "1", "-i", "pipe:0",
- "-af", filter_str,
- "-f", "f32le", "-ar", str(sr), "-ac", "1", "pipe:1",
- ],
- input=arr.tobytes(),
- capture_output=True,
- )
+ timeout_s = max(10.0, min(120.0, (target_samples / max(sr, 1)) * 4.0))
+ try:
+ proc = subprocess.run(
+ [
+ find_ffmpeg(), "-hide_banner", "-loglevel", "error", "-y",
+ "-f", "f32le", "-ar", str(sr), "-ac", "1", "-i", "pipe:0",
+ "-af", filter_str,
+ "-f", "f32le", "-ar", str(sr), "-ac", "1", "pipe:1",
+ ],
+ input=arr.tobytes(),
+ capture_output=True,
+ timeout=timeout_s,
+ )
+ except subprocess.TimeoutExpired as e:
+ raise RuntimeError(f"atempo timed out after {timeout_s:.1f}s") from e🧰 Tools
🪛 Ruff (0.15.14)
[error] 85-85: 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/api/routers/dub_generate.py` around lines 85 - 98, The ffmpeg call in
_pitch_preserving_stretch uses subprocess.run without a timeout which can hang;
add a timeout argument (e.g., timeout=30 or use a configurable constant) to the
subprocess.run(...) call and catch subprocess.TimeoutExpired around that call
(referencing _pitch_preserving_stretch and the subprocess.run invocation) to
terminate/cleanup and raise a clear RuntimeError including timeout context;
ensure you still decode proc.stderr when available or include the timeout
message when TimeoutExpired is raised.
Comment on lines
+85
to
+86
| # WAVs are generated by scripts/build_demos.sh — keep slugs in sync. | ||
| # ───────────────────────────────────────────────────────────────────── |
Contributor
There was a problem hiding this comment.
Demo preset preview URLs currently point to missing assets (guaranteed 404).
These new preview_url entries depend on demo WAVs that are not present yet, so every card preview will fail in this draft state. Please regenerate and commit the demo audio bundle before merge (or gate these presets until assets exist).
Also applies to: 102-103, 121-122, 140-141, 159-160, 178-179, 197-198, 213-214
🤖 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/core/personalities.py` around lines 85 - 86, The new preview_url
entries in backend/core/personalities.py point to demo WAVs that don't exist
yet, causing 404s; either regenerate and commit the demo audio bundle using
scripts/build_demos.sh so the files referenced by each preview_url are present,
or temporarily remove/disable the preview_url fields (or set them to None) for
the affected presets and/or add a guard that only sets preview_url when the
corresponding WAV exists on disk. Locate the preview_url attributes in the
preset definitions (and the scripts/build_demos.sh workflow) and apply one of
these fixes consistently for all presets listed (lines referenced in the review:
102-103, 121-122, 140-141, 159-160, 178-179, 197-198, 213-214).
Comment on lines
+87
to
+107
| useEffect(() => { | ||
| if (!isTauri()) return; | ||
| let unlistenStart, unlistenStop; | ||
| (async () => { | ||
| try { | ||
| const { listen } = await import('@tauri-apps/api/event'); | ||
| unlistenStart = await listen('tray-dictate', () => { | ||
| setHotkeyState('verified'); | ||
| }); | ||
| unlistenStop = await listen('tray-dictate-stop', () => { | ||
| setHotkeyState('verified'); | ||
| }); | ||
| } catch { | ||
| // Tauri event API unavailable — leave state alone. | ||
| } | ||
| })(); | ||
| return () => { | ||
| try { unlistenStart && unlistenStart(); } catch { /* noop */ } | ||
| try { unlistenStop && unlistenStop(); } catch { /* noop */ } | ||
| }; | ||
| }, []); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="frontend/src/components/DictationDemo.jsx"
echo "== Exists? =="
ls -l "$FILE" || true
echo
echo "== Lines 70-140 =="
nl -ba "$FILE" | sed -n '70,140p' || true
echo
echo "== Search for tray-dictate listeners =="
rg -n "tray-dictate(-stop)?" frontend/src/components/DictationDemo.jsx || true
echo
echo "== Search for unlistenStart/unlistenStop/disposed patterns in file =="
rg -n "unlistenStart|unlistenStop|disposed" frontend/src/components/DictationDemo.jsx || trueRepository: debpalash/OmniVoice-Studio
Length of output: 966
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="frontend/src/components/DictationDemo.jsx"
echo "== Segment 80-120 (raw) =="
sed -n '80,120p' "$FILE" || true
echo
echo "== Segment 80-120 with cat -n =="
cat -n "$FILE" | sed -n '80,120p' || trueRepository: debpalash/OmniVoice-Studio
Length of output: 2810
Prevent async dictation listener cleanup race on unmount
The useEffect registers tray-dictate/tray-dictate-stop via async listen() calls that only assign unlistenStart/unlistenStop after the awaits resolve; if the component unmounts before those awaits finish, cleanup runs while the unlisten fns are still undefined, and the listeners can remain attached. Track a disposed flag and, when the listen() promises resolve after unmount, immediately call the returned unlisten functions (or skip assignment when disposed).
🤖 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/components/DictationDemo.jsx` around lines 87 - 107, The async
listener setup in useEffect may assign unlistenStart/unlistenStop after unmount,
leaving listeners active; introduce a disposed boolean (e.g., let disposed =
false) inside the effect and set it true in the cleanup, then after each await
listen(...) check disposed: if disposed immediately call the returned unlisten
function (or skip assigning to unlistenStart/unlistenStop) so listeners are
removed if the component already unmounted; keep references to the listen import
and use isTauri(), setHotkeyState, listen, unlistenStart and unlistenStop names
so the existing cleanup logic still works when not disposed.
Comment on lines
+105
to
+108
| const [demoDismissed, setDemoDismissed] = useState(() => { | ||
| if (typeof window === 'undefined') return false; | ||
| return localStorage.getItem('omnivoice.dubbingDemoDismissed') === '1'; | ||
| }); |
Contributor
There was a problem hiding this comment.
Guard localStorage read in state initializer.
Line 107 can throw in environments where storage access is blocked, which would break render for the whole tab. Mirror the write path’s safety with a read try/catch.
Suggested fix
const [demoDismissed, setDemoDismissed] = useState(() => {
if (typeof window === 'undefined') return false;
- return localStorage.getItem('omnivoice.dubbingDemoDismissed') === '1';
+ try {
+ return localStorage.getItem('omnivoice.dubbingDemoDismissed') === '1';
+ } catch {
+ return false;
+ }
});📝 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.
Suggested change
| const [demoDismissed, setDemoDismissed] = useState(() => { | |
| if (typeof window === 'undefined') return false; | |
| return localStorage.getItem('omnivoice.dubbingDemoDismissed') === '1'; | |
| }); | |
| const [demoDismissed, setDemoDismissed] = useState(() => { | |
| if (typeof window === 'undefined') return false; | |
| try { | |
| return localStorage.getItem('omnivoice.dubbingDemoDismissed') === '1'; | |
| } catch { | |
| return false; | |
| } | |
| }); |
🤖 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` around lines 105 - 108, The state initializer
for demoDismissed in the DubTab component may throw when accessing localStorage
(causing the tab to break); change the initializer to safely attempt the read in
a try/catch (and return false on any error or when window is undefined),
mirroring the write path’s safety—specifically wrap the
localStorage.getItem('omnivoice.dubbingDemoDismissed') call used to set
demoDismissed (and keep using setDemoDismissed elsewhere) so any exception
returns the safe default instead of bubbling up.
Comment on lines
+162
to
+164
| render "Fred" \ | ||
| "Patch the WebGPU shader in renderer dot tsx, then bump pnpm to nine point fifteen and rerun the Vitest suite." \ | ||
| "${DICT_DIR}/en_technical.wav" 16000 |
Contributor
There was a problem hiding this comment.
Transcript mismatch: spoken vs expected.
The dictation script at Line 163 says "renderer dot tsx" but the manifest's expected_transcript at Line 235 says "renderer.tsx". WhisperX will transcribe the spoken words literally, so the expected transcript should match what's actually spoken or the dictation demo test will always fail.
Proposed fix — align manifest to spoken form
"en_technical": {
"wav": "samples/dictation/en_technical.wav",
- "expected_transcript": "Patch the WebGPU shader in renderer.tsx, then bump pnpm to nine point fifteen and rerun the Vitest suite.",
+ "expected_transcript": "Patch the WebGPU shader in renderer dot tsx, then bump pnpm to nine point fifteen and rerun the Vitest suite.",
"language": "en"
},Also applies to: 233-236
🤖 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 `@scripts/build_demos.sh` around lines 162 - 164, The spoken transcript in the
demo invocation uses the literal phrase "renderer dot tsx" (the string passed to
render in scripts/build_demos.sh), but the manifest's expected_transcript uses
"renderer.tsx", causing mismatches; update the manifest's expected_transcript
entries that correspond to this demo (the entry matching the render call with
"Patch the WebGPU shader..." / "${DICT_DIR}/en_technical.wav" 16000 and the
other occurrences noted) so the expected_transcript matches the spoken form
"renderer dot tsx" (or alternatively change the render invocation to speak
"renderer.tsx" if you prefer matching the manifest), ensuring the two strings
are identical.
Comment on lines
+170
to
+172
| echo "" | ||
| echo "── Manifest ───────────────────────────────────────────────" | ||
| cat > "${SAMPLES_DIR}/demo/manifest.json" <<EOF |
Contributor
There was a problem hiding this comment.
Missing directory creation for demo/ before writing manifest.
The script writes to ${SAMPLES_DIR}/demo/manifest.json but never creates the demo/ subdirectory. This will fail with "No such file or directory" on a clean checkout.
Proposed fix
echo ""
echo "── Manifest ───────────────────────────────────────────────"
+mkdir -p "${SAMPLES_DIR}/demo"
cat > "${SAMPLES_DIR}/demo/manifest.json" <<EOF📝 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.
Suggested change
| echo "" | |
| echo "── Manifest ───────────────────────────────────────────────" | |
| cat > "${SAMPLES_DIR}/demo/manifest.json" <<EOF | |
| echo "" | |
| echo "── Manifest ───────────────────────────────────────────────" | |
| mkdir -p "${SAMPLES_DIR}/demo" | |
| cat > "${SAMPLES_DIR}/demo/manifest.json" <<EOF |
🤖 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 `@scripts/build_demos.sh` around lines 170 - 172, The script writes to
"${SAMPLES_DIR}/demo/manifest.json" but never ensures the demo subdirectory
exists; before the cat > "${SAMPLES_DIR}/demo/manifest.json" call in
build_demos.sh, create the directory using a safe mkdir -p on
"${SAMPLES_DIR}/demo" (or equivalent) so the file write cannot fail; reference
the SAMPLES_DIR variable and the manifest write location when adding the mkdir
-p step.
Comment on lines
+20
to
+22
| REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)" | ||
| OUT_DIR="${REPO_ROOT}/backend/assets/demo/dubbing" | ||
| mkdir -p "$OUT_DIR" |
Contributor
There was a problem hiding this comment.
Output directory does not align with the /demo_audio mount path.
The script writes to backend/assets/demo/dubbing/ but main.py mounts /demo_audio at backend/assets/samples/. The frontend's DubbingDemo.jsx fetches /demo_audio/demo/dubbing/manifest.json, which would resolve to backend/assets/samples/demo/dubbing/manifest.json — a path this script never creates.
Proposed fix — write to the samples subdirectory
set -e
REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)"
-OUT_DIR="${REPO_ROOT}/backend/assets/demo/dubbing"
+OUT_DIR="${REPO_ROOT}/backend/assets/samples/demo/dubbing"
mkdir -p "$OUT_DIR"📝 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.
Suggested change
| REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)" | |
| OUT_DIR="${REPO_ROOT}/backend/assets/demo/dubbing" | |
| mkdir -p "$OUT_DIR" | |
| REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)" | |
| OUT_DIR="${REPO_ROOT}/backend/assets/samples/demo/dubbing" | |
| mkdir -p "$OUT_DIR" |
🤖 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 `@scripts/build_dub_demo.sh` around lines 20 - 22, The script currently sets
OUT_DIR to "${REPO_ROOT}/backend/assets/demo/dubbing" which doesn't match the
`/demo_audio` mount used by main.py and DubbingDemo.jsx; update the OUT_DIR
assignment so it writes into the samples mount path (e.g.,
"${REPO_ROOT}/backend/assets/samples/demo/dubbing") and keep the existing mkdir
-p "$OUT_DIR" logic so the directory created matches the frontend's expected
/demo_audio/demo/dubbing/manifest.json location.
Comment on lines
+195
to
+199
| try: | ||
| asyncio.get_running_loop() | ||
| raise RuntimeError("Run this script outside an async context.") | ||
| except RuntimeError: | ||
| model = asyncio.run(get_model()) |
Contributor
There was a problem hiding this comment.
Async context detection is inverted — script will crash instead of showing helpful error.
When a running loop exists, get_running_loop() succeeds, you raise RuntimeError, but that exception is immediately caught by your own except RuntimeError block, which then calls asyncio.run(get_model()). Since a loop is already running, asyncio.run() will raise RuntimeError: cannot be called from a running event loop.
The user sees a confusing asyncio error instead of your clear message.
Proposed fix
try:
import asyncio
from services.model_manager import get_model
try:
asyncio.get_running_loop()
- raise RuntimeError("Run this script outside an async context.")
- except RuntimeError:
- model = asyncio.run(get_model())
+ # Loop exists — can't use asyncio.run()
+ print("ERROR: Run this script outside an async context (no running event loop).")
+ sys.exit(1)
+ except RuntimeError:
+ # No running loop — safe to use asyncio.run()
+ model = asyncio.run(get_model())
except Exception as e:🤖 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 `@scripts/render_demos_omnivoice.py` around lines 195 - 199, The current
async-detection logic is inverted: you call asyncio.get_running_loop(), then
raise RuntimeError which you immediately catch and call
asyncio.run(get_model()), causing a confusing "cannot be called from a running
event loop" when a loop actually exists. Change the control flow so that you
attempt asyncio.get_running_loop() and if it succeeds (loop exists) emit your
clear message and exit (or raise) instead of calling asyncio.run; only when
get_running_loop() raises RuntimeError (no running loop) should you call
asyncio.run(get_model()). Update the block around asyncio.get_running_loop() and
asyncio.run(get_model()) accordingly.
…Stretch Video
Replaces the current audio time-compression default (atempo squeeze to fit
slot) that produced chipmunk/alien output on high-density target languages
like Bengali. Two new user-selectable modes; legacy behaviour kept behind
an explicit "Strict slot" choice.
New `DubRequest.timing_strategy` enum (default "concise"):
- "concise" Translator trims text to fit at natural rate; if it
still overflows, hard-trim at slot with a fade so we
never overlap the next speaker. Surface overflow_s
per segment so the user can shorten the text.
- "stretch_video" Audio plays at natural 1.0× rate. Backend computes a
per-segment new timeline; persists a video_stretch_plan
on the job. Mux step (dub_export) builds an ffmpeg
trim+setpts+concat filter graph that stretches each
segment's video portion to match the natural-rate dub
audio. Gaps/pre-roll/tail pass through at 1.0×.
Sub burn under stretch_video is skipped in one pass
(cues would drift).
- "strict_slot" Legacy atempo squeeze. Retained for back-compat.
Director rate-bias side-effect (seg_speed *= bias) now gated on strict_slot
only, so "urgent"/"slow" direction tokens keep their instruct effect in
the new modes without chipmunking.
Per-segment fit_status emitted in the SSE done event:
{status: "fits" | "overflows" | "video_stretched", overflow_s?, stretch_ratio?}
DubSegmentRow's "Sync: 100%" badge (which was lying — sync_ratio was always
~1.0 because the TTS loop pre-trimmed to slot) is replaced with a truthful
"Fits / Overflows +Ns / Video 1.18×" label.
Frontend:
- prefsSlice.timingStrategy (persisted, store v3→v4 with safe migrate).
- DubTab footer Segmented control: "Concise · Stretch Video · Strict slot".
- useDubWorkflow passes timing_strategy on /dub/generate; consumes fit_status.
Tests: tests/test_dub_timing_strategy.py — 13 cases covering schema
defaults/validation, _build_video_stretch_filter_graph (pre-roll, gap,
tail, empty-plan early return, post-subtitle chain-in), and
_video_stretch_plan_for guards. 30/30 existing dub tests still pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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_export.py`:
- Around line 312-323: The current branch that detects stretch_entry + burn_subs
silently disables burning and logs a warning but leaves downstream subtitle
exporters emitting un-retimed SRT/VTT; instead, make this a hard failure: when
stretch_entry is true and burn_subs is true (or when stretch_entry is true and
any subtitle export path is requested), raise a clear exception (or return an
error) referencing job_id and mention video_stretch_plans so callers know
retiming is required; replace the logger.warning + burn_subs=False logic with an
explicit error path (raise ValueError/HTTPException) so the pipeline fails fast
rather than producing drifted SRT/VTT.
- Around line 254-264: The helper _video_stretch_plan_for currently checks the
job-wide timing_strategy; change it to read the per-track timing strategy stored
on job["dubbed_tracks"][lang_code]["timing_strategy"] (falling back to ""), and
only treat it as stretch_video when that track-level value lowercased equals
"stretch_video"; keep the rest of the logic (fetching
job.get("video_stretch_plans") and returning the entry if entry and
entry.get("plan") exist) unchanged so a stretched track's plan is returned even
if the job-level timing_strategy was later overwritten by dub_generate.
In `@backend/api/routers/dub_generate.py`:
- Around line 364-370: The RVC reload path is currently forcing audio to
target_samples which defeats "concise" and "stretch_video" timing strategies;
update the RVC branch that pads/trims to target_samples so it only enforces
length when the timing strategy expects slot-sized audio (i.e., when
req.timing_strategy == "strict_slot" or when _dur_for_tts is not None). Locate
the RVC reload logic (the code that references target_samples and performs
pad/trim after RVC processing) and gate that behavior on _strategy/_dur_for_tts
so RVC returns the processed natural-duration audio for "concise" and
"stretch_video" while still preserving legacy trimming for "strict_slot" (also
apply the same change in the second occurrence around the 426-440 region).
- Around line 387-396: The strict_slot block is prematurely trimming overlong
audio so later slot_fit logic (e.g., slot_fit == "time_stretch" / "trim") never
sees overflow; modify the _strategy == "strict_slot" handling to only pad short
audio (when target_samples > current_samples) but do not slice/trim audio_tensor
when current_samples > target_samples—remove or disable the audio_tensor[...,
:target_samples] branch so slot_fit can perform time_stretch/trim as intended.
In `@frontend/src/components/DubSegmentRow.jsx`:
- Around line 94-102: The current fallback branch uses seg.sync_ratio without
verifying it's a numeric value, causing null or non-number values to render
misleading badges; update the condition around seg.sync_ratio in
DubSegmentRow.jsx to only enter this branch when typeof seg.sync_ratio ===
'number' and isFinite(seg.sync_ratio) (or Number.isFinite(seg.sync_ratio)), then
assign r = seg.sync_ratio and proceed with the existing >1.25 / 0.95–1.05 / else
logic; if the value is not a valid number, skip setting fitBadge or set a safe
default badge (e.g., undefined or a "Unknown" state) so non-numeric inputs don't
render percentages.
🪄 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: 3bf6b510-edc6-4fda-ae6a-e464b9afc142
📒 Files selected for processing (9)
backend/api/routers/dub_export.pybackend/api/routers/dub_generate.pybackend/schemas/requests.pyfrontend/src/components/DubSegmentRow.jsxfrontend/src/hooks/useDubWorkflow.jsfrontend/src/pages/DubTab.jsxfrontend/src/store/index.tsfrontend/src/store/prefsSlice.tstests/test_dub_timing_strategy.py
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/pages/DubTab.jsx
Comment on lines
+254
to
+264
| def _video_stretch_plan_for(job: dict, lang_code: str) -> dict | None: | ||
| """Return the persisted stretch plan + total durations for `lang_code`, | ||
| or None if this job didn't use stretch_video mode (or no plan exists). | ||
| """ | ||
| if (job.get("timing_strategy") or "").lower() != "stretch_video": | ||
| return None | ||
| plans = job.get("video_stretch_plans") or {} | ||
| entry = plans.get(lang_code) | ||
| if not entry or not entry.get("plan"): | ||
| return None | ||
| return entry |
Contributor
There was a problem hiding this comment.
Use the track's timing strategy here, not the job-wide one.
dub_generate() records timing_strategy on each dubbed_tracks[lang_code], but the job-level field is overwritten on every render. After generating a later concise track in the same job, this helper returns None for an earlier stretched track even though its video_stretch_plans[lang_code] entry still exists, so export falls back to the unstretched video and loses sync.
Suggested change
def _video_stretch_plan_for(job: dict, lang_code: str) -> dict | None:
@@
- if (job.get("timing_strategy") or "").lower() != "stretch_video":
+ track = (job.get("dubbed_tracks") or {}).get(lang_code) or {}
+ if (track.get("timing_strategy") or "").lower() != "stretch_video":
return None🤖 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_export.py` around lines 254 - 264, The helper
_video_stretch_plan_for currently checks the job-wide timing_strategy; change it
to read the per-track timing strategy stored on
job["dubbed_tracks"][lang_code]["timing_strategy"] (falling back to ""), and
only treat it as stretch_video when that track-level value lowercased equals
"stretch_video"; keep the rest of the logic (fetching
job.get("video_stretch_plans") and returning the entry if entry and
entry.get("plan") exist) unchanged so a stretched track's plan is returned even
if the job-level timing_strategy was later overwritten by dub_generate.
Comment on lines
+312
to
+323
| # Subtitle burn under stretch_video would render cues at the original | ||
| # timestamps onto a re-timed video — they'd drift. Skip the burn pass | ||
| # in that combo and log; the user can still export the SRT/VTT | ||
| # separately and the new-layout timing lives there. | ||
| if stretch_entry and burn_subs: | ||
| logger.warning( | ||
| "stretch_video + burn_subs is not supported in one pass; " | ||
| "skipping subtitle burn for job %s. Export the SRT/VTT separately.", | ||
| job_id, | ||
| ) | ||
| burn_subs = False | ||
|
|
Contributor
There was a problem hiding this comment.
The stretch_video subtitle fallback is still broken.
This branch tells users to export SRT/VTT separately, but the subtitle exporters below still use the original segment timestamps. For stretched videos those files drift just like burned subtitles, so this silently drops the user onto another broken path. Until SRT/VTT is retimed from video_stretch_plans, this should fail fast 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/api/routers/dub_export.py` around lines 312 - 323, The current branch
that detects stretch_entry + burn_subs silently disables burning and logs a
warning but leaves downstream subtitle exporters emitting un-retimed SRT/VTT;
instead, make this a hard failure: when stretch_entry is true and burn_subs is
true (or when stretch_entry is true and any subtitle export path is requested),
raise a clear exception (or return an error) referencing job_id and mention
video_stretch_plans so callers know retiming is required; replace the
logger.warning + burn_subs=False logic with an explicit error path (raise
ValueError/HTTPException) so the pipeline fails fast rather than producing
drifted SRT/VTT.
Comment on lines
+364
to
+370
| # In concise / stretch_video modes we pass dur_s=None so the | ||
| # TTS model speaks at its natural rate for this text length — | ||
| # the whole point of the new timing strategies is to never | ||
| # squeeze the speech to fit. strict_slot keeps the legacy | ||
| # behaviour where dur_s is the slot hint. | ||
| _strategy = (req.timing_strategy or "concise").lower() | ||
| _dur_for_tts = seg_duration if _strategy == "strict_slot" else None |
Contributor
There was a problem hiding this comment.
RVC currently defeats concise and stretch_video.
These modes intentionally keep natural-duration audio, but the RVC reload path below always pads/trims back to target_samples. With RVC enabled, concise can no longer overflow and stretch_video builds its plan from slot-sized audio instead of the processed output.
Also applies to: 426-440
🤖 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 364 - 370, The RVC reload
path is currently forcing audio to target_samples which defeats "concise" and
"stretch_video" timing strategies; update the RVC branch that pads/trims to
target_samples so it only enforces length when the timing strategy expects
slot-sized audio (i.e., when req.timing_strategy == "strict_slot" or when
_dur_for_tts is not None). Locate the RVC reload logic (the code that references
target_samples and performs pad/trim after RVC processing) and gate that
behavior on _strategy/_dur_for_tts so RVC returns the processed natural-duration
audio for "concise" and "stretch_video" while still preserving legacy trimming
for "strict_slot" (also apply the same change in the second occurrence around
the 426-440 region).
Comment on lines
+387
to
+396
| if _strategy == "strict_slot": | ||
| # Legacy: pad short audio + trim long audio so the mix | ||
| # loop receives slot-sized buffers. The atempo squeeze | ||
| # in the mix loop never fires here because we already | ||
| # forced size = target_samples. | ||
| if target_samples > current_samples: | ||
| pad_amount = target_samples - current_samples | ||
| audio_tensor = torch.nn.functional.pad(audio_tensor, (0, pad_amount)) | ||
| elif current_samples > target_samples: | ||
| audio_tensor = audio_tensor[..., :target_samples] |
Contributor
There was a problem hiding this comment.
Don't normalize strict_slot audio before slot_fit runs.
This trims every overlong segment to target_samples up front, so the later slot_fit == "time_stretch" / "trim" branch never sees overflow. In practice strict_slot becomes hard-trim-only and loses the legacy behavior this mode is supposed to preserve.
Suggested change
- if _strategy == "strict_slot":
- # Legacy: pad short audio + trim long audio so the mix
- # loop receives slot-sized buffers. The atempo squeeze
- # in the mix loop never fires here because we already
- # forced size = target_samples.
- if target_samples > current_samples:
- pad_amount = target_samples - current_samples
- audio_tensor = torch.nn.functional.pad(audio_tensor, (0, pad_amount))
- elif current_samples > target_samples:
- audio_tensor = audio_tensor[..., :target_samples]
+ if _strategy == "strict_slot":
+ # Keep the synthesized length intact here.
+ # strict_slot overflow handling happens in the mix loop
+ # so slot_fit="time_stretch"/"trim" can still apply.
+ pass📝 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.
Suggested change
| if _strategy == "strict_slot": | |
| # Legacy: pad short audio + trim long audio so the mix | |
| # loop receives slot-sized buffers. The atempo squeeze | |
| # in the mix loop never fires here because we already | |
| # forced size = target_samples. | |
| if target_samples > current_samples: | |
| pad_amount = target_samples - current_samples | |
| audio_tensor = torch.nn.functional.pad(audio_tensor, (0, pad_amount)) | |
| elif current_samples > target_samples: | |
| audio_tensor = audio_tensor[..., :target_samples] | |
| if _strategy == "strict_slot": | |
| # Keep the synthesized length intact here. | |
| # strict_slot overflow handling happens in the mix loop | |
| # so slot_fit="time_stretch"/"trim" can still apply. | |
| pass |
🤖 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 387 - 396, The strict_slot
block is prematurely trimming overlong audio so later slot_fit logic (e.g.,
slot_fit == "time_stretch" / "trim") never sees overflow; modify the _strategy
== "strict_slot" handling to only pad short audio (when target_samples >
current_samples) but do not slice/trim audio_tensor when current_samples >
target_samples—remove or disable the audio_tensor[..., :target_samples] branch
so slot_fit can perform time_stretch/trim as intended.
Comment on lines
+94
to
+102
| } else if (seg.sync_ratio !== undefined) { | ||
| const r = seg.sync_ratio; | ||
| if (r > 1.25) { | ||
| fitBadge = { color: '#fb4934', Icon: AlertCircle, label: `${Math.round(r * 100)}%`, title: `TTS audio is ${Math.round(r * 100)}% of the slot — heavily compressed.` }; | ||
| } else if (r >= 0.95 && r <= 1.05) { | ||
| fitBadge = { color: '#b8bb26', Icon: CheckCircle, label: 'Fits', title: 'Audio fit inside the slot.' }; | ||
| } else { | ||
| fitBadge = { color: '#fabd2f', Icon: Circle, label: `${Math.round(r * 100)}%`, title: `TTS audio is ${Math.round(r * 100)}% of the slot.` }; | ||
| } |
Contributor
There was a problem hiding this comment.
Guard legacy sync_ratio fallback to numeric values only.
Line 94 treats any defined value as numeric; null/non-number values can render misleading badges (e.g., 0%).
Proposed fix
- } else if (seg.sync_ratio !== undefined) {
- const r = seg.sync_ratio;
+ } else if (typeof seg.sync_ratio === 'number' && Number.isFinite(seg.sync_ratio)) {
+ const r = seg.sync_ratio;🤖 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/components/DubSegmentRow.jsx` around lines 94 - 102, The current
fallback branch uses seg.sync_ratio without verifying it's a numeric value,
causing null or non-number values to render misleading badges; update the
condition around seg.sync_ratio in DubSegmentRow.jsx to only enter this branch
when typeof seg.sync_ratio === 'number' and isFinite(seg.sync_ratio) (or
Number.isFinite(seg.sync_ratio)), then assign r = seg.sync_ratio and proceed
with the existing >1.25 / 0.95–1.05 / else logic; if the value is not a valid
number, skip setting fitBadge or set a safe default badge (e.g., undefined or a
"Unknown" state) so non-numeric inputs don't render percentages.
…ad of code-4 black box When a project's underlying media file is gone (moved or deleted between save and reload) the <video> element fires MediaError code 4 and the companion audio fetch returns HTTP 404 — both were silently warned to the console while the user stared at an unresponsive black panel and an empty waveform. - WaveformTimeline now flips loadError when the video element rejects code 3 (decode) or 4 (src not supported), and tracks `sourceMissing` separately so the error UI can name the actual problem. - The audio decode fallback chain catches HTTP 404 specifically and treats it as source-missing instead of loading silent empty peaks — an empty waveform on a deleted source is more confusing than a clear "Re-upload the video to continue" message. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When the dev Vite server restarts (or the main window is created before the backend is ready), the webview load fails and the window is left with `<body></body>` plus a "Could not connect to the server" console error. Clicking "Show OmniVoice" from the tray menu just re-showed the broken window — there was no recovery path short of quit+relaunch. Now the show handler runs a tiny eval after `show()`/`set_focus()` that calls `location.reload()` only when `document.body.childElementCount === 0`. A healthy window doesn't blink (body is non-empty); a blank one self-recovers as soon as the user clicks Show. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bring the branch up to date with main and resolve 5 conflicts as feature-unions so nothing shipped since #133 was opened regresses: - useTTS.js: take main's #141 validator-safe instruct (buildDesignInstruct); #133 held only the stale pre-#141 dedup logic. - dub_pipeline.py: UNION — keep #133's download-task cancel cleanup AND main's plan-04 logging + structured failure event (build_failure). - dubSlice.ts / useDubWorkflow.js: UNION — keep both #133's dub download-progress state (setDubPrepProgress / setDubCurrentSegId) and main's pipeline-error-transparency state (setDubFailure). - bootstrap.rs: take main's shipped plan-03 network-resilience cascade (#140/#142); #133's region-based mirror approach was the superseded alternative for the same concern. get_effective_region stays live (shared via config.rs, used by tools.rs). Verified: frontend typecheck + build clean; 90 backend tests pass (dub / failure / timing / onboarding / personalities), 0 failures. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| logger.warning( | ||
| "stretch_video + burn_subs is not supported in one pass; " | ||
| "skipping subtitle burn for job %s. Export the SRT/VTT separately.", | ||
| job_id, |
| import sqlalchemy as sa | ||
|
|
||
|
|
||
| revision: str = "0002_voice_profile_demo_fields" |
|
|
||
|
|
||
| revision: str = "0002_voice_profile_demo_fields" | ||
| down_revision: Union[str, None] = "0001_phase1_settings" |
|
|
||
| revision: str = "0002_voice_profile_demo_fields" | ||
| down_revision: Union[str, None] = "0001_phase1_settings" | ||
| branch_labels: Union[str, Sequence[str], None] = None |
| revision: str = "0002_voice_profile_demo_fields" | ||
| down_revision: Union[str, None] = "0001_phase1_settings" | ||
| branch_labels: Union[str, Sequence[str], None] = None | ||
| depends_on: Union[str, Sequence[str], None] = None |
| if p.returncode is None: | ||
| try: | ||
| p.kill() | ||
| except ProcessLookupError: |
| pass | ||
| try: | ||
| await asyncio.wait_for(p.wait(), timeout=5.0) | ||
| except asyncio.TimeoutError: |
| pass | ||
| try: | ||
| p.stdout.close() | ||
| except Exception: |
| if rc == 0 and os.path.exists(target) and target != video_path: | ||
| try: | ||
| os.remove(video_path) | ||
| except OSError: |
|
|
||
| import argparse | ||
| import json | ||
| import os |
Address PR #133 review: - ReportBugButton: /system/info exposes `platform` + `device`, not `os`/`torch_device`/`gpu` — those reads silently dropped OS/GPU from every bug report. Map to the real fields (CodeRabbit). Also remove the dead `home` local in stripHome (CodeQL unused-variable). - DictationDemo: drop unused `Loader` import (CodeQL unused-import). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
|
Merged Addressed bot review in
Deferring (tracked, not blocking this merge):
Demo media assets still need rendering ( |
This was referenced May 29, 2026
debpalash
added a commit
that referenced
this pull request
May 29, 2026
_pitch_preserving_stretch ran a blocking subprocess.run() inside the `_stream` async generator (on the event loop). Each ffmpeg atempo call is ~50-100 ms, so on a multi-segment time_stretch dub job it froze health checks, status SSE, and every other concurrent request for seconds. Convert to asyncio.create_subprocess_exec + await communicate() (same pattern as run_proc_streaming_stderr); await the call site in _stream. Drop the now-unused `import subprocess`. Tests: async coroutine + target-length + no-op cases (real ffmpeg). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced May 30, 2026
This was referenced Jun 7, 2026
Merged
This was referenced Jun 29, 2026
This was referenced Jul 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Working-tree snapshot bundling several in-flight v0.3.0 workstreams. Opened as a draft — see the Known Gap below.
What's in here (60 files)
DemoPresetGrid,DictationDemo,DubbingDemo(+ tests), render scripts (render_demos_omnivoice.py,build_demos.sh,build_dub_demo.sh), personality preview URLs, alembic migration0002_voice_profile_demo_fields.ReportBugButton(prefilled GitHub-issue URL path, per the local-first constraint — no telemetry endpoint).errorDocsMapdeeplinks +BootstrapSplash/error wiring.DubSegmentRow/Table,WaveformTimeline,dubSlicetweaks,useDubWorkflow/useTTS..planning/issue-clusters/(plan-01..05 root-cause masters, tracked as [plan-01] Windows Model Storage & HF Cache — symlink-safe, relocatable model dir #128–[plan-05] Voice Design Instruct Validator — reconcile preset builder with the whitelist #132).v0.3.0, no version bumps.The generated demo audio assets are not in this tree, and
backend/assets/samples/demo_voice.wavis deleted.onboarding.pyguards the missing file (skips seeding the demo profile with a warning) → no crash, but first-run Launchpad will be empty.personalities.pypreview URLs point at/demo_audio/voice_design/*.wav→ 404 until assets exist.Before merge: regenerate + commit the demo assets via
scripts/build_demos.sh(and restore/regeneratedemo_voice.wav).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation & Chores