Skip to content

Commit 4c765b3

Browse files
authored
Fix edit tool diff rendering (#601)
1 parent 868e2f1 commit 4c765b3

3 files changed

Lines changed: 107 additions & 1 deletion

File tree

cli/src/components/tools/str-replace.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
extractDiff,
88
extractFilePath,
99
isCreateFile,
10+
shouldShowEditDiff,
1011
} from '../../utils/implementor-helpers'
1112

1213
import type { ToolRenderConfig } from './types'
@@ -60,13 +61,14 @@ export const StrReplaceComponent = defineToolComponent({
6061
const diff = extractDiff(toolBlock)
6162
const filePath = extractFilePath(toolBlock)
6263
const isCreate = isCreateFile(toolBlock)
64+
const showDiff = shouldShowEditDiff(toolBlock)
6365

6466
return {
6567
content: (
6668
<EditBody
6769
name={isCreate ? 'Create' : 'Edit'}
6870
filePath={filePath}
69-
diffText={diff ?? ''}
71+
diffText={showDiff ? (diff ?? '') : ''}
7072
isCreate={isCreate}
7173
/>
7274
),

cli/src/utils/__tests__/implementor-helpers.test.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
groupConsecutiveToolBlocks,
1818
getMultiPromptProgress,
1919
getMultiPromptPreview,
20+
shouldShowEditDiff,
2021
} from '../implementor-helpers'
2122

2223
import type {
@@ -368,6 +369,82 @@ describe('getFileChangeType', () => {
368369
})
369370
})
370371

372+
describe('shouldShowEditDiff', () => {
373+
test('does not show pending str_replace diffs before the result arrives', () => {
374+
const block: ToolContentBlock = {
375+
type: 'tool',
376+
toolCallId: 'test-1',
377+
toolName: 'str_replace',
378+
input: {
379+
replacements: [{ oldString: 'const x = 1', newString: 'const x = 2' }],
380+
},
381+
}
382+
383+
expect(shouldShowEditDiff(block)).toBe(false)
384+
})
385+
386+
test('shows str_replace diffs after a successful result', () => {
387+
const block: ToolContentBlock = {
388+
type: 'tool',
389+
toolCallId: 'test-1',
390+
toolName: 'str_replace',
391+
input: {
392+
replacements: [{ oldString: 'const x = 1', newString: 'const x = 2' }],
393+
},
394+
output: 'file: src/existing.ts\nmessage: String replace applied successfully.',
395+
}
396+
397+
expect(shouldShowEditDiff(block)).toBe(true)
398+
})
399+
400+
test('does not show pending write_file diffs before the result arrives', () => {
401+
const block: ToolContentBlock = {
402+
type: 'tool',
403+
toolCallId: 'test-1',
404+
toolName: 'write_file',
405+
input: { path: 'src/new.ts', content: 'const x = 1\n' },
406+
}
407+
408+
expect(extractDiff(block)).toBe('+ const x = 1\n+ ')
409+
expect(shouldShowEditDiff(block)).toBe(false)
410+
})
411+
412+
test('shows write_file diffs after an overwrite result', () => {
413+
const block: ToolContentBlock = {
414+
type: 'tool',
415+
toolCallId: 'test-1',
416+
toolName: 'write_file',
417+
input: { path: 'src/existing.ts', content: 'const x = 2\n' },
418+
output: 'file: src/existing.ts\nmessage: Overwrote file successfully.',
419+
}
420+
421+
expect(shouldShowEditDiff(block)).toBe(true)
422+
})
423+
424+
test('does not show write_file diffs after a create result', () => {
425+
const block: ToolContentBlock = {
426+
type: 'tool',
427+
toolCallId: 'test-1',
428+
toolName: 'write_file',
429+
input: { path: 'src/new.ts', content: 'const x = 1\n' },
430+
output: 'file: src/new.ts\nmessage: Created file successfully.',
431+
}
432+
433+
expect(shouldShowEditDiff(block)).toBe(false)
434+
})
435+
436+
test('continues to show pending proposed write_file diffs', () => {
437+
const block: ToolContentBlock = {
438+
type: 'tool',
439+
toolCallId: 'test-1',
440+
toolName: 'propose_write_file',
441+
input: { path: 'src/new.ts', content: 'const x = 1\n' },
442+
}
443+
444+
expect(shouldShowEditDiff(block)).toBe(true)
445+
})
446+
})
447+
371448
describe('getFileStatsFromBlocks', () => {
372449
test('aggregates stats for same file', () => {
373450
const blocks: ContentBlock[] = [

cli/src/utils/implementor-helpers.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,33 @@ export function isCreateFile(toolBlock: ToolContentBlock): boolean {
430430
)
431431
}
432432

433+
function hasToolResultOutput(toolBlock: ToolContentBlock): boolean {
434+
const outputStr = typeof toolBlock.output === 'string' ? toolBlock.output : ''
435+
return outputStr.length > 0 || toolBlock.outputRaw !== undefined
436+
}
437+
438+
/**
439+
* Decide whether the direct edit tool renderer should show a diff preview.
440+
*
441+
* Real edit tool calls render immediately with input only, then receive output
442+
* once the edit completes. Wait for that result before showing diffs so create
443+
* operations never briefly flash an input-derived full-file diff.
444+
*/
445+
export function shouldShowEditDiff(toolBlock: ToolContentBlock): boolean {
446+
if (!extractDiff(toolBlock) || isCreateFile(toolBlock)) {
447+
return false
448+
}
449+
450+
if (
451+
!isProposedToolName(toolBlock.toolName) &&
452+
!hasToolResultOutput(toolBlock)
453+
) {
454+
return false
455+
}
456+
457+
return true
458+
}
459+
433460
export interface TimelineItem {
434461
type: 'commentary' | 'edit'
435462
content: string // For commentary: the text. For edits: file path

0 commit comments

Comments
 (0)