fix(parser): cache parse failures so broken files aren't re-read every run (#441 follow-up) (#453)
Some checks are pending
CI / semgrep (push) Waiting to run

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.
This commit is contained in:
Resham Joshi 2026-06-06 21:10:10 +02:00 committed by GitHub
parent e8009c4559
commit efa8593cc5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 68 additions and 11 deletions

View file

@ -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<string, number>()
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
}
}

View file

@ -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 = {

View file

@ -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 },