Skip to content

feat: port the memory engine into a TinyBus module - #4

Merged
senamakel merged 79 commits into
mainfrom
tinymemory-module
Aug 12, 2026
Merged

feat: port the memory engine into a TinyBus module#4
senamakel merged 79 commits into
mainfrom
tinymemory-module

Conversation

@senamakel

@senamakel senamakel commented Aug 12, 2026

Copy link
Copy Markdown
Member

Summary

Ports the memory engine out of the host binary and into a TinyBus module, the
same shape tinydocs and tinywallet already use: a cdylib speaking the
module ABI, admitted through the ABI/manifest/SHA-256 gates and attached to the
host's private in-process broker as an ordinary bus peer.

Two things are worth stating plainly up front, because the first one is the
reason a reader might expect a different justification:

  • This sheds zero dependencies. Measured on both the contributor and product
    profiles with dep-sim.py: 0 third-party crates, 0 native builds. rusqlite
    has five parents in the host, so moving one of them changes nothing.
  • The payoff is build critical path, not dep count. cargo build --timings
    puts the engine at ~14.7s of serial critical path (176s → ~161s, ≈8.4%), and
    the architectural boundary survives compilation in a way a Cargo feature does
    not.

Related issue

None.

API or behavior changes

Additive, no breaking changes.

  • New tinymemory_api::wire — the error-name table, used by both ends so the
    names cannot drift. One name per MemoryError variant rather than per outcome
    class: the host is itself a MemoryProvider to callers above it, and get's
    contract makes a miss Ok(None) while Invalid is a real failure, so
    collapsing variants is observable. PathEscape must never arrive as Invalid.
    An unrecognised name decodes to Other, never Invalid — telling a caller its
    input was wrong when it was not sends it into a rewrite loop.
  • New DriverClass::Module. A host fact, never self-reported; each variant gates
    policy, and neither Embedded nor External describes an in-process peer
    reached over a bus.
  • New crate tinymemory-module (crates/tinymemory-module), serving
    ai.tinyhumans.tinymemory.Memory at /ai/tinyhumans/tinymemory/Memory with 12
    methods.
  • adapters/tinycortex: added entry_to_tinycortex and
    namespace_summary_to_tinycortex, exhaustively destructuring as the rest of
    that module does.

Validation

Commands actually run, with their outcome:

  • cargo fmt --all -- --check — clean
  • cargo clippy --all-targets --all-features -- -D warnings — clean
  • cargo build --all-targets --all-features — clean
  • cargo test --all-features — green

Plus, for the module (its own workspace, so the root --all-targets never
reaches it — this is why CI grew a module job):

  • cargo fmt / clippy -D warnings / build / test under
    --manifest-path crates/tinymemory-module/Cargo.toml — 22 lib tests green
  • loader E2E, one process per test — 8 green

Tests

  • api/src/wire_tests.rs (7) — round-trips every variant, asserts every name is
    distinct, and pins the three collapses that would be bugs: an unknown name is a
    backend failure and not an input error, a path escape does not become
    Invalid, a miss stays NotFound.
  • crates/tinymemory-module/tests/module_e2e.rs (8) — a real broker, a real
    dlopen. Seven are #[ignore]d, and that is not lint-dodging: Broker::spawn
    binds its tasks to the creating tokio runtime, so a second #[tokio::test] in
    the same process finds a broker whose tasks died with the first and hangs
    until some deadline above it fires. Any test driving a real module has to be
    alone in its process. CI runs them in a loop with timeout 300.
  • Module unit tests (22) cover config validation, credential stripping, and the
    embedding seam.

The recall E2E asserts the host embedder's call count rather than result
ranking. Retrieval tuning is not what this port changes, and an assertion on
ranking would fail for reasons unrelated to the seam. Noted in the test.

Documentation

  • docs/specs/tinybus-module.md — the full spec, including the zero-shed
    finding and the critical-path measurement, so the next reader does not redo
    the analysis and reach the wrong conclusion from dep counts alone.

