plugins/connector/permissions/schemas/schema.json (1)

478-488: Remove all stale STT permission references across schema and autogenerated artifacts

The get_stt_connection command has been removed, but its permission variants still linger in the JSON schema, generated reference docs, and TOML definitions. Please remove them to avoid validating against a non-existent command:

• plugins/connector/permissions/schemas/schema.json
Remove these entries around lines 478–488:

-        {
-          "description": "Enables the get_stt_connection command without any pre-configured scope.",
-          "type": "string",
-          "const": "allow-get-stt-connection",
-          "markdownDescription": "Enables the get_stt_connection command without any pre-configured scope."
-        },
-        {
-          "description": "Denies the get_stt_connection command without any pre-configured scope.",
-          "type": "string",
-          "const": "deny-get-stt-connection",
-          "markdownDescription": "Denies the get_stt_connection command without any pre-configured scope."
-        },

• plugins/connector/permissions/autogenerated/reference.md
Remove the sections referencing

`connector:allow-get-stt-connection`
…
Enables the get_stt_connection command without any pre-configured scope.
…
`connector:deny-get-stt-connection`
…
Denies the get_stt_connection command without any pre-configured scope.

• plugins/connector/permissions/autogenerated/commands/get_stt_connection.toml
Either delete this file entirely or remove these blocks:

identifier = "allow-get-stt-connection"
description = "Enables the get_stt_connection command without any pre-configured scope."
commands.allow = ["get_stt_connection"]

identifier = "deny-get-stt-connection"
description = "Denies the get_stt_connection command without any pre-configured scope."
commands.deny = ["get_stt_connection"]

After applying these changes, rerun your search script:

rg -n 'get_stt_connection|allow-get-stt-connection|deny-get-stt-connection'

to confirm no STT tokens remain.

plugins/connector/src/ext.rs (2)

94-101: Avoid panic: replace unwrap() on api_base with explicit error

guard.api_base.clone().unwrap() will panic if the local LLM server is reported as running but the api_base hasn’t been set. Return a meaningful error instead of panicking.

Apply this diff:

