Replaced current search bar with a command/search palette #1072
Replaced current search bar with a command/search palette #1072
Conversation
📝 WalkthroughWalkthroughA new Command Palette feature was introduced to the desktop app, including a new dependency and a React component for a multi-entity search dialog. The search provider and search bar were updated to integrate the Command Palette, and the sessions database search query was expanded to include additional fields and improved result ordering. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SearchBar
participant CommandPalette
participant DB
participant Router
User->>SearchBar: Clicks search bar or presses Mod+K
SearchBar->>CommandPalette: Set open=true
CommandPalette->>User: Shows modal, focuses input
User->>CommandPalette: Types search query
CommandPalette->>DB: Debounced multi-entity search (sessions, events, people, orgs)
DB-->>CommandPalette: Returns grouped results
CommandPalette->>User: Displays results with highlights
User->>CommandPalette: Selects result
CommandPalette->>Router: Navigate to selected entity route
CommandPalette->>CommandPalette: Close modal, clear state
sequenceDiagram
participant CommandPalette
participant DB
CommandPalette->>DB: Search sessions with query
DB->>DB: Strip HTML from memo fields
DB->>DB: Match query in title, enhanced_memo_html, raw_memo_html
DB->>DB: Order by title match priority, then by created_at desc
DB-->>CommandPalette: Return ordered session results
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Clippy (1.86.0)error: failed to load source for dependency Caused by: Caused by: Caused by: ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
apps/desktop/src/components/search-bar.tsx (1)
124-124: Consider the UX implications of keeping the search input interactive.The search bar still allows text input and focus, but clicking the container opens the command palette. This dual behavior might confuse users who type directly in the search input expecting it to work.
Consider either:
- Making the entire search bar read-only and act purely as a button to open the command palette
- Removing the onClick handler from the container and only opening the palette via the keyboard shortcut
For option 1:
<input ref={searchInputRef} type="text" value={searchQuery} onChange={handleInputChange} onKeyDown={handleKeyDown} onFocus={() => { setIsFocused(true); setShowHistory(searchQuery === ""); }} onBlur={() => { setTimeout(() => { setIsFocused(false); setShowHistory(false); }, 150); }} placeholder={t`Search...`} className="flex-1 bg-transparent outline-none text-xs" + readOnly + style={{ cursor: 'pointer' }} />Also applies to: 129-147
crates/db-user/src/sessions_ops.rs (1)
125-142: Consider performance implications of the expanded search query.The implementation correctly expands search to memo fields, but there are performance considerations:
- Multiple
REPLACEoperations on potentially large HTML fields could be slowLIKEqueries with leading wildcards prevent index usageConsider these optimizations:
- Add a full-text search index on searchable fields
- Pre-process and store plain text versions of HTML content
- Use database-specific full-text search capabilities (e.g., SQLite FTS5)
apps/desktop/src/components/command-palette.tsx (1)
192-208: Consider the impact of global style injection.The injected styles affect all modal dialogs, not just the command palette.
Consider a more targeted approach:
- Add a unique class or data attribute to the CommandDialog
- Use CSS modules or styled-components for scoped styles
- Check if the cmdk library provides width configuration options
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
apps/desktop/package.json(1 hunks)apps/desktop/src/components/command-palette.tsx(1 hunks)apps/desktop/src/components/search-bar.tsx(4 hunks)apps/desktop/src/contexts/search.tsx(3 hunks)crates/db-user/src/sessions_ops.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{js,ts,tsx,rs}`: 1. No error handling. 2. No unused imports, variables, or functions. 3. For comments, keep it minimal. It should be about "Why", not "What".
**/*.{js,ts,tsx,rs}: 1. No error handling.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
crates/db-user/src/sessions_ops.rsapps/desktop/src/contexts/search.tsxapps/desktop/src/components/search-bar.tsxapps/desktop/src/components/command-palette.tsx
🧬 Code Graph Analysis (2)
apps/desktop/src/contexts/search.tsx (1)
apps/desktop/src/components/command-palette.tsx (1)
CommandPalette(92-393)
apps/desktop/src/components/search-bar.tsx (1)
apps/desktop/src/components/command-palette.tsx (1)
CommandPalette(92-393)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: ci
- GitHub Check: ci (windows, windows-latest)
- GitHub Check: ci (macos, macos-latest)
🔇 Additional comments (5)
apps/desktop/package.json (1)
72-72: cmdk@1.1.1 version verified – no action neededcmdk@1.1.1 is the latest stable release and has no known direct vulnerabilities (per Snyk). Pinning this version is fine for reproducibility. For ongoing security, continue to:
- Audit indirect dependencies of
cmdk- Integrate tooling (e.g., Snyk, Dependabot) to catch new vulnerabilities as they arise
apps/desktop/src/contexts/search.tsx (1)
1-1: Clean integration of the CommandPalette!The implementation properly manages the command palette state and simplifies the mod+k hotkey handler.
Also applies to: 6-6, 32-33, 38-38, 110-110
apps/desktop/src/components/command-palette.tsx (3)
45-77: Well-implemented content snippet extraction!The function properly handles HTML stripping, context extraction, and edge cases.
79-90: Clean sorting implementation.The date-based sorting logic is correct and handles both sort directions properly.
163-180: Good type-safe navigation handling!The switch statement properly handles all match types with appropriate navigation.
| REPLACE(REPLACE(REPLACE(enhanced_memo_html, '<', ' '), '>', ' '), ' ', ' ') LIKE ? OR | ||
| REPLACE(REPLACE(REPLACE(raw_memo_html, '<', ' '), '>', ' '), ' ', ' ') LIKE ? | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
HTML entity stripping is incomplete.
The current implementation only handles <, >, and , but other common HTML entities like &, ", <, > are not processed.
Consider a more comprehensive approach:
-REPLACE(REPLACE(REPLACE(enhanced_memo_html, '<', ' '), '>', ' '), ' ', ' ') LIKE ?
+REPLACE(REPLACE(REPLACE(REPLACE(REPLACE(REPLACE(REPLACE(enhanced_memo_html, '<', ' '), '>', ' '), ' ', ' '), '&', '&'), '"', '"'), '<', '<'), '>', '>') LIKE ?Or better yet, implement a proper HTML-to-text conversion function in the application layer before inserting into the database.
📝 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.
| REPLACE(REPLACE(REPLACE(enhanced_memo_html, '<', ' '), '>', ' '), ' ', ' ') LIKE ? OR | |
| REPLACE(REPLACE(REPLACE(raw_memo_html, '<', ' '), '>', ' '), ' ', ' ') LIKE ? | |
| ) | |
| REPLACE( | |
| REPLACE( | |
| REPLACE( | |
| REPLACE( | |
| REPLACE( | |
| REPLACE( | |
| REPLACE(enhanced_memo_html, '<', ' '), | |
| '>', ' '), | |
| ' ', ' '), | |
| '&', '&'), | |
| '"', '"'), | |
| '<', '<'), | |
| '>', '>') LIKE ? OR | |
| REPLACE(REPLACE(REPLACE(raw_memo_html, '<', ' '), '>', ' '), ' ', ' ') LIKE ? | |
| ) |
🤖 Prompt for AI Agents
In crates/db-user/src/sessions_ops.rs around lines 128 to 130, the current code
replaces only a few HTML entities like '<', '>', and ' ', missing others
such as '&', '"', '<', and '>'. To fix this, extend the
replacement logic to cover these additional common HTML entities or, preferably,
implement a dedicated HTML-to-text conversion function in the application layer
that fully sanitizes and converts HTML content before it is inserted into the
database, ensuring comprehensive and reliable entity handling.
| const sessionMatches = matches.filter(match => match.type === "session"); | ||
| const eventMatches = matches.filter(match => match.type === "event"); | ||
| const humanMatches = matches.filter(match => match.type === "human"); | ||
| const organizationMatches = matches.filter(match => match.type === "organization"); |
There was a problem hiding this comment.
Fix TypeScript type narrowing for filtered matches.
The filter method doesn't properly narrow types, causing a type mismatch with sortSessionMatches.
Use type predicates for proper type narrowing:
- const sessionMatches = matches.filter(match => match.type === "session");
+ const sessionMatches = matches.filter((match): match is SearchMatch & { type: "session" } => match.type === "session");Or explicitly type the filtered arrays:
- const sessionMatches = matches.filter(match => match.type === "session");
+ const sessionMatches = matches.filter(match => match.type === "session") as Array<SearchMatch & { type: "session" }>;Also applies to: 265-265
🤖 Prompt for AI Agents
In apps/desktop/src/components/command-palette.tsx around lines 183 to 186, the
filter method does not correctly narrow the types of the filtered arrays,
causing type mismatches with functions like sortSessionMatches. To fix this,
define type predicate functions that assert the type of each match and use them
in the filter calls, or explicitly annotate the types of the filtered arrays to
ensure TypeScript understands their narrowed types. Apply the same fix around
line 265 as well.
| // Debounced search | ||
| useEffect(() => { | ||
| const timeoutId = setTimeout(() => { | ||
| performSearch(query); | ||
| }, 200); | ||
|
|
||
| return () => clearTimeout(timeoutId); | ||
| }, [query]); |
There was a problem hiding this comment.
Add missing dependencies to the effect.
The performSearch function uses userId from the component scope, but it's not included in the effect dependencies.
Either add the missing dependencies or use a callback:
useEffect(() => {
const timeoutId = setTimeout(() => {
performSearch(query);
}, 200);
return () => clearTimeout(timeoutId);
- }, [query]);
+ }, [query, userId]);Alternatively, move performSearch inside the effect or wrap it with useCallback.
📝 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.
| // Debounced search | |
| useEffect(() => { | |
| const timeoutId = setTimeout(() => { | |
| performSearch(query); | |
| }, 200); | |
| return () => clearTimeout(timeoutId); | |
| }, [query]); | |
| // Debounced search | |
| useEffect(() => { | |
| const timeoutId = setTimeout(() => { | |
| performSearch(query); | |
| }, 200); | |
| return () => clearTimeout(timeoutId); | |
| }, [query, userId]); |
🤖 Prompt for AI Agents
In apps/desktop/src/components/command-palette.tsx around lines 136 to 143, the
useEffect hook depends on the performSearch function which uses userId from the
component scope, but userId and performSearch are not included in the dependency
array. To fix this, either add userId and performSearch to the dependencies, or
move performSearch inside the useEffect, or wrap performSearch with useCallback
including userId as a dependency to ensure the effect updates correctly when
userId changes.
| const highlightText = (text: string, query: string) => { | ||
| if (!query.trim()) { | ||
| return text; | ||
| } | ||
|
|
||
| const parts = text.split(new RegExp(`(${query})`, "gi")); | ||
| return parts.map((part, index) => | ||
| part.toLowerCase() === query.toLowerCase() | ||
| ? <span key={index} className="font-bold text-neutral-900">{part}</span> | ||
| : part | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Escape special regex characters in the search query.
The query is used directly in a regex pattern without escaping, which could cause errors if users search for special characters.
const highlightText = (text: string, query: string) => {
if (!query.trim()) {
return text;
}
- const parts = text.split(new RegExp(`(${query})`, "gi"));
+ const escapedQuery = query.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
+ const parts = text.split(new RegExp(`(${escapedQuery})`, "gi"));
return parts.map((part, index) =>
- part.toLowerCase() === query.toLowerCase()
+ part.toLowerCase() === query.toLowerCase()
? <span key={index} className="font-bold text-neutral-900">{part}</span>
: part
);
};📝 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 highlightText = (text: string, query: string) => { | |
| if (!query.trim()) { | |
| return text; | |
| } | |
| const parts = text.split(new RegExp(`(${query})`, "gi")); | |
| return parts.map((part, index) => | |
| part.toLowerCase() === query.toLowerCase() | |
| ? <span key={index} className="font-bold text-neutral-900">{part}</span> | |
| : part | |
| ); | |
| }; | |
| const highlightText = (text: string, query: string) => { | |
| if (!query.trim()) { | |
| return text; | |
| } | |
| const escapedQuery = query.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); | |
| const parts = text.split(new RegExp(`(${escapedQuery})`, "gi")); | |
| return parts.map((part, index) => | |
| part.toLowerCase() === query.toLowerCase() | |
| ? <span key={index} className="font-bold text-neutral-900">{part}</span> | |
| : part | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In apps/desktop/src/components/command-palette.tsx around lines 32 to 43, the
search query is used directly in a regex without escaping special characters,
which can cause errors or unexpected behavior. Fix this by implementing a
function to escape all regex special characters in the query string before using
it in the RegExp constructor. Replace the direct use of query with the escaped
version to ensure safe and correct regex matching.
| } catch (error) { | ||
| console.error("Search error:", error); | ||
| } finally { | ||
| setIsSearching(false); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling with user feedback.
Errors are only logged to console without informing the user.
Consider showing an error state:
} catch (error) {
console.error("Search error:", error);
+ // Show error state to user
+ setMatches([]);
+ // Consider adding an error state:
+ // setSearchError("Failed to search. Please try again.");
} finally {
setIsSearching(false);
}📝 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.
| } catch (error) { | |
| console.error("Search error:", error); | |
| } finally { | |
| setIsSearching(false); | |
| } | |
| } catch (error) { | |
| console.error("Search error:", error); | |
| // Show error state to user | |
| setMatches([]); | |
| // Consider adding an error state: | |
| // setSearchError("Failed to search. Please try again."); | |
| } finally { | |
| setIsSearching(false); | |
| } |
🤖 Prompt for AI Agents
In apps/desktop/src/components/command-palette.tsx around lines 129 to 133, the
catch block only logs errors to the console without providing user feedback.
Modify the error handling to update the component state with an error message or
flag that can be displayed in the UI, so users are informed when a search error
occurs. Ensure this error state is cleared or reset appropriately after
handling.
2 participants
[TODOs]