Skip to content

fix: detect semantic Codex cost catch-up progress - #2844

Draft
pavbar wants to merge 2 commits into
steipete:mainfrom
pavbar:codex/fix/codex-cost-catch-up-progress
Draft

fix: detect semantic Codex cost catch-up progress#2844
pavbar wants to merge 2 commits into
steipete:mainfrom
pavbar:codex/fix/codex-cost-catch-up-progress

Conversation

@pavbar

@pavbar pavbar commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Prevent Codex cost catch-up from repeatedly running when only an active, already-complete session continues to append, while preserving progress for finite bounded backlog work.

The progress fingerprint now tracks:

  • stable file membership and completion state
  • completed-file aggregate progress
  • unfinished file byte and JSONL resume offsets
  • session discovery and resumable head-scan cursors
  • active lookback cursors
  • buffered fork dependency transitions

Completed live appends and repeated growth of a fork blocked on the same missing parent no longer look like finite catch-up progress.

The catch-up schedulers also remember semantic progress states seen during the current run. This preserves bounded cursor progress while stopping a repeated validation cycle when its cursor returns to an earlier state.

Root cause

The catch-up scheduler used cached file byte changes as its no-progress signal. Active Codex sessions can append between bounded passes, so an unrelated unresolved dependency could remain pending while those live bytes kept the worker awake. At the same time, some real bounded work advances through semantic cursors or completion aggregates without changing the previously fingerprinted bytes.

This change fingerprints finite backlog state instead of ordinary completed-file growth. Repeated semantic states are detected across the whole catch-up run rather than only between adjacent passes.

Context

Refs #2815.

This addresses the native semantic-stall path and is complementary to #2834, which fixed the separate #2824 scheduling loop.

Validation

Final branch head:

  • make check
    • 0 violations across 1,840 Swift files
  • make test
    • 837 selections
    • 70/70 groups passed
    • zero failures, retries, or timeouts
  • focused progress-key helper: 7/7 passed
  • primary catch-up scheduler: 6/6 passed
  • Spend Dashboard catch-up scheduler: 5/5 passed
  • provider architecture gate: 38/38 passed

Deterministic regressions cover:

  • finite completed-file backlog progress
  • unfinished-file byte and JSONL resume progress
  • completed active appends not becoming false backlog progress
  • unresolved fork appends not masking a stalled dependency
  • bounded discovery-head progress
  • a validation cursor returning to an earlier semantic state in both catch-up schedulers

Private local corpus contents and runtime artifacts are intentionally not published. The public verification in this PR is limited to source, synthetic fixtures, and reproducible repository test results.

@clawsweeper

clawsweeper Bot commented Aug 10, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@clawsweeper clawsweeper Bot added merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. 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. labels Aug 10, 2026
@clawsweeper

clawsweeper Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed August 10, 2026, 2:58 PM ET / 18:58 UTC.

ClawSweeper review

What this changes

The PR changes Codex cost catch-up to track semantic scanner state and stop a scheduler when it revisits a prior pending state.

Merge readiness

Blocked until real behavior proof from a real setup is added - 4 items remain

This PR remains necessary: current main and v0.49.1 still use byte changes as the catch-up progress signal. The revised recurrence handling addresses the prior review concern and has focused synthetic coverage, but merge should wait for redacted real-cache after-fix proof.

Priority: P2
Reviewed head: b1d5c1109938442b3e4aab84d9d16df5d0089697

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The patch and focused tests are coherent, but merge readiness is capped by missing production-boundary proof.
Proof confidence 🦪 silver shellfish (2/6) Needs real behavior proof before merge: The PR reports synthetic fixtures and repository tests, but contains no inspectable after-fix run from a real local Codex cache. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The PR reports synthetic fixtures and repository tests, but contains no inspectable after-fix run from a real local Codex cache. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 5 items Current main still has the byte-only signal: At current main, the progress key hashes parsed bytes and file size for every scoped file, so completed live appends can continue to look like catch-up progress.
Released behavior is unchanged: v0.49.1 contains ae1111e, whose CostUsageFetcher implementation also hashes parsed bytes and size in the catch-up key.
Patch handles the reviewed cursor cycle: The PR head seeds a set of seen progress keys and pauses when a pending status recurs; its added normal and dashboard scheduler tests cover validation-0 → validation-1 → validation-2 → validation-0.
Findings None None.
Security None None.