-        let api_base = if self.is_server_running().await {
-            let state = self.state::<SharedState>();
-            let guard = state.lock().await;
-            guard.api_base.clone().unwrap()
-        } else {
+        let api_base = if self.is_server_running().await {
+            let state = self.state::<SharedState>();
+            let guard = state.lock().await;
+            guard
+                .api_base
+                .clone()
+                .ok_or_else(|| crate::Error::UnknownError(
+                    "local_llm_server_running_but_api_base_missing".into()
+                ))?
+        } else {
             self.start_server().await?
         };

---

113-124: Validate custom LLM config: error if enabled but API base is missing/blank

When CustomEnabled is true, we currently default api_base to an empty string. That defers failure to later HTTP calls with ambiguous errors. Fail fast with a clear error.

Apply this diff:

-        if custom_enabled {
-            let api_base = store
-                .get::<Option<String>>(StoreKey::CustomApiBase)?
-                .flatten()
-                .unwrap_or_default();
-            let api_key = store
-                .get::<Option<String>>(StoreKey::CustomApiKey)?
-                .flatten();
-
-            let conn = ConnectionLLM::Custom(Connection { api_base, api_key });
-            Ok(conn)
-        } else {
+        if custom_enabled {
+            let api_base = store
+                .get::<Option<String>>(StoreKey::CustomApiBase)?
+                .flatten()
+                .filter(|s| !s.trim().is_empty())
+                .ok_or_else(|| crate::Error::UnknownError(
+                    "custom_llm_enabled_but_api_base_missing".into()
+                ))?;
+            let api_key = store
+                .get::<Option<String>>(StoreKey::CustomApiKey)?
+                .flatten();
+
+            Ok(ConnectionLLM::Custom(Connection { api_base, api_key }))
+        } else {

plugins/local-llm/src/ext.rs (1)

42-45: Suspicious hardcoded models directory name "ttt".

"ttt" looks like a leftover placeholder; this will change on-disk layout and likely break existing installs or confuse users.

Apply this diff to use a stable, self-explanatory directory name:

-    fn models_dir(&self) -> PathBuf {
-        self.path().app_data_dir().unwrap().join("ttt")
-    }
+    fn models_dir(&self) -> PathBuf {
+        // Store local LLM models under app data
+        self.path().app_data_dir().unwrap().join("models")
+    }

crates/whisper-local-model/src/lib.rs (1)

82-92: Update all .model_size() calls to .model_size_bytes()

Renaming model_size to model_size_bytes improves clarity but leaves several stale references. Please update the following call sites (and any related proxy methods) to use .model_size_bytes():

• owhisper/owhisper-server/src/server.rs:185

  -    .model_size(config.size.clone())
  +    .model_size_bytes(config.size.clone())

• plugins/local-llm/src/commands.rs:19, 25, 31
• plugins/local-llm/src/ext.rs:75
• plugins/local-llm/src/model.rs:39 (pub fn model_size(&self) -> u64) – consider renaming this proxy to model_size_bytes or delegating internally.

crates/file/src/lib.rs (1)

5-13: Re-export DownloadProgress to avoid breaking callers and fix test pattern matches.

By switching to hypr_download_interface::DownloadProgress without re-exporting it, pattern matches like DownloadProgress::Started in this crate’s tests (and any consumers using the crate’s previous symbol path) will no longer resolve. Re-exporting preserves the prior public API surface and keeps tests working.

Apply this diff:

 pub use local::*;
 pub use remote::*;
 pub use types::*;
+
+// Why: preserve public API and keep tests/consumers using `crate::DownloadProgress` working.
+pub use hypr_download_interface::DownloadProgress;
 
 use {
     crate::Error::OtherError,
     futures_util::{stream::FuturesUnordered, StreamExt, TryStreamExt},
-    hypr_download_interface::DownloadProgress,
     reqwest::StatusCode,
     std::{
         cmp::min,
         fs::File,
         fs::OpenOptions,

apps/desktop/src/components/toast/shared.tsx (1)

65-68: Fix query invalidation key: mismatch prevents dependent UI from refreshing.

Listeners use ["check-stt-model-downloaded"] and ["check-any-stt-model-downloaded"], but this function invalidates ["check-model-downloaded"]. Align the keys to ensure invalidation works.

Apply this diff:

   if (queryClient) {
-    queryClient.invalidateQueries({ queryKey: ["stt-model-downloading"] });
-    queryClient.invalidateQueries({ queryKey: ["check-model-downloaded"] });
+    queryClient.invalidateQueries({ queryKey: ["stt-model-downloading"] });
+    // Why: keep STT model status in sync across UI after download completes.
+    queryClient.invalidateQueries({ queryKey: ["check-stt-model-downloaded"] });
+    queryClient.invalidateQueries({ queryKey: ["check-any-stt-model-downloaded"] });
   }

plugins/listener/src/fsm.rs (1)

565-594: Don’t log secrets: remove api_key from tracing.

api_key is included in the listen_client log line. This leaks credentials to logs.

Apply this diff to remove api_key from logs:

-    tracing::info!(api_base = ?api_base, api_key = ?api_key, languages = ?languages, "listen_client");
+    tracing::info!(api_base = ?api_base, languages = ?languages, "listen_client");

plugins/connector/src/ext.rs (1)

166-172: Optional: reuse a single reqwest Client to reduce overhead

reqwest::Client::new() is cheap but not free; reusing a single client is the recommended pattern and enables connection pooling and keep-alives.

Suggested pattern (outside this hunk):

use once_cell::sync::Lazy;

static HTTP: Lazy<reqwest::Client> = Lazy::new(reqwest::Client::new);

Then here:

-        let mut req = reqwest::Client::new().get(url);
+        let mut req = HTTP.get(url);

crates/transcribe-whisper-local/src/service/streaming.rs (1)

123-126: Nit: simplify with clamp for readability

Same behavior, more idiomatic and readable.

Apply this diff:

-    let redemption_time = Duration::from_millis(std::cmp::min(
-        std::cmp::max(params.redemption_time_ms.unwrap_or(500), 300),
-        1200,
-    ));
+    let redemption_time_ms = params.redemption_time_ms.unwrap_or(500).clamp(300, 1200);
+    let redemption_time = Duration::from_millis(redemption_time_ms);

crates/download-interface/src/lib.rs (1)

1-6: Add Clone/PartialEq/Eq and mark enum non_exhaustive for forward-compat.

Deriving these traits eases comparisons in tests and state machines. non_exhaustive lets you add variants later without breaking downstream exhaustive matches.

Apply this diff:

-#[derive(Debug, serde::Serialize, serde::Deserialize)]
+#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
+#[non_exhaustive]
 pub enum DownloadProgress {
     Started,
     Progress(u64, u64),
     Finished,
 }

crates/transcribe-moonshine/src/service/streaming.rs (1)

152-155: Prefer clamp() for readability and to avoid nested min/max.

This keeps the intent obvious and avoids nesting. No behavior change.

Apply this diff:

-    let redemption_time = Duration::from_millis(std::cmp::min(
-        std::cmp::max(params.redemption_time_ms.unwrap_or(500), 300),
-        1200,
-    ));
+    let redemption_ms = params.redemption_time_ms.unwrap_or(500).clamp(300, 1200);
+    let redemption_time = Duration::from_millis(redemption_ms);

plugins/local-llm/src/ext.rs (1)

98-116: Clamp in-flight progress to 0..=99 to avoid flicker and cast issues.

During Progress events, rounding can emit 100 before Finished triggers. Clamp to 99 for in-flight updates and keep 100 only on Finished for consistent UX.

Apply this diff:

-                DownloadProgress::Progress(downloaded, total_size) => {
-                    let percent = (downloaded as f64 / total_size as f64) * 100.0;
-                    let _ = channel.send(percent as i8);
-                }
+                DownloadProgress::Progress(downloaded, total_size) => {
+                    let percent = (downloaded as f64 / total_size as f64) * 100.0;
+                    // Keep 100 reserved for Finished to avoid flicker
+                    let percent = percent.clamp(0.0, 99.0) as i8;
+                    let _ = channel.send(percent);
+                }

plugins/local-stt/src/server/external.rs (4)

33-36: Return the server status instead of discarding it.

Expose the status payload to callers; it’s more useful than just Ok(()).

Apply this diff:

-    pub async fn status(&self) -> Result<(), crate::Error> {
-        self.client.status().await?;
-        Ok(())
-    }
+    pub async fn status(&self) -> Result<hypr_am::ServerStatus, crate::Error> {
+        self.client.status().await
+    }

---

43-47: Avoid hardcoding the port; prefer dynamic binding or a parameter.

A fixed port (6942) risks conflicts. Consider accepting a port parameter or discovering a free port at runtime and propagating it to the child process.

Would you like a follow-up patch that negotiates an available port and passes it to the external command?

---

1-7: Avoid persisting API keys in long-lived structs.

Storing api_key on ServerHandle increases exposure risk. Prefer passing the key to init() and not keeping it afterward.

If you must keep it, ensure it’s never logged and consider zeroizing after init if possible.

---

47-56: Shutdown channel is unused in this external-process flow.

You create a watch channel but never hold the receiver or propagate it to anything. terminate() kills the child directly, so the channel adds no value.

Options:

• Remove the shutdown field and the channel entirely in this external server variant, or
• Hook the receiver up to some shutdown mechanism if the child can listen for it (e.g., via IPC or a signal).

crates/am/src/model.rs (1)

24-64: Accessor methods are straightforward; consider centralizing metadata.

The repeated match statements are fine, but a static table or macro could reduce duplication and ease future additions.

If desired, I can provide a follow-up refactor using a single metadata table and derive helpers.

apps/desktop/src/components/toast/shared.tsx (2)

60-63: Type the Tauri Channel for stronger type-safety.

Make the message type explicit to match DownloadProgress’s number semantics and reduce implicit any.

Apply this diff:

-  const sttChannel = new Channel();
+  const sttChannel = new Channel<number>();

---

102-106: Nit: type the LLM download channel as well.

Mirror the STT channel typing for consistency and type-safety.

Apply this diff:

-  const llmChannel = new Channel();
+  const llmChannel = new Channel<number>();

apps/desktop/src/components/settings/components/ai/stt-view-local.tsx (1)

170-176: Avoid hard-coding pro model keys; derive from source of truth.

Hard-coded whitelists drift easily. If possible, filter by a property (e.g., a pro/tier flag) returned by listSupportedModels(), or centralize the whitelist in one shared place to keep UI consistent.

Example (if the API exposes a boolean flag):

const models = await localSttCommands.listSupportedModels();
return models.filter((m) => m.pro === true);

If not, at least extract the whitelist to a constant exported from a shared module to avoid duplications across the UI.

apps/desktop/src/components/welcome-modal/model-selection-view.tsx (3)

16-16: Default selection should adapt to available models.

If QuantizedSmall isn’t present, the default will be a non-existent option. Prefer deferring selection until data loads, then choose the first allowed model.

You can add this effect to set a sane default when data arrives:

// Place near other hooks
const allowed = new Set<SupportedSttModel>(["QuantizedTiny", "QuantizedSmall", "QuantizedLargeTurbo"]);
useEffect(() => {
  const first = supportedSTTModels.data?.find((mi) => allowed.has(mi.model.key))?.model.key;
  if (first) setSelectedModel(first);
}, [supportedSTTModels.data]);

If you adopt this, consider initializing selectedModel to a placeholder state (e.g., undefined) and adjusting the button disabled logic accordingly.

---

65-66: Remove redundant type assertion.

model.key is already a SupportedSttModel per generated bindings. The cast is unnecessary.

- onClick={() => setSelectedModel(model.key as SupportedSttModel)}
+ onClick={() => setSelectedModel(model.key)}

---

90-93: Disable Continue while loading or when no valid selection.

selectedModel is always truthy with the current default, so the disabled guard is ineffective. Consider disabling while the query is loading or until a valid selection is set.

- disabled={!selectedModel}
+ disabled={supportedSTTModels.isLoading || !selectedModel}

If you keep a non-null default, you can skip the second clause.

plugins/local-stt/src/lib.rs (1)

30-48: Consider explicitly exporting SupportedSttModel and SttModelInfo types.

Specta should pull these through list_supported_models, but adding explicit .typ::() and .typ::() can make the export intent clearer and resilient to future changes.

   tauri_specta::Builder::<R>::new()
       .plugin_name(PLUGIN_NAME)
       .commands(tauri_specta::collect_commands![
         commands::models_dir::<Wry>,
         // ...
         commands::list_supported_models,
       ])
-      .typ::<hypr_whisper_local_model::WhisperModel>()
+      .typ::<hypr_whisper_local_model::WhisperModel>()
+      .typ::<crate::SupportedSttModel>()
+      .typ::<crate::SttModelInfo>()
       .error_handling(tauri_specta::ErrorHandlingMode::Throw)

crates/am/src/hf.rs (4)

89-107: Relax callback trait bounds; Sync isn’t needed.

Callback is protected by a Mutex; requiring Sync unnecessarily constrains F and callers.

-where
-    F: FnMut(DownloadProgress) + Send + Sync + Clone,
+where
+    F: FnMut(DownloadProgress) + Send + Clone,

And align Progress impl bound similarly:

-where
-    F: FnMut(DownloadProgress) + Send + Sync + Clone,
+where
+    F: FnMut(DownloadProgress) + Send + Clone,

---

217-223: Pre-calc HEAD requests sequentially; consider parallelizing.

HEAD-ing each file in a loop can slow large repos. Fire them concurrently to reduce latency.

-    let mut total_bytes = 0u64;
-    for file_path in &files_to_download {
-        let size = get_file_size(&api, &repo, file_path).await?;
-        total_bytes += size;
-    }
+    let size_futs = files_to_download
+        .iter()
+        .map(|fp| get_file_size(&api, &repo, fp));
+    let sizes = futures_util::future::try_join_all(size_futs).await?;
+    let total_bytes = sizes.into_iter().sum();

Note: This adds a futures_util::future import if not already present.

---

50-87: Unused field: file_total_size.

file_total_size is set but never read. Drop it to reduce noise.

 struct CallbackProgress<F> {
   callback: Arc<Mutex<F>>,
   tracker: ProgressTracker,
   current_file: String,
   current_size: usize,
-  file_total_size: usize,
 }
@@
   async fn init(&mut self, size: usize, _filename: &str) {
-      self.file_total_size = size;
       self.current_size = 0;
   }
@@
   fn clone(&self) -> Self {
       Self {
           callback: self.callback.clone(),
           tracker: self.tracker.clone(),
           current_file: self.current_file.clone(),
           current_size: self.current_size,
-          file_total_size: self.file_total_size,
       }
   }

---

268-304: DRY opportunity: share file selection logic between functions.

download and is_folder_downloaded reimplement the same dir filtering. Factor into a small helper to reduce divergence risk.

I can extract a small helper (should_include(dir_path_str: &str, file_path: &str) -> bool) and reuse it in both places. Want me to push a patch?

plugins/local-stt/src/commands.rs (1)

24-26: Streamline the error handling for list_supported_models

The function returns Result<Vec<SttModelInfo>, String> but the operation cannot fail. Consider removing the Result wrapper since SUPPORTED_MODELS.iter().map(|m| m.info()).collect() is infallible.

-pub async fn list_supported_models() -> Result<Vec<SttModelInfo>, String> {
-    Ok(SUPPORTED_MODELS.iter().map(|m| m.info()).collect())
+pub async fn list_supported_models() -> Vec<SttModelInfo> {
+    SUPPORTED_MODELS.iter().map(|m| m.info()).collect()
}

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

apps/desktop/src/locales/ko/messages.po (2)

1-15: Set correct Plural-Forms for Korean to avoid wrong plural handling at runtime

Plural-Forms is empty. For ko, it should be nplurals=1; plural=0;. Without this, plural selection can be incorrect.

Apply this diff:

 "Language-Team: \n"
-"Plural-Forms: \n"
+"Plural-Forms: nplurals=1; plural=0;\n"
 

---

603-611: Translate “Continue”

This new, widely used CTA should have a Korean translation.

Apply this diff:

 msgid "Continue"
-msgstr ""
+msgstr "계속"

plugins/local-stt/src/types.rs (1)

1-4: Consider adding Clone derive for async boundaries

The Connection struct should also derive Clone for ergonomic usage across async boundaries and method chaining, as suggested in the past review comment.

Apply this diff:

-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct Connection {
     pub base_url: String,
 }

plugins/local-stt/src/ext.rs (1)

316-320: Add error handling for AM model download failures

The AM model download silently ignores errors, unlike the Whisper download which logs errors and sends -1 on failure. This inconsistency was noted in a past review.

 SupportedSttModel::Am(m) => {
     let task = tokio::spawn(async move {
         let callback = create_progress_callback(channel.clone());
-        let _ = hypr_am::download(m.repo_name(), m.model_dir(), callback).await;
+        if let Err(e) = hypr_am::download(m.repo_name(), m.model_dir(), callback).await {
+            tracing::error!("model_download_error: {}", e);
+            let _ = channel.send(-1);
+        }
     });

plugins/windows/src/ext.rs (2)

314-317: Settings window now resizable — add a min size to protect layout; verify borderless-resize UX on Windows

Resizing Settings makes sense given the expanded UI. Recommend enforcing a floor to prevent layout breakage and keep consistency with Finder and other windows that set min_inner_size.

Apply:

 Self::Settings => self
     .window_builder(app, "/app/settings")
     .resizable(true)
     .inner_size(800.0, 600.0)
+    .min_inner_size(800.0, 600.0)
     .build()?,

Also, on Windows we use decorations(false) globally for windows. Please verify that borderless edge-resize works as expected for Settings, or add your existing resize hit-test solution if needed.

---

5-5: Remove unused import tauri_specta::Event

This import isn’t referenced in this file; drop it to satisfy the “no unused imports” guideline and keep clippy quiet.

-use tauri_specta::Event;

owhisper/owhisper-client/Cargo.toml (1)

18-20: Consider gating tracing behind an optional feature to minimize downstream dependency/overhead.

If this crate is a library consumed by others, making tracing optional lets dependents opt-in to telemetry and reduces compile-time footprint by default. Only proceed if usage sites can be feature-gated.

Proposed manifest changes (requires gating usage in code with #[cfg(feature = "telemetry")] or similar):

 [dependencies]
 futures-util = { workspace = true }
-tracing = { workspace = true }
+tracing = { workspace = true, optional = true }

Add a features section:

[features]
telemetry = ["dep:tracing"]

Note: Any direct uses of tracing::* macros/attributes must be conditionally compiled or no-op wrapped when the feature is disabled.

crates/whisper-local/src/model.rs (3)

179-179: Reduce log noise and use structured fields for language selection logs.

These are internal decision logs; debug-level with structured fields makes them more useful and less noisy at info-level.

Apply this diff:

-            tracing::info!("no_language_specified");
+            tracing::debug!("no_language_specified");

-            tracing::info!("single_language_specified: {}", lang);
+            tracing::debug!(?lang, "single_language_specified");

-            tracing::info!("predicted: {:#?}, from: {:#?}", best_lang, self.languages);
+            tracing::debug!(?best_lang, languages = ?self.languages, "language_predicted");

Optionally, prefer idiomatic emptiness check:

-        if self.languages.len() == 0 {
+        if self.languages.is_empty() {

Also applies to: 186-186, 207-207

---

346-347: Tests: guard against missing test model to avoid spurious panics.

If model.bin isn't present in CI/dev environments, this will panic. Skip gracefully when absent.

Apply this diff:

-        let mut whisper = Whisper::builder()
-            .model_path(concat!(env!("CARGO_MANIFEST_DIR"), "/model.bin"))
+        let model_path = concat!(env!("CARGO_MANIFEST_DIR"), "/model.bin");
+        if !std::path::Path::new(model_path).exists() {
+            eprintln!("skipping test_whisper: model.bin not found at {}", model_path);
+            return;
+        }
+        let mut whisper = Whisper::builder()
+            .model_path(model_path)
             .build()
             .unwrap();

---

369-371: Tests: guard llama test Whisper build similarly.

Mirror the skip in the llama test to avoid intermittent failures in environments without model.bin.

Apply this diff:

-        let mut whisper = Whisper::builder()
-            .model_path(concat!(env!("CARGO_MANIFEST_DIR"), "/model.bin"))
-            .build()
-            .unwrap();
+        let model_path = concat!(env!("CARGO_MANIFEST_DIR"), "/model.bin");
+        if !std::path::Path::new(model_path).exists() {
+            eprintln!("skipping test_whisper_with_llama: model.bin not found at {}", model_path);
+            return;
+        }
+        let mut whisper = Whisper::builder()
+            .model_path(model_path)
+            .build()
+            .unwrap();

crates/transcribe-whisper-local/src/service/streaming.rs (2)

129-129: Confirm ownership semantics of Whisper

Passing Whisper by value into the handler implies it’s moved/consumed downstream. If Whisper is heavy and not trivially movable, consider Arc if you plan to fan out or reuse across tasks; otherwise, this is fine for per-connection ownership.

Would you like me to refactor this to Arc to future-proof for multi-stream use?

---

134-137: Clamp looks good; minor readability nit

The min(max()) pattern is correct. Consider using clamp if the type supports it for readability.

For example: params.redemption_time_ms.unwrap_or(500).clamp(300, 1200)

plugins/local-stt/src/lib.rs (1)

137-138: Remove debug print statements from test

The test contains debug print statements that should be removed or replaced with proper assertions.

-        let model = app.get_current_model();
-        println!("model: {:#?}", model);
+        let model = app.get_current_model();
+        assert!(model.is_ok(), "Failed to get current model");

plugins/local-stt/src/ext.rs (1)

113-138: Inefficient loop for single iteration

The is_model_downloaded method for Whisper models uses a for loop with a single iteration, which is unnecessarily complex.

 SupportedSttModel::Whisper(model) => {
     let model_path = self.models_dir().join(model.file_name());

-    for (path, expected) in [(model_path, model.model_size_bytes())] {
-        if !path.exists() {
-            return Ok(false);
-        }
-
-        let actual = hypr_file::file_size(path)?;
-        if actual != expected {
-            return Ok(false);
-        }
+    if !model_path.exists() {
+        return Ok(false);
     }

-    Ok(true)
+    let actual = hypr_file::file_size(model_path)?;
+    Ok(actual == model.model_size_bytes())
 }

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

crates/am/src/client.rs (1)

86-95: Preserve server error context when JSON parsing fails

Falling back to UnexpectedResponse on non-JSON error bodies loses useful diagnostics. Prefer capturing HTTP status and raw body when the error isn’t JSON.

Apply this diff to improve error propagation:

-    async fn handle_error_response(&self, response: Response) -> Error {
-        if let Ok(error_response) = response.json::<ErrorResponse>().await {
-            Error::ServerError {
-                status: error_response.status,
-                message: error_response.message,
-            }
-        } else {
-            Error::UnexpectedResponse
-        }
-    }
+    async fn handle_error_response(&self, response: Response) -> Error {
+        let status = response.status().as_u16();
+        let body = response.text().await.unwrap_or_default();
+        if let Ok(error_response) = serde_json::from_str::<ErrorResponse>(&body) {
+            Error::ServerError {
+                status: error_response.status,
+                message: error_response.message,
+            }
+        } else {
+            Error::ServerError {
+                status,
+                message: body,
+            }
+        }
+    }

Note: This uses serde_json::from_str; ensure serde_json is available in the crate.

plugins/local-stt/src/server/external.rs (1)

16-26: Previous issue resolved: redundant init with hardcoded API key removed

The earlier hardcoded "ax_123" init call is no longer present. Thanks for addressing this.

plugins/windows/src/ext.rs (1)

313-325: Add a brief “why” comment if keeping the workaround.

If the two-step resize is retained due to a known platform quirk, add a minimal comment explaining the rationale (why it’s needed and on which platform). Helps future maintainers avoid reverting it accidentally.

plugins/local-stt/src/server/external.rs (4)

2-7: Good encapsulation with api_key and client; consider restricting base_url visibility

Storing the hypr_am::Client and api_key on the handle is clean. If external mutation isn’t required, consider making base_url non-pub to keep invariants tighter.

---

16-26: Make init model-flexible instead of hardcoding WhisperSmallEn

Allow callers to pass the desired AmModel to support the unified selection flow and future models without touching this layer.

Proposed change:

-    pub async fn init(&self) -> Result<(), crate::Error> {
-        self.client
-            .init(
-                hypr_am::InitRequest::new(self.api_key.clone())
-                    .with_model(hypr_am::AmModel::WhisperSmallEn),
-            )
-            .await?;
-        Ok(())
-    }
+    pub async fn init(&self, model: hypr_am::AmModel) -> Result<(), crate::Error> {
+        self.client
+            .init(
+                hypr_am::InitRequest::new(self.api_key.clone())
+                    .with_model(model),
+            )
+            .await?;
+        Ok(())
+    }

---

28-31: Return the ServerStatus instead of discarding it

Avoid a roundtrip that drops the value; return the status for callers to act on.

Apply:

-    pub async fn status(&self) -> Result<(), crate::Error> {
-        self.client.status().await?;
-        Ok(())
-    }
+    pub async fn status(&self) -> Result<hypr_am::ServerStatus, crate::Error> {
+        let status = self.client.status().await?;
+        Ok(status)
+    }

---

38-39: Avoid binding an unused receiver from spawn()

Use the underscore pattern to avoid creating an unused variable.

-    let (_rx, child) = cmd.args(["--port", &port.to_string()]).spawn()?;
+    let (_, child) = cmd.args(["--port", &port.to_string()]).spawn()?;

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

crates/file/src/lib.rs (1)

192-193: Avoid std::sync::Mutex in async hot path; use AtomicU64 for progress tracking

Locking a std::sync::Mutex inside async tasks can cause unnecessary contention. An AtomicU64 is simpler, faster, and avoids potential poisoning issues.

@@
-    let downloaded = Arc::new(Mutex::new(existing_size));
+    let downloaded = Arc::new(std::sync::atomic::AtomicU64::new(existing_size));
@@
-                let mut downloaded_guard = downloaded_clone.lock().unwrap();
-                *downloaded_guard += chunk.len() as u64;
-                let current_downloaded = *downloaded_guard;
-                drop(downloaded_guard);
-
-                progress_callback_clone(DownloadProgress::Progress(current_downloaded, total_size));
+                let current_downloaded =
+                    downloaded_clone.fetch_add(chunk.len() as u64, std::sync::atomic::Ordering::Relaxed)
+                    + (chunk.len() as u64);
+                progress_callback_clone(DownloadProgress::Progress(
+                    current_downloaded,
+                    total_size,
+                ));

If you prefer explicit imports instead of fully qualified paths, add:

@@
-use {
+use {
@@
-        sync::{Arc, Mutex, OnceLock},
+        sync::{Arc, Mutex, OnceLock, atomic::{AtomicU64, Ordering}},

…and then use AtomicU64/Ordering directly.

Also applies to: 227-233, 20-21

apps/desktop/src/components/settings/components/ai/stt-view-local.tsx (1)

264-270: Add error handling for model switching operations

The onClick handler performs multiple async operations without error handling. If any operation fails, the UI state might become inconsistent.

-      onClick={() => {
+      onClick={async () => {
         if (model.downloaded) {
-          setSelectedSTTModel(model.key as WhisperModel);
-          localSttCommands.setCurrentModel(model.key as WhisperModel);
-          localSttCommands.stopServer(null);
-          localSttCommands.startServer(null);
+          try {
+            await localSttCommands.stopServer(null);
+            await localSttCommands.setCurrentModel(model.key as WhisperModel);
+            setSelectedSTTModel(model.key as WhisperModel);
+            await localSttCommands.startServer(null);
+          } catch (error) {
+            console.error("Failed to switch model:", error);
+            // Consider showing a toast or error message to the user
+          }
         }
       }}

plugins/local-stt/src/server/external.rs (1)

39-39: Hardcoded port 6942 risks collisions

plugins/local-stt/src/ext.rs (1)

302-315: Extract duplicated progress callback logic

plugins/local-stt/js/bindings.gen.ts (1)

60-60: Remove duplicate TAURI_CHANNEL type definition

plugins/local-stt/permissions/schemas/schema.json (1)

502-506: Avoid drift between default list here and default.toml (add an explicit sync note).

The enumerated list is easy to get out of sync with plugins/local-stt/permissions/default.toml. Add a short note to prompt maintainers to keep them aligned.

Apply this diff to append a sync note to both description and markdownDescription:

-          "description": "Default permissions for the plugin\n#### This default permission set includes:\n\n- `allow-models-dir`\n- `allow-is-model-downloaded`\n- `allow-is-model-downloading`\n- `allow-download-model`\n- `allow-start-server`\n- `allow-stop-server`\n- `allow-get-servers`\n- `allow-get-current-model`\n- `allow-set-current-model`\n- `allow-list-supported-models`",
+          "description": "Default permissions for the plugin\n#### This default permission set includes:\n\n- `allow-models-dir`\n- `allow-is-model-downloaded`\n- `allow-is-model-downloading`\n- `allow-download-model`\n- `allow-start-server`\n- `allow-stop-server`\n- `allow-get-servers`\n- `allow-get-current-model`\n- `allow-set-current-model`\n- `allow-list-supported-models`\n\nNote: Keep this list in sync with plugins/local-stt/permissions/default.toml.",
@@
-          "markdownDescription": "Default permissions for the plugin\n#### This default permission set includes:\n\n- `allow-models-dir`\n- `allow-is-model-downloaded`\n- `allow-is-model-downloading`\n- `allow-download-model`\n- `allow-start-server`\n- `allow-stop-server`\n- `allow-get-servers`\n- `allow-get-current-model`\n- `allow-set-current-model`\n- `allow-list-supported-models`"
+          "markdownDescription": "Default permissions for the plugin\n#### This default permission set includes:\n\n- `allow-models-dir`\n- `allow-is-model-downloaded`\n- `allow-is-model-downloading`\n- `allow-download-model`\n- `allow-start-server`\n- `allow-stop-server`\n- `allow-get-servers`\n- `allow-get-current-model`\n- `allow-set-current-model`\n- `allow-list-supported-models`\n\nNote: Keep this list in sync with plugins/local-stt/permissions/default.toml."

crates/file/src/lib.rs (4)

217-219: Make the non-206 range response error actionable (include range and expectation)

The new message is too generic and removes vital context for debugging. Surface the requested range and expected status.

-                return Err(OtherError(format!(
-                    "Something went wrong. Please try again. (status: {})",
-                    response.status()
-                )));
+                return Err(OtherError(format!(
+                    "Unexpected status for range GET {}-{} (expected 206): {}",
+                    start, end, response.status()
+                )));

Also applies to: 215-216

---

41-42: Use typed header constants for correctness and clarity

Typed constants avoid case/typo mistakes and make intent explicit.

@@
-        request = request.header("Range", format!("bytes={}-", start));
+        request = request.header(reqwest::header::RANGE, format!("bytes={}-", start));
@@
-    let supports_ranges = head_response
-        .headers()
-        .get("accept-ranges")
+    let supports_ranges = head_response
+        .headers()
+        .get(reqwest::header::ACCEPT_RANGES)
         .map(|v| v.to_str().unwrap_or(""))
         .unwrap_or("")
         == "bytes";
@@
-                .header("Range", range_header)
+                .header(reqwest::header::RANGE, range_header)

Also applies to: 151-156, 209-213

---

221-223: Pre-allocate per-chunk buffer capacity to reduce reallocations

A minor perf win: we know the chunk size, so we can reserve capacity.

-            let mut bytes = Vec::new();
+            let mut bytes = Vec::with_capacity((end - start + 1) as usize);

---

99-116: Optional: flush file before signaling Finished for consistency

Parallel path flushes file; serial path doesn’t. Flushing before Finished keeps semantics consistent.

@@
     }
 
-    progress_callback(DownloadProgress::Finished);
+    file.flush()?;
+    progress_callback(DownloadProgress::Finished);
 
     Ok(())

plugins/local-stt/src/server/external.rs (3)

2-2: base_url field should be private for proper encapsulation

The base_url field is public but is only used internally within the module. Making it private improves encapsulation.

-    pub base_url: String,
+    base_url: String,

---

29-32: Consider simplifying the status method

The status result is wrapped in Ok() unnecessarily since it already returns a Result.

-    pub async fn status(&self) -> Result<hypr_am::ServerStatus, crate::Error> {
-        let status = self.client.status().await?;
-        Ok(status)
-    }
+    pub async fn status(&self) -> Result<hypr_am::ServerStatus, crate::Error> {
+        self.client.status().await.map_err(Into::into)
+    }

---

46-77: Consider extracting log processing logic

The nested loop handling stdout/stderr could be extracted into a helper function for better readability.

+fn process_command_event(event: tauri_plugin_shell::process::CommandEvent) {
+    match event {
+        tauri_plugin_shell::process::CommandEvent::Stdout(bytes) 
+        | tauri_plugin_shell::process::CommandEvent::Stderr(bytes) => {
+            if let Ok(text) = String::from_utf8(bytes) {
+                let text = text.trim();
+                if !text.is_empty() {
+                    tracing::info!("{}", text);
+                }
+            }
+        }
+        _ => {}
+    }
+}

 tokio::spawn(async move {
     loop {
         tokio::select! {
             _ = shutdown_rx.changed() => {
                 tracing::info!("external_server_shutdown");
                 break;
             }
             event = rx.recv() => {
                 if event.is_none() {
                     break;
                 }
-
-                match event.unwrap() {
-                    tauri_plugin_shell::process::CommandEvent::Stdout(bytes) => {
-                        if let Ok(text) = String::from_utf8(bytes) {
-                            let text = text.trim();
-                            tracing::info!("{}", text);
-                        }
-                    }
-                    tauri_plugin_shell::process::CommandEvent::Stderr(bytes) => {
-                        if let Ok(text) = String::from_utf8(bytes) {
-                            let text = text.trim();
-                            tracing::info!("{}", text);
-                        }
-                    }
-                    _ => {}
-
-                }
+                process_command_event(event.unwrap());
             }
         }
     }
 });

plugins/local-stt/src/ext.rs (1)

317-375: Add consistent error handling for both model types

The AM model download includes proper error handling with tar unpacking (lines 331-334), but consider adding a success log for consistency with error logging.

                     if let Err(e) = m.tar_verify_and_unpack(&tar_path, &final_path) {
                         tracing::error!("model_unpack_error: {}", e);
                         let _ = channel.send(-1);
+                    } else {
+                        tracing::info!("model_unpacked: {}", m.model_dir());
                     }

crates/am/Cargo.toml (1)

22-22: Consider pinning the tar dependency to a workspace version

The tar dependency is pinned to a specific version "0.4.44" while other dependencies use workspace versions. For consistency and easier dependency management across the workspace, consider moving this to the workspace dependencies.

crates/am/src/error.rs (1)

12-13: Consider using the dirs crate's error type instead of a custom variant

The NoHomeDir error could potentially be replaced by propagating the actual error from the dirs crate operations, providing more context about why the home directory couldn't be found.

crates/am/src/model.rs (2)

50-67: Directory existence check has a potential TOCTOU issue

The is_downloaded method checks directory existence and then reads it in separate operations. While not critical for this use case, consider combining operations to be more robust.

 pub fn is_downloaded(
     &self,
     base_dir: impl AsRef<std::path::Path>,
 ) -> Result<bool, crate::Error> {
     let model_path = base_dir.as_ref().join(self.model_dir());
-    if !model_path.exists() {
-        return Ok(false);
-    }
-
-    if !model_path.is_dir() {
-        return Ok(false);
-    }
-
-    let entries = std::fs::read_dir(&model_path)?;
-    let has_files = entries.count() > 0;
-
-    Ok(has_files)
+    match std::fs::read_dir(&model_path) {
+        Ok(entries) => Ok(entries.count() > 0),
+        Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(false),
+        Err(e) => Err(e.into()),
+    }
 }

---

78-83: Checksums should be documented or externalized

The hardcoded CRC32 checksums (1906983049, 1964673816) are critical for security but lack documentation about their source. Consider adding comments or moving them to a configuration file.

 pub fn tar_checksum(&self) -> u32 {
     match self {
-        AmModel::ParakeetV2 => 1906983049,
-        AmModel::WhisperLargeV3 => 1964673816,
+        // CRC32 checksum of nvidia_parakeet-v2_476MB.tar
+        AmModel::ParakeetV2 => 1906983049,
+        // CRC32 checksum of openai_whisper-large-v3-v20240930_626MB.tar  
+        AmModel::WhisperLargeV3 => 1964673816,
     }
 }

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro