fix: improve auto caption runtime setup and generation flow#712
fix: improve auto caption runtime setup and generation flow#712QuentinCrane wants to merge 4 commits into
Conversation
📝 WalkthroughWalkthroughWhisper caption generation now supports executable and model discovery, FFmpeg status checks, companion-audio fallbacks, streamed progress events, expanded IPC APIs, setup-aware editor controls, determinate progress UI, preference clearing, and localized workflow text. ChangesCaption generation workflow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant VideoEditor
participant CaptionIPC
participant CaptionGenerator
participant WhisperProcess
VideoEditor->>CaptionIPC: request caption generation
CaptionIPC->>CaptionGenerator: generate with progress callback
CaptionGenerator->>WhisperProcess: spawn Whisper command
WhisperProcess-->>CaptionGenerator: stream transcription progress
CaptionGenerator-->>CaptionIPC: emit caption progress
CaptionIPC-->>VideoEditor: deliver progress event
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
acd7bf5 to
2591889
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/video-editor/SettingsPanel.tsx (1)
3016-3024: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not expose “Delete Model” for manually selected models.
This button appears for every
whisperModelPath, but its handler deletes only the bundled small model. Track whether the selected path is the downloaded model; otherwise provide a non-destructive “Clear selection” action.🤖 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 `@src/components/video-editor/SettingsPanel.tsx` around lines 3016 - 3024, Update the whisper model action in the SettingsPanel component so “Delete Model” is shown only when the selected whisperModelPath refers to the bundled downloaded small model. For manually selected paths, provide a non-destructive “Clear selection” action that removes the selection without deleting files, using the existing selection state or handler mechanisms.src/i18n/locales/es/settings.json (1)
133-198: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRestore diacritics across the new Latin-language caption translations.
The new strings were ASCII-transliterated, producing visibly incorrect localized UI.
src/i18n/locales/es/settings.json#L133-L198: restore Spanish accents such assubtítulo,ningún,encontró, andconfiguración.src/i18n/locales/fr/settings.json#L134-L198: restore French accents such asmis à jour,modèle,sélectionné, andgénérés.src/i18n/locales/it/settings.json#L182-L218: restore Italian forms such asèandpuò.🤖 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 `@src/i18n/locales/es/settings.json` around lines 133 - 198, Restore the missing language-specific diacritics in the caption translation strings: update the affected Spanish entries in src/i18n/locales/es/settings.json (lines 133-198), French entries in src/i18n/locales/fr/settings.json (lines 134-198), and Italian entries in src/i18n/locales/it/settings.json (lines 182-218), including forms such as “subtítulo”, “ningún”, “encontró”, “configuración”, “mis à jour”, “modèle”, “sélectionné”, “générés”, “è”, and “può”, without changing the translation meanings or keys.
🤖 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 `@electron/ipc/captions/generate.ts`:
- Around line 57-63: Update the timeout handling around the Whisper child
process so the timeout callback only signals termination, then reject from the
child’s close handler after Whisper has exited. Add a short grace period after
child.kill() and forcefully kill the process if it remains alive, while
preserving the existing timeout error and cleanup flow.
In `@electron/ipc/register/captions.ts`:
- Around line 212-215: Update the Windows extension filter near
runWhisperCommand usage to allow only executable files by removing cmd and bat
extensions, unless runWhisperCommand is also changed to launch scripts through
cmd.exe. Preserve the wildcard filter for non-Windows platforms.
In `@src/components/video-editor/VideoEditor.tsx`:
- Around line 2989-3000: Update the success-toast logic near
setAutoCaptionSettings in VideoEditor so empty result.cues do not claim captions
were enabled. Show the existing “generated and turned on” message only when
cues.length is greater than zero, and use an appropriate separate success
message for zero cues or skip the toast.
- Around line 2763-2782: Update the asynchronous runtime-status checks in the
VideoEditor initialization flow to catch rejected getWhisperRuntimeStatus and
getCaptionFfmpegStatus IPC calls, clearing the corresponding executable path and
setting its error/status state instead of leaving it checking. Also clear stale
persisted Whisper and FFmpeg paths when results are unsuccessful, while
preserving the existing successful-path updates.
In `@src/i18n/locales/pt-BR/settings.json`:
- Around line 153-198: Restore Brazilian Portuguese diacritics throughout the
added translation block, including accents for words such as contém, não, áudio,
vídeo, configuração, and exibição. Proofread every value from engineHelp through
generatedToast and correct all comparable missing accents without changing the
translation meaning or keys.
---
Outside diff comments:
In `@src/components/video-editor/SettingsPanel.tsx`:
- Around line 3016-3024: Update the whisper model action in the SettingsPanel
component so “Delete Model” is shown only when the selected whisperModelPath
refers to the bundled downloaded small model. For manually selected paths,
provide a non-destructive “Clear selection” action that removes the selection
without deleting files, using the existing selection state or handler
mechanisms.
In `@src/i18n/locales/es/settings.json`:
- Around line 133-198: Restore the missing language-specific diacritics in the
caption translation strings: update the affected Spanish entries in
src/i18n/locales/es/settings.json (lines 133-198), French entries in
src/i18n/locales/fr/settings.json (lines 134-198), and Italian entries in
src/i18n/locales/it/settings.json (lines 182-218), including forms such as
“subtítulo”, “ningún”, “encontró”, “configuración”, “mis à jour”, “modèle”,
“sélectionné”, “générés”, “è”, and “può”, without changing the translation
meanings or keys.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: aabbefe0-6e0c-4311-afdd-c045b58284af
📒 Files selected for processing (20)
electron/electron-env.d.tselectron/ipc/captions/generate.test.tselectron/ipc/captions/generate.tselectron/ipc/captions/whisper.tselectron/ipc/register/captions.tselectron/preload.tssrc/components/video-editor/SettingsPanel.tsxsrc/components/video-editor/VideoEditor.tsxsrc/components/video-editor/captionSetupStatus.test.tssrc/components/video-editor/captionSetupStatus.tssrc/i18n/locales/en/settings.jsonsrc/i18n/locales/es/settings.jsonsrc/i18n/locales/fr/settings.jsonsrc/i18n/locales/it/settings.jsonsrc/i18n/locales/ko/settings.jsonsrc/i18n/locales/nl/settings.jsonsrc/i18n/locales/pt-BR/settings.jsonsrc/i18n/locales/ru/settings.jsonsrc/i18n/locales/zh-CN/settings.jsonsrc/i18n/locales/zh-TW/settings.json
Summary
WHISPER_CPP_PATH, common Windows/macOS locations, the bundled runtime, andPATHmainContext
Addresses the remaining Windows and runtime-discovery paths reported in #220. The macOS packaging-only path was handled separately in #333. Proxy-aware model downloading from #704 remains a separate network/download concern; this PR provides a clear manual-model path but does not claim to implement proxy transport.
Validation
main(360b160); GitHub reports the PR mergeablenpx tsc --noEmitnpm run lint(passes; existing repository hook warnings remain non-blocking)npx vitest --run electron/ipc/captions src/components/video-editor/captionSetupStatus.test.ts src/components/video-editor/editorPreferences.test.ts(67 tests passed)npm test: 107 suites / 1006 tests passed; one unrelated exporter suite timed out during parallel import, then passed 5/5 when rerun alonef6d38b9git diff --check origin/mainManual verification previously completed
ggml-small.bin, andC:\Tools\whisper\whisper-cli.exeFollow-up direction
A larger caption workflow can build on the merged timeline/editor for translation, proofreading, and burn-in/soft-mux export. This PR stays focused on making Recordly's existing auto-caption generation path reliable and understandable.
Summary by CodeRabbit