Ask <app> and the assistant: a reload keeps notices in place and tool rows named

crates/shell/src/app_chat/model.rs

@@ -29,7 +29,7 @@ use std::collections::{BTreeMap, BTreeSet, HashMap};
 
 use serde_json::Value;
 
-use crate::system_chat::model::{ChatModel, Item, Phase, Role};
+use crate::system_chat::model::{history_turn, ChatModel, Item, Phase, Role, ToolNames};
 
 /// The person's lane (this panel's context) and the system agent's.
 pub const LANE_PERSON: &str = "person";
@@ -232,12 +232,16 @@ impl Conversation {
 
     /// The conversation's history (both transcripts merged by time; rows
     /// `{role, content, lane, speaker?, display_text?, thread_id|turn_id?}`)
-    /// replaces what the panel showed.
+    /// replaces what the panel showed. A tool row keeps the name the panel
+    /// showed for it ([`ToolNames`]: history has none), and what history
+    /// cannot know stays where it stood ([`keep_in_place`]).
     pub fn load_history(&mut self, rows: &Value) {
+        let rows = rows.as_array().map(Vec::as_slice).unwrap_or(&[]);
+        let mut tools = ToolNames::new(&self.chat.items, rows);
         let mut items = Vec::new();
-        for row in rows.as_array().into_iter().flatten() {
+        for row in rows {
             let lane = row["lane"].as_str().unwrap_or(LANE_PERSON);
-            let turn = row["turn_id"].as_str().or_else(|| row["thread_id"].as_str()).map(str::to_string);
+            let turn = history_turn(row);
             if let Some(t) = &turn {
                 self.lanes.entry(t.clone()).or_insert_with(|| lane.to_string());
             }
@@ -259,7 +263,7 @@ impl Conversation {
                 }
                 "tool" => items.push(Item::Tool {
                     call_id: row["tool_call_id"].as_str().unwrap_or("").to_string(),
-                    name: row["name"].as_str().or_else(|| row["tool_name"].as_str()).unwrap_or("tool").to_string(),
+                    name: tools.name(row, turn.as_deref()),
                     status: crate::system_chat::model::ToolStatus::Done,
                     detail: String::new(),
                     turn,
@@ -269,14 +273,11 @@ impl Conversation {
             }
         }
         // What history cannot know stays: notices, and anything still open.
-        for item in std::mem::take(&mut self.chat.items) {
-            let keep = matches!(item, Item::Question { answered: None, .. } | Item::Notice(_))
-                || matches!(&item, Item::Message { turn: Some(t), .. } if self.running.contains(t));
-            if keep {
-                items.push(item);
-            }
-        }
-        self.chat.items = items;
+        let running = &self.running;
+        let shown = std::mem::take(&mut self.chat.items);
+        self.chat.items = keep_in_place(items, shown, |item| {
+            matches!(item, Item::Question { answered: None, .. } | Item::Notice(_)) || matches!(item, Item::Message { turn: Some(t), .. } if running.contains(t))
+        });
         self.settle_phase();
     }
 
@@ -285,6 +286,59 @@ impl Conversation {
     }
 }
 
+/// What a transcript row is, to find it again in a reloaded history: its
+/// kind and its turn (history rows carry the turn, [`history_turn`]).
+fn row_key(item: &Item) -> Option<(&'static str, &str)> {
+    match item {
+        Item::Message { role: Role::User, turn: Some(t), .. } => Some(("user", t)),
+        Item::Message { role: Role::Assistant, turn: Some(t), .. } => Some(("assistant", t)),
+        Item::Tool { turn: Some(t), .. } => Some(("tool", t)),
+        _ => None,
+    }
+}
+
+/// `history`, with the items of `shown` (the transcript it replaces) that
+/// history cannot know (`kept`: notices, an open question, a running turn's
+/// rows) put back where they stood: right after the row they followed in
+/// `shown`, found again in `history` by its kind and turn (the same
+/// occurrence of that pair, or the last one there is). An item kept from
+/// before any row comes first; one whose rows history has none of comes
+/// last. Appending them all after the history moved "Stopped." and the
+/// other notices below every newer row on each reload.
+fn keep_in_place(history: Vec<Item>, shown: Vec<Item>, kept: impl Fn(&Item) -> bool) -> Vec<Item> {
+    let keys: Vec<Option<(&'static str, String)>> = history.iter().map(|i| row_key(i).map(|(k, t)| (k, t.to_string()))).collect();
+    let find = |key: &(&'static str, String), nth: usize| -> Option<usize> {
+        let at: Vec<usize> = keys.iter().enumerate().filter(|(_, k)| k.as_ref() == Some(key)).map(|(i, _)| i).collect();
+        at.get(nth).or(at.last()).copied()
+    };
+    let mut after: Vec<Vec<Item>> = (0..history.len()).map(|_| Vec::new()).collect();
+    let (mut first, mut last) = (Vec::new(), Vec::new());
+    // The rows `shown` had so far: kind, turn, and which occurrence.
+    let mut before: Vec<((&'static str, String), usize)> = Vec::new();
+    let mut seen: HashMap<(&'static str, String), usize> = HashMap::new();
+    for item in shown {
+        if kept(&item) {
+            match before.iter().rev().find_map(|(key, nth)| find(key, *nth)) {
+                Some(row) => after[row].push(item),
+                None if before.is_empty() => first.push(item),
+                None => last.push(item),
+            }
+        } else if let Some((kind, turn)) = row_key(&item) {
+            let key = (kind, turn.to_string());
+            let nth = seen.entry(key.clone()).or_default();
+            before.push((key, *nth));
+            *nth += 1;
+        }
+    }
+    let mut items = first;
+    for (row, placed) in history.into_iter().zip(after) {
+        items.push(row);
+        items.extend(placed);
+    }
+    items.extend(last);
+    items
+}
+
 /// The text after the kernel's origin marker, if it carries one.
 fn strip_marker(text: &str) -> &str {
     crate::ai_host::app_peers::split_origin_marker(text).map(|(_, rest)| rest).unwrap_or(text)

crates/shell/src/app_chat/tests.rs

@@ -149,6 +149,116 @@ fn the_merged_history_names_its_speakers() {
     );
 }
 
+/// The transcript as the panel draws it, row by row: speakers, notices,
+/// tool rows.
+fn transcript(c: &Conversation) -> Vec<String> {
+    c.chat
+        .items
+        .iter()
+        .filter_map(|i| match i {
+            Item::Message { speaker, text, .. } => Some(format!("{}: {text}", speaker.clone().unwrap_or_default())),
+            Item::Notice(n) => Some(format!("({n})")),
+            Item::Tool { name, .. } => Some(format!("[{name}]")),
+            _ => None,
+        })
+        .collect()
+}
+
+fn started(lane: &str, turn: &str, text: &str, kind: &str) -> Value {
+    json!({"method": "turn/started", "lane": lane, "speaker": {"kind": kind}, "request": {"text": text, "speaker": {"kind": kind}}, "params": {"turn_id": turn}})
+}
+
+/// A reload (the panel reopened) puts the notices back where they stood,
+/// after the row they followed, not below every newer row. The instrument
+/// run's order: the person's turn; the system agent's turn and the person's
+/// second one; the person's Stop ("Stopped." and its end), Ctrl+. ("Nothing
+/// of yours was running."), the row's Stop ("Stopped the system agent's
+/// task." and its end); then, with the panel closed, the system agent's
+/// next turn. After the reopen the notices stood below that last turn.
+#[test]
+fn a_reload_keeps_the_notices_where_they_stood() {
+    let mut c = Conversation::new("News");
+    c.apply(&started(LANE_PERSON, "p1", "Hello News", "person"));
+    c.apply(&env(LANE_PERSON, "p1", 2, "assistant_persisted", json!({"assistant_segment_id": "p1:1", "text": "SLOW DONE after 15 s"})));
+    c.apply(&env(LANE_PERSON, "p1", 3, "turn_terminal", json!({"outcome": "completed"})));
+    c.apply(&started(LANE_SYSTEM_AGENT, "s1", "SLOW:45", "system_agent"));
+    c.apply(&started(LANE_PERSON, "p2", "SLOW:40", "person"));
+    c.notice("Stopped.");
+    c.apply(&env(LANE_PERSON, "p2", 5, "turn_terminal", json!({"outcome": "interrupted", "error": {"message": "turn interrupted by client"}})));
+    c.notice("Nothing of yours was running.");
+    c.notice("Stopped the system agent's task.");
+    c.apply(&env(LANE_SYSTEM_AGENT, "s1", 900, "turn_terminal", json!({"outcome": "interrupted", "error": {"message": "turn interrupted by client"}})));
+    // The panel is closed; its follower still hears the system agent's turn.
+    c.apply(&started(LANE_SYSTEM_AGENT, "s2", "SLOW:3", "system_agent"));
+    c.apply(&env(LANE_SYSTEM_AGENT, "s2", 902, "assistant_persisted", json!({"assistant_segment_id": "s2:1", "text": "SLOW DONE after 3 s"})));
+    c.apply(&env(LANE_SYSTEM_AGENT, "s2", 903, "turn_terminal", json!({"outcome": "completed"})));
+    let live = transcript(&c);
+    assert_eq!(
+        live,
+        [
+            "You: Hello News",
+            "News's agent: SLOW DONE after 15 s",
+            "System agent: SLOW:45",
+            "You: SLOW:40",
+            "(Stopped.)",
+            "(turn interrupted by client)",
+            "(Nothing of yours was running.)",
+            "(Stopped the system agent's task.)",
+            "(turn interrupted by client)",
+            "System agent: SLOW:3",
+            "News's agent, to the system agent: SLOW DONE after 3 s",
+        ]
+    );
+    // Reopened: the merged history as the broker sends it. Every row a turn
+    // persisted carries its turn as `thread_id`; the two stopped turns the
+    // kernel never recorded are the broker's rows, with `turn_id`.
+    let history = json!([
+        {"role": "user", "content": "[from the person: News] Hello News", "lane": "person", "speaker": {"kind": "person"}, "display_text": "Hello News", "thread_id": "p1"},
+        {"role": "assistant", "content": "SLOW DONE after 15 s", "lane": "person", "thread_id": "p1"},
+        {"role": "user", "content": "SLOW:45", "display_text": "SLOW:45", "speaker": {"kind": "system_agent"}, "lane": "system_agent", "turn_id": "s1", "unrecorded": true},
+        {"role": "user", "content": "SLOW:40", "display_text": "SLOW:40", "speaker": {"kind": "person"}, "lane": "person", "turn_id": "p2", "unrecorded": true},
+        {"role": "user", "content": "[from the system agent] SLOW:3", "lane": "system_agent", "speaker": {"kind": "system_agent"}, "display_text": "SLOW:3", "thread_id": "s2"},
+        {"role": "assistant", "content": "SLOW DONE after 3 s", "lane": "system_agent", "thread_id": "s2"},
+    ]);
+    c.load_history(&history);
+    assert_eq!(transcript(&c), live, "the reload keeps every notice in its place");
+    c.load_history(&history);
+    assert_eq!(transcript(&c), live, "and so does the next one");
+    // A notice from before any row stays first; one after rows the history
+    // no longer has goes last.
+    let mut c = Conversation::new("News");
+    c.notice("Could not load the conversation: closed");
+    c.apply(&started(LANE_PERSON, "gone", "a turn the history lost", "person"));
+    c.apply(&env(LANE_PERSON, "gone", 2, "turn_terminal", json!({"outcome": "completed"})));
+    c.notice("Stopped.");
+    c.load_history(&json!([{"role": "user", "content": "earlier", "lane": "person", "thread_id": "p0"}]));
+    assert_eq!(transcript(&c), ["(Could not load the conversation: closed)", "You: earlier", "(Stopped.)"]);
+}
+
+/// The history's tool rows carry no tool name (octos's hydrate rows have
+/// none), so a reload showed "⚙ tool · done". A row keeps the name the
+/// panel showed for the same turn's call; a turn the panel never followed
+/// stays "tool" (never another call's name).
+#[test]
+fn a_reload_keeps_the_tool_names_the_panel_showed() {
+    let mut c = Conversation::new("News");
+    c.apply(&started(LANE_PERSON, "p1", "SCN_ASK", "person"));
+    c.apply(&env(LANE_PERSON, "p1", 2, "tool_start", json!({"tool_call_id": "c1", "name": "ask_user_question"})));
+    c.apply(&env(LANE_PERSON, "p1", 3, "tool_end", json!({"tool_call_id": "c1", "status": "complete"})));
+    c.apply(&env(LANE_PERSON, "p1", 4, "assistant_persisted", json!({"assistant_segment_id": "p1:1", "text": "SCN AUDIENCE Team"})));
+    c.apply(&env(LANE_PERSON, "p1", 5, "turn_terminal", json!({"outcome": "completed"})));
+    c.load_history(&json!([
+        {"role": "user", "content": "[from the person: News] older", "lane": "person", "speaker": {"kind": "person"}, "display_text": "older", "thread_id": "p0"},
+        {"role": "assistant", "content": "", "lane": "person", "thread_id": "p0"},
+        {"role": "tool", "content": "{\"ok\":true}", "lane": "person", "thread_id": "p0"},
+        {"role": "user", "content": "[from the person: News] SCN_ASK", "lane": "person", "speaker": {"kind": "person"}, "display_text": "SCN_ASK", "thread_id": "p1"},
+        {"role": "assistant", "content": "", "lane": "person", "thread_id": "p1"},
+        {"role": "tool", "content": "{\"ok\":true,\"kind\":\"user_question_answer\"}", "lane": "person", "thread_id": "p1"},
+        {"role": "assistant", "content": "SCN AUDIENCE Team", "lane": "person", "thread_id": "p1"},
+    ]));
+    assert_eq!(transcript(&c), ["You: older", "[tool]", "You: SCN_ASK", "[ask_user_question]", "News's agent: SCN AUDIENCE Team"]);
+}
+
 /// The live run's order (#184 follow-up): each lane counts its own
 /// `cursor.seq` (the peer's session is long, the person's context new), and
 /// the kernel sends a turn's user message when the turn ENDS. Replayed as

crates/shell/src/system_chat/model.rs

@@ -17,7 +17,10 @@
 //!   agent's on the person's behalf; an approval of any other turn on the
 //!   session (an external client's, e.g. Talk to Octos) is marked
 //!   [`ApprovalAsk::external`] and shown read-only: that client answers it.
-//! - **History**: `session/hydrate`'s rows replace the transcript.
+//! - **History**: `session/hydrate`'s rows replace the transcript; its tool
+//!   rows keep the names the transcript showed ([`ToolNames`]).
+
+use std::collections::HashMap;
 
 use serde_json::Value;
 
@@ -565,19 +568,22 @@ impl ChatModel {
     }
 
     /// The transcript from `session/hydrate` (`result.messages`), replacing
-    /// what the pane showed. Rows are `{role, content, turn_id?}`.
+    /// what the pane showed. Rows are `{role, content, thread_id?,
+    /// turn_id?}` ([`history_turn`]); a tool row is named by [`ToolNames`].
     pub fn load_history(&mut self, messages: &Value) {
+        let rows = messages.as_array().map(Vec::as_slice).unwrap_or(&[]);
+        let mut tools = ToolNames::new(&self.items, rows);
         let mut items = Vec::new();
-        for row in messages.as_array().into_iter().flatten() {
+        for row in rows {
             // The shell's note to the system agent is not the person's words.
             let text = crate::agents::strip_note(row.get("content").and_then(Value::as_str).unwrap_or("")).to_string();
-            let turn = row.get("turn_id").and_then(Value::as_str).map(str::to_string);
+            let turn = history_turn(row);
             match row.get("role").and_then(Value::as_str).unwrap_or("") {
                 "user" => items.push(Item::Message { role: Role::User, text, turn, segment: None, seq: None, speaker: None }),
                 "assistant" if !text.trim().is_empty() => items.push(Item::Message { role: Role::Assistant, text, turn, segment: None, seq: None, speaker: None }),
                 "tool" => items.push(Item::Tool {
                     call_id: row.get("tool_call_id").and_then(Value::as_str).unwrap_or("").to_string(),
-                    name: row.get("name").or_else(|| row.get("tool_name")).and_then(Value::as_str).unwrap_or("tool").to_string(),
+                    name: tools.name(row, turn.as_deref()),
                     status: ToolStatus::Done,
                     detail: String::new(),
                     turn,
@@ -610,6 +616,69 @@ impl ChatModel {
     }
 }
 
+/// A history row's turn: its `turn_id`, else its `thread_id`. octos's
+/// `session/hydrate` rows have no `turn_id` (its `HydratedMessage` leaves it
+/// unset), but every row a turn persists carries `thread_id` = that turn's
+/// id (`pre_stamp_turn_thread_id`); the broker's rows for a turn the kernel
+/// never recorded carry `turn_id`.
+pub fn history_turn(row: &Value) -> Option<String> {
+    row.get("turn_id").and_then(Value::as_str).or_else(|| row.get("thread_id").and_then(Value::as_str)).map(str::to_string)
+}
+
+/// What a reloaded history's tool rows are called. The rows carry no tool
+/// name: octos's `HydratedMessage` (the pinned revision) is `seq`, `role`,
+/// `content`, `thread_id`, `persisted_at` and the like, with no tool name or
+/// call id (the name is only on the assistant's tool call, which hydrate
+/// does not return), and the broker's merged history adds none. So a
+/// reloaded tool row keeps the name the transcript already showed for it
+/// (its live `tool/started`, or an earlier reload): a turn's tool rows, in
+/// order, when the transcript showed as many for that turn. A row's own
+/// `name` or `tool_name` comes first, should a kernel send one; a row the
+/// transcript never showed in full (another run's, a turn followed only in
+/// part) reads "tool".
+pub struct ToolNames {
+    /// The names the replaced transcript showed, per turn, in order.
+    shown: HashMap<String, Vec<String>>,
+    /// How many tool rows the history has per turn.
+    rows: HashMap<String, usize>,
+    /// How many of a turn's rows were named so far.
+    named: HashMap<String, usize>,
+}
+
+impl ToolNames {
+    /// For `rows` (the history) replacing `items` (the transcript).
+    pub fn new(items: &[Item], rows: &[Value]) -> ToolNames {
+        let mut shown: HashMap<String, Vec<String>> = HashMap::new();
+        for item in items {
+            if let Item::Tool { name, turn: Some(turn), .. } = item {
+                shown.entry(turn.clone()).or_default().push(name.clone());
+            }
+        }
+        let mut per_turn: HashMap<String, usize> = HashMap::new();
+        for row in rows.iter().filter(|r| r.get("role").and_then(Value::as_str) == Some("tool")) {
+            if let Some(turn) = history_turn(row) {
+                *per_turn.entry(turn).or_default() += 1;
+            }
+        }
+        ToolNames { shown, rows: per_turn, named: HashMap::new() }
+    }
+
+    /// The name of `row`, the next tool row of `turn`.
+    pub fn name(&mut self, row: &Value, turn: Option<&str>) -> String {
+        let at = turn.map(|turn| {
+            let next = self.named.entry(turn.to_string()).or_default();
+            *next += 1;
+            *next - 1
+        });
+        let own = row.get("name").and_then(Value::as_str).or_else(|| row.get("tool_name").and_then(Value::as_str)).filter(|n| !n.is_empty());
+        let shown = match (turn, at) {
+            (Some(turn), Some(at)) => self.shown.get(turn).filter(|names| names.len() == self.rows.get(turn).copied().unwrap_or(0)).and_then(|names| names.get(at)),
+            _ => None,
+        };
+        own.or(shown.map(String::as_str)).unwrap_or("tool").to_string()
+    }
+}
+
 /// A provider's error as the person may see it: every word that carries a
 /// key, or a visible piece of one, becomes `[key]`. Providers quote a masked
 /// key back ("Your api key: ****fcb0 is invalid", "sk-proj-****abcd"), and