From 60cf2de16fc0f830e47faea9e4d0388f8393150f Mon Sep 17 00:00:00 2001 From: minpeter Date: Tue, 24 Feb 2026 14:46:17 +0900 Subject: [PATCH] fix(hashline-edit): detect overlapping ranges and prevent false unwrap of blank-line spans MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add detectOverlappingRanges() to reject edits with overlapping pos..end ranges instead of crashing with undefined.match() - Add bounds guard (?? "") in edit-operation-primitives for out-of-range line access - Add null guard in leadingWhitespace() for undefined/empty input - Fix restoreOldWrappedLines false unwrap: skip candidate spans containing blank/whitespace-only lines, preventing incorrect collapse of structural blank lines and indentation (the "애국가 bug") - Improve tool description for range replace clarity - Add tests: overlapping range detection, false unwrap prevention --- .../autocorrect-replacement-lines.ts | 5 ++- .../edit-operation-primitives.ts | 2 +- .../hashline-edit/edit-operations.test.ts | 45 +++++++++++++++++++ src/tools/hashline-edit/edit-operations.ts | 5 ++- src/tools/hashline-edit/edit-ordering.ts | 27 +++++++++++ .../hashline-edit/edit-text-normalization.ts | 1 + src/tools/hashline-edit/tool-description.ts | 4 +- 7 files changed, 84 insertions(+), 5 deletions(-) diff --git a/src/tools/hashline-edit/autocorrect-replacement-lines.ts b/src/tools/hashline-edit/autocorrect-replacement-lines.ts index 719f9d6c6..fe0c5c721 100644 --- a/src/tools/hashline-edit/autocorrect-replacement-lines.ts +++ b/src/tools/hashline-edit/autocorrect-replacement-lines.ts @@ -15,6 +15,7 @@ export function stripMergeOperatorChars(text: string): string { } function leadingWhitespace(text: string): string { + if (!text) return "" const match = text.match(/^\s*/) return match ? match[0] : "" } @@ -36,7 +37,9 @@ export function restoreOldWrappedLines(originalLines: string[], replacementLines const candidates: { start: number; len: number; replacement: string; canonical: string }[] = [] for (let start = 0; start < replacementLines.length; start += 1) { for (let len = 2; len <= 10 && start + len <= replacementLines.length; len += 1) { - const canonicalSpan = stripAllWhitespace(replacementLines.slice(start, start + len).join("")) + const span = replacementLines.slice(start, start + len) + if (span.some((line) => line.trim().length === 0)) continue + const canonicalSpan = stripAllWhitespace(span.join("")) const original = canonicalToOriginal.get(canonicalSpan) if (original && original.count === 1 && canonicalSpan.length >= 6) { candidates.push({ start, len, replacement: original.line, canonical: canonicalSpan }) diff --git a/src/tools/hashline-edit/edit-operation-primitives.ts b/src/tools/hashline-edit/edit-operation-primitives.ts index fc07c6113..43904011c 100644 --- a/src/tools/hashline-edit/edit-operation-primitives.ts +++ b/src/tools/hashline-edit/edit-operation-primitives.ts @@ -63,7 +63,7 @@ export function applyReplaceLines( const corrected = autocorrectReplacementLines(originalRange, stripped) const restored = corrected.map((entry, idx) => { if (idx !== 0) return entry - return restoreLeadingIndent(lines[startLine - 1], entry) + return restoreLeadingIndent(lines[startLine - 1] ?? "", entry) }) result.splice(startLine - 1, endLine - startLine + 1, ...restored) return result diff --git a/src/tools/hashline-edit/edit-operations.test.ts b/src/tools/hashline-edit/edit-operations.test.ts index d66c2d946..eb0adbaf0 100644 --- a/src/tools/hashline-edit/edit-operations.test.ts +++ b/src/tools/hashline-edit/edit-operations.test.ts @@ -236,6 +236,22 @@ describe("hashline edit operations", () => { expect(result).toEqual(["if (x) {", " return 3", " return 4", "}"]) }) + it("preserves blank lines and indentation in range replace (no false unwrap)", () => { + //#given — reproduces the 애국가 bug where blank+indented lines collapse + const lines = ["", "동해물과 백두산이 마르고 닳도록", "하느님이 보우하사 우리나라 만세", "", "무궁화 삼천리 화려강산", "대한사람 대한으로 길이 보전하세", ""] + + //#when — replace the range with indented version (blank lines preserved) + const result = applyReplaceLines( + lines, + anchorFor(lines, 1), + anchorFor(lines, 7), + ["", " 동해물과 백두산이 마르고 닳도록", " 하느님이 보우하사 우리나라 만세", "", " 무궁화 삼천리 화려강산", " 대한사람 대한으로 길이 보전하세", ""] + ) + + //#then — all 7 lines preserved with indentation, not collapsed to 3 + expect(result).toEqual(["", " 동해물과 백두산이 마르고 닳도록", " 하느님이 보우하사 우리나라 만세", "", " 무궁화 삼천리 화려강산", " 대한사람 대한으로 길이 보전하세", ""]) + }) + it("collapses wrapped replacement span back to unique original single line", () => { //#given const lines = [ @@ -353,4 +369,33 @@ describe("hashline edit operations", () => { //#then expect(result).toEqual(["const a = 10;", "const b = 20;"]) }) + + it("throws on overlapping range edits", () => { + //#given + const content = "line 1\nline 2\nline 3\nline 4\nline 5" + const lines = content.split("\n") + const edits: HashlineEdit[] = [ + { op: "replace", pos: anchorFor(lines, 1), end: anchorFor(lines, 3), lines: "replaced A" }, + { op: "replace", pos: anchorFor(lines, 2), end: anchorFor(lines, 4), lines: "replaced B" }, + ] + + //#when / #then + expect(() => applyHashlineEdits(content, edits)).toThrow(/overlapping/i) + }) + + it("allows non-overlapping range edits", () => { + //#given + const content = "line 1\nline 2\nline 3\nline 4\nline 5" + const lines = content.split("\n") + const edits: HashlineEdit[] = [ + { op: "replace", pos: anchorFor(lines, 1), end: anchorFor(lines, 2), lines: "replaced A" }, + { op: "replace", pos: anchorFor(lines, 4), end: anchorFor(lines, 5), lines: "replaced B" }, + ] + + //#when + const result = applyHashlineEdits(content, edits) + + //#then + expect(result).toEqual("replaced A\nline 3\nreplaced B") + }) }) diff --git a/src/tools/hashline-edit/edit-operations.ts b/src/tools/hashline-edit/edit-operations.ts index 0aa2e5390..8558b51db 100644 --- a/src/tools/hashline-edit/edit-operations.ts +++ b/src/tools/hashline-edit/edit-operations.ts @@ -1,5 +1,5 @@ import { dedupeEdits } from "./edit-deduplication" -import { collectLineRefs, getEditLineNumber } from "./edit-ordering" +import { collectLineRefs, detectOverlappingRanges, getEditLineNumber } from "./edit-ordering" import type { HashlineEdit } from "./types" import { applyAppend, @@ -36,6 +36,9 @@ export function applyHashlineEditsWithReport(content: string, edits: HashlineEdi const refs = collectLineRefs(sortedEdits) validateLineRefs(lines, refs) + const overlapError = detectOverlappingRanges(sortedEdits) + if (overlapError) throw new Error(overlapError) + for (const edit of sortedEdits) { switch (edit.op) { case "replace": { diff --git a/src/tools/hashline-edit/edit-ordering.ts b/src/tools/hashline-edit/edit-ordering.ts index f56587798..3a9444fc0 100644 --- a/src/tools/hashline-edit/edit-ordering.ts +++ b/src/tools/hashline-edit/edit-ordering.ts @@ -27,3 +27,30 @@ export function collectLineRefs(edits: HashlineEdit[]): string[] { } }) } + +export function detectOverlappingRanges(edits: HashlineEdit[]): string | null { + const ranges: { start: number; end: number; idx: number }[] = [] + for (let i = 0; i < edits.length; i++) { + const edit = edits[i] + if (edit.op !== "replace" || !edit.end) continue + const start = parseLineRef(edit.pos).line + const end = parseLineRef(edit.end).line + ranges.push({ start, end, idx: i }) + } + if (ranges.length < 2) return null + + ranges.sort((a, b) => a.start - b.start || a.end - b.end) + for (let i = 1; i < ranges.length; i++) { + const prev = ranges[i - 1] + const curr = ranges[i] + if (curr.start <= prev.end) { + return ( + `Overlapping range edits detected: ` + + `edit ${prev.idx + 1} (lines ${prev.start}-${prev.end}) overlaps with ` + + `edit ${curr.idx + 1} (lines ${curr.start}-${curr.end}). ` + + `Use pos-only replace for single-line edits.` + ) + } + } + return null +} diff --git a/src/tools/hashline-edit/edit-text-normalization.ts b/src/tools/hashline-edit/edit-text-normalization.ts index 9bae8cc46..7eaf1da24 100644 --- a/src/tools/hashline-edit/edit-text-normalization.ts +++ b/src/tools/hashline-edit/edit-text-normalization.ts @@ -7,6 +7,7 @@ function equalsIgnoringWhitespace(a: string, b: string): boolean { } function leadingWhitespace(text: string): string { + if (!text) return "" const match = text.match(/^\s*/) return match ? match[0] : "" } diff --git a/src/tools/hashline-edit/tool-description.ts b/src/tools/hashline-edit/tool-description.ts index 4a9a6dd5d..958509b7a 100644 --- a/src/tools/hashline-edit/tool-description.ts +++ b/src/tools/hashline-edit/tool-description.ts @@ -34,8 +34,8 @@ FILE CREATION: CRITICAL: only unanchored append/prepend can create a missing file. OPERATION CHOICE: - replace with pos only -> replace one line at pos - replace with pos+end -> replace range pos..end + replace with pos only -> replace one line at pos (MOST COMMON for single-line edits) + replace with pos+end -> replace ENTIRE range pos..end as a block (ranges MUST NOT overlap across edits) append with pos/end anchor -> insert after that anchor prepend with pos/end anchor -> insert before that anchor append/prepend without anchors -> EOF/BOF insertion