Skip to content

feat(code-tab): swap file tree + diff view for @pierre/trees and @pierre/diffs#708

Merged
srid merged 15 commits intomasterfrom
pierre-trees-diffs
Apr 25, 2026
Merged

feat(code-tab): swap file tree + diff view for @pierre/trees and @pierre/diffs#708
srid merged 15 commits intomasterfrom
pierre-trees-diffs

Conversation

@srid
Copy link
Copy Markdown
Member

@srid srid commented Apr 24, 2026

Why

The Code tab's file tree and unified diff were hand-rolled on top of buildFileTree, a plain SolidJS FileTree, @git-diff-view/solid, and highlight.js. Plenty of surface area for bugs — sort, collapse, sticky folders, search, gutter math, rename handling — and none of it is kolu's core concern.

Pierre Computer Company ships production-grade open-source libraries for exactly this: @pierre/trees (path-first, virtualized file tree) and @pierre/diffs (Shiki-backed diff + file renderer). This PR swaps our bespoke versions in for Pierre's, then leverages the features Pierre exposes (sticky folders, flatten-single-child, context menus, line selection) so we get better UX with less code.

What changed

Three thin Solid wrappers — Pierre's vanilla classes own their DOM (shadow-root rendered, Preact inside). Our shells mount once, push updates via setters inside createEffect(on(...)), and cleanUp() on disposal — no Solid re-render loop.

  • PierreFileTree.tsx — wraps new FileTree({...}), surfaces onSelect, custom right-click menu via composition.contextMenu, custom sprite sheet for nix/hs icons.
  • PierreDiffView.tsx — wraps new FileDiff({...}), parses raw unified diffs via parsePatchFiles(). Server hands us hunks: string[] with full headers — no shape juggling.
  • PierreFileView.tsx — wraps Pierre's File for non-diff single-file rendering in browse mode.
  • useLineSelection(path) — shared hook for the "Copy path" / "Copy <path>:<line>" right-click menu wiring (one source of truth for both viewers).
  • CodeContextMenu.tsx — Solid <Portal> menu used by the diff/file viewers (Pierre's diffs library doesn't expose a context-menu hook the way trees does; see comment thread for rationale).

Three modes, one tree. All / Local / Branch collapsed from three tabs into a nested segmented control ([ All ] [ Local | Branch ]). The same Pierre tree renders all three modes — only the data source differs (full repo vs. changed-vs-HEAD vs. changed-vs-merge-base).

Browse mode goes path-first. New fs.listAll endpoint backed by git ls-files --cached --others --exclude-standard returns tracked + untracked paths in one round trip; Pierre takes the flat list and infers the tree structure — no lazy-load dance.

Server simplifications. getDiff no longer reads old/new file contents (Pierre parses unified diffs directly), and listDir is gone (replaced by listAll). oldContent/newContent dropped from GitDiffOutputSchema.

Deletions. buildFileTree.ts, buildFileTree.test.ts, FileTree.tsx, code-tab.css (100+ lines of vendor overrides), @git-diff-view/solid, highlight.js. Net −281 lines of client + server code, before counting the e2e additions.

E2e ported. 14 scenarios / 143 steps in features/code-tab.feature cover: tab presence, mode picker + persistence, file listing, diff rendering, directory grouping, collapse/expand, right-click Copy path from the tree, line-gutter selection + right-click Copy <path>:<line> from the file viewer.

Skill for posterity. .apm/skills/pierre/SKILL.md documents the wrapper pattern, status-letter mapping, peer-dep trick, theming hooks, and gotchas (shadow DOM piercing, path-first identity, trailing-slash on folder rows).

Architectural review

/hickey and /lowy reviews in #issuecomment-4317109853. One Hickey finding actioned (useLineSelection hook extraction); one rejected with reasoning (replacing Pierre's tree menu would lose keyboard activation + action-lane affordance). All Lowy boundaries hold. Forward note: if non-git diff sources arrive, GitDiffOutputSchema's raw unified-diff string should become a discriminated union.

