fix(grok): show validated empty usage periods - #3325
Conversation
Unified-billing SuperGrok plans omit creditUsagePercent from the CLI-proxy payload, and grok.com encodes exact-zero usage by omitting the proto3 scalar. The resolver refused that inferred 0%, so a freshly reset period rendered "usage unavailable" even though the weekly window was known. Adopt the no-usage-yet zero while keeping it unpublished, and still refuse any other inferred percent. Co-Authored-By: Claude Code <noreply@anthropic.com>
|
🦞👀 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. |
|
Codex review: needs maintainer review before merge. Reviewed September 1, 2026, 2:00 AM ET / 06:00 UTC. ClawSweeper reviewWhat this changesThis PR lets Grok’s fallback billing path adopt a parser-validated implicit 0% for a complete active weekly or monthly period while preserving unknown usage for other incomplete or inferred responses. Regression provenancePossible regression — probable (reproduction; reviewed change). No predecessor PR is attributed. Merge readinessKeep open for maintainer confirmation of the deliberate compatibility choice: a fully valid active period with an omitted proto3 usage scalar will now display 0% rather than unavailable usage. The patch is narrowly implemented, documented, and has no discrete correctness or security finding. Priority: P2 Review scores
Verification
How this fits togetherCodexBar’s Grok provider reads billing data from the CLI proxy and, when usage is absent, enriches it from grok.com before producing the usage card. This change governs which fallback results may create the card’s percentage bar. flowchart LR
A[Auth-file token] --> B[CLI proxy billing]
B --> C{Usage percent present?}
C -->|No| D[grok.com billing parser]
D --> E[Provenance gate]
E --> F[Grok usage card]
Decision needed
Why: The code safely narrows the heuristic and a real account exhibited the expected shape, but the wire format leaves a residual user-visible semantic ambiguity. Before merge
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Record acceptance of the bounded active-period interpretation, then merge this parser-provenance approach rather than widening acceptance to arbitrary inferred values. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible: the pinned main guard rejects the parser’s implicit zero, while the supplied redacted real-account transcript shows the resulting unavailable state and the changed resolved output. Is this the best way to solve the issue? Unclear pending the stated compatibility decision: the implementation is the narrowest way to adopt the 0% interpretation, but retaining unknown usage with a neutral status is a valid safer policy alternative. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against dacbd5222bda. 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 |
|
Post-fix proof from this exact head PR body updated with the redacted transcript. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Retain the parser provenance of an omitted proto3 percentage only for a complete response with an active current billing period. Preserve the proxy reset and plan while rejecting arbitrary inferred zeroes, incomplete frames, and historical periods. Co-authored-by: sf-jin-ku <jin.ku@sendbird.com> Co-authored-by: olddonkey <olddonkeyblog@gmail.com>
A freshly reset Grok period could remain “usage unavailable” because the quota resolver discarded the billing parser's implicit zero. Accept that zero only when it comes from one complete protobuf response containing a recognized active current period, with no conflicting percentage. The resolver keeps arbitrary inferred zeros, malformed or truncated zero responses, inactive periods, and invalid percentage payloads unavailable.
The snapshot carries the parser's provenance through plan and reset overlays. The regression fixture exercises the reported payload through parsing, enrichment, and UsageSnapshot conversion, alongside rejection cases and preservation of the existing six-second enrichment timeout.
Fixes #3261. Thanks @sf-jin-ku and @olddonkey for the implementations and reproduction evidence.
Validation: 91 focused tests across four Grok suites; make check; independent Codex review. All nine checks passed on commit 24eba74, including both full macOS test shards and Linux builds (CI run). Local make test remains incomplete: shared filesystem stalls caused unrelated process-fixture and AppKit timeouts; those failures are not counted as passes.