Use descriptor pace rules in CLI - #2489
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f725708fb2
ℹ️ 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".
cf393b0 to
f2ada4a
Compare
|
Codex review: needs real behavior proof before merge. Reviewed July 29, 2026, 8:47 AM ET / 12:47 UTC. ClawSweeper reviewWhat this changesThe PR makes CLI pace calculations use provider descriptor rules, shares calendar-month reset-window resolution with the menu, and adds tertiary pace output to CLI text and JSON. Merge readiness⛔ Blocked until real behavior proof from a real setup is added - 3 items remain Keep this PR open. The focused patch appears to complete the CLI follow-up to the descriptor-based pace refactor and addresses the earlier tertiary-window review concern, but the external PR has only test and CI claims rather than captured after-fix CLI behavior proof; obtain redacted live text/JSON output before merge. Priority: P2 Review scores
Verification
How this fits togetherCodexBar collects quota windows from provider probes and presents their pace in both the macOS menu and the CLI. Provider descriptors define which reset windows have enough known cadence to calculate pace, and the renderers turn that into human-readable and JSON output. flowchart LR
A[Provider quota snapshot] --> B[Provider descriptor]
B --> C[Pace window resolution]
C --> D[Menu pace metrics]
C --> E[CLI text and JSON]
D --> F[Menu bar usage view]
E --> G[CLI consumers]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep provider descriptors as the single source of pace eligibility and add a redacted live CLI text and JSON capture for a descriptor-only window plus a tertiary window before merging. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible: descriptor-supported windows with missing or monthly-sentinel durations follow the old CLI allowlist/primary-secondary path, while the proposed focused tests exercise the resulting absent pace payloads. I did not execute that path in this read-only review. Is this the best way to solve the issue? Yes. Reusing ProviderPaceCapability for CLI rendering is the narrowest maintainable fix because it keeps menu and CLI eligibility and calendar-month duration semantics aligned instead of restoring a second provider allowlist. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against fcf75fb80457. 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
|
Addresses the CLI follow-up discussed in #2431.
Summary
pace.tertiaryJSON fieldRoot cause
CLIRenderer.PaceKind.supportsstill used provider allowlists from before the descriptor capability move in #2364. Descriptor-supported windows such as Grok weekly and Amp monthly were therefore omitted from CLI pace output even though the menu could pace them. The same computation also stopped at primary and secondary slots, so monthly quotas stored insnapshot.tertiaryremained absent from CLI text and JSON.Verification
f2ada4a0, based one92a6fa4nilswift test --filter 'CLISnapshotTests|ProviderPaceCapabilityTests|ZoomMateCreditsHistoryFetcherTests'— 70 tests passed on the final head, including Alibaba and OpenCode Go tertiary text/JSON coveragemake check— passed; SwiftFormat 0/1642 files, SwiftLint 0 violations in 1641 filesgit diff --check upstream/main...HEAD— passedmake test— stopped after its automatic retry on two unchangedAdaptiveRefreshTimerTestsCancellationErrortimeouts; both failures reproduced on the untouched original base, and this PR does not modify that test or timer codeCompatibility
Risk is low. Existing primary/secondary Codex, Claude, Ollama, OpenCode, and Kimi pace behavior is preserved. Tertiary pace is additive and only appears when the existing standard rules or the provider descriptor can compute it. The JSON change is additive:
pace.tertiaryis omitted when no tertiary pace applies.Review focus: descriptor-to-CLI window/kind resolution, tertiary text/JSON parity, and the shared calendar-month duration calculation.