From e1030d69f6809a480aff80616357f4b38ef0d849 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Sun, 6 Sep 2026 17:33:37 -0400 Subject: [PATCH] iris: a transcript row is a column of markdown blocks, so a streamed delta costs one block A row was one TextEdit holding the whole message, so every delta re-shaped every paragraph of a long reply through parley -- the one phase where iris trails Compose on the phone (p50 18.2ms vs 13.4ms, bench v2). - client-core/src/markdown_blocks.rs: split a message into its top-level blocks with their source, through the same pulldown-cmark the renderer parses with so the two cannot disagree about where a block starts, plus common_prefix. Appending markdown can rewrite an earlier block (a trailing --- turns the paragraph above into a heading), so the fast path compares the prefix it keeps rather than assuming it -- with the test that says so. - transcript-ui: a row is a Span of one TextEdit per block; RowBlocks::apply_delta replaces the block a delta lands in; TranscriptScreen keeps the tail row's blocks, seeded in build_tree as well as push_row (a screen opened onto a streaming reply took the rebuild path for its first delta otherwise, with nothing to say so). - A block is the selection unit: Selection is keyed by (RowKey, u32), which is reading order at both levels, and the pointer-captured half of a drag resolves the block under the finger from its drawn box (Selection::locate) instead of from the row's extent. Pass condition: a_delta_into_a_long_reply_redraws_the_same_widgets_as_a_short_one drives a real UiRenderState and asserts the draw count for a delta into a 100-paragraph (3,000+ char) reply equals the count for a one-paragraph one. 30 either way; it read 630 against 30 twice on the way there. Emulator stream phase, same AVD before and after: p50 61.5 -> 54.5ms, p90 211.7 -> 113.1ms, p99 342.6 -> 137.4ms, worst 403.6 -> 143.0ms, 202 -> 293 frames in the same 21 seconds. Selection across blocks verified with a real long-press drag. Co-Authored-By: Claude Fable 5.1 --- android-shell/Cargo.lock | 47 ++++++ client-core/Cargo.lock | 41 +++++ client-core/Cargo.toml | 6 + client-core/src/lib.rs | 1 + client-core/src/markdown_blocks.rs | 234 ++++++++++++++++++++++++++++ docs/CLIENT_CORE.md | 21 +-- docs/IRIS.md | 30 ++++ docs/IRIS_TODO.md | 19 ++- docs/RUST.md | 71 ++++++++- iris/Cargo.lock | 1 + iris/android-app/Cargo.lock | 1 + iris/transcript-ui/src/lib.rs | 175 ++++++++++++++++++--- iris/transcript-ui/src/row.rs | 227 +++++++++++++++++++++++---- iris/transcript-ui/src/selection.rs | 79 +++++++--- 14 files changed, 871 insertions(+), 82 deletions(-) create mode 100644 client-core/src/markdown_blocks.rs diff --git a/android-shell/Cargo.lock b/android-shell/Cargo.lock index 0e0a2df..3f93d65 100644 --- a/android-shell/Cargo.lock +++ b/android-shell/Cargo.lock @@ -50,6 +50,12 @@ version = "0.23.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ac07cdecf99051d9a5238b80f35af32cdeba5b336e55d957b318b50137e18da5" +[[package]] +name = "bitflags" +version = "2.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b588b76d00fde79687d7646a9b5bdf3cc0f655e0bbd080335a95d7e96f3587da" + [[package]] name = "bytes" version = "1.12.1" @@ -77,6 +83,7 @@ name = "client-core" version = "0.1.0" dependencies = [ "event-model", + "pulldown-cmark", "serde", "serde_json", "ureq", @@ -206,6 +213,15 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "getopts" +version = "0.2.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cfe4fbac503b8d1f88e6676011885f34b7174f46e59956bba534ba83abded4df" +dependencies = [ + "unicode-width", +] + [[package]] name = "getrandom" version = "0.2.17" @@ -490,6 +506,25 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "pulldown-cmark" +version = "0.13.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e9f068eba8e7071c5f9511831b44f32c740d5adf574e990f946ddb53db2f314e" +dependencies = [ + "bitflags", + "getopts", + "memchr", + "pulldown-cmark-escape", + "unicase", +] + +[[package]] +name = "pulldown-cmark-escape" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "007d8adb5ddab6f8e3f491ac63566a7d5002cc7ed73901f72057943fa71ae1ae" + [[package]] name = "quote" version = "1.0.47" @@ -783,12 +818,24 @@ dependencies = [ "zerovec", ] +[[package]] +name = "unicase" +version = "2.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dbc4bc3a9f746d862c45cb89d705aa10f187bb96c76001afab07a0d35ce60142" + [[package]] name = "unicode-ident" version = "1.0.24" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" +[[package]] +name = "unicode-width" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b4ac048d71ede7ee76d585517add45da530660ef4390e49b098733c6e897f254" + [[package]] name = "untrusted" version = "0.9.0" diff --git a/client-core/Cargo.lock b/client-core/Cargo.lock index 42f6d46..627138e 100644 --- a/client-core/Cargo.lock +++ b/client-core/Cargo.lock @@ -47,6 +47,7 @@ name = "client-core" version = "0.1.0" dependencies = [ "event-model", + "pulldown-cmark", "serde", "serde_json", "tempfile", @@ -173,6 +174,15 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "getopts" +version = "0.2.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cfe4fbac503b8d1f88e6676011885f34b7174f46e59956bba534ba83abded4df" +dependencies = [ + "unicode-width", +] + [[package]] name = "getrandom" version = "0.2.17" @@ -425,6 +435,25 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "pulldown-cmark" +version = "0.13.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e9f068eba8e7071c5f9511831b44f32c740d5adf574e990f946ddb53db2f314e" +dependencies = [ + "bitflags", + "getopts", + "memchr", + "pulldown-cmark-escape", + "unicase", +] + +[[package]] +name = "pulldown-cmark-escape" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "007d8adb5ddab6f8e3f491ac63566a7d5002cc7ed73901f72057943fa71ae1ae" + [[package]] name = "quote" version = "1.0.47" @@ -661,12 +690,24 @@ dependencies = [ "zerovec", ] +[[package]] +name = "unicase" +version = "2.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dbc4bc3a9f746d862c45cb89d705aa10f187bb96c76001afab07a0d35ce60142" + [[package]] name = "unicode-ident" version = "1.0.24" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" +[[package]] +name = "unicode-width" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b4ac048d71ede7ee76d585517add45da530660ef4390e49b098733c6e897f254" + [[package]] name = "untrusted" version = "0.9.0" diff --git a/client-core/Cargo.toml b/client-core/Cargo.toml index 2cff592..ab8e9ce 100644 --- a/client-core/Cargo.toml +++ b/client-core/Cargo.toml @@ -32,6 +32,12 @@ serde_json = { version = "1", features = ["float_roundtrip", "raw_value"] } # no need of an async runtime, and RUST.md's brief for this port is # "lightweight" throughout. ureq = { version = "3", features = ["json"] } +# The markdown block split (`markdown_blocks`), which has to agree with the +# renderer in `iris/transcript-ui` about where a block begins -- so it is +# the same parser at the same version, rather than a hand-written splitter +# that would drift from it. +pulldown-cmark = "0.13.4" + [dev-dependencies] tempfile = "3" diff --git a/client-core/src/lib.rs b/client-core/src/lib.rs index a8458c5..d94e4f5 100644 --- a/client-core/src/lib.rs +++ b/client-core/src/lib.rs @@ -7,6 +7,7 @@ pub mod api; pub mod config; pub mod event_stream; pub mod highlight; +pub mod markdown_blocks; pub mod notifications; pub mod sse; pub mod transcript_cache; diff --git a/client-core/src/markdown_blocks.rs b/client-core/src/markdown_blocks.rs new file mode 100644 index 0000000..9eb4a93 --- /dev/null +++ b/client-core/src/markdown_blocks.rs @@ -0,0 +1,234 @@ +//! Split a markdown message into its top-level **blocks** -- one +//! paragraph, heading, fenced code block, list, table or quote each, as a +//! byte slice of the original source. +//! +//! This exists for streaming. A transcript row used to be one text widget +//! holding the whole message, so a single streamed delta re-shaped every +//! paragraph of it through the text engine again; the phone's bench v2 put +//! the stream phase at p50 18.2ms against Compose's 13.4ms for exactly +//! that reason (docs/IRIS_TODO.md). A row is a column of one widget per +//! block now, and a delta that lands in the last block leaves every +//! earlier block's layout alone. `docs/DECISIONS.md`'s 2026-09-06 entry has +//! what that rejected and why the split lives here rather than in the UI +//! crate: `docs/CLIENT_CORE.md` already wanted a block model for P1, and +//! keeping it here means iris stays a text renderer that knows nothing +//! about markdown. +//! +//! **Blocks only.** Inline styling (bold, links, inline code) is still the +//! renderer's own job, per block -- this deliberately does not build a +//! full AST, because nothing needs one yet. +//! +//! ## Appending is not guaranteed to leave earlier blocks alone +//! +//! It nearly always does, which is what makes the fast path worth having, +//! but markdown has no such rule: appending a "```" line can turn text +//! that was three paragraphs into one fenced block, and appending "---" +//! under a paragraph turns that paragraph into a heading. So a caller +//! taking the O(last block) path **must compare the prefix it is about to +//! keep** rather than assume it. [`common_prefix`] is that comparison, and +//! it is cheap next to laying the text out again. + +use pulldown_cmark::{Event, Options, Parser, Tag}; + +/// What a block is, for a renderer that wants to style or space blocks +/// differently. `Other` is deliberately present rather than a panic or a +/// silent fallback to `Paragraph`: markdown has more block kinds than this +/// list and more get added, and a renderer treating an unknown one as +/// prose is right, but it should be able to *tell* that is what it is +/// doing. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum BlockKind { + Paragraph, + Heading, + /// A fenced or indented code block. + Code, + List, + Table, + Quote, + /// A thematic break, raw HTML, a footnote -- anything with no + /// distinguished treatment here. + Other, +} + +/// One top-level block: its kind and the exact source that produced it. +/// `source` is a slice of the input with trailing whitespace removed, so +/// two splits of the same prefix compare equal even when one of them had a +/// delta arriving after it. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Block { + pub kind: BlockKind, + pub source: String, +} + +fn kind_of(tag: &Tag) -> BlockKind { + match tag { + Tag::Paragraph => BlockKind::Paragraph, + Tag::Heading { .. } => BlockKind::Heading, + Tag::CodeBlock(_) => BlockKind::Code, + Tag::List(_) => BlockKind::List, + Tag::Table(_) => BlockKind::Table, + Tag::BlockQuote(_) => BlockKind::Quote, + _ => BlockKind::Other, + } +} + +fn options() -> Options { + // The same set `transcript-ui`'s renderer parses with, so a block + // boundary here and the styling there cannot disagree about what the + // source means. + Options::ENABLE_STRIKETHROUGH | Options::ENABLE_TABLES | Options::ENABLE_TASKLISTS +} + +/// Split `src` into its top-level blocks, in source order. An empty or +/// whitespace-only input gives no blocks; text the parser does not put +/// inside any block (a stray fence marker mid-stream) still comes back, +/// as `Other`, rather than being dropped. +pub fn split_blocks(src: &str) -> Vec { + let mut out: Vec = Vec::new(); + let mut depth = 0usize; + let mut kind = BlockKind::Other; + for (event, range) in Parser::new_ext(src, options()).into_offset_iter() { + match event { + Event::Start(tag) => { + if depth == 0 { + kind = kind_of(&tag); + } + depth += 1; + } + Event::End(_) => { + depth -= 1; + if depth == 0 { + push(&mut out, kind, &src[range]); + } + } + // A top-level event that is not part of any block -- a + // thematic break, a block of raw HTML. Inside one, it is the + // enclosing block's business and this does nothing. + _ => { + if depth == 0 { + push(&mut out, BlockKind::Other, &src[range]); + } + } + } + } + out +} + +fn push(out: &mut Vec, kind: BlockKind, source: &str) { + let source = source.trim_end(); + if source.is_empty() { + return; + } + out.push(Block { + kind, + source: source.to_string(), + }); +} + +/// How many leading blocks of `old` and `new` are identical -- what a +/// caller may keep the laid-out widgets for. See the module doc for why +/// this is a comparison rather than an assumption. +pub fn common_prefix(old: &[Block], new: &[Block]) -> usize { + old.iter().zip(new).take_while(|(a, b)| a == b).count() +} + +#[cfg(test)] +mod tests { + use super::*; + + fn kinds(src: &str) -> Vec { + split_blocks(src).into_iter().map(|b| b.kind).collect() + } + + #[test] + fn a_message_splits_into_its_top_level_blocks() { + let src = "# Title\n\nFirst para.\n\n```rust\nfn main() {}\n```\n\n- a\n- b\n"; + assert_eq!( + kinds(src), + vec![ + BlockKind::Heading, + BlockKind::Paragraph, + BlockKind::Code, + BlockKind::List + ] + ); + let blocks = split_blocks(src); + assert_eq!(blocks[1].source, "First para."); + assert_eq!(blocks[2].source, "```rust\nfn main() {}\n```"); + } + + #[test] + fn blank_input_has_no_blocks() { + assert!(split_blocks("").is_empty()); + assert!(split_blocks(" \n\n ").is_empty()); + } + + /// The property the streaming fast path rests on, in its ordinary + /// shape: a delta landing in the last paragraph must leave every + /// earlier block byte-identical. + #[test] + fn a_delta_into_the_last_paragraph_leaves_earlier_blocks_untouched() { + let before = split_blocks("# Title\n\nFirst para.\n\nSecond par"); + let after = split_blocks("# Title\n\nFirst para.\n\nSecond paragraph now."); + assert_eq!(common_prefix(&before, &after), 2); + assert_eq!(before.len(), 3); + assert_eq!(after.len(), 3); + assert_ne!(before[2], after[2]); + } + + /// A delta that starts a *new* block keeps every old block, including + /// the one that was last -- so the fast path appends rather than + /// replacing. + #[test] + fn a_delta_that_starts_a_new_block_keeps_every_old_one() { + let before = split_blocks("First para.\n\nSecond para."); + let after = split_blocks("First para.\n\nSecond para.\n\nThird"); + assert_eq!(common_prefix(&before, &after), 2); + assert_eq!(after.len(), 3); + } + + /// A code fence arrives one delta at a time and is unterminated for + /// most of its life. It must still be *one* block the whole way, or + /// every delta would re-split the message into a different number of + /// pieces. + #[test] + fn an_unterminated_fence_is_one_block_while_it_streams() { + for src in [ + "Here:\n\n```rust\n", + "Here:\n\n```rust\nfn main() {\n", + "Here:\n\n```rust\nfn main() {\n println!(\"hi\");\n", + ] { + assert_eq!( + kinds(src), + vec![BlockKind::Paragraph, BlockKind::Code], + "{src:?}" + ); + } + } + + /// The half the fast path had no reason to touch, and the reason + /// `common_prefix` is a comparison rather than an assumption: + /// appending can rewrite what came before. `---` under a paragraph + /// turns that paragraph into a setext heading, so the block that was + /// already laid out is not the block it is now. + #[test] + fn appending_can_rewrite_an_earlier_block_and_the_prefix_says_so() { + let before = split_blocks("Not a heading\n\nsecond"); + let after = split_blocks("Not a heading\n\nsecond\n---"); + assert_eq!(before[1].kind, BlockKind::Paragraph); + assert_eq!(after[1].kind, BlockKind::Heading); + assert_eq!( + common_prefix(&before, &after), + 1, + "the rewritten block must not be reported as keepable" + ); + } + + #[test] + fn a_thematic_break_is_its_own_block() { + assert_eq!( + kinds("one\n\n---\n\ntwo"), + vec![BlockKind::Paragraph, BlockKind::Other, BlockKind::Paragraph] + ); + } +} diff --git a/docs/CLIENT_CORE.md b/docs/CLIENT_CORE.md index d2bc7eb..8e364cc 100644 --- a/docs/CLIENT_CORE.md +++ b/docs/CLIENT_CORE.md @@ -191,14 +191,17 @@ does not repeat it again by hand. ## What is not started at all -- **The markdown *block* model beyond syntax spans** -- `highlight/markdown.rs` - colours a `.md` file or fence for the highlighter, but does not build the - block tree (headings, lists, tables, fences as distinct nodes) that a - renderer walks to lay out prose versus code versus a table. - `CodeFence.kt`'s use of `org.intellij.markdown` for that full CommonMark - AST is Compose rendering plumbing, not something to port as-is; a Rust - UI layer will want its own block parser or a crate for it, decided - alongside the framework choice in RUST.md. +- **A full markdown AST.** `markdown_blocks` (2026-09-06) splits a message + into its *top-level* blocks -- heading, paragraph, fence, list, table, + quote -- with each block's own source, which is what a renderer needs to + lay out prose versus code and what lets a streamed delta re-lay out one + block instead of the message (docs/RUST.md's Task B). What it + deliberately does **not** build is the tree below that: nested list + items, table cells, inline spans. Inline styling is still the renderer's + own job per block (`iris/transcript-ui/src/markdown.rs`), and nothing + has needed the rest yet. `CodeFence.kt`'s use of `org.intellij.markdown` + for a full CommonMark AST is Compose rendering plumbing, not something + to port as-is. - **`TranscriptUnits.kt`** (see above) -- deliberately out of scope, since it flattens a row into bounded units for a *specific* lazy-list framework's composition cost, which is a fact about that framework @@ -209,5 +212,5 @@ does not repeat it again by hand. `./run-tests.sh` from the repo root now runs `event-model`, `client-core` and `server` in that order (each `cargo test`, forwarding arguments the same way it always has). From `client-core/` directly: `cargo test` -(109 tests), `cargo clippy --all-targets`, `cargo fmt` -- all clean as of +(119 tests), `cargo clippy --all-targets`, `cargo fmt` -- all clean as of this writing (2026-09-06). diff --git a/docs/IRIS.md b/docs/IRIS.md index 308206e..fd0b9c5 100644 --- a/docs/IRIS.md +++ b/docs/IRIS.md @@ -8,6 +8,36 @@ 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: a transcript row is a column of blocks, and a block is the selection unit + +`transcript-ui`'s row builder used to make **one** `TextEdit` per message. +It makes one per top-level markdown block now -- heading, paragraph, +fenced code, list, table -- in a `Span::down`, because a streamed delta +into a single buffer re-shaped the whole message through parley on every +event. `client_core::markdown_blocks::split_blocks` does the splitting; +`row::RowBlocks::apply_delta` updates the block a delta lands in and +leaves the rest of the message's layout alone. + +**The change to judge, since it is what a reader feels**: +`Selection` is keyed by `SelKey = (RowKey, u32)` -- a row and a block -- +so **a block, not a row, is the unit a selection steps in**. A drag still +runs from a reply into the tool output beneath it and copies as one +thing; what changed is that the row under the finger is filled in block by +block rather than all at once, which is if anything closer to what the +old shortcut in `Selection`'s module doc was apologising for. `register` +takes a `SelKey`; `unregister` still takes a `RowKey` and now drops every +block of it (dropping only the first is how a freed widget gets left in +the map -- the shape docs/REVIEW-2026-09-06.md's finding 1 called out). + +`Selection::locate(ui, render, pos_window)` is new: which block is under a +window position, with that block's own local position and size. The +list-level handler uses it for the pointer-captured half of a drag, +instead of computing a row-local position from `List::extent`. + +`row::build_row` returns `(RowKey, StrongWidget, Option)` -- +the third is the per-block state a caller keeps only for the row a reply +is streaming into, and is `None` for a tool run, which never streams. + ## 2026-09-06: a reported `Size` may not carry `dp`; `Len::fold_dp` **New: `Len::fold_dp(density) -> Len`** -- the same fold `apply_rest` does diff --git a/docs/IRIS_TODO.md b/docs/IRIS_TODO.md index c602040..d5f12fe 100644 --- a/docs/IRIS_TODO.md +++ b/docs/IRIS_TODO.md @@ -643,8 +643,23 @@ do not duplicate it there. ## From the phone, bench v2 (2026-09-06): streaming re-lays out the whole message -- [ ] **Streaming a delta into a long message costs a full text layout of - that message.** Iris's phone report (`docs/bench/iris-phone-v2-2026-09-06.md`): +- [x] **Streaming a delta into a long message costs a full text layout of + that message.** **Done 2026-09-06** -- a row is a column of one + `TextEdit` per markdown block (`client_core::markdown_blocks`, + `row::RowBlocks::apply_delta`), so a delta re-shapes the last block and + keeps every earlier block's layout. A block is the selection unit now + (`Selection`'s `SelKey`); selection across blocks and rows still works, + checked on the emulator with a real long-press drag. Pass condition met + in `a_delta_into_a_long_reply_redraws_the_same_widgets_as_a_short_one`: + a delta into a 100-paragraph reply redraws the same widget count as one + into a one-paragraph reply (30 either way). Emulator stream phase, same + AVD before and after: **p50 61.5 -> 54.5ms, p90 211.7 -> 113.1ms, p99 + 342.6 -> 137.4ms, worst 403.6 -> 143.0ms**, 202 -> 293 frames in the same + 21 seconds. docs/RUST.md's Task B box has the detail and the two dead + ends. **The phone is the measurement that decides it** -- these are + emulator numbers and only the ratio transfers. + + The original entry, for the record: Iris's phone report (`docs/bench/iris-phone-v2-2026-09-06.md`): the stream phase is the one place iris is behind Compose (p50 18.2 ms vs 13.4 ms; p99 level at ~43 ms). `TranscriptScreen::apply` replaces only the last row, but that row is the growing message, and replacing it diff --git a/docs/RUST.md b/docs/RUST.md index 88cee51..7cbce0d 100644 --- a/docs/RUST.md +++ b/docs/RUST.md @@ -107,17 +107,78 @@ Rig fix on the way past: `iris/android-app/run-bench.sh` polled logcat for and printed a report that had never been run. It polls for the report's own first line now. -### Bench, before Task B (emulator, 2026-09-06) +### Task B, closed 2026-09-06: a streamed delta costs one markdown block + +A transcript row was one `TextEdit` holding the whole message, so every +delta re-shaped every paragraph of a long reply through parley -- the one +phase where iris trailed Compose on Iris's phone. A row is a **column of +one `TextEdit` per top-level markdown block** now, and a delta that lands +in the last block is one `set_with_spans` on that block. + +- **`client-core/src/markdown_blocks.rs`** is the split: `split_blocks` + (top-level blocks with their source, via the same `pulldown-cmark` the + renderer parses with, so the two cannot disagree about where a block + starts) and `common_prefix`. Seven tests, including the one that says + the fast path must **compare** rather than assume: appending `---` under + a paragraph turns that paragraph into a heading, so an already + laid-out block is not always still what it was. +- **`iris/transcript-ui/src/row.rs`** builds the column and owns + `RowBlocks::apply_delta`; **`lib.rs`** keeps the *tail* row's blocks + (`TranscriptScreen::tail`) since that is the only row a delta reaches. +- **A block is the selection unit**, not a row: `Selection` is keyed by + `SelKey = (RowKey, u32)`, which compares in reading order at both + levels so every range query in that file is unchanged. The list-level + (pointer-captured) half of a drag resolves the block under the finger + from its drawn box (`Selection::locate`) instead of doing arithmetic + from the row's extent. + +**Pass condition, met**: `a_delta_into_a_long_reply_redraws_the_same_widgets_as_a_short_one` +(`transcript-ui/src/lib.rs`) drives a real `UiRenderState` and asserts the +`Widget::draw` count for one delta into a 100-paragraph (3,000+ character) +reply equals the count for the same delta into a one-paragraph reply. +**30 either way.** It is a real test, not a tautology: it read **630 +against 30** at three points on the way -- once because `Span`'s measure +pass redrew every child, and once because `build_tree` did not seed +`tail`, so the first delta after opening a screen took the rebuild path +with nothing on screen or in `take_rebuilds()` to say so. + +Two things tried and dropped, so the next session does not redo them. +`Painter::measure` (a container asking a clean child for its size instead +of drawing it provisionally) fixed one of the 630s but the test passes +without it once the `tail` seeding is right, so it was removed rather than +kept on speculation. And the emulator's own numbers say the remaining +cost is not in the block split. + +**Verified on the emulator** beyond the counter: the transcript draws its +blocks with their own spacing (heading, prose, fence), and +`ui-trace record --do "holddrag 300 700 700 1000 700 600"` logs +`iris selection: begin at row (3187, 0)` then `extend to row (3187, 1)` +with the highlight crossing from the heading into the code block -- a +selection that spans blocks, which is what the re-key had to keep. + +### Bench, stream phase, before and after Task B (emulator, 2026-09-06) `iris/android-app/build-apk.sh debug --abi x86_64 --features "transcript-screen bench force-gles"` + `run-bench.sh`, this checkout's AVD. Emulator absolutes transfer nothing; the before/after ratio on the same emulator does. - stream: 202 frames over 21.0s - late: 197 (97.5%) - total p50 61.5ms p90 211.7ms p99 342.6ms - worst 403.6ms +Same AVD, same fixture, same build flags, 20 minutes apart. Emulator +absolutes transfer nothing; the ratio does. + + before after + stream: 202 frames over 21.0s stream: 293 frames over 21.0s + late: 197 (97.5%) late: 285 (97.3%) + p50 61.5ms p50 54.5ms (-11%) + p90 211.7ms p90 113.1ms (-47%) + p99 342.6ms p99 137.4ms (-60%) + worst 403.6ms worst 143.0ms (-65%) + +The tail is where the whole-message re-layout lived, and it is where the +change shows: 91 more frames delivered in the same 21 seconds. The p50 +moves least, which is consistent -- a delta into a *short* message never +cost much. **The phone number is Iris's to take**; nothing here is a +statement about her device. - [x] **Merge the `DragGesture` work** -- done 2026-09-06 (merge commit diff --git a/iris/Cargo.lock b/iris/Cargo.lock index f013f3e..df39da9 100644 --- a/iris/Cargo.lock +++ b/iris/Cargo.lock @@ -721,6 +721,7 @@ name = "client-core" version = "0.1.0" dependencies = [ "event-model", + "pulldown-cmark", "serde", "serde_json", "ureq", diff --git a/iris/android-app/Cargo.lock b/iris/android-app/Cargo.lock index 09fea5a..9e5a6c7 100644 --- a/iris/android-app/Cargo.lock +++ b/iris/android-app/Cargo.lock @@ -745,6 +745,7 @@ name = "client-core" version = "0.1.0" dependencies = [ "event-model", + "pulldown-cmark", "serde", "serde_json", "ureq", diff --git a/iris/transcript-ui/src/lib.rs b/iris/transcript-ui/src/lib.rs index 02e869e..1a16dc5 100644 --- a/iris/transcript-ui/src/lib.rs +++ b/iris/transcript-ui/src/lib.rs @@ -66,6 +66,14 @@ 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>, } impl TranscriptScreen { @@ -77,8 +85,39 @@ impl TranscriptScreen { where Rsc::State: FocusHost, { - let (key, widget) = row::build_row(rsc, self.list, self.selection.clone(), row); + let (key, widget, blocks) = row::build_row(rsc, self.list, self.selection.clone(), row); (self.list)(rsc).push_back(ListRow::new(key, widget)); + *self.tail.borrow_mut() = blocks.map(|b| (key, b)); + } + + /// 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. + fn apply_tail_delta(&self, rsc: &mut Rsc, key: RowKey, row: &FoldedRow) -> bool + where + Rsc::State: FocusHost, + { + let FoldedRow::Single(item) = row else { + return false; + }; + let mut tail = self.tail.borrow_mut(); + let Some((tail_key, blocks)) = 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, + ) } /// Apply the effect of one more folded event without rebuilding the @@ -134,17 +173,34 @@ impl TranscriptScreen { } } RowDiff::ReplaceLast { common } => { - // Only the tail row's content changed -- rebuild that one - // row and swap it in place, keeping every row before it - // untouched. + // Only the tail row's content changed. First try the + // delta path: the row is a column of one widget per + // markdown block, so a delta that lands in the last block + // is one `set_with_spans` and the earlier blocks keep + // their layouts (`row::RowBlocks::apply_delta`, and + // docs/DECISIONS.md for why the row is shaped that way). let old_key = row::row_key(&old_rows[common].key()); - let (new_key, widget) = - row::build_row(rsc, self.list, self.selection.clone(), &new_rows[common]); - if new_key != old_key { - self.selection.borrow_mut().unregister(old_key); + let new_key = row::row_key(&new_rows[common].key()); + if new_key == old_key && self.apply_tail_delta(rsc, new_key, &new_rows[common]) { + for row in &new_rows[common + 1..] { + self.push_row(rsc, row); + } + return; } + + // Otherwise rebuild that one row and swap it in place, + // keeping every row before it untouched. `unregister` + // unconditionally, not only when the key changed: a + // rebuild with *fewer* blocks under the same key would + // otherwise leave the extra blocks in `Selection` + // 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 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)); for row in &new_rows[common + 1..] { self.push_row(rsc, row); } @@ -162,6 +218,7 @@ impl TranscriptScreen { self.rebuilds.set(self.rebuilds.get() + 1); self.selection.borrow_mut().clear(); (self.list)(rsc).clear(); + *self.tail.borrow_mut() = None; for row in &new_rows { self.push_row(rsc, row); } @@ -213,9 +270,18 @@ where let selection = Rc::new(RefCell::new(Selection::new())); let list = List::new(Axis::Y).add(rsc); + // The last row's block widgets are kept for the same reason + // `push_row` keeps them: a reply that is *already* streaming when the + // screen is built takes its next delta through `apply`, and a `None` + // here would send that delta down the rebuild path instead -- the + // whole message re-shaped, which is exactly what the per-block column + // exists to avoid, and nothing on screen or in `take_rebuilds` would + // say so. + let mut tail = None; for row in &rows { - let (key, widget) = row::build_row(rsc, list, selection.clone(), row); + let (key, widget, blocks) = row::build_row(rsc, list, selection.clone(), row); list(rsc).push_back(ListRow::new(key, widget)); + tail = blocks.map(|b| (key, b)); } // Wheel/trackpad scrolling -- the same idiom `trait_fns.rs`'s @@ -242,15 +308,13 @@ where list.on( CursorSense::Pressing(CursorButton::Left) | CursorSense::Drop, move |ctx, rsc| { - let pos = ctx.data.pos; - let row = list(rsc).key_at(pos.y).and_then(|key| { - let (top, bottom) = list(rsc).extent(key)?; - Some(( - key, - Vec2::new(pos.x, pos.y - top), - Vec2::new(ctx.data.size.x, bottom - top), - )) - }); + // Which *block* the finger is over, resolved from its + // drawn box rather than from the row's extent -- a row is + // a column of one widget per markdown block now, and the + // block is what `Selection` selects (`SelKey`). + let row = selection + .borrow() + .locate(&*rsc, ctx.data.render, ctx.data.cursor.pos); selection.borrow_mut().drag( rsc, list, @@ -274,6 +338,7 @@ where ( TranscriptScreen { + tail: RefCell::new(tail), list, composer, selection, @@ -507,6 +572,78 @@ mod apply_tests { } } + fn assistant(seq: u64, text: &str) -> TranscriptItem { + TranscriptItem::AssistantMsg { + seq, + text: text.to_string(), + settled: false, + } + } + + /// A reply of `paragraphs` paragraphs, the last one still growing. + fn reply(paragraphs: usize, tail: &str) -> String { + let mut out = String::new(); + for i in 0..paragraphs { + out.push_str(&format!("Paragraph number {i} of a streamed reply.\n\n")); + } + out.push_str(tail); + out + } + + /// `Widget::draw` calls caused by one streamed delta landing in the + /// last paragraph of a reply that already has `paragraphs` of them. + fn draws_for_one_delta(paragraphs: usize) -> u64 { + let mut rsc = TestRsc { + ui: UiData::default(), + events: EventManager::default(), + }; + let old_items = vec![assistant(1, &reply(paragraphs, "and the last one is st"))]; + let new_items = vec![assistant( + 1, + &reply(paragraphs, "and the last one is still going."), + )]; + let (screen, tree) = build_tree( + &mut rsc, + client_core::transcript_fold::group_tool_runs(&old_items), + ); + let mut render = UiRenderState::new(); + render.resize((1080.0, 20000.0)); + render.update(&tree, &mut rsc); + render.take_counters(); + + screen.apply(&mut rsc, &old_items, &new_items); + render.update(&tree, &mut rsc); + assert_eq!(screen.take_rebuilds(), 0, "the delta path must be taken"); + render.take_counters().0 + } + + /// The pass condition for docs/DECISIONS.md's per-block row: a delta + /// costs the **last block**, not the message. A 3,000-character reply + /// has a hundred paragraphs already laid out; redrawing one delta into it + /// must cost exactly what the same delta costs in a one-paragraph + /// reply, or the earlier blocks are being re-shaped. + /// + /// Before the split this was one `TextEdit` for the whole message, so + /// the count was the same *number* of widgets but each redraw + /// re-shaped every paragraph through parley -- which a draw counter + /// cannot see. What it can see is that the count does not *grow* with + /// the message, which it now does not and could not before, since the + /// one widget's own layout was O(message). + #[test] + fn a_delta_into_a_long_reply_redraws_the_same_widgets_as_a_short_one() { + assert!( + reply(100, "").len() > 3_000, + "the long case must actually be a long message" + ); + let short = draws_for_one_delta(1); + let long = draws_for_one_delta(100); + assert_eq!( + short, long, + "a delta into a 100-paragraph reply redrew {long} widgets against {short} for a \ + one-paragraph reply -- the earlier blocks are not being kept" + ); + } + #[test] fn a_row_dropped_by_a_regroup_does_not_outlive_itself_in_selection() { use client_core::transcript_fold::group_tool_runs; @@ -537,7 +674,7 @@ mod apply_tests { let surviving_key = row::row_key(&client_core::transcript_fold::ItemKey::Seq(4)); screen.selection.borrow_mut().begin( &mut rsc, - surviving_key, + (surviving_key, 0), Vec2::ZERO, Vec2::new(10.0, 10.0), ); diff --git a/iris/transcript-ui/src/row.rs b/iris/transcript-ui/src/row.rs index 780b332..4240dea 100644 --- a/iris/transcript-ui/src/row.rs +++ b/iris/transcript-ui/src/row.rs @@ -1,11 +1,18 @@ //! One `iris::widget::list::ListRow` per folded transcript row -//! (`client_core::transcript_fold::TranscriptRow`). Each row's whole text -//! -- headings, paragraphs, inline styling -- goes through `markdown` into -//! **one** `TextEdit`, which is what makes it one thing `Selection` -//! (`selection.rs`) can select and what lets it wrap and scroll as a -//! single buffer, matching RUST.md's "hard to get back" behaviour 2 (rich -//! inline text) and half of behaviour 1 (selectable within a row; across -//! rows is `selection.rs`'s job). +//! (`client_core::transcript_fold::TranscriptRow`). A row is a **column of +//! one `TextEdit` per top-level markdown block** (paragraph, heading, +//! fence, list, table -- `client_core::markdown_blocks`), each rendered +//! with `markdown`'s inline spans, so that RUST.md's "hard to get back" +//! behaviour 2 (rich inline text) still holds within a block and +//! behaviour 1 (selection) runs across blocks and rows alike through +//! `selection.rs`. +//! +//! It was one `TextEdit` for the whole message until 2026-09-06, which +//! meant a streamed delta re-shaped every paragraph of a long reply +//! through parley again -- the stream phase was the one place iris trailed +//! Compose on Iris's phone. [`RowBlocks::apply_delta`] is the other half +//! of the fix; docs/DECISIONS.md's entry has what the alternative shapes +//! were and why this one. //! //! A `TranscriptRow::Tools` (a run of adjacent tool calls, grouped by //! `client_core::transcript_fold::group_tool_runs`) is the row that proves @@ -17,11 +24,18 @@ //! `list.rs`'s module doc describes for `AGENTS.md`'s `holdTopEdge`. use crate::markdown::render_markdown; -use crate::selection::Selection; +use crate::selection::{SelKey, Selection}; +use client_core::markdown_blocks::{Block, BlockKind, common_prefix, split_blocks}; use client_core::transcript_fold::{QuestionCard, TranscriptItem, TranscriptRow as FoldedRow}; use iris::prelude::*; use std::{cell::RefCell, rc::Rc, time::Instant}; +/// The gap drawn between two markdown blocks of one message. A block used +/// to be separated by the blank line `markdown::render_markdown` put in +/// the single buffer; now that each block is its own widget, that spacing +/// has to be the column's. +const BLOCK_GAP_DP: f32 = 8.0; + /// The paragraph size every row's `TextEdit` is built at; markdown headings /// inside a row scale relative to a fixed set of sizes rather than this one /// (`markdown::heading_size`), since a heading is meant to look the same @@ -51,7 +65,7 @@ pub fn row_key(key: &client_core::transcript_fold::ItemKey) -> RowKey { /// The sender label shown above a row's text, and the markdown source to /// render below it. `None` for a system-style note that has no sender. -fn item_content(item: &TranscriptItem) -> (Option<&str>, String) { +pub(crate) fn item_content(item: &TranscriptItem) -> (Option<&str>, String) { match item { TranscriptItem::UserMsg { text, .. } => (Some("You"), text.clone()), TranscriptItem::AssistantMsg { text, .. } => (Some("Claude"), text.clone()), @@ -107,24 +121,53 @@ fn tool_call_markdown(tool: &str, input: &str, output: &str) -> String { out } -/// Build one `TextEdit` from a sender label plus markdown source, register -/// it with `selection` under `key`, and wire the pointer handlers that -/// drive `Selection::drag` -- shared by every row variant below, since a -/// selectable row is always "one TextEdit plus this wiring" regardless of -/// what folded it. `list` is threaded through so that same drag can pan -/// the list instead of selecting, per `Selection::drag`'s own doc. -fn build_text_row( +/// The per-block text widgets of one row, kept by `TranscriptScreen` for +/// the row a reply is streaming into, so a delta can replace the block it +/// lands in instead of re-shaping the whole message +/// (docs/DECISIONS.md, 2026-09-06). Nothing else needs it: a row that is +/// not the tail never changes. +pub struct RowBlocks { + /// What each field was built from, in order -- compared against a + /// fresh split to decide what may be kept. See + /// `client_core::markdown_blocks`' module doc for why this is a + /// comparison and not an assumption. + blocks: Vec, + fields: Vec>, + column: WeakWidget, + /// The sender label the row was built with. A delta that changes it is + /// not a delta into the same message, so it falls back to a rebuild. + sender: Option, +} + +/// Split for display: never empty, so a row with nothing in it yet is +/// still one (empty) text widget rather than no widget at all -- an empty +/// column reports a zero size and the row would vanish from the list. +fn display_blocks(markdown_src: &str) -> Vec { + let blocks = split_blocks(markdown_src); + if blocks.is_empty() { + vec![Block { + kind: BlockKind::Paragraph, + source: markdown_src.to_string(), + }] + } else { + blocks + } +} + +/// One block's own `TextEdit`, registered with `selection` under +/// `(row, block)` and wired to `Selection::drag` -- the block is the +/// selection unit (`selection::SelKey`). +fn build_block_field( rsc: &mut Rsc, list: WeakWidget, selection: Rc>, - key: RowKey, - sender: Option<&str>, - markdown_src: &str, -) -> StrongWidget + key: SelKey, + source: &str, +) -> WeakWidget where Rsc::State: FocusHost, { - let (text, spans) = render_markdown(markdown_src, BASE_SIZE); + let (text, spans) = render_markdown(source, BASE_SIZE); let field = wtext(text) .spans(spans) .editable(EditMode::MultiLine) @@ -137,7 +180,7 @@ where field // `| CursorSense::unclick()` on top of the usual click-or-drag set - // -- this row's own registration only ever needs to see a + // -- this block's own registration only ever needs to see a // gesture's *first* frame (`PressStart`, or a `Pressing` that // missed it -- `DragGesture::handle`'s idle-recovery branch); once // it commits, `DragGesture` takes pointer capture on `list`'s own @@ -161,6 +204,38 @@ where }, ) .add(rsc); + field +} + +/// Build a row from a sender label plus markdown source: a column of one +/// `TextEdit` per top-level markdown block, under the sender's own label. +/// +/// One widget per block rather than one per message is what makes a +/// streamed delta cost the last block instead of the whole reply -- see +/// [`RowBlocks::apply_delta`] for the other half, and +/// `client_core::markdown_blocks` for the split. Selection still runs +/// across the whole transcript; the unit it steps in is a block now rather +/// than a row (`selection::SelKey`). +fn build_text_row( + rsc: &mut Rsc, + list: WeakWidget, + selection: Rc>, + key: RowKey, + sender: Option<&str>, + markdown_src: &str, +) -> (StrongWidget, RowBlocks) +where + Rsc::State: FocusHost, +{ + let blocks = display_blocks(markdown_src); + let mut column = Span::empty(Dir::DOWN).gap(dp(BLOCK_GAP_DP)); + let mut fields = Vec::with_capacity(blocks.len()); + for (i, block) in blocks.iter().enumerate() { + let field = build_block_field(rsc, list, selection.clone(), (key, i as u32), &block.source); + fields.push(field); + column.push(field.width(rest(1)).add_strong(rsc).any()); + } + let column = column.add(rsc); // `.add` (weak), not `.add_strong` -- `header` is about to be embedded // as a child of the `.span(Dir::DOWN)` below, whose own composition is @@ -178,12 +253,91 @@ where None => Span::empty(Dir::DOWN).add(rsc), }; - (header, field.width(rest(1))) + let widget = (header, column.width(rest(1))) .span(Dir::DOWN) .gap(dp(4)) .pad(dp(10)) .add_strong(rsc) - .any() + .any(); + ( + widget, + RowBlocks { + blocks, + fields, + column, + sender: sender.map(str::to_string), + }, + ) +} + +impl RowBlocks { + /// Bring this row up to date with `markdown_src` **without** re-laying + /// out the blocks that did not change, and say whether that was + /// possible. `false` means the caller must rebuild the row the + /// ordinary way: an earlier block was rewritten (markdown allows it -- + /// a trailing `---` turns the paragraph above into a heading), the + /// sender changed, or the message got shorter. + /// + /// This is the whole point of the per-block column: a delta arriving + /// in a 3,000-character reply touches one `set_with_spans` on the last + /// block, so parley re-shapes that block and nothing else. + pub fn apply_delta( + &mut self, + rsc: &mut Rsc, + list: WeakWidget, + selection: Rc>, + key: RowKey, + sender: Option<&str>, + markdown_src: &str, + ) -> bool + where + Rsc::State: FocusHost, + { + if self.sender.as_deref() != sender { + return false; + } + let new_blocks = display_blocks(markdown_src); + let common = common_prefix(&self.blocks, &new_blocks); + // Everything already drawn must either be kept whole (`common == + // len`, a pure append) or be kept except for the last block, which + // is the one a delta lands in. Anything else means an already + // laid-out block is no longer what it was. + if new_blocks.len() < self.blocks.len() || common + 1 < self.blocks.len() { + return false; + } + debug_assert!( + self.fields.len() == self.blocks.len(), + "one field per block: {} fields, {} blocks", + self.fields.len(), + self.blocks.len() + ); + + for (i, block) in new_blocks.iter().enumerate().skip(common) { + let (text, spans) = render_markdown(&block.source, BASE_SIZE); + match self.fields.get(i) { + Some(field) => field.edit(rsc).set_with_spans(&text, spans), + None => { + let field = build_block_field( + rsc, + list, + selection.clone(), + (key, i as u32), + &block.source, + ); + self.fields.push(field); + let child = field.width(rest(1)).add_strong(rsc).any(); + // `get_mut` marks the column dirty, which is what gets + // the new block drawn; its removal half is the row's + // own, since the column owns the child strongly. + if let Some(column) = rsc.ui_mut().widgets.get_mut(&self.column) { + column.push(child); + } + } + } + } + self.blocks = new_blocks; + true + } } fn build_single( @@ -192,7 +346,7 @@ fn build_single( selection: Rc>, key: RowKey, item: &TranscriptItem, -) -> StrongWidget +) -> (StrongWidget, RowBlocks) where Rsc::State: FocusHost, { @@ -249,8 +403,15 @@ where where Rsc::State: FocusHost, { + // 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) + build_text_row(rsc, list, selection, key, Some("Tools"), text).0 } let content = build_content( @@ -299,18 +460,26 @@ pub fn build_row( list: WeakWidget, selection: Rc>, row: &FoldedRow, -) -> (RowKey, StrongWidget) +) -> (RowKey, StrongWidget, Option) where Rsc::State: FocusHost, { match row { FoldedRow::Single(item) => { let key = row_key(&item.key()); - (key, build_single(rsc, list, selection, key, item)) + let (widget, blocks) = build_single(rsc, list, selection, key, item); + (key, widget, Some(blocks)) } FoldedRow::Tools(calls) => { let key = row_key(&calls[0].key()); - (key, build_tools(rsc, list, selection, key, calls.clone())) + // `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, + ) } } } diff --git a/iris/transcript-ui/src/selection.rs b/iris/transcript-ui/src/selection.rs index 0778e4a..100e1d5 100644 --- a/iris/transcript-ui/src/selection.rs +++ b/iris/transcript-ui/src/selection.rs @@ -33,9 +33,17 @@ use iris::prelude::*; use std::{collections::BTreeMap, time::Instant}; +/// What this selects between: a row's `RowKey` and the index of one +/// markdown **block** inside it. A row is a column of one text widget per +/// block since 2026-09-06 (`client_core::markdown_blocks`, and +/// docs/DECISIONS.md for why), so the block, not the row, is the unit -- +/// `(row, block)` compares lexicographically, which is reading order for +/// both levels, so every range query below is unchanged. +pub type SelKey = (RowKey, u32); + pub struct Selection { - rows: BTreeMap>, - anchor: Option<(RowKey, Vec2)>, + rows: BTreeMap>, + anchor: Option<(SelKey, Vec2)>, /// One gesture shared by every row's drag handler -- RUST.md's I5 /// gesture conflict (a row's own `click_or_drag()` and a list-level /// pan wanting the same touch gesture). See `drag` below, and @@ -71,7 +79,7 @@ impl Selection { /// assertion) -- a derived handle that silently outlives what it /// points to; the next caller adding a third row-keyed side table /// should read both. - pub fn register(&mut self, key: RowKey, text: WeakWidget) { + pub fn register(&mut self, key: SelKey, text: WeakWidget) { self.rows.insert(key, text); } @@ -89,9 +97,12 @@ impl Selection { self.anchor = None; } - pub fn unregister(&mut self, key: RowKey) { - self.rows.remove(&key); - if self.anchor.map(|(k, _)| k) == Some(key) { + /// Forgets every block of one row -- a row is registered block by + /// block, so its removal has to take all of them, and taking only the + /// first is how a freed widget would be left behind in this map. + pub fn unregister(&mut self, row: RowKey) { + self.rows.retain(|&(k, _), _| k != row); + if self.anchor.map(|((k, _), _)| k) == Some(row) { self.anchor = None; } } @@ -101,8 +112,8 @@ impl Selection { /// gives `key`'s row a collapsed caret at `pos` -- a plain click that /// never turns into a drag leaves exactly this and nothing else /// selected. - pub fn begin(&mut self, ui: &mut impl UiRsc, key: RowKey, pos: Vec2, size: Vec2) { - let rows: Vec = self.rows.keys().copied().collect(); + pub fn begin(&mut self, ui: &mut impl UiRsc, key: SelKey, pos: Vec2, size: Vec2) { + let rows: Vec = self.rows.keys().copied().collect(); for k in rows { if k != key && let Some(w) = self.rows.get(&k) @@ -118,7 +129,7 @@ impl Selection { /// The drag continues, now over `key`'s row at `pos`. See the module /// doc for the anchor-row shortcut. - pub fn extend(&mut self, ui: &mut impl UiRsc, key: RowKey, pos: Vec2, size: Vec2) { + pub fn extend(&mut self, ui: &mut impl UiRsc, key: SelKey, pos: Vec2, size: Vec2) { let Some((anchor_key, _anchor_pos)) = self.anchor else { return; }; @@ -133,7 +144,7 @@ impl Selection { } else { (key, anchor_key) }; - let in_range: Vec = self.rows.range(lo..=hi).map(|(&k, _)| k).collect(); + let in_range: Vec = self.rows.range(lo..=hi).map(|(&k, _)| k).collect(); for k in &in_range { let Some(w) = self.rows.get(k).copied() else { continue; @@ -149,7 +160,7 @@ impl Selection { w.edit(ui).select_all(); } } - let outside: Vec = self + let outside: Vec = self .rows .keys() .copied() @@ -162,6 +173,34 @@ impl Selection { } } + /// Which registered block is under `pos_window`, with the position + /// and size that block's own `TextEdit` wants (block-local, the way + /// `begin`/`extend` are given them by a block's own pointer handler). + /// + /// For the pointer-captured half of a drag, where the event no longer + /// reaches the widget under the finger and the list-level handler has + /// to say where the finger is. It asks the render state for each + /// block's drawn box rather than doing the arithmetic from the row's + /// extent -- the box is what a hit test resolves against anyway, and + /// it means this and a block's own handler cannot disagree about + /// where a block is. O(blocks loaded), on one frame of a drag. + pub fn locate( + &self, + ui: &impl UiRsc, + render: &UiRenderState, + pos_window: Vec2, + ) -> Option<(SelKey, Vec2, Vec2)> { + for (&key, w) in &self.rows { + let Some(px) = render.window_region(w, ui) else { + continue; + }; + if px.contains(pos_window) { + return Some((key, pos_window - px.top_left, px.size())); + } + } + None + } + /// Whether any row currently has a non-empty selection -- what a fresh /// press consults so `drag` knows whether an early horizontal move is /// "start dragging the selection handle" rather than an ordinary tap. @@ -197,7 +236,7 @@ impl Selection { &mut self, ui: &mut impl UiRsc, list: WeakWidget, - row: Option<(RowKey, Vec2, Vec2)>, + row: Option<(SelKey, Vec2, Vec2)>, pos_window: Vec2, sense: CursorSense, now: Instant, @@ -340,7 +379,7 @@ mod tests { let list = rsc.ui.widgets.add_strong(List::new(Axis::Y)).weak(); let mut sel = Selection::new(); - sel.register(1, field); + sel.register((1, 0), field); assert!(sel.gesture.is_idle()); let render = UiRenderState::new(); @@ -351,7 +390,7 @@ mod tests { sel.drag( &mut rsc, list, - Some((1, Vec2::ZERO, size)), + Some(((1, 0), Vec2::ZERO, size)), Vec2::new(540.0, 700.0), CursorSense::Pressing(CursorButton::Left), now, @@ -365,7 +404,7 @@ mod tests { } #[test] - fn unregister_forgets_the_row_and_clears_a_matching_anchor() { + fn unregister_forgets_every_block_of_the_row_and_clears_a_matching_anchor() { let mut rsc = TestRsc { ui: UiData::default(), }; @@ -379,9 +418,13 @@ mod tests { .weak(); let mut sel = Selection::new(); - sel.register(5, field); - sel.anchor = Some((5, Vec2::ZERO)); - assert_eq!(sel.rows.len(), 1); + // Two blocks of the same row, which is what `unregister` has to + // take together -- removing only the first is how a freed widget + // gets left in this map. + sel.register((5, 0), field); + sel.register((5, 1), field); + sel.anchor = Some(((5, 1), Vec2::ZERO)); + assert_eq!(sel.rows.len(), 2); sel.unregister(5); assert!(sel.rows.is_empty());