diff --git a/codex-rs/docs/eval_capture.md b/codex-rs/docs/eval_capture.md index 05728f552d..0a9a9d46d4 100644 --- a/codex-rs/docs/eval_capture.md +++ b/codex-rs/docs/eval_capture.md @@ -13,7 +13,7 @@ Bundles are stored under: Each bundle contains: -- `manifest.json` - metadata about the capture (schema version, start marker, notes, repo base). +- `manifest.json` - metadata about the capture (schema version, rollout start selector, repo git pointer). - `rollout.jsonl` - the full session rollout (multi-turn trajectory). - `repo.patch` - a git patch representing the repository state at the chosen start marker. - `codex-logs.log` - tracing logs to help maintainers debug the session. @@ -23,6 +23,9 @@ Each bundle contains: Bundles include the entire rollout, but also record a start marker to indicate where an eval harness (or a human) should begin replaying/interpreting the trajectory. +`manifest.json` also records a `rollout.start_selector`, which is the preferred way to find the +intended starting turn when replaying/slicing (line numbers in `rollout.jsonl` are not stable). + The repository patch must match that chosen start marker: - If the session has repo snapshots available, `repo.patch` is derived from the ghost snapshot @@ -34,6 +37,17 @@ The repository patch must match that chosen start marker: For reproducibility outside your machine, the base commit recorded in `manifest.json` should be reachable by maintainers (for example, pushed and available on the default branch). +## Reproducibility Contract + +Reproducing the repository state in a bundle must be possible using only: + +- `repo.git.canonical_remote` (clone URL) +- `repo.git.commit` (base commit to `git checkout`) +- `repo.patch` (apply if non-empty) + +The manifest may also include machine-local paths (like `repo.root` / `repo.cwd_rel`) as debugging +hints, but repro should not depend on them. + ## App-Server API (For Integrations) Non-TUI clients can create bundles via the app-server JSON-RPC method: diff --git a/codex-rs/eval-case/Cargo.toml b/codex-rs/eval-case/Cargo.toml index 5c5f2f42d2..19542c9c81 100644 --- a/codex-rs/eval-case/Cargo.toml +++ b/codex-rs/eval-case/Cargo.toml @@ -11,10 +11,9 @@ workspace = true anyhow = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } -time = { workspace = true, features = ["formatting"] } +time = { workspace = true, features = ["formatting", "parsing"] } uuid = { workspace = true, features = ["v4"] } [dev-dependencies] pretty_assertions = { workspace = true } tempfile = { workspace = true } - diff --git a/codex-rs/eval-case/src/lib.rs b/codex-rs/eval-case/src/lib.rs index 6ed9f41012..17976e9a55 100644 --- a/codex-rs/eval-case/src/lib.rs +++ b/codex-rs/eval-case/src/lib.rs @@ -31,21 +31,63 @@ pub struct StartMarker { } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] -pub struct GitBase { - pub sha: String, - pub note: String, +#[serde(rename_all = "snake_case")] +pub enum RolloutStartSelectorKind { + /// Find the first `event_msg` line where `payload.type == "user_message"` and + /// `payload.message` contains `contains`. + EventMsgUserMessageContains, + /// Find the first `response_item` line where `payload.role == "user"` and the combined text + /// from `payload.content[*].text` contains `contains`. + ResponseItemUserMessageContains, } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct RolloutInfo { pub filename: String, + /// Preferred, deterministic selector for slicing the rollout when reproducing a case. + pub start_selector: RolloutStartSelector, + /// Debugging hint only; do not use this for correctness when slicing. pub start: StartMarker, } +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct RolloutStartSelector { + pub kind: RolloutStartSelectorKind, + pub contains: String, + #[serde(skip_serializing_if = "Option::is_none")] + pub after_timestamp: Option, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct GitRemote { + pub name: String, + pub fetch_url: String, + pub push_url: String, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct RepoGitInfo { + /// Base commit to `git checkout` before applying `repo.patch`. + pub commit: String, + pub remotes: Vec, + pub canonical_remote: Option, + pub reproducible: bool, + #[serde(skip_serializing_if = "Option::is_none")] + pub reproducible_reason: Option, + pub is_dirty: bool, + #[serde(skip_serializing_if = "Option::is_none")] + pub describe: Option, +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct RepoInfo { - pub cwd: String, - pub git_base: GitBase, + /// Repository root as reported by `git rev-parse --show-toplevel` (optional). + #[serde(skip_serializing_if = "Option::is_none")] + pub root: Option, + /// Relative path from `repo.root` to capture-time cwd (optional). + #[serde(skip_serializing_if = "Option::is_none")] + pub cwd_rel: Option, + pub git: RepoGitInfo, pub patch_filename: String, } @@ -61,7 +103,7 @@ pub struct Artifacts { } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] -pub struct EvalCaseManifestV0 { +pub struct EvalCaseManifestV1 { pub version: String, pub case_id: String, pub created_at: String, @@ -172,6 +214,13 @@ pub fn create_eval_case_bundle(args: &CreateEvalCaseArgs) -> anyhow::Result { git_patch_between_commits(&args.repo_cwd, &snapshot.base_sha, &snapshot.commit_sha) @@ -198,22 +247,73 @@ pub fn create_eval_case_bundle(args: &CreateEvalCaseArgs) -> anyhow::Result (remotes, true), + Err(_) => (Vec::new(), false), + }; + let canonical_remote = select_canonical_remote(&git_remotes); + let git_is_dirty = git_is_dirty(&args.repo_cwd).unwrap_or(false); + let git_describe = git_stdout( + &args.repo_cwd, + &["describe", "--tags", "--always", "--dirty"], + ) + .ok() + .filter(|s| !s.is_empty()); + + let mut reproducible = true; + let mut reproducible_reason: Option = None; + if !remotes_ok { + reproducible = false; + reproducible_reason = Some("not_a_git_repo".to_string()); + } else if canonical_remote.is_none() { + reproducible = false; + reproducible_reason = Some("no_git_remote".to_string()); + } + if base_sha.len() != 40 || base_sha == "unknown" { + reproducible = false; + if reproducible_reason.is_none() { + reproducible_reason = Some("unknown_commit".to_string()); + } + } + + let start_selector = derive_rollout_start_selector(&args.start, &rollout_text); + + let manifest = EvalCaseManifestV1 { + version: "v1".to_string(), case_id: case_id.clone(), created_at: created_at_rfc3339, conversation_id: args.conversation_id.clone(), source: "cli".to_string(), rollout: RolloutInfo { filename: "rollout.jsonl".to_string(), + start_selector, start: args.start.clone(), }, repo: RepoInfo { - cwd: args.repo_cwd.display().to_string(), - git_base: GitBase { - sha: base_sha, - note: "For reproducibility, the base commit should be reachable (e.g. pushed / on main)." - .to_string(), + root: repo_root, + cwd_rel, + git: RepoGitInfo { + commit: base_sha, + remotes: git_remotes, + canonical_remote, + reproducible, + reproducible_reason, + is_dirty: git_is_dirty, + describe: git_describe, }, patch_filename: "repo.patch".to_string(), }, @@ -233,6 +333,253 @@ pub fn create_eval_case_bundle(args: &CreateEvalCaseArgs) -> anyhow::Result String { + let mut out = String::with_capacity(s.len()); + for part in s.split_whitespace() { + if !out.is_empty() { + out.push(' '); + } + out.push_str(part); + } + out +} + +fn truncate_chars(s: &str, max_chars: usize) -> String { + if s.chars().count() <= max_chars { + return s.to_string(); + } + s.chars().take(max_chars).collect() +} + +fn should_show_start_message(message: &str) -> bool { + // The rollout can include synthetic "user" messages (environment context, AGENTS + // instructions) that should not be used as start selectors. + let trimmed = message.trim_start(); + if trimmed.starts_with("") { + return false; + } + if trimmed.starts_with("# AGENTS.md instructions") { + return false; + } + true +} + +fn parse_rfc3339(s: &str) -> Option { + OffsetDateTime::parse(s, &Rfc3339).ok() +} + +fn is_after_timestamp(timestamp: &str, after_timestamp: &str) -> bool { + match (parse_rfc3339(timestamp), parse_rfc3339(after_timestamp)) { + (Some(ts), Some(after)) => ts >= after, + // Fall back to lexicographic compare; RFC3339 timestamps with consistent formatting + // should still compare correctly. + _ => timestamp >= after_timestamp, + } +} + +fn derive_rollout_start_selector(start: &StartMarker, rollout_text: &str) -> RolloutStartSelector { + let after_timestamp = match &start.value { + StartMarkerValue::Timestamp(ts) => Some(ts.clone()), + StartMarkerValue::LineIndex(idx) => { + let idx = usize::try_from(*idx).ok(); + idx.and_then(|i| rollout_text.lines().nth(i)) + .and_then(|line| { + serde_json::from_str::(line) + .ok() + .and_then(|v| { + v.get("timestamp") + .and_then(serde_json::Value::as_str) + .map(str::to_string) + }) + }) + } + }; + + let (kind, message) = if let Some(message) = + find_first_event_msg_user_message(rollout_text, after_timestamp.as_deref()) + { + ( + RolloutStartSelectorKind::EventMsgUserMessageContains, + Some(message), + ) + } else if let Some(message) = + find_first_response_item_user_message(rollout_text, after_timestamp.as_deref()) + { + ( + RolloutStartSelectorKind::ResponseItemUserMessageContains, + Some(message), + ) + } else { + (RolloutStartSelectorKind::EventMsgUserMessageContains, None) + }; + + let contains = message + .map(|m| truncate_chars(&normalize_whitespace(&m), 200)) + .unwrap_or_default(); + + RolloutStartSelector { + kind, + contains, + after_timestamp, + } +} + +fn find_first_event_msg_user_message( + rollout_text: &str, + after_timestamp: Option<&str>, +) -> Option { + rollout_text.lines().find_map(|line| { + let v = serde_json::from_str::(line).ok()?; + let timestamp = v.get("timestamp")?.as_str()?; + if let Some(after) = after_timestamp + && !is_after_timestamp(timestamp, after) + { + return None; + } + + let ty = v.get("type")?.as_str()?; + if ty != "event_msg" { + return None; + } + let payload = v.get("payload")?; + let payload_ty = payload.get("type")?.as_str()?; + if payload_ty != "user_message" { + return None; + } + let message = payload.get("message")?.as_str()?; + should_show_start_message(message).then(|| message.to_string()) + }) +} + +fn find_first_response_item_user_message( + rollout_text: &str, + after_timestamp: Option<&str>, +) -> Option { + rollout_text.lines().find_map(|line| { + let v = serde_json::from_str::(line).ok()?; + let timestamp = v.get("timestamp")?.as_str()?; + if let Some(after) = after_timestamp + && !is_after_timestamp(timestamp, after) + { + return None; + } + + let ty = v.get("type")?.as_str()?; + if ty != "response_item" { + return None; + } + let payload = v.get("payload")?; + let role = payload.get("role")?.as_str()?; + if role != "user" { + return None; + } + let content = payload.get("content")?.as_array()?; + let mut out = String::new(); + for item in content { + let Some(text) = item.get("text").and_then(serde_json::Value::as_str) else { + continue; + }; + if !out.is_empty() { + out.push(' '); + } + out.push_str(text); + } + let out = normalize_whitespace(out.as_str()); + (!out.is_empty() && should_show_start_message(&out)).then_some(out) + }) +} + +pub fn find_rollout_start_index( + rollout_text: &str, + selector: &RolloutStartSelector, +) -> Option { + if selector.contains.trim().is_empty() { + return None; + } + + let lines = rollout_text.lines().collect::>(); + let matched_idx = lines.iter().enumerate().find_map(|(idx, line)| { + let v = serde_json::from_str::(line).ok()?; + let timestamp = v.get("timestamp")?.as_str()?; + if let Some(after) = selector.after_timestamp.as_deref() + && !is_after_timestamp(timestamp, after) + { + return None; + } + + match selector.kind { + RolloutStartSelectorKind::EventMsgUserMessageContains => { + let ty = v.get("type")?.as_str()?; + if ty != "event_msg" { + return None; + } + let payload = v.get("payload")?; + let payload_ty = payload.get("type")?.as_str()?; + if payload_ty != "user_message" { + return None; + } + let message = payload.get("message")?.as_str()?; + message.contains(selector.contains.as_str()).then_some(idx) + } + RolloutStartSelectorKind::ResponseItemUserMessageContains => { + let ty = v.get("type")?.as_str()?; + if ty != "response_item" { + return None; + } + let payload = v.get("payload")?; + let role = payload.get("role")?.as_str()?; + if role != "user" { + return None; + } + let content = payload.get("content")?.as_array()?; + let mut out = String::new(); + for item in content { + let Some(text) = item.get("text").and_then(serde_json::Value::as_str) else { + continue; + }; + if !out.is_empty() { + out.push(' '); + } + out.push_str(text); + } + let out = normalize_whitespace(out.as_str()); + out.contains(selector.contains.as_str()).then_some(idx) + } + } + })?; + + // Include immediately preceding turn context lines so replay has the right environment/config. + let mut start_idx = matched_idx; + while start_idx > 0 { + let prev_line = lines.get(start_idx.saturating_sub(1))?; + let Ok(v) = serde_json::from_str::(prev_line) else { + break; + }; + let Some(ty) = v.get("type").and_then(serde_json::Value::as_str) else { + break; + }; + if ty != "turn_context" { + break; + } + start_idx = start_idx.saturating_sub(1); + } + + Some(start_idx) +} + +pub fn slice_rollout_from_selector( + rollout_text: &str, + selector: &RolloutStartSelector, +) -> Option { + let start = find_rollout_start_index(rollout_text, selector)?; + let sliced = rollout_text + .lines() + .skip(start) + .collect::>() + .join("\n"); + Some(format!("{sliced}\n")) +} + fn git_patch_against_head(repo_cwd: &Path) -> anyhow::Result<(String, Vec)> { let base_sha = git_stdout(repo_cwd, &["rev-parse", "HEAD"]).unwrap_or_else(|_| "unknown".to_string()); @@ -267,6 +614,88 @@ fn git_patch_against_head(repo_cwd: &Path) -> anyhow::Result<(String, Vec)> Ok((base_sha, patch)) } +fn git_remotes(repo_cwd: &Path) -> anyhow::Result> { + let output = git_stdout(repo_cwd, &["remote", "-v"])?; + // Use a BTreeMap so iteration is deterministic. + let mut by_name: std::collections::BTreeMap, Vec)> = + std::collections::BTreeMap::new(); + + for line in output.lines().map(str::trim).filter(|s| !s.is_empty()) { + let mut parts = line.split_whitespace(); + let Some(name) = parts.next() else { + continue; + }; + let Some(url) = parts.next() else { + continue; + }; + let Some(kind) = parts.next() else { + continue; + }; + + let entry = by_name + .entry(name.to_string()) + .or_insert_with(|| (Vec::new(), Vec::new())); + match kind { + "(fetch)" => entry.0.push(url.to_string()), + "(push)" => entry.1.push(url.to_string()), + _ => {} + } + } + + let remotes = by_name + .into_iter() + .filter_map(|(name, (mut fetch_urls, mut push_urls))| { + fetch_urls.sort(); + push_urls.sort(); + let fetch_url = fetch_urls.into_iter().next()?; + let push_url = push_urls + .into_iter() + .next() + .unwrap_or_else(|| fetch_url.clone()); + Some(GitRemote { + name, + fetch_url, + push_url, + }) + }) + .collect::>(); + Ok(remotes) +} + +fn select_canonical_remote(remotes: &[GitRemote]) -> Option { + if let Some(origin) = remotes.iter().find(|r| r.name == "origin") { + return Some(origin.fetch_url.clone()); + } + remotes + .iter() + .min_by(|a, b| a.name.cmp(&b.name)) + .map(|r| r.fetch_url.clone()) +} + +fn git_is_dirty(repo_cwd: &Path) -> anyhow::Result { + let diff_unstaged = Command::new("git") + .args(["diff", "--quiet"]) + .current_dir(repo_cwd) + .status() + .context("run git diff --quiet")?; + if !diff_unstaged.success() { + return Ok(true); + } + + let diff_staged = Command::new("git") + .args(["diff", "--cached", "--quiet"]) + .current_dir(repo_cwd) + .status() + .context("run git diff --cached --quiet")?; + if !diff_staged.success() { + return Ok(true); + } + + let untracked = + git_stdout(repo_cwd, &["ls-files", "--others", "--exclude-standard"]).unwrap_or_default(); + Ok(untracked.lines().any(|s| !s.trim().is_empty())) +} + fn git_stdout(repo_cwd: &Path, args: &[&str]) -> anyhow::Result { let output = Command::new("git") .args(args) @@ -357,7 +786,13 @@ mod tests { std::fs::write(repo_root.join("README.md"), "changed\n").unwrap(); let rollout_path = repo_root.join("rollout.jsonl"); - std::fs::write(&rollout_path, "line-1\nline-2\n").unwrap(); + let rollout = [ + r#"{"timestamp":"2024-01-01T00:00:00Z","type":"token_count","payload":{"total":1}}"#, + r#"{"timestamp":"2024-01-01T00:00:00Z","type":"event_msg","payload":{"type":"user_message","message":"do the thing"}}"#, + r#"{"timestamp":"2024-01-01T00:00:01Z","type":"event_msg","payload":{"type":"agent_message","message":"ok"}}"#, + ] + .join("\n"); + std::fs::write(&rollout_path, format!("{rollout}\n")).unwrap(); let args = CreateEvalCaseArgs { codex_home: codex_home.path().to_path_buf(), @@ -368,7 +803,7 @@ mod tests { value: StartMarkerValue::LineIndex(1), display: "Start now".to_string(), }, - repo_cwd: repo_root.clone(), + repo_cwd: repo_root, repo_snapshot: None, notes: Notes { what_went_wrong: "bad".to_string(), @@ -386,12 +821,20 @@ mod tests { assert!(out.case_id.contains("my-repo")); let manifest_text = std::fs::read_to_string(out.path.join("manifest.json")).unwrap(); - let manifest: EvalCaseManifestV0 = serde_json::from_str(&manifest_text).unwrap(); - assert_eq!(manifest.version, "v0"); + let manifest: EvalCaseManifestV1 = serde_json::from_str(&manifest_text).unwrap(); + assert_eq!(manifest.version, "v1"); assert_eq!(manifest.conversation_id, "conv-1"); assert_eq!(manifest.notes, args.notes); - assert!(manifest.repo.git_base.sha != "unknown"); + assert!(manifest.repo.git.commit != "unknown"); + assert_eq!(manifest.repo.git.canonical_remote, None); + assert_eq!(manifest.repo.git.reproducible, false); + assert_eq!( + manifest.repo.git.reproducible_reason, + Some("no_git_remote".to_string()) + ); + assert_eq!(manifest.repo.git.is_dirty, true); assert_eq!(manifest.artifacts.include_logs, true); + assert!(!manifest.rollout.start_selector.contains.trim().is_empty()); assert!(out.path.join("repo.patch").exists()); assert!(out.path.join("rollout.jsonl").exists()); @@ -469,7 +912,12 @@ mod tests { std::fs::write(repo_root.join("README.md"), "worktree\n").unwrap(); let rollout_path = repo_root.join("rollout.jsonl"); - std::fs::write(&rollout_path, "line-1\nline-2\n").unwrap(); + let rollout = [ + r#"{"timestamp":"2024-01-01T00:00:00Z","type":"event_msg","payload":{"type":"user_message","message":"snapshot run"}}"#, + r#"{"timestamp":"2024-01-01T00:00:01Z","type":"event_msg","payload":{"type":"agent_message","message":"ok"}}"#, + ] + .join("\n"); + std::fs::write(&rollout_path, format!("{rollout}\n")).unwrap(); let args = CreateEvalCaseArgs { codex_home: codex_home.path().to_path_buf(), @@ -480,7 +928,7 @@ mod tests { value: StartMarkerValue::LineIndex(0), display: "From: test".to_string(), }, - repo_cwd: repo_root.clone(), + repo_cwd: repo_root, repo_snapshot: Some(RepoSnapshot { base_sha: base_sha.clone(), commit_sha: snapshot_sha, @@ -498,8 +946,8 @@ mod tests { assert!(out.path.starts_with(codex_home.path().join("eval-case"))); assert!(out.case_id.contains("my-repo")); let manifest_text = std::fs::read_to_string(out.path.join("manifest.json")).unwrap(); - let manifest: EvalCaseManifestV0 = serde_json::from_str(&manifest_text).unwrap(); - assert_eq!(manifest.repo.git_base.sha, base_sha); + let manifest: EvalCaseManifestV1 = serde_json::from_str(&manifest_text).unwrap(); + assert_eq!(manifest.repo.git.commit, base_sha); let patch_text = String::from_utf8(std::fs::read(out.path.join("repo.patch")).unwrap()).unwrap(); @@ -513,4 +961,33 @@ mod tests { ); assert!(patch_text.contains("snap.txt")); } + + #[test] + fn slices_rollout_using_start_selector_even_when_line_numbers_shift() { + let rollout = [ + r#"{"timestamp":"2024-01-01T00:00:00Z","type":"token_count","payload":{"total":1}}"#, + r#"{"timestamp":"2024-01-01T00:00:00Z","type":"turn_context","payload":{"cwd":"/tmp","approval_policy":"never","sandbox_policy":"none","model":"x","summary":"none"}}"#, + r#"{"timestamp":"2024-01-01T00:00:00Z","type":"event_msg","payload":{"type":"user_message","message":"run the tests please"}}"#, + r#"{"timestamp":"2024-01-01T00:00:01Z","type":"event_msg","payload":{"type":"agent_message","message":"ok"}}"#, + ] + .join("\n"); + + let selector = RolloutStartSelector { + kind: RolloutStartSelectorKind::EventMsgUserMessageContains, + contains: "run the tests".to_string(), + after_timestamp: Some("2024-01-01T00:00:00Z".to_string()), + }; + + let sliced = slice_rollout_from_selector(&rollout, &selector).unwrap(); + // Should include the turn_context that immediately precedes the matched user_message. + assert!( + sliced + .lines() + .next() + .unwrap() + .contains("\"type\":\"turn_context\"") + ); + assert!(sliced.contains("\"type\":\"user_message\"")); + assert!(!sliced.contains("\"type\":\"token_count\"")); + } }