mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat: add multi-actions to presentation tool (#13357)
This commit is contained in:
@@ -85,6 +85,27 @@ pub struct PresentationArtifactRequest {
|
||||
pub args: Value,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
pub struct PresentationArtifactToolRequest {
|
||||
pub artifact_id: Option<String>,
|
||||
pub actions: Vec<PresentationArtifactToolAction>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct PresentationArtifactExecutionRequest {
|
||||
pub artifact_id: Option<String>,
|
||||
pub requests: Vec<PresentationArtifactRequest>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize)]
|
||||
#[serde(deny_unknown_fields)]
|
||||
pub struct PresentationArtifactToolAction {
|
||||
pub action: String,
|
||||
#[serde(default)]
|
||||
pub args: Value,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum PathAccessKind {
|
||||
Read,
|
||||
@@ -99,6 +120,10 @@ pub struct PathAccessRequirement {
|
||||
}
|
||||
|
||||
impl PresentationArtifactRequest {
|
||||
pub fn is_mutating(&self) -> bool {
|
||||
!is_read_only_action(&self.action)
|
||||
}
|
||||
|
||||
pub fn required_path_accesses(
|
||||
&self,
|
||||
cwd: &Path,
|
||||
@@ -175,3 +200,50 @@ impl PresentationArtifactRequest {
|
||||
Ok(access)
|
||||
}
|
||||
}
|
||||
|
||||
impl PresentationArtifactToolRequest {
|
||||
pub fn is_mutating(&self) -> Result<bool, PresentationArtifactError> {
|
||||
Ok(self.actions.iter().any(|request| !is_read_only_action(&request.action)))
|
||||
}
|
||||
|
||||
pub fn into_execution_request(
|
||||
self,
|
||||
) -> Result<PresentationArtifactExecutionRequest, PresentationArtifactError> {
|
||||
if self.actions.is_empty() {
|
||||
return Err(PresentationArtifactError::InvalidArgs {
|
||||
action: "presentation_artifact".to_string(),
|
||||
message: "`actions` must contain at least one item".to_string(),
|
||||
});
|
||||
}
|
||||
Ok(PresentationArtifactExecutionRequest {
|
||||
artifact_id: self.artifact_id,
|
||||
requests: self
|
||||
.actions
|
||||
.into_iter()
|
||||
.map(|request| PresentationArtifactRequest {
|
||||
artifact_id: None,
|
||||
action: request.action,
|
||||
args: request.args,
|
||||
})
|
||||
.collect(),
|
||||
})
|
||||
}
|
||||
|
||||
pub fn required_path_accesses(
|
||||
&self,
|
||||
cwd: &Path,
|
||||
) -> Result<Vec<PathAccessRequirement>, PresentationArtifactError> {
|
||||
let mut accesses = Vec::new();
|
||||
for request in &self.actions {
|
||||
accesses.extend(
|
||||
PresentationArtifactRequest {
|
||||
artifact_id: None,
|
||||
action: request.action.clone(),
|
||||
args: request.args.clone(),
|
||||
}
|
||||
.required_path_accesses(cwd)?,
|
||||
);
|
||||
}
|
||||
Ok(accesses)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -14,6 +14,47 @@ struct HistoryEntry {
|
||||
}
|
||||
|
||||
impl PresentationArtifactManager {
|
||||
pub fn execute_requests(
|
||||
&mut self,
|
||||
request: PresentationArtifactExecutionRequest,
|
||||
cwd: &Path,
|
||||
) -> Result<PresentationArtifactResponse, PresentationArtifactError> {
|
||||
let PresentationArtifactExecutionRequest {
|
||||
artifact_id,
|
||||
requests,
|
||||
} = request;
|
||||
let request_count = requests.len();
|
||||
let mut current_artifact_id = artifact_id;
|
||||
let mut executed_actions = Vec::with_capacity(request_count);
|
||||
let mut exported_paths = Vec::new();
|
||||
let mut last_response = None;
|
||||
|
||||
for mut request in requests {
|
||||
if request.artifact_id.is_none() {
|
||||
request.artifact_id = current_artifact_id.clone();
|
||||
}
|
||||
let response = self.execute(request, cwd)?;
|
||||
current_artifact_id = Some(response.artifact_id.clone());
|
||||
exported_paths.extend(response.exported_paths.iter().cloned());
|
||||
executed_actions.push(response.action.clone());
|
||||
last_response = Some(response);
|
||||
}
|
||||
|
||||
let mut response = last_response.ok_or_else(|| PresentationArtifactError::InvalidArgs {
|
||||
action: "presentation_artifact".to_string(),
|
||||
message: "request sequence must contain at least one action".to_string(),
|
||||
})?;
|
||||
if request_count > 1 {
|
||||
let final_summary = response.summary.clone();
|
||||
response.action = "batch".to_string();
|
||||
response.summary =
|
||||
format!("Executed {request_count} actions sequentially. {final_summary}");
|
||||
response.executed_actions = Some(executed_actions);
|
||||
response.exported_paths = exported_paths;
|
||||
}
|
||||
Ok(response)
|
||||
}
|
||||
|
||||
pub fn execute(
|
||||
&mut self,
|
||||
request: PresentationArtifactRequest,
|
||||
@@ -2163,6 +2204,7 @@ impl PresentationArtifactManager {
|
||||
removed.artifact_id,
|
||||
removed.slides.len()
|
||||
),
|
||||
executed_actions: None,
|
||||
exported_paths: Vec::new(),
|
||||
artifact_snapshot: None,
|
||||
slide_list: None,
|
||||
|
||||
@@ -3,6 +3,8 @@ pub struct PresentationArtifactResponse {
|
||||
pub artifact_id: String,
|
||||
pub action: String,
|
||||
pub summary: String,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub executed_actions: Option<Vec<String>>,
|
||||
#[serde(skip_serializing_if = "Vec::is_empty")]
|
||||
pub exported_paths: Vec<PathBuf>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
@@ -38,6 +40,7 @@ impl PresentationArtifactResponse {
|
||||
artifact_id,
|
||||
action,
|
||||
summary,
|
||||
executed_actions: None,
|
||||
exported_paths: Vec::new(),
|
||||
artifact_snapshot: Some(artifact_snapshot),
|
||||
slide_list: None,
|
||||
@@ -63,6 +66,7 @@ fn response_for_document_state(
|
||||
artifact_id,
|
||||
action,
|
||||
summary,
|
||||
executed_actions: None,
|
||||
exported_paths: Vec::new(),
|
||||
artifact_snapshot: document.map(snapshot_for_document),
|
||||
slide_list: None,
|
||||
@@ -132,4 +136,3 @@ pub struct ThemeSnapshot {
|
||||
pub major_font: Option<String>,
|
||||
pub minor_font: Option<String>,
|
||||
}
|
||||
|
||||
|
||||
@@ -258,6 +258,85 @@ fn exported_images_are_real_pictures_with_media_parts() -> Result<(), Box<dyn st
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_request_accepts_sequential_actions() -> Result<(), Box<dyn std::error::Error>> {
|
||||
let request: PresentationArtifactToolRequest = serde_json::from_value(serde_json::json!({
|
||||
"actions": [
|
||||
{
|
||||
"action": "create",
|
||||
"args": { "name": "Batch Deck" }
|
||||
},
|
||||
{
|
||||
"action": "export_pptx",
|
||||
"args": { "path": "deck.pptx" }
|
||||
}
|
||||
]
|
||||
}))?;
|
||||
|
||||
let execution = request.into_execution_request()?;
|
||||
assert_eq!(execution.artifact_id, None);
|
||||
assert_eq!(execution.requests.len(), 2);
|
||||
assert_eq!(execution.requests[0].action, "create");
|
||||
assert_eq!(execution.requests[1].action, "export_pptx");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn manager_can_execute_sequential_actions() -> Result<(), Box<dyn std::error::Error>> {
|
||||
let temp_dir = tempfile::tempdir()?;
|
||||
let mut manager = PresentationArtifactManager::default();
|
||||
let response = manager.execute_requests(
|
||||
PresentationArtifactExecutionRequest {
|
||||
artifact_id: None,
|
||||
requests: vec![
|
||||
PresentationArtifactRequest {
|
||||
artifact_id: None,
|
||||
action: "create".to_string(),
|
||||
args: serde_json::json!({ "name": "Batch Deck" }),
|
||||
},
|
||||
PresentationArtifactRequest {
|
||||
artifact_id: None,
|
||||
action: "add_slide".to_string(),
|
||||
args: serde_json::json!({}),
|
||||
},
|
||||
PresentationArtifactRequest {
|
||||
artifact_id: None,
|
||||
action: "add_text_shape".to_string(),
|
||||
args: serde_json::json!({
|
||||
"slide_index": 0,
|
||||
"text": "hello",
|
||||
"position": { "left": 40, "top": 40, "width": 200, "height": 80 }
|
||||
}),
|
||||
},
|
||||
],
|
||||
},
|
||||
temp_dir.path(),
|
||||
)?;
|
||||
|
||||
assert_eq!(response.action, "batch");
|
||||
assert_eq!(
|
||||
response.executed_actions,
|
||||
Some(vec![
|
||||
"create".to_string(),
|
||||
"add_slide".to_string(),
|
||||
"add_text_shape".to_string(),
|
||||
])
|
||||
);
|
||||
assert_eq!(
|
||||
response
|
||||
.artifact_snapshot
|
||||
.as_ref()
|
||||
.map(|snapshot| snapshot.slide_count),
|
||||
Some(1)
|
||||
);
|
||||
assert!(
|
||||
response
|
||||
.summary
|
||||
.contains("Executed 3 actions sequentially.")
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn imported_pptx_surfaces_image_elements() -> Result<(), Box<dyn std::error::Error>> {
|
||||
let temp_dir = tempfile::tempdir()?;
|
||||
|
||||
Reference in New Issue
Block a user