Conversation
📝 WalkthroughWalkthroughThe changes replace the "list_supported_models" command and its associated permissions with a new "list_downloaded_model" command in the local-llm plugin. Application logic for displaying model download notifications is updated to use a new React Query hook that checks for downloaded models. Permission files, schemas, and documentation are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as ModelDownloadNotification (React)
participant Query as React Query Hook
participant LocalLLM as localLlmCommands
UI->>Query: useQuery("listDownloadedModels")
Query->>LocalLLM: localLlmCommands.listDownloadedModel()
LocalLLM-->>Query: [list of downloaded models]
Query-->>UI: data
UI->>UI: If data.length === 0, show download notification
Estimated code review effort3 (~45 minutes) Possibly related PRs
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
apps/desktop/src/components/toast/model-download.tsx (1)
144-144: Add missing dependency to useEffect hook.The
useEffecthook useslistDownloadedModels.dataon line 88 but doesn't include it in the dependency array. This could cause stale closures and incorrect behavior.- }, [checkForModelDownload.data, sttModelDownloading.data, llmModelDownloading.data, isDismissed]); + }, [checkForModelDownload.data, sttModelDownloading.data, llmModelDownloading.data, isDismissed, listDownloadedModels.data]);
🧹 Nitpick comments (1)
apps/desktop/src/components/welcome-modal/index.tsx (1)
82-82: LGTM! Session storage flag correctly placed for toast dismissal.The timing is appropriate - setting the flag after model mutation but before modal closure ensures the toast behavior is properly controlled.
Consider extracting the session storage key to a constant to avoid magic strings:
+const MODEL_DOWNLOAD_TOAST_DISMISSED_KEY = "model-download-toast-dismissed"; + const handleModelSelected = (model: SupportedModel) => { selectSTTModel.mutate(model); - sessionStorage.setItem("model-download-toast-dismissed", "true"); + sessionStorage.setItem(MODEL_DOWNLOAD_TOAST_DISMISSED_KEY, "true"); onClose(); };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/desktop/src/components/toast/model-download.tsx(2 hunks)apps/desktop/src/components/welcome-modal/index.tsx(1 hunks)plugins/local-llm/build.rs(1 hunks)plugins/local-llm/permissions/autogenerated/commands/list_downloaded_model.toml(1 hunks)plugins/local-llm/permissions/autogenerated/commands/list_supported_models.toml(0 hunks)plugins/local-llm/permissions/autogenerated/reference.md(2 hunks)plugins/local-llm/permissions/default.toml(1 hunks)plugins/local-llm/permissions/schemas/schema.json(2 hunks)
📓 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-llm/build.rsapps/desktop/src/components/welcome-modal/index.tsxapps/desktop/src/components/toast/model-download.tsx
💤 Files with no reviewable changes (1)
- plugins/local-llm/permissions/autogenerated/commands/list_supported_models.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:
plugins/local-llm/build.rsapps/desktop/src/components/welcome-modal/index.tsxapps/desktop/src/components/toast/model-download.tsx
⏰ 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 (9)
plugins/local-llm/build.rs (1)
12-12: LGTM! Command replacement is correctly implemented.The addition of
"list_downloaded_model"to the COMMANDS array aligns with the broader refactoring to replace the oldlist_supported_modelsfunctionality.plugins/local-llm/permissions/default.toml (1)
14-14: LGTM! Permission update maintains consistency.The permission string correctly reflects the new command name, maintaining the established naming convention.
plugins/local-llm/permissions/autogenerated/commands/list_downloaded_model.toml (1)
1-14: LGTM! Well-structured permission file follows established patterns.The permission definitions correctly implement both allow and deny variants for the new
list_downloaded_modelcommand, following the standard format and including appropriate auto-generation warnings.plugins/local-llm/permissions/autogenerated/reference.md (2)
17-17: LGTM! Default permission list correctly updated.The documentation properly reflects the new permission in the default set.
187-206: LGTM! Permission table entries comprehensively updated.All references to the old
list-supported-modelspermission have been correctly replaced withlist-downloaded-model, maintaining consistency in both identifiers and descriptions.apps/desktop/src/components/toast/model-download.tsx (2)
44-50: LGTM! Clean implementation of the new query hook.The new
listDownloadedModelsquery correctly calls the updated command and uses a 3-second polling interval which is appropriate for this use case.
88-88: Logic change looks correct.Checking if the downloaded models array is empty is a more direct approach than the previous logic and aligns well with the new command functionality.
plugins/local-llm/permissions/schemas/schema.json (2)
370-379: Permission identifiers correctly updated.The permission identifiers have been properly renamed from
list-supported-modelstolist-downloaded-modelfor both allow and deny cases, with matching descriptions and markdown documentation.
442-445: Default permission documentation properly updated.The default permission set description correctly includes the new
allow-list-downloaded-modelpermission, maintaining consistency with the command rename.
No description provided.