From b52a272cff73ee81ead57f7417144c2161d75f35 Mon Sep 17 00:00:00 2001 From: Ian MacLeod Date: Mon, 26 Oct 2026 21:27:35 +0000 Subject: [PATCH] Render async questions and replies with Markdown in the TUI (#57292) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why Async question replies appeared as plain user text, leaving Markdown formatting literal and repeating the full question above each answer. ## What changed - Accent question titles with a `?` prefix while preserving Markdown in titles and suggestions. - Render replies with a `↪ Re:` question caption and a Markdown answer. Limit captions to two rows with an ellipsis on overflow, while keeping answers complete. - Apply the same rendering to live messages and loaded transcripts, preserving inline styles, hyperlinks, and Markdown when copying selections. ## Testing Add snapshots and assertions for matching live and loaded transcript rendering, plus tests for caption clipping, hyperlink bounds, Markdown copying, partial selections, and whitespace-only or nonprinting answers. GitOrigin-RevId: 0290d8a2ebfb42bd5d7374c7037cde5ae8a13b5d --- codex-rs/tui/src/async_question_reply.rs | 29 +++-- .../src/bottom_pane/hooks_browser_render.rs | 5 +- codex-rs/tui/src/chatwidget.rs | 15 ++- codex-rs/tui/src/chatwidget/streaming.rs | 4 +- .../src/chatwidget/tests/questions_tests.rs | 58 ++++++++- ...ions_tests__async_question_transcript.snap | 31 +++++ ...s_tests__desktop_async_question_reply.snap | 22 +++- codex-rs/tui/src/chatwidget/user_messages.rs | 19 +-- .../tui/src/history_cell/async_questions.rs | 82 ++++++++++++ .../tui/src/history_cell/async_replies.rs | 118 +++++++++++++++++ codex-rs/tui/src/history_cell/messages.rs | 122 ++++++++++++------ codex-rs/tui/src/history_cell/mod.rs | 2 + codex-rs/tui/src/markdown_copy.rs | 20 +++ codex-rs/tui/src/terminal_hyperlinks.rs | 45 +++++-- .../tui/src/terminal_hyperlinks/source.rs | 4 + codex-rs/tui/src/thread_transcript.rs | 43 +++--- .../transcript_view/markdown_copy_tests.rs | 96 ++++++++++++++ 17 files changed, 599 insertions(+), 116 deletions(-) create mode 100644 codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__async_question_transcript.snap create mode 100644 codex-rs/tui/src/history_cell/async_questions.rs create mode 100644 codex-rs/tui/src/history_cell/async_replies.rs diff --git a/codex-rs/tui/src/async_question_reply.rs b/codex-rs/tui/src/async_question_reply.rs index 0930137d01..f8a4e6f8d7 100644 --- a/codex-rs/tui/src/async_question_reply.rs +++ b/codex-rs/tui/src/async_question_reply.rs @@ -2,14 +2,17 @@ //! Only complete envelopes, optionally following the standard IDE context prefix, are interpreted. use codex_app_server_protocol::UserInput; +use codex_context_fragments::AnsweredQuestion; +use codex_context_fragments::ContextualUserFragment; +use itertools::Itertools; use serde::Deserialize; -#[derive(Debug, Deserialize, PartialEq)] +#[derive(Clone, Debug, Deserialize, PartialEq, Eq)] #[serde(rename_all = "camelCase")] pub(crate) struct AsyncQuestionReply { pub(crate) question_item_id: String, - question: String, - answer: String, + pub(crate) question: String, + pub(crate) answer: String, } pub(crate) fn parse(text: &str) -> Option> { @@ -26,9 +29,8 @@ pub(crate) fn parse(text: &str) -> Option> { } else { text }; - let json = text - .strip_prefix("")? - .strip_suffix("")?; + let (start, end) = AnsweredQuestion::type_markers(); + let json = text.strip_prefix(start)?.strip_suffix(end)?; let replies = match serde_json::from_str::(json).ok()? { Replies::Many(replies) => replies, Replies::One(reply) => vec![reply], @@ -37,13 +39,14 @@ pub(crate) fn parse(text: &str) -> Option> { } pub(crate) fn display_text(text: &str) -> Option { - Some( - parse(text)? - .iter() - .map(|reply| format!("> {}\n\n{}", reply.question, reply.answer)) - .collect::>() - .join("\n\n"), - ) + Some(display_replies(&parse(text)?)) +} + +pub(crate) fn display_replies(replies: &[AsyncQuestionReply]) -> String { + replies + .iter() + .map(|reply| format!("> {}\n\n{}", reply.question, reply.answer)) + .join("\n\n") } pub(crate) fn parse_input(input: &[UserInput]) -> Option> { diff --git a/codex-rs/tui/src/bottom_pane/hooks_browser_render.rs b/codex-rs/tui/src/bottom_pane/hooks_browser_render.rs index fa7017fb55..7b4b0e1271 100644 --- a/codex-rs/tui/src/bottom_pane/hooks_browser_render.rs +++ b/codex-rs/tui/src/bottom_pane/hooks_browser_render.rs @@ -233,10 +233,7 @@ pub(super) fn detail_wrapped_lines( .collect::(); let visible_end = prefix_width + crate::width::display_width(content.trim_end_matches('…')); last_span.content = content.into(); - last_line.hyperlinks.retain_mut(|link| { - link.columns.end = link.columns.end.min(visible_end); - !link.columns.is_empty() - }); + last_line.clip_hyperlinks_to_width(visible_end); } lines } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 5cf8b6ac3d..da2d0536ff 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -1413,12 +1413,15 @@ impl ChatWidget { || !display.local_images.is_empty() || !display.remote_image_urls.is_empty() { - self.add_to_history(history_cell::new_user_prompt( - display.message, - display.text_elements, - display.local_images, - display.remote_image_urls, - )); + self.add_to_history( + history_cell::new_user_prompt( + display.message, + display.text_elements, + display.local_images, + display.remote_image_urls, + ) + .with_async_replies(display.replies, self.config.cwd.as_path()), + ); } } diff --git a/codex-rs/tui/src/chatwidget/streaming.rs b/codex-rs/tui/src/chatwidget/streaming.rs index 7dee87c171..6229fd735a 100644 --- a/codex-rs/tui/src/chatwidget/streaming.rs +++ b/codex-rs/tui/src/chatwidget/streaming.rs @@ -455,7 +455,8 @@ impl ChatWidget { } } let parsed = parse_assistant_markdown(&message, self.config.cwd.as_path()); - if from_replay + let questions = item.questions.as_deref().unwrap_or_default(); + if (from_replay || !questions.is_empty()) && self.stream_controller.is_none() && !parsed.visible_markdown.is_empty() && !self.completed_agent_message_was_flushed(&message) @@ -475,6 +476,7 @@ impl ChatWidget { self.config.cwd.as_path(), context, ) + .with_questions(questions) .with_copy_source(Some(message.clone())), ); self.handle_stream_finished(); diff --git a/codex-rs/tui/src/chatwidget/tests/questions_tests.rs b/codex-rs/tui/src/chatwidget/tests/questions_tests.rs index 13c345bd1c..9a37fabf65 100644 --- a/codex-rs/tui/src/chatwidget/tests/questions_tests.rs +++ b/codex-rs/tui/src/chatwidget/tests/questions_tests.rs @@ -982,7 +982,7 @@ prompt_stack_back = ["ctrl-x down", "shift-right"] #[tokio::test] async fn desktop_async_answer_dismisses_only_its_question_and_preserves_the_other_draft() { for replay_kind in [None, Some(ReplayKind::ThreadSnapshot)] { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; let questions = vec![ question("Same title?", /*options*/ None), question("Same title?", /*options*/ None), @@ -997,7 +997,7 @@ async fn desktop_async_answer_dismisses_only_its_question_and_preserves_the_othe chat.bottom_pane .handle_paste("My draft for the second question".into()); let reply = r#" -[{"questionItemId":"[\"request_user_input_async\",\"questions\",0]","question":"Same title?","answer":"Answer from desktop"}] +[{"questionItemId":"[\"request_user_input_async\",\"questions\",0]","question":"Same title?","answer":"Answer from **desktop**"}] "#; let reply = format!( "# Context from my IDE setup:\n\n## Open tabs:\n- lib.rs: src/lib.rs\n\n## My request for Codex:\n{reply}" @@ -1010,6 +1010,7 @@ async fn desktop_async_answer_dismisses_only_its_question_and_preserves_the_othe text_elements: Vec::new(), }], }; + drain_insert_history(&mut rx); chat.handle_server_notification( ServerNotification::ItemCompleted(ItemCompletedNotification::new( "thread".into(), @@ -1028,9 +1029,16 @@ async fn desktop_async_answer_dismisses_only_its_question_and_preserves_the_othe crate::thread_transcript::RawReasoningVisibility::Hidden, /*config*/ None, ); - insta::assert_snapshot!( - "desktop_async_question_reply", - lines_to_single_string(&cells[0].transcript_lines(/*width*/ 80)) + crate::terminal_palette::with_test_default_colors( + crate::terminal_probe::DefaultColors { + fg: (220, 220, 220), + bg: (20, 20, 20), + }, + || { + let persisted = cells[0].transcript_lines(/*width*/ 80); + insta::assert_debug_snapshot!("desktop_async_question_reply", persisted); + assert_eq!(drain_insert_history(&mut rx), [persisted]); + }, ); insta::assert_snapshot!( "async_question_answered_on_another_client", @@ -1054,6 +1062,46 @@ async fn desktop_async_answer_dismisses_only_its_question_and_preserves_the_othe } } +#[tokio::test] +async fn async_question_markdown_matches_loaded_history() { + let (mut chat, mut rx, _ops) = make_chatwidget_manual(/*model_override*/ None).await; + let title = "Keep `--verbose` for **failed** runs?"; + let item = AppServerThreadItemPayload::AgentMessage { + id: "ask".into(), + text: format!("{title}\n- Only for **CI**\n\nSee the [guide](https://example.test/guide)?"), + phase: None, + memory_citation: None, + delivery: None, + questions: Some(vec![ + question(title, Some(vec!["Only for **CI**".into()])), + question( + "See the [guide](https://example.test/guide)?", + /*options*/ None, + ), + ]), + }; + drain_insert_history(&mut rx); + crate::terminal_palette::with_test_default_colors( + crate::terminal_probe::DefaultColors { + fg: (220, 220, 220), + bg: (20, 20, 20), + }, + || { + chat.handle_thread_item(item.clone(), "turn-1".into(), ThreadItemRenderSource::Live); + let cells = drain_insert_history(&mut rx); + let paged = crate::thread_transcript::thread_items_to_transcript_cells( + /*thread_id*/ None, + &chat.config.cwd, + [item], + crate::thread_transcript::RawReasoningVisibility::Hidden, + /*config*/ None, + ); + assert_eq!(cells, [paged[0].transcript_lines(/*width*/ 80)]); + insta::assert_debug_snapshot!("async_question_transcript", cells[0]); + }, + ); +} + #[tokio::test] async fn distinct_async_question_replies_with_identical_text_both_render() { use codex_context_fragments::AnsweredQuestion; diff --git a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__async_question_transcript.snap b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__async_question_transcript.snap new file mode 100644 index 0000000000..78ecd694c1 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__async_question_transcript.snap @@ -0,0 +1,31 @@ +--- +source: tui/src/chatwidget/tests/questions_tests.rs +expression: "cells[0]" +--- +[ + Line::from_iter([ + Span::from("? ").fg(Color::Rgb(99, 168, 248)), + Span::from("Keep ").fg(Color::Rgb(99, 168, 248)), + Span::from("--verbose").fg(Color::Rgb(166, 227, 161)), + Span::from(" for ").fg(Color::Rgb(99, 168, 248)), + Span::from("failed").fg(Color::Rgb(243, 139, 168)).bold(), + Span::from(" runs?").fg(Color::Rgb(99, 168, 248)), + ]), + Line::default(), + Line::from_iter([ + Span::from(" "), + Span::from("• ").fg(Color::Rgb(148, 226, 213)), + Span::from("Only for "), + Span::from("CI").fg(Color::Rgb(243, 139, 168)).bold(), + ]), + Line::default(), + Line::from_iter([ + Span::from("? ").fg(Color::Rgb(99, 168, 248)), + Span::from("See the ").fg(Color::Rgb(99, 168, 248)), + Span::from("guide").fg(Color::Rgb(137, 180, 250)).underlined(), + Span::from(" (").fg(Color::Rgb(99, 168, 248)), + Span::from("https://example.test/guide").fg(Color::Rgb(137, 180, 250)).underlined(), + Span::from(")").fg(Color::Rgb(99, 168, 248)), + Span::from("?").fg(Color::Rgb(99, 168, 248)), + ]), +] diff --git a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__desktop_async_question_reply.snap b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__desktop_async_question_reply.snap index 0e6dcf8d13..6b0df05473 100644 --- a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__desktop_async_question_reply.snap +++ b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__desktop_async_question_reply.snap @@ -1,9 +1,19 @@ --- source: tui/src/chatwidget/tests/questions_tests.rs -assertion_line: 856 -expression: "lines_to_single_string(&cells[0].transcript_lines(80))" +expression: persisted --- - -› > Same title? - - Answer from desktop +[ + Line::default().bg(Color::Rgb(57, 57, 57)), + Line::from_iter([ + Span::from("↪ ").fg(Color::Rgb(99, 168, 248)).bg(Color::Rgb(57, 57, 57)).not_dim(), + Span::from("Re: ").fg(Color::Rgb(99, 168, 248)).bg(Color::Rgb(57, 57, 57)).dim(), + Span::from("Same title?").fg(Color::Rgb(99, 168, 248)).bg(Color::Rgb(57, 57, 57)).dim(), + ]).bg(Color::Rgb(57, 57, 57)), + Line::default().bg(Color::Rgb(57, 57, 57)), + Line::from_iter([ + Span::from(" "), + Span::from("Answer from "), + Span::from("desktop").fg(Color::Rgb(243, 139, 168)).bold(), + ]).bg(Color::Rgb(57, 57, 57)), + Line::default().bg(Color::Rgb(57, 57, 57)), +] diff --git a/codex-rs/tui/src/chatwidget/user_messages.rs b/codex-rs/tui/src/chatwidget/user_messages.rs index 864760984b..729d7982eb 100644 --- a/codex-rs/tui/src/chatwidget/user_messages.rs +++ b/codex-rs/tui/src/chatwidget/user_messages.rs @@ -578,7 +578,7 @@ pub(super) fn merge_user_messages_with_history_record( pub(crate) struct UserMessageDisplay { pub(crate) message: String, // Keep distinct replies distinct when their visible question and answer text match. - question_ids: Vec, + pub(super) replies: Vec, pub(crate) remote_image_urls: Vec, pub(crate) local_images: Vec, pub(crate) text_elements: Vec, @@ -709,20 +709,13 @@ impl ChatWidget { local_images: Vec, remote_image_urls: Vec, ) -> UserMessageDisplay { - let question_ids = crate::async_question_reply::parse(&message) - .unwrap_or_default() - .into_iter() - .map(|reply| reply.question_item_id) - .collect(); - let reply_text = crate::async_question_reply::display_text(&message); + let replies = crate::async_question_reply::parse(&message); let (message, prompt_request_offset) = crate::user_prompt::extract_prompt_request_with_offset(&message); - if let Some(message) = - reply_text.or_else(|| crate::async_question_reply::display_text(message)) - { + if let Some(replies) = replies.or_else(|| crate::async_question_reply::parse(message)) { return UserMessageDisplay { - message, - question_ids, + message: crate::async_question_reply::display_replies(&replies), + replies, text_elements: Vec::new(), local_images, remote_image_urls, @@ -750,7 +743,7 @@ impl ChatWidget { UserMessageDisplay { message: message.to_string(), - question_ids: Vec::new(), + replies: Vec::new(), remote_image_urls, local_images, text_elements, diff --git a/codex-rs/tui/src/history_cell/async_questions.rs b/codex-rs/tui/src/history_cell/async_questions.rs new file mode 100644 index 0000000000..9462ef4c12 --- /dev/null +++ b/codex-rs/tui/src/history_cell/async_questions.rs @@ -0,0 +1,82 @@ +//! Render each tagged title and the following suggestions from the tool's visible Markdown. + +use crate::style::accent_color_on; +use crate::terminal_hyperlinks::HyperlinkLine; +use crate::terminal_hyperlinks::prefix_hyperlink_lines; +use codex_protocol::items::AsyncUserInputQuestion; +use ratatui::style::Style; +use ratatui::text::Span; +use std::ops::Range; + +/// Core joins each title and its Markdown bullets, then separates questions by a blank line. +/// Stop decorating if the metadata and visible Markdown diverge. +pub(super) fn title_ranges( + source: &str, + questions: &[AsyncUserInputQuestion], +) -> Vec> { + let mut titles = Vec::new(); + let mut rest = source; + for question in questions { + rest = rest.trim_start_matches('\n'); + let title = question.title.replace("\r\n", "\n"); + let title = title.trim_matches('\n'); + let Some(after_title) = rest.strip_prefix(title) else { + break; + }; + let start = source.len() - rest.len(); + titles.push(start..start + title.len()); + rest = after_title; + for option in question.options.iter().flatten() { + let option = option.replace("\r\n", "\n"); + let Some(after_option) = rest + .trim_start_matches('\n') + .strip_prefix("- ") + .and_then(|s| s.strip_prefix(&option)) + else { + return titles; + }; + rest = after_option; + } + } + titles +} + +pub(super) fn render( + source: &str, + titles: &[Range], + markdown: impl Fn(&str) -> Vec, +) -> Vec { + let accent = Style::new().fg(accent_color_on(/*background*/ None)); + let mut lines = Vec::new(); + for (index, title) in titles.iter().enumerate() { + if !lines.is_empty() { + lines.push(HyperlinkLine::default()); + } + let heading = markdown(&source[title.clone()]) + .into_iter() + .map(|row| { + let style = accent.patch(row.line.style); + row.style(style).resolve_span_styles() + }) + .collect(); + lines.extend(prefix_hyperlink_lines( + heading, + Span::styled("? ", accent), + " ".into(), + )); + + let end = titles + .get(index + 1) + .map_or(source.len(), |next| next.start); + let body = source[title.end..end].trim_matches('\n'); + if !body.is_empty() { + lines.push(HyperlinkLine::default()); + lines.extend(prefix_hyperlink_lines( + markdown(body), + " ".into(), + " ".into(), + )); + } + } + lines +} diff --git a/codex-rs/tui/src/history_cell/async_replies.rs b/codex-rs/tui/src/history_cell/async_replies.rs new file mode 100644 index 0000000000..e2f76354cf --- /dev/null +++ b/codex-rs/tui/src/history_cell/async_replies.rs @@ -0,0 +1,118 @@ +//! Render structured replies on the user-message surface. Only the repeated question is shortened. + +use super::messages::sanitize_user_text; +use crate::async_question_reply::AsyncQuestionReply; +use crate::line_truncation::truncate_line_to_width; +use crate::markdown_render::hide_web_link_destination; +use crate::markdown_render::render_markdown_lines_with_width_cwd_and_hidden_link_destinations; +use crate::style::accent_color_on; +use crate::style::history_prompt_style; +use crate::terminal_hyperlinks::HyperlinkLine; +use crate::terminal_hyperlinks::LogicalLineSource; +use crate::terminal_hyperlinks::prefix_hyperlink_lines; +use crate::terminal_hyperlinks::remap_source_wrapped_line; +use crate::terminal_hyperlinks::retain_initial_prefix; +use crate::wrapping::RtOptions; +use crate::wrapping::adaptive_wrap_line_to_width; +use ratatui::style::Stylize; +use std::path::Path; + +pub(super) fn render( + replies: &[AsyncQuestionReply], + cwd: &Path, + width: usize, +) -> Vec { + let surface = history_prompt_style(); + let accent = accent_color_on(surface.bg); + let markdown = |text: &str, width| { + render_markdown_lines_with_width_cwd_and_hidden_link_destinations( + text, + width, + Some(cwd), + &hide_web_link_destination, + ) + }; + let mut lines = Vec::new(); + for reply in replies { + if !lines.is_empty() { + lines.push(HyperlinkLine::default().style(surface)); + } + let mut question = markdown(&sanitize_user_text(reply.question.as_str().into()), None); + if question.is_empty() { + question.push(HyperlinkLine::default()); + } + let mut wrapped = question + .into_iter() + .enumerate() + .flat_map(|(index, row)| wrap(row, width, if index == 0 { "Re: " } else { "" })); + let mut caption: Vec<_> = wrapped.by_ref().take(2).collect(); + if wrapped.next().is_some() + && let Some(last) = caption.last_mut() + { + last.line = + truncate_line_to_width(std::mem::take(&mut last.line), width.saturating_sub(1)); + last.clip_hyperlinks_to_width(last.width()); + last.line.push_span("…"); + // Copy the visible excerpt as prose alongside the Markdown answer. + caption.iter_mut().for_each(mark_as_prose); + } else { + retain_initial_prefix(&mut caption, "Re: "); + } + let caption = prefix_hyperlink_lines(caption, "↪ ".fg(accent).not_dim(), " ".into()); + lines.extend(caption.into_iter().map(|row| { + let style = surface.fg(accent).dim().patch(row.line.style); + row.style(style).resolve_span_styles() + })); + let text = sanitize_user_text(reply.answer.as_str().into()); + if text.trim().is_empty() { + continue; + } + let mut answer = markdown(&text, Some(width)); + if !answer + .iter() + .flat_map(|row| &row.line.spans) + .any(|span| !span.content.trim().is_empty()) + { + answer = text + .lines() + .map(str::to_owned) + .map(HyperlinkLine::from) + .collect(); + answer.iter_mut().for_each(mark_as_prose); + } + lines.push(HyperlinkLine::default().style(surface)); + let answer = answer + .into_iter() + .flat_map(|row| wrap(row, width, "")) + .map(|row| { + let style = row.line.style.patch(surface); + row.style(style) + }) + .collect(); + lines.extend(prefix_hyperlink_lines(answer, " ".into(), " ".into())); + } + lines +} + +fn mark_as_prose(row: &mut HyperlinkLine) { + row.source = Some(LogicalLineSource { + copy_as_prose: true, + ..LogicalLineSource::from_line(&row.line) + }); +} + +// Markdown leaves some code and URLs overwide. Keep all of the row and its links in the gutter. +fn wrap(row: HyperlinkLine, width: usize, initial: &'static str) -> Vec { + if initial.is_empty() && row.width() <= width { + return vec![row]; + } + let indent = row + .source + .as_ref() + .map(|s| s.continuation_indent.clone()) + .unwrap_or_default(); + let options = RtOptions::new(width) + .initial_indent(initial.into()) + .subsequent_indent(indent); + remap_source_wrapped_line(&row, adaptive_wrap_line_to_width(&row.line, options)) +} diff --git a/codex-rs/tui/src/history_cell/messages.rs b/codex-rs/tui/src/history_cell/messages.rs index 99b41b1b39..f4f4943e6f 100644 --- a/codex-rs/tui/src/history_cell/messages.rs +++ b/codex-rs/tui/src/history_cell/messages.rs @@ -3,6 +3,7 @@ use super::markdown_render_cache::MarkdownRenderCache; use super::*; +use crate::async_question_reply::AsyncQuestionReply; use crate::style::accent_color_on; use crate::style::history_prompt_style; use crate::terminal_hyperlinks::annotate_web_urls; @@ -26,6 +27,14 @@ pub(crate) enum UserMessagePresentation { #[default] Plain, Spoken, + /// Render each submitted answer as Markdown beneath its question caption. + AsyncReplies { + /// Keep the decoded pairs so the two-row limit applies only to question captions. + /// In the flattened message, an answer's own blockquotes can look like captions. + replies: Vec, + /// Capture the session directory so relative Markdown links resolve the same way on reflow. + cwd: PathBuf, + }, } /// Remove CSI sequences and control characters, preserving tabs and newlines. @@ -159,6 +168,20 @@ fn build_user_message_lines_with_elements( } impl UserHistoryCell { + pub(crate) fn with_async_replies( + mut self, + replies: Vec, + cwd: &Path, + ) -> Self { + if !replies.is_empty() { + self.presentation = UserMessagePresentation::AsyncReplies { + replies, + cwd: cwd.to_owned(), + }; + } + self + } + pub(crate) fn is_spoken(&self) -> bool { matches!(self.presentation, UserMessagePresentation::Spoken) } @@ -220,41 +243,43 @@ impl HistoryCell for UserHistoryCell { .wrap_algorithm(textwrap::WrapAlgorithm::FirstFit), )); - let wrapped_message = wrap_user_message(message, text_elements, style, wrap_width); - - if wrapped_images.is_empty() && wrapped_message.is_none() { + let wrapped_message = + if let UserMessagePresentation::AsyncReplies { replies, cwd } = &self.presentation { + super::async_replies::render(replies, cwd, usize::from(wrap_width)) + } else { + let marker = if self.is_spoken() { + "› ".red().bold() + } else { + "› ".bold().dim() + }; + prefix_hyperlink_lines( + wrap_user_message(message, text_elements, style, wrap_width), + marker, + " ".into(), + ) + }; + if wrapped_images.is_empty() && wrapped_message.is_empty() { return Vec::new(); } - let mut lines = vec![HyperlinkLine::new(Line::from("").style(style))]; - if !wrapped_images.is_empty() { lines.extend(prefix_hyperlink_lines( wrapped_images, " ".into(), " ".into(), )); - if wrapped_message.is_some() { + if !wrapped_message.is_empty() { lines.push(HyperlinkLine::new(Line::from("").style(style))); } } - - if let Some(wrapped_message) = wrapped_message { - lines.extend(prefix_hyperlink_lines( - wrapped_message, - if self.is_spoken() { - "› ".red().bold() - } else { - "› ".bold().dim() - }, - " ".into(), - )); - } - + lines.extend(wrapped_message); lines.push(HyperlinkLine::new(Line::from("").style(style))); for source in lines.iter_mut().filter_map(|line| line.source.as_mut()) { source.right_reserve = 1; - source.copy_as_prose = true; + source.copy_as_prose |= matches!( + self.presentation, + UserMessagePresentation::Plain | UserMessagePresentation::Spoken + ); } lines } @@ -283,10 +308,10 @@ fn wrap_user_message( text_elements: &[TextElement], style: Style, wrap_width: u16, -) -> Option> { +) -> Vec { let element_style = style.fg(accent_color_on(style.bg)); if message.is_empty() && text_elements.is_empty() { - return None; + return Vec::new(); } let wrap_options = @@ -328,7 +353,7 @@ fn wrap_user_message( }) { wrapped.pop(); } - (!wrapped.is_empty()).then_some(wrapped) + wrapped } #[derive(Debug)] @@ -518,6 +543,7 @@ pub(crate) struct AgentMarkdownCell { inline_visualization_context: Option, rendered_lines: Option, spoken_artifacts: bool, + question_titles: Vec>, } impl AgentMarkdownCell { @@ -557,9 +583,19 @@ impl AgentMarkdownCell { inline_visualization_context, rendered_lines, spoken_artifacts: false, + question_titles: Vec::new(), } } + pub(crate) fn with_questions( + mut self, + questions: &[codex_protocol::items::AsyncUserInputQuestion], + ) -> Self { + self.question_titles = + super::async_questions::title_ranges(&self.markdown_source, questions); + self + } + pub(crate) fn new_spoken(markdown_source: String, cwd: &Path) -> Self { let mut cell = Self::new_with_inline_visualizations( markdown_source, @@ -609,25 +645,29 @@ impl AgentMarkdownCell { // 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_list_spacing( - &self.markdown_source, - Some(wrap_width), - Some(self.cwd.as_path()), - self.inline_visualization_context.as_ref(), - list_spacing, - ); - let lines = if self.spoken_artifacts { - let mut lines = lines; - super::spoken_artifacts::annotate_spoken_artifacts(&mut lines, &self.cwd); - lines - } else { - lines + let markdown = |source: &str| { + crate::markdown::render_markdown_agent_with_list_spacing( + source, + Some(wrap_width), + Some(self.cwd.as_path()), + self.inline_visualization_context.as_ref(), + list_spacing, + ) }; - normalize_whitespace_only_hyperlink_lines(prefix_hyperlink_lines( - lines, - "• ".dim(), - " ".into(), - )) + let lines = if self.question_titles.is_empty() { + let mut lines = markdown(&self.markdown_source); + if self.spoken_artifacts { + super::spoken_artifacts::annotate_spoken_artifacts(&mut lines, &self.cwd); + } + prefix_hyperlink_lines(lines, "• ".dim(), " ".into()) + } else { + super::async_questions::render( + &self.markdown_source, + &self.question_titles, + markdown, + ) + }; + normalize_whitespace_only_hyperlink_lines(lines) }; if let Some(rendered_lines) = &self.rendered_lines { diff --git a/codex-rs/tui/src/history_cell/mod.rs b/codex-rs/tui/src/history_cell/mod.rs index 5b14559f63..610a5280ce 100644 --- a/codex-rs/tui/src/history_cell/mod.rs +++ b/codex-rs/tui/src/history_cell/mod.rs @@ -104,6 +104,8 @@ const RAW_DIFF_SUMMARY_WIDTH: usize = 10_000; mod activity_details; pub(crate) mod activity_preview; mod approvals; +mod async_questions; +mod async_replies; mod base; mod dynamic; mod exec; diff --git a/codex-rs/tui/src/markdown_copy.rs b/codex-rs/tui/src/markdown_copy.rs index ffdf4f35d5..3713b148df 100644 --- a/codex-rs/tui/src/markdown_copy.rs +++ b/codex-rs/tui/src/markdown_copy.rs @@ -292,6 +292,26 @@ impl SelectedLine { range: Range, separator: &str, ) { + if let Some((label_end, body)) = source.labelled_body.as_deref() { + let body_start = source.text.len() - body.text.len(); + let mut separator = separator; + if range.start < *label_end { + let mut label = LogicalLineSource::new(source.text[..*label_end].to_owned()); + label.copy_as_prose = true; + Self::append( + lines, + label, + range.start..range.end.min(*label_end), + separator, + ); + separator = ""; + } + if range.end > body_start { + let body_range = range.start.saturating_sub(body_start)..range.end - body_start; + Self::append(lines, body.clone(), body_range, separator); + } + return; + } // Streaming can split the same logical line across history cells. if let Some(previous) = lines.last_mut() && Arc::ptr_eq(&previous.source.text, &source.text) diff --git a/codex-rs/tui/src/terminal_hyperlinks.rs b/codex-rs/tui/src/terminal_hyperlinks.rs index 83d949b3b5..2fafe33aa8 100644 --- a/codex-rs/tui/src/terminal_hyperlinks.rs +++ b/codex-rs/tui/src/terminal_hyperlinks.rs @@ -202,6 +202,24 @@ impl HyperlinkLine { } } + pub(crate) fn clip_hyperlinks_to_width(&mut self, width: usize) { + self.hyperlinks.retain_mut(|link| { + link.columns.end = link.columns.end.min(width); + !link.columns.is_empty() + }); + } + + /// Native scrollback paints the row style last. Resolve text styles onto the spans so + /// inline Markdown and gutter overrides survive, keeping the background for empty columns. + pub(crate) fn resolve_span_styles(mut self) -> Self { + let style = std::mem::take(&mut self.line.style); + self.line.style.bg = style.bg; + for span in &mut self.line.spans { + span.style = style.patch(span.style); + } + self + } + pub(crate) fn style(mut self, style: ratatui::style::Style) -> Self { self.line = self.line.style(style); if let Some(source) = &mut self.source { @@ -283,24 +301,32 @@ pub(crate) fn prefix_hyperlink_lines( /// Retain a known first-line label in copy text without changing the existing wrapping. /// -/// Callers pass the suffix of the first row's synthetic prefix that carries meaning, such as -/// a checkbox or `answer:`. Only that row's shared logical source is updated. +/// The label may precede Markdown indentation or follow a display-only gutter. +/// Only the first row's shared logical source is updated. pub(crate) fn retain_initial_prefix(lines: &mut [HyperlinkLine], prefix: &str) { let Some(origin) = lines.first().and_then(|line| line.source.clone()) else { return; }; - debug_assert!(origin.prefix_bytes >= prefix.len()); - let prefix_line = LogicalLineSource::from_line(&lines[0].line) - .styled_range(origin.prefix_bytes - prefix.len()..origin.prefix_bytes); + let first = LogicalLineSource::from_line(&lines[0].line); + let indent = &first.text[..origin.prefix_bytes]; + let Some(label_start) = indent.rfind(prefix) else { + return; + }; + let retained = &indent[label_start..]; + let prefix_line = first.styled_range(label_start..origin.prefix_bytes); let mut styles = LogicalLineSource::from_line(&prefix_line).styles.to_vec(); styles.extend(origin.styles.iter().map(|(range, style)| { ( - range.start + prefix.len()..range.end + prefix.len(), + range.start + retained.len()..range.end + retained.len(), origin.line_style.patch(*style).patch(origin.span_style), ) })); let styles = std::sync::Arc::from(styles); - let text: std::sync::Arc = format!("{prefix}{}", origin.text).into(); + let text: std::sync::Arc = format!("{retained}{}", origin.text).into(); + let labelled_body = origin + .copy + .as_ref() + .map(|_| std::sync::Arc::new((prefix.len(), origin.clone()))); for (index, line) in lines.iter_mut().enumerate() { let Some(source) = line .source @@ -311,12 +337,13 @@ pub(crate) fn retain_initial_prefix(lines: &mut [HyperlinkLine], prefix: &str) { }; source.text = std::sync::Arc::clone(&text); source.styles = std::sync::Arc::clone(&styles); + source.labelled_body = labelled_body.clone(); source.line_style = ratatui::style::Style::default(); source.span_style = ratatui::style::Style::default(); - source.range = source.range.start + prefix.len()..source.range.end + prefix.len(); + source.range = source.range.start + retained.len()..source.range.end + retained.len(); if index == 0 { source.range.start = 0; - source.prefix_bytes -= prefix.len(); + source.prefix_bytes -= retained.len(); } } } diff --git a/codex-rs/tui/src/terminal_hyperlinks/source.rs b/codex-rs/tui/src/terminal_hyperlinks/source.rs index 92039bf11a..b4142a3130 100644 --- a/codex-rs/tui/src/terminal_hyperlinks/source.rs +++ b/codex-rs/tui/src/terminal_hyperlinks/source.rs @@ -24,6 +24,8 @@ pub(crate) enum LineWrapPolicy { pub(crate) struct LogicalLineSource { pub(crate) text: Arc, pub(crate) copy: Option>, + /// Selectable label length and the original body, whose Markdown ranges exclude the label. + pub(crate) labelled_body: Option>, /// Literal user text remains prose when copied alongside rendered Markdown. pub(crate) copy_as_prose: bool, /// Span styles share the text's byte coordinates, including whitespace omitted by wrapping. @@ -46,6 +48,7 @@ impl LogicalLineSource { Self { text: text.into(), copy: None, + labelled_body: None, copy_as_prose: false, styles: vec![(0..end, Style::default())].into(), line_style: Style::default(), @@ -106,6 +109,7 @@ impl LogicalLineSource { Self { text: Arc::clone(&self.text), copy: self.copy.clone(), + labelled_body: self.labelled_body.clone(), copy_as_prose: self.copy_as_prose, styles: Arc::clone(&self.styles), line_style: self.line_style, diff --git a/codex-rs/tui/src/thread_transcript.rs b/codex-rs/tui/src/thread_transcript.rs index 15f573a1b1..b2322a57a2 100644 --- a/codex-rs/tui/src/thread_transcript.rs +++ b/codex-rs/tui/src/thread_transcript.rs @@ -12,7 +12,6 @@ use crate::history_cell::HistoryCell; use crate::history_cell::PlainHistoryCell; use crate::history_cell::PrefixedWrappedHistoryCell; use crate::history_cell::ReasoningSummaryCell; -use crate::history_cell::UserHistoryCell; use crate::history_cell::split_reasoning_summary_parts; use crate::inline_visualization::InlineVisualizationContext; use crate::legacy_core::config::Config; @@ -252,28 +251,36 @@ fn item_to_cells( .collect(), }; let message = item.message(); - let reply_text = crate::async_question_reply::display_text(&message); - let text_elements = if reply_text.is_some() { - Vec::new() + let replies = crate::async_question_reply::parse(&message).unwrap_or_default(); + let (message, elements) = if replies.is_empty() { + (message, item.text_elements()) } else { - item.text_elements() + ( + crate::async_question_reply::display_replies(&replies), + Vec::new(), + ) }; - cells.push(Arc::new(UserHistoryCell { - presentation: Default::default(), - message: reply_text.unwrap_or(message), - text_elements, - local_image_paths: item.local_image_paths(), - remote_image_urls: item.image_urls(), - })); + let cell = crate::history_cell::new_user_prompt( + message, + elements, + item.local_image_paths(), + item.image_urls(), + ); + cells.push(Arc::new(cell.with_async_replies(replies, cwd.as_path()))); } - ThreadItemPayload::AgentMessage { text, .. } => { + ThreadItemPayload::AgentMessage { + text, questions, .. + } => { let parsed = parse_assistant_markdown(&text, cwd.as_path()); if !parsed.visible_markdown.trim().is_empty() { - cells.push(Arc::new(AgentMarkdownCell::new_with_inline_visualizations( - parsed.visible_markdown, - cwd.as_path(), - inline_visualization_context, - ))); + cells.push(Arc::new( + AgentMarkdownCell::new_with_inline_visualizations( + parsed.visible_markdown, + cwd.as_path(), + inline_visualization_context, + ) + .with_questions(questions.as_deref().unwrap_or_default()), + )); } } ThreadItemPayload::FunctionCallOutput { diff --git a/codex-rs/tui/src/transcript_view/markdown_copy_tests.rs b/codex-rs/tui/src/transcript_view/markdown_copy_tests.rs index 6cf9290f8f..c75b5f7138 100644 --- a/codex-rs/tui/src/transcript_view/markdown_copy_tests.rs +++ b/codex-rs/tui/src/transcript_view/markdown_copy_tests.rs @@ -5,6 +5,7 @@ use crate::history_cell::HistoryCell; use pretty_assertions::assert_eq; use ratatui::buffer::Buffer; use ratatui::layout::Rect; +use ratatui::style::Modifier; use std::path::Path; #[path = "table_copy_tests.rs"] @@ -304,6 +305,101 @@ fn mixed_plain_output_remains_literal_in_html() { ); } +fn reply_rows(question: &str, answer: &str, width: u16) -> Vec { + let reply = crate::async_question_reply::AsyncQuestionReply { + question_item_id: "ask".into(), + question: question.into(), + answer: answer.into(), + }; + crate::history_cell::new_user_prompt(answer.into(), vec![], vec![], vec![]) + .with_async_replies(vec![reply], Path::new("/")) + .display_hyperlink_lines(width) +} + +#[test] +fn reply_answer_distinguishes_whitespace_from_nonprinting_markdown() { + let definition = "[docs]: https://example.test"; + for (answer, expected) in [ + (" \t\n ", vec!["", "↪ Re: Why?", ""]), + ( + definition, + vec!["", "↪ Re: Why?", "", " [docs]: https://example.test", ""], + ), + ] { + let rows: Vec<_> = reply_rows("Why?", answer, /*width*/ 60) + .into_iter() + .map(|row| row.line.to_string()) + .collect(); + assert_eq!(rows, expected); + } +} + +#[test] +fn reply_caption_clips_on_overflow_and_copies_with_its_markdown_answer() { + for (width, clipped) in [(17, false), (12, true)] { + let rows = reply_rows( + "https://example.test", + "12345678901234567890\n\n**Tail**", + width, + ); + let last = &rows[2]; // The cell starts with one padding row and two caption rows. + assert_eq!(last.line.to_string().ends_with('…'), clipped); + assert_eq!( + last.hyperlinks.last().unwrap().columns.end, + last.width() - usize::from(clipped) + ); + let tail = rows + .iter() + .flat_map(|row| &row.line.spans) + .find(|span| span.content == "Tail") + .unwrap(); + assert!(tail.style.add_modifier.contains(Modifier::BOLD)); + let layout = TextLayout::new(rows, width); + let (markdown, _) = payload(&layout, 0..layout.text().len()); + assert_eq!(markdown.contains('…'), clipped); + assert_eq!(markdown.matches("Re: ").count(), 1, "{markdown}"); + assert!( + markdown.contains("12345678901234567890") && markdown.contains("**Tail**"), + "{markdown}" + ); + assert!(!markdown.contains("```"), "{markdown}"); + } +} + +#[test] +fn full_reply_caption_preserves_links_and_partial_selections() { + let width = 44; + for prefix in ["", "> ", "- > "] { + let question = format!("{prefix}Read the [setup guide](https://example.test/setup)?"); + let rows = reply_rows(&question, "Yes", width); + let first = rows[1].line.to_string(); + let layout = TextLayout::new(rows, width); + assert!( + layout + .text() + .trim_start() + .starts_with(first.trim_start_matches("↪ ").trim_end()), + "{}", + layout.text() + ); + let (copy, _) = payload(&layout, 0..layout.text().len()); + assert!( + copy.trim_start() + .starts_with(&format!("Re: {prefix}Read the [setup guide")), + "{copy}" + ); + assert!(copy.contains("]()?"), "{copy}"); + let start = layout.text().find("guide").unwrap(); + let question_layout = markdown_layout(&question, width); + let original = question_layout.text().find("guide").unwrap(); + assert_eq!( + payload(&layout, start..start + 5), + payload(&question_layout, original..original + 5), + "{prefix}" + ); + } +} + #[test] fn mixed_user_prose_preserves_literal_text_without_code_fences() { let cell = crate::history_cell::new_user_prompt(