chore: refactoring and tests (#12468)

specs/09-session-page-hot-paths.md

@@ -0,0 +1,105 @@
+## Session hot paths
+
+Reduce render work and duplication in `session.tsx`
+
+---
+
+### Summary
+
+`packages/app/src/pages/session.tsx` mixes routing, commands, tab rendering, review panel wiring, terminal focus logic, and message scrolling. This spec targets hot-path performance + local code quality improvements that can ship together in one session-page-focused PR. It should follow the keyed command-registration pattern introduced in `packages/app/src/context/command.tsx`.
+
+---
+
+### Goals
+
+- Render heavy file-tab content only for the active tab
+- Deduplicate review-panel wiring used in desktop and mobile paths
+- Centralize terminal-focus DOM logic into one helper
+- Reduce churn in command registration setup
+
+---
+
+### Non-goals
+
+- Scroll-spy rewrite (covered by `specs/04-scroll-spy-optimization.md`)
+- Large routing/layout redesign
+- Behavior changes to prompt submission or session history
+
+---
+
+### Parallel execution contract
+
+This spec owns:
+
+- `packages/app/src/pages/session.tsx`
+- New files under `packages/app/src/pages/session/*` (if extracted)
+
+This spec should not modify:
+
+- `packages/app/src/context/*`
+- `packages/app/src/components/prompt-input.tsx`
+- `packages/app/src/components/file-tree.tsx`
+
+---
+
+### Implementation plan
+
+1. Add shared helpers for repeated session-page actions
+
+- Extract `openReviewFile(path)` helper to replace repeated inline `onViewFile` bodies.
+- Extract `focusTerminalById(id)` helper and reuse in both:
+  - terminal active change effect
+  - terminal drag-end focus restoration
+
+2. Deduplicate review panel construction
+
+- Build a shared review props factory (or local render helper) so desktop/mobile paths do not duplicate comment wiring, `onViewFile`, and classes glue.
+- Keep per-surface differences limited to layout classes and diff style.
+
+3. Gate heavy file-tab rendering by active tab
+
+- Keep tab trigger list rendered for all opened tabs.
+- Render `Tabs.Content` body only for `activeTab()`, plus lightweight placeholders as needed.
+- Ensure per-tab scroll state restore still works when reactivating a tab.
+
+4. Reduce command registry reallocation
+
+- Register session commands with a stable key (`command.register("session", ...)`) so remounts replace prior session command entries.
+- Move large command-array construction into smaller memoized blocks:
+  - stable command definitions
+  - dynamic state fields (`disabled`, titles) as narrow computed closures
+- Keep command IDs, keybinds, and behavior identical.
+
+---
+
+### Acceptance criteria
+
+- File tab bodies are not all mounted at once for large open-tab sets.
+- `onViewFile` review behavior is defined in one shared helper.
+- Terminal focus query/dispatch logic lives in one function and is reused.
+- Session command registration uses a stable key (`"session"`) and `command.register` no longer contains one monolithic inline array with repeated inline handlers for shared actions.
+- Session UX remains unchanged for:
+  - opening files from review
+  - drag-reordering terminal tabs
+  - keyboard command execution
+
+---
+
+### Validation plan
+
+- Manual:
+  - Open 12+ file tabs, switch quickly, verify active tab restore and no blank states.
+  - Open review panel (desktop and mobile), use "view file" from diffs, verify same behavior as before.
+  - Drag terminal tab, ensure terminal input focus is restored.
+  - Run key commands: `mod+p`, `mod+w`, `mod+shift+r`, `ctrl+``.
+- Perf sanity:
+  - Compare CPU usage while switching tabs with many opened files before/after.
+
+---
+
+### Risks and mitigations
+
+- Risk: unmounted tab content loses transient editor state.
+  - Mitigation: keep persisted scroll/selection restore path intact and verify reactivation behavior.
+- Risk: command refactor subtly changes command ordering.
+  - Mitigation: keep IDs and registration order stable, diff against current command list in dev.

specs/10-file-content-eviction-accounting.md

@@ -0,0 +1,99 @@
+## File cache accounting
+
+Make file-content eviction bookkeeping O(1)
+
+---
+
+### Summary
+
+`packages/app/src/context/file.tsx` currently recomputes total cached bytes by reducing the entire LRU map inside the eviction loop. This creates avoidable overhead on large file sets. We will switch to incremental byte accounting while keeping LRU behavior unchanged.
+
+---
+
+### Goals
+
+- Remove repeated full-map reductions from eviction path
+- Maintain accurate total byte tracking incrementally
+- Preserve existing eviction semantics (entry count + byte cap)
+
+---
+
+### Non-goals
+
+- Changing cache limits
+- Changing file loading API behavior
+- Introducing cross-session shared caches
+
+---
+
+### Parallel execution contract
+
+This spec owns:
+
+- `packages/app/src/context/file.tsx`
+- Optional tests in `packages/app/src/context/*file*.test.ts`
+
+This spec should not modify:
+
+- `packages/app/src/pages/session.tsx`
+- `packages/app/src/components/file-tree.tsx`
+
+---
+
+### Implementation plan
+
+1. Introduce incremental byte counters
+
+- Add module-level `contentBytesTotal`.
+- Add helper(s):
+  - `setContentBytes(path, nextBytes)`
+  - `removeContentBytes(path)`
+  - `resetContentBytes()`
+
+2. Refactor LRU touch/update path
+
+- Keep `contentLru` as LRU order map.
+- Update byte total only when a path is inserted/updated/removed.
+- Ensure replacing existing byte value updates total correctly.
+
+3. Refactor eviction loop
+
+- Use `contentBytesTotal` in loop condition instead of `Array.from(...).reduce(...)`.
+- On eviction, remove from both `contentLru` and byte counter.
+
+4. Keep scope reset correct
+
+- On directory scope change, clear inflight maps + `contentLru` + byte counter.
+
+---
+
+### Acceptance criteria
+
+- `evictContent` performs no full-map reduction per iteration.
+- Total bytes remain accurate after:
+  - loading file A
+  - loading file B
+  - force-reloading file A with a different size
+  - evicting entries
+  - scope reset
+- Existing caps (`MAX_FILE_CONTENT_ENTRIES`, `MAX_FILE_CONTENT_BYTES`) continue to enforce correctly.
+
+---
+
+### Validation plan
+
+- Manual:
+  - Open many files with mixed sizes and verify old files still evict as before.
+  - Switch directory scope and verify cache clears safely.
+- Optional unit coverage:
+  - size counter updates on overwrite + delete.
+  - eviction condition uses count and bytes as expected.
+
+---
+
+### Risks and mitigations
+
+- Risk: byte counter drifts from map contents.
+  - Mitigation: route all updates through centralized helpers.
+- Risk: stale bytes retained on early returns.
+  - Mitigation: assert cleanup paths in `finally`/scope reset still execute.

specs/11-layout-view-tabs-reactivity.md

@@ -0,0 +1,92 @@
+## Layout reactivity
+
+Reduce per-call reactive overhead in `useLayout`
+
+---
+
+### Summary
+
+`packages/app/src/context/layout.tsx` creates reactive effects inside `view(sessionKey)` and `tabs(sessionKey)` each time these helpers are called. Multiple consumers for the same key can accumulate duplicate watchers. This spec simplifies the API internals so calls stay lightweight while preserving behavior.
+
+---
+
+### Goals
+
+- Remove avoidable per-call `createEffect` allocations in `view()` and `tabs()`
+- Preserve scroll seeding, pruning, and touch semantics
+- Keep external `useLayout` API stable
+
+---
+
+### Non-goals
+
+- Persistence schema migration
+- Session tab behavior redesign
+- New layout features
+
+---
+
+### Parallel execution contract
+
+This spec owns:
+
+- `packages/app/src/context/layout.tsx`
+- `packages/app/src/context/layout-scroll.test.ts` (if updates needed)
+
+This spec should not modify:
+
+- `packages/app/src/pages/session.tsx`
+- `packages/app/src/components/session/*`
+
+---
+
+### Implementation plan
+
+1. Consolidate key-touch logic
+
+- Introduce shared internal helper, e.g. `ensureSessionKey(key)` that performs:
+  - `touch(key)`
+  - `scroll.seed(key)`
+
+2. Remove per-call effects in `view()` / `tabs()`
+
+- Replace internal `createEffect(on(key, ...))` usage with lazy key reads inside accessors/memos.
+- Ensure reads still invoke `ensureSessionKey` at safe points.
+
+3. Keep return API stable
+
+- Preserve current method names and behavior:
+  - `view(...).scroll`, `setScroll`, `terminal`, `reviewPanel`, `review`
+  - `tabs(...).active`, `all`, `open`, `close`, `move`, etc.
+
+4. Verify pruning behavior
+
+- Ensure session-key pruning still runs when key set grows and active key changes.
+
+---
+
+### Acceptance criteria
+
+- `view()` and `tabs()` no longer instantiate per-call key-change effects.
+- Existing callers do not require API changes.
+- Scroll restore and tab persistence still work across session navigation.
+- No regressions in handoff/pending-message behavior.
+
+---
+
+### Validation plan
+
+- Manual:
+  - Navigate across multiple sessions; verify tabs + review open state + scroll positions restore.
+  - Toggle terminal/review panels and confirm persisted state remains consistent.
+- Tests:
+  - Update/add targeted tests for key seeding/pruning if behavior changed.
+
+---
+
+### Risks and mitigations
+
+- Risk: subtle key-touch ordering changes affect prune timing.
+  - Mitigation: keep `touch` and `seed` coupled through one helper and verify prune boundaries.
+- Risk: removing effects misses updates for dynamic accessor keys.
+  - Mitigation: ensure every public accessor path reads current key and calls helper.

specs/12-session-context-metrics-shared.md

@@ -0,0 +1,96 @@
+## Context metrics shared
+
+Unify duplicate session usage calculations
+
+---
+
+### Summary
+
+`session-context-tab.tsx` and `session-context-usage.tsx` both compute overlapping session metrics (cost, last assistant token totals, provider/model context usage). This creates duplicate loops and raises drift risk. We will centralize shared calculations in one helper module and have both components consume it.
+
+---
+
+### Goals
+
+- Compute shared session usage metrics in one place
+- Remove duplicate loops for cost and latest-token context usage
+- Keep UI output unchanged in both components
+
+---
+
+### Non-goals
+
+- Rewriting the detailed context breakdown estimator logic
+- Changing translations or labels
+- Moving metrics into backend API responses
+
+---
+
+### Parallel execution contract
+
+This spec owns:
+
+- `packages/app/src/components/session/session-context-tab.tsx`
+- `packages/app/src/components/session-context-usage.tsx`
+- New helper in `packages/app/src/components/session/*` or `packages/app/src/utils/*`
+
+This spec should not modify:
+
+- `packages/app/src/pages/session.tsx`
+- `packages/app/src/context/sync.tsx`
+
+---
+
+### Implementation plan
+
+1. Add shared metrics helper
+
+- Create helper for raw metrics from message list + provider map, e.g.:
+  - `totalCost`
+  - `lastAssistantWithTokens`
+  - `tokenTotal`
+  - `tokenUsagePercent`
+  - provider/model labels
+- Return raw numeric values; keep locale formatting in consumers.
+
+2. Add memoization guard
+
+- Use reference-based memoization (e.g. by message-array identity) inside helper or component-level memo to avoid duplicate recalculation on unchanged arrays.
+
+3. Migrate both components
+
+- Replace duplicated loops in:
+  - `session-context-tab.tsx`
+  - `session-context-usage.tsx`
+- Keep existing UI structure and i18n keys unchanged.
+
+---
+
+### Acceptance criteria
+
+- Shared cost + token calculations are defined in one module.
+- Both components read from the shared helper.
+- Rendered values remain identical for:
+  - total cost
+  - token totals
+  - usage percentage
+  - provider/model fallback labels
+
+---
+
+### Validation plan
+
+- Manual:
+  - Open session context tab and compare values with header/context indicator tooltip.
+  - Verify values update correctly while new assistant messages stream in.
+- Regression:
+  - locale change still formats numbers/currency correctly.
+
+---
+
+### Risks and mitigations
+
+- Risk: helper changes semantic edge cases (no provider, no model, missing token fields).
+  - Mitigation: preserve existing fallback behavior (`"—"`, null percent).
+- Risk: memoization over-caches stale values.
+  - Mitigation: key cache by message-array reference and dependent IDs only.

specs/13-file-tree-fetch-discipline.md

@@ -0,0 +1,88 @@
+## File tree fetches
+
+Make directory listing triggers explicit and minimal
+
+---
+
+### Summary
+
+`packages/app/src/components/file-tree.tsx` currently invokes `file.tree.list(path)` from a generic effect in each tree instance. Even with inflight guards, this pattern causes avoidable list calls and makes load behavior harder to reason about. This spec tightens fetch triggers.
+
+---
+
+### Goals
+
+- Avoid redundant list invocations from passive rerenders
+- Fetch directory data only when needed (mount + expansion + explicit refresh)
+- Keep tree behavior unchanged for users
+
+---
+
+### Non-goals
+
+- Replacing recursive tree rendering with virtualization
+- Changing file-tree visual design
+- Backend/API changes for file listing
+
+---
+
+### Parallel execution contract
+
+This spec owns:
+
+- `packages/app/src/components/file-tree.tsx`
+
+This spec should not modify:
+
+- `packages/app/src/context/file.tsx`
+- `packages/app/src/pages/session.tsx`
+
+---
+
+### Implementation plan
+
+1. Replace broad list effect with explicit triggers
+
+- Load root path on mount.
+- For nested directories, list only when:
+  - node is expanded, or
+  - parent explicitly requests refresh.
+
+2. Guard expansion-driven fetches
+
+- Keep `file.tree.expand(path)` as the primary source of truth for expansion fetches.
+- Ensure passive rerenders do not retrigger `list(path)` calls for already loaded dirs.
+
+3. Keep filter auto-expand behavior
+
+- Preserve existing "allowed filter" directory auto-expansion.
+- Ensure auto-expanded directories still fetch exactly once unless force refresh occurs.
+
+---
+
+### Acceptance criteria
+
+- `file-tree.tsx` no longer calls `file.tree.list(path)` from an unscoped rerender effect.
+- Expanding a folder still loads its children correctly.
+- Filtering by `allowed` still opens and shows required parent directories.
+- No regressions in change/all tabs where `FileTree` is used.
+
+---
+
+### Validation plan
+
+- Manual:
+  - Expand/collapse deep directory trees repeatedly.
+  - Switch between "changes" and "all" tree tabs.
+  - Open review, click files, verify tree stays responsive.
+- Optional instrumentation:
+  - count list calls per user action and compare before/after.
+
+---
+
+### Risks and mitigations
+
+- Risk: directories fail to load when expansion timing changes.
+  - Mitigation: rely on `expand()` path and verify for root + nested nodes.
+- Risk: filter-driven auto-expand misses one level.
+  - Mitigation: keep existing auto-expand iteration and add regression checks.

specs/14-comments-aggregation-index.md

@@ -0,0 +1,87 @@
+## Comments indexing
+
+Avoid repeated flatten+sort for comment aggregates
+
+---
+
+### Summary
+
+`packages/app/src/context/comments.tsx` derives `all` by flattening all file comment arrays and sorting on every change. This is simple but can become expensive with many comments. We will maintain an indexed aggregate structure incrementally.
+
+---
+
+### Goals
+
+- Keep `comments.list(file)` behavior unchanged
+- Make `comments.all()` retrieval near O(1) for reads
+- Preserve chronological ordering guarantees
+
+---
+
+### Non-goals
+
+- Persisting comments in a new schema
+- Adding new comment metadata fields
+- UI changes for comment display
+
+---
+
+### Parallel execution contract
+
+This spec owns:
+
+- `packages/app/src/context/comments.tsx`
+- Optional tests for comments context
+
+This spec should not modify:
+
+- `packages/app/src/pages/session.tsx`
+- `packages/ui/src/components/line-comment.tsx`
+
+---
+
+### Implementation plan
+
+1. Add aggregate index state
+
+- Maintain `commentsByFile` (existing) plus an `allComments` array in chronological order.
+- Keep both updated through the same mutator paths.
+
+2. Update mutators
+
+- `add`: append new comment to file list and aggregate list.
+- `remove`: remove from file list and aggregate list by id/file.
+- `clear`: reset both structures and focus/active state.
+
+3. Simplify selectors
+
+- `list(file)` reads file list directly.
+- `all()` returns pre-indexed aggregate list without per-read flatten+sort.
+
+---
+
+### Acceptance criteria
+
+- `comments.all()` no longer flattens and sorts every reactive run.
+- Comment order stays chronological by `time`.
+- `add/remove/clear/focus/active` semantics remain unchanged.
+
+---
+
+### Validation plan
+
+- Manual:
+  - Add multiple comments across different files.
+  - Remove one comment and verify both file-level and global views update correctly.
+  - Submit prompt (which clears comments) and verify reset behavior.
+- Optional unit test:
+  - add/remove/clear keeps aggregate ordering and integrity.
+
+---
+
+### Risks and mitigations
+
+- Risk: aggregate list and per-file lists diverge.
+  - Mitigation: funnel all writes through centralized mutators; avoid direct store writes elsewhere.
+- Risk: ID collision edge cases.
+  - Mitigation: keep UUID creation unchanged and remove by `file + id` pair.

specs/15-prompt-input-modularization.md

@@ -0,0 +1,104 @@
+## Prompt input split
+
+Modularize `prompt-input.tsx` without behavior changes
+
+---
+
+### Summary
+
+`packages/app/src/components/prompt-input.tsx` is a very large component that combines editor DOM parsing, popovers, history, drag/drop + paste uploads, worktree/session creation, optimistic messages, and send/abort flow. This spec splits it into focused modules so future changes are safer.
+
+---
+
+### Goals
+
+- Reduce `prompt-input.tsx` complexity and file size
+- Extract cohesive logic into testable hooks/helpers
+- Keep runtime behavior and UX unchanged
+
+---
+
+### Non-goals
+
+- Replacing contenteditable editor approach
+- Major UX redesign of composer controls
+- API contract changes for prompt submission
+
+---
+
+### Parallel execution contract
+
+This spec owns:
+
+- `packages/app/src/components/prompt-input.tsx`
+- New files under `packages/app/src/components/prompt-input/*`
+
+This spec should not modify:
+
+- `packages/app/src/pages/session.tsx`
+- `packages/app/src/context/prompt.tsx` (except minor type-only imports if needed)
+
+---
+
+### Implementation plan
+
+1. Extract editor DOM helpers
+
+- Move pure DOM/selection helpers into `prompt-input/editor-dom.ts`:
+  - `createTextFragment`
+  - `getNodeLength`
+  - `getTextLength`
+  - cursor get/set helpers
+
+2. Extract history controller
+
+- Move prompt history read/write/navigation logic into `prompt-input/history.ts` hook.
+- Keep existing persisted keys and history semantics unchanged.
+
+3. Extract attachment interactions
+
+- Move image/file paste + drag/drop + file-input attachment flows to `prompt-input/attachments.ts` hook.
+
+4. Extract submit pipeline
+
+- Move send/abort/optimistic message pipeline to `prompt-input/submit.ts` service/hook.
+- Keep existing error toasts, worktree handling, and rollback behavior.
+
+5. Keep composition shell stable
+
+- `PromptInput` component remains the integration shell that wires hooks + JSX.
+- Preserve exported component API and props.
+
+---
+
+### Acceptance criteria
+
+- `prompt-input.tsx` becomes primarily orchestration + view code.
+- Extracted modules contain the heavy imperative logic.
+- All existing behaviors remain intact:
+  - slash and @ popovers
+  - history up/down navigation
+  - image attach/paste/drag-drop
+  - shell mode submit/abort
+  - optimistic message + rollback on failure
+
+---
+
+### Validation plan
+
+- Manual regression checklist:
+  - type prompt, submit, stop, retry
+  - use `/` command selection and `@` selector
+  - history navigation with arrows
+  - paste image, drag image, remove attachment
+  - start in new session + worktree create path
+  - failure path restores prompt and context comments
+
+---
+
+### Risks and mitigations
+
+- Risk: subtle ordering changes in submit rollback logic.
+  - Mitigation: migrate logic mechanically first, then cleanup.
+- Risk: editor selection bugs after helper extraction.
+  - Mitigation: keep existing cursor helpers unchanged and add focused manual checks.

specs/16-terminal-cache-key-clarity.md

@@ -0,0 +1,82 @@
+## Terminal cache scope
+
+Clarify workspace-only terminal cache semantics
+
+---
+
+### Summary
+
+`packages/app/src/context/terminal.tsx` accepts `(dir, session)` but currently keys cache entries as `${dir}:${WORKSPACE_KEY}`. The behavior is workspace-scoped, but the API shape suggests session-scoped caching. This spec aligns naming and implementation to avoid confusion and future bugs.
+
+---
+
+### Goals
+
+- Make terminal cache scope explicit (workspace-scoped)
+- Remove misleading unused session-keying surface
+- Preserve existing runtime behavior
+
+---
+
+### Non-goals
+
+- Changing terminal persistence behavior
+- Moving terminals to per-session isolation
+- UI changes to terminal tabs
+
+---
+
+### Parallel execution contract
+
+This spec owns:
+
+- `packages/app/src/context/terminal.tsx`
+
+This spec should not modify:
+
+- `packages/app/src/pages/session.tsx`
+- `packages/app/src/components/session/session-sortable-terminal-tab.tsx`
+
+---
+
+### Implementation plan
+
+1. Rename internals for clarity
+
+- Update internal function names/variables from session-oriented to workspace-oriented where applicable.
+
+2. Remove unused session cache-key parametering
+
+- Simplify `load`/factory signatures so keying intent is explicit.
+- Keep key format workspace-only by directory.
+
+3. Add inline documentation
+
+- Add short comment near cache key creation clarifying why terminals are shared across sessions in the same workspace.
+
+4. Keep behavior stable
+
+- Ensure active terminal, tab order, clone/new/close behavior remain unchanged.
+
+---
+
+### Acceptance criteria
+
+- No unused session-derived cache key logic remains.
+- Code communicates workspace-scoped terminal lifecycle clearly.
+- No functional changes to terminal operations.
+
+---
+
+### Validation plan
+
+- Manual:
+  - Create multiple terminals, navigate between sessions in same workspace, confirm state continuity.
+  - Switch workspace directory, confirm separate terminal state.
+
+---
+
+### Risks and mitigations
+
+- Risk: accidental behavior change to session-scoped terminals.
+  - Mitigation: keep cache key unchanged; refactor naming/signatures only.

specs/parallel-agent-plan.md

@@ -0,0 +1,59 @@
+## Parallel agent plan
+
+Execution map for session-page improvement concerns
+
+---
+
+### New specs added
+
+- `specs/09-session-page-hot-paths.md`
+- `specs/10-file-content-eviction-accounting.md`
+- `specs/11-layout-view-tabs-reactivity.md`
+- `specs/12-session-context-metrics-shared.md`
+- `specs/13-file-tree-fetch-discipline.md`
+- `specs/14-comments-aggregation-index.md`
+- `specs/15-prompt-input-modularization.md`
+- `specs/16-terminal-cache-key-clarity.md`
+
+---
+
+### Existing related specs
+
+- `specs/04-scroll-spy-optimization.md` (session scroll-spy concern)
+- `specs/05-modularize-and-dedupe.md` (broad modularization roadmap)
+
+---
+
+### Parallel-safe batching
+
+Batch A (run one at a time, shared `session.tsx` surface):
+
+- `specs/09-session-page-hot-paths.md`
+- `specs/04-scroll-spy-optimization.md`
+
+Batch B (parallel with each other and with Batch A):
+
+- `specs/10-file-content-eviction-accounting.md`
+- `specs/11-layout-view-tabs-reactivity.md`
+- `specs/12-session-context-metrics-shared.md`
+- `specs/13-file-tree-fetch-discipline.md`
+- `specs/14-comments-aggregation-index.md`
+- `specs/15-prompt-input-modularization.md`
+- `specs/16-terminal-cache-key-clarity.md`
+
+Batch C (broad follow-up after focused specs land):
+
+- `specs/05-modularize-and-dedupe.md`
+
+---
+
+### Suggested assignment
+
+1. Agent A: `specs/09-session-page-hot-paths.md`
+2. Agent B: `specs/10-file-content-eviction-accounting.md`
+3. Agent C: `specs/11-layout-view-tabs-reactivity.md`
+4. Agent D: `specs/12-session-context-metrics-shared.md`
+5. Agent E: `specs/13-file-tree-fetch-discipline.md`
+6. Agent F: `specs/14-comments-aggregation-index.md`
+7. Agent G: `specs/15-prompt-input-modularization.md`
+8. Agent H: `specs/16-terminal-cache-key-clarity.md`