From d46bfdb00be3d2f011ad34d6ba7559b248bb8cfb Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Tue, 14 Jul 2026 15:46:34 -0700 Subject: [PATCH] fix(list): stop repeating the failure count in the list footer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A failed task printed the same count on two adjacent lines: the summary footer's "N tasks failed" clause and the warning header immediately below it that details each failure. The footer clause was never a unique signal — its count is exactly `non_timeout_errors.len()`, and that vector being non-empty is the condition under which the warning renders. Split the two by what each reports rather than suppressing the duplicate: the footer counts timed-out tasks (a timeout leaves an empty cell and no message, so a count is its only home, and the warning filters timeouts out), while failures are named in the warning with git's full error text. `format_summary_message` drops its `error_count` parameter entirely. Also corrects a stale claim in the status_symbols module spec: the footer never listed which tasks timed out per item — it only ever carried counts. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/commands/list/collect/mod.rs | 3 -- src/commands/list/mod.rs | 52 +++++++------------ src/commands/list/model/status_symbols.rs | 8 +-- ...list__list_shows_warning_on_git_error.snap | 2 +- ...warns_when_commit_details_batch_fails.snap | 2 +- 5 files changed, 26 insertions(+), 41 deletions(-) diff --git a/src/commands/list/collect/mod.rs b/src/commands/list/collect/mod.rs index 00dbd8c8d5..d923744384 100644 --- a/src/commands/list/collect/mod.rs +++ b/src/commands/list/collect/mod.rs @@ -1987,8 +1987,6 @@ pub fn collect( item.refresh_status_symbols(primary_target); } - // Count errors for summary - let error_count = errors.len(); let timed_out_count = errors.iter().filter(|e| e.is_timeout()).count(); let table_render = render_table.then(|| TableRenderPlan { @@ -2002,7 +2000,6 @@ pub fn collect( &all_items, show_branches || show_remotes, layout.hidden_column_count, - error_count, timed_out_count, ), }); diff --git a/src/commands/list/mod.rs b/src/commands/list/mod.rs index 39516f5401..4eb46de7ff 100644 --- a/src/commands/list/mod.rs +++ b/src/commands/list/mod.rs @@ -365,12 +365,17 @@ impl SummaryMetrics { } } -/// Format a summary message for the given items (used by both collect/mod.rs and mod.rs) +/// Format the summary line that closes the table. +/// +/// The footer is the only home a timed-out task has: it leaves an empty cell +/// and no message to print, so the count here is all that reports it. Tasks +/// that failed instead get a named entry in the warning that follows the +/// table, and that warning carries its own count — repeating it here would +/// print the same number twice on adjacent lines. pub(crate) fn format_summary_message( items: &[ListItem], show_branches: bool, hidden_column_count: usize, - error_count: usize, timed_out_count: usize, ) -> String { let metrics = SummaryMetrics::from_items(items); @@ -379,25 +384,11 @@ pub(crate) fn format_summary_message( .summary_parts(show_branches, hidden_column_count) .join(", "); - if error_count > 0 { - // "failed" and "timed out" are disjoint here, matching the warning - // that details the non-timeout failures after the table. - let failed_count = error_count - timed_out_count; - let failure_msg = match (failed_count, timed_out_count) { - (0, t) => { - let plural = if t == 1 { "" } else { "s" }; - format!("{t} task{plural} timed out") - } - (f, 0) => { - let plural = if f == 1 { "" } else { "s" }; - format!("{f} task{plural} failed") - } - (f, t) => { - let plural = if f == 1 { "" } else { "s" }; - format!("{f} task{plural} failed, {t} timed out") - } - }; - format!("{INFO_SYMBOL} {dim}Showing {summary}; {failure_msg}{dim:#}") + if timed_out_count > 0 { + let plural = if timed_out_count == 1 { "" } else { "s" }; + format!( + "{INFO_SYMBOL} {dim}Showing {summary}; {timed_out_count} task{plural} timed out{dim:#}" + ) } else { format!("{INFO_SYMBOL} {dim}Showing {summary}{dim:#}") } @@ -538,20 +529,15 @@ mod tests { } #[test] - fn test_format_summary_message_error_variants() { + fn test_format_summary_message_timeout_variants() { use insta::assert_snapshot; - // No errors - assert_snapshot!(format_summary_message(&[], false, 0, 0, 0), @"○ Showing 0 worktrees"); - // All timeouts - assert_snapshot!(format_summary_message(&[], false, 0, 3, 3), @"○ Showing 0 worktrees; 3 tasks timed out"); - // Mixed errors and timeouts - assert_snapshot!(format_summary_message(&[], false, 0, 5, 3), @"○ Showing 0 worktrees; 2 tasks failed, 3 timed out"); - // Only failures, no timeouts - assert_snapshot!(format_summary_message(&[], false, 0, 2, 0), @"○ Showing 0 worktrees; 2 tasks failed"); - // Single error - assert_snapshot!(format_summary_message(&[], false, 0, 1, 0), @"○ Showing 0 worktrees; 1 task failed"); + // Nothing timed out. Failures alone leave the footer untouched — the + // warning after the table names them and carries their count. + assert_snapshot!(format_summary_message(&[], false, 0, 0), @"○ Showing 0 worktrees"); // Single timeout - assert_snapshot!(format_summary_message(&[], false, 0, 1, 1), @"○ Showing 0 worktrees; 1 task timed out"); + assert_snapshot!(format_summary_message(&[], false, 0, 1), @"○ Showing 0 worktrees; 1 task timed out"); + // Several timeouts + assert_snapshot!(format_summary_message(&[], false, 0, 3), @"○ Showing 0 worktrees; 3 tasks timed out"); } } diff --git a/src/commands/list/model/status_symbols.rs b/src/commands/list/model/status_symbols.rs index 60cd27cef9..6a43f2d85f 100644 --- a/src/commands/list/model/status_symbols.rs +++ b/src/commands/list/model/status_symbols.rs @@ -186,9 +186,11 @@ //! //! 1. Each position's last-known state is displayed: resolved positions show //! their symbol; unresolved positions show `·`. -//! 2. The diagnostic footer already lists which tasks did not finish per -//! item — this continues unchanged and gives the user the mapping from -//! "`·` in position X" back to "`TaskKind::Foo` timed out." +//! 2. The `·` is the whole signal — budget truncation is deliberate, so it +//! goes unreported, and nothing maps a `·` in position X back to the +//! `TaskKind` that didn't finish. (A task whose own git command times out +//! is a separate path: it reaches the summary footer as one of "N tasks +//! timed out", a count that likewise doesn't name the task.) //! 3. JSON output omits fields that correspond to unresolved gates //! (`working_tree`, `main_state`, `operation_state`, `upstream_divergence`, //! etc.) so machine consumers can distinguish "loading / timeout" from diff --git a/tests/snapshots/integration__integration_tests__list__list_shows_warning_on_git_error.snap b/tests/snapshots/integration__integration_tests__list__list_shows_warning_on_git_error.snap index 4f8fe21892..c4834ba0b9 100644 --- a/tests/snapshots/integration__integration_tests__list__list_shows_warning_on_git_error.snap +++ b/tests/snapshots/integration__integration_tests__list__list_shows_warning_on_git_error.snap @@ -54,7 +54,7 @@ exit_code: 0 ----- stderr ----- ▲ Failed to batch-fetch commit details: fatal: bad object 0000000000000000000000000000000000000001 -○ Showing 5 worktrees, 3 ahead; 10 tasks failed +○ Showing 5 worktrees, 3 ahead ▲ 10 tasks failed:   main: upstream status (fatal: missing object 0000000000000000000000000000000000000001 for refs/heads/feature)   feature: ahead/behind counts (git merge-base failed for 05a4a45d0b981dad5c27db59dca482836d59f89e 0000000000000000000000000000000000000001: fatal: Not a valid commit name 0000000000000000000000000000000000000001) diff --git a/tests/snapshots/integration__integration_tests__list__list_warns_when_commit_details_batch_fails.snap b/tests/snapshots/integration__integration_tests__list__list_warns_when_commit_details_batch_fails.snap index 78e6261567..d26d8f9755 100644 --- a/tests/snapshots/integration__integration_tests__list__list_warns_when_commit_details_batch_fails.snap +++ b/tests/snapshots/integration__integration_tests__list__list_warns_when_commit_details_batch_fails.snap @@ -54,7 +54,7 @@ exit_code: 0 ----- stderr ----- ▲ Failed to batch-fetch commit details: fatal: bad object 0000000000000000000000000000000000000001 -○ Showing 5 worktrees, 3 ahead; 6 tasks failed +○ Showing 5 worktrees, 3 ahead ▲ 6 tasks failed:   (detached): ahead/behind counts (git merge-base failed for 05a4a45d0b981dad5c27db59dca482836d59f89e 0000000000000000000000000000000000000001: fatal: Not a valid commit name 0000000000000000000000000000000000000001)   (detached): trees-match check (fatal: Needed a single revision)