diff --git a/crates/biorouter/src/agents/code_delta.rs b/crates/biorouter/src/agents/code_delta.rs new file mode 100644 index 000000000..a09d4aa15 --- /dev/null +++ b/crates/biorouter/src/agents/code_delta.rs @@ -0,0 +1,393 @@ +//! The lines a Code Execution script's file edits added and removed. +//! +//! The desktop Chat summary's Code figure counts each successful `text_editor` +//! edit. A direct call carries its whole arguments in the transcript, so the +//! renderer measures it there. A call made from inside a script does not: its +//! record in `biorouter/tool-calls` keeps `args` cut at +//! `MAX_TOOL_CALL_RECORD_TEXT_BYTES` (so a real file's write is no longer +//! JSON), and records past the meta budget are dropped. So the script measures +//! its own edits here, from each sub-call's FULL arguments, and attaches the +//! total as `biorouter/code-delta`. Presentation only, like the records: the +//! model never sees it. +//! +//! The rules are the renderer's (`ui/desktop/src/components/summary/codeDelta.ts`), +//! and both test suites run the same table, `codeDeltaCases.json`, so they +//! cannot drift apart unnoticed: +//! - `write`/`create`: every line of `file_text` is added (the earlier content +//! is not known); +//! - `str_replace`: the line difference between `old_str` and `new_str`, or +//! the `+`/`-` lines of a unified `diff`; +//! - `insert`: every line of `new_str` is added; +//! - `undo_edit`: reverses the last counted edit of the same file. +//! +//! An edit and its undo both count, as two commits would. + +use serde::Serialize; +use serde_json::{Map, Value}; +use std::collections::HashMap; + +/// Result-meta key carrying a script's measured edits for the UI. +pub(crate) const CODE_DELTA_META_KEY: &str = "biorouter/code-delta"; + +/// Above this many line pairs (after the unchanged head and tail are set +/// aside) the changed region is counted whole rather than diffed. Must equal +/// `LCS_CELL_BUDGET` in the renderer. +const LCS_CELL_BUDGET: usize = 1_000_000; + +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize)] +pub(crate) struct CodeDelta { + pub(crate) added: usize, + pub(crate) removed: usize, +} + +impl CodeDelta { + fn reversed(self) -> Self { + Self { + added: self.removed, + removed: self.added, + } + } +} + +enum EditorEdit { + Changed(CodeDelta), + Undo, +} + +/// A script's running total of the edits its successful sub-calls made. +#[derive(Debug, Default)] +pub(crate) struct CodeDeltaTally { + total: CodeDelta, + /// Each file's counted edits, newest last, for `undo_edit` to take back. + history: HashMap>, + /// Whether any editing call was seen, so a script that edited nothing + /// carries no key at all. + observed: bool, +} + +impl CodeDeltaTally { + /// One SUCCESSFUL sub-call, by its prefixed name and full JSON arguments. + /// Anything that is not a `text_editor` edit is ignored. + pub(crate) fn observe(&mut self, tool_name: &str, arguments: &str) { + if !is_editor_tool(tool_name) { + return; + } + if let Ok(Value::Object(args)) = serde_json::from_str::(arguments) { + self.editor_call(&args); + } + } + + fn editor_call(&mut self, args: &Map) { + let Some(edit) = editor_edit(args) else { + return; + }; + self.observed = true; + let path = args + .get("path") + .and_then(Value::as_str) + .unwrap_or_default() + .to_string(); + match edit { + EditorEdit::Undo => { + if let Some(last) = self.history.get_mut(&path).and_then(Vec::pop) { + self.add(last.reversed()); + } + } + EditorEdit::Changed(delta) => { + self.add(delta); + self.history.entry(path).or_default().push(delta); + } + } + } + + fn add(&mut self, delta: CodeDelta) { + self.total.added = self.total.added.saturating_add(delta.added); + self.total.removed = self.total.removed.saturating_add(delta.removed); + } + + /// The `biorouter/code-delta` value, or `None` when the script edited nothing. + pub(crate) fn meta_value(&self) -> Option { + self.observed + .then(|| serde_json::to_value(self.total).unwrap_or_default()) + } +} + +/// `text_editor` is the developer extension's; `str_replace_editor` takes the +/// same arguments. +fn is_editor_tool(tool_name: &str) -> bool { + let base = tool_name.rsplit("__").next().unwrap_or(tool_name); + matches!(base, "text_editor" | "str_replace_editor") +} + +fn editor_edit(args: &Map) -> Option { + let text = |key: &str| args.get(key).and_then(Value::as_str); + let delta = match text("command")? { + "write" | "create" => CodeDelta { + added: split_lines(text("file_text")?).len(), + removed: 0, + }, + "str_replace" => match text("diff") { + Some(diff) => patch_delta(diff), + None => line_delta(text("old_str")?, text("new_str")?), + }, + "insert" => CodeDelta { + added: split_lines(text("new_str")?).len(), + removed: 0, + }, + "undo_edit" => return Some(EditorEdit::Undo), + _ => return None, + }; + Some(EditorEdit::Changed(delta)) +} + +/// A text's lines: a final newline ends the last line rather than starting one. +fn split_lines(text: &str) -> Vec<&str> { + if text.is_empty() { + return Vec::new(); + } + let mut lines: Vec<&str> = text + .split('\n') + .map(|line| line.strip_suffix('\r').unwrap_or(line)) + .collect(); + if lines.last() == Some(&"") { + lines.pop(); + } + lines +} + +/// Lines `after` has that `before` lacks, and the reverse, by longest common +/// subsequence. +fn line_delta(before: &str, after: &str) -> CodeDelta { + let a = split_lines(before); + let b = split_lines(after); + let mut start = 0; + while start < a.len() && start < b.len() && a[start] == b[start] { + start += 1; + } + let (mut end_a, mut end_b) = (a.len(), b.len()); + while end_a > start && end_b > start && a[end_a - 1] == b[end_b - 1] { + end_a -= 1; + end_b -= 1; + } + let (a, b) = (&a[start..end_a], &b[start..end_b]); + let (n, m) = (a.len(), b.len()); + if n == 0 || m == 0 || n.saturating_mul(m) > LCS_CELL_BUDGET { + return CodeDelta { + added: m, + removed: n, + }; + } + let mut previous = vec![0usize; m + 1]; + let mut current = vec![0usize; m + 1]; + for line in a { + // `diagonal` is previous[j - 1], `left` is current[j - 1]. + let (mut diagonal, mut left) = (0, 0); + for (j, other) in b.iter().enumerate() { + let up = previous[j + 1]; + left = if line == other { + diagonal + 1 + } else { + up.max(left) + }; + current[j + 1] = left; + diagonal = up; + } + std::mem::swap(&mut previous, &mut current); + } + let common = previous[m]; + CodeDelta { + added: m - common, + removed: n - common, + } +} + +/// `-a[,count] +b[,count]` from a hunk header, counts defaulting to 1, or +/// `None` when the header carries no usable range. +fn hunk_counts(line: &str) -> Option<(i64, i64)> { + fn range(text: &str) -> Option<(i64, &str)> { + let digits = text.bytes().take_while(u8::is_ascii_digit).count(); + if digits == 0 { + return None; + } + // The digits are ASCII, so these cuts fall on char boundaries. + let rest = text.get(digits..)?; + let Some(rest) = rest.strip_prefix(',') else { + return Some((1, rest)); + }; + let digits = rest.bytes().take_while(u8::is_ascii_digit).count(); + if digits == 0 { + return None; + } + let count = rest.get(..digits)?.parse().unwrap_or(i64::MAX); + Some((count, rest.get(digits..)?)) + } + let (old, rest) = range(line.strip_prefix("@@ -")?)?; + let (new, rest) = range(rest.strip_prefix(" +")?)?; + rest.starts_with(" @@").then_some((old, new)) +} + +#[derive(PartialEq)] +enum PatchState { + Header, + Counted, + Loose, +} + +/// The `+` and `-` lines of a unified diff's hunks. A hunk header's counts say +/// where the hunk ends, so a removed `-- x` line (written `--- x`) is not taken +/// for the next file's header. A header without counts falls back to counting +/// every `+`/`-` line that is not a `+++ `/`--- ` header. +fn patch_delta(diff: &str) -> CodeDelta { + let mut delta = CodeDelta::default(); + let mut state = PatchState::Header; + let (mut old_left, mut new_left) = (0i64, 0i64); + for raw in diff.split('\n') { + let line = raw.strip_suffix('\r').unwrap_or(raw); + if state == PatchState::Counted { + match line.chars().next() { + Some('\\') => continue, + Some('-') => { + delta.removed += 1; + old_left -= 1; + } + Some('+') => { + delta.added += 1; + new_left -= 1; + } + Some(' ') | None => { + old_left -= 1; + new_left -= 1; + } + Some(_) => state = PatchState::Header, + } + if state == PatchState::Counted { + if old_left <= 0 && new_left <= 0 { + state = PatchState::Header; + } + continue; + } + } + if line.starts_with("@@") { + state = match hunk_counts(line) { + Some((old, new)) => { + old_left = old; + new_left = new; + if old > 0 || new > 0 { + PatchState::Counted + } else { + PatchState::Header + } + } + None => PatchState::Loose, + }; + continue; + } + if state == PatchState::Loose { + if line.starts_with("+++ ") || line.starts_with("--- ") { + state = PatchState::Header; + } else if line.starts_with('+') { + delta.added += 1; + } else if line.starts_with('-') { + delta.removed += 1; + } + } + } + delta +} + +#[cfg(test)] +mod tests { + use super::*; + use serde::Deserialize; + + #[derive(Deserialize)] + struct Case { + name: String, + calls: Vec>, + added: usize, + removed: usize, + } + + /// The renderer runs this same table (`codeDelta.test.ts`). + #[test] + fn agrees_with_the_renderer_on_every_shared_case() { + let cases: Vec = serde_json::from_str(include_str!( + "../../../../ui/desktop/src/components/summary/codeDeltaCases.json" + )) + .expect("the shared cases parse"); + assert!(cases.len() >= 10, "the shared table was found and read"); + for case in cases { + let mut tally = CodeDeltaTally::default(); + for args in &case.calls { + tally.observe( + "developer__text_editor", + &serde_json::to_string(args).unwrap(), + ); + } + assert_eq!( + tally.total, + CodeDelta { + added: case.added, + removed: case.removed, + }, + "{}", + case.name + ); + } + } + + #[test] + fn counts_a_write_far_larger_than_a_record_keeps() { + let args = serde_json::json!({ + "command": "write", + "path": "/p/big.ts", + "file_text": "const x = 1;\n".repeat(4000), + }) + .to_string(); + assert!(args.len() > 4 * 2048); + let mut tally = CodeDeltaTally::default(); + tally.observe("developer__text_editor", &args); + assert_eq!( + tally.meta_value(), + Some(serde_json::json!({ "added": 4000, "removed": 0 })) + ); + } + + #[test] + fn a_script_that_edited_nothing_carries_no_total() { + let mut tally = CodeDeltaTally::default(); + tally.observe("developer__shell", r#"{"command":"echo +3 -1 > out.txt"}"#); + tally.observe( + "developer__text_editor", + r#"{"command":"view","path":"/p/a"}"#, + ); + tally.observe("developer__text_editor", "{not json"); + assert_eq!(tally.meta_value(), None); + } + + #[test] + fn an_edit_that_changed_no_line_still_reports_a_total() { + let mut tally = CodeDeltaTally::default(); + tally.observe( + "developer__text_editor", + r#"{"command":"str_replace","path":"/p/a","old_str":"x","new_str":"x"}"#, + ); + assert_eq!( + tally.meta_value(), + Some(serde_json::json!({ "added": 0, "removed": 0 })) + ); + } + + #[test] + fn a_huge_replacement_is_counted_whole_instead_of_diffed() { + let before = (0..2000).map(|i| format!("a{i}\n")).collect::(); + let after = (0..2000).map(|i| format!("b{i}\n")).collect::(); + assert_eq!( + line_delta(&before, &after), + CodeDelta { + added: 2000, + removed: 2000, + } + ); + } +} diff --git a/crates/biorouter/src/agents/code_execution_extension.rs b/crates/biorouter/src/agents/code_execution_extension.rs index 43ed41f9d..6d4c3aa79 100644 --- a/crates/biorouter/src/agents/code_execution_extension.rs +++ b/crates/biorouter/src/agents/code_execution_extension.rs @@ -1,3 +1,4 @@ +use crate::agents::code_delta::{CodeDeltaTally, CODE_DELTA_META_KEY}; use crate::agents::extension::PlatformExtensionContext; use crate::agents::extension_manager::get_parameter_names; use crate::agents::mcp_client::{Error, McpClientTrait, McpMeta}; @@ -2272,6 +2273,10 @@ struct CollectedArtifacts { /// `MAX_TOOL_CALL_META_TOTAL_BYTES`. tool_call_bytes: usize, dropped_tool_calls: usize, + /// Lines the script's successful `text_editor` edits changed, measured + /// from their FULL arguments — a record's `args` is truncated and a record + /// past the budget is dropped, so the records cannot be summed for this. + code_delta: CodeDeltaTally, } impl CollectedArtifacts { @@ -2370,6 +2375,16 @@ impl CollectedArtifacts { )) } + /// A finished sub-call: its record, and, when it succeeded, the file edit + /// it made, measured from its FULL arguments (the record keeps them + /// truncated, and may itself be dropped past the budget). + fn record_call(&mut self, record: ToolCallRecord, tool_name: &str, arguments: &str) { + if record.status == "ok" { + self.code_delta.observe(tool_name, arguments); + } + self.push_tool_call(record); + } + fn push_tool_call(&mut self, record: ToolCallRecord) { let bytes = record.serialized_bytes(); if self.tool_calls.len() >= MAX_TOOL_CALL_RECORDS @@ -2771,6 +2786,10 @@ impl CodeExecutionClient { ); } } + // Read by the Chat summary's Code figure (`summary/codeDelta.ts`). + if let Some(delta) = collected.code_delta.meta_value() { + meta.insert(CODE_DELTA_META_KEY.to_string(), delta); + } (!meta.is_empty()).then_some(rmcp::model::Meta(meta)) } @@ -3644,7 +3663,10 @@ impl CodeExecutionClient { failure_kind, ), }; - collected_artifacts.lock().await.push_tool_call(record); + collected_artifacts + .lock() + .await + .record_call(record, &tool_name, &arguments); } let _ = response_tx.send(result); } diff --git a/crates/biorouter/src/agents/mod.rs b/crates/biorouter/src/agents/mod.rs index 0666a4d3b..f72e123ed 100644 --- a/crates/biorouter/src/agents/mod.rs +++ b/crates/biorouter/src/agents/mod.rs @@ -11,6 +11,9 @@ pub mod budget; // file it on the public issue tracker. pub mod bug_report; pub(crate) mod chatrecall_extension; +// The Chat summary's Code figure for a script's file edits, measured from the +// sub-calls' full arguments (their records are truncated). +pub(crate) mod code_delta; pub(crate) mod code_execution_extension; // BR-48: a deterministic done-ness gate for interactive chat — reuses the // workflow `SuccessCheck` machinery to keep a turn working until its checks diff --git a/crates/biorouter/tests/code_execution_integration.rs b/crates/biorouter/tests/code_execution_integration.rs index cc8f4f187..787cc9cbc 100644 --- a/crates/biorouter/tests/code_execution_integration.rs +++ b/crates/biorouter/tests/code_execution_integration.rs @@ -1246,6 +1246,62 @@ async fn case28_error_result_still_carries_call_telemetry() { assert_eq!(calls[0]["tool"], "developer__shell"); } +// The Chat summary's Code figure read `+0 −0` for a chat whose script wrote a +// file (final check, 2026-10-12). A record's `args` is cut at 2 KB, so a real +// file's write cannot be measured from the records; the script measures its +// edits from the full arguments and carries the total. +#[tokio::test] +async fn case31_result_meta_measures_the_edits_its_records_truncate() { + let m = manager().await; + let (_dir, dir_s) = workdir_tempdir(); + let path_s = format!("{dir_s}/notes.md"); + let result = exec_result( + &m, + &format!( + r##" + import {{ text_editor }} from "developer"; + const path = "{path_s}"; + const body = Array.from({{ length: 500 }}, (_, i) => "line " + i).join("\n") + "\n"; + text_editor({{ command: "write", path, file_text: body }}); + text_editor({{ command: "str_replace", path, old_str: "line 7\n", new_str: "line seven\n" }}); + text_editor({{ command: "str_replace", path, old_str: "line 9\n", new_str: "line nine\n" }}); + text_editor({{ command: "undo_edit", path }}); + let failed = false; + try {{ + text_editor({{ command: "str_replace", path, old_str: "absent", new_str: "x" }}); + }} catch (e) {{ + failed = true; + }} + text_editor({{ command: "view", path }}); + record_result({{ failed }}); + "## + ), + ) + .await; + + assert!( + !result.is_error.unwrap_or(false), + "the script recovered: {:?}", + result.content + ); + let meta = result.meta.expect("telemetry meta"); + let write_args = meta.0["biorouter/tool-calls"][0]["args"] + .as_str() + .expect("the write is recorded"); + assert!( + serde_json::from_str::(write_args).is_err(), + "the precondition: the write's recorded args are truncated past parsing" + ); + // 500 written, one line changed twice, the second change undone; the + // failed replacement and the view change nothing. + assert_eq!( + meta.0.get("biorouter/code-delta"), + Some(&json!({ "added": 503, "removed": 3 })), + "{:?}", + meta.0 + ); +} + #[tokio::test] async fn case25_calling_a_non_function_explains_itself() { let m = manager().await; diff --git a/docs/desktop-ui/chat-summary-rail.md b/docs/desktop-ui/chat-summary-rail.md index 75cd28599..98f1af90f 100644 --- a/docs/desktop-ui/chat-summary-rail.md +++ b/docs/desktop-ui/chat-summary-rail.md @@ -189,7 +189,15 @@ and 12px for labels. primitive's own track is `--background-muted`, which is the fill itself and would vanish. - **Statistics**: a `dl` of 28px rows: Tool calls, Tokens (with an InfoTip saying what is counted), Artifacts, and Code as `+N −M` in success and danger ink, read as words. A count of - zero is muted, so a chat that touched no code shows no red. A figure that is not known yet is + zero is muted, so a chat that touched no code shows no red. Code is the lines this chat's + successful `text_editor` edits added and removed (`summary/codeDelta.ts`), never anything read + off text: it used to match diff-shaped strings anywhere in the transcript, so a chat whose + script wrote a file read `+0 −0` and a chat that only read code read `+0 −9` (final check, + 2026-10-12). A direct call is measured from its own arguments; a Code Execution script carries + `biorouter/code-delta`, which the daemon measures from each sub-call's full arguments + (`agents/code_delta.rs`), because a sub-call's record keeps its arguments cut at 2 KB and a + record past the meta budget is dropped. A file written by a shell command is not counted: + nothing says how many lines it changed. A figure that is not known yet is an empty value in a row that keeps its label and its 28px, never a stand-in the load then replaces: all four while the transcript loads, and Tokens until the chat's usage ledger has answered (`useCostTracking`'s `ledgerSettled`, `summaryTokensValue`). Before that, the @@ -298,9 +306,10 @@ or the pointer is over the summary. | `hooks/useCostTracking.ledger.test.tsx` | `ledgerSettled` false with no rows while the usage request is pending, settled on rows, on none and on a failure, never one chat's ledger for another's, and a refresh of the same chat keeps its answer | | `components/BaseChat.costLedger.test.tsx` | On the real hook and the real footer `CostTracker`: no Tokens value and no cost while the ledger is pending, then exactly one reading ("N/A" and no cost for an uncertified row; "2.2M" and the priced counters for a ledger with no rows) | | `components/ChatSummary.test.tsx` | The content, the status words, the code delta in words and its muted zeros, empty values (and `aria-busy`) for figures not known yet, the gap above the statistics, the row entrances (kept through the re-renders that follow), the glyph crossfade, the thin progress bar, the scrolling body | +| `components/summary/codeDelta.test.ts` | The Code figure from edits only: a script's write counted from its record, the daemon's script total preferred over truncated or dropped records (and kept when the script failed later), nothing from diff-shaped text, a grep, a `git show` or a quoted snippet, no failed or unanswered call, an undo paired with its own file's last edit; and `codeDeltaCases.json`, the table the daemon's `agents::code_delta` tests also run, so the two tallies cannot drift. `tests/code_execution_integration.rs` `case31` holds the daemon's total against a real script whose write record is truncated | | `components/summary/SummaryHeaderButton.test.tsx` | The button's two modes, its tooltip, focus return, the dot with its count, and which flips of the pressed fill snap (geometry) and which fade (a press, the popover, the first turn) | | `components/summary/summaryPopover.browser.test.tsx` | In Chromium, with the whole stylesheet, in a band shaped like BaseChat's header: the old wiring landing on the band line and past the window (the instrument); the popover 4px below the band line and 8px above the window's bottom with its body scrolling, at 1300x900, beside a side preview, at a device pixel ratio of 2 and in a 600px window; a short list at its natural height | -| `components/alerts/toastClearance.test.ts` (WS-MOTION's) | The two class names the toast stack steps aside for, each held at the component that renders it, and the stack 16px left of the docked card and of the popover | +| `components/alerts/toastClearance.test.ts` (WS-MOTION's) | The two class names the toast stack steps aside for, each held at the component that renders it; the stack joining the docked card's column (as wide as the card, right edges aligned, 8px under it, or at the rail top over a card that fills the rail), never left of it over the conversation; and 16px left of the popover | | `styles/summaryRail.test.ts` | The exact grid templates, the `:not([data-preview-layout])` key, unlayered rules, no condition, transition or animation in the geometry, the card's motion, one rail in BaseChat, the `paddingX={6}` and `const contentClassName = 'px-1';` literals the 56px comes from, and the fill: the card's fill and transparent border, no hairline in the content, the track's ground, the scroll-edge fades, the pressed fill and the dot's ring | | `components/BaseChat.summaryRail.test.ts` | BaseChat's wiring: the button, the to-do hook's order and argument, the popover closing for the rail, the composer bar's and the subagent line's gutters, the first turn counted from the commit that creates a new chat, no titlebar padding under the header band, the message list memoised against everything the rail, the popover and the preview change, a loading chat's row reaching the rail, the chip, the placeholder and the ring, the commit before the load counted as loading (rail and empty state), a pending chat's composer bar and subagent line at opacity 0 and never `inert`, the Tokens value and the footer cost waiting for the ledger, and the transcript's follow hook (its arguments, both props, above the early return, the clicked card's hold), and the list settling through `useTranscriptSettle` with no first-settle force of its own (declared after the follow hook, whose hold it asks), the late row hold's arguments, its `followGrowth` on the transcript's `ScrollArea` and the one `ChatTurnError` drawing the row it decided about, and the first send: started in the commit that starts creating the chat, the chat named once created, ended wherever the composer gets the text back, drawn in the transcript slot, the four figures from `summaryFigures`, and the composer's `followUp` from `composerFollowsUp`, with the row kept out of `messagesLength`, and the composer's `reasoningDraftKey` taken from the tab alone, so the hand-off across the first send's remount finds it | | `components/summary/useFirstSend.test.tsx` | A new chat's first send against a fake daemon (`/agent/start` after 500ms, a 280ms load, the ledger 50ms after the session), on the real hook, `summaryFigures`, `ChatSummary`, `FirstSendTranscript` and `ProgressiveMessageList`: the old wiring leaving the text off screen and reading Tokens "0", nothing, "N/A" (the instrument); the text in the first commit after the send and in every commit after it; Tokens from empty to its answer once, the counts held at zero; the stored row the same markup as the first-send row, at rest; a placeholder that follows the store asking the open question under the drawn message (the instrument), and "Ask a follow-up" from the commit that draws it. Also the hook's life (begin, created, spent, another chat, a failure, attachments alone), `composerFollowsUp` and `summaryFigures`' cases | diff --git a/docs/desktop-ui/sidebar-chat-list.md b/docs/desktop-ui/sidebar-chat-list.md index ce28e4688..5ac73385e 100644 --- a/docs/desktop-ui/sidebar-chat-list.md +++ b/docs/desktop-ui/sidebar-chat-list.md @@ -138,6 +138,20 @@ has not read yet is brought in by a head re-read, and a page already in flight w lands does not put the row back (`applyNamesDuringLoad`). The rename route answers with no body, so the time is the renderer's clock until the next re-read carries the daemon's own. +A turn does not wait for one either. Running a turn is activity (R-30), so the moment the running +registry reports a turn in a chat, the sidebar raises that row's `updated_at` to the time it saw the +turn start, with no request, as a saved rename does (`turnSessionIds`, built by `sessionsInTurn`: +running, not finished, and not an agent restart). A restart is what a folder change shows as +running, and it is not a turn: whether the folder change stamps the chat is the daemon's to decide +(see "What moves a chat"), so the list leaves that row to the daemon's order. The raise is applied to every page read while the turn runs (`applyTurnRaises`), so a head +page the daemon answered before it stored the person's message does not move the chat back down. +It is dropped when the turn ends, and the read a finished turn starts carries the daemon's own time, +so a turn the daemon refused before storing anything leaves the chat where it was. A running chat +the list has not read yet is brought in by a head re-read, 250 ms after the start and again after +2 s. Until the final check of 2026-10-12 the list moved a running chat only when its turn finished: +it kept its place, spinner and all, for the whole turn. Opening a chat used to raise it, which hid +that until R-30. + ### What moves a chat Last activity is the daemon's `updated_at`. Since R-30 it moves when something is done to the @@ -320,7 +334,8 @@ buttons below. ### Rows hold still under the pointer -Under Last activity a running chat moves to the top. While the pointer is over the list, or a row is +Under Last activity a running chat moves to the top when its turn starts (see "Keeping the list +current"). While the pointer is over the list, or a row is being renamed, the list keeps the order it had (`applyHeldArrangement`), and the live order applies when the pointer leaves. A row that was removed goes, a new name shows, and rows that arrive below every held row come in; a row that would land above or between held rows waits. @@ -497,6 +512,7 @@ tab stop, so Tab reaches them from the row. |---|---| | `BioRouterSidebar/sidebarChatView.test.ts` | All nine views, folder labels and collisions, the stored view, the held order, `windowSidebarRows`; `sidebarRowTitle` and the name sort by the shown title; the drawn range: `sliceSidebarLines` (gaps of exact line counts, kept rows, a kept folder header or Show more, headers outside the range), `sidebarLinesInView` (lines from group boxes by binary search) and `nextSidebarDrawRange` (a range that holds still while it covers the view) | | `BioRouterSidebar/useSidebarSessions.test.ts` | Paging, and `loadAll` reading the rest in one request with the proof (or page by page from an older daemon), into a buffer shown once, stopping at the cap; `isLoadingAll` reported for the first full read only (not a later tail re-read, but again after a failed read); the head re-read on a new open chat, on focus and on a meta change | +| `BioRouterSidebar/useSidebarSessions.turnActivity.test.tsx` | A chat moving to the top when its turn starts (no request, and not again for an equal set), waiting under the pointer and moving when it leaves, staying on top over a head page read before the daemon stamped the turn, letting go when the turn ends (a refused turn goes back, a stored one stays), a running chat past the first page brought in by a head read, and `sessionsInTurn` leaving out a finished turn and an agent restart | | `BioRouterSidebar/RecentChats.test.tsx`, `.folders.test.tsx` | The header, the buckets, the rows, Group by Folder (and where focus goes after Show more or Show less from the keyboard, and after a pointer's click; the current chat past the first five drawn sixth, Show less collapsing to it, no toggle when nothing is hidden, and that row held under the pointer); a scheduled run's row showing its job id (its names and delete sentence too) with the rename starting from the stored name; a stored view drawn only once its read lands, a switch keeping the default arrangement until then, the complete list held from then on, and a first load held from its own rows | | `BioRouterSidebar/RecentChats.window.test.tsx` | The first 60 lines of the whole arranged list, growth on scroll before another page is asked for, a restart on a view change, a list whose box grew asking for the next page, and `⌘⌥R` drawing the current chat's row | | `BioRouterSidebar/RecentChats.switchCost.test.tsx` | A view switch mounts each drawn row once, also after a scroll; a row builds a menu only when it opens; a list re-render (pointer, rename keystroke) re-renders no other row; a measured list draws its height plus the overscan | diff --git a/ui/desktop/src/App.tsx b/ui/desktop/src/App.tsx index e46507b3d..0fa5c7534 100644 --- a/ui/desktop/src/App.tsx +++ b/ui/desktop/src/App.tsx @@ -412,8 +412,9 @@ export function AppInner() { shownScreen.current = screen; dismissScreenToasts(); }, [screen]); - // V2-R7-4. A toast steps below find in chat and left of the Chat summary - // instead of covering them; the corner stays theirs while they are open. + // V2-R7-4. A toast steps below find in chat and into the docked Chat summary's + // column, under its card, instead of covering them; the corner stays theirs + // while they are open, and the conversation beside the rail stays uncovered. useEffect(() => watchToastClearance(), []); const [chat, setChat] = useState({ @@ -675,7 +676,13 @@ export function AppInner() { extension-load report halfway down the chat pane, which is the bug this value closes. The reasoning and the floor live with `--toast-inset-top` in main.css; do not re-derive it from a header - measurement here. */} + measurement here. + + `--toast-clear-width` is unset except while the stack has joined the + docked Chat summary's column (`alerts/toastClearance.ts`), where it is + the card's width: then min and max are both the card's, so the toast + sits in the rail under the card and never hangs over the conversation + beside it. Unset, both fall back to the stack's own 280 to 420. */} TOAST_SURFACE_CLASS_NAME} @@ -687,8 +694,8 @@ export function AppInner() { '--toastify-z-index': 'var(--z-toast)', top: 'var(--toast-inset-top)', width: 'fit-content', - minWidth: '280px', - maxWidth: 'min(420px, calc(100vw - 32px))', + minWidth: 'var(--toast-clear-width, 280px)', + maxWidth: 'var(--toast-clear-width, min(420px, calc(100vw - 32px)))', } as React.CSSProperties } position="top-right" diff --git a/ui/desktop/src/components/BaseChat.tsx b/ui/desktop/src/components/BaseChat.tsx index ea2422a0d..43bfbc670 100644 --- a/ui/desktop/src/components/BaseChat.tsx +++ b/ui/desktop/src/components/BaseChat.tsx @@ -55,6 +55,7 @@ import { AlertCircle, Terminal } from './icons/app-icons'; import { ChatSummaryRail } from './summary/ChatSummaryRail'; import { SummaryHeaderButton } from './summary/SummaryHeaderButton'; import { summaryCopy } from './summary/copy'; +import { collectCodeDelta } from './summary/codeDelta'; import { useSessionTodos } from '../hooks/useSessionTodos'; import { summaryRailChat, useSummaryRail } from '../hooks/useSummaryRail'; import { useLoadingChat } from './summary/useLoadingChat'; @@ -1250,84 +1251,6 @@ export function countSessionToolCalls(messages: Message[]): number { return total; } -function visitStrings( - value: unknown, - visitor: (text: string) => void, - seen = new WeakSet() -) { - if (typeof value === 'string') { - visitor(value); - return; - } - if (!value || typeof value !== 'object') return; - if (seen.has(value)) return; - seen.add(value); - if (Array.isArray(value)) { - value.forEach((item) => visitStrings(item, visitor, seen)); - return; - } - Object.values(value as Record).forEach((item) => - visitStrings(item, visitor, seen) - ); -} - -function countPatchLines(text: string) { - let added = 0; - let removed = 0; - for (const line of text.split(/\r?\n/)) { - if (line.startsWith('+++') || line.startsWith('---')) continue; - if (line.startsWith('+')) added += 1; - if (line.startsWith('-')) removed += 1; - } - return { added, removed }; -} - -function collectCodeDelta(messages: Message[]) { - let added = 0; - let removed = 0; - const seenMatches = new Set(); - const compactDiffRe = /\+([0-9][\d,]*)\s+[-−]([0-9][\d,]*)/g; - const gitDiffRe = /([0-9][\d,]*)\s+insertions?\(\+\)(?:,\s*([0-9][\d,]*)\s+deletions?\(-\))?/gi; - const codeFenceRe = /```[^\n]*\n([\s\S]*?)```/g; - - for (const message of messages) { - visitStrings(message.content, (text) => { - if (!text.trim()) return; - - for (const match of text.matchAll(compactDiffRe)) { - const key = `compact:${match[0]}`; - if (seenMatches.has(key)) continue; - seenMatches.add(key); - added += Number(match[1].replace(/,/g, '')) || 0; - removed += Number(match[2].replace(/,/g, '')) || 0; - } - - for (const match of text.matchAll(gitDiffRe)) { - const key = `git:${match[0]}`; - if (seenMatches.has(key)) continue; - seenMatches.add(key); - added += Number(match[1].replace(/,/g, '')) || 0; - removed += Number(match[2]?.replace(/,/g, '') ?? 0) || 0; - } - - if (text.includes('*** Begin Patch') || text.includes('diff --git')) { - const delta = countPatchLines(text); - added += delta.added; - removed += delta.removed; - } - - for (const match of text.matchAll(codeFenceRe)) { - const key = `fence:${match[0].slice(0, 120)}:${match[0].length}`; - if (seenMatches.has(key)) continue; - seenMatches.add(key); - added += match[1].split(/\r?\n/).filter((line) => line.trim()).length; - } - }); - } - - return { added, removed }; -} - /** Module-level, so the message list's memoised element keeps one identity. */ const isUserRow = (m: Message) => m.role === 'user'; diff --git a/ui/desktop/src/components/BioRouterSidebar/AppSidebar.test.tsx b/ui/desktop/src/components/BioRouterSidebar/AppSidebar.test.tsx index bead84d6a..7f4211f0e 100644 --- a/ui/desktop/src/components/BioRouterSidebar/AppSidebar.test.tsx +++ b/ui/desktop/src/components/BioRouterSidebar/AppSidebar.test.tsx @@ -8,6 +8,7 @@ import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vite import type { SessionSummary } from '../../api'; import { ChatProvider } from '../../contexts/ChatContext'; import type { ChatType } from '../../types/chat'; +import { BROWSER_SURFACE_MARKER } from '../../utils/surface'; import { SIDEBAR_OVERLAY_BODY_CLASS, Sidebar, @@ -483,3 +484,45 @@ describe('the band and the chat header draw one hairline', () => { expect(sidebar).toContain(`className="h-chrome shrink-0 border-b ${token}"`); }); }); + +/** + * SD-8: a control that can never work here says so. New chat's shortcut is the Electron menu's + * `CmdOrCtrl+T` accelerator (`main.ts`); a page served by `biorouter serve` never receives it, and + * in a Windows or Linux browser Ctrl+T opens the browser's own tab. The final check pressed both + * chords on the browser surface and nothing opened, under a row that read "Ctrl+T". The renderer + * gets no Ctrl+T handler of its own: that chord belongs to the browser. + */ +describe('New chat’s shortcut hint', () => { + afterEach(() => { + delete document.documentElement.dataset.biorouterSurface; + }); + + it('is not drawn on the browser surface, where no shortcut opens a chat', async () => { + document.documentElement.dataset.biorouterSurface = BROWSER_SURFACE_MARKER; + render( + + + + ); + + const newChat = screen.getByTestId('sidebar-new-chat-button'); + expect(newChat).toHaveTextContent(/^New chat$/); + expect(newChat.querySelector('.br-nav-row-hint')).toBeNull(); + expect(screen.queryByText('Ctrl+T')).toBeNull(); + expect(screen.queryByText('⌘T')).toBeNull(); + expect(await screen.findByTestId('recent-chat-session-1')).toBeInTheDocument(); + }); + + it('is drawn in the desktop app, where the menu accelerator opens a chat', async () => { + render( + + + + ); + + const hint = screen.getByTestId('sidebar-new-chat-button').querySelector('.br-nav-row-hint'); + expect(hint).not.toBeNull(); + expect(hint).toHaveTextContent(window.electron?.platform === 'darwin' ? '⌘T' : 'Ctrl+T'); + expect(await screen.findByTestId('recent-chat-session-1')).toBeInTheDocument(); + }); +}); diff --git a/ui/desktop/src/components/BioRouterSidebar/AppSidebar.tsx b/ui/desktop/src/components/BioRouterSidebar/AppSidebar.tsx index d4dcdecc3..1a9daeae8 100644 --- a/ui/desktop/src/components/BioRouterSidebar/AppSidebar.tsx +++ b/ui/desktop/src/components/BioRouterSidebar/AppSidebar.tsx @@ -26,10 +26,11 @@ import { Badge } from '../ui/badge'; import { attentionBadgeText } from '../crew/attention/crewAttention'; import { useCrewAttention } from '../crew/attention/useCrewAttention'; import RecentChats from './RecentChats'; -import useSidebarSessions from './useSidebarSessions'; +import useSidebarSessions, { sessionsInTurn } from './useSidebarSessions'; import { sidebarCopy } from './copy'; import { isDefaultSidebarChatView } from './sidebarChatView'; import { useSidebarChatView } from './useSidebarChatView'; +import { isBrowserSurface } from '../../utils/surface'; import './sidebar.css'; function preloadHome(): void { @@ -172,9 +173,13 @@ const AppSidebar: React.FC = ({ currentPath }) => { // Any view but the default sorts or groups the whole list, so it reads every // page first (spec 3.4 "Paging"); the default keeps lazy 10-row paging. const currentSessionId = currentPath === '/pair' ? searchParams.get('resumeSessionId') : null; + // A chat whose turn starts moves to the top under "Last activity" then, not + // when the turn finishes (see `turnSessionIds`). + const turnSessionIds = useMemo(() => sessionsInTurn(runningChats), [runningChats]); const { sessions, hasMore, isLoading, isLoadingAll, loadMore } = useSidebarSessions({ loadAll: !isDefaultSidebarChatView(chatView), openSessionId: currentSessionId, + turnSessionIds, }); const runningSessionIds = useMemo( () => @@ -183,6 +188,7 @@ const AppSidebar: React.FC = ({ currentPath }) => { ); const homeDir = (window.appConfig?.get('BIOROUTER_HOME_DIR') as string | undefined) ?? null; const isMac = window.electron?.platform === 'darwin'; + const showShortcutHint = !isBrowserSurface(); useEffect(() => { const currentItem = [...primaryItems, ...componentItems, settingsItem].find( @@ -320,7 +326,12 @@ const AppSidebar: React.FC = ({ currentPath }) => { check-contrast.mjs can assert it on every row fill. */} {entry.label} - {isAction ? ( + {/* The shortcut is the Electron menu's accelerator, which a page + served by `biorouter serve` never receives (and in a Windows or + Linux browser Ctrl+T is the browser's own new tab), so the + browser surface draws no hint rather than one that does nothing + (SD-8). */} + {isAction && showShortcutHint ? ( diff --git a/ui/desktop/src/components/BioRouterSidebar/useSidebarSessions.ts b/ui/desktop/src/components/BioRouterSidebar/useSidebarSessions.ts index 23aa4aaf9..3db8970aa 100644 --- a/ui/desktop/src/components/BioRouterSidebar/useSidebarSessions.ts +++ b/ui/desktop/src/components/BioRouterSidebar/useSidebarSessions.ts @@ -1,5 +1,7 @@ import { useCallback, useEffect, useRef, useState } from 'react'; import { listSidebarSessions, type SessionSummary } from '../../api'; +import type { RunningChatEntry } from '../../hooks/chatStreamStore'; +import { ChatState } from '../../types/chatState'; import { userActionHeaders } from '../../utils/userAction'; import { laterActivity, subscribeSessionNameChanges } from '../../utils/sessionNameSync'; import { subscribeSessionListChanges, subscribeSessionRemoved } from '../../utils/sessionListCache'; @@ -31,6 +33,16 @@ export const SIDEBAR_SESSION_LOAD_ALL_CAP = 2_000; */ const OPEN_REFRESH_FOLLOW_UP_MS = 2_000; +/** + * How long after a turn starts in a chat this list has not read yet it reads + * the head a second time. The first read follows the start by 250 ms, and the + * daemon can store the person's message after it (a cold chat waits for its + * extensions first), so the chat is not yet first in the daemon's order. + */ +const TURN_START_REFRESH_FOLLOW_UP_MS = 2_000; + +const NO_TURNS: ReadonlySet = new Set(); + export interface SidebarSessionsOptions { /** * Read every chat, not just the pages scrolled into view. The server pages in @@ -63,6 +75,59 @@ export interface SidebarSessionsOptions { * `activityAt`). Leaving for a page that is not a chat reads nothing. */ openSessionId?: string | null; + /** + * The chats with a turn in progress in this window ({@link sessionsInTurn}). + * Running a turn is activity (R-30), so under "Last activity" a running chat + * moves to the top, and it moves when the turn STARTS: the list raises the + * row's `updated_at` to the moment it saw the turn begin, with no request, + * as a saved rename does. The list used to re-read only when a turn + * finished, so a running chat sat where it was, spinner and all, for the + * whole turn (final check, 2026-10-12). The raise is kept over every page + * read while the turn runs, because a head page the daemon answered before + * it stored the person's message carries the old time; when the turn ends it + * is dropped, and the read that follows a finished turn carries the daemon's + * own time. A chat the list has not read yet is brought in by a head read. + * The move still waits while the pointer is over the list (RecentChats' + * hold, `applyHeldArrangement`). + */ + turnSessionIds?: ReadonlySet; +} + +/** + * The chats with a turn in progress, from the running registry's entries: not + * finished, and not restarting the agent. A restart (a folder change) draws the + * spinner but is not a turn. Whether a folder change stamps the chat is the + * daemon's to decide (R-30: it does for an empty chat), so the list leaves + * that row to the daemon's own order rather than raising it here. + */ +export function sessionsInTurn(entries: readonly RunningChatEntry[]): Set { + return new Set( + entries + .filter((entry) => !entry.completedAt && entry.chatState !== ChatState.RestartingAgent) + .map((entry) => entry.sessionId) + ); +} + +/** + * Raise each running chat's `updated_at` to its turn's start (the later of the + * two), so a page read before the daemon stored the turn's message does not + * move the chat back down. Returns `rows` itself when nothing changes. + */ +export function applyTurnRaises( + rows: SessionSummary[], + raises: ReadonlyMap +): SessionSummary[] { + if (raises.size === 0) return rows; + let changed = false; + const next = rows.map((row) => { + const raise = raises.get(row.id); + if (!raise) return row; + const updatedAt = laterActivity(row.updated_at, raise); + if (updatedAt === row.updated_at) return row; + changed = true; + return { ...row, updated_at: updatedAt }; + }); + return changed ? next : rows; } export interface SidebarSessionsState { @@ -137,6 +202,7 @@ export default function useSidebarSessions( ): SidebarSessionsState { const loadAll = options.loadAll ?? false; const openSessionId = options.openSessionId ?? null; + const turnSessionIds = options.turnSessionIds ?? NO_TURNS; const [sessions, setSessions] = useState([]); const [hasMore, setHasMore] = useState(true); const [isLoading, setIsLoading] = useState(true); @@ -170,6 +236,10 @@ export default function useSidebarSessions( // the rename carries the chat's old name and, for a saved rename, its old // `updated_at`; merging it as it is would put both back (R-30). const namesDuringLoadRef = useRef(new Map()); + // The chats with a turn running, by id, with the time the list saw the turn + // start (see `turnSessionIds`). Applied to every page as it is merged. + const turnRaisesRef = useRef(new Map()); + const turnFollowUpRef = useRef(undefined); // Read by `loadPage` at request time, so a view change takes effect on the // next page without re-creating the loader (and its subscriptions). const pageSizeRef = useRef(loadAll ? SIDEBAR_SESSION_FULL_PAGE_SIZE : SIDEBAR_SESSION_PAGE_SIZE); @@ -179,6 +249,10 @@ export default function useSidebarSessions( mountedRef.current = true; return () => { mountedRef.current = false; + if (turnFollowUpRef.current !== undefined) { + window.clearTimeout(turnFollowUpRef.current); + turnFollowUpRef.current = undefined; + } }; }, []); @@ -208,9 +282,12 @@ export default function useSidebarSessions( const page = response.data; const mergedSessions = appendSessionPage( sessionsRef.current, - applyNamesDuringLoad( - settleRowsReadDuringFetch(page.sessions, rowsReadDuringLoadRef.current, false), - namesDuringLoadRef.current + applyTurnRaises( + applyNamesDuringLoad( + settleRowsReadDuringFetch(page.sessions, rowsReadDuringLoadRef.current, false), + namesDuringLoadRef.current + ), + turnRaisesRef.current ) ); const pageHasMore = @@ -407,6 +484,42 @@ export default function useSidebarSessions( return () => window.clearTimeout(followUp); }, [openSessionId]); + // A turn started in a chat: raise its row now (see `turnSessionIds`). The set + // is rebuilt whenever a running entry changes (thinking, then streaming), so + // a start is an id the list holds no raise for, not a new set. + useEffect(() => { + const raises = turnRaisesRef.current; + for (const sessionId of [...raises.keys()]) { + if (!turnSessionIds.has(sessionId)) raises.delete(sessionId); + } + const started = [...turnSessionIds].filter((sessionId) => !raises.has(sessionId)); + if (started.length === 0) return; + const at = new Date().toISOString(); + for (const sessionId of started) raises.set(sessionId, at); + + if (pendingAllRef.current) { + pendingAllRef.current = applyTurnRaises(pendingAllRef.current, raises); + } + const raised = applyTurnRaises(sessionsRef.current, raises); + if (raised !== sessionsRef.current) { + sessionsRef.current = raised; + setSessions(raised); + } + + // A chat past the pages read so far: the head read brings it in once the + // daemon lists it first, which it does when it has stored the message. + const listed = (sessionId: string) => + sessionsRef.current.some((session) => session.id === sessionId) || + (pendingAllRef.current?.some((session) => session.id === sessionId) ?? false); + if (started.every(listed)) return; + scheduleRefreshRef.current(); + if (turnFollowUpRef.current !== undefined) window.clearTimeout(turnFollowUpRef.current); + turnFollowUpRef.current = window.setTimeout(() => { + turnFollowUpRef.current = undefined; + scheduleRefreshRef.current(); + }, TURN_START_REFRESH_FOLLOW_UP_MS); + }, [turnSessionIds]); + const loadMore = useCallback(() => { void loadPage(false); }, [loadPage]); @@ -446,9 +559,12 @@ export default function useSidebarSessions( const page = response.data; pendingAllRef.current = appendSessionPage( pendingAllRef.current ?? [], - applyNamesDuringLoad( - settleRowsReadDuringFetch(page.sessions, rowsReadDuringLoadRef.current, false), - namesDuringLoadRef.current + applyTurnRaises( + applyNamesDuringLoad( + settleRowsReadDuringFetch(page.sessions, rowsReadDuringLoadRef.current, false), + namesDuringLoadRef.current + ), + turnRaisesRef.current ) ); cursor = page.next_cursor ?? null; diff --git a/ui/desktop/src/components/BioRouterSidebar/useSidebarSessions.turnActivity.test.tsx b/ui/desktop/src/components/BioRouterSidebar/useSidebarSessions.turnActivity.test.tsx new file mode 100644 index 000000000..99a8e633e --- /dev/null +++ b/ui/desktop/src/components/BioRouterSidebar/useSidebarSessions.turnActivity.test.tsx @@ -0,0 +1,277 @@ +/** + * Under "Last activity" a running chat moves to the top (sidebar-chat-list.md, "Rows hold + * still under the pointer"; R-30: running a turn IS activity). + * + * The final check (2026-10-12) sent a turn in the third chat and watched it for 40 s: the daemon + * had already raised the chat's `updated_at`, the pointer was off the list, no menu or rename was + * open, and the row stayed third with its spinner until the turn FINISHED. The list re-read on a + * finished turn, a new chat, a session-meta change, focus and visibility, and the running set only + * drew the spinner. While opening a chat bumped it (before R-30), the open hid this; now it shows. + * + * These drive the sidebar's own list hook and the real row list against a fake daemon. + */ +import { act, fireEvent, render, renderHook, waitFor } from '@testing-library/react'; +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { SessionSummary } from '../../api'; +import { ChatState } from '../../types/chatState'; +import { SidebarProvider } from '../ui/sidebar'; +import RecentChats from './RecentChats'; +import { arrangeSidebarChats, DEFAULT_SIDEBAR_CHAT_VIEW } from './sidebarChatView'; +import useSidebarSessions, { sessionsInTurn } from './useSidebarSessions'; + +const daemon = vi.hoisted(() => ({ + rows: [] as SessionSummary[], + listSidebarSessions: vi.fn(), +})); + +vi.mock('../../api', () => ({ + listSidebarSessions: daemon.listSidebarSessions, + updateSessionName: vi.fn(), +})); + +vi.mock('../../utils/userAction', () => ({ + userActionHeaders: async () => ({ 'X-User-Action': 'test-proof' }), +})); + +vi.mock('../../toasts', () => ({ toastSuccess: vi.fn(), toastError: vi.fn() })); + +beforeAll(() => { + Object.defineProperty(window, 'matchMedia', { + writable: true, + value: vi.fn().mockImplementation((query: string) => ({ + matches: false, + media: query, + onchange: null, + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + addListener: vi.fn(), + removeListener: vi.fn(), + dispatchEvent: vi.fn(), + })), + }); +}); + +/** The daemon's order: `updated_at`, newest first. */ +function headOf(limit = 10) { + const sorted = [...daemon.rows].sort( + (left, right) => Date.parse(right.updated_at) - Date.parse(left.updated_at) + ); + return { + data: { + sessions: sorted.slice(0, limit).map((row) => ({ ...row })), + has_more: sorted.length > limit, + next_cursor: sorted.length > limit ? 'next' : null, + }, + }; +} + +function chat(letter: string, minutesAgo: number): SessionSummary { + const at = new Date(Date.now() - minutesAgo * 60_000).toISOString(); + return { + id: `session-${letter}`, + name: `Chat ${letter}`, + working_dir: '/workspace', + created_at: at, + updated_at: at, + message_count: 2, + user_set_name: false, + }; +} + +/** The daemon stores a message in the chat, which raises its `updated_at`. */ +function daemonStamps(sessionId: string) { + const row = daemon.rows.find((candidate) => candidate.id === sessionId); + if (row) row.updated_at = new Date().toISOString(); +} + +const NO_TURNS: ReadonlySet = new Set(); + +beforeEach(() => { + vi.clearAllMocks(); + window.localStorage.clear(); + daemon.rows = [chat('a', 10), chat('b', 20), chat('c', 30)]; + daemon.listSidebarSessions.mockImplementation(async ({ query }: { query: { limit: number } }) => + headOf(query.limit) + ); +}); + +afterEach(() => { + vi.useRealTimers(); +}); + +function SidebarList({ turns = NO_TURNS }: { turns?: ReadonlySet }) { + const { sessions, hasMore, isLoading, isLoadingAll, loadMore } = useSidebarSessions({ + turnSessionIds: turns, + }); + return ( + + + + ); +} + +const order = () => + Array.from(document.querySelectorAll('[data-row^="chat:"]')).map((row) => + row.getAttribute('data-row') + ); + +const asLoaded = ['chat:session-a', 'chat:session-b', 'chat:session-c']; +const cFirst = ['chat:session-c', 'chat:session-a', 'chat:session-b']; + +describe('a running chat moves to the top while its turn runs', () => { + it('moves the chat the moment its turn starts, not when the turn finishes', async () => { + const view = render(); + await waitFor(() => expect(order()).toEqual(asLoaded)); + const readsBefore = daemon.listSidebarSessions.mock.calls.length; + + // The person sends in the oldest chat. The turn is running; nothing has finished. + view.rerender(); + + expect(order()).toEqual(cFirst); + // Moved from the turn starting, without waiting on a re-read of the list. + expect(daemon.listSidebarSessions).toHaveBeenCalledTimes(readsBefore); + + // A later render with an equal set (the turn went from thinking to streaming) is not a + // second start, and the chat stays where it is. + view.rerender(); + expect(order()).toEqual(cFirst); + }); + + it('waits while the pointer is over the list, and moves when it leaves', async () => { + const view = render(); + await waitFor(() => expect(order()).toEqual(asLoaded)); + const list = view.getByTestId('recent-chat-scroll'); + + fireEvent.pointerEnter(list); + view.rerender(); + expect(order()).toEqual(asLoaded); + + fireEvent.pointerLeave(list); + expect(order()).toEqual(cFirst); + }); + + it('keeps the chat on top over a head read the daemon answered before it stamped the turn', async () => { + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + const view = render(); + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + expect(order()).toEqual(asLoaded); + + view.rerender(); + expect(order()).toEqual(cFirst); + + // The window comes back to the front mid-turn, and the head page answers with the order + // from before the daemon stored the person's message. + const readsBefore = daemon.listSidebarSessions.mock.calls.length; + act(() => { + window.dispatchEvent(new Event('focus')); + }); + await act(async () => { + await vi.advanceTimersByTimeAsync(300); + }); + expect(daemon.listSidebarSessions.mock.calls.length).toBeGreaterThan(readsBefore); + expect(order()).toEqual(cFirst); + }); + + it('lets go when the turn ends, and the daemon’s order stands', async () => { + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + const view = render(); + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + + // A turn in the second chat is refused before the daemon stores anything. + view.rerender(); + expect(order()).toEqual(['chat:session-b', 'chat:session-a', 'chat:session-c']); + view.rerender(); + act(() => { + window.dispatchEvent(new Event('message-stream-finished')); + }); + await act(async () => { + await vi.advanceTimersByTimeAsync(300); + }); + expect(order()).toEqual(asLoaded); + + // A turn in the oldest chat that the daemon stores stays on top after it ends. + view.rerender(); + daemonStamps('session-c'); + view.rerender(); + act(() => { + window.dispatchEvent(new Event('message-stream-finished')); + }); + await act(async () => { + await vi.advanceTimersByTimeAsync(300); + }); + expect(order()).toEqual(cFirst); + }); +}); + +describe('the list hook and a turn in a chat it has not read yet', () => { + const firstUnderLastActivity = (sessions: SessionSummary[]) => + arrangeSidebarChats(sessions, DEFAULT_SIDEBAR_CHAT_VIEW)[0]?.sessions[0]?.id; + + it('reads the head, which brings the running chat in at the top', async () => { + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + daemon.rows = Array.from({ length: 12 }, (_, index) => + chat(String.fromCharCode(97 + index), (index + 1) * 10) + ); + const { result, rerender } = renderHook( + ({ turns }: { turns: ReadonlySet }) => useSidebarSessions({ turnSessionIds: turns }), + { initialProps: { turns: NO_TURNS } } + ); + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + expect(result.current.sessions.map((session) => session.id)).not.toContain('session-l'); + + // A turn starts in a chat past the first page (opened from History, say), and the daemon + // stores the message a moment later than the first head read. + rerender({ turns: new Set(['session-l']) }); + await act(async () => { + await vi.advanceTimersByTimeAsync(300); + }); + daemonStamps('session-l'); + await act(async () => { + await vi.advanceTimersByTimeAsync(2_500); + }); + + expect(firstUnderLastActivity(result.current.sessions)).toBe('session-l'); + }); +}); + +describe('sessionsInTurn', () => { + const entry = (sessionId: string, chatState: ChatState, completedAt?: number) => ({ + sessionId, + title: sessionId, + chatState, + startedAt: 1, + completedAt, + }); + + it('counts a running turn, and not a finished one or an agent restart', () => { + const turns = sessionsInTurn([ + entry('thinking', ChatState.Thinking), + entry('streaming', ChatState.Streaming), + entry('asking', ChatState.WaitingForUserInput), + entry('compacting', ChatState.Compacting), + entry('finished', ChatState.Idle, 2), + // A folder change restarts the agent: the spinner shows, but it is not a turn, and + // whether it stamps the chat is the daemon's to decide (R-30). + entry('restarting', ChatState.RestartingAgent), + ]); + expect([...turns].sort()).toEqual(['asking', 'compacting', 'streaming', 'thinking']); + }); +}); diff --git a/ui/desktop/src/components/MarkdownContent.test.tsx b/ui/desktop/src/components/MarkdownContent.test.tsx index 6c53072ed..fb2523a83 100644 --- a/ui/desktop/src/components/MarkdownContent.test.tsx +++ b/ui/desktop/src/components/MarkdownContent.test.tsx @@ -1326,12 +1326,53 @@ Another very long URL: https://www.example.com/very/long/path/with/many/segments expect(region).toHaveAttribute('tabindex', '0'); }); + // The fade is never wider than what is still out of view (`--br-overflow-fade`, + // read by the mask in main.css). Final check, 2026-10-12: a table 475px wide + // in a 465px box had 10px past its edge and a 40px fade, which dimmed a + // right-aligned last column ("Value", "ls -la", "2") nearly to nothing. + const fadeOf = (region: HTMLElement) => region.style.getPropertyValue('--br-overflow-fade'); + + it('sizes the fade to the 10px a nearly fitting table has left, shrinking as it scrolls', () => { + vi.spyOn(HTMLElement.prototype, 'scrollWidth', 'get').mockReturnValue(475); + vi.spyOn(HTMLElement.prototype, 'clientWidth', 'get').mockReturnValue(465); + render( + + ); + const region = screen.getByRole('region', { name: transcriptCopy.code.tableRegion }); + expect(region).toHaveAttribute('data-overflow', 'true'); + expect(fadeOf(region)).toBe('10px'); + region.scrollLeft = 4; + fireEvent.scroll(region); + expect(fadeOf(region)).toBe('6px'); + region.scrollLeft = 10; + fireEvent.scroll(region); + expect(region).not.toHaveAttribute('data-overflow'); + expect(fadeOf(region)).toBe(''); + }); + + it('caps the fade at 40px while more than that lies past the edge', () => { + layoutWidths(true); + render(); + const region = screen.getByRole('region', { name: transcriptCopy.code.tableRegion }); + expect(fadeOf(region)).toBe('40px'); + // 900 wide in 400: at 480 there are 20px left, so the fade is 20px. + region.scrollLeft = 480; + fireEvent.scroll(region); + expect(fadeOf(region)).toBe('20px'); + }); + it('does the same for a fenced block that scrolls', async () => { layoutWidths(true); render(); const region = await screen.findByRole('region', { name: transcriptCopy.code.region }); expect(region).toHaveClass('br-md-code-body'); expect(region).toHaveAttribute('data-overflow', 'true'); + expect(fadeOf(region)).toBe('40px'); }); it('leaves a block that fits as a plain box', async () => { diff --git a/ui/desktop/src/components/MarkdownContent.tsx b/ui/desktop/src/components/MarkdownContent.tsx index 033430284..fe4305b6b 100644 --- a/ui/desktop/src/components/MarkdownContent.tsx +++ b/ui/desktop/src/components/MarkdownContent.tsx @@ -321,7 +321,7 @@ const CodeBlock = memo(function CodeBlock({ {/* A region a keyboard can reach only while it scrolls (Crew's rule), and - the right-edge fade while there is more to its right. */} + the right-edge fade while there is more to its right, sized to it. */}
) * radius 8 with no frame, rules between rows only, a muted 12px header, 13px * tabular cells. The scroll box is a * region a keyboard can reach only while the table is wider than the column, - * and fades at its right edge while there is more to see. + * and fades at its right edge while there is more to see, the fade no wider + * than what is still past the edge (`--br-overflow-fade`), so a table that + * nearly fits keeps its last column readable. */ function MarkdownTable({ node: _node, className, ...props }: SlotProps<'table'>) { const [measure, overflow] = useOverflowsSideways(); diff --git a/ui/desktop/src/components/ToolCallConfirmation.test.tsx b/ui/desktop/src/components/ToolCallConfirmation.test.tsx index 3b68964b7..0113f1160 100644 --- a/ui/desktop/src/components/ToolCallConfirmation.test.tsx +++ b/ui/desktop/src/components/ToolCallConfirmation.test.tsx @@ -494,3 +494,107 @@ describe('ToolCallConfirmation shape (spec 3.6 cards)', () => { expect(screen.queryByRole('button')).toBeNull(); }); }); + +/** + * Final check, /bug (2026-10-12): the report approval card promised a body the + * person could not read. + * + * `platform__report_bug` sends its arguments as one `arguments` preview, and the + * body (the text that becomes a world-readable GitHub issue) arrived as a + * single escaped JSON line: 7,662 characters in a 724px box, about 90 of them + * visible, with literal `\n`. The card above it says "Read the body below". + */ +describe('ToolCallConfirmation for a /bug report (final check)', () => { + const body = + '**Describe the bug**\n\nIn this chat, the assistant created a new markdown file.\n\n' + + '**Steps to reproduce**\n\n1. Open a chat.\n2. Ask for a file.\n\n' + + 'The summary rail kept saying +0 -0 after the file was written. '.repeat(100); + const args = { + repository: 'github.com/BaranziniLab/biorouter', + title: 'Chat summary Code delta stays +0 -0 after file creation', + body, + labels: ['bug'], + chatPrivacyTier: 'private', + }; + const reportCard = (overrides: Parameters[0] = {}) => + renderCard({ + toolName: 'platform__report_bug', + risk: 'high', + arguments: args, + preview: { kind: 'arguments', json: JSON.stringify(args, null, 2), truncated: false }, + prompt: 'Publish this bug report to the public issue tracker? Read the body below.', + ...overrides, + }); + + it('shows the body as the exact text, with real line breaks, in the wrapping text style', () => { + reportCard(); + const shown = screen.getByTestId('tool-preview-value-body'); + // The exact text that will be published: real newlines, no JSON escapes. + expect(shown.textContent).toBe(body); + expect(shown.textContent).not.toContain('\\n'); + // Wrapping is authored CSS (tool-call.css), not a utility class. + expect(shown).toHaveClass('br-tool-preview-text'); + expect(screen.getByTestId('tool-preview-value-title')).toHaveTextContent(args.title); + // Every field is named, in the order the backend sent them. + expect( + screen.getAllByTestId('tool-preview-field').map((f) => f.querySelector('dt')!.textContent) + ).toEqual(['repository', 'title', 'body', 'labels', 'chatPrivacyTier']); + // A value that is not a string stays JSON, so `["bug"]` is not read as prose. + expect(screen.getByTestId('tool-preview-value-labels').textContent).toBe('[\n "bug"\n]'); + }); + + it('collapses a long body behind "Show more" so the decision stays in reach', () => { + reportCard(); + expect(screen.getByTestId('tool-preview-body').className).toContain('max-h-52'); + fireEvent.click(screen.getByRole('button', { name: 'Show more' })); + expect(screen.getByTestId('tool-preview-body').className).not.toContain('max-h-52'); + }); + + it('collapses a single long line, which a newline count alone would leave open', () => { + const oneLine = { body: 'x'.repeat(4000) }; + renderCard({ + toolName: 'platform__report_bug', + preview: { kind: 'arguments', json: JSON.stringify(oneLine, null, 2), truncated: false }, + }); + expect(screen.getByRole('button', { name: 'Show more' })).toBeInTheDocument(); + }); + + it('wraps a clipped argument blob it cannot parse, and still says it was clipped', () => { + const json = JSON.stringify(args, null, 2).slice(0, 300); + reportCard({ preview: { kind: 'arguments', json, truncated: true } }); + expect(screen.queryByTestId('tool-preview-field')).toBeNull(); + const raw = screen.getByTestId('tool-preview-raw'); + expect(raw.textContent).toBe(json); + expect(raw).toHaveClass('br-tool-preview-text'); + expect(screen.getByText('Truncated')).toBeInTheDocument(); + }); + + it('draws one warning glyph: a leading ⚠ in the prompt is not repeated beside the icon', () => { + for (const glyph of ['⚠ ', '\u26A0\uFE0F ', ' ⚠ ']) { + const { unmount } = reportCard({ + prompt: `${glyph}This chat is PRIVATE: it ran on a private model. Read the body below.`, + }); + const finding = screen.getByTestId('tool-security-finding'); + expect(finding.textContent).toBe( + 'This chat is PRIVATE: it ran on a private model. Read the body below.' + ); + // The icon is the one glyph. + expect(finding.querySelectorAll('svg')).toHaveLength(1); + // Still a finding: "Always allow" stays withheld. + expect(screen.queryByRole('button', { name: /Always allow/i })).toBeNull(); + unmount(); + } + }); + + it('keeps a warning glyph that is not the lead, and a prompt that is only a glyph', () => { + const { unmount } = reportCard({ + prompt: 'Publish this? ⚠ "Describe the bug" is YOUR last message.', + }); + expect(screen.getByTestId('tool-security-finding').textContent).toBe( + 'Publish this? ⚠ "Describe the bug" is YOUR last message.' + ); + unmount(); + reportCard({ prompt: '⚠' }); + expect(screen.getByTestId('tool-security-finding').textContent).toBe('⚠'); + }); +}); diff --git a/ui/desktop/src/components/ToolCallConfirmation.tsx b/ui/desktop/src/components/ToolCallConfirmation.tsx index c4b5db465..4f9074e4f 100644 --- a/ui/desktop/src/components/ToolCallConfirmation.tsx +++ b/ui/desktop/src/components/ToolCallConfirmation.tsx @@ -30,6 +30,26 @@ function refusalIsPermanent(error: unknown): boolean { ); } +/** A leading warning sign (with or without its emoji selector) and the space after it. */ +const LEADING_WARNING_GLYPH = /^\s*\u26A0\uFE0F?\s*/u; + +/** + * The finding as the card shows it, beside its own warning icon. + * + * A producer may lead its prompt with "⚠ ", because the terminal prints the + * prompt as bare text and that glyph is its only warning sign there: + * `bug_report::private_warning` does ("⚠ This chat is PRIVATE: …"). Drawn + * after the card's icon it made two warning signs side by side (final check, + * 2026-10-12), so the card drops a LEADING one and keeps the rest of the text + * exactly: a ⚠ inside the sentence marks a second warning and stays. Stripping + * here rather than at the producer also fixes cards persisted by an older + * daemon. A prompt that is only the glyph keeps it, rather than going blank. + */ +function findingText(prompt: string): string { + const stripped = prompt.replace(LEADING_WARNING_GLYPH, ''); + return stripped.length > 0 ? stripped : prompt; +} + const ALLOW_ONCE = 'allow_once'; const ALWAYS_ALLOW = 'always_allow'; const DENY = 'deny'; @@ -228,7 +248,7 @@ export default function ToolConfirmation({ className="flex items-start gap-2 text-body text-text-warning" >
) : null; diff --git a/ui/desktop/src/components/ToolCallPreview.tsx b/ui/desktop/src/components/ToolCallPreview.tsx index 87844d694..f8739b75a 100644 --- a/ui/desktop/src/components/ToolCallPreview.tsx +++ b/ui/desktop/src/components/ToolCallPreview.tsx @@ -159,6 +159,106 @@ function CodeBlock({ text }: { text: string }) { ); } +/** + * Columns a wrapped line is estimated at, for deciding whether to collapse. + * The narrowest card the preview is drawn in (a chat beside an open side + * panel) holds about 70 characters of the 13px code face; under-estimating + * only costs a "Show more" that opens a little. + */ +const WRAP_COLUMNS = 64; + +/** Lines `text` takes once wrapped at {@link WRAP_COLUMNS}. */ +function wrappedLines(text: string): number { + return text + .split('\n') + .reduce((n, line) => n + Math.max(1, Math.ceil(line.length / WRAP_COLUMNS)), 0); +} + +/** + * The backend's pretty JSON as `[key, value]` pairs, or `null` when it is not + * one whole object. A clipped blob is never parsed: what is shown must be what + * was sent, and the "Truncated" note already says the rest is missing. + */ +function argumentFields(json: string, truncated: boolean): Array<[string, unknown]> | null { + if (truncated) return null; + try { + const value: unknown = JSON.parse(json); + if (value !== null && typeof value === 'object' && !Array.isArray(value)) { + return Object.entries(value); + } + } catch { + // Not JSON after all: shown as text below. + } + return null; +} + +/** + * A string is shown as itself, with real line breaks. Anything else (and an + * empty string, which would otherwise read as a missing value) stays JSON, so + * `["bug"]` is not mistaken for prose. + */ +function fieldText(value: unknown): string { + return typeof value === 'string' && value !== '' ? value : JSON.stringify(value, null, 2); +} + +/** + * The arguments of a call, as fields a person can read. + * + * Final check, /bug (2026-10-12): shown as the backend's pretty JSON in a + * `whitespace-pre` box, a string argument was one escaped line. + * `platform__report_bug`'s body, the text that becomes a world-readable issue, + * was 7,662 characters wide in a 724px box, about 90 of them visible, with + * literal `\n`, under a card saying "Read the body below". So each field is + * named, and its value is the exact text, wrapped (`.br-tool-preview-text`, + * authored in tool-call.css so it holds wherever class scanning did not run). + * The text is never rendered as Markdown: the person is checking what will be + * published, and rendering hides some of it (an HTML comment, a link's URL). + */ +function ArgumentFields({ fields }: { fields: Array<[string, unknown]> }) { + return ( +
+ {fields.map(([key, value]) => ( +
+
{key}
+
+ {fieldText(value)} +
+
+ ))} +
+ ); +} + +function ArgumentsPreview({ json, truncated }: { json: string; truncated: boolean }) { + const fields = argumentFields(json, truncated); + // One key line per field, then its wrapped value. + const lines = fields + ? fields.reduce((n, [, value]) => n + 1 + wrappedLines(fieldText(value)), 0) + : wrappedLines(json); + return ( + } + title={PREVIEW_COPY.arguments} + truncated={truncated} + collapsible={lines > COLLAPSED_LINES} + > + {fields ? ( + + ) : ( +
+          {json}
+        
+ )} +
+ ); +} + /** Render whatever the backend resolved this call into. */ export function ToolCallPreview({ preview }: { preview: ToolPreview }) { switch (preview.kind) { @@ -210,16 +310,7 @@ export function ToolCallPreview({ preview }: { preview: ToolPreview }) { ); case 'arguments': - return ( - } - title={PREVIEW_COPY.arguments} - truncated={preview.truncated} - collapsible={countLines(preview.json) > COLLAPSED_LINES} - > - - - ); + return ; default: // An older backend, or a preview kind this build does not know yet. diff --git a/ui/desktop/src/components/alerts/toastClearance.browser.test.tsx b/ui/desktop/src/components/alerts/toastClearance.browser.test.tsx index 332dda0ce..a29b901fc 100644 --- a/ui/desktop/src/components/alerts/toastClearance.browser.test.tsx +++ b/ui/desktop/src/components/alerts/toastClearance.browser.test.tsx @@ -5,14 +5,17 @@ * In the running app "Chat ID copied" sat at 1004,44 to 1284,109 at 1300 wide, over the open * find bar (844,52 to 1284,94): its count, info tip, Aa, both arrows and Close were hidden. At * 1440 the same toast covered the docked summary rail's "To do" header and progress bar. - * `toastClearance.ts` moves the stack below find and left of the summary. + * `toastClearance.ts` moves the stack below find, left of the summary popover, and into the + * docked rail's column under its card. Stepped left of the rail it had covered the person's own + * message instead (final app check, "Toasts do not cover transcript content"). * * jsdom has no layout, so the real find bar (`SearchBar`, with `styles/search.css`), the real * react-toastify container as App.tsx configures it, and the watcher App.tsx starts are bundled * with esbuild, `styles/main.css` is compiled by Tailwind (the vendored ReactToastify.css after * it, as App imports them), and Chromium measures. The rail card and the popover are boxes with * their real class names and the geometry the app gives them (a 280px rail under the 44px - * header with a 12px gutter; a 280px popover 4px under the band, 16px in from the right). + * header with a 12px gutter, its card as tall as its content or, `tall`, as the rail; a 280px + * popover 4px under the band, 16px in from the right). */ import { readFileSync } from 'node:fs'; import { createRequire } from 'node:module'; @@ -35,8 +38,8 @@ import { TooltipProvider } from '../ui/Tooltip'; import { NotificationContent, TOAST_SURFACE_CLASS_NAME } from './NotificationSurface'; import { watchToastClearance } from './toastClearance'; -type Layout = { find: boolean; rail: boolean; popover: boolean; split: boolean }; -const NONE: Layout = { find: false, rail: false, popover: false, split: false }; +type Layout = { find: boolean; rail: boolean; tall: boolean; popover: boolean; split: boolean }; +const NONE: Layout = { find: false, rail: false, tall: false, popover: false, split: false }; function Find() { return ( @@ -69,8 +72,22 @@ function Harness() { {layout.find && !layout.split && } {layout.rail && ( -