Skip to content

Feature/Fix: Run lineage accounting in shadow mode#2144

Closed
iam-brain wants to merge 5 commits into
steipete:mainfrom
iam-brain:iam-brain/shadow-lineage-pipeline
Closed

Feature/Fix: Run lineage accounting in shadow mode#2144
iam-brain wants to merge 5 commits into
steipete:mainfrom
iam-brain:iam-brain/shadow-lineage-pipeline

Conversation

@iam-brain

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

Copy link
Copy Markdown
Contributor

Stack dependency

This is slice 5 of 15. It follows Feature/Fix: Preserve lineage accounting dimensions; review and merge that PR first.

Summary

  • run bounded lineage discovery and reconciliation after each refreshed legacy Codex scan
  • record privacy-safe per-day token differences and lineage coverage diagnostics
  • reject malformed timestamps from shadow totals while recording their count
  • propagate cancellation and contain all other shadow failures without changing production reports or cache persistence

This is the fifth isolated component of the planned Codex scanner rearchitecture. The legacy scanner remains authoritative.

Validation

  • swift test --filter CodexLineageShadowTests
  • make check
  • env SDKROOT=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX26.5.sdk make test (641 selections)
  • repository review gate: no unresolved material findings

Related investigation: steipete/CodexBar#2037.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. 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. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. 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:19 PM ET / July 14, 2026, 01:19 UTC.

Summary
This PR adds transitive Codex lineage discovery, an experimental lineage ledger, per-day shadow comparisons and diagnostics, parser adaptations, and focused tests while keeping legacy accounting authoritative.

Reproducibility: yes. at the source level: the canonical issue and merged sanitized fork-family fixtures establish the overcount shape and need for cross-file lineage accounting. This PR lacks a new live affected-corpus run for its shadow pipeline. (github.com)

Review metrics: 3 noteworthy metrics.

  • Patch surface: 8 files, +1,202/-11. The slice is mostly additive but substantial for an always-run scanner path.
  • New accounting modules: 3 files, 501 lines added. Discovery, ledger, and comparison logic create a parallel accounting subsystem needing integration-level proof.
  • Focused test surface: 3 files, 555 lines added. Coverage is broad at the fixture and unit level but remains supplemental to real-corpus behavior proof.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #2037
Summary: This PR is a non-authoritative implementation slice toward the canonical cross-file lineage fix; adjacent stacked pull requests preserve prerequisite dimensions and classify resulting residuals.

Members:

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
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:

  • Post redacted initial and cached-repeat live scanner output showing unchanged legacy totals, shadow diagnostics, cancellation behavior, and elapsed time.
  • Rebase the slice after Feature/Fix: Preserve lineage accounting dimensions #2143 lands so maintainers can review the true incremental diff.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR reports focused and full automated tests but no after-fix run on a real affected corpus; add redacted initial and cached-repeat terminal output with legacy totals, diagnostics, cancellation, and timings, then update the PR body to trigger review or ask a maintainer to comment @clawsweeper re-review.

Risk before merge

  • [P1] Every refreshed legacy scan performs additional lineage parsing and may traverse active and archived roots to resolve ancestors; automated tests do not establish acceptable latency or cancellation behavior on a large real corpus.
  • [P1] The parser-hash change can force existing Codex cost-cache work to repeat on upgrade even though authoritative totals are intended to remain unchanged.
  • [P1] The branch includes its still-open prerequisite, obscuring the true incremental diff and stack ownership until Feature/Fix: Preserve lineage accounting dimensions #2143 lands.
  • [P1] Without an affected initial and cached repeat scan, containment of non-cancellation failures and production-report equivalence remain unproven in the contributor's setup.

Maintainer options:

  1. Prove and bound the shadow path (recommended)
    Merge the prerequisite, avoid repeated full-root discovery where possible, and add redacted initial/repeat live output with timings and unchanged legacy totals before landing this slice.
  2. Accept the observational rollout cost
    Maintainers may intentionally accept parser-cache rebuilding and additional refresh latency to collect diagnostics, but should document that decision in the PR.
  3. Pause until the stack is reviewable
    Keep this draft paused if the prerequisite or later shared-index design is likely to materially reshape the implementation.

Next step before merge

  • [P1] Wait for the prerequisite, require contributor-owned real-corpus proof, and have the scanner owner decide whether refresh and cache costs are acceptable; no narrow automated repair is established.

