Fix search case sensitivity in file search#231
Conversation
WalkthroughUpdates search argument construction for file-name searches to use --iglob/--glob based on ignoreCase, while keeping -i for content searches. Adds ignoreCase to StartSearch tool documentation. Tests import and invoke handleStopSearch to clean up sessions and remove a redundant apiLimit. Exposes handleStopSearch from dist/handlers/search-handlers.js. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Server as Server (StartSearch)
participant Manager as SearchManager
participant RG as ripgrep
Client->>Server: StartSearch({ query, searchType, ignoreCase })
Server->>Manager: buildArgs(searchType, ignoreCase, pattern)
alt searchType === "files"
Note over Manager: Choose globFlag = ignoreCase !== false ? --iglob : --glob
Manager->>RG: rg [globFlag pattern ...]
else searchType === "content"
Note over Manager: Use -i for case-insensitive content search
Manager->>RG: rg [-i] pattern ...
end
RG-->>Manager: matches/none
Manager-->>Server: results
Server-->>Client: results
sequenceDiagram
autonumber
participant Test as Test Runner
participant Search as Search Session
participant Stop as handleStopSearch
Test->>Search: start search and await completion
Note over Test: finally cleanup
Test->>Stop: handleStopSearch({ sessionId })
Stop-->>Test: Promise resolved (errors ignored)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/search-manager.ts (1)
305-378: Respect maxResults for files by early-stopping the rg process
-mdoesn’t limit--files. Without early termination, large trees will be scanned fully. Kill the process once collected results reachmaxResults.Add this guard in
processBufferedOutput(outside this hunk):// After incrementing totals/pushing a file result: if ( session.options.searchType === 'files' && session.options.maxResults && session.totalMatches >= session.options.maxResults && !session.process.killed ) { try { session.process.kill('SIGTERM'); } catch {} }Optionally also short-circuit parsing once killed.
🧹 Nitpick comments (2)
src/server.ts (1)
321-322: Add content-mode examples mirroring files-modeConsider adding examples for case-sensitive/insensitive content search to match the new files examples, e.g.,
searchType="content", pattern="README", ignoreCase=falseandignoreCase=true.src/search-manager.ts (1)
357-368: Substring matching now applies to full paths, not just basenames
--(i)glob "*pattern*"matches anywhere in the path. If the UX promise is “file names,” this broadens scope (e.g., matches by directory names). Either (a) document that matching applies to full paths, or (b) post-filter by basename before storing results.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/search-manager.ts(2 hunks)src/server.ts(1 hunks)test/test_improved_search_truncation.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/search-manager.ts (2)
src/tools/search.ts (2)
searchCodeFallback(149-257)searchCode(16-146)src/tools/filesystem.ts (2)
searchFilesNodeJS(972-1026)searchFiles(905-969)
test/test_improved_search_truncation.js (3)
test/test-search-code-edge-cases.js (2)
teardownEdgeCases(113-141)searchAndWaitForCompletion(29-66)src/handlers/search-handlers.ts (1)
handleStopSearch(171-206)test/test_search_truncation.js (1)
searchAndWaitForCompletion(7-44)
🔇 Additional comments (3)
src/server.ts (1)
315-315: Clarify or implement case-insensitive content filtering
TheignoreCaseoption currently only applies to file names—content searches still use-g(case-sensitive), so the docs are incorrect. Either switch to--iglobfor content whenignoreCase !== falseor update the doc to state that content filters remain case-sensitive.src/search-manager.ts (1)
320-323: Separation of case handling is correctUsing
-ifor content and glob-based flags for files is sound. No issues here.test/test_improved_search_truncation.js (1)
2-2: Good: explicit session cleanupImporting and calling
handleStopSearchprevents dangling searches. Nice addition.
| // For file search: use --iglob for case-insensitive or --glob for case-sensitive | ||
| if (options.ignoreCase !== false) { | ||
| args.push('--iglob', p); | ||
| } else { | ||
| args.push('--glob', p); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make content-mode file filters honor ignoreCase (use --iglob/--glob)
Currently content mode uses -g (case-sensitive). Switch to --iglob/--glob to align with docs and files-mode behavior.
Apply this diff:
- if (options.searchType === 'content') {
- args.push('-g', p);
- } else {
+ if (options.searchType === 'content') {
+ if (options.ignoreCase !== false) {
+ args.push('--iglob', p);
+ } else {
+ args.push('--glob', p);
+ }
+ } else {
// For file search: use --iglob for case-insensitive or --glob for case-sensitive
if (options.ignoreCase !== false) {
args.push('--iglob', p);
} else {
args.push('--glob', p);
}
}📝 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.
| // For file search: use --iglob for case-insensitive or --glob for case-sensitive | |
| if (options.ignoreCase !== false) { | |
| args.push('--iglob', p); | |
| } else { | |
| args.push('--glob', p); | |
| } | |
| } | |
| if (options.searchType === 'content') { | |
| if (options.ignoreCase !== false) { | |
| args.push('--iglob', p); | |
| } else { | |
| args.push('--glob', p); | |
| } | |
| } else { | |
| // For file search: use --iglob for case-insensitive or --glob for case-sensitive | |
| if (options.ignoreCase !== false) { | |
| args.push('--iglob', p); | |
| } else { | |
| args.push('--glob', p); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/search-manager.ts around lines 344 to 350, the content-mode file filter
currently uses the short case-sensitive -g flag; update logic to use long flags
--iglob for case-insensitive (when options.ignoreCase !== false) and --glob for
case-sensitive to match docs and files-mode behavior. Replace the '-g' usage for
content-mode filters with the conditional push of '--iglob' or '--glob' (same
predicate as files-mode) so file filter behavior honors options.ignoreCase.
1 participant
Summary by CodeRabbit