From 7355ca48c5a9b4d54dac25d2bf220354109af10c Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Thu, 25 Sep 2025 15:12:25 -0700 Subject: [PATCH] fix (#4251) # External (non-OpenAI) Pull Request Requirements Before opening this Pull Request, please read the dedicated "Contributing" markdown file or your PR may be closed: https://github.com/openai/codex/blob/main/docs/contributing.md If your PR conforms to our contribution guidelines, replace this text with a detailed and high quality description of your changes. --- codex-rs/core/src/client.rs | 19 +++-- codex-rs/tui/src/chatwidget.rs | 2 +- codex-rs/tui/src/status/card.rs | 21 +++--- codex-rs/tui/src/status/format.rs | 23 ++++--- codex-rs/tui/src/status/rate_limits.rs | 32 ++++++++- ...tatus_snapshot_includes_monthly_limit.snap | 18 +++++ ...snapshot_shows_missing_limits_message.snap | 18 +++++ codex-rs/tui/src/status/tests.rs | 69 ++++++++++++++++++- 8 files changed, 171 insertions(+), 31 deletions(-) create mode 100644 codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_includes_monthly_limit.snap create mode 100644 codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_shows_missing_limits_message.snap diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 1d1082c2..aedea488 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -572,12 +572,21 @@ fn parse_rate_limit_window( window_minutes_header: &str, resets_header: &str, ) -> Option { - let used_percent = parse_header_f64(headers, used_percent_header)?; + let used_percent: Option = parse_header_f64(headers, used_percent_header); - Some(RateLimitWindow { - used_percent, - window_minutes: parse_header_u64(headers, window_minutes_header), - resets_in_seconds: parse_header_u64(headers, resets_header), + used_percent.and_then(|used_percent| { + let window_minutes = parse_header_u64(headers, window_minutes_header); + let resets_in_seconds = parse_header_u64(headers, resets_header); + + let has_data = used_percent != 0.0 + || window_minutes.is_some_and(|minutes| minutes != 0) + || resets_in_seconds.is_some_and(|seconds| seconds != 0); + + has_data.then_some(RateLimitWindow { + used_percent, + window_minutes, + resets_in_seconds, + }) }) } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 8bd15b53..2a9eaab3 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -185,7 +185,7 @@ impl RateLimitWarningState { } } -fn get_limits_duration(windows_minutes: u64) -> String { +pub(crate) fn get_limits_duration(windows_minutes: u64) -> String { const MINUTES_PER_HOUR: u64 = 60; const MINUTES_PER_DAY: u64 = 24 * MINUTES_PER_HOUR; const MINUTES_PER_WEEK: u64 = 7 * MINUTES_PER_DAY; diff --git a/codex-rs/tui/src/status/card.rs b/codex-rs/tui/src/status/card.rs index a977240b..8f7188d5 100644 --- a/codex-rs/tui/src/status/card.rs +++ b/codex-rs/tui/src/status/card.rs @@ -145,7 +145,7 @@ impl StatusHistoryCell { Span::from(" "), Span::from(format_status_limit_summary(row.percent_used)), ]; - let base_spans = formatter.full_spans(row.label, value_spans); + let base_spans = formatter.full_spans(row.label.as_str(), value_spans); let base_line = Line::from(base_spans.clone()); if let Some(resets_at) = row.resets_at.as_ref() { @@ -176,18 +176,14 @@ impl StatusHistoryCell { } } - fn collect_rate_limit_labels( - &self, - seen: &mut BTreeSet<&'static str>, - labels: &mut Vec<&'static str>, - ) { + fn collect_rate_limit_labels(&self, seen: &mut BTreeSet, labels: &mut Vec) { match &self.rate_limits { StatusRateLimitData::Available(rows) => { if rows.is_empty() { push_label(labels, seen, "Limits"); } else { for row in rows { - push_label(labels, seen, row.label); + push_label(labels, seen, row.label.as_str()); } } } @@ -224,9 +220,12 @@ impl HistoryCell for StatusHistoryCell { } }); - let mut labels: Vec<&'static str> = - vec!["Model", "Directory", "Approval", "Sandbox", "Agents.md"]; - let mut seen: BTreeSet<&'static str> = labels.iter().copied().collect(); + let mut labels: Vec = + vec!["Model", "Directory", "Approval", "Sandbox", "Agents.md"] + .into_iter() + .map(str::to_string) + .collect(); + let mut seen: BTreeSet = labels.iter().cloned().collect(); if account_value.is_some() { push_label(&mut labels, &mut seen, "Account"); @@ -237,7 +236,7 @@ impl HistoryCell for StatusHistoryCell { push_label(&mut labels, &mut seen, "Token Usage"); self.collect_rate_limit_labels(&mut seen, &mut labels); - let formatter = FieldFormatter::from_labels(labels.iter().copied()); + let formatter = FieldFormatter::from_labels(labels.iter().map(String::as_str)); let value_width = formatter.value_width(available_inner_width); let mut model_spans = vec![Span::from(self.model_name.clone())]; diff --git a/codex-rs/tui/src/status/format.rs b/codex-rs/tui/src/status/format.rs index 556e5bd1..fda72904 100644 --- a/codex-rs/tui/src/status/format.rs +++ b/codex-rs/tui/src/status/format.rs @@ -15,10 +15,13 @@ pub(crate) struct FieldFormatter { impl FieldFormatter { pub(crate) const INDENT: &'static str = " "; - pub(crate) fn from_labels(labels: impl IntoIterator) -> Self { + pub(crate) fn from_labels(labels: impl IntoIterator) -> Self + where + S: AsRef, + { let label_width = labels .into_iter() - .map(UnicodeWidthStr::width) + .map(|label| UnicodeWidthStr::width(label.as_ref())) .max() .unwrap_or(0); let indent_width = UnicodeWidthStr::width(Self::INDENT); @@ -53,7 +56,7 @@ impl FieldFormatter { pub(crate) fn full_spans( &self, - label: &'static str, + label: &str, mut value_spans: Vec>, ) -> Vec> { let mut spans = Vec::with_capacity(value_spans.len() + 1); @@ -79,14 +82,14 @@ impl FieldFormatter { } } -pub(crate) fn push_label( - labels: &mut Vec<&'static str>, - seen: &mut BTreeSet<&'static str>, - label: &'static str, -) { - if seen.insert(label) { - labels.push(label); +pub(crate) fn push_label(labels: &mut Vec, seen: &mut BTreeSet, label: &str) { + if seen.contains(label) { + return; } + + let owned = label.to_string(); + seen.insert(owned.clone()); + labels.push(owned); } pub(crate) fn line_display_width(line: &Line<'static>) -> usize { diff --git a/codex-rs/tui/src/status/rate_limits.rs b/codex-rs/tui/src/status/rate_limits.rs index 17d2f170..8da3a1c6 100644 --- a/codex-rs/tui/src/status/rate_limits.rs +++ b/codex-rs/tui/src/status/rate_limits.rs @@ -1,3 +1,5 @@ +use crate::chatwidget::get_limits_duration; + use super::helpers::format_reset_timestamp; use chrono::DateTime; use chrono::Duration as ChronoDuration; @@ -13,7 +15,7 @@ pub(crate) const RESET_BULLET: &str = "·"; #[derive(Debug, Clone)] pub(crate) struct StatusRateLimitRow { - pub label: &'static str, + pub label: String, pub percent_used: f64, pub resets_at: Option, } @@ -28,6 +30,7 @@ pub(crate) enum StatusRateLimitData { pub(crate) struct RateLimitWindowDisplay { pub used_percent: f64, pub resets_at: Option, + pub window_minutes: Option, } impl RateLimitWindowDisplay { @@ -41,6 +44,7 @@ impl RateLimitWindowDisplay { Self { used_percent: window.used_percent, resets_at, + window_minutes: window.window_minutes, } } } @@ -75,16 +79,26 @@ pub(crate) fn compose_rate_limit_data( let mut rows = Vec::with_capacity(2); if let Some(primary) = snapshot.primary.as_ref() { + let label: String = primary + .window_minutes + .map(get_limits_duration) + .unwrap_or_else(|| "5h".to_string()); + let label = capitalize_first(&label); rows.push(StatusRateLimitRow { - label: "5h limit", + label: format!("{label} limit"), percent_used: primary.used_percent, resets_at: primary.resets_at.clone(), }); } if let Some(secondary) = snapshot.secondary.as_ref() { + let label: String = secondary + .window_minutes + .map(get_limits_duration) + .unwrap_or_else(|| "weekly".to_string()); + let label = capitalize_first(&label); rows.push(StatusRateLimitRow { - label: "Weekly limit", + label: format!("{label} limit"), percent_used: secondary.used_percent, resets_at: secondary.resets_at.clone(), }); @@ -115,3 +129,15 @@ pub(crate) fn render_status_limit_progress_bar(percent_used: f64) -> String { pub(crate) fn format_status_limit_summary(percent_used: f64) -> String { format!("{percent_used:.0}% used") } + +fn capitalize_first(label: &str) -> String { + let mut chars = label.chars(); + match chars.next() { + Some(first) => { + let mut capitalized = first.to_uppercase().collect::(); + capitalized.push_str(chars.as_str()); + capitalized + } + None => String::new(), + } +} diff --git a/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_includes_monthly_limit.snap b/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_includes_monthly_limit.snap new file mode 100644 index 00000000..25fd9c8a --- /dev/null +++ b/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_includes_monthly_limit.snap @@ -0,0 +1,18 @@ +--- +source: tui/src/status/tests.rs +expression: sanitized +--- +/status + +╭──────────────────────────────────────────────────────────────────────────╮ +│ >_ OpenAI Codex (v0.0.0) │ +│ │ +│ Model: gpt-5-codex (reasoning none, summaries auto) │ +│ Directory: [[workspace]] │ +│ Approval: on-request │ +│ Sandbox: read-only │ +│ Agents.md: │ +│ │ +│ Token Usage: 1.2K total (800 input + 400 output) │ +│ Monthly limit: [██░░░░░░░░░░░░░░░░░░] 12% used · resets 7 May (07:08) │ +╰──────────────────────────────────────────────────────────────────────────╯ diff --git a/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_shows_missing_limits_message.snap b/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_shows_missing_limits_message.snap new file mode 100644 index 00000000..eabcc2a4 --- /dev/null +++ b/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__status_snapshot_shows_missing_limits_message.snap @@ -0,0 +1,18 @@ +--- +source: tui/src/status/tests.rs +expression: sanitized +--- +/status + +╭──────────────────────────────────────────────────────────────╮ +│ >_ OpenAI Codex (v0.0.0) │ +│ │ +│ Model: gpt-5-codex (reasoning none, summaries auto) │ +│ Directory: [[workspace]] │ +│ Approval: on-request │ +│ Sandbox: read-only │ +│ Agents.md: │ +│ │ +│ Token Usage: 750 total (500 input + 250 output) │ +│ Limits: data not available yet │ +╰──────────────────────────────────────────────────────────────╯ diff --git a/codex-rs/tui/src/status/tests.rs b/codex-rs/tui/src/status/tests.rs index 467b9dd7..d583a5e5 100644 --- a/codex-rs/tui/src/status/tests.rs +++ b/codex-rs/tui/src/status/tests.rs @@ -93,7 +93,7 @@ fn status_snapshot_includes_reasoning_details() { }), secondary: Some(RateLimitWindow { used_percent: 45.0, - window_minutes: Some(1_440), + window_minutes: Some(10080), resets_in_seconds: Some(1_200), }), }; @@ -114,6 +114,47 @@ fn status_snapshot_includes_reasoning_details() { assert_snapshot!(sanitized); } +#[test] +fn status_snapshot_includes_monthly_limit() { + let temp_home = TempDir::new().expect("temp home"); + let mut config = test_config(&temp_home); + config.model = "gpt-5-codex".to_string(); + config.model_provider_id = "openai".to_string(); + config.cwd = PathBuf::from("/workspace/tests"); + + let usage = TokenUsage { + input_tokens: 800, + cached_input_tokens: 0, + output_tokens: 400, + reasoning_output_tokens: 0, + total_tokens: 1_200, + }; + + let snapshot = RateLimitSnapshot { + primary: Some(RateLimitWindow { + used_percent: 12.0, + window_minutes: Some(43_200), + resets_in_seconds: Some(86_400), + }), + secondary: None, + }; + let captured_at = chrono::Local + .with_ymd_and_hms(2024, 5, 6, 7, 8, 9) + .single() + .expect("timestamp"); + let rate_display = rate_limit_snapshot_display(&snapshot, captured_at); + + let composite = new_status_output(&config, &usage, &None, Some(&rate_display)); + let mut rendered_lines = render_lines(&composite.display_lines(80)); + if cfg!(windows) { + for line in &mut rendered_lines { + *line = line.replace('\\', "/"); + } + } + let sanitized = sanitize_directory(rendered_lines).join("\n"); + assert_snapshot!(sanitized); +} + #[test] fn status_card_token_usage_excludes_cached_tokens() { let temp_home = TempDir::new().expect("temp home"); @@ -181,3 +222,29 @@ fn status_snapshot_truncates_in_narrow_terminal() { assert_snapshot!(sanitized); } + +#[test] +fn status_snapshot_shows_missing_limits_message() { + let temp_home = TempDir::new().expect("temp home"); + let mut config = test_config(&temp_home); + config.model = "gpt-5-codex".to_string(); + config.cwd = PathBuf::from("/workspace/tests"); + + let usage = TokenUsage { + input_tokens: 500, + cached_input_tokens: 0, + output_tokens: 250, + reasoning_output_tokens: 0, + total_tokens: 750, + }; + + let composite = new_status_output(&config, &usage, &None, None); + let mut rendered_lines = render_lines(&composite.display_lines(80)); + if cfg!(windows) { + for line in &mut rendered_lines { + *line = line.replace('\\', "/"); + } + } + let sanitized = sanitize_directory(rendered_lines).join("\n"); + assert_snapshot!(sanitized); +}