Repository navigation
Conversation
Update the vendored tinybus submodule to the latest upstream commit. Auto-committed-on: macbook
Update the vendored tinybus submodule to the latest upstream commit. Auto-committed-on: macbook
Register the new tinydocs-client crate as a workspace member and include it in the default members so it builds with the rest of the workspace by default. Auto-committed-on: macbook
Introduce a new tinydocs-client crate exposing the client library entry point. A recording broker test double is included so client behaviour can be exercised without a live broker. Auto-committed-on: macbook
Extend the recording broker fixture with configurable malformed replies and a separate resume signal so blocked reads can be released independently of output release. Add cases asserting that a malformed typed reply surfaces as a failure and that a digest mismatch releases the held output. Auto-committed-on: macbook
…oad stream Add assertions that generating a PPTX from a deck with no images succeeds and sends a null upload argument, confirming the text-only path avoids opening a stream. Auto-committed-on: macbook
Adds a recording-broker case asserting that an InvalidInput fault on read output surfaces as DocumentCallError::InvalidInput, still releases the output, and leaves no outputs behind. Auto-committed-on: macbook
Extend the recording broker test to check that a render call failing on invalid base64 input still releases the output it held, and clone the resume notification handle so the fixture lock is not held across the notify. Auto-committed-on: macbook
…calls The README now describes the SDK additions the client contract tests require: detailed streaming calls that keep module readiness, registry-address validation, timeout, and failure reporting, plus detailed calls on `ModuleObjectClient` so held-output wire errors retain their contract distinction. The recording broker tests were reformatted to match, with no change in what they assert. Auto-committed-on: macbook
The crate-level documentation no longer claims the client takes the host's bus connection, matching the actual constructor signature. Test assertions were also tightened to hold the fixture state lock across both checks and to avoid borrow conflicts. Auto-committed-on: macbook
Update the vendored tinybus submodule to the latest upstream commit. Auto-committed-on: macbook
Update the tinybus dependency to 0.2.2 and adjust the recording broker test to assert the failure variant inline. The test no longer checks that the error message omits private generator detail. Auto-committed-on: macbook
…input The recording broker now drains the stream argument before returning a fault, so tests exercise the same path as a real broker that fails a streamed call. Added cases asserting that extract_document and inspect_image still map wire errors to InvalidInput and Failed, and the README now describes the streaming and held-output call paths the SDK exposes. Auto-committed-on: macbook
Update the tinybus and tinybus-module dependencies to 0.2.2, which pulls in icu_casemap, rawzip, and unicode-normalization while dropping the zip 2.x and arbitrary crates. This keeps the module aligned with the current TinyBus host ABI. Auto-committed-on: macbook
Renamed the local fixture constructor to test_fixture and updated all call sites, and dropped the now-unused Reason import. This avoids the helper shadowing the fixture variable and keeps the test module free of unused imports. Auto-committed-on: macbook
Auto-committed-on: macbook
Introduce a client type that owns TinyDocs contract calls and held-output transfer, along with an error type for document call failures, and wire in tracing for diagnostics. Auto-committed-on: macbook
…e failures Adds integration tests for the client's error paths: a disabled module reports Unavailable without issuing any call, local image bounds reject empty and oversized input before dispatch, non-UTF-8 text outputs fail while still releasing the held output, and a release failure does not discard already-collected bytes. The recording broker gains an enabled flag and per-member output overrides to support these cases, and the client now implements Debug. Auto-committed-on: macbook
Extend the recording broker fixture with a configurable release delay and client timeout so tests can verify that ordinary output release honors the module client's configured deadline. Add tests confirming that credential-like remote text in invalid-input errors is redacted from both display and debug formatting. Auto-committed-on: macbook
Output release calls no longer wrap the RPC in a five second timeout, so a slow release is awaited to completion instead of being abandoned. The timeout was dropped because the release path already tolerates failures and the extra wrapper only added noise. Auto-committed-on: macbook
Display for DocumentCallError now reports only the error category instead of the module-provided text, matching Debug so untrusted remote text cannot leak into logs or diagnostics. The raw message is still available through a new raw_message accessor for callers that apply their own redaction policy. Auto-committed-on: macbook
Explain that DocumentCallError keeps unavailable, invalid-input, and execution failures distinct while its Debug and Display forms expose only the category, and note that callers needing input detail can opt into raw_message() and apply their own redaction policy before surfacing that untrusted text. Auto-committed-on: macbook
…tion The README now states that cancellation triggers a best-effort release call on the caller's Tokio runtime, and that if the runtime is gone or the call cannot be delivered, module-side expiry remains the fallback for reclaiming the output. Auto-committed-on: macbook
Wrapped `TinyDocs` in backticks throughout the doc comments and added the missing semicolons on the debug log statements. Also replaced the lossy `MAX_DOCUMENT_BYTES as usize` cast with a checked conversion so the capacity clamp stays correct on 32-bit targets. Auto-committed-on: macbook
… tests Refactored the recording broker fixture so call recording, fault injection, malformed replies, and member dispatch live in separate helpers, and split the oversized module wire failure test into focused cases covering input availability, streamed failures, held output release, and malformed typed replies. Auto-committed-on: macbook
…awaits The assertions in the held-output test now take the fixture state lock inside a block so the guard is dropped before the next fixture is created, keeping the test free of a lock held across subsequent async work. Auto-committed-on: macbook
Reformat the recording broker test to satisfy rustfmt and drop the fixture state lock before the test continues, so the guard is not held across the remaining assertions. Auto-committed-on: macbook
The single held-output test was divided into three separate tests covering in-order reads with release after success, release on bad digest or declared size, and release for truncated, invalid, and overlong chunks. This makes failures easier to localise to a specific release path. Auto-committed-on: macbook
Add regression tests that transfer the well-known bus name to a replacement peer after the output object has resolved, asserting that generation, read, and release all stay pinned to the original peer for both docx and pptx flows. Auto-committed-on: macbook
The HandoverAccess test double and its atomic lookup counter were no longer referenced, so they have been deleted to keep the recording broker fixture lean. A handover_to field was added to the broker state to support the same scenario through the shared fixture instead. Auto-committed-on: macbook
…hecks The DOCX and PPTX peer-hop tests now gather their byte, call-count, and input-stream expectations into booleans and tuples so a single assert reports every mismatch at once. This replaces the chain of separate assert_eq! calls, which stopped at the first failure and hid the rest of the state. Auto-committed-on: macbook
The handover fixture now seeds distinct output overrides for the original and replacement members so the held pptx assertion reflects the bytes each member actually serves. The generate_pptx handler holds its output against the calling member rather than a fixed payload. Auto-committed-on: macbook
The pptx handover tests asserted against the wrong byte strings, so the original and replacement expectations were swapped or mismatched. The assertions now compare against the bytes each peer actually returns. Auto-committed-on: macbook
Update the vendored tinybus submodule to the latest upstream commit. Auto-committed-on: macbook
Each operation now binds the module once and reuses that pinned client for both the method call and the output object client, instead of resolving the object client separately. This keeps the call and its output transfer on the same module instance. Auto-committed-on: macbook
Advance the workspace version to 0.2.5 and update the internal tinydocs-bus dependency to match, while moving the client and module crates onto tinybus 0.3.2. The recording broker test fixture now records the new release version and tag URL. Auto-committed-on: macbook
…thods The recording broker now triggers a peer handover after extract_text and convert_markdown, and the handover fixture seeds distinct original and replacement outputs for both methods so tests can verify which peer served the result. Auto-committed-on: macbook
Add tests asserting that streamed text and multi-page render outputs stay bound to their producing peer after a module alias handover, and update the README to describe per-operation pinning. Auto-committed-on: macbook
Updated the client documentation to explain that operations returning held outputs bind the serving peer for their whole lifetime, while stateless one-call operations still go through ModuleClient. Added a test asserting that a detached output owner fails rather than falling through to an alias replacement. Auto-committed-on: macbook
Updated the pinned commit for the shared module-release workflow to pick up the latest changes from the central repository. Auto-committed-on: macbook
The release workflow now accepts a release_mode input that selects either the ordinary main path or a reviewed-epic admission path, with reviewed_pr required in the latter case. The resolve job validates both inputs and gates the existing packaging and module-release jobs to main mode while routing reviewed-epic releases to a dedicated reusable workflow. Auto-committed-on: macbook
Updated the locked versions of the tinybus and tinydocs workspace crates to reflect their new releases. Auto-committed-on: macbook
The output_client helper no longer takes &self since it never used the client instance, so it is now called as Self::output_client from each method. Auto-committed-on: macbook
The recording broker test fixture now builds its client with a two-second timeout so the handover test cannot hang indefinitely when a peer stops responding. Auto-committed-on: macbook
Tiny Sweeper review
Last completed reportTiny Sweeper reviewTiny Sweeper completed its review; deterministic results follow. State: Incomplete Review snapshot
Completeness: Incomplete What changedAdds a host-side tinydocs-client crate with typed TinyDocs calls, peer-pinned held-output transfer, redacted call errors, and documentation; prepares workspace version 0.2.5 with TinyBus 0.3.2 pins across the module and client crates; and adds a reviewed-epic release path to the release workflow while gating the ordinary main path behind `release_mode == 'main'`. Features
Tests
FindingsNo active actionable findings. Lower-confidence notes
Could not review: crates/tinydocs-client/src/client.rs, crates/tinydocs-client/tests/recording_broker.rs Before merge
How this fits togetherflowchart LR
n0["Result"]:::impacted
n1["collect"]:::impacted
n2["OutputRef"]:::impacted
n3["bind"]:::impacted
n4["output_client"]:::impacted
n5["dispatch_member"]:::impacted
n1 -->|uses| n0
n1 -->|uses| n2
n3 -->|uses| n0
n4 -->|uses| n0
n5 -->|uses| n0
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
Agent review detailscritique
security
tests
commits
description
Evidence and run details
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84b38907af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Replaced the emptiness assertions in the recording broker test with explicit length checks against zero, so a failure reports the actual number of recorded calls instead of only that the list was non-empty. Auto-committed-on: macbook
![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, but could not review everything, so this is not an approval: .github/workflows/release.yml, crates/tinydocs-client/src/client.rs, crates/tinydocs-client/tests/recording_broker.rs, tinysweeper/tests.
$0.0412 · 1,480,543 in / 255,748 out · 527,360 cached (36%) · ladder/vectors-oai3, deepseek-v4.1-flash · 701 embedded
critique: $0.0181 · 586,690 in / 101,165 out · 259,200 cached (44%) · deepseek-v4.1-flash
security: $0.0166 · 686,900 in / 111,515 out · 195,968 cached (29%) · deepseek-v4.1-flash
tests: $0.0027 · 79,125 in / 18,943 out · 39,040 cached (49%) · deepseek-v4.1-flash
description: $0.0033 · 97,313 in / 21,684 out · 33,152 cached (34%) · deepseek-v4.1-flash
tinysweeper
Bot
added
the
priority: p3
Add tests asserting that an incompatible contract is classified as unavailable for both detailed and SDK errors, and that the peer's remote detail text never leaks into the message or debug output. Also verify that a module fault carrying a compatibility failure code is still classified as failed. Auto-committed-on: macbook
… cases Extracted the shared unavailable-error assertions into a helper and split the combined test into separate cases for the detailed failure and SDK error paths. The module fault check was dropped from this test, leaving each case focused on a single classification path. Auto-committed-on: macbook
There was a problem hiding this comment.
tinysweeper found nothing blocking, but could not review everything, so this is not an approval: crates/tinydocs-client/tests/recording_broker.rs, tinysweeper/tests.
$0.0168 · 346,164 in / 67,966 out · 116,608 cached (34%) · ladder/vectors-oai3, deepseek-v4.1-flash · 701 embedded
critique: $0.0021 · 61,695 in / 14,549 out · 33,920 cached (55%) · deepseek-v4.1-flash
security: $0.0037 · 118,839 in / 15,407 out · 39,936 cached (34%) · deepseek-v4.1-flash
tests: $0.0022 · 39,048 in / 15,353 out · 1,792 cached (5%) · deepseek-v4.1-flash
description: $0.0083 · 95,910 in / 20,413 out · 40,704 cached (42%) · deepseek-v4.1-flash
Failures caused by an incompatible module contract are now classified as unavailable rather than a generic failure, matching how an unavailable module is already handled so callers can react to both conditions the same way. Auto-committed-on: macbook
…replies The malformed typed stream reply test now asserts that a contract mismatch surfaces as DocumentCallError::Unavailable rather than Failed, and the test name was updated to reflect that expectation. Auto-committed-on: macbook
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26c533047e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| [dependencies] | ||
| # Module loading and typed failure reporting stay host-owned; this is the | ||
| # published host-facing SDK without its native loader. |
There was a problem hiding this comment.
For an ordinary main release of 0.2.5, the checked .github/workflows/release.yml source job still runs only cargo package --locked --package tinydocs-bus (line 82) and uploads only that crate plus the TinyBus source (lines 92-94). This manifest explicitly declares tinydocs-client as the published host-facing SDK, so the source artifact handed to the normal immutable release omits the new SDK package; add it to the package/upload list with the matching bus package available first, or mark it private.
Useful? React with 👍 / 👎.
| &self, | ||
| document: &[u8], | ||
| spec: &RenderPdfSpec, | ||
| ) -> Result<Vec<(u32, Vec<u8>)>, DocumentCallError> { |
There was a problem hiding this comment.
When RenderPdf returns selected pages (for example, the fixture returns page_count = 3 for pages 1 and 3), this return type and the push at line 241 expose only (page, bytes), discarding RenderedPdf::page_count and each page's width and height. docs/specs/document-intake-render.md:22-28 defines those values as returned page metadata, and tinydocs-bus/src/intake/types.rs:92-109 carries them, so callers of the typed client cannot discover the total source page count without reparsing the PDF or retain the supplied raster dimensions; return a collected typed structure that preserves this metadata while replacing each OutputRef with bytes.
AGENTS.md reference: AGENTS.md:L201-L203
Useful? React with 👍 / 👎.
| type: string | ||
| required: true | ||
| release_mode: | ||
| description: Use the ordinary main path or reviewed-epic release admission |
There was a problem hiding this comment.
When maintainers use the new reviewed-epic admission path rather than the ordinary main path, the repository release instructions still state that Release checks every tagged commit is on main and document no reviewed_pr procedure. That leaves the operational contract contradictory for this high-impact path; update the release runbook in the same change to state the reviewed-epic exception and its required validation.
AGENTS.md reference: AGENTS.md:L245-L248
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
tinysweeper found nothing blocking, but could not review everything, so this is not an approval: crates/tinydocs-client/src/client.rs, crates/tinydocs-client/tests/recording_broker.rs.
$0.0276 · 767,465 in / 186,451 out · 315,392 cached (41%) · ladder/vectors-oai3, deepseek-v4.1-flash · 701 embedded
critique: $0.0050 · 168,586 in / 32,925 out · 67,200 cached (40%) · deepseek-v4.1-flash
security: $0.0144 · 391,090 in / 97,041 out · 165,888 cached (42%) · deepseek-v4.1-flash
tests: $0.0035 · 79,424 in / 24,760 out · 41,984 cached (53%) · deepseek-v4.1-flash
description: $0.0042 · 96,630 in / 29,341 out · 40,064 cached (41%) · deepseek-v4.1-flash
This branch has not been deployed
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.
A public bus-name handover between document generation and output collection could send reads and release calls to another module instance with the same output IDs. Add
tinydocs-clientwith typed host calls and retain one ordinary pinned peer through each GenerateDocx, GeneratePptx, ExtractText, ConvertMarkdown, and RenderPdf operation, including uploads, streamed reads, explicit release, and cancellation cleanup.The client preserves linked-module and fixture operation. ExtractDocument and InspectImage remain independent calls. Disconnects fail without reading or releasing the replacement peer's output. Drop cleanup remains best effort while the caller runtime is available.
Prepare TinyDocs 0.2.5 against published TinyBus 0.3.2 and add the guarded reviewed-epic release route. The shared release profile builds the native artifacts, bus source package, and pinned SDK source from the admitted commit.
Validation:
Hosted CI is pending on this head. Release follows review and verified merge into the canonical epic branch.
Refs senamakel#1. Part of tinyhumansai/openhuman#8056.
Review follow-up: 26c5330 maps incompatible contracts to Unavailable in both client classifiers, with 6 unit and 19 broker tests passing. Latest hosted functional/platform checks pass; supply-chain remains blocked by ZIP advisories in direct/transitive dependencies. No advisory ignores were added.