Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09cf7edb0f
ℹ️ 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".
|
Codex review: blocked before merge. Reviewed September 12, 2026, 1:51 AM ET / 05:51 UTC (Revision 60). ClawSweeper reviewWhat this changesRead completed Grok CLI turns, distinguish recorded spend from estimates, and include eligible OpenCodex OAuth usage in the spend dashboard. Merge readiness⛔ Blocked before merge - 3 items remain This remains useful work absent from main and v0.59.0. The prior overflow findings are resolved, and the supplied real behavior proof is sufficient; product acceptance remains outstanding. Priority: P2 Review scores
Verification
How this fits togetherCodexBar combines local usage logs and remote provider snapshots into menu, widget, and spend-dashboard summaries. This change updates Grok accounting and the optional OpenCodex log import feeding those summaries. flowchart TD
A[Grok CLI completed-turn logs] --> B[Bounded local scanner]
C[Recorded spend and price catalog] --> B
D[Optional OpenCodex logs] --> E[Reported OAuth attempt filter]
B --> F[Windowed usage and cost provenance]
E --> F
F --> G[Menu and spend dashboard]
Decision needed
Why: The owner supported completed-turn accounting, but the current proposal explicitly leaves acceptance of the revised dollar-display policy unresolved. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Adopt accurate completed-turn accounting with source-specific, non-bill disclosures under an explicitly approved default-display policy. Do we have a high-confidence way to reproduce the issue? Yes, source establishes that main sums context occupancy rather than completed-turn consumption; independent corpus reports corroborate the mismatch. This read-only review did not execute a reproduction. Is this the best way to solve the issue? Yes for the accounting implementation: bounded completed-turn parsing and recorded-cost precedence fit the evidence. Whether dollars should appear automatically remains a product choice. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against a998f677dd1f. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (59 earlier review cycles; latest 8 shown)
|
08360b5 to
e3cd3b9
Compare
|
Both automated findings are addressed, plus the review's other checklist items. The inline comments were left against P1 — Preserve the Grok fallback on repeated probe failuresFixed in if provider == .grok {
if self.tokenSnapshotPublicationForCurrentProviderConfig(for: provider) == nil {
Task { @MainActor [weak self] in
await self?.scanAndPublishGrokLocalTokenSnapshot(...)
}
}
} else if Self.tokenCostRequiresProviderSnapshot(provider) {
self.clearTokenSnapshot(for: provider)
}Regression coverage is in P2 — Refresh pricing before scanning Grok sessionsCorrect, and thank you — this was a genuine gap and not one the local tests would have surfaced. Fixed in Note the inline comment still points at Coverage: Real-session evidence
The same corpus on Those figures were cross-checked against an independent reimplementation of the pricing formula over the same logs; the two agree to the cent. Merge risk / branch stateRebased onto current One thing deliberately left undone: no |
|
Addressed both current findings in
Validation on the exact pushed head:
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Synced #3135 with main The generated parser-hash conflict is resolved by regeneration from the merged source ( Main's Linux cache fixes, stacked-chart rendering, provider/widget changes, and release metadata are preserved. The two Grok changelog entries now live under 0.56.8 Unreleased; published release sections match main. The evidence report also records that the producer contract landed through the attributed OpenCodex #3762 carry; its original captured bytes remain pinned and are not represented as a new capture of that carry. Validation on this final head:
The PR description now contains the final source, commands, and evidence. The default Grok dollar display remains an owner decision. Please review the final head and description. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Updated to Current-head validation: The default Grok dollar display remains an owner decision, and the prior maintainer change-request review still needs reconsideration. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@steipete The latest ClawSweeper review of |
|
Updated to
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Final validation for
The PR body is updated with the current source and validation. The default Grok dollar-display policy and the earlier maintainer changes-requested review still require the owner's decision. |
|
Synced with main The parser hash is regenerated as
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
I retrieved the landed OpenCodex carry directly through the GitHub API to clarify the dependency-inspection limitation in the latest review. #3762 is merged, with immutable merge commit
For content identity, the inspected schema/persistence blob is This verifies the landed source contract. The executable capture remains pinned to @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Final validation for
The PR body now has the final validation results and the immutable landed-producer source audit. The owner display-policy decision and older changes-requested review remain unresolved. The review environment's inability to retrieve the later producer carry is also recorded; the supplied source audit does not expand the pinned executable capture into a claim about a released producer version. |
|
Merged latest main The conflict resolution preserves upstream's checked numeric aggregation, parser-revision migrations, and Grok terminal-billing work gate while retaining completed-turn accounting, recorded-versus-estimated disclosure, custom-price precedence, and per-attempt OAuth attribution. The generated parser hash is Validation on this head:
The PR body records the current results and distinguishes them from the historical native-corpus proof. The owner display-policy decision remains open. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Addressed the newly reported native overflow finding in The native parser now validates NSNumber conversions, preserves unknown counts, and uses checked addition across model/day/window totals. Later valid records and cache rereads cannot clear an unknown aggregate. Explicit valid totals retain precedence, valid neighboring token classes and recorded spend remain available, and incomplete token accounting does not establish full coverage. The added production-scanner regression suite covers per-record and cross-record overflow, multiple days, nested models, malformed number types, explicit totals, snapshot projection, and cache reuse. Validation:
The PR body has the current results. The new CI run is pending; the owner dollar-display decision remains open. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Addressed the remaining menu-projection overflow finding in A checked, complete-count helper now covers the remote-backed Grok window projection, window request totals, comparison summaries, and menu/Widget fallback totals. Unknown daily values are retained as unknown instead of being dropped into a partial sum. This complements the native scanner repair in the preceding commit. The new end-to-end regression writes native completed-turn logs and exercises both remote-backed and fallback live consumers. It covers individually representable Int.max/1 totals across days and an already-unknown day followed by a valid day. The menu, Widget, and dashboard keep the aggregate unavailable; narrowing to the unaffected one-day window restores its known count. All four scenarios passed. Current-head validation: The owner's default dollar-display decision remains open. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Final author-side validation for
The PR body now has the final local validation and exact CI limitation. The default-dollar display policy and the owner's outstanding changes-requested review remain maintainer decisions. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |


