feat(presentation): embed images in generated decks (#3209)#3299
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds per-slide image support to generate_presentation: new types/limits, artifact/file byte resolution, PNG/JPEG sniffing and dimension extraction, engine layout and embedding with aspect-ratio scaling, SecurityPolicy wiring, re-exported artifact byte helper, updated tests, and orchestrator prompt docs. ChangesImage Embedding for Presentations
Sequence Diagram(s)sequenceDiagram
participant Agent
participant Tool as PresentationTool
participant ImageRes as resolve_images
participant ArtifactStore as artifacts::store
participant ImageUtil as image_util
participant Engine as engine::generate
participant PPTRs as ppt-rs
Agent->>Tool: execute(input with slide images)
Tool->>ImageRes: resolve_images per slide
ImageRes->>ArtifactStore: read_artifact_bytes (artifact source)
ImageRes->>Tool: validate file path via SecurityPolicy (file source)
ImageRes->>ImageUtil: sniff_format(bytes)
ImageUtil-->>ImageRes: PNG or JPEG
ImageRes->>ImageUtil: pixel_dimensions(bytes, format)
ImageUtil-->>ImageRes: (width, height)
ImageRes-->>Tool: Vec<Vec<ResolvedSlideImage>> (skip failures with warnings)
Tool->>Engine: generate(input, resolved_images, deadline)
Engine->>Engine: build_slides (attach images)
Engine->>Engine: place_single_column, fit_within (layout & scale)
Engine->>PPTRs: Image::from_bytes(...).add_to(slide)
PPTRs-->>Engine: slide with embedded image
Engine-->>Tool: pptx bytes
Tool-->>Agent: result (artifact, image_warnings)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/openhuman/tools/impl/presentation/mod.rs`:
- Around line 366-381: The File branch in SlideImageSource::File currently reads
arbitrary filesystem paths without validation; before calling
tokio::fs::metadata or tokio::fs::read, run the path through the existing Rust
validators (is_path_string_allowed and/or validate_path) and return an Err if
validation fails; use the same `path` variable and produce a clear error string
(e.g., "file {path} not allowed" or the validate_path message) so no filesystem
I/O occurs for disallowed paths and permitted paths proceed to metadata/read as
before.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 010027c5-ddac-483b-bfa4-43cad0dd9027
📒 Files selected for processing (8)
src/openhuman/agent_registry/agents/orchestrator/prompt.mdsrc/openhuman/artifacts/mod.rssrc/openhuman/artifacts/store.rssrc/openhuman/tools/impl/presentation/engine.rssrc/openhuman/tools/impl/presentation/image_util.rssrc/openhuman/tools/impl/presentation/mod.rssrc/openhuman/tools/impl/presentation/tests.rssrc/openhuman/tools/impl/presentation/types.rs
…humansai#3209) CodeRabbit: File-source image reads bypassed the path-permission model. Route agent-supplied paths through SecurityPolicy::validate_path (inject Arc<SecurityPolicy> at construction, matching the browser/image_info tools) so a path like /etc/shadow or ~/.ssh/id_rsa is rejected before any I/O.
Summary
generate_presentationnow embeds images in slides, not just text — each slide accepts animagesarray alongsidetitle/body/bullets.artifact(bytes from a prior tool's output) andfile(a local path). Remote URLs are deferred (SSRF surface).ppt-rs'sSlideContent::add_image.image_warnings) and the markdown reply.artifacts::read_artifact_bytesis the single source of truth for resolving an artifact id → on-disk bytes.imagesarg and a grounding rule (don't claim what an image shows without verifying it).Problem
Decks produced by
generate_presentation(added in #3016, parent #2778) were text-only. Real presentations need charts, screenshots, and diagrams.ppt-rs0.2.14 can embed images, so the agent should be able to attach them — sourced from artifacts it produced earlier or from local files — without a separate editing step.Solution
types.rs):SlideImage { source, caption }withSlideImageSource::{Artifact{artifact_id}, File{path}}(internally tagged). New per-deck / per-slide / per-image caps.#[serde(deny_unknown_fields)]preserved.mod.rs::resolve_images): artifact →read_artifact_bytes; file → size-checked local read. Validated via a self-contained PNG/JPEG sniffer + pixel-dimension reader (image_util.rs). Failures become skip-warnings, never hard errors.engine.rs): resolved(bytes, format, dims)are scaled aspect-preserving and stacked single-column in the slide's lower band, attached after the text. Caption renders as a trailing bullet.agent::multimodal's helpers: those are private + outside this change's boundary, they accept webp/gif/bmp (whichppt-rs0.2.14 cannot embed — no webp Content-Type default,automisclassifies), and they don't expose pixel dimensions (needed for placement). v1 therefore restricts embeddable formats to PNG + JPEG.Scope cuts (stated, not silent): URL source deferred (SSRF); webp/gif deferred (ppt-rs can't embed safely in 0.2.14); single-column layout only (multi-image grid deferred). No
ppt-rsbump needed.Submission Checklist
ppt/media/image1.png+ Content-Types), and tool-level skip-with-warning + cap-reject paths.read_artifact_bytes). CIdiff-coveris authoritative.generate_presentationfeature; no new/removed/renamed feature row.Closes #3209in the## Relatedsection.Impact
generate_presentation); nosrc-tauri/ frontend change. Runs in-process like before.artifactspath validation.imagesdefaults to empty, so existing text-only callers and the tool's input/output schema for non-image use are unchanged.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
feat/3209-presentation-images0754dc66908b62fe1371dd4cfa6cc433f0aaeff7Validation Run
pnpm --filter openhuman-app format:check— no frontend changes.pnpm typecheck— no frontend changes.cargo test --lib openhuman::tools::implementations::presentation→ 28 passed, 0 failed.cargo fmt --checkclean,cargo clippy -p openhuman --all-targets -- -D warningsclean,cargo check -p openhumanclean.app/src-taurichanges.Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
generate_presentationcan now embed PNG/JPEG images per slide from artifact or file sources..pptxdecks contain images beneath slide text; skipped images are reported back to the agent/user.Parity Contract
slide_countsemantics unchanged).Duplicate / Superseded PR Handling
Summary by CodeRabbit