From b8d2b1a5764c31678238286eb4c9b80c9ff3f366 Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Tue, 16 Sep 2025 16:42:43 -0700 Subject: [PATCH] restyle thinking outputs (#3755) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Screenshot 2025-09-16 at 2 23 18 PM --- codex-rs/tui/src/chatwidget.rs | 7 +- codex-rs/tui/src/history_cell.rs | 116 +++++++++++++++++++--------- codex-rs/tui/src/markdown_stream.rs | 4 +- codex-rs/tui/src/wrapping.rs | 2 - 4 files changed, 85 insertions(+), 44 deletions(-) diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index c03e7d3b..fb91d754 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -226,12 +226,11 @@ impl ChatWidget { // At the end of a reasoning block, record transcript-only content. self.full_reasoning_buffer.push_str(&self.reasoning_buffer); if !self.full_reasoning_buffer.is_empty() { - for cell in history_cell::new_reasoning_summary_block( + let cell = history_cell::new_reasoning_summary_block( self.full_reasoning_buffer.clone(), &self.config, - ) { - self.add_boxed_history(cell); - } + ); + self.add_boxed_history(cell); } self.reasoning_buffer.clear(); self.full_reasoning_buffer.clear(); diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index d6add6d3..9cdd29dd 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -121,6 +121,45 @@ impl HistoryCell for UserHistoryCell { } } +#[derive(Debug)] +pub(crate) struct ReasoningSummaryCell { + _header: Vec>, + content: Vec>, +} + +impl ReasoningSummaryCell { + pub(crate) fn new(header: Vec>, content: Vec>) -> Self { + Self { + _header: header, + content, + } + } +} + +impl HistoryCell for ReasoningSummaryCell { + fn display_lines(&self, width: u16) -> Vec> { + let summary_lines = self + .content + .iter() + .map(|l| l.clone().dim().italic()) + .collect::>(); + + word_wrap_lines( + &summary_lines, + RtOptions::new(width as usize) + .initial_indent("• ".into()) + .subsequent_indent(" ".into()), + ) + } + + fn transcript_lines(&self) -> Vec> { + let mut out: Vec> = Vec::new(); + out.push("thinking".magenta().bold().into()); + out.extend(self.content.clone()); + out + } +} + #[derive(Debug)] pub(crate) struct AgentMessageCell { lines: Vec>, @@ -1417,7 +1456,7 @@ pub(crate) fn new_reasoning_block( pub(crate) fn new_reasoning_summary_block( full_reasoning_buffer: String, config: &Config, -) -> Vec> { +) -> Box { if config.model_family.reasoning_summary_format == ReasoningSummaryFormat::Experimental { // Experimental format is following: // ** header ** @@ -1434,27 +1473,19 @@ pub(crate) fn new_reasoning_summary_block( // then we don't have a summary to inject into history if after_close_idx < full_reasoning_buffer.len() { let header_buffer = full_reasoning_buffer[..after_close_idx].to_string(); - let summary_buffer = full_reasoning_buffer[after_close_idx..].to_string(); - - let mut header_lines: Vec> = Vec::new(); - header_lines.push(Line::from("Thinking".magenta().italic())); + let mut header_lines = Vec::new(); append_markdown(&header_buffer, &mut header_lines, config); - let mut summary_lines: Vec> = Vec::new(); - summary_lines.push(Line::from("Thinking".magenta().bold())); + let summary_buffer = full_reasoning_buffer[after_close_idx..].to_string(); + let mut summary_lines = Vec::new(); append_markdown(&summary_buffer, &mut summary_lines, config); - return vec![ - Box::new(TranscriptOnlyHistoryCell { - lines: header_lines, - }), - Box::new(AgentMessageCell::new(summary_lines, true)), - ]; + return Box::new(ReasoningSummaryCell::new(header_lines, summary_lines)); } } } } - vec![Box::new(new_reasoning_block(full_reasoning_buffer, config))] + Box::new(new_reasoning_block(full_reasoning_buffer, config)) } struct OutputLinesParams { @@ -1558,6 +1589,7 @@ mod tests { use codex_core::config::ConfigOverrides; use codex_core::config::ConfigToml; use dirs::home_dir; + use pretty_assertions::assert_eq; fn test_config() -> Config { Config::load_from_base_config_with_overrides( @@ -2076,17 +2108,35 @@ mod tests { let rendered = render_lines(&lines).join("\n"); insta::assert_snapshot!(rendered); } + #[test] + fn reasoning_summary_block() { + let mut config = test_config(); + config.model_family.reasoning_summary_format = ReasoningSummaryFormat::Experimental; + + let cell = new_reasoning_summary_block( + "**High level reasoning**\n\nDetailed reasoning goes here.".to_string(), + &config, + ); + + let rendered_display = render_lines(&cell.display_lines(80)); + assert_eq!(rendered_display, vec!["• Detailed reasoning goes here."]); + + let rendered_transcript = render_transcript(cell.as_ref()); + assert_eq!( + rendered_transcript, + vec!["thinking", "Detailed reasoning goes here."] + ); + } #[test] fn reasoning_summary_block_returns_reasoning_cell_when_feature_disabled() { let mut config = test_config(); config.model_family.reasoning_summary_format = ReasoningSummaryFormat::Experimental; - let cells = + let cell = new_reasoning_summary_block("Detailed reasoning goes here.".to_string(), &config); - assert_eq!(cells.len(), 1); - let rendered = render_transcript(cells[0].as_ref()); + let rendered = render_transcript(cell.as_ref()); assert_eq!(rendered, vec!["thinking", "Detailed reasoning goes here."]); } @@ -2095,13 +2145,12 @@ mod tests { let mut config = test_config(); config.model_family.reasoning_summary_format = ReasoningSummaryFormat::Experimental; - let cells = new_reasoning_summary_block( + let cell = new_reasoning_summary_block( "**High level reasoning without closing".to_string(), &config, ); - assert_eq!(cells.len(), 1); - let rendered = render_transcript(cells[0].as_ref()); + let rendered = render_transcript(cell.as_ref()); assert_eq!( rendered, vec!["thinking", "**High level reasoning without closing"] @@ -2113,25 +2162,23 @@ mod tests { let mut config = test_config(); config.model_family.reasoning_summary_format = ReasoningSummaryFormat::Experimental; - let cells = new_reasoning_summary_block( + let cell = new_reasoning_summary_block( "**High level reasoning without closing**".to_string(), &config, ); - assert_eq!(cells.len(), 1); - let rendered = render_transcript(cells[0].as_ref()); + let rendered = render_transcript(cell.as_ref()); assert_eq!( rendered, vec!["thinking", "High level reasoning without closing"] ); - let cells = new_reasoning_summary_block( + let cell = new_reasoning_summary_block( "**High level reasoning without closing**\n\n ".to_string(), &config, ); - assert_eq!(cells.len(), 1); - let rendered = render_transcript(cells[0].as_ref()); + let rendered = render_transcript(cell.as_ref()); assert_eq!( rendered, vec!["thinking", "High level reasoning without closing"] @@ -2143,21 +2190,18 @@ mod tests { let mut config = test_config(); config.model_family.reasoning_summary_format = ReasoningSummaryFormat::Experimental; - let cells = new_reasoning_summary_block( + let cell = new_reasoning_summary_block( "**High level plan**\n\nWe should fix the bug next.".to_string(), &config, ); - assert_eq!(cells.len(), 2); - - let header_lines = render_transcript(cells[0].as_ref()); - assert_eq!(header_lines, vec!["Thinking", "High level plan"]); - - let summary_lines = render_transcript(cells[1].as_ref()); + let rendered_display = render_lines(&cell.display_lines(80)); + assert_eq!(rendered_display, vec!["• We should fix the bug next."]); + let rendered_transcript = render_transcript(cell.as_ref()); assert_eq!( - summary_lines, - vec!["codex", "Thinking", "We should fix the bug next."] - ) + rendered_transcript, + vec!["thinking", "We should fix the bug next."] + ); } } diff --git a/codex-rs/tui/src/markdown_stream.rs b/codex-rs/tui/src/markdown_stream.rs index 7245afb1..12b43082 100644 --- a/codex-rs/tui/src/markdown_stream.rs +++ b/codex-rs/tui/src/markdown_stream.rs @@ -333,11 +333,11 @@ mod tests { ); for (i, l) in non_blank.iter().enumerate() { assert_eq!( - l.style.fg, + l.spans[0].style.fg, Some(Color::Green), "wrapped line {} should preserve green style, got {:?}", i, - l.style.fg + l.spans[0].style.fg ); } } diff --git a/codex-rs/tui/src/wrapping.rs b/codex-rs/tui/src/wrapping.rs index bb309784..c97f0e3c 100644 --- a/codex-rs/tui/src/wrapping.rs +++ b/codex-rs/tui/src/wrapping.rs @@ -187,7 +187,6 @@ where // Build first wrapped line with initial indent. let mut first_line = rt_opts.initial_indent.clone(); - first_line.style = first_line.style.patch(line.style); { let sliced = slice_line_spans(line, &span_bounds, first_line_range); let mut spans = first_line.spans; @@ -216,7 +215,6 @@ where continue; } let mut subsequent_line = rt_opts.subsequent_indent.clone(); - subsequent_line.style = subsequent_line.style.patch(line.style); let offset_range = (r.start + base)..(r.end + base); let sliced = slice_line_spans(line, &span_bounds, &offset_range); let mut spans = subsequent_line.spans;