Two notes for reviewers, both deliberate:

  • The module carries no credentials. resolve_api_key returns None
    unconditionally and ModuleConfig::strip_host_credentials drops
    agentmemory_secret before the config crosses the boundary, warning when it
    does. This was found while debugging: an earlier structural test only checked
    top-level config keys and missed a nested secret.
  • worker_threads = 2, lazy = false. A recall that triggers an embed makes
    an outbound call from inside an inbound one, which deadlocks on a single
    worker; and store bring-up opens a database and runs migrations, so deferring
    it to first call would move that cost somewhere surprising.

crates/tinymemory-module is its own workspace root. A path dep into a nested
workspace resolves workspace.package against the enclosing root, and
exclude governs membership rather than inheritance resolution — so the
inherited-field errors could not be fixed from the root manifest. There is a
comment on the exclude entry explaining this.

Checklist

  • The change is focused on one logical change
  • No new #[allow(...)], #[ignore], or relaxed lints — seven #[ignore]s
    added deliberately
    , for the runtime-affinity reason above; they are run by
    CI one process at a time, not skipped
  • No secrets, tokens, or .env contents in the diff or the description

Summary by CodeRabbit

  • New Features

    • Added support for loadable TinyMemory modules with storage, recall, import/export, and lifecycle operations.
    • Added host-provided embedding support with batching and dimension validation.
    • Added module driver classification and stable error handling for transport operations.
  • Release Improvements

    • Added automated native module builds and packaged releases for Linux, macOS, and Windows.
    • Added release verification and checksum metadata.
  • Documentation

    • Added comprehensive module setup, configuration, security, and operational guidance.

senamakel and others added 30 commits August 11, 2026 23:48
Updated the pinned commit of the tinybus vendored submodule to incorporate upstream changes.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds the initial Cargo.toml configuration for the tinymemory-module crate, establishing its metadata and dependencies to enable building and publishing the module.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds the initial Cargo.toml manifest for the tinymemory-module crate, establishing its package metadata and dependencies to enable building and publishing the module.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The import of `HashMap` from `std::collections` was no longer used in the config module, so it has been removed to keep the code clean and avoid compiler warnings.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduce a new embedding module that provides functionality for generating and managing embeddings within the tinymemory system. This change enables vector-based similarity search and storage capabilities.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
When the embedding module receives an empty input, it now returns an empty vector instead of attempting to process the input and potentially causing an error. This change ensures consistent behavior for edge cases where no text is provided.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Return an empty vector when the input text is empty, preventing a panic in the underlying embedding model that does not accept empty strings. This ensures the embedding function behaves gracefully for edge cases where no text is provided.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
When the memory service receives an empty list of memories, it now returns an empty result instead of panicking or producing an error. This change ensures graceful handling of edge cases where no memories are available for processing.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Ensure that an empty payload in a wire message does not cause a panic during deserialization by returning an appropriate error instead. This prevents a crash when processing malformed or incomplete messages.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The wire test for deserializing an empty payload was asserting the wrong error variant, causing a false positive. Updated the assertion to match the actual error returned by the parser, ensuring the test correctly validates the expected failure behavior.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Fixed an issue where error handling in the API library was not properly propagating errors, which could lead to silent failures. The change ensures that errors are correctly returned to the caller instead of being ignored.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Removed an unused import statement from the API library's root module to clean up the code and eliminate a compiler warning.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
When the memory service receives a request to list memories but no memories exist, it now returns an empty list instead of an error. This change ensures consistent behavior with other list operations and prevents unnecessary error handling on the client side.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
When the memory service receives a request to list memories but no memories exist, it now returns an empty list instead of failing with an error. This change ensures consistent behavior across different memory states and prevents unnecessary error handling in client code.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
When the memory list is empty, the service now returns an empty result instead of panicking. This fixes a crash that occurred when querying memories before any had been stored.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Fixes a bug where the memory allocation logic could return an incorrect pointer when the requested size exactly matches the remaining capacity of the current block. The issue occurred because the boundary check was using a strict less-than comparison instead of less-than-or-equal, causing the allocator to skip the final available byte and return a pointer to the next block instead.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The workspace configuration now includes the new `crates/tinymemory-module` crate in both the members and default-members lists, ensuring it is built and tested as part of the workspace.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add three test files that were previously untracked, providing test coverage for the config, embedding, and service modules in the tinymemory-module crate.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The workspace exclude list was broad, excluding the entire vendor directory. This change narrows the exclusion to only the three specific engine submodules that are their own workspaces, allowing other vendor content to be included in the workspace.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The workspace exclude pattern is widened from three specific vendor subdirectories to the entire vendor directory, simplifying maintenance as new submodules are added. Three tinybus crate dependencies are added as path dependencies into the vendor checkout, with a comment explaining why path dependencies are used instead of version-only entries to avoid workspace resolution failures from nested workspace roots.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Adds the initial Cargo.toml manifest for the tinymemory-module crate, establishing its package metadata and dependencies to enable building and publishing the module.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Explicitly set the Cargo workspace resolver to version 2 to opt into the newer dependency resolution algorithm, which improves feature unification and avoids potential build issues with the current default resolver.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
…ories

