diff --git a/src/main/native-chat/transcript-line-decoders-claude.ts b/src/main/native-chat/transcript-line-decoders-claude.ts index f819656bdb2..5202035bbc6 100644 --- a/src/main/native-chat/transcript-line-decoders-claude.ts +++ b/src/main/native-chat/transcript-line-decoders-claude.ts @@ -3,6 +3,8 @@ import { NATIVE_CHAT_INTERRUPTED_STATUS_TEXT, type NativeChatBlock, + type NativeChatEditPatch, + type NativeChatEditPatchHunk, type NativeChatMessage } from '../../shared/native-chat-types' import { @@ -15,6 +17,59 @@ import { imageSourcePathFromText } from '../../shared/native-chat-image-transcri import { claudeContentBlocks } from './transcript-record-blocks' import { claudeInterruptedMessageId } from './transcript-turn-markers' +const MAX_EDIT_PATCH_HUNKS = 40 +const MAX_EDIT_PATCH_HUNK_LINES = 400 + +/** Claude reports an edit as a snippet pair on the call, which cannot locate the + * change in the file. The result record carries the hunks it resolved against + * the real file, so keep them for the renderer's line-number gutter. */ +function claudeEditPatch(record: Record): NativeChatEditPatch | null { + const result = asRecord(record.toolUseResult) + const raw = result?.structuredPatch + if (!Array.isArray(raw) || raw.length === 0) { + return null + } + const hunks: NativeChatEditPatchHunk[] = [] + for (const entry of raw.slice(0, MAX_EDIT_PATCH_HUNKS)) { + const hunk = asRecord(entry) + const lines = hunk?.lines + if ( + typeof hunk?.oldStart !== 'number' || + typeof hunk.newStart !== 'number' || + !Array.isArray(lines) + ) { + continue + } + hunks.push({ + oldStart: hunk.oldStart, + oldLines: typeof hunk.oldLines === 'number' ? hunk.oldLines : 0, + newStart: hunk.newStart, + newLines: typeof hunk.newLines === 'number' ? hunk.newLines : 0, + lines: lines + .slice(0, MAX_EDIT_PATCH_HUNK_LINES) + .flatMap((line) => (typeof line === 'string' ? [line] : [])) + }) + } + if (hunks.length === 0) { + return null + } + const filePath = extractString(result?.filePath) + return { ...(filePath ? { filePath } : {}), hunks } +} + +/** Attaches the resolved hunks to the record's tool result, which is the only + * block in a Claude result turn. */ +function withEditPatch(blocks: NativeChatBlock[], patch: NativeChatEditPatch): NativeChatBlock[] { + let attached = false + return blocks.map((block) => { + if (attached || block.type !== 'tool-result') { + return block + } + attached = true + return { ...block, editPatch: patch } + }) +} + export function decodeClaudeTranscriptLine( line: string, fallbackId: string @@ -41,7 +96,9 @@ export function decodeClaudeTranscriptLine( } } const message = asRecord(record.message) - const decodedBlocks = claudeContentBlocks(message?.content) + const editPatch = claudeEditPatch(record) + const contentBlocks = claudeContentBlocks(message?.content) + const decodedBlocks = editPatch ? withEditPatch(contentBlocks, editPatch) : contentBlocks if (decodedBlocks.length === 0) { return null } diff --git a/src/main/native-chat/transcript-line-decoders-codex.ts b/src/main/native-chat/transcript-line-decoders-codex.ts index c4b0ddbf5ba..d70bd7a7c80 100644 --- a/src/main/native-chat/transcript-line-decoders-codex.ts +++ b/src/main/native-chat/transcript-line-decoders-codex.ts @@ -202,6 +202,10 @@ function codexTurnItemBlocks(content: unknown): NativeChatBlock[] { return blocks } +/** The argument payload is passed through exactly as it arrived. Decoding it + * here would change the shape every `.input` consumer sees — including the ask + * surface, which reads a question shape out of any tool's input — so the one + * consumer that needs structure decodes it for itself. */ function codexCallInput(payload: Record): unknown { if (payload.arguments !== undefined) { return payload.arguments 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..5d18bec8c85 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 @@ -2,6 +2,7 @@ import { mkdtemp, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' +import { extractPendingAsk } from '../../shared/native-chat-ask' import { decodeCodexTranscriptLine } from './transcript-line-decoders-codex' import { readNativeChatTranscript } from './transcript-reader' import { readNativeChatTranscriptTail } from './transcript-tail-reader' @@ -247,4 +248,43 @@ describe('Codex transcript history modes', () => { blocks: [{ type: 'tool-result', output: 'ok' }] }) }) + + it('passes an argument payload through untouched, so no consumer changes shape', () => { + const call = decodeCodexTranscriptLine( + JSON.stringify({ + type: 'response_item', + payload: { + type: 'function_call', + id: 'call-2', + name: 'shell', + arguments: '{"command":["bash","-lc","echo hi"]}' + } + }), + 'fallback-args' + ) + + expect(call?.blocks[0]).toMatchObject({ + type: 'tool-call', + name: 'shell', + input: '{"command":["bash","-lc","echo hi"]}' + }) + }) + + it('does not raise a question card from an unrelated tool that carries a questions payload', () => { + const call = decodeCodexTranscriptLine( + JSON.stringify({ + type: 'response_item', + payload: { + type: 'function_call', + id: 'call-3', + name: 'some_mcp_tool', + arguments: '{"questions":[{"question":"Which branch?","options":["main","dev"]}]}' + } + }), + 'fallback-questions' + ) + + expect(call).not.toBeNull() + expect(extractPendingAsk(call ? [call] : [])).toBeNull() + }) }) diff --git a/src/renderer/src/assets/main.css b/src/renderer/src/assets/main.css index e3d267cb353..3527f11c44e 100644 --- a/src/renderer/src/assets/main.css +++ b/src/renderer/src/assets/main.css @@ -229,6 +229,10 @@ --git-decoration-untracked: #007100; --git-decoration-copied: #007acc; --git-decoration-ignored: #8c8c8c; + --diff-added-ground: color-mix(in srgb, var(--git-decoration-added) 13%, transparent); + --diff-added-gutter: color-mix(in srgb, var(--git-decoration-added) 26%, transparent); + --diff-removed-ground: color-mix(in srgb, var(--git-decoration-deleted) 11%, transparent); + --diff-removed-gutter: color-mix(in srgb, var(--git-decoration-deleted) 22%, transparent); --git-graph-ref: #007acc; --git-graph-remote-ref: #b66dff; --git-graph-base-ref: #ea5c00; @@ -341,6 +345,10 @@ --git-decoration-untracked: #73c991; --git-decoration-copied: #73c991; --git-decoration-ignored: #6e6e6e; + --diff-added-ground: color-mix(in srgb, var(--git-decoration-added) 16%, transparent); + --diff-added-gutter: color-mix(in srgb, var(--git-decoration-added) 30%, transparent); + --diff-removed-ground: color-mix(in srgb, var(--git-decoration-deleted) 18%, transparent); + --diff-removed-gutter: color-mix(in srgb, var(--git-decoration-deleted) 32%, transparent); --git-graph-ref: #3794ff; --git-graph-remote-ref: #b66dff; --git-graph-base-ref: #ea5c00; diff --git a/src/renderer/src/components/native-chat/NativeChatCopyButton.tsx b/src/renderer/src/components/native-chat/NativeChatCopyButton.tsx index 36fa791f57d..0e933584d41 100644 --- a/src/renderer/src/components/native-chat/NativeChatCopyButton.tsx +++ b/src/renderer/src/components/native-chat/NativeChatCopyButton.tsx @@ -12,9 +12,12 @@ import { translate } from '@/i18n/i18n' */ export function NativeChatCopyButton({ text, + label: copyLabel, className }: { text: string + /** What this button copies, when it is not the whole message. */ + label?: string className?: string }): React.JSX.Element { const [copied, setCopied] = useState(false) @@ -46,7 +49,7 @@ export function NativeChatCopyButton({ const label = copied ? translate('components.native-chat.copyMessage.copied', 'Copied') - : translate('components.native-chat.copyMessage.copy', 'Copy message') + : (copyLabel ?? translate('components.native-chat.copyMessage.copy', 'Copy message')) return ( +
+ {file.oldPath ? ( + <> + + {baseName(file.oldPath)} + + → + + ) : null} + + {baseName(file.path)} + + + {file.truncated ? ( + // Beside the counts rather than under the rows: a collapsed card, and + // one clipped down to no rows at all, would otherwise say nothing. + + {translate('components.native-chat.tool.diffTruncated', 'Diff truncated')} + + ) : null} + +
+ {hasBody && expanded ? ( + // Focusable so the rows can be scrolled from the keyboard. +
+ {(() => { + const seen = new Map() + return file.lines.map((line) => { + const signature = `${line.kind}:${line.oldLineNumber}:${line.newLineNumber}:${line.text}` + const occurrence = seen.get(signature) ?? 0 + seen.set(signature, occurrence + 1) + return ( + + ) + }) + })()} +
+ ) : null} + + ) +} diff --git a/src/renderer/src/components/native-chat/NativeChatToolRun.test.tsx b/src/renderer/src/components/native-chat/NativeChatToolRun.test.tsx index 965ed0ad176..050b3f7c84b 100644 --- a/src/renderer/src/components/native-chat/NativeChatToolRun.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatToolRun.test.tsx @@ -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' @@ -32,6 +32,9 @@ describe('NativeChatToolRun', () => { { type: 'tool-call', name: 'apply_patch', + // The patch lives on the call in this lane, so the provider's own + // completion is what says the edit landed. + state: 'completed', input: { changes: [ { @@ -46,8 +49,9 @@ describe('NativeChatToolRun', () => { const { container } = render() - expect(screen.getByText('+after')).toBeInTheDocument() - expect(screen.getByText('-before')).toBeInTheDocument() + expect(screen.getByText('after')).toBeInTheDocument() + expect(screen.getByText('before')).toBeInTheDocument() + expect(screen.getByText('Edited file')).toBeInTheDocument() expect(container.querySelector('pre')).toBeNull() }) @@ -78,18 +82,156 @@ describe('NativeChatToolRun', () => { ) - expect(screen.getByText('+after')).toHaveClass( - 'bg-emerald-500/10', - 'text-[var(--git-decoration-added)]' - ) - expect(screen.getByText('-before')).toHaveClass( - 'bg-rose-500/10', - 'text-[var(--git-decoration-deleted)]' - ) + // Row grounds come from the diff tokens, not a hardcoded palette value. + expect(screen.getByText('after').closest('div')).toHaveClass('bg-[var(--diff-added-ground)]') + expect(screen.getByText('before').closest('div')).toHaveClass('bg-[var(--diff-removed-ground)]') expect(container).not.toHaveTextContent('"changes"') expect(container.querySelector('pre')).toBeNull() }) + it('keeps the provider error visible for an edit the agent could not apply', () => { + const blocks: NativeChatBlock[] = [ + { + type: 'tool-call', + name: 'Edit', + input: { file_path: '/repo/a.ts', old_string: 'missing', new_string: 'now' } + }, + { type: 'tool-result', output: 'String to replace not found in file.', isError: true } + ] + + const { container } = render() + + expect(screen.queryByText('Edited file')).toBeNull() + const body = container.querySelector('pre') + expect(body).toHaveTextContent('String to replace not found in file.') + expect(body).toHaveClass('text-destructive') + }) + + it('leaves a `git diff` command as a command row rather than an edit card', () => { + const blocks: NativeChatBlock[] = [ + { type: 'tool-call', name: 'exec', input: { command: 'git diff' }, state: 'completed' }, + { + type: 'tool-result', + output: 'diff --git a/a.ts b/a.ts\n--- a/a.ts\n+++ b/a.ts\n@@ -1 +1 @@\n-was\n+now' + } + ] + + const { container } = render() + + expect(screen.queryByText('Edited file')).toBeNull() + expect(container).toHaveTextContent('git diff') + }) + + it('shows no gutter number for a snippet edit, which cannot locate itself', () => { + const blocks: NativeChatBlock[] = [ + { + type: 'tool-call', + name: 'Edit', + input: { file_path: '/repo/a.ts', old_string: 'was', new_string: 'now' }, + state: 'completed' + }, + { type: 'tool-result', output: 'ok' } + ] + + render() + + // Exact, because a snippet-relative number would sit ahead of the marker. + expect(screen.getByText('now').closest('div')?.textContent).toBe('+now') + 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('says a diff was clipped even while the card is collapsed', () => { + const blocks: NativeChatBlock[] = [ + { + type: 'tool-call', + name: 'Diff', + input: { path: 'src/a.ts' }, + state: 'completed' + }, + { type: 'tool-result', output: '@@ -1,3 +1,3 @@\n ctx\n-was\n+now\n… (48210 bytes)' } + ] + + // A defined expandOverride opens the run while leaving each card closed. + render() + + 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() + 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' }, diff --git a/src/renderer/src/components/native-chat/NativeChatToolRun.tsx b/src/renderer/src/components/native-chat/NativeChatToolRun.tsx index 14cb0163f50..e91177b375a 100644 --- a/src/renderer/src/components/native-chat/NativeChatToolRun.tsx +++ b/src/renderer/src/components/native-chat/NativeChatToolRun.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from 'react' +import { useEffect, useMemo, useState } from 'react' import { Check, ChevronRight, SquareTerminal, Wrench } from 'lucide-react' import { cn } from '@/lib/utils' import { translate } from '@/i18n/i18n' @@ -8,6 +8,13 @@ import { type NativeChatBlock } from '../../../../shared/native-chat-types' import { diffFromText, diffFromToolCall, type DiffLine } from './native-chat-diff' +import { NativeChatDiffCard } from './NativeChatDiffCard' +import { pairToolBlocks } from './native-chat-tool-fold' +import { + editFilesFromToolPair, + isEditToolName +} from '../../../../shared/native-chat-edit-normalize' +import type { NativeChatEditFile } from '../../../../shared/native-chat-edit-model' import { countToolCalls, createToolInputDisplay, @@ -128,6 +135,50 @@ function ToolLine({ ) } +type EditCardModel = { + editCards: Map + /** Result blocks the card already speaks for, so they render no second row. */ + consumedResults: Set +} + +const NO_EDIT_CARDS: EditCardModel = { editCards: new Map(), consumedResults: new Set() } + +/** An edit renders as one card, so its result block is folded into the call. The + * model decides which calls have landed; a call that has not keeps the generic + * tool view, its result still visible as the provider's own error. */ +function buildEditCards(blocks: NativeChatBlock[]): EditCardModel { + const editCards: EditCardModel['editCards'] = new Map() + const consumedResults: EditCardModel['consumedResults'] = new Set() + for (const [index, pair] of pairToolBlocks(blocks).entries()) { + const call = pair.call + if (!call || !isEditToolName(call.name)) { + continue + } + const files = editFilesFromToolPair({ + name: call.name, + input: call.input, + ...(call.state ? { state: call.state } : {}), + ...(pair.result + ? { + result: { + output: pair.result.output, + isError: pair.result.isError, + editPatch: pair.result.editPatch + } + } + : {}) + }) + if (!files || files.length === 0) { + continue + } + editCards.set(call, { files, key: `${call.name}:${index}` }) + if (pair.result) { + consumedResults.add(pair.result) + } + } + return { editCards, consumedResults } +} + /** A run of a message's tool calls/results, collapsed to a one-line summary that * expands to the individual inline tool lines. `expandSignal` lets the global * toolbar toggle drive every run at once while still allowing per-run override. */ @@ -160,6 +211,12 @@ export function NativeChatToolRun({ // The turn caret opens the activity group, while each child tool remains // collapsed. The global expand toolbar still opens child details together. const expandToolLines = expandOverride === undefined ? open : false + // Diffing every edit is the run's most expensive work, so a collapsed run — + // which renders none of it — never pays for it. + const { editCards, consumedResults } = useMemo( + () => (open ? buildEditCards(blocks) : NO_EDIT_CARDS), + [open, blocks] + ) const ActiveToolIcon = latestActiveCall && isCommandToolName(latestActiveCall.name) ? SquareTerminal : Wrench const fallbackLabel = @@ -233,6 +290,23 @@ export function NativeChatToolRun({ {(() => { const seen = new Map() return blocks.map((block) => { + const edit = editCards.get(block) + if (edit) { + return ( +
+ {edit.files.map((file, fileIndex) => ( + + ))} +
+ ) + } + if (consumedResults.has(block)) { + return null + } const signature = block.type === 'tool-call' ? `${block.type}:${block.name}:${JSON.stringify(block.input)}` diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 95de4fab458..243771fee8f 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -16958,7 +16958,14 @@ "ranCommandsOneToolSummary": "Ran {{commandCount}} commands and used {{toolCount}} tool", "ranCommandsManyToolsSummary": "Ran {{commandCount}} commands and used {{toolCount}} tools", "usedOneSummary": "Used 1 tool", - "usedManySummary": "Used {{toolCount}} tools" + "usedManySummary": "Used {{toolCount}} tools", + "editedFile": "Edited file", + "addedFile": "Added file", + "deletedFile": "Deleted file", + "renamedFile": "Renamed file", + "copyDiff": "Copy diff", + "diffGap": "Lines not shown", + "diffTruncated": "Diff truncated" }, "providerFrame": { "byteLength": "{{value0}} bytes" diff --git a/src/shared/native-chat-begin-patch.ts b/src/shared/native-chat-begin-patch.ts new file mode 100644 index 00000000000..becd0ece46a --- /dev/null +++ b/src/shared/native-chat-begin-patch.ts @@ -0,0 +1,173 @@ +import { finalizeEditFile, type NativeChatEditFile } from './native-chat-edit-model' +import { editLinesFromUnifiedPatch, editLinesFromWholeFile } from './native-chat-unified-patch' + +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:)/ + +/** The envelope reaches a command tool as one of its patch or command + * arguments, either whole or as one word of the argument vector it runs. + * Recover its text. Callers must gate this on the tool being one that runs a + * patch: a file's own contents may quote an envelope. + * + * `requireApplyCommand` is for a general command tool, where the envelope + * proves nothing on its own — a command writing documentation quotes one + * without applying it, and the command must say it is applying it. */ +export function unwrapBeginPatch( + input: unknown, + options?: { requireApplyCommand?: boolean } +): string | null { + const source = envelopeSource(input, options?.requireApplyCommand === true) + if (!source) { + return null + } + const start = source.indexOf(BEGIN) + if (start === -1) { + return null + } + const end = source.indexOf(END, start) + if (end === -1) { + // Without the closing marker there is nothing separating the patch body from + // whatever the command line continues with, and trailing shell syntax would + // render as file content the agent never wrote. + return null + } + return source.slice(start, end + END.length) +} + +/** The arguments that carry a patch or the command line that applies one. Only + * these are searched: any other value is data the tool operates on, and a file + * whose own contents quote an envelope would otherwise be read as a patch + * against some other file entirely. */ +const ENVELOPE_ARGUMENTS = ['input', 'command', 'patch', 'arguments', 'script'] as const +/** What a command tool runs to apply an envelope, as opposed to quoting one. + * Both spellings the runner accepts, since either one really applies it. */ +const APPLY_COMMAND = /apply_?patch/ + +/** The call payload may itself be a string holding JSON. Decoding it here, in + * the one consumer that needs its structure, keeps every other reader of the + * call input seeing exactly what the provider sent. */ +function envelopeSource(input: unknown, requireApplyCommand: boolean): string | null { + if (typeof input === 'string') { + const record = jsonRecord(input) + return record + ? envelopeArgument(record, requireApplyCommand) + : applied(input, requireApplyCommand) + } + return typeof input === 'object' && input !== null + ? envelopeArgument(input as Record, requireApplyCommand) + : null +} + +function applied(value: string, requireApplyCommand: boolean): string | null { + return !requireApplyCommand || APPLY_COMMAND.test(value) ? value : null +} + +function jsonRecord(value: string): Record | null { + if (!value.trimStart().startsWith('{')) { + return null + } + try { + const parsed: unknown = JSON.parse(value) + return typeof parsed === 'object' && parsed !== null && !Array.isArray(parsed) + ? (parsed as Record) + : null + } catch { + return null + } +} + +/** A command tool's argument is a vector, so the envelope sits one level in and + * the words that apply it may be a different element than the envelope. */ +function envelopeArgument( + record: Record, + requireApplyCommand: boolean +): string | null { + for (const key of ENVELOPE_ARGUMENTS) { + const value = record[key] + const words = + typeof value === 'string' + ? [value] + : Array.isArray(value) + ? value.filter((entry): entry is string => typeof entry === 'string') + : [] + if (requireApplyCommand && !words.some((word) => APPLY_COMMAND.test(word))) { + continue + } + const word = words.find((entry) => entry.includes(BEGIN)) + if (word) { + return word + } + } + return null +} + +/** 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[] }[] = [] + const moves = new Map() + + // 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({ + kind: header[1] as 'Add' | 'Update' | 'Delete', + path: header[2]!.trim(), + body: [] + }) + continue + } + const move = MOVE_HEADER.exec(raw) + if (move && sections.length > 0) { + moves.set(sections.length - 1, move[1]!.trim()) + continue + } + if (raw === BEGIN || raw === END || CONTROL_LINE.test(raw) || sections.length === 0) { + continue + } + sections.at(-1)!.body.push(raw) + } + + return sections.flatMap((section, index) => { + const body = section.body.join('\n') + const moved = moves.get(index) ?? null + if (section.kind === 'Add' || section.kind === 'Delete') { + const sign = section.kind === 'Add' ? '+' : '-' + // Add/Delete bodies carry a sign per line but no hunk header. + const stripped = section.body + .map((line) => (line.startsWith(sign) ? line.slice(1) : line)) + .join('\n') + const whole = editLinesFromWholeFile(stripped, section.kind === 'Add' ? 'add' : 'del') + return [ + finalizeEditFile({ + path: section.path, + oldPath: null, + changeKind: section.kind === 'Add' ? 'added' : 'deleted', + lines: whole.lines, + lineNumbersKnown: true, + truncated: whole.truncated + }) + ] + } + // The first chunk of an update may carry no hunk header at all, and a + // section may carry no body either. The envelope named the file, so it is + // reported with whatever rows it has rather than dropped from a multi-file + // envelope with nothing to say it went missing. + const parsed = editLinesFromUnifiedPatch(body, { implicitFirstHunk: true }) + return [ + finalizeEditFile({ + path: moved ?? section.path, + oldPath: moved ? section.path : null, + changeKind: moved ? 'renamed' : 'edited', + lines: parsed?.lines ?? [], + lineNumbersKnown: parsed?.lineNumbersKnown ?? false, + truncated: parsed?.truncated ?? false + }) + ] + }) +} diff --git a/src/shared/native-chat-diff.ts b/src/shared/native-chat-diff.ts index 1df82db4800..a6dde625a14 100644 --- a/src/shared/native-chat-diff.ts +++ b/src/shared/native-chat-diff.ts @@ -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,})$/ @@ -28,11 +31,17 @@ type DiffStructure = { } /** - * Locates the `--- ` / `+++ ` file headers. A bare `---`/`+++` prefix - * is not enough to spot one: a removed line whose content began with `--` - * (SQL/Lua `-- comment`, C `--i`) is emitted as `---`. Real headers - * always come as an adjacent pair and never appear inside a hunk. + * True when the row at `index` opens a `--- ` / `+++ ` file header. A + * bare `---`/`+++` prefix is not enough to spot one: a removed line whose + * content began with `--` (SQL/Lua `-- comment`, C `--i`) is emitted as + * `---`. Real headers always come as an adjacent pair and never appear + * inside a hunk, so callers must check this only outside one. */ +export function isFileHeaderPair(lines: readonly string[], index: number): boolean { + return (lines[index] ?? '').startsWith('--- ') && (lines[index + 1] ?? '').startsWith('+++ ') +} + +/** Locates the file headers and rules that are structure rather than content. */ function scanDiffStructure(lines: string[]): DiffStructure { const metaIndices = new Set() let isStructuredDiff = false @@ -57,7 +66,7 @@ function scanDiffStructure(lines: string[]): DiffStructure { metaIndices.add(index) continue } - if (line.startsWith('--- ') && (lines[index + 1] ?? '').startsWith('+++ ')) { + if (isFileHeaderPair(lines, index)) { metaIndices.add(index) metaIndices.add(index + 1) index += 1 diff --git a/src/shared/native-chat-edit-lcs.ts b/src/shared/native-chat-edit-lcs.ts new file mode 100644 index 00000000000..733d5f5c700 --- /dev/null +++ b/src/shared/native-chat-edit-lcs.ts @@ -0,0 +1,105 @@ +import { + MAX_EDIT_DIFF_CELLS, + splitEditContent, + type NativeChatEditLine +} from './native-chat-edit-model' + +/** Line diff between two contents, interleaved with context. Numbers are + * positions within the given contents, so they locate rows in the file only + * when the caller passed whole files. */ +export function editLinesFromContents( + originalContent: string, + modifiedContent: string +): { lines: NativeChatEditLine[]; truncated: boolean } { + const original = splitEditContent(originalContent) + const modified = splitEditContent(modifiedContent) + const lines = + original.lines.length * modified.lines.length <= MAX_EDIT_DIFF_CELLS + ? lcsLines(original.lines, modified.lines) + : prefixSuffixLines(original.lines, modified.lines) + return { lines, truncated: original.truncated || modified.truncated } +} + +function context(text: string, oldNo: number, newNo: number): NativeChatEditLine { + return { kind: 'context', text, oldLineNumber: oldNo, newLineNumber: newNo } +} + +function removal(text: string, oldNo: number): NativeChatEditLine { + return { kind: 'del', text, oldLineNumber: oldNo, newLineNumber: null } +} + +function addition(text: string, newNo: number): NativeChatEditLine { + return { kind: 'add', text, oldLineNumber: null, newLineNumber: newNo } +} + +function lcsLines(original: string[], modified: string[]): NativeChatEditLine[] { + const width = modified.length + 1 + const dp = new Uint32Array((original.length + 1) * width) + for (let i = original.length - 1; i >= 0; i -= 1) { + for (let j = modified.length - 1; j >= 0; j -= 1) { + dp[i * width + j] = + original[i] === modified[j] + ? dp[(i + 1) * width + j + 1]! + 1 + : Math.max(dp[(i + 1) * width + j]!, dp[i * width + j + 1]!) + } + } + + const lines: NativeChatEditLine[] = [] + let oldIndex = 0 + let newIndex = 0 + while (oldIndex < original.length && newIndex < modified.length) { + if (original[oldIndex] === modified[newIndex]) { + lines.push(context(original[oldIndex] ?? '', oldIndex + 1, newIndex + 1)) + oldIndex += 1 + newIndex += 1 + } else if (dp[(oldIndex + 1) * width + newIndex]! >= dp[oldIndex * width + newIndex + 1]!) { + lines.push(removal(original[oldIndex] ?? '', oldIndex + 1)) + oldIndex += 1 + } else { + lines.push(addition(modified[newIndex] ?? '', newIndex + 1)) + newIndex += 1 + } + } + for (; oldIndex < original.length; oldIndex += 1) { + lines.push(removal(original[oldIndex] ?? '', oldIndex + 1)) + } + for (; newIndex < modified.length; newIndex += 1) { + lines.push(addition(modified[newIndex] ?? '', newIndex + 1)) + } + return lines +} + +function prefixSuffixLines(original: string[], modified: string[]): NativeChatEditLine[] { + let prefix = 0 + while ( + prefix < original.length && + prefix < modified.length && + original[prefix] === modified[prefix] + ) { + prefix += 1 + } + let suffix = 0 + while ( + suffix + prefix < original.length && + suffix + prefix < modified.length && + original[original.length - suffix - 1] === modified[modified.length - suffix - 1] + ) { + suffix += 1 + } + + const lines: NativeChatEditLine[] = [] + for (let i = 0; i < prefix; i += 1) { + lines.push(context(original[i] ?? '', i + 1, i + 1)) + } + for (let i = prefix; i < original.length - suffix; i += 1) { + lines.push(removal(original[i] ?? '', i + 1)) + } + for (let i = prefix; i < modified.length - suffix; i += 1) { + lines.push(addition(modified[i] ?? '', i + 1)) + } + for (let i = original.length - suffix; i < original.length; i += 1) { + const newIndex = modified.length - suffix + (i - (original.length - suffix)) + lines.push(context(original[i] ?? '', i + 1, newIndex + 1)) + } + return lines +} diff --git a/src/shared/native-chat-edit-model.ts b/src/shared/native-chat-edit-model.ts new file mode 100644 index 00000000000..6a26f87c599 --- /dev/null +++ b/src/shared/native-chat-edit-model.ts @@ -0,0 +1,107 @@ +/** 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. `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 + text: string + oldLineNumber: number | null + newLineNumber: number | null +} + +export type NativeChatEditChangeKind = 'added' | 'deleted' | 'edited' | 'renamed' + +export type NativeChatEditFile = { + path: string + /** Set only when the change moved the file. */ + oldPath: string | null + changeKind: NativeChatEditChangeKind + lines: NativeChatEditLine[] + added: number + removed: number + /** False when the numbers locate a row inside a snippet rather than the file, + * which is the case whenever the provider gave us no resolved hunk ranges. */ + lineNumbersKnown: boolean + truncated: boolean +} + +export const MAX_EDIT_LINES = 2_000 +export const MAX_EDIT_CHARS = 96_000 +/** The LCS table is quadratic; above this a linear prefix/suffix diff is used. */ +export const MAX_EDIT_DIFF_CELLS = 200_000 + +/** Rows of a source string, plus whether it was clipped before splitting. */ +export type EditContentLines = { lines: string[]; truncated: boolean } + +/** The one row splitter for every edit shape. Splits on both newline forms so a + * CRLF file never carries a trailing `\r` into a row, where it would render as + * a stray character, defeat the phantom-row guard, and reach the clipboard. */ +export function splitEditContent(content: string): EditContentLines { + if (content.length === 0) { + return { lines: [], truncated: false } + } + const truncated = content.length > MAX_EDIT_CHARS + const body = truncated ? content.slice(0, MAX_EDIT_CHARS) : content + const lines = body.split(/\r?\n/) + // Tested against the clipped body: on the un-clipped string this popped a + // real line whenever the slice fired. + if (body.endsWith('\n')) { + lines.pop() + } + return { lines, truncated } +} + +/** The break between two regions of a file. Carries no text and no position. */ +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 { + return line.kind === 'del' ? line.oldLineNumber : (line.newLineNumber ?? line.oldLineNumber) +} + +export function finalizeEditFile( + input: Omit & { + /** Set when the source text was clipped before it became rows. */ + truncated?: boolean + } +): NativeChatEditFile { + 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 + ? trimmed + : trimmed.map((line) => ({ ...line, oldLineNumber: null, newLineNumber: null })) + let added = 0 + let removed = 0 + for (const line of lines) { + if (line.kind === 'add') { + added += 1 + } else if (line.kind === 'del') { + removed += 1 + } + } + return { ...input, lines, added, removed, truncated } +} diff --git a/src/shared/native-chat-edit-normalize.test.ts b/src/shared/native-chat-edit-normalize.test.ts new file mode 100644 index 00000000000..f49e23be296 --- /dev/null +++ b/src/shared/native-chat-edit-normalize.test.ts @@ -0,0 +1,609 @@ +import { describe, expect, it } from 'vitest' +import { editFilesFromToolPair, isEditToolName } from './native-chat-edit-normalize' +import { MAX_EDIT_CHARS, unifiedLineNumber } from './native-chat-edit-model' +import { editLinesFromUnifiedPatch } from './native-chat-unified-patch' +import { unwrapBeginPatch } from './native-chat-begin-patch' + +const gutter = (files: ReturnType): (number | null)[] => + (files ?? []).flatMap((file) => file.lines.map((line) => unifiedLineNumber(line))) + +/** A card takes evidence the edit landed, so these cases report the call as + * complete. Cases about the lifecycle itself pass their own state. */ +const settledFiles = ( + pair: Parameters[0] +): ReturnType => + editFilesFromToolPair({ state: 'completed', ...pair }) + +describe('editLinesFromUnifiedPatch', () => { + it('numbers deletes from the old side and adds from the new side', () => { + const parsed = editLinesFromUnifiedPatch('@@ -12,3 +12,3 @@\n ctx\n-was\n+now\n tail') + expect(parsed?.lineNumbersKnown).toBe(true) + expect(parsed?.lines.map((line) => [line.kind, unifiedLineNumber(line)])).toEqual([ + ['context', 12], + ['del', 13], + ['add', 13], + ['context', 14] + ]) + }) + + it('leaves rows unnumbered when the hunk header carries no ranges', () => { + const parsed = editLinesFromUnifiedPatch('@@\n ctx\n-was\n+now') + expect(parsed?.lineNumbersKnown).toBe(false) + expect(parsed?.lines.every((line) => unifiedLineNumber(line) === null)).toBe(true) + }) + + it('returns null for text with no hunk header', () => { + expect(editLinesFromUnifiedPatch('just prose\n- a bullet')).toBeNull() + }) + + it('keeps the hunk open across a mid-hunk no-newline marker', () => { + const parsed = editLinesFromUnifiedPatch( + '@@ -1,2 +1,2 @@\n keep\n-old\n\\ No newline at end of file\n+new\n\\ No newline at end of file' + ) + expect(parsed?.lines.map((line) => [line.kind, line.text])).toEqual([ + ['context', 'keep'], + ['del', 'old'], + ['add', 'new'] + ]) + }) + + it('reads a removed line that starts with `--` as content, not a file header', () => { + const parsed = editLinesFromUnifiedPatch('@@ -1,4 +1,3 @@\n keep\n--- comment\n-gone\n tail') + expect(parsed?.lines.map((line) => [line.kind, line.text])).toEqual([ + ['context', 'keep'], + ['del', '-- comment'], + ['del', 'gone'], + ['context', 'tail'] + ]) + }) + + it('skips a real file header pair, which only appears outside a hunk', () => { + const parsed = editLinesFromUnifiedPatch( + 'diff --git a/a.ts b/a.ts\n--- a/a.ts\n+++ b/a.ts\n@@ -1,1 +1,1 @@\n-was\n+now' + ) + expect(parsed?.lines.map((line) => [line.kind, line.text])).toEqual([ + ['del', 'was'], + ['add', 'now'] + ]) + }) + + it('splits CRLF rows without leaving a carriage return or a phantom row', () => { + const parsed = editLinesFromUnifiedPatch('@@ -1,2 +1,2 @@\r\n ctx\r\n-was\r\n+now\r\n') + expect(parsed?.lines.map((line) => line.text)).toEqual(['ctx', 'was', 'now']) + }) + + it('reports truncation when the patch text runs past the character cap', () => { + 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 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', () => { + const plain = '*** Begin Patch\n*** Update File: a.ts\n@@\n-x\n+y\n*** End Patch' + expect(unwrapBeginPatch(plain)).toBe(plain) + }) + + it('ignores input with no envelope', () => { + expect(unwrapBeginPatch('ls -la')).toBeNull() + }) + + it('declines an envelope with no closing marker rather than swallowing the command line', () => { + const command = 'bash -c "*** Begin Patch\n*** Update File: a.ts\n@@\n-x\n+y" && echo ok' + expect(unwrapBeginPatch(command)).toBeNull() + expect(settledFiles({ name: 'shell', input: command })).toBeNull() + }) +}) + +describe('editFilesFromToolPair', () => { + it('renders an apply_patch run through a command tool, which produced no diff', () => { + const files = settledFiles({ + name: 'exec', + input: { + command: [ + 'bash', + '-lc', + "apply_patch <<'EOF'\n*** Begin Patch\n*** Update File: src/a.ts\n@@\n ctx\n-was\n+now\n*** End Patch\nEOF" + ] + } + }) + expect(files).toHaveLength(1) + expect(files?.[0]?.path).toBe('src/a.ts') + expect(files?.[0]?.changeKind).toBe('edited') + expect(files?.[0]?.added).toBe(1) + expect(files?.[0]?.removed).toBe(1) + // Codex hunk headers are context anchors, so no row may claim a file position. + expect(files?.[0]?.lineNumbersKnown).toBe(false) + }) + + it('numbers an added file from 1', () => { + const files = settledFiles({ + name: 'exec', + input: + "apply_patch <<'EOF'\n*** Begin Patch\n*** Add File: new.ts\n+one\n+two\n*** End Patch\nEOF" + }) + expect(files?.[0]?.changeKind).toBe('added') + expect(files?.[0]?.lineNumbersKnown).toBe(true) + expect(gutter(files)).toEqual([1, 2]) + }) + + it('keeps a file whose update body carries no hunk header', () => { + const files = settledFiles({ + 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 = settledFiles({ + 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 = settledFiles({ + 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 = settledFiles({ + 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 = settledFiles({ + 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 = settledFiles({ + name: 'exec', + input: + "apply_patch <<'EOF'\n*** Begin Patch\n*** Update File: old.ts\n*** Move to: new.ts\n@@\n-a\n+b\n*** End Patch\nEOF" + }) + expect(files?.[0]?.changeKind).toBe('renamed') + expect(files?.[0]?.oldPath).toBe('old.ts') + expect(files?.[0]?.path).toBe('new.ts') + }) + + it('interleaves a Claude snippet pair without claiming line positions', () => { + const files = settledFiles({ + name: 'Edit', + input: { + file_path: '/repo/a.ts', + old_string: 'keep\nwas\ntail', + new_string: 'keep\nnow\ntail' + } + }) + expect(files?.[0]?.lines.map((line) => line.kind)).toEqual(['context', 'del', 'add', 'context']) + expect(files?.[0]?.lineNumbersKnown).toBe(false) + }) + + it('prefers the resolved hunks on the result over the snippet pair', () => { + const files = settledFiles({ + name: 'Edit', + input: { file_path: '/repo/a.ts', old_string: 'was', new_string: 'now' }, + result: { + editPatch: { + filePath: '/repo/a.ts', + hunks: [ + { + oldStart: 12, + oldLines: 3, + newStart: 12, + newLines: 3, + lines: [' ctx', '-was', '+now', ' tail'] + } + ] + } + } + }) + expect(files?.[0]?.lineNumbersKnown).toBe(true) + expect(gutter(files)).toEqual([12, 13, 13, 14]) + }) + + it('treats a Write the provider reported as a creation as an added file', () => { + const files = settledFiles({ + name: 'Write', + input: { file_path: '/repo/new.ts', content: 'one\ntwo\n' }, + result: { output: 'File created successfully at: /repo/new.ts' } + }) + expect(files?.[0]?.changeKind).toBe('added') + expect(gutter(files)).toEqual([1, 2]) + }) + + it('does not claim a creation for a Write over an existing file', () => { + const overwrite = settledFiles({ + name: 'Write', + input: { file_path: '/repo/a.ts', content: 'one\ntwo\n' }, + result: { output: 'The file /repo/a.ts has been updated.' } + }) + expect(overwrite?.[0]?.changeKind).toBe('edited') + // With no result at all there is no evidence of a creation either. + const unreported = settledFiles({ + name: 'Write', + input: { file_path: '/repo/a.ts', content: 'one\n' } + }) + expect(unreported?.[0]?.changeKind).toBe('edited') + }) + + it('reads a MultiEdit, whose snippet pairs sit in edits[]', () => { + const files = settledFiles({ + name: 'MultiEdit', + input: { + file_path: '/repo/a.ts', + edits: [ + { old_string: 'was', new_string: 'now' }, + { old_string: 'gone', new_string: 'kept' } + ] + } + }) + expect(files).toHaveLength(1) + expect(files?.[0]?.path).toBe('/repo/a.ts') + // 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) + }) + + it('leaves NotebookEdit to the generic tool view', () => { + expect(isEditToolName('NotebookEdit')).toBe(false) + }) + + it('drops the gutter numbers whenever they locate a snippet rather than the file', () => { + const files = settledFiles({ + name: 'Edit', + input: { file_path: '/repo/a.ts', old_string: 'keep\nwas', new_string: 'keep\nnow' } + }) + expect(files?.[0]?.lineNumbersKnown).toBe(false) + expect(gutter(files)).toEqual([null, null, null]) + expect( + files?.[0]?.lines.every((line) => line.oldLineNumber === null && line.newLineNumber === null) + ).toBe(true) + }) + + it('renders no card for an edit the provider rejected or has not landed', () => { + const failedInput = { file_path: '/repo/a.ts', old_string: 'missing', new_string: 'now' } + expect( + settledFiles({ + name: 'Edit', + input: failedInput, + result: { output: 'String to replace not found in file.', isError: true } + }) + ).toBeNull() + expect( + settledFiles({ + name: 'apply_patch', + input: { + changes: [{ path: 'a.ts', kind: { type: 'update' }, diff: '@@ -1 +1 @@\n-a\n+b' }] + }, + state: 'failed' + }) + ).toBeNull() + expect(settledFiles({ name: 'Edit', input: failedInput, state: 'running' })).toBeNull() + }) + + it('does not read a command tool result as a file edit', () => { + const patch = 'diff --git a/a.ts b/a.ts\n--- a/a.ts\n+++ b/a.ts\n@@ -1 +1 @@\n-was\n+now' + expect( + settledFiles({ + name: 'exec', + input: { command: 'git diff' }, + result: { output: patch } + }) + ).toBeNull() + // The structured journal's `Diff` item carries its patch only on the result. + const diffed = settledFiles({ + name: 'Diff', + input: { path: '/repo/a.ts' }, + result: { output: patch } + }) + expect(diffed?.[0]?.path).toBe('/repo/a.ts') + expect(diffed?.[0]?.added).toBe(1) + }) + + it('reports truncation when the content runs past the character cap', () => { + const files = settledFiles({ + name: 'Write', + input: { file_path: '/repo/a.ts', content: `${'x'.repeat(MAX_EDIT_CHARS)}\nlast\n` } + }) + expect(files?.[0]?.truncated).toBe(true) + // The clipped body ends mid-line, so its one row is real and must survive. + expect(files?.[0]?.lines).toHaveLength(1) + }) + + it('reads Codex structured changes, stripping the move marker from the body', () => { + const files = settledFiles({ + name: 'apply_patch', + input: { + changes: [ + { + path: 'old.ts', + kind: { type: 'update', move_path: 'new.ts' }, + diff: '@@ -1,2 +1,2 @@\n-a\n+b\n\nMoved to: new.ts' + } + ] + } + }) + expect(files?.[0]?.changeKind).toBe('renamed') + expect(files?.[0]?.lines.some((line) => line.text.includes('Moved to'))).toBe(false) + expect(gutter(files)).toEqual([1, 1]) + }) + + it('reads a Codex add change, which arrives as raw content with no hunk header', () => { + const files = settledFiles({ + name: 'apply_patch', + input: { changes: [{ path: 'new.ts', kind: { type: 'add' }, diff: 'one\ntwo' }] } + }) + expect(files?.[0]?.changeKind).toBe('added') + expect(files?.[0]?.added).toBe(2) + }) + + it('renders the file a write actually wrote, not one its content quotes', () => { + const files = settledFiles({ + name: 'Write', + input: { + file_path: 'docs/patch-format.md', + content: + 'Example:\n\n*** Begin Patch\n*** Update File: src/victim.ts\n@@\n-a\n+b\n*** End Patch\n' + }, + result: { output: 'File created successfully at: docs/patch-format.md' } + }) + expect(files?.map((file) => file.path)).toEqual(['docs/patch-format.md']) + expect(files?.[0]?.lines.some((line) => line.text.includes('Begin Patch'))).toBe(true) + }) + + it('finds an envelope in a command payload that arrived as JSON text', () => { + const envelope = '*** Begin Patch\n*** Update File: src/a.ts\n@@\n-was\n+now\n*** End Patch' + const files = settledFiles({ + name: 'shell', + input: JSON.stringify({ + command: ['bash', '-lc', `apply_patch <<'EOF'\n${envelope}\nEOF`], + workdir: '/repo' + }) + }) + expect(files?.[0]?.path).toBe('src/a.ts') + expect(files?.[0]?.added).toBe(1) + }) + + it('renders no card for a call the turn never answered', () => { + const input = { file_path: '/repo/a.ts', old_string: 'was', new_string: 'now' } + // No lifecycle and no result: nothing says the edit was applied. + expect(editFilesFromToolPair({ name: 'Edit', input })).toBeNull() + expect(editFilesFromToolPair({ name: 'Edit', input, state: 'completed' })).toHaveLength(1) + expect(editFilesFromToolPair({ name: 'Edit', input, result: { output: 'ok' } })).toHaveLength(1) + }) + + it('splits a multi-file patch into one card per file', () => { + const files = settledFiles({ + name: 'Diff', + input: { path: '/repo/one.ts' }, + result: { + output: + 'diff --git a/one.ts b/one.ts\n--- a/one.ts\n+++ b/one.ts\n@@ -1,1 +1,1 @@\n-first\n+FIRST\n' + + 'diff --git a/two.ts b/two.ts\n--- a/two.ts\n+++ b/two.ts\n@@ -10,1 +10,1 @@\n-second\n+SECOND' + } + }) + expect(files?.map((file) => file.path)).toEqual(['one.ts', 'two.ts']) + expect(files?.[0]?.lines.map((line) => line.text)).toEqual(['first', 'FIRST']) + expect(gutter(files?.slice(1) ?? null)).toEqual([10, 10]) + }) + + it('keeps a file whose envelope section carries no body at all', () => { + const files = settledFiles({ + name: 'apply_patch', + input: { + input: + '*** Begin Patch\n*** Update File: first.ts\n@@\n-a\n+b\n*** Update File: second.ts\n*** Update File: third.ts\n@@\n-c\n+d\n*** End Patch' + } + }) + expect(files?.map((file) => file.path)).toEqual(['first.ts', 'second.ts', 'third.ts']) + expect(files?.[1]?.lines).toEqual([]) + }) + + it('splits a multi-file patch written without per-file preamble lines', () => { + const files = settledFiles({ + name: 'Diff', + input: { path: '/repo/one.ts' }, + result: { + output: + '--- a/one.ts\n+++ b/one.ts\n@@ -1,1 +1,1 @@\n-first\n+FIRST\n' + + '--- a/two.ts\n+++ b/two.ts\n@@ -10,1 +10,1 @@\n-second\n+SECOND' + } + }) + expect(files?.map((file) => file.path)).toEqual(['one.ts', 'two.ts']) + expect(gutter(files?.slice(1) ?? null)).toEqual([10, 10]) + }) + + it('refuses a card when the call names a file count instead of a file', () => { + expect( + settledFiles({ + name: 'Diff', + input: { path: '2 files' }, + result: { output: '@@\n-a\n+b\n@@\n-c\n+d' } + }) + ).toBeNull() + }) + + it('reports a clipped patch as truncated instead of rendering its marker', () => { + const files = settledFiles({ + name: 'Diff', + input: { path: 'src/a.ts' }, + result: { output: '@@ -1,3 +1,3 @@\n ctx\n-was\n+now\n… (48210 bytes)' } + }) + expect(files?.[0]?.truncated).toBe(true) + expect(files?.[0]?.lines.map((line) => line.text)).toEqual(['ctx', 'was', 'now']) + }) + + it('reads a move appended to the patch body as a rename, as the other lane does', () => { + const files = settledFiles({ + name: 'Diff', + input: { path: 'src/old.ts' }, + result: { output: '@@ -1,1 +1,1 @@\n-a\n+b\n\nMoved to: src/new.ts' } + }) + expect(files?.[0]?.changeKind).toBe('renamed') + expect(files?.[0]?.path).toBe('src/new.ts') + expect(files?.[0]?.oldPath).toBe('src/old.ts') + expect(files?.[0]?.lines.some((line) => line.text.includes('Moved to'))).toBe(false) + }) + + it('does not read a row that merely mentions a move as one', () => { + const body = '@@ -1,2 +1,2 @@\n ctx\n+See Moved to: docs/archive/index.md' + const fromPatch = settledFiles({ + name: 'Diff', + input: { path: 'docs/index.md' }, + result: { output: body } + }) + const fromChanges = settledFiles({ + name: 'apply_patch', + input: { changes: [{ path: 'docs/index.md', kind: { type: 'update' }, diff: body }] } + }) + for (const files of [fromPatch, fromChanges]) { + expect(files?.[0]?.changeKind).toBe('edited') + expect(files?.[0]?.oldPath).toBeNull() + expect(files?.[0]?.path).toBe('docs/index.md') + expect(files?.[0]?.lines.at(-1)?.text).toBe('See Moved to: docs/archive/index.md') + } + }) + + it('accepts either spelling of the command that applies an envelope', () => { + const envelope = '*** Begin Patch\n*** Update File: src/a.ts\n@@\n-was\n+now\n*** End Patch' + const files = settledFiles({ + name: 'shell', + input: { command: ['bash', '-lc', `applypatch <<'EOF'\n${envelope}\nEOF`] } + }) + expect(files?.[0]?.path).toBe('src/a.ts') + }) + + it('keeps the header destination for a rename the call names by its old path', () => { + const files = settledFiles({ + name: 'Diff', + input: { path: 'old.txt' }, + result: { + output: + 'diff --git a/old.txt b/new.txt\n--- a/old.txt\n+++ b/new.txt\n@@ -1,1 +1,1 @@\n-a\n+b' + } + }) + expect(files?.[0]?.path).toBe('new.txt') + expect(files?.[0]?.oldPath).toBe('old.txt') + }) + + it('does not read a command that only quotes an envelope as an edit', () => { + const envelope = '*** Begin Patch\n*** Update File: src/real.ts\n@@\n-a\n+b\n*** End Patch' + expect( + settledFiles({ + name: 'shell', + input: { command: ['bash', '-lc', `cat > notes.md <<'EOF'\n${envelope}\nEOF`] } + }) + ).toBeNull() + }) + + it('does not call two compared directories a rename', () => { + const files = settledFiles({ + name: 'Diff', + input: {}, + result: { output: '--- d1/x.ts\n+++ d2/x.ts\n@@ -1,1 +1,1 @@\n-a\n+b' } + }) + expect(files?.[0]?.changeKind).toBe('edited') + expect(files?.[0]?.oldPath).toBeNull() + expect(files?.[0]?.path).toBe('d2/x.ts') + }) + + it('lets the call name the file when a preamble precedes the only header', () => { + const files = settledFiles({ + name: 'Diff', + input: { path: '/repo/one.ts' }, + result: { + output: + 'warning: something\ndiff --git a/one.ts b/one.ts\n--- a/one.ts\n+++ b/one.ts\n@@ -1,1 +1,1 @@\n-a\n+b' + } + }) + // The preamble is its own nameless section, and must not make this look + // like a patch over several files. + expect(files?.map((file) => file.path)).toEqual(['/repo/one.ts']) + }) + + it('returns null for a tool that did not edit a file', () => { + expect(settledFiles({ name: 'Bash', input: { command: 'ls' } })).toBeNull() + expect(isEditToolName('Bash')).toBe(false) + expect(isEditToolName('Edit')).toBe(true) + }) +}) diff --git a/src/shared/native-chat-edit-normalize.ts b/src/shared/native-chat-edit-normalize.ts new file mode 100644 index 00000000000..95c2ca18bd5 --- /dev/null +++ b/src/shared/native-chat-edit-normalize.ts @@ -0,0 +1,349 @@ +import { editFilesFromBeginPatch, unwrapBeginPatch } from './native-chat-begin-patch' +import { editLinesFromContents } from './native-chat-edit-lcs' +import { + finalizeEditFile, + pushEditGap, + type NativeChatEditFile, + type NativeChatEditLine +} from './native-chat-edit-model' +import { stripBoundedTextMarker } from './structured-agent-session-projection' +import { + editLinesFromUnifiedPatch, + editLinesFromWholeFile, + unifiedPatchSections, + type UnifiedPatchSection +} from './native-chat-unified-patch' +import type { NativeChatEditPatch } from './native-chat-types' + +// `NotebookEdit` is deliberately absent: its input carries only the new cell +// source, so a card would render an unchanged cell as wholly added. It falls +// through to the generic tool view instead. +const CLAUDE_EDIT_TOOLS = new Set(['Edit', 'MultiEdit', 'Write', 'str_replace']) +/** Command tools, which run a patch as one of many things they can run, so a + * quoted envelope is not evidence that one was applied. */ +const COMMAND_PATCH_TOOLS = new Set(['exec', 'shell', 'local_shell']) +/** Tools whose input may wrap a `*** Begin Patch` envelope. The dedicated patch + * tool applies whatever it is given; a command tool must say that it is. */ +const PATCH_ENVELOPE_TOOLS = new Set(['apply_patch', ...COMMAND_PATCH_TOOLS]) +/** A count standing in for a path, from a producer that joined several files' + * patches and kept no per-file path. */ +const FILE_COUNT_PATH = /^\d+ files?$/ +/** Tools whose whole payload is patch text. `Diff` reaches its patch only + * through the result, because the structured journal projects a diff item as a + * call carrying just the path. */ +const PATCH_TEXT_TOOLS = new Set(['apply_patch', 'Diff']) + +export function isEditToolName(name: string): boolean { + return CLAUDE_EDIT_TOOLS.has(name) || PATCH_ENVELOPE_TOOLS.has(name) || PATCH_TEXT_TOOLS.has(name) +} + +function record(value: unknown): Record | null { + return typeof value === 'object' && value !== null ? (value as Record) : null +} + +function text(value: unknown): string | null { + return typeof value === 'string' ? value : null +} + +/** Rows straight from resolved hunks, which is the only path with true numbers + * for a provider that reports its edits as a snippet pair. */ +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) { + if (raw.startsWith('+')) { + lines.push({ kind: 'add', text: raw.slice(1), oldLineNumber: null, newLineNumber: newNo }) + newNo += 1 + } else if (raw.startsWith('-')) { + lines.push({ kind: 'del', text: raw.slice(1), oldLineNumber: oldNo, newLineNumber: null }) + oldNo += 1 + } else { + lines.push({ + kind: 'context', + text: raw.startsWith(' ') ? raw.slice(1) : raw, + oldLineNumber: oldNo, + newLineNumber: newNo + }) + oldNo += 1 + newNo += 1 + } + } + } + return lines +} + +/** A whole-content write looks identical whether it created the file or + * overwrote one, so only positive evidence may claim a creation. With no + * evidence either way this errs toward the weaker claim: calling a creation an + * edit is imprecise, while calling an overwrite a creation is false and paints + * an existing file as wholly new. */ +const CREATED_FILE_RESULT = /^\s*File created successfully/ + +function wholeContentChangeKind( + input: Record, + output: string | undefined +): 'added' | 'edited' { + if (text(input.command) === 'create') { + return 'added' + } + return output !== undefined && CREATED_FILE_RESULT.test(output) ? 'added' : 'edited' +} + +/** `MultiEdit` carries its snippet pairs in `edits[]`, not at the top level. */ +function multiEditFiles(input: Record, path: string): NativeChatEditFile[] | null { + if (!Array.isArray(input.edits)) { + return null + } + const lines: NativeChatEditLine[] = [] + let truncated = false + for (const entry of input.edits) { + const edit = record(entry) + const oldString = text(edit?.old_string) ?? text(edit?.oldString) + const newString = text(edit?.new_string) ?? text(edit?.newString) + 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 + } + if (lines.length === 0) { + return null + } + return [ + finalizeEditFile({ + path, + oldPath: null, + changeKind: 'edited', + lines, + // A snippet pair cannot say where in the file it sits. + lineNumbersKnown: false, + truncated + }) + ] +} + +function claudeEditFiles( + name: string, + input: Record, + output: string | undefined +): NativeChatEditFile[] | null { + const path = text(input.file_path) ?? text(input.path) ?? 'file' + if (name === 'MultiEdit') { + return multiEditFiles(input, path) + } + const oldString = text(input.old_string) ?? text(input.oldString) + const newString = text(input.new_string) ?? text(input.newString) + const content = text(input.content) ?? text(input.file_text) + if (oldString === null && content !== null) { + const whole = editLinesFromWholeFile(content, 'add') + return [ + finalizeEditFile({ + path, + oldPath: null, + changeKind: wholeContentChangeKind(input, output), + lines: whole.lines, + lineNumbersKnown: true, + truncated: whole.truncated + }) + ] + } + if (oldString === null && newString === null) { + return null + } + const diffed = editLinesFromContents(oldString ?? '', newString ?? content ?? '') + return [ + finalizeEditFile({ + path, + oldPath: null, + changeKind: 'edited', + lines: diffed.lines, + // A snippet pair cannot say where in the file it sits. + lineNumbersKnown: false, + truncated: diffed.truncated + }) + ] +} + +/** A move is appended to the patch body as prose rather than a header field, on + * every lane that carries the body as text. Left in place it renders as a + * numbered line of the file it moved. + * + * Anchored to the start of the final line: unanchored, a row whose own content + * mentions a move was cut in half and the file it names claimed as a rename + * that never happened. */ +const MOVE_MARKER = /(?:^|\n)Moved to: (.+)$/ + +function splitMoveMarker(patch: string): { body: string; movedTo: string | null } { + const match = MOVE_MARKER.exec(patch) + return match + ? { body: patch.slice(0, match.index), movedTo: match[1]!.trim() } + : { body: patch, movedTo: null } +} + +function codexChangeFiles(changes: unknown[]): NativeChatEditFile[] { + return changes.flatMap((entry) => { + const change = record(entry) + const path = text(change?.path) + const diff = text(change?.diff) + if (!change || !path || !diff) { + return [] + } + const kind = record(change.kind) + const kindType = text(kind?.type) ?? text(change.kind) ?? 'update' + const movePath = text(kind?.move_path) ?? text(change.movePath) + if (kindType === 'add' || kindType === 'delete') { + // Add and delete arrive as raw file content, with no hunk header or signs. + const whole = editLinesFromWholeFile(diff, kindType === 'add' ? 'add' : 'del') + return [ + finalizeEditFile({ + path, + oldPath: null, + changeKind: kindType === 'add' ? 'added' : 'deleted', + lines: whole.lines, + lineNumbersKnown: true, + truncated: whole.truncated + }) + ] + } + const parsed = editLinesFromUnifiedPatch(splitMoveMarker(diff).body) + if (!parsed) { + return [] + } + return [ + finalizeEditFile({ + path: movePath ?? path, + oldPath: movePath ? path : null, + changeKind: movePath ? 'renamed' : 'edited', + lines: parsed.lines, + lineNumbersKnown: parsed.lineNumbersKnown, + truncated: parsed.truncated + }) + ] + }) +} + +/** One diff model for a tool call and its result, across every shape the + * supported agents use to report a file edit. */ +export function editFilesFromToolPair(pair: { + name: string + input: unknown + /** Provider lifecycle for the call, when the lane reports one. */ + state?: 'running' | 'completed' | 'failed' + result?: { output?: string; isError?: boolean; editPatch?: NativeChatEditPatch } +}): NativeChatEditFile[] | null { + // A card states the edit as made, so it takes evidence that it landed: the + // provider reporting the call complete, or a result that is not an error. + // Anything else — failed, still running, or a turn that stopped before the + // call was answered — keeps the generic tool view and its error body. + if (pair.state === 'failed' || pair.state === 'running' || pair.result?.isError === true) { + return null + } + if (pair.state !== 'completed' && pair.result === undefined) { + return null + } + const input = record(pair.input) + const patch = pair.result?.editPatch + if (patch && patch.hunks.length > 0) { + return [ + finalizeEditFile({ + path: patch.filePath ?? text(input?.file_path) ?? 'file', + oldPath: null, + changeKind: 'edited', + lines: linesFromEditPatch(patch), + lineNumbersKnown: true + }) + ] + } + + // Only a tool that runs a patch may be searched for an envelope: a file's own + // contents can quote one, and scanning a write's payload rendered a card for + // the quoted file while the file actually written never appeared. + if (PATCH_ENVELOPE_TOOLS.has(pair.name)) { + const envelope = unwrapBeginPatch(pair.input, { + requireApplyCommand: COMMAND_PATCH_TOOLS.has(pair.name) + }) + const files = envelope ? editFilesFromBeginPatch(envelope) : [] + if (files.length > 0) { + return files + } + } + + if (input && Array.isArray(input.changes)) { + const files = codexChangeFiles(input.changes) + if (files.length > 0) { + return files + } + } + + if (input && CLAUDE_EDIT_TOOLS.has(pair.name)) { + return claudeEditFiles(pair.name, input, pair.result?.output) + } + + if (!PATCH_TEXT_TOOLS.has(pair.name)) { + return null + } + // The result fallback is scoped to `Diff`, whose call carries only a path. + // Reading any command tool's output as a patch reclassified `git diff` as a + // file edit and swallowed the command line with it. + const patchText = + text(input?.patch) ?? text(input?.diff) ?? (pair.name === 'Diff' ? pair.result?.output : null) + if (!patchText) { + return null + } + // The body carries its own marker when the journal clipped it. Read as + // content it becomes a numbered line of the file, and the rows that follow + // are reported complete. + const bounded = stripBoundedTextMarker(patchText) + const moved = splitMoveMarker(bounded.text) + // One card per file the patch touches: run together, the later files' rows + // and gutter numbers sit under the first file's name. + const split = unifiedPatchSections(moved.body) + const callerPath = text(input?.path) ?? text(input?.file_path) + if (callerPath !== null && FILE_COUNT_PATH.test(callerPath)) { + // The producer joined several files' patches and kept a count in place of a + // path, so nothing here can name a file. Naming the card after the count + // would assert a file that does not exist. + return null + } + // A patch that names one file is the file the call is reporting on, so the + // call's own path wins — it is the provider's, where the header's is relative + // to the patch. A patch naming several has no one path, and a rename's + // destination is only ever in the header. Sections that name nothing are + // preamble and must not change that count. + const namedSections = split.sections.filter((section) => section.path !== null).length + const named = (section: UnifiedPatchSection): string => + (namedSections <= 1 && section.oldPath === null + ? (callerPath ?? section.path) + : (section.path ?? callerPath)) ?? 'file' + const files = split.sections.flatMap((section) => { + const parsed = editLinesFromUnifiedPatch(section.body) + if (!parsed && section.path === null) { + return [] + } + return [ + finalizeEditFile({ + path: named(section), + oldPath: section.oldPath, + changeKind: section.changeKind, + lines: parsed?.lines ?? [], + lineNumbersKnown: parsed?.lineNumbersKnown ?? false, + truncated: bounded.truncated || split.truncated || (parsed?.truncated ?? false) + }) + ] + }) + // The move marker names where the whole patch moved, so it can only speak for + // a patch describing one file. + if (moved.movedTo !== null && files.length === 1 && files[0]) { + const only = files[0] + return [{ ...only, path: moved.movedTo, oldPath: only.path, changeKind: 'renamed' }] + } + return files.length > 0 ? files : null +} diff --git a/src/shared/native-chat-types.ts b/src/shared/native-chat-types.ts index 5daa16f760e..124ee55dbe1 100644 --- a/src/shared/native-chat-types.ts +++ b/src/shared/native-chat-types.ts @@ -55,11 +55,31 @@ export type NativeChatToolCallBlock = { state?: 'running' | 'completed' | 'failed' } +/** One resolved hunk from a provider's edit result, carrying true file ranges. */ +export type NativeChatEditPatchHunk = { + oldStart: number + oldLines: number + newStart: number + newLines: number + /** Signed unified rows, as the provider emitted them. */ + lines: string[] +} + +/** Hunks the provider resolved against the real file before reporting the edit. + * Claude supplies these on its edit results; Codex resolves equivalently before + * sending, so its patch already carries ranges and needs no companion. */ +export type NativeChatEditPatch = { + filePath?: string + hunks: NativeChatEditPatchHunk[] +} + /** The result returned to the agent for a prior tool call. */ export type NativeChatToolResultBlock = { type: 'tool-result' output: string isError?: boolean + /** Present only for edit tools whose result reported resolved hunks. */ + editPatch?: NativeChatEditPatch } /** A reference to an image, by local path or remote URL. Exactly the field diff --git a/src/shared/native-chat-unified-patch.ts b/src/shared/native-chat-unified-patch.ts new file mode 100644 index 00000000000..2e34c79a6b3 --- /dev/null +++ b/src/shared/native-chat-unified-patch.ts @@ -0,0 +1,255 @@ +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+))? @@/ + +export type UnifiedPatchLines = { + lines: NativeChatEditLine[] + /** True only when every hunk carried real `@@` ranges. */ + lineNumbersKnown: boolean + /** The patch text was clipped before it became rows. */ + truncated: boolean +} + +/** Parses unified patch text, keeping the `@@` ranges as per-row line numbers. + * 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 = options?.implicitFirstHunk === true + let ranged = true + 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) + 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 + } + // `\ No newline at end of file` sits mid-hunk, between the removed old last + // line and the added new one, so it ends nothing. + if (raw.startsWith('\\')) { + continue + } + if (!inHunk && isFileHeaderPair(rows, index)) { + index += 1 + continue + } + if (FILE_SECTION_START.test(raw)) { + inHunk = false + continue + } + 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', + text: raw.slice(1), + oldLineNumber: null, + newLineNumber: newNo + }) + newNo = newNo === null ? null : newNo + 1 + continue + } + if (raw.startsWith('-')) { + lines.push({ + kind: 'del', + text: raw.slice(1), + oldLineNumber: oldNo, + newLineNumber: null + }) + oldNo = oldNo === null ? null : oldNo + 1 + continue + } + lines.push({ + kind: 'context', + text: raw.startsWith(' ') ? raw.slice(1) : raw, + oldLineNumber: oldNo, + newLineNumber: newNo + }) + oldNo = oldNo === null ? null : oldNo + 1 + newNo = newNo === null ? null : newNo + 1 + } + + if (!sawHunk || lines.length === 0) { + return null + } + return { lines, lineNumbersKnown: ranged, truncated: source.truncated } +} + +const GIT_DIFF_HEADER = 'diff --git ' + +export type UnifiedPatchSection = { + /** Null when the patch text named no file, leaving it to the caller. */ + path: string | null + oldPath: string | null + changeKind: 'added' | 'deleted' | 'edited' | 'renamed' + body: string +} + +type Section = { + rows: string[] + oldPath: string | null + newPath: string | null + named: boolean + /** A `--- `/`+++ ` pair already named this section, so the next one is a new file. */ + hasHeaderPair: boolean + /** Only a `diff --git` header states both sides of a move as such. A bare + * pair with differing paths is just as likely two directories compared. */ + fromGitHeader: boolean +} + +/** Splits patch text into one section per file it touches. Without this a + * multi-file patch renders as a single card under the first file's name, with + * the later files' rows and gutter numbers beneath it. */ +export function unifiedPatchSections(text: string): { + sections: UnifiedPatchSection[] + truncated: boolean +} { + const source = splitEditContent(text) + const rows = source.lines + const sections: Section[] = [] + let current: Section | null = null + let inHunk = false + + const open = (): Section => { + const section: Section = { + rows: [], + oldPath: null, + newPath: null, + named: false, + hasHeaderPair: false, + fromGitHeader: false + } + sections.push(section) + return section + } + + for (let index = 0; index < rows.length; index += 1) { + const raw = rows[index] ?? '' + if (raw.startsWith(GIT_DIFF_HEADER)) { + const paths = gitHeaderPaths(raw) + current = open() + current.oldPath = paths.oldPath + current.newPath = paths.newPath + current.named = true + current.fromGitHeader = true + inHunk = false + continue + } + // A header pair is structure outside a hunk. Inside one it is also a file + // boundary, but only when a hunk header follows it immediately: a removed + // `-- x` over an added `++ y` is never followed by a column-0 `@@`, and + // that is what separates the files of a patch written without `diff --git` + // headers, where nothing else would end the previous file's hunk. + if (isFileHeaderPair(rows, index) && (!inHunk || (rows[index + 2] ?? '').startsWith('@@'))) { + // The pair names the section a `diff --git` just opened; a second pair in + // the same section is the next file of a patch written without them. + if (!current || current.hasHeaderPair) { + current = open() + } + current.oldPath = sourceHeaderPath(rows[index] ?? '') + current.newPath = sourceHeaderPath(rows[index + 1] ?? '') + current.named = true + current.hasHeaderPair = true + inHunk = false + index += 1 + continue + } + if (raw.startsWith('@@')) { + inHunk = true + } else if (FILE_SECTION_START.test(raw)) { + inHunk = false + } + current ??= open() + current.rows.push(raw) + } + + return { + sections: sections.map((section) => ({ + path: section.newPath ?? section.oldPath, + oldPath: sectionChangeKind(section) === 'renamed' ? section.oldPath : null, + changeKind: sectionChangeKind(section), + body: section.rows.join('\n') + })), + truncated: source.truncated + } +} + +function sectionChangeKind(section: Section): UnifiedPatchSection['changeKind'] { + if (!section.named) { + return 'edited' + } + if (section.newPath === null) { + return 'deleted' + } + if (section.oldPath === null) { + return 'added' + } + if (section.oldPath === section.newPath) { + return 'edited' + } + // Differing sides are a move only where the header says so. Bare pairs carry + // whatever paths the producer compared, which may be two directories. + return section.fromGitHeader ? 'renamed' : 'edited' +} + +/** `--- a/` / `+++ b/`, where the absent side is `/dev/null` and a + * trailing tab introduces the timestamp some producers append. */ +function sourceHeaderPath(line: string): string | null { + const value = (line.slice(4).split('\t')[0] ?? '').trim() + return value === '' || value === '/dev/null' ? null : value.replace(/^[ab]\//, '') +} + +function gitHeaderPaths(line: string): { oldPath: string | null; newPath: string | null } { + const rest = line.slice(GIT_DIFF_HEADER.length) + // Both halves carry the same path unless the file moved, so the second one + // starts at the last ` b/` rather than at the first space. + const split = rest.lastIndexOf(' b/') + if (split === -1) { + return { oldPath: null, newPath: null } + } + return { + oldPath: rest.slice(0, split).replace(/^a\//, ''), + newPath: rest.slice(split + 1).replace(/^b\//, '') + } +} + +/** Rows for a whole-file add or delete, which legitimately number from 1. */ +export function editLinesFromWholeFile( + content: string, + kind: 'add' | 'del' +): { lines: NativeChatEditLine[]; truncated: boolean } { + const body = splitEditContent(content) + return { + lines: body.lines.map((text, index) => ({ + kind, + text, + oldLineNumber: kind === 'del' ? index + 1 : null, + newLineNumber: kind === 'add' ? index + 1 : null + })), + truncated: body.truncated + } +} diff --git a/src/shared/structured-agent-session-projection.ts b/src/shared/structured-agent-session-projection.ts index 71cffa43762..94938dc955f 100644 --- a/src/shared/structured-agent-session-projection.ts +++ b/src/shared/structured-agent-session-projection.ts @@ -6,6 +6,22 @@ function boundedText(payload: { head: string; truncated: boolean; byteLength: nu return payload.truncated ? `${payload.head}\n… (${payload.byteLength} bytes)` : payload.head } +/** The markers a clipped payload carries in its own text, anchored to the end + * so nothing that merely looks like one inside the body can match. */ +const BOUNDED_TEXT_MARKERS = [ + /\n… \(\d+ bytes\)$/, + /\n\[Orca: output truncated — \d+ bytes total, digest [0-9a-f]+\]$/ +] + +/** Recovers the clipped body from a bounded payload's text, and says whether a + * marker was there. A reader that treats the text as content renders the + * marker as a line of it — with a line number, which reads as a real position + * in the file — and reports the body as complete. */ +export function stripBoundedTextMarker(text: string): { text: string; truncated: boolean } { + const stripped = BOUNDED_TEXT_MARKERS.reduce((value, marker) => value.replace(marker, ''), text) + return { text: stripped, truncated: stripped.length !== text.length } +} + function itemBlocks(item: AgentJournalRenderItem): { role: NativeChatMessage['role'] blocks: NativeChatBlock[]