Fix compact subagent copied prefixes#2285
Conversation
- 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.
There was a problem hiding this comment.
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
CodexSubagentRolloutShapeto 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.
|
CI note: the only failing check is the Linux ARM64 run. It fails in the untouched The same base SHA (
I tried to rerun the failed job, but GitHub restricts upstream reruns to repository admins. I am |
|
Codex review: needs maintainer review before merge. Reviewed July 18, 2026, 3:31 PM ET / 19:31 UTC. Summary 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.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest 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 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 (1 earlier review cycle)
|
|
Verified before merge:
Merged as |
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.subagentas lineage evidence and adds a narrowly gated second proof path:turn_context, andinter_agent_communication(trigger_turn: true)is physically adjacent;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:
An isolated scan of the same local files produced identical cold, warm, and forced results:
gpt-5.6-solcodex-auto-reviewThe 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:the existing dependency is preserved, with no missing or unexpected dependency modes.
The proof used parser hash
48ac20dad61e9a7f. It was run onorigin/main@1f422d579437abc4301a5b4d3cdb57e491445d5aplus this patch. The branch is now rebased onto9e9f739b6a96e2a786467adb59f1ce6b35f1d6ac; the intervening commits do not changeVendored/CostUsageor 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 checkCodexSubagentRolloutShapeTests(17 tests)CodexCompactSubagentAccountingTests(3 tests)CodexSubagentAccountingIntegrationTests(11 tests)CostUsageCacheTests(9 tests)CostUsageScannerBreakdownTests(92 tests)The focused suites passed with
CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1andDYLD_FRAMEWORK_PATH=.build/out/Products/Debug; the latter supplies Sparkle's local build path to Xcode beta's test runner.The full
make testrunner stops inClaudeOAuthCredentialsStoreTemporaryKeychainCacheTests(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