Remove CommandPaletteProvider wrapper and enhance search functionality#1154
Conversation
WalkthroughThis change replaces the basic search bar with a comprehensive command palette in the mail app, introducing advanced filtering, multiple views, and AI-assisted search. It refactors context and UI components, updates hotkey handling, adds localization strings, adjusts AI prompt guidelines, and simplifies server-side AI search query generation and email tool logic. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CommandPalette
participant AI_Search_Service
participant MailBackend
User->>CommandPalette: Opens palette (hotkey/Cmd+K)
CommandPalette->>User: Displays main/search/filter views
User->>CommandPalette: Enters search/filter query
CommandPalette->>AI_Search_Service: Sends query for AI-assisted search
AI_Search_Service->>CommandPalette: Returns Gmail-compatible search string
CommandPalette->>MailBackend: Queries emails with search string/filters
MailBackend->>CommandPalette: Returns filtered email threads
CommandPalette->>User: Displays results, filters, and options
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
MrgSub
changed the title
MrgSub
force-pushed
the
05-29-natural_language_search_with_ai
branch
from
3f3d3e0 to
4603e46
Compare
MrgSub
changed the title
BlankParticle
force-pushed
the
staging
branch
6 times, most recently
from
e47318e to
a6ec63a
Compare
BlankParticle
force-pushed
the
05-29-natural_language_search_with_ai
branch
from
4603e46 to
7516fc6
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
apps/mail/components/context/command-palette-context.tsx (1)
187-1897: 🛠️ Refactor suggestionConsider breaking down this large component for better maintainability.
The
CommandPalettecomponent is over 1700 lines and handles multiple responsibilities. This makes it difficult to maintain and test. Consider refactoring into smaller, focused components:
- Extract view components (SearchView, FilterView, LabelsView, etc.)
- Move filter logic to a custom hook (useFilters)
- Extract search logic to a custom hook (useSearch)
- Separate localStorage management into a utility module
This would improve code organization, reusability, and testability.
Example structure:
command-palette/ ├── CommandPalette.tsx (main component) ├── hooks/ │ ├── useFilters.ts │ ├── useSearch.ts │ └── useCommandPaletteState.ts ├── views/ │ ├── SearchView.tsx │ ├── FilterView.tsx │ ├── LabelsView.tsx │ └── HelpView.tsx └── utils/ └── localStorage.ts🧰 Tools
🪛 Biome (1.9.4)
[error] 428-428: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 602-602: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 961-962: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1420-1421: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🧹 Nitpick comments (5)
apps/server/src/routes/chat.ts (1)
193-207: LGTM! Enhanced AI summarization capability.The integration of vector database lookup and AI summarization using Cloudflare's BART model significantly enhances the user experience by providing concise thread summaries when metadata is available.
Consider adding error handling for the AI summarization process:
const response = await env.VECTORIZE.getByIds([s.threadId]); if (response.length && response?.[0]?.metadata?.['content']) { const content = response[0].metadata['content'] as string; - const shortResponse = await env.AI.run('@cf/facebook/bart-large-cnn', { - input_text: content, - }); + try { + const shortResponse = await env.AI.run('@cf/facebook/bart-large-cnn', { + input_text: content, + }); + return { + content: [ + { + type: 'text', + text: shortResponse.summary, + }, + ], + }; + } catch (error) { + console.error('AI summarization failed:', error); + // Fall through to default behavior + } +}apps/mail/components/context/command-palette-context.tsx (4)
428-428: Use optional chaining for cleaner code.- if (query && query.trim()) { + if (query?.trim()) { saveRecentSearch(query); setRecentSearches(getRecentSearches()); }- if (query && query.trim()) { + if (query?.trim()) { saveRecentSearch(query); setRecentSearches(getRecentSearches()); }Also applies to: 602-602
🧰 Tools
🪛 Biome (1.9.4)
[error] 428-428: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
961-963: Use optional chaining for safer property access.- if (thread && thread.id) { + if (thread?.id) { navigate(`/inbox?threadId=${thread.id}`); }- (label.name && label.name.toLowerCase().includes(searchQuery.toLowerCase())), + label.name?.toLowerCase().includes(searchQuery.toLowerCase()),Also applies to: 1420-1421
🧰 Tools
🪛 Biome (1.9.4)
[error] 961-962: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
452-454: Remove or replace console.warn statements.Debug logging should be removed or replaced with proper logging/monitoring for production code.
- console.warn('Search applied', { - description: finalQuery, - }); + // Consider using a proper logging service or removing this for productionAlso applies to: 615-617
619-623: Provide more specific error messages to users.The error handling only shows a generic "Failed to process search" message. Consider providing more specific feedback based on the error type.
} catch (error) { console.error('Search error:', error); - toast.error('Failed to process search'); + if (error instanceof Error) { + toast.error(`Search failed: ${error.message}`); + } else { + toast.error('Failed to process search. Please try again.'); + } } finally {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/mail/app/(routes)/layout.tsx(1 hunks)apps/mail/components/context/command-palette-context.tsx(5 hunks)apps/mail/components/mail/mail.tsx(2 hunks)apps/mail/components/ui/ai-sidebar.tsx(1 hunks)apps/mail/hooks/use-mail-navigation.ts(1 hunks)apps/mail/lib/hotkeys/use-hotkey-utils.ts(2 hunks)apps/mail/locales/en.json(1 hunks)apps/server/src/lib/prompts.ts(1 hunks)apps/server/src/routes/agent/tools.ts(0 hunks)apps/server/src/routes/chat.ts(2 hunks)apps/server/src/trpc/routes/ai/search.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/server/src/routes/agent/tools.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/server/src/trpc/routes/ai/search.ts (2)
apps/server/src/trpc/trpc.ts (1)
activeDriverProcedure(44-78)apps/server/src/lib/prompts.ts (1)
GmailSearchAssistantSystemPrompt(233-268)
apps/mail/components/mail/mail.tsx (1)
apps/mail/components/context/command-palette-context.tsx (1)
CommandPalette(187-1897)
🪛 Biome (1.9.4)
apps/mail/components/context/command-palette-context.tsx
[error] 428-428: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 602-602: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 80-80: Shouldn't redeclare 'CommandItem'. Consider to delete it or rename it.
'CommandItem' is defined here:
(lint/suspicious/noRedeclare)
[error] 961-962: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1420-1421: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (12)
apps/server/src/lib/prompts.ts (1)
261-263: LGTM: Good enhancement to search relevance.The addition of the promotional email exclusion guideline is a thoughtful improvement that will enhance search quality by filtering out promotional content by default. The implementation is clean and follows the existing prompt structure pattern.
apps/mail/lib/hotkeys/use-hotkey-utils.ts (2)
160-160: LGTM: Consistent hotkey standardization.The addition of
useKey: truealigns with the application-wide standardization of hotkey handling mentioned in the AI summary. This ensures consistent keyboard interaction behavior.
231-231: LGTM: Consistent hotkey standardization.The addition of
useKey: truemaintains consistency with the useShortcut function above and aligns with the broader hotkey standardization effort across the application.apps/mail/components/mail/mail.tsx (2)
31-31: LGTM: Clean import for enhanced command palette.The import is properly added to support the new command palette functionality that replaces the basic search bar.
531-531: LGTM: Excellent upgrade from SearchBar to CommandPalette.This replacement significantly enhances the user experience by providing a comprehensive command palette with advanced features including:
- Multi-view interface (search, filters, labels, saved searches)
- AI-assisted query generation
- Keyboard shortcuts for quick navigation
- Filter management and persistence
- Natural language search processing
The integration is clean and maintains the existing layout structure.
apps/mail/hooks/use-mail-navigation.ts (1)
223-240: LGTM: Comprehensive hotkey standardization.The systematic addition of
useKey: trueto all hotkey registrations (ArrowUp, ArrowDown, J, K, Enter, Escape) ensures consistent keyboard interaction behavior throughout the mail navigation system. This aligns with the application-wide hotkey standardization effort while preserving all existing navigation functionality.apps/mail/app/(routes)/layout.tsx (1)
7-9:Details
✅ Verification successful
LGTM! Architecture improvement by moving command palette closer to usage.
The removal of
CommandPaletteProviderwrapper simplifies the layout and appears to be part of moving the command palette functionality directly into the mail component where it's actually used.Please verify that the command palette functionality is properly maintained with the new integration approach:
🏁 Script executed:
#!/bin/bash # Description: Verify command palette is properly integrated in mail component # Expected: Find CommandPalette component usage and context provider # Search for CommandPalette component usage rg -A 5 "CommandPalette" --type tsx --type ts # Search for command palette context provider rg -A 10 "CommandPaletteProvider|CommandPaletteContext" --type tsx --type tsLength of output: 180
To account for the
rg: unrecognized file type: tsxerror, let’s search the entire repo without type filters:
🏁 Script executed:
#!/usr/bin/env bash # Search for CommandPalette usages in any file rg -n -A5 "CommandPalette" . # Search for CommandPaletteProvider or CommandPaletteContext in any file rg -n -A10 "CommandPaletteProvider|CommandPaletteContext" .Length of output: 8447
Let’s check where the palette context hooks are actually used:
🏁 Script executed:
#!/usr/bin/env bash # Find any calls to the context hook rg -n "useCommandPalette" . # Find any direct openModal usage rg -n "openModal" .Length of output: 628
Command Palette integration confirmed
The
CommandPaletteis now imported and rendered directly inapps/mail/components/mail/mail.tsx(around line 531), and there are no remaining calls touseCommandPaletteor reliance onCommandPaletteProviderelsewhere. Removing the top-level provider wrapper does not break functionality.• Confirmed
<CommandPalette />usage inapps/mail/components/mail/mail.tsx
• No other references touseCommandPaletteorCommandPaletteProvideroutside the context fileapps/mail/components/ui/ai-sidebar.tsx (2)
447-455: LGTM! Consistent hotkey configuration.The addition of
{ useKey: true }option to theuseHotkeyscall aligns with the broader hotkey handling refactoring mentioned in the AI summary. This ensures consistent behavior across the application.
457-465: LGTM! Consistent hotkey configuration.The addition of
{ useKey: true }option maintains consistency with the previous hotkey handler and the broader refactoring effort.apps/mail/locales/en.json (1)
94-107: LGTM! Comprehensive localization support for command palette features.The new localization strings provide excellent coverage for the enhanced command palette functionality, including search placeholders, filter options, examples, and user guidance messages. The naming convention is consistent and the content is clear and user-friendly.
apps/server/src/routes/chat.ts (1)
168-168: LGTM! Improved response formatting with proper fallback handling.The consolidation of thread details into a single text entry per thread improves readability, and the addition of a "No threads found" fallback message when no threads exist provides better user feedback.
Also applies to: 174-181
apps/server/src/trpc/routes/ai/search.ts (1)
10-23: Clean simplification of the search query generation!The refactoring from a complex multi-step AI interaction to a straightforward
generateObjectcall is a good improvement. The implementation correctly uses theactiveDriverProcedurefor connection handling and leverages the enhancedGmailSearchAssistantSystemPromptfor consistent query generation.
| const translationKey = `common.commandPalette.groups.${groupKey}` as any; | ||
| groupTitle = t(translationKey) || groupKey; | ||
| } catch { | ||
| // Fallback to the original key if translation fails | ||
| } | ||
| } catch {} | ||
|
|
There was a problem hiding this comment.
Handle translation errors properly.
The empty catch block silently swallows errors. Consider logging the error or providing a fallback.
try {
const translationKey = `common.commandPalette.groups.${groupKey}` as any;
groupTitle = t(translationKey) || groupKey;
- } catch {}
+ } catch (error) {
+ console.error('Translation error:', error);
+ groupTitle = groupKey;
+ }📝 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.
| const translationKey = `common.commandPalette.groups.${groupKey}` as any; | |
| groupTitle = t(translationKey) || groupKey; | |
| } catch { | |
| // Fallback to the original key if translation fails | |
| } | |
| } catch {} | |
| try { | |
| const translationKey = `common.commandPalette.groups.${groupKey}` as any; | |
| groupTitle = t(translationKey) || groupKey; | |
| } catch (error) { | |
| console.error('Translation error:', error); | |
| groupTitle = groupKey; | |
| } |
🤖 Prompt for AI Agents
In apps/mail/components/context/command-palette-context.tsx around lines 783 to
786, the empty catch block after attempting to translate group titles silently
swallows any errors. Modify the catch block to log the caught error for
debugging purposes and provide a sensible fallback value for groupTitle to
ensure the application handles translation failures gracefully.
| interface CommandItem { | ||
| title: string; | ||
| icon?: React.ComponentType<{ size?: number; strokeWidth?: number; className?: string }>; | ||
| icon?: ComponentType<{ size?: number; strokeWidth?: number; className?: string }>; | ||
| url?: string; | ||
| onClick?: () => unknown; | ||
| shortcut?: string; | ||
| isBackButton?: boolean; | ||
| disabled?: boolean; | ||
| }; | ||
| keywords?: string[]; | ||
| description?: string; | ||
| } | ||
|
|
There was a problem hiding this comment.
Rename the interface to avoid conflicts.
The CommandItem interface conflicts with the one imported from @/components/ui/command. Since this interface has additional properties specific to the command palette, consider renaming it.
-interface CommandItem {
+interface PaletteCommandItem {
title: string;
icon?: ComponentType<{ size?: number; strokeWidth?: number; className?: string }>;
url?: string;
onClick?: () => unknown;
shortcut?: string;
isBackButton?: boolean;
disabled?: boolean;
keywords?: string[];
description?: string;
}Also update the type references throughout the file:
- const allCommands = useMemo(() => {
+ const allCommands = useMemo((): { group: string; items: PaletteCommandItem[] }[] => {
type CommandGroup = {
group: string;
- items: CommandItem[];
+ items: PaletteCommandItem[];
};
- const searchCommands: CommandItem[] = [];
- const mailCommands: CommandItem[] = [];
- const settingsCommands: CommandItem[] = [];
- const otherCommands: Record<string, CommandItem[]> = {};
+ const searchCommands: PaletteCommandItem[] = [];
+ const mailCommands: PaletteCommandItem[] = [];
+ const settingsCommands: PaletteCommandItem[] = [];
+ const otherCommands: Record<string, PaletteCommandItem[]> = {};📝 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.
| interface CommandItem { | |
| title: string; | |
| icon?: React.ComponentType<{ size?: number; strokeWidth?: number; className?: string }>; | |
| icon?: ComponentType<{ size?: number; strokeWidth?: number; className?: string }>; | |
| url?: string; | |
| onClick?: () => unknown; | |
| shortcut?: string; | |
| isBackButton?: boolean; | |
| disabled?: boolean; | |
| }; | |
| keywords?: string[]; | |
| description?: string; | |
| } | |
| // Rename the locally declared interface to avoid conflict | |
| interface PaletteCommandItem { | |
| title: string; | |
| icon?: ComponentType<{ size?: number; strokeWidth?: number; className?: string }>; | |
| url?: string; | |
| onClick?: () => unknown; | |
| shortcut?: string; | |
| isBackButton?: boolean; | |
| disabled?: boolean; | |
| keywords?: string[]; | |
| description?: string; | |
| } |
| interface CommandItem { | |
| title: string; | |
| icon?: React.ComponentType<{ size?: number; strokeWidth?: number; className?: string }>; | |
| icon?: ComponentType<{ size?: number; strokeWidth?: number; className?: string }>; | |
| url?: string; | |
| onClick?: () => unknown; | |
| shortcut?: string; | |
| isBackButton?: boolean; | |
| disabled?: boolean; | |
| }; | |
| keywords?: string[]; | |
| description?: string; | |
| } | |
| // Update all references to use the new name | |
| const allCommands = useMemo((): { group: string; items: PaletteCommandItem[] }[] => { | |
| type CommandGroup = { | |
| group: string; | |
| items: PaletteCommandItem[]; | |
| }; | |
| const searchCommands: PaletteCommandItem[] = []; | |
| const mailCommands: PaletteCommandItem[] = []; | |
| const settingsCommands: PaletteCommandItem[] = []; | |
| const otherCommands: Record<string, PaletteCommandItem[]> = {}; | |
| // ...rest of implementation | |
| }, []); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 80-80: Shouldn't redeclare 'CommandItem'. Consider to delete it or rename it.
'CommandItem' is defined here:
(lint/suspicious/noRedeclare)
🤖 Prompt for AI Agents
In apps/mail/components/context/command-palette-context.tsx around lines 80 to
91, the CommandItem interface conflicts with an imported interface of the same
name. Rename this interface to a unique name like CommandPaletteItem to avoid
conflicts, and update all references to this interface throughout the file
accordingly.
7516fc6 to
b5e1ad8
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/mail/components/context/command-palette-context.tsx (3)
48-48:⚠️ Potential issueRemove unused server-side imports.
These imports are not used in this client component and should be removed:
GmailSearchAssistantSystemPromptfrom server code (violates client-server boundary)openaifrom '@ai-sdk/openai'-import { GmailSearchAssistantSystemPrompt } from '../../../server/src/lib/prompts'; import { DialogDescription, DialogTitle } from '@/components/ui/dialog';-import { openai } from '@ai-sdk/openai'; import { Button } from '../ui/button';Also applies to: 67-67
76-86:⚠️ Potential issueRename the interface to avoid naming conflict.
The
CommandIteminterface conflicts with the one imported from@/components/ui/command. Since this interface has additional properties specific to the command palette, it should be renamed.-interface CommandItem { +interface PaletteCommandItem { title: string; icon?: ComponentType<{ size?: number; strokeWidth?: number; className?: string }>; url?: string; onClick?: () => unknown; shortcut?: string; isBackButton?: boolean; disabled?: boolean; keywords?: string[]; description?: string; }Also update all type references throughout the file to use
PaletteCommandIteminstead ofCommandItem.🧰 Tools
🪛 Biome (1.9.4)
[error] 76-76: Shouldn't redeclare 'CommandItem'. Consider to delete it or rename it.
'CommandItem' is defined here:
(lint/suspicious/noRedeclare)
779-781: 🛠️ Refactor suggestionAdd proper error handling for translation failures.
The empty catch block silently swallows errors, making debugging difficult.
try { const translationKey = `common.commandPalette.groups.${groupKey}` as any; groupTitle = t(translationKey) || groupKey; - } catch {} + } catch (error) { + console.error('Translation error:', error); + groupTitle = groupKey; + }
🧹 Nitpick comments (3)
apps/mail/components/context/command-palette-context.tsx (3)
424-424: Use optional chaining for cleaner code.These null checks can be simplified using optional chaining.
- if (query && query.trim()) { + if (query?.trim()) {Also applies to: 598-598
🧰 Tools
🪛 Biome (1.9.4)
[error] 424-424: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
957-958: Simplify null checks with optional chaining.- if (thread && thread.id) { + if (thread?.id) {🧰 Tools
🪛 Biome (1.9.4)
[error] 957-958: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
1416-1417: Use optional chaining for label name check.- label.name && label.name.toLowerCase().includes(searchQuery.toLowerCase())), + label.name?.toLowerCase().includes(searchQuery.toLowerCase())),🧰 Tools
🪛 Biome (1.9.4)
[error] 1416-1417: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/mail/app/(routes)/layout.tsx(1 hunks)apps/mail/components/context/command-palette-context.tsx(5 hunks)apps/mail/components/mail/mail.tsx(2 hunks)apps/mail/components/ui/ai-sidebar.tsx(1 hunks)apps/mail/hooks/use-mail-navigation.ts(1 hunks)apps/mail/lib/hotkeys/use-hotkey-utils.ts(2 hunks)apps/mail/locales/en.json(1 hunks)apps/server/src/lib/prompts.ts(1 hunks)apps/server/src/routes/agent/tools.ts(0 hunks)apps/server/src/routes/chat.ts(2 hunks)apps/server/src/trpc/routes/ai/search.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/server/src/routes/agent/tools.ts
✅ Files skipped from review due to trivial changes (1)
- apps/mail/lib/hotkeys/use-hotkey-utils.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/mail/app/(routes)/layout.tsx
- apps/mail/components/mail/mail.tsx
- apps/mail/components/ui/ai-sidebar.tsx
- apps/mail/hooks/use-mail-navigation.ts
- apps/server/src/lib/prompts.ts
- apps/mail/locales/en.json
- apps/server/src/routes/chat.ts
🧰 Additional context used
🪛 Biome (1.9.4)
apps/mail/components/context/command-palette-context.tsx
[error] 424-424: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 598-598: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 76-76: Shouldn't redeclare 'CommandItem'. Consider to delete it or rename it.
'CommandItem' is defined here:
(lint/suspicious/noRedeclare)
[error] 957-958: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1416-1417: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (2)
apps/server/src/trpc/routes/ai/search.ts (1)
1-23: LGTM! Clean simplification of the search query generation.The refactoring from a complex multi-step process to a streamlined
generateObjectcall is a good improvement. The mutation now has a clear single responsibility of generating Gmail-compatible search queries.apps/mail/components/context/command-palette-context.tsx (1)
183-1908: Excellent implementation of the comprehensive command palette!The command palette implementation is well-structured with:
- Multiple views for different functionalities (search, filter, labels, etc.)
- Proper state management and persistence
- Good keyboard shortcut support
- Natural language search processing
- Clean separation of rendering logic for each view
The integration with the AI-powered search query generation is seamless.
🧰 Tools
🪛 Biome (1.9.4)
[error] 424-424: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 598-598: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 957-958: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1416-1417: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
BlankParticle
force-pushed
the
05-29-natural_language_search_with_ai
branch
from
b5e1ad8 to
8fb08c8
Compare
Merge activity
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/mail/components/context/command-palette-context.tsx (2)
779-781: Handle translation errors properly.The empty catch block silently swallows errors. Consider logging the error or providing a fallback.
try { const translationKey = `common.commandPalette.groups.${groupKey}` as any; groupTitle = t(translationKey) || groupKey; - } catch {} + } catch (error) { + console.error('Translation error:', error); + groupTitle = groupKey; + }
76-86:⚠️ Potential issueFix interface name conflict.
The
CommandIteminterface conflicts with the imported interface from@/components/ui/command. This creates a redeclaration error and potential confusion.-interface CommandItem { +interface PaletteCommandItem { title: string; icon?: ComponentType<{ size?: number; strokeWidth?: number; className?: string }>; url?: string; onClick?: () => unknown; shortcut?: string; isBackButton?: boolean; disabled?: boolean; keywords?: string[]; description?: string; }Then update all references throughout the file:
- const searchCommands: CommandItem[] = []; - const mailCommands: CommandItem[] = []; - const settingsCommands: CommandItem[] = []; + const searchCommands: PaletteCommandItem[] = []; + const mailCommands: PaletteCommandItem[] = []; + const settingsCommands: PaletteCommandItem[] = [];🧰 Tools
🪛 Biome (1.9.4)
[error] 76-76: Shouldn't redeclare 'CommandItem'. Consider to delete it or rename it.
'CommandItem' is defined here:
(lint/suspicious/noRedeclare)
🧹 Nitpick comments (4)
apps/mail/components/context/command-palette-context.tsx (4)
420-453: Use optional chaining for safer property access.The function has proper logic but could benefit from optional chaining for safer property access.
- if (query && query.trim()) { + if (query?.trim()) { saveRecentSearch(query); setRecentSearches(getRecentSearches()); }🧰 Tools
🪛 Biome (1.9.4)
[error] 424-424: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
571-624: Use optional chaining and improve error handling.The search handler has good structure but could benefit from safer property access and more specific error messaging.
- if (query && query.trim()) { + if (query?.trim()) { saveRecentSearch(query); setRecentSearches(getRecentSearches()); }Also consider providing more specific error messages based on the error type:
} catch (error) { console.error('Search error:', error); - toast.error('Failed to process search'); + toast.error(error instanceof Error ? error.message : 'Failed to process search'); } finally {🧰 Tools
🪛 Biome (1.9.4)
[error] 598-598: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
954-964: Use optional chaining for safer thread navigation.The thread navigation logic could be simplified with optional chaining.
onSelect={() => { runCommand(() => { try { - if (thread && thread.id) { + if (thread?.id) { navigate(`/inbox?threadId=${thread.id}`); } } catch (error) { console.error('Error navigating to thread:', error); toast.error('Failed to open email'); } }); }}🧰 Tools
🪛 Biome (1.9.4)
[error] 957-958: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
1413-1417: Use optional chaining for label filtering.The label filtering logic can be simplified with optional chaining.
.filter( (label) => !searchQuery || - (label.name && label.name.toLowerCase().includes(searchQuery.toLowerCase())), + label.name?.toLowerCase().includes(searchQuery.toLowerCase()), )🧰 Tools
🪛 Biome (1.9.4)
[error] 1416-1417: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
apps/mail/app/(routes)/layout.tsx(1 hunks)apps/mail/components/context/command-palette-context.tsx(5 hunks)apps/mail/components/mail/mail.tsx(2 hunks)apps/mail/components/ui/ai-sidebar.tsx(1 hunks)apps/mail/hooks/use-mail-navigation.ts(1 hunks)apps/mail/lib/hotkeys/use-hotkey-utils.ts(2 hunks)apps/mail/locales/en.json(1 hunks)apps/server/src/lib/prompts.ts(1 hunks)apps/server/src/routes/agent/tools.ts(0 hunks)apps/server/src/routes/chat.ts(2 hunks)apps/server/src/trpc/routes/ai/search.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/server/src/routes/agent/tools.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- apps/mail/components/ui/ai-sidebar.tsx
- apps/mail/hooks/use-mail-navigation.ts
- apps/server/src/lib/prompts.ts
- apps/mail/app/(routes)/layout.tsx
- apps/mail/locales/en.json
- apps/mail/components/mail/mail.tsx
- apps/mail/lib/hotkeys/use-hotkey-utils.ts
- apps/server/src/routes/chat.ts
🧰 Additional context used
🪛 Biome (1.9.4)
apps/mail/components/context/command-palette-context.tsx
[error] 424-424: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 598-598: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 76-76: Shouldn't redeclare 'CommandItem'. Consider to delete it or rename it.
'CommandItem' is defined here:
(lint/suspicious/noRedeclare)
[error] 957-958: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 1416-1417: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (5)
apps/server/src/trpc/routes/ai/search.ts (1)
1-23: LGTM! Clean simplification of AI search query generation.The refactored mutation successfully separates concerns by focusing only on generating search queries rather than executing searches and processing results. The implementation is clean and straightforward:
- Uses the appropriate GPT-4o model
- Leverages the centralized system prompt
- Simple input validation with single string query
- Returns the generated object directly
This aligns well with the enhanced command palette frontend that now handles search execution and result processing.
apps/mail/components/context/command-palette-context.tsx (4)
131-152: LGTM! Well-structured localStorage utilities.The recent searches localStorage functions are well-implemented with proper error handling and reasonable limits (10 items). Good practices:
- Try-catch blocks around localStorage operations
- Filtering duplicates before adding new items
- Slice to maintain reasonable history size
- Proper error logging
207-209: LGTM! Proper integration with AI search mutation.The TRPC mutation hook is correctly set up to integrate with the simplified AI search endpoint. This creates a clean separation where the frontend handles the user interaction and calls the backend only for AI-powered query generation.
1855-1892: LGTM! Well-designed command palette UI.The main render function creates an excellent user experience:
- Clear visual indicator of active filters with badge
- Proper keyboard shortcut display
- Responsive design with different text for different screen sizes
- Proper dialog state management
- Good accessibility with VisuallyHidden titles
The component successfully integrates all the complex functionality into a clean, usable interface.
1895-1908: LGTM! Clean provider implementation.The provider properly exposes the context API and renders the command palette. The
openModalcallback provides a clean way for external components to trigger the palette.
2 participants
Remove CommandPaletteProvider and Enhance Command Palette Functionality
Description
This PR significantly enhances the command palette functionality while simplifying its implementation. The main changes include:
CommandPaletteProviderwrapper from the mail app layoutType of Change
Areas Affected
Testing Done
Checklist
Additional Notes
The command palette now provides a more powerful and intuitive way to search and filter emails, with support for Gmail-compatible search syntax and natural language processing. Users can now easily filter by sender, recipient, subject, date ranges, and more.
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Backend Improvements