diff --git a/codex-rs/tui/src/history_cell/markdown_render_cache.rs b/codex-rs/tui/src/history_cell/markdown_render_cache.rs new file mode 100644 index 0000000000..95e4f6be39 --- /dev/null +++ b/codex-rs/tui/src/history_cell/markdown_render_cache.rs @@ -0,0 +1,49 @@ +//! Single-width render cache shared by finalized source-backed markdown history cells. + +use crate::terminal_hyperlinks::HyperlinkLine; +use std::sync::Mutex; +use std::sync::PoisonError; + +#[derive(Debug, Default)] +pub(super) struct MarkdownRenderCache { + pub(super) cached: Mutex)>>, +} + +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(super) struct MarkdownRenderCacheKey { + pub(super) width: u16, + pub(super) syntax_theme_revision: u64, + pub(super) terminal_fg: Option<(u8, u8, u8)>, + pub(super) terminal_bg: Option<(u8, u8, u8)>, + pub(super) color_level: crate::terminal_palette::StdoutColorLevel, +} + +impl MarkdownRenderCache { + /// Return lines cached for this width and terminal render state, rendering on a cache miss. + /// + /// Only the most recent entry is retained, so changing width, syntax theme, or terminal colors + /// replaces the cached render. + pub(super) fn render( + &self, + width: u16, + render: impl FnOnce() -> Vec, + ) -> Vec { + let key = MarkdownRenderCacheKey { + width, + syntax_theme_revision: crate::render::highlight::syntax_theme_revision(), + terminal_fg: crate::terminal_palette::default_fg(), + terminal_bg: crate::terminal_palette::default_bg(), + color_level: crate::terminal_palette::stdout_color_level(), + }; + let mut cached = self.cached.lock().unwrap_or_else(PoisonError::into_inner); + if let Some((cached_key, lines)) = cached.as_ref() + && *cached_key == key + { + return lines.clone(); + } + + let lines = render(); + *cached = Some((key, lines.clone())); + lines + } +} diff --git a/codex-rs/tui/src/history_cell/messages.rs b/codex-rs/tui/src/history_cell/messages.rs index 9afb55dfbd..ce5082fef4 100644 --- a/codex-rs/tui/src/history_cell/messages.rs +++ b/codex-rs/tui/src/history_cell/messages.rs @@ -1,5 +1,6 @@ //! User, assistant, reasoning, and streaming message history cells. +use super::markdown_render_cache::MarkdownRenderCache; use super::*; #[derive(Debug)] @@ -361,11 +362,15 @@ impl HistoryCell for AgentMessageCell { /// The cell snapshots `cwd` at construction so local file-link display remains aligned with the /// session that produced the message. Reusing the current process cwd during reflow would make old /// transcript content change meaning after a later `/cd` or resumed session. +/// +/// Ordinary markdown caches its latest rich render. Visualization directives bypass that cache +/// because resolving their local file links depends on filesystem state that can change later. #[derive(Debug)] pub(crate) struct AgentMarkdownCell { markdown_source: String, cwd: PathBuf, inline_visualization_context: Option, + rendered_lines: Option, } impl AgentMarkdownCell { @@ -390,10 +395,14 @@ impl AgentMarkdownCell { crate::inline_visualization::InlineVisualizationContext, >, ) -> Self { + let rendered_lines = (!markdown_source + .contains(crate::inline_visualization::DIRECTIVE_PREFIX)) + .then(MarkdownRenderCache::default); Self { markdown_source, cwd: cwd.to_path_buf(), inline_visualization_context, + rendered_lines, } } } @@ -404,25 +413,33 @@ impl HistoryCell for AgentMarkdownCell { } fn display_hyperlink_lines(&self, width: u16) -> Vec { - let Some(wrap_width) = - crate::width::usable_content_width_u16(width, /*reserved_cols*/ 2) - else { - return prefix_hyperlink_lines( - vec![HyperlinkLine::new(Line::default())], - "• ".dim(), - " ".into(), + let render = || { + let Some(wrap_width) = + crate::width::usable_content_width_u16(width, /*reserved_cols*/ 2) + else { + return prefix_hyperlink_lines( + vec![HyperlinkLine::new(Line::default())], + "• ".dim(), + " ".into(), + ); + }; + + // Re-render markdown from source at the current width. Reserve 2 columns for the "• " / + // " " prefix prepended below. + let lines = crate::markdown::render_markdown_agent_with_links_cwd_and_visualizations( + &self.markdown_source, + Some(wrap_width), + Some(self.cwd.as_path()), + self.inline_visualization_context.as_ref(), ); + prefix_hyperlink_lines(lines, "• ".dim(), " ".into()) }; - // Re-render markdown from source at the current width. Reserve 2 columns for the "• " / - // " " prefix prepended below. - let lines = crate::markdown::render_markdown_agent_with_links_cwd_and_visualizations( - &self.markdown_source, - Some(wrap_width), - Some(self.cwd.as_path()), - self.inline_visualization_context.as_ref(), - ); - prefix_hyperlink_lines(lines, "• ".dim(), " ".into()) + if let Some(rendered_lines) = &self.rendered_lines { + rendered_lines.render(width, render) + } else { + render() + } } fn transcript_hyperlink_lines(&self, width: u16) -> Vec { @@ -434,6 +451,10 @@ impl HistoryCell for AgentMarkdownCell { } } +#[cfg(test)] +#[path = "messages_tests.rs"] +mod tests; + /// Transient active-cell representation of the mutable tail of an agent stream. /// /// During streaming, lines that have not yet been committed to scrollback because they belong to diff --git a/codex-rs/tui/src/history_cell/messages_tests.rs b/codex-rs/tui/src/history_cell/messages_tests.rs new file mode 100644 index 0000000000..eae1ade5cf --- /dev/null +++ b/codex-rs/tui/src/history_cell/messages_tests.rs @@ -0,0 +1,82 @@ +use super::*; +use crate::history_cell::markdown_render_cache::MarkdownRenderCacheKey; +use pretty_assertions::assert_eq; + +fn replace_cached_lines( + cell: &AgentMarkdownCell, + update_key: impl FnOnce(&mut MarkdownRenderCacheKey), +) { + let rendered_lines = cell + .rendered_lines + .as_ref() + .expect("ordinary markdown should be cacheable"); + let mut rendered_lines = rendered_lines.cached.lock().expect("render cache lock"); + let (key, lines) = rendered_lines + .as_mut() + .expect("render cache should be populated"); + *lines = vec![HyperlinkLine::from("cached")]; + update_key(key); +} + +#[test] +fn finalized_markdown_reuses_lines_primed_by_transcript_height() { + let cell = AgentMarkdownCell::new("finalized **markdown**".to_string(), Path::new("/tmp")); + let width = 48; + + assert_eq!(cell.desired_transcript_height(width), 1); + replace_cached_lines(&cell, |_| {}); + + assert_eq!( + visible_lines(cell.transcript_hyperlink_lines(width)), + vec![Line::from("cached")] + ); +} + +#[test] +fn finalized_markdown_cache_misses_when_width_or_render_style_changes() { + let cell = AgentMarkdownCell::new("finalized **markdown**".to_string(), Path::new("/tmp")); + let width = 48; + let expected = cell.display_lines(width); + + replace_cached_lines(&cell, |key| key.width = key.width.saturating_sub(1)); + assert_eq!(cell.display_lines(width), expected); + + replace_cached_lines(&cell, |key| { + key.syntax_theme_revision = key.syntax_theme_revision.wrapping_sub(1); + }); + assert_eq!(cell.display_lines(width), expected); + + replace_cached_lines(&cell, |key| { + key.terminal_fg = key + .terminal_fg + .map_or(Some((1, 2, 3)), |(r, g, b)| Some((r ^ 1, g, b))); + }); + assert_eq!(cell.display_lines(width), expected); +} + +#[test] +fn raw_markdown_bypasses_the_rich_render_cache() { + let source = "finalized **markdown**"; + let cell = AgentMarkdownCell::new(source.to_string(), Path::new("/tmp")); + let width = 48; + + cell.display_lines(width); + replace_cached_lines(&cell, |_| {}); + + assert_eq!( + cell.display_lines_for_mode(width, HistoryRenderMode::Raw), + vec![Line::from(source)] + ); +} + +#[test] +fn visualization_directives_are_not_cached() { + let cell = AgentMarkdownCell::new( + "::codex-inline-vis{file=\"chart.html\"}".to_string(), + Path::new("/tmp"), + ); + + cell.display_lines(/*width*/ 48); + + assert!(cell.rendered_lines.is_none()); +} diff --git a/codex-rs/tui/src/history_cell/mod.rs b/codex-rs/tui/src/history_cell/mod.rs index 9cef3e4fa5..0eb41da021 100644 --- a/codex-rs/tui/src/history_cell/mod.rs +++ b/codex-rs/tui/src/history_cell/mod.rs @@ -112,6 +112,7 @@ mod approvals; mod base; mod exec; mod hook_cell; +mod markdown_render_cache; mod mcp; mod messages; mod notices; diff --git a/codex-rs/tui/src/history_cell/plans.rs b/codex-rs/tui/src/history_cell/plans.rs index 81d7beb622..4847d30ca4 100644 --- a/codex-rs/tui/src/history_cell/plans.rs +++ b/codex-rs/tui/src/history_cell/plans.rs @@ -1,5 +1,6 @@ //! Proposed-plan and plan-update history cells. +use super::markdown_render_cache::MarkdownRenderCache; use super::*; /// Transient active-cell representation of the mutable tail of a proposed-plan stream. @@ -58,6 +59,7 @@ pub(crate) fn new_proposed_plan(plan_markdown: String, cwd: &Path) -> ProposedPl ProposedPlanCell { plan_markdown, cwd: cwd.to_path_buf(), + rendered_lines: MarkdownRenderCache::default(), } } @@ -84,6 +86,7 @@ pub(crate) struct ProposedPlanCell { plan_markdown: String, /// Session cwd used to keep local file-link display aligned with live streamed plan rendering. cwd: PathBuf, + rendered_lines: MarkdownRenderCache, } /// Transient proposed-plan history emitted while a plan is still streaming. @@ -103,27 +106,29 @@ impl HistoryCell for ProposedPlanCell { } fn display_hyperlink_lines(&self, width: u16) -> Vec { - let mut lines = vec![ - HyperlinkLine::new(vec!["• ".dim(), "Proposed Plan".bold()].into()), - HyperlinkLine::new(Line::from(" ")), - ]; + self.rendered_lines.render(width, || { + let mut lines = vec![ + HyperlinkLine::new(vec!["• ".dim(), "Proposed Plan".bold()].into()), + HyperlinkLine::new(Line::from(" ")), + ]; - let mut plan_lines = vec![HyperlinkLine::new(Line::from(" "))]; - let plan_style = proposed_plan_style(); - let wrap_width = width.saturating_sub(4).max(1) as usize; - let mut body = crate::markdown::render_markdown_agent_with_links_and_cwd( - &self.plan_markdown, - Some(wrap_width), - Some(self.cwd.as_path()), - ); - if body.is_empty() { - body.push(HyperlinkLine::new(Line::from("(empty)".dim().italic()))); - } - plan_lines.extend(prefix_hyperlink_lines(body, " ".into(), " ".into())); - plan_lines.push(HyperlinkLine::new(Line::from(" "))); + let mut plan_lines = vec![HyperlinkLine::new(Line::from(" "))]; + let plan_style = proposed_plan_style(); + let wrap_width = width.saturating_sub(4).max(1) as usize; + let mut body = crate::markdown::render_markdown_agent_with_links_and_cwd( + &self.plan_markdown, + Some(wrap_width), + Some(self.cwd.as_path()), + ); + if body.is_empty() { + body.push(HyperlinkLine::new(Line::from("(empty)".dim().italic()))); + } + plan_lines.extend(prefix_hyperlink_lines(body, " ".into(), " ".into())); + plan_lines.push(HyperlinkLine::new(Line::from(" "))); - lines.extend(plan_lines.into_iter().map(|line| line.style(plan_style))); - lines + lines.extend(plan_lines.into_iter().map(|line| line.style(plan_style))); + lines + }) } fn transcript_hyperlink_lines(&self, width: u16) -> Vec { @@ -135,6 +140,10 @@ impl HistoryCell for ProposedPlanCell { } } +#[cfg(test)] +#[path = "plans_tests.rs"] +mod tests; + impl HistoryCell for ProposedPlanStreamCell { fn display_lines(&self, _width: u16) -> Vec> { visible_lines(self.lines.clone()) diff --git a/codex-rs/tui/src/history_cell/plans_tests.rs b/codex-rs/tui/src/history_cell/plans_tests.rs new file mode 100644 index 0000000000..f32945486a --- /dev/null +++ b/codex-rs/tui/src/history_cell/plans_tests.rs @@ -0,0 +1,22 @@ +use super::*; +use pretty_assertions::assert_eq; + +#[test] +fn finalized_plan_reuses_lines_primed_by_transcript_height() { + let cell = new_proposed_plan("1. Inspect **markdown**".to_string(), Path::new("/tmp")); + let width = 48; + + assert_eq!(cell.desired_transcript_height(width), 8); + cell.rendered_lines + .cached + .lock() + .expect("render cache lock") + .as_mut() + .expect("render cache should be populated") + .1 = vec![HyperlinkLine::from("cached")]; + + assert_eq!( + visible_lines(cell.transcript_hyperlink_lines(width)), + vec![Line::from("cached")] + ); +} diff --git a/codex-rs/tui/src/render/highlight.rs b/codex-rs/tui/src/render/highlight.rs index ef4d7a2f35..f0b77f6652 100644 --- a/codex-rs/tui/src/render/highlight.rs +++ b/codex-rs/tui/src/render/highlight.rs @@ -2,12 +2,13 @@ //! //! Wraps [syntect] with the [two_face] grammar and theme bundles to provide //! ~250-language syntax highlighting and 32 bundled color themes. The module -//! owns four process-global singletons: +//! owns five process-global singletons: //! //! | Singleton | Type | Purpose | //! |---|---|---| //! | `SYNTAX_SET` | `OnceLock` | Grammar database, immutable after init | //! | `THEME` | `OnceLock>` | Active color theme, swappable at runtime | +//! | `THEME_REVISION` | `AtomicU64` | Invalidates rendered-content caches after theme swaps | //! | `THEME_OVERRIDE` | `OnceLock>` | Persisted user preference (write-once) | //! | `CODEX_HOME` | `OnceLock>` | Root for custom `.tmTheme` discovery | //! @@ -30,6 +31,8 @@ use std::path::Path; use std::path::PathBuf; use std::sync::OnceLock; use std::sync::RwLock; +use std::sync::atomic::AtomicU64; +use std::sync::atomic::Ordering; use syntect::easy::HighlightLines; use syntect::highlighting::Color as SyntectColor; use syntect::highlighting::FontStyle; @@ -47,6 +50,7 @@ use two_face::theme::EmbeddedThemeName; static SYNTAX_SET: OnceLock = OnceLock::new(); static THEME: OnceLock> = OnceLock::new(); +static THEME_REVISION: AtomicU64 = AtomicU64::new(0); static THEME_OVERRIDE: OnceLock> = OnceLock::new(); static CODEX_HOME: OnceLock> = OnceLock::new(); @@ -237,13 +241,19 @@ fn theme_lock() -> &'static RwLock { THEME.get_or_init(|| RwLock::new(build_default_theme())) } -/// Swap the active syntax theme at runtime (for live preview). +/// Swap the active syntax theme at runtime and invalidate rendered-content caches. pub(crate) fn set_syntax_theme(theme: Theme) { let mut guard = match theme_lock().write() { Ok(guard) => guard, Err(poisoned) => poisoned.into_inner(), }; *guard = theme; + THEME_REVISION.fetch_add(1, Ordering::Release); +} + +/// Return the revision of the active syntax theme for rendered-content caches. +pub(crate) fn syntax_theme_revision() -> u64 { + THEME_REVISION.load(Ordering::Acquire) } /// Clone the current syntax theme (e.g. to save for cancel-restore).