From 6ec0ff732ba6a231740bc29454463850ff571787 Mon Sep 17 00:00:00 2001 From: minpeter Date: Tue, 24 Feb 2026 03:00:38 +0900 Subject: [PATCH] refactor(hashline-edit): align tool payload to op/pos/end/lines Unify hashline_edit input with replace/append/prepend + pos/end/lines semantics so callers use a single stable shape. Add normalization coverage and refresh tool guidance/tests to reduce schema confusion and stale legacy payload usage. --- .../hashline-edit/normalize-edits.test.ts | 64 +++++ src/tools/hashline-edit/normalize-edits.ts | 218 ++++++++---------- src/tools/hashline-edit/tool-description.ts | 39 ++-- src/tools/hashline-edit/tools.test.ts | 51 ++-- src/tools/hashline-edit/tools.ts | 27 +-- 5 files changed, 210 insertions(+), 189 deletions(-) create mode 100644 src/tools/hashline-edit/normalize-edits.test.ts diff --git a/src/tools/hashline-edit/normalize-edits.test.ts b/src/tools/hashline-edit/normalize-edits.test.ts new file mode 100644 index 00000000..87cd05a6 --- /dev/null +++ b/src/tools/hashline-edit/normalize-edits.test.ts @@ -0,0 +1,64 @@ +import { describe, expect, it } from "bun:test" +import { normalizeHashlineEdits, type RawHashlineEdit } from "./normalize-edits" + +describe("normalizeHashlineEdits", () => { + it("maps replace with pos to set_line", () => { + //#given + const input: RawHashlineEdit[] = [{ op: "replace", pos: "2#VK", lines: "updated" }] + + //#when + const result = normalizeHashlineEdits(input) + + //#then + expect(result).toEqual([{ type: "set_line", line: "2#VK", text: "updated" }]) + }) + + it("maps replace with pos and end to replace_lines", () => { + //#given + const input: RawHashlineEdit[] = [{ op: "replace", pos: "2#VK", end: "4#MB", lines: ["a", "b"] }] + + //#when + const result = normalizeHashlineEdits(input) + + //#then + expect(result).toEqual([{ type: "replace_lines", start_line: "2#VK", end_line: "4#MB", text: ["a", "b"] }]) + }) + + it("maps anchored append and prepend to insert operations", () => { + //#given + const input: RawHashlineEdit[] = [ + { op: "append", pos: "2#VK", lines: ["after"] }, + { op: "prepend", pos: "4#MB", lines: ["before"] }, + ] + + //#when + const result = normalizeHashlineEdits(input) + + //#then + expect(result).toEqual([ + { type: "insert_after", line: "2#VK", text: ["after"] }, + { type: "insert_before", line: "4#MB", text: ["before"] }, + ]) + }) + + it("prefers pos over end for prepend anchors", () => { + //#given + const input: RawHashlineEdit[] = [{ op: "prepend", pos: "3#AA", end: "7#BB", lines: ["before"] }] + + //#when + const result = normalizeHashlineEdits(input) + + //#then + expect(result).toEqual([{ type: "insert_before", line: "3#AA", text: ["before"] }]) + }) + + it("rejects legacy payload without op", () => { + //#given + const input = [{ type: "set_line", line: "2#VK", text: "updated" }] as unknown as Parameters< + typeof normalizeHashlineEdits + >[0] + + //#when / #then + expect(() => normalizeHashlineEdits(input)).toThrow(/legacy format was removed/i) + }) +}) diff --git a/src/tools/hashline-edit/normalize-edits.ts b/src/tools/hashline-edit/normalize-edits.ts index b4e49d38..913b86ed 100644 --- a/src/tools/hashline-edit/normalize-edits.ts +++ b/src/tools/hashline-edit/normalize-edits.ts @@ -1,142 +1,114 @@ import type { HashlineEdit } from "./types" +type HashlineToolOp = "replace" | "append" | "prepend" + export interface RawHashlineEdit { - type?: - | "set_line" - | "replace_lines" - | "insert_after" - | "insert_before" - | "insert_between" - | "replace" - | "append" - | "prepend" - line?: string - start_line?: string - end_line?: string - after_line?: string - before_line?: string - text?: string | string[] - old_text?: string - new_text?: string | string[] + op?: HashlineToolOp + pos?: string + end?: string + lines?: string | string[] | null } -function firstDefined(...values: Array): string | undefined { - for (const value of values) { - if (typeof value === "string" && value.trim() !== "") return value +function normalizeAnchor(value: string | undefined): string | undefined { + if (typeof value !== "string") return undefined + const trimmed = value.trim() + return trimmed === "" ? undefined : trimmed +} + +function requireLines(edit: RawHashlineEdit, index: number): string | string[] { + if (edit.lines === undefined) { + throw new Error(`Edit ${index}: lines is required for ${edit.op ?? "unknown"}`) } - return undefined -} - -function requireText(edit: RawHashlineEdit, index: number): string | string[] { - const text = edit.text ?? edit.new_text - if (text === undefined) { - throw new Error(`Edit ${index}: text is required for ${edit.type ?? "unknown"}`) + if (edit.lines === null) { + return [] } - return text + return edit.lines } -function requireLine(anchor: string | undefined, index: number, op: string): string { +function requireLine(anchor: string | undefined, index: number, op: HashlineToolOp): string { if (!anchor) { - throw new Error(`Edit ${index}: ${op} requires at least one anchor line reference`) + throw new Error(`Edit ${index}: ${op} requires at least one anchor line reference (pos or end)`) } return anchor } -export function normalizeHashlineEdits(rawEdits: RawHashlineEdit[]): HashlineEdit[] { - const normalized: HashlineEdit[] = [] +function normalizeReplaceEdit(edit: RawHashlineEdit, index: number): HashlineEdit { + const pos = normalizeAnchor(edit.pos) + const end = normalizeAnchor(edit.end) + const anchor = requireLine(pos ?? end, index, "replace") + const text = requireLines(edit, index) - for (let index = 0; index < rawEdits.length; index += 1) { - const edit = rawEdits[index] ?? {} - const type = edit.type - - switch (type) { - case "set_line": { - const anchor = firstDefined(edit.line, edit.start_line, edit.end_line, edit.after_line, edit.before_line) - normalized.push({ - type: "set_line", - line: requireLine(anchor, index, "set_line"), - text: requireText(edit, index), - }) - break - } - case "replace_lines": { - const startAnchor = firstDefined(edit.start_line, edit.line, edit.after_line) - const endAnchor = firstDefined(edit.end_line, edit.line, edit.before_line) - - if (!startAnchor && !endAnchor) { - throw new Error(`Edit ${index}: replace_lines requires start_line or end_line`) - } - - if (startAnchor && endAnchor) { - normalized.push({ - type: "replace_lines", - start_line: startAnchor, - end_line: endAnchor, - text: requireText(edit, index), - }) - } else { - normalized.push({ - type: "set_line", - line: requireLine(startAnchor ?? endAnchor, index, "replace_lines"), - text: requireText(edit, index), - }) - } - break - } - case "insert_after": { - const anchor = firstDefined(edit.line, edit.after_line, edit.end_line, edit.start_line) - normalized.push({ - type: "insert_after", - line: requireLine(anchor, index, "insert_after"), - text: requireText(edit, index), - }) - break - } - case "insert_before": { - const anchor = firstDefined(edit.line, edit.before_line, edit.start_line, edit.end_line) - normalized.push({ - type: "insert_before", - line: requireLine(anchor, index, "insert_before"), - text: requireText(edit, index), - }) - break - } - case "insert_between": { - const afterLine = firstDefined(edit.after_line, edit.line, edit.start_line) - const beforeLine = firstDefined(edit.before_line, edit.end_line, edit.line) - normalized.push({ - type: "insert_between", - after_line: requireLine(afterLine, index, "insert_between.after_line"), - before_line: requireLine(beforeLine, index, "insert_between.before_line"), - text: requireText(edit, index), - }) - break - } - case "replace": { - const oldText = edit.old_text - const newText = edit.new_text ?? edit.text - if (!oldText) { - throw new Error(`Edit ${index}: replace requires old_text`) - } - if (newText === undefined) { - throw new Error(`Edit ${index}: replace requires new_text or text`) - } - normalized.push({ type: "replace", old_text: oldText, new_text: newText }) - break - } - case "append": { - normalized.push({ type: "append", text: requireText(edit, index) }) - break - } - case "prepend": { - normalized.push({ type: "prepend", text: requireText(edit, index) }) - break - } - default: { - throw new Error(`Edit ${index}: unsupported type "${String(type)}"`) - } + if (pos && end) { + return { + type: "replace_lines", + start_line: pos, + end_line: end, + text, } } - return normalized + return { + type: "set_line", + line: anchor, + text, + } +} + +function normalizeAppendEdit(edit: RawHashlineEdit, index: number): HashlineEdit { + const pos = normalizeAnchor(edit.pos) + const end = normalizeAnchor(edit.end) + const anchor = pos ?? end + const text = requireLines(edit, index) + + if (!anchor) { + return { + type: "append", + text, + } + } + + return { + type: "insert_after", + line: anchor, + text, + } +} + +function normalizePrependEdit(edit: RawHashlineEdit, index: number): HashlineEdit { + const pos = normalizeAnchor(edit.pos) + const end = normalizeAnchor(edit.end) + const anchor = pos ?? end + const text = requireLines(edit, index) + + if (!anchor) { + return { + type: "prepend", + text, + } + } + + return { + type: "insert_before", + line: anchor, + text, + } +} + +export function normalizeHashlineEdits(rawEdits: RawHashlineEdit[]): HashlineEdit[] { + return rawEdits.map((rawEdit, index) => { + const edit = rawEdit ?? {} + + switch (edit.op) { + case "replace": + return normalizeReplaceEdit(edit, index) + case "append": + return normalizeAppendEdit(edit, index) + case "prepend": + return normalizePrependEdit(edit, index) + default: + throw new Error( + `Edit ${index}: unsupported op "${String(edit.op)}". Legacy format was removed; use op/pos/end/lines.` + ) + } + }) } diff --git a/src/tools/hashline-edit/tool-description.ts b/src/tools/hashline-edit/tool-description.ts index d255ea8c..f95ea902 100644 --- a/src/tools/hashline-edit/tool-description.ts +++ b/src/tools/hashline-edit/tool-description.ts @@ -8,10 +8,11 @@ WORKFLOW: 5. Use anchors as "LINE#ID" only (never include trailing ":content"). VALIDATION: - 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 - 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. Refs/tags are interpreted against the last-read version of the file. + Payload shape: { "filePath": string, "edits": [...], "delete"?: boolean, "rename"?: string } + Each edit must be one of: replace, append, prepend + Edit shape: { "op": "replace"|"append"|"prepend", "pos"?: "LINE#ID", "end"?: "LINE#ID", "lines"?: string|string[]|null } + lines 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. Refs/tags are interpreted against the last-read version of the file. LINE#ID FORMAT (CRITICAL): Each line reference must be in "LINE#ID" format where: @@ -23,22 +24,21 @@ FILE MODES: rename moves final content to a new path and removes old path CONTENT FORMAT: - text/new_text can be a string (single line) or string[] (multi-line, preferred). - If you pass a multi-line string, it is split by real newline characters. - Literal "\\n" is preserved as text. + lines can be a string (single line) or string[] (multi-line, preferred). + If you pass a multi-line string, it is split by real newline characters. + Literal "\\n" is preserved as text. FILE CREATION: - append: adds content at EOF. 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. + append without anchors adds content at EOF. If file does not exist, creates it. + prepend without anchors adds content at BOF. If file does not exist, creates it. + CRITICAL: only unanchored append/prepend can create a missing file. OPERATION CHOICE: - One line wrong -> set_line - Adjacent block rewrite or swap/move -> replace_lines (prefer one range op over many single-line ops) - Both boundaries known -> insert_between (ALWAYS prefer over insert_after/insert_before) - One boundary known -> insert_after or insert_before - New file or EOF/BOF addition -> append or prepend - No LINE#ID available -> replace (last resort) + replace with pos only -> replace one line at pos + replace with pos+end -> replace range pos..end + 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 RULES (CRITICAL): 1. Minimize scope: one logical mutation site per operation. @@ -53,10 +53,9 @@ RULES (CRITICAL): 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. + - 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. diff --git a/src/tools/hashline-edit/tools.test.ts b/src/tools/hashline-edit/tools.test.ts index 8f2c4f65..46f663a3 100644 --- a/src/tools/hashline-edit/tools.test.ts +++ b/src/tools/hashline-edit/tools.test.ts @@ -31,7 +31,7 @@ describe("createHashlineEditTool", () => { fs.rmSync(tempDir, { recursive: true, force: true }) }) - it("applies set_line with LINE#ID anchor", async () => { + it("applies replace with single LINE#ID anchor", async () => { //#given const filePath = path.join(tempDir, "test.txt") fs.writeFileSync(filePath, "line1\nline2\nline3") @@ -41,7 +41,7 @@ describe("createHashlineEditTool", () => { const result = await tool.execute( { filePath, - edits: [{ type: "set_line", line: `2#${hash}`, text: "modified line2" }], + edits: [{ op: "replace", pos: `2#${hash}`, lines: "modified line2" }], }, createMockContext(), ) @@ -51,7 +51,7 @@ describe("createHashlineEditTool", () => { expect(result).toBe(`Updated ${filePath}`) }) - it("applies replace_lines and insert_after", async () => { + it("applies ranged replace and anchored append", async () => { //#given const filePath = path.join(tempDir, "test.txt") fs.writeFileSync(filePath, "line1\nline2\nline3\nline4") @@ -65,15 +65,15 @@ describe("createHashlineEditTool", () => { filePath, edits: [ { - type: "replace_lines", - start_line: `2#${line2Hash}`, - end_line: `3#${line3Hash}`, - text: "replaced", + op: "replace", + pos: `2#${line2Hash}`, + end: `3#${line3Hash}`, + lines: "replaced", }, { - type: "insert_after", - line: `4#${line4Hash}`, - text: "inserted", + op: "append", + pos: `4#${line4Hash}`, + lines: "inserted", }, ], }, @@ -93,7 +93,7 @@ describe("createHashlineEditTool", () => { const result = await tool.execute( { filePath, - edits: [{ type: "set_line", line: "1#ZZ", text: "new" }], + edits: [{ op: "replace", pos: "1#ZZ", lines: "new" }], }, createMockContext(), ) @@ -113,7 +113,7 @@ describe("createHashlineEditTool", () => { await tool.execute( { filePath, - edits: [{ type: "set_line", line: `1#${line1Hash}`, text: "join(\\n)" }], + edits: [{ op: "replace", pos: `1#${line1Hash}`, lines: "join(\\n)" }], }, createMockContext(), ) @@ -121,7 +121,7 @@ describe("createHashlineEditTool", () => { await tool.execute( { filePath, - edits: [{ type: "insert_after", line: `1#${computeLineHash(1, "join(\\n)")}`, text: ["a", "b"] }], + edits: [{ op: "append", pos: `1#${computeLineHash(1, "join(\\n)")}`, lines: ["a", "b"] }], }, createMockContext(), ) @@ -130,12 +130,11 @@ describe("createHashlineEditTool", () => { expect(fs.readFileSync(filePath, "utf-8")).toBe("join(\\n)\na\nb\nline2") }) - it("supports insert_before and insert_between", async () => { + it("supports anchored prepend and anchored append", async () => { //#given const filePath = path.join(tempDir, "test.txt") fs.writeFileSync(filePath, "line1\nline2\nline3") const line1 = computeLineHash(1, "line1") - const line2 = computeLineHash(2, "line2") const line3 = computeLineHash(3, "line3") //#when @@ -143,8 +142,8 @@ describe("createHashlineEditTool", () => { { filePath, edits: [ - { type: "insert_before", line: `3#${line3}`, text: ["before3"] }, - { type: "insert_between", after_line: `1#${line1}`, before_line: `2#${line2}`, text: ["between"] }, + { op: "prepend", pos: `3#${line3}`, lines: ["before3"] }, + { op: "append", pos: `1#${line1}`, lines: ["between"] }, ], }, createMockContext(), @@ -164,7 +163,7 @@ describe("createHashlineEditTool", () => { const result = await tool.execute( { filePath, - edits: [{ type: "insert_after", line: `1#${line1}`, text: [] }], + edits: [{ op: "append", pos: `1#${line1}`, lines: [] }], }, createMockContext(), ) @@ -186,7 +185,7 @@ describe("createHashlineEditTool", () => { { filePath, rename: renamedPath, - edits: [{ type: "set_line", line: `2#${line2}`, text: "line2-updated" }], + edits: [{ op: "replace", pos: `2#${line2}`, lines: "line2-updated" }], }, createMockContext(), ) @@ -226,8 +225,8 @@ describe("createHashlineEditTool", () => { { filePath, edits: [ - { type: "append", text: ["line2"] }, - { type: "prepend", text: ["line1"] }, + { op: "append", lines: ["line2"] }, + { op: "prepend", lines: ["line1"] }, ], }, createMockContext(), @@ -239,7 +238,7 @@ describe("createHashlineEditTool", () => { expect(result).toBe(`Updated ${filePath}`) }) - it("accepts replace_lines with one anchor and downgrades to set_line", async () => { + it("accepts replace with one anchor", async () => { //#given const filePath = path.join(tempDir, "degrade.txt") fs.writeFileSync(filePath, "line1\nline2\nline3") @@ -249,7 +248,7 @@ describe("createHashlineEditTool", () => { const result = await tool.execute( { filePath, - edits: [{ type: "replace_lines", start_line: `2#${line2Hash}`, text: ["line2-updated"] }], + edits: [{ op: "replace", pos: `2#${line2Hash}`, lines: ["line2-updated"] }], }, createMockContext(), ) @@ -259,7 +258,7 @@ describe("createHashlineEditTool", () => { expect(result).toBe(`Updated ${filePath}`) }) - it("accepts insert_after using after_line alias", async () => { + it("accepts anchored append using end alias", async () => { //#given const filePath = path.join(tempDir, "alias.txt") fs.writeFileSync(filePath, "line1\nline2") @@ -269,7 +268,7 @@ describe("createHashlineEditTool", () => { await tool.execute( { filePath, - edits: [{ type: "insert_after", after_line: `1#${line1Hash}`, text: ["inserted"] }], + edits: [{ op: "append", end: `1#${line1Hash}`, lines: ["inserted"] }], }, createMockContext(), ) @@ -289,7 +288,7 @@ describe("createHashlineEditTool", () => { await tool.execute( { filePath, - edits: [{ type: "set_line", line: `2#${line2Hash}`, text: "line2-updated" }], + edits: [{ op: "replace", pos: `2#${line2Hash}`, lines: "line2-updated" }], }, createMockContext(), ) diff --git a/src/tools/hashline-edit/tools.ts b/src/tools/hashline-edit/tools.ts index 98a25894..13265029 100644 --- a/src/tools/hashline-edit/tools.ts +++ b/src/tools/hashline-edit/tools.ts @@ -20,32 +20,19 @@ export function createHashlineEditTool(): ToolDefinition { edits: tool.schema .array( tool.schema.object({ - type: tool.schema + op: tool.schema .union([ - tool.schema.literal("set_line"), - tool.schema.literal("replace_lines"), - tool.schema.literal("insert_after"), - tool.schema.literal("insert_before"), - tool.schema.literal("insert_between"), tool.schema.literal("replace"), tool.schema.literal("append"), tool.schema.literal("prepend"), ]) - .describe("Edit operation type"), - line: tool.schema.string().optional().describe("Anchor line in LINE#ID format"), - start_line: tool.schema.string().optional().describe("Range start in LINE#ID format"), - end_line: tool.schema.string().optional().describe("Range end in LINE#ID format"), - after_line: tool.schema.string().optional().describe("Insert boundary (after) in LINE#ID format"), - before_line: tool.schema.string().optional().describe("Insert boundary (before) in LINE#ID format"), - text: tool.schema - .union([tool.schema.string(), tool.schema.array(tool.schema.string())]) + .describe("Hashline edit operation mode"), + pos: tool.schema.string().optional().describe("Primary anchor in LINE#ID format"), + end: tool.schema.string().optional().describe("Range end anchor in LINE#ID format"), + lines: tool.schema + .union([tool.schema.string(), tool.schema.array(tool.schema.string()), tool.schema.null()]) .optional() - .describe("Operation content"), - old_text: tool.schema.string().optional().describe("Legacy text replacement source"), - new_text: tool.schema - .union([tool.schema.string(), tool.schema.array(tool.schema.string())]) - .optional() - .describe("Legacy text replacement target"), + .describe("Replacement or inserted lines. null/[] deletes with replace"), }) ) .describe("Array of edit operations to apply (empty when delete=true)"),