fix: Windows port conflict + HF_HUB_OFFLINE for offline model loading#85
Open
caaaaaleb wants to merge 4 commits into
Open
fix: Windows port conflict + HF_HUB_OFFLINE for offline model loading#85caaaaaleb wants to merge 4 commits into
caaaaaleb wants to merge 4 commits into
Conversation
- Implement kill_orphan_on_port for Windows using netstat + taskkill - Set HF_HUB_OFFLINE=1 in main.py before any HF library import - Set HF_HUB_OFFLINE=1 in model_manager._load_model_sync with restore - Add local_files_only=True to OmniVoice.from_pretrained call Fixes "Could not import module AutoModel" error caused by port conflict when stale backend processes occupy the port on Windows. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughREADME replaced with a Simplified Chinese Windows usage guide; frontend adds full i18n (en+zh) and uses translations across UI; backend forces HF offline at startup and during model loads (restores env), uses local_files_only=True, and Windows tauri backend kills orphan TCP listeners; Vite binds 127.0.0.1; Windows start scripts added. ChangesDocumentation & Windows scripts
Frontend i18n
Backend offline/model behavior
Windows process cleanup (tauri backend)
Sequence Diagram(s)sequenceDiagram
participant BackendMain as backend/main.py
participant HFLib as HuggingFaceLibraries
BackendMain->>BackendMain: set HF_HUB_OFFLINE=1
BackendMain->>HFLib: import HF libraries (initialize offline)
sequenceDiagram
participant LoadFunc as _load_model_sync()
participant HFEnv as HF_HUB_OFFLINE Env
participant OmniVoice as OmniVoice.from_pretrained
LoadFunc->>HFEnv: save prior HF_HUB_OFFLINE
LoadFunc->>HFEnv: set HF_HUB_OFFLINE=1
LoadFunc->>OmniVoice: from_pretrained(local_files_only=True)
OmniVoice->>OmniVoice: load weights from local files
LoadFunc->>HFEnv: restore previous HF_HUB_OFFLINE or remove
sequenceDiagram
participant KillFunc as kill_orphan_on_port()
participant NetStat as netstat -ano -p TCP
participant TaskKill as taskkill /F
KillFunc->>NetStat: run netstat to get TCP listeners
NetStat-->>KillFunc: connection table lines
KillFunc->>KillFunc: filter LISTENING entries for port
KillFunc->>KillFunc: parse PID
KillFunc->>TaskKill: taskkill /PID <pid> /F
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
- 全文翻译为中文 - 添加本 Fork 修复版说明（Windows 端口冲突、离线模式） - 更新下载链接指向本 Fork Release - 增加国内网络环境使用指南 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![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: 1
🧹 Nitpick comments (3)
README.md (2)
281-293: 💤 Low valueAdd language identifier to fenced code block.
The architecture diagram code block should specify a language identifier to satisfy markdown linting rules. Consider using
textorplaintextfor ASCII art diagrams.📝 Proposed fix
-``` +```text ┌─────────────────────────────────────────────────┐ │ 前端 (React) │As per coding guidelines, static analysis (markdownlint-cli2) reports: "Fenced code blocks should have a language specified (MD040)".
🤖 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 `@README.md` around lines 281 - 293, The fenced ASCII-art diagram block (the triple-backtick block containing the box with "前端 (React)" and "后端 (FastAPI)" lines) needs a language identifier to satisfy MD040; update the opening fence from ``` to ```text (or ```plaintext) so the block becomes a labeled code fence (e.g., ```text) and keep the enclosed ASCII art unchanged.
8-9: 💤 Low valueConsider badge consistency with fork messaging.
The stars badge references the original repository (
debpalash/OmniVoice-Studio) while the release badge references the fork (caaaaaleb/OmniVoice-Studio). This mixing could confuse users about which repository they're viewing.Since this is a documented fork providing fixes, consider either:
- Using fork references for both badges, or
- Adding a note clarifying that stars reflect the upstream project
🤖 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 `@README.md` around lines 8 - 9, The README mixes upstream and fork badges (stars link uses https://github.com/debpalash/OmniVoice-Studio while release uses https://github.com/caaaaaleb/OmniVoice-Studio); make them consistent by either updating the stars badge URL to the fork (change the href/src that reference debpalash to caaaaaleb) or add a short clarifying sentence beneath the badges stating that the star count reflects the upstream repository; ensure the two anchor/img entries shown in the diff are updated together so both badges point to the same repository or are accompanied by the clarification text.frontend/src-tauri/src/backend.rs (1)
103-109: ⚡ Quick winSurface cleanup failures in the logs.
This branch silently returns when
netstatcannot be launched and ignorestaskkillexit status. If cleanup fails because of permissions or a missing binary, we lose the only clue and fall back to the same bind error this PR is trying to fix.Proposed change
let out = match std::process::Command::new("netstat") .args(["-ano", "-p", "TCP"]) .output() { Ok(o) => o, - Err(_) => return, + Err(e) => { + log::warn!("Failed to run netstat for port {}: {}", port, e); + return; + } }; + if !out.status.success() { + log::warn!( + "netstat exited with status {:?} while probing port {}: {}", + out.status.code(), + port, + String::from_utf8_lossy(&out.stderr) + ); + return; + } ... - let _ = std::process::Command::new("taskkill") - .args(["/PID", &pid.to_string(), "/F"]) - .output(); + match std::process::Command::new("taskkill") + .args(["/PID", &pid.to_string(), "/F"]) + .output() + { + Ok(kill) if !kill.status.success() => { + log::warn!( + "taskkill failed for pid {} on port {}: {}", + pid, + port, + String::from_utf8_lossy(&kill.stderr) + ); + } + Err(e) => { + log::warn!( + "Failed to invoke taskkill for pid {} on port {}: {}", + pid, + port, + e + ); + } + _ => {} + }Also applies to: 135-137
🤖 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/backend.rs` around lines 103 - 109, The code silently returns when std::process::Command::new("netstat").output() fails and also ignores the exit status of the subsequent taskkill invocation; update the Err(_) arm of the Command::new("netstat")...output() match to log the actual error (include the error from output()) instead of returning, and after spawning/running taskkill check its ExitStatus and log a clear error if it failed (include stdout/stderr if available) so permission/missing-binary failures are visible; update the same pattern referenced around the other invocation (lines near 135-137) to perform the same logging and non-silent failure handling.
🤖 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 `@frontend/src-tauri/src/backend.rs`:
- Around line 128-137: The current Windows branch blindly taskkills whatever PID
netstat reports; update the logic around the PID extraction (the block that
parses parts/ pid_str and produces pid and the taskkill call) to first query the
target process command line or executable and verify it belongs to our spawned
OmniVoice backend before killing: after parsing pid, run a safe metadata lookup
(e.g. via PowerShell/Get-Process or wmic for the given pid) to obtain
ProcessName/Path/CommandLine, compare that value against the expected backend
executable name or unique marker used when we spawn the backend, and only call
std::process::Command::new("taskkill") if the verification matches; otherwise
log a port-conflict error and surface the conflict instead of killing the
process.
---
Nitpick comments:
In `@frontend/src-tauri/src/backend.rs`:
- Around line 103-109: The code silently returns when
std::process::Command::new("netstat").output() fails and also ignores the exit
status of the subsequent taskkill invocation; update the Err(_) arm of the
Command::new("netstat")...output() match to log the actual error (include the
error from output()) instead of returning, and after spawning/running taskkill
check its ExitStatus and log a clear error if it failed (include stdout/stderr
if available) so permission/missing-binary failures are visible; update the same
pattern referenced around the other invocation (lines near 135-137) to perform
the same logging and non-silent failure handling.
In `@README.md`:
- Around line 281-293: The fenced ASCII-art diagram block (the triple-backtick
block containing the box with "前端 (React)" and "后端 (FastAPI)" lines) needs a
language identifier to satisfy MD040; update the opening fence from ``` to
```text (or ```plaintext) so the block becomes a labeled code fence (e.g.,
```text) and keep the enclosed ASCII art unchanged.
- Around line 8-9: The README mixes upstream and fork badges (stars link uses
https://github.com/debpalash/OmniVoice-Studio while release uses
https://github.com/caaaaaleb/OmniVoice-Studio); make them consistent by either
updating the stars badge URL to the fork (change the href/src that reference
debpalash to caaaaaleb) or add a short clarifying sentence beneath the badges
stating that the star count reflects the upstream repository; ensure the two
anchor/img entries shown in the diff are updated together so both badges point
to the same repository or are accompanied by the clarification text.
🪄 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: d3c53b90-30d8-4ff7-9193-1a53c28c786e
📒 Files selected for processing (4)
README.mdbackend/main.pybackend/services/model_manager.pyfrontend/src-tauri/src/backend.rs
Comment on lines
+128
to
+137
| let parts: Vec<&str> = line.split_whitespace().collect(); | ||
| if let Some(pid_str) = parts.last() { | ||
| if let Ok(pid) = pid_str.parse::<u32>() { | ||
| log::warn!( | ||
| "Killing orphan process {} on port {} (Windows)", | ||
| pid, port | ||
| ); | ||
| let _ = std::process::Command::new("taskkill") | ||
| .args(["/PID", &pid.to_string(), "/F"]) | ||
| .output(); |
Contributor
There was a problem hiding this comment.
Don't taskkill arbitrary listeners on this port.
Line 135 force-kills whichever PID netstat reports, but this helper never verifies that the process is actually our backend. If some other local service is bound to the configured port, startup can terminate an unrelated app instead of surfacing a port conflict. Please check the process command line/executable matches the spawned OmniVoice backend before killing it.
🤖 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/backend.rs` around lines 128 - 137, The current
Windows branch blindly taskkills whatever PID netstat reports; update the logic
around the PID extraction (the block that parses parts/ pid_str and produces pid
and the taskkill call) to first query the target process command line or
executable and verify it belongs to our spawned OmniVoice backend before
killing: after parsing pid, run a safe metadata lookup (e.g. via
PowerShell/Get-Process or wmic for the given pid) to obtain
ProcessName/Path/CommandLine, compare that value against the expected backend
executable name or unique marker used when we spawn the backend, and only call
std::process::Command::new("taskkill") if the verification matches; otherwise
log a port-conflict error and surface the conflict instead of killing the
process.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@README.md`:
- Around line 11-13: The directory tree lists both README.md and 使用指南.md but the
tree labels 使用指南.md as “本文件” while this PR edits README.md, causing confusion;
update the directory tree entry so the “本文件” label correctly refers to README.md
(or rename the file consistently) by changing the label for 使用指南.md to its
proper description and/or marking README.md as “本文件”, and ensure the README.md
and 使用指南.md filenames and their descriptive comments (in the README listing) are
consistent.
- Around line 9-25: The README.md's directory tree fenced code block is missing
a language tag (causing MD040); open the fenced block in README.md and add a
language tag (e.g., replace "```" with "```text") so the tree block is
recognized as plain text; locate the block shown in the diff (the directory tree
starting with "E:\OmniVoice-Studio\") and update its opening fence accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
9
to
25
| ``` | ||
|
|
||
| Open normally after. One-time fix. | ||
| </details> | ||
|
|
||
| <details> | ||
| <summary><b>Windows — first launch takes 5–10 minutes</b></summary> | ||
| <br/> | ||
|
|
||
| The app bootstraps a Python virtual environment, installs dependencies, and downloads ffmpeg on first run. The splash screen shows each step. Subsequent launches start in seconds. | ||
| </details> | ||
|
|
||
| <details> | ||
| <summary><b>Linux — AppImage needs FUSE</b></summary> | ||
| <br/> | ||
|
|
||
| If FUSE isn't available, use the `.deb` package or extract-and-run: | ||
|
|
||
| ```bash | ||
| chmod +x OmniVoice.Studio_*.AppImage | ||
| ./OmniVoice.Studio_*.AppImage --appimage-extract-and-run | ||
| ``` | ||
| </details> | ||
|
|
||
| <details> | ||
| <summary><b>Linux — White screen on Fedora 44 / Ubuntu 24.04</b></summary> | ||
| <br/> | ||
|
|
||
| Some newer distros ship a WebKit/GTK version with compositing issues. Try: | ||
|
|
||
| ```bash | ||
| WEBKIT_DISABLE_COMPOSITING_MODE=1 ./OmniVoice.Studio_*.AppImage | ||
| E:\OmniVoice-Studio\ | ||
| ├── README.md # 项目说明（中文） | ||
| ├── 使用指南.md # 本文件 | ||
| ├── OmniVoice_Studio_0.2.7_x64.msi # Windows 安装包 (~7 MB) | ||
| ├── backend/ # Python 后端 | ||
| │ ├── main.py # FastAPI 入口 | ||
| │ └── services/ | ||
| │ └── model_manager.py # 模型加载 | ||
| ├── frontend/ # React 前端 (Tauri) | ||
| │ └── src-tauri/ | ||
| │ └── src/ | ||
| │ └── backend.rs # 后端进程管理 | ||
| ├── omnivoice/ # OmniVoice Python 包 | ||
| ├── pyproject.toml # Python 项目配置 | ||
| └── uv.lock # 依赖锁定文件 | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add a language tag to the fenced code block to satisfy markdownlint.
Line 9 opens a fenced block without a language, triggering MD040 and causing lint noise.
Suggested patch
-```
+```text
E:\OmniVoice-Studio\
├── README.md # 项目说明（中文）
...
└── uv.lock # 依赖锁定文件</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @README.md around lines 9 - 25, The README.md's directory tree fenced code
block is missing a language tag (causing MD040); open the fenced block in
README.md and add a language tag (e.g., replace "" with "text") so the
tree block is recognized as plain text; locate the block shown in the diff (the
directory tree starting with "E:\OmniVoice-Studio") and update its opening
fence accordingly.
</details>
<!-- fingerprinting:phantom:triton:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
- Add complete zh/en i18n across 50+ source files with i18next - Add language switcher (中/EN toggle) in header with localStorage persistence - Fix _gpu_pool NameError in model_manager.py (__getattr__ internal access) - Fix TritonMissing on Windows: disable torch.compile/dynamo/inductor via env vars + explicit check - Fix Vite IPv6-only binding: force host to 127.0.0.1 - Add start.bat (production, .venv only) and start-dev.bat (Vite HMR) - Rebuild frontend dist with all fixes - Update README with new features and fix table Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
start.bat (1)
1-66:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: Convert file to Windows line endings (CRLF).
The file uses Unix line endings (LF-only), which will cause parsing failures on Windows due to CMD.exe parser bugs. Batch files require CRLF line endings to work reliably.
🔧 How to fix
Convert the file to Windows line endings using one of:
Git:
# Configure git to auto-convert on checkout (run once in repo root) git config core.autocrlf true git rm --cached start.bat git reset --hardCommand line:
# Using unix2dos (if available) unix2dos start.bat # Or using PowerShell (Get-Content start.bat) | Set-Content start.batEditor:
- VS Code: Click "LF" in status bar → select "CRLF"
- Notepad++: Edit → EOL Conversion → Windows (CR LF)
As per coding guidelines, every fix must work on Windows (x64) with no platform-only regressions.
🤖 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 `@start.bat` around lines 1 - 66, The start.bat file currently uses LF-only line endings which will break CMD parsing; convert start.bat to Windows CRLF endings (ensure every line ends with CRLF) and commit that change; you can do this by enabling Git autocrlf and re-checking out the file (git config core.autocrlf true; git rm --cached start.bat; git reset --hard), or run a conversion tool (unix2dos start.bat) or change the EOL in your editor (e.g., VS Code: click LF → select CRLF), then verify the batch commands like the for loops, start "OmniVoice-Backend", curl health check and taskkill lines run correctly on Windows before committing.start-dev.bat (1)
1-89:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: Convert file to Windows line endings (CRLF).
The file uses Unix line endings (LF-only), which will cause parsing failures on Windows due to CMD.exe parser bugs. Batch files require CRLF line endings to work reliably.
🔧 How to fix
Convert the file to Windows line endings using one of:
Git:
# Configure git to auto-convert on checkout (run once in repo root) git config core.autocrlf true git rm --cached start-dev.bat git reset --hardCommand line:
# Using unix2dos (if available) unix2dos start-dev.bat # Or using PowerShell (Get-Content start-dev.bat) | Set-Content start-dev.batEditor:
- VS Code: Click "LF" in status bar → select "CRLF"
- Notepad++: Edit → EOL Conversion → Windows (CR LF)
As per coding guidelines, every fix must work on Windows (x64) with no platform-only regressions.
🤖 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 `@start-dev.bat` around lines 1 - 89, The batch script uses LF-only line endings which break CMD parsing; convert start-dev.bat to Windows CRLF line endings and commit the change so Windows can execute it reliably. Fix by converting the file (e.g., run unix2dos on start-dev.bat, use PowerShell Set-Content to rewrite with CRLF, enable git core.autocrlf and refresh the file, or change EOL to CRLF in your editor/VS Code) and verify the top of the script (starts with "`@echo` off") runs successfully on a Windows x64 environment before pushing.frontend/src/pages/Projects.jsx (1)
163-176:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winVariable shadowing causes runtime error.
The loop variable
tat line 163 shadows the translation functiontfromuseTranslation()declared at line 90. When line 167 attempts to callt('projects.transcription_item'), it will fail withTypeError: t is not a functionbecausetnow refers to the current transcription object, not the translation function.🐛 Proposed fix: rename loop variable
- for (const t of transcriptions) { + for (const trans of transcriptions) { list.push({ type: 'transcripts', - id: t.id || String(Math.random()), - title: (t.text || t('projects.transcription_item')).slice(0, 120), - subtitle: [t.language, t.duration_s ? `${Math.round(t.duration_s)}s` : ''].filter(Boolean).join(' · '), - ts: t.timestamp ? Date.parse(t.timestamp) : 0, + id: trans.id || String(Math.random()), + title: (trans.text || t('projects.transcription_item')).slice(0, 120), + subtitle: [trans.language, trans.duration_s ? `${Math.round(trans.duration_s)}s` : ''].filter(Boolean).join(' · '), + ts: trans.timestamp ? Date.parse(trans.timestamp) : 0, accent: '`#83a598`', Icon: FileText, onClick: () => { - navigator.clipboard.writeText(t.text || ''); + navigator.clipboard.writeText(trans.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 `@frontend/src/pages/Projects.jsx` around lines 163 - 176, The loop is shadowing the translation function t from useTranslation() by using `for (const t of transcriptions)`, causing `t('projects.transcription_item')` to fail; rename the loop variable (e.g., `for (const transcript of transcriptions)` or `tr`) and update all references inside the loop (`t.id`, `t.text`, `t.language`, `t.duration_s`, `t.timestamp`) to the new name so the translation function `t` remains callable.
🧹 Nitpick comments (2)
frontend/src/components/ExportModal.jsx (1)
76-80: 💤 Low valueConsider adding language to useMemo dependencies.
The
useMemousest('dub.original')but only depends ondubTracks. While the component will re-render when language changes (via react-i18next context), explicitly tracking language in dependencies makes the intent clearer and satisfies ESLint exhaustive-deps.📦 Suggested addition to dependencies
+import { useTranslation } from 'react-i18next'; + export default function ExportModal({ ... }) { const { t, i18n } = useTranslation(); ... const allTracks = useMemo(() => { const out = [{ code: 'original', label: t('dub.original'), kind: 'original' }]; (dubTracks || []).forEach(t => out.push({ code: t, label: t.toUpperCase(), kind: 'dub' })); return out; - }, [dubTracks]); + }, [dubTracks, i18n.language, t]);🤖 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/ExportModal.jsx` around lines 76 - 80, The memo for allTracks uses the i18n translator t('dub.original') but only lists dubTracks in its dependency array; update the useMemo dependencies for the allTracks constant (the useMemo call that builds out from dubTracks) to include the translator/language so the memo updates when language changes—e.g., add either the t function or the current i18n.language value to the dependency array to satisfy exhaustive-deps and ensure labels update on locale change.frontend/src/components/GlossaryPanel.jsx (1)
175-182: 💤 Low valueConsider renaming map parameter to avoid shadowing.
The loop variable
tshadows the translation function fromuseTranslation(). While this doesn't cause a runtime error (since the translation function isn't called inside the map), it reduces code clarity and could confuse maintainers.♻️ Suggested refactor
- {terms.map(t => ( + {terms.map(term => ( <GlossaryRow - key={t.id} - term={t} - onUpdate={(patch) => onUpdate(t.id, patch)} - onDelete={() => onDelete(t.id)} + key={term.id} + term={term} + onUpdate={(patch) => onUpdate(term.id, patch)} + onDelete={() => onDelete(term.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 `@frontend/src/components/GlossaryPanel.jsx` around lines 175 - 182, Rename the map loop parameter that currently uses "t" to avoid shadowing the translation function from useTranslation(); for example, change the iterator in the terms.map callback to "term" or "termItem" and update all references inside the callback (key, term prop, onUpdate/onDelete closures) so GlossaryRow receives the same data but no longer conflicts with the translation function name.
🤖 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 `@frontend/src/App.jsx`:
- Around line 216-218: The default compareText initializer references an
undefined i18n instead of using the translation hook; inside the App component
call useTranslation() (e.g., const { t } = useTranslation()) and update the
useState initializer for compareText to use t('voice.default_test_phrase') (with
a try/catch or fallback string) so the localized default is used; update any
usages that referenced i18n.t to use the destructured t function and ensure
useTranslation is imported and invoked within the App component scope.
In `@frontend/src/pages/CloneDesignTab.jsx`:
- Line 211: The concatenation at the JSX expression using
t('voice.using_profile') + profiles.find(p => p.id === selectedProfile)?.name
breaks localization; instead pass the profile name as an interpolation variable
to the translator. Replace the concatenated expression with a single t call that
supplies the name (e.g., t('voice.using_profile', { name: profiles.find(p =>
p.id === selectedProfile)?.name })) so translators control spacing/word order;
update the JSX where selectedProfile and profiles are referenced accordingly.
In `@frontend/src/pages/Transcriptions.jsx`:
- Around line 76-82: The useCallback hooks for copyText and exportAll capture
the translation function t but do not include it in their dependency arrays,
causing stale translations after language changes; update the useCallback
declarations for copyText and exportAll to include t (e.g., useCallback(..., [t,
...otherDeps])) so they re-create with the latest translator when language
switches.
---
Outside diff comments:
In `@frontend/src/pages/Projects.jsx`:
- Around line 163-176: The loop is shadowing the translation function t from
useTranslation() by using `for (const t of transcriptions)`, causing
`t('projects.transcription_item')` to fail; rename the loop variable (e.g., `for
(const transcript of transcriptions)` or `tr`) and update all references inside
the loop (`t.id`, `t.text`, `t.language`, `t.duration_s`, `t.timestamp`) to the
new name so the translation function `t` remains callable.
In `@start-dev.bat`:
- Around line 1-89: The batch script uses LF-only line endings which break CMD
parsing; convert start-dev.bat to Windows CRLF line endings and commit the
change so Windows can execute it reliably. Fix by converting the file (e.g., run
unix2dos on start-dev.bat, use PowerShell Set-Content to rewrite with CRLF,
enable git core.autocrlf and refresh the file, or change EOL to CRLF in your
editor/VS Code) and verify the top of the script (starts with "`@echo` off") runs
successfully on a Windows x64 environment before pushing.
In `@start.bat`:
- Around line 1-66: The start.bat file currently uses LF-only line endings which
will break CMD parsing; convert start.bat to Windows CRLF endings (ensure every
line ends with CRLF) and commit that change; you can do this by enabling Git
autocrlf and re-checking out the file (git config core.autocrlf true; git rm
--cached start.bat; git reset --hard), or run a conversion tool (unix2dos
start.bat) or change the EOL in your editor (e.g., VS Code: click LF → select
CRLF), then verify the batch commands like the for loops, start
"OmniVoice-Backend", curl health check and taskkill lines run correctly on
Windows before committing.
---
Nitpick comments:
In `@frontend/src/components/ExportModal.jsx`:
- Around line 76-80: The memo for allTracks uses the i18n translator
t('dub.original') but only lists dubTracks in its dependency array; update the
useMemo dependencies for the allTracks constant (the useMemo call that builds
out from dubTracks) to include the translator/language so the memo updates when
language changes—e.g., add either the t function or the current i18n.language
value to the dependency array to satisfy exhaustive-deps and ensure labels
update on locale change.
In `@frontend/src/components/GlossaryPanel.jsx`:
- Around line 175-182: Rename the map loop parameter that currently uses "t" to
avoid shadowing the translation function from useTranslation(); for example,
change the iterator in the terms.map callback to "term" or "termItem" and update
all references inside the callback (key, term prop, onUpdate/onDelete closures)
so GlossaryRow receives the same data but no longer conflicts with the
translation function name.
🪄 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: a747c065-c8fb-4fe7-8464-b682031eb714
📒 Files selected for processing (56)
.gitignoreREADME.mdbackend/main.pybackend/services/model_manager.pyfrontend/src/App.jsxfrontend/src/components/AudioTrimmer.jsxfrontend/src/components/BatchAddDialog.jsxfrontend/src/components/BootstrapSplash.jsxfrontend/src/components/CaptureWidget.jsxfrontend/src/components/CastingView.jsxfrontend/src/components/CheckpointBanner.jsxfrontend/src/components/CompareModal.jsxfrontend/src/components/DirectionDialog.jsxfrontend/src/components/DubSegmentRow.jsxfrontend/src/components/DubSegmentTable.jsxfrontend/src/components/ErrorBoundary.jsxfrontend/src/components/ExportModal.jsxfrontend/src/components/FloatingPill.jsxfrontend/src/components/GlossaryPanel.jsxfrontend/src/components/Header.jsxfrontend/src/components/KeyboardCheatsheet.jsxfrontend/src/components/LogsFooter.jsxfrontend/src/components/MultiLangPicker.jsxfrontend/src/components/NavRail.jsxfrontend/src/components/NotificationPanel.jsxfrontend/src/components/ReadinessChecklist.jsxfrontend/src/components/SearchableSelect.jsxfrontend/src/components/Sidebar.jsxfrontend/src/components/StoriesEditor.jsxfrontend/src/components/VoicePreview.jsxfrontend/src/components/WaveformTimeline.jsxfrontend/src/hooks/useAppData.jsfrontend/src/hooks/useDubWorkflow.jsfrontend/src/hooks/useProfiles.jsfrontend/src/hooks/useTTS.jsfrontend/src/i18n/index.tsfrontend/src/i18n/locales/en.jsonfrontend/src/i18n/locales/zh.jsonfrontend/src/main-app.jsxfrontend/src/pages/BatchQueue.jsxfrontend/src/pages/CloneDesignTab.jsxfrontend/src/pages/DonatePage.jsxfrontend/src/pages/DubTab.jsxfrontend/src/pages/EnterprisePage.jsxfrontend/src/pages/Launchpad.jsxfrontend/src/pages/Projects.jsxfrontend/src/pages/Settings.jsxfrontend/src/pages/SetupWizard.jsxfrontend/src/pages/ToolsPage.jsxfrontend/src/pages/Transcriptions.jsxfrontend/src/pages/VoiceGallery.jsxfrontend/src/pages/VoiceProfile.jsxfrontend/src/ui/Dialog.jsxfrontend/vite.config.jsstart-dev.batstart.bat
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/main.py
Comment on lines
+216
to
+218
| const [compareText, setCompareText] = useState(() => { | ||
| try { return i18n?.t?.('voice.default_test_phrase'); } catch { return "The quick brown fox jumps over the lazy dog."; } | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP '\bi18n\?\.t\?\(' frontend/src/App.jsx
rg -nP '^import .*\\bi18n\\b' frontend/src/App.jsxRepository: debpalash/OmniVoice-Studio
Length of output: 52
🏁 Script executed:
#!/bin/bash
# Check if file exists and get line count
ls -la frontend/src/App.jsx 2>/dev/null || echo "File not found at frontend/src/App.jsx"
wc -l frontend/src/App.jsx 2>/dev/null || echo "Cannot count lines"Repository: debpalash/OmniVoice-Studio
Length of output: 167
🏁 Script executed:
#!/bin/bash
# Read lines 216-218 and surrounding context
if [ -f frontend/src/App.jsx ]; then
echo "=== Lines 210-225 ==="
sed -n '210,225p' frontend/src/App.jsx | cat -n
fiRepository: debpalash/OmniVoice-Studio
Length of output: 985
🏁 Script executed:
#!/bin/bash
# Search for import statements related to i18n, translation, or useTranslation
if [ -f frontend/src/App.jsx ]; then
echo "=== Import statements for i18n/translation ==="
head -50 frontend/src/App.jsx | grep -nE 'import.*i18n|import.*useTranslation|import.*translation'
fiRepository: debpalash/OmniVoice-Studio
Length of output: 167
🏁 Script executed:
#!/bin/bash
# Check for all imports at the top of the file
if [ -f frontend/src/App.jsx ]; then
echo "=== First 60 lines (imports section) ==="
head -60 frontend/src/App.jsx | cat -n
fiRepository: debpalash/OmniVoice-Studio
Length of output: 3581
🏁 Script executed:
#!/bin/bash
# Search for useTranslation usage in App.jsx
rg -n 'useTranslation\|const \{ t \}' frontend/src/App.jsx | head -20Repository: debpalash/OmniVoice-Studio
Length of output: 52
Undefined i18n reference prevents localized default compare text.
Line 217 references i18n without a binding. The useTranslation hook is imported but never instantiated in the App component, so the i18n object doesn't exist and the try/catch always falls back to English text.
Call useTranslation() to destructure the t function and use it directly:
Suggested fix
+ const { t } = useTranslation();
+
const [compareText, setCompareText] = useState(() => {
- try { return i18n?.t?.('voice.default_test_phrase'); } catch { return "The quick brown fox jumps over the lazy dog."; }
+ return t('voice.default_test_phrase', { defaultValue: 'The quick brown fox jumps over the lazy dog.' });
});📝 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 [compareText, setCompareText] = useState(() => { | |
| try { return i18n?.t?.('voice.default_test_phrase'); } catch { return "The quick brown fox jumps over the lazy dog."; } | |
| }); | |
| const { t } = useTranslation(); | |
| const [compareText, setCompareText] = useState(() => { | |
| return t('voice.default_test_phrase', { defaultValue: 'The quick brown fox jumps over the lazy dog.' }); | |
| }); |
🤖 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/App.jsx` around lines 216 - 218, The default compareText
initializer references an undefined i18n instead of using the translation hook;
inside the App component call useTranslation() (e.g., const { t } =
useTranslation()) and update the useState initializer for compareText to use
t('voice.default_test_phrase') (with a try/catch or fallback string) so the
localized default is used; update any usages that referenced i18n.t to use the
destructured t function and ensure useTranslation is imported and invoked within
the App component scope.
| <div className="clone-profile-banner"> | ||
| <span className="clone-profile-banner__label"> | ||
| Using profile: {profiles.find(p => p.id === selectedProfile)?.name} | ||
| {t('voice.using_profile')}{profiles.find(p => p.id === selectedProfile)?.name} |
Contributor
There was a problem hiding this comment.
Use interpolation for the selected-profile label.
Line 211 concatenates translated text with the profile name, which breaks spacing/word order in some locales. Pass name through t(...) interpolation instead.
Proposed fix
- {t('voice.using_profile')}{profiles.find(p => p.id === selectedProfile)?.name}
+ {t('voice.using_profile', {
+ name: profiles.find(p => p.id === selectedProfile)?.name ?? '',
+ })}📝 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
| {t('voice.using_profile')}{profiles.find(p => p.id === selectedProfile)?.name} | |
| {t('voice.using_profile', { | |
| name: profiles.find(p => p.id === selectedProfile)?.name ?? '', | |
| })} |
🤖 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/CloneDesignTab.jsx` at line 211, The concatenation at the
JSX expression using t('voice.using_profile') + profiles.find(p => p.id ===
selectedProfile)?.name breaks localization; instead pass the profile name as an
interpolation variable to the translator. Replace the concatenated expression
with a single t call that supplies the name (e.g., t('voice.using_profile', {
name: profiles.find(p => p.id === selectedProfile)?.name })) so translators
control spacing/word order; update the JSX where selectedProfile and profiles
are referenced accordingly.
Comment on lines
76
to
82
| const copyText = useCallback((text) => { | ||
| navigator.clipboard.writeText(text).then( | ||
| () => toast.success('Copied to clipboard'), | ||
| () => toast.error('Copy failed') | ||
| () => toast.success(t('transcriptions.copied_to_clipboard')), | ||
| () => toast.error(t('transcriptions.copy_failed')) | ||
| ); | ||
| }, []); | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify callback dependency arrays around translated toasts.
rg -n -C2 "const copyText = useCallback|const exportAll = useCallback|toast\\.success\\(|toast\\.error\\(" frontend/src/pages/Transcriptions.jsxRepository: debpalash/OmniVoice-Studio
Length of output: 681
Add t to callback dependency arrays to prevent stale translations after language switch.
The copyText (line 76) and exportAll (line 96) callbacks use the t function but omit it from their dependency arrays. This causes the callbacks to retain old translations when the language changes at runtime.
🔧 Proposed fix
const copyText = useCallback((text) => {
navigator.clipboard.writeText(text).then(
() => toast.success(t('transcriptions.copied_to_clipboard')),
() => toast.error(t('transcriptions.copy_failed'))
);
- }, []);
+ }, [t]);
@@
const exportAll = useCallback(() => {
@@
URL.revokeObjectURL(url);
toast.success(t('transcriptions.exported_transcriptions'));
- }, [transcriptions]);
+ }, [transcriptions, t]);🤖 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/Transcriptions.jsx` around lines 76 - 82, The useCallback
hooks for copyText and exportAll capture the translation function t but do not
include it in their dependency arrays, causing stale translations after language
changes; update the useCallback declarations for copyText and exportAll to
include t (e.g., useCallback(..., [t, ...otherDeps])) so they re-create with the
latest translator when language switches.
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
1 participant
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.
Summary
Three fixes to improve reliability on Windows and restricted networks:
1. Windows
kill_orphan_on_portimplementationThe Windows version was an empty function, causing stale backend processes to occupy the port. When a new backend spawned, uvicorn couldn't bind → shutdown → model loading continued in half-dead process → confusing "Could not import module AutoModel" error.
Fix: Implement
kill_orphan_on_portfor Windows usingnetstat -anoto find the listening PID andtaskkill /PID /Fto terminate it.2.
HF_HUB_OFFLINE=1at import timeOn restricted networks, lazy imports deep in the HuggingFace stack could trigger
httpx.ConnectTimeoutbeforelocal_files_only=Truetakes effect.Fix: Set
HF_HUB_OFFLINE=1inmain.pybefore ANY library import, and inmodel_manager._load_model_sync()with proper save/restore.3.
local_files_only=Trueforfrom_pretrainedAdded
local_files_only=TruetoOmniVoice.from_pretrained()call as belt-and-suspenders alongsideHF_HUB_OFFLINE.Tested
/system/inforeturns correct model checkpoint infoSummary by CodeRabbit
Documentation
New Features
Bug Fixes
Chores