mirror of
https://github.com/openai/codex.git
synced 2026-11-09 20:50:33 +00:00
Include older itemless Cloud failures in full transcripts (#60320)
## Why Failed Cloud turns can contain an error without any items. Loading history through item pages alone can omit these older failures from the full transcript. ## What changed - Add a `FullTranscript` hydration scope that fetches remaining Cloud turn metadata in pages of 100 after loading items. - Deduplicate overlapping turn pages while preserving chronological order and already loaded items. - Use the new scope for session transcripts while retaining `Complete` hydration for exports. ## Testing Add a regression test covering older itemless failures, overlapping turn pages, repeated pagination cursors, transcript ordering, and item-only exports. GitOrigin-RevId: 51183e35984aa0183d044061768555bb6f5d1189
This commit is contained in:
committed by
copyberry
parent
8fcba7fae9
commit
b9796386e5
@@ -331,3 +331,124 @@ async fn history_hydration_archived_retry_uses_first_attempt_runtime_settings()
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn cloud_complete_transcript_includes_error_from_an_older_zero_item_turn() -> Result<()> {
|
||||
use crate::app::agents_overview::tests::overview_thread;
|
||||
use crate::app::transcript_export::load_export_transcript;
|
||||
use crate::thread_transcript::RawReasoningVisibility::Hidden;
|
||||
use crate::thread_transcript::load_session_transcript;
|
||||
use codex_app_server_client::CloudClient;
|
||||
use codex_app_server_client::CloudOperation;
|
||||
use codex_app_server_protocol::ThreadItemsListCursor::Opaque;
|
||||
use codex_app_server_protocol::ThreadStatus::Idle;
|
||||
use serde_json::json;
|
||||
|
||||
let id = ThreadId::new();
|
||||
let mut thread = overview_thread(id, /*parent_thread_id*/ None, "Cloud", Idle);
|
||||
thread.history_mode = codex_app_server_protocol::ThreadHistoryMode::Paginated;
|
||||
let failure = "offline failure with no items";
|
||||
let recent = json!({
|
||||
"id": "recent", "status": "failed", "itemsView": "notLoaded", "items": [],
|
||||
"error": {"message": "recent failure"}
|
||||
});
|
||||
let middle = test_turn("middle", TurnStatus::Completed, vec![]);
|
||||
let prior = json!({"type": "agentMessage", "id": "prior", "text": "older answer"});
|
||||
let answer = json!({"type": "agentMessage", "id": "item", "text": "exported answer"});
|
||||
let failed = json!({
|
||||
"id": "older", "status": "failed", "itemsView": "notLoaded", "items": [],
|
||||
"error": {"message": failure}
|
||||
});
|
||||
let (client, mut commands, _events) = CloudClient::channel(/*capacity*/ 1);
|
||||
let mut server = AppServerSession::new(
|
||||
crate::backend_id::BackendId::new(),
|
||||
codex_app_server_client::AppServerClient::Cloud(client),
|
||||
crate::ThreadParamsMode::Cloud,
|
||||
);
|
||||
let service = tokio::spawn(async move {
|
||||
let mut turn_pages = Vec::new();
|
||||
let mut item_pages = Vec::new();
|
||||
while let Some(command) = commands.recv().await {
|
||||
let CloudOperation::Request(request) = command.operation else {
|
||||
panic!("expected a history read")
|
||||
};
|
||||
let response = match *request {
|
||||
ClientRequest::ThreadRead { .. } => json!({"thread": thread}),
|
||||
ClientRequest::ThreadTurnsList { params, .. } => {
|
||||
let page = match params.cursor.as_deref() {
|
||||
None => json!({"data": [recent], "nextCursor": "overlap"}),
|
||||
Some("overlap") => json!({"data": [recent], "nextCursor": "middle"}),
|
||||
Some("middle") => json!({"data": [middle], "nextCursor": "older"}),
|
||||
Some("older") => json!({"data": [failed], "nextCursor": "older"}),
|
||||
other => panic!("unexpected metadata cursor {other:?}"),
|
||||
};
|
||||
turn_pages.push((params.cursor, params.limit));
|
||||
page
|
||||
}
|
||||
ClientRequest::ThreadItemsList { params, .. } => {
|
||||
item_pages.push(params.cursor.clone());
|
||||
match params.cursor {
|
||||
None => json!({"data": [
|
||||
{"turnId": "recent", "item": answer}, {"turnId": "middle", "item": prior}
|
||||
], "nextCursor": "end"}),
|
||||
Some(Opaque(cursor)) if cursor == "end" => {
|
||||
json!({"data": [{"turnId": "middle", "item": {
|
||||
"type":"agentMessage", "id":"first", "text":"first answer"
|
||||
}}], "nextCursor": "end"})
|
||||
}
|
||||
other => panic!("unexpected item cursor {other:?}"),
|
||||
}
|
||||
}
|
||||
_ => panic!("unexpected request"),
|
||||
};
|
||||
command.response.send(Ok(Ok(response))).unwrap();
|
||||
}
|
||||
(turn_pages, item_pages)
|
||||
});
|
||||
let cells = load_session_transcript(&mut server, id, Hidden, /*config*/ None).await?;
|
||||
let rendered = lines_to_single_string(&cells[0].display_lines(/*width*/ 80));
|
||||
insta::assert_snapshot!(rendered, @"■ offline failure with no items");
|
||||
assert_eq!(cells.len(), 5);
|
||||
for (cell, expected) in cells[1..].iter().zip([
|
||||
"first answer",
|
||||
"older answer",
|
||||
"exported answer",
|
||||
"recent failure",
|
||||
]) {
|
||||
assert!(lines_to_single_string(&cell.display_lines(/*width*/ 80)).contains(expected));
|
||||
}
|
||||
let exported = load_export_transcript(&mut server, id, Hidden, /*config*/ None, vec![])
|
||||
.await
|
||||
.expect("export history");
|
||||
assert_eq!(exported.len(), 3);
|
||||
for (cell, expected) in exported
|
||||
.iter()
|
||||
.zip(["first answer", "older answer", "exported answer"])
|
||||
{
|
||||
assert!(lines_to_single_string(&cell.raw_lines()).contains(expected));
|
||||
}
|
||||
drop(server);
|
||||
let (turn_pages, item_pages) = service.await?;
|
||||
assert_eq!(
|
||||
item_pages,
|
||||
[
|
||||
None,
|
||||
Some(Opaque("end".into())),
|
||||
None,
|
||||
Some(Opaque("end".into()))
|
||||
]
|
||||
);
|
||||
assert_eq!(
|
||||
turn_pages,
|
||||
[
|
||||
(None, Some(5)),
|
||||
(Some("overlap".into()), Some(1)),
|
||||
(Some("middle".into()), Some(1)),
|
||||
(Some("older".into()), Some(100)),
|
||||
(None, Some(5)),
|
||||
(Some("overlap".into()), Some(1)),
|
||||
(Some("middle".into()), Some(1)),
|
||||
]
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -29,6 +29,7 @@ use color_eyre::eyre::Result;
|
||||
use color_eyre::eyre::WrapErr;
|
||||
|
||||
pub(crate) const INITIAL_HISTORY_TURN_LIMIT: u32 = 5;
|
||||
const HISTORY_TURN_PAGE_LIMIT: u32 = 100;
|
||||
pub(crate) const HISTORY_ITEM_PAGE_LIMIT: u32 = 100;
|
||||
pub(crate) const HISTORY_ITEM_SCAN_LIMIT: usize = 4 * HISTORY_ITEM_PAGE_LIMIT as usize;
|
||||
|
||||
@@ -36,6 +37,8 @@ pub(crate) const HISTORY_ITEM_SCAN_LIMIT: usize = 4 * HISTORY_ITEM_PAGE_LIMIT as
|
||||
pub(crate) enum HistoryHydrationScope<'a> {
|
||||
Initial,
|
||||
Complete,
|
||||
/// Include older turns that have no items.
|
||||
FullTranscript,
|
||||
ThroughTurn(&'a str),
|
||||
}
|
||||
|
||||
@@ -68,7 +71,7 @@ impl HistoryLoadBudget {
|
||||
let rows = local_settings
|
||||
.and_then(|settings| resize_reflow_max_rows(settings.terminal_resize_reflow()));
|
||||
let items = match (scope, config, rows) {
|
||||
(HistoryHydrationScope::Complete, _, _)
|
||||
(HistoryHydrationScope::Complete | HistoryHydrationScope::FullTranscript, _, _)
|
||||
| (HistoryHydrationScope::ThroughTurn(_), _, _)
|
||||
| (HistoryHydrationScope::Initial, Some(_), None) => None,
|
||||
(HistoryHydrationScope::Initial, Some(_), Some(max_rows)) => {
|
||||
@@ -127,6 +130,15 @@ fn advancing_cursor(
|
||||
next.filter(|next| seen_cursors.insert(next.clone()))
|
||||
}
|
||||
|
||||
fn prepend_older_turns(turns: &mut Vec<Turn>, mut older: Vec<Turn>) {
|
||||
let mut new_ids: HashSet<_> = older.iter().map(|turn| turn.id.clone()).collect();
|
||||
for known in turns.iter() {
|
||||
new_ids.remove(&known.id);
|
||||
}
|
||||
older.retain(|turn| new_ids.remove(&turn.id));
|
||||
turns.splice(0..0, older.into_iter().rev());
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Default)]
|
||||
pub(crate) struct ThreadHistoryPagination {
|
||||
pub(super) history_mode: ThreadHistoryMode,
|
||||
@@ -307,7 +319,7 @@ impl AppServerSession {
|
||||
for turn in &page.data {
|
||||
missing_turn_ids.remove(&turn.id);
|
||||
}
|
||||
turns.splice(0..0, page.data.into_iter().rev());
|
||||
prepend_older_turns(turns, page.data);
|
||||
}
|
||||
if let Some(turn) = turns.iter_mut().find(|turn| turn.id == entry.turn_id)
|
||||
&& !turn.items.iter().any(|item| item.id() == entry.item.id())
|
||||
@@ -422,6 +434,22 @@ impl AppServerSession {
|
||||
rendered_rows = rendered_rows.saturating_add(items.len());
|
||||
}
|
||||
}
|
||||
// A failed Cloud turn may have only an error and no items.
|
||||
if self.thread_params_mode() == super::ThreadParamsMode::Cloud
|
||||
&& scope == HistoryHydrationScope::FullTranscript
|
||||
{
|
||||
while let Some(cursor) = state.next_turn_cursor.take() {
|
||||
let page = self
|
||||
.thread_turns_page(thread_id, Some(cursor.clone()), HISTORY_TURN_PAGE_LIMIT)
|
||||
.await?;
|
||||
state.next_turn_cursor = advancing_cursor(
|
||||
Some(&cursor),
|
||||
page.next_cursor,
|
||||
&mut state.seen_turn_cursors,
|
||||
);
|
||||
prepend_older_turns(&mut thread.turns, page.data);
|
||||
}
|
||||
}
|
||||
if config.is_some() {
|
||||
self.history_pagination.insert(thread_id, state);
|
||||
}
|
||||
|
||||
@@ -81,7 +81,7 @@ pub(crate) async fn load_session_transcript(
|
||||
/*item_cursor*/ None,
|
||||
/*config*/ None,
|
||||
/*local_settings*/ None,
|
||||
HistoryHydrationScope::Complete,
|
||||
HistoryHydrationScope::FullTranscript,
|
||||
)
|
||||
.await
|
||||
.map_err(std::io::Error::other)?;
|
||||
|
||||
Reference in New Issue
Block a user