fix(hashline-edit): align autocorrect, BOM/CRLF, and tool description with oh-my-pi
- Rewrite restoreOldWrappedLines to use oh-my-pi's span-scanning algorithm - Add stripTrailingContinuationTokens and stripMergeOperatorChars helpers - Fix detectLineEnding to use first-occurrence logic instead of any-match - Fix applyAppend/applyPrepend to replace empty-line placeholder in empty files - Enhance tool description with 7 critical rules, tag guidance, and anti-patterns
This commit is contained in:
parent
5d1d87cc10
commit
e6868e9112
@ -2,18 +2,63 @@ function normalizeTokens(text: string): string {
|
|||||||
return text.replace(/\s+/g, "")
|
return text.replace(/\s+/g, "")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function stripAllWhitespace(text: string): string {
|
||||||
|
return normalizeTokens(text)
|
||||||
|
}
|
||||||
|
|
||||||
|
export function stripTrailingContinuationTokens(text: string): string {
|
||||||
|
return text.replace(/(?:&&|\|\||\?\?|\?|:|=|,|\+|-|\*|\/|\.|\()\s*$/u, "")
|
||||||
|
}
|
||||||
|
|
||||||
|
export function stripMergeOperatorChars(text: string): string {
|
||||||
|
return text.replace(/[|&?]/g, "")
|
||||||
|
}
|
||||||
|
|
||||||
function leadingWhitespace(text: string): string {
|
function leadingWhitespace(text: string): string {
|
||||||
const match = text.match(/^\s*/)
|
const match = text.match(/^\s*/)
|
||||||
return match ? match[0] : ""
|
return match ? match[0] : ""
|
||||||
}
|
}
|
||||||
|
|
||||||
export function restoreOldWrappedLines(originalLines: string[], replacementLines: string[]): string[] {
|
export function restoreOldWrappedLines(originalLines: string[], replacementLines: string[]): string[] {
|
||||||
if (replacementLines.length <= 1) return replacementLines
|
if (originalLines.length === 0 || replacementLines.length < 2) return replacementLines
|
||||||
if (originalLines.length !== replacementLines.length) return replacementLines
|
|
||||||
const original = normalizeTokens(originalLines.join("\n"))
|
const canonicalToOriginal = new Map<string, { line: string; count: number }>()
|
||||||
const replacement = normalizeTokens(replacementLines.join("\n"))
|
for (const line of originalLines) {
|
||||||
if (original !== replacement) return replacementLines
|
const canonical = stripAllWhitespace(line)
|
||||||
return originalLines
|
const existing = canonicalToOriginal.get(canonical)
|
||||||
|
if (existing) {
|
||||||
|
existing.count += 1
|
||||||
|
} else {
|
||||||
|
canonicalToOriginal.set(canonical, { line, count: 1 })
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
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 original = canonicalToOriginal.get(canonicalSpan)
|
||||||
|
if (original && original.count === 1 && canonicalSpan.length >= 6) {
|
||||||
|
candidates.push({ start, len, replacement: original.line, canonical: canonicalSpan })
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (candidates.length === 0) return replacementLines
|
||||||
|
|
||||||
|
const canonicalCounts = new Map<string, number>()
|
||||||
|
for (const candidate of candidates) {
|
||||||
|
canonicalCounts.set(candidate.canonical, (canonicalCounts.get(candidate.canonical) ?? 0) + 1)
|
||||||
|
}
|
||||||
|
|
||||||
|
const uniqueCandidates = candidates.filter((candidate) => (canonicalCounts.get(candidate.canonical) ?? 0) === 1)
|
||||||
|
if (uniqueCandidates.length === 0) return replacementLines
|
||||||
|
|
||||||
|
uniqueCandidates.sort((a, b) => b.start - a.start)
|
||||||
|
const correctedLines = [...replacementLines]
|
||||||
|
for (const candidate of uniqueCandidates) {
|
||||||
|
correctedLines.splice(candidate.start, candidate.len, candidate.replacement)
|
||||||
|
}
|
||||||
|
return correctedLines
|
||||||
}
|
}
|
||||||
|
|
||||||
export function maybeExpandSingleLineMerge(
|
export function maybeExpandSingleLineMerge(
|
||||||
|
|||||||
@ -134,6 +134,9 @@ export function applyAppend(lines: string[], text: string | string[]): string[]
|
|||||||
if (normalized.length === 0) {
|
if (normalized.length === 0) {
|
||||||
throw new Error("append requires non-empty text")
|
throw new Error("append requires non-empty text")
|
||||||
}
|
}
|
||||||
|
if (lines.length === 1 && lines[0] === "") {
|
||||||
|
return [...normalized]
|
||||||
|
}
|
||||||
return [...lines, ...normalized]
|
return [...lines, ...normalized]
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -142,6 +145,9 @@ export function applyPrepend(lines: string[], text: string | string[]): string[]
|
|||||||
if (normalized.length === 0) {
|
if (normalized.length === 0) {
|
||||||
throw new Error("prepend requires non-empty text")
|
throw new Error("prepend requires non-empty text")
|
||||||
}
|
}
|
||||||
|
if (lines.length === 1 && lines[0] === "") {
|
||||||
|
return [...normalized]
|
||||||
|
}
|
||||||
return [...normalized, ...lines]
|
return [...normalized, ...lines]
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -1,5 +1,6 @@
|
|||||||
import { describe, expect, it } from "bun:test"
|
import { describe, expect, it } from "bun:test"
|
||||||
import { applyHashlineEdits, applyInsertAfter, applyReplace, applyReplaceLines, applySetLine } from "./edit-operations"
|
import { applyHashlineEdits, applyInsertAfter, applyReplace, applyReplaceLines, applySetLine } from "./edit-operations"
|
||||||
|
import { applyAppend, applyPrepend } from "./edit-operation-primitives"
|
||||||
import { computeLineHash } from "./hash-computation"
|
import { computeLineHash } from "./hash-computation"
|
||||||
import type { HashlineEdit } from "./types"
|
import type { HashlineEdit } from "./types"
|
||||||
|
|
||||||
@ -249,6 +250,72 @@ describe("hashline edit operations", () => {
|
|||||||
expect(result).toEqual(["if (x) {", " return 3", " return 4", "}"])
|
expect(result).toEqual(["if (x) {", " return 3", " return 4", "}"])
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it("collapses wrapped replacement span back to unique original single line", () => {
|
||||||
|
//#given
|
||||||
|
const lines = [
|
||||||
|
"const request = buildRequest({ method: \"GET\", retries: 3 })",
|
||||||
|
"const done = true",
|
||||||
|
]
|
||||||
|
|
||||||
|
//#when
|
||||||
|
const result = applyReplaceLines(
|
||||||
|
lines,
|
||||||
|
anchorFor(lines, 1),
|
||||||
|
anchorFor(lines, 1),
|
||||||
|
["const request = buildRequest({", "method: \"GET\", retries: 3 })"]
|
||||||
|
)
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(result).toEqual([
|
||||||
|
"const request = buildRequest({ method: \"GET\", retries: 3 })",
|
||||||
|
"const done = true",
|
||||||
|
])
|
||||||
|
})
|
||||||
|
|
||||||
|
it("keeps wrapped replacement when canonical match is not unique in original lines", () => {
|
||||||
|
//#given
|
||||||
|
const lines = ["const query = a + b", "const query = a+b", "const done = true"]
|
||||||
|
|
||||||
|
//#when
|
||||||
|
const result = applyReplaceLines(lines, anchorFor(lines, 1), anchorFor(lines, 2), ["const query = a +", "b"])
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(result).toEqual(["const query = a +", "b", "const done = true"])
|
||||||
|
})
|
||||||
|
|
||||||
|
it("keeps wrapped replacement when same canonical candidate appears multiple times", () => {
|
||||||
|
//#given
|
||||||
|
const lines = ["const expression = alpha + beta + gamma", "const done = true"]
|
||||||
|
|
||||||
|
//#when
|
||||||
|
const result = applyReplaceLines(lines, anchorFor(lines, 1), anchorFor(lines, 1), [
|
||||||
|
"const expression = alpha +",
|
||||||
|
"beta + gamma",
|
||||||
|
"const expression = alpha +",
|
||||||
|
"beta + gamma",
|
||||||
|
])
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(result).toEqual([
|
||||||
|
"const expression = alpha +",
|
||||||
|
"beta + gamma",
|
||||||
|
"const expression = alpha +",
|
||||||
|
"beta + gamma",
|
||||||
|
"const done = true",
|
||||||
|
])
|
||||||
|
})
|
||||||
|
|
||||||
|
it("keeps wrapped replacement when canonical match is shorter than threshold", () => {
|
||||||
|
//#given
|
||||||
|
const lines = ["a + b", "const done = true"]
|
||||||
|
|
||||||
|
//#when
|
||||||
|
const result = applyReplaceLines(lines, anchorFor(lines, 1), anchorFor(lines, 1), ["a +", "b"])
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(result).toEqual(["a +", "b", "const done = true"])
|
||||||
|
})
|
||||||
|
|
||||||
it("applies append and prepend operations", () => {
|
it("applies append and prepend operations", () => {
|
||||||
//#given
|
//#given
|
||||||
const content = "line 1\nline 2"
|
const content = "line 1\nline 2"
|
||||||
@ -263,6 +330,28 @@ describe("hashline edit operations", () => {
|
|||||||
expect(result).toEqual("line 0\nline 1\nline 2\nline 3")
|
expect(result).toEqual("line 0\nline 1\nline 2\nline 3")
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it("appends to empty file without extra blank line", () => {
|
||||||
|
//#given
|
||||||
|
const lines = [""]
|
||||||
|
|
||||||
|
//#when
|
||||||
|
const result = applyAppend(lines, ["line1"])
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(result).toEqual(["line1"])
|
||||||
|
})
|
||||||
|
|
||||||
|
it("prepends to empty file without extra blank line", () => {
|
||||||
|
//#given
|
||||||
|
const lines = [""]
|
||||||
|
|
||||||
|
//#when
|
||||||
|
const result = applyPrepend(lines, ["line1"])
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(result).toEqual(["line1"])
|
||||||
|
})
|
||||||
|
|
||||||
it("autocorrects single-line merged replacement into original line count", () => {
|
it("autocorrects single-line merged replacement into original line count", () => {
|
||||||
//#given
|
//#given
|
||||||
const lines = ["const a = 1;", "const b = 2;"]
|
const lines = ["const a = 1;", "const b = 2;"]
|
||||||
|
|||||||
@ -5,7 +5,11 @@ export interface FileTextEnvelope {
|
|||||||
}
|
}
|
||||||
|
|
||||||
function detectLineEnding(content: string): "\n" | "\r\n" {
|
function detectLineEnding(content: string): "\n" | "\r\n" {
|
||||||
return content.includes("\r\n") ? "\r\n" : "\n"
|
const crlfIndex = content.indexOf("\r\n")
|
||||||
|
const lfIndex = content.indexOf("\n")
|
||||||
|
if (lfIndex === -1) return "\n"
|
||||||
|
if (crlfIndex === -1) return "\n"
|
||||||
|
return crlfIndex < lfIndex ? "\r\n" : "\n"
|
||||||
}
|
}
|
||||||
|
|
||||||
function stripBom(content: string): { content: string; hadBom: boolean } {
|
function stripBom(content: string): { content: string; hadBom: boolean } {
|
||||||
|
|||||||
@ -1,29 +1,23 @@
|
|||||||
export const HASHLINE_EDIT_DESCRIPTION = `Edit files using LINE#ID format for precise, safe modifications.
|
export const HASHLINE_EDIT_DESCRIPTION = `Edit files using LINE#ID format for precise, safe modifications.
|
||||||
|
|
||||||
WORKFLOW:
|
WORKFLOW:
|
||||||
1. Read the file and copy exact LINE#ID anchors.
|
1. Read target file/range and copy exact LINE#ID tags.
|
||||||
2. Submit one edit call with all related operations for that file.
|
2. Pick the smallest operation per logical mutation site.
|
||||||
3. If more edits are needed after success, use the latest anchors from read/edit output.
|
3. Submit one edit call per file with all related operations.
|
||||||
4. Use anchors as "LINE#ID" only (never include trailing ":content").
|
4. If same file needs another call, re-read first.
|
||||||
|
5. Use anchors as "LINE#ID" only (never include trailing ":content").
|
||||||
|
|
||||||
VALIDATION:
|
VALIDATION:
|
||||||
Payload shape: { "filePath": string, "edits": [...], "delete"?: boolean, "rename"?: string }
|
Payload shape: { "filePath": string, "edits": [...], "delete"?: boolean, "rename"?: string }
|
||||||
Each edit must be one of: set_line, replace_lines, insert_after, insert_before, insert_between, replace, append, prepend
|
Each edit must be one of: set_line, replace_lines, insert_after, insert_before, insert_between, replace, append, prepend
|
||||||
text/new_text must contain plain replacement text only (no LINE#ID prefixes, no diff + markers)
|
text/new_text must contain plain replacement text only (no LINE#ID prefixes, no diff + markers)
|
||||||
|
CRITICAL: all operations validate against the same pre-edit file snapshot and apply bottom-up.
|
||||||
|
|
||||||
LINE#ID FORMAT (CRITICAL - READ CAREFULLY):
|
LINE#ID FORMAT (CRITICAL):
|
||||||
Each line reference must be in "LINE#ID" format where:
|
Each line reference must be in "LINE#ID" format where:
|
||||||
LINE: 1-based line number
|
LINE: 1-based line number
|
||||||
ID: Two CID letters from the set ZPMQVRWSNKTXJBYH
|
ID: Two CID letters from the set ZPMQVRWSNKTXJBYH
|
||||||
|
|
||||||
OPERATION TYPES:
|
|
||||||
1. set_line
|
|
||||||
2. replace_lines
|
|
||||||
3. insert_after
|
|
||||||
4. insert_before
|
|
||||||
5. insert_between
|
|
||||||
6. replace
|
|
||||||
|
|
||||||
FILE MODES:
|
FILE MODES:
|
||||||
delete=true deletes file and requires edits=[] with no rename
|
delete=true deletes file and requires edits=[] with no rename
|
||||||
rename moves final content to a new path and removes old path
|
rename moves final content to a new path and removes old path
|
||||||
@ -36,17 +30,34 @@ CONTENT FORMAT:
|
|||||||
FILE CREATION:
|
FILE CREATION:
|
||||||
append: adds content at EOF. If file does not exist, creates it.
|
append: adds content at EOF. If file does not exist, creates it.
|
||||||
prepend: adds content at BOF. If file does not exist, creates it.
|
prepend: adds content at BOF. If file does not exist, creates it.
|
||||||
CRITICAL: append/prepend are the ONLY operations that work without an existing file.
|
CRITICAL: append/prepend are the only operations that work without an existing file.
|
||||||
|
|
||||||
OPERATION CHOICE:
|
OPERATION CHOICE:
|
||||||
One line wrong \u2192 set_line
|
One line wrong -> set_line
|
||||||
Block rewrite \u2192 replace_lines
|
Adjacent block rewrite or swap/move -> replace_lines (prefer one range op over many single-line ops)
|
||||||
New content between known anchors \u2192 insert_between (safest \u2014 dual-anchor pinning)
|
Both boundaries known -> insert_between (ALWAYS prefer over insert_after/insert_before)
|
||||||
New content at boundary \u2192 insert_after or insert_before
|
One boundary known -> insert_after or insert_before
|
||||||
New file or EOF/BOF addition \u2192 append or prepend
|
New file or EOF/BOF addition -> append or prepend
|
||||||
No LINE#ID available \u2192 replace (last resort)
|
No LINE#ID available -> replace (last resort)
|
||||||
|
|
||||||
AUTOCORRECT (built-in \u2014 you do NOT need to handle these):
|
RULES (CRITICAL):
|
||||||
|
1. Minimize scope: one logical mutation site per operation.
|
||||||
|
2. Preserve formatting: keep indentation, punctuation, line breaks, trailing commas, brace style.
|
||||||
|
3. Prefer insertion over neighbor rewrites: anchor to structural boundaries (}, ], },), not interior property lines.
|
||||||
|
4. No no-ops: replacement content must differ from current content.
|
||||||
|
5. Touch only requested code: avoid incidental edits.
|
||||||
|
6. Use exact current tokens: NEVER rewrite approximately.
|
||||||
|
7. For swaps/moves: prefer one range operation over multiple single-line operations.
|
||||||
|
|
||||||
|
TAG CHOICE (ALWAYS):
|
||||||
|
- Copy tags exactly from read output or >>> mismatch output.
|
||||||
|
- NEVER guess tags.
|
||||||
|
- Prefer insert_between over insert_after/insert_before when both boundaries are known.
|
||||||
|
- Anchor to structural lines (function/class/brace), NEVER blank lines.
|
||||||
|
- Anti-pattern warning: blank/whitespace anchors are fragile.
|
||||||
|
- Re-read after each successful edit call before issuing another on the same file.
|
||||||
|
|
||||||
|
AUTOCORRECT (built-in - you do NOT need to handle these):
|
||||||
Merged lines are auto-expanded back to original line count.
|
Merged lines are auto-expanded back to original line count.
|
||||||
Indentation is auto-restored from original lines.
|
Indentation is auto-restored from original lines.
|
||||||
BOM and CRLF line endings are preserved automatically.
|
BOM and CRLF line endings are preserved automatically.
|
||||||
|
|||||||
@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, mock } from "bun:test"
|
|||||||
import type { ToolContext } from "@opencode-ai/plugin/tool"
|
import type { ToolContext } from "@opencode-ai/plugin/tool"
|
||||||
import { createHashlineEditTool } from "./tools"
|
import { createHashlineEditTool } from "./tools"
|
||||||
import { computeLineHash } from "./hash-computation"
|
import { computeLineHash } from "./hash-computation"
|
||||||
|
import { canonicalizeFileText } from "./file-text-canonicalization"
|
||||||
import * as fs from "node:fs"
|
import * as fs from "node:fs"
|
||||||
import * as os from "node:os"
|
import * as os from "node:os"
|
||||||
import * as path from "node:path"
|
import * as path from "node:path"
|
||||||
@ -262,4 +263,26 @@ describe("createHashlineEditTool", () => {
|
|||||||
expect(bytes[2]).toBe(0xbf)
|
expect(bytes[2]).toBe(0xbf)
|
||||||
expect(bytes.toString("utf-8")).toBe("\uFEFFline1\r\nline2-updated\r\n")
|
expect(bytes.toString("utf-8")).toBe("\uFEFFline1\r\nline2-updated\r\n")
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it("detects LF as line ending when LF appears before CRLF", () => {
|
||||||
|
//#given
|
||||||
|
const content = "line1\nline2\r\nline3"
|
||||||
|
|
||||||
|
//#when
|
||||||
|
const envelope = canonicalizeFileText(content)
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(envelope.lineEnding).toBe("\n")
|
||||||
|
})
|
||||||
|
|
||||||
|
it("detects CRLF as line ending when CRLF appears before LF", () => {
|
||||||
|
//#given
|
||||||
|
const content = "line1\r\nline2\nline3"
|
||||||
|
|
||||||
|
//#when
|
||||||
|
const envelope = canonicalizeFileText(content)
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(envelope.lineEnding).toBe("\r\n")
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user