Skip to content

Feature/Fix: Profile and optimize shadow lineage accounting#2154

Closed
iam-brain wants to merge 16 commits into
steipete:mainfrom
iam-brain:iam-brain/lineage-performance-profiling
Closed

Feature/Fix: Profile and optimize shadow lineage accounting#2154
iam-brain wants to merge 16 commits into
steipete:mainfrom
iam-brain:iam-brain/lineage-performance-profiling

Conversation

@iam-brain

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

Copy link
Copy Markdown
Contributor

Stack dependency

This is slice 15 of 15. It follows Feature/Fix: Diagnose Codex branch-frontier collisions; review and merge that PR first.

Summary

  • add stage-level timing for lineage discovery, loading, reconciliation, and composition
  • eliminate repeated copy-on-write growth and timestamp parsing in family reconciliation
  • hash JSONL bytes during parsing so two-pass mutation checks need fewer full-file reads
  • add a bounded two-million-observation regression test

This is another isolated component of the planned scanner rearchitecture tracked in #10. It keeps lineage accounting in shadow mode and does not remove the legacy fallback.

Real-data validation

On the retained 570-document / 819,779-observation corpus, daily UTC totals stayed identical while lineage runtime fell from about 225s in the instrumented baseline to about 164s. Compared with the earlier roughly 360s replay, the current path is about 54% faster. Family reconciliation dropped from 72.4s to 15.2s; discovery and document loading remain the next performance constraints.

Tests

  • swift test --filter CodexLineageEngineTests --filter CodexLineageTwoPassDiscoveryTests --filter CodexLineageLedgerTests
  • two-million-observation regression test (29.5s in the full suite)
  • retained local UTC replay (exact daily totals preserved)
  • make check
  • make test (649 selections, 55/55 groups, no failures, retries, or timeouts)

Final UTC comparison

The figures below compare completed UTC days from July 9 through July 11. Positive percentages are overcounts versus OpenAI; negative percentages are undercounts.

UTC day Live app cache vs OpenAI Partial fix vs OpenAI New scanner vs OpenAI OpenAI official
2026-07-09 976,079,823 +14.47% 764,026,920 -10.40% 764,026,920 -10.40% 852,682,935
2026-07-10 7,456,347,992 +371.86% 1,438,632,374 -8.96% 1,498,664,841 -5.16% 1,580,199,588
2026-07-11 98,382,045,260 +7,708.15% 846,608,173 -32.81% 1,315,404,262 +4.40% 1,259,990,957
Aggregate 106,814,473,075 +2,792.45% 3,049,267,467 -17.43% 3,578,096,023 -3.11% 3,692,873,480

Sources and interpretation

  • Live app cache is the token total retained by the installed CodexBar cost-usage cache—the historical value the app could display—not a recomputed legacy scan. Both retained v8 and v9 caches agree for these days.
  • Partial fix is the scanner after the interleaved-cost containment fix already landed upstream.
  • New scanner is the latest lineage selector after all slices in this stack.
  • OpenAI official is the finalized UTC daily usage total from the account usage data used during validation.

This is materially closer to reality in aggregate, but it is not presented as exact. July 9 remains 10.40% low and July 11 remains 4.40% high, so legacy removal stays blocked behind further real-world validation and the rollback path remains intact.

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:24 PM ET / July 14, 2026, 01:24 UTC.

Summary
Adds a shadow-only Codex lineage accounting engine, discovery and diagnostic stages, reversible promotion gating, scanner performance optimizations, and extensive regression coverage.

Reproducibility: yes. at source and corpus level: the canonical investigation documents fork/interleaving overcounts, current main contains only the partial containment fix, and the PR reports a retained real-history replay. This review did not independently rerun the private corpus.

Review metrics: 4 noteworthy metrics.

  • Patch surface: 24 files, +5,623/-20. The final stacked PR exposes a broad scanner architecture for review even though its stated focus is profiling and optimization.
  • New lineage coverage: 10 source modules and 11 test files added. The implementation is heavily decomposed and accompanied by focused coverage across discovery, reconciliation, diagnostics, selection, and promotion.
  • Measured corpus: 570 documents, 819,779 observations. The submitted after-change replay exercises substantially more realistic data than unit fixtures alone.
  • Measured runtime: 225s to 164s. The reported instrumented-baseline improvement is about 27%, with family reconciliation falling from 72.4 seconds to 15.2 seconds.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #2037
Summary: This PR is a candidate architectural continuation of the canonical Codex fork-accounting investigation, building on a merged partial containment fix and a directly preceding stack slice.

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] This draft is slice 15 of a stacked series and explicitly depends on Feature/Fix: Diagnose Codex branch-frontier collisions #2153, so maintainers cannot assess the final isolated merge delta until the prerequisite lands and the branch is refreshed.
  • [P1] The submitted lineage totals are materially better in aggregate but still differ from the official source by -10.40% and +4.40% on individual validation days; the lineage authority must remain disabled by default.
  • [P1] Changing the parser hash can force an existing user's cached scanner state through a full rescan, and the PR does not present equally explicit fresh-install and cache-upgrade/cancellation proof for that path.

