ui polishes #1653
ui polishes #1653
Conversation
📝 WalkthroughWalkthroughAdds model-aware empty-chat UI with settings navigation and quick actions, LLM/STT banners and provider-driven model selection, richer chat input controls, permissions/onboarding and template updates (category/targets), many import/classname cleanups, and a pnpm version bump. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChatView
participant ConfigState as ModelConfig
participant ChatBody
participant ChatBodyEmpty
participant SettingsWindow
ChatView->>ConfigState: read current model/provider
ConfigState-->>ChatView: isModelConfigured
alt configured
ChatView->>ChatBody: render with isModelConfigured=true
ChatBody->>ChatBodyEmpty: render quick actions (if no messages)
else not configured
ChatView->>ChatBody: render with isModelConfigured=false
ChatBody->>ChatBodyEmpty: render "Go to settings" prompt
User->>ChatBodyEmpty: clicks "Go to settings"
ChatBodyEmpty->>SettingsWindow: open settings via windows commands
end
sequenceDiagram
participant User
participant ProviderSelect
participant ModelStore
participant BannerForSTT
User->>ProviderSelect: select provider
ProviderSelect->>ModelStore: clear model & fetch models
ModelStore-->>ProviderSelect: models list
ProviderSelect->>ProviderSelect: auto-select first model (if any)
BannerForSTT->>ModelStore: query download/status for known models
alt none downloaded
BannerForSTT-->>User: show "download a model" banner
else available
BannerForSTT-->>User: no banner
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Areas needing extra attention:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 2 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![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/onboarding/welcome.tsx (1)
37-49: Remove commented-out code.Commented-out code should be removed since version control maintains the history. If this feature may return, consider adding a TODO comment with a tracking issue instead.
Apply this diff to remove the commented code:
- { - /*<div - className={cn([ - "flex flex-row items-center gap-1", - "text-neutral-400 transition-colors hover:text-neutral-800", - ])} - > - <button className="text-sm underline" onClick={() => onNext({ local: true })}> - Or proceed without an account - </button> - <CircleQuestionMarkIcon className="h-4 w-4 cursor-help" /> - </div>*/ - }apps/desktop/src/components/chat/body/empty.tsx (1)
16-19: TODO: Implement quick action handler.The
handleQuickActionfunction currently only logs the action. Consider implementing the actual functionality or adding a TODO comment to track this work.Do you want me to generate an implementation for the quick action handler or open a new issue to track this task?
apps/desktop/src/components/chat/input.tsx (1)
46-59: TODO: Implement file attachment, screenshot, and voice input handlers.These handlers are currently stubbed with console.log statements. Consider implementing the actual functionality or opening issues to track these features.
Do you want me to help generate implementation code for these handlers or open issues to track them?
apps/desktop/src/components/settings/ai/stt/no-model-banner.tsx (1)
39-46: Optimize query execution to run conditionally.These four model download queries execute unconditionally but are only used when
current_stt_provider === "hyprnote"(lines 59-72). For all other providers, these queries waste resources.Consider restructuring to conditionally execute these queries:
function useHasSTTModel(): { hasModel: boolean; message: string } { const { current_stt_provider, current_stt_model } = useConfigValues( ["current_stt_provider", "current_stt_model"] as const, ); const configuredProviders = main.UI.useResultTable(main.QUERIES.sttProviders, main.STORE_ID); - const [p2, p3, tinyEn, smallEn] = useQueries({ - queries: [ - sttModelQueries.isDownloaded("am-parakeet-v2"), - sttModelQueries.isDownloaded("am-parakeet-v3"), - sttModelQueries.isDownloaded("QuantizedTinyEn"), - sttModelQueries.isDownloaded("QuantizedSmallEn"), - ], - }); + const shouldCheckHyprnoteModels = current_stt_provider === "hyprnote"; + const [p2, p3, tinyEn, smallEn] = useQueries({ + queries: shouldCheckHyprnoteModels ? [ + sttModelQueries.isDownloaded("am-parakeet-v2"), + sttModelQueries.isDownloaded("am-parakeet-v3"), + sttModelQueries.isDownloaded("QuantizedTinyEn"), + sttModelQueries.isDownloaded("QuantizedSmallEn"), + ] : [], + }); if (!current_stt_provider || !current_stt_model) { return { hasModel: false, message: "Please select a provider and model" }; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (17)
apps/desktop/src-tauri/icons/nightly/128x128.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/128x128@2x.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/16.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/32x32.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/64x64.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square107x107Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square142x142Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square150x150Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square284x284Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square30x30Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square310x310Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square44x44Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square71x71Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square89x89Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/StoreLogo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/icon.icois excluded by!**/*.icoapps/desktop/src-tauri/icons/nightly/icon.pngis excluded by!**/*.png
📒 Files selected for processing (31)
apps/desktop/src/components/chat/body/empty.tsx(1 hunks)apps/desktop/src/components/chat/body/index.tsx(3 hunks)apps/desktop/src/components/chat/header.tsx(1 hunks)apps/desktop/src/components/chat/index.tsx(1 hunks)apps/desktop/src/components/chat/input.tsx(3 hunks)apps/desktop/src/components/chat/interactive.tsx(1 hunks)apps/desktop/src/components/chat/trigger.tsx(1 hunks)apps/desktop/src/components/chat/view.tsx(1 hunks)apps/desktop/src/components/main/body/index.tsx(2 hunks)apps/desktop/src/components/main/body/search.tsx(2 hunks)apps/desktop/src/components/main/body/sessions/floating/listen.tsx(1 hunks)apps/desktop/src/components/main/body/shared.tsx(2 hunks)apps/desktop/src/components/main/sidebar/banner/registry.tsx(3 hunks)apps/desktop/src/components/onboarding/calendar.tsx(0 hunks)apps/desktop/src/components/onboarding/permissions.tsx(2 hunks)apps/desktop/src/components/onboarding/shared.tsx(2 hunks)apps/desktop/src/components/onboarding/welcome.tsx(3 hunks)apps/desktop/src/components/settings/ai/llm/configure.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/index.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/no-model-banner.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/select.tsx(4 hunks)apps/desktop/src/components/settings/ai/stt/configure.tsx(2 hunks)apps/desktop/src/components/settings/ai/stt/index.tsx(1 hunks)apps/desktop/src/components/settings/ai/stt/no-model-banner.tsx(1 hunks)apps/desktop/src/components/settings/ai/stt/select.tsx(3 hunks)apps/desktop/src/components/settings/ai/stt/shared.tsx(1 hunks)apps/desktop/src/components/settings/memory/custom-vocabulary.tsx(8 hunks)apps/desktop/src/routes/app.tsx(1 hunks)apps/desktop/src/routes/app/main/_layout.index.tsx(1 hunks)apps/desktop/src/routes/app/onboarding/index.tsx(1 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (1)
- apps/desktop/src/components/onboarding/calendar.tsx
🧰 Additional context used
🧬 Code graph analysis (12)
apps/desktop/src/components/settings/ai/stt/no-model-banner.tsx (4)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/config/use-config.ts (1)
useConfigValues(32-53)apps/desktop/src/components/settings/ai/stt/shared.tsx (1)
sttModelQueries(248-275)apps/desktop/src/store/tinybase/schema-internal.ts (1)
AIProviderStorage(40-40)
apps/desktop/src/components/settings/ai/llm/index.tsx (1)
apps/desktop/src/components/settings/ai/llm/no-model-banner.tsx (1)
NoModelBanner(11-31)
apps/desktop/src/components/chat/view.tsx (1)
apps/desktop/src/components/chat/input.tsx (1)
ChatMessageInput(12-135)
apps/desktop/src/components/main/body/search.tsx (3)
apps/desktop/src/contexts/search/ui.tsx (1)
useSearch(236-242)apps/desktop/src/hooks/useCmdKeyPressed.tsx (1)
useCmdKeyPressed(3-35)packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/settings/ai/llm/no-model-banner.tsx (3)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/auth.tsx (1)
useAuth(177-185)apps/desktop/src/config/use-config.ts (1)
useConfigValues(32-53)
apps/desktop/src/components/settings/ai/stt/index.tsx (1)
apps/desktop/src/components/settings/ai/stt/no-model-banner.tsx (1)
NoModelBanner(10-30)
apps/desktop/src/components/chat/input.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/chat/body/index.tsx (1)
apps/desktop/src/components/chat/body/empty.tsx (1)
ChatBodyEmpty(4-68)
apps/desktop/src/components/onboarding/permissions.tsx (2)
apps/desktop/src/components/onboarding/shared.tsx (1)
OnboardingContainer(21-43)packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/settings/ai/stt/select.tsx (2)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/components/settings/ai/stt/shared.tsx (3)
ProviderId(8-8)PROVIDERS(46-104)displayModelId(22-44)
apps/desktop/src/components/main/body/shared.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/settings/memory/custom-vocabulary.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
🔇 Additional comments (37)
apps/desktop/src/components/chat/trigger.tsx (1)
1-3: Import reordering looks good.The reorganization of imports aligns with the PR's broader effort to standardize imports across chat components. The component logic remains sound and unchanged.
apps/desktop/src/components/onboarding/welcome.tsx (1)
29-35: LGTM! Button styling improvements enhance the CTA.The full-width button with large size is appropriate for a welcome screen's primary action.
apps/desktop/src/routes/app.tsx (1)
1-1: LGTM! Import organization improvement.Moving the TooltipProvider import to the top of the file improves code organization and readability.
apps/desktop/src/routes/app/main/_layout.index.tsx (1)
19-19: LGTM! Padding simplified to uniform value.The change from
pl-1 pr-0 pt-1 pb-1top-1adds right padding (pr-1) where there was previously none (pr-0), creating uniform padding on all sides. This appears to be an intentional layout simplification aligned with the broader UI refinements in this PR.apps/desktop/src/components/main/body/index.tsx (1)
79-79: LGTM! Consistent icon color styling.The addition of
text-neutral-600to both the PanelLeftOpenIcon and the button with PlusIcon provides consistent visual styling across the UI. These are purely cosmetic changes with no behavioral impact.Also applies to: 154-156
apps/desktop/src/components/settings/ai/llm/configure.tsx (1)
1-3: LGTM! Duplicate imports eliminated.Consolidating the Accordion components and
cnimports at the top of the file eliminates duplication and follows standard import organization practices.apps/desktop/src/components/chat/index.tsx (1)
1-2: LGTM! Import moved to top.The
windowsCommandsimport has been appropriately relocated to the top of the file, improving code organization.apps/desktop/src/components/main/body/sessions/floating/listen.tsx (1)
1-6: LGTM! Imports consolidated at top.Multiple imports (miscCommands, windowsCommands, Button, Popover, Spinner, Tooltip) have been properly relocated to the top of the file, improving code organization and readability.
apps/desktop/src/routes/app/onboarding/index.tsx (1)
41-42: LGTM! Onboarding layout restructured.The layout has been restructured to use a flex column container with uniform padding and an absolute-positioned drag region at the top. This provides better structure for the onboarding flow and ensures the drag region is properly positioned for window interactions in the Tauri application.
package.json (1)
5-5: No breaking changes detected in pnpm@10.21.0 upgrade.There are no new breaking changes listed for pnpm 10.20 or 10.21. The update from pnpm@10.19.0 to pnpm@10.21.0 consists only of minor version bumps with additive features (Node.js runtime installation support, trustPolicy setting) and patch fixes. These changes are backward compatible and won't impact the existing build or development workflow.
apps/desktop/src/components/settings/ai/stt/shared.tsx (1)
1-2: LGTM! Import consolidation improves code organization.The local-stt imports are now at the top of the file, eliminating duplicate imports later in the file. This improves readability and follows best practices.
apps/desktop/src/components/chat/interactive.tsx (1)
1-2: LGTM! Import consolidation aligns with PR-wide refactoring.Moving the
cnimport to the top of the file removes duplicate imports and improves consistency across the codebase.apps/desktop/src/components/settings/ai/stt/configure.tsx (2)
267-267: LGTM! Visual enhancement for progress overlay.The semi-transparent black background (
bg-black/50) improves contrast and readability during download progress.
379-383: LGTM! Proper loading state handling prevents premature reset.The additional check for
!isDownloading.isLoadingprevents the task state from being reset while the query is still loading its initial state. The dependency array is correctly updated to includeisDownloading.isLoading.apps/desktop/src/components/main/sidebar/banner/registry.tsx (3)
1-1: LGTM! Icons better represent their respective features.
AudioLinesIconis more semantically appropriate for STT (speech-to-text), andSparklesIconbetter represents LLM capabilities compared to the previous icons.
32-36: LGTM! Banner content is clearer and more actionable.The updated title "Configure STT model" and description clearly communicate what's needed. The action label "Go to settings" is concise and direct.
46-50: LGTM! Consistent improvements to LLM banner.The updates mirror the STT banner improvements with clear, actionable content and consistent styling.
apps/desktop/src/components/main/body/search.tsx (2)
61-70: LGTM! Button component adoption improves consistency.Replacing the native button with the UI
Buttoncomponent provides consistent styling across the application. The icon sizing updates using Tailwind classes (size-4) are cleaner than explicit width/height props.
78-91: LGTM! Responsive behavior enhances UX on narrow screens.The
hasSpacecondition ensures the search input maintains optimal width on narrow screens (alwaysw-[250px]), while allowing dynamic width changes on wider screens. The computedwidthvariable is clearer than inline conditionals.apps/desktop/src/components/main/body/shared.tsx (1)
45-156: LGTM! Hover interactions enhance tab UX.The hover state management adds polished visual feedback:
- Icons transition to close buttons on hover
- Active tabs show a pulsing red dot indicator
- Smooth opacity transitions create professional feel
The implementation correctly prevents event bubbling for the close buttons and maintains proper z-index layering for the overlays.
apps/desktop/src/components/chat/body/empty.tsx (2)
4-40: LGTM! Configuration-aware rendering improves onboarding.The
isModelConfiguredprop enables dynamic empty state rendering. When unconfigured, the component guides users to settings with a clear call-to-action. The default value oftruemaintains backward compatibility.
42-67: LGTM! Enhanced empty state provides better user guidance.The quick action buttons provide clear starting points for users. The visual design with the Hyprnote branding and animated icon creates a welcoming experience.
apps/desktop/src/components/settings/ai/llm/no-model-banner.tsx (2)
11-31: LGTM! Banner provides clear visual feedback for missing configuration.The red alert banner with icon effectively communicates configuration issues to users. Conditional rendering ensures it only appears when needed.
33-72: LGTM! Comprehensive validation logic with clear messaging.The hook systematically checks all configuration requirements:
- Provider and model selection
- Provider existence in registry
- Authentication for "hyprnote" provider
- Provider configuration (base URL)
- API key requirements
Each validation step provides a clear, actionable message. The
useMemooptimization with correct dependencies prevents unnecessary recalculations.apps/desktop/src/components/settings/ai/llm/index.tsx (1)
2-2: LGTM!The addition of the NoModelBanner component is clean and integrates well with the existing LLM settings UI.
Also applies to: 8-8
apps/desktop/src/components/settings/ai/stt/index.tsx (1)
2-2: LGTM!The NoModelBanner integration mirrors the LLM settings implementation, maintaining consistency across AI settings.
Also applies to: 8-8
apps/desktop/src/components/chat/header.tsx (1)
1-3: LGTM!The import reorganization improves code structure by consolidating related imports.
apps/desktop/src/components/chat/view.tsx (2)
107-107: LGTM!The isModelConfigured prop properly propagates model configuration state to ChatBody, enabling model-aware UI rendering.
110-110: LGTM!The simplified disabled logic correctly prevents input when the model is unavailable or the chat is not ready.
apps/desktop/src/components/chat/input.tsx (3)
35-44: LGTM!The useEffect properly guards against uninitialized or destroyed editor instances before focusing.
63-132: LGTM!The UI restructure provides a comprehensive input interface with attachment, screenshot, voice, and send controls. The disabled state handling and visual feedback are well-implemented.
154-159: LGTM!The ChatPlaceholder implementation correctly uses the PlaceholderFunction type and provides contextual placeholder text.
apps/desktop/src/components/settings/ai/stt/no-model-banner.tsx (2)
10-30: LGTM!The NoModelBanner component provides clear visual feedback when no STT model is configured.
48-89: LGTM!The validation logic provides comprehensive checks with clear, actionable error messages for different scenarios.
apps/desktop/src/components/chat/body/index.tsx (3)
1-1: LGTM!The cn import consolidation improves code organization.
17-17: LGTM!The isModelConfigured prop is properly typed with a sensible default that maintains backward compatibility.
Also applies to: 24-24
39-39: LGTM!The layout changes with the spacer div properly align the chat content, and the isModelConfigured prop propagation enables model-aware empty state rendering.
Also applies to: 43-43, 45-45
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/components/settings/ai/llm/select.tsx(4 hunks)apps/desktop/src/components/settings/ai/stt/select.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/components/settings/ai/llm/select.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/desktop/src/components/settings/ai/stt/select.tsx (2)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/components/settings/ai/stt/shared.tsx (3)
ProviderId(8-8)PROVIDERS(46-104)displayModelId(22-44)
ComputelessComputer
force-pushed
the
john-desktop-wip
branch
2 times, most recently
from
dcfbf03 to
1345b59
Compare
There was a problem hiding this comment.
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)
apps/desktop/src/components/settings/ai/llm/select.tsx (1)
192-194: Do not log API credentials.This
console.logprints the provider’s API key to the developer console. Logging secrets is a critical security leak—remove the log (or, if absolutely needed, omit the key).- console.log(provider.id, baseUrl, apiKey);
♻️ Duplicate comments (1)
apps/desktop/src/components/settings/ai/stt/select.tsx (1)
74-83: Guard the auto-select against stale providers.The deferred
setTimeoutcan still run after the user switches providers, seeding a model from the previous provider (e.g., Hyprnote → Deepgram ends up with a Hyprnote model). Keep the provider/model pair consistent by verifying the provider inside the timeout before writing.if (availableModels.length > 0) { - setTimeout(() => { - form.setFieldValue("model", availableModels[0].id); - }, 0); + const [firstModel] = availableModels; + setTimeout(() => { + if (form.getFieldValue("provider") !== providerId) { + return; + } + form.setFieldValue("model", firstModel.id); + }, 0);
🧹 Nitpick comments (4)
apps/desktop/src/components/chat/input.tsx (1)
46-59: Verify TODO placeholders are intentional for this PR.Three action handlers (attach file, screenshot, voice input) are stubbed with console.log statements. Ensure these placeholders align with the PR's "ui polishes" scope.
Do these TODO implementations need to be tracked in a follow-up issue?
apps/desktop/src/components/chat/body/empty.tsx (2)
9-19: Quick actions are hardcoded.Consider whether the
quickActionsarray should be configurable or derived from available features rather than being hardcoded.If these actions will vary based on context or user preferences, consider making them props or deriving them from configuration.
16-19: Verify console.log placeholder is intentional.The
handleQuickActionfunction only logs the action. Ensure this placeholder aligns with the PR's scope, similar to the TODO handlers ininput.tsx.Do these placeholders need to be implemented in this PR or tracked separately?
apps/desktop/src/components/settings/memory/custom-vocabulary.tsx (1)
194-222: LGTM! Item numbering implementation is clean.The item number display with conditional visibility during search provides good UX. The layout structure clearly separates the numeric indicator from the content.
Optional: The conditional class on line 195 could be simplified to:
- <span className={cn(["text-sm text-neutral-400 w-4 flex-shrink-0 text-center", isSearching && "invisible"])}> + <span className={cn("text-sm text-neutral-400 w-4 flex-shrink-0 text-center", isSearching && "invisible")}>
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (17)
apps/desktop/src-tauri/icons/nightly/128x128.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/128x128@2x.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/16.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/32x32.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/64x64.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square107x107Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square142x142Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square150x150Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square284x284Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square30x30Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square310x310Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square44x44Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square71x71Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square89x89Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/StoreLogo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/icon.icois excluded by!**/*.icoapps/desktop/src-tauri/icons/nightly/icon.pngis excluded by!**/*.png
📒 Files selected for processing (31)
apps/desktop/src/components/chat/body/empty.tsx(1 hunks)apps/desktop/src/components/chat/body/index.tsx(3 hunks)apps/desktop/src/components/chat/header.tsx(1 hunks)apps/desktop/src/components/chat/index.tsx(1 hunks)apps/desktop/src/components/chat/input.tsx(3 hunks)apps/desktop/src/components/chat/interactive.tsx(1 hunks)apps/desktop/src/components/chat/trigger.tsx(1 hunks)apps/desktop/src/components/chat/view.tsx(1 hunks)apps/desktop/src/components/main/body/index.tsx(5 hunks)apps/desktop/src/components/main/body/search.tsx(2 hunks)apps/desktop/src/components/main/body/sessions/floating/listen.tsx(1 hunks)apps/desktop/src/components/main/body/shared.tsx(2 hunks)apps/desktop/src/components/main/sidebar/banner/registry.tsx(3 hunks)apps/desktop/src/components/onboarding/calendar.tsx(0 hunks)apps/desktop/src/components/onboarding/permissions.tsx(2 hunks)apps/desktop/src/components/onboarding/shared.tsx(2 hunks)apps/desktop/src/components/onboarding/welcome.tsx(3 hunks)apps/desktop/src/components/settings/ai/llm/configure.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/index.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/no-model-banner.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/select.tsx(3 hunks)apps/desktop/src/components/settings/ai/stt/configure.tsx(2 hunks)apps/desktop/src/components/settings/ai/stt/index.tsx(1 hunks)apps/desktop/src/components/settings/ai/stt/no-model-banner.tsx(1 hunks)apps/desktop/src/components/settings/ai/stt/select.tsx(3 hunks)apps/desktop/src/components/settings/ai/stt/shared.tsx(1 hunks)apps/desktop/src/components/settings/memory/custom-vocabulary.tsx(8 hunks)apps/desktop/src/routes/app.tsx(1 hunks)apps/desktop/src/routes/app/main/_layout.index.tsx(1 hunks)apps/desktop/src/routes/app/onboarding/index.tsx(1 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (1)
- apps/desktop/src/components/onboarding/calendar.tsx
🚧 Files skipped from review as they are similar to previous changes (15)
- apps/desktop/src/components/settings/ai/llm/configure.tsx
- apps/desktop/src/components/settings/ai/stt/index.tsx
- apps/desktop/src/components/settings/ai/llm/index.tsx
- apps/desktop/src/routes/app/main/_layout.index.tsx
- apps/desktop/src/components/settings/ai/stt/configure.tsx
- apps/desktop/src/components/onboarding/shared.tsx
- package.json
- apps/desktop/src/components/onboarding/welcome.tsx
- apps/desktop/src/components/chat/body/index.tsx
- apps/desktop/src/components/main/sidebar/banner/registry.tsx
- apps/desktop/src/components/main/body/sessions/floating/listen.tsx
- apps/desktop/src/components/chat/header.tsx
- apps/desktop/src/routes/app.tsx
- apps/desktop/src/components/chat/interactive.tsx
- apps/desktop/src/components/settings/ai/stt/shared.tsx
🧰 Additional context used
🧬 Code graph analysis (11)
apps/desktop/src/components/settings/ai/llm/no-model-banner.tsx (3)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/auth.tsx (1)
useAuth(177-185)apps/desktop/src/config/use-config.ts (1)
useConfigValues(32-53)
apps/desktop/src/components/main/body/index.tsx (1)
plugins/windows/src/ext.rs (1)
close(56-62)
apps/desktop/src/components/main/body/shared.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/chat/view.tsx (1)
apps/desktop/src/components/chat/input.tsx (1)
ChatMessageInput(12-135)
apps/desktop/src/components/settings/ai/llm/select.tsx (3)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/components/settings/ai/llm/shared.tsx (1)
PROVIDERS(6-63)apps/desktop/src/components/settings/ai/shared/model-combobox.tsx (1)
ModelCombobox(45-233)
apps/desktop/src/components/settings/ai/stt/no-model-banner.tsx (4)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/config/use-config.ts (1)
useConfigValues(32-53)apps/desktop/src/components/settings/ai/stt/shared.tsx (1)
sttModelQueries(248-275)apps/desktop/src/store/tinybase/schema-internal.ts (1)
AIProviderStorage(40-40)
apps/desktop/src/components/main/body/search.tsx (3)
apps/desktop/src/contexts/search/ui.tsx (1)
useSearch(236-242)apps/desktop/src/hooks/useCmdKeyPressed.tsx (1)
useCmdKeyPressed(3-35)packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/onboarding/permissions.tsx (2)
apps/desktop/src/components/onboarding/shared.tsx (1)
OnboardingContainer(21-43)packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/settings/memory/custom-vocabulary.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/chat/input.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/settings/ai/stt/select.tsx (2)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/components/settings/ai/stt/shared.tsx (3)
ProviderId(8-8)PROVIDERS(46-104)displayModelId(22-44)
🔇 Additional comments (24)
apps/desktop/src/components/chat/trigger.tsx (1)
3-4: LGTM! Fixes missing import.This adds the necessary import for
createPortalwhich is already being used in the component (line 6). Without this import, the component would throw a ReferenceError at runtime.apps/desktop/src/routes/app/onboarding/index.tsx (1)
41-42: Verify drag region remains interactive if content overlaps.The separation of the drag region from the content container is a good pattern. However, without an explicit
z-indexon the drag region, tall content that extends into the top 56px could overlap and potentially obscure the draggable area depending on stacking context.Consider adding
z-index: 10(or similar) to the drag region div to ensure it remains interactive:- <div data-tauri-drag-region className="h-14 w-full absolute top-0 left-0 right-0" /> + <div data-tauri-drag-region className="h-14 w-full absolute top-0 left-0 right-0 z-10" />Alternatively, verify that the onboarding content is never tall enough to extend into the top region.
apps/desktop/src/components/chat/index.tsx (1)
1-2: LGTM! Clean import organization.Relocating the import to the top improves readability and eliminates the duplicate import that was likely present later in the file.
apps/desktop/src/components/chat/input.tsx (3)
41-44: Focus management looks correct.The conditional focus logic properly prevents focusing the editor when disabled, avoiding potential UI issues.
63-132: Consistent action bar UI with proper disabled states.The button layout and disabled styling are well-structured. The use of
cn()with conditional classes ensures consistent visual feedback.
154-159: Placeholder component implementation is correct.The
ChatPlaceholderfunction properly implements thePlaceholderFunctionsignature and only renders for the first paragraph node.apps/desktop/src/components/chat/view.tsx (2)
107-107: Configuration awareness properly threaded.The
isModelConfiguredprop correctly derives from the model's existence and will enable the empty state to render configuration-aware UI.
110-110: Simplified disabled logic is cleaner.The boolean expression
!model || status !== "ready"is more maintainable than the previous conditional object structure, while still accepting the union type defined inChatMessageInput.apps/desktop/src/components/chat/body/empty.tsx (3)
4-4: Prop signature enables configuration-aware rendering.The
isModelConfiguredprop with a sensible default allows the component to gracefully handle both configured and unconfigured states.
21-40: Configuration prompt UI looks good.The unconfigured state properly guides users to settings with clear messaging and an actionable button.
27-27: Asset path verified and valid.The
dynamic.giffile exists atapps/desktop/public/assets/dynamic.gif, which is correctly referenced by the path/assets/dynamic.gifin the code. No action required.apps/desktop/src/components/main/body/index.tsx (3)
1-2: LGTM! Import consolidation aligns with UI standardization.The import changes replace local/alternative components with standardized UI components from
@hypr/uiand utilities from@hypr/utils, which is consistent with the broader UI polish objectives of this PR.
374-385: Simplified mod+n behavior looks correct.The updated logic creates a note in the current tab when it's empty, otherwise creates a new note. The dependency array correctly includes
currentTab,newNote, andnewNoteCurrent.
394-403: Verify that removing app window close is intentional.The mod+w shortcut no longer closes the application window when the last tab is closed. This is a user-facing behavior change that some users might find unexpected if they're accustomed to closing the app with mod+w.
Please confirm this is the intended behavior.
apps/desktop/src/components/main/body/search.tsx (2)
56-72: LGTM! Button component provides better consistency.The refactor replaces a native button with the standardized
Buttoncomponent and updates icon sizing to use Tailwind'ssize-4utility class. This aligns with the PR's UI polish objectives and improves consistency across the codebase.
74-92: Responsive width logic enhances narrow-screen UX.The responsive behavior ensures the search input is appropriately sized on both wide and narrow screens. On narrow screens, the search bar maintains full width when expanded, while on wide screens it dynamically adjusts based on focus state.
apps/desktop/src/components/main/body/shared.tsx (4)
45-45: LGTM! Clean hover state implementation.The hover state tracking using a wrapper div is a good approach for managing the tab item's hover effects.
Also applies to: 68-72
73-91: InteractiveButton refactor maintains functionality with enhanced structure.The refactored structure uses the
asChildpattern and adds transition classes for smooth hover effects. The border and color logic for active/selected states is correctly preserved.
92-144: Well-implemented hover-driven close button overlay.The overlay logic smoothly transitions between showing the tab icon/indicator and the close button on hover. Key aspects:
- Correct use of
stopPropagationprevents unintended tab selection when clicking close- Smooth opacity transitions between normal and hover states
- Different visual treatments for active vs inactive tabs enhance clarity
The nested interactive structure is acceptable here since event propagation is properly managed.
147-154: Keyboard shortcut positioning updated correctly.The shortcut badge positioning and styling adapt to the new layout, with
pointer-events-noneensuring it doesn't interfere with interactions.apps/desktop/src/components/onboarding/permissions.tsx (2)
1-28: LGTM!The imports, hook usage, and permission status logic are well-structured and correct.
130-134: LGTM!The continue button correctly enforces that all permissions must be granted before proceeding, and provides clear feedback to the user about what's needed.
apps/desktop/src/components/settings/memory/custom-vocabulary.tsx (2)
50-55: LGTM! Clean header simplification.The streamlined header with just title and description improves clarity and aligns well with the UI polish objectives.
64-90: LGTM! Search UI enhancements improve usability.The addition of the Search icon and styling updates create a more polished search experience. The Add button now has more visual emphasis without the ghost variant.
apps/desktop/src/components/settings/memory/custom-vocabulary.tsx
Outdated
Show resolved
Hide resolved
coderabbit
ComputelessComputer
force-pushed
the
john-desktop-wip
branch
from
1345b59 to
593bbc1
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
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/components/onboarding/welcome.tsx (1)
37-49: Remove commented code instead of leaving it in the codebase.Commented-out code should be removed entirely. If this functionality might be needed in the future, rely on git history to retrieve it or create an issue to track the potential feature restoration.
Apply this diff to remove the commented code:
- { - /*<div - className={cn([ - "flex flex-row items-center gap-1", - "text-neutral-400 transition-colors hover:text-neutral-800", - ])} - > - <button className="text-sm underline" onClick={() => onNext({ local: true })}> - Or proceed without an account - </button> - <CircleQuestionMarkIcon className="h-4 w-4 cursor-help" /> - </div>*/ - }apps/desktop/src/components/settings/ai/llm/select.tsx (1)
192-192: Remove debug logging of sensitive data.The
console.logstatement logs the API key, which could expose sensitive credentials in production. Remove this debug statement before merging.Apply this diff:
- console.log(provider.id, baseUrl, apiKey);
♻️ Duplicate comments (9)
apps/desktop/src/components/onboarding/permissions.tsx (7)
33-125: Consider extracting repeated permission blocks into a reusable component.All three permission blocks follow an identical pattern. Extracting a
PermissionBlockcomponent would reduce duplication and improve maintainability.
34-47: Fix layout:gap-2requires flex or grid container.The
gap-2utility on line 34 has no effect because the div isn't usingdisplay: flexordisplay: grid.Apply this diff to fix the layout:
- <div className="gap-2"> + <div className="flex flex-col gap-2">
48-58: Addaria-labelfor screen reader accessibility.The icon-only button should include an
aria-labelto ensure screen readers can convey the button's purpose to users.Apply this diff:
<Button variant={micPermissionStatus.data === "authorized" ? "outline" : "default"} size="icon" onClick={handleMicPermissionAction} disabled={micPermission.isPending || micPermissionStatus.data === "authorized"} className={cn(["size-8", micPermissionStatus.data === "authorized" && "bg-stone-100 text-stone-800"])} + aria-label={micPermissionStatus.data === "authorized" ? "Microphone permission granted" : "Request microphone permission"} >
62-77: Fix layout:gap-2requires flex or grid container.Same issue as the Microphone block—the
gap-2utility has no effect without flex or grid.Apply this diff:
- <div className="gap-2"> + <div className="flex flex-col gap-2">
78-91: Addaria-labelfor screen reader accessibility.The icon-only button needs an
aria-label.Apply this diff:
<Button variant={systemAudioPermissionStatus.data === "authorized" ? "outline" : "default"} size="icon" onClick={handleSystemAudioPermissionAction} disabled={systemAudioPermission.isPending || systemAudioPermissionStatus.data === "authorized"} className={cn([ "size-8", systemAudioPermissionStatus.data === "authorized" && "bg-stone-100 text-stone-800", ])} + aria-label={systemAudioPermissionStatus.data === "authorized" ? "System audio permission granted" : "Request system audio permission"} >
95-110: Fix layout:gap-2requires flex or grid container.Same layout issue—
gap-2has no effect without flex or grid.Apply this diff:
- <div className="gap-2"> + <div className="flex flex-col gap-2">
111-124: Addaria-labelfor screen reader accessibility.The icon-only button needs an
aria-label.Apply this diff:
<Button variant={accessibilityPermissionStatus.data === "authorized" ? "outline" : "default"} size="icon" onClick={handleAccessibilityPermissionAction} disabled={accessibilityPermission.isPending || accessibilityPermissionStatus.data === "authorized"} className={cn([ "size-8", accessibilityPermissionStatus.data === "authorized" && "bg-stone-100 text-stone-800", ])} + aria-label={accessibilityPermissionStatus.data === "authorized" ? "Accessibility permission granted" : "Request accessibility permission"} >apps/desktop/src/components/settings/memory/custom-vocabulary.tsx (1)
100-116: Performance issue: O(n²) complexity from findIndex in map loop.This performance concern was already identified in a previous review. The
findIndexcall on line 101 inside themaploop creates quadratic time complexity. Please refer to the earlier review comment for the detailed solution using a precomputed index map.apps/desktop/src/components/settings/ai/stt/select.tsx (1)
70-83: Guard the deferred model auto-fill.The deferred
setTimeoutcan still run after the user flips to another provider (e.g., Custom), so we end up persisting a Deepgram model while the selected provider is now Custom. That breaks the configuration flow and matches the race we previously chased. Please guard the timeout with the provider that scheduled it (or remove the defer entirely) so stale callbacks bail out.- if (availableModels.length > 0) { - setTimeout(() => { - form.setFieldValue("model", availableModels[0].id); - }, 0); - } + if (availableModels.length > 0) { + const [firstModel] = availableModels; + setTimeout(() => { + if (form.getFieldValue("provider") !== providerId) { + return; + } + form.setFieldValue("model", firstModel.id); + }, 0); + }
🧹 Nitpick comments (8)
apps/desktop/src/components/main/body/search.tsx (2)
78-78: Consider passinghasSpaceas a prop to avoid duplicate media queries.The
hasSpacemedia query is already computed in the parentSearchcomponent (line 13). Computing it again here creates a redundant subscription. Consider passing it as a prop toExpandedSearchinstead.Apply this diff to accept
hasSpaceas a prop:-function ExpandedSearch({ onFocus, onBlur }: { onFocus?: () => void; onBlur?: () => void }) { +function ExpandedSearch({ onFocus, onBlur, hasSpace }: { onFocus?: () => void; onBlur?: () => void; hasSpace: boolean }) { const { query, setQuery, isSearching, isIndexing, inputRef } = useSearch(); const [isFocused, setIsFocused] = useState(false); const isCmdPressed = useCmdKeyPressed(); - const hasSpace = useMediaQuery("(min-width: 900px)");And update the call site:
if (shouldShowExpanded) { - return <ExpandedSearch onFocus={handleExpandedFocus} onBlur={handleExpandedBlur} />; + return <ExpandedSearch onFocus={handleExpandedFocus} onBlur={handleExpandedBlur} hasSpace={hasSpace} />; }
95-97: Consider usingsize-4for icon consistency.The icons here still use
h-4 w-4classes, whileCollapsedSearchwas updated to use the more concisesize-4class (lines 68-69). For consistency, consider updating these icons to match.Apply this diff:
{showLoading - ? <Loader2Icon className={cn(["h-4 w-4 absolute left-3 text-neutral-400 animate-spin"])} /> - : <SearchIcon className={cn(["h-4 w-4 absolute left-3 text-neutral-400"])} />} + ? <Loader2Icon className={cn(["size-4 absolute left-3 text-neutral-400 animate-spin"])} /> + : <SearchIcon className={cn(["size-4 absolute left-3 text-neutral-400"])} />}apps/desktop/src/components/main/body/shared.tsx (1)
92-146: Consider extracting the icon/close button logic into sub-components.The nested conditional rendering for the icon and close button overlay is complex and spans many lines. Extracting this into smaller, focused components would improve readability and maintainability.
For example, you could create:
TabIconcomponent for the icon display logicTabCloseButtoncomponent for the close button overlayapps/desktop/src/components/settings/general/permissions.tsx (1)
38-72: Consider adding accessible labels for icons.The AlertCircleIcon (line 42) and Check icon (line 66) don't have accessible text, which could impact screen reader users.
Apply this diff to add accessibility:
- {!isAuthorized && <AlertCircleIcon className="size-4" />} + {!isAuthorized && <AlertCircleIcon className="size-4" aria-label="Warning" />}- {isAuthorized ? <Check size={16} /> : null} + {isAuthorized ? <Check size={16} aria-label="Granted" /> : null}apps/desktop/src/components/settings/template/shared.tsx (1)
49-56: LGTM with a minor note.The targets chip rendering is well-implemented with appropriate spacing and responsive layout. The optional chaining handles the null/undefined array correctly.
Note: While React gracefully handles null/undefined elements in the map, consider adding
target?.optional chaining for consistency with the filtering logic inremote.tsx(once that's fixed).If desired, apply this diff for defensive consistency:
<div className="flex items-center gap-2 flex-wrap"> - {targets?.map((target, index) => ( + {targets?.map((target, index) => target && ( <span key={index} className="text-xs text-neutral-600 bg-neutral-100 px-2 py-0.5 rounded"> {target} </span> ))} </div>apps/desktop/src/components/chat/input.tsx (1)
46-59: TODO: Implement stub handlersThe file attachment, screenshot, and voice input handlers are currently stubs with console.log placeholders. These features are wired to UI buttons but not yet functional.
Would you like me to help implement these handlers or open issues to track their completion?
apps/desktop/src/components/settings/ai/llm/select.tsx (2)
239-277: Consider usinguseMemofor status computation.The IIFE pattern works but recomputes on every render. Using
useMemowould make the intent clearer and follow React best practices by memoizing the computation based on dependencies.- const { status, message, textColor } = (() => { + const { status, message, textColor } = useMemo(() => { if (!model) { return { status: "No model configured", message: "Please configure a provider and model", textColor: "text-red-600", }; } if (text.isPending) { return { status: "Checking connection", message: "Testing model connection", textColor: "text-yellow-600", }; } if (text.isError) { return { status: "Connection failed", message: text.error?.message || "Unable to connect to the model", textColor: "text-red-600", }; } if (text.isSuccess) { return { status: "Connected!", message: "Model is ready to use", textColor: "text-green-600", }; } return { status: "Unknown status", message: "Connection status unknown", textColor: "text-red-600", }; - })(); + }, [model, text.isPending, text.isError, text.isSuccess, text.error]);
287-293: Loading animation uses inline styles.The dots animation works correctly but uses inline
styleattributes foranimationDelay, which is inconsistent with the Tailwind-first approach used throughout the file. Consider using Tailwind's animation utilities or defining a CSS animation if you want to maintain consistency.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (17)
apps/desktop/src-tauri/icons/nightly/128x128.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/128x128@2x.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/16.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/32x32.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/64x64.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square107x107Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square142x142Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square150x150Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square284x284Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square30x30Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square310x310Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square44x44Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square71x71Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/Square89x89Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/StoreLogo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/nightly/icon.icois excluded by!**/*.icoapps/desktop/src-tauri/icons/nightly/icon.pngis excluded by!**/*.png
📒 Files selected for processing (36)
apps/desktop/src/components/chat/body/empty.tsx(1 hunks)apps/desktop/src/components/chat/body/index.tsx(3 hunks)apps/desktop/src/components/chat/header.tsx(1 hunks)apps/desktop/src/components/chat/index.tsx(1 hunks)apps/desktop/src/components/chat/input.tsx(3 hunks)apps/desktop/src/components/chat/interactive.tsx(1 hunks)apps/desktop/src/components/chat/trigger.tsx(1 hunks)apps/desktop/src/components/chat/view.tsx(1 hunks)apps/desktop/src/components/main/body/index.tsx(5 hunks)apps/desktop/src/components/main/body/search.tsx(2 hunks)apps/desktop/src/components/main/body/sessions/floating/listen.tsx(1 hunks)apps/desktop/src/components/main/body/shared.tsx(2 hunks)apps/desktop/src/components/main/sidebar/banner/registry.tsx(3 hunks)apps/desktop/src/components/onboarding/calendar.tsx(0 hunks)apps/desktop/src/components/onboarding/permissions.tsx(2 hunks)apps/desktop/src/components/onboarding/shared.tsx(2 hunks)apps/desktop/src/components/onboarding/welcome.tsx(3 hunks)apps/desktop/src/components/settings/ai/llm/configure.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/index.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/no-model-banner.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/select.tsx(3 hunks)apps/desktop/src/components/settings/ai/shared/model-combobox.tsx(2 hunks)apps/desktop/src/components/settings/ai/stt/configure.tsx(2 hunks)apps/desktop/src/components/settings/ai/stt/index.tsx(1 hunks)apps/desktop/src/components/settings/ai/stt/no-model-banner.tsx(1 hunks)apps/desktop/src/components/settings/ai/stt/select.tsx(3 hunks)apps/desktop/src/components/settings/ai/stt/shared.tsx(1 hunks)apps/desktop/src/components/settings/general/permissions.tsx(2 hunks)apps/desktop/src/components/settings/memory/custom-vocabulary.tsx(8 hunks)apps/desktop/src/components/settings/template/remote.tsx(2 hunks)apps/desktop/src/components/settings/template/shared.tsx(2 hunks)apps/desktop/src/components/shared/permission-row.tsx(0 hunks)apps/desktop/src/routes/app.tsx(1 hunks)apps/desktop/src/routes/app/main/_layout.index.tsx(1 hunks)apps/desktop/src/routes/app/onboarding/index.tsx(1 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (2)
- apps/desktop/src/components/onboarding/calendar.tsx
- apps/desktop/src/components/shared/permission-row.tsx
🚧 Files skipped from review as they are similar to previous changes (15)
- apps/desktop/src/components/settings/ai/stt/index.tsx
- apps/desktop/src/components/chat/body/empty.tsx
- apps/desktop/src/components/chat/interactive.tsx
- apps/desktop/src/routes/app/main/_layout.index.tsx
- apps/desktop/src/components/settings/ai/stt/shared.tsx
- apps/desktop/src/components/main/body/sessions/floating/listen.tsx
- apps/desktop/src/components/settings/ai/llm/configure.tsx
- apps/desktop/src/components/onboarding/shared.tsx
- apps/desktop/src/components/chat/index.tsx
- apps/desktop/src/components/chat/body/index.tsx
- apps/desktop/src/components/chat/header.tsx
- apps/desktop/src/routes/app/onboarding/index.tsx
- package.json
- apps/desktop/src/components/main/body/index.tsx
- apps/desktop/src/routes/app.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-12T01:18:52.105Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: packages/nango/.cursor/rules/nango.mdc:0-0
Timestamp: 2025-10-12T01:18:52.105Z
Learning: Applies to packages/nango/**/models.ts : Do not edit the models.ts file; it is auto-generated at compilation time
Applied to files:
apps/desktop/src/components/settings/ai/llm/index.tsx
🧬 Code graph analysis (11)
apps/desktop/src/components/settings/ai/stt/no-model-banner.tsx (4)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/config/use-config.ts (1)
useConfigValues(32-53)apps/desktop/src/components/settings/ai/stt/shared.tsx (1)
sttModelQueries(248-275)apps/desktop/src/store/tinybase/schema-internal.ts (1)
AIProviderStorage(40-40)
apps/desktop/src/components/main/body/search.tsx (3)
apps/desktop/src/contexts/search/ui.tsx (1)
useSearch(236-242)apps/desktop/src/hooks/useCmdKeyPressed.tsx (1)
useCmdKeyPressed(3-35)packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/settings/ai/llm/no-model-banner.tsx (3)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/auth.tsx (1)
useAuth(177-185)apps/desktop/src/config/use-config.ts (1)
useConfigValues(32-53)
apps/desktop/src/components/settings/general/permissions.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/settings/ai/stt/select.tsx (2)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/components/settings/ai/stt/shared.tsx (3)
ProviderId(8-8)PROVIDERS(46-104)displayModelId(22-44)
apps/desktop/src/components/chat/input.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/onboarding/permissions.tsx (2)
apps/desktop/src/components/onboarding/shared.tsx (1)
OnboardingContainer(21-43)packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/main/body/shared.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/settings/ai/llm/select.tsx (3)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/components/settings/ai/llm/shared.tsx (1)
PROVIDERS(6-63)apps/desktop/src/components/settings/ai/shared/model-combobox.tsx (1)
ModelCombobox(45-233)
apps/desktop/src/components/chat/view.tsx (1)
apps/desktop/src/components/chat/input.tsx (1)
ChatMessageInput(12-135)
apps/desktop/src/components/settings/ai/llm/index.tsx (1)
apps/desktop/src/components/settings/ai/llm/no-model-banner.tsx (1)
NoModelBanner(11-31)
🔇 Additional comments (27)
apps/desktop/src/components/settings/ai/shared/model-combobox.tsx (1)
2-2: LGTM! Icon change improves semantic clarity.The switch from
ChevronsUpDowntoChevronDownis more semantically appropriate for a combobox dropdown. The negative right margin (-mr-1) tightens the spacing between the icon and the button edge, which is a common design pattern for trailing icons in button components.Also applies to: 103-103
apps/desktop/src/components/onboarding/welcome.tsx (2)
12-12: LGTM: Clean refactor to React fragment.Removing the unnecessary wrapper div results in a cleaner DOM structure with no functional impact.
Also applies to: 50-50
31-32: LGTM: Button styling update.The larger size and full-width styling align with the UI polish objectives of this PR.
apps/desktop/src/components/main/sidebar/banner/registry.tsx (3)
1-1: LGTM! Improved icon semantics.The new icon choices are more semantically appropriate:
AudioLinesIconclearly represents audio/speech functionality for STT, andSparklesIconis a well-established choice for AI features.
32-38: LGTM! Clear and user-friendly STT banner.The updated content is more specific and informative. The title clearly indicates STT model configuration, the description explains why it's needed, and the action label "Go to settings" is concise and consistent.
46-52: LGTM! Clear and user-friendly LLM banner.The updated content effectively communicates the purpose of the LLM. The description clearly explains that it powers summarization and chat features, and the action label maintains consistency with the STT banner.
apps/desktop/src/components/main/body/search.tsx (1)
61-70: LGTM: Button component migration.The refactor to use the
Buttoncomponent with consistent icon sizing improves maintainability and aligns with the design system.apps/desktop/src/components/onboarding/permissions.tsx (3)
2-4: Imports look good.The
cnutility and new icon imports are appropriate for the status-aware permission blocks.
31-31: Title simplification improves clarity.The updated title is more concise and direct.
128-130: Excellent UX with state-aware button text.The Continue button provides clear feedback by showing why it's disabled and when users can proceed.
apps/desktop/src/components/settings/memory/custom-vocabulary.tsx (5)
1-8: LGTM: Import updates support UI enhancements.The new imports (
Button,cn,Searchicon) are properly utilized throughout the component for the UI polish improvements.
50-55: LGTM: Header simplification improves clarity.The streamlined header with clearer description text aligns well with the PR's UI polish objectives.
64-88: LGTM: Search UI enhancements improve user experience.The addition of the Search icon, background color update, and button styling refinements create a more polished interface.
126-146: LGTM: Props interface correctly extended.The
itemNumberandisSearchingprops are appropriately typed and support the new item numbering feature.
194-222: LGTM: Item numbering implementation enhances usability.The item number display with search-aware visibility provides clear visual ordering. Hiding numbers during search (line 195) is a good UX choice since filtered results may show non-consecutive indices.
apps/desktop/src/components/settings/general/permissions.tsx (2)
1-7: LGTM!The imports are properly organized and all necessary for the internal PermissionRow component implementation.
92-112: LGTM!The simplified permission titles are more concise while maintaining clarity, and all PermissionRow props are correctly passed.
apps/desktop/src/components/settings/template/remote.tsx (1)
32-33: LGTM!The new props are passed correctly to
TemplateCard, maintaining consistency with the extended template data structure.apps/desktop/src/components/settings/template/shared.tsx (2)
6-7: LGTM!The new props are properly typed with optional and nullable types, maintaining consistency with the existing prop definitions.
Also applies to: 12-13
41-48: LGTM!The inline category badge is well-implemented with appropriate layout changes and conditional rendering. The visual hierarchy is clear with the category displayed in a muted color.
apps/desktop/src/components/chat/trigger.tsx (1)
3-4: LGTM: Import reorderingThe import reordering is purely stylistic with no functional impact.
apps/desktop/src/components/chat/view.tsx (1)
107-110: LGTM: Model configuration awarenessThe changes cleanly thread model configuration state through the chat UI:
isModelConfigured={!!model}properly converts the model to a boolean for downstream components- The simplified
disabledlogic clearly gates input when either the model is missing or the chat status is not readyapps/desktop/src/components/chat/input.tsx (3)
41-44: Good improvement: Conditional focus behaviorGating the editor focus on the
disabledstate prevents stealing focus when the input is unavailable, improving the user experience.
154-159: LGTM: Well-implemented placeholder componentThe
ChatPlaceholderfunction correctly implements thePlaceholderFunctioninterface, properly checks for the first paragraph position, and provides friendly, contextually appropriate placeholder text.
77-131: LGTM: Clean multi-action button layoutThe new layout organizes actions logically (media operations on the left, communication on the right) with consistent styling and proper disabled state handling across all buttons.
apps/desktop/src/components/settings/ai/llm/index.tsx (1)
8-8: Banner placement looks good.Mounting
NoModelBannerahead of the selector keeps the warning visible without disrupting the existing layout.apps/desktop/src/components/settings/ai/llm/select.tsx (1)
84-86: Race condition fix correctly implemented!The guard check ensures that the model is only set if the provider hasn't changed since the fetch began. This prevents the invalid provider+model pairing that was flagged in the previous review.
ComputelessComputer
force-pushed
the
john-desktop-wip
branch
from
593bbc1 to
f2ecd16
Compare
ComputelessComputer
force-pushed
the
john-desktop-wip
branch
from
f2ecd16 to
0c1e4fe
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/desktop/src/components/main/body/shared.tsx (1)
115-132: Add accessible label and button type.The close button is still missing an
aria-labelattribute (flagged in previous reviews) and should includetype="button"to prevent unintended form submissions.Apply this diff:
<button + type="button" + aria-label="Close tab" onClick={(e) => { e.stopPropagation(); handleCloseThis(); }} className={cn(apps/desktop/src/components/settings/general/permissions.tsx (1)
26-36: Previous feedback addressed; minor terminology inconsistency remains.The casual and awkward phrasing from the previous review has been resolved with professional messaging. However, there's a minor inconsistency: line 27 uses "Permission granted" while line 33 uses "Access Granted" for the same authorized state.
Consider unifying the terminology:
const buttonText = isAuthorized - ? "Access Granted" + ? "Granted" : isDenied ? "Open Settings" : "Enable";
🧹 Nitpick comments (1)
apps/desktop/src/components/main/sidebar/profile/ota/index.tsx (1)
53-55: Array wrapping is redundant but consistent.Since
cnuses rest parameters (...inputs), wrapping classes in an array is unnecessary—you can pass them directly as separate arguments. However, if this array pattern is intentional for codebase consistency, it's fine.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (36)
packages/ui/src/components/ui/accordion.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/avatar.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/badge.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/bottom-sheet.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/breadcrumb.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/button-group.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/button.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/card.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/carousel.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/checkbox.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/command.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/context-menu.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/dialog.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/dropdown-menu.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/form.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/hover-card.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/input-group.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/kbd.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/label.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/marquee.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/modal.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/popover.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/progress.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/progressive-blur.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/radio-group.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/select.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/separator.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/slider.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/spinner.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/splash.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/switch.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/tabs.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/text-animate.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/textarea.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/tooltip.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/typewriter.tsxis excluded by!packages/ui/src/components/ui/**
📒 Files selected for processing (15)
apps/desktop/src/components/main/body/shared.tsx(2 hunks)apps/desktop/src/components/main/sidebar/profile/index.tsx(2 hunks)apps/desktop/src/components/main/sidebar/profile/ota/index.tsx(3 hunks)apps/desktop/src/components/settings/billing.tsx(1 hunks)apps/desktop/src/components/settings/general/permissions.tsx(2 hunks)apps/desktop/src/components/settings/template/remote.tsx(2 hunks)apps/web/src/components/mock-window.tsx(1 hunks)apps/web/src/components/social-card.tsx(1 hunks)apps/web/src/routes/_view/index.tsx(2 hunks)apps/web/src/routes/_view/product/ai-notetaking.tsx(11 hunks)packages/ui/src/docs/accordion.tsx(1 hunks)packages/ui/src/docs/callouts.tsx(3 hunks)packages/ui/src/docs/card.tsx(1 hunks)packages/ui/src/docs/columns.tsx(1 hunks)packages/ui/src/docs/steps.tsx(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- apps/web/src/routes/_view/index.tsx
- apps/web/src/routes/_view/product/ai-notetaking.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/components/settings/template/remote.tsx
🧰 Additional context used
🧬 Code graph analysis (7)
apps/web/src/components/social-card.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/main/body/shared.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/settings/billing.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/main/sidebar/profile/ota/index.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/settings/general/permissions.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
packages/ui/src/docs/steps.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/main/sidebar/profile/index.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
🔇 Additional comments (23)
apps/desktop/src/components/main/sidebar/profile/ota/index.tsx (2)
3-5: LGTM! Improved import organization.The import grouping improves readability by separating utility imports from icon imports.
162-167: ****Same array wrapping pattern as line 53-55, already addressed.
apps/desktop/src/components/settings/billing.tsx (1)
27-27: LGTM! Consistency improvement.Wrapping the conditional className in an array standardizes the
cn()call pattern to match the usage elsewhere in this file (lines 65-71) and aligns with the broader refactoring effort described in the PR.apps/web/src/components/social-card.tsx (1)
79-79: LGTM!The className array format is functionally equivalent to the previous implementation and maintains consistency with the broader refactor pattern across the PR.
apps/web/src/components/mock-window.tsx (1)
18-21: LGTM!The array-based className composition preserves the conditional mobile/desktop styling logic while aligning with the project-wide refactor.
packages/ui/src/docs/steps.tsx (2)
11-11: LGTM!The className composition for the Step component correctly uses the array format while preserving all styling logic.
39-42: LGTM!The className composition for the Steps component correctly uses the array format while preserving all styling logic.
packages/ui/src/docs/card.tsx (1)
14-14: LGTM!The className composition correctly merges the default Card styles with the optional className prop using the array format.
packages/ui/src/docs/columns.tsx (1)
13-19: LGTM!The className composition correctly handles conditional grid layouts using the array format. The boolean short-circuit expressions work properly within the array, as
clsxfilters out falsy values.packages/ui/src/docs/accordion.tsx (1)
28-28: LGTM!The className composition for the Accordion root element correctly merges border styles with the optional className prop using the array format.
packages/ui/src/docs/callouts.tsx (3)
13-16: LGTM!The Info callout component correctly uses the array format for className composition while preserving the stone color scheme.
31-34: LGTM!The Note callout component correctly uses the array format for className composition while preserving the neutral color scheme.
49-52: LGTM!The Tip callout component correctly uses the array format for className composition while preserving the stone color scheme.
apps/desktop/src/components/main/sidebar/profile/index.tsx (3)
253-260: LGTM!The ProfileButton's root element className composition correctly uses the array format, preserving all layout, transition, and conditional styling based on the
isExpandedstate.
266-273: LGTM!The avatar container's className composition correctly uses the array format, preserving the gradient background, border styling, and transition effects.
288-292: LGTM!The chevron icon's className composition correctly uses the array format with conditional rotation and color changes based on the
isExpandedstate.apps/desktop/src/components/main/body/shared.tsx (4)
45-45: LGTM!The hover state is correctly initialized and follows standard React patterns.
68-72: LGTM!The hover tracking wrapper is correctly implemented with proper event handlers and height preservation.
73-91: LGTM!The InteractiveButton refactoring maintains all necessary props and handlers while adding smooth transitions and updated styling. The
asChildpattern and class composition usingcnare implemented correctly.
137-144: LGTM!The keyboard shortcut display logic is correctly preserved and properly positioned with appropriate color states.
apps/desktop/src/components/settings/general/permissions.tsx (3)
1-8: LGTM!The imports are well-organized and include all necessary dependencies for the refactored component.
51-70: LGTM!The button logic correctly handles all permission states with appropriate visual feedback (spinner during requests, check icon when granted) and interaction states (disabled when already granted or pending).
75-116: LGTM!The Permissions component is well-structured with clear, user-friendly descriptions for each permission that explain their purpose in the context of meeting functionality.
| <div className="flex items-center gap-2 text-sm flex-1 min-w-0"> | ||
| <div className="flex-shrink-0 relative w-4 h-4"> | ||
| <div | ||
| className={cn( | ||
| "absolute inset-0 flex items-center justify-center transition-opacity duration-200", | ||
| isHovered ? "opacity-0" : "opacity-100", | ||
| )} | ||
| > | ||
| {active | ||
| ? ( | ||
| <div className="relative size-2"> | ||
| <div className="absolute inset-0 rounded-full bg-red-600"></div> | ||
| <div className="absolute inset-0 rounded-full bg-red-300 animate-ping"></div> | ||
| </div> | ||
| ) | ||
| : icon} | ||
| </div> | ||
| <div | ||
| className={cn( | ||
| "absolute inset-0 flex items-center justify-center transition-opacity duration-200", | ||
| isHovered ? "opacity-100" : "opacity-0", | ||
| )} | ||
| > | ||
| <button | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| handleCloseThis(); | ||
| }} | ||
| className={cn( | ||
| [ | ||
| "flex items-center justify-center transition-colors", | ||
| active | ||
| ? "text-red-600 hover:text-red-700" | ||
| : selected | ||
| ? "text-neutral-700 hover:text-neutral-900" | ||
| : "text-neutral-500 hover:text-neutral-700", | ||
| ], | ||
| )} | ||
| > | ||
| <X size={16} /> | ||
| </button> | ||
| </div> | ||
| </div> | ||
| <span className="truncate">{title}</span> | ||
| </div> |
There was a problem hiding this comment.
Consider keyboard accessibility for the hover-only close button.
The close button is only visible on hover, which creates accessibility barriers for keyboard-only users. When navigating with Tab, users may focus on an invisible button or be unable to access the close functionality at all.
Consider one of these approaches:
- Always show the close button when the tab has focus, not just on hover
- Add focus-visible state to show the button:
isHovered || hasFocus - Use
pointer-events: noneorinerton the hidden button to prevent focus when opacity is 0
Example implementation using focus state:
const [isFocused, setIsFocused] = useState(false);
// In the wrapper div:
<div
onMouseEnter={() => setIsHovered(true)}
onMouseLeave={() => setIsHovered(false)}
onFocus={() => setIsFocused(true)}
onBlur={() => setIsFocused(false)}
className="h-full"
>
{/* ... */}
<div
className={cn(
"absolute inset-0 flex items-center justify-center transition-opacity duration-200",
isHovered || isFocused ? "opacity-100" : "opacity-0 pointer-events-none",
)}
>🤖 Prompt for AI Agents
In apps/desktop/src/components/main/body/shared.tsx around lines 92-136, the
close button is only revealed on hover which makes it unreachable for keyboard
users; add keyboard accessibility by tracking focus and using that to reveal the
button (e.g., add a isFocused state with onFocus/onBlur on the wrapper and
change the close-button container class to use isHovered || isFocused), and
ensure the hidden button cannot be tabbed when not visible (add
pointer-events-none or aria-hidden/inert when opacity is 0) so it doesn't
receive focus while invisible; keep the existing onClick stopPropagation
behavior.
| <div className={cn(["flex items-center gap-2 mb-1", !isAuthorized && "text-red-500"])}> | ||
| {!isAuthorized && <AlertCircleIcon className="size-4" />} |
There was a problem hiding this comment.
Error styling applies too broadly.
The red error styling and alert icon display for any non-authorized state, including "prompt" (not yet requested) and undefined (loading). This could confuse users by showing error indicators when no error has occurred.
Apply this diff to show error styling only when permission is explicitly denied:
- <div className={cn(["flex items-center gap-2 mb-1", !isAuthorized && "text-red-500"])}>
- {!isAuthorized && <AlertCircleIcon className="size-4" />}
+ <div className={cn(["flex items-center gap-2 mb-1", isDenied && "text-red-500"])}>
+ {isDenied && <AlertCircleIcon className="size-4" />}
<h3 className="text-sm font-medium">📝 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.
| <div className={cn(["flex items-center gap-2 mb-1", !isAuthorized && "text-red-500"])}> | |
| {!isAuthorized && <AlertCircleIcon className="size-4" />} | |
| <div className={cn(["flex items-center gap-2 mb-1", isDenied && "text-red-500"])}> | |
| {isDenied && <AlertCircleIcon className="size-4" />} |
🤖 Prompt for AI Agents
In apps/desktop/src/components/settings/general/permissions.tsx around lines
41-42, the red error styling and AlertCircleIcon are currently shown for any
falsy isAuthorized (including "prompt" or undefined); change the conditions so
the error styles and icon render only when permission is explicitly denied
(isAuthorized === false). Update the className conditional to apply
"text-red-500" only when isAuthorized === false and render the AlertCircleIcon
only when isAuthorized === false, leaving prompt/undefined states unstyled and
without the error icon.
ef1c124 to
ca8e7f4
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/components/main/sidebar/profile/index.tsx (1)
277-277: Avoid empty string for imagesrc.When
profile.datais nullish, the empty string fallback causes the browser to request the current page URL as an image, resulting in a failed request and console error.Consider one of these approaches:
Option 1: Conditional rendering
- <img - src={profile.data ?? ""} - alt="Profile" - className="h-full w-full rounded-full" - /> + {profile.data && ( + <img + src={profile.data} + alt="Profile" + className="h-full w-full rounded-full" + /> + )}Option 2: Use a placeholder avatar
- <img - src={profile.data ?? ""} - alt="Profile" - className="h-full w-full rounded-full" - /> + <img + src={profile.data ?? "/path/to/default-avatar.png"} + alt="Profile" + className="h-full w-full rounded-full" + />
♻️ Duplicate comments (3)
apps/desktop/src/components/main/body/shared.tsx (2)
68-146: Keyboard accessibility issue remains unresolved.The close button is only visible on hover, which prevents keyboard-only users from accessing the close functionality. This issue was previously flagged but has not been addressed.
Consider tracking focus state in addition to hover:
- Add
isFocusedstate withonFocus/onBlurhandlers- Show close button when
isHovered || isFocused- Add
pointer-events-noneto the hidden close button container (line 109-113) to prevent focus when invisibleExample implementation:
- const [isHovered, setIsHovered] = useState(false); + const [isHovered, setIsHovered] = useState(false); + const [isFocused, setIsFocused] = useState(false); // ... <div onMouseEnter={() => setIsHovered(true)} onMouseLeave={() => setIsHovered(false)} + onFocus={() => setIsFocused(true)} + onBlur={() => setIsFocused(false)} className="h-full" > {/* ... */} <div className={cn( "absolute inset-0 flex items-center justify-center transition-opacity duration-200", - isHovered ? "opacity-0" : "opacity-100", + isHovered || isFocused ? "opacity-0" : "opacity-100", )} > {/* ... */} <div className={cn( "absolute inset-0 flex items-center justify-center transition-opacity duration-200", - isHovered ? "opacity-100" : "opacity-0", + isHovered || isFocused ? "opacity-100" : "opacity-0 pointer-events-none", )} >
115-132: Missing accessible label on close button.The close button still lacks an
aria-labelattribute, preventing screen readers from announcing its purpose. This issue was previously raised and remains unresolved.Apply this diff to add the aria-label and explicit button type:
<button + type="button" + aria-label="Close tab" onClick={(e) => { e.stopPropagation(); handleCloseThis(); }} className={cn( [ "flex items-center justify-center transition-colors", active ? "text-red-600 hover:text-red-700" : selected ? "text-neutral-700 hover:text-neutral-900" : "text-neutral-500 hover:text-neutral-700", ], )} > <X size={16} /> </button>apps/desktop/src/components/settings/general/permissions.tsx (1)
41-42: Error styling still applies too broadly.The red error styling and alert icon display for any non-authorized state, including "prompt" (not yet requested) and undefined (loading). This was flagged in previous reviews but remains unresolved. Users see error indicators when no error has occurred.
Apply this diff to show error styling only when permission is explicitly denied:
- <div className={cn(["flex items-center gap-2 mb-1", !isAuthorized && "text-red-500"])}> - {!isAuthorized && <AlertCircleIcon className="size-4" />} + <div className={cn(["flex items-center gap-2 mb-1", isDenied && "text-red-500"])}> + {isDenied && <AlertCircleIcon className="size-4" />} <h3 className="text-sm font-medium">
🧹 Nitpick comments (5)
apps/desktop/src/components/settings/billing.tsx (1)
27-27: Unnecessary array wrapper for single className value.The array wrapper around the single boolean expression is redundant. The
cnfunction already accepts variadic arguments, so wrapping a single value in an array adds an extra level of nesting without benefit.Apply this diff to simplify:
- className={cn([index === 0 && "border-r border-neutral-200"])} + className={cn(index === 0 && "border-r border-neutral-200")}Note: Lines 65-71 demonstrate the appropriate use case for array arguments when grouping multiple class values together.
apps/desktop/src/components/main/sidebar/profile/index.tsx (1)
242-248: Consider adding error handling for avatar query.The query doesn't handle error or loading states. If
getAvatarUrl()fails orauthis undefined, the avatar silently falls back to an empty/broken state.Consider handling query states:
const profile = useQuery({ queryKey: ["profile"], + enabled: !!auth, queryFn: async () => { const avatarUrl = await auth?.getAvatarUrl(); return avatarUrl; }, }); + + // Then in the JSX, handle loading/error states: + // {profile.isLoading && <LoadingSpinner />} + // {profile.error && <DefaultAvatar />}apps/desktop/src/components/chat/body/empty.tsx (1)
15-18: Incomplete quick action handler.The
handleQuickActionfunction currently only logs the action. To make the quick actions functional, this should likely populate the chat input with the selected action text or directly send it as a message to the AI.Would you like me to help implement the complete handler? For example:
const handleQuickAction = (action: string) => { // Option 1: Send directly to chat onSendMessage?.(action); // Option 2: Populate input field // onSetInputValue?.(action); };This would require passing the appropriate callback prop to
ChatBodyEmpty.apps/desktop/src/components/onboarding/permissions.tsx (1)
24-27: Consider distinguishing "denied" from "prompt" states.The error styling (
text-red-500) and alert icon display for any non-authorized state, including "prompt" (not yet requested) and undefined (loading). In the onboarding context where all permissions are required, this may be intentional, but it differs from the settings screen approach (which only shows red for explicitly denied permissions).This could momentarily confuse users when permissions are still loading or haven't been requested yet.
If you want to match the settings behavior, apply this diff:
function PermissionBlock({ name, status, description, isPending, onAction }: PermissionBlockProps) { const isAuthorized = status === "authorized"; + const isDenied = status === "denied"; return ( <div className="flex items-center justify-between"> <div className="flex flex-col gap-2"> <div className={cn([ "flex items-center gap-2", - !isAuthorized ? "text-red-500" : "text-neutral-900", + isDenied ? "text-red-500" : "text-neutral-900", ])} > - {!isAuthorized && <AlertCircleIcon className="size-4" />} + {isDenied && <AlertCircleIcon className="size-4" />} <span className="text-base font-medium">{name}</span>apps/web/src/routes/_view/product/ai-notetaking.tsx (1)
74-78: LGTM! Consistentcn()usage across the file.The changes standardize all
cn()calls to pass a single array argument rather than multiple arguments. While this introduces an extra level of array nesting (sincecnuses a rest parameter),clsxhandles nested arrays correctly, so there are no functional issues.Optional note: If this array-wrapping pattern is now the codebase standard, consider updating the
cnfunction signature fromcn(...inputs: ClassValue[])tocn(inputs: ClassValue[])to better match the new calling convention and avoid unnecessary nesting.Also applies to: 333-333, 342-342, 351-351, 375-375, 385-385, 397-397, 411-411, 419-419, 625-629, 636-637, 644-645, 652-653, 663-667, 674-675, 682-683, 753-754, 761-762, 769-770, 782-783, 790-791, 995-999, 1092-1097
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (36)
packages/ui/src/components/ui/accordion.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/avatar.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/badge.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/bottom-sheet.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/breadcrumb.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/button-group.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/button.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/card.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/carousel.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/checkbox.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/command.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/context-menu.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/dialog.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/dropdown-menu.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/form.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/hover-card.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/input-group.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/kbd.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/label.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/marquee.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/modal.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/popover.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/progress.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/progressive-blur.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/radio-group.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/select.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/separator.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/slider.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/spinner.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/splash.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/switch.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/tabs.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/text-animate.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/textarea.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/tooltip.tsxis excluded by!packages/ui/src/components/ui/**packages/ui/src/components/ui/typewriter.tsxis excluded by!packages/ui/src/components/ui/**
📒 Files selected for processing (18)
apps/desktop/src/components/chat/body/empty.tsx(1 hunks)apps/desktop/src/components/main/body/shared.tsx(2 hunks)apps/desktop/src/components/main/sidebar/profile/index.tsx(2 hunks)apps/desktop/src/components/main/sidebar/profile/ota/index.tsx(3 hunks)apps/desktop/src/components/onboarding/permissions.tsx(2 hunks)apps/desktop/src/components/settings/billing.tsx(1 hunks)apps/desktop/src/components/settings/general/permissions.tsx(2 hunks)apps/desktop/src/components/settings/memory/custom-vocabulary.tsx(8 hunks)apps/desktop/src/components/settings/template/remote.tsx(2 hunks)apps/web/src/components/mock-window.tsx(1 hunks)apps/web/src/components/social-card.tsx(1 hunks)apps/web/src/routes/_view/index.tsx(2 hunks)apps/web/src/routes/_view/product/ai-notetaking.tsx(11 hunks)packages/ui/src/docs/accordion.tsx(1 hunks)packages/ui/src/docs/callouts.tsx(3 hunks)packages/ui/src/docs/card.tsx(1 hunks)packages/ui/src/docs/columns.tsx(1 hunks)packages/ui/src/docs/steps.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/ui/src/docs/callouts.tsx
- apps/web/src/components/mock-window.tsx
- packages/ui/src/docs/columns.tsx
- apps/web/src/routes/_view/index.tsx
- packages/ui/src/docs/accordion.tsx
- apps/web/src/components/social-card.tsx
- packages/ui/src/docs/steps.tsx
- packages/ui/src/docs/card.tsx
- apps/desktop/src/components/main/sidebar/profile/ota/index.tsx
🧰 Additional context used
🧬 Code graph analysis (7)
apps/web/src/routes/_view/product/ai-notetaking.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/onboarding/permissions.tsx (2)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/components/onboarding/shared.tsx (1)
OnboardingContainer(21-43)
apps/desktop/src/components/settings/billing.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/settings/memory/custom-vocabulary.tsx (2)
apps/desktop/src/components/main/body/search.tsx (1)
Search(12-54)packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/main/body/shared.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/settings/general/permissions.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/main/sidebar/profile/index.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
🔇 Additional comments (19)
apps/desktop/src/components/settings/template/remote.tsx (2)
32-33: LGTM—props align with updated TemplateCard signature.The addition of
categoryandtargetsprops correctly extends the template card rendering to display the new fields.
61-69: LGTM—defensive checks properly implemented.The filtering logic correctly handles nullable fields with optional chaining. The previous concern about null/undefined elements in the
targetsarray has been addressed withtarget?.toLowerCase()on line 66.apps/desktop/src/components/main/sidebar/profile/index.tsx (1)
253-260: LGTM! Array-based className composition improves readability.The refactor to use array arguments with
cn()is functionally equivalent and improves code formatting by enabling one class per line. This aligns with the broader refactor pattern across the PR.Also applies to: 266-274, 288-293
apps/desktop/src/components/chat/body/empty.tsx (4)
1-2: LGTM!The imports are correctly structured and used appropriately in the component.
4-7: LGTM!The function signature properly defines the
isModelConfiguredprop with appropriate TypeScript typing and default value. The settings navigation handler correctly uses the Windows commands API.
20-39: LGTM!The unconfigured state UI clearly communicates the need to set up a model and provides an accessible path to settings. The component structure and styling are well-implemented.
41-66: LGTM!The configured state UI provides a clean, accessible interface with the quick action buttons. The greeting text has been corrected (as noted in the past review), and the button implementation follows React best practices with proper keys and event handlers.
apps/desktop/src/components/settings/memory/custom-vocabulary.tsx (5)
1-2: LGTM! Clean import additions.The new imports for
Button,cn, andSearchicon are all utilized in the enhanced UI.Also applies to: 5-5
44-46: Excellent! O(n²) complexity issue resolved.The
itemIndexMapprecomputation eliminates the O(n²)findIndexcall inside the render loop, exactly as suggested in the previous review. The Map lookup on line 109 is now O(1).
54-59: LGTM! Clean UI polish.The header simplification, search icon addition, and subtle background color enhance the visual hierarchy and user experience without affecting functionality.
Also applies to: 68-68, 70-70, 90-91
104-119: Verify the non-null assertion safety.The code correctly uses
itemIndexMapfor O(1) lookups. The non-null assertion on line 109 (itemIndexMap.get(item.rowId)!) is likely safe sincefilteredItemsderives fromvocabItemsanditemIndexMapis built from the same source. However, if there's ever a timing issue or stale closure, this could throw.Consider optional chaining with a fallback for defensive coding:
- itemNumber={itemIndexMap.get(item.rowId)!} + itemNumber={itemIndexMap.get(item.rowId) ?? 0}
197-225: LGTM! Well-structured item numbering.The item number display with conditional visibility (
isSearching && "invisible") is a nice UX touch. The layout ensures consistent alignment withw-4 flex-shrink-0 text-center, and the existing edit/keyboard handling remains intact.apps/desktop/src/components/main/body/shared.tsx (2)
45-45: LGTM: Hover state declaration.The hover state is properly declared for tracking mouse interactions.
137-144: LGTM: Shortcut display implementation.The keyboard shortcut rendering is properly implemented with appropriate positioning and styling.
apps/desktop/src/components/onboarding/permissions.tsx (2)
9-48: LGTM! Past accessibility and layout issues resolved.The
PermissionBlockcomponent successfully addresses previous feedback:
- Line 22:
gap-2now works correctly withflex flex-colcontainer- Line 42:
aria-labelprovides screen reader contextThe component is well-structured with clear props, appropriate conditional rendering, and proper disabled states.
99-101: Clear UX for permission requirements.The button correctly prevents progression until all permissions are granted, and the dynamic label clearly communicates the requirement. Good user experience design.
apps/desktop/src/components/settings/general/permissions.tsx (3)
26-36: Much clearer user-facing text.The messages are now professional and clear:
- "Permission granted" instead of "Good to go :)"
- "Please enable this permission in System Settings" instead of the awkward manual toggle phrasing
This significantly improves the UX in the settings interface.
51-70: Good pending state handling.The button appropriately shows a spinner and "Requesting..." during pending operations, and displays a check icon when authorized. The disabled states are correctly applied.
93-108: Professional and descriptive permission descriptions.The updated titles and descriptions are clear and professional:
- "Required to record your voice during meetings and calls"
- "Required to capture other participants' voices in meetings"
- "Required to detect meeting apps and sync mute status"
These clearly communicate the purpose of each permission to users.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/desktop/src/components/settings/ai/llm/banner.tsx (1)
19-58: Well-structured validation logic with comprehensive edge case handling.The hook correctly validates LLM model availability across multiple scenarios including provider selection, authentication, configuration, and API key requirements.
For a minor optimization, consider using
auth?.sessionin the dependency array instead of the entireauthobject to reduce unnecessary re-computations:- }, [current_llm_provider, current_llm_model, configuredProviders, auth]); + }, [current_llm_provider, current_llm_model, configuredProviders, auth?.session]);This prevents re-renders when other auth properties change that don't affect this hook's logic.
apps/desktop/src/components/settings/ai/stt/banner.tsx (1)
18-75: Consider adding useMemo for consistency and performance.The validation logic is comprehensive and correct. However, unlike the LLM version (
llm/banner.tsxline 26), this hook doesn't wrap its logic inuseMemo, causing recalculation on every render.Wrap the validation logic in
useMemoto match the LLM implementation pattern and avoid unnecessary recalculations:function useHasSTTModel(): { hasModel: boolean; message: string } { const { current_stt_provider, current_stt_model } = useConfigValues( ["current_stt_provider", "current_stt_model"] as const, ); const configuredProviders = main.UI.useResultTable(main.QUERIES.sttProviders, main.STORE_ID); const [p2, p3, tinyEn, smallEn] = useQueries({ queries: [ sttModelQueries.isDownloaded("am-parakeet-v2"), sttModelQueries.isDownloaded("am-parakeet-v3"), sttModelQueries.isDownloaded("QuantizedTinyEn"), sttModelQueries.isDownloaded("QuantizedSmallEn"), ], }); + const result = useMemo(() => { if (!current_stt_provider || !current_stt_model) { return { hasModel: false, message: "Please select a provider and model" }; } // ... rest of the logic ... return { hasModel: true, message: "" }; + }, [current_stt_provider, current_stt_model, configuredProviders, p2.data, p3.data, tinyEn.data, smallEn.data]); + return result; - return { hasModel: true, message: "" }; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/desktop/src/components/settings/ai/llm/banner.tsx(1 hunks)apps/desktop/src/components/settings/ai/llm/index.tsx(1 hunks)apps/desktop/src/components/settings/ai/shared/index.tsx(2 hunks)apps/desktop/src/components/settings/ai/stt/banner.tsx(1 hunks)apps/desktop/src/components/settings/ai/stt/index.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/components/settings/ai/llm/index.tsx
🧰 Additional context used
🧬 Code graph analysis (4)
apps/desktop/src/components/settings/ai/stt/index.tsx (1)
apps/desktop/src/components/settings/ai/stt/banner.tsx (1)
BannerForSTT(8-16)
apps/desktop/src/components/settings/ai/stt/banner.tsx (4)
apps/desktop/src/components/settings/ai/shared/index.tsx (1)
Banner(101-115)apps/desktop/src/config/use-config.ts (1)
useConfigValues(32-53)apps/desktop/src/components/settings/ai/stt/shared.tsx (1)
sttModelQueries(248-275)apps/desktop/src/store/tinybase/schema-internal.ts (1)
AIProviderStorage(40-40)
apps/desktop/src/components/settings/ai/shared/index.tsx (1)
packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/settings/ai/llm/banner.tsx (3)
apps/desktop/src/components/settings/ai/shared/index.tsx (1)
Banner(101-115)apps/desktop/src/auth.tsx (1)
useAuth(177-185)apps/desktop/src/config/use-config.ts (1)
useConfigValues(32-53)
🔇 Additional comments (5)
apps/desktop/src/components/settings/ai/shared/index.tsx (2)
1-5: LGTM! Clean imports for new Banner component.The new imports are properly organized and all are utilized—
cnfor className merging,AnyFieldApias a type annotation, andAlertCircleIconfor the alert UI.
101-115: LGTM! Well-structured alert banner component.The Banner component is cleanly implemented with appropriate styling for an alert/warning state. The negative margins indicate it's designed to break out of its parent container's padding, which is a common pattern for full-width banners.
apps/desktop/src/components/settings/ai/llm/banner.tsx (1)
9-17: LGTM! Clean conditional rendering.The component correctly renders the banner only when no LLM model is configured, providing appropriate guidance to users.
apps/desktop/src/components/settings/ai/stt/index.tsx (1)
1-12: LGTM! Clean integration of STT banner.The banner is appropriately positioned at the top of the STT settings section to provide immediate feedback about configuration status.
apps/desktop/src/components/settings/ai/stt/banner.tsx (1)
8-16: LGTM! Clean conditional rendering.The component correctly renders the banner only when no STT model is configured, mirroring the LLM banner pattern.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/desktop/src/components/chat/body/empty.tsx (3)
18-22: Move static quickActions array outside the component.The
quickActionsarray is recreated on every render. Since it contains static values, consider moving it outside the component as a module-level constant.Apply this diff:
+const QUICK_ACTIONS = [ + "Make a 1-paragraph summary", + "Draft a follow-up mail for others", + "What are the next steps for me", +]; + export function ChatBodyEmpty({ isModelConfigured = true }: { isModelConfigured?: boolean }) { const handleGoToSettings = useCallback(() => { windowsCommands.windowShow({ type: "settings" }) .then(() => new Promise((resolve) => setTimeout(resolve, 1000))) .then(() => windowsCommands.windowEmitNavigate({ type: "settings" }, { path: "/app/settings", search: { tab: "intelligence" }, }) ); }, []); - const quickActions = [ - "Make a 1-paragraph summary", - "Draft a follow-up mail for others", - "What are the next steps for me", - ];Then reference
QUICK_ACTIONSinstead ofquickActionsin the component.
63-69: Use Button component consistently for quick actions.The quick action buttons use plain
buttonelements with inline styling, while the "Go to settings" button (line 41) uses theButtoncomponent from@hypr/ui. Consider using theButtoncomponent consistently for better maintainability and styling uniformity.Apply this diff:
{quickActions.map((action) => ( - <button + <Button key={action} onClick={() => handleQuickAction(action)} - className="px-3 py-2 text-sm bg-white hover:bg-neutral-50 text-neutral-700 rounded-lg transition-colors border border-neutral-200 text-left" + variant="outline" + size="sm" + className="justify-start" > {action} - </button> + </Button> ))}
7-16: Harden window navigation with event-driven readiness check instead of hardcoded delay.The 1000ms delay is an intentional pattern repeated across the codebase (5+ instances) when chaining
windowShow()withwindowEmitNavigate(), indicating it's a known workaround for window initialization sequencing. While this pattern works, it's brittle and suboptimal.Consider refactoring to use an event-driven mechanism (e.g., a window-ready event from
@hypr/plugin-windowsor a state-based check) to ensure reliability across varying system conditions rather than relying on a fixed timeout.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/components/chat/body/empty.tsx(1 hunks)
⏰ 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). (1)
- GitHub Check: ci (macos, macos-14)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/desktop/src/components/settings/ai/llm/select.tsx (3)
66-72: Consider a gentler visual transition when provider is changed.When the provider changes, the model is reset (line 78), causing the background to shift from
bg-neutral-50tobg-red-50. This sudden color change might feel abrupt to users who are actively configuring their settings.Consider using a neutral color (like
bg-neutral-50) during intermediate states, reserving red only for explicit validation errors.
223-261: Consider extracting the status derivation logic.The IIFE pattern works but creates a large inline block. For better readability, consider extracting this into a separate function or using early returns with a helper function.
Example refactor:
function deriveHealthStatus(model: any, text: QueryResult) { if (!model) { return { status: "No model configured", message: "Please configure a provider and model", textColor: "text-red-600", }; } if (text.isPending) { return { status: "Checking connection", message: "Testing model connection", textColor: "text-yellow-600", }; } // ... remaining conditions }
271-277: Loading animation works well; CSS classes could reduce inline styles.The animated dots provide good visual feedback. If you prefer to avoid inline styles, the animation delays could be moved to CSS classes.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/components/settings/ai/llm/select.tsx(3 hunks)apps/desktop/src/components/settings/ai/stt/select.tsx(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/desktop/src/components/settings/ai/llm/select.tsx (3)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/components/settings/ai/llm/shared.tsx (1)
PROVIDERS(6-63)apps/desktop/src/components/settings/ai/shared/model-combobox.tsx (1)
ModelCombobox(45-233)
apps/desktop/src/components/settings/ai/stt/select.tsx (2)
packages/utils/src/cn.ts (1)
cn(20-22)apps/desktop/src/components/settings/ai/stt/shared.tsx (3)
PROVIDERS(46-104)ProviderId(8-8)displayModelId(22-44)
⏰ 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). (1)
- GitHub Check: ci (macos, macos-14)
🔇 Additional comments (4)
apps/desktop/src/components/settings/ai/llm/select.tsx (4)
1-3: LGTM! Explicit imports improve clarity.The explicit imports for Select, Tooltip, and the cn utility are clear and follow best practices.
112-136: LGTM! Defensive handling of unconfigured providers.The model field correctly handles cases where the provider isn't configured by returning an empty result (lines 118-120), and the
disabledprop prevents interaction whenmaybeListModelsis null.
138-138: LGTM! HealthCheck visibility controlled by saved configuration.The conditional rendering ensures HealthCheck only appears after the provider and model have been saved to the config, preventing confusion during intermediate configuration states.
Note: The HealthCheck component has additional null checks for the model (lines 224-230), which are now redundant but provide extra defensive coding.
47-80: The form will save an empty model, but safeguards prevent it from being used downstream.The concern about submitting with an empty model is partially mitigated by existing code. All downstream usage checks for both
current_llm_provider && current_llm_modelbefore using the model—seeuseLLMConnection.tsline 101,banner.tsxline 27, and theHealthCheckguard at line 138. Additionally, the store schema allowscurrent_llm_modelto be optional, so submitting an empty value won't crash.However, this does cause an inefficient UX flow: changing the provider immediately submits an empty model (since the form auto-submits on change), then the user selects a new model and submits again. While the app remains functional, consider preventing this intermediate submission:
- Add form-level validation to block submit when model is empty after provider change
- Debounce the form onChange or use an explicit "Save" button
No description provided.