Show a 0% Grok usage bar for a period with no usage yet - #3294
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
clawsweeper
Bot
added
merge-risk: 🚨 other
|
Codex review: needs maintainer review before merge. Reviewed August 30, 2026, 10:21 PM ET / August 31, 2026, 02:21 UTC. ClawSweeper reviewWhat this changesThe PR lets Grok’s fallback display a 0% usage bar for the parser’s no-usage-yet billing frame, with regression tests, opt-in live proof, documentation, and release notes. Merge readinessKeep this PR open for a maintainer policy choice. The previous documentation blocker is fixed and the real-account proof is sufficient, but the new exception deliberately treats an indistinguishable absent field as 0%, reversing the fail-closed behavior established for missing usage data. Priority: P2 Review scores
Verification
How this fits togetherCodexBar combines a Grok CLI credits response with a best-effort Grok billing fallback, then turns the resolved snapshot into a menu-bar usage card. This change determines whether an absent percentage in a constrained billing frame becomes a visible 0% bar. flowchart LR
A[CLI credits proxy] --> C[Usage resolver]
B[Grok billing fallback] --> C
C --> D{Published percent or inferred zero?}
D --> E[Resolved usage snapshot]
E --> F[Menu-bar usage card]
Decision needed
Why: The parser can narrowly identify the observed fresh-period frame, but proto3 omission makes an actual zero and a withheld percentage indistinguishable at the protocol boundary; code and tests cannot resolve the desired user-facing truthfulness policy. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Preserve one explicit fail-closed policy for ambiguous provider data; the safer current direction is to retain unknown usage and replace the alarming unavailable message with neutral reset-period text unless a maintainer accepts the visible-zero inference. Do we have a high-confidence way to reproduce the issue? Yes—source-reproducible with high confidence: a period-only CLI-proxy snapshot plus the parser’s inferred-zero billing frame reaches the changed resolver guard and produces a primary usage bar. The supplied live-account transcript and inspected card corroborate that exact branch without requiring reviewer-side credential access. Is this the best way to solve the issue? No: the implementation is narrow and well-proven for the observed frame, but it is not unambiguously the best product solution because the provider protocol cannot distinguish zero from withholding a value; neutral unknown-usage text is safer unless maintainers choose otherwise. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 5a18e8ee9dc7. LabelsLabel 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
History |
grok.com's credits frame carries the usage percentage as a proto3 scalar, so a period whose usage is exactly zero omits it from the wire. The parser already reads that shape as 0 when the frame still carries a live period and no percentage anywhere, and marks it as not wire-published. The enrichment gate added in steipete#3181 refused that reading, so every freshly reset period fell back to the credits proxy's unknown percent, produced no primary rate window, and rendered the explicit "usage unavailable" diagnostic instead of a bar. Adopt a grok.com percent when it is wire-published or exactly zero. The credits proxy never produces an inferred value, so the fabricated 0% steipete#3157 removed cannot return through this path; the adopted zero keeps its not-wire-published marker as it travels; and any other inferred value is still refused, with a regression pinning that. Proof is rendered from the real fetch types by a gated developer test, so the committed before/after cards cannot drift from the shipped behavior.
ClawSweeper's review of the fixture-only evidence asked for a real-account result from a newly reset period. Add an opt-in proof that drives the shipped path against the live surfaces and renders the resulting card with personal information hidden. The test reads the bearer from ~/.grok/auth.json, the same file the provider reads, and touches neither the Keychain nor browser cookies. It prints aggregate fields only, and asserts the resolver's contract on whichever branch the live account exercises, so it stays meaningful once the period records usage.
1b8df5e to
31ced6b
Compare
|
Added the real-account proof the review asked for, and rebased onto Real behavior, from a live account on a freshly reset periodThis is the shipped On the remaining P1The exact-zero exception cannot be made unambiguous in code: a proto3 scalar omits an exact zero, so "0%" and "not published" are the same bytes. What the live run adds is that a real account on a fresh period does produce exactly the shape the parser requires, and that the resolver turns it into a bar rather than the failure message. Two supporting facts are in the PR body: this same account and surface published If you would rather not carry the inference at all, the conservative alternative is in the PR body: keep usage unknown, and replace the red failure text with a neutral "no usage reported for this period yet" line plus the reset time. That keeps main's semantics and removes only the alarm. Gate on this head
|
clawsweeper
Bot
added
proof: sufficient
docs/grok.md still described the pre-change rule: only a wire-published percentage adopted, and the parser's no-usage-yet zero explicitly rejected. Describe what the resolver now does, why the frame cannot publish an exact zero, what stays refused, and that the reading is an inference from frame shape rather than a value the surface stated.
|
Fixed the actionable finding from the 2026-08-31 re-review. Exact head P2 — the Grok fallback contractYou were right that the documentation promised the opposite behavior, and I had missed it. Two adjacent passages are reconciled with it: the parser section, which already documented the omitted proto3 scalar as zero usage, now says the retry adopts that reading; and the "SuperGrok Heavy with no That covers both the P2 finding and the P2 merge risk about conflicting guidance. Still yours to decideThe P1 is unchanged and not closable in code: the wire cannot distinguish an exact zero from a withheld percentage. Your recommendation was to retain unknown usage and replace the alarming copy with neutral reset-period text. I am happy to swap this PR to that shape — it is a smaller change than what is here, and the live proof in the body applies to it just as well, since it records what each surface actually returned. Say which you prefer and I will push it.
|
clawsweeper
Bot
added
rating: 🐚 platinum hermit
|
The zero-usage fix landed in #3325 as e7d0011, with parser provenance and active-period validation preserving unknown results for malformed or inactive payloads. This PR's production change is covered there, so I am closing it as superseded. @olddonkey's contribution is preserved in the squash commit co-author trailer and changelog thanks, alongside @sf-jin-ku. The integrated fix passed 91 focused Grok tests and all nine CI checks, including both full macOS shards: https://github.com/steipete/CodexBar/actions/runs/33475470989. Thank you for the implementation and reproduction proof. |
2 participants
Problem
A Grok billing period that has not recorded usage yet renders no usage bar at all. The card falls back to the explicit failure message instead:
The cause is on the wire. grok.com's credits frame carries the usage percentage as a proto3 scalar, so a period whose usage is exactly zero omits the field entirely — "zero" and "not reported" are byte-identical.
GrokWebBillingFetcheralready handles that: when the frame carries a live period and no percentage anywhere, it reads the value as0and marks itusedPercentIsWirePublished = false.The enrichment gate added in #3181 then refused that reading, because promoting an inferred percent is what rebuilt the fabricated 0% that #3157 removed. So the resolver fell back to the credits proxy's unknown percent,
toUsageSnapshot()built no primary rate window, and the card showedusageUnavailableMessage. Every account sees this from the moment its period resets until its first request lands.What this changes
One guard in
GrokOAuthFetchStrategy.resolvingUnknownUsage:Three properties keep #3157's fabricated zero from returning through it:
nil(GrokCreditsProxyFetcher), so the surface that caused Grok: period-only CLI-proxy response reports 0% after Grok Build says free usage limit reached #3157 is untouched.completing(with:)preservesusedPercentIsWirePublished = false, so no downstream consumer mistakes it for a published reading.Real-account proof
Produced on this head against a real SuperGrok account whose weekly period had just reset, using the shipped fetchers and the shipped resolver. Redacted transcript:
The card rendered from that live result, with personal information hidden:
GrokZeroUsageLiveProofTestsreads the bearer from~/.grok/auth.json— the same file the provider reads — and touches neither the Keychain nor browser cookies. It makes one request per surface, prints aggregate fields only, and asserts the resolver's contract on whichever of the four branches the account exercises, so it stays meaningful (and green) once the period records usage.The fixture-rendered before/after pair is still in the PR, produced by
GrokZeroUsageScreenshotRenderTestsfrom the real snapshot and card types:ClawSweeper 2026-08-30 review — each item answered
The 2026-08-31 re-review raised one more, now fixed:
docs/grok.mdstill described the pre-change rule: only a wire-published percentage adopted, with the parser's no-usage-yet zero explicitly rejected. It now describes what the resolver does, why the frame cannot publish an exact zero, what stays refused, and that the reading is an inference from frame shape rather than a value the surface stated. The parser section, which already documented the omitted scalar as zero usage, now says the retry adopts that reading, and the SuperGrok Heavy note is scoped to the credits payload it was describing.Residual limitation, stated plainly
Because a proto3 scalar omits an exact zero, no client can distinguish "the period is at 0%" from "this surface declined to publish a percentage" by inspecting the frame alone. The parser's requirements narrow it — the frame must carry a live period, usage-period evidence, and no percentage anywhere — and the live run above shows a real account hitting exactly that shape on a fresh period.
What supports reading it as a genuine zero:
20.0on 2026-08-22 while its period had usage, so this surface does publish nonzero percentages rather than withholding them. That measurement predates this branch and I have not re-run it on this head; the current period is at zero, so it cannot be reproduced until the account records usage again.What is not closable in code: if some plan or outage state ever returns a live period while withholding a nonzero percentage, this change shows 0% where
mainshows the failure message. The failure mode is bounded to the grok.com surface and to accounts whose credits proxy publishes no percentage.Maintainer decision. If you would rather not carry that inference at all, the conservative alternative is to keep usage unknown and replace the red failure text with a neutral "no usage reported for this period yet" line plus the reset time. That keeps
main's semantics and removes only the alarm; say the word and I will swap this PR to that shape.Testing
Verified on exact head
8e61b2e3fcd5dd39ac37dc15cc42e9c28c0f61b5:make check: passed — SwiftFormat clean, SwiftLint 0 violations in 2065 files, provider/package/documentation and repository-size gates green.make test: passed on31ced6bcb— 976/976 selections, 82/82 groups successful on the first pass, 0 failed groups, 0 retries, 0 timeouts (678.5 s). The only change since that run is thedocs/grok.mdreconciliation, covered bymake check's documentation gates andDocumentationLinkTests8/8.GrokCreditsProxyFetcherTests,GrokWebBillingFetcherTests,GrokAccountContextTests— 85 tests, all passing.an inferred grok dot com zero restores the bar without claiming a published percentfails on all three assertions (usedPercent → nil, the wire-published marker, andsourceLabel → "grok-cli-proxy").main83977905e, 0 behind,git diff --checkclean.Both proof tests are opt-in and skipped by default, so CI behavior is unchanged.