Maintainer options:

  1. Verify the upgrade path before landing (recommended)
    After the prerequisite merges, validate that parser-hash cache invalidation performs an atomic, cancellable rebuild while preserving legacy totals and settings.
  2. Accept a documented full-rescan upgrade
    Merge with legacy authority retained while explicitly accepting that existing installations may perform a potentially lengthy cache rebuild.
  3. Pause the final stack slice
    Leave the PR in draft if the review cost or upgrade rescan is not justified while lineage remains inaccurate on individual days.

Next step before merge

  • [P2] Wait for the prerequisite stack slice, refresh the branch, and have the scanner owners decide whether the shadow-only architecture and cache-upgrade cost are acceptable; there is no narrow mechanical defect for an automated repair lane.

Maintainer decision needed

  • Question: After Feature/Fix: Diagnose Codex branch-frontier collisions #2153 lands, should this final slice be accepted as shadow-only infrastructure despite remaining day-level lineage error and parser-cache invalidation?
  • Rationale: The implementation deliberately avoids changing the default totals, but accepting a large internal scanner architecture and its upgrade rescan cost is a maintainer ownership decision rather than a mechanical correctness question.
  • Likely owner: steipete — The canonical investigation, central scanner history, and prior Phase 1 merge all point to steipete as the best available owner for the architecture and rollout decision.
  • Options:
    • Land shadow-only after targeted upgrade proof (recommended): Refresh the branch after its prerequisite, confirm cache invalidation, cancellation, and legacy-default behavior on the exact head, then merge without promoting lineage authority.
    • Hold for broader accuracy validation: Keep the stack in draft until additional real histories reduce the remaining per-day discrepancies and discovery/loading constraints.

Security
Cleared: The diff adds no dependency, workflow, secret, permission, publishing, or downloaded-code changes, and its opt-in local validation is designed to emit aggregate-only output.

Review details

Best possible solution:

Land the stack only with legacy accounting still authoritative, lineage restricted to shadow diagnostics, and explicit cache-rebuild and cancellation validation; retain the promotion gates until broader day-level accuracy evidence supports changing authority.

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

Yes at source and corpus level: the canonical investigation documents fork/interleaving overcounts, current main contains only the partial containment fix, and the PR reports a retained real-history replay. This review did not independently rerun the private corpus.

Is this the best way to solve the issue?

Yes for an incremental rollout: shadow computation, a legacy fallback, atomic publication, cancellation checks, and explicit promotion gates are safer than immediately replacing production accounting. The final stack still needs exact-head upgrade-cache validation before merge.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P2: This is a substantial but shadow-only scanner correctness and performance improvement with legacy behavior retained as the default.
  • add merge-risk: 🚨 compatibility: The parser-hash change can invalidate existing cache state and trigger a full upgrade rescan, while an accidental lineage-authority promotion would alter displayed totals.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides after-change live replay results from a retained 570-document corpus, including preserved totals, stage timings, and full-suite results, which directly demonstrate the changed scanner behavior.
  • 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 retained 570-document corpus, including preserved totals, stage timings, and full-suite results, which directly demonstrate the changed scanner behavior.

Label justifications:

  • P2: This is a substantial but shadow-only scanner correctness and performance improvement with legacy behavior retained as the default.
  • merge-risk: 🚨 compatibility: The parser-hash change can invalidate existing cache state and trigger a full upgrade rescan, while an accidental lineage-authority promotion would alter displayed 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 retained 570-document corpus, including preserved totals, stage timings, and full-suite results, which directly demonstrate the changed scanner behavior.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides after-change live replay results from a retained 570-document corpus, including preserved totals, stage timings, and full-suite results, which directly demonstrate the changed scanner behavior.
Evidence reviewed

What I checked:

Likely related people:

  • steipete: Opened the canonical overcount investigation, authored central scanner history, and merged the prior Phase 1 containment change that this lineage stack extends. (role: scanner feature owner and merger; confidence: high; commits: 538872456251, 7d00af3ed8fd; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift, Tests/CodexBarTests/CostUsageScannerCodexTests.swift)
  • Zihao-Qi: Authored the merged Phase 1 interleaved-cost containment work in Contain Codex interleaved overcount and add fork provenance fixtures (#2037) #2082, making them directly connected to the current-main behavior this PR preserves. (role: recent area contributor; confidence: high; commits: be56cc9aff5e; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift, Tests/CodexBarTests/CostUsageScannerCodexTests.swift)
  • xx205: Reviewed the merged Phase 1 scanner correction and is a useful routing candidate for compatibility with that established behavior. (role: reviewer of adjacent scanner behavior; confidence: medium; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift, Tests/CodexBarTests/CostUsageScannerCodexTests.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