Restore the bundled example, and make a refusal an Error (#18 §E7) by YellowSnnowmann · Pull Request #27 · tinyhumansai/tinymemory

YellowSnnowmann · 2026-08-17T12:13:07Z

Stacked on #19 → #20 → #21 → #22 → #23 → #24 → #25 → #26. Once those merge this reduces to the single commit Restore the bundled example, and make a refusal an Error.

Summary

Issue #18 §E7.

AGENTS.md documents an examples/ directory (line 30) and tells the reader to run cargo run --example basic (line 74). Neither existed — there is no examples/ directory at all, so the documented command has been failing for as long as it has been documented.

(Small correction to the issue: it attributes the reference to README.md. It is AGENTS.md.)

The example

Binds the null driver — no engine, no workspace, no network, so it runs anywhere. It walks the order a host actually follows, because that order is the part worth demonstrating:

Admit an id — engine-neutral, answers "is this driver id real, and may it answer for memory"
Construct — the host's job, needs everything an engine needs
Audit the negotiated capabilities
Use the mandatory families

Swapping NullMemoryProvider for an adapter's provider changes nothing else in the file, which is the point.

$ cargo run --example basic
contract version: (2, 2)
admitted 'null' as Null
driver 'null' serves 3 families: core, recall, portability
read back: nothing — the null driver retains no writes

The last line is correct, not a failure: the null driver accepts writes and discards them, and the example says so rather than looking broken.

It found a real API gap

Which is the argument for having a compiled example at all.

FallbackReason implemented Display but not std::error::Error. So the obvious host code —

let admission = registry.admit(NULL_DRIVER_ID, None, ConfigLabels::default())?;

— did not compile in a function returning Box<dyn Error> or anyhow::Error. A refusal is exactly the thing a host propagates upward, so this is where the type is most likely to be used.

Adding the impl is purely additive and changes nothing about the type. It makes the existing message usable where refusals actually travel.

CI runs it, rather than only building it

cargo build --all-targets compiles examples, so a broken example still passes that check — and a compiled example can panic on its first line. Running it is what turns the documented command into a promise instead of a comment.

Deliberately not attempted

§E7 also asks for "one example per engine". The three hosted engines need a live endpoint and a credential — which is exactly why adapters/remote/examples/conformance.rs is a manual CLI and not a CI target — and the embedded engine needs the host's process-global seams installed before a client will open. Neither belongs behind a cargo run --example a contributor is told to run. Flagging rather than silently dropping it.

cargo doc -D warnings was already green and stays green.

Public API changes

Additive only:

New: impl std::error::Error for FallbackReason
New: examples/basic.rs

Validation

Command	Result
`cargo fmt --all -- --check`	pass
`cargo clippy --all-targets --all-features -- -D warnings`	pass
`cargo clippy -p tinymemory-tinycortex --all-targets --no-default-features -- -D warnings`	pass
`cargo build --all-targets --all-features`	pass
`cargo test --all-features`	pass — 1110 passed, 0 failed
`RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-features`	pass
`cargo run --example basic`	exit 0 — the documented command, now true
module crate `cargo test --lib`	pass — 35

`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)

Issue tinyhumansai#18 §A5. The driver registry could answer "is this driver id real, and is it allowed to answer for memory", and nothing asked it: `DriverRegistry::admit` had no caller outside its own tests, `MemoryHostConfig::memory_provider()` had no reader, and the memory client factory constructed TinyCortex unconditionally. Configuration could not choose an engine. `DriverRegistry::select` closes that: it reads the engine from the host's configuration and puts it through `admit`, so both surfaces are now live. Two corrections to the issue, both load-bearing. §A5 names `memory_provider()` as the selector. That method is a `provider:model` routing string for the memory *workload* — which language model does summarisation and entity extraction — not the store the memory lives in. Reading it would have let a model change repoint a company's storage. Selection reads a new `memory_driver()` instead, defaulted to `None` so it breaks no existing implementation, and a test pins that the two fields stay independent. §A5 also asks that `create_memory_*` return a bound `Arc<dyn MemoryProvider>`. It cannot, and the reason is structural rather than unfinished: since §C3 `adapters/tinycortex` depends on `tinymemory-core`, so a core factory returning a constructed adapter provider is a dependency cycle. Selection therefore resolves the decision and the host constructs — which is what `src/registry`'s module docs have said all along: "It resolves the class, not the instance." A configuration naming no engine gets the reserved embedded default, so adding selection does not turn "I configured nothing" into a host that fails to start. Going through `select` does not loosen admission either: an external engine named in config is still refused without endpoint, credential and trust. Refs tinyhumansai#18 (§A5)

The `memory-git` feature added alongside the lifted diff family gates that family, and the test asserting it is *withheld* without the feature is `#[cfg(not(feature = "memory-git"))]`. Both existing jobs — `--all-features` and default — compile that test out, so it was checked by nothing: a feature-gated test whose default fate is to be built by one job and executed by none. Adds the configuration as its own step, and asserts the property the feature exists for: `cargo tree --no-default-features` must link no `git2` or `libgit2-sys`. A guard rather than a comment, because "this feature keeps the native build out of the graph" is a claim that silently stops being true the first time a dependency picks it up transitively. Verified: `diff_is_withheld_when_the_snapshot_store_is_compiled_out` appears in `--no-default-features -- --list` and is absent from the `--all-features` listing, which is what "running nowhere" looked like. Refs tinyhumansai#18 (§C3, §E2)

Issue tinyhumansai#18 §D4, with §D3 alongside it. `api/Cargo.toml` spells out a `cargo tree` command in a comment and asks that the contract crate never link a storage engine, a native library, an HTTP client, or an async runtime. It was left as a comment, so nothing ran it. A forbidden dependency does not arrive by someone typing it into the manifest; it arrives transitively, through a feature enabled two crates away, which is exactly the way nobody notices. The forward form is the one that works, and the manifest already explains why: `cargo tree -i <crate> -p tinymemory-api` discards the `-p` scope, prints the whole-workspace inverse tree, and exits 0 looking clean even when this crate is the one at fault. This runs what the comment says to run. Verified in both directions. The rule holds today — no match against `rusqlite|libsqlite|git2|reqwest|regex|tokio`. And injecting `regex = "1"` into the manifest makes the guard fire on `regex`, `regex-automata` and `regex-syntax`, then reverting makes it pass again. A guard nobody has watched fail is not yet a guard. §D3 asks that `--no-default-features` still compile and bind `NullMemoryProvider`. It does, so this pins it rather than changing anything: the minimal configuration builds, and tinyhumansai#21's `null_provider` integration test runs against it — so "usable" is asserted, not just "compiles". That test is the one that would catch a null driver which panics instead of answering, or reports a fault instead of `Ready`. Refs tinyhumansai#18 (§D3, §D4)

Issue tinyhumansai#18 §E6: "backend error, timeout, and partial-page responses on every remote adapter — currently zero coverage". The three existing per-adapter test files all drive a backend that answers correctly, which is the half that was never in doubt. Six tests, each running against all three adapters over a real TCP socket, using the axum-double harness the happy-path tests already use: - a 500 on write is reported rather than swallowed; - a 500 on read is not laundered into `Ok(None)`; - a 401 is not presented as an empty store, on either `list` or `recall`; - a `200 OK` carrying unparseable JSON is an error and not a panic; - an unreachable backend is reported rather than hanging; - a paginated export terminates instead of looping. The read case is the one that matters. `Ok(None)` after a 500 says "this memory does not exist" when the truth is "I could not ask", and a caller cannot tell those apart: it writes the memory again, or tells a user their memory is gone, or a sync job treats the empty read as authoritative and prunes. Nothing surfaces until much later. All three adapters already behave correctly — this pins behaviour rather than fixing it. That is worth stating plainly, because six tests passing on the first run is exactly what a test that never exercises its subject also looks like. So the read assertion additionally requires the backend's status to survive into the error message; without that it would pass just as happily if the adapter had failed on URL construction and never reached the network. Confirmed against the live errors: `memory API v3/container-tags/list returned HTTP 500`, `memory API memories?top_k=1000 returned HTTP 500`, and `memory API api/v1/datasets returned HTTP 500`. The assertions are deliberately about whether a failure comes back at all, not about which error it is. The adapters' HTTP layer `bail!`s into `anyhow`, so every one of these arrives as `MemoryError::Other` and "unsupported" is not yet distinguishable from "failed" — that is §A4, and it is not what this change is. The unreachable-backend test binds a port, reads its number, and drops the listener, so the address is reliably closed rather than merely unlikely to be in use. The export test is bounded by a timeout so a non-terminating implementation fails the test instead of hanging the suite. Refs tinyhumansai#18 (§E6)

Issue tinyhumansai#18 §E7. `AGENTS.md` documents an `examples/` directory and tells the reader to run `cargo run --example basic`. Neither existed: no `examples/` directory at all, so the documented command has been failing for as long as it has been documented. (The issue attributes the reference to `README.md`; it is `AGENTS.md` lines 30 and 74.) The example binds the null driver, which needs no engine, no workspace and no network, so it runs anywhere. It walks the order a host actually follows — admit an id, then construct, then check the negotiated capabilities, then use the mandatory families — because that order is the part worth demonstrating. Admission is engine-neutral and answers "is this id real and may it answer for memory"; construction needs everything an engine needs. Swapping `NullMemoryProvider` for an adapter's provider changes nothing else in the file. Writing it surfaced an API gap, which is the argument for having a compiled example at all: `FallbackReason` implemented `Display` but not `std::error::Error`, so the obvious `registry.admit(..)?` in a function returning `Box<dyn Error>` or `anyhow::Error` did not compile. Adding the impl is purely additive and changes nothing about the type; it makes the message usable where refusals actually travel. CI runs the example rather than only building it. `cargo build --all-targets` compiles examples, so a broken one still passes — and a compiled example can panic on its first line. Running it is what makes the documented command a promise rather than a comment. Not attempted here: §E7's "one example per engine". The three hosted engines need a live endpoint and a credential, which is why `adapters/remote/examples/conformance.rs` is a manual CLI rather than a CI target, and the embedded engine needs the host's process-global seams installed. Neither belongs in a `cargo run --example` a contributor is told to run. Refs tinyhumansai#18 (§E7)

coderabbitai · 2026-08-17T12:13:26Z

Important

Review available on request

🔍 Trigger review

Reviews should be triggered manually for repositories with fewer than 10 stars. Select Trigger review above or comment @coderabbitai review to review the latest changes. For a full review, comment @coderabbitai full review.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ff65ae78-86f9-4dcf-8ebc-8e41a5b3728b

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.}

