⚡ Bolt: Fast bitflag early-exit in Burst Search#587
Conversation
Co-authored-by: AhmmedSamier <[email protected]>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughBurst search now performs separate name and fullName bitflag checks, derives lowercase names only when needed, and gates fallback substring, prefix, and fuzzy scoring accordingly. The optimization is documented in the changelog note. ChangesBurst search optimization
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af0c00b8c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!namePasses && !fullPasses) { | ||
| return; |
There was a problem hiding this comment.
Preserve relative-path matches for file items
When the query only matches a file's directory/relative path, this early return drops the result from burstSearch. Indexed file items are created with fullName: relativePath, but computeItemBitflags() leaves itemFullNameBitflags at 0 when fullName === relativeFilePath, so fullPasses is false even though the old calculateMatchScore() would still search item.fullName by lowercasing it. In the inspected workspace-indexer flow, a file like src/foo/Bar.ts will no longer be returned by burst search for foo unless the basename also contains those characters.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@language-server/src/core/search-engine.ts`:
- Around line 2465-2472: In the search-result scoring flow after the early
return, replace the redundant namePasses/fullPasses conditional with a const
nameLower that directly selects the prepared target or lowercased item name. Use
single quotes for the empty-string literal and reformat the calculateMatchScore
call to stay within 120 characters; then run bun run test in language-server.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c00b8af9-d30c-4d19-b42f-2f57d1868e24
📒 Files selected for processing (2)
.jules/bolt.mdlanguage-server/src/core/search-engine.ts
| let nameLower = ""; | ||
| if (namePasses || fullPasses) { | ||
| nameLower = prepared | ||
| ? (prepared as unknown as ExtendedPrepared)._targetLower | ||
| : item.name.toLowerCase(); | ||
| } | ||
|
|
||
| const score = this.calculateMatchScore(nameLower, item.fullName, this.preparedFullNames[i], queryLower); | ||
| const score = this.calculateMatchScore(nameLower, item.fullName, this.preparedFullNames[i], queryLower, namePasses, fullPasses); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Simplify redundant condition and fix formatting issues.
The if (namePasses || fullPasses) check is redundant because the function already returns early at line 2457 if both are false. You can simplify this by initializing nameLower as a const.
Additionally, the long function call exceeds the line length limit, and the initial empty string uses double quotes instead of single quotes.
As per coding guidelines, use single quotes for strings, limit line length to 120 characters, and run bun run test in language-server after changing the search engine to verify correctness.
♻️ Proposed fix
- let nameLower = "";
- if (namePasses || fullPasses) {
- nameLower = prepared
- ? (prepared as unknown as ExtendedPrepared)._targetLower
- : item.name.toLowerCase();
- }
-
- const score = this.calculateMatchScore(nameLower, item.fullName, this.preparedFullNames[i], queryLower, namePasses, fullPasses);
+ const nameLower = prepared
+ ? (prepared as unknown as ExtendedPrepared)._targetLower
+ : item.name.toLowerCase();
+
+ const score = this.calculateMatchScore(
+ nameLower,
+ item.fullName,
+ this.preparedFullNames[i],
+ queryLower,
+ namePasses,
+ fullPasses
+ );📝 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.
| let nameLower = ""; | |
| if (namePasses || fullPasses) { | |
| nameLower = prepared | |
| ? (prepared as unknown as ExtendedPrepared)._targetLower | |
| : item.name.toLowerCase(); | |
| } | |
| const score = this.calculateMatchScore(nameLower, item.fullName, this.preparedFullNames[i], queryLower); | |
| const score = this.calculateMatchScore(nameLower, item.fullName, this.preparedFullNames[i], queryLower, namePasses, fullPasses); | |
| const nameLower = prepared | |
| ? (prepared as unknown as ExtendedPrepared)._targetLower | |
| : item.name.toLowerCase(); | |
| const score = this.calculateMatchScore( | |
| nameLower, | |
| item.fullName, | |
| this.preparedFullNames[i], | |
| queryLower, | |
| namePasses, | |
| fullPasses | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@language-server/src/core/search-engine.ts` around lines 2465 - 2472, In the
search-result scoring flow after the early return, replace the redundant
namePasses/fullPasses conditional with a const nameLower that directly selects
the prepared target or lowercased item name. Use single quotes for the
empty-string literal and reformat the calculateMatchScore call to stay within
120 characters; then run bun run test in language-server.
Source: Coding guidelines
💡 What: Added property-specific bitflag checks (
itemNameBitflagsanditemFullNameBitflags) tofindBurstMatchesbefore evaluating the fallback string methods incalculateMatchScore.🎯 Why: Items could pass the aggregate
itemBitflagscheck if characters only existed in therelativeFilePath. This resulted in unnecessary string evaluations (nameLower.indexOf(...)) when the item logically could not match the query in thenameorfullNameproperties evaluated during burst search.📊 Impact: Speeds up burst search by safely aborting string evaluation when query characters are missing from target substrings, saving string allocations and evaluation cycles.
🔬 Measurement: Verified via
bun testinlanguage-server/src/core/search-engine.test.ts(especially the burst search coverage tests), all passing correctly.PR created automatically by Jules for task 641604326408207145 started by @AhmmedSamier
Summary by CodeRabbit