Skip to content

Commit fa7eb42

Browse files
harryalbertoz-agent
andcommitted
TUI: remeasure running agent commands on row growth
Co-Authored-By: Oz <oz-agent@warp.dev>
1 parent c7ed842 commit fa7eb42

5 files changed

Lines changed: 180 additions & 67 deletions

File tree

crates/warp_tui/src/agent_block.rs

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1126,16 +1126,13 @@ impl TuiAIBlock {
11261126
/// Whether the cached height is stale at `width`.
11271127
pub(super) fn needs_height_measurement(&self, width: u16, app: &AppContext) -> bool {
11281128
self.last_measured_width.get() != Some(width)
1129-
|| self.block_model.status(app).is_streaming()
11301129
|| self.action_views.values().any(|view| match view {
11311130
TuiToolCallView::AskQuestion(_)
11321131
| TuiToolCallView::FileEdits(_)
11331132
| TuiToolCallView::Generic(_)
11341133
| TuiToolCallView::Plan(_)
11351134
| TuiToolCallView::OrchestrationBlock(_) => false,
1136-
TuiToolCallView::ShellCommand(view) => {
1137-
view.as_ref(app).needs_continuous_height_measurement()
1138-
}
1135+
TuiToolCallView::ShellCommand(view) => view.as_ref(app).needs_height_measurement(),
11391136
})
11401137
}
11411138

@@ -1178,16 +1175,35 @@ impl TuiAIBlock {
11781175
ctx: &mut TuiLayoutContext,
11791176
app: &AppContext,
11801177
) -> usize {
1178+
let command_extents = self
1179+
.action_views
1180+
.values()
1181+
.filter_map(|view| match view {
1182+
TuiToolCallView::ShellCommand(view) => {
1183+
Some((view, view.as_ref(app).dynamic_content_extent()))
1184+
}
1185+
TuiToolCallView::AskQuestion(_)
1186+
| TuiToolCallView::FileEdits(_)
1187+
| TuiToolCallView::Generic(_)
1188+
| TuiToolCallView::Plan(_)
1189+
| TuiToolCallView::OrchestrationBlock(_) => None,
1190+
})
1191+
.collect::<Vec<_>>();
11811192
let mut element = self.render_element(app);
1182-
usize::from(
1193+
let height = usize::from(
11831194
element
11841195
.layout(
11851196
TuiConstraint::loose(TuiSize::new(width, u16::MAX)),
11861197
ctx,
11871198
app,
11881199
)
11891200
.height,
1190-
)
1201+
);
1202+
for (view, command_extent) in command_extents {
1203+
view.as_ref(app)
1204+
.record_content_extent_measurement(command_extent);
1205+
}
1206+
height
11911207
}
11921208

11931209
/// Logical (unwrapped) text for a selection over this block's text

crates/warp_tui/src/tui_block_list_viewport_source.rs

Lines changed: 56 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -103,64 +103,75 @@ impl TuiBlockListViewportSource {
103103
///
104104
/// A non-dirty band block is re-measured only when its cached height cannot
105105
/// be trusted: its last measurement was at a different width (reflow), it
106-
/// has never been measured (no recorded width), or it is still streaming
107-
/// (its height can grow without a per-update invalidation — e.g. an
108-
/// expanded, still-running shell command). At a stable width with no
109-
/// dynamic height, nothing extra is measured and the cached
110-
/// `last_laid_out_height` is reused. Off-band blocks keep their cached
111-
/// height until they scroll into the band.
106+
/// has never been measured (no recorded width), or an expanded running
107+
/// shell command's terminal row extent changed. Agent output updates dirty
108+
/// their block directly, so streaming status alone does not require layout
109+
/// polling. At a stable width with no dynamic height change, the cached
110+
/// height is reused. Off-band blocks keep their cached height until they
111+
/// scroll into the band.
112112
fn agent_heights_to_measure(
113113
&self,
114114
window: TuiViewportWindow,
115115
available_width: u16,
116116
app: &AppContext,
117117
) -> HashSet<EntityId> {
118-
let mut model = self.model.lock();
119-
let mut view_ids = model.block_list_mut().take_dirty_rich_content_items();
118+
let mut view_ids = self
119+
.model
120+
.lock()
121+
.block_list_mut()
122+
.take_dirty_rich_content_items();
123+
let band_view_ids = {
124+
let model = self.model.lock();
125+
let block_list = model.block_list();
126+
let band_top = window.scroll_top.saturating_sub(OVERHANG_ROWS);
127+
let band_bottom = window
128+
.scroll_top
129+
.saturating_add(usize::from(window.viewport_height))
130+
.saturating_add(OVERHANG_ROWS);
131+
let mut cursor = block_list
132+
.block_heights()
133+
.cursor::<BlockHeight, BlockHeightSummary>();
134+
cursor.seek_clamped(&BlockHeight::from(band_top as f64), SeekBias::Left);
135+
let mut band_view_ids = Vec::new();
136+
while let Some(item) = cursor.item() {
137+
let item_top = cursor.start().height.as_f64().floor().max(0.0) as usize;
138+
if item_top >= band_bottom {
139+
break;
140+
}
141+
let item_bottom = item_top.saturating_add(item.height().as_f64().ceil() as usize);
142+
if item_bottom > band_top
143+
&& let BlockHeightItem::RichContent(rich_content) = item
144+
&& !rich_content.should_hide
145+
{
146+
band_view_ids.push(rich_content.view_id);
147+
}
148+
cursor.next();
149+
}
150+
band_view_ids
151+
};
120152

121153
let agent_blocks = self.agent_blocks.borrow();
122154
let cli_subagent_blocks = self.cli_subagent_blocks.borrow();
123155
let handoff_blocks = self.handoff_blocks.borrow();
124-
let block_list = model.block_list();
125-
let band_top = window.scroll_top.saturating_sub(OVERHANG_ROWS);
126-
let band_bottom = window
127-
.scroll_top
128-
.saturating_add(usize::from(window.viewport_height))
129-
.saturating_add(OVERHANG_ROWS);
130-
let mut cursor = block_list
131-
.block_heights()
132-
.cursor::<BlockHeight, BlockHeightSummary>();
133-
cursor.seek_clamped(&BlockHeight::from(band_top as f64), SeekBias::Left);
134-
while let Some(item) = cursor.item() {
135-
let item_top = cursor.start().height.as_f64().floor().max(0.0) as usize;
136-
if item_top >= band_bottom {
137-
break;
138-
}
139-
let item_bottom = item_top.saturating_add(item.height().as_f64().ceil() as usize);
140-
if item_bottom > band_top
141-
&& let BlockHeightItem::RichContent(rich_content) = item
142-
&& !rich_content.should_hide
143-
{
144-
if let Some(view) = agent_blocks.get(&rich_content.view_id) {
145-
if view
146-
.as_ref(app)
147-
.needs_height_measurement(available_width, app)
148-
{
149-
view_ids.insert(rich_content.view_id);
150-
}
151-
} else if let Some(view) = cli_subagent_blocks.get(&rich_content.view_id)
152-
&& view
153-
.as_ref(app)
154-
.needs_height_measurement(available_width, app)
156+
for view_id in band_view_ids {
157+
if let Some(view) = agent_blocks.get(&view_id) {
158+
if view
159+
.as_ref(app)
160+
.needs_height_measurement(available_width, app)
155161
{
156-
view_ids.insert(rich_content.view_id);
157-
} else if let Some(view) = handoff_blocks.get(&rich_content.view_id)
158-
&& view.as_ref(app).needs_height_measurement(available_width)
159-
{
160-
view_ids.insert(rich_content.view_id);
162+
view_ids.insert(view_id);
161163
}
164+
} else if let Some(view) = cli_subagent_blocks.get(&view_id)
165+
&& view
166+
.as_ref(app)
167+
.needs_height_measurement(available_width, app)
168+
{
169+
view_ids.insert(view_id);
170+
} else if let Some(view) = handoff_blocks.get(&view_id)
171+
&& view.as_ref(app).needs_height_measurement(available_width)
172+
{
173+
view_ids.insert(view_id);
162174
}
163-
cursor.next();
164175
}
165176
view_ids
166177
}

crates/warp_tui/src/tui_block_list_viewport_source_tests.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -363,21 +363,21 @@ fn tui_transcript_scroll_reuses_cached_heights_at_stable_width() {
363363
}
364364

365365
#[test]
366-
fn tui_agent_streaming_block_remeasured_at_stable_width() {
366+
fn tui_agent_streaming_block_reuses_cached_height_without_an_update() {
367367
App::test((), |mut app| async move {
368368
app.add_singleton_model(|_| Appearance::mock());
369-
// A streaming block's height can grow without a per-update
370-
// invalidation, so it must be re-measured at a stable width.
369+
// Agent output updates explicitly dirty their rich-content item, so
370+
// streaming status alone does not require polling its full layout.
371371
let (source, model, agent_block) = streaming_agent_block_source(&mut app);
372372

373373
request_top_window(&app, &source, 10);
374374
source.take_selection_row_resizes();
375375

376-
// Seed a wrong height at the same width without dirtying; the streaming
377-
// block is still re-measured, correcting it.
376+
// Seed a wrong height at the same width without dirtying. With no
377+
// output update, the cached height remains untouched.
378378
seed_clean_height(&app, &model, &agent_block, 1234.0, 80);
379379
request_top_window(&app, &source, 10);
380-
assert_ne!(rich_content_height(&model, agent_block.id()), Some(1234.0));
380+
assert_eq!(rich_content_height(&model, agent_block.id()), Some(1234.0));
381381
});
382382
}
383383

crates/warp_tui/src/tui_shell_command_view.rs

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ use warpui_core::{
2929
use crate::agent_block_sections::render_fallback_tool_call_section;
3030
use crate::editor_view::{TuiEditorView, TuiEditorViewEvent};
3131
use crate::keybindings::{TUI_BINDING_GROUP, is_tui_owned_binding};
32-
use crate::terminal_block::TerminalBlockElement;
32+
use crate::terminal_block::{TerminalBlockElement, block_content_rows};
3333
use crate::terminal_use::user_controls_running_command;
3434
use crate::tool_call_labels::{
3535
CommandBlockState, ResolvedCommandBlock, styled_tool_call_label_spans, tool_call_display_state,
@@ -75,6 +75,12 @@ pub(crate) fn init(app: &mut AppContext) {
7575
]);
7676
app.register_tui_binding_validator::<TuiShellCommandView>(is_tui_owned_binding);
7777
}
78+
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
79+
pub(super) struct ShellCommandContentExtent {
80+
rendered_height: usize,
81+
command_rows: usize,
82+
output_rows: usize,
83+
}
7884

7985
struct ShellCommandViewState {
8086
collapsed: bool,
@@ -120,7 +126,7 @@ pub(super) struct TuiShellCommandView {
120126
permission_prompt: ViewHandle<TuiPermissionPrompt>,
121127
command_was_edited: bool,
122128
state: ShellCommandViewState,
123-
command_running: Cell<bool>,
129+
last_measured_content_extent: Cell<Option<ShellCommandContentExtent>>,
124130
header_mouse_state: MouseStateHandle,
125131
cli_subagent_view: Option<ViewHandle<TuiCLISubagentView>>,
126132
}
@@ -196,7 +202,7 @@ impl TuiShellCommandView {
196202
permission_prompt,
197203
command_was_edited: false,
198204
state: ShellCommandViewState::new_collapsed(),
199-
command_running: Cell::new(false),
205+
last_measured_content_extent: Cell::new(None),
200206
header_mouse_state: MouseStateHandle::default(),
201207
cli_subagent_view: None,
202208
}
@@ -343,9 +349,31 @@ impl TuiShellCommandView {
343349
}
344350
});
345351
}
346-
/// Whether expanded command output can still grow between layout events.
347-
pub(super) fn needs_continuous_height_measurement(&self) -> bool {
348-
!self.state.is_collapsed() && self.command_running.get()
352+
/// Returns the live row extent when command output can change this view's height.
353+
pub(super) fn dynamic_content_extent(&self) -> Option<ShellCommandContentExtent> {
354+
let model = self.terminal_model.lock();
355+
let block = model.block_list().block_for_ai_action_id(&self.action.id)?;
356+
let expanded = !self.state.is_collapsed() || user_controls_running_command(block);
357+
(expanded && !block.finished()).then(|| ShellCommandContentExtent {
358+
rendered_height: block_content_rows(block).len(),
359+
command_rows: usize::from(!block.should_hide_command_grid())
360+
.saturating_mul(block.prompt_and_command_grid().len_displayed()),
361+
output_rows: usize::from(!block.should_hide_output_grid())
362+
.saturating_mul(block.output_grid().len_displayed()),
363+
})
364+
}
365+
/// Whether running command output has changed height since the last layout.
366+
pub(super) fn needs_height_measurement(&self) -> bool {
367+
self.dynamic_content_extent()
368+
.is_some_and(|extent| self.last_measured_content_extent.get() != Some(extent))
369+
}
370+
371+
/// Records the row extent represented by the latest agent-block layout.
372+
pub(super) fn record_content_extent_measurement(
373+
&self,
374+
extent: Option<ShellCommandContentExtent>,
375+
) {
376+
self.last_measured_content_extent.set(extent);
349377
}
350378
pub(super) fn is_expanded(&self) -> bool {
351379
!self.state.is_collapsed()
@@ -437,11 +465,9 @@ impl TuiView for TuiShellCommandView {
437465
.as_ref(app)
438466
.get_action_status(&self.action.id);
439467
if matches!(status, Some(AIActionStatus::Blocked)) {
440-
self.command_running.set(false);
441468
return self.render_blocked(app);
442469
}
443470
let Some(block) = self.resolved_block(status.as_ref()) else {
444-
self.command_running.set(false);
445471
return render_fallback_tool_call_section(
446472
&self.action,
447473
status.as_ref(),
@@ -450,8 +476,6 @@ impl TuiView for TuiShellCommandView {
450476
app,
451477
);
452478
};
453-
self.command_running
454-
.set(matches!(block.details.state, CommandBlockState::Running));
455479

456480
let builder = TuiUiBuilder::from_app(app);
457481
let display_state =

crates/warp_tui/src/tui_shell_command_view_tests.rs

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,50 @@ fn terminal_block_is_collapsed_by_default_and_expands_inline() {
371371
});
372372
}
373373

374+
#[test]
375+
fn running_expanded_command_remeasures_only_when_row_extent_changes() {
376+
App::test((), |mut app| async move {
377+
let action = command_action("action-1", "printf rows");
378+
let terminal_model = terminal_model_with_running_command(&action, "printf rows", "partial");
379+
let view = add_shell_view(&mut app, action, terminal_model.clone());
380+
view.update(&mut app, |view, ctx| {
381+
view.handle_action(&TuiShellCommandViewAction::ToggleExpanded, ctx);
382+
});
383+
384+
let initial_extent = app.read(|app| {
385+
let view = view.as_ref(app);
386+
let extent = view
387+
.dynamic_content_extent()
388+
.expect("expanded running command has dynamic content");
389+
assert!(view.needs_height_measurement());
390+
view.record_content_extent_measurement(Some(extent));
391+
assert!(!view.needs_height_measurement());
392+
extent
393+
});
394+
395+
terminal_model.lock().process_bytes("\rshort");
396+
app.read(|app| {
397+
assert!(!view.as_ref(app).needs_height_measurement());
398+
});
399+
400+
let added_rows = "next row\r\n".repeat(100);
401+
terminal_model.lock().process_bytes(added_rows.as_str());
402+
app.read(|app| {
403+
let view = view.as_ref(app);
404+
let current_extent = view
405+
.dynamic_content_extent()
406+
.expect("command remains active after receiving output");
407+
assert_ne!(current_extent, initial_extent);
408+
assert!(view.needs_height_measurement());
409+
});
410+
411+
terminal_model.lock().finish_block();
412+
app.read(|app| {
413+
assert!(!view.as_ref(app).needs_height_measurement());
414+
});
415+
});
416+
}
417+
374418
#[test]
375419
fn long_path_command_wraps_in_full_with_the_chevron_on_the_first_row() {
376420
App::test((), |mut app| async move {
@@ -635,6 +679,24 @@ fn terminal_model_with_command(
635679
Arc::new(FairMutex::new(model))
636680
}
637681

682+
fn terminal_model_with_running_command(
683+
action: &AIAgentAction,
684+
command: &str,
685+
output: &str,
686+
) -> Arc<FairMutex<TerminalModel>> {
687+
let mut model = TerminalModel::mock(None, None);
688+
model.simulate_long_running_block(command, output);
689+
model
690+
.block_list_mut()
691+
.active_block_mut()
692+
.set_agent_interaction_mode_for_requested_command(
693+
action.id.clone(),
694+
None,
695+
AIConversationId::new(),
696+
);
697+
Arc::new(FairMutex::new(model))
698+
}
699+
638700
fn command_action(id: &str, command: &str) -> AIAgentAction {
639701
AIAgentAction {
640702
id: AIAgentActionId::from(id.to_owned()),

0 commit comments

Comments
 (0)