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
71 changes: 62 additions & 9 deletions app/src/ai/blocklist/agent_view/orchestration_pill_bar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>) -> 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>) -> 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(
Expand Down Expand Up @@ -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<i64>,
}

#[derive(Clone, Copy)]
Expand Down Expand Up @@ -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.
Expand All @@ -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()),
});
}

Expand Down Expand Up @@ -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<Box<dyn Element>> = None;
let mut pinned_pills: Vec<Box<dyn Element>> = Vec::new();
let mut unpinned_pills: Vec<Box<dyn Element>> = Vec::new();
for spec in specs {
let mut pinned_pills: Vec<(u8, i64, usize, Box<dyn Element>)> = Vec::new();
let mut unpinned_pills: Vec<(u8, i64, usize, Box<dyn Element>)> = Vec::new();
for (spawn_index, spec) in specs.into_iter().enumerate() {
let mouse_state = mouse_states
.entry(spec.conversation_id)
.or_default()
Expand All @@ -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,
Expand All @@ -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);
}

Expand Down
88 changes: 88 additions & 0 deletions app/src/ai/blocklist/agent_view/orchestration_pill_bar_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<i64>)> = 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<usize> = 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<usize> = 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<usize> = entries.iter().map(|(_, _, idx)| *idx).collect();
assert_eq!(spawn_order, vec![1, 0]);
}
Loading