From ca41ed337fde50dce0b9fee4cd894d93699b3175 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Sat, 26 Sep 2026 04:02:02 +0000 Subject: [PATCH] Show turn tips while working and after completion in the TUI (#48352) ## What changed - Show a random tip beneath the working status after 30 seconds, using the existing tooltip catalog and current key bindings. - Show completion tips only after successful turns with a final answer, starting with the third turn and spacing displayed tips by at least three turn starts. Limit completion tips to two per app session, and skip them when a working tip was already shown for that turn. - Respect `tui.show_tooltips`, hide tips during composer or transcript interactions, and omit tips when space is insufficient. Count exposure only when a tip is rendered. - Anchor completion tips after queued history updates, keep them outside transcript selection and search, and dismiss them on history replay or transcript clearing. ## Testing Add lifecycle tests for timing, completion cadence, duplicate notifications, failed and interrupted turns, and hidden tips. Add rendering snapshots and assertions for completion ordering, resizing, limited terminal space, and exclusion from transcript selection and search. GitOrigin-RevId: 9e956f5b0377e3be71f9574639c3c6ef11ad1168 --- codex-rs/tui/src/app.rs | 2 + codex-rs/tui/src/app/event_dispatch.rs | 4 + codex-rs/tui/src/app/history_ui.rs | 1 + codex-rs/tui/src/app/owned_transcript.rs | 43 ++- .../tui/src/app/owned_transcript_tests.rs | 7 +- ...turn_tips__tests__turn_tip_placements.snap | 110 ++++++ codex-rs/tui/src/app/startup.rs | 1 + codex-rs/tui/src/app/test_support.rs | 1 + codex-rs/tui/src/app/tests.rs | 2 + codex-rs/tui/src/app/thread_routing.rs | 6 + codex-rs/tui/src/app/turn_tips.rs | 235 ++++++++++++ codex-rs/tui/src/app/turn_tips_tests.rs | 347 ++++++++++++++++++ codex-rs/tui/src/app_event.rs | 5 + .../bottom_pane/chat_composer/footer_state.rs | 1 + codex-rs/tui/src/bottom_pane/mod.rs | 6 + codex-rs/tui/src/chatwidget/rendering.rs | 4 + .../tui/src/chatwidget/rendering_tests.rs | 1 + codex-rs/tui/src/lib.rs | 1 + codex-rs/tui/src/tooltips.rs | 6 +- codex-rs/tui/src/transcript_view.rs | 3 + codex-rs/tui/src/transcript_view/layout.rs | 9 +- .../tui/src/transcript_view/layout_tests.rs | 3 + codex-rs/tui/src/transcript_view/text.rs | 12 +- codex-rs/tui/src/transcript_view/turn_tip.rs | 59 +++ codex-rs/tui/src/turn_tip.rs | 32 ++ 25 files changed, 887 insertions(+), 14 deletions(-) create mode 100644 codex-rs/tui/src/app/snapshots/codex_tui__app__turn_tips__tests__turn_tip_placements.snap create mode 100644 codex-rs/tui/src/app/turn_tips.rs create mode 100644 codex-rs/tui/src/app/turn_tips_tests.rs create mode 100644 codex-rs/tui/src/transcript_view/turn_tip.rs create mode 100644 codex-rs/tui/src/turn_tip.rs diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index 7868caa193..25c6f2c732 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -230,6 +230,7 @@ mod managed_worktree_creation; mod misalignment_policy; mod model_defaults; mod new_session; +mod turn_tips; pub(crate) use new_session::has_launch_setting; mod clipboard; mod native_history; @@ -581,6 +582,7 @@ pub(crate) struct App { pub(crate) transcript_cells: Vec>, native_history: native_history::NativeHistory, + turn_tips: turn_tips::TurnTips, pub(crate) transcript_view: crate::transcript_view::TranscriptView, last_rendered_history_tail: Option, last_thread_usage_status_cell: Option, diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 4e19fb3969..48c60cd129 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -834,6 +834,10 @@ impl App { AppEvent::InsertHistoryCell(cell) => { self.insert_history_cell(tui, cell); } + AppEvent::TurnTipReady { thread_id, turn_id } => { + self.turn_tips.ready(thread_id, &turn_id, self.transcript_cells.last()); + tui.frame_requester().schedule_frame(); + } AppEvent::EndInitialHistoryReplayBuffer => { self.scrollback_has_older_history = self .chat_widget diff --git a/codex-rs/tui/src/app/history_ui.rs b/codex-rs/tui/src/app/history_ui.rs index fe379b2a57..d6928226f3 100644 --- a/codex-rs/tui/src/app/history_ui.rs +++ b/codex-rs/tui/src/app/history_ui.rs @@ -395,6 +395,7 @@ impl App { pub(super) fn reset_transcript_state_after_clear(&mut self) { self.overlay = None; self.transcript_cells.clear(); + self.turn_tips.dismiss(); self.chat_widget.warning_display_state.dismissed.clear(); self.chat_widget.warning_display_state.transcript = Arc::default(); self.chat_widget.warning_display_state.synced_cells = None; diff --git a/codex-rs/tui/src/app/owned_transcript.rs b/codex-rs/tui/src/app/owned_transcript.rs index fd6b707a72..c4b9dbd2c3 100644 --- a/codex-rs/tui/src/app/owned_transcript.rs +++ b/codex-rs/tui/src/app/owned_transcript.rs @@ -1,9 +1,10 @@ //! Compose the owned transcript above the composer and route their selection gestures. //! Reserve a cleared row below activity and previews, immediately above the composer. //! Slash suggestions overlay already-painted rows so opening or closing them leaves transcript -//! geometry unchanged. +//! geometry unchanged. Turn tips remain separate from selectable transcript text. //! Plain Enter returns an empty composer to latest after transcript interactions and prompt editing. +use super::turn_tips::TipSurface; use super::*; use crate::history_cell::HistoryRenderMode; use crate::keymap::KeymapContext; @@ -86,6 +87,16 @@ impl App { self.sync_owned_transcript(screen_size.width); let transcript_width = self.chat_widget.history_wrap_width(screen_size.width); let composer_hint = self.composer_hint(transcript_width); + let now = Instant::now(); + let turn_tip = self.turn_tip(transcript_width, now, &tui.frame_requester()); + let working_tip = turn_tip + .as_ref() + .filter(|(surface, _)| *surface == TipSurface::Working) + .map(|(_, tip)| tip); + let completion_tip = turn_tip + .as_ref() + .filter(|(surface, _)| *surface == TipSurface::Completion) + .map(|(_, tip)| tip); let mut composer_gap = (!self.chat_widget.has_active_view() && !self.chat_widget.is_external_writer_view()) .then(crate::bottom_pane::ComposerGap::default); @@ -114,6 +125,7 @@ impl App { crate::bottom_pane::CommandPopupPlacement::Overlay }, composer_gap.as_ref(), + working_tip, ); let dashboard_visible = chat_widget .selected_index_for_present_view(AGENTS_OVERVIEW_VIEW_ID) @@ -136,21 +148,22 @@ impl App { let mut rendered_cursor = None; let mut footer_height_changed = false; let mut feedback_tick = None; - let now = Instant::now(); + let mut transcript_bottom = available.saturating_sub(u16::from(composer_gap.is_none())); tui.draw(screen_size.height, |frame| { ratatui::widgets::Clear.render( Rect::new(/*x*/ 0, /*y*/ 0, screen_size.width, available), frame.buffer, ); - view.render( + let mut completion_tip_area = view.render_with_turn_tip_space( Rect::new( /*x*/ 0, /*y*/ 0, transcript_width, - available.saturating_sub(u16::from(composer_gap.is_none())), + transcript_bottom, ), frame.buffer, &self.transcript_cells, + completion_tip, ); if let Some(gap) = composer_gap.as_mut() { gap.needs_separator = available > 1 @@ -180,6 +193,7 @@ impl App { crate::bottom_pane::CommandPopupPlacement::Overlay }, composer_gap.as_ref(), + working_tip, ); footer_height_changed = !dashboard_visible && bottom @@ -195,14 +209,24 @@ impl App { // Resolve controls with the compact viewport first, then make room for // their separator. Resizing must not preserve a stale return control. ratatui::widgets::Clear.render(bottom_area, frame.buffer); - view.render( - Rect::new(/*x*/ 0, /*y*/ 0, transcript_width, bottom_area.y), + transcript_bottom = bottom_area.y; + completion_tip_area = view.render_with_turn_tip_space( + Rect::new( + /*x*/ 0, + /*y*/ 0, + transcript_width, + transcript_bottom, + ), frame.buffer, &self.transcript_cells, + completion_tip, ); footer_height_changed = false; } bottom.render(bottom_area, frame.buffer); + if let (Some(tip), Some(area)) = (completion_tip, completion_tip_area) { + tip.render(area, frame.buffer); + } let follow_area = if let Some(gap) = composer_gap.as_ref() { Some(Rect { width: transcript_width, @@ -228,6 +252,11 @@ impl App { frame.set_cursor_position(position); } })?; + if let Some((surface, tip)) = &turn_tip + && tip.rendered.get() + { + self.turn_tips.acknowledge(*surface); + } if footer_height_changed { tui.frame_requester().schedule_frame(); } @@ -525,7 +554,7 @@ impl App { #[cfg(test)] #[path = "owned_transcript_tests.rs"] -mod tests; +pub(super) mod tests; #[cfg(test)] #[path = "empty_state_animation_tests.rs"] diff --git a/codex-rs/tui/src/app/owned_transcript_tests.rs b/codex-rs/tui/src/app/owned_transcript_tests.rs index 48c247aed9..46c0da6d9e 100644 --- a/codex-rs/tui/src/app/owned_transcript_tests.rs +++ b/codex-rs/tui/src/app/owned_transcript_tests.rs @@ -23,7 +23,7 @@ use crossterm::event::MouseEventKind::Up; use pretty_assertions::assert_eq; use ratatui::buffer::Buffer; -fn user_cell(message: &str) -> Arc { +pub(in crate::app) fn user_cell(message: &str) -> Arc { Arc::new(UserHistoryCell { spoken: false, message: message.to_string(), @@ -33,7 +33,7 @@ fn user_cell(message: &str) -> Arc { }) } -fn attach_thread(app: &mut App, thread_id: ThreadId) { +pub(in crate::app) fn attach_thread(app: &mut App, thread_id: ThreadId) { app.chat_widget.handle_thread_session(ThreadSessionState { windows_sandbox_host: crate::app::WindowsSandboxHost::Local, thread_id, @@ -59,7 +59,7 @@ fn attach_thread(app: &mut App, thread_id: ThreadId) { }); } -pub(super) fn buffer_text(buffer: &Buffer) -> String { +pub(in crate::app) fn buffer_text(buffer: &Buffer) -> String { buffer .content() .chunks(usize::from(buffer.area.width)) @@ -515,6 +515,7 @@ async fn owned_details_keep_the_composer_cursor_and_screen() -> Result<()> { /*footer*/ None, crate::bottom_pane::CommandPopupPlacement::Overlay, Some(&crate::bottom_pane::ComposerGap::default()), + /*working_tip*/ None, ) .cursor_pos(bottom_area) .expect("composer cursor"); diff --git a/codex-rs/tui/src/app/snapshots/codex_tui__app__turn_tips__tests__turn_tip_placements.snap b/codex-rs/tui/src/app/snapshots/codex_tui__app__turn_tips__tests__turn_tip_placements.snap new file mode 100644 index 0000000000..d23bcc64e9 --- /dev/null +++ b/codex-rs/tui/src/app/snapshots/codex_tui__app__turn_tips__tests__turn_tip_placements.snap @@ -0,0 +1,110 @@ +--- +source: tui/src/app/turn_tips_tests.rs +assertion_line: 337 +expression: "crate::chatwidget::tests::helpers::normalize_snapshot_paths(screens.join(\"\\n\\n\"))" +--- +working=true, response rows=1, 80x12 + + +Response row 0 + +• Working (0s • esc to interrupt) + └ Tip: Try /help. + + +› Ask Codex to do anything + + gpt-test default · /tmp/project + ? for shortcuts + +working=true, response rows=1, 80x6 +• Working (0s • esc to interrupt) + +› Ask Codex to do anything + + gpt-test default · /tmp/project + ? for shortcuts + +working=false, response rows=1, 80x12 +Show a response. + +• Done. + └ Tip: Try /help. + + [completion time] + + +› Ask Codex to do anything + + gpt-test default · /tmp/project + ? for shortcuts + +working=false, response rows=30, 80x12 +Show a response. + +• Done. + └ Tip: Try /help. + + [completion time] + + +› Ask Codex to do anything + + gpt-test default · /tmp/project + ? for shortcuts + +working=false, response rows=1, 80x9 +• Done. + + [completion time] + + +› Ask Codex to do anything + + gpt-test default · /tmp/project + ? for shortcuts + +working=false, response rows=1, 80x10 +Show a response. +• Done. + + [completion time] + + +› Ask Codex to do anything + + gpt-test default · /tmp/project + ? for shortcuts + +working=false, response rows=1, 80x11 +Show a response. +• Done. + └ Tip: Try /help. + + [completion time] + + +› Ask Codex to do anything + + gpt-test default · /tmp/project + ? for shortcuts + +working=false, response rows=1, 12x12 +Response row +0 + +• Done. + + [completion time] + + +› Ask Codex + + gpt-test … + ? for shor + +working=false, response rows=1, 80x4 + +› Ask Codex to do anything + + ? for shortcuts diff --git a/codex-rs/tui/src/app/startup.rs b/codex-rs/tui/src/app/startup.rs index cb2c77f5db..ca3c2f4800 100644 --- a/codex-rs/tui/src/app/startup.rs +++ b/codex-rs/tui/src/app/startup.rs @@ -765,6 +765,7 @@ See the Codex keymap documentation for supported actions and examples." key_chord_matcher: KeyChordMatcher::default(), transcript_cells: Vec::new(), native_history: Default::default(), + turn_tips: Default::default(), transcript_view: Default::default(), last_rendered_history_tail: None, last_thread_usage_status_cell: None, diff --git a/codex-rs/tui/src/app/test_support.rs b/codex-rs/tui/src/app/test_support.rs index 93c33b7257..c88edb3b50 100644 --- a/codex-rs/tui/src/app/test_support.rs +++ b/codex-rs/tui/src/app/test_support.rs @@ -39,6 +39,7 @@ pub(crate) async fn make_test_app() -> App { file_search, transcript_cells: Vec::new(), native_history: Default::default(), + turn_tips: Default::default(), transcript_view: Default::default(), last_rendered_history_tail: None, last_thread_usage_status_cell: None, diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 31a785431b..aaf745c149 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -6012,6 +6012,7 @@ async fn make_test_app() -> Box { file_search, transcript_cells: Vec::new(), native_history: Default::default(), + turn_tips: Default::default(), transcript_view: Default::default(), last_rendered_history_tail: None, last_thread_usage_status_cell: None, @@ -6127,6 +6128,7 @@ pub(super) async fn make_test_app_with_channels() -> ( file_search, transcript_cells: Vec::new(), native_history: Default::default(), + turn_tips: Default::default(), transcript_view: Default::default(), last_rendered_history_tail: None, last_thread_usage_status_cell: None, diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index 21c420b7a0..a3501c3a1b 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -2023,8 +2023,13 @@ impl App { match event { ThreadBufferedEvent::Notification(notification) => { self.cache_collab_receiver_threads_for_notification(notification.as_ref()); + let tip_ready = self.turn_tips.observe(¬ification, Instant::now()); self.chat_widget .handle_server_notification(*notification, /*replay_kind*/ None); + // History cells queued by completion must be applied before anchoring its tip. + if let Some(event) = tip_ready { + self.app_event_tx.send(event); + } } ThreadBufferedEvent::Request(request) => { if self @@ -2056,6 +2061,7 @@ impl App { } pub(super) fn handle_thread_event_replay(&mut self, event: ThreadBufferedEvent) { + self.turn_tips.dismiss(); match event { ThreadBufferedEvent::Notification(notification) => self .chat_widget diff --git a/codex-rs/tui/src/app/turn_tips.rs b/codex-rs/tui/src/app/turn_tips.rs new file mode 100644 index 0000000000..868f9552c6 --- /dev/null +++ b/codex-rs/tui/src/app/turn_tips.rs @@ -0,0 +1,235 @@ +//! Random foreground-turn tips with separate working and completion frequency rules. +//! A FIFO completion event anchors the tip after queued history; replay never creates tips. + +use super::*; +use crate::terminal_hyperlinks::prefix_hyperlink_lines; +use crate::turn_tip::TurnTip; +use codex_protocol::models::MessagePhase; +use rand::seq::SliceRandom; +use std::sync::Weak; + +const WORKING_DELAY: Duration = Duration::from_secs(/*secs*/ 30); +const COMPLETION_INTERVAL: usize = 3; +const COMPLETION_LIMIT: usize = 2; + +#[derive(Default)] +pub(super) struct TurnTips { + starts: usize, + completions_shown: usize, + next_completion: usize, + previous: Option<&'static str>, + current: Option, +} + +struct TurnTipState { + thread_id: ThreadId, + turn_id: String, + started_at: Instant, + phase: Phase, + has_final_answer: bool, + template: Option<&'static str>, + shown: bool, +} + +enum Phase { + Working, + WaitingForHistory, + Complete(Weak), + Finished, +} + +#[derive(Clone, Copy, PartialEq, Eq)] +pub(super) enum TipSurface { + Working, + Completion, +} + +fn is_final_answer(item: &ThreadItem) -> bool { + matches!(item, ThreadItem::AgentMessage { text, phase: Some(MessagePhase::FinalAnswer) | None, .. } + if !text.trim().is_empty()) +} + +impl TurnTips { + pub(super) fn dismiss(&mut self) { + self.current = None; + } + + pub(super) fn observe( + &mut self, + notification: &ServerNotification, + now: Instant, + ) -> Option { + if let ServerNotification::TurnStarted(started) = notification { + let thread_id = ThreadId::from_string(&started.thread_id).ok()?; + if self.current.as_ref().is_some_and(|current| { + current.thread_id == thread_id && current.turn_id == started.turn.id + }) { + return None; + } + self.starts = self.starts.saturating_add(/*rhs*/ 1); + self.current = Some(TurnTipState { + thread_id, + turn_id: started.turn.id.clone(), + started_at: now, + phase: Phase::Working, + has_final_answer: false, + template: None, + shown: false, + }); + return None; + } + let current = self.current.as_mut()?; + match notification { + ServerNotification::ItemCompleted(item) + if item.thread_id == current.thread_id.to_string() + && item.turn_id == current.turn_id => + { + current.has_final_answer |= is_final_answer(&item.item); + } + ServerNotification::TurnCompleted(completed) + if completed.thread_id == current.thread_id.to_string() + && completed.turn.id == current.turn_id + && matches!(current.phase, Phase::Working) => + { + current.has_final_answer |= completed.turn.items.iter().any(is_final_answer); + if completed.turn.status == TurnStatus::Completed + && current.has_final_answer + && !current.shown + && self.completions_shown < COMPLETION_LIMIT + && self.starts >= self.next_completion.max(COMPLETION_INTERVAL) + { + current.phase = Phase::WaitingForHistory; + return Some(AppEvent::TurnTipReady { + thread_id: current.thread_id, + turn_id: current.turn_id.clone(), + }); + } + current.phase = Phase::Finished; + } + _ => {} + } + None + } + + pub(super) fn ready( + &mut self, + thread_id: ThreadId, + turn_id: &str, + tail: Option<&Arc>, + ) { + if let Some(current) = self.current.as_mut() + && current.thread_id == thread_id + && current.turn_id == turn_id + && matches!(current.phase, Phase::WaitingForHistory) + { + current.phase = tail.map_or(Phase::Finished, |tail| { + Phase::Complete(Arc::downgrade(tail)) + }); + } + } + + pub(super) fn acknowledge(&mut self, surface: TipSurface) { + if let Some(current) = self.current.as_mut() + && !current.shown + { + current.shown = true; + self.previous = current.template; + if surface == TipSurface::Completion { + self.completions_shown += 1; + self.next_completion = self.starts.saturating_add(COMPLETION_INTERVAL); + } + } + } +} + +impl App { + pub(super) fn turn_tip( + &mut self, + width: u16, + now: Instant, + frame_requester: &tui::FrameRequester, + ) -> Option<(TipSurface, TurnTip)> { + if !self.local_settings.tui.show_tooltips + || !self.chat_widget.no_modal_or_popup_active() + || !self.chat_widget.composer_is_empty() + || self.chat_widget.is_external_writer_view() + || self.chat_widget.has_queued_follow_up_messages() + || !self.transcript_view.is_following() + || self.transcript_view.has_active_interaction() + || self.backtrack.primed + || self.backtrack.overlay_preview_active + || self.composer_hint(width).is_some() + { + return None; + } + let current = self.turn_tips.current.as_mut()?; + if self.chat_widget.thread_id() != Some(current.thread_id) { + return None; + } + let surface = match ¤t.phase { + Phase::Working if self.chat_widget.is_agent_turn_running() => { + let remaining = + WORKING_DELAY.saturating_sub(now.saturating_duration_since(current.started_at)); + if !remaining.is_zero() { + frame_requester.schedule_frame_in(remaining); + return None; + } + TipSurface::Working + } + Phase::Complete(tail) if !self.chat_widget.is_user_turn_pending_or_running() => { + if !self + .transcript_cells + .last() + .is_some_and(|last| tail.ptr_eq(&Arc::downgrade(last))) + { + current.phase = Phase::Finished; + return None; + } + TipSurface::Completion + } + _ => return None, + }; + let content_width = usize::from(width.checked_sub(/*rhs*/ 4)?); + let render = |template| { + let text = crate::tooltips::render_tooltip(template, Some(&self.keymap))?; + let lines = crate::tooltips::render_tooltip_lines( + &text, + content_width, + self.config.cwd.as_path(), + ); + match lines.as_slice() { + [line] if line.width() <= content_width => Some(line.clone()), + _ => None, + } + }; + if current.template.is_none() { + let mut templates = crate::tooltips::tooltip_templates().collect::>(); + templates.shuffle(&mut rand::rng()); + current.template = templates + .iter() + .copied() + .filter(|template| Some(*template) != self.turn_tips.previous) + .find(|template| render(template).is_some()) + .or_else(|| { + templates + .into_iter() + .find(|template| render(template).is_some()) + }); + } + let line = render(current.template?)?; + let line = prefix_hyperlink_lines(vec![line], " └ ".dim(), " ".dim()) + .pop()? + .style(crate::style::secondary_text_style()); + Some(( + surface, + TurnTip { + line, + rendered: Default::default(), + }, + )) + } +} + +#[cfg(test)] +#[path = "turn_tips_tests.rs"] +mod tests; diff --git a/codex-rs/tui/src/app/turn_tips_tests.rs b/codex-rs/tui/src/app/turn_tips_tests.rs new file mode 100644 index 0000000000..5e17648c15 --- /dev/null +++ b/codex-rs/tui/src/app/turn_tips_tests.rs @@ -0,0 +1,347 @@ +//! Foreground lifecycle, actual exposure, and dedicated-row geometry. + +use super::*; +use crate::app::owned_transcript::tests::attach_thread; +use crate::app::owned_transcript::tests::buffer_text; +use crate::app::owned_transcript::tests::user_cell; +use pretty_assertions::assert_eq; + +fn notification(method: &str, thread: ThreadId, turn: usize, status: &str) -> ServerNotification { + serde_json::from_value(serde_json::json!({ + "method": method, + "params": { "threadId": thread.to_string(), "turn": { + "id": turn.to_string(), "items": [], "itemsView": "full", "status": status, + "error": null, "startedAt": null, "completedAt": null, "durationMs": null, + }}, + })) + .unwrap() +} + +fn answer(thread: ThreadId, turn: usize) -> ServerNotification { + serde_json::from_value(serde_json::json!({ + "method": "item/completed", "params": { + "threadId": thread.to_string(), "turnId": turn.to_string(), "completedAtMs": 0, + "item": {"type": "agentMessage", "id": "answer", "text": "Done.", + "phase": "final_answer", "memoryCitation": null, "delivery": null, "questions": null}, + }, + })).unwrap() +} + +fn deliver(app: &mut App, notification: ServerNotification) { + app.handle_thread_event_now(ThreadBufferedEvent::Notification(Box::new(notification))); +} + +#[test] +fn completion_cadence_counts_exposure_and_ignores_duplicate_or_failed_turns() { + let thread = ThreadId::new(); + let now = Instant::now(); + let mut tips = TurnTips::default(); + let mut candidates = Vec::new(); + for turn in 1..=12 { + let start = notification("turn/started", thread, turn, "inProgress"); + tips.observe(&start, now); + tips.observe(&start, now); + assert_eq!(tips.starts, turn); + if turn == 3 { + tips.acknowledge(TipSurface::Working); + } + tips.observe(&answer(thread, turn), now); + let status = match turn { + 4 => "failed", + 5 => "interrupted", + _ => "completed", + }; + let completed = notification("turn/completed", thread, turn, status); + if tips.observe(&completed, now).is_some() { + candidates.push(turn); + // A hidden candidate spends nothing; two painted frames spend only once. + if turn != 6 { + tips.acknowledge(TipSurface::Completion); + tips.acknowledge(TipSurface::Completion); + } + } + assert!(tips.observe(&completed, now).is_none()); + } + assert_eq!( + (candidates, tips.completions_shown, tips.next_completion), + (vec![6, 7, 10], 2, 13) + ); + tips.dismiss(); + assert!( + tips.observe( + ¬ification("turn/completed", thread, /*turn*/ 12, "completed"), + now + ) + .is_none() + ); + assert_eq!(tips.starts, 12); +} + +#[tokio::test] +async fn working_deadline_rearms_and_hidden_rows_do_not_spend_exposure() { + let (mut app, _events, _ops) = crate::app::tests::make_test_app_with_channels().await; + let thread = ThreadId::new(); + attach_thread(&mut app, thread); + app.local_settings.tui.animations = false; + app.local_settings.tui.show_tooltips = true; + let now = Instant::now(); + deliver( + &mut app, + notification("turn/started", thread, /*turn*/ 1, "inProgress"), + ); + app.turn_tips.current.as_mut().unwrap().started_at = now; + let (frames, mut requests) = tui::FrameRequester::test_channel(); + for elapsed in [0, 10, 29] { + assert!( + app.turn_tip( + /*width*/ 100, + now + Duration::from_secs(elapsed), + &frames + ) + .is_none() + ); + let remaining = requests + .try_recv() + .unwrap() + .saturating_duration_since(Instant::now()); + assert!(remaining <= WORKING_DELAY - Duration::from_secs(elapsed)); + assert!(remaining > WORKING_DELAY - Duration::from_secs(elapsed + 1)); + } + let current = app.turn_tips.current.as_mut().unwrap(); + // Use a catalog-independent key-free fixture for width and visibility. + current.template = Some("Try /help."); + assert!( + app.turn_tip(/*width*/ 8, now + WORKING_DELAY, &frames) + .is_none() + ); + assert!(!app.turn_tips.current.as_ref().unwrap().shown); + app.chat_widget.apply_external_edit("draft".into()); + assert!( + app.turn_tip(/*width*/ 100, now + WORKING_DELAY, &frames) + .is_none() + ); + app.chat_widget.apply_external_edit(String::new()); + app.local_settings.tui.show_tooltips = false; + assert!( + app.turn_tip(/*width*/ 100, now + WORKING_DELAY, &frames) + .is_none() + ); + app.local_settings.tui.show_tooltips = true; + assert!( + app.turn_tip(/*width*/ 100, now + WORKING_DELAY, &frames) + .is_some() + ); + assert!(requests.try_recv().is_err()); + + app.handle_thread_event_replay(ThreadBufferedEvent::Notification(Box::new(notification( + "turn/started", + thread, + /*turn*/ 1, + "inProgress", + )))); + assert!(app.turn_tips.current.is_none()); + assert_eq!( + (app.turn_tips.starts, app.turn_tips.completions_shown), + (1, 0) + ); + app.turn_tips.starts = 2; + deliver( + &mut app, + notification("turn/started", thread, /*turn*/ 3, "inProgress"), + ); + app.chat_widget + .apply_external_edit("queued follow-up".into()); + app.chat_widget + .handle_key_event(KeyEvent::new(KeyCode::Tab, KeyModifiers::NONE)); + deliver(&mut app, answer(thread, /*turn*/ 3)); + deliver( + &mut app, + notification("turn/completed", thread, /*turn*/ 3, "completed"), + ); + app.transcript_cells + .push(Arc::new(history_cell::PlainHistoryCell::new(vec![ + "Done.".into(), + ]))); + app.turn_tips + .ready(thread, "3", app.transcript_cells.last()); + assert!(app.chat_widget.is_user_turn_pending_or_running()); + assert!( + app.turn_tip(/*width*/ 100, now + WORKING_DELAY, &frames) + .is_none() + ); + app.reset_transcript_state_after_clear(); + assert!(app.turn_tips.current.is_none()); + assert_eq!( + (app.turn_tips.starts, app.turn_tips.completions_shown), + (3, 0) + ); +} + +#[tokio::test] +async fn turn_tip_placements_and_completion_barrier() -> Result<()> { + let mut screens = Vec::new(); + for (working, rows, width, height) in [ + (true, 1, 80, 12), + (true, 1, 80, 6), + (false, 1, 80, 12), + (false, 30, 80, 12), + (false, 1, 80, 9), + (false, 1, 80, 10), + (false, 1, 80, 11), + (false, 1, 12, 12), + (false, 1, 80, 4), + ] { + let (mut app, mut events, _ops) = crate::app::tests::make_test_app_with_channels().await; + let thread = ThreadId::new(); + attach_thread(&mut app, thread); + app.local_settings.tui.show_tooltips = true; + app.local_settings.tui.animations = false; + app.turn_tips.starts = 2; + let mut tui = crate::tui::test_support::make_test_tui()?; + tui.set_owned_screen(/*owned*/ true)?; + let size = Size::new(width, height); + tui.terminal.resize(size)?; + while events.try_recv().is_ok() {} + app.transcript_cells = vec![ + user_cell("Show a response."), + Arc::new(history_cell::PlainHistoryCell::new( + (0..rows) + .map(|row| format!("Response row {row}").into()) + .collect(), + )), + ]; + deliver( + &mut app, + notification("turn/started", thread, /*turn*/ 3, "inProgress"), + ); + app.turn_tips.current.as_mut().unwrap().started_at = Instant::now() - WORKING_DELAY; + app.turn_tips.current.as_mut().unwrap().template = Some("Try /help."); + if !working { + deliver(&mut app, answer(thread, /*turn*/ 3)); + deliver( + &mut app, + notification("turn/completed", thread, /*turn*/ 3, "completed"), + ); + app.render_owned_transcript(&mut tui, size)?; + assert!(!app.turn_tips.current.as_ref().unwrap().shown); + } + let mut saw_barrier = false; + while let Ok(event) = events.try_recv() { + match event { + AppEvent::InsertHistoryCell(cell) => { + assert!(!saw_barrier); + app.insert_history_cell(&mut tui, cell); + } + AppEvent::TurnTipReady { thread_id, turn_id } => { + app.turn_tips + .ready(thread_id, &turn_id, app.transcript_cells.last()); + saw_barrier = true; + } + _ => {} + } + } + assert_eq!(saw_barrier, !working); + app.render_owned_transcript(&mut tui, size)?; + let screen = crate::chatwidget::tests::helpers::normalize_completion_timestamps( + app.transcript_cells.last().unwrap().as_ref(), + buffer_text(crate::custom_terminal::test_support::last_rendered_buffer( + &tui.terminal, + )), + ); + let shown = screen.contains("└ Tip:"); + if shown && !working { + assert!(screen.contains("• Done."), "{screen}"); + } + assert_eq!(app.turn_tips.current.as_ref().unwrap().shown, shown); + assert_eq!( + app.turn_tips.completions_shown, + usize::from(!working && shown) + ); + screens.push(format!( + "working={working}, response rows={rows}, {width}x{height}\n{screen}" + )); + if shown { + let narrow = Size::new(/*width*/ 8, height); + tui.terminal.resize(narrow)?; + app.render_owned_transcript(&mut tui, narrow)?; + assert!( + !buffer_text(crate::custom_terminal::test_support::last_rendered_buffer( + &tui.terminal + )) + .contains("Tip:") + ); + tui.terminal.resize(size)?; + app.render_owned_transcript(&mut tui, size)?; + assert_eq!(app.turn_tips.completions_shown, usize::from(!working)); + } + if !working && shown { + assert!( + !app.transcript_cells + .iter() + .flat_map(|cell| cell.raw_lines()) + .any(|line| line.to_string().contains("Try /help")) + ); + let response_row = screen + .lines() + .position(|line| line.contains("• Done.")) + .unwrap() as u16; + for (kind, column, row) in [ + ( + crossterm::event::MouseEventKind::Down(crossterm::event::MouseButton::Left), + 0, + response_row, + ), + ( + crossterm::event::MouseEventKind::Drag(crossterm::event::MouseButton::Left), + width - 1, + height - 1, + ), + ] { + app.transcript_view.handle_mouse( + crossterm::event::MouseEvent { + kind, + column, + row, + modifiers: KeyModifiers::NONE, + }, + &app.transcript_cells, + ); + } + let selected = app + .transcript_view + .selected_text(&app.transcript_cells) + .unwrap(); + assert!(selected.contains("Done.")); + assert!( + selected.contains(&app.transcript_cells.last().unwrap().raw_lines()[0].to_string()) + ); + assert!(!selected.contains("Try /help")); + app.transcript_view.end_selection(&app.transcript_cells); + app.transcript_view.begin_search(); + app.transcript_view.paste_search("Try /help"); + while app.transcript_view.advance_search(&app.transcript_cells) {} + app.render_owned_transcript(&mut tui, size)?; + assert!( + buffer_text(crate::custom_terminal::test_support::last_rendered_buffer( + &tui.terminal + )) + .contains("No matches") + ); + app.transcript_view.cancel_search(); + app.transcript_cells + .push(Arc::new(history_cell::PlainHistoryCell::new(vec![ + "Unrelated output".into(), + ]))); + assert!( + app.turn_tip(width, Instant::now(), &tui.frame_requester()) + .is_none() + ); + } + tui.set_owned_screen(/*owned*/ false)?; + } + insta::assert_snapshot!( + "turn_tip_placements", + crate::chatwidget::tests::helpers::normalize_snapshot_paths(screens.join("\n\n")) + ); + Ok(()) +} diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index e810df4d09..9d6156d664 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -1106,6 +1106,11 @@ pub(crate) enum AppEvent { FollowTranscript, InsertHistoryCell(Box), + /// FIFO barrier after the completed turn's history insertions. + TurnTipReady { + thread_id: ThreadId, + turn_id: String, + }, /// Move visible completed voice captions into history in one app event. CommitRealtimeTranscriptHistory, diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/footer_state.rs b/codex-rs/tui/src/bottom_pane/chat_composer/footer_state.rs index c734ccad7d..9cdae65217 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer/footer_state.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer/footer_state.rs @@ -65,6 +65,7 @@ pub(crate) enum CommandPopupPlacement { pub(crate) struct ComposerRenderOptions<'a> { /// Reserve a shared hint row independently of whether it currently contains a notice. pub(crate) composer_gap: Option<&'a crate::bottom_pane::ComposerGap>, + pub(crate) working_tip: Option<&'a crate::turn_tip::TurnTip>, pub(crate) warning_count: usize, pub(crate) textarea_right_reserve: u16, /// Keep configured status below the composer while hints occupy the final row. diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 87ab81d313..fcd3077742 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -2248,6 +2248,12 @@ impl BottomPane { || self.hook_status_message.is_some() || !self.unified_exec_footer.is_empty(); let has_inline_previews = has_pending_thread_approvals || has_pending_input; + if !has_inline_previews + && self.status_widget().is_some() + && let Some(tip) = options.working_tip + { + flex.push(/*flex*/ 1, RenderableItem::Borrowed(tip)); + } if has_inline_previews && has_status_or_footer { flex.push(/*flex*/ 0, RenderableItem::Owned("".into())); } diff --git a/codex-rs/tui/src/chatwidget/rendering.rs b/codex-rs/tui/src/chatwidget/rendering.rs index 345146fadb..36c873351f 100644 --- a/codex-rs/tui/src/chatwidget/rendering.rs +++ b/codex-rs/tui/src/chatwidget/rendering.rs @@ -136,6 +136,7 @@ impl ChatWidget { /*footer*/ None, crate::bottom_pane::CommandPopupPlacement::AboveComposer, /*composer_gap*/ None, + /*working_tip*/ None, ); } @@ -201,6 +202,7 @@ impl ChatWidget { /*footer*/ None, crate::bottom_pane::CommandPopupPlacement::AboveComposer, /*composer_gap*/ None, + /*working_tip*/ None, ) .inset(Insets::tlbr( /*top*/ 1, /*left*/ 0, /*bottom*/ 0, /*right*/ 0, @@ -218,6 +220,7 @@ impl ChatWidget { footer: Option<&'a crate::bottom_pane::TranscriptFooter>, command_popup_placement: crate::bottom_pane::CommandPopupPlacement, composer_gap: Option<&'a crate::bottom_pane::ComposerGap>, + working_tip: Option<&'a crate::turn_tip::TurnTip>, ) -> RenderableItem<'a> { if self.fork_in_progress { RenderableItem::Owned(Box::new( @@ -244,6 +247,7 @@ impl ChatWidget { self.bottom_pane .as_renderable_with_options(crate::bottom_pane::ComposerRenderOptions { composer_gap, + working_tip, warning_count: self.warning_display_state.count, textarea_right_reserve: right_reserve, separate_status_line: command_popup_placement diff --git a/codex-rs/tui/src/chatwidget/rendering_tests.rs b/codex-rs/tui/src/chatwidget/rendering_tests.rs index aef12d6f6c..dc483a41b8 100644 --- a/codex-rs/tui/src/chatwidget/rendering_tests.rs +++ b/codex-rs/tui/src/chatwidget/rendering_tests.rs @@ -87,6 +87,7 @@ async fn owned_bottom_pane_preserves_draft_cursor_and_read_only_notice() { /*footer*/ None, crate::bottom_pane::CommandPopupPlacement::Overlay, /*composer_gap*/ None, + /*working_tip*/ None, ); let mut buffer = Buffer::empty(area); bottom.render(area, &mut buffer); diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index a32dee4e0e..7b6a6bfa75 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -230,6 +230,7 @@ mod transcript_mode; mod transcript_reflow; mod transcript_view; mod tui; +mod turn_tip; mod ui_consts; mod unarchive_prompt; pub(crate) mod update_action; diff --git a/codex-rs/tui/src/tooltips.rs b/codex-rs/tui/src/tooltips.rs index 7dff716092..a8cceac303 100644 --- a/codex-rs/tui/src/tooltips.rs +++ b/codex-rs/tui/src/tooltips.rs @@ -205,9 +205,13 @@ pub(crate) fn resolved_tooltips( .filter_map(move |tip| render_tooltip(tip, keymap)) } +pub(crate) fn tooltip_templates() -> impl Iterator { + ALL_TOOLTIPS.iter().copied() +} + /// Substitute `{key:context.action}` with the current primary shortcut in a Markdown code span. /// Skip the tip if a placeholder is invalid or its action has no binding. -fn render_tooltip(mut template: &str, keymap: Option<&RuntimeKeymap>) -> Option { +pub(crate) fn render_tooltip(mut template: &str, keymap: Option<&RuntimeKeymap>) -> Option { let mut rendered = String::new(); while let Some((prefix, rest)) = template.split_once("{key:") { let (action, suffix) = rest.split_once('}')?; diff --git a/codex-rs/tui/src/transcript_view.rs b/codex-rs/tui/src/transcript_view.rs index df77eb16c1..d6a25096e6 100644 --- a/codex-rs/tui/src/transcript_view.rs +++ b/codex-rs/tui/src/transcript_view.rs @@ -19,6 +19,7 @@ mod search; mod selection; mod snapshot; mod text; +mod turn_tip; use std::sync::Arc; @@ -89,6 +90,7 @@ pub(crate) struct TranscriptView { follow_control: follow_control::FollowControl, copy_feedback: Option, composer_tip: Option<(Rect, HyperlinkLine)>, + turn_tip_key: Option, cache: LayoutCache, live: Option>, live_separated: Option>, @@ -120,6 +122,7 @@ impl Default for TranscriptView { follow_control: follow_control::FollowControl::default(), copy_feedback: None, composer_tip: None, + turn_tip_key: None, cache: LayoutCache::default(), live: None, live_separated: None, diff --git a/codex-rs/tui/src/transcript_view/layout.rs b/codex-rs/tui/src/transcript_view/layout.rs index fe66fb9b91..d9640bf586 100644 --- a/codex-rs/tui/src/transcript_view/layout.rs +++ b/codex-rs/tui/src/transcript_view/layout.rs @@ -70,6 +70,7 @@ struct CachedLayout { #[derive(Clone, Copy, PartialEq, Eq)] pub(super) struct CellPresentation { separated: bool, + turn_tip_space: bool, expanded: bool, disclosure: bool, } @@ -136,6 +137,7 @@ impl TranscriptView { let separated = index > 0 && !cell.is_stream_continuation(); let presentation = CellPresentation { separated, + turn_tip_space: self.turn_tip_key == Some(EntryKey::cell(cell)), expanded, disclosure, }; @@ -203,10 +205,15 @@ impl LayoutCache { return layout; } let layout = render(); - let layout = Arc::new(if presentation.separated { + let layout = if presentation.separated { layout.with_leading_separator() } else { layout + }; + let layout = Arc::new(if presentation.turn_tip_space { + layout.with_leading_spacer() + } else { + layout }); self.entries.retain(|entry| !entry.source.ptr_eq(&source)); self.entries.push(CachedLayout { diff --git a/codex-rs/tui/src/transcript_view/layout_tests.rs b/codex-rs/tui/src/transcript_view/layout_tests.rs index 8cdc12352b..f0b3702043 100644 --- a/codex-rs/tui/src/transcript_view/layout_tests.rs +++ b/codex-rs/tui/src/transcript_view/layout_tests.rs @@ -47,6 +47,7 @@ fn layouts_refresh_for_width_animation_and_mutable_frames() { width, CellPresentation { separated: false, + turn_tip_space: false, expanded: false, disclosure: false, }, @@ -101,6 +102,7 @@ fn recent_entries_are_reused_and_old_entries_are_evicted() { /*width*/ 20, CellPresentation { separated: false, + turn_tip_space: false, expanded: false, disclosure: false, }, @@ -119,6 +121,7 @@ fn recent_entries_are_reused_and_old_entries_are_evicted() { /*width*/ 20, CellPresentation { separated: false, + turn_tip_space: false, expanded: false, disclosure: false, }, diff --git a/codex-rs/tui/src/transcript_view/text.rs b/codex-rs/tui/src/transcript_view/text.rs index 3504047986..5779498cc6 100644 --- a/codex-rs/tui/src/transcript_view/text.rs +++ b/codex-rs/tui/src/transcript_view/text.rs @@ -142,7 +142,16 @@ impl TextLayout { /// Add visual spacing between entries without changing any source position. pub(super) fn with_leading_separator(mut self) -> Self { - if !self.separated && !self.rows.is_empty() { + if !self.separated { + self = self.with_leading_spacer(); + self.separated = !self.rows.is_empty(); + } + self + } + + /// Reserve one presentation-only row before existing spacing and source text. + pub(super) fn with_leading_spacer(mut self) -> Self { + if !self.rows.is_empty() { self.rows.insert( /*index*/ 0, TextRow { @@ -158,7 +167,6 @@ impl TextLayout { if let Some(control) = &mut self.disclosure_control { control.row += 1; } - self.separated = true; } self } diff --git a/codex-rs/tui/src/transcript_view/turn_tip.rs b/codex-rs/tui/src/transcript_view/turn_tip.rs new file mode 100644 index 0000000000..854bef6adf --- /dev/null +++ b/codex-rs/tui/src/transcript_view/turn_tip.rs @@ -0,0 +1,59 @@ +//! Reserve a temporary tip before completion metadata, outside selectable source text. +//! Return its final draw area only when at least one response row remains visible. + +use super::*; +use crate::turn_tip::TurnTip; + +impl TranscriptView { + pub(crate) fn render_with_turn_tip_space( + &mut self, + area: Rect, + buf: &mut Buffer, + cells: &[Arc], + tip: Option<&TurnTip>, + ) -> Option { + let tip = tip.filter(|_| area.height > 1); + self.turn_tip_key = tip.and_then(|_| { + cells + .last() + .filter(|cell| { + cell.as_any() + .is::() + && !cell.display_lines(area.width).is_empty() + }) + .map(EntryKey::cell) + }); + self.render( + Rect { + height: area.height - u16::from(tip.is_some() && self.turn_tip_key.is_none()), + ..area + }, + buf, + cells, + ); + // The final viewport includes prompt-header and footer reservations. Let the tip yield + // rather than leave only completion metadata visible on a short terminal. + if let Some(key) = self.turn_tip_key + && self.visible.first().is_none_or(|row| row.key == key) + { + self.turn_tip_key = None; + self.render(area, buf, cells); + return None; + } + tip?; + let row = match self.turn_tip_key { + Some(key) => self + .visible + .iter() + .position(|row| row.key == key && row.row == 0)?, + None => self.visible.len(), + }; + let y = self.area.y.saturating_add(row as u16); + (self.tail_visible && !self.visible.is_empty() && y < area.bottom()).then_some(Rect::new( + self.area.x, + y, + self.area.width, + /*height*/ 1, + )) + } +} diff --git a/codex-rs/tui/src/turn_tip.rs b/codex-rs/tui/src/turn_tip.rs new file mode 100644 index 0000000000..2005a9a859 --- /dev/null +++ b/codex-rs/tui/src/turn_tip.rs @@ -0,0 +1,32 @@ +//! One expendable tip row shared by working status and completed-response presentation. +//! Drawing records exposure; measuring or clipping the row does not. + +use std::cell::Cell; + +use ratatui::buffer::Buffer; +use ratatui::layout::Rect; +use ratatui::style::Style; +use ratatui::widgets::Widget; + +use crate::render::renderable::Renderable; +use crate::terminal_hyperlinks::HyperlinkLine; +use crate::terminal_hyperlinks::HyperlinkParagraph; + +pub(crate) struct TurnTip { + pub(crate) line: HyperlinkLine, + pub(crate) rendered: Cell, +} + +impl Renderable for TurnTip { + fn render(&self, area: Rect, buf: &mut Buffer) { + if !area.is_empty() && self.line.width() <= usize::from(area.width) { + HyperlinkParagraph::new(std::slice::from_ref(&self.line), Style::default()) + .render(Rect { height: 1, ..area }, buf); + self.rendered.set(/*val*/ true); + } + } + + fn desired_height(&self, width: u16) -> u16 { + u16::from(self.line.width() <= usize::from(width)) + } +}