fix(ui,agent-adapter): show nearby rows and line numbers in the inline diff card (CODE-399) - #304
fix(ui,agent-adapter): show nearby rows and line numbers in the inline diff card (CODE-399)#304Zerlight wants to merge 5 commits into
Conversation
There was a problem hiding this comment.
Request changes
The overall direction is sound, but the blocking issue below should be fixed before merge.
[P1] A panel mounted after the first commit never registers its ResizeObserver
packages/presentation/ui/src/chat/file-preview-card.tsx:57-70,115-147
subscribeToPanelSize is wrapped in a useCallback with no dependencies and reads panelRef.current only when the subscription is created. A live Read first mounts as a header-only card, so the ref is null and the subscription returns noop. When the settle later mounts the body panel on the same card instance, the subscribe function is unchanged and useSyncExternalStore does not resubscribe. The long body is therefore clipped by chat-card-peek while panelOverflowing remains permanently false, leaving no fade and no Expand button.
This was reproduced against the PR head by rendering the header-only card first, then rerendering it with a body whose scrollHeight=600 and clientHeight=200: the ResizeObserver constructor count remained 0 and the Expand button was absent. Store the panel element through a callback ref and make the element a dependency of the subscribe/getSnapshot callbacks. Please also add a header-only → late-body regression test.
[P2] Subagent Edit history replay does not recover structuredPatch
packages/host/agent-adapter/src/native/claude-code.ts:1643-1655
The main-agent mapper receives supplement.toolUsePatches, but readSubagentTranscripts still calls createClaudeHistoryEventMapper(asHistoryId(sessionId)) without a patch supplement. The SDK's getSubagentMessages projection drops toolUseResult, so a subagent Edit has real hunks and line numbers while live, then falls back to the announce-time old/new fragment after reload. Build a patch supplement from each raw subagent transcript and add a live/history parity test.
[P2] Keyboard focus can enter visually clipped Markdown controls
packages/presentation/ui/src/styles.css:346-361
packages/presentation/ui/src/chat/file-preview-card.tsx:117-147
overflow: clip hides content visually but does not remove links, checkboxes, or other controls from the tab order, and the Expand button comes after the panel. Keyboard users can therefore focus an invisible control, while clip prevents scrolling it into view. Make the collapsed and overflowing panel inert until it is expanded, or automatically expand when focus enters the panel, and add keyboard coverage.
[P3] Diff statistics disagree with the rendered text fallback
packages/presentation/ui/src/chat/diff-block.tsx:69-86
packages/presentation/ui/src/chat/diff-utils.ts:102-125
chatFileDiff falls back to oldText/newText when a patch produces no hunks, but diffContentStats still uses the empty or invalid patch and reports 0/0. The card can show a visible diff with no addition/deletion badge. When patchLines is empty and text is available, diffStats should fall back to diffLines as well.
Remote CI is currently green and git diff --check passes; these cases are not covered by the existing tests.
Summary
The inline diff card in chat showed a bare list of changed lines — no line numbers, and for
claude-code no surrounding rows at all, so you couldn't tell where in a file an edit landed.
Two independent causes, both fixed here.
The data never arrived. claude-code built the card from the tool input —
old_string/new_stringverbatim — which is the replaced fragment: no line numbers, nocontext. The SDK does hand the adapter
tool_use_result.structuredPatchon the settle frame(real hunk offsets, three context lines a side), but
toolUseResultEnvelopedropped it alongwith the other bulk payloads. The adapter now lifts it into the
patchfield that alreadyexisted, on both the live and history-replay paths.
The renderer discarded what it did get.
patchLinesskipped every@@header — the onlyplace file line numbers exist — so even codex, which already shipped a real git patch, rendered
unnumbered and unhunked.
DiffBlocknow renders through@pierre/diffs, the same vieweralready behind the right-panel git diff and the Files viewer: numbered gutter, hunk separators,
context rows, syntax highlighting.
Also in here:
The control only appears when the body actually overflows, so short diffs are untouched.
unifiedPatchTexthelper in@linkcode/schema, replacing a hand-rolled copy inim-render.ResizeObserverandElement.prototype.getAnimations;jsdom has neither and pierre / base-ui need both.
Decisions worth flagging for review:
newText, no patch. Its dominantcreatecase ships an emptystructuredPatch, and for a create the whole file beats an all-+patch.oldText/newTextstay alongsidepatch, as codex already does: consumers preferpatchwhen present, andfile-tool-presentationderives "this was a deletion" from the textfields, so dropping them would silently change navigation.
@@hunk streams, and pierre parses zero hunks out of those — the card would render blank —so
DiffBlocksynthesizes a--- path/+++ pathpreamble.WIRE_PROTOCOL_VERSIONbump.patch: { format: 'git_patch', text }already existed(CODE-388); this adds a plain TS interface and a function, no zod variant changed, nothing
under
src/wiretouched.Verification
pnpm check:ciandpnpm testboth pass (1967 passed, 1 skipped).New coverage: the
chatFileDiffbranches (headerless codex patch, multi-hunk git patch,fragment-only text, create / delete / rename, binary), the peek overflow gate, the adapter's
editResultDiffContentextractor, and live-vs-history shape parity for the replayed patch.Rendering was driven through headless playwright against
dev:mockrather than the in-apppreview pane — that pane runs
document.hidden, which freezes the frame loop, soResizeObservernever delivers and the overflow gate reads as broken when it isn't. In a realframe loop: a multi-hunk fixture renders absolute line numbers (12–19, then
@@ -84,9 +85,11 @@,then 85–90) with context on both sides; the overflowing card gets the clamp, fade and toggle,
and the short one correctly gets none of them.
Not verified: the history-replay half against a real daemon. The mock host rejects
history.read, so cold-resume parity for a claude Edit was only checked at unit level. Worth amanual pass before merge — make an Edit, reload the thread, confirm the replayed card matches
the live one.
Checklist
pnpm check:ciandpnpm testboth passWIRE_PROTOCOL_VERSIONis bumped — n/a, no wire variant changed