diff --git a/crates/openab-core/src/adapter.rs b/crates/openab-core/src/adapter.rs index 1cdf29fd0..844cefbab 100644 --- a/crates/openab-core/src/adapter.rs +++ b/crates/openab-core/src/adapter.rs @@ -1401,10 +1401,59 @@ impl ToolEntry { } } -/// Maximum number of finished tool entries to show individually -/// during streaming before collapsing into a summary line. +/// Maximum number of **post-grouping** finished/running lines to show +/// individually during streaming before collapsing into a summary line. +/// +/// Gates both the streaming finished-branch and the streaming +/// running-branch, and is compared against grouped-line count (a run of +/// N identical repeats counts as ONE line, not N). Once the grouped-line +/// count itself exceeds this threshold the fallback path still fires โ€” +/// the finished branch shows the raw-count summary `โœ… N tool(s) completed` +/// and the running branch shows `๐Ÿ”ง N more running` + the trailing few +/// visible groups. const TOOL_COLLAPSE_THRESHOLD: usize = 3; +/// Collapse a sequence of tool entries into one entry per **consecutive** run +/// of same `(title, state)` โ€” the tuple carries the count. +/// +/// Called ONCE over the full unfiltered `tool_lines` slice so adjacency is +/// evaluated in true call order. Callers then filter the resulting groups by +/// state; this prevents `A(Completed), B(Running), A(Completed)` from folding +/// to `A(Completed)ร—2` after the caller strips Running entries first. +fn group_entries<'a>( + entries: impl IntoIterator, +) -> Vec<(String, ToolState, usize)> { + let mut out: Vec<(String, ToolState, usize)> = Vec::new(); + for e in entries { + match out.last_mut() { + Some((t, s, n)) if *t == e.title && *s == e.state => *n += 1, + _ => out.push((e.title.clone(), e.state, 1)), + } + } + out +} + +/// Render one grouped entry. Repeats append ` (ร—N)`; for `Running` the count +/// sits BEFORE the trailing `...` so the string reads +/// ``๐Ÿ”ง `curl` (ร—3)...`` instead of ``๐Ÿ”ง `curl`... (ร—3)``. +fn render_group(title: &str, state: ToolState, count: usize) -> String { + let base = ToolEntry { + id: String::new(), + title: title.to_string(), + state, + } + .render(); + if count <= 1 { + return base; + } + if state == ToolState::Running { + let trimmed = base.trim_end_matches("..."); + format!("{trimmed} (ร—{count})...") + } else { + format!("{base} (ร—{count})") + } +} + // --- Empty-turn classification (pure helper, unit-testable) --- /// Message to show the consumer when a silent failure is detected. @@ -1445,7 +1494,6 @@ fn compose_display( .iter() .filter(|e| e.state == ToolState::Running) .count(); - let finished = done + failed; match tool_display { ToolDisplay::Compact => { @@ -1465,15 +1513,29 @@ fn compose_display( } } ToolDisplay::Full => { + // Group once over the FULL sequence so adjacency reflects + // true call order (a Running entry between two identical + // Completed entries splits them into two groups, not one). + let groups = group_entries(tool_lines.iter()); + let finished_groups: Vec<&(String, ToolState, usize)> = groups + .iter() + .filter(|(_, s, _)| *s != ToolState::Running) + .collect(); + let running_groups: Vec<&(String, ToolState, usize)> = groups + .iter() + .filter(|(_, s, _)| *s == ToolState::Running) + .collect(); + if streaming { - let running_entries: Vec<_> = tool_lines - .iter() - .filter(|e| e.state == ToolState::Running) - .collect(); - - if finished <= TOOL_COLLAPSE_THRESHOLD { - for entry in tool_lines.iter().filter(|e| e.state != ToolState::Running) { - out.push_str(&entry.render()); + // Threshold on GROUPED-line count, not raw entries: N + // repeats of a single tool count as 1, so 4ร— the same + // tool renders as `โœ… X (ร—4)`. The `>THRESHOLD` fallback + // still fires once the number of distinct groups itself + // exceeds the threshold โ€” that summary reports raw call + // counts (`โœ… N ยท โŒ M tool(s) completed`). + if finished_groups.len() <= TOOL_COLLAPSE_THRESHOLD { + for (t, s, n) in &finished_groups { + out.push_str(&render_group(t, *s, *n)); out.push('\n'); } } else { @@ -1487,22 +1549,33 @@ fn compose_display( out.push_str(&format!("{} tool(s) completed\n", parts.join(" ยท "))); } - if running_entries.len() <= TOOL_COLLAPSE_THRESHOLD { - for entry in &running_entries { - out.push_str(&entry.render()); + if running_groups.len() <= TOOL_COLLAPSE_THRESHOLD { + for (t, s, n) in &running_groups { + out.push_str(&render_group(t, *s, *n)); out.push('\n'); } } else { - let hidden = running_entries.len() - TOOL_COLLAPSE_THRESHOLD; - out.push_str(&format!("๐Ÿ”ง {hidden} more running\n")); - for entry in running_entries.iter().skip(hidden) { - out.push_str(&entry.render()); + // Index by group boundary (never split a run) but + // report the summary in tool-call units so the number + // matches the sibling finished-fallback summary and + // the pre-PR raw-count behaviour that users are used + // to. A hidden group of `aร—2` contributes 2, not 1. + let hidden_groups = + running_groups.len() - TOOL_COLLAPSE_THRESHOLD; + let hidden_calls: usize = running_groups + .iter() + .take(hidden_groups) + .map(|(_, _, n)| *n) + .sum(); + out.push_str(&format!("๐Ÿ”ง {hidden_calls} more running\n")); + for (t, s, n) in running_groups.iter().skip(hidden_groups) { + out.push_str(&render_group(t, *s, *n)); out.push('\n'); } } } else { - for entry in tool_lines { - out.push_str(&entry.render()); + for (t, s, n) in &groups { + out.push_str(&render_group(t, *s, *n)); out.push('\n'); } } @@ -1908,6 +1981,254 @@ mod tests { assert!(out.contains("๐Ÿ”ง 1"), "expected running count: {out}"); } + #[test] + fn compose_display_full_collapses_consecutive_duplicates() { + // Claude often calls the same tool multiple times in a row (e.g. three + // ToolSearch calls to look up related MCP tools). Show one line with a + // ร—N suffix instead of three identical lines. + let tools = vec![ + tool("1", "ToolSearch", ToolState::Completed), + tool("2", "ToolSearch", ToolState::Completed), + tool("3", "ToolSearch", ToolState::Completed), + ]; + let out = compose_display(&tools, "done", false, ToolDisplay::Full); + assert!( + out.contains("`ToolSearch` (ร—3)"), + "expected grouped line: {out}" + ); + // Must render only one tool line, not three + assert_eq!(out.matches("`ToolSearch`").count(), 1, "output: {out}"); + } + + #[test] + fn compose_display_full_preserves_order_across_different_titles() { + // A `curl` between two `grep`s should not merge the grep entries. + let tools = vec![ + tool("1", "grep", ToolState::Completed), + tool("2", "curl", ToolState::Completed), + tool("3", "grep", ToolState::Completed), + ]; + let out = compose_display(&tools, "done", false, ToolDisplay::Full); + assert!(!out.contains("(ร—"), "should not collapse across order: {out}"); + assert_eq!(out.matches("`grep`").count(), 2, "output: {out}"); + assert_eq!(out.matches("`curl`").count(), 1, "output: {out}"); + } + + #[test] + fn compose_display_full_groups_mixed_state_runs_separately() { + // Same title but different states must NOT merge (completed vs failed). + let tools = vec![ + tool("1", "curl", ToolState::Completed), + tool("2", "curl", ToolState::Failed), + tool("3", "curl", ToolState::Failed), + ]; + let out = compose_display(&tools, "done", false, ToolDisplay::Full); + assert!(out.contains("โœ… `curl`"), "output: {out}"); + assert!(out.contains("โŒ `curl` (ร—2)"), "output: {out}"); + } + + #[test] + fn compose_display_full_streaming_groups_beyond_threshold_dups() { + // 5 identical entries: raw count 5 > TOOL_COLLAPSE_THRESHOLD (3), but + // group count is 1, so the grouped line MUST render โ€” never the + // generic "5 tool(s) completed" fallback. Regression test for the + // reviewer's finding that the threshold used to gate on raw count. + let tools = vec![ + tool("1", "ToolSearch", ToolState::Completed), + tool("2", "ToolSearch", ToolState::Completed), + tool("3", "ToolSearch", ToolState::Completed), + tool("4", "ToolSearch", ToolState::Completed), + tool("5", "ToolSearch", ToolState::Completed), + ]; + let out = compose_display(&tools, "done", true, ToolDisplay::Full); + assert!( + out.contains("`ToolSearch` (ร—5)"), + "expected grouped ร—5 line: {out}" + ); + assert!( + !out.contains("5 tool(s) completed"), + "should not fall back to count summary: {out}" + ); + } + + #[test] + fn compose_display_full_streaming_running_dups_collapse() { + // The streaming Running branch also has to collapse identical + // in-flight tool invocations (rare in claude-agent-acp, common with + // parallel-tool-call backends). (ร—N) sits BEFORE the `...` so the + // marker keeps its "still working" meaning. + let tools = vec![ + tool("1", "curl", ToolState::Running), + tool("2", "curl", ToolState::Running), + tool("3", "curl", ToolState::Running), + ]; + let out = compose_display(&tools, "", true, ToolDisplay::Full); + assert!( + out.contains("`curl` (ร—3)..."), + "expected running (ร—N) before ...: {out}" + ); + assert_eq!(out.matches("`curl`").count(), 1, "output: {out}"); + } + + #[test] + fn compose_display_full_streaming_true_order_preserved_across_state_boundaries() { + // Reviewer finding #1: A(Completed), B(Running), A(Completed) must + // NOT collapse into A(ร—2). Filtering Running out AFTER grouping (as + // this PR now does) keeps the two A entries as distinct groups so + // the finished-view still shows two lines, matching true call order. + let tools = vec![ + tool("1", "ToolSearch", ToolState::Completed), + tool("2", "Bash", ToolState::Running), + tool("3", "ToolSearch", ToolState::Completed), + ]; + let out = compose_display(&tools, "", true, ToolDisplay::Full); + assert!( + !out.contains("(ร—2)"), + "must not merge non-adjacent finished entries across a Running: {out}" + ); + assert_eq!( + out.matches("`ToolSearch`").count(), + 2, + "expected two separate ToolSearch lines: {out}" + ); + assert!(out.contains("`Bash`"), "output: {out}"); + } + + #[test] + fn compose_display_full_streaming_running_hidden_count_is_tool_calls_not_groups() { + // Fixture: 7 running entries in 5 groups โ€” a(ร—2), b(ร—2), c, d, e. + // At `TOOL_COLLAPSE_THRESHOLD = 3`, hidden_groups = 2 (`a` + `b`), + // representing 4 tool calls. The summary must say "4 more running", + // matching the raw-count units used by the sibling finished-branch + // fallback and the pre-PR behaviour. Regression test for reviewer + // finding F1: previously the summary reported hidden group count. + let tools = vec![ + tool("1", "a", ToolState::Running), + tool("2", "a", ToolState::Running), + tool("3", "b", ToolState::Running), + tool("4", "b", ToolState::Running), + tool("5", "c", ToolState::Running), + tool("6", "d", ToolState::Running), + tool("7", "e", ToolState::Running), + ]; + let out = compose_display(&tools, "", true, ToolDisplay::Full); + assert!( + out.contains("๐Ÿ”ง 4 more running"), + "hidden summary must count tool calls: {out}" + ); + assert!( + !out.contains("๐Ÿ”ง 2 more running"), + "must not report hidden group count: {out}" + ); + // Visible tail = last THRESHOLD groups = c, d, e (each ร—1). + // Neither hidden group (a, b) should appear in the visible tail. + assert!(!out.contains("`a`"), "`a` should be hidden: {out}"); + assert!(!out.contains("`b`"), "`b` should be hidden: {out}"); + assert!(out.contains("๐Ÿ”ง `c`..."), "output: {out}"); + assert!(out.contains("๐Ÿ”ง `d`..."), "output: {out}"); + assert!(out.contains("๐Ÿ”ง `e`..."), "output: {out}"); + } + + #[test] + fn compose_display_full_streaming_hidden_boundary_preserves_group() { + // Fixture per reviewer F2: `[a, b, b, c, d]` โ€” 5 entries, 4 groups. + // With correct group-boundary skipping, `a` is hidden and the + // visible tail is `b(ร—2), c, d`. A regression to raw-entry skipping + // would hide `[a, b]` (leaving a bare `b, c, d` and losing the + // `(ร—2)` collapse). Pinning the exact strings catches both the raw + // vs group indexing bug AND F1 (hidden count in tool-call units: + // 1 group hidden = 1 tool hidden). + let tools = vec![ + tool("1", "a", ToolState::Running), + tool("2", "b", ToolState::Running), + tool("3", "b", ToolState::Running), + tool("4", "c", ToolState::Running), + tool("5", "d", ToolState::Running), + ]; + let out = compose_display(&tools, "", true, ToolDisplay::Full); + assert!( + out.contains("๐Ÿ”ง 1 more running"), + "expected exact hidden count 1: {out}" + ); + assert!( + out.contains("๐Ÿ”ง `b` (ร—2)..."), + "grouped `b (ร—2)` must survive in visible tail: {out}" + ); + assert_eq!( + out.matches("`b`").count(), + 1, + "must not split the grouped `b` run across boundary: {out}" + ); + assert!(out.contains("๐Ÿ”ง `c`..."), "output: {out}"); + assert!(out.contains("๐Ÿ”ง `d`..."), "output: {out}"); + assert!(!out.contains("`a`"), "`a` should be hidden: {out}"); + } + + #[test] + fn compose_display_full_streaming_finished_fallback_reports_raw_counts() { + // >TOOL_COLLAPSE_THRESHOLD distinct FINISHED groups triggers the + // fallback branch that was previously untested (reviewer F5). The + // summary must report raw call counts (deliberately different units + // from the group-count threshold gate above it): `a(ร—2) + b + c + d` + // = 5 successes, plus a failed `e` = 1 failure. String is exactly + // "โœ… 5 ยท โŒ 1 tool(s) completed". + let tools = vec![ + tool("1", "a", ToolState::Completed), + tool("2", "a", ToolState::Completed), + tool("3", "b", ToolState::Completed), + tool("4", "c", ToolState::Completed), + tool("5", "d", ToolState::Completed), + tool("6", "e", ToolState::Failed), + ]; + let out = compose_display(&tools, "answer", true, ToolDisplay::Full); + assert!( + out.contains("โœ… 5 ยท โŒ 1 tool(s) completed"), + "expected raw-count fallback summary: {out}" + ); + // Individual lines must NOT appear (we're in the fallback branch). + assert!(!out.contains("`a`"), "individual lines suppressed: {out}"); + assert!(!out.contains("(ร—2)"), "grouped line suppressed: {out}"); + } + + #[test] + fn compose_display_full_streaming_finished_at_threshold_shows_lines() { + // Boundary: EXACTLY `TOOL_COLLAPSE_THRESHOLD` distinct groups still + // renders individual lines (gate uses `<=`). Companion to the >3 + // fallback test above โ€” together they pin the boundary against a + // silent `<=` โ†’ `<` regression. Reviewer F4. + let tools = vec![ + tool("1", "a", ToolState::Completed), + tool("2", "b", ToolState::Completed), + tool("3", "c", ToolState::Completed), + ]; + let out = compose_display(&tools, "answer", true, ToolDisplay::Full); + assert!(out.contains("โœ… `a`"), "output: {out}"); + assert!(out.contains("โœ… `b`"), "output: {out}"); + assert!(out.contains("โœ… `c`"), "output: {out}"); + assert!( + !out.contains("tool(s) completed"), + "must not fall through to summary at threshold: {out}" + ); + } + + #[test] + fn compose_display_full_streaming_running_at_threshold_shows_lines() { + // Same boundary check for the running branch. + let tools = vec![ + tool("1", "a", ToolState::Running), + tool("2", "b", ToolState::Running), + tool("3", "c", ToolState::Running), + ]; + let out = compose_display(&tools, "", true, ToolDisplay::Full); + assert!(out.contains("๐Ÿ”ง `a`..."), "output: {out}"); + assert!(out.contains("๐Ÿ”ง `b`..."), "output: {out}"); + assert!(out.contains("๐Ÿ”ง `c`..."), "output: {out}"); + assert!( + !out.contains("more running"), + "must not fall through to summary at threshold: {out}" + ); + } + #[test] fn compose_display_none_hides_tools() { let tools = vec![tool( diff --git a/docs/tool-display.md b/docs/tool-display.md index 553b1451d..6bc89ef96 100644 --- a/docs/tool-display.md +++ b/docs/tool-display.md @@ -22,11 +22,11 @@ agents: ### `full` (default) -Shows each tool call with its complete title. When more than 3 tools finish, they collapse into a count summary automatically. +Shows each tool call with its complete title. **Consecutive** repeats of the same tool are collapsed into a single line with an `(ร—N)` suffix, so a burst like three back-to-back `ToolSearch` calls renders as one line โ€” not three. A non-consecutive repeat (e.g. `curl โ†’ grep โ†’ curl`) still renders as three separate lines โ€” grouping is adjacency-only, order-preserving. When the resulting **run count** exceeds 3 for either the finished or the still-running set mid-stream, that set collapses into a raw-count summary (`โœ… 5 ยท โŒ 1 tool(s) completed` / `๐Ÿ”ง 4 more running` + the trailing few grouped runs). ``` โœ… `curl -s "https://ghcr.io/v2/openabdev/charts/openab/tags/list"` -โœ… `grep -r "pattern" src/` +โœ… `grep -r "pattern" src/` (ร—2) ๐Ÿ”ง `npm install`... Agent response text here... @@ -68,4 +68,4 @@ Best for: clean output when you only care about the final answer. - **Default**: `full` shows complete tool titles. Use `tool_display = "compact"` for a cleaner count-only summary, or `"none"` to hide tools entirely. - **Reaction emojis are independent**: The emoji reactions on messages (๐Ÿ‘€โ†’๐Ÿค”โ†’๐Ÿ”งโ†’๐Ÿ†—) work regardless of `tool_display` setting. -- **Streaming behavior**: In `compact` mode, the count updates in real-time as tools start and finish. In `full` mode, individual tool lines appear and update during streaming. +- **Streaming behavior**: In `compact` mode, the count updates in real-time as tools start and finish. In `full` mode, individual and grouped-repeat lines appear up to 3 runs per set (finished / running); once either set has more than 3 runs, that set switches to a raw-count summary (`โœ… N ยท โŒ M tool(s) completed` / `๐Ÿ”ง N more running` + the trailing groups).