From 2258cd36f348aafaede5a1d280dd72e837856463 Mon Sep 17 00:00:00 2001 From: lucieleblanc Date: Mon, 4 May 2026 16:43:38 -0400 Subject: [PATCH] Handle agent management view updates based on event type (#9866) Co-authored-by: Oz --- PR_DESCRIPTION.md | 30 ------------ app/src/ai/agent_conversations_model.rs | 36 +++++++++------ app/src/ai/agent_conversations_model_tests.rs | 46 ++++++------------- app/src/ai/agent_management/view.rs | 43 ++++++++++++++--- 4 files changed, 74 insertions(+), 81 deletions(-) delete mode 100644 PR_DESCRIPTION.md diff --git a/PR_DESCRIPTION.md b/PR_DESCRIPTION.md deleted file mode 100644 index 7a4af65c..00000000 --- a/PR_DESCRIPTION.md +++ /dev/null @@ -1,30 +0,0 @@ -## Description - -The "Runs" agent management view rebuilds its entire card list whenever any conversation's status changes. Most of those events are visually no-ops (re-emitted statuses or status changes that don't cross the active filter), but subscribers can't tell because the event carries no detail. - -This PR is the first of two and only changes the shape of the conversation status update event. Subscribers continue to behave exactly as before. The follow-up PR uses the new payload to skip rebuilds in the common cases. - -Concretely: -- `BlocklistAIHistoryEvent::UpdatedConversationStatus` now carries the previous and new `ConversationStatus`. Restoration events report `prev_status: None` since restoration doesn't change the underlying status. -- `AgentConversationsModelEvent::ConversationUpdated` now carries a `conversation_id` and a typed `ConversationUpdateKind` of either `Restored` or `StatusSet { prev_filter, new_filter }`. Filter buckets are precomputed at emission time so subscribers don't have to recompute membership. -- A small helper `conversation_status_filter` maps `ConversationStatus` to its `StatusFilter` bucket. -- All existing subscribers were updated to the new event shape with `{ .. }` patterns and ignore the new fields. The variant is annotated with `#[allow(dead_code)]` until the follow-up PR lands. - -## Testing - -- Added/updated four unit tests on the model: `Restored` emission, `StatusSet` transitions across filter buckets, same-bucket re-emission, and the `ConversationStatus → StatusFilter` mapping. -- Existing `agent_conversations_model` tests pass against the new payload. -- `cargo check`, `cargo fmt`, `cargo clippy` all clean. -- No user-visible behavior change, so no manual smoke test needed. - -## Server API dependencies - -No server dependencies — purely client-side event-shape changes. - -## Agent Mode - -- [x] Warp Agent Mode - This PR was created via Warp's AI Agent Mode - -## Changelog Entries for Stable - -(No changelog entry — no user-visible behavior change. The follow-up PR carries the changelog entry.) diff --git a/app/src/ai/agent_conversations_model.rs b/app/src/ai/agent_conversations_model.rs index 06e7428e..d102b40a 100644 --- a/app/src/ai/agent_conversations_model.rs +++ b/app/src/ai/agent_conversations_model.rs @@ -101,6 +101,25 @@ pub enum StatusFilter { Failed, } +impl StatusFilter { + /// Returns `true` if a status transition from `prev_bucket` to `new_bucket` flips + /// whether an item is included by this filter. `All` matches every bucket so it + /// is never crossed; the other variants are crossed when exactly one of the buckets + /// equals this filter. + pub(crate) fn is_membership_crossed( + self, + prev_bucket: StatusFilter, + new_bucket: StatusFilter, + ) -> bool { + match self { + StatusFilter::All => false, + StatusFilter::Working | StatusFilter::Done | StatusFilter::Failed => { + (prev_bucket == self) != (new_bucket == self) + } + } + } +} + #[derive(Clone, PartialEq, Eq, Debug, Default, Serialize, Deserialize)] pub enum SourceFilter { #[default] @@ -871,12 +890,7 @@ pub enum AgentConversationsModelEvent { /// Existing task data may have been updated (e.g., state changes). TasksUpdated, /// Conversation status data was updated - ConversationUpdated { - #[allow(dead_code)] - conversation_id: AIConversationId, - #[allow(dead_code)] - kind: ConversationUpdateKind, - }, + ConversationUpdated { kind: ConversationUpdateKind }, /// Conversation artifacts were updated (plans, PRs, etc.) ConversationArtifactsUpdated { conversation_id: AIConversationId }, } @@ -1454,10 +1468,7 @@ impl AgentConversationsModel { // Status changes - just trigger re-render since status is looked up at render time BlocklistAIHistoryEvent::UpdatedConversationStatus { - conversation_id, - update, - new_status, - .. + update, new_status, .. } => { let kind = match update { ConversationStatusUpdate::Restored => ConversationUpdateKind::Restored, @@ -1472,10 +1483,7 @@ impl AgentConversationsModel { } } }; - ctx.emit(AgentConversationsModelEvent::ConversationUpdated { - conversation_id: *conversation_id, - kind, - }); + ctx.emit(AgentConversationsModelEvent::ConversationUpdated { kind }); } // Artifact changes - sync live artifacts into the cached task and notify. diff --git a/app/src/ai/agent_conversations_model_tests.rs b/app/src/ai/agent_conversations_model_tests.rs index be1d288c..ec16df0e 100644 --- a/app/src/ai/agent_conversations_model_tests.rs +++ b/app/src/ai/agent_conversations_model_tests.rs @@ -62,7 +62,7 @@ fn create_test_task( } } -type CapturedConversationUpdate = Mutex>; +type CapturedConversationUpdate = Mutex>; /// Test-only handler that mirrors the production view subscription: extracts the /// `ConversationUpdated` payload and stashes it on a shared cell that test cases assert @@ -71,12 +71,8 @@ fn handle_agent_conversation_model_event( captured: &CapturedConversationUpdate, event: &AgentConversationsModelEvent, ) { - if let AgentConversationsModelEvent::ConversationUpdated { - conversation_id, - kind, - } = event - { - *captured.lock() = Some((*conversation_id, *kind)); + if let AgentConversationsModelEvent::ConversationUpdated { kind } = event { + *captured.lock() = Some(*kind); } } @@ -105,11 +101,10 @@ fn test_restored_conversation_emits_restored_kind() { let agent_model = app.add_singleton_model(|_| create_test_model()); let captured = subscribe_to_conversation_updated(&mut app, &agent_model); - let conversation_id = AIConversationId::new(); agent_model.update(&mut app, |model, ctx| { model.handle_history_event( &BlocklistAIHistoryEvent::UpdatedConversationStatus { - conversation_id, + conversation_id: AIConversationId::new(), terminal_view_id: EntityId::new(), update: ConversationStatusUpdate::Restored, new_status: ConversationStatus::Success, @@ -119,10 +114,7 @@ fn test_restored_conversation_emits_restored_kind() { }); let captured = *captured.lock(); - assert_eq!( - captured, - Some((conversation_id, ConversationUpdateKind::Restored)), - ); + assert_eq!(captured, Some(ConversationUpdateKind::Restored)); }); } @@ -134,11 +126,10 @@ fn test_status_transition_emits_status_set_with_filter_buckets() { let agent_model = app.add_singleton_model(|_| create_test_model()); let captured = subscribe_to_conversation_updated(&mut app, &agent_model); - let conversation_id = AIConversationId::new(); agent_model.update(&mut app, |model, ctx| { model.handle_history_event( &BlocklistAIHistoryEvent::UpdatedConversationStatus { - conversation_id, + conversation_id: AIConversationId::new(), terminal_view_id: EntityId::new(), update: ConversationStatusUpdate::Changed { prev_status: ConversationStatus::InProgress, @@ -152,13 +143,10 @@ fn test_status_transition_emits_status_set_with_filter_buckets() { let captured = *captured.lock(); assert_eq!( captured, - Some(( - conversation_id, - ConversationUpdateKind::StatusSet { - prev_filter: StatusFilter::Working, - new_filter: StatusFilter::Done, - }, - )), + Some(ConversationUpdateKind::StatusSet { + prev_filter: StatusFilter::Working, + new_filter: StatusFilter::Done, + }), ); }); } @@ -171,11 +159,10 @@ fn test_same_bucket_re_emission_emits_status_set_with_equal_filters() { let agent_model = app.add_singleton_model(|_| create_test_model()); let captured = subscribe_to_conversation_updated(&mut app, &agent_model); - let conversation_id = AIConversationId::new(); agent_model.update(&mut app, |model, ctx| { model.handle_history_event( &BlocklistAIHistoryEvent::UpdatedConversationStatus { - conversation_id, + conversation_id: AIConversationId::new(), terminal_view_id: EntityId::new(), update: ConversationStatusUpdate::Changed { prev_status: ConversationStatus::InProgress, @@ -189,13 +176,10 @@ fn test_same_bucket_re_emission_emits_status_set_with_equal_filters() { let captured = *captured.lock(); assert_eq!( captured, - Some(( - conversation_id, - ConversationUpdateKind::StatusSet { - prev_filter: StatusFilter::Working, - new_filter: StatusFilter::Working, - }, - )), + Some(ConversationUpdateKind::StatusSet { + prev_filter: StatusFilter::Working, + new_filter: StatusFilter::Working, + }), ); }); } diff --git a/app/src/ai/agent_management/view.rs b/app/src/ai/agent_management/view.rs index 6304631f..d83e6d82 100644 --- a/app/src/ai/agent_management/view.rs +++ b/app/src/ai/agent_management/view.rs @@ -13,8 +13,8 @@ use warpui::ui_components::button::ButtonVariant; use crate::ai::agent::conversation::AIConversationId; use crate::ai::agent_conversations_model::{ AgentConversationsModel, AgentConversationsModelEvent, AgentManagementFilters, ArtifactFilter, - ConversationOrTask, CreatedOnFilter, CreatorFilter, EnvironmentFilter, HarnessFilter, - OwnerFilter, SessionStatus, SourceFilter, StatusFilter, + ConversationOrTask, ConversationUpdateKind, CreatedOnFilter, CreatorFilter, EnvironmentFilter, + HarnessFilter, OwnerFilter, SessionStatus, SourceFilter, StatusFilter, }; use crate::ai::agent_management::agent_type_selector::{ AgentType, AgentTypeSelector, AgentTypeSelectorEvent, @@ -1237,10 +1237,8 @@ impl AgentManagementView { self.refresh_details_panel_if_needed(ctx); self.get_tasks_from_model(ctx); } - AgentConversationsModelEvent::ConversationUpdated { .. } => { - self.get_tasks_from_model(ctx); - self.refresh_details_panel_if_needed(ctx); - ctx.notify(); + AgentConversationsModelEvent::ConversationUpdated { kind } => { + self.handle_conversation_updated(*kind, ctx); } AgentConversationsModelEvent::ConversationArtifactsUpdated { conversation_id } => { self.update_artifacts_for_conversation(*conversation_id, ctx); @@ -1256,6 +1254,39 @@ impl AgentManagementView { } } + /// Decide how much work a `ConversationUpdated` event requires, based on its kind and the + /// active status filter: + /// * `Restored`: the underlying status didn't change, so the visible cards don't change + /// either. Just refresh the details panel. + /// * `StatusSet` that crosses the active status filter: rebuild the + /// card list via `get_tasks_from_model`. + /// * `StatusSet` that doesn't cross the active filter (or `All` is active): + /// just refresh the details panel re-render so the status icon picks up the new value. + fn handle_conversation_updated( + &mut self, + kind: ConversationUpdateKind, + ctx: &mut ViewContext, + ) { + match kind { + ConversationUpdateKind::Restored => {} + ConversationUpdateKind::StatusSet { + prev_filter, + new_filter, + } => { + if self + .filters + .status + .is_membership_crossed(prev_filter, new_filter) + { + self.get_tasks_from_model(ctx); + } else { + ctx.notify(); + } + } + } + self.refresh_details_panel_if_needed(ctx); + } + /// Update the details panel with fresh data for the given item. fn update_details_panel_for_item( &mut self,