Fix hashline-edit deduplication and validation
- Canonicalize anchors in dedupe keys to handle whitespace variants - Make lines field required in edit operations - Only allow unanchored append/prepend to create missing files - Reorder delete/rename validation to prevent edge cases - Add allow_non_gpt_model and max_prompt_tokens to config schema ```
This commit is contained in:
parent
58201220cc
commit
b1203b9501
@ -1,18 +1,24 @@
|
|||||||
import type { HashlineEdit } from "./types"
|
import type { HashlineEdit } from "./types"
|
||||||
import { toNewLines } from "./edit-text-normalization"
|
import { toNewLines } from "./edit-text-normalization"
|
||||||
|
import { normalizeLineRef } from "./validation"
|
||||||
|
|
||||||
function normalizeEditPayload(payload: string | string[]): string {
|
function normalizeEditPayload(payload: string | string[]): string {
|
||||||
return toNewLines(payload).join("\n")
|
return toNewLines(payload).join("\n")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function canonicalAnchor(anchor: string | undefined): string {
|
||||||
|
if (!anchor) return ""
|
||||||
|
return normalizeLineRef(anchor)
|
||||||
|
}
|
||||||
|
|
||||||
function buildDedupeKey(edit: HashlineEdit): string {
|
function buildDedupeKey(edit: HashlineEdit): string {
|
||||||
switch (edit.op) {
|
switch (edit.op) {
|
||||||
case "replace":
|
case "replace":
|
||||||
return `replace|${edit.pos}|${edit.end ?? ""}|${normalizeEditPayload(edit.lines)}`
|
return `replace|${canonicalAnchor(edit.pos)}|${edit.end ? canonicalAnchor(edit.end) : ""}|${normalizeEditPayload(edit.lines)}`
|
||||||
case "append":
|
case "append":
|
||||||
return `append|${edit.pos ?? ""}|${normalizeEditPayload(edit.lines)}`
|
return `append|${canonicalAnchor(edit.pos)}|${normalizeEditPayload(edit.lines)}`
|
||||||
case "prepend":
|
case "prepend":
|
||||||
return `prepend|${edit.pos ?? ""}|${normalizeEditPayload(edit.lines)}`
|
return `prepend|${canonicalAnchor(edit.pos)}|${normalizeEditPayload(edit.lines)}`
|
||||||
default:
|
default:
|
||||||
return JSON.stringify(edit)
|
return JSON.stringify(edit)
|
||||||
}
|
}
|
||||||
|
|||||||
@ -1,5 +1,5 @@
|
|||||||
import { describe, expect, it } from "bun:test"
|
import { describe, expect, it } from "bun:test"
|
||||||
import { applyHashlineEdits } from "./edit-operations"
|
import { applyHashlineEdits, applyHashlineEditsWithReport } from "./edit-operations"
|
||||||
import { applyAppend, applyInsertAfter, applyPrepend, applyReplaceLines, applySetLine } from "./edit-operation-primitives"
|
import { applyAppend, applyInsertAfter, applyPrepend, applyReplaceLines, applySetLine } from "./edit-operation-primitives"
|
||||||
import { computeLineHash } from "./hash-computation"
|
import { computeLineHash } from "./hash-computation"
|
||||||
import type { HashlineEdit } from "./types"
|
import type { HashlineEdit } from "./types"
|
||||||
@ -389,3 +389,23 @@ describe("hashline edit operations", () => {
|
|||||||
expect(result).toEqual("replaced A\nline 3\nreplaced B")
|
expect(result).toEqual("replaced A\nline 3\nreplaced B")
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe("dedupe anchor canonicalization", () => {
|
||||||
|
it("deduplicates edits with whitespace-variant anchors", () => {
|
||||||
|
//#given
|
||||||
|
const content = "line 1\nline 2"
|
||||||
|
const lines = content.split("\n")
|
||||||
|
const canonical = `1#${computeLineHash(1, lines[0])}`
|
||||||
|
const spaced = ` 1 # ${computeLineHash(1, lines[0])} `
|
||||||
|
|
||||||
|
//#when
|
||||||
|
const report = applyHashlineEditsWithReport(content, [
|
||||||
|
{ op: "append", pos: canonical, lines: ["inserted"] },
|
||||||
|
{ op: "append", pos: spaced, lines: ["inserted"] },
|
||||||
|
])
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(report.deduplicatedEdits).toBe(1)
|
||||||
|
expect(report.content).toBe("line 1\ninserted\nline 2")
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|||||||
@ -33,7 +33,7 @@ function resolveToolCallID(ctx: ToolContextWithCallID): string | undefined {
|
|||||||
|
|
||||||
function canCreateFromMissingFile(edits: HashlineEdit[]): boolean {
|
function canCreateFromMissingFile(edits: HashlineEdit[]): boolean {
|
||||||
if (edits.length === 0) return false
|
if (edits.length === 0) return false
|
||||||
return edits.every((edit) => edit.op === "append" || edit.op === "prepend")
|
return edits.every((edit) => (edit.op === "append" || edit.op === "prepend") && !edit.pos)
|
||||||
}
|
}
|
||||||
|
|
||||||
function buildSuccessMeta(
|
function buildSuccessMeta(
|
||||||
@ -86,19 +86,19 @@ export async function executeHashlineEditTool(args: HashlineEditArgs, context: T
|
|||||||
const filePath = args.filePath
|
const filePath = args.filePath
|
||||||
const { delete: deleteMode, rename } = args
|
const { delete: deleteMode, rename } = args
|
||||||
|
|
||||||
|
if (deleteMode && rename) {
|
||||||
|
return "Error: delete and rename cannot be used together"
|
||||||
|
}
|
||||||
|
if (deleteMode && args.edits.length > 0) {
|
||||||
|
return "Error: delete mode requires edits to be an empty array"
|
||||||
|
}
|
||||||
|
|
||||||
if (!deleteMode && (!args.edits || !Array.isArray(args.edits) || args.edits.length === 0)) {
|
if (!deleteMode && (!args.edits || !Array.isArray(args.edits) || args.edits.length === 0)) {
|
||||||
return "Error: edits parameter must be a non-empty array"
|
return "Error: edits parameter must be a non-empty array"
|
||||||
}
|
}
|
||||||
|
|
||||||
const edits = deleteMode ? [] : normalizeHashlineEdits(args.edits)
|
const edits = deleteMode ? [] : normalizeHashlineEdits(args.edits)
|
||||||
|
|
||||||
if (deleteMode && rename) {
|
|
||||||
return "Error: delete and rename cannot be used together"
|
|
||||||
}
|
|
||||||
if (deleteMode && edits.length > 0) {
|
|
||||||
return "Error: delete mode requires edits to be an empty array"
|
|
||||||
}
|
|
||||||
|
|
||||||
const file = Bun.file(filePath)
|
const file = Bun.file(filePath)
|
||||||
const exists = await file.exists()
|
const exists = await file.exists()
|
||||||
if (!exists && !deleteMode && !canCreateFromMissingFile(edits)) {
|
if (!exists && !deleteMode && !canCreateFromMissingFile(edits)) {
|
||||||
|
|||||||
@ -10,7 +10,7 @@ WORKFLOW:
|
|||||||
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: replace, append, prepend
|
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 }
|
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)
|
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.
|
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.
|
||||||
|
|
||||||
|
|||||||
@ -341,4 +341,81 @@ describe("createHashlineEditTool", () => {
|
|||||||
//#then
|
//#then
|
||||||
expect(envelope.lineEnding).toBe("\r\n")
|
expect(envelope.lineEnding).toBe("\r\n")
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it("rejects delete=true with non-empty edits before normalization", async () => {
|
||||||
|
//#given
|
||||||
|
const filePath = path.join(tempDir, "delete-reject.txt")
|
||||||
|
fs.writeFileSync(filePath, "line1")
|
||||||
|
|
||||||
|
//#when
|
||||||
|
const result = await tool.execute(
|
||||||
|
{
|
||||||
|
filePath,
|
||||||
|
delete: true,
|
||||||
|
edits: [{ op: "replace", pos: "1#ZZ", lines: "bad" }],
|
||||||
|
},
|
||||||
|
createMockContext(),
|
||||||
|
)
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(result).toContain("delete mode requires edits to be an empty array")
|
||||||
|
expect(fs.existsSync(filePath)).toBe(true)
|
||||||
|
})
|
||||||
|
|
||||||
|
it("rejects delete=true combined with rename", async () => {
|
||||||
|
//#given
|
||||||
|
const filePath = path.join(tempDir, "delete-rename.txt")
|
||||||
|
fs.writeFileSync(filePath, "line1")
|
||||||
|
|
||||||
|
//#when
|
||||||
|
const result = await tool.execute(
|
||||||
|
{
|
||||||
|
filePath,
|
||||||
|
delete: true,
|
||||||
|
rename: path.join(tempDir, "new-name.txt"),
|
||||||
|
edits: [],
|
||||||
|
},
|
||||||
|
createMockContext(),
|
||||||
|
)
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(result).toContain("delete and rename cannot be used together")
|
||||||
|
expect(fs.existsSync(filePath)).toBe(true)
|
||||||
|
})
|
||||||
|
|
||||||
|
it("rejects missing file creation with anchored append", async () => {
|
||||||
|
//#given
|
||||||
|
const filePath = path.join(tempDir, "nonexistent.txt")
|
||||||
|
|
||||||
|
//#when
|
||||||
|
const result = await tool.execute(
|
||||||
|
{
|
||||||
|
filePath,
|
||||||
|
edits: [{ op: "append", pos: "1#ZZ", lines: ["bad"] }],
|
||||||
|
},
|
||||||
|
createMockContext(),
|
||||||
|
)
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(result).toContain("File not found")
|
||||||
|
})
|
||||||
|
|
||||||
|
it("allows missing file creation with unanchored append", async () => {
|
||||||
|
//#given
|
||||||
|
const filePath = path.join(tempDir, "newfile.txt")
|
||||||
|
|
||||||
|
//#when
|
||||||
|
const result = await tool.execute(
|
||||||
|
{
|
||||||
|
filePath,
|
||||||
|
edits: [{ op: "append", lines: ["created"] }],
|
||||||
|
},
|
||||||
|
createMockContext(),
|
||||||
|
)
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(fs.existsSync(filePath)).toBe(true)
|
||||||
|
expect(fs.readFileSync(filePath, "utf-8")).toBe("created")
|
||||||
|
expect(result).toBe(`Updated ${filePath}`)
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@ -31,7 +31,6 @@ export function createHashlineEditTool(): ToolDefinition {
|
|||||||
end: tool.schema.string().optional().describe("Range end anchor in LINE#ID format"),
|
end: tool.schema.string().optional().describe("Range end anchor in LINE#ID format"),
|
||||||
lines: tool.schema
|
lines: tool.schema
|
||||||
.union([tool.schema.string(), tool.schema.array(tool.schema.string()), tool.schema.null()])
|
.union([tool.schema.string(), tool.schema.array(tool.schema.string()), tool.schema.null()])
|
||||||
.optional()
|
|
||||||
.describe("Replacement or inserted lines. null/[] deletes with replace"),
|
.describe("Replacement or inserted lines. null/[] deletes with replace"),
|
||||||
})
|
})
|
||||||
)
|
)
|
||||||
|
|||||||
@ -15,7 +15,7 @@ const MISMATCH_CONTEXT = 2
|
|||||||
|
|
||||||
const LINE_REF_EXTRACT_PATTERN = /([0-9]+#[ZPMQVRWSNKTXJBYH]{2})/
|
const LINE_REF_EXTRACT_PATTERN = /([0-9]+#[ZPMQVRWSNKTXJBYH]{2})/
|
||||||
|
|
||||||
function normalizeLineRef(ref: string): string {
|
export function normalizeLineRef(ref: string): string {
|
||||||
const originalTrimmed = ref.trim()
|
const originalTrimmed = ref.trim()
|
||||||
let trimmed = originalTrimmed
|
let trimmed = originalTrimmed
|
||||||
trimmed = trimmed.replace(/^(?:>>>|[+-])\s*/, "")
|
trimmed = trimmed.replace(/^(?:>>>|[+-])\s*/, "")
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user