Co-authored-by: adamelmore <2363879+adamdottv@users.noreply.github.com> Co-authored-by: David Hill <iamdavidhill@gmail.com>
7.2 KiB
7.2 KiB
Session Review Cross-Diff Search Plan
One search input for all diffs in the review pane
Goal
Add a single search UI to SessionReview that searches across all diff files in the accordion and supports next/previous navigation across files.
Navigation should auto-open the target accordion item and reveal the active match inside the existing unified File diff viewer.
Non-goals
- Do not change diff rendering visuals, line comments, or file selection behavior.
- Do not add regex, fuzzy search, or replace.
- Do not change
@pierre/diffsinternals.
Current behavior
SessionReviewrenders oneFilediff viewer per accordion item, but only mounts the viewer when that item is expanded.- Large diffs may be blocked behind the
MAX_DIFF_CHANGED_LINESgate until the user clicks "render anyway". Fileowns a local search engine (createFileFind) with:- query state
- hit counting
- current match index
- highlighting (CSS Highlight API or overlay fallback)
Cmd/Ctrl+FandCmd/Ctrl+Gkeyboard handling
FileSearchBaris currently rendered per viewer.- There is no parent-level search state in
SessionReview.
UX requirements
- Add one search bar in the
SessionReviewheader (input, total count, prev, next, close). - Show a global count like
3/17across all searchable diffs. Cmd/Ctrl+Finside the session review pane opens the session-level search.Cmd/Ctrl+G,Shift+Cmd/Ctrl+G,Enter, andShift+Enternavigate globally.- Navigating to a match in a collapsed file auto-expands that file.
- The active match scrolls into view and is highlighted in the target viewer.
- Media/binary diffs are excluded from search.
- Empty query clears highlights and resets to
0/0.
Architecture proposal
Use a hybrid model:
- A session-level match index for global searching/counting/navigation across all diffs.
- The existing per-viewer search engine for local highlighting and scrolling in the active file.
This avoids mounting every accordion item just to search while reusing the existing DOM highlight behavior.
High-level pieces
SessionReviewowns the global query, hit list, and active hit index.Fileexposes a small controlled search handle (register, set query, clear, reveal hit).SessionReviewkeeps a map of mounted file viewers and their search handles.SessionReviewresolves next/prev hits, expands files as needed, then tells the target viewer to reveal the hit.
Data model and interfaces
type SessionSearchHit = {
file: string
side: "additions" | "deletions"
line: number
col: number
len: number
}
type SessionSearchState = {
query: string
hits: SessionSearchHit[]
active: number
}
type FileSearchReveal = {
side: "additions" | "deletions"
line: number
col: number
len: number
}
type FileSearchHandle = {
setQuery: (value: string) => void
clear: () => void
reveal: (hit: FileSearchReveal) => boolean
refresh: () => void
}
type FileSearchControl = {
shortcuts?: "global" | "disabled"
showBar?: boolean
register: (handle: FileSearchHandle | null) => void
}
Integration steps
Phase 1: Expose controlled search on File
- Extend
createFileFindandFileto support a controlled search handle. - Keep existing per-viewer search behavior as the default path.
- Add a way to disable per-viewer global shortcuts when hosted inside
SessionReview.
Acceptance
Filestill supports local search unchanged by default.Filecan optionally register a search handle and accept controlled reveal calls.
Phase 2: Add session-level search state in SessionReview
- Add a single search UI in the
SessionReviewheader (can reuseFileSearchBarvisuals or extract shared presentational pieces). - Build a global hit list from
props.diffsstring content. - Index hits by file/side/line/column/length.
Acceptance
- Header search appears once for the pane.
- Global hit count updates as query changes.
- Media/binary diffs are excluded.
Phase 3: Wire global navigation to viewers
- Register a
FileSearchHandleper mounted diff viewer. - On next/prev, resolve the active global hit and:
- expand the target file if needed
- wait for the viewer to mount/render
- call
handle.setQuery(query)andhandle.reveal(hit)
Acceptance
- Next/prev moves across files.
- Collapsed targets auto-open.
- Active match is highlighted in the target diff.
Phase 4: Handle large-diff gating
- Lift
render anywaystate from local accordion item state into a file-keyed map inSessionReview. - If navigation targets a gated file, force-render it before reveal.
Acceptance
- Global search can navigate into a large diff without manual user expansion/render.
Phase 5: Keyboard and race-condition polish
- Route
Cmd/Ctrl+F,Cmd/Ctrl+G,Shift+Cmd/Ctrl+Gto session search when focus is in the review pane. - Add token/cancel guards so fast navigation does not reveal stale targets after async mounts.
Acceptance
- Keyboard shortcuts consistently target session-level search.
- No stale reveal jumps during rapid navigation.
Edge cases
- Empty query: clear all viewer highlights, reset count/index.
- No results: keep the search bar open, disable prev/next.
- Added/deleted files: index only the available side.
- Collapsed files: queue reveal until
onRenderedfires. - Large diffs: auto-force render before reveal.
- Split diff mode: handle duplicate text on both sides without losing side info.
- Do not clear line comment draft or selected lines when navigating search results.
Testing plan
Unit tests
- Session hit-index builder:
- line/column mapping
- additions/deletions side tagging
- wrap-around next/prev behavior
Filecontrolled search handle:setQueryclearrevealby side/line/column in unified and split diff
Component / integration tests
- Search across multiple diffs and navigate across collapsed accordion items.
- Global counter updates correctly (
current/total). - Split and unified diff styles both navigate correctly.
- Large diff target auto-renders on navigation.
- Existing line comment draft remains intact while searching.
Manual verification
Cmd/Ctrl+Fopens session-level search in the review pane.Cmd/Ctrl+G/Shift+Cmd/Ctrl+Gnavigate globally.- Highlighting and scroll behavior stay stable with many open diffs.
Risks and rollback
Key risks
- Global index and DOM highlights can drift if line/column mapping does not match viewer DOM content exactly.
- Keyboard shortcut conflicts between session-level search and per-viewer search.
- Performance impact when indexing many large diffs in one session.
Rollback plan
- Gate session-level search behind a
SessionReviewprop/flag during rollout. - If unstable, disable the session-level path and keep existing per-viewer search unchanged.
Open questions
- Should search match file paths as well as content, or content only?
- In split mode, should the same text on both sides count as two matches?
- Should auto-navigation into gated large diffs silently render them, or show a prompt first?
- Should the session-level search bar reuse
FileSearchBardirectly or split out a shared non-portal variant?