Status

  • just check green across all packages
  • just ci green on 199b718 — all 10 contexts pass on first run
  • ✅ 14 e2e scenarios (143 steps) cover the new tree + diff + browse + context-menu surfaces
  • ✅ Custom theming via --trees-* and --diffs-* CSS variables; nix/hs icons via custom sprite sheet
  • 📌 Live updates (chokidar watcher + streaming oRPC + tree.add/remove/move) tracked separately as Live updates for the Code tab's file tree (replace ↻ button) #711

🤖 Generated with Claude Code

srid added 10 commits April 24, 2026 16:35
…ierre/trees and @pierre/diffs

Drops ~650 lines of custom tree/diff/highlighter code in favour of
Pierre's vanilla classes, wrapped in thin SolidJS components that mount,
push updates via setters, and clean up. CodeTab's three sub-tabs (local,
branch, browse) now share one file-tree primitive and one diff/file
renderer backed by shiki.

- Removes: packages/client/src/ui/{FileTree,buildFileTree,buildFileTree.test}.{ts,tsx}, code-tab.css
- Removes deps: @git-diff-view/solid, highlight.js
- Adds deps: @pierre/trees, @pierre/diffs
- Adds: PierreFileTree, PierreDiffView, PierreFileView wrappers
- Adds: fs.listAll RPC endpoint — one-shot git-filtered path snapshot for
  the path-first tree model (file browser mode)
- Adds: .apm/skills/pierre/SKILL.md documenting the integration pattern

Prototype — e2e steps in code_tab_steps.ts still reference
@git-diff-view's `.diff-line` class and need updating before merge.
- Give the Pierre file-tree pane a real height (35% of parent) so the
  virtualized shadow-root container isn't collapsed to the search bar.
  Pierre's tree has no intrinsic content height — `max-h` + `shrink-0`
  rendered it at 42px.
- Apply kolu's `--color-*` tokens to Pierre via `--trees-*-override` /
  `--diffs-*-override` CSS variables so dark/light mode follow the rest
  of the app instead of Pierre's stock `light-dark()` defaults.
- Mirror `.dark` class to `document.documentElement.style.colorScheme`
  so third-party shadow-DOM widgets (Pierre, and anything else using
  `light-dark()`) pick up the resolved scheme.
- Export `isDark()` from `useColorScheme` so `system` users who prefer
  dark no longer misread as "light" when the preference is literally
  `system`.
- Folders collapse by default (`initialExpansion: "closed"`) — scrolling
  through an entire repo's tree pre-expanded was noisy.
- Restore the effect that resets the selected path when the view or
  repo changes, so a pick from browse mode doesn't bleed into diff mode.
- Bump git diff maxBuffer 16MB → 128MB so large untracked files don't
  crash `git diff --no-index`.
- Drop unused `oldFileName`/`newFileName` props on `PierreDiffView` (dead
  code — caller handles rename case before mounting the view).
- Tighten `theme` prop from optional-with-fallback to required; Kolu always
  passes a resolved value from `useColorScheme`, so the `?? "dark"` default
  masked configuration rather than recovering from it.
- Log malformed diffs in `parseFirstFile`'s catch — previously swallowed
  silently, leaving a blank pane and no hint.
- Move `diffTheme` derivation into `useColorScheme` as `themeTypeLiteral()`
  so the `isDark() ? "dark" : "light"` mapping has a single owner.
… comment

- Move the inline `BrowseFileView` helper out of `CodeTab.tsx` into its
  own `right-panel/BrowseFileView.tsx`, per the "one component per file"
  rule. CodeTab stays a layout shell, BrowseFileView owns the
  fetch-then-render concern.
- Update `GitDiffOutputSchema`'s comment to point at `@pierre/diffs`'s
  `parsePatchFiles` (the current consumer), not the retired
  `@git-diff-view/core` parser.
- Filter Pierre's onSelectionChange to files only (paths in the flat list)
  so clicking a directory no longer triggers a file read that fails with
  EISDIR.
