mirror of
https://github.com/openai/codex.git
synced 2026-09-16 12:13:30 +00:00
Preserve voice caption order when replaying TUI history (#44749)
## Why Retained voice captions were appended after replayed history, placing earlier speech after later typed turns when switching back to a thread. ## What changed Anchor completed captions to the next live turn and restore them before that turn during replay. Handle buffered item, delta, and completion events even when the turn-start event has been evicted. Restore remaining captions at the end of replay. ## Testing Extend regression coverage to verify caption order and avoid duplicates across repeated thread switches. Add coverage for inactive-thread captions replayed before a later buffered turn without its start event, with and without a message delta. GitOrigin-RevId: 6860a649bdcdda4937afb3ab4bbd75cf4ea5b4e6
This commit is contained in:
committed by
copyberry
parent
eab107fed0
commit
ab95cd4dd9
@@ -146,6 +146,15 @@ impl App {
|
||||
thread_id: ThreadId,
|
||||
notification: &ServerNotification,
|
||||
) {
|
||||
if let ServerNotification::TurnStarted(started) = notification
|
||||
&& let Some(records) = self.pending_realtime_transcript_replay.get_mut(&thread_id)
|
||||
{
|
||||
for record in records {
|
||||
if record.complete && record.before_turn_id.is_none() {
|
||||
record.before_turn_id = Some(started.turn.id.clone());
|
||||
}
|
||||
}
|
||||
}
|
||||
let (role, text, complete) = match notification {
|
||||
ServerNotification::ThreadRealtimeTranscriptDelta(n) => {
|
||||
(n.role.as_str(), n.delta.as_str(), false)
|
||||
@@ -197,6 +206,7 @@ impl App {
|
||||
role: role.to_string(),
|
||||
text: bounded,
|
||||
complete: true,
|
||||
before_turn_id: None,
|
||||
});
|
||||
} else {
|
||||
if text.is_empty() {
|
||||
@@ -231,6 +241,7 @@ impl App {
|
||||
role: role.to_string(),
|
||||
text: bounded,
|
||||
complete: false,
|
||||
before_turn_id: None,
|
||||
});
|
||||
}
|
||||
}
|
||||
@@ -276,13 +287,12 @@ impl App {
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn restore_realtime_replay_state_after_replay(
|
||||
pub(super) fn prepare_realtime_transcript_replay(
|
||||
&mut self,
|
||||
replayed_final_items: &HashMap<(String, String), String>,
|
||||
mut replayed_voice_texts: ReplayedVoiceTextCounts,
|
||||
) {
|
||||
) -> HashMap<String, usize> {
|
||||
let Some(thread_id) = self.chat_widget.thread_id() else {
|
||||
return;
|
||||
return HashMap::new();
|
||||
};
|
||||
self.realtime_replay_order
|
||||
.retain(|saved| *saved != thread_id);
|
||||
@@ -308,8 +318,21 @@ impl App {
|
||||
.or_default() += 1;
|
||||
}
|
||||
}
|
||||
self.chat_widget.restore_realtime_transcript_cells(cells);
|
||||
self.chat_widget
|
||||
.queue_realtime_transcripts_for_replay(cells);
|
||||
}
|
||||
retained_assistant_captions
|
||||
}
|
||||
|
||||
pub(super) fn restore_realtime_replay_state_after_replay(
|
||||
&mut self,
|
||||
replayed_final_items: &HashMap<(String, String), String>,
|
||||
mut retained_assistant_captions: HashMap<String, usize>,
|
||||
) {
|
||||
let Some(thread_id) = self.chat_widget.thread_id() else {
|
||||
return;
|
||||
};
|
||||
self.chat_widget.finish_realtime_transcript_replay();
|
||||
let Some(pending) = self.pending_realtime_speech_replay.remove(&thread_id) else {
|
||||
return;
|
||||
};
|
||||
|
||||
@@ -588,6 +588,7 @@ async fn replay_reconciles_only_matching_voice_captions_one_for_one() {
|
||||
role: role.to_string(),
|
||||
text: text.to_string(),
|
||||
complete: true,
|
||||
before_turn_id: None,
|
||||
})
|
||||
.collect();
|
||||
app.pending_realtime_transcript_replay
|
||||
@@ -660,6 +661,7 @@ async fn retained_caption_consumes_only_one_matching_answer_fallback_on_reattach
|
||||
role: "assistant".into(),
|
||||
text: "Same answer".into(),
|
||||
complete: true,
|
||||
before_turn_id: None,
|
||||
}]
|
||||
.into(),
|
||||
);
|
||||
@@ -755,6 +757,7 @@ async fn buffered_voice_items_reconcile_captions_after_thread_switch() {
|
||||
role: role.to_string(),
|
||||
text: text.to_string(),
|
||||
complete: true,
|
||||
before_turn_id: None,
|
||||
})
|
||||
.collect(),
|
||||
);
|
||||
@@ -834,6 +837,7 @@ async fn unrendered_buffered_items_do_not_consume_retained_captions() {
|
||||
role: role.to_string(),
|
||||
text: text.to_string(),
|
||||
complete: true,
|
||||
before_turn_id: None,
|
||||
})
|
||||
.collect(),
|
||||
);
|
||||
@@ -948,6 +952,16 @@ async fn completed_voice_caption_survives_repeated_thread_replacement() {
|
||||
.collect::<String>();
|
||||
assert_eq!(initial.matches("spoken complete").count(), 1);
|
||||
|
||||
let typed = test_turn(
|
||||
"later-typed-turn",
|
||||
TurnStatus::Completed,
|
||||
vec![test_user_message("later-user", "Later typed question")],
|
||||
);
|
||||
app.chat_widget.handle_server_notification(
|
||||
turn_started_notification(source, &typed.id),
|
||||
/*replay_kind*/ None,
|
||||
);
|
||||
|
||||
for cycle in 0..2 {
|
||||
let side = ThreadId::new();
|
||||
let (side_widget, _, mut side_events, _) = make_chatwidget_manual_with_sender().await;
|
||||
@@ -969,10 +983,14 @@ async fn completed_voice_caption_survives_repeated_thread_replacement() {
|
||||
let (source_widget, _, mut source_events, _) = make_chatwidget_manual_with_sender().await;
|
||||
app.active_thread_id = Some(source);
|
||||
app.replace_chat_widget(source_widget);
|
||||
app.replay_thread_snapshot(
|
||||
empty_thread_snapshot(&app, source),
|
||||
/*resume_restored_queue*/ false,
|
||||
);
|
||||
let mut snapshot = empty_thread_snapshot(&app, source);
|
||||
snapshot.turns.push(Turn {
|
||||
id: "earlier-typed-turn".into(),
|
||||
items: vec![test_agent_message("earlier-answer", "Earlier answer")],
|
||||
..typed.clone()
|
||||
});
|
||||
snapshot.turns.push(typed.clone());
|
||||
app.replay_thread_snapshot(snapshot, /*resume_restored_queue*/ false);
|
||||
assert!(!app.pending_realtime_transcript_replay.contains_key(&source));
|
||||
let rendered = std::iter::from_fn(|| source_events.try_recv().ok())
|
||||
.filter_map(|event| match event {
|
||||
@@ -988,6 +1006,13 @@ async fn completed_voice_caption_survives_repeated_thread_replacement() {
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
assert_eq!(rendered.matches("spoken complete").count(), 1);
|
||||
assert!(
|
||||
rendered.find("Earlier answer").unwrap() < rendered.find("spoken complete").unwrap()
|
||||
);
|
||||
assert!(
|
||||
rendered.find("spoken complete").unwrap()
|
||||
< rendered.find("Later typed question").unwrap()
|
||||
);
|
||||
if cycle == 0 {
|
||||
insta::assert_snapshot!(
|
||||
"voice_completed_after_thread_switch",
|
||||
@@ -997,6 +1022,78 @@ async fn completed_voice_caption_survives_repeated_thread_replacement() {
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn inactive_caption_precedes_later_buffered_turn_without_start_event() {
|
||||
for include_delta in [false, true] {
|
||||
let (mut app, _initial_events, _ops) = make_test_app_with_channels().await;
|
||||
let source = ThreadId::new();
|
||||
app.retain_inactive_realtime_transcript(
|
||||
source,
|
||||
&ServerNotification::ThreadRealtimeTranscriptDone(
|
||||
codex_app_server_protocol::ThreadRealtimeTranscriptDoneNotification {
|
||||
thread_id: source.to_string(),
|
||||
role: "assistant".into(),
|
||||
text: "Earlier spoken answer".into(),
|
||||
},
|
||||
),
|
||||
);
|
||||
app.retain_inactive_realtime_transcript(
|
||||
source,
|
||||
&turn_started_notification(source, "later-turn"),
|
||||
);
|
||||
let (widget, _, mut events, _) = make_chatwidget_manual_with_sender().await;
|
||||
app.active_thread_id = Some(source);
|
||||
app.replace_chat_widget(widget);
|
||||
let mut snapshot = empty_thread_snapshot(&app, source);
|
||||
if include_delta {
|
||||
snapshot
|
||||
.events
|
||||
.push(ThreadBufferedEvent::Notification(Box::new(
|
||||
ServerNotification::AgentMessageDelta(
|
||||
codex_app_server_protocol::AgentMessageDeltaNotification {
|
||||
thread_id: source.to_string(),
|
||||
turn_id: "later-turn".into(),
|
||||
item_id: "later-answer".into(),
|
||||
delta: "Later typed question".into(),
|
||||
},
|
||||
),
|
||||
)));
|
||||
}
|
||||
// The bounded replay buffer may have evicted both the start and item notifications.
|
||||
snapshot
|
||||
.events
|
||||
.push(ThreadBufferedEvent::Notification(Box::new(
|
||||
ServerNotification::TurnCompleted(TurnCompletedNotification {
|
||||
thread_id: source.to_string(),
|
||||
turn: test_turn(
|
||||
"later-turn",
|
||||
TurnStatus::Completed,
|
||||
vec![test_agent_message("later-answer", "Later typed question")],
|
||||
),
|
||||
}),
|
||||
)));
|
||||
app.replay_thread_snapshot(snapshot, /*resume_restored_queue*/ false);
|
||||
let rendered = std::iter::from_fn(|| events.try_recv().ok())
|
||||
.filter_map(|event| match event {
|
||||
AppEvent::InsertHistoryCell(cell) => Some(
|
||||
cell.transcript_lines(/*width*/ 80)
|
||||
.into_iter()
|
||||
.map(|line| line.to_string())
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n"),
|
||||
),
|
||||
_ => None,
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
assert_eq!(rendered.matches("Earlier spoken answer").count(), 1);
|
||||
assert!(
|
||||
rendered.find("Earlier spoken answer").unwrap()
|
||||
< rendered.find("Later typed question").unwrap()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn rejected_realtime_speech_restores_the_delegated_final_answer() -> Result<()> {
|
||||
let (mut app, mut events, mut ops) = make_test_app_with_channels().await;
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
---
|
||||
source: tui/src/app/tests/realtime_requests.rs
|
||||
assertion_line: 182
|
||||
expression: "rendered.replace(&app.config.cwd.display().to_string(), \"/tmp/project\")"
|
||||
assertion_line: 1023
|
||||
expression: "normalize_voice_snapshot_directory(&rendered, &app.config.cwd)"
|
||||
---
|
||||
╭────────────────────────────────────────╮
|
||||
│ >_ OpenAI Codex (v0.0.0) │
|
||||
@@ -17,4 +17,7 @@ expression: "rendered.replace(&app.config.cwd.display().to_string(), \"/tmp/proj
|
||||
/permissions - choose what Codex is allowed to do
|
||||
/model - choose what model and reasoning effort to use
|
||||
/review - review any changes and find issues
|
||||
• Earlier answer
|
||||
• spoken complete
|
||||
|
||||
› Later typed question
|
||||
|
||||
@@ -1545,6 +1545,8 @@ impl App {
|
||||
let should_buffer_initial_replay = !turns.is_empty();
|
||||
let replayed_final_items = realtime_delivery::completed_agent_items_from_turns(&turns);
|
||||
let replayed_voice_texts = realtime_delivery::replayed_voice_texts_from_turns(&turns);
|
||||
let retained_assistant_captions =
|
||||
self.prepare_realtime_transcript_replay(replayed_voice_texts);
|
||||
if should_buffer_initial_replay {
|
||||
self.app_event_tx
|
||||
.send(AppEvent::BeginInitialHistoryReplayBuffer);
|
||||
@@ -1562,7 +1564,7 @@ impl App {
|
||||
}
|
||||
self.restore_realtime_replay_state_after_replay(
|
||||
&replayed_final_items,
|
||||
replayed_voice_texts,
|
||||
retained_assistant_captions,
|
||||
);
|
||||
if matches!(presentation, ThreadAttachPresentation::PromptEdit) {
|
||||
self.chat_widget.emit_prompt_edit_thread_event();
|
||||
@@ -1824,6 +1826,8 @@ impl App {
|
||||
self.chat_widget.handle_thread_session(session);
|
||||
}
|
||||
}
|
||||
let retained_assistant_captions =
|
||||
self.prepare_realtime_transcript_replay(replayed_voice_texts);
|
||||
for turn_id in &snapshot.delegated_turns {
|
||||
self.chat_widget
|
||||
.remember_realtime_delegated_reasoning_turn(turn_id);
|
||||
@@ -1866,7 +1870,7 @@ impl App {
|
||||
}
|
||||
self.restore_realtime_replay_state_after_replay(
|
||||
&replayed_final_items,
|
||||
replayed_voice_texts,
|
||||
retained_assistant_captions,
|
||||
);
|
||||
self.chat_widget
|
||||
.set_queue_autosend_suppressed(/*suppressed*/ false);
|
||||
|
||||
@@ -65,6 +65,11 @@ impl ChatWidget {
|
||||
self.on_thread_settings_updated(notification);
|
||||
}
|
||||
ServerNotification::TurnStarted(notification) => {
|
||||
if from_replay {
|
||||
self.restore_realtime_transcripts_before_turn(¬ification.turn.id);
|
||||
} else {
|
||||
self.anchor_realtime_transcripts_before_turn(¬ification.turn.id);
|
||||
}
|
||||
if replay_kind.is_none() {
|
||||
self.clear_misalignment_for_new_turn(
|
||||
¬ification.turn.id,
|
||||
@@ -79,6 +84,7 @@ impl ChatWidget {
|
||||
}
|
||||
}
|
||||
ServerNotification::TurnCompleted(notification) => {
|
||||
self.restore_realtime_transcripts_before_turn(¬ification.turn.id);
|
||||
self.handle_turn_completed_notification(notification, replay_kind);
|
||||
}
|
||||
ServerNotification::ItemStarted(notification) => {
|
||||
@@ -88,6 +94,7 @@ impl ChatWidget {
|
||||
self.handle_item_completed_notification(notification, replay_kind);
|
||||
}
|
||||
ServerNotification::AgentMessageDelta(notification) => {
|
||||
self.restore_realtime_transcripts_before_turn(¬ification.turn_id);
|
||||
if !self.is_realtime_delegated_reasoning_turn(¬ification.turn_id)
|
||||
&& (from_replay
|
||||
|| !self.is_realtime_delegated_agent_item(
|
||||
@@ -98,7 +105,10 @@ impl ChatWidget {
|
||||
self.on_agent_message_delta(notification.delta);
|
||||
}
|
||||
}
|
||||
ServerNotification::PlanDelta(notification) => self.on_plan_delta(notification.delta),
|
||||
ServerNotification::PlanDelta(notification) => {
|
||||
self.restore_realtime_transcripts_before_turn(¬ification.turn_id);
|
||||
self.on_plan_delta(notification.delta);
|
||||
}
|
||||
ServerNotification::ReasoningSummaryTextDelta(notification) => {
|
||||
if !self.is_realtime_delegated_reasoning_item(
|
||||
¬ification.turn_id,
|
||||
@@ -477,6 +487,7 @@ impl ChatWidget {
|
||||
notification: ItemStartedNotification,
|
||||
replay_kind: Option<ReplayKind>,
|
||||
) {
|
||||
self.restore_realtime_transcripts_before_turn(¬ification.turn_id);
|
||||
match notification.item {
|
||||
ThreadItem::UserMessage { content, .. } if replay_kind.is_none() => {
|
||||
self.note_realtime_user_item_started(¬ification.turn_id, &content);
|
||||
@@ -565,6 +576,7 @@ impl ChatWidget {
|
||||
notification: ItemCompletedNotification,
|
||||
replay_kind: Option<ReplayKind>,
|
||||
) {
|
||||
self.restore_realtime_transcripts_before_turn(¬ification.turn_id);
|
||||
if replay_kind.is_none()
|
||||
&& self.is_realtime_delegated_reasoning_turn(¬ification.turn_id)
|
||||
&& realtime::is_private_realtime_agent_item(¬ification.item)
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
//! Completed captions and both speakers' partials stay bounded across widget replacement.
|
||||
|
||||
mod recording_controls;
|
||||
mod transcript_replay;
|
||||
|
||||
use super::ChatWidget;
|
||||
use super::HistoryCell;
|
||||
@@ -72,6 +73,7 @@ pub(crate) struct RealtimeTranscriptRecord {
|
||||
pub(crate) role: String,
|
||||
pub(crate) text: String,
|
||||
pub(crate) complete: bool,
|
||||
pub(crate) before_turn_id: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, PartialEq, Eq)]
|
||||
@@ -161,6 +163,7 @@ pub(super) struct RealtimeConversationUiState {
|
||||
pub(super) live_transcript_cell: Option<Box<dyn HistoryCell>>,
|
||||
pending_history_cells: VecDeque<Box<dyn HistoryCell>>,
|
||||
accepted_transcripts: VecDeque<RealtimeTranscriptRecord>,
|
||||
replay_transcripts: Option<VecDeque<RealtimeTranscriptRecord>>,
|
||||
latest_input_was_voice: bool,
|
||||
input_generation: u64,
|
||||
latest_voice_input_fingerprint: Option<(usize, u64)>,
|
||||
@@ -1014,6 +1017,7 @@ impl ChatWidget {
|
||||
role,
|
||||
text,
|
||||
complete: false,
|
||||
before_turn_id: None,
|
||||
});
|
||||
};
|
||||
if let Some((role, text)) = self.realtime_conversation.interleaved_transcript.take() {
|
||||
@@ -1347,6 +1351,7 @@ impl ChatWidget {
|
||||
role: role.clone(),
|
||||
text: text.clone(),
|
||||
complete: true,
|
||||
before_turn_id: None,
|
||||
},
|
||||
);
|
||||
text.clone()
|
||||
@@ -1488,6 +1493,7 @@ impl ChatWidget {
|
||||
role: role.clone(),
|
||||
text: text.clone(),
|
||||
complete: true,
|
||||
before_turn_id: None,
|
||||
});
|
||||
while self.realtime_conversation.pending_history_cells.len()
|
||||
>= MAX_PENDING_TRANSCRIPT_CELLS
|
||||
@@ -1713,6 +1719,7 @@ impl ChatWidget {
|
||||
role,
|
||||
text,
|
||||
complete: true,
|
||||
before_turn_id: None,
|
||||
});
|
||||
}
|
||||
let pending_history_cells =
|
||||
|
||||
57
codex-rs/tui/src/chatwidget/realtime/transcript_replay.rs
Normal file
57
codex-rs/tui/src/chatwidget/realtime/transcript_replay.rs
Normal file
@@ -0,0 +1,57 @@
|
||||
//! Keep retained voice captions before the subsequent agent turn during history replay.
|
||||
//!
|
||||
//! Anchors are assigned only to completed captions when a new live turn starts. Partial
|
||||
//! captions remain eligible for late completion, and captions with no later turn stay last.
|
||||
|
||||
use super::ChatWidget;
|
||||
use super::MAX_REPLAY_TRANSCRIPT_CELLS;
|
||||
use super::RealtimeTranscriptRecord;
|
||||
use std::collections::VecDeque;
|
||||
|
||||
impl ChatWidget {
|
||||
pub(crate) fn anchor_realtime_transcripts_before_turn(&mut self, turn_id: &str) {
|
||||
for record in &mut self.realtime_conversation.accepted_transcripts {
|
||||
if record.complete && record.before_turn_id.is_none() {
|
||||
record.before_turn_id = Some(turn_id.to_string());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn queue_realtime_transcripts_for_replay(
|
||||
&mut self,
|
||||
records: VecDeque<RealtimeTranscriptRecord>,
|
||||
) {
|
||||
self.realtime_conversation.replay_transcripts = Some(records);
|
||||
}
|
||||
|
||||
pub(crate) fn restore_realtime_transcripts_before_turn(&mut self, turn_id: &str) {
|
||||
let mut remaining = VecDeque::new();
|
||||
let Some(records) = self.realtime_conversation.replay_transcripts.take() else {
|
||||
return;
|
||||
};
|
||||
for record in records {
|
||||
if record.before_turn_id.as_deref() == Some(turn_id) {
|
||||
// Insert after the queued consolidation, before replaying the next turn.
|
||||
let cell = self.realtime_transcript_history_cell(&record.role, &record.text);
|
||||
self.add_boxed_history(cell);
|
||||
if self.realtime_conversation.accepted_transcripts.len()
|
||||
>= MAX_REPLAY_TRANSCRIPT_CELLS
|
||||
{
|
||||
self.realtime_conversation.accepted_transcripts.pop_front();
|
||||
}
|
||||
self.realtime_conversation
|
||||
.accepted_transcripts
|
||||
.push_back(record);
|
||||
} else {
|
||||
remaining.push_back(record);
|
||||
}
|
||||
}
|
||||
self.realtime_conversation.replay_transcripts = Some(remaining);
|
||||
}
|
||||
|
||||
pub(crate) fn finish_realtime_transcript_replay(&mut self) {
|
||||
if let Some(records) = self.realtime_conversation.replay_transcripts.take() {
|
||||
self.restore_realtime_transcript_cells(records);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -333,6 +333,7 @@ async fn restored_partial_caption_accepts_late_completion_without_duplicate_hist
|
||||
role: "user".into(),
|
||||
text: "last ".into(),
|
||||
complete: false,
|
||||
before_turn_id: None,
|
||||
},
|
||||
]));
|
||||
let live = chat
|
||||
@@ -383,6 +384,7 @@ async fn empty_late_completion_discards_the_restored_partial() {
|
||||
role: "user".into(),
|
||||
text: "unfinished".into(),
|
||||
complete: false,
|
||||
before_turn_id: None,
|
||||
},
|
||||
]));
|
||||
chat.on_realtime_transcript_done("user".into(), String::new());
|
||||
|
||||
@@ -112,6 +112,7 @@ impl ChatWidget {
|
||||
}))
|
||||
.collect::<Vec<_>>();
|
||||
for (turn, hidden_nested_review_turn) in turns.into_iter().zip(hidden_nested_review_turns) {
|
||||
self.restore_realtime_transcripts_before_turn(&turn.id);
|
||||
// Defer completed metadata-only turns until their page loads. Active
|
||||
// turns must restore their lifecycle even before any items are available.
|
||||
if turn.status == TurnStatus::Completed
|
||||
|
||||
Reference in New Issue
Block a user