From 34f51c1b0d44e8611dbed12485446c127ee48618 Mon Sep 17 00:00:00 2001 From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com> Date: Thu, 2 Apr 2026 15:15:09 -0300 Subject: [PATCH] agent_ui: Remeasure changed entries in the thread view list (#53017) This PR fixes an issue where, sometimes, you couldn't scroll all the way to the bottom of the thread's content. The scrollbar would show up at the bottom of the scrollable container but the content was visibly cut off. Turns out that's a consequence of the top-down thread generation introduced in https://github.com/zed-industries/zed/pull/52440, where changing the list alignment to `Top` made it visible that sometimes, the maximum scroll area would get underestimated because the items in the thread view's list would have a stale height measurement. So, the way this PR fixes the issue is by calling `splice_focusable` in the `EntryUpdated` event, too, so that the height of the items in the overdraw area get marked as unmeasured, triggering a list re-render and re-measuring. We started by writing a test at the list level that would reproduce the regression but then later figured out that this is not an inherent list problem; it was rather a problem with its use within the thread view layer. Then, we explored writing a test that documented the regression, but it turned out to be very hard to simulate this sort of set up in which certain elements would have its height changed during streaming, which would be how you'd get to a mismatched height situation. Therefore, given `AcpThreadEvent::NewEntry` already called `splice_focusable` and don't have a test for it, we figure it'd be safe to move forward without one, too. We then introduced a helper that's now shared between `AcpThreadEvent::NewEntry` and `AcpThreadEvent::EntryUpdated`. Release Notes: - Agent: Fixed an issue where sometimes you couldn't scroll all the way to the bottom of the thread even though there's visibly more content below the fold. Co-authored-by: Eric Holk --- crates/agent_ui/src/conversation_view.rs | 57 +++++++++++++++++++----- 1 file changed, 45 insertions(+), 12 deletions(-) diff --git a/crates/agent_ui/src/conversation_view.rs b/crates/agent_ui/src/conversation_view.rs index 83a0c158a11c54be1ff54f553ce4b427da2cabc2..924f59437e51b02217289a5570f9560948c23ca2 100644 --- a/crates/agent_ui/src/conversation_view.rs +++ b/crates/agent_ui/src/conversation_view.rs @@ -1240,15 +1240,15 @@ impl ConversationView { if let Some(active) = self.thread_view(&thread_id) { let entry_view_state = active.read(cx).entry_view_state.clone(); let list_state = active.read(cx).list_state.clone(); - entry_view_state.update(cx, |view_state, cx| { - view_state.sync_entry(index, thread, window, cx); - list_state.splice_focusable( - index..index, - [view_state - .entry(index) - .and_then(|entry| entry.focus_handle(cx))], - ); - }); + notify_entry_changed( + &entry_view_state, + &list_state, + index..index, + index, + thread, + window, + cx, + ); active.update(cx, |active, cx| { active.sync_editor_mode_for_empty_state(cx); }); @@ -1257,9 +1257,16 @@ impl ConversationView { AcpThreadEvent::EntryUpdated(index) => { if let Some(active) = self.thread_view(&thread_id) { let entry_view_state = active.read(cx).entry_view_state.clone(); - entry_view_state.update(cx, |view_state, cx| { - view_state.sync_entry(*index, thread, window, cx) - }); + let list_state = active.read(cx).list_state.clone(); + notify_entry_changed( + &entry_view_state, + &list_state, + *index..*index + 1, + *index, + thread, + window, + cx, + ); active.update(cx, |active, cx| { active.auto_expand_streaming_thought(cx); }); @@ -2598,6 +2605,32 @@ impl ConversationView { } } +/// Syncs an entry's view state with the latest thread data and splices +/// the list item so the list knows to re-measure it on the next paint. +/// +/// Used by both `NewEntry` (splice range `index..index` to insert) and +/// `EntryUpdated` (splice range `index..index+1` to replace), which is +/// why the caller provides the splice range. +fn notify_entry_changed( + entry_view_state: &Entity, + list_state: &ListState, + splice_range: std::ops::Range, + index: usize, + thread: &Entity, + window: &mut Window, + cx: &mut App, +) { + entry_view_state.update(cx, |view_state, cx| { + view_state.sync_entry(index, thread, window, cx); + list_state.splice_focusable( + splice_range, + [view_state + .entry(index) + .and_then(|entry| entry.focus_handle(cx))], + ); + }); +} + fn loading_contents_spinner(size: IconSize) -> AnyElement { Icon::new(IconName::LoadCircle) .size(size)