Diff review comments: store locally and batch submit to agent (#46669)

Doesn't support editing or deleting comments yet, but inputs work:

<img width="797" height="881" alt="Screenshot 2026-01-14 at 11 51 36 AM"
src="https://github.com/user-attachments/assets/746e085b-a919-475a-b804-ee0377e40a0b"
/>


<img width="698" height="178" alt="Screenshot 2026-01-14 at 11 47 51 AM"
src="https://github.com/user-attachments/assets/3edac0e8-62e5-4af5-a324-df2bd293f3ca"
/>

<img width="695" height="695" alt="Screenshot 2026-01-14 at 11 47 58 AM"
src="https://github.com/user-attachments/assets/64795ffd-62b4-48ea-a46c-7724221095d7"
/>


### Features implemented:
- **Local comment storage**: Comments are stored per-hunk in the Editor,
with chronological ordering
- **Expandable comments section**: Shows "N Comments" header that can
expand/collapse
- **Inline editing**: Click edit to transform a comment row into an
editable text field with confirm/cancel buttons
- **Delete functionality**: Remove individual comments
- **Send Review to Agent**: Toolbar button with badge showing comment
count, batch-submits all comments across all files/hunks to the Agent
panel
- **User avatars**: Shows the current user's avatar (from their Zed
account) next to comments, falls back to Person icon when not logged in
- **Visual tests**: Added tests for one comment, multiple comments
expanded, and comments collapsed

Note that this is behind a feature flag, so no release notes yet.

Release Notes:

- N/A

---------

Co-authored-by: Zed Zippy <234243425+zed-zippy[bot]@users.noreply.github.com>
Co-authored-by: David Baldwin <baldwindavid@gmail.com>
This commit is contained in:
authored and GitHub committed 2026-01-14 17:43:38 +00:00
1 parent 0350e0ca87
commit 415f384ff3
10 files changed
+2491 -42

No files matched your search

