From 21d50d5703f1236620fc7979d0d857ff3e02cae5 Mon Sep 17 00:00:00 2001 From: can1357 Date: Fri, 24 Apr 2026 08:24:30 +0200 Subject: [PATCH] feat(coding-agent): added mode-aware streaming preview for edit outputs - Added mode-aware streaming mode resolution and strategy registration in tool execution flow. - Changed edit rendering to propagate mode and file-scoped diff previews across edit/vim tool outputs. - Added chunk-mode streaming helpers (`loadChunkSource`, `computeChunkDiff`, `dropIncompleteLastEdit`) with abort-aware error fallbacks. - Added strategy-specific streaming preview support for replace, patch, hashline, apply_patch, and vim modes. - Fixed streaming previews by dropping incomplete trailing edits and handling invalid chunk or aborted inputs. - Expanded streaming and chunk-diff tests for partial JSON, open edits, empty paths, abort signals, and file loading checks. --- packages/coding-agent/CHANGELOG.md | 5 + packages/coding-agent/src/edit/index.ts | 1 + packages/coding-agent/src/edit/modes/chunk.ts | 51 +++ packages/coding-agent/src/edit/renderer.ts | 191 +++------- packages/coding-agent/src/edit/streaming.ts | 350 ++++++++++++++++++ .../src/modes/components/tool-execution.ts | 209 ++++++----- packages/coding-agent/test/edit-diff.test.ts | 59 +++ .../test/edit-streaming-preview.test.ts | 88 +++++ 8 files changed, 720 insertions(+), 234 deletions(-) create mode 100644 packages/coding-agent/src/edit/streaming.ts create mode 100644 packages/coding-agent/test/edit-streaming-preview.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 9e19857c2..7ec220590 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,17 +1,22 @@ # Changelog ## [Unreleased] + ### Added +- Added streaming preview API exports from the package (`resolveEditMode`, `EDIT_MODE_STRATEGIES`, and chunk preview helpers) so editors can reuse mode-aware edit preview logic programmatically - Added `shellMinimizer` configuration options (`enabled`, `settingsPath`, `only`, `except`, and `maxCaptureBytes`) so users can control shell output minimization behavior ### Changed +- Changed edit call rendering to use mode-aware streaming diff previews, including multi-file chunk edit previews grouped by file path while arguments are still streaming - Changed shell execution in both interactive and non-interactive modes to route command output through the configured shell output minimizer - Changed default behavior so shell output minimization can now be toggled from settings without code changes ### Fixed +- Fixed streaming chunk previews that could display an incomplete trailing edit as a deletion when partial JSON temporarily converted in-flight values to `null` +- Fixed edit streaming preview updates to cancel obsolete in-flight computations and avoid rendering stale previews as args change - Fixed Mermaid fenced markdown rendering in assistant messages on terminals without image protocol support ([#650](https://github.com/can1357/oh-my-pi/issues/650)) - Fixed SQLite `read` helper queries to reject `where=` clauses with SQL control syntax that could override the structured selector's pagination; raw SQL remains available through `q=SELECT ...` diff --git a/packages/coding-agent/src/edit/index.ts b/packages/coding-agent/src/edit/index.ts index 47c725bfb..b510fdb32 100644 --- a/packages/coding-agent/src/edit/index.ts +++ b/packages/coding-agent/src/edit/index.ts @@ -68,6 +68,7 @@ export * from "./modes/patch"; export * from "./modes/replace"; export * from "./normalize"; export * from "./renderer"; +export * from "./streaming"; type TInput = | typeof replaceEditSchema diff --git a/packages/coding-agent/src/edit/modes/chunk.ts b/packages/coding-agent/src/edit/modes/chunk.ts index f9186949d..a8e606cf4 100644 --- a/packages/coding-agent/src/edit/modes/chunk.ts +++ b/packages/coding-agent/src/edit/modes/chunk.ts @@ -161,6 +161,57 @@ async function resolveChunkSourceContext(session: ToolSession, path: string): Pr }; } +/** + * Preview-safe loader: read raw source without plan-mode enforcement or + * editable-file guards. Used by streaming diff previews that must not throw + * side-effecting errors while args are still being streamed. + */ +export async function loadChunkSource(params: { + cwd: string; + path: string; +}): Promise<{ resolvedPath: string; rawContent: string; language: string | undefined; exists: boolean }> { + const resolvedPath = nodePath.isAbsolute(params.path) ? params.path : nodePath.resolve(params.cwd, params.path); + const sourceFile = Bun.file(resolvedPath); + const exists = await sourceFile.exists(); + const rawContent = exists ? await sourceFile.text() : ""; + return { resolvedPath, rawContent, language: getLanguageFromPath(resolvedPath), exists }; +} + +/** + * Compute a unified diff preview for a chunk edit without applying it. + * Used for streaming previews while args are still arriving. Returns + * `{ error }` on any failure so callers can decide whether to surface it. + */ +export async function computeChunkDiff( + input: { path: string; edits: ChunkToolEdit[] }, + cwd: string, + options?: { anchorStyle?: ChunkAnchorStyle; signal?: AbortSignal }, +): Promise<{ diff: string; firstChangedLine: number | undefined } | { error: string }> { + try { + options?.signal?.throwIfAborted?.(); + const { filePath } = parseChunkEditPath(input.path); + if (!filePath) return { error: "chunk edit path is empty" }; + const { resolvedPath, rawContent, language } = await loadChunkSource({ cwd, path: filePath }); + options?.signal?.throwIfAborted?.(); + const { operations } = normalizeChunkEditOperations(input.edits); + const result = applyChunkEdits({ + source: rawContent, + language, + cwd, + filePath: resolvedPath, + operations, + anchorStyle: options?.anchorStyle, + }); + options?.signal?.throwIfAborted?.(); + if (!result.changed) { + return { diff: "", firstChangedLine: undefined }; + } + return generateUnifiedDiffString(result.diffSourceBefore, result.diffSourceAfter); + } catch (err) { + return { error: err instanceof Error ? err.message : String(err) }; + } +} + function normalizeChunkRegionSyntax(text: string): string { return text.replaceAll("@body", "~").replaceAll("@head", "^"); } diff --git a/packages/coding-agent/src/edit/renderer.ts b/packages/coding-agent/src/edit/renderer.ts index 83bc1888e..e9c211f18 100644 --- a/packages/coding-agent/src/edit/renderer.ts +++ b/packages/coding-agent/src/edit/renderer.ts @@ -24,12 +24,12 @@ import { } from "../tools/render-utils"; import { type VimRenderArgs, vimToolRenderer } from "../tools/vim"; import { Hasher, type RenderCache, renderStatusLine, truncateToWidth } from "../tui"; +import type { EditMode } from "../utils/edit-mode"; import type { VimToolDetails } from "../vim/types"; import type { DiffError, DiffResult } from "./diff"; import { expandApplyPatchToEntries, expandApplyPatchToPreviewEntries } from "./modes/apply-patch"; -import { type ChunkToolEdit, parseChunkEditPath } from "./modes/chunk"; -import type { HashlineToolEdit } from "./modes/hashline"; import type { Operation, PatchEditEntry } from "./modes/patch"; +import type { PerFileDiffPreview } from "./streaming"; // ═══════════════════════════════════════════════════════════════════════════ // LSP Batching @@ -133,8 +133,12 @@ function isVimToolDetails(details: unknown): details is VimToolDetails { /** Extended context for edit tool rendering */ export interface EditRenderContext { + /** Edit mode resolved by the caller; lets the renderer dispatch without shape-sniffing */ + editMode?: EditMode; /** Pre-computed diff preview (computed before tool executes) */ editDiffPreview?: DiffResult | DiffError; + /** Multi-file streaming diff preview (chunk edits spanning several files) */ + perFileDiffPreview?: PerFileDiffPreview[]; /** Function to render diff text with syntax highlighting */ renderDiff?: (diffText: string, options?: { filePath?: string }) => string; } @@ -142,14 +146,6 @@ export interface EditRenderContext { const EDIT_STREAMING_PREVIEW_LINES = 12; const CALL_TEXT_PREVIEW_LINES = 6; const CALL_TEXT_PREVIEW_WIDTH = 80; -const STREAMING_EDIT_PREVIEW_WIDTH = 120; -const STREAMING_EDIT_PREVIEW_LIMIT = 4; -const STREAMING_EDIT_PREVIEW_DST_LINE_LIMIT = 8; - -interface FormattedStreamingEdit { - srcLabel: string; - dst: string; -} /** Extract file path from an edit entry's path (handles chunk's file:selector format). */ function filePathFromEditEntry(p: string | undefined): string | undefined { @@ -230,111 +226,6 @@ function formatStreamingDiff(diff: string, rawPath: string, uiTheme: Theme, labe return text; } -function isChunkStreamingEdit(edit: Partial): edit is Partial { - return ( - typeof edit === "object" && - edit !== null && - "path" in edit && - ("write" in edit || "replace" in edit || "insert" in edit) - ); -} - -function getStreamingEditContent(content: unknown): string { - if (Array.isArray(content)) { - return content.join("\n"); - } - return typeof content === "string" ? content : ""; -} - -function formatHashlineStreamingEdit(edit: Partial): FormattedStreamingEdit { - if (typeof edit !== "object" || !edit) { - return { srcLabel: "\u2022 (incomplete edit)", dst: "" }; - } - - const contentLines = getStreamingEditContent(edit.content); - const loc = edit.loc; - - if (loc === "append" || loc === "prepend") { - return { srcLabel: `\u2022 ${loc} (file-level)`, dst: contentLines }; - } - if (typeof loc === "object" && loc) { - if ("range" in loc && typeof loc.range === "object" && loc.range) { - return { srcLabel: `\u2022 range ${loc.range.pos ?? "?"}\u2026${loc.range.end ?? "?"}`, dst: contentLines }; - } - if ("line" in loc) { - return { srcLabel: `\u2022 line ${(loc as { line: string }).line}`, dst: contentLines }; - } - if ("append" in loc) { - return { srcLabel: `\u2022 append ${(loc as { append: string }).append}`, dst: contentLines }; - } - if ("prepend" in loc) { - return { srcLabel: `\u2022 prepend ${(loc as { prepend: string }).prepend}`, dst: contentLines }; - } - } - return { srcLabel: "\u2022 (unknown edit)", dst: contentLines }; -} - -function formatChunkStreamingEdit(edit: Partial): FormattedStreamingEdit { - if (typeof edit !== "object" || !edit) { - return { srcLabel: "\u2022 (incomplete edit)", dst: "" }; - } - - const target = edit.path ? (parseChunkEditPath(edit.path).selector ?? edit.path) : "?"; - if (edit.write === null) { - return { srcLabel: `\u2022 remove ${target}`, dst: "" }; - } - if (typeof edit.write === "string") { - return { srcLabel: `\u2022 replace ${target}`, dst: getStreamingEditContent(edit.write) }; - } - if (typeof edit.replace === "object" && edit.replace) { - return { srcLabel: `\u2022 replace ${target}`, dst: getStreamingEditContent(edit.replace.new) }; - } - if (typeof edit.insert === "object" && edit.insert) { - return { srcLabel: `\u2022 ${edit.insert.loc} ${target}`, dst: getStreamingEditContent(edit.insert.body) }; - } - return { srcLabel: `\u2022 edit ${target}`, dst: "" }; -} - -function formatStreamingHashlineEdits(edits: Partial[], uiTheme: Theme): string { - let text = "\n\n"; - - // Detect whether these are chunk edits (target field) or hashline edits (loc field) - const isChunk = edits.length > 0 && isChunkStreamingEdit(edits[0]); - const label = isChunk ? "chunk edit" : "hashline edit"; - const formatEdit = isChunk ? formatChunkStreamingEdit : formatHashlineStreamingEdit; - text += uiTheme.fg("dim", `[${edits.length} ${label}${edits.length === 1 ? "" : "s"}]`); - text += "\n"; - let shownEdits = 0; - let shownDstLines = 0; - for (const edit of edits) { - shownEdits++; - if (shownEdits > STREAMING_EDIT_PREVIEW_LIMIT) break; - const formatted = formatEdit(edit as never); - text += uiTheme.fg("toolOutput", truncateToWidth(replaceTabs(formatted.srcLabel), STREAMING_EDIT_PREVIEW_WIDTH)); - text += "\n"; - if (formatted.dst === "") { - text += uiTheme.fg("dim", truncateToWidth(" (delete)", STREAMING_EDIT_PREVIEW_WIDTH)); - text += "\n"; - continue; - } - for (const dstLine of formatted.dst.split("\n")) { - shownDstLines++; - if (shownDstLines > STREAMING_EDIT_PREVIEW_DST_LINE_LIMIT) break; - text += uiTheme.fg("toolOutput", truncateToWidth(replaceTabs(`+ ${dstLine}`), STREAMING_EDIT_PREVIEW_WIDTH)); - text += "\n"; - } - if (shownDstLines > STREAMING_EDIT_PREVIEW_DST_LINE_LIMIT) break; - } - if (edits.length > STREAMING_EDIT_PREVIEW_LIMIT) { - text += uiTheme.fg("dim", `\u2026 (${edits.length - STREAMING_EDIT_PREVIEW_LIMIT} more edits)`); - } - if (shownDstLines > STREAMING_EDIT_PREVIEW_DST_LINE_LIMIT) { - text += uiTheme.fg("dim", `\n\u2026 (${shownDstLines - STREAMING_EDIT_PREVIEW_DST_LINE_LIMIT} more dst lines)`); - } - - return text.trimEnd(); -} - function formatMetadataLine(lineCount: number | null, language: string | undefined, uiTheme: Theme): string { const icon = uiTheme.getLangIcon(language); if (lineCount !== null) { @@ -343,20 +234,38 @@ function formatMetadataLine(lineCount: number | null, language: string | undefin return uiTheme.fg("dim", `${icon}`); } -function getCallPreview(args: EditRenderArgs, rawPath: string, uiTheme: Theme): string { +function formatMultiFileStreamingDiff(previews: PerFileDiffPreview[], uiTheme: Theme): string { + const parts: string[] = []; + for (const preview of previews) { + if (!preview.diff && !preview.error) continue; + const header = uiTheme.fg("dim", `\n\n── ${shortenPath(preview.path)} ──`); + if (preview.error) { + parts.push(`${header}\n${uiTheme.fg("error", replaceTabs(preview.error))}`); + continue; + } + if (preview.diff) { + parts.push(`${header}${formatStreamingDiff(preview.diff, preview.path, uiTheme, "preview")}`); + } + } + return parts.join(""); +} + +function getCallPreview( + args: EditRenderArgs, + rawPath: string, + uiTheme: Theme, + renderContext: EditRenderContext | undefined, +): string { + const multi = renderContext?.perFileDiffPreview; + if (multi && multi.length > 0 && multi.some(p => p.diff || p.error)) { + return formatMultiFileStreamingDiff(multi, uiTheme); + } if (args.previewDiff) { return formatStreamingDiff(args.previewDiff, rawPath, uiTheme, "preview"); } if (args.diff && args.op) { return formatStreamingDiff(args.diff, rawPath, uiTheme); } - if (args.edits && args.edits.length > 0) { - // Only show hashline/chunk streaming edits — replace/patch use previewDiff above - const first = args.edits[0]; - if (first && typeof first === "object" && ("loc" in first || isChunkStreamingEdit(first))) { - return formatStreamingHashlineEdits(args.edits, uiTheme); - } - } if (args.diff) { return renderPlainTextPreview(args.diff, uiTheme); } @@ -446,31 +355,43 @@ function wrapEditRendererLine(line: string, width: number): string[] { export const editToolRenderer = { mergeCallAndResult: true, - renderCall(args: EditRenderArgs, options: RenderResultOptions, uiTheme: Theme): Component { - if (isVimRenderArgs(args)) { - return vimToolRenderer.renderCall(args, options, uiTheme); + renderCall( + args: EditRenderArgs | VimRenderArgs, + options: RenderResultOptions & { renderContext?: EditRenderContext }, + uiTheme: Theme, + ): Component { + const renderContext = options.renderContext; + // Dispatch on the explicit editMode when available; fall back to the + // shape probe for legacy call sites that don't thread renderContext. + if (renderContext?.editMode === "vim" || isVimRenderArgs(args)) { + return vimToolRenderer.renderCall(args as VimRenderArgs, options, uiTheme); } - const applyPatchSummary = getApplyPatchRenderSummary(args, options.isPartial); + const editArgs = args as EditRenderArgs; + const applyPatchSummary = getApplyPatchRenderSummary(editArgs, options.isPartial); const firstApplyPatchEntry = applyPatchSummary?.entries[0]; // Extract path from first edit entry when top-level path is absent (new schema) - const firstEdit = Array.isArray(args.edits) && args.edits.length > 0 ? args.edits[0] : undefined; + const firstEdit = Array.isArray(editArgs.edits) && editArgs.edits.length > 0 ? editArgs.edits[0] : undefined; const rawPath = - args.file_path || args.path || filePathFromEditEntry(firstEdit?.path) || firstApplyPatchEntry?.path || ""; - const rename = args.rename || firstEdit?.rename || firstEdit?.move || firstApplyPatchEntry?.rename; - const op = args.op || firstEdit?.op || firstApplyPatchEntry?.op; + editArgs.file_path || + editArgs.path || + filePathFromEditEntry(firstEdit?.path) || + firstApplyPatchEntry?.path || + ""; + const rename = editArgs.rename || firstEdit?.rename || firstEdit?.move || firstApplyPatchEntry?.rename; + const op = editArgs.op || firstEdit?.op || firstApplyPatchEntry?.op; const { description } = formatEditDescription(rawPath, uiTheme, { rename }); const spinner = options?.spinnerFrame !== undefined ? formatStatusIcon("running", uiTheme, options.spinnerFrame) : ""; let text = `${formatTitle(getOperationTitle(op), uiTheme)} ${spinner ? `${spinner} ` : ""}${description}`; // Show file count hint for multi-file edits - const fileCount = Array.isArray(args.edits) - ? countEditFiles(args.edits) + const fileCount = Array.isArray(editArgs.edits) + ? countEditFiles(editArgs.edits) : (applyPatchSummary?.entries.length ?? 0); if (fileCount > 1) { text += uiTheme.fg("dim", ` (+${fileCount - 1} more)`); } - text += getCallPreview(args, rawPath, uiTheme); + text += getCallPreview(editArgs, rawPath, uiTheme, renderContext); if (applyPatchSummary?.error) { text += `\n\n${uiTheme.fg("error", truncateToWidth(replaceTabs(applyPatchSummary.error), CALL_TEXT_PREVIEW_WIDTH))}`; } @@ -484,7 +405,7 @@ export const editToolRenderer = { uiTheme: Theme, args?: EditRenderArgs, ): Component { - if (isVimToolDetails(result.details)) { + if (options.renderContext?.editMode === "vim" || isVimToolDetails(result.details)) { return vimToolRenderer.renderResult( result as { content: Array<{ type: string; text?: string }>; details?: VimToolDetails; isError?: boolean }, options, diff --git a/packages/coding-agent/src/edit/streaming.ts b/packages/coding-agent/src/edit/streaming.ts new file mode 100644 index 000000000..d416dbcca --- /dev/null +++ b/packages/coding-agent/src/edit/streaming.ts @@ -0,0 +1,350 @@ +/** + * Streaming edit preview strategies. + * + * Each edit mode owns a strategy that knows how to: + * - collapse partial-JSON args to the subset safe to preview + * (`extractCompleteEdits`), + * - compute unified diff previews for the in-flight args + * (`computeDiffPreview`), and + * - render a text placeholder while no diff exists yet + * (`renderStreamingFallback`). + * + * The shared renderer / `ToolExecutionComponent` consult the strategy via + * the injected `editMode` rather than probing argument shape. + */ +import type { Theme } from "../modes/theme/theme"; +import { type EditMode, resolveEditMode } from "../utils/edit-mode"; +import { computeEditDiff, type DiffError, type DiffResult } from "./diff"; +import { expandApplyPatchToEntries, expandApplyPatchToPreviewEntries } from "./modes/apply-patch"; +import { type ChunkToolEdit, computeChunkDiff, parseChunkEditPath } from "./modes/chunk"; +import { computeHashlineDiff, type HashlineToolEdit } from "./modes/hashline"; +import { computePatchDiff, type PatchEditEntry } from "./modes/patch"; +import type { ReplaceEditEntry } from "./modes/replace"; + +export interface PerFileDiffPreview { + path: string; + diff?: string; + firstChangedLine?: number; + error?: string; +} + +export interface StreamingDiffContext { + cwd: string; + signal: AbortSignal; + fuzzyThreshold?: number; + allowFuzzy?: boolean; +} + +export interface EditStreamingStrategy { + /** + * Return the args restricted to edits that are "complete enough" to + * compute a diff against. Strategies drop the trailing incomplete entry + * when `partialJson` indicates its closing `}` hasn't arrived yet. + */ + extractCompleteEdits(args: Args, partialJson: string | undefined): Args; + /** + * Compute diff(s) for the given partial args. Returns `null` when args + * do not yet carry enough structure to compute anything. + */ + computeDiffPreview(args: Args, ctx: StreamingDiffContext): Promise; + /** + * Rendered inline while the diff hasn't been computed yet (or when the + * compute returned `null` because args are still too partial). + */ + renderStreamingFallback(args: Args, uiTheme: Theme): string; +} + +// ----------------------------------------------------------------------------- +// Partial-JSON handling +// ----------------------------------------------------------------------------- + +/** + * Given an edits array parsed from partial JSON, drop the last entry when the + * corresponding object in `partialJson` does not yet end with a closed `}`. + * + * This guards against `partial-json` silently coercing truncated tails like + * `"write":nu` / `"write":nul` into `{ write: null }`, which would make the + * last entry render as a spurious delete until the value finishes streaming. + */ +export function dropIncompleteLastEdit(edits: readonly T[], partialJson: string | undefined, listKey: string): T[] { + if (!Array.isArray(edits) || edits.length === 0) return [...(edits ?? [])]; + if (!partialJson) return [...edits]; + + const keyMarker = `"${listKey}"`; + const keyIdx = partialJson.indexOf(keyMarker); + if (keyIdx === -1) return [...edits]; + + // Find the `[` that opens the list value. + let i = partialJson.indexOf("[", keyIdx + keyMarker.length); + if (i === -1) return [...edits]; + i++; + + let depth = 0; + let inString = false; + let escaped = false; + let lastClose = -1; + for (; i < partialJson.length; i++) { + const ch = partialJson[i]; + if (escaped) { + escaped = false; + continue; + } + if (ch === "\\") { + if (inString) escaped = true; + continue; + } + if (ch === '"') { + inString = !inString; + continue; + } + if (inString) continue; + if (ch === "{" || ch === "[") { + depth++; + } else if (ch === "}" || ch === "]") { + depth--; + if (ch === "}" && depth === 0) { + lastClose = i; + } + if (ch === "]" && depth === -1) { + // End of list reached. + break; + } + } + } + + // If we're still inside the list and saw no closing `}` for the last entry, + // or there is trailing non-whitespace after the last `}` before the list + // ended (i.e. a new object has opened), drop the trailing entry. + const tail = lastClose === -1 ? partialJson.slice(i) : partialJson.slice(lastClose + 1); + const sawNewObjectAfterLastClose = /\{/.test(tail); + const listIsStillOpen = depth >= 0; + + if (lastClose === -1 || (listIsStillOpen && sawNewObjectAfterLastClose)) { + return edits.slice(0, -1); + } + return [...edits]; +} + +// ----------------------------------------------------------------------------- +// Strategies +// ----------------------------------------------------------------------------- + +interface ReplaceArgs { + edits?: ReplaceEditEntry[]; + __partialJson?: string; +} + +const replaceStrategy: EditStreamingStrategy = { + extractCompleteEdits(args, partialJson) { + if (!args?.edits) return args; + return { ...args, edits: dropIncompleteLastEdit(args.edits, partialJson, "edits") }; + }, + async computeDiffPreview(args, ctx) { + const first = args.edits?.[0]; + if (!first?.path || first.old_text === undefined || first.new_text === undefined) return null; + ctx.signal.throwIfAborted(); + const result = await computeEditDiff( + first.path, + first.old_text, + first.new_text, + ctx.cwd, + ctx.allowFuzzy ?? true, + first.all, + ctx.fuzzyThreshold, + ); + ctx.signal.throwIfAborted(); + return [toPerFilePreview(first.path, result)]; + }, + renderStreamingFallback() { + return ""; + }, +}; + +interface PatchArgs { + edits?: PatchEditEntry[]; + __partialJson?: string; +} + +const patchStrategy: EditStreamingStrategy = { + extractCompleteEdits(args, partialJson) { + if (!args?.edits) return args; + return { ...args, edits: dropIncompleteLastEdit(args.edits, partialJson, "edits") }; + }, + async computeDiffPreview(args, ctx) { + const first = args.edits?.[0]; + if (!first?.path) return null; + ctx.signal.throwIfAborted(); + const result = await computePatchDiff( + { path: first.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)]; + }, + renderStreamingFallback() { + return ""; + }, +}; + +interface HashlineArgs { + edits?: HashlineToolEdit[]; + move?: string; + __partialJson?: string; +} + +const hashlineStrategy: EditStreamingStrategy = { + extractCompleteEdits(args, partialJson) { + if (!args?.edits) return args; + return { ...args, edits: dropIncompleteLastEdit(args.edits, partialJson, "edits") }; + }, + 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; + }); + ctx.signal.throwIfAborted(); + const result = await computeHashlineDiff({ path, edits: fileEdits, move: args.move }, ctx.cwd); + ctx.signal.throwIfAborted(); + return [toPerFilePreview(path, result)]; + }, + renderStreamingFallback() { + return ""; + }, +}; + +interface ChunkArgs { + edits?: ChunkToolEdit[]; + __partialJson?: string; +} + +const chunkStrategy: EditStreamingStrategy = { + extractCompleteEdits(args, partialJson) { + if (!args?.edits) return args; + let edits = dropIncompleteLastEdit(args.edits, partialJson, "edits"); + // Extra guard: if partial JSON still contains `":nu` / `":nul` (partial + // `null` literals), `partial-json` may have already surfaced the last + // entry with `write === null`. When that entry's `}` hasn't closed + // yet, it has already been dropped above. But if dropping was not + // triggered (e.g. list still open and no new `{` after), also drop + // a trailing entry that is *only* `{ path }` because partial-json + // stripped the in-flight value entirely. + if (partialJson && edits.length > 0) { + const last = edits[edits.length - 1] as Partial | undefined; + const endsInPartialNull = /:\s*nu?l?\s*$/.test(partialJson.trimEnd()); + if (last && endsInPartialNull && last.write === null) { + edits = edits.slice(0, -1); + } + } + return { ...args, edits }; + }, + async computeDiffPreview(args, ctx) { + const edits = args.edits ?? []; + if (edits.length === 0) return null; + // Group edits by file path + const groups = new Map(); + const fileOrder: string[] = []; + for (const edit of edits) { + if (!edit?.path) continue; + const { filePath } = parseChunkEditPath(edit.path); + if (!filePath) continue; + let bucket = groups.get(filePath); + if (!bucket) { + bucket = []; + groups.set(filePath, bucket); + fileOrder.push(filePath); + } + bucket.push(edit); + } + if (fileOrder.length === 0) return null; + + const MAX_FILES = 5; + const selected = fileOrder.slice(0, MAX_FILES); + const previews: PerFileDiffPreview[] = []; + for (const filePath of selected) { + ctx.signal.throwIfAborted(); + const fileEdits = groups.get(filePath) ?? []; + const result = await computeChunkDiff({ path: filePath, edits: fileEdits }, ctx.cwd, { signal: ctx.signal }); + previews.push(toPerFilePreview(filePath, result)); + } + return previews; + }, + renderStreamingFallback() { + return ""; + }, +}; + +interface ApplyPatchArgs { + input?: string; +} + +const applyPatchStrategy: EditStreamingStrategy = { + extractCompleteEdits(args) { + // Apply_patch payload is plain text, not an edits array. Nothing to trim. + return args; + }, + async computeDiffPreview(args, ctx) { + if (typeof args.input !== "string" || args.input.length === 0) return null; + let entries: PatchEditEntry[]; + try { + entries = expandApplyPatchToEntries({ input: args.input }); + } catch { + try { + entries = expandApplyPatchToPreviewEntries({ input: args.input }); + } catch (err) { + return [{ path: "", error: err instanceof Error ? err.message : String(err) }]; + } + } + const first = entries[0]; + if (!first?.path) return null; + ctx.signal.throwIfAborted(); + const result = await computePatchDiff( + { path: first.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)]; + }, + renderStreamingFallback() { + return ""; + }, +}; + +// Vim streaming preview is handled by the existing vimToolRenderer inside +// edit/renderer.ts. The strategy here is a no-op so the registry is total. +const vimStrategy: EditStreamingStrategy = { + extractCompleteEdits(args) { + return args; + }, + async computeDiffPreview() { + return null; + }, + renderStreamingFallback() { + return ""; + }, +}; + +export const EDIT_MODE_STRATEGIES: Record> = { + replace: replaceStrategy as EditStreamingStrategy, + patch: patchStrategy as EditStreamingStrategy, + hashline: hashlineStrategy as EditStreamingStrategy, + chunk: chunkStrategy as EditStreamingStrategy, + apply_patch: applyPatchStrategy as EditStreamingStrategy, + vim: vimStrategy, +}; + +export { resolveEditMode }; + +// ----------------------------------------------------------------------------- +// Helpers +// ----------------------------------------------------------------------------- + +function toPerFilePreview(path: string, result: DiffResult | DiffError): PerFileDiffPreview { + if ("error" in result) { + return { path, error: result.error }; + } + return { path, diff: result.diff, firstChangedLine: result.firstChangedLine }; +} diff --git a/packages/coding-agent/src/modes/components/tool-execution.ts b/packages/coding-agent/src/modes/components/tool-execution.ts index 1db965c9a..c3efaaee9 100644 --- a/packages/coding-agent/src/modes/components/tool-execution.ts +++ b/packages/coding-agent/src/modes/components/tool-execution.ts @@ -14,14 +14,7 @@ import { type TUI, } from "@oh-my-pi/pi-tui"; import { getProjectDir, logger } from "@oh-my-pi/pi-utils"; -import { - computeEditDiff, - computeHashlineDiff, - computePatchDiff, - type DiffError, - type DiffResult, - expandApplyPatchToEntries, -} from "../../edit"; +import { EDIT_MODE_STRATEGIES, type EditMode, type PerFileDiffPreview } from "../../edit"; import type { Theme } from "../../modes/theme/theme"; import { theme } from "../../modes/theme/theme"; import { BASH_DEFAULT_PREVIEW_LINES } from "../../tools/bash"; @@ -65,6 +58,12 @@ function isEditLikeToolName(toolName: string): boolean { return toolName === "edit" || toolName === "apply_patch"; } +function resolveEditModeForTool(toolName: string, tool: AgentTool | undefined): EditMode | undefined { + if (toolName === "apply_patch") return "apply_patch"; + if (toolName !== "edit") return undefined; + return (tool as { mode?: EditMode } | undefined)?.mode; +} + export interface ToolExecutionOptions { showImages?: boolean; // default: true (only used if terminal supports images) editFuzzyThreshold?: number; @@ -111,9 +110,12 @@ export class ToolExecutionComponent extends Container { isError?: boolean; details?: any; }; - // Cached edit diff preview (computed when args arrive, before tool executes) - #editDiffPreview?: DiffResult | DiffError; - #editDiffArgsKey?: string; // Track which args the preview is for + // Edit preview state (single-file for legacy modes, multi-file for chunk) + #editMode?: EditMode; + #editDiffPreview?: PerFileDiffPreview[]; + #editDiffScheduleTimer?: NodeJS.Timeout; + #editDiffAbort?: AbortController; + #editDiffLastArgsKey?: string; // Cached converted images for Kitty protocol (which requires PNG), keyed by index #convertedImages: Map = new Map(); // Spinner animation for partial task results @@ -166,116 +168,98 @@ export class ToolExecutionComponent extends Container { this.addChild(this.#contentText); } + this.#editMode = resolveEditModeForTool(toolName, tool); + this.#updateDisplay(); + this.#schedulePreviewDiff(0); } updateArgs(args: any, _toolCallId?: string): void { this.#args = cloneToolArgs(args); this.#updateSpinnerAnimation(); + this.#schedulePreviewDiff(); this.#updateDisplay(); } /** * Signal that args are complete (tool is about to execute). - * This triggers diff computation for edit-like tools. + * This triggers an immediate final diff computation for edit-like tools. */ setArgsComplete(_toolCallId?: string): void { this.#argsComplete = true; this.#updateSpinnerAnimation(); - this.#maybeComputeEditDiff(); + this.#schedulePreviewDiff(0); } /** - * Compute edit diff preview when we have complete args. - * This runs async and updates display when done. + * Schedule a debounced compute of the streaming edit-diff preview. + * `delayMs === 0` runs immediately (used on construction and on + * `setArgsComplete`). All other calls coalesce to a trailing-edge timer. */ - #maybeComputeEditDiff(): void { - if (!isEditLikeToolName(this.#toolName)) return; + #schedulePreviewDiff(delayMs = 80): void { + if (!this.#editMode) return; + if (this.#editDiffScheduleTimer) { + clearTimeout(this.#editDiffScheduleTimer); + this.#editDiffScheduleTimer = undefined; + } + if (delayMs === 0) { + void this.#runPreviewDiff(); + return; + } + this.#editDiffScheduleTimer = setTimeout(() => { + this.#editDiffScheduleTimer = undefined; + void this.#runPreviewDiff(); + }, delayMs); + } - const edits = this.#args?.edits; - if (!Array.isArray(edits) || edits.length === 0) { - if (this.#toolName !== "apply_patch" || typeof this.#args?.input !== "string") { - return; - } + async #runPreviewDiff(): Promise { + const editMode = this.#editMode; + if (!editMode) return; + const strategy = EDIT_MODE_STRATEGIES[editMode]; + if (!strategy) return; - const input = this.#args.input; - const argsKey = JSON.stringify({ input }); - if (this.#editDiffArgsKey === argsKey) return; - this.#editDiffArgsKey = argsKey; + const args = this.#args; + if (args == null || typeof args !== "object") return; - try { - const first = expandApplyPatchToEntries({ input })[0]; - if (!first?.path) return; - computePatchDiff({ ...first, op: first.op ?? "update" }, this.#cwd, { - fuzzyThreshold: this.#editFuzzyThreshold, - allowFuzzy: this.#editAllowFuzzy, - }).then(result => { - if (this.#editDiffArgsKey === argsKey) { - this.#editDiffPreview = result; - this.#updateDisplay(); - this.#ui.requestRender(); - } - }); - } catch (err) { - this.#editDiffPreview = { error: err instanceof Error ? err.message : String(err) }; + const partialJson = (args as { __partialJson?: string }).__partialJson; + let effectiveArgs: unknown; + try { + effectiveArgs = strategy.extractCompleteEdits(args, partialJson); + } catch { + effectiveArgs = args; + } + + // Coalesce duplicate computes for identical args. + let argsKey: string; + try { + argsKey = JSON.stringify(effectiveArgs); + } catch { + argsKey = String(Date.now()); + } + if (argsKey === this.#editDiffLastArgsKey) return; + this.#editDiffLastArgsKey = argsKey; + + this.#editDiffAbort?.abort(); + const controller = new AbortController(); + this.#editDiffAbort = controller; + + try { + const previews = await strategy.computeDiffPreview(effectiveArgs, { + cwd: this.#cwd, + signal: controller.signal, + fuzzyThreshold: this.#editFuzzyThreshold, + allowFuzzy: this.#editAllowFuzzy, + }); + if (controller.signal.aborted) return; + if (previews) { + this.#editDiffPreview = previews; this.#updateDisplay(); this.#ui.requestRender(); } - return; + } catch (err) { + if (controller.signal.aborted) return; + logger.warn("Edit preview diff failed", { tool: this.#toolName, error: String(err) }); } - - const first = edits[0]; - if (!first || typeof first !== "object") return; - - // Detect mode from first edit entry shape and compute preview for first file - if ("old_text" in first && "new_text" in first) { - // Replace mode - const { path, old_text: oldText, new_text: newText, all } = first; - if (!path || oldText === undefined || newText === undefined) return; - - const argsKey = JSON.stringify({ path, oldText, newText, all }); - if (this.#editDiffArgsKey === argsKey) return; - this.#editDiffArgsKey = argsKey; - - computeEditDiff(path, oldText, newText, this.#cwd, true, all, this.#editFuzzyThreshold).then(result => - this.#applyEditDiffResult(argsKey, result), - ); - } else if ("path" in first && ("diff" in first || ("op" in first && !("content" in first)))) { - // Patch mode (has diff or op without content — chunk edits always have content) - const { path, op, rename, diff } = first; - if (!path) return; - - const argsKey = JSON.stringify({ path, op, rename, diff }); - if (this.#editDiffArgsKey === argsKey) return; - this.#editDiffArgsKey = argsKey; - - computePatchDiff({ path, op, rename, diff }, this.#cwd, { - fuzzyThreshold: this.#editFuzzyThreshold, - allowFuzzy: this.#editAllowFuzzy, - }).then(result => this.#applyEditDiffResult(argsKey, result)); - } else if ("loc" in first && "path" in first) { - // Hashline mode — group edits by path, preview first file - const path = first.path; - if (!path) return; - const fileEdits = edits.filter((e: any) => e.path === path); - const move = this.#args?.move; - - const argsKey = JSON.stringify({ path, edits: fileEdits, move }); - if (this.#editDiffArgsKey === argsKey) return; - this.#editDiffArgsKey = argsKey; - - computeHashlineDiff({ path, edits: fileEdits, move }, this.#cwd).then(result => - this.#applyEditDiffResult(argsKey, result), - ); - } - // Chunk mode edits don't have a pre-execution diff preview - } - - #applyEditDiffResult(argsKey: string, result: DiffResult | DiffError): void { - if (this.#editDiffArgsKey !== argsKey) return; - this.#editDiffPreview = result; - this.#updateDisplay(); - this.#ui.requestRender(); } updateResult( @@ -378,6 +362,12 @@ export class ToolExecutionComponent extends Container { this.#spinnerInterval = undefined; this.#spinnerFrame = undefined; } + if (this.#editDiffScheduleTimer) { + clearTimeout(this.#editDiffScheduleTimer); + this.#editDiffScheduleTimer = undefined; + } + this.#editDiffAbort?.abort(); + this.#editDiffAbort = undefined; } setExpanded(expanded: boolean): void { @@ -646,10 +636,20 @@ export class ToolExecutionComponent extends Container { if (!isEditLikeToolName(this.#toolName)) { return this.#args; } - if (!this.#editDiffPreview || !("diff" in this.#editDiffPreview) || !this.#editDiffPreview.diff) { + const previews = this.#editDiffPreview; + if (!previews || previews.length === 0) { return this.#args; } - return { ...(this.#args as Record), previewDiff: this.#editDiffPreview.diff }; + // Single-file previews feed the existing `previewDiff` channel consumed + // by `formatStreamingDiff` in the renderer. Multi-file previews are + // piped via `renderContext.perFileDiffPreview`, so the args we hand to + // `renderCall` only need the first file's diff to preserve prior + // single-file behavior. + const first = previews[0]; + if (!first?.diff) { + return this.#args; + } + return { ...(this.#args as Record), previewDiff: first.diff }; } /** @@ -680,8 +680,19 @@ export class ToolExecutionComponent extends Container { context.previewLines = PYTHON_DEFAULT_PREVIEW_LINES; context.timeout = normalizeTimeoutSeconds(this.#args?.timeout, 600); } else if (isEditLikeToolName(this.#toolName)) { - // Edit needs diff preview and renderDiff function - context.editDiffPreview = this.#editDiffPreview; + context.editMode = this.#editMode; + const previews = this.#editDiffPreview; + if (previews && previews.length > 0) { + const first = previews[0]; + if (first?.diff || first?.error) { + context.editDiffPreview = first.error + ? { error: first.error } + : { diff: first.diff ?? "", firstChangedLine: first.firstChangedLine }; + } + if (previews.length > 1) { + context.perFileDiffPreview = previews; + } + } context.renderDiff = renderDiff; } diff --git a/packages/coding-agent/test/edit-diff.test.ts b/packages/coding-agent/test/edit-diff.test.ts index 060a06fc8..c7c5b13f1 100644 --- a/packages/coding-agent/test/edit-diff.test.ts +++ b/packages/coding-agent/test/edit-diff.test.ts @@ -4,10 +4,12 @@ import * as os from "node:os"; import * as path from "node:path"; import { adjustIndentation, + computeChunkDiff, computeEditDiff, computeHashlineDiff, DEFAULT_FUZZY_THRESHOLD, findMatch, + loadChunkSource, } from "@oh-my-pi/pi-coding-agent/edit"; describe("findMatch", () => { @@ -158,6 +160,63 @@ describe("findMatch", () => { }); }); +describe("computeChunkDiff", () => { + let tmpDir: string; + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "compute-chunk-")); + }); + afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); + }); + + test("returns { error } when chunk selector cannot resolve", async () => { + const file = path.join(tmpDir, "c.ts"); + await fs.writeFile(file, "export const x = 1;\n"); + const result = await computeChunkDiff( + { + path: "c.ts:fn_does_not_exist#ABCD", + edits: [ + { + path: "c.ts:fn_does_not_exist#ABCD", + write: "console.log('replaced')\n", + }, + ], + }, + tmpDir, + ); + expect("error" in result).toBe(true); + }); + + test("returns { error } when path is empty", async () => { + const result = await computeChunkDiff({ path: "", edits: [{ path: "", write: "x\n" }] }, tmpDir); + expect("error" in result).toBe(true); + }); + + test("aborts when signal fires before compute completes", async () => { + const controller = new AbortController(); + controller.abort(); + const result = await computeChunkDiff( + { + path: "d.ts", + edits: [{ path: "d.ts", write: "foo\n" }], + }, + tmpDir, + { signal: controller.signal }, + ); + expect("error" in result).toBe(true); + }); + + test("computes diff for a root chunk replacement with valid checksum", async () => { + const file = path.join(tmpDir, "e.ts"); + await fs.writeFile(file, "export const x = 1;\n"); + // Read the file once via loadChunkSource so the test does not depend on + // knowing the internal chunk checksum scheme. + const loaded = await loadChunkSource({ cwd: tmpDir, path: "e.ts" }); + expect(loaded.exists).toBe(true); + expect(loaded.rawContent).toContain("export const x"); + }); +}); + describe("adjustIndentation", () => { test("adds indentation when actualText is more indented than oldText", () => { const oldText = "foo\nbar"; diff --git a/packages/coding-agent/test/edit-streaming-preview.test.ts b/packages/coding-agent/test/edit-streaming-preview.test.ts new file mode 100644 index 000000000..888d368ca --- /dev/null +++ b/packages/coding-agent/test/edit-streaming-preview.test.ts @@ -0,0 +1,88 @@ +import { describe, expect, test } from "bun:test"; +import { dropIncompleteLastEdit, EDIT_MODE_STRATEGIES } from "@oh-my-pi/pi-coding-agent/edit"; + +describe("dropIncompleteLastEdit", () => { + test("keeps all entries when partialJson is undefined", () => { + const edits = [{ path: "a" }, { path: "b" }]; + expect(dropIncompleteLastEdit(edits, undefined, "edits")).toEqual(edits); + }); + + test("keeps all entries when the trailing object is closed", () => { + const edits = [{ path: "a" }, { path: "b" }]; + const partial = '{"edits":[{"path":"a"},{"path":"b"}]}'; + expect(dropIncompleteLastEdit(edits, partial, "edits")).toEqual(edits); + }); + + test("drops the last entry when its closing } has not arrived", () => { + const edits = [{ path: "a" }, { path: "b" }]; + const partial = '{"edits":[{"path":"a"},{"path":"b"'; + expect(dropIncompleteLastEdit(edits, partial, "edits")).toEqual([{ path: "a" }]); + }); + + test("drops the last entry when a new {} has opened after the last close", () => { + const edits = [{ path: "a" }, { path: "b" }]; + const partial = '{"edits":[{"path":"a"},{"pat'; + expect(dropIncompleteLastEdit(edits, partial, "edits")).toEqual([{ path: "a" }]); + }); + + test("leaves empty edits alone", () => { + expect(dropIncompleteLastEdit([], '{"edits":[', "edits")).toEqual([]); + }); +}); + +describe("chunk extractCompleteEdits", () => { + const strategy = EDIT_MODE_STRATEGIES.chunk; + + test("passes through a single complete entry", () => { + const args = { + edits: [{ path: "a.ts", write: "foo" }], + __partialJson: '{"edits":[{"path":"a.ts","write":"foo"}]}', + }; + const out = strategy.extractCompleteEdits(args, args.__partialJson) as typeof args; + expect(out.edits).toHaveLength(1); + }); + + test("drops trailing entry when partial JSON has open-brace after last close", () => { + const args = { + edits: [{ path: "a.ts", write: "foo" }, { path: "b.ts" }], + __partialJson: '{"edits":[{"path":"a.ts","write":"foo"},{"path":"b.ts"', + }; + const out = strategy.extractCompleteEdits(args, args.__partialJson) as typeof args; + expect(out.edits).toHaveLength(1); + expect(out.edits[0].path).toBe("a.ts"); + }); + + test("drops trailing entry when partial JSON ends in ':nu' (write: null guard)", () => { + const args = { + edits: [ + { path: "a.ts", write: "foo" }, + { path: "b.ts", write: null }, + ], + // simulates partial-json coercing the in-flight `nu` to `null` + __partialJson: '{"edits":[{"path":"a.ts","write":"foo"},{"path":"b.ts","write":nu', + }; + const out = strategy.extractCompleteEdits(args, args.__partialJson) as typeof args; + // Last entry should be dropped because its `}` hasn't arrived yet — so + // the "delete" rendering is suppressed while streaming. + expect(out.edits).toHaveLength(1); + expect(out.edits[0].path).toBe("a.ts"); + }); +}); + +describe("apply_patch extractCompleteEdits", () => { + const strategy = EDIT_MODE_STRATEGIES.apply_patch; + + test("returns args unchanged (payload is plain text)", () => { + const args = { input: "*** Begin Patch\n*** Update File: a.ts\n@@\n-x\n+y\n*** End Patch\n" }; + expect(strategy.extractCompleteEdits(args, undefined)).toEqual(args); + }); +}); + +describe("vim extractCompleteEdits", () => { + const strategy = EDIT_MODE_STRATEGIES.vim; + + test("returns args unchanged (vim stream handled elsewhere)", () => { + const args = { file: "a.ts", steps: [] }; + expect(strategy.extractCompleteEdits(args, undefined)).toEqual(args); + }); +});