Price unreported Cursor events and show real spend coverage - #3129
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9fb5d824e6
ℹ️ 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: needs real behavior proof before merge. Reviewed August 27, 2026, 6:31 AM ET / 10:31 UTC. ClawSweeper reviewWhat this changesThis PR estimates Cursor events whose API cost is absent, keeps invalid reported costs unpriced, preserves compatible local caches, and corrects spend-history coverage in the Overview card. Merge readiness⛔ Blocked until real behavior proof from a real setup is added - 3 items remain Keep open: the current main baseline does not contain the requested fallback, and the active owner integration has resolved the prior source findings. The remaining merge blocker is real Cursor behavior proof, because the submitted captures are synthetic XCTest renderer output rather than an observed Cursor session. Priority: P2 Review scores
Verification
Live VerificationCommand: Result: FAIL (failed) — execution before step 1 Assertions:
How this fits togetherCursor usage events are decoded into daily usage and cost reports, then combined into the app’s spend dashboard. This change adds a local list-price fallback before those reports feed the Overview summary. flowchart LR
A[Cursor usage events] --> B[Cost state decoder]
B --> C{Was cost reported?}
C -->|Missing or null| D[Local API-price catalog]
C -->|Reported or invalid| E[Daily usage report]
D --> E
E --> F[Spend overview card]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Land the bounded local-price fallback after a redacted real Cursor-session capture demonstrates an omitted or null cost flowing to the corrected spend output. Do we have a high-confidence way to reproduce the issue? No for live behavior: source and fixtures clearly exercise the new path, but no real Cursor session with an omitted or null cost is evidenced. Is this the best way to solve the issue? Yes structurally: the fallback is local, preserves reported and invalid values, and has focused coverage; live vendor-response proof is still needed before merge. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 8b1b9668e946. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (11 earlier review cycles; latest 8 shown)
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7eed14f69f
ℹ️ 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".
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45160c53d3
ℹ️ 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".
45160c5 to
bd3b0be
Compare
|
@clawsweeper re-review |
|
🦞👀 Re-review progress:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 380c1e5807
ℹ️ 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".
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Cursor emits claude-4.5-sonnet while bundled Claude catalog keys on claude-sonnet-4-5; map alias before fallback pricing so catalog estimates are not silently missed. When the same model has both valid and rejected (negative/non-finite) costs, aggregate costUSD fails closed but per-event priced coverage must still be visible; track per-event pricedRequests and expose via pricedRequestCount to keep coverageCounts honest.
Decode totalCents into a typed Cost state (valid, omitted, invalid) so present non-finite (NaN, Infinity), negative, and malformed strings are distinguished from genuine omissions or nulls. Only genuinely omitted/null totalCents are eligible for list-price estimation; invalid reported values stay unpriced, fail closed for the model aggregate, and never manufacture estimates. Keep chargedCents and Cursor-metered totals untouched. Update list-price helper documentation to note cached and bundled catalog lookups without network access. Add comprehensive tests decoding JSON through CursorUsageEventsPage and makeDailyReport covering omitted, null, zero, valid string, negative, non-finite, and malformed costs mixed with valid events of the same model.
3fa975a to
dd30637
Compare
|
@clawsweeper re-review |
|
🦞👀 Re-review progress:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd3063777a
ℹ️ 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".
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85257dec9e
ℹ️ 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".
|
Thanks @Yuxin-Qiao. I rechecked I’m taking the remaining bounded integration work on this PR after the current test-only cleanup lands: preserve unchanged native Codex and Pi/OMP caches across the optional report-field hash transition, resolve the cached pricing catalog once per report with deterministic fixtures, correct the dated dotted Claude alias ordering, and complete decoded-input/cache/Overview rendering proof. The estimate remains API list price; Cursor-metered charges and account/auth behavior stay unchanged. This is a source-review acknowledgment, not a claim that the current head has passed local execution or is ready to merge. I’ll update the branch and final proof here before landing. |
Integrate current main and complete catalog-once resolution, date-safe Claude aliases, decoded cost-state coverage, and compatible retained-report adoption. Keep Cursor API list-price estimates separate from metered billing and native custom pricing, with production Overview rendering and cache regressions. Co-authored-by: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com>
Keep Codex local history and Pi/OMP caches compatible when report gains optional pricedRequestCount field. Add predecessor hash 55f640e6bb0ccba4 to CostUsageStore.compatiblePredecessorParserHashes and allow PiSessionCostScanner to match pricingKey for new hash 21f10143afe00c55 via compatible key c6c46a376ba16304, avoiding unnecessary rebuild of unrelated Codex history.
f5810c4 to
7bf75c9
Compare
Preserve the contributor's parallel cache-adoption commit and the complete tested integration. Keep both audited Pi/OMP predecessor keys, along with native cache adoption and the existing regression coverage. Co-authored-by: Yuxin Qiao <104957188+Yuxin-Qiao@users.noreply.github.com>
|
Thanks @Yuxin-Qiao — I incorporated your concurrent The complete tree is byte-identical to the repair that passed 942 selections across 79 groups, with no failed groups, retries, or timeouts. Focused decoding/cache/publication tests, format/lint, independent review, and the production Overview light/dark plus accessibility captures are complete; the PR body contains the evidence and its nonlive limitations. I verified and resolved the 11 inline findings against that implementation. The earlier CI run was canceled by the concurrent branch update, not a test failure. I’m owning the final landing pass on the new exact-head CI run. Please leave this head stable while it completes; I’ll address any new actionable findings before merging. |
steipete
left a comment
There was a problem hiding this comment.
Reviewed 8ad662c3e021368e54ca9a8a1074b72b351a0d0f. The original missing/null-versus-invalid decoder finding is addressed, along with report-scoped catalog resolution, dated aliases, valid request coverage, and compatible native/Pi cache adoption. Cursor estimates remain catalog API list prices, separate from metered charges and native custom pricing. All 11 inline findings were checked against the implementation and resolved.
The integrated tree matches the independently reviewed and locally tested repair byte-for-byte: 277 focused tests, 39 architecture tests, 942 selections/79 full-suite groups with no retries/timeouts, and clean format/lint. Before/after production-card and hosted-accessibility proof is attached; authenticated provider/menu/VoiceOver behavior was not claimed. Exact-head CI passed all eight jobs, including both macOS shards and Linux x64/ARM64/musl; GitGuardian passed too. No unresolved review threads remain.
The automated exact-head rereview also reports no actionable code or security findings. Its remaining request is a naturally occurring authenticated Cursor example with an omitted cost. For this bounded local report transformation, I accept the decoded-input, cache-upgrade and production-view proof: transport/auth/pagination and metered billing are unchanged, and the report/estimate claims remain explicitly limited to those tested inputs. This is not a claim that a live account probe was performed. The bot's attempted test stopped in its pnpm setup before Swift execution; it does not contradict the completed local tests or exact-head CI.
|
Landed as 5b8602981baac73a52228e266084586a2c2b7b1a after verifying exact head Verification used
The 2,000-event fixtures prove at most one catalog metadata read per report, including a missing catalog. Retained native reports and both audited Pi/OMP predecessors hydrate without unnecessary reparsing, while actual pricing changes still invalidate. Mixed invalid/estimated/reported events preserve honest coverage. No real account, credential, browser import, authenticated provider request, app relaunch, live menu, or VoiceOver test was used. The Overview proof is the actual production view fed by synthetic report/publication data; the dated alias is a deterministic regression, not a claimed wire incident. Metered Cursor billing is unchanged. After merging, |
Summary
Estimate Cursor events only when the vendor genuinely omits or nulls the cost. Explicit zero remains reported; malformed, nonfinite, negative, wrong-type, or overflowing costs remain unpriced. Estimates use existing cached/bundled API list prices, never Cursor-metered billing or Codex custom pricing.
This preserves @Yuxin-Qiao's typed cost-state repair and contributor commits, and completes the integration with current main:
Docs and the Unreleased changelog are updated. Auth, fetching/pagination, charged cents, metered totals, provider ownership, and pricing data are unchanged.
Verification
Integrated head:
8ad662c3e021368e54ca9a8a1074b72b351a0d0f. This preserves all 13 contributor commits, including the concurrent cache-adoption update, and has a byte-identical Git tree to the fully tested/reviewedf5810c4ed0b33e03b39b4d1ec2fcaffe750b9420repair. The overlapping two-file update was reconciled without a force push; both audited Pi/OMP predecessors remain covered.All tests/checks unset
CODEXBAR_ALLOW_TEST_KEYCHAIN_ACCESSand setCODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 CODEXBAR_TEST_CODEX_FILE_ISOLATION=1; no real credentials, accounts, browser imports, or live provider requests were used.swift test --skip-build --filter ProviderArchitectureGatekeeperTests: 39 passed.make check: passed, zero violations in 2,013 Swift files.make test: 942 selections / 79 groups passed first time, zero failures, retries, or timeouts; 812.3 seconds including discovery.The 2,000-event known/unknown fixture measures one catalog metadata read per report with either a cached or absent catalog, and zero with an injected catalog. Reported/invalid/rejected-token inputs also measure zero. Decoded JSON covers omission/null/zero, ten invalid-value spellings, mixed same-model ordering, and coverage through the actual report/snapshot path.
Native retained JSON and Pi/OMP fixtures prove adoption of both audited predecessor hashes without rebuilding/reparsing. Changed catalog/custom-pricing/formula inputs and unrelated parser hashes still invalidate and reparse. This optional-field change does not alter pricing formulas or the cache schema.
Before / after: production Overview
The same fixed synthetic Cursor event, actual report-to-snapshot-to-publication path, production card view, and hosted accessibility export are used in both captures. One Cursor subscription has $12 / 1,000 tokens and 30 established history days; another selected subscription is unavailable. The before source is exact main
8b1b9668e946a71ad9d6e98f6a53102e1488159f; only the production Overview implementation changes for the after capture. Both inputs are byte-identical.Before: incorrect
Coverage: 0 / 30despite established history.After:
Coverage: 30 / 30;~$12.00,~1K tokens, and1 of 2 subscriptions have spendremain unchanged. Both light/dark images were inspected and the required text is present in hosted accessibility output.Render command:
swift test --filter MenuLayoutScreenshotRenderTests.test_renderCursorOverviewCoverageProofwith an isolatedCODEXBAR_CURSOR_OVERVIEW_SCREENSHOT_DIR; the before capture additionally sets the expected-days assertion to zero. Seven-day and unestablished-history controls pass through the real model tests.This is deterministic production-view and hosted-accessibility proof, not a claim of authenticated Cursor, live menu routing, or VoiceOver testing. The dated alias is a tested transformation, not a claimed observed wire incident. API list-price estimates are not actual Cursor invoices.