Problem and resulting behavior
Grok's local fallback reads ending context occupancy from
signals.json, which is not completed-turn usage, and publishes no local cost. This PR reads completed turns from bounded native CLI logs and carries recorded-versus-estimated cost through the menu and Usage & Spend.session/updateand_x.ai/session/updaterecords inupdates.jsonl, with daily, model, and request breakdowns.usage.costUsdTicks / 1e10as the authoritative turn total. Count it once. Show nested model dollars only when all nested ticks exist and reconcile to that total; otherwise retain model tokens with unknown model dollars. Missing recorded cost falls back to disclosed public xAI list prices.Grok CLI-recorded spend, list price where unrecorded · not a bill.OpenCodex integration
With the existing Include OpenCodex usage logs switch enabled (off by default), Usage & Spend also includes reported physical Grok OAuth attempts. The producer contract originally proposed in OpenCodex #3642 has now landed in
devthrough the attributed carry #3762, merged asf00f2bcaea251ebe7ad4de9e38337b4be0ccee47. It writesattempts[].credentialSourcefrom the resolved upstream transport. Compatibility is verified against the pinned producer commit in the evidence below; availability in a released OpenCodex version is not claimed.The landed carry's immutable source contract has additionally been inspected directly, including resolved-adapter stamping, persistence normalization, and OAuth/API-key regression assertions. The executable capture remains pinned to its documented producer revision; this source audit does not claim a new capture or released-version coverage.
Only
provider: "xai",credentialSource: "grok-oauth"attempts with an upstream send and reported token usage qualify. API-key traffic, historic rows, locally answered requests, unknown sources, and estimated/unreported token counts stay excluded. Current configuration and top-level credential metadata cannot retroactively classify usage.Combo requests contribute each qualifying attempt's own token counts, never the parent aggregate. Duplicate request IDs are resolved before grouping; duplicate attempt ordinals are rejected. SQLite cache schema 3 retains attempt metadata and rebuilds older derived caches from the log. OpenCodex dollars use list prices and remain estimates, including when combined with native CLI-recorded spend. Missing token classes and unknown prices retain tokens without inventing a dollar value. The Grok menu continues to use native CLI logs.
Producer-to-dashboard evidence and review fixes
The committed evidence report and raw ledger fixture contain output from OpenCodex's unmodified production handlers and durable usage writer at
146ed679c9633e5d68726217fcadc8e0b107339b. Two localhost HTTP requests exercised OAuth 401 replay through Responses and native Chat API-key dispatch. Upstream responses and credentials used isolated fixtures; this is production-path capture evidence, not live vendor authentication or billing evidence. The capture helper rejects unexpected external fetches and is reproducible against that pinned checkout.CodexBar imports the exact captured bytes through the production disk loader, with no injected entries or loader closure, and reopens the persisted cache:
Standalone reports retain explicit xAI custom-price estimates from either the caller overlay or the application overlay, including known zero. Raw xAI records without an explicit price remain token-only, and the subscription fan-out still excludes API-key and historic records.
opencodeandopencode-freeretain their existing catalog and custom prices.Application overlays first match the original model name before the provider-qualified catalog name. Bare keys retain precedence when both keys exist; incomplete rates stay unknown, cached-input accounting is preserved, and caller-supplied custom pricing still takes precedence over the application overlay. New regressions cover these cases through the application-overlay parameter and the standalone disk/cache loader. Coverage includes known-zero overrides, incomplete rates, cache accounting, and both snapshot and application overlays.
Both async native Grok scan entry points propagate the executor's cancellation callback through discovery, JSONL reads, and aggregation. An in-flight regression cancels after parsing begins, observes
CancellationError, and confirms the next queued scan runs within one second. Cancelled partial parses are uncacheable and cannot establish complete history. The earlierc3919a224serial proof stopped at 4,874/40,000 decoded records and released the queue after 0.001864583 seconds. The evidence report also retains the initial measurement.Native numeric safety
Native token conversion rejects booleans, fractions, negative values, and out-of-range numbers without trusting NSNumber's clamping integer bridge. An absent token class retains the established zero default; a malformed count remains unknown. Explicit valid totals retain precedence, so decoding never adds input and output unnecessarily.
Checked addition carries unknown or overflowed values through model, day, and window aggregation, including after later valid records and cached-log rereads. Valid neighboring token classes and CLI-recorded spend remain available. Incomplete token accounting does not establish full history coverage; estimated pricing requires representable inputs. Production-scanner regressions cover Int.max plus one within a turn, sums across turns and days, nested models, invalid JSON number types, explicit totals, snapshots, and cache reuse.
The same checked, complete-count aggregation now covers remote-backed Grok menu projection, rolling-window requests, comparison summaries, and menu/Widget fallback totals. Unknown daily counts cannot be dropped into a plausible partial sum. An end-to-end scanner regression covers remote-backed and fallback projections for cross-day overflow and an unknown day followed by a valid day; menu, Widget, and dashboard totals stay unavailable, while narrowing to a valid one-day window restores its count.
Bounds and compatibility
Native scans run on the dedicated executor with limits of 64 MiB / 20,000 turns per file, 1 MiB per record, 256 sessions / 256 MiB / 100,000 turns per scan, and 4,096 discovery entries. The process cache retains at most 64 files or 50,000 turns. Cancellation and truncated history cannot publish complete coverage.
Merged main
8b254dbec11ddd5c5547878d9640e4e965306c71, retaining its checked token aggregation, daily spend ledger, native parser-revision migration, and Grok terminal-billing-failure work avoidance. The local-summary injection seam now invokes the PR's pricing-aware 365-day scanner only after the upstream billing/identity gate permits a snapshot. Failed billing still refreshes local spend through the app's existing independent fallback path.OpenCodex token counts retain upstream's safe numeric conversion and truncation policy. Attempt ordinals and send counts remain exact integers: booleans, fractional counts, and out-of-range values cannot qualify an OAuth attempt. Aggregation retains overflow as unavailable while keeping valid neighboring token classes and Grok estimate coverage. The upstream cursor parser version invalidates legacy numeric caches; schema 3 still preserves request-time attempt provenance.
Regenerated the native parser hash from the merged source:
a8559238a5fc0480. Current-main hashf5fdba377006d7beand prior-PR hashc3a879df4eff7187join the compatible predecessors. Stored rows and checkpoints are retained, while upstream's per-file parser revisions schedule bounded reparsing where needed. SQLite adoption tests cover the current-main and prior-PR hashes, and upstream whitespace/subagent migration tests remain in the validation set. The architecture gate keeps its exact provider-reference checks at their updated source locations. Grok release notes are under0.59.1 — Unreleased; all published sections match main.Validation
Head:
a41736133e7a2d922db7bbc5b0e18e7e729e855d.make checkpassed: zero violations in 2,199 Swift files.vendorMeteredprovenance and populated menu/dashboard disclosures. Empty 1-day/7-day windows retainunknown. The pinned producer ledger imports five OAuth tokens, excludes API-key traffic, and reopens its cache without log reads.make testpassed: 1,081 selections, 91/91 groups successful on the first attempt, zero failures/retries/timeouts (632.5 seconds execution; 634.5 seconds total). No code changed after validation.git diff --checkpassed.The supplemental proof and local-corpus measurements below were captured at
c3919a224. They remain historical, source-linked evidence; current-head validation is recorded separately above. The supplemental proof uses the repository’s serial execution mode because those suites share scanner-cache counters.The September 6 local-corpus proof retained
vendorMeteredfor populated windows: 7 days = 95,891,889 tokens / $12.57875572; 30 days = 98,631,812 tokens / $12.94957366. The empty 1-day window retainedunknown. Controlled JSONL fixtures also proved recorded-only and estimate-only windows after narrowing mixed source history.The OpenCodex regressions cover mixed OAuth/API-key/provider attempts, historical and malformed records, duplicate suppression, cache reopen/incremental append/schema upgrade, missing-price behavior, recorded-plus-estimated date filtering, and the dashboard's opt-in switch. Existing native regressions cover bounded parsing, outer/nested cost reconciliation, local fallback freshness, account isolation, and menu/dashboard provenance.
All ordinary tests suppress Keychain access and isolate provider files. The optional native proof reads local Grok logs and the cached pricing catalog without authentication, browser-cookie import, or live provider requests. Presentation evidence uses production menu/dashboard models; no app-bundle screenshot is claimed.
Maintainer decision
Please revisit the 2026-08-21 Grok cost ruling before merge. It preferred public-card pricing based partly on my incorrect
1e9divisor; #3345 established1e10, and the corrected measurement explains the difference. This branch uses recorded spend with public-card fallback.Whether existing Grok users should receive this disclosed dollar surface by default remains an owner decision. The accounting corrections and source labeling do not override that decision. Maintainer approval is still required.