From bb5aa594027c6776392141dc515312fbb9403b3b Mon Sep 17 00:00:00 2001 From: lucieleblanc Date: Fri, 1 May 2026 15:03:40 -0500 Subject: [PATCH 1/2] add details to AgentConversationsModelEvent::ConversationUpdated Co-Authored-By: Oz --- app/src/ai/agent/conversation.rs | 4 + app/src/ai/agent_conversations_model.rs | 53 +++++- app/src/ai/agent_conversations_model_tests.rs | 172 +++++++++++++++--- .../agent_management_model.rs | 1 + app/src/ai/agent_management/view.rs | 2 +- app/src/ai/blocklist/history_model.rs | 8 + app/src/terminal/view.rs | 2 +- app/src/workspace/view.rs | 2 +- .../view/conversation_list/view_model.rs | 2 +- 9 files changed, 216 insertions(+), 30 deletions(-) diff --git a/app/src/ai/agent/conversation.rs b/app/src/ai/agent/conversation.rs index 05a2fbbe8..ac439f053 100644 --- a/app/src/ai/agent/conversation.rs +++ b/app/src/ai/agent/conversation.rs @@ -669,11 +669,15 @@ impl AIConversation { } else { None }; + let prev_status = self.status.clone(); + let new_status = status.clone(); self.status = status; ctx.emit(BlocklistAIHistoryEvent::UpdatedConversationStatus { conversation_id: self.id, terminal_view_id, is_restored: false, + prev_status: Some(prev_status), + new_status, }); } diff --git a/app/src/ai/agent_conversations_model.rs b/app/src/ai/agent_conversations_model.rs index 89e16f603..b9e33dabc 100644 --- a/app/src/ai/agent_conversations_model.rs +++ b/app/src/ai/agent_conversations_model.rs @@ -872,13 +872,38 @@ pub enum AgentConversationsModelEvent { /// Existing task data may have been updated (e.g., state changes). TasksUpdated, /// Conversation status data was updated - ConversationUpdated, + ConversationUpdated { + conversation_id: AIConversationId, + kind: ConversationUpdateKind, + }, /// Conversation artifacts were updated (plans, PRs, etc.) ConversationArtifactsUpdated { conversation_id: AIConversationId }, /// A task was manually opened from the management page. TaskManuallyOpened, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ConversationUpdateKind { + /// The conversation was re-loaded into a terminal view. + Restored, + /// The conversation's status was set. + StatusSet { + prev_filter: StatusFilter, + new_filter: StatusFilter, + }, +} + +/// Maps a `ConversationStatus` to its `StatusFilter` bucket. +pub(crate) fn conversation_status_filter(status: &ConversationStatus) -> StatusFilter { + match status { + ConversationStatus::InProgress => StatusFilter::Working, + ConversationStatus::Success => StatusFilter::Done, + ConversationStatus::Error + | ConversationStatus::Cancelled + | ConversationStatus::Blocked { .. } => StatusFilter::Failed, + } +} + impl Entity for AgentConversationsModel { type Event = AgentConversationsModelEvent; } @@ -1422,8 +1447,30 @@ impl AgentConversationsModel { } // Status changes - just trigger re-render since status is looked up at render time - BlocklistAIHistoryEvent::UpdatedConversationStatus { .. } => { - ctx.emit(AgentConversationsModelEvent::ConversationUpdated); + BlocklistAIHistoryEvent::UpdatedConversationStatus { + conversation_id, + is_restored, + prev_status, + new_status, + .. + } => { + let kind = if *is_restored { + ConversationUpdateKind::Restored + } else { + let new_filter = conversation_status_filter(new_status); + let prev_filter = prev_status + .as_ref() + .map(conversation_status_filter) + .unwrap_or(new_filter); + ConversationUpdateKind::StatusSet { + prev_filter, + new_filter, + } + }; + ctx.emit(AgentConversationsModelEvent::ConversationUpdated { + conversation_id: *conversation_id, + 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 3042520a3..db5bc1a2e 100644 --- a/app/src/ai/agent_conversations_model_tests.rs +++ b/app/src/ai/agent_conversations_model_tests.rs @@ -1,15 +1,10 @@ use chrono::{DateTime, Duration, Utc}; use instant::Instant; +use parking_lot::Mutex; use persistence::model::AgentConversationData; -use std::{ - collections::HashMap, - sync::{ - atomic::{AtomicBool, Ordering}, - Arc, - }, -}; +use std::{collections::HashMap, sync::Arc}; use warp_core::features::FeatureFlag; -use warpui::{App, EntityId}; +use warpui::{App, EntityId, ModelHandle}; use crate::ai::agent::conversation::{AIConversation, AIConversationId, ConversationStatus}; use crate::ai::ambient_agents::task::{TaskCreatorInfo, TaskStatusMessage}; @@ -23,10 +18,10 @@ use crate::auth::AuthStateProvider; use crate::test_util::ai_agent_tasks::{create_api_task, create_message}; use super::{ - AgentConversationsModel, AgentConversationsModelEvent, AgentManagementFilters, - AgentRunDisplayStatus, ArtifactFilter, ConversationMetadata, ConversationOrTask, - EnvironmentFilter, HarnessFilter, OwnerFilter, StatusFilter, TaskFetchState, - MAX_PERSONAL_TASKS, MAX_TEAM_TASKS, + conversation_status_filter, AgentConversationsModel, AgentConversationsModelEvent, + AgentManagementFilters, AgentRunDisplayStatus, ArtifactFilter, ConversationMetadata, + ConversationOrTask, ConversationUpdateKind, EnvironmentFilter, HarnessFilter, OwnerFilter, + StatusFilter, TaskFetchState, MAX_PERSONAL_TASKS, MAX_TEAM_TASKS, }; use crate::ai::ambient_agents::task::HarnessConfig; use warp_cli::agent::Harness; @@ -65,38 +60,169 @@ fn create_test_task( } } +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 +/// against. +fn handle_agent_conversation_model_event( + captured: &CapturedConversationUpdate, + event: &AgentConversationsModelEvent, +) { + if let AgentConversationsModelEvent::ConversationUpdated { + conversation_id, + kind, + } = event + { + *captured.lock() = Some((*conversation_id, *kind)); + } +} + +/// Subscribes a [`handle_agent_conversation_model_event`] capture cell to `model` and +/// returns the cell so individual cases can assert on the most recent emission without +/// re-implementing the subscription bookkeeping. +fn subscribe_to_conversation_updated( + app: &mut App, + model: &ModelHandle, +) -> Arc { + let captured = Arc::new(Mutex::new(None)); + let captured_clone = captured.clone(); + app.update(|ctx| { + ctx.subscribe_to_model(model, move |_, event, _| { + handle_agent_conversation_model_event(&captured_clone, event); + }); + }); + captured +} + #[test] -fn test_conversation_status_update_emits_conversation_updated() { +fn test_restored_conversation_emits_restored_kind() { App::test((), |mut app| async move { let _interactive_management_guard = FeatureFlag::InteractiveConversationManagementView.override_enabled(true); let agent_model = app.add_singleton_model(|_| create_test_model()); - let saw_conversation_updated = Arc::new(AtomicBool::new(false)); + let captured = subscribe_to_conversation_updated(&mut app, &agent_model); - app.update(|ctx| { - let saw_conversation_updated = saw_conversation_updated.clone(); - ctx.subscribe_to_model(&agent_model, move |_, event, _| { - if matches!(event, AgentConversationsModelEvent::ConversationUpdated) { - saw_conversation_updated.store(true, Ordering::SeqCst); - } - }); + let conversation_id = AIConversationId::new(); + agent_model.update(&mut app, |model, ctx| { + model.handle_history_event( + &BlocklistAIHistoryEvent::UpdatedConversationStatus { + conversation_id, + terminal_view_id: EntityId::new(), + is_restored: true, + prev_status: None, + new_status: ConversationStatus::Success, + }, + ctx, + ); + }); + + let captured = *captured.lock(); + assert_eq!( + captured, + Some((conversation_id, ConversationUpdateKind::Restored)), + ); + }); +} + +#[test] +fn test_status_transition_emits_status_set_with_filter_buckets() { + App::test((), |mut app| async move { + let _interactive_management_guard = + FeatureFlag::InteractiveConversationManagementView.override_enabled(true); + 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, + terminal_view_id: EntityId::new(), + is_restored: false, + prev_status: Some(ConversationStatus::InProgress), + new_status: ConversationStatus::Success, + }, + ctx, + ); }); + let captured = *captured.lock(); + assert_eq!( + captured, + Some(( + conversation_id, + ConversationUpdateKind::StatusSet { + prev_filter: StatusFilter::Working, + new_filter: StatusFilter::Done, + }, + )), + ); + }); +} + +#[test] +fn test_same_bucket_re_emission_emits_status_set_with_equal_filters() { + App::test((), |mut app| async move { + let _interactive_management_guard = + FeatureFlag::InteractiveConversationManagementView.override_enabled(true); + 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: AIConversationId::new(), + conversation_id, terminal_view_id: EntityId::new(), is_restored: false, + prev_status: Some(ConversationStatus::InProgress), + new_status: ConversationStatus::InProgress, }, ctx, ); }); - assert!(saw_conversation_updated.load(Ordering::SeqCst)); + let captured = *captured.lock(); + assert_eq!( + captured, + Some(( + conversation_id, + ConversationUpdateKind::StatusSet { + prev_filter: StatusFilter::Working, + new_filter: StatusFilter::Working, + }, + )), + ); }); } +#[test] +fn test_conversation_status_filter_mapping() { + assert_eq!( + conversation_status_filter(&ConversationStatus::InProgress), + StatusFilter::Working, + ); + assert_eq!( + conversation_status_filter(&ConversationStatus::Success), + StatusFilter::Done, + ); + assert_eq!( + conversation_status_filter(&ConversationStatus::Error), + StatusFilter::Failed, + ); + assert_eq!( + conversation_status_filter(&ConversationStatus::Cancelled), + StatusFilter::Failed, + ); + assert_eq!( + conversation_status_filter(&ConversationStatus::Blocked { + blocked_action: "approve_command".to_string(), + }), + StatusFilter::Failed, + ); +} + #[test] fn test_display_status_uses_setup_task_states() { App::test((), |mut app| async move { diff --git a/app/src/ai/agent_management/agent_management_model.rs b/app/src/ai/agent_management/agent_management_model.rs index 71819526b..63f5e8076 100644 --- a/app/src/ai/agent_management/agent_management_model.rs +++ b/app/src/ai/agent_management/agent_management_model.rs @@ -241,6 +241,7 @@ impl AgentNotificationsModel { conversation_id, // We shouldn't trigger toasts when restoring conversations on startup. is_restored: false, + .. } = event else { return; diff --git a/app/src/ai/agent_management/view.rs b/app/src/ai/agent_management/view.rs index 16076f06b..f9b7ceb1c 100644 --- a/app/src/ai/agent_management/view.rs +++ b/app/src/ai/agent_management/view.rs @@ -1237,7 +1237,7 @@ impl AgentManagementView { self.refresh_details_panel_if_needed(ctx); self.get_tasks_from_model(ctx); } - AgentConversationsModelEvent::ConversationUpdated => { + AgentConversationsModelEvent::ConversationUpdated { .. } => { self.get_tasks_from_model(ctx); self.refresh_details_panel_if_needed(ctx); ctx.notify(); diff --git a/app/src/ai/blocklist/history_model.rs b/app/src/ai/blocklist/history_model.rs index 3beeb8b4e..89cf0751d 100644 --- a/app/src/ai/blocklist/history_model.rs +++ b/app/src/ai/blocklist/history_model.rs @@ -682,6 +682,7 @@ impl BlocklistAIHistoryModel { } } + let new_status = conversation.status().clone(); self.conversations_by_id .insert(conversation_id, conversation); @@ -691,6 +692,8 @@ impl BlocklistAIHistoryModel { conversation_id, terminal_view_id, is_restored: true, + prev_status: None, + new_status, }); } @@ -2093,6 +2096,11 @@ pub enum BlocklistAIHistoryEvent { conversation_id: AIConversationId, terminal_view_id: EntityId, is_restored: bool, + /// The conversation's status before this update, if known. + /// Restoration events do not have a previous status. + prev_status: Option, + /// The conversation's status after this update. + new_status: ConversationStatus, }, /// The active conversation was set to another conversation in the history. diff --git a/app/src/terminal/view.rs b/app/src/terminal/view.rs index e8a47b0c0..3f6325cbe 100644 --- a/app/src/terminal/view.rs +++ b/app/src/terminal/view.rs @@ -3495,7 +3495,7 @@ impl TerminalView { event, AgentConversationsModelEvent::TasksUpdated | AgentConversationsModelEvent::NewTasksReceived - | AgentConversationsModelEvent::ConversationUpdated + | AgentConversationsModelEvent::ConversationUpdated { .. } | AgentConversationsModelEvent::ConversationArtifactsUpdated { .. } ); // Only refresh panel if it's currently open (avoids unnecessary work) diff --git a/app/src/workspace/view.rs b/app/src/workspace/view.rs index 6af9ef532..c93ac2a95 100644 --- a/app/src/workspace/view.rs +++ b/app/src/workspace/view.rs @@ -2904,7 +2904,7 @@ impl Workspace { // Update transcript details if task or conversation data is updated AgentConversationsModelEvent::NewTasksReceived | AgentConversationsModelEvent::TasksUpdated - | AgentConversationsModelEvent::ConversationUpdated + | AgentConversationsModelEvent::ConversationUpdated { .. } | AgentConversationsModelEvent::ConversationArtifactsUpdated { .. } => { me.update_transcript_details_panel_data(ctx); } diff --git a/app/src/workspace/view/conversation_list/view_model.rs b/app/src/workspace/view/conversation_list/view_model.rs index af2b31580..370f6b143 100644 --- a/app/src/workspace/view/conversation_list/view_model.rs +++ b/app/src/workspace/view/conversation_list/view_model.rs @@ -42,7 +42,7 @@ impl ConversationListViewModel { } // Status changes don't affect the set of IDs (status is read // at render time via get_item_by_id); just signal a re-render. - AgentConversationsModelEvent::ConversationUpdated => { + AgentConversationsModelEvent::ConversationUpdated { .. } => { ctx.emit(ConversationListViewModelEvent); } // Artifact updates don't affect the conversation list From e9584d70fcab39169640af975d8ccb1f83b3d79c Mon Sep 17 00:00:00 2001 From: lucieleblanc Date: Fri, 1 May 2026 17:19:11 -0500 Subject: [PATCH 2/2] suppress dead code warnings on new fields --- app/src/ai/agent_conversations_model.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/src/ai/agent_conversations_model.rs b/app/src/ai/agent_conversations_model.rs index b9e33dabc..010ab2738 100644 --- a/app/src/ai/agent_conversations_model.rs +++ b/app/src/ai/agent_conversations_model.rs @@ -873,7 +873,9 @@ pub enum AgentConversationsModelEvent { TasksUpdated, /// Conversation status data was updated ConversationUpdated { + #[allow(dead_code)] conversation_id: AIConversationId, + #[allow(dead_code)] kind: ConversationUpdateKind, }, /// Conversation artifacts were updated (plans, PRs, etc.)