From 166fc6af61014174d96de129bad181da8d9e3b07 Mon Sep 17 00:00:00 2001 From: oxoxDev <164490987+oxoxDev@users.noreply.github.com> Date: Tue, 2 Jun 2026 18:31:42 +0530 Subject: [PATCH] feat(tools): add generate_presentation tool (native rust engine, ppt-rs) (#2778) (#3016) Co-authored-by: Cyrus Gray --- Cargo.lock | 206 ++++++++- Cargo.toml | 1 + .../agents/orchestrator/agent.toml | 10 + src/openhuman/artifacts/mod.rs | 1 + src/openhuman/artifacts/store.rs | 150 +++++++ src/openhuman/artifacts/store_tests.rs | 1 + src/openhuman/artifacts/types.rs | 8 + src/openhuman/tools/impl/mod.rs | 2 + .../tools/impl/presentation/engine.rs | 405 ++++++++++++++++++ src/openhuman/tools/impl/presentation/mod.rs | 257 +++++++++++ .../tools/impl/presentation/tests.rs | 186 ++++++++ .../tools/impl/presentation/types.rs | 213 +++++++++ src/openhuman/tools/ops.rs | 8 + 13 files changed, 1442 insertions(+), 6 deletions(-) create mode 100644 src/openhuman/tools/impl/presentation/engine.rs create mode 100644 src/openhuman/tools/impl/presentation/mod.rs create mode 100644 src/openhuman/tools/impl/presentation/tests.rs create mode 100644 src/openhuman/tools/impl/presentation/types.rs diff --git a/Cargo.lock b/Cargo.lock index 403354743..82839b7a8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -233,7 +233,7 @@ dependencies = [ "base64ct", "blake2", "cpufeatures 0.2.17", - "password-hash", + "password-hash 0.5.0", ] [[package]] @@ -557,6 +557,15 @@ version = "0.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32637268377fc7b10a8c6d51de3e7fba1ce5dd371a96e342b34e6078db558e7f" +[[package]] +name = "bincode" +version = "1.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b1f45e9417d87227c7a56d22e471c6206462cba514c7590c09aff4cf6d1ddcad" +dependencies = [ + "serde", +] + [[package]] name = "bincode" version = "2.0.1" @@ -597,6 +606,21 @@ dependencies = [ "syn 2.0.117", ] +[[package]] +name = "bit-set" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08807e080ed7f9d5433fa9b275196cfc35414f66a0c79d864dc51a0d825231a3" +dependencies = [ + "bit-vec", +] + +[[package]] +name = "bit-vec" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e764a1d40d510daf35e07be9eb06e75770908c27d411ee6c92109c9840eaaf7" + [[package]] name = "bitcoin" version = "0.32.9" @@ -697,7 +721,7 @@ dependencies = [ "arrayvec", "cc", "cfg-if", - "constant_time_eq", + "constant_time_eq 0.4.2", "cpufeatures 0.3.0", ] @@ -813,6 +837,26 @@ version = "2.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6bd91ee7b2422bcb158d90ef4d14f75ef67f340943fc4149891dcce8f8b972a3" +[[package]] +name = "bzip2" +version = "0.4.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bdb116a6ef3f6c3698828873ad02c3014b3c85cadb88496095628e3ef1e347f8" +dependencies = [ + "bzip2-sys", + "libc", +] + +[[package]] +name = "bzip2-sys" +version = "0.1.13+1.0.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "225bff33b2141874fe80d71e07d6eec4f85c5c216453dd96388240f96e1acc14" +dependencies = [ + "cc", + "pkg-config", +] + [[package]] name = "cbc" version = "0.1.2" @@ -1196,6 +1240,12 @@ dependencies = [ "typewit", ] +[[package]] +name = "constant_time_eq" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "245097e9a4535ee1e3e3931fcfcd55a796a44c643e8596ff6566d68f09b87bbc" + [[package]] name = "constant_time_eq" version = "0.4.2" @@ -2341,6 +2391,17 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7360491ce676a36bf9bb3c56c1aa791658183a54d2744120f27285738d90465a" +[[package]] +name = "fancy-regex" +version = "0.16.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "998b056554fbe42e03ae0e152895cd1a7e1002aec800fdc6635d20270260c46f" +dependencies = [ + "bit-set", + "regex-automata", + "regex-syntax", +] + [[package]] name = "fancy_constructor" version = "2.1.0" @@ -2688,6 +2749,15 @@ dependencies = [ "windows-link", ] +[[package]] +name = "getopts" +version = "0.2.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cfe4fbac503b8d1f88e6676011885f34b7174f46e59956bba534ba83abded4df" +dependencies = [ + "unicode-width", +] + [[package]] name = "getrandom" version = "0.2.17" @@ -5124,6 +5194,7 @@ dependencies = [ "parking_lot", "pdf-extract", "postgres", + "ppt-rs", "prometheus", "prost 0.14.3", "rand 0.10.1", @@ -5181,7 +5252,7 @@ dependencies = [ "wiremock", "x25519-dalek", "xz2", - "zip", + "zip 2.4.2", ] [[package]] @@ -5381,6 +5452,17 @@ dependencies = [ "windows-link", ] +[[package]] +name = "password-hash" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7676374caaee8a325c9e7a2ae557f216c5563a171d6997b0ef8a65af35147700" +dependencies = [ + "base64ct", + "rand_core 0.6.4", + "subtle", +] + [[package]] name = "password-hash" version = "0.5.0" @@ -5399,6 +5481,9 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "83a0692ec44e4cf1ef28ca317f14f8f07da2d95ec3fa01f86e4467b725e60917" dependencies = [ "digest 0.10.7", + "hmac 0.12.1", + "password-hash 0.4.2", + "sha2 0.10.9", ] [[package]] @@ -5734,6 +5819,23 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "439ee305def115ba05938db6eb1644ff94165c5ab5e9420d1c1bcedbba909391" +[[package]] +name = "ppt-rs" +version = "0.2.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eb145020aba8cd682d92b8d00174f4b81fc53feec7919abc15357b9f011c2df5" +dependencies = [ + "chrono", + "clap", + "pulldown-cmark 0.10.3", + "regex", + "syntect", + "thiserror 1.0.69", + "uuid 1.23.1", + "xml-rs", + "zip 0.6.6", +] + [[package]] name = "ppv-lite86" version = "0.2.21" @@ -5913,6 +6015,19 @@ dependencies = [ "prost 0.14.3", ] +[[package]] +name = "pulldown-cmark" +version = "0.10.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "76979bea66e7875e7509c4ec5300112b316af87fa7a252ca91c448b32dfe3993" +dependencies = [ + "bitflags 2.11.1", + "getopts", + "memchr", + "pulldown-cmark-escape 0.10.1", + "unicase", +] + [[package]] name = "pulldown-cmark" version = "0.13.3" @@ -5921,10 +6036,16 @@ checksum = "7c3a14896dfa883796f1cb410461aef38810ea05f2b2c33c5aded3649095fdad" dependencies = [ "bitflags 2.11.1", "memchr", - "pulldown-cmark-escape", + "pulldown-cmark-escape 0.11.0", "unicase", ] +[[package]] +name = "pulldown-cmark-escape" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bd348ff538bc9caeda7ee8cad2d1d48236a1f443c1fa3913c6a02fe0043b1dd3" + [[package]] name = "pulldown-cmark-escape" version = "0.11.0" @@ -6532,7 +6653,7 @@ dependencies = [ "js_int", "js_option", "percent-encoding", - "pulldown-cmark", + "pulldown-cmark 0.13.3", "regex", "ruma-common", "ruma-identifiers-validation", @@ -7590,6 +7711,24 @@ dependencies = [ "syn 2.0.117", ] +[[package]] +name = "syntect" +version = "5.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "656b45c05d95a5704399aeef6bd0ddec7b2b3531b7c9e900abbf7c4d2190c925" +dependencies = [ + "bincode 1.3.3", + "fancy-regex", + "flate2", + "fnv", + "once_cell", + "regex-syntax", + "serde", + "serde_derive", + "thiserror 2.0.18", + "walkdir", +] + [[package]] name = "sysinfo" version = "0.33.1" @@ -9002,7 +9141,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dce56415e95e6f22caf45ea1aec64c67d69b9d6f636a1b20fcc322dff8a787cd" dependencies = [ "async-trait", - "bincode", + "bincode 2.0.1", "diesel", "diesel_migrations", "libsqlite3-sys", @@ -9832,6 +9971,12 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b9cc00251562a284751c9973bace760d86c0276c471b4be569fe6b068ee97a56" +[[package]] +name = "xml-rs" +version = "0.8.28" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3ae8337f8a065cfc972643663ea4279e04e7256de865aa66fe25cec5fb912d3f" + [[package]] name = "xxhash-rust" version = "0.8.15" @@ -10010,6 +10155,26 @@ dependencies = [ "syn 2.0.117", ] +[[package]] +name = "zip" +version = "0.6.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "760394e246e4c28189f19d488c058bf16f564016aefac5d32bb1f3b51d5e9261" +dependencies = [ + "aes", + "byteorder", + "bzip2", + "constant_time_eq 0.1.5", + "crc32fast", + "crossbeam-utils", + "flate2", + "hmac 0.12.1", + "pbkdf2 0.11.0", + "sha1", + "time", + "zstd", +] + [[package]] name = "zip" version = "2.4.2" @@ -10051,6 +10216,35 @@ dependencies = [ "simd-adler32", ] +[[package]] +name = "zstd" +version = "0.11.2+zstd.1.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "20cc960326ece64f010d2d2107537f26dc589a6573a316bd5b1dba685fa5fde4" +dependencies = [ + "zstd-safe", +] + +[[package]] +name = "zstd-safe" +version = "5.0.2+zstd.1.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1d2a5585e04f9eea4b2a3d1eca508c4dee9592a89ef6f450c11719da0726f4db" +dependencies = [ + "libc", + "zstd-sys", +] + +[[package]] +name = "zstd-sys" +version = "2.0.16+zstd.1.5.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "91e19ebc2adc8f83e43039e79776e3fda8ca919132d68a1fed6a5faca2683748" +dependencies = [ + "cc", + "pkg-config", +] + [[package]] name = "zune-core" version = "0.5.1" diff --git a/Cargo.toml b/Cargo.toml index 43b054015..c15afd907 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -197,6 +197,7 @@ whatsapp-rust = { version = "0.5", optional = true, default-features = false, fe whatsapp-rust-tokio-transport = { version = "0.5", optional = true, default-features = false } whatsapp-rust-ureq-http-client = { version = "0.5", optional = true } wacore = { version = "0.5", optional = true, default-features = false } +ppt-rs = "0.2.14" [target.'cfg(windows)'.dependencies] # Windows: tokio-tungstenite uses native-tls (schannel) so wss:// diff --git a/src/openhuman/agent_registry/agents/orchestrator/agent.toml b/src/openhuman/agent_registry/agents/orchestrator/agent.toml index 1680ad60a..595e8bf0b 100644 --- a/src/openhuman/agent_registry/agents/orchestrator/agent.toml +++ b/src/openhuman/agent_registry/agents/orchestrator/agent.toml @@ -160,4 +160,14 @@ named = [ # Both are routed through the same security/approval gate as `shell`. "workflow_load", "workflow_phase", + # Presentation generation (#2778). Synthesises a .pptx from a + # structured slide spec via a native Rust engine (`ppt-rs`) running + # in-process — no Python subprocess, no managed venv. Output lands + # in the workspace artifacts directory and the tool returns the + # artifact id + absolute path so the orchestrator can quote it in + # the reply. Full agent-definition wiring + per-tier tool_filter + # policy lands in #2780; this entry exists so the orchestrator can + # drive the tool today (matches the user-facing "create slides" + # routing case the parent issue #1535 asks for). + "generate_presentation", ] diff --git a/src/openhuman/artifacts/mod.rs b/src/openhuman/artifacts/mod.rs index 7131ed2e7..21fa2e3ed 100644 --- a/src/openhuman/artifacts/mod.rs +++ b/src/openhuman/artifacts/mod.rs @@ -8,4 +8,5 @@ pub use schemas::{ all_controller_schemas as all_artifacts_controller_schemas, all_registered_controllers as all_artifacts_registered_controllers, }; +pub use store::{create_artifact, fail_artifact, finalize_artifact}; pub use types::{ArtifactKind, ArtifactMeta, ArtifactStatus}; diff --git a/src/openhuman/artifacts/store.rs b/src/openhuman/artifacts/store.rs index 10245c48f..3d21ca0ef 100644 --- a/src/openhuman/artifacts/store.rs +++ b/src/openhuman/artifacts/store.rs @@ -233,6 +233,156 @@ pub(crate) async fn delete_artifact(workspace_dir: &Path, artifact_id: &str) -> #[allow(dead_code)] fn _assert_status_used(_: ArtifactStatus) {} +/// Maximum length of a sanitized artifact filename stem. Keeps the +/// rendered filename short enough to round-trip on every filesystem +/// (Windows MAX_PATH, ext4 NAME_MAX) without truncating the +/// `.extension` suffix or the UUID-named parent directory. +const MAX_SANITIZED_FILENAME_LEN: usize = 80; + +/// Convert a human-readable title into a filesystem-safe filename +/// stem. Strips path-traversal characters, collapses whitespace to +/// single dashes, lowercases, and caps the length. Falls back to +/// `"artifact"` when the resulting stem is empty (e.g. title was +/// `"///"` or only emoji that survive ASCII-only sanitisation). +fn sanitize_filename_stem(title: &str) -> String { + let mut out = String::with_capacity(title.len().min(MAX_SANITIZED_FILENAME_LEN)); + let mut prev_dash = false; + for ch in title.chars() { + let mapped = match ch { + 'a'..='z' | 'A'..='Z' | '0'..='9' | '-' | '_' => ch.to_ascii_lowercase(), + ' ' | '\t' | '\n' | '\r' | '.' | '/' | '\\' | ':' => '-', + _ => continue, + }; + if mapped == '-' { + if prev_dash { + continue; + } + prev_dash = true; + } else { + prev_dash = false; + } + out.push(mapped); + if out.chars().count() >= MAX_SANITIZED_FILENAME_LEN { + break; + } + } + let trimmed = out.trim_matches('-').to_string(); + if trimmed.is_empty() { + "artifact".to_string() + } else { + trimmed + } +} + +/// Allocate a fresh artifact directory and persist a pending +/// [`ArtifactMeta`] record. Returns the metadata plus the absolute +/// path where the producer should write the artifact bytes. +/// +/// On success the caller MUST follow up with [`finalize_artifact`] +/// once the bytes are on disk (or [`fail_artifact`] if generation +/// failed) so the status flips off `Pending`. Leaving a record in +/// `Pending` is harmless — the list RPC will still surface it — but +/// downstream consumers (UI, download endpoints) treat `Pending` as +/// "not yet ready", so a stuck record means a stuck spinner. +/// +/// `extension` is the file extension WITHOUT the leading dot +/// (e.g. `"pptx"`, `"pdf"`). Used to build the rendered filename +/// under the artifact directory. +pub async fn create_artifact( + workspace_dir: &Path, + kind: super::types::ArtifactKind, + title: &str, + extension: &str, +) -> Result<(ArtifactMeta, PathBuf), String> { + let trimmed_title = title.trim(); + if trimmed_title.is_empty() { + return Err("[artifacts] create_artifact: title must not be empty".to_string()); + } + let trimmed_ext = extension.trim(); + if trimmed_ext.is_empty() { + return Err("[artifacts] create_artifact: extension must not be empty".to_string()); + } + if trimmed_ext.contains('/') || trimmed_ext.contains('\\') || trimmed_ext.contains('.') { + return Err(format!( + "[artifacts] create_artifact: extension must not contain '/', '\\', or '.': {trimmed_ext:?}" + )); + } + + let id = uuid::Uuid::new_v4().to_string(); + let filename = format!("{}.{trimmed_ext}", sanitize_filename_stem(trimmed_title)); + let relative_path = format!("{id}/{filename}"); + + let root = artifacts_root(workspace_dir).await?; + let artifact_dir = root.join(&id); + assert_within_root(&root, &artifact_dir)?; + tokio::fs::create_dir_all(&artifact_dir) + .await + .map_err(|e| { + format!( + "[artifacts] create_artifact: failed to mkdir {:?}: {e}", + artifact_dir + ) + })?; + let absolute_path = artifact_dir.join(&filename); + + let meta = ArtifactMeta { + id: id.clone(), + kind, + title: trimmed_title.to_string(), + path: relative_path, + size_bytes: 0, + status: ArtifactStatus::Pending, + created_at: chrono::Utc::now(), + error: None, + }; + save_artifact_meta(workspace_dir, &meta).await?; + + log::debug!( + "[artifacts] create_artifact: id={id} kind={} path={:?}", + meta.kind.as_str(), + absolute_path + ); + Ok((meta, absolute_path)) +} + +/// Flip a pending artifact to [`ArtifactStatus::Ready`] and persist +/// the final size. Idempotent on already-ready artifacts (no-op + log). +/// Returns the updated metadata. +pub async fn finalize_artifact( + workspace_dir: &Path, + artifact_id: &str, + size_bytes: u64, +) -> Result { + let mut meta = get_artifact(workspace_dir, artifact_id).await?; + if matches!(meta.status, ArtifactStatus::Ready) && meta.size_bytes == size_bytes { + log::debug!("[artifacts] finalize_artifact: id={artifact_id} already Ready, no-op"); + return Ok(meta); + } + meta.status = ArtifactStatus::Ready; + meta.size_bytes = size_bytes; + meta.error = None; + save_artifact_meta(workspace_dir, &meta).await?; + log::debug!("[artifacts] finalize_artifact: id={artifact_id} -> Ready size={size_bytes}"); + Ok(meta) +} + +/// Flip an artifact to [`ArtifactStatus::Failed`] and persist a +/// failure reason. The producer should call this when generation +/// fails so the UI / RPC consumer can surface a useful message +/// instead of an indefinite spinner. Returns the updated metadata. +pub async fn fail_artifact( + workspace_dir: &Path, + artifact_id: &str, + reason: &str, +) -> Result { + let mut meta = get_artifact(workspace_dir, artifact_id).await?; + meta.status = ArtifactStatus::Failed; + meta.error = Some(reason.to_string()); + save_artifact_meta(workspace_dir, &meta).await?; + log::warn!("[artifacts] fail_artifact: id={artifact_id} -> Failed reason={reason:?}"); + Ok(meta) +} + #[cfg(test)] #[path = "store_tests.rs"] mod tests; diff --git a/src/openhuman/artifacts/store_tests.rs b/src/openhuman/artifacts/store_tests.rs index 7c017ffda..48452acc5 100644 --- a/src/openhuman/artifacts/store_tests.rs +++ b/src/openhuman/artifacts/store_tests.rs @@ -13,6 +13,7 @@ fn make_meta(id: &str, title: &str, created_at: chrono::DateTime) -> Artifa size_bytes: 100, status: ArtifactStatus::Ready, created_at, + error: None, } } diff --git a/src/openhuman/artifacts/types.rs b/src/openhuman/artifacts/types.rs index 33d2c6972..7d89e22f8 100644 --- a/src/openhuman/artifacts/types.rs +++ b/src/openhuman/artifacts/types.rs @@ -93,6 +93,12 @@ pub struct ArtifactMeta { pub status: ArtifactStatus, /// UTC timestamp when this artifact was created. pub created_at: DateTime, + /// Failure reason set when [`ArtifactStatus::Failed`]; `None` + /// otherwise. Persisted so list/get RPCs can surface why a build + /// did not produce a usable file without callers having to scrape + /// stderr from a separate log. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub error: Option, } #[cfg(test)] @@ -207,6 +213,7 @@ mod tests { size_bytes: 204800, status: ArtifactStatus::Ready, created_at: Utc.with_ymd_and_hms(2025, 6, 1, 12, 0, 0).unwrap(), + error: None, }; let json = serde_json::to_value(&meta).unwrap(); assert_eq!(json["id"], "abc-123"); @@ -229,6 +236,7 @@ mod tests { size_bytes: 0, status: ArtifactStatus::Pending, created_at: Utc.with_ymd_and_hms(2025, 1, 1, 0, 0, 0).unwrap(), + error: None, }; let v = serde_json::to_value(&meta).unwrap(); // Verify all expected fields are present diff --git a/src/openhuman/tools/impl/mod.rs b/src/openhuman/tools/impl/mod.rs index 1d2e8dc0b..2ede3cc3a 100644 --- a/src/openhuman/tools/impl/mod.rs +++ b/src/openhuman/tools/impl/mod.rs @@ -2,10 +2,12 @@ pub mod browser; pub mod computer; pub mod filesystem; pub mod network; +pub mod presentation; pub mod system; pub use browser::*; pub use computer::*; pub use filesystem::*; pub use network::*; +pub use presentation::PresentationTool; pub use system::*; diff --git a/src/openhuman/tools/impl/presentation/engine.rs b/src/openhuman/tools/impl/presentation/engine.rs new file mode 100644 index 000000000..47f8e8695 --- /dev/null +++ b/src/openhuman/tools/impl/presentation/engine.rs @@ -0,0 +1,405 @@ +//! Native Rust `.pptx` generator (replaces the python-pptx subprocess +//! path shipped in #2778). +//! +//! Backed by the [`ppt-rs`](https://crates.io/crates/ppt-rs) crate +//! (Apache-2.0). Pure CPU, no subprocess, no managed runtime, no +//! first-call venv-setup latency. Output is a byte buffer the caller +//! writes to the artifact's `output_path`. +//! +//! ## Mapping `SlideSpec` → `ppt-rs` +//! +//! `ppt-rs::SlideContent` does not expose a separate "body paragraph" +//! slot today; everything below the title is a bullet. We collapse +//! [`SlideSpec::body`] into a leading bullet so the body text still +//! reaches the rendered slide: +//! +//! ```text +//! SlideSpec { title, body: Some(body), bullets: [b1, b2], speaker_notes: Some(n) } +//! → SlideContent::new(title).add_bullet(body).add_bullet(b1).add_bullet(b2).notes(n) +//! ``` +//! +//! Empty / whitespace-only entries are filtered out so a trailing +//! blank `body` does not produce an empty bullet marker. +//! +//! ## Title slide +//! +//! `ppt-rs::create_pptx_with_content(title, slides)` treats `title` +//! as deck metadata only (lands in `docProps/core.xml`) — it does NOT +//! emit a separate title-slide. To preserve the python-pptx +//! contract — title slide first, content slides after, with +//! [`GeneratePresentationOutput::slide_count`] excluding the title +//! slide — we prepend a synthetic title slide built from +//! [`GeneratePresentationInput::title`] (+ optional `author` byline). +//! +//! ## Runtime +//! +//! `ppt-rs::create_pptx_with_content` is synchronous, CPU-bound, and +//! typically completes in <100 ms even for the 64-slide cap. We still +//! drive it through `spawn_blocking` so the async executor is not +//! blocked, and wrap the whole call in a `tokio::time::timeout` so a +//! runaway generation cannot wedge the agent loop. + +use std::time::Duration; + +use ppt_rs::generator::{create_pptx_with_content, SlideContent}; +use tokio::task::JoinError; +use tokio::time::{error::Elapsed, timeout}; + +use super::types::{GeneratePresentationInput, PresentationError}; + +/// Run the synthesis. Returns the serialised `.pptx` bytes ready to +/// be written to the artifact path. +/// +/// The `deadline` covers the entire blocking call (including the +/// `spawn_blocking` thread acquisition). Hitting it surfaces as +/// [`PresentationError::GenerationTimeout`]. +pub(super) async fn generate( + input: &GeneratePresentationInput, + deadline: Duration, +) -> Result, PresentationError> { + // Build the SlideContent vector on the async thread — cheap allocation + // work, no need to send the original `input` across the blocking + // boundary as a borrow. + let slides = build_slides(input); + let deck_title = input.title.clone(); + let started = std::time::Instant::now(); + let slide_count = slides.len(); + let deadline_secs = deadline.as_secs(); + let title_chars = deck_title.chars().count(); + + tracing::debug!( + target: "presentation", + deadline_secs, + slide_count, + title_chars, + "[presentation:engine] generate:start" + ); + + let join: Result, EngineFailure>, _>, Elapsed> = timeout( + deadline, + tokio::task::spawn_blocking(move || generate_blocking(&deck_title, slides)), + ) + .await; + + let elapsed_ms = started.elapsed().as_millis() as u64; + match join { + Err(_elapsed) => { + tracing::warn!( + target: "presentation", + elapsed_ms, + deadline_secs, + slide_count, + "[presentation:engine] generate:timeout" + ); + Err(PresentationError::GenerationTimeout { + timeout_secs: deadline_secs, + }) + } + Ok(Err(join_err)) => { + let err = map_join_error(join_err); + tracing::warn!( + target: "presentation", + elapsed_ms, + kind = "join_error", + err = %err, + "[presentation:engine] generate:failure" + ); + Err(err) + } + Ok(Ok(Err(engine_err))) => { + let err = map_engine_failure(engine_err); + tracing::warn!( + target: "presentation", + elapsed_ms, + kind = "engine_failure", + err = %err, + "[presentation:engine] generate:failure" + ); + Err(err) + } + Ok(Ok(Ok(bytes))) => { + tracing::debug!( + target: "presentation", + elapsed_ms, + bytes = bytes.len(), + slide_count, + "[presentation:engine] generate:done" + ); + Ok(bytes) + } + } +} + +/// Pure transformation from our schema to `ppt-rs`'s. Pulled out of +/// `generate` for unit-testability — the slide ordering + empty +/// filtering rules are load-bearing for the rendered deck shape. +fn build_slides(input: &GeneratePresentationInput) -> Vec { + let mut out = Vec::with_capacity(input.slides.len() + 1); + + // Synthetic title slide — preserves the python-pptx behaviour where + // the first rendered slide carries the deck title (+ optional author + // byline). Without this prepend, the deck would open straight onto + // the first content slide and the `title` argument would only land + // in core.xml metadata. + let mut title_slide = SlideContent::new(&input.title); + if let Some(author) = input.author.as_deref().filter(|a| !a.trim().is_empty()) { + title_slide = title_slide.add_bullet(author); + } + out.push(title_slide); + + for spec in &input.slides { + let mut slide = SlideContent::new(&spec.title); + if let Some(body) = spec.body.as_deref().filter(|b| !b.trim().is_empty()) { + slide = slide.add_bullet(body); + } + for bullet in &spec.bullets { + if !bullet.trim().is_empty() { + slide = slide.add_bullet(bullet); + } + } + if let Some(notes) = spec + .speaker_notes + .as_deref() + .filter(|n| !n.trim().is_empty()) + { + slide = slide.notes(notes); + } + out.push(slide); + } + + out +} + +/// Blocking inner — runs on the `spawn_blocking` pool. Returns a +/// dedicated `EngineFailure` so the async wrapper can distinguish +/// "library returned an error" from "the blocking task itself panicked +/// or was cancelled". +fn generate_blocking( + deck_title: &str, + slides: Vec, +) -> Result, EngineFailure> { + create_pptx_with_content(deck_title, slides) + .map_err(|err| EngineFailure::Library(format!("{err}"))) +} + +/// Internal failure shape used to keep the blocking-thread surface +/// `Send`-clean (the `ppt-rs` error type is not guaranteed to be +/// `Send + Sync + 'static`). +#[derive(Debug)] +enum EngineFailure { + Library(String), +} + +fn map_engine_failure(failure: EngineFailure) -> PresentationError { + match failure { + EngineFailure::Library(msg) => PresentationError::GenerationFailed { + exit_code: -1, + stderr_truncated: PresentationError::truncate_stderr(&msg), + }, + } +} + +fn map_join_error(err: JoinError) -> PresentationError { + // A bare panic indicates a `ppt-rs` bug or an OOM on the blocking + // pool; surface as `GenerationFailed` so the user sees a structured + // error and the agent can retry with a smaller deck. + // + // Cancellation (non-panic `JoinError`) is a distinct shape: the + // outer `tokio::time::timeout` already routes the timeout case + // before us, so a cancellation that reaches `map_join_error` is + // something else — runtime shutdown, an explicit abort, or the + // runtime cancelling the blocking task for unrelated reasons. + // Reporting it as `GenerationTimeout { timeout_secs: 0 }` produced + // a misleading "exceeded 0s timeout" message and discarded the + // underlying `JoinError` detail that's valuable for triage. We + // surface it as `GenerationFailed` and preserve the cancellation + // context in `stderr_truncated`. + if err.is_panic() { + PresentationError::GenerationFailed { + exit_code: -1, + stderr_truncated: PresentationError::truncate_stderr("presentation engine panicked"), + } + } else { + PresentationError::GenerationFailed { + exit_code: -1, + stderr_truncated: PresentationError::truncate_stderr(&format!( + "presentation engine task cancelled: {err}" + )), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::openhuman::tools::implementations::presentation::types::SlideSpec; + + fn input_with_one_slide() -> GeneratePresentationInput { + GeneratePresentationInput { + title: "Quarterly review".to_string(), + author: Some("Alice".to_string()), + theme: None, + slides: vec![SlideSpec { + title: "Highlights".to_string(), + body: Some("Revenue up 12% QoQ.".to_string()), + bullets: vec![ + "Closed two key deals".to_string(), + "Hired 3 engineers".to_string(), + ], + speaker_notes: Some("Emphasise headcount efficiency.".to_string()), + }], + } + } + + #[test] + fn build_slides_prepends_title_slide_with_author_byline() { + let slides = build_slides(&input_with_one_slide()); + // Title slide + 1 content slide. + assert_eq!(slides.len(), 2); + // ppt-rs SlideContent fields are pub but private to this crate + // boundary; downstream `create_pptx_with_content` is the only + // semantically meaningful assertion — covered by the + // `generate_round_trips_to_valid_pptx` test below. Here we only + // assert the *count* invariant (title-slide prepended), since + // the public API of SlideContent does not expose its bullets. + } + + #[test] + fn build_slides_drops_blank_body_and_bullet_entries() { + let mut input = input_with_one_slide(); + input.author = Some(" ".to_string()); + input.slides[0].body = Some("".to_string()); + input.slides[0].bullets = vec!["real".to_string(), " ".to_string(), "".to_string()]; + input.slides[0].speaker_notes = Some("\n\t ".to_string()); + + let slides = build_slides(&input); + // 2 slides regardless — empty filtering happens INSIDE the slide, + // not at the slide-list level. Behaviour assertion: the call + // does not panic on whitespace-only fields and downstream + // ppt-rs generation succeeds (cross-checked by the round-trip + // test below). + assert_eq!(slides.len(), 2); + } + + #[tokio::test] + async fn generate_round_trips_to_valid_pptx() { + // End-to-end: build → ppt-rs → byte buffer → re-open as zip → + // confirm OOXML skeleton entries. This is the load-bearing + // assertion that the engine swap produces a deck that any + // OOXML reader (PowerPoint, Keynote, LibreOffice, Google + // Slides) can open. + let input = input_with_one_slide(); + let bytes = generate(&input, Duration::from_secs(30)) + .await + .expect("generate should succeed on a 1-slide deck"); + + assert!( + bytes.len() > 1000, + "deck unexpectedly small ({} bytes)", + bytes.len() + ); + + let cursor = std::io::Cursor::new(&bytes); + let mut zip = zip::ZipArchive::new(cursor).expect("output is a valid zip archive"); + + let names: Vec = (0..zip.len()) + .map(|i| zip.by_index(i).unwrap().name().to_string()) + .collect(); + + // OOXML spec-required entries — without these PowerPoint will + // refuse to open the file with "PowerPoint found a problem". + for required in [ + "[Content_Types].xml", + "_rels/.rels", + "ppt/presentation.xml", + "ppt/_rels/presentation.xml.rels", + "ppt/theme/theme1.xml", + "ppt/slideMasters/slideMaster1.xml", + "ppt/slideLayouts/slideLayout1.xml", + "docProps/core.xml", + "docProps/app.xml", + ] { + assert!( + names.iter().any(|n| n == required), + "missing OOXML entry: {required} (got: {names:?})" + ); + } + + // Title slide (slide1) + 1 content slide (slide2) = 2. + assert!(names.iter().any(|n| n == "ppt/slides/slide1.xml")); + assert!(names.iter().any(|n| n == "ppt/slides/slide2.xml")); + // No slide3 — we only had one SlideSpec. + assert!(!names.iter().any(|n| n == "ppt/slides/slide3.xml")); + + // Speaker notes were set on the content slide → notesSlide + // must materialise. Without this, the notes pane in PowerPoint + // / Keynote stays empty even though the agent populated it. + assert!(names + .iter() + .any(|n| n.starts_with("ppt/notesSlides/notesSlide"))); + + // Sanity: the title text shows up somewhere in the generated + // slide XML. We do not assert exact placement (the placeholder + // structure is owned by ppt-rs's slide layout) — only that the + // string was not dropped on the floor. + let mut slide1 = zip.by_name("ppt/slides/slide1.xml").unwrap(); + let mut slide1_body = String::new(); + std::io::Read::read_to_string(&mut slide1, &mut slide1_body).unwrap(); + assert!( + slide1_body.contains("Quarterly review"), + "deck title missing from rendered slide1.xml" + ); + } + + #[tokio::test] + async fn map_join_error_cancellation_becomes_generation_failed() { + // A non-panic JoinError (cancellation via abort) MUST NOT surface + // as GenerationTimeout { timeout_secs: 0 } — that produces a + // misleading "exceeded 0s timeout" message and loses the + // JoinError detail useful for triage. Cancellation belongs in + // GenerationFailed with the cancellation context preserved. + let handle = tokio::spawn(async { + // Park forever; we abort before this returns. + tokio::time::sleep(Duration::from_secs(3600)).await; + }); + handle.abort(); + let join_err = handle.await.expect_err("aborted task yields JoinError"); + assert!( + !join_err.is_panic(), + "abort() should produce a cancellation JoinError, not a panic" + ); + + match map_join_error(join_err) { + PresentationError::GenerationFailed { + exit_code, + stderr_truncated, + } => { + assert_eq!(exit_code, -1, "cancellation maps to exit_code -1"); + assert!( + stderr_truncated.contains("presentation engine task cancelled"), + "cancellation context missing from stderr_truncated: {stderr_truncated:?}" + ); + } + other => panic!("expected GenerationFailed for cancellation, got {other:?}"), + } + } + + #[tokio::test] + async fn generate_surfaces_timeout_under_tiny_deadline() { + // A 1 ns deadline cannot complete any real work — we expect a + // structured Timeout, not a panic or a half-written buffer. + let input = input_with_one_slide(); + let err = generate(&input, Duration::from_nanos(1)) + .await + .expect_err("1 ns deadline should never satisfy the timeout"); + match err { + PresentationError::GenerationTimeout { timeout_secs } => { + assert_eq!( + timeout_secs, 0, + "nanosecond timeout rounds down to 0 seconds" + ); + } + other => panic!("expected GenerationTimeout, got {other:?}"), + } + } +} diff --git a/src/openhuman/tools/impl/presentation/mod.rs b/src/openhuman/tools/impl/presentation/mod.rs new file mode 100644 index 000000000..6fc99c29b --- /dev/null +++ b/src/openhuman/tools/impl/presentation/mod.rs @@ -0,0 +1,257 @@ +//! Tool: `generate_presentation` — build a `.pptx` deck from a +//! structured slide spec via the native-Rust [`engine`] module. +//! +//! Flow: +//! 1. Validate the JSON-Schema input early (`types::validate_input`) +//! so the agent gets a structured `InvalidInput` it can self-correct +//! on instead of a low-level error. +//! 2. Allocate an artifact dir via `artifacts::create_artifact`. The +//! returned `meta` starts at `ArtifactStatus::Pending` so an +//! interrupted run never surfaces as Ready. +//! 3. Generate the deck bytes via [`engine::generate`] — pure Rust, +//! `ppt-rs`-backed, no Python runtime, no subprocess. Wrapped in +//! `spawn_blocking` + `tokio::time::timeout` so the synchronous +//! library work neither blocks the async executor nor can wedge +//! the agent loop. +//! 4. Write the bytes to the artifact's output path, stat for size, +//! flip artifact to `Ready` via `artifacts::finalize_artifact`, +//! return the artifact id + path. +//! 5. On failure: flip artifact to `Failed` via +//! `artifacts::fail_artifact` so the UI can surface the reason. +//! +//! Originally shipped in #2778 against a managed python-pptx venv; +//! refactored to a native-Rust engine in #2780-follow-up to drop the +//! Python runtime + first-call venv-install latency + 50 MB+ Python +//! disk footprint. Tool name / input schema / output schema / artifact +//! layout are byte-identical across the swap so #3017 ArtifactCard, +//! #3026 Files panel, and the orchestrator grounding rule in #3029 +//! continue to work without change. + +use std::path::PathBuf; +use std::time::Duration; + +use async_trait::async_trait; +use serde_json::{json, Value}; + +use crate::openhuman::artifacts::{ + create_artifact, fail_artifact, finalize_artifact, ArtifactKind, +}; +use crate::openhuman::tools::traits::{PermissionLevel, Tool, ToolResult}; + +mod engine; +mod types; + +#[cfg(test)] +#[path = "tests.rs"] +mod tests; + +use self::types::{ + validate_input, GeneratePresentationInput, GeneratePresentationOutput, PresentationError, +}; + +/// Generation timeout. `ppt-rs` typically completes the full 64-slide +/// cap in well under a second; the 30 s ceiling is a defensive bound +/// against pathological inputs slipping past `validate_input` and +/// the worst-case `spawn_blocking` thread-acquisition latency on a +/// saturated runtime. +const GENERATION_TIMEOUT: Duration = Duration::from_secs(30); + +/// Tool name surfaced to the agent. Stable; do not rename without +/// coordinating with the orchestrator agent definition list. +pub const TOOL_NAME: &str = "generate_presentation"; + +/// One-shot `.pptx` generator. See module docs for the request flow. +pub struct PresentationTool { + workspace_dir: PathBuf, +} + +impl PresentationTool { + /// Production constructor. The engine is stateless — no runtime + /// resolution, venv setup, or cache directory needed. Pass the + /// workspace directory the artifact pipeline writes into. + pub fn new(workspace_dir: PathBuf) -> Self { + Self { workspace_dir } + } +} + +#[async_trait] +impl Tool for PresentationTool { + fn name(&self) -> &str { + TOOL_NAME + } + + fn description(&self) -> &str { + // Router-rule format per the existing tool conventions (see + // `current_time.rs` etc.): tell the orchestrator when to use + // this tool and when NOT to. + "Generate a PowerPoint (.pptx) presentation from a structured slide spec. \ + USE THIS when the user asks for slides, a deck, a presentation, or a \ + slide-by-slide breakdown of a topic. Provide `title` plus a `slides` \ + array of `{title, body?, bullets?, speaker_notes?}` objects. NOT for: \ + per-slide image generation, live editing of existing decks, or non-PPT \ + formats (PDF, Keynote, Google Slides exports). The generated file is \ + persisted as an artifact in the workspace and the tool returns the \ + artifact id + absolute path so the agent can reference it in the reply." + } + + fn parameters_schema(&self) -> Value { + json!({ + "type": "object", + "additionalProperties": false, + "required": ["title", "slides"], + "properties": { + "title": { + "type": "string", + "description": "Deck title. Surfaced on the title slide and used as the artifact's human-readable name. Required, non-empty.", + "maxLength": types::MAX_TEXT_CHARS, + }, + "author": { + "type": "string", + "description": "Optional author byline shown on the title slide.", + "maxLength": types::MAX_TEXT_CHARS, + }, + "theme": { + "type": "string", + "description": "Reserved for future template-selection work. Currently informational only.", + "maxLength": types::MAX_TEXT_CHARS, + }, + "slides": { + "type": "array", + "minItems": 1, + "maxItems": types::MAX_SLIDES, + "description": "Slide specs in display order. At least one entry required; hard cap to bound generation time + output size.", + "items": { + "type": "object", + "additionalProperties": false, + "properties": { + "title": { "type": "string", "maxLength": types::MAX_TEXT_CHARS }, + "body": { "type": "string", "maxLength": types::MAX_TEXT_CHARS }, + "bullets": { + "type": "array", + "maxItems": types::MAX_BULLETS_PER_SLIDE, + "items": { "type": "string", "maxLength": types::MAX_TEXT_CHARS } + }, + "speaker_notes": { "type": "string", "maxLength": types::MAX_TEXT_CHARS } + } + } + } + } + }) + } + + fn permission_level(&self) -> PermissionLevel { + // We write files to the workspace artifacts dir. Treat as + // Write rather than ReadOnly. No subprocess / network reach. + PermissionLevel::Write + } + + fn supports_markdown(&self) -> bool { + true + } + + async fn execute(&self, args: Value) -> anyhow::Result { + let input: GeneratePresentationInput = match serde_json::from_value(args.clone()) { + Ok(v) => v, + Err(err) => { + let msg = format!("invalid generate_presentation arguments: {err}"); + tracing::warn!(target: "presentation", err = %err, "[presentation] deserialisation failed"); + return Ok(ToolResult::error(msg)); + } + }; + + if let Err(err) = validate_input(&input) { + tracing::debug!(target: "presentation", err = %err, "[presentation] validation rejected input"); + return Ok(ToolResult::error(err.to_string())); + } + + tracing::info!( + target: "presentation", + title_chars = input.title.chars().count(), + has_author = input.author.is_some(), + slide_count = input.slides.len(), + "[presentation] generation request accepted" + ); + + let (meta, output_path) = create_artifact( + &self.workspace_dir, + ArtifactKind::Presentation, + &input.title, + "pptx", + ) + .await + .map_err(anyhow::Error::msg)?; + + let bytes = match engine::generate(&input, GENERATION_TIMEOUT).await { + Ok(bytes) => bytes, + Err(err) => { + let _ = fail_artifact(&self.workspace_dir, &meta.id, &err.to_string()).await; + tracing::warn!( + target: "presentation", + err = %err, + "[presentation] engine generation failed" + ); + return Ok(ToolResult::error(err.to_string())); + } + }; + + if let Err(err) = tokio::fs::write(&output_path, &bytes).await { + let filename = output_path + .file_name() + .map(|n| n.to_string_lossy().into_owned()) + .unwrap_or_default(); + let reason = format!("failed to write generated deck ({filename}): {err}"); + let _ = fail_artifact(&self.workspace_dir, &meta.id, &reason).await; + tracing::warn!( + target: "presentation", + err = %err, + artifact_id = %meta.id, + filename = %filename, + "[presentation] artifact file write failed" + ); + return Ok(ToolResult::error(reason)); + } + + let size_bytes = bytes.len() as u64; + let updated = match finalize_artifact(&self.workspace_dir, &meta.id, size_bytes).await { + Ok(updated) => updated, + Err(err) => { + let reason = format!("failed to finalize artifact: {err}"); + // File is already on disk but the ledger transition failed. + // Flip the artifact to Failed so the UI surfaces the error + // instead of leaving it stuck in `Pending`. Fail-artifact + // errors are swallowed — they can only happen if the same + // ledger backend is unavailable, in which case nothing we + // do here will help. + let _ = fail_artifact(&self.workspace_dir, &meta.id, &reason).await; + tracing::warn!( + target: "presentation", + err = %err, + artifact_id = %meta.id, + "[presentation] finalize_artifact failed; flipped to Failed" + ); + return Ok(ToolResult::error(reason)); + } + }; + + tracing::info!( + target: "presentation", + artifact_id = %updated.id, + size_bytes, + slide_count = input.slides.len(), + "[presentation] generation complete" + ); + + let out = GeneratePresentationOutput { + artifact_id: updated.id.clone(), + artifact_path: output_path.display().to_string(), + slide_count: input.slides.len(), + size_bytes, + }; + let payload = serde_json::to_value(&out)?; + let markdown = format!( + "Generated {}-slide presentation at `{}` (artifact `{}`, {} bytes).", + out.slide_count, out.artifact_path, out.artifact_id, out.size_bytes + ); + Ok(ToolResult::success_with_markdown(payload, markdown)) + } +} diff --git a/src/openhuman/tools/impl/presentation/tests.rs b/src/openhuman/tools/impl/presentation/tests.rs new file mode 100644 index 000000000..a8451f9c7 --- /dev/null +++ b/src/openhuman/tools/impl/presentation/tests.rs @@ -0,0 +1,186 @@ +//! Unit tests for the `generate_presentation` tool. +//! +//! The engine layer (`engine.rs`) ships its own focused tests covering +//! the `SlideSpec` → `ppt-rs` mapping, OOXML round-trip, and timeout +//! handling. The tests here cover the tool-level concerns: input +//! validation rejection branches, the parameters schema contract, the +//! `description` router rules, the artifact-pipeline glue, and the +//! happy-path output shape (artifact id + path + slide count + size). +//! +//! No mocks or interpreters — the real engine runs every test, so the +//! happy-path assertion doubles as a contract check that the engine +//! swap continues to produce a valid `.pptx` from this tool's +//! perspective. + +use super::types::{PresentationError, MAX_BULLETS_PER_SLIDE, MAX_SLIDES, MAX_TEXT_CHARS}; +use super::*; + +use std::path::PathBuf; + +fn workspace() -> tempfile::TempDir { + tempfile::tempdir().expect("create temp workspace") +} + +fn minimal_input_json() -> serde_json::Value { + json!({ + "title": "Quarterly Review", + "slides": [ + { "title": "Highlights", "bullets": ["Up and to the right"] } + ] + }) +} + +#[test] +fn parameters_schema_shape_matches_contract() { + let tool = PresentationTool::new(PathBuf::from("/tmp/never-read")); + let schema = tool.parameters_schema(); + assert_eq!(schema["type"], "object"); + let required = schema["required"].as_array().expect("required is array"); + assert!(required.iter().any(|v| v.as_str() == Some("title"))); + assert!(required.iter().any(|v| v.as_str() == Some("slides"))); + assert_eq!(schema["additionalProperties"], false); + let title_props = &schema["properties"]["title"]; + assert_eq!(title_props["type"], "string"); + assert_eq!(title_props["maxLength"], MAX_TEXT_CHARS); + let slides = &schema["properties"]["slides"]; + assert_eq!(slides["minItems"], 1); + assert_eq!(slides["maxItems"], MAX_SLIDES); + let slide_item = &slides["items"]; + assert_eq!(slide_item["additionalProperties"], false); + let bullets = &slide_item["properties"]["bullets"]; + assert_eq!(bullets["maxItems"], MAX_BULLETS_PER_SLIDE); +} + +#[test] +fn permission_level_is_write() { + let tool = PresentationTool::new(PathBuf::from("/tmp/never-read")); + assert_eq!(tool.permission_level(), PermissionLevel::Write); +} + +#[test] +fn description_includes_router_rules() { + let tool = PresentationTool::new(PathBuf::from("/tmp/never-read")); + let desc = tool.description(); + assert!(desc.contains("USE THIS")); + assert!(desc.contains("NOT for")); + assert!(desc.contains("slides") || desc.contains("deck") || desc.contains("presentation")); +} + +#[tokio::test] +async fn execute_rejects_empty_title() { + let ws = workspace(); + let tool = PresentationTool::new(ws.path().to_path_buf()); + let args = json!({ "title": "", "slides": [{ "title": "x", "bullets": ["y"] }] }); + let result = tool.execute(args).await.expect("execute returns Ok"); + assert!(result.is_error); + assert!(result.text().contains("title")); +} + +#[tokio::test] +async fn execute_rejects_empty_slides_array() { + let ws = workspace(); + let tool = PresentationTool::new(ws.path().to_path_buf()); + let args = json!({ "title": "Deck", "slides": [] }); + let result = tool.execute(args).await.expect("execute returns Ok"); + assert!(result.is_error); + assert!(result.text().contains("slides")); +} + +#[tokio::test] +async fn execute_rejects_slide_with_no_content() { + let ws = workspace(); + let tool = PresentationTool::new(ws.path().to_path_buf()); + let args = json!({ + "title": "Deck", + "slides": [{ "title": "", "body": "", "bullets": [], "speaker_notes": "" }] + }); + let result = tool.execute(args).await.expect("execute returns Ok"); + assert!(result.is_error); +} + +#[tokio::test] +async fn execute_rejects_oversize_body() { + let ws = workspace(); + let tool = PresentationTool::new(ws.path().to_path_buf()); + let big = "x".repeat(MAX_TEXT_CHARS + 1); + let args = json!({ + "title": "Deck", + "slides": [{ "title": "ok", "body": big }] + }); + let result = tool.execute(args).await.expect("execute returns Ok"); + assert!(result.is_error); +} + +#[tokio::test] +async fn execute_rejects_too_many_slides() { + let ws = workspace(); + let tool = PresentationTool::new(ws.path().to_path_buf()); + let slides: Vec<_> = (0..(MAX_SLIDES + 1)) + .map(|i| json!({ "title": format!("Slide {i}"), "bullets": ["x"] })) + .collect(); + let args = json!({ "title": "Big deck", "slides": slides }); + let result = tool.execute(args).await.expect("execute returns Ok"); + assert!(result.is_error); + assert!(result.text().contains(&MAX_SLIDES.to_string())); +} + +#[tokio::test] +async fn execute_happy_path_returns_artifact_metadata() { + // End-to-end: drives the real ppt-rs engine and the artifact + // pipeline. Asserts the tool's success contract — `slide_count` + // excludes the synthetic title slide, the artifact is finalised + // on disk, and the markdown reply quotes the path + size. + let ws = workspace(); + let tool = PresentationTool::new(ws.path().to_path_buf()); + let result = tool + .execute(minimal_input_json()) + .await + .expect("execute returns Ok"); + + assert!( + !result.is_error, + "happy path should not be flagged as error" + ); + + let payload = match result.content.first().expect("at least one content block") { + crate::openhuman::skills::types::ToolContent::Json { data } => data.clone(), + other => panic!("expected Json content block, got {other:?}"), + }; + assert_eq!(payload["slide_count"].as_u64(), Some(1)); + let artifact_path = payload["artifact_path"] + .as_str() + .expect("artifact_path is a string"); + let artifact_id = payload["artifact_id"] + .as_str() + .expect("artifact_id is a string"); + let size_bytes = payload["size_bytes"] + .as_u64() + .expect("size_bytes is an integer"); + + assert!( + std::path::Path::new(artifact_path).exists(), + "artifact file must exist at {artifact_path}" + ); + assert!( + size_bytes > 1000, + "deck unexpectedly small ({size_bytes} bytes)" + ); + + let md = result + .markdown_formatted + .as_deref() + .expect("success_with_markdown sets markdown_formatted"); + assert!(md.contains(artifact_id)); + assert!(md.contains(artifact_path)); + assert!(md.contains("1-slide")); +} + +#[test] +fn truncate_stderr_caps_payload_with_suffix() { + let raw = "y".repeat(2000); + let out = PresentationError::truncate_stderr(&raw); + assert!(out.chars().count() <= 500); + assert!(out.ends_with("[…truncated]")); + let short = "tiny stderr"; + assert_eq!(PresentationError::truncate_stderr(short), short); +} diff --git a/src/openhuman/tools/impl/presentation/types.rs b/src/openhuman/tools/impl/presentation/types.rs new file mode 100644 index 000000000..10845f6c1 --- /dev/null +++ b/src/openhuman/tools/impl/presentation/types.rs @@ -0,0 +1,213 @@ +//! Typed input / output / error contracts for the `generate_presentation` tool. + +use serde::{Deserialize, Serialize}; + +/// Maximum number of slides a single `generate_presentation` call may +/// produce. Hard cap to bound generation time and output size; the +/// LLM is asked to break larger decks into multiple calls. +pub(super) const MAX_SLIDES: usize = 64; + +/// Maximum length of a single text field (title, body, individual +/// bullet, speaker notes). Bounds the payload size sent to the +/// `ppt-rs` engine and avoids pathological inputs that would balloon +/// the deck. +pub(super) const MAX_TEXT_CHARS: usize = 2_000; + +/// Maximum number of bullets per slide. Higher counts produce +/// unreadable slides and bloat the output file. +pub(super) const MAX_BULLETS_PER_SLIDE: usize = 32; + +/// Slide spec — one entry per content slide in the generated deck. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct SlideSpec { + /// Slide title. Empty / omitted is allowed for visually + /// minimalist decks but at least one of `title` / `body` / + /// `bullets` must be populated. + #[serde(default)] + pub title: String, + /// Paragraph body text. Plain text only — rendered into the + /// default content layout's body placeholder by `ppt-rs`. + #[serde(default)] + pub body: Option, + /// Bullet points rendered after the body text (if any). + #[serde(default)] + pub bullets: Vec, + /// Speaker notes attached to the slide. + #[serde(default)] + pub speaker_notes: Option, +} + +/// Top-level input for the `generate_presentation` tool. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct GeneratePresentationInput { + /// Deck title. Surfaces on the title slide and as the artifact's + /// human-readable name. + pub title: String, + /// Optional author byline, surfaced on the title slide. + #[serde(default)] + pub author: Option, + /// Optional theme hint. Currently informational only; the `ppt-rs` + /// engine uses its default template regardless. Reserved for + /// future template-selection work. + #[serde(default)] + pub theme: Option, + /// Slide specs, in display order. Must contain at least one entry. + #[serde(default)] + pub slides: Vec, +} + +/// Tool output returned via [`crate::openhuman::tools::traits::ToolResult`] +/// as the JSON `data` field. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct GeneratePresentationOutput { + /// UUID of the persisted artifact record. Use with the + /// `ai_get_artifact` / `ai_delete_artifact` RPCs. + pub artifact_id: String, + /// Absolute filesystem path to the generated `.pptx`. Useful for + /// the agent to reference in its reply ("saved to …"). + pub artifact_path: String, + /// Number of content slides actually produced (excludes the + /// title slide). + pub slide_count: usize, + /// On-disk size of the produced `.pptx` in bytes. + pub size_bytes: u64, +} + +/// Structured error variants surfaced to the agent. Aligned with the +/// taxonomy #2780 will surface to the user via the orchestrator. +#[derive(Debug, thiserror::Error)] +pub enum PresentationError { + #[error("invalid input for field '{field}': {reason}")] + InvalidInput { field: String, reason: String }, + + #[error("presentation generation failed (exit={exit_code}): {stderr_truncated}")] + GenerationFailed { + exit_code: i32, + stderr_truncated: String, + }, + + #[error("presentation generation exceeded {timeout_secs}s timeout")] + GenerationTimeout { timeout_secs: u64 }, +} + +impl PresentationError { + /// Truncate a stderr string to the per-#2780 cap of 500 chars + /// (UTF-8-safe). Used when wrapping a non-zero exit into + /// `GenerationFailed` so the variant never carries an unbounded + /// payload back to the agent. + pub(super) fn truncate_stderr(raw: &str) -> String { + const MAX: usize = 500; + const SUFFIX: &str = " […truncated]"; + let total = raw.chars().count(); + if total <= MAX { + return raw.to_string(); + } + let keep = MAX.saturating_sub(SUFFIX.chars().count()); + let mut out: String = raw.chars().take(keep).collect(); + out.push_str(SUFFIX); + out + } +} + +/// Validate the input early — before spawning Python — so the agent +/// gets a structured `InvalidInput` it can self-correct on instead of +/// a generic Python traceback. +pub(super) fn validate_input(input: &GeneratePresentationInput) -> Result<(), PresentationError> { + if input.title.trim().is_empty() { + return Err(PresentationError::InvalidInput { + field: "title".to_string(), + reason: "must not be empty".to_string(), + }); + } + if input.title.chars().count() > MAX_TEXT_CHARS { + return Err(PresentationError::InvalidInput { + field: "title".to_string(), + reason: format!("must be ≤ {MAX_TEXT_CHARS} chars"), + }); + } + if let Some(author) = input.author.as_deref() { + if author.chars().count() > MAX_TEXT_CHARS { + return Err(PresentationError::InvalidInput { + field: "author".to_string(), + reason: format!("must be ≤ {MAX_TEXT_CHARS} chars"), + }); + } + } + if let Some(theme) = input.theme.as_deref() { + if theme.chars().count() > MAX_TEXT_CHARS { + return Err(PresentationError::InvalidInput { + field: "theme".to_string(), + reason: format!("must be ≤ {MAX_TEXT_CHARS} chars"), + }); + } + } + if input.slides.is_empty() { + return Err(PresentationError::InvalidInput { + field: "slides".to_string(), + reason: "must contain at least one slide".to_string(), + }); + } + if input.slides.len() > MAX_SLIDES { + return Err(PresentationError::InvalidInput { + field: "slides".to_string(), + reason: format!("must contain ≤ {MAX_SLIDES} slides"), + }); + } + for (i, slide) in input.slides.iter().enumerate() { + let has_title = !slide.title.trim().is_empty(); + let has_body = slide + .body + .as_deref() + .map(|s| !s.trim().is_empty()) + .unwrap_or(false); + // Reject whitespace-only bullets too: build_slides() trims and drops + // empty entries, so a slide with only [" "] would render blank + // despite passing this "at least one of title/body/bullets" gate. + let has_bullets = slide.bullets.iter().any(|b| !b.trim().is_empty()); + if !has_title && !has_body && !has_bullets { + return Err(PresentationError::InvalidInput { + field: format!("slides[{i}]"), + reason: "must have at least one of title / body / bullets".to_string(), + }); + } + if slide.title.chars().count() > MAX_TEXT_CHARS { + return Err(PresentationError::InvalidInput { + field: format!("slides[{i}].title"), + reason: format!("must be ≤ {MAX_TEXT_CHARS} chars"), + }); + } + if let Some(body) = slide.body.as_deref() { + if body.chars().count() > MAX_TEXT_CHARS { + return Err(PresentationError::InvalidInput { + field: format!("slides[{i}].body"), + reason: format!("must be ≤ {MAX_TEXT_CHARS} chars"), + }); + } + } + if slide.bullets.len() > MAX_BULLETS_PER_SLIDE { + return Err(PresentationError::InvalidInput { + field: format!("slides[{i}].bullets"), + reason: format!("must contain ≤ {MAX_BULLETS_PER_SLIDE} bullets"), + }); + } + for (b, bullet) in slide.bullets.iter().enumerate() { + if bullet.chars().count() > MAX_TEXT_CHARS { + return Err(PresentationError::InvalidInput { + field: format!("slides[{i}].bullets[{b}]"), + reason: format!("must be ≤ {MAX_TEXT_CHARS} chars"), + }); + } + } + if let Some(notes) = slide.speaker_notes.as_deref() { + if notes.chars().count() > MAX_TEXT_CHARS { + return Err(PresentationError::InvalidInput { + field: format!("slides[{i}].speaker_notes"), + reason: format!("must be ≤ {MAX_TEXT_CHARS} chars"), + }); + } + } + } + Ok(()) +} diff --git a/src/openhuman/tools/ops.rs b/src/openhuman/tools/ops.rs index ab9f9909e..ada742452 100644 --- a/src/openhuman/tools/ops.rs +++ b/src/openhuman/tools/ops.rs @@ -447,6 +447,14 @@ pub fn all_tools_with_runtime( Box::new(WorkspaceInitTool), ]; + // Presentation generation (#2778). Native-Rust engine (ppt-rs + // backed) as of the #2780-follow-up rust-engine refactor — no + // managed Python venv, no first-call install latency. Always + // registered. + tools.push(Box::new(PresentationTool::new( + root_config.workspace_dir.clone(), + ))); + if browser_config.enabled { // Unified web-access allowlist (merge fetch + browser firewalls): the // browser tool shares the single `http_request.allowed_domains` host