diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index b40bfdb396..0f61ce4f85 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -4735,18 +4735,11 @@ async fn persist_pre_turn_items_for_compaction_outcome( response_item: ResponseItem, ) { match outcome { - PreTurnCompactionOutcome::CompactedWithIncomingItems => { - // Incoming turn items were already part of pre-turn compaction input, and the - // user prompt is already persisted in history after compaction. Emit lifecycle events - // only so UI/consumers still observe a normal user turn item transition. - let turn_item = TurnItem::UserMessage(UserMessageItem::new(input)); - sess.emit_turn_item_started(turn_context.as_ref(), &turn_item) - .await; - sess.emit_turn_item_completed(turn_context.as_ref(), turn_item) - .await; - sess.ensure_rollout_materialized().await; - } - PreTurnCompactionOutcome::NotNeeded => { + PreTurnCompactionOutcome::CompactedWithIncomingItems + | PreTurnCompactionOutcome::NotNeeded => { + // Pre-turn compaction includes incoming items only for the compaction model's request. + // We always persist canonical pre-turn updates and the current user item after the + // compaction summary so model-visible layout is stable regardless of compaction mode. if !pre_turn_context_items.is_empty() { sess.record_conversation_items(turn_context, pre_turn_context_items) .await; @@ -6150,7 +6143,7 @@ mod tests { } #[tokio::test] - async fn compacted_with_incoming_items_emits_lifecycle_without_history_writes() { + async fn compacted_with_incoming_items_persists_context_and_prompt() { let (session, turn_context) = make_session_and_context().await; let session = Arc::new(session); let turn_context = Arc::new(turn_context); @@ -6175,12 +6168,13 @@ mod tests { PreTurnCompactionOutcome::CompactedWithIncomingItems, &stale_pre_turn_context_items, &input, - response_item, + response_item.clone(), ) .await; let actual = session.clone_history().await.raw_items().to_vec(); - assert_eq!(actual, Vec::::new()); + let expected = vec![stale_pre_turn_context_items[0].clone(), response_item]; + assert_eq!(actual, expected); } #[tokio::test] diff --git a/codex-rs/core/src/compact.rs b/codex-rs/core/src/compact.rs index 92321eda14..c649c01566 100644 --- a/codex-rs/core/src/compact.rs +++ b/codex-rs/core/src/compact.rs @@ -301,15 +301,6 @@ async fn run_compact_task_inner( let summary_suffix = get_last_assistant_message_from_turn(history_items).unwrap_or_default(); let summary_text = format!("{SUMMARY_PREFIX}\n{summary_suffix}"); let user_messages = collect_user_messages(history_items); - let incoming_user_items = match incoming_items.as_ref() { - Some(items) => items - .iter() - .filter(|item| should_keep_compacted_history_item(item)) - .cloned() - .collect(), - None => Vec::new(), - }; - let initial_context = match turn_context_reinjection { TurnContextReinjection::ReinjectAboveLastRealUser => { sess.build_initial_context(turn_context.as_ref()).await @@ -318,7 +309,6 @@ async fn run_compact_task_inner( }; let compacted_history = build_compacted_history_with_limit( &user_messages, - &incoming_user_items, &summary_text, COMPACT_USER_MESSAGE_MAX_TOKENS, ); @@ -327,9 +317,12 @@ async fn run_compact_task_inner( &initial_context, turn_context_reinjection, ); - // Reattach the stripped model-switch update only after successful compaction so the model - // still sees the switch instructions on the next real sampling request. - if let Some(model_switch_item) = stripped_model_switch_item { + // Reattach stripped model-switch updates only for compaction paths that do not carry + // incoming turn items. Pre-turn compaction appends turn context and user input after + // compaction in run_turn. + if incoming_items.is_none() + && let Some(model_switch_item) = stripped_model_switch_item + { new_history.push(model_switch_item); } let ghost_snapshots: Vec = history_items @@ -465,7 +458,7 @@ fn is_user_shell_command_record(item: &ResponseItem) -> bool { /// /// This intentionally keeps compaction-generated summary messages because they /// parse as `TurnItem::UserMessage`. -fn should_keep_compacted_history_item(item: &ResponseItem) -> bool { +pub(crate) fn should_keep_compacted_history_item(item: &ResponseItem) -> bool { match item { ResponseItem::Message { role, .. } => { if role == "developer" { @@ -503,17 +496,11 @@ pub(crate) fn build_compacted_history( user_messages: &[String], summary_text: &str, ) -> Vec { - build_compacted_history_with_limit( - user_messages, - &[], - summary_text, - COMPACT_USER_MESSAGE_MAX_TOKENS, - ) + build_compacted_history_with_limit(user_messages, summary_text, COMPACT_USER_MESSAGE_MAX_TOKENS) } fn build_compacted_history_with_limit( user_messages: &[String], - incoming_user_items: &[ResponseItem], summary_text: &str, max_tokens: usize, ) -> Vec { @@ -550,8 +537,6 @@ fn build_compacted_history_with_limit( }); } - history.extend(incoming_user_items.iter().cloned()); - let summary_text = if summary_text.is_empty() { "(no summary available)".to_string() } else { @@ -920,7 +905,6 @@ do things let big = "word ".repeat(200); let history = super::build_compacted_history_with_limit( std::slice::from_ref(&big), - &[], "SUMMARY", max_tokens, ); @@ -976,28 +960,9 @@ do things } #[test] - fn build_compacted_history_preserves_incoming_user_item_structure() { - let preserved_user_item = ResponseItem::Message { - id: None, - role: "user".to_string(), - content: vec![ - ContentItem::InputImage { - image_url: "data:image/png;base64,AAAA".to_string(), - }, - ContentItem::InputText { - text: "latest user with image".to_string(), - }, - ], - end_turn: None, - phase: None, - }; - - let history = super::build_compacted_history_with_limit( - &["older user".to_string()], - std::slice::from_ref(&preserved_user_item), - "SUMMARY", - 128, - ); + fn build_compacted_history_preserves_user_message_structure() { + let history = + super::build_compacted_history_with_limit(&["older user".to_string()], "SUMMARY", 128); let expected = vec![ ResponseItem::Message { @@ -1009,7 +974,6 @@ do things end_turn: None, phase: None, }, - preserved_user_item, ResponseItem::Message { id: None, role: "user".to_string(), diff --git a/codex-rs/core/src/compact_remote.rs b/codex-rs/core/src/compact_remote.rs index 4768abe0d6..f91b270497 100644 --- a/codex-rs/core/src/compact_remote.rs +++ b/codex-rs/core/src/compact_remote.rs @@ -7,6 +7,7 @@ use crate::compact::AutoCompactCallsite; use crate::compact::TurnContextReinjection; use crate::compact::extract_latest_model_switch_update_from_items; use crate::compact::extract_trailing_model_switch_update_for_compaction_request; +use crate::compact::should_keep_compacted_history_item; use crate::context_manager::ContextManager; use crate::context_manager::TotalTokenUsageBreakdown; use crate::context_manager::estimate_item_token_count; @@ -122,7 +123,7 @@ async fn run_remote_compact_task_inner_impl( &base_instructions, incoming_items.as_deref(), ); - if let Some(incoming_items) = incoming_items { + if let Some(incoming_items) = incoming_items.as_ref() { history.record_items(incoming_items.iter(), turn_context.truncation_policy); } if !history.raw_items().iter().any(is_user_turn_boundary) { @@ -182,9 +183,52 @@ async fn run_remote_compact_task_inner_impl( new_history = sess .process_compacted_history(turn_context, new_history, turn_context_reinjection) .await; - // Reattach the stripped model-switch update only after successful compaction so the model - // still sees the switch instructions on the next real sampling request. - if let Some(model_switch_item) = stripped_model_switch_item { + if let Some(incoming_items) = incoming_items.as_ref() { + let incoming_history_items: Vec = incoming_items + .iter() + .filter(|item| should_keep_compacted_history_item(item)) + .cloned() + .collect(); + for incoming_item in incoming_history_items { + if let Some(index) = + new_history + .iter() + .rposition(|candidate| match (candidate, &incoming_item) { + ( + ResponseItem::Message { + role: candidate_role, + content: candidate_content, + .. + }, + ResponseItem::Message { + role: incoming_role, + content: incoming_content, + .. + }, + ) => { + candidate_role == incoming_role && candidate_content == incoming_content + } + ( + ResponseItem::Compaction { + encrypted_content: candidate_content, + }, + ResponseItem::Compaction { + encrypted_content: incoming_content, + }, + ) => candidate_content == incoming_content, + _ => candidate == &incoming_item, + }) + { + new_history.remove(index); + } + } + } + // Reattach stripped model-switch updates only for compaction paths that do not carry + // incoming turn items. Pre-turn compaction appends turn context and user input after + // compaction in run_turn. + if incoming_items.is_none() + && let Some(model_switch_item) = stripped_model_switch_item + { new_history.push(model_switch_item); } diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__compact__pre_turn_compaction_including_incoming_shapes.snap b/codex-rs/core/tests/suite/snapshots/all__suite__compact__pre_turn_compaction_including_incoming_shapes.snap index bd4eb41236..c30d304879 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__compact__pre_turn_compaction_including_incoming_shapes.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__compact__pre_turn_compaction_including_incoming_shapes.snap @@ -1,5 +1,6 @@ --- source: core/tests/suite/compact.rs +assertion_line: 3138 expression: "format_labeled_requests_snapshot(\"Pre-turn auto-compaction with a context override includes incoming user content in the compact request and preserves it after compaction.\",\n&[(\"Local Compaction Request\", &requests[2]),\n(\"Local Post-Compaction History Layout\", &requests[3]),])" --- Scenario: Pre-turn auto-compaction with a context override includes incoming user content in the compact request and preserves it after compaction. @@ -18,9 +19,10 @@ Scenario: Pre-turn auto-compaction with a context override includes incoming use ## Local Post-Compaction History Layout 00:message/user:USER_ONE -01:message/user:USER_TWO -02:message/developer: -03:message/user: -04:message/user: -05:message/user: | | | USER_THREE -06:message/user:\nPRE_TURN_SUMMARY +01:message/developer: +02:message/user: +03:message/user: +04:message/user:USER_TWO +05:message/user:\nPRE_TURN_SUMMARY +06:message/user: +07:message/user: | | | USER_THREE diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__compact__pre_turn_compaction_strips_incoming_model_switch_shapes.snap b/codex-rs/core/tests/suite/snapshots/all__suite__compact__pre_turn_compaction_strips_incoming_model_switch_shapes.snap index 48b721f2c2..fcbad1059b 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__compact__pre_turn_compaction_strips_incoming_model_switch_shapes.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__compact__pre_turn_compaction_strips_incoming_model_switch_shapes.snap @@ -1,5 +1,6 @@ --- source: core/tests/suite/compact.rs +assertion_line: 3288 expression: "format_labeled_requests_snapshot(\"Pre-turn compaction during model switch (without pre-sampling model-switch compaction): current behavior strips incoming from the compact request and restores it in the post-compaction follow-up request.\",\n&[(\"Initial Request (Previous Model)\", &requests[0]),\n(\"Local Compaction Request\", &requests[1]),\n(\"Local Post-Compaction History Layout\", &requests[2]),])" --- Scenario: Pre-turn compaction during model switch (without pre-sampling model-switch compaction): current behavior strips incoming from the compact request and restores it in the post-compaction follow-up request. @@ -22,11 +23,11 @@ Scenario: Pre-turn compaction during model switch (without pre-sampling model-sw 07:message/user: ## Local Post-Compaction History Layout -00:message/user:BEFORE_SWITCH_USER -01:message/developer: -02:message/developer: The user has requested a new communication st... -03:message/user: -04:message/user:> -05:message/user:AFTER_SWITCH_USER -06:message/user:\nPRETURN_SWITCH_SUMMARY -07:message/developer:\nThe user was previously using a different model.... +00:message/developer: +01:message/developer: The user has requested a new communication st... +02:message/user: +03:message/user:> +04:message/user:BEFORE_SWITCH_USER +05:message/user:\nPRETURN_SWITCH_SUMMARY +06:message/developer:\nThe user was previously using a different model.... +07:message/user:AFTER_SWITCH_USER diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__compact_remote__remote_pre_turn_compaction_including_incoming_shapes.snap b/codex-rs/core/tests/suite/snapshots/all__suite__compact_remote__remote_pre_turn_compaction_including_incoming_shapes.snap index ec30aa0861..7b28f8151a 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__compact_remote__remote_pre_turn_compaction_including_incoming_shapes.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__compact_remote__remote_pre_turn_compaction_including_incoming_shapes.snap @@ -1,5 +1,6 @@ --- source: core/tests/suite/compact_remote.rs +assertion_line: 1390 expression: "format_labeled_requests_snapshot(\"Remote pre-turn auto-compaction with a context override includes incoming user content in the compact request and preserves it after compaction.\",\n&[(\"Remote Compaction Request\", &compact_request),\n(\"Remote Post-Compaction History Layout\", &requests[2]),])" --- Scenario: Remote pre-turn auto-compaction with a context override includes incoming user content in the compact request and preserves it after compaction. @@ -21,5 +22,6 @@ Scenario: Remote pre-turn auto-compaction with a context override includes incom 02:message/developer: 03:message/user: 04:message/user: -05:message/user:USER_THREE -06:message/user:\nREMOTE_PRE_TURN_SUMMARY +05:message/user:\nREMOTE_PRE_TURN_SUMMARY +06:message/user: +07:message/user:USER_THREE diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__compact_remote__remote_pre_turn_compaction_strips_incoming_model_switch_shapes.snap b/codex-rs/core/tests/suite/snapshots/all__suite__compact_remote__remote_pre_turn_compaction_strips_incoming_model_switch_shapes.snap index 0a789f5b69..2b03802b7f 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__compact_remote__remote_pre_turn_compaction_strips_incoming_model_switch_shapes.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__compact_remote__remote_pre_turn_compaction_strips_incoming_model_switch_shapes.snap @@ -1,6 +1,7 @@ --- source: core/tests/suite/compact_remote.rs -expression: "format_labeled_requests_snapshot(\"Remote pre-turn compaction during model switch strips incoming from the compact request and restores it in the post-compaction follow-up request.\",\n&[(\"Initial Request (Previous Model)\", &requests[0]),\n(\"Remote Compaction Request\", &compact_request),\n(\"Remote Post-Compaction History Layout\", &requests[1]),])" +assertion_line: 1538 +expression: "format_labeled_requests_snapshot(\"Remote pre-turn compaction during model switch strips incoming from the compact request and restores it in the post-compaction follow-up request.\",\n&[(\"Initial Request (Previous Model)\", &initial_turn_request),\n(\"Remote Compaction Request\", &compact_request),\n(\"Remote Post-Compaction History Layout\", &post_compact_turn_request),])" --- Scenario: Remote pre-turn compaction during model switch strips incoming from the compact request and restores it in the post-compaction follow-up request. @@ -24,6 +25,6 @@ Scenario: Remote pre-turn compaction during model switch strips incoming The user has requested a new communication st... 03:message/user: 04:message/user:> -05:message/user:AFTER_SWITCH_USER -06:message/user:\nREMOTE_SWITCH_SUMMARY -07:message/developer:\nThe user was previously using a different model.... +05:message/user:\nREMOTE_SWITCH_SUMMARY +06:message/developer:\nThe user was previously using a different model.... +07:message/user:AFTER_SWITCH_USER