From c24e1ee25f9c8441d435a0da44a621e3a1d5cc29 Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Wed, 14 Oct 2026 16:30:15 +0200 Subject: [PATCH] perf(cli, conversation): Stop cloning config per event in `c show` `jp c show` on a 9,138-event conversation spent 2.4 seconds on its main thread building the details block, against 76 ms to load the conversation. Counting turns through `iter_turns()` and reading the last timestamp through `last()` each resolved and cloned the full `PartialAppConfig` for every event in the stream. The command now uses `turn_count()` and `last_turn_event()`, which read event content only, and finishes in about 90 ms of main-thread work. The confirmation prompt's details block used the same two calls and gets the same fix. `ConversationStream::last()` itself walked the stream forward and cloned the config at every event to return the final one. It now steps back from the end and folds the config deltas once, returning the same event and config. `pop()` and `Extend` call it and benefit too. Signed-off-by: Jean Mertz --- crates/jp_cli/src/cmd/conversation/show.rs | 4 ++-- crates/jp_cli/src/shared/confirm.rs | 4 ++-- crates/jp_conversation/src/stream.rs | 4 +++- crates/jp_conversation/src/stream/iter.rs | 7 +++--- crates/jp_conversation/src/stream_tests.rs | 25 ++++++++++++++++++++++ 5 files changed, 35 insertions(+), 9 deletions(-) diff --git a/crates/jp_cli/src/cmd/conversation/show.rs b/crates/jp_cli/src/cmd/conversation/show.rs index ca9d2399d..7f3845721 100644 --- a/crates/jp_cli/src/cmd/conversation/show.rs +++ b/crates/jp_cli/src/cmd/conversation/show.rs @@ -45,9 +45,9 @@ impl Show { let compactions = events.compactions().map(compaction_detail_item).collect(); let details = DetailsFmt::new(id) - .with_last_message_at(events.last().map(|v| v.event.timestamp)) + .with_last_message_at(events.last_turn_event().map(|event| event.timestamp)) .with_event_count(events.len()) - .with_turn_count(events.iter_turns().len()) + .with_turn_count(events.turn_count()) .with_title(conversation.title.as_ref()) .with_model(config.assistant.model.id.resolved().to_string()) .with_last_activated_at(Some(conversation.last_activated_at)) diff --git a/crates/jp_cli/src/shared/confirm.rs b/crates/jp_cli/src/shared/confirm.rs index 13d1f7045..2a2724084 100644 --- a/crates/jp_cli/src/shared/confirm.rs +++ b/crates/jp_cli/src/shared/confirm.rs @@ -227,8 +227,8 @@ fn action_details( DetailsFmt::new(id) .with_title(meta.title.as_ref()) .with_event_count(events.len()) - .with_turn_count(events.iter_turns().len()) - .with_last_message_at(events.last().map(|v| v.event.timestamp)) + .with_turn_count(events.turn_count()) + .with_last_message_at(events.last_turn_event().map(|event| event.timestamp)) .with_last_activated_at(Some(meta.last_activated_at)) .with_pinned_flag(meta.is_pinned()) .with_local_flag(matches!(lock.projection(), Projection::LocalOnly)) diff --git a/crates/jp_conversation/src/stream.rs b/crates/jp_conversation/src/stream.rs index f362866c9..37fa35e7a 100644 --- a/crates/jp_conversation/src/stream.rs +++ b/crates/jp_conversation/src/stream.rs @@ -723,7 +723,9 @@ impl ConversationStream { /// at the time the event was added. #[must_use] pub fn last(&self) -> Option> { - self.iter().last() + // `next_back` folds the deltas before the event once; `Iterator::last` + // would walk forward and clone the config at every event. + self.iter().next_back() } /// Similar to [`Self::last`], but returns a mutable reference to the last diff --git a/crates/jp_conversation/src/stream/iter.rs b/crates/jp_conversation/src/stream/iter.rs index 2a7f28afa..508f4d181 100644 --- a/crates/jp_conversation/src/stream/iter.rs +++ b/crates/jp_conversation/src/stream/iter.rs @@ -42,10 +42,9 @@ use crate::{ConversationEvent, EventId, EventKind}; /// A recomputed delta is a new entry and is assigned a new ID. impl Extend for ConversationStream { fn extend>(&mut self, iter: T) { - // Cache the running tail config across iterations. Without this, every - // push falls through `push_with_config_delta` → `self.last()`, which - // walks the whole stream and deep-clones `PartialAppConfig` on each - // step — making `extend(n)` O(n²) in clones. + // Cache the running tail config across iterations. Recomputing it with + // `self.last()` per event folds every config delta in the stream each + // time, which makes `extend(n)` quadratic. let mut tail = self .last() .map_or_else(|| self.base_config().to_partial(), |v| v.config); diff --git a/crates/jp_conversation/src/stream_tests.rs b/crates/jp_conversation/src/stream_tests.rs index 0068852f3..d26968b89 100644 --- a/crates/jp_conversation/src/stream_tests.rs +++ b/crates/jp_conversation/src/stream_tests.rs @@ -806,6 +806,31 @@ fn a_delta_may_introduce_a_model_alias_the_base_config_never_had() { ); } +#[test] +fn last_carries_the_config_before_it_and_not_after() { + let mut before = PartialAppConfig::empty(); + before.conversation.tools.defaults.run = Some(RunMode::Allow); + let mut after = PartialAppConfig::empty(); + after.conversation.tools.defaults.run = Some(RunMode::Skip); + + let mut stream = ConversationStream::new_test(); + stream.start_turn("first"); + stream.add_config_delta(before); + stream.push(ConversationEvent::now(ChatResponse::message("last"))); + stream.add_config_delta(after); + + let last = stream.last().unwrap(); + assert_eq!( + last.event.as_chat_response(), + Some(&ChatResponse::message("last")) + ); + // A delta applies to the events after it, so the trailing one is excluded. + assert_eq!( + last.config.conversation.tools.defaults.run, + Some(RunMode::Allow) + ); +} + #[test] fn pop_and_trim_preserve_trailing_config_delta() { // The other global event variant: a config delta must survive the same