Skip to content

Feature/Fix: Bound lineage reconciliation work#2147

Closed
iam-brain wants to merge 8 commits into
steipete:mainfrom
iam-brain:iam-brain/lineage-performance-bounds
Closed

Feature/Fix: Bound lineage reconciliation work#2147
iam-brain wants to merge 8 commits into
steipete:mainfrom
iam-brain:iam-brain/lineage-performance-bounds

Conversation

@iam-brain

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

Copy link
Copy Markdown
Contributor

Stack dependency

This is slice 8 of 15. It follows Feature/Fix: Contain ambiguous lineage families; review and merge that PR first.

Summary

  • prepare deterministic, home-scoped lineage families with SHA-256 fingerprints
  • reuse unchanged family results through an immutable in-memory candidate cache
  • recompute only families affected by changed evidence or newly discovered parents
  • make cancellation and publication atomic while reporting honest structural peak metrics

This isolated scanner-rearchitecture component bounds reconciliation scratch state to the largest family. End-to-end scan memory still requires the follow-up two-pass discovery work tracked in #33.

Validation

  • swift test --filter CodexLineageEngineTests (8 passed)
  • make check
  • env SDKROOT=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX26.5.sdk make test (643 selections, 54 groups, no failures or retries)

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: 🚨 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
The PR adds home-scoped lineage discovery, deterministic family accounting, fingerprint-based reconciliation reuse, shadow comparison, residual classification, scanner integration, and focused tests.

Reproducibility: yes. at source level: the canonical issue, merged Phase 1 fixtures, and this branch's focused lineage tests establish the fork/interleaved accounting path. A failing current-main run against the reporter's private Ultra corpus was not reproduced here.

Review metrics: 3 noteworthy metrics.

  • Patch surface: 12 files, +2,763/-12. This is a large internal scanner rearchitecture despite being presented as one isolated stack slice.
  • New coverage structure: 5 production modules and 5 test suites added. The branch separates the lineage pipeline into focused seams and supplies substantial supplemental test coverage.
  • Stack position: slice 8 of 15. Its predecessor is unmerged and its successor owns completion of the stated end-to-end memory objective.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #2037
Summary: This PR is an active family-ledger implementation candidate for the canonical fork/interleaved Codex accounting issue, building on a merged containment phase and participating in a dependent implementation stack.

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 terminal or linked-artifact evidence from an affected real setup showing initial and cached repeat scans, expected totals, peak memory, cancellation, and unchanged-family reuse.
  • Refresh the branch after the predecessor lands and state whether this slice is intended to merge independently or with the two-pass successor.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR supplies tests and CI-style validation but no after-fix run from an affected real Codex history; add redacted initial and cached repeat-scan output with peak-memory/cancellation evidence, then update the PR body to trigger review or ask a maintainer for @clawsweeper re-review.

Risk before merge

  • [P1] Landing this slice independently can add lineage shadow work while the PR itself says total scan memory is not bounded until the two-pass successor, so large histories may still incur substantial memory or refresh-latency cost.
  • [P1] The 2,763-line branch is one middle slice of an active stack; merging or reviewing it out of order risks overlapping assumptions with the unmerged predecessor and successor.
  • [P1] Unit and fixture tests do not demonstrate cancellation, peak memory, unchanged-family reuse, or initial-versus-cached results on an affected real Codex history.

Maintainer options:

  1. Complete and prove the full memory bound (recommended)
    Review this slice with the two-pass successor and require redacted cold/warm scan output plus peak-memory and cancellation evidence before merge.
  2. Accept a staged performance boundary
    Merge after the predecessor and real runtime proof while explicitly accepting that discovery remains unbounded until the successor lands.
  3. Pause the stack
    Keep the draft unmerged if the intermediate shadow-scanning cost is not worth carrying before the complete pipeline is ready.

Next step before merge

  • [P1] Keep the draft in stack order, require real cold/warm scanner proof, and have the maintainer decide whether the incomplete memory-bound slice may land before its successor.

