diff --git a/codex-rs/core/src/rollout/mod.rs b/codex-rs/core/src/rollout/mod.rs index 19c66a19..c6b4fc58 100644 --- a/codex-rs/core/src/rollout/mod.rs +++ b/codex-rs/core/src/rollout/mod.rs @@ -3,6 +3,7 @@ pub(crate) const SESSIONS_SUBDIR: &str = "sessions"; pub mod list; +pub(crate) mod policy; pub mod recorder; pub use recorder::RolloutRecorder; diff --git a/codex-rs/core/src/rollout/policy.rs b/codex-rs/core/src/rollout/policy.rs new file mode 100644 index 00000000..3eb52903 --- /dev/null +++ b/codex-rs/core/src/rollout/policy.rs @@ -0,0 +1,16 @@ +use codex_protocol::models::ResponseItem; + +/// Whether a `ResponseItem` should be persisted in rollout files. +#[inline] +pub(crate) fn is_persisted_response_item(item: &ResponseItem) -> bool { + match item { + ResponseItem::Message { .. } + | ResponseItem::Reasoning { .. } + | ResponseItem::LocalShellCall { .. } + | ResponseItem::FunctionCall { .. } + | ResponseItem::FunctionCallOutput { .. } + | ResponseItem::CustomToolCall { .. } + | ResponseItem::CustomToolCallOutput { .. } => true, + ResponseItem::WebSearchCall { .. } | ResponseItem::Other => false, + } +} diff --git a/codex-rs/core/src/rollout/recorder.rs b/codex-rs/core/src/rollout/recorder.rs index 20d30df6..68bcb439 100644 --- a/codex-rs/core/src/rollout/recorder.rs +++ b/codex-rs/core/src/rollout/recorder.rs @@ -23,6 +23,7 @@ use super::SESSIONS_SUBDIR; use super::list::ConversationsPage; use super::list::Cursor; use super::list::get_conversations; +use super::policy::is_persisted_response_item; use crate::config::Config; use crate::conversation_manager::InitialHistory; use crate::git_info::GitInfo; @@ -137,21 +138,11 @@ impl RolloutRecorder { pub(crate) async fn record_items(&self, items: &[ResponseItem]) -> std::io::Result<()> { let mut filtered = Vec::new(); for item in items { - match item { - // Note that function calls may look a bit strange if they are - // "fully qualified MCP tool calls," so we could consider - // reformatting them in that case. - ResponseItem::Message { .. } - | ResponseItem::LocalShellCall { .. } - | ResponseItem::FunctionCall { .. } - | ResponseItem::FunctionCallOutput { .. } - | ResponseItem::CustomToolCall { .. } - | ResponseItem::CustomToolCallOutput { .. } - | ResponseItem::Reasoning { .. } => filtered.push(item.clone()), - ResponseItem::WebSearchCall { .. } | ResponseItem::Other => { - // These should never be serialized. - continue; - } + // Note that function calls may look a bit strange if they are + // "fully qualified MCP tool calls," so we could consider + // reformatting them in that case. + if is_persisted_response_item(item) { + filtered.push(item.clone()); } } if filtered.is_empty() { @@ -195,16 +186,11 @@ impl RolloutRecorder { continue; } match serde_json::from_value::(v.clone()) { - Ok(item) => match item { - ResponseItem::Message { .. } - | ResponseItem::LocalShellCall { .. } - | ResponseItem::FunctionCall { .. } - | ResponseItem::FunctionCallOutput { .. } - | ResponseItem::CustomToolCall { .. } - | ResponseItem::CustomToolCallOutput { .. } - | ResponseItem::Reasoning { .. } => items.push(item), - ResponseItem::WebSearchCall { .. } | ResponseItem::Other => {} - }, + Ok(item) => { + if is_persisted_response_item(&item) { + items.push(item); + } + } Err(e) => { warn!("failed to parse item: {v:?}, error: {e}"); } @@ -305,17 +291,8 @@ async fn rollout_writer( match cmd { RolloutCmd::AddItems(items) => { for item in items { - match item { - ResponseItem::Message { .. } - | ResponseItem::LocalShellCall { .. } - | ResponseItem::FunctionCall { .. } - | ResponseItem::FunctionCallOutput { .. } - | ResponseItem::CustomToolCall { .. } - | ResponseItem::CustomToolCallOutput { .. } - | ResponseItem::Reasoning { .. } => { - writer.write_line(&item).await?; - } - ResponseItem::WebSearchCall { .. } | ResponseItem::Other => {} + if is_persisted_response_item(&item) { + writer.write_line(&item).await?; } } }