Skip to content

fix(ui,agent-adapter): show nearby rows and line numbers in the inline diff card (CODE-399) - #304

Open
Zerlight wants to merge 5 commits into
masterfrom
ruocheng/code-399
Open

fix(ui,agent-adapter): show nearby rows and line numbers in the inline diff card (CODE-399)#304
Zerlight wants to merge 5 commits into
masterfrom
ruocheng/code-399

Conversation

@Zerlight

Copy link
Copy Markdown
Member

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_string verbatim — which is the replaced fragment: no line numbers, no
context. The SDK does hand the adapter tool_use_result.structuredPatch on the settle frame
(real hunk offsets, three context lines a side), but toolUseResultEnvelope dropped it along
with the other bulk payloads. The adapter now lifts it into the patch field that already
existed, on both the live and history-replay paths.

The renderer discarded what it did get. patchLines skipped every @@ header — the only
place file line numbers exist — so even codex, which already shipped a real git patch, rendered
unnumbered and unhunked. DiffBlock now renders through @pierre/diffs, the same viewer
already behind the right-panel git diff and the Files viewer: numbered gutter, hunk separators,
context rows, syntax highlighting.

Also in here:

  • File cards clamp a long body to a peek with an Expand / Collapse control in the frame footer.
    The control only appears when the body actually overflows, so short diffs are untouched.
  • A shared unifiedPatchText helper in @linkcode/schema, replacing a hand-rolled copy in
    im-render.
  • Root vitest gains a setup file polyfilling ResizeObserver and Element.prototype.getAnimations;
    jsdom has neither and pierre / base-ui need both.

Decisions worth flagging for review:

  • Write keeps whole-file newText, no patch. Its dominant create case ships an empty
    structuredPatch, and for a create the whole file beats an all-+ patch.
  • oldText / newText stay alongside patch, as codex already does: consumers prefer
    patch when present, and file-tool-presentation derives "this was a deletion" from the text
    fields, so dropping them would silently change navigation.
  • Headerless patches are normalized in the UI. Codex and the new claude path both emit bare
    @@ hunk streams, and pierre parses zero hunks out of those — the card would render blank —
    so DiffBlock synthesizes a --- path / +++ path preamble.
  • No WIRE_PROTOCOL_VERSION bump. 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/wire touched.

Verification

pnpm check:ci and pnpm test both pass (1967 passed, 1 skipped).

New coverage: the chatFileDiff branches (headerless codex patch, multi-hunk git patch,
fragment-only text, create / delete / rename, binary), the peek overflow gate, the adapter's
editResultDiffContent extractor, and live-vs-history shape parity for the replayed patch.

Rendering was driven through headless playwright against dev:mock rather than the in-app
preview pane — that pane runs document.hidden, which freezes the frame loop, so
ResizeObserver never delivers and the overflow gate reads as broken when it isn't. In a real
frame 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 a
manual pass before merge — make an Edit, reload the thread, confirm the replayed card matches
the live one.

Checklist

  • pnpm check:ci and pnpm test both pass
  • I ran the affected surface and observed the change working — mock renderer; see the caveat above
  • If a wire message changed: WIRE_PROTOCOL_VERSION is bumped — n/a, no wire variant changed
  • New code and assets are my own work, or their origin and license compatibility are noted above
  • Docs and comments are updated where behavior changed

@linear-code

linear-code Bot commented Jul 28, 2026

Copy link
Copy Markdown

CODE-399

@lucas77778 lucas77778 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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.

2 participants