mirror of
https://github.com/openai/codex.git
synced 2026-09-20 12:47:38 +00:00
Defer standalone image save-path hint
This commit is contained in:
1
codex-rs/Cargo.lock
generated
1
codex-rs/Cargo.lock
generated
@@ -3042,7 +3042,6 @@ dependencies = [
|
||||
"codex-model-provider-info",
|
||||
"codex-protocol",
|
||||
"codex-tools",
|
||||
"codex-utils-absolute-path",
|
||||
"http 1.4.0",
|
||||
"pretty_assertions",
|
||||
"schemars 0.8.22",
|
||||
|
||||
@@ -38,7 +38,7 @@ const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(60);
|
||||
const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(10);
|
||||
|
||||
#[tokio::test]
|
||||
async fn standalone_image_generation_returns_saved_path_hint_to_model() -> Result<()> {
|
||||
async fn standalone_image_generation_persists_image_and_returns_it_to_model() -> Result<()> {
|
||||
let call_id = "image-run-1";
|
||||
let server = responses::start_mock_server().await;
|
||||
mount_image_response(&server).await;
|
||||
@@ -144,13 +144,7 @@ async fn standalone_image_generation_returns_saved_path_hint_to_model() -> Resul
|
||||
"detail": "high",
|
||||
})
|
||||
);
|
||||
let output_hint = output["output"][1]["text"]
|
||||
.as_str()
|
||||
.context("image output should include model-visible path hint")?;
|
||||
assert!(
|
||||
output_hint.contains(&saved_path.display().to_string()),
|
||||
"output hint should identify the path core saved"
|
||||
);
|
||||
assert_eq!(output["output"].as_array().map(Vec::len), Some(1));
|
||||
assert!(
|
||||
!requests[1]
|
||||
.message_input_texts("developer")
|
||||
|
||||
@@ -1,16 +1,6 @@
|
||||
use super::ContextualUserFragment;
|
||||
use std::fmt::Display;
|
||||
|
||||
/// Returns the model-facing hint for the host's generated-image artifact path.
|
||||
pub fn image_generation_output_hint(
|
||||
image_output_dir: impl Display,
|
||||
image_output_path: impl Display,
|
||||
) -> String {
|
||||
format!(
|
||||
"Generated images are saved to {image_output_dir} as {image_output_path} by default.\nIf you need to use a generated image at another path, copy it and leave the original in place unless the user explicitly asks you to delete it."
|
||||
)
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq)]
|
||||
pub(crate) struct ImageGenerationInstructions {
|
||||
image_output_dir: String,
|
||||
@@ -40,6 +30,9 @@ impl ContextualUserFragment for ImageGenerationInstructions {
|
||||
}
|
||||
|
||||
fn body(&self) -> String {
|
||||
image_generation_output_hint(&self.image_output_dir, &self.image_output_path)
|
||||
format!(
|
||||
"Generated images are saved to {} as {} by default.\nIf you need to use a generated image at another path, copy it and leave the original in place unless the user explicitly asks you to delete it.",
|
||||
self.image_output_dir, self.image_output_path
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -46,7 +46,6 @@ pub(crate) use fragments::AdditionalContextUserFragment;
|
||||
pub(crate) use guardian_followup_review_reminder::GuardianFollowupReviewReminder;
|
||||
pub(crate) use hook_additional_context::HookAdditionalContext;
|
||||
pub(crate) use image_generation_instructions::ImageGenerationInstructions;
|
||||
pub use image_generation_instructions::image_generation_output_hint;
|
||||
pub use internal_model_context::InternalContextSource;
|
||||
pub use internal_model_context::InternalModelContextFragment;
|
||||
pub use internal_model_context::InvalidInternalContextSource;
|
||||
|
||||
@@ -97,7 +97,6 @@ pub(crate) use skills::manager;
|
||||
pub(crate) use skills::maybe_emit_implicit_skill_invocation;
|
||||
pub(crate) use skills::skills_load_input_from_config;
|
||||
mod stream_events_utils;
|
||||
pub use stream_events_utils::image_generation_artifact_path;
|
||||
pub mod test_support;
|
||||
mod unified_exec;
|
||||
pub mod windows_sandbox;
|
||||
|
||||
@@ -38,8 +38,7 @@ use tracing::warn;
|
||||
|
||||
const GENERATED_IMAGE_ARTIFACTS_DIR: &str = "generated_images";
|
||||
|
||||
/// Returns the host-owned default artifact path for a generated image.
|
||||
pub fn image_generation_artifact_path(
|
||||
pub(crate) fn image_generation_artifact_path(
|
||||
codex_home: &AbsolutePathBuf,
|
||||
session_id: &str,
|
||||
call_id: &str,
|
||||
|
||||
@@ -111,7 +111,6 @@ async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall {
|
||||
let conversation_history =
|
||||
ConversationHistory::new(invocation.session.clone_history().await.into_raw_items());
|
||||
ExtensionToolCall {
|
||||
thread_id: invocation.session.conversation_id.to_string(),
|
||||
turn_id: invocation.turn.sub_id.clone(),
|
||||
call_id: invocation.call_id.clone(),
|
||||
tool_name: invocation.tool_name.clone(),
|
||||
@@ -279,7 +278,6 @@ mod tests {
|
||||
let (session, turn, rx) = crate::session::tests::make_session_and_context_with_rx().await;
|
||||
let weak_session = Arc::downgrade(&session);
|
||||
let weak_turn = Arc::downgrade(&turn);
|
||||
let thread_id = session.conversation_id.to_string();
|
||||
let turn_id = turn.sub_id.clone();
|
||||
let truncation_policy = turn.truncation_policy;
|
||||
let history_item = ResponseItem::Message {
|
||||
@@ -318,7 +316,6 @@ mod tests {
|
||||
let captured_call = captured_call.lock().await.clone().expect("captured call");
|
||||
assert!(weak_session.upgrade().is_none());
|
||||
assert!(weak_turn.upgrade().is_none());
|
||||
assert_eq!(captured_call.thread_id, thread_id);
|
||||
assert_eq!(captured_call.turn_id, turn_id);
|
||||
assert_eq!(captured_call.call_id, "call-extension");
|
||||
assert_eq!(
|
||||
|
||||
@@ -23,7 +23,6 @@ codex-model-provider = { workspace = true }
|
||||
codex-model-provider-info = { workspace = true }
|
||||
codex-protocol = { workspace = true }
|
||||
codex-tools = { workspace = true }
|
||||
codex-utils-absolute-path = { workspace = true }
|
||||
http = { workspace = true }
|
||||
schemars = { workspace = true }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
|
||||
@@ -13,7 +13,6 @@ use codex_features::Feature;
|
||||
use codex_login::AuthManager;
|
||||
use codex_model_provider::create_model_provider;
|
||||
use codex_model_provider_info::ModelProviderInfo;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
|
||||
use crate::backend::CodexImagesBackend;
|
||||
use crate::tool::ImageGenerationTool;
|
||||
@@ -27,7 +26,6 @@ struct ImageGenerationExtension {
|
||||
struct ImageGenerationExtensionConfig {
|
||||
enabled: bool,
|
||||
provider: ModelProviderInfo,
|
||||
codex_home: AbsolutePathBuf,
|
||||
}
|
||||
|
||||
impl From<&Config> for ImageGenerationExtensionConfig {
|
||||
@@ -37,7 +35,6 @@ impl From<&Config> for ImageGenerationExtensionConfig {
|
||||
enabled: config.features.enabled(Feature::ImageGenExt)
|
||||
&& config.model_provider.is_openai(),
|
||||
provider: config.model_provider.clone(),
|
||||
codex_home: config.codex_home.clone(),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -79,13 +76,9 @@ impl ToolContributor for ImageGenerationExtension {
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
vec![Arc::new(ImageGenerationTool::new(
|
||||
CodexImagesBackend::new(create_model_provider(
|
||||
config.provider.clone(),
|
||||
Some(self.auth_manager.clone()),
|
||||
)),
|
||||
config.codex_home.clone(),
|
||||
))]
|
||||
vec![Arc::new(ImageGenerationTool::new(CodexImagesBackend::new(
|
||||
create_model_provider(config.provider.clone(), Some(self.auth_manager.clone())),
|
||||
)))]
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -3,7 +3,6 @@ use codex_api::ImageEditRequest;
|
||||
use codex_api::ImageGenerationRequest;
|
||||
use codex_api::ImageQuality;
|
||||
use codex_api::ImageUrl;
|
||||
use codex_core::context::image_generation_output_hint;
|
||||
use codex_extension_api::ToolOutput;
|
||||
use codex_extension_api::ToolPayload;
|
||||
use codex_extension_api::ToolSpec;
|
||||
@@ -55,11 +54,9 @@ fn generate_uses_fixed_request_defaults() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn generated_output_returns_image_input_and_output_hint() {
|
||||
let output_hint = image_generation_output_hint("/tmp", "/tmp/call-1.png");
|
||||
fn generated_output_returns_image_input() {
|
||||
let output = GeneratedImageOutput {
|
||||
result: RESULT.to_string(),
|
||||
output_hint: output_hint.clone(),
|
||||
};
|
||||
|
||||
let ResponseInputItem::FunctionCallOutput {
|
||||
@@ -74,13 +71,10 @@ fn generated_output_returns_image_input_and_output_hint() {
|
||||
};
|
||||
assert_eq!(
|
||||
content_items,
|
||||
vec![
|
||||
FunctionCallOutputContentItem::InputImage {
|
||||
image_url: format!("data:image/png;base64,{RESULT}"),
|
||||
detail: Some(DEFAULT_IMAGE_DETAIL),
|
||||
},
|
||||
FunctionCallOutputContentItem::InputText { text: output_hint },
|
||||
]
|
||||
vec![FunctionCallOutputContentItem::InputImage {
|
||||
image_url: format!("data:image/png;base64,{RESULT}"),
|
||||
detail: Some(DEFAULT_IMAGE_DETAIL),
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -3,8 +3,6 @@ use codex_api::ImageEditRequest;
|
||||
use codex_api::ImageGenerationRequest;
|
||||
use codex_api::ImageQuality;
|
||||
use codex_api::ImageUrl;
|
||||
use codex_core::context::image_generation_output_hint;
|
||||
use codex_core::image_generation_artifact_path;
|
||||
use codex_extension_api::ExtensionTurnItem;
|
||||
use codex_extension_api::FunctionCallError;
|
||||
use codex_extension_api::ToolCall;
|
||||
@@ -27,7 +25,6 @@ use codex_tools::ResponsesApiNamespaceTool;
|
||||
use codex_tools::ResponsesApiTool;
|
||||
use codex_tools::ToolExposure;
|
||||
use codex_tools::default_namespace_description;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use schemars::JsonSchema;
|
||||
use schemars::r#gen::SchemaSettings;
|
||||
use serde::Deserialize;
|
||||
@@ -45,16 +42,12 @@ const IMAGEGEN_DESCRIPTION: &str = include_str!("../imagegen_description.md");
|
||||
#[derive(Clone)]
|
||||
pub(crate) struct ImageGenerationTool {
|
||||
backend: CodexImagesBackend,
|
||||
codex_home: AbsolutePathBuf,
|
||||
}
|
||||
|
||||
impl ImageGenerationTool {
|
||||
/// Creates an image-generation tool backed by an image API executor.
|
||||
pub(crate) fn new(backend: CodexImagesBackend, codex_home: AbsolutePathBuf) -> Self {
|
||||
Self {
|
||||
backend,
|
||||
codex_home,
|
||||
}
|
||||
pub(crate) fn new(backend: CodexImagesBackend) -> Self {
|
||||
Self { backend }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -123,16 +116,7 @@ impl ToolExecutor<ToolCall> for ImageGenerationTool {
|
||||
saved_path: None,
|
||||
}))
|
||||
.await;
|
||||
let output_path =
|
||||
image_generation_artifact_path(&self.codex_home, &call.thread_id, &call.call_id);
|
||||
let output_dir = output_path
|
||||
.parent()
|
||||
.unwrap_or_else(|| self.codex_home.clone());
|
||||
let output_hint = image_generation_output_hint(output_dir.display(), output_path.display());
|
||||
Ok(Box::new(GeneratedImageOutput {
|
||||
result,
|
||||
output_hint,
|
||||
}))
|
||||
Ok(Box::new(GeneratedImageOutput { result }))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -313,7 +297,6 @@ fn imagegen_tool_spec() -> ToolSpec {
|
||||
|
||||
struct GeneratedImageOutput {
|
||||
result: String,
|
||||
output_hint: String,
|
||||
}
|
||||
|
||||
impl ToolOutput for GeneratedImageOutput {
|
||||
@@ -327,15 +310,12 @@ impl ToolOutput for GeneratedImageOutput {
|
||||
true
|
||||
}
|
||||
|
||||
/// Returns generated bytes and persisted-artifact context for model follow-up.
|
||||
/// Returns generated bytes for model follow-up.
|
||||
fn to_response_item(&self, call_id: &str, _payload: &ToolPayload) -> ResponseInputItem {
|
||||
let mut content = vec![FunctionCallOutputContentItem::InputImage {
|
||||
let content = vec![FunctionCallOutputContentItem::InputImage {
|
||||
image_url: format!("data:image/png;base64,{}", self.result),
|
||||
detail: Some(DEFAULT_IMAGE_DETAIL),
|
||||
}];
|
||||
content.push(FunctionCallOutputContentItem::InputText {
|
||||
text: self.output_hint.clone(),
|
||||
});
|
||||
ResponseInputItem::FunctionCallOutput {
|
||||
call_id: call_id.to_string(),
|
||||
output: FunctionCallOutputPayload {
|
||||
|
||||
@@ -66,7 +66,6 @@ impl TurnItemEmitter for NoopTurnItemEmitter {
|
||||
// TODO: this is temporary and will disappear in the next PR (as we make codex-extension-api generic on Invocation.
|
||||
#[derive(Clone)]
|
||||
pub struct ToolCall {
|
||||
pub thread_id: String,
|
||||
pub turn_id: String,
|
||||
pub call_id: String,
|
||||
pub tool_name: ToolName,
|
||||
@@ -79,7 +78,6 @@ pub struct ToolCall {
|
||||
impl std::fmt::Debug for ToolCall {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
f.debug_struct("ToolCall")
|
||||
.field("thread_id", &self.thread_id)
|
||||
.field("turn_id", &self.turn_id)
|
||||
.field("call_id", &self.call_id)
|
||||
.field("tool_name", &self.tool_name)
|
||||
|
||||
Reference in New Issue
Block a user