diff --git a/crates/pi-natives/src/chunk/indent.rs b/crates/pi-natives/src/chunk/indent.rs index 342724753..b620796ed 100644 --- a/crates/pi-natives/src/chunk/indent.rs +++ b/crates/pi-natives/src/chunk/indent.rs @@ -432,15 +432,17 @@ fn hashline_prefix_len(line: &str) -> Option { } // Match exactly one BPE bigram (2 ASCII chars) from HASHLINE_BIGRAMS. - if remainder.len() < 2 { + // Use char-boundary-safe slicing to avoid panicking on multi-byte content. + let bigram_end = 2; + if remainder.len() < bigram_end || !remainder.is_char_boundary(bigram_end) { return None; } - let bigram = &remainder[..2]; + let bigram = &remainder[..bigram_end]; if !bigram.is_ascii() || !HASHLINE_BIGRAMS.contains(&bigram) { return None; } - offset += 2; - remainder = &remainder[2..]; + offset += bigram_end; + remainder = &remainder[bigram_end..]; remainder.strip_prefix(':').map(|_| offset + 1) } diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index b2f1a0bd4..131cdbb9e 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## [Unreleased] + ### Breaking Changes - Changed the hashline and chunk anchor ID format from the prior hex-like tokens to two-letter BPE bigrams (for example `#th`), which invalidates previously captured `LINE#ID`/chunk selectors and requires re-reading to refresh anchors @@ -18,6 +19,9 @@ ### Fixed +- Fixed `atom` mode to apply multiple edits on the same anchor line without index-shift artifacts, so mixed operations like `before`, `after`, `set`, `sub`, `ins`, and `del` now resolve consistently +- Fixed `atom` mode `append_file` insertion to preserve a file’s trailing newline sentinel when appending content +- Fixed `read` output for raw archive entries so hashline anchors, line numbers, and chunked formatting are not injected into raw content - Fixed hashline parsing so lines like `# Note:` or `# TODO:` are no longer misinterpreted and stripped as hashline prefixes - Adjusted patch and replace validation to report a clear missing-path error when neither an entry path nor a top-level path is provided diff --git a/packages/coding-agent/src/edit/index.ts b/packages/coding-agent/src/edit/index.ts index 41d5c9de3..0e78a594e 100644 --- a/packages/coding-agent/src/edit/index.ts +++ b/packages/coding-agent/src/edit/index.ts @@ -18,26 +18,14 @@ import type { ToolSession } from "../tools"; import { VimTool, vimSchema } from "../tools/vim"; import { type EditMode, normalizeEditMode, resolveEditMode } from "../utils/edit-mode"; import type { VimToolDetails } from "../vim/types"; -import { - type ApplyPatchParams, - applyPatchSchema, - expandApplyPatchToEntries, - isApplyPatchParams, -} from "./modes/apply-patch"; +import { type ApplyPatchParams, applyPatchSchema, expandApplyPatchToEntries } from "./modes/apply-patch"; import applyPatchGrammar from "./modes/apply-patch.lark" with { type: "text" }; -import { - type AtomParams, - type AtomToolEdit, - atomEditParamsSchema, - executeAtomSingle, - isAtomParams, -} from "./modes/atom"; +import { type AtomParams, type AtomToolEdit, atomEditParamsSchema, executeAtomSingle } from "./modes/atom"; import { type ChunkParams, type ChunkToolEdit, chunkEditParamsSchema, executeChunkSingle, - isChunkParams, parseChunkEditPath, resolveAnchorStyle, resolveChunkAutoIndent, @@ -47,22 +35,9 @@ import { type HashlineParams, type HashlineToolEdit, hashlineEditParamsSchema, - isHashlineParams, } from "./modes/hashline"; -import { - executePatchSingle, - isPatchParams, - type PatchEditEntry, - type PatchParams, - patchEditSchema, -} from "./modes/patch"; -import { - executeReplaceSingle, - isReplaceParams, - type ReplaceEditEntry, - type ReplaceParams, - replaceEditSchema, -} from "./modes/replace"; +import { executePatchSingle, type PatchEditEntry, type PatchParams, patchEditSchema } from "./modes/patch"; +import { executeReplaceSingle, type ReplaceEditEntry, type ReplaceParams, replaceEditSchema } from "./modes/replace"; import { type EditToolDetails, type EditToolPerFileResult, getLspBatchRequest, type LspBatchRequest } from "./renderer"; export { DEFAULT_EDIT_MODE, type EditMode, normalizeEditMode } from "../utils/edit-mode"; @@ -102,8 +77,6 @@ type EditToolResultDetails = EditToolDetails | VimToolDetails; type EditModeDefinition = { description: (session: ToolSession) => string; parameters: TInput; - invalidParamsMessage: string; - validate: (params: EditParams) => boolean; execute: ( tool: EditTool, params: EditParams, @@ -126,10 +99,6 @@ function resolveConfiguredEditMode(rawEditMode: string): EditMode | undefined { return editMode; } -function isVimParams(params: EditParams): params is VimParams { - return typeof params === "object" && params !== null && "file" in params && typeof params.file === "string"; -} - function resolveAllowFuzzy(session: ToolSession, rawValue: string): boolean { switch (rawValue) { case "true": @@ -343,10 +312,6 @@ export class EditTool implements AgentTool { context?: AgentToolContext, ): Promise> { const modeDefinition = this.#getModeDefinition(); - if (!modeDefinition.validate(params)) { - throw new Error(modeDefinition.invalidParamsMessage); - } - return modeDefinition.execute(this, params, signal, getLspBatchRequest(context?.toolCall), onUpdate); } @@ -359,9 +324,6 @@ export class EditTool implements AgentTool { chunkAutoIndent: resolveChunkAutoIndent(), }), parameters: chunkEditParamsSchema, - invalidParamsMessage: - "Invalid edit parameters for chunk mode. Expected `{ edits: [{ path: 'file:selector', ...op }, ...] }` with at least one edit. Each edit needs a `path`; supply exactly one of `write: 'content'`, `insert: { loc, body }`, or `delete: true`.", - validate: isChunkParams, execute: ( tool: EditTool, params: EditParams, @@ -391,8 +353,6 @@ export class EditTool implements AgentTool { patch: { description: () => prompt.render(patchDescription), parameters: patchEditSchema, - invalidParamsMessage: "Invalid edit parameters for patch mode.", - validate: isPatchParams, execute: ( tool: EditTool, params: EditParams, @@ -422,8 +382,6 @@ export class EditTool implements AgentTool { apply_patch: { description: () => prompt.render(applyPatchDescription), parameters: applyPatchSchema, - invalidParamsMessage: "Invalid edit parameters for apply_patch mode.", - validate: isApplyPatchParams, execute: ( tool: EditTool, params: EditParams, @@ -452,8 +410,6 @@ export class EditTool implements AgentTool { hashline: { description: () => prompt.render(hashlineDescription), parameters: hashlineEditParamsSchema, - invalidParamsMessage: "Invalid edit parameters for hashline mode.", - validate: isHashlineParams, execute: ( tool: EditTool, params: EditParams, @@ -483,9 +439,6 @@ export class EditTool implements AgentTool { atom: { description: () => prompt.render(atomDescription), parameters: atomEditParamsSchema, - invalidParamsMessage: - "Edit tool requires `{ edits: [...] }` (an array of entries). Each entry needs exactly one op key (set, before, after, del, sub, ins, append, prepend) plus optional `path`.", - validate: isAtomParams, execute: ( tool: EditTool, params: EditParams, @@ -515,8 +468,6 @@ export class EditTool implements AgentTool { replace: { description: () => prompt.render(replaceDescription), parameters: replaceEditSchema, - invalidParamsMessage: "Invalid edit parameters for replace mode.", - validate: isReplaceParams, execute: ( tool: EditTool, params: EditParams, @@ -546,8 +497,6 @@ export class EditTool implements AgentTool { vim: { description: () => this.#vimTool.description, parameters: vimSchema, - invalidParamsMessage: "Invalid edit parameters for vim mode.", - validate: isVimParams, execute: async ( tool: EditTool, params: EditParams, diff --git a/packages/coding-agent/src/edit/modes/apply-patch.ts b/packages/coding-agent/src/edit/modes/apply-patch.ts index 8395da69e..95b1149b9 100644 --- a/packages/coding-agent/src/edit/modes/apply-patch.ts +++ b/packages/coding-agent/src/edit/modes/apply-patch.ts @@ -22,15 +22,6 @@ export const applyPatchSchema = Type.Object({ export type ApplyPatchParams = Static; -export function isApplyPatchParams(params: unknown): params is ApplyPatchParams { - return ( - typeof params === "object" && - params !== null && - "input" in params && - typeof (params as { input: unknown }).input === "string" - ); -} - /** * Parse the envelope and lower each hunk to a `PatchEditEntry` so it can * be routed through `executePatchSingle`. diff --git a/packages/coding-agent/src/edit/modes/atom.ts b/packages/coding-agent/src/edit/modes/atom.ts index 803a217ac..a484a18f4 100644 --- a/packages/coding-agent/src/edit/modes/atom.ts +++ b/packages/coding-agent/src/edit/modes/atom.ts @@ -48,7 +48,7 @@ const linesSchema = Type.Union([Type.Array(Type.String()), Type.String()]); /** * Flat entry shape: every op key is optional, and the runtime validator - * (`isAtomParams` + `resolveAtomToolEdit`) enforces that exactly one op key + * (`resolveAtomToolEdit`) enforces that exactly one op key * is present per entry. We use a flat schema instead of a 9-member discriminated * union to keep the tool definition compact (the schema is re-sent on every * turn, so duplicating `path` + descriptions across 9 union members 2×'s @@ -111,15 +111,6 @@ export type AtomEdit = const ATOM_OP_KEYS = ["set", "before", "after", "del", "sub", "ins", "append", "prepend"] as const; -export function isAtomParams(params: unknown): params is AtomParams { - // Minimal shape check. Per-entry validation (op key presence, exclusivity, - // payload sanity) all happens in `resolveAtomToolEdit` so the model gets a - // specific actionable error rather than a generic "invalid parameters" message. - if (typeof params !== "object" || params === null) return false; - if (!("edits" in params) || !Array.isArray(params.edits)) return false; - return true; -} - // ═══════════════════════════════════════════════════════════════════════════ // Resolution // ═══════════════════════════════════════════════════════════════════════════ @@ -273,122 +264,30 @@ function validateNoConflictingAnchorOps(edits: AtomEdit[]): void { // Apply // ═══════════════════════════════════════════════════════════════════════════ -function getAtomEditSortKey(edit: AtomEdit, fileLineCount: number): { sortLine: number; precedence: number } { - switch (edit.op) { - case "sub": - case "ins": - return { sortLine: edit.pos.line, precedence: -1 }; - case "set": - case "del": - return { sortLine: edit.pos.line, precedence: 0 }; - case "after": - return { sortLine: edit.pos.line, precedence: 1 }; - case "before": - return { sortLine: edit.pos.line, precedence: 2 }; - case "append_file": - return { sortLine: fileLineCount + 1, precedence: 3 }; - case "prepend_file": - return { sortLine: 0, precedence: 3 }; +function applySubInsToLine( + edit: { op: "sub" | "ins"; pos: Anchor; find: string; to: string }, + current: string, +): string { + const first = current.indexOf(edit.find); + if (first === -1) { + throw new Error( + `${edit.op}: substring \`${edit.find}\` not found on line ${edit.pos.line}. ` + + `Current line content: ${JSON.stringify(current)}`, + ); } -} - -function applyAtomEditToLines(edit: AtomEdit, fileLines: string[], trackFirstChanged: (line: number) => void): void { - switch (edit.op) { - case "set": { - const lines = edit.lines.length === 0 ? [""] : edit.lines; - fileLines.splice(edit.pos.line - 1, 1, ...lines); - trackFirstChanged(edit.pos.line); - break; - } - case "del": { - fileLines.splice(edit.pos.line - 1, 1); - trackFirstChanged(edit.pos.line); - break; - } - case "before": { - if (edit.lines.length === 0) break; - fileLines.splice(edit.pos.line - 1, 0, ...edit.lines); - trackFirstChanged(edit.pos.line); - break; - } - case "after": { - if (edit.lines.length === 0) break; - fileLines.splice(edit.pos.line, 0, ...edit.lines); - trackFirstChanged(edit.pos.line + 1); - break; - } - case "sub": { - const idx = edit.pos.line - 1; - const current = fileLines[idx]; - 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)}`, - ); - } - const next = current.slice(0, first) + edit.to + current.slice(first + edit.find.length); - // Allow `to` to introduce newlines, expanding into multiple lines. - const newLines = next.includes("\n") ? next.split("\n") : [next]; - fileLines.splice(idx, 1, ...newLines); - trackFirstChanged(edit.pos.line); - break; - } - case "ins": { - const idx = edit.pos.line - 1; - const current = fileLines[idx]; - const first = current.indexOf(edit.find); - if (first === -1) { - throw new Error( - `ins: 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( - `ins: substring \`${edit.find}\` occurs more than once on line ${edit.pos.line}; ` + - `use a longer substring that uniquely identifies the position. ` + - `Current line content: ${JSON.stringify(current)}`, - ); - } - // Replace from start of `find` to end-of-line with `to` (vim-insert style). - const next = current.slice(0, first) + edit.to; - const newLines = next.includes("\n") ? next.split("\n") : [next]; - fileLines.splice(idx, 1, ...newLines); - trackFirstChanged(edit.pos.line); - break; - } - case "append_file": { - if (edit.lines.length === 0) break; - if (fileLines.length === 1 && fileLines[0] === "") { - fileLines.splice(0, 1, ...edit.lines); - trackFirstChanged(1); - } else { - fileLines.splice(fileLines.length, 0, ...edit.lines); - trackFirstChanged(fileLines.length - edit.lines.length + 1); - } - break; - } - case "prepend_file": { - if (edit.lines.length === 0) break; - if (fileLines.length === 1 && fileLines[0] === "") { - fileLines.splice(0, 1, ...edit.lines); - } else { - fileLines.splice(0, 0, ...edit.lines); - } - trackFirstChanged(1); - break; - } + const second = current.indexOf(edit.find, first + 1); + if (second !== -1) { + throw new Error( + `${edit.op}: substring \`${edit.find}\` occurs more than once on line ${edit.pos.line}; ` + + `use a longer substring that uniquely identifies the ${edit.op === "sub" ? "target" : "position"}. ` + + `Current line content: ${JSON.stringify(current)}`, + ); } + if (edit.op === "sub") { + return current.slice(0, first) + edit.to + current.slice(first + edit.find.length); + } + // `ins`: replace from start of `find` to end-of-line (vim-insert style). + return current.slice(0, first) + edit.to; } function maybeAutocorrectEscapedTabIndentation(edits: AtomEdit[], warnings: string[]): void { @@ -439,21 +338,127 @@ export function applyAtomEdits( validateNoConflictingAnchorOps(edits); maybeAutocorrectEscapedTabIndentation(edits, warnings); - const annotated = edits - .map((edit, idx) => { - const { sortLine, precedence } = getAtomEditSortKey(edit, fileLines.length); - return { edit, idx, sortLine, precedence }; - }) - .sort((a, b) => b.sortLine - a.sortLine || a.precedence - b.precedence || a.idx - b.idx); - const trackFirstChanged = (line: number) => { if (firstChangedLine === undefined || line < firstChangedLine) { firstChangedLine = line; } }; - for (const { edit } of annotated) { - applyAtomEditToLines(edit, fileLines, trackFirstChanged); + // Partition: anchor-scoped vs file-scoped. Preserve original order via the + // captured idx so multiple before/after/append/prepend on the same target + // are emitted in the order the model produced them. + type Indexed = { edit: T; idx: number }; + const anchorEdits: Indexed>[] = []; + const appendEdits: Indexed>[] = []; + const prependEdits: Indexed>[] = []; + edits.forEach((edit, idx) => { + if (edit.op === "append_file") appendEdits.push({ edit, idx }); + else if (edit.op === "prepend_file") prependEdits.push({ edit, idx }); + else anchorEdits.push({ edit, idx }); + }); + + // Group anchor edits by line so all ops on the same line are applied as a + // single splice. This makes the per-anchor outcome independent of index + // shifts caused by sibling ops (e.g. `after` paired with `del` on the same + // anchor, or repeated `before`/`after` inserts that previously reversed). + const byLine = new Map[]>(); + for (const entry of anchorEdits) { + const line = entry.edit.pos.line; + let bucket = byLine.get(line); + if (!bucket) { + bucket = []; + byLine.set(line, bucket); + } + bucket.push(entry); + } + + // Process anchor groups bottom-up so earlier-line edits aren't perturbed. + const sortedLines = [...byLine.keys()].sort((a, b) => b - a); + for (const line of sortedLines) { + const bucket = byLine.get(line)!; + bucket.sort((a, b) => a.idx - b.idx); + + const idx = line - 1; + let currentLine = fileLines[idx]; + let replacement: string[] = [currentLine]; + let replacementSet = false; + let anchorMutated = false; + let anchorDeleted = false; + const beforeLines: string[] = []; + const afterLines: string[] = []; + + for (const { edit } of bucket) { + switch (edit.op) { + case "before": + beforeLines.push(...edit.lines); + break; + case "after": + afterLines.push(...edit.lines); + break; + case "del": + replacement = []; + replacementSet = true; + anchorDeleted = true; + break; + case "set": + replacement = edit.lines.length === 0 ? [""] : [...edit.lines]; + replacementSet = true; + anchorMutated = true; + break; + case "sub": + case "ins": { + currentLine = applySubInsToLine(edit, currentLine); + replacement = currentLine.includes("\n") ? currentLine.split("\n") : [currentLine]; + replacementSet = true; + anchorMutated = true; + break; + } + } + } + + const noOp = !replacementSet && beforeLines.length === 0 && afterLines.length === 0; + if (noOp) continue; + + const combined = [...beforeLines, ...replacement, ...afterLines]; + fileLines.splice(idx, 1, ...combined); + + if (beforeLines.length > 0 || anchorMutated || anchorDeleted) { + trackFirstChanged(line); + } else if (afterLines.length > 0) { + trackFirstChanged(line + 1); + } + } + + // Apply prepend_file ops in original order so the first one ends up at the + // very top of the file. + prependEdits.sort((a, b) => a.idx - b.idx); + for (const { edit } of prependEdits) { + if (edit.lines.length === 0) continue; + if (fileLines.length === 1 && fileLines[0] === "") { + fileLines.splice(0, 1, ...edit.lines); + } else { + // Insert in reverse cumulative order so later splices push earlier + // content further down, preserving the original op order. + fileLines.splice(0, 0, ...edit.lines); + } + trackFirstChanged(1); + } + + // Apply append_file ops in original order. When the file ends with a + // trailing newline (last split element is the empty sentinel), insert + // before that sentinel so the trailing newline is preserved. + appendEdits.sort((a, b) => a.idx - b.idx); + for (const { edit } of appendEdits) { + if (edit.lines.length === 0) continue; + if (fileLines.length === 1 && fileLines[0] === "") { + fileLines.splice(0, 1, ...edit.lines); + trackFirstChanged(1); + continue; + } + const hasTrailingNewline = fileLines.length > 0 && fileLines[fileLines.length - 1] === ""; + const insertIdx = hasTrailingNewline ? fileLines.length - 1 : fileLines.length; + fileLines.splice(insertIdx, 0, ...edit.lines); + trackFirstChanged(insertIdx + 1); } return { diff --git a/packages/coding-agent/src/edit/modes/chunk.ts b/packages/coding-agent/src/edit/modes/chunk.ts index 2364a1dd6..570a0dc0c 100644 --- a/packages/coding-agent/src/edit/modes/chunk.ts +++ b/packages/coding-agent/src/edit/modes/chunk.ts @@ -552,9 +552,11 @@ export function missingChunkReadTarget(selector: string): ChunkReadTarget { export const chunkToolEditSchema = Type.Object( { - path: Type.String({ - description: "File path with chunk selector. Examples: 'src/app.ts:fn_foo#thth~', 'src/app.ts:class_Bar'.", - }), + path: Type.Optional( + Type.String({ + description: "File path with chunk selector. Examples: 'src/app.ts:fn_foo#thth~', 'src/app.ts:class_Bar'.", + }), + ), write: Type.Optional( Type.Union([Type.String(), Type.Null()], { description: @@ -602,22 +604,6 @@ export interface ExecuteChunkSingleOptions { beginDeferredDiagnosticsForPath: (path: string) => WritethroughDeferredHandle; } -export function isChunkParams(params: unknown): params is ChunkParams { - if ( - typeof params !== "object" || - params === null || - !("edits" in params) || - !Array.isArray(params.edits) || - params.edits.length === 0 - ) { - return false; - } - const first = params.edits[0]; - // Accept a bare `{ path }` entry so the executor can return a targeted - // "missing operation" error instead of the generic schema failure. - return typeof first === "object" && first !== null && "path" in first; -} - /** Auto-correct indentation for content targeting a body region (`~`) when autoIndent is on. * Handles two patterns: * 1. Tab-based over-indentation: models include the function's base \t indent. diff --git a/packages/coding-agent/src/edit/modes/hashline.ts b/packages/coding-agent/src/edit/modes/hashline.ts index 62568ad71..ad9d180d9 100644 --- a/packages/coding-agent/src/edit/modes/hashline.ts +++ b/packages/coding-agent/src/edit/modes/hashline.ts @@ -43,12 +43,14 @@ export type HashlineEdit = | { op: "append_file"; lines: string[] } | { op: "prepend_file"; lines: string[] }; +// Tight prefix matchers. The bare `#BIGRAM:` form (no line number) intentionally +// disallows whitespace between `#` and the bigram so real comments like `# th: ...` +// or `# in: ...` (a `#`, a space, then a common English bigram) are not mistaken +// for hashline anchors and stripped. const HASHLINE_PREFIX_RE = new RegExp( - `^\\s*(?:>>>|>>)?\\s*(?:\\+?\\s*(?:\\d+\\s*#\\s*|#\\s*)|\\+)\\s*${HASHLINE_BIGRAM_RE_SRC}:`, -); -const HASHLINE_PREFIX_PLUS_RE = new RegExp( - `^\\s*(?:>>>|>>)?\\s*\\+\\s*(?:\\d+\\s*#\\s*|#\\s*)?${HASHLINE_BIGRAM_RE_SRC}:`, + `^\\s*(?:>>>|>>)?\\s*(?:\\+?\\s*\\d+\\s*#\\s*|\\+?#|\\+\\s*)${HASHLINE_BIGRAM_RE_SRC}:`, ); +const HASHLINE_PREFIX_PLUS_RE = new RegExp(`^\\s*(?:>>>|>>)?\\s*\\+\\s*(?:\\d+\\s*#\\s*|#)?${HASHLINE_BIGRAM_RE_SRC}:`); const DIFF_PLUS_RE = /^[+](?![+])/; const READ_TRUNCATION_NOTICE_RE = /^\[(?:Showing lines \d+-\d+ of \d+|\d+ more lines? in (?:file|\S+))\b.*\bsel=L\d+/; @@ -188,15 +190,6 @@ export function hashlineParseText(edit: string[] | string | null | undefined): s return stripNewLinePrefixes(edit); } -export function isHashlineParams(params: unknown): params is HashlineParams { - if (typeof params !== "object" || params === null || !("edits" in params) || !Array.isArray(params.edits)) - return false; - if (params.edits.length === 0) return true; - const first = params.edits[0]; - if (typeof first !== "object" || first === null) return false; - return "loc" in first; -} - function resolveEditAnchors(edits: HashlineToolEdit[]): HashlineEdit[] { return edits.map(resolveEditAnchor); } diff --git a/packages/coding-agent/src/edit/modes/patch.ts b/packages/coding-agent/src/edit/modes/patch.ts index 0c89af124..d8f0c6fa9 100644 --- a/packages/coding-agent/src/edit/modes/patch.ts +++ b/packages/coding-agent/src/edit/modes/patch.ts @@ -1606,14 +1606,6 @@ export interface ExecutePatchSingleOptions { beginDeferredDiagnosticsForPath: (path: string) => WritethroughDeferredHandle; } -export function isPatchParams(params: unknown): params is PatchParams { - if (typeof params !== "object" || params === null) return false; - if (!("edits" in params) || !Array.isArray((params as any).edits)) return false; - const first = (params as any).edits[0]; - if (!first || typeof first !== "object") return false; - return "path" in first && !("old_text" in first) && !("new_text" in first); -} - class LspFileSystem implements FileSystem { #lastDiagnostics: FileDiagnosticsResult | undefined; #fileCache: Record = {}; diff --git a/packages/coding-agent/src/edit/modes/replace.ts b/packages/coding-agent/src/edit/modes/replace.ts index 29a570de9..e41ca4174 100644 --- a/packages/coding-agent/src/edit/modes/replace.ts +++ b/packages/coding-agent/src/edit/modes/replace.ts @@ -1002,13 +1002,6 @@ export interface ExecuteReplaceSingleOptions { beginDeferredDiagnosticsForPath: (path: string) => WritethroughDeferredHandle; } -export function isReplaceParams(params: unknown): params is ReplaceParams { - if (typeof params !== "object" || params === null) return false; - if (!("edits" in params) || !Array.isArray((params as any).edits)) return false; - const first = (params as any).edits[0]; - return first && typeof first === "object" && "old_text" in first && "new_text" in first; -} - export async function executeReplaceSingle( options: ExecuteReplaceSingleOptions, ): Promise> { diff --git a/packages/coding-agent/src/edit/streaming.ts b/packages/coding-agent/src/edit/streaming.ts index 0b95a9cd2..1d5296590 100644 --- a/packages/coding-agent/src/edit/streaming.ts +++ b/packages/coding-agent/src/edit/streaming.ts @@ -131,6 +131,7 @@ export function dropIncompleteLastEdit(edits: readonly T[], partialJson: stri // ----------------------------------------------------------------------------- interface ReplaceArgs { + path?: string; edits?: ReplaceEditEntry[]; __partialJson?: string; } @@ -142,10 +143,12 @@ const replaceStrategy: EditStreamingStrategy = { }, async computeDiffPreview(args, ctx) { const first = args.edits?.[0]; - if (!first?.path || first.old_text === undefined || first.new_text === undefined) return null; + if (!first) return null; + const path = first.path ?? args.path; + if (!path || first.old_text === undefined || first.new_text === undefined) return null; ctx.signal.throwIfAborted(); const result = await computeEditDiff( - first.path, + path, first.old_text, first.new_text, ctx.cwd, @@ -154,7 +157,7 @@ const replaceStrategy: EditStreamingStrategy = { ctx.fuzzyThreshold, ); ctx.signal.throwIfAborted(); - return [toPerFilePreview(first.path, result)]; + return [toPerFilePreview(path, result)]; }, renderStreamingFallback() { return ""; @@ -162,6 +165,7 @@ const replaceStrategy: EditStreamingStrategy = { }; interface PatchArgs { + path?: string; edits?: PatchEditEntry[]; __partialJson?: string; } @@ -173,15 +177,16 @@ const patchStrategy: EditStreamingStrategy = { }, async computeDiffPreview(args, ctx) { const first = args.edits?.[0]; - if (!first?.path) return null; + const path = first?.path ?? args.path; + if (!path) return null; ctx.signal.throwIfAborted(); const result = await computePatchDiff( - { path: first.path, op: first.op ?? "update", rename: first.rename, diff: first.diff }, + { path, op: first?.op ?? "update", rename: first?.rename, diff: first?.diff }, ctx.cwd, { fuzzyThreshold: ctx.fuzzyThreshold, allowFuzzy: ctx.allowFuzzy }, ); ctx.signal.throwIfAborted(); - return [toPerFilePreview(first.path, result)]; + return [toPerFilePreview(path, result)]; }, renderStreamingFallback() { return ""; @@ -189,6 +194,7 @@ const patchStrategy: EditStreamingStrategy = { }; interface HashlineArgs { + path?: string; edits?: HashlineToolEdit[]; __partialJson?: string; } @@ -200,11 +206,16 @@ const hashlineStrategy: EditStreamingStrategy = { }, async computeDiffPreview(args, ctx) { const first = args.edits?.[0] as (HashlineToolEdit & { path?: string }) | undefined; - if (!first?.path) return null; - const path = first.path; - const fileEdits = (args.edits ?? []).filter((e): e is HashlineToolEdit & { path: string } => { - return !!e && typeof e === "object" && (e as { path?: string }).path === path; - }); + const path = first?.path ?? args.path; + if (!path) return null; + const fileEdits = (args.edits ?? []) + .map(e => { + if (!e || typeof e !== "object") return undefined; + const entryPath = (e as { path?: string }).path ?? args.path; + if (!entryPath || entryPath !== path) return undefined; + return { ...(e as HashlineToolEdit), path } as HashlineToolEdit & { path: string }; + }) + .filter((e): e is HashlineToolEdit & { path: string } => e !== undefined); ctx.signal.throwIfAborted(); const result = await computeHashlineDiff({ path, edits: fileEdits }, ctx.cwd); ctx.signal.throwIfAborted(); @@ -216,6 +227,7 @@ const hashlineStrategy: EditStreamingStrategy = { }; interface ChunkArgs { + path?: string; edits?: ChunkToolEdit[]; __partialJson?: string; } @@ -247,8 +259,10 @@ const chunkStrategy: EditStreamingStrategy = { const groups = new Map(); const fileOrder: string[] = []; for (const edit of edits) { - if (!edit?.path) continue; - const { filePath } = parseChunkEditPath(edit.path); + if (!edit) continue; + const editPath = edit.path ?? args.path; + if (!editPath) continue; + const { filePath } = parseChunkEditPath(editPath); if (!filePath) continue; let bucket = groups.get(filePath); if (!bucket) { @@ -256,7 +270,7 @@ const chunkStrategy: EditStreamingStrategy = { groups.set(filePath, bucket); fileOrder.push(filePath); } - bucket.push(edit); + bucket.push({ ...edit, path: editPath }); } if (fileOrder.length === 0) return null; diff --git a/packages/coding-agent/src/prompts/tools/chunk-edit.md b/packages/coding-agent/src/prompts/tools/chunk-edit.md index 5ca211a2a..0e0bc336d 100644 --- a/packages/coding-agent/src/prompts/tools/chunk-edit.md +++ b/packages/coding-agent/src/prompts/tools/chunk-edit.md @@ -140,17 +140,17 @@ Only body changes; doc comment, signature, and closing `}` are preserved. `{ "path": "counter.rs:impl_Counte.fn_increm#ouer", "write": "/// Increments by the given step, clamping at max.\npub fn increment(&mut self, step: i32) {\n\tself.value = (self.value + step).min(self.max);\n}\n" }` Everything is rewritten. Omitting the doc comment or signature deletes them. # Write head (`^` — attributes, doc comments, signature) -`{ "path": "counter.rs:impl_Counte.fn_get#arco^", "write": "/// Returns the current counter value.\n#[inline]\npub fn get(&self) → i32 {\n" }` +`{ "path": "counter.rs:impl_Counte.fn_get#arco^", "write": "/// Returns the current counter value.\n#[inline]\npub fn get(&self) -> i32 {\n" }` Head changes (all `^` lines + opening brace); body untouched. # Insert before a chunk (`prepend`) `{ "path": "counter.rs:impl_Counte.fn_get", "insert": { "loc": "prepend", "body": "/// Resets the counter to zero.\npub fn reset(&mut self) {\n\tself.value = 0;\n}\n\n" } }` # Insert after a chunk (`append`) -`{ "path": "counter.rs:struct_Counte", "insert": { "loc": "append", "body": "\nimpl Default for Counter {\n\tfn default() → Self {\n\t\tSelf { value: 0, max: 100 }\n\t}\n}\n" } }` +`{ "path": "counter.rs:struct_Counte", "insert": { "loc": "append", "body": "\nimpl Default for Counter {\n\tfn default() -> Self {\n\t\tSelf { value: 0, max: 100 }\n\t}\n}\n" } }` # Insert at start of container body (`~` + `prepend`) -`{ "path": "counter.rs:impl_Counte~", "insert": { "loc": "prepend", "body": "/// Creates a counter starting at the given value.\npub fn with_value(value: i32, max: i32) → Self {\n\tSelf { value: value.min(max), max }\n}\n\n" } }` +`{ "path": "counter.rs:impl_Counte~", "insert": { "loc": "prepend", "body": "/// Creates a counter starting at the given value.\npub fn with_value(value: i32, max: i32) -> Self {\n\tSelf { value: value.min(max), max }\n}\n\n" } }` Lands at the top of the impl body, before existing methods. # Insert at end of container body (`~` + `append`) -`{ "path": "counter.rs:impl_Counte~", "insert": { "loc": "append", "body": "\n/// Returns true if the counter is at its maximum.\npub fn is_maxed(&self) → bool {\n\tself.value ≥ self.max\n}\n" } }` +`{ "path": "counter.rs:impl_Counte~", "insert": { "loc": "append", "body": "\n/// Returns true if the counter is at its maximum.\npub fn is_maxed(&self) -> bool {\n\tself.value >= self.max\n}\n" } }` Lands at the end of the impl body, before the closing `}`. # Delete a chunk `{ "path": "counter.rs:impl_Counte.fn_decrem#arve", "delete": true }` diff --git a/packages/coding-agent/src/tools/read.ts b/packages/coding-agent/src/tools/read.ts index 51fec2c56..cd6575207 100644 --- a/packages/coding-agent/src/tools/read.ts +++ b/packages/coding-agent/src/tools/read.ts @@ -599,9 +599,10 @@ export class ReadTool implements AgentTool { sourceInternal?: string; entityLabel: string; ignoreResultLimits?: boolean; + raw?: boolean; }, ): AgentToolResult { - const displayMode = resolveFileDisplayMode(this.session); + const displayMode = resolveFileDisplayMode(this.session, { raw: options.raw }); const details = options.details ?? {}; const allLines = text.split("\n"); const totalLines = allLines.length; @@ -743,6 +744,7 @@ export class ReadTool implements AgentTool { limit: number | undefined, resolvedArchivePath: ResolvedArchiveReadPath, signal?: AbortSignal, + options?: { raw?: boolean }, ): Promise> { throwIfAborted(signal); const archive = await openArchive(resolvedArchivePath.absolutePath); @@ -787,6 +789,7 @@ export class ReadTool implements AgentTool { details, sourcePath: resolvedArchivePath.absolutePath, entityLabel: "archive entry", + raw: options?.raw, }); const firstText = result.content.find((content): content is TextContent => content.type === "text"); if (firstText) { @@ -978,7 +981,7 @@ export class ReadTool implements AgentTool { const archivePath = await this.#resolveArchiveReadPath(localReadPath, signal); if (archivePath) { const { offset, limit } = selToOffsetLimit(parsed); - return this.#readArchive(readPath, offset, limit, archivePath, signal); + return this.#readArchive(readPath, offset, limit, archivePath, signal, { raw: parsed.kind === "raw" }); } const sqlitePath = await this.#resolveSqliteReadPath(readPath, signal); diff --git a/packages/coding-agent/src/utils/file-display-mode.ts b/packages/coding-agent/src/utils/file-display-mode.ts index e4140f90a..e9a26ee8d 100644 --- a/packages/coding-agent/src/utils/file-display-mode.ts +++ b/packages/coding-agent/src/utils/file-display-mode.ts @@ -22,18 +22,24 @@ export interface FileDisplayModeSession { /** * Computes effective line display mode from session settings/env. * Hashline mode takes precedence and implies line-addressed output everywhere. - * Hashlines are suppressed when the edit tool is not available (e.g. explore agents). + * Hashlines are suppressed when the edit tool is not available (e.g. explore agents) + * and when the caller signals a `raw` read — raw output should be returned as-is + * without injecting hashline anchors or line numbers. */ -export function resolveFileDisplayMode(session: FileDisplayModeSession): FileDisplayMode { +export function resolveFileDisplayMode( + session: FileDisplayModeSession, + options?: { raw?: boolean }, +): FileDisplayMode { const { settings } = session; const hasEditTool = session.hasEditTool ?? true; const editMode = resolveEditMode(session); const usesHashLineAnchors = editMode === "hashline" || editMode === "atom"; - const hashLines = hasEditTool && usesHashLineAnchors && settings.get("readHashLines") !== false; - const chunked = hasEditTool && resolveEditMode(session) === "chunk"; + const raw = options?.raw === true; + const hashLines = !raw && hasEditTool && usesHashLineAnchors && settings.get("readHashLines") !== false; + const chunked = !raw && hasEditTool && editMode === "chunk"; return { hashLines, - lineNumbers: hashLines || settings.get("readLineNumbers") === true, + lineNumbers: !raw && (hashLines || settings.get("readLineNumbers") === true), chunked, }; } diff --git a/packages/coding-agent/test/core/atom.test.ts b/packages/coding-agent/test/core/atom.test.ts index fabe553bf..86e1fe202 100644 --- a/packages/coding-agent/test/core/atom.test.ts +++ b/packages/coding-agent/test/core/atom.test.ts @@ -4,7 +4,6 @@ import { applyAtomEdits, computeLineHash, HashlineMismatchError, - isAtomParams, resolveAtomToolEdit, } from "@oh-my-pi/pi-coding-agent/edit"; import type { Anchor } from "@oh-my-pi/pi-coding-agent/edit/modes/hashline"; @@ -13,36 +12,6 @@ function tag(line: number, content: string): Anchor { return { line, hash: computeLineHash(line, content) }; } -describe("isAtomParams", () => { - it("accepts empty edits", () => { - expect(isAtomParams({ edits: [] })).toBe(true); - }); - - it("accepts a `set` op", () => { - expect(isAtomParams({ edits: [{ path: "a.ts", set: "1#aa", lines: "x" }] })).toBe(true); - }); - - it("accepts a `sub` op", () => { - expect(isAtomParams({ edits: [{ path: "a.ts", sub: "1#aa", find: "x", lines: "y" }] })).toBe(true); - }); - - it("accepts file-scoped append", () => { - expect(isAtomParams({ edits: [{ path: "a.ts", append: "z" }] })).toBe(true); - }); - - it("defers multi-op entries to the resolver for an actionable error", () => { - // Schema-level guard accepts; resolver throws with named keys. - expect(isAtomParams({ edits: [{ path: "a.ts", set: "1#aa", lines: "x", del: "1#aa" }] })).toBe(true); - expect(() => resolveAtomToolEdit({ path: "a.ts", set: "1#aa", lines: "x", del: "1#aa" } as never)).toThrow( - /multiple op keys.*set, del/, - ); - }); - - it("accepts entries without path (resolved at dispatcher from top-level)", () => { - expect(isAtomParams({ path: "a.ts", edits: [{ set: "1#XQ", lines: "x" }] as unknown[] })).toBe(true); - }); -}); - describe("applyAtomEdits — set", () => { it("replaces a single line", () => { const content = "aaa\nbbb\nccc"; diff --git a/packages/coding-agent/test/tools/search-path-lists.test.ts b/packages/coding-agent/test/tools/search-path-lists.test.ts index fb219517a..cf88d0987 100644 --- a/packages/coding-agent/test/tools/search-path-lists.test.ts +++ b/packages/coding-agent/test/tools/search-path-lists.test.ts @@ -352,8 +352,8 @@ describe("search tool path lists", () => { const text = getText(result); expect(text).toContain("match lines use ':'; context lines use '-'"); - expect(text).toMatch(/1(?:#[A-Z0-9]+)?-#if FLAG/); - expect(text).toMatch(/2(?:#[A-Z0-9]+)?:needle/); - expect(text).toMatch(/3(?:#[A-Z0-9]+)?-#endif/); + expect(text).toMatch(/1(?:#[A-Za-z0-9]+)?-#if FLAG/); + expect(text).toMatch(/2(?:#[A-Za-z0-9]+)?:needle/); + expect(text).toMatch(/3(?:#[A-Za-z0-9]+)?-#endif/); }); }); diff --git a/packages/typescript-edit-benchmark/src/runner.ts b/packages/typescript-edit-benchmark/src/runner.ts index bae6fe70c..aea863ab9 100644 --- a/packages/typescript-edit-benchmark/src/runner.ts +++ b/packages/typescript-edit-benchmark/src/runner.ts @@ -191,11 +191,11 @@ const HASHLINE_SUBTYPES = ["set", "set_range", "insert"] as const; const CHUNK_OP_SUBTYPES = ["append", "prepend", "replace", "delete"] as const; -const BENCHMARK_TOOL_NAMES = ["read", "edit", "vim", "write"] as const; -const EDIT_TOOL_NAMES = ["edit", "vim"] as const; +const BENCHMARK_TOOL_NAMES = ["read", "edit", "vim", "write", "apply_patch"] as const; +const EDIT_TOOL_NAMES = ["edit", "vim", "apply_patch"] as const; function isEditTool(toolName: unknown): toolName is (typeof EDIT_TOOL_NAMES)[number] { - return toolName === "edit" || toolName === "vim"; + return toolName === "edit" || toolName === "vim" || toolName === "apply_patch"; } function isMutationTool(toolName: unknown): boolean {