Merge PR #8849: perf(read): materialize a local file once per read (@alphastorm)

This commit is contained in:
can1357
2026-08-19 01:36:59 +02:00
8 changed files with 538 additions and 81 deletions
+7
View File
@@ -58,6 +58,13 @@
### Changed
- Send the `omp/<version>` User-Agent on xAI chat (`xai` and `xai-oauth`) unless the request already set its own ([#8840](https://github.com/can1357/oh-my-pi/pull/8840) by [@Jaaneek](https://github.com/Jaaneek)).
### Changed
- The `read` tool now materializes a local text file once per invocation instead of once per consumer. A ranged read of a file within the snapshot cap previously cost four opens and three UTF-8 decodes — an 8KiB binary sniff, a streaming scan for the rendered window, a whole-file read for bracket context, and another whole-file read to hash the snapshot — with two of those readers separately normalizing line endings; whole-file reads under the structural summarizer paid a fifth read. Byte counts and truncation boundaries are now measured on the buffered bytes, so they stay exact for content that is not valid UTF-8. Files above the snapshot cap keep streaming, since nothing on that path wants the whole file. Raw reads, which skip the tree-sitter parse that dominates other reads, are 30-45% faster (2.7MB file: 10.3ms to 5.7ms).
### Fixed
- Fixed a whole-file `read` of a file with a UTF-8 BOM minting a hashline tag hashed from BOM-bearing text. Because the patcher's live read strips the BOM, the next edit to that file only applied through stale-hash recovery and reported that the file had changed externally when it had not.
## [17.3.6] - 2026-08-17
+16 -2
View File
@@ -72,7 +72,20 @@ export async function readHashlineHeaderContext(
absolutePath: string,
cwd: string,
): Promise<HashlineHeaderContext> {
const fullText = await Bun.file(absolutePath).text();
return hashlineHeaderContextForText(session, absolutePath, cwd, await Bun.file(absolutePath).text());
}
/**
* {@link readHashlineHeaderContext} for a caller that already holds the file's
* full text, so the file is not reopened just to hash it. Line endings are
* normalized here, exactly as the reading variant does.
*/
export function hashlineHeaderContextForText(
session: ToolSession,
absolutePath: string,
cwd: string,
fullText: string,
): HashlineHeaderContext {
const context = recordFullHashlineContext(
session,
absolutePath,
@@ -392,6 +405,7 @@ export function buildInMemoryTextResult(
const buildLineEntries = (endLineDisplay: number): LineEntry[] =>
buildLineEntriesWithBlockContext(allLines, [{ startLine: startLineDisplay, endLine: endLineDisplay }], {
path: options.sourcePath,
text,
});
let outputText: string;
@@ -534,7 +548,7 @@ export function buildInMemoryMultiRangeResult(
if (options.raw === true) {
outputText = rawParts.length > 0 ? rawParts.join("\n\n…\n\n") : "";
} else if (visibleSpans.length > 0) {
const entries = buildLineEntriesWithBlockContext(allLines, visibleSpans, { path: options.sourcePath });
const entries = buildLineEntriesWithBlockContext(allLines, visibleSpans, { path: options.sourcePath, text });
if (shouldAddHashLines) seenLines = lineNumbersFromEntries(entries);
const firstLine = entries.find(entry => entry.kind === "line");
if (firstLine?.kind === "line") {
@@ -52,21 +52,26 @@ export function routeReadThroughBridge(
if (!bridge?.capabilities.readTextFile || !bridge.readTextFile) return undefined;
return bridge.readTextFile({ path: absolutePath, ...options });
}
/**
* Structural summary of `absolutePath`, or `null` when the file is too large,
* too short, or unparseable. `diskText` lets a caller that already read the file
* hand those bytes over instead of forcing a second read; an ACP bridge still
* wins, since the editor's buffer is the source of truth.
*/
export async function trySummarize(
session: ToolSession,
absolutePath: string,
fileSize: number,
signal?: AbortSignal,
diskText?: string,
): Promise<SummaryResult | null> {
if (fileSize > MAX_SUMMARY_BYTES) return null;
try {
throwIfAborted(signal);
const bridgePromise = routeReadThroughBridge(session, absolutePath);
const code =
bridgePromise !== undefined
? await bridgePromise.catch(() => Bun.file(absolutePath).text())
: await Bun.file(absolutePath).text();
const readDisk = async () => diskText ?? (await Bun.file(absolutePath).text());
const code = bridgePromise !== undefined ? await bridgePromise.catch(readDisk) : await readDisk();
throwIfAborted(signal);
const lineCount = countTextLines(code);
if (lineCount > MAX_SUMMARY_LINES) return null;
+306 -72
View File
@@ -10,7 +10,15 @@ import type {
ToolTier,
} from "@oh-my-pi/pi-agent-core";
import type { ImageContent, TextContent } from "@oh-my-pi/pi-ai";
import { type ImageMetadata, isProbablyBinary, logger, prompt, readImageMetadata } from "@oh-my-pi/pi-utils";
import {
BINARY_SNIFF_BYTES,
type ImageMetadata,
isProbablyBinary,
isProbablyBinaryHeader,
logger,
prompt,
readImageMetadata,
} from "@oh-my-pi/pi-utils";
import {
canonicalSnapshotKey,
getFileSnapshotStore,
@@ -85,6 +93,7 @@ import {
formatTextWithMode,
type HashlineHeaderContext,
hashlineHeaderContext,
hashlineHeaderContextForText,
lineNumbersFromSpans,
markMarkdownContentType,
prependHashlineHeader,
@@ -117,15 +126,198 @@ export { readToolRenderer } from "./read-renderer";
const MAX_PROFILE_SUMMARY_BYTES = 32 * 1024 * 1024;
const MAX_ARTIFACT_RAW_INLINE_BYTES = DEFAULT_MAX_BYTES;
async function readBracketContextFullLines(absolutePath: string, fileSize: number): Promise<string[] | undefined> {
if (fileSize > SNAPSHOT_MAX_BYTES) return undefined;
/** LF byte, scanned natively to find line boundaries in a buffered file. */
const LF_BYTE = 0x0a;
/**
* Whole-file bytes plus every view the local text read path consumes,
* materialized exactly once.
*
* The binary sniff, the structural summary, the emitted line window, bracket
* context and the snapshot hash all want the same bytes. Each used to open the
* file for itself, so a single ranged read cost up to four opens, three UTF-8
* decodes and two CRLF normalization passes over identical content.
*
* Only files at or below {@link SNAPSHOT_MAX_BYTES} are buffered: past that cap
* bracket context and the snapshot are skipped anyway, so streaming a window
* stays strictly cheaper than materializing the file.
*/
interface BufferedFileText {
/** File bytes, verbatim. */
readonly bytes: Buffer;
/** Verbatim UTF-8 decode: a leading BOM and CRLF line endings both survive. */
readonly rawText: string;
/** {@link rawText} split on LF, CR retained, so segments stay byte-faithful. */
readonly rawSegments: readonly string[];
/** BOM-stripped, CRLF-preserving text: what `Bun.file(path).text()` returns. */
readonly strippedText: string;
/** {@link strippedText} normalized to LF — the exact text the snapshot store hashes. */
readonly normalizedText: string;
/** Addressable lines of {@link normalizedText}; bracket context indexes these. */
readonly addressableLines: readonly string[];
/** Whether the final byte is LF. */
readonly endsWithNewline: boolean;
}
/**
* Read the whole file, or `undefined` when the bytes cannot be read — which
* drops the caller back to the streaming reader and reproduces today's error
* surface.
*
* Kept separate from {@link deriveBufferedFileText} so the binary sniff can run
* on the bytes first: a file that decodes to mojibake is refused, and building
* three string views of it before finding that out would be pure waste.
*/
async function readWholeFile(absolutePath: string): Promise<Buffer | undefined> {
try {
return splitAddressableFileLines(normalizeToLF(await Bun.file(absolutePath).text()));
return await fs.readFile(absolutePath);
} catch {
return undefined;
}
}
/**
* Derive every view of `bytes` the read path needs, decoding exactly once.
*
* `Bun.file(path).text()` strips a leading BOM while `Buffer.toString` keeps it,
* and the snapshot store plus the patcher's live-file read both go through the
* stripping decoder. {@link BufferedFileText.strippedText} therefore reproduces
* that decode for hashing while {@link BufferedFileText.rawText} stays verbatim
* for the emitted lines and their byte accounting.
*/
function deriveBufferedFileText(bytes: Buffer): BufferedFileText {
const rawText = bytes.toString("utf-8");
const strippedText = rawText.charCodeAt(0) === 0xfeff ? rawText.slice(1) : rawText;
// `normalizeToLF` allocates a copy; skip it outright for the common LF file.
const normalizedText = strippedText.includes("\r") ? normalizeToLF(strippedText) : strippedText;
const rawSegments = rawText.split("\n");
let addressableLines: readonly string[];
if (normalizedText === rawText) {
// Nothing was rewritten, so display and bracket context share one array;
// the terminal newline sentinel is dropped exactly as
// `splitAddressableFileLines` does.
const last = rawSegments.length - 1;
addressableLines = last > 0 && rawSegments[last] === "" ? rawSegments.slice(0, last) : rawSegments;
} else {
addressableLines = splitAddressableFileLines(normalizedText);
}
return {
bytes,
rawText,
rawSegments,
strippedText,
normalizedText,
addressableLines,
endsWithNewline: bytes.length > 0 && bytes[bytes.length - 1] === LF_BYTE,
};
}
/** The line window a range read renders, with the budget accounting behind it. */
interface ReadLineWindow {
lines: string[];
totalFileLines: number;
collectedBytes: number;
stoppedByByteLimit: boolean;
firstLinePreview?: { text: string; bytes: number };
firstLineByteLength?: number;
/** Whether the fully scanned source ended in a newline. */
hasTrailingNewline: boolean;
/** False when `stopScanAfterCollect` cut the scan short — `totalFileLines` is then a lower bound. */
reachedEof: boolean;
}
/**
* Slice the window {@link streamLinesFromFile} would have collected out of an
* already-buffered file, under the identical line and byte budgets.
*
* Line byte lengths are walked out of the buffer rather than measured on the
* decoded strings: a file that is not valid UTF-8 decodes to U+FFFD, whose
* encoded length differs from the bytes on disk, and those lengths decide both
* the reported byte counts and where truncation lands.
*/
function collectLineWindowFromBuffer(
file: BufferedFileText,
startLine: number,
maxLinesToCollect: number,
maxBytes: number,
selectedLineLimit: number,
includeTerminalNewline: boolean,
): ReadLineWindow {
const { bytes, rawSegments, endsWithNewline } = file;
// A trailing LF closes the last line rather than opening an empty one, except
// in raw mode where that terminal sentinel is addressable.
const totalFileLines =
endsWithNewline && !includeTerminalNewline && rawSegments.length > 1
? rawSegments.length - 1
: rawSegments.length;
const window: ReadLineWindow = {
lines: [],
totalFileLines,
collectedBytes: 0,
stoppedByByteLimit: false,
hasTrailingNewline: endsWithNewline,
reachedEof: true,
};
if (startLine >= totalFileLines) return window;
let lineStart = 0;
for (let index = 0; index < startLine; index++) {
const newlineAt = bytes.indexOf(LF_BYTE, lineStart);
if (newlineAt === -1) {
lineStart = bytes.length;
break;
}
lineStart = newlineAt + 1;
}
let doneCollecting = false;
let selectedLinesSeen = 0;
for (let index = startLine; index < totalFileLines; index++) {
const newlineAt = bytes.indexOf(LF_BYTE, lineStart);
const lineEnd = newlineAt === -1 ? bytes.length : newlineAt;
const lineByteLength = lineEnd - lineStart;
if (selectedLinesSeen < selectedLineLimit) selectedLinesSeen++;
// Preview covers the first selected line only, capped at the byte budget:
// the oversized-first-line branch renders it when no full line fits.
if (window.lines.length === 0 && window.firstLinePreview === undefined && lineByteLength > 0) {
const previewEnd = Math.min(lineEnd, lineStart + maxBytes);
const { text, bytes: previewBytes } = truncateHeadBytes(bytes.subarray(lineStart, previewEnd), maxBytes);
window.firstLinePreview = { text, bytes: previewBytes };
}
if (!doneCollecting) {
const separatorBytes = window.lines.length > 0 ? 1 : 0;
if (window.lines.length >= maxLinesToCollect) {
doneCollecting = true;
} else if (window.lines.length === 0 && lineByteLength > maxBytes) {
window.stoppedByByteLimit = true;
doneCollecting = true;
window.firstLineByteLength ??= lineByteLength;
} else if (window.lines.length > 0 && window.collectedBytes + separatorBytes + lineByteLength > maxBytes) {
window.stoppedByByteLimit = true;
doneCollecting = true;
} else {
window.lines.push(rawSegments[index] ?? "");
window.collectedBytes += separatorBytes + lineByteLength;
window.firstLineByteLength ??= lineByteLength;
if (window.collectedBytes > maxBytes) {
window.stoppedByByteLimit = true;
doneCollecting = true;
} else if (window.lines.length >= maxLinesToCollect) {
doneCollecting = true;
}
}
} else if (window.firstLineByteLength === undefined) {
window.firstLineByteLength = lineByteLength;
}
if (doneCollecting && selectedLinesSeen >= selectedLineLimit) break;
lineStart = lineEnd + 1;
}
return window;
}
interface StreamFileLinesOptions {
includeTerminalNewline?: boolean;
stopScanAfterCollect?: boolean;
@@ -139,19 +331,7 @@ async function streamLinesFromFile(
selectedLineLimit: number | null,
signal?: AbortSignal,
options: StreamFileLinesOptions = {},
): Promise<{
lines: string[];
totalFileLines: number;
collectedBytes: number;
stoppedByByteLimit: boolean;
firstLinePreview?: { text: string; bytes: number };
firstLineByteLength?: number;
selectedBytesTotal: number;
/** Whether the fully scanned source ended in a newline. */
hasTrailingNewline: boolean;
/** False when `stopScanAfterCollect` cut the scan short — `totalFileLines` is then a lower bound. */
reachedEof: boolean;
}> {
): Promise<ReadLineWindow> {
const { includeTerminalNewline = false, stopScanAfterCollect = false } = options;
const bufferChunk = Buffer.allocUnsafe(READ_CHUNK_SIZE);
const collectedLines: string[] = [];
@@ -168,7 +348,6 @@ async function streamLinesFromFile(
let firstLinePreviewBytes = 0;
const firstLinePreviewChunks: Buffer[] = [];
let firstLineByteLength: number | undefined;
let selectedBytesTotal = 0;
let selectedLinesSeen = 0;
let captureLine = false;
let discardLineChunks = false;
@@ -219,7 +398,6 @@ async function streamLinesFromFile(
const finalizeLine = () => {
if (lineIndex >= startLine && (selectedLineLimit === null || selectedLinesSeen < selectedLineLimit)) {
selectedBytesTotal += currentLineLength + (selectedLinesSeen > 0 ? 1 : 0);
selectedLinesSeen++;
}
@@ -337,7 +515,6 @@ async function streamLinesFromFile(
stoppedByByteLimit,
firstLinePreview,
firstLineByteLength,
selectedBytesTotal,
reachedEof,
hasTrailingNewline: reachedEof && endedWithNewline,
};
@@ -659,15 +836,17 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
}
/**
* Stream multiple non-contiguous ranges from a local file. ACP bridge takes
* priority when present (editor buffer is source of truth); otherwise each
* range is streamed independently with its own line/byte budget. Out-of-bounds
* ranges surface as inline notices rather than aborting the read.
* Render multiple non-contiguous ranges of a local file. ACP bridge takes
* priority when present (editor buffer is source of truth); otherwise ranges
* are sliced out of `buffered` when the caller already materialized the file,
* and streamed independently with their own line/byte budget when it did not.
* Out-of-bounds ranges surface as inline notices rather than aborting the read.
*/
async #readLocalFileMultiRange(
absolutePath: string,
ranges: readonly LineRange[],
fileSize: number,
buffered: BufferedFileText | undefined,
parsed: ParsedSelector,
displayMode: { hashLines: boolean; lineNumbers: boolean },
suffixResolution: { from: string; to: string } | undefined,
@@ -715,7 +894,7 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
const notices: string[] = [];
const visibleSpans: Array<{ startLine: number; endLine: number }> = [];
const displayLineByNumber = new Map<number, string>();
const fullLines = rawSelector ? undefined : await readBracketContextFullLines(absolutePath, fileSize);
const fullLines = rawSelector ? undefined : buffered?.addressableLines;
let columnTruncated = 0;
let displayContent: { text: string; startLine: number; lineNumbers?: Array<number | null> } | undefined;
@@ -724,27 +903,25 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
const requestedLength = range.endLine !== undefined ? range.endLine - range.startLine + 1 : this.#defaultLimit;
const maxLines = Math.min(requestedLength, DEFAULT_MAX_LINES);
// When the full file is already in memory (the common case for files
// within the snapshot byte cap), slice ranges from it instead of
// re-streaming the file once per range.
// The file is already in memory for everything within the snapshot byte
// cap, so slice ranges out of it instead of re-streaming per range. Raw
// mode cannot use the addressable lines (it keeps CR bytes and the
// terminal newline sentinel) but still slices the same buffer.
let collectedLines: string[];
let totalFileLines: number;
const maxBytesForRead = Math.max(DEFAULT_MAX_BYTES, maxLines * 512);
if (fullLines) {
totalFileLines = fullLines.length;
collectedLines = fullLines.slice(rangeStart, rangeStart + maxLines);
} else {
const maxBytesForRead = Math.max(DEFAULT_MAX_BYTES, maxLines * 512);
const streamResult = await streamLinesFromFile(
absolutePath,
rangeStart,
maxLines,
maxBytesForRead,
maxLines,
signal,
{ includeTerminalNewline: rawSelector, stopScanAfterCollect: fileSize > SNAPSHOT_MAX_BYTES },
);
totalFileLines = streamResult.totalFileLines;
collectedLines = streamResult.lines;
const window = buffered
? collectLineWindowFromBuffer(buffered, rangeStart, maxLines, maxBytesForRead, maxLines, rawSelector)
: await streamLinesFromFile(absolutePath, rangeStart, maxLines, maxBytesForRead, maxLines, signal, {
includeTerminalNewline: rawSelector,
stopScanAfterCollect: fileSize > SNAPSHOT_MAX_BYTES,
});
totalFileLines = window.totalFileLines;
collectedLines = window.lines;
}
if (rangeStart >= totalFileLines) {
@@ -786,7 +963,7 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
const entries = buildLineEntriesWithBlockContext(
fullLines,
visibleSpans,
{ path: absolutePath },
{ path: absolutePath, text: buffered?.normalizedText },
{
lineText: (lineNumber, sourceText) => {
const visibleText = displayLineByNumber.get(lineNumber);
@@ -811,14 +988,26 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
outputText = blocks.join("\n\n…\n\n");
}
if (shouldAddHashLines && outputText) {
const tag = await recordFileSnapshot(this.session, absolutePath);
const tag = buffered
? getFileSnapshotStore(this.session).record(canonicalSnapshotKey(absolutePath), buffered.normalizedText)
: await recordFileSnapshot(this.session, absolutePath);
if (tag) {
recordSeenLinesFromBody(this.session, absolutePath, tag, outputText);
outputText = `${formatReadHashlineHeader(formatPathRelativeToCwd(absolutePath, this.session.cwd), tag)}\n${outputText}`;
}
} else if (rawSelector && visibleSpans.length > 0) {
const rawSeenLines = lineNumbersFromSpans(visibleSpans);
if (rawSeenLines.length > 0) await recordFileSnapshot(this.session, absolutePath, rawSeenLines);
if (rawSeenLines.length > 0) {
if (buffered) {
getFileSnapshotStore(this.session).record(
canonicalSnapshotKey(absolutePath),
buffered.normalizedText,
rawSeenLines,
);
} else {
await recordFileSnapshot(this.session, absolutePath, rawSeenLines);
}
}
}
if (notices.length > 0) {
outputText = outputText ? `${outputText}\n${notices.join("\n")}` : notices.join("\n");
@@ -1161,6 +1350,12 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
content = [{ type: "text", text: `[Cannot read ${ext} file: conversion failed]` }];
}
} else {
// One read for every consumer below. The sniff, the structural summary,
// the rendered window, bracket context and the snapshot hash all want
// the same bytes; past the snapshot cap nothing wants the whole file,
// so the streaming reader keeps that case cheap.
const wholeFileBytes = fileSize <= SNAPSHOT_MAX_BYTES ? await readWholeFile(absolutePath) : undefined;
// Binary sniff before any UTF-8 text materialization. A binary file
// (font, object, archive, packed blob) decodes to NUL/control bytes and
// U+FFFD mojibake that corrupts the terminal and burns context. Images,
@@ -1168,7 +1363,12 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
// everything reaching here is meant to be plain text. `:raw` stays the
// explicit escape hatch for reading bytes verbatim. This single guard
// covers both the multi-range and single-range disk paths below.
if (!isRawSelector(parsed) && (await isProbablyBinary(absolutePath))) {
const looksBinary =
!isRawSelector(parsed) &&
(wholeFileBytes
? isProbablyBinaryHeader(wholeFileBytes.subarray(0, BINARY_SNIFF_BYTES))
: await isProbablyBinary(absolutePath));
if (looksBinary) {
return toolResult<ReadToolDetails>({ resolvedPath: absolutePath, suffixResolution })
.text(
prependSuffixResolutionNotice(
@@ -1179,13 +1379,15 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
.sourcePath(absolutePath)
.done();
}
// Decode only what survived the sniff.
const buffered = wholeFileBytes ? deriveBufferedFileText(wholeFileBytes) : undefined;
if (
parsed.kind === "none" &&
this.session.settings.get("read.summarize.enabled") &&
(this.session.settings.get("read.summarize.prose") || !isProseSummaryPath(absolutePath))
) {
const summary = await trySummarize(this.session, absolutePath, fileSize, signal);
const summary = await trySummarize(this.session, absolutePath, fileSize, signal, buffered?.strippedText);
if (summary?.parsed && summary.elided) {
const renderedSummary = renderSummary(this.session, summary);
const footer = formatSummaryElisionFooter(
@@ -1194,7 +1396,14 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
renderedSummary.elidedLines,
);
const summaryHashContext = displayMode.hashLines
? await readHashlineHeaderContext(this.session, absolutePath, this.session.cwd)
? buffered
? hashlineHeaderContextForText(
this.session,
absolutePath,
this.session.cwd,
buffered.normalizedText,
)
: await readHashlineHeaderContext(this.session, absolutePath, this.session.cwd)
: undefined;
const bodyText = footer ? `${renderedSummary.text}\n\n${footer}` : renderedSummary.text;
const modelText = prependHashlineHeader(bodyText, summaryHashContext);
@@ -1221,6 +1430,7 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
absolutePath,
parsed.ranges,
fileSize,
buffered,
parsed,
displayMode,
suffixResolution,
@@ -1285,15 +1495,24 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
// Assume ~512 bytes/line average; never go below the shared default.
const maxBytesForRead = Math.max(DEFAULT_MAX_BYTES, maxLinesToCollect * 512);
const streamResult = await streamLinesFromFile(
absolutePath,
startLine,
maxLinesToCollect,
maxBytesForRead,
selectedLineLimit,
undefined, // plain-file read: deterministic and fast, never abort mid-read
{ includeTerminalNewline: rawSelector, stopScanAfterCollect: fileSize > SNAPSHOT_MAX_BYTES },
);
const lineWindow = buffered
? collectLineWindowFromBuffer(
buffered,
startLine,
maxLinesToCollect,
maxBytesForRead,
selectedLineLimit,
rawSelector,
)
: await streamLinesFromFile(
absolutePath,
startLine,
maxLinesToCollect,
maxBytesForRead,
selectedLineLimit,
undefined, // plain-file read: deterministic and fast, never abort mid-read
{ includeTerminalNewline: rawSelector, stopScanAfterCollect: fileSize > SNAPSHOT_MAX_BYTES },
);
const {
lines: collectedLines,
@@ -1304,7 +1523,7 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
firstLineByteLength,
reachedEof,
hasTrailingNewline,
} = streamResult;
} = lineWindow;
// Check if offset is out of bounds - return graceful message instead of throwing
if (requestedStart >= totalFileLines) {
@@ -1347,9 +1566,7 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
for (let i = 0; i < displayLines.length; i++) {
displayLineByNumber.set(startLineDisplay + i, displayLines[i] ?? "");
}
const bracketContextFullLines = rawSelector
? undefined
: await readBracketContextFullLines(absolutePath, fileSize);
const bracketContextFullLines = rawSelector ? undefined : buffered?.addressableLines;
const displayedEndLine = startLineDisplay + Math.max(0, displayLines.length - 1);
const selectedContent = displayLines.join("\n");
@@ -1376,17 +1593,22 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
const shouldAddLineNumbers = rawSelector ? false : shouldAddHashLines ? false : displayMode.lineNumbers;
let hashContext: HashlineHeaderContext | undefined;
if (shouldAddHashLines && collectedLines.length > 0 && !firstLineExceedsLimit) {
// The tag is a content hash of the WHOLE file. A whole-file read
// already holds every line in memory; a range read re-reads the
// file (bounded by SNAPSHOT_MAX_BYTES) so the tag fingerprints the
// full file and any anchor validates while the file is unchanged.
// The tag is a content hash of the WHOLE file, so any anchor the
// model returns validates while the live file is unchanged. The
// buffered text is that whole file; above the snapshot cap only a
// non-truncated whole-file window can supply it.
const isWholeFile = offset === undefined && limit === undefined && !wasTruncated;
const tag = isWholeFile
const tag = buffered
? getFileSnapshotStore(this.session).record(
canonicalSnapshotKey(absolutePath),
normalizeToLF(`${collectedLines.join("\n")}${hasTrailingNewline ? "\n" : ""}`),
buffered.normalizedText,
)
: await recordFileSnapshot(this.session, absolutePath);
: isWholeFile
? getFileSnapshotStore(this.session).record(
canonicalSnapshotKey(absolutePath),
normalizeToLF(`${collectedLines.join("\n")}${hasTrailingNewline ? "\n" : ""}`),
)
: await recordFileSnapshot(this.session, absolutePath);
if (tag) {
hashContext = hashlineHeaderContext(formatPathRelativeToCwd(absolutePath, this.session.cwd), tag);
}
@@ -1413,7 +1635,7 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
const entries = buildLineEntriesWithBlockContext(
bracketContextFullLines,
[{ startLine: startLineDisplay, endLine: displayedEndLine }],
{ path: absolutePath },
{ path: absolutePath, text: buffered?.normalizedText },
{
lineText: (lineNumber, sourceText) => {
const visibleText = displayLineByNumber.get(lineNumber);
@@ -1500,11 +1722,18 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
recordSeenLinesFromBody(this.session, absolutePath, hashContext.tag, outputText);
}
if (rawSelector && !firstLineExceedsLimit && collectedLines.length > 0) {
await recordFileSnapshot(
this.session,
absolutePath,
contiguousLineNumbers(startLineDisplay, collectedLines.length),
);
// A raw read emits no header, but recording the range it displayed
// lets a same-content hashline tag inherit its provenance.
const seenLines = contiguousLineNumbers(startLineDisplay, collectedLines.length);
if (buffered) {
getFileSnapshotStore(this.session).record(
canonicalSnapshotKey(absolutePath),
buffered.normalizedText,
seenLines,
);
} else {
await recordFileSnapshot(this.session, absolutePath, seenLines);
}
}
if (capturedDisplayContent) {
@@ -1685,10 +1914,15 @@ export class ReadTool implements AgentTool<typeof readSchema, ReadToolDetails> {
const rawSelector = isRawSelector(parsedSel);
const displayMode = resolveFileDisplayMode(this.session, { raw: rawSelector, immutable: true });
if (isMultiRange(parsedSel) && parsedSel.kind === "lines") {
// Bracket context and per-range slicing both want the whole artifact, so
// materialize it once exactly as the plain-file path does.
const artifactBytes = artifact.size <= SNAPSHOT_MAX_BYTES ? await readWholeFile(artifact.path) : undefined;
const buffered = artifactBytes ? deriveBufferedFileText(artifactBytes) : undefined;
const read = await this.#readLocalFileMultiRange(
artifact.path,
parsedSel.ranges,
artifact.size,
buffered,
parsedSel,
displayMode,
undefined,
@@ -22,6 +22,20 @@ export interface LineSpan {
export interface BlockContextSource {
path?: string;
lang?: string;
/**
* The whole source `fullLines` was split from, when the caller still holds it.
* Supplying it skips re-joining every line into a fresh whole-file string on
* the way to the parser. It MUST be the same content as `fullLines`; a
* differing trailing newline is the only tolerated variation, since it moves
* no node's line number.
*
* Every current supplier derives both from one buffer in the same breath, so
* the two cannot drift. Do NOT set it on a source object that is reused
* across two different line arrays — a before/after diff pair, say — because
* the boundary lines tree-sitter reports would then be indexed into the wrong
* array and surface off-by-N context rows.
*/
text?: string;
}
export type LineEntry = { kind: "line"; lineNumber: number; text: string; context: boolean } | { kind: "ellipsis" };
@@ -105,7 +119,7 @@ function nativeBlockContext(
let boundaries: number[] | null;
try {
boundaries = enclosingBlockBoundaries({
code: fullLines.join("\n"),
code: source.text ?? fullLines.join("\n"),
path: source.path,
lang: source.lang,
ranges,
@@ -0,0 +1,175 @@
/**
* The local text read path materializes a file once and derives every view from
* those bytes: the binary sniff, the rendered window and its byte accounting,
* bracket context, and the whole-file snapshot hash. These tests pin the parts
* of that contract a plausible rewrite would silently break — the decode the
* snapshot tag is hashed from, exact on-disk byte counts, the raw terminal
* newline sentinel, and the absence of a second whole-file read.
*/
import { afterEach, beforeEach, describe, expect, it } from "bun:test";
import * as fs from "node:fs/promises";
import * as os from "node:os";
import * as path from "node:path";
import { Patch, Patcher } from "@oh-my-pi/hashline";
import type { AgentToolResult } from "@oh-my-pi/pi-agent-core";
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
import { getFileSnapshotStore } from "@oh-my-pi/pi-coding-agent/edit/file-snapshot-store";
import { HashlineFilesystem } from "@oh-my-pi/pi-coding-agent/edit/hashline/filesystem";
import { writethroughNoop } from "@oh-my-pi/pi-coding-agent/lsp";
import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools";
import type { ReadToolDetails } from "@oh-my-pi/pi-coding-agent/tools/read";
import { ReadTool } from "@oh-my-pi/pi-coding-agent/tools/read";
import { formatBytes } from "@oh-my-pi/pi-coding-agent/tools/render-utils";
import { removeWithRetries } from "@oh-my-pi/pi-utils";
function textOutput(result: AgentToolResult<ReadToolDetails>): string {
return result.content
.filter(c => c.type === "text")
.map(c => c.text)
.join("\n");
}
function createSession(cwd: string): ToolSession {
const settings = Settings.isolated();
// Structural summarization would answer whole-file reads from the summarizer
// instead of the range path under test.
settings.set("read.summarize.enabled", false);
return {
cwd,
hasUI: false,
getSessionFile: () => path.join(cwd, "session.jsonl"),
getSessionSpawns: () => "*",
getArtifactsDir: () => path.join(cwd, "artifacts"),
allocateOutputArtifact: async () => ({ id: "artifact-1", path: path.join(cwd, "artifact-1.log") }),
settings,
} as ToolSession;
}
describe("read tool single-pass file access", () => {
let tmpDir: string;
beforeEach(async () => {
tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "read-single-pass-"));
});
afterEach(async () => {
await removeWithRetries(tmpDir);
});
it("hashes the snapshot from BOM-stripped text so a whole-file tag validates without recovery", async () => {
// `Bun.file().text()` strips a leading BOM and the patcher's live read goes
// through it, so a tag hashed from BOM-bearing text only ever applies via
// stale-hash recovery — which tells the model the file changed externally
// when nothing changed.
const filePath = path.join(tmpDir, "bom.ts");
await fs.writeFile(filePath, Buffer.from("\uFEFFexport const a = 1;\nexport const b = 2;\n", "utf-8"));
const session = createSession(tmpDir);
const header = textOutput(await new ReadTool(session).execute("bom-read", { path: filePath })).split("\n")[0];
expect(header).toMatch(/^\[bom\.ts#[0-9A-F]{4}\]$/);
const patcher = new Patcher({
fs: new HashlineFilesystem({
session,
writethrough: writethroughNoop,
beginDeferredDiagnosticsForPath: () => {
throw new Error("deferred diagnostics are unused");
},
}),
snapshots: getFileSnapshotStore(session),
});
const applied = await patcher.apply(Patch.parse(`${header}\nPUT 2.=2:\n+export const b = 22;`, { cwd: tmpDir }));
expect(applied.sections[0]?.warnings).toEqual([]);
// The BOM survives the write; only the addressed line changed.
expect(await fs.readFile(filePath, "utf8")).toBe("\uFEFFexport const a = 1;\nexport const b = 22;\n");
});
it("reports on-disk byte lengths for a line that is not valid UTF-8", async () => {
// Decoding replaces each stray byte with U+FFFD, which re-encodes to three
// bytes. Measuring the decoded string instead of the buffer would inflate
// every reported length by two bytes per stray byte.
const strayBytes = 512;
const lineBytes = 60 * 1024;
const filePath = path.join(tmpDir, "invalid-utf8.txt");
await fs.writeFile(
filePath,
Buffer.concat([
// Stray bytes sit past the 8KiB binary sniff window, so the file still
// reads as text and reaches the oversized-line notice.
Buffer.from("z".repeat(lineBytes - strayBytes), "utf-8"),
Buffer.from(Array.from({ length: strayBytes }, () => 0xff)),
Buffer.from("\ntail\n", "utf-8"),
]),
);
// `:1-1` keeps the byte budget at its 50KB floor, which the 60KB line exceeds.
const text = textOutput(await new ReadTool(createSession(tmpDir)).execute("bytes", { path: `${filePath}:1-1` }));
expect(text).toContain(`[Line 1 is ${formatBytes(lineBytes)}, exceeds ${formatBytes(50 * 1024)} limit`);
expect(text).not.toContain(formatBytes(lineBytes + strayBytes * 2));
});
it("counts the terminal newline as an addressable line only in raw mode", async () => {
const filePath = path.join(tmpDir, "trailing.txt");
await fs.writeFile(filePath, "alpha\nbeta\n");
const tool = new ReadTool(createSession(tmpDir));
// Non-raw: the trailing LF closes line 2 rather than opening line 3.
expect(textOutput(await tool.execute("beyond", { path: `${filePath}:3` }))).toBe(
"Line 3 is beyond end of file (2 lines total). Use :1 to read from the start, or :2 to read the last line.",
);
// Raw: the sentinel is addressable, so line 3 exists and is empty.
expect(textOutput(await tool.execute("raw-sentinel", { path: `${filePath}:raw:3` }))).toBe("");
expect(textOutput(await tool.execute("raw-beyond", { path: `${filePath}:raw:4` }))).toContain(
"beyond end of file (3 lines total)",
);
});
it("does not re-read a file it already materialized", async () => {
// Bracket context and the snapshot hash each used to pull the whole file
// through `Bun.file(path).text()`, so a ranged read of one file opened it
// four times. Any reintroduced whole-file re-read trips this counter.
const filePath = path.join(tmpDir, "counted.ts");
await fs.writeFile(
filePath,
`${Array.from({ length: 400 }, (_, i) => `export const value${i} = ${i};`).join("\n")}\n`,
);
type FileFactory = typeof Bun.file;
const originalFile: FileFactory = Bun.file;
const bunNamespace = Bun as unknown as { file: FileFactory };
let wholeFileReads = 0;
bunNamespace.file = ((target: Parameters<FileFactory>[0], ...rest: unknown[]) => {
const factory = originalFile as unknown as (t: unknown, ...r: unknown[]) => Bun.BunFile;
const handle = factory(target, ...rest);
if (typeof target !== "string" || target !== filePath) return handle;
return new Proxy(handle, {
// Receiver must be the real BunFile: native accessors such as `size`
// throw when `this` is the proxy.
get(obj, prop) {
const value = Reflect.get(obj, prop);
if (prop === "text" || prop === "bytes" || prop === "arrayBuffer") {
const reader = value as (...a: unknown[]) => Promise<unknown>;
return (...args: unknown[]) => {
wholeFileReads++;
return reader.apply(obj, args);
};
}
return typeof value === "function" ? value.bind(obj) : value;
},
});
}) as FileFactory;
try {
const tool = new ReadTool(createSession(tmpDir));
const text = textOutput(await tool.execute("counted", { path: `${filePath}:100-120` }));
expect(text).toContain("export const value100 = 100;");
expect(text).toMatch(/^\[counted\.ts#[0-9A-F]{4}\]$/m);
} finally {
bunNamespace.file = originalFile;
}
expect(wholeFileReads).toBe(0);
});
});
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### Added
- Exported `BINARY_SNIFF_BYTES`, the header window `isProbablyBinary` sniffs, so a caller holding the whole file in memory can classify the identical prefix through `isProbablyBinaryHeader` instead of reopening the file.
## [17.3.5] - 2026-08-16
### Fixed
+6 -2
View File
@@ -14,8 +14,12 @@
*/
import { peekFile, peekFileSync } from "./peek-file";
/** Header window sniffed for the binary heuristic; mirrors git's 8000-byte scan. */
const BINARY_SNIFF_BYTES = 8192;
/**
* Header window sniffed for the binary heuristic; mirrors git's 8000-byte scan.
* Exported so callers that already hold the whole file in memory can sniff the
* identical prefix through {@link isProbablyBinaryHeader} instead of reopening.
*/
export const BINARY_SNIFF_BYTES = 8192;
/**
* Classify an in-memory byte header as binary (non-UTF-8-text).