Merge CX#7: one active cell mutated in place

Replaces "tool start pushes new cell" with a single ActiveCell that
collects parallel/serial tool entries at the transcript tail and
flushes as a contiguous block on first assistant text or turn complete.
Stops the bounce when many tools fire concurrently.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Hunter Bown
2026-04-25 23:14:07 -05:00
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,