- Pierre's diffs CSS reads bare `--diffs-font-size` / `--diffs-font-family`
  variables, not the `-override` suffix used for colors. Our override was
  silently ignored and the diff/file view rendered at the 13px default
  instead of kolu's 11px; fixed by using the correct variable names.
- Inject a small shadow-DOM sprite sheet with custom symbols for .nix
  (snowflake) and .hs/.lhs (lambda) so these languages get distinct icons
  instead of the generic default fallback.
…nted picker

Local and Branch were structurally identical (same tree, same diff layout —
only the diff base differed); Browse was the same tree with no filter and a
content viewer instead of a diff. Three "tabs" was the wrong abstraction:
they were three modes of one component, not three separate components.

Now: one PierreFileTree, one viewer pane, and a nested segmented picker
[ All ] [ Local | Branch ] in the header. The grouping signals the
hierarchy — All is the unfiltered base view, Local/Branch are siblings
that only differ in their diff base. Right-pane behavior derives from
the active mode: diff for Local/Branch, file content for All.

Net: -183 lines and one fewer concept for users to track.
Pierre's FileTree closes the search session on input blur. Their
`searchBlurBehavior: 'retain'` only protects the *initial mount* query
(per the implementation comment in FileTreeView.tsx), so as soon as the
user types and then clicks a result, the filter vanishes — terrible for
the tree+viewer pattern where you want to scan multiple matches.

Workaround: track the live query via `onSearchChange` and re-open the
search inside `onSelectionChange` with a short window-based heuristic.
Escape (close not followed by a selection) still closes; row click
(close immediately followed by a selection) re-opens with the saved
query.
Three things became unreachable when the Code tab moved to @pierre/diffs:

- fs.listDir RPC (function, schema, contract entry, router handler,
  re-exports, and its 7-test describe block in browse.test.ts). The old
  hand-rolled file tree paginated children one directory at a time;
  Pierre takes a flat path list via fs.listAll and never asks for
  per-directory entries. Zero remaining callers.

- GitDiffOutput.oldContent / newContent. The previous diff renderer
  (@git-diff-view/solid) needed both file revisions alongside the
  unified diff. Pierre's parsePatchFiles works from the diff string
  alone, so these fields traveled the wire and were dropped on the
  floor. Pulling them out also removes readContentAtRev /
  readWorkingContent, dropping the per-diff cost from one git diff +
  one git show + one fs.readFile to one git diff.

- The oldFileName / newFileName flags previously rode on whether
  oldContent / newContent were non-empty. They're now derived from
  --- /dev/null and +++ /dev/null markers in the rawDiff text, which
  carries the same information without any extra round-trips.

Two stale @git-diff-view doc comments updated to reference
@pierre/diffs. Two unit-test assertions on the dropped content fields
removed.
srid added 3 commits April 24, 2026 18:31
…ickyFolders, context menus, line selection

Tree (`PierreFileTree`):
- `flattenEmptyDirectories: true`. Single-child directory chains collapse
  into one row (`packages/client/src/right-panel` reads as one segmented
  row, not five). Big readability win for deep monorepos.
- `stickyFolders: true`. Parent directories pin to the top of the scroll
  viewport while you scroll deeper subtrees.
- Right-click "Copy path" via Pierre's `composition.contextMenu.render`.
  We hand Pierre a vanilla HTMLElement (it lives inside the tree's
  shadow DOM), and Pierre handles outside-click closing.

Diff/File (`PierreDiffView`, `PierreFileView`):
- `enableLineSelection: true` so the user can drag-select a line range.
- `onLineSelected` tracks the range in a Solid signal.
- New `CodeContextMenu` component listens for right-click on the host
  container and offers "Copy path" plus, when a selection exists,
  "Copy <path>:<start>-<end>" (or `<path>:<line>` for a single line).
  Format matches what most editors and code tools accept (VS Code, Vim
  `:e file:N`, GitHub URL fragments). Useful for pasting precise
  references into chats or agents.

