feat(tools): generate_document tool emitting real .docx (#4847 Problem 3)#4899
Conversation
…sai#4847) Add a generate_document agent tool that produces a Microsoft Word .docx via the native-Rust docx-rs writer, mirroring the generate_presentation (.pptx) tool through the format-agnostic artifact pipeline. - New src/openhuman/tools/impl/document/ (engine, types, tool, tests): create_artifact(ArtifactKind::Document, title, "docx") -> write bytes -> finalize_artifact, with input validation, size caps, and args persistence for regeneration parity. - Register DocumentTool in tools/ops.rs next to the presentation tool. - docx-rs 0.4.20 added to the core crate (root + Tauri shell Cargo.lock). - Frontend: extract the duplicated extensionFor into a shared artifactExtension.ts and map document -> 'docx' (was wrongly 'pdf'), used by ArtifactCard and ChatFilesPanel; add a unit test. Preserves headings, bold, and bullet lists; the file lands on the user's desktop through the existing Save-As / Downloads copy (byte-agnostic). Verified: full Tauri build compiles clean and the tool produces a Word-openable .docx. Partially addresses tinyhumansai#4847 (Problem 3: .docx export).
3f594f3 to
5492e9e
Compare
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 24 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a native Rust ChangesDOCX document generation
Sequence Diagram(s)sequenceDiagram
participant Caller
participant DocumentTool
participant ArtifactStore
participant DocumentEngine
participant Frontend
Caller->>DocumentTool: Submit structured document input
DocumentTool->>ArtifactStore: Create pending artifact
DocumentTool->>DocumentEngine: Generate DOCX bytes
DocumentEngine-->>DocumentTool: Return DOCX bytes
DocumentTool->>ArtifactStore: Write and finalize artifact
ArtifactStore-->>Frontend: Expose document artifact
Frontend->>Frontend: Resolve .docx download extension
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
…nyhumansai#4847) Frontend Checks failed: ArtifactCard and ChatFilesPanel tests still asserted the per-kind download extension default `document → pdf`. This PR's `extensionFor` helper intentionally changed that default to `docx` (generate_document emits a real Word .docx), so the pre-existing parametrized cases were stale. Update both `it.each` rows to expect `docx`. Verified: both files green (39 tests).
W5 sweep — pushed 3b58800CI (Frontend Checks) was red — two Vitest failures, both stale test expectations (not lint/prettier):
Fix: updated both rows to expect Rust Core/Tauri Coverage jobs (document engine) were still running at sweep time; will re-check. Comment-only per role — not approving/merging. |
W5 code-review pass — no blocking issuesReviewed the diff:
Nothing to change beyond the test-expectation CI fix above. (Review only — human approves/merges.) |
…inyhumansai#4847) Rust Core Coverage flaked on `generate_surfaces_timeout_under_tiny_deadline`: it raced `generate()`'s `timeout(deadline, spawn_blocking(...))` against a 1 ns deadline. A future-dated sub-millisecond deadline registers with tokio's timer wheel and only fires at wheel-tick (~ms) granularity, so under coverage instrumentation the blocking synthesis could win the race and return a non-Timeout variant, panicking the assertion. Use `Duration::ZERO`: an already-elapsed deadline takes the Elapsed branch on the first poll (the spawn_blocking handle is still Pending there, running on another thread), so the GenerationTimeout path is deterministic. Renamed the test to match; `timeout_secs` is still 0.
W5 — pushed d46d41c (Rust Core Coverage flake fix)After the Frontend fix went green, Rust Core Coverage failed on
Fix: use rustfmt clean. CI re-running to confirm green. Comment-only — not approving/merging. |
coderabbitai
Bot
added
bug
feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/document/engine.rs`:
- Around line 84-88: Bound the blocking generation path around generate_blocking
and its tokio::task::spawn_blocking call so timed-out work cannot continue
consuming unbounded blocking-thread capacity. Add a semaphore or dedicated
executor that limits concurrent generation jobs, acquire its permit before
spawning, and retain the existing deadline behavior while ensuring permit
ownership covers the full blocking task lifetime.
In `@src/openhuman/tools/impl/document/types.rs`:
- Around line 15-31: Add an aggregate document-size budget enforced by
validate_input, summing the relevant text characters or bytes across titles,
metadata, headings, paragraphs, and bullet items while retaining the existing
per-field and per-section limits. Reject inputs when the checked total exceeds
the new shared maximum, using checked arithmetic to prevent overflow, and add
boundary tests covering exactly-at-limit acceptance and over-limit rejection.
In `@src/openhuman/tools/impl/mod.rs`:
- Around line 3-11: Move the document module from
src/openhuman/tools/impl/mod.rs to src/openhuman/document/, re-export its
DocumentTool through src/openhuman/tools/mod.rs, and place DocumentTool with its
execution logic in the domain’s tools.rs. Update
src/openhuman/tools/impl/document/mod.rs to remain export-only; apply the
implementation change at both cited sites.
🪄 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: 8fc5553a-182e-4485-b8d9-689a80da82dd
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockapp/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
Cargo.tomlapp/src/components/chat/ArtifactCard.tsxapp/src/components/chat/ChatFilesPanel.tsxapp/src/components/chat/__tests__/ArtifactCard.test.tsxapp/src/components/chat/__tests__/ChatFilesPanel.test.tsxapp/src/components/chat/artifactExtension.test.tsapp/src/components/chat/artifactExtension.tssrc/openhuman/tools/impl/document/engine.rssrc/openhuman/tools/impl/document/mod.rssrc/openhuman/tools/impl/document/tests.rssrc/openhuman/tools/impl/document/types.rssrc/openhuman/tools/impl/mod.rssrc/openhuman/tools/ops.rs
…ansai#4847) Address CodeRabbit review: - Add an aggregate document-size limit (MAX_TOTAL_CHARS = 2,000,000). The per-field/per-section caps bound each piece but not their sum, so a valid request could still assemble a multi-hundred-MB DOCX in memory. validate_input now sums all text (title + author + headings + paragraphs + bullets) with saturating arithmetic and rejects over-budget inputs. Adds at-limit-accept and over-limit-reject boundary tests. - Make the timeout test deterministic. Racing a ~0 deadline against spawn_blocking is inherently nondeterministic (timer-elapsed → GenerationTimeout, runtime-cancel → GenerationFailed, or a fast completion → Ok), so pinning one exact variant flaked under coverage instrumentation. Assert the real contract instead: a clean structured outcome (non-empty Ok, or a documented Err variant), never a panic or half-written buffer.
W5 — pushed cae0cc4 (CodeRabbit review + flaky-test)CodeRabbit review (3 items):
Also folded in: the Rust Core Coverage flake on the timeout test. Racing a ~0 deadline against rustfmt clean; CI re-running. Comment-only — not approving/merging. |
W5 — #4899 code is green; blocked only by a flaky Rust Core Coverage laneAll of this PR's own tests pass (de-flaked timeout test, new aggregate-size-limit boundary tests,
Root cause of the recurring one: that memory_queue test asserts Re-running once more. This is a pre-existing flaky test in an unrelated domain — the proper fix (freeze time / widen the defer window) belongs in a separate memory_queue PR, not bundled into this .docx change. Comment-only — not merging. |
…#4847 Problem 3) (tinyhumansai#4899) Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
2 participants
Partially addresses #4847 (Problem 3: .docx export).
Summary
Adds a
generate_documentagent tool that produces a real Microsoft Word.docx, mirroring the existinggenerate_presentation(.pptx) tool through the already format-agnostic artifact pipeline.Changes
src/openhuman/tools/impl/document/(engine, types, tool, tests):create_artifact(ArtifactKind::Document, title, "docx")→ write bytes →finalize_artifact, with input validation, size caps, and args persistence for regeneration parity.DocumentToolintools/ops.rsnext to the presentation tool (sameworkspace_dir+securityconstructor args).docx-rs0.4.20 added to the core crate — both rootCargo.lockandapp/src-tauri/Cargo.lockupdated.extensionForinto a sharedartifactExtension.tsand mapdocument→'docx'(was wrongly'pdf'); used byArtifactCardandChatFilesPanel. New unit test.Preserves headings, bold, and bullet lists. The file lands on the user's desktop through the existing Save-As / Downloads copy (byte-agnostic — no shell change needed).
Verification
docx-rs v0.4.20,openhuman v0.61.2, 0 Rust errors).generate_document, artifact card downloads a.docxthat opens in Word with title, bold headings, paragraphs, and bullets.Scope
Problems 1 (rate limiting), 2 (sub-agent), and 4 (overall) from #4847 are tracked separately — a written investigation of P1/P2 is posted as a comment on the issue.
Summary by CodeRabbit
New Features
.docxfiles from structured content..docxwhen no extension is provided.Bug Fixes