mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
Track image generation request IDs in analytics (#40714)
## What changed - Read `x-codex-imagegen-request-id` from image generation and edit responses. - Propagate the ID to `codex_image_generation_event` analytics. - Keep the ID in process only, excluding it from extension item serialization, generated types, app-server wire data, and rollout history. ## Testing - Cover response-header extraction and missing-header behavior in the images client. - Verify end-to-end analytics emission and the unchanged image-generation item wire shape. GitOrigin-RevId: 9b8e09b7d655c0c2faa84f0eee99aabae583668d
This commit is contained in:
@@ -5121,6 +5121,7 @@ async fn image_generation_events_preserve_transparent_background_metadata() {
|
||||
transparent_background,
|
||||
failure: None,
|
||||
saved_path: None,
|
||||
imagegen_request_id: None,
|
||||
});
|
||||
|
||||
reducer
|
||||
@@ -5265,6 +5266,7 @@ async fn turn_event_counts_completed_tool_items() {
|
||||
transparent_background: None,
|
||||
failure: None,
|
||||
saved_path: None,
|
||||
imagegen_request_id: Some("req-imagegen-123".to_string()),
|
||||
}),
|
||||
];
|
||||
|
||||
@@ -5354,6 +5356,17 @@ async fn turn_event_counts_completed_tool_items() {
|
||||
.to_vec()
|
||||
);
|
||||
|
||||
let image_generation_event = out
|
||||
.iter()
|
||||
.find(|event| matches!(event, TrackEventRequest::ImageGeneration(_)))
|
||||
.expect("image generation event should be emitted");
|
||||
let payload =
|
||||
serde_json::to_value(image_generation_event).expect("serialize image generation event");
|
||||
assert_eq!(
|
||||
payload["event_params"]["imagegen_request_id"],
|
||||
json!("req-imagegen-123")
|
||||
);
|
||||
|
||||
let mcp_tool_call_event = out
|
||||
.iter()
|
||||
.find(|event| matches!(event, TrackEventRequest::McpToolCall(_)))
|
||||
|
||||
@@ -896,6 +896,7 @@ pub(crate) struct CodexImageGenerationEventParams {
|
||||
pub(crate) revised_prompt_present: bool,
|
||||
pub(crate) saved_path_present: bool,
|
||||
pub(crate) transparent_background: Option<bool>,
|
||||
pub(crate) imagegen_request_id: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Serialize)]
|
||||
|
||||
@@ -2833,6 +2833,7 @@ fn tool_item_event(input: ToolItemEventInput<'_>) -> Option<TrackEventRequest> {
|
||||
revised_prompt_present: item.revised_prompt.is_some(),
|
||||
saved_path_present: item.saved_path.is_some(),
|
||||
transparent_background: item.transparent_background,
|
||||
imagegen_request_id: item.imagegen_request_id.clone(),
|
||||
},
|
||||
},
|
||||
))
|
||||
|
||||
@@ -859,6 +859,7 @@ impl ThreadHistoryBuilder {
|
||||
transparent_background: None,
|
||||
failure: None,
|
||||
saved_path: None,
|
||||
imagegen_request_id: None,
|
||||
});
|
||||
self.upsert_item_in_current_turn(item);
|
||||
}
|
||||
@@ -872,6 +873,7 @@ impl ThreadHistoryBuilder {
|
||||
transparent_background: payload.transparent_background,
|
||||
failure: payload.failure.clone(),
|
||||
saved_path: payload.saved_path.clone(),
|
||||
imagegen_request_id: None,
|
||||
});
|
||||
self.upsert_item_in_current_turn(item);
|
||||
}
|
||||
@@ -2083,6 +2085,7 @@ mod tests {
|
||||
transparent_background: Some(true),
|
||||
failure: None,
|
||||
saved_path: Some(saved_path.clone()),
|
||||
imagegen_request_id: None,
|
||||
},
|
||||
)),
|
||||
started_at_ms: Some(0),
|
||||
@@ -2115,6 +2118,7 @@ mod tests {
|
||||
transparent_background: Some(true),
|
||||
failure: None,
|
||||
saved_path: Some(saved_path),
|
||||
imagegen_request_id: None,
|
||||
})]
|
||||
);
|
||||
}
|
||||
@@ -2432,6 +2436,7 @@ mod tests {
|
||||
},
|
||||
),
|
||||
saved_path: Some(test_path_buf("/tmp/ig_123.png").abs()),
|
||||
imagegen_request_id: None,
|
||||
}),
|
||||
],
|
||||
}
|
||||
|
||||
@@ -973,6 +973,7 @@ impl From<CoreTurnItem> for ThreadItem {
|
||||
transparent_background: None,
|
||||
failure: None,
|
||||
saved_path: image.saved_path,
|
||||
imagegen_request_id: None,
|
||||
})
|
||||
}
|
||||
CoreTurnItem::EnteredReviewMode(review) => ThreadItem::EnteredReviewMode {
|
||||
|
||||
@@ -110,6 +110,7 @@ mod tests {
|
||||
transparent_background: None,
|
||||
failure: None,
|
||||
saved_path: Some(test_path_buf("/tmp/ig-1.png").abs()),
|
||||
imagegen_request_id: None,
|
||||
}),
|
||||
]);
|
||||
|
||||
|
||||
@@ -34,6 +34,9 @@ use wiremock::ResponseTemplate;
|
||||
use wiremock::matchers::method;
|
||||
use wiremock::matchers::path;
|
||||
|
||||
use super::analytics::mount_analytics_capture;
|
||||
use super::analytics::wait_for_analytics_event;
|
||||
|
||||
const RESULT: &str = "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==";
|
||||
const TINY_PNG_BYTES: &[u8] = &[
|
||||
137, 80, 78, 71, 13, 10, 26, 10, 0, 0, 0, 13, 73, 72, 68, 82, 0, 0, 0, 1, 0, 0, 0, 1, 8, 6, 0,
|
||||
@@ -87,11 +90,7 @@ async fn standalone_image_generation_returns_saved_path_hint_to_model() -> Resul
|
||||
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml(codex_home.path(), &server.uri(), ImagegenTestMode::Direct)?;
|
||||
write_chatgpt_auth(
|
||||
codex_home.path(),
|
||||
ChatGptAuthFixture::new("access-chatgpt"),
|
||||
AuthCredentialsStoreMode::File,
|
||||
)?;
|
||||
mount_analytics_capture(&server, codex_home.path()).await?;
|
||||
|
||||
let mut mcp = TestAppServer::builder()
|
||||
.with_codex_home(codex_home.path())
|
||||
@@ -183,6 +182,17 @@ async fn standalone_image_generation_returns_saved_path_hint_to_model() -> Resul
|
||||
"standalone image generation should not emit the legacy developer-message hint"
|
||||
);
|
||||
|
||||
let event = wait_for_analytics_event(
|
||||
&server,
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
"codex_image_generation_event",
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(
|
||||
event["event_params"]["imagegen_request_id"],
|
||||
json!("req-imagegen-123")
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -419,6 +429,7 @@ async fn standalone_image_generation_failure_emits_terminal_item() -> Result<()>
|
||||
transparent_background: None,
|
||||
failure: None,
|
||||
saved_path: None,
|
||||
imagegen_request_id: None,
|
||||
})
|
||||
);
|
||||
|
||||
@@ -922,11 +933,15 @@ async fn mount_image_response(server: &MockServer) {
|
||||
async fn mount_image_response_with_background(server: &MockServer, background: &str) {
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/api/codex/images/generations"))
|
||||
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
|
||||
"created": 1,
|
||||
"background": background,
|
||||
"data": [{"b64_json": RESULT}],
|
||||
})))
|
||||
.respond_with(
|
||||
ResponseTemplate::new(200)
|
||||
.insert_header("x-codex-imagegen-request-id", "req-imagegen-123")
|
||||
.set_body_json(json!({
|
||||
"created": 1,
|
||||
"background": background,
|
||||
"data": [{"b64_json": RESULT}],
|
||||
})),
|
||||
)
|
||||
.expect(1)
|
||||
.mount(server)
|
||||
.await;
|
||||
|
||||
@@ -13,6 +13,8 @@ use serde::Serialize;
|
||||
use serde_json::to_value;
|
||||
use std::sync::Arc;
|
||||
|
||||
const X_CODEX_IMAGEGEN_REQUEST_ID_HEADER: &str = "x-codex-imagegen-request-id";
|
||||
|
||||
pub struct ImagesClient<T: HttpTransport> {
|
||||
session: EndpointSession<T>,
|
||||
}
|
||||
@@ -34,7 +36,7 @@ impl<T: HttpTransport> ImagesClient<T> {
|
||||
&self,
|
||||
request: &ImageGenerationRequest,
|
||||
extra_headers: HeaderMap,
|
||||
) -> Result<ImageResponse, ApiError> {
|
||||
) -> Result<(ImageResponse, Option<String>), ApiError> {
|
||||
self.post_image_request(
|
||||
"images/generations",
|
||||
request,
|
||||
@@ -48,7 +50,7 @@ impl<T: HttpTransport> ImagesClient<T> {
|
||||
&self,
|
||||
request: &ImageEditRequest,
|
||||
extra_headers: HeaderMap,
|
||||
) -> Result<ImageResponse, ApiError> {
|
||||
) -> Result<(ImageResponse, Option<String>), ApiError> {
|
||||
self.post_image_request("images/edits", request, extra_headers, "image edit")
|
||||
.await
|
||||
}
|
||||
@@ -59,15 +61,22 @@ impl<T: HttpTransport> ImagesClient<T> {
|
||||
request: &R,
|
||||
extra_headers: HeaderMap,
|
||||
operation: &str,
|
||||
) -> Result<ImageResponse, ApiError> {
|
||||
) -> Result<(ImageResponse, Option<String>), ApiError> {
|
||||
let body = to_value(request)
|
||||
.map_err(|e| ApiError::Stream(format!("failed to encode {operation} request: {e}")))?;
|
||||
let resp = self
|
||||
.session
|
||||
.execute(Method::POST, path, extra_headers, Some(body))
|
||||
.await?;
|
||||
serde_json::from_slice(&resp.body)
|
||||
.map_err(|e| ApiError::Stream(format!("failed to decode {operation} response: {e}")))
|
||||
let imagegen_request_id = resp
|
||||
.headers
|
||||
.get(X_CODEX_IMAGEGEN_REQUEST_ID_HEADER)
|
||||
.and_then(|value| value.to_str().ok())
|
||||
.filter(|request_id| !request_id.is_empty())
|
||||
.map(str::to_string);
|
||||
let response = serde_json::from_slice(&resp.body)
|
||||
.map_err(|e| ApiError::Stream(format!("failed to decode {operation} response: {e}")))?;
|
||||
Ok((response, imagegen_request_id))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -102,13 +111,19 @@ mod tests {
|
||||
struct CapturingTransport {
|
||||
last_request: Arc<Mutex<Option<Request>>>,
|
||||
response_body: Arc<Vec<u8>>,
|
||||
response_headers: HeaderMap,
|
||||
}
|
||||
|
||||
impl CapturingTransport {
|
||||
fn new(response_body: Vec<u8>) -> Self {
|
||||
Self::with_response_headers(response_body, HeaderMap::new())
|
||||
}
|
||||
|
||||
fn with_response_headers(response_body: Vec<u8>, response_headers: HeaderMap) -> Self {
|
||||
Self {
|
||||
last_request: Arc::new(Mutex::new(None)),
|
||||
response_body: Arc::new(response_body),
|
||||
response_headers,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -118,7 +133,7 @@ mod tests {
|
||||
*self.last_request.lock().expect("lock request store") = Some(req);
|
||||
Ok(Response {
|
||||
status: StatusCode::OK,
|
||||
headers: HeaderMap::new(),
|
||||
headers: self.response_headers.clone(),
|
||||
body: self.response_body.as_ref().clone().into(),
|
||||
})
|
||||
}
|
||||
@@ -193,10 +208,20 @@ mod tests {
|
||||
|
||||
#[tokio::test]
|
||||
async fn generate_posts_typed_request_and_parses_image_response() {
|
||||
let transport = CapturingTransport::new(response_body());
|
||||
let mut response_headers = HeaderMap::new();
|
||||
response_headers.insert(
|
||||
X_CODEX_IMAGEGEN_REQUEST_ID_HEADER,
|
||||
http::HeaderValue::from_static("req-imagegen-123"),
|
||||
);
|
||||
response_headers.insert(
|
||||
"x-request-id",
|
||||
http::HeaderValue::from_static("req-outer-456"),
|
||||
);
|
||||
let transport =
|
||||
CapturingTransport::with_response_headers(response_body(), response_headers);
|
||||
let client = ImagesClient::new(transport.clone(), provider(), Arc::new(DummyAuth));
|
||||
|
||||
let response = client
|
||||
let (response, imagegen_request_id) = client
|
||||
.generate(
|
||||
&ImageGenerationRequest {
|
||||
prompt: "a red fox in a field".to_string(),
|
||||
@@ -212,6 +237,7 @@ mod tests {
|
||||
.expect("image generation request should succeed");
|
||||
|
||||
assert_eq!(response, expected_response());
|
||||
assert_eq!(imagegen_request_id.as_deref(), Some("req-imagegen-123"));
|
||||
|
||||
let request = captured_request(&transport);
|
||||
assert_eq!(
|
||||
@@ -235,7 +261,7 @@ mod tests {
|
||||
let transport = CapturingTransport::new(response_body());
|
||||
let client = ImagesClient::new(transport.clone(), provider(), Arc::new(DummyAuth));
|
||||
|
||||
let response = client
|
||||
let (response, imagegen_request_id) = client
|
||||
.edit(
|
||||
&ImageEditRequest {
|
||||
images: vec![ImageUrl {
|
||||
@@ -254,6 +280,7 @@ mod tests {
|
||||
.expect("image edit request should succeed");
|
||||
|
||||
assert_eq!(response, expected_response());
|
||||
assert_eq!(imagegen_request_id, None);
|
||||
|
||||
let request = captured_request(&transport);
|
||||
assert_eq!(request.url, "https://example.com/api/codex/images/edits");
|
||||
|
||||
@@ -517,6 +517,7 @@ mod tests {
|
||||
transparent_background: None,
|
||||
failure: None,
|
||||
saved_path: None,
|
||||
imagegen_request_id: None,
|
||||
});
|
||||
let expected_completed_item = ExtensionItem::ImageGeneration(ImageGenerationItem {
|
||||
id: "call-image".to_string(),
|
||||
@@ -526,6 +527,7 @@ mod tests {
|
||||
transparent_background: Some(true),
|
||||
failure: None,
|
||||
saved_path: Some(expected_path.clone()),
|
||||
imagegen_request_id: None,
|
||||
});
|
||||
codex_tools::TurnItemEmitter::emit_started(
|
||||
&emitter,
|
||||
|
||||
@@ -82,7 +82,7 @@ impl CodexImagesBackend {
|
||||
&self,
|
||||
request: ImageGenerationRequest,
|
||||
turn_id: &str,
|
||||
) -> Result<ImageResponse, ImageBackendError> {
|
||||
) -> Result<(ImageResponse, Option<String>), ImageBackendError> {
|
||||
self.client()
|
||||
.await?
|
||||
.generate(
|
||||
@@ -98,7 +98,7 @@ impl CodexImagesBackend {
|
||||
&self,
|
||||
request: ImageEditRequest,
|
||||
turn_id: &str,
|
||||
) -> Result<ImageResponse, ImageBackendError> {
|
||||
) -> Result<(ImageResponse, Option<String>), ImageBackendError> {
|
||||
self.client()
|
||||
.await?
|
||||
.edit(
|
||||
|
||||
@@ -151,6 +151,7 @@ impl ImageGenerationTool {
|
||||
transparent_background: None,
|
||||
failure: None,
|
||||
saved_path: None,
|
||||
imagegen_request_id: None,
|
||||
},
|
||||
EventMsg::ImageGenerationBegin(ImageGenerationBeginEvent {
|
||||
call_id: call.call_id.clone(),
|
||||
@@ -167,7 +168,7 @@ impl ImageGenerationTool {
|
||||
usage_limit_failure(error.codex_error()),
|
||||
)
|
||||
})
|
||||
.and_then(|response| {
|
||||
.and_then(|(response, imagegen_request_id)| {
|
||||
let transparent_background = match response.background {
|
||||
Some(ImageBackground::Transparent) => Some(true),
|
||||
Some(ImageBackground::Opaque) => Some(false),
|
||||
@@ -177,10 +178,10 @@ impl ImageGenerationTool {
|
||||
.data
|
||||
.into_iter()
|
||||
.next()
|
||||
.map(|data| (data.b64_json, transparent_background))
|
||||
.map(|data| (data.b64_json, transparent_background, imagegen_request_id))
|
||||
.ok_or_else(|| ("image generation returned no image data".to_string(), None))
|
||||
});
|
||||
let (result, transparent_background) = match result {
|
||||
let (result, transparent_background, imagegen_request_id) = match result {
|
||||
Ok(result) => result,
|
||||
Err((message, failure)) => {
|
||||
let item = ImageGenerationItem {
|
||||
@@ -191,6 +192,7 @@ impl ImageGenerationTool {
|
||||
transparent_background: None,
|
||||
failure,
|
||||
saved_path: None,
|
||||
imagegen_request_id: None,
|
||||
};
|
||||
let legacy_event = legacy_end_event(&item);
|
||||
call.turn_item_emitter
|
||||
@@ -215,6 +217,7 @@ impl ImageGenerationTool {
|
||||
transparent_background,
|
||||
failure: None,
|
||||
saved_path: saved_path.clone(),
|
||||
imagegen_request_id,
|
||||
};
|
||||
let legacy_event = legacy_end_event(&item);
|
||||
call.turn_item_emitter
|
||||
|
||||
@@ -38,4 +38,12 @@ pub struct ImageGenerationItem {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub saved_path: Option<AbsolutePathBuf>,
|
||||
/// Exact nested ImageGen request ID retained only for in-process analytics.
|
||||
///
|
||||
/// This is deliberately excluded from the extension/app-server wire shape
|
||||
/// and rollout persistence because it is not a client-facing item field.
|
||||
#[serde(skip)]
|
||||
#[schemars(skip)]
|
||||
#[ts(skip)]
|
||||
pub imagegen_request_id: Option<String>,
|
||||
}
|
||||
|
||||
@@ -17,6 +17,7 @@ fn completed_image_generation_item() -> ExtensionItem {
|
||||
transparent_background: None,
|
||||
failure: None,
|
||||
saved_path: None,
|
||||
imagegen_request_id: None,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -89,6 +90,25 @@ fn image_generation_transparency_is_optional_in_typescript() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn image_generation_request_id_stays_internal() {
|
||||
let ExtensionItem::ImageGeneration(mut image) = completed_image_generation_item() else {
|
||||
panic!("expected image-generation item");
|
||||
};
|
||||
image.imagegen_request_id = Some("req-imagegen-123".to_string());
|
||||
let item = ExtensionItem::ImageGeneration(image);
|
||||
let value = serde_json::to_value(&item).expect("serialize extension item");
|
||||
|
||||
assert!(value.get("imagegenRequestId").is_none());
|
||||
let ExtensionItem::ImageGeneration(round_tripped) =
|
||||
serde_json::from_value::<ExtensionItem>(value).expect("deserialize extension item")
|
||||
else {
|
||||
panic!("expected image-generation item");
|
||||
};
|
||||
assert_eq!(round_tripped.imagegen_request_id, None);
|
||||
assert!(!ImageGenerationItem::inline().contains("imagegenRequestId"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn web_search_item_preserves_stable_wire_shape() {
|
||||
let item = ExtensionItem::WebSearch(WebSearchItem {
|
||||
|
||||
@@ -166,6 +166,7 @@ pub(super) fn completed_item(
|
||||
transparent_background: event.transparent_background,
|
||||
failure: event.failure.clone(),
|
||||
saved_path: event.saved_path.clone(),
|
||||
imagegen_request_id: None,
|
||||
})),
|
||||
None,
|
||||
)),
|
||||
|
||||
@@ -441,6 +441,7 @@ async fn migration_preserves_image_generation_failure_metadata() {
|
||||
resets_at: Some(1_786_150_800),
|
||||
}),
|
||||
saved_path: None,
|
||||
imagegen_request_id: None,
|
||||
};
|
||||
let image_completion =
|
||||
RolloutItem::EventMsg(EventMsg::ImageGenerationEnd(ImageGenerationEndEvent {
|
||||
|
||||
@@ -715,6 +715,7 @@ pub(super) fn handle_image_generation_end(
|
||||
transparent_background: None,
|
||||
failure: None,
|
||||
saved_path,
|
||||
imagegen_request_id: None,
|
||||
}),
|
||||
}),
|
||||
/*replay_kind*/ None,
|
||||
|
||||
Reference in New Issue
Block a user