Skip to content

Fix compact subagent copied prefixes#2285

Merged
steipete merged 2 commits into
steipete:mainfrom
hhh2210:codex/fix-preturn-subagent-prefix
Jul 18, 2026
Merged

Fix compact subagent copied prefixes#2285
steipete merged 2 commits into
steipete:mainfrom
hhh2210:codex/fix-preturn-subagent-prefix

Conversation

@hhh2210

@hhh2210 hhh2210 commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Summary

CodexBar 0.44.0 includes #2228, but compact single-leaf subagent rollouts can still copy the parent counters before their first turn without embedding ancestor metadata. The scanner classifies that shape as independent, so the copied prefix appears as Unknown model.

This patch keeps source.subagent as lineage evidence and adds a narrowly gated second proof path:

  • propose a local suffix only when an explicit parent exists, nonzero totals precede the first turn_context, and inter_agent_communication(trigger_turn: true) is physically adjacent;
  • promote that candidate only when its input, cached-input, and output totals exactly match the parent's snapshot at the fork boundary;
  • keep mismatch and unresolved-parent cases independent;
  • make every candidate outcome depend on the parent snapshot, so a parent change invalidates the cached child classification.

The copied prefix is suppressed rather than relabeled. In the affected live and frozen corpora,
attributed model totals remain unchanged.

Evidence

The released 0.44.0 build showed the residual on Jul 17:

codex-clipboard-1a0e1ad5-b380-4f86-ad90-3b208cd79342.png

An isolated scan of the same local files produced identical cold, warm, and forced results:

Jul 17 CodexBar 0.44.0 Patched scanner
Total tokens 549,936,863 500,408,358
gpt-5.6-sol 485,647,099 485,647,099
codex-auto-review 14,761,259 14,761,259
Unknown 49,528,505 0

The patch removes exactly the Unknown copied-prefix bucket.

I also reran the frozen 2,012-file corpus used to validate #2228. Its canonical manifest SHA-256 is
7ec548cf8a49821c9abba5d0901e1d130d4546316e40f92f2cbb0decb1240368:

  • cold, warm, and forced scans are identical;
  • Jul 16 remains 231,475,556 tokens, $186.318118, with zero Unknown and zero strict-matcher residual;
  • the previous 30-day Unknown bucket falls from 307,441,975 to zero, and total tokens fall by exactly the same amount;
  • only Jul 13 and Jul 14 change, by exactly their previous Unknown buckets;
  • ten compact children that previously had lineage-only cache entries gain parent snapshot dependencies;
    the existing dependency is preserved, with no missing or unexpected dependency modes.

The proof used parser hash 48ac20dad61e9a7f. It was run on origin/main@1f422d579437abc4301a5b4d3cdb57e491445d5a plus this patch. The branch is now rebased onto 9e9f739b6a96e2a786467adb59f1ce6b35f1d6ac; the intervening commits do not change Vendored/CostUsage or the Codex subagent scanner paths.

Maintenance invariant

An embedded ancestor can prove a copied prefix by itself. A compact rollout cannot: it needs both a first-turn boundary and an exact parent snapshot match. Missing or mismatched evidence must remain independent, never be guessed or relabeled. Future scanner changes should preserve both the positive and negative cases in these tests, including parent-dependent cache invalidation.

Tests

  • make check
  • CodexSubagentRolloutShapeTests (17 tests)
  • CodexCompactSubagentAccountingTests (3 tests)
  • CodexSubagentAccountingIntegrationTests (11 tests)
  • CostUsageCacheTests (9 tests)
  • CostUsageScannerBreakdownTests (92 tests)

The focused suites passed with CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 and
DYLD_FRAMEWORK_PATH=.build/out/Products/Debug; the latter supplies Sparkle's local build path to Xcode beta's test runner.

The full make test runner stops in ClaudeOAuthCredentialsStoreTemporaryKeychainCacheTests
(four assertions, also reproducible when that suite is run alone). This diff does not touch Claude credential storage.

This extends the rollout-shape coverage added in #2228. It does not introduce or replace the broader Phase 2 lineage ledger.

Fixes #2108
Refs #2037

