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.
2026-02-24 03:00:38 +09:00
parent ebd26b7421
commit 6ec0ff732b
5 changed files with 210 additions and 189 deletions
--- a/src/tools/hashline-edit/normalize-edits.test.ts
+++ 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)
+  })
+})
--- 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>): string | undefined {
-  for (const value of values) {
-    if (typeof value === "string" && value.trim() !== "") return value
-  }
-  return undefined
+function normalizeAnchor(value: string | undefined): string | undefined {
+  if (typeof value !== "string") return undefined
+  const trimmed = value.trim()
+  return trimmed === "" ? undefined : trimmed
 }

-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"}`)
+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 text
+  if (edit.lines === null) {
+    return []
+  }
+  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({
+  if (pos && end) {
+    return {
      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)}"`)
-      }
+      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.`
+        )
+    }
+  })
 }
--- a/src/tools/hashline-edit/tool-description.ts
+++ b/src/tools/hashline-edit/tool-description.ts
@@ -9,8 +9,9 @@ WORKFLOW:

 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)
+  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):
@@ -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).
+  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,7 +53,6 @@ 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.
--- 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(),
    )
--- 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)"),