diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index a1e7029b0..eb4f4931a 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -13,8 +13,7 @@ ### Fixed -- Fixed Anthropic strict tool-call retry logic to also handle 400 errors reporting `schema is too complex` when grammar compilation fails -- +- Fixed Anthropic tool schema compilation failures by keeping the `write` tool out of the strict-tool allowlist when the full coding-agent tool set is active - Fixed Anthropic 400 `tools.*.custom: For 'object' type, property 'minItems' is not supported` by stripping `minItems` from object-shaped JSON schema nodes (array nodes still keep supported `minItems` values) - Fixed Anthropic tool schemas that used tuple-style arrays by stripping unsupported `maxItems` and only preserving provider-supported `minItems` values - Fixed Anthropic and OpenRouter Anthropic tool calls that previously failed with `compiled grammar is too large` by retrying automatically without strict tool schemas and reusing non-strict mode for subsequent requests in the same provider session diff --git a/packages/ai/src/providers/anthropic.ts b/packages/ai/src/providers/anthropic.ts index 3997301cc..6606a6d99 100644 --- a/packages/ai/src/providers/anthropic.ts +++ b/packages/ai/src/providers/anthropic.ts @@ -216,7 +216,8 @@ function isAnthropicStrictGrammarTooLargeError(error: unknown): boolean { if (extractHttpStatusFromError(error) !== 400) return false; const message = error instanceof Error ? error.message : String(error); const isStrictGrammarTooLarge = /compiled grammar/i.test(message) && /too large/i.test(message); - const isSchemaCompilationTooComplex = /schema/i.test(message) && /too complex/i.test(message) && /compil/i.test(message); + const isSchemaCompilationTooComplex = + /schema/i.test(message) && /too complex/i.test(message) && /compil/i.test(message); return /invalid_request_error/i.test(message) && (isStrictGrammarTooLarge || isSchemaCompilationTooComplex); } @@ -1754,7 +1755,7 @@ export function convertAnthropicMessages( } const ANTHROPIC_UNSUPPORTED_TOOL_SCHEMA_FIELDS = new Set(["maxItems", "patternProperties"]); -const ANTHROPIC_STRICT_TOOL_ALLOWLIST = new Set(["bash", "python", "edit", "find", "write"]); +const ANTHROPIC_STRICT_TOOL_ALLOWLIST = new Set(["bash", "python", "edit", "find"]); const MAX_ANTHROPIC_STRICT_TOOLS = 20; const MAX_ANTHROPIC_STRICT_OPTIONAL_PARAMETERS = 24; const MAX_ANTHROPIC_STRICT_UNION_PARAMETERS = 16; diff --git a/packages/ai/test/anthropic-alignment.test.ts b/packages/ai/test/anthropic-alignment.test.ts index be2a27a7d..b089df8c4 100644 --- a/packages/ai/test/anthropic-alignment.test.ts +++ b/packages/ai/test/anthropic-alignment.test.ts @@ -437,7 +437,7 @@ describe("Anthropic request fingerprint alignment", () => { it("marks only the Anthropic strict allowlist strict", async () => { const tools: Tool[] = [ - ...(["bash", "python", "edit", "find", "write"] as const).map(name => ({ + ...(["bash", "python", "edit", "find"] as const).map(name => ({ name, description: `${name} tool`, strict: true, @@ -447,7 +447,7 @@ describe("Anthropic request fingerprint alignment", () => { required: ["requiredValue"], } as unknown as TSchema, })), - ...(["grep", "read", "task", "todo_write", "web_search", "ast_grep"] as const).map(name => ({ + ...(["write", "grep", "read", "task", "todo_write", "web_search", "ast_grep"] as const).map(name => ({ name, description: `${name} tool`, strict: true, @@ -473,11 +473,11 @@ describe("Anthropic request fingerprint alignment", () => { const strictNames = (payload.tools ?? []).filter(tool => tool.strict === true).map(tool => tool.name); - expect(strictNames).toEqual(["bash", "python", "edit", "find", "write"]); + expect(strictNames).toEqual(["bash", "python", "edit", "find"]); expect(payload.tools?.find(tool => tool.name === "bash")?.input_schema?.required).toEqual(["requiredValue"]); }); - it("honors strict=false for allowlisted Anthropic tools", async () => { + it("honors strict=false and skips non-allowlisted Anthropic tools", async () => { const tools: Tool[] = [ { name: "bash", @@ -531,7 +531,7 @@ describe("Anthropic request fingerprint alignment", () => { )) as { tools?: Array<{ name?: string; strict?: boolean }> }; const strictNames = (payload.tools ?? []).filter(tool => tool.strict === true).map(tool => tool.name); - expect(strictNames).toEqual(["python", "write"]); + expect(strictNames).toEqual(["python"]); }); it("drops fine-grained tool-streaming beta from default Anthropic client options", () => { diff --git a/packages/ai/test/anthropic-stream-envelope.test.ts b/packages/ai/test/anthropic-stream-envelope.test.ts index 641bd48a6..5af3fcd5c 100644 --- a/packages/ai/test/anthropic-stream-envelope.test.ts +++ b/packages/ai/test/anthropic-stream-envelope.test.ts @@ -265,8 +265,8 @@ describe("anthropic stream envelope handling", () => { ...context, tools: [ { - name: "write", - description: "Write a value", + name: "edit", + description: "Edit a value", strict: true, parameters: Type.Object({ query: Type.String() }), }, @@ -321,8 +321,8 @@ describe("anthropic stream envelope handling", () => { ...context, tools: [ { - name: "write", - description: "Write a value", + name: "edit", + description: "Edit a value", strict: true, parameters: Type.Object({ query: Type.String() }), }, diff --git a/packages/coding-agent/src/edit/modes/atom.ts b/packages/coding-agent/src/edit/modes/atom.ts index ef3cfd8e4..e980bcc43 100644 --- a/packages/coding-agent/src/edit/modes/atom.ts +++ b/packages/coding-agent/src/edit/modes/atom.ts @@ -1,7 +1,7 @@ /** * * Flat locator + verb edit mode backed by hashline anchors. Each entry carries - * one shared `loc` selector plus one or more verbs (`pre`, `set`, `post`, `sub`). + * one shared `loc` selector plus one or more verbs (`pre`, `set`, `post`). * The runtime resolves those verbs into internal anchor-scoped edits and still * reuses hashline's staleness scheme (`computeLineHash`) verbatim. * @@ -9,13 +9,12 @@ * { path, loc: "5th", set: ["..."] } * { path, loc: "5th", pre: ["..."] } * { path, loc: "5th", post: ["..."] } - * { path, loc: "5th", sub: ["find", "replace"] } * { path, loc: "5th", pre: [...], set: [...], post: [...] } * { path, loc: "^", pre: [...] } // prepend to BOF * { path, loc: "$", post: [...] } // append to EOF * * `set: []` on a single-anchor locator deletes that line. `set:[""]` preserves - * a blank line. Line ranges are not supported; `set` and `sub` cannot coexist + * a blank line. Line ranges are not supported. * in the same entry. * * For deleting or moving files, the agent should use bash. @@ -66,18 +65,6 @@ export const atomEditSchema = Type.Object( set: Type.Optional(textSchema), pre: Type.Optional(textSchema), post: Type.Optional(textSchema), - sub: Type.Optional( - Type.Array(Type.String(), { - description: "substring search and replace as [find, replace]", - minItems: 2, - maxItems: 2, - examples: [ - ["||", "??"], - ["true", "false"], - ["i--", "i++"], - ], - }), - ), }, { additionalProperties: false }, ); @@ -102,7 +89,6 @@ export type AtomEdit = | { op: "pre"; pos: Anchor; lines: string[] } | { op: "post"; pos: Anchor; lines: string[] } | { op: "del"; pos: Anchor } - | { op: "sub"; pos: Anchor; find: string; to: string } | { op: "append_file"; lines: string[] } | { op: "prepend_file"; lines: string[] }; @@ -110,7 +96,7 @@ export type AtomEdit = // Param guards // ═══════════════════════════════════════════════════════════════════════════ -const ATOM_VERB_KEYS = ["set", "pre", "post", "sub"] as const; +const ATOM_VERB_KEYS = ["set", "pre", "post"] as const; type AtomOptionalKey = "path" | "loc" | (typeof ATOM_VERB_KEYS)[number]; const ATOM_OPTIONAL_KEYS = ["path", "loc", ...ATOM_VERB_KEYS] as const satisfies readonly AtomOptionalKey[]; @@ -227,20 +213,6 @@ function parseLoc(raw: string, editIndex: number): ParsedAtomLoc { return { kind: "anchor", pos: parseAnchor(raw, "loc") }; } -function parseSubSpec(sub: AtomToolEdit["sub"], editIndex: number): { find: string; withText: string } { - if (!Array.isArray(sub) || sub.length !== 2) { - throw new Error(`Edit ${editIndex}: sub must be a 2-item tuple [find, replace].`); - } - const [find, withText] = sub; - if (typeof find !== "string" || find.length === 0) { - throw new Error("sub requires a non-empty `find` string (the unique substring on the anchored line)."); - } - if (typeof withText !== "string") { - throw new Error("sub replacement must be a string."); - } - return { find, withText }; -} - function classifyAtomEdit(edit: AtomToolEdit): string { const entry = stripNullAtomFields(edit); const verbs = ATOM_VERB_KEYS.filter(k => entry[k] !== undefined); @@ -255,9 +227,6 @@ function resolveAtomToolEdit(edit: AtomToolEdit, editIndex = 0): AtomEdit[] { `Edit ${editIndex}: missing verb. Each entry must include at least one of: ${ATOM_VERB_KEYS.join(", ")}.`, ); } - if (entry.set !== undefined && entry.sub !== undefined) { - throw new Error(`Edit ${editIndex}: set and sub cannot be used together in the same entry.`); - } if (typeof entry.loc !== "string") { throw new Error(`Edit ${editIndex}: missing loc. Use a selector like "160sr", "^", or "$".`); } @@ -266,7 +235,7 @@ function resolveAtomToolEdit(edit: AtomToolEdit, editIndex = 0): AtomEdit[] { const resolved: AtomEdit[] = []; if (loc.kind === "bof") { - if (entry.set !== undefined || entry.sub !== undefined || entry.post !== undefined) { + if (entry.set !== undefined || entry.post !== undefined) { throw new Error(`Edit ${editIndex}: loc "^" only supports pre.`); } if (entry.pre !== undefined) { @@ -276,7 +245,7 @@ function resolveAtomToolEdit(edit: AtomToolEdit, editIndex = 0): AtomEdit[] { } if (loc.kind === "eof") { - if (entry.set !== undefined || entry.sub !== undefined || entry.pre !== undefined) { + if (entry.set !== undefined || entry.pre !== undefined) { throw new Error(`Edit ${editIndex}: loc "$" only supports post.`); } if (entry.post !== undefined) { @@ -295,10 +264,6 @@ function resolveAtomToolEdit(edit: AtomToolEdit, editIndex = 0): AtomEdit[] { resolved.push({ op: "set", pos: loc.pos, lines: hashlineParseText(entry.set) }); } } - if (entry.sub !== undefined) { - const { find, withText } = parseSubSpec(entry.sub, editIndex); - resolved.push({ op: "sub", pos: loc.pos, find, to: withText }); - } if (entry.post !== undefined) { resolved.push({ op: "post", pos: loc.pos, lines: hashlineParseText(entry.post) }); } @@ -315,7 +280,6 @@ function* getAtomAnchors(edit: AtomEdit): Iterable { case "pre": case "post": case "del": - case "sub": yield edit.pos; return; default: @@ -348,16 +312,16 @@ function validateAtomAnchors(edits: AtomEdit[], fileLines: string[], warnings: s } function validateNoConflictingAnchorOps(edits: AtomEdit[]): void { - // For each anchor line, at most one mutating op (set/del/sub). + // For each anchor line, at most one mutating op (set/del). // `pre`/`post` (insert ops) may coexist with them — they don't mutate the anchor line. const mutatingPerLine = new Map(); for (const edit of edits) { - if (edit.op !== "set" && edit.op !== "del" && edit.op !== "sub") continue; + if (edit.op !== "set" && edit.op !== "del") continue; const existing = mutatingPerLine.get(edit.pos.line); if (existing) { throw new Error( `Conflicting ops on anchor line ${edit.pos.line}: \`${existing}\` and \`${edit.op}\`. ` + - `At most one of set/del/sub is allowed per anchor.`, + `At most one of set/del is allowed per anchor.`, ); } mutatingPerLine.set(edit.pos.line, edit.op); @@ -368,26 +332,6 @@ function validateNoConflictingAnchorOps(edits: AtomEdit[]): void { // Apply // ═══════════════════════════════════════════════════════════════════════════ -function applySubToLine(edit: { op: "sub"; pos: Anchor; find: string; to: string }, current: string): string { - const first = current.indexOf(edit.find); - if (first === -1) { - throw new Error( - `sub: substring \`${edit.find}\` not found on line ${edit.pos.line}. ` + - `Current line content: ${JSON.stringify(current)}`, - ); - } - const second = current.indexOf(edit.find, first + 1); - if (second !== -1) { - throw new Error( - `sub: substring \`${edit.find}\` occurs more than once on line ${edit.pos.line}; ` + - `use a longer substring that uniquely identifies the target. ` + - `Current line content: ${JSON.stringify(current)}`, - ); - } - // `sub`: replace only the matched span; tail preserved. - return current.slice(0, first) + edit.to + current.slice(first + edit.find.length); -} - function maybeAutocorrectEscapedTabIndentation(edits: AtomEdit[], warnings: string[]): void { const enabled = Bun.env.PI_HASHLINE_AUTOCORRECT_ESCAPED_TABS !== "0"; if (!enabled) return; @@ -487,7 +431,7 @@ export function applyAtomEdits( bucket.sort((a, b) => a.idx - b.idx); const idx = line - 1; - let currentLine = fileLines[idx]; + const currentLine = fileLines[idx]; let replacement: string[] = [currentLine]; let replacementSet = false; let anchorMutated = false; @@ -513,13 +457,6 @@ export function applyAtomEdits( replacementSet = true; anchorMutated = true; break; - case "sub": { - currentLine = applySubToLine(edit, currentLine); - replacement = currentLine.includes("\n") ? currentLine.split("\n") : [currentLine]; - replacementSet = true; - anchorMutated = true; - break; - } } } @@ -535,9 +472,7 @@ export function applyAtomEdits( if (replacementProducesNoChange) { const firstEdit = bucket[0]?.edit; const loc = firstEdit ? `${firstEdit.pos.line}${firstEdit.pos.hash}` : `${line}`; - const reason = bucket.some(b => b.edit.op === "sub") - ? "sub produced no change (find/replace already applied or both substrings are equal)" - : "replacement is identical to the current line content"; + const reason = "replacement is identical to the current line content"; noopEdits.push({ editIndex: bucket[0]?.idx ?? 0, loc, diff --git a/packages/coding-agent/src/prompts/tools/atom.md b/packages/coding-agent/src/prompts/tools/atom.md index e1cc02369..fd3bdb724 100644 --- a/packages/coding-agent/src/prompts/tools/atom.md +++ b/packages/coding-agent/src/prompts/tools/atom.md @@ -15,14 +15,11 @@ Verbs: - `set: ["…"]` — replace the anchor line - `pre: ["…"]` — insert before the anchor line (or at BOF when `loc:"^"`) - `post: ["…"]` — insert after the anchor line (or at EOF when `loc:"$"`) -- `sub: [find, replace]` — replace a unique substring on the anchored line; the line tail is preserved automatically Combination rules: -- On a single-anchor `loc`, you may combine `pre`, **one of** `set` or `sub`, and `post` in the same entry. +- On a single-anchor `loc`, you may combine `pre`, `set`, and `post` in the same entry. - `set: []` on a single-anchor `loc` deletes that line. - `set:[""]` is **not** delete — it replaces the line with a blank line. - -`sub` is the cheapest op when only part of a line changes. `find` and `replace` should both be the smallest fragment that does the job. @@ -45,28 +42,23 @@ All examples below reference the same file: {{hline 14 "}"}} ``` -# Swap an operator with `sub` +# Swap an operator by replacing the line Original line 4: `const fallback = group.targetFramework || 'All Frameworks';` -`{path:"a.ts",edits:[{loc:{{href 4 "const fallback = group.targetFramework || 'All Frameworks';"}},sub:["||","??"]}]}` +`{path:"a.ts",edits:[{loc:{{href 4 "const fallback = group.targetFramework || 'All Frameworks';"}},set:["const fallback = group.targetFramework ?? 'All Frameworks';"]}]}` -# Flip a literal with `sub` +# Flip a literal by replacing the line Original line 2: `const timeout = 5000;` -`{path:"a.ts",edits:[{loc:{{href 2 "const timeout = 5000;"}},sub:["5000","30_000"]}]}` +`{path:"a.ts",edits:[{loc:{{href 2 "const timeout = 5000;"}},set:["const timeout = 30_000;"]}]}` -# Negate a condition with `sub` +# Negate a condition by replacing the line Original line 10: `\tif (x) {` -`{path:"a.ts",edits:[{loc:{{href 10 "\tif (x) {"}},sub:["(x)","(!x)"]}]}` - -# Off-by-one with `sub` -For a single-digit/operator nudge, `sub` is the cheapest op. Do **not** rewrite the whole line with `set`. -Original line 2: `const timeout = 5000;` -`{path:"a.ts",edits:[{loc:{{href 2 "const timeout = 5000;"}},sub:["5000","5001"]}]}` +`{path:"a.ts",edits:[{loc:{{href 10 "\tif (x) {"}},set:["\tif (!x) {"]}]}` # Combine `pre` + `set` + `post` in one entry `{path:"a.ts",edits:[{loc:{{href 6 "\tlog();"}},pre:["\tvalidate();"],set:["\tlog();"],post:["\tcleanup();"]}]}` # Replace one whole line with `set` -Use `set` when you're rewriting most of the line, or when `sub` would need a long `find`. +Use `set` to replace the full anchored line, preserving any unchanged surrounding lines yourself. `{path:"a.ts",edits:[{loc:{{href 3 "const tag = \"DO NOT SHIP\";"}},set:["const tag = \"OK\";"]}]}` # Replace multiple non-adjacent lines @@ -87,23 +79,18 @@ Use `set` when you're rewriting most of the line, or when `sub` would need a lon `{path:"a.ts",edits:[{loc:"$",post:["","export const VERSION = \"1.0.0\";"]}]}` # Cross-file override inside `loc` -`{path:"a.ts",edits:[{loc:"b.ts:{{href 2 "const timeout = 5000;"}}",sub:["5000","30_000"]}]}` +`{path:"a.ts",edits:[{loc:"b.ts:{{href 2 "const timeout = 5000;"}}",set:["const timeout = 30_000;"]}]}` - Make the minimum exact edit. - Copy the full anchors exactly as shown by `read/grep` (for example `160sr`, not just `sr`). - `loc` chooses the target. Verbs describe what to do there. -- On a single-anchor `loc`, you may combine `pre`, **one of** `set` or `sub`, and `post`. -- `set` and `sub` cannot appear together in the same entry. -- On a range `loc`, only `set` is allowed. +- On a single-anchor `loc`, you may combine `pre`, `set`, and `post`. - `loc:"^"` only supports `pre`. `loc:"$"` only supports `post`. -- For `sub`, the first tuple element (`find`) must occur **exactly once on the anchored line**. It never spans newlines. -- Prefer the **smallest** `sub` fragments. On a single line of code, 1–4 chars is usually enough (`"||"`, `"true"`, `"i--"`). -- **Switch to `set` when `sub` gets long.** If `find` would be more than ~half the line, or the replacement would restate most of the line, use `set` instead. - `set: []` deletes the anchored line. `set:[""]` preserves a blank line. - Within a single request you may submit edits in any order — the runtime applies them bottom-up so they don't shift each other. After any request that mutates a file, anchors below the mutation are stale on disk; re-read before issuing more edits to that file. -- `set`/`sub`/delete target the current file content only. Do not try to reference old line text after the file has changed. +- `set` operations target the current file content only. Do not try to reference old line text after the file has changed. - Text content must be literal file content with matching indentation. If the file uses tabs, use real tabs. - You **MUST NOT** use this tool to reformat or clean up unrelated code. diff --git a/packages/coding-agent/test/core/atom.test.ts b/packages/coding-agent/test/core/atom.test.ts index 9a8d874d6..71107c2af 100644 --- a/packages/coding-agent/test/core/atom.test.ts +++ b/packages/coding-agent/test/core/atom.test.ts @@ -3,12 +3,14 @@ import { type AtomEdit, type AtomToolEdit, applyAtomEdits, + atomEditSchema, computeLineHash, HashlineMismatchError, resolveAtomEntryPaths, resolveAtomToolEdit, } from "@oh-my-pi/pi-coding-agent/edit"; import type { Anchor } from "@oh-my-pi/pi-coding-agent/edit/modes/hashline"; +import { Value } from "@sinclair/typebox/value"; function tag(line: number, content: string): Anchor { return { line, hash: computeLineHash(line, content) }; @@ -83,47 +85,9 @@ describe("applyAtomEdits — pre/post", () => { }); }); -describe("applyAtomEdits — sub", () => { - it("replaces a unique substring", () => { - const content = "const timeout = 5000;"; - const edits: AtomEdit[] = [{ op: "sub", pos: tag(1, content), find: "5000", to: "30_000" }]; - const result = applyAtomEdits(content, edits); - expect(result.lines).toBe("const timeout = 30_000;"); - }); - - it("preserves the line tail (trailing semicolon, comma, brace)", () => { - const content = " required: true,"; - const edits: AtomEdit[] = [{ op: "sub", pos: tag(1, content), find: "true", to: "false" }]; - const result = applyAtomEdits(content, edits); - expect(result.lines).toBe(" required: false,"); - }); - - it("swaps an operator without restating the surrounding expression", () => { - const content = "\tfor (let i = 0; i < value.length; i--) {"; - const edits: AtomEdit[] = [{ op: "sub", pos: tag(1, content), find: "i--", to: "i++" }]; - const result = applyAtomEdits(content, edits); - expect(result.lines).toBe("\tfor (let i = 0; i < value.length; i++) {"); - }); - - it("errors when find is absent", () => { - const content = "abc def"; - const edits: AtomEdit[] = [{ op: "sub", pos: tag(1, content), find: "missing", to: "x" }]; - expect(() => applyAtomEdits(content, edits)).toThrow(/not found/); - }); - - it("errors when find is non-unique", () => { - const content = "abc abc"; - const edits: AtomEdit[] = [{ op: "sub", pos: tag(1, content), find: "abc", to: "Z" }]; - expect(() => applyAtomEdits(content, edits)).toThrow(/more than once/); - }); - - it("rejects conflict with set on same anchor", () => { - const content = "abc"; - const edits: AtomEdit[] = [ - { op: "sub", pos: tag(1, "abc"), find: "abc", to: "x" }, - { op: "set", pos: tag(1, "abc"), lines: ["y"] }, - ]; - expect(() => applyAtomEdits(content, edits)).toThrow(/Conflicting ops/); +describe("atom edit schema", () => { + it("rejects sub edits", () => { + expect(Value.Check(atomEditSchema, { loc: "1ab", sub: ["5000", "30_000"] })).toBe(false); }); }); @@ -175,7 +139,7 @@ describe("resolveAtomToolEdit — loc syntax", () => { it("ignores null optional verb fields", () => { const content = "aaa\nbbb\nccc"; const loc = `2${computeLineHash(2, "bbb")}`; - const toolEdit = { loc, pre: null, set: "BBB", post: null, sub: null } as unknown as AtomToolEdit; + const toolEdit = { loc, pre: null, set: "BBB", post: null } as unknown as AtomToolEdit; const resolved = resolveAtomToolEdit(toolEdit); expect(resolved).toEqual([{ op: "set", pos: tag(2, "bbb"), lines: ["BBB"] }]);