Skip to content

Preserve spend totals while local history catches up - #3051

Draft
mauriciopolvora wants to merge 1 commit into
steipete:mainfrom
mauriciopolvora:fix-retained-spend-catch-up
Draft

Preserve spend totals while local history catches up#3051
mauriciopolvora wants to merge 1 commit into
steipete:mainfrom
mauriciopolvora:fix-retained-spend-catch-up

Conversation

@mauriciopolvora

Copy link
Copy Markdown

Summary

  • snapshot the last established local-cost report before a routine bounded refresh can enter catch-up
  • keep same-account, same-window established totals visible in the menu and Spend Dashboard while the new scan is incomplete
  • replace the retained value after convergence, while continuing to drop it for account changes, scope changes, invalidation, or confirmed-empty history

Root cause

An incomplete bounded refresh is a successful scan result, not a hard failure. It correctly carries historyCoverageIsEstablished == false, and the Spend Dashboard correctly excludes such rows from totals. However, that successful incomplete result replaced the previous established snapshot, so Codex disappeared from Usage & Spend while Cursor remained visible.

The existing retained-history path covered scanner failures, but not successful exact-to-pending transitions during routine refreshes.

Safety invariants

  • The live scanner and CLI still report coverage as incomplete while catch-up is pending.
  • A retained report must match the requested date window, time zone, and session-root fingerprint.
  • In-memory retention is limited to the current provider-config revision and cost-history scope.
  • Invalidated and confirmed-empty sources are never retained.
  • No external helper, network access, Keychain access, or real account data is added.

Validation

  • XCODE_DEFAULT_TOOLCHAIN_OVERRIDE=/Library/Developer/CommandLineTools make check — passed, 0 violations
  • focused Swift Testing run for CostUsageFetcherCacheSnapshotTests|SpendDashboardCachedPresentationTests|UsageStoreCodexCostCatchUpTests — 35 tests passed in 3 suites
  • make test — attempted, but this host has Command Line Tools only and the unmodified runner cannot discover tests without Xcode's Testing module; the focused suites were run with local framework paths and the pull-request CI uses the repository's Xcode 26.2/26.3 matrix

The regression fixture proves an exact scan, an appended active tail, a bounded pending refresh, an unchanged retained total, and an eventual complete replacement. Additional tests cover dashboard presentation and cross-scope rejection.

No screenshot is included because this changes state retention, not visual styling, and the available reproduction contains private account information.

Refs #2815, #2628, #2520, #2918.

@clawsweeper

clawsweeper Bot commented Aug 18, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

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

@mauriciopolvora
mauriciopolvora force-pushed the fix-retained-spend-catch-up branch from a597df3 to fa7c088 Compare August 18, 2026 15:44
@clawsweeper clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 18, 2026
@clawsweeper

clawsweeper Bot commented Aug 18, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed August 18, 2026, 11:48 AM ET / 15:48 UTC.

ClawSweeper review

What this changes

This PR preserves an established Codex local spend total in the menu and Spend Dashboard while a bounded history refresh is incomplete, then replaces it after convergence.

Merge readiness

Blocked until real behavior proof from a real setup is added - 4 items remain

Keep this draft PR open. Current main lacks the compatible established-snapshot retention proposed here, and the branch has focused coverage with no definite code defect found; it still needs after-fix real-behavior proof before merge.

Priority: P2
Reviewed head: fa7c0882fff072e8ad8b03f3cfbea11bfa272b15

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The implementation and focused regression coverage are solid, but merge-quality real behavior proof is still absent.
Proof confidence 🧂 unranked krab (1/6) Needs real behavior proof before merge: The PR body describes focused test results, but it contains no after-fix real-system output or media; the prepared images are from the earlier cached-spend PR, not this branch. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The PR body describes focused test results, but it contains no after-fix real-system output or media; the prepared images are from the earlier cached-spend PR, not this branch. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 5 items Current-main gap: The current-main version of the scanner only captured a previous report for an explicit rescan; this branch broadens capture to any compatible routine refresh, which is the central requested behavior.
Scope-preserving publication: The new publication guard retains only a current-provider-config snapshot; the existing publication lookup checks both provider-config revision and token-cost scope signature.
Regression fixture: The new fixture establishes a cache, appends an active tail, forces a bounded pending pass, and asserts that the cached retained total remains established while the scanner stays pending.
Findings None None.
Security None None.

Live Verification

