diff --git a/src/openhuman/agent_registry/agents/orchestrator/prompt.md b/src/openhuman/agent_registry/agents/orchestrator/prompt.md index d7ae7ea0f1..e896f40ec4 100644 --- a/src/openhuman/agent_registry/agents/orchestrator/prompt.md +++ b/src/openhuman/agent_registry/agents/orchestrator/prompt.md @@ -191,6 +191,27 @@ Modes: Start cheap (`query_source` / `smart_walk` summaries), only drill_down/fetch_leaves when you need verbatim content. +## Presentation generation + +`generate_presentation` builds a `.pptx` deck from a structured slide spec via a native Rust engine (`ppt-rs`) running in-process — no Python subprocess, no managed venv. Use it for any "make slides", "build a deck", "draft a presentation", "create a pitch" request. + +**Grounding rule (do not skip).** Before calling `generate_presentation` on a topical or factual deck — anything where the slides need real-world facts, current events, statistics, names, dates, quotes, or domain context — you MUST first establish a grounding context. Pick at least one: + +- `memory_tree` (`query_source` / `smart_walk`) — when the topic plausibly lives in the user's ingested history (their notes, prior chats, emails on the subject). +- `research` — when the topic needs live web facts (current events, recent stats, comparative product data, anything time-sensitive). +- `query_memory` — when the user has previously summarised the exact topic in this thread or in a saved memory. + +Only after the grounding tool returns may you call `generate_presentation`, and the slide bullets / body / speaker_notes you pass MUST be drawn from the grounding output — not invented from priors. + +**When to skip grounding.** You may dispatch `generate_presentation` directly when: + +- The user pasted source material in the same turn (text, doc summary, bullet list to convert). +- A prior turn in this same thread already established the source material (and you can quote from it). +- The deck is content-free or structural (e.g. "make me a 3-slide blank template titled 'Q3 Review'", "an empty deck with a title slide and two body slides"). +- The user explicitly waived grounding ("don't research, just generate from your priors", "I know it'll be approximate"). + +**Why this rule exists.** Without grounding, the model invents slide bullets and speaker notes from training-data priors. That confabulates statistics, misattributes quotes, and ages out fast. A single `research` or `memory_tree` call up front grounds the deck in verifiable sources and lets the slides cite real material instead of fabricated text. + ## Citations When your answer is informed by retrieved memory, cite it with footnote markers: diff --git a/src/openhuman/agent_registry/agents/orchestrator/prompt.rs b/src/openhuman/agent_registry/agents/orchestrator/prompt.rs index 08cf5a2536..3a41bfb2da 100644 --- a/src/openhuman/agent_registry/agents/orchestrator/prompt.rs +++ b/src/openhuman/agent_registry/agents/orchestrator/prompt.rs @@ -338,6 +338,74 @@ mod tests { assert!(!body.contains("- **linear**")); } + #[test] + fn build_requires_grounding_before_generate_presentation() { + // #2780 grounding rule: orchestrator must ground a topical/factual + // deck via `memory_tree` / `research` / `query_memory` before + // dispatching `generate_presentation`. The rule is expressed in + // `prompt.md` and must reach the assembled body verbatim — the + // test pins both the section header and the load-bearing "do + // not skip" instruction so a future prompt refactor can't + // quietly drop the guardrail. The live-web grounding option is + // named `research` (the researcher agent's `delegate_name` + // override, not the default `delegate_researcher` / + // `delegate_research`); it must match the actual tool name the + // orchestrator exposes so the model can dispatch it instead of + // hallucinating a non-tool. + let body = build(&ctx_with(&[])).unwrap(); + assert!( + body.contains("## Presentation generation"), + "presentation section header missing from orchestrator prompt" + ); + assert!( + body.contains("Grounding rule (do not skip)"), + "grounding rule missing from presentation section" + ); + assert!( + body.contains("Before calling `generate_presentation`"), + "grounding pre-condition phrasing missing from presentation section" + ); + // Each of the three permitted grounding tools must be named so + // the model knows the menu, not just the constraint. + assert!(body.contains("`memory_tree`")); + assert!( + body.contains("`research`"), + "grounding rule must name the researcher tool by its actual \ + delegate_name (`research`), not `delegate_research`" + ); + assert!( + !body.contains("`delegate_research`"), + "stale `delegate_research` reference — researcher's \ + delegate_name override is `research`" + ); + assert!(body.contains("`query_memory`")); + // All four waiver escape hatches must reach the assembled body + // verbatim — without each one the rule blocks a legitimate + // dispatch case. Per graycyrus on #3029: pinning only one waiver + // lets a future prompt edit silently drop or expand the list and + // the safety-critical guardrail decays unnoticed. + // (1) user pasted source material in the same turn. + assert!( + body.contains("user pasted source material"), + "waiver clause for pasted-source decks missing" + ); + // (2) prior turn in same thread already established source material. + assert!( + body.contains("prior turn in this same thread"), + "waiver clause for prior-turn established material missing" + ); + // (3) structural / content-free deck (blank template, layout-only). + assert!( + body.contains("content-free or structural"), + "waiver clause for structural / content-free decks missing" + ); + // (4) explicit user waiver ("don't research, just generate…"). + assert!( + body.contains("explicitly waived grounding"), + "waiver clause for explicit user waiver missing" + ); + } + #[test] fn build_omits_guide_when_no_integrations_connected() { let integrations = vec![ConnectedIntegration { diff --git a/src/openhuman/tools/impl/presentation/tests.rs b/src/openhuman/tools/impl/presentation/tests.rs index a8451f9c7b..9e92ba4c19 100644 --- a/src/openhuman/tools/impl/presentation/tests.rs +++ b/src/openhuman/tools/impl/presentation/tests.rs @@ -184,3 +184,19 @@ fn truncate_stderr_caps_payload_with_suffix() { let short = "tiny stderr"; assert_eq!(PresentationError::truncate_stderr(short), short); } + +#[test] +fn unsupported_file_type_display_includes_extension_and_supported() { + // Reserved-for-future variant (#2780): confirms the Display impl + // renders both the rejected extension and the supported set so a + // downstream mapper can surface a user-correctable message verbatim + // once a `format` selector lands. + let err = PresentationError::UnsupportedFileType { + extension: "key".to_string(), + supported: "pptx".to_string(), + }; + let rendered = err.to_string(); + assert!(rendered.contains("unsupported file type")); + assert!(rendered.contains("'key'")); + assert!(rendered.contains("supported: pptx")); +} diff --git a/src/openhuman/tools/impl/presentation/types.rs b/src/openhuman/tools/impl/presentation/types.rs index 10845f6c1f..ddc6b84ab3 100644 --- a/src/openhuman/tools/impl/presentation/types.rs +++ b/src/openhuman/tools/impl/presentation/types.rs @@ -90,6 +90,19 @@ pub enum PresentationError { #[error("presentation generation exceeded {timeout_secs}s timeout")] GenerationTimeout { timeout_secs: u64 }, + + /// Reserved for the planned `format` selector that will let callers + /// request alternative deck formats (`.pdf` / `.key` / image + /// strips). Today the tool only emits `.pptx`, so this variant is + /// not constructed by `execute` — it exists ahead of #2780's + /// follow-up wiring so downstream error-handling sites can pattern- + /// match exhaustively without a churn-y enum bump later. + #[allow(dead_code)] + #[error("unsupported file type '{extension}'; supported: {supported}")] + UnsupportedFileType { + extension: String, + supported: String, + }, } impl PresentationError { @@ -111,9 +124,9 @@ impl PresentationError { } } -/// 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. +/// Validate the input early — before invoking the `ppt-rs` engine — so +/// the agent gets a structured `InvalidInput` it can self-correct on +/// instead of a generic engine error. pub(super) fn validate_input(input: &GeneratePresentationInput) -> Result<(), PresentationError> { if input.title.trim().is_empty() { return Err(PresentationError::InvalidInput { diff --git a/tests/orchestrator_presentation_wiring.rs b/tests/orchestrator_presentation_wiring.rs new file mode 100644 index 0000000000..925a453af0 --- /dev/null +++ b/tests/orchestrator_presentation_wiring.rs @@ -0,0 +1,58 @@ +//! Pins the orchestrator's `generate_presentation` wiring contract +//! (#2780 — companion to the tool itself shipped in #2778). +//! +//! Two invariants: +//! +//! 1. The orchestrator's `agent.toml` MUST list `generate_presentation` +//! in `[tools].named`. Without this entry the orchestrator's +//! function-calling schema does not include the tool and the +//! "create slides" routing case from parent epic #1535 silently +//! falls back to refusing the request. +//! +//! 2. The `code_executor` agent MUST NOT list `generate_presentation`. +//! Presentation rendering is not a code-exec task: it runs in-process +//! via the native Rust `ppt-rs` engine (no Python, no subprocess, +//! distinct from code_executor's `node_exec` / shell surface), and +//! exposing the tool to code_executor would create a second, +//! duplicate dispatch path that bypasses the orchestrator's +//! grounding-rule prompt. +//! +//! Exact-line matching (not substring) so commented-out entries or +//! prefixed names (`generate_presentation_v2`, `generate_presentation_legacy`) +//! cannot satisfy the assertion accidentally. + +const ORCHESTRATOR_TOML: &str = + include_str!("../src/openhuman/agent_registry/agents/orchestrator/agent.toml"); + +const CODE_EXECUTOR_TOML: &str = + include_str!("../src/openhuman/agent_registry/agents/code_executor/agent.toml"); + +const TOOL_NAME: &str = "generate_presentation"; + +fn lists_named_tool(toml: &str, name: &str) -> bool { + let bare = format!("\"{name}\""); + let trailing = format!("\"{name}\","); + toml.lines() + .map(str::trim) + .any(|line| line == bare || line == trailing) +} + +#[test] +fn orchestrator_lists_generate_presentation() { + assert!( + lists_named_tool(ORCHESTRATOR_TOML, TOOL_NAME), + "orchestrator agent.toml must list '{TOOL_NAME}' as a named tool entry — \ + removing it silently disables the 'create slides' routing case (#1535)" + ); +} + +#[test] +fn code_executor_does_not_list_generate_presentation() { + assert!( + !lists_named_tool(CODE_EXECUTOR_TOML, TOOL_NAME), + "code_executor agent.toml must NOT list '{TOOL_NAME}' — pptx rendering \ + is not a code-exec task; it runs in-process via the native Rust ppt-rs \ + engine and adding it here would bypass the orchestrator grounding-rule \ + prompt (#2780)" + ); +}