From 3b3a91ccf0cb09fc8f713385d6eaae98e8919162 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 14 Aug 2026 16:25:12 +0000 Subject: [PATCH] Fix resume transcript preview line selection (#38605) ## What changed - Scan legacy and paginated transcript items newest-first and stop after collecting the six newest nonblank preview lines. - Keep paging through bounded paginated history when newer items do not produce visible preview text, while preserving chronological display order and assistant markdown rewrites. - Move transcript preview loading into a focused module and add coverage for reverse scanning, legacy/paginated parity, and paging past invisible items. GitOrigin-RevId: 576afe37d034b32db363691bd4f401815fffaea7 --- codex-rs/tui/src/resume_picker.rs | 155 +----------- .../src/resume_picker_transcript_preview.rs | 204 ++++++++++++++++ .../resume_picker_transcript_preview_tests.rs | 230 ++++++++++++++++++ 3 files changed, 442 insertions(+), 147 deletions(-) create mode 100644 codex-rs/tui/src/resume_picker_transcript_preview.rs create mode 100644 codex-rs/tui/src/resume_picker_transcript_preview_tests.rs diff --git a/codex-rs/tui/src/resume_picker.rs b/codex-rs/tui/src/resume_picker.rs index c031cc3fb8..3123bd54af 100644 --- a/codex-rs/tui/src/resume_picker.rs +++ b/codex-rs/tui/src/resume_picker.rs @@ -5,13 +5,9 @@ use std::path::PathBuf; use std::sync::Arc; use crate::app_server_session::AppServerSession; -use crate::app_server_session::HISTORY_ITEM_PAGE_LIMIT; -use crate::app_server_session::HISTORY_ITEM_SCAN_LIMIT; use crate::clipboard_paste::normalize_pasted_search_query; use crate::color::blend; use crate::color::is_light; -use crate::git_action_directives::parse_assistant_markdown; -use crate::inline_visualization::InlineVisualizationContext; use crate::key_hint::KeyBindingListExt; use crate::key_hint::is_plain_text_key_event; use crate::keymap::ListAction; @@ -44,8 +40,6 @@ use codex_app_server_protocol::RequestId; use codex_app_server_protocol::Thread; use codex_app_server_protocol::ThreadArchiveParams; use codex_app_server_protocol::ThreadArchiveResponse; -use codex_app_server_protocol::ThreadHistoryMode; -use codex_app_server_protocol::ThreadItem; use codex_app_server_protocol::ThreadListCwdFilter; use codex_app_server_protocol::ThreadListParams; use codex_app_server_protocol::ThreadSortKey; @@ -85,6 +79,11 @@ mod page_loading; use page_loading::PageLoadMode; use page_loading::PaginationState; +#[path = "resume_picker_transcript_preview.rs"] +mod transcript_preview; + +pub(crate) use transcript_preview::load_transcript_preview; + const PAGE_SIZE: usize = 25; const LOAD_NEAR_THRESHOLD: usize = 5; const SESSION_META_INDENT_WIDTH: usize = 2; @@ -836,13 +835,13 @@ enum SessionTranscriptState { Failed, } -#[derive(Clone)] +#[derive(Clone, Debug, Eq, PartialEq)] pub(crate) struct TranscriptPreviewLine { speaker: TranscriptPreviewSpeaker, text: String, } -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Debug, Eq, PartialEq)] enum TranscriptPreviewSpeaker { User, Assistant, @@ -882,145 +881,6 @@ async fn load_app_server_page( }) } -pub(crate) async fn load_transcript_preview( - app_server: &mut AppServerSession, - thread_id: ThreadId, - config: Option<&Config>, -) -> std::io::Result> { - const MAX_PREVIEW_LINES: usize = 6; - - let mut thread = app_server - .thread_read(thread_id, /*include_turns*/ false) - .await - .map_err(std::io::Error::other)?; - if thread.history_mode == ThreadHistoryMode::Legacy { - app_server - .hydrate_initial_thread_history( - &mut thread, - /*turn_cursor*/ None, - /*item_cursor*/ None, - /*config*/ None, - crate::app_server_session::HistoryHydrationScope::Initial, - ) - .await - .map_err(std::io::Error::other)?; - } - let cwd = thread.cwd.as_path(); - let inline_visualization_context = config.and_then(|config| { - ThreadId::from_string(&thread.id) - .ok() - .and_then(|thread_id| InlineVisualizationContext::from_config(config, thread_id)) - }); - let mut lines = if thread.history_mode == ThreadHistoryMode::Paginated { - let mut groups = Vec::new(); - let mut visible_lines = 0_usize; - let mut scanned_items = 0_usize; - let mut cursor = None; - let mut seen_cursors = HashSet::new(); - loop { - let page = app_server - .thread_items_page( - thread_id, - /*turn_id*/ None, - cursor.clone(), - HISTORY_ITEM_PAGE_LIMIT, - ) - .await - .map_err(std::io::Error::other)?; - scanned_items = scanned_items.saturating_add(page.data.len()); - for entry in page.data { - let item_lines = transcript_preview_lines_for_item( - &entry.item, - cwd, - inline_visualization_context.as_ref(), - ); - visible_lines = visible_lines.saturating_add(item_lines.len()); - if !item_lines.is_empty() { - groups.push(item_lines); - } - if visible_lines >= MAX_PREVIEW_LINES { - break; - } - } - if visible_lines >= MAX_PREVIEW_LINES || scanned_items >= HISTORY_ITEM_SCAN_LIMIT { - break; - } - let Some(next_cursor) = page - .next_cursor - .filter(|next| seen_cursors.insert(next.clone())) - else { - break; - }; - cursor = Some(next_cursor); - } - groups.into_iter().rev().flatten().collect::>() - } else { - thread - .turns - .iter() - .flat_map(|turn| turn.items.iter()) - .flat_map(|item| { - transcript_preview_lines_for_item(item, cwd, inline_visualization_context.as_ref()) - }) - .collect::>() - }; - if lines.len() > MAX_PREVIEW_LINES { - lines.drain(..lines.len() - MAX_PREVIEW_LINES); - } - Ok(lines) -} - -fn transcript_preview_lines_for_item( - item: &ThreadItem, - cwd: &Path, - inline_visualization_context: Option<&InlineVisualizationContext>, -) -> Vec { - let line = match item { - ThreadItem::UserMessage { content, .. } => TranscriptPreviewLine { - speaker: TranscriptPreviewSpeaker::User, - text: content - .iter() - .filter_map(|input| match input { - codex_app_server_protocol::UserInput::Text { text, .. } => Some(text.as_str()), - _ => None, - }) - .collect::>() - .join(" "), - }, - ThreadItem::AgentMessage { text, .. } => { - let visible_markdown = parse_assistant_markdown(text, cwd).visible_markdown; - let rewritten = crate::inline_visualization::rewrite_inline_visualizations( - &visible_markdown, - inline_visualization_context, - ); - let mut text = rewritten.markdown.into_owned(); - for (placeholder, link) in &rewritten.trusted_file_links { - text = text.replace( - &format!( - "{} \n[{}]({placeholder})", - link.markdown_label, link.markdown_destination_label - ), - &format!("{} \n{}", link.display_label, link.destination), - ); - } - TranscriptPreviewLine { - speaker: TranscriptPreviewSpeaker::Assistant, - text, - } - } - _ => return Vec::new(), - }; - - line.text - .lines() - .filter(|text| !text.trim().is_empty()) - .map(|text| TranscriptPreviewLine { - speaker: line.speaker, - text: text.trim().to_string(), - }) - .collect() -} - impl SearchState { fn active_token(&self) -> Option { match self { @@ -3567,6 +3427,7 @@ fn render_empty_state_line(state: &PickerState) -> Line<'static> { mod tests { use super::*; use chrono::Duration; + use codex_app_server_protocol::ThreadItem; use codex_app_server_protocol::ThreadSourceKind; use codex_config::CONFIG_TOML_FILE; use codex_protocol::ThreadId; diff --git a/codex-rs/tui/src/resume_picker_transcript_preview.rs b/codex-rs/tui/src/resume_picker_transcript_preview.rs new file mode 100644 index 0000000000..5caf471a5e --- /dev/null +++ b/codex-rs/tui/src/resume_picker_transcript_preview.rs @@ -0,0 +1,204 @@ +use std::borrow::Cow; +use std::collections::HashSet; +use std::path::Path; + +use super::TranscriptPreviewLine; +use super::TranscriptPreviewSpeaker; +use crate::app_server_session::AppServerSession; +use crate::app_server_session::HISTORY_ITEM_PAGE_LIMIT; +use crate::app_server_session::HISTORY_ITEM_SCAN_LIMIT; +use crate::app_server_session::HistoryHydrationScope; +use crate::git_action_directives::parse_assistant_markdown; +use crate::inline_visualization::InlineVisualizationContext; +use crate::legacy_core::config::Config; +use codex_app_server_protocol::ThreadHistoryMode; +use codex_app_server_protocol::ThreadItem; +use codex_protocol::ThreadId; + +const MAX_TRANSCRIPT_PREVIEW_LINES: usize = 6; +const TRANSCRIPT_PREVIEW_ITEMS_PAGE_SIZE: u32 = 6; + +/// Loads the newest nonblank transcript lines within a bounded paginated-item scan. +pub(crate) async fn load_transcript_preview( + app_server: &mut AppServerSession, + thread_id: ThreadId, + config: Option<&Config>, +) -> std::io::Result> { + let mut thread = app_server + .thread_read(thread_id, /*include_turns*/ false) + .await + .map_err(std::io::Error::other)?; + if thread.history_mode == ThreadHistoryMode::Legacy { + app_server + .hydrate_initial_thread_history( + &mut thread, + /*turn_cursor*/ None, + /*item_cursor*/ None, + /*config*/ None, + HistoryHydrationScope::Initial, + ) + .await + .map_err(std::io::Error::other)?; + } + let cwd = thread.cwd.as_path(); + let inline_visualization_context = config.and_then(|config| { + ThreadId::from_string(&thread.id) + .ok() + .and_then(|thread_id| InlineVisualizationContext::from_config(config, thread_id)) + }); + let mut lines = Vec::with_capacity(MAX_TRANSCRIPT_PREVIEW_LINES); + match thread.history_mode { + ThreadHistoryMode::Legacy => { + append_transcript_preview_lines( + &mut lines, + thread + .turns + .iter() + .rev() + .flat_map(|turn| turn.items.iter().rev()), + cwd, + inline_visualization_context.as_ref(), + ); + } + ThreadHistoryMode::Paginated => { + let mut cursor = None; + let mut seen_cursors = HashSet::new(); + let mut scanned_items = 0_usize; + loop { + let remaining_items = HISTORY_ITEM_SCAN_LIMIT.saturating_sub(scanned_items); + let page_size = if cursor.is_none() { + TRANSCRIPT_PREVIEW_ITEMS_PAGE_SIZE + } else { + HISTORY_ITEM_PAGE_LIMIT + } + .min(remaining_items as u32); + if page_size == 0 { + break; + } + let page = app_server + .thread_items_page(thread_id, /*turn_id*/ None, cursor.clone(), page_size) + .await + .map_err(std::io::Error::other)?; + scanned_items = scanned_items.saturating_add(page.data.len()); + append_transcript_preview_lines( + &mut lines, + page.data + .iter() + .take(remaining_items) + .map(|entry| &entry.item), + cwd, + inline_visualization_context.as_ref(), + ); + if lines.len() == MAX_TRANSCRIPT_PREVIEW_LINES + || scanned_items >= HISTORY_ITEM_SCAN_LIMIT + { + break; + } + let Some(next_cursor) = page + .next_cursor + .filter(|next| seen_cursors.insert(next.clone())) + else { + break; + }; + cursor = Some(next_cursor); + } + } + } + + lines.reverse(); + Ok(lines) +} + +/// Appends the newest preview lines from items already ordered newest-first. +fn append_transcript_preview_lines<'a>( + lines: &mut Vec, + items: impl Iterator, + cwd: &Path, + inline_visualization_context: Option<&InlineVisualizationContext>, +) { + for item in items { + match item { + ThreadItem::UserMessage { content, .. } => { + let text = content + .iter() + .filter_map(|input| match input { + codex_app_server_protocol::UserInput::Text { text, .. } => { + Some(text.as_str()) + } + _ => None, + }) + .collect::>() + .join(" "); + append_transcript_preview_text( + lines, + TranscriptPreviewSpeaker::User, + &text, + cwd, + inline_visualization_context, + ); + } + ThreadItem::AgentMessage { text, .. } => { + append_transcript_preview_text( + lines, + TranscriptPreviewSpeaker::Assistant, + text, + cwd, + inline_visualization_context, + ); + } + _ => continue, + } + if lines.len() == MAX_TRANSCRIPT_PREVIEW_LINES { + break; + } + } +} + +/// Appends a message's newest nonblank lines while preserving assistant display rewrites. +fn append_transcript_preview_text( + lines: &mut Vec, + speaker: TranscriptPreviewSpeaker, + text: &str, + cwd: &Path, + inline_visualization_context: Option<&InlineVisualizationContext>, +) { + let visible_markdown; + let text = match speaker { + TranscriptPreviewSpeaker::User => Cow::Borrowed(text), + TranscriptPreviewSpeaker::Assistant => { + visible_markdown = parse_assistant_markdown(text, cwd).visible_markdown; + let rewritten = crate::inline_visualization::rewrite_inline_visualizations( + &visible_markdown, + inline_visualization_context, + ); + let mut text = rewritten.markdown; + for (placeholder, link) in &rewritten.trusted_file_links { + text = Cow::Owned(text.replace( + &format!( + "{} \n[{}]({placeholder})", + link.markdown_label, link.markdown_destination_label + ), + &format!("{} \n{}", link.display_label, link.destination), + )); + } + text + } + }; + + let remaining = MAX_TRANSCRIPT_PREVIEW_LINES - lines.len(); + lines.extend( + text.lines() + .rev() + .map(str::trim) + .filter(|text| !text.is_empty()) + .take(remaining) + .map(|text| TranscriptPreviewLine { + speaker, + text: text.to_string(), + }), + ); +} + +#[cfg(test)] +#[path = "resume_picker_transcript_preview_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/resume_picker_transcript_preview_tests.rs b/codex-rs/tui/src/resume_picker_transcript_preview_tests.rs new file mode 100644 index 0000000000..a817ebde25 --- /dev/null +++ b/codex-rs/tui/src/resume_picker_transcript_preview_tests.rs @@ -0,0 +1,230 @@ +use super::*; +use crate::legacy_core::config::ConfigBuilder; +use app_test_support::create_mock_responses_server_sequence; +use app_test_support::write_mock_responses_config_toml; +use codex_app_server_client::AppServerEvent; +use codex_app_server_protocol::ClientRequest; +use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::ServerNotification; +use codex_app_server_protocol::SortDirection; +use codex_app_server_protocol::ThreadItemsListParams; +use codex_app_server_protocol::ThreadItemsListResponse; +use codex_app_server_protocol::ThreadStartParams; +use codex_app_server_protocol::ThreadStartResponse; +use codex_app_server_protocol::TurnStartParams; +use codex_app_server_protocol::TurnStartResponse; +use codex_app_server_protocol::UserInput; +use core_test_support::responses; +use pretty_assertions::assert_eq; +use tempfile::tempdir; + +fn preview_line(speaker: TranscriptPreviewSpeaker, text: &str) -> TranscriptPreviewLine { + TranscriptPreviewLine { + speaker, + text: text.to_string(), + } +} + +#[test] +fn transcript_preview_reverse_scan_stops_before_older_items() { + let newest = ThreadItem::AgentMessage { + id: String::from("final"), + text: String::from("one\ntwo\nthree\nfour\nfive\nsix"), + phase: None, + memory_citation: None, + }; + let mut lines = Vec::new(); + + append_transcript_preview_lines( + &mut lines, + std::iter::once(&newest).chain(std::iter::from_fn(|| { + panic!("preview should stop after finding six lines") + })), + Path::new("/tmp"), + /*inline_visualization_context*/ None, + ); + + assert_eq!( + lines, + vec![ + preview_line(TranscriptPreviewSpeaker::Assistant, "six"), + preview_line(TranscriptPreviewSpeaker::Assistant, "five"), + preview_line(TranscriptPreviewSpeaker::Assistant, "four"), + preview_line(TranscriptPreviewSpeaker::Assistant, "three"), + preview_line(TranscriptPreviewSpeaker::Assistant, "two"), + preview_line(TranscriptPreviewSpeaker::Assistant, "one"), + ] + ); +} + +async fn transcript_preview_for_history_mode( + history_mode: ThreadHistoryMode, + invisible_newer_turns: usize, +) -> Vec { + let model_response = responses::sse(vec![ + responses::ev_response_created("resp-1"), + responses::ev_assistant_message("commentary", "commentary one\ncommentary two"), + responses::ev_assistant_message("final", "final one\nfinal two\nfinal three"), + responses::ev_completed("resp-1"), + ]); + let model_responses = std::iter::once(model_response) + .chain((0..invisible_newer_turns).map(|index| { + responses::sse(vec![ + responses::ev_response_created(&format!("empty-resp-{index}")), + responses::ev_completed(&format!("empty-resp-{index}")), + ]) + })) + .collect(); + let server = create_mock_responses_server_sequence(model_responses).await; + let codex_home = tempdir().expect("tempdir"); + write_mock_responses_config_toml( + codex_home.path(), + &server.uri(), + &Default::default(), + /*auto_compact_limit*/ 100_000, + /*requires_openai_auth*/ None, + "mock_provider", + "compact", + ) + .expect("write mock config"); + let config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(codex_home.path().to_path_buf())) + .build() + .await + .expect("build config"); + let mut app_server = crate::start_embedded_app_server_for_picker(&config) + .await + .expect("start app server"); + let request_handle = app_server.request_handle(); + let started: ThreadStartResponse = request_handle + .request_typed(ClientRequest::ThreadStart { + request_id: RequestId::String(String::from("preview-thread-start")), + params: ThreadStartParams { + model: Some(String::from("mock-model")), + model_provider: Some(String::from("mock_provider")), + cwd: Some(codex_home.path().display().to_string()), + history_mode: Some(history_mode), + ..Default::default() + }, + }) + .await + .expect("start thread"); + let thread_id = ThreadId::from_string(&started.thread.id).expect("thread id"); + for index in 0..=invisible_newer_turns { + let input = if index == 0 { + UserInput::Text { + text: String::from("recent user"), + text_elements: Vec::new(), + } + } else { + UserInput::Image { + url: String::from( + "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==", + ), + detail: None, + } + }; + let _: TurnStartResponse = request_handle + .request_typed(ClientRequest::TurnStart { + request_id: RequestId::String(format!("preview-turn-start-{index}")), + params: TurnStartParams { + thread_id: thread_id.to_string(), + input: vec![input], + ..Default::default() + }, + }) + .await + .expect("start turn"); + tokio::time::timeout(std::time::Duration::from_secs(10), async { + loop { + let Some(event) = app_server.next_event().await else { + panic!("app-server event stream closed before turn completed"); + }; + if matches!( + event, + AppServerEvent::ServerNotification(notification) + if matches!(notification.as_ref(), ServerNotification::TurnCompleted(_)) + ) { + break; + } + } + }) + .await + .expect("turn should complete"); + } + + if history_mode == ThreadHistoryMode::Paginated { + let first_page: ThreadItemsListResponse = request_handle + .request_typed(ClientRequest::ThreadItemsList { + request_id: RequestId::String(String::from("preview-items-list")), + params: ThreadItemsListParams { + thread_id: thread_id.to_string(), + turn_id: None, + cursor: None, + limit: Some(TRANSCRIPT_PREVIEW_ITEMS_PAGE_SIZE), + sort_direction: Some(SortDirection::Desc), + }, + }) + .await + .expect("list first items page"); + assert_eq!(first_page.data.len(), MAX_TRANSCRIPT_PREVIEW_LINES); + let mut first_page_lines = Vec::new(); + append_transcript_preview_lines( + &mut first_page_lines, + first_page.data.iter().map(|entry| &entry.item), + codex_home.path(), + /*inline_visualization_context*/ None, + ); + assert_eq!(first_page_lines, Vec::new()); + assert!(first_page.next_cursor.is_some()); + } + + load_transcript_preview(&mut app_server, thread_id, Some(&config)) + .await + .expect("load transcript preview") +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn transcript_preview_preserves_legacy_and_paginated_output() { + let invisible_newer_turns = TRANSCRIPT_PREVIEW_ITEMS_PAGE_SIZE as usize; + let legacy = + transcript_preview_for_history_mode(ThreadHistoryMode::Legacy, invisible_newer_turns).await; + let paginated = + transcript_preview_for_history_mode(ThreadHistoryMode::Paginated, invisible_newer_turns) + .await; + + assert_eq!(legacy, paginated); + assert_eq!( + paginated, + vec![ + preview_line(TranscriptPreviewSpeaker::User, "recent user"), + preview_line(TranscriptPreviewSpeaker::Assistant, "commentary one"), + preview_line(TranscriptPreviewSpeaker::Assistant, "commentary two"), + preview_line(TranscriptPreviewSpeaker::Assistant, "final one"), + preview_line(TranscriptPreviewSpeaker::Assistant, "final two"), + preview_line(TranscriptPreviewSpeaker::Assistant, "final three"), + ] + ); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn paginated_transcript_preview_pages_past_old_item_budget() { + let invisible_newer_turns = TRANSCRIPT_PREVIEW_ITEMS_PAGE_SIZE as usize * 4; + + let lines = + transcript_preview_for_history_mode(ThreadHistoryMode::Paginated, invisible_newer_turns) + .await; + + assert_eq!( + lines, + vec![ + preview_line(TranscriptPreviewSpeaker::User, "recent user"), + preview_line(TranscriptPreviewSpeaker::Assistant, "commentary one"), + preview_line(TranscriptPreviewSpeaker::Assistant, "commentary two"), + preview_line(TranscriptPreviewSpeaker::Assistant, "final one"), + preview_line(TranscriptPreviewSpeaker::Assistant, "final two"), + preview_line(TranscriptPreviewSpeaker::Assistant, "final three"), + ] + ); +}