- Confirm first-turn suffix candidates only when pre-turn totals exactly match the parent snapshot; mismatches remain independent.
- Keep candidate classifications parent-dependent so cache reuse tracks parent changes.
- Add shape, replay, accounting, and invalidation regressions and rotate the generated parser hash.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refines Codex subagent rollout accounting in CostUsageScanner to eliminate the “Unknown model” bucket caused by compact single-leaf subagent rollouts that copy parent counters before their first turn, while keeping unresolved/mismatched cases independent and enforcing parent-snapshot-based cache invalidation.

Changes:

  • Extend CodexSubagentRolloutShape to optionally propose a local owned-suffix candidate for compact rollouts only when an explicit parent exists and a narrowly defined first-turn marker shape is detected.
  • In CostUsageScanner, promote that candidate to a copied-prefix boundary only when the parent’s resolved snapshot exactly matches the child’s fork-boundary totals, and ensure cache dependency keys reflect parent snapshot changes.
  • Add focused tests and a reusable compact-rollout fixture covering confirmation, invalidation, and negative/unconfirmed cases.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
Tests/CodexBarTests/CodexSubagentRolloutShapeTests.swift Adds unit coverage for the new “parent-confirmed suffix candidate” proposal gate and its negative cases.
Tests/CodexBarTests/CodexCompactSubagentFixture.swift Introduces a compact parent/child JSONL fixture generator used by new accounting tests.
Tests/CodexBarTests/CodexCompactSubagentAccountingTests.swift Adds end-to-end accounting + cache dependency invalidation coverage for compact copied-prefix suppression.
Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift Implements candidate promotion via parent snapshot match; ensures parent-dependent cache invalidation even when the child uses a local boundary.
Sources/CodexBarCore/Vendored/CostUsage/CodexSubagentRolloutShape.swift Adds ownedSuffixCandidate and the narrowly gated detection logic for compact rollouts with an explicit parent.
Sources/CodexBarCore/Generated/CodexParserHash.generated.swift Updates the parser hash to reflect the semantic change.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@hhh2210

hhh2210 commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

CI note: the only failing check is the Linux ARM64 run. It fails in the untouched
TestsLinux/PlatformGatingTests.swift:128, where both parameterized cases compare a temporary
Claude CLI invocation log. This PR does not change that test or the Claude provider.

The same base SHA (9e9f739b) shows the reciprocal flake on main: ARM64 passed while Linux x64
failed in a different temporary-file test. On this PR, Linux x64 passed and ARM64 failed. Both macOS
shards, lint, and the aggregate lint-build-test gate passed.

I tried to rerun the failed job, but GitHub restricts upstream reruns to repository admins. I am
keeping the scanner patch unchanged rather than mixing an unrelated Linux test stabilization into
this PR.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. 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. P2 Normal priority bug or improvement with limited blast radius. labels Jul 18, 2026
@clawsweeper

clawsweeper Bot commented Jul 18, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed July 18, 2026, 3:31 PM ET / 19:31 UTC.

Summary
The branch adds exact parent-snapshot confirmation for compact Codex subagent rollouts and regression coverage so proven copied pre-turn prefixes are excluded from usage accounting.

Reproducibility: yes. at source level: the added compact parent/child fixture encodes the explicit-parent, pre-turn-total, and adjacent-turn-marker shape, with tests covering exact-match, mismatch, and unresolved-parent outcomes. The reported real corpus output is strong supplemental evidence but was not independently rerun in this read-only review.

Review metrics: 2 noteworthy metrics.

  • Patch scope: 6 files affected; 534 added, 17 removed. Most of the patch is focused regression coverage around two CostUsage production files.
  • Regression coverage: 2 test files added; 1 rollout-shape suite extended. The proposed behavior is covered at both classifier and end-to-end accounting/cache seams.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

  • [P1] Merging deliberately changes displayed historical Codex usage when a compact rollout matches a parent boundary exactly; maintainers should confirm the exact-match invariant remains conservative enough for real customer corpus data.

Maintainer options:

  1. Accept the conservative accounting correction (recommended)
    Merge after normal maintainer review if the exact snapshot-match invariant is accepted as the intended boundary for changing reported historical totals.
  2. Pause for a narrower corpus review
    Hold the PR if maintainers need an independently inspected compact-rollout sample before accepting the new accounting classification.

Next step before merge

  • No discrete repair finding is supported; the remaining action is normal maintainer review of whether the exact parent-snapshot invariant is acceptable for production usage accounting.

