mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
Pass thread IDs to attachment uploads (#47121)
## Why Attachment stores need the receiving thread ID to support thread-based placement. ## What changed Add `thread_id` to `UploadRequest` and pass the session's thread ID through image preparation for messages and tool outputs. Include the ID in debug output while continuing to redact attachment bytes. ## Testing Extend the image rollout test to assert that a new image uploaded after resume carries the resumed session's thread ID. Update the debug-output test for the new field. GitOrigin-RevId: f81e3bd761dca704064287177b8ecd7175662f07
This commit is contained in:
committed by
copyberry
parent
5aa4939be4
commit
4a8cfe6bb7
@@ -36,6 +36,8 @@ pub trait AttachmentStore: Send + Sync {
|
||||
/// Attachment data supplied to [`AttachmentStore::upload`].
|
||||
#[derive(Clone, Eq, PartialEq)]
|
||||
pub struct UploadRequest {
|
||||
/// Thread receiving this attachment; storage backends may use it for placement.
|
||||
pub thread_id: String,
|
||||
/// Optional name associated with the attachment.
|
||||
pub file_name: Option<String>,
|
||||
/// Attachment bytes to persist.
|
||||
@@ -46,6 +48,7 @@ impl fmt::Debug for UploadRequest {
|
||||
fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
formatter
|
||||
.debug_struct("UploadRequest")
|
||||
.field("thread_id", &self.thread_id)
|
||||
.field("file_name", &self.file_name)
|
||||
.field("data", &"<redacted>")
|
||||
.finish()
|
||||
|
||||
@@ -6,6 +6,7 @@ use super::*;
|
||||
#[test]
|
||||
fn attachment_debug_output_redacts_bytes() {
|
||||
let request = UploadRequest {
|
||||
thread_id: "thread-1".to_string(),
|
||||
file_name: Some("image.png".to_string()),
|
||||
data: b"secret".to_vec(),
|
||||
};
|
||||
@@ -16,7 +17,7 @@ fn attachment_debug_output_redacts_bytes() {
|
||||
assert_eq!(
|
||||
(format!("{request:?}"), format!("{result:?}")),
|
||||
(
|
||||
r#"UploadRequest { file_name: Some("image.png"), data: "<redacted>" }"#.to_string(),
|
||||
r#"UploadRequest { thread_id: "thread-1", file_name: Some("image.png"), data: "<redacted>" }"#.to_string(),
|
||||
r#"Inline { bytes: "<redacted>" }"#.to_string(),
|
||||
)
|
||||
);
|
||||
@@ -47,6 +48,7 @@ async fn inline_store_preserves_image_bytes() {
|
||||
for (file_name, data) in cases {
|
||||
let attachment = InlineAttachmentStore
|
||||
.upload(UploadRequest {
|
||||
thread_id: "thread-1".to_string(),
|
||||
file_name: Some(file_name.to_string()),
|
||||
data: data.to_vec(),
|
||||
})
|
||||
|
||||
@@ -52,6 +52,7 @@ pub(crate) fn unified_image_budget_enabled(
|
||||
|
||||
#[derive(Clone, Copy, Debug)]
|
||||
struct ImageOrigin<'a> {
|
||||
thread_id: &'a str,
|
||||
message_role: Option<&'a str>,
|
||||
item_id: Option<&'a str>,
|
||||
}
|
||||
@@ -118,6 +119,7 @@ impl ImagePreparationError {
|
||||
}
|
||||
|
||||
pub(crate) async fn prepare_response_items(
|
||||
thread_id: &str,
|
||||
items: &mut Vec<ResponseItem>,
|
||||
mode: ImagePreparationMode,
|
||||
resize_notice_mode: ImageResizeNoticeMode,
|
||||
@@ -135,6 +137,7 @@ pub(crate) async fn prepare_response_items(
|
||||
let resized_images = prepare_message_content(
|
||||
&mut content,
|
||||
ImageOrigin {
|
||||
thread_id,
|
||||
message_role: Some(role.as_str()),
|
||||
item_id: None,
|
||||
},
|
||||
@@ -157,6 +160,7 @@ pub(crate) async fn prepare_response_items(
|
||||
call_id, output, ..
|
||||
} => {
|
||||
prepare_tool_output(
|
||||
thread_id,
|
||||
output,
|
||||
call_id.as_deref(),
|
||||
resize_notice_mode,
|
||||
@@ -170,6 +174,7 @@ pub(crate) async fn prepare_response_items(
|
||||
call_id, output, ..
|
||||
} => {
|
||||
prepare_tool_output(
|
||||
thread_id,
|
||||
output,
|
||||
Some(call_id.as_str()),
|
||||
resize_notice_mode,
|
||||
@@ -205,6 +210,7 @@ pub(crate) async fn prepare_response_items(
|
||||
}
|
||||
|
||||
async fn prepare_tool_output(
|
||||
thread_id: &str,
|
||||
output: &mut FunctionCallOutputPayload,
|
||||
item_id: Option<&str>,
|
||||
resize_notice_mode: ImageResizeNoticeMode,
|
||||
@@ -216,6 +222,7 @@ async fn prepare_tool_output(
|
||||
let resized_images = prepare_tool_output_content(
|
||||
content,
|
||||
ImageOrigin {
|
||||
thread_id,
|
||||
message_role: None,
|
||||
item_id,
|
||||
},
|
||||
@@ -349,6 +356,7 @@ async fn prepare_image(
|
||||
metadata.push(prepared.metadata(origin));
|
||||
let resize = prepared.resize;
|
||||
let request = UploadRequest {
|
||||
thread_id: origin.thread_id.to_owned(),
|
||||
file_name: None,
|
||||
data: prepared.encoded.bytes.to_vec(),
|
||||
};
|
||||
|
||||
@@ -100,6 +100,7 @@ async fn preparation_preserves_small_image_bytes_and_replaces_remote_urls() {
|
||||
});
|
||||
|
||||
prepare_response_items(
|
||||
"image-preparation-thread",
|
||||
&mut items,
|
||||
ImagePreparationMode::DetailBased,
|
||||
ImageResizeNoticeMode::Disabled,
|
||||
@@ -185,6 +186,7 @@ async fn detail_policies_apply_the_expected_budgets() {
|
||||
}];
|
||||
|
||||
let metadata = prepare_response_items(
|
||||
"image-preparation-thread",
|
||||
&mut items,
|
||||
ImagePreparationMode::DetailBased,
|
||||
ImageResizeNoticeMode::Disabled,
|
||||
@@ -238,6 +240,7 @@ async fn preparation_reports_tool_output_item_id() {
|
||||
internal_chat_message_metadata_passthrough: None,
|
||||
}];
|
||||
let metadata = prepare_response_items(
|
||||
"image-preparation-thread",
|
||||
&mut items,
|
||||
ImagePreparationMode::DetailBased,
|
||||
ImageResizeNoticeMode::Disabled,
|
||||
@@ -274,6 +277,7 @@ async fn upload_failure_keeps_resized_image_inline() {
|
||||
}];
|
||||
|
||||
prepare_response_items(
|
||||
"image-preparation-thread",
|
||||
&mut items,
|
||||
ImagePreparationMode::DetailBased,
|
||||
ImageResizeNoticeMode::Disabled,
|
||||
@@ -374,6 +378,7 @@ async fn resize_notices_count_file_backed_images_and_skip_failed_images() {
|
||||
];
|
||||
|
||||
prepare_response_items(
|
||||
"image-preparation-thread",
|
||||
&mut items,
|
||||
ImagePreparationMode::DetailBased,
|
||||
ImageResizeNoticeMode::Enabled,
|
||||
@@ -561,6 +566,7 @@ async fn preparation_replaces_only_failed_tool_images_and_preserves_metadata() {
|
||||
}];
|
||||
|
||||
prepare_response_items(
|
||||
"image-preparation-thread",
|
||||
&mut items,
|
||||
ImagePreparationMode::DetailBased,
|
||||
ImageResizeNoticeMode::Disabled,
|
||||
|
||||
@@ -1726,6 +1726,7 @@ impl Session {
|
||||
// inline bytes, while existing file references bypass preparation and remain unchanged.
|
||||
// Bound replay future size now that image preparation can await storage.
|
||||
let _ = Box::pin(prepare_image_response_items(
|
||||
&self.thread_id.to_string(),
|
||||
&mut prepared_history,
|
||||
ImagePreparationMode::DetailBased,
|
||||
ImageResizeNoticeMode::Disabled,
|
||||
@@ -3412,6 +3413,7 @@ impl Session {
|
||||
};
|
||||
// Keep nested image-upload futures out of every caller's future frame.
|
||||
let image_preparations = Box::pin(prepare_image_response_items(
|
||||
&self.thread_id.to_string(),
|
||||
&mut items,
|
||||
image_preparation_mode,
|
||||
image_resize_notice_mode,
|
||||
|
||||
@@ -679,6 +679,10 @@ async fn resumed_history_only_emits_resize_notices_for_new_images() -> anyhow::R
|
||||
let [upload] = uploads.as_slice() else {
|
||||
panic!("only the new image should be uploaded");
|
||||
};
|
||||
assert_eq!(
|
||||
upload.thread_id,
|
||||
resumed.session_configured.thread_id.to_string(),
|
||||
);
|
||||
assert_eq!(
|
||||
image::load_from_memory(&upload.data)?.dimensions(),
|
||||
(2048, 768)
|
||||
|
||||
Reference in New Issue
Block a user