fix: Highlight focused AI block matches under async find#11205
fix: Highlight focused AI block matches under async find#11205vkodithala wants to merge 2 commits into
Conversation
When the `AsyncFind` feature flag is enabled, pressing up/down on the find bar advances the current/total counter through AI block matches but never highlights one, never scrolls to it, and never auto-expands a collapsed reasoning block containing it. The cached focused match in `AsyncFindController` was terminal-only, so AI rendering and reasoning auto-expand (which both look up the focused `RichContentMatchId` via `BlockListFindRun`) saw nothing. Refactor the controller's focused-match cache to a single `Option<FocusedMatchResolution>` that resolves to either a terminal or an AI match. Plumb the AI variant through `TerminalFindModel::focused_block_list_match()` so `scroll_to_match` routes to `FindMatchScrollLocation::RichContent`, and add a path-agnostic `focused_rich_content_match_id()` helper used by both AI highlight rendering and the reasoning-block auto-expand handler. AI matches inside one block are walked bottom-to-top for `MostRecentLast` to match sync find's per-AI-block traversal. Co-Authored-By: Oz <oz-agent@warp.dev>
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR updates async find so focused AI block matches resolve through the same rich-content match ID path used by sync find, enabling focused-match highlighting, scrolling, and collapsed reasoning auto-expand behavior. The code/security review did not find a diff-line correctness or security issue.
Concerns
- For this user-facing change, please include screenshots or a screen recording demonstrating the focused AI match highlight, scroll-to-match, and reasoning auto-expand flow working end to end. The PR currently marks screenshots/videos as N/A, but this behavior is visually observable even if it is gated by the AsyncFind dogfood flag.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
Per review feedback: the only caller of `compute_focused_match` was the single-line wrapper `update_cached_focused_match`. Inline the body so the\nresolution writes directly into `self.cached_focused_match` instead of\nbouncing through an intermediate `Option<FocusedMatchResolution>` return.\n\nCo-Authored-By: Oz <oz-agent@warp.dev>
vkodithala
changed the title
| /// The id of the focused match within the rich content block. | ||
| pub match_id: RichContentMatchId, | ||
| /// The total index of the rich content block in the blocklist sumtree. | ||
| pub total_index: TotalIndex, |
There was a problem hiding this comment.
we might want to clarify in the doc comment for the struct what the expected lifetime is, as these two values can get stale as new output is streamed in/blocks get created.
as-is, nothing prevents an AsyncFocusedAiMatch from being stored somewhere and getting stale. if it's intended to be a short-lived/transient state holder, we may want to consider removing Clone, so that we can return a reference to it, but nothing outside of this module can ever hold onto one.
| } | ||
|
|
||
| /// Returns the focused match as an AsyncBlockGridMatch if it's a terminal match. | ||
| /// Returns the focused match as an `AsyncBlockGridMatch` if it's a terminal match. |
There was a problem hiding this comment.
nit/tip: further wrapping this in brackets (e.g.: "[`" with a pair at the end) will linkify it, if the item within the brackets is a symbol in the current scope.
2 participants
Description
Highlights focused AI block matches under async find. With the
AsyncFindfeature flag enabled (default-off for dogfood), pressing up/down on the find bar advances thecurrent/totalcounter through AI matches but never (1) applies the focused-match highlight color to the AI match, (2) scrolls to the AI block containing the focused match, or (3) auto-expands a collapsed reasoning block containing it. Sync find handles all three correctly.Root cause:
AsyncFindController's focused-match cache was terminal-only, so the AI-rendering call site and the reasoning auto-expand handler — both of which look up the focusedRichContentMatchIdvia the syncBlockListFindRun— seeNonewhenever async find is active.Fix:
Option<FocusedMatchResolution>that resolves to either a terminal grid match or an AI match, replacing the previous terminal-onlyOption<AsyncBlockGridMatch>.focused_terminal_match()andfocused_ai_match()are thin pattern-matching accessors over the unified cache.TerminalFindModel::focused_block_list_match()soTerminalView::scroll_to_matchroutes toFindMatchScrollLocation::RichContentfor AI matches.TerminalFindModel::focused_rich_content_match_id()helper used by bothget_highlight_ranges_for_find_matches(highlight rendering) andAIBlock::handle_find_match_focus_change(reasoning auto-expand) in place of their previous sync-only lookups.MostRecentLastso per-AI-block focus traversal matches sync find.Linked Issue
N/A; found while dogfooding.
Testing
./script/runAdded three unit tests in
async_find_tests.rs:test_focused_ai_match_resolves_only_ai_block— seeds a single AI block with two matches and verifiesfocused_ai_match()resolves correctly underMostRecentLast(per-block iteration reversal).test_focused_ai_match_most_recent_first_preserves_storage_order— same fixture underMostRecentFirstto exercise un-reversed iteration.test_focused_match_index_walks_across_terminal_and_ai_blocks— mixed terminal + AI fixture asserting the unified cache holds at most one variant at a time and the focused index walks across both block types in TotalIndex-descending order.Existing parity tests (
test_async_focused_order_matches_sync_*) and all 18 prior async-find tests continue to pass (21 total).Verified:
cargo check -p warp --tests→ cleancargo clippy -p warp --tests -- -D warnings→ cleancargo fmt -p warp→ cleancargo test -p warp --lib async_find→ 21 passedScreenshots / Videos
Manually tested; demo here:
Screen.Recording.2026-05-18.at.12.21.36.PM.mov
First half is the existing implementation; notice how matches in AI blocks are highlighted, but not properly focused (aka given orange highlights and scrolled to) when the match index changes. Second half (new implementation) fixes this.
There's a KI (that's around for the sync implementation, too) around match highlights for AI blocks not being dismissed when the find bar is. i.e., if I type a find query with matches in AI blocks, then dismiss that find bar with
esc, matches in AI blocks aren't dismissed until I refocus the pane being searched. Matches in terminal blocks are handled fine. I plan on fixing this in a stacked PR.Agent Mode