diff --git a/Cargo.lock b/Cargo.lock index d9c31a6186..9240b0bc7a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1022,6 +1022,12 @@ dependencies = [ "thiserror 1.0.69", ] +[[package]] +name = "color_quant" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3d7b894f5411737b7867f4827955924d7c254fc9f4d91a6aad6b097804b1018b" + [[package]] name = "colorchoice" version = "1.0.5" @@ -1698,6 +1704,21 @@ dependencies = [ "litrs", ] +[[package]] +name = "docx-rs" +version = "0.4.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed73cbf5e1c37baa23f4132569ac1187829f03922c206bd68fe109e3001a343d" +dependencies = [ + "base64 0.22.1", + "image", + "quick-xml 0.36.2", + "serde", + "serde_json", + "thiserror 2.0.18", + "zip 0.6.6", +] + [[package]] name = "dotenvy" version = "0.15.7" @@ -2512,6 +2533,16 @@ dependencies = [ "polyval 0.7.1", ] +[[package]] +name = "gif" +version = "0.14.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ee8cfcc411d9adbbaba82fb72661cc1bcca13e8bba98b364e62b2dba8f960159" +dependencies = [ + "color_quant", + "weezl", +] + [[package]] name = "gimli" version = "0.32.3" @@ -2863,7 +2894,7 @@ dependencies = [ "js-sys", "log", "wasm-bindgen", - "windows-core 0.58.0", + "windows-core 0.62.2", ] [[package]] @@ -3066,6 +3097,8 @@ checksum = "85ab80394333c02fe689eaf900ab500fbd0c2213da414687ebf995a65d5a6104" dependencies = [ "bytemuck", "byteorder-lite", + "color_quant", + "gif", "moxcms", "num-traits", "png", @@ -4369,6 +4402,7 @@ dependencies = [ "dialoguer", "directories", "dirs 5.0.1", + "docx-rs", "dotenvy", "ed25519-dalek", "enigo", @@ -4866,7 +4900,7 @@ checksum = "740ebea15c5d1428f910cd1a5f52cebf8d25006245ed8ade92702f4943d91e07" dependencies = [ "base64 0.22.1", "indexmap", - "quick-xml", + "quick-xml 0.38.4", "serde", "time", ] @@ -5191,6 +5225,16 @@ version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a993555f31e5a609f617c12db6250dedcac1b0a85076912c436e6fc9b2c8e6a3" +[[package]] +name = "quick-xml" +version = "0.36.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f7649a7b4df05aed9ea7ec6f628c67c9953a43869b8bc50929569b2999d443fe" +dependencies = [ + "encoding_rs", + "memchr", +] + [[package]] name = "quick-xml" version = "0.38.4" @@ -8277,7 +8321,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.61.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index e85b7ca481..7e6061d62b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -268,6 +268,13 @@ pdf-extract = "0.10" # stack) now lives in the tinychannels crate; the `whatsapp-web` feature forwards # to `tinychannels/whatsapp-web`. ppt-rs = "0.2.14" +# Native-Rust `.docx` writer for the `generate_document` tool (GH #4847). +# Pure Rust (no subprocess / managed runtime), MIT-licensed, actively +# maintained (2.8M downloads, last release 0.4.20 — Apr 2026). Mirrors the +# `ppt-rs` presentation engine: synthesise bytes in-process, hand them to +# the byte-agnostic artifact pipeline. `default-features` keeps the crate's +# image support (unused here, but avoids a bespoke feature list drifting). +docx-rs = "0.4.20" [target.'cfg(windows)'.dependencies] # Windows: tokio-tungstenite uses native-tls (schannel) so wss:// diff --git a/app/src-tauri/Cargo.lock b/app/src-tauri/Cargo.lock index ee102721b5..9604cdc772 100644 --- a/app/src-tauri/Cargo.lock +++ b/app/src-tauri/Cargo.lock @@ -250,7 +250,7 @@ dependencies = [ "objc2-foundation 0.3.2", "parking_lot", "percent-encoding", - "windows-sys 0.59.0", + "windows-sys 0.60.2", "x11rb", ] @@ -1335,6 +1335,12 @@ dependencies = [ "thiserror 1.0.69", ] +[[package]] +name = "color_quant" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3d7b894f5411737b7867f4827955924d7c254fc9f4d91a6aad6b097804b1018b" + [[package]] name = "colorchoice" version = "1.0.5" @@ -2093,7 +2099,7 @@ dependencies = [ "libc", "option-ext", "redox_users 0.5.2", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -2169,6 +2175,21 @@ dependencies = [ "litrs", ] +[[package]] +name = "docx-rs" +version = "0.4.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ed73cbf5e1c37baa23f4132569ac1187829f03922c206bd68fe109e3001a343d" +dependencies = [ + "base64 0.22.1", + "image", + "quick-xml 0.36.2", + "serde", + "serde_json", + "thiserror 2.0.18", + "zip 0.6.6", +] + [[package]] name = "dom_query" version = "0.27.0" @@ -2496,7 +2517,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -3199,6 +3220,16 @@ dependencies = [ "polyval", ] +[[package]] +name = "gif" +version = "0.14.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ee8cfcc411d9adbbaba82fb72661cc1bcca13e8bba98b364e62b2dba8f960159" +dependencies = [ + "color_quant", + "weezl", +] + [[package]] name = "gimli" version = "0.32.3" @@ -3731,7 +3762,7 @@ dependencies = [ "js-sys", "log", "wasm-bindgen", - "windows-core 0.58.0", + "windows-core 0.62.2", ] [[package]] @@ -3876,6 +3907,8 @@ checksum = "85ab80394333c02fe689eaf900ab500fbd0c2213da414687ebf995a65d5a6104" dependencies = [ "bytemuck", "byteorder-lite", + "color_quant", + "gif", "moxcms", "num-traits", "png 0.18.1", @@ -4130,7 +4163,7 @@ dependencies = [ "portable-atomic", "portable-atomic-util", "serde_core", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -5024,7 +5057,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -5592,6 +5625,7 @@ dependencies = [ "dialoguer", "directories 6.0.0", "dirs 5.0.1", + "docx-rs", "dotenvy", "ed25519-dalek", "enigo", @@ -6639,6 +6673,16 @@ version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a993555f31e5a609f617c12db6250dedcac1b0a85076912c436e6fc9b2c8e6a3" +[[package]] +name = "quick-xml" +version = "0.36.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f7649a7b4df05aed9ea7ec6f628c67c9953a43869b8bc50929569b2999d443fe" +dependencies = [ + "encoding_rs", + "memchr", +] + [[package]] name = "quick-xml" version = "0.37.5" @@ -6719,7 +6763,7 @@ dependencies = [ "once_cell", "socket2", "tracing", - "windows-sys 0.59.0", + "windows-sys 0.60.2", ] [[package]] @@ -7306,7 +7350,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -7365,7 +7409,7 @@ dependencies = [ "security-framework 3.7.0", "security-framework-sys", "webpki-root-certs", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -8991,7 +9035,7 @@ dependencies = [ "getrandom 0.4.2", "once_cell", "rustix", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -10634,7 +10678,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.61.2", ] [[package]] diff --git a/app/src/components/chat/ArtifactCard.tsx b/app/src/components/chat/ArtifactCard.tsx index 540420aa4e..d5ff4b0168 100644 --- a/app/src/components/chat/ArtifactCard.tsx +++ b/app/src/components/chat/ArtifactCard.tsx @@ -7,6 +7,7 @@ import { saveArtifactViaDialog, } from '../../services/artifactDownloadService'; import type { ArtifactSnapshot } from '../../store/chatRuntimeSlice'; +import { extensionFor } from './artifactExtension'; /** * Inline chat card surfacing a single agent-generated artifact (#2779). @@ -33,30 +34,6 @@ export interface ArtifactCardProps { onRetry?: (artifactId: string) => void; } -/** - * Extension hint for the Tauri download command. Falls back to a - * lowercased kind slug when the title doesn't carry an explicit - * extension (defensive — `create_artifact` sanitises the title + - * extension separately, but a malformed title shouldn't crash the - * card). - */ -function extensionFor(kind: ArtifactSnapshot['kind'], title: string): string { - const dot = title.lastIndexOf('.'); - if (dot > 0 && dot < title.length - 1) { - return title.slice(dot + 1).toLowerCase(); - } - switch (kind) { - case 'presentation': - return 'pptx'; - case 'document': - return 'pdf'; - case 'image': - return 'png'; - default: - return 'bin'; - } -} - function KindIcon({ kind }: { kind: ArtifactSnapshot['kind'] }) { const stroke = 'currentColor'; switch (kind) { diff --git a/app/src/components/chat/ChatFilesPanel.tsx b/app/src/components/chat/ChatFilesPanel.tsx index 034ef17365..0bed3469d9 100644 --- a/app/src/components/chat/ChatFilesPanel.tsx +++ b/app/src/components/chat/ChatFilesPanel.tsx @@ -15,6 +15,7 @@ import { } from '../../store/chatRuntimeSlice'; import { useAppDispatch } from '../../store/hooks'; import Button from '../ui/Button'; +import { extensionFor } from './artifactExtension'; /** * Popover panel listing every `ready` artifact for a thread (#3024). @@ -76,23 +77,6 @@ function localizeErrorCode( } } -function extensionFor(kind: ArtifactSnapshot['kind'], title: string): string { - const dot = title.lastIndexOf('.'); - if (dot > 0 && dot < title.length - 1) { - return title.slice(dot + 1).toLowerCase(); - } - switch (kind) { - case 'presentation': - return 'pptx'; - case 'document': - return 'pdf'; - case 'image': - return 'png'; - default: - return 'bin'; - } -} - function KindIcon({ kind }: { kind: ArtifactSnapshot['kind'] }) { const stroke = 'currentColor'; switch (kind) { diff --git a/app/src/components/chat/__tests__/ArtifactCard.test.tsx b/app/src/components/chat/__tests__/ArtifactCard.test.tsx index b4c550c3bf..096048a568 100644 --- a/app/src/components/chat/__tests__/ArtifactCard.test.tsx +++ b/app/src/components/chat/__tests__/ArtifactCard.test.tsx @@ -130,7 +130,7 @@ describe('ArtifactCard', () => { }); it.each([ - ['document' as const, 'pdf'], + ['document' as const, 'docx'], ['image' as const, 'png'], ['other' as const, 'bin'], ['presentation' as const, 'pptx'], diff --git a/app/src/components/chat/__tests__/ChatFilesPanel.test.tsx b/app/src/components/chat/__tests__/ChatFilesPanel.test.tsx index a43deec115..d9d59d5ca3 100644 --- a/app/src/components/chat/__tests__/ChatFilesPanel.test.tsx +++ b/app/src/components/chat/__tests__/ChatFilesPanel.test.tsx @@ -262,7 +262,7 @@ describe('ChatFilesPanel', () => { }); it.each([ - ['document' as const, 'pdf'], + ['document' as const, 'docx'], ['image' as const, 'png'], ['other' as const, 'bin'], ])( diff --git a/app/src/components/chat/artifactExtension.test.ts b/app/src/components/chat/artifactExtension.test.ts new file mode 100644 index 0000000000..1b65954b42 --- /dev/null +++ b/app/src/components/chat/artifactExtension.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from 'vitest'; + +import { extensionFor } from './artifactExtension'; + +describe('extensionFor', () => { + it('maps document artifacts to docx (GH #4847, was pdf)', () => { + // The regression this guards: a `document` artifact carrying no + // explicit extension in its title must export as `.docx` — the + // format `generate_document` actually emits — not the stale `pdf`. + expect(extensionFor('document', 'Quarterly Report')).toBe('docx'); + }); + + it('maps the other known kinds to their default extensions', () => { + expect(extensionFor('presentation', 'Q3 Deck')).toBe('pptx'); + expect(extensionFor('image', 'Chart')).toBe('png'); + expect(extensionFor('other', 'Blob')).toBe('bin'); + }); + + it('prefers an explicit extension already present on the title', () => { + // The title-carried extension wins over the per-kind default, so a + // legacy pdf document still round-trips as pdf. + expect(extensionFor('document', 'legacy.pdf')).toBe('pdf'); + expect(extensionFor('document', 'notes.DOCX')).toBe('docx'); + expect(extensionFor('image', 'photo.jpeg')).toBe('jpeg'); + }); + + it('ignores a leading or trailing dot that is not a real extension', () => { + // No extension segment → fall back to the kind default. + expect(extensionFor('document', '.hidden')).toBe('docx'); + expect(extensionFor('document', 'trailing.')).toBe('docx'); + }); +}); diff --git a/app/src/components/chat/artifactExtension.ts b/app/src/components/chat/artifactExtension.ts new file mode 100644 index 0000000000..968161382a --- /dev/null +++ b/app/src/components/chat/artifactExtension.ts @@ -0,0 +1,36 @@ +import type { ArtifactSnapshot } from '../../store/chatRuntimeSlice'; + +/** + * Extension hint for the Tauri download / Save-As commands. + * + * Prefers an explicit extension already present on the artifact title + * (defensive — `create_artifact` sanitises the title + extension + * separately, but a malformed title shouldn't crash the card), otherwise + * falls back to a per-kind default. + * + * Shared by {@link ArtifactCard} and {@link ChatFilesPanel}, which both + * derive the download filename extension from `(kind, title)` and must + * stay in lockstep — hence a single source of truth rather than two + * hand-kept copies. + * + * `document` → `docx`: the document artifact producer (`generate_document`, + * GH #4847) emits a real Word `.docx`. The prior `pdf` default predated any + * document producer and would have handed the byte-agnostic export a wrong + * extension. + */ +export function extensionFor(kind: ArtifactSnapshot['kind'], title: string): string { + const dot = title.lastIndexOf('.'); + if (dot > 0 && dot < title.length - 1) { + return title.slice(dot + 1).toLowerCase(); + } + switch (kind) { + case 'presentation': + return 'pptx'; + case 'document': + return 'docx'; + case 'image': + return 'png'; + default: + return 'bin'; + } +} diff --git a/src/openhuman/tools/impl/document/engine.rs b/src/openhuman/tools/impl/document/engine.rs new file mode 100644 index 0000000000..7247fcb24a --- /dev/null +++ b/src/openhuman/tools/impl/document/engine.rs @@ -0,0 +1,427 @@ +//! Native Rust `.docx` generator, backed by the +//! [`docx-rs`](https://crates.io/crates/docx-rs) crate (MIT). Pure CPU, +//! no subprocess, no managed runtime — the document analogue of the +//! `ppt-rs`-backed presentation [`engine`](super::super::presentation). +//! Output is a byte buffer the caller writes to the artifact's +//! `output_path`. +//! +//! ## Mapping `GenerateDocumentInput` → OOXML +//! +//! The document is emitted as a linear paragraph stream: +//! +//! ```text +//! title → bold, large "Title"-styled paragraph +//! author (opt) → italic paragraph beneath the title +//! per section: +//! heading (opt) → bold "Heading1"-styled paragraph +//! paragraphs[] → one normal paragraph each +//! bullets[] → single-level `•` list (shared numbering id) +//! ``` +//! +//! Headings carry BOTH a style id (`Title` / `Heading1`, which Word maps +//! to its built-in outline styles) AND an explicit bold+size run, so the +//! visual hierarchy survives even if a reader ignores the style table. +//! Empty / whitespace-only paragraphs and bullets are filtered so a +//! trailing blank entry does not emit an empty run. +//! +//! ## Runtime +//! +//! `docx-rs` synthesis is synchronous and CPU-bound (well under 100 ms +//! for the section cap). We still drive it through `spawn_blocking` so +//! the async executor is not blocked, and wrap the call in a +//! `tokio::time::timeout` so a runaway generation cannot wedge the agent +//! loop — identical control-flow to the presentation engine. + +use std::time::Duration; + +use docx_rs::{ + AbstractNumbering, Docx, IndentLevel, Level, LevelJc, LevelText, NumberFormat, Numbering, + NumberingId, Paragraph, Run, Start, +}; +use tokio::task::JoinError; +use tokio::time::{error::Elapsed, timeout}; + +use super::types::{DocumentError, GenerateDocumentInput}; + +/// Shared numbering id for the single-level bullet list. One abstract +/// numbering + one concrete numbering is registered on the document and +/// every bullet paragraph references it at indent level 0. +const BULLET_NUMBERING_ID: usize = 1; + +/// Run font size for the document title, in OOXML half-points (28 pt). +const TITLE_SIZE_HALF_PT: usize = 56; +/// Run font size for a section heading, in half-points (16 pt). +const HEADING_SIZE_HALF_PT: usize = 32; +/// Run font size for the author byline, in half-points (12 pt). +const AUTHOR_SIZE_HALF_PT: usize = 24; + +/// Run the synthesis. Returns the serialised `.docx` 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 +/// [`DocumentError::GenerationTimeout`]. +pub(super) async fn generate( + input: &GenerateDocumentInput, + deadline: Duration, +) -> Result, DocumentError> { + // Clone the input across the blocking boundary — cheap relative to the + // synthesis, and keeps the blocking closure `'static`. + let owned = input.clone(); + let started = std::time::Instant::now(); + let section_count = owned.sections.len(); + let deadline_secs = deadline.as_secs(); + let title_chars = owned.title.chars().count(); + + tracing::debug!( + target: "document", + deadline_secs, + section_count, + title_chars, + "[document:engine] generate:start" + ); + + let join: Result, EngineFailure>, _>, Elapsed> = timeout( + deadline, + tokio::task::spawn_blocking(move || generate_blocking(&owned)), + ) + .await; + + let elapsed_ms = started.elapsed().as_millis() as u64; + match join { + Err(_elapsed) => { + tracing::warn!( + target: "document", + elapsed_ms, + deadline_secs, + section_count, + "[document:engine] generate:timeout" + ); + Err(DocumentError::GenerationTimeout { + timeout_secs: deadline_secs, + }) + } + Ok(Err(join_err)) => { + let err = map_join_error(join_err); + tracing::warn!( + target: "document", + elapsed_ms, + kind = "join_error", + err = %err, + "[document:engine] generate:failure" + ); + Err(err) + } + Ok(Ok(Err(engine_err))) => { + let err = map_engine_failure(engine_err); + tracing::warn!( + target: "document", + elapsed_ms, + kind = "engine_failure", + err = %err, + "[document:engine] generate:failure" + ); + Err(err) + } + Ok(Ok(Ok(bytes))) => { + tracing::debug!( + target: "document", + elapsed_ms, + bytes = bytes.len(), + section_count, + "[document:engine] generate:done" + ); + Ok(bytes) + } + } +} + +/// Blocking inner — runs on the `spawn_blocking` pool. Builds the whole +/// `docx-rs` document from the input then serialises it into an in-memory +/// zip buffer. Returns a dedicated [`EngineFailure`] so the async wrapper +/// can distinguish a library error from a panic / cancellation. +fn generate_blocking(input: &GenerateDocumentInput) -> Result, EngineFailure> { + let docx = build_document(input); + + // `XMLDocx::pack` takes a `Write + Seek` writer by value; a + // `&mut Cursor>` satisfies both traits, so we can pack into an + // in-memory buffer and recover the bytes via `into_inner`. + let mut cursor = std::io::Cursor::new(Vec::::new()); + docx.build() + .pack(&mut cursor) + .map_err(|err| EngineFailure::Library(format!("{err}")))?; + Ok(cursor.into_inner()) +} + +/// Pure transformation from our schema to a `docx-rs` [`Docx`]. Pulled +/// out for unit-testability — the paragraph ordering + empty-filtering +/// rules are load-bearing for the rendered document shape. +fn build_document(input: &GenerateDocumentInput) -> Docx { + // Register the shared single-level bullet list once. `NumberFormat` + // "bullet" + a `•` level text renders an unordered list; every bullet + // paragraph binds to this numbering id at indent level 0. + let mut docx = Docx::new() + .add_abstract_numbering( + AbstractNumbering::new(BULLET_NUMBERING_ID).add_level(Level::new( + 0, + Start::new(1), + NumberFormat::new("bullet"), + LevelText::new("•"), + LevelJc::new("left"), + )), + ) + .add_numbering(Numbering::new(BULLET_NUMBERING_ID, BULLET_NUMBERING_ID)); + + // Title — bold + large, styled as the built-in "Title" outline style. + docx = docx.add_paragraph( + Paragraph::new().style("Title").add_run( + Run::new() + .add_text(input.title.trim()) + .bold() + .size(TITLE_SIZE_HALF_PT), + ), + ); + + // Author byline — italic, if present and non-blank. + if let Some(author) = input.author.as_deref().filter(|a| !a.trim().is_empty()) { + docx = docx.add_paragraph( + Paragraph::new().add_run( + Run::new() + .add_text(author.trim()) + .italic() + .size(AUTHOR_SIZE_HALF_PT), + ), + ); + } + + for section in &input.sections { + if let Some(heading) = section.heading.as_deref().filter(|h| !h.trim().is_empty()) { + docx = docx.add_paragraph( + Paragraph::new().style("Heading1").add_run( + Run::new() + .add_text(heading.trim()) + .bold() + .size(HEADING_SIZE_HALF_PT), + ), + ); + } + for paragraph in §ion.paragraphs { + let text = paragraph.trim(); + if !text.is_empty() { + docx = docx.add_paragraph(Paragraph::new().add_run(Run::new().add_text(text))); + } + } + for bullet in §ion.bullets { + let text = bullet.trim(); + if !text.is_empty() { + docx = docx.add_paragraph( + Paragraph::new() + .add_run(Run::new().add_text(text)) + .numbering(NumberingId::new(BULLET_NUMBERING_ID), IndentLevel::new(0)), + ); + } + } + } + + docx +} + +/// Internal failure shape used to keep the blocking-thread surface +/// `Send`-clean (the underlying library error types are not guaranteed +/// to be `Send + Sync + 'static`). +#[derive(Debug)] +enum EngineFailure { + Library(String), +} + +fn map_engine_failure(failure: EngineFailure) -> DocumentError { + match failure { + EngineFailure::Library(msg) => DocumentError::GenerationFailed { + stderr_truncated: DocumentError::truncate_stderr(&msg), + }, + } +} + +fn map_join_error(err: JoinError) -> DocumentError { + // The outer `tokio::time::timeout` already routes the timeout case, so + // a `JoinError` here is a panic (docx-rs bug / OOM on the blocking + // pool) or a cancellation (runtime shutdown / explicit abort). Both + // surface as `GenerationFailed` with context preserved — mirrors the + // presentation engine so a "0s timeout" message is never fabricated. + if err.is_panic() { + DocumentError::GenerationFailed { + stderr_truncated: DocumentError::truncate_stderr("document engine panicked"), + } + } else { + DocumentError::GenerationFailed { + stderr_truncated: DocumentError::truncate_stderr(&format!( + "document engine task cancelled: {err}" + )), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::openhuman::tools::implementations::document::types::DocumentSection; + + fn sample_input() -> GenerateDocumentInput { + GenerateDocumentInput { + title: "Project Charter".to_string(), + author: Some("Alice".to_string()), + sections: vec![ + DocumentSection { + heading: Some("Overview".to_string()), + paragraphs: vec!["This document describes the plan.".to_string()], + bullets: vec![], + }, + DocumentSection { + heading: Some("Goals".to_string()), + paragraphs: vec![], + bullets: vec!["Ship v1".to_string(), "Delight users".to_string()], + }, + ], + } + } + + /// Read the entry names of a produced `.docx` byte buffer. + fn docx_entry_names(bytes: &[u8]) -> Vec { + let cursor = std::io::Cursor::new(bytes.to_vec()); + let mut zip = zip::ZipArchive::new(cursor).expect("output is a valid zip archive"); + (0..zip.len()) + .map(|i| zip.by_index(i).unwrap().name().to_string()) + .collect() + } + + /// Read one entry's UTF-8 body out of a produced `.docx`. + fn docx_entry_body(bytes: &[u8], name: &str) -> String { + let cursor = std::io::Cursor::new(bytes.to_vec()); + let mut zip = zip::ZipArchive::new(cursor).expect("valid zip"); + let mut entry = zip.by_name(name).expect("entry present"); + let mut body = String::new(); + std::io::Read::read_to_string(&mut entry, &mut body).unwrap(); + body + } + + #[tokio::test] + async fn generate_round_trips_to_valid_docx() { + // End-to-end: build → docx-rs → byte buffer → re-open as zip → + // confirm the OOXML skeleton + that our text reached document.xml. + let bytes = generate(&sample_input(), Duration::from_secs(30)) + .await + .expect("generate should succeed"); + + // A `.docx` is a zip: the magic bytes are the local-file-header + // signature `PK\x03\x04`. This is the acceptance-criteria check + // that any OOXML reader can open the file. + assert!( + bytes.len() > 200, + "docx unexpectedly small ({} bytes)", + bytes.len() + ); + assert_eq!(&bytes[0..2], b"PK", "docx must start with the zip magic PK"); + + let names = docx_entry_names(&bytes); + for required in ["[Content_Types].xml", "_rels/.rels", "word/document.xml"] { + assert!( + names.iter().any(|n| n == required), + "missing OOXML entry: {required} (got: {names:?})" + ); + } + + // Numbering was used → the numbering part must materialise. + assert!( + names.iter().any(|n| n == "word/numbering.xml"), + "bullet list should emit word/numbering.xml (got: {names:?})" + ); + + // Our title, heading, paragraph, and bullet text all reach the + // rendered document body — none dropped on the floor. + let doc = docx_entry_body(&bytes, "word/document.xml"); + for needle in [ + "Project Charter", + "Overview", + "This document describes the plan.", + "Goals", + "Ship v1", + "Delight users", + ] { + assert!( + doc.contains(needle), + "document.xml missing text: {needle:?}" + ); + } + } + + #[tokio::test] + async fn generate_drops_blank_paragraphs_and_bullets() { + // Whitespace-only entries must not blow up generation and must not + // emit empty runs — the engine trims + drops them. + let input = GenerateDocumentInput { + title: "Trimmed".to_string(), + author: Some(" ".to_string()), + sections: vec![DocumentSection { + heading: Some("Kept".to_string()), + paragraphs: vec!["real".to_string(), " ".to_string(), String::new()], + bullets: vec!["item".to_string(), "\t\n".to_string()], + }], + }; + let bytes = generate(&input, Duration::from_secs(30)) + .await + .expect("generate should succeed on whitespace-only entries"); + let doc = docx_entry_body(&bytes, "word/document.xml"); + assert!(doc.contains("real")); + assert!(doc.contains("item")); + } + + #[tokio::test] + async fn generate_yields_clean_structured_result_under_zero_deadline() { + // Contract under an impossibly-short deadline: `generate` must surface a + // clean, structured outcome — never a panic or a half-written buffer. + // + // Which outcome we get is inherently racy and must NOT be pinned: a + // near-zero `timeout` wrapping `spawn_blocking` usually elapses first + // (GenerationTimeout), but the runtime can instead cancel the blocking + // task, which `map_join_error` maps to GenerationFailed, and a trivial + // input can even finish before the timer fires (Ok). Asserting one exact + // variant made this flake under coverage instrumentation. We assert the + // real invariant: any Ok is a non-empty buffer, any Err is one of the + // two documented structured variants, and nothing panics. + match generate(&sample_input(), Duration::ZERO).await { + Ok(bytes) => assert!(!bytes.is_empty(), "a completed docx must be non-empty"), + Err(DocumentError::GenerationTimeout { timeout_secs }) => { + assert_eq!(timeout_secs, 0, "zero deadline reports 0 seconds"); + } + Err(DocumentError::GenerationFailed { .. }) => { + // Blocking task cancelled before the timer fired — still clean. + } + Err(other) => panic!("unexpected error variant under a zero deadline: {other:?}"), + } + } + + #[tokio::test] + async fn map_join_error_cancellation_becomes_generation_failed() { + // A non-panic JoinError (cancellation via abort) surfaces as + // GenerationFailed with the cancellation context preserved — never + // a fabricated "0s timeout". + let handle = tokio::spawn(async { + 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() yields a cancellation, not a panic" + ); + match map_join_error(join_err) { + DocumentError::GenerationFailed { stderr_truncated } => { + assert!( + stderr_truncated.contains("document engine task cancelled"), + "cancellation context missing: {stderr_truncated:?}" + ); + } + other => panic!("expected GenerationFailed, got {other:?}"), + } + } +} diff --git a/src/openhuman/tools/impl/document/mod.rs b/src/openhuman/tools/impl/document/mod.rs new file mode 100644 index 0000000000..b4ad4751ee --- /dev/null +++ b/src/openhuman/tools/impl/document/mod.rs @@ -0,0 +1,291 @@ +//! Tool: `generate_document` — build a `.docx` file from a structured +//! section spec via the native-Rust [`engine`] module. +//! +//! Flow (mirrors `generate_presentation` exactly so the two artifact +//! producers stay parallel): +//! 1. Validate the JSON-Schema input early (`types::validate_input`) so +//! the agent gets a structured `InvalidInput` it can self-correct on. +//! 2. Allocate an artifact dir via `artifacts::create_artifact` (kind = +//! [`ArtifactKind::Document`], extension `"docx"`). The returned +//! `meta` starts at `ArtifactStatus::Pending` so an interrupted run +//! never surfaces as Ready. +//! 3. Persist the verbatim args via `save_artifact_args` for regeneration +//! parity (the failed-card Retry path, #3162). +//! 4. Generate the `.docx` bytes via [`engine::generate`] — pure Rust, +//! `docx-rs`-backed, no subprocess. Wrapped in `spawn_blocking` + +//! `tokio::time::timeout`. +//! 5. Write the bytes, stat for size, flip the artifact to `Ready` via +//! `finalize_artifact`, return the artifact id + path. +//! 6. On any failure: flip the artifact to `Failed` via `fail_artifact` +//! so the UI surfaces the reason instead of an indefinite spinner. +//! +//! Added in GH #4847 (Problem 3: `.docx` export). The +//! [`ArtifactKind::Document`] enum variant already existed but had no +//! producer; the byte-agnostic artifact pipeline (Save-As dialog + +//! Downloads copy) and the frontend `document → docx` extension map need +//! no format-specific changes. + +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, +}; +use crate::openhuman::security::SecurityPolicy; +use crate::openhuman::tools::traits::{PermissionLevel, Tool, ToolResult}; + +mod engine; +mod types; + +#[cfg(test)] +#[path = "tests.rs"] +mod tests; + +use self::types::{validate_input, GenerateDocumentInput, GenerateDocumentOutput}; + +/// Generation timeout. `docx-rs` typically completes the full section cap +/// in well under a second; the 30 s ceiling is a defensive bound against +/// pathological inputs slipping past `validate_input` and 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_document"; + +/// One-shot `.docx` generator. See module docs for the request flow. +pub struct DocumentTool { + workspace_dir: PathBuf, + /// Retained for constructor parity with [`PresentationTool`] (both are + /// registered identically in `tools::ops`) and for future features + /// (e.g. embedding a `File`-source image) that will need the same + /// path-validation surface. Not read by the current text-only engine. + #[allow(dead_code)] + security: Arc, +} + +impl DocumentTool { + /// Production constructor. The engine is stateless. Pass the workspace + /// directory the artifact pipeline writes into, plus the active + /// [`SecurityPolicy`] (same signature as [`PresentationTool::new`] so + /// both tools register with an identical call). + pub fn new(workspace_dir: PathBuf, security: Arc) -> Self { + Self { + workspace_dir, + security, + } + } +} + +#[async_trait] +impl Tool for DocumentTool { + fn name(&self) -> &str { + TOOL_NAME + } + + fn description(&self) -> &str { + // Router-rule format per the existing tool conventions: tell the + // orchestrator when to use this tool and when NOT to. + "Generate a Word (.docx) document from a structured section spec. \ + USE THIS when the user asks for a document, a report, a letter, an \ + essay, meeting notes, or any prose deliverable they want as an \ + editable Word file. Provide `title` plus a `sections` array of \ + `{heading?, paragraphs?, bullets?}` objects; headings render bold, \ + bullets render as a list. NOT for: slide decks or presentations \ + (use generate_presentation), spreadsheets, or non-Word formats \ + (PDF, Google Docs 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 { + // Built as separate `json!` bindings to keep macro-expansion depth + // low and to mirror the presentation tool's schema style. + let section_item_schema = json!({ + "type": "object", + "additionalProperties": false, + "properties": { + "heading": { + "type": "string", + "maxLength": types::MAX_TEXT_CHARS, + "description": "Optional section heading, rendered bold." + }, + "paragraphs": { + "type": "array", + "maxItems": types::MAX_PARAGRAPHS_PER_SECTION, + "description": "Body paragraphs, one rendered paragraph each.", + "items": { "type": "string", "maxLength": types::MAX_PARAGRAPH_CHARS } + }, + "bullets": { + "type": "array", + "maxItems": types::MAX_BULLETS_PER_SECTION, + "description": "Bullet-list items rendered after the paragraphs.", + "items": { "type": "string", "maxLength": types::MAX_PARAGRAPH_CHARS } + } + } + }); + + json!({ + "type": "object", + "additionalProperties": false, + "required": ["title", "sections"], + "properties": { + "title": { + "type": "string", + "description": "Document title. Rendered as the leading title line and used as the artifact's human-readable name. Required, non-empty.", + "maxLength": types::MAX_TEXT_CHARS, + }, + "author": { + "type": "string", + "description": "Optional author byline, rendered italic beneath the title.", + "maxLength": types::MAX_TEXT_CHARS, + }, + "sections": { + "type": "array", + "minItems": 1, + "maxItems": types::MAX_SECTIONS, + "description": "Sections in display order. At least one entry required; each must have at least one of heading / paragraphs / bullets. Hard cap to bound generation time + output size.", + "items": section_item_schema, + } + } + }) + } + + fn permission_level(&self) -> PermissionLevel { + // Writes a file to the workspace artifacts dir. No subprocess / + // network reach. + PermissionLevel::Write + } + + fn supports_markdown(&self) -> bool { + true + } + + async fn execute(&self, args: Value) -> anyhow::Result { + let input: GenerateDocumentInput = match serde_json::from_value(args.clone()) { + Ok(v) => v, + Err(err) => { + let msg = format!("invalid generate_document arguments: {err}"); + tracing::warn!(target: "document", err = %err, "[document] deserialisation failed"); + return Ok(ToolResult::error(msg)); + } + }; + + if let Err(err) = validate_input(&input) { + tracing::debug!(target: "document", err = %err, "[document] validation rejected input"); + return Ok(ToolResult::error(err.to_string())); + } + + tracing::info!( + target: "document", + title_chars = input.title.chars().count(), + has_author = input.author.is_some(), + section_count = input.sections.len(), + "[document] generation request accepted" + ); + + let (meta, output_path) = create_artifact( + &self.workspace_dir, + ArtifactKind::Document, + &input.title, + "docx", + ) + .await + .map_err(anyhow::Error::msg)?; + + // Persist the verbatim args next to meta.json so a failed card's + // Retry can re-dispatch this exact spec (#3162). Best-effort: a + // write failure only forfeits regeneration, never aborts an + // otherwise-successful generation. + if let Err(err) = crate::openhuman::artifacts::store::save_artifact_args( + &self.workspace_dir, + &meta.id, + &args, + ) + .await + { + tracing::warn!( + target: "document", + err = %err, + artifact_id = %meta.id, + "[document] failed to persist args.json; artifact will not be regenerable" + ); + } + + 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: "document", + err = %err, + "[document] 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 document ({filename}): {err}"); + let _ = fail_artifact(&self.workspace_dir, &meta.id, &reason).await; + tracing::warn!( + target: "document", + err = %err, + artifact_id = %meta.id, + filename = %filename, + "[document] 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 to Failed so the UI surfaces the error instead of a + // stuck `Pending` spinner. Fail-artifact errors are + // swallowed — they can only recur if the same ledger backend + // is unavailable. + let _ = fail_artifact(&self.workspace_dir, &meta.id, &reason).await; + tracing::warn!( + target: "document", + err = %err, + artifact_id = %meta.id, + "[document] finalize_artifact failed; flipped to Failed" + ); + return Ok(ToolResult::error(reason)); + } + }; + + tracing::info!( + target: "document", + artifact_id = %updated.id, + size_bytes, + section_count = input.sections.len(), + "[document] generation complete" + ); + + let out = GenerateDocumentOutput { + artifact_id: updated.id.clone(), + artifact_path: output_path.display().to_string(), + section_count: input.sections.len(), + size_bytes, + }; + let payload = serde_json::to_value(&out)?; + let markdown = format!( + "Generated a {}-section document at `{}` (artifact `{}`, {} bytes).", + out.section_count, out.artifact_path, out.artifact_id, out.size_bytes + ); + Ok(ToolResult::success_with_markdown(payload, markdown)) + } +} diff --git a/src/openhuman/tools/impl/document/tests.rs b/src/openhuman/tools/impl/document/tests.rs new file mode 100644 index 0000000000..074b0c2556 --- /dev/null +++ b/src/openhuman/tools/impl/document/tests.rs @@ -0,0 +1,225 @@ +//! Unit tests for the `generate_document` tool. +//! +//! The engine layer (`engine.rs`) ships its own focused tests covering +//! the schema → OOXML mapping, the zip round-trip, empty-filtering, and +//! timeout handling. The tests here cover the tool-level concerns: input +//! validation rejection branches, the parameters schema contract, the +//! `description` router rules, and the happy-path output shape + artifact +//! finalisation (id + path + section count + size, file on disk). +//! +//! No mocks — the real `docx-rs` engine runs every test, so the happy +//! path doubles as a contract check that generation produces a valid, +//! openable `.docx` from the tool's perspective. + +use super::types::{DocumentError, MAX_SECTIONS, MAX_TEXT_CHARS}; +use super::*; + +use std::path::Path; + +fn workspace() -> tempfile::TempDir { + tempfile::tempdir().expect("create temp workspace") +} + +/// A permissive policy rooted at `workspace`. The current engine never +/// reads it, but the constructor takes it for parity with the +/// presentation tool. +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() + }) +} + +fn make_tool(workspace: &Path) -> DocumentTool { + DocumentTool::new(workspace.to_path_buf(), test_security(workspace)) +} + +fn minimal_input_json() -> serde_json::Value { + json!({ + "title": "Project Charter", + "sections": [ + { "heading": "Overview", "paragraphs": ["The plan in brief."], "bullets": ["Ship v1"] } + ] + }) +} + +/// 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:?}"), + } +} + +#[test] +fn parameters_schema_shape_matches_contract() { + 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"); + assert!(required.iter().any(|v| v.as_str() == Some("title"))); + assert!(required.iter().any(|v| v.as_str() == Some("sections"))); + 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 sections = &schema["properties"]["sections"]; + assert_eq!(sections["minItems"], 1); + assert_eq!(sections["maxItems"], MAX_SECTIONS); + let section_item = §ions["items"]; + assert_eq!(section_item["additionalProperties"], false); +} + +#[test] +fn permission_level_is_write() { + let tool = make_tool(Path::new("/tmp/never-read")); + assert_eq!(tool.permission_level(), PermissionLevel::Write); +} + +#[test] +fn description_includes_router_rules() { + let tool = make_tool(Path::new("/tmp/never-read")); + let desc = tool.description(); + assert!(desc.contains("USE THIS")); + assert!(desc.contains("NOT for")); + assert!(desc.contains("document") || desc.contains("docx")); +} + +#[tokio::test] +async fn execute_rejects_empty_title() { + let ws = workspace(); + let tool = make_tool(ws.path()); + let args = json!({ "title": "", "sections": [{ "heading": "x", "paragraphs": ["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_sections_array() { + let ws = workspace(); + let tool = make_tool(ws.path()); + let args = json!({ "title": "Doc", "sections": [] }); + let result = tool.execute(args).await.expect("execute returns Ok"); + assert!(result.is_error); + assert!(result.text().contains("section")); +} + +#[tokio::test] +async fn execute_rejects_section_with_no_content() { + let ws = workspace(); + let tool = make_tool(ws.path()); + let args = json!({ + "title": "Doc", + "sections": [{ "heading": "", "paragraphs": [" "], "bullets": [] }] + }); + let result = tool.execute(args).await.expect("execute returns Ok"); + assert!(result.is_error); +} + +#[tokio::test] +async fn execute_rejects_oversize_heading() { + let ws = workspace(); + let tool = make_tool(ws.path()); + let big = "x".repeat(MAX_TEXT_CHARS + 1); + let args = json!({ + "title": "Doc", + "sections": [{ "heading": big, "paragraphs": ["ok"] }] + }); + let result = tool.execute(args).await.expect("execute returns Ok"); + assert!(result.is_error); +} + +#[tokio::test] +async fn execute_rejects_too_many_sections() { + let ws = workspace(); + let tool = make_tool(ws.path()); + let sections: Vec<_> = (0..(MAX_SECTIONS + 1)) + .map(|i| json!({ "heading": format!("S{i}"), "paragraphs": ["x"] })) + .collect(); + let args = json!({ "title": "Big doc", "sections": sections }); + let result = tool.execute(args).await.expect("execute returns Ok"); + assert!(result.is_error); + assert!(result.text().contains(&MAX_SECTIONS.to_string())); +} + +#[tokio::test] +async fn execute_rejects_unknown_field() { + // `deny_unknown_fields` on the input structs means a stray key is a + // deserialisation error surfaced as a tool error, not a silent drop. + let ws = workspace(); + let tool = make_tool(ws.path()); + let args = json!({ "title": "Doc", "sections": [{ "paragraphs": ["x"] }], "bogus": 1 }); + let result = tool.execute(args).await.expect("execute returns Ok"); + assert!(result.is_error); +} + +#[tokio::test] +async fn execute_happy_path_returns_artifact_metadata() { + // End-to-end: drives the real docx-rs engine + artifact pipeline. + // Asserts the success contract — the artifact is finalised on disk and + // the markdown reply quotes the path + id. + let ws = workspace(); + let tool = make_tool(ws.path()); + 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 = payload_of(&result); + assert_eq!(payload["section_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!( + artifact_path.ends_with(".docx"), + "artifact should be a .docx: {artifact_path}" + ); + assert!( + size_bytes > 200, + "document unexpectedly small ({size_bytes} bytes)" + ); + + // The written file is a real, openable OOXML zip. + let bytes = std::fs::read(artifact_path).expect("artifact file readable"); + assert_eq!(&bytes[0..2], b"PK", "artifact must be a zip (PK magic)"); + + 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-section")); +} + +#[test] +fn truncate_stderr_caps_payload_with_suffix() { + let raw = "y".repeat(2000); + let out = DocumentError::truncate_stderr(&raw); + assert!(out.chars().count() <= 500); + assert!(out.ends_with("[…truncated]")); + let short = "tiny error"; + assert_eq!(DocumentError::truncate_stderr(short), short); +} diff --git a/src/openhuman/tools/impl/document/types.rs b/src/openhuman/tools/impl/document/types.rs new file mode 100644 index 0000000000..4917ce6374 --- /dev/null +++ b/src/openhuman/tools/impl/document/types.rs @@ -0,0 +1,456 @@ +//! Typed input / output / error contracts for the `generate_document` tool. +//! +//! Deliberately mirrors the `generate_presentation` contracts +//! (`presentation::types`) so the two artifact producers stay +//! structurally parallel: same validate-early ethos, same size caps +//! pattern, same structured `InvalidInput` the agent can self-correct +//! on. Where the presentation tool models a deck of `slides`, the +//! document tool models a linear body of ordered `sections`. + +use serde::{Deserialize, Serialize}; + +/// Maximum number of sections a single `generate_document` call may +/// produce. Hard cap to bound generation time + output size; the LLM is +/// asked to break larger documents into multiple calls. +pub(super) const MAX_SECTIONS: usize = 128; + +/// Maximum length of a single short text field (title, author, section +/// heading). Bounds the payload handed to the `docx-rs` engine. +pub(super) const MAX_TEXT_CHARS: usize = 2_000; + +/// Maximum length of a single body paragraph. Prose paragraphs run +/// longer than a slide's bullet, so this cap is more generous than +/// [`MAX_TEXT_CHARS`] while still bounding the worst-case payload. +pub(super) const MAX_PARAGRAPH_CHARS: usize = 20_000; + +/// Maximum number of body paragraphs in a single section. Beyond this a +/// section should be split; keeps one `execute` call bounded. +pub(super) const MAX_PARAGRAPHS_PER_SECTION: usize = 200; + +/// Maximum number of bullet-list items in a single section. +pub(super) const MAX_BULLETS_PER_SECTION: usize = 200; + +/// Aggregate cap on the total body text (title + author + every heading, +/// paragraph, and bullet) across the whole document, in Unicode scalar +/// values. The per-field/per-section limits above bound each piece, but +/// their product (`MAX_SECTIONS × MAX_PARAGRAPHS_PER_SECTION × +/// MAX_PARAGRAPH_CHARS` alone is ~512M chars) would still let a single +/// valid request build a multi-hundred-megabyte DOCX in memory. This +/// checked total keeps the worst-case in-memory payload bounded to a few +/// megabytes of text while remaining generous for any real document. +pub(super) const MAX_TOTAL_CHARS: usize = 2_000_000; + +/// One section of the document, rendered in input order. A section is a +/// heading (optional) followed by any number of body paragraphs and/or a +/// bullet list. At least one of the three must be populated — a wholly +/// empty section is rejected by [`validate_input`] rather than rendering +/// a blank run. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct DocumentSection { + /// Section heading, rendered as a bold heading paragraph. Optional — + /// a section may be pure body/bullets under the document title. + #[serde(default)] + pub heading: Option, + /// Body paragraphs, each rendered as its own normal paragraph in + /// order. Empty / whitespace-only entries are dropped by the engine. + #[serde(default)] + pub paragraphs: Vec, + /// Bullet-list items, rendered as a single-level `•` bulleted list + /// after the section's body paragraphs. Empty / whitespace-only + /// entries are dropped by the engine. + #[serde(default)] + pub bullets: Vec, +} + +impl DocumentSection { + /// `true` when the section carries no renderable content at all + /// (heading blank/absent, and every paragraph/bullet blank). Used by + /// [`validate_input`] to reject empty sections the same way the + /// presentation tool rejects an empty slide. + pub(super) fn is_empty(&self) -> bool { + let has_heading = self + .heading + .as_deref() + .map(|h| !h.trim().is_empty()) + .unwrap_or(false); + let has_paragraph = self.paragraphs.iter().any(|p| !p.trim().is_empty()); + let has_bullet = self.bullets.iter().any(|b| !b.trim().is_empty()); + !(has_heading || has_paragraph || has_bullet) + } +} + +/// Top-level input for the `generate_document` tool. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct GenerateDocumentInput { + /// Document title. Surfaces as the leading title paragraph and as the + /// artifact's human-readable name. Required, non-empty. + pub title: String, + /// Optional author byline, rendered as an italic line beneath the + /// title. + #[serde(default)] + pub author: Option, + /// Section specs, in display order. Must contain at least one entry. + #[serde(default)] + pub sections: Vec, +} + +/// Tool output returned via [`crate::openhuman::tools::traits::ToolResult`] +/// as the JSON `data` field. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct GenerateDocumentOutput { + /// 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 `.docx`. + pub artifact_path: String, + /// Number of sections rendered into the document body. + pub section_count: usize, + /// On-disk size of the produced `.docx` in bytes. + pub size_bytes: u64, +} + +/// Structured error variants surfaced to the agent. Mirrors +/// [`presentation::types::PresentationError`](super::super::presentation) +/// so downstream error handling stays uniform across artifact producers. +#[derive(Debug, thiserror::Error)] +pub enum DocumentError { + #[error("invalid input for field '{field}': {reason}")] + InvalidInput { field: String, reason: String }, + + #[error("document generation failed: {stderr_truncated}")] + GenerationFailed { stderr_truncated: String }, + + #[error("document generation exceeded {timeout_secs}s timeout")] + GenerationTimeout { timeout_secs: u64 }, +} + +impl DocumentError { + /// Truncate a library-error string to a 500-char cap (UTF-8-safe) so + /// the variant never carries an unbounded payload back to the agent. + /// Same cap/suffix as the presentation tool's `truncate_stderr`. + 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 invoking the `docx-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: &GenerateDocumentInput) -> Result<(), DocumentError> { + if input.title.trim().is_empty() { + return Err(DocumentError::InvalidInput { + field: "title".to_string(), + reason: "must not be empty".to_string(), + }); + } + if input.title.chars().count() > MAX_TEXT_CHARS { + return Err(DocumentError::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(DocumentError::InvalidInput { + field: "author".to_string(), + reason: format!("must be ≤ {MAX_TEXT_CHARS} chars"), + }); + } + } + if input.sections.is_empty() { + return Err(DocumentError::InvalidInput { + field: "sections".to_string(), + reason: "must contain at least one section".to_string(), + }); + } + if input.sections.len() > MAX_SECTIONS { + return Err(DocumentError::InvalidInput { + field: "sections".to_string(), + reason: format!("must contain ≤ {MAX_SECTIONS} sections"), + }); + } + for (i, section) in input.sections.iter().enumerate() { + // Reject wholly-empty sections: the engine trims + drops empty + // paragraph/bullet entries, so a section with only [" "] would + // render blank despite carrying entries. + if section.is_empty() { + return Err(DocumentError::InvalidInput { + field: format!("sections[{i}]"), + reason: "must have at least one of heading / paragraphs / bullets".to_string(), + }); + } + if let Some(heading) = section.heading.as_deref() { + if heading.chars().count() > MAX_TEXT_CHARS { + return Err(DocumentError::InvalidInput { + field: format!("sections[{i}].heading"), + reason: format!("must be ≤ {MAX_TEXT_CHARS} chars"), + }); + } + } + if section.paragraphs.len() > MAX_PARAGRAPHS_PER_SECTION { + return Err(DocumentError::InvalidInput { + field: format!("sections[{i}].paragraphs"), + reason: format!("must contain ≤ {MAX_PARAGRAPHS_PER_SECTION} paragraphs"), + }); + } + for (p, paragraph) in section.paragraphs.iter().enumerate() { + if paragraph.chars().count() > MAX_PARAGRAPH_CHARS { + return Err(DocumentError::InvalidInput { + field: format!("sections[{i}].paragraphs[{p}]"), + reason: format!("must be ≤ {MAX_PARAGRAPH_CHARS} chars"), + }); + } + } + if section.bullets.len() > MAX_BULLETS_PER_SECTION { + return Err(DocumentError::InvalidInput { + field: format!("sections[{i}].bullets"), + reason: format!("must contain ≤ {MAX_BULLETS_PER_SECTION} bullets"), + }); + } + for (b, bullet) in section.bullets.iter().enumerate() { + if bullet.chars().count() > MAX_PARAGRAPH_CHARS { + return Err(DocumentError::InvalidInput { + field: format!("sections[{i}].bullets[{b}]"), + reason: format!("must be ≤ {MAX_PARAGRAPH_CHARS} chars"), + }); + } + } + } + // Aggregate budget: the per-field caps above bound each piece, but not + // their sum, so a valid request could still assemble a huge in-memory DOCX. + // Sum every text field with saturating arithmetic (can't overflow) and + // reject once the whole document exceeds MAX_TOTAL_CHARS. + let mut total_chars = input.title.chars().count(); + if let Some(author) = input.author.as_deref() { + total_chars = total_chars.saturating_add(author.chars().count()); + } + for section in &input.sections { + if let Some(heading) = section.heading.as_deref() { + total_chars = total_chars.saturating_add(heading.chars().count()); + } + for paragraph in §ion.paragraphs { + total_chars = total_chars.saturating_add(paragraph.chars().count()); + } + for bullet in §ion.bullets { + total_chars = total_chars.saturating_add(bullet.chars().count()); + } + } + if total_chars > MAX_TOTAL_CHARS { + return Err(DocumentError::InvalidInput { + field: "sections".to_string(), + reason: format!("total document text must be ≤ {MAX_TOTAL_CHARS} chars"), + }); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// One valid section carrying a heading + a paragraph + a bullet. + fn section() -> DocumentSection { + DocumentSection { + heading: Some("Overview".to_string()), + paragraphs: vec!["A body paragraph.".to_string()], + bullets: vec!["A bullet".to_string()], + } + } + + /// A minimal valid input; individual tests mutate one field to drive a + /// single rejection branch. + fn base() -> GenerateDocumentInput { + GenerateDocumentInput { + title: "Charter".to_string(), + author: Some("Alice".to_string()), + sections: vec![section()], + } + } + + /// Assert `validate_input` rejects `input` naming `field` in the error. + fn assert_rejects(input: &GenerateDocumentInput, field: &str) { + match validate_input(input) { + Err(DocumentError::InvalidInput { field: f, .. }) => { + assert!( + f.contains(field), + "expected error field to contain {field:?}, got {f:?}" + ); + } + other => panic!("expected InvalidInput({field}), got {other:?}"), + } + } + + #[test] + fn accepts_a_well_formed_input() { + assert!(validate_input(&base()).is_ok()); + } + + #[test] + fn is_empty_reflects_content_presence() { + assert!(!section().is_empty()); + assert!(DocumentSection { + heading: Some(" ".to_string()), + paragraphs: vec![" ".to_string(), String::new()], + bullets: vec!["\t".to_string()], + } + .is_empty()); + // Any one populated field is enough. + assert!(!DocumentSection { + heading: None, + paragraphs: vec![], + bullets: vec!["x".to_string()], + } + .is_empty()); + } + + #[test] + fn rejects_empty_title() { + let mut i = base(); + i.title = " ".to_string(); + assert_rejects(&i, "title"); + } + + #[test] + fn rejects_oversize_title() { + let mut i = base(); + i.title = "t".repeat(MAX_TEXT_CHARS + 1); + assert_rejects(&i, "title"); + } + + #[test] + fn rejects_oversize_author() { + let mut i = base(); + i.author = Some("a".repeat(MAX_TEXT_CHARS + 1)); + assert_rejects(&i, "author"); + } + + #[test] + fn rejects_no_sections() { + let mut i = base(); + i.sections = vec![]; + assert_rejects(&i, "sections"); + } + + #[test] + fn rejects_too_many_sections() { + let mut i = base(); + i.sections = (0..(MAX_SECTIONS + 1)).map(|_| section()).collect(); + assert_rejects(&i, "sections"); + } + + #[test] + fn rejects_empty_section() { + let mut i = base(); + i.sections = vec![DocumentSection { + heading: Some(" ".to_string()), + paragraphs: vec![" ".to_string()], + bullets: vec![], + }]; + assert_rejects(&i, "sections[0]"); + } + + #[test] + fn rejects_oversize_heading() { + let mut i = base(); + i.sections[0].heading = Some("h".repeat(MAX_TEXT_CHARS + 1)); + assert_rejects(&i, "sections[0].heading"); + } + + #[test] + fn rejects_too_many_paragraphs() { + let mut i = base(); + i.sections[0].paragraphs = (0..(MAX_PARAGRAPHS_PER_SECTION + 1)) + .map(|n| format!("p{n}")) + .collect(); + assert_rejects(&i, "sections[0].paragraphs"); + } + + #[test] + fn rejects_oversize_paragraph() { + let mut i = base(); + i.sections[0].paragraphs = vec!["p".repeat(MAX_PARAGRAPH_CHARS + 1)]; + assert_rejects(&i, "sections[0].paragraphs[0]"); + } + + #[test] + fn rejects_too_many_bullets() { + let mut i = base(); + i.sections[0].bullets = (0..(MAX_BULLETS_PER_SECTION + 1)) + .map(|n| format!("b{n}")) + .collect(); + assert_rejects(&i, "sections[0].bullets"); + } + + #[test] + fn rejects_oversize_bullet() { + let mut i = base(); + i.sections[0].bullets = vec!["b".repeat(MAX_PARAGRAPH_CHARS + 1)]; + assert_rejects(&i, "sections[0].bullets[0]"); + } + + #[test] + fn accepts_document_exactly_at_total_char_budget() { + // 99 full paragraphs + one one-short paragraph + a 1-char title sum to + // exactly MAX_TOTAL_CHARS, while every field stays within its own cap. + let mut paragraphs: Vec = + (0..99).map(|_| "p".repeat(MAX_PARAGRAPH_CHARS)).collect(); + paragraphs.push("p".repeat(MAX_PARAGRAPH_CHARS - 1)); + assert_eq!( + 1 + 99 * MAX_PARAGRAPH_CHARS + (MAX_PARAGRAPH_CHARS - 1), + MAX_TOTAL_CHARS, + "test fixture must total exactly the budget" + ); + let input = GenerateDocumentInput { + title: "T".to_string(), + author: None, + sections: vec![DocumentSection { + heading: None, + paragraphs, + bullets: vec![], + }], + }; + assert!( + validate_input(&input).is_ok(), + "at-limit input must be accepted" + ); + } + + #[test] + fn rejects_document_over_total_char_budget() { + // 101 max-size paragraphs = 2_020_000 chars > MAX_TOTAL_CHARS, even + // though each paragraph and the section/paragraph counts stay within + // their own limits — only the aggregate budget catches it. + let mut i = base(); + i.sections[0].paragraphs = (0..101).map(|_| "p".repeat(MAX_PARAGRAPH_CHARS)).collect(); + match validate_input(&i) { + Err(DocumentError::InvalidInput { field, reason }) => { + assert_eq!(field, "sections"); + assert!( + reason.contains("total document text"), + "expected aggregate-budget error, got: {reason}" + ); + } + other => panic!("expected InvalidInput(sections), got {other:?}"), + } + } + + #[test] + fn truncate_stderr_caps_and_passes_short_through() { + let long = "z".repeat(4000); + let out = DocumentError::truncate_stderr(&long); + assert!(out.chars().count() <= 500); + assert!(out.ends_with("[…truncated]")); + assert_eq!(DocumentError::truncate_stderr("short"), "short"); + } +} diff --git a/src/openhuman/tools/impl/mod.rs b/src/openhuman/tools/impl/mod.rs index 2ede3cc3ae..0dbd542fc1 100644 --- a/src/openhuman/tools/impl/mod.rs +++ b/src/openhuman/tools/impl/mod.rs @@ -1,5 +1,6 @@ pub mod browser; pub mod computer; +pub mod document; pub mod filesystem; pub mod network; pub mod presentation; @@ -7,6 +8,7 @@ pub mod system; pub use browser::*; pub use computer::*; +pub use document::DocumentTool; pub use filesystem::*; pub use network::*; pub use presentation::PresentationTool; diff --git a/src/openhuman/tools/ops.rs b/src/openhuman/tools/ops.rs index d9203d79b5..7c12a9ae46 100644 --- a/src/openhuman/tools/ops.rs +++ b/src/openhuman/tools/ops.rs @@ -696,6 +696,15 @@ pub fn all_tools_with_runtime( security.clone(), ))); + // Document generation (#4847, Problem 3). Native-Rust engine + // (docx-rs backed) — no managed runtime, no subprocess — emitting a + // real `.docx` through the same byte-agnostic artifact pipeline as + // the presentation tool. Always registered; same constructor shape. + tools.push(Box::new(DocumentTool::new( + root_config.workspace_dir.clone(), + security.clone(), + ))); + // Long-term goals list tools. Used primarily by the background // `goals_agent` (which filters to these via its `[tools] named` // allowlist); also available to the main agent for explicit edits.