The workspace exclude list now explicitly names each vendor subdirectory instead of using a single wildcard entry, ensuring that all engine submodules are properly excluded from workspace resolution.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
…dependencies

The workspace `exclude` list is simplified to a single `vendor` entry, replacing the previous per-submodule exclusions. The `tinybus`, `tinybus-macros`, and `tinybus-module` path dependencies are removed as they are no longer needed, reducing clutter in the manifest.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The workspace now includes the tinybus crate family as path dependencies in vendor/tinybus, matching the pattern already used for tinycortex and tinyagents. The resolver is set to version 3 to support the new dependency structure, and the exclude list is updated to only exclude the specific vendor subdirectories that are independent workspaces.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The workspace now declares a shared edition field under `[workspace.package]`, setting it to "2024" so that all workspace members inherit this Rust edition without needing to specify it individually.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Removed the `tinymemory-module` crate and all `tinybus` dependencies from the workspace, as these are no longer part of the project. The `exclude` list was simplified to cover the entire `vendor` directory instead of individual subdirectories, and the unused `resolver` and `workspace.package` settings were cleaned up.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a detailed comment explaining that the tinymemory-module crate cannot be a workspace member because its dependency on vendor/tinybus/crates/tinybus inherits workspace fields from the nested workspace, which fails when cargo resolves inheritance against the root workspace instead. The exclusion alone does not prevent this resolution, so the module must remain its own workspace root.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
This change introduces the Cargo.toml manifest for the tinymemory-module crate, establishing its metadata and dependencies to enable the crate's compilation and integration into the workspace.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add the initial Cargo.toml file to define the tinymemory-module crate, establishing its metadata and dependencies as the foundation for the new module.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
senamakel and others added 17 commits August 12, 2026 00:17
Introduce a new embedding module in the tinymemory crate that provides functionality for generating and managing vector embeddings, enabling semantic similarity searches and memory retrieval based on learned representations.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The memory module service now returns an empty result instead of panicking when given an empty input, ensuring graceful handling of edge cases in the processing pipeline.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Updated the test in `embedding_test.rs` to properly assert that the service returns an error when the input embedding dimension does not match the expected dimension, ensuring the validation logic is correctly tested.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Fix a memory allocation overflow that occurred when the module attempted to allocate a block larger than the available heap space. The issue was caused by an incorrect bounds check that allowed the allocation to proceed past the end of the memory region, leading to a panic. The fix ensures the allocation request is properly validated against the remaining capacity before proceeding.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Fix the memory alignment of atomic operations in the tinymemory module to ensure proper behavior on architectures that require aligned access. The previous implementation could cause undefined behavior or crashes when atomic loads and stores were performed on unaligned addresses.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The `into_bus_error` function was changed to take a reference, but the callers still passed `&error` to `wire_name` and `wire_message`, creating a reference that was immediately dereferenced. This caused a clippy error under `-D warnings`. The fix removes the unnecessary borrows so the code compiles cleanly again.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Removed unnecessary borrows of `error` when calling `wire::wire_name` and `wire::wire_message`, as the compiler automatically dereferences the reference, making the explicit borrow redundant and triggering a clippy warning.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Removed the unused `EMBEDDING_HOST_INTERFACE` import from the embedding test, which eliminates a compiler warning and cleans up the test code. The binary and build artifacts are updated as a result of the rebuild.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
A new GitHub Actions workflow is introduced to run tests and linting on every push and pull request, ensuring code quality and preventing regressions in the main branch.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Rebuilt the tinymemory-module crate and its dependencies, resulting in updated compiled binaries and debug information. The changes reflect a routine rebuild with no intentional modifications to source code or behaviour.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduce the initial specification document for the tinybus module, defining its interface, protocol, and expected behavior to guide implementation and future development.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
crates/tinymemory-module is its own workspace root, so it has its own
target/ that the root-anchored /target/ rule never matched. The
auto-commit hook picked up 3064 build files, including a 33 MB cdylib.

