Conversation
📝 WalkthroughWalkthroughThis update introduces a multi-step onboarding flow in the welcome modal for configuring large language models (LLMs). It adds new UI components for LLM selection and custom endpoint configuration, integrates form validation for multiple providers, and updates localization files with new strings. Conditional logic for downloading and setting up LLMs is also introduced. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WelcomeModal
participant LLMSelectionView
participant CustomEndpointView
participant Backend
User->>WelcomeModal: Opens onboarding
WelcomeModal->>LLMSelectionView: Render LLM selection step
User->>LLMSelectionView: Selects LLM type and continues
LLMSelectionView->>WelcomeModal: onContinue(selection)
alt selection == "hyprllm"
WelcomeModal->>WelcomeModal: Proceed to model selection & download steps
else selection == "byom"
WelcomeModal->>CustomEndpointView: Render custom endpoint step
User->>CustomEndpointView: Selects provider, enters config, continues
CustomEndpointView->>WelcomeModal: onContinue()
WelcomeModal->>Backend: configureCustomEndpoint(config)
Backend-->>WelcomeModal: Ack
WelcomeModal->>WelcomeModal: Proceed to model selection step
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ 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. 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
apps/desktop/src/components/welcome-modal/llm-selection-view.tsx (1)
22-36: Consider extracting options to a constant outside the componentThe options array is recreated on every render, which is unnecessary. Consider moving it outside the component or using
useMemofor better performance.+const LLM_OPTIONS = [ + { + id: "hyprllm" as const, + title: "HyprLLM (Local)", + subtitle: "Privacy matters more than anything to me", + icon: ShieldIcon, + }, + { + id: "byom" as const, + title: "Bring Your Own Model", + subtitle: "I want first-in-class meeting summarization", + icon: Network, + }, +]; export function LLMSelectionView({ onContinue }: LLMSelectionViewProps) { const [selected, setSelected] = useState<"hyprllm" | "byom" | null>(null); const handleContinue = () => { if (selected) { onContinue(selected); } }; - const options = [ - { - id: "hyprllm", - title: "HyprLLM (Local)", - subtitle: "Privacy matters more than anything to me", - icon: ShieldIcon, - }, - { - id: "byom", - title: "Bring Your Own Model", - subtitle: "I want first-in-class meeting summarization", - icon: Network, - }, - ] as const; + const options = LLM_OPTIONS;apps/desktop/src/components/welcome-modal/custom-endpoint-view.tsx (2)
22-46: Consider fetching model lists dynamically.Hardcoded model lists may become outdated as providers release new models. Consider fetching these lists from a configuration API or making them configurable.
194-196: Document the model filtering logic.The code filters out certain model types but doesn't explain why. Add a comment explaining the reasoning.
+// Filter out non-chat models (image generation, audio, code completion) const excludeKeywords = ["dall-e", "codex", "whisper"];apps/desktop/src/components/welcome-modal/index.tsx (1)
4-4: Remove unnecessary comment.The comment is stating the obvious.
-import { ArrowLeft } from "lucide-react"; // Add this import +import { ArrowLeft } from "lucide-react";
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/desktop/src/components/welcome-modal/custom-endpoint-view.tsx(1 hunks)apps/desktop/src/components/welcome-modal/download-progress-view.tsx(7 hunks)apps/desktop/src/components/welcome-modal/index.tsx(11 hunks)apps/desktop/src/components/welcome-modal/llm-selection-view.tsx(1 hunks)apps/desktop/src/components/welcome-modal/model-selection-view.tsx(1 hunks)apps/desktop/src/locales/en/messages.po(15 hunks)apps/desktop/src/locales/ko/messages.po(15 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:
apps/desktop/src/components/welcome-modal/model-selection-view.tsxapps/desktop/src/components/welcome-modal/download-progress-view.tsxapps/desktop/src/components/welcome-modal/llm-selection-view.tsxapps/desktop/src/components/welcome-modal/custom-endpoint-view.tsxapps/desktop/src/components/welcome-modal/index.tsx
🪛 GitHub Actions: .github/workflows/fmt.yaml
apps/desktop/src/components/welcome-modal/custom-endpoint-view.tsx
[error] 4-11: dprint formatting check failed. The import statements are not formatted correctly. Run 'dprint fmt' to fix code style issues.
⏰ 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 (windows, windows-latest)
- GitHub Check: ci (macos, macos-latest)
🔇 Additional comments (17)
apps/desktop/src/components/welcome-modal/llm-selection-view.tsx (3)
1-8: Check import usage and remove unused importsAll imports appear to be used correctly in the component. The imports are well-organized with external libraries first, then UI components, and finally React hooks.
13-21: LGTM! Clean component initialization and event handlingThe component properly initializes state and handles the continue action with appropriate null checking. The conditional call to
onContinueprevents potential issues.
97-102: Good conditional UX patternThe conditional message display when no option is selected provides clear user guidance. This is a good UX pattern.
apps/desktop/src/components/welcome-modal/model-selection-view.tsx (1)
73-73: Good clarification in heading textAdding "(STT)" to the heading clarifies that this is for Speech-to-Text model selection, which helps distinguish it from the new LLM selection step in the onboarding flow.
apps/desktop/src/locales/ko/messages.po (4)
263-265: LGTM! Proper localization entries addedThe new localization entries support the custom endpoint configuration UI with appropriate source references.
439-443: LGTM! Back button localization addedProper localization entries for the Back button functionality in the welcome modal.
521-524: LGTM! LLM selection localizationAppropriate localization entries for the new LLM selection component.
549-556: LGTM! Custom endpoint configuration localizationGood localization entries for the custom endpoint configuration flow.
apps/desktop/src/components/welcome-modal/download-progress-view.tsx (5)
20-24: LGTM! Proper prop interface extensionThe interface correctly adds the new
llmSelectionprop with appropriate typing to support the conditional LLM download flow.
112-117: LGTM! Clean conditional LLM download initiationThe conditional logic properly initiates the LLM download only when
"hyprllm"is selected, and marks it as completed immediately for other cases. This prevents unnecessary downloads for the "bring your own model" flow.
131-145: LGTM! Consistent conditional event handlingThe event handler setup follows the same conditional pattern as the download initiation, ensuring progress tracking only occurs when needed.
185-197: LGTM! Proper conditional model setupThe LLM completion handler correctly checks both completion status and selection type before setting up the model and starting the server. The effect dependencies are properly updated to include
llmSelection.
244-252: LGTM! Conditional UI rendering matches logicThe UI correctly shows the LLM progress card only when
llmSelectionis"hyprllm", maintaining consistency with the download logic.apps/desktop/src/locales/en/messages.po (1)
263-266: LGTM!The localization strings for the new LLM onboarding flow are appropriately added.
apps/desktop/src/components/welcome-modal/custom-endpoint-view.tsx (1)
71-78: Add error handling for configuration.The
configureCustomEndpointcall lacks error handling. If the configuration fails, users won't receive any feedback.Consider wrapping in try-catch and showing an error message:
-configureCustomEndpoint({ - provider: "openai", - api_base: "", - api_key: values.api_key, - model: values.model, -}); -setIsConfigured(true); +try { + await configureCustomEndpoint({ + provider: "openai", + api_base: "", + api_key: values.api_key, + model: values.model, + }); + setIsConfigured(true); +} catch (error) { + console.error('Failed to configure OpenAI endpoint:', error); + // Show error to user + setIsConfigured(false); +}⛔ Skipped due to learnings
Learnt from: plyght PR: fastrepl/hyprnote#921 File: plugins/location-connectivity/src/error.rs:1-41 Timestamp: 2025-06-06T16:31:46.457Z Learning: In the location-connectivity plugin (plugins/location-connectivity/), comprehensive error handling via LocationConnectivityError is necessary despite the "no error handling" guideline, as the plugin performs system-level operations (WiFi detection, persistent storage) that require proper error propagation to the frontend.apps/desktop/src/components/welcome-modal/index.tsx (2)
320-326: Good conditional logic!The code correctly checks if HyprLLM was selected before attempting to download the model.
158-161: Improve error handling with user feedback.The error is only logged to console without informing the user. Consider propagating the error or showing a user-friendly message.
} catch (error) { console.error("Failed to configure custom endpoint:", error); + // Show error message to user + throw error; // Let the caller handle the error }⛔ Skipped due to learnings
Learnt from: plyght PR: fastrepl/hyprnote#921 File: plugins/location-connectivity/src/error.rs:1-41 Timestamp: 2025-06-06T16:31:46.457Z Learning: In the location-connectivity plugin (plugins/location-connectivity/), comprehensive error handling via LocationConnectivityError is necessary despite the "no error handling" guideline, as the plugin performs system-level operations (WiFi detection, persistent storage) that require proper error propagation to the frontend.
| <div className="flex items-center gap-2 mb-3"> | ||
| <svg | ||
| fill="currentColor" | ||
| fill-rule="evenodd" |
There was a problem hiding this comment.
Fix SVG attribute naming.
Use camelCase for React SVG attributes.
-fill-rule="evenodd"
+fillRule="evenodd"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fill-rule="evenodd" | |
| fillRule="evenodd" |
🤖 Prompt for AI Agents
In apps/desktop/src/components/welcome-modal/custom-endpoint-view.tsx at line
423, the SVG attribute "fill-rule" is used with a hyphen, which is not valid in
React. Change the attribute name to camelCase "fillRule" to comply with React's
SVG attribute naming conventions.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
cubic analysis
4 issues found across 7 files • Review in cubic
React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| const options = [ | ||
| { | ||
| id: "hyprllm", | ||
| title: "HyprLLM (Local)", |
There was a problem hiding this comment.
User-visible string is hard-coded without a wrapper, so it will not be included in translation extraction.
Prompt for AI agents
Address the following comment on apps/desktop/src/components/welcome-modal/llm-selection-view.tsx at line 25:
<comment>User-visible string is hard-coded without a <Trans> wrapper, so it will not be included in translation extraction.</comment>
<file context>
@@ -0,0 +1,104 @@
+import { Trans } from "@lingui/react/macro";
+import { Network, ShieldIcon } from "lucide-react";
+
+import { Card, CardContent } from "@hypr/ui/components/ui/card";
+import PushableButton from "@hypr/ui/components/ui/pushable-button";
+import { cn } from "@hypr/ui/lib/utils";
+import { useState } from "react";
+
+interface LLMSelectionViewProps {
</file context>
| ? "ring-2 ring-blue-500 border-blue-500 bg-blue-50" | ||
| : "hover:border-gray-400", | ||
| )} | ||
| onClick={() => setSelected(option.id)} |
There was a problem hiding this comment.
Interactive Card is not keyboard-accessible because it lacks role and tabIndex attributes (Based on your team's feedback about ensuring all interactive elements are accessible via keyboard).
Prompt for AI agents
Address the following comment on apps/desktop/src/components/welcome-modal/llm-selection-view.tsx at line 63:
<comment>Interactive Card is not keyboard-accessible because it lacks role and tabIndex attributes (Based on your team's feedback about ensuring all interactive elements are accessible via keyboard).</comment>
<file context>
@@ -0,0 +1,104 @@
+import { Trans } from "@lingui/react/macro";
+import { Network, ShieldIcon } from "lucide-react";
+
+import { Card, CardContent } from "@hypr/ui/components/ui/card";
+import PushableButton from "@hypr/ui/components/ui/pushable-button";
+import { cn } from "@hypr/ui/lib/utils";
+import { useState } from "react";
+
+interface LLMSelectionViewProps {
</file context>
| if (llmSelection === "hyprllm") { | ||
| localLlmCommands.downloadModel("HyprLLM", llmDownload.channel); | ||
| } else { | ||
| setLlmDownload(prev => ({ ...prev, completed: true })); |
There was a problem hiding this comment.
The else-branch marks the LLM download as completed even though no download occurred; if the user later changes the selection to "hyprllm", llmDownload.completed remains true and handleLlmCompletion will start the LLM server before the model is actually present, leading to runtime failures. A more robust approach would reset llmDownload state (progress 0, completed false) when switching from BYOM to HyprLLM instead of permanently setting completed true.
Prompt for AI agents
Address the following comment on apps/desktop/src/components/welcome-modal/download-progress-view.tsx at line 115:
<comment>The else-branch marks the LLM download as completed even though no download occurred; if the user later changes the selection to "hyprllm", llmDownload.completed remains true and handleLlmCompletion will start the LLM server before the model is actually present, leading to runtime failures. A more robust approach would reset llmDownload state (progress 0, completed false) when switching from BYOM to HyprLLM instead of permanently setting completed true.</comment>
<file context>
@@ -107,7 +109,11 @@ export const DownloadProgressView = ({
useEffect(() => {
localSttCommands.downloadModel(selectedSttModel, sttDownload.channel);
- localLlmCommands.downloadModel("HyprLLM", llmDownload.channel);
+ if (llmSelection === "hyprllm") {
+ localLlmCommands.downloadModel("HyprLLM", llmDownload.channel);
+ } else {
+ setLlmDownload(prev => ({ ...prev, completed: true }));
+ }
</file context>
| })); | ||
| }; | ||
| } | ||
| }, [selectedSttModel, sttDownload.channel, llmDownload.channel, llmSelection]); |
There was a problem hiding this comment.
Adding llmSelection to the dependency array forces this effect to run every time the user toggles the LLM option, which re-invokes localSttCommands.downloadModel and begins a new STT download even when the STT model has not changed. This can create duplicate downloads and waste bandwidth. (Based on your team's feedback about preventing unnecessary re-downloads in effects)
Prompt for AI agents
Address the following comment on apps/desktop/src/components/welcome-modal/download-progress-view.tsx at line 145:
<comment>Adding llmSelection to the dependency array forces this effect to run every time the user toggles the LLM option, which re-invokes localSttCommands.downloadModel and begins a new STT download even when the STT model has not changed. This can create duplicate downloads and waste bandwidth. (Based on your team's feedback about preventing unnecessary re-downloads in effects)</comment>
<file context>
@@ -122,19 +128,21 @@ export const DownloadProgressView = ({
}));
};
- llmDownload.channel.onmessage = (progress) => {
- if (progress < 0) {
- setLlmDownload(prev => ({ ...prev, error: true }));
- return;
- }
+ if (llmSelection === "hyprllm") {
</file context>
| }, [selectedSttModel, sttDownload.channel, llmDownload.channel, llmSelection]); | |
| }, [selectedSttModel, sttDownload.channel, llmDownload.channel]); |
1 participant
Summary by cubic
Added a new onboarding flow that lets users choose between a local language model (HyprLLM) or bring their own model, with guided setup for custom endpoints and improved step navigation.