From 00ff89f00fc771c41c36bb577c6e50893e4a9625 Mon Sep 17 00:00:00 2001 From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com> Date: Sun, 2 Nov 2025 14:30:50 -0300 Subject: [PATCH] agent_ui: Make single file review actions match panel (#41718) When we introduced the ACP-based agent panel, the condition that the "review" | "reject" | "keep" buttons observed to be displayed got mismatched between the panel and the pane (when in the single file review scenario). In the panel, the buttons appear as soon as there are changed buffers, whereas in the pane, they appear when response generation is done. I believe that making them appear at the same time, observing the same condition, is the desired behavior. Thus, I think the panel behavior is more correct, because there are loads of times where agent response generation isn't technically done (e.g., when there's a command waiting for permission to be run) but the _file edit_ has already been performed and is in a good state to be already accepted or rejected. So, this is what this PR is doing; effectively removing the "generating" state from the agent diff, and switching to `EditorState::Reviewing` when there are changed buffers. Release Notes: - Improved agent edit single file reviews by making the "reject" and "accept" buttons appear at the same time. --- crates/agent_ui/src/agent_diff.rs | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/crates/agent_ui/src/agent_diff.rs b/crates/agent_ui/src/agent_diff.rs index a0f117b0bf30abee9d2182cf8c3fadd10099b1f0..63eb2ac49731a5e57b4eae5bf33b821b2e223c25 100644 --- a/crates/agent_ui/src/agent_diff.rs +++ b/crates/agent_ui/src/agent_diff.rs @@ -70,14 +70,6 @@ impl AgentDiffThread { } } - fn is_generating(&self, cx: &App) -> bool { - match self { - AgentDiffThread::AcpThread(thread) => { - thread.read(cx).status() == acp_thread::ThreadStatus::Generating - } - } - } - fn has_pending_edit_tool_uses(&self, cx: &App) -> bool { match self { AgentDiffThread::AcpThread(thread) => thread.read(cx).has_pending_edit_tool_calls(), @@ -970,9 +962,7 @@ impl AgentDiffToolbar { None => ToolbarItemLocation::Hidden, Some(AgentDiffToolbarItem::Pane(_)) => ToolbarItemLocation::PrimaryRight, Some(AgentDiffToolbarItem::Editor { state, .. }) => match state { - EditorState::Generating | EditorState::Reviewing => { - ToolbarItemLocation::PrimaryRight - } + EditorState::Reviewing => ToolbarItemLocation::PrimaryRight, EditorState::Idle => ToolbarItemLocation::Hidden, }, } @@ -1050,7 +1040,6 @@ impl Render for AgentDiffToolbar { let content = match state { EditorState::Idle => return Empty.into_any(), - EditorState::Generating => vec![spinner_icon], EditorState::Reviewing => vec![ h_flex() .child( @@ -1222,7 +1211,6 @@ pub struct AgentDiff { pub enum EditorState { Idle, Reviewing, - Generating, } struct WorkspaceThread { @@ -1545,15 +1533,11 @@ impl AgentDiff { multibuffer.add_diff(diff_handle.clone(), cx); }); - let new_state = if thread.is_generating(cx) { - EditorState::Generating - } else { - EditorState::Reviewing - }; + let reviewing_state = EditorState::Reviewing; let previous_state = self .reviewing_editors - .insert(weak_editor.clone(), new_state.clone()); + .insert(weak_editor.clone(), reviewing_state.clone()); if previous_state.is_none() { editor.update(cx, |editor, cx| { @@ -1566,7 +1550,9 @@ impl AgentDiff { unaffected.remove(weak_editor); } - if new_state == EditorState::Reviewing && previous_state != Some(new_state) { + if reviewing_state == EditorState::Reviewing + && previous_state != Some(reviewing_state) + { // Jump to first hunk when we enter review mode editor.update(cx, |editor, cx| { let snapshot = multibuffer.read(cx).snapshot(cx);