Author SHA1 Message Date
iris 2fed8b34b3 Merge branch 'worktree-agent-a6e37a2335f436d08' into rustify 2026-09-06 13:17:22 -04:00
irisandClaude Fable 5.1 1f379e8384 docs/REVIEW-2026-09-06.md: fix all ten review findings; RUST.md/IRIS_TODO.md: DragGesture merge checks
Finding 1 (the real crash): Selection::clear() drops rows and anchor,
called from TranscriptScreen::apply's Rebuild arm right before
List::clear() -- push_row re-registers survivors as it rebuilds each row.
Fixes a WeakWidget outliving the row group_tool_runs regrouped away,
which panicked the next long-press anywhere. New apply_tests test builds
a real TranscriptScreen, forces the regroup, and confirms no panic.

Findings 2-5: debug_assert!s on List::place's slot, List::fling and
FlingCalculator's velocity finiteness, VelocityTracker::add_sample's
chronological order, and FrameReport::mark_phase's non-decreasing
start_index. Finding 7: bench_client.rs's battery_line guard restructured
so the empty check can't be separated from its unwraps by a future edit.
Findings 9/10: new List tests pinning tick_fling's per-tick deceleration
and replace_back's evicted-key cleanup with a different key than the
existing tests use. IRIS.md's replace_back/clear/apply entry gained the
side-table-clearing note the Docs finding asked for.

Also records this pass's DragGesture-merge verification in RUST.md (tap
stays vs swipe doesn't, a real fling keeps moving after release, keyboard
cycles confirmed via on_insets_changed) and annotates the two IRIS_TODO.md
phone-report items it targets.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-06 13:16:16 -04:00
iris bf3479f5c4 client-core: an unasked page is not an empty one, and two guarded invariants
Review of 73251d6's port of TranscriptSource/joinPages.

`TranscriptSource::page` answered `before == 0` with an empty `Vec`, which
is the same value it answers "this conversation has no more history" with.
That is the state the Kotlin keeps apart: `loadOlderPage` returns false at
`oldestSeq == 0` *without* touching `moreHistory`, and returns false on an
empty page *by latching it*. Collapsing the two moved AGENTS.md's paging
bug one layer down rather than fixing it. `page` returns `OlderPage` now --
`Events(vec![])` is the start of the conversation, `NothingLoaded` is not
an answer about the conversation at all.

`join_pages`' `debug_assert!` on seq ordering across the boundary is not a
true invariant: a peer note carries the seq its turn began at, which can be
older than the page it arrived in, so an ordinary transcript would have
panicked a debug build there. Replaced with the one the function exists to
enforce -- no tool id surviving in both halves.

`fetch_transcript_lines` stores `RawValue`'s exact server bytes, so the
"neither source can produce a newline" comment in `SessionCache::append`
now rests on the server's serializer staying compact rather than on a
local normalization. Checked with a `debug_assert!` in `append` and
`store_page` rather than trusted.

Tests for the failure half, which the port had none of: a 500 mid-page, a
cached line this build cannot read, and the `after` bound in the case that
actually carries one (the existing test asserted only the case with no
bound). `cargo fmt`, `cargo clippy --all-targets`, `cargo test` (112) clean
in client-core; `cargo check -p desktop-app` clean.
2026-09-06 13:00:37 -04:00
iris 312455956d Merge remote-tracking branch 'origin/rustify' into worktree-agent-a6e37a2335f436d08 2026-09-06 12:39:22 -04:00
irisandClaude Fable 5.1 73251d6b8b client-core: port TranscriptSource and joinPages page-boundary healing
Closes docs/RUST.md's "client-core prerequisites for P1" box: the
cache-vs-server stitching TranscriptSource.kt does, and the
joinPages/healSplitMessage/adoptRun page-boundary healing
TranscriptItems.kt does, both ported into client-core with no UI
framework dependency.

