feat(code-tab): swap file tree + diff view for @pierre/trees and @pierre/diffs#708
feat(code-tab): swap file tree + diff view for @pierre/trees and @pierre/diffs#708
Conversation
…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.
This reverts commit f989baa.
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.
…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".
E2e test coverage for the Pierre migration
What's covered
Selectors note Pierre's
Intentionally out of scope
CI
|
…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).
Architectural review (Hickey + Lowy)After the test work landed, ran two parallel architectural lenses over the whole branch. Hickey — Simple Made EasyTwo findings. 1. Dual context-menu implementations — Rejected after consideration. The proposal was to disable Pierre's built-in tree menu and mount
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 Actioned in 199b718. Extracted Lowy — Volatility-based decompositionSix boundaries audited (
Forward note (no current change): if non-git diff sources arrive (LSP, AST), Why does
|
| 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;CodeContextMenuis ~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.contextMenuto 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 withitem.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.
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`)._
Why
The Code tab's file tree and unified diff were hand-rolled on top of
buildFileTree, a plain SolidJSFileTree,@git-diff-view/solid, andhighlight.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(...)), andcleanUp()on disposal — no Solid re-render loop.PierreFileTree.tsx— wrapsnew FileTree({...}), surfacesonSelect, custom right-click menu viacomposition.contextMenu, custom sprite sheet for nix/hs icons.PierreDiffView.tsx— wrapsnew FileDiff({...}), parses raw unified diffs viaparsePatchFiles(). Server hands ushunks: string[]with full headers — no shape juggling.PierreFileView.tsx— wraps Pierre'sFilefor 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.listAllendpoint backed bygit ls-files --cached --others --exclude-standardreturns tracked + untracked paths in one round trip; Pierre takes the flat list and infers the tree structure — no lazy-load dance.Server simplifications.
getDiffno longer reads old/new file contents (Pierre parses unified diffs directly), andlistDiris gone (replaced bylistAll).oldContent/newContentdropped fromGitDiffOutputSchema.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.featurecover: tab presence, mode picker + persistence, file listing, diff rendering, directory grouping, collapse/expand, right-clickCopy pathfrom the tree, line-gutter selection + right-clickCopy <path>:<line>from the file viewer.Skill for posterity.
.apm/skills/pierre/SKILL.mddocuments 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
/hickeyand/lowyreviews in #issuecomment-4317109853. One Hickey finding actioned (useLineSelectionhook 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 checkgreen across all packagesjust cigreen on199b718— all 10 contexts pass on first run--trees-*and--diffs-*CSS variables; nix/hs icons via custom sprite sheettree.add/remove/move) tracked separately as Live updates for the Code tab's file tree (replace ↻ button) #711🤖 Generated with Claude Code