mirror of
https://github.com/openai/codex.git
synced 2026-09-13 11:47:17 +00:00
core: simplify compaction tail merge helper
Rename the append-only compaction merge helper, switch it to a boolean success signal, and document the exact concurrency assumption. Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -212,13 +212,11 @@ async fn run_compact_task_inner(
|
||||
.collect();
|
||||
new_history.extend(ghost_snapshots);
|
||||
let latest_history_snapshot = sess.clone_history().await;
|
||||
if merge_appended_history_items(
|
||||
if !append_concurrent_history_tail_if_append_only(
|
||||
&mut new_history,
|
||||
history_items,
|
||||
latest_history_snapshot.raw_items(),
|
||||
)
|
||||
.is_none()
|
||||
{
|
||||
) {
|
||||
warn!(
|
||||
turn_id = %turn_context.sub_id,
|
||||
"session history changed non-append-only during compaction; skipping concurrent tail merge"
|
||||
@@ -284,21 +282,28 @@ pub(crate) fn is_summary_message(message: &str) -> bool {
|
||||
message.starts_with(format!("{SUMMARY_PREFIX}\n").as_str())
|
||||
}
|
||||
|
||||
pub(crate) fn merge_appended_history_items(
|
||||
/// Appends items added after `base_history` only when `latest_history` still
|
||||
/// preserves `base_history` as an exact prefix.
|
||||
///
|
||||
/// Returns `true` when no concurrent history change occurred or when the newer
|
||||
/// history differs only by append-only tail growth, in which case that tail is
|
||||
/// appended to `new_history`. Returns `false` when concurrent history mutation
|
||||
/// rewrote or removed earlier items, since this helper cannot safely merge that
|
||||
/// shape.
|
||||
pub(crate) fn append_concurrent_history_tail_if_append_only(
|
||||
new_history: &mut Vec<ResponseItem>,
|
||||
base_history: &[ResponseItem],
|
||||
latest_history: &[ResponseItem],
|
||||
) -> Option<usize> {
|
||||
) -> bool {
|
||||
if latest_history == base_history {
|
||||
return Some(0);
|
||||
return true;
|
||||
}
|
||||
if !latest_history.starts_with(base_history) {
|
||||
return None;
|
||||
return false;
|
||||
}
|
||||
|
||||
let appended_count = latest_history.len() - base_history.len();
|
||||
new_history.extend_from_slice(&latest_history[base_history.len()..]);
|
||||
Some(appended_count)
|
||||
true
|
||||
}
|
||||
|
||||
/// Inserts canonical initial context into compacted replacement history at the
|
||||
|
||||
@@ -6,8 +6,8 @@ use crate::codex::Session;
|
||||
use crate::codex::TurnContext;
|
||||
use crate::codex::built_tools;
|
||||
use crate::compact::InitialContextInjection;
|
||||
use crate::compact::append_concurrent_history_tail_if_append_only;
|
||||
use crate::compact::insert_initial_context_before_last_real_user_or_summary;
|
||||
use crate::compact::merge_appended_history_items;
|
||||
use crate::context_manager::ContextManager;
|
||||
use crate::context_manager::TotalTokenUsageBreakdown;
|
||||
use crate::context_manager::estimate_response_item_model_visible_bytes;
|
||||
@@ -152,13 +152,11 @@ async fn run_remote_compact_task_inner_impl(
|
||||
new_history.extend(ghost_snapshots);
|
||||
}
|
||||
let latest_history_snapshot = sess.clone_history().await;
|
||||
if merge_appended_history_items(
|
||||
if !append_concurrent_history_tail_if_append_only(
|
||||
&mut new_history,
|
||||
history_snapshot.raw_items(),
|
||||
latest_history_snapshot.raw_items(),
|
||||
)
|
||||
.is_none()
|
||||
{
|
||||
) {
|
||||
warn!(
|
||||
turn_id = %turn_context.sub_id,
|
||||
"session history changed non-append-only during remote compaction; skipping concurrent tail merge"
|
||||
|
||||
@@ -186,7 +186,7 @@ fn build_token_limited_compacted_history_appends_summary_message() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_appended_history_items_appends_concurrent_tail() {
|
||||
fn append_concurrent_history_tail_if_append_only_appends_concurrent_tail() {
|
||||
let base_history = vec![ResponseItem::Message {
|
||||
id: None,
|
||||
role: "user".to_string(),
|
||||
@@ -215,10 +215,13 @@ fn merge_appended_history_items_appends_concurrent_tail() {
|
||||
phase: None,
|
||||
}];
|
||||
|
||||
let merged =
|
||||
merge_appended_history_items(&mut compacted_history, &base_history, &latest_history);
|
||||
let merged = append_concurrent_history_tail_if_append_only(
|
||||
&mut compacted_history,
|
||||
&base_history,
|
||||
&latest_history,
|
||||
);
|
||||
|
||||
assert_eq!(merged, Some(1));
|
||||
assert!(merged);
|
||||
assert_eq!(
|
||||
compacted_history,
|
||||
vec![
|
||||
@@ -237,7 +240,7 @@ fn merge_appended_history_items_appends_concurrent_tail() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_appended_history_items_rejects_non_append_only_changes() {
|
||||
fn append_concurrent_history_tail_if_append_only_rejects_non_append_only_changes() {
|
||||
let base_history = vec![ResponseItem::Message {
|
||||
id: None,
|
||||
role: "user".to_string(),
|
||||
@@ -266,10 +269,13 @@ fn merge_appended_history_items_rejects_non_append_only_changes() {
|
||||
phase: None,
|
||||
}];
|
||||
|
||||
let merged =
|
||||
merge_appended_history_items(&mut compacted_history, &base_history, &latest_history);
|
||||
let merged = append_concurrent_history_tail_if_append_only(
|
||||
&mut compacted_history,
|
||||
&base_history,
|
||||
&latest_history,
|
||||
);
|
||||
|
||||
assert_eq!(merged, None);
|
||||
assert!(!merged);
|
||||
assert_eq!(
|
||||
compacted_history,
|
||||
vec![ResponseItem::Message {
|
||||
|
||||
Reference in New Issue
Block a user