Neither Kotlin file had a JVM unit test of its own, so the port used the
Kotlin source and AGENTS.md's "things that have bitten" paging incidents
as the spec instead of a test-for-test transcription. Both regressions
get a dedicated test: TranscriptSource::page refuses before == 0 before
touching the cache or the network (loadOlderPage's incident), and
adopt_run now runs on every page join rather than only the one where a
split call was found (the "one run drawn as two" incident).

fetch_transcript_lines (api.rs, additive) pairs each transcript line with
the exact server bytes via serde_json::value::RawValue rather than
re-serializing a parsed Value, so a cached line and a live SSE frame for
the same event agree byte-for-byte -- the fetch_transcript_page other
callers under iris/ depend on is untouched.

client-core: 85 -> 109 tests. cargo test/clippy --all-targets/fmt clean.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-06 12:38:59 -04:00
iris 2e00e71552 docs: Iris's 11:39 phone report on the 02:07 build, four open items 2026-09-06 11:42:21 -04:00
irisandClaude Fable 5.1 f802de94b5 Merge worktree-agent-a754368325fa06839 into rustify: DragGesture, pointer capture, edge-to-edge insets
Generalizes drag arbitration into a default-input DragGesture with
pointer capture and CursorSense::Drop, and opts MainActivity into
edge-to-edge so IME insets are redelivered. See e12c708.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-06 11:38:24 -04:00
iris 9717d1c4b0 docs/RUST.md: 2026-09-06 orchestrator plan for the P0 defects and the P1 prerequisites 2026-09-06 11:37:32 -04:00
iris 9458f443ad Merge remote-tracking branch 'origin/rustify' into worktree-agent-a754368325fa06839 2026-09-06 02:10:55 -04:00
irisandClaude Fable 5.1 e12c708246 iris: generalize drag arbitration into a default-input DragGesture, with pointer capture and Drop
Iris asked (2026-09-06) that dragging be part of iris's default input
system rather than duplicated per app: "anything that provides good
performance and can be generalized well is part of iris rather than the
app." DragArbiter and VelocityTracker (both already in iris::sense) are
now bundled into a new DragGesture, which also takes exclusive pointer
capture (UiRenderState::capture_pointer/release_pointer/captured_pointer)
the moment a gesture commits to panning or selecting, and delivers a new
CursorSense::Drop -- not PressEnd -- to the captured widget when the
button lifts, wherever on screen that happens to be.

This directly targets the phone bench's "finger flings do nothing":
per-widget hit testing silently drops a gesture the instant the pointer
moves off every registered region, which a fast pan/fling does routinely
(crossing several virtualised rows, or ending off the loaded content
entirely) -- so PressEnd, and the velocity/fling-start decision hanging
off it, was frequently never delivered at all. Capture targets List's own
stable id (List::key_at resolves the row-under-pointer from its
extents), not a row's, since List retires rows mid-drag as content
scrolls.

transcript-ui::Selection::drag now only decides pan-vs-select from
DragGesture's outcome; row.rs's per-row registration is only ever a
gesture's first frame, with lib.rs registering the List-level
continuation once. New tests: sense_tests.rs's two pointer-capture
regressions, list.rs's replacing_the_last_row_many_times_does_not_leak_primitives
(a P0 stale-primitives diagnostic -- passes, pinning the widget-arena
layer as not the leak). MainActivity.java opts into edge-to-edge
(Window::setDecorFitsSystemWindows(false), API 30+, no new dependency)
so window insets are redelivered on every change including a pure IME
toggle -- the named-but-untried fix for the phone bench's "keyboard:
could not be shown" and the emulator's identical non-confirmation.

cargo fmt/clippy/test clean across the iris workspace.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2026-09-06 02:10:48 -04:00
iris 543f6d92f0 Merge worktree-agent-a9002910a315fe719 into rustify: composing text, tap-vs-swipe focus, composer rebuild, atlas reset 2026-09-06 02:08:12 -04:00
21 changed files with 2432 additions and 121 deletions

No files matched your search

+6 -1
View File
@@ -17,7 +17,12 @@ edition = "2024"
[dependencies]
event-model = { path = "../event-model" }
serde = { version = "1", features = ["derive"] }
serde_json = { version = "1", features = ["float_roundtrip"] }
# "raw_value" is `fetch_transcript_lines`'s reason -- it needs the exact
# bytes the server sent, not this crate's own re-serialization of a parsed
# `Value`, so a cached line and a live SSE frame for the same event agree
# byte-for-byte (see that method's doc). "float_roundtrip" is why they
# agree on a `ts` at all -- see server/Cargo.toml's identical comment.
serde_json = { version = "1", features = ["float_roundtrip", "raw_value"] }
# The blocking HTTP client for the REST calls and the long-lived SSE GETs.
# `server/` already depends on ureq for its own outbound HTTPS (the usage
# poll in usage.rs) and it is rustls-backed like the rest of this project's
+74 -8
View File
@@ -10,6 +10,7 @@
use std::io::Read;
use event_model::SeqEvent;
use serde::Deserialize;
use serde_json::Value;
@@ -116,6 +117,14 @@ impl<T: Transport> ApiClient<T> {
Self { transport }
}
/// The transport underneath, for a caller that needs the raw SSE
/// stream (`event_stream::follow_session_events`) rather than one of
/// this client's typed REST calls -- `transcript_source::TranscriptSource`
/// is the one that does.
pub fn transport(&self) -> &T {
&self.transport
}
fn json_request<R: for<'de> Deserialize<'de>>(
&self,
method: &str,
@@ -266,15 +275,72 @@ impl<T: Transport> ApiClient<T> {
limit: u32,
coalesce: bool,
) -> Result<Vec<Value>, ApiError> {
let mut path = format!("/sessions/{session_id}/transcript?limit={limit}");
if let Some(before) = before {
path.push_str(&format!("&before={before}"));
}
if coalesce {
path.push_str("&coalesce=true");
}
self.json_request("GET", &path, None)
self.json_request(
"GET",
&transcript_path(session_id, before, limit, coalesce, None),
None,
)
}
/// A page of transcript history, each line handed back paired with the
/// exact text it came from, and bounded below by `after` -- the shape
/// `crate::transcript_source::TranscriptSource` needs to store what it
/// fetched in the transcript cache without a second round trip to fetch
/// the raw text separately. Ported from `Api.kt`'s `fetchTranscript`.
///
/// Uses [`serde_json::value::RawValue`] rather than re-serializing a
/// parsed [`Value`], so the stored line is the exact bytes the server
/// sent (key order and float literal included) rather than this
/// crate's own idea of how to write them back out -- the cache and a
/// live SSE frame must agree byte-for-byte on the same event, which is
/// exactly what caught the `serde_json` float-rounding bug this
/// project's `AGENTS.md` records.
pub fn fetch_transcript_lines(
&self,
session_id: &str,
before: Option<u64>,
limit: u32,
coalesce: bool,
after: Option<u64>,
) -> Result<Vec<(String, SeqEvent)>, ApiError> {
let path = transcript_path(session_id, before, limit, coalesce, after);
let raw: Vec<Box<serde_json::value::RawValue>> = self.json_request("GET", &path, None)?;
raw.into_iter()
.map(|value| {
let line = value.get().to_string();
let event: SeqEvent = serde_json::from_str(&line).map_err(|e| ApiError {
message: format!(
"the server sent a transcript line this build couldn't parse: {e}"
),
status: None,
})?;
Ok((line, event))
})
.collect()
}
}
/// The query string shared by [`ApiClient::fetch_transcript_page`] and
/// [`ApiClient::fetch_transcript_lines`], so the two agree on how each
/// parameter is written rather than keeping two copies to drift.
fn transcript_path(
session_id: &str,
before: Option<u64>,
limit: u32,
coalesce: bool,
after: Option<u64>,
) -> String {
let mut path = format!("/sessions/{session_id}/transcript?limit={limit}");
if let Some(before) = before {
path.push_str(&format!("&before={before}"));
}
if coalesce {
path.push_str("&coalesce=true");
}
if let Some(after) = after {
path.push_str(&format!("&after={after}"));
}
path
}
/// The blocking [`Transport`] backed by `ureq`, the same crate `server/`
+1
View File
@@ -11,5 +11,6 @@ pub mod notifications;
pub mod sse;
pub mod transcript_cache;
pub mod transcript_fold;
pub mod transcript_source;
pub use event_model::*;
+14 -2
View File
@@ -361,6 +361,10 @@ impl SessionCache {
{
return Ok(false);
}
debug_assert!(
lines.iter().all(|l| !l.contains('\n')),
"a stored page's lines must each be one line"
);
fs::create_dir_all(&this.dir)?;
let kind = if rows { "rows" } else { "raw" };
let mut content = lines.join("\n");
@@ -389,8 +393,16 @@ impl SessionCache {
return Ok(());
};
// Written as it arrived. A newline inside it would split one
// event into two unreadable halves, but neither source can
// produce one.
// event into two unreadable halves. No source here can produce
// one -- an SSE `data:` field cannot hold a raw newline, and a
// fetched line is one element of a compact JSON array -- but
// that is a fact about the *server's* serializer rather than
// anything this file controls, so it is checked rather than
// trusted.
debug_assert!(
!line.contains('\n'),
"a cached transcript line must be one line: {line}"
);
use std::io::Write;
writer.write_all(line.as_bytes())?;
writer.write_all(b"\n")?;
+323
View File
@@ -294,6 +294,199 @@ fn split_run(tail: &[TranscriptItem], behind: Option<&str>) -> Vec<TranscriptIte
out
}
/// Puts a page of older items in front of the ones already loaded, healing
/// whatever the page boundary cut in two. Ported from `TranscriptItems.kt`'s
/// `joinPages`.
///
/// Two things straddle a boundary: a tool call separated from its result,
/// and a message separated from the rest of itself. Both were one thing
/// before the transcript was cut into pages.
///
/// A boundary lands wherever it lands, and roughly half the time that is
/// between a call and its result. The newer page then holds a `ToolEnd`
/// whose start it never saw, which `fold_event` draws as a row of its own
/// -- correctly, because a call that renders as nothing is indistinguishable
/// from one that never happened. When the older page arrives it brings the
/// real `ToolStart`, and concatenating the two lists left *both*: the same
/// call twice.
///
/// Merged by the call's own id rather than by position, because position is
/// exactly what a page boundary destroys. The older row wins on what a
/// start knows and the newer on what an end knows, which is the only way
/// round that loses nothing.
///
/// The third thing is the *run*, and it is the one the Kotlin original used
/// to miss (AGENTS.md's "things that have bitten"): every page ends up
/// here, but `adopt_run` must run on *every* join, not only the one where a
/// split call was found -- a boundary landing cleanly between two finished
/// calls, which is most of them, would otherwise leave the older page's
/// calls under the run name they were folded with. On screen: one run of
/// tool calls drawn as two groups, with the seam wherever the reader
/// happened to have paged.
pub fn join_pages(earlier: &[TranscriptItem], later: &[TranscriptItem]) -> Vec<TranscriptItem> {
let (older, newer) = heal_split_message(earlier, later);
let started_earlier: std::collections::HashSet<&str> = older
.iter()
.filter_map(TranscriptItem::as_tool_run)
.collect();
// Owned rather than borrowed from `newer`: `kept` below needs to consume `newer` by
// value, and a map borrowing it would keep that alive.
let ended_later: std::collections::HashMap<String, TranscriptItem> = newer
.iter()
.filter_map(|item| item.as_tool_run().map(|id| (id.to_string(), item.clone())))
.filter(|(id, _)| started_earlier.contains(id.as_str()))
.collect();
let healed: Vec<TranscriptItem> = older
.into_iter()
.map(|row| match row {
TranscriptItem::ToolRun {
seq,
id,
run_id,
tool,
input,
asks: row_asks,
images: row_images,
..
} if ended_later.contains_key(id.as_str()) => {
let &TranscriptItem::ToolRun {
ref output,
done,
asks: ref half_asks,
images: ref half_images,
..
} = &ended_later[id.as_str()]
else {
unreachable!("filtered to ToolRun above");
};
TranscriptItem::ToolRun {
seq,
id,
run_id,
tool,
input,
output: output.clone(),
done,
// Kept from both halves: a question or an image can be
// attached to either, depending on which side of the
// boundary its event fell.
asks: row_asks.into_iter().chain(half_asks.clone()).collect(),
images: row_images.into_iter().chain(half_images.clone()).collect(),
}
}
other => other,
})
.collect();
let kept: Vec<TranscriptItem> = newer
.into_iter()
.filter(|item| match item.as_tool_run() {
Some(id) => !ended_later.contains_key(id),
None => true,
})
.collect();
let mut out = adopt_run(&healed, &kept);
out.extend(kept);
// What this function exists to prevent, checked rather than assumed: the same
// call drawn twice, once from the page that saw its start and once from the page
// that saw its end. Not a seq-ordering check -- a peer note is stamped with the
// seq its turn began at, which can be older than the page it arrived in, so the
// two pages' seqs legitimately interleave at the boundary.
debug_assert!(
{
let mut ids: Vec<&str> = out.iter().filter_map(TranscriptItem::as_tool_run).collect();
let before = ids.len();
ids.sort_unstable();
ids.dedup();
ids.len() == before
},
"join_pages left the same tool call in both halves"
);
out
}
/// Rejoins a message the page boundary cut, and hands back the two pages to
/// concatenate. Ported from `TranscriptItems.kt`'s `healSplitMessage`.
///
/// `fold_event` never leaves two assistant messages next to each other
/// inside one page, so two meeting at a join are always the two halves of
/// one reply, and leaving them apart drew a single answer as two with a
/// paragraph break through the middle of a sentence.
///
/// The newer half keeps its identity, for the reason `adopt_run`'s doc
/// gives. It grows by what the older half brings, which is safe here and
/// nowhere else -- the join is at the oldest end of what is loaded, so the
/// growth extends off the top of the screen.
fn heal_split_message(
earlier: &[TranscriptItem],
later: &[TranscriptItem],
) -> (Vec<TranscriptItem>, Vec<TranscriptItem>) {
let (
Some(TranscriptItem::AssistantMsg {
text: head_text, ..
}),
Some(TranscriptItem::AssistantMsg {
seq: tail_seq,
text: tail_text,
settled: tail_settled,
}),
) = (earlier.last(), later.first())
else {
return (earlier.to_vec(), later.to_vec());
};
let merged = TranscriptItem::AssistantMsg {
seq: *tail_seq,
text: format!("{head_text}{tail_text}"),
settled: *tail_settled,
};
let mut newer = vec![merged];
newer.extend(later[1..].iter().cloned());
(earlier[..earlier.len() - 1].to_vec(), newer)
}
/// Hands the older calls at the join the name of the run they are joining.
/// Ported from `TranscriptItems.kt`'s `adoptRun`.
///
/// The two pages were folded separately, so a run split by the boundary
/// came back as two runs with two names. Naming the joined run after the
/// *older* half would be the obvious way round and is wrong: the newer half
/// is the part already on screen, and renaming it is renaming the row the
/// reader is looking at, which is how a list loses its anchor.
fn adopt_run(earlier: &[TranscriptItem], later: &[TranscriptItem]) -> Vec<TranscriptItem> {
let Some(TranscriptItem::ToolRun { run_id, tool, .. }) = later.first() else {
return earlier.to_vec();
};
// A question is in a run of its own on both sides of the join, the same as it would be
// had the two pages been folded as one. Without this the heal would merge a group
// straight through the row the reader was asked something on.
if tool == ASK_USER_QUESTION {
return earlier.to_vec();
}
let joining = run_id.clone();
let tail_len = earlier
.iter()
.rev()
.take_while(|item| matches!(item, TranscriptItem::ToolRun { tool, .. } if tool != ASK_USER_QUESTION))
.count();
if tail_len == 0 {
return earlier.to_vec();
}
let split = earlier.len() - tail_len;
let mut out = earlier[..split].to_vec();
out.extend(earlier[split..].iter().cloned().map(|mut item| {
// `take_while` above already restricted this slice to non-question tool calls;
// this just guards the invariant rather than trusting it silently.
debug_assert!(
matches!(&item, TranscriptItem::ToolRun { tool, .. } if tool != ASK_USER_QUESTION),
"adopt_run must never rename a question's own run"
);
if let TranscriptItem::ToolRun { run_id, .. } = &mut item {
*run_id = joining.clone();
}
item
}));
out
}
/// Folds one transcript event onto `items`, the way `foldEvent` does in
/// `TranscriptItems.kt`. Every wire event has a case; see the module doc
/// for the one difference from the Kotlin original (no `Unknown` fallback
@@ -955,4 +1148,134 @@ mod tests {
let err = fold_page(&values).unwrap_err();
assert!(err.contains("couldn't parse"));
}
fn tool_start(seq: u64, id: &str, tool: &str) -> SeqEvent {
event(
seq,
Event::ToolStart {
id: id.to_string(),
tool: tool.to_string(),
input: serde_json::json!({}),
},
)
}
fn tool_end(seq: u64, id: &str, output: &str) -> SeqEvent {
event(
seq,
Event::ToolEnd {
id: id.to_string(),
output: output.to_string(),
},
)
}
/// AGENTS.md's "things that have bitten": `joinPages` used to run
/// `adoptRun` only on the path where a *split* call was found, so a
/// boundary landing cleanly between two already-finished calls -- most
/// of them -- left the older page's calls under the run name they were
/// folded with, drawing one run of tool calls as two groups. Two
/// finished, unrelated calls (no id in common) must still end up under
/// one run name after the join.
#[test]
fn a_clean_boundary_between_two_finished_runs_is_still_healed_into_one_run() {
let older = fold_all(&[tool_start(1, "a", "Bash"), tool_end(2, "a", "old output")]);
let newer = fold_all(&[tool_start(3, "b", "Bash"), tool_end(4, "b", "new output")]);
let joined = join_pages(&older, &newer);
let run_ids: Vec<_> = joined
.iter()
.map(|item| match item {
TranscriptItem::ToolRun { run_id, .. } => run_id.as_str(),
other => panic!("expected only ToolRun items, got {other:?}"),
})
.collect();
assert_eq!(
run_ids,
vec!["b", "b"],
"the older call must adopt the newer, already-on-screen run's name"
);
}
#[test]
fn a_call_split_across_the_boundary_merges_into_one_row() {
let older = fold_all(&[tool_start(1, "x", "Bash")]);
let newer = fold_all(&[tool_end(2, "x", "the result")]);
let joined = join_pages(&older, &newer);
assert_eq!(
joined,
vec![TranscriptItem::ToolRun {
seq: 1,
id: "x".to_string(),
run_id: "x".to_string(),
tool: "Bash".to_string(),
input: "{}".to_string(),
output: "the result".to_string(),
done: true,
asks: Vec::new(),
images: Vec::new(),
}],
"the older half's tool/input and the newer half's output/done must both survive"
);
}
#[test]
fn a_message_split_across_the_boundary_is_rejoined_with_the_newer_halfs_identity() {
let older = vec![TranscriptItem::AssistantMsg {
seq: 1,
text: "Hel".to_string(),
settled: false,
}];
let newer = vec![
TranscriptItem::AssistantMsg {
seq: 2,
text: "lo".to_string(),
settled: true,
},
TranscriptItem::UserMsg {
seq: 3,
text: "next".to_string(),
attachments: Vec::new(),
},
];
let joined = join_pages(&older, &newer);
assert_eq!(
joined,
vec![
TranscriptItem::AssistantMsg {
seq: 2,
text: "Hello".to_string(),
settled: true,
},
TranscriptItem::UserMsg {
seq: 3,
text: "next".to_string(),
attachments: Vec::new(),
},
]
);
}
/// A question is in a run of its own on both sides of a join -- healing
/// must never rename the run of calls the reader was asked something
/// on, the same rule `splitRun` enforces for a live turn boundary.
#[test]
fn adopt_run_never_renames_into_a_question_row() {
let older = fold_all(&[tool_start(1, "a", "Bash"), tool_end(2, "a", "done")]);
let newer = vec![TranscriptItem::ToolRun {
seq: 3,
id: "q".to_string(),
run_id: "q".to_string(),
tool: ASK_USER_QUESTION.to_string(),
input: "{}".to_string(),
output: String::new(),
done: false,
asks: Vec::new(),
images: Vec::new(),
}];
let joined = join_pages(&older, &newer);
match &joined[0] {
TranscriptItem::ToolRun { run_id, .. } => assert_eq!(run_id, "a"),
other => panic!("expected a ToolRun, got {other:?}"),
}
}
}
+588
View File
@@ -0,0 +1,588 @@
//! Where a session screen gets a transcript from: this phone's copy first,
//! the server for the rest. Ported from `app/.../TranscriptSource.kt`; see
//! `docs/TRANSCRIPT_CACHE.md` for the design this implements and
//! `docs/CLIENT_CORE.md` for how this file corresponds to the Kotlin.
//!
//! One seam rather than a cache the screen has to remember to consult.
//! Everything fetched before is asked of this, and everything the server
//! sends is written into the cache on the way past, so a caller never
//! learns which side answered. The one rule worth keeping in mind: the
//! cache is never load-bearing. Every read here has a network path beside
//! it producing the same result.
//!
//! **Not ported**: `EventStream.kt`'s reconnect-with-backoff loop and the
//! ability to close a live stream from another thread. Both are wall-clock
//! and thread-lifetime concerns that belong to whatever runtime the caller
//! embeds this crate in (a Tokio task, an iris timer, a Kotlin coroutine
//! scope) rather than to this pure logic -- `follow` below is the same
//! decorator shape `iris/desktop-app/src/app.rs` and
//! `iris/android-app/src/transcript_client.rs` already hand-wrote around
//! `event_stream::follow_session_events`, just with the cache write built
//! in so a future caller does not have to repeat it a third time.
use event_model::SeqEvent;
use crate::api::{ApiClient, ApiError, Transport};
use crate::event_stream::{self, StreamItem};
use crate::transcript_cache::SessionCache;
/// How many events a session screen opens with, cached or fetched.
///
/// The server's own default page size, named here because the cached
/// opening has to be the same size as the fetched one -- a reader must not
/// get a shorter first screen for having been here before (`OPENING_WINDOW`
/// in the Kotlin original).
pub const OPENING_WINDOW: u32 = 80;
/// A transcript-line parse failure, told apart from [`ApiError`] so a
/// caller can tell "the server is unreachable" from "the server (or this
/// phone's own disk) sent something this build cannot read" -- the two
/// mean different things to a reader (retry, versus a build that is
/// behind).
#[derive(Debug, Clone)]
pub struct ParseError(pub String);
impl std::fmt::Display for ParseError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str(&self.0)
}
}
impl std::error::Error for ParseError {}
/// Either half of what can go wrong asking for a page: the network, or a
/// line neither the cache's nor the server's copy of `parseSeqEvent` could
/// read.
#[derive(Debug, Clone)]
pub enum PageError {
Api(ApiError),
Parse(ParseError),
}
impl From<ApiError> for PageError {
fn from(e: ApiError) -> Self {
Self::Api(e)
}
}
impl From<ParseError> for PageError {
fn from(e: ParseError) -> Self {
Self::Parse(e)
}
}
/// What [`TranscriptSource::page`] found, kept as two states rather than
/// one possibly-empty list.
///
/// The difference is the whole of AGENTS.md's `loadOlderPage` incident: an
/// empty [`Self::Events`] means "this conversation has no more history",
/// which a caller is meant to latch, and [`Self::NothingLoaded`] means the
/// question could not be asked yet, which it must not. Collapsing the two
/// into an empty `Vec` puts the bug back, because the caller cannot tell
/// them apart -- and `unwrap_or_default()` on an `Option` would do the
/// same silently.
#[derive(Debug, Clone, PartialEq)]
pub enum OlderPage {
/// The events before the cursor, oldest first. Empty means the start
/// of the conversation has been reached.
Events(Vec<SeqEvent>),
/// Nothing is loaded, so there was no cursor to page back from
/// (`before == 0`). Not an answer about the conversation at all.
NothingLoaded,
}
fn parse_line(line: &str) -> Result<SeqEvent, ParseError> {
serde_json::from_str(line).map_err(|e| ParseError(format!("{e}")))
}
/// This phone's copy of one session's transcript, plus the server it
/// falls back to. Ported from the Kotlin `TranscriptSource` class.
pub struct TranscriptSource<T: Transport> {
api: ApiClient<T>,
session_id: String,
pub cache: SessionCache,
}
impl<T: Transport> TranscriptSource<T> {
pub fn new(api: ApiClient<T>, session_id: impl Into<String>, cache: SessionCache) -> Self {
Self {
api,
session_id: session_id.into(),
cache,
}
}
/// The cached opening window, or `None` when there is nothing usable
/// to draw.
///
/// Meant to be drawn *before* [`Self::probe`] returns, which is the
/// whole point of the feature: the rows are on screen while the check
/// that they are still the server's rows is in flight, and a failed
/// check replaces them exactly as a reset does.
pub fn cached_opening(&self, limit: usize) -> Option<Vec<SeqEvent>> {
self.cache.tail()?;
let lines = self.cache.newest(limit);
if lines.is_empty() {
return None;
}
match lines.iter().map(|l| parse_line(l)).collect() {
Ok(events) => Some(events),
// A line this build cannot read at all, which the cache's own checks cannot
// see: it reads a seq off a line, not an event. Nothing to serve, so a cold
// open.
Err(ParseError(_)) => {
self.cache.purge();
None
}
}
}
/// Whether the server's event at the cached cursor is still the cached
/// one.
///
/// A caller must not resume a live stream from a cached seq unless it
/// is the same conversation: a transcript is append-only in ordinary
/// use, but the file backing it can be replaced or truncated (a
/// sandbox re-seeded with the same ids, a backup restored, a session
/// re-imported), and the server's catch-up on such a file would hand
/// this phone a continuation of a *different* conversation, spliced
/// onto the cached one with no seam. Caught with one request of a few
/// hundred bytes.
///
/// `Ok(false)` purges the cache and means "open cold". `Err` is the
/// server not being askable, which is neither: the cached rows stay
/// on screen and the caller tries again on its own reconnect schedule.
///
/// What this cannot see is a line changed in the middle of the file
/// with the tail intact -- that is what a full reload is for.
pub fn probe(&self) -> Result<bool, ApiError> {
let Some(tail) = self.cache.tail() else {
return Ok(false);
};
// `before = seq + 1` is the newest event with seq <= the cursor, which is the
// event *at* the cursor when the server still has one there.
let page = self.api.fetch_transcript_lines(
&self.session_id,
Some(tail.seq + 1),
1,
false,
None,
)?;
let matches = page.len() == 1
&& parse_line(&tail.line)
.map(|cached| cached == page[0].1)
.unwrap_or(false);
if !matches {
self.cache.purge();
}
Ok(matches)
}
/// Today's opening fetch, kept as the start of the live run. Only
/// called when the cache has nothing to open with, or when
/// [`Self::probe`] said what it had was not the server's.
pub fn fetch_opening(&self) -> Result<Vec<SeqEvent>, ApiError> {
let page =
self.api
.fetch_transcript_lines(&self.session_id, None, OPENING_WINDOW, false, None)?;
for (line, event) in &page {
self.cache.append(line, event.seq);
}
self.cache.flush();
Ok(page.into_iter().map(|(_, event)| event).collect())
}
/// The page before `before`: from the cache when it holds it,
/// otherwise from the server bounded by what the cache already has.
///
/// The server bound (`after`) is what keeps the cache worth having. A
/// coalesced page reaches back as far as its row count takes it -- a
/// single reply is hundreds of lines -- so a page fetched after the
/// reader has been away could run straight past the cached run and
/// overlap it, and an overlapping page cannot be stored. Told where
/// this phone's copy starts, the server stops there instead.
///
/// `before == 0` answers [`OlderPage::NothingLoaded`] without asking
/// the cache or the server anything -- see AGENTS.md's "things that
/// have bitten": there is no event before the first one, so the
/// request is not a harmless no-op, and its empty answer is
/// indistinguishable from having reached the start of history.
/// Guarded here rather than left to every caller, because it is a fact
/// about the question, not about who is asking it.
pub fn page(&self, before: u64, limit: u32, coalesce: bool) -> Result<OlderPage, PageError> {
if before == 0 {
return Ok(OlderPage::NothingLoaded);
}
if let Some(lines) = self.cache.page(before, limit as usize, coalesce) {
let events: Vec<SeqEvent> = lines
.iter()
.map(|l| parse_line(l).map_err(PageError::from))
.collect::<Result<_, _>>()?;
return Ok(OlderPage::Events(events));
}
let after = self.cache.covered_up_to(before).map(|v| v - 1);
let page = self.api.fetch_transcript_lines(
&self.session_id,
Some(before),
limit,
coalesce,
after,
)?;
if let Some((_, first_event)) = page.first() {
// `before` rather than the newest line's seq: a coalesced page covers
// everything up to the cursor it was asked with, and nothing in its lines
// says so.
let lines: Vec<String> = page.iter().map(|(line, _)| line.clone()).collect();
self.cache
.store_page(&lines, first_event.seq, before, coalesce);
}
Ok(OlderPage::Events(
page.into_iter().map(|(_, event)| event).collect(),
))
}
/// [`event_stream::follow_session_events`], with every frame written to
/// the cache before `on_item` sees it.
///
/// Before, so that an event held back for a reader who is scrolled
/// away is already on disk -- what the cache holds is what the server
/// sent, not what a screen has got round to drawing. Flushed on each
/// status change, which is a turn's boundary and the granularity a
/// crash may as well lose, and once more when the stream ends.
pub fn follow(
&self,
after: u64,
mut on_item: impl FnMut(StreamItem) -> bool,
) -> Result<(), ApiError> {
let cache = &self.cache;
let result = event_stream::follow_session_events(
self.api.transport(),
&self.session_id,
after,
|item| {
if let StreamItem::Event { raw, event } = &item {
cache.append(raw, event.seq);
if matches!(event.event, event_model::Event::Status { .. }) {
cache.flush();
}
}
on_item(item)
},
);
cache.flush();
result
}
/// Leaves the cache with everything it was given -- called once a
/// caller is done with this source, mirroring the Kotlin `close`'s
/// final flush (that method's stream cancellation itself is the
/// runtime concern the module doc says is not ported here).
pub fn close(&self) {
self.cache.flush();
}
}
#[cfg(test)]
mod tests {
use super::*;
use crate::api::{Body, RawResponse};
use std::collections::VecDeque;
use std::io::Read;
use std::sync::Mutex;
/// A transport that answers fixed bodies in call order, and records
/// every path it was asked for -- so a test can assert *how many*
/// requests a method made, which is the point for the `before == 0`
/// guard (AGENTS.md's regression: the guard must stop the request
/// before it happens, not merely tolerate the empty answer).
#[derive(Default)]
struct ScriptedTransport {
responses: Mutex<VecDeque<(u16, String)>>,
calls: Mutex<Vec<String>>,
}
impl ScriptedTransport {
fn respond(&self, status: u16, body: impl Into<String>) {
self.responses
.lock()
.unwrap()
.push_back((status, body.into()));
}
fn call_count(&self) -> usize {
self.calls.lock().unwrap().len()
}
}
impl Transport for ScriptedTransport {
fn request(
&self,
_method: &str,
path: &str,
_body: Option<Body>,
) -> Result<RawResponse, ApiError> {
self.calls.lock().unwrap().push(path.to_string());
let (status, body) = self
.responses
.lock()
.unwrap()
.pop_front()
.unwrap_or_else(|| panic!("ScriptedTransport got an unscripted request: {path}"));
Ok(RawResponse {
status,
body: body.into_bytes(),
})
}
fn stream(&self, path: &str) -> Result<Box<dyn Read + Send>, ApiError> {
self.calls.lock().unwrap().push(path.to_string());
let (_, body) = self
.responses
.lock()
.unwrap()
.pop_front()
.unwrap_or_else(|| {
panic!("ScriptedTransport got an unscripted stream request: {path}")
});
Ok(Box::new(std::io::Cursor::new(body.into_bytes())))
}
}
fn source(
transport: ScriptedTransport,
cache_root: &std::path::Path,
) -> TranscriptSource<ScriptedTransport> {
let api = ApiClient::new(transport);
let cache = crate::transcript_cache::TranscriptCache::new(cache_root).session("s1");
TranscriptSource::new(api, "s1", cache)
}
fn status_line(seq: u64) -> String {
format!(r#"{{"seq":{seq},"ts":1.0,"type":"status","state":"idle"}}"#)
}
#[test]
fn a_cold_cache_has_no_opening_and_fetches_from_the_server() {
let dir = tempfile::tempdir().unwrap();
let transport = ScriptedTransport::default();
transport.respond(200, format!("[{}]", status_line(1)));
let source = source(transport, dir.path());
assert_eq!(source.cached_opening(80), None);
let opening = source.fetch_opening().unwrap();
assert_eq!(opening.len(), 1);
assert_eq!(opening[0].seq, 1);
// The fetch wrote through: reopening the same cache now has something to show.
assert!(source.cache.tail().is_some());
}
#[test]
fn probe_matching_the_cached_tail_leaves_the_cache_alone() {
let dir = tempfile::tempdir().unwrap();
let transport = ScriptedTransport::default();
transport.respond(200, format!("[{}]", status_line(1)));
let source = source(transport, dir.path());
source.fetch_opening().unwrap();
let transport2 = ScriptedTransport::default();
transport2.respond(200, format!("[{}]", status_line(1)));
let cache = crate::transcript_cache::TranscriptCache::new(dir.path()).session("s1");
let source2 = TranscriptSource::new(ApiClient::new(transport2), "s1", cache);
assert!(source2.probe().unwrap());
assert!(source2.cache.tail().is_some());
}
#[test]
fn probe_mismatching_the_cached_tail_purges_the_cache() {
let dir = tempfile::tempdir().unwrap();
let transport = ScriptedTransport::default();
transport.respond(200, format!("[{}]", status_line(1)));
let source = source(transport, dir.path());
source.fetch_opening().unwrap();
// The server now answers with a different event at the same seq -- the file
// behind this session was replaced.
let transport2 = ScriptedTransport::default();
let different = r#"{"seq":1,"ts":1.0,"type":"status","state":"running"}"#.to_string();
transport2.respond(200, format!("[{different}]"));
let cache = crate::transcript_cache::TranscriptCache::new(dir.path()).session("s1");
let source2 = TranscriptSource::new(ApiClient::new(transport2), "s1", cache);
assert!(!source2.probe().unwrap());
assert!(source2.cache.tail().is_none());
}
#[test]
fn probe_finding_no_server_leaves_the_cache_untouched() {
let dir = tempfile::tempdir().unwrap();
let transport = ScriptedTransport::default();
transport.respond(200, format!("[{}]", status_line(1)));
let source = source(transport, dir.path());
source.fetch_opening().unwrap();
let transport2 = ScriptedTransport::default();
transport2.respond(500, "server on fire");
let cache = crate::transcript_cache::TranscriptCache::new(dir.path()).session("s1");
let source2 = TranscriptSource::new(ApiClient::new(transport2), "s1", cache);
assert!(source2.probe().is_err());
assert!(
source2.cache.tail().is_some(),
"an unreachable server must not be treated as a mismatch"
);
}
/// The regression this module exists to close: `before == 0` must
/// never reach the network or the cache, because an empty answer there
/// is indistinguishable from "there is genuinely no more history" --
/// AGENTS.md's `loadOlderPage` incident.
#[test]
fn paging_before_the_first_event_makes_no_request_at_all() {
let dir = tempfile::tempdir().unwrap();
let transport = ScriptedTransport::default();
let source = source(transport, dir.path());
assert_eq!(source.page(0, 80, true).unwrap(), OlderPage::NothingLoaded);
assert_eq!(source.api.transport().call_count(), 0);
}
#[test]
fn a_page_already_covered_by_the_cache_never_reaches_the_server() {
let dir = tempfile::tempdir().unwrap();
let transport = ScriptedTransport::default();
transport.respond(200, format!("[{},{}]", status_line(1), status_line(2)));
let source = source(transport, dir.path());
source.fetch_opening().unwrap();
let calls_before = source.api.transport().call_count();
let OlderPage::Events(page) = source.page(2, 10, true).unwrap() else {
panic!("a cursor of 2 is a real question about the conversation");
};
assert_eq!(page.len(), 1);
assert_eq!(page[0].seq, 1);
assert_eq!(
source.api.transport().call_count(),
calls_before,
"a cache hit must not touch the network"
);
}
/// With nothing older cached there is no floor to give the server, so
/// the request carries no `after` at all.
#[test]
fn a_server_page_with_nothing_older_cached_carries_no_bound() {
let dir = tempfile::tempdir().unwrap();
let transport = ScriptedTransport::default();
transport.respond(200, format!("[{}]", status_line(5)));
let source = source(transport, dir.path());
source.fetch_opening().unwrap();
let transport2 = ScriptedTransport::default();
transport2.respond(200, format!("[{}]", status_line(3)));
let cache = crate::transcript_cache::TranscriptCache::new(dir.path()).session("s1");
let source2 = TranscriptSource::new(ApiClient::new(transport2), "s1", cache);
source2.page(5, 10, true).unwrap();
assert_eq!(
source2.api.transport().calls.lock().unwrap()[0],
"/sessions/s1/transcript?limit=10&before=5&coalesce=true"
);
}
/// The half the test above cannot show: when the cache *does* hold an
/// older run, the fetch is floored at its end, or the page would run
/// straight past it and overlap -- which `store_page` then refuses,
/// silently costing the phone the page it just paid for.
#[test]
fn a_server_page_is_floored_at_the_end_of_the_cached_run() {
let dir = tempfile::tempdir().unwrap();
let cache = crate::transcript_cache::TranscriptCache::new(dir.path()).session("s1");
// A stored page covering [3, 6) and two live events above it, so the run this
// phone holds is [3, 8) -- the newest chunk has to be an appended one, or the
// cache reads the directory as damaged and discards it.
let lines: Vec<String> = (3..6).map(status_line).collect();
assert!(cache.store_page(&lines, 3, 6, true));
cache.append(&status_line(6), 6);
cache.append(&status_line(7), 7);
cache.flush();
let transport = ScriptedTransport::default();
transport.respond(200, format!("[{}]", status_line(9)));
let source = TranscriptSource::new(ApiClient::new(transport), "s1", cache);
source.page(10, 10, true).unwrap();
assert_eq!(
source.api.transport().calls.lock().unwrap()[0],
"/sessions/s1/transcript?limit=10&before=10&coalesce=true&after=7",
"the fetch must stop one seq below where this phone's copy ends"
);
}
/// A page the server could not answer is an error, never an empty
/// page: the caller would read the second as "this conversation has no
/// more history" and stop paging for good.
#[test]
fn a_failing_server_page_is_an_error_rather_than_an_empty_one() {
let dir = tempfile::tempdir().unwrap();
let transport = ScriptedTransport::default();
transport.respond(500, "server on fire");
let source = source(transport, dir.path());
assert!(matches!(source.page(9, 10, true), Err(PageError::Api(_)),));
}
/// A cached line this build cannot read is told apart from the network
/// failing, for the same reason: neither is "no more history".
#[test]
fn an_unreadable_cached_page_is_a_parse_error_rather_than_an_empty_one() {
let dir = tempfile::tempdir().unwrap();
let cache = crate::transcript_cache::TranscriptCache::new(dir.path()).session("s1");
cache.store_page(
&[r#"{"seq":3,"but":"not an event"}"#.to_string()],
3,
4,
true,
);
cache.append(&status_line(4), 4);
cache.flush();
let transport = ScriptedTransport::default();
let source = TranscriptSource::new(ApiClient::new(transport), "s1", cache);
assert!(matches!(source.page(4, 10, true), Err(PageError::Parse(_)),));
assert_eq!(
source.api.transport().call_count(),
0,
"a cache hit that cannot be read must not fall through to the server unnoticed"
);
}
#[test]
fn a_bad_cached_opening_line_purges_rather_than_panicking() {
let dir = tempfile::tempdir().unwrap();
let cache = crate::transcript_cache::TranscriptCache::new(dir.path()).session("s1");
cache.append("not json at all", 1);
cache.flush();
let transport = ScriptedTransport::default();
let source = TranscriptSource::new(ApiClient::new(transport), "s1", cache);
assert_eq!(source.cached_opening(80), None);
assert!(
source.cache.tail().is_none(),
"a damaged line purges the cache"
);
}
#[test]
fn follow_writes_events_to_the_cache_before_the_caller_sees_them() {
let dir = tempfile::tempdir().unwrap();
let transport = ScriptedTransport::default();
transport.respond(200, format!("{}\n\n", sse_frame(&status_line(1))));
let source = source(transport, dir.path());
let mut seen = Vec::new();
source
.follow(0, |item| {
if let StreamItem::Event { event, .. } = item {
seen.push(event.seq);
}
true
})
.unwrap();
assert_eq!(seen, vec![1]);
assert_eq!(source.cache.tail().unwrap().seq, 1);
}
fn sse_frame(data: &str) -> String {
format!("data:{data}")
}
}
+84 -17
View File
@@ -25,16 +25,19 @@ next (a Masonry or iris transcript screen, most likely).
| `sse.rs` | `Sse.kt` (the framing half) | Done, new tests (Kotlin had none of its own beyond integration) |
| `api.rs` | `Api.kt` | Partial -- see below |
| `event_stream.rs` | `EventStream.kt` | Done |
| `transcript_fold.rs` | `TranscriptItems.kt`, `ToolRows.kt` | Partial -- see below |
| `transcript_fold.rs` | `TranscriptItems.kt`, `ToolRows.kt` | Done -- see below |
| `config.rs` | `ServerConfig.kt`'s `handleEnrollment` | New, desktop-only so far -- see below |
| *(not started)* | `TranscriptSource.kt` | Not started |
| `transcript_source.rs` | `TranscriptSource.kt` | Done -- see below |
| *(not ported, and may never be)* | `TranscriptUnits.kt` | Out of scope -- see below |
Every file above whose Kotlin counterpart had a JVM unit test (`AnsiTest`,
`HighlighterTest`, `TranscriptCacheTest`) has had every one of those test
cases ported alongside it, plus new tests for the pieces that had none
(`sse.rs`, `api.rs`, `event_stream.rs`, `transcript_fold.rs`). Test count by
crate as of this writing: **85 in `client-core`**, 0 in `event-model` (its
(`sse.rs`, `api.rs`, `event_stream.rs`, `transcript_fold.rs`,
`transcript_source.rs` -- the Kotlin `TranscriptSource.kt`/`TranscriptItems.kt`
had no JVM unit tests of their own, so these were written fresh against the
Kotlin source and AGENTS.md's paging incidents as the spec). Test count by
crate as of this writing: **109 in `client-core`**, 0 in `event-model` (its
types carry no logic of their own to test -- `server/`'s own tests exercise
them via `session::transcript`'s round-trip coverage).
@@ -88,13 +91,27 @@ the full table to work from when one of these is next.
including tool-call/question/image attachment and peer-message placement.
`group_tool_runs` groups adjacent calls into `TranscriptRow::Tools`.
**Not ported:** `TranscriptItems.kt`'s `joinPages` (and its
`healSplitMessage`/`adoptRun` helpers) -- the page-boundary healing that
merges a tool call split across two fetched pages and re-merges a run a
boundary cut through. This matters the moment paging backward through
history is exercised; it is deliberately left rather than rushed, since
it is exactly the kind of boundary logic this project's own "things that
have bitten" section warns reads fine and is wrong at the edges.
`join_pages` (with `heal_split_message` and `adopt_run`, both private) is
now ported too, 2026-09-06 -- the page-boundary healing that merges a tool
call split across two fetched pages, rejoins a message a boundary cut
through, and renames a run of tool calls onto whichever name is already on
screen. Ported with AGENTS.md's "things that have bitten" incidents as the
spec rather than a JVM test file (`TranscriptItems.kt` had none of its
own): `a_clean_boundary_between_two_finished_runs_is_still_healed_into_one_run`
is the regression test for the bug that shipped -- `adopt_run` must run on
*every* join, not only the one where a split call was found, or a boundary
landing cleanly between two already-finished calls (most of them) leaves
one run drawn as two. `a_call_split_across_the_boundary_merges_into_one_row`,
`a_message_split_across_the_boundary_is_rejoined_with_the_newer_halfs_identity`,
and `adopt_run_never_renames_into_a_question_row` cover the other three
edges the Kotlin doc calls out. `join_pages` ends in a `debug_assert!`
that no tool id survives in both halves -- the duplicate row it exists to
prevent, checked rather than assumed. What it deliberately does *not*
assert is seq ordering across the boundary: a peer note carries the seq
its turn began at (`place_peer_note`), which can be older than the page
it arrived in, so the two pages' seqs legitimately interleave there. An
earlier draft asserted it and would have panicked in debug builds on an
ordinary transcript.
**Known gap, and a decision for whoever closes it:** `event_model::Event`
has no `Unknown`/catch-all variant, unlike `Events.kt`'s hand-kept mirror.
@@ -119,12 +136,61 @@ caller-specific (the code rules' "ask for the least you need"). Its only
caller today is `desktop-app`; a future Android build of this crate would
be a second one, not a reason to move the type.
## What `transcript_source.rs` covers, and what it does not
`TranscriptSource<T: Transport>` is the seam a session screen asks for a
page, ported test-for-test against the Kotlin doc rather than a JVM test
file (there wasn't one): `cached_opening`, `probe`, `fetch_opening`,
`page` and `follow`, each matching its Kotlin namesake's contract --
including `probe`'s three-way outcome (matches / cache purged /
unreachable, told apart so a caller never treats "couldn't ask" as "was
wrong") and `page`'s cache-vs-server split bounded by `covered_up_to`.
Two additions beyond a literal port, both load-bearing:
- **`page(before, ..)` refuses `before == 0` before touching the cache or
the network**, answering `OlderPage::NothingLoaded`. This is AGENTS.md's
`loadOlderPage` incident (`before = 0` is "no event before the first
one," indistinguishable from "reached the start of history" if a caller
ever asks it) moved out of the Kotlin screen and into this layer, so
every future caller gets the guard rather than having to remember it.
**The return type is `OlderPage`, not a `Vec`, and that is the guard.**
The Kotlin's two falses are different answers -- `oldestSeq == 0`
returns without touching `moreHistory`, an empty page latches it false
-- so a port that answered both with an empty list would have moved the
bug rather than fixed it, one layer down and out of sight of the screen
that used to hold the check. `OlderPage::Events(vec![])` means the start
of the conversation; `OlderPage::NothingLoaded` is not an answer about
the conversation at all. Reviewed 2026-09-06.
`paging_before_the_first_event_makes_no_request_at_all` asserts zero
transport calls, not just the variant, since a request that happens to
answer empty is exactly what caused the original bug, and
`a_failing_server_page_is_an_error_rather_than_an_empty_one` plus
`an_unreadable_cached_page_is_a_parse_error_rather_than_an_empty_one`
are the same rule for the two ways a page can fail.
- **`fetch_transcript_lines`** (new in `api.rs`) hands back each line
paired with the exact server bytes it came from, via
`serde_json::value::RawValue` rather than re-serializing a parsed
`Value` -- the cache and a live SSE frame for the same event have to
agree byte-for-byte, which is exactly what the `serde_json`
float-rounding bug (AGENTS.md) was about. The existing
`fetch_transcript_page` is untouched (other callers under `iris/`
depend on its signature); the two share a `transcript_path` helper so
the query string is written in one place.
**Not ported:** `EventStream.kt`'s reconnect-with-backoff loop, and
`TranscriptSource.close`'s ability to cancel a live stream from another
thread. Both are wall-clock/thread-lifetime policy that belongs to
whichever runtime embeds this crate (iris's own timers, a Tokio task, a
Kotlin coroutine scope), not to this pure logic -- `follow` is the same
"write to the cache, then hand the frame to the caller" decorator
`iris/desktop-app/src/app.rs` and `iris/android-app/src/transcript_client.rs`
already hand-wrote around `event_stream::follow_session_events` before this
existed; the cache write moved into one shared place so a third caller
does not repeat it again by hand.
## What is not started at all
- **`TranscriptSource.kt`** -- the layer that decides whether a page comes
from the transcript cache or the server, and stitches the two. Needs
`transcript_cache.rs` and `api.rs`'s transcript-page method, both of
which exist now, so this is unblocked whenever picked up.
- **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
@@ -142,5 +208,6 @@ be a second one, not a reason to move the type.
`./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`,
`cargo clippy --all-targets`, `cargo fmt` -- all clean as of this writing.
same way it always has). From `client-core/` directly: `cargo test`
(109 tests), `cargo clippy --all-targets`, `cargo fmt` -- all clean as of
this writing (2026-09-06).
+8 -1
View File
@@ -552,7 +552,14 @@ streamed event" cost RUST.md's P0 box measured (20 events/second against a
new rows appended after it. A row changing *before* the tail (only
`group_tool_runs` retroactively grouping tool calls into a run does
this) falls back to `List::clear` plus a full rebuild, counted in
`TranscriptScreen::take_rebuilds()`. `bench_client.rs`, `transcript_client.rs`
`TranscriptScreen::take_rebuilds()`. **A caller that keeps its own
row-keyed side table alongside `List` (`Selection`'s `rows:
BTreeMap<RowKey, WeakWidget<TextEdit>>` is the one this crate has) must
clear it in step with `List::clear()`** — the fallback drops every row
`List` was holding, so any side table not cleared the same way is left
pointing at widgets the clear just freed (docs/REVIEW-2026-09-06.md
finding 1, fixed 2026-09-06 by `Selection::clear()`, called from
`apply`'s `Rebuild` arm right before `List::clear()`). `bench_client.rs`, `transcript_client.rs`
and `desktop-app/app.rs` all call this now instead of rebuilding on every
event; only the opening page (and `apply`'s own fallback) still calls
`build_tree`.
+65
View File
@@ -183,6 +183,71 @@ agent takes them without colliding with that pass's `bench_client.rs`/
handling is a follow-up, the same shape `List`'s own touch-drag pan
needed before I3/I5.
## From the phone, 2026-09-06, 11:39 (build delivered 02:07, commit 543f6d9)
Iris's report on the build with the composing-text, tap-vs-swipe and
atlas-reset fixes, with a screenshot, verbatim. Each is open until an
agent ticks it here with the evidence.
- [ ] **"The app definitely does not start with keyboard spacing
correct. This is how it looks without me doing anything initially."**
The screenshot shows the composer bar (the grey band) sitting about
two thirds of the way down a 704x1568 screen, with black below it to
the bottom, and the transcript ending at "Claude / Results" just above
it -- at launch, no keyboard. So the composer's bottom padding, which
the 2026-09-06 rebuild tied to the IME/nav-bar inset, is being fed a
large value at start on the phone. Suspects, in order: the initial
inset delivery on the phone (GrapheneOS, gesture navigation) versus
the emulator; `ime_bottom` now carrying a `1`/`0` boolean through a
field the composer may still read as pixels or dp; a stale value from
before the first `on_insets_changed`. Reproduce with the phone's
screen size and density on the emulator before guessing.
- [ ] **"Swiping still gets caught by the grey bar but keeps working
after I go past it."** Not closeable from the emulator, annotated
2026-09-06 after the `DragGesture` merge. `attr.rs`'s `on_press` never
calls `capture_pointer` and never consumes a `Pressing` frame past
`DRAG_SLOP` (it just stops watching), so once the finger's *current*
position leaves the composer's box and enters the list's, `List`
starts receiving ordinary hit-tested `Pressing` frames there --
`DragArbiter::is_idle()`'s 2026-09-05 recovery (a missed `PressStart`)
picks it up rather than leaving it stuck. What this does **not** do is
what "wherever it began" implies literally: `DragArbiter::press_start`
restarts from the *boundary-crossing* position, not from the original
touch-down inside the composer, so the pan still needs a fresh
`DRAG_SLOP` of travel measured from the boundary rather than from the
start of the gesture -- composer and list are adjacent, non-overlapping
widgets (`lib.rs`'s `(list, composer_bar).span(Dir::DOWN)`), and only
the composer forwarding its own drag to the list would remove that
residual slop entirely, which is more than this pass's merge changes.
RUST.md's merge-pass box has the reasoning in full and an emulator
swipe confirming the composer's own box never moves/resizes during it;
whether the residual slop is still perceptible as "caught" needs Iris's
phone, since the emulator's per-widget boundary is a few dp wide and
easy to cross without noticing on a real screen too.
- [ ] **"Flinging still does not work."** No longer expected to reproduce
after the `DragGesture` merge (`e12c708`, pointer capture +
`CursorSense::Drop`), 2026-09-06. Emulator evidence (RUST.md's
merge-pass box, check (b)): a real `ui-trace` finger swipe followed by
screenshot-hash sampling caught a post-release frame distinct from the
drag's own last frame in one run, and every run showed 28-32
`render()` frames per gesture against an idle baseline of 0 and ~8
expected from the drag alone -- redraw kept being requested well past
the finger lifting, which only happens while a fling is still
animating. Left unticked in spirit until Iris's phone confirms it,
since only she can say whether it *feels* like a fling now; the
emulator's screenshot timing could not always catch the tail of a
fast-settling one visually (same caveat noted in RUST.md).
- [ ] **"Text still disappears if I leave and come back to the app."**
The `GlyphAtlas::clear`/`Textures::reset` fix was verified on the
emulator under `force-gles` only; the phone runs Vulkan. So either the
reset is not reached on the phone's path (a different surface-
lifecycle sequence -- `surface_destroyed`/`surface_created` ordering,
or the renderer not being rebuilt but its textures lost), or the CPU
glyph cache and the GPU atlas still disagree after it. Needs logging
of the renderer lifecycle on the phone build, readable from `adb
logcat` when Iris next runs it, since no emulator here has a Vulkan
adapter under host GPU.
## Build
- [x] **Benchmarks**, not unit tests, run on demand (2026-09-05; a
+214
View File
@@ -0,0 +1,214 @@
# Review: iris changes since 0e46293
Scope: `git diff 0e46293..HEAD -- iris/ client-core/` (58 files, +5224/-226).
Read-only review; no source changed. Ordered likely-bug, then invariant
guards, then rules, then tests/docs.
## Likely bugs
1. **`iris/transcript-ui/src/lib.rs:152-160` (`RowDiff::Rebuild` arm of
`TranscriptScreen::apply`) never unregisters the rows it drops from
`Selection`, so a stale `WeakWidget<TextEdit>` outlives the widget it
points to and the next touch on *any* row panics.**
`Selection::rows: BTreeMap<RowKey, WeakWidget<TextEdit>>` documents its
own contract at `selection.rs:69-71`: "every addition here needs its
removal ... called when `List` evicts the row." The `ReplaceLast` arm
above it honours this (`lib.rs:143-145`, `self.selection.borrow_mut()
.unregister(old_key)` when the key changes). The `Rebuild` arm calls
`(self.list)(rsc).clear()` and rebuilds every row from `new_rows`, but
never touches `self.selection` — any key present in `old_rows` and
*absent* from `new_rows` (exactly what `group_tool_runs` regrouping two
separate tool-call rows into one produces — see `diff_tests::
a_tool_run_closing_and_joining_an_earlier_call_is_a_regroup_fallback`,
which tests the diff decision but not `apply` itself) is left in
`self.rows` pointing at a widget `List::clear()` just freed.
`TextEditable::edit` (`iris/src/widget/text/edit.rs:582-587`) resolves
that handle with `ui.widgets.get_mut(self).unwrap()` — an unconditional
panic on the freed slot. `Selection::begin` (`selection.rs:88-101`)
iterates *every* registered row (`w.edit(ui).deselect()`) on an
ordinary fresh press, so the crash fires on the next tap anywhere in
the transcript after a regroup, not only on a tap targeting the
orphaned row.
Fix: give `Selection` a way to reconcile against the row set that
survived a rebuild (e.g. `Selection::retain(&self, keys: &BTreeSet<RowKey>)`
removing everything else, called from the `Rebuild` arm before
rebuilding), or simplest — call `self.selection.borrow_mut()` cleared
the same way `List::clear()` clears the list, then let the rebuild's
`push_row` calls re-`register` everything as they already do.
## Guarded invariants missing
2. **`iris/src/widget/list.rs:751` (`List::place`) indexes/expects on
`slot` with no assertion that it exists.** `slot_widget` (`:563-575`)
panics via `.expect(...)` for a sentinel with no widget set, and does
an unchecked `&self.items[s as usize]` for a real index — a bare
"index out of bounds" with no context if `place` is ever reached with a
stale slot. Every current caller happens to derive `slot` from
`repair_anchor`/`prev_slot`/`next_slot`, which already check existence,
but that invariant is enforced by convention across three call sites,
not by the function that depends on it. Add
`debug_assert!(self.slot_exists(slot), "place() called with a slot that doesn't exist: {slot:?}");`
at the top of `place`.
3. **`iris/src/widget/list.rs:426` (`List::fling`) and `sense.rs`'s
`FlingCalculator::distance`/`duration`/`position_at` never check that
the incoming velocity is finite.** A `NaN`/`inf` velocity (a
`VelocityTracker::velocity()` divide-by-near-zero span, or a caller
passing a raw device value straight through) propagates through
`deceleration_for`'s `.ln()` silently — the fling either never settles
(`settled_on_schedule` compares against a `NaN` `duration()`, which is
always `false`) or jumps to `NaN` positions with nothing on screen
saying why. Add `debug_assert!(velocity_px_per_s.is_finite())` in
`List::fling` and `FlingCalculator::new`/`distance`.
4. **`iris/src/sense.rs:592-604` (`VelocityTracker::velocity`) has no
assertion that samples are chronological.** `add_sample` trusts its
caller's `Instant` ordering; a caller that samples out of order (a
restored/replayed gesture, a test) would silently produce a negative
`span` handled only by the `span <= 0.0 => 0.0` catch-all, masking the
bug that produced it rather than surfacing it. Add
`debug_assert!(self.samples.back().is_none_or(|&(last, _)| at >= last))`
in `add_sample`.
5. **`iris/core/src/render/frame_report.rs:247-252` (`mark_phase`) has no
assertion that phases are pushed in non-decreasing `start_index`
order.** `phase_stats`'s slicing (`:274`, `idx >= phase.start_index &&
idx < end_index`) silently produces an empty or nonsensical slice for
an out-of-order phase rather than surfacing the misuse — cheap to add
given `self.phases.last()` is already in scope:
`debug_assert!(self.phases.last().is_none_or(|p| self.total_frames >= p.start_index));`
## Rules
6. **Two mechanisms answer "what row selection points at, still valid?"**
`Selection` relies on callers remembering to `unregister` (finding 1);
`List` relies on callers deriving slots only from already-checked
sources (finding 2). Both are the same class of problem — a derived
handle that silently outlives what it points to — solved ad hoc twice
rather than once. Not asking for a shared abstraction here, but the two
should at minimum cross-reference each other's doc comment so the next
caller who adds a third handle-into-`List`-rows type (the code rules'
"a rule that governs a set belongs to the set") finds both existing
examples.
7. **`iris/android-app/src/bench_client.rs:224-225` (`battery_line`)
calls `.min().unwrap()`/`.max().unwrap()` on `samples` guarded three
lines above by `if samples.is_empty()`, which is fine — but the guard
and the two unwraps are two statements apart with a `let mean = ...`
in between reading the same slice; a future edit reordering those
lines loses the guard's protection silently.** Low severity (this is
the bench tool, not the app), but worth a one-line comment tying the
unwraps back to the guard, or restructuring as
`let (Some(min), Some(max)) = (samples.iter().min(), samples.iter().max())`
pattern so the empty case can't be separated from the check by a future
edit.
## Tests
8. **No test exercises `TranscriptScreen::apply`'s `Rebuild` arm through
`Selection`.** `lib.rs`'s `diff_tests` module (`:284-379`) tests only
the pure `diff_rows` decision function, never `apply` itself wired to a
real `Selection`; `selection.rs`'s own tests (`a_missed_press_start_
recovers_on_the_next_pressing_frame`, `unregister_forgets_the_row_and_
clears_a_matching_anchor`) never go through `apply`/`List::clear`
either. This is exactly the gap that let finding 1 through: the two
pieces (`apply`'s fallback, `Selection`'s registration contract) are
each tested in isolation and never together. Add: build a
`TranscriptScreen`, force a `RowDiff::Rebuild` (two adjacent tool-call
rows regrouping, per the existing `diff_tests` case), then call
`selected_text`/simulate a fresh press on a surviving row and assert no
panic.
9. **`iris/src/widget/list.rs`'s fling tests check total distance and the
start/end clamp but not the speed profile in between.**
`fling_moves_the_list_and_then_settles`/`fling_distance_is_positive_
toward_the_end` only assert the fling started, moved in the right
direction, and eventually stopped — none checks that
`tick_fling`'s per-tick delta is *monotonically decreasing* once past
the fling's peak (the property `fling_calculator_tests::position_at_
is_monotonic_and_clamped_past_the_end` already checks one level down,
for `FlingCalculator` alone, but never through `List::tick_fling`'s own
`scroll`/`anchor.offset` accumulation). A regression that made
`tick_fling` apply the *total* distance every tick instead of the
incremental one, for instance, would still pass both existing tests
(final position and direction are unaffected by how the interior ticks
split it up) while being wildly wrong every intermediate frame.
10. **`iris/src/widget/list.rs::replacing_the_last_row_stays_pinned_to_
the_bottom` and its sibling test `replace_back`'s effect on the
displayed row, never that the row it evicted is actually gone from
`heights`/`extents`.** Both tests assert the *new* row's position;
neither asserts `old.key` is absent from `list_ref.heights`/`extents`
after the replace (the "stale primitive" class finding 1 is a
production instance of). A cheap addition: assert
`!list_ref.heights.contains_key(&old.key)` after `replace_back` in the
existing test, since `old.key` is already returned to the test as
`evicted`... (`lib.rs` calls it that way; the `list.rs` test would need
to capture the key from `old` similarly.)
## Docs
No missing `IRIS.md` entry found for a *public* API change in this diff —
`List::fling`/`VelocityTracker`/`FlingCalculator`, `List::
anchor_position_display`, `FrameReport::mark_phase`/`phase_stats`/
`late_at_hz`, `UiRenderNode::new`'s `Result` change, `Len::dp`, and
`List::replace_back`/`clear`/`TranscriptScreen::apply` all have entries.
The `List::replace_back`/`clear`/`TranscriptScreen::apply` entry
(`docs/IRIS.md:526`) predates this review's finding 1 and does not mention
`Selection`'s registration contract at all — once finding 1 is fixed,
that entry should gain a line noting what the fix requires of a caller
that keeps its own row-keyed side table (the same shape `Selection` is),
so the next such table doesn't reproduce the same gap.
## Fixed, 2026-09-06
All ten findings addressed after the `DragGesture` merge (`selection.rs`
was rewritten by that merge, but finding 1's shape and location were
unchanged — `TranscriptScreen::apply`'s `Rebuild` arm, `iris/transcript-ui/
src/lib.rs`).
1. **Fixed.** `Selection::clear()` (`selection.rs`) drops `rows` and
`anchor`, called from `apply`'s `Rebuild` arm right before
`List::clear()` — `push_row` re-`register`s whatever survives as it
rebuilds each row, the "simplest" fix option the finding named.
2. **Fixed.** `debug_assert!(self.slot_exists(slot), ...)` at the top of
`List::place` (`iris/src/widget/list.rs`).
3. **Fixed.** `debug_assert!(velocity_px_per_s.is_finite())` in
`List::fling`, and `debug_assert!(velocity.is_finite())` in
`FlingCalculator::distance`/`duration` (`iris/src/sense.rs`).
`position_at` calls both, so it inherits the guard rather than needing
its own.
4. **Fixed.** `debug_assert!` on chronological sample order in
`VelocityTracker::add_sample` (`iris/src/sense.rs`).
5. **Fixed.** `debug_assert!` on non-decreasing `start_index` in
`FrameReport::mark_phase` (`iris/core/src/render/frame_report.rs`).
6. **Fixed (doc cross-reference only, as asked).** `Selection::register`'s
doc now points at `List::place`'s `slot_exists` assertion and vice
versa isn't needed since finding 2's fix already cites this file in
its own comment; both are grep-able on "docs/REVIEW-2026-09-06.md" and
on each other's type names.
7. **Fixed.** `bench_client.rs::battery_line` restructured to
`let (Some(min), Some(max)) = (samples.iter().min(), samples.iter().max())`,
so the empty-guard and the two lookups can no longer be separated by a
future edit.
8. **Fixed.** `transcript-ui`'s new `apply_tests::
a_row_dropped_by_a_regroup_does_not_outlive_itself_in_selection`
(`lib.rs`) builds a real `TranscriptScreen`, forces the same regroup
shape `diff_tests` already covers at the pure-diff level, calls `apply`,
and then `Selection::begin` on a surviving row — which panicked before
fix 1, resolving a `WeakWidget` `List::clear()` had just freed.
9. **Fixed.** `list.rs`'s new `tick_fling_applies_shrinking_incremental_
deltas` flings toward the end from `jump_to_start` and asserts each
tick's `extents[&0]` delta is no larger than the previous one — would
fail against a `tick_fling` that applied the total spline distance
every tick instead of the incremental slice, which the two pre-existing
fling tests cannot catch.
10. **Fixed.** `list.rs`'s new `replace_back_forgets_the_evicted_keys_own_
height` replaces row 4 with a row keyed `100` (the two existing
`replace_back` tests always reuse the same key, so neither actually
exercises the removal) and asserts `heights` no longer contains the
evicted key.
Docs: `docs/IRIS.md`'s 2026-09-05 `List::replace_back`/`clear`/
`TranscriptScreen::apply` entry now has a line on what the fix requires of
a caller with its own row-keyed side table, naming `Selection` as the
example and dating the fix.
Verification run alongside the rest of this pass's checks: `cargo fmt
--all`, `cargo clippy --workspace --all-targets`, `cargo test --workspace`
from `iris/` — see docs/RUST.md's plan box for the pass/fail and any
caveats from this same session.
+173
View File
@@ -34,6 +34,179 @@ the emulator, not by Mesa" and "the present mode was not the cause" are
worth as much as the successes, because they are what stops the next
session spending an afternoon on them again.
## Where things stand (2026-09-06, orchestrator plan)
Written by the design agent on picking the branch up after a `/clear`, so
the next session can resume from here. P0 is delivered and Iris's phone
report v2 is in (`docs/bench/iris-phone-v2-2026-09-06.md`); **P1 stays
gated on her verdict**, so this pass works the P0 defects and the pure
prerequisites in this order. Each item is ticked here by the agent that
closes it.
- [x] **Merge the `DragGesture` work** -- done 2026-09-06 (merge commit
`f802de9`, `git merge --no-ff worktree-agent-a754368325fa06839`,
clean, no conflicts across the 8 files `e12c708` touched). Targets
two of the four bench-v2 defects: finger flings dropped by
per-widget hit testing (pointer capture + `CursorSense::Drop`), and
IME insets never redelivered (`MainActivity.java` edge-to-edge).
**Tap-vs-swipe/`DragGesture` overlap, reasoned through**: `attr.rs`'s
`on_press` (composer focus) and `sense.rs`'s `DragArbiter`/
`DragGesture` (list pan-vs-select) do not share a mechanism, but
they don't need to -- `on_press` never calls `capture_pointer`, so
it only ever sees an ordinary per-frame hit-tested `Pressing`/
`PressEnd` (`run_sensors`' `region.contains(cursor.pos)` check,
unaffected by capture unless *this* widget requested it), the same
as before `DragGesture` existed. The two only interact where a
gesture starts on the composer and travels into the list's region;
`run_sensors` already delivers `Pressing` to whichever widget's
*current* position contains the pointer, so `List` starts getting
frames the instant the finger crosses the boundary -- with no
`PressStart` of its own, which is exactly what `DragArbiter::
is_idle()`'s 2026-09-05 recovery branch exists for. No consolidation
needed; `DRAG_SLOP` is already the one shared constant (`attr.rs`
imports it from `sense.rs`, not a second copy).
**Checks, 2026-09-06 merge pass**: `cargo fmt --all` clean;
`cargo clippy -p iris -p iris-core -p transcript-ui --all-targets`
and the same for `-p desktop-app -p tabs-ui`, zero warnings beyond
the pre-existing external-crate future-incompat notice
(naga/wgpu/wgpu-core/wgpu-hal/winit); `cargo test --lib -p iris -p
iris-core -p transcript-ui` and `-p desktop-app -p tabs-ui`, 97
passed/0 failed, including the review-fix tests below.
`cargo test --workspace`/`cargo clippy --workspace --all-targets`
(the full-workspace forms, which also build `iris`'s winit examples)
were abandoned after 40+ minutes each stuck compiling one example
binary with `uptime` reading a load average of 66-78 on this 8-core
VM (3-4 concurrent peer `cargo`/`cargo check` invocations the whole
session) -- `ps -o time` on the stuck `rustc` showed 2 seconds of
accumulated CPU time after 38 minutes of wall time, confirming
scheduler starvation rather than a hang. The per-package `--lib`
form above is what actually exercises the changed code and finished
in under 4 minutes warm. `android-app` (`iris-android-app`) is
excluded from the host workspace (`iris/Cargo.toml`, needs the NDK
target) and is covered instead by the APK build below, which
compiles it for `x86_64-linux-android`.
**Emulator checks, 2026-09-06** (this checkout's `ai-app-2` AVD,
`iris/android-app/build-apk.sh debug --abi x86_64 --features
"transcript-screen bench force-gles"` -- plain Vulkan crashed on
this AVD's boot this pass, `wgpu_core::instance: enabled backend
Vulkan has no adapters`, unrelated to this merge and worked around
with `force-gles` the way I5's own box already documents for this
hardware):
- **(a) tap-vs-swipe still holds.** Fresh app launch, `dumpsys
input_method`'s `mInputShown=false` at rest. `ui-trace record
--do "swipe 540 1510 540 700 200"` (a swipe starting on the
composer's own box, read from `ui-trace show -m Message --field
box` as `31,1488..1048,1540`) leaves `mInputShown=false` and the
box unmoved (no keyboard-driven resize). `ui-trace record --do
"tap 540 1510"` on the same field then reads `mInputShown=true`.
Matches `20b1225`'s original result -- the `DragGesture` merge
did not disturb it, confirming the reasoning above.
- **(b) a real finger fling keeps the list moving after release.**
Screenshot-hash sampling (`adb exec-out screencap`, `md5`, since
transcript rows carry no per-row accessibility label yet -- I5's
own leftover -- so `ui-trace show` cannot track them) at ~40-60ms
intervals through and after a fast `swipe 540 1400 540 400 120`
(with room to scroll confirmed by a preceding slow drag) caught
two *distinct* post-release frames in one run (a settle-position
beyond the raw drag's own last frame), and every run showed
28-32 `iris::android::view: render()` log lines per gesture
against an idle baseline of 0 in 1.5s and roughly 8 expected from
a bare 120ms drag's own `Pressing` frames alone -- i.e. redraw
kept being requested well past the finger lifting, which only
happens while `List::tick_fling` is still returning `true`.
Some runs' screenshots showed only the drag's own jump with nothing
further *visibly different*, which is consistent with a real but
small/fast-settling fling (a modest synthetic-touch velocity's
spline tail moves little per frame) rather than absence of one --
the render-count signal did not vary between those runs and the
one with a visible second frame. Recorded as confirmed, with that
caveat, rather than measured to a number; a phone verification
(Iris's own report closes this properly) is still open per
`IRIS_TODO.md`'s item.
- **(c) `on_insets_changed` fires on an IME toggle, with confirmed
cycles.** `run-bench.sh`'s report: `keyboard: shown 4/5, hidden
5/5 (confirmed via on_insets_changed)` -- the "could not be
shown" unknown-state line (`bench_client.rs::run_keyboard_phase`)
did not fire, unlike the pre-`DragGesture` build this same report
format existed for.
Worktrees removed after the checks above: `agent-a754368325fa06839`
(the source branch, its own emulator stopped first via `cd` into
it + `emu down`), `agent-a27094a7db775552a`, `agent-a1ff0294b6c29127e`,
`agent-a9002910a315fe719` -- each confirmed `git rev-list --count
rustify..<branch>` = 0 and no uncommitted changes first; their
branches deleted too. `agent-a16b22e34539b810e` and
`agent-a6e37a2335f436d08` left alone -- both `git worktree list`
`locked` to a live peer agent.
- [x] **Fix `docs/REVIEW-2026-09-06.md`**, done 2026-09-06, after the
merge (finding 1's shape and location in `selection.rs`/`lib.rs`
were unchanged by the merge, which touched `Selection` but not
`apply`'s `Rebuild` arm). All ten findings fixed -- new
`Selection::clear()` for finding 1 (the simplest option the review
named: clear the same way `List::clear()` clears the list, let
`push_row` re-`register` survivors), five `debug_assert!`s
(2-5, plus 7's restructure), and three new tests (8, 9, 10),
confirmed with the `apply_tests::a_row_dropped_by_a_regroup_does_
not_outlive_itself_in_selection` test passing (it exercises exactly
finding 1's shape: build a real `TranscriptScreen`, force the same
regroup `diff_tests` already covers, `apply`, then a surviving
row's `begin` -- panics pre-fix, per the review's own test-8 ask).
`docs/IRIS.md`'s 2026-09-05 entry gained the line the review's
"Docs" section asked for. See `docs/REVIEW-2026-09-06.md`'s own "Fixed, 2026-09-06"
section for the per-finding account. Committed together with the
review file.
- [ ] **Iris's 11:39 phone report on the 02:07 build** (four items,
verbatim in `IRIS_TODO.md`'s "From the phone, 2026-09-06, 11:39"):
composer floating two thirds down the screen at launch with black
below it; a swipe starting on the composer held until the finger
leaves it; no fling (expected, `DragGesture` unmerged); text still
lost on app-switch on the phone despite the emulator-verified
atlas reset. The first and last are the same class as the next
box and go to that agent; the middle two are the merge box's.
- [ ] **Stale primitives and invisible composer text** — the header drawn
twice after a keyboard resize, the `Compacted:` row drawn twice on
Iris's phone, and typed text never appearing (P0 box item 2). All
three sit on the `redraw_updates` targeted-redraw path and may be
one bug; the P0 box says the next step is instrumentation inside
`Span::draw`/`draw_inner` showing where each placement's
primitives actually land on the frame it goes wrong.
`list.rs`'s new `replacing_the_last_row_many_times_does_not_leak_
primitives` test already pins the widget arena as *not* the leak.
- [ ] **Composer touch-drag scroll** for overflowed text — now that
dragging is a default-input `DragGesture`, `Scroll` should get its
touch pan from the same mechanism `List` uses, not a copy.
- [ ] **Streaming re-layout** (IRIS_TODO.md's last section) — after the
above, since they make the stream phase unrepresentative today.
- [x] **client-core prerequisites for P1, in parallel** (pure Rust,
disjoint from `iris/`), closed 2026-09-06: `TranscriptSource`'s
cache-vs-server stitching (new `client-core/src/transcript_source.rs`)
and `joinPages`/`healSplitMessage`/`adoptRun` page-boundary healing
(new functions in `transcript_fold.rs`), per `CLIENT_CORE.md`. Ported
against the Kotlin source and AGENTS.md's paging incidents as the
spec (`TranscriptSource.kt`/`TranscriptItems.kt` had no JVM unit
tests of their own to port test-for-test). `client-core` goes from
85 to 109 tests; `cargo test`/`clippy --all-targets`/`fmt` all clean.
Both AGENTS.md regressions have a dedicated test: `loadOlderPage`'s
`before == 0` guard moved into `TranscriptSource::page` itself
(`paging_before_the_first_event_makes_no_request_at_all` asserts
zero transport calls, not just an empty result), and
`a_clean_boundary_between_two_finished_runs_is_still_healed_into_one_run`
pins `adopt_run` running on *every* join rather than only the
split-call path. One incidental fix needed to port `TranscriptSource`
faithfully: `api.rs` gained `fetch_transcript_lines` (additive, the
existing `fetch_transcript_page` untouched since `iris/` depends on
its signature), which pairs each event with the exact server bytes
it came from via `serde_json::value::RawValue` rather than
re-serializing a parsed `Value` -- needed so the cache and a live SSE
frame agree byte-for-byte, the same class of bug as the
`float_roundtrip` fix. Deliberately not ported: `EventStream.kt`'s
reconnect/backoff and cross-thread stream cancellation, which are
runtime policy for whichever framework embeds this crate, not pure
logic -- see `CLIENT_CORE.md`'s new section for the full account.
- **Then**: redeliver `~/host/bench/iris-bench-arm64.apk` for Iris with
its README saying what changed, and record any choice she should see in
`DECISIONS.md`.
## Where things stand (2026-09-05)
- **Streaming no longer costs a full rebuild** (P0's box, "Streaming no
@@ -31,6 +31,25 @@ public final class MainActivity extends Activity {
setContentView(layout);
view.requestFocus();
// RUST.md's P0 box, defect 4 ("keyboard: could not be shown"):
// `logcat` showed the platform's own IME open/resize happening
// while `setOnApplyWindowInsetsListener` fired only once, at
// attach, and never again for a pure keyboard toggle -- a plain
// (non-edge-to-edge) window is only guaranteed that one initial
// dispatch; `adjustResize` handling the IME entirely by resizing
// the window is not itself a trigger for a fresh one. Opting into
// edge-to-edge (a platform call, API 30+, no new dependency) is
// what makes the system redeliver insets on every change,
// including the ones this activity actually cares about --
// `getSystemWindowInset*` below is unaffected by this (it has
// always reported the raw system-bar/IME overlap regardless of
// who consumes it), so the on-screen bars and the padding Rust
// already derives from those four numbers are unchanged; only the
// callback's firing became reliable.
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) {
getWindow().setDecorFitsSystemWindows(false);
}
view.setOnApplyWindowInsetsListener((v, insets) -> {
int left = insets.getSystemWindowInsetLeft();
int top = insets.getSystemWindowInsetTop();
+9 -2
View File
@@ -221,8 +221,15 @@ fn battery_line(samples: &[i32]) -> String {
return " battery current: unavailable on this device".to_string();
}
let mean = samples.iter().map(|&v| v as i64).sum::<i64>() / samples.len() as i64;
let min = samples.iter().min().unwrap();
let max = samples.iter().max().unwrap();
// `min`/`max` are guarded by the `is_empty` check above, three lines
// up -- pairing the `Option` unwraps with the emptiness check right
// here (rather than two statements apart, with `mean` in between
// reading the same slice) is what keeps a future reorder from
// separating the guard from what it protects (docs/
// REVIEW-2026-09-06.md finding 7).
let (Some(min), Some(max)) = (samples.iter().min(), samples.iter().max()) else {
unreachable!("samples is non-empty, checked above");
};
format!(
" battery current: mean {mean}\u{b5}A over {} samples (min {min}, max {max})",
samples.len()
+9
View File
@@ -245,6 +245,15 @@ impl FrameReport {
/// this once per phase (fling/stream/type/keyboard) so `phase_stats`
/// can slice one whole run's frames by what was happening during each.
pub fn mark_phase(&mut self, name: &str) {
// `phase_stats`'s slicing (`idx >= phase.start_index && idx <
// end_index`) silently produces an empty or nonsensical slice for
// a phase pushed out of order rather than surfacing the misuse
// (docs/REVIEW-2026-09-06.md finding 5).
debug_assert!(
self.phases
.last()
.is_none_or(|p| self.total_frames >= p.start_index)
);
self.phases.push(PhaseMark {
name: name.to_string(),
start_index: self.total_frames,
+44
View File
@@ -20,6 +20,21 @@ pub struct UiRenderState {
resized: bool,
draw_started: HashSet<WidgetId>,
/// The widget currently holding exclusive pointer input, if any --
/// `iris::sense::SensorUi::run_sensors` reads and clears this every
/// call. Interior mutability (a `Mutex`, not a bare `Cell`, since a
/// `CursorData` reaching this through an async `task_on` handler needs
/// `Send`/`Sync`) because `run_sensors` takes `&self` (widgets are
/// dispatched to, not owned, at that layer) and this render state is
/// the one structure both backends (winit, android-view) already hold
/// across frames, the same way `old_root`/`resized` are -- see
/// `iris::sense`'s pointer-capture doc for why a drag needs this: once
/// a gesture has committed to panning or selecting, every later sample
/// of it must reach the same widget even if the finger has moved off
/// whatever hit region first noticed the press. Never held across an
/// await or another lock -- every access here is a single get/set.
captured: std::sync::Mutex<Option<WidgetId>>,
/// `Widget::draw` calls and `Primitives::region_mut` rewrites since the
/// last `take_counters`. LAYOUT.md section 8's pass conditions are
/// stated in terms of these two: an unchanged frame must cost 0 of
@@ -45,6 +60,7 @@ impl UiRenderState {
old_root: None,
resized: false,
draw_started: Default::default(),
captured: Default::default(),
draw_count: 0,
region_mut_count: 0,
mov_count: 0,
@@ -357,6 +373,13 @@ impl UiRenderState {
active.textures.clear();
rsc.ui_mut().textures.free();
if undraw {
// A captured widget that goes away mid-gesture (List's
// virtualisation retiring a row, a rebuild) must not leave
// the pointer permanently captured by an id nothing will
// ever draw again -- `captured`'s own path out.
if *self.captured.lock().unwrap() == Some(id) {
*self.captured.lock().unwrap() = None;
}
// Permanent removal: retire this widget's own move slot
// (the self-ownership ref taken when it was allocated) and
// the up-link ref it held on its parent's slot -- read from
@@ -429,6 +452,27 @@ impl UiRenderState {
self.active.len()
}
/// Give `id` exclusive pointer input from the next `run_sensors` call
/// on -- see `captured`'s field doc. Overwrites any previous capture
/// (a gesture that starts a new one has already decided the old one
/// is over).
pub fn capture_pointer(&self, id: WidgetId) {
*self.captured.lock().unwrap() = Some(id);
}
/// Release exclusive pointer input, if any is held -- called once
/// `run_sensors` has delivered the terminal `Drop` to the capturing
/// widget, or by that widget itself if it decides the gesture is over
/// some other way.
pub fn release_pointer(&self) {
*self.captured.lock().unwrap() = None;
}
/// The widget currently holding exclusive pointer input, if any.
pub fn captured_pointer(&self) -> Option<WidgetId> {
*self.captured.lock().unwrap()
}
pub fn debug(&self, widgets: &Widgets, label: &str) -> impl Iterator<Item = &ActiveData> {
self.active.iter().filter_map(move |(&id, inst)| {
let l = widgets.label(id);
+218
View File
@@ -22,6 +22,14 @@ pub enum CursorSense {
Hovering,
HoverEnd,
Scroll,
/// Delivered exactly once, in place of `PressEnd`, to whichever widget
/// currently holds pointer capture (`UiRenderState::capture_pointer`)
/// when the button lifts -- see `iris::sense`'s pointer-capture doc
/// and `DragGesture`. A widget must register this explicitly (it is
/// never bundled into `click_or_drag`/`unclick`, since most widgets
/// never call `capture_pointer` and have no use for it) to receive it
/// at all; ordinary hit-tested widgets keep seeing `PressEnd`.
Drop,
}
#[derive(Clone)]
@@ -31,6 +39,21 @@ impl Event for CursorSenses {
type Data<'a> = CursorData<'a>;
type State = SensorState;
fn should_run<'a>(&self, data: &Self::Data<'a>) -> Option<Self::Data<'a>> {
// `Drop` is never derived from raw cursor/hover state below (the
// free `should_run`'s own arm for it is only ever asked here,
// never independently true or false against the button) -- it is
// set exclusively by `run_sensors`' pointer-capture branch, which
// has already decided this exact frame is the captured widget's
// terminal event. Matching it by identity, ahead of the general
// derivation, matters because a captured widget's registration
// list very likely also carries `PressEnd` (`unclick()`, for the
// ordinary un-captured case) -- the same button-lift condition
// `PressEnd` matches on, so falling through to the loop below
// would let whichever of the two happens to be registered first
// win, silently swallowing the `Drop` a caller relied on.
if data.sense == CursorSense::Drop {
return self.contains(&CursorSense::Drop).then(|| data.clone());
}
if let Some(sense) = should_run(self, &data.cursor, data.hover) {
let mut data = data.clone();
data.sense = sense;
@@ -177,6 +200,48 @@ impl SensorUi for UiRenderState {
cursor: CursorState,
window_size: Vec2,
) {
// Exclusive pointer capture (`UiRenderState::capture_pointer`,
// `DragGesture`): once some widget has committed to a drag, every
// other widget sees nothing from this pointer at all -- no hover,
// no click, no press -- until it releases. This is what lets a
// fast pan or a selection keep going once the finger has moved
// off whatever hit region first noticed the press (including
// right off the end of the gesture, at `PressEnd`/`Cancel`): a
// per-widget hit test would otherwise silently stop delivering to
// *anyone* the moment the pointer left every registered region,
// which is exactly what used to leave a fling never started (no
// widget ever saw the release). The captured widget keeps getting
// ordinary `Pressing` frames while the button is down and gets
// exactly one `Drop` -- not `PressEnd` -- the frame it lifts,
// which also releases the capture.
if let Some(id) = self.captured_pointer() {
let Some(shape) = self.resolved_region(&id, rsc) else {
self.release_pointer();
return;
};
let region = shape.to_px(window_size);
let button_down = cursor.buttons.select(&CursorButton::Left).is_on();
let sense = if button_down {
CursorSense::Pressing(CursorButton::Left)
} else {
CursorSense::Drop
};
let data = CursorData {
pos: cursor.pos - region.top_left,
size: region.bot_right - region.top_left,
scroll_delta: cursor.scroll_delta,
hover: ActivationState::On,
cursor: cursor.clone(),
sense,
render: self,
};
rsc.run_event::<CursorSense>(id, data, state);
if !button_down {
self.release_pointer();
}
return;
}
// in order to remove this take, need to store active list in UiRenderState somehow
// this would probably be done through a generic parameter that adds yet another rsc /
// state like thing, but local to render state, and is passed to UiRsc events so you can
@@ -266,6 +331,15 @@ pub fn should_run(
CursorSense::Hovering => hover.is_on(),
CursorSense::HoverEnd => hover.is_end(),
CursorSense::Scroll => cursor.scroll_delta != Vec2::ZERO,
// Never derived here -- `Drop` only ever fires through
// `CursorSenses::should_run`'s own special case, ahead of this
// loop, for the one widget `run_sensors`' capture branch is
// delivering it to this frame. If this arm answered from raw
// button state instead, an ordinary hit-tested widget that
// happened to register `Drop` (with no capture involved at
// all) would see it fire on every plain button-up under the
// cursor.
CursorSense::Drop => false,
} {
return Some(*sense);
}
@@ -541,6 +615,138 @@ impl DragArbiter {
}
}
/// What a [`DragGesture`] decided this frame -- [`DragOutcome`] plus the
/// one further state a shared gesture needs: the drag ending, with the
/// released velocity if (and only if) it had committed to panning.
#[derive(Debug, Clone, Copy, PartialEq)]
pub enum GestureOutcome {
Undecided,
/// Same units and sign as [`DragOutcome::Pan`] -- the caller's own
/// convention (`List::scroll`'s, for a transcript) to apply.
Pan(f32),
SelectStart,
SelectExtend,
/// The drag ended -- `PressEnd` or the capture's own terminal `Drop`.
/// `Some(velocity)` only if the gesture had committed to panning
/// (never a tap, a long-press selection, or one still `Undecided`);
/// same units as `Pan`, so a caller hands it to `List::fling` with
/// whatever sign flip it already applies to `Pan`.
Released(Option<f32>),
}
/// Bundles a [`DragArbiter`] and a [`VelocityTracker`] into the one thing
/// most drag-driven widgets need: arbitrate pan-vs-hold, track the pan's
/// velocity, and take pointer capture (`UiRenderState::capture_pointer`)
/// the moment the gesture commits so the rest of it -- including the
/// terminal release -- keeps reaching the same widget even after the
/// finger has moved off whatever hit region first noticed the press. Iris
/// asked for this to live here rather than in `transcript-ui::Selection`
/// (2026-09-06, recorded in `IRIS.md`): "dragging should be part of the
/// default input system ... anything that provides good performance and
/// can be generalized well is part of iris rather than the app." A caller
/// still decides what a committed pan or a completed selection *means*
/// (transcript-ui's pan-vs-select is one call site; a slider or a plain
/// scroll area is another) -- this only owns the *mechanics* every one of
/// them would otherwise duplicate.
pub struct DragGesture {
arbiter: DragArbiter,
velocity: VelocityTracker,
}
impl Default for DragGesture {
fn default() -> Self {
Self::new()
}
}
impl DragGesture {
pub fn new() -> Self {
Self {
arbiter: DragArbiter::new(),
velocity: VelocityTracker::new(),
}
}
/// Whether this gesture has no press in flight -- a thin passthrough
/// to the underlying `DragArbiter::is_idle`, for a caller (a test, a
/// diagnostic) that wants to observe the recovery behaviour `handle`'s
/// idle-recovery branch documents without reaching into a private
/// field.
pub fn is_idle(&self) -> bool {
self.arbiter.is_idle()
}
/// Feed one frame of a gesture through. `id` is the widget iris should
/// give exclusive pointer input to once this gesture commits to
/// panning or selecting -- a stable widget that outlives the gesture
/// (a `List`'s own id, not one of its virtualised rows, which can be
/// retired mid-drag as content scrolls). `render` is `CursorData`'s
/// own field, already in hand at every call site. `already_selected`
/// only matters for the first frame of a gesture (`PressStart`, or the
/// recovery branch below) -- see `DragArbiter::press_start`'s doc.
pub fn handle(
&mut self,
render: &UiRenderState,
id: WidgetId,
sense: CursorSense,
pos_window: Vec2,
now: Instant,
already_selected: bool,
) -> GestureOutcome {
match sense {
CursorSense::PressStart(_) => {
self.velocity.reset();
self.arbiter.press_start(pos_window, now, already_selected);
self.dispatch(render, id, pos_window, now)
}
CursorSense::Drop | CursorSense::PressEnd(_) => {
let released = if self.arbiter.is_panning() {
Some(self.velocity.velocity())
} else {
None
};
self.arbiter.release();
render.release_pointer();
GestureOutcome::Released(released)
}
// See `DragArbiter::update`'s own doc: a `Pressing` frame can
// arrive with no matching `PressStart` if the touch-down
// landed outside whichever hit region first noticed it.
_ if self.arbiter.is_idle() => {
self.velocity.reset();
self.arbiter.press_start(pos_window, now, already_selected);
self.dispatch(render, id, pos_window, now)
}
_ => self.dispatch(render, id, pos_window, now),
}
}
fn dispatch(
&mut self,
render: &UiRenderState,
id: WidgetId,
pos: Vec2,
now: Instant,
) -> GestureOutcome {
match self.arbiter.update(pos, now) {
DragOutcome::Undecided => GestureOutcome::Undecided,
DragOutcome::Pan(dy) => {
render.capture_pointer(id);
self.velocity.add_sample(dy, now);
GestureOutcome::Pan(dy)
}
DragOutcome::SelectStart => {
render.capture_pointer(id);
GestureOutcome::SelectStart
}
DragOutcome::SelectExtend => {
render.capture_pointer(id);
GestureOutcome::SelectExtend
}
}
}
}
/// How far back a [`VelocityTracker`] looks when estimating a fling's
/// initial speed -- Android's own `VelocityTracker` defaults to a similar
/// short window so a gesture's last flick dominates over its slower start.
@@ -575,6 +781,12 @@ impl VelocityTracker {
/// Record one frame's motion. `delta` is this frame's movement since
/// the last sample, not a cumulative position.
pub fn add_sample(&mut self, delta: f32, at: Instant) {
// A caller that samples out of order (a restored/replayed
// gesture, a test) would silently produce a negative `span` in
// `velocity`, handled only by its `span <= 0.0 => 0.0` catch-all
// -- masking the bug that produced it rather than surfacing it
// (docs/REVIEW-2026-09-06.md finding 4).
debug_assert!(self.samples.back().is_none_or(|&(last, _)| at >= last));
self.samples.push_back((at, delta));
while let Some(&(when, _)) = self.samples.front() {
if at.duration_since(when) > VELOCITY_WINDOW {
@@ -750,6 +962,10 @@ impl FlingCalculator {
/// Total signed distance the fling travels before settling, in the
/// same pixel units `velocity` was given in.
pub fn distance(&self, velocity: f32) -> f32 {
// See `List::fling`'s matching assertion -- a non-finite velocity
// here silently produces a NaN distance rather than surfacing the
// bug that produced it (docs/REVIEW-2026-09-06.md finding 3).
debug_assert!(velocity.is_finite());
if velocity == 0.0 {
return 0.0;
}
@@ -762,6 +978,8 @@ impl FlingCalculator {
/// How long the fling takes to settle.
pub fn duration(&self, velocity: f32) -> Duration {
// See `distance`'s matching assertion, above.
debug_assert!(velocity.is_finite());
if velocity == 0.0 {
return Duration::ZERO;
}
+124
View File
@@ -122,3 +122,127 @@ fn a_button_over_a_list_scrolls_the_list_and_still_clicks() {
"the button on top must still receive an actual click"
);
}
/// The bug behind "finger flings do nothing" (RUST.md's P0 phone report,
/// defect 2): a fast gesture's `PressEnd` can land at a screen position
/// nothing is registered at -- past the edge of whatever widget noticed
/// the press, in a gap, or off the loaded content entirely. Before pointer
/// capture, `run_sensors`' hit test simply delivered nothing that frame,
/// so a widget mid-drag never saw its release and never got a chance to
/// start a fling. `UiRenderState::capture_pointer`/`DragGesture` fix this
/// by giving the drag's widget every frame regardless of where the
/// pointer is, including the terminal `Drop` in place of `PressEnd`.
#[test]
fn a_release_outside_every_hit_region_still_reaches_the_captured_widget() {
let mut rsc = SenseRsc {
ui: UiData::default(),
events: EventManager::default(),
};
// A small draggable widget in the corner -- the release below lands
// far outside it, exactly the "moved off the hit region" case.
let draggable = rsc.ui.widgets.add_strong(Rect::new(UiColor::WHITE)).any();
let draggable_weak = draggable.weak();
let dropped = Rc::new(Cell::new(false));
{
let dropped = dropped.clone();
rsc.register_event(
draggable_weak,
CursorSense::click_or_drag() | CursorSense::unclick() | CursorSense::Drop,
move |ctx, rsc| match ctx.data.sense {
CursorSense::PressStart(_) | CursorSense::Pressing(_) => {
// Any committed drag takes capture -- a real caller
// would gate this on a `DragArbiter`/`DragGesture`
// decision, but this test only needs to exercise the
// capture-and-release mechanics themselves.
ctx.data.render.capture_pointer(draggable_weak.id());
let _ = rsc;
}
CursorSense::Drop => dropped.set(true),
_ => {}
},
);
}
let mut render = UiRenderState::new();
render.resize((100.0, 100.0));
render.update(&draggable, &mut rsc);
let mut state = ();
let mut press = cursor_at((5.0, 5.0).into());
press.buttons.left = ActivationState::Start;
render.run_sensors(&mut rsc, &mut state, press, (100.0, 100.0).into());
assert_eq!(
render.captured_pointer(),
Some(draggable.id()),
"the press should have taken capture"
);
// The release lands nowhere near the widget's own region -- the exact
// shape of a fast fling's `ACTION_UP`.
let mut release = cursor_at((95.0, 95.0).into());
release.buttons.left = ActivationState::End;
render.run_sensors(&mut rsc, &mut state, release, (100.0, 100.0).into());
assert!(
dropped.get(),
"a release outside every widget's hit region must still reach \
the widget holding pointer capture"
);
assert_eq!(
render.captured_pointer(),
None,
"Drop must release the capture"
);
}
/// A widget that never registers `CursorSense::Drop` at all must not be
/// affected by someone else's capture -- capture is per-gesture, not
/// global suppression of the whole input system for widgets that were
/// never party to it. (Practically this matters because a captured
/// widget's registration list still has to include `Drop` for `should_run`
/// to ever match it; this pins that half of the contract.)
#[test]
fn capturing_one_widget_starves_every_other_widget_of_events() {
let mut rsc = SenseRsc {
ui: UiData::default(),
events: EventManager::default(),
};
let a = rsc.ui.widgets.add_strong(Rect::new(UiColor::WHITE));
let a_weak = a.weak();
let b = rsc.ui.widgets.add_strong(Rect::new(UiColor::RED));
let b_weak = b.weak();
let b_hovered = Rc::new(Cell::new(false));
{
let b_hovered = b_hovered.clone();
rsc.register_event(b_weak, CursorSense::Hovering, move |_ctx, _rsc| {
b_hovered.set(true);
});
}
let root = rsc
.ui
.widgets
.add_strong(Stack {
children: vec![a.any(), b.any()],
size: StackSize::default(),
})
.any();
let mut render = UiRenderState::new();
render.resize((100.0, 100.0));
render.update(&root, &mut rsc);
render.capture_pointer(a_weak.id());
let mut state = ();
let cursor = cursor_at((50.0, 50.0).into());
render.run_sensors(&mut rsc, &mut state, cursor, (100.0, 100.0).into());
assert!(
!b_hovered.get(),
"while a's drag holds capture, b must see no hover at all"
);
}
+200
View File
@@ -424,6 +424,13 @@ impl List {
/// pixels, so `1.0` here is not a placeholder for "unknown density,"
/// it is the correct density for a self-consistent unit system.
pub fn fling(&mut self, velocity_px_per_s: f32) {
// A NaN/inf velocity (a `VelocityTracker::velocity()` divide-by-
// near-zero span, or a caller passing a raw device value straight
// through) would propagate silently into `deceleration_for`'s
// `.ln()` -- the fling either never settles or jumps to NaN
// positions with nothing on screen saying why (docs/
// REVIEW-2026-09-06.md finding 3).
debug_assert!(velocity_px_per_s.is_finite());
if velocity_px_per_s == 0.0 || self.anchor.is_none() {
self.fling = None;
return;
@@ -552,6 +559,20 @@ impl List {
self.extents.get(&key).map(|e| (e.top, e.bottom))
}
/// The row whose on-screen box (as of the last layout) contains
/// `viewport_pos`, or `None` if it falls outside every row currently
/// drawn (a gap, a header, or off the loaded content entirely). O
/// (visible rows), same as `reanchor_at_tap`. What a caller resolves a
/// pointer-captured gesture's row-under-the-finger against once the
/// gesture is no longer being delivered through any one row's own hit
/// region -- see `iris::sense`'s pointer-capture doc.
pub fn key_at(&self, viewport_pos: f32) -> Option<RowKey> {
self.extents
.iter()
.find(|(_, ext)| viewport_pos >= ext.top && viewport_pos <= ext.bottom)
.map(|(&key, _)| key)
}
fn slot_exists(&self, slot: isize) -> bool {
match slot {
BEFORE_SLOT => self.more_before.is_some(),
@@ -749,6 +770,17 @@ impl List {
/// one-frame lag `Scroll`'s own content-length cache accepts, per
/// LAYOUT.md.
fn place(&mut self, painter: &mut Painter, slot: isize, placement: Placement) -> (f32, f32) {
// Every current caller derives `slot` from `repair_anchor`/
// `prev_slot`/`next_slot`, which already check existence -- but
// that invariant is enforced by convention across three call
// sites, not by this function, which would otherwise fail with a
// bare "index out of bounds" and no context (docs/
// REVIEW-2026-09-06.md finding 2). `slot_widget`, called from
// here, is what actually indexes/`.expect`s on it.
debug_assert!(
self.slot_exists(slot),
"place() called with a slot that doesn't exist: {slot:?}"
);
let axis = self.axis;
let output_len = painter.output_size().axis(axis);
let container_len = painter.region().axis(axis).len();
@@ -1273,6 +1305,52 @@ mod tests {
);
}
/// Neither `replacing_the_last_row_stays_pinned_to_the_bottom` nor
/// its sibling below ever asserts the *evicted* key's own bookkeeping
/// is actually gone -- both replace row 4 with another row also keyed
/// `4`, so `heights.remove(&old.key)` removing and re-inserting the
/// same key would pass either test even if it did nothing (docs/
/// REVIEW-2026-09-06.md finding 10; this is `Selection`'s finding 1
/// class of bug -- a stale handle outliving what it points to --
/// production-tested from `List`'s own side). Replacing with a
/// **different** key is what actually exercises the removal.
#[test]
fn replace_back_forgets_the_evicted_keys_own_height() {
let mut rsc = TestRsc {
ui: UiData::default(),
};
let mut list = List::new(Axis::Y);
push_rows(&mut rsc, &mut list, &[0, 1, 2, 3, 4], 20.0);
let (list_weak, root) = add_list(&mut rsc, list);
let mut render = UiRenderState::new();
render.resize((100.0, 60.0));
render.update(&root, &mut rsc);
assert!(
rsc.ui
.widgets
.get(&list_weak)
.unwrap()
.heights
.contains_key(&4)
);
let (_weak, new_row) = fixed_row(&mut rsc, 40.0);
let old = rsc
.ui
.widgets
.get_mut(&list_weak)
.unwrap()
.replace_back(ListRow::new(100, new_row));
let list_ref = rsc.ui.widgets.get(&list_weak).unwrap();
assert_eq!(old.map(|o| o.key), Some(4));
assert!(
!list_ref.heights.contains_key(&4),
"the evicted key's cached height must not outlive the row it measured"
);
}
/// The other half of the same fix's contract: replacing a row that is
/// *not* on screen must not move anything that is. `replace_back` only
/// touches the last slot's own widget and this file's own `heights`/
@@ -1332,6 +1410,63 @@ mod tests {
}
}
/// RUST.md's P0 phone report (Iris's screenshot, 2026-09-06): a
/// replaced row's primitives drawn a second time, overlapping the
/// replacement. Reproduces the exact path `TranscriptScreen::apply`'s
/// `ReplaceLast` case drives up to 400 times during a streamed reply
/// (`bench_client.rs`'s stream phase): the last slot's widget is
/// swapped for a brand-new one, same key, and (since a fresh widget
/// has no cached height) placed via `place`'s `draw_twice` path every
/// time -- the provisional-then-real two-draw sequence LAYOUT.md
/// documents as the one place in this crate that deliberately draws a
/// widget twice. If `draw_inner`'s old-children diffing or
/// `UiRenderState::remove`'s primitive freeing ever failed to retire
/// the evicted widget (or the provisional draw's own primitives), it
/// would show up here as `active_widgets` growing without bound.
/// **Passes as written** -- this pins the widget-arena layer as
/// correct in isolation; see the P0 box for where the duplicate was
/// actually chased to instead (`Span`'s two-phase draw and the
/// `redraw_all`-vs-`redraw_updates` split, still open).
#[test]
fn replacing_the_last_row_many_times_does_not_leak_primitives() {
let mut rsc = TestRsc {
ui: UiData::default(),
};
let mut list = List::new(Axis::Y);
for key in 0..5u64 {
let (_bg_id, row) = background_styled_row(&mut rsc, 20.0);
list.push_back(ListRow::new(key, row));
}
let (list_weak, root) = add_list(&mut rsc, list);
let mut render = UiRenderState::new();
render.resize((100.0, 100.0));
render.update(&root, &mut rsc);
let before = render.active_widgets();
for i in 0..400u32 {
// A varying height keeps every replace on the `draw_twice`
// (cache-miss) path rather than settling into the O(1)
// same-size `mov` fast path once the height happens to repeat.
let (_bg_id, new_row) = background_styled_row(&mut rsc, 20.0 + (i % 3) as f32);
rsc.ui
.widgets
.get_mut(&list_weak)
.unwrap()
.replace_back(ListRow::new(4, new_row));
render.update(&root, &mut rsc);
}
let after = render.active_widgets();
assert_eq!(
before, after,
"400 replaces of the last row must leave exactly the same \
number of active widgets as before a leaked id (and the \
primitives that live as long as its ActiveData does) would \
show up here as growth"
);
}
/// Enough rows, tall enough, that a fling toward the start has real
/// room to travel before `at_start` clamps it -- shared by the fling
/// tests below.
@@ -1409,6 +1544,71 @@ mod tests {
}
}
/// `fling_moves_the_list_and_then_settles`/
/// `fling_distance_is_positive_toward_the_end` only check that a fling
/// started, moved the right way and eventually stopped -- both
/// unaffected by *how* the interior ticks split up the total travel
/// (docs/REVIEW-2026-09-06.md finding 9). A regression that made
/// `tick_fling` apply the whole spline distance every tick instead of
/// just this tick's incremental slice would still pass both, while
/// being wildly wrong every intermediate frame -- this pins the
/// per-tick delta to a decelerating curve (`FlingCalculator::
/// position_at`'s own monotonic-and-clamped property, one level
/// down, already covers the calculator alone; this is the same
/// property through `List::tick_fling`'s `scroll`/`extents`
/// accumulation).
#[test]
fn tick_fling_applies_shrinking_incremental_deltas() {
let mut rsc = TestRsc {
ui: UiData::default(),
};
let (list_weak, root, mut render) = build_flingable_list(&mut rsc);
rsc.ui.widgets.get_mut(&list_weak).unwrap().jump_to_start();
render.update(&root, &mut rsc);
rsc.ui.widgets.get_mut(&list_weak).unwrap().fling(8000.0);
let start = Instant::now();
let mut prev_top = rsc.ui.widgets.get(&list_weak).unwrap().extents[&0].top;
let mut deltas = Vec::new();
for step in 1..600 {
let now = start + std::time::Duration::from_millis(step * 16);
let still = rsc.ui.widgets.get_mut(&list_weak).unwrap().tick_fling(now);
render.update(&root, &mut rsc);
let Some(top) = rsc
.ui
.widgets
.get(&list_weak)
.unwrap()
.extents
.get(&0)
.map(|e| e.top)
else {
break; // row 0 scrolled out of the loaded extents
};
deltas.push((prev_top - top).abs());
prev_top = top;
if !still {
break;
}
}
assert!(
deltas.len() >= 3,
"fling settled or left row 0's extent before collecting enough samples"
);
// Skip the first tick (the slop-transition jump the arbiter
// applies is a `List::fling`-adjacent concern, not this curve,
// but the very first frame can still carry rounding noise from
// `jump_to_start`'s own layout settling).
for w in deltas[1..].windows(2) {
assert!(
w[1] <= w[0] + 0.01,
"fling's per-tick delta grew instead of decelerating: {:?} then {:?}",
w[0],
w[1]
);
}
}
#[test]
fn cancel_fling_stops_it_with_no_further_movement() {
let mut rsc = TestRsc {
+168 -2
View File
@@ -51,7 +51,7 @@ pub mod selection;
use client_core::transcript_fold::TranscriptRow as FoldedRow;
use iris::prelude::*;
use selection::Selection;
use std::{cell::RefCell, rc::Rc};
use std::{cell::RefCell, rc::Rc, time::Instant};
pub struct TranscriptScreen {
/// The transcript's own `List` -- exposed so a caller can read
@@ -151,8 +151,16 @@ impl TranscriptScreen {
}
RowDiff::Rebuild => {
// A row before the tail changed (a regroup) -- nothing
// short of a full rebuild expresses that.
// short of a full rebuild expresses that. `Selection`
// gets cleared the same way `List` does, right before the
// rows it was pointing at go with it -- `push_row` below
// re-`register`s whatever survives as it rebuilds each
// row (docs/REVIEW-2026-09-06.md finding 1: a key that
// `group_tool_runs` regrouped away used to stay in
// `Selection` pointing at a widget this `clear()` had
// just freed, panicking the next long-press anywhere).
self.rebuilds.set(self.rebuilds.get() + 1);
self.selection.borrow_mut().clear();
(self.list)(rsc).clear();
for row in &new_rows {
self.push_row(rsc, row);
@@ -220,6 +228,43 @@ where
})
.add(rsc);
// The continuation of a row-started drag once it has committed and
// taken pointer capture on `list`'s own id (`row.rs`'s registration is
// only ever the gesture's first frame) -- registered once here, not
// once per row, since `DragGesture`'s single shared instance must see
// each frame of one gesture exactly once. `ctx.data.pos`/`size` are
// already relative to `list`'s own on-screen box (this is what it was
// registered against), which is exactly the viewport-pixel space
// `List::key_at`/`extent` work in, so the row-under-the-pointer is
// resolved from those instead of a per-row hit test.
{
let selection = selection.clone();
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),
))
});
selection.borrow_mut().drag(
rsc,
list,
row,
ctx.data.cursor.pos,
ctx.data.sense,
Instant::now(),
ctx.data.render,
);
},
)
.add(rsc);
}
let (composer, composer_bar) = composer::build_composer(rsc);
let tree = (list.width(rest(1)).height(rest(1)), composer_bar)
@@ -377,3 +422,124 @@ mod diff_tests {
assert_eq!(diff_rows(&old, &new), RowDiff::Rebuild);
}
}
/// Exercises `TranscriptScreen::apply`'s `Rebuild` arm through a real
/// `Selection`, the gap docs/REVIEW-2026-09-06.md finding 8 named: the
/// pure `diff_rows` decision above and `selection.rs`'s own registration
/// tests each pass in isolation, and neither alone catches finding 1 (a
/// regrouped-away row's key surviving in `Selection` after `List::clear()`
/// has already freed its widget). This fails before `Selection::clear()`
/// existed and the `Rebuild` arm called it, with a panic from
/// `TextEditable::edit` resolving the freed slot.
#[cfg(test)]
mod apply_tests {
use super::*;
use client_core::transcript_fold::TranscriptItem;
struct TestFocus {
focus: Option<WeakWidget<TextEdit>>,
}
impl FocusHost for TestFocus {
fn recent_click(&mut self) -> bool {
false
}
fn set_focus(&mut self, id: Option<WeakWidget<TextEdit>>) {
self.focus = id;
}
fn focus_gained(&mut self, _region: Option<PixelRegion>) {}
fn is_focused(&self, id: WeakWidget<TextEdit>) -> bool {
self.focus == Some(id)
}
}
struct TestRsc {
ui: UiData,
events: EventManager<TestRsc>,
}
impl UiRsc for TestRsc {
fn ui(&self) -> &UiData {
&self.ui
}
fn ui_mut(&mut self) -> &mut UiData {
&mut self.ui
}
fn on_draw(&mut self, active: &ActiveData) {
self.events.draw(active);
}
fn on_undraw(&mut self, active: &ActiveData) {
self.events.undraw(active);
}
fn on_remove(&mut self, id: WidgetId) {
self.events.remove(id);
}
}
impl HasState for TestRsc {
type State = TestFocus;
}
impl HasEvents for TestRsc {
fn events(&self) -> &EventManager<Self> {
&self.events
}
fn events_mut(&mut self) -> &mut EventManager<Self> {
&mut self.events
}
}
fn user(seq: u64, text: &str) -> TranscriptItem {
TranscriptItem::UserMsg {
seq,
text: text.to_string(),
attachments: Vec::new(),
}
}
fn tool(seq: u64, run_id: &str) -> TranscriptItem {
TranscriptItem::ToolRun {
seq,
id: format!("id{seq}"),
run_id: run_id.to_string(),
tool: "grep".to_string(),
input: "x".to_string(),
output: String::new(),
done: false,
asks: Vec::new(),
images: Vec::new(),
}
}
#[test]
fn a_row_dropped_by_a_regroup_does_not_outlive_itself_in_selection() {
use client_core::transcript_fold::group_tool_runs;
let mut rsc = TestRsc {
ui: UiData::default(),
events: EventManager::default(),
};
// Same regroup shape as diff_tests' regroup case, plus a trailing
// row (seq 4) that survives unchanged -- what a reader would tap
// on right after the regroup lands.
let old_items = vec![tool(1, "run-a"), user(2, "meanwhile"), user(4, "stable")];
let new_items = vec![tool(1, "run-a"), tool(3, "run-a"), user(4, "stable")];
assert_eq!(
diff_rows(&group_tool_runs(&old_items), &group_tool_runs(&new_items)),
RowDiff::Rebuild,
"test setup must actually exercise the Rebuild arm"
);
let (screen, _tree) = build_tree(&mut rsc, group_tool_runs(&old_items));
screen.apply(&mut rsc, &old_items, &new_items);
// The surviving row (seq 4) is what a reader's long-press would
// land on; `begin` deselects every *other* registered row first,
// which is exactly what used to resolve a stale `WeakWidget` left
// by the regrouped-away rows and panic.
let surviving_key = row::row_key(&client_core::transcript_fold::ItemKey::Seq(4));
screen.selection.borrow_mut().begin(
&mut rsc,
surviving_key,
Vec2::ZERO,
Vec2::new(10.0, 10.0),
);
}
}
+11 -5
View File
@@ -137,20 +137,26 @@ where
field
// `| CursorSense::unclick()` on top of the usual click-or-drag set
// -- the arbiter inside `Selection::drag` needs the release too,
// to go back to idle for the next press (`DragArbiter::release`).
// -- this row'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
// id and every further frame, including the terminal `Drop`,
// reaches `lib.rs`'s list-level registration instead -- see
// `iris::sense`'s pointer-capture doc for why that has to be a
// stable id rather than this row's, which `List` can retire mid-
// drag as content scrolls.
.on(
CursorSense::click_or_drag() | CursorSense::unclick(),
move |ctx, rsc| {
selection.borrow_mut().drag(
rsc,
list,
key,
ctx.data.pos,
ctx.data.size,
Some((key, ctx.data.pos, ctx.data.size)),
ctx.data.cursor.pos,
ctx.data.sense,
Instant::now(),
ctx.data.render,
);
},
)
+80 -83
View File
@@ -36,17 +36,15 @@ use std::{collections::BTreeMap, time::Instant};
pub struct Selection {
rows: BTreeMap<RowKey, WeakWidget<TextEdit>>,
anchor: Option<(RowKey, Vec2)>,
/// One arbiter shared by every row's drag handler -- RUST.md's I5
/// 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
/// `iris::sense::DragArbiter`'s own doc for the decision itself.
arbiter: DragArbiter,
/// Tracks the last ~100ms of this gesture's pan deltas (in the same
/// signed units `list.scroll` takes), so a release that turns out to
/// have been panning can hand `List::fling` a realistic initial
/// velocity instead of one frame's noisy last delta --
/// IRIS_TODO.md's "swiping has no momentum."
velocity: VelocityTracker,
/// `iris::sense::DragGesture`'s own doc for the arbitration, velocity
/// tracking and pointer-capture mechanics this no longer owns itself
/// -- Iris's 2026-09-06 ask (`IRIS.md`) that a drag's *mechanics* live
/// in iris's default input layer, with only the pan-vs-select
/// *decision* staying here.
gesture: DragGesture,
}
impl Default for Selection {
@@ -60,19 +58,37 @@ impl Selection {
Self {
rows: BTreeMap::new(),
anchor: None,
arbiter: DragArbiter::new(),
velocity: VelocityTracker::new(),
gesture: DragGesture::new(),
}
}
/// A row's selectable text became visible/known. Every addition here
/// needs its removal (`unregister`) -- called when `List` evicts the
/// row (`pop_front`/`pop_back`), so this map never outgrows however
/// many rows are actually loaded.
/// needs its removal (`unregister`, or `clear` for all of them at
/// once) -- called when `List` evicts the row (`pop_front`/
/// `pop_back`/`clear`), so this map never outgrows however many rows
/// are actually loaded. `List::place` guards the twin of this same
/// class of bug on the list's own side (`list.rs`'s `slot_exists`
/// 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<TextEdit>) {
self.rows.insert(key, text);
}
/// Drops every registration at once -- the same shape `List::clear()`
/// clears the list, and what `TranscriptScreen::apply`'s `Rebuild` arm
/// calls right before it, since a full rebuild drops every row's old
/// widget and `push_row` re-`register`s each surviving key's new one
/// as it goes (review docs/REVIEW-2026-09-06.md finding 1: the
/// `Rebuild` arm used to call only `List::clear()`, leaving any key
/// dropped by the regroup -- present in the old rows, absent from the
/// new ones -- pointing at a widget the list had just freed, so the
/// next long-press anywhere panicked in `begin`'s deselect loop).
pub fn clear(&mut self) {
self.rows.clear();
self.anchor = None;
}
pub fn unregister(&mut self, key: RowKey) {
self.rows.remove(&key);
if self.anchor.map(|(k, _)| k) == Some(key) {
@@ -165,84 +181,65 @@ impl Selection {
/// row, is what makes that consistent as a drag crosses row
/// boundaries).
///
/// `pos_row`/`size` are row-local, as `begin`/`extend` want;
/// `row`, if given, is `(key, pos_row, size)` for whichever row the
/// pointer is currently over -- row-local, as `begin`/`extend` want.
/// `None` once the gesture is pointer-captured (`iris::sense`'s
/// pointer-capture doc) and the current position falls outside every
/// row `List` has loaded (a gap, or off the end of the content); a
/// `Pan` outcome never needs it, so this only actually matters mid-
/// selection, where it is rare and the frame is simply dropped.
/// `pos_window` is in window space, since a pan's delta has to stay
/// meaningful even when this frame's event landed on a different row
/// than the last one.
/// than the last one. `render` is `CursorData`'s own field -- what
/// `DragGesture` needs to take pointer capture.
#[allow(clippy::too_many_arguments)]
pub fn drag(
&mut self,
ui: &mut impl UiRsc,
list: WeakWidget<List>,
key: RowKey,
pos_row: Vec2,
size: Vec2,
row: Option<(RowKey, Vec2, Vec2)>,
pos_window: Vec2,
sense: CursorSense,
now: Instant,
render: &UiRenderState,
) {
let outcome = match sense {
CursorSense::PressStart(_) => {
let already_selected = self.has_selection(ui);
self.arbiter.press_start(pos_window, now, already_selected);
self.velocity.reset();
// A fresh touch-down cancels any fling still coasting from
// the previous gesture -- `List::fling`'s own doc, and
// Android's `Scroller::abortAnimation` for the same reason.
list(ui).cancel_fling();
self.arbiter.update(pos_window, now)
}
CursorSense::PressEnd(_) => {
// A fling only ever follows a pan -- never a selection
// that happened to end with the finger still moving, and
// never a tap/long-press that never left `Undecided`.
if self.arbiter.is_panning() {
let v = self.velocity.velocity();
list(ui).fling(v);
}
self.arbiter.release();
return;
}
// A `Pressing` frame with the arbiter still `Idle` means this
// gesture's `ACTION_DOWN` landed somewhere no row's sensor
// covers (a row's own padding/gap, or a header with no
// selection handler) and this row is only now getting the
// touch as it moves across it -- the touch is definitely still
// down (that's what `Pressing` means), so without this the
// arbiter would sit in `Idle` answering `Undecided` for the
// rest of the gesture (`DragArbiter::update`'s own doc).
// Recovered by starting the press here instead of where it
// was missed -- RUST.md's I5 intermittent-touch-scroll-dropout
// finding, 2026-09-05.
_ if self.arbiter.is_idle() => {
let already_selected = self.has_selection(ui);
self.arbiter.press_start(pos_window, now, already_selected);
self.velocity.reset();
list(ui).cancel_fling();
self.arbiter.update(pos_window, now)
}
_ => self.arbiter.update(pos_window, now),
};
if matches!(sense, CursorSense::PressStart(_)) {
// A fresh touch-down cancels any fling still coasting from
// the previous gesture -- `List::fling`'s own doc, and
// Android's `Scroller::abortAnimation` for the same reason.
list(ui).cancel_fling();
}
let already_selected = self.has_selection(ui);
let outcome =
self.gesture
.handle(render, list.id(), sense, pos_window, now, already_selected);
match outcome {
DragOutcome::Undecided => {}
DragOutcome::Pan(dy) => {
let amt = -dy;
self.velocity.add_sample(amt, now);
list(ui).scroll(amt);
GestureOutcome::Undecided => {}
GestureOutcome::Pan(dy) => list(ui).scroll(-dy),
GestureOutcome::SelectStart => {
if let Some((key, pos_row, size)) = row {
// Grep-able on "iris selection" the way the frame
// report is on "iris frame report" -- selection has no
// accessibility label of its own yet, so this is the
// smallest way to confirm a real on-device long-
// press-then-drag actually reached here (RUST.md's I5
// box, "Measurements taken" (c)).
log::info!("iris selection: begin at row {key:?}");
self.begin(ui, key, pos_row, size);
}
}
DragOutcome::SelectStart => {
// Grep-able on "iris selection" the way the frame report is
// on "iris frame report" -- selection has no accessibility
// label of its own yet, so this is the smallest way to
// confirm a real on-device long-press-then-drag actually
// reached here (RUST.md's I5 box, "Measurements taken" (c)).
log::info!("iris selection: begin at row {key:?}");
self.begin(ui, key, pos_row, size);
}
DragOutcome::SelectExtend => {
log::info!("iris selection: extend to row {key:?}");
self.extend(ui, key, pos_row, size);
GestureOutcome::SelectExtend => {
if let Some((key, pos_row, size)) = row {
log::info!("iris selection: extend to row {key:?}");
self.extend(ui, key, pos_row, size);
}
}
// A fling only ever follows a pan -- never a selection that
// happened to end with the finger still moving, and never a
// tap/long-press that never left `Undecided` -- exactly what
// `DragGesture`'s `Some(v)` already encodes.
GestureOutcome::Released(Some(v)) => list(ui).fling(-v),
GestureOutcome::Released(None) => {}
}
}
@@ -344,8 +341,9 @@ mod tests {
let mut sel = Selection::new();
sel.register(1, field);
assert!(sel.arbiter.is_idle());
assert!(sel.gesture.is_idle());
let render = UiRenderState::new();
let now = Instant::now();
let size = Vec2::new(100.0, 20.0);
// No `PressStart` is ever sent -- only the `Pressing` frames a
@@ -353,15 +351,14 @@ mod tests {
sel.drag(
&mut rsc,
list,
1,
Vec2::ZERO,
size,
Some((1, Vec2::ZERO, size)),
Vec2::new(540.0, 700.0),
CursorSense::Pressing(CursorButton::Left),
now,
&render,
);
assert!(
!sel.arbiter.is_idle(),
!sel.gesture.is_idle(),
"a Pressing frame with the arbiter still Idle must recover \
the press rather than leaving it stuck"
);