Skip to content

Feature/Fix: Diagnose Codex branch-frontier collisions#2153

Closed
iam-brain wants to merge 14 commits into
steipete:mainfrom
iam-brain:iam-brain/lineage-branch-frontier-diagnostics
Closed

Feature/Fix: Diagnose Codex branch-frontier collisions#2153
iam-brain wants to merge 14 commits into
steipete:mainfrom
iam-brain:iam-brain/lineage-branch-frontier-diagnostics

Conversation

@iam-brain

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

Copy link
Copy Markdown
Contributor

Stack dependency

This is slice 14 of 15. It follows Feature/Fix: Diagnose Codex reset-epoch token collisions; review and merge that PR first.

Summary

  • add a bounded, family-local shadow analyzer that separates copied prefixes from equal numeric token snapshots after branch divergence
  • require resolved parent edges, prefix-compatible physical copies, independent divergence witnesses, and strong neighboring-flow evidence before classifying suffix collisions
  • emit aggregate-only diagnostics with overflow, cancellation, UTC, and one-million-observation family guards
  • keep the analyzer structurally disconnected from accounting rows, caches, and selector inputs

Real-world result

This theory also does not explain the remaining July 9 undercount. The frozen corpus resolved 365 parent edges and identified 314 shared-prefix fingerprints, but found zero ambiguous post-frontier collisions and zero estimated suppressed tokens on every day.

Selected totals remain unchanged. The negative result points the remaining July 9 gap toward discovery, parsing, or reference semantics rather than numeric fingerprint convergence after forks.

Tests

  • focused frontier/shadow/local-validation suites (8 tests passed before review; focused frontier suite passed after review fixes)
  • make check
  • opt-in frozen replay over 819,779 observations
  • make test (649 selections in 55 groups; 55 passed, 0 failures, 0 retries)

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: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. 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 an experimental Codex lineage-accounting subsystem with discovery, reconciliation, shadow comparison, authority selection, promotion gates, reset diagnostics, and branch-frontier collision analysis.

Reproducibility: yes. for the proposed diagnostic behavior: focused tests and the reported frozen-corpus replay exercise the analyzer, although the private corpus prevents an independent current-checkout reproduction here.

Review metrics: 4 noteworthy metrics.

  • Cumulative branch size: 23 files, +5,467/-19. The merge unit is far broader than the final diagnostic named in the title.
  • Commit stack: 14 commits. The branch includes all preceding lineage slices and must be reviewed in dependency order.
  • Real corpus replay: 819,779 observations. The aggregate-only replay provides meaningful evidence that the analyzer runs on a substantial real dataset.
  • Observed collisions: 0 ambiguous, 0 suppressed tokens. The diagnostic safely rules out this theory for the retained corpus but does not fix the remaining undercount.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #2037
Summary: The open accounting investigation is canonical; this PR is one experimental diagnostic candidate within its larger lineage stack, while the neighboring PRs cover preceding collision analysis and subsequent performance work.

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:

  • [P2] Merge the required predecessor and rebase this PR so its branch-frontier diagnostic is the isolated review diff.

Risk before merge

  • [P1] Merging this head would land the entire 14-commit experimental lineage-accounting stack, not merely the final diagnostic described by the PR title.
  • [P1] The lineage authority can materially change token totals when enabled; the reported corpus remains 10.40% low on July 9 and the broader stack therefore is not ready to replace the legacy path.
  • [P1] The branch explicitly depends on an open predecessor, so reviewing or merging it before the stack is narrowed can obscure which slice introduced a regression.

Maintainer options:

  1. Narrow the stacked diff first (recommended)
    Merge the required predecessor, then rebase this branch so reviewers can validate the branch-frontier analyzer independently from the preceding architecture.
  2. Land the cumulative subsystem
    Accept that this head introduces the full experimental lineage stack and require explicit owner review of the accounting and rollback boundaries.
  3. Pause the lineage stack
    Close or defer the stack if the ongoing maintenance cost is not justified by a diagnostic that produced a negative result.

