Upgrade local TTS to Supertonic 3#554
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds Supertonic3 model-family support with pinned assets, per-file integrity checks and revision marker, model-version-aware preprocessing and chunking, expanded trace logging, new chunking/synthesis commands, and frontend upgrade-state wiring plus chunked Web Audio playback and upgrade UI. ChangesTTS Model Versioning and Upgrade Support
Sequence DiagramsequenceDiagram
participant User
participant UI as Dialog / TTSButton
participant Context as TTSContext
participant Backend as Rust TTS Backend
participant Engine as TextToSpeech Engine
User->>UI: request play / open dialog
UI->>Context: speak() / checkStatus()
Context->>Backend: tts_get_status()
Backend->>Context: models_present_but_incompatible=true, model_version=legacy
Context->>UI: status=upgrade_available, upgradeAvailable=true
User->>UI: delete old models
UI->>Context: deleteModels()
Context->>Backend: tts_delete_models()
Backend->>Context: success
User->>UI: download Supertonic3
UI->>Context: downloadModels()
Context->>Backend: tts_download_models()
Backend->>Backend: download files, verify size+SHA, write revision marker
Context->>Backend: tts_load_models()
Backend->>Context: model_version=supertonic3
UI->>Context: speak()
Context->>Backend: tts_chunk_text -> chunks
loop per chunk
Context->>Backend: tts_synthesize_chunk(chunk)
Backend->>Engine: preprocess_text / synthesize chunk
Engine->>Context: base64 audio
Context->>UI: schedule AudioBufferSourceNode
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Deploying maple with
|
| Latest commit: |
a400809
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bd13cda0.maple-ca8.pages.dev |
| Branch Preview URL: | https://supertonic-3.maple-ca8.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/TTSDownloadDialog.tsx (1)
35-37:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPrevent duplicate delete requests while model deletion is in flight.
Line 35 currently allows repeated
deleteModels()calls, and the delete buttons at Lines 112, 181, and 197 stay clickable during the deleting transition. Add an in-flight guard and disable those buttons to avoid concurrent delete attempts.🔧 Proposed fix
- const handleDelete = async () => { - await deleteModels(); - }; + const handleDelete = async () => { + if (isDeleting) return; + await deleteModels(); + }; ... <Button variant="outline" size="sm" onClick={handleDelete} + disabled={isDeleting} className="text-destructive hover:text-destructive" > ... <Button variant="outline" size="sm" onClick={handleDelete} + disabled={isDeleting} className="text-destructive hover:text-destructive" > ... <Button variant="outline" size="sm" onClick={handleDelete} + disabled={isDeleting} className="text-destructive hover:text-destructive" >Also applies to: 112-120, 181-189, 197-205
🤖 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/TTSDownloadDialog.tsx` around lines 35 - 37, Add an in-flight guard by introducing a boolean state (e.g., isDeleting) in the TTSDownloadDialog component, early-return from handleDelete if isDeleting, set isDeleting = true before calling await deleteModels() and reset it in finally so it always clears after success/error; then wire that state to the delete buttons (the buttons that trigger handleDelete at the spots referenced) by adding disabled={isDeleting} (and optionally a loading indicator) so repeated clicks are ignored while a delete is in progress.
🧹 Nitpick comments (1)
frontend/src-tauri/src/tts.rs (1)
1002-1006: 💤 Low valueMinor: Revision marker is written twice.
The revision marker file is written both before starting downloads (line 1002) and after completing all downloads (line 1147). The second write is redundant since the content is identical. Consider removing one of them—keeping only the post-download write would more accurately reflect successful completion.
That said, this is harmless since
installed_model_version()validates both the revision marker AND all file sizes, so partial downloads won't be incorrectly detected as complete.Also applies to: 1147-1151
🤖 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/tts.rs` around lines 1002 - 1006, Remove the redundant pre-download write of the revision marker so it is only written after successful downloads complete; specifically, delete the fs::write call that writes MODEL_REVISION_FILE with HUGGINGFACE_REVISION before downloads start and keep the existing post-download write (the one around lines 1147) so installed_model_version() still validates revision and file sizes to confirm completion.
🤖 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.
Outside diff comments:
In `@frontend/src/components/TTSDownloadDialog.tsx`:
- Around line 35-37: Add an in-flight guard by introducing a boolean state
(e.g., isDeleting) in the TTSDownloadDialog component, early-return from
handleDelete if isDeleting, set isDeleting = true before calling await
deleteModels() and reset it in finally so it always clears after success/error;
then wire that state to the delete buttons (the buttons that trigger
handleDelete at the spots referenced) by adding disabled={isDeleting} (and
optionally a loading indicator) so repeated clicks are ignored while a delete is
in progress.
---
Nitpick comments:
In `@frontend/src-tauri/src/tts.rs`:
- Around line 1002-1006: Remove the redundant pre-download write of the revision
marker so it is only written after successful downloads complete; specifically,
delete the fs::write call that writes MODEL_REVISION_FILE with
HUGGINGFACE_REVISION before downloads start and keep the existing post-download
write (the one around lines 1147) so installed_model_version() still validates
revision and file sizes to confirm completion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 86b6de01-511a-4f82-b3f1-08bcf2174777
📒 Files selected for processing (4)
frontend/src-tauri/src/tts.rsfrontend/src/components/TTSDownloadDialog.tsxfrontend/src/components/UnifiedChat.tsxfrontend/src/services/tts/TTSContext.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/src/components/TTSDownloadDialog.tsx (1)
64-64: 💤 Low valueConsider breaking long lines to meet the 100-character guideline.
Lines 64, 72, and 73 appear to exceed the 100-character limit specified in the coding guidelines for TypeScript/React files. While Prettier checks passed, you could improve compliance by breaking these template literals across multiple lines or extracting them to constants.
As per coding guidelines: "enforce 100-character line limit in TypeScript/React code."
Example refactor for line 72
- : isUpgradeAvailable - ? `Supertonic 3 is available. Delete your current local TTS models, then download the new ~${Math.round(totalSizeMB)} MB model set.` + : isUpgradeAvailable + ? `Supertonic 3 is available. Delete your current local ` + + `TTS models, then download the new ~${Math.round(totalSizeMB)} MB model set.`Also applies to: 72-73
🤖 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/TTSDownloadDialog.tsx` at line 64, The long inline user-facing strings in TTSDownloadDialog.tsx (e.g., the ternary branch string "TTS is ready with your current local model. Supertonic 3 is available if you want to upgrade." and the other two long template literals later) exceed the 100-character guideline; refactor by extracting each long string into a named constant (e.g., READY_MESSAGE, UPGRADE_NOTE) or split the template literal across multiple concatenated/template-line segments, then reference those constants in the JSX/ternary (look for the ternary that returns that message and the subsequent template literals). Ensure each resulting source line stays under 100 characters and keep the constant names descriptive.
🤖 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.
Nitpick comments:
In `@frontend/src/components/TTSDownloadDialog.tsx`:
- Line 64: The long inline user-facing strings in TTSDownloadDialog.tsx (e.g.,
the ternary branch string "TTS is ready with your current local model.
Supertonic 3 is available if you want to upgrade." and the other two long
template literals later) exceed the 100-character guideline; refactor by
extracting each long string into a named constant (e.g., READY_MESSAGE,
UPGRADE_NOTE) or split the template literal across multiple
concatenated/template-line segments, then reference those constants in the
JSX/ternary (look for the ternary that returns that message and the subsequent
template literals). Ensure each resulting source line stays under 100 characters
and keep the constant names descriptive.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 79a004d0-3a92-4fd4-a5a4-39684d9acf87
📒 Files selected for processing (2)
frontend/src-tauri/src/tts.rsfrontend/src/components/TTSDownloadDialog.tsx
💤 Files with no reviewable changes (1)
- frontend/src-tauri/src/tts.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/tts.rs`:
- Around line 1205-1212: The Supertonic3 path is being forced to
DEFAULT_TTS_LANGUAGE because the lang isn't passed through; update the Tauri
command handler (the function handling the tts_synthesize call in
frontend/src-tauri/src/tts.rs) to accept a lang parameter and pass that lang
into TextToSpeech::synthesize (update the TextToSpeech::synthesize signature to
take a &str or Option<String> lang), ensure the Supertonic3 fallback uses the
provided lang rather than DEFAULT_TTS_LANGUAGE, and update the frontend caller
in frontend/src/services/tts/TTSContext.tsx (the code invoking the
tts_synthesize command) to include the selected/detected language when calling
the command.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0935306f-e928-4e8e-b90a-74a5979a1738
📒 Files selected for processing (1)
frontend/src-tauri/src/tts.rs
| let speed = default_tts_speed(tts.model_version); | ||
|
|
||
| if text.trim().is_empty() { | ||
| return Err("No text to synthesize".to_string()); | ||
| } | ||
|
|
||
| let audio = tts | ||
| .synthesize(&text, &style, 10, 1.2) | ||
| .synthesize(&text, &style, TTS_TOTAL_STEPS, speed) |
There was a problem hiding this comment.
Plumb lang through tts_synthesize instead of forcing English.
Line 1212 still calls synthesize with only text, and the Supertonic3 path inside TextToSpeech::synthesize falls back to DEFAULT_TTS_LANGUAGE. Combined with frontend/src/services/tts/TTSContext.tsx:259-279, which invokes this command with only { text }, every Supertonic3 request is wrapped as <en>...</en>. That makes the new language-aware preprocessing unreachable for non-English input.
Suggested direction
pub async fn tts_synthesize(
text: String,
+ lang: Option<String>,
state: tauri::State<'_, Mutex<TTSState>>,
) -> Result<TTSSynthesizeResponse, String> {
let mut guard = state.lock().map_err(|e| e.to_string())?;
@@
let tts = guard.tts.as_mut().ok_or("TTS engine not loaded")?;
let speed = default_tts_speed(tts.model_version);
+ let lang = lang.as_deref().unwrap_or(DEFAULT_TTS_LANGUAGE);
@@
let audio = tts
- .synthesize(&text, &style, TTS_TOTAL_STEPS, speed)
+ .synthesize(&text, lang, &style, TTS_TOTAL_STEPS, speed)
.map_err(|e| format!("TTS synthesis failed: {e}"))?;This also needs the matching TextToSpeech::synthesize signature change and a frontend update to pass the selected/detected language.
🤖 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/tts.rs` around lines 1205 - 1212, The Supertonic3 path
is being forced to DEFAULT_TTS_LANGUAGE because the lang isn't passed through;
update the Tauri command handler (the function handling the tts_synthesize call
in frontend/src-tauri/src/tts.rs) to accept a lang parameter and pass that lang
into TextToSpeech::synthesize (update the TextToSpeech::synthesize signature to
take a &str or Option<String> lang), ensure the Supertonic3 fallback uses the
provided lang rather than DEFAULT_TTS_LANGUAGE, and update the frontend caller
in frontend/src/services/tts/TTSContext.tsx (the code invoking the
tts_synthesize command) to include the selected/detected language when calling
the command.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/services/tts/TTSContext.tsx (1)
631-684:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCancel the active TTS request during unmount.
The new chunked flow uses
requestIdRefto ignore stale async work, but this cleanup path never invalidates that request. If a pendingtts_chunk_text/tts_synthesize_chunkpromise resolves after unmount, the stale request can still continue decoding or scheduling audio against a torn-down provider.Suggested fix
- useEffect(() => { - const scheduledSourceNodes = scheduledSourceNodesRef.current; - - return () => { - if (unlistenRef.current) { - unlistenRef.current(); - } - if (sourceNodeRef.current) { - try { - sourceNodeRef.current.stop(); - } catch { - // Ignore - } - } - for (const source of scheduledSourceNodes) { - try { - source.stop(); - } catch { - // Ignore - } - } - scheduledSourceNodes.clear(); - if (audioContextRef.current) { - void audioContextRef.current.close().catch(() => { - // Ignore - }); - } - if (audioSessionPrevTypeRef.current) { - try { - // eslint-disable-next-line `@typescript-eslint/no-explicit-any` - const nav = navigator as any; - if (nav.audioSession && typeof nav.audioSession.type === "string") { - nav.audioSession.type = audioSessionPrevTypeRef.current; - } - } catch { - // Ignore - } - audioSessionPrevTypeRef.current = null; - } - - if (mediaSessionPrevStateRef.current) { - try { - if ("mediaSession" in navigator) { - navigator.mediaSession.metadata = mediaSessionPrevStateRef.current.metadata; - navigator.mediaSession.playbackState = mediaSessionPrevStateRef.current.playbackState; - } - } catch { - // Ignore - } - mediaSessionPrevStateRef.current = null; - } - }; - }, []); + useEffect(() => { + return () => { + cleanupDownloadListener(); + stop(); + }; + }, [cleanupDownloadListener, stop]);🤖 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/services/tts/TTSContext.tsx` around lines 631 - 684, Invalidate the active TTS request in the unmount cleanup by resetting the requestIdRef so any in-flight tts_chunk_text / tts_synthesize_chunk promise handlers detect a stale request and bail; inside the existing useEffect cleanup (alongside unlistenRef, sourceNodeRef, scheduledSourceNodesRef, audioContextRef, mediaSessionPrevStateRef) set requestIdRef.current to null (or increment it) so existing async chunk handlers that check requestIdRef will stop decoding/scheduling against the torn-down audio provider.frontend/src/components/UnifiedChat.tsx (1)
809-828:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winExpose the busy TTS states in the ARIA label.
showSpinnernow includes checking/downloading/loading/deleting, but the label still falls back to"Read aloud"for those disabled states. Screen readers will announce an available action while the control is actually busy.Suggested fix
disabled={isDisabled} aria-label={ - isThisPreparing ? "Preparing speech" : isThisPlaying ? "Stop speaking" : "Read aloud" + isThisPreparing + ? "Preparing speech" + : status === "checking" + ? "Checking text-to-speech status" + : status === "downloading" + ? "Downloading text-to-speech models" + : status === "loading" + ? "Loading text-to-speech models" + : status === "deleting" + ? "Removing text-to-speech models" + : isThisPlaying + ? "Stop speaking" + : "Read aloud" }🤖 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/UnifiedChat.tsx` around lines 809 - 828, The ARIA label still announces "Read aloud" when the TTS control is busy; update the Button's aria-label logic to use the same busy condition as showSpinner so screen readers reflect checking/downloading/loading/deleting: use showSpinner (or isThisPreparing) and isThisPlaying/isDisabled to return "Preparing speech" or a short busy label like "Speech busy" when showSpinner is true, otherwise "Stop speaking" when isThisPlaying is true, else "Read aloud"; update the aria-label expression where Button is rendered (referencing showSpinner, isThisPreparing, isThisPlaying, isDisabled) so disabled/busy states are exposed to assistive tech.
🧹 Nitpick comments (1)
frontend/src/services/tts/TTSContext.tsx (1)
362-365: ⚡ Quick winReplace the
anycast on the AudioContext fallback.This new hot-path fallback drops type safety right where the provider now does most of its playback orchestration. A narrow window extension keeps the Safari fallback without punching a hole through the file's strict typing.
As per coding guidelines, "Maintain strict TypeScript typing; avoid using `any` type".Suggested fix
- // eslint-disable-next-line `@typescript-eslint/no-explicit-any` - const AudioContextClass = window.AudioContext || (window as any).webkitAudioContext; + const audioWindow = window as Window & + typeof globalThis & { + webkitAudioContext?: typeof AudioContext; + }; + const AudioContextClass = + audioWindow.AudioContext || audioWindow.webkitAudioContext;🤖 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/services/tts/TTSContext.tsx` around lines 362 - 365, The fallback casting to any for the Safari webkitAudioContext drops TypeScript safety; instead declare webkitAudioContext with the proper type and type AudioContextClass as typeof AudioContext | undefined. Add a Window augmentation (or narrow the expression with Window & { webkitAudioContext?: typeof AudioContext }) so you can use window.AudioContext || window.webkitAudioContext without using any, then assign that to AudioContextClass (symbol AudioContextClass) so playback orchestration remains strictly typed.
🤖 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.
Outside diff comments:
In `@frontend/src/components/UnifiedChat.tsx`:
- Around line 809-828: The ARIA label still announces "Read aloud" when the TTS
control is busy; update the Button's aria-label logic to use the same busy
condition as showSpinner so screen readers reflect
checking/downloading/loading/deleting: use showSpinner (or isThisPreparing) and
isThisPlaying/isDisabled to return "Preparing speech" or a short busy label like
"Speech busy" when showSpinner is true, otherwise "Stop speaking" when
isThisPlaying is true, else "Read aloud"; update the aria-label expression where
Button is rendered (referencing showSpinner, isThisPreparing, isThisPlaying,
isDisabled) so disabled/busy states are exposed to assistive tech.
In `@frontend/src/services/tts/TTSContext.tsx`:
- Around line 631-684: Invalidate the active TTS request in the unmount cleanup
by resetting the requestIdRef so any in-flight tts_chunk_text /
tts_synthesize_chunk promise handlers detect a stale request and bail; inside
the existing useEffect cleanup (alongside unlistenRef, sourceNodeRef,
scheduledSourceNodesRef, audioContextRef, mediaSessionPrevStateRef) set
requestIdRef.current to null (or increment it) so existing async chunk handlers
that check requestIdRef will stop decoding/scheduling against the torn-down
audio provider.
---
Nitpick comments:
In `@frontend/src/services/tts/TTSContext.tsx`:
- Around line 362-365: The fallback casting to any for the Safari
webkitAudioContext drops TypeScript safety; instead declare webkitAudioContext
with the proper type and type AudioContextClass as typeof AudioContext |
undefined. Add a Window augmentation (or narrow the expression with Window & {
webkitAudioContext?: typeof AudioContext }) so you can use window.AudioContext
|| window.webkitAudioContext without using any, then assign that to
AudioContextClass (symbol AudioContextClass) so playback orchestration remains
strictly typed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 996c5930-5a38-4088-853a-39dbb93457d6
📒 Files selected for processing (4)
frontend/src-tauri/src/lib.rsfrontend/src-tauri/src/tts.rsfrontend/src/components/UnifiedChat.tsxfrontend/src/services/tts/TTSContext.tsx
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if audio.is_empty() { | ||
| return Err("No speakable text after preprocessing".to_string()); | ||
| } |
There was a problem hiding this comment.
🔴 tts_synthesize_chunk returns error for chunks empty after normalization, aborting entire playback
The tts_synthesize_chunk command treats empty-after-normalization audio as a hard error (frontend/src-tauri/src/tts.rs:1510-1511), but the internal synthesize method gracefully skips such chunks with continue (frontend/src-tauri/src/tts.rs:647-648). The root cause: tts_chunk_text chunks text before normalization and filters only for non-empty raw text (frontend/src-tauri/src/tts.rs:1383), so a chunk containing only emojis/symbols (e.g. "😊😊😊") passes the filter but becomes empty after normalize_text_for_tts strips them. When synthesize_chunk returns Ok(Vec::new()), the command converts it to Err("No speakable text after preprocessing"). On the frontend, this rejects the invoke promise, which is caught by the outer catch block in speak() (frontend/src/services/tts/TTSContext.tsx:686-703), calling stop() and showing a playback error — aborting the entire multi-chunk session even though other chunks have valid speakable text.
Prompt for agents
The tts_synthesize_chunk Tauri command at tts.rs:1510-1512 returns Err when audio is empty, but the internal synthesize method at tts.rs:647-648 gracefully skips empty chunks. Two things need to change:
1. In tts_synthesize_chunk (tts.rs:1510-1512): Instead of returning an error for empty audio, return a valid TTSSynthesizeResponse with an empty or minimal-silence audio_base64, duration_seconds of 0.0, and the correct sample_rate. Alternatively, return a sentinel that the frontend can detect.
2. In the frontend synthesizeAndDecodeChunk (TTSContext.tsx:548-553): Handle the case where the response contains zero-duration audio — skip scheduling that chunk and continue to the next one instead of trying to decode and play it.
The key constraint is that the frontend for-loop (TTSContext.tsx:652-667) awaits nextChunkPromise and then calls scheduleAudioBuffer. If synthesizeAndDecodeChunk can indicate "skip this chunk" (e.g. returning null or a special marker), the loop should handle it gracefully and continue to the next chunk rather than trying to schedule playback for empty audio.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/TTSDownloadDialog.tsx (1)
114-136:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't expose model deletion while legacy models are auto-loading.
checkStatus()setsupgradeAvailablebefore it awaitstts_load_models()for legacy installs, so this branch also renders duringstatus === "loading". That makes "Delete Old Models" clickable while the backend is loading the same files, which can race into a bogusreadystate or a failed load. Gate this panel onstatus === "upgrade_available"or at least disable the action for all processing states.Proposed fix
- {isUpgradeAvailable && !isReady && ( + {status === "upgrade_available" && ( <div className="space-y-3 rounded-lg border p-3"> @@ <Button variant="outline" size="sm" onClick={handleDelete} - disabled={isDeleting} + disabled={isProcessing} className="text-destructive hover:text-destructive" >🤖 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/TTSDownloadDialog.tsx` around lines 114 - 136, The UI currently shows the "Delete Old Models" panel whenever isUpgradeAvailable && !isReady, which can occur while checkStatus() is still awaiting tts_load_models() and causes a race; update the render condition to require status === "upgrade_available" instead of just isUpgradeAvailable (i.e., replace the outer condition with status === "upgrade_available" && !isReady) or, if you prefer to keep the panel visible earlier, make the Delete action safe by disabling the Button unless status === "upgrade_available" (ensure handleDelete remains disabled when status is any processing state or when isDeleting is true).
🧹 Nitpick comments (2)
frontend/src/components/TTSDownloadDialog.tsx (1)
190-199: ⚡ Quick winUse the shared slider component here instead of a raw range input.
This bypasses the project UI kit and will drift from shared styling/accessibility behavior. As per coding guidelines, "Use existing shadcn/ui components from
src/components/ui/for UI elements in React".🤖 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/TTSDownloadDialog.tsx` around lines 190 - 199, The raw range input in TTSDownloadDialog (the <input type="range"> using playbackSpeed, setPlaybackSpeed and constants TTS_MIN_PLAYBACK_SPEED / TTS_MAX_PLAYBACK_SPEED / TTS_PLAYBACK_SPEED_STEP) should be replaced with the shared Slider UI component used across the project; import and use the shared Slider from the ui components, bind its value to playbackSpeed and update via setPlaybackSpeed (convert to Number if the Slider emits strings), and pass the min/max/step props plus the same className/aria-label to preserve styling and accessibility consistent with other components.frontend/src-tauri/src/tts.rs (1)
1226-1234: ⚡ Quick winUse the approved Rust log macros in these re-download paths.
These branches log with
log::warn!, but the repository Rust rules restrict logging tolog::info!andlog::error!. As per coding guidelines, "Uselog::info!andlog::error!for logging in Rust code".🤖 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/tts.rs` around lines 1226 - 1234, Replace the unapproved log::warn! calls in the TTS model checksum verification branches: in the Ok(actual_sha256) branch (where expected_sha256 vs actual_sha256 are compared for file_name) change the log macro to log::info!; in the Err(err) branch (where verification failed for file_name with err) change the log macro to log::error! so the code uses only the approved log::info! and log::error! macros.
🤖 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.
Outside diff comments:
In `@frontend/src/components/TTSDownloadDialog.tsx`:
- Around line 114-136: The UI currently shows the "Delete Old Models" panel
whenever isUpgradeAvailable && !isReady, which can occur while checkStatus() is
still awaiting tts_load_models() and causes a race; update the render condition
to require status === "upgrade_available" instead of just isUpgradeAvailable
(i.e., replace the outer condition with status === "upgrade_available" &&
!isReady) or, if you prefer to keep the panel visible earlier, make the Delete
action safe by disabling the Button unless status === "upgrade_available"
(ensure handleDelete remains disabled when status is any processing state or
when isDeleting is true).
---
Nitpick comments:
In `@frontend/src-tauri/src/tts.rs`:
- Around line 1226-1234: Replace the unapproved log::warn! calls in the TTS
model checksum verification branches: in the Ok(actual_sha256) branch (where
expected_sha256 vs actual_sha256 are compared for file_name) change the log
macro to log::info!; in the Err(err) branch (where verification failed for
file_name with err) change the log macro to log::error! so the code uses only
the approved log::info! and log::error! macros.
In `@frontend/src/components/TTSDownloadDialog.tsx`:
- Around line 190-199: The raw range input in TTSDownloadDialog (the <input
type="range"> using playbackSpeed, setPlaybackSpeed and constants
TTS_MIN_PLAYBACK_SPEED / TTS_MAX_PLAYBACK_SPEED / TTS_PLAYBACK_SPEED_STEP)
should be replaced with the shared Slider UI component used across the project;
import and use the shared Slider from the ui components, bind its value to
playbackSpeed and update via setPlaybackSpeed (convert to Number if the Slider
emits strings), and pass the min/max/step props plus the same
className/aria-label to preserve styling and accessibility consistent with other
components.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c5303bd2-0c39-4647-9780-940de7e28fdf
📒 Files selected for processing (3)
frontend/src-tauri/src/tts.rsfrontend/src/components/TTSDownloadDialog.tsxfrontend/src/services/tts/TTSContext.tsx
1 participant
Summary
Verification
Summary by CodeRabbit
New Features
Bug Fixes