Split PreviewEdit edits into lineEdits + textEdits

.claude/CLAUDE.md

@@ -237,7 +237,10 @@ Opt-in via `shellicarMcp: true` config. Registers an in-process MCP server (`she
 <!-- END:REPO:known-debt -->
 <!-- BEGIN:REPO:recent-decisions -->
 - **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 `<document>` block attachment; `d` removes selected chip; `← →` select chips. On `ctrl+enter` submit, attachments are folded into the prompt as `<document>` 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.

.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`

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<string, PreviewEditOutputType>) {
   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,32 +101,23 @@ export function createPreviewEdit(fs: IFileSystem, store: Map<string, PreviewEdi
       if (input.previousPatchId != null) {
         const prev = store.get(input.previousPatchId);
         if (!prev) {
-          throw new Error('Previous patch not found. The patch store is in-memory — please run PreviewEdit again.');
+          throw new Error('Previous patch not found. The patch store is in-memory \u2014 please run PreviewEdit again.');
         }
         if (prev.file !== filePath) {
           throw new Error(`File mismatch: previousPatchId is for "${prev.file}" but this edit targets "${filePath}"`);
         }
         baseContent = prev.newContent;
-        // Inherit the root originalHash rather than hashing prev.newContent.
-        // This means any patch in the chain can be applied directly to the original
-        // disk file (EditFile always checks disk == originalHash before writing).
-        // The trade-off: patches cannot be applied incrementally in sequence —
-        // EditFile(patch1) then EditFile(patch2) will fail because after patch1
-        // is written, disk no longer matches the inherited hash.
-        // If incremental sequential application were needed, each patch would
-        // store hash(prev.newContent) instead, but the practical use case
-        // is unclear given that EditFile(finalPatch) already writes everything.
         originalHash = prev.originalHash;
       } else {
         baseContent = await fs.readFile(filePath);
         originalHash = createHash('sha256').update(baseContent).digest('hex');
       }
 
       const baseLines = baseContent.split('\n');
-      const resolvedEdits = resolveReplaceText(baseContent, input.edits);
-      validateEdits(baseLines, resolvedEdits);
-      const newLines = applyEdits(baseLines, resolvedEdits);
-      const newContent = newLines.join('\n');
+      const sorted = sortBottomToTop(input.lineEdits);
+      validateLineEdits(baseLines, sorted);
+      const afterLineEdits = applyEdits(baseLines, sorted);
+      const newContent = applyTextEdits(afterLineEdits.join('\n'), input.textEdits);
       const diff = generateDiff(toDisplayPath(filePath), baseContent, newContent);
       const output = PreviewEditOutputSchema.parse({
         patchId: randomUUID(),