daily-cache: never lose history the sources can no longer re-derive (v14)#755
Conversation
…v14) Session files are ephemeral (Claude Code deletes transcripts after ~30 days), so a cached day whose sources are gone exists nowhere else. Every invalidation path (schema bump, savings-config change, timezone change, incomplete-hydration retry) used to discard all cached days and re-derive from surviving sources, silently truncating history to the source retention window. Five bumps between June 22 and July 16 erased everything before April 24 on a machine with usage since March. Invalidations now re-derive what they can and carry forward every (day, provider) slice they cannot, marked 'carried'. Loading a missing or corrupt cache file adopts days from every older daily-cache file in the cache dir (legacy, versioned, .bak copies, orphaned .tmp) as a union per (day, provider), higher schema version winning per pair. Provider slices now store the full per-provider breakdown (tokens, models, categories, sessions) so carry-forwards stay exact across future rebuilds. Merge rules hardened by adversarial review: opaque pre-v5 days (totals without provider slices) merge all-or-nothing so partial parses cannot double-count into them; zero-data placeholder slices neither block nor lose carried data (session counts deduplicated by max); a partial parse never overwrites finalized baseline slices, only fills gaps; adoption purges today/future entries, applies retention, and clamps a stale lastComputedDate so a purged day cannot be skipped forever. Verified end-to-end against a copy of a real cache dir: the rebuilt v14 cache holds every (day, provider) pair present in any older cache file, including Claude days from early April that the July rebuilds had dropped.
|
Added per-project daily rollups (v15, d2216d6): days and provider slices now carry a projects breakdown (cost/calls/savings/sessions per project), so project history survives source-file deletion like models and categories already do. First schema bump to ride the v14 carry-forward; verified lossless on a real cache dir (identical totals, zero lost pairs, project splits on all 36 derivable days). Two adversarial review rounds on the increment; findings fixed with regression tests, including a real prototype-pollution path via foreign cache keys named proto. Final verdict SHIP. |
Project history previously lived only in the session layer, so it faded with the source files even though day totals now survive. Days and provider slices carry a projects breakdown (cost/calls/savings/sessions per project) filled by the day aggregator and folded through carry merges, making the By Project dimension as durable as models and categories. Days recorded before v15 keep their totals with no project split; nothing can reconstruct one once sources are gone. This is the first bump to ride the v14 carry-forward: the v14 cache is adopted losslessly and only source-backed days re-derive (verified on a real cache dir: identical totals, zero lost pairs, project splits on every derivable day). Hardening from adversarial review: placeholder-aware project session dedup so totals reconcile; migrateDays sanitizes provider slices and nested projects from foreign caches; all foreign-keyed map access in the merge path uses hasOwn reads and defineProperty writes, closing a real prototype-pollution path a regression test caught when a cache key is named __proto__.
d2216d6 to
b287614
Compare
avs-io
left a comment
There was a problem hiding this comment.
There is one blocking integration gap in the final menubar payload path.
buildPeriodDataFromDays(allDays, ...) correctly builds cache-backed historical totals at src/usage-aggregator.ts:275, but the all-provider path immediately replaces that result at line 291 with buildPeriodData(...scanProjects). scanProjects contains only sessions whose source files still survive. The new project rollups are similarly not surfaced: buildPeriodDataFromDays() never folds DailyEntry.projects into PeriodData.projects, and lines 411-431 rebuild currentData.projects solely from scanProjects.
I reproduced this on the exact PR head (b2876145) with an empty source directory and a v15 cache containing one carried $100 historical day/project:
history.dailycontained the $100 day (the new cache logic worked)current.costwas $0current.callswas 0current.topProjectswas empty
This means the cache artifact is repaired, but the user-visible 6-month/all-time headline totals and By Project view described in the PR remain truncated once source files disappear.
Suggested implementation shape:
- Keep the
buildPeriodDataFromDays(allDays, ...)result as the authority for cache-backed totals instead of replacing it wholesale. - Merge only fields that genuinely require the surviving-session parse, such as
estimatedCostUSD,unpricedModels, efficiency, and session-detail metadata. - Fold
DailyEntry.projectsacross the selected cached days intoPeriodData.projects, then merge today's live project rollups. Historical projects can omitsessionDetailswhen the source sessions no longer exist. - Add an end-to-end
buildMenubarPayloadForRange()regression test that seeds a populated current-version daily cache, points discovery at an empty source directory, and asserts that headline cost/calls andtopProjectsmatch the carried cache data—not onlyhistory.daily.
The cache carry-forward implementation and focused tests otherwise look thoughtful. I ran the 70 affected daily-cache/day-aggregator tests, tsc --noEmit, and git diff --check; all passed. The missing final-payload coverage is the blocker.
Review finding on this PR: the all-provider headline built cache-backed totals and then replaced them wholesale with a rebuild from the surviving-session parse, so current.cost/calls, the models table, and topProjects stayed truncated to the source-retention window even though history.daily carried the full record. The replacement existed only to keep the estimated-cost markers alive. The cache-backed period data is now the authority; the scan contributes exactly what day entries lack: estimated-cost markers, unpriced-model detection, per-session drill-downs, and a fresher project path. Project totals come from the same day set as the headline, with ProjectDayStats gaining a path so carried-only projects still display a friendly name. Sessions merge by max: the cache buckets a session on its start day, the scan counts it on any active day, and both undercount differently. End-to-end regression test seeds a cache whose only day is carried (no session files exist) and asserts the headline, models, and topProjects all reflect it. Verified on a real cache dir: the 6-month headline now equals the history sum to the cent.
|
Confirmed the finding and fixed it in 41209d9. You were right: line 291 replaced the cache-backed period data wholesale, so the headline, models table, and topProjects were all truncated to surviving sessions; my earlier end-to-end check asserted history.daily and the provider panel but never current.cost. Fix follows your suggested shape: buildPeriodDataFromDays(allDays) is now the authority on the unscoped all-provider path; the scan merges in only estimatedCostUSD, unpricedModels, per-model estimated markers, sessionDetails, and project paths. Project totals fold DailyEntry.projects over the same day set as the headline (ProjectDayStats gained a stored path so carried-only projects keep a friendly display name). Sessions merge by max since start-day bucketing and active-day counting undercount differently; a --day filter regression surfaced exactly that and is covered. Your regression test shape is implemented in tests/menubar-carried-headline.test.ts: seeded current-version cache with a carried day, empty HOME, asserts current.cost/calls/sessions, topModels, and topProjects against the carried data. Verified on a real cache dir: the 6-month headline now equals the history.daily sum to the cent, including the rescued April days. Known honest gap, documented in-code: days recorded before v15 have totals but no project split, so the projects list can sum below the headline. |
…ization v15 sanitizes provider slices and projects at ingestion and guards merge lookups with Object.hasOwn/setOwn. Three residual gaps versus the invariant (no JSON-parseable cache content may throw or produce NaN/garbage), plus one found in review: - Day-level models/categories were not sanitized: a day with models:'bad' survived load and buildPeriodDataFromDays iterated the string per-character, yielding NaN in period model totals. Day-level maps now reuse the same sanitizers as slice-level maps. - Map keys shadowing Object.prototype (constructor, toString, ...) still entered the cache from foreign files; modelTotals[name] ?? init in buildPeriodDataFromDays resolves such a key to the inherited prototype member and produces NaN. All sanitized maps now drop these keys at ingestion (deliberate tradeoff: such names are reclassified invalid). - Top-level lastComputedDate was unvalidated; a non-string later hit .slice() in the gap-start parse and threw. Kept only when it is a YYYY-MM-DD string, else null (forces a plain re-backfill). - Stale savingsConfigHash comment still described pre-v14 discard behavior; reworded to re-derive + carry forward. 3 regression tests, each verified to fail on the pre-fix code. Relevant suites 71/71 green, tsc clean. Valid-input behavior unchanged except the prototype-key reclassification above.
|
Pushed cba064f: closes the residual corrupt-input gaps in the v15 ingestion sanitization, found during an adversarial review of this PR. What it covers:
Verification: 3 regression tests, each verified to fail on the pre-fix code; relevant suites 73/73 green incl. the new menubar-carried-headline tests; tsc clean; an independent corrupt-cache battery (null slices, string providers maps, non-numeric fields, prototype keys, malformed lastComputedDate) passes on load + hydration + period aggregation. Review findings NOT addressed here, worth separate issues:
|
… headline The carry-forward headline (#755) serves totals from the durable day set and merges only session-derived fields from the scan. The new workflow, topReworkedFiles, and pricingCoverage fields were computed by the scan but never propagated, so they silently defaulted on the all-provider path (pricingCoverage rendered a dishonest 1.0). Propagate them like the other scan-only fields and pin it with a carried-headline regression test.
…t, file churn, coverage) + dash wire-up (#756) * feat(insights): workflow intelligence (corrections, time-to-first-edit, file churn, coverage) + dash wire-up * menubar: propagate workflow-intelligence fields onto the cache-backed headline The carry-forward headline (#755) serves totals from the durable day set and merges only session-derived fields from the scan. The new workflow, topReworkedFiles, and pricingCoverage fields were computed by the scan but never propagated, so they silently defaulted on the all-provider path (pricingCoverage rendered a dishonest 1.0). Propagate them like the other scan-only fields and pin it with a carried-headline regression test.
Problem
Long-term usage history was silently destroyed, repeatedly. A user with heavy spend since March saw a 6-month total that was roughly half of reality, days like Apr 15 and Apr 19 rendered as 0.00 with 0 calls despite hundreds of dollars of tracked spend at the time, and an all-time number that never grew past ~15k.
Root cause chain:
Proof from a real machine: the v9 cache backup (Jul 2) held Claude spend for Apr 26 through Jun 1 that the current v13 cache had zeroed. The all-time plateau is the sliding-window equilibrium of ~30 days of Claude plus older Codex files that never get deleted.
Fix
One invariant, enforced everywhere: an invalidation may re-derive a (day, provider) slice only if the parse actually produced data for it; everything else is carried forward.
ensureCacheHydratedrebuilds by per-(day, provider) merge instead of wipe. Carried slices keep the accounting of the version that recorded them and are markedcarriedfor provenance.Hardening from adversarial review (three review rounds, five real findings fixed, final verdict SHIP):
Verification
tests/daily-cache-carry-forward.test.ts): every invalidation trigger run against an empty re-parse must preserve the baseline; corrections must still win for re-derivable days; adoption union priority; the real Apr-26 rescue scenario as a fixture. Full suite green (3653 tests).Follow-ups (not in this PR)
parser.ts) to close the residual case where partially expired sources re-derive a day slightly lower at invalidation time (~0.1% observed).codeburn doctorwarning about Claude Code'scleanupPeriodDays.