Fix duplicate /review messages in app-server TUI (#15839)

## Symptoms
When `/review` ran through `tui_app_server`, the TUI could show
duplicate review content:
- the `>> Code review started: ... <<` banner appeared twice
- the final review body could also appear twice

## Problem
`tui_app_server` was treating review lifecycle items as renderable
content on more than one delivery path.

Specifically:
- `EnteredReviewMode` was rendered both when the item started and again
when it completed
- `ExitedReviewMode` rendered the review text itself, even though the
same review text was also delivered later as the assistant message item

That meant the same logical review event was committed into history
multiple times.

## Solution
Make review lifecycle items control state transitions only once, and
keep the final review body sourced from the assistant message item:
- render the review-start banner from the live `ItemStarted` path, while
still allowing replay to restore it once
- treat `ExitedReviewMode` as a mode-exit/finish-banner event instead of
rendering the review body from it
- preserve the existing assistant-message rendering path as the single
source of final review text
This commit is contained in:
Eric Traut
2026-03-26 10:55:18 -06:00
committed by GitHub
Unverified
parent 352f37db03
commit 0d44bd708e
+35 -34
View File
@@ -850,7 +850,6 @@ pub(crate) struct ChatWidget {
quit_shortcut_key: Option<KeyBinding>,
// Simple review mode flag; used to adjust layout and banners.
is_review_mode: bool,
#[cfg(test)]
// Snapshot of token usage to restore after review mode exits.
pre_review_token_info: Option<Option<TokenUsageInfo>>,
// Whether the next streamed assistant content should be preceded by a final message separator.
@@ -2509,7 +2508,6 @@ impl ChatWidget {
Some(info.total_token_usage.tokens_in_context_window())
}
#[cfg(test)]
fn restore_pre_review_token_info(&mut self) {
if let Some(saved) = self.pre_review_token_info.take() {
match saved {
@@ -4467,7 +4465,6 @@ impl ChatWidget {
quit_shortcut_expires_at: None,
quit_shortcut_key: None,
is_review_mode: false,
#[cfg(test)]
pre_review_token_info: None,
needs_final_message_separator: false,
had_work_activity: false,
@@ -5962,17 +5959,12 @@ impl ChatWidget {
});
}
ThreadItem::EnteredReviewMode { review, .. } => {
self.add_to_history(history_cell::new_review_status_line(format!(
">> Code review started: {review} <<"
)));
if !self.bottom_pane.is_task_running() {
self.bottom_pane.set_task_running(/*running*/ true);
if from_replay {
self.enter_review_mode_with_hint(review, /*from_replay*/ true);
}
self.is_review_mode = true;
}
ThreadItem::ExitedReviewMode { review, .. } => {
self.on_agent_message(review);
self.is_review_mode = false;
ThreadItem::ExitedReviewMode { .. } => {
self.exit_review_mode_after_item();
}
ThreadItem::ContextCompaction { .. } => {
self.on_agent_message("Context compacted".to_owned());
@@ -6097,7 +6089,7 @@ impl ChatWidget {
self.handle_turn_completed_notification(notification, replay_kind);
}
ServerNotification::ItemStarted(notification) => {
self.handle_item_started_notification(notification);
self.handle_item_started_notification(notification, replay_kind.is_some());
}
ServerNotification::ItemCompleted(notification) => {
self.handle_item_completed_notification(notification, replay_kind);
@@ -6381,7 +6373,11 @@ impl ChatWidget {
}
}
fn handle_item_started_notification(&mut self, notification: ItemStartedNotification) {
fn handle_item_started_notification(
&mut self,
notification: ItemStartedNotification,
from_replay: bool,
) {
match notification.item {
ThreadItem::CommandExecution {
id,
@@ -6458,10 +6454,9 @@ impl ChatWidget {
agents_states,
}),
ThreadItem::EnteredReviewMode { review, .. } => {
self.add_to_history(history_cell::new_review_status_line(format!(
">> Code review started: {review} <<"
)));
self.is_review_mode = true;
if !from_replay {
self.enter_review_mode_with_hint(review, /*from_replay*/ false);
}
}
_ => {}
}
@@ -6861,28 +6856,41 @@ impl ChatWidget {
}
}
#[cfg(test)]
fn on_entered_review_mode(&mut self, review: ReviewRequest, from_replay: bool) {
// Enter review mode and emit a concise banner
fn enter_review_mode_with_hint(&mut self, hint: String, from_replay: bool) {
if self.pre_review_token_info.is_none() {
self.pre_review_token_info = Some(self.token_info.clone());
}
// Avoid toggling running state for replayed history events on resume.
if !from_replay && !self.bottom_pane.is_task_running() {
self.bottom_pane.set_task_running(/*running*/ true);
}
self.is_review_mode = true;
let hint = review
.user_facing_hint
.unwrap_or_else(|| codex_core::review_prompts::user_facing_hint(&review.target));
let banner = format!(">> Code review started: {hint} <<");
self.add_to_history(history_cell::new_review_status_line(banner));
self.request_redraw();
}
fn exit_review_mode_after_item(&mut self) {
self.flush_answer_stream_with_separator();
self.flush_interrupt_queue();
self.flush_active_cell();
self.is_review_mode = false;
self.restore_pre_review_token_info();
self.add_to_history(history_cell::new_review_status_line(
"<< Code review finished >>".to_string(),
));
self.request_redraw();
}
#[cfg(test)]
fn on_entered_review_mode(&mut self, review: ReviewRequest, from_replay: bool) {
let hint = review
.user_facing_hint
.unwrap_or_else(|| codex_core::review_prompts::user_facing_hint(&review.target));
self.enter_review_mode_with_hint(hint, from_replay);
}
#[cfg(test)]
fn on_exited_review_mode(&mut self, review: ExitedReviewModeEvent) {
// Leave review mode; if output is present, flush pending stream + show results.
if let Some(output) = review.review_output {
self.flush_answer_stream_with_separator();
self.flush_interrupt_queue();
@@ -6911,14 +6919,7 @@ impl ChatWidget {
}
// Final message is rendered as part of the AgentMessage.
}
self.is_review_mode = false;
self.restore_pre_review_token_info();
// Append a finishing banner at the end of this turn.
self.add_to_history(history_cell::new_review_status_line(
"<< Code review finished >>".to_string(),
));
self.request_redraw();
self.exit_review_mode_after_item();
}
fn on_user_message_event(&mut self, event: UserMessageEvent) {