diff --git a/crates/agent/src/active_thread.rs b/crates/agent/src/active_thread.rs index 45bea67f6275e01772a920b5a9fec128bdd1523f..ad4cb63261294bbbe810e6c22e150f94e177d088 100644 --- a/crates/agent/src/active_thread.rs +++ b/crates/agent/src/active_thread.rs @@ -62,7 +62,7 @@ pub struct ActiveThread { copied_code_block_ids: HashSet<(MessageId, usize)>, _subscriptions: Vec, notification_subscriptions: HashMap, Vec>, - feedback_message_editor: Option>, + open_feedback_editors: HashMap>, } struct RenderedMessage { @@ -636,7 +636,7 @@ impl ActiveThread { notifications: Vec::new(), _subscriptions: subscriptions, notification_subscriptions: HashMap::default(), - feedback_message_editor: None, + open_feedback_editors: HashMap::default(), }; for message in thread.read(cx).messages().cloned().collect::>() { @@ -939,7 +939,7 @@ impl ActiveThread { |this, _, event, window, cx| match event { AgentNotificationEvent::Accepted => { let handle = window.window_handle(); - cx.activate(true); // Switch back to the Zed application + cx.activate(true); let workspace_handle = this.workspace.clone(); @@ -1111,34 +1111,37 @@ impl ActiveThread { fn handle_feedback_click( &mut self, + message_id: MessageId, feedback: ThreadFeedback, window: &mut Window, cx: &mut Context, ) { + 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 { ThreadFeedback::Positive => { - let report = self - .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); + self.open_feedback_editors.remove(&message_id); } 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) { - if self.feedback_message_editor.is_some() { - return; - } - + fn handle_show_feedback_comments( + &mut self, + message_id: MessageId, + window: &mut Window, + cx: &mut Context, + ) { let buffer = cx.new(|cx| { let empty_string = String::new(); 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); - self.feedback_message_editor = Some(editor); + self.open_feedback_editors.insert(message_id, editor); cx.notify(); } - fn submit_feedback_message(&mut self, cx: &mut Context) { - let Some(editor) = self.feedback_message_editor.clone() else { + fn submit_feedback_message(&mut self, message_id: MessageId, cx: &mut Context) { + let Some(editor) = self.open_feedback_editors.get(&message_id) else { return; }; 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); if !comments.is_empty() { 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.feedback_message_editor = None; + self.open_feedback_editors.remove(&message_id); - 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); + cx.spawn(async move |this, 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) -> AnyElement { @@ -1214,7 +1230,18 @@ impl ActiveThread { let is_first_message = ix == 0; 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); @@ -1287,8 +1314,9 @@ impl ActiveThread { let editor_bg_color = colors.editor_background; 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_items = match self.thread.read(cx).feedback() { + let feedback_container = h_flex().py_2().px_4().gap_1().justify_between(); + + let feedback_items = match self.thread.read(cx).message_feedback(message_id) { Some(feedback) => feedback_container .child( Label::new(match feedback { @@ -1302,18 +1330,20 @@ impl ActiveThread { ) .child( h_flex() + .pr_1() .gap_1() .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_color(match feedback { ThreadFeedback::Positive => Color::Accent, ThreadFeedback::Negative => Color::Ignored, }) - .shape(ui::IconButtonShape::Square) .tooltip(Tooltip::text("Helpful Response")) .on_click(cx.listener(move |this, _, window, cx| { this.handle_feedback_click( + message_id, ThreadFeedback::Positive, window, cx, @@ -1321,16 +1351,17 @@ impl ActiveThread { })), ) .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_color(match feedback { ThreadFeedback::Positive => Color::Ignored, ThreadFeedback::Negative => Color::Accent, }) - .shape(ui::IconButtonShape::Square) .tooltip(Tooltip::text("Not Helpful")) .on_click(cx.listener(move |this, _, window, cx| { this.handle_feedback_click( + message_id, ThreadFeedback::Negative, window, cx, @@ -1351,13 +1382,14 @@ impl ActiveThread { h_flex() .gap_1() .child( - IconButton::new("feedback-thumbs-up", IconName::ThumbsUp) + IconButton::new(("feedback-thumbs-up", ix), IconName::ThumbsUp) .icon_size(IconSize::XSmall) .icon_color(Color::Ignored) .shape(ui::IconButtonShape::Square) .tooltip(Tooltip::text("Helpful Response")) .on_click(cx.listener(move |this, _, window, cx| { this.handle_feedback_click( + message_id, ThreadFeedback::Positive, window, cx, @@ -1365,13 +1397,14 @@ impl ActiveThread { })), ) .child( - IconButton::new("feedback-thumbs-down", IconName::ThumbsDown) + IconButton::new(("feedback-thumbs-down", ix), IconName::ThumbsDown) .icon_size(IconSize::XSmall) .icon_color(Color::Ignored) .shape(ui::IconButtonShape::Square) .tooltip(Tooltip::text("Not Helpful")) .on_click(cx.listener(move |this, _, window, cx| { this.handle_feedback_click( + message_id, ThreadFeedback::Negative, window, cx, @@ -1669,31 +1702,31 @@ impl ActiveThread { .child(generating_label.unwrap()), ) }) - .when(show_feedback && !is_generating, |parent| { + .when(show_feedback, move |parent| { parent.child(feedback_items).when_some( - self.feedback_message_editor.clone(), - |parent, feedback_editor| { + self.open_feedback_editors.get(&message_id), + move |parent, feedback_editor| { let focus_handle = feedback_editor.focus_handle(cx); parent.child( v_flex() .key_context("AgentFeedbackMessageEditor") - .on_action(cx.listener(|this, _: &menu::Cancel, _, cx| { - this.feedback_message_editor = None; + .on_action(cx.listener(move |this, _: &menu::Cancel, _, cx| { + this.open_feedback_editors.remove(&message_id); cx.notify(); })) - .on_action(cx.listener(|this, _: &menu::Confirm, _, cx| { - this.submit_feedback_message(cx); + .on_action(cx.listener(move |this, _: &menu::Confirm, _, cx| { + this.submit_feedback_message(message_id, cx); cx.notify(); })) .on_action(cx.listener(Self::confirm_editing_message)) - .my_3() + .mb_2() .mx_4() .p_2() .rounded_md() .border_1() .border_color(cx.theme().colors().border) .bg(cx.theme().colors().editor_background) - .child(feedback_editor) + .child(feedback_editor.clone()) .child( h_flex() .gap_1() @@ -1710,10 +1743,13 @@ impl ActiveThread { ) .map(|kb| kb.size(rems_from_px(10.))), ) - .on_click(cx.listener(|this, _, _, cx| { - this.feedback_message_editor = None; - cx.notify(); - })), + .on_click(cx.listener( + move |this, _, _window, cx| { + this.open_feedback_editors + .remove(&message_id); + cx.notify(); + }, + )), ) .child( Button::new( @@ -1732,9 +1768,9 @@ impl ActiveThread { .map(|kb| kb.size(rems_from_px(10.))), ) .on_click( - cx.listener(|this, _, _, cx| { - this.submit_feedback_message(cx); - cx.notify(); + cx.listener(move |this, _, _window, cx| { + this.submit_feedback_message(message_id, cx); + cx.notify() }), ), ), diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index 4167bfbbd57e77f3557e0209a601dc5195055e1f..691431221c0fb0a22ae65419e4915ae1b5639a00 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -182,7 +182,7 @@ pub struct ThreadCheckpoint { git_checkpoint: GitStoreCheckpoint, } -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum ThreadFeedback { Positive, Negative, @@ -260,6 +260,7 @@ pub struct Thread { initial_project_snapshot: Shared>>>, cumulative_token_usage: TokenUsage, feedback: Option, + message_feedback: HashMap, } impl Thread { @@ -298,6 +299,7 @@ impl Thread { }, cumulative_token_usage: TokenUsage::default(), feedback: None, + message_feedback: HashMap::default(), } } @@ -361,6 +363,7 @@ impl Thread { initial_project_snapshot: Task::ready(serialized.initial_project_snapshot).shared(), cumulative_token_usage: serialized.cumulative_token_usage, feedback: None, + message_feedback: HashMap::default(), } } @@ -1518,24 +1521,38 @@ impl Thread { canceled } - /// Returns the feedback given to the thread, if any. pub fn feedback(&self) -> Option { self.feedback } - /// Reports feedback about the thread and stores it in our telemetry backend. - pub fn report_feedback( + pub fn message_feedback(&self, message_id: MessageId) -> Option { + self.message_feedback.get(&message_id).copied() + } + + pub fn report_message_feedback( &mut self, + message_id: MessageId, feedback: ThreadFeedback, cx: &mut Context, ) -> Task> { + 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 serialized_thread = self.serialize(cx); let thread_id = self.id().clone(); let client = self.project.read(cx).client(); - self.feedback = Some(feedback); + + self.message_feedback.insert(message_id, feedback); + cx.notify(); + let message_content = self + .message(message_id) + .map(|msg| msg.to_string()) + .unwrap_or_default(); + cx.background_spawn(async move { let final_project_snapshot = final_project_snapshot.await; let serialized_thread = serialized_thread.await?; @@ -1550,6 +1567,8 @@ impl Thread { "Assistant Thread Rated", rating, thread_id, + message_id = message_id.0, + message_content, thread_data, final_project_snapshot ); @@ -1559,6 +1578,52 @@ impl Thread { }) } + pub fn report_feedback( + &mut self, + feedback: ThreadFeedback, + cx: &mut Context, + ) -> Task> { + 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. fn project_snapshot( project: Entity, diff --git a/crates/zeta/src/rate_completion_modal.rs b/crates/zeta/src/rate_completion_modal.rs index 17c3b0bdef46fc18f92e8e53ce27d511272cd5e8..a13e935f9142dc8a2f7ca0d7f31d194c9c0a5fc2 100644 --- a/crates/zeta/src/rate_completion_modal.rs +++ b/crates/zeta/src/rate_completion_modal.rs @@ -498,10 +498,12 @@ impl RateCompletionModal { cx )) .on_click(cx.listener(move |this, _, window, cx| { - this.thumbs_down_active( - &ThumbsDownActiveCompletion, - window, cx, - ); + if this.active_completion.is_some() { + this.thumbs_down_active( + &ThumbsDownActiveCompletion, + window, cx, + ); + } })), ) .child( @@ -517,7 +519,9 @@ impl RateCompletionModal { 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); + } })), ), ),