-
Notifications
You must be signed in to change notification settings - Fork 415
Add custom stt endpoint support #1412
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughRenames get_current_model/set_current_model to get_local_model/set_local_model; adds Provider and SupportedSttModel::Custom; introduces get/set commands for custom base URL, API key, provider, and custom model; updates plugin ext, store, server types, JS bindings, permissions, desktop UI callsites, and related locales. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Desktop UI (JS)
participant Tauri as Tauri Commands
participant Ext as LocalSttPluginExt
participant Store as Plugin Store
participant Conn as STT Connection
participant Server as External STT Server
rect rgb(240,248,255)
note right of UI: Configure custom provider
UI->>Tauri: set_custom_base_url(baseUrl)
Tauri->>Ext: set_custom_base_url(baseUrl)
Ext->>Store: write CustomBaseUrl
Store-->>Ext: ok
UI->>Tauri: set_custom_api_key(apiKey)
Tauri->>Ext: set_custom_api_key(apiKey)
Ext->>Store: write CustomApiKey
Store-->>Ext: ok
end
rect rgb(245,255,245)
note right of UI: Use Custom model for recognition
UI->>Tauri: start/recognize (model = Custom("id"))
Tauri->>Ext: get_connection()
Ext->>Store: read Provider, LocalModel/CustomModel
Store-->>Ext: Provider=Custom, Custom("id")
Ext->>Store: read CustomBaseUrl & CustomApiKey
Store-->>Ext: baseUrl / apiKey
Ext->>Conn: create Connection(model, baseUrl, apiKey)
Conn->>Server: send audio
Server-->>Conn: transcribed text
Conn-->>Tauri: result
Tauri-->>UI: result
end
rect rgb(255,245,245)
note right of Ext: Guarded operations
UI->>Tauri: download_model(Custom("id"))
Tauri->>Ext: download_model(Custom("id"))
Ext-->>Tauri: Err(UnsupportedModelType)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (9)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/local-stt/src/ext.rs (1)
515-521: Setting Custom model erroneously fails by starting a server
set_current_modelunconditionally starts a server. ForCustom,start_serverreturnsUnsupportedModelType, causing this command to fail even though the model was stored.Apply:
async fn set_current_model(&self, model: SupportedSttModel) -> Result<(), crate::Error> { let store = self.local_stt_store(); store.set(crate::StoreKey::CurrentModel, model.clone())?; self.stop_server(None).await?; - self.start_server(Some(model)).await?; - Ok(()) + match &model { + SupportedSttModel::Custom(_) => Ok(()), // no server to start for custom endpoints + _ => { + self.start_server(Some(model)).await?; + Ok(()) + } + } }
🧹 Nitpick comments (8)
plugins/local-stt/src/model.rs (2)
28-29: Keep Custom(String) last in untagged enum and document why.Current comment says “must be the last item” (what). Add the why: in serde(untagged), a bare String fallback would preempt other string-deserializable variants if not last.
Apply:
- // must be the last item + // must be the last item: serde(untagged) tries variants in order; this String fallback + // would swallow Whisper/Am string forms if placed earlier.
200-204: info() for Custom returns size_bytes=0 — confirm UI semantics.Some UIs display “0 B”; ensure it’s treated as “N/A/remote” instead of confusing the user. Add a brief why-comment if needed.
plugins/local-stt/src/commands.rs (1)
114-144: API surface LGTM; guard secrets and consider clearer unset semantics.
- get_custom_api_key exposes sensitive data; rely on strict permissions and keep default as deny.
- set_custom_api_key takes String; there’s no explicit “unset.” Consider accepting Option or adding a clear_custom_api_key command so empty string isn’t ambiguous.
Would you like a follow-up patch to introduce clear_custom_api_key and wire TS bindings/permissions accordingly?
plugins/local-stt/js/bindings.gen.ts (1)
76-76: Type widened to string weakens safety
SupportedSttModel = WhisperModel | AmModel | stringallows arbitrary strings and bypasses type safety. Prefer a discriminated form (e.g.,{ Custom: string }) via specta tagging to keep compile-time guarantees.plugins/local-stt/src/ext.rs (3)
75-80: Empty-string default masks missing configReturning
""for unset base URL hides configuration errors. Prefer surfacingErr(...)or returningOption<String>and handling it explicitly.
377-380: Duplicate Custom guard in download_model
Customis rejected twice (pre-check and match arm). Remove one for clarity.Apply:
- if let SupportedSttModel::Custom(_) = model { - return Err(crate::Error::UnsupportedModelType); - } ... - SupportedSttModel::Custom(_) => { - return Err(crate::Error::UnsupportedModelType); - } + if let SupportedSttModel::Custom(_) = model { + return Err(crate::Error::UnsupportedModelType); + }(or delete the match arm instead)
Also applies to: 410-414
87-91: Validate base URL on writePrevent writing invalid values by parsing with
url::Urland requiring http/https.Example:
fn set_custom_base_url(&self, base_url: impl Into<String>) -> Result<(), crate::Error> { let store = self.local_stt_store(); - store.set(crate::StoreKey::CustomBaseUrl, base_url.into())?; + let s = base_url.into(); + let u = url::Url::parse(&s).map_err(|_| crate::Error::UnsupportedModelType)?; + if u.scheme() != "http" && u.scheme() != "https" { + return Err(crate::Error::UnsupportedModelType); + } + store.set(crate::StoreKey::CustomBaseUrl, s)?; Ok(()) }plugins/local-stt/permissions/autogenerated/reference.md (1)
87-134: Permission table: ensure deny tokens exist and are documented as recommended defaultsFor API-key commands, emphasize deny as recommended; ensure generators keep these tokens out of defaults.
Also applies to: 503-551
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
plugins/local-stt/build.rs(1 hunks)plugins/local-stt/js/bindings.gen.ts(2 hunks)plugins/local-stt/permissions/autogenerated/commands/get_custom_api_key.toml(1 hunks)plugins/local-stt/permissions/autogenerated/commands/get_custom_base_url.toml(1 hunks)plugins/local-stt/permissions/autogenerated/commands/set_custom_api_key.toml(1 hunks)plugins/local-stt/permissions/autogenerated/commands/set_custom_base_url.toml(1 hunks)plugins/local-stt/permissions/autogenerated/reference.md(3 hunks)plugins/local-stt/permissions/default.toml(1 hunks)plugins/local-stt/permissions/schemas/schema.json(3 hunks)plugins/local-stt/src/commands.rs(1 hunks)plugins/local-stt/src/ext.rs(9 hunks)plugins/local-stt/src/lib.rs(1 hunks)plugins/local-stt/src/model.rs(3 hunks)plugins/local-stt/src/store.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. No error handling.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
plugins/local-stt/src/lib.rsplugins/local-stt/build.rsplugins/local-stt/js/bindings.gen.tsplugins/local-stt/src/commands.rsplugins/local-stt/src/model.rsplugins/local-stt/src/store.rsplugins/local-stt/src/ext.rs
🧬 Code graph analysis (4)
plugins/local-stt/src/lib.rs (3)
plugins/local-stt/js/bindings.gen.ts (1)
commands(9-58)plugins/local-stt/src/commands.rs (4)
get_custom_base_url(116-118)get_custom_api_key(122-126)set_custom_base_url(130-135)set_custom_api_key(139-144)plugins/local-stt/src/ext.rs (8)
get_custom_base_url(24-24)get_custom_base_url(75-79)get_custom_api_key(25-25)get_custom_api_key(81-85)set_custom_base_url(26-26)set_custom_base_url(87-91)set_custom_api_key(27-27)set_custom_api_key(93-97)
plugins/local-stt/src/commands.rs (1)
plugins/local-stt/src/ext.rs (8)
get_custom_base_url(24-24)get_custom_base_url(75-79)get_custom_api_key(25-25)get_custom_api_key(81-85)set_custom_base_url(26-26)set_custom_base_url(87-91)set_custom_api_key(27-27)set_custom_api_key(93-97)
plugins/local-stt/src/model.rs (1)
plugins/local-stt/js/bindings.gen.ts (2)
SupportedSttModel(76-76)SttModelInfo(75-75)
plugins/local-stt/src/ext.rs (2)
plugins/local-stt/src/commands.rs (7)
models_dir(11-13)list_ggml_backends(17-21)get_custom_base_url(116-118)get_custom_api_key(122-126)set_custom_base_url(130-135)set_custom_api_key(139-144)set_current_model(71-78)plugins/local-stt/js/bindings.gen.ts (3)
GgmlBackend(71-71)SupportedSttModel(76-76)WhisperModel(78-78)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: ci (macos, macos-latest)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (8)
plugins/local-stt/permissions/schemas/schema.json (2)
321-344: New permission tokens for get_custom_{api_key,base_url} look consistent.Names match command ids and existing naming conventions.
513-536: Set permission tokens added correctly.The four allow/deny entries align with the new commands.
plugins/local-stt/build.rs (1)
14-17: Commands appended correctly; ensure end-to-end wiring.Confirm these four are implemented, registered in lib.rs/specta, have permissions and TS bindings generated.
Use the script in the schema.json comment to verify presence across src/, permissions/, and bindings.
plugins/local-stt/src/model.rs (1)
184-185: Clarify empty-list semantics for Custom STT models
No occurrences ofsupported_languages().is_empty()orsupportedLanguages().length === 0were found, so an emptyVecmay flow through to the UI unchecked. Please verify that any consumer (especially UI code) treats an empty list as “unknown/dynamic” rather than “unsupported,” and document this intent inline:
- File:
plugins/local-stt/src/model.rs, lines 184–185- SupportedSttModel::Custom(_) => vec![], + SupportedSttModel::Custom(_) => vec![], // empty indicates dynamic/unknown languagesplugins/local-stt/permissions/autogenerated/commands/set_custom_base_url.toml (1)
1-13: Autogenerated manifest looks correct.Allow/Deny blocks match command id and schema conventions.
plugins/local-stt/src/lib.rs (1)
51-54: Permission tokens are defined and default permissions are correct
- Confirmed each custom command (
get_custom_base_url,get_custom_api_key,set_custom_base_url,set_custom_api_key) has bothallow-*anddeny-*identifiers in its autogenerated permissions file underplugins/local-stt/permissions/autogenerated/commands/.- Verified
plugins/local-stt/permissions/default.tomlincludes the following default allows:
"allow-get-custom-base-url""allow-get-custom-api-key""allow-set-custom-base-url""allow-set-custom-api-key"All permission tokens exist and the default configuration matches expectations. No further changes needed.
plugins/local-stt/permissions/autogenerated/commands/get_custom_base_url.toml (1)
1-13: Autogenerated manifest for base URL is fine.Schema hook and allow/deny entries are correct.
plugins/local-stt/permissions/autogenerated/commands/set_custom_api_key.toml (1)
1-13: Autogenerated manifest is correct; pair this with deny-by-default.Ensure default profile does not include the “allow-set-custom-api-key” token (see default.toml change).
| async getCustomBaseUrl() : Promise<string> { | ||
| return await TAURI_INVOKE("plugin:local-stt|get_custom_base_url"); | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Validate baseUrl and prevent empty writes
These commands accept any string; add backend validation (scheme http/https, non-empty) and surface an error if invalid. Prevents unusable “Custom” configs.
Also applies to: 52-56
plugins/local-stt/permissions/autogenerated/commands/get_custom_api_key.toml
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
8 issues found across 14 files
React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai to give feedback, ask questions, or re-run the review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/desktop/src/components/toast/model-download.tsx (1)
26-61: Guard all STT download checks and toasts when Provider=Custom.For Custom endpoints, calls like isModelDownloaded/isModelDownloading/getServers/download toasts must not run (they’ll return UnsupportedModelType). Add a provider query and gate these branches.
Add this outside the selected range:
// new const sttProvider = useQuery({ queryKey: ["stt-provider"], queryFn: () => localSttCommands.getProvider(), staleTime: Infinity, });Then gate:
- checkForModelDownload.enabled → sttProvider.data === "Local" && !!currentSttModel.data
- sttModelDownloading.enabled → sttProvider.data === "Local" && !checkForModelDownload.data?.sttModelDownloaded
- sttModelExists.enabled → sttProvider.data === "Local"
- In checkForModelDownload.queryFn, if sttProvider.data !== "Local", return { sttModelDownloaded: true, llmModelDownloaded: } to short-circuit.
- In the click handler, before showSttModelDownloadToast, bail when sttProvider.data !== "Local".
Minimal guard in the button handler:
- if (needsSttModel && !sttModelDownloading.data) { + if (sttProvider.data === "Local" && needsSttModel && !sttModelDownloading.data) { showSttModelDownloadToast(currentSttModel.data!, undefined, queryClient); }Also applies to: 62-78, 140-157
apps/desktop/src/components/editor-area/note-header/listen-button.tsx (1)
77-82: Short-circuit model/server checks for Custom provider.isModelDownloaded/getServers are not applicable for Custom and may error. Treat Custom as “ready” here (or validate base URL elsewhere).
Apply:
- const currentModel = await localSttCommands.getLocalModel(); - const isDownloaded = await localSttCommands.isModelDownloaded(currentModel); - const servers = await localSttCommands.getServers(); - const isServerAvailable = (servers.external === "ready") || (servers.internal === "ready"); - return isDownloaded && isServerAvailable; + const [provider, currentModel] = await Promise.all([ + localSttCommands.getProvider(), + localSttCommands.getLocalModel(), + ]); + if (provider === "Custom") { + // For custom endpoints we don't download or start local servers. + return true; + } + const isDownloaded = await localSttCommands.isModelDownloaded(currentModel); + const servers = await localSttCommands.getServers(); + const isServerAvailable = (servers.external === "ready") || (servers.internal === "ready"); + return isDownloaded && isServerAvailable;plugins/local-stt/permissions/autogenerated/reference.md (2)
61-82: Inconsistent/duplicated permission: “current-model” rows describe get_local_modelIdentifiers still use get-current-model while description references get_local_model, and duplicate with the dedicated get-local-model rows below. Either (a) remove these “current-model” rows or (b) keep them as deprecated and fix the description to “get_current_model”.
Minimal doc-only fix if keeping deprecated rows:
- Enables the get_local_model command without any pre-configured scope. + (Deprecated) Enables the get_current_model command without any pre-configured scope.And same for the deny row.
500-525: Inconsistent/duplicated permission: “set-current-model” rows describe set_local_modelSame issue as with get-current-model. Either delete these rows or mark deprecated and fix text to “set_current_model”. Also ensure the default set uses set-local-model instead.
Doc-only tweak if retaining deprecated rows:
- Enables the set_local_model command without any pre-configured scope. + (Deprecated) Enables the set_current_model command without any pre-configured scope.And same for the deny row.
♻️ Duplicate comments (9)
plugins/local-stt/src/store.rs (1)
8-10: Back-compat: serde rename doesn’t affect Display; keep Display string as “DefaultModel”Only serde rename is present; code that uses Display/to_string for keys will write “LocalModel”, breaking reads of existing “DefaultModel”. Add strum serialize to preserve the on-disk key.
#[serde(rename = "DefaultModel")] // for backward compatibility - LocalModel, + #[strum(serialize = "DefaultModel")] + LocalModel,plugins/local-stt/js/bindings.gen.ts (1)
45-69: Do not return raw secrets to the webview; expose presence onlyReplace getCustomApiKey with getCustomApiKeyPresent(): Promise and remove raw secret flow. Also validate baseUrl before storing/using to avoid unusable configs.
-async getCustomApiKey() : Promise<string | null> { - return await TAURI_INVOKE("plugin:local-stt|get_custom_api_key"); -}, +async getCustomApiKeyPresent() : Promise<boolean> { + return await TAURI_INVOKE("plugin:local-stt|get_custom_api_key_present"); +},Note: update Rust commands first; this file is generated.
plugins/local-stt/src/ext.rs (4)
86-90: Avoid returning raw API keys; store in secure keystoreDo not persist or return secrets via tauri_plugin_store2 (plaintext). Use platform keychain (e.g., keyring) and expose only presence to the webview.
-fn get_custom_api_key(&self) -> Result<Option<String>, crate::Error>; +// Prefer a presence check for webview; keep a private backend-only accessor for actual use +fn get_custom_api_key_present(&self) -> Result<bool, crate::Error>;And implement presence by checking the keystore. Keep the real accessor private to server code.
98-108: Persisting API key in plaintext storeset_custom_api_key writes to store; move to OS keychain.
- let store = self.local_stt_store(); - store.set(StoreKey::CustomApiKey, api_key.into())?; + // e.g., use keyring with an app-specific service name + let key: String = api_key.into(); + crate::secrets::set("local-stt.custom_api_key", &key)?; // wrap keyring errors into crate::Error Ok(())If keyring already exists in the repo (auth plugin), reuse that helper.
116-124: Validate base URL before building Connection and avoid exposing keyFor Custom, ensure base_url is non-empty/valid; use presence flag and fetch secret internally only when making requests.
- SupportedSttModel::Custom(_) => { - let base_url = self.get_custom_base_url()?; - let api_key = self.get_custom_api_key()?; - Ok(Connection { base_url, api_key }) - } + SupportedSttModel::Custom(_) => { + let base_url = self.get_custom_base_url()?; // now validated + // retrieve secret from keychain internally (do not expose to webview) + let api_key = crate::secrets::get("local-stt.custom_api_key")?; + Ok(Connection { base_url, api_key }) + }Optionally parse base_url as a URL and map parse errors to crate::Error.
80-90: Empty base URL allowed — fail fastget_custom_base_url returns "" by default. Reject empty/whitespace to avoid constructing invalid Connections.
- let v = store.get(StoreKey::CustomBaseUrl)?; - Ok(v.unwrap_or_default()) + let v: Option<String> = store.get(StoreKey::CustomBaseUrl)?; + let base = v.unwrap_or_default(); + if base.trim().is_empty() { + return Err(crate::Error::UnsupportedModelType); + } + Ok(base)plugins/local-stt/permissions/autogenerated/reference.md (3)
18-21: Deny-by-default for secrets and mutations in the default setRemove default allowances for reading the API key and mutating endpoint/secret. Ship deny-by-default; opt-in per app.
Apply after updating default.toml and regenerating:
- - `allow-get-custom-base-url` - - `allow-get-custom-api-key` - - `allow-set-custom-base-url` - - `allow-set-custom-api-key` + - `allow-get-custom-base-url`
526-550: Default ability to set API key should be deny-by-default and scopedSetting credentials should require explicit, restricted scope; do not include in default set. Consider adding a “no-log” note for this command.
552-576: Default ability to set base URL should be deny-by-defaultMutating the endpoint can redirect traffic; require explicit opt-in (not part of defaults).
🧹 Nitpick comments (11)
apps/desktop/src/components/toast/model-select.tsx (1)
9-13: OK on API rename; consider also skipping for Provider≠Local.Today this toast won’t show for Custom models due to includes(), but being explicit avoids edge cases.
Outside the selected range:
const [provider, currentModel] = await Promise.all([ localSttCommands.getProvider(), localSttCommands.getLocalModel(), ]); if (provider !== "Local" || language === "en" || !englishModels.includes(currentModel)) return;apps/desktop/src/components/toast/shared.tsx (1)
83-85: Switched to setLocalModel on completion — add cache invalidation for current modelTo keep UI instantly in sync, also invalidate the “current-stt-model” query after setting the model.
sonnerToast.dismiss(id); localSttCommands.setLocalModel(model); + queryClient?.invalidateQueries({ queryKey: ["current-stt-model"] }); localSttCommands.startServer(null);apps/desktop/src/components/settings/components/ai/stt-view-local.tsx (1)
347-351: Await stop→start to avoid race conditions when switching modelsWithout awaiting, stop/start can overlap and cause flaky restarts. Make the handler async and await each call.
- const handleClick = () => { + const handleClick = async () => { if (isSelectable) { setSelectedSTTModel(model.key as SupportedSttModel); - localSttCommands.setLocalModel(model.key as SupportedSttModel); - localSttCommands.stopServer(null); - localSttCommands.startServer(null); + await localSttCommands.setLocalModel(model.key as SupportedSttModel); + await localSttCommands.stopServer(null); + await localSttCommands.startServer(null); } };plugins/local-stt/permissions/autogenerated/commands/get_current_model.toml (1)
7-13: Deprecate back-compat alias “get-current-model” in generated STT permissionsWe’ve confirmed that both
allow-get-current-model/deny-get-current-modelandallow-get-local-model/deny-get-local-modelexist in the autogenerated manifests, each mapping to the sameget_local_modelcommand (see get_current_model.toml and get_local_model.toml). Ifget-current-modelis only a legacy alias, we should explicitly mark it as deprecated in the TOML generator to prevent policy drift; otherwise, remove the alias entirely.Locations to update:
plugins/local-stt/permissions/autogenerated/commands/get_current_model.toml(lines 6–13)- (Optionally) confirm generator output for
get_local_model.tomlremains unchangedSuggested diff in
get_current_model.toml:[[permission]] identifier = "allow-get-current-model" +deprecated = true description = "Enables the get_local_model command without any pre-configured scope." commands.allow = ["get_local_model"] [[permission]] identifier = "deny-get-current-model" +deprecated = true description = "Denies the get_local_model command without any pre-configured scope." commands.deny = ["get_local_model"]This adds deprecation metadata for the legacy alias while keeping both tokens in lockstep.
plugins/local-stt/permissions/schemas/schema.json (3)
310-320: Back-compat alias should be labeled deprecatedConst is allow-get-current-model but description references get_local_model. Add “Deprecated alias for allow-get-local-model” in both description and markdownDescription to reduce confusion.
- "description": "Enables the get_local_model command without any pre-configured scope.", + "description": "[DEPRECATED] Alias for `allow-get-local-model`. Enables the get_local_model command without any pre-configured scope.", - "markdownDescription": "Enables the get_local_model command without any pre-configured scope." + "markdownDescription": "[DEPRECATED] Alias for `allow-get-local-model`. Enables the get_local_model command without any pre-configured scope."
358-368: Duplicate tokens (allow/deny-get-local-model) vs “current” aliasBoth families exist. Keep both only if strictly required; otherwise, remove one or mark the “current” pair deprecated (see above).
514-524: Same deprecation note for set_current_model aliasMirror the deprecation note for set_* tokens to avoid mixed naming.
- "description": "Enables the set_local_model command without any pre-configured scope.", + "description": "[DEPRECATED] Alias for `allow-set-local-model`. Enables the set_local_model command.", - "markdownDescription": "Enables the set_local_model command without any pre-configured scope." + "markdownDescription": "[DEPRECATED] Alias for `allow-set-local-model`. Enables the set_local_model command."plugins/local-stt/src/ext.rs (2)
394-396: Redundant Custom checkCustom is already rejected above; this second guard is harmless but redundant.
- if let SupportedSttModel::Custom(_) = model { - return Err(crate::Error::UnsupportedModelType); - } + // Custom rejected in match below
15-16: Nit: ensure Connection/api_key semantics documented (“Why”)Minimal comment at type/constructor level explaining why api_key may be None (Whisper) vs Some (AM/Custom).
plugins/local-stt/src/commands.rs (2)
126-133: Validate base_url on set (scheme + host)Consider rejecting non-http(s), empty, or file: URLs to prevent misconfiguration.
Validation likely belongs in Ext/store; if already present, ignore.
163-167: LGTM: custom model getter aligns with storeOptional: document that absence returns None.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (21)
apps/desktop/src/components/editor-area/note-header/listen-button.tsx(1 hunks)apps/desktop/src/components/settings/components/ai/stt-view-local.tsx(2 hunks)apps/desktop/src/components/settings/views/ai-stt.tsx(1 hunks)apps/desktop/src/components/toast/model-download.tsx(1 hunks)apps/desktop/src/components/toast/model-select.tsx(1 hunks)apps/desktop/src/components/toast/shared.tsx(1 hunks)apps/desktop/src/components/welcome-modal/download-progress-view.tsx(1 hunks)apps/desktop/src/components/welcome-modal/index.tsx(1 hunks)apps/desktop/src/routes/app.tsx(1 hunks)plugins/local-stt/build.rs(1 hunks)plugins/local-stt/js/bindings.gen.ts(3 hunks)plugins/local-stt/permissions/autogenerated/commands/get_current_model.toml(1 hunks)plugins/local-stt/permissions/autogenerated/commands/get_local_model.toml(1 hunks)plugins/local-stt/permissions/autogenerated/commands/set_current_model.toml(1 hunks)plugins/local-stt/permissions/autogenerated/commands/set_local_model.toml(1 hunks)plugins/local-stt/permissions/autogenerated/reference.md(6 hunks)plugins/local-stt/permissions/schemas/schema.json(4 hunks)plugins/local-stt/src/commands.rs(2 hunks)plugins/local-stt/src/ext.rs(11 hunks)plugins/local-stt/src/lib.rs(2 hunks)plugins/local-stt/src/store.rs(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- plugins/local-stt/permissions/autogenerated/commands/set_local_model.toml
- apps/desktop/src/components/welcome-modal/index.tsx
- plugins/local-stt/permissions/autogenerated/commands/set_current_model.toml
- plugins/local-stt/permissions/autogenerated/commands/get_local_model.toml
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. No error handling.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop/src/components/toast/model-download.tsxapps/desktop/src/components/toast/model-select.tsxapps/desktop/src/routes/app.tsxapps/desktop/src/components/settings/components/ai/stt-view-local.tsxapps/desktop/src/components/settings/views/ai-stt.tsxapps/desktop/src/components/editor-area/note-header/listen-button.tsxapps/desktop/src/components/welcome-modal/download-progress-view.tsxapps/desktop/src/components/toast/shared.tsxplugins/local-stt/js/bindings.gen.tsplugins/local-stt/build.rsplugins/local-stt/src/commands.rsplugins/local-stt/src/store.rsplugins/local-stt/src/ext.rsplugins/local-stt/src/lib.rs
🧬 Code graph analysis (5)
apps/desktop/src/components/settings/components/ai/stt-view-local.tsx (1)
plugins/local-stt/js/bindings.gen.ts (1)
SupportedSttModel(89-89)
plugins/local-stt/src/commands.rs (2)
plugins/local-stt/src/ext.rs (20)
get_local_model(45-45)get_local_model(525-529)set_local_model(46-49)set_local_model(532-538)get_custom_base_url(24-24)get_custom_base_url(80-84)get_custom_api_key(26-26)get_custom_api_key(86-90)set_custom_base_url(25-25)set_custom_base_url(98-102)set_custom_api_key(27-27)set_custom_api_key(104-108)get_provider(28-28)get_provider(92-96)set_provider(29-29)set_provider(110-114)get_custom_model(51-51)get_custom_model(541-545)set_custom_model(52-52)set_custom_model(548-552)plugins/local-stt/js/bindings.gen.ts (2)
SupportedSttModel(89-89)Provider(85-85)
plugins/local-stt/src/store.rs (1)
plugins/local-stt/js/bindings.gen.ts (1)
Provider(85-85)
plugins/local-stt/src/ext.rs (3)
plugins/connector/js/bindings.gen.ts (1)
Connection(115-115)plugins/local-stt/js/bindings.gen.ts (4)
Provider(85-85)GgmlBackend(83-83)SupportedSttModel(89-89)WhisperModel(91-91)plugins/local-stt/src/commands.rs (12)
models_dir(11-13)list_ggml_backends(17-21)get_custom_base_url(114-116)set_custom_base_url(128-133)get_custom_api_key(120-124)set_custom_api_key(137-142)get_provider(146-150)set_provider(154-159)get_local_model(63-67)set_local_model(71-76)get_custom_model(163-167)set_custom_model(171-176)
plugins/local-stt/src/lib.rs (3)
plugins/local-stt/js/bindings.gen.ts (1)
commands(9-70)plugins/local-stt/src/commands.rs (15)
get_local_model(63-67)set_local_model(71-76)get_servers(100-104)start_server(80-85)stop_server(89-96)list_supported_models(25-27)list_supported_languages(108-110)get_custom_base_url(114-116)get_custom_api_key(120-124)set_custom_base_url(128-133)set_custom_api_key(137-142)get_provider(146-150)set_provider(154-159)get_custom_model(163-167)set_custom_model(171-176)plugins/local-stt/src/ext.rs (26)
get_local_model(45-45)get_local_model(525-529)set_local_model(46-49)set_local_model(532-538)get_servers(41-43)get_servers(363-386)start_server(33-36)start_server(202-328)stop_server(37-40)stop_server(331-360)get_custom_base_url(24-24)get_custom_base_url(80-84)get_custom_api_key(26-26)get_custom_api_key(86-90)set_custom_base_url(25-25)set_custom_base_url(98-102)set_custom_api_key(27-27)set_custom_api_key(104-108)get_provider(28-28)get_provider(92-96)set_provider(29-29)set_provider(110-114)get_custom_model(51-51)get_custom_model(541-545)set_custom_model(52-52)set_custom_model(548-552)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-latest)
🔇 Additional comments (15)
apps/desktop/src/components/toast/model-download.tsx (1)
18-19: Rename is correct (getLocalModel).Matches the new API surface.
apps/desktop/src/components/settings/views/ai-stt.tsx (1)
21-41: No leftover provider/model API usages in STT code — migrations complete.I searched for any straggling calls to the old
getCurrentModel/setCurrentModelAPIs and found they only occur in LLM-related modules. The new STT feature exclusively useslocalSttCommands.setLocalModel, and there are no remaining references to the old API inapps/desktop/src/components/settings/views/ai-stt.tsxor elsewhere.apps/desktop/src/components/welcome-modal/download-progress-view.tsx (1)
175-177: Migration to setLocalModel is correctThe completion sequence matches the new API and already has try/catch around it. LGTM.
apps/desktop/src/components/settings/components/ai/stt-view-local.tsx (1)
63-65: getLocalModel usage aligned with new APIQuery now calls getLocalModel; matches bindings and PR intent. LGTM.
plugins/local-stt/src/lib.rs (1)
152-154: LGTM: test updated to get_local_modelRename aligns with the new API.
plugins/local-stt/permissions/schemas/schema.json (1)
322-332: Default should not advertise “allow-get-custom-api-key”Documenting/including an allow token for retrieving secrets is unsafe. Remove from defaults and prefer a deny token plus a boolean “present” command.
- "const": "allow-get-custom-api-key", + "const": "deny-get-custom-api-key", - "markdownDescription": "Enables the get_custom_api_key command without any pre-configured scope." + "markdownDescription": "Denies the get_custom_api_key command (secret exposure). Prefer a boolean presence command."Likely an incorrect or invalid review comment.
plugins/local-stt/js/bindings.gen.ts (1)
25-30: LGTM: renamed get/setLocalModel bindingsBindings match the Rust rename.
plugins/local-stt/src/ext.rs (2)
203-211: Good: start_server forbids CustomMatches PR objective: external custom endpoints shouldn’t start local servers.
540-552: LGTM: separate get/set_custom_model keysKeeps custom model independent from local model selection.
plugins/local-stt/permissions/autogenerated/reference.md (3)
110-134: LGTM: base URL read is non-sensitiveDocumenting allow/deny for get_custom_base_url is fine; ensure only “get” (not “set”) is in the default set.
165-186: Prefer the new get-local-model identifiers; align the default listThe table correctly adds get-local-model/deny-get-local-model. Update the default set at the top to use these and remove get-current-model entries to avoid confusion.
Action: update default.toml → regenerate this file.
581-600: Prefer set-local-model identifiers; align defaults and deprecationsGood to have set-local-model. Update defaults to this, and deprecate/remove the “set-current-model” rows to avoid duplicate knobs.
plugins/local-stt/src/commands.rs (3)
114-116: LGTM: simple passthrough for base URL readMatches Ext behavior; minimal and consistent.
146-150: LGTM: provider read passthroughConsistent with Ext; no further notes.
171-176: LGTM: custom model setter is minimalNo side effects here (no downloads/starts) match PR intent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src/locales/ko/messages.po (2)
401-406: Add shared translation: “API Base URL” (ko).msgid "API Base URL" -msgstr "" +msgstr "API 기본 URL"
407-419: Add shared translation: “API Key” (ko).msgid "API Key" -msgstr "" +msgstr "API 키"
♻️ Duplicate comments (6)
plugins/local-stt/permissions/default.toml (1)
15-22: Remove default permission to read/write the custom API key. Secrets must be deny-by-default.Reading or mutating credentials should never be enabled in the default profile.
Apply:
permissions = [ @@ - "allow-get-custom-base-url", - "allow-get-custom-api-key", - "allow-set-custom-base-url", - "allow-set-custom-api-key", + "allow-get-custom-base-url", + "allow-set-custom-base-url", "allow-get-provider", "allow-set-provider", "allow-get-custom-model", "allow-set-custom-model", ]plugins/local-stt/permissions/schemas/schema.json (2)
322-332: Do not expose an allow token for get_custom_api_key; make this deny-only or replace with presence check.Eliminate the “allow-get-custom-api-key” permission to prevent raw secret reads.
Apply:
- { - "description": "Enables the get_custom_api_key command without any pre-configured scope.", - "type": "string", - "const": "allow-get-custom-api-key", - "markdownDescription": "Enables the get_custom_api_key command without any pre-configured scope." - }, { "description": "Denies the get_custom_api_key command without any pre-configured scope.", "type": "string", "const": "deny-get-custom-api-key", "markdownDescription": "Denies the get_custom_api_key command without any pre-configured scope." },
634-638: Default set must not document API-key read/write as enabled.Remove the bullets for allow-get-custom-api-key and allow-set-custom-api-key from both description and markdownDescription.
Apply:
- "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- `allow-list-supported-languages`\n- `allow-get-custom-base-url`\n- `allow-get-custom-api-key`\n- `allow-set-custom-base-url`\n- `allow-set-custom-api-key`\n- `allow-get-provider`\n- `allow-set-provider`\n- `allow-get-custom-model`\n- `allow-set-custom-model`", + "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- `allow-list-supported-languages`\n- `allow-get-custom-base-url`\n- `allow-set-custom-base-url`\n- `allow-get-provider`\n- `allow-set-provider`\n- `allow-get-custom-model`\n- `allow-set-custom-model`", @@ - "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- `allow-list-supported-languages`\n- `allow-get-custom-base-url`\n- `allow-get-custom-api-key`\n- `allow-set-custom-base-url`\n- `allow-set-custom-api-key`\n- `allow-get-provider`\n- `allow-set-provider`\n- `allow-get-custom-model`\n- `allow-set-custom-model`" + "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- `allow-list-supported-languages`\n- `allow-get-custom-base-url`\n- `allow-set-custom-base-url`\n- `allow-get-provider`\n- `allow-set-provider`\n- `allow-get-custom-model`\n- `allow-set-custom-model`"plugins/local-stt/permissions/autogenerated/reference.md (2)
18-25: Default permissions must not include API-key read/write.Delete these bullets; regenerate after updating default.toml.
Apply:
- `allow-list-supported-languages` -- `allow-get-custom-base-url` -- `allow-get-custom-api-key` -- `allow-set-custom-base-url` -- `allow-set-custom-api-key` +- `allow-get-custom-base-url` +- `allow-set-custom-base-url` - `allow-get-provider` - `allow-set-provider` - `allow-get-custom-model` - `allow-set-custom-model`
88-113: Remove the “allow-get-custom-api-key” row from the table.Do not document or ship an allow path to read secrets.
Apply:
-<tr> -<td> - -`local-stt:allow-get-custom-api-key` - -</td> -<td> - -Enables the get_custom_api_key command without any pre-configured scope. - -</td> -</tr>apps/desktop/src/components/settings/components/ai/stt-view-local.tsx (1)
342-375: Model selection flow correctly flips provider and restarts server.Addresses prior review about forcing Local when choosing a local model.
🧹 Nitpick comments (12)
plugins/local-stt/permissions/default.toml (1)
15-22: Consider not default-allowing endpoint mutation.Setting base URL/model/provider changes network and execution paths; require explicit opt-in in trusted profiles.
Apply:
permissions = [ @@ - "allow-set-custom-base-url", - "allow-set-provider", - "allow-set-custom-model", + # opt-in in trusted profiles: + # "allow-set-custom-base-url", + # "allow-set-provider", + # "allow-set-custom-model", ]plugins/local-stt/permissions/schemas/schema.json (1)
310-320: Token/name drift: current-model vs local-model duplicates.Schema defines both legacy “get/set-current-model” tokens (with local-model descriptions) and new “get/set-local-model” tokens. This is confusing unless explicitly marked as deprecated/back-compat.
Proposed options:
- Keep legacy tokens but mark descriptions as “Deprecated — use allow/deny-*-local-model”.
- Or remove legacy entries and regenerate docs/CLI.
If keeping legacy, adjust descriptions to match the token names (current_model) and add a deprecation note.
Also applies to: 370-380, 538-548, 586-596
plugins/local-stt/permissions/autogenerated/reference.md (1)
559-579: Fix identifier/description mismatch for current/local model rows.Identifiers still say “current-model” but descriptions mention set_local_model; align or mark deprecated and point to set-local-model rows.
Apply (if deprecating):
-`local-stt:allow-set-current-model` +`local-stt:allow-set-current-model` (Deprecated — use `local-stt:allow-set-local-model`) @@ -Enables the set_local_model command without any pre-configured scope. +Deprecated. Use the set_local_model command permissions instead.Then regenerate the reference.
apps/desktop/src/locales/en/messages.po (1)
800-803: Base URL help text is accurate and scoped.Consider adding an example in parentheses later if users struggle (optional).
apps/desktop/src/components/settings/views/ai-stt.tsx (2)
21-34: Avoid in-component error handling per guidelines; make mutation sequential.Remove onError logging (guideline: “No error handling”). Also await stop before switching to Custom to avoid races.
- const setProviderMutation = useMutation({ - mutationFn: (provider: "Local" | "Custom") => { - if (provider === "Custom") { - localSttCommands.stopServer(null); - } - return localSttCommands.setProvider(provider); - }, - onSuccess: () => { - providerQuery.refetch(); - }, - onError: (error) => { - console.error("Failed to set provider:", error); - }, - }); + const setProviderMutation = useMutation({ + mutationFn: async (provider: "Local" | "Custom") => { + if (provider === "Custom") { + await localSttCommands.stopServer(null); + } + return localSttCommands.setProvider(provider); + }, + onSuccess: () => { + providerQuery.refetch(); + }, + });
57-74: Type safety: eliminate any-casts in model selection flow.Prefer threading SupportedSttModel through props to drop casts.
If you agree, update SharedSTTProps.handleModelDownload to accept SupportedSttModel and import the type here; then:
- setSelectedSTTModel(modelKey); - localSttCommands.setLocalModel(modelKey as any); + setSelectedSTTModel(modelKey); + localSttCommands.setLocalModel(modelKey as import("@hypr/plugin-local-stt").SupportedSttModel); setProviderToLocal();Run a quick repo-wide type check after changing SharedSTTProps.
apps/desktop/src/components/settings/components/ai/stt-view-remote.tsx (5)
26-39: Minimize API key exposure in cache; gate queries by provider.
- Set gcTime low and disable when provider ≠ "Custom" so secrets aren’t retained.
- const apiBaseQuery = useQuery({ + const apiBaseQuery = useQuery({ queryKey: ["custom-stt-base-url"], - queryFn: () => localSttCommands.getCustomBaseUrl(), + queryFn: () => localSttCommands.getCustomBaseUrl(), + enabled: provider === "Custom", + staleTime: 0, + gcTime: 0, }); - const apiKeyQuery = useQuery({ + const apiKeyQuery = useQuery({ queryKey: ["custom-stt-api-key"], - queryFn: () => localSttCommands.getCustomApiKey(), + queryFn: () => localSttCommands.getCustomApiKey(), + enabled: provider === "Custom", + staleTime: 0, + gcTime: 0, }); - const modelQuery = useQuery({ + const modelQuery = useQuery({ queryKey: ["custom-stt-model"], - queryFn: () => localSttCommands.getCustomModel(), + queryFn: () => localSttCommands.getCustomModel(), + enabled: provider === "Custom", });
92-100: Remove “what” style comment per guidelines.The JSX comment is redundant; keep comments minimal and about “why”.
- {/* Custom STT Endpoint Box */}
133-139: Persist base URL onBlur and improve semantic input type.Also prevents parent card click; already stopping propagation.
- <Input + <Input {...field} - placeholder="https://api.example.com/v1" + placeholder="https://api.example.com/v1" + type="url" onClick={(e) => e.stopPropagation()} onFocus={() => setProviderToCustom()} + onBlur={() => setApiBaseMutation.mutate(field.value || "")} />
155-160: Persist API key onBlur and disable browser autofill.Reduces accidental leakage and noisy writes.
- <Input + <Input {...field} type="password" placeholder="your-api-key" + autoComplete="off" onClick={(e) => e.stopPropagation()} onFocus={() => setProviderToCustom()} + onBlur={() => setApiKeyMutation.mutate(field.value || "")} />
179-184: Persist model onBlur.Consistent with other fields.
- <Input + <Input {...field} placeholder="whisper-1" onClick={(e) => e.stopPropagation()} onFocus={() => setProviderToCustom()} + onBlur={() => setModelMutation.mutate(field.value || "")} />apps/desktop/src/components/settings/components/ai/stt-view-local.tsx (1)
58-66: Remove debug log.Avoid console noise in production UIs.
- const servers = await localSttCommands.getServers(); - console.log(servers); - return servers; + return await localSttCommands.getServers();
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
apps/desktop/src/components/settings/components/ai/stt-view-local.tsx(12 hunks)apps/desktop/src/components/settings/components/ai/stt-view-remote.tsx(1 hunks)apps/desktop/src/components/settings/views/ai-stt.tsx(5 hunks)apps/desktop/src/locales/en/messages.po(7 hunks)apps/desktop/src/locales/ko/messages.po(7 hunks)plugins/local-stt/build.rs(1 hunks)plugins/local-stt/permissions/autogenerated/commands/get_custom_model.toml(1 hunks)plugins/local-stt/permissions/autogenerated/commands/get_provider.toml(1 hunks)plugins/local-stt/permissions/autogenerated/commands/set_custom_model.toml(1 hunks)plugins/local-stt/permissions/autogenerated/commands/set_provider.toml(1 hunks)plugins/local-stt/permissions/autogenerated/reference.md(6 hunks)plugins/local-stt/permissions/default.toml(1 hunks)plugins/local-stt/permissions/schemas/schema.json(4 hunks)
✅ Files skipped from review due to trivial changes (2)
- plugins/local-stt/permissions/autogenerated/commands/set_custom_model.toml
- plugins/local-stt/permissions/autogenerated/commands/get_custom_model.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/local-stt/build.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. No error handling.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop/src/components/settings/components/ai/stt-view-remote.tsxapps/desktop/src/components/settings/views/ai-stt.tsxapps/desktop/src/components/settings/components/ai/stt-view-local.tsx
🧬 Code graph analysis (3)
apps/desktop/src/components/settings/components/ai/stt-view-remote.tsx (3)
packages/ui/src/lib/utils.ts (1)
cn(4-6)packages/ui/src/components/ui/form.tsx (7)
Form(158-158)FormField(158-158)FormItem(158-158)FormLabel(158-158)FormDescription(158-158)FormControl(158-158)FormMessage(158-158)packages/ui/src/components/ui/input.tsx (1)
Input(22-22)
apps/desktop/src/components/settings/views/ai-stt.tsx (2)
apps/desktop/src/components/settings/components/ai/shared.tsx (1)
SharedSTTProps(91-98)apps/desktop/src/components/settings/components/ai/stt-view-remote.tsx (1)
STTViewRemote(19-198)
apps/desktop/src/components/settings/components/ai/stt-view-local.tsx (1)
plugins/local-stt/js/bindings.gen.ts (1)
SupportedSttModel(89-89)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci (macos, macos-latest)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (9)
plugins/local-stt/permissions/autogenerated/commands/get_provider.toml (1)
1-13: LGTM — permission manifest is consistent and scoped.Autogenerated header, schema pointer, identifiers, and allow/deny entries look correct.
plugins/local-stt/permissions/autogenerated/commands/set_provider.toml (1)
1-13: LGTM — matches get_provider structure.No issues; consistent identifiers and schema reference.
apps/desktop/src/locales/en/messages.po (3)
578-581: Good addition: new STT remote copy reads clearly.String is concise and consistent with LLM remote copy. No issues.
681-684: Introduce “Custom STT Endpoint” i18n entry — LGTM.Matches the new provider concept and UI selection card.
808-811: Model name helper text — LGTM.Aligns with input placeholder “whisper-1”.
apps/desktop/src/components/settings/views/ai-stt.tsx (2)
38-45: Provider-driven tab sync — LGTM.Keeps UI in sync with backend provider state.
114-115: Props wiring to STTViewRemote — LGTM.Matches the new provider contract.
apps/desktop/src/components/settings/components/ai/stt-view-local.tsx (2)
68-71: Switched to getLocalModel — LGTM.Aligns with new API and provider split.
234-254: Pro models query — LGTM.Filters AM variants and keeps download status fresh.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
♻️ Duplicate comments (7)
plugins/local-stt/src/ext.rs (3)
595-607: Avoid double-start and error for Custom model in Local providerIf
modelisCustom,start_server(Some(model))errors withUnsupportedModelTypeafter stopping servers.if matches!(provider, Provider::Local) { self.stop_server(None).await?; - self.start_server(Some(model)).await?; + if !matches!(model, SupportedSttModel::Custom(_)) { + self.start_server(Some(model)).await?; + } }This also prevents UI-visible flapping.
116-124: Guardget_connectionfor Custom with empty/invalid URLReturn an explicit error instead of constructing a broken
Connection.Provider::Custom => { let base_url = self.get_custom_base_url()?; - let api_key = self.get_custom_api_key()?; - Ok(Connection { base_url, api_key }) + let api_key = self.get_custom_api_key()?; + if base_url.trim().is_empty() { + return Err(crate::Error::InvalidCustomBaseUrl); + } + Ok(Connection { base_url, api_key }) }
86-90: Critical: Do not persist or expose raw API keys viaStore
tauri_plugin_store2writes plaintext JSON. Returning the key to the webview leaks secrets.
- Store the secret in the OS keychain (e.g.,
keyringcrate, already used elsewhere in the repo) and keep only a presence flag in the store.- Deprecate
get_custom_api_keyfor a non-secrethas_custom_api_key() -> Result<bool, Error>. Never return the raw key to the frontend.- Use the key only inside Rust when constructing requests.
Sketch:
-fn get_custom_api_key(&self) -> Result<Option<String>, crate::Error> { - let store = self.local_stt_store(); - let v = store.get(StoreKey::CustomApiKey)?; - Ok(v) -} +fn get_custom_api_key(&self) -> Result<Option<String>, crate::Error> { + // INTERNAL-ONLY: fetch from keychain; do NOT expose via commands + let service = "hyprnote.local-stt.custom-api-key"; + let entry = keyring::Entry::new(service, "default").map_err(crate::Error::from)?; + match entry.get_password() { + Ok(p) if !p.is_empty() => Ok(Some(p)), + _ => Ok(None), + } +}And for setter:
-fn set_custom_api_key(&self, api_key: impl Into<String>) -> Result<(), crate::Error> { - let store = self.local_stt_store(); - store.set(StoreKey::CustomApiKey, api_key.into())?; - Ok(()) -} +fn set_custom_api_key(&self, api_key: impl Into<String>) -> Result<(), crate::Error> { + let service = "hyprnote.local-stt.custom-api-key"; + let entry = keyring::Entry::new(service, "default").map_err(crate::Error::from)?; + entry.set_password(&api_key.into()).map_err(crate::Error::from)?; + // Optionally: store only a boolean presence in StoreKey::CustomApiKeyPresence + Ok(()) +}Follow-up: remove the public command that returns the secret; provide
has_custom_api_key.plugins/local-stt/permissions/autogenerated/reference.md (2)
88-113: Remove API-key read permissions from the tableExposing a raw API key is unsafe even behind a permission gate. Replace with a non-secret “has-custom-api-key”.
- Delete
allow/deny-get-custom-api-keyrows.- Add
allow/deny-has-custom-api-keyrows.- Update commands/bindings accordingly and regenerate docs.
582-607: Do not allow API-key mutation by defaultSetting credentials should require explicit opt-in scope; remove from defaults and mark as sensitive.
- Remove
allow-set-custom-api-keyfrom defaults.- Consider scoping (per window/origin) and consent UX.
plugins/local-stt/js/bindings.gen.ts (2)
46-54: Validate and normalize base URL at the backend; return nullable when unset.Avoid accepting/returning arbitrary strings; enforce http(s) scheme and non-empty, and consider returning
nullwhen not configured to distinguish from"".Backend change (Rust), then regenerate bindings. JS surface if adopting nullability:
-async getCustomBaseUrl() : Promise<string> { +async getCustomBaseUrl() : Promise<string | null> { return await TAURI_INVOKE("plugin:local-stt|get_custom_base_url"); },
49-51: Do not return raw API keys to the webview.Expose only a presence flag (boolean). Keep secrets backend-only to reduce XSS/extension exfiltration risk.
Proposed binding (after Rust change):
-async getCustomApiKey() : Promise<string | null> { - return await TAURI_INVOKE("plugin:local-stt|get_custom_api_key"); -}, +async getCustomApiKeyPresent() : Promise<boolean> { + return await TAURI_INVOKE("plugin:local-stt|get_custom_api_key_present"); +},
🧹 Nitpick comments (4)
plugins/local-stt/permissions/autogenerated/reference.md (1)
14-16: Rename artifacts: “current-model” → “local-model”Default list still references
allow-get-current-model/allow-set-current-model. Update to the new tokens to avoid confusion and keep docs consistent with bindings.- - `allow-get-current-model` - - `allow-set-current-model` + - `allow-get-local-model` + - `allow-set-local-model`Regenerate this file from the source manifests after the change.
apps/desktop/src/components/editor-area/note-header/listen-button.tsx (1)
77-83: Health gating: consider provider-specific readinessYou OR internal/external/custom. If a stale server remains “ready” in state, gating could pass incorrectly. Prefer checking the active provider and the corresponding server type.
Example:
- const servers = await localSttCommands.getServers(); - const isServerAvailable = (servers.external === "ready") || (servers.internal === "ready") - || (servers.custom === "ready"); + const [provider, servers] = await Promise.all([ + localSttCommands.getProvider(), + localSttCommands.getServers(), + ]); + const isServerAvailable = provider === "Custom" + ? servers.custom === "ready" + : (servers.external === "ready" || servers.internal === "ready");apps/desktop/src/components/settings/components/ai/stt-view-local.tsx (1)
110-110: Guard unknown keys gracefully.Indexing with
data[model.key as SupportedSttModel] || falseis fine; consider?? falseto avoid treating explicitfalseas falsy if the API ever returns non-boolean values.- downloaded: sttModelDownloadStatus.data[model.key as SupportedSttModel] || false, + downloaded: sttModelDownloadStatus.data[model.key as SupportedSttModel] ?? false,plugins/local-stt/js/bindings.gen.ts (1)
58-63: Provider get/set exposed — prefer a single transactional setter.Consider a single
configureProvider({ provider, model?, baseUrl?, apiKey? })to avoid inconsistent state across multiple async calls.I can sketch the Rust + bindings shape if helpful.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (35)
apps/desktop/src/components/editor-area/note-header/listen-button.tsx(1 hunks)apps/desktop/src/components/settings/components/ai/stt-view-local.tsx(11 hunks)apps/desktop/src/components/settings/components/ai/stt-view-remote.tsx(1 hunks)apps/desktop/src/components/settings/views/ai-stt.tsx(5 hunks)apps/desktop/src/components/toast/model-download.tsx(1 hunks)apps/desktop/src/components/toast/model-select.tsx(1 hunks)apps/desktop/src/components/toast/shared.tsx(1 hunks)apps/desktop/src/components/welcome-modal/download-progress-view.tsx(1 hunks)apps/desktop/src/components/welcome-modal/index.tsx(1 hunks)apps/desktop/src/locales/en/messages.po(7 hunks)apps/desktop/src/locales/ko/messages.po(7 hunks)apps/desktop/src/routes/app.tsx(1 hunks)plugins/local-stt/build.rs(1 hunks)plugins/local-stt/js/bindings.gen.ts(3 hunks)plugins/local-stt/permissions/autogenerated/commands/get_current_model.toml(1 hunks)plugins/local-stt/permissions/autogenerated/commands/get_custom_api_key.toml(1 hunks)plugins/local-stt/permissions/autogenerated/commands/get_custom_base_url.toml(1 hunks)plugins/local-stt/permissions/autogenerated/commands/get_custom_model.toml(1 hunks)plugins/local-stt/permissions/autogenerated/commands/get_local_model.toml(1 hunks)plugins/local-stt/permissions/autogenerated/commands/get_provider.toml(1 hunks)plugins/local-stt/permissions/autogenerated/commands/set_current_model.toml(1 hunks)plugins/local-stt/permissions/autogenerated/commands/set_custom_api_key.toml(1 hunks)plugins/local-stt/permissions/autogenerated/commands/set_custom_base_url.toml(1 hunks)plugins/local-stt/permissions/autogenerated/commands/set_custom_model.toml(1 hunks)plugins/local-stt/permissions/autogenerated/commands/set_local_model.toml(1 hunks)plugins/local-stt/permissions/autogenerated/commands/set_provider.toml(1 hunks)plugins/local-stt/permissions/autogenerated/reference.md(6 hunks)plugins/local-stt/permissions/default.toml(1 hunks)plugins/local-stt/permissions/schemas/schema.json(4 hunks)plugins/local-stt/src/commands.rs(2 hunks)plugins/local-stt/src/ext.rs(14 hunks)plugins/local-stt/src/lib.rs(2 hunks)plugins/local-stt/src/model.rs(3 hunks)plugins/local-stt/src/server/mod.rs(1 hunks)plugins/local-stt/src/store.rs(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- plugins/local-stt/permissions/autogenerated/commands/get_custom_base_url.toml
- plugins/local-stt/permissions/autogenerated/commands/set_local_model.toml
🚧 Files skipped from review as they are similar to previous changes (26)
- plugins/local-stt/permissions/autogenerated/commands/set_custom_base_url.toml
- plugins/local-stt/permissions/autogenerated/commands/set_provider.toml
- plugins/local-stt/permissions/autogenerated/commands/get_custom_model.toml
- plugins/local-stt/permissions/autogenerated/commands/set_current_model.toml
- plugins/local-stt/permissions/autogenerated/commands/get_custom_api_key.toml
- apps/desktop/src/components/welcome-modal/download-progress-view.tsx
- plugins/local-stt/permissions/autogenerated/commands/set_custom_api_key.toml
- apps/desktop/src/routes/app.tsx
- plugins/local-stt/permissions/autogenerated/commands/set_custom_model.toml
- apps/desktop/src/components/toast/model-select.tsx
- plugins/local-stt/build.rs
- apps/desktop/src/locales/en/messages.po
- apps/desktop/src/components/settings/views/ai-stt.tsx
- plugins/local-stt/src/commands.rs
- plugins/local-stt/src/model.rs
- plugins/local-stt/permissions/autogenerated/commands/get_local_model.toml
- apps/desktop/src/components/welcome-modal/index.tsx
- plugins/local-stt/permissions/default.toml
- plugins/local-stt/permissions/autogenerated/commands/get_current_model.toml
- apps/desktop/src/components/toast/model-download.tsx
- apps/desktop/src/components/settings/components/ai/stt-view-remote.tsx
- apps/desktop/src/locales/ko/messages.po
- plugins/local-stt/permissions/autogenerated/commands/get_provider.toml
- plugins/local-stt/src/lib.rs
- plugins/local-stt/permissions/schemas/schema.json
- plugins/local-stt/src/store.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. No error handling.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
plugins/local-stt/src/server/mod.rsapps/desktop/src/components/toast/shared.tsxapps/desktop/src/components/editor-area/note-header/listen-button.tsxplugins/local-stt/js/bindings.gen.tsplugins/local-stt/src/ext.rsapps/desktop/src/components/settings/components/ai/stt-view-local.tsx
🧬 Code graph analysis (1)
plugins/local-stt/src/ext.rs (5)
plugins/connector/js/bindings.gen.ts (1)
Connection(115-115)plugins/local-stt/js/bindings.gen.ts (6)
Provider(85-85)GgmlBackend(83-83)SupportedSttModel(89-89)ServerType(87-87)ServerHealth(86-86)WhisperModel(91-91)plugins/local-stt/src/commands.rs (13)
models_dir(11-13)list_ggml_backends(17-21)get_custom_base_url(114-116)set_custom_base_url(128-133)get_custom_api_key(120-124)set_custom_api_key(137-142)get_provider(146-150)set_provider(154-159)get_local_model(63-67)set_local_model(71-76)get_custom_model(163-167)set_custom_model(171-176)is_model_downloaded(31-38)plugins/connector/src/ext.rs (2)
store(140-141)store(144-145)plugins/task/src/ctx.rs (1)
new(19-27)
🔇 Additional comments (7)
plugins/local-stt/src/server/mod.rs (1)
12-14: AddCustomserver type — looks goodSerde tag matches the JS bindings (“custom”). No further issues.
plugins/local-stt/src/ext.rs (1)
68-71: Minor: these changes look solid
- Store scoping and Provider default.
- Provider-aware downloaded/downloading semantics.
- Guarding Custom for download flows.
- Local model key migration.
Also applies to: 92-96, 190-195, 452-455, 574-579, 588-592, 610-620
apps/desktop/src/components/settings/components/ai/stt-view-local.tsx (4)
82-82: LGTM: explicit model list typed viasatisfies.Good use of
satisfies SupportedSttModel[]to keep the list type-safe while allowing thestringextension.
359-361: Selection gating by provider is correct.Highlighting only when
provider === "Local"prevents accidental selection glow while in Custom mode.
366-371: Await model/provider updates before restarting server
localSttCommands.setLocalModel,stopServer, andstartServerreturn promises; without awaiting, restarting may act on stale state. MakehandleClickasync, await these calls in sequence, and pass the explicitkeytostartServer.
Verify ifsetProviderToLocalpersists the provider to the plugin—if not, replace it withawait localSttCommands.setProvider("Local")before callingsetLocalModel.
31-33: Provider persistence confirmed viasetProviderToLocalmutation. ThesetProviderToLocalhelper invokes the React Query mutation, which callslocalSttCommands.setProvider("Local")before any server restart, satisfying the persistence requirement.plugins/local-stt/js/bindings.gen.ts (1)
25-30: Rename to get/setLocalModel looks consistent.Bindings match the renamed Rust commands; downstream callers should be updated accordingly.
| console.log(servers); | ||
| return servers; | ||
| }, | ||
| queryFn: async () => localSttCommands.getServers(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add basic error handling to react-query calls.
Wrap queryFn bodies with try/catch or supply onError to surface failures (toast/log) per guidelines. Silent failures hurt UX and debugging.
Apply:
- const servers = useQuery({
+ const servers = useQuery({
queryKey: ["local-stt-servers"],
- queryFn: async () => localSttCommands.getServers(),
+ queryFn: async () => {
+ try { return await localSttCommands.getServers(); }
+ catch (e) { console.error("getServers failed", e); throw e; }
+ },
refetchInterval: REFETCH_INTERVALS.servers,
});
- const currentSTTModel = useQuery({
+ const currentSTTModel = useQuery({
queryKey: ["current-stt-model"],
- queryFn: () => localSttCommands.getLocalModel(),
+ queryFn: async () => {
+ try { return await localSttCommands.getLocalModel(); }
+ catch (e) { console.error("getLocalModel failed", e); throw e; }
+ },
});Also applies to: 66-66
🤖 Prompt for AI Agents
In apps/desktop/src/components/settings/components/ai/stt-view-local.tsx around
lines 60 and 66, the react-query queryFn calls are silent on failure; wrap each
queryFn body in try/catch and rethrow or add an onError handler to surface
failures (e.g., call the existing toast/logger with a clear message and the
error). Specifically, modify the queryFn to try { return await
localSttCommands.getServers() } catch (err) { toast.error("Failed to load STT
servers"); logger.error("getServers error", err); throw err } (or supply an
onError option that performs the same toast/logger actions) so errors are logged
and shown to the user. Ensure both query usages at lines 60 and 66 are updated
consistently.
Summary by cubic
Adds support for custom STT endpoints by introducing a Custom model type and new commands to store a base URL and API key. This lets the app connect to external STT services without running or downloading local models.
New Features
Migration