daily-cache: fix tz-change double-count and multi-provider turn-stat inflation#762
Conversation
…tribution P2: a tz-change re-derive no longer double-counts turns whose day bucket moved across midnight — secondary slices are carried only outside the provider's fresh coverage (floor widened by one day on tzChanged). P3: a turn's editTurns/oneShotTurns/category counts are attributed to the turn's primary provider slice only, so slice sums equal day-level counts and partial carries cannot inflate day totals.
…cover empty-call turns Review addendum: the one-day tz widening assumes an offset delta of at most 24h (date-line crossings are a known limit); day-level projects of skinny kept slices are dropped by the coverage rebuild by design. Adds a regression test pinning the existing skip of empty-call turns.
The P2 coverage floor is reverted pending redesign: on a forced tz re-derive against a real cache copy it dropped a genuine, non-duplicated day (claude 2026-06-18, 399.70 USD, 1572 calls) that exists nowhere else, and the floor also applied to every non-tz re-derive where the same class of drop is latent. Trading a bounded midnight double-count for unbounded history loss inverts the never-lose priority; the surgical alternative (drop only the migrated turn's old-day contribution) is tracked separately. P3 stands as authored: turn-level stats attribute to exactly one slice (majority-calls provider, tie broken by first call), restoring slice-sums equal day-counts on fresh builds, with its regression tests.
|
Ran the full adversarial verification before merge (worktree merge with current main, suite, mutation checks on both fixes, and forced re-derive drives against a copy of a real 15k USD cache). Verdict: P3 is sound and ships; P2's coverage floor is reverted pending redesign. P3: majority-calls attribution restores slice-sums equal day-counts, lands cleanly on the hardened addSliceIntoDay (hasOwn/setOwn/placeholder dedup intact), and your three attribution tests catch mutations. Merged as authored, including the updated day-aggregator assertion. P2: the intent is right and the de-dup works: on a forced Berlin-to-NY re-derive, main double-counts about 117 USD across codex/hermes/grok midnight turns and your floor correctly eliminates every one of them. But the same run drops claude 2026-06-18 entirely: 399.70 USD, 1,572 calls, present in the pristine baseline, non-duplicated, gone from the merged result because the widened floor cannot distinguish a midnight-migrated turn from a source-gone day. The floor also engages on every non-tz complete-parse re-derive (ensureCacheHydrated always passes the policy; only widenByOneDay is tz-gated), where a mid-range source hole gets its carried slice silently blocked; the savings-hash test does not exercise that hole (its extra baseline day sits before the oldest fresh day, so it passes with or without the floor). Trading a bounded over-count for unbounded history loss inverts the never-lose priority, so the floor cannot ship as designed. The surgical alternative, subtracting only the migrated turns' old-day contribution, is tracked in #770 with the full evidence; your implementation and tests remain in this PR's history (e07f324) for that work. The tz double-count stays a documented, accepted edge until then. |
Follow-up to the #755 review (P2 and P3 there, deferred as a separate PR).
P2: timezone-change double count
On a tzChanged re-derive the fresh parse re-buckets turns under the new timezone; a turn adjacent to midnight moves from day D to D+1. The merge then carried the old (D, provider) slice because fresh day D lacked that provider, so the turn counted on both D (carried) and D+1 (fresh).
Fix: an opt-in coverage policy on the re-derive merge. A secondary slice (D, P) is carried only before provider P's oldest fresh day; on tzChanged the floor widens by one day (tz re-bucketing moves a bucket by at most one calendar day; the 26h date-line delta is a documented, accepted limit). Providers with no fresh data keep full carry, sessions-only placeholders keep their behavior, and adoption plus partial-parse merges are untouched. Secondary-only days that straddle the floor are rebuilt from eligible slices so blocked totals cannot leak through day fields.
P3: multi-provider turn stats inflated on partial carry
aggregateProjectsIntoDays attributed editTurns/oneShotTurns/category turn counts in full to every provider present in a turn, while day level counted once; a partial carry then re-added the carried slice's counts into day totals.
Fix: a turn's counts attribute to exactly one slice, the majority-calls provider (tie broken by the first call's provider), so slice sums equal day-level counts on fresh builds. Per-provider cost attribution unchanged. Legacy caches with duplicated counts load unchanged (documented; merges of historical slices may still overstate).
Tests
Nine new regression tests (tz migration, genuine carry survival, plain floor on savings-hash re-derives, placeholder retention, majority/tie attribution, partial-carry non-inflation, legacy loadability, empty-call turn skip). One existing assertion updated: the non-primary provider's category slice no longer counts the shared turn (tests/day-aggregator.test.ts). Root suite 2058+ passing, tsc clean.