diff --git a/client-core/src/durations.rs b/client-core/src/durations.rs new file mode 100644 index 0000000..9adb933 --- /dev/null +++ b/client-core/src/durations.rs @@ -0,0 +1,100 @@ +//! A span of milliseconds, written the way somebody reads it -- the port +//! of `Durations.kt`'s `formatMillis`/`formatMillisText`, with its tests. +//! +//! Only the tool-timeout half is here. `formatSpan` (the usage +//! countdown's rounding-up rule) belongs with whatever draws the usage +//! bar, and nothing in this crate needs it yet. + +/// A span of milliseconds, written the way somebody reads it. +/// +/// A tool's timeout arrives as `480000`, which nobody reads as eight +/// minutes. The rule has two halves, because a short span and a long one +/// are read for different things. Under a minute the question is "roughly +/// how long", so only the largest unit is shown and a fraction carries the +/// rest -- `2.5s`. At a minute or more the question is "how long exactly", +/// so every unit with something in it is written out -- `5d 12h 4m`. Empty +/// units are left out rather than written as zero. +/// +/// Sub-second precision is dropped past a minute: nothing that takes days +/// is measured in milliseconds. +pub fn format_millis(ms: i64) -> String { + if ms < 0 { + return format!("-{}", format_millis(-ms)); + } + if ms < 1000 { + return format!("{ms}ms"); + } + if ms < 60_000 { + let tenths = (ms + 50) / 100; + let (whole, rest) = (tenths / 10, tenths % 10); + return if rest == 0 { + format!("{whole}s") + } else { + format!("{whole}.{rest}s") + }; + } + let seconds = ms / 1000; + [ + ("d", seconds / 86_400), + ("h", seconds / 3600 % 24), + ("m", seconds / 60 % 60), + ("s", seconds % 60), + ] + .iter() + .filter(|(_, n)| *n > 0) + .map(|(unit, n)| format!("{n}{unit}")) + .collect::>() + .join(" ") +} + +/// `text` as a span when it is a whole number of milliseconds, and +/// unchanged when it is not. +pub fn format_millis_text(text: &str) -> String { + match text.trim().parse::() { + Ok(ms) => format_millis(ms), + Err(_) => text.to_string(), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The two ways a span of time is written here, and the rule each of + /// them follows -- ported from `DurationsTest.kt`, whose doc says why: + /// both are read off a screen to make a decision, so what matters is + /// that the shortest form that answers the question is what appears. + #[test] + fn under_a_minute_is_the_largest_unit_alone() { + assert_eq!(format_millis(30), "30ms"); + assert_eq!(format_millis(999), "999ms"); + assert_eq!(format_millis(1000), "1s"); + assert_eq!(format_millis(2500), "2.5s"); + // One decimal, rounded rather than cut: 2.46s is nearer two and a + // half than two and four. + assert_eq!(format_millis(2460), "2.5s"); + assert_eq!(format_millis(59_900), "59.9s"); + } + + #[test] + fn a_minute_or_more_is_every_unit_that_has_something_in_it() { + // The figure this rule was written for: a tool timeout, which + // arrives as milliseconds and is unreadable as 480000. + assert_eq!(format_millis(480_000), "8m"); + assert_eq!(format_millis(60_000), "1m"); + assert_eq!(format_millis(90_000), "1m 30s"); + assert_eq!(format_millis(475_440_000), "5d 12h 4m"); + // Empty units are left out rather than written as zero: the labels + // say which is which, and "5d 0h 4m" is only longer. + assert_eq!(format_millis(432_240_000), "5d 4m"); + } + + #[test] + fn only_a_whole_number_of_milliseconds_is_rewritten() { + assert_eq!(format_millis_text(" 480000 "), "8m"); + // A timeout a tool expressed some other way is its own words, + // passed through rather than guessed at. + assert_eq!(format_millis_text("2 minutes"), "2 minutes"); + assert_eq!(format_millis_text(""), ""); + } +} diff --git a/client-core/src/lib.rs b/client-core/src/lib.rs index d94e4f5..c7a91d8 100644 --- a/client-core/src/lib.rs +++ b/client-core/src/lib.rs @@ -5,11 +5,13 @@ pub mod ansi; pub mod api; pub mod config; +pub mod durations; pub mod event_stream; pub mod highlight; pub mod markdown_blocks; pub mod notifications; pub mod sse; +pub mod tool_summary; pub mod transcript_cache; pub mod transcript_fold; pub mod transcript_source; diff --git a/client-core/src/tool_summary.rs b/client-core/src/tool_summary.rs new file mode 100644 index 0000000..6bcb91a --- /dev/null +++ b/client-core/src/tool_summary.rs @@ -0,0 +1,244 @@ +//! A tool call's input, read rather than dumped -- the port of +//! `ToolInput.kt`'s `parseToolInput`, which is what both the collapsed +//! card's one-line summary and the expanded card's key/value list are +//! derived from. +//! +//! Every tool's input arrives as JSON, and showing it raw makes the reader +//! parse `{"command":"…","timeout":120000}` themselves to find the one +//! line they care about. So the fields that carry the meaning are pulled +//! out, and anything left over is still shown, because dropping a field +//! would be claiming the tool has no other input when it might. +//! +//! Pure, and here rather than in the widget crate, for the reason the rest +//! of this crate exists: the derivation is the same on a phone and on a +//! desktop, and it is testable without a renderer. + +use crate::durations::format_millis_text; +use crate::highlight::Language; +use serde_json::{Map, Value}; + +/// A tool call's input, split into the parts a card draws separately. +#[derive(Debug, Clone, PartialEq, Eq, Default)] +pub struct ToolInput { + /// The thing that will actually be run or read, if this tool has one. + pub subject: Option, + /// The language [`ToolInput::subject`] is written in, for + /// highlighting. + pub language: Option, + /// The tool's own one-line summary, when it wrote one. + pub description: Option, + /// How long the call may take, in the largest units it fits. Shown + /// apart because it is a limit on the call rather than part of what + /// the call does. + pub timeout: Option, + /// Everything else, as `name: value` lines. Never dropped. + pub rest: Vec, +} + +impl ToolInput { + /// The one line to show when there is only room for one: what this + /// call is for. + pub fn title(&self) -> Option<&str> { + self.description + .as_deref() + .or(self.subject.as_deref()) + // A subject that is only whitespace would draw as an empty + // summary line, which reads as a tool with nothing to say + // rather than as one whose subject was blank. + .filter(|t| !t.trim().is_empty()) + } +} + +/// Which field of which tool is the subject. +/// +/// A table rather than a chain of `if`s: adding a tool is a row, and the +/// shape stops any of them from being the special case that gets its own +/// code path. Unknown tools fall through to "no subject, everything is +/// rest". +const SUBJECTS: &[(&str, &str, Option)] = &[ + ("Bash", "command", Some(Language::Shell)), + ("Read", "file_path", None), + ("Write", "file_path", None), + ("Edit", "file_path", None), + ("Glob", "pattern", None), + ("Grep", "pattern", None), + ("WebFetch", "url", None), +]; + +/// Fields that are the tool's own prose about itself rather than input to +/// it. +const DESCRIPTIONS: &[&str] = &["description", "prompt"]; + +/// One JSON value as the Kotlin's `JSONObject.optString`/`get` wrote it: a +/// string is its own characters, anything else is its JSON form. +/// +/// One function rather than two, because the same coercion decides both +/// what a subject reads as and what a leftover field's value reads as, and +/// two copies would eventually disagree about a number. +fn as_text(value: &Value) -> String { + match value { + Value::String(s) => s.clone(), + other => other.to_string(), + } +} + +fn non_blank(value: Option<&Value>) -> Option { + let text = as_text(value?); + (!text.trim().is_empty()).then_some(text) +} + +/// Split `input` (a tool call's JSON) into the parts a card draws. +/// +/// Input that is not a JSON object -- older transcripts and some tools +/// send a bare string -- is still the input, so it is still shown, as the +/// whole of `rest`. +pub fn parse_tool_input(tool: &str, input: &str) -> ToolInput { + let Ok(Value::Object(json)) = serde_json::from_str::(input) else { + return ToolInput { + rest: match input.trim().is_empty() { + true => Vec::new(), + false => vec![input.to_string()], + }, + ..ToolInput::default() + }; + }; + parse_object(tool, &json) +} + +fn parse_object(tool: &str, json: &Map) -> ToolInput { + let (subject_key, language) = SUBJECTS + .iter() + .find(|(name, ..)| *name == tool) + .map(|(_, key, language)| (Some(*key), *language)) + .unwrap_or((None, None)); + let subject = subject_key.and_then(|key| non_blank(json.get(key))); + let description = DESCRIPTIONS + .iter() + .find_map(|key| non_blank(json.get(*key))); + let timeout = non_blank(json.get("timeout")).map(|t| format_millis_text(&t)); + + // Sorted, so the leftovers are in the same order every time this call + // is drawn rather than in whatever order the JSON happened to arrive + // in. A field is left out only when it is already drawn somewhere + // else on the card. + let mut keys: Vec<&String> = json + .keys() + .filter(|k| Some(k.as_str()) != subject_key || subject.is_none()) + .filter(|k| !DESCRIPTIONS.contains(&k.as_str()) || description.is_none()) + .filter(|k| k.as_str() != "timeout" || timeout.is_none()) + .collect(); + keys.sort(); + let rest = keys + .into_iter() + .map(|key| format!("{key}: {}", as_text(&json[key]))) + .collect(); + + ToolInput { + subject, + language, + description, + timeout, + rest, + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn each_tool_in_the_table_has_its_own_subject() { + // One assertion per row of `SUBJECTS`, because the table is the + // whole of the rule and a row lost in an edit would otherwise + // only show up as a card with no summary line. + let cases = [ + ("Bash", r#"{"command":"ls -la"}"#, "ls -la"), + ("Read", r#"{"file_path":"/tmp/x.rs"}"#, "/tmp/x.rs"), + ("Write", r#"{"file_path":"/tmp/y.rs"}"#, "/tmp/y.rs"), + ("Edit", r#"{"file_path":"/tmp/z.rs"}"#, "/tmp/z.rs"), + ("Glob", r#"{"pattern":"**/*.rs"}"#, "**/*.rs"), + ("Grep", r#"{"pattern":"fn main"}"#, "fn main"), + ("WebFetch", r#"{"url":"https://x/y"}"#, "https://x/y"), + ]; + for (tool, input, expected) in cases { + let parsed = parse_tool_input(tool, input); + assert_eq!(parsed.subject.as_deref(), Some(expected), "{tool}"); + assert_eq!(parsed.title(), Some(expected), "{tool}"); + assert!(parsed.rest.is_empty(), "{tool}: {:?}", parsed.rest); + } + assert_eq!( + parse_tool_input("Bash", r#"{"command":"ls"}"#).language, + Some(Language::Shell), + "a Bash command is shell, and is the one row that names a language" + ); + } + + #[test] + fn a_tools_own_description_is_what_the_one_line_says() { + // The description wins over the subject: it is the tool's own + // prose about what this call is for, which is what a reader + // scanning a collapsed run is looking for. + let parsed = parse_tool_input( + "Bash", + r#"{"command":"cargo test -p iris","description":"Run the iris tests"}"#, + ); + assert_eq!(parsed.title(), Some("Run the iris tests")); + assert_eq!(parsed.subject.as_deref(), Some("cargo test -p iris")); + assert!(parsed.rest.is_empty(), "{:?}", parsed.rest); + } + + #[test] + fn a_timeout_is_read_as_a_span_and_kept_apart_from_the_rest() { + let parsed = parse_tool_input("Bash", r#"{"command":"sleep 500","timeout":480000}"#); + assert_eq!(parsed.timeout.as_deref(), Some("8m")); + assert!(parsed.rest.is_empty(), "{:?}", parsed.rest); + } + + #[test] + fn every_field_not_drawn_elsewhere_is_still_shown() { + // The half the "never dropped" promise is about: a tool this + // build has never heard of has no subject, so *everything* is + // rest -- and a known tool's extra fields are too. + let parsed = parse_tool_input( + "Edit", + r#"{"file_path":"/a.rs","old_string":"x","new_string":"y","replace_all":true}"#, + ); + assert_eq!( + parsed.rest, + vec![ + "new_string: y".to_string(), + "old_string: x".to_string(), + "replace_all: true".to_string(), + ], + "sorted, and a non-string value written as JSON" + ); + let unknown = parse_tool_input("SomeNewTool", r#"{"b":2,"a":"one"}"#); + assert_eq!(unknown.subject, None); + assert_eq!(unknown.rest, vec!["a: one".to_string(), "b: 2".to_string()]); + } + + #[test] + fn input_that_is_not_an_object_is_still_the_input() { + // Older transcripts and some tools send a bare string; a card + // that dropped it would claim the call had no input at all. + assert_eq!( + parse_tool_input("Bash", "just a string").rest, + vec!["just a string".to_string()] + ); + assert_eq!(parse_tool_input("Bash", " ").rest, Vec::::new()); + assert_eq!(parse_tool_input("Bash", "").title(), None); + } + + #[test] + fn a_blank_subject_is_no_subject_rather_than_an_empty_summary_line() { + let parsed = parse_tool_input("Bash", r#"{"command":" ","other":1}"#); + assert_eq!(parsed.subject, None); + assert_eq!(parsed.title(), None); + // Not dropped just because it was blank -- it is still a field + // the call carried. + assert_eq!( + parsed.rest, + vec!["command: ".to_string(), "other: 1".to_string()] + ); + } +} diff --git a/client-core/src/transcript_fold.rs b/client-core/src/transcript_fold.rs index a4cf03c..42030a4 100644 --- a/client-core/src/transcript_fold.rs +++ b/client-core/src/transcript_fold.rs @@ -78,6 +78,11 @@ pub enum TranscriptItem { input: String, output: String, done: bool, + /// Whether the result that arrived said the call failed + /// ([`Event::ToolEnd`]'s `is_error`). Meaningless while `done` is + /// false, and [`ToolState::of`] is the only thing that reads the + /// pair, so the two cannot be combined wrongly at a call site. + failed: bool, asks: Vec, images: Vec, }, @@ -352,6 +357,7 @@ pub fn join_pages(earlier: &[TranscriptItem], later: &[TranscriptItem]) -> Vec Vec Vec Vec { + Event::ToolEnd { + id, + output, + is_error, + } => { if items.iter().any(|i| i.as_tool_run() == Some(id.as_str())) { update_tool(items, id, |item| { if let TranscriptItem::ToolRun { - output: out, done, .. + output: out, + done, + failed, + .. } = item { *out = output.clone(); *done = true; + *failed = *is_error; } }) } else { @@ -594,6 +610,7 @@ pub fn fold_event(items: &[TranscriptItem], entry: &SeqEvent) -> Vec Vec { for ask in asks.iter_mut() { @@ -665,6 +683,7 @@ pub fn fold_event(items: &[TranscriptItem], entry: &SeqEvent) -> Vec Vec Option { + let TranscriptItem::ToolRun { + done, failed, asks, .. + } = item + else { + return None; + }; + debug_assert!( + !failed || *done, + "a call cannot have failed before its result arrived" + ); + Some(if asks.iter().any(|ask| ask.answers.is_empty()) { + // Ahead of `done`: a call waiting on permission has not + // finished either, and which of the two the reader is being + // told about is the one they can act on. + Self::Deciding + } else if !*done { + match session_working { + true => Self::Running, + false => Self::NoResult, + } + } else if *failed { + Self::Failed + } else { + Self::Succeeded + }) + } +} + /// One row as the transcript draws it: a run of consecutive tool calls, or /// anything else. Ported from `ToolRows.kt`'s `TranscriptRow` and /// `groupToolRuns` -- the Compose card rendering in that file is not part @@ -989,6 +1076,7 @@ mod tests { Event::ToolEnd { id: "x".to_string(), output: "done".to_string(), + is_error: false, }, )]); assert_eq!( @@ -1001,6 +1089,7 @@ mod tests { input: String::new(), output: "done".to_string(), done: true, + failed: false, asks: Vec::new(), images: Vec::new(), }] @@ -1166,6 +1255,7 @@ mod tests { Event::ToolEnd { id: id.to_string(), output: output.to_string(), + is_error: false, }, ) } @@ -1211,6 +1301,7 @@ mod tests { input: "{}".to_string(), output: "the result".to_string(), done: true, + failed: false, asks: Vec::new(), images: Vec::new(), }], @@ -1269,6 +1360,7 @@ mod tests { input: "{}".to_string(), output: String::new(), done: false, + failed: false, asks: Vec::new(), images: Vec::new(), }]; @@ -1279,3 +1371,150 @@ mod tests { } } } + +/// [`ToolState`] is what a card colours itself by, so each of its five +/// states is asserted from the events that actually produce it rather than +/// from a hand-built item -- a mapping that agreed with a fixture and +/// disagreed with the fold would be invisible until it was on screen. +#[cfg(test)] +mod tool_state_tests { + use super::*; + + fn event(seq: u64, e: Event) -> SeqEvent { + SeqEvent { + seq, + ts: 0.0, + event: e, + } + } + + fn fold_all(events: &[SeqEvent]) -> Vec { + events + .iter() + .fold(Vec::new(), |items, e| fold_event(&items, e)) + } + + fn start(id: &str) -> SeqEvent { + event( + 1, + Event::ToolStart { + id: id.to_string(), + tool: "Bash".to_string(), + input: serde_json::json!({"command": "ls"}), + }, + ) + } + + fn end(id: &str, output: &str, is_error: bool) -> SeqEvent { + event( + 2, + Event::ToolEnd { + id: id.to_string(), + output: output.to_string(), + is_error, + }, + ) + } + + fn state_of(events: &[SeqEvent], session_working: bool) -> ToolState { + let items = fold_all(events); + ToolState::of(&items[0], session_working).expect("the fixture's first item is a tool call") + } + + #[test] + fn a_result_that_arrived_is_read_from_is_error() { + assert_eq!( + state_of(&[start("a"), end("a", "ok", false)], false), + ToolState::Succeeded + ); + assert_eq!( + state_of(&[start("a"), end("a", "No such file", true)], false), + ToolState::Failed + ); + } + + /// The pair this enum exists for. Both calls have an empty `output` + /// and nothing else distinguishes them, so a card that only looked at + /// the text would draw the interrupted one as a call that ran fine and + /// printed nothing. + #[test] + fn a_call_that_printed_nothing_is_not_a_call_that_never_answered() { + assert_eq!( + state_of(&[start("a"), end("a", "", false)], false), + ToolState::Succeeded, + "a result arrived; it was empty" + ); + assert_eq!( + state_of(&[start("a")], false), + ToolState::NoResult, + "no result, and the session is not working any more" + ); + } + + /// The same call, mid-turn: still running rather than abandoned. The + /// only thing separating the two is the session's own status, which is + /// why `of` takes it. + #[test] + fn no_result_while_the_session_works_is_still_running() { + assert_eq!(state_of(&[start("a")], true), ToolState::Running); + } + + #[test] + fn an_unanswered_ask_is_the_readers_move_whatever_else_is_true() { + let asking = event( + 3, + Event::Question { + id: "q1".to_string(), + prompt: "Allow?".to_string(), + header: None, + options: vec![QuestionOption { + label: "Allow".to_string(), + description: None, + preview: None, + }], + multi_select: false, + about: Some("a".to_string()), + }, + ); + let answered = event( + 4, + Event::Answered { + id: "q1".to_string(), + answers: vec!["Allow".to_string()], + }, + ); + // Ahead of both "still running" and "no result": the reader can + // act on this one, and cannot act on either of those. + assert_eq!( + state_of(&[start("a"), asking.clone()], true), + ToolState::Deciding + ); + assert_eq!( + state_of(&[start("a"), asking.clone()], false), + ToolState::Deciding + ); + assert_eq!( + state_of( + &[start("a"), asking, answered, end("a", "ok", false)], + false + ), + ToolState::Succeeded, + "once it is answered the call is an ordinary one again" + ); + } + + #[test] + fn nothing_but_a_tool_call_has_a_tool_state() { + assert_eq!( + ToolState::of( + &TranscriptItem::UserMsg { + seq: 1, + text: "hi".to_string(), + attachments: Vec::new(), + }, + true + ), + None + ); + } +} diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md index 505e16f..6eb087a 100644 --- a/docs/DECISIONS.md +++ b/docs/DECISIONS.md @@ -5,6 +5,45 @@ they can be judged and reversed later. Detail lives in RUST.md (and IRIS.md for iris API changes); this file is only the summary. Newest first. Items marked **DEFERRED** are ones the agent chose not to decide alone. +## 2026-09-06 (how a tool call looks, P1b) + +- **A card that never got a result says "no result", in yellow, and it is + a state Compose cannot say.** A call that finished having printed + nothing and a call whose turn was interrupted before anything came back + both leave an empty output. Compose draws both as an ordinary finished + call, which reads as a fact somebody established. There are five states + now, each with a word and a colour: nothing at all for a call that + worked, "running" (grey), "your turn" (peach, Compose's own wording and + colour), "failed" (red), "no result" (yellow). + +- **A failed call is drawn as failed, which needed a field on the wire.** + `is_error` is on the CLI's `tool_result` and was being dropped; the + server now carries it to the phone. Reversible, but the alternative is a + card that says a call succeeded because it cannot tell. + +- **A group's cards do not each carry their own surface.** Compose gives + each card a fill and squares the corners where it faces a neighbour, so + a run reads as one object broken into parts. iris has no per-corner + radius, and -- more to the point -- a group built the way Compose builds + it hit a framework layout defect that drew every card's text a card + below its own box. So a group is one surface with its cards on it, + separated by a small gap, and the 4dp inset Compose holds them off the + edge by is gone. Worth revisiting once the layout defect is fixed + (docs/IRIS_TODO.md). + +- **A long tool output is capped at 80 lines or 4 kB with a "Show all N + lines".** Compose draws the whole thing, and gets away with it because + its `Text` inside a `LazyColumn` lays out lazily; here the output is one + text widget and shaping a hundred kilobytes of it costs what the file + editor's 32 kB limit was measured against. If iris's text gets cheaper, + this is the number to move. + +- **A card's command is clipped, not pannable, and its summary line is + clipped rather than ellipsised.** Both are framework gaps rather than + choices (`scrollable_on` on a non-editable text draws nothing; there is + no overflow ellipsis), and both are worse than Compose today. Named here + because they are visible. + ## 2026-09-06 (how a markdown block looks, P1a) - **A table is drawn as padded monospace columns, not as a grid.** Your diff --git a/docs/IRIS.md b/docs/IRIS.md index 8f1cbe7..9a5f02b 100644 --- a/docs/IRIS.md +++ b/docs/IRIS.md @@ -8,6 +8,51 @@ capability that moved. Small and trivial changes do not go here. An entry gives the date, what changed, why, and a short before/after where it helps judge the change without the session that made it. Newest first. +## 2026-09-06: tool cards, `ToolState`, and a screen that knows whether its session is working + +`transcript_ui::tool` is new: a card per tool call, a group per run +(P1b). Three things in the public surface follow from it. + +**`client_core::transcript_fold::ToolState`** is what a card colours +itself by -- `Running`, `Deciding`, `Succeeded`, `Failed`, `NoResult` -- +built by `ToolState::of(&item, session_working)`. The pair it exists for +is `Succeeded` against `NoResult`: a call that finished having printed +nothing and a call whose result never arrived both leave an empty +`output`, and drawing them the same way states a verdict nobody reached. +Only the session's own status separates them, which is why `of` takes it. + +**`event_model::Event::ToolEnd` gained `is_error`** (`#[serde(default)]`, +so an older transcript still parses), and +`client_core::transcript_fold::TranscriptItem::ToolRun` gained `failed`. +Without them a result was everything a card knew and a broken call drew +exactly as confidently as one that worked -- the missing state, not a +wrong one. Every construction site of both had to gain a field; the value +comes from the CLI's own `tool_result`, read in one place +(`import::tool_result_is_error`) by both the live translator and the +import replay. + +**`TranscriptScreen::set_session_working(rsc, bool)`** is new, and is the +only thing that writes it. Before: a card with no result was drawn the +same whether its turn was still going or had been interrupted. After: +only the *newest* row can say "running", because every row behind it +belongs to a turn that has ended, and changing the flag redraws that one +row rather than the screen. `TranscriptScreen::expand_tail_tools(rsc, +bool)` joins it, answering whether there was a tool run to act on -- a +group's expanded appearance is otherwise unreachable from anything that +cannot press the screen. + +**`transcript_ui::row::build_row` now returns a `TailRow`** rather than an +`Option`: `Blocks` for a message (a delta costs the last +markdown block) or `Tools` for a run (an arriving result costs one card). +One mechanism for "what can this row change cheaply", asked of the row +rather than decided again at each call site. It also takes the row's own +`working` flag. + +Two smaller ones. `client_core::tool_summary::parse_tool_input` is +`ToolInput.kt`'s subject/description/timeout/rest split, and +`client_core::durations::format_millis` is `Durations.kt`'s -- both pure, +both with the Kotlin's own tests ported. + ## 2026-09-06: a tap is its own gesture outcome, and opening a URL is a backend capability Three related additions, all for following a markdown link. diff --git a/docs/IRIS_TODO.md b/docs/IRIS_TODO.md index 6f8df4a..91fb614 100644 --- a/docs/IRIS_TODO.md +++ b/docs/IRIS_TODO.md @@ -695,6 +695,49 @@ Iris's report, verbatim, with a screenshot. Phone: Mali-G715 (Vulkan), it is its own item, most likely the report pane not masking or not claiming its region. +## Found by P1b (2026-09-06), all with a headless repro + +Each was found by looking at `iris/run-headless.sh transcript -- -p +transcript-ui` rather than at a diff, and each is worked around in +`transcript-ui/src/tool.rs` rather than fixed here. docs/RUST.md's P1b box +has the fuller account. + +- [ ] **A `Span` of `Pad`ded children inside another `Span` places those + children a slot out of step.** Each child drew its content one sibling's + height below its own box. Repro: `IRIS_TOOLS_EXPANDED=1 + iris/run-headless.sh transcript --shot /tmp/x.png -- -p transcript-ui` + with `tool.rs`'s group built as `Span(DOWN)[header, Pad(Span(DOWN) + [cards]), bar]` instead of the single `Span` it uses now. Bisected: + removing the inner `Span` fixes it, and so does removing the children's + own `Pad`; the background `Stack`, the `Sized` wrappers and the + `WidgetPtr` per child make no difference. **Not** the `mov`-vs- + `reposition` fault f5b8893 fixed -- it survives that commit. The + workaround costs the group the 4dp inset its Compose counterpart holds + its cards off the edge by, so this is worth fixing. +- [ ] **`scrollable_on(Axis::X)` on a non-editable `Text` draws nothing.** + The panel is drawn and the text inside it is not. A markdown fence does + the same to a `TextEdit` and is fine, so it is the widget kind rather + than the chain. `tool.rs`'s `raw_block` is `masked()` only until this is + fixed, which means a long command is clipped rather than pannable. +- [ ] **No overflow ellipsis.** `TextAttrs` can wrap or not wrap; there is + no "one line, ellipsised" the way `maxLines = 1` + `TextOverflow. + Ellipsis` gives Compose. A tool card's summary is clipped instead, so + nothing on screen says it was cut. Whichever end is cut has to be a + choice when this lands: a path is identified by its tail, a command by + its head. +- [ ] **A drawn chevron.** `Chevron.kt` draws its own strokes precisely + because a chevron from a font is a glyph a system font may not have -- + and the bundled `NotoSans-Regular.ttf` indeed has no U+25B8/25BE/25B4, + while `NotoSansMono-Regular.ttf` does. `tool.rs` sets the mark in the + monospace face as a result. A real fix needs a line/path primitive; + iris has rects, text and textures only. +- [ ] **A tool card's text is not selectable.** `Selection` is keyed + `(RowKey, block index)` and a card has no markdown blocks, so nothing in + a card registers. Compose's `SelectionContainer` covers tool output, + which is the text people most want to copy. Needs a key for "the nth + text of this row" that a card can mint without colliding with a + message's blocks. + ## Build (for the port) Widgets `RUST.md`'s "The port, in order (decided 2026-09-05)" needs and diff --git a/docs/RUST.md b/docs/RUST.md index 8e215eb..1bd626d 100644 --- a/docs/RUST.md +++ b/docs/RUST.md @@ -5687,12 +5687,113 @@ device. correctly, and an emulator bench run with assertions live and no abort (`2438 frames over 147.7s, p50 27.2ms`). - - [ ] **P1b — tool-call cards and grouping.** `ToolRows.kt`/ - `ToolInput.kt`'s cards: a collapsed row per call with name - and a one-line summary, expand to input and output, runs of - calls grouped (`adopt_run` already groups in `client-core`), - the busy/failed states, and the kilobyte outputs the fixture - carries without laying them out while collapsed. + - [x] **P1b — tool-call cards and grouping.** Done 2026-09-06. + `ToolRows.kt`/`ToolInput.kt` ported to + `iris/transcript-ui/src/tool.rs` plus two new pure modules in + `client-core`. **Screenshots: + `docs/bench/p1b-2026-09-06/iris-tools-collapsed.png` and + `iris-tools-expanded.png`**, both from + `iris/run-headless.sh transcript -- -p transcript-ui` on the + desktop/winit backend (the emulator was not touched this pass + -- another agent held this checkout's AVD). The expanded one + is taken with `IRIS_TOOLS_EXPANDED=1`, which the example reads + to call `TranscriptScreen::expand_tail_tools` -- the expanded + appearance is otherwise unreachable on a machine with no + display and no finger. + **What the cards look like, against `ToolRows.kt`:** + - *A collapsed card* -- a mark, the tool's name (14pt), the + one-line summary `parse_tool_input` derives (12pt, Subtext + 0, one line, clipped), and the state word at the far right. + Same as Compose, except that Compose ellipsises the summary + and iris clips it: there is no overflow-ellipsis in + `TextAttrs` yet (IRIS_TODO). + - *An open card* -- the timeout at the top right, the tool's + own description, the subject in a `Verbatim` panel with + `client_core::highlight`'s spans, the leftover input fields + under it, then the output. Same order as Compose. + - *A group* -- "Called N tools" (Compose's exact wording, and + so the name a `ui-trace` script taps), the cards on a Mantle + surface, and a chevron bar at the foot that closes it from + the end the reader is looking at. + - *States* -- `client_core::transcript_fold::ToolState`, five + of them, each with its own word and colour: nothing for + `Succeeded`, "running" (Subtext 0), "your turn" (Peach, the + Compose card's own wording and colour), "failed" (Red) and + **"no result" (Yellow)**. The last two are new -- Compose + can say neither. + **Two things the port had to add to be able to say "it + broke".** `event_model::Event::ToolEnd` gained `is_error` + (`#[serde(default)]`), read from the CLI's own `tool_result` + by one function used by both the live translator and the + import replay (`import::tool_result_is_error`); without it a + result was all a card had and a failed call drew exactly as + confidently as one that worked. And `ToolState` separates + `Succeeded`-with-empty-output from `NoResult`: both leave the + same empty string, and only the session's own status tells + them apart, which is why `TranscriptScreen:: + set_session_working` exists and why only the *newest* row can + be "running" (every row behind it belongs to a turn that has + ended). + **Pass condition, met**: + `collapsed_cards_shape_only_their_summary_lines` + (`transcript-ui/src/lib.rs`) opens a group of three cards + whose calls carry 88 kB of output each and asserts the + text-shape count equals the same group's over three bytes. + **17 either way.** Confirmed to be a real test, not a + tautology, by pushing the output block into the collapsed + branch: **17 against 20**. + `a_result_arriving_redraws_one_card_whatever_the_run_holds` is + the second: one `ToolEnd` costs the same number of + `Widget::draw` calls in a twelve-call run as in a three-call + one. + **Three defects found on the way, all by looking at the + render rather than at the diff:** + 1. **A `Span` of `Pad`ded children inside another `Span` + places those children a slot out of step.** Every card drew + its content one card's height below its own box, so the + group read as empty bars with somebody else's summary in + them. Bisected against + `IRIS_TOOLS_EXPANDED=1 iris/run-headless.sh transcript`: + removing the inner `Span` fixes it, and so does removing + the cards' own `Pad`; the card background, the `Sized` + wrappers and the per-card `WidgetPtr` all make no + difference. Worked around by building the group as **one** + `Span` (header, cards, collapse bar), which costs the 4dp + inset Compose holds its cards off the group's edge by. The + framework defect is still open -- IRIS_TODO has it, and it + is not the `mov`/`reposition` one f5b8893 fixed (it + survives that commit). + 2. **`scrollable_on(Axis::X)` on a non-editable `Text` draws + nothing at all** -- an empty panel where the command should + be. A markdown fence does the same thing to a `TextEdit` + and is fine. So a card's verbatim block is `masked()` and + clips rather than panning; when this is fixed the pan + belongs there too, because the long command is the one + being read closely. + 3. **`NotoSans-Regular.ttf` has no U+25B8/25BE/25B4** (read + out of the bundled `cmap`s) while `NotoSansMono-Regular` + does, so the expander mark is set in the monospace face at + the one place the character is written. The old + `build_tools` summary drew that codepoint in the sans face, + which was a missing glyph nobody had looked closely enough + to see. + **Checks**: `cargo fmt --all --check` clean in both + workspaces; `cargo clippy -p iris -p iris-core -p + transcript-ui -p desktop-app -p tabs-ui --all-targets` and + `cargo clippy --all-targets` in `client-core`/`server`/ + `event-model` warning-free; tests 86 (iris) + 13 (iris-core) + + 36 (transcript-ui, +5) + 137 (client-core, +11) + 160 + (server, +1). + **Not done**: nothing on the emulator or the phone (the AVD + was another agent's this pass, so no frame times were taken); + a card's text is not selectable, unlike Compose's, since + `Selection` is keyed per markdown block and a card has none + (IRIS_TODO); no per-corner radius, so the "connected stack" + shape `connectedShape` draws is a 2dp gap instead; + `AskUserQuestionBody`/`PermissionAsk`'s answer buttons are not + ported -- an unanswered ask forces its card open and says + "your turn", but there is nothing to press yet, which is P1d's + modal/controls work. - [ ] **P1c — history paging and jump-to-latest.** Wire `client-core::transcript_source` into `transcript-ui`: the opening page, paging back on scroll with the cushion diff --git a/docs/bench/p1b-2026-09-06/iris-tools-collapsed.png b/docs/bench/p1b-2026-09-06/iris-tools-collapsed.png new file mode 100644 index 0000000..599fcba Binary files /dev/null and b/docs/bench/p1b-2026-09-06/iris-tools-collapsed.png differ diff --git a/docs/bench/p1b-2026-09-06/iris-tools-expanded.png b/docs/bench/p1b-2026-09-06/iris-tools-expanded.png new file mode 100644 index 0000000..2b2a19b Binary files /dev/null and b/docs/bench/p1b-2026-09-06/iris-tools-expanded.png differ diff --git a/event-model/src/lib.rs b/event-model/src/lib.rs index 4c40c11..f81f28e 100644 --- a/event-model/src/lib.rs +++ b/event-model/src/lib.rs @@ -150,6 +150,19 @@ pub enum Event { ToolEnd { id: String, output: String, + /// Whether the tool reported that the call *failed*, from the + /// CLI's own `is_error` on the `tool_result`. + /// + /// Added 2026-09-06 with the tool-call cards (RUST.md's P1b), + /// because without it a result is the only thing a card has and a + /// failed call is drawn as confidently as a successful one -- the + /// missing state, not a wrong one. `#[serde(default)]` so a + /// transcript written before this field, or a peer on an older + /// build, reads back as "not reported to have failed" rather than + /// failing to parse; that is the same claim the field's absence + /// used to make implicitly. + #[serde(default)] + is_error: bool, }, /// An image the session produced or was sent, saved under the session /// dir and referenced by id; the phone fetches it by URL. diff --git a/iris/transcript-ui/examples/transcript.rs b/iris/transcript-ui/examples/transcript.rs index c5c2e10..5fe8a26 100644 --- a/iris/transcript-ui/examples/transcript.rs +++ b/iris/transcript-ui/examples/transcript.rs @@ -13,7 +13,8 @@ //! with `ui-trace record --do "tap 'Tools'"` on Android, to prove //! hold-the-edge expand). -use client_core::transcript_fold::{TranscriptItem, TranscriptRow as FoldedRow}; +use client_core::QuestionOption; +use client_core::transcript_fold::{QuestionCard, TranscriptItem, TranscriptRow as FoldedRow}; use iris::prelude::*; fn main() { @@ -43,6 +44,75 @@ fn msg(seq: u64, from_user: bool, text: &str) -> FoldedRow { }) } +/// One tool call. `result` is `None` for a call with no result yet and +/// `Some((output, failed))` for one that answered. +fn tool_call(id: &str, tool: &str, input: &str, result: Option<(&str, bool)>) -> TranscriptItem { + tool_call_in("run1", id, tool, input, result) +} + +/// The same, in a named run. Two runs in one transcript must not share a +/// `run_id`: it is the row's identity in the list (`row::row_key`), and +/// two rows under one key is the duplicate-key fault AGENTS.md's +/// "Importing" section describes. Here it made two rows swap cached +/// heights and draw at each other's boxes. +fn tool_call_in( + run: &str, + id: &str, + tool: &str, + input: &str, + result: Option<(&str, bool)>, +) -> TranscriptItem { + TranscriptItem::ToolRun { + seq: 3, + id: id.into(), + run_id: run.into(), + tool: tool.into(), + input: input.into(), + output: result.map(|(out, _)| out.to_string()).unwrap_or_default(), + done: result.is_some(), + failed: result.is_some_and(|(_, failed)| failed), + asks: Vec::new(), + images: Vec::new(), + } +} + +/// A call stopped on the reader: one unanswered permission question. +fn asking(id: &str, tool: &str, input: &str) -> TranscriptItem { + let mut call = tool_call_in("run2", id, tool, input, None); + if let TranscriptItem::ToolRun { asks, .. } = &mut call { + asks.push(QuestionCard { + seq: 9, + id: format!("{id}-q"), + prompt: "Allow this command?".into(), + header: None, + options: vec![ + QuestionOption { + label: "Allow".into(), + description: None, + preview: None, + }, + QuestionOption { + label: "Deny".into(), + description: None, + preview: None, + }, + ], + multi_select: false, + answers: Vec::new(), + }); + } + call +} + +/// Longer than the card's own cap, so the "Show all N lines" control is on +/// screen in the expanded shot. +fn long_output() -> String { + (0..200) + .map(|i| format!("test transcript_ui::case_{i} ... ok")) + .collect::>() + .join("\n") +} + fn synthetic_rows() -> Vec { vec![ msg( @@ -55,41 +125,39 @@ fn synthetic_rows() -> Vec { false, "# Sure\n\nHere's a [link to the repo](https://example.com/ai-app-2) and a fenced block:\n\n```rust\nfn main() {\n println!(\"hi\");\n}\n```", ), + // Every state a tool card has to draw, in one run (P1b): a call + // that worked, one the tool reported as failed, one whose result + // never arrived, and one still running. The last two look the same + // in the events -- an empty output and `done: false` -- and are + // told apart only by whether the session is still working, which + // is what `TranscriptScreen::set_session_working` says. FoldedRow::Tools(vec![ - TranscriptItem::ToolRun { - seq: 3, - id: "t1".into(), - run_id: "run1".into(), - tool: "Read".into(), - input: "{\"file\": \"src/main.rs\"}".into(), - output: "fn main() {}\n".into(), - done: true, - asks: Vec::new(), - images: Vec::new(), - }, - TranscriptItem::ToolRun { - seq: 4, - id: "t2".into(), - run_id: "run1".into(), - tool: "Edit".into(), - input: "{\"file\": \"src/main.rs\"}".into(), - output: "ok".into(), - done: true, - asks: Vec::new(), - images: Vec::new(), - }, - TranscriptItem::ToolRun { - seq: 5, - id: "t3".into(), - run_id: "run1".into(), - tool: "Bash".into(), - input: "cargo build".into(), - output: "Compiling...\nFinished.".into(), - done: true, - asks: Vec::new(), - images: Vec::new(), - }, + tool_call( + "t1", + "Read", + r#"{"file_path": "src/main.rs"}"#, + Some(("fn main() {}\n", false)), + ), + tool_call( + "t2", + "Bash", + r#"{"command": "cargo build --release", "timeout": 480000, "description": "Build it"}"#, + Some(( + "error: could not compile `iris`\nCaused by: linker not found", + true, + )), + ), + tool_call("t3", "Grep", r#"{"pattern": "fn fold_event"}"#, None), ]), + // A lone call is a card too rather than a group of one -- and this + // one carries the kilobyte output a collapsed card must not lay + // out. + FoldedRow::Single(tool_call( + "t5", + "Bash", + r#"{"command": "cargo test -p transcript-ui -- --nocapture"}"#, + Some((&long_output(), false)), + )), msg(6, true, "Looks good, thanks!"), // Every block kind `client_core::markdown_blocks` names, in one // row, so P1a's appearance can be looked at against the Compose @@ -151,6 +219,52 @@ impl DefaultAppState for Client { text: "clear".into(), }), ); + // A second run at the live end, so the *running* state is on + // screen too. It cannot share a row with "no result": the two are + // the same events and are told apart only by whether the session + // is working, which is a property of the row rather than of the + // call (`TranscriptScreen::set_session_working`). + screen.push_row( + rsc, + &FoldedRow::Tools(vec![ + tool_call_in( + "run2", + "t6", + "Read", + r#"{"file_path": "docs/RUST.md"}"#, + Some(("# Moving the app to Rust\n", false)), + ), + tool_call_in( + "run2", + "t7", + "Bash", + r#"{"command": "cargo clippy --workspace --all-targets"}"#, + Some(("error: unused variable `x`", true)), + ), + tool_call_in("run2", "t8", "Glob", r#"{"pattern": "**/*.rs"}"#, None), + // Waiting on a permission, so this card is drawn *open* + // whatever the reader last chose -- the command is the + // thing being decided, and a row saying only "Bash" + // cannot be decided on. It is also how the expanded card + // (input block, output block, timeout) gets into the + // screenshot without a finger. + asking( + "t9", + "Bash", + r#"{"command": "rm -rf target", "timeout": 120000, "description": "Clear the build"}"#, + ), + ]), + ); + screen.set_session_working(rsc, true); + // The expanded picture has no other way to be looked at on a + // machine with no display and no finger -- see `run-headless.sh` + // and docs/RUST.md's P1b box. + if std::env::var_os("IRIS_TOOLS_EXPANDED").is_some() { + assert!( + screen.expand_tail_tools(rsc, true), + "the newest row must be the tool run this flag is about" + ); + } Self { ui_state, screen } } } diff --git a/iris/transcript-ui/src/lib.rs b/iris/transcript-ui/src/lib.rs index 7ca5fa9..ece235e 100644 --- a/iris/transcript-ui/src/lib.rs +++ b/iris/transcript-ui/src/lib.rs @@ -47,6 +47,7 @@ pub mod composer; pub mod markdown; pub mod row; pub mod selection; +pub mod tool; use client_core::transcript_fold::TranscriptRow as FoldedRow; use iris::prelude::*; @@ -66,14 +67,17 @@ pub struct TranscriptScreen { /// interior mutability, per `push_row`'s existing `&self`). Drained by /// [`Self::take_rebuilds`]. rebuilds: std::cell::Cell, - /// The per-block widgets of the row at the live end of the list -- - /// the only row a streamed delta ever lands in -- so - /// [`Self::apply`]'s `ReplaceLast` can replace one markdown block - /// instead of rebuilding the message - /// (`row::RowBlocks::apply_delta`). `None` for a tail that has no - /// delta path (a tool run) or before anything has been pushed. Its - /// removal is every path that replaces or drops the tail row, below. - tail: RefCell>, + /// What the row at the live end of the list kept so the next event + /// can change part of it rather than all of it -- one markdown block + /// of a streaming message (`row::RowBlocks::apply_delta`), or one card + /// of a tool run whose result just arrived (`tool::ToolRow:: + /// apply_calls`). `None` before anything has been pushed. Its removal + /// is every path that replaces or drops the tail row, below. + tail: RefCell>, + /// Whether the session is still working -- see + /// [`Self::set_session_working`], which is the only thing that writes + /// it. `Cell`, like `rebuilds`, so every method here stays `&self`. + session_working: std::cell::Cell, } impl TranscriptScreen { @@ -85,39 +89,117 @@ impl TranscriptScreen { where Rsc::State: FocusHost + OpenUrl, { - let (key, widget, blocks) = row::build_row(rsc, self.list, self.selection.clone(), row); + let (key, widget, tail) = row::build_row( + rsc, + self.list, + self.selection.clone(), + row, + self.session_working.get(), + ); (self.list)(rsc).push_back(ListRow::new(key, widget)); - *self.tail.borrow_mut() = blocks.map(|b| (key, b)); + *self.tail.borrow_mut() = tail.map(|t| (key, t)); } - /// The `ReplaceLast` fast path: update the tail row's blocks in place - /// if this really is a delta into the same message, and say whether - /// that worked. `false` for anything the caller must rebuild instead - /// -- a tail with no block state (a tool run), a row that is not a - /// `Single`, or a change `RowBlocks::apply_delta` will not take. + /// Whether the session this transcript belongs to is still doing + /// something (`client_core::transcript_fold::session_working`). + /// + /// The one thing a tool card cannot read off its own call: a call with + /// no result is *running* while the session works and *never came + /// back* once it stops, and those are different things to tell a + /// reader. Only the newest row is affected -- every row behind it + /// belongs to a turn that has already ended -- so changing it re-draws + /// that row and nothing else. + pub fn set_session_working(&self, rsc: &mut Rsc, working: bool) + where + Rsc::State: FocusHost + OpenUrl, + { + if self.session_working.replace(working) == working { + return; + } + let mut tail = self.tail.borrow_mut(); + if let Some((_, row::TailRow::Tools(tools))) = tail.as_mut() { + let calls = tools.calls(); + tools.apply_calls(rsc, &calls, working); + } + } + + /// How many tool cards the newest row is drawing, `0` when it is not a + /// tool row or its group is closed. Only the tests read it; nothing on + /// screen is decided by it. + #[cfg(test)] + fn tail_card_count(&self) -> usize { + match self.tail.borrow().as_ref() { + Some((_, row::TailRow::Tools(tools))) => tools.card_count(), + _ => 0, + } + } + + /// Open or close the newest row's tool run, when it is one -- what a + /// caller with no finger needs (`run-headless.sh`'s screenshot on this + /// displayless machine, and the tests below). Answers whether there + /// was such a row to act on, so a caller that expected one can say so + /// rather than silently producing the collapsed picture. + pub fn expand_tail_tools(&self, rsc: &mut Rsc, expanded: bool) -> bool + where + Rsc::State: FocusHost + OpenUrl, + { + let tail = self.tail.borrow(); + let Some((_, row::TailRow::Tools(tools))) = tail.as_ref() else { + return false; + }; + tools.set_group_expanded(rsc, expanded); + true + } + + /// The `ReplaceLast` fast path: update the tail row in place if this + /// really is a change to the same row, and say whether that worked. + /// `false` for anything the caller must rebuild instead. + /// + /// Two kinds of row have such a path and they are asked the same + /// question: a message's blocks take a delta into the last block, and + /// a tool row's cards take an arriving result on one card. Which one + /// this is comes from what the row kept, not from a second decision + /// here. fn apply_tail_delta(&self, rsc: &mut Rsc, key: RowKey, row: &FoldedRow) -> bool where Rsc::State: FocusHost + OpenUrl, { - let FoldedRow::Single(item) = row else { - return false; - }; let mut tail = self.tail.borrow_mut(); - let Some((tail_key, blocks)) = tail.as_mut() else { + let Some((tail_key, kept)) = tail.as_mut() else { return false; }; if *tail_key != key { return false; } - let (sender, markdown_src) = row::item_content(item); - blocks.apply_delta( - rsc, - self.list, - self.selection.clone(), - key, - sender, - &markdown_src, - ) + match (kept, row) { + (row::TailRow::Blocks(blocks), FoldedRow::Single(item)) => { + let (sender, markdown_src) = row::item_content(item); + // A tool call is drawn as a card, never as markdown, so a + // row that kept blocks and now holds one is a different + // row -- rebuild it. + if matches!( + item, + client_core::transcript_fold::TranscriptItem::ToolRun { .. } + ) { + return false; + } + blocks.apply_delta( + rsc, + self.list, + self.selection.clone(), + key, + sender, + &markdown_src, + ) + } + (row::TailRow::Tools(tools), FoldedRow::Tools(calls)) => { + tools.apply_calls(rsc, calls, self.session_working.get()) + } + (row::TailRow::Tools(tools), FoldedRow::Single(item)) => { + tools.apply_calls(rsc, std::slice::from_ref(item), self.session_working.get()) + } + (row::TailRow::Blocks(_), FoldedRow::Tools(_)) => false, + } } /// Apply the effect of one more folded event without rebuilding the @@ -196,11 +278,16 @@ impl TranscriptScreen { // pointing at widgets the `drop` below frees (the shape // docs/REVIEW-2026-09-06.md's finding 1 called out). self.selection.borrow_mut().unregister(old_key); - let (new_key, widget, blocks) = - row::build_row(rsc, self.list, self.selection.clone(), &new_rows[common]); + let (new_key, widget, kept) = row::build_row( + rsc, + self.list, + self.selection.clone(), + &new_rows[common], + self.session_working.get(), + ); let evicted = (self.list)(rsc).replace_back(ListRow::new(new_key, widget)); drop(evicted); // frees the old row's widget, same as a pop would - *self.tail.borrow_mut() = blocks.map(|b| (new_key, b)); + *self.tail.borrow_mut() = kept.map(|t| (new_key, t)); for row in &new_rows[common + 1..] { self.push_row(rsc, row); } @@ -279,9 +366,13 @@ where // say so. let mut tail = None; for row in &rows { - let (key, widget, blocks) = row::build_row(rsc, list, selection.clone(), row); + // `false`: a row built here is history until the caller says the + // session is working (`TranscriptScreen::set_session_working`), + // and claiming a call is running because the screen happens to be + // opening is exactly the inferred-as-measured mistake. + let (key, widget, kept) = row::build_row(rsc, list, selection.clone(), row, false); list(rsc).push_back(ListRow::new(key, widget)); - tail = blocks.map(|b| (key, b)); + tail = kept.map(|t| (key, t)); } // Wheel/trackpad scrolling -- the same idiom `trait_fns.rs`'s @@ -339,6 +430,7 @@ where ( TranscriptScreen { tail: RefCell::new(tail), + session_working: std::cell::Cell::new(false), list, composer, selection, @@ -421,6 +513,7 @@ mod diff_tests { input: "x".to_string(), output: String::new(), done: false, + failed: false, asks: Vec::new(), images: Vec::new(), } @@ -577,6 +670,7 @@ mod apply_tests { input: "x".to_string(), output: String::new(), done: false, + failed: false, asks: Vec::new(), images: Vec::new(), } @@ -765,4 +859,244 @@ mod apply_tests { Vec2::new(10.0, 10.0), ); } + + /// A tool call with `output` bytes of output, `done` or not. + fn call(id: &str, output: &str, done: bool) -> TranscriptItem { + TranscriptItem::ToolRun { + seq: 1, + id: id.to_string(), + run_id: "run".to_string(), + tool: "Bash".to_string(), + input: format!(r#"{{"command":"grep -rn {id} ."}}"#), + output: output.to_string(), + done, + failed: false, + asks: Vec::new(), + images: Vec::new(), + } + } + + fn run_of(count: usize, output: &str, done: bool) -> Vec { + (0..count) + .map(|i| call(&format!("t{i}"), output, done)) + .collect() + } + + /// A screen holding one tool run, with the group opened the way a tap + /// opens it, plus the counters drained -- so what a caller measures + /// next is only what it asked for. + fn open_run( + rsc: &mut TestRsc, + items: &[TranscriptItem], + ) -> (TranscriptScreen, StrongWidget, UiRenderState) { + let (screen, tree) = build_tree(rsc, client_core::transcript_fold::group_tool_runs(items)); + let mut render = UiRenderState::new(); + render.resize((1080.0, 20000.0)); + render.update(&tree, rsc); + assert!( + screen.expand_tail_tools(rsc, true), + "the fixture's only row must be the tool run" + ); + render.update(&tree, rsc); + render.take_counters(); + (screen, tree, render) + } + + /// The text shapes it costs to *open* a group of three cards whose + /// calls carry `output` -- the cards themselves, since the collapsed + /// group before the expansion drew none. + fn shapes_to_open(output: &str) -> u64 { + let mut rsc = TestRsc { + ui: UiData::default(), + events: EventManager::default(), + }; + let items = run_of(3, output, true); + let (screen, tree) = build_tree( + &mut rsc, + client_core::transcript_fold::group_tool_runs(&items), + ); + let mut render = UiRenderState::new(); + render.resize((1080.0, 20000.0)); + render.update(&tree, &mut rsc); + render.take_counters(); + + assert!( + screen.expand_tail_tools(&mut rsc, true), + "the fixture's only row must be the tool run" + ); + render.update(&tree, &mut rsc); + let (_, _, _, shapes) = render.take_counters(); + shapes + } + + /// **The O(last block) discipline, for tool cards** (RUST.md's P1b). + /// A collapsed card draws its summary line and nothing else, so the + /// kilobyte outputs the bench fixture carries cost nothing until + /// somebody opens one. Counted in *text shapes*, the number a draw + /// counter cannot stand in for: the widgets are the same either way, + /// and it is parley's work that would grow with the output. + /// + /// The group is *opened* here, so all three cards are really drawn -- + /// the cheap version of this test (a closed group, which draws no + /// cards at all) would pass without saying anything about a card. + #[test] + fn collapsed_cards_shape_only_their_summary_lines() { + let long: String = std::iter::repeat_n("a line of tool output\n", 4_000).collect(); + assert!(long.len() > 80_000, "the long case must actually be long"); + + let short_shapes = shapes_to_open("ok\n"); + let long_shapes = shapes_to_open(&long); + assert!( + short_shapes > 0, + "opening a group must shape something, or this compares two zeroes" + ); + assert_eq!( + short_shapes, long_shapes, + "three collapsed cards shaped {long_shapes} text layouts over 80 kB of output \ + against {short_shapes} over three bytes -- a collapsed card is laying out \ + something it does not draw" + ); + } + + /// What one arriving result costs, in `Widget::draw` calls, in a run of + /// `count` calls -- with the group open, so every card is really on + /// screen and a rebuild of the wrong scope would show. + fn cost_of_one_result(count: usize) -> u64 { + let mut rsc = TestRsc { + ui: UiData::default(), + events: EventManager::default(), + }; + let before = run_of(count, "", false); + let mut after = before.clone(); + after[0] = call("t0", "the result", true); + + let (screen, tree, mut render) = open_run(&mut rsc, &before); + screen.apply(&mut rsc, &before, &after); + render.update(&tree, &mut rsc); + assert_eq!( + screen.take_rebuilds(), + 0, + "a result arriving must not rebuild the whole screen" + ); + let (draws, _, _, _) = render.take_counters(); + draws + } + + /// **A result changes one card**, whatever else is in the run -- + /// `RowBlocks::apply_delta`'s discipline applied to a group, which is + /// a column of cards (`tool::ToolRow::apply_calls`). Stated as a + /// comparison rather than a number, because the number is whatever a + /// card happens to be made of and would have to be edited every time + /// the card gains a widget; what must not change is that it does not + /// grow with the run. + #[test] + fn a_result_arriving_redraws_one_card_whatever_the_run_holds() { + let small = cost_of_one_result(3); + let large = cost_of_one_result(12); + assert!( + small > 0, + "a result must redraw *something*, or this compares two zeroes" + ); + assert_eq!( + small, large, + "one result redrew {large} widgets in a twelve-call run against {small} in a \ + three-call one -- the other cards are being rebuilt with it" + ); + } + + /// The group's own state: opening it draws the cards, closing it takes + /// them away again, and the reader's choice survives a result arriving + /// in the middle of it. + #[test] + fn a_group_opens_and_closes_and_keeps_its_state_across_a_result() { + let mut rsc = TestRsc { + ui: UiData::default(), + events: EventManager::default(), + }; + let before = run_of(3, "", false); + let mut after = before.clone(); + after[1] = call("t1", "done", true); + + let (screen, tree) = build_tree( + &mut rsc, + client_core::transcript_fold::group_tool_runs(&before), + ); + let mut render = UiRenderState::new(); + render.resize((1080.0, 20000.0)); + render.update(&tree, &mut rsc); + + // Closed, a group is one line: no card is registered at all, which + // is what makes the kilobyte outputs free. + assert_eq!(screen.tail_card_count(), 0); + assert!(screen.expand_tail_tools(&mut rsc, true)); + assert_eq!(screen.tail_card_count(), 3); + + // A result arriving must not close what the reader opened -- the + // card is rebuilt, and being open is the reader's state rather + // than the event's. + screen.apply(&mut rsc, &before, &after); + render.update(&tree, &mut rsc); + assert_eq!(screen.take_rebuilds(), 0); + assert_eq!( + screen.tail_card_count(), + 3, + "the group closed under a result" + ); + + assert!(screen.expand_tail_tools(&mut rsc, false)); + assert_eq!(screen.tail_card_count(), 0); + } + + /// A call that joins a run while it is the live row appends one card + /// rather than rebuilding the row -- the other half of `apply_calls`, + /// and the case a page join does *not* produce (that one goes through + /// `Rebuild`). + #[test] + fn a_call_joining_an_open_run_appends_one_card() { + let mut rsc = TestRsc { + ui: UiData::default(), + events: EventManager::default(), + }; + let before = run_of(2, "ok", true); + let mut after = before.clone(); + after.push(call("t2", "", false)); + + let (screen, tree, mut render) = open_run(&mut rsc, &before); + assert_eq!(screen.tail_card_count(), 2); + screen.apply(&mut rsc, &before, &after); + render.update(&tree, &mut rsc); + assert_eq!( + screen.take_rebuilds(), + 0, + "an appended call is not a rebuild" + ); + assert_eq!(screen.tail_card_count(), 3); + } + + /// A tool row that becomes something else is a different row, not a + /// changed one. Without the guard in `apply_calls` a `UserMsg` would + /// reach the card builder, whose `debug_assert` is the last line of + /// defence rather than the first. + #[test] + fn a_tail_that_stops_being_tool_calls_falls_back_to_a_rebuild() { + let mut rsc = TestRsc { + ui: UiData::default(), + events: EventManager::default(), + }; + let before = vec![user(1, "stable"), call("t0", "", false)]; + let after = vec![user(1, "stable"), user(2, "not a tool call at all")]; + let (screen, _tree) = build_tree( + &mut rsc, + client_core::transcript_fold::group_tool_runs(&before), + ); + screen.apply(&mut rsc, &before, &after); + assert_eq!( + screen.take_rebuilds(), + 0, + "this is a ReplaceLast, not a whole-screen rebuild" + ); + // The row that replaced it is a message, so it keeps blocks rather + // than cards -- and nothing panicked on the way. + assert_eq!(screen.tail_card_count(), 0); + } } diff --git a/iris/transcript-ui/src/markdown.rs b/iris/transcript-ui/src/markdown.rs index 3cc9c94..883c741 100644 --- a/iris/transcript-ui/src/markdown.rs +++ b/iris/transcript-ui/src/markdown.rs @@ -399,7 +399,12 @@ fn options() -> Options { /// (`highlight`'s module doc), so the offsets are walked once rather than /// converted per span -- a fence is scanned on every delta that lands in /// it, and it is the only block a delta re-renders. -fn highlight_into(spans: &mut Vec, text: &str, range: Range, language: Language) { +pub(crate) fn highlight_into( + spans: &mut Vec, + text: &str, + range: Range, + language: Language, +) { let code = &text[range.clone()]; // char index -> byte offset within `code`, plus the end, so a span's // `end` is always in range. diff --git a/iris/transcript-ui/src/row.rs b/iris/transcript-ui/src/row.rs index 9256a1b..b546222 100644 --- a/iris/transcript-ui/src/row.rs +++ b/iris/transcript-ui/src/row.rs @@ -25,6 +25,7 @@ use crate::markdown::{BlockFrame, Link, frame_of, render_block}; use crate::selection::{SelKey, Selection}; +use crate::tool::ToolRow; use client_core::markdown_blocks::{Block, BlockKind, common_prefix, split_blocks}; use client_core::transcript_fold::{QuestionCard, TranscriptItem, TranscriptRow as FoldedRow}; use iris::prelude::*; @@ -461,105 +462,20 @@ where build_text_row(rsc, list, selection, key, sender, &markdown_src) } -/// A run of adjacent tool calls: collapsed to a one-line summary by -/// default, expanding in place to every call's own tool/input/output on -/// tap -- see the module doc for the hold-the-edge contract this wires -/// against `list`. -fn build_tools( - rsc: &mut Rsc, - list: WeakWidget, - selection: Rc>, - key: RowKey, - calls: Vec, -) -> StrongWidget -where - Rsc::State: FocusHost + OpenUrl, -{ - let expanded = Rc::new(RefCell::new(false)); - // `.add_strong` (not `.add`) because nothing else in the tree holds a - // strong reference to this `WidgetPtr` the way a container's own - // `add_strong`-on-its-children does for an ordinary child -- this row - // *is* the top of its own subtree, so it has to own itself. - let ptr_strong = WidgetPtr::new().add_strong(rsc); - let ptr = ptr_strong.weak(); - - let summary_text = format!("\u{25b8} {} tool calls", calls.len()); - let full_text = calls - .iter() - .map(|c| match c { - TranscriptItem::ToolRun { - tool, - input, - output, - .. - } => tool_call_markdown(tool, input, output), - other => item_content(other).1, - }) - .collect::>() - .join("\n\n"); - - fn build_content( - rsc: &mut Rsc, - list: WeakWidget, - selection: Rc>, - key: RowKey, - expanded: bool, - summary: &str, - full: &str, - ) -> StrongWidget - where - Rsc::State: FocusHost + OpenUrl, - { - // Every block of the previous content goes first: collapsing a - // five-block expansion back to a one-line summary registers only - // `(key, 0)`, and blocks 1..5 would be left in `Selection` - // pointing at widgets `ptr.replace` is about to free -- the same - // class of bug docs/REVIEW-2026-09-06.md's finding 1 found in the - // `Rebuild` arm, reached the other way. - selection.borrow_mut().unregister(key); - let text = if expanded { full } else { summary }; - build_text_row(rsc, list, selection, key, Some("Tools"), text).0 - } - - let content = build_content( - rsc, - list, - selection.clone(), - key, - false, - &summary_text, - &full_text, - ); - ptr(rsc).set(content); - - ptr.on(CursorSense::click(), move |ctx, rsc| { - // `List::note_tap` wants a viewport-relative position, but the - // click event only knows where inside *this row* it landed - // (`ctx.data.pos`) -- `List::extent` (last frame's on-screen box - // for this row's key) is what turns the two into the position - // `list.rs`'s hold-the-edge layout pass resolves against, per the - // module doc's contract. - let (top, _bottom) = list(rsc).extent(key).unwrap_or((0.0, 0.0)); - list(rsc).note_tap(top + ctx.data.pos.y); - - let was_expanded = *expanded.borrow(); - *expanded.borrow_mut() = !was_expanded; - let content = build_content( - rsc, - list, - selection.clone(), - key, - !was_expanded, - &summary_text, - &full_text, - ); - // The old content's `StrongWidget` is freed when this drops -- - // the removal half of the row this click just replaced. - let _old = ptr(rsc).replace(content); - }) - .add(rsc); - - ptr_strong.any() +/// What a row keeps so the next event can change part of it instead of +/// all of it -- one variant per kind of row that has such a path. +/// +/// Two mechanisms would have been two answers to the same question ("what +/// can this row do cheaply?"), so the caller holds one of these for its +/// tail row and asks it, rather than holding a `RowBlocks` and a +/// `ToolRow` and choosing between them at each call site. +pub enum TailRow { + /// A message: a column of one text widget per markdown block, so a + /// streamed delta costs the last block. + Blocks(RowBlocks), + /// A tool call or a run of them: a column of cards, so an arriving + /// result costs one card. + Tools(ToolRow), } pub fn build_row( @@ -567,26 +483,32 @@ pub fn build_row( list: WeakWidget, selection: Rc>, row: &FoldedRow, -) -> (RowKey, StrongWidget, Option) + working: bool, +) -> (RowKey, StrongWidget, Option) where Rsc::State: FocusHost + OpenUrl, { - match row { - FoldedRow::Single(item) => { - let key = row_key(&item.key()); - let (widget, blocks) = build_single(rsc, list, selection, key, item); - (key, widget, Some(blocks)) - } - FoldedRow::Tools(calls) => { - let key = row_key(&calls[0].key()); - // `None`: a run of tool calls is never what a reply streams - // into, and its own expand/collapse replaces the whole - // content anyway, so there is no delta path to keep state for. - ( - key, - build_tools(rsc, list, selection, key, calls.clone()), - None, - ) + // A lone tool call is a card too, not a message with markdown in it: + // `group_tool_runs` leaves one call as a `Single` because "Called 1 + // tool" hides a card to say the same thing in more words, and the + // *card* is what both cases draw (`ToolRows.kt`). + let calls = match row { + FoldedRow::Single(item @ TranscriptItem::ToolRun { .. }) => { + Some(std::slice::from_ref(item)) } + FoldedRow::Tools(calls) => Some(calls.as_slice()), + FoldedRow::Single(_) => None, + }; + if let Some(calls) = calls { + let key = row_key(&calls[0].key()); + let (widget, tools) = + crate::tool::build_tool_row(rsc, list, selection, key, calls.to_vec(), working); + return (key, widget, Some(TailRow::Tools(tools))); } + let FoldedRow::Single(item) = row else { + unreachable!("every Tools row took the branch above"); + }; + let key = row_key(&item.key()); + let (widget, blocks) = build_single(rsc, list, selection, key, item); + (key, widget, Some(TailRow::Blocks(blocks))) } diff --git a/iris/transcript-ui/src/tool.rs b/iris/transcript-ui/src/tool.rs new file mode 100644 index 0000000..4bb4630 --- /dev/null +++ b/iris/transcript-ui/src/tool.rs @@ -0,0 +1,897 @@ +//! Tool-call cards and the runs they are grouped into -- the port of +//! `ToolRows.kt`/`ToolInput.kt` (RUST.md's P1b). +//! +//! One card per call. Closed, it is a single line: the tool's name and +//! what the call is for ([`client_core::tool_summary::parse_tool_input`]'s +//! `title`). The command itself is not on it, because a wrapped command +//! turns one row into four and a run of them into a wall. Open, it shows +//! the description, the input and the output. +//! +//! **A collapsed card lays out its summary line and nothing else.** Not an +//! optimisation -- the discipline this crate is built to. The bench +//! fixture carries tool outputs of tens of kilobytes, and a collapsed card +//! that built a text widget for one would pay parley for text nobody can +//! see. `collapsed_cards_shape_only_their_summary_lines` in `lib.rs` holds +//! it, counting `UiRenderState`'s text-shape counter the same way +//! `a_delta_into_a_long_reply_...` counts it for a streamed delta. +//! +//! **Two or more adjacent calls are one group** -- decided in +//! `client_core::transcript_fold::group_tool_runs`/`adopt_run` and never +//! re-derived here. A group is a header, a column of cards on its own +//! surface, and a bar at its foot: it closes from either end, because a +//! long group's header scrolls off while its last call is still on screen, +//! and the reader who wants it shut is looking at the bottom. +//! +//! **A result arriving replaces one card.** [`ToolRow::apply_calls`] is +//! the group's half of `RowBlocks::apply_delta`'s discipline: a group is a +//! column of cards, and a `ToolEnd` changes exactly one of them. +//! +//! **Every tap here is a tap** -- `GestureOutcome::Tapped` out of the one +//! `DragArbiter` `Selection` already owns, never a second detector. A +//! finger that panned the list past a card must not also open it; that +//! rule is written once, in the gesture machine, and this file only reads +//! its answer. + +use crate::markdown::{TEXT_COLOR, VERBATIM_BACKGROUND, highlight_into}; +use crate::selection::Selection; +use client_core::tool_summary::{ToolInput, parse_tool_input}; +use client_core::transcript_fold::{ToolState, TranscriptItem}; +use iris::prelude::*; +use std::{cell::Cell, cell::RefCell, collections::HashMap, rc::Rc, time::Instant}; + +/// A card's own fill: Surface 0, what Material's filled `Card` resolves to +/// under `Theme.kt`'s scheme. One step *above* the page, so a card reads +/// as an object on it. +const CARD_FILL: UiColor = UiColor::new(0x31, 0x32, 0x44, 255); +/// The surface a group's cards sit on: Mantle, one step *below* the page. +/// That surface is the single cue saying these calls belong together, and +/// it goes below rather than above because the cards are already above -- +/// two steps in the same direction render as one flat block. +const GROUP_FILL: UiColor = UiColor::new(0x18, 0x18, 0x25, 255); +/// A tool's name, and any of the call's own words. +const NAME_COLOR: UiColor = TEXT_COLOR; +/// The summary line and the leftover input fields: Subtext 0, the Compose +/// app's `onSurfaceVariant` -- structure about the call rather than the +/// call's own words. +const MUTED_COLOR: UiColor = UiColor::new(0xA6, 0xAD, 0xC8, 255); + +/// Waiting on a person -- Peach, `Theme.kt`'s `awaitingColor`. The same +/// colour a question card takes, because it is the same fact. +const AWAITING_COLOR: UiColor = UiColor::new(0xFA, 0xB3, 0x87, 255); +/// The call itself failed -- Red, the scheme's `error`/`failedColor`. +const FAILED_COLOR: UiColor = UiColor::new(0xF3, 0x8B, 0xA8, 255); +/// **Nobody found out** -- Yellow, `Theme.kt`'s `warningColor`. Its own +/// colour *and* its own word: the expensive confusion is between this and +/// a call that finished having printed nothing, those two share an empty +/// output, and a difference in kind cannot be carried by colour alone. +const UNKNOWN_COLOR: UiColor = UiColor::new(0xF9, 0xE2, 0xAF, 255); + +/// A tool's name (Material `titleSmall`). +const NAME_SIZE: f32 = 14.0; +/// The summary line, and the input and output text (`bodySmall`). +const BODY_SIZE: f32 = 12.0; +/// The state word, the "Output" heading and the group's own count +/// (`labelSmall`). +const LABEL_SIZE: f32 = 11.0; + +/// The room inside a card, and so the height a bar of one line of text +/// comes to (`ToolRows.kt`'s `GROUP_INSET_LARGE`). +const CARD_PAD_DP: f32 = 12.0; +/// A card's corner: `shapes.medium`, the same as every other card in the +/// app. +const CARD_RADIUS_DP: f32 = 12.0; +/// The gap between the parts of a card's header line, and between the +/// stacked parts of an open card. +const GAP_DP: f32 = 8.0; +/// Smaller than a card's radius, and deliberately: a verbatim block sits +/// *inside* one, and a rounded rectangle drawn at the same radius as the +/// one behind it reads as a misprint (`RawBlock.kt`). +const RAW_RADIUS_DP: f32 = 4.0; +/// The room inside a verbatim block. +const RAW_PAD_DP: f32 = 8.0; + +/// How much of a tool's output an open card draws before it offers the +/// rest behind a tap. +/// +/// **A divergence from Compose, on purpose.** `ToolCard` draws the whole +/// output however long, and gets away with it because a Compose `Text` +/// inside a `LazyColumn` is laid out lazily; here the output is one text +/// widget, and shaping a hundred kilobytes of it through parley is the +/// cost `docs/EXPLORER.md`'s `EDIT_LIMIT` was measured against. Lines +/// *and* bytes because the two run out at different times -- a diff is +/// many short lines, a minified file is one enormous one. +const OUTPUT_LINES: usize = 80; +const OUTPUT_BYTES: usize = 4096; +/// A cap of nothing would draw an empty panel and a "Show all" for every +/// output there is, which reads as a rendering fault rather than as a cap. +/// Checked at compile time, since both are constants. +const _: () = assert!(OUTPUT_LINES > 0 && OUTPUT_BYTES > 0); + +/// The mark that says a card opens, always drawn from the **monospace** +/// face. +/// +/// Not a style choice: `NotoSans-Regular.ttf`, which every other string +/// here is set in, has no glyph at U+25B8/U+25BE/U+25B4 at all, while +/// `NotoSansMono-Regular.ttf` does -- read out of both bundled `cmap`s on +/// 2026-09-06. A missing glyph is the failure nobody who wrote the code +/// ever sees, so the face that has the glyph is named at the one place the +/// character is written. IRIS_TODO's "a drawn chevron" has the real fix, +/// which needs a line primitive iris does not have. +const CLOSED_MARK: &str = "\u{25b8}"; +const OPEN_MARK: &str = "\u{25be}"; +const UP_MARK: &str = "\u{25b4}"; + +/// Which cards the reader has opened, and which have had their whole +/// output asked for. +/// +/// Outside the widget tree on purpose: a card is rebuilt when its result +/// arrives, and being open is the reader's state rather than the event's +/// -- held in the widget, it would silently close the moment the tool +/// answered. Keyed by the call's own id, which survives a regroup. Its +/// path out is [`ToolRow::apply_calls`], which drops the entry for any +/// call no longer in the row. +#[derive(Default)] +struct ToolRowState { + group_expanded: bool, + open: HashMap, + whole_output: HashMap, +} + +/// Everything a handler needs to redraw part of this row, in one `Rc` so +/// that a handler registered once keeps working against calls that arrive +/// later. The rebuild functions read `calls` fresh rather than capturing a +/// call, which is what lets [`ToolRow::apply_calls`] replace a card's +/// content without re-registering its gesture. +struct Shared { + calls: RefCell>, + state: RefCell, + /// One `WidgetPtr` per call, in order -- what makes a result cost one + /// card. Empty while the group is collapsed, because a collapsed group + /// draws no cards at all. Its path out is [`build_content`], which + /// clears it before building whatever replaces them. + cards: RefCell>>, + /// The whole row's content, swapped when the group opens or closes. + /// Filled in immediately after construction -- the `WidgetPtr` cannot + /// exist before the `Rc` every handler inside it captures. + content: RefCell>>, + list: WeakWidget, + selection: Rc>, + key: RowKey, + /// Whether a call in this row could still be running -- the caller's + /// `session_working`, and `false` for every row behind the newest, + /// whose turn has already ended. The one input to [`ToolState`] that + /// is not a property of the call itself, and what separates "still + /// going" from "nobody found out". + working: Cell, +} + +/// One transcript row's worth of tool calls, kept by the caller for the +/// row a result can still land in -- the tool-call counterpart of +/// [`crate::row::RowBlocks`], and the reason a `ToolEnd` costs one card +/// rather than a row. +pub struct ToolRow { + shared: Rc, +} + +/// Register `f` as this widget's **tap**, panning the list instead when +/// the finger moves. +/// +/// The one gesture entry point in this file. `Selection::drag` with no row +/// is the same call `row.rs` makes with one: it drives the shared +/// `DragArbiter`, so a drag starting on a card scrolls (and flings) the +/// transcript exactly as one starting on a paragraph does, and only a +/// press that committed to nothing comes back as `Tapped`. A bare +/// `CursorSense::click()` here would be a second, disagreeing detector -- +/// it fires at the end of a pan too, so every scroll that began on a card +/// would also toggle it. +fn on_tap( + rsc: &mut Rsc, + ptr: WeakWidget, + shared: &Rc, + f: impl Fn(&mut Rsc) + 'static, +) where + Rsc::State: FocusHost + OpenUrl, +{ + let (list, selection) = (shared.list, shared.selection.clone()); + ptr.on( + CursorSense::click_or_drag() | CursorSense::unclick(), + move |ctx, rsc| { + let outcome = selection.borrow_mut().drag( + rsc, + list, + None, + ctx.data.cursor.pos, + ctx.data.sense, + Instant::now(), + ctx.data.render, + ); + if outcome == GestureOutcome::Tapped { + f(rsc); + } + }, + ) + .add(rsc); +} + +/// Hold the edge the reader is looking at while this row changes height. +/// +/// `List::note_tap` wants a viewport-relative position and this row only +/// knows its own box, so `List::extent` (last frame's on-screen box for +/// this key) turns the two into the position `list.rs`'s hold-the-edge +/// pass resolves against -- the two-step contract that module's doc +/// describes for `AGENTS.md`'s `holdTopEdge`. +fn note_tap(rsc: &mut impl UiRsc, shared: &Shared) { + // Only when the list actually has an extent for this row. `None` + // means the row has not been drawn yet -- which happens the moment + // something opens a group before the first frame + // (`TranscriptScreen::expand_tail_tools`, the headless screenshot) -- + // and standing in `0.0` for it tells the layout pass to hold an edge + // at the top of the viewport that nothing was ever at. The whole list + // then places itself against that invented anchor: rows drawn at each + // other's cached heights, tool cards as empty bars with their text a + // group's height below them (`docs/bench/p1b-2026-09-06/`'s first + // attempt). Nothing to hold is not the same as an edge at zero. + if let Some((top, _bottom)) = (shared.list)(rsc).extent(shared.key) { + (shared.list)(rsc).note_tap(top); + } +} + +fn text(content: impl Into, size: f32, color: UiColor) -> TextBuilder { + wtext(content) + .size(size) + .color(color) + .text_align(Align::LEFT) +} + +/// A verbatim block: monospace on the surface everything verbatim in this +/// app sits on, not wrapped, panning sideways on a finger. +/// +/// Not wrapped for `ToolInput.kt`'s reason -- a wrapped command hides +/// where its arguments end, and the long one is the one being read +/// closely. A long line is **clipped** here rather than pannable, which a +/// markdown fence (`row.rs`'s `BlockFrame::Verbatim`) is not: adding +/// `.scrollable_on(Axis::X)` to this non-editable `Text` made it draw +/// nothing at all -- an empty panel where the command should be, seen on +/// 2026-09-06 in `docs/bench/p1b-2026-09-06/` and bisected to that one +/// call (the fence, which does the same thing to a `TextEdit`, is fine). +/// Recorded in docs/IRIS_TODO.md; when it is fixed, the pan belongs here +/// too, because the long command is the one being read closely. +fn raw_block(rsc: &mut Rsc, body: TextBuilder) -> StrongWidget +where + Rsc::State: FocusHost, +{ + let field = body + .family(Family::Monospace) + .size(BODY_SIZE) + .wrap(false) + .add(rsc); + field + .masked() + .pad(dp(RAW_PAD_DP)) + .background(rect(VERBATIM_BACKGROUND).radius(dp(RAW_RADIUS_DP))) + .width(rest(1)) + .add_strong(rsc) + .any() +} + +/// The word a card shows for what became of the call, and the colour it is +/// in. `None` for a call that simply worked -- the ordinary outcome says +/// nothing, the way it says nothing in Compose. +/// +/// Colour by consequence: the same red wherever something failed, the same +/// peach wherever the turn is stopped on a person, yellow where the answer +/// is that nobody knows. +fn state_mark(state: ToolState) -> Option<(&'static str, UiColor)> { + match state { + // A spinner would say the machine is working; while this call + // waits on an answer the machine is doing nothing at all, so the + // card says whose move it is instead (`ToolRows.kt`). + ToolState::Deciding => Some(("your turn", AWAITING_COLOR)), + ToolState::Running => Some(("running", MUTED_COLOR)), + ToolState::Failed => Some(("failed", FAILED_COLOR)), + ToolState::NoResult => Some(("no result", UNKNOWN_COLOR)), + ToolState::Succeeded => None, + } +} + +/// What a screen reader is given for one card, and what a `ui-trace` +/// script taps by: the tool, what the call is for, and how it went when +/// that is anything but "fine" -- the same three things the Compose card's +/// own text says, in the order it says them. +fn card_label(tool: &str, parsed: &ToolInput, state: ToolState) -> String { + let mut name = tool.to_string(); + if let Some(title) = parsed.title() { + name.push_str(": "); + name.push_str(title); + } + if let Some((word, _)) = state_mark(state) { + name.push_str(" ("); + name.push_str(word); + name.push(')'); + } + name +} + +/// The heading a group carries, closed or open. Compose's exact wording, +/// because it is also the name every `ui-trace` script taps it by. +fn group_label(count: usize) -> String { + format!("Called {count} tools") +} + +/// `output` cut to what an open card draws, with the line count it was cut +/// from; `None` when the whole of it fits. +/// +/// Cut at the **head**, keeping the beginning: a tool's output is read +/// from the top, and the line saying what went wrong is nearly always the +/// first. (A path is identified by its other end; this is not a path.) +fn capped(output: &str) -> Option<(&str, usize)> { + let by_lines = output + .char_indices() + .filter(|(_, c)| *c == '\n') + .nth(OUTPUT_LINES - 1) + .map(|(i, _)| i); + let by_bytes = (output.len() > OUTPUT_BYTES).then(|| { + let mut end = OUTPUT_BYTES; + while !output.is_char_boundary(end) { + end -= 1; + } + end + }); + let cut = match (by_lines, by_bytes) { + (Some(a), Some(b)) => a.min(b), + (a, b) => a.or(b)?, + }; + Some((&output[..cut], output.lines().count())) +} + +/// The tool's output, or the reason there is none to show. +/// +/// The empty cases are drawn rather than left blank: "it printed nothing" +/// and "nothing ever came back" are the pair [`ToolState`] exists to keep +/// apart, and a card that drew neither would show the same thing for both. +fn output_block( + rsc: &mut Rsc, + shared: &Rc, + index: usize, + id: &str, + output: &str, + call_state: ToolState, +) -> StrongWidget +where + Rsc::State: FocusHost + OpenUrl, +{ + if output.is_empty() { + let (words, colour) = match call_state { + ToolState::Succeeded => ("No output", MUTED_COLOR), + ToolState::Failed => ("Failed, with no output", FAILED_COLOR), + ToolState::NoResult => ("No result ever arrived", UNKNOWN_COLOR), + ToolState::Running | ToolState::Deciding => ("No output yet", MUTED_COLOR), + }; + return text(words, LABEL_SIZE, colour).add_strong(rsc).any(); + } + + let whole = shared + .state + .borrow() + .whole_output + .get(id) + .copied() + .unwrap_or(false); + let shown = if whole { None } else { capped(output) }; + let mut column = Span::empty(Dir::DOWN).gap(dp(2)); + column.push(text("Output", LABEL_SIZE, NAME_COLOR).add_strong(rsc).any()); + // What the tool printed, in the face it was written for: this is + // column-aligned far more often than it is prose, and a proportional + // font destroys the alignment that carried the meaning. + let body = text( + shown.map_or(output, |(head, _)| head).to_string(), + BODY_SIZE, + NAME_COLOR, + ); + column.push(raw_block(rsc, body)); + if let Some((_, lines)) = shown { + let label = format!("Show all {lines} lines"); + let more_strong = WidgetPtr::new().add_strong(rsc); + let more = more_strong.weak(); + let words = text(label.clone(), LABEL_SIZE, MUTED_COLOR) + .label(label) + .add_strong(rsc); + more(rsc).set(words); + let shared_for_tap = shared.clone(); + let id = id.to_string(); + on_tap(rsc, more, shared, move |rsc| { + note_tap(rsc, &shared_for_tap); + shared_for_tap + .state + .borrow_mut() + .whole_output + .insert(id.clone(), true); + redraw_card(rsc, &shared_for_tap, index); + }); + column.push(more_strong.any()); + } + column.width(rest(1)).add_strong(rsc).any() +} + +/// One tool call's card content. +/// +/// Collapsed, this is one `Span` of at most four short strings -- no +/// input, no output, nothing whose size is the call's size. +fn build_card(rsc: &mut Rsc, shared: &Rc, index: usize) -> StrongWidget +where + Rsc::State: FocusHost + OpenUrl, +{ + let call = shared.calls.borrow()[index].clone(); + let TranscriptItem::ToolRun { + id, + tool, + input, + output, + .. + } = &call + else { + debug_assert!(false, "a tool row holds only tool calls, not {call:?}"); + return Span::empty(Dir::DOWN).add_strong(rsc).any(); + }; + let parsed = parse_tool_input(tool, input); + let call_state = ToolState::of(&call, shared.working.get()).expect("matched ToolRun above"); + // A call waiting on permission is shown open whatever the reader last + // chose: the command is the thing being decided, and a row saying only + // "Bash" cannot be decided on (`ToolRows.kt`). + let open = shared.state.borrow().open.get(id).copied().unwrap_or(false) + || call_state == ToolState::Deciding; + + let mut header = Span::empty(Dir::RIGHT).gap(dp(GAP_DP)); + header.push( + text( + if open { OPEN_MARK } else { CLOSED_MARK }, + BODY_SIZE, + MUTED_COLOR, + ) + .family(Family::Monospace) + .add_strong(rsc) + .any(), + ); + header.push( + text(tool.clone(), NAME_SIZE, NAME_COLOR) + .add_strong(rsc) + .any(), + ); + match (open, parsed.title()) { + // Open, the summary is redundant -- the input below is the same + // thing in full -- and the space goes to the timeout instead, at + // the far end, since it is a limit on the call rather than part of + // what the call does. + (true, _) | (false, None) => { + header.push(Span::empty(Dir::RIGHT).width(rest(1)).add_strong(rsc).any()) + } + // One line, clipped rather than shrunk or wrapped: a wrapped + // command turns one row into four and a run of them into a wall. + (false, Some(title)) => header.push( + text(title.to_string(), BODY_SIZE, MUTED_COLOR) + .wrap(false) + .masked() + .width(rest(1)) + .add_strong(rsc) + .any(), + ), + } + if open && let Some(timeout) = &parsed.timeout { + header.push( + text(format!("timeout {timeout}"), LABEL_SIZE, MUTED_COLOR) + .add_strong(rsc) + .any(), + ); + } + if let Some((word, colour)) = state_mark(call_state) { + header.push(text(word, LABEL_SIZE, colour).add_strong(rsc).any()); + } + + let mut column = Span::empty(Dir::DOWN).gap(dp(GAP_DP / 2.0)); + column.push(header.width(rest(1)).add_strong(rsc).any()); + if open { + if let Some(description) = &parsed.description { + // The tool's own prose about what it is doing, so it belongs + // with the reader's text rather than inside the machine's -- + // above the input block rather than in it (`ToolInput.kt`). + column.push( + text(description.clone(), BODY_SIZE, MUTED_COLOR) + .width(rest(1)) + .add_strong(rsc) + .any(), + ); + } + if let Some(subject) = &parsed.subject { + let spans = match parsed.language { + Some(language) => { + let mut spans = Vec::new(); + highlight_into(&mut spans, subject, 0..subject.len(), language); + spans + } + // An unknown language is drawn plain rather than coloured + // by the nearest one -- P1a's rule for a fence, and the + // same reason: a wrong highlight is read as a fact. + None => Vec::new(), + }; + let body = text(subject.clone(), BODY_SIZE, NAME_COLOR).spans(spans); + column.push(raw_block(rsc, body)); + } + if !parsed.rest.is_empty() { + // Never dropped: a field left out would be claiming the tool + // had no other input when it might (`ToolInput.kt`). + let body = text(parsed.rest.join("\n"), BODY_SIZE, MUTED_COLOR); + column.push(raw_block(rsc, body)); + } + column.push(output_block(rsc, shared, index, id, output, call_state)); + } + + column + .width(rest(1)) + .pad(dp(CARD_PAD_DP)) + .background(rect(CARD_FILL).radius(dp(CARD_RADIUS_DP))) + .width(rest(1)) + .label(card_label(tool, &parsed, call_state)) + .add_strong(rsc) + .any() +} + +/// Rebuild card `index` in place. The removal half is the returned +/// `StrongWidget` being dropped, which frees the content this replaced. +fn redraw_card(rsc: &mut Rsc, shared: &Rc, index: usize) +where + Rsc::State: FocusHost + OpenUrl, +{ + let Some(ptr) = shared.card_ptr(index) else { + // Reached only if a handler outlives the card it was registered + // on, which `apply_calls` is written to prevent. + debug_assert!(false, "card {index} has no widget to redraw"); + return; + }; + let content = build_card(rsc, shared, index); + let _old = ptr(rsc).replace(content); +} + +/// A card and the tap that opens it. The gesture is registered **once**, +/// on a `WidgetPtr` whose content is replaced as often as needed -- which +/// is why every rebuild reads the call out of [`Shared`] rather than +/// capturing one. +fn build_card_ptr( + rsc: &mut Rsc, + shared: &Rc, + index: usize, +) -> (StrongWidget, WeakWidget) +where + Rsc::State: FocusHost + OpenUrl, +{ + // The strong handle is the card's one real registration and goes to + // whatever container holds it; the weak one is what the gesture and + // every later redraw address it by. + let strong = WidgetPtr::new().add_strong(rsc); + let ptr = strong.weak(); + shared.cards.borrow_mut().push(ptr); + debug_assert_eq!( + shared.cards.borrow().len(), + index + 1, + "a card's index is its position, and both are the call's" + ); + let content = build_card(rsc, shared, index); + ptr(rsc).set(content); + let for_tap = shared.clone(); + on_tap(rsc, ptr, shared, move |rsc| { + note_tap(rsc, &for_tap); + let Some(id) = for_tap.call_id(index) else { + debug_assert!(false, "tapped card {index} is no longer in the row"); + return; + }; + let was = for_tap + .state + .borrow() + .open + .get(&id) + .copied() + .unwrap_or(false); + for_tap.state.borrow_mut().open.insert(id, !was); + redraw_card(rsc, &for_tap, index); + }); + (strong.any(), ptr) +} + +/// A bar the height of one line of `LABEL_SIZE` text, carrying `mark` +/// centred -- the group's collapse control at its foot. +/// +/// Given the same content as the heading above rather than a height that +/// looks close, so the surface the calls sit on is the same thickness at +/// both ends (`ToolRows.kt`'s `groupBarHeight`, which derives the number +/// from the type for the same reason). +fn collapse_bar(rsc: &mut Rsc, shared: &Rc) -> StrongWidget +where + Rsc::State: FocusHost + OpenUrl, +{ + let strong = WidgetPtr::new().add_strong(rsc); + let ptr = strong.weak(); + let mark = text(UP_MARK, BODY_SIZE, MUTED_COLOR) + .family(Family::Monospace) + .center() + .width(rest(1)) + .pad(dp(CARD_PAD_DP)) + // Anything shown only as a mark still needs a name: this is what + // a screen reader reads and what a `ui-trace` script taps. + .label("Collapse these tool calls") + .add_strong(rsc); + ptr(rsc).set(mark); + let for_tap = shared.clone(); + on_tap(rsc, ptr, shared, move |rsc| toggle_group(rsc, &for_tap)); + strong.any() +} + +/// The row's whole content: a lone card, a closed group's one line, or an +/// open group's header, cards and foot. +/// +/// Rebuilt whole when the group opens or closes, because that is a change +/// of what the row *is* rather than of one card in it. Everything a single +/// card's tap does goes through [`redraw_card`] instead. +fn build_content(rsc: &mut Rsc, shared: &Rc) -> StrongWidget +where + Rsc::State: FocusHost + OpenUrl, +{ + shared.cards.borrow_mut().clear(); + let count = shared.calls.borrow().len(); + debug_assert!(count > 0, "a tool row with no calls has nothing to draw"); + + // One call is left alone: "Called 1 tool" hides a card to say the same + // thing in more words, and the run this grouping exists for is the + // burst of five greps nobody wants to scroll past (`ToolRows.kt`). + if count == 1 { + return build_card_ptr(rsc, shared, 0).0; + } + + if !shared.state.borrow().group_expanded { + let heading = group_label(count); + return text(heading.clone(), NAME_SIZE, NAME_COLOR) + .pad(dp(CARD_PAD_DP)) + .width(rest(1)) + .background(rect(CARD_FILL).radius(dp(CARD_RADIUS_DP))) + .width(rest(1)) + .label(heading) + .add_strong(rsc) + .any(); + } + + let heading = group_label(count); + let mut group = Span::empty(Dir::DOWN); + group.push( + text(heading.clone(), NAME_SIZE, NAME_COLOR) + .pad(dp(CARD_PAD_DP)) + .width(rest(1)) + .label(heading) + .add_strong(rsc) + .any(), + ); + // The cards go straight into the group's own `Span`, not into a + // second one inside it. **A `Span` of `Pad`ded children inside another + // `Span` places those children a slot out of step** -- each card's + // content drew one card's height below its own box, so the cards read + // as empty bars with somebody else's summary in them. Bisected on + // 2026-09-06 against `iris/run-headless.sh transcript` with + // `IRIS_TOOLS_EXPANDED=1`: removing the inner `Span` fixes it and + // removing the cards' own `Pad` fixes it, while the card background, + // the `Sized` wrappers and the per-card `WidgetPtr` all make no + // difference. It is a framework defect rather than this file's -- + // docs/RUST.md's P1b box and docs/IRIS_TODO.md carry the repro -- and + // one `Span` is the shape that works today. What it costs is the 4dp + // inset the Compose group holds its cards off its edge by; the cards' + // own padding stands in for it. + for index in 0..count { + group.push(build_card_ptr(rsc, shared, index).0); + } + // Shutting it from here anchors the other end: the reader is at the + // bottom of a long group, and what they are looking at is what follows + // it (`ToolRows.kt`'s `CollapseBar`). + group.push(collapse_bar(rsc, shared)); + group + .width(rest(1)) + .background(rect(GROUP_FILL).radius(dp(CARD_RADIUS_DP))) + .width(rest(1)) + .add_strong(rsc) + .any() +} + +fn toggle_group(rsc: &mut Rsc, shared: &Rc) +where + Rsc::State: FocusHost + OpenUrl, +{ + note_tap(rsc, shared); + let was = shared.state.borrow().group_expanded; + shared.state.borrow_mut().group_expanded = !was; + let content = build_content(rsc, shared); + shared.set_content(rsc, content); +} + +impl Shared { + /// Swap the row's whole content. The old `StrongWidget` is freed as it + /// drops here, which is the removal half of what replaced it. + fn set_content(&self, rsc: &mut impl UiRsc, content: StrongWidget) { + let Some(ptr) = *self.content.borrow() else { + debug_assert!( + false, + "the row's content pointer is set before anything can tap it" + ); + return; + }; + let _old = ptr(rsc).replace(content); + } + + fn call_id(&self, index: usize) -> Option { + match self.calls.borrow().get(index) { + Some(TranscriptItem::ToolRun { id, .. }) => Some(id.clone()), + _ => None, + } + } + + fn card_ptr(&self, index: usize) -> Option> { + self.cards.borrow().get(index).copied() + } +} + +/// Build a tool row: one card, or a run of them under one heading. +/// +/// `working` is the caller's `session_working` **for this row** -- true +/// only for the newest row of a session that is still doing something. +/// Every row behind it belongs to a turn that has ended, so a call in one +/// with no result never came back rather than still running. +pub fn build_tool_row( + rsc: &mut Rsc, + list: WeakWidget, + selection: Rc>, + key: RowKey, + calls: Vec, + working: bool, +) -> (StrongWidget, ToolRow) +where + Rsc::State: FocusHost + OpenUrl, +{ + let shared = Rc::new(Shared { + calls: RefCell::new(calls), + state: RefCell::new(ToolRowState::default()), + cards: RefCell::new(Vec::new()), + content: RefCell::new(None), + list, + selection, + key, + working: Cell::new(working), + }); + // `.add_strong`, not `.add`: this row *is* the top of its own subtree, + // so nothing else holds it and it has to own itself (`row.rs`). + let content_strong = WidgetPtr::new().add_strong(rsc); + let content = content_strong.weak(); + *shared.content.borrow_mut() = Some(content); + let inner = build_content(rsc, &shared); + content(rsc).set(inner); + (content_strong.any(), ToolRow { shared }) +} + +impl ToolRow { + /// The calls this row is currently drawing -- what a caller passes + /// back to [`Self::apply_calls`] when something other than the calls + /// themselves changed (the session's status). + pub fn calls(&self) -> Vec { + self.shared.calls.borrow().clone() + } + + /// How many cards this row currently draws -- zero for a closed + /// group, which is the whole reason its calls' outputs cost nothing. + /// Only the tests ask; nothing on screen is decided by it. + #[cfg(test)] + pub(crate) fn card_count(&self) -> usize { + self.shared.cards.borrow().len() + } + + /// Open or close this row's group without a tap. + /// + /// Exists because the expanded appearance is otherwise unreachable + /// from anything that cannot press the screen -- a headless + /// screenshot on this displayless machine, and a test. Same path a tap + /// takes, including `List::note_tap`, so what it produces is what a + /// reader would have got. + pub fn set_group_expanded(&self, rsc: &mut Rsc, expanded: bool) + where + Rsc::State: FocusHost + OpenUrl, + { + if self.shared.state.borrow().group_expanded != expanded { + toggle_group(rsc, &self.shared); + } + } + + /// Bring this row up to date with `calls` **without** rebuilding the + /// cards that did not change, and say whether that was possible. + /// `false` means the caller must rebuild the row the ordinary way. + /// + /// This is what the per-card `WidgetPtr` exists for: a `ToolEnd` + /// changes one call, so it costs one card, whatever else is in the + /// run. The same rule `RowBlocks::apply_delta` follows for the blocks + /// of a message. + /// + /// Refused when a call *left* the row or the calls were reordered: a + /// card's index is its call's position, and every registered handler + /// closed over that index. A run only ever grows at its end while it + /// is the live row, so the refused cases are the ones a page join + /// produces -- and those go through `Rebuild` already. + pub fn apply_calls( + &mut self, + rsc: &mut Rsc, + calls: &[TranscriptItem], + working: bool, + ) -> bool + where + Rsc::State: FocusHost + OpenUrl, + { + // A row that was tool calls and now holds something else is a + // different row, not a changed one -- and nothing here could draw + // a message anyway. + if calls.is_empty() + || !calls + .iter() + .all(|c| matches!(c, TranscriptItem::ToolRun { .. })) + { + return false; + } + let old = self.shared.calls.borrow().clone(); + if calls.len() < old.len() { + return false; + } + // Whether the group is drawn as one card or as a stack changes at + // exactly one call, and that is a different row, not a changed + // one. + if (old.len() == 1) != (calls.len() == 1) { + return false; + } + let changed: Vec = (0..old.len()).filter(|&i| old[i] != calls[i]).collect(); + self.shared.working.set(working); + *self.shared.calls.borrow_mut() = calls.to_vec(); + // The path out for the reader's own state: a call that is no + // longer in this row keeps no entry in `open`/`whole_output`. + let ids: std::collections::HashSet = calls + .iter() + .filter_map(|c| match c { + TranscriptItem::ToolRun { id, .. } => Some(id.clone()), + _ => None, + }) + .collect(); + { + let mut state = self.shared.state.borrow_mut(); + state.open.retain(|id, _| ids.contains(id)); + state.whole_output.retain(|id, _| ids.contains(id)); + } + + // A collapsed group draws no cards, so a changed call is worth + // nothing on screen -- unless the *count* changed, which is the + // whole of what its one line says. + if self.shared.cards.borrow().is_empty() { + if calls.len() != old.len() { + let content = build_content(rsc, &self.shared); + self.shared.set_content(rsc, content); + } + return true; + } + debug_assert_eq!( + self.shared.cards.borrow().len(), + old.len(), + "an open row draws exactly one card per call" + ); + + for index in changed { + redraw_card(rsc, &self.shared, index); + } + // A call *joining* the run rebuilds the row's content rather than + // appending one card: the group's `Span` holds its collapse bar + // after the cards, and `Span::push` would put the new card behind + // it. That is still O(this row) -- every other row is untouched -- + // and it is much rarer than a result arriving, which is the case + // the per-card `WidgetPtr` above exists for. + if calls.len() > old.len() { + let content = build_content(rsc, &self.shared); + self.shared.set_content(rsc, content); + } + true + } +} diff --git a/server/src/session/claude/translate.rs b/server/src/session/claude/translate.rs index 72964e4..d37bd2b 100644 --- a/server/src/session/claude/translate.rs +++ b/server/src/session/claude/translate.rs @@ -700,6 +700,7 @@ impl Translator { events.push(Event::ToolEnd { id: about.clone(), output: texts.join("\n"), + is_error: crate::session::import::tool_result_is_error(block), }); // Deliberately does *not* finish a subagent `about` might name: // the Task tool runs in the background by default, so this @@ -1015,7 +1016,44 @@ mod tests { }, Event::ToolEnd { id: "toolu_01".to_string(), - output: "probe-ok".to_string() + output: "probe-ok".to_string(), + is_error: false, + }, + ] + ); + } + + /// The other half of the test above, and the one it cannot stand in + /// for: a call the tool itself reported as failed. Both lines are + /// `tool_result`s and both carry output, so nothing but `is_error` + /// tells them apart -- which is why dropping the field made a broken + /// call draw exactly like one that worked. + #[test] + fn a_failed_tool_result_says_so() { + let dir = tempfile::tempdir().expect("tempdir"); + let mut translator = Translator::new(dir.path().to_path_buf(), test_subagents(&dir)); + let events = translate_lines( + &mut translator, + &[ + r#"{"type":"user","message":{"role":"user","content":[{"tool_use_id":"toolu_02","type":"tool_result","content":"No such file or directory","is_error":true}]},"parent_tool_use_id":null}"#, + // No `is_error` at all: every transcript written before + // the field was read looks like this, and it means the + // call was not reported to have failed. + r#"{"type":"user","message":{"role":"user","content":[{"tool_use_id":"toolu_03","type":"tool_result","content":"fine"}]},"parent_tool_use_id":null}"#, + ], + ); + assert_eq!( + events, + vec![ + Event::ToolEnd { + id: "toolu_02".to_string(), + output: "No such file or directory".to_string(), + is_error: true, + }, + Event::ToolEnd { + id: "toolu_03".to_string(), + output: "fine".to_string(), + is_error: false, }, ] ); @@ -1459,6 +1497,7 @@ mod tests { Event::ToolEnd { id: "toolu_05".to_string(), output: "took a screenshot".to_string(), + is_error: false, } ); } diff --git a/server/src/session/echo.rs b/server/src/session/echo.rs index c899524..3e055c8 100644 --- a/server/src/session/echo.rs +++ b/server/src/session/echo.rs @@ -134,6 +134,7 @@ impl EchoDriver { self.emit(Event::ToolEnd { id, output: format!("{label} step {index} finished"), + is_error: false, }); } } @@ -645,6 +646,7 @@ impl EchoDriver { send(Event::ToolEnd { id, output: format!("call {i} finished"), + is_error: false, }); } finish(); @@ -698,6 +700,7 @@ impl EchoDriver { send(Event::ToolEnd { id, output: format!("ran: {command}"), + is_error: false, }); } @@ -717,6 +720,7 @@ impl EchoDriver { send(Event::ToolEnd { id, output: format!("echoed: {input}"), + is_error: false, }); } @@ -817,6 +821,7 @@ async fn write_beat(sink: &EventSink, session_dir: &Path, beat: usize) { send(Event::ToolEnd { id, output: format!("beat {beat}: forty-two lines of nothing in particular"), + is_error: false, }); } // A run of three, which the app folds into one collapsed group -- the @@ -829,9 +834,20 @@ async fn write_beat(sink: &EventSink, session_dir: &Path, beat: usize) { tool: if i % 2 == 0 { "Bash" } else { "Grep" }.to_string(), input: serde_json::json!({ "command": format!("grep -rn 'beat {beat}' /tmp") }), }); + // The middle one fails, so this fixture carries a run in + // which the three calls are not all in the same state -- + // the case a card drawing every finished call the same way + // looks correct on. `is_error` is the CLI's own field + // (`import::tool_result_is_error`), and a driver that + // never sets it makes the failed appearance unreachable + // from the sandbox. send(Event::ToolEnd { id, - output: format!("beat {beat}, call {i} of 3"), + output: match i == 2 { + true => format!("beat {beat}, call {i} of 3: No such file or directory"), + false => format!("beat {beat}, call {i} of 3"), + }, + is_error: i == 2, }); } } @@ -856,6 +872,7 @@ async fn write_beat(sink: &EventSink, session_dir: &Path, beat: usize) { send(Event::ToolEnd { id, output: format!("beat {beat}: captured"), + is_error: false, }); } // Somebody else's voice, which is its own row shape. @@ -904,6 +921,7 @@ async fn run_helper(id: String, sink: EventSink, subagents: Arc) { Event::ToolEnd { id: tool_id, output: "helper done".to_string(), + is_error: false, }, ); let target = Duration::from_secs(3); @@ -915,6 +933,7 @@ async fn run_helper(id: String, sink: EventSink, subagents: Arc) { let _ = sink.send(Event::ToolEnd { id, output: "subagent finished".to_string(), + is_error: false, }); } @@ -1069,6 +1088,7 @@ impl Driver for EchoDriver { self.emit(Event::ToolEnd { id: call, output: format!("answered: {answer}"), + is_error: false, }); // The work carries on where it left off, which is what makes the // asked-here row a boundary with a group on each side rather than diff --git a/server/src/session/import.rs b/server/src/session/import.rs index b9bfc74..d5289da 100644 --- a/server/src/session/import.rs +++ b/server/src/session/import.rs @@ -363,6 +363,22 @@ fn is_hidden(record: &Value) -> bool { || record.get("isMeta").and_then(Value::as_bool) == Some(true) } +/// Whether a `tool_result` block says the call itself failed. +/// +/// One reader for the field rather than one per caller: the live +/// translator (`translate.rs`) and this replay of the CLI's own file look +/// at the same block shape, and a call drawn as failed in one and as +/// succeeded in the other would be the same conversation disagreeing with +/// itself. Absent means "not reported to have failed" -- which is what the +/// CLI writes for a call that went fine, and also what every transcript +/// written before this field was read says. +pub(crate) fn tool_result_is_error(block: &Value) -> bool { + block + .get("is_error") + .and_then(Value::as_bool) + .unwrap_or(false) +} + fn text_of(content: &Value) -> String { match content { Value::String(text) => text.clone(), @@ -549,6 +565,7 @@ fn push_user(events: &mut Vec, content: &Value, session_dir: &std::path:: events.push(Event::ToolEnd { id: id.to_string(), output: text_of(block.get("content").unwrap_or(&Value::Null)), + is_error: tool_result_is_error(block), }); } } diff --git a/server/src/session/transcript.rs b/server/src/session/transcript.rs index 2ac74fd..68f4469 100644 --- a/server/src/session/transcript.rs +++ b/server/src/session/transcript.rs @@ -744,6 +744,7 @@ mod tests { Event::ToolEnd { id: "t1".into(), output: "done".into(), + is_error: false, }, Event::Image { image: "img1".into(),