Merge PR #3036 from Hmbown: hide internal IDs from normal UI — stable labels for turns and agents
fix(tui): hide internal IDs from normal UI — stable labels for turns and agents
This commit is contained in:
@@ -70,6 +70,18 @@ fn release_resident_leases_for(agent_id: &str) {
|
||||
/// the `SubAgentManager`.
|
||||
const DEFAULT_MAX_STEPS: u32 = u32::MAX;
|
||||
const TOOL_TIMEOUT: Duration = Duration::from_secs(30);
|
||||
|
||||
/// Format a step counter for sub-agent progress messages.
|
||||
///
|
||||
/// When `max_steps == u32::MAX` (the default), the denominator is a sentinel
|
||||
/// meaning "unbounded" — render just `step N` instead of `step N/4294967295`.
|
||||
fn format_step_counter(steps: u32, max_steps: u32) -> String {
|
||||
if max_steps == u32::MAX {
|
||||
format!("step {steps}")
|
||||
} else {
|
||||
format!("step {steps}/{max_steps}")
|
||||
}
|
||||
}
|
||||
// Non-streaming sub-agents need enough response budget to carry large tool-call
|
||||
// arguments, especially write_file content. The API bills generated tokens, not
|
||||
// the requested ceiling.
|
||||
@@ -4169,7 +4181,7 @@ async fn run_subagent(
|
||||
record_agent_progress(
|
||||
runtime,
|
||||
&agent_id,
|
||||
format!("step {steps}/{max_steps}: cancelled"),
|
||||
format!("{}: cancelled", format_step_counter(steps, max_steps)),
|
||||
);
|
||||
if let Some(mb) = runtime.mailbox.as_ref() {
|
||||
let _ = mb.send(MailboxMessage::Cancelled {
|
||||
@@ -4221,7 +4233,10 @@ async fn run_subagent(
|
||||
record_agent_progress(
|
||||
runtime,
|
||||
&agent_id,
|
||||
format!("step {steps}/{max_steps}: requesting model response"),
|
||||
format!(
|
||||
"{}: requesting model response",
|
||||
format_step_counter(steps, max_steps)
|
||||
),
|
||||
);
|
||||
|
||||
while let Ok(input) = input_rx.try_recv() {
|
||||
@@ -4278,7 +4293,7 @@ async fn run_subagent(
|
||||
record_agent_progress(
|
||||
runtime,
|
||||
&agent_id,
|
||||
format!("step {steps}/{max_steps}: cancelled mid-request"),
|
||||
format!("{}: cancelled mid-request", format_step_counter(steps, max_steps)),
|
||||
);
|
||||
if let Some(mb) = runtime.mailbox.as_ref() {
|
||||
let _ = mb.send(MailboxMessage::Cancelled {
|
||||
@@ -4341,7 +4356,7 @@ async fn run_subagent(
|
||||
record_agent_progress(
|
||||
runtime,
|
||||
&agent_id,
|
||||
format!("step {steps}/{max_steps}: interrupted; {reason}"),
|
||||
format!("{}: interrupted; {reason}", format_step_counter(steps, max_steps)),
|
||||
);
|
||||
let status = SubAgentStatus::Interrupted(reason.clone());
|
||||
let duration_ms =
|
||||
@@ -4375,7 +4390,7 @@ async fn run_subagent(
|
||||
record_agent_progress(
|
||||
runtime,
|
||||
&agent_id,
|
||||
format!("step {steps}/{max_steps}: cancelled while interrupted"),
|
||||
format!("{}: cancelled while interrupted", format_step_counter(steps, max_steps)),
|
||||
);
|
||||
if let Some(mb) = runtime.mailbox.as_ref() {
|
||||
let _ = mb.send(MailboxMessage::Cancelled {
|
||||
@@ -4499,7 +4514,7 @@ async fn run_subagent(
|
||||
record_agent_progress(
|
||||
runtime,
|
||||
&agent_id,
|
||||
format!("step {steps}/{max_steps}: {progress}"),
|
||||
format!("{}: {progress}", format_step_counter(steps, max_steps)),
|
||||
);
|
||||
messages.push(Message {
|
||||
role: "user".to_string(),
|
||||
@@ -4535,7 +4550,7 @@ async fn run_subagent(
|
||||
record_agent_progress(
|
||||
runtime,
|
||||
&agent_id,
|
||||
format!("step {steps}/{max_steps}: complete"),
|
||||
format!("{}: complete", format_step_counter(steps, max_steps)),
|
||||
);
|
||||
break;
|
||||
}
|
||||
@@ -4546,7 +4561,8 @@ async fn run_subagent(
|
||||
runtime,
|
||||
&agent_id,
|
||||
format!(
|
||||
"step {steps}/{max_steps}: executing {} tool call(s)",
|
||||
"{}: executing {} tool call(s)",
|
||||
format_step_counter(steps, max_steps),
|
||||
tool_uses.len()
|
||||
),
|
||||
);
|
||||
@@ -4555,7 +4571,10 @@ async fn run_subagent(
|
||||
record_agent_progress(
|
||||
runtime,
|
||||
&agent_id,
|
||||
format!("step {steps}/{max_steps}: running tool '{tool_name}'"),
|
||||
format!(
|
||||
"{}: running tool '{tool_name}'",
|
||||
format_step_counter(steps, max_steps)
|
||||
),
|
||||
);
|
||||
if let Some(mb) = runtime.mailbox.as_ref() {
|
||||
let _ = mb.send(MailboxMessage::ToolCallStarted {
|
||||
@@ -4579,7 +4598,10 @@ async fn run_subagent(
|
||||
record_agent_progress(
|
||||
runtime,
|
||||
&agent_id,
|
||||
format!("step {steps}/{max_steps}: finished tool '{tool_name}'"),
|
||||
format!(
|
||||
"{}: finished tool '{tool_name}'",
|
||||
format_step_counter(steps, max_steps)
|
||||
),
|
||||
);
|
||||
if let Some(mb) = runtime.mailbox.as_ref() {
|
||||
let _ = mb.send(MailboxMessage::ToolCallCompleted {
|
||||
|
||||
@@ -3196,3 +3196,18 @@ fn model_catalog_only_advertises_canonical_subagent_tools() {
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// ── #3030: step-counter formatting ──────────────────────────────────────────
|
||||
|
||||
#[test]
|
||||
fn format_step_counter_hides_unbounded_sentinel() {
|
||||
// DEFAULT_MAX_STEPS is u32::MAX, meaning "unbounded" — rendering the
|
||||
// sentinel as a denominator produced "step 16/4294967295".
|
||||
assert_eq!(format_step_counter(16, u32::MAX), "step 16");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn format_step_counter_keeps_concrete_budgets() {
|
||||
assert_eq!(format_step_counter(3, 25), "step 3/25");
|
||||
assert_eq!(format_step_counter(0, 1), "step 0/1");
|
||||
}
|
||||
|
||||
@@ -1445,6 +1445,13 @@ pub struct App {
|
||||
pub pending_subagent_dispatch: Option<String>,
|
||||
/// Animation anchor for status-strip active sub-agent spinner.
|
||||
pub agent_activity_started_at: Option<Instant>,
|
||||
/// Monotonic counter for stable agent labels (#3030).
|
||||
/// Incremented each time a sub-agent is spawned; used to generate
|
||||
/// "Agent 1", "Agent 2", etc.
|
||||
pub agent_counter: u64,
|
||||
/// Maps raw agent_id to a stable user-facing label (#3030).
|
||||
/// Populated when `AgentSpawned` fires; read by sidebar rendering.
|
||||
pub agent_label_map: HashMap<String, String>,
|
||||
/// Last time a sub-agent progress event triggered a redraw.
|
||||
/// Used to throttle redraws under high sub-agent concurrency (#3033).
|
||||
pub last_agent_progress_redraw: Option<Instant>,
|
||||
@@ -1631,6 +1638,9 @@ pub struct App {
|
||||
pub runtime_turn_id: Option<String>,
|
||||
/// Current runtime turn status (if known).
|
||||
pub runtime_turn_status: Option<String>,
|
||||
/// Monotonic turn counter for stable user-facing labels (#3030).
|
||||
/// Incremented each time a new turn starts; displayed as "Turn N".
|
||||
pub turn_counter: u64,
|
||||
/// When the UI accepted a user message but has not observed `TurnStarted` yet.
|
||||
pub dispatch_started_at: Option<Instant>,
|
||||
|
||||
@@ -2177,6 +2187,8 @@ impl App {
|
||||
last_fanout_card_index: None,
|
||||
pending_subagent_dispatch: None,
|
||||
agent_activity_started_at: None,
|
||||
agent_counter: 0,
|
||||
agent_label_map: HashMap::new(),
|
||||
last_agent_progress_redraw: None,
|
||||
ui_theme,
|
||||
theme_id,
|
||||
@@ -2266,6 +2278,7 @@ impl App {
|
||||
last_balance_fetch: None,
|
||||
runtime_turn_id: None,
|
||||
runtime_turn_status: None,
|
||||
turn_counter: 0,
|
||||
dispatch_started_at: None,
|
||||
workspace_context: None,
|
||||
workspace_context_cell: std::sync::Arc::new(std::sync::Mutex::new(None)),
|
||||
@@ -2731,6 +2744,28 @@ impl App {
|
||||
self.collapsed_cell_map.clear();
|
||||
}
|
||||
|
||||
/// #3030: return the stable user-facing label for an agent id
|
||||
/// ("Agent 3"), assigning the next sequential label on first sight.
|
||||
pub(crate) fn ensure_agent_label(&mut self, agent_id: &str) -> String {
|
||||
if let Some(label) = self.agent_label_map.get(agent_id) {
|
||||
return label.clone();
|
||||
}
|
||||
self.agent_counter = self.agent_counter.saturating_add(1);
|
||||
let label = format!("Agent {}", self.agent_counter);
|
||||
self.agent_label_map
|
||||
.insert(agent_id.to_string(), label.clone());
|
||||
label
|
||||
}
|
||||
|
||||
/// #3030: read-only label lookup with raw-id fallback for agents the
|
||||
/// label map has never seen.
|
||||
pub(crate) fn agent_display_label(&self, agent_id: &str) -> String {
|
||||
self.agent_label_map
|
||||
.get(agent_id)
|
||||
.cloned()
|
||||
.unwrap_or_else(|| agent_id.to_string())
|
||||
}
|
||||
|
||||
pub fn mark_history_updated(&mut self) {
|
||||
self.history_version = self.history_version.wrapping_add(1);
|
||||
// Resync per-cell revisions to history.len(). This is the
|
||||
|
||||
+145
-24
@@ -767,21 +767,22 @@ fn task_panel_lines(app: &App, content_width: usize, max_rows: usize) -> Vec<Lin
|
||||
let theme = &app.ui_theme;
|
||||
let mut lines: Vec<Line<'static>> = Vec::with_capacity(max_rows.max(4));
|
||||
|
||||
if let Some(turn_id) = app.runtime_turn_id.as_ref() {
|
||||
if app.runtime_turn_id.is_some() {
|
||||
let status = app
|
||||
.runtime_turn_status
|
||||
.as_deref()
|
||||
.unwrap_or("unknown")
|
||||
.to_string();
|
||||
// Show enough of the turn id prefix to identify it for
|
||||
// task_read / task_cancel. A UUID needs ~13 chars before the
|
||||
// first hyphen; 16 chars gives a safe prefix for disambiguation.
|
||||
let turn_prefix = truncate_line_to_width(turn_id, 16);
|
||||
// #3030: Use a stable turn number ("Turn 1") instead of the raw
|
||||
// UUID prefix. The full UUID is preserved in the hover text
|
||||
// (task_panel_hover_texts) for inspection.
|
||||
let turn_label = if app.turn_counter > 0 {
|
||||
format!("Turn {} ({status})", app.turn_counter)
|
||||
} else {
|
||||
format!("Current turn ({status})")
|
||||
};
|
||||
lines.push(Line::from(Span::styled(
|
||||
truncate_line_to_width(
|
||||
&format!("turn {turn_prefix} ({status})",),
|
||||
content_width.max(1),
|
||||
),
|
||||
truncate_line_to_width(&turn_label, content_width.max(1)),
|
||||
Style::default().fg(theme.accent_primary),
|
||||
)));
|
||||
}
|
||||
@@ -1834,9 +1835,17 @@ fn sidebar_agent_rows(app: &App) -> Vec<SidebarAgentRow> {
|
||||
.map(summarize_tool_output)
|
||||
.filter(|summary| !summary.trim().is_empty())
|
||||
});
|
||||
// #3030: Prefer the user-assigned nickname > stable label
|
||||
// ("Agent 1") > raw name. Every spawned agent gets a label-map
|
||||
// entry, so the generated label must not shadow nicknames.
|
||||
let display_name = agent
|
||||
.nickname
|
||||
.clone()
|
||||
.or_else(|| app.agent_label_map.get(&agent.agent_id).cloned())
|
||||
.unwrap_or_else(|| agent.name.clone());
|
||||
SidebarAgentRow {
|
||||
id: agent.agent_id.clone(),
|
||||
name: agent.nickname.clone().unwrap_or_else(|| agent.name.clone()),
|
||||
name: display_name,
|
||||
role: agent.agent_type.as_str().to_string(),
|
||||
status: subagent_status_text(&agent.status).to_string(),
|
||||
git_branch: agent.git_branch.clone(),
|
||||
@@ -1856,17 +1865,25 @@ fn sidebar_agent_rows(app: &App) -> Vec<SidebarAgentRow> {
|
||||
app.agent_progress
|
||||
.iter()
|
||||
.filter(|(id, _)| !cached_ids.contains(id.as_str()))
|
||||
.map(|(id, progress)| SidebarAgentRow {
|
||||
id: id.clone(),
|
||||
name: id.clone(),
|
||||
role: "agent".to_string(),
|
||||
status: "running".to_string(),
|
||||
git_branch: None,
|
||||
progress: Some(progress.clone()),
|
||||
steps_taken: 0,
|
||||
duration_ms: app.agent_activity_started_at.map(|started| {
|
||||
u64::try_from(started.elapsed().as_millis()).unwrap_or(u64::MAX)
|
||||
}),
|
||||
.map(|(id, progress)| {
|
||||
// #3030: Prefer stable label for progress-only agents too.
|
||||
let display_name = app
|
||||
.agent_label_map
|
||||
.get(id.as_str())
|
||||
.cloned()
|
||||
.unwrap_or_else(|| id.clone());
|
||||
SidebarAgentRow {
|
||||
id: id.clone(),
|
||||
name: display_name,
|
||||
role: "agent".to_string(),
|
||||
status: "running".to_string(),
|
||||
git_branch: None,
|
||||
progress: Some(progress.clone()),
|
||||
steps_taken: 0,
|
||||
duration_ms: app.agent_activity_started_at.map(|started| {
|
||||
u64::try_from(started.elapsed().as_millis()).unwrap_or(u64::MAX)
|
||||
}),
|
||||
}
|
||||
}),
|
||||
);
|
||||
|
||||
@@ -1965,8 +1982,9 @@ pub fn subagent_panel_lines(
|
||||
if lines.len() >= max_rows {
|
||||
break;
|
||||
}
|
||||
// #3030: keep raw agent ids out of the compact detail line — the
|
||||
// full id remains available in the hover text.
|
||||
let mut detail_parts = Vec::new();
|
||||
detail_parts.push(truncate_line_to_width(&row.id, 10));
|
||||
if row.steps_taken > 0 {
|
||||
detail_parts.push(format!("{} step(s)", row.steps_taken));
|
||||
}
|
||||
@@ -1981,6 +1999,9 @@ pub fn subagent_panel_lines(
|
||||
if let Some(duration) = row.duration_ms {
|
||||
detail_parts.push(format_duration_ms(duration));
|
||||
}
|
||||
if detail_parts.is_empty() {
|
||||
detail_parts.push(row.status.clone());
|
||||
}
|
||||
lines.push(Line::from(Span::styled(
|
||||
format!(
|
||||
" {}",
|
||||
@@ -2355,8 +2376,8 @@ mod tests {
|
||||
SidebarSubagentSummary, SidebarToolRow, SidebarWorkChecklistItem, SidebarWorkStrategyStep,
|
||||
SidebarWorkSummary, ToolRowOrder, auto_sidebar_panels, editorial_tool_rows,
|
||||
normalize_activity_text, sidebar_hover_rows, sidebar_work_summary,
|
||||
subagent_panel_hover_texts, subagent_panel_lines, task_panel_lines, work_panel_empty_hint,
|
||||
work_panel_hover_texts, work_panel_lines,
|
||||
subagent_panel_hover_texts, subagent_panel_lines, task_panel_hover_texts, task_panel_lines,
|
||||
work_panel_empty_hint, work_panel_hover_texts, work_panel_lines,
|
||||
};
|
||||
use crate::config::Config;
|
||||
use crate::palette;
|
||||
@@ -3514,4 +3535,104 @@ mod tests {
|
||||
"hover text should include the full progress before popover wrapping: {hover:?}"
|
||||
);
|
||||
}
|
||||
|
||||
// ── #3030: stable labels instead of raw internal ids ───────────────────
|
||||
|
||||
#[test]
|
||||
fn tasks_panel_shows_stable_turn_label_not_uuid() {
|
||||
let mut app = create_test_app();
|
||||
app.runtime_turn_id = Some("0196f0a3-1111-2222-3333-444455556666".to_string());
|
||||
app.runtime_turn_status = Some("in_progress".to_string());
|
||||
app.turn_counter = 3;
|
||||
|
||||
let text = lines_to_text(&task_panel_lines(&app, 64, 8));
|
||||
assert!(
|
||||
text[0].contains("Turn 3 (in_progress)"),
|
||||
"compact row must show the stable turn label: {text:?}"
|
||||
);
|
||||
assert!(
|
||||
!text[0].contains("0196f0a3"),
|
||||
"raw turn UUID must stay out of the compact row: {text:?}"
|
||||
);
|
||||
|
||||
let hover = task_panel_hover_texts(&app, 8);
|
||||
assert!(
|
||||
hover[0].contains("0196f0a3-1111-2222-3333-444455556666"),
|
||||
"full turn UUID must remain available in hover text: {hover:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tasks_panel_turn_label_falls_back_before_first_counted_turn() {
|
||||
let mut app = create_test_app();
|
||||
app.runtime_turn_id = Some("0196f0a3-1111-2222-3333-444455556666".to_string());
|
||||
app.runtime_turn_status = Some("in_progress".to_string());
|
||||
app.turn_counter = 0;
|
||||
|
||||
let text = lines_to_text(&task_panel_lines(&app, 64, 8));
|
||||
assert!(
|
||||
text[0].contains("Current turn (in_progress)"),
|
||||
"zero counter falls back to a generic label: {text:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ensure_agent_label_assigns_stable_sequential_labels() {
|
||||
let mut app = create_test_app();
|
||||
assert_eq!(app.ensure_agent_label("agent_aaa111"), "Agent 1");
|
||||
assert_eq!(app.ensure_agent_label("agent_bbb222"), "Agent 2");
|
||||
// Re-seeing a known agent keeps its original label.
|
||||
assert_eq!(app.ensure_agent_label("agent_aaa111"), "Agent 1");
|
||||
assert_eq!(app.agent_counter, 2);
|
||||
// Read-only lookup falls back to the raw id for unknown agents.
|
||||
assert_eq!(app.agent_display_label("agent_bbb222"), "Agent 2");
|
||||
assert_eq!(app.agent_display_label("agent_zzz999"), "agent_zzz999");
|
||||
}
|
||||
|
||||
fn cached_agent(
|
||||
agent_id: &str,
|
||||
nickname: Option<&str>,
|
||||
) -> crate::tools::subagent::SubAgentResult {
|
||||
crate::tools::subagent::SubAgentResult {
|
||||
name: "implementation-worker".to_string(),
|
||||
agent_id: agent_id.to_string(),
|
||||
context_mode: "fresh".to_string(),
|
||||
fork_context: false,
|
||||
workspace: None,
|
||||
git_branch: None,
|
||||
agent_type: crate::tools::subagent::SubAgentType::General,
|
||||
assignment: crate::tools::subagent::SubAgentAssignment {
|
||||
objective: "task".to_string(),
|
||||
role: Some("worker".to_string()),
|
||||
},
|
||||
model: String::new(),
|
||||
nickname: nickname.map(str::to_string),
|
||||
status: crate::tools::subagent::SubAgentStatus::Running,
|
||||
result: None,
|
||||
steps_taken: 1,
|
||||
checkpoint: None,
|
||||
duration_ms: 100,
|
||||
from_prior_session: false,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sidebar_agent_rows_prefer_nickname_over_generated_label() {
|
||||
let mut app = create_test_app();
|
||||
let agent_id = "agent_cafe0123";
|
||||
app.ensure_agent_label(agent_id);
|
||||
app.subagent_cache
|
||||
.push(cached_agent(agent_id, Some("doc-fixer")));
|
||||
|
||||
let rows = super::sidebar_agent_rows(&app);
|
||||
assert_eq!(
|
||||
rows[0].name, "doc-fixer",
|
||||
"user nickname must beat the generated Agent-N label"
|
||||
);
|
||||
|
||||
// Without a nickname the generated label is used.
|
||||
app.subagent_cache[0].nickname = None;
|
||||
let rows = super::sidebar_agent_rows(&app);
|
||||
assert_eq!(rows[0].name, "Agent 1");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1797,6 +1797,7 @@ async fn run_event_loop(
|
||||
}
|
||||
app.runtime_turn_id = Some(turn_id);
|
||||
app.runtime_turn_status = Some("in_progress".to_string());
|
||||
app.turn_counter = app.turn_counter.saturating_add(1);
|
||||
app.reasoning_buffer.clear();
|
||||
app.reasoning_header = None;
|
||||
app.last_reasoning = None;
|
||||
@@ -2307,8 +2308,10 @@ async fn run_event_loop(
|
||||
if app.agent_activity_started_at.is_none() {
|
||||
app.agent_activity_started_at = Some(Instant::now());
|
||||
}
|
||||
app.status_message =
|
||||
Some(format!("Sub-agent {id} starting: {prompt_summary}"));
|
||||
// #3030: Assign a stable user-facing label for this
|
||||
// agent and keep the raw id out of the status bar.
|
||||
let label = app.ensure_agent_label(&id);
|
||||
app.status_message = Some(format!("{label} starting: {prompt_summary}"));
|
||||
let _ = engine_handle.send(Op::ListSubAgents).await;
|
||||
}
|
||||
EngineEvent::AgentProgress { id, status } => {
|
||||
@@ -2323,7 +2326,10 @@ async fn run_event_loop(
|
||||
if app.agent_activity_started_at.is_none() {
|
||||
app.agent_activity_started_at = Some(Instant::now());
|
||||
}
|
||||
app.status_message = Some(format!("Sub-agent {id}: {display}"));
|
||||
// #3030: progress can arrive before AgentSpawned is
|
||||
// observed — assign the stable label on first sight.
|
||||
let label = app.ensure_agent_label(&id);
|
||||
app.status_message = Some(format!("{label}: {display}"));
|
||||
// #3033: Throttle redraws from rapid AgentProgress events.
|
||||
// When 4+ sub-agents are running concurrently, each firing
|
||||
// progress events, the per-event `needs_redraw = true` saturates
|
||||
@@ -2362,8 +2368,10 @@ async fn run_event_loop(
|
||||
&& matches!(agent.status, SubAgentStatus::Running)
|
||||
});
|
||||
app.agent_progress.remove(&id);
|
||||
// #3030: stable label with raw-id fallback.
|
||||
let label = app.agent_display_label(&id);
|
||||
app.status_message = Some(format!(
|
||||
"Sub-agent {id} completed: {}",
|
||||
"{label} completed: {}",
|
||||
summarize_tool_output(&result)
|
||||
));
|
||||
let should_recapture_terminal =
|
||||
|
||||
Reference in New Issue
Block a user