Skip to content

perf(history): limit parsing to recent entries #247

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
404-Page-Found wants to merge 6 commits into
base: main
Choose a base branch
from
Open

perf(history): limit parsing to recent entries #247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6e855b9
perf(history): limit parsing to recent entries
404-Page-Found Jul 28, 2026
6c4e6a4
fix(history): preserve UTF-8 at chunk boundaries
404-Page-Found Jul 28, 2026
4d4f04f
test: verify loadEntries handles corrupted rows during partial scan
404-Page-Found Jul 28, 2026
2e3378c
fix: preserve UTF-8 characters when reading history entries backwards
404-Page-Found Jul 28, 2026
0a971b6
perf: defer Buffer.concat in loadEntries to reduce memory allocations
404-Page-Found Jul 28, 2026
4b1d68e
feat(history): add readHistoryChunk to handle short reads from filesy…
404-Page-Found Jul 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 89 additions & 20 deletions src/history/store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,25 @@
const HISTORY_LOCK_TIMEOUT_MS = HISTORY_LOCK_STALE_MS + 10_000;
const HISTORY_LOCK_HEARTBEAT_MS = 15_000;
const HISTORY_LOCK_TAKEOVER_SUFFIX = '.takeover';
const HISTORY_READ_CHUNK_SIZE = 64 * 1024;

/** Read a complete history chunk, retrying when the filesystem returns a short read. */
export async function readHistoryChunk(
history: Pick<FileHandle, 'read'>,
buffer: Buffer,
position: number,
bytesToRead: number,
): Promise<number> {
let bytesRead = 0;

while (bytesRead < bytesToRead) {
const result = await history.read(buffer, bytesRead, bytesToRead - bytesRead, position + bytesRead);
if (result.bytesRead === 0) break;
bytesRead += result.bytesRead;
}

return bytesRead;
}

function hasErrorCode(error: unknown, code: string): boolean {
return typeof error === 'object' && error !== null && 'code' in error && error.code === code;
Expand Down Expand Up @@ -206,29 +225,79 @@
}

