feat(web): add find-in-thread search with Cmd+F#3779
Conversation
Restore thread-local search with highlighted matches, next/previous navigation, and scroll-to-active-match behavior in the chat timeline. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
github-actions
Bot
added
vouch:unvouched
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 6 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3175584. Configure here.
| results.push({ messageId: message.id, match }); | ||
| } | ||
| } | ||
| return results; |
There was a problem hiding this comment.
Find counts hidden user text
Medium Severity
The "Find in Thread" feature searches the raw message.text, but the UI renders a stripped version of this text (e.g., without terminal/element context). This causes match counts and navigation to include text that isn't visible or highlightable in the chat.
Reviewed by Cursor Bugbot for commit 3175584. Configure here.
| } | ||
| } | ||
| return results; | ||
| }, [findQuery, findOpen, timelineMessagesFlat]); |
There was a problem hiding this comment.
Proposed plans omitted from matches
Medium Severity
The "Find in Thread" feature only counts and navigates matches within chat messages. This means that while ProposedPlanCard content can still highlight matching text, these matches are excluded from the total count, next/previous navigation, and active-match styling, resulting in a confusing 0/0 display despite visible highlights in plans.
Reviewed by Cursor Bugbot for commit 3175584. Configure here.
| element?.scrollIntoView({ behavior: "smooth", block: "center" }); | ||
| } | ||
| return next; | ||
| }); |
There was a problem hiding this comment.
Folded turn matches not reachable
Medium Severity
The "Find in Thread" next/previous navigation silently fails to scroll to matches. This occurs because it relies on document.querySelector for message elements, which are not present in the DOM when messages are part of collapsed turns or unmounted by the LegendList's virtualization.
Reviewed by Cursor Bugbot for commit 3175584. Configure here.
| const handleFindQueryChange = useCallback((query: string) => { | ||
| setFindQuery(query); | ||
| setFindActiveMatchIndex(0); | ||
| }, []); |
There was a problem hiding this comment.
Active match index not clamped
Medium Severity
findActiveMatchIndex resets when the query changes or find closes, but not when the match list shrinks (switching threads, reverting messages, etc.). The UI can show an index above totalMatches, and findMatches[findActiveMatchIndex] becomes undefined so no row gets data-search-active-match.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 3175584. Configure here.
| } | ||
| return next; | ||
| }); | ||
| }, [findTotalMatches, findMessageIdByMatchIndex]); |
There was a problem hiding this comment.
First Enter skips first match
Low Severity
After typing a query the active index starts at 0, but Enter runs handleFindNext, which moves to index 1 when there are multiple hits. The first match is never scrolled into view via Enter, and the first Enter jumps to the second occurrence.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 3175584. Configure here.
| () => ({ | ||
| mark({ children }) { | ||
| return <mark className="search-highlight">{children}</mark>; | ||
| }, |
There was a problem hiding this comment.
Task checkbox offsets break with search
Medium Severity
Task list items compute data-task-marker-offset from the original text prop while ReactMarkdown parses highlightedText with injected mark tags. With an active searchQuery, AST offsets no longer align with findTaskListMarkerOffset(text, …), so toggling tasks can target the wrong checkbox.
Reviewed by Cursor Bugbot for commit 3175584. Configure here.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| const TimelineRowContent = memo(function TimelineRowContent({ row }: { row: TimelineRow }) { | ||
| const ctx = use(TimelineRowCtx); | ||
| const messageId = row.kind === "message" ? row.message.id : null; | ||
| const hasActiveMatch = |
There was a problem hiding this comment.
🟡 Medium chat/MessagesTimeline.tsx:816
When a single message contains multiple search matches, data-search-active-match is applied to the entire message row, so every <mark class="search-highlight"> inside it gets active styling simultaneously. Advancing findActiveMatchIndex within the same message highlights all occurrences instead of just the active one, so the per-match highlight is wrong for repeated matches. The check at line 820 only compares messageId, not the specific match index within the message. Consider tracking the per-message match index so only the currently active <mark> receives the active styling.
Also found in 1 other location(s)
apps/web/src/index.css:906
The selector
[data-search-active-match] mark.search-highlightat line906styles every highlighted occurrence inside the active message as the current match.MessagesTimelineonly setsdata-search-active-matchon the message row whosemessageIdownsfindActiveMatchIndex, so if one message contains multiple matches, all of them get the active outline/background instead of just the selected occurrence. This breaks next/previous navigation because the UI no longer identifies which match is actually active.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/MessagesTimeline.tsx around line 816:
When a single message contains multiple search matches, `data-search-active-match` is applied to the entire message row, so every `<mark class="search-highlight">` inside it gets active styling simultaneously. Advancing `findActiveMatchIndex` within the same message highlights all occurrences instead of just the active one, so the per-match highlight is wrong for repeated matches. The check at line 820 only compares `messageId`, not the specific match index within the message. Consider tracking the per-message match index so only the currently active `<mark>` receives the active styling.
Also found in 1 other location(s):
- apps/web/src/index.css:906 -- The selector `[data-search-active-match] mark.search-highlight` at line `906` styles **every** highlighted occurrence inside the active message as the current match. `MessagesTimeline` only sets `data-search-active-match` on the message row whose `messageId` owns `findActiveMatchIndex`, so if one message contains multiple matches, all of them get the active outline/background instead of just the selected occurrence. This breaks next/previous navigation because the UI no longer identifies which match is actually active.
| /> | ||
| {query && ( | ||
| <span className="whitespace-nowrap tabular-nums text-muted-foreground/60"> | ||
| {hasMatches ? `${matchIndex + 1}/${totalMatches}` : "0/0"} |
There was a problem hiding this comment.
🟡 Medium chat/FindInThread.tsx:91
The match counter renders matchIndex + 1 without clamping, so when the thread contents change while find is open (e.g., a streaming message update reduces the hit count) the counter can show impossible values like 5/2 because matchIndex is only reset on open/close/query change. In that state the displayed position no longer corresponds to any real match, and the counter stays inconsistent until the user manually navigates. Consider clamping matchIndex to totalMatches - 1 (or resetting it to 0) when totalMatches drops below the current index.
| {hasMatches ? `${matchIndex + 1}/${totalMatches}` : "0/0"} | |
| {hasMatches ? `${Math.min(matchIndex, totalMatches - 1) + 1}/${totalMatches}` : "0/0"} |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/FindInThread.tsx around line 91:
The match counter renders `matchIndex + 1` without clamping, so when the thread contents change while find is open (e.g., a streaming message update reduces the hit count) the counter can show impossible values like `5/2` because `matchIndex` is only reset on open/close/query change. In that state the displayed position no longer corresponds to any real match, and the counter stays inconsistent until the user manually navigates. Consider clamping `matchIndex` to `totalMatches - 1` (or resetting it to 0) when `totalMatches` drops below the current index.
| let pos = 0; | ||
| while ((pos = lowerText.indexOf(lowerQuery, pos)) !== -1) { | ||
| matches.push({ start: pos, end: pos + lowerQuery.length }); | ||
| pos += 1; |
There was a problem hiding this comment.
🟡 Medium lib/searchHighlight.ts:18
findAllMatches advances pos by only 1, so for query aa in text aaa it returns two matches at offsets 0 and 1. highlightHtml uses global regex replacement that marks only non-overlapping occurrences, so the totalMatches count and navigation positions include matches that are never highlighted, causing the find bar to show more matches than are actually marked. Advancing pos by lowerQuery.length makes the matches align with what highlightHtml renders.
| pos += 1; | |
| pos += lowerQuery.length; |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/lib/searchHighlight.ts around line 18:
`findAllMatches` advances `pos` by only `1`, so for query `aa` in text `aaa` it returns two matches at offsets `0` and `1`. `highlightHtml` uses global regex replacement that marks only non-overlapping occurrences, so the `totalMatches` count and navigation positions include matches that are never highlighted, causing the find bar to show more matches than are actually marked. Advancing `pos` by `lowerQuery.length` makes the matches align with what `highlightHtml` renders.
| } | ||
|
|
||
| /** Wraps all occurrences of `query` in `text` with `<mark>` tags. */ | ||
| export function highlightHtml(text: string, query: string): string { |
There was a problem hiding this comment.
🟠 High lib/searchHighlight.ts:24
highlightHtml wraps matches with literal <mark> tags before the text is passed to a markdown parser, so those tags end up as raw text inside fenced code blocks. A hit inside code renders as <mark class="search-highlight">foo</mark> instead of foo, corrupting the displayed snippet and any copied source. Consider highlighting after parsing (e.g., marking rendered text nodes) rather than injecting HTML into the markdown source.
Also found in 1 other location(s)
apps/web/src/components/ChatMarkdown.tsx:1262
Wrapping matches with
<mark>insidehighlightedTextmutates fenced code block contents before markdown parsing. If the search term occurs inside a code fence,extractCodeBlock(children)receives code containing literal<mark class="search-highlight">...</mark>text, so the rendered code block is changed by the search feature instead of merely highlighted. Copying or reading code while search is active shows corrupted source text.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/lib/searchHighlight.ts around line 24:
`highlightHtml` wraps matches with literal `<mark>` tags before the text is passed to a markdown parser, so those tags end up as raw text inside fenced code blocks. A hit inside code renders as `<mark class="search-highlight">foo</mark>` instead of `foo`, corrupting the displayed snippet and any copied source. Consider highlighting after parsing (e.g., marking rendered text nodes) rather than injecting HTML into the markdown source.
Also found in 1 other location(s):
- apps/web/src/components/ChatMarkdown.tsx:1262 -- Wrapping matches with `<mark>` inside `highlightedText` mutates fenced code block contents before markdown parsing. If the search term occurs inside a code fence, `extractCodeBlock(children)` receives code containing literal `<mark class="search-highlight">...</mark>` text, so the rendered code block is changed by the search feature instead of merely highlighted. Copying or reading code while search is active shows corrupted source text.
ApprovabilityVerdict: Needs human review 4 blocking correctness issues found. This PR introduces a new user-facing search feature with multiple unresolved review comments identifying bugs, including a high-severity issue where search highlighting corrupts code blocks by injecting HTML into markdown source. Human review recommended to address these issues before merging. You can customize Macroscope's approvability policy. Learn more. |
1 participant
Summary
Test plan
Made with Cursor
Note
Add find-in-thread search triggered by Cmd+F in chat view
FindInThreadUI bar that opens with Cmd/Ctrl+F and closes with Escape, with next/previous match navigation via buttons or Enter/Shift+Enter.searchHighlight.tswithfindAllMatchesandhighlightHtmlutilities for case-insensitive substring detection and<mark>tag injection.findQueryand match state throughMessagesTimelinecontext to highlight matches in assistant messages, user messages, and proposed plan cards viaChatMarkdown.ChatMarkdown.tsxto allow<mark>tags, and adds CSS inindex.cssto style highlighted and active matches with scroll-margin support.📊 Macroscope summarized 3175584. 6 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.