mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
Preserve question context in recovered TUI answer drafts (#50052)
## Why When a turn ends, unsent question answers are recovered into the main composer. Recovering only the answers loses which question each answer belongs to. ## What changed Prepend each recovered answer with its question title as a Markdown blockquote. Separate questions, answers, and existing composer text with blank lines. Sanitize question titles and limit them to 512 bytes at a UTF-8 character boundary, adding an ellipsis when truncated. Keep answer text untruncated. ## Testing Add coverage for bounded titles, multibyte characters, control characters, and multiline question-and-answer submission. Update recovery assertions and snapshots for turn completion, failure, interruption, large pastes, and Vim undo. GitOrigin-RevId: 141dacfb64bcad5fcabc5be217d2340ef7de1eb5
This commit is contained in:
@@ -2,8 +2,13 @@
|
||||
//! Message IDs survive removal so replay cannot reopen an answered or skipped question.
|
||||
|
||||
use super::*;
|
||||
use crate::history_cell::sanitize_user_text;
|
||||
use codex_context_fragments::AnsweredQuestion;
|
||||
use codex_context_fragments::ContextualUserFragment;
|
||||
use codex_utils_string::take_bytes_at_char_boundary;
|
||||
|
||||
// Match the model-authored title budget used by AnsweredQuestion.
|
||||
const MAX_RECOVERED_QUESTION_TITLE_BYTES: usize = 512;
|
||||
|
||||
impl AsyncQuestions {
|
||||
pub(crate) fn append(&mut self, message_id: &str, questions: &[AsyncUserInputQuestion]) {
|
||||
@@ -206,15 +211,14 @@ impl AsyncQuestions {
|
||||
self.composer.reset_vim_mode();
|
||||
}
|
||||
|
||||
/// Recover unsent typed answers before clearing all pending questions.
|
||||
/// Recover unsent typed answers with their quoted questions before clearing pending questions.
|
||||
pub(crate) fn take_pending_drafts(&mut self) -> Vec<String> {
|
||||
self.save_current_draft();
|
||||
let drafts = self
|
||||
.state
|
||||
.pending
|
||||
.iter()
|
||||
.map(|question| question.draft.text_with_pending().trim().to_string())
|
||||
.filter(|text| !text.is_empty())
|
||||
.filter_map(PendingQuestion::recovered_draft)
|
||||
.collect();
|
||||
self.clear_pending();
|
||||
drafts
|
||||
@@ -247,3 +251,28 @@ impl AsyncQuestions {
|
||||
self.resolve_answers(&answered_ids.into_iter().collect::<Vec<_>>());
|
||||
}
|
||||
}
|
||||
|
||||
impl PendingQuestion {
|
||||
fn recovered_draft(&self) -> Option<String> {
|
||||
let answer = self.draft.text_with_pending();
|
||||
let answer = answer.trim();
|
||||
if answer.is_empty() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let title = &self.question.title;
|
||||
let prefix = take_bytes_at_char_boundary(title, MAX_RECOVERED_QUESTION_TITLE_BYTES);
|
||||
let sanitized = sanitize_user_text(prefix.into());
|
||||
let mut quoted = textwrap::indent(&sanitized, "> ");
|
||||
if quoted.ends_with('\n') {
|
||||
quoted.pop();
|
||||
}
|
||||
if quoted.is_empty() {
|
||||
quoted.push_str("> ");
|
||||
}
|
||||
if prefix.len() < title.len() {
|
||||
quoted.push('…');
|
||||
}
|
||||
Some(format!("{quoted}\n\n{answer}"))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -30,6 +30,7 @@ fn editor() -> AsyncQuestions {
|
||||
#[test]
|
||||
fn taking_pending_drafts_preserves_order_expands_pastes_and_skips_blank_drafts() {
|
||||
let mut editor = editor();
|
||||
editor.state.pending[0].question.title = "First\n\n".into();
|
||||
editor.set_expanded(/*expanded*/ true);
|
||||
let large_answer = "typed answer ".repeat(/*n*/ 200);
|
||||
editor.handle_paste(large_answer.clone());
|
||||
@@ -40,12 +41,35 @@ fn taking_pending_drafts_preserves_order_expands_pastes_and_skips_blank_drafts()
|
||||
|
||||
assert_eq!(
|
||||
editor.take_pending_drafts(),
|
||||
vec![large_answer.trim().to_string(), "other answer".to_string()]
|
||||
vec![
|
||||
format!("> First\n>\n\n{}", large_answer.trim()),
|
||||
"> Second\n\nother answer".to_string()
|
||||
]
|
||||
);
|
||||
assert_eq!((editor.unanswered_count(), editor.expanded), (0, false));
|
||||
assert!(editor.submission.is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn taking_pending_drafts_bounds_question_titles_without_truncating_answers() {
|
||||
for (character, retained_count) in [("a", 512), ("界", 170), ("\x07", 0)] {
|
||||
let mut editor = editor();
|
||||
editor.state.pending[0].question.title =
|
||||
character.repeat(codex_protocol::user_input::MAX_USER_INPUT_TEXT_CHARS + 1);
|
||||
let answer = "typed answer ".repeat(/*n*/ 200);
|
||||
editor.handle_paste(answer.clone());
|
||||
|
||||
assert_eq!(
|
||||
editor.take_pending_drafts(),
|
||||
vec![format!(
|
||||
"> {}…\n\n{}",
|
||||
character.repeat(retained_count),
|
||||
answer.trim(),
|
||||
)]
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn replay_preserves_drafts_and_does_not_reopen_handled_questions() {
|
||||
let mut original = editor();
|
||||
|
||||
@@ -96,8 +96,9 @@
|
||||
//! # Question Draft Recovery
|
||||
//!
|
||||
//! Live terminal turns recover typed, unsubmitted question answers into the main composer.
|
||||
//! The append flushes buffered input, dismisses unused sparkle eligibility, and adds a newline
|
||||
//! after existing text. The separator and recovered answer form one Vim edit; large answers
|
||||
//! Each recovered answer follows its original question as a Markdown blockquote, with blank lines
|
||||
//! separating questions, answers, and existing text. The append flushes buffered input and dismisses
|
||||
//! unused sparkle eligibility. The separator and recovered answer form one Vim edit; large answers
|
||||
//! use atomic paste placeholders backed by their original text.
|
||||
//! Recovery escapes a shell or slash command prefix before appending answers, keeping the
|
||||
//! combined draft editable and its later submission literal. Shell mode's separate `!` becomes
|
||||
|
||||
@@ -46,7 +46,7 @@ impl ChatComposer {
|
||||
}
|
||||
self.move_cursor_to_end();
|
||||
if !self.current_text().is_empty() {
|
||||
self.insert_str("\n");
|
||||
self.insert_str("\n\n");
|
||||
}
|
||||
let char_count = drafts.chars().count();
|
||||
if char_count > LARGE_PASTE_CHAR_THRESHOLD {
|
||||
|
||||
@@ -104,7 +104,7 @@ fn recovered_answers_undo_as_one_vim_edit() {
|
||||
composer.current_text_with_pending(),
|
||||
composer.draft.textarea.vim_mode_label(),
|
||||
),
|
||||
(format!("{edited}\n{answer}"), Some(mode))
|
||||
(format!("{edited}\n\n{answer}"), Some(mode))
|
||||
);
|
||||
escape(&mut composer);
|
||||
let after = composer.draft_snapshot();
|
||||
@@ -127,7 +127,7 @@ fn recovered_answers_undo_as_one_vim_edit() {
|
||||
composer.draft.textarea.vim_query().is_some(),
|
||||
composer.draft.textarea.is_vim_operator_pending(),
|
||||
),
|
||||
(format!("existing draft\n{answer}"), false, false)
|
||||
(format!("existing draft\n\n{answer}"), false, false)
|
||||
);
|
||||
let after = composer.draft_snapshot();
|
||||
keys(&mut composer, "u");
|
||||
@@ -151,7 +151,7 @@ fn recovered_answers_undo_as_one_vim_edit() {
|
||||
escape(&mut composer);
|
||||
assert_eq!(
|
||||
composer.current_text_with_pending(),
|
||||
format!("bc\n{answer}")
|
||||
format!("bc\n\n{answer}")
|
||||
);
|
||||
let after = composer.draft_snapshot();
|
||||
keys(&mut composer, "u");
|
||||
|
||||
@@ -59,7 +59,7 @@ impl BottomPane {
|
||||
return;
|
||||
}
|
||||
self.composer
|
||||
.edit_stored_draft(|composer| composer.append_recovered_drafts(&drafts.join("\n")));
|
||||
.edit_stored_draft(|composer| composer.append_recovered_drafts(&drafts.join("\n\n")));
|
||||
self.request_redraw();
|
||||
}
|
||||
|
||||
|
||||
+2
-1
@@ -1,13 +1,14 @@
|
||||
---
|
||||
source: tui/src/bottom_pane/chat_composer.rs
|
||||
assertion_line: 5289
|
||||
expression: terminal.backend()
|
||||
---
|
||||
" "
|
||||
"› existing draft "
|
||||
" "
|
||||
" answer "
|
||||
" "
|
||||
" "
|
||||
" "
|
||||
" "
|
||||
" "
|
||||
" 100% context left | Vim: Normal "
|
||||
|
||||
@@ -5,11 +5,11 @@ use crate::bottom_pane::QueuedInputAction;
|
||||
use codex_protocol::items::AsyncUserInputQuestion;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
fn questions(chat: &mut ChatWidget, message_id: &str) {
|
||||
fn questions(chat: &mut ChatWidget, message_id: &str, title: &str) {
|
||||
chat.add_async_questions(
|
||||
message_id,
|
||||
&[AsyncUserInputQuestion {
|
||||
title: "Which way?".into(),
|
||||
title: title.into(),
|
||||
options: None,
|
||||
}],
|
||||
);
|
||||
@@ -94,8 +94,8 @@ async fn question_turn_end_appends_open_drafts_in_order_once() {
|
||||
Vec::new(),
|
||||
);
|
||||
chat.bottom_pane.set_composer_cursor(/*cursor*/ 0);
|
||||
questions(&mut chat, "first");
|
||||
questions(&mut chat, "second");
|
||||
questions(&mut chat, "first", "Which style?");
|
||||
questions(&mut chat, "second", "What budget?");
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::ALT));
|
||||
chat.bottom_pane.handle_paste(" Over-ear ".into());
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::ALT));
|
||||
@@ -103,6 +103,8 @@ async fn question_turn_end_appends_open_drafts_in_order_once() {
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Char('r'), KeyModifiers::CONTROL));
|
||||
chat.handle_key_event(KeyEvent::from(KeyCode::Char('e')));
|
||||
|
||||
let recovered =
|
||||
"$tool existing draft\n\n> Which style?\n\nOver-ear\n\n> What budget?\n\nUnder 250";
|
||||
handle_turn_completed(&mut chat, "ended-turn", /*duration_ms*/ None);
|
||||
assert_eq!(
|
||||
(
|
||||
@@ -110,11 +112,7 @@ async fn question_turn_end_appends_open_drafts_in_order_once() {
|
||||
chat.bottom_pane.composer_text_elements(),
|
||||
chat.bottom_pane.question_editor().unanswered_count()
|
||||
),
|
||||
(
|
||||
"$tool existing draft\nOver-ear\nUnder 250".into(),
|
||||
vec![element],
|
||||
0
|
||||
)
|
||||
(recovered.into(), vec![element], 0)
|
||||
);
|
||||
assert!(ops.try_recv().is_err());
|
||||
insta::assert_snapshot!(
|
||||
@@ -123,19 +121,16 @@ async fn question_turn_end_appends_open_drafts_in_order_once() {
|
||||
);
|
||||
|
||||
handle_turn_completed(&mut chat, "ended-turn", /*duration_ms*/ None);
|
||||
assert_recovered_draft(&mut chat, "$tool existing draft\nOver-ear\nUnder 250");
|
||||
assert_recovered_draft(&mut chat, recovered);
|
||||
|
||||
questions(&mut chat, "large");
|
||||
questions(&mut chat, "large", "Which way?");
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::ALT));
|
||||
let large_answer = "long answer ".repeat(/*n*/ 200);
|
||||
chat.bottom_pane.handle_paste(large_answer.clone());
|
||||
handle_turn_completed(&mut chat, "ended-turn", /*duration_ms*/ None);
|
||||
assert_eq!(
|
||||
chat.bottom_pane.composer_text_with_pending(),
|
||||
format!(
|
||||
"$tool existing draft\nOver-ear\nUnder 250\n{}",
|
||||
large_answer.trim()
|
||||
)
|
||||
format!("{recovered}\n\n> Which way?\n\n{}", large_answer.trim())
|
||||
);
|
||||
assert_eq!(chat.bottom_pane.composer_pending_pastes().len(), 1);
|
||||
insta::assert_snapshot!(
|
||||
@@ -151,7 +146,7 @@ async fn question_turn_end_recovers_collapsed_drafts_on_completion_and_failure()
|
||||
chat.thread_id = Some(ThreadId::new());
|
||||
chat.bottom_pane.set_disable_paste_burst(/*disabled*/ false);
|
||||
handle_turn_started(&mut chat, "turn");
|
||||
questions(&mut chat, "question");
|
||||
questions(&mut chat, "question", "Which way?");
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::ALT));
|
||||
// Leave typing buffered, then collapse before completion.
|
||||
for ch in "answer".chars() {
|
||||
@@ -182,7 +177,7 @@ async fn question_turn_end_recovers_collapsed_drafts_on_completion_and_failure()
|
||||
.composer
|
||||
.unwrap()
|
||||
.text,
|
||||
"main\nanswer"
|
||||
"main\n\n> Which way?\n\nanswer"
|
||||
);
|
||||
}
|
||||
chat.handle_server_notification(
|
||||
@@ -205,7 +200,7 @@ async fn question_turn_end_recovers_collapsed_drafts_on_completion_and_failure()
|
||||
assert_eq!(chat.bottom_pane.composer_text(), "earlier prompt");
|
||||
assert!(!chat.bottom_pane.no_modal_or_popup_active());
|
||||
chat.handle_key_event(KeyEvent::from(KeyCode::Esc));
|
||||
assert_recovered_draft(&mut chat, "main\nanswer");
|
||||
assert_recovered_draft(&mut chat, "main\n\n> Which way?\n\nanswer");
|
||||
assert!(ops.try_recv().is_err());
|
||||
}
|
||||
}
|
||||
@@ -227,7 +222,7 @@ async fn question_turn_end_recovers_after_interruption_restores_queued_input() {
|
||||
chat.bottom_pane.set_disable_paste_burst(/*disabled*/ false);
|
||||
chat.bottom_pane
|
||||
.set_composer_text("main draft".into(), Vec::new(), Vec::new());
|
||||
questions(&mut chat, "question");
|
||||
questions(&mut chat, "question", "Which way?");
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::ALT));
|
||||
chat.bottom_pane.handle_paste("answer".into());
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::ALT));
|
||||
@@ -266,7 +261,7 @@ async fn question_turn_end_recovers_after_interruption_restores_queued_input() {
|
||||
.composer
|
||||
.unwrap()
|
||||
.text,
|
||||
"pending steer\nqueued prompt\nmain draft\nanswer"
|
||||
"pending steer\nqueued prompt\nmain draft\n\n> Which way?\n\nanswer"
|
||||
);
|
||||
// A query miss and unsuccessful Enter must retain both the search and the answers.
|
||||
chat.handle_key_event(KeyEvent::from(KeyCode::Char('z')));
|
||||
@@ -290,7 +285,7 @@ async fn question_turn_end_recovers_after_interruption_restores_queued_input() {
|
||||
} else {
|
||||
assert_recovered_draft(
|
||||
&mut chat,
|
||||
&format!("pending steer\nqueued prompt\n{main_draft}\nanswer"),
|
||||
&format!("pending steer\nqueued prompt\n{main_draft}\n\n> Which way?\n\nanswer"),
|
||||
);
|
||||
}
|
||||
assert!(!chat.has_queued_follow_up_messages());
|
||||
@@ -298,6 +293,45 @@ async fn question_turn_end_recovers_after_interruption_restores_queued_input() {
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn question_turn_end_keeps_multiline_question_and_answer_as_text() {
|
||||
let (mut chat, _rx, mut ops) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.thread_id = Some(ThreadId::new());
|
||||
handle_turn_started(&mut chat, "turn");
|
||||
questions(
|
||||
&mut chat,
|
||||
"question",
|
||||
"First line\r\n\r\n\x1b[31mLast\r line?\x1b[0m\x07",
|
||||
);
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::ALT));
|
||||
chat.bottom_pane
|
||||
.handle_paste(" First answer line\n\nLast answer line ".into());
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::ALT));
|
||||
|
||||
handle_turn_completed(&mut chat, "turn", /*duration_ms*/ None);
|
||||
let recovered = "> First line\n>\n> Last line?\n\nFirst answer line\n\nLast answer line";
|
||||
assert_eq!(
|
||||
(
|
||||
chat.bottom_pane.composer_text_with_pending(),
|
||||
chat.bottom_pane.question_editor().unanswered_count(),
|
||||
),
|
||||
(recovered.to_string(), 0),
|
||||
);
|
||||
assert!(ops.try_recv().is_err());
|
||||
|
||||
chat.handle_key_event(KeyEvent::from(KeyCode::Enter));
|
||||
let Op::UserTurn { items, .. } = ops.try_recv().unwrap() else {
|
||||
panic!("user turn")
|
||||
};
|
||||
assert_eq!(
|
||||
items,
|
||||
vec![UserInput::Text {
|
||||
text: recovered.into(),
|
||||
text_elements: Vec::new(),
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
fn assert_recovered_draft(chat: &mut ChatWidget, text: &str) {
|
||||
assert_eq!(
|
||||
(
|
||||
|
||||
+9
-2
@@ -1,11 +1,18 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests/question_turn_end_tests.rs
|
||||
assertion_line: 86
|
||||
assertion_line: 73
|
||||
expression: "normalize_snapshot_paths(render_bottom_popup(&chat, 80))"
|
||||
---
|
||||
› $tool existing draft
|
||||
|
||||
> Which style?
|
||||
|
||||
Over-ear
|
||||
|
||||
> What budget?
|
||||
|
||||
Under 250
|
||||
[Pasted Content 2399 chars]
|
||||
|
||||
[Pasted Content 2413 chars]
|
||||
|
||||
GPT-5.6-Sol default · /tmp/project
|
||||
|
||||
+8
-1
@@ -1,9 +1,16 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests/question_turn_end_tests.rs
|
||||
expression: "render_bottom_popup(&chat, 80)"
|
||||
assertion_line: 55
|
||||
expression: "normalize_snapshot_paths(render_bottom_popup(&chat, 80))"
|
||||
---
|
||||
› $tool existing draft
|
||||
|
||||
> Which style?
|
||||
|
||||
Over-ear
|
||||
|
||||
> What budget?
|
||||
|
||||
Under 250
|
||||
|
||||
GPT-5.6-Sol default · /tmp/project
|
||||
|
||||
Reference in New Issue
Block a user