diff --git a/packages/coding-agent/src/edit/modes/atom.ts b/packages/coding-agent/src/edit/modes/atom.ts index a50e64af1..2a39b8bfa 100644 --- a/packages/coding-agent/src/edit/modes/atom.ts +++ b/packages/coding-agent/src/edit/modes/atom.ts @@ -148,39 +148,46 @@ function parseLidStmt(body: string, lineNum: number): ParsedStmt[] | null { // Range replace: `LidA..LidB=TEXT` deletes the inclusive range LidA..LidB // and inserts TEXT in its place. Following insert statements append more - // replacement lines through the normal hunk reorder path. + // replacement lines through the normal hunk reorder path. Legacy `|` is + // accepted as a set separator for parity with single-line `Lid|TEXT`. + // Bare `LidA..LidB` recovers the common missing-`-` typo for range delete. if (rest.startsWith("..")) { const m2 = LID_RE.exec(rest.slice(2)); if (m2) { const endLn = Number.parseInt(m2[1], 10); const endHash = m2[2]; const after = rest.slice(2 + m2[0].length); - const eq = /^[ \t]*=(.*)$/.exec(after); - if (eq) { - if (endLn < ln) { - throw new Error( - `Diff line ${lineNum}: range \`${ln}${hash}..${endLn}${endHash}\` ends before it starts. Use \`LidA..LidB=TEXT\` with LidA's line number ≤ LidB's.`, - ); - } - if (endLn === ln && endHash !== hash) { - throw new Error( - `Diff line ${lineNum}: range \`${ln}${hash}..${endLn}${endHash}\` uses two different hashes for the same line. Copy the same Lid at both endpoints or use \`${ln}${hash}=TEXT\` for a single-line replacement.`, - ); - } - if (eq[1].includes("\r")) { + const range = `${ln}${hash}..${endLn}${endHash}`; + if (endLn < ln) { + throw new Error( + `Diff line ${lineNum}: range \`${range}\` ends before it starts. Use \`LidA..LidB=TEXT\` with LidA's line number ≤ LidB's.`, + ); + } + if (endLn === ln && endHash !== hash) { + throw new Error( + `Diff line ${lineNum}: range \`${range}\` uses two different hashes for the same line. Copy the same Lid at both endpoints or use \`${ln}${hash}=TEXT\` for a single-line replacement.`, + ); + } + + const stmts: ParsedStmt[] = []; + for (let l = ln; l <= endLn; l++) { + const h = l === ln ? hash : l === endLn ? endHash : RANGE_INTERIOR_HASH; + stmts.push({ + kind: "anchor_op", + anchor: { line: l, hash: h }, + op: { op: "delete" }, + lineNum, + }); + } + + if (after.trim().length === 0) return stmts; + + const replacement = /^[ \t]*([=|])(.*)$/.exec(after); + if (replacement) { + if (replacement[2].includes("\r")) { throw new Error(`Diff line ${lineNum}: set value contains a carriage return; use a single-line value.`); } - const stmts: ParsedStmt[] = []; - for (let l = ln; l <= endLn; l++) { - const h = l === ln ? hash : l === endLn ? endHash : RANGE_INTERIOR_HASH; - stmts.push({ - kind: "anchor_op", - anchor: { line: l, hash: h }, - op: { op: "delete" }, - lineNum, - }); - } - stmts.push({ kind: "insert", text: eq[1], lineNum }); + stmts.push({ kind: "insert", text: replacement[2], lineNum }); return stmts; } } @@ -289,6 +296,17 @@ function parseDeleteStmt(body: string, lineNum: number): ParsedStmt[] | null { return null; } +function parseIndentedHashlineStmt(line: string, lineNum: number): ParsedStmt[] | null { + const trimmed = line.trimStart(); + if (trimmed === line) return null; + const stmts = parseLidStmt(trimmed, lineNum); + if (!stmts) return null; + const safeHashlineEcho = stmts.every( + stmt => stmt.kind === "bare_anchor" || (stmt.kind === "anchor_op" && stmt.op.op === "set"), + ); + return safeHashlineEcho ? stmts : null; +} + function throwMalformedLidDiagnostic(line: string, lineNum: number, raw: string): never { const text = line.trimStart(); const withoutLegacyMove = text.startsWith("@@ ") ? text.slice(3).trimStart() : text; @@ -323,6 +341,9 @@ function parseDiffLine(raw: string, lineNum: number): ParsedStmt[] { // inserts corrupts files, and the canonical syntax has no comment op. if (line[0] === "#") return []; + const indentedHashline = parseIndentedHashlineStmt(line, lineNum); + if (indentedHashline) return indentedHashline; + // `+TEXT` inserts at the cursor. Everything after `+` is content. A // `+Lid|TEXT` or `+Lid=TEXT` line is a diff-ish add (unified-diff trap): // emit a tagged stmt so the normalizer can fuse it with a preceding `-Lid`. @@ -476,13 +497,13 @@ function parseDiffLine(raw: string, lineNum: number): ParsedStmt[] { } // Lines that look like recognized atom ops. Used to delimit range-replace -// recovery continuation: after `LidA..LidB=TEXT`, an unprefixed non-op line +// recovery continuation: after `LidA..LidB=TEXT` (or legacy `|` separator), // is treated as literal replacement text for backward compatibility. const OP_LINE_HEAD_RE = /^([+\-@$^!]|[1-9]\d*[a-z]{2}|[ \t]*$)/; const RANGE_CONTINUATION_SENTINEL = "\u0000"; function isRangeReplaceStart(line: string): boolean { - return /^[1-9]\d*[a-z]{2}\.\.[1-9]\d*[a-z]{2}[ \t]*=/.test(line); + return /^[1-9]\d*[a-z]{2}\.\.[1-9]\d*[a-z]{2}[ \t]*[=|]/.test(line); } // A single-line `Lid=TEXT` (or legacy `Lid|TEXT`, with optional leading `@`) @@ -1534,6 +1555,60 @@ async function executeAtomWholeFileOperation( }; } +async function preflightAtomSection(options: ExecuteAtomSingleOptions & AtomInputSection): Promise { + const { session, path: sectionPath, diff } = options; + if (options.wholeFileOperation) { + const { wholeFileOperation } = options; + const absolutePath = resolvePlanPath(session, sectionPath); + if (sectionPath.endsWith(".ipynb")) { + throw new Error("Cannot edit Jupyter notebooks with the Edit tool. Use the NotebookEdit tool instead."); + } + if (wholeFileOperation.kind === "delete") { + enforcePlanModeWrite(session, sectionPath, { op: "delete" }); + await assertEditableFile(absolutePath, sectionPath); + return; + } + + const destinationPath = wholeFileOperation.destination; + if (destinationPath.endsWith(".ipynb")) { + throw new Error("Cannot edit Jupyter notebooks with the Edit tool. Use the NotebookEdit tool instead."); + } + enforcePlanModeWrite(session, sectionPath, { op: "update", move: destinationPath }); + const absoluteDestinationPath = resolvePlanPath(session, destinationPath); + if (absoluteDestinationPath === absolutePath) { + throw new Error("rename path is the same as source path"); + } + await assertEditableFile(absolutePath, sectionPath); + return; + } + + const { edits } = parseAtomWithWarnings(diff); + if (edits.length === 0 && diff.trim().length > 0) { + throw new Error(formatNoAtomEditDiagnostic(sectionPath, diff)); + } + + enforcePlanModeWrite(session, sectionPath, { op: "update" }); + if (sectionPath.endsWith(".ipynb") && edits.length > 0) { + throw new Error("Cannot edit Jupyter notebooks with the Edit tool. Use the NotebookEdit tool instead."); + } + + const absolutePath = resolvePlanPath(session, sectionPath); + const source = await readAtomFile(absolutePath); + if (!source.exists && hasAnchorScopedEdit(edits)) { + throw new Error(`File not found: ${sectionPath}`); + } + if (source.exists) { + assertEditableFileContent(source.rawContent, sectionPath); + } + + const { text } = stripBom(source.rawContent); + const originalNormalized = normalizeToLF(text); + const result = applyAtomEdits(originalNormalized, edits); + if (originalNormalized === result.lines && (result.noopEdits?.length ?? 0) === 0) { + throw new Error(formatNoChangeDiagnostic(sectionPath, result)); + } +} + async function executeAtomSection( options: ExecuteAtomSingleOptions & AtomInputSection, ): Promise> { @@ -1629,6 +1704,10 @@ export async function executeAtomSingle( return executeAtomSection({ ...options, ...section }); } + for (const section of sections) { + await preflightAtomSection({ ...options, ...section }); + } + const results = []; for (const section of sections) { results.push({ diff --git a/packages/coding-agent/src/edit/modes/hashline.ts b/packages/coding-agent/src/edit/modes/hashline.ts index 6f6ee0c1c..00a0ee4ad 100644 --- a/packages/coding-agent/src/edit/modes/hashline.ts +++ b/packages/coding-agent/src/edit/modes/hashline.ts @@ -606,9 +606,9 @@ export class HashlineMismatchError extends Error { "The edit was NOT applied, please use the updated file content shown below, and issue another edit tool-call.", ); - // Content-based recovery hint: if the original hash uniquely matches a - // line elsewhere in the file (outside the auto-rebase window), suggest - // the new Lid so the model can retry without a full re-read. + // Content-based recovery hint: the two-letter hash is weak, so a + // unique match elsewhere is only a candidate. Keep this advisory; never + // silently retarget stale edits based on a whole-file hash-only match. const hints: string[] = []; for (const m of mismatches) { const matches: number[] = []; @@ -621,7 +621,7 @@ export class HashlineMismatchError extends Error { } } if (hints.length > 0) { - lines.push("Likely shifted (hash matched a unique line elsewhere):"); + lines.push("Hash-only shifted candidate; verify content/context before using:"); lines.push(...hints); } diff --git a/packages/coding-agent/src/prompts/tools/atom.md b/packages/coding-agent/src/prompts/tools/atom.md index 20daac464..65b25f61b 100644 --- a/packages/coding-agent/src/prompts/tools/atom.md +++ b/packages/coding-agent/src/prompts/tools/atom.md @@ -35,7 +35,7 @@ Lid= blank the anchored line's content but KEEP the line (results in an em - To insert ABOVE a line, you **MUST** use `^Lid` then `+TEXT`. To insert above line 1, you **MUST** use `^` (BOF) then `+TEXT`. To insert below a line, you **MUST** use `@Lid` then `+TEXT`. - Multiple `---PATH` sections **MAY** appear in one input; each section is applied in order. - `!rm` / `!mv DEST` **MUST NOT** be combined with line edits in the same section. -- Lids contain a content hash. If a line has changed since you read it, the tool rejects the edit and shows the current content; you **MUST** re-read and retry with fresh Lids. Small drift (≤5 lines) where the original hash still matches a nearby line auto-rebases with a warning; larger shifts require a re-read. +- Lids contain a content hash. If a line has changed since you read it, the tool rejects the edit and shows the current content; you **MUST** re-read and retry with fresh Lids. Small drift (≤5 lines) where the original hash still matches a nearby line auto-rebases with a warning. Larger shifts may show a hash-only candidate, but two-letter hashes collide; verify surrounding content or re-read before using it. - After `+TEXT` (or `+`) the cursor advances past the inserted line, so consecutive `+TEXT` ops stack in order. After `Lid=TEXT` the cursor sits on the modified anchor; after `-Lid` it sits on the slot the deleted line vacated. You **MUST** use a fresh `@Lid` / `^Lid` / `^` / `$` to reposition. - The tool is syntax-blind: it will not check brackets, indentation, table column counts, or fence integrity. You **MUST** verify indentation-sensitive or structured files after editing (Python, Markdown tables/fences). - A section whose PATH does not yet exist creates the file from your `+TEXT` lines (use `^` or `$` then `+TEXT…`). No separate "create file" op is needed. diff --git a/packages/coding-agent/test/core/atom.test.ts b/packages/coding-agent/test/core/atom.test.ts index 8d13b39be..44a7760f3 100644 --- a/packages/coding-agent/test/core/atom.test.ts +++ b/packages/coding-agent/test/core/atom.test.ts @@ -112,6 +112,12 @@ describe("atom parser — basic forms", () => { expect(applyDiff(longer, diff)).toBe("aaa\nREPLACED\neee"); }); + it("bare `LidA..LidB` recovers a missing `-` range delete typo", () => { + const longer = "aaa\nbbb\nccc\nddd\neee"; + const diff = `${tag(2, "bbb")}..${tag(4, "ddd")}`; + expect(applyDiff(longer, diff)).toBe("aaa\neee"); + }); + it("`-LidA..LidB` rejects a reversed range", () => { const longer = "aaa\nbbb\nccc\nddd"; const diff = `-${tag(3, "ccc")}..${tag(2, "bbb")}`; @@ -180,6 +186,12 @@ describe("atom parser — basic forms", () => { expect(applyDiff(longer, diff)).toBe("aaa\nexport function label() {\n return 1;\n}\neee"); }); + it("`LidA..LidB|FIRST` accepts legacy pipe as range replacement separator", () => { + const longer = "aaa\nbbb\nccc\nddd\neee"; + const diff = [`${tag(2, "bbb")}..${tag(4, "ddd")}|ONE`, "\\TWO"].join("\n"); + expect(applyDiff(longer, diff)).toBe("aaa\nONE\nTWO\neee"); + }); + it("bare backslash continuation inserts a blank replacement line", () => { const longer = "aaa\nbbb\nccc\nddd\neee"; const diff = [`${tag(2, "bbb")}..${tag(4, "ddd")}=ONE`, "\\", "\\THREE"].join("\n"); @@ -521,6 +533,15 @@ describe("atom parser — edge cases", () => { expect(applyDiff(content, diff)).toBe("aaa\nBBB\nccc"); }); + it("repairs indented read-output lines as hashline replacements", () => { + const content = '{\n "mode": "demo",\n "strict": true'; + const t1 = tag(1, "{"); + const t2 = tag(2, ' "mode": "demo",'); + const t3 = tag(3, ' "strict": true'); + const diff = `${t1}|{\n ${t2}| "mode": "demo2",\n ${t3}| "strict": true`; + expect(applyDiff(content, diff)).toBe('{\n "mode": "demo2",\n "strict": true'); + }); + it("same-line OLD|NEW repair works through `@` prefix slip", () => { const t = tag(2, "bbb"); const diff = `@${t}|bbb|BBB`; @@ -965,6 +986,22 @@ describe("atom executor — whole-file operations", () => { expect(await Bun.file(path.join(tempDir, "file.ts")).text()).toBe(content); }); }); + + it("preflights all sections before writing a multi-file edit", async () => { + await withTempDir(async tempDir => { + const aPath = path.join(tempDir, "a.ts"); + const bPath = path.join(tempDir, "b.ts"); + await Bun.write(aPath, "aaa\n"); + await Bun.write(bPath, "bbb\n"); + + const input = [`---a.ts`, `${tag(1, "aaa")}=AAA`, `---b.ts`, `${mistag(1, "bbb")}=BBB`].join("\n"); + await expect(executeAtomSingle(atomExecuteOptions(tempDir, input))).rejects.toThrow( + /changed since the last read/, + ); + expect(await Bun.file(aPath).text()).toBe("aaa\n"); + expect(await Bun.file(bPath).text()).toBe("bbb\n"); + }); + }); }); // ─────────────────────────────────────────────────────────────────────────── diff --git a/packages/coding-agent/test/core/hashline.test.ts b/packages/coding-agent/test/core/hashline.test.ts index c4a6c6e3a..c4ec308be 100644 --- a/packages/coding-agent/test/core/hashline.test.ts +++ b/packages/coding-agent/test/core/hashline.test.ts @@ -696,7 +696,7 @@ describe("applyHashlineEdits — errors", () => { } }); - it("stale hash error suggests new Lid when content moved", () => { + it("stale hash error suggests a hash-only candidate when content moved", () => { // Original line: "moved" was at line 2. Now it's at line 8 (way beyond ±5 rebase). // Caller still references it via the old `2` lid. const content = "aaa\nbbb\nccc\nddd\neee\nfff\nggg\nmoved\niii"; @@ -708,7 +708,7 @@ describe("applyHashlineEdits — errors", () => { } catch (err) { expect(err).toBeInstanceOf(HashlineMismatchError); const msg = (err as HashlineMismatchError).message; - expect(msg).toContain("Likely shifted"); + expect(msg).toContain("Hash-only shifted candidate"); expect(msg).toContain(`2${movedHash} → 8${movedHash}`); } }); diff --git a/packages/typescript-edit-benchmark/src/report.ts b/packages/typescript-edit-benchmark/src/report.ts index 1b167f88a..d8350bc1b 100644 --- a/packages/typescript-edit-benchmark/src/report.ts +++ b/packages/typescript-edit-benchmark/src/report.ts @@ -3,7 +3,7 @@ */ import { formatDuration, formatPercent, truncate } from "@oh-my-pi/pi-utils"; -import { EDIT_FAILURE_CATEGORIES, type BenchmarkResult, type TaskResult } from "./runner"; +import { type BenchmarkResult, EDIT_FAILURE_CATEGORIES, type TaskResult } from "./runner"; function getStatusEmoji(successRate: number, runsPerTask: number): string { const passing = Math.round(successRate * runsPerTask);