+67 -1
View File
@@ -33,7 +33,7 @@ use rope::Point;
use settings::Settings;
use std::{cell::RefCell, fmt::Write, rc::Rc, sync::Arc};
use theme::ThemeSettings;
use ui::{ContextMenu, prelude::*};
use ui::{ButtonLike, ButtonStyle, ContextMenu, Disclosure, ElevationIndex, prelude::*};
use util::{ResultExt, debug_panic};
use workspace::{CollaboratorId, Workspace};
use zed_actions::agent::{Chat, PasteRaw};
@@ -803,6 +803,72 @@ impl MessageEditor {
.detach();
}
/// Inserts code snippets as creases into the editor.
/// Each tuple contains (code_text, crease_title).
pub fn insert_code_creases(
&mut self,
creases: Vec<(String, String)>,
window: &mut Window,
cx: &mut Context<Self>,
) {
use editor::display_map::{Crease, FoldPlaceholder};
use multi_buffer::MultiBufferRow;
use rope::Point;
self.editor.update(cx, |editor, cx| {
editor.insert("\n", window, cx);
for (text, crease_title) in creases {
let point = editor
.selections
.newest::<Point>(&editor.display_snapshot(cx))
.head();
let start_row = MultiBufferRow(point.row);
editor.insert(&text, window, cx);
let snapshot = editor.buffer().read(cx).snapshot(cx);
let anchor_before = snapshot.anchor_after(point);
let anchor_after = editor
.selections
.newest_anchor()
.head()
.bias_left(&snapshot);
editor.insert("\n", window, cx);
let fold_placeholder = FoldPlaceholder {
render: Arc::new({
let title = crease_title.clone();
move |_fold_id, _fold_range, _cx| {
ButtonLike::new("code-crease")
.style(ButtonStyle::Filled)
.layer(ElevationIndex::ElevatedSurface)
.child(Icon::new(IconName::TextSnippet))
.child(Label::new(title.clone()).single_line())
.into_any_element()
}
}),
merge_adjacent: false,
..Default::default()
};
let crease = Crease::inline(
anchor_before..anchor_after,
fold_placeholder,
|row, is_folded, fold, _window, _cx| {
Disclosure::new(("code-crease-toggle", row.0 as u64), !is_folded)
.toggle_state(is_folded)
.on_click(move |_e, window, cx| fold(!is_folded, window, cx))
.into_any_element()
},
|_, _, _, _| gpui::Empty.into_any(),
);
editor.insert_creases(vec![crease], cx);
editor.fold_at(start_row, window, cx);
}
});
}
pub fn insert_selections(&mut self, window: &mut Window, cx: &mut Context<Self>) {
let editor = self.editor.read(cx);
let editor_buffer = editor.buffer().read(cx);
+12
View File
@@ -6787,6 +6787,18 @@ impl AcpThreadView {
});
}
/// Inserts code snippets as creases into the message editor.
pub(crate) fn insert_code_crease(
&self,
creases: Vec<(String, String)>,
window: &mut Window,
cx: &mut Context<Self>,
) {
self.message_editor.update(cx, |message_editor, cx| {
message_editor.insert_code_creases(creases, window, cx);
});
}
fn render_thread_retry_status_callout(
&self,
_window: &mut Window,
+1 -1
View File
@@ -775,7 +775,7 @@ impl AgentPanel {
.unwrap_or(true)
}
fn active_thread_view(&self) -> Option<&Entity<AcpThreadView>> {
pub(crate) fn active_thread_view(&self) -> Option<&Entity<AcpThreadView>> {
match &self.active_view {
ActiveView::ExternalAgentThread { thread_view, .. } => Some(thread_view),
ActiveView::TextThread { .. }
+158 -3
View File
@@ -1,4 +1,5 @@
use crate::{
agent_panel::AgentType,
language_model_selector::{LanguageModelSelector, language_model_selector},
ui::{BurnModeTooltip, ModelSelectorTooltip},
};
@@ -10,8 +11,8 @@ use client::{proto, zed_urls};
use collections::{BTreeSet, HashMap, HashSet, hash_map};
use editor::{
Anchor, Editor, EditorEvent, MenuEditPredictionsPolicy, MultiBuffer, MultiBufferOffset,
MultiBufferSnapshot, RowExt, ToOffset as _, ToPoint,
actions::{MoveToEndOfLine, Newline, ShowCompletions},
MultiBufferSnapshot, RowExt, ToOffset as _, ToPoint as _,
actions::{MoveToEndOfLine, Newline, SendReviewToAgent, ShowCompletions},
display_map::{
BlockPlacement, BlockProperties, BlockStyle, Crease, CreaseMetadata, CustomBlockId, FoldId,
RenderBlock, ToDisplayPoint,
@@ -220,7 +221,8 @@ impl TextThreadEditor {
.register_action(TextThreadEditor::quote_selection)
.register_action(TextThreadEditor::insert_selection)
.register_action(TextThreadEditor::copy_code)
.register_action(TextThreadEditor::handle_insert_dragged_files);
.register_action(TextThreadEditor::handle_insert_dragged_files)
.register_action(TextThreadEditor::handle_send_review_to_agent);
},
)
.detach();
@@ -1517,6 +1519,159 @@ impl TextThreadEditor {
agent_panel_delegate.quote_selection(workspace, selections, buffer, window, cx);
}
/// Handles the SendReviewToAgent action from the ProjectDiff toolbar.
/// Collects ALL stored review comments from ALL hunks and sends them
/// to the Agent panel as creases.
pub fn handle_send_review_to_agent(
workspace: &mut Workspace,
_: &SendReviewToAgent,
window: &mut Window,
cx: &mut Context<Workspace>,
) {
use editor::{DiffHunkKey, StoredReviewComment};
use git_ui::project_diff::ProjectDiff;
// Find the ProjectDiff item
let Some(project_diff) = workspace.items_of_type::<ProjectDiff>(cx).next() else {
workspace.show_toast(
Toast::new(
NotificationId::unique::<SendReviewToAgent>(),
"No Project Diff panel found. Open it first to add review comments.",
),
cx,
);
return;
};
// Get the buffer reference first (before taking comments)
let buffer = project_diff.update(cx, |project_diff, cx| {
project_diff
.editor()
.read(cx)
.primary_editor()
.read(cx)
.buffer()
.clone()
});
// Extract all stored comments from all hunks
let all_comments: Vec<(DiffHunkKey, Vec<StoredReviewComment>)> =
project_diff.update(cx, |project_diff, cx| {
let editor = project_diff.editor().read(cx).primary_editor().clone();
editor.update(cx, |editor, cx| editor.take_all_review_comments(cx))
});
// Flatten: we have Vec<(DiffHunkKey, Vec<StoredReviewComment>)>
// Convert to Vec<StoredReviewComment> for processing
let comments: Vec<StoredReviewComment> = all_comments
.into_iter()
.flat_map(|(_, comments)| comments)
.collect();
if comments.is_empty() {
workspace.show_toast(
Toast::new(
NotificationId::unique::<SendReviewToAgent>(),
"No review comments to send. Add comments using the + button in the diff view.",
),
cx,
);
return;
}
// Get or create the agent panel
let Some(panel) = workspace.panel::<crate::AgentPanel>(cx) else {
workspace.show_toast(
Toast::new(
NotificationId::unique::<SendReviewToAgent>(),
"Agent panel is not available.",
),
cx,
);
return;
};
// Create a new thread if there isn't an active one (synchronous call)
let has_active_thread = panel.read(cx).active_thread_view().is_some();
if !has_active_thread {
panel.update(cx, |panel, cx| {
panel.new_agent_thread(AgentType::NativeAgent, window, cx);
});
}
// Focus the agent panel
workspace.focus_panel::<crate::AgentPanel>(window, cx);
// Defer inserting creases until after the current update cycle completes,
// allowing the newly created thread (if any) to fully initialize.
cx.defer_in(window, move |workspace, window, cx| {
let Some(panel) = workspace.panel::<crate::AgentPanel>(cx) else {
workspace.show_toast(
Toast::new(
NotificationId::unique::<SendReviewToAgent>(),
"Agent panel closed unexpectedly.",
),
cx,
);
return;
};
let thread_view = panel.read(cx).active_thread_view().cloned();
let Some(thread_view) = thread_view else {
workspace.show_toast(
Toast::new(
NotificationId::unique::<SendReviewToAgent>(),
"No active thread view available after creating thread.",
),
cx,
);
return;
};
// Build creases for all comments, grouping by code snippet
// so each snippet appears once with all its comments
let snapshot = buffer.read(cx).snapshot(cx);
// Group comments by their point range (code snippet)
let mut comments_by_range: std::collections::BTreeMap<
(rope::Point, rope::Point),
Vec<String>,
> = std::collections::BTreeMap::new();
for comment in comments {
let start = comment.anchor_range.start.to_point(&snapshot);
let end = comment.anchor_range.end.to_point(&snapshot);
comments_by_range
.entry((start, end))
.or_default()
.push(comment.comment);
}
// Build one crease per unique code snippet with all its comments
let mut all_creases = Vec::new();
for ((start, end), comment_texts) in comments_by_range {
let point_range = start..end;
let mut creases =
selections_creases(vec![point_range.clone()], snapshot.clone(), cx);
// Append all comments after the code snippet
for (code_text, crease_title) in &mut creases {
let comments_section = comment_texts.join("\n\n");
*code_text = format!("{}\n\n{}", code_text, comments_section);
*crease_title = format!("Review: {}", crease_title);
}
all_creases.extend(creases);
}
// Insert all creases into the message editor
thread_view.update(cx, |thread_view, cx| {
thread_view.insert_code_crease(all_creases, window, cx);
});
});
}
pub fn quote_ranges(
&mut self,
ranges: Vec<Range<Point>>,
+38
View File
@@ -844,6 +844,12 @@ actions!(
/// Toggles diff display for selected hunks.
#[action(deprecated_aliases = ["editor::ToggleHunkDiff"])]
ToggleSelectedDiffHunks,
/// Stores the diff review comment locally (for later batch submission).
SubmitDiffReviewComment,
/// Toggles the expanded state of the comments section in the overlay.
ToggleReviewCommentsExpanded,
/// Sends all stored review comments to the Agent panel.
SendReviewToAgent,
/// Toggles the selection menu.
ToggleSelectionMenu,
/// Toggles soft wrap mode.
@@ -890,3 +896,35 @@ impl Default for FindAllReferences {
}
}
}
/// Edits a stored review comment inline.
#[derive(PartialEq, Clone, Deserialize, JsonSchema, Action)]
#[action(namespace = editor)]
#[serde(deny_unknown_fields)]
pub struct EditReviewComment {
pub id: usize,
}
/// Deletes a stored review comment.
#[derive(PartialEq, Clone, Deserialize, JsonSchema, Action)]
#[action(namespace = editor)]
#[serde(deny_unknown_fields)]
pub struct DeleteReviewComment {
pub id: usize,
}
/// Confirms an inline edit of a review comment.
#[derive(PartialEq, Clone, Deserialize, JsonSchema, Action)]
#[action(namespace = editor)]
#[serde(deny_unknown_fields)]
pub struct ConfirmEditReviewComment {
pub id: usize,
}
/// Cancels an inline edit of a review comment.
#[derive(PartialEq, Clone, Deserialize, JsonSchema, Action)]
#[action(namespace = editor)]
#[serde(deny_unknown_fields)]
pub struct CancelEditReviewComment {
pub id: usize,
}
File diff suppressed because it is too large. Load diff
+739
View File
@@ -30454,6 +30454,745 @@ async fn test_diff_review_button_shown_when_ai_enabled(cx: &mut TestAppContext)
});
}
/// Helper function to create a DiffHunkKey for testing.
/// Uses Anchor::min() as a placeholder anchor since these tests don't need
/// real buffer positioning.
fn test_hunk_key(file_path: &str) -> DiffHunkKey {
DiffHunkKey {
file_path: if file_path.is_empty() {
Arc::from(util::rel_path::RelPath::empty())
} else {
Arc::from(util::rel_path::RelPath::unix(file_path).unwrap())
},
hunk_start_anchor: Anchor::min(),
}
}
/// Helper function to create a DiffHunkKey with a specific anchor for testing.
fn test_hunk_key_with_anchor(file_path: &str, anchor: Anchor) -> DiffHunkKey {
DiffHunkKey {
file_path: if file_path.is_empty() {
Arc::from(util::rel_path::RelPath::empty())
} else {
Arc::from(util::rel_path::RelPath::unix(file_path).unwrap())
},
hunk_start_anchor: anchor,
}
}
/// Helper function to add a review comment with default anchors for testing.
fn add_test_comment(
editor: &mut Editor,
key: DiffHunkKey,
comment: &str,
display_row: u32,
cx: &mut Context<Editor>,
) -> usize {
editor.add_review_comment(
key,
comment.to_string(),
DisplayRow(display_row),
Anchor::min()..Anchor::max(),
cx,
)
}
#[gpui::test]
fn test_review_comment_add_to_hunk(cx: &mut TestAppContext) {
init_test(cx, |_| {});
let editor = cx.add_window(|window, cx| Editor::single_line(window, cx));
_ = editor.update(cx, |editor: &mut Editor, _window, cx| {
let key = test_hunk_key("");
let id = add_test_comment(editor, key.clone(), "Test comment", 0, cx);
let snapshot = editor.buffer().read(cx).snapshot(cx);
assert_eq!(editor.total_review_comment_count(), 1);
assert_eq!(editor.hunk_comment_count(&key, &snapshot), 1);
let comments = editor.comments_for_hunk(&key, &snapshot);
assert_eq!(comments.len(), 1);
assert_eq!(comments[0].comment, "Test comment");
assert_eq!(comments[0].id, id);
});
}
#[gpui::test]
fn test_review_comments_are_per_hunk(cx: &mut TestAppContext) {
init_test(cx, |_| {});
let editor = cx.add_window(|window, cx| Editor::single_line(window, cx));
_ = editor.update(cx, |editor: &mut Editor, _window, cx| {
let snapshot = editor.buffer().read(cx).snapshot(cx);
let anchor1 = snapshot.anchor_before(Point::new(0, 0));
let anchor2 = snapshot.anchor_before(Point::new(0, 0));
let key1 = test_hunk_key_with_anchor("file1.rs", anchor1);
let key2 = test_hunk_key_with_anchor("file2.rs", anchor2);
add_test_comment(editor, key1.clone(), "Comment for file1", 0, cx);
add_test_comment(editor, key2.clone(), "Comment for file2", 10, cx);
let snapshot = editor.buffer().read(cx).snapshot(cx);
assert_eq!(editor.total_review_comment_count(), 2);
assert_eq!(editor.hunk_comment_count(&key1, &snapshot), 1);
assert_eq!(editor.hunk_comment_count(&key2, &snapshot), 1);
assert_eq!(
editor.comments_for_hunk(&key1, &snapshot)[0].comment,
"Comment for file1"
);
assert_eq!(
editor.comments_for_hunk(&key2, &snapshot)[0].comment,
"Comment for file2"
);
});
}
#[gpui::test]
fn test_review_comment_remove(cx: &mut TestAppContext) {
init_test(cx, |_| {});
let editor = cx.add_window(|window, cx| Editor::single_line(window, cx));
_ = editor.update(cx, |editor: &mut Editor, _window, cx| {
let key = test_hunk_key("");
let id = add_test_comment(editor, key, "To be removed", 0, cx);
assert_eq!(editor.total_review_comment_count(), 1);
let removed = editor.remove_review_comment(id, cx);
assert!(removed);
assert_eq!(editor.total_review_comment_count(), 0);
// Try to remove again
let removed_again = editor.remove_review_comment(id, cx);
assert!(!removed_again);
});
}
#[gpui::test]
fn test_review_comment_update(cx: &mut TestAppContext) {
init_test(cx, |_| {});
let editor = cx.add_window(|window, cx| Editor::single_line(window, cx));
_ = editor.update(cx, |editor: &mut Editor, _window, cx| {
let key = test_hunk_key("");
let id = add_test_comment(editor, key.clone(), "Original text", 0, cx);
let updated = editor.update_review_comment(id, "Updated text".to_string(), cx);
assert!(updated);
let snapshot = editor.buffer().read(cx).snapshot(cx);
let comments = editor.comments_for_hunk(&key, &snapshot);
assert_eq!(comments[0].comment, "Updated text");
assert!(!comments[0].is_editing); // Should clear editing flag
});
}
#[gpui::test]
fn test_review_comment_take_all(cx: &mut TestAppContext) {
init_test(cx, |_| {});
let editor = cx.add_window(|window, cx| Editor::single_line(window, cx));
_ = editor.update(cx, |editor: &mut Editor, _window, cx| {
let snapshot = editor.buffer().read(cx).snapshot(cx);
let anchor1 = snapshot.anchor_before(Point::new(0, 0));
let anchor2 = snapshot.anchor_before(Point::new(0, 0));
let key1 = test_hunk_key_with_anchor("file1.rs", anchor1);
let key2 = test_hunk_key_with_anchor("file2.rs", anchor2);
let id1 = add_test_comment(editor, key1.clone(), "Comment 1", 0, cx);
let id2 = add_test_comment(editor, key1.clone(), "Comment 2", 1, cx);
let id3 = add_test_comment(editor, key2.clone(), "Comment 3", 10, cx);
// IDs should be sequential starting from 0
assert_eq!(id1, 0);
assert_eq!(id2, 1);
assert_eq!(id3, 2);
assert_eq!(editor.total_review_comment_count(), 3);
let taken = editor.take_all_review_comments(cx);
// Should have 2 entries (one per hunk)
assert_eq!(taken.len(), 2);
// Total comments should be 3
let total: usize = taken
.iter()
.map(|(_, comments): &(DiffHunkKey, Vec<StoredReviewComment>)| comments.len())
.sum();
assert_eq!(total, 3);
// Storage should be empty
assert_eq!(editor.total_review_comment_count(), 0);
// After taking all comments, ID counter should reset
// New comments should get IDs starting from 0 again
let new_id1 = add_test_comment(editor, key1, "New Comment 1", 0, cx);
let new_id2 = add_test_comment(editor, key2, "New Comment 2", 0, cx);
assert_eq!(new_id1, 0, "ID counter should reset after take_all");
assert_eq!(new_id2, 1, "IDs should be sequential after reset");
});
}
#[gpui::test]
fn test_diff_review_overlay_show_and_dismiss(cx: &mut TestAppContext) {
init_test(cx, |_| {});
let editor = cx.add_window(|window, cx| Editor::single_line(window, cx));
// Show overlay
editor
.update(cx, |editor, window, cx| {
editor.show_diff_review_overlay(DisplayRow(0), window, cx);
})
.unwrap();
// Verify overlay is shown
editor
.update(cx, |editor, _window, _cx| {
assert!(!editor.diff_review_overlays.is_empty());
assert_eq!(editor.diff_review_display_row(), Some(DisplayRow(0)));
assert!(editor.diff_review_prompt_editor().is_some());
})
.unwrap();
// Dismiss overlay
editor
.update(cx, |editor, _window, cx| {
editor.dismiss_all_diff_review_overlays(cx);
})
.unwrap();
// Verify overlay is dismissed
editor
.update(cx, |editor, _window, _cx| {
assert!(editor.diff_review_overlays.is_empty());
assert_eq!(editor.diff_review_display_row(), None);
assert!(editor.diff_review_prompt_editor().is_none());
})
.unwrap();
}
#[gpui::test]
fn test_diff_review_overlay_dismiss_via_cancel(cx: &mut TestAppContext) {
init_test(cx, |_| {});
let editor = cx.add_window(|window, cx| Editor::single_line(window, cx));
// Show overlay
editor
.update(cx, |editor, window, cx| {
editor.show_diff_review_overlay(DisplayRow(0), window, cx);
})
.unwrap();
// Verify overlay is shown
editor
.update(cx, |editor, _window, _cx| {
assert!(!editor.diff_review_overlays.is_empty());
})
.unwrap();
// Dismiss via dismiss_menus_and_popups (which is called by cancel action)
editor
.update(cx, |editor, window, cx| {
editor.dismiss_menus_and_popups(true, window, cx);
})
.unwrap();
// Verify overlay is dismissed
editor
.update(cx, |editor, _window, _cx| {
assert!(editor.diff_review_overlays.is_empty());
})
.unwrap();
}
#[gpui::test]
fn test_diff_review_empty_comment_not_submitted(cx: &mut TestAppContext) {
init_test(cx, |_| {});
let editor = cx.add_window(|window, cx| Editor::single_line(window, cx));
// Show overlay
editor
.update(cx, |editor, window, cx| {
editor.show_diff_review_overlay(DisplayRow(0), window, cx);
})
.unwrap();
// Try to submit without typing anything (empty comment)
editor
.update(cx, |editor, window, cx| {
editor.submit_diff_review_comment(window, cx);
})
.unwrap();
// Verify no comment was added
editor
.update(cx, |editor, _window, _cx| {
assert_eq!(editor.total_review_comment_count(), 0);
})
.unwrap();
// Try to submit with whitespace-only comment
editor
.update(cx, |editor, window, cx| {
if let Some(prompt_editor) = editor.diff_review_prompt_editor().cloned() {
prompt_editor.update(cx, |pe, cx| {
pe.insert(" \n\t ", window, cx);
});
}
editor.submit_diff_review_comment(window, cx);
})
.unwrap();
// Verify still no comment was added
editor
.update(cx, |editor, _window, _cx| {
assert_eq!(editor.total_review_comment_count(), 0);
})
.unwrap();
}
#[gpui::test]
fn test_diff_review_inline_edit_flow(cx: &mut TestAppContext) {
init_test(cx, |_| {});
let editor = cx.add_window(|window, cx| Editor::single_line(window, cx));
// Add a comment directly
let comment_id = editor
.update(cx, |editor, _window, cx| {
let key = test_hunk_key("");
add_test_comment(editor, key, "Original comment", 0, cx)
})
.unwrap();
// Set comment to editing mode
editor
.update(cx, |editor, _window, cx| {
editor.set_comment_editing(comment_id, true, cx);
})
.unwrap();
// Verify editing flag is set
editor
.update(cx, |editor, _window, cx| {
let key = test_hunk_key("");
let snapshot = editor.buffer().read(cx).snapshot(cx);
let comments = editor.comments_for_hunk(&key, &snapshot);
assert_eq!(comments.len(), 1);
assert!(comments[0].is_editing);
})
.unwrap();
// Update the comment
editor
.update(cx, |editor, _window, cx| {
let updated =
editor.update_review_comment(comment_id, "Updated comment".to_string(), cx);
assert!(updated);
})
.unwrap();
// Verify comment was updated and editing flag is cleared
editor
.update(cx, |editor, _window, cx| {
let key = test_hunk_key("");
let snapshot = editor.buffer().read(cx).snapshot(cx);
let comments = editor.comments_for_hunk(&key, &snapshot);
assert_eq!(comments[0].comment, "Updated comment");
assert!(!comments[0].is_editing);
})
.unwrap();
}
#[gpui::test]
fn test_orphaned_comments_are_cleaned_up(cx: &mut TestAppContext) {
init_test(cx, |_| {});
// Create an editor with some text
let editor = cx.add_window(|window, cx| {
let buffer = cx.new(|cx| Buffer::local("line 1\nline 2\nline 3\n", cx));
let multi_buffer = cx.new(|cx| MultiBuffer::singleton(buffer, cx));
Editor::new(EditorMode::full(), multi_buffer, None, window, cx)
});
// Add a comment with an anchor on line 2
editor
.update(cx, |editor, _window, cx| {
let snapshot = editor.buffer().read(cx).snapshot(cx);
let anchor = snapshot.anchor_after(Point::new(1, 0)); // Line 2
let key = DiffHunkKey {
file_path: Arc::from(util::rel_path::RelPath::empty()),
hunk_start_anchor: anchor,
};
editor.add_review_comment(
key,
"Comment on line 2".to_string(),
DisplayRow(1),
anchor..anchor,
cx,
);
assert_eq!(editor.total_review_comment_count(), 1);
})
.unwrap();
// Delete all content (this should orphan the comment's anchor)
editor
.update(cx, |editor, window, cx| {
editor.select_all(&SelectAll, window, cx);
editor.insert("completely new content", window, cx);
})
.unwrap();
// Trigger cleanup
editor
.update(cx, |editor, _window, cx| {
editor.cleanup_orphaned_review_comments(cx);
// Comment should be removed because its anchor is invalid
assert_eq!(editor.total_review_comment_count(), 0);
})
.unwrap();
}
#[gpui::test]
fn test_orphaned_comments_cleanup_called_on_buffer_edit(cx: &mut TestAppContext) {
init_test(cx, |_| {});
// Create an editor with some text
let editor = cx.add_window(|window, cx| {
let buffer = cx.new(|cx| Buffer::local("line 1\nline 2\nline 3\n", cx));
let multi_buffer = cx.new(|cx| MultiBuffer::singleton(buffer, cx));
Editor::new(EditorMode::full(), multi_buffer, None, window, cx)
});
// Add a comment with an anchor on line 2
editor
.update(cx, |editor, _window, cx| {
let snapshot = editor.buffer().read(cx).snapshot(cx);
let anchor = snapshot.anchor_after(Point::new(1, 0)); // Line 2
let key = DiffHunkKey {
file_path: Arc::from(util::rel_path::RelPath::empty()),
hunk_start_anchor: anchor,
};
editor.add_review_comment(
key,
"Comment on line 2".to_string(),
DisplayRow(1),
anchor..anchor,
cx,
);
assert_eq!(editor.total_review_comment_count(), 1);
})
.unwrap();
// Edit the buffer - this should trigger cleanup via on_buffer_event
// Delete all content which orphans the anchor
editor
.update(cx, |editor, window, cx| {
editor.select_all(&SelectAll, window, cx);
editor.insert("completely new content", window, cx);
// The cleanup is called automatically in on_buffer_event when Edited fires
})
.unwrap();
// Verify cleanup happened automatically (not manually triggered)
editor
.update(cx, |editor, _window, _cx| {
// Comment should be removed because its anchor became invalid
// and cleanup was called automatically on buffer edit
assert_eq!(editor.total_review_comment_count(), 0);
})
.unwrap();
}
#[gpui::test]
fn test_comments_stored_for_multiple_hunks(cx: &mut TestAppContext) {
init_test(cx, |_| {});
// This test verifies that comments can be stored for multiple different hunks
// and that hunk_comment_count correctly identifies comments per hunk.
let editor = cx.add_window(|window, cx| Editor::single_line(window, cx));
_ = editor.update(cx, |editor, _window, cx| {
let snapshot = editor.buffer().read(cx).snapshot(cx);
// Create two different hunk keys (simulating two different files)
let anchor = snapshot.anchor_before(Point::new(0, 0));
let key1 = DiffHunkKey {
file_path: Arc::from(util::rel_path::RelPath::unix("file1.rs").unwrap()),
hunk_start_anchor: anchor,
};
let key2 = DiffHunkKey {
file_path: Arc::from(util::rel_path::RelPath::unix("file2.rs").unwrap()),
hunk_start_anchor: anchor,
};
// Add comments to first hunk
editor.add_review_comment(
key1.clone(),
"Comment 1 for file1".to_string(),
DisplayRow(0),
anchor..anchor,
cx,
);
editor.add_review_comment(
key1.clone(),
"Comment 2 for file1".to_string(),
DisplayRow(1),
anchor..anchor,
cx,
);
// Add comment to second hunk
editor.add_review_comment(
key2.clone(),
"Comment for file2".to_string(),
DisplayRow(0),
anchor..anchor,
cx,
);
// Verify total count
assert_eq!(editor.total_review_comment_count(), 3);
// Verify per-hunk counts
let snapshot = editor.buffer().read(cx).snapshot(cx);
assert_eq!(
editor.hunk_comment_count(&key1, &snapshot),
2,
"file1 should have 2 comments"
);
assert_eq!(
editor.hunk_comment_count(&key2, &snapshot),
1,
"file2 should have 1 comment"
);
// Verify comments_for_hunk returns correct comments
let file1_comments = editor.comments_for_hunk(&key1, &snapshot);
assert_eq!(file1_comments.len(), 2);
assert_eq!(file1_comments[0].comment, "Comment 1 for file1");
assert_eq!(file1_comments[1].comment, "Comment 2 for file1");
let file2_comments = editor.comments_for_hunk(&key2, &snapshot);
assert_eq!(file2_comments.len(), 1);
assert_eq!(file2_comments[0].comment, "Comment for file2");
});
}
#[gpui::test]
fn test_same_hunk_detected_by_matching_keys(cx: &mut TestAppContext) {
init_test(cx, |_| {});
// This test verifies that hunk_keys_match correctly identifies when two
// DiffHunkKeys refer to the same hunk (same file path and anchor point).
let editor = cx.add_window(|window, cx| Editor::single_line(window, cx));
_ = editor.update(cx, |editor, _window, cx| {
let snapshot = editor.buffer().read(cx).snapshot(cx);
let anchor = snapshot.anchor_before(Point::new(0, 0));
// Create two keys with the same file path and anchor
let key1 = DiffHunkKey {
file_path: Arc::from(util::rel_path::RelPath::unix("file.rs").unwrap()),
hunk_start_anchor: anchor,
};
let key2 = DiffHunkKey {
file_path: Arc::from(util::rel_path::RelPath::unix("file.rs").unwrap()),
hunk_start_anchor: anchor,
};
// Add comment to first key
editor.add_review_comment(
key1,
"Test comment".to_string(),
DisplayRow(0),
anchor..anchor,
cx,
);
// Verify second key (same hunk) finds the comment
let snapshot = editor.buffer().read(cx).snapshot(cx);
assert_eq!(
editor.hunk_comment_count(&key2, &snapshot),
1,
"Same hunk should find the comment"
);
// Create a key with different file path
let different_file_key = DiffHunkKey {
file_path: Arc::from(util::rel_path::RelPath::unix("other.rs").unwrap()),
hunk_start_anchor: anchor,
};
// Different file should not find the comment
assert_eq!(
editor.hunk_comment_count(&different_file_key, &snapshot),
0,
"Different file should not find the comment"
);
});
}
#[gpui::test]
fn test_overlay_comments_expanded_state(cx: &mut TestAppContext) {
init_test(cx, |_| {});
// This test verifies that set_diff_review_comments_expanded correctly
// updates the expanded state of overlays.
let editor = cx.add_window(|window, cx| Editor::single_line(window, cx));
// Show overlay
editor
.update(cx, |editor, window, cx| {
editor.show_diff_review_overlay(DisplayRow(0), window, cx);
})
.unwrap();
// Verify initially expanded (default)
editor
.update(cx, |editor, _window, _cx| {
assert!(
editor.diff_review_overlays[0].comments_expanded,
"Should be expanded by default"
);
})
.unwrap();
// Set to collapsed using the public method
editor
.update(cx, |editor, _window, cx| {
editor.set_diff_review_comments_expanded(false, cx);
})
.unwrap();
// Verify collapsed
editor
.update(cx, |editor, _window, _cx| {
assert!(
!editor.diff_review_overlays[0].comments_expanded,
"Should be collapsed after setting to false"
);
})
.unwrap();
// Set back to expanded
editor
.update(cx, |editor, _window, cx| {
editor.set_diff_review_comments_expanded(true, cx);
})
.unwrap();
// Verify expanded again
editor
.update(cx, |editor, _window, _cx| {
assert!(
editor.diff_review_overlays[0].comments_expanded,
"Should be expanded after setting to true"
);
})
.unwrap();
}
#[gpui::test]
fn test_calculate_overlay_height(cx: &mut TestAppContext) {
init_test(cx, |_| {});
// This test verifies that calculate_overlay_height returns correct heights
// based on comment count and expanded state.
let editor = cx.add_window(|window, cx| Editor::single_line(window, cx));
_ = editor.update(cx, |editor, _window, cx| {
let snapshot = editor.buffer().read(cx).snapshot(cx);
let anchor = snapshot.anchor_before(Point::new(0, 0));
let key = DiffHunkKey {
file_path: Arc::from(util::rel_path::RelPath::empty()),
hunk_start_anchor: anchor,
};
// No comments: base height of 2
let height_no_comments = editor.calculate_overlay_height(&key, true, &snapshot);
assert_eq!(
height_no_comments, 2,
"Base height should be 2 with no comments"
);
// Add one comment
editor.add_review_comment(
key.clone(),
"Comment 1".to_string(),
DisplayRow(0),
anchor..anchor,
cx,
);
let snapshot = editor.buffer().read(cx).snapshot(cx);
// With comments expanded: base (2) + header (1) + 2 per comment
let height_expanded = editor.calculate_overlay_height(&key, true, &snapshot);
assert_eq!(
height_expanded,
2 + 1 + 2, // base + header + 1 comment * 2
"Height with 1 comment expanded"
);
// With comments collapsed: base (2) + header (1)
let height_collapsed = editor.calculate_overlay_height(&key, false, &snapshot);
assert_eq!(
height_collapsed,
2 + 1, // base + header only
"Height with comments collapsed"
);
// Add more comments
editor.add_review_comment(
key.clone(),
"Comment 2".to_string(),
DisplayRow(0),
anchor..anchor,
cx,
);
editor.add_review_comment(
key.clone(),
"Comment 3".to_string(),
DisplayRow(0),
anchor..anchor,
cx,
);
let snapshot = editor.buffer().read(cx).snapshot(cx);
// With 3 comments expanded
let height_3_expanded = editor.calculate_overlay_height(&key, true, &snapshot);
assert_eq!(
height_3_expanded,
2 + 1 + (3 * 2), // base + header + 3 comments * 2
"Height with 3 comments expanded"
);
// Collapsed height stays the same regardless of comment count
let height_3_collapsed = editor.calculate_overlay_height(&key, false, &snapshot);
assert_eq!(
height_3_collapsed,
2 + 1, // base + header only
"Height with 3 comments collapsed should be same as 1 comment collapsed"
);
});
}
#[gpui::test]
async fn test_move_to_start_end_of_larger_syntax_node_single_cursor(cx: &mut TestAppContext) {
init_test(cx, |_| {});
+12 -21
View File
@@ -504,6 +504,12 @@ impl EditorElement {
register_action(editor, window, Editor::unstage_and_next);
register_action(editor, window, Editor::expand_all_diff_hunks);
register_action(editor, window, Editor::collapse_all_diff_hunks);
register_action(editor, window, Editor::toggle_review_comments_expanded);
register_action(editor, window, Editor::submit_diff_review_comment_action);
register_action(editor, window, Editor::edit_review_comment);
register_action(editor, window, Editor::delete_review_comment);
register_action(editor, window, Editor::confirm_edit_review_comment_action);
register_action(editor, window, Editor::cancel_edit_review_comment_action);
register_action(editor, window, Editor::go_to_previous_change);
register_action(editor, window, Editor::go_to_next_change);
register_action(editor, window, Editor::go_to_prev_reference);
@@ -3116,26 +3122,6 @@ impl EditorElement {
Some((display_row, buffer_row))
}
fn diff_review_button(width: Pixels, cx: &mut App) -> AnyElement {
let text_c = cx.theme().colors().text;
let icon_c = cx.theme().colors().icon_accent;
h_flex()
.id("diff_review_button")
.cursor_pointer()
.w(width - px(1.))
.h(relative(0.9))
.justify_center()
.rounded_sm()
.border_1()
.border_color(text_c.opacity(0.1))
.bg(text_c.opacity(0.15))
.hover(|s| s.bg(icon_c.opacity(0.4)).border_color(icon_c.opacity(0.5)))
.child(Icon::new(IconName::Plus).size(IconSize::Small))
.tooltip(Tooltip::text("Add Review"))
.into_any_element()
}
#[allow(clippy::too_many_arguments)]
fn layout_run_indicators(
&self,
@@ -10460,8 +10446,13 @@ impl Element for EditorElement {
available_width + em_width - px(6.)
};
let button = self.editor.update(cx, |editor, cx| {
editor
.render_diff_review_button(display_row, button_width, cx)
.into_any_element()
});
prepaint_gutter_button(
Self::diff_review_button(button_width, cx),
button,
display_row,
line_height,
&gutter_dimensions,
+47 -4
View File
@@ -9,7 +9,7 @@ use buffer_diff::{BufferDiff, DiffHunkSecondaryStatus};
use collections::{HashMap, HashSet};
use editor::{
Addon, Editor, EditorEvent, SelectionEffects, SplittableEditor,
actions::{GoToHunk, GoToPreviousHunk},
actions::{GoToHunk, GoToPreviousHunk, SendReviewToAgent},
multibuffer_context_lines,
scroll::Autoscroll,
};
@@ -70,6 +70,7 @@ pub struct ProjectDiff {
workspace: WeakEntity<Workspace>,
focus_handle: FocusHandle,
pending_scroll: Option<PathKey>,
review_comment_count: usize,
_task: Task<Result<()>>,
_subscription: Subscription,
}
@@ -311,8 +312,16 @@ impl ProjectDiff {
});
diff_display_editor
});
cx.subscribe_in(&editor, window, Self::handle_editor_event)
.detach();
let editor_subscription = cx.subscribe_in(&editor, window, Self::handle_editor_event);
let primary_editor = editor.read(cx).primary_editor().clone();
let review_comment_subscription =
cx.subscribe(&primary_editor, |this, _editor, event: &EditorEvent, cx| {
if let EditorEvent::ReviewCommentsChanged { total_count } = event {
this.review_comment_count = *total_count;
cx.notify();
}
});
let branch_diff_subscription = cx.subscribe_in(
&branch_diff,
@@ -363,8 +372,12 @@ impl ProjectDiff {
multibuffer,
buffer_diff_subscriptions: Default::default(),
pending_scroll: None,
review_comment_count: 0,
_task: task,
_subscription: branch_diff_subscription,
_subscription: Subscription::join(
branch_diff_subscription,
Subscription::join(editor_subscription, review_comment_subscription),
),
}
}
@@ -453,6 +466,16 @@ impl ProjectDiff {
}
}
/// Returns the total count of review comments across all hunks/files.
pub fn total_review_comment_count(&self) -> usize {
self.review_comment_count
}
/// Returns a reference to the splittable editor.
pub fn editor(&self) -> &Entity<SplittableEditor> {
&self.editor
}
fn button_states(&self, cx: &App) -> ButtonStates {
let editor = self.editor.read(cx).primary_editor().read(cx);
let snapshot = self.multibuffer.read(cx).snapshot(cx);
@@ -1278,6 +1301,7 @@ impl Render for ProjectDiffToolbar {
};
let focus_handle = project_diff.focus_handle(cx);
let button_states = project_diff.read(cx).button_states(cx);
let review_count = project_diff.read(cx).total_review_comment_count();
h_group_xl()
.my_neg_1()
@@ -1419,6 +1443,25 @@ impl Render for ProjectDiffToolbar {
})),
),
)
// "Send Review to Agent" button (only shown when there are review comments)
.when(review_count > 0, |el| {
el.child(vertical_divider()).child(
Button::new(
"send-review",
format!("Send Review to Agent ({})", review_count),
)
.icon(IconName::ZedAssistant)
.icon_position(IconPosition::Start)
.tooltip(Tooltip::for_action_title_in(
"Send all review comments to the Agent panel",
&SendReviewToAgent,
&focus_handle,
))
.on_click(cx.listener(|this, _, window, cx| {
this.dispatch_action(&SendReviewToAgent, window, cx)
})),
)
})
}
}
+214 -6
View File
@@ -50,6 +50,7 @@ use {
agent_servers::{AgentServer, AgentServerDelegate},
anyhow::{Context as _, Result},
assets::Assets,
editor::display_map::DisplayRow,
feature_flags::FeatureFlagAppExt as _,
git_ui::project_diff::ProjectDiff,
gpui::{
@@ -1358,6 +1359,198 @@ import { AiPaneTabContext } from 'context';
update_baseline,
)?;
// Test 4: Show the diff review overlay on the regular editor
regular_window
.update(cx, |workspace, window, cx| {
// Get the first editor from the workspace
let editors: Vec<_> = workspace.items_of_type::<editor::Editor>(cx).collect();
if let Some(editor) = editors.into_iter().next() {
editor.update(cx, |editor, cx| {
editor.show_diff_review_overlay(DisplayRow(1), window, cx);
});
}
})
.ok();
// Wait for overlay to render
for _ in 0..3 {
cx.advance_clock(Duration::from_millis(100));
cx.run_until_parked();
}
// Refresh window
cx.update_window(regular_window.into(), |_, window, _cx| {
window.refresh();
})?;
cx.run_until_parked();
// Capture Test 4: Regular editor with overlay shown
let test4_result = run_visual_test(
"diff_review_overlay_shown",
regular_window.into(),
cx,
update_baseline,
)?;
// Test 5: Type text into the diff review prompt and submit it
// First, get the prompt editor from the overlay and type some text
regular_window
.update(cx, |workspace, window, cx| {
let editors: Vec<_> = workspace.items_of_type::<editor::Editor>(cx).collect();
if let Some(editor) = editors.into_iter().next() {
editor.update(cx, |editor, cx| {
// Get the prompt editor from the overlay and insert text
if let Some(prompt_editor) = editor.diff_review_prompt_editor().cloned() {
prompt_editor.update(cx, |prompt_editor: &mut editor::Editor, cx| {
prompt_editor.insert(
"This change needs better error handling",
window,
cx,
);
});
}
});
}
})
.ok();
// Wait for text to be inserted
for _ in 0..3 {
cx.advance_clock(Duration::from_millis(100));
cx.run_until_parked();
}
// Refresh window
cx.update_window(regular_window.into(), |_, window, _cx| {
window.refresh();
})?;
cx.run_until_parked();
// Capture Test 5: Diff review overlay with typed text
let test5_result = run_visual_test(
"diff_review_overlay_with_text",
regular_window.into(),
cx,
update_baseline,
)?;
// Test 6: Submit a comment to store it locally
regular_window
.update(cx, |workspace, window, cx| {
let editors: Vec<_> = workspace.items_of_type::<editor::Editor>(cx).collect();
if let Some(editor) = editors.into_iter().next() {
editor.update(cx, |editor, cx| {
// Submit the comment that was typed in test 5
editor.submit_diff_review_comment(window, cx);
});
}
})
.ok();
// Wait for comment to be stored
for _ in 0..3 {
cx.advance_clock(Duration::from_millis(100));
cx.run_until_parked();
}
// Refresh window
cx.update_window(regular_window.into(), |_, window, _cx| {
window.refresh();
})?;
cx.run_until_parked();
// Capture Test 6: Overlay with one stored comment
let test6_result = run_visual_test(
"diff_review_one_comment",
regular_window.into(),
cx,
update_baseline,
)?;
// Test 7: Add more comments to show multiple comments expanded
regular_window
.update(cx, |workspace, window, cx| {
let editors: Vec<_> = workspace.items_of_type::<editor::Editor>(cx).collect();
if let Some(editor) = editors.into_iter().next() {
editor.update(cx, |editor, cx| {
// Add second comment
if let Some(prompt_editor) = editor.diff_review_prompt_editor().cloned() {
prompt_editor.update(cx, |pe, cx| {
pe.insert("Second comment about imports", window, cx);
});
}
editor.submit_diff_review_comment(window, cx);
// Add third comment
if let Some(prompt_editor) = editor.diff_review_prompt_editor().cloned() {
prompt_editor.update(cx, |pe, cx| {
pe.insert("Third comment about naming conventions", window, cx);
});
}
editor.submit_diff_review_comment(window, cx);
});
}
})
.ok();
// Wait for comments to be stored
for _ in 0..3 {
cx.advance_clock(Duration::from_millis(100));
cx.run_until_parked();
}
// Refresh window
cx.update_window(regular_window.into(), |_, window, _cx| {
window.refresh();
})?;
cx.run_until_parked();
// Capture Test 7: Overlay with multiple comments expanded
let test7_result = run_visual_test(
"diff_review_multiple_comments_expanded",
regular_window.into(),
cx,
update_baseline,
)?;
// Test 8: Collapse the comments section
regular_window
.update(cx, |workspace, _window, cx| {
let editors: Vec<_> = workspace.items_of_type::<editor::Editor>(cx).collect();
if let Some(editor) = editors.into_iter().next() {
editor.update(cx, |editor, cx| {
// Toggle collapse using the public method
editor.set_diff_review_comments_expanded(false, cx);
});
}
})
.ok();
// Wait for UI to update
for _ in 0..3 {
cx.advance_clock(Duration::from_millis(100));
cx.run_until_parked();
}
// Refresh window
cx.update_window(regular_window.into(), |_, window, _cx| {
window.refresh();
})?;
cx.run_until_parked();
// Capture Test 8: Comments collapsed
let test8_result = run_visual_test(
"diff_review_comments_collapsed",
regular_window.into(),
cx,
update_baseline,
)?;
// Clean up: remove worktrees to stop background scanning
workspace_window
.update(cx, |workspace, _window, cx| {
@@ -1391,12 +1584,27 @@ import { AiPaneTabContext } from 'context';
}
// Return combined result
match (&test1_result, &test2_result, &test3_result) {
(TestResult::Passed, TestResult::Passed, TestResult::Passed) => Ok(TestResult::Passed),
(TestResult::BaselineUpdated(p), _, _)
| (_, TestResult::BaselineUpdated(p), _)
| (_, _, TestResult::BaselineUpdated(p)) => Ok(TestResult::BaselineUpdated(p.clone())),
}
let all_results = [
&test1_result,
&test2_result,
&test3_result,
&test4_result,
&test5_result,
&test6_result,
&test7_result,
&test8_result,
];
// Combine results: if any test updated a baseline, return BaselineUpdated;
// otherwise return Passed. The exhaustive match ensures the compiler
// verifies we handle all TestResult variants.
let result = all_results
.iter()
.fold(TestResult::Passed, |acc, r| match r {
TestResult::Passed => acc,
TestResult::BaselineUpdated(p) => TestResult::BaselineUpdated(p.clone()),
});
Ok(result)
}
/// A stub AgentServer for visual testing that returns a pre-programmed connection.