Security
Cleared: The listed diff changes scanner logic, generated parser identity, and XCTest fixtures without adding dependencies, secrets access, workflows, downloads, or supply-chain execution paths.

Review details

Best possible solution:

Land the narrow parent-confirmed classification only if maintainers agree that exact input, cached-input, and output equality is sufficient evidence of inherited counters, while retaining independent accounting for every unresolved or mismatched rollout.

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

Yes at source level: the added compact parent/child fixture encodes the explicit-parent, pre-turn-total, and adjacent-turn-marker shape, with tests covering exact-match, mismatch, and unresolved-parent outcomes. The reported real corpus output is strong supplemental evidence but was not independently rerun in this read-only review.

Is this the best way to solve the issue?

Yes, provisionally: an exact parent snapshot match is a narrower and safer proof than inferring copied ownership from subagent lineage alone, and the patch preserves independent accounting when the evidence is absent or mismatched.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 167db4dca93e.

Label changes

Label changes:

  • add merge-risk: 🚨 other: The exact-match heuristic intentionally changes persisted scan results and displayed historical usage totals for a narrowly defined rollout shape.
  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.

Label justifications:

  • P2: This fixes bounded but user-visible Codex usage-accounting inaccuracies without evidence of a release-blocking outage.
  • merge-risk: 🚨 other: The exact-match heuristic intentionally changes persisted scan results and displayed historical usage totals for a narrowly defined rollout shape.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body provides a redacted before-state screenshot plus after-fix local scan tables, cold/warm/forced parity results, and a frozen-corpus comparison showing only the previously unknown copied-prefix buckets change.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides a redacted before-state screenshot plus after-fix local scan tables, cold/warm/forced parity results, and a frozen-corpus comparison showing only the previously unknown copied-prefix buckets change.
Evidence reviewed

What I checked:

Likely related people:

  • hhh2210: Authored the earlier merged copied-prefix accounting change and this narrowly scoped continuation, including the affected rollout classifier, scanner path, and regression coverage. (role: introduced related behavior and recent area contributor; confidence: high; commits: a1fad485d668, 1860a33525eb; files: Sources/CodexBarCore/Vendored/CostUsage/CodexSubagentRolloutShape.swift, Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift, Tests/CodexBarTests/CodexCompactSubagentAccountingTests.swift)
  • steipete: Authored the branch commit that integrates current main, making them a relevant routing candidate for the current landing and release path. (role: recent integration contributor; confidence: medium; commits: acf512d6ac40; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • 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.
Review history (1 earlier review cycle)
  • reviewed 2026-07-18T06:49:01.929Z sha 1860a33 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 18, 2026
@steipete
steipete merged commit dbcba17 into steipete:main Jul 18, 2026
9 checks passed
@steipete

Copy link
Copy Markdown
Owner

Verified before merge:

  • Reviewed the full scanner/parser/test diff and the Fix copied-prefix Codex subagent accounting #2228 cache contract. The compact shape is only promoted after an explicit parent, first-turn adjacent trigger, nonzero pre-boundary totals, and an exact parent snapshot match. Mismatch or unresolved parents remain independently counted.
  • Confirmed cache safety: candidate classifications always retain the parent dependency key; parent changes invalidate the child. Scripts/regenerate-codex-parser-hash.sh --check reported 48ac20dad61e9a7f. The producer hash rotated; no cache schema/layout changed, so no artifact-version bump was needed.
  • Ran CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 swift test --filter 'Codex(Subagent|CompactSubagent)|CostUsageScannerBreakdown|CostUsageCacheTests': 129 tests in 5 suites passed.
  • Ran make check: SwiftFormat clean; SwiftLint found 0 violations in 1,574 files; all repository checks passed.
  • Ran maintainer autoreview twice, including immediately before merge: no accepted/actionable findings.
  • Exact PR head acf512d6ac408cca623b54addd242254b3d5bda9 passed all 9 checks in CI run 29657884872: lint, Linux x64, Linux arm64, Linux musl, both macOS shards, aggregate, changes, and GitGuardian.

Merged as dbcba17b28b2841d38017a30c52ded5fb330a9e0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor 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.

Codex real fork corpus: 5,427 replayed parent events across rewritten timestamps

3 participants