diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 4cee6a123..15b3b527b 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## [Unreleased] + ### Breaking Changes - Renamed atom edit operations from `before` and `after` to `pre` and `post`, so existing `atom` payloads using the old operation keys must be updated @@ -18,6 +19,8 @@ ### Changed +- Changed edit diff wrapping to preserve the active line-prefix separator (`|` or `│`) while keeping continuation lines aligned by line-number width +- Changed Vim focus and viewport rendering to align cursor/selection markers and line numbers in a single gutter format - Changed auto image provider selection for `providers.image=auto` to try active GPT image generation before Antigravity, OpenRouter, and Gemini - Updated atom and hashline anchor validation to require the full `line+suffix` anchor format and report missing-line-number errors more clearly, including guidance when only a 2-letter suffix is provided - Changed read, grep, and ast-edit line-prefixed output to drop fixed-width line number padding, so anchors render in natural width without leading spaces diff --git a/packages/coding-agent/src/edit/diff.ts b/packages/coding-agent/src/edit/diff.ts index d59ddf61e..e9703d229 100644 --- a/packages/coding-agent/src/edit/diff.ts +++ b/packages/coding-agent/src/edit/diff.ts @@ -50,7 +50,6 @@ export class ApplyPatchError extends Error { // Diff String Generation // ═══════════════════════════════════════════════════════════════════════════ - function formatNumberedDiffLine(prefix: "+" | "-" | " ", lineNum: number, content: string): string { return `${prefix}${lineNum}|${content}`; } @@ -63,7 +62,6 @@ export function generateDiffString(oldContent: string, newContent: string, conte const parts = Diff.diffLines(oldContent, newContent); const output: string[] = []; - let oldLineNum = 1; let newLineNum = 1; let lastWasChange = false; diff --git a/packages/coding-agent/src/edit/index.ts b/packages/coding-agent/src/edit/index.ts index b35c8219f..20df32e5d 100644 --- a/packages/coding-agent/src/edit/index.ts +++ b/packages/coding-agent/src/edit/index.ts @@ -20,7 +20,13 @@ import { type EditMode, normalizeEditMode, resolveEditMode } from "../utils/edit import type { VimToolDetails } from "../vim/types"; 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 } from "./modes/atom"; +import { + type AtomParams, + type AtomToolEdit, + atomEditParamsSchema, + executeAtomSingle, + resolveAtomEntryPaths, +} from "./modes/atom"; import { type ChunkParams, type ChunkToolEdit, @@ -449,7 +455,7 @@ export class EditTool implements AgentTool { onUpdate?: (partialResult: AgentToolResult) => void, ) => { const { edits, path: topPath } = params as AtomParams & { path?: string }; - const resolved = resolveEntryPaths(edits as AtomToolEdit[], topPath); + const resolved = resolveAtomEntryPaths(edits as AtomToolEdit[], topPath); const byFile = groupBy(resolved, e => e.path); const entries = [...byFile.entries()].map(([path, fileEdits]) => ({ path, diff --git a/packages/coding-agent/src/edit/modes/hashline.ts b/packages/coding-agent/src/edit/modes/hashline.ts index d184677f7..7c3dda2f0 100644 --- a/packages/coding-agent/src/edit/modes/hashline.ts +++ b/packages/coding-agent/src/edit/modes/hashline.ts @@ -23,6 +23,7 @@ import { invalidateFsScanAfterWrite } from "../../tools/fs-cache-invalidation"; import { outputMeta } from "../../tools/output-meta"; import { resolveToCwd } from "../../tools/path-utils"; import { enforcePlanModeWrite, resolvePlanPath } from "../../tools/plan-mode-guard"; +import { formatCodeFrameLine } from "../../tools/render-utils"; import { generateDiffString } from "../diff"; import { computeLineHash, formatLineHash, HASHLINE_BIGRAM_RE_SRC } from "../line-hash"; import { detectLineEnding, normalizeToLF, restoreLineEndings, stripBom } from "../normalize"; @@ -214,10 +215,10 @@ function resolveHashlineEditsForDiff(edits: HashlineEditInput[]): HashlineEdit[] export function formatFullAnchorRequirement(raw?: string): string { const suffix = typeof raw === "string" ? raw.trim() : ""; const hashOnlyHint = /^[A-Za-z]{2}$/.test(suffix) - ? ` It looks like you supplied only the 2-letter suffix (${JSON.stringify(suffix)}). Copy the full anchor exactly as shown (for example, \"160${suffix}\").` + ? ` It looks like you supplied only the 2-letter suffix (${JSON.stringify(suffix)}). Copy the full anchor exactly as shown (for example, "160${suffix}").` : ""; const received = raw === undefined ? "" : ` Received ${JSON.stringify(raw)}.`; - return `the full anchor exactly as shown by read/grep (line number + 2-letter suffix, for example \"160sr\")${received}${hashOnlyHint}`; + return `the full anchor exactly as shown by read/grep (line number + 2-letter suffix, for example "160sr")${received}${hashOnlyHint}`; } function tryParseTag(raw: string): Anchor | undefined { @@ -241,8 +242,12 @@ function requireParsedRange(range: { pos: string; end: string }): { pos: Anchor; const invalid = [ !pos ? `pos=${JSON.stringify(range.pos)}` : null, !end ? `end=${JSON.stringify(range.end)}` : null, - ].filter(Boolean).join(", "); - throw new Error(`range requires valid pos and end anchors. Use ${formatFullAnchorRequirement()}. Invalid: ${invalid}.`); + ] + .filter(Boolean) + .join(", "); + throw new Error( + `range requires valid pos and end anchors. Use ${formatFullAnchorRequirement()}. Invalid: ${invalid}.`, + ); } return { pos, end }; } @@ -562,13 +567,14 @@ export class HashlineMismatchError extends Error { "", ]; + const lineNumberWidth = sorted.reduce((width, lineNum) => Math.max(width, String(lineNum).length), 0); let prevLine = -1; for (const lineNum of sorted) { if (prevLine !== -1 && lineNum > prevLine + 1) out.push("..."); prevLine = lineNum; const text = fileLines[lineNum - 1]; - const marker = mismatchSet.has(lineNum) ? "*" : ""; - out.push(`${marker}${lineNum}│${text}`); + const marker = mismatchSet.has(lineNum) ? "*" : " "; + out.push(formatCodeFrameLine(marker, lineNum, text ?? "", lineNumberWidth)); } return out.join("\n"); } diff --git a/packages/coding-agent/src/edit/renderer.ts b/packages/coding-agent/src/edit/renderer.ts index 07684b941..d64bc9b2e 100644 --- a/packages/coding-agent/src/edit/renderer.ts +++ b/packages/coding-agent/src/edit/renderer.ts @@ -380,18 +380,18 @@ function wrapEditRendererLine(line: string, width: number): string[] { const startAnsi = line.match(/^((?:\x1b\[[0-9;]*m)*)/)?.[1] ?? ""; const bodyWithReset = line.slice(startAnsi.length); const body = bodyWithReset.endsWith("\x1b[39m") ? bodyWithReset.slice(0, -"\x1b[39m".length) : bodyWithReset; - const diffMatch = /^([+\-\s])(\s*\d+)\|(.*)$/s.exec(body); + const diffMatch = /^([+\-\s])(\s*\d+)([|│])(.*)$/s.exec(body); if (!diffMatch) { return wrapTextWithAnsi(line, width); } - const [, marker, lineNum, content] = diffMatch; - const prefix = `${marker}${lineNum}|`; + const [, marker, lineNum, separator, content] = diffMatch; + const prefix = `${marker}${lineNum}${separator}`; const prefixWidth = visibleWidth(prefix); const contentWidth = Math.max(1, width - prefixWidth); - const continuationPrefix = `${" ".repeat(Math.max(0, prefixWidth - 1))}|`; - const wrappedContent = wrapTextWithAnsi(content, contentWidth); + const continuationPrefix = `${" ".repeat(Math.max(0, prefixWidth - 1))}${separator}`; + const wrappedContent = wrapTextWithAnsi(content ?? "", contentWidth); return wrappedContent.map( (segment, index) => `${startAnsi}${index === 0 ? prefix : continuationPrefix}${segment}\x1b[39m`, diff --git a/packages/coding-agent/src/modes/components/diff.ts b/packages/coding-agent/src/modes/components/diff.ts index 709aaba6c..bdab7ddf8 100644 --- a/packages/coding-agent/src/modes/components/diff.ts +++ b/packages/coding-agent/src/modes/components/diff.ts @@ -1,7 +1,7 @@ import { getIndentation } from "@oh-my-pi/pi-utils"; import * as Diff from "diff"; import { theme } from "../../modes/theme/theme"; -import { replaceTabs } from "../../tools/render-utils"; +import { type CodeFrameMarker, formatCodeFrameLine, replaceTabs } from "../../tools/render-utils"; /** SGR dim on / normal intensity — additive, preserves fg/bg colors. */ const DIM = "\x1b[2m"; @@ -37,14 +37,14 @@ function visualizeIndent(text: string, filePath?: string): string { * Parse diff line to extract prefix, line number, and content. * Supported formats: "+123|content" (canonical) and "+123 content" (legacy). */ -function parseDiffLine(line: string): { prefix: string; lineNum: string; content: string } | null { +function parseDiffLine(line: string): { prefix: CodeFrameMarker; lineNum: string; content: string } | null { const canonical = line.match(/^([+-\s])(\s*\d+)\|(.*)$/); if (canonical) { - return { prefix: canonical[1], lineNum: canonical[2], content: canonical[3] }; + return { prefix: canonical[1] as CodeFrameMarker, lineNum: canonical[2] ?? "", content: canonical[3] ?? "" }; } const legacy = line.match(/^([+-\s])(?:(\s*\d+)\s)?(.*)$/); if (!legacy) return null; - return { prefix: legacy[1], lineNum: legacy[2] ?? "", content: legacy[3] }; + return { prefix: legacy[1] as CodeFrameMarker, lineNum: legacy[2] ?? "", content: legacy[3] ?? "" }; } /** @@ -108,6 +108,11 @@ export interface RenderDiffOptions { export function renderDiff(diffText: string, options: RenderDiffOptions = {}): string { const lines = diffText.split("\n"); const result: string[] = []; + const parsedLines = lines.map(parseDiffLine); + const lineNumberWidth = parsedLines.reduce((width, parsed) => { + const lineNumber = parsed?.lineNum.trim() ?? ""; + return Math.max(width, lineNumber.length); + }, 0); // Track the line number rendered on the previous emitted line so we can // blank out duplicate gutters. Two cases trigger this: @@ -115,16 +120,15 @@ export function renderDiff(diffText: string, options: RenderDiffOptions = {}): s // 2. Insertion followed by context (`+N` then ` N` if producer used oldLine). let prevLineNum = ""; - const formatLine = (prefix: string, lineNum: string, content: string): string => { + const formatLine = (prefix: CodeFrameMarker, lineNum: string, content: string): string => { if (lineNum.trim().length === 0) { prevLineNum = ""; return `${prefix}${content}`; } const trimmed = lineNum.trim(); - const displayNum = trimmed === prevLineNum ? " ".repeat(lineNum.length) : lineNum; + const displayNum = trimmed === prevLineNum ? "" : trimmed; prevLineNum = trimmed; - // Use box-drawing `│` instead of `|` so adjacent lines visually connect into a continuous gutter. - return `${prefix}${displayNum}│${content}`; + return formatCodeFrameLine(prefix, displayNum, content, lineNumberWidth); }; let i = 0; diff --git a/packages/coding-agent/src/tools/ast-edit.ts b/packages/coding-agent/src/tools/ast-edit.ts index 0ccd09e09..cbadf0193 100644 --- a/packages/coding-agent/src/tools/ast-edit.ts +++ b/packages/coding-agent/src/tools/ast-edit.ts @@ -24,6 +24,7 @@ import { } from "./path-utils"; import { dedupeParseErrors, + formatCodeFrameLine, formatCount, formatEmptyMessage, formatErrorMessage, @@ -221,6 +222,10 @@ export class AstEditTool implements AgentTool { const fileChanges = changesByFile.get(relativePath) ?? []; + const lineNumberWidth = fileChanges.reduce( + (width, change) => Math.max(width, String(change.startLine).length), + 0, + ); for (const change of fileChanges) { const beforeFirstLine = change.before.split("\n", 1)[0] ?? ""; const afterFirstLine = change.after.split("\n", 1)[0] ?? ""; @@ -235,8 +240,8 @@ export class AstEditTool implements AgentTool { const fileMatches = matchesByFile.get(relativePath) ?? []; + const lineNumberWidth = fileMatches.reduce((width, match) => { + const lineCount = match.text.split("\n").length; + const endLine = match.startLine + lineCount - 1; + return Math.max(width, String(match.startLine).length, String(endLine).length); + }, 0); for (const match of fileMatches) { const matchLines = match.text.split("\n"); for (let index = 0; index < matchLines.length; index++) { const lineNumber = match.startLine + index; const isMatch = index === 0; - const line = matchLines[index]; + const line = matchLines[index] ?? ""; outputLines.push(formatMatchLine(lineNumber, line, isMatch, { useHashLines })); - const marker = isMatch ? "*" : ""; - displayLines.push(`${marker}${lineNumber}│${line}`); + displayLines.push(formatCodeFrameLine(isMatch ? "*" : " ", lineNumber, line, lineNumberWidth)); } if (match.metaVariables && Object.keys(match.metaVariables).length > 0) { const serializedMeta = Object.entries(match.metaVariables) diff --git a/packages/coding-agent/src/tools/browser.ts b/packages/coding-agent/src/tools/browser.ts index c12c2af0c..babdb06f6 100644 --- a/packages/coding-agent/src/tools/browser.ts +++ b/packages/coding-agent/src/tools/browser.ts @@ -406,7 +406,9 @@ const browserSchema = Type.Object({ include_all: Type.Optional(Type.Boolean({ description: "include non-interactive nodes" })), viewport_only: Type.Optional(Type.Boolean({ description: "limit to viewport" })), args: Type.Optional(puppeteerGetArgsSchema), - script: Type.Optional(Type.String({ description: "javascript expression", examples: ["document.title", "window.location.href"] })), + script: Type.Optional( + Type.String({ description: "javascript expression", examples: ["document.title", "window.location.href"] }), + ), text: Type.Optional(Type.String({ description: "text to type", examples: ["hello world"] })), value: Type.Optional(Type.String({ description: "value to set", examples: ["hello"] })), attribute: Type.Optional(Type.String({ description: "attribute to read", examples: ["href", "data-id"] })), diff --git a/packages/coding-agent/src/tools/debug.ts b/packages/coding-agent/src/tools/debug.ts index d943f107c..06382a2a0 100644 --- a/packages/coding-agent/src/tools/debug.ts +++ b/packages/coding-agent/src/tools/debug.ts @@ -87,7 +87,9 @@ const debugSchema = Type.Object({ ), program: Type.Optional(Type.String({ description: "program path", examples: ["./my_app", "src/main.py"] })), args: Type.Optional(Type.Array(Type.String(), { description: "program arguments", examples: [["--verbose"]] })), - adapter: Type.Optional(Type.String({ description: "debugger adapter", examples: ["gdb", "lldb-dap", "debugpy", "dlv"] })), + adapter: Type.Optional( + Type.String({ description: "debugger adapter", examples: ["gdb", "lldb-dap", "debugpy", "dlv"] }), + ), cwd: Type.Optional(Type.String({ description: "working directory", examples: ["src/"] })), file: Type.Optional(Type.String({ description: "source file", examples: ["src/main.c"] })), line: Type.Optional(Type.Number({ description: "source line", examples: [42] })), @@ -96,7 +98,9 @@ const debugSchema = Type.Object({ condition: Type.Optional(Type.String({ description: "breakpoint condition", examples: ["i == 10", "x > 0"] })), hit_condition: Type.Optional(Type.String({ description: "hit condition" })), expression: Type.Optional(Type.String({ description: "expression to evaluate", examples: ["x + 1", "obj.field"] })), - context: Type.Optional(Type.String({ description: "evaluate context", examples: ["watch", "repl", "hover", "variables", "clipboard"] })), + context: Type.Optional( + Type.String({ description: "evaluate context", examples: ["watch", "repl", "hover", "variables", "clipboard"] }), + ), frame_id: Type.Optional(Type.Number({ description: "stack frame id" })), scope_id: Type.Optional(Type.Number({ description: "scope variables reference" })), variable_ref: Type.Optional(Type.Number({ description: "variable reference" })), @@ -104,10 +108,10 @@ const debugSchema = Type.Object({ port: Type.Optional(Type.Number({ description: "remote attach port", examples: [4711] })), host: Type.Optional(Type.String({ description: "remote attach host", examples: ["127.0.0.1"] })), levels: Type.Optional(Type.Number({ description: "max stack frames" })), - memory_reference: Type.Optional(Type.String({ description: "memory reference or address", examples: ["0x7ffd1234"] })), - instruction_reference: Type.Optional( - Type.String({ description: "instruction address or reference" }), + memory_reference: Type.Optional( + Type.String({ description: "memory reference or address", examples: ["0x7ffd1234"] }), ), + instruction_reference: Type.Optional(Type.String({ description: "instruction address or reference" })), instruction_count: Type.Optional(Type.Number({ description: "instructions to disassemble" })), instruction_offset: Type.Optional(Type.Number({ description: "instruction offset" })), count: Type.Optional(Type.Number({ description: "bytes to read" })), diff --git a/packages/coding-agent/src/tools/gh.ts b/packages/coding-agent/src/tools/gh.ts index 3dd35b0a2..e484a8ce2 100644 --- a/packages/coding-agent/src/tools/gh.ts +++ b/packages/coding-agent/src/tools/gh.ts @@ -146,33 +146,24 @@ const ghRepoViewSchema = Type.Object({ }); const ghIssueViewSchema = Type.Object({ - issue: Type.String({ description: "issue number or url", examples: ["123", "https://github.com/owner/repo/issues/123"] }), - repo: Type.Optional( - Type.String({ description: "owner/repo", examples: ["facebook/react"] }), - ), + issue: Type.String({ + description: "issue number or url", + examples: ["123", "https://github.com/owner/repo/issues/123"], + }), + repo: Type.Optional(Type.String({ description: "owner/repo", examples: ["facebook/react"] })), comments: Type.Optional(Type.Boolean({ description: "include issue comments", default: true })), }); const ghPrViewSchema = Type.Object({ - pr: Type.Optional( - Type.String({ description: "pr number, url, or branch", examples: ["123", "feature-branch"] }), - ), - repo: Type.Optional( - Type.String({ description: "owner/repo", examples: ["facebook/react"] }), - ), + pr: Type.Optional(Type.String({ description: "pr number, url, or branch", examples: ["123", "feature-branch"] })), + repo: Type.Optional(Type.String({ description: "owner/repo", examples: ["facebook/react"] })), comments: Type.Optional(Type.Boolean({ description: "include pr comments", default: true })), }); const ghPrDiffSchema = Type.Object({ - pr: Type.Optional( - Type.String({ description: "pr number, url, or branch", examples: ["123", "feature-branch"] }), - ), - repo: Type.Optional( - Type.String({ description: "owner/repo", examples: ["facebook/react"] }), - ), - nameOnly: Type.Optional( - Type.Boolean({ description: "return file names only" }), - ), + pr: Type.Optional(Type.String({ description: "pr number, url, or branch", examples: ["123", "feature-branch"] })), + repo: Type.Optional(Type.String({ description: "owner/repo", examples: ["facebook/react"] })), + nameOnly: Type.Optional(Type.Boolean({ description: "return file names only" })), exclude: Type.Optional( Type.Array(Type.String({ description: "glob to exclude" }), { description: "file globs to exclude", @@ -181,16 +172,10 @@ const ghPrDiffSchema = Type.Object({ }); const ghPrCheckoutSchema = Type.Object({ - pr: Type.Optional( - Type.String({ description: "pr number, url, or branch", examples: ["123", "feature-branch"] }), - ), - repo: Type.Optional( - Type.String({ description: "owner/repo", examples: ["facebook/react"] }), - ), + pr: Type.Optional(Type.String({ description: "pr number, url, or branch", examples: ["123", "feature-branch"] })), + repo: Type.Optional(Type.String({ description: "owner/repo", examples: ["facebook/react"] })), branch: Type.Optional(Type.String({ description: "local branch name", examples: ["main", "develop"] })), - worktree: Type.Optional( - Type.String({ description: "worktree path" }), - ), + worktree: Type.Optional(Type.String({ description: "worktree path" })), force: Type.Optional( Type.Boolean({ description: "reset existing local branch", @@ -221,18 +206,14 @@ const ghSearchPrsSchema = Type.Object({ }); const ghRunWatchSchema = Type.Object({ - run: Type.Optional( - Type.String({ description: "actions run id or url", examples: ["123456"] }), - ), + run: Type.Optional(Type.String({ description: "actions run id or url", examples: ["123456"] })), branch: Type.Optional( Type.String({ description: "branch to inspect", examples: ["main", "develop"], }), ), - tail: Type.Optional( - Type.Number({ description: "log lines per failed job", default: 15 }), - ), + tail: Type.Optional(Type.Number({ description: "log lines per failed job", default: 15 })), }); type GhRepoViewInput = Static; diff --git a/packages/coding-agent/src/tools/grep.ts b/packages/coding-agent/src/tools/grep.ts index aa8eb331d..7debcaa84 100644 --- a/packages/coding-agent/src/tools/grep.ts +++ b/packages/coding-agent/src/tools/grep.ts @@ -26,7 +26,13 @@ import { resolveMultiSearchPath, resolveToCwd, } from "./path-utils"; -import { formatCount, formatEmptyMessage, formatErrorMessage, PREVIEW_LIMITS } from "./render-utils"; +import { + formatCodeFrameLine, + formatCount, + formatEmptyMessage, + formatErrorMessage, + PREVIEW_LIMITS, +} from "./render-utils"; import { ToolError } from "./tool-errors"; import { toolResult } from "./tool-result"; @@ -430,17 +436,24 @@ export class GrepTool implements AgentTool { return resultBuilder.done(); } const displayLines: string[] = []; - const renderMatchesForFile = ( - relativePath: string, - ): { model: string[]; display: string[] } => { + const renderMatchesForFile = (relativePath: string): { model: string[]; display: string[] } => { const modelOut: string[] = []; const displayOut: string[] = []; const fileMatches = matchesByFile.get(relativePath) ?? []; + const lineNumberWidth = fileMatches.reduce((width, match) => { + let nextWidth = Math.max(width, String(match.lineNumber).length); + for (const ctx of match.contextBefore ?? []) { + nextWidth = Math.max(nextWidth, String(ctx.lineNumber).length); + } + for (const ctx of match.contextAfter ?? []) { + nextWidth = Math.max(nextWidth, String(ctx.lineNumber).length); + } + return nextWidth; + }, 0); for (const match of fileMatches) { const pushLine = (lineNumber: number, line: string, isMatch: boolean) => { modelOut.push(formatMatchLine(lineNumber, line, isMatch, { useHashLines })); - const marker = isMatch ? "*" : ""; - displayOut.push(`${marker}${lineNumber}│${line}`); + displayOut.push(formatCodeFrameLine(isMatch ? "*" : " ", lineNumber, line, lineNumberWidth)); }; if (match.contextBefore) { for (const ctx of match.contextBefore) { diff --git a/packages/coding-agent/src/tools/match-line-format.ts b/packages/coding-agent/src/tools/match-line-format.ts index 0e96d154c..d21d07204 100644 --- a/packages/coding-agent/src/tools/match-line-format.ts +++ b/packages/coding-agent/src/tools/match-line-format.ts @@ -18,4 +18,4 @@ export function formatMatchLine( return `${lineNumber}${computeLineHash(lineNumber, line)}${separator}${line}`; } return `${lineNumber}${separator}${line}`; -} \ No newline at end of file +} diff --git a/packages/coding-agent/src/tools/notebook.ts b/packages/coding-agent/src/tools/notebook.ts index ad0a21851..00eb31e67 100644 --- a/packages/coding-agent/src/tools/notebook.ts +++ b/packages/coding-agent/src/tools/notebook.ts @@ -64,8 +64,7 @@ type NotebookParams = Static; export class NotebookTool implements AgentTool { readonly name = "notebook"; readonly label = "Notebook"; - readonly description = - "Edit, insert, or delete cells in Jupyter notebooks (.ipynb). cell_index is 0-based."; + readonly description = "Edit, insert, or delete cells in Jupyter notebooks (.ipynb). cell_index is 0-based."; readonly parameters = notebookSchema; readonly strict = true; readonly concurrency = "exclusive"; diff --git a/packages/coding-agent/src/tools/render-utils.ts b/packages/coding-agent/src/tools/render-utils.ts index 0d64d0b23..d064569c5 100644 --- a/packages/coding-agent/src/tools/render-utils.ts +++ b/packages/coding-agent/src/tools/render-utils.ts @@ -182,6 +182,24 @@ export function formatEmptyMessage(message: string, theme: Theme): string { return `${theme.styledSymbol("status.warning", "warning")} ${theme.fg("muted", message)}`; } +// ============================================================================= +// Code Frame Formatting +// ============================================================================= + +export type CodeFrameMarker = "" | " " | "*" | "+" | "-" | ">"; + +export function formatCodeFrameLine( + marker: CodeFrameMarker, + lineNumber: string | number, + content: string, + lineNumberWidth: number, +): string { + const markerText = marker.trim(); + const lineNumberText = String(lineNumber).trim(); + const gutterText = markerText && lineNumberText ? `${markerText}${lineNumberText}` : lineNumberText || markerText; + return `${gutterText.padStart(lineNumberWidth + 1, " ")}│${content}`; +} + // ============================================================================= // Tool UI Helpers // ============================================================================= diff --git a/packages/coding-agent/src/tools/search-tool-bm25.ts b/packages/coding-agent/src/tools/search-tool-bm25.ts index c4c33e53c..108ea73eb 100644 --- a/packages/coding-agent/src/tools/search-tool-bm25.ts +++ b/packages/coding-agent/src/tools/search-tool-bm25.ts @@ -26,9 +26,7 @@ const MATCH_DESCRIPTION_LEN = 96; const searchToolBm25Schema = Type.Object({ query: Type.String({ description: "mcp search query", examples: ["kubernetes pod", "image processing"] }), - limit: Type.Optional( - Type.Integer({ description: "max matches", minimum: 1 }), - ), + limit: Type.Optional(Type.Integer({ description: "max matches", minimum: 1 })), }); type SearchToolBm25Params = Static; diff --git a/packages/coding-agent/src/tools/todo-write.ts b/packages/coding-agent/src/tools/todo-write.ts index 9cafcc30b..822c3762c 100644 --- a/packages/coding-agent/src/tools/todo-write.ts +++ b/packages/coding-agent/src/tools/todo-write.ts @@ -49,9 +49,7 @@ const InputTask = Type.Object({ description: "task status", }), ), - details: Type.Optional( - Type.String({ description: "implementation details" }), - ), + details: Type.Optional(Type.String({ description: "implementation details" })), }); const InputPhase = Type.Object({ diff --git a/packages/coding-agent/src/tools/vim.ts b/packages/coding-agent/src/tools/vim.ts index 6d4cb6c71..01ca213ee 100644 --- a/packages/coding-agent/src/tools/vim.ts +++ b/packages/coding-agent/src/tools/vim.ts @@ -127,15 +127,15 @@ function renderViewportCursor(line: VimViewportLine, styledText: string, uiTheme } function renderViewportLine(line: VimViewportLine, styledText: string, padWidth: number, uiTheme: Theme): string { - const lineNoStr = String(line.line).padStart(padWidth, " "); - const lineNoStyled = line.isCursor - ? uiTheme.fg("accent", lineNoStr) + const marker = line.isCursor ? ">" : line.isSelected ? "*" : ""; + const gutterText = `${marker}${line.line}`.padStart(padWidth + 1, " "); + const gutterStyled = line.isCursor + ? uiTheme.fg("accent", gutterText) : line.isSelected - ? uiTheme.fg("warning", lineNoStr) - : uiTheme.fg("dim", lineNoStr); + ? uiTheme.fg("warning", gutterText) + : uiTheme.fg("dim", gutterText); const separator = uiTheme.fg("dim", "│"); - const prefix = line.isCursor ? uiTheme.fg("accent", ">") : line.isSelected ? uiTheme.fg("warning", "*") : " "; - return `${prefix}${lineNoStyled}${separator}${renderViewportCursor(line, styledText, uiTheme)}`; + return `${gutterStyled}${separator}${renderViewportCursor(line, styledText, uiTheme)}`; } function splitTokensBySequence(kbd: string[]): Array<{ sequence: string; tokens: VimKeyToken[] }> { diff --git a/packages/coding-agent/src/vim/render.ts b/packages/coding-agent/src/vim/render.ts index af1f57f10..6851eaf42 100644 --- a/packages/coding-agent/src/vim/render.ts +++ b/packages/coding-agent/src/vim/render.ts @@ -1,5 +1,5 @@ import { extractSegments } from "@oh-my-pi/pi-tui"; -import { truncateToWidth } from "../tools/render-utils"; +import { formatCodeFrameLine, truncateToWidth } from "../tools/render-utils"; import type { VimErrorLocation, VimFocusLine, @@ -207,7 +207,7 @@ export function renderVimDetails(details: VimToolDetails): string { } if (details.focus) { - const focusPrefix = `>${String(details.focus.line).padStart(String(details.viewport.end).length, " ")}│`; + const focusPrefix = formatCodeFrameLine(">", details.focus.line, "", String(details.viewport.end).length); const caretPrefix = `${" ".repeat(focusPrefix.length)} `; const caretPadding = " ".repeat(Math.max(0, details.focus.caretCol)); lines.push("Focus:"); @@ -219,8 +219,8 @@ export function renderVimDetails(details: VimToolDetails): string { const padWidth = String(details.viewport.end).length; lines.push("Viewport:"); for (const line of details.viewportLines) { - const prefix = line.isCursor ? ">" : line.isSelected ? "*" : " "; - lines.push(`${prefix}${String(line.line).padStart(padWidth, " ")}│${renderPlainViewportCursor(line)}`); + const marker = line.isCursor ? ">" : line.isSelected ? "*" : " "; + lines.push(formatCodeFrameLine(marker, line.line, renderPlainViewportCursor(line), padWidth)); } } diff --git a/packages/coding-agent/test/core/atom.test.ts b/packages/coding-agent/test/core/atom.test.ts index b571e3a60..3543401e7 100644 --- a/packages/coding-agent/test/core/atom.test.ts +++ b/packages/coding-agent/test/core/atom.test.ts @@ -4,6 +4,7 @@ import { applyAtomEdits, 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"; @@ -81,7 +82,6 @@ describe("applyAtomEdits — pre/post", () => { }); }); - describe("applyAtomEdits — sub", () => { it("replaces a unique substring", () => { const content = "const timeout = 5000;"; @@ -126,28 +126,55 @@ describe("applyAtomEdits — sub", () => { }); }); -describe("applyAtomEdits — file-scoped via pre:\"\" / post:\"\"", () => { - it("post:\"\" appends at EOF", () => { +describe("resolveAtomToolEdit — loc syntax", () => { + it('loc:"$" appends at EOF', () => { const content = "aaa\nbbb"; - const resolved = resolveAtomToolEdit({ post: "", lines: ["ccc"] }) as AtomEdit; - expect(resolved.op).toBe("append_file"); - const result = applyAtomEdits(content, [resolved]); + const resolved = resolveAtomToolEdit({ loc: "$", post: ["ccc"] }); + expect(resolved).toHaveLength(1); + expect(resolved[0]?.op).toBe("append_file"); + const result = applyAtomEdits(content, resolved); expect(result.lines).toBe("aaa\nbbb\nccc"); }); - it("pre:\"\" prepends at BOF", () => { + it('loc:"^" prepends at BOF', () => { const content = "aaa\nbbb"; - const resolved = resolveAtomToolEdit({ pre: "", lines: ["ZZZ"] }) as AtomEdit; - expect(resolved.op).toBe("prepend_file"); - const result = applyAtomEdits(content, [resolved]); + const resolved = resolveAtomToolEdit({ loc: "^", pre: ["ZZZ"] }); + expect(resolved).toHaveLength(1); + expect(resolved[0]?.op).toBe("prepend_file"); + const result = applyAtomEdits(content, resolved); expect(result.lines).toBe("ZZZ\naaa\nbbb"); }); - it("post:\"\" on empty file replaces empty line", () => { - const content = ""; - const resolved = resolveAtomToolEdit({ post: "", lines: ["aaa"] }) as AtomEdit; - const result = applyAtomEdits(content, [resolved]); - expect(result.lines).toBe("aaa"); + it("expands pre + set + post from one entry", () => { + const content = "aaa\nbbb\nccc"; + const loc = `2${computeLineHash(2, "bbb")}`; + const resolved = resolveAtomToolEdit({ loc, pre: ["B"], set: ["BBB"], post: ["A"] }); + const result = applyAtomEdits(content, resolved); + expect(result.lines).toBe("aaa\nB\nBBB\nA\nccc"); + }); + + it("set: [] deletes the anchor line", () => { + const content = "aaa\nbbb\nccc"; + const loc = `2${computeLineHash(2, "bbb")}`; + const resolved = resolveAtomToolEdit({ loc, set: [] }); + expect(resolved[0]?.op).toBe("del"); + const result = applyAtomEdits(content, resolved); + expect(result.lines).toBe("aaa\nccc"); + }); + + it('set:[""] preserves a blank line', () => { + const content = "aaa\nbbb\nccc"; + const loc = `2${computeLineHash(2, "bbb")}`; + const resolved = resolveAtomToolEdit({ loc, set: [""] }); + expect(resolved[0]?.op).toBe("set"); + const result = applyAtomEdits(content, resolved); + expect(result.lines).toBe("aaa\n\nccc"); + }); + + it("supports path override inside loc", () => { + const resolved = resolveAtomEntryPaths([{ loc: "a.ts:1ab", set: "X" }], undefined); + expect(resolved[0]?.path).toBe("a.ts"); + expect(resolved[0]?.loc).toBe("1ab"); }); }); @@ -163,11 +190,11 @@ describe("parseAnchor (atom tolerant) + applyAtomEdits", () => { it("surfaces correct anchor + content when the model invents an out-of-alphabet hash", () => { const content = "alpha\nbravo\ncharlie"; // `XG` is not in the alphabet; should be rejected with the actual anchor exposed. - const toolEdit = { path: "a.ts", set: "2XG", lines: "BRAVO" }; - const resolved = resolveAtomToolEdit(toolEdit) as AtomEdit; - expect(() => applyAtomEdits(content, [resolved])).toThrow(HashlineMismatchError); + const toolEdit = { path: "a.ts", loc: "2XG", set: "BRAVO" }; + const resolved = resolveAtomToolEdit(toolEdit); + expect(() => applyAtomEdits(content, resolved)).toThrow(HashlineMismatchError); try { - applyAtomEdits(content, [resolved]); + applyAtomEdits(content, resolved); } catch (err) { const msg = (err as Error).message; expect(msg).toMatch(/^\d+[a-z]{2}:/m); @@ -178,25 +205,24 @@ describe("parseAnchor (atom tolerant) + applyAtomEdits", () => { it("surfaces correct anchor + content when the model omits the hash entirely", () => { const content = "alpha\nbravo\ncharlie"; - const toolEdit = { path: "a.ts", set: "2", lines: "BRAVO" }; - const resolved = resolveAtomToolEdit(toolEdit) as AtomEdit; - expect(() => applyAtomEdits(content, [resolved])).toThrow(HashlineMismatchError); + const toolEdit = { path: "a.ts", loc: "2", set: "BRAVO" }; + const resolved = resolveAtomToolEdit(toolEdit); + expect(() => applyAtomEdits(content, resolved)).toThrow(HashlineMismatchError); }); it("surfaces correct anchor when the model uses pipe-separator (LINE|content) form", () => { const content = "alpha\nbravo\ncharlie"; - const toolEdit = { path: "a.ts", set: "2|bravo", lines: "BRAVO" }; - const resolved = resolveAtomToolEdit(toolEdit) as AtomEdit; - expect(() => applyAtomEdits(content, [resolved])).toThrow(HashlineMismatchError); + const toolEdit = { path: "a.ts", loc: "2|bravo", set: "BRAVO" }; + const resolved = resolveAtomToolEdit(toolEdit); + expect(() => applyAtomEdits(content, resolved)).toThrow(HashlineMismatchError); }); it("throws a usage-style error when no line number can be extracted", () => { - const toolEdit = { path: "a.ts", set: " if (!x) return;", lines: "x" }; + const toolEdit = { path: "a.ts", loc: " if (!x) return;", set: "x" }; expect(() => resolveAtomToolEdit(toolEdit)).toThrow(/Could not find a line number/); }); }); - describe("applyAtomEdits — between", () => { it("replaces lines strictly between two surviving anchors (function body, keep braces)", () => { const content = "function alpha() {\n\told();\n\tmore();\n}"; @@ -224,18 +250,14 @@ describe("applyAtomEdits — between", () => { it("is a pure insertion when after.line + 1 == before.line", () => { const content = "top\nbottom"; - const edits: AtomEdit[] = [ - { op: "between", after: tag(1, "top"), before: tag(2, "bottom"), lines: ["middle"] }, - ]; + const edits: AtomEdit[] = [{ op: "between", after: tag(1, "top"), before: tag(2, "bottom"), lines: ["middle"] }]; const result = applyAtomEdits(content, edits); expect(result.lines).toBe("top\nmiddle\nbottom"); }); it("rejects when after.line >= before.line", () => { const content = "a\nb\nc"; - const edits: AtomEdit[] = [ - { op: "between", after: tag(2, "b"), before: tag(2, "b"), lines: ["x"] }, - ]; + const edits: AtomEdit[] = [{ op: "between", after: tag(2, "b"), before: tag(2, "b"), lines: ["x"] }]; expect(() => applyAtomEdits(content, edits)).toThrow(/after\.line < before\.line/); }); @@ -285,22 +307,23 @@ describe("applyAtomEdits — between", () => { expect(() => applyAtomEdits(content, edits)).toThrow(HashlineMismatchError); }); - it("resolveAtomToolEdit accepts `set: [open, close]` tuple and parses both anchors", () => { + it("resolveAtomToolEdit accepts range loc and parses both anchors", () => { const toolEdit = { path: "a.ts", - set: ["1xx", "4yy"] as [string, string], - lines: ["X"], + loc: "1xx-4yy", + set: ["X"], }; - const resolved = resolveAtomToolEdit(toolEdit) as AtomEdit; - expect(resolved.op).toBe("between"); - if (resolved.op !== "between") throw new Error("unreachable"); - expect(resolved.after.line).toBe(1); - expect(resolved.before.line).toBe(4); - expect(resolved.lines).toEqual(["X"]); + const resolved = resolveAtomToolEdit(toolEdit); + expect(resolved).toHaveLength(1); + const between = resolved[0]; + expect(between?.op).toBe("between"); + if (!between || between.op !== "between") throw new Error("unreachable"); + expect(between.after.line).toBe(1); + expect(between.before.line).toBe(4); + expect(between.lines).toEqual(["X"]); }); - it("resolveAtomToolEdit rejects `set` arrays with non-string elements", () => { - const toolEdit = { path: "a.ts", set: [1, 2] as unknown as [string, string], lines: ["X"] }; - expect(() => resolveAtomToolEdit(toolEdit)).toThrow(/2-tuple requires both elements to be anchor strings/); + it("resolveAtomToolEdit rejects range loc without set", () => { + expect(() => resolveAtomToolEdit({ path: "a.ts", loc: "1xx-4yy", pre: ["X"] })).toThrow(/range loc requires set/); }); -}); \ No newline at end of file +}); diff --git a/packages/coding-agent/test/tools/apply-patch-renderer.test.ts b/packages/coding-agent/test/tools/apply-patch-renderer.test.ts index d9170882d..bf2eb82a6 100644 --- a/packages/coding-agent/test/tools/apply-patch-renderer.test.ts +++ b/packages/coding-agent/test/tools/apply-patch-renderer.test.ts @@ -127,4 +127,42 @@ describe("apply_patch rendering", () => { await fs.rm(tmpDir, { recursive: true, force: true }); } }); + + it("aligns rendered edit diff separators", async () => { + await getUiTheme(); + const uiStub = { requestRender() {} } as unknown as TUI; + const component = new ToolExecutionComponent( + "edit", + { path: "packages/coding-agent/src/tools/image-gen.ts" }, + {}, + undefined, + uiStub, + ); + + component.updateResult( + { + content: [{ type: "text", text: "" }], + details: { + path: "packages/coding-agent/src/tools/image-gen.ts", + op: "update", + diff: [ + " 10|}", + '+11|import { CODEX_INSTRUCTIONS } from "@oh-my-pi/pi-ai/providers/openai-codex-responses";', + " 12|\t$env,", + " 228|\toutput_format: typeof OPENAI_IMAGE_OUTPUT_FORMAT;", + "+235|\tinstructions?: string;", + ' 234|\tinput: Array<{ role: "user"; content: OpenAIInputContent[] }>;', + ].join("\n"), + }, + }, + false, + ); + + const rendered = Bun.stripANSI(component.render(220).join("\n")); + expect(rendered).toContain(" 10│}"); + expect(rendered).toContain(" +11│import"); + expect(rendered).toContain(" 228│"); + expect(rendered).toContain("+235│"); + expect(rendered).toContain(" 234│"); + }); }); diff --git a/packages/coding-agent/test/tools/render-utils.test.ts b/packages/coding-agent/test/tools/render-utils.test.ts index 4e97363c0..f287745b9 100644 --- a/packages/coding-agent/test/tools/render-utils.test.ts +++ b/packages/coding-agent/test/tools/render-utils.test.ts @@ -5,6 +5,7 @@ import { getThemeByName } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; import { dedupeParseErrors, formatDiagnostics, + formatCodeFrameLine, formatParseErrors, formatScreenshot, } from "@oh-my-pi/pi-coding-agent/tools/render-utils"; @@ -174,3 +175,12 @@ describe("formatDiagnostics", () => { expect(formatted.replace(/\s+/g, " ")).toContain("1 error(s)"); }); }); + +describe("formatCodeFrameLine", () => { + it("pads markers as part of the gutter", () => { + expect(formatCodeFrameLine(" ", 447, "context", 3)).toBe(" 447│context"); + expect(formatCodeFrameLine("*", 448, "match", 3)).toBe("*448│match"); + expect(formatCodeFrameLine("+", 11, "added", 3)).toBe(" +11│added"); + expect(formatCodeFrameLine("+", 235, "added", 3)).toBe("+235│added"); + }); +}); diff --git a/packages/typescript-edit-benchmark/src/runner.ts b/packages/typescript-edit-benchmark/src/runner.ts index 86cb184d2..41aabb4fc 100644 --- a/packages/typescript-edit-benchmark/src/runner.ts +++ b/packages/typescript-edit-benchmark/src/runner.ts @@ -1024,17 +1024,23 @@ async function runSingleTask( const statsBefore = await client.getSessionStats(); let events: Array<{ type: string; [key: string]: unknown }>; try { - events = await collectPromptEvents(client, delivery, config, logEvent, buildEarlyStop({ + events = await collectPromptEvents( + client, + delivery, config, - cwd, - expectedDir, - files: task.files, logEvent, - attempt: attempt + 1, - onMatched: () => { - earlyStoppedByMatch = true; - }, - })); + buildEarlyStop({ + config, + cwd, + expectedDir, + files: task.files, + logEvent, + attempt: attempt + 1, + onMatched: () => { + earlyStoppedByMatch = true; + }, + }), + ); } catch (err) { if (err instanceof PromptTurnLimitError) { error = err.message;