refactor(native-chat): tidy what the diff-card work left behind

The copy text is joined from every row of the diff, which a collapsed card
renders none of, and it was rebuilt on every render to seed a prop. It is
memoized on the rows, matching how the run memoizes its edit model.

The two scanners that read patch text kept the same file-section alternation
verbatim, so they could drift apart while both looking correct; there is one
definition now, beside the header-pair rule that already lives there.

Also: the row that marks a break between regions is built in one place, so it
is no longer exported; the move destination in the envelope reader was a
function-wide binding written and read within one iteration, which read as if
a move carried between sections; and a test comment named the wrong mechanism
for keeping a card collapsed.

Adds the missing pin on what the copy affordance actually copies.
This commit is contained in:
Merge Sim
2026-09-04 22:05:46 -07:00
parent 15db74cab5
commit 31a756ee3a
6 changed files with 49 additions and 22 deletions
@@ -1,4 +1,4 @@
import { useState } from 'react'
import { useMemo, useState } from 'react'
import { ChevronRight, FilePlus2, FileMinus2, FilePen } from 'lucide-react'
import { cn } from '@/lib/utils'
import { translate } from '@/i18n/i18n'
@@ -38,8 +38,8 @@ function baseName(path: string): string {
return path.split(/[\\/]/).at(-1) || path
}
function patchText(file: NativeChatEditFile): string {
return file.lines
function patchText(lines: readonly NativeChatEditLine[]): string {
return lines
.filter((line) => line.kind !== 'gap')
.map((line) => `${line.kind === 'add' ? '+' : line.kind === 'del' ? '-' : ' '}${line.text}`)
.join('\n')
@@ -117,6 +117,9 @@ export function NativeChatDiffCard({
initiallyExpanded?: boolean
}): React.JSX.Element {
const [expanded, setExpanded] = useState(initiallyExpanded)
// Joining every row to seed the copy button is the card's most expensive
// work, and a collapsed card renders none of those rows.
const copyText = useMemo(() => patchText(file.lines), [file.lines])
const hasBody = file.lines.length > 0
const widest = file.lineNumbersKnown
? file.lines.reduce((max, line) => Math.max(max, unifiedLineNumber(line) ?? 0), 0)
@@ -171,7 +174,7 @@ export function NativeChatDiffCard({
</span>
) : null}
<NativeChatCopyButton
text={patchText(file)}
text={copyText}
label={translate('components.native-chat.tool.copyDiff', 'Copy diff')}
className="ml-auto shrink-0"
/>
@@ -2,8 +2,8 @@
import '@testing-library/jest-dom/vitest'
import { cleanup, render, screen } from '@testing-library/react'
import { afterEach, describe, expect, it } from 'vitest'
import { cleanup, fireEvent, render, screen } from '@testing-library/react'
import { afterEach, describe, expect, it, vi } from 'vitest'
import type { AgentJournalRenderItem } from '../../../../shared/agent-session-journal-types'
import type { NativeChatBlock } from '../../../../shared/native-chat-types'
import { projectStructuredItemToNativeChat } from '../../../../shared/structured-agent-session-projection'
@@ -196,13 +196,42 @@ describe('NativeChatToolRun', () => {
{ type: 'tool-result', output: '@@ -1,3 +1,3 @@\n ctx\n-was\n+now\n… (48210 bytes)' }
]
// expandSignal false leaves every card's body closed.
// A defined expandOverride opens the run while leaving each card closed.
render(<NativeChatToolRun blocks={blocks} expandSignal={false} expandOverride />)
expect(screen.getByText('Diff truncated')).toBeInTheDocument()
expect(screen.queryByText('was')).toBeNull()
})
it('copies the diff as signed rows, with the region breaks left out', () => {
const writeClipboardText = vi.fn()
Object.assign(window, { api: { ui: { writeClipboardText } } })
const blocks: NativeChatBlock[] = [
{
type: 'tool-call',
name: 'Edit',
input: { file_path: '/repo/a.ts' },
state: 'completed'
},
{
type: 'tool-result',
output: 'ok',
editPatch: {
filePath: '/repo/a.ts',
hunks: [
{ oldStart: 1, oldLines: 2, newStart: 1, newLines: 2, lines: [' ctx', '-was', '+now'] },
{ oldStart: 90, oldLines: 1, newStart: 90, newLines: 1, lines: ['+tail'] }
]
}
}
]
render(<NativeChatToolRun blocks={blocks} expandSignal />)
fireEvent.click(screen.getByRole('button', { name: 'Copy diff' }))
expect(writeClipboardText).toHaveBeenCalledWith(' ctx\n-was\n+now\n+tail')
})
it('keeps a grouped active run to one stable row showing only the latest tool', () => {
const blocks: NativeChatBlock[] = [
{ type: 'tool-call', name: 'shell', input: { command: 'date' }, state: 'completed' },
+1 -3
View File
@@ -108,7 +108,6 @@ function envelopeArgument(
/** Splits a `*** Begin Patch` envelope into one entry per file it touches. */
export function editFilesFromBeginPatch(envelope: string): NativeChatEditFile[] {
const sections: { kind: 'Add' | 'Update' | 'Delete'; path: string; body: string[] }[] = []
let movePath: string | null = null
const moves = new Map<number, string>()
// Split on both newline forms once, so every marker below can be matched
@@ -125,8 +124,7 @@ export function editFilesFromBeginPatch(envelope: string): NativeChatEditFile[]
}
const move = MOVE_HEADER.exec(raw)
if (move && sections.length > 0) {
movePath = move[1]!.trim()
moves.set(sections.length - 1, movePath)
moves.set(sections.length - 1, move[1]!.trim())
continue
}
if (raw === BEGIN || raw === END || CONTROL_LINE.test(raw) || sections.length === 0) {
+5 -2
View File
@@ -14,8 +14,11 @@ const DIFF_TRUNCATED_LINE: NativeChatDiffLine = {
}
const HUNK_HEADER = /^@@ -\d+(?:,\d+)? \+\d+(?:,\d+)? @@/
// Lines that open a new file section, so any hunk before them has ended.
const FILE_SECTION_START =
/** Lines that open a new file section, so any hunk before them has ended.
* `--- `/`+++ ` are deliberately absent: inside a hunk they are content — a
* removed `-- comment` is emitted as `--- comment` — so they go through
* `isFileHeaderPair` instead. */
export const FILE_SECTION_START =
/^(?:diff |index |old mode |new mode |new file mode |deleted file mode |similarity index |dissimilarity index |rename |copy |Binary files )/
// Markdown thematic break or YAML document separator, not a marker.
const BARE_RULE = /^(?:-{3,}|\+{3,})$/
+1 -1
View File
@@ -54,7 +54,7 @@ export function splitEditContent(content: string): EditContentLines {
}
/** The break between two regions of a file. Carries no text and no position. */
export function editGapLine(): NativeChatEditLine {
function editGapLine(): NativeChatEditLine {
return { kind: 'gap', text: '', oldLineNumber: null, newLineNumber: null }
}
+3 -9
View File
@@ -1,13 +1,7 @@
import { isFileHeaderPair } from './native-chat-diff'
import { FILE_SECTION_START, isFileHeaderPair } from './native-chat-diff'
import { pushEditGap, splitEditContent, type NativeChatEditLine } from './native-chat-edit-model'
const HUNK_RANGES = /^@@+ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@/
/** Structural lines that open a file section, so any hunk before them has ended.
* `--- `/`+++ ` are deliberately absent: inside a hunk they are content — a
* removed `-- comment` is emitted as `--- comment` — so they go through
* `isFileHeaderPair` instead. */
const FILE_SECTION =
/^(?:diff |index |old mode |new mode |new file mode |deleted file mode |similarity index |dissimilarity index |rename |copy |Binary files )/
export type UnifiedPatchLines = {
lines: NativeChatEditLine[]
@@ -59,7 +53,7 @@ export function editLinesFromUnifiedPatch(
index += 1
continue
}
if (FILE_SECTION.test(raw)) {
if (FILE_SECTION_START.test(raw)) {
inHunk = false
continue
}
@@ -186,7 +180,7 @@ export function unifiedPatchSections(text: string): {
}
if (raw.startsWith('@@')) {
inHunk = true
} else if (FILE_SECTION.test(raw)) {
} else if (FILE_SECTION_START.test(raw)) {
inHunk = false
}
current ??= open()