From 615524e9ce7d271df59bc24825fba22b313c483f Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Fri, 4 Sep 2026 20:44:22 -0700 Subject: [PATCH] fix(native-chat): keep every edited file, and mark where the diff breaks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A run of hunks was concatenated into one flat row list, so the gutter jumped from one region of the file to a distant one with nothing between them and the reader saw two unrelated spans as one continuous block. Rows now carry an explicit break: it holds no text and no position, counts toward neither side of the change, is trimmed from the end where it would mark nothing, and is left out of the copied text. The patch envelope lost files, and lost them silently: - An update chunk may carry no hunk header at all. The parser required one, returned nothing, and the caller dropped that file from a multi-file envelope with nothing to say it had gone. A header-less body now opens as a hunk of unknown position, and whether the rows are locatable is read off the rows themselves rather than off the header. - The envelope's own control lines rendered as content rows in the card. - A delete names its file and carries no body, which rendered as a card with an empty expandable row list. The header states the change and offers no disclosure behind it. - The header patterns are anchored and `.` excludes a carriage return, so a CRLF envelope matched no header at all and produced no card whatsoever. The envelope is split on both newline forms once, up front, rather than each pattern having to tolerate the extra character. A tool call's argument payload arrives as a string holding JSON. It was passed along undecoded, which is the only reason this code carried a hand-rolled string-literal unescaper. It is decoded once at the transcript decoder now — defensively, since the transcript is untrusted, so anything that is not a JSON object is left exactly as it arrived — and the unescaper is gone. Recovering the envelope no longer guesses at argument names either: it looks at the values, including the words of an argument vector, which is where the envelope actually sits once the payload is decoded. --- .../transcript-line-decoders-codex.ts | 20 ++- ...anscript-reader-codex-history-mode.test.ts | 33 +++++ .../native-chat/transcript-reader.test.ts | 3 +- .../native-chat/NativeChatDiffCard.tsx | 47 +++++-- .../native-chat/NativeChatToolRun.test.tsx | 45 +++++++ src/renderer/src/i18n/locales/en.json | 1 + src/shared/native-chat-begin-patch.ts | 48 +++---- src/shared/native-chat-edit-model.ts | 30 ++++- src/shared/native-chat-edit-normalize.test.ts | 124 ++++++++++++++++-- src/shared/native-chat-edit-normalize.ts | 6 + src/shared/native-chat-unified-patch.ts | 36 ++--- 11 files changed, 328 insertions(+), 65 deletions(-) diff --git a/src/main/native-chat/transcript-line-decoders-codex.ts b/src/main/native-chat/transcript-line-decoders-codex.ts index c4b0ddbf5ba..8c55ec4a69f 100644 --- a/src/main/native-chat/transcript-line-decoders-codex.ts +++ b/src/main/native-chat/transcript-line-decoders-codex.ts @@ -202,13 +202,29 @@ function codexTurnItemBlocks(content: unknown): NativeChatBlock[] { return blocks } +/** A call's arguments arrive as a string holding JSON, so decode them once here + * rather than leaving every consumer to unescape the payload for itself. The + * transcript is untrusted, so anything that is not a JSON object stays exactly + * as it arrived. */ function codexCallInput(payload: Record): unknown { - if (payload.arguments !== undefined) { - return payload.arguments + const args = payload.arguments + if (args !== undefined) { + return typeof args === 'string' ? (parsedJsonObject(args) ?? args) : args } return payload.input ?? payload.action ?? null } +function parsedJsonObject(value: string): Record | null { + try { + const parsed: unknown = JSON.parse(value) + return typeof parsed === 'object' && parsed !== null && !Array.isArray(parsed) + ? (parsed as Record) + : null + } catch { + return null + } +} + function codexToolResult(output: unknown): NativeChatBlock { const record = asRecord(output) const isError = record?.success === false || record?.is_error === true diff --git a/src/main/native-chat/transcript-reader-codex-history-mode.test.ts b/src/main/native-chat/transcript-reader-codex-history-mode.test.ts index 5b0f3dc014f..23e503ae0a4 100644 --- a/src/main/native-chat/transcript-reader-codex-history-mode.test.ts +++ b/src/main/native-chat/transcript-reader-codex-history-mode.test.ts @@ -247,4 +247,37 @@ describe('Codex transcript history modes', () => { blocks: [{ type: 'tool-result', output: 'ok' }] }) }) + + it('decodes a call argument payload once, so consumers see real values', () => { + const call = decodeCodexTranscriptLine( + JSON.stringify({ + type: 'response_item', + payload: { + type: 'function_call', + id: 'call-2', + name: 'shell', + arguments: JSON.stringify({ command: ['bash', '-lc', 'echo one\necho two'] }) + } + }), + 'fallback-args' + ) + + expect(call?.blocks[0]).toMatchObject({ + type: 'tool-call', + name: 'shell', + input: { command: ['bash', '-lc', 'echo one\necho two'] } + }) + }) + + it('leaves an argument payload that is not an object exactly as it arrived', () => { + const call = decodeCodexTranscriptLine( + JSON.stringify({ + type: 'response_item', + payload: { type: 'function_call', id: 'call-3', name: 'shell', arguments: '{ not json' } + }), + 'fallback-bad-args' + ) + + expect(call?.blocks[0]).toMatchObject({ type: 'tool-call', input: '{ not json' }) + }) }) diff --git a/src/main/native-chat/transcript-reader.test.ts b/src/main/native-chat/transcript-reader.test.ts index ff48548804d..8de4e8892ec 100644 --- a/src/main/native-chat/transcript-reader.test.ts +++ b/src/main/native-chat/transcript-reader.test.ts @@ -246,10 +246,11 @@ describe('readNativeChatTranscript (codex)', () => { expect(roles).toEqual(['user', 'reasoning', 'assistant', 'tool', 'assistant']) const call = result.messages.find((m) => m.blocks[0]?.type === 'tool-call') + // The argument payload is decoded once here rather than by every consumer. expect(call?.blocks[0]).toEqual({ type: 'tool-call', name: 'shell', - input: '{"command":["bash","-lc","make"]}' + input: { command: ['bash', '-lc', 'make'] } }) const toolResult = result.messages.find((m) => m.blocks[0]?.type === 'tool-result') diff --git a/src/renderer/src/components/native-chat/NativeChatDiffCard.tsx b/src/renderer/src/components/native-chat/NativeChatDiffCard.tsx index 7adf3f12736..115ce9de70b 100644 --- a/src/renderer/src/components/native-chat/NativeChatDiffCard.tsx +++ b/src/renderer/src/components/native-chat/NativeChatDiffCard.tsx @@ -40,11 +40,29 @@ function baseName(path: string): string { function patchText(file: NativeChatEditFile): string { return file.lines + .filter((line) => line.kind !== 'gap') .map((line) => `${line.kind === 'add' ? '+' : line.kind === 'del' ? '-' : ' '}${line.text}`) .join('\n') } +/** The break between two regions of the file, quiet enough not to read as a + * row of content but present enough that the gutter's jump is accounted for. */ +function DiffGapRow(): React.JSX.Element { + return ( +
+ ⋯ +
+ ) +} + function DiffRow({ line, gutterWidth }: { line: NativeChatEditLine; gutterWidth: number }) { + if (line.kind === 'gap') { + return + } return (
0 const widest = file.lineNumbersKnown ? file.lines.reduce((max, line) => Math.max(max, unifiedLineNumber(line) ?? 0), 0) : 0 @@ -107,20 +127,25 @@ export function NativeChatDiffCard({
{file.oldPath ? ( @@ -144,7 +169,7 @@ export function NativeChatDiffCard({ className="ml-auto shrink-0" />
- {expanded ? ( + {hasBody && expanded ? ( // Focusable so the rows can be scrolled from the keyboard.
{ expect(screen.getByText('was').closest('div')?.textContent).toBe('-was') }) + it('separates two regions of a file so the gutter jump is accounted for', () => { + 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: 42, oldLines: 1, newStart: 42, newLines: 1, lines: ['-was', '+now'] }, + { oldStart: 310, oldLines: 1, newStart: 310, newLines: 1, lines: ['-old', '+new'] } + ] + } + } + ] + + render() + + const separators = screen.getAllByRole('separator') + expect(separators).toHaveLength(1) + expect(separators[0]).toHaveAccessibleName('Lines not shown') + }) + + it('offers no empty body for a delete, which names the file and nothing else', () => { + const blocks: NativeChatBlock[] = [ + { + type: 'tool-call', + name: 'apply_patch', + input: { input: '*** Begin Patch\n*** Delete File: gone.ts\n*** End Patch' }, + state: 'completed' + } + ] + + render() + + expect(screen.getByTitle('gone.ts')).toBeInTheDocument() + // The header states the change; there is no body behind a disclosure. + expect(screen.getByText('Deleted file').closest('button')).not.toHaveAttribute('aria-expanded') + }) + 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' }, diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 6cc8072f840..243771fee8f 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -16964,6 +16964,7 @@ "deletedFile": "Deleted file", "renamedFile": "Renamed file", "copyDiff": "Copy diff", + "diffGap": "Lines not shown", "diffTruncated": "Diff truncated" }, "providerFrame": { diff --git a/src/shared/native-chat-begin-patch.ts b/src/shared/native-chat-begin-patch.ts index ee9736f393f..f409108d188 100644 --- a/src/shared/native-chat-begin-patch.ts +++ b/src/shared/native-chat-begin-patch.ts @@ -5,15 +5,17 @@ const BEGIN = '*** Begin Patch' const END = '*** End Patch' const FILE_HEADER = /^\*\*\* (Add|Update|Delete) File: (.+)$/ const MOVE_HEADER = /^\*\*\* Move to: (.+)$/ +/** Envelope structure that carries no file content of its own. */ +const CONTROL_LINE = /^\*\*\* (?:End of File|Environment ID:)/ -/** Codex sends `apply_patch` as source for its `exec` tool, so the envelope - * arrives inside a JavaScript string literal. Recover the envelope text. */ +/** The envelope reaches a tool call as one of its argument values, either whole + * or as an element of the argument vector the agent runs. Recover its text. */ export function unwrapBeginPatch(input: unknown): string | null { const source = typeof input === 'string' ? input : typeof input === 'object' && input !== null - ? firstStringField(input as Record) + ? envelopeArgument(input as Record) : null if (!source) { return null @@ -29,38 +31,37 @@ export function unwrapBeginPatch(input: unknown): string | null { // render as file content the agent never wrote. return null } - const region = source.slice(start, end + END.length) - // A region with no real newlines is still a single-line string literal. - return region.includes('\n') ? region : decodeStringLiteral(region) + return source.slice(start, end + END.length) } -function firstStringField(record: Record): string | null { - for (const key of ['input', 'command', 'patch', 'arguments', 'script']) { - const value = record[key] +/** Any argument value may hold the envelope, including one word of an argument + * vector, so look at the values rather than guessing at key names. */ +function envelopeArgument(record: Record): string | null { + for (const value of Object.values(record)) { if (typeof value === 'string' && value.includes(BEGIN)) { return value } + if (Array.isArray(value)) { + const word = value.find( + (entry): entry is string => typeof entry === 'string' && entry.includes(BEGIN) + ) + if (word) { + return word + } + } } return null } -function decodeStringLiteral(value: string): string { - return value.replace(/\\(u[0-9a-fA-F]{4}|.)/g, (_match, escape: string) => { - if (escape.startsWith('u')) { - return String.fromCharCode(Number.parseInt(escape.slice(1), 16)) - } - const known: Record = { n: '\n', t: '\t', r: '\r', b: '\b', f: '\f', v: '\v' } - return known[escape] ?? escape - }) -} - /** 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() - for (const raw of envelope.split('\n')) { + // Split on both newline forms once, so every marker below can be matched + // exactly rather than each pattern having to tolerate a trailing `\r`. + for (const raw of envelope.split(/\r?\n/)) { const header = FILE_HEADER.exec(raw) if (header) { sections.push({ @@ -76,7 +77,7 @@ export function editFilesFromBeginPatch(envelope: string): NativeChatEditFile[] moves.set(sections.length - 1, movePath) continue } - if (raw === BEGIN || raw === END || sections.length === 0) { + if (raw === BEGIN || raw === END || CONTROL_LINE.test(raw) || sections.length === 0) { continue } sections.at(-1)!.body.push(raw) @@ -103,7 +104,10 @@ export function editFilesFromBeginPatch(envelope: string): NativeChatEditFile[] }) ] } - const parsed = editLinesFromUnifiedPatch(body) + // The first chunk of an update may carry no hunk header at all, and a file + // whose body cannot be read as a hunk would otherwise vanish from a + // multi-file envelope with nothing to say it was dropped. + const parsed = editLinesFromUnifiedPatch(body, { implicitFirstHunk: true }) if (!parsed) { return [] } diff --git a/src/shared/native-chat-edit-model.ts b/src/shared/native-chat-edit-model.ts index b5d729c0c6a..1b0beef2c78 100644 --- a/src/shared/native-chat-edit-model.ts +++ b/src/shared/native-chat-edit-model.ts @@ -1,6 +1,8 @@ /** One rendered diff row. Numbers are per side: a removed row has no new-side - * number and an added row has no old-side number. */ -export type NativeChatEditLineKind = 'context' | 'add' | 'del' + * number and an added row has no old-side number. `gap` marks the break + * between two regions of the file, which are otherwise concatenated and read + * as one continuous block even as the gutter jumps hundreds of lines. */ +export type NativeChatEditLineKind = 'context' | 'add' | 'del' | 'gap' export type NativeChatEditLine = { kind: NativeChatEditLineKind @@ -51,6 +53,19 @@ export function splitEditContent(content: string): EditContentLines { return { lines, truncated } } +/** The break between two regions of a file. Carries no text and no position. */ +export function editGapLine(): NativeChatEditLine { + return { kind: 'gap', text: '', oldLineNumber: null, newLineNumber: null } +} + +/** Appends a gap when rows already exist, so the break never opens a diff or + * doubles up behind an empty region. */ +export function pushEditGap(lines: NativeChatEditLine[]): void { + if (lines.length > 0 && lines.at(-1)?.kind !== 'gap') { + lines.push(editGapLine()) + } +} + /** Unified line numbering: a removed row is located on the old side, everything * else on the new side. One column, so a replaced line repeats its number. */ export function unifiedLineNumber(line: NativeChatEditLine): number | null { @@ -66,12 +81,19 @@ export function finalizeEditFile( const overLineCap = input.lines.length > MAX_EDIT_LINES const truncated = overLineCap || input.truncated === true const capped = overLineCap ? input.lines.slice(0, MAX_EDIT_LINES) : input.lines + // A gap marks a break between regions, so one at the end marks nothing. The + // row cap can leave one behind even when the source did not. + let end = capped.length + while (end > 0 && capped[end - 1]?.kind === 'gap') { + end -= 1 + } + const trimmed = end === capped.length ? capped : capped.slice(0, end) // Without resolved ranges the numbers locate a row inside a snippet; dropping // them keeps a plausible-looking wrong position out of the gutter, the copy // text, and the row keys. const lines = input.lineNumbersKnown - ? capped - : capped.map((line) => ({ ...line, oldLineNumber: null, newLineNumber: null })) + ? trimmed + : trimmed.map((line) => ({ ...line, oldLineNumber: null, newLineNumber: null })) let added = 0 let removed = 0 for (const line of lines) { diff --git a/src/shared/native-chat-edit-normalize.test.ts b/src/shared/native-chat-edit-normalize.test.ts index dad791ea091..0fd64b1ab02 100644 --- a/src/shared/native-chat-edit-normalize.test.ts +++ b/src/shared/native-chat-edit-normalize.test.ts @@ -69,15 +69,38 @@ describe('editLinesFromUnifiedPatch', () => { const body = `@@ -1,1 +1,1 @@\n${'+x\n'.repeat(MAX_EDIT_CHARS)}` expect(editLinesFromUnifiedPatch(body)?.truncated).toBe(true) }) + + it('marks the break between hunks, and only between them', () => { + const parsed = editLinesFromUnifiedPatch( + '@@ -40,2 +40,2 @@\n keep\n-was\n@@ -310,2 +310,2 @@\n+now\n tail' + ) + expect(parsed?.lines.map((line) => [line.kind, unifiedLineNumber(line)])).toEqual([ + ['context', 40], + ['del', 41], + ['gap', null], + ['add', 310], + ['context', 311] + ]) + }) + + it('reads a body that opens with no hunk header as an unlocatable hunk', () => { + const parsed = editLinesFromUnifiedPatch('-was\n+now', { implicitFirstHunk: true }) + expect(parsed?.lines.map((line) => line.kind)).toEqual(['del', 'add']) + expect(parsed?.lineNumbersKnown).toBe(false) + // Without the option the same body is not a patch at all. + expect(editLinesFromUnifiedPatch('-was\n+now')).toBeNull() + }) }) describe('unwrapBeginPatch', () => { - it('recovers an envelope escaped inside a JavaScript string literal', () => { - const source = - 'const patch = "*** Begin Patch\\n*** Update File: a.ts\\n@@\\n-x\\n+y\\n*** End Patch"' - expect(unwrapBeginPatch(source)).toBe( - '*** Begin Patch\n*** Update File: a.ts\n@@\n-x\n+y\n*** End Patch' - ) + it('recovers an envelope carried in one word of an argument vector', () => { + const envelope = '*** Begin Patch\n*** Update File: a.ts\n@@\n-x\n+y\n*** End Patch' + expect( + unwrapBeginPatch({ + command: ['bash', '-lc', `apply_patch <<'EOF'\n${envelope}\nEOF`], + workdir: '/repo' + }) + ).toBe(envelope) }) it('leaves an already-decoded envelope alone', () => { @@ -97,11 +120,16 @@ describe('unwrapBeginPatch', () => { }) describe('editFilesFromToolPair', () => { - it('renders a Codex exec apply_patch, which previously produced no diff', () => { + it('renders an apply_patch run through a command tool, which produced no diff', () => { const files = editFilesFromToolPair({ name: 'exec', - input: - 'const patch = "*** Begin Patch\\n*** Update File: src/a.ts\\n@@\\n ctx\\n-was\\n+now\\n*** End Patch"' + input: { + command: [ + 'bash', + '-lc', + '*** Begin Patch\n*** Update File: src/a.ts\n@@\n ctx\n-was\n+now\n*** End Patch' + ] + } }) expect(files).toHaveLength(1) expect(files?.[0]?.path).toBe('src/a.ts') @@ -122,6 +150,75 @@ describe('editFilesFromToolPair', () => { expect(gutter(files)).toEqual([1, 2]) }) + it('keeps a file whose update body carries no hunk header', () => { + const files = editFilesFromToolPair({ + name: 'apply_patch', + input: { + input: + '*** Begin Patch\n*** Update File: first.ts\n ctx\n-was\n+now\n*** Update File: second.ts\n@@ -1,1 +1,1 @@\n-a\n+b\n*** End Patch' + } + }) + expect(files?.map((file) => file.path)).toEqual(['first.ts', 'second.ts']) + expect(files?.[0]?.lines.map((line) => line.kind)).toEqual(['context', 'del', 'add']) + expect(files?.[0]?.lineNumbersKnown).toBe(false) + }) + + it('does not render envelope control lines as file content', () => { + const files = editFilesFromToolPair({ + name: 'apply_patch', + input: { + input: + '*** Begin Patch\n*** Environment ID: abc123\n*** Update File: a.ts\n@@\n-was\n+now\n*** End of File\n*** End Patch' + } + }) + expect(files?.[0]?.lines.map((line) => line.text)).toEqual(['was', 'now']) + }) + + it('reports a delete that names the file and carries no body', () => { + const files = editFilesFromToolPair({ + name: 'apply_patch', + input: { input: '*** Begin Patch\n*** Delete File: gone.ts\n*** End Patch' } + }) + expect(files).toHaveLength(1) + expect(files?.[0]?.changeKind).toBe('deleted') + expect(files?.[0]?.path).toBe('gone.ts') + expect(files?.[0]?.lines).toEqual([]) + }) + + it('reads a CRLF envelope, whose markers otherwise match nothing', () => { + const files = editFilesFromToolPair({ + name: 'apply_patch', + input: { + input: + '*** Begin Patch\r\n*** Update File: a.ts\r\n@@ -1,2 +1,2 @@\r\n-was\r\n+now\r\n*** End Patch' + } + }) + expect(files).toHaveLength(1) + expect(files?.[0]?.path).toBe('a.ts') + expect(files?.[0]?.lines.map((line) => line.text)).toEqual(['was', 'now']) + }) + + it('marks the break between resolved hunks that sit far apart', () => { + const files = editFilesFromToolPair({ + name: 'Edit', + input: { file_path: '/repo/a.ts' }, + result: { + editPatch: { + filePath: '/repo/a.ts', + hunks: [ + { oldStart: 42, oldLines: 1, newStart: 42, newLines: 1, lines: ['-was', '+now'] }, + { oldStart: 310, oldLines: 1, newStart: 310, newLines: 1, lines: ['-old', '+new'] } + ] + } + } + }) + expect(files?.[0]?.lines.map((line) => line.kind)).toEqual(['del', 'add', 'gap', 'del', 'add']) + // A break marks nothing at either end, and counts no change of its own. + expect(files?.[0]?.added).toBe(2) + expect(files?.[0]?.removed).toBe(2) + expect(gutter(files)).toEqual([42, 42, null, 310, 310]) + }) + it('reads a move header as a rename', () => { const files = editFilesFromToolPair({ name: 'exec', @@ -207,7 +304,14 @@ describe('editFilesFromToolPair', () => { }) expect(files).toHaveLength(1) expect(files?.[0]?.path).toBe('/repo/a.ts') - expect(files?.[0]?.lines.map((line) => line.text)).toEqual(['was', 'now', 'gone', 'kept']) + // Each entry is its own region, so a break separates them. + expect(files?.[0]?.lines.map((line) => [line.kind, line.text])).toEqual([ + ['del', 'was'], + ['add', 'now'], + ['gap', ''], + ['del', 'gone'], + ['add', 'kept'] + ]) expect(files?.[0]?.added).toBe(2) expect(files?.[0]?.removed).toBe(2) }) diff --git a/src/shared/native-chat-edit-normalize.ts b/src/shared/native-chat-edit-normalize.ts index 78e46d6b4ef..6e2f8b610c0 100644 --- a/src/shared/native-chat-edit-normalize.ts +++ b/src/shared/native-chat-edit-normalize.ts @@ -2,6 +2,7 @@ import { editFilesFromBeginPatch, unwrapBeginPatch } from './native-chat-begin-p import { editLinesFromContents } from './native-chat-edit-lcs' import { finalizeEditFile, + pushEditGap, type NativeChatEditFile, type NativeChatEditLine } from './native-chat-edit-model' @@ -37,6 +38,9 @@ function text(value: unknown): string | null { function linesFromEditPatch(patch: NativeChatEditPatch): NativeChatEditLine[] { const lines: NativeChatEditLine[] = [] for (const hunk of patch.hunks) { + // Hunks are separate regions of the file; run together the gutter jumps + // from one to the next with nothing marking the skipped span. + pushEditGap(lines) let oldNo = hunk.oldStart let newNo = hunk.newStart for (const raw of hunk.lines) { @@ -89,6 +93,8 @@ function multiEditFiles(input: Record, path: string): NativeCha if (oldString === null && newString === null) { continue } + // Each entry is its own snippet, so it starts a new region. + pushEditGap(lines) const diffed = editLinesFromContents(oldString ?? '', newString ?? '') lines.push(...diffed.lines) truncated ||= diffed.truncated diff --git a/src/shared/native-chat-unified-patch.ts b/src/shared/native-chat-unified-patch.ts index 3c3ec325ef0..b2eb6df73c6 100644 --- a/src/shared/native-chat-unified-patch.ts +++ b/src/shared/native-chat-unified-patch.ts @@ -1,5 +1,5 @@ import { isFileHeaderPair } from './native-chat-diff' -import { splitEditContent, type NativeChatEditLine } from './native-chat-edit-model' +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. @@ -18,31 +18,34 @@ export type UnifiedPatchLines = { } /** Parses unified patch text, keeping the `@@` ranges as per-row line numbers. - * Codex writes hunk headers whose `@@` is a bare context anchor with no - * ranges; those rows are emitted without numbers rather than numbered from 1, - * because a wrong number reads as authoritative. */ -export function editLinesFromUnifiedPatch(text: string): UnifiedPatchLines | null { + * A hunk header whose `@@` is a bare context anchor with no ranges leaves its + * rows unnumbered rather than numbered from 1, because a wrong number reads as + * authoritative. + * + * `implicitFirstHunk` opens the body as a hunk of unknown position, for the + * patch dialect whose first chunk may carry no header at all. */ +export function editLinesFromUnifiedPatch( + text: string, + options?: { implicitFirstHunk?: boolean } +): UnifiedPatchLines | null { const source = splitEditContent(text) const rows = source.lines const lines: NativeChatEditLine[] = [] let oldNo: number | null = null let newNo: number | null = null - let sawHunk = false + let sawHunk = options?.implicitFirstHunk === true let ranged = true - let inHunk = false + let inHunk = sawHunk for (let index = 0; index < rows.length; index += 1) { const raw = rows[index] ?? '' if (raw.startsWith('@@')) { const match = HUNK_RANGES.exec(raw) - if (match) { - oldNo = Number(match[1]) - newNo = Number(match[3]) - } else { - oldNo = null - newNo = null - ranged = false - } + oldNo = match ? Number(match[1]) : null + newNo = match ? Number(match[3]) : null + // Successive hunks are separate regions of the file; concatenated with no + // break the gutter jumps and the reader sees one continuous block. + pushEditGap(lines) sawHunk = true inHunk = true continue @@ -63,6 +66,9 @@ export function editLinesFromUnifiedPatch(text: string): UnifiedPatchLines | nul if (!inHunk) { continue } + // Read off the rows rather than the header, so a body that opened with no + // header is reported as unlocatable just like a rangeless `@@`. + ranged &&= oldNo !== null || newNo !== null if (raw.startsWith('+')) { lines.push({ kind: 'add',