clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-05-16 17:23:57 +00:00
Code Issues Packages Projects Releases Wiki Activity

codex: address PR review feedback (#18290)

Browse Source
This commit is contained in:
Eric Traut
2026-04-17 01:53:08 -07:00
parent a582474d40
commit e925c8d801
3 changed files with 107 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files

124
codex-rs/tui/src/chatwidget.rs
View File

@@ -2424,7 +2424,8 @@ impl ChatWidget {
self.unified_exec_wait_streak = None;
self.request_redraw();
let had_unacknowledged_pending_steers = self.queue_unacknowledged_pending_steers();
let had_unacknowledged_pending_steers =
!from_replay && self.queue_unacknowledged_pending_steers();
self.refresh_pending_input_preview();
if !from_replay
@@ -2549,6 +2550,42 @@ impl ChatWidget {
self.request_redraw();
}
fn on_user_message_event_reconciling_pending_steer(
&mut self,
event: UserMessageEvent,
compare_key: PendingSteerCompareKey,
) {
let rendered = Self::rendered_user_message_event_from_event(&event);
if self
.pending_steers
.front()
.is_some_and(|pending| pending.compare_key == compare_key)
{
if let Some(pending) = self.pending_steers.pop_front() {
self.refresh_pending_input_preview();
let pending_event = UserMessageEvent {
message: pending.user_message.text,
images: Some(pending.user_message.remote_image_urls),
local_images: pending
.user_message
.local_images
.into_iter()
.map(|image| image.path)
.collect(),
text_elements: pending.user_message.text_elements,
};
self.on_user_message_event(pending_event);
} else if self.last_rendered_user_message_event.as_ref() != Some(&rendered) {
tracing::warn!(
"pending steer matched compare key but queue was empty when rendering committed user message"
);
self.on_user_message_event(event);
}
} else if self.last_rendered_user_message_event.as_ref() != Some(&rendered) {
self.on_user_message_event(event);
}
}
fn pop_next_queued_user_message(&mut self) -> Option<UserMessage> {
if self.rejected_steers_queue.is_empty() {
self.queued_user_messages.pop_front()
@@ -5779,42 +5816,8 @@ impl ChatWidget {
else {
unreachable!("user message item should convert to a user message event");
};
if from_replay {
self.on_user_message_event(event);
} else {
let rendered = Self::rendered_user_message_event_from_event(&event);
let compare_key =
Self::pending_steer_compare_key_from_items(&user_message.content);
if self
.pending_steers
.front()
.is_some_and(|pending| pending.compare_key == compare_key)
{
if let Some(pending) = self.pending_steers.pop_front() {
self.refresh_pending_input_preview();
let pending_event = UserMessageEvent {
message: pending.user_message.text,
images: Some(pending.user_message.remote_image_urls),
local_images: pending
.user_message
.local_images
.into_iter()
.map(|image| image.path)
.collect(),
text_elements: pending.user_message.text_elements,
};
self.on_user_message_event(pending_event);
} else if self.last_rendered_user_message_event.as_ref() != Some(&rendered)
{
tracing::warn!(
"pending steer matched compare key but queue was empty when rendering committed user message"
);
self.on_user_message_event(event);
}
} else if self.last_rendered_user_message_event.as_ref() != Some(&rendered) {
self.on_user_message_event(event);
}
}
let compare_key = Self::pending_steer_compare_key_from_items(&user_message.content);
self.on_user_message_event_reconciling_pending_steer(event, compare_key);
}
ThreadItem::AgentMessage {
id,
@@ -6237,6 +6240,8 @@ impl ChatWidget {
}
} else if from_replay {
self.last_non_retry_error = None;
self.finalize_turn();
self.request_redraw();
} else {
self.last_non_retry_error = Some((
notification.turn_id.clone(),
@@ -6763,10 +6768,12 @@ impl ChatWidget {
message,
codex_error_info,
}) => {
if from_replay
|| codex_error_info
.as_ref()
.is_some_and(|info| self.handle_steer_rejected_error(info))
if from_replay {
self.finalize_turn();
self.request_redraw();
} else if codex_error_info
.as_ref()
.is_some_and(|info| self.handle_steer_rejected_error(info))
{
} else if let Some(kind) = codex_error_info
.as_ref()
@@ -6933,41 +6940,12 @@ impl ChatWidget {
}
EventMsg::ItemCompleted(event) => {
let item = event.item;
if !from_replay && let codex_protocol::items::TurnItem::UserMessage(item) = &item {
if let codex_protocol::items::TurnItem::UserMessage(item) = &item {
let EventMsg::UserMessage(event) = item.as_legacy_event() else {
unreachable!("user message item should convert to a legacy user message");
};
let rendered = Self::rendered_user_message_event_from_event(&event);
let compare_key = Self::pending_steer_compare_key_from_item(item);
if self
.pending_steers
.front()
.is_some_and(|pending| pending.compare_key == compare_key)
{
if let Some(pending) = self.pending_steers.pop_front() {
self.refresh_pending_input_preview();
let pending_event = UserMessageEvent {
message: pending.user_message.text,
images: Some(pending.user_message.remote_image_urls),
local_images: pending
.user_message
.local_images
.into_iter()
.map(|image| image.path)
.collect(),
text_elements: pending.user_message.text_elements,
};
self.on_user_message_event(pending_event);
} else if self.last_rendered_user_message_event.as_ref() != Some(&rendered)
{
tracing::warn!(
"pending steer matched compare key but queue was empty when rendering committed user message"
);
self.on_user_message_event(event);
}
} else if self.last_rendered_user_message_event.as_ref() != Some(&rendered) {
self.on_user_message_event(event);
}
let compare_key = Self::pending_steer_compare_key_from_items(&item.content);
self.on_user_message_event_reconciling_pending_steer(event, compare_key);
}
if let codex_protocol::items::TurnItem::Plan(plan_item) = &item {
self.on_plan_item_completed(plan_item.text.clone());

7
codex-rs/tui/src/chatwidget/realtime.rs
View File

@@ -132,13 +132,6 @@ impl ChatWidget {
}
}
#[cfg(test)]
pub(super) fn pending_steer_compare_key_from_item(
item: &codex_protocol::items::UserMessageItem,
) -> PendingSteerCompareKey {
Self::pending_steer_compare_key_from_items(&item.content)
}
#[cfg(test)]
pub(super) fn rendered_user_message_event_from_inputs(
items: &[UserInput],

56
codex-rs/tui/src/chatwidget/tests/review_mode.rs
View File

@@ -467,6 +467,62 @@ async fn unacknowledged_pending_steer_is_retried_as_follow_up_when_turn_complete
assert!(chat.rejected_steers_queue.is_empty());
}
#[tokio::test]
async fn replayed_user_message_commit_clears_pending_steer_without_retry() {
let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
chat.thread_id = Some(ThreadId::new());
chat.on_task_started();
chat.bottom_pane
.set_composer_text("already committed".to_string(), Vec::new(), Vec::new());
chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
match next_submit_op(&mut op_rx) {
Op::UserTurn { .. } => {}
other => panic!("expected running-turn steer submit, got {other:?}"),
}
assert_eq!(chat.pending_steers.len(), 1);
chat.replay_thread_turns(
vec![AppServerTurn {
id: "older-turn".to_string(),
items: Vec::new(),
status: AppServerTurnStatus::Completed,
error: None,
started_at: None,
completed_at: None,
duration_ms: None,
}],
ReplayKind::ThreadSnapshot,
);
assert_eq!(chat.pending_steers.len(), 1);
assert_no_submit_op(&mut op_rx);
chat.replay_thread_turns(
vec![AppServerTurn {
id: "turn-with-commit".to_string(),
items: vec![AppServerThreadItem::UserMessage {
id: "user-1".to_string(),
content: vec![AppServerUserInput::Text {
text: "already committed".to_string(),
text_elements: Vec::new(),
}],
}],
status: AppServerTurnStatus::Completed,
error: None,
started_at: None,
completed_at: None,
duration_ms: None,
}],
ReplayKind::ThreadSnapshot,
);
assert!(chat.pending_steers.is_empty());
assert!(chat.rejected_steers_queue.is_empty());
assert_no_submit_op(&mut op_rx);
}
#[tokio::test]
async fn steer_enter_uses_pending_steers_while_final_answer_stream_is_active() {
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await;