Skip to content

Commit 4891ee2

Browse files
refactor transcript view to handle HistoryCells (#3538)
No (intended) functional change. This refactors the transcript view to hold a list of HistoryCells instead of a list of Lines. This simplifies and makes much of the logic more robust, as well as laying the groundwork for future changes, e.g. live-updating history cells in the transcript. Similar to #2879 in goal. Fixes #2755.
1 parent bac8a42 commit 4891ee2

File tree

9 files changed

+309
-389
lines changed

9 files changed

+309
-389
lines changed

codex-rs/core/src/conversation_manager.rs

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -144,19 +144,19 @@ impl ConversationManager {
144144
self.conversations.write().await.remove(conversation_id)
145145
}
146146

147-
/// Fork an existing conversation by dropping the last `drop_last_messages`
148-
/// user/assistant messages from its transcript and starting a new
147+
/// Fork an existing conversation by taking messages up to the given position
148+
/// (not including the message at the given position) and starting a new
149149
/// conversation with identical configuration (unless overridden by the
150150
/// caller's `config`). The new conversation will have a fresh id.
151151
pub async fn fork_conversation(
152152
&self,
153-
num_messages_to_drop: usize,
153+
nth_user_message: usize,
154154
config: Config,
155155
path: PathBuf,
156156
) -> CodexResult<NewConversation> {
157157
// Compute the prefix up to the cut point.
158158
let history = RolloutRecorder::get_rollout_history(&path).await?;
159-
let history = truncate_after_dropping_last_messages(history, num_messages_to_drop);
159+
let history = truncate_after_nth_user_message(history, nth_user_message);
160160

161161
// Spawn a new conversation with the computed initial history.
162162
let auth_manager = self.auth_manager.clone();
@@ -169,14 +169,10 @@ impl ConversationManager {
169169
}
170170
}
171171

172-
/// Return a prefix of `items` obtained by dropping the last `n` user messages
173-
/// and all items that follow them.
174-
fn truncate_after_dropping_last_messages(history: InitialHistory, n: usize) -> InitialHistory {
175-
if n == 0 {
176-
return InitialHistory::Forked(history.get_rollout_items());
177-
}
178-
179-
// Work directly on rollout items, and cut the vector at the nth-from-last user message input.
172+
/// Return a prefix of `items` obtained by cutting strictly before the nth user message
173+
/// (0-based) and all items that follow it.
174+
fn truncate_after_nth_user_message(history: InitialHistory, n: usize) -> InitialHistory {
175+
// Work directly on rollout items, and cut the vector at the nth user message input.
180176
let items: Vec<RolloutItem> = history.get_rollout_items();
181177

182178
// Find indices of user message inputs in rollout order.
@@ -189,13 +185,13 @@ fn truncate_after_dropping_last_messages(history: InitialHistory, n: usize) -> I
189185
}
190186
}
191187

192-
// If fewer than n user messages exist, treat as empty.
193-
if user_positions.len() < n {
188+
// If fewer than or equal to n user messages exist, treat as empty (out of range).
189+
if user_positions.len() <= n {
194190
return InitialHistory::New;
195191
}
196192

197-
// Cut strictly before the nth-from-last user message (do not keep the nth itself).
198-
let cut_idx = user_positions[user_positions.len() - n];
193+
// Cut strictly before the nth user message (do not keep the nth itself).
194+
let cut_idx = user_positions[n];
199195
let rolled: Vec<RolloutItem> = items.into_iter().take(cut_idx).collect();
200196

201197
if rolled.is_empty() {
@@ -262,7 +258,7 @@ mod tests {
262258
.cloned()
263259
.map(RolloutItem::ResponseItem)
264260
.collect();
265-
let truncated = truncate_after_dropping_last_messages(InitialHistory::Forked(initial), 1);
261+
let truncated = truncate_after_nth_user_message(InitialHistory::Forked(initial), 1);
266262
let got_items = truncated.get_rollout_items();
267263
let expected_items = vec![
268264
RolloutItem::ResponseItem(items[0].clone()),
@@ -279,7 +275,7 @@ mod tests {
279275
.cloned()
280276
.map(RolloutItem::ResponseItem)
281277
.collect();
282-
let truncated2 = truncate_after_dropping_last_messages(InitialHistory::Forked(initial2), 2);
278+
let truncated2 = truncate_after_nth_user_message(InitialHistory::Forked(initial2), 2);
283279
assert!(matches!(truncated2, InitialHistory::New));
284280
}
285281
}

codex-rs/core/tests/suite/fork_conversation.rs

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,8 @@ async fn fork_conversation_twice_drops_to_first_message() {
104104
items
105105
};
106106

107-
// Compute expected prefixes after each fork by truncating base rollout at nth-from-last user input.
107+
// Compute expected prefixes after each fork by truncating base rollout
108+
// strictly before the nth user input (0-based).
108109
let base_items = read_items(&base_path);
109110
let find_user_input_positions = |items: &[RolloutItem]| -> Vec<usize> {
110111
let mut pos = Vec::new();
@@ -126,11 +127,8 @@ async fn fork_conversation_twice_drops_to_first_message() {
126127
};
127128
let user_inputs = find_user_input_positions(&base_items);
128129

129-
// After dropping last user input (n=1), cut strictly before that input if present, else empty.
130-
let cut1 = user_inputs
131-
.get(user_inputs.len().saturating_sub(1))
132-
.copied()
133-
.unwrap_or(0);
130+
// After cutting at nth user input (n=1 → second user message), cut strictly before that input.
131+
let cut1 = user_inputs.get(1).copied().unwrap_or(0);
134132
let expected_after_first: Vec<RolloutItem> = base_items[..cut1].to_vec();
135133

136134
// After dropping again (n=1 on fork1), compute expected relative to fork1's rollout.
@@ -161,12 +159,12 @@ async fn fork_conversation_twice_drops_to_first_message() {
161159
serde_json::to_value(&expected_after_first).unwrap()
162160
);
163161

164-
// Fork again with n=1 → drops the (new) last user message, leaving only the first.
162+
// Fork again with n=0 → drops the (new) last user message, leaving only the first.
165163
let NewConversation {
166164
conversation: codex_fork2,
167165
..
168166
} = conversation_manager
169-
.fork_conversation(1, config_for_fork.clone(), fork1_path.clone())
167+
.fork_conversation(0, config_for_fork.clone(), fork1_path.clone())
170168
.await
171169
.expect("fork 2");
172170

codex-rs/tui/src/app.rs

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use crate::app_event::AppEvent;
33
use crate::app_event_sender::AppEventSender;
44
use crate::chatwidget::ChatWidget;
55
use crate::file_search::FileSearchManager;
6+
use crate::history_cell::HistoryCell;
67
use crate::pager_overlay::Overlay;
78
use crate::resume_picker::ResumeSelection;
89
use crate::tui;
@@ -46,7 +47,7 @@ pub(crate) struct App {
4647

4748
pub(crate) file_search: FileSearchManager,
4849

49-
pub(crate) transcript_lines: Vec<Line<'static>>,
50+
pub(crate) transcript_cells: Vec<Arc<dyn HistoryCell>>,
5051

5152
// Pager overlay state (Transcript or Static like Diff)
5253
pub(crate) overlay: Option<Overlay>,
@@ -131,7 +132,7 @@ impl App {
131132
model_saved_to_global: false,
132133
file_search,
133134
enhanced_keys_supported,
134-
transcript_lines: Vec::new(),
135+
transcript_cells: Vec::new(),
135136
overlay: None,
136137
deferred_history_lines: Vec::new(),
137138
has_emitted_history_lines: false,
@@ -213,15 +214,12 @@ impl App {
213214
tui.frame_requester().schedule_frame();
214215
}
215216
AppEvent::InsertHistoryCell(cell) => {
216-
let mut cell_transcript = cell.transcript_lines();
217-
if !cell.is_stream_continuation() && !self.transcript_lines.is_empty() {
218-
cell_transcript.insert(0, Line::from(""));
219-
}
217+
let cell: Arc<dyn HistoryCell> = cell.into();
220218
if let Some(Overlay::Transcript(t)) = &mut self.overlay {
221-
t.insert_lines(cell_transcript.clone());
219+
t.insert_cell(cell.clone());
222220
tui.frame_requester().schedule_frame();
223221
}
224-
self.transcript_lines.extend(cell_transcript.clone());
222+
self.transcript_cells.push(cell.clone());
225223
let mut display = cell.display_lines(tui.terminal.last_known_screen_size.width);
226224
if !display.is_empty() {
227225
// Only insert a separating blank line for new cells that are not
@@ -437,7 +435,7 @@ impl App {
437435
} => {
438436
// Enter alternate screen and set viewport to full size.
439437
let _ = tui.enter_alt_screen();
440-
self.overlay = Some(Overlay::new_transcript(self.transcript_lines.clone()));
438+
self.overlay = Some(Overlay::new_transcript(self.transcript_cells.clone()));
441439
tui.frame_requester().schedule_frame();
442440
}
443441
KeyEvent {
@@ -470,7 +468,7 @@ impl App {
470468
kind: KeyEventKind::Press,
471469
..
472470
} if self.backtrack.primed
473-
&& self.backtrack.count > 0
471+
&& self.backtrack.nth_user_message != usize::MAX
474472
&& self.chat_widget.composer_is_empty() =>
475473
{
476474
// Delegate to helper for clarity; preserves behavior.
@@ -503,7 +501,6 @@ mod tests {
503501
use crate::file_search::FileSearchManager;
504502
use codex_core::CodexAuth;
505503
use codex_core::ConversationManager;
506-
use ratatui::text::Line;
507504
use std::sync::Arc;
508505
use std::sync::atomic::AtomicBool;
509506

@@ -525,7 +522,7 @@ mod tests {
525522
model_saved_to_profile: false,
526523
model_saved_to_global: false,
527524
file_search,
528-
transcript_lines: Vec::<Line<'static>>::new(),
525+
transcript_cells: Vec::new(),
529526
overlay: None,
530527
deferred_history_lines: Vec::new(),
531528
has_emitted_history_lines: false,

0 commit comments

Comments
 (0)