From 07f18d5ff74bb361b46e1eacab0bddc28da26752 Mon Sep 17 00:00:00 2001 From: Ian MacLeod Date: Sat, 5 Sep 2026 00:23:47 +0000 Subject: [PATCH] Support selectable answers for asynchronous TUI questions (#42894) ## Why Asynchronous questions can include suggested answers, but the TUI previously displayed only a freeform input. Show those choices and require them to be fully visible before submission. ## What changed - Render numbered, wrapped choices with a default selection, list navigation, and digit shortcuts. Submit the selected label as the answer. - Block submission of clipped choices and prompt the user to expand the terminal. Limit option processing to the first 32 entries and discard labels over 512 bytes. - Preserve freeform input when no usable choices remain, and add contextual submit, skip, and question navigation hints. - Ignore repeated submit keys so holding a key cannot answer another question. ## Testing Add state, snapshot, and chat widget tests covering wrapped and clipped choices, constrained layouts, option limits, long answer preservation, oversized answer rejection, key repeats, and conflicting keybindings. GitOrigin-RevId: 5d3ebadf61da62a9680ba819e6dfcf8302c4e9da --- .../src/bottom_pane/async_questions/input.rs | 64 ++++- .../src/bottom_pane/async_questions/layout.rs | 114 ++++++++ .../src/bottom_pane/async_questions/mod.rs | 81 ++++++ .../src/bottom_pane/async_questions/render.rs | 261 ++++++++++++++---- ...ions__tests__long_prompt_active_input.snap | 2 +- ...sts__question_clipped_choice_rejected.snap | 11 + ...tions__tests__question_clipped_prompt.snap | 6 + ..._tests__question_wrapped_named_option.snap | 13 + ...sts__question_wrapped_selected_option.snap | 13 + .../src/bottom_pane/async_questions/state.rs | 53 +++- .../async_questions/state_tests.rs | 93 +++++++ .../bottom_pane/chat_composer/inline_input.rs | 8 + .../src/chatwidget/tests/questions_tests.rs | 55 ++++ ...s__questions_tests__freeform_question.snap | 2 + ...ests__questions_tests__named_question.snap | 14 + ...__questions_tests__oversized_question.snap | 9 + ...ests__single_question_working_spacing.snap | 4 +- 17 files changed, 729 insertions(+), 74 deletions(-) create mode 100644 codex-rs/tui/src/bottom_pane/async_questions/layout.rs create mode 100644 codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__question_clipped_choice_rejected.snap create mode 100644 codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__question_clipped_prompt.snap create mode 100644 codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__question_wrapped_named_option.snap create mode 100644 codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__question_wrapped_selected_option.snap create mode 100644 codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__named_question.snap create mode 100644 codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__oversized_question.snap diff --git a/codex-rs/tui/src/bottom_pane/async_questions/input.rs b/codex-rs/tui/src/bottom_pane/async_questions/input.rs index 493bb15b2c..79175a7a74 100644 --- a/codex-rs/tui/src/bottom_pane/async_questions/input.rs +++ b/codex-rs/tui/src/bottom_pane/async_questions/input.rs @@ -4,7 +4,7 @@ use super::*; impl AsyncQuestions { pub(crate) fn handles_key_as_editing(&self, key: KeyEvent) -> bool { - self.composer.handles_key_as_editing(key) + self.focus_is_notes() && self.composer.handles_key_as_editing(key) } pub(super) fn edit(&mut self, key: KeyEvent) { @@ -31,6 +31,12 @@ impl AsyncQuestions { } } + pub(super) fn select_option(&mut self, index: usize) { + self.state.pending[self.state.current_idx] + .options_state + .selected_idx = Some(index); + } + pub(crate) fn set_keymap(&mut self, keymap: &RuntimeKeymap) { self.keymap = keymap.clone(); self.composer.set_keymap_bindings(keymap); @@ -43,7 +49,11 @@ impl AsyncQuestions { impl BottomPaneView for AsyncQuestions { fn keymap_contexts(&self) -> crate::keymap::KeymapContextSet { - self.composer.keymap_contexts().with(KeymapContext::Chat) + if self.has_options() { + crate::keymap::KeymapContextSet::new(KeymapContext::List).with(KeymapContext::Chat) + } else { + self.composer.keymap_contexts().with(KeymapContext::Chat) + } } fn handle_key_event(&mut self, key: KeyEvent) { @@ -54,9 +64,6 @@ impl BottomPaneView for AsyncQuestions { self.edit(key); return; } - if key.kind != KeyEventKind::Press && self.keymap.composer.submit.is_pressed(key) { - return; - } if self.keymap.chat.interrupt_turn.is_pressed(key) { self.app_event_tx.interrupt(); return; @@ -69,11 +76,48 @@ impl BottomPaneView for AsyncQuestions { } if self.keymap.chat.edit_queued_message.is_pressed(key) { self.navigate(/*forward*/ true); - } else if self.keymap.chat.prompt_stack_back.is_pressed(key) { - self.navigate(/*forward*/ false); - } else { - self.edit(key); + return; } + if self.keymap.chat.prompt_stack_back.is_pressed(key) { + self.navigate(/*forward*/ false); + return; + } + if !self.has_options() { + if key.kind == KeyEventKind::Press || !self.keymap.composer.submit.is_pressed(key) { + self.edit(key); + } + return; + } + let count = self.options_len(); + let visible = self.visible_options.get().1.max(1); + let mut state = self.state.pending[self.state.current_idx].options_state; + match self.keymap.list.action_for(key) { + Some(ListAction::MoveUp) => state.move_up_wrap(count), + Some(ListAction::MoveDown) => state.move_down_wrap(count), + Some(ListAction::PageUp) => state.page_up_clamped(count, visible), + Some(ListAction::PageDown) => state.page_down_clamped(count, visible), + Some(ListAction::JumpTop) => state.jump_top(count, visible), + Some(ListAction::JumpBottom) => state.jump_bottom(count, visible), + Some(ListAction::Accept) => { + if key.kind == KeyEventKind::Press { + self.go_next_or_submit(); + } + } + Some(ListAction::Cancel) => self.set_expanded(/*expanded*/ false), + Some(ListAction::MoveLeft | ListAction::MoveRight) => {} + None => { + if key.kind == KeyEventKind::Press + && !crate::key_hint::has_ctrl_or_alt(key.modifiers) + && let KeyCode::Char(ch) = key.code + && let Some(index) = self.option_index_for_digit(ch) + { + self.select_option(index); + self.go_next_or_submit(); + return; + } + } + } + self.state.pending[self.state.current_idx].options_state = state; } fn is_complete(&self) -> bool { @@ -95,7 +139,7 @@ impl BottomPaneView for AsyncQuestions { return false; } self.composer.flush_pending_input(); - self.composer.handle_paste(text) + !self.has_options() && self.composer.handle_paste(text) } fn flush_paste_burst_if_due(&mut self) -> bool { self.composer.flush_paste_burst_if_due() diff --git a/codex-rs/tui/src/bottom_pane/async_questions/layout.rs b/codex-rs/tui/src/bottom_pane/async_questions/layout.rs new file mode 100644 index 0000000000..d4317d095c --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/async_questions/layout.rs @@ -0,0 +1,114 @@ +//! Allocate question, options, input, and footer rows without intermediate layout plans. +//! Tight layouts reserve freeform editing space; named choices require the complete prompt. + +use ratatui::layout::Rect; + +use super::AsyncQuestions; +use super::DESIRED_SPACERS_BETWEEN_SECTIONS; + +pub(super) struct LayoutSections { + pub(super) progress_area: Rect, + pub(super) question_area: Rect, + pub(super) question_lines: Vec, + pub(super) options_area: Rect, + pub(super) notes_area: Rect, + pub(super) footer_lines: u16, + pub(super) spacer_after_input: u16, +} + +impl AsyncQuestions { + pub(super) fn layout_sections(&self, area: Rect) -> LayoutSections { + let has_options = self.has_options(); + let mut question_lines = self.wrapped_question_lines(area.width); + let mut footer_pref = self.footer_lines(area.width, /*option_tip*/ None).len() as u16; + if has_options + && question_lines.len() + + usize::from(self.options_required_height(area.width)) + + usize::from(footer_pref) + + usize::from(self.unanswered_count() > 1) + + usize::from(DESIRED_SPACERS_BETWEEN_SECTIONS) + > usize::from(area.height) + { + footer_pref = self.footer_lines(area.width, Some(self.option_tip())).len() as u16; + } + let progress_pref = u16::from(self.unanswered_count() > 1); + let min_notes = u16::from(!has_options).min(area.height); + let available = area.height.saturating_sub(min_notes); + let question_height = question_lines.len().min(usize::from(available)) as u16; + question_lines.truncate(usize::from(question_height)); + let mut remaining = available.saturating_sub(question_height); + let mut options_height = 0; + let mut spacer_before_question = 0; + let spacer_after_question; + let mut spacer_after_options = 0; + let mut spacer_after_input = 0; + let progress_height; + let footer_lines; + let notes_height; + + if has_options { + let full_options_height = self.options_required_height(area.width); + let min_options_height = remaining.min(1); + // Reserve hints and spacers while retaining a row for the selected option. + let reserved = footer_pref + progress_pref + DESIRED_SPACERS_BETWEEN_SECTIONS; + options_height = + full_options_height.min(remaining.saturating_sub(reserved).max(min_options_height)); + remaining -= options_height; + progress_height = progress_pref.min(remaining); + remaining -= progress_height; + + spacer_after_options = u16::from(remaining > footer_pref); + remaining -= spacer_after_options; + footer_lines = footer_pref.min(remaining); + remaining -= footer_lines; + spacer_after_question = u16::from(remaining > 0); + remaining -= spacer_after_question; + options_height += remaining.min(full_options_height.saturating_sub(options_height)); + notes_height = 0; + } else { + // Freeform answers take their preferred input height before hints and progress. + let preferred_notes = self + .composer + .inline_input_height(area.width) + .clamp(1, 8) + .saturating_sub(min_notes) + .min(remaining); + remaining -= preferred_notes; + footer_lines = footer_pref.min(remaining); + remaining -= footer_lines; + progress_height = progress_pref.min(remaining); + remaining -= progress_height; + spacer_after_input = u16::from(remaining > 0); + remaining -= spacer_after_input; + // Drop padding before input or prompt content in a constrained viewport. + spacer_before_question = u16::from(remaining >= 2 && self.unanswered_count() > 1); + spacer_after_question = u16::from(remaining >= 1); + notes_height = min_notes + preferred_notes + remaining + - spacer_before_question + - spacer_after_question; + } + + let mut y = area.y; + let mut take_rows = |height| { + let rows = Rect::new(area.x, y, area.width, height); + y = y.saturating_add(height); + rows + }; + let progress_area = take_rows(progress_height); + take_rows(spacer_before_question); + let question_area = take_rows(question_height); + take_rows(spacer_after_question); + let options_area = take_rows(options_height); + take_rows(spacer_after_options); + let notes_area = take_rows(notes_height); + LayoutSections { + progress_area, + question_area, + question_lines, + options_area, + notes_area, + footer_lines, + spacer_after_input, + } + } +} diff --git a/codex-rs/tui/src/bottom_pane/async_questions/mod.rs b/codex-rs/tui/src/bottom_pane/async_questions/mod.rs index 1905b75308..6fdbdf7d8f 100644 --- a/codex-rs/tui/src/bottom_pane/async_questions/mod.rs +++ b/codex-rs/tui/src/bottom_pane/async_questions/mod.rs @@ -8,21 +8,32 @@ use crate::bottom_pane::ChatComposerConfig; use crate::bottom_pane::InputResult; use crate::bottom_pane::bottom_pane_view::BottomPaneView; use crate::bottom_pane::chat_composer::ComposerDraft; +use crate::bottom_pane::scroll_state::ScrollState; +use crate::bottom_pane::selection_popup_common::GenericDisplayRow; +use crate::bottom_pane::selection_popup_common::measure_rows_height; use crate::key_hint::KeyBindingListExt; use crate::keymap::KeymapContext; +use crate::keymap::ListAction; use crate::keymap::RuntimeKeymap; use codex_protocol::items::AsyncUserInputQuestion; +use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyEventKind; use std::collections::HashSet; +use unicode_width::UnicodeWidthStr; mod input; +mod layout; mod render; mod state; +pub(super) const TIP_SEPARATOR: &str = " "; +pub(super) const DESIRED_SPACERS_BETWEEN_SECTIONS: u16 = 2; + #[derive(Debug, Clone, PartialEq)] struct PendingQuestion { question: AsyncUserInputQuestion, + options_state: ScrollState, draft: ComposerDraft, } @@ -45,6 +56,7 @@ pub(crate) struct AsyncQuestions { pub(crate) expanded: bool, pub(crate) delivery_enabled: bool, pub(crate) submission: Option, + visible_options: std::cell::Cell<(usize, usize)>, pub(crate) next_hint: Option, keymap: RuntimeKeymap, pub(super) composer: ChatComposer, @@ -74,6 +86,7 @@ impl AsyncQuestions { expanded: false, delivery_enabled: true, submission: None, + visible_options: std::cell::Cell::new((0, 0)), next_hint: None, keymap, composer, @@ -98,6 +111,74 @@ impl AsyncQuestions { format!("{current} of {total}") } + fn options(&self) -> &[String] { + self.current_question() + .and_then(|q| q.options.as_deref()) + .unwrap_or_default() + } + fn has_options(&self) -> bool { + !self.options().is_empty() + } + + fn options_len(&self) -> usize { + self.options().len() + } + + fn option_index_for_digit(&self, ch: char) -> Option { + let idx = ch.to_digit(10)?.checked_sub(1)? as usize; + (idx < self.options_len()).then_some(idx) + } + + fn selected_option_index(&self) -> Option { + self.current_answer() + .and_then(|answer| answer.options_state.selected_idx) + } + + pub(super) fn wrapped_question_lines(&self, width: u16) -> Vec { + self.current_question() + .map(|q| { + textwrap::wrap(&q.title, width.max(1) as usize) + .into_iter() + .map(|line| line.to_string()) + .collect::>() + }) + .unwrap_or_default() + } + + fn focus_is_notes(&self) -> bool { + !self.has_options() + } + + pub(super) fn option_rows(&self) -> Vec { + self.options() + .iter() + .enumerate() + .map(|(index, label)| { + let prefix = if self.selected_option_index() == Some(index) { + '›' + } else { + ' ' + }; + let number = index + 1; + let prefix = format!("{prefix} {number}. "); + GenericDisplayRow { + name: format!("{prefix}{label}"), + wrap_indent: Some(prefix.width()), + ..Default::default() + } + }) + .collect() + } + + pub(super) fn options_required_height(&self, width: u16) -> u16 { + if !self.has_options() { + return 0; + } + let rows = self.option_rows(); + let state = ScrollState::default(); + measure_rows_height(&rows, &state, rows.len(), width.saturating_add(1)) + } + fn save_current_draft(&mut self) { self.composer.flush_pending_input(); let draft = self.composer.snapshot_draft(); diff --git a/codex-rs/tui/src/bottom_pane/async_questions/render.rs b/codex-rs/tui/src/bottom_pane/async_questions/render.rs index 4994a440cc..eb4313d880 100644 --- a/codex-rs/tui/src/bottom_pane/async_questions/render.rs +++ b/codex-rs/tui/src/bottom_pane/async_questions/render.rs @@ -1,79 +1,228 @@ -//! Render freeform questions inline using the existing modal editing engine. -use super::AsyncQuestions; -use crate::bottom_pane::selection_popup_common::menu_surface_inset; -use crate::bottom_pane::selection_popup_common::menu_surface_padding_height; -use crate::bottom_pane::selection_popup_common::render_menu_surface; -use crate::render::renderable::Renderable; +//! Lay out question text, choices, inline editing, and key hints within the available rows. + use ratatui::buffer::Buffer; use ratatui::layout::Rect; use ratatui::style::Stylize; use ratatui::text::Line; +use ratatui::text::Span; use ratatui::widgets::Paragraph; use ratatui::widgets::Widget; -impl AsyncQuestions { - fn question_lines(&self, width: u16) -> Vec> { - self.current_question() - .map(|q| { - textwrap::wrap(&q.title, usize::from(width.max(1))) - .into_iter() - .map(Line::from) - .collect() - }) - .unwrap_or_default() - } - fn input_area(&self, area: Rect) -> Rect { - let area = menu_surface_inset(area); - let input_height = self - .composer - .inline_input_height(area.width) - .min(area.height); - Rect::new( - area.x, - area.bottom().saturating_sub(input_height), - area.width, - input_height, - ) - } -} +use crate::bottom_pane::selection_popup_common::menu_surface_inset; +use crate::bottom_pane::selection_popup_common::menu_surface_padding_height; +use crate::bottom_pane::selection_popup_common::render_menu_surface; +use crate::render::renderable::Renderable; + +use super::AsyncQuestions; +use super::TIP_SEPARATOR; +use crate::bottom_pane::request_user_input::render::render_rows_bottom_aligned; +use crate::bottom_pane::request_user_input::render::truncate_line_word_boundary_with_ellipsis; +use crate::keymap::KeymapContext; + impl Renderable for AsyncQuestions { fn cursor_style(&self, area: Rect) -> crossterm::cursor::SetCursorStyle { self.composer.cursor_style(area) } fn desired_height(&self, width: u16) -> u16 { let width = menu_surface_inset(Rect::new(/*x*/ 0, /*y*/ 0, width, u16::MAX)).width; - self.question_lines(width).len() as u16 - + self.composer.inline_input_height(width) - + 1 + let extra_height = self.options_required_height(width) + + if !self.has_options() { + self.composer.inline_input_height(width).clamp(1, 8) + } else { + 0 + } + + 2 + + u16::from(!self.has_options() && self.unanswered_count() > 1) + + self.footer_lines(width, /*option_tip*/ None).len() as u16 + u16::from(self.unanswered_count() > 1) - + u16::from(self.unanswered_count() > 1) - + menu_surface_padding_height() + + menu_surface_padding_height(); + u16::try_from(self.wrapped_question_lines(width).len() + usize::from(extra_height)) + .unwrap_or(u16::MAX) } + fn render(&self, area: Rect, buf: &mut Buffer) { - let input = self.input_area(area); - let mut prompt = render_menu_surface(area, buf); - prompt.height = input.y.saturating_sub(prompt.y); - if self.unanswered_count() > 1 && prompt.height > 0 { - Paragraph::new(self.progress_prefix_text().dim()).render( - Rect::new(prompt.x, prompt.y, prompt.width, /*height*/ 1), - buf, - ); - prompt.y += 1; - prompt.height -= 1; + self.visible_options.set((0, 0)); + ratatui::widgets::Clear.render(area, buf); + let content_area = render_menu_surface(area, buf); + if content_area.is_empty() { + return; } - if self.unanswered_count() > 1 - && prompt.height > self.question_lines(prompt.width).len() as u16 - { - prompt.y += 1; - prompt.height -= 1; - } - Paragraph::new(self.question_lines(prompt.width)) + let sections = self.layout_sections(content_area); + Paragraph::new(self.progress_prefix_text().dim()).render(sections.progress_area, buf); + Paragraph::new(sections.question_lines.join("\n")) .style(crate::style::accent_style()) .bold() - .render(prompt, buf); - self.composer.render_inline_input(input, buf); + .render(sections.question_area, buf); + + // The shared measurer reserves a scrollbar column; this renderer uses the full width. + let option_rows = self.option_rows(); + + if self.has_options() && sections.options_area.height > 0 { + let mut options_state = self.state.pending[self.state.current_idx].options_state; + let selected = options_state.selected_idx.unwrap_or(0); + let heights: Vec<_> = option_rows + .iter() + .map(|row| { + super::measure_rows_height( + std::slice::from_ref(row), + &super::ScrollState::default(), + /*max_results*/ 1, + sections.options_area.width.saturating_add(1), + ) + }) + .collect(); + let available = sections.options_area.height; + if heights.iter().sum::() <= available { + options_state.scroll_top = 0; + } + let mut first = options_state.scroll_top.min(selected); + while first < selected && heights[first..=selected].iter().sum::() > available { + first += 1; + } + let mut used = 0; + let visible = heights[first..] + .iter() + .take_while(|&&height| { + used += height; + used <= available + }) + .count(); + options_state.scroll_top = first; + self.visible_options.set((first, visible)); + render_rows_bottom_aligned( + sections.options_area, + buf, + &option_rows, + &options_state, + option_rows.len().max(1), + "No options", + ); + } + + if !self.has_options() { + self.composer.render_inline_input(sections.notes_area, buf); + } + + let footer_area = Rect::new( + content_area.x, + sections.notes_area.bottom() + sections.spacer_after_input, + content_area.width, + sections.footer_lines, + ); + let option_tip = (self.has_options() + && sections.options_area.height > 0 + && self.options_required_height(content_area.width) > sections.options_area.height) + .then(|| self.option_tip()); + let lines = self + .footer_lines(footer_area.width, option_tip) + .into_iter() + .map(|line| truncate_line_word_boundary_with_ellipsis(line, footer_area.width as usize)) + .collect::>(); + Paragraph::new(lines).render(footer_area, buf); } + fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> { - self.composer.inline_cursor_pos(self.input_area(area)) + if !self.focus_is_notes() { + return None; + } + let content_area = menu_surface_inset(area); + if content_area.width == 0 || content_area.height == 0 { + return None; + } + let sections = self.layout_sections(content_area); + let input_area = sections.notes_area; + if input_area.width == 0 || input_area.height == 0 { + return None; + } + self.composer.inline_cursor_pos(input_area) + } +} + +impl AsyncQuestions { + pub(super) fn footer_lines( + &self, + width: u16, + option_tip: Option>, + ) -> Vec> { + if !self.focus_is_notes() + && let Some(flash) = self.composer.inline_flash() + { + return vec![flash]; + } + let mut tips = Vec::new(); + let chat_hint = |action| self.keymap.primary_hint(KeymapContext::Chat, action); + let (context, action) = if self.focus_is_notes() { + (KeymapContext::Composer, "submit") + } else { + (KeymapContext::List, "accept") + }; + // Existing cross-context keymaps keep chat priority in questions. + if let Some(key) = self.keymap.primary_hint(context, action) + && (self.focus_is_notes() + || !matches!(key, crate::key_hint::ShortcutHint::Single(binding) + if self.keymap.chat.interrupt_turn.contains(&binding) + || self.keymap.chat.edit_queued_message.contains(&binding))) + { + tips.push( + Span::styled( + format!("{} submit", key.display_label()), + crate::style::accent_style(), + ) + .bold(), + ); + } + if self.focus_is_notes() + && let Some(mode) = self.composer.vim_mode_indicator_span() + { + tips.push(Span::from(mode.content.into_owned()).dim()); + } + if let Some(key) = self + .keymap + .primary_hint(KeymapContext::Chat, "skip_question") + { + tips.push(format!("{} skip", key.display_label()).dim()); + } + tips.extend(option_tip); + if let Some(key) = chat_hint("prompt_stack_back") { + let label = if self.state.current_idx > 0 { + "prev question" + } else { + "main prompt" + }; + tips.push(format!("{} {label}", key.display_label()).dim()); + } + let next = if self.state.current_idx + 1 < self.state.pending.len() { + Some("next question") + } else { + None + }; + if let Some(label) = next + && let Some(key) = self.next_hint + { + tips.push(format!("{} {label}", key.display_label()).dim()); + } + let mut lines = Vec::new(); + let mut line = Line::default(); + for tip in tips { + if !line.spans.is_empty() { + if line.width() + TIP_SEPARATOR.len() + tip.width() > usize::from(width) { + lines.push(std::mem::take(&mut line)); + } else { + line.spans.push(TIP_SEPARATOR.into()); + } + } + line.spans.push(tip); + } + lines.push(line); + lines + } + + pub(super) fn option_tip(&self) -> Span<'static> { + format!( + "option {}/{}", + self.selected_option_index().unwrap_or(0) + 1, + self.options_len() + ) + .dim() } } diff --git a/codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__long_prompt_active_input.snap b/codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__long_prompt_active_input.snap index 577ffd2732..045d5532da 100644 --- a/codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__long_prompt_active_input.snap +++ b/codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__long_prompt_active_input.snap @@ -3,7 +3,7 @@ source: tui/src/bottom_pane/async_questions/state_tests.rs expression: text --- - 1 of 2 + A lengthy prompt. A lengthy prompt. A lengthy prompt. A lengthy prompt. A lengthy prompt. A lengthy prompt. A lengthy prompt. A lengthy prompt. diff --git a/codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__question_clipped_choice_rejected.snap b/codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__question_clipped_choice_rejected.snap new file mode 100644 index 0000000000..662c13a4ca --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__question_clipped_choice_rejected.snap @@ -0,0 +1,11 @@ +--- +source: tui/src/bottom_pane/async_questions/state_tests.rs +expression: "buffer_text(&render_editor(&editor, 50, 8))" +--- + + Second + + › 2. A suggested answer that is long enough to + wrap across multiple rows + + Expand terminal to read the entire option diff --git a/codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__question_clipped_prompt.snap b/codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__question_clipped_prompt.snap new file mode 100644 index 0000000000..eb8920a91f --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__question_clipped_prompt.snap @@ -0,0 +1,6 @@ +--- +source: tui/src/bottom_pane/async_questions/state_tests.rs +expression: buffer_text(&clipped) +--- + + Long question Long question Long question diff --git a/codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__question_wrapped_named_option.snap b/codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__question_wrapped_named_option.snap new file mode 100644 index 0000000000..c32deb7429 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__question_wrapped_named_option.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/bottom_pane/async_questions/state_tests.rs +expression: buffer_text(&buffer) +--- + + Second + + › 1. A suggested answer that is + long enough to wrap across + multiple rows + + enter submit ctrl + ] skip + ⌥ + ↓ main prompt diff --git a/codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__question_wrapped_selected_option.snap b/codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__question_wrapped_selected_option.snap new file mode 100644 index 0000000000..e5efffabc0 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/async_questions/snapshots/codex_tui__bottom_pane__async_questions__tests__question_wrapped_selected_option.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/bottom_pane/async_questions/state_tests.rs +expression: buffer_text(&clipped) +--- + + Second + + › 2. A suggested answer that is + long enough to wrap across + multiple rows + + enter submit ctrl + ] skip + option 2/2 ⌥ + ↓ main prompt diff --git a/codex-rs/tui/src/bottom_pane/async_questions/state.rs b/codex-rs/tui/src/bottom_pane/async_questions/state.rs index da68d6c1f1..a25f7772c7 100644 --- a/codex-rs/tui/src/bottom_pane/async_questions/state.rs +++ b/codex-rs/tui/src/bottom_pane/async_questions/state.rs @@ -11,12 +11,31 @@ impl AsyncQuestions { return; } let was_empty = self.state.pending.is_empty(); - self.state - .pending - .extend(questions.iter().map(|question| PendingQuestion { - question: question.clone(), + self.state.pending.extend(questions.iter().map(|question| { + // Bound work before cloning or wrapping model-authored suggestions. + let question = AsyncUserInputQuestion { + title: question.title.clone(), + options: question.options.as_ref().map(|options| { + options + .iter() + .take(32) + .filter(|label| label.len() <= 512) + .cloned() + .collect() + }), + }; + let has_options = question + .options + .as_ref() + .is_some_and(|options| !options.is_empty()); + let mut options_state = ScrollState::new(); + options_state.selected_idx = has_options.then_some(0); + PendingQuestion { + question, + options_state, draft: ComposerDraft::default(), - })); + } + })); if was_empty { self.state.current_idx = 0; self.restore_current_draft(); @@ -38,6 +57,7 @@ impl AsyncQuestions { return false; }; self.save_current_draft(); + self.visible_options.set((0, 0)); self.state.current_idx = next; self.restore_current_draft(); true @@ -51,7 +71,27 @@ impl AsyncQuestions { let Some(answer) = self.current_answer() else { return; }; - let text = answer.draft.text_with_pending(); + let selected = answer + .options_state + .selected_idx + .and_then(|index| answer.question.options.as_ref()?.get(index)) + .map(String::as_str) + .unwrap_or_default(); + // Only a fully displayed model-authored option may become user authorization. + let (first, count) = self.visible_options.get(); + let index = self.selected_option_index().unwrap_or(0); + if !self.focus_is_notes() && !(first..first + count).contains(&index) { + self.composer.show_footer_flash( + "Expand terminal to read the entire option".into(), + std::time::Duration::from_secs(5), + ); + return; + } + let text = if self.focus_is_notes() { + answer.draft.text_with_pending() + } else { + selected.to_string() + }; let text = text.trim(); let framing = AnsweredQuestion::new(&answer.question.title).render(); let limit = codex_protocol::user_input::MAX_USER_INPUT_TEXT_CHARS - framing.chars().count(); @@ -70,6 +110,7 @@ impl AsyncQuestions { return; } self.composer.flush_pending_input(); + self.visible_options.set((0, 0)); self.state.pending.remove(self.state.current_idx); if self.state.current_idx >= self.state.pending.len() { self.state.current_idx = 0; diff --git a/codex-rs/tui/src/bottom_pane/async_questions/state_tests.rs b/codex-rs/tui/src/bottom_pane/async_questions/state_tests.rs index caa64d7438..3363b29ae8 100644 --- a/codex-rs/tui/src/bottom_pane/async_questions/state_tests.rs +++ b/codex-rs/tui/src/bottom_pane/async_questions/state_tests.rs @@ -16,6 +16,7 @@ fn editor() -> AsyncQuestions { /*disable_paste_burst*/ true, RuntimeKeymap::defaults(), ); + editor.next_hint = Some(crate::key_hint::alt(KeyCode::Up).into()); editor.append( "message", &[ @@ -88,6 +89,83 @@ fn buffer_text(buffer: &ratatui::buffer::Buffer) -> String { .join("\n") } +#[test] +fn question_wrapped_options_and_other_share_the_text_indent() { + let mut editor = editor(); + editor.state.pending.remove(0); + editor.state.pending[0].question.options = Some(vec![ + "A suggested answer that is long enough to wrap across multiple rows".into(), + ]); + editor.restore_current_draft(); + let buffer = render_editor(&editor, /*width*/ 36, /*height*/ 16); + insta::assert_snapshot!("question_wrapped_named_option", buffer_text(&buffer)); + let options = editor.state.pending[0].question.options.as_mut().unwrap(); + options.insert( + 0, + "Another suggested answer that wraps over several rows before the selected option".into(), + ); + editor.select_option(/*index*/ 1); + let clipped = render_editor(&editor, /*width*/ 36, /*height*/ 10); + insta::assert_snapshot!("question_wrapped_selected_option", buffer_text(&clipped)); + render_editor(&editor, /*width*/ 0, /*height*/ 0); + editor.go_next_or_submit(); + assert!(editor.submission.is_none()); + render_editor(&editor, /*width*/ 36, /*height*/ 6); + editor.go_next_or_submit(); + assert!(editor.submission.is_none()); + insta::assert_snapshot!( + "question_clipped_choice_rejected", + buffer_text(&render_editor( + &editor, /*width*/ 50, /*height*/ 8 + )) + ); + render_editor(&editor, /*width*/ 80, /*height*/ 20); + editor.go_next_or_submit(); + assert!(editor.submission.take().is_some()); + editor.append( + "many", + &[question("Bounded", Some(vec!["x".repeat(41); 1000]))], + ); + editor.navigate(/*forward*/ true); + assert_eq!(editor.options().len(), 32); + render_editor(&editor, /*width*/ 50, /*height*/ 10); + let height = editor.desired_height(/*width*/ 50); + assert_eq!(editor.visible_options.get().1, 2); + editor.handle_key_event(KeyEvent::from(KeyCode::End)); + assert_eq!(editor.desired_height(/*width*/ 50), height); + assert_eq!( + editor.current_answer().unwrap().options_state, + ScrollState { + selected_idx: Some(31), + scroll_top: 32 - editor.visible_options.get().1, + } + ); + render_editor(&editor, /*width*/ 80, /*height*/ 80); + assert_eq!(editor.visible_options.get(), (0, 32)); + editor.state.pending[editor.state.current_idx] + .question + .title = "Long question ".repeat(3 * 65_536); + assert_eq!(editor.desired_height(/*width*/ 50), u16::MAX); + let clipped = render_editor(&editor, /*width*/ 50, /*height*/ 3); + editor.go_next_or_submit(); + assert!(editor.submission.is_none()); + insta::assert_snapshot!("question_clipped_prompt", buffer_text(&clipped)); + editor.state.pending.last_mut().unwrap().question.title = "Question".into(); + for action in ["move_left", "move_right", "cancel"] { + let config = toml::from_str(&format!("[list]\n{action} = '1'")).unwrap(); + editor.set_keymap(&RuntimeKeymap::from_config(&config).unwrap()); + editor.set_expanded(/*expanded*/ true); + render_editor(&editor, /*width*/ 80, /*height*/ 80); + for modifiers in [KeyModifiers::CONTROL, KeyModifiers::ALT] { + editor.handle_key_event(KeyEvent::new(KeyCode::Char('1'), modifiers)); + assert!(editor.submission.is_none()); + } + editor.handle_key_event(KeyEvent::from(KeyCode::Char('1'))); + assert_eq!(editor.expanded, action != "cancel"); + assert!(editor.submission.is_none()); + } +} + fn question(title: &str, options: Option>) -> AsyncUserInputQuestion { AsyncUserInputQuestion { title: title.into(), @@ -101,3 +179,18 @@ fn render_editor(editor: &AsyncQuestions, width: u16, height: u16) -> Buffer { editor.render(area, &mut buffer); buffer } + +#[test] +fn existing_cross_context_keymaps_load_without_misleading_submit_hints() { + for action in ["interrupt_turn", "edit_queued_message"] { + let config = + toml::from_str(&format!("[chat]\n{action} = 'f12'\n[list]\naccept = 'f12'")).unwrap(); + let keymap = RuntimeKeymap::from_config(&config).unwrap(); + let mut editor = editor(); + editor.navigate(/*forward*/ true); + editor.set_keymap(&keymap); + insta::allow_duplicates! { insta::assert_snapshot!(editor.footer_lines(/*width*/ 100, /*option_tip*/ None)[0].to_string(), @"ctrl + ] skip ⌥ + ↓ prev question"); } + editor.handle_key_event(KeyEvent::from(KeyCode::F(12))); + assert!(editor.submission.is_none()); + } +} diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/inline_input.rs b/codex-rs/tui/src/bottom_pane/chat_composer/inline_input.rs index 621cc7513b..f8f49ca495 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer/inline_input.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer/inline_input.rs @@ -19,6 +19,14 @@ impl ChatComposer { self.restore_draft(draft); } + pub(crate) fn inline_flash(&self) -> Option> { + self.footer + .flash + .as_ref() + .filter(|_| self.footer.flash_visible()) + .map(|flash| flash.line.clone()) + } + pub(crate) fn reset_vim_mode(&mut self) { self.vim_history = VimHistory::default(); self.draft.textarea.enter_vim_insert_mode(); diff --git a/codex-rs/tui/src/chatwidget/tests/questions_tests.rs b/codex-rs/tui/src/chatwidget/tests/questions_tests.rs index ad8b58143e..76ce8b1d7b 100644 --- a/codex-rs/tui/src/chatwidget/tests/questions_tests.rs +++ b/codex-rs/tui/src/chatwidget/tests/questions_tests.rs @@ -80,6 +80,41 @@ fn question_count(chat: &ChatWidget) -> usize { .unanswered_count() } +#[tokio::test] +async fn selected_answers_preserve_long_labels_and_reject_oversized_submissions() { + for (length, snapshot) in [ + (300, "named_question"), + ( + codex_protocol::user_input::MAX_USER_INPUT_TEXT_CHARS, + "oversized_question", + ), + ] { + let (mut chat, _rx, mut ops) = make_chatwidget_manual(/*model_override*/ None).await; + chat.thread_id = Some(ThreadId::new()); + let label = format!("{} but do not deploy", "x".repeat(length)); + chat.add_async_questions( + "message", + &[question("What next?", Some(vec![label.clone()]))], + ); + chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::ALT)); + insta::assert_snapshot!(snapshot, render_bottom_popup(&chat, /*width*/ 80)); + let saved = question_count(&chat); + chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); + if length > 512 { + assert_eq!(question_count(&chat), saved); + assert!(ops.try_recv().is_err()); + chat.bottom_pane + .handle_paste("x".repeat(codex_protocol::user_input::MAX_USER_INPUT_TEXT_CHARS)); + chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); + assert_eq!(question_count(&chat), saved); + let rendered = render_bottom_popup(&chat, /*width*/ 80); + insta::assert_snapshot!(rendered.lines().find(|line| line.contains("Answer too long")).unwrap(), @" Answer too long; limit 1048562 characters"); + } else { + assert_answer(ops.try_recv().unwrap(), &format!("> What next?\n\n{label}")); + } + } +} + fn open_questions(chat: &mut ChatWidget, options: Option>) { chat.thread_id = Some(ThreadId::new()); chat.on_agent_message_item_completed( @@ -125,12 +160,30 @@ async fn question_queue_key_does_not_steer_the_running_turn() { assert!(ops.try_recv().is_err()); } +#[tokio::test] +async fn question_key_repeats_do_not_consume_another_question() { + for key in [KeyCode::Enter, KeyCode::Char('1')] { + let (mut chat, _rx, mut ops) = make_chatwidget_manual(/*model_override*/ None).await; + open_questions(&mut chat, Some(vec!["First".into()])); + chat.handle_key_event(KeyEvent::from(key)); + ops.try_recv().unwrap(); + let mut repeat = KeyEvent::from(key); + repeat.kind = KeyEventKind::Repeat; + chat.handle_key_event(repeat); + assert!(ops.try_recv().is_err()); + assert_eq!(question_count(&chat), 1); + chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); + assert!(ops.try_recv().is_err()); + } +} + #[tokio::test] async fn single_question_spacing_with_working_status() { let (mut chat, _rx, _ops) = make_chatwidget_manual(/*model_override*/ None).await; chat.bottom_pane.set_task_running(/*running*/ true); chat.add_async_questions("single", &[question("Only question?", /*options*/ None)]); chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::ALT)); + chat.bottom_pane.handle_paste("A typed answer".into()); let rendered = render_bottom_popup(&chat, /*width*/ 80); let rows: Vec<_> = rendered.lines().collect(); let question = rows @@ -139,6 +192,8 @@ async fn single_question_spacing_with_working_status() { .unwrap(); assert!(rows[question - 2].contains("Working")); assert!(rows[question - 1].is_empty()); + assert_eq!(rows[question + 2].trim(), "A typed answer"); + assert!(rows[question + 3].is_empty()); insta::assert_snapshot!("single_question_working_spacing", rendered); } diff --git a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__freeform_question.snap b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__freeform_question.snap index 0be20d8ce8..ce76526a08 100644 --- a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__freeform_question.snap +++ b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__freeform_question.snap @@ -7,3 +7,5 @@ expression: "render_bottom_popup(&chat, 80)" Which way? Type your answer + + enter submit ctrl + ] skip ⌥ + ↓ main prompt ⌥ + ↑ next question diff --git a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__named_question.snap b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__named_question.snap new file mode 100644 index 0000000000..cb4c1c64b2 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__named_question.snap @@ -0,0 +1,14 @@ +--- +source: tui/src/chatwidget/tests/questions_tests.rs +expression: "render_bottom_popup(&chat, 80)" +--- + What next? + + › 1. + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + xxxxxxxxxxxxxxxx but do not deploy + + enter submit ctrl + ] skip ⌥ + ↓ main prompt diff --git a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__oversized_question.snap b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__oversized_question.snap new file mode 100644 index 0000000000..77b0cc3278 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__oversized_question.snap @@ -0,0 +1,9 @@ +--- +source: tui/src/chatwidget/tests/questions_tests.rs +expression: "render_bottom_popup(&chat, 80)" +--- + What next? + + Type your answer + + enter submit ctrl + ] skip ⌥ + ↓ main prompt diff --git a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__single_question_working_spacing.snap b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__single_question_working_spacing.snap index fc08f77828..5681933858 100644 --- a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__single_question_working_spacing.snap +++ b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__single_question_working_spacing.snap @@ -6,4 +6,6 @@ expression: rendered Only question? - Type your answer + A typed answer + + enter submit ctrl + ] skip ⌥ + ↓ main prompt