From e9f3d1f0c5a22714d5bf2fc9976bef8bb09b0111 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Tue, 7 Apr 2026 23:47:25 +1000 Subject: [PATCH 1/3] Split PreviewEdit edits into lineEdits + textEdits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old flat `edits` array forced Claude to mentally simulate line-number offsets when mixing insert/delete ops with text-search ops in a single call. The implementation applied text-search patches against the original file positions, then merged them with structural edits — but never adjusted for insert/delete offsets. Result: text replacements landed N lines too early silently. New schema: - `lineEdits` (insert | replace | delete): all line numbers reference the file before the call. Tool sorts bottom-to-top internally, so no offset arithmetic needed from the caller. - `textEdits` (replace_text | regex_text): applied in order after all lineEdits complete, searching post-lineEdit content. This eliminates the entire class of offset bugs and the workaround rules that documented them. The Ref default limit description was also updated (limit was raised from 1000 to 10000 in a previous commit but the description string and tests still said 1000). --- .claude/CLAUDE.md | 5 +- .../claude-sdk-tools/src/EditFile/EditFile.ts | 120 +++----- .../claude-sdk-tools/src/EditFile/schema.ts | 36 ++- .../claude-sdk-tools/src/EditFile/types.ts | 5 +- .../src/EditFile/validateEdits.ts | 51 ++-- packages/claude-sdk-tools/src/Ref/Ref.ts | 2 +- packages/claude-sdk-tools/src/Ref/schema.ts | 2 +- .../claude-sdk-tools/test/EditFile.spec.ts | 256 ++++++++++-------- packages/claude-sdk-tools/test/Ref.spec.ts | 12 +- 9 files changed, 243 insertions(+), 246 deletions(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index da4ff60..afef2cd 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -237,7 +237,10 @@ Opt-in via `shellicarMcp: true` config. Registers an in-process MCP server (`she - **MVVM architecture refactor** (2026-04-06): Three-layer model — State (pure data + transitions), Renderer (pure `(state, cols) → string[]`), ScreenCoordinator (owns screen, routes events, calls renderers). Pull-based: coordinator decides when to render. Plan in `.claude/plans/architecture-refactor.md`. Enables unit testing of state and render logic without terminal knowledge. -- **`previousPatchId` chaining for multi-patch edits** (2026-04-06): When making sequential edits to the same file, chain `PreviewEdit` calls using `previousPatchId`, then apply once with `EditFile`. Previewing without applying then moving to a second patch is the failure mode — only the second patch gets applied, first is silently dropped. esbuild and vitest don't catch this; it only surfaces at runtime. +- **`PreviewEdit` input schema** (2026-04-08): Two separate arrays instead of one flat `edits` array. + - `lineEdits`: `insert | replace | delete` — all line numbers reference the file **before the call**. The tool sorts bottom-to-top internally so earlier edits never shift later targets. Safe to specify in any order. + - `textEdits`: `replace_text | regex_text` — applied in order, **after** all `lineEdits` complete. They see the post-`lineEdits` content, not the original. + - Use `previousPatchId` when you need truly sequential steps where each step depends on the previous result (e.g., a `replace_text` that only matches after a previous `PreviewEdit` + `EditFile` has landed). - **f command clipboard system** (2026-04-05): Three-stage `readClipboardPath()` — (1) pbpaste filtered by `looksLikePath`, (2) VS Code `code/file-list` JXA probe (file:// URI → POSIX path), (3) osascript `furl` filtered by `sanitiseFurlResult`. Injectable `readClipboardPathCore` for tests. `looksLikePath` is permissive (accepts bare-relative like `apps/foo/bar.ts`); `isLikelyPath` in AppLayout is strict (explicit prefixes only) and only used for the missing-chip case. `sanitiseFurlResult` rejects paths containing `:` (HFS artifacts). `f` handler is stat-first: if the file exists attach it directly; only apply `isLikelyPath` if stat fails. - **Clipboard text attachments** (2026-04-06): `ctrl+/` enters command mode; `t` reads clipboard via `pbpaste` and adds a `` block attachment; `d` removes selected chip; `← →` select chips. On `ctrl+enter` submit, attachments are folded into the prompt as `` XML blocks and cleared. - **ConversationHistory ID tagging** (2026-04-06): `push(msg, { id? })` tags messages for later removal. `remove(id)` splices the last item with matching ID. IDs are session-scoped (not persisted). Used by `IAnthropicAgent.injectContext/removeContext` for skills context management. diff --git a/packages/claude-sdk-tools/src/EditFile/EditFile.ts b/packages/claude-sdk-tools/src/EditFile/EditFile.ts index 45fad1f..efab060 100644 --- a/packages/claude-sdk-tools/src/EditFile/EditFile.ts +++ b/packages/claude-sdk-tools/src/EditFile/EditFile.ts @@ -6,44 +6,8 @@ import type { IFileSystem } from '../fs/IFileSystem'; import { applyEdits } from './applyEdits'; import { generateDiff } from './generateDiff'; import { PreviewEditInputSchema, PreviewEditOutputSchema } from './schema'; -import type { EditOperationType, PreviewEditOutputType, ResolvedEditOperationType } from './types'; -import { validateEdits } from './validateEdits'; - -/** - * Given two versions of a file split into lines, return a minimal set of - * line-based operations (replace / delete / insert) that transforms the - * original into the new content. The algorithm finds the longest common - * prefix and suffix and emits a single operation for the changed middle - * region, which is sufficient for all replace_text use-cases. - */ -function findChangedRegions(originalLines: string[], newLines: string[]): ResolvedEditOperationType[] { - if (originalLines.join('\n') === newLines.join('\n')) { - return []; - } - - let start = 0; - while (start < originalLines.length && start < newLines.length && originalLines[start] === newLines[start]) { - start++; - } - - let endOrig = originalLines.length - 1; - let endNew = newLines.length - 1; - while (endOrig > start && endNew > start && originalLines[endOrig] === newLines[endNew]) { - endOrig--; - endNew--; - } - - if (endOrig < start) { - // Pure insertion between lines - return [{ action: 'insert', after_line: start, content: newLines.slice(start, endNew + 1).join('\n') }]; - } - if (endNew < start) { - // Pure deletion - return [{ action: 'delete', startLine: start + 1, endLine: endOrig + 1 }]; - } - // Replace (covers single-line changes, line-count-changing replacements, etc.) - return [{ action: 'replace', startLine: start + 1, endLine: endOrig + 1, content: newLines.slice(start, endNew + 1).join('\n') }]; -} +import type { EditFileLineOperationType, EditFileTextOperationType, PreviewEditOutputType } from './types'; +import { validateLineEdits } from './validateEdits'; /** * Convert an absolute file path to a display-friendly path relative to cwd @@ -60,80 +24,73 @@ function toDisplayPath(absolutePath: string): string { return resolved; } -/** - * Resolve any `replace_text` or `regex_text` operations in `edits` into equivalent - * line-based operations. All other operation types are passed through - * unchanged. Each replace_text edit is applied against the accumulated - * result of all previous replace_text edits so that multiple ops on the - * same file chain correctly. - */ -function resolveReplaceText(originalContent: string, edits: EditOperationType[]): ResolvedEditOperationType[] { - const explicitOps: ResolvedEditOperationType[] = []; - let currentContent = originalContent; +function lineKey(edit: EditFileLineOperationType): number { + return edit.action === 'insert' ? edit.after_line : edit.startLine; +} - for (const edit of edits) { - if (edit.action !== 'replace_text' && edit.action !== 'regex_text') { - explicitOps.push(edit); - continue; - } +function sortBottomToTop(edits: EditFileLineOperationType[]): EditFileLineOperationType[] { + return [...edits].sort((a, b) => lineKey(b) - lineKey(a)); +} +function applyTextEdits(content: string, edits: EditFileTextOperationType[]): string { + let current = content; + for (const edit of edits) { const pattern = edit.action === 'regex_text' ? edit.pattern : edit.oldString.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - - const matches = [...currentContent.matchAll(new RegExp(pattern, 'g'))]; - + const display = edit.action === 'regex_text' ? `pattern "${edit.pattern}"` : `"${edit.oldString}"`; + const matches = [...current.matchAll(new RegExp(pattern, 'g'))]; if (matches.length === 0) { - throw new Error(`${edit.action}: pattern "${pattern}" not found in file`); + throw new Error(`${edit.action}: ${display} not found in file`); } if (matches.length > 1 && !edit.replaceMultiple) { - throw new Error(`${edit.action}: pattern "${pattern}" matched ${matches.length} times — set replaceMultiple: true to replace all`); + throw new Error(`${edit.action}: ${display} matched ${matches.length} times \u2014 set replaceMultiple: true to replace all`); } - // replace_text: use a replacer function so $ in the replacement is never interpreted // specially by String.prototype.replace (which treats $$ $& $1 etc. as special patterns). // regex_text keeps the string form so $1, $&, $$ etc. work as documented. const replacer = edit.action === 'replace_text' ? () => edit.replacement : edit.replacement; - currentContent = currentContent.replace(new RegExp(pattern, edit.replaceMultiple ? 'g' : ''), replacer as string); - } - - if (currentContent !== originalContent) { - explicitOps.push(...findChangedRegions(originalContent.split('\n'), currentContent.split('\n'))); + current = current.replace(new RegExp(pattern, edit.replaceMultiple ? 'g' : ''), replacer as string); } - return explicitOps; + return current; } export function createPreviewEdit(fs: IFileSystem, store: Map) { return defineTool({ name: 'PreviewEdit', - description: 'Preview edits to a file. Returns a diff for review — does not write to disk.', + description: 'Preview edits to a file. Returns a diff for review \u2014 does not write to disk.', operation: 'read', input_schema: PreviewEditInputSchema, input_examples: [ { file: '/path/to/file.ts', - edits: [{ action: 'insert', after_line: 0, content: '// hello world' }], + lineEdits: [{ action: 'insert', after_line: 0, content: '// hello world' }], }, { file: '/path/to/file.ts', - edits: [{ action: 'replace', startLine: 5, endLine: 7, content: 'const x = 1;' }], + lineEdits: [{ action: 'replace', startLine: 5, endLine: 7, content: 'const x = 1;' }], }, { file: '/path/to/file.ts', - edits: [{ action: 'delete', startLine: 10, endLine: 12 }], + lineEdits: [{ action: 'delete', startLine: 10, endLine: 12 }], }, { file: '/path/to/file.ts', - edits: [ + lineEdits: [ { action: 'delete', startLine: 3, endLine: 3 }, { action: 'replace', startLine: 8, endLine: 9, content: 'export default foo;' }, ], }, { file: '/path/to/file.ts', - edits: [{ action: 'regex_text', pattern: 'import type \\{ (\\w+) \\}', replacement: 'import { $1 }' }], + textEdits: [{ action: 'regex_text', pattern: 'import type \\{ (\\w+) \\}', replacement: 'import { $1 }' }], + }, + { + file: '/path/to/file.ts', + textEdits: [{ action: 'replace_text', oldString: 'import type { MyClass }', replacement: 'import { MyClass }' }], }, { file: '/path/to/file.ts', - edits: [{ action: 'replace_text', oldString: 'import type { MyClass }', replacement: 'import { MyClass }' }], + lineEdits: [{ action: 'insert', after_line: 34, content: '\nfunction helper() {}' }], + textEdits: [{ action: 'replace_text', oldString: 'oldCall()', replacement: 'helper()' }], }, ], handler: async (input) => { @@ -144,21 +101,12 @@ export function createPreviewEdit(fs: IFileSystem, store: Map input.lineEdits.length > 0 || input.textEdits.length > 0, { + message: 'At least one edit must be provided (lineEdits or textEdits)', + }); export const PreviewEditOutputSchema = z.object({ patchId: z.uuid(), diff --git a/packages/claude-sdk-tools/src/EditFile/types.ts b/packages/claude-sdk-tools/src/EditFile/types.ts index 9fc2041..9e7c4ce 100644 --- a/packages/claude-sdk-tools/src/EditFile/types.ts +++ b/packages/claude-sdk-tools/src/EditFile/types.ts @@ -1,9 +1,10 @@ import type { z } from 'zod'; -import type { EditFileInputSchema, EditFileOperationSchema, EditFileOutputSchema, EditFileResolvedOperationSchema, PreviewEditInputSchema, PreviewEditOutputSchema } from './schema'; +import type { EditFileInputSchema, EditFileLineOperationSchema, EditFileOutputSchema, EditFileResolvedOperationSchema, EditFileTextOperationSchema, PreviewEditInputSchema, PreviewEditOutputSchema } from './schema'; export type PreviewEditInputType = z.infer; export type PreviewEditOutputType = z.infer; export type EditFileInputType = z.infer; export type EditFileOutputType = z.infer; -export type EditOperationType = z.infer; +export type EditFileLineOperationType = z.infer; +export type EditFileTextOperationType = z.infer; export type ResolvedEditOperationType = z.infer; diff --git a/packages/claude-sdk-tools/src/EditFile/validateEdits.ts b/packages/claude-sdk-tools/src/EditFile/validateEdits.ts index 3839353..185fde9 100644 --- a/packages/claude-sdk-tools/src/EditFile/validateEdits.ts +++ b/packages/claude-sdk-tools/src/EditFile/validateEdits.ts @@ -1,39 +1,40 @@ -import type { ResolvedEditOperationType } from './types'; +import type { EditFileLineOperationType } from './types'; -export function validateEdits(lines: string[], edits: ResolvedEditOperationType[]): void { - const getLines = (edit: ResolvedEditOperationType) => { - switch (edit.action) { - case 'insert': - case 'replace': { - return edit.content.split('\n').length; - } - case 'delete': { - return 0; - } - } - }; - - let currentLintCount = lines.length; +export function validateLineEdits(lines: string[], edits: EditFileLineOperationType[]): void { + const total = lines.length; for (const edit of edits) { - const lines = getLines(edit); - currentLintCount += lines; if (edit.action === 'insert') { - if (edit.after_line > currentLintCount) { - throw new Error(`insert after_line ${edit.after_line} out of bounds (file has ${currentLintCount} lines)`); + if (edit.after_line > total) { + throw new Error(`insert after_line ${edit.after_line} out of bounds (file has ${total} lines)`); } } else { - if (edit.startLine > currentLintCount) { - throw new Error(`${edit.action} startLine ${edit.startLine} out of bounds (file has ${currentLintCount} lines)`); + if (edit.startLine > total) { + throw new Error(`${edit.action} startLine ${edit.startLine} out of bounds (file has ${total} lines)`); } - if (edit.endLine > currentLintCount) { - throw new Error(`${edit.action} endLine ${edit.endLine} out of bounds (file has ${currentLintCount} lines)`); + if (edit.endLine > total) { + throw new Error(`${edit.action} endLine ${edit.endLine} out of bounds (file has ${total} lines)`); } if (edit.startLine > edit.endLine) { throw new Error(`${edit.action} startLine ${edit.startLine} is greater than endLine ${edit.endLine}`); } - const removed = edit.endLine - edit.startLine + 1; - currentLintCount -= removed; + } + } + + // All line numbers refer to the same original file, so overlapping ranges + // indicate conflicting edits that have no well-defined result. + const ranges = edits.map((e) => ({ + start: e.action === 'insert' ? e.after_line : e.startLine, + end: e.action === 'insert' ? e.after_line : e.endLine, + })); + + for (let i = 0; i < ranges.length; i++) { + for (let j = i + 1; j < ranges.length; j++) { + const a = ranges[i]; + const b = ranges[j]; + if (a != null && b != null && a.start <= b.end && b.start <= a.end) { + throw new Error(`line edits overlap: edit at ${a.start}\u2013${a.end} and edit at ${b.start}\u2013${b.end} target the same lines`); + } } } } diff --git a/packages/claude-sdk-tools/src/Ref/Ref.ts b/packages/claude-sdk-tools/src/Ref/Ref.ts index f2354b3..4caf506 100644 --- a/packages/claude-sdk-tools/src/Ref/Ref.ts +++ b/packages/claude-sdk-tools/src/Ref/Ref.ts @@ -13,7 +13,7 @@ export type CreateRefResult = { export function createRef(store: RefStore, threshold: number): CreateRefResult { const tool = defineTool({ name: 'Ref', - description: `Fetch the content of a stored ref. When a tool result contains { ref, size, hint } instead of the full value, use this tool to retrieve it. Returns at most \`limit\` characters starting at \`start\`. Both default (start=0, limit=1000) so a bare { id } call gives the first 1000 chars — safe for arbitrarily large refs. The response includes \`hint\` (what produced the ref), \`totalSize\`, and the slice bounds so you know whether to page further.`, + description: `Fetch the content of a stored ref. When a tool result contains { ref, size, hint } instead of the full value, use this tool to retrieve it. Returns at most \`limit\` characters starting at \`start\`. Both default (start=0, limit=10000) so a bare { id } call gives the first 10000 chars — safe for arbitrarily large refs. The response includes \`hint\` (what produced the ref), \`totalSize\`, and the slice bounds so you know whether to page further.`, input_schema: RefInputSchema, input_examples: [{ id: 'uuid-...' }, { id: 'uuid-...', start: 1000, limit: 1000 }], handler: async (input): Promise => { diff --git a/packages/claude-sdk-tools/src/Ref/schema.ts b/packages/claude-sdk-tools/src/Ref/schema.ts index 5f176db..da548df 100644 --- a/packages/claude-sdk-tools/src/Ref/schema.ts +++ b/packages/claude-sdk-tools/src/Ref/schema.ts @@ -3,5 +3,5 @@ import { z } from 'zod'; export const RefInputSchema = z.object({ id: z.string().describe('The ref ID returned in a { ref, size, hint } token.'), start: z.number().int().min(0).default(0).describe('Start character offset (inclusive). Default 0.'), - limit: z.number().int().min(1).max(2000).default(1000).describe('Maximum number of characters to return. Max 2000, default 1000. Use start+limit to page through large refs.'), + limit: z.number().int().min(1).max(20_000).default(10_000).describe('Maximum number of characters to return. Max 20000, default 10000. Use start+limit to page through large refs.'), }); diff --git a/packages/claude-sdk-tools/test/EditFile.spec.ts b/packages/claude-sdk-tools/test/EditFile.spec.ts index 83b94aa..26e540c 100644 --- a/packages/claude-sdk-tools/test/EditFile.spec.ts +++ b/packages/claude-sdk-tools/test/EditFile.spec.ts @@ -6,11 +6,11 @@ import { call } from './helpers'; const originalContent = 'line one\nline two\nline three'; -describe('createPreviewEdit \u2014 staging', () => { +describe('createPreviewEdit — staging', () => { it('stores a patch in the store and returns a patchId', async () => { const fs = new MemoryFileSystem({ '/file.ts': originalContent }); const { previewEdit } = createEditFilePair(fs); - const result = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'insert', after_line: 0, content: '// header' }] }); + const result = await call(previewEdit, { file: '/file.ts', lineEdits: [{ action: 'insert', after_line: 0, content: '// header' }] }); expect(result).toMatchObject({ file: '/file.ts' }); expect(typeof result.patchId).toBe('string'); }); @@ -18,7 +18,7 @@ describe('createPreviewEdit \u2014 staging', () => { it('computes the correct originalHash', async () => { const fs = new MemoryFileSystem({ '/file.ts': originalContent }); const { previewEdit } = createEditFilePair(fs); - const result = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'delete', startLine: 1, endLine: 1 }] }); + const result = await call(previewEdit, { file: '/file.ts', lineEdits: [{ action: 'delete', startLine: 1, endLine: 1 }] }); const expected = createHash('sha256').update(originalContent).digest('hex'); expect(result.originalHash).toBe(expected); }); @@ -26,7 +26,7 @@ describe('createPreviewEdit \u2014 staging', () => { it('includes a unified diff', async () => { const fs = new MemoryFileSystem({ '/file.ts': originalContent }); const { previewEdit } = createEditFilePair(fs); - const result = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'replace', startLine: 2, endLine: 2, content: 'line TWO' }] }); + const result = await call(previewEdit, { file: '/file.ts', lineEdits: [{ action: 'replace', startLine: 2, endLine: 2, content: 'line TWO' }] }); expect(result.diff).toContain('line two'); expect(result.diff).toContain('line TWO'); }); @@ -34,32 +34,31 @@ describe('createPreviewEdit \u2014 staging', () => { it('diff includes context lines around the change', async () => { const fs = new MemoryFileSystem({ '/file.ts': originalContent }); const { previewEdit } = createEditFilePair(fs); - // originalContent = 'line one\nline two\nline three'; edit middle line only - const result = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'replace', startLine: 2, endLine: 2, content: 'line TWO' }] }); - expect(result.diff).toContain(' line one'); // unchanged line before — space-prefixed context + const result = await call(previewEdit, { file: '/file.ts', lineEdits: [{ action: 'replace', startLine: 2, endLine: 2, content: 'line TWO' }] }); + expect(result.diff).toContain(' line one'); // unchanged line before — space-prefixed context expect(result.diff).toContain(' line three'); // unchanged line after — space-prefixed context }); it('diff contains a standard @@ hunk header', async () => { const fs = new MemoryFileSystem({ '/file.ts': originalContent }); const { previewEdit } = createEditFilePair(fs); - const result = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'replace', startLine: 2, endLine: 2, content: 'line TWO' }] }); + const result = await call(previewEdit, { file: '/file.ts', lineEdits: [{ action: 'replace', startLine: 2, endLine: 2, content: 'line TWO' }] }); expect(result.diff).toMatch(/@@ -\d+,\d+ \+\d+,\d+ @@/); }); it('expands ~ in file path', async () => { const fs = new MemoryFileSystem({ '/home/testuser/file.ts': originalContent }, '/home/testuser'); const { previewEdit } = createEditFilePair(fs); - const result = await call(previewEdit, { file: '~/file.ts', edits: [{ action: 'delete', startLine: 1, endLine: 1 }] }); + const result = await call(previewEdit, { file: '~/file.ts', lineEdits: [{ action: 'delete', startLine: 1, endLine: 1 }] }); expect(result.file).toBe('/home/testuser/file.ts'); }); }); -describe('createEditFile \u2014 applying', () => { +describe('createEditFile — applying', () => { it('applies the patch and writes the new content', async () => { const fs = new MemoryFileSystem({ '/file.ts': originalContent }); const { previewEdit, editFile } = createEditFilePair(fs); - const staged = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'replace', startLine: 1, endLine: 1, content: 'line ONE' }] }); + const staged = await call(previewEdit, { file: '/file.ts', lineEdits: [{ action: 'replace', startLine: 1, endLine: 1, content: 'line ONE' }] }); const confirmed = await call(editFile, { patchId: staged.patchId, file: staged.file }); expect(confirmed).toMatchObject({ linesAdded: 1, linesRemoved: 1 }); expect(await fs.readFile('/file.ts')).toBe('line ONE\nline two\nline three'); @@ -68,7 +67,7 @@ describe('createEditFile \u2014 applying', () => { it('throws when the file was modified after staging', async () => { const fs = new MemoryFileSystem({ '/file.ts': originalContent }); const { previewEdit, editFile } = createEditFilePair(fs); - const staged = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'delete', startLine: 1, endLine: 1 }] }); + const staged = await call(previewEdit, { file: '/file.ts', lineEdits: [{ action: 'delete', startLine: 1, endLine: 1 }] }); await fs.writeFile('/file.ts', 'completely different content'); await expect(call(editFile, { patchId: staged.patchId, file: staged.file })).rejects.toThrow('has been modified since the edit was staged'); }); @@ -84,56 +83,56 @@ describe('regex_text action', () => { it('replaces a unique match', async () => { const fs = new MemoryFileSystem({ '/file.ts': 'line one\nline two\nline three' }); const { previewEdit } = createEditFilePair(fs); - const result = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'regex_text', pattern: 'line two', replacement: 'line TWO' }] }); + const result = await call(previewEdit, { file: '/file.ts', textEdits: [{ action: 'regex_text', pattern: 'line two', replacement: 'line TWO' }] }); expect(result.newContent).toBe('line one\nline TWO\nline three'); }); it('replaces a substring within a line, not the whole line', async () => { const fs = new MemoryFileSystem({ '/file.ts': "const x: string = 'hello';" }); const { previewEdit } = createEditFilePair(fs); - const result = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'regex_text', pattern: ': string', replacement: '' }] }); + const result = await call(previewEdit, { file: '/file.ts', textEdits: [{ action: 'regex_text', pattern: ': string', replacement: '' }] }); expect(result.newContent).toBe("const x = 'hello';"); }); it('find is treated as a regex pattern', async () => { const fs = new MemoryFileSystem({ '/file.ts': 'version: 42' }); const { previewEdit } = createEditFilePair(fs); - const result = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'regex_text', pattern: '\\d+', replacement: '99' }] }); + const result = await call(previewEdit, { file: '/file.ts', textEdits: [{ action: 'regex_text', pattern: '\\d+', replacement: '99' }] }); expect(result.newContent).toBe('version: 99'); }); it('supports capture groups in replacement', async () => { const fs = new MemoryFileSystem({ '/file.ts': "import type { MyType } from 'types';" }); const { previewEdit } = createEditFilePair(fs); - const result = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'regex_text', pattern: 'import type \\{ (\\w+) \\}', replacement: 'import { $1 }' }] }); + const result = await call(previewEdit, { file: '/file.ts', textEdits: [{ action: 'regex_text', pattern: 'import type \\{ (\\w+) \\}', replacement: 'import { $1 }' }] }); expect(result.newContent).toBe("import { MyType } from 'types';"); }); it('$& in replacement inserts the matched text', async () => { const fs = new MemoryFileSystem({ '/file.ts': 'hello world' }); const { previewEdit } = createEditFilePair(fs); - const result = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'regex_text', pattern: 'world', replacement: '[$&]' }] }); + const result = await call(previewEdit, { file: '/file.ts', textEdits: [{ action: 'regex_text', pattern: 'world', replacement: '[$&]' }] }); expect(result.newContent).toBe('hello [world]'); }); it('$$ in replacement produces a literal dollar sign', async () => { const fs = new MemoryFileSystem({ '/file.ts': 'cost is 100' }); const { previewEdit } = createEditFilePair(fs); - const result = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'regex_text', pattern: '100', replacement: '$$100' }] }); + const result = await call(previewEdit, { file: '/file.ts', textEdits: [{ action: 'regex_text', pattern: '100', replacement: '$$100' }] }); expect(result.newContent).toBe('cost is $100'); }); it('matches across multiple lines', async () => { const fs = new MemoryFileSystem({ '/file.ts': 'line one\nline two\nline three' }); const { previewEdit } = createEditFilePair(fs); - const result = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'regex_text', pattern: 'line one\\nline two', replacement: 'LINES ONE AND TWO' }] }); + const result = await call(previewEdit, { file: '/file.ts', textEdits: [{ action: 'regex_text', pattern: 'line one\\nline two', replacement: 'LINES ONE AND TWO' }] }); expect(result.newContent).toBe('LINES ONE AND TWO\nline three'); }); it('includes the old and new text in the diff', async () => { const fs = new MemoryFileSystem({ '/file.ts': 'line one\nline two\nline three' }); const { previewEdit } = createEditFilePair(fs); - const result = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'regex_text', pattern: 'line two', replacement: 'line TWO' }] }); + const result = await call(previewEdit, { file: '/file.ts', textEdits: [{ action: 'regex_text', pattern: 'line two', replacement: 'line TWO' }] }); expect(result.diff).toContain('line two'); expect(result.diff).toContain('line TWO'); }); @@ -141,7 +140,7 @@ describe('regex_text action', () => { it('confirmed edit writes the correct content to disk', async () => { const fs = new MemoryFileSystem({ '/file.ts': 'line one\nline two\nline three' }); const { previewEdit, editFile } = createEditFilePair(fs); - const staged = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'regex_text', pattern: 'line two', replacement: 'line TWO' }] }); + const staged = await call(previewEdit, { file: '/file.ts', textEdits: [{ action: 'regex_text', pattern: 'line two', replacement: 'line TWO' }] }); await call(editFile, { patchId: staged.patchId, file: staged.file }); expect(await fs.readFile('/file.ts')).toBe('line one\nline TWO\nline three'); }); @@ -149,26 +148,26 @@ describe('regex_text action', () => { it('throws when the pattern matches nothing', async () => { const fs = new MemoryFileSystem({ '/file.ts': 'line one\nline two\nline three' }); const { previewEdit } = createEditFilePair(fs); - await expect(call(previewEdit, { file: '/file.ts', edits: [{ action: 'regex_text', pattern: 'not in file', replacement: 'x' }] })).rejects.toThrow(); + await expect(call(previewEdit, { file: '/file.ts', textEdits: [{ action: 'regex_text', pattern: 'not in file', replacement: 'x' }] })).rejects.toThrow(); }); it('throws when the pattern matches multiple times and replaceMultiple is not set', async () => { const fs = new MemoryFileSystem({ '/file.ts': 'foo\nfoo\nbar' }); const { previewEdit } = createEditFilePair(fs); - await expect(call(previewEdit, { file: '/file.ts', edits: [{ action: 'regex_text', pattern: 'foo', replacement: 'baz' }] })).rejects.toThrow('2'); + await expect(call(previewEdit, { file: '/file.ts', textEdits: [{ action: 'regex_text', pattern: 'foo', replacement: 'baz' }] })).rejects.toThrow('2'); }); it('replaces all occurrences across lines when replaceMultiple is true', async () => { const fs = new MemoryFileSystem({ '/file.ts': 'foo\nfoo\nbar' }); const { previewEdit } = createEditFilePair(fs); - const result = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'regex_text', pattern: 'foo', replacement: 'baz', replaceMultiple: true }] }); + const result = await call(previewEdit, { file: '/file.ts', textEdits: [{ action: 'regex_text', pattern: 'foo', replacement: 'baz', replaceMultiple: true }] }); expect(result.newContent).toBe('baz\nbaz\nbar'); }); it('replaces all occurrences on the same line when replaceMultiple is true', async () => { const fs = new MemoryFileSystem({ '/file.ts': 'foo foo\nbar' }); const { previewEdit } = createEditFilePair(fs); - const result = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'regex_text', pattern: 'foo', replacement: 'baz', replaceMultiple: true }] }); + const result = await call(previewEdit, { file: '/file.ts', textEdits: [{ action: 'regex_text', pattern: 'foo', replacement: 'baz', replaceMultiple: true }] }); expect(result.newContent).toBe('baz baz\nbar'); }); }); @@ -177,170 +176,204 @@ describe('replace_text action', () => { it('replaces a unique literal string match', async () => { const fs = new MemoryFileSystem({ '/file.ts': 'line one\nline two\nline three' }); const { previewEdit } = createEditFilePair(fs); - const result = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'replace_text', oldString: 'line two', replacement: 'line TWO' }] }); + const result = await call(previewEdit, { file: '/file.ts', textEdits: [{ action: 'replace_text', oldString: 'line two', replacement: 'line TWO' }] }); expect(result.newContent).toBe('line one\nline TWO\nline three'); }); it('treats special regex chars in oldString as literals', async () => { const fs = new MemoryFileSystem({ '/file.ts': 'price: (100)' }); const { previewEdit } = createEditFilePair(fs); - const result = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'replace_text', oldString: '(100)', replacement: '(200)' }] }); + const result = await call(previewEdit, { file: '/file.ts', textEdits: [{ action: 'replace_text', oldString: '(100)', replacement: '(200)' }] }); expect(result.newContent).toBe('price: (200)'); }); it('treats $ in replacement as a literal dollar sign, not a special pattern', async () => { const fs = new MemoryFileSystem({ '/file.ts': 'cost: 100' }); const { previewEdit } = createEditFilePair(fs); - const result = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'replace_text', oldString: '100', replacement: '$100' }] }); + const result = await call(previewEdit, { file: '/file.ts', textEdits: [{ action: 'replace_text', oldString: '100', replacement: '$100' }] }); expect(result.newContent).toBe('cost: $100'); }); it('$$ in replacement produces two dollar signs, not one', async () => { const fs = new MemoryFileSystem({ '/file.ts': 'x' }); const { previewEdit } = createEditFilePair(fs); - const result = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'replace_text', oldString: 'x', replacement: '$$' }] }); + const result = await call(previewEdit, { file: '/file.ts', textEdits: [{ action: 'replace_text', oldString: 'x', replacement: '$$' }] }); expect(result.newContent).toBe('$$'); }); it('$& in replacement is literal, not the matched text', async () => { const fs = new MemoryFileSystem({ '/file.ts': 'hello world' }); const { previewEdit } = createEditFilePair(fs); - const result = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'replace_text', oldString: 'world', replacement: '$&' }] }); + const result = await call(previewEdit, { file: '/file.ts', textEdits: [{ action: 'replace_text', oldString: 'world', replacement: '$&' }] }); expect(result.newContent).toBe('hello $&'); }); it('throws when oldString is not found', async () => { const fs = new MemoryFileSystem({ '/file.ts': 'line one' }); const { previewEdit } = createEditFilePair(fs); - await expect(call(previewEdit, { file: '/file.ts', edits: [{ action: 'replace_text', oldString: 'not here', replacement: 'x' }] })).rejects.toThrow(); + await expect(call(previewEdit, { file: '/file.ts', textEdits: [{ action: 'replace_text', oldString: 'not here', replacement: 'x' }] })).rejects.toThrow(); }); it('replaces all occurrences when replaceMultiple is true', async () => { const fs = new MemoryFileSystem({ '/file.ts': 'foo\nfoo\nbar' }); const { previewEdit } = createEditFilePair(fs); - const result = await call(previewEdit, { file: '/file.ts', edits: [{ action: 'replace_text', oldString: 'foo', replacement: 'baz', replaceMultiple: true }] }); + const result = await call(previewEdit, { file: '/file.ts', textEdits: [{ action: 'replace_text', oldString: 'foo', replacement: 'baz', replaceMultiple: true }] }); expect(result.newContent).toBe('baz\nbaz\nbar'); }); }); -describe('multiple edits — sequential semantics', () => { - // Edits are applied in order, top-to-bottom. - // Each edit's line numbers reference the file *as it looks after all previous edits*, - // not the original file. +describe('lineEdits — bottom-to-top semantics', () => { + // All line numbers reference the file as it exists before the call. + // The tool sorts edits bottom-to-top internally before applying them, + // so earlier edits never shift the line numbers of later ones. - it('delete then replace: second edit uses post-delete line numbers', async () => { - // The user's example: delete lines 5–7 from a 10-line file, - // then the original lines 9–10 are now at positions 6–7. - const content = '1\n2\n3\n4\n5\n6\n7\n8\n9\n10'; - const fs = new MemoryFileSystem({ '/file.ts': content }); + it('two non-overlapping replaces both reference original line numbers', async () => { + // A=1, B=2, C=3, D=4, E=5 + const fs = new MemoryFileSystem({ '/file.ts': 'A\nB\nC\nD\nE' }); const { previewEdit } = createEditFilePair(fs); const result = await call(previewEdit, { file: '/file.ts', - edits: [ - { action: 'delete', startLine: 5, endLine: 7 }, // removes 5,6,7 → [1,2,3,4,8,9,10] - { action: 'replace', startLine: 6, endLine: 7, content: 'nine\nten' }, // 9,10 are now at 6,7 + lineEdits: [ + { action: 'replace', startLine: 2, endLine: 2, content: 'XX' }, // B → XX (original line 2) + { action: 'replace', startLine: 4, endLine: 4, content: 'YY' }, // D → YY (original line 4) ], }); - expect(result.newContent).toBe('1\n2\n3\n4\n8\nnine\nten'); + expect(result.newContent).toBe('A\nXX\nC\nYY\nE'); }); - it('insert shifts subsequent edits: second edit uses post-insert line numbers', async () => { - // Insert a line after line 1 → original line 2 is now at line 3. - const fs = new MemoryFileSystem({ '/file.ts': 'line one\nline two\nline three' }); + it('specification order does not affect the result', async () => { + // Same edits as above but reversed — must produce identical output. + const fs = new MemoryFileSystem({ '/file.ts': 'A\nB\nC\nD\nE' }); const { previewEdit } = createEditFilePair(fs); const result = await call(previewEdit, { file: '/file.ts', - edits: [ - { action: 'insert', after_line: 1, content: 'inserted' }, // → [line one, inserted, line two, line three] - { action: 'replace', startLine: 3, endLine: 3, content: 'line TWO' }, // line two is now at 3 + lineEdits: [ + { action: 'replace', startLine: 4, endLine: 4, content: 'YY' }, + { action: 'replace', startLine: 2, endLine: 2, content: 'XX' }, ], }); - expect(result.newContent).toBe('line one\ninserted\nline TWO\nline three'); + expect(result.newContent).toBe('A\nXX\nC\nYY\nE'); }); - it('two consecutive deletes at the same position both use current state', async () => { - // Delete line 2 twice: first removes B (line 2), second removes C (now line 2). - const fs = new MemoryFileSystem({ '/file.ts': 'A\nB\nC\nD\nE' }); + it('insert and replace both reference the original file, not each other', async () => { + // Original: line one(1) / line two(2) / line three(3) + // insert after_line 1 adds a line between line one and line two. + // replace line 3 replaces line three — line 3 in the ORIGINAL, not post-insert. + const fs = new MemoryFileSystem({ '/file.ts': 'line one\nline two\nline three' }); const { previewEdit } = createEditFilePair(fs); const result = await call(previewEdit, { file: '/file.ts', - edits: [ - { action: 'delete', startLine: 2, endLine: 2 }, // removes B → [A,C,D,E] - { action: 'delete', startLine: 2, endLine: 2 }, // removes C (now line 2) → [A,D,E] + lineEdits: [ + { action: 'insert', after_line: 1, content: 'inserted' }, + { action: 'replace', startLine: 3, endLine: 3, content: 'line THREE' }, ], }); - expect(result.newContent).toBe('A\nD\nE'); + expect(result.newContent).toBe('line one\ninserted\nline two\nline THREE'); }); - it('two inserts in sequence: second insert references post-first-insert line numbers', async () => { - // Insert X after line 1, then insert Y after line 2 (where X now is). + it('two inserts at different positions both reference the original file', async () => { + // Original: A(1) B(2) C(3) + // Insert after 1 → between A and B + // Insert after 3 → after C + // Both after_line values are from the original. const fs = new MemoryFileSystem({ '/file.ts': 'A\nB\nC' }); const { previewEdit } = createEditFilePair(fs); const result = await call(previewEdit, { file: '/file.ts', - edits: [ - { action: 'insert', after_line: 1, content: 'X' }, // → [A, X, B, C] - { action: 'insert', after_line: 2, content: 'Y' }, // after X (now line 2) → [A, X, Y, B, C] + lineEdits: [ + { action: 'insert', after_line: 1, content: 'after-A' }, + { action: 'insert', after_line: 3, content: 'after-C' }, ], }); - expect(result.newContent).toBe('A\nX\nY\nB\nC'); + expect(result.newContent).toBe('A\nafter-A\nB\nC\nafter-C'); }); - it('replace expanding lines shifts subsequent edits down', async () => { - // Replace B (line 2) with 3 lines → C shifts from line 3 to line 5. + it('delete and replace on non-overlapping lines both reference original', async () => { + // Delete line 2, replace line 4 — both from original. const fs = new MemoryFileSystem({ '/file.ts': 'A\nB\nC\nD\nE' }); const { previewEdit } = createEditFilePair(fs); const result = await call(previewEdit, { file: '/file.ts', - edits: [ - { action: 'replace', startLine: 2, endLine: 2, content: 'B1\nB2\nB3' }, // → [A,B1,B2,B3,C,D,E] - { action: 'replace', startLine: 5, endLine: 5, content: 'X' }, // C is now at line 5 + lineEdits: [ + { action: 'delete', startLine: 2, endLine: 2 }, + { action: 'replace', startLine: 4, endLine: 4, content: 'DD' }, ], }); - expect(result.newContent).toBe('A\nB1\nB2\nB3\nX\nD\nE'); + expect(result.newContent).toBe('A\nC\nDD\nE'); }); - it('replace shrinking lines shifts subsequent edits up', async () => { - // Replace lines 1–3 with a single line → D shifts from line 4 to line 2. + it('throws when two edits target overlapping lines', async () => { const fs = new MemoryFileSystem({ '/file.ts': 'A\nB\nC\nD\nE' }); const { previewEdit } = createEditFilePair(fs); + await expect( + call(previewEdit, { + file: '/file.ts', + lineEdits: [ + { action: 'replace', startLine: 2, endLine: 3, content: 'X' }, + { action: 'replace', startLine: 3, endLine: 4, content: 'Y' }, + ], + }), + ).rejects.toThrow(); + }); + + it('throws when a line number is beyond the end of the file', async () => { + const fs = new MemoryFileSystem({ '/file.ts': 'A\nB\nC' }); // 3 lines + const { previewEdit } = createEditFilePair(fs); + await expect( + call(previewEdit, { + file: '/file.ts', + lineEdits: [{ action: 'replace', startLine: 5, endLine: 5, content: 'X' }], + }), + ).rejects.toThrow('out of bounds'); + }); +}); + +describe('lineEdits + textEdits — combined', () => { + // textEdits are applied after all lineEdits complete. + // They search the post-lineEdit content, not the original. + + it('textEdits see the content after lineEdits are applied', async () => { + // This is the key regression test for the bug that motivated the redesign. + // Previously, combining an insert + replace_text in one call would land the + // replacement N lines too early because text ops ran against the original positions. + // + // Original: line one / line two / line three + // lineEdit: insert '// generated' before line 1 (after_line: 0) + // textEdit: replace 'line two' → 'line TWO' + // Expected: // generated / line one / line TWO / line three + const fs = new MemoryFileSystem({ '/file.ts': 'line one\nline two\nline three' }); + const { previewEdit } = createEditFilePair(fs); const result = await call(previewEdit, { file: '/file.ts', - edits: [ - { action: 'replace', startLine: 1, endLine: 3, content: 'ABC' }, // → [ABC, D, E] - { action: 'replace', startLine: 2, endLine: 2, content: 'X' }, // D is now at line 2 - ], + lineEdits: [{ action: 'insert', after_line: 0, content: '// generated' }], + textEdits: [{ action: 'replace_text', oldString: 'line two', replacement: 'line TWO' }], }); - expect(result.newContent).toBe('ABC\nX\nE'); + expect(result.newContent).toBe('// generated\nline one\nline TWO\nline three'); }); - it('can reference a line that was added by a previous insert', async () => { - // insert expands the file beyond its original length; the second edit must be valid - const fs = new MemoryFileSystem({ '/file.ts': 'A\nB\nC' }); + it('multiple textEdits are applied in order after all lineEdits', async () => { + const fs = new MemoryFileSystem({ '/file.ts': 'foo\nbar\nbaz' }); const { previewEdit } = createEditFilePair(fs); const result = await call(previewEdit, { file: '/file.ts', - edits: [ - { action: 'insert', after_line: 3, content: 'D\nE' }, // → [A,B,C,D,E] - { action: 'replace', startLine: 4, endLine: 5, content: 'X\nY' }, // line 4,5 only exist post-insert + lineEdits: [{ action: 'replace', startLine: 1, endLine: 1, content: 'FOO' }], + textEdits: [ + { action: 'replace_text', oldString: 'bar', replacement: 'BAR' }, + { action: 'replace_text', oldString: 'baz', replacement: 'BAZ' }, ], }); - expect(result.newContent).toBe('A\nB\nC\nX\nY'); + expect(result.newContent).toBe('FOO\nBAR\nBAZ'); }); - it('throws when a subsequent edit references a line removed by a previous delete', async () => { - // delete shrinks the file; the second edit references a line that no longer exists - const fs = new MemoryFileSystem({ '/file.ts': 'A\nB\nC\nD\nE' }); + it('textEdit can match a string that only exists after the lineEdit inserts it', async () => { + const fs = new MemoryFileSystem({ '/file.ts': 'hello world' }); const { previewEdit } = createEditFilePair(fs); - await expect( - call(previewEdit, { - file: '/file.ts', - edits: [ - { action: 'delete', startLine: 1, endLine: 4 }, // → [E] (1 line left) - { action: 'replace', startLine: 3, endLine: 3, content: 'X' }, // line 3 no longer exists - ], - }), - ).rejects.toThrow('out of bounds'); + const result = await call(previewEdit, { + file: '/file.ts', + lineEdits: [{ action: 'insert', after_line: 1, content: 'inserted line' }], + textEdits: [{ action: 'replace_text', oldString: 'inserted line', replacement: 'INSERTED LINE' }], + }); + expect(result.newContent).toBe('hello world\nINSERTED LINE'); }); }); @@ -350,11 +383,11 @@ describe('chained previews — previousPatchId', () => { const { previewEdit } = createEditFilePair(fs); const patch1 = await call(previewEdit, { file: '/file.ts', - edits: [{ action: 'replace_text', oldString: 'line two', replacement: 'LINE TWO' }], + textEdits: [{ action: 'replace_text', oldString: 'line two', replacement: 'LINE TWO' }], }); const patch2 = await call(previewEdit, { file: '/file.ts', - edits: [{ action: 'replace_text', oldString: 'line three', replacement: 'LINE THREE' }], + textEdits: [{ action: 'replace_text', oldString: 'line three', replacement: 'LINE THREE' }], previousPatchId: patch1.patchId, }); expect(patch2.newContent).toBe('line one\nLINE TWO\nLINE THREE'); @@ -366,11 +399,11 @@ describe('chained previews — previousPatchId', () => { const { previewEdit } = createEditFilePair(fs); const patch1 = await call(previewEdit, { file: '/file.ts', - edits: [{ action: 'replace_text', oldString: 'line one', replacement: 'LINE ONE' }], + textEdits: [{ action: 'replace_text', oldString: 'line one', replacement: 'LINE ONE' }], }); const patch2 = await call(previewEdit, { file: '/file.ts', - edits: [{ action: 'replace_text', oldString: 'line two', replacement: 'LINE TWO' }], + textEdits: [{ action: 'replace_text', oldString: 'line two', replacement: 'LINE TWO' }], previousPatchId: patch1.patchId, }); expect(patch2.originalHash).toBe(patch1.originalHash); @@ -381,15 +414,14 @@ describe('chained previews — previousPatchId', () => { const { previewEdit } = createEditFilePair(fs); const patch1 = await call(previewEdit, { file: '/file.ts', - edits: [{ action: 'replace_text', oldString: 'line one', replacement: 'LINE ONE' }], + textEdits: [{ action: 'replace_text', oldString: 'line one', replacement: 'LINE ONE' }], }); const patch2 = await call(previewEdit, { file: '/file.ts', - edits: [{ action: 'replace_text', oldString: 'line three', replacement: 'LINE THREE' }], + textEdits: [{ action: 'replace_text', oldString: 'line three', replacement: 'LINE THREE' }], previousPatchId: patch1.patchId, }); - // patch2 diff should not show line one as a *changed* line (it's already settled in patch1) - // It may appear as context (space-prefixed), but must not appear as + or - lines + // patch2 diff should not show line one as a changed line (it's already settled in patch1) const changedLines = patch2.diff.split('\n').filter((l) => l.startsWith('+') || l.startsWith('-')); expect(changedLines.join('\n')).not.toContain('line one'); expect(changedLines.join('\n')).not.toContain('LINE ONE'); @@ -403,11 +435,11 @@ describe('chained previews — previousPatchId', () => { const { previewEdit, editFile } = createEditFilePair(fs); const patch1 = await call(previewEdit, { file: '/file.ts', - edits: [{ action: 'replace_text', oldString: 'line one', replacement: 'LINE ONE' }], + textEdits: [{ action: 'replace_text', oldString: 'line one', replacement: 'LINE ONE' }], }); const patch2 = await call(previewEdit, { file: '/file.ts', - edits: [{ action: 'replace_text', oldString: 'line two', replacement: 'LINE TWO' }], + textEdits: [{ action: 'replace_text', oldString: 'line two', replacement: 'LINE TWO' }], previousPatchId: patch1.patchId, }); await call(editFile, { patchId: patch2.patchId, file: patch2.file }); @@ -419,12 +451,12 @@ describe('chained previews — previousPatchId', () => { const { previewEdit, editFile } = createEditFilePair(fs); const patch1 = await call(previewEdit, { file: '/file.ts', - edits: [{ action: 'replace_text', oldString: 'line one', replacement: 'LINE ONE' }], + textEdits: [{ action: 'replace_text', oldString: 'line one', replacement: 'LINE ONE' }], }); // build patch2 but don't apply it await call(previewEdit, { file: '/file.ts', - edits: [{ action: 'replace_text', oldString: 'line two', replacement: 'LINE TWO' }], + textEdits: [{ action: 'replace_text', oldString: 'line two', replacement: 'LINE TWO' }], previousPatchId: patch1.patchId, }); // apply only patch1 @@ -438,7 +470,7 @@ describe('chained previews — previousPatchId', () => { await expect( call(previewEdit, { file: '/file.ts', - edits: [{ action: 'replace_text', oldString: 'hello', replacement: 'world' }], + textEdits: [{ action: 'replace_text', oldString: 'hello', replacement: 'world' }], previousPatchId: '00000000-0000-4000-8000-000000000000', }), ).rejects.toThrow('Previous patch not found'); @@ -449,12 +481,12 @@ describe('chained previews — previousPatchId', () => { const { previewEdit } = createEditFilePair(fs); const patch1 = await call(previewEdit, { file: '/a.ts', - edits: [{ action: 'replace_text', oldString: 'hello', replacement: 'HELLO' }], + textEdits: [{ action: 'replace_text', oldString: 'hello', replacement: 'HELLO' }], }); await expect( call(previewEdit, { file: '/b.ts', - edits: [{ action: 'replace_text', oldString: 'world', replacement: 'WORLD' }], + textEdits: [{ action: 'replace_text', oldString: 'world', replacement: 'WORLD' }], previousPatchId: patch1.patchId, }), ).rejects.toThrow('File mismatch'); diff --git a/packages/claude-sdk-tools/test/Ref.spec.ts b/packages/claude-sdk-tools/test/Ref.spec.ts index 1149846..7b0989a 100644 --- a/packages/claude-sdk-tools/test/Ref.spec.ts +++ b/packages/claude-sdk-tools/test/Ref.spec.ts @@ -17,7 +17,7 @@ describe('createRef — full fetch', () => { const { store, ids } = makeStore({ a: 'hello world' }); const { tool: Ref } = createRef(store, 1000); const result = await call(Ref, { id: ids.a }); - // default start=0, limit=1000 — content is 11 chars so end clamps to 11 + // default start=0, limit=10000 — content is 11 chars so end clamps to 11 expect(result).toMatchObject({ found: true, content: 'hello world', totalSize: 11, start: 0, end: 11 }); }); @@ -56,19 +56,19 @@ describe('createRef — slicing', () => { const { store, ids } = makeStore({ a: 'abcdef' }); const { tool: Ref } = createRef(store, 1000); const result = await call(Ref, { id: ids.a, start: 3 }); - // limit defaults to 1000; content is 6 chars so end clamps to 6 + // limit defaults to 10000; content is 6 chars so end clamps to 6 expect(result).toMatchObject({ found: true, content: 'def', totalSize: 6, start: 3, end: 6 }); }); - it('default start=0, limit=1000 never dumps the whole ref for large content', async () => { - const bigContent = 'x'.repeat(5000); + it('default start=0, limit=10000 never dumps the whole ref for large content', async () => { + const bigContent = 'x'.repeat(15000); const store = new RefStore(); const id = store.store(bigContent); const { tool: Ref } = createRef(store, 10); const result = (await call(Ref, { id })) as { found: boolean; content: string; end: number }; expect(result.found).toBe(true); - expect(result.content.length).toBe(1000); - expect(result.end).toBe(1000); + expect(result.content.length).toBe(10000); + expect(result.end).toBe(10000); }); }); From 4b0cacb53ae7a06bf187f9b50765e139a6b1be58 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Tue, 7 Apr 2026 23:51:50 +1000 Subject: [PATCH 2/3] Fix biome trailing-space flag in EditFile test comment --- packages/claude-sdk-tools/test/EditFile.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/claude-sdk-tools/test/EditFile.spec.ts b/packages/claude-sdk-tools/test/EditFile.spec.ts index 26e540c..7a9b444 100644 --- a/packages/claude-sdk-tools/test/EditFile.spec.ts +++ b/packages/claude-sdk-tools/test/EditFile.spec.ts @@ -35,7 +35,7 @@ describe('createPreviewEdit — staging', () => { const fs = new MemoryFileSystem({ '/file.ts': originalContent }); const { previewEdit } = createEditFilePair(fs); const result = await call(previewEdit, { file: '/file.ts', lineEdits: [{ action: 'replace', startLine: 2, endLine: 2, content: 'line TWO' }] }); - expect(result.diff).toContain(' line one'); // unchanged line before — space-prefixed context + expect(result.diff).toContain(' line one'); // unchanged line before — space-prefixed context expect(result.diff).toContain(' line three'); // unchanged line after — space-prefixed context }); From 95a745a308bd1a6267c3fb61c5d99dea2731fb25 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Tue, 7 Apr 2026 23:54:02 +1000 Subject: [PATCH 3/3] Add session log for PreviewEdit redesign --- .claude/sessions/2026-04-08.md | 64 ++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/.claude/sessions/2026-04-08.md b/.claude/sessions/2026-04-08.md index f421bc2..118717c 100644 --- a/.claude/sessions/2026-04-08.md +++ b/.claude/sessions/2026-04-08.md @@ -53,3 +53,67 @@ Tests compare against the full `{ type: 'ephemeral', ttl: CacheTtl.OneHour }` ob - `packages/claude-sdk/src/public/types.ts` — minor (no behaviour change) - `packages/claude-sdk/src/private/AgentRun.ts` — minor (no behaviour change) - `apps/claude-sdk-cli/src/systemPrompts.ts` — minor (no behaviour change) + + +--- + +## Split PreviewEdit edits into lineEdits + textEdits (PR #206) + +This session picked up where a previous session left off. That session had diagnosed the bug and designed the fix but was restarted before writing the tests. + +### The bug + +Combining an `insert` lineEdit with a `replace_text` in a single `PreviewEdit` call caused the text replacement to land N lines too early. The implementation maintained two independent streams: structural (line-number) ops passed through unchanged while text-search ops were applied against `currentContent`, which never had structural ops applied to it. When the two streams were merged, text replacements hit the wrong line. The bug was silent. + +### The fix + +Replaced the flat `edits` array with two separate arrays: + +- `lineEdits` (`insert | replace | delete`): all line numbers reference the file before the call. The tool sorts bottom-to-top internally, so callers specify edits in any order and the sequencing is handled automatically. +- `textEdits` (`replace_text | regex_text`): applied in order after all `lineEdits` complete, searching the post-`lineEdits` content. + +The old approach tried to reconcile two independent position streams. The new approach sequences them: structural edits always settle first, then text edits search the result. There is only ever one stream. + +### Implementation + +**`schema.ts`**: `EditFileOperationSchema` removed. `EditFileLineOperationSchema` and `EditFileTextOperationSchema` replace it. `PreviewEditInputSchema` has `lineEdits` + `textEdits`, both optional arrays, with a `.refine()` requiring at least one to be non-empty. + +**`validateEdits.ts`**: Renamed `validateEdits` to `validateLineEdits`. Running `currentLintCount` removed (no longer needed since all line numbers reference the original). Added overlap detection: throws when two edits target the same lines. + +**`EditFile.ts`**: Removed `findChangedRegions` and `resolveReplaceText`. Added `lineKey`, `sortBottomToTop`, `applyTextEdits`. Handler flow: sort lineEdits bottom-to-top, validate against original, applyEdits, then applyTextEdits. + +**`EditFile.spec.ts`**: Rewritten from scratch using `CreateFile` (not `PreviewEdit`). The old "multiple edits — sequential semantics" describe block described the now-removed behavior and was deleted. Replaced with: +- "lineEdits — bottom-to-top semantics" (6 tests): two non-overlapping edits both reference original, order of specification does not affect result, insert+replace both reference original, overlap throws, out-of-bounds throws. +- "lineEdits + textEdits — combined" (3 tests): textEdits see post-lineEdit content, multiple textEdits applied in order, textEdit can match a string inserted by lineEdit. + +**Ref tool**: Fixed description string and test that still said "limit=1000" after the default was raised to 10000 in a prior commit. + +**CLAUDE.md**: Replaced the old workaround entry ("here is how to avoid the bug") in recent decisions with the actual design. + +### Decisions + +**`CreateFile` to rewrite the test file, not `PreviewEdit`**: The test file is 463 lines. Generating it in a `PreviewEdit` response hits the output token limit. `CreateFile` places the content directly on disk without going through the response, so there is no size limit. + +**Sort bottom-to-top in the tool, not by the caller**: Callers describe what they want changed, not how to sequence patches. Sorting internally means callers think in terms of the original file throughout. Requiring the caller to sort would put mechanical burden back on the user of the tool. + +**`.refine()` requiring at least one of `lineEdits`/`textEdits`**: A call with both arrays empty does nothing useful and is almost certainly a mistake. The schema should catch it at the boundary rather than silently producing a no-op diff. + +### Pending local changes + +The working tree has uncommitted changes in `claude-sdk` and `apps/claude-sdk-cli/src/runAgent.ts`: +- `CacheTtl` enum moved from `types.ts` to `enums.ts` and exported from the package index +- `runAgent.ts` updated to use `CacheTtl.OneHour`; `maxTokens` raised from 8000 to 32000 + +These are unrelated to PreviewEdit and should go in a separate PR off a new branch. + +## Files changed (PR #206) + +- `packages/claude-sdk-tools/src/EditFile/schema.ts` +- `packages/claude-sdk-tools/src/EditFile/types.ts` +- `packages/claude-sdk-tools/src/EditFile/validateEdits.ts` +- `packages/claude-sdk-tools/src/EditFile/EditFile.ts` +- `packages/claude-sdk-tools/src/Ref/Ref.ts` +- `packages/claude-sdk-tools/src/Ref/schema.ts` +- `packages/claude-sdk-tools/test/EditFile.spec.ts` +- `packages/claude-sdk-tools/test/Ref.spec.ts` +- `.claude/CLAUDE.md`