Introduce Rollout Policy (#3116)

Have a helper function for deciding if we are rolling out a function or
not
This commit is contained in:
Ahmed Ibrahim
2025-09-03 10:37:07 -07:00
committed by GitHub
parent c636f821ae
commit daaadfb260
3 changed files with 30 additions and 36 deletions

View File

@@ -3,6 +3,7 @@
pub(crate) const SESSIONS_SUBDIR: &str = "sessions"; pub(crate) const SESSIONS_SUBDIR: &str = "sessions";
pub mod list; pub mod list;
pub(crate) mod policy;
pub mod recorder; pub mod recorder;
pub use recorder::RolloutRecorder; pub use recorder::RolloutRecorder;

View File

@@ -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,
}
}

View File

@@ -23,6 +23,7 @@ use super::SESSIONS_SUBDIR;
use super::list::ConversationsPage; use super::list::ConversationsPage;
use super::list::Cursor; use super::list::Cursor;
use super::list::get_conversations; use super::list::get_conversations;
use super::policy::is_persisted_response_item;
use crate::config::Config; use crate::config::Config;
use crate::conversation_manager::InitialHistory; use crate::conversation_manager::InitialHistory;
use crate::git_info::GitInfo; use crate::git_info::GitInfo;
@@ -137,21 +138,11 @@ impl RolloutRecorder {
pub(crate) async fn record_items(&self, items: &[ResponseItem]) -> std::io::Result<()> { pub(crate) async fn record_items(&self, items: &[ResponseItem]) -> std::io::Result<()> {
let mut filtered = Vec::new(); let mut filtered = Vec::new();
for item in items { for item in items {
match item { // Note that function calls may look a bit strange if they are
// Note that function calls may look a bit strange if they are // "fully qualified MCP tool calls," so we could consider
// "fully qualified MCP tool calls," so we could consider // reformatting them in that case.
// reformatting them in that case. if is_persisted_response_item(item) {
ResponseItem::Message { .. } filtered.push(item.clone());
| 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;
}
} }
} }
if filtered.is_empty() { if filtered.is_empty() {
@@ -195,16 +186,11 @@ impl RolloutRecorder {
continue; continue;
} }
match serde_json::from_value::<ResponseItem>(v.clone()) { match serde_json::from_value::<ResponseItem>(v.clone()) {
Ok(item) => match item { Ok(item) => {
ResponseItem::Message { .. } if is_persisted_response_item(&item) {
| ResponseItem::LocalShellCall { .. } items.push(item);
| ResponseItem::FunctionCall { .. } }
| ResponseItem::FunctionCallOutput { .. } }
| ResponseItem::CustomToolCall { .. }
| ResponseItem::CustomToolCallOutput { .. }
| ResponseItem::Reasoning { .. } => items.push(item),
ResponseItem::WebSearchCall { .. } | ResponseItem::Other => {}
},
Err(e) => { Err(e) => {
warn!("failed to parse item: {v:?}, error: {e}"); warn!("failed to parse item: {v:?}, error: {e}");
} }
@@ -305,17 +291,8 @@ async fn rollout_writer(
match cmd { match cmd {
RolloutCmd::AddItems(items) => { RolloutCmd::AddItems(items) => {
for item in items { for item in items {
match item { if is_persisted_response_item(&item) {
ResponseItem::Message { .. } writer.write_line(&item).await?;
| ResponseItem::LocalShellCall { .. }
| ResponseItem::FunctionCall { .. }
| ResponseItem::FunctionCallOutput { .. }
| ResponseItem::CustomToolCall { .. }
| ResponseItem::CustomToolCallOutput { .. }
| ResponseItem::Reasoning { .. } => {
writer.write_line(&item).await?;
}
ResponseItem::WebSearchCall { .. } | ResponseItem::Other => {}
} }
} }
} }