mirror of
https://github.com/earendil-works/pi.git
synced 2026-10-28 09:44:37 +08:00
fix(coding-agent): normalize read line-range labels (#10938)
- share range formatting across the TUI, exports, and examples - add regression coverage without changing execution arguments - fixes #9887
This commit is contained in:
1 parent
5712b134ed
commit
910149912c
9 files changed
+214
-47
No files matched your search
@@ -13,6 +13,7 @@
|
||||
- Fixed stopping a codemode script or reaching its deadline waiting for a non-interruptible built-in to return on Bun ([#10748](https://github.com/earendil-works/pi/issues/10748))
|
||||
- Fixed repeated codemode timeouts or aborts accumulating surviving workers; admission now limits live workers to two across sessions, counting cancelled workers until actual exit ([#10748](https://github.com/earendil-works/pi/issues/10748))
|
||||
- Fixed `--model` choosing a provider by catalog order when several providers offer the match; Pi now prefers the provider with credentials and otherwise reports the ambiguity. `--models` and `enabledModels` add a model for each provider that offers it and warn about the ambiguity ([#10160](https://github.com/earendil-works/pi/issues/10160))
|
||||
- Fixed `read` line-range labels for numeric-string or null arguments in the TUI, session tree, HTML exports, and examples ([#9887](https://github.com/earendil-works/pi/issues/9887))
|
||||
|
||||
## [1.2.0] - 2026-10-12
|
||||
|
||||
|
||||
@@ -25,6 +25,7 @@ import {
|
||||
createLsToolDefinition,
|
||||
createReadToolDefinition,
|
||||
createWriteToolDefinition,
|
||||
formatReadLineRange,
|
||||
} from "@earendil-works/pi-coding-agent";
|
||||
import { Text } from "@earendil-works/pi-tui";
|
||||
import { homedir } from "os";
|
||||
@@ -65,11 +66,9 @@ export default function (pi: ExtensionAPI) {
|
||||
const path = shortenPath(args.path || "");
|
||||
let pathDisplay = path ? theme.fg("accent", path) : theme.fg("toolOutput", "...");
|
||||
|
||||
// Show line range if specified
|
||||
if (args.offset !== undefined || args.limit !== undefined) {
|
||||
const startLine = args.offset ?? 1;
|
||||
const endLine = args.limit !== undefined ? startLine + args.limit - 1 : "";
|
||||
pathDisplay += theme.fg("warning", `:${startLine}${endLine ? `-${endLine}` : ""}`);
|
||||
const lineRange = formatReadLineRange(args.offset, args.limit);
|
||||
if (lineRange) {
|
||||
pathDisplay += theme.fg("warning", lineRange);
|
||||
}
|
||||
|
||||
return new Text(`${theme.fg("toolTitle", theme.bold("read"))} ${pathDisplay}`, 0, 0);
|
||||
|
||||
@@ -22,6 +22,7 @@ import { StringEnum } from "@earendil-works/pi-ai";
|
||||
import {
|
||||
CONFIG_DIR_NAME,
|
||||
type ExtensionAPI,
|
||||
formatReadLineRange,
|
||||
getAgentDir,
|
||||
getMarkdownTheme,
|
||||
withFileMutationQueue,
|
||||
@@ -87,13 +88,10 @@ function formatToolCall(
|
||||
case "read": {
|
||||
const rawPath = (args.file_path || args.path || "...") as string;
|
||||
const filePath = shortenPath(rawPath);
|
||||
const offset = args.offset as number | undefined;
|
||||
const limit = args.limit as number | undefined;
|
||||
const lineRange = formatReadLineRange(args.offset, args.limit);
|
||||
let text = themeFg("accent", filePath);
|
||||
if (offset !== undefined || limit !== undefined) {
|
||||
const startLine = offset ?? 1;
|
||||
const endLine = limit !== undefined ? startLine + limit - 1 : "";
|
||||
text += themeFg("warning", `:${startLine}${endLine ? `-${endLine}` : ""}`);
|
||||
if (lineRange) {
|
||||
text += themeFg("warning", lineRange);
|
||||
}
|
||||
return themeFg("muted", "read ") + text;
|
||||
}
|
||||
|
||||
@@ -575,15 +575,7 @@
|
||||
switch (name) {
|
||||
case 'read': {
|
||||
const path = shortenPath(String(args.path || args.file_path || ''));
|
||||
const offset = args.offset;
|
||||
const limit = args.limit;
|
||||
let display = path;
|
||||
if (offset !== undefined || limit !== undefined) {
|
||||
const start = offset ?? 1;
|
||||
const end = limit !== undefined ? start + limit - 1 : '';
|
||||
display += `:${start}${end ? `-${end}` : ''}`;
|
||||
}
|
||||
return `[read: ${display}]`;
|
||||
return `[read: ${path}${formatReadLineRange(args.offset, args.limit)}]`;
|
||||
}
|
||||
case 'write':
|
||||
return `[write: ${shortenPath(String(args.path || args.file_path || ''))}]`;
|
||||
@@ -825,6 +817,23 @@
|
||||
return null;
|
||||
}
|
||||
|
||||
// Mirrors ../tools/render-utils.ts.
|
||||
function parseLineNumber(value) {
|
||||
if (typeof value === 'string' && value.trim() !== '') value = Number(value);
|
||||
return typeof value === 'number' && Number.isFinite(value) ? value : undefined;
|
||||
}
|
||||
|
||||
function formatReadLineRange(offset, limit) {
|
||||
if (offset == null && limit == null) return '';
|
||||
const start = offset == null ? 1 : parseLineNumber(offset);
|
||||
if (start === undefined) return '';
|
||||
if (limit == null) return `:${start}`;
|
||||
const count = parseLineNumber(limit);
|
||||
if (count === undefined) return '';
|
||||
const end = start + count - 1;
|
||||
return `:${start}${end ? `-${end}` : ''}`;
|
||||
}
|
||||
|
||||
function getLanguageFromPath(filePath) {
|
||||
const ext = filePath.split('.').pop()?.toLowerCase();
|
||||
const extToLang = {
|
||||
@@ -965,14 +974,10 @@
|
||||
}
|
||||
case 'read': {
|
||||
const filePath = str(args.file_path ?? args.path);
|
||||
const offset = args.offset;
|
||||
const limit = args.limit;
|
||||
|
||||
const readLineRange = formatReadLineRange(args.offset, args.limit);
|
||||
let pathHtml = filePath === null ? invalidArg : escapeHtml(shortenPath(filePath || ''));
|
||||
if (filePath !== null && (offset !== undefined || limit !== undefined)) {
|
||||
const startLine = offset ?? 1;
|
||||
const endLine = limit !== undefined ? startLine + limit - 1 : '';
|
||||
pathHtml += `<span class="line-numbers">:${startLine}${endLine ? '-' + endLine : ''}</span>`;
|
||||
if (filePath !== null && readLineRange) {
|
||||
pathHtml += `<span class="line-numbers">${escapeHtml(readLineRange)}</span>`;
|
||||
}
|
||||
|
||||
html += `<div class="tool-header"><span class="tool-name">read</span> <span class="tool-path">${pathHtml}</span></div>`;
|
||||
|
||||
@@ -29,6 +29,24 @@ export function str(value: unknown): string | null {
|
||||
return null;
|
||||
}
|
||||
|
||||
function parseLineNumber(value: unknown): number | undefined {
|
||||
if (typeof value === "string" && value.trim() !== "") value = Number(value);
|
||||
return typeof value === "number" && Number.isFinite(value) ? value : undefined;
|
||||
}
|
||||
|
||||
/** Format finite numbers or numeric strings as a read line range, omitting malformed ranges. */
|
||||
export function formatReadLineRange(offset: unknown, limit: unknown): string {
|
||||
// Strict schemas represent omitted optional arguments as null.
|
||||
if (offset == null && limit == null) return "";
|
||||
const start = offset == null ? 1 : parseLineNumber(offset);
|
||||
if (start === undefined) return "";
|
||||
if (limit == null) return `:${start}`;
|
||||
const count = parseLineNumber(limit);
|
||||
if (count === undefined) return "";
|
||||
const end = start + count - 1;
|
||||
return `:${start}${end ? `-${end}` : ""}`;
|
||||
}
|
||||
|
||||
export function replaceTabs(text: string): string {
|
||||
return text.replace(/\t/g, " ");
|
||||
}
|
||||
|
||||
@@ -16,7 +16,7 @@ import { formatPathRelativeToCwdOrAbsolute } from "../../../utils/paths.ts";
|
||||
import type { ToolDefinition, ToolRenderResultOptions } from "../../extensions/types.ts";
|
||||
import { resolveToCwd } from "../path-utils.ts";
|
||||
import type { ReadToolDetails } from "../read.ts";
|
||||
import { getTextOutput, renderToolPath, replaceTabs, str } from "../render-utils.ts";
|
||||
import { formatReadLineRange, getTextOutput, renderToolPath, replaceTabs, str } from "../render-utils.ts";
|
||||
import { DEFAULT_MAX_BYTES, DEFAULT_MAX_LINES, formatSize } from "../truncate.ts";
|
||||
|
||||
interface CompactReadClassification {
|
||||
@@ -24,17 +24,14 @@ interface CompactReadClassification {
|
||||
label: string;
|
||||
}
|
||||
const COMPACT_RESOURCE_FILE_NAMES = new Set(["AGENTS.override.md", "AGENTS.md", "AGENTS.MD", "CLAUDE.md", "CLAUDE.MD"]);
|
||||
type ReadRenderArgs = { path?: string; file_path?: string; offset?: number; limit?: number };
|
||||
function formatReadLineRange(args: ReadRenderArgs | undefined, theme: Theme): string {
|
||||
// Strict tool schemas make models send null for omitted optional fields.
|
||||
if (args?.offset == null && args?.limit == null) return "";
|
||||
const startLine = args.offset ?? 1;
|
||||
const endLine = args.limit != null ? startLine + args.limit - 1 : "";
|
||||
return theme.fg("warning", `:${startLine}${endLine ? `-${endLine}` : ""}`);
|
||||
type ReadRenderArgs = { path?: string; file_path?: string; offset?: unknown; limit?: unknown };
|
||||
function styledReadLineRange(args: ReadRenderArgs | undefined, theme: Theme): string {
|
||||
const range = formatReadLineRange(args?.offset, args?.limit);
|
||||
return range ? theme.fg("warning", range) : "";
|
||||
}
|
||||
function formatReadCall(args: ReadRenderArgs | undefined, theme: Theme, cwd: string): string {
|
||||
const pathDisplay = renderToolPath(str(args?.file_path ?? args?.path), theme, cwd);
|
||||
return `${theme.fg("toolTitle", theme.bold("read"))} ${pathDisplay}${formatReadLineRange(args, theme)}`;
|
||||
return `${theme.fg("toolTitle", theme.bold("read"))} ${pathDisplay}${styledReadLineRange(args, theme)}`;
|
||||
}
|
||||
function trimTrailingEmptyLines(lines: string[]): string[] {
|
||||
let end = lines.length;
|
||||
@@ -96,7 +93,7 @@ function formatCompactReadCall(
|
||||
return (
|
||||
theme.fg("customMessageLabel", `\x1b[1m[skill]\x1b[22m `) +
|
||||
theme.fg("customMessageText", classification.label) +
|
||||
formatReadLineRange(args, theme) +
|
||||
styledReadLineRange(args, theme) +
|
||||
expandHint
|
||||
);
|
||||
}
|
||||
@@ -105,7 +102,7 @@ function formatCompactReadCall(
|
||||
theme.fg("toolTitle", theme.bold(`read ${classification.kind}`)) +
|
||||
" " +
|
||||
theme.fg("accent", classification.label) +
|
||||
formatReadLineRange(args, theme) +
|
||||
styledReadLineRange(args, theme) +
|
||||
expandHint
|
||||
);
|
||||
}
|
||||
|
||||
@@ -390,6 +390,7 @@ export {
|
||||
type WriteToolOptions,
|
||||
withFileMutationQueue,
|
||||
} from "./core/tools/index.ts";
|
||||
export { formatReadLineRange } from "./core/tools/render-utils.ts";
|
||||
export {
|
||||
hasTrustRequiringProjectResources,
|
||||
type ProjectTrustDecision,
|
||||
|
||||
@@ -13,6 +13,7 @@ import {
|
||||
wrapTextWithAnsi,
|
||||
} from "@earendil-works/pi-tui";
|
||||
import type { SessionTreeNode } from "../../../core/session-manager.ts";
|
||||
import { formatReadLineRange } from "../../../core/tools/render-utils.ts";
|
||||
import { theme } from "../theme/theme.ts";
|
||||
import { DynamicBorder } from "./dynamic-border.ts";
|
||||
import { formatKeyText, keyHint } from "./keybinding-hints.ts";
|
||||
@@ -953,15 +954,7 @@ class TreeList implements Component {
|
||||
switch (name) {
|
||||
case "read": {
|
||||
const path = shortenPath(String(args.path || args.file_path || ""));
|
||||
const offset = args.offset as number | undefined;
|
||||
const limit = args.limit as number | undefined;
|
||||
let display = path;
|
||||
if (offset !== undefined || limit !== undefined) {
|
||||
const start = offset ?? 1;
|
||||
const end = limit !== undefined ? start + limit - 1 : "";
|
||||
display += `:${start}${end ? `-${end}` : ""}`;
|
||||
}
|
||||
return `[read: ${display}]`;
|
||||
return `[read: ${path}${formatReadLineRange(args.offset, args.limit)}]`;
|
||||
}
|
||||
case "write": {
|
||||
const path = shortenPath(String(args.path || args.file_path || ""));
|
||||
|
||||
@@ -0,0 +1,155 @@
|
||||
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { stripVTControlCharacters } from "node:util";
|
||||
import { runInNewContext } from "node:vm";
|
||||
import type { TUI } from "@earendil-works/pi-tui";
|
||||
import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
||||
import { exportFromFile } from "../src/core/export-html/index.ts";
|
||||
import {
|
||||
CURRENT_SESSION_VERSION,
|
||||
type SessionHeader,
|
||||
SessionManager,
|
||||
type SessionMessageEntry,
|
||||
} from "../src/core/session-manager.ts";
|
||||
import { createReadToolDefinition } from "../src/core/tools/read.ts";
|
||||
import { formatReadLineRange } from "../src/core/tools/render-utils.ts";
|
||||
import { ToolExecutionComponent } from "../src/modes/interactive/components/tool-execution.ts";
|
||||
import { TreeSelectorComponent } from "../src/modes/interactive/components/tree-selector.ts";
|
||||
import { initTheme } from "../src/modes/interactive/theme/theme.ts";
|
||||
|
||||
interface HtmlViewer {
|
||||
formatReadLineRange: typeof formatReadLineRange;
|
||||
getTreeNodeDisplayHtml(entry: SessionMessageEntry): string;
|
||||
renderEntry(entry: SessionMessageEntry): string;
|
||||
}
|
||||
|
||||
// Runs the exported viewer's real data loading and renderers, stopping before DOM initialization.
|
||||
function loadHtmlViewer(html: string): HtmlViewer {
|
||||
const sessionData = html.match(/<script id="session-data" type="application\/json">([^<]+)<\/script>/)?.[1];
|
||||
const script = [...html.matchAll(/<script>([\s\S]*?)<\/script>/g)].at(-1)?.[1];
|
||||
if (!sessionData || !script) throw new Error("Missing exported session data or viewer script");
|
||||
return runInNewContext(
|
||||
script.replace(
|
||||
"marked.use({",
|
||||
"return { formatReadLineRange, getTreeNodeDisplayHtml, renderEntry }; marked.use({",
|
||||
),
|
||||
{
|
||||
atob,
|
||||
TextDecoder,
|
||||
URLSearchParams,
|
||||
document: { getElementById: () => ({ textContent: sessionData }), querySelector: () => null },
|
||||
window: { location: { search: "" } },
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
// #9887
|
||||
describe("read line range display", () => {
|
||||
let tempDir: string;
|
||||
const cases: [unknown, unknown, string][] = [
|
||||
[25, 13, ":25-37"],
|
||||
["25", "13", ":25-37"],
|
||||
[null, "13", ":1-13"],
|
||||
["25", null, ":25"],
|
||||
[undefined, undefined, ""],
|
||||
["abc", 13, ""],
|
||||
[25, "", ""],
|
||||
[25, "Infinity", ""],
|
||||
];
|
||||
|
||||
beforeAll(() => {
|
||||
initTheme("dark");
|
||||
tempDir = mkdtempSync(join(tmpdir(), "pi-read-range-"));
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
rmSync(tempDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it("formats raw read arguments", () => {
|
||||
for (const [offset, limit, range] of cases) {
|
||||
expect(formatReadLineRange(offset, limit), `${offset}, ${limit}`).toBe(range);
|
||||
}
|
||||
});
|
||||
|
||||
it("shows string ranges in the TUI, session tree, and HTML export", async () => {
|
||||
const args = { path: "notes.txt", offset: "25", limit: "13" };
|
||||
const assistant: SessionMessageEntry = {
|
||||
type: "message",
|
||||
id: "assistant",
|
||||
parentId: null,
|
||||
timestamp: new Date(0).toISOString(),
|
||||
message: {
|
||||
role: "assistant",
|
||||
content: [{ type: "toolCall", id: "call", name: "read", arguments: args }],
|
||||
api: "anthropic-messages",
|
||||
provider: "anthropic",
|
||||
model: "test-model",
|
||||
usage: {
|
||||
input: 0,
|
||||
output: 0,
|
||||
cacheRead: 0,
|
||||
cacheWrite: 0,
|
||||
totalTokens: 0,
|
||||
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
|
||||
},
|
||||
stopReason: "toolUse",
|
||||
timestamp: 0,
|
||||
},
|
||||
};
|
||||
const result: SessionMessageEntry = {
|
||||
type: "message",
|
||||
id: "result",
|
||||
parentId: assistant.id,
|
||||
timestamp: assistant.timestamp,
|
||||
message: {
|
||||
role: "toolResult",
|
||||
toolCallId: "call",
|
||||
toolName: "read",
|
||||
content: [],
|
||||
isError: false,
|
||||
timestamp: 0,
|
||||
},
|
||||
};
|
||||
const header = {
|
||||
type: "session",
|
||||
version: CURRENT_SESSION_VERSION,
|
||||
id: "range-test",
|
||||
timestamp: assistant.timestamp,
|
||||
cwd: tempDir,
|
||||
} satisfies SessionHeader;
|
||||
const sessionPath = join(tempDir, "session.jsonl");
|
||||
writeFileSync(sessionPath, `${[header, assistant, result].map((entry) => JSON.stringify(entry)).join("\n")}\n`);
|
||||
|
||||
const tool = new ToolExecutionComponent(
|
||||
"read",
|
||||
"call",
|
||||
args,
|
||||
{},
|
||||
createReadToolDefinition(tempDir),
|
||||
// SAFETY: ToolExecutionComponent only calls ui.requestRender(), which this stub implements.
|
||||
{ requestRender() {} } as TUI,
|
||||
tempDir,
|
||||
);
|
||||
expect(stripVTControlCharacters(tool.render(120).join("\n"))).toContain("read notes.txt:25-37");
|
||||
|
||||
const tree = new TreeSelectorComponent(
|
||||
SessionManager.open(sessionPath).getTree(),
|
||||
result.id,
|
||||
40,
|
||||
() => {},
|
||||
() => {},
|
||||
);
|
||||
expect(tree.getTreeList().render(120).map(stripVTControlCharacters).join("\n")).toContain(
|
||||
"[read: notes.txt:25-37]",
|
||||
);
|
||||
|
||||
const viewer = loadHtmlViewer(readFileSync(await exportFromFile(sessionPath, join(tempDir, "out.html")), "utf8"));
|
||||
for (const [offset, limit, range] of cases) {
|
||||
expect(viewer.formatReadLineRange(offset, limit), `${offset}, ${limit}`).toBe(range);
|
||||
}
|
||||
expect(viewer.getTreeNodeDisplayHtml(result)).toBe('<span class="tree-role-tool">[read: notes.txt:25-37]</span>');
|
||||
expect(viewer.renderEntry(assistant)).toContain('<span class="line-numbers">:25-37</span>');
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user