Add Grok weekly usage projections - #2170
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1bd3dbe404
ℹ️ 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".
| case .cursor: | ||
| window.windowMinutes != nil | ||
| case .grok: | ||
| GrokProviderDescriptor.primaryLabel(window: window, now: now) == "Weekly" |
There was a problem hiding this comment.
Handle >7-day Grok web resets before enabling pace
When a Grok web window has no windowMinutes and its reset is 8–12 days away, this predicate returns true because GrokProviderDescriptor.primaryLabel classifies that reset distance as Weekly. But resetWindowPaceDetail then calls UsagePace.weekly with defaultWindowMinutes: 10080, and that helper returns nil whenever timeUntilReset exceeds the 7-day duration, so these newly supported weekly rows still show no deficit/run-out text or pace marker. Either infer a duration that covers the reset distance or avoid marking those windows as pace-supported.
Useful? React with 👍 / 👎.
|
Codex review: needs real behavior proof before merge. Reviewed July 16, 2026, 4:32 PM ET / 20:32 UTC. Summary Reproducibility: not applicable. as a bug reproduction: the linked issue requests a new Grok display capability, and the focused tests deterministically describe the desired output. Review metrics: 2 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the narrow shared-helper implementation after a freshly built CodexBar bundle visibly demonstrates the intended Grok weekly projection while monthly and unclassified rows remain unchanged. Do we have a high-confidence way to reproduce the issue? Not applicable as a bug reproduction: the linked issue requests a new Grok display capability, and the focused tests deterministically describe the desired output. Is this the best way to solve the issue? Yes, subject to runtime proof: reusing the existing weekly pace helper through the stable menu-model seam is narrower and more maintainable than adding Grok-specific projection logic. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 65e2ae6a6363. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
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. How this review workflow works
Review history (7 earlier review cycles)
|
548a1be to
bd7bf69
Compare
|
Verified, repaired after main integration, and merged.
Merge commit: e63b188 |
Summary
Why
Grok was excluded from the generic reset-window pace support used by other providers. Web-derived Grok windows also relied on the wall clock during label classification instead of the model input timestamp, which prevented deterministic weekly classification in the menu model.
The shared
UsagePace.weeklyhelper remains responsible for validating reset timing and window duration.User impact
Weekly Grok usage rows with a valid weekly reset now show the existing deficit/reserve text, projected run-out estimate, and pace marker. Monthly and unclassified windows remain unchanged.
Validation
swift test --filter GrokMenuCardModelTests— 4 tests passedswift test --skip-build --filter Grok— 48 tests passedmake check— SwiftFormat and SwiftLint passedgit diff --check— passedCLIOpenAIDashboardCacheTestsgroup reported 3 cache-restoration failuresswift test --skip-build --filter CLIOpenAIDashboardCacheTests— all 10 tests passed in isolationScreenshots
Not included. The behavior is covered through the stable menu model seam without running a live Grok provider probe.
Fixes #2165