From 7095ff179e80d53c5062f1fb7e7a1e5f105c9d94 Mon Sep 17 00:00:00 2001 From: Anton Panasenko Date: Mon, 29 Jun 2026 15:50:48 -0700 Subject: [PATCH] codex: address PR review feedback (#30618) --- codex-rs/core/src/lib.rs | 1 + ...ch_history.rs => response_item_history.rs} | 3 +- codex-rs/core/src/session/mod.rs | 38 ++++++++++--------- 3 files changed, 24 insertions(+), 18 deletions(-) rename codex-rs/core/src/{session/tool_search_history.rs => response_item_history.rs} (94%) diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index e9b749fea9..754262a736 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -12,6 +12,7 @@ mod client_common; mod realtime_context; mod realtime_conversation; mod realtime_prompt; +mod response_item_history; mod responses_metadata; mod responses_retry; pub(crate) mod session; diff --git a/codex-rs/core/src/session/tool_search_history.rs b/codex-rs/core/src/response_item_history.rs similarity index 94% rename from codex-rs/core/src/session/tool_search_history.rs rename to codex-rs/core/src/response_item_history.rs index 600f57dace..2a964f0c0b 100644 --- a/codex-rs/core/src/session/tool_search_history.rs +++ b/codex-rs/core/src/response_item_history.rs @@ -5,8 +5,9 @@ use serde::Deserialize; use std::borrow::Cow; use tracing::warn; -pub(super) const INVALID_TOOL_SEARCH_QUERY: &str = "[invalid tool_search arguments omitted]"; +const INVALID_TOOL_SEARCH_QUERY: &str = "[invalid tool_search arguments omitted]"; +/// Applies response-item-specific normalization before model-visible history and persistence. pub(super) fn prepare_for_durable_history<'a>( thread_id: &ThreadId, items: &'a [ResponseItem], diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 3fd2b9451a..7f7d1bbc7a 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -216,7 +216,6 @@ pub(crate) mod session; pub(crate) mod step_context; pub(crate) mod time_reminder; mod token_budget; -mod tool_search_history; pub(crate) mod turn; pub(crate) mod turn_context; mod world_state; @@ -2710,14 +2709,16 @@ impl Session { } } - fn prepare_raw_response_items<'a>( + /// Records conversation items: append to history, persist to rollout, and + /// notify clients observing raw response items. + pub(crate) fn prepare_conversation_items_for_history<'a>( &self, turn_context: &TurnContext, items: &'a [ResponseItem], ) -> Cow<'a, [ResponseItem]> { let mut items = Cow::Borrowed(items); prepare_response_items(items.to_mut()); - // Most response items get their passthrough turn ID at the recording boundary. + // Most response items get their passthrough turn ID at the durable history boundary. for item in items.to_mut() { item.set_turn_id_if_missing(&turn_context.sub_id); } @@ -2776,20 +2777,23 @@ impl Session { turn_context: &TurnContext, items: &[ResponseItem], ) { - let raw_items = self.prepare_raw_response_items(turn_context, items); - let history_items = - tool_search_history::prepare_for_durable_history(&self.thread_id, raw_items.as_ref()); + let raw_items = self.prepare_conversation_items_for_history(turn_context, items); + let history_items = crate::response_item_history::prepare_for_durable_history( + &self.thread_id, + raw_items.as_ref(), + ); { let mut state = self.state.lock().await; state .current_time_reminder - .note_recorded_items(&history_items); + .note_recorded_items(history_items.as_ref()); state.record_items( history_items.iter(), turn_context.model_info.truncation_policy.into(), ); } - self.persist_rollout_response_items(&history_items).await; + self.persist_rollout_response_items(history_items.as_ref()) + .await; self.send_raw_response_items(turn_context, raw_items.as_ref()) .await; } @@ -2881,16 +2885,17 @@ impl Session { ) { communication.set_turn_id_if_missing(&turn_context.sub_id); let response_item = communication.to_model_input_item(); - let raw_items = - self.prepare_raw_response_items(turn_context, std::slice::from_ref(&response_item)); - let response_item = raw_items[0].clone(); + let items = self.prepare_conversation_items_for_history( + turn_context, + std::slice::from_ref(&response_item), + ); + let items = items.as_ref(); + let response_item = items[0].clone(); { let mut state = self.state.lock().await; - state - .current_time_reminder - .note_recorded_items(raw_items.as_ref()); + state.current_time_reminder.note_recorded_items(items); state.record_items( - raw_items.iter(), + items.iter(), turn_context.model_info.truncation_policy.into(), ); } @@ -2901,8 +2906,7 @@ impl Session { RolloutItem::ResponseItem(response_item), ]) .await; - self.send_raw_response_items(turn_context, raw_items.as_ref()) - .await; + self.send_raw_response_items(turn_context, items).await; } async fn maybe_warn_on_server_model_mismatch(