mirror of
https://github.com/openai/codex.git
synced 2026-05-03 12:52:11 +03:00
fix(tui): only show 'Worked for' separator when actual work was performed (#8958)
Fixes #7919. This PR addresses a TUI display bug where the "Worked for" separator would appear prematurely during the planning stage. **Changes:** - Added `had_work_activity` flag to `ChatWidget` to track if actual work (exec commands, MCP tool calls, patches) was performed in the current turn. - Updated `handle_streaming_delta` to only display the `FinalMessageSeparator` if both `needs_final_message_separator` AND `had_work_activity` are true. - Updated `handle_exec_end_now`, `handle_patch_apply_end_now`, and `handle_mcp_end_now` to set `had_work_activity = true`. **Verification:** - Ran `cargo test -p codex-tui` to ensure no regressions. - Manual verification confirms the separator now only appears after actual work is completed. --------- Co-authored-by: Josh McKinney <joshka@openai.com>
This commit is contained in:
@@ -462,8 +462,17 @@ pub(crate) struct ChatWidget {
|
||||
is_review_mode: bool,
|
||||
// Snapshot of token usage to restore after review mode exits.
|
||||
pre_review_token_info: Option<Option<TokenUsageInfo>>,
|
||||
// Whether to add a final message separator after the last message
|
||||
// Whether the next streamed assistant content should be preceded by a final message separator.
|
||||
//
|
||||
// This is set whenever we insert a visible history cell that conceptually belongs to a turn.
|
||||
// The separator itself is only rendered if the turn recorded "work" activity (see
|
||||
// `had_work_activity`).
|
||||
needs_final_message_separator: bool,
|
||||
// Whether the current turn performed "work" (exec commands, MCP tool calls, patch applications).
|
||||
//
|
||||
// This gates rendering of the "Worked for …" separator so purely conversational turns don't
|
||||
// show an empty divider. It is reset when the separator is emitted.
|
||||
had_work_activity: bool,
|
||||
|
||||
last_rendered_width: std::cell::Cell<Option<usize>>,
|
||||
// Feedback sink for /feedback
|
||||
@@ -1348,13 +1357,19 @@ impl ChatWidget {
|
||||
self.flush_active_cell();
|
||||
|
||||
if self.stream_controller.is_none() {
|
||||
if self.needs_final_message_separator {
|
||||
// If the previous turn inserted non-stream history (exec output, patch status, MCP
|
||||
// calls), render a separator before starting the next streamed assistant message.
|
||||
if self.needs_final_message_separator && self.had_work_activity {
|
||||
let elapsed_seconds = self
|
||||
.bottom_pane
|
||||
.status_widget()
|
||||
.map(super::status_indicator_widget::StatusIndicatorWidget::elapsed_seconds);
|
||||
self.add_to_history(history_cell::FinalMessageSeparator::new(elapsed_seconds));
|
||||
self.needs_final_message_separator = false;
|
||||
self.had_work_activity = false;
|
||||
} else if self.needs_final_message_separator {
|
||||
// Reset the flag even if we don't show separator (no work was done)
|
||||
self.needs_final_message_separator = false;
|
||||
}
|
||||
self.stream_controller = Some(StreamController::new(
|
||||
self.last_rendered_width.get().map(|w| w.saturating_sub(2)),
|
||||
@@ -1423,6 +1438,8 @@ impl ChatWidget {
|
||||
self.request_redraw();
|
||||
}
|
||||
}
|
||||
// Mark that actual work was done (command executed)
|
||||
self.had_work_activity = true;
|
||||
}
|
||||
|
||||
pub(crate) fn handle_patch_apply_end_now(
|
||||
@@ -1434,6 +1451,8 @@ impl ChatWidget {
|
||||
if !event.success {
|
||||
self.add_to_history(history_cell::new_patch_apply_failure(event.stderr));
|
||||
}
|
||||
// Mark that actual work was done (patch applied)
|
||||
self.had_work_activity = true;
|
||||
}
|
||||
|
||||
pub(crate) fn handle_exec_approval_now(&mut self, id: String, ev: ExecApprovalRequestEvent) {
|
||||
@@ -1599,6 +1618,8 @@ impl ChatWidget {
|
||||
if let Some(extra) = extra_cell {
|
||||
self.add_boxed_history(extra);
|
||||
}
|
||||
// Mark that actual work was done (MCP tool call)
|
||||
self.had_work_activity = true;
|
||||
}
|
||||
|
||||
pub(crate) fn new(common: ChatWidgetInit, thread_manager: Arc<ThreadManager>) -> Self {
|
||||
@@ -1687,6 +1708,7 @@ impl ChatWidget {
|
||||
is_review_mode: false,
|
||||
pre_review_token_info: None,
|
||||
needs_final_message_separator: false,
|
||||
had_work_activity: false,
|
||||
last_rendered_width: std::cell::Cell::new(None),
|
||||
feedback,
|
||||
current_rollout_path: None,
|
||||
@@ -1784,6 +1806,7 @@ impl ChatWidget {
|
||||
is_review_mode: false,
|
||||
pre_review_token_info: None,
|
||||
needs_final_message_separator: false,
|
||||
had_work_activity: false,
|
||||
last_rendered_width: std::cell::Cell::new(None),
|
||||
feedback,
|
||||
current_rollout_path: None,
|
||||
|
||||
Reference in New Issue
Block a user