Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions PR_DESCRIPTION.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
## 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.)
7 changes: 5 additions & 2 deletions app/src/ai/agent/conversation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ use crate::{
todos::AIAgentTodoList,
AIAgentOutputMessage, AIAgentOutputMessageType, MessageToAIAgentOutputMessageError,
},
blocklist::BlocklistAIHistoryEvent,
blocklist::{BlocklistAIHistoryEvent, ConversationStatusUpdate},
},
persistence::{
model::{AgentConversationData, PersistedAutoexecuteMode},
Expand Down Expand Up @@ -673,11 +673,14 @@ 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,
update: ConversationStatusUpdate::Changed { prev_status },
new_status,
});
}

Expand Down
47 changes: 43 additions & 4 deletions app/src/ai/agent_conversations_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ use crate::ai::agent::conversation::{AIConversationId, ConversationStatus};
use crate::ai::ambient_agents::AmbientAgentTaskId;
use crate::ai::ambient_agents::{AgentSource, AmbientAgentTask, AmbientAgentTaskState};
use crate::ai::artifacts::Artifact;
use crate::ai::blocklist::{format_credits, BlocklistAIHistoryEvent, BlocklistAIHistoryModel};
use crate::ai::blocklist::{
format_credits, BlocklistAIHistoryEvent, BlocklistAIHistoryModel, ConversationStatusUpdate,
};
use crate::ai::cloud_environments::CloudAmbientAgentEnvironment;
use crate::ai::conversation_navigation::ConversationNavigationData;
use crate::auth::auth_manager::{AuthManager, AuthManagerEvent};
Expand Down Expand Up @@ -869,11 +871,27 @@ pub enum AgentConversationsModelEvent {
/// Existing task data may have been updated (e.g., state changes).
TasksUpdated,
/// Conversation status data was updated
ConversationUpdated,
ConversationUpdated {
#[allow(dead_code)]
conversation_id: AIConversationId,
#[allow(dead_code)]
kind: ConversationUpdateKind,
},
/// Conversation artifacts were updated (plans, PRs, etc.)
ConversationArtifactsUpdated { conversation_id: AIConversationId },
}

#[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,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on storing AgentRunDisplayStatus here instead of StatusFilter? We can use from_conversation_status to get it, and means we don't need a new conversation_status_filter method since AgentRunDisplayStatus already has a status_filter

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed, thank you!

new_filter: StatusFilter,
},
}

impl Entity for AgentConversationsModel {
type Event = AgentConversationsModelEvent;
}
Expand Down Expand Up @@ -1435,8 +1453,29 @@ 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,
update,
new_status,
..
} => {
let kind = match update {
ConversationStatusUpdate::Restored => ConversationUpdateKind::Restored,
ConversationStatusUpdate::Changed { prev_status } => {
ConversationUpdateKind::StatusSet {
prev_filter: AgentRunDisplayStatus::from_conversation_status(
prev_status,
)
.status_filter(),
new_filter: AgentRunDisplayStatus::from_conversation_status(new_status)
.status_filter(),
}
}
};
ctx.emit(AgentConversationsModelEvent::ConversationUpdated {
conversation_id: *conversation_id,
kind,
});
}

// Artifact changes - sync live artifacts into the cached task and notify.
Expand Down
149 changes: 126 additions & 23 deletions app/src/ai/agent_conversations_model_tests.rs
Original file line number Diff line number Diff line change
@@ -1,32 +1,29 @@
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};
use crate::ai::ambient_agents::AgentConfigSnapshot;
use crate::ai::ambient_agents::AmbientAgentTaskId;
use crate::ai::ambient_agents::{AmbientAgentTask, AmbientAgentTaskState};
use crate::ai::artifacts::Artifact;
use crate::ai::blocklist::history_model::{BlocklistAIHistoryEvent, BlocklistAIHistoryModel};
use crate::ai::blocklist::history_model::{
BlocklistAIHistoryEvent, BlocklistAIHistoryModel, ConversationStatusUpdate,
};
use crate::ai::conversation_navigation::ConversationNavigationData;
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,
ConversationUpdateKind, EnvironmentFilter, HarnessFilter, OwnerFilter, StatusFilter,
TaskFetchState, MAX_PERSONAL_TASKS, MAX_TEAM_TASKS,
};
use crate::ai::ambient_agents::task::HarnessConfig;
use warp_cli::agent::Harness;
Expand Down Expand Up @@ -65,35 +62,141 @@ fn create_test_task(
}
}

type CapturedConversationUpdate = Mutex<Option<(AIConversationId, ConversationUpdateKind)>>;

/// 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<AgentConversationsModel>,
) -> Arc<CapturedConversationUpdate> {
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(),
update: ConversationStatusUpdate::Restored,
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(),
update: ConversationStatusUpdate::Changed {
prev_status: 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,
update: ConversationStatusUpdate::Changed {
prev_status: 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,
},
)),
);
});
}

Expand Down
5 changes: 3 additions & 2 deletions app/src/ai/agent_management/agent_management_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::ai::agent_management::notifications::{
NotificationSourceAgent,
};
use crate::ai::artifacts::Artifact;
use crate::ai::blocklist::BlocklistAIHistoryEvent;
use crate::ai::blocklist::{BlocklistAIHistoryEvent, ConversationStatusUpdate};
use crate::server::telemetry::TelemetryEvent;
use crate::terminal::cli_agent_sessions::{
CLIAgentSessionStatus, CLIAgentSessionsModel, CLIAgentSessionsModelEvent,
Expand Down Expand Up @@ -250,7 +250,8 @@ impl AgentNotificationsModel {
terminal_view_id,
conversation_id,
// We shouldn't trigger toasts when restoring conversations on startup.
is_restored: false,
update: ConversationStatusUpdate::Changed { .. },
..
} = event
else {
return;
Expand Down
2 changes: 1 addition & 1 deletion app/src/ai/agent_management/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
19 changes: 17 additions & 2 deletions app/src/ai/blocklist/history_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,7 @@ impl BlocklistAIHistoryModel {
}
}

let new_status = conversation.status().clone();
self.conversations_by_id
.insert(conversation_id, conversation);

Expand All @@ -690,7 +691,8 @@ impl BlocklistAIHistoryModel {
ctx.emit(BlocklistAIHistoryEvent::UpdatedConversationStatus {
conversation_id,
terminal_view_id,
is_restored: true,
update: ConversationStatusUpdate::Restored,
new_status,
});
}

Expand Down Expand Up @@ -2053,6 +2055,16 @@ fn agent_id_key(conversation: &AIConversation) -> Option<String> {
conversation.orchestration_agent_id()
}

/// Whether an `UpdatedConversationStatus` event represents a restoration
/// (the conversation was re-loaded into a terminal view; the underlying
/// `ConversationStatus` did not change) or a real status set, in which case
/// the previous status is included.
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum ConversationStatusUpdate {
Restored,
Changed { prev_status: ConversationStatus },
}

#[derive(Clone, Debug)]
pub enum BlocklistAIHistoryEvent {
/// A new conversation was started.
Expand Down Expand Up @@ -2106,7 +2118,10 @@ pub enum BlocklistAIHistoryEvent {
UpdatedConversationStatus {
conversation_id: AIConversationId,
terminal_view_id: EntityId,
is_restored: bool,
/// Distinguishes a restoration from a real status set.
update: ConversationStatusUpdate,
/// The conversation's status after this update.
new_status: ConversationStatus,
},

/// The active conversation was set to another conversation in the history.
Expand Down
Loading
Loading