Maintainer decision needed

  • Question: Should shadow lineage discovery run after every refreshed legacy scan and change parser cache identity before live large-corpus latency and repeat-scan compatibility are demonstrated?
  • Rationale: The architecture is observational, but it adds filesystem work to a user-facing refresh path and changes cache identity; the acceptable overhead and rollout boundary require repository-owner judgment.
  • Likely owner: steipete — The repository owner set the current scanner's containment, cache, fail-open, and runtime-proof boundaries in the immediately preceding merged work.
  • Options:
    • Require bounded live proof (recommended): Land the prerequisite first, then require redacted initial/repeat scans and either cached discovery or measured evidence that traversal stays within an acceptable refresh budget.
    • Accept shadow overhead: Merge after the prerequisite and required checks, explicitly accepting cache rebuilding and per-refresh discovery cost to gather diagnostics sooner.
    • Pause the shadow stack: Hold this slice until the full rollout design can share indexing and avoid temporary duplicate scanning infrastructure.

Security
Cleared: The diff adds no dependency, workflow, permission, secret, network, downloaded-code, or package-resolution change, and diagnostics are limited to dates, aggregate token deltas, and counts.

Review details

Best possible solution:

Close this PR as superseded by #2082.

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

Yes at the source level: the canonical issue and merged sanitized fork-family fixtures establish the overcount shape and need for cross-file lineage accounting. This PR lacks a new live affected-corpus run for its shadow pipeline. (github.com)

Is this the best way to solve the issue?

Unclear as submitted. A shadow-first rollout is maintainable, but unconditional refresh integration and the parser-hash change need prerequisite isolation and live initial/repeat-scan proof before this is established as the best landing shape.

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 advances a normal-priority local accounting fix with limited blast radius and does not change authoritative totals yet.
  • add merge-risk: 🚨 compatibility: The parser hash changes and can invalidate existing scanner cache work even though the new accounting path is intended to be observational.
  • add merge-risk: 🚨 availability: Transitive lineage discovery and parsing run on the refresh path and may add material latency or stalls on large session trees.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab 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 reports focused and full automated tests but no after-fix run on a real affected corpus; add redacted initial and cached-repeat terminal output with legacy totals, diagnostics, cancellation, and timings, then update the PR body to trigger review or ask a maintainer to comment @clawsweeper re-review.

Label justifications:

  • P2: The PR advances a normal-priority local accounting fix with limited blast radius and does not change authoritative totals yet.
  • merge-risk: 🚨 compatibility: The parser hash changes and can invalidate existing scanner cache work even though the new accounting path is intended to be observational.
  • merge-risk: 🚨 availability: Transitive lineage discovery and parsing run on the refresh path and may add material latency or stalls on large session trees.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab 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 reports focused and full automated tests but no after-fix run on a real affected corpus; add redacted initial and cached-repeat terminal output with legacy totals, diagnostics, cancellation, and timings, then update the PR body to trigger review or ask a maintainer to comment @clawsweeper re-review.
Evidence reviewed

What I checked:

Likely related people:

  • steipete: Authored final refinements and runtime proof for the merged Issue 2037 containment work and merged the current-main implementation defining the fail-open and performance boundaries this pipeline extends. (github.com) (role: recent area contributor, reviewer, and merger; confidence: high; commits: be56cc9aff5e, cb6538b18d09, ab65dd331faf; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift, Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner+CacheHelpers.swift, Tests/CodexBarTests/CostUsageScannerBreakdownTests.swift)
  • Zihao-Qi: Authored the merged provenance-fixture and containment branch that documented the remaining cross-file family-ledger work and supplied canonical sanitized fork-family evidence. (github.com) (role: feature-history contributor; confidence: high; commits: 7465c6a11674; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift, Tests/CodexBarTests/Issue2037ScannerIntegrationTests.swift)
  • xx205: Authored merged parent-baseline and replay-accounting work that lineage accounting must preserve while extending behavior across fork families. (github.com) (role: introduced adjacent fork accounting behavior; confidence: high; commits: 45b68c34ec48; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift, Tests/CodexBarTests/CostUsageScannerBreakdownTests.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 #2156; closing this draft to keep the upstream review surface manageable.

@iam-brain iam-brain closed this Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. 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: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. 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