mirror of
https://github.com/openai/codex.git
synced 2026-10-21 16:38:24 +00:00
Cache finalized Markdown history rendering (#34223)
## Why Finalized Markdown can be rendered repeatedly at the same width while measuring and displaying transcript history. ## What changed - Cache the latest rendered lines for finalized agent messages and proposed plans. - Invalidate cached lines when the width, syntax theme, terminal colors, or color level changes. - Keep visualization directives uncached because resolving their local files depends on current filesystem state. ## Testing Add tests for cache reuse, render-state invalidation, raw Markdown rendering, and visualization cache bypass. GitOrigin-RevId: ba08140041a229b8cb9d297de87dc7bb39ebfe5d
This commit is contained in:
@@ -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<Option<(MarkdownRenderCacheKey, Vec<HyperlinkLine>)>>,
|
||||
}
|
||||
|
||||
#[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<HyperlinkLine>,
|
||||
) -> Vec<HyperlinkLine> {
|
||||
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
|
||||
}
|
||||
}
|
||||
@@ -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<crate::inline_visualization::InlineVisualizationContext>,
|
||||
rendered_lines: Option<MarkdownRenderCache>,
|
||||
}
|
||||
|
||||
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<HyperlinkLine> {
|
||||
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<HyperlinkLine> {
|
||||
@@ -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
|
||||
|
||||
@@ -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());
|
||||
}
|
||||
@@ -112,6 +112,7 @@ mod approvals;
|
||||
mod base;
|
||||
mod exec;
|
||||
mod hook_cell;
|
||||
mod markdown_render_cache;
|
||||
mod mcp;
|
||||
mod messages;
|
||||
mod notices;
|
||||
|
||||
@@ -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<HyperlinkLine> {
|
||||
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<HyperlinkLine> {
|
||||
@@ -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<Line<'static>> {
|
||||
visible_lines(self.lines.clone())
|
||||
|
||||
@@ -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")]
|
||||
);
|
||||
}
|
||||
@@ -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<SyntaxSet>` | Grammar database, immutable after init |
|
||||
//! | `THEME` | `OnceLock<RwLock<Theme>>` | Active color theme, swappable at runtime |
|
||||
//! | `THEME_REVISION` | `AtomicU64` | Invalidates rendered-content caches after theme swaps |
|
||||
//! | `THEME_OVERRIDE` | `OnceLock<Option<String>>` | Persisted user preference (write-once) |
|
||||
//! | `CODEX_HOME` | `OnceLock<Option<PathBuf>>` | 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<SyntaxSet> = OnceLock::new();
|
||||
static THEME: OnceLock<RwLock<Theme>> = OnceLock::new();
|
||||
static THEME_REVISION: AtomicU64 = AtomicU64::new(0);
|
||||
static THEME_OVERRIDE: OnceLock<Option<String>> = OnceLock::new();
|
||||
static CODEX_HOME: OnceLock<Option<PathBuf>> = OnceLock::new();
|
||||
|
||||
@@ -237,13 +241,19 @@ fn theme_lock() -> &'static RwLock<Theme> {
|
||||
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).
|
||||
|
||||
Reference in New Issue
Block a user