Select an engine by feature (#18 §D1) - #44
Conversation
Adds the per-engine features §D1 asks for -- `tinycortex`, `supermemory`,
`mem0`, `cognee`, and the `memory-git` capability add-on -- so a host names
an engine instead of taking a second dependency on an adapter crate:
tinymemory = { version = "1", features = ["tinycortex"] }
and `tinymemory::tinycortex::provider(backend)` binds it.
§D1's snippet could not be written as given
-------------------------------------------
`tinycortex = ["dep:tinymemory-tinycortex", ...]` on this crate is a package
cycle, which cargo rejects:
tinymemory -> tinymemory-tinycortex -> tinymemory-core -> tinymemory
Three edges had to go, and each is worth knowing:
1. The adapters depended on this crate for `mandatory::MemoryTraitProvider`.
That module moved to `tinymemory-api`. It costs the contract crate
nothing -- it names only `async_trait`, `std`, and contract types -- and
the contract is where a composition over `traits::Memory` belongs anyway.
2. The adapters depended on this crate for the reserved driver ids. Those
moved to `tinymemory_api::drivers`, following the precedent that
`NULL_DRIVER_ID` already lived in the contract. Reserving a name there
does not teach the contract about the engine: admission still decides the
class, in this crate's registry, where the trust decision belongs.
3. `tinymemory-core` declared a dependency on this crate and used nothing
from it. That dead declaration is what actually closed the cycle.
Both moves are re-exported, so `tinymemory::mandatory::MemoryTraitProvider`
and `tinymemory::registry::TINYCORTEX_DRIVER_ID` still resolve. That is
load-bearing rather than tidy: OpenCompany imports the first of those.
What it costs, measured
-----------------------
default / none 47 crates
supermemory 133
supermemory,mem0,cognee 133 (one adapter serves all three)
tinycortex 224
memory-git 228
Nothing links an engine until one is named.
Two incidental fixes the move forced: `tokio` becomes a dev-dependency of
`tinymemory-api` (the moved tests are async -- dev-only, so §D4's
forbidden-dependency rule is untouched), and the moved module's `//!` doc
links need explicit paths where its `///` item docs resolve bare.
cargo fmt --all -- --check: clean
cargo clippy --workspace --all-targets: clean
RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-features: clean
cargo test --workspace: 1206 passed, 0 failed -- identical to main, so the
module move lost no tests
cargo check -p tinymemory --features {tinycortex,supermemory,mem0,cognee,memory-git}: 0 errors each
|
Important Review available on request
Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 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 |
How this change flows0 changed behaviours across 1 relationship. 2 surrounding behaviours are shown (60 graph nodes walked). 49 further behaviours left out to keep the diagram readable. flowchart LR
n0["provider"]:::impacted
n1["MemoryTraitProvider"]:::impacted
n0 -->|uses| n1
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge. |
`Supply chain` failed: cargo-deny rejected `webpki-roots` for `CDLA-Permissive-2.0`. Advisories, bans and sources all pass; this was licences alone. It is a real consequence of this PR rather than a flake. `deny.toml` sets `[graph] all-features = true`, and before this branch the facade had no features to enable, so nothing on it pulled the remote adapter's TLS stack. Giving the facade `supermemory`/`mem0`/`cognee` puts `reqwest`-with-rustls, and so Mozilla's CA bundle, into the evaluated graph. `main` passes for exactly that reason, not because the crate was absent from its tree. Allowed rather than avoided, because the alternative is dropping HTTPS from the hosted engines. The licence is a *data* licence, which is why it was not already on a list of code licences: `webpki-roots` is Mozilla's trust store rendered as a Rust array, not software. CDLA-Permissive-2.0 places no conditions on use or redistribution of that data and adds no obligations to binaries that embed it. The entry carries that reasoning inline so the next person does not have to re-derive it. cargo deny check: advisories ok, bans ok, licences ok, sources ok
tinyhumansai#42 landed while this branch was open, and the two interact in three ways: 1. It added `NAMESPACE_DRIVER_ID` as a `const` in the facade's registry, where this branch had moved the other four ids to `tinymemory_api::drivers`. Moved the fifth with them -- splitting the set across two crates would mean the next id lands in whichever place its author happened to be reading. The facade re-exports all five. 2. It added a `tinymemory::mandatory::MemoryTraitProvider` call site in `tinymemory-core`, and this branch removes core's dependency on the facade as dead. Together those do not compile. Production code now names `tinymemory_api::` directly, which is where both items live. 3. Its admission test needs `registry::DriverRegistry`, which genuinely stays in the facade -- the trust decision belongs there. So the facade is back as a **dev**-dependency of core. Cargo permits a cycle through dev-deps; it must never become a normal one again, which is the whole point of §D1. cargo check -p tinymemory-core --all-targets: clean cargo check -p tinymemory --features tinycortex: clean, no cycle
…tures # Conflicts: # core/Cargo.toml
Docs CI (-D warnings) rejects two intra-doc links the moves carried along: email_clean's //! header pointed at its old engine sibling (super::email -> crate::email_markdown here), and host.rs's //! header used a bare [MemoryClient] that resolves in ///-position but not in module docs. The tinyhumansai#44 commit recorded this exact asymmetry; it holds. RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-features: clean
Issue #18 §D1 — engine features on the facade.
A host now names an engine instead of taking a second dependency on an adapter crate:
and
tinymemory::tinycortex::provider(backend)binds it. Features added:tinycortex,supermemory,mem0,cognee, plus thememory-gitcapability add-on, which requires the engine that serves it so asking for a capability cannot produce a build where nothing implements it.§D1's snippet could not be written as given
The issue proposes
tinycortex = ["dep:tinymemory-tinycortex", "dep:tinycortex"]on this crate. That is a package cycle, which cargo rejects outright:Three edges had to go, and each is worth recording:
The adapters depended on this crate for
mandatory::MemoryTraitProvider. That module moved totinymemory-api. It costs the contract crate nothing — it names onlyasync_trait,std, and contract types — and a composition overtraits::Memoryarguably belongs in the contract anyway.The adapters depended on this crate for the reserved driver ids. Those moved to
tinymemory_api::drivers, following the precedent thatNULL_DRIVER_IDalready lived in the contract. Reserving a name there does not teach the contract about the engine: admission still decides the class, in this crate's registry, where the trust decision belongs.tinymemory-coredeclared a dependency on this crate and used nothing from it. That dead declaration is what actually closed the cycle.Both moves are re-exported, so
tinymemory::mandatory::MemoryTraitProviderandtinymemory::registry::TINYCORTEX_DRIVER_IDstill resolve. That is load-bearing rather than tidy — OpenCompany imports the first of those (src/store/memory/test.rs).What each feature costs, measured
supermemorysupermemory,mem0,cogneetinycortexmemory-gitNothing links an engine until one is named.
--no-default-featuresis unchanged at 40 by the §D5 budget script.Two incidental fixes the move forced
tokiobecomes a dev-dependency oftinymemory-api. The moved mandatory-composition tests are async. Dev-only, so §D4's forbidden-dependency rule for the normal graph is untouched.//!header needs explicit paths where its///item docs resolve bare — an asymmetry worth knowing before moving another module between these crates.Validation
cargo fmt --all -- --checkcargo clippy --workspace --all-targetsRUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-featurescargo test --workspacecargo check -p tinymemory --features <each of the five>The test count is identical to
main, which is the check that matters for a module move: nothing was silently left behind.Merge order
Independent of #42 (§A3) and #43 (§B2/§D2) — disjoint files. The
vendor/gitlinks are byte-identical tomain.One follow-up, not blocking: #42 adds a
tinymemory::mandatory::MemoryTraitProvidercall site intinymemory-core. It resolves through the re-export either way, but once both land it should point attinymemory_api::mandatorydirectly so core stays off the facade — otherwise the dead dependency this PR removed grows back.