feat: stand up the TinyMemory workspace — contract, driver registry, shared mandatory families, TinyCortex adapter - #1
Merged
Merged
Conversation
…apabilities_tests.rs,api/src/ch 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>
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>
…space The project has been renamed from the generic "rust-template" to "tinymemory" and restructured as a workspace with the root package and a new `api` sub-crate. The root package now depends on `tinymemory-api` via a path dependency, making the contract types available through a single dependency. All metadata, descriptions, and documentation links have been updated to reflect the new project identity, and the license has been changed from GPL-3.0-only to MIT. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
When looking up a class by name in the registry, the code now returns an appropriate error instead of panicking if the class is not found. This ensures the registry behaves predictably for missing entries rather than crashing the caller. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Updated the test helper function to return an empty string instead of panicking when the package name is empty, ensuring robustness against edge cases in test data. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…driver registry Converts the rust-template scaffold into a Cargo workspace and moves the engine-neutral memory layer into it. - api/: the contract, moved verbatim from tinycortex-api. Every type name, serde representation and enum wire string is byte-identical -- MemoryTaint's serde form and chunks::chunk_id's derivation are persisted on disk, and MemoryTaint::from_db_str fails closed. Only the crate identifier and the prose claiming the contract belongs to one engine changed. - src/registry/: driver admission, lifted from OpenHuman's memory/binding.rs. Reserved ids, the confirm-never-override rule for a built-in id's class, and the fail-closed external-driver trust gate. Pure, so the rules stay testable without booting a host. - src/lib.rs: re-exports the contract wholesale so a host takes one dependency and tinymemory::provider::MemoryProvider is tinymemory_api's same type. Policy stays host-side by design: a driver that could be swapped for one that skips enforcement is exactly why the guard lives in the host. Co-authored-by: Medulla <medulla@tinyhumans.ai>
The mandatory module now correctly processes empty input instead of panicking or returning an error. This change ensures that the function gracefully handles edge cases where no data is provided, improving robustness and user experience. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a fallback in the mandatory module to handle cases where no provider is configured, preventing a panic and returning a clear error instead. This improves robustness when the system is used without a mandatory provider setup. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Updated the test assertion to properly validate the mandatory field behavior, ensuring the test correctly reflects the expected validation logic. The previous assertion was incorrectly checking for an error case that did not match the actual implementation. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
… engines MemoryCore, MemoryRecall and MemoryPortability are supertraits of MemoryProvider, so every driver must implement them. For a backend that already implements the Memory storage trait almost all of that is mechanical -- and the parts that are not are the parts each backend gets wrong the same way. They live here once now, with a test that reproduces each trap: - store routes through store_with_taint, never store, which hard-codes Internal and would launder externally-sourced content into internal-trust content; - list(None, ..) spans every namespace, because a backend that normalises a None namespace to the global one returns one namespace and calls it everything; - a scoped recall is refused rather than silently answered in full or post-filtered, both of which leak; - import persists the taint each record carries instead of re-stamping it. MemoryTraitProvider makes any Memory backend a bindable driver advertising exactly the mandatory three, so its optional surface is absent rather than present-and-failing -- audit_provider pins the two halves together. Co-authored-by: Medulla <medulla@tinyhumans.ai>
This change introduces the tinycortex library as a vendored dependency by adding it as a git submodule and updating the Cargo.toml in the adapters directory to reference it, enabling the project to use its functionality without relying on an external package registry. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
When converting a TinyCortex configuration, the code now gracefully handles the case where the cortex version field is absent by defaulting to a sensible fallback value instead of failing with an error. This ensures backward compatibility with older configuration files that do not explicitly specify the version. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Fix a bug where memory read and write operations could fail or produce incorrect results when the address was not aligned to the natural boundary of the data type. The change adds explicit alignment handling to ensure correct behavior for all memory access patterns. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Updated the project dependencies to their latest compatible versions, ensuring the build remains reproducible and benefiting from recent bug fixes and improvements in the dependency tree. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…as a submodule TinyCortex now lives under vendor/ as a nested submodule, and adapters/tinycortex is the single seam between its contract and TinyMemory's. The two describe the same values but are distinct crates, so something has to convert; one small audited crate is far better than a conversion scattered across every call site in a host. Every conversion destructures exhaustively rather than using or building onto a Default. Two contracts that may drift will drift, and a lenient conversion drops the new field silently -- a full destructuring pattern turns that into a compile error naming the field. The enums match every variant for the same reason: a new category or a third taint level cannot fall into a catch-all and be quietly downgraded. TinycortexMemory overrides store_with_taint rather than inheriting the trait default, which drops the taint -- that default is how externally-sourced content would get laundered into internal-trust content. Tests cover it in both directions and end-to-end across two engines via export/import. The adapter names tinycortex by version requirement, not path, so a host that already pins its own checkout unifies onto one copy through its patch table; the workspace root patches it to vendor/ for standalone builds. A path dep would give a host two engines with two incompatible Memory traits. Also finishes the template conversion: README, AGENTS.md, issue templates. Co-authored-by: Medulla <medulla@tinyhumans.ai>
|
Warning Review limit reached
Next review available in: 4 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 (15)
📝 WalkthroughWalkthroughTinyMemory replaces the Rust template with an engine-neutral Cargo workspace. It adds public API contracts, provider capabilities, mandatory storage and portability composition, driver admission, a TinyCortex adapter, tests, documentation, CI updates, and local submodule integration. ChangesTinyMemory workspace
Engine-neutral API
Runtime composition
TinyCortex integration
Estimated code review effort: 5 (Critical) | ~120 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
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: 15
🧹 Nitpick comments (11)
README.md (1)
74-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the documented validation commands match CI.
This block omits
cargo build --all-targets --all-features, and the test command does not enable all features. Use the same validation commands as.github/workflows/ci.ymlLines 37-50.Proposed update
cargo fmt --all -- --check -cargo test --workspace -cargo clippy --workspace --all-targets -- -D warnings +cargo clippy --all-targets --all-features -- -D warnings +cargo build --all-targets --all-features +cargo test --all-featuresBased on learnings, run all four contract commands from the repository root:
cargo fmt --all -- --check,cargo clippy --all-targets --all-features -- -D warnings,cargo build --all-targets --all-features, andcargo test --all-features.🤖 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 `@README.md` around lines 74 - 79, Update the README validation command block to match CI’s four repository-root commands: keep cargo fmt, add --all-features to cargo clippy, add cargo build --all-targets --all-features, and run cargo test with --all-features.Source: Learnings
adapters/tinycortex/src/memory_test.rs (1)
9-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an end-to-end recall test through the adapter.
This file covers store, get, list, forget, namespaces, and export/import against the real engine, but it never calls
recall.MemoryRecallis not imported. The adapter'srecallat adapters/tinycortex/src/memory.rs Lines 105-118 is the least mechanical method in the seam: it chains two local bindings and takes a borrow through(&engine_owned).into()so the owned conversion outlives the engine's borrowingRecallOpts.
adapters/tinycortex/src/convert_test.rsLines 143-160 testrecall_opts_to_tinycortexin isolation. Nothing proves the filters reach the engine and narrow a real query. Add a test that stores entries in two namespaces, then recalls withOwnedRecallOpts { namespace: Some(..), .. }and asserts the result set is narrowed.🤖 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 `@adapters/tinycortex/src/memory_test.rs` around lines 9 - 18, Add an end-to-end recall test in the existing memory adapter tests, importing MemoryRecall and using the real engine from engine(). Store entries across two namespaces, invoke recall with OwnedRecallOpts containing one namespace, and assert only entries from that namespace are returned, exercising the adapter’s recall path and filter propagation.src/mandatory/mod.rs (1)
365-377: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog the partial import counts before the batch aborts.
A backend write failure returns
Errat Line 375. The records already written stay in the target store, andoutcomeis dropped, so an operator cannot tell how far the restore got. The trait signature fixes the return type, so the counts can only be preserved in a log line.♻️ Proposed change to keep the partial counts diagnosable
- memory + if let Err(error) = memory .store_with_taint( &entry.namespace, &entry.key, &entry.content, entry.category, entry.session_id.as_deref(), record.taint, ) .await - .map_err(engine_error)?; + { + log::error!( + "[tinymemory:mandatory] import aborted after imported={} failed={}", + outcome.imported, + outcome.failed + ); + return Err(engine_error(error)); + } outcome.imported = outcome.imported.saturating_add(1);🤖 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 `@src/mandatory/mod.rs` around lines 365 - 377, Before propagating the error from the store_with_taint call in the import loop, log the current partial import counts from outcome so successfully written records remain diagnosable when a batch aborts. Preserve the existing engine_error mapping and return behavior after emitting the log.src/mandatory/test.rs (1)
73-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a
Memorydouble that fails a write, and cover theimport_recordsabort path.
VecMemory::store_with_taintalways returnsOk, so no test reaches the abort inimport_recordsat src/mandatory/mod.rs Line 375. That path is a documented contract: a backend write failure must abort the batch instead of reporting a partial restore as a success. The coding guidelines require tests to cover failure paths.Add a
fail_writesflag toVecMemory, return ananyhowerror fromstore_with_taintwhen it is set, and assert thatimport_recordsreturnsMemoryError::Other.As per coding guidelines: "cover failure paths and test every new error variant".
🤖 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 `@src/mandatory/test.rs` around lines 73 - 98, Extend VecMemory with a fail_writes flag and make store_with_taint return an anyhow error when the flag is enabled. Add an import_records test that configures this failing double, triggers a backend write failure, and asserts the result is MemoryError::Other, covering the batch-abort path without changing successful imports.Source: Coding guidelines
api/src/tool_memory_tests.rs (1)
73-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the UUID encoding test deterministic.
Lines 73-79 assert that two random v4 values differ. A UUID collision can fail CI without a product regression. Extract the UUID-to-ID encoding into a private helper and test that helper with fixed UUID bytes. Keep
generate_idcovered by shape validation.As per coding guidelines, “Tests must be deterministic and independent of network, wall-clock time, and execution order.”
🤖 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 `@api/src/tool_memory_tests.rs` around lines 73 - 79, Update rule_generate_id_produces_unique_values so it no longer asserts inequality between random generate_id results. Extract the UUID-to-ID encoding from ToolMemoryRule::generate_id into a private helper, test that helper using fixed UUID bytes and expected deterministic output, and retain generate_id coverage through the existing prefix and character-shape assertions.Source: Coding guidelines
api/src/provider/mod.rs (1)
55-73: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider narrowing the submodule visibility to a single public path.
Every type here has two public paths:
provider::mandatory::MemoryCoreandprovider::MemoryCore. A third-party driver can then depend on either, which doubles the surface you must keep stable. If the flat re-exports are the intended path, change the submodules topub(crate) modand keep thepub uselines.♻️ Proposed change
-pub mod audit; -pub mod content; -pub mod driver; -pub mod knowledge; -pub mod mandatory; -pub mod records; -pub mod types; +pub(crate) mod audit; +pub(crate) mod content; +pub(crate) mod driver; +pub(crate) mod knowledge; +pub(crate) mod mandatory; +pub(crate) mod records; +pub(crate) mod types;If you intend both paths, no change is needed.
As per coding guidelines: "Keep the public surface minimal: default to private and export deliberately from
src/lib.rs".🤖 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 `@api/src/provider/mod.rs` around lines 55 - 73, Keep the flat re-export paths in provider, but narrow the module declarations for audit, content, driver, knowledge, mandatory, records, and types from public to crate-visible (`pub(crate) mod`). Preserve the existing `pub use` exports so consumers continue using paths such as `provider::MemoryCore` without exposing parallel submodule paths.Source: Coding guidelines
api/src/chunks.rs (1)
306-310: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueThe comment does not describe what the code guards.
text.chars().count()returnsusize. Theas u32cast truncates by wrapping beforesaturating_addruns, sosaturating_addguards only the+3, not the input length. Use the same accumulate-in-u64-then-clamp form thatconservative_token_estimatealready uses at Line 334.♻️ Proposed change
/// Approximate token count (GPT-family heuristic: 1 token ≈ 4 chars). pub fn approx_token_count(text: &str) -> u32 { - // saturating_add guards against absurdly long inputs - let chars = text.chars().count() as u32; - chars.saturating_add(3) / 4 + // Count in u64 and clamp, so an absurdly long input saturates instead of + // wrapping through an `as u32` cast. + let chars = text.chars().count() as u64; + let tokens = chars.div_ceil(4); + tokens.min(u64::from(u32::MAX)) as u32 }🤖 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 `@api/src/chunks.rs` around lines 306 - 310, Update approx_token_count to avoid casting the character count directly to u32 before addition; accumulate in u64 and clamp the final result to u32 using the same pattern as conservative_token_estimate. Revise the nearby comment to accurately describe the overflow protection.api/src/goals_tests.rs (1)
15-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the empty-id and empty-text rejection branch.
GoalsDoc::parserejects an item when the id or the text is empty (api/src/goals.rslines 75-77). No test exercises that branch. The coding guidelines require tests to cover failure paths.💚 Proposed test
+#[test] +fn parse_rejects_an_empty_id_or_an_empty_text() { + let doc = GoalsDoc::parse("- [] has no id\n- [g2]\n- [g3] kept\n"); + assert_eq!(doc.items.len(), 1); + assert_eq!(doc.items[0].id, "g3"); +}As per coding guidelines: "cover failure paths and test every new error variant".
🤖 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 `@api/src/goals_tests.rs` around lines 15 - 22, Extend the goals parsing tests around parse_ignores_non_item_lines with a case containing item syntax whose id or text is empty, and assert that GoalsDoc::parse rejects or omits it according to its existing behavior. Ensure the test exercises the empty-id/empty-text validation branch in GoalsDoc::parse without changing production code.Source: Coding guidelines
api/src/lib.rs (1)
1-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a short runnable example to the crate docs.
The crate documentation gives an overview and a module map, but no runnable example. A doctest here also proves that the primary entry points compile against the published surface. Add a small example that builds a driver and reads its capabilities and health.
//! ``` //! use tinymemory_api::{null::NullMemoryProvider, provider::MemoryProvider}; //! //! let driver = NullMemoryProvider::new(); //! assert_eq!(driver.driver_id(), "null"); //! assert_eq!(driver.capabilities(), tinymemory_api::capabilities::Capabilities::mandatory()); //! ```As per coding guidelines, "
src/lib.rsmust provide a crate-level overview, primary entry points, and a short runnable example."🤖 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 `@api/src/lib.rs` around lines 1 - 60, Add a runnable doctest to the crate-level documentation in src/lib.rs, near the overview or module map, importing NullMemoryProvider and MemoryProvider. Construct the null driver, assert its driver_id is "null", and verify its capabilities equal Capabilities::mandatory(); also read the driver’s health as requested by the comment, using only the published API surface.Source: Coding guidelines
api/src/provider/types.rs (1)
98-102: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid the per-candidate allocation in
allows_source_id.The closure builds a new
Stringfor every allowed id on every call. Recall paths evaluate this predicate per row, so the allocation count scales with rows × allowed ids.strip_prefixgives the same equality-or-prefix semantics with no allocation.♻️ Proposed refactor
pub fn allows_source_id(&self, source_id: &str) -> bool { - self.allow.iter().any(|allowed| { - source_id == allowed || source_id.starts_with(&format!("mem_src:{allowed}:")) - }) + let attributed = source_id.strip_prefix("mem_src:"); + self.allow.iter().any(|allowed| { + source_id == allowed + || attributed.is_some_and(|rest| { + rest.strip_prefix(allowed.as_str()) + .is_some_and(|tail| tail.starts_with(':')) + }) + }) + }🤖 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 `@api/src/provider/types.rs` around lines 98 - 102, Update Provider::allows_source_id to avoid constructing a formatted String for each allowed id; preserve the exact and mem_src:{allowed}: prefix matching semantics by using strip_prefix or equivalent allocation-free checks within the iterator.api/src/traits.rs (1)
89-146: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
# Errorssections to the remaining fallible methods.
storedocuments its failure mode. The other fallible methods do not:store_with_taint,recall,recall_relevant_by_vector,get,list,forget,namespace_summaries, andcount. Each returnsanyhow::Result. Add a short# Errorssection to each one. The module note that a returnedErris opaque belongs in those sections too, so implementors read it at the method they are writing.As per coding guidelines, "Document a
# Errorssection on every public fallible function and a# Panicssection on anything that can panic."🤖 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 `@api/src/traits.rs` around lines 89 - 146, Add concise Rustdoc # Errors sections to the public fallible methods store_with_taint, recall, recall_relevant_by_vector, get, list, forget, namespace_summaries, and count. Describe that Err indicates an opaque backend failure, while preserving each method’s existing success and not-found semantics; do not alter behavior or signatures.Source: Coding guidelines
🤖 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 `@adapters/tinycortex/src/convert.rs`:
- Around line 21-28: Update the rustdoc link in the module documentation for
taint conversion to reference the in-scope `tm::MemoryTaint` symbol instead of
the unresolved bare `MemoryTaint` link, leaving the surrounding security
guidance unchanged.
In `@adapters/tinycortex/src/memory_test.rs`:
- Around line 183-196: Bound the paging loop in the test around export_page with
a page counter, following the existing pattern in mandatory test paging logic.
Increment the counter per iteration and assert or fail once a reasonable maximum
is exceeded, while preserving normal cursor advancement and the final
records.len() assertion.
In `@api/Cargo.toml`:
- Line 4: Update the edition setting in api/Cargo.toml to edition = "2021" so it
matches the workspace and sibling crates; do not use Rust 2024 unless explicitly
approved.
In `@api/src/capabilities.rs`:
- Around line 395-397: Move module-local tests into dedicated test.rs files and
wire each owner with #[cfg(test)] mod test;. In api/src/capabilities.rs lines
395-397 and api/src/error.rs lines 102-104 and api/src/version.rs lines 89-91,
replace path-based wiring; move capabilities_tests.rs to
api/src/capabilities/test.rs, chunks_tests.rs to api/src/chunks/test.rs,
error_tests.rs to api/src/error/test.rs, tool_memory_tests.rs to
api/src/tool_memory/test.rs, tree_tests.rs to api/src/tree/test.rs,
types_tests.rs to api/src/types/test.rs, version_tests.rs to
api/src/version/test.rs, and provider/types_tests.rs to
api/src/provider/types/test.rs, updating each owning module accordingly.
In `@api/src/chunks_tests.rs`:
- Around line 7-12: Update chunk_id_is_deterministic to assert the fixed
historical digest for the specified input, rather than only comparing two newly
computed values. Retain the length assertion if useful, and use the pre-move
contract’s exact 32-character golden value to detect changes to field ordering,
separators, hashing, or truncation.
In `@api/src/chunks.rs`:
- Around line 40-48: Add a Rustdoc `# Errors` section to both public `parse`
functions, `SourceKind::parse` and `DataSource::parse`, describing that they
return an error when the input string does not match a supported
source/data-source kind. Preserve their existing parsing behavior.
In `@api/src/traits.rs`:
- Around line 58-79: Update the default implementation of the MemoryStore
trait’s store_with_taint method to fail closed for MemoryTaint::ExternalSync and
other external taints instead of delegating to store and losing provenance;
return an error unless the backend overrides this method, while preserving the
existing store delegation only for MemoryTaint::Internal if appropriate.
In `@api/src/tree.rs`:
- Around line 190-201: Update node_id_to_path to validate node_id before
constructing any PathBuf, accepting only "root" or slash-separated numeric time
components. Reject traversal segments, absolute paths, and any other malformed
identifiers using the function’s existing error-handling contract, and perform
level_from_node_id only after validation.
- Around line 145-148: Update estimate_tokens so converting text.len() to u32
saturates at u32::MAX instead of wrapping, then retain the existing div_ceil(4)
calculation.
In `@api/src/types.rs`:
- Around line 135-165: Update MemoryCategory::from_str so the exact value
"custom:" deserializes to Custom(String::new()) instead of falling through to
the generic custom-value branch. Preserve the existing prefixed parsing for
non-empty custom names and the built-in category handling.
In `@Cargo.toml`:
- Line 11: Remove the manual Cargo.toml version change and restore the package
version to its pre-PR value, leaving version updates under release-workflow
control. Do not modify the version field elsewhere unless the release workflow
itself needs the initial transition.
In `@src/mandatory/mod.rs`:
- Around line 22-27: Remove the redundant explicit intra-doc link targets while
preserving the shorthand links: in src/mandatory/mod.rs lines 22-27, update the
GLOBAL_NAMESPACE link because it is imported; in adapters/tinycortex/src/lib.rs
lines 16-18, update the MemoryTraitProvider link because it is imported. Keep
the other explicit links in src/mandatory/mod.rs unchanged.
In `@src/registry/class.rs`:
- Around line 58-64: Replace the String error returned by DriverClass::parse
with a documented DriverClassParseError type, including an Unknown { raw: String
} variant for unrecognized input. Update parse and the DriverClass FromStr
implementation to return this typed error and set Self::Err =
DriverClassParseError, preserving successful parsing through Self::ALL.
In `@src/registry/mod.rs`:
- Around line 173-175: Update DriverRegistry::with_reserved in
src/registry/mod.rs:173-175 to prevent overwriting an existing reservation,
either preserving the original class or returning the established typed
registration-conflict error. First add failing reproducer coverage in
src/registry/test.rs:188-200 for re-reserving NULL_DRIVER_ID and
TINYCORTEX_DRIVER_ID with different DriverClass values, then implement the
smallest fix that preserves the fixed-class invariant.
- Around line 229-230: Update the class parsing branch in
src/registry/mod.rs:229-230 to map DriverClass::parse errors to a generic
refusal reason instead of passing the error through refuse(&e), ensuring invalid
raw input is never published. Add a regression test in
src/registry/test.rs:215-232 using a secret-shaped class value such as
https://user:token@host and assert that the resulting FallbackReason.reason does
not contain that input.
---
Nitpick comments:
In `@adapters/tinycortex/src/memory_test.rs`:
- Around line 9-18: Add an end-to-end recall test in the existing memory adapter
tests, importing MemoryRecall and using the real engine from engine(). Store
entries across two namespaces, invoke recall with OwnedRecallOpts containing one
namespace, and assert only entries from that namespace are returned, exercising
the adapter’s recall path and filter propagation.
In `@api/src/chunks.rs`:
- Around line 306-310: Update approx_token_count to avoid casting the character
count directly to u32 before addition; accumulate in u64 and clamp the final
result to u32 using the same pattern as conservative_token_estimate. Revise the
nearby comment to accurately describe the overflow protection.
In `@api/src/goals_tests.rs`:
- Around line 15-22: Extend the goals parsing tests around
parse_ignores_non_item_lines with a case containing item syntax whose id or text
is empty, and assert that GoalsDoc::parse rejects or omits it according to its
existing behavior. Ensure the test exercises the empty-id/empty-text validation
branch in GoalsDoc::parse without changing production code.
In `@api/src/lib.rs`:
- Around line 1-60: Add a runnable doctest to the crate-level documentation in
src/lib.rs, near the overview or module map, importing NullMemoryProvider and
MemoryProvider. Construct the null driver, assert its driver_id is "null", and
verify its capabilities equal Capabilities::mandatory(); also read the driver’s
health as requested by the comment, using only the published API surface.
In `@api/src/provider/mod.rs`:
- Around line 55-73: Keep the flat re-export paths in provider, but narrow the
module declarations for audit, content, driver, knowledge, mandatory, records,
and types from public to crate-visible (`pub(crate) mod`). Preserve the existing
`pub use` exports so consumers continue using paths such as
`provider::MemoryCore` without exposing parallel submodule paths.
In `@api/src/provider/types.rs`:
- Around line 98-102: Update Provider::allows_source_id to avoid constructing a
formatted String for each allowed id; preserve the exact and mem_src:{allowed}:
prefix matching semantics by using strip_prefix or equivalent allocation-free
checks within the iterator.
In `@api/src/tool_memory_tests.rs`:
- Around line 73-79: Update rule_generate_id_produces_unique_values so it no
longer asserts inequality between random generate_id results. Extract the
UUID-to-ID encoding from ToolMemoryRule::generate_id into a private helper, test
that helper using fixed UUID bytes and expected deterministic output, and retain
generate_id coverage through the existing prefix and character-shape assertions.
In `@api/src/traits.rs`:
- Around line 89-146: Add concise Rustdoc # Errors sections to the public
fallible methods store_with_taint, recall, recall_relevant_by_vector, get, list,
forget, namespace_summaries, and count. Describe that Err indicates an opaque
backend failure, while preserving each method’s existing success and not-found
semantics; do not alter behavior or signatures.
In `@README.md`:
- Around line 74-79: Update the README validation command block to match CI’s
four repository-root commands: keep cargo fmt, add --all-features to cargo
clippy, add cargo build --all-targets --all-features, and run cargo test with
--all-features.
In `@src/mandatory/mod.rs`:
- Around line 365-377: Before propagating the error from the store_with_taint
call in the import loop, log the current partial import counts from outcome so
successfully written records remain diagnosable when a batch aborts. Preserve
the existing engine_error mapping and return behavior after emitting the log.
In `@src/mandatory/test.rs`:
- Around line 73-98: Extend VecMemory with a fail_writes flag and make
store_with_taint return an anyhow error when the flag is enabled. Add an
import_records test that configures this failing double, triggers a backend
write failure, and asserts the result is MemoryError::Other, covering the
batch-abort path without changing successful imports.
🪄 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: 7dacc9ed-05df-48d1-b596-91136bd34b19
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (65)
.github/ISSUE_TEMPLATE/config.yml.github/workflows/ci.yml.gitmodulesAGENTS.mdCargo.tomlREADME.mdROADMAP.mdadapters/tinycortex/Cargo.tomladapters/tinycortex/src/convert.rsadapters/tinycortex/src/convert_test.rsadapters/tinycortex/src/lib.rsadapters/tinycortex/src/memory.rsadapters/tinycortex/src/memory_test.rsapi/Cargo.tomlapi/src/capabilities.rsapi/src/capabilities_tests.rsapi/src/chunks.rsapi/src/chunks_tests.rsapi/src/error.rsapi/src/error_tests.rsapi/src/goals.rsapi/src/goals_tests.rsapi/src/health.rsapi/src/health_tests.rsapi/src/lib.rsapi/src/null.rsapi/src/null_tests.rsapi/src/provider/audit.rsapi/src/provider/audit_tests.rsapi/src/provider/content.rsapi/src/provider/driver.rsapi/src/provider/knowledge.rsapi/src/provider/mandatory.rsapi/src/provider/mod.rsapi/src/provider/records.rsapi/src/provider/types.rsapi/src/provider/types_tests.rsapi/src/recall.rsapi/src/recall_tests.rsapi/src/tool_memory.rsapi/src/tool_memory_tests.rsapi/src/traits.rsapi/src/tree.rsapi/src/tree_tests.rsapi/src/types.rsapi/src/types_tests.rsapi/src/version.rsapi/src/version_tests.rsclippy.tomldocs/plans/example-retry-policy.mddocs/specs/example-retry-policy.mdexamples/basic.rssrc/error/mod.rssrc/error/test.rssrc/greeting/mod.rssrc/greeting/test.rssrc/lib.rssrc/mandatory/mod.rssrc/mandatory/provider.rssrc/mandatory/test.rssrc/registry/class.rssrc/registry/mod.rssrc/registry/test.rstests/public_api.rsvendor/tinycortex
💤 Files with no reviewable changes (9)
- examples/basic.rs
- src/error/test.rs
- ROADMAP.md
- src/greeting/mod.rs
- tests/public_api.rs
- docs/specs/example-retry-policy.md
- docs/plans/example-retry-policy.md
- src/greeting/test.rs
- src/error/mod.rs
Auto-committed-on: dragonfly
The idna_adapter dependency was downgraded from version 1.2.1 to 1.1.0, which replaces the ICU-based normalization and properties crates with simpler unicode-bidi and unicode-normalization alternatives. This change removes a large set of ICU-related transitive dependencies including icu_normalizer, icu_properties, and their supporting crates such as displaydoc, yoke, zerofrom, and zerovec, significantly reducing the dependency tree. Auto-committed-on: dragonfly
Auto-committed-on: dragonfly
Auto-committed-on: dragonfly
The binary file has been reduced from approximately 4.15 MB to 3.49 MB, saving about 0.66 MB of disk space. Auto-committed-on: dragonfly
Auto-committed-on: dragonfly
Auto-committed-on: dragonfly
Updated doc comments in the tinycortex adapter to use fully qualified paths for `MemoryTaint` and `MemoryTraitProvider`, making the documentation clearer and more consistent with Rust conventions for cross-crate references. Auto-committed-on: dragonfly
Introduce a `custom:` memory category that accepts an empty label, add proper error types for driver class parsing, and harden several parsing functions against invalid input. The `estimate_tokens` function now safely handles overflow, `node_id_to_path` rejects malformed node IDs, and the default `store_with_taint` implementation validates taint support. A test guard prevents infinite export loops, and the registry's `with_reserved` method avoids overwriting existing reservations. Auto-committed-on: dragonfly
Reformatted two method chains that exceeded the line length limit, splitting them across multiple lines to improve code readability without changing any behaviour. Auto-committed-on: dragonfly
Removed the `thiserror` dependency from `DriverClassParseError` by replacing the derive macro with manual implementations of `Display` and `Error`. This eliminates an external dependency while preserving the same error message and behavior, and updates the test assertion to compare against the error type directly. Auto-committed-on: dragonfly
Auto-committed-on: dragonfly
The `raw` field in the `DriverClassParseError::Unknown` variant now carries a documentation comment explaining its purpose, and the test assertion for the error message is updated to match the current output format. Auto-committed-on: dragonfly
The chunk_id_is_deterministic test now also asserts the exact expected hash value, making the test more specific and ensuring the hash algorithm produces the correct output rather than just checking consistency between calls. Auto-committed-on: dragonfly
Auto-committed-on: dragonfly
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
2 participants
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.
What
Turns the
rust-templatescaffold into the real TinyMemory workspace: the engine-neutral memory layer that OpenHuman (and any other host) performs memory operations through, with TinyCortex as the default engine behind it rather than as the thing hosts talk to directly.The pieces
api/— the contract, moved fromtinycortex-api. Every type name, serde representation and enum wire string is byte-identical:MemoryTaint's serde form andchunks::chunk_id's derivation are persisted on disk, andMemoryTaint::from_db_stris security-critical and fails closed. Only the crate identifier and the prose claiming the contract belongs to one engine changed. A contract named after one engine is not a contract a second engine can enter through.src/registry/— driver admission, lifted out of OpenHuman'smemory/binding.rs. These are the rules with the same correct answer for every host:classline may confirm it but never override it. Without that,driver = "null"plusclass = "embedded"builds the real engine and persists memory under the id documented as/dev/null;Refusals are operator-facing and logged, so
DriverEntrydeliberately carries onlyclassandtrust_state— never an endpoint or a credential reference. A refusal structurally cannot leak a secret.src/mandatory/— the piece with the most leverage.MemoryCore,MemoryRecallandMemoryPortabilityare supertraits ofMemoryProvider, so every driver implements them. For a backend that already implementsMemorymost of that is mechanical — and the parts that are not are the parts each backend gets wrong the same way. They live here once, each with a test that reproduces the trap:storeviastore_with_taint, neverstorestorehard-codesInternal, laundering externally-sourced content into internal-trust contentlist(None, ..)spans every namespaceNoneto the global namespace returns one namespace and calls it "everything"limitbe eaten by rows the caller may not seeMemoryTraitProvidermakes anyMemorybackend a bindable driver advertising exactly the mandatory three, so its optional surface is absent rather than present-and-failing.audit_providerpins the advertised set against the reachable accessors.adapters/tinycortex/— the seam. TinyCortex's contract and TinyMemory's describe the same values but are distinct crates, so something must convert; one small audited crate beats a conversion scattered across a host. Every conversion destructures exhaustively rather than using..: two contracts that may drift will drift, and a lenient conversion drops the new field silently, whereas a full destructuring pattern is a compile error naming it. The enums match every variant for the same reason.Notes for review
[patch.crates-io]; the workspace root patches it tovendor/for standalone builds. A path dep would hand a host two engines with two incompatibleMemorytraits.api/is held byte-identical rather than reformatted to a stricter lint set.Testing
cargo test --workspace— 183 tests + doctests green.cargo clippy --workspace --all-targets -- -D warningsclean.cargo fmt --all --checkclean.Contract purity is guarded in the forward form (
cargo tree -p tinymemory-api …), sincecargo tree -idiscards the-pscope and exits 0 even when the scoped crate is the culprit.Summary by CodeRabbit
New Features
Documentation