Command: swift test --filter CostUsageFetcherCacheSnapshotTests

Result: FAIL (failed) — execution before step 1 run: sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.22.0.tgz

sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.22.0.tgz

Assertions:

  • FAIL expect_output: bounded tail refresh retains the prior established cached snapshot
  • FAIL expect_output: passed

How this fits together

CodexBar scans local Codex session files into cost snapshots for the menu and Spend Dashboard. Bounded scanning can temporarily leave history incomplete, so this change chooses a compatible established snapshot for presentation while catch-up continues.

flowchart LR
A[Local Codex session files] --> B[Bounded history scan]
B --> C{Coverage established?}
C -->|Yes| D[Current cost snapshot]
C -->|No| E[Compatible retained snapshot]
D --> F[Menu and Spend Dashboard]
E --> F
B --> G[Catch-up scheduler]
Loading

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The PR body describes focused test results, but it contains no after-fix real-system output or media; the prepared images are from the earlier cached-spend PR, not this branch. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Resolve merge risk (P1) - If catch-up pauses indefinitely, users continue to see the last established total rather than newly appended local usage; the retained presentation must remain clearly associated with the existing catch-up state.
  • Resolve merge risk (P1) - The PR body reports focused tests but provides no after-fix real-system trace, terminal output, log, recording, or current-head screenshot for this retention path.
  • Complete next step (P2) - The remaining blocker is contributor-supplied real behavior proof, not a narrow repair ClawSweeper can safely make on the branch.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Implementation and regression coverage production +41/-7, tests +157/-3 across 8 files The patch changes three presentation/scan layers and adds targeted coverage for cache, dashboard, and store publication behavior.

Merge-risk options

Maintainer options:

  1. Prove retained-state lifecycle before merge (recommended)
    Attach redacted after-fix evidence that shows an established total during pending catch-up and the replacement total after convergence.
  2. Accept stale-total exposure
    Merge with the documented trade-off that newly appended local usage can remain hidden until the bounded catch-up cycle finishes or pauses.

Technical review

Best possible solution:

Retain only same-scope established totals during pending scans, preserve the scanner’s incomplete coverage signal, and attach a redacted after-fix terminal or app trace that shows stable retained spend followed by convergence.

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

Yes—source fixtures establish a compatible cache, append an active local-session tail, impose a bounded scan budget, and verify retained presentation while coverage remains pending; this review did not execute the test.

Is this the best way to solve the issue?

Yes—the patch preserves the existing incomplete scanner signal while retaining only compatible established presentation state, rather than treating incomplete data as complete.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 01f8d603b917.

Labels

Label changes:

  • add P2: This is a bounded local spend-presentation bug with limited provider-specific blast radius.
  • add merge-risk: 🚨 compatibility: Merging changes what existing users see during pending history scans by retaining an older established total.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body describes focused test results, but it contains no after-fix real-system output or media; the prepared images are from the earlier cached-spend PR, not this branch. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P2: This is a bounded local spend-presentation bug with limited provider-specific blast radius.
  • merge-risk: 🚨 compatibility: Merging changes what existing users see during pending history scans by retaining an older established total.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body describes focused test results, but it contains no after-fix real-system output or media; the prepared images are from the earlier cached-spend PR, not this branch. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

Likely related people:

  • Peter Steinberger: Recent history includes retained-spend work and established-empty Codex-history fixes on the affected fetcher path. (role: recent cost-history contributor; confidence: high; commits: 698c332a250f, 5ca592ee1844; files: Sources/CodexBarCore/CostUsageFetcher.swift, Sources/CodexBar/SpendDashboardController.swift)
  • Yuxin Qiao: Added recent Spend Dashboard token-activity coverage and established-empty Codex history work adjacent to this retention flow. (role: recent dashboard and catch-up test contributor; confidence: high; commits: 68f26b5d65c9, aef668ecb3b8; files: Sources/CodexBar/SpendDashboardController.swift, Sources/CodexBarCore/CostUsageFetcher.swift)
  • Xu Xiang: Introduced the bounded Codex catch-up behavior that this PR extends at the scanner boundary. (role: bounded-scan contributor; confidence: high; commits: 2920019bc16d; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift)

Rank-up moves

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

  • Add a redacted terminal transcript or recording from this head showing retained spend during a pending scan and replacement after convergence.
  • Update the PR body with that evidence; if no fresh review appears, ask a maintainer to comment @clawsweeper re-review.

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.

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. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant