Expose Codex scan completeness in cost JSON - #2520
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 30, 2026, 5:15 PM ET / 21:15 UTC. ClawSweeper reviewWhat this changesThe branch adds a Codex history-scan completeness field to cost JSON, persists it through the incremental scan cache, and adds Merge readiness⛔ Blocked until stronger real behavior proof is added - 4 items remain This PR appears to implement the scan-completeness contract narrowly and covers the two earlier cache-continuation concerns, but it remains merge-blocked by proof and product-contract review: the posted terminal evidence names commit Priority: P2 Review scores
Verification
How this fits togetherCodexBar’s flowchart LR
A[Codex session logs] --> B[Bounded history scanner]
C[Pi and OMP mirrors] --> D[Optional merged history]
B --> E[Coverage decision]
D --> F[Cost aggregation]
E --> G[Cost snapshot]
F --> G
G --> H[CLI JSON output]
Decision needed
Why: The implementation is additive, but these names and semantics become compatibility commitments for external JSON consumers and cost-reporting workflows. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Adopt the completeness field as an additive, documented Codex JSON contract while preserving merged scans as the default, then merge only after a redacted final-head run demonstrates both default and Do we have a high-confidence way to reproduce the issue? Not applicable: this PR adds an observable completeness contract and an opt-in CLI mode rather than repairing a reported current-main failure. The scanner edge cases are covered by focused tests and a redacted terminal transcript, but the transcript is not from the final head. Is this the best way to solve the issue? Unclear: the scanner-to-snapshot propagation is a narrow implementation and addresses the earlier review concerns, but a maintainer must still decide whether the field and native-only flag should be a supported public CLI contract. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 8ef86077e70a. LabelsLabel justifications:
EvidenceWhat 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 (4 earlier review cycles)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc4d81844e
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| $0.codexScanComplete == false | ||
| && $0.touchesCodexScanWindow( | ||
| sinceKey: range.scanSinceKey, | ||
| untilKey: range.scanUntilKey) |
There was a problem hiding this comment.
Keep unfinished lookback files in the coverage check
When a recently modified session outside the requested date partitions is only partially scanned before reaching events in the requested window, its cached days do not touch this range. Subsequent refreshes neither repeat the cold-cache lookback nor select it through cachedCodexSessionFiles, and this predicate consequently ignores its codexScanComplete == false state; with no new per-refresh deferrals, the cache is then persisted as complete even though that file was never finished, so cost JSON can expose historyCoverageIsEstablished: true alongside partial totals. Track unfinished candidates independently of their parsed days or keep the lookback active while prior coverage is false.
Useful? React with 👍 / 👎.
| now: now, | ||
| historyDays: clampedHistoryDays, | ||
| calendar: options.calendar, | ||
| historyCoverageIsEstablished: cache.codexHistoryCoverageIsEstablished == true, |
There was a problem hiding this comment.
Validate the native cache before reusing its coverage flag
When the Pi cache is valid but the native Codex cache is rejected by the root, timezone, or requested-window checks above, reports can still contain the Pi report and reach this assignment. The coverage value nevertheless comes unconditionally from that rejected native cache; for example, after a complete 30-day native scan and a wider Pi scan, switching to 365 days can hydrate a Pi-only snapshot with historyCoverageIsEstablished: true even though native Codex history for the expanded window was never scanned. Gate this flag on the same native-cache validity checks, otherwise use false when only Pi data is loaded.
Useful? React with 👍 / 👎.
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
All contributor-side review findings are resolved, and the downstream deletion-first integration is ready. This PR is the release gate for NickGuAI/monorepo-g#2010, whose accepted contract requires pinning an official CodexBar release rather than a fork commit. When this additive field is acceptable, could it please be merged and included in the next release? |
Summary
CostUsageTokenSnapshot.historyCoverageIsEstablishedfieldhistoryCoverageIsEstablishedincodexbar cost --format jsonContract
Consumers can distinguish a cold, partial scan from a complete zero or complete total without parsing diagnostic logs. A successful non-Codex cost snapshot reports
true; provider error rows omit the field.Completeness remains fail-closed while a recently modified session outside the date partition is only partly parsed. A Pi-only or mixed cached result also cannot inherit completeness from a native Codex cache that is invalid for the requested window.
Related consumer work: https://github.com/NickGuAI/monorepo-g/issues/2010
Real CLI behavior
Built and ran
CodexBarCLIfrom this PR's exact head (1b2f75a) on Swift 6.2 / Ubuntu 24.04. Provider-native session files were mounted read-only; the transcript projects only public state and redacts usage values.First, a cold scan of one completed real Codex session:
Then, using that cache, the same provider roots were expanded to the real 30-day active and archived corpus. The bounded work exceeded one refresh, and the previously complete state was not reused:
An independent cold-cache run against the full corpus also emitted
false. No prompts, completions, tool content, native log files, private paths, private cache fields, or usage totals are included in this evidence.Regression coverage
truefalsewhen the native Codex cache is invalid for the requested windowVerification
make teston Swift 6.2 / Ubuntu 24.04: all 48 discovered selections passed in 4 groups./Scripts/lint.sh lint-linuxportable checks passed on Ubuntu 24.042172c319c3d32054)git diff --check