mirror of
https://github.com/openai/codex.git
synced 2026-10-21 17:48:44 +00:00
fix(tui): preserve streamed tails across remaps
This commit is contained in:
+5
@@ -0,0 +1,5 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests/status_and_layout.rs
|
||||
expression: raw_tail
|
||||
---
|
||||
**tail remains visible**
|
||||
@@ -1483,6 +1483,42 @@ async fn finalize_turn_persists_held_plan_stream_tail() {
|
||||
assert!(chat.plan_stream_controller.is_none());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn finalize_turn_persists_structural_plan_tail_as_history_cell() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
let cwd = chat.config.cwd.to_path_buf();
|
||||
|
||||
let mut controller = crate::streaming::controller::PlanStreamController::new(
|
||||
Some(/*width*/ 80),
|
||||
cwd.as_path(),
|
||||
HistoryRenderMode::Rich,
|
||||
);
|
||||
controller.push("| Step | Owner |\n");
|
||||
controller.push("| --- | --- |\n");
|
||||
controller.push("| Preserve held table tail | Agent |\n");
|
||||
assert!(
|
||||
controller.requires_final_scrollback_reflow(),
|
||||
"expected table plan tail to require consolidation reflow",
|
||||
);
|
||||
chat.plan_stream_controller = Some(controller);
|
||||
|
||||
while rx.try_recv().is_ok() {}
|
||||
|
||||
chat.finalize_turn();
|
||||
|
||||
let inserted = drain_insert_history(&mut rx);
|
||||
let rendered = inserted
|
||||
.iter()
|
||||
.map(|cell| lines_to_single_string(cell))
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
assert!(
|
||||
rendered.contains("Preserve held table tail"),
|
||||
"expected structural plan tail to be persisted directly, got {rendered:?}",
|
||||
);
|
||||
assert!(chat.plan_stream_controller.is_none());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn raw_output_toggle_refreshes_active_stream_tail() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
@@ -1519,6 +1555,7 @@ async fn raw_output_toggle_refreshes_active_stream_tail() {
|
||||
raw_tail.contains("**tail remains visible**"),
|
||||
"expected active tail to refresh after raw toggle, got {raw_tail:?}",
|
||||
);
|
||||
insta::assert_snapshot!(raw_tail);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -117,10 +117,9 @@ impl ChatWidget {
|
||||
// If a stream is currently active, finalize it.
|
||||
self.flush_answer_stream_with_separator();
|
||||
if let Some(mut controller) = self.plan_stream_controller.take() {
|
||||
let requires_scrollback_reflow = controller.requires_final_scrollback_reflow();
|
||||
self.clear_active_stream_tail();
|
||||
let (cell, source) = controller.finalize();
|
||||
if !requires_scrollback_reflow && let Some(cell) = cell {
|
||||
if let Some(cell) = cell {
|
||||
self.add_boxed_history(cell);
|
||||
}
|
||||
if let Some(source) = source {
|
||||
@@ -296,10 +295,9 @@ impl ChatWidget {
|
||||
pub(super) fn finalize_turn(&mut self) {
|
||||
self.flush_answer_stream_with_separator();
|
||||
if let Some(mut controller) = self.plan_stream_controller.take() {
|
||||
let requires_scrollback_reflow = controller.requires_final_scrollback_reflow();
|
||||
self.clear_active_stream_tail();
|
||||
let (cell, source) = controller.finalize();
|
||||
if !requires_scrollback_reflow && let Some(cell) = cell {
|
||||
if let Some(cell) = cell {
|
||||
self.add_boxed_history(cell);
|
||||
}
|
||||
if let Some(source) = source {
|
||||
|
||||
@@ -123,6 +123,7 @@ struct StablePrefixLenCache {
|
||||
struct PartialSourceLine {
|
||||
source_start: usize,
|
||||
source_end: usize,
|
||||
emitted_source_end: usize,
|
||||
emitted_prefix: Vec<HyperlinkLine>,
|
||||
}
|
||||
|
||||
@@ -284,7 +285,9 @@ impl StreamCore {
|
||||
let had_pending_queue = self.state.queued_len() > 0;
|
||||
let had_structural_tail = self.requires_final_scrollback_reflow();
|
||||
let partial_line = if had_pending_queue {
|
||||
self.partial_source_line_after_rendered_len(self.emitted_stable_len)
|
||||
self.synthetic_queue_partial
|
||||
.clone()
|
||||
.or_else(|| self.partial_source_line_after_rendered_len(self.emitted_stable_len))
|
||||
} else {
|
||||
None
|
||||
};
|
||||
@@ -359,7 +362,9 @@ impl StreamCore {
|
||||
let had_pending_queue = self.state.queued_len() > 0;
|
||||
let had_structural_tail = self.requires_final_scrollback_reflow();
|
||||
let partial_line = if had_pending_queue {
|
||||
self.partial_source_line_after_rendered_len(self.emitted_stable_len)
|
||||
self.synthetic_queue_partial
|
||||
.clone()
|
||||
.or_else(|| self.partial_source_line_after_rendered_len(self.emitted_stable_len))
|
||||
} else {
|
||||
None
|
||||
};
|
||||
@@ -470,10 +475,16 @@ impl StreamCore {
|
||||
self.clear_stable_queue();
|
||||
self.emitted_stable_len = source_line_start_len;
|
||||
|
||||
let mut queued = trim_rendered_prefix(
|
||||
self.rendered_lines[source_line_start_len..source_line_end_len].to_vec(),
|
||||
&partial_line.emitted_prefix,
|
||||
);
|
||||
let mut queued = if self.render_mode == HistoryRenderMode::Raw {
|
||||
plain_hyperlink_lines(raw_lines_from_source(
|
||||
&self.raw_source[partial_line.emitted_source_end..partial_line.source_end],
|
||||
))
|
||||
} else {
|
||||
trim_rendered_prefix(
|
||||
self.rendered_lines[source_line_start_len..source_line_end_len].to_vec(),
|
||||
&partial_line.emitted_prefix,
|
||||
)
|
||||
};
|
||||
let synthetic_len = queued.len();
|
||||
if source_line_end_len < target_stable_len {
|
||||
queued.extend(self.rendered_lines[source_line_end_len..target_stable_len].to_vec());
|
||||
@@ -513,10 +524,26 @@ impl StreamCore {
|
||||
let mut ordinary_lines = step.len();
|
||||
if self.synthetic_queue_remaining > 0 {
|
||||
let synthetic_lines = ordinary_lines.min(self.synthetic_queue_remaining);
|
||||
let mut emitted_prefix = None;
|
||||
if let Some(partial_line) = &mut self.synthetic_queue_partial {
|
||||
partial_line
|
||||
.emitted_prefix
|
||||
.extend(step[..synthetic_lines].to_vec());
|
||||
emitted_prefix = Some((
|
||||
partial_line.source_start,
|
||||
partial_line.source_end,
|
||||
partial_line.emitted_prefix.clone(),
|
||||
));
|
||||
}
|
||||
if let Some((source_start, source_end, emitted_prefix)) = emitted_prefix {
|
||||
let emitted_source_end = self.source_boundary_after_rendered_prefix(
|
||||
source_start,
|
||||
source_end,
|
||||
&emitted_prefix,
|
||||
);
|
||||
if let Some(partial_line) = &mut self.synthetic_queue_partial {
|
||||
partial_line.emitted_source_end = emitted_source_end;
|
||||
}
|
||||
}
|
||||
ordinary_lines -= synthetic_lines;
|
||||
self.synthetic_queue_remaining -= synthetic_lines;
|
||||
@@ -664,11 +691,18 @@ impl StreamCore {
|
||||
if rendered_len <= source_start_rendered_len {
|
||||
return None;
|
||||
}
|
||||
let emitted_prefix =
|
||||
self.rendered_lines[source_start_rendered_len..rendered_len].to_vec();
|
||||
let emitted_source_end = self.source_boundary_after_rendered_prefix(
|
||||
source_start,
|
||||
source_end,
|
||||
&emitted_prefix,
|
||||
);
|
||||
return Some(PartialSourceLine {
|
||||
source_start,
|
||||
source_end,
|
||||
emitted_prefix: self.rendered_lines[source_start_rendered_len..rendered_len]
|
||||
.to_vec(),
|
||||
emitted_source_end,
|
||||
emitted_prefix,
|
||||
});
|
||||
}
|
||||
source_start = source_end;
|
||||
@@ -689,6 +723,54 @@ impl StreamCore {
|
||||
boundaries
|
||||
}
|
||||
|
||||
fn source_boundary_after_rendered_prefix(
|
||||
&self,
|
||||
source_start: usize,
|
||||
source_end: usize,
|
||||
emitted_prefix: &[HyperlinkLine],
|
||||
) -> usize {
|
||||
if emitted_prefix.is_empty() {
|
||||
return source_start;
|
||||
}
|
||||
let emitted_text = emitted_prefix
|
||||
.iter()
|
||||
.map(hyperlink_line_text)
|
||||
.collect::<Vec<_>>();
|
||||
let emitted_visible_text = emitted_text.join(" ");
|
||||
if let Some(boundary) = visible_text_source_boundary(
|
||||
&self.raw_source[source_start..source_end],
|
||||
&emitted_visible_text,
|
||||
) {
|
||||
return source_start + boundary;
|
||||
}
|
||||
|
||||
let mut boundaries = Vec::new();
|
||||
boundaries.push(source_start);
|
||||
boundaries.extend(
|
||||
self.raw_source[source_start..source_end]
|
||||
.char_indices()
|
||||
.map(|(idx, ch)| source_start + idx + ch.len_utf8()),
|
||||
);
|
||||
|
||||
for boundary in boundaries {
|
||||
let rendered = self.render_source(&self.raw_source[..boundary]);
|
||||
if rendered.len() < emitted_prefix.len() {
|
||||
continue;
|
||||
}
|
||||
let rendered_text = rendered[..emitted_prefix.len()]
|
||||
.iter()
|
||||
.map(hyperlink_line_text)
|
||||
.collect::<Vec<_>>();
|
||||
if rendered_text == emitted_text {
|
||||
return boundary;
|
||||
}
|
||||
}
|
||||
|
||||
self.source_boundary_after_rendered_len(emitted_prefix.len())
|
||||
.min(source_end)
|
||||
.max(source_start)
|
||||
}
|
||||
|
||||
fn default_tail_budget_lines(&mut self) -> usize {
|
||||
if self.default_tail_lines == 0 || self.rendered_lines.is_empty() {
|
||||
return 0;
|
||||
@@ -716,6 +798,83 @@ impl StreamCore {
|
||||
}
|
||||
}
|
||||
|
||||
fn visible_text_source_boundary(source: &str, visible_prefix: &str) -> Option<usize> {
|
||||
if visible_prefix.is_empty() {
|
||||
return Some(0);
|
||||
}
|
||||
|
||||
let mut visible_chars = visible_prefix.chars().peekable();
|
||||
let mut iter = source.char_indices().peekable();
|
||||
while let Some((idx, ch)) = iter.next() {
|
||||
if visible_chars.peek().is_none() {
|
||||
return Some(idx);
|
||||
}
|
||||
|
||||
if ch == '[' {
|
||||
let mut label_end = None;
|
||||
let mut label_chars = Vec::new();
|
||||
for (label_idx, label_ch) in iter.by_ref() {
|
||||
if label_ch == ']' {
|
||||
label_end = Some(label_idx + label_ch.len_utf8());
|
||||
break;
|
||||
}
|
||||
label_chars.push((label_idx, label_ch));
|
||||
}
|
||||
let label_end = label_end?;
|
||||
for (label_idx, label_ch) in label_chars {
|
||||
match visible_chars.peek().copied() {
|
||||
Some(expected) if expected == label_ch => {
|
||||
visible_chars.next();
|
||||
if visible_chars.peek().is_none() {
|
||||
return Some(
|
||||
skip_link_destination(source, label_end)
|
||||
.unwrap_or(label_idx + label_ch.len_utf8()),
|
||||
);
|
||||
}
|
||||
}
|
||||
_ => return None,
|
||||
}
|
||||
}
|
||||
if let Some(destination_end) = skip_link_destination(source, label_end) {
|
||||
while let Some((next_idx, _)) = iter.peek().copied() {
|
||||
if next_idx < destination_end {
|
||||
iter.next();
|
||||
} else {
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
if matches!(ch, '*' | '_' | '`') {
|
||||
continue;
|
||||
}
|
||||
|
||||
match visible_chars.peek().copied() {
|
||||
Some(expected) if expected == ch => {
|
||||
visible_chars.next();
|
||||
if visible_chars.peek().is_none() {
|
||||
return Some(idx + ch.len_utf8());
|
||||
}
|
||||
}
|
||||
_ => return None,
|
||||
}
|
||||
}
|
||||
|
||||
visible_chars.peek().is_none().then_some(source.len())
|
||||
}
|
||||
|
||||
fn skip_link_destination(source: &str, label_end: usize) -> Option<usize> {
|
||||
let after_label = source.get(label_end..)?;
|
||||
if !after_label.starts_with('(') {
|
||||
return None;
|
||||
}
|
||||
after_label
|
||||
.find(')')
|
||||
.map(|destination_end| label_end + destination_end + ')'.len_utf8())
|
||||
}
|
||||
|
||||
fn trim_rendered_prefix(
|
||||
lines: Vec<HyperlinkLine>,
|
||||
emitted_prefix: &[HyperlinkLine],
|
||||
@@ -1225,6 +1384,31 @@ mod tests {
|
||||
snapshots
|
||||
}
|
||||
|
||||
fn collect_visible_plan_stream_snapshots(
|
||||
deltas: &[&str],
|
||||
width: Option<usize>,
|
||||
) -> Vec<Vec<String>> {
|
||||
let mut ctrl = plan_stream_controller(width);
|
||||
let mut committed_lines = Vec::new();
|
||||
let mut snapshots = Vec::new();
|
||||
for delta in deltas {
|
||||
ctrl.push(delta);
|
||||
while let (Some(cell), idle) = ctrl.on_commit_tick() {
|
||||
committed_lines.extend(cell.transcript_lines(u16::MAX));
|
||||
if idle {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
let mut visible = lines_to_plain_strings(&committed_lines);
|
||||
visible.extend(hyperlink_lines_to_plain_strings(
|
||||
&ctrl.current_tail_display_lines(),
|
||||
));
|
||||
snapshots.push(visible);
|
||||
}
|
||||
snapshots
|
||||
}
|
||||
|
||||
fn format_stream_snapshots(snapshots: &[Vec<String>]) -> String {
|
||||
snapshots
|
||||
.iter()
|
||||
@@ -1414,6 +1598,16 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn plan_controller_streamed_list_continuation_snapshot() {
|
||||
let snapshots = collect_visible_plan_stream_snapshots(
|
||||
HAPPY_PATH_DUPLICATE_REPRO_DELTAS,
|
||||
Some(/*width*/ 80),
|
||||
);
|
||||
|
||||
insta::assert_snapshot!(format_stream_snapshots(&snapshots));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn controller_live_tail_rerenders_table_tail_after_resize() {
|
||||
let mut ctrl = stream_controller(Some(96));
|
||||
@@ -2542,6 +2736,41 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn controller_set_width_reuses_synthetic_partial_across_repeated_resize() {
|
||||
let mut ctrl = stream_controller(Some(/*width*/ 18));
|
||||
ctrl.push("alpha beta gamma delta epsilon zeta eta theta iota kappa lambda mu\n");
|
||||
ctrl.push("tail line\n");
|
||||
|
||||
let (first_emit, idle) = ctrl.on_commit_tick();
|
||||
let first_emit = first_emit
|
||||
.expect("expected first wrapped line emission")
|
||||
.transcript_lines(u16::MAX);
|
||||
assert!(!idle, "expected remaining wrapped content after one tick");
|
||||
|
||||
ctrl.set_width(Some(/*width*/ 80));
|
||||
assert!(
|
||||
ctrl.queued_lines() > 0,
|
||||
"expected synthetic queue after first resize"
|
||||
);
|
||||
ctrl.set_width(Some(/*width*/ 24));
|
||||
|
||||
let (cell, _source) = ctrl.finalize();
|
||||
let remaining = cell
|
||||
.map(|c| lines_to_plain_strings(&c.transcript_lines(u16::MAX)))
|
||||
.unwrap_or_default();
|
||||
let first_emit = lines_to_plain_strings(&first_emit).join("\n");
|
||||
let joined = remaining.join("\n");
|
||||
assert!(
|
||||
!joined.contains("alpha beta"),
|
||||
"finalize must not replay the already-visible prefix after repeated resize; emitted before resize: {first_emit:?}, finalized after resize: {joined:?}",
|
||||
);
|
||||
assert!(
|
||||
joined.contains("lambda mu") && joined.contains("tail line"),
|
||||
"repeated resize must preserve un-emitted suffix and tail; emitted before resize: {first_emit:?}, finalized after resize: {joined:?}",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn controller_set_render_mode_partial_wrapped_emit_preserves_remaining_content() {
|
||||
let mut ctrl = stream_controller(Some(/*width*/ 18));
|
||||
@@ -2606,6 +2835,38 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn controller_set_render_mode_raw_suffix_starts_after_rich_link_prefix() {
|
||||
let mut ctrl = stream_controller(Some(/*width*/ 10));
|
||||
ctrl.push(
|
||||
"[alpha beta](https://example.com) gamma delta epsilon zeta eta theta iota kappa lambda mu\n",
|
||||
);
|
||||
ctrl.push("tail line\n");
|
||||
|
||||
let (first_emit, idle) = ctrl.on_commit_tick();
|
||||
let first_emit = first_emit
|
||||
.expect("expected first rich wrapped line emission")
|
||||
.transcript_lines(u16::MAX);
|
||||
assert!(!idle, "expected remaining rich content after one tick");
|
||||
|
||||
ctrl.set_render_mode(HistoryRenderMode::Raw);
|
||||
|
||||
let (cell, _source) = ctrl.finalize();
|
||||
let remaining = cell
|
||||
.map(|c| lines_to_plain_strings(&c.transcript_lines(u16::MAX)))
|
||||
.unwrap_or_default();
|
||||
let first_emit = lines_to_plain_strings(&first_emit).join("\n");
|
||||
let joined = remaining.join("\n");
|
||||
assert!(
|
||||
!joined.contains("https://example.com") && !joined.contains("](https://"),
|
||||
"raw suffix must not start inside already-visible link markup; emitted before toggle: {first_emit:?}, finalized after toggle: {joined:?}",
|
||||
);
|
||||
assert!(
|
||||
joined.contains("gamma") && joined.contains("tail line"),
|
||||
"raw suffix should preserve un-emitted source after the rendered link label; emitted before toggle: {first_emit:?}, finalized after toggle: {joined:?}",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn controller_clear_queue_advances_synthetic_accounting_before_finalize() {
|
||||
let mut ctrl = stream_controller(Some(/*width*/ 18));
|
||||
|
||||
+88
@@ -0,0 +1,88 @@
|
||||
---
|
||||
source: tui/src/streaming/controller.rs
|
||||
expression: format_stream_snapshots(&snapshots)
|
||||
---
|
||||
after delta 0:
|
||||
• Proposed Plan
|
||||
|
||||
|
||||
Happy Path
|
||||
---
|
||||
after delta 1:
|
||||
• Proposed Plan
|
||||
|
||||
|
||||
Happy Path
|
||||
---
|
||||
after delta 2:
|
||||
• Proposed Plan
|
||||
|
||||
|
||||
Happy Path
|
||||
|
||||
1. In the second terminal, make the smoke home non-writable:
|
||||
---
|
||||
after delta 3:
|
||||
• Proposed Plan
|
||||
|
||||
|
||||
Happy Path
|
||||
|
||||
1. In the second terminal, make the smoke home non-writable:
|
||||
|
||||
---
|
||||
after delta 4:
|
||||
• Proposed Plan
|
||||
|
||||
|
||||
Happy Path
|
||||
|
||||
1. In the second terminal, make the smoke home non-writable:
|
||||
|
||||
chmod a-w $CODEX_SMOKE_HOME
|
||||
---
|
||||
after delta 5:
|
||||
• Proposed Plan
|
||||
|
||||
|
||||
Happy Path
|
||||
|
||||
1. In the second terminal, make the smoke home non-writable:
|
||||
|
||||
chmod a-w $CODEX_SMOKE_HOME
|
||||
---
|
||||
after delta 6:
|
||||
• Proposed Plan
|
||||
|
||||
|
||||
Happy Path
|
||||
|
||||
1. In the second terminal, make the smoke home non-writable:
|
||||
|
||||
chmod a-w $CODEX_SMOKE_HOME
|
||||
Expected: the running TUI stays open.
|
||||
---
|
||||
after delta 7:
|
||||
• Proposed Plan
|
||||
|
||||
|
||||
Happy Path
|
||||
|
||||
1. In the second terminal, make the smoke home non-writable:
|
||||
|
||||
chmod a-w $CODEX_SMOKE_HOME
|
||||
Expected: the running TUI stays open.
|
||||
---
|
||||
after delta 8:
|
||||
• Proposed Plan
|
||||
|
||||
|
||||
Happy Path
|
||||
|
||||
1. In the second terminal, make the smoke home non-writable:
|
||||
|
||||
chmod a-w $CODEX_SMOKE_HOME
|
||||
|
||||
Expected: the running TUI stays open.
|
||||
|
||||
2. In the TUI, type /model and press Enter.
|
||||
Reference in New Issue
Block a user