Skip to content

Feature/Fix: Adapt rollout snapshots for lineage accounting#2141

Closed
iam-brain wants to merge 2 commits into
steipete:mainfrom
iam-brain:iam-brain/lineage-document-adapter
Closed

Feature/Fix: Adapt rollout snapshots for lineage accounting#2141
iam-brain wants to merge 2 commits into
steipete:mainfrom
iam-brain:iam-brain/lineage-document-adapter

Conversation

@iam-brain

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

Copy link
Copy Markdown
Contributor

Stack dependency

This is slice 2 of 15. It follows Feature/Fix: Add a shadow lineage ledger for Codex token accounting; review and merge that PR first.

Summary

Adds the first scanner adapter for the planned lineage-ledger rearchitecture. The existing fast, cancellation-aware JSONL scan now produces raw complete token observations alongside its legacy cumulative snapshots, then adapts that parsed evidence into CodexLineageLedger.Document values.

This is a stacked PR on top of the shadow ledger foundation. It does not run the ledger from production discovery and does not change published totals.

Details

  • Reuses one CostUsageJsonl pass; no second rollout parser
  • Preserves canonical filename ownership, metadata session identity, and fork-parent identity
  • Preserves complete raw (last_token_usage, total_token_usage) observations
  • Keeps incomplete states available to the legacy snapshot path while excluding them from the ledger contract
  • Propagates cancellation
  • Leaves malformed timestamp policy to the mapped conservative-evidence slice

Validation

  • swift test --filter CodexLineageLedgerTests — 9 tests passed
  • make check
  • SDKROOT=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX26.5.sdk make test — all 639 selections passed across 54 groups with no retries or timeouts
  • Repository review gate: no material findings

Tracks Feed the fast parser into typed rollout documents.

Related investigation: steipete/CodexBar#2037.

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. 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 real behavior proof before merge. Reviewed July 13, 2026, 9:08 PM ET / July 14, 2026, 01:08 UTC.

Summary
The draft adds a shadow Codex lineage ledger, adapts the existing rollout scan into typed ledger documents with complete token observations, updates the parser hash, and adds focused tests.

Reproducibility: not applicable. as a standalone bug reproduction: this draft is a shadow architecture adapter, while the underlying accounting problem is represented by the canonical issue and existing sanitized fixtures.

Review metrics: 3 noteworthy metrics.

  • Patch surface: 4 files, +472/-5. The slice is primarily new ledger and test code but also changes the production scanner and generated cache producer hash.
  • Reported validation: 9 focused tests; 639 full-suite selections. Automated coverage is substantial but does not replace required real-rollout proof.
  • Stack position: 2 of 15. The open predecessor and downstream slices materially affect review and landing order.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #2037
Summary: The issue remains canonical for exact cross-file lineage accounting; this PR is one shadow implementation slice following the merged conservative containment phase.

Members:

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

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • [P1] Add redacted real-rollout initial and cached repeat-scan output showing unchanged published totals.
  • Land or rebase onto the predecessor PR before requesting final review.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR provides automated tests and checks only; add redacted terminal or live output from a real affected rollout, including initial and cached repeat scans, then update the PR body to trigger review or ask a maintainer for @clawsweeper re-review, with private paths, IDs, endpoints, keys, and account data removed.

Risk before merge

  • [P1] The parser producer-hash change rebuilds existing Codex cost caches during upgrade despite the stated intent to leave published totals unchanged.
  • [P1] The patch depends on an unmerged predecessor, so landing or reviewing it independently would blur the intended architectural boundary.
  • [P1] Automated fixtures do not establish that real rollout metadata, rewritten timestamps, incomplete token states, and cached repeat scans preserve user-visible totals.

Maintainer options:

  1. Prove upgrade behavior (recommended)
    After the predecessor lands, provide redacted real-rollout initial and cached repeat-scan output confirming unchanged published totals and the expected one-time cache rebuild.
  2. Accept test-only risk
    Merge after the predecessor using automated coverage alone, accepting that existing users' cost caches will rebuild without live upgrade proof.
  3. Pause this stack slice
    Keep the draft open until more of the lineage stack is ready if the intermediate cache invalidation and separate review overhead are not worthwhile.

Next step before merge

  • [P1] Keep the draft open for ordered stack review and contributor-supplied real-rollout upgrade proof; there is no narrow automated code repair to queue.

Security
Cleared: The diff introduces no dependency, workflow, permission, secret-handling, downloaded-code, package-resolution, or external execution concern.

Review details

Best possible solution:

Review and land the shadow-ledger foundation first, then require a redacted real-rollout initial and cached repeat-scan transcript showing correct document identity, preserved cancellation, unchanged published totals, and acceptable one-time cache rebuilding.

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

Not applicable as a standalone bug reproduction: this draft is a shadow architecture adapter, while the underlying accounting problem is represented by the canonical issue and existing sanitized fixtures.

Is this the best way to solve the issue?

Unclear pending stack and runtime proof. Reusing the existing cancellation-aware scan is a narrow maintainable approach, but its real upgrade and accounting behavior must be demonstrated before acceptance.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P2: This is a bounded local cost-accounting improvement with meaningful correctness implications but no core-runtime or release-blocking failure.
  • add merge-risk: 🚨 compatibility: The parser-hash change rebuilds existing users' local cost caches on upgrade without initial-versus-cached real-session proof.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR provides automated tests and checks only; add redacted terminal or live output from a real affected rollout, including initial and cached repeat scans, then update the PR body to trigger review or ask a maintainer for @clawsweeper re-review, with private paths, IDs, endpoints, keys, and account data removed.

Label justifications:

  • P2: This is a bounded local cost-accounting improvement with meaningful correctness implications but no core-runtime or release-blocking failure.
  • merge-risk: 🚨 compatibility: The parser-hash change rebuilds existing users' local cost caches on upgrade without initial-versus-cached real-session proof.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR provides automated tests and checks only; add redacted terminal or live output from a real affected rollout, including initial and cached repeat scans, then update the PR body to trigger review or ask a maintainer for @clawsweeper re-review, with private paths, IDs, endpoints, keys, and account data removed.
Evidence reviewed

What I checked:

Likely related people:

  • steipete: Substantially refined and merged the current-main Phase 1 interleaved-counter containment, including its cache and fail-open semantics. (role: recent area contributor and merger; confidence: high; commits: cb6538b18d09, ab65dd331faf, 6a7c577acf58; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift, Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner+CacheHelpers.swift, Tests/CodexBarTests/CostUsageScannerBreakdownTests.swift)
  • xx205: Authored the merged parent-baseline and replay-accounting work that the proposed lineage design must preserve. (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 pricing and incremental cost-history work in this area and authored the current shadow-ledger stack. (role: cost-scanner feature contributor; confidence: medium; commits: 6170f5c8c745, 547567d51686; files: Sources/CodexBarCore/Providers/Codex/CodexLineageLedger.swift, Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift, Tests/CodexBarTests/CodexLineageLedgerTests.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 #2155; 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. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant