fix(hashline): restored balance-validated boundary repair in applyEdits

- Replacement hunks now drop duplicated closing delimiters restated in the payload but surviving just outside the range.
- Ranges that swallow a structural closer the payload omits now spare that closer instead of deleting it.
- Each repair surfaces a `delimiter-balance` warning through `ApplyResult.warnings`.
- Brackets inside strings, template literals, and comments are skipped to avoid false positives.
This commit is contained in:
can1357
2026-05-29 16:20:57 +02:00
parent b8538faaa2
commit 0bc83f04ff
6 changed files with 498 additions and 5 deletions
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### Fixed
- Restored automatic repair of `edit` range hunks that break bracket balance — the failure class that previously left a duplicated closing line (a `</>` / `);` / `}` echoed just below the range) or dropped one (the range swallowed a `});` the payload never restated), leaving the file syntactically broken until a follow-up edit. The hashline applier now normalizes each replacement so its payload preserves the deleted region's delimiter balance, dropping a duplicated bordering closer or sparing a deleted one, and surfaces a warning on the tool result. Always on and balance-validated (no `edit.hashlineAutoDropPureInsertDuplicates` setting); see `@oh-my-pi/hashline` for the contract.
## [15.5.12] - 2026-05-29
### Added
@@ -313,14 +313,20 @@ describe("hashline parser — range-anchor syntax", () => {
expect(applyDiff(suffixSource, suffixDiff)).toBe(["new();", "// one", "// two", "// one", "// two"].join("\n"));
});
it("keeps duplicated structural replacement boundaries literal", () => {
it("de-duplicates structural replacement boundaries (balance-validated)", () => {
// `1 1` replaces `old();` but the payload also restates the `};` that
// survives at line 2 — a duplicate close that would unbalance braces.
const suffixSource = ["old();", "};"].join("\n");
const suffixDiff = [`${sameLineRange(tag(1, "old();"))}`, repl("new();"), repl("};")].join("\n");
expect(applyDiff(suffixSource, suffixDiff)).toBe(["new();", "};", "};"].join("\n"));
expect(applyDiff(suffixSource, suffixDiff)).toBe(["new();", "};"].join("\n"));
// Mirror case at the leading edge.
const prefixSource = ["};", "old();"].join("\n");
const prefixDiff = [`${sameLineRange(tag(2, "old();"))}`, repl("};"), repl("new();")].join("\n");
expect(applyDiff(prefixSource, prefixDiff)).toBe(["};", "};", "new();"].join("\n"));
expect(applyDiff(prefixSource, prefixDiff)).toBe(["};", "new();"].join("\n"));
const result = applyHashlineEdits(suffixSource, parseHashline(suffixDiff).edits);
expect(result.warnings?.some(w => /delimiter-balance/.test(w))).toBe(true);
});
it("keeps duplicated single non-structural replacement boundaries literal", () => {
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### Added
- Re-introduced balance-validated boundary repair in `applyEdits`. A replacement hunk (`A B` + body) is normalized so its payload preserves the deleted region's delimiter balance: when the body restates a closing delimiter that survives just outside the range (duplicate `}` / `);` / `]`) the echo is dropped, and when the range deletes a structural closer the body never restates (missing closer) the closer is spared instead of deleted. A repair fires only when one boundary operation drives the per-channel `()` / `[]` / `{}` imbalance to exactly zero while leaving surrounding text byte-identical (single-line ops are limited to pure structural-closer lines), so balance-preserving edits and intentional balanced duplicates are never touched. Bracket counting skips strings, template literals, and comments. Each repair surfaces a `delimiter-balance` warning through `ApplyResult.warnings`.
## [15.5.12] - 2026-05-29
### Changed
+314 -2
View File
@@ -1,6 +1,11 @@
/**
* Apply a parsed list of {@link Edit}s to a text body and return the
* post-edit lines. Pure function: no FS, no mutation of the input.
* post-edit lines plus any diagnostic warnings. Pure function: no FS, no
* mutation of the input.
*
* Replacement groups are first normalized by {@link repairBoundaryBalance},
* which fixes the common model mistake of a payload that duplicates or drops
* the closing delimiter bordering the range (balance-validated; see below).
*/
import { cloneCursor } from "./tokenizer";
import type { Anchor, ApplyResult, Cursor, Edit } from "./types";
@@ -132,6 +137,311 @@ function bucketAnchorEditsByLine(edits: IndexedEdit[]): Map<number, IndexedEdit[
return byLine;
}
// ═══════════════════════════════════════════════════════════════════════════
// Boundary-balance repair
//
// Models routinely miscount a replacement range's edges. The payload either
// re-states a closing delimiter that still lives just outside the range
// (producing a DUPLICATE `}` / `);` / `]`) or the range deletes a closer the
// payload never restates (DROPPING it). Both are the same defect — a
// replacement whose payload does not preserve the deleted region's delimiter
// balance — and both leave the file syntactically broken.
//
// A repair fires only when (a) the group's payload balance differs from the
// deleted region's balance and (b) one boundary operation drives that
// difference to exactly zero while leaving the surrounding text byte-identical.
// The operation only ever drops an exact multi-line boundary echo or a single
// pure structural-closer line, or spares a deleted pure structural-closer line,
// so content lines are never moved or lost. Balance-preserving edits are left
// strictly alone.
/** A line that is nothing but closing delimiters: `}`, `)`, `];`, `})`, `},`. */
const STRUCTURAL_CLOSER_RE = /^\s*[)\]}]+[;,]?\s*$/;
interface DelimiterBalance {
paren: number;
bracket: number;
brace: number;
}
/**
* Net `()` / `[]` / `{}` delta across `lines`, skipping delimiters inside line
* comments (`//`), block comments, and string/template literals. Block-comment
* and backtick-template state carry across lines; `"` / `'` reset at EOL since
* they cannot span lines. Deliberately language-light: constructs it cannot
* classify (e.g. regex literals) are counted naively, which can only suppress a
* repair (the safe direction), never force one.
*/
function computeDelimiterBalance(lines: readonly string[]): DelimiterBalance {
const balance: DelimiterBalance = { paren: 0, bracket: 0, brace: 0 };
let inBlockComment = false;
let quote = "";
for (const line of lines) {
for (let i = 0; i < line.length; i++) {
const ch = line[i];
if (inBlockComment) {
if (ch === "*" && line[i + 1] === "/") {
inBlockComment = false;
i++;
}
continue;
}
if (quote) {
if (ch === "\\") i++;
else if (ch === quote) quote = "";
continue;
}
if (ch === '"' || ch === "'" || ch === "`") {
quote = ch;
continue;
}
if (ch === "/" && line[i + 1] === "/") break;
if (ch === "/" && line[i + 1] === "*") {
inBlockComment = true;
i++;
continue;
}
switch (ch) {
case "(":
balance.paren++;
break;
case ")":
balance.paren--;
break;
case "[":
balance.bracket++;
break;
case "]":
balance.bracket--;
break;
case "{":
balance.brace++;
break;
case "}":
balance.brace--;
break;
}
}
// `"` / `'` cannot span lines; only backtick templates and block comments do.
if (quote === '"' || quote === "'") quote = "";
}
return balance;
}
function balanceDelta(a: DelimiterBalance, b: DelimiterBalance): DelimiterBalance {
return { paren: a.paren - b.paren, bracket: a.bracket - b.bracket, brace: a.brace - b.brace };
}
function balanceNegate(a: DelimiterBalance): DelimiterBalance {
return { paren: -a.paren, bracket: -a.bracket, brace: -a.brace };
}
function balanceEqual(a: DelimiterBalance, b: DelimiterBalance): boolean {
return a.paren === b.paren && a.bracket === b.bracket && a.brace === b.brace;
}
function balanceIsZero(a: DelimiterBalance): boolean {
return a.paren === 0 && a.bracket === 0 && a.brace === 0;
}
interface ReplacementGroup {
/** Positions in the edit array of the payload inserts, in payload order. */
insertIndices: number[];
/** Positions in the edit array of the range deletes, ascending by line. */
deleteIndices: number[];
payload: string[];
/** First deleted line (1-indexed). */
startLine: number;
/** Last deleted line (1-indexed). */
endLine: number;
}
/**
* Detect a replacement group starting at `start`: a run of `before_anchor`
* replacement inserts sharing one source op line, immediately followed by the
* contiguous range deletes for that same op. Mirrors how the parser lowers an
* `A B` hunk with a body.
*/
function findReplacementGroup(edits: readonly AppliedEdit[], start: number): ReplacementGroup | undefined {
const first = edits[start];
if (first?.kind !== "insert" || first.mode !== "replacement" || first.cursor.kind !== "before_anchor") {
return undefined;
}
const { lineNum } = first;
const anchorLine = first.cursor.anchor.line;
const insertIndices: number[] = [];
const payload: string[] = [];
let i = start;
for (; i < edits.length; i++) {
const edit = edits[i];
if (edit.kind !== "insert" || edit.mode !== "replacement" || edit.lineNum !== lineNum) break;
if (edit.cursor.kind !== "before_anchor" || edit.cursor.anchor.line !== anchorLine) break;
insertIndices.push(i);
payload.push(edit.text);
}
const deleteIndices: number[] = [];
let expectedLine = anchorLine;
for (; i < edits.length; i++) {
const edit = edits[i];
if (edit.kind !== "delete" || edit.lineNum !== lineNum || edit.anchor.line !== expectedLine) break;
deleteIndices.push(i);
expectedLine++;
}
if (deleteIndices.length === 0) return undefined;
return {
insertIndices,
deleteIndices,
payload,
startLine: anchorLine,
endLine: anchorLine + deleteIndices.length - 1,
};
}
/**
* Largest `k` such that the payload's last `k` lines exactly equal the `k`
* surviving file lines just below the range AND dropping them zeroes `delta`.
* Single-line drops are limited to pure structural closers.
*/
function findDuplicateSuffix(group: ReplacementGroup, fileLines: readonly string[], delta: DelimiterBalance): number {
const { payload, endLine } = group;
const maxK = Math.min(payload.length, fileLines.length - endLine);
for (let k = maxK; k >= 1; k--) {
let matches = true;
for (let t = 0; t < k; t++) {
if (payload[payload.length - k + t] !== fileLines[endLine + t]) {
matches = false;
break;
}
}
if (!matches) continue;
if (k === 1 && !STRUCTURAL_CLOSER_RE.test(payload[payload.length - 1])) continue;
if (balanceEqual(computeDelimiterBalance(payload.slice(payload.length - k)), delta)) return k;
}
return 0;
}
/**
* Largest `j` such that the payload's first `j` lines exactly equal the `j`
* surviving file lines just above the range AND dropping them zeroes `delta`.
*/
function findDuplicatePrefix(group: ReplacementGroup, fileLines: readonly string[], delta: DelimiterBalance): number {
const { payload, startLine } = group;
const maxJ = Math.min(payload.length, startLine - 1);
for (let j = maxJ; j >= 1; j--) {
let matches = true;
for (let t = 0; t < j; t++) {
if (payload[t] !== fileLines[startLine - 1 - j + t]) {
matches = false;
break;
}
}
if (!matches) continue;
if (j === 1 && !STRUCTURAL_CLOSER_RE.test(payload[0])) continue;
if (balanceEqual(computeDelimiterBalance(payload.slice(0, j)), delta)) return j;
}
return 0;
}
/**
* Smallest `m` such that the range's last `m` deleted lines are all pure
* structural closers and sparing them (keeping instead of deleting) zeroes
* `delta`. The mirror mistake: a range that swallows a closing delimiter the
* payload never restates.
*/
function findDroppedSuffixClosers(
group: ReplacementGroup,
fileLines: readonly string[],
delta: DelimiterBalance,
): number {
const wanted = balanceNegate(delta);
const maxM = group.deleteIndices.length;
for (let m = 1; m <= maxM; m++) {
if (!STRUCTURAL_CLOSER_RE.test(fileLines[group.endLine - m] ?? "")) break;
if (balanceEqual(computeDelimiterBalance(fileLines.slice(group.endLine - m, group.endLine)), wanted)) return m;
}
return 0;
}
function describeBoundaryRepair(group: ReplacementGroup, action: string): string {
return (
`Auto-repaired a delimiter-balance mismatch in the replacement at line ${group.startLine}: ${action}. ` +
`Issue the payload as the final desired content only — never restate or omit a closing bracket bordering the range.`
);
}
/**
* Normalize each replacement group so its payload preserves the deleted
* region's delimiter balance. See the section header for the contract. Returns
* the (possibly trimmed) edit list plus one warning per repaired group.
*/
function repairBoundaryBalance(
edits: readonly AppliedEdit[],
fileLines: readonly string[],
): {
edits: AppliedEdit[];
warnings: string[];
} {
const out: AppliedEdit[] = [];
const warnings: string[] = [];
let i = 0;
while (i < edits.length) {
const group = findReplacementGroup(edits, i);
if (!group) {
out.push(edits[i]);
i++;
continue;
}
const inserts = group.insertIndices.map(idx => edits[idx]);
const deletes = group.deleteIndices.map(idx => edits[idx]);
i = group.deleteIndices[group.deleteIndices.length - 1] + 1;
const delta = balanceDelta(
computeDelimiterBalance(group.payload),
computeDelimiterBalance(fileLines.slice(group.startLine - 1, group.endLine)),
);
if (balanceIsZero(delta)) {
out.push(...inserts, ...deletes);
continue;
}
const dupSuffix = findDuplicateSuffix(group, fileLines, delta);
if (dupSuffix > 0) {
warnings.push(
describeBoundaryRepair(
group,
`dropped ${dupSuffix} duplicated trailing payload line(s) already present below the range`,
),
);
out.push(...inserts.slice(0, inserts.length - dupSuffix), ...deletes);
continue;
}
const dupPrefix = findDuplicatePrefix(group, fileLines, delta);
if (dupPrefix > 0) {
warnings.push(
describeBoundaryRepair(
group,
`dropped ${dupPrefix} duplicated leading payload line(s) already present above the range`,
),
);
out.push(...inserts.slice(dupPrefix), ...deletes);
continue;
}
const droppedClosers = findDroppedSuffixClosers(group, fileLines, delta);
if (droppedClosers > 0) {
warnings.push(
describeBoundaryRepair(
group,
`kept ${droppedClosers} structural closing line(s) the range deleted without restating`,
),
);
out.push(...inserts, ...deletes.slice(0, deletes.length - droppedClosers));
continue;
}
out.push(...inserts, ...deletes);
}
return { edits: out, warnings };
}
/**
* Apply a parsed list of edits to a text body. Pure function — no I/O.
*
@@ -151,12 +461,13 @@ export function applyEdits(text: string, edits: Edit[]): ApplyResult {
const targetEdits = expandRepeatEdits(edits, fileLines);
validateLineBounds(targetEdits, fileLines);
const { edits: repaired, warnings } = repairBoundaryBalance(targetEdits, fileLines);
// Partition edits into BOF, EOF, and anchor-targeted buckets.
const bofLines: string[] = [];
const eofLines: string[] = [];
const anchorEdits: IndexedEdit[] = [];
targetEdits.forEach((edit, idx) => {
repaired.forEach((edit, idx) => {
if (edit.kind === "insert" && edit.cursor.kind === "bof") {
bofLines.push(edit.text);
} else if (edit.kind === "insert" && edit.cursor.kind === "eof") {
@@ -213,5 +524,6 @@ export function applyEdits(text: string, edits: Edit[]): ApplyResult {
return {
text: fileLines.join("\n"),
firstChangedLine,
...(warnings.length > 0 ? { warnings } : {}),
};
}
+1
View File
@@ -30,6 +30,7 @@ Every file section starts with `¶PATH#HASH`. `HASH` is the snapshot tag from yo
- Line numbers refer to the ORIGINAL file and stay valid for the whole patch — they do not shift as your hunks land.
- An empty body **deletes** the selected range entirely. To replace lines A..B with completely new content, list the new content under the hunk header (do not write `&A..B` for the lines you are replacing).
- `@@` is NOT a hashline construct. Do not wrap headers in `@@ ... @@` — write the anchor bare.
- Keep `A B` aligned to your body: select exactly the lines the body replaces. Never restate a bordering closer (`}`, `);`, `]`) that survives just outside the range, and never let `A B` swallow a closer your body omits — either unbalances the file. (An obvious off-by-one closer is auto-repaired with a warning; still aim to get the range right.)
</rules>
@@ -0,0 +1,166 @@
import { describe, expect, it } from "bun:test";
import { applyEdits, InMemorySnapshotStore, parsePatch, Recovery } from "@oh-my-pi/hashline";
function apply(text: string, diff: string): { text: string; warnings: string[] } {
const result = applyEdits(text, parsePatch(diff).edits);
return { text: result.text, warnings: result.warnings ?? [] };
}
describe("boundary-balance repair", () => {
// The canonical incident: a range-replace whose payload restates the
// fragment + paren close that still live just below the range, doubling
// `</>` and `);`. `11 31` covers `const …` through the second `/>`.
it("drops a duplicated multi-line closing block (the Root.tsx incident)", () => {
const file = [
'import type React from "react";',
'import { Composition } from "remotion";',
'import { Sizzle, type SizzleProps } from "./compositions/Sizzle";',
'import { FPS, totalDurationInFrames } from "./lib/scenes";',
"",
"export const RemotionRoot: React.FC = () => {",
"\tconst durationInFrames = totalDurationInFrames();",
"\treturn (",
"\t\t<>",
"\t\t\t<Composition",
'\t\t\t\tid="Sizzle"',
"\t\t\t\tcomponent={Sizzle}",
"\t\t\t\tdurationInFrames={durationInFrames}",
"\t\t\t\twidth={1920}",
'\t\t\t\tdefaultProps={{ layout: "landscape" }}',
"\t\t\t/>",
"\t\t</>",
"\t);",
"};",
].join("\n");
// Range 7..16 = `const …` through the first `/>`; payload restates the
// `</>` + `);` that survive at lines 17-18.
const diff = [
"7 16",
"+\treturn (",
"+\t\t<>",
"+\t\t\t<Composition",
'+\t\t\t\tid="Sizzle"',
"+\t\t\t\tcomponent={Sizzle}",
"+\t\t\t\tdurationInFrames={durationInFrames}",
"+\t\t\t\twidth={1920}",
'+\t\t\t\tdefaultProps={{ layout: "landscape" } satisfies SizzleProps}',
"+\t\t\t/>",
"+\t\t</>",
"+\t);",
].join("\n");
const { text, warnings } = apply(file, diff);
// Exactly one `</>` and one `);` survive — no doubling.
expect(text.split("\n").filter(l => l.trim() === "</>")).toHaveLength(1);
expect(text.split("\n").filter(l => l.trim() === ");")).toHaveLength(1);
expect(text.endsWith("\t\t</>\n\t);\n};")).toBe(true);
expect(warnings.some(w => /delimiter-balance/.test(w))).toBe(true);
});
// Single structural-closer duplication: the range ends one line short and
// the payload restates the `});` that survives just below it.
it("drops a single duplicated structural closer (`});`)", () => {
const file = ["it('a', () => {", "\tsetup();", "\trun();", "});", "after();"].join("\n");
// `2 3` replaces the two body lines but the payload also restates the
// `});` at line 4, which survives — a duplicate close.
const diff = ["2 3", "+\tsetup2();", "+\trun2();", "+});"].join("\n");
const { text, warnings } = apply(file, diff);
expect(text).toBe(["it('a', () => {", "\tsetup2();", "\trun2();", "});", "after();"].join("\n"));
expect(warnings.some(w => /delimiter-balance/.test(w))).toBe(true);
});
// Genuine missing-closer: payload omits the trailing `});`.
it("spares the deleted closing line when the payload omits it", () => {
const file = ["const handlers = {", "\ta() {", "\t\treturn 1;", "\t},", "};"].join("\n");
// `5 5` is the final `};`. Model inserts a new method but forgets to
// restate `};`; sparing it keeps the object literal balanced.
const diff = ["5 5", "+\tb() {", "+\t\treturn 2;", "+\t},"].join("\n");
const { text, warnings } = apply(file, diff);
expect(text).toBe(
["const handlers = {", "\ta() {", "\t\treturn 1;", "\t},", "\tb() {", "\t\treturn 2;", "\t},", "};"].join(
"\n",
),
);
expect(warnings.some(w => /delimiter-balance/.test(w))).toBe(true);
});
// Balance-preserving edits are never touched, even when the payload's last
// line coincidentally equals the line just below the range.
it("leaves a balance-preserving replacement alone (no false positive)", () => {
const file = ["foo();", "bar();", "bar();", "baz();"].join("\n");
// Replace line 2 with two balanced statements; the tail `bar();` equals
// the surviving line 3 but the payload is balanced — must NOT be dropped.
const diff = ["2 2", "+qux();", "+bar();"].join("\n");
const { text, warnings } = apply(file, diff);
expect(text).toBe(["foo();", "qux();", "bar();", "bar();", "baz();"].join("\n"));
expect(warnings).toHaveLength(0);
});
// A duplicated full statement (balance-neutral) is left intact: dropping it
// could discard intended content, and it does not break syntax.
it("does not drop a balance-neutral duplicated statement", () => {
const file = ["a = 1;", "b = 2;", "c = 3;"].join("\n");
const diff = ["1 1", "+a = 1;", "+b = 2;"].join("\n");
const { text, warnings } = apply(file, diff);
expect(text).toBe(["a = 1;", "b = 2;", "b = 2;", "c = 3;"].join("\n"));
expect(warnings).toHaveLength(0);
});
// Brackets inside strings must not trigger a spurious balance mismatch.
it("ignores brackets inside string literals", () => {
const file = ['const a = "}";', 'const b = "x";', 'const c = "y";'].join("\n");
const diff = ["2 2", '+const b = "}}}";'].join("\n");
const { text, warnings } = apply(file, diff);
expect(text).toBe(['const a = "}";', 'const b = "}}}";', 'const c = "y";'].join("\n"));
expect(warnings).toHaveLength(0);
});
});
describe("boundary-balance repair through stale-snapshot recovery", () => {
const PATH = "/tmp/__hashline-boundary-recovery__.ts";
// Recovery composes `applyEdits` to compute the intended change, so the
// boundary repair runs there too. The snapshot (what the model read)
// carries the structure; the live file has drifted far from the edit
// region, so the stale-hash 3-way merge succeeds and the repaired
// (de-duplicated) hunk lands without doubling the closer.
it("de-duplicates a closer while recovering from a drifted file", () => {
const snapshotLines = [
'import { x } from "y";',
"",
"it('a', () => {",
"\tsetup();",
"\trun();",
"});",
"",
"function filler1() { return 1; }",
"function filler2() { return 2; }",
"function filler3() { return 3; }",
"function filler4() { return 4; }",
"function filler5() { return 5; }",
"const tail = 0;",
"export { tail };",
];
const snapshotText = `${snapshotLines.join("\n")}\n`;
// Live file drifted only at the tail (line 13) — far outside the edit
// region (lines 4-6), so the 3-way merge applies cleanly.
const currentText = snapshotText.replace("const tail = 0;", "const tail = 99;");
const store = new InMemorySnapshotStore();
const fileHash = store.recordContiguous(PATH, 1, snapshotText.split("\n"), { fullText: snapshotText });
// `4 5` replaces the body lines but the payload also restates the `});`
// that survives at line 6 — the duplicate-closer mistake.
const { edits } = parsePatch(["4 5", "+\tsetup2();", "+\trun2();", "+});"].join("\n"));
const recovered = new Recovery(store).tryRecover({ path: PATH, currentText, fileHash, edits });
expect(recovered).not.toBeNull();
// Exactly one `});` — the duplicate was absorbed during recovery.
expect(recovered?.text.split("\n").filter(l => l === "});")).toHaveLength(1);
expect(recovered?.text).toContain("setup2();");
expect(recovered?.text).toContain("run2();");
// The unrelated drift on the live file survives the merge.
expect(recovered?.text).toContain("const tail = 99;");
// The repair warning propagates out through the recovery result.
expect(recovered?.warnings.some(w => /delimiter-balance/.test(w))).toBe(true);
});
});