From 081caa5a6b77cde2624d414f280af6f0701fb22f Mon Sep 17 00:00:00 2001 From: Jeremy Rose <172423086+nornagon-openai@users.noreply.github.com> Date: Wed, 6 Aug 2025 12:03:45 -0700 Subject: [PATCH] show a transient history cell for commands (#1824) Adds a new "active history cell" for history bits that need to render more than once before they're inserted into the history. Only used for commands right now. https://github.com/user-attachments/assets/925f01a0-e56d-4613-bc25-fdaa85d8aea5 --------- Co-authored-by: easong-openai --- codex-rs/tui/src/chatwidget.rs | 47 ++++++++-- codex-rs/tui/src/history_cell.rs | 143 +++++++++++++++++-------------- 2 files changed, 119 insertions(+), 71 deletions(-) diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 64b65b3d..69f1600c 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -30,6 +30,8 @@ use codex_core::protocol::TurnDiffEvent; use crossterm::event::KeyEvent; use crossterm::event::KeyEventKind; use ratatui::buffer::Buffer; +use ratatui::layout::Constraint; +use ratatui::layout::Layout; use ratatui::layout::Rect; use ratatui::widgets::Widget; use ratatui::widgets::WidgetRef; @@ -62,6 +64,7 @@ pub(crate) struct ChatWidget<'a> { app_event_tx: AppEventSender, codex_op_tx: UnboundedSender, bottom_pane: BottomPane<'a>, + active_history_cell: Option, config: Config, initial_user_message: Option, token_usage: TokenUsage, @@ -107,6 +110,17 @@ fn create_initial_user_message(text: String, image_paths: Vec) -> Optio } impl ChatWidget<'_> { + fn layout_areas(&self, area: Rect) -> [Rect; 2] { + Layout::vertical([ + Constraint::Max( + self.active_history_cell + .as_ref() + .map_or(0, |c| c.desired_height(area.width)), + ), + Constraint::Min(self.bottom_pane.desired_height(area.width)), + ]) + .areas(area) + } fn emit_stream_header(&mut self, kind: StreamKind) { use ratatui::text::Line as RLine; if self.stream_header_emitted { @@ -178,6 +192,7 @@ impl ChatWidget<'_> { has_input_focus: true, enhanced_keys_supported, }), + active_history_cell: None, config, initial_user_message: create_initial_user_message( initial_prompt.unwrap_or_default(), @@ -197,6 +212,10 @@ impl ChatWidget<'_> { pub fn desired_height(&self, width: u16) -> u16 { self.bottom_pane.desired_height(width) + + self + .active_history_cell + .as_ref() + .map_or(0, |c| c.desired_height(width)) } pub(crate) fn handle_key_event(&mut self, key_event: KeyEvent) { @@ -425,9 +444,11 @@ impl ChatWidget<'_> { cwd: cwd.clone(), }, ); - self.add_to_history(HistoryCell::new_active_exec_command(command)); + self.active_history_cell = Some(HistoryCell::new_active_exec_command(command)); + } + EventMsg::ExecCommandOutputDelta(_) => { + // TODO } - EventMsg::ExecCommandOutputDelta(_) => {} EventMsg::PatchApplyBegin(PatchApplyBeginEvent { call_id: _, auto_approved, @@ -438,8 +459,12 @@ impl ChatWidget<'_> { changes, )); } - EventMsg::PatchApplyEnd(patch_apply_end_event) => { - self.add_to_history(HistoryCell::new_patch_end_event(patch_apply_end_event)); + EventMsg::PatchApplyEnd(event) => { + self.add_to_history(HistoryCell::new_patch_apply_end( + event.stdout, + event.stderr, + event.success, + )); } EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id, @@ -450,6 +475,7 @@ impl ChatWidget<'_> { }) => { // Compute summary before moving stdout into the history cell. let cmd = self.running_commands.remove(&call_id); + self.active_history_cell = None; self.add_to_history(HistoryCell::new_completed_exec_command( cmd.map(|cmd| cmd.command).unwrap_or_else(|| vec![call_id]), CommandOutput { @@ -543,6 +569,7 @@ impl ChatWidget<'_> { CancellationEvent::Ignored => {} } if self.bottom_pane.is_task_running() { + self.active_history_cell = None; self.bottom_pane.clear_ctrl_c_quit_hint(); self.submit_op(Op::Interrupt); self.bottom_pane.set_task_running(false); @@ -596,7 +623,8 @@ impl ChatWidget<'_> { } pub fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> { - self.bottom_pane.cursor_pos(area) + let [_, bottom_pane_area] = self.layout_areas(area); + self.bottom_pane.cursor_pos(bottom_pane_area) } } @@ -700,10 +728,11 @@ impl ChatWidget<'_> { impl WidgetRef for &ChatWidget<'_> { fn render_ref(&self, area: Rect, buf: &mut Buffer) { - // In the hybrid inline viewport mode we only draw the interactive - // bottom pane; history entries are injected directly into scrollback - // via `Terminal::insert_before`. - (&self.bottom_pane).render(area, buf); + let [active_cell_area, bottom_pane_area] = self.layout_areas(area); + (&self.bottom_pane).render(bottom_pane_area, buf); + if let Some(cell) = &self.active_history_cell { + cell.render_ref(active_cell_area, buf); + } } } diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 5b7d9246..facb0e0a 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -11,7 +11,6 @@ use codex_core::plan_tool::StepStatus; use codex_core::plan_tool::UpdatePlanArgs; use codex_core::protocol::FileChange; use codex_core::protocol::McpInvocation; -use codex_core::protocol::PatchApplyEndEvent; use codex_core::protocol::SessionConfiguredEvent; use codex_core::protocol::TokenUsage; use image::DynamicImage; @@ -24,6 +23,9 @@ use ratatui::style::Modifier; use ratatui::style::Style; use ratatui::text::Line as RtLine; use ratatui::text::Span as RtSpan; +use ratatui::widgets::Paragraph; +use ratatui::widgets::WidgetRef; +use ratatui::widgets::Wrap; use std::collections::HashMap; use std::io::Cursor; use std::path::PathBuf; @@ -62,35 +64,23 @@ fn line_to_static(line: &Line) -> Line<'static> { /// scrollable list. pub(crate) enum HistoryCell { /// Welcome message. - WelcomeMessage { - view: TextBlock, - }, + WelcomeMessage { view: TextBlock }, /// Message from the user. - UserPrompt { - view: TextBlock, - }, + UserPrompt { view: TextBlock }, // AgentMessage and AgentReasoning variants were unused and have been removed. /// An exec tool call that has not finished yet. - ActiveExecCommand { - view: TextBlock, - }, + ActiveExecCommand { view: TextBlock }, /// Completed exec tool call. - CompletedExecCommand { - view: TextBlock, - }, + CompletedExecCommand { view: TextBlock }, /// An MCP tool call that has not finished yet. - ActiveMcpToolCall { - view: TextBlock, - }, + ActiveMcpToolCall { view: TextBlock }, /// Completed MCP tool call where we show the result serialized as JSON. - CompletedMcpToolCall { - view: TextBlock, - }, + CompletedMcpToolCall { view: TextBlock }, /// Completed MCP tool call where the result is an image. /// Admittedly, [mcp_types::CallToolResult] can have multiple content types, @@ -100,51 +90,34 @@ pub(crate) enum HistoryCell { // resized version avoids doing the potentially expensive rescale twice // because the scroll-view first calls `height()` for layouting and then // `render_window()` for painting. - CompletedMcpToolCallWithImageOutput { - _image: DynamicImage, - }, + CompletedMcpToolCallWithImageOutput { _image: DynamicImage }, /// Background event. - BackgroundEvent { - view: TextBlock, - }, + BackgroundEvent { view: TextBlock }, /// Output from the `/diff` command. - GitDiffOutput { - view: TextBlock, - }, + GitDiffOutput { view: TextBlock }, /// Output from the `/status` command. - StatusOutput { - view: TextBlock, - }, + StatusOutput { view: TextBlock }, /// Error event from the backend. - ErrorEvent { - view: TextBlock, - }, + ErrorEvent { view: TextBlock }, /// Info describing the newly-initialized session. - SessionInfo { - view: TextBlock, - }, + SessionInfo { view: TextBlock }, /// A pending code patch that is awaiting user approval. Mirrors the /// behaviour of `ActiveExecCommand` so the user sees *what* patch the /// model wants to apply before being prompted to approve or deny it. - PendingPatch { - view: TextBlock, - }, - - PatchEventEnd { - view: TextBlock, - }, + PendingPatch { view: TextBlock }, /// A human‑friendly rendering of the model's current plan and step /// statuses provided via the `update_plan` tool. - PlanUpdate { - view: TextBlock, - }, + PlanUpdate { view: TextBlock }, + + /// Result of applying a patch (success or failure) with optional output. + PatchApplyResult { view: TextBlock }, } const TOOL_CALL_MAX_LINES: usize = 5; @@ -165,8 +138,8 @@ impl HistoryCell { | HistoryCell::CompletedExecCommand { view } | HistoryCell::CompletedMcpToolCall { view } | HistoryCell::PendingPatch { view } - | HistoryCell::PatchEventEnd { view } | HistoryCell::PlanUpdate { view } + | HistoryCell::PatchApplyResult { view } | HistoryCell::ActiveExecCommand { view, .. } | HistoryCell::ActiveMcpToolCall { view, .. } => { view.lines.iter().map(line_to_static).collect() @@ -177,6 +150,15 @@ impl HistoryCell { ], } } + + pub(crate) fn desired_height(&self, width: u16) -> u16 { + Paragraph::new(Text::from(self.plain_lines())) + .wrap(Wrap { trim: false }) + .line_count(width) + .try_into() + .unwrap_or(0) + } + pub(crate) fn new_session_info( config: &Config, event: SessionConfiguredEvent, @@ -612,7 +594,10 @@ impl HistoryCell { PatchEventType::ApplyBegin { auto_approved: false, } => { - let lines = vec![Line::from("patch applied".magenta().bold())]; + let lines: Vec> = vec![ + Line::from("applying patch".magenta().bold()), + Line::from(""), + ]; return Self::PendingPatch { view: TextBlock::new(lines), }; @@ -661,29 +646,63 @@ impl HistoryCell { } } - pub(crate) fn new_patch_end_event(patch_apply_end_event: PatchApplyEndEvent) -> Self { - let PatchApplyEndEvent { - call_id: _, - stdout: _, - stderr, - success, - } = patch_apply_end_event; + pub(crate) fn new_patch_apply_end(stdout: String, stderr: String, success: bool) -> Self { + let mut lines: Vec> = Vec::new(); - let mut lines: Vec> = if success { - vec![Line::from("patch applied successfully".italic())] + let status = if success { + RtSpan::styled("patch applied", Style::default().fg(Color::Green)) } else { - let mut lines = vec![Line::from("patch failed".italic())]; - lines.extend(stderr.lines().map(|l| Line::from(l.to_string()))); - lines + RtSpan::styled( + "patch failed", + Style::default().fg(Color::Red).add_modifier(Modifier::BOLD), + ) }; + lines.push(RtLine::from(vec![ + "patch".magenta().bold(), + " ".into(), + status, + ])); + + let src = if success { + if stdout.trim().is_empty() { + &stderr + } else { + &stdout + } + } else if stderr.trim().is_empty() { + &stdout + } else { + &stderr + }; + + if !src.trim().is_empty() { + lines.push(Line::from("")); + let mut iter = src.lines(); + for raw in iter.by_ref().take(TOOL_CALL_MAX_LINES) { + lines.push(ansi_escape_line(raw).dim()); + } + let remaining = iter.count(); + if remaining > 0 { + lines.push(Line::from(format!("... {remaining} additional lines")).dim()); + } + } + lines.push(Line::from("")); - HistoryCell::PatchEventEnd { + HistoryCell::PatchApplyResult { view: TextBlock::new(lines), } } } +impl WidgetRef for &HistoryCell { + fn render_ref(&self, area: Rect, buf: &mut Buffer) { + Paragraph::new(Text::from(self.plain_lines())) + .wrap(Wrap { trim: false }) + .render(area, buf); + } +} + fn create_diff_summary(changes: HashMap) -> Vec { // Build a concise, human‑readable summary list similar to the // `git status` short format so the user can reason about the