CX#7: one active cell, mutated in place

Codex pattern — instead of appending a new ToolCall history cell for each
parallel tool invocation, keep one Exploring/Searching/Reading active cell at
the tail of the transcript and mutate its contents in place as new tool calls
fire. Drops cell churn and keeps the visual anchor stable while multiple tools
stream concurrently.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Hunter Bown
2026-04-25 23:13:57 -05:00
parent 585dd2f7d0
commit 63d7391ff8
6 changed files with 1218 additions and 123 deletions
+375
View File
@@ -0,0 +1,375 @@
//! Active in-flight tool/exec cell — single mutable group that buffers parallel
//! tool work for the current turn.
//!
//! ## Why
//!
//! When the model issues parallel tool calls in a single assistant turn (e.g.
//! two `read_file` and one `grep_files` running concurrently), naively
//! appending each tool start as its own history cell makes the transcript
//! "bounce" as completions arrive out of order. Codex's pattern is to keep all
//! in-flight tool work in ONE active cell that mutates in place; once the turn
//! resolves the active cell finalizes into the transcript.
//!
//! ## Contract
//!
//! - At most one [`ActiveCell`] per turn. It holds zero or more
//! [`HistoryCell`]s that are still being mutated (status `Running`, output
//! pending, etc.).
//! - The owning [`crate::tui::app::App`] renders the active cell's contents
//! AFTER `App.history` so they appear at the live tail.
//! - Cell indices used by helpers like `tool_cells` / `tool_details_by_cell`
//! address the virtual sequence `App.history ++ active_cell.entries`. Each
//! entry's index is `App.history.len() + entry_offset`.
//! - When a tool completes whose `tool_id` does not match any active entry
//! (orphan), the caller pushes a finalized standalone cell into `App.history`
//! instead of mutating the active group. This keeps `active_cell` a stable
//! reflection of what was actually started, and avoids merging unrelated
//! tool work.
//! - On `TurnComplete` (or cancellation) the active cell is "flushed":
//! in-progress entries are marked with the supplied terminal status, then
//! every entry is appended to `App.history`. Companion maps
//! (`tool_cells`, `tool_details_by_cell`) are rewritten to point at the new
//! `App.history` indices.
//!
//! ## Revision counter
//!
//! Cells inside the active group mutate without changing pointer identity, so
//! the transcript cache cannot rely on enum-equality for invalidation. We
//! expose `revision()` and `bump_revision()`; the renderer combines this with
//! `App.history_version` when computing per-cell revisions for the cache.
use crate::tui::history::{ExploringCell, ExploringEntry, HistoryCell, ToolCell, ToolStatus};
/// In-flight active cell: a sequence of mutable [`HistoryCell`] entries.
///
/// Conceptually a single "live tail" cell in the Codex sense: it appears as
/// one logical block at the end of the transcript, but internally it is
/// composed of one or more entries (each rendered as its own
/// [`HistoryCell`]). The reason we keep them as separate entries — rather
/// than fusing into a single conceptual block — is that they may have
/// different shapes (an `ExecCell`, an `ExploringCell` aggregate, an MCP
/// tool result, …) and the existing renderers already know how to draw each
/// shape correctly. Coalescing into a single render path would duplicate
/// logic we already have.
#[derive(Debug, Clone, Default)]
pub struct ActiveCell {
entries: Vec<HistoryCell>,
/// Tool ids currently associated with this active cell. The map values are
/// indices into [`Self::entries`]. Multiple tool ids can map to the same
/// entry (the existing `ExploringCell` aggregates several reads into a
/// single entry).
tool_to_entry: std::collections::HashMap<String, usize>,
/// Index of the current `ExploringCell` entry (when present), so additional
/// exploring tool starts append to it instead of creating new cells.
exploring_entry: Option<usize>,
/// Bumped on every mutation. Used by the transcript cache to know that
/// the active cell needs re-rendering even though its position in the
/// virtual cell list is unchanged.
revision: u64,
}
impl ActiveCell {
/// Create an empty active cell.
#[must_use]
pub fn new() -> Self {
Self::default()
}
/// Number of entries (each rendered as its own [`HistoryCell`]).
#[must_use]
#[allow(dead_code)] // Public surface used by tests and future renderers.
pub fn entry_count(&self) -> usize {
self.entries.len()
}
/// Whether the active cell has any entries.
#[must_use]
pub fn is_empty(&self) -> bool {
self.entries.is_empty()
}
/// Read-only access to the underlying entries (for rendering).
#[must_use]
pub fn entries(&self) -> &[HistoryCell] {
&self.entries
}
/// Mutable access to a specific entry. Bumps the revision counter so the
/// renderer knows the cached lines are stale.
pub fn entry_mut(&mut self, index: usize) -> Option<&mut HistoryCell> {
if index < self.entries.len() {
self.bump_revision();
self.entries.get_mut(index)
} else {
None
}
}
/// Current revision counter. Wraps on overflow which is fine for cache
/// invalidation; the chance of a wrap-around collision is astronomical
/// over a single session and any miss only causes one extra re-render.
#[must_use]
#[allow(dead_code)] // Used by App::bump_active_cell_revision and future cache wiring.
pub fn revision(&self) -> u64 {
self.revision
}
/// Increment the revision counter. Call any time an entry is mutated.
pub fn bump_revision(&mut self) {
self.revision = self.revision.wrapping_add(1);
}
/// Add a tool entry to the active cell.
///
/// Returns the entry index (which the caller can record in
/// `tool_cells_in_active`). If the cell is an exploring tool start and
/// there is already an exploring entry in the active group, the entry is
/// appended to that aggregate instead of creating a new entry.
///
/// `tool_id` is registered for the new (or updated) entry so future
/// completion lookups can find it.
pub fn push_tool(&mut self, tool_id: impl Into<String>, cell: HistoryCell) -> usize {
let tool_id = tool_id.into();
// If this is an exploring start and we already have an exploring
// entry, append to that entry rather than creating a new cell.
if let HistoryCell::Tool(ToolCell::Exploring(new_cell)) = &cell
&& let Some(entry_idx) = self.exploring_entry
&& let Some(HistoryCell::Tool(ToolCell::Exploring(existing))) =
self.entries.get_mut(entry_idx)
{
// The caller hands us a brand-new ExploringCell with one entry.
// Move that entry into the existing aggregate.
for explore_entry in &new_cell.entries {
let _ = existing.insert_entry(explore_entry.clone());
}
self.tool_to_entry.insert(tool_id, entry_idx);
self.bump_revision();
return entry_idx;
}
// Otherwise, push a new entry.
let entry_idx = self.entries.len();
if matches!(cell, HistoryCell::Tool(ToolCell::Exploring(_))) {
self.exploring_entry = Some(entry_idx);
}
self.entries.push(cell);
self.tool_to_entry.insert(tool_id, entry_idx);
self.bump_revision();
entry_idx
}
/// Push an entry with no tool id binding (used for non-tool grouping if
/// ever needed). Currently unused; kept for symmetry with Codex which
/// allows e.g. session-header cells to live in `active_cell`.
#[allow(dead_code)]
pub fn push_untracked(&mut self, cell: HistoryCell) -> usize {
let entry_idx = self.entries.len();
self.entries.push(cell);
self.bump_revision();
entry_idx
}
/// Look up the entry index that holds the given tool id.
#[must_use]
#[allow(dead_code)] // Reserved for the Codex-style "exec end target" lookup.
pub fn entry_index_for_tool(&self, tool_id: &str) -> Option<usize> {
self.tool_to_entry.get(tool_id).copied()
}
/// Append an [`ExploringEntry`] to the existing exploring aggregate (if
/// any), binding the supplied tool id to it. Returns
/// `(entry_index, entry_within_exploring)` on success.
///
/// Used when a second exploring tool starts during the same active group:
/// rather than allocating another ExploringCell entry in the active group
/// we extend the one that's already there.
pub fn append_to_exploring(
&mut self,
tool_id: impl Into<String>,
explore_entry: ExploringEntry,
) -> Option<(usize, usize)> {
let entry_idx = self.exploring_entry?;
let HistoryCell::Tool(ToolCell::Exploring(cell)) = self.entries.get_mut(entry_idx)? else {
return None;
};
let inner_idx = cell.insert_entry(explore_entry);
self.tool_to_entry.insert(tool_id.into(), entry_idx);
self.bump_revision();
Some((entry_idx, inner_idx))
}
/// Ensure an [`ExploringCell`] exists in the active group; create it if
/// not. Returns its entry index.
pub fn ensure_exploring(&mut self) -> usize {
if let Some(idx) = self.exploring_entry {
return idx;
}
let idx = self.entries.len();
self.entries
.push(HistoryCell::Tool(ToolCell::Exploring(ExploringCell {
entries: Vec::new(),
})));
self.exploring_entry = Some(idx);
self.bump_revision();
idx
}
/// Remove the tool-id binding for an entry without removing the entry
/// itself (the entry remains in the active group, presumably with its
/// status updated).
#[allow(dead_code)] // Reserved for cancellation paths that prune ids without flushing.
pub fn forget_tool(&mut self, tool_id: &str) -> Option<usize> {
self.tool_to_entry.remove(tool_id)
}
/// Drain every entry, returning them in insertion order. Resets internal
/// state (revision is bumped via `bump_revision`).
///
/// Callers use this on `TurnComplete` (or cancellation) to flush the
/// active group into `App.history`.
pub fn drain(&mut self) -> Vec<HistoryCell> {
let entries = std::mem::take(&mut self.entries);
self.tool_to_entry.clear();
self.exploring_entry = None;
self.bump_revision();
entries
}
/// Mark every still-running tool entry as `Failed` (used when the turn is
/// cancelled mid-flight). Entries that already completed are left alone.
///
/// `Failed` is the closest existing variant for "interrupted"; the cell's
/// surrounding context (turn-status banner) tells the user it was a
/// cancellation rather than a tool error.
pub fn mark_in_progress_as_interrupted(&mut self) {
for cell in &mut self.entries {
mark_running_as_interrupted(cell);
}
self.bump_revision();
}
}
fn mark_running_as_interrupted(cell: &mut HistoryCell) {
let HistoryCell::Tool(tool_cell) = cell else {
return;
};
match tool_cell {
ToolCell::Exec(exec) if exec.status == ToolStatus::Running => {
exec.status = ToolStatus::Failed;
}
ToolCell::Exploring(explore) => {
for entry in &mut explore.entries {
if entry.status == ToolStatus::Running {
entry.status = ToolStatus::Failed;
}
}
}
ToolCell::PlanUpdate(plan) if plan.status == ToolStatus::Running => {
plan.status = ToolStatus::Failed;
}
ToolCell::PatchSummary(patch) if patch.status == ToolStatus::Running => {
patch.status = ToolStatus::Failed;
}
ToolCell::Review(review) if review.status == ToolStatus::Running => {
review.status = ToolStatus::Failed;
}
ToolCell::Mcp(mcp) if mcp.status == ToolStatus::Running => {
mcp.status = ToolStatus::Failed;
}
ToolCell::WebSearch(search) if search.status == ToolStatus::Running => {
search.status = ToolStatus::Failed;
}
ToolCell::Generic(generic) if generic.status == ToolStatus::Running => {
generic.status = ToolStatus::Failed;
}
_ => {}
}
}
#[cfg(test)]
mod tests {
use super::*;
use crate::tui::history::{
ExecCell, ExecSource, ExploringCell, ExploringEntry, GenericToolCell,
};
use std::time::Instant;
fn exec_cell(command: &str) -> HistoryCell {
HistoryCell::Tool(ToolCell::Exec(ExecCell {
command: command.to_string(),
status: ToolStatus::Running,
output: None,
started_at: Some(Instant::now()),
duration_ms: None,
source: ExecSource::Assistant,
interaction: None,
}))
}
fn exploring_cell_with(label: &str) -> HistoryCell {
HistoryCell::Tool(ToolCell::Exploring(ExploringCell {
entries: vec![ExploringEntry {
label: label.to_string(),
status: ToolStatus::Running,
}],
}))
}
fn generic_cell(name: &str) -> HistoryCell {
HistoryCell::Tool(ToolCell::Generic(GenericToolCell {
name: name.to_string(),
status: ToolStatus::Running,
input_summary: None,
output: None,
}))
}
#[test]
fn push_tool_records_entry_and_revision_advances() {
let mut cell = ActiveCell::new();
let r0 = cell.revision();
let idx = cell.push_tool("t1", exec_cell("ls"));
assert_eq!(idx, 0);
assert_eq!(cell.entry_count(), 1);
assert!(cell.revision() != r0);
assert_eq!(cell.entry_index_for_tool("t1"), Some(0));
}
#[test]
fn parallel_exploring_starts_share_one_entry() {
let mut cell = ActiveCell::new();
let idx_a = cell.push_tool("a", exploring_cell_with("Read foo.rs"));
let idx_b = cell.push_tool("b", exploring_cell_with("Read bar.rs"));
assert_eq!(
idx_a, idx_b,
"both exploring starts should land in same entry"
);
assert_eq!(cell.entry_count(), 1);
let HistoryCell::Tool(ToolCell::Exploring(explore)) = &cell.entries()[0] else {
panic!("expected exploring cell")
};
assert_eq!(explore.entries.len(), 2);
}
#[test]
fn drain_resets_state_and_returns_in_order() {
let mut cell = ActiveCell::new();
cell.push_tool("a", exec_cell("ls"));
cell.push_tool("b", generic_cell("foo"));
let drained = cell.drain();
assert_eq!(drained.len(), 2);
assert!(cell.is_empty());
assert_eq!(cell.entry_index_for_tool("a"), None);
}
#[test]
fn interrupt_marks_running_entries_failed() {
let mut cell = ActiveCell::new();
cell.push_tool("a", exec_cell("ls"));
cell.mark_in_progress_as_interrupted();
let HistoryCell::Tool(ToolCell::Exec(exec)) = &cell.entries()[0] else {
panic!("expected exec")
};
assert_eq!(exec.status, ToolStatus::Failed);
}
}
+181 -3
View File
@@ -21,6 +21,7 @@ use crate::settings::Settings;
use crate::tools::plan::{SharedPlanState, new_shared_plan_state};
use crate::tools::subagent::SubAgentResult;
use crate::tools::todo::{SharedTodoList, new_shared_todo_list};
use crate::tui::active_cell::ActiveCell;
use crate::tui::approval::ApprovalMode;
use crate::tui::clipboard::{ClipboardContent, ClipboardHandler};
use crate::tui::history::{HistoryCell, TranscriptRenderOptions};
@@ -414,6 +415,10 @@ pub struct App {
pub slash_menu_selected: usize,
/// Temporary hide flag for slash menu until next input edit.
pub slash_menu_hidden: bool,
/// `@`-mention completion popup selection index in composer.
pub mention_menu_selected: usize,
/// Temporary hide flag for the @-mention popup until next input edit.
pub mention_menu_hidden: bool,
#[allow(dead_code)]
pub compact_threshold: usize,
pub max_input_history: usize,
@@ -468,13 +473,31 @@ pub struct App {
pub session_cost: f64,
/// Active skill to apply to next user message
pub active_skill: Option<String>,
/// Tool call cells by tool id
/// Tool call cells by tool id (for cells already finalized in `history`).
/// While a tool call is in flight inside `active_cell`, it is tracked by
/// `active_tool_entries` instead and migrated here at flush time.
pub tool_cells: HashMap<String, usize>,
/// Full tool input/output keyed by history cell index.
pub tool_details_by_cell: HashMap<usize, ToolDetailRecord>,
/// Active exploring cell index
/// In-flight tool/exec group for the current turn. Mutated in place as
/// parallel tool calls start and complete; flushed into `history` on
/// `TurnComplete`.
pub active_cell: Option<ActiveCell>,
/// Revision counter for `active_cell`. Combined with `active_cell.revision`
/// when feeding the transcript cache so cached lines for the synthetic
/// active-cell row are invalidated on every mutation.
pub active_cell_revision: u64,
/// Pending tool details for entries that live inside `active_cell`.
/// Keyed by tool id rather than cell index because the active cell's
/// virtual index can shift (orphan completions push real cells in
/// between). Migrated into `tool_details_by_cell` on flush.
pub active_tool_details: HashMap<String, ToolDetailRecord>,
/// Active exploring cell entry index (within `active_cell.entries`).
/// `None` once the active cell flushes or no exploring entry exists.
pub exploring_cell: Option<usize>,
/// Mapping of exploring tool ids to (cell index, entry index)
/// Mapping of exploring tool ids to `(entry index in active_cell, entry
/// within ExploringCell)`. Used to update individual exploring entries
/// when their tools complete.
pub exploring_entries: HashMap<String, (usize, usize)>,
/// Tool calls that should be ignored by the UI
pub ignored_tool_calls: HashSet<String>,
@@ -726,6 +749,8 @@ impl App {
sidebar_width_percent,
sidebar_focus,
slash_menu_selected: 0,
mention_menu_selected: 0,
mention_menu_hidden: false,
slash_menu_hidden: false,
compact_threshold,
max_input_history,
@@ -772,6 +797,9 @@ impl App {
active_skill: None,
tool_cells: HashMap::new(),
tool_details_by_cell: HashMap::new(),
active_cell: None,
active_cell_revision: 0,
active_tool_details: HashMap::new(),
exploring_cell: None,
exploring_entries: HashMap::new(),
ignored_tool_calls: HashSet::new(),
@@ -949,6 +977,144 @@ impl App {
self.needs_redraw = true;
}
/// Bump the active-cell revision counter and request a redraw.
///
/// Use this whenever an entry inside `active_cell` is mutated. The
/// transcript cache combines this counter with `history_version` to
/// produce a per-cell revision so the synthetic active-cell row can be
/// re-rendered without invalidating committed history cells.
pub fn bump_active_cell_revision(&mut self) {
self.active_cell_revision = self.active_cell_revision.wrapping_add(1);
if let Some(active) = self.active_cell.as_mut() {
active.bump_revision();
}
self.history_version = self.history_version.wrapping_add(1);
self.needs_redraw = true;
}
/// Total number of cells in the *virtual* transcript: `history.len()`
/// plus active cell entries (if any).
#[must_use]
#[allow(dead_code)] // Reserved for renderers that need a unified cell count.
pub fn virtual_cell_count(&self) -> usize {
self.history.len() + self.active_cell.as_ref().map_or(0, ActiveCell::entry_count)
}
/// The next cell index a freshly-pushed entry would occupy in the virtual
/// transcript. Used by `register_tool_cell`-style callsites that record
/// cell-index metadata before the active cell flushes to history.
#[must_use]
#[allow(dead_code)] // Reserved for the eventual merged push helper.
pub fn next_virtual_cell_index(&self) -> usize {
self.virtual_cell_count()
}
/// Resolve a virtual cell index to either a committed history cell or an
/// active-cell entry. Used by the pager / details lookup code so it can
/// transparently address still-in-flight cells.
#[must_use]
#[allow(dead_code)] // Used by the upcoming pager rewrite (read-only resolver).
pub fn cell_at_virtual_index(&self, index: usize) -> Option<&HistoryCell> {
if index < self.history.len() {
self.history.get(index)
} else {
let entry_idx = index - self.history.len();
self.active_cell
.as_ref()
.and_then(|active| active.entries().get(entry_idx))
}
}
/// Mutable variant of [`Self::cell_at_virtual_index`]. Bumps the
/// appropriate revision counter (active-cell revision when targeting an
/// in-flight entry, history version otherwise).
pub fn cell_at_virtual_index_mut(&mut self, index: usize) -> Option<&mut HistoryCell> {
if index < self.history.len() {
self.history_version = self.history_version.wrapping_add(1);
self.history.get_mut(index)
} else {
let entry_idx = index - self.history.len();
self.active_cell_revision = self.active_cell_revision.wrapping_add(1);
self.history_version = self.history_version.wrapping_add(1);
self.active_cell
.as_mut()
.and_then(|active| active.entry_mut(entry_idx))
}
}
/// Drain the active cell into history. Companion maps that reference
/// active-cell entries by virtual index (`tool_cells`,
/// `tool_details_by_cell`) are rewritten to point at the new history
/// indices. Idempotent — calling this when there is no active cell is a
/// no-op.
///
/// Caller is responsible for first marking in-progress entries with the
/// terminal status they want (e.g. via
/// [`ActiveCell::mark_in_progress_as_interrupted`]).
pub fn flush_active_cell(&mut self) {
let Some(mut active) = self.active_cell.take() else {
return;
};
if active.is_empty() {
// Reset auxiliary state regardless so a future tool can start a
// fresh active cell.
self.exploring_cell = None;
self.exploring_entries.clear();
self.active_tool_details.clear();
self.bump_active_cell_revision();
return;
}
let drained = active.drain();
let base_index = self.history.len();
// Rewrite per-tool indices that targeted entries inside the active
// group: their new home is `base_index + entry_offset`.
let mut details = std::mem::take(&mut self.active_tool_details);
for (tool_id, detail) in details.drain() {
// Try to recover the entry offset from `tool_cells`-style maps.
// Tool ids registered for active-cell entries live in
// `tool_cells` with `index = base_index_at_register_time +
// entry_offset`. After rewriting once, those indices are correct.
self.tool_details_by_cell
.entry(self.tool_cells.get(&tool_id).copied().unwrap_or(base_index))
.or_insert(detail);
}
// tool_cells already contains the virtual index. After the drain,
// history.len() == base_index + drained.len(), so any virtual index
// in [base_index, base_index + drained.len()) is now a real history
// index. No rewrite needed.
self.exploring_cell = None;
self.exploring_entries.clear();
for cell in drained {
self.history.push(cell);
}
self.history_version = self.history_version.wrapping_add(1);
self.needs_redraw = true;
let selection_has_range = self
.transcript_selection
.ordered_endpoints()
.is_some_and(|(start, end)| start != end);
if self.transcript_scroll.is_at_tail()
&& !self.transcript_selection.dragging
&& !selection_has_range
&& !self.user_scrolled_during_stream
{
self.scroll_to_bottom();
}
}
/// Mark every still-running entry in the active cell as interrupted, then
/// flush. Convenience helper for cancellation paths.
pub fn finalize_active_cell_as_interrupted(&mut self) {
if let Some(active) = self.active_cell.as_mut() {
active.mark_in_progress_as_interrupted();
}
self.flush_active_cell();
}
pub fn push_status_toast(
&mut self,
text: impl Into<String>,
@@ -1139,6 +1305,8 @@ impl App {
self.input.insert_str(byte_index, text);
self.cursor_position = cursor + char_count(text);
self.slash_menu_hidden = false;
self.mention_menu_hidden = false;
self.mention_menu_selected = 0;
self.needs_redraw = true;
}
@@ -1273,6 +1441,8 @@ impl App {
self.input.insert(byte_index, c);
self.cursor_position = cursor + 1;
self.slash_menu_hidden = false;
self.mention_menu_hidden = false;
self.mention_menu_selected = 0;
self.needs_redraw = true;
}
@@ -1285,6 +1455,8 @@ impl App {
if removed {
self.cursor_position = target;
self.slash_menu_hidden = false;
self.mention_menu_hidden = false;
self.mention_menu_selected = 0;
self.needs_redraw = true;
}
}
@@ -1299,6 +1471,8 @@ impl App {
self.cursor_position = char_count(&self.input);
}
self.slash_menu_hidden = false;
self.mention_menu_hidden = false;
self.mention_menu_selected = 0;
self.needs_redraw = true;
}
@@ -1341,6 +1515,8 @@ impl App {
// Cursor stays at the same character index (start of removed range).
self.cursor_position = cursor;
self.slash_menu_hidden = false;
self.mention_menu_hidden = false;
self.mention_menu_selected = 0;
self.needs_redraw = true;
true
}
@@ -1358,6 +1534,8 @@ impl App {
self.input.insert_str(byte_index, &text);
self.cursor_position = cursor + char_count(&text);
self.slash_menu_hidden = false;
self.mention_menu_hidden = false;
self.mention_menu_selected = 0;
self.needs_redraw = true;
true
}
+1
View File
@@ -2,6 +2,7 @@
// === Submodules ===
pub mod active_cell;
pub mod app;
pub mod approval;
pub mod clipboard;
+334 -117
View File
@@ -67,6 +67,7 @@ use crate::tui::session_picker::SessionPickerView;
use crate::tui::ui_text::{history_cell_to_text, line_to_plain, slice_text, text_display_width};
use crate::tui::user_input::UserInputView;
use super::active_cell::ActiveCell;
use super::app::{
App, AppAction, AppMode, OnboardingState, QueuedMessage, SidebarFocus, StatusToastLevel,
TaskPanelEntry, ToolDetailRecord, TuiOptions,
@@ -75,9 +76,9 @@ use super::approval::{
ApprovalMode, ApprovalRequest, ApprovalView, ElevationRequest, ElevationView, ReviewDecision,
};
use super::history::{
DiffPreviewCell, ExecCell, ExecSource, ExploringCell, ExploringEntry, GenericToolCell,
HistoryCell, McpToolCell, PatchSummaryCell, PlanStep, PlanUpdateCell, ReviewCell, ToolCell,
ToolStatus, ViewImageCell, WebSearchCell, history_cells_from_message, summarize_mcp_output,
DiffPreviewCell, ExecCell, ExecSource, ExploringEntry, GenericToolCell, HistoryCell,
McpToolCell, PatchSummaryCell, PlanStep, PlanUpdateCell, ReviewCell, ToolCell, ToolStatus,
ViewImageCell, WebSearchCell, history_cells_from_message, summarize_mcp_output,
summarize_tool_args, summarize_tool_output,
};
use super::views::{ConfigView, HelpView, ModalKind, ViewEvent};
@@ -345,6 +346,12 @@ async fn run_event_loop(
received_engine_event = true;
match event {
EngineEvent::MessageStarted { .. } => {
// Assistant text starting after parallel tool work
// means the tool group is done. Flush the active
// cell first so the message lands BELOW the
// committed tool group (Codex pattern: streamed
// assistant content always flows after work).
app.flush_active_cell();
current_streaming_text.clear();
app.streaming_state.reset();
app.streaming_state.start_text(0, None);
@@ -355,6 +362,12 @@ async fn run_event_loop(
if sanitized.is_empty() {
continue;
}
// First delta of a fresh stream has no streaming
// cell yet; flush active so the tool group settles
// before the assistant prose appears below it.
if app.streaming_message_index.is_none() {
app.flush_active_cell();
}
current_streaming_text.push_str(&sanitized);
let index =
ensure_streaming_history_cell(app, StreamingCellKind::Assistant);
@@ -416,6 +429,9 @@ async fn run_event_loop(
}
}
EngineEvent::ThinkingStarted { .. } => {
// A fresh thinking block after parallel tools means
// that batch is done; settle it into history.
app.flush_active_cell();
app.reasoning_buffer.clear();
app.reasoning_header = None;
app.thinking_started_at = Some(Instant::now());
@@ -518,6 +534,19 @@ async fn run_event_loop(
status,
error,
} => {
// Finalize any in-flight tool group. Cancellation
// marks still-running entries as Failed so the user
// sees they were interrupted rather than the spinner
// hanging forever.
if matches!(
status,
crate::core::events::TurnOutcomeStatus::Interrupted
| crate::core::events::TurnOutcomeStatus::Failed
) {
app.finalize_active_cell_as_interrupted();
} else {
app.flush_active_cell();
}
app.is_loading = false;
app.offline_mode = false;
app.streaming_state.reset();
@@ -3246,6 +3275,10 @@ fn apply_loaded_session(app: &mut App, session: &SavedSession) {
app.history.clear();
app.tool_cells.clear();
app.tool_details_by_cell.clear();
app.active_cell = None;
app.active_tool_details.clear();
app.active_cell_revision = app.active_cell_revision.wrapping_add(1);
app.exploring_cell = None;
app.exploring_entries.clear();
app.ignored_tool_calls.clear();
app.pending_tool_uses.clear();
@@ -4461,34 +4494,44 @@ fn format_task_detail(task: &TaskRecord) -> String {
#[allow(clippy::too_many_lines)]
fn handle_tool_call_started(app: &mut App, id: &str, name: &str, input: &serde_json::Value) {
let id = id.to_string();
// All in-flight tool work for the current turn lives in `app.active_cell`
// until the turn completes. This mirrors Codex's contract: ONE active cell
// mutates in place; finalized history isn't touched until flush. This
// keeps the transcript stable while parallel completions arrive in any
// order.
if app.active_cell.is_none() {
app.active_cell = Some(ActiveCell::new());
}
if is_exploring_tool(name) {
let label = exploring_label(name, input);
let cell_index = if let Some(idx) = app.exploring_cell {
idx
} else {
app.add_message(HistoryCell::Tool(ToolCell::Exploring(ExploringCell {
entries: Vec::new(),
})));
let idx = app.history.len().saturating_sub(1);
app.exploring_cell = Some(idx);
idx
};
if let Some(HistoryCell::Tool(ToolCell::Exploring(cell))) = app.history.get_mut(cell_index)
{
let entry_index = cell.insert_entry(ExploringEntry {
label,
status: ToolStatus::Running,
});
app.mark_history_updated();
app.exploring_entries
.insert(id.clone(), (cell_index, entry_index));
}
register_tool_cell(app, &id, name, input, cell_index);
// ensure_exploring + append_to_exploring keeps all parallel exploring
// starts in a single ExploringCell entry.
let active = app.active_cell.as_mut().expect("active_cell just ensured");
let entry_idx = active.ensure_exploring();
let inner = active
.append_to_exploring(
id.clone(),
ExploringEntry {
label,
status: ToolStatus::Running,
},
)
.map_or(0, |(_, inner)| inner);
app.exploring_cell = Some(entry_idx);
let virtual_index = app.history.len() + entry_idx;
app.exploring_entries
.insert(id.clone(), (virtual_index, inner));
register_tool_cell(app, &id, name, input, virtual_index);
app.mark_history_updated();
return;
}
app.exploring_cell = None;
// Non-exploring tool: each is its own entry inside the active cell. We
// intentionally do NOT clear `exploring_cell` here — the active cell can
// hold both an exploring aggregate AND independent tool entries
// simultaneously, which is exactly the case CX#7 fixes.
if is_exec_tool(name) {
let command = exec_command_from_input(input).unwrap_or_else(|| "<command>".to_string());
@@ -4511,17 +4554,21 @@ fn handle_tool_call_started(app: &mut App, id: &str, name: &str, input: &serde_j
app.last_exec_wait_command = Some(command.clone());
}
app.add_message(HistoryCell::Tool(ToolCell::Exec(ExecCell {
command,
status: ToolStatus::Running,
output: None,
started_at: Some(Instant::now()),
duration_ms: None,
source,
interaction: Some(summary.clone()),
})));
let cell_index = app.history.len().saturating_sub(1);
register_tool_cell(app, &id, name, input, cell_index);
push_active_tool_cell(
app,
&id,
name,
input,
HistoryCell::Tool(ToolCell::Exec(ExecCell {
command,
status: ToolStatus::Running,
output: None,
started_at: Some(Instant::now()),
duration_ms: None,
source,
interaction: Some(summary.clone()),
})),
);
return;
}
@@ -4538,69 +4585,87 @@ fn handle_tool_call_started(app: &mut App, id: &str, name: &str, input: &serde_j
app.last_exec_wait_command = Some(command.clone());
}
app.add_message(HistoryCell::Tool(ToolCell::Exec(ExecCell {
command,
status: ToolStatus::Running,
output: None,
started_at: Some(Instant::now()),
duration_ms: None,
source,
interaction: None,
})));
let cell_index = app.history.len().saturating_sub(1);
register_tool_cell(app, &id, name, input, cell_index);
push_active_tool_cell(
app,
&id,
name,
input,
HistoryCell::Tool(ToolCell::Exec(ExecCell {
command,
status: ToolStatus::Running,
output: None,
started_at: Some(Instant::now()),
duration_ms: None,
source,
interaction: None,
})),
);
return;
}
if name == "update_plan" {
let (explanation, steps) = parse_plan_input(input);
app.add_message(HistoryCell::Tool(ToolCell::PlanUpdate(PlanUpdateCell {
explanation,
steps,
status: ToolStatus::Running,
})));
let cell_index = app.history.len().saturating_sub(1);
register_tool_cell(app, &id, name, input, cell_index);
push_active_tool_cell(
app,
&id,
name,
input,
HistoryCell::Tool(ToolCell::PlanUpdate(PlanUpdateCell {
explanation,
steps,
status: ToolStatus::Running,
})),
);
return;
}
if name == "apply_patch" {
let (path, summary) = parse_patch_summary(input);
app.add_message(HistoryCell::Tool(ToolCell::PatchSummary(
PatchSummaryCell {
push_active_tool_cell(
app,
&id,
name,
input,
HistoryCell::Tool(ToolCell::PatchSummary(PatchSummaryCell {
path,
summary,
status: ToolStatus::Running,
error: None,
},
)));
let cell_index = app.history.len().saturating_sub(1);
register_tool_cell(app, &id, name, input, cell_index);
})),
);
return;
}
if name == "review" {
let target = review_target_label(input);
app.add_message(HistoryCell::Tool(ToolCell::Review(ReviewCell {
target,
status: ToolStatus::Running,
output: None,
error: None,
})));
let cell_index = app.history.len().saturating_sub(1);
register_tool_cell(app, &id, name, input, cell_index);
push_active_tool_cell(
app,
&id,
name,
input,
HistoryCell::Tool(ToolCell::Review(ReviewCell {
target,
status: ToolStatus::Running,
output: None,
error: None,
})),
);
return;
}
if is_mcp_tool(name) {
app.add_message(HistoryCell::Tool(ToolCell::Mcp(McpToolCell {
tool: name.to_string(),
status: ToolStatus::Running,
content: None,
is_image: false,
})));
let cell_index = app.history.len().saturating_sub(1);
register_tool_cell(app, &id, name, input, cell_index);
push_active_tool_cell(
app,
&id,
name,
input,
HistoryCell::Tool(ToolCell::Mcp(McpToolCell {
tool: name.to_string(),
status: ToolStatus::Running,
content: None,
is_image: false,
})),
);
return;
}
@@ -4611,36 +4676,65 @@ fn handle_tool_call_started(app: &mut App, id: &str, name: &str, input: &serde_j
.strip_prefix(&app.workspace)
.unwrap_or(&raw_path)
.to_path_buf();
app.add_message(HistoryCell::Tool(ToolCell::ViewImage(ViewImageCell {
path: display_path,
})));
let cell_index = app.history.len().saturating_sub(1);
register_tool_cell(app, &id, name, input, cell_index);
push_active_tool_cell(
app,
&id,
name,
input,
HistoryCell::Tool(ToolCell::ViewImage(ViewImageCell { path: display_path })),
);
}
return;
}
if is_web_search_tool(name) {
let query = web_search_query(input);
app.add_message(HistoryCell::Tool(ToolCell::WebSearch(WebSearchCell {
query,
status: ToolStatus::Running,
summary: None,
})));
let cell_index = app.history.len().saturating_sub(1);
register_tool_cell(app, &id, name, input, cell_index);
push_active_tool_cell(
app,
&id,
name,
input,
HistoryCell::Tool(ToolCell::WebSearch(WebSearchCell {
query,
status: ToolStatus::Running,
summary: None,
})),
);
return;
}
let input_summary = summarize_tool_args(input);
app.add_message(HistoryCell::Tool(ToolCell::Generic(GenericToolCell {
name: name.to_string(),
status: ToolStatus::Running,
input_summary,
output: None,
})));
let cell_index = app.history.len().saturating_sub(1);
register_tool_cell(app, &id, name, input, cell_index);
push_active_tool_cell(
app,
&id,
name,
input,
HistoryCell::Tool(ToolCell::Generic(GenericToolCell {
name: name.to_string(),
status: ToolStatus::Running,
input_summary,
output: None,
})),
);
}
/// Push a tool cell as a new entry in `active_cell`, register the tool id,
/// and write a stub detail record so the pager / Ctrl+O can find it.
fn push_active_tool_cell(
app: &mut App,
tool_id: &str,
tool_name: &str,
input: &serde_json::Value,
cell: HistoryCell,
) {
if app.active_cell.is_none() {
app.active_cell = Some(ActiveCell::new());
}
let active = app.active_cell.as_mut().expect("active_cell just ensured");
let entry_idx = active.push_tool(tool_id.to_string(), cell);
let virtual_index = app.history.len() + entry_idx;
register_tool_cell(app, tool_id, tool_name, input, virtual_index);
app.mark_history_updated();
}
fn register_tool_cell(
@@ -4651,27 +4745,43 @@ fn register_tool_cell(
cell_index: usize,
) {
app.tool_cells.insert(tool_id.to_string(), cell_index);
app.tool_details_by_cell.insert(
cell_index,
ToolDetailRecord {
tool_id: tool_id.to_string(),
tool_name: tool_name.to_string(),
input: input.clone(),
output: None,
},
);
let record = ToolDetailRecord {
tool_id: tool_id.to_string(),
tool_name: tool_name.to_string(),
input: input.clone(),
output: None,
};
if cell_index < app.history.len() {
app.tool_details_by_cell.insert(cell_index, record);
} else {
// Active-cell entry: keep the detail record in `active_tool_details`
// until the active cell flushes. `flush_active_cell` migrates these
// records into `tool_details_by_cell` keyed by the eventual real
// cell index.
app.active_tool_details.insert(tool_id.to_string(), record);
}
}
fn store_tool_detail_output(
app: &mut App,
tool_id: &str,
cell_index: usize,
result: &Result<ToolResult, ToolError>,
) {
if let Some(detail) = app.tool_details_by_cell.get_mut(&cell_index) {
detail.output = Some(match result {
Ok(tool_result) => tool_result.content.clone(),
Err(err) => err.to_string(),
});
let payload = Some(match result {
Ok(tool_result) => tool_result.content.clone(),
Err(err) => err.to_string(),
});
if cell_index < app.history.len()
&& let Some(detail) = app.tool_details_by_cell.get_mut(&cell_index)
{
detail.output = payload.clone();
}
// Also write to the active table while the entry might still live there;
// some callsites pre-rewrite cell_index but the active_tool_details map is
// the canonical source for in-flight outputs.
if let Some(detail) = app.active_tool_details.get_mut(tool_id) {
detail.output = payload;
}
}
@@ -4679,18 +4789,20 @@ fn store_tool_detail_output(
fn handle_tool_call_complete(
app: &mut App,
id: &str,
_name: &str,
name: &str,
result: &Result<ToolResult, ToolError>,
) {
if app.ignored_tool_calls.remove(id) {
return;
}
// Exploring entries land in the per-tool map regardless of whether they
// live in the active cell or in finalized history; the path is the same.
if let Some((cell_index, entry_index)) = app.exploring_entries.remove(id) {
app.tool_cells.remove(id);
store_tool_detail_output(app, cell_index, result);
if let Some(HistoryCell::Tool(ToolCell::Exploring(cell))) = app.history.get_mut(cell_index)
store_tool_detail_output(app, id, cell_index, result);
if let Some(HistoryCell::Tool(ToolCell::Exploring(cell))) =
app.cell_at_virtual_index_mut(cell_index)
&& let Some(entry) = cell.entries.get_mut(entry_index)
{
entry.status = match result.as_ref() {
@@ -4698,15 +4810,31 @@ fn handle_tool_call_complete(
Ok(_) | Err(_) => ToolStatus::Failed,
};
app.mark_history_updated();
// Mutating the in-flight exploring cell needs an active-cell
// revision bump so the transcript cache invalidates the synthetic
// tail row.
if cell_index >= app.history.len() {
app.active_cell_revision = app.active_cell_revision.wrapping_add(1);
if let Some(active) = app.active_cell.as_mut() {
active.bump_revision();
}
}
}
return;
}
// Look up the cell by tool id. If the id isn't registered, that's an
// orphan completion (race condition where the started event was lost or
// a tool result arrived after the active cell was already flushed). Build
// a finalized standalone cell from the result so the user can still see
// the output, but DO NOT touch the active cell.
let Some(cell_index) = app.tool_cells.remove(id) else {
push_orphan_tool_completion(app, id, name, result);
return;
};
store_tool_detail_output(app, cell_index, result);
store_tool_detail_output(app, id, cell_index, result);
let in_active = cell_index >= app.history.len();
let status = match result.as_ref() {
Ok(tool_result) => match tool_result.metadata.as_ref() {
@@ -4729,7 +4857,7 @@ fn handle_tool_call_complete(
Err(_) => ToolStatus::Failed,
};
if let Some(cell) = app.history.get_mut(cell_index) {
if let Some(cell) = app.cell_at_virtual_index_mut(cell_index) {
match cell {
HistoryCell::Tool(ToolCell::Exec(exec)) => {
exec.status = status;
@@ -4832,6 +4960,95 @@ fn handle_tool_call_complete(
_ => {}
}
}
// If the mutated cell lived inside the active group, bump the active-cell
// revision so the transcript cache re-renders the synthetic tail row.
if in_active {
app.active_cell_revision = app.active_cell_revision.wrapping_add(1);
if let Some(active) = app.active_cell.as_mut() {
active.bump_revision();
}
}
}
/// Build a finalized standalone history cell for a tool completion whose
/// start was never registered (orphan). This preserves the contract that
/// every tool result is visible somewhere; the alternative (silently
/// dropping it) hides errors and breaks debuggability.
///
/// Choice of cell type: we use `GenericToolCell` because we have no input
/// payload to reconstruct a more specific cell. The pager remains usable —
/// `tool_details_by_cell` is populated with the result text.
///
/// ## Index drift
///
/// If an active cell is in flight when the orphan arrives, pushing the
/// orphan into `app.history` shifts every active-cell virtual index forward
/// by 1. We must rewrite `tool_cells` / `exploring_entries` accordingly so
/// later completion lookups still find the right entries.
fn push_orphan_tool_completion(
app: &mut App,
tool_id: &str,
name: &str,
result: &Result<ToolResult, ToolError>,
) {
let status = match result.as_ref() {
Ok(tool_result) => {
if tool_result.success {
ToolStatus::Success
} else {
ToolStatus::Failed
}
}
Err(_) => ToolStatus::Failed,
};
let output = match result.as_ref() {
Ok(tool_result) => Some(summarize_tool_output(&tool_result.content)),
Err(err) => Some(err.to_string()),
};
let history_threshold_before_push = app.history.len();
let active_in_flight = app.active_cell.is_some();
app.add_message(HistoryCell::Tool(ToolCell::Generic(GenericToolCell {
name: name.to_string(),
status,
input_summary: None,
output,
})));
let cell_index = app.history.len().saturating_sub(1);
app.tool_details_by_cell.insert(
cell_index,
ToolDetailRecord {
tool_id: tool_id.to_string(),
tool_name: name.to_string(),
input: serde_json::Value::Null,
output: match result.as_ref() {
Ok(tool_result) => Some(tool_result.content.clone()),
Err(err) => Some(err.to_string()),
},
},
);
// Shift active-cell virtual indices forward by 1 to absorb the new
// history cell. Without this, the next completion would address the
// wrong entry.
if active_in_flight {
let threshold = history_threshold_before_push;
for idx in app.tool_cells.values_mut() {
if *idx >= threshold {
*idx = idx.wrapping_add(1);
}
}
for (cell_idx, _) in app.exploring_entries.values_mut() {
if *cell_idx >= threshold {
*cell_idx = cell_idx.wrapping_add(1);
}
}
if let Some(idx) = app.exploring_cell.as_mut()
&& *idx >= threshold
{
*idx = idx.wrapping_add(1);
}
}
}
fn is_exploring_tool(name: &str) -> bool {
+301 -1
View File
@@ -4,7 +4,7 @@ use crate::tui::file_mention::{
find_file_mention_completions, partial_file_mention_at_cursor, try_autocomplete_file_mention,
user_request_with_file_mentions,
};
use crate::tui::history::{GenericToolCell, ToolCell, ToolStatus};
use crate::tui::history::{GenericToolCell, HistoryCell, ToolCell, ToolStatus};
use crate::tui::views::{ModalView, ViewAction};
use std::path::PathBuf;
use std::process::Command;
@@ -1056,3 +1056,303 @@ fn try_autocomplete_file_mention_returns_false_outside_mention() {
app.cursor_position = app.input.chars().count();
assert!(!try_autocomplete_file_mention(&mut app));
}
// === CX#7 — single active cell mutated in place for parallel tool calls ===
/// Build a minimal successful ToolResult with the given content.
fn ok_result(
content: &str,
) -> Result<crate::tools::spec::ToolResult, crate::tools::spec::ToolError> {
Ok(crate::tools::spec::ToolResult::success(content))
}
#[test]
fn parallel_exploring_tool_starts_share_one_active_entry() {
// Three exploring tools start in any order; they must collapse into one
// entry inside the active cell rather than three separate cells. This is
// the central CX#7 contract for the most common parallel case.
let mut app = create_test_app();
handle_tool_call_started(
&mut app,
"t-a",
"read_file",
&serde_json::json!({"path": "alpha.rs"}),
);
handle_tool_call_started(
&mut app,
"t-b",
"read_file",
&serde_json::json!({"path": "beta.rs"}),
);
handle_tool_call_started(
&mut app,
"t-c",
"grep_files",
&serde_json::json!({"pattern": "TODO"}),
);
// History must remain empty: nothing flushes until the turn ends.
assert_eq!(app.history.len(), 0, "no history cells written mid-turn");
let active = app.active_cell.as_ref().expect("active cell created");
assert_eq!(
active.entry_count(),
1,
"all exploring starts share one entry"
);
let HistoryCell::Tool(ToolCell::Exploring(explore)) = &active.entries()[0] else {
panic!("expected exploring cell")
};
assert_eq!(explore.entries.len(), 3);
for entry in &explore.entries {
assert_eq!(entry.status, ToolStatus::Running);
}
}
#[test]
fn out_of_order_completes_finalize_one_history_cell_per_turn() {
// Three parallel tools complete in reverse order; we then signal turn
// complete and assert exactly one tool history cell exists (the
// finalized active group). This proves the active cell didn't bounce
// mid-turn and that the flush path correctly migrates entries.
let mut app = create_test_app();
handle_tool_call_started(
&mut app,
"t-1",
"read_file",
&serde_json::json!({"path": "a.rs"}),
);
handle_tool_call_started(
&mut app,
"t-2",
"read_file",
&serde_json::json!({"path": "b.rs"}),
);
handle_tool_call_started(
&mut app,
"t-3",
"grep_files",
&serde_json::json!({"pattern": "x"}),
);
// Out-of-order completion: t-3, then t-1, then t-2.
handle_tool_call_complete(&mut app, "t-3", "grep_files", &ok_result("two hits"));
handle_tool_call_complete(&mut app, "t-1", "read_file", &ok_result("contents A"));
handle_tool_call_complete(&mut app, "t-2", "read_file", &ok_result("contents B"));
// Still nothing in history: the active cell holds everything.
assert_eq!(app.history.len(), 0);
let active = app.active_cell.as_ref().expect("active cell still present");
let HistoryCell::Tool(ToolCell::Exploring(explore)) = &active.entries()[0] else {
panic!("expected exploring cell")
};
assert!(
explore
.entries
.iter()
.all(|e| e.status == ToolStatus::Success),
"all exploring entries should be Success after their tools complete"
);
// Flush via the explicit helper (mirrors what TurnComplete does).
app.flush_active_cell();
assert!(app.active_cell.is_none(), "active cell cleared after flush");
// The flushed group is exactly one history cell — the merged exploring
// aggregate. This is the heart of CX#7: parallel work renders as ONE
// finalized cell, regardless of completion order.
let tool_cells = app
.history
.iter()
.filter(|c| matches!(c, HistoryCell::Tool(_)))
.count();
assert_eq!(
tool_cells, 1,
"exactly one tool history cell after parallel turn"
);
}
#[test]
fn mixed_parallel_tools_render_in_single_active_cell() {
// Tools of different shapes — exploring + exec + generic — all in flight
// at once. The active cell must hold them all without bouncing.
let mut app = create_test_app();
handle_tool_call_started(
&mut app,
"ex-1",
"read_file",
&serde_json::json!({"path": "x.rs"}),
);
handle_tool_call_started(
&mut app,
"shell-1",
"exec_shell",
&serde_json::json!({"command": "ls"}),
);
handle_tool_call_started(
&mut app,
"gen-1",
"todo_write",
&serde_json::json!({"items": []}),
);
assert_eq!(app.history.len(), 0);
let active = app.active_cell.as_ref().expect("active cell present");
// 3 entries: exploring aggregate (1) + exec + generic.
assert_eq!(active.entry_count(), 3);
handle_tool_call_complete(&mut app, "shell-1", "exec_shell", &ok_result("ok"));
handle_tool_call_complete(&mut app, "gen-1", "todo_write", &ok_result("done"));
handle_tool_call_complete(&mut app, "ex-1", "read_file", &ok_result("file body"));
// After all complete, still in active until flush.
assert_eq!(app.history.len(), 0);
app.flush_active_cell();
let tool_cells: Vec<_> = app
.history
.iter()
.filter(|c| matches!(c, HistoryCell::Tool(_)))
.collect();
assert_eq!(
tool_cells.len(),
3,
"three distinct tool shapes finalize as three cells in stable insertion order"
);
}
#[test]
fn orphan_tool_complete_with_unknown_id_pushes_separate_cell() {
// A ToolCallComplete with no matching ToolCallStarted — the orphan path.
// Per the design we render it as a finalized standalone cell so the user
// still sees the output, but we must NOT flush or contaminate any active
// cell that's currently in flight.
let mut app = create_test_app();
handle_tool_call_started(
&mut app,
"live-1",
"read_file",
&serde_json::json!({"path": "live.rs"}),
);
// Orphan completion arrives.
handle_tool_call_complete(&mut app, "ghost-id", "mystery_tool", &ok_result("oops"));
// Active cell is intact.
let active = app
.active_cell
.as_ref()
.expect("active cell preserved after orphan");
assert_eq!(active.entry_count(), 1);
// The orphan rendered as a separate finalized cell pushed to history.
assert_eq!(app.history.len(), 1, "orphan added one finalized cell");
let HistoryCell::Tool(ToolCell::Generic(generic)) = &app.history[0] else {
panic!("orphan should render as a Generic tool cell")
};
assert_eq!(generic.name, "mystery_tool");
assert_eq!(generic.status, ToolStatus::Success);
}
#[test]
fn turn_complete_flushes_active_cell_into_history() {
// The full path through the public flush helper. Verifies that a
// mid-turn snapshot (exec running, exploring complete) becomes a stable
// history slice on flush.
let mut app = create_test_app();
handle_tool_call_started(
&mut app,
"ex-1",
"read_file",
&serde_json::json!({"path": "a.rs"}),
);
handle_tool_call_complete(&mut app, "ex-1", "read_file", &ok_result("body"));
handle_tool_call_started(
&mut app,
"shell-1",
"exec_shell",
&serde_json::json!({"command": "ls"}),
);
// Don't complete shell-1 — simulate cancellation mid-shell.
app.finalize_active_cell_as_interrupted();
assert!(app.active_cell.is_none(), "active cell cleared on flush");
let exec_cells: Vec<_> = app
.history
.iter()
.filter_map(|c| match c {
HistoryCell::Tool(ToolCell::Exec(exec)) => Some(exec),
_ => None,
})
.collect();
assert_eq!(exec_cells.len(), 1);
assert_eq!(
exec_cells[0].status,
ToolStatus::Failed,
"interrupted shell entry marked Failed (closest available terminal status)"
);
}
#[test]
fn orphan_during_active_keeps_subsequent_completion_routed_correctly() {
// Regression cover for the index-shift trap: when an orphan arrives
// mid-active, it pushes a real history cell that bumps virtual indices
// by one. A subsequent legitimate completion must still find its entry.
let mut app = create_test_app();
handle_tool_call_started(
&mut app,
"live",
"exec_shell",
&serde_json::json!({"command": "ls"}),
);
// Orphan completion arrives FIRST (before live's completion).
handle_tool_call_complete(&mut app, "ghost", "weird_tool", &ok_result("ghost-out"));
// Now complete the live tool — it should still mutate the active entry,
// not silently drop or hit a stale index.
handle_tool_call_complete(&mut app, "live", "exec_shell", &ok_result("hello"));
// Active cell still present (turn hasn't completed).
let active = app.active_cell.as_ref().expect("active cell present");
let HistoryCell::Tool(ToolCell::Exec(exec)) = &active.entries()[0] else {
panic!("expected exec cell")
};
assert_eq!(exec.status, ToolStatus::Success);
// History contains exactly the orphan.
assert_eq!(app.history.len(), 1);
let HistoryCell::Tool(ToolCell::Generic(generic)) = &app.history[0] else {
panic!("expected orphan generic cell")
};
assert_eq!(generic.name, "weird_tool");
// Flush settles the active exec into history below the orphan.
app.flush_active_cell();
assert_eq!(app.history.len(), 2);
}
#[test]
fn tool_details_survive_active_cell_flush() {
// The pager / Ctrl+O resolves tool details by cell index. Flushing the
// active cell must move detail records into `tool_details_by_cell` so
// the pager keeps working after the turn settles.
let mut app = create_test_app();
handle_tool_call_started(
&mut app,
"tid",
"exec_shell",
&serde_json::json!({"command": "echo hi"}),
);
handle_tool_call_complete(&mut app, "tid", "exec_shell", &ok_result("hi"));
app.flush_active_cell();
// The exec cell is now at index 0 in history.
assert_eq!(app.history.len(), 1);
let detail = app
.tool_details_by_cell
.get(&0)
.expect("detail record migrated to flushed cell index");
assert_eq!(detail.tool_id, "tid");
assert_eq!(detail.tool_name, "exec_shell");
}
+26 -2
View File
@@ -69,9 +69,33 @@ impl ChatWidget {
// we synthesize a uniform slice from the global history_version so
// any mutation invalidates every cell (matches the pre-cache
// semantics).
let cell_revisions = vec![app.history_version; app.history.len()];
//
// The active in-flight cell (if any) is appended as the last cell so
// its mutations show up at the live tail. Each entry inside the
// active cell becomes a virtual cell at index `history.len() + i`,
// matching `App::cell_at_virtual_index`. Active-cell entries get a
// distinct revision derived from `active_cell_revision` so changes to
// them only re-render those rows.
let active_entries: &[HistoryCell] = app
.active_cell
.as_ref()
.map_or(&[], |active| active.entries());
let mut combined_cells: Vec<HistoryCell> =
Vec::with_capacity(app.history.len() + active_entries.len());
combined_cells.extend_from_slice(&app.history);
combined_cells.extend_from_slice(active_entries);
let mut cell_revisions = vec![app.history_version; combined_cells.len()];
// Salt the active-cell revisions with `active_cell_revision` so they
// invalidate independently of the history version when only the
// active cell mutates.
if !active_entries.is_empty() {
let active_rev = app.active_cell_revision;
for rev in cell_revisions.iter_mut().skip(app.history.len()) {
*rev = rev.wrapping_add(active_rev.wrapping_mul(0x9E37_79B9_7F4A_7C15));
}
}
app.transcript_cache.ensure(
&app.history,
&combined_cells,
&cell_revisions,
content_area.width.max(1),
render_options,