Fix copied-prefix Codex subagent accounting#2228
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes Codex local usage accounting for forked/subagent rollouts that copy an ancestor prefix by classifying rollout shape and, when possible, attributing only the leaf-owned suffix instead of treating source.subagent as synonymous with an independent cumulative counter. It also updates cache dependency handling so fork lineage can be recorded without forcing parent-baseline cache invalidation when the child no longer depends on parent totals.
Changes:
- Add a subagent rollout shape classifier (embedded ancestor detection + local marker boundary + reset-evidence handling) and integrate it into the Codex JSONL scanner.
- Record “lineage-only” cache dependency mode for forked sessions that do not depend on parent totals, and skip parent dependency validation for those entries.
- Add focused unit + integration tests covering copied-prefix + marker-owned suffix behavior, fast/Foundation parity, and cache mode persistence.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/CodexBarTests/CostUsageScannerBreakdownTests.swift | Extends regression coverage to assert cold/warm/force parity and lineage-only cache dependency mode. |
| Tests/CodexBarTests/CodexSubagentRolloutShapeTests.swift | New unit tests for the rollout shape classifier invariants and boundary detection. |
| Tests/CodexBarTests/CodexSubagentAccountingIntegrationTests.swift | New integration tests verifying inherited-baseline vs owned-suffix behavior and cache mode persistence (fast + fallback parser). |
| Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner+CacheHelpers.swift | Stores and validates fork dependency keys with a lineage-only mode that bypasses parent dependency checks. |
| Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift | Buffers subagent fast lines, classifies copied-prefix vs independent semantics, and replays events through the existing reducer using the correct baseline/boundary. |
| Sources/CodexBarCore/Vendored/CostUsage/CodexSubagentRolloutShape.swift | New classifier implementation for distinguishing independent counters vs copied prefixes and locating leaf-owned suffixes. |
| Sources/CodexBarCore/Generated/CodexParserHash.generated.swift | Updates producer hash to invalidate stale Codex caches after semantic parser changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
clawsweeper
Bot
added
rating: 🦐 gold shrimp
|
Codex review: needs real behavior proof before merge. Reviewed July 16, 2026, 5:18 PM ET / 21:18 UTC. Summary Reproducibility: yes. for the reported pre-hardening revision: the frozen 2,012-file corpus shows the then-current main producing 4.086B July 16 tokens and 886M Unknown tokens, versus 231M and zero Unknown on 6786227. The same live comparison has not yet been shown for current head cc5a205. 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:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Retain the hardened shape classifier and parser-hash invalidation, rerun the same isolated frozen-corpus scan on cc5a205, then merge after exact-head checks pass and continue the broader historical Unknown-token investigation separately. Do we have a high-confidence way to reproduce the issue? Yes for the reported pre-hardening revision: the frozen 2,012-file corpus shows the then-current main producing 4.086B July 16 tokens and 886M Unknown tokens, versus 231M and zero Unknown on 6786227. The same live comparison has not yet been shown for current head cc5a205. Is this the best way to solve the issue? Yes. Deriving counter semantics from embedded lineage and a local turn marker, then replaying the owned range through the existing totals reducer, is narrower and safer than restoring either blanket subagent rule. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 6ffdfeba6c6e. Label changesLabel changes:
Label 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 (3 earlier review cycles) |
clawsweeper
Bot
added
proof: sufficient
clawsweeper
Bot
added
rating: 🦐 gold shrimp
…pied-prefix-accounting # Conflicts: # Sources/CodexBarCore/Generated/CodexParserHash.generated.swift
|
Merged as Local verification on head
CI run https://github.com/steipete/CodexBar/actions/runs/29540917080 completed green on the exact head: lint, changes, Linux x64, Linux arm64, both macOS test shards, and lint-build-test. GitGuardian also passed. PR state was Maintainer commits: |
Summary
The regression is an abstraction error, not a missing special case.
source.subagentdescribes lineage provenance. It does not guarantee that the file owns an independent cumulative counter.User-visible failure
The currently installed CodexBar build on the affected machine attributed 591M Jul 16 tokens to
Unknown modeland displayed 1.3B tokens for the day:The screenshot is symptom evidence. The quantitative comparison below uses the same frozen 2,012-file corpus and isolated empty caches for both revisions.
origin/main(6d71af30)67862278)Cold, warm, and forced scans on the patch produced identical report values. Jul 16 resolves to 223,869,513
gpt-5.6-soltokens plus 7,606,043codex-auto-reviewtokens. The 30-day result is 3,799,232,783 tokens / $2,765.5475077; it still contains 307,441,975 historical Unknown tokens, so this PR does not claim to complete #2037.The bisect and representative-file arithmetic are in the #2208 regression report. #2208 fixed the real reset-only shape from #2193, but generalized that fixture into an unconditional
source.subagentrule. That bypasses the parent resolver, so #2118 cannot invalidate or repair the resulting attribution.Design
The scanner keeps its existing metadata probe and one full streaming pass; this PR does not add a second full raw-JSON pass. For explicit subagent rollouts it buffers compact parsed observations for one file, not raw JSON, then classifies the rollout before replaying events through the existing totals reducer.
The classifier uses these invariants:
session_metaowns leaf identity. Later metadata may enrich missing fields for the same concrete leaf ID, but cannot replace it.turn_context+inter_agent_communication_metadata(trigger_turn: true)pair opens the leaf-owned suffix.lastand is below the pre-marker baseline.The fourth rule matters in the real corpus. Of 197 copied-prefix files with a usable local boundary, 191 contain 10,197 cumulative token rows between the final ancestor metadata and the first valid marker. Cutting at metadata would discard valid baseline evolution.
The final cache audit found 208 forked entries: 197 marker-owned suffixes, 10 independent counters, and one inherited dependency. No forked entry lacked a dependency mode, and no unexpected lineage-only entry was produced.
There is an explicit source comment against restoring either blanket rule.
source.subagentmust not be treated as synonymous with “independent counter” or “inherited counter.”Cache compatibility
This keeps the cache contract proposed in #1042 and landed separately by the maintainer as
5aded294with co-author credit:3c42ef1a44c90442here);codex-vNchanges only for an incompatible stored schema or cache layout.If a parent changes across both snapshot-read attempts, the resolver's dependency key remains nil. The child cache therefore cannot be reused on the next refresh, preserving #2118's unstable-parent invalidation contract.
No
codex-v10bump is needed. This is exactly the kind of parser semantic change the producer hash is meant to invalidate automatically.Validation
make checkafter rebasing ontoorigin/main: passed; SwiftFormat and SwiftLint report zero violations.CostUsageScannerBreakdownTests, existing Ultra-mode Terra and Sol sessions can overcount forked context #2037 regression fixtures, fast/Foundation parser parity, late lineage metadata, cold/warm/force cache behavior, unstable-parent invalidation, and reset-only preservation.origin/maincold: 5.84s / 298.2 MiB and 6.04s / 299.2 MiB;origin/mainrelease harness using the same corpus and cache isolation, average wall time is about 16.8% lower and peak RSS about 62.6% lower. This small local A/B is directional evidence only, not a general performance claim. The result is consistent with reduced copied-prefix row materialization, but that cause was not profiler-proven.make testis not fully green on this machine becauseClaudeOAuthCredentialsStoreCLIStorageOwnershipTestsdeterministically fails 7/8 tests on cleanorigin/mainat6d71af30with the same eight assertions. The branch reproduces that unrelated baseline exactly; no scanner or cache test fails.Fixes #2108.
Refs #2037, #2118, #2193, and #2208.