diff --git a/packages/hashline/CHANGELOG.md b/packages/hashline/CHANGELOG.md index 4d5c90dbd..91efd9d37 100644 --- a/packages/hashline/CHANGELOG.md +++ b/packages/hashline/CHANGELOG.md @@ -1,9 +1,11 @@ # Changelog ## [Unreleased] - ### Fixed +- Improved delimiter-balance repair to correctly identify and spare deleted structural closers +- Prevented premature deletion of structural closers when existing code below the range covers them +- Accurate tracking of inserted lines to improve boundary repair logic for surrounding code blocks - Fixed delimiter-balance repair so deleted closer suffixes are kept only when the replacement prefix still has unmatched openers for them, avoiding duplicated trailing braces while preserving omitted outer closers. ## [16.1.8] - 2026-06-20 @@ -297,4 +299,4 @@ All notable changes to this package will be documented in this file. - Fixed repeated patch application mutating cached `after_anchor` edits between target snapshots - Fixed multi-section patching to preflight write policies and reject duplicate canonical targets before any section is committed -- Fixed mixed line-ending restoration to preserve the first newline style instead of rewriting ties to LF +- Fixed mixed line-ending restoration to preserve the first newline style instead of rewriting ties to LF \ No newline at end of file diff --git a/packages/hashline/src/apply.ts b/packages/hashline/src/apply.ts index e9295a9a6..cc92a940a 100644 --- a/packages/hashline/src/apply.ts +++ b/packages/hashline/src/apply.ts @@ -430,38 +430,167 @@ function findDuplicatePrefix(group: ReplacementGroup, fileLines: readonly string } return 0; } +interface DroppedSuffixClosers { + readonly startLine: number; + readonly count: number; + readonly balance: DelimiterBalance; +} + +function countPayloadRestatedSuffixHead(payload: readonly string[], suffixLines: readonly string[]): number { + const maxCount = Math.min(payload.length, suffixLines.length); + for (let count = maxCount; count >= 1; count--) { + let matches = true; + for (let offset = 0; offset < count; offset++) { + if (payload[payload.length - count + offset] !== suffixLines[offset]) { + matches = false; + break; + } + } + if (matches) return count; + } + return 0; +} + +function countProjectedBelowSuffixTail( + group: ReplacementGroup, + fileLines: readonly string[], + deletedLines: ReadonlySet, + insertedLineMaps: InsertedLineMaps, + suffixLines: readonly string[], +): number { + const below: string[] = []; + const appendCloserLines = (lines: readonly string[] | undefined): boolean => { + if (!lines) return true; + for (const text of lines) { + if (!STRUCTURAL_CLOSER_RE.test(text)) return false; + below.push(text); + } + return true; + }; + if (!appendCloserLines(insertedLineMaps.after.get(group.endLine))) return 0; + for (let line = group.endLine + 1; line <= fileLines.length; line++) { + if (!appendCloserLines(insertedLineMaps.before.get(line))) break; + if (!deletedLines.has(line)) { + const text = fileLines[line - 1] ?? ""; + if (!STRUCTURAL_CLOSER_RE.test(text)) break; + below.push(text); + } + if (!appendCloserLines(insertedLineMaps.after.get(line))) break; + } + const maxCount = Math.min(below.length, suffixLines.length); + for (let count = maxCount; count >= 1; count--) { + let matches = true; + for (let offset = 0; offset < count; offset++) { + if (below[offset] !== suffixLines[suffixLines.length - count + offset]) { + matches = false; + break; + } + } + if (matches) return count; + } + return 0; +} + +interface InsertedLineMaps { + readonly before: ReadonlyMap; + readonly after: ReadonlyMap; +} + +function computeProjectedPrefixBalance( + group: ReplacementGroup, + fileLines: readonly string[], + deletedLines: ReadonlySet, + insertedByLine: ReadonlyMap, + insertedLineMaps: InsertedLineMaps, +): DelimiterBalance { + const prefix: string[] = []; + for (let line = 1; line < group.startLine; line++) { + const inserted = insertedByLine.get(line); + if (inserted) prefix.push(...inserted); + if (!deletedLines.has(line)) prefix.push(fileLines[line - 1] ?? ""); + } + const insertedAtStart = insertedLineMaps.before.get(group.startLine); + if (insertedAtStart) prefix.push(...insertedAtStart); + prefix.push(...group.payload); + return computeDelimiterBalance(prefix); +} function prefixCanCoverSuffixClosers( group: ReplacementGroup, fileLines: readonly string[], suffixBalance: DelimiterBalance, + coveredBelowBalance: DelimiterBalance, + deletedLines: ReadonlySet, + insertedByLine: ReadonlyMap, + insertedLineMaps: InsertedLineMaps, ): boolean { const neededOpeners = balanceNegate(suffixBalance); - const prefixBalance = computeDelimiterBalance([...fileLines.slice(0, group.startLine - 1), ...group.payload]); - return balanceCovers(prefixBalance, neededOpeners); + const prefixBalance = computeProjectedPrefixBalance( + group, + fileLines, + deletedLines, + insertedByLine, + insertedLineMaps, + ); + const uncoveredPrefixBalance = balanceSum(prefixBalance, coveredBelowBalance); + return balanceCovers(uncoveredPrefixBalance, neededOpeners); } /** - * Smallest `m` such that the range's last `m` deleted lines are all pure - * structural closers, sparing them (keeping instead of deleting) zeroes `delta`, - * and the final prefix before the spared suffix still has unmatched openers for - * those closers. The mirror mistake: a range that swallows a closing delimiter - * the payload never restates. + * Missing segment of the range's deleted structural-closer suffix that should + * be spared. Payload lines that already restate the suffix head are not kept + * again, and projected closers immediately below the range satisfy the suffix + * tail. The remaining middle segment is kept only when backed by unmatched + * openers plus the whole-patch residual. */ 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; - const suffixBalance = computeDelimiterBalance(fileLines.slice(group.endLine - m, group.endLine)); - if (!balanceEqual(suffixBalance, wanted)) continue; - if (prefixCanCoverSuffixClosers(group, fileLines, suffixBalance)) return m; + remainingDelta: DelimiterBalance, + deletedPrefixBalance: DelimiterBalance, + deletedLines: ReadonlySet, + insertedByLine: ReadonlyMap, + insertedLineMaps: InsertedLineMaps, +): DroppedSuffixClosers | undefined { + let suffixLength = 0; + while ( + suffixLength < group.deleteIndices.length && + STRUCTURAL_CLOSER_RE.test(fileLines[group.endLine - suffixLength - 1] ?? "") + ) { + suffixLength++; } - return 0; + if (suffixLength === 0) return undefined; + + const suffixStartLine = group.endLine - suffixLength + 1; + const suffixLines = fileLines.slice(group.endLine - suffixLength, group.endLine); + const restatedHead = countPayloadRestatedSuffixHead(group.payload, suffixLines); + const coveredTail = countProjectedBelowSuffixTail(group, fileLines, deletedLines, insertedLineMaps, suffixLines); + const keepStart = restatedHead; + const keepEnd = suffixLength - coveredTail; + if (keepStart >= keepEnd) return undefined; + + const keptLines = suffixLines.slice(keepStart, keepEnd); + const keptBalance = computeDelimiterBalance(keptLines); + const neededOpeners = balanceNegate(keptBalance); + const coveredBelowBalance = computeDelimiterBalance(suffixLines.slice(keepEnd)); + if (!balanceCovers(delta, neededOpeners)) return undefined; + if (balanceCovers(deletedPrefixBalance, neededOpeners)) return undefined; + if (!balanceCovers(remainingDelta, neededOpeners)) return undefined; + if ( + !prefixCanCoverSuffixClosers( + group, + fileLines, + keptBalance, + coveredBelowBalance, + deletedLines, + insertedByLine, + insertedLineMaps, + ) + ) { + return undefined; + } + return { startLine: suffixStartLine + keepStart, count: keepEnd - keepStart, balance: keptBalance }; } interface BoundaryEcho { @@ -755,10 +884,6 @@ function repairReplacementBoundaries( slots.push({ kind: "candidate", group, inserts, deletes, delta }); } - // Pass 2: project the post-pass-1 stream to learn which lines are deleted and - // what is inserted where, sum the whole-patch residual balance bleed-free, - // then resolve each deferred missing-closer candidate against that residual, - // consuming it per repair so one missing closer is never spent twice. const projected: AppliedEdit[] = []; for (const slot of slots) { projected.push(...(slot.kind === "candidate" ? [...slot.inserts, ...slot.deletes] : slot.edits)); @@ -768,6 +893,10 @@ function repairReplacementBoundaries( if (edit.kind === "delete") deletedLines.add(edit.anchor.line); } const insertedByLine = new Map(); + const insertedLineMaps: { before: Map; after: Map } = { + before: new Map(), + after: new Map(), + }; for (const edit of projected) { if (edit.kind !== "insert") continue; for (const anchor of getCursorAnchors(edit.cursor)) { @@ -775,6 +904,12 @@ function repairReplacementBoundaries( if (lines) lines.push(edit.text); else insertedByLine.set(anchor.line, [edit.text]); } + if (edit.cursor.kind === "before_anchor" || edit.cursor.kind === "after_anchor") { + const bySide = edit.cursor.kind === "before_anchor" ? insertedLineMaps.before : insertedLineMaps.after; + const lines = bySide.get(edit.cursor.anchor.line); + if (lines) lines.push(edit.text); + else bySide.set(edit.cursor.anchor.line, [edit.text]); + } } let remainingDelta: DelimiterBalance = { paren: 0, bracket: 0, brace: 0 }; for (const slot of slots) remainingDelta = balanceSum(remainingDelta, slotPatchDelta(slot, fileLines)); @@ -787,20 +922,37 @@ function repairReplacementBoundaries( out.push(...slot.edits); continue; } - const prefixBalance = netDeletedPrefixBalance(slot.group, deletedLines, insertedByLine, fileLines); - const droppedClosers = - !balanceCovers(prefixBalance, slot.delta) && balanceCovers(remainingDelta, slot.delta) - ? findDroppedSuffixClosers(slot.group, fileLines, slot.delta) - : 0; - if (droppedClosers > 0) { + const deletedPrefixBalance = netDeletedPrefixBalance(slot.group, deletedLines, insertedByLine, fileLines); + const droppedClosers = findDroppedSuffixClosers( + slot.group, + fileLines, + slot.delta, + remainingDelta, + deletedPrefixBalance, + deletedLines, + insertedByLine, + insertedLineMaps, + ); + if (droppedClosers) { warnings.push( describeBoundaryRepair( slot.group, - `kept ${droppedClosers} structural closing line(s) the range deleted without restating`, + `kept ${droppedClosers.count} structural closing line(s) the range deleted without restating`, ), ); - out.push(...slot.inserts, ...slot.deletes.slice(0, slot.deletes.length - droppedClosers)); - remainingDelta = balanceDelta(remainingDelta, slot.delta); + out.push( + ...slot.inserts, + ...slot.deletes.filter( + edit => + edit.kind !== "delete" || + edit.anchor.line < droppedClosers.startLine || + edit.anchor.line >= droppedClosers.startLine + droppedClosers.count, + ), + ); + for (let line = droppedClosers.startLine; line < droppedClosers.startLine + droppedClosers.count; line++) { + deletedLines.delete(line); + } + remainingDelta = balanceSum(remainingDelta, droppedClosers.balance); continue; } out.push(...slot.inserts, ...slot.deletes); diff --git a/packages/hashline/test/boundary-repair.test.ts b/packages/hashline/test/boundary-repair.test.ts index e0f263ec6..cfbadd92c 100644 --- a/packages/hashline/test/boundary-repair.test.ts +++ b/packages/hashline/test/boundary-repair.test.ts @@ -403,6 +403,95 @@ describe("boundary-balance repair", () => { expect(warnings.filter(warning => /structural closing line/.test(warning))).toHaveLength(0); }); + it("keeps only the non-restated outer closer for a nested deleted suffix", () => { + const file = ["class C {", "\told();", "\t}", "}"].join("\n"); + const diff = ["SWAP 2.=4:", "+\tnewMethod() {", "+\t\treturn 1;", "+\t}"].join("\n"); + const { text, warnings } = apply(file, diff); + expect(text).toBe(["class C {", "\tnewMethod() {", "\t\treturn 1;", "\t}", "}"].join("\n")); + expect(warnings.filter(warning => /structural closing line/.test(warning))).toHaveLength(1); + }); + + it("ignores non-contiguously deleted openers when choosing which closer to keep", () => { + const file = ["if (a) {", "\told();", "\tmore();", "}", "const obj = {", "\ta: 1,", "};"].join("\n"); + const diff = ["DEL 1", "SWAP 3.=4:", "+\tnew();", "SWAP 7.=7:", "+\tb: 2,"].join("\n"); + const { text, warnings } = apply(file, diff); + expect(text).toBe(["\told();", "\tnew();", "const obj = {", "\ta: 1,", "\tb: 2,", "};"].join("\n")); + expect(warnings.filter(warning => /structural closing line/.test(warning))).toHaveLength(1); + }); + + it("counts earlier kept closers in later projected prefixes", () => { + const file = [ + "if (a) {", + "\told();", + "}", + "const NO_REASONING_LABEL_PATTERN = /no/i;", + "\treturn config.supportsThinking === true;", + "\t}", + ].join("\n"); + const diff = [ + "SWAP 2.=3:", + "+\tnew();", + "SWAP 4.=6:", + "+function supportsDevinThinking(config: ClientModelConfig): boolean {", + "+\treturn config.supportsThinking === true;", + "+}", + ].join("\n"); + const { text, warnings } = apply(file, diff); + expect(text).toBe( + [ + "if (a) {", + "\tnew();", + "}", + "function supportsDevinThinking(config: ClientModelConfig): boolean {", + "\treturn config.supportsThinking === true;", + "}", + ].join("\n"), + ); + expect(warnings.filter(warning => /structural closing line/.test(warning))).toHaveLength(1); + }); + + it("does not let an earlier kept closer cover a later orphan closer", () => { + const file = ["if (a) {", "\told();", "}", "}"].join("\n"); + const diff = ["SWAP 2.=3:", "+\tnew();", "SWAP 4.=4:", "+after();"].join("\n"); + const { text, warnings } = apply(file, diff); + expect(text).toBe(["if (a) {", "\tnew();", "}", "after();"].join("\n")); + expect(warnings.filter(warning => /structural closing line/.test(warning))).toHaveLength(1); + }); + + it("does not keep a deleted outer closer when one survives below the range", () => { + const file = ["class C {", "\tmethod() {", "\t\told();", "\t}", "}", "}"].join("\n"); + const diff = ["SWAP 2.=5:", "+\tmethod() {", "+\t\tnew();", "+\t}"].join("\n"); + const { text, warnings } = apply(file, diff); + expect(text).toBe(["class C {", "\tmethod() {", "\t\tnew();", "\t}", "}"].join("\n")); + expect(warnings.filter(warning => /structural closing line/.test(warning))).toHaveLength(0); + }); + + it("keeps an omitted inner closer when the outer closer survives below", () => { + const file = ["class C {", "\tmethod() {", "\t\told();", "\t}", "}", "}"].join("\n"); + const diff = ["SWAP 2.=5:", "+\tmethod() {", "+\t\tnew();"].join("\n"); + const { text, warnings } = apply(file, diff); + expect(text).toBe(["class C {", "\tmethod() {", "\t\tnew();", "\t}", "}"].join("\n")); + expect(warnings.filter(warning => /structural closing line/.test(warning))).toHaveLength(1); + }); + + it("counts same-line inserted prefixes before replacement payload", () => { + const file = ["\told();", "}"].join("\n"); + const diff = ["INS.PRE 1:", "+if (a) {", "SWAP 1.=2:", "+\tnew();"].join("\n"); + const { text, warnings } = apply(file, diff); + expect(text).toBe(["if (a) {", "\tnew();", "}"].join("\n")); + expect(warnings.filter(warning => /structural closing line/.test(warning))).toHaveLength(1); + }); + + it("counts a separately inserted closer immediately below the range", () => { + const file = ["class C {", "\told();", "}", "after();", "const obj = {", "\ta: 1,", "};"].join("\n"); + const diff = ["SWAP 2.=3:", "+\tnew();", "INS.PRE 4:", "+}", "SWAP 7.=7:", "+\tb: 2,"].join("\n"); + const { text, warnings } = apply(file, diff); + expect(text).toBe( + ["class C {", "\tnew();", "}", "after();", "const obj = {", "\ta: 1,", "\tb: 2,", "};"].join("\n"), + ); + expect(warnings.filter(warning => /structural closing line/.test(warning))).toHaveLength(1); + }); + it("keeps an omitted outer closer even when the payload restates an inner closer", () => { const file = ["if (a) {", "\tif (b) {", "\t\told();", "\t}", "}", "after();"].join("\n"); const diff = ["SWAP 1.=5:", "+if (a) {", "+\tif (c) {", "+\t\tnew();", "+\t}"].join("\n");