diff --git a/codex-rs/tui/src/history_cell/base.rs b/codex-rs/tui/src/history_cell/base.rs index e9ade84945..a90a35c151 100644 --- a/codex-rs/tui/src/history_cell/base.rs +++ b/codex-rs/tui/src/history_cell/base.rs @@ -162,4 +162,8 @@ impl HistoryCell for CompositeHistoryCell { } out } + + fn has_stable_transcript_height(&self) -> bool { + false + } } diff --git a/codex-rs/tui/src/history_cell/messages.rs b/codex-rs/tui/src/history_cell/messages.rs index ce5082fef4..58892e4261 100644 --- a/codex-rs/tui/src/history_cell/messages.rs +++ b/codex-rs/tui/src/history_cell/messages.rs @@ -449,6 +449,10 @@ impl HistoryCell for AgentMarkdownCell { fn raw_lines(&self) -> Vec> { raw_lines_from_source(&self.markdown_source) } + + fn has_stable_transcript_height(&self) -> bool { + self.rendered_lines.is_some() + } } #[cfg(test)] diff --git a/codex-rs/tui/src/history_cell/mod.rs b/codex-rs/tui/src/history_cell/mod.rs index 0eb41da021..5708f27a21 100644 --- a/codex-rs/tui/src/history_cell/mod.rs +++ b/codex-rs/tui/src/history_cell/mod.rs @@ -279,6 +279,14 @@ pub(crate) trait HistoryCell: std::fmt::Debug + Send + Sync + Any { .unwrap_or(0) } + /// Whether the transcript height remains valid across later overlay renders. + /// + /// Cells backed by external state should return `false` so the pager remeasures them before + /// rendering instead of reusing a height that may now clip their content. + fn has_stable_transcript_height(&self) -> bool { + true + } + fn is_stream_continuation(&self) -> bool { false } diff --git a/codex-rs/tui/src/inline_visualization_tests.rs b/codex-rs/tui/src/inline_visualization_tests.rs index a883e58b67..4243d9517c 100644 --- a/codex-rs/tui/src/inline_visualization_tests.rs +++ b/codex-rs/tui/src/inline_visualization_tests.rs @@ -2,10 +2,15 @@ use super::*; use crate::history_cell::AgentMarkdownCell; use crate::history_cell::HistoryCell; use crate::history_cell::HistoryRenderMode; +use crate::keymap::RuntimeKeymap; +use crate::pager_overlay::TranscriptOverlay; use crate::streaming::controller::StreamController; use pretty_assertions::assert_eq; +use ratatui::buffer::Buffer; +use ratatui::layout::Rect; use ratatui::style::Style; use ratatui::style::Stylize; +use std::sync::Arc; use tempfile::TempDir; fn context_with_fragment(fragment: &str) -> (TempDir, InlineVisualizationContext) { @@ -47,6 +52,28 @@ fn line_text(line: &ratatui::text::Line<'_>) -> String { .collect() } +fn buffer_to_text(buffer: &Buffer, width: u16) -> String { + buffer + .content + .chunks(usize::from(width)) + .map(|row| { + row.iter() + .map(|cell| { + let symbol = cell.symbol(); + symbol + .strip_prefix("\x1b]8;;") + .and_then(|symbol| symbol.split_once('\x07')) + .and_then(|(_, symbol)| symbol.strip_suffix("\x1b]8;;\x07")) + .unwrap_or(symbol) + }) + .collect::() + .trim_end() + .to_string() + }) + .collect::>() + .join("\n") +} + #[test] fn rewrites_complete_directive_to_trusted_static_file_placeholder() { let (_codex_home, context) = context_with_fragment("
chart
"); @@ -250,6 +277,60 @@ fn finalized_agent_cell_replays_visualization_link() { ); } +#[test] +fn transcript_overlay_remeasures_visualization_when_artifact_becomes_available() { + let codex_home = tempfile::tempdir().expect("temp codex home"); + let context = InlineVisualizationContext::new(codex_home.path(), ThreadId::new()) + .expect("UUIDv7 thread id should provide a timestamp"); + fs::create_dir_all(&context.thread_dir).expect("create visualization directory"); + + let cell = AgentMarkdownCell::new_with_inline_visualizations( + "::codex-inline-vis{file=\"chart.html\"}".to_string(), + Path::new("/workspace"), + Some(context.clone()), + ); + let mut overlay = TranscriptOverlay::new(vec![Arc::new(cell)], RuntimeKeymap::defaults().pager); + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 240, /*height*/ 12, + ); + let mut buffer = Buffer::empty(area); + + overlay.render(area, &mut buffer); + let unavailable = buffer_to_text(&buffer, area.width); + assert!(unavailable.contains("Visualization unavailable on this device")); + + fs::write(context.thread_dir.join("chart.html"), "
chart
") + .expect("write visualization fragment"); + overlay.insert_cell(Arc::new(AgentMarkdownCell::new( + "next message".to_string(), + Path::new("/workspace"), + ))); + buffer = Buffer::empty(area); + overlay.render(area, &mut buffer); + + let available = buffer_to_text(&buffer, area.width); + assert!(available.contains("Open chart visualization in the browser")); + assert!( + available.contains("file://"), + "viewer URL was clipped: {available:?}" + ); + + let available = available + .lines() + .map(|line| { + line.find("file://").map_or_else( + || line.to_string(), + |start| format!("{}file://", &line[..start]), + ) + }) + .collect::>() + .join("\n"); + insta::assert_snapshot!( + "transcript_overlay_visualization_becomes_available", + format!("before:\n{unavailable}\n\nafter:\n{available}") + ); +} + #[test] fn agent_code_blocks_preserve_visualization_directive_literals() { let (_codex_home, context) = context_with_fragment("
chart
"); diff --git a/codex-rs/tui/src/pager_overlay.rs b/codex-rs/tui/src/pager_overlay.rs index cf263dd9a8..e3aee09268 100644 --- a/codex-rs/tui/src/pager_overlay.rs +++ b/codex-rs/tui/src/pager_overlay.rs @@ -393,14 +393,23 @@ impl Renderable for CachedRenderable { struct CellRenderable { cell: Arc, - style: Style, + highlighted: bool, } impl Renderable for CellRenderable { fn render(&self, area: Rect, buf: &mut Buffer) { let hyperlink_lines = self.cell.transcript_hyperlink_lines(area.width); + let style = if self.cell.as_any().is::() { + if self.highlighted { + user_message_style().reversed() + } else { + user_message_style() + } + } else { + Style::default() + }; let p = Paragraph::new(Text::from(visible_lines(hyperlink_lines.clone()))) - .style(self.style) + .style(style) .wrap(Wrap { trim: false }); p.render(area, buf); mark_buffer_hyperlinks(buf, area, &hyperlink_lines, /*scroll_rows*/ 0); @@ -488,40 +497,39 @@ impl TranscriptOverlay { cells .iter() .enumerate() - .flat_map(|(i, c)| { - let mut v: Vec> = Vec::new(); - let mut cell_renderable = if c.as_any().is::() { - Box::new(CachedRenderable::new(CellRenderable { - cell: c.clone(), - style: if highlight_cell == Some(i) { - user_message_style().reversed() - } else { - user_message_style() - }, - })) as Box - } else { - Box::new(CachedRenderable::new(CellRenderable { - cell: c.clone(), - style: Style::default(), - })) as Box - }; - if !c.is_stream_continuation() && i > 0 { - cell_renderable = Box::new(InsetRenderable::new( - cell_renderable, - Insets::tlbr( - /*top*/ 1, /*left*/ 0, /*bottom*/ 0, /*right*/ 0, - ), - )); - } - v.push(cell_renderable); - v - }) + .map(|(i, cell)| Self::render_cell(cell, i, highlight_cell)) .collect() } + /// Build the renderable for a committed cell, caching its height when the cell is stable. + fn render_cell( + cell: &Arc, + index: usize, + highlight_cell: Option, + ) -> Box { + let cell_renderable = CellRenderable { + cell: cell.clone(), + highlighted: highlight_cell == Some(index), + }; + let mut cell_renderable: Box = if cell.has_stable_transcript_height() { + Box::new(CachedRenderable::new(cell_renderable)) + } else { + Box::new(cell_renderable) + }; + if !cell.is_stream_continuation() && index > 0 { + cell_renderable = Box::new(InsetRenderable::new( + cell_renderable, + Insets::tlbr( + /*top*/ 1, /*left*/ 0, /*bottom*/ 0, /*right*/ 0, + ), + )); + } + cell_renderable + } + /// Insert a committed history cell while keeping any cached live tail. /// - /// The live tail is temporarily removed, the committed cells are rebuilt, + /// The live tail is temporarily removed, the new committed cell is appended, /// then the tail is reattached. If the tail previously had no leading /// spacing because it was the only renderable, we add the missing inset /// when the first committed cell arrives. @@ -533,8 +541,9 @@ impl TranscriptOverlay { let follow_bottom = self.view.is_scrolled_to_bottom(); let had_prior_cells = !self.cells.is_empty(); let tail_renderable = self.take_live_tail_renderable(); + let cell_renderable = Self::render_cell(&cell, self.cells.len(), self.highlight_cell); self.cells.push(cell); - self.view.renderables = Self::render_cells(&self.cells, self.highlight_cell); + self.view.renderables.push(cell_renderable); if let Some(tail) = tail_renderable { let tail = if !had_prior_cells && self @@ -940,6 +949,8 @@ mod tests { use std::collections::HashMap; use std::path::PathBuf; use std::sync::Arc; + use std::sync::atomic::AtomicUsize; + use std::sync::atomic::Ordering; use std::time::Duration; use crate::diff_model::FileChange; @@ -971,6 +982,26 @@ mod tests { } } + #[derive(Debug)] + struct HeightCountingCell { + height_calls: Arc, + } + + impl crate::history_cell::HistoryCell for HeightCountingCell { + fn display_lines(&self, _width: u16) -> Vec> { + vec![Line::from("counted")] + } + + fn raw_lines(&self) -> Vec> { + vec![Line::from("counted")] + } + + fn desired_transcript_height(&self, _width: u16) -> u16 { + self.height_calls.fetch_add(1, Ordering::Relaxed); + 1 + } + } + fn paragraph_block(label: &str, lines: usize) -> Box { let text = Text::from( (0..lines) @@ -1295,6 +1326,26 @@ mod tests { assert_eq!(overlay.view.scroll_offset, 0); } + #[test] + fn transcript_overlay_insert_preserves_cached_cell_heights() { + let height_calls = Arc::new(AtomicUsize::new(0)); + let mut overlay = transcript_overlay(vec![Arc::new(HeightCountingCell { + height_calls: height_calls.clone(), + })]); + let area = Rect::new(0, 0, 40, 12); + let mut buf = Buffer::empty(area); + + overlay.render(area, &mut buf); + assert_eq!(height_calls.load(Ordering::Relaxed), 1); + + overlay.insert_cell(Arc::new(TestCell { + lines: vec![Line::from("inserted")], + })); + overlay.render(area, &mut buf); + + assert_eq!(height_calls.load(Ordering::Relaxed), 1); + } + #[test] fn transcript_overlay_consolidation_remaps_highlight_inside_range() { let mut overlay = transcript_overlay( diff --git a/codex-rs/tui/src/snapshots/codex_tui__inline_visualization__tests__transcript_overlay_visualization_becomes_available.snap b/codex-rs/tui/src/snapshots/codex_tui__inline_visualization__tests__transcript_overlay_visualization_becomes_available.snap new file mode 100644 index 0000000000..fce2ba5a08 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__inline_visualization__tests__transcript_overlay_visualization_becomes_available.snap @@ -0,0 +1,30 @@ +--- +source: tui/src/inline_visualization_tests.rs +expression: "format!(\"before:\\n{unavailable}\\n\\nafter:\\n{available}\")" +--- +before: +/ T R A N S C R I P T / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / +• Visualization unavailable on this device. +~ +~ +~ +~ +~ +~ +───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── 100% ─ + ↑/↓ to scroll pgup/pgdn to page home/end to jump + q to quit esc to edit prev + + +after: +/ T R A N S C R I P T / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / / +• Open chart visualization in the browser + file:// + +• next message +~ +~ +~ +───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── 100% ─ + ↑/↓ to scroll pgup/pgdn to page home/end to jump + q to quit esc to edit prev diff --git a/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__transcript_overlay_status_rate_limit_refresh.snap b/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__transcript_overlay_status_rate_limit_refresh.snap new file mode 100644 index 0000000000..e2f12c8436 --- /dev/null +++ b/codex-rs/tui/src/status/snapshots/codex_tui__status__tests__transcript_overlay_status_rate_limit_refresh.snap @@ -0,0 +1,66 @@ +--- +source: tui/src/status/tests.rs +expression: "format!(\"before:\\n{before}\\n\\nafter:\\n{after}\")" +--- +before: +/ T R A N S C R I P T / / / / / / / / / / / / / / / / / / / / / / / / / / / / / +/status + +╭──────────────────────────────────────────────────────────────────────────╮ +│ >_ OpenAI Codex (v0.0.0) │ +│ │ +│ Visit https://chatgpt.com/codex/settings/usage for up-to-date │ +│ information on rate limits and credits │ +│ │ +│ Model: gpt-5.1-codex-max (reasoning none, summaries auto) │ +│ Directory: [[workspace]] │ +│ Permissions: Custom (workspace with network access, Ask for approval) │ +│ Agents.md: │ +│ │ +│ Token usage: 0 total (0 input + 0 output) │ +│ Limits: refresh requested; run /status again shortly. │ +╰──────────────────────────────────────────────────────────────────────────╯ +~ +~ +~ +~ +~ +~ +~ +~ +~ +───────────────────────────────────────────────────────────────────────── 100% ─ + ↑/↓ to scroll pgup/pgdn to page home/end to jump + q to quit esc to edit prev + + +after: +/ T R A N S C R I P T / / / / / / / / / / / / / / / / / / / / / / / / / / / / / +/status + +╭───────────────────────────────────────────────────────────────────────────╮ +│ >_ OpenAI Codex (v0.0.0) │ +│ │ +│ Visit https://chatgpt.com/codex/settings/usage for up-to-date │ +│ information on rate limits and credits │ +│ │ +│ Model: gpt-5.1-codex-max (reasoning none, summaries auto) │ +│ Directory: [[workspace]] │ +│ Permissions: Custom (workspace with network access, Ask for approval) │ +│ Agents.md: │ +│ │ +│ Token usage: 0 total (0 input + 0 output) │ +│ spark limit: │ +│ 5h limit: [███████████░░░░░░░░░] 55% left (resets soon) │ +│ Weekly limit: [██████████████░░░░░░] 70% left (resets later) │ +╰───────────────────────────────────────────────────────────────────────────╯ + +next message +~ +~ +~ +~ +~ +───────────────────────────────────────────────────────────────────────── 100% ─ + ↑/↓ to scroll pgup/pgdn to page home/end to jump + q to quit esc to edit prev diff --git a/codex-rs/tui/src/status/tests.rs b/codex-rs/tui/src/status/tests.rs index eedcfdfff2..6215899d2e 100644 --- a/codex-rs/tui/src/status/tests.rs +++ b/codex-rs/tui/src/status/tests.rs @@ -8,9 +8,12 @@ use super::rate_limits::SpendControlLimitSnapshotDisplay; use super::rate_limits::StatusRateLimitData; use super::rate_limits::compose_rate_limit_data_many; use crate::history_cell::HistoryCell; +use crate::history_cell::PlainHistoryCell; +use crate::keymap::RuntimeKeymap; use crate::legacy_core::config::Config; use crate::legacy_core::config::ConfigBuilder; use crate::legacy_core::config::PermissionProfileSnapshot; +use crate::pager_overlay::TranscriptOverlay; use crate::status::StatusAccountDisplay; use crate::status::remote_connection::RemoteConnectionStatus; use crate::test_support::PathBufExt; @@ -52,6 +55,7 @@ use codex_utils_absolute_path::AbsolutePathBuf; use insta::assert_snapshot; use pretty_assertions::assert_eq; use ratatui::prelude::*; +use std::sync::Arc; use tempfile::TempDir; use unicode_width::UnicodeWidthStr; @@ -206,6 +210,28 @@ fn sanitize_directory(lines: Vec) -> Vec { .collect() } +fn buffer_to_text(buffer: &Buffer, width: u16) -> String { + let lines = buffer + .content + .chunks(usize::from(width)) + .map(|row| { + row.iter() + .map(|cell| { + let symbol = cell.symbol(); + symbol + .strip_prefix("\x1b]8;;") + .and_then(|symbol| symbol.split_once('\x07')) + .and_then(|(_, symbol)| symbol.strip_suffix("\x1b]8;;\x07")) + .unwrap_or(symbol) + }) + .collect::() + .trim_end() + .to_string() + }) + .collect::>(); + sanitize_directory(lines).join("\n") +} + fn reset_at_from(captured_at: &chrono::DateTime, seconds: i64) -> i64 { (*captured_at + ChronoDuration::seconds(seconds)) .with_timezone(&Utc) @@ -1666,6 +1692,89 @@ async fn status_snapshot_shows_refreshing_limits_notice() { assert_snapshot!(sanitized); } +#[tokio::test] +async fn transcript_overlay_remeasures_status_after_rate_limit_refresh() { + let temp_home = TempDir::new().expect("temp home"); + let mut config = test_config(&temp_home).await; + config.model = Some("gpt-5.1-codex-max".to_string()); + set_workspace_cwd(&mut config, test_path_buf("/workspace/tests").abs()); + let usage = TokenUsage::default(); + let now = Local + .with_ymd_and_hms(2024, 6, 7, 8, 9, 10) + .single() + .expect("timestamp"); + let model_slug = get_model_offline_for_tests(config.model.as_deref()); + + let (status, handle) = new_status_output_with_rate_limits_handle( + &config, + /*runtime_model_provider_base_url*/ None, + /*remote_connection*/ None, + /*account_display*/ None, + /*token_info*/ None, + &usage, + &None, + /*thread_name*/ None, + /*forked_from*/ None, + /*rate_limits*/ &[], + None, + now, + &model_slug, + /*collaboration_mode*/ None, + /*reasoning_effort_override*/ None, + "".to_string(), + /*refreshing_rate_limits*/ true, + ); + let mut overlay = + TranscriptOverlay::new(vec![Arc::new(status)], RuntimeKeymap::defaults().pager); + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 80, /*height*/ 30, + ); + let mut buffer = Buffer::empty(area); + overlay.render(area, &mut buffer); + let before = buffer_to_text(&buffer, area.width); + + handle.finish_rate_limit_refresh( + &[RateLimitSnapshotDisplay { + limit_name: "spark".to_string(), + captured_at: now, + primary: Some(RateLimitWindowDisplay { + used_percent: 45.0, + resets_at: Some("soon".to_string()), + window_minutes: Some(300), + }), + secondary: Some(RateLimitWindowDisplay { + used_percent: 30.0, + resets_at: Some("later".to_string()), + window_minutes: Some(10_080), + }), + credits: None, + individual_limit: None, + }], + now, + ); + overlay.insert_cell(Arc::new(PlainHistoryCell::new(vec!["next message".into()]))); + buffer = Buffer::empty(area); + overlay.render(area, &mut buffer); + let after = buffer_to_text(&buffer, area.width); + + assert!( + after.contains("spark limit"), + "status output was clipped: {after:?}" + ); + assert!( + after.contains("5h limit"), + "status output was clipped: {after:?}" + ); + assert!( + after.contains("Weekly limit"), + "status output was clipped: {after:?}" + ); + insta::assert_snapshot!( + "transcript_overlay_status_rate_limit_refresh", + format!("before:\n{before}\n\nafter:\n{after}") + ); +} + #[tokio::test] async fn status_snapshot_includes_credits_and_limits() { let temp_home = TempDir::new().expect("temp home");