diff --git a/specs/file-component-unification-plan.md b/specs/file-component-unification-plan.md deleted file mode 100644 index d63d665fd..000000000 --- a/specs/file-component-unification-plan.md +++ /dev/null @@ -1,426 +0,0 @@ -# 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-composer-refactor-plan.md b/specs/session-composer-refactor-plan.md deleted file mode 100644 index 08fb0d832..000000000 --- a/specs/session-composer-refactor-plan.md +++ /dev/null @@ -1,240 +0,0 @@ -# Session Composer Refactor Plan - -## Goal - -Improve structure, ownership, and reuse for the bottom-of-session composer area without changing user-visible behavior. - -Scope: - -- `packages/ui/src/components/dock-prompt.tsx` -- `packages/app/src/components/session-todo-dock.tsx` -- `packages/app/src/components/question-dock.tsx` -- `packages/app/src/pages/session/session-prompt-dock.tsx` -- related shared UI in `packages/app/src/components/prompt-input.tsx` - -## Decisions Up Front - -1. **`session-prompt-dock` should stay route-scoped.** - It is session-page orchestration, so it belongs under `pages/session`, not global `src/components`. - -2. **The orchestrator should keep blocking ownership.** - A single component should decide whether to show blockers (`question`/`permission`) or the regular prompt input. This avoids drift and duplicate logic. - -3. **Current component does too much.** - Split state derivation, permission actions, and rendering into smaller units while preserving behavior. - -4. **There is style duplication worth addressing.** - The prompt top shell and lower tray (`prompt-input.tsx`) visually overlap with dock shells/footers and todo containers. We should extract reusable dock surface primitives. - ---- - -## Phase 0 (Mandatory Gate): Baseline E2E Coverage - -No refactor work starts until this phase is complete and green locally. - -### 0.1 Deterministic test harness - -Add a test-only way to put a session into exact dock states, so tests do not rely on model/tool nondeterminism. - -Proposed implementation: - -- Add a guarded e2e route in backend (enabled only when a dedicated env flag is set by e2e-local runner). - - New route file: `packages/opencode/src/server/routes/e2e.ts` - - Mount from: `packages/opencode/src/server/server.ts` - - Gate behind env flag (for example `OPENCODE_E2E=1`) so this route is never exposed in normal runs. -- Add seed helpers in app e2e layer: - - `packages/app/e2e/actions.ts` (or `fixtures.ts`) helpers to: - - seed question request for a session - - seed permission request for a session - - seed/update todos for a session - - clear seeded blockers/todos -- Update e2e-local runner to set the flag: - - `packages/app/script/e2e-local.ts` - -### 0.2 New e2e spec - -Create a focused spec: - -- `packages/app/e2e/session/session-composer-dock.spec.ts` - -Test matrix (minimum required): - -1. **Default prompt dock** - - no blocker state - - assert prompt input is visible and focusable - - assert blocker cards are absent - -2. **Blocked question flow** - - seed question request for session - - assert question dock renders - - assert prompt input is not shown/active - - answer and submit - - assert unblock and prompt input returns - -3. **Blocked permission flow** - - seed permission request with patterns + optional description - - assert permission dock renders expected actions - - assert prompt input is not shown/active - - test each response path (`once`, `always`, `reject`) across tests - - assert unblock behavior - -4. **Todo dock transitions and collapse behavior** - - seed todos with `pending`/`in_progress` - - assert todo dock appears above prompt and can collapse/expand - - update todos to all completed/cancelled - - assert close animation path and eventual hide - -5. **Keyboard focus behavior while blocked** - - with blocker active, typing from document context must not focus prompt input - - blocker actions remain keyboard reachable - -Notes: - -- Prefer stable selectors (`data-component`, `data-slot`, role/name). -- Extend `packages/app/e2e/selectors.ts` as needed. -- Use `expect.poll` for async transitions. - -### 0.3 Gate commands (must pass before Phase 1) - -Run from `packages/app` (never from repo root): - -```bash -bun test:e2e:local -- e2e/session/session-composer-dock.spec.ts -bun test:e2e:local -- e2e/prompt/prompt.spec.ts e2e/prompt/prompt-multiline.spec.ts e2e/commands/input-focus.spec.ts -bun test:e2e:local -``` - -If any fail, stop and fix before refactor. - ---- - -## Phase 1: Structural Refactor (No Intended Behavior Changes) - -### 1.1 Colocate session-composer files - -Create a route-local composer folder: - -```txt -packages/app/src/pages/session/composer/ - session-composer-region.tsx # rename/move from session-prompt-dock.tsx - session-composer-state.ts # derived state + actions - session-permission-dock.tsx # extracted from inline JSX - session-question-dock.tsx # moved from src/components/question-dock.tsx - session-todo-dock.tsx # moved from src/components/session-todo-dock.tsx - index.ts -``` - -Import updates: - -- `packages/app/src/pages/session.tsx` imports `SessionComposerRegion` from `pages/session/composer`. - -### 1.2 Split responsibilities - -- Keep `session-composer-region.tsx` focused on rendering orchestration: - - blocker mode vs normal mode - - relative stacking (todo above prompt) - - handoff fallback rendering -- Move side-effect/business pieces into `session-composer-state.ts`: - - derive `questionRequest`, `permissionRequest`, `blocked`, todo visibility state - - permission response action + in-flight state - - todo close/open animation state - -### 1.3 Remove duplicate blocked logic in `session.tsx` - -Current `session.tsx` computes `blocked` independently. Make the composer state the single source for blocker status consumed by both: - -- page-level keydown autofocus guard -- composer rendering guard - -### 1.4 Keep prompt gating in orchestrator - -`session-composer-region` should remain responsible for choosing whether `PromptInput` renders when blocked. - -Rationale: - -- this is layout-mode orchestration, not prompt implementation detail -- keeps blocker and prompt transitions coordinated in one place - -### 1.5 Phase 1 acceptance criteria - -- No intentional behavior deltas. -- Phase 0 suite remains green. -- `session-prompt-dock` no longer exists as a large mixed-responsibility component. -- Session composer files are colocated under `pages/session/composer`. - ---- - -## Phase 2: Reuse + Styling Maintainability - -### 2.1 Extract shared dock surface primitives - -Create reusable shell/tray wrappers to remove repeated visual scaffolding: - -- primary elevated surface (prompt top shell / dock body) -- secondary tray surface (prompt bottom bar / dock footer / todo shell) - -Proposed targets: - -- `packages/ui/src/components` for shared primitives if reused by both app and ui components -- or `packages/app/src/pages/session/composer` first, then promote to ui after proving reuse - -### 2.2 Apply primitives to current components - -Adopt in: - -- `packages/app/src/components/prompt-input.tsx` -- `packages/app/src/pages/session/composer/session-todo-dock.tsx` -- `packages/ui/src/components/dock-prompt.tsx` (where appropriate) - -Focus on deduping patterns seen in: - -- prompt elevated shell styles (`prompt-input.tsx` form container) -- prompt lower tray (`prompt-input.tsx` bottom panel) -- dock prompt footer/body and todo dock container - -### 2.3 De-risk style ownership - -- Move dock-specific styling out of overly broad files (for example, avoid keeping new dock-specific rules buried in unrelated message-part styling files). -- Keep slot names stable unless tests are updated in the same PR. - -### 2.4 Optional follow-up (if low risk) - -Evaluate extracting shared question/permission presentational pieces used by: - -- `packages/app/src/pages/session/composer/session-question-dock.tsx` -- `packages/ui/src/components/message-part.tsx` - -Only do this if behavior parity is protected by tests and the change is still reviewable. - -### 2.5 Phase 2 acceptance criteria - -- Reduced duplicated shell/tray styling code. -- No regressions in blocker/todo/prompt transitions. -- Phase 0 suite remains green. - ---- - -## Implementation Sequence (single branch) - -1. **Step A - Baseline safety net** - - Add e2e harness + new session composer dock spec + selector/helpers. - - Must pass locally before any refactor work proceeds. - -2. **Step B - Phase 1 colocation/splitting** - - Move/rename files, extract state and permission component, keep behavior. - -3. **Step C - Phase 1 dedupe blocked source** - - Remove duplicate blocked derivation and wire page autofocus guard to shared source. - -4. **Step D - Phase 2 style primitives** - - Introduce shared surface primitives and migrate prompt/todo/dock usage. - -5. **Step E (optional) - shared question/permission presentational extraction** - ---- - -## Rollback Strategy - -- Keep each step logically isolated and easy to revert. -- If regressions occur, revert the latest completed step first and rerun the Phase 0 suite. -- If style extraction destabilizes behavior, keep structural Phase 1 changes and revert only Phase 2 styling commits. diff --git a/specs/session-review-cross-diff-search-plan.md b/specs/session-review-cross-diff-search-plan.md deleted file mode 100644 index 6a15d5bec..000000000 --- a/specs/session-review-cross-diff-search-plan.md +++ /dev/null @@ -1,234 +0,0 @@ -# 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?