/** Load recent valid history entries while warning once about corrupted JSONL rows. */
export async function loadEntries(limit = 200): Promise<CommitEntry[]> {

Check failure on line 228 in src/history/store.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 24 to the 15 allowed. 


See more on https://sonarcloud.io/project/issues?id=404-PF_commit-echo&issues=AZ-wcdqCtHObAKalajC5&open=AZ-wcdqCtHObAKalajC5&pullRequest=247
const historyPath = getHistoryPath();
if (!existsSync(historyPath)) return [];

const raw = await readFile(historyPath, 'utf-8');
const lines = raw
.split('\n')
.map((line, index) => ({ line, lineNumber: index + 1 }))
.filter(({ line }) => line.trim().length > 0)
.reverse();

const entries: CommitEntry[] = [];
const corruptedLineNumbers: number[] = [];
const corruptedLineNumbers: Array<number | undefined> = [];
const history = await open(historyPath, 'r');

for (const { line, lineNumber } of lines) {
try {
const entry = JSON.parse(line) as CommitEntry;
if (entries.length < limit) {
entries.push(entry);
try {
const { size } = await history.stat();
let position = size;
let remainderChunks: Buffer[] = [];
let processedLineCount = 0;
let totalLineCount: number | undefined;
const corruptedLineIndexesFromEnd: number[] = [];

const parseLine = (lineBytes: Buffer) => {
const lineIndexFromEnd = processedLineCount++;
const line = lineBytes.toString('utf-8');
if (line.trim().length === 0) return;

try {
entries.push(JSON.parse(line) as CommitEntry);
} catch {
corruptedLineIndexesFromEnd.push(lineIndexFromEnd);
}
} catch {
corruptedLineNumbers.push(lineNumber);
};

while (position > 0 && entries.length < limit) {
const chunkEnd = position;
position = Math.max(0, chunkEnd - HISTORY_READ_CHUNK_SIZE - 3);
const bytesToRead = chunkEnd - position;
const buffer = Buffer.allocUnsafe(bytesToRead);
const bytesRead = await readHistoryChunk(history, buffer, position, bytesToRead);
const chunk = buffer.subarray(0, bytesRead);
Comment on lines +258 to +262

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Retry short history reads before moving backward

When FileHandle.read() returns fewer bytes than requested, as permitted on some network or FUSE filesystems, position has already moved backward by the full bytesToRead, while only bytesRead bytes are retained. The unread suffix is then permanently skipped, which can merge JSONL fragments, discard otherwise valid recent entries, or report them as corrupted. Continue reading until the requested interval is filled, or update the cursor based on the bytes actually read.

Useful? React with 👍 / 👎.

const firstLineEnd = chunk.indexOf(0x0a);
if (firstLineEnd === -1) {
remainderChunks.unshift(chunk);
continue;
}

const combined = remainderChunks.length === 0 ? chunk : Buffer.concat([chunk, ...remainderChunks]);
remainderChunks = [combined.subarray(0, firstLineEnd)];
const lines: Buffer[] = [];
let lineStart = firstLineEnd + 1;
for (let index = lineStart; index < combined.length; index++) {
if (combined[index] === 0x0a) {
lines.push(combined.subarray(lineStart, index));
lineStart = index + 1;
}
}
lines.push(combined.subarray(lineStart));
const processedBeforeChunk = processedLineCount;

if (position === 0) {
totalLineCount = processedBeforeChunk + lines.length + 1;
}

for (let index = lines.length - 1; index >= 0 && entries.length < limit; index--) {
parseLine(lines[index]!);
}
}

if (position === 0 && entries.length < limit) {
const remainder = remainderChunks.length === 1 ? remainderChunks[0]! : Buffer.concat(remainderChunks);
parseLine(remainder);
}

for (const lineIndexFromEnd of corruptedLineIndexesFromEnd) {
corruptedLineNumbers.push(totalLineCount === undefined ? undefined : totalLineCount - lineIndexFromEnd);
}
} finally {
await history.close();
}

warnCorruptedHistory(historyPath, corruptedLineNumbers);
Expand All @@ -237,20 +306,20 @@
}

/** Emit a compact warning that identifies corrupted history line numbers. */
function warnCorruptedHistory(historyPath: string, corruptedLineNumbers: number[]): void {
function warnCorruptedHistory(historyPath: string, corruptedLineNumbers: Array<number | undefined>): void {
if (corruptedLineNumbers.length === 0) return;

const count = corruptedLineNumbers.length;
const noun = count === 1 ? 'entry' : 'entries';
const lines = corruptedLineNumbers
const knownLineNumbers = corruptedLineNumbers.filter((lineNumber): lineNumber is number => lineNumber !== undefined);
const lines = knownLineNumbers
.slice(0, 5)
.sort((a, b) => a - b)
.join(', ');
const suffix = count > 5 ? `, +${count - 5} more` : '';
const location = knownLineNumbers.length === count ? `line ${lines}${suffix}` : 'recently scanned rows';

console.warn(
`Warning: ignored ${count} corrupted commit history ${noun} in ${historyPath} (line ${lines}${suffix}).`,
);
console.warn(`Warning: ignored ${count} corrupted commit history ${noun} in ${historyPath} (${location}).`);
}

/** Count raw history rows in the configured history file. */
Expand Down
93 changes: 92 additions & 1 deletion tests/history-corruption.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { join } from 'node:path';
import { tmpdir } from 'node:os';

import { getConfigDir } from '../dist/config/store.js';
import { countEntries, loadEntries } from '../dist/history/store.js';
import { countEntries, loadEntries, readHistoryChunk } from '../dist/history/store.js';

function writeHistory(lines) {
const configDir = getConfigDir();
Expand All @@ -31,6 +31,35 @@ function validEntry(message, timestamp) {
});
}

test('readHistoryChunk retries short reads from the unread offset', async () => {
const source = Buffer.from('history entry with a short read');
const destination = Buffer.alloc(source.length);
const reads = [];
const history = {
async read(buffer, offset, length, position) {
reads.push({ offset, length, position });
const bytesToCopy = Math.min(5, length);
source.copy(buffer, offset, position, position + bytesToCopy);
return { bytesRead: bytesToCopy, buffer };
},
};

assert.equal(await readHistoryChunk(history, destination, 0, source.length), source.length);
assert.deepEqual(destination, source);
assert.deepEqual(
reads.map(({ offset, position }) => ({ offset, position })),
[
{ offset: 0, position: 0 },
{ offset: 5, position: 5 },
{ offset: 10, position: 10 },
{ offset: 15, position: 15 },
{ offset: 20, position: 20 },
{ offset: 25, position: 25 },
{ offset: 30, position: 30 },
],
);
});

async function withIsolatedHistory(lines, assertion) {
const originalHome = process.env.HOME;
const originalAppData = process.env.APPDATA;
Expand Down Expand Up @@ -137,6 +166,40 @@ test('loadEntries warns and returns empty entries when all lines are corrupted',
});
});

test('loadEntries stops parsing once it has enough recent valid entries', async () => {
await withIsolatedHistory(
[
'{invalid old line',
validEntry('fix: keep older valid entry', '2026-06-01T00:00:00Z'),
validEntry('feat: keep newest valid entry', '2026-06-01T00:00:01Z'),
],
async ({ warnings }) => {
const entries = await loadEntries(1);

assert.deepEqual(entries.map((entry) => entry.message), ['feat: keep newest valid entry']);
assert.deepEqual(warnings, []);
},
);
});
Comment on lines +169 to +183

@coderabbitai coderabbitai Bot Jul 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider covering the "recently scanned rows" fallback path.

This test only exercises the case where a corrupted row is skipped entirely because it's never scanned (stop happens before reaching it). It doesn't cover the case where a corrupted row is scanned but the read stops before reaching the physical start of the file (a real multi-chunk history), which is when totalLineCount stays undefined and warnCorruptedHistory should fall back to the generic "recently scanned rows" wording. If that scenario isn't covered elsewhere in this file, consider adding a fixture large enough to span multiple 64KB chunks with a corrupted row inside the scanned range.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/history-corruption.test.mjs` around lines 140 - 154, Add coverage in
the loadEntries test suite for the recently scanned rows fallback: create a
history fixture larger than one 64KB chunk, place a corrupted row within the
portion scanned before loadEntries(1) stops, and assert warnCorruptedHistory
emits the generic recently scanned rows wording when totalLineCount remains
undefined. Keep the existing assertions for returned entries and warning
behavior where applicable.


test('loadEntries uses a generic location for corrupted rows in a partial scan', async () => {
const olderEntries = Array.from({ length: 700 }, (_, index) =>
validEntry(`chore: keep older entry ${index}`, `2026-06-01T00:00:${String(index).padStart(2, '0')}Z`),
);

await withIsolatedHistory(
[...olderEntries, validEntry('fix: keep newest valid entry', '2026-06-01T00:01:00Z'), '{invalid newest line'],
async ({ warnings }) => {
const entries = await loadEntries(1);

assert.deepEqual(entries.map((entry) => entry.message), ['fix: keep newest valid entry']);
assert.equal(warnings.length, 1);
assert.match(warnings[0], /ignored 1 corrupted commit history entry/);
assert.match(warnings[0], /recently scanned rows/);
},
);
});

test('countEntries counts raw non-empty history rows, including malformed JSON lines', async () => {
await withIsolatedHistory(
[
Expand All @@ -151,3 +214,31 @@ test('countEntries counts raw non-empty history rows, including malformed JSON l
},
);
});

test('loadEntries preserves UTF-8 characters split across backward-read chunks', async () => {
const prefix = '{"timestamp":"2026-06-01T00:00:00Z","message":"a';
const suffix = '","diff":"","model":"test-model","provider":"local"}';
const messageTailLength = 2 * 1024 * 1024 - Buffer.byteLength(suffix) - 1;
const row = `${prefix}\u00e9x\u00e9${'b'.repeat(messageTailLength)}${suffix}`;

const originalConcat = Buffer.concat;
let concatCalls = 0;
Buffer.concat = (...args) => {
concatCalls += 1;
return originalConcat(...args);
};

await withIsolatedHistory([row], async () => {
try {
const entries = await loadEntries(1);

assert.equal(entries.length, 1);
assert.equal(entries[0].message, `a\u00e9x\u00e9${'b'.repeat(messageTailLength)}`);
assert.doesNotMatch(entries[0].message, /\uFFFD/);
} finally {
Buffer.concat = originalConcat;
}
});

assert.equal(concatCalls, 1);
});