mirror of
https://github.com/openai/codex.git
synced 2026-10-30 19:18:49 +00:00
Append incoming pre-turn items after compaction summary
This commit is contained in:
@@ -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::<ResponseItem>::new());
|
||||
let expected = vec![stale_pre_turn_context_items[0].clone(), response_item];
|
||||
assert_eq!(actual, expected);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -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<ResponseItem> = 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<ResponseItem> {
|
||||
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<ResponseItem> {
|
||||
@@ -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(),
|
||||
|
||||
@@ -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<ResponseItem> = 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);
|
||||
}
|
||||
|
||||
|
||||
+8
-6
@@ -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:<PERMISSIONS_INSTRUCTIONS>
|
||||
03:message/user:<AGENTS_MD>
|
||||
04:message/user:<ENVIRONMENT_CONTEXT:cwd=PRETURN_CONTEXT_DIFF_CWD>
|
||||
05:message/user:<image> | <input_image:image_url> | </image> | USER_THREE
|
||||
06:message/user:<COMPACTION_SUMMARY>\nPRE_TURN_SUMMARY
|
||||
01:message/developer:<PERMISSIONS_INSTRUCTIONS>
|
||||
02:message/user:<AGENTS_MD>
|
||||
03:message/user:<ENVIRONMENT_CONTEXT:cwd=PRETURN_CONTEXT_DIFF_CWD>
|
||||
04:message/user:USER_TWO
|
||||
05:message/user:<COMPACTION_SUMMARY>\nPRE_TURN_SUMMARY
|
||||
06:message/user:<ENVIRONMENT_CONTEXT:cwd=PRETURN_CONTEXT_DIFF_CWD>
|
||||
07:message/user:<image> | <input_image:image_url> | </image> | USER_THREE
|
||||
|
||||
+9
-8
@@ -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 <model_switch> 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 <model_switch> 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:<SUMMARIZATION_PROMPT>
|
||||
|
||||
## Local Post-Compaction History Layout
|
||||
00:message/user:BEFORE_SWITCH_USER
|
||||
01:message/developer:<PERMISSIONS_INSTRUCTIONS>
|
||||
02:message/developer:<personality_spec> The user has requested a new communication st...
|
||||
03:message/user:<AGENTS_MD>
|
||||
04:message/user:<ENVIRONMENT_CONTEXT:cwd=<CWD>>
|
||||
05:message/user:AFTER_SWITCH_USER
|
||||
06:message/user:<COMPACTION_SUMMARY>\nPRETURN_SWITCH_SUMMARY
|
||||
07:message/developer:<model_switch>\nThe user was previously using a different model....
|
||||
00:message/developer:<PERMISSIONS_INSTRUCTIONS>
|
||||
01:message/developer:<personality_spec> The user has requested a new communication st...
|
||||
02:message/user:<AGENTS_MD>
|
||||
03:message/user:<ENVIRONMENT_CONTEXT:cwd=<CWD>>
|
||||
04:message/user:BEFORE_SWITCH_USER
|
||||
05:message/user:<COMPACTION_SUMMARY>\nPRETURN_SWITCH_SUMMARY
|
||||
06:message/developer:<model_switch>\nThe user was previously using a different model....
|
||||
07:message/user:AFTER_SWITCH_USER
|
||||
|
||||
+4
-2
@@ -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:<PERMISSIONS_INSTRUCTIONS>
|
||||
03:message/user:<AGENTS_MD>
|
||||
04:message/user:<ENVIRONMENT_CONTEXT:cwd=PRETURN_CONTEXT_DIFF_CWD>
|
||||
05:message/user:USER_THREE
|
||||
06:message/user:<COMPACTION_SUMMARY>\nREMOTE_PRE_TURN_SUMMARY
|
||||
05:message/user:<COMPACTION_SUMMARY>\nREMOTE_PRE_TURN_SUMMARY
|
||||
06:message/user:<ENVIRONMENT_CONTEXT:cwd=PRETURN_CONTEXT_DIFF_CWD>
|
||||
07:message/user:USER_THREE
|
||||
|
||||
+5
-4
@@ -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 <model_switch> 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 <model_switch> 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 <model_switch> 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 <model_
|
||||
02:message/developer:<personality_spec> The user has requested a new communication st...
|
||||
03:message/user:<AGENTS_MD>
|
||||
04:message/user:<ENVIRONMENT_CONTEXT:cwd=<CWD>>
|
||||
05:message/user:AFTER_SWITCH_USER
|
||||
06:message/user:<COMPACTION_SUMMARY>\nREMOTE_SWITCH_SUMMARY
|
||||
07:message/developer:<model_switch>\nThe user was previously using a different model....
|
||||
05:message/user:<COMPACTION_SUMMARY>\nREMOTE_SWITCH_SUMMARY
|
||||
06:message/developer:<model_switch>\nThe user was previously using a different model....
|
||||
07:message/user:AFTER_SWITCH_USER
|
||||
|
||||
Reference in New Issue
Block a user