diff --git a/app/src/ai/blocklist/agent_view/orchestration_pill_bar.rs b/app/src/ai/blocklist/agent_view/orchestration_pill_bar.rs index bf48bf8b3b..cfb10ad4ec 100644 --- a/app/src/ai/blocklist/agent_view/orchestration_pill_bar.rs +++ b/app/src/ai/blocklist/agent_view/orchestration_pill_bar.rs @@ -102,6 +102,41 @@ pub(crate) fn pill_initial(name: &str) -> char { .map(|c| c.to_ascii_uppercase()) .unwrap_or('A') } + +/// Status key for the trailing "done" bucket (Cancelled + Success). +/// Named so render code and tests don't have to chase a literal `3`. +pub(super) const DONE_STATUS_KEY: u8 = 3; + +/// Sort priority within a pill section. Lower sorts leftmost. Cancelled +/// and Success share one "done" bucket; recency decides their order. +fn pill_status_sort_key(status: Option<&ConversationStatus>) -> u8 { + match status { + Some(ConversationStatus::Blocked { .. }) => 0, + Some(ConversationStatus::Error) => 1, + Some(ConversationStatus::InProgress) => 2, + Some(ConversationStatus::Cancelled) | Some(ConversationStatus::Success) => DONE_STATUS_KEY, + None => 2, + } +} + +/// Recency tiebreaker for done pills, sorted ascending: newer finish +/// times sort first; unknown sorts last. `saturating_neg` so a wild +/// `i64::MIN` (impossible for real timestamps) couldn't panic. +fn pill_done_recency_key(last_modified_ms: Option) -> i64 { + last_modified_ms.unwrap_or(0).saturating_neg() +} + +/// Combines status key + recency into the secondary sort key. Recency +/// wins inside the done bucket; everything else collapses to 0 so the +/// spawn-index tiebreaker decides. Shared so render and tests can't drift. +pub(super) fn pill_secondary_sort_key(status_key: u8, last_modified_ms: Option) -> i64 { + if status_key == DONE_STATUS_KEY { + pill_done_recency_key(last_modified_ms) + } else { + 0 + } +} + /// Renders the orchestrator avatar disc shared by pill, breadcrumb, and transcript /// surfaces. pub(crate) fn render_orchestrator_avatar_disc( @@ -170,6 +205,9 @@ struct PillSpec { pin_state: PillPinState, /// Child running on a remote worker; drives the cloud-shaped badge variant. is_remote_child: bool, + /// Epoch ms of the conversation's last activity; recency tiebreaker + /// within the done bucket. + last_modified_ms: Option, } #[derive(Clone, Copy)] @@ -592,6 +630,8 @@ impl OrchestrationPillBar { pin_state: PillPinState::Unpinned, // Unused: orchestrator pills don't render a status overlay. is_remote_child: false, + // Unused: orchestrator pills aren't sorted (they render first). + last_modified_ms: None, }); // Stamp each child's current pin state; partitioning happens at render. @@ -616,6 +656,7 @@ impl OrchestrationPillBar { kind: PillKind::Child, pin_state, is_remote_child: child.is_remote_child(), + last_modified_ms: child.last_modified_at().map(|t| t.timestamp_millis()), }); } @@ -1021,12 +1062,13 @@ impl View for OrchestrationPillBar { // the orchestrator pane, so any child whose owner differs from // this id has been split off into another pane/tab. let self_terminal_view_id = self.agent_view_controller.as_ref(app).terminal_view_id(); - // Bucket pills so the row is: orchestrator, pinned, divider, unpinned. - // Spawn order within each child bucket is preserved by iteration order. + // Row layout: orchestrator, pinned, divider, unpinned. Each + // child bucket is sorted by status priority, then recency for + // done pills and spawn order otherwise. let mut orchestrator_pill: Option> = None; - let mut pinned_pills: Vec> = Vec::new(); - let mut unpinned_pills: Vec> = Vec::new(); - for spec in specs { + let mut pinned_pills: Vec<(u8, i64, usize, Box)> = Vec::new(); + let mut unpinned_pills: Vec<(u8, i64, usize, Box)> = Vec::new(); + for (spawn_index, spec) in specs.into_iter().enumerate() { let mouse_state = mouse_states .entry(spec.conversation_id) .or_default() @@ -1046,6 +1088,8 @@ impl View for OrchestrationPillBar { let menu_is_open_for_this = menu_open_for == Some(spec.conversation_id); let kind = spec.kind; let pin_state = spec.pin_state; + let status_key = pill_status_sort_key(spec.status.as_ref()); + let secondary_key = pill_secondary_sort_key(status_key, spec.last_modified_ms); let pill = render_pill( spec, mouse_state, @@ -1057,27 +1101,36 @@ impl View for OrchestrationPillBar { ); match (kind, pin_state) { (PillKind::Orchestrator, _) => orchestrator_pill = Some(pill), - (PillKind::Child, PillPinState::Pinned) => pinned_pills.push(pill), - (PillKind::Child, PillPinState::Unpinned) => unpinned_pills.push(pill), + (PillKind::Child, PillPinState::Pinned) => { + pinned_pills.push((status_key, secondary_key, spawn_index, pill)); + } + (PillKind::Child, PillPinState::Unpinned) => { + unpinned_pills.push((status_key, secondary_key, spawn_index, pill)); + } } } drop(mouse_states); drop(overflow_states); drop(pin_states); + // Explicit spawn-index tiebreaker keeps ordering deterministic. + let sort_key = |(k, s, idx, _): &(u8, i64, usize, _)| (*k, *s, *idx); + pinned_pills.sort_by_key(sort_key); + unpinned_pills.sort_by_key(sort_key); + if let Some(pill) = orchestrator_pill { row.add_child(pill); } let has_pinned = !pinned_pills.is_empty(); let has_unpinned = !unpinned_pills.is_empty(); - for pill in pinned_pills { + for (.., pill) in pinned_pills { row.add_child(pill); } // Only show the divider when both sides actually have pills. if has_pinned && has_unpinned { row.add_child(render_pinned_divider(app)); } - for pill in unpinned_pills { + for (.., pill) in unpinned_pills { row.add_child(pill); } diff --git a/app/src/ai/blocklist/agent_view/orchestration_pill_bar_tests.rs b/app/src/ai/blocklist/agent_view/orchestration_pill_bar_tests.rs index 8694f357b2..abde4d7dbd 100644 --- a/app/src/ai/blocklist/agent_view/orchestration_pill_bar_tests.rs +++ b/app/src/ai/blocklist/agent_view/orchestration_pill_bar_tests.rs @@ -27,3 +27,91 @@ fn navigation_action_for_orchestrator_pill_switches_in_place() { } if actual_id == conversation_id )); } + +#[test] +fn pill_status_sort_key_orders_attention_then_in_progress_then_done() { + let blocked = ConversationStatus::Blocked { + blocked_action: String::new(), + }; + let blocked_key = pill_status_sort_key(Some(&blocked)); + let error_key = pill_status_sort_key(Some(&ConversationStatus::Error)); + let in_progress_key = pill_status_sort_key(Some(&ConversationStatus::InProgress)); + let cancelled_key = pill_status_sort_key(Some(&ConversationStatus::Cancelled)); + let success_key = pill_status_sort_key(Some(&ConversationStatus::Success)); + + assert!(blocked_key < error_key); + assert!(error_key < in_progress_key); + assert!(in_progress_key < cancelled_key); + // Cancelled and Success share the done bucket; recency decides within it. + assert_eq!(cancelled_key, success_key); +} + +#[test] +fn pill_status_sort_key_treats_none_as_in_progress() { + // Safety default for the orchestrator path (never sorted in practice). + assert_eq!( + pill_status_sort_key(None), + pill_status_sort_key(Some(&ConversationStatus::InProgress)), + ); +} + +#[test] +fn pill_done_recency_key_puts_most_recent_first_and_unknown_last() { + let older = pill_done_recency_key(Some(1_000)); + let newer = pill_done_recency_key(Some(2_000)); + let unknown = pill_done_recency_key(None); + assert!(newer < older); + assert!(older < unknown); +} + +#[test] +fn sort_pills_bubbles_attention_in_progress_keeps_spawn_done_uses_recency() { + let blocked = ConversationStatus::Blocked { + blocked_action: String::new(), + }; + // (status, finish time) per spawn index. + let inputs: Vec<(ConversationStatus, Option)> = vec![ + (ConversationStatus::Success, Some(100)), + (ConversationStatus::InProgress, None), + (blocked.clone(), None), + (ConversationStatus::Cancelled, Some(300)), + (ConversationStatus::InProgress, None), + (ConversationStatus::Error, None), + (ConversationStatus::Success, Some(200)), + ]; + let mut sortable: Vec<(u8, i64, usize)> = inputs + .iter() + .enumerate() + .map(|(idx, (status, ms))| { + let status_key = pill_status_sort_key(Some(status)); + (status_key, pill_secondary_sort_key(status_key, *ms), idx) + }) + .collect(); + sortable.sort_by_key(|(k, s, idx)| (*k, *s, *idx)); + let spawn_order: Vec = sortable.iter().map(|(_, _, idx)| *idx).collect(); + // Blocked, Error, InProgress (spawn order), then done by recency desc. + assert_eq!(spawn_order, vec![2, 5, 1, 4, 3, 6, 0]); +} + +#[test] +fn sort_pills_is_stable_within_in_progress_bucket() { + let in_progress_key = pill_status_sort_key(Some(&ConversationStatus::InProgress)); + let mut entries: Vec<(u8, i64, usize)> = vec![(in_progress_key, 0, 7), (in_progress_key, 0, 3)]; + entries.sort_by_key(|(k, s, idx)| (*k, *s, *idx)); + let spawn_order: Vec = entries.iter().map(|(_, _, idx)| *idx).collect(); + assert_eq!(spawn_order, vec![3, 7]); +} + +#[test] +fn sort_pills_done_bucket_orders_by_recency_regardless_of_completion_type() { + // Old Cancelled sinks behind a fresh Success. + let cancelled_old = pill_secondary_sort_key(DONE_STATUS_KEY, Some(100)); + let success_new = pill_secondary_sort_key(DONE_STATUS_KEY, Some(500)); + let mut entries: Vec<(u8, i64, usize)> = vec![ + (DONE_STATUS_KEY, cancelled_old, 0), + (DONE_STATUS_KEY, success_new, 1), + ]; + entries.sort_by_key(|(k, s, idx)| (*k, *s, *idx)); + let spawn_order: Vec = entries.iter().map(|(_, _, idx)| *idx).collect(); + assert_eq!(spawn_order, vec![1, 0]); +}