[app-server] Include reset-credit details in rate limits - #30395
Conversation
faed994 to
136386e
Compare
jayp-oai
changed the title
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Love having the reset credit details exposed for the UI — super helpful 🤩 Maintainer? Turn off weaves from non-maintainers → |
jayp-oai
changed the title
| - The backend may cap `rateLimitResetCredits.credits`, so `availableCount` is the authoritative total and can be greater than the number of detail rows. | ||
| - Reset-credit `id` values are opaque strings. `grantedAt` and `expiresAt` are Unix timestamps in seconds; `expiresAt` is `null` for a credit that does not expire. | ||
| - `title` and `description` are backend-provided display text. Either field can be `null` when the backend does not provide it. | ||
| - `resetType` is currently `codexRateLimits`. `status` reports the credit lifecycle state: `available`, `redeeming`, or `redeemed`. |
There was a problem hiding this comment.
field-by-field documentation seems quite overkill... but i see this is just following the pattern above 😬
we already have field-level documentation via docstrings
| /// Opaque backend identifier for this reset credit. | ||
| pub id: String, | ||
| pub reset_type: RateLimitResetType, | ||
| pub status: RateLimitResetCreditStatus, |
There was a problem hiding this comment.
Can we add an unknown variant for resetType / status? Users can be on older Codex binary versions while codex-backend deploys frequently, so an older app should degrade gracefully if the backend introduces a new value
Looks like we currently drop all reset-credit details
3 participants
Why
Supported clients currently receive only a reset-credit count from
account/rateLimits/read. The redemption UI and other app-server clients need each available credit's expiry and ID so they can explain what will expire and consume the credit a user selected. This information belongs on the existing rate-limit read surface rather than a second app-server list RPC that clients would need to coordinate.What changed
rateLimitResetCreditsonaccount/rateLimits/readwith nullablecreditsdetail rowscredits: nullcreditIdtoaccount/rateLimitResetCredit/consume; omitting it preserves the existing automatic-selection behaviorThe TUI consumer is stacked in #30488.
Validation
just test -p codex-app-server-protocol(251 passed)just test -p codex-backend-client(16 passed)just test -p codex-app-server rate_limit(18 passed)Part of #29618.