Skip to content

Feature/Fix: Validate lineage accounting on frozen local histories#2151

Closed
iam-brain wants to merge 12 commits into
steipete:mainfrom
iam-brain:iam-brain/lineage-real-world-validation
Closed

Feature/Fix: Validate lineage accounting on frozen local histories#2151
iam-brain wants to merge 12 commits into
steipete:mainfrom
iam-brain:iam-brain/lineage-real-world-validation

Conversation

@iam-brain

@iam-brain iam-brain commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Stack dependency

This is slice 12 of 15. It follows Feature/Fix: Add a reversible lineage accounting selector; review and merge that PR first.

Summary

  • add an opt-in, aggregate-only frozen-history replay for comparing legacy, ledger, and private OpenAI UTC totals without exposing rollout contents, identities, paths, models, or account totals
  • stop treating retained metadata IDs shared by fork siblings as physical ancestry edges
  • key contained-family fallback by physical owner and scope so sibling rollouts cannot envelope one another
  • keep legacy removal explicitly blocked pending repeat validation, ordinary-day reference totals, and acceptable performance

Real-world result

On the frozen July 9–11 UTC history, ledger selection reduced aggregate absolute error by about 65% relative to the legacy scanner and finished about 3.1% below the OpenAI aggregate.

The remaining evidence is not strong enough to promote the ledger yet. July 9 remains 10.4% low and unchanged from legacy, which points to discovery, parsing, or reference semantics outside lineage deduplication. July 11 is 4.4% high. July 5–8 diverge enough from legacy that they need OpenAI daily totals before they can be classified as corrections or regressions. The replay therefore emits supportsLegacyRemoval: false.

The frozen replay covered 819,779 observations across 37 families. It also showed that the current lineage path is materially slower than legacy scanning on this corpus, so performance remains a promotion blocker.

Tests

  • swift test --jobs 4 --filter 'CodexLineage(Ledger|AccountingSelector|LocalValidation)Tests' (32 tests passed)
  • make check
  • opt-in frozen local replay
  • make test (647 selections in 54 groups; 54 passed, 0 failures, 0 retries)

Part of the planned scanner rearchitecture.

Related investigation: steipete/CodexBar#2037.

@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. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jul 14, 2026
@clawsweeper

clawsweeper Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed July 13, 2026, 9:21 PM ET / July 14, 2026, 01:21 UTC.

Summary
Adds a lineage-ledger accounting stack, reversible authority selector and promotion gates, integrates it with the Codex cost scanner, and adds aggregate-only frozen-history validation with focused tests.

Reproducibility: yes. at source-reproducible confidence: the opt-in frozen-history test provides a concrete replay path and the PR reports detailed results over 819,779 observations, though the private corpus cannot be independently replayed from the checkout alone.

Review metrics: 3 noteworthy metrics.

  • Patch surface: 19 files, +4,506/-19. The slice is a large accounting-architecture addition despite preserving legacy as the default.
  • Validation surface: 9 test files added. Focused selector, discovery, ledger, engine, promotion, shadow, and frozen-replay coverage materially improves reviewability.
  • Frozen replay: 819,779 observations across 37 families. The reported corpus provides meaningful real-world evidence while also exposing unresolved accuracy and performance limits.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #2037
Summary: This PR is a later-stage candidate for the canonical cross-file lineage-accounting work left open after the merged bounded-containment phase.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

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

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

Rank-up moves:

Risk before merge

  • [P1] The generated parser-key change rebuilds existing Codex cost caches, so users may observe changed historical totals after upgrade even while legacy remains the default authority.
  • [P1] The branch depends on Feature/Fix: Add a reversible lineage accounting selector #2150; reviewing or merging it first would evaluate a cumulative stack rather than this slice's final incremental diff.
  • [P1] The ledger still has unresolved day-level undercount and overcount and is materially slower on the frozen corpus, so accidentally promoting it beyond opt-in validation could produce inaccurate totals or slow scans.

Maintainer options:

  1. Merge as a legacy-default experiment (recommended)
    Land this slice only after its predecessor, verify the exact stacked head, preserve .legacy as the default, and document that parser-cache rebuilding may change historical totals.
  2. Pause the accounting stack
    Keep the PR in draft until day-level residuals and scanner performance no longer block a credible promotion path.

Next step before merge

  • [P2] Wait for the predecessor and a maintainer decision on accepting the staged lineage architecture; the review found no narrow mechanical repair suitable for an automated worker.

