A focused Hickey + Lowy review of the Code-tab subsystem (right-panel diff/browse) found three narrow structural fixes worth landing. Methodology mirrors #601 — surface-level sweep surfaces candidates, focused re-review on each one lands the verdict. Unlike #601's 6 candidates (4 shipped, 2 false positives), this pass found exactly 3 real items and green-lit the rest ("CodeTab's 494 LOC", dual simpleGit+execFile in review.ts, fs.*-namespace-hiding-git-filter, GitResult→ORPCError boundary translation) as correctly decomposed.

Ordered by impact:

1. CodeTab bypasses the streaming-transport wrapper (Lowy) → tracked in #616

packages/client/src/right-panel/CodeTab.tsx:151,163,200,221 call client.git.status/diff and client.fs.listDir/readFile directly. Every other server-state consumer in Kolu routes through the stream namespace in packages/client/src/rpc/rpc.ts so ClientRetryPlugin can transparently re-subscribe on WebSocket drops — preferences, activity, session, metadata have all migrated. CodeTab is the one holdout.

• Lowy lens: "pull vs server-push" is a declared volatility axis (see .claude/rules/lowy-volatilities.md) and has already shifted three times in this codebase. The moment a repo-watcher lands (a plausible next feature — e.g. file-tree invalidation on git checkout), CodeTab's four createResource call sites all get rewritten.

What to do: add stream.git.{status,diff,worktreeCreate,worktreeRemove} and stream.fs.{listDir,readFile} in rpc.ts aliasing to client.* today. Zero behavioral change; future server-push migration lands without touching CodeTab.tsx.

Now tracked in #616, which bundles this item with the server-side streaming endpoints from Phase 5 of #514 ("Live file watching"). Shipping them separately is net-zero — the wrapper is pure aliasing without a streaming server to point at. Shipping them together: one migration, one PR.

2. oldPath typing is too loose (Hickey)

packages/integrations/git/src/schemas.ts:49-55 — GitChangedFileSchema types oldPath as z.string().optional(). The invariant is "present iff status ∈ {R, C}" (rename/copy), enforced only by a comment. The client must remember to thread oldPath back into git.diff's input on those rows (CodeTab.tsx:160) — a comment-enforced coupling across two RPC boundaries.

• Hickey lens: two facts that are mutually constrained ("status tells you whether oldPath is present") braided into one optional field. The compiler can enforce this.

What to do: replace GitChangedFileSchema with a discriminated union: {status: "R" | "C", path, oldPath: string} | {status: "M" | "A" | "D" | "U" | "T" | "?", path, oldPath?: never}. Moves the invariant from comment to compile-time check.

3. Raw git diff strings cross the wire (Hickey + Lowy)

packages/integrations/git/src/schemas.ts:101-111 — GitDiffOutputSchema.hunks: z.array(z.string()). Server runs git diff, gets structured output, stringifies it, sends it across; client re-parses via @git-diff-view/solid. The server generated the structure and threw it away.

• Hickey lens: the schema says string[] but the invariant is "valid unified-diff" — enforced only by whatever library happens to parse it downstream. Classic complect of "what" (the diff) with "how it's serialized" (git CLI text format).
• Lowy lens: also a thin leak of review.ts's provider choice (git CLI) into the wire shape — the comment at schemas.ts:106-110 admits the passthrough explicitly. Today's blast radius is small (one consumer, @git-diff-view).

What to do: parse server-side in review.ts, ship { header: {oldStart, oldLines, newStart, newLines}, lines: Array<{kind: "context" | "add" | "del", text}> } (or whatever shape matches @git-diff-view's internal data model). Defer until a second diff consumer lands or the provider swaps, if the current layout is load-bearing for the single consumer.

Priority

Item 1 is now tracked in #616 — it ships as part of the Phase 5 live-watching migration rather than standalone aliasing. 2 next (tiny, moves an invariant to the type system). 3 is deferable if the only consumer stays @git-diff-view.

What was explicitly green-lit (leave alone)

• fs.* contract namespace hiding git-filtering — deliberate semantic inversion; user concern is "browse the repo's files," git-filter is the mechanism.
• buildFileTree(changed-files) vs paginated listDir loads feeding the same <FileTree> component — inputs are semantically distinct; converging them would force browse mode to fabricate git status.
• simpleGit + execFile coexisting in review.ts — staged fallback, not competing styles. execFile handles git diff --no-index's exit-1 convention that simpleGit fights.
• GitResult → ORPCError at the router boundary via unwrapGit() — textbook boundary discipline; concentrated in one helper.
• CodeTab.tsx at 494 LOC across three sub-views — the right-panel slot IS the grouping axis. Split only if browse grows search/blame/find-in-files.
• 1 MB MAX_READ_BYTES in readFile — encapsulated volatility (choose a number, ship it), not a missing pagination axis.

Related

• Structural cleanup pass before codebase grows (Hickey + Lowy review) #601 — the parent roadmap that triggered the review methodology (sweep + focused re-review).
• refactor: unify agent detection behind one AgentProvider contract #603, refactor(meta): fence TerminalMetadata by write authority #611, refactor(git): kolu-git owns the resolve+watch compose loop #613 — prior structural-cleanup ships that used the same workflow.