From 634498dd21fa4882dd24161171317bf04fea3337 Mon Sep 17 00:00:00 2001 From: Aleksandar Grbic Date: Sun, 28 Jun 2026 21:45:56 +0200 Subject: [PATCH 1/4] fix(edit): kill the auto-format edit-rejection round-trip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The local model's #1 reported friction: the write-guard reformats a file right after create/edit (eslint --fix + prettier), so the model's next edit oldString no longer matches disk → not-found → a wasted turn re-reading the file. Two fixes: - fuzzyLineReplace now also tolerates a line-trailing comma/semicolon (prettier adds trailing commas in multiline literals/args and normalizes semicolons), so more near-miss edits land silently. Unique-window guard still blocks ambiguity. - A not-found edit rejection now INLINES the file's current content (numbered, <=400 lines) and drops the 'go read it' instruction, so the model repairs the stale anchor in the same turn. Falls back to advising read for huge files. Tests: genuine multi-line fuzzy cases (trailing comma, semicolon) + a doEdit rejection-carries-content test. From the DeepSeek harness feedback; borrowed-ideas #3 (+#2). --- docs/borrowed-ideas.md | 8 +++ packages/core/src/files/edit.ts | 6 +- packages/core/src/loop/tools/file-ops.ts | 44 ++++++++++++++- packages/core/tests/files-edit.test.ts | 70 ++++++++++++++++++++++++ 4 files changed, 125 insertions(+), 3 deletions(-) diff --git a/docs/borrowed-ideas.md b/docs/borrowed-ideas.md index 2c93bb96..f0b8c6b9 100644 --- a/docs/borrowed-ideas.md +++ b/docs/borrowed-ideas.md @@ -82,6 +82,14 @@ decoding — returning schema-conformant output. See memory existing `files-edit` / `salvage-toolcall` tests). **ROI:** high. - **Done when:** near-miss edits (whitespace/indent drift) apply instead of failing, and malformed-but-recoverable tool calls parse without a retry. +- **Status (PR #57):** the **edit-on-autoformat** friction (the local model's #1 + reported pain — the write-guard reformats after a write, so the next edit's + oldString no longer matches) is addressed: (1) `fuzzyLineReplace` now also + tolerates prettier trailing commas + semicolon drift, so more near-miss edits + just land; (2) a not-found rejection now **inlines the file's current content** + (numbered, ≤400 lines) so the model repairs the stale anchor in the SAME turn + instead of spending one on a re-`read`. BAML-style lenient tool-call parsing and + the create-side ACI echo (#2) remain follow-ups. --- diff --git a/packages/core/src/files/edit.ts b/packages/core/src/files/edit.ts index d4b9804e..707a1b07 100644 --- a/packages/core/src/files/edit.ts +++ b/packages/core/src/files/edit.ts @@ -176,7 +176,11 @@ function fuzzyLineReplace( .trim() .replace(/['"`]/gu, '"') // unify quote style (straight + the folded curly) .replace(/\s+/gu, " ") // collapse internal whitespace runs - .replace(/\s*([^\w\s])\s*/gu, "$1"); // drop whitespace AROUND punctuation + .replace(/\s*([^\w\s])\s*/gu, "$1") // drop whitespace AROUND punctuation + .replace(/[;,]+$/u, ""); // drop a line-trailing `,`/`;` (prettier adds trailing + // commas in multiline literals/args and normalizes semicolons — the model + // writes `b` where the formatted file has `b,`, or `foo()` vs `foo();`). The + // unique-window guard still blocks any ambiguous match this might widen into. // ^ `foo( a, b )`, `foo(a,b)`, and `foo(a, b)` all normalize equal, but two // identifiers keep their separating space (`const x` never becomes `constx`), // so the match stays meaningful. Unique-window guard still blocks ambiguity. diff --git a/packages/core/src/loop/tools/file-ops.ts b/packages/core/src/loop/tools/file-ops.ts index c651b87e..b125dfb1 100644 --- a/packages/core/src/loop/tools/file-ops.ts +++ b/packages/core/src/loop/tools/file-ops.ts @@ -643,14 +643,54 @@ export async function doEdit( edit.edits.length > 1 ? ` (replacement #${result.index + 1})` : ""; const authored = ctx.touched?.has(edit.file.replaceAll("\\", "/")) ?? false; + let help = editFailHelp(edit.file, result, authored); + + // A not-found edit is almost always a STALE ANCHOR — the auto-formatter rewrote + // the file after the model's last write, so its oldString no longer matches. The + // harness already has the current bytes, so inline them rather than make the + // model spend a turn re-`read`ing (its #1 reported friction). + if (result.reason === EDIT_FAIL_REASON.notFound) { + const view = await currentFileView(ctx.cwd, edit.file); + + help += + view === null + ? ` \`read\` ${edit.file} to see its exact current content, then edit with text copied verbatim.` + : ` Its CURRENT content is below — copy oldString verbatim from it and retry (no need to \`read\`):\n\n${view}`; + } return reject( ctx, `edit:${result.reason}`, - `edit ${edit.file} REJECTED${where}: ${editFailHelp(edit.file, result, authored)}` + `edit ${edit.file} REJECTED${where}: ${help}` ); } +/** Cap on lines inlined into a not-found edit rejection; above this, fall back to + * advising a `read` so a huge file can't flood the model's context. */ +const EDIT_REJECT_MAX_LINES = 400; + +/** The file's current content as numbered lines (same shape as `read`), or null + * if it's missing or too large to inline. Used to repair a stale-anchor edit in + * the SAME turn — the model copies its oldString from the post-format text. */ +async function currentFileView( + cwd: string, + file: string +): Promise { + const handle = Bun.file(join(cwd, file)); + + if (!(await handle.exists())) { + return null; + } + + const lines = (await handle.text()).split("\n"); + + if (lines.length > EDIT_REJECT_MAX_LINES) { + return null; + } + + return lines.map((line, i) => `${i + 1}${HL_LINE_SEP}${line}`).join("\n"); +} + /** * Turn an edit-failure reason into ACTIONABLE feedback. The bare reason strings * ("not-found", "missing-file") were fatally ambiguous: a slow local model read @@ -681,7 +721,7 @@ function editFailHelp( ? ` Since you created ${file} this session, you may also \`create\` it again to fully rewrite it.` : " Do NOT use `create` (it already exists)."; - return `the file ${file} EXISTS, but your oldString text was not found in it.${rewrite} \`read\` the file to see its exact current contents, then edit with text copied verbatim from it.`; + return `the file ${file} EXISTS, but your oldString text was not found in it (it was likely auto-reformatted after your last write — imports reordered, quotes/commas normalized).${rewrite}`; } return result.reason; diff --git a/packages/core/tests/files-edit.test.ts b/packages/core/tests/files-edit.test.ts index 1e3706b8..149ceb34 100644 --- a/packages/core/tests/files-edit.test.ts +++ b/packages/core/tests/files-edit.test.ts @@ -3,6 +3,8 @@ import { mkdtemp, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { applyEdit, applyEdits } from "../src/files/edit"; +import { doEdit } from "../src/loop/tools/file-ops"; +import type { IToolContext } from "../src/loop/tools/tool-context"; async function tmp(files: Record): Promise { const dir = await mkdtemp(join(tmpdir(), "tsforge-edit-")); @@ -139,6 +141,44 @@ test("fuzzy edit preserves CRLF line endings (no mixed endings) — issue #24", } }); +test("fuzzy edit tolerates a prettier trailing comma (multiline literal)", async () => { + // Prettier added a trailing comma after `b: 2`. The model's oldString ends the + // window at `b: 2` (no comma), so the EXACT match misses (`b: 2\n}` ≠ `b: 2,\n}`); + // only the comma-tolerant fuzzy path can match. + const dir = await tmp({ "a.ts": "const o = {\n a: 1,\n b: 2,\n};\n" }); + + try { + const r = await applyEdits(dir, "a.ts", [ + { oldString: " a: 1,\n b: 2\n}", newString: " a: 1,\n b: 3\n}" }, + ]); + + expect(r.ok).toBe(true); + expect(await Bun.file(join(dir, "a.ts")).text()).toContain("b: 3"); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + +test("fuzzy edit tolerates semicolon-style drift", async () => { + // File has trailing semicolons (prettier); the model's multi-line oldString + // omits them, so the exact match misses and the semicolon-tolerant fuzzy fires. + const dir = await tmp({ "a.ts": "const a = 1;\nconst b = 2;\n" }); + + try { + const r = await applyEdits(dir, "a.ts", [ + { + oldString: "const a = 1\nconst b = 2", + newString: "const a = 1\nconst b = 99", + }, + ]); + + expect(r.ok).toBe(true); + expect(await Bun.file(join(dir, "a.ts")).text()).toContain("const b = 99"); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + test("fuzzy delete (empty newString) removes the matched lines, no blank line", async () => { // Wrong indentation forces the fuzzy path; an empty newString should DELETE // the matched line, not replace it with a blank line. @@ -345,3 +385,33 @@ test("applyEdits sees the result of earlier replacements (sequential)", async () await rm(dir, { recursive: true, force: true }); } }); + +// A not-found edit (stale anchor after auto-format) inlines the file's CURRENT +// content into the rejection, so the model repairs it in the SAME turn instead +// of spending one on a re-`read` — the model's #1 reported friction. +test("not-found edit rejection carries the file's current content", async () => { + const dir = await tmp({ "a.ts": "const x = 1;\nconst y = 2;\n" }); + const ctx: IToolContext = { + cwd: dir, + files: ["**/*.ts"], + task: "t", + report: () => undefined, + }; + + try { + const msg = await doEdit( + { file: "a.ts", oldString: "const z = 99;", newString: "const z = 0;" }, + ctx + ); + + expect(msg).toContain("REJECTED"); + expect(msg).toContain("CURRENT content is below"); + // The actual current lines are present so the model can copy oldString verbatim. + expect(msg).toContain("const x = 1;"); + expect(msg).toContain("const y = 2;"); + // And it does NOT tell the model to go `read` the file (content is inline). + expect(msg).not.toContain("`read` a.ts to see"); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); From 067859df20067efb66c086506b230b54ea0e2871 Mon Sep 17 00:00:00 2001 From: Aleksandar Grbic Date: Sun, 28 Jun 2026 21:50:40 +0200 Subject: [PATCH 2/4] fix(edit): guard currentFileView against I/O errors (Gemini review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The edit already failed when we build its rejection; if reading the file to inline its content throws (race, permissions, lock), return null and fall back to advising a read — never crash doEdit. --- packages/core/src/loop/tools/file-ops.ts | 27 +++++++++++++++--------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/packages/core/src/loop/tools/file-ops.ts b/packages/core/src/loop/tools/file-ops.ts index b125dfb1..e19c5171 100644 --- a/packages/core/src/loop/tools/file-ops.ts +++ b/packages/core/src/loop/tools/file-ops.ts @@ -670,25 +670,32 @@ export async function doEdit( const EDIT_REJECT_MAX_LINES = 400; /** The file's current content as numbered lines (same shape as `read`), or null - * if it's missing or too large to inline. Used to repair a stale-anchor edit in - * the SAME turn — the model copies its oldString from the post-format text. */ + * if it's missing, too large to inline, or unreadable. Used to repair a stale- + * anchor edit in the SAME turn — the model copies its oldString from the post- + * format text. Returns null on any I/O error (race, permissions): the edit has + * already failed, so enriching its message must never crash the tool — the caller + * then falls back to advising a `read`. */ async function currentFileView( cwd: string, file: string ): Promise { - const handle = Bun.file(join(cwd, file)); + try { + const handle = Bun.file(join(cwd, file)); - if (!(await handle.exists())) { - return null; - } + if (!(await handle.exists())) { + return null; + } - const lines = (await handle.text()).split("\n"); + const lines = (await handle.text()).split("\n"); - if (lines.length > EDIT_REJECT_MAX_LINES) { + if (lines.length > EDIT_REJECT_MAX_LINES) { + return null; + } + + return lines.map((line, i) => `${i + 1}${HL_LINE_SEP}${line}`).join("\n"); + } catch { return null; } - - return lines.map((line, i) => `${i + 1}${HL_LINE_SEP}${line}`).join("\n"); } /** From 6c9ab5ee4023fbb09a963fcfb93d24d5b026c99b Mon Sep 17 00:00:00 2001 From: Aleksandar Grbic Date: Sun, 28 Jun 2026 22:13:42 +0200 Subject: [PATCH 3/4] test(e2e): edit anchor survives the real write-guard auto-format Drives the actual formatFile (eslint --fix + prettier), not a hand-built string: prettier adds a trailing comma/semicolon, and the model's pre-format anchor still lands via the widened fuzzy matcher instead of rejecting. Institutionalizes the manual verification of the edit-on-autoformat fix. --- .../core/tests/edit-autoformat.e2e.test.ts | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 packages/core/tests/edit-autoformat.e2e.test.ts diff --git a/packages/core/tests/edit-autoformat.e2e.test.ts b/packages/core/tests/edit-autoformat.e2e.test.ts new file mode 100644 index 00000000..079abca9 --- /dev/null +++ b/packages/core/tests/edit-autoformat.e2e.test.ts @@ -0,0 +1,42 @@ +import { test, expect } from "bun:test"; +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { formatFile } from "../src/detect-gate"; +import { applyEdits } from "../src/files/edit"; + +// e2e: the write-guard auto-formats a file (real eslint --fix + prettier) right +// after a write, so the model's next edit anchor no longer matches disk. This +// drives the REAL formatter (not a hand-built "formatted" string) and proves a +// pre-format anchor still lands via the widened fuzzy matcher — the local model's +// #1 reported friction. (A structural rewrite the fuzzy can't bridge falls back to +// the inlined-content rejection, covered in files-edit.test.ts.) +test("a pre-format edit anchor survives the real write-guard auto-format", async () => { + const dir = await mkdtemp(join(tmpdir(), "tsforge-fmt-e2e-")); + const file = "demo.ts"; + // No trailing comma, no semicolons — the formatter will add both. + const before = "const o = {\n a: 1,\n b: 2\n}\n"; + + await Bun.write(join(dir, file), before); + + try { + await formatFile(dir, file); + const after = await Bun.file(join(dir, file)).text(); + + // Sanity: the formatter actually reformatted (else the test proves nothing). + expect(after).not.toBe(before); + expect(after).toContain("b: 2,"); // prettier added the trailing comma + + // The model's pre-format anchor (`b: 2\n}`, no trailing comma) exact-misses the + // formatted file (`b: 2,\n};`) — it must still apply via the fuzzy fallback, + // NOT reject and force a re-read. + const r = await applyEdits(dir, file, [ + { oldString: " a: 1,\n b: 2\n}", newString: " a: 1,\n b: 999\n}" }, + ]); + + expect(r.ok).toBe(true); + expect(await Bun.file(join(dir, file)).text()).toContain("b: 999"); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); From ec8e391f3a660caa34d4dcce3eeece985636eb10 Mon Sep 17 00:00:00 2001 From: Aleksandar Grbic Date: Sun, 28 Jun 2026 22:20:09 +0200 Subject: [PATCH 4/4] docs(edit): clarify currentFileView shape vs read (Copilot review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It returns read's numbered body WITHOUT the hashline header — by design (it repairs a str_replace edit, which anchors on verbatim text, not a line hash). --- packages/core/src/loop/tools/file-ops.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/core/src/loop/tools/file-ops.ts b/packages/core/src/loop/tools/file-ops.ts index e19c5171..fdfd37eb 100644 --- a/packages/core/src/loop/tools/file-ops.ts +++ b/packages/core/src/loop/tools/file-ops.ts @@ -669,10 +669,11 @@ export async function doEdit( * advising a `read` so a huge file can't flood the model's context. */ const EDIT_REJECT_MAX_LINES = 400; -/** The file's current content as numbered lines (same shape as `read`), or null - * if it's missing, too large to inline, or unreadable. Used to repair a stale- - * anchor edit in the SAME turn — the model copies its oldString from the post- - * format text. Returns null on any I/O error (race, permissions): the edit has +/** The file's current content as numbered rows (line number + `HL_LINE_SEP` + text) + * — like `read`'s body but WITHOUT its hashline header (this repairs a `str_replace` + * edit, which anchors on verbatim text, not a line hash). Null if the file is + * missing, too large to inline, or unreadable. Used to repair a stale-anchor edit in + * the SAME turn — the model copies its oldString from the post-format text. Returns null on any I/O error (race, permissions): the edit has * already failed, so enriching its message must never crash the tool — the caller * then falls back to advising a `read`. */ async function currentFileView(