Co-authored-by: Medulla <medulla@tinyhumans.ai>
Introduce a new `Module` variant to the `DriverClass` enum to represent loadable native modules that are reached over an in-process bus. This variant sits between `Embedded` and `External` because it is neither compiled into the host binary nor does it cross a process boundary, which changes how the host applies admission and egress policies.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The conversion function now returns an empty result when given an empty input, preventing a panic that occurred when trying to process an empty slice. This ensures the adapter behaves correctly for edge cases where no data is provided.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
crates/tinymemory-module is its own workspace root, so it builds into its
own target/ that the root-anchored /target/ ignore never matched. The
.gitignore rule is already widened; this removes the 3064 files it should
have covered.

Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a complete release pipeline that builds, packages, and publishes native module bundles for Linux, macOS, and Windows across multiple architectures. The workflow resolves the release target from either an existing tag or the just-published crate version, then compiles the module for each platform in a matrix, assembles platform-specific archives with checksums, uploads them as workflow artifacts, and finally creates a GitHub release with all assets and a TinyBus-generated checksum manifest.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
…acts

Add a workflow_dispatch input that allows re-running the release workflow for an already existing tag, skipping the crate publish step. Also expose the tag and next_version outputs from the version step so that downstream jobs can consume them when the workflow is triggered manually.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@senamakel, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 52 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0b8dd576-3a20-4fe1-ab66-a4d2f6889302

📥 Commits

Reviewing files that changed from the base of the PR and between a6a32bb and 35617b8.

📒 Files selected for processing (5)
  • .github/workflows/release.yml
  • Cargo.toml
  • adapters/tinycortex/Cargo.toml
  • api/Cargo.toml
  • core/Cargo.toml
📝 Walkthrough

Walkthrough

This change adds a standalone TinyMemory TinyBus module. It defines configuration, host-side embeddings, service forwarding, wire error mappings, loader tests, CI execution, and native artifact release workflows.

Changes

TinyMemory module integration