tinysweeper

tinysweeper found nothing blocking. Approving.

$0.0000 · 0 in / 0 out · 795 embedded · openrouter/openai/text-embedding-3-small

tinysweeper · 2026-08-17T12:14:25Z

How this change flows

0 changed behaviours across 3 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["main"]:::impacted
  n2["error"]:::impacted
  n3["admit"]:::impacted
  n4["TinycortexProvider"]:::impacted
  n1 -->|uses| n2
  n1 -->|calls| n3
  n4 -->|implements| n0
  classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
  classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
  classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
  classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff

_{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.}

M3gA-Mind

Reviewed this PR's own commit (7128412). Clean.

I came to this one expecting a contract change — the title "make a refusal an Error" reads like a return-shape change, and I was looking for callers matching on the old shape. It is not that. The entire library change is one line:

impl std::error::Error for FallbackReason {}

Purely additive. FallbackReason's fields are unchanged, no function's return type changed, no variant moved. Nothing that compiled before stops compiling and no match breaks, so there is nothing for callers to update and nothing to declare. The only theoretical break is a downstream blanket impl<E: Error> From<E> for _ newly becoming applicable and creating an inference ambiguity — that would surface as a compile error rather than silent breakage, and I found no such pattern in-tree. UNVERIFIED for out-of-tree consumers, but the risk is negligible.

If you want the title to carry that, "…and implement Error for FallbackReason" would signal additive rather than breaking. Minor.

Two things I liked:

CI runs the example rather than only building it. cargo build --all-targets compiles examples, so a broken one passes — and a compiled example can panic on its first line. cargo run --example basic is what makes the documented command a promise. Given that AGENTS.md has pointed at a command that did not exist for as long as it has been documented, running it is the fix, not adding it.

The Error gap was found by writing the example. That is the actual argument for keeping compiled examples around, and it is good that the commit message says so rather than presenting the impl as an unrelated tidy-up.

Not attempting §E7's "one example per engine" is the right call, and the reason given (live endpoint + credential for the three hosted engines; process-global seams for the embedded one) is sound — neither belongs in a command a contributor is told to run cold. Worth noting on #18 so that half of §E7 stays visibly open rather than looking delivered.

YellowSnnowmann added 11 commits August 17, 2026 15:38

tinysweeper Bot approved these changes Aug 17, 2026

View reviewed changes

tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Aug 17, 2026

YellowSnnowmann mentioned this pull request Aug 17, 2026

Measure the build: feature powerset, dependency budget, coverage (#18 §D5/§E2/§E8) #28

Merged

M3gA-Mind mentioned this pull request Aug 17, 2026

Housekeeping: drop committed binaries, align the MSRV, document core/ #19

Merged

M3gA-Mind reviewed Aug 17, 2026

View reviewed changes

YellowSnnowmann merged commit 849b815 into tinyhumansai:main Aug 18, 2026
12 checks passed

Navigation Menu

Uh oh!

Restore the bundled example, and make a refusal an Error (#18 §E7) - #27

Restore the bundled example, and make a refusal an Error (#18 §E7)#27
YellowSnnowmann merged 11 commits into
tinyhumansai:mainfrom
YellowSnnowmann:feat/18-e7-restore-the-bundled-example

YellowSnnowmann commented Aug 17, 2026

Uh oh!

coderabbitai Bot commented Aug 17, 2026

Review available on request

Uh oh!

tinysweeper Bot left a comment

Uh oh!

tinysweeper Bot commented Aug 17, 2026

Uh oh!

M3gA-Mind left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

YellowSnnowmann commented Aug 17, 2026

Summary

The example

It found a real API gap

CI runs it, rather than only building it

Deliberately not attempted

Public API changes

Validation

Related

Uh oh!

coderabbitai Bot commented Aug 17, 2026

Review available on request

Uh oh!

tinysweeper Bot left a comment

Choose a reason for hiding this comment

Uh oh!

tinysweeper Bot commented Aug 17, 2026

How this change flows

Uh oh!

M3gA-Mind left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants