mirror of
https://github.com/openai/codex.git
synced 2026-10-15 16:48:52 +00:00
Reserve spacing after recaps at the transcript tail (#47355)
## What changed Reserve a blank row below a trailing recap when the transcript viewport has room. Keep this spacing in layout rather than selectable content, and omit it when a completed or live message follows the recap. ## Testing Add snapshots covering recap tail spacing across terminal sizes, with and without a next action, and with completed or streaming follow-up messages. Verify that reflowed history retains a single separator between a recap and the following message. GitOrigin-RevId: f9552931fac4eabd7d72c2b312ffba717257e9e1
This commit is contained in:
Felipe Coury
@@ -259,6 +259,74 @@ async fn older_page_loading_uses_the_status_row_without_moving_content_or_cursor
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn recap_spacing_belongs_to_the_transcript_tail() -> Result<()> {
|
||||
let mut snapshots = Vec::new();
|
||||
for (width, height, next_action) in [
|
||||
(80, 10, None),
|
||||
(80, 12, Some("Review the changes.")),
|
||||
(32, 12, Some("Review the changes.")),
|
||||
(80, 7, Some("Review the changes.")),
|
||||
(32, 6, Some("Review the changes.")),
|
||||
] {
|
||||
let mut app = crate::app::test_support::make_test_app().await;
|
||||
attach_thread(&mut app, ThreadId::new());
|
||||
app.local_settings.tui.show_tooltips = true;
|
||||
app.local_settings.tui.animations = false;
|
||||
app.transcript_cells = vec![
|
||||
Arc::new(crate::history_cell::PlainHistoryCell::new(
|
||||
(1..=20)
|
||||
.map(|row| format!("History {row}").into())
|
||||
.collect(),
|
||||
)),
|
||||
Arc::new(
|
||||
crate::history_cell::ThreadRecapHistoryCell::new("The draft is ready.".into())
|
||||
.with_next_action(next_action.map(str::to_owned)),
|
||||
),
|
||||
];
|
||||
crate::app::test_support::select_catalog_tip(
|
||||
&mut app,
|
||||
/*width*/ 80,
|
||||
"Tip: Use /mcp to list configured MCP tools.",
|
||||
);
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
tui.set_owned_screen(/*owned*/ true)?;
|
||||
let size = Size::new(width, height);
|
||||
tui.terminal.resize(size)?;
|
||||
let mut snapshot = |app: &mut App, label: &str| -> Result<()> {
|
||||
app.render_owned_transcript(&mut tui, size)?;
|
||||
let buffer = crate::custom_terminal::test_support::last_rendered_buffer(&tui.terminal);
|
||||
snapshots.push(format!(
|
||||
"{width}x{height}, {label}\n{}",
|
||||
buffer_text(buffer)
|
||||
));
|
||||
Ok(())
|
||||
};
|
||||
snapshot(&mut app, "Recap at tail")?;
|
||||
app.transcript_cells
|
||||
.push(Arc::new(crate::history_cell::AgentMessageCell::new(
|
||||
vec!["Follow-up response.".into()],
|
||||
/*is_first_line*/ true,
|
||||
)));
|
||||
snapshot(&mut app, "Message after recap")?;
|
||||
app.transcript_cells.pop();
|
||||
crate::chatwidget::tests::helpers::set_active_cell(
|
||||
&mut app.chat_widget,
|
||||
Box::new(crate::history_cell::AgentMessageCell::new(
|
||||
vec!["Streaming response.".into()],
|
||||
/*is_first_line*/ true,
|
||||
)),
|
||||
);
|
||||
snapshot(&mut app, "Live message after recap")?;
|
||||
tui.set_owned_screen(/*owned*/ false)?;
|
||||
}
|
||||
insta::assert_snapshot!(
|
||||
"recap_tail_spacing",
|
||||
normalize_snapshot_paths(snapshots.join("\n\n"))
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn owned_transcript_reserves_a_row_above_the_composer() -> Result<()> {
|
||||
let mut app = crate::app::test_support::make_test_app().await;
|
||||
|
||||
@@ -698,6 +698,34 @@ fn recap_history_cell_wraps_in_narrow_terminals() {
|
||||
");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn recap_history_uses_one_separator_before_following_message() {
|
||||
let mut app = make_test_app().await;
|
||||
app.transcript_cells = vec![
|
||||
Arc::new(
|
||||
ThreadRecapHistoryCell::new("The draft is ready.".into())
|
||||
.with_next_action(Some("Review the changes.".into())),
|
||||
),
|
||||
Arc::new(AgentMessageCell::new(
|
||||
vec!["Follow-up response.".into()],
|
||||
/*is_first_line*/ true,
|
||||
)),
|
||||
];
|
||||
let rendered = app
|
||||
.render_transcript_lines_for_reflow(/*width*/ 80)
|
||||
.lines
|
||||
.iter()
|
||||
.map(|line| line.line.to_string())
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
insta::assert_snapshot!(rendered, @r"
|
||||
↳ Recap: The draft is ready.
|
||||
Next: Review the changes.
|
||||
|
||||
• Follow-up response.
|
||||
");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn recap_history_cell_preserves_unicode_and_url_tokens() {
|
||||
let cell = ThreadRecapHistoryCell::new(
|
||||
|
||||
+174
@@ -0,0 +1,174 @@
|
||||
---
|
||||
source: tui/src/app/owned_transcript_tests.rs
|
||||
expression: "normalize_snapshot_paths(snapshots.join(\"\\n\\n\"))"
|
||||
---
|
||||
80x10, Recap at tail
|
||||
History 20
|
||||
|
||||
↳ Recap: The draft is ready.
|
||||
|
||||
Tip: Use /mcp to list configured MCP tools.
|
||||
|
||||
› Ask Codex to do anything
|
||||
|
||||
gpt-test default · /tmp/project
|
||||
? for shortcuts
|
||||
|
||||
80x10, Message after recap
|
||||
|
||||
↳ Recap: The draft is ready.
|
||||
|
||||
• Follow-up response.
|
||||
Tip: Use /mcp to list configured MCP tools.
|
||||
|
||||
› Ask Codex to do anything
|
||||
|
||||
gpt-test default · /tmp/project
|
||||
? for shortcuts
|
||||
|
||||
80x10, Live message after recap
|
||||
|
||||
↳ Recap: The draft is ready.
|
||||
|
||||
• Streaming response.
|
||||
Tip: Use /mcp to list configured MCP tools.
|
||||
|
||||
› Ask Codex to do anything
|
||||
|
||||
gpt-test default · /tmp/project
|
||||
? for shortcuts
|
||||
|
||||
80x12, Recap at tail
|
||||
History 19
|
||||
History 20
|
||||
|
||||
↳ Recap: The draft is ready.
|
||||
Next: Review the changes.
|
||||
|
||||
Tip: Use /mcp to list configured MCP tools.
|
||||
|
||||
› Ask Codex to do anything
|
||||
|
||||
gpt-test default · /tmp/project
|
||||
? for shortcuts
|
||||
|
||||
80x12, Message after recap
|
||||
History 20
|
||||
|
||||
↳ Recap: The draft is ready.
|
||||
Next: Review the changes.
|
||||
|
||||
• Follow-up response.
|
||||
Tip: Use /mcp to list configured MCP tools.
|
||||
|
||||
› Ask Codex to do anything
|
||||
|
||||
gpt-test default · /tmp/project
|
||||
? for shortcuts
|
||||
|
||||
80x12, Live message after recap
|
||||
History 20
|
||||
|
||||
↳ Recap: The draft is ready.
|
||||
Next: Review the changes.
|
||||
|
||||
• Streaming response.
|
||||
Tip: Use /mcp to list configured MCP tools.
|
||||
|
||||
› Ask Codex to do anything
|
||||
|
||||
gpt-test default · /tmp/project
|
||||
? for shortcuts
|
||||
|
||||
32x12, Recap at tail
|
||||
History 20
|
||||
|
||||
↳ Recap: The draft is ready.
|
||||
Next: Review the
|
||||
changes.
|
||||
|
||||
|
||||
|
||||
› Ask Codex to do anything
|
||||
|
||||
gpt-test default · /tmp/proje…
|
||||
? for shortcuts
|
||||
|
||||
32x12, Message after recap
|
||||
|
||||
↳ Recap: The draft is ready.
|
||||
Next: Review the
|
||||
changes.
|
||||
|
||||
• Follow-up response.
|
||||
|
||||
|
||||
› Ask Codex to do anything
|
||||
|
||||
gpt-test default · /tmp/proje…
|
||||
? for shortcuts
|
||||
|
||||
32x12, Live message after recap
|
||||
|
||||
↳ Recap: The draft is ready.
|
||||
Next: Review the
|
||||
changes.
|
||||
|
||||
• Streaming response.
|
||||
|
||||
|
||||
› Ask Codex to do anything
|
||||
|
||||
gpt-test default · /tmp/proje…
|
||||
? for shortcuts
|
||||
|
||||
80x7, Recap at tail
|
||||
Next: Review the changes.
|
||||
Tip: Use /mcp to list configured MCP tools.
|
||||
|
||||
› Ask Codex to do anything
|
||||
|
||||
gpt-test default · /tmp/project
|
||||
? for shortcuts
|
||||
|
||||
80x7, Message after recap
|
||||
• Follow-up response.
|
||||
Tip: Use /mcp to list configured MCP tools.
|
||||
|
||||
› Ask Codex to do anything
|
||||
|
||||
gpt-test default · /tmp/project
|
||||
? for shortcuts
|
||||
|
||||
80x7, Live message after recap
|
||||
• Streaming response.
|
||||
Tip: Use /mcp to list configured MCP tools.
|
||||
|
||||
› Ask Codex to do anything
|
||||
|
||||
gpt-test default · /tmp/project
|
||||
? for shortcuts
|
||||
|
||||
32x6, Recap at tail
|
||||
|
||||
|
||||
› Ask Codex to do anything
|
||||
|
||||
gpt-test default · /tmp/proje…
|
||||
? for shortcuts
|
||||
|
||||
32x6, Message after recap
|
||||
|
||||
|
||||
› Ask Codex to do anything
|
||||
|
||||
gpt-test default · /tmp/proje…
|
||||
? for shortcuts
|
||||
|
||||
32x6, Live message after recap
|
||||
|
||||
|
||||
› Ask Codex to do anything
|
||||
|
||||
gpt-test default · /tmp/proje…
|
||||
? for shortcuts
|
||||
@@ -3,6 +3,7 @@
|
||||
//! The app owns history and pagination. This view owns only reading/selection state and bounded
|
||||
//! layout caches. Anchors address content inside an entry, so prepending history never renumbers
|
||||
//! them and rewrapping does not turn a reading position into an unrelated screen row.
|
||||
//! Recap tail spacing is reserved by layout and never enters selectable cell content.
|
||||
|
||||
mod activity;
|
||||
mod bookmark;
|
||||
@@ -505,7 +506,20 @@ impl TranscriptView {
|
||||
let height = self
|
||||
.layout(cells, last)
|
||||
.map_or(/*default*/ 0, |l| l.row_count());
|
||||
self.move_rows(cells, last, height, -(self.area.height as isize))
|
||||
let recap_gap = usize::from(
|
||||
self.area.height > 1
|
||||
&& !has_live
|
||||
&& cells.last().is_some_and(|cell| {
|
||||
cell.as_any()
|
||||
.is::<crate::history_cell::ThreadRecapHistoryCell>()
|
||||
}),
|
||||
);
|
||||
self.move_rows(
|
||||
cells,
|
||||
last,
|
||||
height + recap_gap,
|
||||
-(self.area.height as isize),
|
||||
)
|
||||
}
|
||||
|
||||
fn move_rows(
|
||||
|
||||
Reference in New Issue
Block a user