mirror of
https://github.com/stablyai/orca.git
synced 2026-10-04 00:02:21 +00:00
fix(native-chat): keep every edited file, and mark where the diff breaks
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.
This commit is contained in:
Merge Sim
@@ -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<string, unknown>): 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<string, unknown> | null {
|
||||
try {
|
||||
const parsed: unknown = JSON.parse(value)
|
||||
return typeof parsed === 'object' && parsed !== null && !Array.isArray(parsed)
|
||||
? (parsed as Record<string, unknown>)
|
||||
: null
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
function codexToolResult(output: unknown): NativeChatBlock {
|
||||
const record = asRecord(output)
|
||||
const isError = record?.success === false || record?.is_error === true
|
||||
|
||||
@@ -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' })
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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')
|
||||
|
||||
@@ -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 (
|
||||
<div
|
||||
role="separator"
|
||||
aria-label={translate('components.native-chat.tool.diffGap', 'Lines not shown')}
|
||||
className="select-none border-y border-border/60 bg-accent/30 py-0.5 text-center text-muted-foreground"
|
||||
>
|
||||
⋯
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
function DiffRow({ line, gutterWidth }: { line: NativeChatEditLine; gutterWidth: number }) {
|
||||
if (line.kind === 'gap') {
|
||||
return <DiffGapRow />
|
||||
}
|
||||
return (
|
||||
<div
|
||||
className={cn(
|
||||
@@ -89,7 +107,8 @@ function DiffRow({ line, gutterWidth }: { line: NativeChatEditLine; gutterWidth:
|
||||
/** Inline card for one file an agent edited: verb header, path with change
|
||||
* counts, and the unified rows. The gutter is blank when the provider gave no
|
||||
* resolved ranges, because a snippet-relative number would read as a file
|
||||
* position. */
|
||||
* position. A change reported with no body — a delete names the file and
|
||||
* nothing else — keeps the header rows and offers no empty disclosure. */
|
||||
export function NativeChatDiffCard({
|
||||
file,
|
||||
initiallyExpanded = false
|
||||
@@ -98,6 +117,7 @@ export function NativeChatDiffCard({
|
||||
initiallyExpanded?: boolean
|
||||
}): React.JSX.Element {
|
||||
const [expanded, setExpanded] = useState(initiallyExpanded)
|
||||
const hasBody = file.lines.length > 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({
|
||||
<div className="my-1 overflow-hidden rounded-md border border-border">
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => setExpanded((value) => !value)}
|
||||
className="group flex w-full items-center gap-1.5 px-2 py-1 text-left hover:bg-accent/30"
|
||||
aria-expanded={expanded}
|
||||
onClick={() => hasBody && setExpanded((value) => !value)}
|
||||
className={cn(
|
||||
'group flex w-full items-center gap-1.5 px-2 py-1 text-left',
|
||||
hasBody ? 'cursor-pointer hover:bg-accent/30' : 'cursor-default'
|
||||
)}
|
||||
aria-expanded={hasBody ? expanded : undefined}
|
||||
>
|
||||
<VerbIcon kind={file.changeKind} />
|
||||
<span className="shrink-0 text-[11px] text-muted-foreground group-hover:text-foreground/80">
|
||||
{verbLabel(file)}
|
||||
</span>
|
||||
<ChevronRight
|
||||
className={cn(
|
||||
'size-3.5 shrink-0 text-muted-foreground transition-transform',
|
||||
expanded && 'rotate-90'
|
||||
)}
|
||||
/>
|
||||
{hasBody ? (
|
||||
<ChevronRight
|
||||
className={cn(
|
||||
'size-3.5 shrink-0 text-muted-foreground transition-transform',
|
||||
expanded && 'rotate-90'
|
||||
)}
|
||||
/>
|
||||
) : null}
|
||||
</button>
|
||||
<div className="flex items-center gap-1.5 border-t border-border bg-accent/40 px-2 py-1">
|
||||
{file.oldPath ? (
|
||||
@@ -144,7 +169,7 @@ export function NativeChatDiffCard({
|
||||
className="ml-auto shrink-0"
|
||||
/>
|
||||
</div>
|
||||
{expanded ? (
|
||||
{hasBody && expanded ? (
|
||||
// Focusable so the rows can be scrolled from the keyboard.
|
||||
<div
|
||||
tabIndex={0}
|
||||
|
||||
@@ -137,6 +137,51 @@ describe('NativeChatToolRun', () => {
|
||||
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(<NativeChatToolRun blocks={blocks} expandSignal />)
|
||||
|
||||
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(<NativeChatToolRun blocks={blocks} expandSignal />)
|
||||
|
||||
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' },
|
||||
|
||||
@@ -16964,6 +16964,7 @@
|
||||
"deletedFile": "Deleted file",
|
||||
"renamedFile": "Renamed file",
|
||||
"copyDiff": "Copy diff",
|
||||
"diffGap": "Lines not shown",
|
||||
"diffTruncated": "Diff truncated"
|
||||
},
|
||||
"providerFrame": {
|
||||
|
||||
@@ -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<string, unknown>)
|
||||
? envelopeArgument(input as Record<string, unknown>)
|
||||
: 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, unknown>): 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, unknown>): 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<string, string> = { 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<number, string>()
|
||||
|
||||
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 []
|
||||
}
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
|
||||
@@ -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<string, unknown>, 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
|
||||
|
||||
@@ -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',
|
||||
|
||||
Reference in New Issue
Block a user