diff options
| author | Adam <[email protected]> | 2026-02-26 18:23:04 -0600 |
|---|---|---|
| committer | GitHub <[email protected]> | 2026-02-26 18:23:04 -0600 |
| commit | fc52e4b2d3a41efde772e6de8fb2e01f27821701 (patch) | |
| tree | cf23af294a00a10e55f230232585344c111f0bb9 /specs | |
| parent | 9a6bfeb782766099d4ce3a98bb9e7b4e79f8bfe6 (diff) | |
| download | opencode-fc52e4b2d3a41efde772e6de8fb2e01f27821701.tar.gz opencode-fc52e4b2d3a41efde772e6de8fb2e01f27821701.zip | |
feat(app): better diff/code comments (#14621)
Co-authored-by: adamelmore <[email protected]>
Co-authored-by: David Hill <[email protected]>
Diffstat (limited to 'specs')
| -rw-r--r-- | specs/file-component-unification-plan.md | 426 | ||||
| -rw-r--r-- | specs/session-review-cross-diff-search-plan.md | 234 |
2 files changed, 660 insertions, 0 deletions
diff --git a/specs/file-component-unification-plan.md b/specs/file-component-unification-plan.md new file mode 100644 index 000000000..d63d665fd --- /dev/null +++ b/specs/file-component-unification-plan.md @@ -0,0 +1,426 @@ +# File Component Unification Plan + +Single path for text, diff, and media + +--- + +## Define goal + +Introduce one public UI component API that renders plain text files or diffs from the same entry point, so selection, comments, search, theming, and media behavior are maintained once. + +### Goal + +- Add a unified `File` component in `packages/ui/src/components/file.tsx` that chooses plain or diff rendering from props. +- Centralize shared behavior now split between `packages/ui/src/components/code.tsx` and `packages/ui/src/components/diff.tsx`. +- Bring the existing find/search UX to diff rendering through a shared engine. +- Consolidate media rendering logic currently split across `packages/ui/src/components/session-review.tsx` and `packages/app/src/pages/session/file-tabs.tsx`. +- Provide a clear SSR path for preloaded diffs without keeping a third independent implementation. + +### Non-goal + +- Do not change `@pierre/diffs` behavior or fork its internals. +- Do not redesign line comment UX, diff visuals, or keyboard shortcuts. +- Do not remove legacy `Code`/`Diff` APIs in the first pass. +- Do not add new media types beyond parity unless explicitly approved. +- Do not refactor unrelated session review or file tab layout code outside integration points. + +--- + +## Audit duplication + +The current split duplicates runtime logic and makes feature parity drift likely. + +### Duplicate categories + +- Rendering lifecycle is duplicated in `code.tsx` and `diff.tsx`, including instance creation, cleanup, `onRendered` readiness, and shadow root lookup. +- Theme sync is duplicated in `code.tsx`, `diff.tsx`, and `diff-ssr.tsx` through similar `applyScheme` and `MutationObserver` code. +- Line selection wiring is duplicated in `code.tsx` and `diff.tsx`, including drag state, shadow selection reads, and line-number bridge integration. +- Comment annotation rerender flow is duplicated in `code.tsx`, `diff.tsx`, and `diff-ssr.tsx`. +- Commented line marking is split across `markCommentedFileLines` and `markCommentedDiffLines`, with similar timing and effect wiring. +- Diff selection normalization (`fixSelection`) exists twice in `diff.tsx` and `diff-ssr.tsx`. +- Search exists only in `code.tsx`, so diff lacks find and the feature cannot be maintained in one place. +- Contexts are split (`context/code.tsx`, `context/diff.tsx`), which forces consumers to choose paths early. +- Media rendering is duplicated outside the core viewers in `session-review.tsx` and `file-tabs.tsx`. + +### Drift pain points + +- Any change to comments, theming, or selection requires touching multiple files. +- Diff SSR and client diff can drift because they carry separate normalization and marking code. +- Search cannot be added to diff cleanly without more duplication unless the viewer runtime is unified. + +--- + +## Design architecture + +Use one public component with a discriminated prop shape and split shared behavior into small runtime modules. + +### Public API proposal + +- Add `packages/ui/src/components/file.tsx` as the primary client entry point. +- Export a single `File` component that accepts a discriminated union with two primary modes. +- Use an explicit `mode` prop (`"text"` or `"diff"`) to avoid ambiguous prop inference and keep type errors clear. + +### Proposed prop shape + +- Shared props: + - `annotations` + - `selectedLines` + - `commentedLines` + - `onLineSelected` + - `onLineSelectionEnd` + - `onLineNumberSelectionEnd` + - `onRendered` + - `class` + - `classList` + - selection and hover flags already supported by current viewers +- Text mode props: + - `mode: "text"` + - `file` (`FileContents`) + - text renderer options from `@pierre/diffs` `FileOptions` +- Diff mode props: + - `mode: "diff"` + - `before` + - `after` + - `diffStyle` + - diff renderer options from `FileDiffOptions` + - optional `preloadedDiff` only for SSR-aware entry or hydration adapter +- Media props (shared, optional): + - `media` config for `"auto" | "off"` behavior + - path/name metadata + - optional lazy loader (`readFile`) for session review use + - optional custom placeholders for binary or removed content + +### Internal module split + +- `packages/ui/src/components/file.tsx` + Public unified component and mode routing. +- `packages/ui/src/components/file-ssr.tsx` + Unified SSR entry for preloaded diff hydration. +- `packages/ui/src/components/file-search.tsx` + Shared find bar UI and host registration. +- `packages/ui/src/components/file-media.tsx` + Shared image/audio/svg/binary rendering shell. +- `packages/ui/src/pierre/file-runtime.ts` + Common render lifecycle, instance setup, cleanup, scheme sync, and readiness notification. +- `packages/ui/src/pierre/file-selection.ts` + Shared selection/drag/line-number bridge controller with mode adapters. +- `packages/ui/src/pierre/diff-selection.ts` + Diff-specific `fixSelection` and row/side normalization reused by client and SSR. +- `packages/ui/src/pierre/file-find.ts` + Shared find engine (scan, highlight API, overlay fallback, match navigation). +- `packages/ui/src/pierre/media.ts` + MIME normalization, data URL helpers, and media type detection. + +### Wrapper strategy + +- Keep `packages/ui/src/components/code.tsx` as a thin compatibility wrapper over unified `File` in text mode. +- Keep `packages/ui/src/components/diff.tsx` as a thin compatibility wrapper over unified `File` in diff mode. +- Keep `packages/ui/src/components/diff-ssr.tsx` as a thin compatibility wrapper over unified SSR entry. + +--- + +## Phase delivery + +Ship this in small phases so each step is reviewable and reversible. + +### Phase 0: Align interfaces + +- Document the final prop contract and adapter behavior before moving logic. +- Add a short migration note in the plan PR description so reviewers know wrappers stay in place. + +#### Acceptance + +- Final prop names and mode shape are agreed up front. +- No runtime code changes land yet. + +### Phase 1: Extract shared runtime pieces + +- Move duplicated theme sync and render readiness logic from `code.tsx` and `diff.tsx` into shared runtime helpers. +- Move diff selection normalization (`fixSelection` and helpers) out of both `diff.tsx` and `diff-ssr.tsx` into `packages/ui/src/pierre/diff-selection.ts`. +- Extract shared selection controller flow into `packages/ui/src/pierre/file-selection.ts` with mode callbacks for line parsing and normalization. +- Keep `code.tsx`, `diff.tsx`, and `diff-ssr.tsx` behavior unchanged from the outside. + +#### Acceptance + +- `code.tsx`, `diff.tsx`, and `diff-ssr.tsx` are smaller and call shared helpers. +- Line selection, comments, and theme sync still work in current consumers. +- No consumer imports change yet. + +### Phase 2: Introduce unified client entry + +- Create `packages/ui/src/components/file.tsx` and wire it to shared runtime pieces. +- Route text mode to `@pierre/diffs` `File` or `VirtualizedFile` and diff mode to `FileDiff` or `VirtualizedFileDiff`. +- Preserve current performance rules, including virtualization thresholds and large-diff options. +- Keep search out of this phase if it risks scope creep, but leave extension points in place. + +#### Acceptance + +- New unified component renders text and diff with parity to existing components. +- `code.tsx` and `diff.tsx` can be rewritten as thin adapters without behavior changes. +- Existing consumers still work through old `Code` and `Diff` exports. + +### Phase 3: Add unified context path + +- Add `packages/ui/src/context/file.tsx` with `FileComponentProvider` and `useFileComponent`. +- Update `packages/ui/src/context/index.ts` to export the new context. +- Keep `context/code.tsx` and `context/diff.tsx` as compatibility shims that adapt to `useFileComponent`. +- Migrate `packages/app/src/app.tsx` and `packages/enterprise/src/routes/share/[shareID].tsx` to provide the unified component once wrappers are stable. + +#### Acceptance + +- New consumers can use one context path. +- Existing `useCodeComponent` and `useDiffComponent` hooks still resolve and render correctly. +- Provider wiring in app and enterprise stays compatible during transition. + +### Phase 4: Share find and enable diff search + +- Extract the find engine and find bar UI from `code.tsx` into shared modules. +- Hook the shared find host into unified `File` for both text and diff modes. +- Keep current shortcuts (`Ctrl/Cmd+F`, `Ctrl/Cmd+G`, `Shift+Ctrl/Cmd+G`) and active-host behavior. +- Preserve CSS Highlight API support with overlay fallback. + +#### Acceptance + +- Text mode search behaves the same as today. +- Diff mode now supports the same find UI and shortcuts. +- Multiple viewer instances still route shortcuts to the focused/active host correctly. + +### Phase 5: Consolidate media rendering + +- Extract media type detection and data URL helpers from `session-review.tsx` and `file-tabs.tsx` into shared UI helpers. +- Add `file-media.tsx` and let unified `File` optionally render media or binary placeholders before falling back to text/diff. +- Migrate `session-review.tsx` and `file-tabs.tsx` to pass media props instead of owning media-specific branches. +- Keep session-specific layout and i18n strings in the consumer where they are not generic. + +#### Acceptance + +- Image/audio/svg/binary handling no longer duplicates core detection and load state logic. +- Session review and file tabs still render the same media states and placeholders. +- Text/diff comment and selection behavior is unchanged when media is not shown. + +### Phase 6: Align SSR and preloaded diffs + +- Create `packages/ui/src/components/file-ssr.tsx` with the same unified prop shape plus `preloadedDiff`. +- Reuse shared diff normalization, theme sync, and commented-line marking helpers. +- Convert `packages/ui/src/components/diff-ssr.tsx` into a thin adapter that forwards to the unified SSR entry in diff mode. +- Migrate enterprise share page imports to `@opencode-ai/ui/file-ssr` when convenient, but keep `diff-ssr` export working. + +#### Acceptance + +- Preloaded diff hydration still works in `packages/enterprise/src/routes/share/[shareID].tsx`. +- SSR diff and client diff now share normalization and comment marking helpers. +- No duplicate `fixSelection` implementation remains. + +### Phase 7: Clean up and document + +- Remove dead internal helpers left behind in `code.tsx` and `diff.tsx`. +- Add a short migration doc for downstream consumers that want to switch from `Code`/`Diff` to unified `File`. +- Mark `Code`/`Diff` contexts and components as compatibility APIs in comments or docs. + +#### Acceptance + +- No stale duplicate helpers remain in legacy wrappers. +- Unified path is the default recommendation for new UI work. + +--- + +## Preserve compatibility + +Keep old APIs working while moving internals under them. + +### Context migration strategy + +- Introduce `FileComponentProvider` without deleting `CodeComponentProvider` or `DiffComponentProvider`. +- Implement `useCodeComponent` and `useDiffComponent` as adapters around the unified context where possible. +- If full adapter reuse is messy at first, keep old contexts and providers as thin wrappers that internally provide mapped unified props. + +### Consumer migration targets + +- `packages/app/src/pages/session/file-tabs.tsx` should move from `useCodeComponent` to `useFileComponent`. +- `packages/ui/src/components/session-review.tsx`, `session-turn.tsx`, and `message-part.tsx` should move from `useDiffComponent` to `useFileComponent`. +- `packages/app/src/app.tsx` and `packages/enterprise/src/routes/share/[shareID].tsx` should eventually provide only the unified provider. +- Keep legacy hooks available until all call sites are migrated and reviewed. + +### Compatibility checkpoints + +- `@opencode-ai/ui/code`, `@opencode-ai/ui/diff`, and `@opencode-ai/ui/diff-ssr` imports must keep working during migration. +- Existing prop names on `Code` and `Diff` wrappers should remain stable to avoid broad app changes in one PR. + +--- + +## Unify search + +Port the current find feature into a shared engine and attach it to both modes. + +### Shared engine plan + +- Move keyboard host registry and active-target logic out of `code.tsx` into `packages/ui/src/pierre/file-find.ts`. +- Move the find bar UI into `packages/ui/src/components/file-search.tsx`. +- Keep DOM-based scanning and highlight/overlay rendering shared, since both text and diff render into the same shadow-root patterns. + +### Diff-specific handling + +- Search should scan both unified and split diff columns through the same selectors used in the current code find feature. +- Match navigation should scroll the active range into view without interfering with line selection state. +- Search refresh should run after `onRendered`, diff style changes, annotation rerenders, and query changes. + +### Scope guard + +- Preserve the current DOM-scan behavior first, even if virtualized search is limited to mounted rows. +- If full-document virtualized search is required, treat it as a follow-up with a text-index layer rather than blocking the core refactor. + +--- + +## Consolidate media + +Move media rendering logic into shared UI so text, diff, and media routing live behind one entry. + +### Ownership plan + +- Put media detection and normalization helpers in `packages/ui/src/pierre/media.ts`. +- Put shared rendering UI in `packages/ui/src/components/file-media.tsx`. +- Keep layout-specific wrappers in `session-review.tsx` and `file-tabs.tsx`, but remove duplicated media branching and load-state code from them. + +### Proposed media props + +- `media.mode`: `"auto"` or `"off"` for default behavior. +- `media.path`: file path for extension checks and labels. +- `media.current`: loaded file content for plain-file views. +- `media.before` and `media.after`: diff-side values for image/audio previews. +- `media.readFile`: optional lazy loader for session review expansion. +- `media.renderBinaryPlaceholder`: optional consumer override for binary states. +- `media.renderLoading` and `media.renderError`: optional consumer overrides when generic text is not enough. + +### Parity targets + +- Keep current image and audio support from session review. +- Keep current SVG and binary handling from file tabs. +- Defer video or PDF support unless explicitly requested. + +--- + +## Align SSR + +Make SSR diff hydration a mode of the unified viewer instead of a parallel implementation. + +### SSR plan + +- Add `packages/ui/src/components/file-ssr.tsx` as the unified SSR entry with a diff-only path in phase one. +- Reuse shared diff helpers for `fixSelection`, theme sync, and commented-line marking. +- Keep the private `fileContainer` hydration workaround isolated in the SSR module so client code stays clean. + +### Integration plan + +- Keep `packages/ui/src/components/diff-ssr.tsx` as a forwarding adapter for compatibility. +- Update enterprise share route to the unified SSR import after client and context migrations are stable. +- Align prop names with the client `File` component so `SessionReview` can swap client/SSR providers without branching logic. + +### Defer item + +- Plain-file SSR hydration is not needed for this refactor and can stay out of scope. + +--- + +## Verify behavior + +Use typechecks and targeted UI checks after each phase, and avoid repo-root runs. + +### Typecheck plan + +- Run `bun run typecheck` from `packages/ui` after phases 1-7 changes there. +- Run `bun run typecheck` from `packages/app` after migrating file tabs or app provider wiring. +- Run `bun run typecheck` from `packages/enterprise` after SSR/provider changes on the share route. + +### Targeted UI checks + +- Text mode: + - small file render + - virtualized large file render + - drag selection and line-number selection + - comment annotations and commented-line marks + - find shortcuts and match navigation +- Diff mode: + - unified and split styles + - large diff fallback options + - diff selection normalization across sides + - comments and commented-line marks + - new find UX parity +- Media: + - image, audio, SVG, and binary states in file tabs + - image and audio diff previews in session review + - lazy load and error placeholders +- SSR: + - enterprise share page preloaded diffs hydrate correctly + - theme switching still updates hydrated diffs + +### Regression focus + +- Watch scroll restore behavior in `packages/app/src/pages/session/file-tabs.tsx`. +- Watch multi-instance find shortcut routing in screens with many viewers. +- Watch cleanup paths for listeners and virtualizers to avoid leaks. + +--- + +## Manage risk + +Keep wrappers and adapters in place until the unified path is proven. + +### Key risks + +- Selection regressions are the highest risk because text and diff have similar but not identical line semantics. +- SSR hydration can break subtly if client and SSR prop shapes drift. +- Shared find host state can misroute shortcuts when many viewers are mounted. +- Media consolidation can accidentally change placeholder timing or load behavior. + +### Rollback strategy + +- Land each phase in separate PRs or clearly separated commits on `dev`. +- If a phase regresses behavior, revert only that phase and keep earlier extractions. +- Keep `code.tsx`, `diff.tsx`, and `diff-ssr.tsx` wrappers intact until final verification, so a rollback only changes internals. +- If diff search is unstable, disable it behind the unified component while keeping the rest of the refactor. + +--- + +## Order implementation + +Follow this sequence to keep reviews small and reduce merge risk. + +1. Finalize prop shape and file names for the unified component and context. +2. Extract shared diff normalization, theme sync, and render-ready helpers with no public API changes. +3. Extract shared selection controller and migrate `code.tsx` and `diff.tsx` to it. +4. Add the unified client `File` component and convert `code.tsx`/`diff.tsx` into wrappers. +5. Add `FileComponentProvider` and migrate provider wiring in `app.tsx` and enterprise share route. +6. Migrate consumer hooks (`file-tabs`, `session-review`, `message-part`, `session-turn`) to the unified context. +7. Extract and share find engine/UI, then enable search in diff mode. +8. Extract media helpers/UI and migrate `session-review.tsx` and `file-tabs.tsx`. +9. Add unified `file-ssr.tsx`, convert `diff-ssr.tsx` to a wrapper, and migrate enterprise imports. +10. Remove dead duplication and write a short migration note for future consumers. + +--- + +## Decide open items + +Resolve these before coding to avoid rework mid-refactor. + +### API decisions + +- Should the unified component require `mode`, or should it infer mode from props for convenience. +- Should the public export be named `File` only, or also ship a temporary alias like `UnifiedFile` for migration clarity. +- Should `preloadedDiff` live on the main `File` props or only on `file-ssr.tsx`. + +### Search decisions + +- Is DOM-only search acceptable for virtualized content in the first pass. +- Should find state reset on every rerender, or preserve query and index across diff style toggles. + +### Media decisions + +- Which placeholders and strings should stay consumer-owned versus shared in UI. +- Whether SVG should be treated as media-only, text-only, or a mixed mode with both preview and source. +- Whether video support should be included now or explicitly deferred. + +### Migration decisions + +- How long `CodeComponentProvider` and `DiffComponentProvider` should remain supported. +- Whether to migrate all consumers in one PR after wrappers land, or in follow-up PRs by surface area. +- Whether `diff-ssr` should remain as a permanent alias for compatibility. diff --git a/specs/session-review-cross-diff-search-plan.md b/specs/session-review-cross-diff-search-plan.md new file mode 100644 index 000000000..6a15d5bec --- /dev/null +++ b/specs/session-review-cross-diff-search-plan.md @@ -0,0 +1,234 @@ +# 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/diffs` internals. + +--- + +## Current behavior + +- `SessionReview` renders one `File` diff viewer per accordion item, but only mounts the viewer when that item is expanded. +- Large diffs may be blocked behind the `MAX_DIFF_CHANGED_LINES` gate until the user clicks "render anyway". +- `File` owns a local search engine (`createFileFind`) with: + - query state + - hit counting + - current match index + - highlighting (CSS Highlight API or overlay fallback) + - `Cmd/Ctrl+F` and `Cmd/Ctrl+G` keyboard handling +- `FileSearchBar` is currently rendered per viewer. +- There is no parent-level search state in `SessionReview`. + +--- + +## UX requirements + +- Add one search bar in the `SessionReview` header (input, total count, prev, next, close). +- Show a global count like `3/17` across all searchable diffs. +- `Cmd/Ctrl+F` inside the session review pane opens the session-level search. +- `Cmd/Ctrl+G`, `Shift+Cmd/Ctrl+G`, `Enter`, and `Shift+Enter` navigate 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 + +- `SessionReview` owns the global query, hit list, and active hit index. +- `File` exposes a small controlled search handle (register, set query, clear, reveal hit). +- `SessionReview` keeps a map of mounted file viewers and their search handles. +- `SessionReview` resolves next/prev hits, expands files as needed, then tells the target viewer to reveal the hit. + +--- + +## Data model and interfaces + +```ts +type SessionSearchHit = { + file: string + side: "additions" | "deletions" + line: number + col: number + len: number +} + +type SessionSearchState = { + query: string + hits: SessionSearchHit[] + active: number +} +``` + +```ts +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 +} +``` + +```ts +type FileSearchControl = { + shortcuts?: "global" | "disabled" + showBar?: boolean + register: (handle: FileSearchHandle | null) => void +} +``` + +--- + +## Integration steps + +### Phase 1: Expose controlled search on `File` + +- Extend `createFileFind` and `File` to 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 + +- `File` still supports local search unchanged by default. +- `File` can 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 `SessionReview` header (can reuse `FileSearchBar` visuals or extract shared presentational pieces). +- Build a global hit list from `props.diffs` string 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 `FileSearchHandle` per mounted diff viewer. +- On next/prev, resolve the active global hit and: + 1. expand the target file if needed + 2. wait for the viewer to mount/render + 3. call `handle.setQuery(query)` and `handle.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 anyway` state from local accordion item state into a file-keyed map in `SessionReview`. +- 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+G` to 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 `onRendered` fires. +- 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 +- `File` controlled search handle: + - `setQuery` + - `clear` + - `reveal` by 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+F` opens session-level search in the review pane. +- `Cmd/Ctrl+G` / `Shift+Cmd/Ctrl+G` navigate 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 `SessionReview` prop/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 `FileSearchBar` directly or split out a shared non-portal variant? |
