Skip to content

perf: avoid reprocessing stable Codex cost cache - #2834

Merged
steipete merged 1 commit into
mainfrom
steipete/codexbar-cpu-optimization-a10b72
Aug 10, 2026
Merged

perf: avoid reprocessing stable Codex cost cache#2834
steipete merged 1 commit into
mainfrom
steipete/codexbar-cpu-optimization-a10b72

Conversation

@steipete

Copy link
Copy Markdown
Owner

Summary

Stable Codex cost-cache saves were converting every unchanged historical token snapshot and encoding every row before deciding whether persistence could reuse, append, or replace the existing data.

This change plans persistence first: stable collections are reused without materialization, safe appends materialize only the suffix using absolute indexes, and the cache is fully replaced whenever append safety is lost.

Performance

  • The installed 0.49.0 production trace recorded 8,993 ms of sampled CPU in 45 seconds. The saveCodexCache branch accounted for 4,085 ms and CostUsageTimestampParser.parseISO for 3,307 ms.
  • The copied real-world cache contained 168,047 token snapshots and 126,227 rows.
  • Patched stable saves averaged 0.98 seconds over 35 runs, compared with 4.09 seconds for the traced pre-fix save branch.
  • A corrected 30-second post-fix Time Profiler trace contained zero samples in CostUsageTimestampParser or parseISO.

Validation

  • Focused regression suite: 87 tests across CostUsageStoreTests, CodexForkAppendResumeTests, and CostUsagePerformanceGateTests passed.
  • DEVELOPER_DIR=/Applications/Xcode-beta.app/Contents/Developer make check passed.
  • git diff --check passed.
  • Pre-commit autoreview found no accepted or actionable findings.
  • The local full make test ran 834 selections and failed only the pre-existing unrelated MenuSwitchFlickerProbeTests expectation that its synthetic 400 ms probe log contains handled=true. The same expectation fails unchanged in isolation under the machine's extreme load.

@clawsweeper

clawsweeper Bot commented Aug 10, 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 P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Aug 10, 2026
@clawsweeper

clawsweeper Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed August 10, 2026, 3:26 AM ET / 07:26 UTC.

ClawSweeper review

What this changes

The PR plans Codex cost-cache persistence before conversion so stable records are reused and safe appends encode only new snapshots and rows.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

Keep open for normal owner review and CI completion. The current main implementation still materializes every snapshot and row before reuse is decided; this PR replaces that work with equivalent reuse, append, and replacement branches, and no blocking correctness defect was found.

Priority: P2
Reviewed head: 422c4f35c9b11b5e34238eafdae3bb64ac5a6694

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The focused implementation and coverage are solid, with real-cache profiling evidence reported in the PR body and no correctness findings.
Proof confidence 🌊 off-meta tidepool Not applicable: This owner-authored internal performance PR is not subject to the external contributor proof gate; its body nevertheless provides after-fix profiler and real-cache timing evidence.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: This owner-authored internal performance PR is not subject to the external contributor proof gate; its body nevertheless provides after-fix profiler and real-cache timing evidence.
Evidence reviewed 4 items Current-main performance path: Current main converts every token snapshot and encodes every usage row before checking whether the existing stored records can be reused, so the reported repeated work remains present.
PR implementation and coverage: The PR head plans reuse/append/replace before materializing records and its added test covers stable reuse, absolute append indexes, and replacement after file identity changes.
Feature history: The local cache persistence path appears to date to the transaction-safety work, with subsequent current-main maintenance by the same author and an adjacent pruning fix.
Findings None None.
Security None None.

How this fits together

CodexBar scans Codex usage into token snapshots and usage rows, then persists them in a local SQLite cost cache for later usage displays. This change sits between scanned usage data and the stored cache records.

flowchart LR
A[Codex usage scan] --> B[Usage snapshots and rows]
B --> C[Cost cache save]
C --> D[Persistence planner]
D --> E[Reuse stored records]
D --> F[Append new records]
D --> G[Replace changed records]
E --> H[Local SQLite cost cache]
F --> H
G --> H
Loading

Before merge

  • Resolve merge risk (P1) - The branch predates the current release-note layout; GitHub reports it cleanly mergeable, but the final merge-ref review should confirm the current 0.49.2 release metadata is retained.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production versus test growth production +62, tests +141, release notes +1 The added persistence-planning logic is covered by more focused regression code than production growth.

Merge-risk options

Maintainer options:

  1. Decide the mitigation before merge
    Land the planner with its focused persistence coverage once required checks complete and the final merge result preserves current release metadata.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Land the planner with its focused persistence coverage once required checks complete and the final merge result preserves current release metadata.

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

Not applicable as a bug report: this is a performance optimization PR, and its body supplies a real-cache profiler comparison rather than an issue reproduction.

Is this the best way to solve the issue?

Yes. Deciding reuse, append, or replacement before conversion is the narrowest maintainable way to eliminate unchanged-record encoding while retaining the established cache-state rules.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 930b1c4d0d43.

Labels

Label justifications:

  • P2: This targets sustained CPU work during Codex cache refreshes, a meaningful but bounded local performance issue.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This owner-authored internal performance PR is not subject to the external contributor proof gate; its body nevertheless provides after-fix profiler and real-cache timing evidence.

Evidence

What I checked:

Likely related people:

  • steipete: Introduced the transactional cost-cache persistence work and authored the recent executor-safety follow-up; also owns this PR. (role: feature owner and recent area contributor; confidence: high; commits: 8051ce40e4b8, 27054465bd86, 422c4f35c9b1; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageStore+CodexCache.swift, Tests/CodexBarTests/CostUsageStoreTests.swift)
  • Yuxin Qiao: Recent history includes a related cost-store pruning fix in the same persistence and cache lifecycle area. (role: adjacent recent contributor; confidence: medium; commits: c41a419421a3; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageStore+CodexCache.swift)

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

P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. 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.

1 participant