Add .pptx and .pdf, and move the module onto TinyBus streams - #6
Merged
Conversation
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The spec types, their limits, and `DocumentSpec::validate` were reachable only with the `docx` feature on, because they lived in `src/docx/types.rs` behind a gated module. That coupled the wire contract to the OOXML writer: a host whose synthesis happens elsewhere — in another process, or behind the TinyBus module in this repo — had to either pull `docx-rs` in to name `DocumentSpec`, or re-declare the spec and let the two definitions drift. Move them to a new ungated `src/spec/`, which depends on nothing but `serde` and the crate error type, and re-export from `docx` so `tinydocs::docx::DocumentSpec` still names the same type. `cargo check -p tinydocs --no-default-features` now compiles the whole contract with `docx-rs` absent from the dependency graph. Validation moves with the types, so it is no longer gated either — which is the point: validating at a host boundary is the cheap half, and it should not require the codec. Public API: purely additive. `tinydocs::spec` is new; every existing path resolves to the same item. Tests split along the same line. `src/spec/test.rs` owns validation, the blank/aggregate rules and the JSON contract, and must pass with every format feature off; `src/docx/test.rs` keeps the OOXML mapping assertions. Two new cases cover the aggregate-budget branches that only a bullet or a heading can cross, taking `src/spec/document.rs` to 100% line coverage. Co-authored-by: Medulla <medulla@tinyhumans.ai>
Ports the presentation engine from the OpenHuman host that already drove
`ppt-rs` directly, on the same split the docx path uses: the spec and its
validation go in ungated `spec/presentation`, the OOXML mapping goes in a gated
`pptx` module, and the host keeps its executor and deadline policy.
Three things are deliberately different from the code this came from.
Images are bytes, not references. The host's spec named an image by artifact id
or filesystem path, and resolving either is policy this crate must not hold —
which directories an agent may read, whether an identifier belongs to the
caller. `SlideImage` therefore carries the bytes, format and dimensions, and
`SlideImage::from_bytes` does the mechanical half of the hand-off. Because
identification lives in the ungated `spec::image`, a host can build and validate
a whole deck with `pptx` off.
Validation re-derives an image's format and dimensions from its bytes and
rejects any disagreement with what the spec declares. `from_bytes` keeps those
fields consistent by construction, but a spec can also arrive as JSON, where the
three are independent: a wrong format yields a part the reader refuses to open,
and wrong dimensions distort the image silently. Both now fail by name.
`fit_within` is integer arithmetic in u64 rather than f64 scaling. The float
spelling made the result depend on binary64 rounding for no benefit — an EMU is
1/914,400 inch, so a one-unit difference is invisible — and it could not satisfy
this repo's cast lints without an allow.
The host's `GenerationFailed { exit_code, stderr_truncated }` collapses onto
`Error::GenerationFailed { detail }`; `exit_code` was a vestige of a retired
python-pptx subprocess and was always -1. Timeout and cancellation stay host-
side, where the deadline is known.
Also fixes a mistake in the previous commit: `spec/document`'s tests were moved
without a `mod test;` declaration, so all 20 stopped running and the module's
coverage read 50%. Declared, they run again and it is back to 100%.
Public API: additive. `spec::presentation`, `spec::image` and `pptx` are new.
Format limits are reached through their own module rather than re-exported flat
from `spec`, because `MAX_TEXT_CHARS` means a different number per format.
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Completes the document surface a host needs from this crate: it writes .docx and .pptx, and now reads .pdf. Ported from the OpenHuman host, which called `pdf_extract::extract_text_from_mem` directly from its multimodal ingest path. This is the only module here that reads rather than writes, and the asymmetry shapes it. Everything else turns a spec the caller authored into bytes, so there is a contract to validate; extraction takes a document somebody else produced, so the input is arbitrary and often damaged. Two consequences: `ExtractionFailed` is a new `Error` variant rather than a reuse of `GenerationFailed`. The two have opposite causes and opposite remedies — generation fails on our output path and usually means a bug or an exhausted resource, whereas extraction fails on someone else's input and usually means the document is encrypted, damaged, or has no text layer. A caller that retries one should not retry the other. `Error` is `#[non_exhaustive]`, so adding it does not break a match. The boundary checks are `InvalidInput`, not extraction failures: empty input, input over `MAX_DOCUMENT_BYTES`, and input with no `%PDF-` signature all name the offending field instead of surfacing a parser's phrasing for "you handed me a JPEG". `MAX_DOCUMENT_BYTES` exists because extraction allocates well past the input size while parsing, and the caller is handing over something it did not produce; a host wanting a tighter bound applies it first. A document that parses but carries no text layer — a scan — yields an empty string rather than an error. Nothing to extract is not a failure to retry, and OCR is out of scope. The 60s timeout and the host's decision to degrade a failed extraction to a file reference stay where they were: this call is synchronous and holds no opinion about deadlines, and here that matters more than for synthesis, because the cost is set by the input rather than by a spec this crate has already bounded. Tests build a real single-page PDF in-process, computing its cross-reference offsets from the bytes emitted, so extraction is exercised end to end without a checked-in binary fixture that could go stale. Co-authored-by: Medulla <medulla@tinyhumans.ai>
The module could only ever return a `.docx` inline, and that stops working the moment the other two formats arrive. A TinyBus frame is a 16 MiB JSON document and a `Vec<u8>` serialises as an array of integers — roughly 3.5 bytes of frame per byte of payload — so the real inline ceiling is a few megabytes. A deck may legally carry 8 images of 5 MiB, and a `.pdf` handed in for extraction is bounded only by what the host accepted. TinyBus says as much itself: large payloads are meant to travel as paths, not inline. So bytes now move through a staging area in base64 chunks (1.34x rather than 3.5x), addressed by opaque blob ids, and no method returns bytes inline. A caller stages a document, calls a format method, and reads the result back the same way; frame size stops being part of the contract, and the caller's code path is the same regardless of size. Every bound in `blobs` is load-bearing rather than defensive. A module is trusted in-process code that TinyBus never unloads, so an abandoned upload is never reclaimed by a process exit that does not come — hence four independent limits (per chunk, per blob, total staged, blob count) and expiry of untouched blobs. The budget is reserved at `BeginBlob` rather than counted on arrival, so an admitted transfer can always finish instead of failing halfway when somebody else fills the area. Transfers are append-only: `offset` must equal the bytes received so far, which makes "complete" mean "length reached" and turns a lost or duplicated chunk into a named error at the moment it happens rather than a corrupt blob discovered later. Completion verifies the caller's SHA-256 before the blob becomes readable. Expiry takes the clock as a parameter instead of calling `Instant::now`, which is what makes the TTL rules testable at all; the tests drive time explicitly and cover both directions — an abandoned blob is reaped, a slow but live transfer never is. Transfer errors are grouped by what the caller should do next rather than by what went wrong internally: `UnknownBlob` means restart, `TransferRefused` means a budget is full and the same request may work later, `TransferFailed` means re-send. `BlobStore`'s `Debug` is written by hand so staged bytes cannot reach a log line. This replaces `ai.tinyhumans.tinydocs.Docx` rather than extending it. Returning a `BlobRef` where callers expect bytes is a breaking contract, and TinyBus's module guidance is explicit that those get a new interface name. It is not served alongside the old one because `module_export!` attaches its method list to the first entry in `provides` and leaves any others empty, so a second fully-declared interface would have to under-declare its members and break the invariant that manifest methods and dispatch members stay identical. Serving both needs a TinyBus change; retiring one at a pre-1.0 minor bump does not. The loader E2E test now covers all three formats through the real dynamic loader and moves an image across several chunks — a single-chunk transfer would not prove the offsets line up, which is the whole point of the change. The one `#[allow]` added is `clippy::unused_async` on the interface block: the macro rejects a non-async method outright, so the four transfer methods are async because the dispatch contract requires it, and the lint can never be actionable there. Co-authored-by: Medulla <medulla@tinyhumans.ai>
TinyBus gained chunked, flow-controlled streams (tinyhumansai/tinybus#9), which is the facility the hand-rolled staging area in the previous commit was standing in for. Inbound payloads now use it, and the half of that code they replace is deleted. What goes away is the risky half. `BeginBlob`, `PutChunk`, the append-only offset protocol, the upload digest check, the reserve-at-begin budget and the inbound TTL all existed to answer one question — what happens when a caller starts sending a document and never finishes — and TinyBus now answers it, per peer, with a window, a size cap and an idle timeout. A stream is also tied to the call that opened it and writable only by the peer that opened it, which is authorisation the blob ids never had: any peer that guessed an id could have written to somebody else's transfer. What stays is the output half, because replies cannot stream. `Interface::call` receives a member name and a JSON body — no caller identity, no connection — so a served object cannot open a stream back to whoever called it. A produced document is still held and pulled with `ReadOutput`, since returning it inline would put it through a 16 MiB JSON frame where a `Vec<u8>` costs ~3.5 bytes per byte. `outputs` is what is left of `blobs` once only that direction remains: still bounded four ways and still expiring, because TinyBus never unloads a module. A reply-stream seam upstream would delete it, and the spec now says so. A deck's images share one stream, concatenated in slide order, because a call has one stream and a deck has many pictures. Each image declares its `byte_len` in the spec rather than framing itself in the stream, which is what makes a truncated or over-long transfer a named rejection instead of a deck containing a picture assembled from two different images. A text-only deck passes no stream at all rather than opening an empty one. The E2E test moves an image pair across a real stream through the real dynamic loader and asserts the mismatch case, which is the only place the streaming paths can be tested honestly: a stream needs two connected peers and a broker, so a unit test against a bare struct cannot reach one. Net: 5 methods instead of 7, and the module no longer implements transfer. Co-authored-by: Medulla <medulla@tinyhumans.ai>
The bus-facing deck shape — the one whose images name a length in a stream rather than carrying bytes — lived in the private module crate. It belongs beside the spec it mirrors: a host driving the module over a bus needs those types, and the module crate is `publish = false`, so the host would have had to re-declare them. That is exactly the drift the `spec` carve-out exists to prevent. The wire deck is `serde` and nothing else, so it sits in `spec::presentation::wire` and is compiled in every build, including `--no-default-features`. The module crate re-exports it rather than owning it, and both sides now share one definition of the shape. Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
Warning Review limit reached
Next review available in: 19 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThe change expands the crate from DOCX generation to validated DOCX/PPTX generation and PDF text extraction. It adds shared specifications, staged output storage, streamed TinyBus inputs, chunked output reads, release operations, feature flags, and comprehensive unit and end-to-end tests. ChangesShared specification contracts
PPTX and PDF writers
Staged output storage
Documents TinyBus service
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
crates/tinydocs-module/tests/module_e2e.rs (1)
240-266: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the wire error name, not only failure.
mismatched.is_err()also passes if the call fails for an unrelated reason, such as a transfer or dispatch failure. Check the name so the test proves the length rejection.♻️ Proposed change
- assert!( - mismatched.is_err(), - "a stream shorter than the declared images should be refused" - ); + let err = mismatched.expect_err("a stream shorter than the declared images should be refused"); + assert_eq!( + err.wire_name(), + "ai.tinyhumans.tinydocs.Error.InvalidInput", + "the refusal should name the spec/stream mismatch" + );🤖 Prompt for 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. In `@crates/tinydocs-module/tests/module_e2e.rs` around lines 240 - 266, Update refuses_a_stream_that_contradicts_the_spec to inspect the returned error’s wire-level name instead of asserting only mismatched.is_err(). Verify it matches the expected stream-length validation error, while preserving the existing test setup and failure message.
🤖 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 `@crates/tinydocs-module/src/outputs/mod.rs`:
- Around line 271-274: Replace the predictable counter-based ID generation in
allocate_id with a cryptographically secure random capability for every output,
while preserving the String return type and uniqueness expectations. Update the
related output creation and ReadOutput/ReleaseOutput paths to use the generated
capability unchanged, rather than exposing or accepting predictable out-N
identifiers.
- Around line 207-211: Update ReadOutput’s read_chunk flow to refresh
output.last_read only when the computed range is non-empty (end > start), while
preserving the existing slice and return behavior. Add a regression test that
performs repeated zero-byte/end-of-document reads beyond IDLE_TTL and verifies
the output is reaped rather than kept alive.
In `@crates/tinydocs-module/src/service/mod.rs`:
- Around line 173-215: Update the image-length handling around the expected-byte
calculation and SlideImage construction: use checked addition when summing
WireSlideImage.byte_len values and when computing cursor + len, returning
BusError::MethodFailed with INVALID_INPUT_ERROR on overflow. Replace direct
payload slicing with payload.get(cursor..end), mapping an invalid range to the
same INVALID_INPUT_ERROR instead of panicking.
In `@docs/specs/tinybus-module.md`:
- Around line 34-43: Update the method count in the module description to match
the five methods listed in the code block, without changing the method
signatures or module details.
- Around line 59-69: Update the documented transfer error names to
Error.UnknownOutput and Error.OutputRefused, matching the definitions in the
module service. In the surrounding lifecycle description, replace the retired
“blobs” terminology with “outputs” while preserving the existing staging,
bounds, and expiration behavior.
In `@src/pdf/mod.rs`:
- Around line 83-84: Replace the PDF parsing dependency used by the extraction
flow around pdf_extract::extract_text_from_mem so the resolved lopdf version is
at least 0.42.0, or configure and enforce an equivalent nesting-depth limit
before parsing. Preserve the existing Error::extraction_failed mapping and
successful text extraction behavior.
In `@src/pptx/mod.rs`:
- Around line 207-212: Update the image sizing flow around fit_within so the
returned dimensions never exceed the image’s natural EMU width and height,
preserving the documented no-upscaling contract. Keep the existing slot
constraints for larger images and use the computed natural dimensions from
img.width_px and img.height_px.
In `@src/spec/image/mod.rs`:
- Around line 103-107: Treat the JPEG TEM marker (0x01) as standalone in the
marker parsing condition alongside SOI, EOI, RSTn, and fill bytes, so it does
not consume a segment length; update src/spec/image/mod.rs lines 103-107. Add a
regression test covering TEM before SOF in src/spec/image/test.rs lines 113-124.
In `@src/spec/presentation/wire.rs`:
- Around line 52-54: Update the documentation comment for the images field in
the wire presentation type to state that each WireSlideImage describes a byte
range in the concatenated image stream, rather than naming a staged blob.
- Around line 26-28: Update resolve_presentation to validate every
WireSlideImage.byte_len against MAX_IMAGE_BYTES before calling read_stream or
slicing, and accumulate the expected stream length with checked_add. Reject
oversized values and addition overflow as invalid input, preventing cursor + len
from wrapping or panicking.
---
Nitpick comments:
In `@crates/tinydocs-module/tests/module_e2e.rs`:
- Around line 240-266: Update refuses_a_stream_that_contradicts_the_spec to
inspect the returned error’s wire-level name instead of asserting only
mismatched.is_err(). Verify it matches the expected stream-length validation
error, while preserving the existing test setup and failure message.
🪄 Autofix
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 Plus
Run ID: 6e8bea62-c2c5-42f8-acbe-8e4bcce14a6a
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (28)
.gitignoreCargo.tomlREADME.mdcrates/tinydocs-module/Cargo.tomlcrates/tinydocs-module/src/lib.rscrates/tinydocs-module/src/outputs/mod.rscrates/tinydocs-module/src/outputs/test.rscrates/tinydocs-module/src/service/mod.rscrates/tinydocs-module/src/service/test.rscrates/tinydocs-module/tests/module_e2e.rsdocs/specs/tinybus-module.mdsrc/docx/mod.rssrc/docx/test.rssrc/error/mod.rssrc/lib.rssrc/pdf/mod.rssrc/pdf/test.rssrc/pptx/mod.rssrc/pptx/test.rssrc/spec/document/mod.rssrc/spec/document/test.rssrc/spec/image/mod.rssrc/spec/image/test.rssrc/spec/mod.rssrc/spec/presentation/mod.rssrc/spec/presentation/test.rssrc/spec/presentation/wire.rsvendor/tinybus
Update the optional pdf-extract dependency from 0.10 to 0.12, pulling in its newer transitive dependencies including an upgraded lopdf, rand, and getrandom stack. The lockfile is regenerated to reflect the new crate versions and to remove several intermediate dependencies that are no longer required. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Downgrade the windows-sys dependency from 0.61.2 to 0.52.0 across all transitive dependencies in the lockfile, and relax the ppt-rs version requirement from exact 0.2.14 to the more permissive 0.2 range to allow compatible patch updates. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Set `default-features = false` on the `ppt-rs` optional dependency to disable its `pdf-native` feature, which pulled in `pdfrs` and a chain of transitive dependencies including `syntect`, `yaml-rust`, `bincode`, and `ttf-parser`. This eliminates three RUSTSEC unmaintained advisories and removes over 400 lines of lock-file entries, since nothing in this crate calls the PDF export functionality that the subtree existed to support. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
The ppt-rs dependency previously had `default-features = false` to avoid pulling in the pdf-native feature and its transitive dependencies, which carry unmaintained RUSTSEC advisories. However, ppt-rs 0.2.24 unconditionally imports `pdfrs` in its slide render module without a cfg guard, causing a build failure when the feature is disabled. Default features are now enabled to restore compilation, and the advisory exemptions are documented in deny.toml until ppt-rs fixes the conditional compilation. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
# Conflicts: # Cargo.lock
Ten findings from the review, all legitimate. Grouped by what they actually were. **A real vulnerability, fixed rather than ignored.** `pdf-extract` was pinned at 0.10, which resolves lopdf 0.38 — RUSTSEC-2026-0187, an uncatchable stack-overflow SIGABRT from a ~21 KB PDF with deeply nested objects. The `%PDF-` and size checks do nothing against it. Pinning `pdf-extract = "0.12"` resolves lopdf 0.42.0, where it is fixed. This matters more than the advisory count suggests: the extraction path is the one that sees untrusted user attachments. **Caller-controlled arithmetic could wrap.** Every `byte_len` in a wire deck comes from the caller, and the total was summed unchecked: `u64::MAX + 1` wraps to zero in a release build, a zero-byte stream then satisfies the aggregate check, and the first slice panics. Each length is now bounded against `MAX_IMAGE_BYTES` before it is summed, the sum and the cursor use `checked_add`, and slicing goes through `payload.get(..)` so a bad range is an `InvalidInput` rather than a panic. **Output ids were authorisation, and were guessable.** `ReadOutput` and `ReleaseOutput` take an id and nothing else — a method receives no caller identity, so the store cannot bind an output to whoever produced it. That makes the id the whole authorisation story, and it was `out-1`, `out-2`. Any peer on the bus could take another peer's document. Ids are now 128 bits of OS randomness. My own comment claiming ids were "never authorisation tokens" was the tell that this was wrong; it is corrected rather than deleted. **Empty reads refreshed the TTL.** A zero-length read, or one at the exact end of a document, returns nothing and costs nothing — and reset `last_read`, so repeating it pinned an output in the store forever. Only a read that returned bytes counts as activity now. **A valid JPEG could be rejected.** TEM (`0xFF01`) is a standalone marker with no length field. The walk read the following two bytes as one, desynchronised, and gave up on a file that places TEM before its frame header. **Two of my own doc comments contradicted the code**, which is worth fixing loudly. The pptx module claimed images are never upscaled past their natural size; the implementation scales to fill the slot in both directions, which is what the code this was ported from did and what its test asserts — so the sentence was wrong, not the behaviour. And the spec said "seven methods" over a list of five, named `Error.UnknownBlob` / `Error.TransferRefused` where the code defines `UnknownOutput` / `OutputRefused` (a caller matching the documented names would never match), and still called outputs "blobs" after that concept was removed. **Supply chain.** Three unmaintained advisories remain, ignored in `deny.toml` with the exposure and the plan written out, per that file's own rule. Two of them (`yaml-rust`, `bincode`) arrive through ppt-rs's `pdf-native` default feature — a PDF exporter behind a syntax highlighter that `.pptx` synthesis never reaches. It cannot simply be turned off: ppt-rs 0.2.24 fails to build with `default-features = false` because `src/export/slide_render.rs` has an unguarded `use pdfrs::...`. When that is fixed upstream, one line drops `pdfrs`, `syntect`, `yaml-rust`, `bincode` and `pulldown-cmark` together. `CC0-1.0` is added to the license allowlist for `constant_time_eq` — a public-domain dedication, strictly more permissive than everything already on the list. The E2E's mismatch case now asserts the wire error name instead of only that the call failed, which would also have passed if it failed for an unrelated reason. New regression tests: empty reads not extending the TTL, ids being unguessable and unique, and a TEM-before-SOF JPEG. Co-authored-by: Medulla <medulla@tinyhumans.ai>
senamakel
force-pushed
the
tinydocs-port
branch
from
August 11, 2026 06:03
bfe2a17 to
c97d2ac
Compare
12 tasks
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.3794 · 245,489 in / 97,152 out · 59,664 cached (24%) · z-ai/glm-5.2
critique: $0.1427 · 58,746 in / 49,605 out · 32,347 cached (55%) · z-ai/glm-5.2
security: $0.0930 · 51,232 in / 29,957 out · 27,189 cached (53%) · z-ai/glm-5.2
tests: $0.0829 · 67,034 in / 13,611 out · 64 cached (0%) · z-ai/glm-5.2
description: $0.0609 · 68,477 in / 3,979 out · 64 cached (0%) · z-ai/glm-5.2
|
|
||
| [features] | ||
| default = ["docx"] | ||
| default = ["docx", "pptx", "pdf"] |
There was a problem hiding this comment.
Keep pdf-extract out of the default feature set
pdf-extract is added to the default feature set via default = ["docx", "pptx", "pdf"], but its own dependency comment argues the opposite. The comment at # \.pdf` text extraction via `pdf-extract`.says the parser stack is "only the extraction path needs, so a host that never reads a PDF should not carry it." Puttingpdfindefaultmeans every default consumer pullslopdfand the CFF/Type1/CMap parsers regardless, contradicting that rationale. Unlike thepptxentry, which explicitly explains why it is forced on, thepdfentry gives no reason it must be a default rather than an opt-in feature. Movepdfout ofdefault` so the extraction stack is opt-in as the comment intends.
[RULE] Enable only needed dependency features, using default-features = false to trim the tree; gate optional deps behind Cargo features. ·
|
|
||
| ```text | ||
| GenerateDocx(DocumentSpec) -> Vec<u8> | ||
| GenerateDocx(DocumentSpec) -> OutputRef |
There was a problem hiding this comment.
Define how OutputRef relates to the output_id parameter
The generation and extraction methods return OutputRef, but ReadOutput and ReleaseOutput take output_id. The spec never states whether OutputRef is the output ID, contains it, or wraps it. An implementer cannot determine which value to pass back to ReadOutput from what the interface listing shows. Either rename the return type to match (OutputId) or add a sentence defining OutputRef and which field the caller supplies as output_id.
**[RULE] ** ·
| method list to the first entry in `provides` and leaves the rest empty, so a | ||
| second fully-declared interface is not expressible without a TinyBus change. | ||
|
|
||
| ## Invariants and constraints |
There was a problem hiding this comment.
Add output-ID unguessability to the constraints section
The prior finding "Make output IDs unguessable so they can serve as access capabilities" still stands at the spec level. ReadOutput(output_id, offset, len) returns document content to any caller that supplies a valid output_id; if the IDs are sequential or otherwise predictable, one D-Bus caller can enumerate and read another's staged documents. The "Invariants and constraints" section lists per-document bounds, a total cap, a count cap, and an idle TTL, but says nothing about the ID format. Add a constraint that output IDs must be unguessable (e.g., 128-bit random tokens).
**[RULE] ** ·
|
|
||
| [features] | ||
| default = ["docx"] | ||
| default = ["docx", "pptx", "pdf"] |
There was a problem hiding this comment.
Keep the pdf-extract untrusted-parser stack out of the default feature set
The pdf feature is included in default = ["docx", "pptx", "pdf"]. The pdf-extract crate brings lopdf and CFF/Type1/CMap parsers that consume untrusted PDF input. Making this opt-out rather than opt-in means every downstream consumer inherits a parser-for-untrusted-data attack surface even when they never read a PDF. The previous default was docx alone; the new parsing dependency should stay behind an explicit feature flag, not be pulled in by default.
[RULE] Enable only needed dependency features, using default-features = false to trim the tree; gate optional deps behind Cargo features. ·
What this change touches28 files, +4146 -417 across 9 components. The code graph knows nothing about these files yet — normal for newly added files, and a cold index otherwise. flowchart LR
n0["crates/tinydocs-module<br/>7 files +1507 -62"]:::changed
n1["src/spec<br/>8 files +1549 -2"]:::changed
n2["src/pptx<br/>2 files +586 -0"]:::changed
n3["src/docx<br/>2 files +13 -312"]:::changed
n4["src/pdf<br/>2 files +203 -0"]:::changed
n5["root<br/>4 files +166 -19<br/>2 findings"]:::flagged
n6["docs/specs<br/>1 file +63 -19<br/>2 findings"]:::flagged
n7["src<br/>1 file +34 -3"]:::changed
n8["src/error<br/>1 file +25 -0"]:::changed
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Green: changed. Grey: untouched, reached through an import or a call. Orange: has findings. Red: has a finding that blocks the merge.
Changed files
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Teaches TinyDocs the other two document formats and makes the TinyBus module the
way a host reaches all three.
.pptxsynthesis and.pdftext extraction areported in behind their own feature gates, the spec types move out from behind the
format gates so a host can share the wire contract without any writer, and the
module's bus surface is rebuilt on TinyBus streams.
The motivating consumer is OpenHuman, which carries
docx-rs,ppt-rsandpdf-extracttoday purely to support three agent tools. With this landed itcarries none of them: 39 crates leave its lockfile and its product profile drops
from 505 to 448 unique crate names.
Related issue
None.
API or behavior changes
Additive on the library side.
tinydocs::specis new and is where every spectype, limit and
validatenow lives — compiled in every build, including--no-default-features, withserdeand the crate error type as its onlydependencies. The format modules re-export what they consume, so
tinydocs::docx::DocumentSpecstill names the same type and existing code keepscompiling. New
pptxandpdfmodules behind new default-ON features. NewError::ExtractionFailedvariant (Erroris#[non_exhaustive], so this doesnot break a match).
Breaking on the module's bus surface.
ai.tinyhumans.tinydocs.Docxis retiredand replaced by
ai.tinyhumans.tinydocs.Documents. Returning a handle wherecallers expect inline bytes is a breaking contract, and FOR_AGENTS.md is explicit
that those get a new interface name.
It is replaced rather than served alongside because
module_export!attachesits
methodslist to the first entry inprovidesand leaves any others empty —a second fully-declared interface is not expressible today, and a manifest that
under-declared its members would break the invariant that manifest methods and
dispatch members stay identical. Noted under Open questions in the spec.
Validation
Commands actually run, with their outcome:
cargo fmt --all -- --check— cleancargo clippy --all-targets --all-features -- -D warnings— cleancargo build --all-targets --all-features— cleancargo test --all-features— 121 passed, 0 failed, 1 ignored (the loader E2E).github/scripts/check-file-coverage.sh 90 coverage.json— every file ≥ 90%Also run, because they are the ones that actually prove this change:
cargo check -p tinydocs --no-default-features— the spec-only build compiles,and
cargo tree -p tinydocs --no-default-features -i docx-rsfinds nothing.If this ever fails, the carve-out is broken and no host can take the contract
without a codec.
docx,pptx,pdf, all-features.TINYDOCS_TEST_MODULE=… cargo test -p tinydocs-module --test module_e2e -- --ignored— the builtcdylibthrough the real dynamic loader and a realbroker.
Tests
121 unit tests plus the loader E2E.
The blob/stream work is weighted towards refusals rather than happy paths,
deliberately: a module is never unloaded, so what decides whether an abandoned
transfer leaks is the bounds and the expiry, not whether bytes move. Every bound
has a test that trips it, and the TTL is tested in both directions — an abandoned
output is reaped, a slow consumer reading in chunks never is. The clock is a
parameter rather than
Instant::now, which is what makes those deterministic.The E2E covers all three formats through the real loader and moves an image pair
across a stream in several chunks, because a single-chunk transfer would not prove
the reassembly. It also asserts the refusal case: an image stream that does not
match the lengths the deck declares.
Two deliberate gaps:
a broker between them, so there is no honest way to test one against a bare
struct; they are covered in the E2E instead.
.pdffixture is a PDF assembled in-process, cross-reference offsetscomputed from the bytes emitted, rather than a checked-in binary that could go
stale.
Documentation
README.md— the separable-spec section, the presentation limits table, therewritten TinyBus module section, feature flags, layout.
docs/specs/tinybus-module.md— rewritten for the new interface, the transferasymmetry, and the bounds.
mechanics: why images are bytes and not references, why the output store exists
at all, why transfers are append-only.
Checklist
#[allow(...)],#[ignore], or relaxed lints.envcontents in the diff or the descriptionOn the second box — one
#[allow]was added, and I would rather flag it thantick the box.
crates/tinydocs-module/src/service/mod.rscarries#[allow(clippy::unused_async)]on the interface block.tinybus::interfacerejects a non-async method outright, so the two output methods are
asyncbecausethe dispatch contract requires it, not because they await anything — the lint can
never be actionable in that block. It is scoped to the one block with a
reason,not a crate-level relax. Happy to take a different approach if you would rather.
The one
#[ignore]is the pre-existing loader E2E, which cannot run without abuilt artifact.
Note for the reviewer
The consuming OpenHuman PR pins this branch's head as its
vendor/tinydocsgitlink and cannot merge until this lands and v0.2.0 is cut — its module
registry pins per-platform SHA-256 digests from the release, and the ones there
now are v0.1.11's, which predate every commit here.
Summary by CodeRabbit
.pptx) generation with text, bullets, notes, captions, and PNG/JPEG images.