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 <noreply@anthropic.com>
This commit is contained in:
1 parent
9a33cb5384
commit
78aff64844
3 files changed
+125
-123
No files matched your search
@@ -78,14 +78,14 @@ fn percent_decode(s: &str) -> String {
|
|||||||
let mut out = Vec::with_capacity(bytes.len());
|
let mut out = Vec::with_capacity(bytes.len());
|
||||||
let mut i = 0;
|
let mut i = 0;
|
||||||
while i < bytes.len() {
|
while i < bytes.len() {
|
||||||
if bytes[i] == b'%' && i + 2 < bytes.len() {
|
if bytes[i] == b'%'
|
||||||
if let Ok(byte) =
|
&& i + 2 < bytes.len()
|
||||||
|
&& let Ok(byte) =
|
||||||
u8::from_str_radix(std::str::from_utf8(&bytes[i + 1..i + 3]).unwrap_or(""), 16)
|
u8::from_str_radix(std::str::from_utf8(&bytes[i + 1..i + 3]).unwrap_or(""), 16)
|
||||||
{
|
{
|
||||||
out.push(byte);
|
out.push(byte);
|
||||||
i += 3;
|
i += 3;
|
||||||
continue;
|
continue;
|
||||||
}
|
|
||||||
}
|
}
|
||||||
out.push(bytes[i]);
|
out.push(bytes[i]);
|
||||||
i += 1;
|
i += 1;
|
||||||
|
|||||||
@@ -606,6 +606,37 @@ pub fn group_tool_runs(items: &[TranscriptItem]) -> Vec<TranscriptRow> {
|
|||||||
rows
|
rows
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Folds a page of raw transcript lines (`ApiClient::fetch_transcript_page`'s
|
||||||
|
/// `Vec<Value>`) 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<Vec<TranscriptItem>, 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<u64> {
|
||||||
|
value.get("seq")?.as_u64()
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
@@ -824,4 +855,88 @@ mod tests {
|
|||||||
other => panic!("expected a QuestionCard, got {other:?}"),
|
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"));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
+3
-116
@@ -45,7 +45,9 @@
|
|||||||
|
|
||||||
use client_core::api::{ApiClient, SessionSummary, UreqTransport};
|
use client_core::api::{ApiClient, SessionSummary, UreqTransport};
|
||||||
use client_core::event_stream::{StreamItem, follow_session_events};
|
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 event_model::SeqEvent;
|
||||||
use iris::prelude::*;
|
use iris::prelude::*;
|
||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
@@ -421,118 +423,3 @@ fn placeholder(rsc: &mut DefaultRsc<Client>, message: &str) -> StrongWidget {
|
|||||||
.add_strong(rsc)
|
.add_strong(rsc)
|
||||||
.any()
|
.any()
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Folds a page of raw transcript lines (`ApiClient::fetch_transcript_page`'s
|
|
||||||
/// `Vec<Value>`) 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<Vec<TranscriptItem>, 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<u64> {
|
|
||||||
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"));
|
|
||||||
}
|
|
||||||
}
|
|
||||||
Reference in new issue
Block a user