Feature/Fix: Validate and optimize lineage token accounting#2158
Feature/Fix: Validate and optimize lineage token accounting#2158iam-brain wants to merge 28 commits into
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 14, 2026, 9:38 AM ET / 13:38 UTC. Summary Reproducibility: yes. at the source-and-corpus level: the PR documents a frozen retained-history replay where retained totals massively overcount and the current partial scanner still differs from finalized UTC totals. The private corpus was not independently rerun during this review. Review metrics: 3 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Land the stack only in dependency order as default-off infrastructure, preserve legacy authority and whole-scan rollback, and require exact-head upgrade testing plus broader ordinary-day parity evidence before any production promotion. Do we have a high-confidence way to reproduce the issue? Yes at the source-and-corpus level: the PR documents a frozen retained-history replay where retained totals massively overcount and the current partial scanner still differs from finalized UTC totals. The private corpus was not independently rerun during this review. Is this the best way to solve the issue? Yes for a default-off, reversible infrastructure path; no for promoting lineage accounting to authority yet, because individual UTC days still differ materially from the reference. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against c61e01e774c4. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
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. How this review workflow works
Review history (8 earlier review cycles)
|
c01c5a8 to
19ed384
Compare
19ed384 to
0b5a00e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1cb89f8b1
ℹ️ 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".
| static func codexAccountingProducerKey(mode: CodexLineageAccountingMode) -> String { | ||
| let base = CostUsageCacheIO.currentProducerKey(provider: .codex) ?? "codex" | ||
| return mode == .legacy ? base : base + ":" + mode.producerKeySuffix |
There was a problem hiding this comment.
Include authorization state in lineage cache identity
When .lineage is run with a promotion authorization, the promoted lineage totals are saved under the same producer key used for .lineage without authorization. If authorization is later removed, makeCodexRefreshPlan can decide shouldRefresh == false, so the cached lineage-authoritative days are returned without re-running select and the reversible switch does not fall back to legacy totals until a refresh or force rescan. Include the authorization state in the cache identity or force a refresh when it changes.
Useful? React with 👍 / 👎.
| if selection.usedLineageAuthority { | ||
| cache.days = selection.days |
There was a problem hiding this comment.
Preserve retained days when publishing lineage totals
In authorized .lineage mode, selection.days is computed only from files, which is limited to the current scan window, but just above this block the cache may intentionally retain a wider retainedSinceKey/retainedUntilKey. A refresh for a narrowed date range can therefore replace the whole cache with only the narrowed window while still saving the wider scan bounds, causing later queries inside that retained window to return missing lineage history instead of triggering a rebuild. Recompute over the retained window or merge unchanged retained days when publishing lineage authority.
Useful? React with 👍 / 👎.
Stack dependency
This is PR 4 of 4. It follows PR #2157; review and merge that PR first.
Summary
Adds the final validation, diagnostic, and performance slice for the shadow-lineage rearchitecture.
The scanner is materially closer to OpenAI's finalized UTC totals, but it is not exact and further real-world validation remains necessary.
Final UTC comparison
Positive percentages are overcounts versus OpenAI; negative percentages are undercounts.
Sources and interpretation
The aggregate is much closer than the partial fix, but day-level residuals remain significant. This does not claim exact parity or support removing the legacy fallback.
Performance
The cold retained-data replay used the same 616-document, 918,677-observation corpus for both paths.
The lineage time breaks down as follows (minor rounding and timer-boundary overhead account for the remainder):
The ledger held at most one family at a time: 39 families total, 355,168 observations in the largest family, and 856,177 duplicate observations suppressed. This shows that discovery and repeated JSONL loading—not the lineage calculation itself—are the remaining performance targets.
Validation
make checkRelated investigation: #2037.