diff --git a/.changeset/tool-rendering-listing-notices.md b/.changeset/tool-rendering-listing-notices.md new file mode 100644 index 0000000..3f8c63a --- /dev/null +++ b/.changeset/tool-rendering-listing-notices.md @@ -0,0 +1,5 @@ +--- +"@eleith-pi/tool-rendering": patch +--- + +Separate native ls/find limit and byte-truncation notices from filenames when rendering. Keep actionable notices visible as warnings, exclude them from entry/file and hidden counts, and mark byte-truncated results in the summary. Execution, stored results, and legacy display overrides remain unchanged. diff --git a/extensions/tool-rendering/src/file-renderers.test.ts b/extensions/tool-rendering/src/file-renderers.test.ts index 6542429..d221d5c 100644 --- a/extensions/tool-rendering/src/file-renderers.test.ts +++ b/extensions/tool-rendering/src/file-renderers.test.ts @@ -1,5 +1,15 @@ import { rmSync } from "node:fs"; -import { truncateHead, type Theme, type ToolRenderers } from "@earendil-works/pi-coding-agent"; +import { + createLsTool, + createFindTool, + DEFAULT_MAX_BYTES, + truncateHead, + type AgentToolResult, + type LsToolDetails, + type FindToolDetails, + type Theme, + type ToolRenderers, +} from "@earendil-works/pi-coding-agent"; import { visibleWidth, type Component } from "@earendil-works/pi-tui"; import { afterAll, expect, it, vi } from "vitest"; import { buildReadRendering } from "./read.ts"; @@ -544,3 +554,176 @@ it("keeps empty searches and directories distinct from errors", () => { ), ).toContain("0 entries"); }); + +const nativeListingCases = (["ls", "find"] as const).flatMap((name) => [ + { name, limit: 2, limitReached: true, truncated: false, mode: "many" }, + { name, limit: 1000, limitReached: false, truncated: true, mode: "many" }, + { name, limit: 300, limitReached: true, truncated: true, mode: "many" }, + { name, limit: 1000, limitReached: false, truncated: true, mode: "oversized" }, + { name, limit: 1, limitReached: true, truncated: false, mode: "sentinel" }, + { name, limit: 1, limitReached: true, truncated: false, mode: "unterminated" }, +]); +it.each(nativeListingCases)( + "separates native $name notices (limit=$limit, mode=$mode) in both views", + async ({ name, limit, limitReached, truncated, mode }) => { + const sentinel = name === "ls" ? "(empty directory)" : "No files found matching pattern"; + let entries: string[]; + if (mode === "sentinel") entries = [sentinel, "z"]; + else if (mode === "unterminated") entries = ["a\x1b]unfinished", "z"]; + else if (mode === "oversized") entries = ["x".repeat(DEFAULT_MAX_BYTES + 1)]; + else { + const suffix = "x".repeat(200); + entries = Array.from({ length: 400 }, (_, index) => `entry-${index}-${suffix}`); + } + const native = + name === "ls" + ? createLsTool(directory, { + operations: { + exists: () => true, + stat: (path) => ({ isDirectory: () => path === directory }), + readdir: () => [...entries], + }, + }) + : createFindTool(directory, { + operations: { + exists: () => true, + glob: (_pattern, _cwd, options) => entries.slice(0, options.limit), + }, + }); + const args = { path: directory, pattern: "*", limit }; + const result: AgentToolResult = + await native.execute("native-listing", args); + const original = structuredClone(result); + const nativeContent = result.content[0]; + if (nativeContent.type !== "text") throw new Error("Expected native listing text"); + const noticeStart = nativeContent.text.lastIndexOf("\n\n["); + expect(noticeStart).toBeGreaterThanOrEqual(0); + const expectedNotice = nativeContent.text.slice(noticeStart + 2); + const limitKey = name === "ls" ? "entryLimitReached" : "resultLimitReached"; + const hasLimit = result.details !== undefined && limitKey in result.details; + expect(hasLimit).toBe(limitReached); + const isTruncated = result.details?.truncation?.truncated ?? false; + expect(isTruncated).toBe(truncated); + const count = result.details?.truncation?.outputLines ?? limit; + const unit = name === "ls" ? "entries" : "files"; + const limitLabel = limitReached ? " · limit reached" : ""; + const truncatedLabel = truncated ? " · truncated" : ""; + const summary = `${count} ${unit}${limitLabel}${truncatedLabel}`; + const captured = capture(name); + const tool = erased(captured.tool); + const fg = vi.fn(minimalTheme.fg); + const coloredTheme = { ...minimalTheme, fg } as unknown as Theme; + for (const expanded of [false, true]) { + fg.mockClear(); + const renderOptions = { ...options, expanded }; + const ctx = context(args, { expanded }); + const component = tool.renderResult!(result, renderOptions, coloredTheme, ctx); + const output = text(component); + expect(output).toContain(summary); + expect(output).not.toMatch(/(?:├──|└──) \[/); + const warnings = fg.mock.calls.filter(([color]) => color === "warning"); + expect(warnings).toEqual([["warning", expectedNotice]]); + const outputLines = output.split("\n"); + const noticeIndex = outputLines.indexOf(expectedNotice); + expect(noticeIndex).toBe(outputLines.length - 2); + const footer = outputLines.at(-1); + expect(footer).toContain(summary); + if (mode === "sentinel") expect(output).toContain(`└── ${sentinel}`); + if (mode === "unterminated") { + expect(output).toContain("└── a"); + expect(output).not.toContain("\x1b"); + } + const previewSize = name === "ls" ? 10 : 3; + const hidden = expanded ? 0 : Math.max(0, count - previewSize); + if (hidden > 0) expect(output).toContain(`collapsed · ${hidden} hidden`); + else expect(output).not.toContain("hidden"); + if (!expanded) { + const renderedLines = component.render(180); + const maxLines = name === "ls" ? 13 : 11; + expect(renderedLines.length).toBeLessThanOrEqual(maxLines); + } + expect(result).toEqual(original); + } + }, +); + +it("keeps the fd find suggestion outside directory groups", () => { + const { tool } = capture("find"); + const notice = "[2 results limit reached. Use limit=4 for more, or refine pattern]"; + const result = { + content: [{ type: "text" as const, text: `src/a.ts\nsrc/b.ts\n\n${notice}` }], + details: { resultLimitReached: 2 }, + }; + const ctx = context(presets.find.args); + const component = tool.renderResult!(result, options, theme, ctx); + const output = text(component); + expect(output).toContain(`src/\n ├── a.ts\n └── b.ts\n\n${notice}\n`); + expect(output).toContain("2 files · limit reached · collapsed"); + expect(output).not.toContain("hidden"); +}); + +it.each(["ls", "find"] as const)( + "preserves unrecognized %s bracketed filenames even with limit metadata", + (name) => { + const captured = capture(name); + const tool = erased(captured.tool); + const result = { + content: [{ type: "text" as const, text: "first\nsecond\n\n[not a native notice]" }], + details: { entryLimitReached: 2, resultLimitReached: 2 }, + }; + const ctx = context(presets[name].args); + const component = tool.renderResult!(result, options, theme, ctx); + const output = text(component); + const unit = name === "ls" ? "entries" : "files"; + expect(output).toContain("└── [not a native notice]"); + expect(output).toContain(`3 ${unit} · limit reached · collapsed`); + }, +); + +it.each(["ls", "find"] as const)( + "does not strip native-looking %s filenames when truncation metadata is false", + (name) => { + const captured = capture(name); + const tool = erased(captured.tool); + const rawText = "first\n\n[50.0KB limit reached]"; + const truncation = truncateHead(rawText); + const result = { + content: [{ type: "text" as const, text: rawText }], + details: { truncation }, + }; + const ctx = context(presets[name].args); + const component = tool.renderResult!(result, options, theme, ctx); + const output = text(component); + const unit = name === "ls" ? "entries" : "files"; + expect(output).toContain("└── [50.0KB limit reached]"); + expect(output).toContain(`2 ${unit} · collapsed`); + expect(output).not.toContain("truncated"); + }, +); + +it.each(["ls", "find"] as const)( + "keeps legacy %s display overrides authoritative when byte metadata is present", + (name) => { + const captured = capture(name); + const tool = erased(captured.tool); + const truncation = truncateHead("model-first\nmodel-second", { maxBytes: 12 }); + const result = { + content: [{ type: "text" as const, text: "model-facing text" }], + details: { + truncation, + rendering: { text: "\x1b[31msrc/old.ts\x1b[0m\nsrc/next.ts" }, + }, + }; + const original = structuredClone(result); + const ctx = context(presets[name].args); + const component = tool.renderResult!(result, options, theme, ctx); + const output = text(component); + const unit = name === "ls" ? "entries" : "files"; + expect(output).toContain("old.ts"); + expect(output).toContain("next.ts"); + expect(output).toContain(`2 ${unit} · truncated · collapsed`); + expect(output).not.toContain("model-"); + expect(output).not.toContain("\x1b"); + expect(result).toEqual(original); + }, +); diff --git a/extensions/tool-rendering/src/find.ts b/extensions/tool-rendering/src/find.ts index 4918166..cfc57c2 100644 --- a/extensions/tool-rendering/src/find.ts +++ b/extensions/tool-rendering/src/find.ts @@ -13,8 +13,14 @@ import { frameTop, resultLabel, } from "./frame.ts"; -import { normalizeLineEndings, sanitizeDisplayText, textFromResult } from "./tool-result.ts"; +import { + normalizeLineEndings, + rawTextFromResult, + sanitizeDisplayText, + textFromResult, +} from "./tool-result.ts"; import { previewLines } from "./preview.ts"; +import { splitListingNotice } from "./listing-output.ts"; type BuiltinFindTool = ReturnType; type FindRenderCall = NonNullable; @@ -62,24 +68,35 @@ function renderFindResult( const status = frameStatus(context); if (context.isError) return frameToolError(textFromResult(result), expanded, theme, width); - const rawText = result.details?.rendering?.text ?? textFromResult(result); - const sanitizedText = sanitizeDisplayText(rawText); - const text = normalizeLineEndings(sanitizedText); - const paths = - text.trim() === "No files found matching pattern" - ? [] - : text.split("\n").filter((line) => line.trim().length > 0); + const rawText = result.details?.rendering?.text ?? rawTextFromResult(result); + const normalizedText = normalizeLineEndings(rawText); + const resultLimit = result.details?.resultLimitReached; + const truncation = result.details?.truncation; + const { text: listingText, notice: listingNotice } = splitListingNotice( + normalizedText, + "find", + resultLimit, + truncation, + ); + const text = sanitizeDisplayText(listingText); + const notice = sanitizeDisplayText(listingNotice); + const empty = + text.trim() === "No files found matching pattern" && + resultLimit === undefined && + !truncation?.truncated; + const paths = empty ? [] : text.split("\n").filter((line) => line.trim().length > 0); // Three paths in separate directories use at most eight grouped lines. const { shown, hidden } = previewLines(paths, expanded, 3); - const body = renderGroupedPaths(shown, theme); - const limit = result.details?.resultLimitReached ? " · limit reached" : ""; - return frameResultWithBottomLabel( - body, - resultLabel(`${paths.length} files${limit}`, expanded, hidden, theme), - status, - theme, - width, - ); + let body = renderGroupedPaths(shown, theme); + if (notice) { + const warning = theme.fg("warning", notice); + body = body ? `${body}\n\n${warning}` : warning; + } + const limit = resultLimit === undefined ? "" : " · limit reached"; + const truncated = truncation?.truncated ? " · truncated" : ""; + const summary = `${paths.length} files${limit}${truncated}`; + const label = resultLabel(summary, expanded, hidden, theme); + return frameResultWithBottomLabel(body, label, status, theme, width); } function renderGroupedPaths(paths: string[], theme: FindTheme): string { diff --git a/extensions/tool-rendering/src/listing-output.ts b/extensions/tool-rendering/src/listing-output.ts new file mode 100644 index 0000000..f7199e6 --- /dev/null +++ b/extensions/tool-rendering/src/listing-output.ts @@ -0,0 +1,48 @@ +import { + DEFAULT_MAX_BYTES, + formatSize, + type TruncationResult, +} from "@earendil-works/pi-coding-agent"; + +/** + * Separate exact metadata-backed native suffixes, leaving other bracketed text alone. + * Call before sanitation so malformed filename controls cannot swallow Pi's notice. + */ +export function splitListingNotice( + text: string, + tool: "ls" | "find", + limitReached: number | undefined, + truncation: TruncationResult | undefined, +): { text: string; notice: string } { + let limitNotices = [""]; + if (limitReached !== undefined) { + const unit = tool === "ls" ? "entries" : "results"; + const limitNotice = `${limitReached} ${unit} limit reached`; + const nextLimit = limitReached * 2; + let suggestion = `Use limit=${nextLimit} for more`; + if (tool === "find") suggestion += ", or refine pattern"; + limitNotices = [`${limitNotice}. ${suggestion}`]; + // Custom FindOperations omit the follow-up suggestion that fd includes. + if (tool === "find") limitNotices.push(limitNotice); + } + + let byteNotice = ""; + if (truncation?.truncated) { + const maxBytes = truncation.maxBytes ?? DEFAULT_MAX_BYTES; + const size = formatSize(maxBytes); + byteNotice = `${size} limit reached`; + } + + for (const limitNotice of limitNotices) { + const notices = [limitNotice, byteNotice].filter(Boolean); + if (notices.length === 0) continue; + const message = notices.join(". "); + const notice = `[${message}]`; + const suffix = `\n\n${notice}`; + if (text.endsWith(suffix)) { + const body = text.slice(0, -suffix.length); + return { text: body, notice }; + } + } + return { text, notice: "" }; +} diff --git a/extensions/tool-rendering/src/ls.ts b/extensions/tool-rendering/src/ls.ts index d0f6578..7c370b1 100644 --- a/extensions/tool-rendering/src/ls.ts +++ b/extensions/tool-rendering/src/ls.ts @@ -12,8 +12,14 @@ import { frameTop, resultLabel, } from "./frame.ts"; -import { normalizeLineEndings, sanitizeDisplayText, textFromResult } from "./tool-result.ts"; +import { + normalizeLineEndings, + rawTextFromResult, + sanitizeDisplayText, + textFromResult, +} from "./tool-result.ts"; import { previewLines } from "./preview.ts"; +import { splitListingNotice } from "./listing-output.ts"; type BuiltinLsTool = ReturnType; type LsRenderCall = NonNullable; @@ -60,23 +66,32 @@ function renderLsResult( const status = frameStatus(context); if (context.isError) return frameToolError(textFromResult(result), expanded, theme, width); - const rawText = result.details?.rendering?.text ?? textFromResult(result); - const sanitizedText = sanitizeDisplayText(rawText); - const text = normalizeLineEndings(sanitizedText); - const lines = - text.trim() === "(empty directory)" - ? [] - : text.split("\n").filter((line) => line.trim().length > 0); - const { shown, hidden } = previewLines(lines, expanded, 10); - const body = renderEntries(shown, theme); - const limit = result.details?.entryLimitReached ? " · limit reached" : ""; - return frameResultWithBottomLabel( - body, - resultLabel(`${lines.length} entries${limit}`, expanded, hidden, theme), - status, - theme, - width, + const rawText = result.details?.rendering?.text ?? rawTextFromResult(result); + const normalizedText = normalizeLineEndings(rawText); + const entryLimit = result.details?.entryLimitReached; + const truncation = result.details?.truncation; + const { text: listingText, notice: listingNotice } = splitListingNotice( + normalizedText, + "ls", + entryLimit, + truncation, ); + const text = sanitizeDisplayText(listingText); + const notice = sanitizeDisplayText(listingNotice); + const empty = + text.trim() === "(empty directory)" && entryLimit === undefined && !truncation?.truncated; + const lines = empty ? [] : text.split("\n").filter((line) => line.trim().length > 0); + const { shown, hidden } = previewLines(lines, expanded, 10); + let body = renderEntries(shown, theme); + if (notice) { + const warning = theme.fg("warning", notice); + body = body ? `${body}\n\n${warning}` : warning; + } + const limit = entryLimit === undefined ? "" : " · limit reached"; + const truncated = truncation?.truncated ? " · truncated" : ""; + const summary = `${lines.length} entries${limit}${truncated}`; + const label = resultLabel(summary, expanded, hidden, theme); + return frameResultWithBottomLabel(body, label, status, theme, width); } function renderEntries(lines: string[], theme: LsTheme): string { diff --git a/extensions/tool-rendering/src/tool-result.test.ts b/extensions/tool-rendering/src/tool-result.test.ts index 69da8bb..f6df174 100644 --- a/extensions/tool-rendering/src/tool-result.test.ts +++ b/extensions/tool-rendering/src/tool-result.test.ts @@ -1,6 +1,6 @@ import { execFileSync } from "node:child_process"; import { expect, it } from "vitest"; -import { sanitizeDisplayText, textFromResult } from "./tool-result.ts"; +import { rawTextFromResult, sanitizeDisplayText, textFromResult } from "./tool-result.ts"; it.each([ ["SGR styles", "\x1b[31mvisible\x1b[0m"], @@ -79,6 +79,8 @@ it("sanitizes text blocks without changing model-facing results or image data", for (const item of result.content) Object.freeze(item); Object.freeze(result.details); Object.freeze(result.structuredContent); + const raw = rawTextFromResult(result); + expect(raw).toBe("\x1b[31mone\x1b[0m\n\x1b]52;c;private\x07two\x00"); const actual = textFromResult(result); expect(actual).toBe("one\ntwo"); expect(result).toEqual(original); diff --git a/extensions/tool-rendering/src/tool-result.ts b/extensions/tool-rendering/src/tool-result.ts index 3565e23..5bf22b8 100644 --- a/extensions/tool-rendering/src/tool-result.ts +++ b/extensions/tool-rendering/src/tool-result.ts @@ -41,11 +41,16 @@ function stripOscAndApcStrings(text: string): string { return parts.join(""); } -export function textFromResult(result: AgentToolResult): string { - const text = result.content +/** Read text blocks without sanitation so structured suffixes can be separated first. */ +export function rawTextFromResult(result: AgentToolResult): string { + return result.content .filter((item): item is { type: "text"; text: string } => item.type === "text") .map((item) => item.text) .join("\n"); +} + +export function textFromResult(result: AgentToolResult): string { + const text = rawTextFromResult(result); return sanitizeDisplayText(text); }