Maintainer decision needed

  • Question: Should CodexBar accept this lineage subsystem now as staged, opt-in infrastructure behind the legacy default, despite unresolved residual accuracy and performance blockers?
  • Rationale: The patch deliberately separates experimental infrastructure from promotion, but accepting roughly 4,500 lines of new accounting architecture before it is authority-ready is a repository direction and maintenance-cost choice rather than a mechanical correctness decision.
  • Likely owner: steipete — He made and documented the accepted Phase 1 accounting tradeoffs and is the strongest available owner for deciding whether the larger staged architecture belongs in the repository.
  • Options:
    • Stage behind legacy default (recommended): Merge the ordered stack after Feature/Fix: Add a reversible lineage accounting selector #2150, retain legacy authority, and require the existing evidence gates before any promotion.
    • Wait for promotion-ready evidence: Keep the stack in draft until ordinary-day references, residual classifications, and performance targets demonstrate that lineage can plausibly replace legacy accounting.
    • Keep only the validation harness: Narrow the work to privacy-safe replay and diagnostics while deferring the production lineage engine and selector architecture.

Security
Cleared: The diff adds no dependency, workflow, permission, secret-access, package-resolution, or downloaded-code change, and the frozen replay is designed to emit aggregate-only redacted evidence.

Review details

Best possible solution:

Land the ordered stack only if legacy accounting remains the default and the promotion evaluator continues blocking lineage authority until ordinary-day references, repeat corpus validation, and acceptable cold/warm performance are demonstrated.

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

Yes, at source-reproducible confidence: the opt-in frozen-history test provides a concrete replay path and the PR reports detailed results over 819,779 observations, though the private corpus cannot be independently replayed from the checkout alone.

Is this the best way to solve the issue?

Yes for staged evidence gathering, but not yet for production promotion; a reversible legacy-default selector plus explicit evidence gates is safer than replacing the current scanner while residual accuracy and performance remain unresolved.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 0437067deaa8.

Label changes

Label changes:

  • add P2: The PR targets materially incorrect local Codex cost accounting, but the blast radius is limited to the cost-history scanner and the new authority remains non-default.
  • add merge-risk: 🚨 compatibility: Merging changes parser-derived cache identity and introduces alternate accounting semantics that can rebuild and alter users' historical local totals.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides after-change live replay results from a frozen local corpus, including aggregate errors, day-level residuals, corpus size, and performance findings without exposing private rollout data.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body provides after-change live replay results from a frozen local corpus, including aggregate errors, day-level residuals, corpus size, and performance findings without exposing private rollout data.

Label justifications:

  • P2: The PR targets materially incorrect local Codex cost accounting, but the blast radius is limited to the cost-history scanner and the new authority remains non-default.
  • merge-risk: 🚨 compatibility: Merging changes parser-derived cache identity and introduces alternate accounting semantics that can rebuild and alter users' historical local totals.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body provides after-change live replay results from a frozen local corpus, including aggregate errors, day-level residuals, corpus size, and performance findings without exposing private rollout data.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides after-change live replay results from a frozen local corpus, including aggregate errors, day-level residuals, corpus size, and performance findings without exposing private rollout data.
Evidence reviewed

What I checked:

Likely related people:

  • steipete: Led the final Phase 1 scanner refinements, runtime proof, and merge, and previously carried fork-accounting fixes into current main. (role: recent area contributor, reviewer, and merger; confidence: high; commits: c852c13, 275da53, 60f4a4e8ebcd; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift, Tests/CodexBarTests/CostUsageScannerBreakdownTests.swift)
  • Zihao-Qi: Authored the merged Phase 1 interleaved-counter containment and fork-provenance work that this lineage stack extends. (role: introduced current provenance baseline; confidence: high; commits: be56cc9aff5e; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift, Tests/CodexBarTests/CostUsageScannerBreakdownTests.swift)
  • xx205: Authored the merged parent-baseline and replay-accounting implementation that remains a required compatibility invariant. (role: introduced adjacent fork accounting behavior; confidence: high; commits: 45b68c34ec48; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift, Tests/CodexBarTests/CostUsageScannerBreakdownTests.swift)
  • iam-brain: Has prior merged Codex cost and pricing work in this repository and authored the ordered lineage-accounting implementation and validation stack. (role: recent cost-accounting contributor and lineage-stack implementer; confidence: high; commits: 6170f5c8c745, 734f0f6abf09, f95188c17644; files: Sources/CodexBarCore/Providers/Codex/CodexLineageLedger.swift, Sources/CodexBarCore/Providers/Codex/CodexLineageEngine.swift, Tests/CodexBarTests/CodexLineageLocalValidationTests.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.

@iam-brain

Copy link
Copy Markdown
Contributor Author

Superseded by the consolidated four-PR review stack. This slice is now included in #2158; closing this draft to keep the upstream review surface manageable.

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

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant