From 78aff6484447f1464cd77738576630ebace28d31 Mon Sep 17 00:00:00 2001 From: iris <2+iris@noreply.localhost> Date: Sat, 5 Sep 2026 13:25:46 -0400 Subject: [PATCH] client-core: hoist transcript_fold::{fold_page,raw_seq} out of desktop-app Both desktop-app's app.rs and the new Android transcript client (RUST.md's I5) need the same page-fold and live-stream resume-cursor logic; per CODE_RULES's "write the logic once" it now lives in client-core alongside fold_event/group_tool_runs instead of being duplicated. desktop-app calls the shared functions; its own copies and their tests moved with them. Also fixes a clippy::collapsible_if in config.rs's percent_decode, found while re-running clippy after this change (let-chains are stable now). Co-Authored-By: Claude Fable 5.1 --- client-core/src/config.rs | 14 ++-- client-core/src/transcript_fold.rs | 115 ++++++++++++++++++++++++++++ iris/desktop-app/src/app.rs | 119 +---------------------------- 3 files changed, 125 insertions(+), 123 deletions(-) diff --git a/client-core/src/config.rs b/client-core/src/config.rs index 99e420c..f525fa1 100644 --- a/client-core/src/config.rs +++ b/client-core/src/config.rs @@ -78,14 +78,14 @@ fn percent_decode(s: &str) -> String { let mut out = Vec::with_capacity(bytes.len()); let mut i = 0; while i < bytes.len() { - if bytes[i] == b'%' && i + 2 < bytes.len() { - if let Ok(byte) = + if bytes[i] == b'%' + && i + 2 < bytes.len() + && let Ok(byte) = u8::from_str_radix(std::str::from_utf8(&bytes[i + 1..i + 3]).unwrap_or(""), 16) - { - out.push(byte); - i += 3; - continue; - } + { + out.push(byte); + i += 3; + continue; } out.push(bytes[i]); i += 1; diff --git a/client-core/src/transcript_fold.rs b/client-core/src/transcript_fold.rs index 8b7cdcd..1c02a8d 100644 --- a/client-core/src/transcript_fold.rs +++ b/client-core/src/transcript_fold.rs @@ -606,6 +606,37 @@ pub fn group_tool_runs(items: &[TranscriptItem]) -> Vec { rows } +/// Folds a page of raw transcript lines (`ApiClient::fetch_transcript_page`'s +/// `Vec`) into the flat item list this module works over. A line +/// this build can't parse fails the whole page rather than being skipped -- +/// CODE_RULES's "an enumeration must be able to say 'it broke'" -- since +/// silently dropping one event could hide, say, a user message that then +/// looks like it was never sent. Moved here from `desktop-app`'s `app.rs` +/// (RUST.md's E4) when the Android transcript client (I5) needed the same +/// fold: "write the logic once" applies to any caller embedding +/// `transcript-ui` against a live server, not just the first one. +pub fn fold_page(values: &[serde_json::Value]) -> Result, String> { + let mut items = Vec::new(); + for value in values { + let event: SeqEvent = serde_json::from_value(value.clone()).map_err(|e| { + format!("the server sent a transcript line this build couldn't parse: {e}") + })?; + items = fold_event(&items, &event); + } + Ok(items) +} + +/// The wire `seq` a raw transcript line carries -- the live-stream resume +/// cursor after loading a page must be this, not a folded item's `seq()`. +/// A folded `AssistantMsg` keeps the seq of the *first* delta it +/// accumulated (`fold_event`'s own doc), so resuming from that seq would +/// re-deliver every delta already folded into it, duplicating the tail of +/// a reply that was mid-stream when the page was fetched -- found via a +/// real screenshot in E4 (RUST.md), where the assistant's line doubled. +pub fn raw_seq(value: &serde_json::Value) -> Option { + value.get("seq")?.as_u64() +} + #[cfg(test)] mod tests { use super::*; @@ -824,4 +855,88 @@ mod tests { other => panic!("expected a QuestionCard, got {other:?}"), } } + + fn line(seq: u64, json: serde_json::Value) -> serde_json::Value { + let mut obj = json; + obj["seq"] = serde_json::json!(seq); + obj["ts"] = serde_json::json!(1.0); + obj + } + + /// The regression for a bug a real `run-headless.sh` screenshot found + /// in `desktop-app` (E4, RUST.md): resuming the live stream from the + /// last *item's* seq re-delivers the deltas already folded into a + /// still-open assistant message, doubling its tail. `raw_seq` of the + /// last wire line must be the true high-water mark instead, which for a + /// run of deltas is higher than every item's own `seq()`. + #[test] + fn the_resume_cursor_is_the_last_wire_seq_not_the_last_items_seq() { + let values = vec![ + line(1, serde_json::json!({"type": "userMessage", "text": "hi"})), + line( + 2, + serde_json::json!({"type": "assistantText", "delta": "a"}), + ), + line( + 3, + serde_json::json!({"type": "assistantText", "delta": "b"}), + ), + line( + 4, + serde_json::json!({"type": "assistantText", "delta": "c"}), + ), + ]; + let after = raw_seq(values.last().unwrap()).unwrap(); + assert_eq!(after, 4); + + let items = fold_page(&values).unwrap(); + let assistant_seq = items + .iter() + .find(|i| matches!(i, TranscriptItem::AssistantMsg { .. })) + .unwrap() + .seq(); + assert_eq!(assistant_seq, 2); + assert_ne!( + after, assistant_seq, + "the fixed bug: these must differ here" + ); + } + + #[test] + fn a_page_folds_into_one_settled_assistant_message() { + let values = vec![ + line(1, serde_json::json!({"type": "userMessage", "text": "hi"})), + line( + 2, + serde_json::json!({"type": "assistantText", "delta": "hel"}), + ), + line( + 3, + serde_json::json!({"type": "assistantText", "delta": "lo"}), + ), + ]; + let items = fold_page(&values).unwrap(); + assert_eq!( + items, + vec![ + TranscriptItem::UserMsg { + seq: 1, + text: "hi".to_string(), + attachments: Vec::new(), + }, + TranscriptItem::AssistantMsg { + seq: 2, + text: "hello".to_string(), + settled: false, + }, + ] + ); + } + + #[test] + fn an_unparseable_line_fails_the_whole_page() { + let values = vec![serde_json::json!({"seq": 1, "ts": 1.0, "type": "not-a-real-type"})]; + let err = fold_page(&values).unwrap_err(); + assert!(err.contains("couldn't parse")); + } } diff --git a/iris/desktop-app/src/app.rs b/iris/desktop-app/src/app.rs index 28e86d2..683494f 100644 --- a/iris/desktop-app/src/app.rs +++ b/iris/desktop-app/src/app.rs @@ -45,7 +45,9 @@ use client_core::api::{ApiClient, SessionSummary, UreqTransport}; use client_core::event_stream::{StreamItem, follow_session_events}; -use client_core::transcript_fold::{TranscriptItem, fold_event, group_tool_runs}; +use client_core::transcript_fold::{ + TranscriptItem, fold_event, fold_page, group_tool_runs, raw_seq, +}; use event_model::SeqEvent; use iris::prelude::*; use std::sync::Arc; @@ -421,118 +423,3 @@ fn placeholder(rsc: &mut DefaultRsc, message: &str) -> StrongWidget { .add_strong(rsc) .any() } - -/// Folds a page of raw transcript lines (`ApiClient::fetch_transcript_page`'s -/// `Vec`) into the flat item list `client_core::transcript_fold` -/// works over. A line this build can't parse fails the whole page rather -/// than being skipped -- CODE_RULES's "an enumeration must be able to say -/// 'it broke'" -- since silently dropping one event could hide, say, the -/// user message the composer is about to look like it never sent. -fn fold_page(values: &[serde_json::Value]) -> Result, String> { - let mut items = Vec::new(); - for value in values { - let event: SeqEvent = serde_json::from_value(value.clone()).map_err(|e| { - format!("the server sent a transcript line this build couldn't parse: {e}") - })?; - items = fold_event(&items, &event); - } - Ok(items) -} - -/// The wire `seq` a raw transcript line carries -- see `select_session`'s -/// comment on why the live-stream cursor has to be this, not a folded -/// item's `seq()`. -fn raw_seq(value: &serde_json::Value) -> Option { - value.get("seq")?.as_u64() -} - -#[cfg(test)] -mod tests { - use super::*; - - fn line(seq: u64, json: serde_json::Value) -> serde_json::Value { - let mut obj = json; - obj["seq"] = serde_json::json!(seq); - obj["ts"] = serde_json::json!(1.0); - obj - } - - /// The regression for the bug a real `run-headless.sh` screenshot - /// found (see `select_session`'s comment): resuming the live stream - /// from the last *item's* seq re-delivers the deltas already folded - /// into a still-open assistant message, doubling its tail. `raw_seq` - /// of the last wire line must be the true high-water mark instead, - /// which for a run of deltas is higher than every item's own `seq()`. - #[test] - fn the_resume_cursor_is_the_last_wire_seq_not_the_last_items_seq() { - let values = vec![ - line(1, serde_json::json!({"type": "userMessage", "text": "hi"})), - line( - 2, - serde_json::json!({"type": "assistantText", "delta": "a"}), - ), - line( - 3, - serde_json::json!({"type": "assistantText", "delta": "b"}), - ), - line( - 4, - serde_json::json!({"type": "assistantText", "delta": "c"}), - ), - ]; - let after = raw_seq(values.last().unwrap()).unwrap(); - assert_eq!(after, 4); - - let items = fold_page(&values).unwrap(); - // The folded item keeps the *first* delta's seq (2), which is - // exactly the value that must not be used as the resume cursor. - let assistant_seq = items - .iter() - .find(|i| matches!(i, TranscriptItem::AssistantMsg { .. })) - .unwrap() - .seq(); - assert_eq!(assistant_seq, 2); - assert_ne!( - after, assistant_seq, - "the fixed bug: these must differ here" - ); - } - - #[test] - fn a_page_folds_into_one_settled_assistant_message() { - let values = vec![ - line(1, serde_json::json!({"type": "userMessage", "text": "hi"})), - line( - 2, - serde_json::json!({"type": "assistantText", "delta": "hel"}), - ), - line( - 3, - serde_json::json!({"type": "assistantText", "delta": "lo"}), - ), - ]; - let items = fold_page(&values).unwrap(); - assert_eq!( - items, - vec![ - TranscriptItem::UserMsg { - seq: 1, - text: "hi".to_string(), - attachments: Vec::new(), - }, - TranscriptItem::AssistantMsg { - seq: 2, - text: "hello".to_string(), - settled: false, - }, - ] - ); - } - - #[test] - fn an_unparseable_line_fails_the_whole_page() { - let values = vec![serde_json::json!({"seq": 1, "ts": 1.0, "type": "not-a-real-type"})]; - let err = fold_page(&values).unwrap_err(); - assert!(err.contains("couldn't parse")); - } -}