mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
Tag reports with the selected turn's model and effort (#35802)
## What changed - Read the selected turn's `model` and reasoning `effort` from its rollout and add them to the report's upload tags. - Use the latest turn context when the request has no `turn_id`, without substituting another turn when a requested ID is missing. - Prefer the request-derived model and effort over values captured in the report snapshot. ## Testing - Added coverage for selecting a reported turn, falling back to the latest turn, handling a missing turn, preserving an unspecified effort, and upload-tag precedence. GitOrigin-RevId: 133bab7730e18b28c4b26ae55fbce55d8e5705fb
This commit is contained in:
@@ -6,6 +6,7 @@ use codex_feedback::CODEX_APP_DIRECTORY_CACHE_ATTACHMENT_FILENAME;
|
||||
use codex_feedback::CODEX_APPS_TOOLS_CACHE_ATTACHMENT_FILENAME;
|
||||
#[cfg(target_os = "windows")]
|
||||
use codex_feedback::WINDOWS_SANDBOX_LOG_ATTACHMENT_FILENAME;
|
||||
use codex_rollout::RolloutRecorder;
|
||||
|
||||
const MAX_FEEDBACK_TREE_THREADS: usize = 8;
|
||||
|
||||
@@ -75,6 +76,20 @@ impl FeedbackRequestProcessor {
|
||||
None => None,
|
||||
};
|
||||
|
||||
if let Some(conversation_id) = conversation_id
|
||||
&& let Some(rollout_path) = self
|
||||
.resolve_rollout_path(conversation_id, self.state_db.as_ref())
|
||||
.await
|
||||
&& let Some((model, reasoning_effort)) = feedback_model_and_effort_from_rollout(
|
||||
&rollout_path,
|
||||
upload_tags.get("turn_id").map(String::as_str),
|
||||
)
|
||||
.await
|
||||
{
|
||||
upload_tags.insert("model".to_string(), model);
|
||||
upload_tags.insert("effort".to_string(), format!("{reasoning_effort:?}"));
|
||||
}
|
||||
|
||||
let auth = self.auth_manager.auth_cached();
|
||||
if let Some(chatgpt_user_id) = auth
|
||||
.as_ref()
|
||||
@@ -285,6 +300,24 @@ impl FeedbackRequestProcessor {
|
||||
}
|
||||
}
|
||||
|
||||
async fn feedback_model_and_effort_from_rollout(
|
||||
rollout_path: &Path,
|
||||
turn_id: Option<&str>,
|
||||
) -> Option<(String, Option<ReasoningEffort>)> {
|
||||
let (items, _, _) = RolloutRecorder::load_rollout_items(rollout_path)
|
||||
.await
|
||||
.ok()?;
|
||||
|
||||
items.into_iter().rev().find_map(|item| match item {
|
||||
RolloutItem::TurnContext(context)
|
||||
if turn_id.is_none() || context.turn_id.as_deref() == turn_id =>
|
||||
{
|
||||
Some((context.model, context.effort))
|
||||
}
|
||||
_ => None,
|
||||
})
|
||||
}
|
||||
|
||||
fn tool_cache_feedback_attachments(
|
||||
codex_home: &Path,
|
||||
chatgpt_base_url: &str,
|
||||
@@ -349,8 +382,114 @@ fn windows_sandbox_log_attachment(_codex_home: &Path) -> Option<FeedbackAttachme
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use codex_protocol::protocol::RolloutLine;
|
||||
use codex_protocol::protocol::TurnContextItem;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[tokio::test]
|
||||
async fn feedback_model_and_effort_use_the_reported_turn() {
|
||||
let (_tempdir, rollout_path) = feedback_rollout(&[
|
||||
("turn-1", "reported-model", Some(ReasoningEffort::High)),
|
||||
("turn-2", "newer-model", Some(ReasoningEffort::Ultra)),
|
||||
]);
|
||||
|
||||
assert_eq!(
|
||||
feedback_model_and_effort_from_rollout(&rollout_path, Some("turn-1")).await,
|
||||
Some(("reported-model".to_string(), Some(ReasoningEffort::High)))
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn feedback_model_and_effort_use_the_latest_turn_when_no_turn_is_reported() {
|
||||
let (_tempdir, rollout_path) = feedback_rollout(&[
|
||||
("turn-1", "older-model", Some(ReasoningEffort::High)),
|
||||
("turn-2", "latest-model", Some(ReasoningEffort::Ultra)),
|
||||
]);
|
||||
|
||||
assert_eq!(
|
||||
feedback_model_and_effort_from_rollout(&rollout_path, /*turn_id*/ None).await,
|
||||
Some(("latest-model".to_string(), Some(ReasoningEffort::Ultra)))
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn feedback_model_and_effort_do_not_substitute_a_different_turn() {
|
||||
let (_tempdir, rollout_path) =
|
||||
feedback_rollout(&[("turn-1", "different-model", Some(ReasoningEffort::High))]);
|
||||
|
||||
assert_eq!(
|
||||
feedback_model_and_effort_from_rollout(&rollout_path, Some("missing-turn")).await,
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn feedback_model_and_effort_preserve_unspecified_effort() {
|
||||
let (_tempdir, rollout_path) =
|
||||
feedback_rollout(&[("turn-1", "reported-model", /*effort*/ None)]);
|
||||
|
||||
assert_eq!(
|
||||
feedback_model_and_effort_from_rollout(&rollout_path, Some("turn-1")).await,
|
||||
Some(("reported-model".to_string(), None))
|
||||
);
|
||||
}
|
||||
|
||||
fn feedback_rollout(
|
||||
turns: &[(&str, &str, Option<ReasoningEffort>)],
|
||||
) -> (tempfile::TempDir, PathBuf) {
|
||||
let tempdir = tempfile::tempdir().expect("create feedback rollout directory");
|
||||
let rollout_path = tempdir.path().join("feedback-rollout.jsonl");
|
||||
let mut lines = vec![RolloutLine {
|
||||
timestamp: "2026-07-24T00:00:00Z".to_string(),
|
||||
ordinal: None,
|
||||
item: RolloutItem::SessionMeta(SessionMetaLine {
|
||||
meta: codex_protocol::protocol::SessionMeta {
|
||||
cwd: tempdir.path().to_path_buf(),
|
||||
..Default::default()
|
||||
},
|
||||
git: None,
|
||||
}),
|
||||
}];
|
||||
lines.extend(turns.iter().map(|(turn_id, model, effort)| {
|
||||
RolloutLine {
|
||||
timestamp: "2026-07-24T00:00:01Z".to_string(),
|
||||
ordinal: None,
|
||||
item: RolloutItem::TurnContext(TurnContextItem {
|
||||
turn_id: Some((*turn_id).to_string()),
|
||||
cwd: AbsolutePathBuf::from_absolute_path(tempdir.path())
|
||||
.expect("absolute feedback rollout directory"),
|
||||
workspace_roots: None,
|
||||
current_date: None,
|
||||
timezone: None,
|
||||
approval_policy: codex_protocol::protocol::AskForApproval::Never,
|
||||
approvals_reviewer: None,
|
||||
sandbox_policy: codex_protocol::protocol::SandboxPolicy::new_read_only_policy(),
|
||||
permission_profile: None,
|
||||
network: None,
|
||||
file_system_sandbox_policy: None,
|
||||
model: (*model).to_string(),
|
||||
comp_hash: None,
|
||||
personality: None,
|
||||
collaboration_mode: None,
|
||||
multi_agent_version: None,
|
||||
multi_agent_mode: None,
|
||||
realtime_active: None,
|
||||
effort: effort.clone(),
|
||||
summary: ReasoningSummary::Auto,
|
||||
}),
|
||||
}
|
||||
}));
|
||||
let contents = lines
|
||||
.iter()
|
||||
.map(serde_json::to_string)
|
||||
.collect::<Result<Vec<_>, _>>()
|
||||
.expect("serialize feedback rollout")
|
||||
.join("\n");
|
||||
std::fs::write(&rollout_path, format!("{contents}\n")).expect("write feedback rollout");
|
||||
|
||||
(tempdir, rollout_path)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_cache_feedback_attachments_include_existing_active_cache_files() {
|
||||
let codex_home = tempfile::tempdir().expect("create tempdir");
|
||||
|
||||
@@ -894,6 +894,7 @@ mod tests {
|
||||
tags.insert("reason".to_string(), "wrong-reason".to_string());
|
||||
tags.insert("account_id".to_string(), "actual-account".to_string());
|
||||
tags.insert("model".to_string(), "gpt-5".to_string());
|
||||
tags.insert("effort".to_string(), "Some(High)".to_string());
|
||||
let snapshot = FeedbackSnapshot {
|
||||
bytes: Vec::new(),
|
||||
tags,
|
||||
@@ -917,6 +918,8 @@ mod tests {
|
||||
);
|
||||
client_tags.insert("reason".to_string(), "wrong-client-reason".to_string());
|
||||
client_tags.insert("client_tag".to_string(), "from-client".to_string());
|
||||
client_tags.insert("model".to_string(), "mewthree".to_string());
|
||||
client_tags.insert("effort".to_string(), "Some(Ultra)".to_string());
|
||||
|
||||
let upload_tags = snapshot.upload_tags(
|
||||
"bug",
|
||||
@@ -949,10 +952,17 @@ mod tests {
|
||||
upload_tags.get("account_id").map(String::as_str),
|
||||
Some("actual-account")
|
||||
);
|
||||
assert_eq!(
|
||||
upload_tags.get("model").map(String::as_str),
|
||||
Some("mewthree")
|
||||
);
|
||||
assert_eq!(
|
||||
upload_tags.get("effort").map(String::as_str),
|
||||
Some("Some(Ultra)")
|
||||
);
|
||||
assert_eq!(
|
||||
upload_tags.get("client_tag").map(String::as_str),
|
||||
Some("from-client")
|
||||
);
|
||||
assert_eq!(upload_tags.get("model").map(String::as_str), Some("gpt-5"));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user