Skip to content

[codex] Thread backend-selected near-limit prompt payload through rate limits#20937

Open
jchu-oai wants to merge 2 commits into
mainfrom
jchu/codex-cli-near-limit-usage-snapshot-contract-2026.05.01
Open

[codex] Thread backend-selected near-limit prompt payload through rate limits#20937
jchu-oai wants to merge 2 commits into
mainfrom
jchu/codex-cli-near-limit-usage-snapshot-contract-2026.05.01

Conversation

@jchu-oai
Copy link
Copy Markdown

@jchu-oai jchu-oai commented May 4, 2026

Summary

  • add a nullable backend-selected near-limit prompt payload to shared rate-limit snapshot models
  • expose the payload through app-server v2 reads and notifications
  • keep unsupported or missing prompt payloads fail-closed

Copy link
Copy Markdown
Contributor

@dhruvgupta-oai dhruvgupta-oai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

itd be good to put this behind a gate just in case

credits,
plan_type: None,
rate_limit_reached_type: None,
current_usage_limit_nudge: None,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex review: The notification path is still fed by token-count rate-limit snapshots from response headers/SSE events, and those parsers explicitly set current_usage_limit_nudge: None. account/rateLimits/read fetches /wham/usage and returns the nudge directly, but it does not update the session state used by AccountRateLimitsUpdatedNotification, so the new notification field will remain null in normal backend-selected nudge flow.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that’s intentional in the final shape. Live rate-limit notifications are only a coarse freshness signal - /usage is the authoritative source for current_usage_limit_nudge. The stacked TUI PR uses the live update to trigger a bounded /usage refresh rather than reading prompt state from the notification itself. This matches the existing partial-snapshot pattern for fields like rate_limit_reached_type.

Comment on lines +22 to +27
#[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Hash, Serialize, Deserialize)]
pub enum UsageLimitNudgeAction {
#[serde(rename = "add_credits")]
AddCredits,
#[serde(rename = "upgrade")]
Upgrade,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex review: The PR says unsupported prompt payloads fail closed, but this generated enum has no #[serde(other)]/unknown variant. If the backend returns a new current_usage_limit_nudge.action, serde_json::from_str::<RateLimitStatusPayload> fails in get_rate_limits_many, so account/rateLimits/read returns an error instead of dropping only the unsupported nudge. Make the backend model/action mapping tolerate unknown actions and map them to None.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated: a9db1a9

@jchu-oai jchu-oai marked this pull request as ready for review May 5, 2026 15:24
@jchu-oai jchu-oai requested a review from a team as a code owner May 5, 2026 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants