Improve filesystem fuzzy search results#1123
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR introduces a file-search feature with a TRPC backend procedure using Fuse.js for indexing and caching filesystem entries, paired with frontend components for displaying search results, a debounced search input, and a custom hook for API integration via electronTrpc. Changes
Sequence DiagramsequenceDiagram
participant User
participant FileTreeToolbar
participant useFileSearch Hook
participant TRPC Client
participant Search Procedure
participant Fuse.js Index
participant FilesView
participant FileSearchResultNode
User->>FileTreeToolbar: Types search query
FileTreeToolbar->>FileTreeToolbar: Debounce (500ms)
FileTreeToolbar->>useFileSearch Hook: onSearchChange with trimmed query
useFileSearch Hook->>TRPC Client: searchFiles(rootPath, query, includeHidden, limit)
TRPC Client->>Search Procedure: Send search request
Search Procedure->>Search Procedure: Check searchIndexCache for TTL validity
alt Cache valid
Search Procedure->>Fuse.js Index: Use cached index
else Cache expired or missing
Search Procedure->>Search Procedure: buildSearchIndex (enumerate files with fast-glob)
Search Procedure->>Fuse.js Index: Create new Fuse index
Search Procedure->>Search Procedure: Store in searchIndexCache with timestamp
end
Fuse.js Index->>Search Procedure: Return ranked results
Search Procedure->>TRPC Client: Return [{ id, name, relativePath, path, isDirectory, score }]
TRPC Client->>useFileSearch Hook: Resolve with searchResults
useFileSearch Hook->>FilesView: Update searchResults, isFetching, hasQuery
FilesView->>FilesView: Conditional render (search vs. tree)
FilesView->>FileSearchResultNode: Render each search result node
FileSearchResultNode->>User: Display file with icon, folder label, score
User->>FileSearchResultNode: Interact (click, double-click, edit)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 3
🤖 Fix all issues with AI agents
In `@apps/desktop/src/lib/trpc/routers/filesystem/index.ts`:
- Around line 60-68: The glob call that builds `entries` currently sets
`suppressErrors: true`, which swallows permission/IO errors; remove the
`suppressErrors: true` option from the fg(...) call in the filesystem router so
globbing errors are allowed to throw and propagate to the existing `searchFiles`
error handling/logging path (locate the fg invocation that assigns `entries` and
delete the `suppressErrors` property).
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileSearchResultNode/FileSearchResultNode.tsx`:
- Around line 97-121: The input's onKeyDown handler in FileSearchResultNode
allows key events to bubble to the parent row, causing Enter/Space to trigger
row handlers; update the onKeyDown callback (the one that checks e.key ===
"Enter" / "Escape" and calls node.submit(node.reset())) to call
e.stopPropagation() at the start of the handler so key events during renaming do
not propagate to the parent row and interfere with activation/toggle behavior.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileTreeToolbar/FileTreeToolbar.tsx`:
- Around line 95-101: The clear-search button in FileTreeToolbar is icon-only
and needs an accessible label: update the button element (the one calling
handleClearSearch and containing <LuX />) to include an appropriate aria-label
(e.g., aria-label="Clear search" or aria-label="Clear file search") and
optionally a title attribute for tooltip support so screen readers and keyboard
users know its purpose.
🧹 Nitpick comments (3)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileSearchResultNode/FileSearchResultNode.tsx (1)
19-42: Prefer a params object fortruncatePathStart.
This avoids positional arguments and scales better if you add options.As per coding guidelines, Functions with 2+ parameters should accept a single params object with named properties instead of positional arguments.♻️ Suggested refactor
-function truncatePathStart(value: string, maxLength: number): string { +function truncatePathStart({ + value, + maxLength, +}: { + value: string; + maxLength: number; +}): string { if (value.length <= maxLength) { return value; } const sliceLength = Math.max(1, maxLength - 3); return `...${value.slice(value.length - sliceLength)}`; } ... - const folderLabelDisplay = truncatePathStart( - folderLabel, - PATH_LABEL_MAX_CHARS, - ); + const folderLabelDisplay = truncatePathStart({ + value: folderLabel, + maxLength: PATH_LABEL_MAX_CHARS, + });apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/hooks/useFileSearch/useFileSearch.ts (2)
19-32: Missing error state exposure.The hook doesn't expose
errororisErrorfrom the tRPC query. Consumers have no visibility into failed searches, which violates the guideline to never silently swallow errors. Consider exposing error state so the UI can inform users when search fails.♻️ Proposed fix to expose error state
const { data: searchResults, isFetching } = + const { data: searchResults, isFetching, error } = electronTrpc.filesystem.searchFiles.useQuery( { rootPath: worktreePath ?? "", query: trimmedQuery, includeHidden, limit, }, { enabled: Boolean(worktreePath) && trimmedQuery.length > 0, staleTime: 1000, placeholderData: (previous) => previous ?? [], }, ); return { searchResults: searchResults ?? [], isFetching, hasQuery: trimmedQuery.length > 0, + error, };As per coding guidelines: "Never swallow errors silently; at minimum log them with context".
29-29: Extract magic number to a named constant.
staleTime: 1000is a magic number. Consider extracting it to a named constant at module top for clarity and maintainability.♻️ Proposed fix
import { electronTrpc } from "renderer/lib/electron-trpc"; import { SEARCH_RESULT_LIMIT } from "../../constants"; +const SEARCH_STALE_TIME_MS = 1000; + interface UseFileSearchParams {Then use:
- staleTime: 1000, + staleTime: SEARCH_STALE_TIME_MS,As per coding guidelines: "Avoid magic numbers by extracting them to named constants at module top".
| const entries = await fg("**/*", { | ||
| cwd: rootPath, | ||
| onlyFiles: true, | ||
| dot: includeHidden, | ||
| followSymbolicLinks: false, | ||
| unique: true, | ||
| suppressErrors: true, | ||
| ignore: DEFAULT_IGNORE_PATTERNS, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/desktop/src/lib/trpc/routers/filesystem/index.ts | head -100Repository: superset-sh/superset
Length of output: 2915
🏁 Script executed:
wc -l apps/desktop/src/lib/trpc/routers/filesystem/index.tsRepository: superset-sh/superset
Length of output: 121
🏁 Script executed:
# Get context around lines 60-68
sed -n '40,90p' apps/desktop/src/lib/trpc/routers/filesystem/index.tsRepository: superset-sh/superset
Length of output: 1134
🏁 Script executed:
# Check what function contains this code and its error handling
sed -n '1,150p' apps/desktop/src/lib/trpc/routers/filesystem/index.ts | cat -nRepository: superset-sh/superset
Length of output: 4435
🏁 Script executed:
sed -n '90,250p' apps/desktop/src/lib/trpc/routers/filesystem/index.ts | grep -A 20 -B 5 "searchFiles\|catch"Repository: superset-sh/superset
Length of output: 2913
🏁 Script executed:
# Check if there's logging around these procedures
rg -A 10 "searchFiles|buildSearchIndex" apps/desktop/src/lib/trpc/routers/filesystem/index.ts | head -60Repository: superset-sh/superset
Length of output: 1496
Remove suppressErrors: true to surface glob errors.
suppressErrors: true silently skips permission and IO errors during file globbing, resulting in incomplete search indexes with no indication of failure. Even though searchFiles has error handling, it won't catch suppressed errors since they're never thrown. Remove this option so errors propagate to the existing error logging path.
Suggested fix
const entries = await fg("**/*", {
cwd: rootPath,
onlyFiles: true,
dot: includeHidden,
followSymbolicLinks: false,
unique: true,
- suppressErrors: true,
ignore: DEFAULT_IGNORE_PATTERNS,
});Per coding guidelines: Never swallow errors silently; at minimum log them with context.
📝 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 entries = await fg("**/*", { | |
| cwd: rootPath, | |
| onlyFiles: true, | |
| dot: includeHidden, | |
| followSymbolicLinks: false, | |
| unique: true, | |
| suppressErrors: true, | |
| ignore: DEFAULT_IGNORE_PATTERNS, | |
| }); | |
| const entries = await fg("**/*", { | |
| cwd: rootPath, | |
| onlyFiles: true, | |
| dot: includeHidden, | |
| followSymbolicLinks: false, | |
| unique: true, | |
| ignore: DEFAULT_IGNORE_PATTERNS, | |
| }); |
🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/filesystem/index.ts` around lines 60 - 68,
The glob call that builds `entries` currently sets `suppressErrors: true`, which
swallows permission/IO errors; remove the `suppressErrors: true` option from the
fg(...) call in the filesystem router so globbing errors are allowed to throw
and propagate to the existing `searchFiles` error handling/logging path (locate
the fg invocation that assigns `entries` and delete the `suppressErrors`
property).
| <input | ||
| type="text" | ||
| defaultValue={data.name} | ||
| onFocus={(e) => { | ||
| const dotIndex = data.name.lastIndexOf("."); | ||
| if (dotIndex > 0) { | ||
| e.target.setSelectionRange(0, dotIndex); | ||
| return; | ||
| } | ||
| e.target.select(); | ||
| }} | ||
| onBlur={() => node.reset()} | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter") { | ||
| const newName = e.currentTarget.value.trim(); | ||
| if (newName && newName !== data.name) { | ||
| node.submit(newName); | ||
| } else { | ||
| node.reset(); | ||
| } | ||
| } | ||
| if (e.key === "Escape") { | ||
| node.reset(); | ||
| } | ||
| }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd FileSearchResultNode.tsxRepository: superset-sh/superset
Length of output: 207
🏁 Script executed:
cat -n apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileSearchResultNode/FileSearchResultNode.tsxRepository: superset-sh/superset
Length of output: 4624
🏁 Script executed:
# Search for row handlers and parent container event listeners
rg "onKeyDown|onClick|onContextMenu" apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/ -A 2 -B 2Repository: superset-sh/superset
Length of output: 18871
🏁 Script executed:
# Check if there's a FilesView or similar parent that handles keyboard events
rg "FilesView|FileSearchResult" apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/ -lRepository: superset-sh/superset
Length of output: 532
Stop keydown propagation from the rename input to prevent parent row handler interference.
While editing a filename, keydown events from the input bubble to the parent div's handler, causing Enter to trigger unwanted node activation and Space to both prevent typing spaces and trigger the toggle action. Add e.stopPropagation() at the start of the input's onKeyDown to isolate rename editing from row-level keyboard handling.
Fix
onKeyDown={(e) => {
+ e.stopPropagation();
if (e.key === "Enter") {
+ e.preventDefault();
const newName = e.currentTarget.value.trim();
if (newName && newName !== data.name) {
node.submit(newName);
} else {
node.reset();
}
}
if (e.key === "Escape") {
+ e.preventDefault();
node.reset();
}
}}📝 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.
| <input | |
| type="text" | |
| defaultValue={data.name} | |
| onFocus={(e) => { | |
| const dotIndex = data.name.lastIndexOf("."); | |
| if (dotIndex > 0) { | |
| e.target.setSelectionRange(0, dotIndex); | |
| return; | |
| } | |
| e.target.select(); | |
| }} | |
| onBlur={() => node.reset()} | |
| onKeyDown={(e) => { | |
| if (e.key === "Enter") { | |
| const newName = e.currentTarget.value.trim(); | |
| if (newName && newName !== data.name) { | |
| node.submit(newName); | |
| } else { | |
| node.reset(); | |
| } | |
| } | |
| if (e.key === "Escape") { | |
| node.reset(); | |
| } | |
| }} | |
| <input | |
| type="text" | |
| defaultValue={data.name} | |
| onFocus={(e) => { | |
| const dotIndex = data.name.lastIndexOf("."); | |
| if (dotIndex > 0) { | |
| e.target.setSelectionRange(0, dotIndex); | |
| return; | |
| } | |
| e.target.select(); | |
| }} | |
| onBlur={() => node.reset()} | |
| onKeyDown={(e) => { | |
| e.stopPropagation(); | |
| if (e.key === "Enter") { | |
| e.preventDefault(); | |
| const newName = e.currentTarget.value.trim(); | |
| if (newName && newName !== data.name) { | |
| node.submit(newName); | |
| } else { | |
| node.reset(); | |
| } | |
| } | |
| if (e.key === "Escape") { | |
| e.preventDefault(); | |
| node.reset(); | |
| } | |
| }} |
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileSearchResultNode/FileSearchResultNode.tsx`
around lines 97 - 121, The input's onKeyDown handler in FileSearchResultNode
allows key events to bubble to the parent row, causing Enter/Space to trigger
row handlers; update the onKeyDown callback (the one that checks e.key ===
"Enter" / "Escape" and calls node.submit(node.reset())) to call
e.stopPropagation() at the start of the handler so key events during renaming do
not propagate to the parent row and interfere with activation/toggle behavior.
| <button | ||
| type="button" | ||
| onClick={handleClearSearch} | ||
| className="absolute right-1 top-1/2 -translate-y-1/2 p-0.5 rounded text-muted-foreground hover:text-foreground hover:bg-muted-foreground/20 transition-colors" | ||
| > | ||
| <LuX className="size-3.5" /> | ||
| </button> |
There was a problem hiding this comment.
Add an accessible label to the clear button.
Icon-only buttons need an aria-label so screen readers can announce the action.
✅ Suggested fix
{localSearchTerm && (
<button
type="button"
onClick={handleClearSearch}
+ aria-label="Clear search"
className="absolute right-1 top-1/2 -translate-y-1/2 p-0.5 rounded text-muted-foreground hover:text-foreground hover:bg-muted-foreground/20 transition-colors"
>
<LuX className="size-3.5" />
</button>📝 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.
| <button | |
| type="button" | |
| onClick={handleClearSearch} | |
| className="absolute right-1 top-1/2 -translate-y-1/2 p-0.5 rounded text-muted-foreground hover:text-foreground hover:bg-muted-foreground/20 transition-colors" | |
| > | |
| <LuX className="size-3.5" /> | |
| </button> | |
| <button | |
| type="button" | |
| onClick={handleClearSearch} | |
| aria-label="Clear search" | |
| className="absolute right-1 top-1/2 -translate-y-1/2 p-0.5 rounded text-muted-foreground hover:text-foreground hover:bg-muted-foreground/20 transition-colors" | |
| > | |
| <LuX className="size-3.5" /> | |
| </button> |
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/components/FileTreeToolbar/FileTreeToolbar.tsx`
around lines 95 - 101, The clear-search button in FileTreeToolbar is icon-only
and needs an accessible label: update the button element (the one calling
handleClearSearch and containing <LuX />) to include an appropriate aria-label
(e.g., aria-label="Clear search" or aria-label="Clear file search") and
optionally a title attribute for tooltip support so screen readers and keyboard
users know its purpose.
1 participant
Summary by CodeRabbit
New Features
UI/UX Improvements
✏️ Tip: You can customize this high-level summary in your review settings.