How this fits together

CodexBar scans local Codex JSONL session history in bounded passes and persists progress in its cost cache. The normal refresh path and Spend Dashboard consume that cache to decide whether history coverage is complete or catch-up should continue.

flowchart LR
A[Local Codex session files] --> B[Bounded cost scanner]
B --> C[Persisted cost cache]
C --> D[Semantic progress state]
D --> E[Normal catch-up scheduler]
D --> F[Spend Dashboard scheduler]
E --> G[Usage coverage and status]
F --> G
Loading

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The PR reports synthetic fixtures and repository tests, but contains no inspectable after-fix run from a real local Codex cache. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Resolve merge risk (P1) - The new recurrence cutoff controls whether local history remains pending; without a real-cache run, an edge case could pause legitimate catch-up or fail to stop a live append loop.
  • Resolve merge risk (P1) - The branch predates current main's idle cost-scan work, so validation should be refreshed against the current base before merge.
  • Complete next step (P2) - Needs contributor-provided real behavior proof and refreshed validation on the current base; no narrow code defect was found in the revised patch.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production versus test delta production +80/-13, tests +361/-0 across 6 files Most of the patch is targeted regression coverage, while the production change alters the termination signal for two catch-up schedulers.

Merge-risk options

Maintainer options:

  1. Add real cache proof (recommended)
    Before merge, provide a redacted terminal capture, runtime log, or recording from a real Codex cache showing the repeated-state path pauses while finite catch-up still completes.
  2. Keep the draft pending validation
    Leave this PR in draft until production-boundary evidence can confirm the new termination rule.

Technical review

Best possible solution:

Retain the semantic-state design and land it only after a redacted real Codex-cache run demonstrates both finite completion and safe pause behavior.

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

Unclear: the source and synthetic regressions establish the intended cycle, but no reusable real Codex-cache capture demonstrates the failure and after-fix behavior on current main.

Is this the best way to solve the issue?

Yes for the bounded-state problem: semantic cursors plus recurrence detection are narrower than treating every live byte append as progress, pending real-cache validation.

AGENTS.md: found and applied where relevant.

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

Labels

Label justifications:

  • P2: This is a bounded Codex cost-history scheduling bug with meaningful but not yet production-proven user impact.
  • merge-risk: 🚨 other: A changed semantic termination condition can affect whether local history continues catching up or pauses.
  • 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 reports synthetic fixtures and repository tests, but contains no inspectable after-fix run from a real local Codex cache. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

Likely related people:

  • Peter Steinberger: Current main is headed by Peter's idle-cost-scan optimization, and current scheduler/source attribution is at Peter's checked-out history boundary. (role: recent cost-scan contributor; confidence: high; commits: d7934a5f6121, ae1111e39912, 17afef7b84eb; files: Sources/CodexBar/UsageStore+CodexCostCatchUp.swift, Sources/CodexBar/UsageStore+SpendDashboardCodexCostCatchUp.swift, Sources/CodexBarCore/CostUsageFetcher.swift)
  • xx205: Commit 2920019 introduced the nearby bounded Codex catch-up fork-accuracy work. (role: bounded catch-up and fork-history contributor; confidence: medium; commits: 2920019bc16d; files: Sources/CodexBarCore/Vendored/CostUsage/CostUsageScanner.swift)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Add a redacted terminal capture, recording, or runtime log from a real Codex cache that demonstrates the after-fix result.
  • Refresh validation against current main after the recent idle cost-scan changes.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

History

Review history (2 earlier review cycles)
  • reviewed 2026-08-10T15:14:32.174Z sha a78e5eb :: needs real behavior proof before merge. :: none
  • reviewed 2026-08-10T17:17:53.517Z sha b1d5c11 :: needs real behavior proof before merge. :: none

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a78e5ebbce

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Sources/CodexBarCore/CostUsageFetcher.swift
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. 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