diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index b4769b276..fe6f545ed 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -15,6 +15,8 @@ ### Fixed +- Fixed edit tool headers to hide first-change line suffixes, middle-elide long paths, and show compact change stats. + - Fixed Esc interrupts rendering a redundant `Interrupted by user` assistant transcript line while preserving the interrupt reason for tool-result placeholders and continuation logic. - LSP writethrough no longer burns the full diagnostics poll on every edit/write. `typescript-language-server` never echoes the document version in `publishDiagnostics` ([upstream #983](https://github.com/typescript-language-server/typescript-language-server/issues/983)), so the exact-version gate never passed; `waitForDiagnostics` now accepts an exact version match instantly and otherwise settles on the latest publish after a short quiescence window, dropping superseded in-flight diagnostics. diff --git a/packages/coding-agent/src/edit/renderer.ts b/packages/coding-agent/src/edit/renderer.ts index 6bedb5947..edaed012e 100644 --- a/packages/coding-agent/src/edit/renderer.ts +++ b/packages/coding-agent/src/edit/renderer.ts @@ -4,7 +4,7 @@ import { HL_FILE_PREFIX, HL_FILE_SUFFIX } from "@oh-my-pi/hashline"; import type { Component } from "@oh-my-pi/pi-tui"; -import { visibleWidth, wrapTextWithAnsi } from "@oh-my-pi/pi-tui"; +import { sliceWithWidth, visibleWidth, wrapTextWithAnsi } from "@oh-my-pi/pi-tui"; import { sanitizeText } from "@oh-my-pi/pi-utils"; import type { RenderResultOptions } from "../extensibility/custom-tools/types"; import type { FileDiagnosticsResult } from "../lsp"; @@ -13,7 +13,6 @@ import { getLanguageFromPath, type Theme } from "../modes/theme/theme"; import type { OutputMeta } from "../tools/output-meta"; import { formatDiagnostics, - formatDiffStats, formatExpandHint, formatStatusIcon, getDiffStats, @@ -182,44 +181,120 @@ function getOperationTitle(op: Operation | undefined): string { return op === "create" ? "Create" : op === "delete" ? "Delete" : "Edit"; } +interface EditPathDisplayOptions { + rename?: string; + firstChangedLine?: number; + linkPath?: string; + renameLinkPath?: string; + maxPathWidth?: number; +} + +function truncateEditTitlePath(displayPath: string, maxWidth: number | undefined): string { + if (maxWidth === undefined) return displayPath; + const width = visibleWidth(displayPath); + const safeMaxWidth = Math.max(0, Math.floor(maxWidth)); + if (width <= safeMaxWidth) return displayPath; + + const contentWidth = safeMaxWidth - 1; + if (contentWidth <= 0) return "…"; + + const headWidth = Math.floor(contentWidth / 2); + const tailWidth = contentWidth - headWidth; + const head = sliceWithWidth(displayPath, 0, headWidth, true).text; + const tail = sliceWithWidth(displayPath, Math.max(0, width - tailWidth), tailWidth, true).text; + return `${head}…${tail}`; +} + +function formatEditTitlePath(pathValue: string, maxWidth?: number): string { + return truncateEditTitlePath(replaceTabs(shortenPath(pathValue), pathValue), maxWidth); +} + function formatEditPathDisplay( rawPath: string, uiTheme: Theme, - options?: { rename?: string; firstChangedLine?: number; linkPath?: string; renameLinkPath?: string }, -): string { + options?: EditPathDisplayOptions, +): { text: string; pathWidth: number } { // `rawPath`/`rename` are shown (cwd-relative) but the OSC 8 link targets the - // absolute path when known — a relative `rawPath` would yield a `file:///rel` - // URI that resolves against filesystem root instead of cwd. + // absolute path when known — a relative `rawPath` would otherwise yield a + // `file:///rel` URI that resolves against filesystem root instead of cwd. const linkTarget = options?.linkPath || rawPath; + const lineLink = options?.firstChangedLine ? { line: options.firstChangedLine } : undefined; + const primaryDisplay = rawPath ? formatEditTitlePath(rawPath, options?.maxPathWidth) : "…"; let pathDisplay = rawPath - ? fileHyperlink(linkTarget, uiTheme.fg("accent", shortenPath(rawPath))) - : uiTheme.fg("toolOutput", "…"); - - if (options?.firstChangedLine) { - pathDisplay += uiTheme.fg("warning", `:${options.firstChangedLine}`); - } + ? fileHyperlink(linkTarget, uiTheme.fg("accent", primaryDisplay), lineLink) + : uiTheme.fg("toolOutput", primaryDisplay); + let pathWidth = visibleWidth(primaryDisplay); if (options?.rename) { const renameTarget = options.renameLinkPath || options.rename; - pathDisplay += ` ${uiTheme.fg("dim", "→")} ${fileHyperlink(renameTarget, uiTheme.fg("accent", shortenPath(options.rename)))}`; + const renameDisplay = formatEditTitlePath(options.rename, options.maxPathWidth); + pathDisplay += ` ${uiTheme.fg("dim", "→")} ${fileHyperlink(renameTarget, uiTheme.fg("accent", renameDisplay))}`; + pathWidth += visibleWidth(renameDisplay); } - return pathDisplay; + return { text: pathDisplay, pathWidth }; } function formatEditDescription( rawPath: string, uiTheme: Theme, - options?: { rename?: string; firstChangedLine?: number; linkPath?: string; renameLinkPath?: string }, -): { language: string; description: string } { + options?: EditPathDisplayOptions, +): { language: string; description: string; pathWidth: number } { const language = getLanguageFromPath(rawPath) ?? "text"; const icon = uiTheme.fg("muted", uiTheme.getLangIcon(language)); + const pathDisplay = formatEditPathDisplay(rawPath, uiTheme, options); return { language, - description: `${icon} ${formatEditPathDisplay(rawPath, uiTheme, options)}`, + description: `${icon} ${pathDisplay.text}`, + pathWidth: pathDisplay.pathWidth, }; } +function editHeaderLabelBudget(width: number, uiTheme: Theme): number { + const leftGlyphs = `${uiTheme.boxSharp.topLeft}${uiTheme.boxSharp.horizontal.repeat(3)}`; + return Math.max(0, width - visibleWidth(leftGlyphs) - visibleWidth(uiTheme.boxSharp.topRight) - 2); +} + +function renderEditHeader( + width: number, + uiTheme: Theme, + options: { + icon: "pending" | "success" | "error"; + spinnerFrame?: number; + op?: Operation; + rawPath: string; + rename?: string; + firstChangedLine?: number; + linkPath?: string; + statsSuffix?: string; + extraSuffix?: string; + }, +): string { + const title = getOperationTitle(options.op); + const descriptionOptions: EditPathDisplayOptions = { + rename: options.rename, + firstChangedLine: options.firstChangedLine, + linkPath: options.linkPath, + }; + const formatted = formatEditDescription(options.rawPath, uiTheme, descriptionOptions); + const suffix = `${options.statsSuffix ?? ""}${options.extraSuffix ?? ""}`; + const buildHeader = (description: string): string => + renderStatusLine({ icon: options.icon, spinnerFrame: options.spinnerFrame, title, description }, uiTheme) + + suffix; + + const header = buildHeader(formatted.description); + const overflow = visibleWidth(header) - editHeaderLabelBudget(width, uiTheme); + if (overflow <= 0 || formatted.pathWidth <= 1) return header; + + const pathCount = Math.max(1, (options.rawPath ? 1 : 0) + (options.rename ? 1 : 0)); + const fittedPathWidth = Math.max(1, Math.floor((formatted.pathWidth - overflow) / pathCount)); + const fitted = formatEditDescription(options.rawPath, uiTheme, { + ...descriptionOptions, + maxPathWidth: fittedPathWidth, + }); + return buildHeader(fitted.description); +} + function renderPlainTextPreview(text: string, uiTheme: Theme, filePath?: string): string { const previewLines = sanitizeText(text).split("\n"); let preview = "\n\n"; @@ -379,10 +454,13 @@ function getApplyPatchRenderSummary( } function formatDiffStatsSuffix(diff: string, uiTheme: Theme): string { - const { added, removed, hunks } = getDiffStats(diff); - const stats = formatDiffStats(added, removed, hunks, uiTheme); - if (!stats) return ""; - return ` ${uiTheme.fg("dim", uiTheme.format.bracketLeft)}${stats}${uiTheme.fg("dim", uiTheme.format.bracketRight)}`; + const { added, removed } = getDiffStats(diff); + if (added === 0 && removed === 0) return ""; + const stats = [ + added > 0 ? uiTheme.fg("toolDiffAdded", `+${added}`) : undefined, + removed > 0 ? uiTheme.fg("toolDiffRemoved", `-${removed}`) : undefined, + ].filter(value => value !== undefined); + return ` ${uiTheme.fg("dim", uiTheme.format.bracketLeft)}${stats.join(uiTheme.fg("dim", "/"))}${uiTheme.fg("dim", uiTheme.format.bracketRight)}`; } function renderDiffSection( @@ -462,17 +540,19 @@ export const editToolRenderer = { ""; const rename = editArgs.rename || firstEdit?.rename || firstEdit?.move || firstApplyPatchEntry?.rename; const op = editArgs.op || firstEdit?.op || firstApplyPatchEntry?.op; - const { description } = formatEditDescription(rawPath, uiTheme, { rename }); let fileCount = hashlineInputSummary?.entries.length ?? applyPatchSummary?.entries.length ?? 0; if (Array.isArray(editArgs.edits)) { fileCount = countEditFiles(editArgs.edits); } return framedBlock(uiTheme, width => { - let header = renderStatusLine( - { icon: "pending", spinnerFrame: options?.spinnerFrame, title: getOperationTitle(op), description }, - uiTheme, - ); - if (fileCount > 1) header += uiTheme.fg("dim", ` (+${fileCount - 1} more)`); + const header = renderEditHeader(width, uiTheme, { + icon: "pending", + spinnerFrame: options?.spinnerFrame, + op, + rawPath, + rename, + extraSuffix: fileCount > 1 ? uiTheme.fg("dim", ` (+${fileCount - 1} more)`) : undefined, + }); let body = getCallPreview(editArgs, rawPath, uiTheme, renderContext, options.expanded); if (applyPatchSummary?.error) { body += `\n${uiTheme.fg("error", truncateToWidth(replaceTabs(applyPatchSummary.error, rawPath), Math.max(1, width - 2)))}`; @@ -546,15 +626,20 @@ function renderSingleFileResult( (editDiffPreview && "firstChangedLine" in editDiffPreview ? editDiffPreview.firstChangedLine : undefined) || (details && !isError ? details.firstChangedLine : undefined); const linkPath = details && "path" in details ? details.path : undefined; - const { description } = formatEditDescription(rawPath, uiTheme, { rename, firstChangedLine, linkPath }); // Change stats ride inline on the header bar next to the path. const previewDiff = editDiffPreview && !("error" in editDiffPreview) ? editDiffPreview.diff : undefined; const headerDiff = isError ? undefined : details?.diff || previewDiff; const statsSuffix = headerDiff ? formatDiffStatsSuffix(headerDiff, uiTheme) : ""; - const header = - renderStatusLine({ icon: isError ? "error" : "success", title: getOperationTitle(op), description }, uiTheme) + - statsSuffix; + const header = renderEditHeader(width, uiTheme, { + icon: isError ? "error" : "success", + op, + rawPath, + rename, + firstChangedLine, + linkPath, + statsSuffix, + }); let body = ""; if (isError) { diff --git a/packages/coding-agent/src/tui/hyperlink.ts b/packages/coding-agent/src/tui/hyperlink.ts index 1886302df..dd9c1fafd 100644 --- a/packages/coding-agent/src/tui/hyperlink.ts +++ b/packages/coding-agent/src/tui/hyperlink.ts @@ -5,6 +5,7 @@ * sequences when the active terminal supports hyperlinks and the user setting * permits it. Falls back to plain text when disabled. */ +import * as url from "node:url"; import { TERMINAL } from "@oh-my-pi/pi-tui"; import { settings } from "../config/settings"; import { @@ -28,21 +29,12 @@ function buildLinkId(uri: string): string { return (h >>> 0).toString(16).padStart(8, "0"); } -/** Build a `file://` URI for an absolute path with optional line/col query params. */ -function buildFileUri(absPath: string, opts?: { line?: number; col?: number }): string { - // Normalize backslashes for Windows paths before constructing the URL. - const normalized = absPath.replaceAll("\\", "/"); - const prefix = normalized.startsWith("/") ? "file://" : "file:///"; - // Split on slashes, encode each component, reassemble. - const encoded = normalized - .split("/") - .map(segment => encodeURIComponent(segment)) - .join("/"); - const params: string[] = []; - if (opts?.line !== undefined) params.push(`line=${opts.line}`); - if (opts?.col !== undefined) params.push(`col=${opts.col}`); - const query = params.length > 0 ? `?${params.join("&")}` : ""; - return `${prefix}${encoded}${query}`; +/** Build a properly encoded `file://` URI with optional line/col query params. */ +function buildFileUri(filePath: string, opts?: { line?: number; col?: number }): string { + const uri = url.pathToFileURL(filePath); + if (opts?.line !== undefined) uri.searchParams.set("line", String(opts.line)); + if (opts?.col !== undefined) uri.searchParams.set("col", String(opts.col)); + return uri.href; } /** @@ -104,21 +96,19 @@ export function urlHyperlink(url: string, displayText: string): string { } /** - * Wrap `displayText` in an OSC 8 hyperlink pointing at the given absolute file path. + * Wrap `displayText` in an OSC 8 hyperlink pointing at a filesystem path. * * Returns `displayText` unchanged when hyperlinks are disabled or when * the text already contains an OSC 8 sequence (prevents double-wrapping). + * Relative paths resolve against the current working directory before URI + * encoding so the OSC 8 target is always a valid `file://` URL. * - * The caller is responsible for passing an absolute path. Relative paths - * produce invalid `file://` URIs and are accepted silently to avoid runtime - * errors in renderer hot paths. - * - * @param absPath - Absolute filesystem path + * @param filePath - Filesystem path * @param displayText - Text to render as the hyperlink anchor (may contain ANSI codes) * @param opts - Optional line/col position appended as `?line=N&col=M` query params */ -export function fileHyperlink(absPath: string, displayText: string, opts?: { line?: number; col?: number }): string { - return wrapHyperlink(buildFileUri(absPath, opts), displayText); +export function fileHyperlink(filePath: string, displayText: string, opts?: { line?: number; col?: number }): string { + return wrapHyperlink(buildFileUri(filePath, opts), displayText); } /** diff --git a/packages/coding-agent/test/tools/edit-renderer.test.ts b/packages/coding-agent/test/tools/edit-renderer.test.ts index 01e8abd97..13084b5c3 100644 --- a/packages/coding-agent/test/tools/edit-renderer.test.ts +++ b/packages/coding-agent/test/tools/edit-renderer.test.ts @@ -199,6 +199,33 @@ describe("editToolRenderer", () => { expect(rendered).not.toContain(" …"); }); + it("omits changed-line suffixes from completed edit headers and middle-elides long paths", async () => { + const uiTheme = await getUiTheme(); + const component = editToolRenderer.renderResult( + { + content: [{ type: "text", text: "Updated transcript-container.test.ts" }], + details: { + diff: "+1│const value = 2;", + firstChangedLine: 251, + op: "update", + path: "/tmp/project/packages/coding-agent/test/modes/components/transcript-container.test.ts", + }, + }, + { expanded: false, isPartial: false, renderContext: { editMode: "hashline" } }, + uiTheme, + { file_path: "packages/coding-agent/test/modes/components/transcript-container.test.ts" }, + ); + + const wideHeader = Bun.stripANSI(component.render(160)[0]); + expect(wideHeader).toContain("packages/coding-agent/test/modes/components/transcript-container.test.ts"); + expect(wideHeader).not.toContain(":251"); + + const narrowHeader = Bun.stripANSI(component.render(72)[0]); + expect(narrowHeader).toContain("…"); + expect(narrowHeader).toContain("container.test.ts"); + expect(narrowHeader).not.toContain(":251"); + }); + it("computes the hashline preview diff once a single-line edit finishes streaming", async () => { await getUiTheme(); const uiStub = { requestRender() {} } as unknown as TUI; @@ -323,11 +350,11 @@ describe("editToolRenderer", () => { expect(lines[0]).toContain("demo.go"); expect(lines[0]).toContain("+2"); expect(lines[0]).toContain("-1"); - expect(lines[0]).toContain("1 hunk"); + expect(lines[0]).toContain("+2/-1"); // …only there (no standalone stats row), and the diff starts immediately // below the header (no blank line, no lone lang-icon metadata row). expect(lines[1]).toContain("115│ ctx"); - expect(lines.filter(line => line.includes("hunk"))).toHaveLength(1); + expect(lines.filter(line => line.includes("+2/-1"))).toHaveLength(1); }); it("renders completed edit gutters without inherited frame padding", async () => { diff --git a/packages/coding-agent/test/tui/hyperlink.test.ts b/packages/coding-agent/test/tui/hyperlink.test.ts index 2d78ab219..733930404 100644 --- a/packages/coding-agent/test/tui/hyperlink.test.ts +++ b/packages/coding-agent/test/tui/hyperlink.test.ts @@ -131,6 +131,21 @@ describe("fileHyperlink", () => { expect(uri).not.toContain(" "); }); + it("percent-encodes URL-reserved path bytes before appending query params", () => { + setHyperlinkMode("always"); + const result = fileHyperlink("/Users/foo/a#b?c% d.ts", "a#b?c% d.ts", { line: 12 }); + const uri = extractLinkUri(result); + expect(uri).toBe("file:///Users/foo/a%23b%3Fc%25%20d.ts?line=12"); + }); + + it("resolves relative paths before building file URIs", () => { + setHyperlinkMode("always"); + const result = fileHyperlink("relative file#1.ts", "relative file#1.ts"); + const uri = extractLinkUri(result); + expect(uri).toBeDefined(); + expect(decodeURIComponent(new URL(uri!).pathname)).toEndWith("/relative file#1.ts"); + }); + it("appends line and col as query params when provided", () => { setHyperlinkMode("always"); const result = fileHyperlink("/Users/foo/bar.ts", "bar.ts", { line: 42, col: 7 });