Skip to content

fix(parser): tolerate string message content; isolate per-file parse failures (#441)#450

Merged
iamtoruk merged 1 commit into
mainfrom
fix/parser-string-content-resilience
Jun 6, 2026
Merged

fix(parser): tolerate string message content; isolate per-file parse failures (#441)#450
iamtoruk merged 1 commit into
mainfrom
fix/parser-string-content-resilience

Conversation

@iamtoruk
Copy link
Copy Markdown
Member

@iamtoruk iamtoruk commented Jun 6, 2026

Fixes #441. Likely also fixes #425 (previous-day-always-0) for the throwing-file cause.

Root cause (as diagnosed by @jacobk — excellent report)

Some agents write a message's content as a string instead of an array of content blocks. Parsers did (msg.content ?? []).filter(...); ?? [] only guards null/undefined, so a string reaches .filterTypeError. Because the 365-day daily-cache backfill swallows parse errors silently, one malformed session wiped the entire menubar trend/history (only "today" remained).

Fix — two layers

1. Correctness — stop the crash at the source
New src/content-utils.ts normalizeContentBlocks(content): array → returned by reference (no copy on the happy path; also drops null/undefined elements so the same crash can't recur one level down), string → [{type:'text', text}], anything else → []. Applied at every (content ?? []).filter/.some site: pi.ts, codex.ts, droid.ts, cursor-agent.ts, and the Claude path in parser.ts.

2. Resilience — one bad file must not disable the whole feature

  • parseProviderSources now skips a file that fails to parse (warn once per provider) instead of re-throwing and aborting the backfill. The stale cache entry was already cleared, so the file is cleanly excluded; the rest of the backfill completes.
  • hydrateCache now surfaces backfill failures on stderr instead of silently returning an empty cache.

Why both layers

Layer 1 fixes the known Pi crash. Layer 2 ensures any future schema drift in any provider degrades to "skip that one file," not "no history at all."

Validation

  • Multi-agent review (correctness + adversarial). The adversarial pass found that arrays containing null elements would still crash one level down — fixed by sanitizing array elements in the helper (added a regression test).
  • New tests: content-utils.test.ts (array/string/null/undefined/null-element/same-reference) + a Pi string-content regression test.
  • Full suite: 1057 tests pass. tsc --noEmit clean.

Known follow-ups (not blocking, noted for transparency)

  • A permanently-unparseable file is re-read every refresh (no negative-result cache) — bounded per file, not compounding.
  • The per-provider warn shows only the first offending path; many failures of one provider surface as one line.

…failures (#441)

Some agents (Pi, and others for injected turns) write a message's `content`
as a string instead of an array of blocks. Parsers did `(content ?? []).filter`,
which throws on a string — and because the daily-cache backfill swallowed parse
errors, one bad session silently wiped the entire trend/history.

- Add normalizeContentBlocks(): string -> one text block, array -> as-is (by
  reference; drops null/undefined elements so the same crash can't happen one
  level down), else -> []. Applied across pi/codex/droid/cursor-agent and the
  Claude path in parser.ts.
- Isolate per-file parse failures in parseProviderSources: skip the offending
  file (warn once per provider) instead of re-throwing and aborting the whole
  backfill. The stale cache entry is already cleared, so the file is excluded.
- Surface backfill failures in hydrateCache via stderr instead of silently
  returning an empty cache.

Likely fixes #425 (previous-day always 0) for the throwing-file cause.
Tests: content-utils unit tests + a Pi string-content regression test.
@iamtoruk iamtoruk merged commit e8009c4 into main Jun 6, 2026
3 checks passed
iamtoruk added a commit that referenced this pull request Jun 6, 2026
…y run (#441 follow-up) (#453)

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Menubar trend chart / daily history is empty [Bug] Previous day usage always reported as 0

1 participant