mirror of
https://github.com/openai/codex.git
synced 2026-10-21 14:23:14 +00:00
Deduplicate deprecation notices in the TUI transcript (#47065)
## Why Global deprecation notices can be delivered again by hidden threads, creating duplicate transcript entries. ## What changed Skip incoming deprecation notices whose summary and details match a notice already in retained history. Preserve notices with different summaries or details, and allow notices to appear again after clearing history or rebuilding the transcript during a thread switch. ## Testing Add a regression test and transcript snapshot covering repeated live and replayed notifications, an open transcript overlay, distinct notice content, and delivery after clearing history or switching threads. GitOrigin-RevId: 9c0512f588dfb54f7934380119211b57b0df0ee4
This commit is contained in:
@@ -23,6 +23,20 @@ pub(super) struct ThreadUsageStatusHistory {
|
||||
|
||||
impl App {
|
||||
pub(super) fn insert_history_cell(&mut self, tui: &mut tui::Tui, cell: Box<dyn HistoryCell>) {
|
||||
// Global deprecations can be delivered again by hidden threads. Scope deduplication to
|
||||
// retained history so clearing or rebuilding a transcript can show the notice again.
|
||||
if let Some(notice) = cell
|
||||
.as_any()
|
||||
.downcast_ref::<history_cell::DeprecationNoticeCell>()
|
||||
&& self.transcript_cells.iter().any(|existing| {
|
||||
existing
|
||||
.as_any()
|
||||
.downcast_ref::<history_cell::DeprecationNoticeCell>()
|
||||
== Some(notice)
|
||||
})
|
||||
{
|
||||
return;
|
||||
}
|
||||
if !crate::empty_state_animation::is_startup_cell(cell.as_ref()) {
|
||||
self.chat_widget
|
||||
.empty_state_animation
|
||||
|
||||
+50
@@ -0,0 +1,50 @@
|
||||
---
|
||||
source: tui/src/app/tests/startup_warnings_tests.rs
|
||||
expression: "format!(\"{buffer:?}\")"
|
||||
---
|
||||
Buffer {
|
||||
area: Rect { x: 0, y: 0, width: 80, height: 12 },
|
||||
content: [
|
||||
"/ T R A N S C R I P T / / / / / / / / / / / / / / / / / / / / / / / / / / / / / ",
|
||||
"⚠ `transcript_v2` is deprecated. ",
|
||||
"Remove it from your configuration. ",
|
||||
" ",
|
||||
" ",
|
||||
"› Hello ",
|
||||
" Please say hello. ",
|
||||
" ",
|
||||
"Ctrl+Space select ",
|
||||
" ↑/↓ to scroll · pgup/pgdn to page · home/end to jump ",
|
||||
" q close · f3 find · esc browse prompts ",
|
||||
" ",
|
||||
],
|
||||
styles: [
|
||||
x: 0, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 0, y: 1, fg: Red, bg: Reset, underline: Reset, modifier: BOLD,
|
||||
x: 2, y: 1, fg: Red, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 32, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 0, y: 2, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 34, y: 2, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 0, y: 5, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD | DIM,
|
||||
x: 2, y: 5, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 0, y: 8, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD,
|
||||
x: 10, y: 8, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 17, y: 8, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 0, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 1, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD,
|
||||
x: 4, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 17, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD,
|
||||
x: 26, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 37, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD,
|
||||
x: 45, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 53, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
x: 0, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 1, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD,
|
||||
x: 2, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 11, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD,
|
||||
x: 13, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 21, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD,
|
||||
x: 24, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: DIM,
|
||||
x: 39, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: NONE,
|
||||
]
|
||||
}
|
||||
@@ -41,6 +41,101 @@ fn drain_history(app: &mut App, tui: &mut tui::Tui, events: &mut UnboundedReceiv
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn deprecation_delivery_deduplicates_retained_transcript() -> Result<()> {
|
||||
let (mut app, mut events, _op_rx) = make_test_app_with_channels().await;
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
tui.set_owned_screen(/*owned*/ false)?;
|
||||
let notice = ServerNotification::DeprecationNotice(
|
||||
codex_app_server_protocol::DeprecationNoticeNotification {
|
||||
summary: "`transcript_v2` is deprecated.".into(),
|
||||
details: Some("Remove it from your configuration.".into()),
|
||||
},
|
||||
);
|
||||
app.chat_widget
|
||||
.handle_server_notification(notice.clone(), /*replay_kind*/ None);
|
||||
drain_history(&mut app, &mut tui, &mut events);
|
||||
app.chat_widget.open_warnings(&app.transcript_cells);
|
||||
app.chat_widget.handle_key_event(KeyCode::Esc.into());
|
||||
app.insert_history_cell(
|
||||
&mut tui,
|
||||
Box::new(UserHistoryCell {
|
||||
message: "Hello\nPlease say hello.".into(),
|
||||
text_elements: Vec::new(),
|
||||
local_image_paths: Vec::new(),
|
||||
remote_image_urls: Vec::new(),
|
||||
spoken: false,
|
||||
}),
|
||||
);
|
||||
app.open_transcript_overlay(&mut tui);
|
||||
// Deliver both duplicates before draining: insertion must not depend on frame timing.
|
||||
for replay_kind in [None, Some(crate::chatwidget::ReplayKind::ThreadSnapshot)] {
|
||||
app.chat_widget
|
||||
.handle_server_notification(notice.clone(), replay_kind);
|
||||
}
|
||||
drain_history(&mut app, &mut tui, &mut events);
|
||||
assert_eq!(app.transcript_cells.len(), 2);
|
||||
assert_eq!(history_cell::warning_count(&app.transcript_cells), 1);
|
||||
let area = Rect::new(
|
||||
/*x*/ 0, /*y*/ 0, /*width*/ 80, /*height*/ 12,
|
||||
);
|
||||
let mut buffer = Buffer::empty(area);
|
||||
let Some(Overlay::Transcript(overlay)) = app.overlay.as_mut() else {
|
||||
panic!("expected transcript")
|
||||
};
|
||||
overlay.render(area, &mut buffer);
|
||||
insta::assert_snapshot!("deduplicated_deprecation_transcript", format!("{buffer:?}"));
|
||||
|
||||
for (summary, details) in [
|
||||
("`transcript_v2` is deprecated.", None),
|
||||
("`transcript_v2` is deprecated.", Some("Updated guidance.")),
|
||||
("Another deprecated setting.", Some("Updated guidance.")),
|
||||
] {
|
||||
app.chat_widget.handle_server_notification(
|
||||
ServerNotification::DeprecationNotice(
|
||||
codex_app_server_protocol::DeprecationNoticeNotification {
|
||||
summary: summary.into(),
|
||||
details: details.map(str::to_owned),
|
||||
},
|
||||
),
|
||||
/*replay_kind*/ None,
|
||||
);
|
||||
}
|
||||
drain_history(&mut app, &mut tui, &mut events);
|
||||
assert_eq!(app.transcript_cells.len(), 5);
|
||||
assert_eq!(history_cell::warning_count(&app.transcript_cells), 2);
|
||||
assert_eq!(
|
||||
history_cell::warning_entries(&app.transcript_cells)
|
||||
.into_iter()
|
||||
.map(|entry| entry.details)
|
||||
.collect::<Vec<_>>(),
|
||||
vec![
|
||||
"`transcript_v2` is deprecated.\nRemove it from your configuration.\n\n`transcript_v2` is deprecated.\nUpdated guidance.",
|
||||
"Another deprecated setting.\nUpdated guidance.",
|
||||
],
|
||||
);
|
||||
|
||||
app.reset_app_ui_state_after_clear();
|
||||
app.chat_widget
|
||||
.handle_server_notification(notice.clone(), /*replay_kind*/ None);
|
||||
drain_history(&mut app, &mut tui, &mut events);
|
||||
assert_eq!(app.transcript_cells.len(), 1);
|
||||
assert_eq!(history_cell::warning_count(&app.transcript_cells), 1);
|
||||
|
||||
app.reset_for_thread_switch(&mut tui)?;
|
||||
for replay_kind in [
|
||||
Some(crate::chatwidget::ReplayKind::ResumeInitialMessages),
|
||||
None,
|
||||
] {
|
||||
app.chat_widget
|
||||
.handle_server_notification(notice.clone(), replay_kind);
|
||||
}
|
||||
drain_history(&mut app, &mut tui, &mut events);
|
||||
assert_eq!(app.transcript_cells.len(), 1);
|
||||
assert_eq!(history_cell::warning_count(&app.transcript_cells), 1);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn startup_warnings_preserve_stream_repair_and_backtrack_selection() -> Result<()> {
|
||||
let mut app = crate::app::test_support::make_test_app().await;
|
||||
|
||||
@@ -252,7 +252,7 @@ impl HistoryCell for SafetyAccessBlockCell {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
#[derive(Debug, Eq, PartialEq)]
|
||||
pub(crate) struct DeprecationNoticeCell {
|
||||
summary: String,
|
||||
details: Option<String>,
|
||||
|
||||
Reference in New Issue
Block a user