Next step before merge

  • A repository owner should review the predecessor first and decide whether to sponsor the broader shadow-lineage architecture before this cumulative draft is considered for merge.

Maintainer decision needed

  • Question: Should CodexBar adopt this experimental lineage-accounting stack in core while retaining legacy authority, or should the investigation remain external until its residual errors are better understood?
  • Rationale: The code is deliberately reversible and well tested, but it introduces a substantial second accounting architecture whose eventual promotion criteria and maintenance cost require repository-owner intent.
  • Likely owner: steipete — Recent merge history and repository ownership make steipete the best available decision owner for introducing a second accounting architecture.
  • Options:
    • Sponsor the shadow stack (recommended): Review and merge the predecessor slices in order, rebase this PR to a narrow diagnostic diff, and keep legacy authority as the production default.
    • Keep investigation external: Pause the core stack and retain the corpus analyzer as an external investigation tool until the remaining reference mismatch has a clearer cause.
    • Accept cumulative landing: Merge the current cumulative branch as one unit and explicitly own the broader review and rollback burden.

Security
Cleared: The diff adds no dependency, workflow, permission, credential, or downloaded-code changes, and the new diagnostics intentionally expose aggregate counts rather than rollout identities or contents.

Review details

Best possible solution:

Keep legacy accounting as the default, land and rebase the stack in dependency order so this PR exposes only its diagnostic slice, and retain the analyzer as aggregate-only shadow evidence until broader lineage accuracy and rollback gates are approved.

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

Yes for the proposed diagnostic behavior: focused tests and the reported frozen-corpus replay exercise the analyzer, although the private corpus prevents an independent current-checkout reproduction here.

Is this the best way to solve the issue?

Unclear as a merge unit: the isolated aggregate-only analyzer is a conservative investigation tool, but the cumulative branch should be rebased after its predecessor lands so the final slice can be reviewed independently.

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 accounting improvement and diagnostic with meaningful accuracy value, but legacy remains available and no current production outage is shown.
  • add merge-risk: 🚨 other: Merging this head would land a 14-commit experimental accounting subsystem rather than the narrow diagnostic suggested by the PR title.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body reports after-change live output from an opt-in frozen replay over 819,779 observations, including resolved-edge, fingerprint, collision, suppression, and unchanged-total results.
  • 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 reports after-change live output from an opt-in frozen replay over 819,779 observations, including resolved-edge, fingerprint, collision, suppression, and unchanged-total results.

Label justifications:

  • P2: This is a bounded accounting improvement and diagnostic with meaningful accuracy value, but legacy remains available and no current production outage is shown.
  • merge-risk: 🚨 other: Merging this head would land a 14-commit experimental accounting subsystem rather than the narrow diagnostic suggested by the PR title.
  • 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 reports after-change live output from an opt-in frozen replay over 819,779 observations, including resolved-edge, fingerprint, collision, suppression, and unchanged-total results.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body reports after-change live output from an opt-in frozen replay over 819,779 observations, including resolved-edge, fingerprint, collision, suppression, and unchanged-total results.
Evidence reviewed

What I checked:

Likely related people:

  • steipete: Merged the recent interleaved-cost containment change and is the strongest available routing candidate for deciding whether the broader lineage-accounting stack should enter core. (role: merger and recent scanner decision owner; confidence: high; commits: f5a6934; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift)
  • xx205: Implemented the recent current-main interleaved Codex cost containment behavior that this lineage stack extends and compares against. (role: recent accounting-path contributor; confidence: high; commits: 3a91eb3; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift, Tests/CodexBarTests/CostUsageInterleavedCodexTests.swift)
  • Zihao-Qi: Recent merged work on scanner cancellation and traversal behavior makes this person relevant to the new discovery, cancellation, and bounded-reconciliation paths. (role: adjacent scanner contributor; confidence: medium; commits: 2c51cd3; 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.

@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: 🚨 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: 🐚 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