Skip to content

Preserve wider retained Codex cost history across dashboard projections - #2920

Merged
steipete merged 1 commit into
mainfrom
fix/2914-projection-retention
Aug 14, 2026
Merged

Preserve wider retained Codex cost history across dashboard projections#2920
steipete merged 1 commit into
mainfrom
fix/2914-projection-retention

Conversation

@steipete

Copy link
Copy Markdown
Owner

Summary

Usage & Spend requests a narrow 7- or 30-day reporting window while the cost cache may retain a wider configured history. Budget enforcement in the save path treated the requested reporting window as the retention boundary: under row/byte pressure, a narrow dashboard save pruned completed cached rows and daily aggregates outside the projection and rewrote the persisted scan coverage down to the narrow request. This matches the reporter evidence in #2914 of history collapsing to the last 1-2 days after updates.

This lands the fix from #2915 by @thomaschow19, rebased onto current main (post-#2918 catch-up cluster): saveCodexCache now derives a budget-protection window as the union of the cache's retained coverage (scanSinceKey/scanUntilKey) and the requested window, and passes that to enforceBudgets on both the identical-content and full-save paths. Projections stay read slices; retention stays owned by the persisted history window.

Proof

  • New synthetic regression (narrow dashboard windows preserve wider retained cache under byte pressure) fails on unfixed main with 11 issues (older row deleted, aggregates pruned, scanSinceDay narrowed) and passes with the fix, covering both the identical-content and full-save paths under a 1-byte file budget.
  • swift test --filter CostUsageStoreTests: 73 tests pass.
  • make check: 0 violations.
  • Codex autoreview: clean.

Fixes #2914
Supersedes #2915 (same fix, rebased and verified against current main). Thanks @thomaschow19!

Usage & Spend saves a 7- or 30-day requested reporting window while the
cache may retain a wider configured history. Budget enforcement treated
the requested window as the retention boundary, so a narrow dashboard
save under row/byte pressure pruned older retained rows and aggregates
and rewrote the persisted scan coverage down to the narrow request.

Derive the budget-protection window as the union of the cache's retained
coverage and the requested window, on both the identical-content and
full-save paths, and add a synthetic regression covering both.

Fixes #2914

Co-authored-by: CodexBar Contributor <codexbar-contributor@users.noreply.github.com>
@clawsweeper

clawsweeper Bot commented Aug 14, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@clawsweeper clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Aug 14, 2026
@clawsweeper

clawsweeper Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed August 13, 2026, 11:32 PM ET / August 14, 2026, 03:32 UTC.

ClawSweeper review

What this changes

The PR protects the cache’s full retained Codex cost-history window when a shorter Usage & Spend dashboard projection triggers budget enforcement.

Regression provenance

Possible regression — probable (reviewed change; known regression link). No predecessor PR is attributed.

Merge readiness

⚠️ Ready for maintainer review - 3 items remain

Keep this owner-authored PR open for normal merge review. The supplied diff is a focused repair for persisted Codex cost-cache retention and includes regression coverage for both save paths, but local source/history inspection could not run because the sandbox command failed before execution.

Priority: P2
Reviewed head: b4fde0d974fcd6e271541e15b61c94ccef5a0efd

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The patch and regression are well targeted in the supplied context, but local independent inspection was unavailable.
Proof confidence 🌊 off-meta tidepool Not applicable: This repository-owner PR provides a synthetic regression and focused validation claim; the external-contributor real-behavior-proof gate does not apply.
Patch quality 🦐 gold shrimp (3/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: This repository-owner PR provides a synthetic regression and focused validation claim; the external-contributor real-behavior-proof gate does not apply.
Evidence reviewed 4 items Focused persistence change: The supplied PR diff derives a protection window from retained cache coverage plus the requested window, then uses it in both identical-content and full-save budget enforcement paths.
Regression coverage: The supplied PR adds a synthetic byte-pressure regression that exercises narrow dashboard windows through identical-content and full-save persistence paths.
Canonical bug report: The linked report describes the same loss of older cached rows, daily aggregates, and persisted scan coverage: #2914.
Findings None None.
Security None None.

How this fits together

Codex usage scanning persists a local cost cache that feeds Usage & Spend dashboard projections. A projection save enforces cache budgets and updates persisted rows, aggregates, and scan coverage used by subsequent dashboard reads.

flowchart LR
  A[Codex usage files] --> B[Cost cache]
  C[Dashboard date range] --> D[Cache save]
  B --> D
  D --> E[Budget enforcement]
  E --> F[Persisted rows and aggregates]
  F --> G[Usage and Spend dashboard]
Loading

Before merge

  • Resolve merge risk (P1) - This changes persisted cache-retention semantics; macOS test completion should confirm both save paths preserve retained coverage while budgets remain best-effort.
  • Resolve merge risk (P1) - Independent local source and git-history verification was unavailable because the read-only sandbox failed before commands executed.
  • Complete next step (P2) - This owner-authored PR needs normal merge review and pending validation rather than an automated repair lane.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch surface 3 files affected: production +22/-4, tests +68, release notes +3 The patch is narrowly scoped to persisted cache budgeting and its regression coverage.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #2914
Summary: This PR is the candidate repair for the linked retained-history pruning report.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

  1. Complete retained-window validation (recommended)
    Confirm the focused cache-store regression and pending macOS checks preserve retained scan coverage in both save paths before merge.

Technical review

Best possible solution:

Retain the persisted cache coverage as the budget-protection boundary, preserve the focused regression, and merge after the pending validation confirms the intended compatibility behavior.

Do we have a high-confidence way to reproduce the issue?

Yes, the supplied regression design gives a concrete byte-pressure reproduction through both cache-save paths, though independent local execution was blocked by the sandbox failure.

Is this the best way to solve the issue?

Yes, based on the supplied diff: separating retained cache coverage from a read projection is the narrowest way to prevent projection saves from redefining retention.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 3a848565bcd2.

Labels

Label changes:

  • add P2: The PR addresses loss of retained local cost-history data under cache-budget pressure.
  • add merge-risk: 🚨 compatibility: It changes how existing persisted cache coverage is protected during dashboard saves.
  • add merge-risk: 🚨 session-state: It changes persistence of cache rows, aggregates, and scan-range state.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This repository-owner PR provides a synthetic regression and focused validation claim; the external-contributor real-behavior-proof gate does not apply.

Label justifications:

  • P2: The PR addresses loss of retained local cost-history data under cache-budget pressure.
  • merge-risk: 🚨 compatibility: It changes how existing persisted cache coverage is protected during dashboard saves.
  • merge-risk: 🚨 session-state: It changes persistence of cache rows, aggregates, and scan-range state.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This repository-owner PR provides a synthetic regression and focused validation claim; the external-contributor real-behavior-proof gate does not apply.

Evidence

What I checked:

Likely related people:

  • steipete: The supplied PR metadata identifies steipete as the repository owner and author of the focused current implementation. (role: current implementation author; confidence: medium; commits: b4fde0d974fc; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageStore+CodexCache.swift, Tests/CodexBarTests/CostUsageStoreTests.swift)
  • thomaschow19: The PR and linked draft identify thomaschow19 as the contributor of the preceding version of this same retained-history repair. (role: prior implementation contributor; confidence: medium; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageStore+CodexCache.swift, Tests/CodexBarTests/CostUsageStoreTests.swift)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Let the pending macOS test checks complete and retain the focused byte-pressure regression as merge evidence.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete
steipete merged commit dc4168c into main Aug 14, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Usage & Spend projections can prune wider retained Codex cost history

1 participant