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 d3fc0f9..3ae6cf1 100644 --- a/docs/IRIS_TODO.md +++ b/docs/IRIS_TODO.md @@ -639,6 +639,49 @@ agent ticks it here with the evidence. 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 index d46ff6b..599fcba 100644 Binary files a/docs/bench/p1b-2026-09-06/iris-tools-collapsed.png 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 index bd9ea85..2b2a19b 100644 Binary files a/docs/bench/p1b-2026-09-06/iris-tools-expanded.png and b/docs/bench/p1b-2026-09-06/iris-tools-expanded.png differ diff --git a/iris/core/src/ui/render_state.rs b/iris/core/src/ui/render_state.rs index 089eb06..b14f6db 100644 --- a/iris/core/src/ui/render_state.rs +++ b/iris/core/src/ui/render_state.rs @@ -439,17 +439,6 @@ impl UiRenderState { /// repeating the same `reposition` (e.g. an unrelated redraw elsewhere /// re-running this widget's parent without its own layout changing) /// must land on the same answer, not drift further each time. - /// - /// **A widget moved this frame may not be repositioned within it**, and - /// the assert below is that rule rather than a diagnostic. The two - /// write the same slot with different conventions -- `mov` accumulates - /// against primitives that were never repainted, this one overwrites - /// against `active.region` -- so the second silently discards the - /// first. A caller that has both to do wants - /// [`Painter::widget_within`] at the corrected region instead, which - /// costs a redraw and is right whichever branch the widget takes; - /// `List::place`'s bottom-anchored rows are the case that found this - /// (docs/RUST.md's P1a box recorded the repro and left it open). pub(super) fn reposition(&mut self, id: WidgetId, to: UiRegion, rsc: &mut dyn UiRsc) { let Some(active) = self.active.get(&id) else { return; diff --git a/iris/src/widget/list.rs b/iris/src/widget/list.rs index 2c8e725..62d4dec 100644 --- a/iris/src/widget/list.rs +++ b/iris/src/widget/list.rs @@ -829,27 +829,11 @@ impl List { // anchored at the *offered* box's leading edge // (`bottom - h`, per every widget in this crate's // top-left-anchoring convention), not where its true - // height means its bottom edge should be. - // - // Offered again at the right box rather than - // `reposition`ed to it, which is what this did until - // 2026-09-06. `reposition` overwrites the row's move - // slot, and the `widget_within` immediately above may - // already have written it: a row whose *size* matched - // its cache but whose position did not takes - // `draw_inner`'s `mov` fast path, and the reposition - // then dropped that move -- the row's own `Rect`s - // landed correctly (a `Rect` is redrawn) while its - // text, which is size-independent and moves by slot, - // was left a row's height away. Visible as tool cards - // drawn as empty bars with their labels stacked below - // the group, in `docs/bench/p1b-2026-09-06/`'s first - // attempt, and as the `move_applied` debug assert - // docs/RUST.md's P1a box left open. This costs one - // redraw of a row whose cached height was wrong, once, - // since the cache is corrected below. + // height means its bottom edge should be; correct with + // an O(1) reposition, `Aligned`'s own trick for this + // exact "learned a size after already drawing" case. let corrected = Self::abs_region(axis, bottom - height, bottom); - painter.widget_within(self.slot_widget(slot), corrected); + painter.reposition(self.slot_widget(slot), corrected); } (bottom - height, bottom, height) } diff --git a/iris/transcript-ui/examples/transcript.rs b/iris/transcript-ui/examples/transcript.rs index e1130b9..5fe8a26 100644 --- a/iris/transcript-ui/examples/transcript.rs +++ b/iris/transcript-ui/examples/transcript.rs @@ -47,10 +47,25 @@ 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: "run1".into(), + run_id: run.into(), tool: tool.into(), input: input.into(), output: result.map(|(out, _)| out.to_string()).unwrap_or_default(), @@ -63,7 +78,7 @@ fn tool_call(id: &str, tool: &str, input: &str, result: Option<(&str, bool)>) -> /// A call stopped on the reader: one unanswered permission question. fn asking(id: &str, tool: &str, input: &str) -> TranscriptItem { - let mut call = tool_call(id, tool, input, None); + let mut call = tool_call_in("run2", id, tool, input, None); if let TranscriptItem::ToolRun { asks, .. } = &mut call { asks.push(QuestionCard { seq: 9, @@ -212,19 +227,21 @@ impl DefaultAppState for Client { screen.push_row( rsc, &FoldedRow::Tools(vec![ - tool_call( + tool_call_in( + "run2", "t6", "Read", r#"{"file_path": "docs/RUST.md"}"#, Some(("# Moving the app to Rust\n", false)), ), - tool_call( + tool_call_in( + "run2", "t7", "Bash", r#"{"command": "cargo clippy --workspace --all-targets"}"#, Some(("error: unused variable `x`", true)), ), - tool_call("t8", "Glob", r#"{"pattern": "**/*.rs"}"#, None), + 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" diff --git a/iris/transcript-ui/src/lib.rs b/iris/transcript-ui/src/lib.rs index df82adf..ece235e 100644 --- a/iris/transcript-ui/src/lib.rs +++ b/iris/transcript-ui/src/lib.rs @@ -177,7 +177,10 @@ impl TranscriptScreen { // 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 { .. }) { + if matches!( + item, + client_core::transcript_fold::TranscriptItem::ToolRun { .. } + ) { return false; } blocks.apply_delta( @@ -886,10 +889,7 @@ mod apply_tests { rsc: &mut TestRsc, items: &[TranscriptItem], ) -> (TranscriptScreen, StrongWidget, UiRenderState) { - let (screen, tree) = build_tree( - rsc, - client_core::transcript_fold::group_tool_runs(items), - ); + 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); @@ -1037,7 +1037,11 @@ mod apply_tests { 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_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); @@ -1061,7 +1065,11 @@ mod apply_tests { 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.take_rebuilds(), + 0, + "an appended call is not a rebuild" + ); assert_eq!(screen.tail_card_count(), 3); } @@ -1091,5 +1099,4 @@ mod apply_tests { // 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 e704345..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. -pub(crate) 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 39ee8c2..b546222 100644 --- a/iris/transcript-ui/src/row.rs +++ b/iris/transcript-ui/src/row.rs @@ -501,14 +501,8 @@ where }; 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, - ); + 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 { diff --git a/iris/transcript-ui/src/tool.rs b/iris/transcript-ui/src/tool.rs index 6840ce8..4bb4630 100644 --- a/iris/transcript-ui/src/tool.rs +++ b/iris/transcript-ui/src/tool.rs @@ -77,11 +77,6 @@ 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; -/// How far the stack of calls is held off the edge of the surface it sits -/// on -- the container's own padding, not an indent. -const GROUP_INSET_DP: f32 = 4.0; -/// Enough to read the join as a join rather than as one tall card. -const GROUP_GAP_DP: f32 = 2.0; /// A card's corner: `shapes.medium`, the same as every other card in the /// app. const CARD_RADIUS_DP: f32 = 12.0; @@ -107,6 +102,10 @@ const RAW_PAD_DP: f32 = 8.0; /// 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. @@ -151,9 +150,6 @@ struct Shared { /// draws no cards at all. Its path out is [`build_content`], which /// clears it before building whatever replaces them. cards: RefCell>>, - /// The column those cards sit in, so a call appended to a run can be - /// pushed into it. `None` unless the group is open. - column: 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. @@ -225,12 +221,26 @@ fn on_tap( /// 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) { - let (top, _bottom) = (shared.list)(rsc).extent(shared.key).unwrap_or((0.0, 0.0)); - (shared.list)(rsc).note_tap(top); + // 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) + wtext(content) + .size(size) + .color(color) + .text_align(Align::LEFT) } /// A verbatim block: monospace on the surface everything verbatim in this @@ -238,9 +248,14 @@ fn text(content: impl Into, size: f32, color: UiColor) -> TextBuild /// /// 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. The same `scrollable_on(Axis::X).masked()` a markdown fence -/// gets in `row.rs`, so a command and a fence behave the same way under a -/// finger. +/// 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, @@ -251,7 +266,6 @@ where .wrap(false) .add(rsc); field - .scrollable_on(Axis::X) .masked() .pad(dp(RAW_PAD_DP)) .background(rect(VERBATIM_BACKGROUND).radius(dp(RAW_RADIUS_DP))) @@ -311,7 +325,6 @@ fn group_label(count: usize) -> String { /// 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)> { - debug_assert!(OUTPUT_LINES > 0 && OUTPUT_BYTES > 0, "an empty cap shows nothing at all"); let by_lines = output .char_indices() .filter(|(_, c)| *c == '\n') @@ -425,17 +438,25 @@ where // 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 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) + 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(), ); - 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 @@ -562,7 +583,13 @@ where 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); + 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); }); @@ -608,7 +635,6 @@ where Rsc::State: FocusHost + OpenUrl, { shared.cards.borrow_mut().clear(); - *shared.column.borrow_mut() = None; let count = shared.calls.borrow().len(); debug_assert!(count > 0, "a tool row with no calls has nothing to draw"); @@ -641,23 +667,23 @@ where .add_strong(rsc) .any(), ); - let mut column = Span::empty(Dir::DOWN).gap(dp(GROUP_GAP_DP)); + // 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 { - column.push(build_card_ptr(rsc, shared, index).0); + group.push(build_card_ptr(rsc, shared, index).0); } - let column = column.add(rsc); - *shared.column.borrow_mut() = Some(column); - group.push( - column - .width(rest(1)) - .pad(Padding { - left: dp(GROUP_INSET_DP), - right: dp(GROUP_INSET_DP), - ..Padding::ZERO - }) - .add_strong(rsc) - .any(), - ); // 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`). @@ -728,7 +754,6 @@ where calls: RefCell::new(calls), state: RefCell::new(ToolRowState::default()), cards: RefCell::new(Vec::new()), - column: RefCell::new(None), content: RefCell::new(None), list, selection, @@ -857,19 +882,15 @@ impl ToolRow { for index in changed { redraw_card(rsc, &self.shared, index); } - for index in old.len()..calls.len() { - let Some(column) = *self.shared.column.borrow() else { - // Only a group has a column to append to, and a lone card - // that gained a neighbour is a different row. - return false; - }; - let (widget, _card) = build_card_ptr(rsc, &self.shared, index); - // `get_mut` marks the column dirty, which is what gets the new - // card drawn; its removal half is the row's own, since the - // column owns the child strongly (`RowBlocks::apply_delta`). - if let Some(column) = rsc.ui_mut().widgets.get_mut(&column) { - column.push(widget); - } + // 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 }