From 679d463e0938bf3301fe7af903cbf0f61a0513a0 Mon Sep 17 00:00:00 2001 From: oxoxDev <164490987+oxoxDev@users.noreply.github.com> Date: Thu, 4 Jun 2026 10:23:30 +0530 Subject: [PATCH] feat(presentation): embed images in generated decks (#3209) (#3299) --- .../agents/orchestrator/prompt.md | 16 ++ src/openhuman/artifacts/mod.rs | 2 +- src/openhuman/artifacts/store.rs | 34 +++ .../tools/impl/presentation/engine.rs | 211 ++++++++++++++++- .../tools/impl/presentation/image_util.rs | 167 ++++++++++++++ src/openhuman/tools/impl/presentation/mod.rs | 213 ++++++++++++++++-- .../tools/impl/presentation/tests.rs | 208 ++++++++++++++++- .../tools/impl/presentation/types.rs | 115 ++++++++++ src/openhuman/tools/ops.rs | 1 + 9 files changed, 925 insertions(+), 42 deletions(-) create mode 100644 src/openhuman/tools/impl/presentation/image_util.rs diff --git a/src/openhuman/agent_registry/agents/orchestrator/prompt.md b/src/openhuman/agent_registry/agents/orchestrator/prompt.md index e896f40ec..e1b0d21c9 100644 --- a/src/openhuman/agent_registry/agents/orchestrator/prompt.md +++ b/src/openhuman/agent_registry/agents/orchestrator/prompt.md @@ -221,3 +221,19 @@ When your answer is informed by retrieved memory, cite it with footnote markers: > [^1]: gmail · alice@example.com · 2026-04-22 · node:abc123 Inline marker `[^N]` and a numbered footnote at the end carrying the node_id and source_ref from the RetrievalHit. Do not invent quotes — only quote text that appears verbatim in a hit's `content` field. + +## Presentations with images + +`generate_presentation` builds a `.pptx` from a `title` + `slides` array. Each slide may also carry an `images` array to embed pictures (charts, screenshots, diagrams) beneath the text: + +``` +"images": [ + { "source": { "type": "artifact", "artifact_id": "" }, "caption": "Q2 revenue" }, + { "source": { "type": "file", "path": "/abs/path/chart.png" }, "caption": "Funnel" } +] +``` + +- Two sources: `artifact` (bytes from a prior tool's output artifact) and `file` (a local path the agent can read). Remote URLs are **not** supported — fetch or save the image first, then reference it by artifact id or path. +- Embeddable formats are **PNG and JPEG only**, ≤5 MB each, ≤6 per slide, ≤8 per deck. An image that fails these checks is **skipped** and reported in the tool's `image_warnings` — the deck is still produced. If you see warnings, tell the user which images were dropped and why. +- **Grounding rule (mandatory):** only attach an image whose content you have actually verified. Before claiming "this chart shows X" in a caption or in your reply, confirm it via `research` / `memory_tree` / the tool output that produced the image. Never assert what an image depicts from its filename or the user's request alone — if you have not seen the contents, describe it neutrally or omit the claim. +- v1 layout is single-column (images stacked beneath the text); a multi-image grid and richer placement are not yet available. diff --git a/src/openhuman/artifacts/mod.rs b/src/openhuman/artifacts/mod.rs index 21fa2e3ed..61dd55be6 100644 --- a/src/openhuman/artifacts/mod.rs +++ b/src/openhuman/artifacts/mod.rs @@ -8,5 +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 store::{create_artifact, fail_artifact, finalize_artifact, read_artifact_bytes}; pub use types::{ArtifactKind, ArtifactMeta, ArtifactStatus}; diff --git a/src/openhuman/artifacts/store.rs b/src/openhuman/artifacts/store.rs index fffb4f736..fb914f164 100644 --- a/src/openhuman/artifacts/store.rs +++ b/src/openhuman/artifacts/store.rs @@ -231,6 +231,40 @@ pub(crate) async fn get_artifact( Ok(meta) } +/// Read the raw bytes of a finalized artifact's output file. +/// +/// Single source of truth for resolving an artifact id → on-disk bytes: +/// callers (e.g. the presentation image pipeline) must not reconstruct +/// the `//` path scheme themselves. Validates the id, +/// confirms the resolved path stays under the artifacts root, and refuses +/// artifacts that are not yet [`ArtifactStatus::Ready`] (a `Pending` / +/// `Failed` record may have no bytes — or partial bytes — on disk). +pub async fn read_artifact_bytes( + workspace_dir: &Path, + artifact_id: &str, +) -> Result, String> { + log::debug!("[artifacts] read_artifact_bytes: id={artifact_id}"); + let meta = get_artifact(workspace_dir, artifact_id).await?; + if !matches!(meta.status, ArtifactStatus::Ready) { + return Err(format!( + "[artifacts] artifact id={artifact_id} is not ready (status={:?})", + meta.status + )); + } + let root = artifacts_root(workspace_dir).await?; + // `meta.path` is the store-internal `/` relative path. + let file_path = root.join(&meta.path); + assert_within_root(&root, &file_path)?; + let bytes = tokio::fs::read(&file_path) + .await + .map_err(|e| format!("[artifacts] failed to read artifact bytes id={artifact_id}: {e}"))?; + log::debug!( + "[artifacts] read_artifact_bytes: id={artifact_id} read {} bytes", + bytes.len() + ); + Ok(bytes) +} + /// Delete an artifact directory and all its contents. pub(crate) async fn delete_artifact(workspace_dir: &Path, artifact_id: &str) -> Result<(), String> { log::debug!("[artifacts] delete_artifact: id={artifact_id}"); diff --git a/src/openhuman/tools/impl/presentation/engine.rs b/src/openhuman/tools/impl/presentation/engine.rs index 47f8e8695..a71321c4b 100644 --- a/src/openhuman/tools/impl/presentation/engine.rs +++ b/src/openhuman/tools/impl/presentation/engine.rs @@ -41,11 +41,26 @@ use std::time::Duration; -use ppt_rs::generator::{create_pptx_with_content, SlideContent}; +use ppt_rs::generator::{create_pptx_with_content, Image, SlideContent}; use tokio::task::JoinError; use tokio::time::{error::Elapsed, timeout}; -use super::types::{GeneratePresentationInput, PresentationError}; +use super::types::{GeneratePresentationInput, PresentationError, ResolvedSlideImage}; + +/// Slide geometry (matches `ppt-rs`'s default 4:3 deck: 10in × 7.5in). +const SLIDE_WIDTH_EMU: u32 = 9_144_000; +const SLIDE_HEIGHT_EMU: u32 = 6_858_000; +/// 1 inch side margins → usable content column width. +const SIDE_MARGIN_EMU: u32 = 914_400; +/// Images live in the lower band of the slide, beneath the title/body +/// placeholder. Top of that band ≈ slide midpoint; bottom keeps a 0.5in +/// margin. v1 single-column: images stack vertically inside this band. +const IMAGE_BAND_TOP_EMU: u32 = 3_429_000; +const IMAGE_BAND_BOTTOM_MARGIN_EMU: u32 = 457_200; +/// Vertical gap between stacked images. +const IMAGE_STACK_GAP_EMU: u32 = 91_440; +/// EMU per pixel at 96 DPI (matches `ppt-rs`'s own px→EMU convention). +const EMU_PER_PX: u32 = 9_525; /// Run the synthesis. Returns the serialised `.pptx` bytes ready to /// be written to the artifact path. @@ -55,12 +70,13 @@ use super::types::{GeneratePresentationInput, PresentationError}; /// [`PresentationError::GenerationTimeout`]. pub(super) async fn generate( input: &GeneratePresentationInput, + images: &[Vec], 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 slides = build_slides(input, images); let deck_title = input.title.clone(); let started = std::time::Instant::now(); let slide_count = slides.len(); @@ -133,21 +149,24 @@ pub(super) async fn generate( /// 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 { +fn build_slides( + input: &GeneratePresentationInput, + images: &[Vec], +) -> 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. + // in core.xml metadata. The title slide carries no images. 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 { + for (idx, spec) in input.slides.iter().enumerate() { 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); @@ -157,6 +176,16 @@ fn build_slides(input: &GeneratePresentationInput) -> Vec { slide = slide.add_bullet(bullet); } } + // Attach resolved images AFTER the text, single-column in the + // lower band of the slide. Each image's caption (if any) is + // rendered as a trailing bullet so the label is not lost. + let slide_images = images.get(idx).map(Vec::as_slice).unwrap_or(&[]); + for placed in place_single_column(slide_images) { + slide = slide.add_image(placed.image); + if let Some(caption) = placed.caption { + slide = slide.add_bullet(&caption); + } + } if let Some(notes) = spec .speaker_notes .as_deref() @@ -170,6 +199,69 @@ fn build_slides(input: &GeneratePresentationInput) -> Vec { out } +/// A positioned `ppt-rs` image plus its (optional) caption text. +struct PlacedImage { + image: Image, + caption: Option, +} + +/// Lay resolved images out in a single vertical column inside the lower +/// band of the slide. Each image is scaled to fit its slot while +/// preserving aspect ratio and centred horizontally + vertically within +/// the slot. v1 layout — a multi-image grid is deferred. +fn place_single_column(images: &[ResolvedSlideImage]) -> Vec { + let n = images.len() as u32; + if n == 0 { + return Vec::new(); + } + + let content_left = SIDE_MARGIN_EMU; + let content_width = SLIDE_WIDTH_EMU.saturating_sub(2 * SIDE_MARGIN_EMU); + let band_height = SLIDE_HEIGHT_EMU + .saturating_sub(IMAGE_BAND_TOP_EMU) + .saturating_sub(IMAGE_BAND_BOTTOM_MARGIN_EMU); + let total_gap = IMAGE_STACK_GAP_EMU.saturating_mul(n.saturating_sub(1)); + let slot_height = band_height.saturating_sub(total_gap) / n; + + images + .iter() + .enumerate() + .map(|(i, img)| { + let slot_top = IMAGE_BAND_TOP_EMU + (i as u32) * (slot_height + IMAGE_STACK_GAP_EMU); + let (w, h) = fit_within( + img.width_px.saturating_mul(EMU_PER_PX), + img.height_px.saturating_mul(EMU_PER_PX), + content_width, + slot_height, + ); + let x = content_left + content_width.saturating_sub(w) / 2; + let y = slot_top + slot_height.saturating_sub(h) / 2; + let image = Image::from_bytes(img.bytes.clone(), w, h, img.format).position(x, y); + PlacedImage { + image, + caption: img.caption.clone(), + } + }) + .collect() +} + +/// Scale `(w, h)` (EMU) to fit within `(max_w, max_h)` preserving aspect +/// ratio. Never upscales beyond the bounding box; only shrinks when the +/// source overflows. Falls back to the bounding box if the source has a +/// degenerate zero dimension. +fn fit_within(w: u32, h: u32, max_w: u32, max_h: u32) -> (u32, u32) { + if w == 0 || h == 0 { + return (max_w, max_h); + } + // Scale factor in fixed-point-ish f64; min of the two axis ratios. + let ratio_w = max_w as f64 / w as f64; + let ratio_h = max_h as f64 / h as f64; + let scale = ratio_w.min(ratio_h); + let fit_w = ((w as f64) * scale).round().max(1.0) as u32; + let fit_h = ((h as f64) * scale).round().max(1.0) as u32; + (fit_w.min(max_w).max(1), fit_h.min(max_h).max(1)) +} + /// 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 @@ -247,13 +339,14 @@ mod tests { "Hired 3 engineers".to_string(), ], speaker_notes: Some("Emphasise headcount efficiency.".to_string()), + images: vec![], }], } } #[test] fn build_slides_prepends_title_slide_with_author_byline() { - let slides = build_slides(&input_with_one_slide()); + 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 @@ -272,7 +365,7 @@ mod tests { 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); + 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 @@ -289,7 +382,7 @@ mod tests { // OOXML reader (PowerPoint, Keynote, LibreOffice, Google // Slides) can open. let input = input_with_one_slide(); - let bytes = generate(&input, Duration::from_secs(30)) + let bytes = generate(&input, &[], Duration::from_secs(30)) .await .expect("generate should succeed on a 1-slide deck"); @@ -389,7 +482,7 @@ mod tests { // 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)) + let err = generate(&input, &[], Duration::from_nanos(1)) .await .expect_err("1 ns deadline should never satisfy the timeout"); match err { @@ -402,4 +495,102 @@ mod tests { other => panic!("expected GenerationTimeout, got {other:?}"), } } + + /// Canonical 1×1 PNG used to exercise the embed path. + fn png_1x1() -> Vec { + use base64::{engine::general_purpose::STANDARD, Engine as _}; + STANDARD + .decode("iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg==") + .unwrap() + } + + fn resolved_png(caption: Option<&str>) -> ResolvedSlideImage { + ResolvedSlideImage { + bytes: png_1x1(), + format: "PNG", + width_px: 320, + height_px: 240, + caption: caption.map(ToString::to_string), + } + } + + #[test] + fn fit_within_preserves_aspect_and_clamps() { + // 2:1 source into a square box → width-limited, half-height. + let (w, h) = fit_within(2000, 1000, 1000, 1000); + assert_eq!(w, 1000); + assert_eq!(h, 500); + // Never exceeds the bounding box on either axis. + let (w, h) = fit_within(10, 10, 4000, 800); + assert!(w <= 4000 && h <= 800); + // Degenerate zero dimension falls back to the box. + assert_eq!(fit_within(0, 10, 500, 600), (500, 600)); + } + + #[test] + fn place_single_column_stacks_within_slide_bounds() { + let imgs = vec![resolved_png(None), resolved_png(None), resolved_png(None)]; + let placed = place_single_column(&imgs); + assert_eq!(placed.len(), 3); + let mut prev_bottom = 0u32; + for p in &placed { + // Horizontally inside the content column. + assert!(p.image.x >= SIDE_MARGIN_EMU); + assert!(p.image.x + p.image.width <= SLIDE_WIDTH_EMU - SIDE_MARGIN_EMU + 1); + // Vertically inside the lower band and monotonically stacked. + assert!(p.image.y >= IMAGE_BAND_TOP_EMU); + assert!(p.image.y + p.image.height <= SLIDE_HEIGHT_EMU); + assert!( + p.image.y >= prev_bottom, + "images must not overlap vertically" + ); + prev_bottom = p.image.y + p.image.height; + } + } + + #[tokio::test] + async fn generate_embeds_image_into_media_and_content_types() { + // Images are supplied via the resolved arg, not the input spec. + let input = input_with_one_slide(); + let images = vec![vec![resolved_png(Some("Figure 1"))]]; + + let bytes = generate(&input, &images, Duration::from_secs(30)) + .await + .expect("generate with one image should succeed"); + + let cursor = std::io::Cursor::new(&bytes); + let mut zip = zip::ZipArchive::new(cursor).expect("valid zip"); + let names: Vec = (0..zip.len()) + .map(|i| zip.by_index(i).unwrap().name().to_string()) + .collect(); + + assert!( + names.iter().any(|n| n == "ppt/media/image1.png"), + "embedded PNG missing from ppt/media (got: {names:?})" + ); + + let ct_body = { + let mut ct = zip.by_name("[Content_Types].xml").unwrap(); + let mut body = String::new(); + std::io::Read::read_to_string(&mut ct, &mut body).unwrap(); + body + }; + assert!( + ct_body.contains("Extension=\"png\""), + "[Content_Types].xml missing png default extension" + ); + + // Caption rendered as a bullet → its text lands in slide2.xml + // (slide1 is the synthetic title slide). + let slide2_body = { + let mut slide2 = zip.by_name("ppt/slides/slide2.xml").unwrap(); + let mut body = String::new(); + std::io::Read::read_to_string(&mut slide2, &mut body).unwrap(); + body + }; + assert!( + slide2_body.contains("Figure 1"), + "caption text missing from rendered slide2.xml" + ); + } } diff --git a/src/openhuman/tools/impl/presentation/image_util.rs b/src/openhuman/tools/impl/presentation/image_util.rs new file mode 100644 index 000000000..9000083d3 --- /dev/null +++ b/src/openhuman/tools/impl/presentation/image_util.rs @@ -0,0 +1,167 @@ +//! Self-contained PNG / JPEG sniffing + pixel-dimension extraction for +//! the presentation image pipeline. +//! +//! This deliberately does **not** reuse `agent::multimodal`'s private +//! magic-byte helpers, for three reasons: +//! +//! 1. Those helpers live outside this feature's edit boundary. +//! 2. They accept `webp` / `gif` / `bmp`, none of which `ppt-rs` 0.2.14 +//! can embed safely — there is no `webp` default in the generated +//! `[Content_Types].xml`, and `ImageBuilder::auto` misclassifies +//! `webp` as `PNG`, producing a part PowerPoint refuses to render. +//! v1 therefore restricts embeddable images to PNG + JPEG. +//! 3. They do not expose pixel dimensions, which we need to place +//! images aspect-correctly in the single-column layout. + +/// Return the `ppt-rs` format token (`"PNG"` / `"JPEG"`) for `bytes`, +/// or `None` if the bytes are not one of the two embeddable formats. +pub(super) fn sniff_format(bytes: &[u8]) -> Option<&'static str> { + if bytes.len() >= 8 && bytes.starts_with(&[0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]) { + Some("PNG") + } else if bytes.len() >= 3 && bytes.starts_with(&[0xFF, 0xD8, 0xFF]) { + Some("JPEG") + } else { + None + } +} + +/// Native `(width, height)` in pixels for a PNG or JPEG. Returns `None` +/// when the header is truncated / malformed or the format is unsupported. +pub(super) fn pixel_dimensions(bytes: &[u8], format: &str) -> Option<(u32, u32)> { + match format { + "PNG" => png_dimensions(bytes), + "JPEG" => jpeg_dimensions(bytes), + _ => None, + } +} + +/// PNG: 8-byte signature, then an `IHDR` chunk whose width / height are +/// big-endian `u32`s at byte offsets 16 and 20. +fn png_dimensions(bytes: &[u8]) -> Option<(u32, u32)> { + if bytes.len() < 24 || &bytes[12..16] != b"IHDR" { + return None; + } + let w = u32::from_be_bytes([bytes[16], bytes[17], bytes[18], bytes[19]]); + let h = u32::from_be_bytes([bytes[20], bytes[21], bytes[22], bytes[23]]); + if w == 0 || h == 0 { + return None; + } + Some((w, h)) +} + +/// JPEG: walk the marker segments until a Start-Of-Frame (`SOF0`/`SOF2`, +/// and the other non-differential / progressive SOF markers) is hit; its +/// payload carries height then width as big-endian `u16`s. +fn jpeg_dimensions(bytes: &[u8]) -> Option<(u32, u32)> { + let mut i = 2; // skip the leading FF D8 SOI + while i + 3 < bytes.len() { + if bytes[i] != 0xFF { + i += 1; + continue; + } + let marker = bytes[i + 1]; + i += 2; + // Standalone markers (no length field): padding fill bytes and + // RSTn / SOI / EOI. Skip without consuming a segment length. + if marker == 0xFF || marker == 0xD8 || marker == 0xD9 || (0xD0..=0xD7).contains(&marker) { + continue; + } + if i + 1 >= bytes.len() { + return None; + } + let seg_len = u16::from_be_bytes([bytes[i], bytes[i + 1]]) as usize; + if seg_len < 2 { + return None; + } + // SOF markers carrying frame dimensions. Excludes 0xC4 (DHT), + // 0xC8 (JPG), 0xCC (DAC), which share the 0xCn range but are not + // frame headers. + let is_sof = matches!( + marker, + 0xC0 | 0xC1 + | 0xC2 + | 0xC3 + | 0xC5 + | 0xC6 + | 0xC7 + | 0xC9 + | 0xCA + | 0xCB + | 0xCD + | 0xCE + | 0xCF + ); + if is_sof { + // segment: [len_hi len_lo precision h_hi h_lo w_hi w_lo ...] + if i + 6 >= bytes.len() { + return None; + } + let h = u16::from_be_bytes([bytes[i + 3], bytes[i + 4]]) as u32; + let w = u16::from_be_bytes([bytes[i + 5], bytes[i + 6]]) as u32; + if w == 0 || h == 0 { + return None; + } + return Some((w, h)); + } + i += seg_len; + } + None +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Canonical 1×1 PNG (full IHDR + IDAT + IEND). + fn png_1x1() -> Vec { + use base64::{engine::general_purpose::STANDARD, Engine as _}; + STANDARD + .decode("iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg==") + .unwrap() + } + + /// Minimal JPEG: SOI + APP0 stub + SOF0 declaring 7×5. + fn jpeg_7x5() -> Vec { + vec![ + 0xFF, 0xD8, // SOI + 0xFF, 0xE0, 0x00, 0x04, 0x00, 0x00, // APP0, len=4, 2 payload bytes + 0xFF, 0xC0, 0x00, 0x0B, // SOF0, len=11 + 0x08, // precision + 0x00, 0x05, // height = 5 + 0x00, 0x07, // width = 7 + 0x03, 0x00, 0x00, 0x00, // components (filler) + 0xFF, 0xD9, // EOI + ] + } + + #[test] + fn sniffs_png_and_jpeg() { + assert_eq!(sniff_format(&png_1x1()), Some("PNG")); + assert_eq!(sniff_format(&jpeg_7x5()), Some("JPEG")); + } + + #[test] + fn rejects_non_image_and_unsupported() { + assert_eq!(sniff_format(b"not an image"), None); + // GIF magic — recognised by multimodal but NOT embeddable here. + assert_eq!(sniff_format(b"GIF89a....."), None); + // WebP magic — same story. + assert_eq!(sniff_format(b"RIFF\0\0\0\0WEBP"), None); + } + + #[test] + fn reads_png_dimensions() { + assert_eq!(pixel_dimensions(&png_1x1(), "PNG"), Some((1, 1))); + } + + #[test] + fn reads_jpeg_dimensions() { + assert_eq!(pixel_dimensions(&jpeg_7x5(), "JPEG"), Some((7, 5))); + } + + #[test] + fn truncated_headers_yield_none() { + assert_eq!(png_dimensions(&[0x89, 0x50, 0x4E, 0x47]), None); + assert_eq!(jpeg_dimensions(&[0xFF, 0xD8]), None); + } +} diff --git a/src/openhuman/tools/impl/presentation/mod.rs b/src/openhuman/tools/impl/presentation/mod.rs index 6fc99c29b..c6929e432 100644 --- a/src/openhuman/tools/impl/presentation/mod.rs +++ b/src/openhuman/tools/impl/presentation/mod.rs @@ -28,17 +28,20 @@ //! continue to work without change. use std::path::PathBuf; +use std::sync::Arc; 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, + create_artifact, fail_artifact, finalize_artifact, read_artifact_bytes, ArtifactKind, }; +use crate::openhuman::security::SecurityPolicy; use crate::openhuman::tools::traits::{PermissionLevel, Tool, ToolResult}; mod engine; +mod image_util; mod types; #[cfg(test)] @@ -46,7 +49,8 @@ mod types; mod tests; use self::types::{ - validate_input, GeneratePresentationInput, GeneratePresentationOutput, PresentationError, + validate_input, GeneratePresentationInput, GeneratePresentationOutput, ResolvedSlideImage, + SlideImage, SlideImageSource, MAX_IMAGE_BYTES, }; /// Generation timeout. `ppt-rs` typically completes the full 64-slide @@ -63,14 +67,23 @@ pub const TOOL_NAME: &str = "generate_presentation"; /// One-shot `.pptx` generator. See module docs for the request flow. pub struct PresentationTool { workspace_dir: PathBuf, + /// Security policy used to validate agent-supplied `File` image paths + /// before any filesystem read — an image path must pass the same + /// `validate_path` checks (allowed-location, symlink-escape, forbidden + /// dirs) as any other file-read operation. + security: Arc, } 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 } + /// workspace directory the artifact pipeline writes into, plus the + /// active [`SecurityPolicy`] for validating `File`-source image paths. + pub fn new(workspace_dir: PathBuf, security: Arc) -> Self { + Self { + workspace_dir, + security, + } } } @@ -95,6 +108,57 @@ impl Tool for PresentationTool { } fn parameters_schema(&self) -> Value { + // Built as separate `json!` bindings (rather than one deeply-nested + // literal) to keep macro expansion within the crate's default + // recursion limit — the per-slide `images` sub-schema adds enough + // depth to overflow a single combined literal. + let image_item_schema = json!({ + "type": "object", + "additionalProperties": false, + "required": ["source"], + "properties": { + "source": { + "type": "object", + "additionalProperties": false, + "required": ["type"], + "description": "Image data source. Use `{type:'artifact', artifact_id}` for a prior tool's output, or `{type:'file', path}` for a readable local file.", + "properties": { + "type": { "type": "string", "enum": ["artifact", "file"] }, + "artifact_id": { "type": "string", "description": "Required when type='artifact'." }, + "path": { "type": "string", "description": "Required when type='file'. Absolute or action-dir-relative path." } + } + }, + "caption": { + "type": "string", + "maxLength": types::MAX_TEXT_CHARS, + "description": "Optional caption rendered as a text bullet beneath the image." + } + } + }); + + let images_schema = json!({ + "type": "array", + "maxItems": types::MAX_IMAGES_PER_SLIDE, + "description": "Images to embed beneath the slide text, single-column. PNG or JPEG only (≤5 MB each, ≤8 per deck). Each image's bytes come from a workspace artifact or a local file path the agent can read. Only attach an image whose content you have actually inspected — do not claim an image shows something you have not verified.", + "items": image_item_schema, + }); + + let slide_item_schema = json!({ + "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 }, + "images": images_schema, + } + }); + json!({ "type": "object", "additionalProperties": false, @@ -120,20 +184,7 @@ impl Tool for PresentationTool { "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 } - } - } + "items": slide_item_schema, } } }) @@ -172,6 +223,18 @@ impl Tool for PresentationTool { "[presentation] generation request accepted" ); + // Resolve + validate per-slide images at the async boundary. A + // bad image is skipped with a warning (partial success) rather + // than failing the whole deck — mirrors the #3076 ethos. + let (resolved_images, image_warnings) = self.resolve_images(&input).await; + if !image_warnings.is_empty() { + tracing::info!( + target: "presentation", + warning_count = image_warnings.len(), + "[presentation] some images skipped with warnings" + ); + } + let (meta, output_path) = create_artifact( &self.workspace_dir, ArtifactKind::Presentation, @@ -181,7 +244,7 @@ impl Tool for PresentationTool { .await .map_err(anyhow::Error::msg)?; - let bytes = match engine::generate(&input, GENERATION_TIMEOUT).await { + let bytes = match engine::generate(&input, &resolved_images, GENERATION_TIMEOUT).await { Ok(bytes) => bytes, Err(err) => { let _ = fail_artifact(&self.workspace_dir, &meta.id, &err.to_string()).await; @@ -246,12 +309,120 @@ impl Tool for PresentationTool { artifact_path: output_path.display().to_string(), slide_count: input.slides.len(), size_bytes, + image_warnings, }; let payload = serde_json::to_value(&out)?; - let markdown = format!( + let mut markdown = format!( "Generated {}-slide presentation at `{}` (artifact `{}`, {} bytes).", out.slide_count, out.artifact_path, out.artifact_id, out.size_bytes ); + if !out.image_warnings.is_empty() { + markdown.push_str("\n\n⚠️ Some images were skipped:"); + for warning in &out.image_warnings { + markdown.push_str(&format!("\n- {warning}")); + } + } Ok(ToolResult::success_with_markdown(payload, markdown)) } } + +impl PresentationTool { + /// Resolve + validate every slide's images at the async boundary, + /// returning a per-slide vec of embeddable images (aligned 1:1 with + /// `input.slides`) plus a flat list of human-readable warnings for + /// images that were skipped. + /// + /// A skip is never fatal: a deck with one bad image still renders, + /// minus that image, and the agent is told why via the warning list. + async fn resolve_images( + &self, + input: &GeneratePresentationInput, + ) -> (Vec>, Vec) { + let mut per_slide: Vec> = Vec::with_capacity(input.slides.len()); + let mut warnings: Vec = Vec::new(); + + for (slide_idx, spec) in input.slides.iter().enumerate() { + let mut resolved = Vec::with_capacity(spec.images.len()); + for (img_idx, image) in spec.images.iter().enumerate() { + match self.resolve_one_image(image).await { + Ok(r) => resolved.push(r), + Err(reason) => { + // 1-based indices in the message (matches how the + // agent thinks about "slide 2, image 1"). + warnings.push(format!( + "slide {} image {}: {reason}", + slide_idx + 1, + img_idx + 1 + )); + } + } + } + per_slide.push(resolved); + } + + (per_slide, warnings) + } + + /// Resolve a single [`SlideImage`] to validated, embeddable bytes. + /// Returns `Err(reason)` (a short human-readable string) when the + /// image cannot be embedded — the caller turns that into a skip + /// warning. + async fn resolve_one_image(&self, image: &SlideImage) -> Result { + let bytes = match &image.source { + SlideImageSource::Artifact { artifact_id } => { + read_artifact_bytes(&self.workspace_dir, artifact_id) + .await + .map_err(|e| format!("artifact {artifact_id} unreadable: {e}"))? + } + SlideImageSource::File { path } => { + // Validate the agent-supplied path against the security + // policy BEFORE touching the filesystem. This enforces the + // same allowed-location / symlink-escape / forbidden-dir + // checks as every other file-read tool — without it an + // agent could embed `/etc/shadow` or `~/.ssh/id_rsa`. + // `validate_path` returns the canonical, in-policy path. + let resolved = self + .security + .validate_path(path) + .await + .map_err(|e| format!("file {path} not allowed: {e}"))?; + // Stat first so a pathologically large file is rejected + // before we pull it into memory. + let meta = tokio::fs::metadata(&resolved) + .await + .map_err(|e| format!("file {path} unreadable: {e}"))?; + if meta.len() as usize > MAX_IMAGE_BYTES { + return Err(format!( + "file {path} is {} bytes, exceeds {MAX_IMAGE_BYTES}-byte cap", + meta.len() + )); + } + tokio::fs::read(&resolved) + .await + .map_err(|e| format!("file {path} unreadable: {e}"))? + } + }; + + if bytes.len() > MAX_IMAGE_BYTES { + return Err(format!( + "image is {} bytes, exceeds {MAX_IMAGE_BYTES}-byte cap", + bytes.len() + )); + } + + let format = image_util::sniff_format(&bytes).ok_or_else(|| { + "unsupported image type (only PNG and JPEG are embeddable)".to_string() + })?; + + let (width_px, height_px) = image_util::pixel_dimensions(&bytes, format) + .ok_or_else(|| format!("could not read {format} dimensions (corrupt header?)"))?; + + Ok(ResolvedSlideImage { + bytes, + format, + width_px, + height_px, + caption: image.caption.clone(), + }) + } +} diff --git a/src/openhuman/tools/impl/presentation/tests.rs b/src/openhuman/tools/impl/presentation/tests.rs index 9e92ba4c1..bb60795e5 100644 --- a/src/openhuman/tools/impl/presentation/tests.rs +++ b/src/openhuman/tools/impl/presentation/tests.rs @@ -15,12 +15,32 @@ use super::types::{PresentationError, MAX_BULLETS_PER_SLIDE, MAX_SLIDES, MAX_TEXT_CHARS}; use super::*; -use std::path::PathBuf; +use std::path::Path; fn workspace() -> tempfile::TempDir { tempfile::tempdir().expect("create temp workspace") } +/// A permissive policy rooted at `workspace` so File-source images written +/// under the temp workspace pass `validate_path`. Mirrors the pattern used +/// by the browser tool tests (`image_info.rs`). +fn test_security(workspace: &Path) -> Arc { + use crate::openhuman::security::AutonomyLevel; + Arc::new(SecurityPolicy { + autonomy: AutonomyLevel::Full, + workspace_dir: workspace.to_path_buf(), + action_dir: workspace.to_path_buf(), + workspace_only: false, + forbidden_paths: vec![], + ..SecurityPolicy::default() + }) +} + +/// Build a tool whose security policy is rooted at `workspace`. +fn make_tool(workspace: &Path) -> PresentationTool { + PresentationTool::new(workspace.to_path_buf(), test_security(workspace)) +} + fn minimal_input_json() -> serde_json::Value { json!({ "title": "Quarterly Review", @@ -32,7 +52,7 @@ fn minimal_input_json() -> serde_json::Value { #[test] fn parameters_schema_shape_matches_contract() { - let tool = PresentationTool::new(PathBuf::from("/tmp/never-read")); + let tool = make_tool(Path::new("/tmp/never-read")); let schema = tool.parameters_schema(); assert_eq!(schema["type"], "object"); let required = schema["required"].as_array().expect("required is array"); @@ -53,13 +73,13 @@ fn parameters_schema_shape_matches_contract() { #[test] fn permission_level_is_write() { - let tool = PresentationTool::new(PathBuf::from("/tmp/never-read")); + let tool = make_tool(Path::new("/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 tool = make_tool(Path::new("/tmp/never-read")); let desc = tool.description(); assert!(desc.contains("USE THIS")); assert!(desc.contains("NOT for")); @@ -69,7 +89,7 @@ fn description_includes_router_rules() { #[tokio::test] async fn execute_rejects_empty_title() { let ws = workspace(); - let tool = PresentationTool::new(ws.path().to_path_buf()); + let tool = make_tool(ws.path()); let args = json!({ "title": "", "slides": [{ "title": "x", "bullets": ["y"] }] }); let result = tool.execute(args).await.expect("execute returns Ok"); assert!(result.is_error); @@ -79,7 +99,7 @@ async fn execute_rejects_empty_title() { #[tokio::test] async fn execute_rejects_empty_slides_array() { let ws = workspace(); - let tool = PresentationTool::new(ws.path().to_path_buf()); + let tool = make_tool(ws.path()); let args = json!({ "title": "Deck", "slides": [] }); let result = tool.execute(args).await.expect("execute returns Ok"); assert!(result.is_error); @@ -89,7 +109,7 @@ async fn execute_rejects_empty_slides_array() { #[tokio::test] async fn execute_rejects_slide_with_no_content() { let ws = workspace(); - let tool = PresentationTool::new(ws.path().to_path_buf()); + let tool = make_tool(ws.path()); let args = json!({ "title": "Deck", "slides": [{ "title": "", "body": "", "bullets": [], "speaker_notes": "" }] @@ -101,7 +121,7 @@ async fn execute_rejects_slide_with_no_content() { #[tokio::test] async fn execute_rejects_oversize_body() { let ws = workspace(); - let tool = PresentationTool::new(ws.path().to_path_buf()); + let tool = make_tool(ws.path()); let big = "x".repeat(MAX_TEXT_CHARS + 1); let args = json!({ "title": "Deck", @@ -114,7 +134,7 @@ async fn execute_rejects_oversize_body() { #[tokio::test] async fn execute_rejects_too_many_slides() { let ws = workspace(); - let tool = PresentationTool::new(ws.path().to_path_buf()); + let tool = make_tool(ws.path()); let slides: Vec<_> = (0..(MAX_SLIDES + 1)) .map(|i| json!({ "title": format!("Slide {i}"), "bullets": ["x"] })) .collect(); @@ -131,7 +151,7 @@ async fn execute_happy_path_returns_artifact_metadata() { // 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 tool = make_tool(ws.path()); let result = tool .execute(minimal_input_json()) .await @@ -175,6 +195,174 @@ async fn execute_happy_path_returns_artifact_metadata() { assert!(md.contains("1-slide")); } +/// Canonical 1×1 PNG, written to disk to exercise the File source path. +fn png_1x1() -> Vec { + use base64::{engine::general_purpose::STANDARD, Engine as _}; + STANDARD + .decode("iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg==") + .unwrap() +} + +/// Pull the JSON payload out of a tool result. +fn payload_of(result: &ToolResult) -> serde_json::Value { + match result.content.first().expect("a content block") { + crate::openhuman::skills::types::ToolContent::Json { data } => data.clone(), + other => panic!("expected Json content block, got {other:?}"), + } +} + +/// Read the entry names of the produced .pptx artifact. +fn pptx_entry_names(artifact_path: &str) -> Vec { + let bytes = std::fs::read(artifact_path).expect("artifact file readable"); + let cursor = std::io::Cursor::new(bytes); + let mut zip = zip::ZipArchive::new(cursor).expect("artifact is a valid zip"); + (0..zip.len()) + .map(|i| zip.by_index(i).unwrap().name().to_string()) + .collect() +} + +#[tokio::test] +async fn execute_embeds_file_image_into_deck() { + let ws = workspace(); + let img_path = ws.path().join("chart.png"); + std::fs::write(&img_path, png_1x1()).expect("write png"); + + let tool = make_tool(ws.path()); + let args = json!({ + "title": "Deck with image", + "slides": [{ + "title": "Chart", + "bullets": ["See below"], + "images": [{ + "source": { "type": "file", "path": img_path.to_string_lossy() }, + "caption": "Quarterly revenue" + }] + }] + }); + let result = tool.execute(args).await.expect("execute returns Ok"); + assert!(!result.is_error, "valid image should not error the deck"); + + let payload = payload_of(&result); + assert!( + payload["image_warnings"] + .as_array() + .map(|a| a.is_empty()) + .unwrap_or(true), + "no warnings expected for a valid PNG: {:?}", + payload["image_warnings"] + ); + + let names = pptx_entry_names(payload["artifact_path"].as_str().unwrap()); + assert!( + names.iter().any(|n| n == "ppt/media/image1.png"), + "embedded PNG missing from artifact (got: {names:?})" + ); +} + +#[tokio::test] +async fn execute_skips_unsupported_mime_image_with_warning() { + let ws = workspace(); + let txt_path = ws.path().join("notanimage.txt"); + std::fs::write(&txt_path, b"i am plain text, not an image").expect("write txt"); + + let tool = make_tool(ws.path()); + let args = json!({ + "title": "Deck", + "slides": [{ + "title": "Slide", + "bullets": ["text"], + "images": [{ "source": { "type": "file", "path": txt_path.to_string_lossy() } }] + }] + }); + let result = tool.execute(args).await.expect("execute returns Ok"); + // Partial success: deck still produced, but the bad image is reported. + assert!(!result.is_error, "bad image must not fail the whole deck"); + let payload = payload_of(&result); + let warnings = payload["image_warnings"] + .as_array() + .expect("warnings array"); + assert_eq!(warnings.len(), 1, "exactly one image warning expected"); + assert!( + warnings[0] + .as_str() + .unwrap() + .contains("unsupported image type"), + "warning should name the MIME problem: {:?}", + warnings[0] + ); +} + +#[tokio::test] +async fn execute_skips_oversize_image_with_warning() { + let ws = workspace(); + let big_path = ws.path().join("huge.png"); + // 5 MB + 1 byte — exceeds the per-image cap. Caught at metadata stat + // before the bytes are pulled into memory. + std::fs::write(&big_path, vec![0u8; 5 * 1024 * 1024 + 1]).expect("write big file"); + + let tool = make_tool(ws.path()); + let args = json!({ + "title": "Deck", + "slides": [{ + "title": "Slide", + "bullets": ["text"], + "images": [{ "source": { "type": "file", "path": big_path.to_string_lossy() } }] + }] + }); + let result = tool.execute(args).await.expect("execute returns Ok"); + assert!(!result.is_error); + let payload = payload_of(&result); + let warnings = payload["image_warnings"] + .as_array() + .expect("warnings array"); + assert_eq!(warnings.len(), 1); + assert!( + warnings[0].as_str().unwrap().contains("cap"), + "warning should mention the size cap: {:?}", + warnings[0] + ); +} + +#[tokio::test] +async fn execute_skips_missing_artifact_with_warning() { + let ws = workspace(); + let tool = make_tool(ws.path()); + let args = json!({ + "title": "Deck", + "slides": [{ + "title": "Slide", + "bullets": ["text"], + "images": [{ "source": { "type": "artifact", "artifact_id": "does-not-exist" } }] + }] + }); + let result = tool.execute(args).await.expect("execute returns Ok"); + assert!(!result.is_error); + let payload = payload_of(&result); + let warnings = payload["image_warnings"] + .as_array() + .expect("warnings array"); + assert_eq!(warnings.len(), 1); + assert!(warnings[0].as_str().unwrap().contains("unreadable")); +} + +#[tokio::test] +async fn execute_rejects_too_many_images_per_deck() { + let ws = workspace(); + let tool = make_tool(ws.path()); + // 9 images total across slides — exceeds the deck cap of 8. This is a + // hard validation reject (cheap structural check), not a skip. + let images: Vec<_> = (0..9) + .map(|i| json!({ "source": { "type": "artifact", "artifact_id": format!("a{i}") } })) + .collect(); + let args = json!({ + "title": "Deck", + "slides": [{ "title": "Slide", "bullets": ["x"], "images": images }] + }); + let result = tool.execute(args).await.expect("execute returns Ok"); + assert!(result.is_error, "9 images should be rejected outright"); + assert!(result.text().contains("images")); +} + #[test] fn truncate_stderr_caps_payload_with_suffix() { let raw = "y".repeat(2000); diff --git a/src/openhuman/tools/impl/presentation/types.rs b/src/openhuman/tools/impl/presentation/types.rs index ddc6b84ab..6a242ba94 100644 --- a/src/openhuman/tools/impl/presentation/types.rs +++ b/src/openhuman/tools/impl/presentation/types.rs @@ -17,6 +17,50 @@ pub(super) const MAX_TEXT_CHARS: usize = 2_000; /// unreadable slides and bloat the output file. pub(super) const MAX_BULLETS_PER_SLIDE: usize = 32; +/// Maximum number of images attached to a single slide. The v1 +/// single-column layout stacks images vertically in the lower band of +/// the slide; beyond this count each image is too small to read. +pub(super) const MAX_IMAGES_PER_SLIDE: usize = 6; + +/// Maximum number of images across the whole deck. Bounds the embedded +/// media payload (and therefore the artifact size) regardless of how +/// the images are distributed across slides. +pub(super) const MAX_IMAGES_PER_DECK: usize = 8; + +/// Per-image byte cap. Mirrors the multimodal pipeline's image ceiling +/// (`agent::multimodal`) so a single oversized asset cannot balloon the +/// deck or stall generation. Enforced at resolution time (once the +/// bytes are in hand), not at schema-validation time. +pub(super) const MAX_IMAGE_BYTES: usize = 5 * 1024 * 1024; + +/// Where a slide image's bytes come from. `Url` is intentionally +/// **deferred** in v1 — fetching agent-supplied URLs at generation time +/// is an SSRF surface that needs its own all- / deny-list design. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(tag = "type", rename_all = "snake_case")] +pub enum SlideImageSource { + /// Bytes from a workspace artifact (e.g. a chart the agent produced + /// earlier via another tool). Resolved through + /// [`crate::openhuman::artifacts::read_artifact_bytes`]. + Artifact { artifact_id: String }, + /// Bytes from a local filesystem path the agent already has read + /// access to (e.g. a screenshot saved under the action dir). + File { path: String }, +} + +/// One image attached to a slide. `caption`, when present, is rendered +/// as a trailing text bullet beneath the image in v1 (a true +/// image-anchored caption shape is deferred with the grid layout). +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct SlideImage { + /// Image data source. See [`SlideImageSource`]. + pub source: SlideImageSource, + /// Optional caption rendered as a bullet under the image. + #[serde(default)] + pub caption: Option, +} + /// Slide spec — one entry per content slide in the generated deck. #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(deny_unknown_fields)] @@ -36,6 +80,25 @@ pub struct SlideSpec { /// Speaker notes attached to the slide. #[serde(default)] pub speaker_notes: Option, + /// Images attached to the slide, rendered single-column beneath the + /// text. Resolved + validated (MIME / size / dimensions) at the + /// async `execute()` boundary; a bad image is skipped with a + /// warning rather than failing the whole deck. + #[serde(default)] + pub images: Vec, +} + +/// An image resolved + validated at the async boundary, ready for the +/// (synchronous, pure) engine to embed. Carries the decoded bytes, the +/// `ppt-rs` format token (`"PNG"` / `"JPEG"`), and the native pixel +/// dimensions used for aspect-preserving placement. +#[derive(Debug, Clone)] +pub(super) struct ResolvedSlideImage { + pub bytes: Vec, + pub format: &'static str, + pub width_px: u32, + pub height_px: u32, + pub caption: Option, } /// Top-level input for the `generate_presentation` tool. @@ -73,6 +136,12 @@ pub struct GeneratePresentationOutput { pub slide_count: usize, /// On-disk size of the produced `.pptx` in bytes. pub size_bytes: u64, + /// Per-image warnings for assets that could not be embedded (bad + /// MIME, oversize, unreadable, undecodable dimensions). The deck is + /// still produced without those images — partial success rather than + /// a hard failure. Empty when every image resolved cleanly. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub image_warnings: Vec, } /// Structured error variants surfaced to the agent. Aligned with the @@ -168,6 +237,13 @@ pub(super) fn validate_input(input: &GeneratePresentationInput) -> Result<(), Pr reason: format!("must contain ≤ {MAX_SLIDES} slides"), }); } + let total_images: usize = input.slides.iter().map(|s| s.images.len()).sum(); + if total_images > MAX_IMAGES_PER_DECK { + return Err(PresentationError::InvalidInput { + field: "slides[].images".to_string(), + reason: format!("deck must contain ≤ {MAX_IMAGES_PER_DECK} images total"), + }); + } for (i, slide) in input.slides.iter().enumerate() { let has_title = !slide.title.trim().is_empty(); let has_body = slide @@ -221,6 +297,45 @@ pub(super) fn validate_input(input: &GeneratePresentationInput) -> Result<(), Pr }); } } + if slide.images.len() > MAX_IMAGES_PER_SLIDE { + return Err(PresentationError::InvalidInput { + field: format!("slides[{i}].images"), + reason: format!("must contain ≤ {MAX_IMAGES_PER_SLIDE} images"), + }); + } + for (img_idx, image) in slide.images.iter().enumerate() { + // Cheap structural checks only — MIME, size, and decodability + // are validated at resolution time when the bytes are in hand + // (and a failure there is a skip-with-warning, not a hard + // reject). Here we only catch malformed specs the agent can + // self-correct without touching the filesystem. + match &image.source { + SlideImageSource::Artifact { artifact_id } => { + if artifact_id.trim().is_empty() { + return Err(PresentationError::InvalidInput { + field: format!("slides[{i}].images[{img_idx}].source.artifact_id"), + reason: "must not be empty".to_string(), + }); + } + } + SlideImageSource::File { path } => { + if path.trim().is_empty() { + return Err(PresentationError::InvalidInput { + field: format!("slides[{i}].images[{img_idx}].source.path"), + reason: "must not be empty".to_string(), + }); + } + } + } + if let Some(caption) = image.caption.as_deref() { + if caption.chars().count() > MAX_TEXT_CHARS { + return Err(PresentationError::InvalidInput { + field: format!("slides[{i}].images[{img_idx}].caption"), + reason: format!("must be ≤ {MAX_TEXT_CHARS} chars"), + }); + } + } + } } Ok(()) } diff --git a/src/openhuman/tools/ops.rs b/src/openhuman/tools/ops.rs index b1ff14009..8c9f2e810 100644 --- a/src/openhuman/tools/ops.rs +++ b/src/openhuman/tools/ops.rs @@ -465,6 +465,7 @@ pub fn all_tools_with_runtime( // registered. tools.push(Box::new(PresentationTool::new( root_config.workspace_dir.clone(), + security.clone(), ))); if browser_config.enabled {