Avoid constantly scrolling thread history to top as agent generates (#54365)
Max Brunsfeld
created 1 week ago
Fixes two problems:
* We were not preserving scroll position in the thread history view when
the thread metadata store updated
* We were updating thread metadata on every single chunk streamed by the
agent, even though the content of the thread does not affect its
metadata.
Release Notes:
- N/A
Change summary
crates/agent_ui/src/conversation_view.rs | 8 ++++----
crates/agent_ui/src/threads_archive_view.rs | 10 +++-------
2 files changed, 7 insertions(+), 11 deletions(-)
Detailed changes
@@ -396,18 +396,18 @@ fn affects_thread_metadata(event: &AcpThreadEvent) -> bool {
match event {
AcpThreadEvent::NewEntry
| AcpThreadEvent::TitleUpdated
- | AcpThreadEvent::EntryUpdated(_)
- | AcpThreadEvent::EntriesRemoved(_)
| AcpThreadEvent::ToolAuthorizationRequested(_)
| AcpThreadEvent::ToolAuthorizationReceived(_)
- | AcpThreadEvent::Retry(_)
| AcpThreadEvent::Stopped(_)
| AcpThreadEvent::Error
| AcpThreadEvent::LoadError(_)
| AcpThreadEvent::Refusal
| AcpThreadEvent::WorkingDirectoriesUpdated => true,
// --
- AcpThreadEvent::TokenUsageUpdated
+ AcpThreadEvent::EntryUpdated(_)
+ | AcpThreadEvent::EntriesRemoved(_)
+ | AcpThreadEvent::Retry(_)
+ | AcpThreadEvent::TokenUsageUpdated
| AcpThreadEvent::PromptCapabilitiesUpdated
| AcpThreadEvent::AvailableCommandsUpdated(_)
| AcpThreadEvent::ModeUpdated(_)
@@ -320,11 +320,7 @@ impl ThreadsArchiveView {
let preserve = self.preserve_selection_on_next_update;
self.preserve_selection_on_next_update = false;
- let saved_scroll = if preserve {
- Some(self.list_state.logical_scroll_top())
- } else {
- None
- };
+ let saved_scroll = self.list_state.logical_scroll_top();
self.list_state.reset(items.len());
self.items = items;
@@ -337,9 +333,9 @@ impl ThreadsArchiveView {
}
}
- if let Some(scroll_top) = saved_scroll {
- self.list_state.scroll_to(scroll_top);
+ self.list_state.scroll_to(saved_scroll);
+ if preserve {
if let Some(ix) = self.selection {
let next = self.find_next_selectable(ix).or_else(|| {
ix.checked_sub(1)