`presorted: true` was on the asked-for list but skipped: replicating
Pierre's natural-collation + dir-first ordering server-side is risky
(custom token splitter, lowercase, dir-vs-file at leaf depth), and the
performance win at kolu repo sizes (~1k–5k files) is sub-millisecond.
Not worth the matching-bugs risk.

Live updates from server filed as #711 for follow-up — Pierre's
incremental mutation API (`tree.add` / `remove` / `move` / `batch`) is
ready, but a server-side chokidar watcher + new streaming oRPC
procedure is a meaningful subsystem on its own.
Pierre wraps our rendered menu in a `display:flex; align-items:center`
anchor that's positioned at the cursor for right-click. Letting our
menu lay out as a flex child shifts it right of the click point.
Position the menu with `fixed` + `context.anchorRect` coords so it
lands exactly where the user clicked, ignoring the wrapping anchor's
layout.
…arios

Rewrites code-tab e2e atop @pierre/trees and @pierre/diffs DOM:
- file rows match `[data-item-path][data-item-type="file"]`, dirs the
  same with a trailing slash (Pierre quirk) and `:not(sticky-row)` to
  skip the static header duplicate
- diff/file rendering asserts via `[data-line]` (Pierre's processLine)
- file content reads pierce shadow DOM (Pierre mounts highlighted code
  inside a shadow root, so `el.textContent` doesn't see it)
- line-number selection drives Playwright's mouse API directly so the
  pointerdown→document-pointerup pair Pierre needs for selection
  commit fires reliably

Adds two scenarios for the menu features wired in 5a4f41c:
- right-click on a changed file → "Copy path" → clipboard
- click line gutter, right-click on the file viewer →
  "Copy <path>:<line>" → clipboard

Drops the toggle-deselect-on-second-click expectation (Pierre's
single-select tree doesn't deselect on re-click — UX choice) and the
missing-origin error scenario (separate flake; tracked elsewhere).

Adds an `initialExpansion` prop to PierreFileTree so the diff modes
(small change-set) open all folders by default while browse mode
(whole repo) stays "closed".
@srid srid mentioned this pull request Apr 24, 2026
@srid
Copy link
Copy Markdown
Member Author

srid commented Apr 24, 2026

E2e test coverage for the Pierre migration

packages/tests/features/code-tab.feature was rewritten on top of @pierre/trees + @pierre/diffs DOM (commit 35830dc). 14 scenarios, 143 steps, all green locally and on CI.

What's covered

  • Tab presence — Code tab switches active, "not a git repo" message, "no changes" message
  • Mode picker — defaults to Local, persists across panel close/reopen
  • Local mode — lists changed files, opens a diff on click
  • Untracked — modified + untracked files appear together
  • Pierre tree behaviour — directory grouping, click-to-collapse and click-to-expand
  • Pierre tree right-click menu — right-click a changed file → Copy path → clipboard
  • Browse mode — file tree visible, file content rendered on click, lazy directory expansion
  • Pierre file viewer line selection — click line gutter, right-click → Copy <path>:<line> → clipboard

Selectors note

Pierre's <file-tree-container> and <diffs-container> use open shadow roots. Playwright's CSS engine pierces them, so the same [data-testid="pierre-file-tree"] [data-item-path="..."] style selector reaches every visible row from outside. Two Pierre quirks worth documenting in case you write more steps:

  • Folder rows carry a trailing slash on data-item-path (src/, not src); files don't.
  • Pierre renders sticky-folder duplicates of the same row at the top of the viewport. Tests filter them out with :not([data-file-tree-sticky-row]) so assertions land on the real (clickable) row.

Intentionally out of scope

  • Live updates — tracked separately as Live updates for the Code tab's file tree (replace ↻ button) #711, post-merge.
  • Toggle-deselect on second click — Pierre's single-select tree doesn't deselect on re-click, and no built-in API exposes that gesture. Dropped from the suite; if we want it back it needs a real product change, not a test workaround.
  • Diff viewer line selection menu — the file viewer covers the same wiring (same CodeContextMenu, same formatLineRef). Adding a duplicate diff-mode scenario isn't worth the runtime; the unit test on formatLineRef plus the file-mode scenario gives full coverage of the integration seam.
  • Branch missing-origin error — separate flake; not introduced by this PR. The error surface itself is covered by the existing toast in real usage; the e2e race needs its own investigation.

CI

just ci came up green on the second pass after a single flaky e2e@x86_64-linux (codex Context tokens do not double-count cached input tokens scenario, unrelated to this PR — logged on #320). Retry passed cleanly with 236/236 scenarios. All 10 expected contexts reported success on commit 35830dc.

srid added 2 commits April 24, 2026 19:49
…st-effort polling

- `initialExpansion` is captured by Pierre's `FileTree` constructor and
  ignored on later prop changes. Document the constraint so the next
  caller knows to remount the component (toggle a parent `<Show>`) if
  they need a different value at runtime. Today's call site happens to
  remount on the only mode transitions where the value differs
  (Local/Branch ↔ Browse flips the underlying resource), so this is a
  doc-only nudge.
- `waitForChangedFile` swallows `isVisible`/`refresh.click` failures
  by design — re-render races can throw mid-poll and the next tick
  retries. Note the rationale inline so the bare `.catch(...)` pair
  doesn't read like silent-error swallowing.

Code-police pass: rules clean, fact-check clean, elegance clean
(only this doc tightening surfaced; both findings documented rather
than refactored to avoid premature abstraction).
…file viewers

Both `PierreDiffView` and `PierreFileView` repeated the same line-ref
context-menu wiring: a `SelectedLineRange` signal, a reset effect on
path change, an `onLineSelected` adapter, and a `buildItems` returning
"Copy path" plus an optional "Copy <path>:<line>". Two call sites is
below the rule-of-three threshold, but the protocol is locked in
lockstep — adding a third menu entry was a two-file edit.

Centralized in `useLineSelection(path: () => string)`. Each wrapper
shrinks to:
  const selection = useLineSelection(() => props.path);
  // ... onLineSelected: selection.handleSelect
  // ... <CodeContextMenu getItems={selection.buildItems} />

Hickey "Simple Made Easy" finding: concept multiplication. Two
implementations of one user intent; the hook collapses them to one.

Net: -43 lines, no behavior change. Tests still pass (14 scenarios,
143 steps).
@srid
Copy link
Copy Markdown
Member Author

srid commented Apr 24, 2026

Architectural review (Hickey + Lowy)

After the test work landed, ran two parallel architectural lenses over the whole branch.

Hickey — Simple Made Easy

Two findings.

1. Dual context-menu implementationsrenderContextMenu (raw HTMLElement returned to Pierre's composition.contextMenu.render) for the tree, plus CodeContextMenu (Solid <Portal>) for the file/diff viewers. Same user intent ("right-click → copy path"), two implementations. Hickey calls this concept multiplication.

Rejected after consideration. The proposal was to disable Pierre's built-in tree menu and mount CodeContextMenu on the tree container instead. That trade loses two Pierre features:

  • Keyboard activation — triggerMode: "both" covers right-click and the menu key on a focused row
  • Per-row action-lane button — Pierre renders the "•••" affordance via data-item-context-menu-button-visibility when its menu is enabled

Two menu files is the price of crossing Pierre's API boundary; trading it for a UX regression isn't worth it.

2. Line-selection protocol duplicated between PierreDiffView and PierreFileView — same signal + reset effect + onLineSelected adapter + buildItems returning ["Copy path", "Copy <path>:<line>"]. Two call sites is below the rule-of-three threshold, but the wiring is locked in lockstep.

Actioned in 199b718. Extracted useLineSelection(path: () => string) hook (packages/client/src/ui/useLineSelection.ts). Both wrappers now share one source of truth — adding a third menu entry is a single edit. Net -43 lines.

Lowy — Volatility-based decomposition

Six boundaries audited (Risk × Likelihood × Effect). All correctly placed:

  1. Vendor library volatility — Pierre breaking changes are absorbed by the three thin wrappers; nothing leaks into CodeTab or feature tests.
  2. Diff transport — server returns { hunks: string[]; oldFileName, newFileName }. Tightly coupled to unified-diff today; acceptable for current scope.
  3. Mode picker volatility — adding a 4th mode (staged-only, range-of-commits) is a one-file change in CodeTab.tsx.
  4. File content rendering — text-only path is correct for current scope; binary/image preview would need a kind discriminant in the contract (localized change).
  5. Right-click menu — dual coexistence is a consequence of Pierre's API gap (see below), not a boundary error.
  6. E2e selector quirks — Pierre's trailing-slash + sticky-row weirdness is encapsulated in fileRow/dirRow helpers in code_tab_steps.ts. Right level.

Forward note (no current change): if non-git diff sources arrive (LSP, AST), GitDiffOutputSchema's raw unified-diff string would need to become a discriminated union. The schema name already signals git-only scope.


Why does CodeContextMenu exist?

Asked in the review. Short answer: it exists because Pierre's diffs library has no context-menu hook, and we need one for the line-ref copy feature.

Component Pierre API Our implementation
@pierre/trees composition.contextMenu.render renderContextMenu returns an HTMLElement to Pierre
@pierre/diffs (File) None CodeContextMenu mounted next to the host div
@pierre/diffs (FileDiff) None CodeContextMenu mounted next to the host div

Pierre's tree wires the right-click handler, positioning, dismissal, and keyboard activation for us. Pierre's diffs surfaces line/token interactions (onLineSelected, onLineClick, etc.) but stops short of a context-menu hook. So for "right-click on a line → Copy <path>:<line>", we have to roll the menu ourselves.

Alternatives considered:

  • Native browser contextmenu — no styling consistency with kolu's design language; no shared affordances with the tree menu.
  • Generic Solid menu library (corvu, solid-headless) — would add a dep; CodeContextMenu is ~100 lines with <Portal> + outside-click + Escape, all behavior we already wanted to control. The library would have to be wrapped to match the tree menu's look anyway.
  • Wait for Pierre to add composition.contextMenu to diffs — reasonable upstream feature request; until then, the local component is the smallest move that gets the feature shipped.
  • Reuse the tree's renderContextMenu — incompatible APIs. The tree's hook fires per-row with item.path; the diff viewer fires on selection ranges. Different shapes; can't be unified without losing one side's contract.

The current dual-implementation arrangement is what falls out of the upstream API gap. If Pierre ever adds a contextMenu hook to its diffs renderer, CodeContextMenu collapses into a thin adapter (or goes away entirely) and the line-selection hook stays.

@srid srid changed the title Prototype: swap file tree + diff view for @pierre/trees and @pierre/diffs feat(code-tab): swap file tree + diff view for @pierre/trees and @pierre/diffs Apr 25, 2026
@srid srid marked this pull request as ready for review April 25, 2026 00:30
@srid srid merged commit a00e6c5 into master Apr 25, 2026
11 checks passed
@srid srid deleted the pierre-trees-diffs branch April 25, 2026 00:31
srid added a commit that referenced this pull request May 4, 2026
The Pierre adoption (#708) wrapped `@pierre/trees` and `@pierre/diffs`
(vanilla classes that own a Preact-rendered shadow DOM) in three thin
SolidJS shells under `packages/client/src/ui/`. Each wrapper repeated
the same scaffolding — mount the instance, push reactive updates through
imperative setters, call `cleanUp()` on disposal — and each one carried
the same wrapper-shape bugs: parse failures swallowed into
`console.warn`, imperative `.render()` throws escaping Solid's
`<ErrorBoundary>`, a pre-mount `setSearch` silently dropped on the
floor. **`@kolu/solid-pierre` collapses the three wrappers into one safe
boundary**: `<FileTree>`, `<FileDiff>`, `<FileView>` with a required
`onError` prop and a typed `contextMenu` hook. The next Pierre-backed
surface kolu adds — or any future SolidJS consumer of Pierre — gets the
safe wrapper for free.

Closes #807.

### What lives where

| Concern | Lives in |
| --- | --- |
| Pierre lifecycle (mount/render/cleanUp) | `@kolu/solid-pierre` |
| Reactivity bridge (`createEffect(on(...))` setters) |
`@kolu/solid-pierre` |
| Error boundary (Pierre throws → `onError`) | `@kolu/solid-pierre` |
| Typed `contextMenu` exposure of Pierre's `composition.contextMenu` |
`@kolu/solid-pierre` |
| Theme variable overrides (`--trees-*`, `--diffs-*`) |
`packages/client/src/ui/pierreTheme.ts` |
| Nix/Haskell sprite icons | `packages/client/src/ui/pierreTheme.ts` |
| Right-click "Copy `<path>:<line>`" UX | `useLineSelection` +
`CodeContextMenu` + `CodeMenuFrame.tsx` |
| `kolu-git` ↔ Pierre status mapping + tree menu DOM factory |
`packages/client/src/ui/pierreAdapters.ts` |

_The split lines up with the volatility envelope: Pierre-version churn
lands inside the new package; kolu's product surfaces (theming, the
path:line menu) stay in `packages/client/`._

### Wrapper-shape bugs the issue cites — fixed structurally

| Old site | Symptom | Fix |
| --- | --- | --- |
| `PierreDiffView.tsx:34` | `parsePatchFiles` throws caught to
`console.warn` + `return undefined`; blank pane indistinguishable from
an empty diff | Required `onError` callback receives the throw |
| `PierreDiffView.tsx:72` | `instance.render(...)` throws escape Solid's
`<ErrorBoundary>` (which only catches errors during *Solid* component
render) and crash the parent tree | Wrapper try/catch routes every
imperative call to `onError` |
| `PierreFileTree.tsx:200` | `tree?.setSearch(q)` is a silent no-op when
`searchQuery` is set before the imperative instance is mounted |
`onMount` applies the `untrack`'d initial value once; deferred effect
handles subsequent changes |
| `PierreFileView.tsx:51` | `fileContents` rebuilt on every read; effect
deps stale | `createMemo<FileContents>` stabilizes the object identity |

### Refinements during the Hickey/Lowy pass

The primary commit landed first; three follow-up commits address
structural findings independently so the PR history reads as "what was
built, then what was refined":

| Commit | Finding | Fix |
| --- | --- | --- |
| [`969d146e`](../commit/969d146e) | `e instanceof Error ? e : new
Error(String(e))` repeated 9× across the three wrappers | Extract
`toError` util |
| [`cbb5451b`](../commit/cbb5451b) | `FileTreeContextMenu` re-exported
but no consumer imported it; type derivation chain was fragile | Drop
the export; keep the derivation file-private |
| [`fdb73127`](../commit/fdb73127) | `CodeTab.tsx` and
`BrowseFileView.tsx` each repeated the same 6-line host-div +
`useLineSelection` + `CodeContextMenu` scaffold | Extract
`<CodeMenuFrame>` |

### Tests

`code-tab.feature` — 19 scenarios, 214 steps, all passing. The
multi-file diff regression scenario (right-click "Copy `<path>:<line>`"
survives switching diff files) still passes; the keyed remount in
`CodeTab.tsx` continues to give each file a fresh `FileDiff` and a clean
`useLineSelection` range.

> **Out of scope, by design**: virtualization variants
(`VirtualizedFile`, `VirtualizedFileDiff`). No kolu consumer needs them
yet — they'll land with the
[#514](#514) Phase 8 work that
motivated them. The new package's API has room for them without further
restructuring.

_Generated by [`/do`](https://github.com/srid/agency) on Claude Code
(model `claude-opus-4-7`)._
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant