From efa8593cc5f0b80858eeb1471886a3a11868bf7d Mon Sep 17 00:00:00 2001 From: Resham Joshi <65915470+iamtoruk@users.noreply.github.com> Date: Sat, 6 Jun 2026 21:10:10 +0200 Subject: [PATCH] fix(parser): cache parse failures so broken files aren't re-read every run (#441 follow-up) (#453) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #450. When a session file throws during parse it was excluded but left uncached, so every refresh (~4x/min in the menubar) re-read and re-parsed it, and only the first failing file per provider was ever surfaced. - Add a negative-result marker: a failed file is cached as { fingerprint, turns: [], failed: true }. reconcileFile treats it as 'unchanged' at the same fingerprint, so it's skipped (no re-read) until the file changes. Empty turns => contributes no usage. - Warn per offending file (with its path), capped at 5 per provider per run, instead of once-per-provider — so a systemic break surfaces more than one file without flooding. Cached markers keep it quiet across refreshes. Tests: marker round-trips through save/load; reconcile stays 'unchanged' at the same fingerprint and re-parses when the file changes. --- src/parser.ts | 37 ++++++++++++++++++++++++++----------- src/session-cache.ts | 5 +++++ tests/session-cache.test.ts | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 11 deletions(-) diff --git a/src/parser.ts b/src/parser.ts index 55eb17d9..235b311e 100644 --- a/src/parser.ts +++ b/src/parser.ts @@ -1832,15 +1832,23 @@ function warnProviderReadFailureOnce(providerName: string, err: unknown): void { } } -function warnProviderParseFailureOnce(providerName: string, sourcePath: string, err: unknown): void { - const key = `${providerName}:parse-failure` - if (warnedProviderReadFailures.has(key)) return - warnedProviderReadFailures.add(key) +// Warn per offending file (so a systemic break surfaces more than one path), +// but cap per provider per run to avoid a flood. Cached failure markers mean a +// given broken file is only re-encountered when it changes, so this stays quiet +// across refreshes. +const parseFailureCounts = new Map() +const PARSE_FAILURE_WARN_CAP = 5 + +function warnProviderParseFailure(providerName: string, sourcePath: string, err: unknown): void { + const n = (parseFailureCounts.get(providerName) ?? 0) + 1 + parseFailureCounts.set(providerName, n) + if (n > PARSE_FAILURE_WARN_CAP) return const msg = err instanceof Error ? err.message : String(err) + const tail = n === PARSE_FAILURE_WARN_CAP + ? ` (further ${providerName} parse failures this run are suppressed)` + : '' process.stderr.write( - `codeburn: skipping ${providerName} session(s) that failed to parse (${msg}). ` + - `First offending file: ${sourcePath}. Further ${providerName} parse failures this run are suppressed; ` + - `other sessions still aggregate normally.\n` + `codeburn: skipped ${providerName} session that failed to parse: ${sourcePath} (${msg})${tail}\n` ) } @@ -1868,7 +1876,10 @@ async function parseProviderSources( const cached = section.files[source.path] const action = reconcileFile(fp, cached) - if (action.action === 'unchanged' && cached && !cachedFileNeedsProviderReparse(providerName, source.path, cached)) { + // A cached parse failure at this same fingerprint stays skipped — don't + // re-read a file that already threw and hasn't changed. It re-parses only + // when the file changes (then `reconcileFile` reports non-'unchanged'). + if (action.action === 'unchanged' && cached && (cached.failed || !cachedFileNeedsProviderReparse(providerName, source.path, cached))) { unchangedSources.push({ source, cached }) } else { changedSources.push({ source, fp }) @@ -1919,9 +1930,13 @@ async function parseProviderSources( } // A single malformed session file must not abort the entire run — that // would silently empty the daily-cache backfill and wipe the trend / - // history (issue #441). Skip just this file (its stale cache entry was - // already cleared above, so it's excluded) and keep going. - warnProviderParseFailureOnce(providerName, source.path, err) + // history (issue #441). Record a negative-result marker keyed by the + // current fingerprint so we don't re-read + re-throw this unchanged file + // on every refresh; it re-parses only if it changes. Empty turns => no + // usage contributed. + section.files[source.path] = { fingerprint: fp, mcpInventory: [], turns: [], failed: true } + ;(diskCache as { _dirty?: boolean })._dirty = true + warnProviderParseFailure(providerName, source.path, err) continue } } diff --git a/src/session-cache.ts b/src/session-cache.ts index 53f7a78c..df40f604 100644 --- a/src/session-cache.ts +++ b/src/session-cache.ts @@ -57,6 +57,11 @@ export type CachedFile = { canonicalProjectName?: string mcpInventory: string[] turns: CachedTurn[] + // Negative-result marker: this file threw while parsing at the recorded + // fingerprint. Cached so we don't re-read + re-throw it on every refresh; it + // is re-parsed only when the file changes (fingerprint differs). Carries no + // turns, so it contributes no usage. (issue #441 follow-up) + failed?: boolean } export type ProviderSection = { diff --git a/tests/session-cache.test.ts b/tests/session-cache.test.ts index 94d065b1..a8f4eeb4 100644 --- a/tests/session-cache.test.ts +++ b/tests/session-cache.test.ts @@ -112,6 +112,27 @@ describe('loadCache / saveCache', () => { expect(loaded).toEqual(cache) }) + it('persists a failed-parse marker across save/load (negative-result cache)', async () => { + const cache: SessionCache = { + version: CACHE_VERSION, + providers: { + pi: { + envFingerprint: 'abc123', + files: { + '/path/to/bad.jsonl': makeCachedFile({ turns: [], failed: true }), + }, + }, + }, + } + + await saveCache(cache) + const loaded = await loadCache() + // The `failed` flag and empty turns survive validation + load, so the file + // stays skipped on the next run instead of being re-read and re-thrown. + expect(loaded.providers['pi']?.files['/path/to/bad.jsonl']?.failed).toBe(true) + expect(loaded.providers['pi']?.files['/path/to/bad.jsonl']?.turns).toEqual([]) + }) + it('returns empty cache on version mismatch', async () => { const bad: SessionCache = { version: 999, providers: { claude: { envFingerprint: 'x', files: {} } } } await mkdir(TMP_DIR, { recursive: true }) @@ -261,6 +282,22 @@ describe('reconcileFile', () => { expect(reconcileFile(current, cached)).toEqual({ action: 'modified' }) }) + it('a failed marker at the same fingerprint stays "unchanged" (not re-parsed)', () => { + const fp: FileFingerprint = { dev: 1, ino: 100, mtimeMs: 1000, sizeBytes: 5000 } + const marker = makeCachedFile({ fingerprint: { ...fp }, turns: [], failed: true }) + expect(reconcileFile(fp, marker)).toEqual({ action: 'unchanged' }) + }) + + it('a failed marker is re-parsed once the file changes', () => { + const marker = makeCachedFile({ + fingerprint: { dev: 1, ino: 100, mtimeMs: 1000, sizeBytes: 5000 }, + turns: [], + failed: true, + }) + const changed: FileFingerprint = { dev: 1, ino: 100, mtimeMs: 2000, sizeBytes: 6000 } + expect(reconcileFile(changed, marker)).toEqual({ action: 'modified' }) + }) + it('returns "modified" when size shrank', () => { const cached = makeCachedFile({ fingerprint: { dev: 1, ino: 100, mtimeMs: 1000, sizeBytes: 5000 },