mirror of
https://github.com/openai/codex.git
synced 2026-05-02 20:32:04 +03:00
fix(tui): keep unified exec summary on working line (#10962)
## Problem When unified-exec background sessions appear while the status indicator is visible, the bottom pane can grow by one row to show a dedicated footer line. That row insertion/removal makes the composer jump vertically and produces visible jitter/flicker during streaming turns. ## Mental model The bottom pane should expose one canonical background-exec summary string, but it should surface that string in only one place at a time: - if the status indicator row is visible, show the summary inline on that row; - if the status indicator row is hidden, show the summary as the standalone unified-exec footer row. This keeps status information visible while preserving a stable pane height. ## Non-goals This change does not alter unified-exec lifecycle, process tracking, or `/ps` behavior. It does not redesign status text copy, spinner timing, or interrupt handling semantics. ## Tradeoffs Inlining the summary preserves layout stability and keeps interrupt affordances in a fixed location, but it reduces horizontal space for long status/detail text in narrow terminals. We accept that truncation risk in exchange for removing vertical jitter and keeping the composer anchored. ## Architecture `UnifiedExecFooter` remains the source of truth for background-process summary copy via `summary_text()`. `BottomPane` mirrors that text into `StatusIndicatorWidget::update_inline_message()` whenever process state changes or a status widget is created. Rendering enforces single-surface output: the standalone footer row is skipped while status is present, and the status row appends the summary after the elapsed/interrupt segment. ## Documentation pass Added non-functional docs/comments that make the new invariant explicit: - status row owns inline summary when present; - unified-exec footer row renders only when status row is absent; - summary ordering keeps elapsed/interrupt affordance in a stable position. ## Observability No new telemetry or logs are introduced. The behavior is traceable through: - `BottomPane::set_unified_exec_processes()` for state updates, - `BottomPane::sync_status_inline_message()` for status-row synchronization, - `StatusIndicatorWidget::render()` for final inline ordering. ## Tests - Added `bottom_pane::tests::unified_exec_summary_does_not_increase_height_when_status_visible` to lock the no-height-growth invariant. - Updated the unified-exec status restoration snapshot to match inline rendering order. - Validated with: - `just fmt` - `cargo test -p codex-tui --lib` --------- Co-authored-by: Sayan Sisodiya <sayan@openai.com>
This commit is contained in:
@@ -158,7 +158,10 @@ pub(crate) struct BottomPane {
|
||||
|
||||
/// Inline status indicator shown above the composer while a task is running.
|
||||
status: Option<StatusIndicatorWidget>,
|
||||
/// Unified exec session summary shown above the composer.
|
||||
/// Unified exec session summary source.
|
||||
///
|
||||
/// When a status row exists, this summary is mirrored inline in that row;
|
||||
/// when no status row exists, it renders as its own footer row.
|
||||
unified_exec_footer: UnifiedExecFooter,
|
||||
/// Queued user messages to show above the composer while a turn is running.
|
||||
queued_user_messages: QueuedUserMessages,
|
||||
@@ -606,6 +609,7 @@ impl BottomPane {
|
||||
if let Some(status) = self.status.as_mut() {
|
||||
status.set_interrupt_hint_visible(true);
|
||||
}
|
||||
self.sync_status_inline_message();
|
||||
self.request_redraw();
|
||||
}
|
||||
} else {
|
||||
@@ -628,6 +632,7 @@ impl BottomPane {
|
||||
self.frame_requester.clone(),
|
||||
self.animations_enabled,
|
||||
));
|
||||
self.sync_status_inline_message();
|
||||
self.request_redraw();
|
||||
}
|
||||
}
|
||||
@@ -684,12 +689,27 @@ impl BottomPane {
|
||||
self.request_redraw();
|
||||
}
|
||||
|
||||
/// Update the unified-exec process set and refresh whichever summary surface is active.
|
||||
///
|
||||
/// The summary may be displayed inline in the status row or as a dedicated
|
||||
/// footer row depending on whether a status indicator is currently visible.
|
||||
pub(crate) fn set_unified_exec_processes(&mut self, processes: Vec<String>) {
|
||||
if self.unified_exec_footer.set_processes(processes) {
|
||||
self.sync_status_inline_message();
|
||||
self.request_redraw();
|
||||
}
|
||||
}
|
||||
|
||||
/// Copy unified-exec summary text into the active status row, if any.
|
||||
///
|
||||
/// This keeps status-line inline text synchronized without forcing the
|
||||
/// standalone unified-exec footer row to be visible.
|
||||
fn sync_status_inline_message(&mut self) {
|
||||
if let Some(status) = self.status.as_mut() {
|
||||
status.update_inline_message(self.unified_exec_footer.summary_text());
|
||||
}
|
||||
}
|
||||
|
||||
/// Update custom prompts available for the slash popup.
|
||||
pub(crate) fn set_custom_prompts(&mut self, prompts: Vec<CustomPrompt>) {
|
||||
self.composer.set_custom_prompts(prompts);
|
||||
@@ -884,7 +904,9 @@ impl BottomPane {
|
||||
if let Some(status) = &self.status {
|
||||
flex.push(0, RenderableItem::Borrowed(status));
|
||||
}
|
||||
if !self.unified_exec_footer.is_empty() {
|
||||
// Avoid double-surfacing the same summary and avoid adding an extra
|
||||
// row while the status line is already visible.
|
||||
if self.status.is_none() && !self.unified_exec_footer.is_empty() {
|
||||
flex.push(0, RenderableItem::Borrowed(&self.unified_exec_footer));
|
||||
}
|
||||
let has_queued_messages = !self.queued_user_messages.messages.is_empty();
|
||||
@@ -1177,6 +1199,35 @@ mod tests {
|
||||
assert_snapshot!("status_only_snapshot", render_snapshot(&pane, area));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unified_exec_summary_does_not_increase_height_when_status_visible() {
|
||||
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
|
||||
let tx = AppEventSender::new(tx_raw);
|
||||
let mut pane = BottomPane::new(BottomPaneParams {
|
||||
app_event_tx: tx,
|
||||
frame_requester: FrameRequester::test_dummy(),
|
||||
has_input_focus: true,
|
||||
enhanced_keys_supported: false,
|
||||
placeholder_text: "Ask Codex to do anything".to_string(),
|
||||
disable_paste_burst: false,
|
||||
animations_enabled: true,
|
||||
skills: Some(Vec::new()),
|
||||
});
|
||||
|
||||
pane.set_task_running(true);
|
||||
let width = 120;
|
||||
let before = pane.desired_height(width);
|
||||
|
||||
pane.set_unified_exec_processes(vec!["sleep 5".to_string()]);
|
||||
let after = pane.desired_height(width);
|
||||
|
||||
assert_eq!(after, before);
|
||||
|
||||
let area = Rect::new(0, 0, width, after);
|
||||
let rendered = render_snapshot(&pane, area);
|
||||
assert!(rendered.contains("background terminal running · /ps to view"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn status_with_details_and_queued_messages_snapshot() {
|
||||
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
|
||||
|
||||
Reference in New Issue
Block a user