Fix Codex cost catch-up cluster: bounded indexing, configured Finish-now window, semantic progress - #2918
Conversation
- Clear queued restarts before an active pass returns from Stop - Cover the primary worker cancellation race with a regression
- Cap unique candidate inspections before cache eligibility checks\n- Preserve discovered URL identities for queued lookback files\n- Prove bounded selection across consecutive large-corpus passes
- Limit persisted lookback validation to 512 paths per invocation\n- Defer exact inventory proof until no bounded work remains\n- Prove load and save traversal against a 988-path queue
- Page active window directories within the catch-up limit - Persist discovery cursors and defer exact inventory proof - Cover oversized histories and relaunch rehydration
…exing-performance # Conflicts: # CHANGELOG.md
Migrate v0.49.2 stores without rebuilding completed history Bound device identity validation and defer remaining paths Restore Linux directory cursor builds and lint compliance
…exing-performance
Co-authored-by: thomaschow19 <29773444+thomaschow19@users.noreply.github.com>
Co-authored-by: pavbar <pavlo@barzdun.com>
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
Codex review: needs changes before merge. Reviewed August 13, 2026, 5:33 PM ET / 21:33 UTC. ClawSweeper reviewWhat this changesThe PR bounds and persists Codex cost-history catch-up, makes Finish now use the configured history window, and tracks semantic scan progress. Regression provenancePossible regression — probable (reviewed change; failure trace). No predecessor PR is attributed. Merge readinessKeep open: this owner-authored consolidation has three unresolved current review comments covering relaunch progress, deleted queued files, and root replacement during paged catch-up. Likely related people: steipete and Quicksaver (medium confidence). Priority: P2 Review scores
Verification
How this fits togetherCodexBar scans local Codex session files into a persisted cost cache for Usage & Spend and the Spend Dashboard. Catch-up workers advance bounded scan state and publish coverage and progress to those views. flowchart LR
A[Codex session files] --> B[Bounded history scanner]
B --> C[Persisted cost cache]
C --> D[Catch-up status]
D --> E[Primary catch-up worker]
D --> F[Spend Dashboard worker]
E --> G[Usage and Spend UI]
F --> G
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Copy recommended automerge instructionTechnical reviewBest possible solution: Repair the three queued-state and paged-discovery paths with focused regressions, then refresh merge review. Do we have a high-confidence way to reproduce the issue? Yes, from the supplied current review comments: resume a persisted cursor beyond one directory page, remove a queued session, or replace the session root while catch-up is pending. Local reproduction could not be verified because repository inspection was unavailable. Is this the best way to solve the issue? No; the branch needs the three concrete persisted-queue and paged-discovery repairs identified in the supplied current review comments. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 283bd99e18c7. LabelsLabel justifications:
EvidenceAcceptance criteria:
What I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01f72618cd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return CodexDirectoryPage( | ||
| files: files, | ||
| nextOffset: max(resumeOffset, cursor.logicalOffset), | ||
| visits: visits) |
There was a problem hiding this comment.
Advance persisted directory pages after relaunch
When the process relaunches with a persisted resumeOffset greater than visitLimit, the new DIR cursor starts at logical 0 and this returns the old resumeOffset until enough in-memory reads catch up. That post-relaunch pass persists no cursor change and no files, so the UsageStore seen-progress check pauses as noProgress after one pass; large (>512 entry) catch-ups can be left pending with no active worker for the remaining entries.
Useful? React with 👍 / 👎.
| for path in normalizedPrefix { | ||
| let fileURL = URL(fileURLWithPath: path) | ||
| let pathKey = Self.codexPathKey(fileURL) | ||
| guard seenPaths.insert(pathKey).inserted else { continue } | ||
| fileURLsByPathKey[pathKey] = fileURL | ||
| files.append(fileURL) |
There was a problem hiding this comment.
Skip missing queued paths before parsing
When a path in pendingFilePaths was deleted or archived after it was queued, this now appends it to the scan list instead of filtering it out. scanCodexFile eventually opens that URL with FileHandle(forReadingFrom:), so a single missing queued JSONL throws and pauses the whole catch-up rather than dropping the stale cache entry; this is likely with rotating or removed Codex session files.
Useful? React with 👍 / 👎.
| let shouldDropAllUnscannedFiles = options.forceRescan || plan.rootsChanged || cache.files.isEmpty | ||
| || plan.needsProjectMetadataMigration | ||
| for key in cache.files.keys where !filePathsInScan.contains(key) { | ||
| guard let old = cache.files[key] else { continue } | ||
| let shouldDrop = shouldDropAllUnscannedFiles || | ||
| old.touchesCodexScanWindow(sinceKey: range.scanSinceKey, untilKey: range.scanUntilKey) | ||
| guard shouldDrop else { continue } | ||
| Self.applyFileDays(cache: &cache, fileDays: old.days, sign: -1) | ||
| cache.files.removeValue(forKey: key) | ||
| } | ||
| if !shouldPageDiscovery { |
There was a problem hiding this comment.
Drop old-root rows during paged root changes
When the Codex sessions root changes while codexScanCatchUpPending is already true, shouldPageDiscovery is true, so this guard skips the removal loop even though shouldDropAllUnscannedFiles includes plan.rootsChanged. The save below then records the new roots fingerprint while retaining old-root cache.files and day aggregates, causing usage from the previous Codex home/profile to be reported until a later exact proof pass finally drops it.
Useful? React with 👍 / 👎.
Summary
One coherent landing for the Codex cost-history catch-up cluster. This branch contains PR #2849 (rebased onto current main via merge, all conflicts resolved) plus the two remaining root-cause fixes folded in with contributor credit.
Fixes #2861
Refs #2815
Supersedes #2849, #2864, #2844
Root causes fixed
Merge-resolution notes (vs #2849 as filed)
What remains for #2815
The native defects are addressed: bounded passes stay bounded, semantic progress is recognized, completion is durable, and growing active tails no longer keep accelerated catch-up alive forever (complete-file appends are excluded from the progress key, so the scheduler backs off). Two follow-ups from the #2815 thread remain open and are NOT in this branch: honoring the in-app Background-work low-power preference in the catch-up policy (it currently checks only the macOS system Low Power Mode), and surfacing per-file stall reasons in the UI. With the native path repaired, the opt-in ccusage fallback (#2817) is no longer needed as a correctness escape hatch; if still wanted, it is a separate product decision (bundled-executable provenance/signing) best kept as a test oracle instead.
Recommended disposition of sibling PRs
Validation
Thanks @Quicksaver for the substantial indexing/durability work, @thomaschow19 for the Finish-now report and fix, @pavbar for the semantic-progress design, and @Yoroin, @xiehaibin18, @Astro-Han for the detailed privacy-safe diagnostics on #2815.