agent: Add reactions at the response level (#27958)

Release Notes:

- Added the user reaction (👍 or 👎) to each agent response.
- 👎 will trigger a comment box linked to the response

---------

Co-authored-by: Danilo Leal <daniloleal09@gmail.com>
Co-authored-by: Agus Zubiaga <hi@aguz.me>
This commit is contained in:
Thomas Mickley-Doyle 2025-04-09 12:21:07 -05:00 committed by GitHub
parent b47aa33459
commit 342134fbab
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 173 additions and 68 deletions

View file

@ -62,7 +62,7 @@ pub struct ActiveThread {
copied_code_block_ids: HashSet<(MessageId, usize)>, copied_code_block_ids: HashSet<(MessageId, usize)>,
_subscriptions: Vec<Subscription>, _subscriptions: Vec<Subscription>,
notification_subscriptions: HashMap<WindowHandle<AgentNotification>, Vec<Subscription>>, notification_subscriptions: HashMap<WindowHandle<AgentNotification>, Vec<Subscription>>,
feedback_message_editor: Option<Entity<Editor>>, open_feedback_editors: HashMap<MessageId, Entity<Editor>>,
} }
struct RenderedMessage { struct RenderedMessage {
@ -636,7 +636,7 @@ impl ActiveThread {
notifications: Vec::new(), notifications: Vec::new(),
_subscriptions: subscriptions, _subscriptions: subscriptions,
notification_subscriptions: HashMap::default(), notification_subscriptions: HashMap::default(),
feedback_message_editor: None, open_feedback_editors: HashMap::default(),
}; };
for message in thread.read(cx).messages().cloned().collect::<Vec<_>>() { for message in thread.read(cx).messages().cloned().collect::<Vec<_>>() {
@ -939,7 +939,7 @@ impl ActiveThread {
|this, _, event, window, cx| match event { |this, _, event, window, cx| match event {
AgentNotificationEvent::Accepted => { AgentNotificationEvent::Accepted => {
let handle = window.window_handle(); let handle = window.window_handle();
cx.activate(true); // Switch back to the Zed application cx.activate(true);
let workspace_handle = this.workspace.clone(); let workspace_handle = this.workspace.clone();
@ -1111,34 +1111,37 @@ impl ActiveThread {
fn handle_feedback_click( fn handle_feedback_click(
&mut self, &mut self,
message_id: MessageId,
feedback: ThreadFeedback, feedback: ThreadFeedback,
window: &mut Window, window: &mut Window,
cx: &mut Context<Self>, cx: &mut Context<Self>,
) { ) {
let report = self.thread.update(cx, |thread, cx| {
thread.report_message_feedback(message_id, feedback, cx)
});
cx.spawn(async move |this, cx| {
report.await?;
this.update(cx, |_this, cx| cx.notify())
})
.detach_and_log_err(cx);
match feedback { match feedback {
ThreadFeedback::Positive => { ThreadFeedback::Positive => {
let report = self self.open_feedback_editors.remove(&message_id);
.thread
.update(cx, |thread, cx| thread.report_feedback(feedback, cx));
let this = cx.entity().downgrade();
cx.spawn(async move |_, cx| {
report.await?;
this.update(cx, |_this, cx| cx.notify())
})
.detach_and_log_err(cx);
} }
ThreadFeedback::Negative => { ThreadFeedback::Negative => {
self.handle_show_feedback_comments(window, cx); self.handle_show_feedback_comments(message_id, window, cx);
} }
} }
} }
fn handle_show_feedback_comments(&mut self, window: &mut Window, cx: &mut Context<Self>) { fn handle_show_feedback_comments(
if self.feedback_message_editor.is_some() { &mut self,
return; message_id: MessageId,
} window: &mut Window,
cx: &mut Context<Self>,
) {
let buffer = cx.new(|cx| { let buffer = cx.new(|cx| {
let empty_string = String::new(); let empty_string = String::new();
MultiBuffer::singleton(cx.new(|cx| Buffer::local(empty_string, cx)), cx) MultiBuffer::singleton(cx.new(|cx| Buffer::local(empty_string, cx)), cx)
@ -1160,34 +1163,47 @@ impl ActiveThread {
}); });
editor.read(cx).focus_handle(cx).focus(window); editor.read(cx).focus_handle(cx).focus(window);
self.feedback_message_editor = Some(editor); self.open_feedback_editors.insert(message_id, editor);
cx.notify(); cx.notify();
} }
fn submit_feedback_message(&mut self, cx: &mut Context<Self>) { fn submit_feedback_message(&mut self, message_id: MessageId, cx: &mut Context<Self>) {
let Some(editor) = self.feedback_message_editor.clone() else { let Some(editor) = self.open_feedback_editors.get(&message_id) else {
return; return;
}; };
let report_task = self.thread.update(cx, |thread, cx| { let report_task = self.thread.update(cx, |thread, cx| {
thread.report_feedback(ThreadFeedback::Negative, cx) thread.report_message_feedback(message_id, ThreadFeedback::Negative, cx)
}); });
let comments = editor.read(cx).text(cx); let comments = editor.read(cx).text(cx);
if !comments.is_empty() { if !comments.is_empty() {
let thread_id = self.thread.read(cx).id().clone(); let thread_id = self.thread.read(cx).id().clone();
let comments_value = String::from(comments.as_str());
telemetry::event!("Assistant Thread Feedback Comments", thread_id, comments); let message_content = self
.thread
.read(cx)
.message(message_id)
.map(|msg| msg.to_string())
.unwrap_or_default();
telemetry::event!(
"Assistant Thread Feedback Comments",
thread_id,
message_id = message_id.0,
message_content,
comments = comments_value
);
self.open_feedback_editors.remove(&message_id);
cx.spawn(async move |this, cx| {
report_task.await?;
this.update(cx, |_this, cx| cx.notify())
})
.detach_and_log_err(cx);
} }
self.feedback_message_editor = None;
let this = cx.entity().downgrade();
cx.spawn(async move |_, cx| {
report_task.await?;
this.update(cx, |_this, cx| cx.notify())
})
.detach_and_log_err(cx);
} }
fn render_message(&self, ix: usize, window: &mut Window, cx: &mut Context<Self>) -> AnyElement { fn render_message(&self, ix: usize, window: &mut Window, cx: &mut Context<Self>) -> AnyElement {
@ -1214,7 +1230,18 @@ impl ActiveThread {
let is_first_message = ix == 0; let is_first_message = ix == 0;
let is_last_message = ix == self.messages.len() - 1; let is_last_message = ix == self.messages.len() - 1;
let show_feedback = is_last_message && message.role != Role::User;
let show_feedback = (!is_generating && is_last_message && message.role != Role::User)
|| self.messages.get(ix + 1).map_or(false, |next_id| {
self.thread
.read(cx)
.message(*next_id)
.map_or(false, |next_message| {
next_message.role == Role::User
&& thread.tool_uses_for_message(*next_id, cx).is_empty()
&& thread.tool_results_for_message(*next_id).is_empty()
})
});
let needs_confirmation = tool_uses.iter().any(|tool_use| tool_use.needs_confirmation); let needs_confirmation = tool_uses.iter().any(|tool_use| tool_use.needs_confirmation);
@ -1287,8 +1314,9 @@ impl ActiveThread {
let editor_bg_color = colors.editor_background; let editor_bg_color = colors.editor_background;
let bg_user_message_header = editor_bg_color.blend(active_color.opacity(0.25)); let bg_user_message_header = editor_bg_color.blend(active_color.opacity(0.25));
let feedback_container = h_flex().pt_2().pb_4().px_4().gap_1().justify_between(); let feedback_container = h_flex().py_2().px_4().gap_1().justify_between();
let feedback_items = match self.thread.read(cx).feedback() {
let feedback_items = match self.thread.read(cx).message_feedback(message_id) {
Some(feedback) => feedback_container Some(feedback) => feedback_container
.child( .child(
Label::new(match feedback { Label::new(match feedback {
@ -1302,18 +1330,20 @@ impl ActiveThread {
) )
.child( .child(
h_flex() h_flex()
.pr_1()
.gap_1() .gap_1()
.child( .child(
IconButton::new("feedback-thumbs-up", IconName::ThumbsUp) IconButton::new(("feedback-thumbs-up", ix), IconName::ThumbsUp)
.shape(ui::IconButtonShape::Square)
.icon_size(IconSize::XSmall) .icon_size(IconSize::XSmall)
.icon_color(match feedback { .icon_color(match feedback {
ThreadFeedback::Positive => Color::Accent, ThreadFeedback::Positive => Color::Accent,
ThreadFeedback::Negative => Color::Ignored, ThreadFeedback::Negative => Color::Ignored,
}) })
.shape(ui::IconButtonShape::Square)
.tooltip(Tooltip::text("Helpful Response")) .tooltip(Tooltip::text("Helpful Response"))
.on_click(cx.listener(move |this, _, window, cx| { .on_click(cx.listener(move |this, _, window, cx| {
this.handle_feedback_click( this.handle_feedback_click(
message_id,
ThreadFeedback::Positive, ThreadFeedback::Positive,
window, window,
cx, cx,
@ -1321,16 +1351,17 @@ impl ActiveThread {
})), })),
) )
.child( .child(
IconButton::new("feedback-thumbs-down", IconName::ThumbsDown) IconButton::new(("feedback-thumbs-down", ix), IconName::ThumbsDown)
.shape(ui::IconButtonShape::Square)
.icon_size(IconSize::XSmall) .icon_size(IconSize::XSmall)
.icon_color(match feedback { .icon_color(match feedback {
ThreadFeedback::Positive => Color::Ignored, ThreadFeedback::Positive => Color::Ignored,
ThreadFeedback::Negative => Color::Accent, ThreadFeedback::Negative => Color::Accent,
}) })
.shape(ui::IconButtonShape::Square)
.tooltip(Tooltip::text("Not Helpful")) .tooltip(Tooltip::text("Not Helpful"))
.on_click(cx.listener(move |this, _, window, cx| { .on_click(cx.listener(move |this, _, window, cx| {
this.handle_feedback_click( this.handle_feedback_click(
message_id,
ThreadFeedback::Negative, ThreadFeedback::Negative,
window, window,
cx, cx,
@ -1351,13 +1382,14 @@ impl ActiveThread {
h_flex() h_flex()
.gap_1() .gap_1()
.child( .child(
IconButton::new("feedback-thumbs-up", IconName::ThumbsUp) IconButton::new(("feedback-thumbs-up", ix), IconName::ThumbsUp)
.icon_size(IconSize::XSmall) .icon_size(IconSize::XSmall)
.icon_color(Color::Ignored) .icon_color(Color::Ignored)
.shape(ui::IconButtonShape::Square) .shape(ui::IconButtonShape::Square)
.tooltip(Tooltip::text("Helpful Response")) .tooltip(Tooltip::text("Helpful Response"))
.on_click(cx.listener(move |this, _, window, cx| { .on_click(cx.listener(move |this, _, window, cx| {
this.handle_feedback_click( this.handle_feedback_click(
message_id,
ThreadFeedback::Positive, ThreadFeedback::Positive,
window, window,
cx, cx,
@ -1365,13 +1397,14 @@ impl ActiveThread {
})), })),
) )
.child( .child(
IconButton::new("feedback-thumbs-down", IconName::ThumbsDown) IconButton::new(("feedback-thumbs-down", ix), IconName::ThumbsDown)
.icon_size(IconSize::XSmall) .icon_size(IconSize::XSmall)
.icon_color(Color::Ignored) .icon_color(Color::Ignored)
.shape(ui::IconButtonShape::Square) .shape(ui::IconButtonShape::Square)
.tooltip(Tooltip::text("Not Helpful")) .tooltip(Tooltip::text("Not Helpful"))
.on_click(cx.listener(move |this, _, window, cx| { .on_click(cx.listener(move |this, _, window, cx| {
this.handle_feedback_click( this.handle_feedback_click(
message_id,
ThreadFeedback::Negative, ThreadFeedback::Negative,
window, window,
cx, cx,
@ -1669,31 +1702,31 @@ impl ActiveThread {
.child(generating_label.unwrap()), .child(generating_label.unwrap()),
) )
}) })
.when(show_feedback && !is_generating, |parent| { .when(show_feedback, move |parent| {
parent.child(feedback_items).when_some( parent.child(feedback_items).when_some(
self.feedback_message_editor.clone(), self.open_feedback_editors.get(&message_id),
|parent, feedback_editor| { move |parent, feedback_editor| {
let focus_handle = feedback_editor.focus_handle(cx); let focus_handle = feedback_editor.focus_handle(cx);
parent.child( parent.child(
v_flex() v_flex()
.key_context("AgentFeedbackMessageEditor") .key_context("AgentFeedbackMessageEditor")
.on_action(cx.listener(|this, _: &menu::Cancel, _, cx| { .on_action(cx.listener(move |this, _: &menu::Cancel, _, cx| {
this.feedback_message_editor = None; this.open_feedback_editors.remove(&message_id);
cx.notify(); cx.notify();
})) }))
.on_action(cx.listener(|this, _: &menu::Confirm, _, cx| { .on_action(cx.listener(move |this, _: &menu::Confirm, _, cx| {
this.submit_feedback_message(cx); this.submit_feedback_message(message_id, cx);
cx.notify(); cx.notify();
})) }))
.on_action(cx.listener(Self::confirm_editing_message)) .on_action(cx.listener(Self::confirm_editing_message))
.my_3() .mb_2()
.mx_4() .mx_4()
.p_2() .p_2()
.rounded_md() .rounded_md()
.border_1() .border_1()
.border_color(cx.theme().colors().border) .border_color(cx.theme().colors().border)
.bg(cx.theme().colors().editor_background) .bg(cx.theme().colors().editor_background)
.child(feedback_editor) .child(feedback_editor.clone())
.child( .child(
h_flex() h_flex()
.gap_1() .gap_1()
@ -1710,10 +1743,13 @@ impl ActiveThread {
) )
.map(|kb| kb.size(rems_from_px(10.))), .map(|kb| kb.size(rems_from_px(10.))),
) )
.on_click(cx.listener(|this, _, _, cx| { .on_click(cx.listener(
this.feedback_message_editor = None; move |this, _, _window, cx| {
cx.notify(); this.open_feedback_editors
})), .remove(&message_id);
cx.notify();
},
)),
) )
.child( .child(
Button::new( Button::new(
@ -1732,9 +1768,9 @@ impl ActiveThread {
.map(|kb| kb.size(rems_from_px(10.))), .map(|kb| kb.size(rems_from_px(10.))),
) )
.on_click( .on_click(
cx.listener(|this, _, _, cx| { cx.listener(move |this, _, _window, cx| {
this.submit_feedback_message(cx); this.submit_feedback_message(message_id, cx);
cx.notify(); cx.notify()
}), }),
), ),
), ),

View file

@ -182,7 +182,7 @@ pub struct ThreadCheckpoint {
git_checkpoint: GitStoreCheckpoint, git_checkpoint: GitStoreCheckpoint,
} }
#[derive(Copy, Clone, Debug)] #[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub enum ThreadFeedback { pub enum ThreadFeedback {
Positive, Positive,
Negative, Negative,
@ -260,6 +260,7 @@ pub struct Thread {
initial_project_snapshot: Shared<Task<Option<Arc<ProjectSnapshot>>>>, initial_project_snapshot: Shared<Task<Option<Arc<ProjectSnapshot>>>>,
cumulative_token_usage: TokenUsage, cumulative_token_usage: TokenUsage,
feedback: Option<ThreadFeedback>, feedback: Option<ThreadFeedback>,
message_feedback: HashMap<MessageId, ThreadFeedback>,
} }
impl Thread { impl Thread {
@ -298,6 +299,7 @@ impl Thread {
}, },
cumulative_token_usage: TokenUsage::default(), cumulative_token_usage: TokenUsage::default(),
feedback: None, feedback: None,
message_feedback: HashMap::default(),
} }
} }
@ -361,6 +363,7 @@ impl Thread {
initial_project_snapshot: Task::ready(serialized.initial_project_snapshot).shared(), initial_project_snapshot: Task::ready(serialized.initial_project_snapshot).shared(),
cumulative_token_usage: serialized.cumulative_token_usage, cumulative_token_usage: serialized.cumulative_token_usage,
feedback: None, feedback: None,
message_feedback: HashMap::default(),
} }
} }
@ -1518,24 +1521,38 @@ impl Thread {
canceled canceled
} }
/// Returns the feedback given to the thread, if any.
pub fn feedback(&self) -> Option<ThreadFeedback> { pub fn feedback(&self) -> Option<ThreadFeedback> {
self.feedback self.feedback
} }
/// Reports feedback about the thread and stores it in our telemetry backend. pub fn message_feedback(&self, message_id: MessageId) -> Option<ThreadFeedback> {
pub fn report_feedback( self.message_feedback.get(&message_id).copied()
}
pub fn report_message_feedback(
&mut self, &mut self,
message_id: MessageId,
feedback: ThreadFeedback, feedback: ThreadFeedback,
cx: &mut Context<Self>, cx: &mut Context<Self>,
) -> Task<Result<()>> { ) -> Task<Result<()>> {
if self.message_feedback.get(&message_id) == Some(&feedback) {
return Task::ready(Ok(()));
}
let final_project_snapshot = Self::project_snapshot(self.project.clone(), cx); let final_project_snapshot = Self::project_snapshot(self.project.clone(), cx);
let serialized_thread = self.serialize(cx); let serialized_thread = self.serialize(cx);
let thread_id = self.id().clone(); let thread_id = self.id().clone();
let client = self.project.read(cx).client(); let client = self.project.read(cx).client();
self.feedback = Some(feedback);
self.message_feedback.insert(message_id, feedback);
cx.notify(); cx.notify();
let message_content = self
.message(message_id)
.map(|msg| msg.to_string())
.unwrap_or_default();
cx.background_spawn(async move { cx.background_spawn(async move {
let final_project_snapshot = final_project_snapshot.await; let final_project_snapshot = final_project_snapshot.await;
let serialized_thread = serialized_thread.await?; let serialized_thread = serialized_thread.await?;
@ -1550,6 +1567,8 @@ impl Thread {
"Assistant Thread Rated", "Assistant Thread Rated",
rating, rating,
thread_id, thread_id,
message_id = message_id.0,
message_content,
thread_data, thread_data,
final_project_snapshot final_project_snapshot
); );
@ -1559,6 +1578,52 @@ impl Thread {
}) })
} }
pub fn report_feedback(
&mut self,
feedback: ThreadFeedback,
cx: &mut Context<Self>,
) -> Task<Result<()>> {
let last_assistant_message_id = self
.messages
.iter()
.rev()
.find(|msg| msg.role == Role::Assistant)
.map(|msg| msg.id);
if let Some(message_id) = last_assistant_message_id {
self.report_message_feedback(message_id, feedback, cx)
} else {
let final_project_snapshot = Self::project_snapshot(self.project.clone(), cx);
let serialized_thread = self.serialize(cx);
let thread_id = self.id().clone();
let client = self.project.read(cx).client();
self.feedback = Some(feedback);
cx.notify();
cx.background_spawn(async move {
let final_project_snapshot = final_project_snapshot.await;
let serialized_thread = serialized_thread.await?;
let thread_data = serde_json::to_value(serialized_thread)
.unwrap_or_else(|_| serde_json::Value::Null);
let rating = match feedback {
ThreadFeedback::Positive => "positive",
ThreadFeedback::Negative => "negative",
};
telemetry::event!(
"Assistant Thread Rated",
rating,
thread_id,
thread_data,
final_project_snapshot
);
client.telemetry().flush_events();
Ok(())
})
}
}
/// Create a snapshot of the current project state including git information and unsaved buffers. /// Create a snapshot of the current project state including git information and unsaved buffers.
fn project_snapshot( fn project_snapshot(
project: Entity<Project>, project: Entity<Project>,

View file

@ -498,10 +498,12 @@ impl RateCompletionModal {
cx cx
)) ))
.on_click(cx.listener(move |this, _, window, cx| { .on_click(cx.listener(move |this, _, window, cx| {
this.thumbs_down_active( if this.active_completion.is_some() {
&ThumbsDownActiveCompletion, this.thumbs_down_active(
window, cx, &ThumbsDownActiveCompletion,
); window, cx,
);
}
})), })),
) )
.child( .child(
@ -517,7 +519,9 @@ impl RateCompletionModal {
cx cx
)) ))
.on_click(cx.listener(move |this, _, window, cx| { .on_click(cx.listener(move |this, _, window, cx| {
this.thumbs_up_active(&ThumbsUpActiveCompletion, window, cx); if this.active_completion.is_some() {
this.thumbs_up_active(&ThumbsUpActiveCompletion, window, cx);
}
})), })),
), ),
), ),