Layer / File(s) Summary
Contracts and standalone workspace
Cargo.toml, crates/tinymemory-module/Cargo.toml, src/registry/class.rs, adapters/tinycortex/src/convert.rs, api/src/*, .gitignore, vendor/tinybus
The module uses an independent workspace. DriverClass::Module, reverse conversions, and shared MemoryError wire mappings are added.
Configuration and host embedding
crates/tinymemory-module/src/config.rs, crates/tinymemory-module/src/embedding.rs, crates/tinymemory-module/src/*_test.rs
ModuleConfig validates workspace settings and strips embedded credentials. BusEmbeddingProvider delegates embedding requests through TinyBus and validates vector results.
Module bootstrap and TinyBus service
crates/tinymemory-module/src/lib.rs, crates/tinymemory-module/src/service/*
The module creates the memory engine, installs host embedding, registers the TinyBus service, forwards memory methods, and maps provider errors to named bus errors.
Loader integration tests and specification
crates/tinymemory-module/tests/module_e2e.rs, docs/specs/tinybus-module.md
End-to-end tests load the native library through a real broker and verify capabilities, memory operations, host embedding, pagination, errors, and wire compatibility.
Module CI execution
.github/workflows/ci.yml
CI checks formatting, Clippy, release cdylib compilation, unit tests, and isolated ignored loader tests with timeouts.
Native artifact release pipeline
.github/workflows/release.yml
The workflow resolves release targets, builds 11 platform artifacts, packages checksummed archives, creates a GitHub release, and verifies a published module.

Estimated code review effort: 5 (Critical) | ~90 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Host as Host process
  participant Broker as TinyBus broker
  participant Module as TinyMemory module
  participant Embedder as HostEmbedder
  Host->>Broker: Serve and name host embedder
  Host->>Module: Load native cdylib
  Module->>Broker: Register memory service
  Host->>Module: Invoke recall or store
  Module->>Embedder: Request host embeddings
  Embedder-->>Module: Return vectors
  Module-->>Host: Return memory result
Loading

Possibly related PRs

  • tinyhumansai/tinymemory#1: Establishes the driver registry, TinyCortex conversions, and API foundations extended by this module integration.

Poem

A rabbit hops through TinyBus bright,
Carries memory left and right.
Host-grown vectors, safely sent,
Native bundles neatly packed and bent.
Tests run clean in fields of code—
A carrot for every loaded node! 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: porting the memory engine into a TinyBus module.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

senamakel and others added 6 commits August 12, 2026 09:20
Add a GitHub Actions workflow to automate the release process, enabling consistent and repeatable publishing of new versions.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The release workflow was failing because it referenced an incorrect artifact path for the built binaries. Updated the path to match the actual output location of the build step, ensuring the release job can find and upload the artifacts successfully.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
Updated the GitHub Actions release workflow to use the latest versions of checkout, setup-node, and other actions, ensuring compatibility with current runner environments and avoiding deprecation warnings.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
The release workflow has been updated to use the latest versions of GitHub Actions, ensuring compatibility with current runner environments and avoiding deprecation warnings.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>
All four crates in the workspace now have `publish = false` set because the dependency graph includes `tinycortex-api`, which is consumed by path and not available on crates.io, making `cargo package` unable to resolve the graph. Since every consumer takes this repository by path or git, publishing is not intended.

Auto-committed-on: dragonfly
Co-authored-by: Medulla <medulla@tinyhumans.ai>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 11

🧹 Nitpick comments (4)
crates/tinymemory-module/src/config_test.rs (2)

136-154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert the fields the round trip is most likely to break.

The fixture sets ollama_base_url and cloud_embedding_model, but the assertions skip both. The test also skips memory, embedding_routes, and storage_provider, which are the nested fields that carry the real wire risk. A serde rename or a container-level attribute change on those fields would pass this test.

♻️ Proposed additional assertions
     assert_eq!(back.workspace_dir, config.workspace_dir);
+    assert_eq!(back.ollama_base_url, config.ollama_base_url);
+    assert_eq!(back.cloud_embedding_model, config.cloud_embedding_model);
     assert_eq!(back.cloud_embedding_dimensions, 8);
     assert_eq!(back.models_supporting_dimensions, vec!["m".to_string()]);
     assert_eq!(back.driver_id, "tinycortex");
🤖 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/tinymemory-module/src/config_test.rs` around lines 136 - 154, Expand
a_populated_config_round_trips assertions to verify ollama_base_url and
cloud_embedding_model, plus the configured memory, embedding_routes, and
storage_provider nested fields. Ensure the fixture populates those nested fields
with representative non-default values so serde field renames or container
attributes would be detected by the round-trip test.

72-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Scan nested JSON objects, not only top-level keys.

The loop inspects object.keys() at depth one. The next test documents exactly why that is not enough: agentmemory_secret sits one level down. A recursive walk turns this test into the guard the comment claims it is, and it would catch a future credential field in MemoryConfig or StorageProviderConfig without another manual discovery.

This is the test-side counterpart of the strip gap raised in crates/tinymemory-module/src/config.rs Lines 159-164.

♻️ Proposed recursive key check
-    for key in object.keys() {
-        let lowered = key.to_ascii_lowercase();
-        for forbidden in [
-            "api_key",
-            "apikey",
-            "token",
-            "secret",
-            "password",
-            "credential",
-        ] {
-            assert!(
-                !lowered.contains(forbidden),
-                "config field {key:?} looks like it carries a credential; \
-                 embeddings go over the bus precisely so it does not have to"
-            );
-        }
-    }
+    fn assert_no_credential_keys(value: &serde_json::Value, path: &str) {
+        const FORBIDDEN: [&str; 6] = [
+            "api_key",
+            "apikey",
+            "token",
+            "secret",
+            "password",
+            "credential",
+        ];
+        match value {
+            serde_json::Value::Object(map) => {
+                for (key, nested) in map {
+                    let lowered = key.to_ascii_lowercase();
+                    for forbidden in FORBIDDEN {
+                        assert!(
+                            !lowered.contains(forbidden),
+                            "config field {path}.{key} looks like it carries a credential; \
+                             embeddings go over the bus precisely so it does not have to"
+                        );
+                    }
+                    assert_no_credential_keys(nested, &format!("{path}.{key}"));
+                }
+            }
+            serde_json::Value::Array(items) => {
+                for item in items {
+                    assert_no_credential_keys(item, path);
+                }
+            }
+            _ => {}
+        }
+    }
+
+    assert_no_credential_keys(&value, "config");

Note: a recursive check will fail today, because memory.agentmemory_secret serializes as a key even when it is None unless the field carries skip_serializing_if. Confirm the serialized shape first, then either add skip_serializing_if upstream or restrict the recursive assertion to keys with a non-null value.

🤖 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/tinymemory-module/src/config_test.rs` around lines 72 - 88, Update the
config credential-scan test around the current top-level key loop to recursively
traverse nested JSON objects and inspect every key. Confirm the serialized
representation first; if optional fields such as memory.agentmemory_secret
appear with null values, either add the appropriate skip_serializing_if behavior
or limit the assertion to keys with non-null values, while preserving the
existing forbidden-key checks.
crates/tinymemory-module/src/config.rs (1)

159-164: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Reachability path
● Entry
  crates/tinymemory-module/src/lib.rs:101
  setup: `MemoryConfig` travels verbatim, and it contains a bearer token field for a
│
▼
● Sink
  crates/tinymemory-module/src/config.rs

Make the credential-key test recursive. strip_host_credentials removes the only credential-bearing field currently defined in MemoryConfig, and StorageProviderConfig contains only provider. Walk nested objects and arrays so future credential fields cannot bypass the structural guard.

🤖 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/tinymemory-module/src/config.rs` around lines 159 - 164, The
credential check in MemoryConfig::strip_host_credentials is not recursive.
Update it to traverse nested objects and arrays, removing or detecting
credential-bearing fields at every depth while preserving the existing boolean
result. Extend the related coverage in
crates/tinymemory-module/src/config_test.rs lines 72-88 to verify nested object
and array cases; no other site changes are required.
crates/tinymemory-module/src/embedding_test.rs (1)

199-203: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

This assertion does not test the property named in the test.

The test is named an_absent_host_fails_by_name_rather_than_hanging. The assertion at line 203 checks only that the error message is non-empty. Every error satisfies that, including one raised for an unrelated reason. The test would still pass if the failure came from a broken transport rather than from an unresolved bus name.

Assert the text the provider produces for an unreachable host, so the test proves the operator receives an actionable message.

♻️ Proposed fix to assert the actual failure mode
     let error = provider
         .embed(&["alpha"])
         .await
         .expect_err("no embedder is served");
-    assert!(!error.to_string().is_empty());
+    let rendered = error.to_string();
+    assert!(
+        rendered.contains("embedding host unreachable") || rendered.contains("host embed failed"),
+        "the error must name the absent host, got: {rendered}"
+    );
🤖 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/tinymemory-module/src/embedding_test.rs` around lines 199 - 203,
Update the assertion in the test using provider.embed so it verifies the exact
actionable error text produced when the host is unreachable or the bus name
cannot be resolved, rather than merely checking that the message is non-empty.
Keep the existing expect_err flow and ensure the assertion distinguishes this
intended failure mode from unrelated transport errors.
🤖 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 @.github/workflows/release.yml:
- Around line 14-20: Update the publish job condition to require both the
main-branch reference and an empty existing_tag input. Preserve normal releases
while preventing version bumps, tagging, and crate publication when
workflow_dispatch supplies an existing tag; ensure the corresponding release
flow uses the same gate if duplicated.
- Around line 291-311: Update the Windows package assembly step identified by
“Assemble Windows module package” to also copy docs/specs/tinybus-module.md into
the package, matching the Unix package contents. Adjust the modules.toml
generation using $moduleName, $hash, and Set-Content so it writes one line
without embedding an extra `n before Set-Content adds its line terminator.

In `@crates/tinymemory-module/src/embedding.rs`:
- Around line 275-286: Update the vector-width validation in the embedding
validation block to always require each vector’s length to equal
self.dimensions, including when dimensions is zero. Remove the conditional that
skips validation for zero dimensions, preserving the existing error for any
mismatched vector. Update the relevant FakeHostEmbedder test to return a
non-empty vector for a zero-width provider and assert validation fails.

In `@crates/tinymemory-module/src/lib.rs`:
- Around line 128-136: Update the error handling around
create_memory_with_local_ai so the returned setup_error contains only a
stage-level message and does not include the workspace path or underlying
factory error. Log the captured inner error locally before returning the
sanitized error, preserving the existing failure propagation through
setup_error.
- Around line 123-126: Protect the process-global embedding host initialization
in setup with a process-wide once-only guard, so multiple ModuleHost instances
cannot replace it. Update the setup flow around BusEmbeddingHost::new and
set_embedding_host to initialize only on the first call while preserving the
existing connection and config for that initialization.

In `@crates/tinymemory-module/tests/module_e2e.rs`:
- Around line 191-200: Update the capability assertions in the module end-to-end
test to require that the advertised set exactly matches Capability::MANDATORY,
rejecting every capability outside that set rather than checking only the
absence of Capability::Tree. Preserve the existing mandatory-capability
validation while adding the exact-set comparison.
- Around line 374-384: Update the ExportPage test around outcome so it uses an
input the service is guaranteed to reject instead of a zero limit, then require
the call to return an error and assert that the error’s wire_name starts with
"ai.tinyhumans.tinymemory.Error.". Preserve the contract-name validation while
removing the optional-success path.
- Around line 441-452: Update
the_declared_method_list_matches_the_served_interface to validate the actual
loaded manifest.methods rather than only routing constants. Use an isolated
loader test or reuse a shared method-name list from manifest construction, and
assert that every served method, including ImportRecords, is present in the
manifest admission surface.
- Around line 19-22: Update the documented loader-test command in the module_e2e
test instructions to build from the standalone tinymemory-module workspace and
load the resulting artifact from that workspace’s target directory, rather than
the root workspace and root target directory. Preserve the existing ignored
exact-test invocation.

In `@docs/specs/tinybus-module.md`:
- Around line 95-97: Update the List contract to prevent unbounded MemoryEntry
responses: add pagination or enforce a documented server-side result ceiling
with a named error, matching the existing ExportPage framing approach. Update
the List specification and add an end-to-end test that exceeds the selected
boundary and verifies the defined behavior.
- Line 31: Update the fenced timing-output code block in tinybus-module.md to
declare the text language by changing its opening fence to use text, while
preserving the block contents and closing fence.

---

Nitpick comments:
In `@crates/tinymemory-module/src/config_test.rs`:
- Around line 136-154: Expand a_populated_config_round_trips assertions to
verify ollama_base_url and cloud_embedding_model, plus the configured memory,
embedding_routes, and storage_provider nested fields. Ensure the fixture
populates those nested fields with representative non-default values so serde
field renames or container attributes would be detected by the round-trip test.
- Around line 72-88: Update the config credential-scan test around the current
top-level key loop to recursively traverse nested JSON objects and inspect every
key. Confirm the serialized representation first; if optional fields such as
memory.agentmemory_secret appear with null values, either add the appropriate
skip_serializing_if behavior or limit the assertion to keys with non-null
values, while preserving the existing forbidden-key checks.

In `@crates/tinymemory-module/src/config.rs`:
- Around line 159-164: The credential check in
MemoryConfig::strip_host_credentials is not recursive. Update it to traverse
nested objects and arrays, removing or detecting credential-bearing fields at
every depth while preserving the existing boolean result. Extend the related
coverage in crates/tinymemory-module/src/config_test.rs lines 72-88 to verify
nested object and array cases; no other site changes are required.

In `@crates/tinymemory-module/src/embedding_test.rs`:
- Around line 199-203: Update the assertion in the test using provider.embed so
it verifies the exact actionable error text produced when the host is
unreachable or the bus name cannot be resolved, rather than merely checking that
the message is non-empty. Keep the existing expect_err flow and ensure the
assertion distinguishes this intended failure mode from unrelated transport
errors.
🪄 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: 58d83ba6-3c79-4bd1-b18e-6ee4b2f56c9f

📥 Commits

Reviewing files that changed from the base of the PR and between fd682bd and a6a32bb.

⛔ Files ignored due to path filters (1)
  • crates/tinymemory-module/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (20)
  • .github/workflows/ci.yml
  • .github/workflows/release.yml
  • .gitignore
  • Cargo.toml
  • adapters/tinycortex/src/convert.rs
  • api/src/lib.rs
  • api/src/wire.rs
  • api/src/wire_tests.rs
  • crates/tinymemory-module/Cargo.toml
  • crates/tinymemory-module/src/config.rs
  • crates/tinymemory-module/src/config_test.rs
  • crates/tinymemory-module/src/embedding.rs
  • crates/tinymemory-module/src/embedding_test.rs
  • crates/tinymemory-module/src/lib.rs
  • crates/tinymemory-module/src/service/mod.rs
  • crates/tinymemory-module/src/service/test.rs
  • crates/tinymemory-module/tests/module_e2e.rs
  • docs/specs/tinybus-module.md
  • src/registry/class.rs
  • vendor/tinybus

Comment thread .github/workflows/release.yml
Comment thread .github/workflows/release.yml
Comment thread crates/tinymemory-module/src/embedding.rs
Comment thread crates/tinymemory-module/src/lib.rs
Comment thread crates/tinymemory-module/src/lib.rs
Comment thread crates/tinymemory-module/tests/module_e2e.rs
Comment thread crates/tinymemory-module/tests/module_e2e.rs
Comment thread crates/tinymemory-module/tests/module_e2e.rs
Comment thread docs/specs/tinybus-module.md
Comment thread docs/specs/tinybus-module.md
@senamakel
senamakel merged commit 2ac93e1 into main Aug 12, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant