Lift the optional capability families into the TinyCortex adapter (#18 §C3) - #22
Merged
YellowSnnowmann merged 6 commits intoAug 18, 2026
Conversation
`rust_out` and `tmp` are 3.6 MB Linux x86-64 ELF executables with debug info, committed into a repository developed on macOS. Nothing references either name in source, config, CI, or docs; both are the output of an ad-hoc build that was staged by accident, and every clone has carried 7.3 MB of them since. Ignored with anchored paths so a legitimate `tmp` directory inside a crate is unaffected.
The workspace declared three different answers: 1.96 at the root and for the module crate, 1.85 for `core` and both adapters, and nothing at all for `api`. The low ones were not true — the root facade requires 1.96 and depends on all of them, so nothing in this workspace builds on 1.85, and no CI job checked the claim. Aligning on 1.96 immediately surfaced three lints that the understated MSRV had been suppressing: clippy only suggests `is_multiple_of` when the declared minimum supports it. Those are fixed here rather than allowed, since they are a consequence of this change and not separable from it. `api/` gaining an explicit MSRV is worth noting for downstream: it is the dependency-light crate an embedding host binds directly, so it now states a floor rather than leaving consumers to infer one.
The layout section omitted `tinymemory-core` entirely — the largest crate in the repository, and the one a real host actually depends on. It also omitted `crates/tinymemory-module`. Adds both, and states plainly that `core` is not dependency-light the way `api` is, since that asymmetry is the thing a reader most needs to know before choosing which to depend on. Also documents `git submodule update --init --recursive`. Nothing builds without it, and because `core` names its engines by path through `vendor/`, an uninitialized checkout fails at manifest resolution with an error that does not mention submodules.
`audit_provider` checks that a driver's advertised capabilities match its reachable accessors — a structural check, proving the shape is honest, not the behaviour. Nothing checked that two drivers answer the same question the same way, which is the claim "swap the engine without the host learning anything new" actually rests on. `tinymemory-conformance` is that check. `assert_provider` takes any bound driver and drives the contract: the mandatory three families, upsert on `(namespace, key)`, namespace isolation, provenance preservation, recall limits and scoping, export pagination, import round-tripping, and unicode / empty / 64 KiB / control-character content. It depends on `tinymemory-api` and nothing else of substance. A suite that pulled in an engine could not prove interchangeability, having already chosen one; reaching `tinymemory-core` would drag in a bundled SQLite besides. Ships an in-memory reference driver, for two reasons. A suite that only ever ran against real engines cannot distinguish "the engine is wrong" from "the assertion is wrong", and a driver whose behaviour is obvious by inspection separates those. It also documents the contract by example — the upsert, the verbatim taint, the cursor that terminates on `None` rather than on an empty page. Writing it immediately found a distinction worth naming. The contract permits a driver that accepts writes and discards them; `NullMemoryProvider` is exactly that, and the storage assertions are vacuous for it. Rather than weakening each assertion to tolerate an empty read — which would let a driver that *intends* to retain and silently does not slip through — the suite probes retention once and reports which half it ran. The contract-shape assertions are never skipped. Provenance is the sharp assertion. A driver reading back `Internal` for content stored as `ExternalSync` has laundered external content into internal-trust content, and every downstream gate keyed on taint is then silently wrong. Refs tinyhumansai#18 (§E1)
Issue tinyhumansai#18 §E3. `AGENTS.md` mandates a `tests/` directory exercising only the public API; the repository had none at the root, and `core/tests/` holds fixtures with no test files. Four targets, matching the four §E3 names: `driver_selection.rs` pins admission: reserved ids resolve to fixed classes, an external driver with no entry is refused fail-closed, an untrusted external driver is refused even with one, a reserved id's class cannot be overridden by config, and a class typo is echoed back so the operator can find the line. `capability_negotiation.rs` covers both directions of the bind-time negotiation, including a deliberately lying provider that advertises a summary tree it has no accessor for — the failure `audit_provider` exists to catch, and which was previously asserted only in unit tests inside the contract crate. `taint_end_to_end.rs` drives provenance through store, get, list, recall, and the export/import round trip, at every driver this workspace ships. It also pins the fail-closed reading of an unknown persisted value, which is the one direction that cannot be undone. `null_provider.rs` asserts the compiled-out configuration is genuinely usable: every mandatory method answers rather than panicking, it reports Ready rather than a fault, and no optional family is either advertised or reachable. Two of the four are deliberately narrower than §E3 describes, and both say so in their module docs. `driver_selection.rs` cannot yet assert that a bound provider's `driver_id()` matches configuration, because nothing selects an engine from config — that is §A5. `taint_end_to_end.rs` cannot drive the sync path, because sync is welded to the engine until §B. Both are written against current behaviour, per the sequencing note in the issue, and each names where its missing leg joins. Refs tinyhumansai#18 (§E3)
Issue tinyhumansai#18 §C3. The adapter advertised Core, Recall and Portability and nothing else, which is the reason anything wanting a summary tree, entities, or a diff ledger reached past the contract to the engine directly — the families existed, but not through `MemoryProvider`. `crates/tinymemory-module` had grown all of them, because it needed them and nowhere else had them. They were never module-specific. Every one delegates to `tinymemory-core` on a blocking thread, and the two types they hold — `MemoryClient` and the host config — are core and contract types respectively. So the whole `ModuleMemoryProvider` moves down to `tinymemory-tinycortex` as `engine::TinycortexProvider`, and the module crate keeps only the thing that genuinely is its own: turning a `ModuleConfig` into the engine's runtime configuration. Its `provider.rs` goes from 2189 lines to 38. Nineteen family implementations move: documents, ingest, graph, goals, tool-memory, tree, entities, diff, sources, maintenance, people, chunks, retrieval, profile, and episodic, alongside the mandatory three. The diff family gets a `memory-git` feature rather than riding along unconditionally. It is what drags `git2` / `libgit2-sys` / `libz-sys` — a native build — into the graph, and this adapter had no such dependency before today; making it unconditional would hand every consumer a libgit2 build they never asked for. `cargo tree --no-default-features` confirms none is linked. The gate reaches `capabilities()`, not just the accessor. A build without `memory-git` neither advertises nor reaches `Diff`, so `audit_provider` still passes — which is the whole reason that audit exists. That rule is extracted as `engine::advertised_capabilities` so it can be tested directly: constructing a provider needs a `MemoryClient`, which needs the host's process-global seams installed, and a test that installs a process global is order-dependent. The new module is `engine`, not `provider`: the crate already has a `provider` function returning the mandatory-only driver, and both are worth keeping — a host with no workspace, config, or client still has the lighter one. Refs tinyhumansai#18 (§C3)
|
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 |
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
How this change flows0 changed behaviours across 5 relationships. 5 surrounding behaviours are shown (60 graph nodes walked). 36 further behaviours left out to keep the diagram readable. flowchart LR
n0["MemoryProvider"]:::impacted
n1["MemoryCore"]:::impacted
n2["assert_provider"]:::impacted
n3["assert_export_import_round_trip"]:::impacted
n4["TinycortexProvider"]:::impacted
n2 -->|uses| n0
n2 -->|calls| n3
n3 -->|uses| n0
n4 -->|implements| n0
n4 -->|implements| 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. |
tinysweeper
Bot
added
the
priority: p3
This was referenced Aug 17, 2026
This was referenced Aug 17, 2026
M3gA-Mind
reviewed
Aug 17, 2026
M3gA-Mind
left a comment
M3gA-Mind
left a comment
There was a problem hiding this comment.
Reviewed this PR's own commit (26629c6). Clean.
It does what §C3 asked: 2,180 lines move out of crates/tinymemory-module/src/provider.rs into adapters/tinycortex/src/engine/mod.rs (2,236 lines), leaving the module crate a thin bus transport. The line counts corroborate the claim rather than just asserting it.
The memory-git feature is the part I looked at hardest, and it is right:
- Off by default, with a documented reason (keeps
git2/libgit2-sys/libz-sys— a native build — out of the default graph, which this adapter genuinely did not have before the lift). - The gate reaches
advertised_capabilities(), not just the accessor. That is the detail that matters: a build advertisingDiffwhile unable to serve it is exactly whataudit_providerexists to catch, and gating only the accessor would have created that state. The comment says so explicitly. - Both branches are asserted (
the_full_engine_advertises_every_family_with_memory_git/diff_is_withheld_when_the_snapshot_store_is_compiled_out), and the second one checks that withholding one family does not quietly withhold the rest — a good assertion to have written.
I confirmed both branches are actually executed in CI: the memory-git-on test runs under cargo test --all-features, and the off test under the -p tinymemory-tinycortex --no-default-features step that #24 adds. Worth noting that without #24 the off-branch test would have compiled everywhere and run nowhere.
One observation, not a request: 54 lines of test for 2,236 lines lifted is thin on its face. It is mitigated by the lift being mechanical rather than a rewrite, and engine/test.rs explains clearly why provider-level construction cannot be tested here (needs the host's process-global seams; AGENTS.md rules out order-dependent tests). Reasonable — but it does mean the lifted code's behaviour rests on whatever exercised it in the module crate before the move.
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.
Summary
Issue #18 §C3 — step 4 of the sequencing.
adapters/tinycortexadvertised Core, Recall and Portability and nothing else. That is exactly the finding in §1.2: the families existed, but not throughMemoryProvider, so anything wanting a summary tree, entities or a diff ledger reached past the contract to the engine directly.crates/tinymemory-modulehad grown all of them, because it needed them and nowhere else had them.They were never module-specific. Every one delegates to
tinymemory-coreon a blocking thread, and the two things they hold are atinymemory_core::store::MemoryClientand a config built fromtinymemory_api::hosttypes. So the wholeModuleMemoryProvidermoves down to the adapter asengine::TinycortexProvider, and the module keeps only what genuinely is its own — turning aModuleConfiginto the engine's runtime configuration.crates/tinymemory-module/src/provider.rs: 2189 lines → 38.Nineteen family implementations move: documents, ingest, graph, goals, tool-memory, tree, entities, diff, sources, maintenance, people, chunks, retrieval, profile and episodic, alongside the mandatory three.
The one judgement call:
memory-gitThe diff family does not ride along unconditionally. It is what drags
git2/libgit2-sys/libz-sys— a native build — into the graph, and this adapter had no such dependency before today. Making it unconditional would hand every consumer a libgit2 build they never asked for, which is the opposite of where §D is going.So it is a feature, forwarding to
tinymemory-core/memory-git. Verified:The gate reaches
capabilities(), not just the accessor. A build withoutmemory-gitneither advertises nor reachesDiff, soaudit_providerstill passes — which is the whole reason that audit exists. A version that gated onlyas_diffwould advertise a family it cannot serve, and #20's conformance suite would catch it.That rule is extracted as
engine::advertised_capabilities()so it can be tested directly. Constructing a provider needs aMemoryClient, which needs the host's process-global seams (set_embedding_hostand friends) installed — and a test that installs a process global is order-dependent, whichAGENTS.mdrules out.Naming
The new module is
engine, notprovider: the crate already has aproviderfunction returning the mandatory-only driver. Both are worth keeping — a host with no workspace, config, orMemoryClientstill has the lighter one, and the crate docs now explain when each applies.Public API changes
Additive, plus one dependency note:
tinymemory_tinycortex::engine::{TinycortexProvider, EngineRuntimeConfig, advertised_capabilities}memory-gitfeature, off by defaultTinycortexMemory,provider(),convert,TINYCORTEX_DRIVER_IDcrates/tinymemory-module:ModuleMemoryProvideris gone, replaced by apub(crate)helper. It waspub(crate), so nothing outside that crate could name it.tinymemory-core, so it is heavier than the mandatory-only version it replaces. That weight is §D's subject, and §D is sequenced after this and §A3 for exactly that reason.Validation
Both workspaces — the module crate has its own lockfile and CI job:
cargo fmt --all -- --checkcargo clippy --all-targets --all-features -- -D warningscargo clippy -p tinymemory-tinycortex --all-targets --no-default-features -- -D warningscargo build --all-targets --all-featurescargo test --all-featurescargo clippy --manifest-path crates/tinymemory-module/Cargo.toml --all-targets -- -D warningscargo test --manifest-path crates/tinymemory-module/Cargo.toml --libcargo build --locked --manifest-path crates/tinymemory-module/Cargo.toml --releaseRUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-featuresBoth feature configurations of the adapter are linted and tested.
Why this and not §A1+§A2 (step 3)
Step 3 is blocked outside this repository, and it is structural rather than a preference:
tinymemory-apiispublish = false, sotinycortex-apicannot depend on it by versiontinycortexvendors onlytinyagents;tinymemoryvendorstinycortex— so a path dependency would need tinycortex to vendor tinymemory, i.e. a submodule cycle§A1 therefore needs a publishing decision (make
tinymemory-apipublishable, then depend by version — mirroring the rule the README already states for adapters) plus a PR intinyhumansai/tinycortex. Happy to do that work once the call is made.§C3 does not depend on §A1 or §A2 — the lifted code was already written against
tinymemory_apitypes — and §A3 depends on §C3, so landing it now keeps the sequence moving.Related
Part of #18. Unblocks §A3, which can now route
core/throughMemoryProviderwithout the mandatory-three ceiling being the reason it cannot.