Handle model-switch base instructions after compaction (#11659)
Strip trailing <model_switch> during model-switch compaction request, and append <model_switch> after model switch compaction
This commit is contained in:
parent
8156c57234
commit
67e577da53
4 changed files with 174 additions and 0 deletions
|
|
@ -1932,6 +1932,19 @@ impl Session {
|
|||
Some(DeveloperInstructions::model_switch_message(model_instructions).into())
|
||||
}
|
||||
|
||||
pub(crate) fn is_model_switch_developer_message(item: &ResponseItem) -> bool {
|
||||
let ResponseItem::Message { role, content, .. } = item else {
|
||||
return false;
|
||||
};
|
||||
role == "developer"
|
||||
&& content.iter().any(|content_item| {
|
||||
matches!(
|
||||
content_item,
|
||||
ContentItem::InputText { text } if text.starts_with("<model_switch>")
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
fn build_settings_update_items(
|
||||
&self,
|
||||
previous_context: Option<&Arc<TurnContext>>,
|
||||
|
|
@ -4438,6 +4451,12 @@ async fn run_pre_sampling_compact(
|
|||
Ok(())
|
||||
}
|
||||
|
||||
/// Runs pre-sampling compaction against the previous model when switching to a smaller
|
||||
/// context-window model.
|
||||
///
|
||||
/// Returns `Ok(())` when compaction either completed successfully or was skipped because the
|
||||
/// model/context-window preconditions were not met. Returns `Err(_)` only when compaction was
|
||||
/// attempted and failed.
|
||||
async fn maybe_run_previous_model_inline_compact(
|
||||
sess: &Arc<Session>,
|
||||
turn_context: &Arc<TurnContext>,
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ use crate::client_common::ResponseEvent;
|
|||
use crate::codex::Session;
|
||||
use crate::codex::TurnContext;
|
||||
use crate::codex::get_last_assistant_message_from_turn;
|
||||
use crate::context_manager::ContextManager;
|
||||
use crate::error::CodexErr;
|
||||
use crate::error::Result as CodexResult;
|
||||
use crate::protocol::CompactedItem;
|
||||
|
|
@ -35,6 +36,31 @@ pub(crate) fn should_use_remote_compact_task(provider: &ModelProviderInfo) -> bo
|
|||
provider.is_openai()
|
||||
}
|
||||
|
||||
pub(crate) fn extract_trailing_model_switch_update_for_compaction_request(
|
||||
history: &mut ContextManager,
|
||||
) -> Option<ResponseItem> {
|
||||
let history_items = history.raw_items();
|
||||
let last_user_turn_boundary_index = history_items
|
||||
.iter()
|
||||
.rposition(crate::context_manager::is_user_turn_boundary);
|
||||
let model_switch_index = history_items
|
||||
.iter()
|
||||
.enumerate()
|
||||
.rev()
|
||||
.find_map(|(i, item)| {
|
||||
let is_trailing = last_user_turn_boundary_index.is_none_or(|boundary| i > boundary);
|
||||
if is_trailing && Session::is_model_switch_developer_message(item) {
|
||||
Some(i)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
})?;
|
||||
let mut replacement = history_items.to_vec();
|
||||
let model_switch_item = replacement.remove(model_switch_index);
|
||||
history.replace(replacement);
|
||||
Some(model_switch_item)
|
||||
}
|
||||
|
||||
pub(crate) async fn run_inline_auto_compact_task(
|
||||
sess: Arc<Session>,
|
||||
turn_context: Arc<TurnContext>,
|
||||
|
|
@ -75,6 +101,10 @@ async fn run_compact_task_inner(
|
|||
let initial_input_for_turn: ResponseInputItem = ResponseInputItem::from(input);
|
||||
|
||||
let mut history = sess.clone_history().await;
|
||||
// Keep compaction prompts in-distribution: if a model-switch update was injected at the
|
||||
// tail of history (between turns), exclude it from the compaction request payload.
|
||||
let stripped_model_switch_item =
|
||||
extract_trailing_model_switch_update_for_compaction_request(&mut history);
|
||||
history.record_items(
|
||||
&[initial_input_for_turn.into()],
|
||||
turn_context.truncation_policy,
|
||||
|
|
@ -180,6 +210,11 @@ async fn run_compact_task_inner(
|
|||
|
||||
let initial_context = sess.build_initial_context(turn_context.as_ref()).await;
|
||||
let mut new_history = build_compacted_history(initial_context, &user_messages, &summary_text);
|
||||
// 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 {
|
||||
new_history.push(model_switch_item);
|
||||
}
|
||||
let ghost_snapshots: Vec<ResponseItem> = history_items
|
||||
.iter()
|
||||
.filter(|item| matches!(item, ResponseItem::GhostSnapshot { .. }))
|
||||
|
|
@ -443,6 +478,107 @@ mod tests {
|
|||
assert_eq!(None, joined);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extract_trailing_model_switch_update_for_compaction_request_removes_trailing_item() {
|
||||
let mut history = ContextManager::new();
|
||||
history.replace(vec![
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
role: "user".to_string(),
|
||||
content: vec![ContentItem::InputText {
|
||||
text: "USER_MESSAGE".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
role: "assistant".to_string(),
|
||||
content: vec![ContentItem::OutputText {
|
||||
text: "ASSISTANT_REPLY".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
role: "developer".to_string(),
|
||||
content: vec![ContentItem::InputText {
|
||||
text: "<model_switch>\nNEW_MODEL_INSTRUCTIONS".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
]);
|
||||
|
||||
let model_switch_item =
|
||||
extract_trailing_model_switch_update_for_compaction_request(&mut history);
|
||||
|
||||
assert_eq!(history.raw_items().len(), 2);
|
||||
assert!(model_switch_item.is_some());
|
||||
assert!(
|
||||
history
|
||||
.raw_items()
|
||||
.iter()
|
||||
.all(|item| !Session::is_model_switch_developer_message(item))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extract_trailing_model_switch_update_for_compaction_request_keeps_historical_item() {
|
||||
let mut history = ContextManager::new();
|
||||
history.replace(vec![
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
role: "user".to_string(),
|
||||
content: vec![ContentItem::InputText {
|
||||
text: "FIRST_USER_MESSAGE".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
role: "developer".to_string(),
|
||||
content: vec![ContentItem::InputText {
|
||||
text: "<model_switch>\nOLDER_MODEL_INSTRUCTIONS".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
role: "assistant".to_string(),
|
||||
content: vec![ContentItem::OutputText {
|
||||
text: "ASSISTANT_REPLY".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
ResponseItem::Message {
|
||||
id: None,
|
||||
role: "user".to_string(),
|
||||
content: vec![ContentItem::InputText {
|
||||
text: "SECOND_USER_MESSAGE".to_string(),
|
||||
}],
|
||||
end_turn: None,
|
||||
phase: None,
|
||||
},
|
||||
]);
|
||||
|
||||
let model_switch_item =
|
||||
extract_trailing_model_switch_update_for_compaction_request(&mut history);
|
||||
|
||||
assert_eq!(history.raw_items().len(), 4);
|
||||
assert!(model_switch_item.is_none());
|
||||
assert!(
|
||||
history
|
||||
.raw_items()
|
||||
.iter()
|
||||
.any(Session::is_model_switch_developer_message)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn collect_user_messages_extracts_user_text_only() {
|
||||
let items = vec![
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ use std::sync::Arc;
|
|||
use crate::Prompt;
|
||||
use crate::codex::Session;
|
||||
use crate::codex::TurnContext;
|
||||
use crate::compact::extract_trailing_model_switch_update_for_compaction_request;
|
||||
use crate::context_manager::ContextManager;
|
||||
use crate::context_manager::TotalTokenUsageBreakdown;
|
||||
use crate::context_manager::estimate_response_item_model_visible_bytes;
|
||||
|
|
@ -65,6 +66,10 @@ async fn run_remote_compact_task_inner_impl(
|
|||
sess.emit_turn_item_started(turn_context, &compaction_item)
|
||||
.await;
|
||||
let mut history = sess.clone_history().await;
|
||||
// Keep compaction prompts in-distribution: if a model-switch update was injected at the
|
||||
// tail of history (between turns), exclude it from the compaction request payload.
|
||||
let stripped_model_switch_item =
|
||||
extract_trailing_model_switch_update_for_compaction_request(&mut history);
|
||||
let base_instructions = sess.get_base_instructions().await;
|
||||
let deleted_items = trim_function_call_history_to_fit_context_window(
|
||||
&mut history,
|
||||
|
|
@ -120,6 +125,11 @@ async fn run_remote_compact_task_inner_impl(
|
|||
new_history = sess
|
||||
.process_compacted_history(turn_context, new_history)
|
||||
.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 {
|
||||
new_history.push(model_switch_item);
|
||||
}
|
||||
|
||||
if !ghost_snapshots.is_empty() {
|
||||
new_history.extend(ghost_snapshots);
|
||||
|
|
|
|||
|
|
@ -142,6 +142,15 @@ fn assert_pre_sampling_switch_compaction_requests(
|
|||
body_contains_text(&compact_body, SUMMARIZATION_PROMPT),
|
||||
"pre-sampling compact request should include summarization prompt"
|
||||
);
|
||||
assert!(
|
||||
!compact_body.contains("<model_switch>"),
|
||||
"pre-sampling compact request should strip trailing model-switch update item"
|
||||
);
|
||||
let follow_up_body = follow_up.to_string();
|
||||
assert!(
|
||||
follow_up_body.contains("<model_switch>"),
|
||||
"follow-up request after successful model-switch compaction should include model-switch update item"
|
||||
);
|
||||
}
|
||||
|
||||
async fn assert_compaction_uses_turn_lifecycle_id(codex: &std::sync::Arc<codex_core::CodexThread>) {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue