mirror of
https://github.com/warpdotdev/warp.git
synced 2026-05-06 15:22:21 +08:00
Handle agent management view updates based on event type (#9866)
Co-authored-by: Oz <oz-agent@warp.dev>
This commit is contained in:
@@ -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.)
|
||||
@@ -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.
|
||||
|
||||
@@ -62,7 +62,7 @@ fn create_test_task(
|
||||
}
|
||||
}
|
||||
|
||||
type CapturedConversationUpdate = Mutex<Option<(AIConversationId, ConversationUpdateKind)>>;
|
||||
type CapturedConversationUpdate = Mutex<Option<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
|
||||
@@ -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,
|
||||
}),
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -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<Self>,
|
||||
) {
|
||||
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,
|
||||
|
||||
Reference in New Issue
Block a user