diff --git a/packages/coding-agent/src/hashline/constants.ts b/packages/coding-agent/src/hashline/constants.ts index b09ff7aa3..61284f8d8 100644 --- a/packages/coding-agent/src/hashline/constants.ts +++ b/packages/coding-agent/src/hashline/constants.ts @@ -28,3 +28,14 @@ export const ABORT_WARNING = */ export const REPLACE_PAIR_COALESCED_WARNING = "Detected an identical-range before/after replace pair; kept only the second block's payload. Issue ONE op per range — the payload is the final desired content, never both old and new."; + +/** + * Warning text appended when a single-line replace op like `83: content` + * arrives while a multi-line replace `A-B:` is still pending and `83` is + * inside `A-B`. The model used the read-output `LINE:TEXT` format as if it + * were a payload-continuation line; we strip the `LINE:` prefix and treat + * `content` as the next payload line, but warn so the model learns the + * cleaner format on its own. + */ +export const PAYLOAD_LINE_PREFIX_DEMOTED_WARNING = + "Detected one or more `LINE:TEXT` lines whose anchors fell inside the pending replace range; treated them as payload-continuation lines and stripped the `LINE:` prefix. Inside a multi-line `A-B:` block, payload lines after the first do not need a line-number prefix."; diff --git a/packages/coding-agent/src/hashline/executor.ts b/packages/coding-agent/src/hashline/executor.ts index c567165d0..509966ea9 100644 --- a/packages/coding-agent/src/hashline/executor.ts +++ b/packages/coding-agent/src/hashline/executor.ts @@ -1,4 +1,4 @@ -import { ABORT_WARNING, REPLACE_PAIR_COALESCED_WARNING } from "./constants"; +import { ABORT_WARNING, PAYLOAD_LINE_PREFIX_DEMOTED_WARNING, REPLACE_PAIR_COALESCED_WARNING } from "./constants"; import { HL_OP_CHARS, HL_OP_DELETE, HL_OP_INSERT_AFTER, HL_OP_INSERT_BEFORE, HL_OP_REPLACE } from "./hash"; import { cloneCursor, @@ -19,6 +19,10 @@ function rangesEqual(a: ParsedRange, b: ParsedRange): boolean { return a.start.line === b.start.line && a.end.line === b.end.line; } +function rangeContains(outer: ParsedRange, inner: ParsedRange): boolean { + return outer.start.line <= inner.start.line && inner.end.line <= outer.end.line; +} + function expandRange(range: ParsedRange): Anchor[] { const anchors: Anchor[] = []; for (let line = range.start.line; line <= range.end.line; line++) { @@ -113,20 +117,31 @@ export class HashlineExecutor { return; case "op-replace": validateRangeOrder(token.range, token.lineNum); - // Common shape: model emits the same `A-B:` block twice — a "before" - // payload followed by an "after" payload. The first op's payload is - // just informational (and will be replaced wholesale by the second - // op's payload anyway), so discard the pending op silently and let - // the second op proceed. Other overlap shapes (different ranges, - // replace+delete, delete+delete) still hit the post-hoc validator. - if ( - this.#pending !== undefined && - this.#pending.op.kind === "replace" && - rangesEqual(this.#pending.op.range, token.range) - ) { - this.#pending = undefined; - if (!this.#warnings.includes(REPLACE_PAIR_COALESCED_WARNING)) { - this.#warnings.push(REPLACE_PAIR_COALESCED_WARNING); + if (this.#pending !== undefined && this.#pending.op.kind === "replace") { + const outer = this.#pending.op.range; + const inner = token.range; + if (rangesEqual(outer, inner)) { + // Identical-range before/after pair. Drop the "before" payload + // silently; the second op proceeds as the lone winner. Other + // overlap shapes (different ranges, replace+delete, + // delete+delete) still hit the post-hoc validator. + this.#pending = undefined; + if (!this.#warnings.includes(REPLACE_PAIR_COALESCED_WARNING)) { + this.#warnings.push(REPLACE_PAIR_COALESCED_WARNING); + } + } else if (rangeContains(outer, inner)) { + // Model wrote a payload line in read-output `LINE:TEXT` format + // (or `A-B:TEXT` for a sub-range) inside an outer `A-B:` block. + // The tokenizer can't tell payload from op when the anchor and + // sigil shape are identical, so demote: append the op's inline + // body to the pending payload, strip the `LINE:` prefix, and + // keep accumulating. Without this the inner anchors would each + // register as their own delete and clash with the outer range. + this.#pending.payload.push(token.inlineBody ?? ""); + if (!this.#warnings.includes(PAYLOAD_LINE_PREFIX_DEMOTED_WARNING)) { + this.#warnings.push(PAYLOAD_LINE_PREFIX_DEMOTED_WARNING); + } + return; } } this.#flushPending(); diff --git a/packages/coding-agent/test/core/hashline.test.ts b/packages/coding-agent/test/core/hashline.test.ts index 665656e85..d54f5d289 100644 --- a/packages/coding-agent/test/core/hashline.test.ts +++ b/packages/coding-agent/test/core/hashline.test.ts @@ -459,11 +459,45 @@ describe("hashline parser — suffix-op syntax", () => { ]); }); - it("still rejects two replace ops with non-identical overlapping ranges", () => { - const diff = `${tag(2, "bbb")}-${tag(4, "ddd")}:NEW1\n${tag(3, "ccc")}-${tag(4, "ddd")}:NEW2`; + it("still rejects two replace ops whose ranges partially overlap without containment", () => { + // 3-5 extends past the outer 2-4, so it is neither identical nor contained. + // The inner anchors still clash with the outer range's deletes and the + // post-hoc validator catches the overlap. + const diff = `${tag(2, "bbb")}-${tag(4, "ddd")}:NEW1\n${tag(3, "ccc")}-${tag(5, "eee")}:NEW2`; expect(() => parseHashline(diff).edits).toThrow(/anchor line 3 is already targeted by the .+ op on line 1/); }); + it("demotes a single-line `N:` op inside a pending `A-B:` to a payload line", () => { + const diff = `${tag(2, "bbb")}-${tag(4, "ddd")}:line one\n${tag(3, "ccc")}:line two\n${tag(4, "ddd")}:line three`; + const { edits, warnings } = parseHashline(diff); + expect(applyHashlineEdits("aaa\nbbb\nccc\nddd\neee", edits).lines).toBe( + "aaa\nline one\nline two\nline three\neee", + ); + expect(warnings).toEqual([ + "Detected one or more `LINE:TEXT` lines whose anchors fell inside the pending replace range; treated them as payload-continuation lines and stripped the `LINE:` prefix. Inside a multi-line `A-B:` block, payload lines after the first do not need a line-number prefix.", + ]); + }); + + it("demotes a sub-range `A-B:` inside a pending outer `A-B:` to a payload line", () => { + const diff = `${tag(2, "bbb")}-${tag(5, "eee")}:line one\n${tag(3, "ccc")}-${tag(4, "ddd")}:collapsed pair`; + const { edits, warnings } = parseHashline(diff); + expect(applyHashlineEdits("aaa\nbbb\nccc\nddd\neee\nfff", edits).lines).toBe( + "aaa\nline one\ncollapsed pair\nfff", + ); + expect(warnings).toEqual([ + "Detected one or more `LINE:TEXT` lines whose anchors fell inside the pending replace range; treated them as payload-continuation lines and stripped the `LINE:` prefix. Inside a multi-line `A-B:` block, payload lines after the first do not need a line-number prefix.", + ]); + }); + + it("treats `N:` outside the pending range as a separate op (no demote)", () => { + const diff = `${tag(2, "bbb")}-${tag(3, "ccc")}:line one\n${tag(5, "eee")}:line five`; + const { edits, warnings } = parseHashline(diff); + expect(applyHashlineEdits("aaa\nbbb\nccc\nddd\neee\nfff", edits).lines).toBe( + "aaa\nline one\nddd\nline five\nfff", + ); + expect(warnings).toEqual([]); + }); + it("rejects a replace overlapping a later delete", () => { const diff = `${tag(2, "bbb")}-${tag(4, "ddd")}:X\n${tag(3, "ccc")}!`; expect(() => parseHashline(diff).edits).toThrow(/anchor line 3 is already targeted by the .+ op on line 1/);