Maintainer decision needed

  • Question: Should this reconciliation-only slice land independently before the two-pass discovery successor completes the end-to-end memory bound?
  • Rationale: The code can be reviewed mechanically, but the PR explicitly leaves its user-facing performance objective incomplete until the next stack slice, so maintainers must choose the acceptable landing boundary.
  • Likely owner: steipete — They merged and validated the current-main Phase 1 containment and are best positioned to choose the acceptable staged landing boundary.
  • Options:
    • Review the memory-bound slices together (recommended): Keep this draft paired with Feature/Fix: Stream lineage families in two passes #2148 and land only after the combined cold/warm runtime proof shows an end-to-end bound.
    • Land this slice independently: Accept the temporary unbounded discovery path after the predecessor merges, provided real cold/warm scan proof shows no unacceptable availability regression.

Security
Cleared: The diff adds no workflow, permission, secret-handling, package-resolution, downloaded-code, or external execution change; lineage processing remains local to known Codex history roots.

Review details

Best possible solution:

Complete the family-ledger pipeline so discovery and reconciliation are bounded end-to-end, preserve the merged fail-open containment invariants, publish cache state atomically, and demonstrate accurate cold and warm scans on a redacted affected corpus.

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

Yes at source level: the canonical issue, merged Phase 1 fixtures, and this branch's focused lineage tests establish the fork/interleaved accounting path. A failing current-main run against the reporter's private Ultra corpus was not reproduced here.

Is this the best way to solve the issue?

Unclear as a standalone landing unit. The family-ledger and immutable-cache design is a plausible maintainable direction, but this slice does not yet prove the complete memory objective or real cold/warm behavior without its successor.

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 addresses materially incorrect local Codex usage accounting and scanner performance, but its blast radius is limited to local cost-history processing.
  • add merge-risk: 🚨 availability: The branch adds reconciliation and shadow-scanning work while explicitly deferring the complete end-to-end memory bound, leaving possible refresh latency or memory pressure on large histories.
  • 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 supplies tests and CI-style validation but no after-fix run from an affected real Codex history; add redacted initial and cached repeat-scan output with peak-memory/cancellation evidence, then update the PR body to trigger review or ask a maintainer for @clawsweeper re-review.

Label justifications:

  • P2: The PR addresses materially incorrect local Codex usage accounting and scanner performance, but its blast radius is limited to local cost-history processing.
  • merge-risk: 🚨 availability: The branch adds reconciliation and shadow-scanning work while explicitly deferring the complete end-to-end memory bound, leaving possible refresh latency or memory pressure on large histories.
  • 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 supplies tests and CI-style validation but no after-fix run from an affected real Codex history; add redacted initial and cached repeat-scan output with peak-memory/cancellation evidence, then update the PR body to trigger review or ask a maintainer for @clawsweeper re-review.
Evidence reviewed

What I checked:

Likely related people:

  • steipete: Merged and substantially refined the current-main Phase 1 containment in Contain Codex interleaved overcount and add fork provenance fixtures (#2037) #2082 and documented its accepted fail-open and cache-rebuild behavior. (role: recent area contributor, reviewer, and merger; confidence: high; commits: 0437067deaa8, be56cc9aff5e; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift, Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner+CacheHelpers.swift, Tests/CodexBarTests/CostUsageScannerBreakdownTests.swift)
  • iam-brain: Authored this lineage stack and previously contributed shipped cost-history and pricing work through pull requests 863, 884, and 917. (role: recent cost-accounting contributor and lineage feature owner; confidence: high; commits: 6170f5c8c745, 547567d51686, 1097b0a03571; files: Sources/CodexBarCore/Providers/Codex/CodexLineageEngine.swift, Sources/CodexBarCore/Providers/Codex/CodexLineageLedger.swift, Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift)
  • Zihao-Qi: Authored the merged Phase 1 branch that established the fork-provenance fixtures and conservative accounting invariants this lineage stack extends. (role: introduced current-main containment behavior; confidence: high; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift, Tests/CodexBarTests/Issue2037ScannerIntegrationTests.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 #2157; 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. 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