agent_ui: Improve auto-scroll when editing previous messages (#52876)

Danilo Leal created 1 week ago

This PR improves the unreliability of the new top-down flow when editing
previous user messages. Previously, sometimes the thread wouldn't
auto-scroll as you're editing older messages and re-submitting them. It
turns out, the culprit seems to be a bit of a race condition where the
Stopped event (which sets the follow_tail method to false) could still
be emitted a while after a generation has been stopped. So depending on
time, one given possibility could happen. But now, this is what we do:

- If it is still generating, that means the stop/error event is stale
relative to a newer turn, so we do not run the cleanup that disables
follow-tail.
- If it is not generating, then it’s a real completion for the active
turn, and we do the normal cleanup.

--- 

Release Notes:

- N/A

Change summary

crates/agent_ui/src/conversation_view.rs | 81 ++++++++++++++++++++++++-
1 file changed, 76 insertions(+), 5 deletions(-)

Detailed changes

crates/agent_ui/src/conversation_view.rs

@@ -1295,10 +1295,17 @@ impl ConversationView {
             }
             AcpThreadEvent::Stopped(stop_reason) => {
                 if let Some(active) = self.thread_view(&thread_id) {
+                    let is_generating =
+                        matches!(thread.read(cx).status(), ThreadStatus::Generating);
                     active.update(cx, |active, cx| {
-                        active.thread_retry_status.take();
-                        active.clear_auto_expand_tracking();
-                        active.list_state.set_follow_tail(false);
+                        if !is_generating {
+                            active.thread_retry_status.take();
+                            active.clear_auto_expand_tracking();
+                            if active.list_state.is_following_tail() {
+                                active.list_state.scroll_to_end();
+                                active.list_state.set_follow_tail(false);
+                            }
+                        }
                         active.sync_generating_indicator(cx);
                     });
                 }
@@ -1367,9 +1374,16 @@ impl ConversationView {
             }
             AcpThreadEvent::Error => {
                 if let Some(active) = self.thread_view(&thread_id) {
+                    let is_generating =
+                        matches!(thread.read(cx).status(), ThreadStatus::Generating);
                     active.update(cx, |active, cx| {
-                        active.thread_retry_status.take();
-                        active.list_state.set_follow_tail(false);
+                        if !is_generating {
+                            active.thread_retry_status.take();
+                            if active.list_state.is_following_tail() {
+                                active.list_state.scroll_to_end();
+                                active.list_state.set_follow_tail(false);
+                            }
+                        }
                         active.sync_generating_indicator(cx);
                     });
                 }
@@ -4851,6 +4865,63 @@ pub(crate) mod tests {
         });
     }
 
+    #[gpui::test]
+    async fn test_stale_stop_does_not_disable_follow_tail_during_regenerate(
+        cx: &mut TestAppContext,
+    ) {
+        init_test(cx);
+
+        let connection = StubAgentConnection::new();
+
+        let (conversation_view, cx) =
+            setup_conversation_view(StubAgentServer::new(connection.clone()), cx).await;
+        add_to_workspace(conversation_view.clone(), cx);
+
+        let message_editor = message_editor(&conversation_view, cx);
+        message_editor.update_in(cx, |editor, window, cx| {
+            editor.set_text("Original message to edit", window, cx);
+        });
+        active_thread(&conversation_view, cx)
+            .update_in(cx, |view, window, cx| view.send(window, cx));
+
+        cx.run_until_parked();
+
+        let user_message_editor = conversation_view.read_with(cx, |view, cx| {
+            view.active_thread()
+                .map(|active| &active.read(cx).entry_view_state)
+                .as_ref()
+                .unwrap()
+                .read(cx)
+                .entry(0)
+                .unwrap()
+                .message_editor()
+                .unwrap()
+                .clone()
+        });
+
+        cx.focus(&user_message_editor);
+        user_message_editor.update_in(cx, |editor, window, cx| {
+            editor.set_text("Edited message content", window, cx);
+        });
+
+        user_message_editor.update_in(cx, |_editor, window, cx| {
+            window.dispatch_action(Box::new(Chat), cx);
+        });
+
+        cx.run_until_parked();
+
+        conversation_view.read_with(cx, |view, cx| {
+            let active = view.active_thread().unwrap();
+            let active = active.read(cx);
+
+            assert_eq!(active.thread.read(cx).status(), ThreadStatus::Generating);
+            assert!(
+                active.list_state.is_following_tail(),
+                "stale stop events from the cancelled turn must not disable follow-tail for the new turn"
+            );
+        });
+    }
+
     struct GeneratingThreadSetup {
         conversation_view: Entity<ConversationView>,
         thread: Entity<AcpThread>,