-
Notifications
You must be signed in to change notification settings - Fork 5k
Introduce rollout items #3380
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce rollout items #3380
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codex Review: Here are some suggestions.
Reply with @codex fix comments
to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
pub(crate) async fn get_rollout_history(path: &Path) -> std::io::Result<InitialHistory> { | ||
info!("Resuming rollout from {path:?}"); | ||
tracing::error!("Resuming rollout from {path:?}"); | ||
let text = tokio::fs::read_to_string(path).await?; | ||
let mut lines = text.lines(); | ||
let first_line = lines | ||
.next() | ||
.ok_or_else(|| IoError::other("empty session file"))?; | ||
let conversation_id = match serde_json::from_str::<SessionMeta>(first_line) { | ||
Ok(rollout_session_meta) => { | ||
tracing::error!( | ||
"Parsed conversation ID from rollout file: {:?}", | ||
rollout_session_meta.id | ||
); | ||
Some(rollout_session_meta.id) | ||
} | ||
Err(e) => { | ||
return Err(IoError::other(format!( | ||
"failed to parse first line of rollout file as SessionMeta: {e}" | ||
))); | ||
} | ||
}; | ||
if text.trim().is_empty() { | ||
return Err(IoError::other("empty session file")); | ||
} | ||
|
||
let mut items = Vec::new(); | ||
for line in lines { | ||
let mut items: Vec<RolloutItem> = Vec::new(); | ||
let mut conversation_id: Option<ConversationId> = None; | ||
for line in text.lines() { | ||
if line.trim().is_empty() { | ||
continue; | ||
} | ||
let v: Value = match serde_json::from_str(line) { | ||
Ok(v) => v, | ||
Err(_) => continue, | ||
Err(e) => { | ||
warn!("failed to parse line as JSON: {line:?}, error: {e}"); | ||
continue; | ||
} | ||
}; | ||
if v.get("record_type") | ||
.and_then(|rt| rt.as_str()) | ||
.map(|s| s == "state") | ||
.unwrap_or(false) | ||
{ | ||
continue; | ||
} | ||
match serde_json::from_value::<ResponseItem>(v.clone()) { | ||
Ok(item) => { | ||
if is_persisted_response_item(&item) { | ||
items.push(item); | ||
|
||
// Parse the rollout line structure | ||
match serde_json::from_value::<RolloutLine>(v.clone()) { | ||
Ok(rollout_line) => match rollout_line.item { | ||
RolloutItem::SessionMeta(session_meta_line) => { | ||
tracing::error!( | ||
"Parsed conversation ID from rollout file: {:?}", | ||
session_meta_line.meta.id | ||
); | ||
conversation_id = Some(session_meta_line.meta.id); | ||
items.push(RolloutItem::SessionMeta(session_meta_line)); | ||
} | ||
} | ||
RolloutItem::ResponseItem(item) => { | ||
items.push(RolloutItem::ResponseItem(item)); | ||
} | ||
RolloutItem::EventMsg(_ev) => { | ||
items.push(RolloutItem::EventMsg(_ev)); | ||
} | ||
}, | ||
Err(e) => { | ||
warn!("failed to parse item: {v:?}, error: {e}"); | ||
warn!("failed to parse rollout line: {v:?}, error: {e}"); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] Handle legacy rollout files without new schema envelope
The new parsing path assumes every persisted line is a RolloutLine
containing type
/payload
(lines 230‑272). Rollout files written by previous releases contain bare SessionMeta
/ResponseItem
JSON and cannot deserialize into that enum, leaving conversation_id
unset and causing failed to parse conversation ID
for any existing session. The listing path now makes the same assumption, so those files are filtered out as well. After upgrading, users with pre‑existing conversations can no longer list or resume them. A fallback that understands the old format is needed when RolloutLine
parsing fails.
Useful? React with 👍 / 👎.
codex-rs/core/src/rollout/list.rs
Outdated
RolloutItem::EventMsg(ev) => { | ||
// Capture whether an early agent/user message event exists within the head window | ||
use EventMsg::*; | ||
if let UserMessage(_) = ev { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EventMsg::UserMessage?
This PR introduces Rollout items. This enable us to rollout eventmsgs and session meta.
This is mostly #3214 with rebase on main