smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(coding-agent/hashline): resolved nested replace demotion in hashline

Browse Source 
- Handled an inner `replace` op that appears inside a pending multiline `A-B:` block by appending its body to the outer payload and preserving `LINE:`/`A-B:` body content as continuation.
- Retained same-range replace-pair coalescing while adding a warning when nested replace anchors are demoted from op markers to payload lines.
- Added hashline tests for nested payload demotion behavior, expected warnings, and non-demoted out-of-range `N:` replacements.
This commit is contained in:
can1357
2026-05-27 00:30:46 +02:00
parent 1612fb0df3
commit d409b6d2f6
3 changed files with 77 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/src/hashline/constants.ts
+11
View File
@@ -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.";
packages/coding-agent/src/hashline/executor.ts
+30 -15
View File
@@ -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();
packages/coding-agent/test/core/hashline.test.ts
+36 -2
View File
@@ -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/);