agent: Refresh UI when context or thread history changes (#28188)

I found a few more cases where the UI wasn't updated immediately after
an interaction.

Release Notes:

- agent: Fixed delay after removing threads from "Past Interactions"
- agent: Fixed delay after adding/remove context via keyboard
This commit is contained in:
Agus Zubiaga 2025-04-06 09:35:15 -05:00 committed by GitHub
parent b1f7133a7b
commit 57669b4908
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 95 additions and 38 deletions

View file

@ -253,6 +253,8 @@ impl AssistantPanel {
let history_store =
cx.new(|cx| HistoryStore::new(thread_store.clone(), context_store.clone(), cx));
cx.observe(&history_store, |_, _, cx| cx.notify()).detach();
let active_view = ActiveView::thread(thread.clone(), window, cx);
let thread_subscription = cx.subscribe(&thread, |_, _, event, cx| {
if let ThreadEvent::MessageAdded(_) = &event {

View file

@ -13,7 +13,8 @@ use editor::display_map::{Crease, FoldId};
use editor::{Anchor, AnchorRangeExt as _, Editor, ExcerptId, FoldPlaceholder, ToOffset};
use file_context_picker::render_file_context_entry;
use gpui::{
App, DismissEvent, Empty, Entity, EventEmitter, FocusHandle, Focusable, Task, WeakEntity,
App, DismissEvent, Empty, Entity, EventEmitter, FocusHandle, Focusable, Subscription, Task,
WeakEntity,
};
use multi_buffer::MultiBufferRow;
use project::{Entry, ProjectPath};
@ -105,6 +106,7 @@ pub(super) struct ContextPicker {
context_store: WeakEntity<ContextStore>,
thread_store: Option<WeakEntity<ThreadStore>>,
confirm_behavior: ConfirmBehavior,
_subscriptions: Vec<Subscription>,
}
impl ContextPicker {
@ -116,6 +118,22 @@ impl ContextPicker {
window: &mut Window,
cx: &mut Context<Self>,
) -> Self {
let subscriptions = context_store
.upgrade()
.map(|context_store| {
cx.observe(&context_store, |this, _, cx| this.notify_current_picker(cx))
})
.into_iter()
.chain(
thread_store
.as_ref()
.and_then(|thread_store| thread_store.upgrade())
.map(|thread_store| {
cx.observe(&thread_store, |this, _, cx| this.notify_current_picker(cx))
}),
)
.collect::<Vec<Subscription>>();
ContextPicker {
mode: ContextPickerState::Default(ContextMenu::build(
window,
@ -126,6 +144,7 @@ impl ContextPicker {
context_store,
thread_store,
confirm_behavior,
_subscriptions: subscriptions,
}
}
@ -370,6 +389,16 @@ impl ContextPicker {
recent_context_picker_entries(context_store, self.thread_store.clone(), workspace, cx)
}
fn notify_current_picker(&mut self, cx: &mut Context<Self>) {
match &self.mode {
ContextPickerState::Default(entity) => entity.update(cx, |_, cx| cx.notify()),
ContextPickerState::File(entity) => entity.update(cx, |_, cx| cx.notify()),
ContextPickerState::Symbol(entity) => entity.update(cx, |_, cx| cx.notify()),
ContextPickerState::Fetch(entity) => entity.update(cx, |_, cx| cx.notify()),
ContextPickerState::Thread(entity) => entity.update(cx, |_, cx| cx.notify()),
}
}
}
impl EventEmitter<DismissEvent> for ContextPicker {}

View file

@ -232,8 +232,8 @@ impl ContextPickerCompletionProvider {
url_to_fetch.to_string(),
))
.await?;
context_store.update(cx, |context_store, _| {
context_store.add_fetched_url(url_to_fetch.to_string(), content)
context_store.update(cx, |context_store, cx| {
context_store.add_fetched_url(url_to_fetch.to_string(), content, cx)
})
})
.detach_and_log_err(cx);

View file

@ -213,8 +213,8 @@ impl PickerDelegate for FetchContextPickerDelegate {
this.update_in(cx, |this, window, cx| {
this.delegate
.context_store
.update(cx, |context_store, _cx| {
context_store.add_fetched_url(url, text);
.update(cx, |context_store, cx| {
context_store.add_fetched_url(url, text, cx)
})?;
match confirm_behavior {

View file

@ -98,11 +98,11 @@ impl ContextStore {
let buffer = open_buffer_task.await?;
let buffer_id = this.update(cx, |_, cx| buffer.read(cx).remote_id())?;
let already_included = this.update(cx, |this, _cx| {
let already_included = this.update(cx, |this, cx| {
match this.will_include_buffer(buffer_id, &project_path.path) {
Some(FileInclusion::Direct(context_id)) => {
if remove_if_exists {
this.remove_context(context_id);
this.remove_context(context_id, cx);
}
true
}
@ -120,8 +120,8 @@ impl ContextStore {
let text = text_task.await;
this.update(cx, |this, _cx| {
this.insert_file(make_context_buffer(buffer_info, text));
this.update(cx, |this, cx| {
this.insert_file(make_context_buffer(buffer_info, text), cx);
})?;
anyhow::Ok(())
@ -139,19 +139,20 @@ impl ContextStore {
let text = text_task.await;
this.update(cx, |this, _cx| {
this.insert_file(make_context_buffer(buffer_info, text))
this.update(cx, |this, cx| {
this.insert_file(make_context_buffer(buffer_info, text), cx)
})?;
anyhow::Ok(())
})
}
fn insert_file(&mut self, context_buffer: ContextBuffer) {
fn insert_file(&mut self, context_buffer: ContextBuffer, cx: &mut Context<Self>) {
let id = self.next_context_id.post_inc();
self.files.insert(context_buffer.id, id);
self.context
.push(AssistantContext::File(FileContext { id, context_buffer }));
cx.notify();
}
pub fn add_directory(
@ -171,7 +172,7 @@ impl ContextStore {
let already_included = match self.includes_directory(&project_path.path) {
Some(FileInclusion::Direct(context_id)) => {
if remove_if_exists {
self.remove_context(context_id);
self.remove_context(context_id, cx);
}
true
}
@ -238,15 +239,20 @@ impl ContextStore {
));
}
this.update(cx, |this, _| {
this.insert_directory(project_path, context_buffers);
this.update(cx, |this, cx| {
this.insert_directory(project_path, context_buffers, cx);
})?;
anyhow::Ok(())
})
}
fn insert_directory(&mut self, project_path: ProjectPath, context_buffers: Vec<ContextBuffer>) {
fn insert_directory(
&mut self,
project_path: ProjectPath,
context_buffers: Vec<ContextBuffer>,
cx: &mut Context<Self>,
) {
let id = self.next_context_id.post_inc();
self.directories.insert(project_path.path.to_path_buf(), id);
@ -256,6 +262,7 @@ impl ContextStore {
project_path,
context_buffers,
}));
cx.notify();
}
pub fn add_symbol(
@ -286,7 +293,7 @@ impl ContextStore {
if let Some(id) = matching_symbol_id {
if remove_if_exists {
self.remove_context(id);
self.remove_context(id, cx);
}
return Task::ready(Ok(false));
}
@ -301,21 +308,24 @@ impl ContextStore {
cx.spawn(async move |this, cx| {
let content = collect_content_task.await;
this.update(cx, |this, _cx| {
this.insert_symbol(make_context_symbol(
buffer_info,
project_path,
symbol_name,
symbol_range,
symbol_enclosing_range,
content,
))
this.update(cx, |this, cx| {
this.insert_symbol(
make_context_symbol(
buffer_info,
project_path,
symbol_name,
symbol_range,
symbol_enclosing_range,
content,
),
cx,
)
})?;
anyhow::Ok(true)
})
}
fn insert_symbol(&mut self, context_symbol: ContextSymbol) {
fn insert_symbol(&mut self, context_symbol: ContextSymbol, cx: &mut Context<Self>) {
let id = self.next_context_id.post_inc();
self.symbols.insert(context_symbol.id.clone(), id);
self.symbols_by_path
@ -328,6 +338,7 @@ impl ContextStore {
id,
context_symbol,
}));
cx.notify();
}
pub fn add_thread(
@ -338,7 +349,7 @@ impl ContextStore {
) {
if let Some(context_id) = self.includes_thread(&thread.read(cx).id()) {
if remove_if_exists {
self.remove_context(context_id);
self.remove_context(context_id, cx);
}
} else {
self.insert_thread(thread, cx);
@ -353,14 +364,14 @@ impl ContextStore {
})
}
fn insert_thread(&mut self, thread: Entity<Thread>, cx: &mut App) {
fn insert_thread(&mut self, thread: Entity<Thread>, cx: &mut Context<Self>) {
if let Some(summary_task) =
thread.update(cx, |thread, cx| thread.generate_detailed_summary(cx))
{
let thread = thread.clone();
let thread_store = self.thread_store.clone();
self.thread_summary_tasks.push(cx.spawn(async move |cx| {
self.thread_summary_tasks.push(cx.spawn(async move |_, cx| {
summary_task.await;
if let Some(thread_store) = thread_store {
@ -382,15 +393,26 @@ impl ContextStore {
self.threads.insert(thread.read(cx).id().clone(), id);
self.context
.push(AssistantContext::Thread(ThreadContext { id, thread, text }));
cx.notify();
}
pub fn add_fetched_url(&mut self, url: String, text: impl Into<SharedString>) {
pub fn add_fetched_url(
&mut self,
url: String,
text: impl Into<SharedString>,
cx: &mut Context<ContextStore>,
) {
if self.includes_url(&url).is_none() {
self.insert_fetched_url(url, text);
self.insert_fetched_url(url, text, cx);
}
}
fn insert_fetched_url(&mut self, url: String, text: impl Into<SharedString>) {
fn insert_fetched_url(
&mut self,
url: String,
text: impl Into<SharedString>,
cx: &mut Context<ContextStore>,
) {
let id = self.next_context_id.post_inc();
self.fetched_urls.insert(url.clone(), id);
@ -400,6 +422,7 @@ impl ContextStore {
url: url.into(),
text: text.into(),
}));
cx.notify();
}
pub fn accept_suggested_context(
@ -426,7 +449,7 @@ impl ContextStore {
Task::ready(Ok(()))
}
pub fn remove_context(&mut self, id: ContextId) {
pub fn remove_context(&mut self, id: ContextId, cx: &mut Context<Self>) {
let Some(ix) = self.context.iter().position(|context| context.id() == id) else {
return;
};
@ -458,6 +481,8 @@ impl ContextStore {
self.threads.retain(|_, context_id| *context_id != id);
}
}
cx.notify();
}
/// Returns whether the buffer is already included directly in the context, or if it will be

View file

@ -59,6 +59,7 @@ impl ContextStrip {
let focus_handle = cx.focus_handle();
let subscriptions = vec![
cx.observe(&context_store, |_, _, cx| cx.notify()),
cx.subscribe_in(&context_picker, window, Self::handle_context_picker_event),
cx.on_focus(&focus_handle, window, Self::handle_focus),
cx.on_blur(&focus_handle, window, Self::handle_blur),
@ -290,9 +291,9 @@ impl ContextStrip {
if let Some(index) = self.focused_index {
let mut is_empty = false;
self.context_store.update(cx, |this, _cx| {
self.context_store.update(cx, |this, cx| {
if let Some(item) = this.context().get(index) {
this.remove_context(item.id());
this.remove_context(item.id(), cx);
}
is_empty = this.context().is_empty();
@ -475,8 +476,8 @@ impl Render for ContextStrip {
Some({
let context_store = self.context_store.clone();
Rc::new(cx.listener(move |_this, _event, _window, cx| {
context_store.update(cx, |this, _cx| {
this.remove_context(id);
context_store.update(cx, |this, cx| {
this.remove_context(id, cx);
});
cx.notify();
}))