Skip to content

fix(conformance): make every family the reference driver serves actually readable - #151

Merged
YellowSnnowmann merged 5 commits into
tinyhumansai:mainfrom
YellowSnnowmann:fix/full-driver-retains-every-family
Sep 9, 2026
Merged

fix(conformance): make every family the reference driver serves actually readable#151
YellowSnnowmann merged 5 commits into
tinyhumansai:mainfrom
YellowSnnowmann:fix/full-driver-retains-every-family

Conversation

@YellowSnnowmann

Copy link
Copy Markdown
Contributor

Summary

The reference driver in tinymemory-conformance serves 27 capability families. Four of them accepted writes and answered as if nothing had happened, and one broke a documented validation rule. Every one of the five passed the whole suite.

They were found by binding this driver, downstream, in place of a real engine — tinyhumansai/openhuman#6161 does exactly that so its test suite no longer compiles 133k lines of engine — and watching 22 of that host's handler tests fail on the round trip.

What was wrong

Family Wrote Read back
documents put_document stored list_documents[]
tool_memory put_tool_rule stored tool_rules[], delete_tool_rulefalse
goals set_goals stored goals → the default
graph put_relation stored relations[]

Plus three the same host surfaced once those were storing:

  • list_documents and delete_document are the only two methods in the contract that answer an untyped serde_json::Value, and neither doc comment said what was inside. The engine returns {"count": N, "documents": [{"documentId": …}]} and {deleted, namespace, documentId}; this driver returned snake_case without a count, and {deleted} alone. A host decoding either got missing field \documentId`` from one driver and rows from the other. Both shapes are now written into the trait's doc comments and asserted by the suite.
  • search_entities accepted any kind string. The contract requires MemoryError::Invalid for an unrecognised one, and its module docs give the reason: "silently matching nothing would look identical to a genuine empty result". A driver with an empty entity index is where this is least visible and most dangerous. That is precisely how it showed up downstream — a misspelled kind came back as [] and read as "no such entity".
  • recall_documents returned empty context unconditionally, so a namespace holding documents reported itself empty. The contract's "an empty namespace returns empty context" carries the converse.

Why the suite could not see any of this

assert_provider gates every optional family on as_*() and then checks its shape. A driver that advertises a family, accepts its writes and answers its reads empty satisfies all of that. The retains_writes probe added in #150 closes the hole for the entry tier only — it stores through MemoryCore and reads through MemoryCore — and the four families above are reached through their own accessors.

So the suite gained readers, not just shapes:

  • assert_documents_round_trip now lists, deletes, and re-deletes, pinning both untyped envelopes and the "a missing document is an outcome, not an error" rule the doc comment already promised.
  • assert_search_entities_rejects_unknown_kind runs above the retains_writes gate, because it is a shape rule that applies to a driver retaining nothing.
  • Query-less recall must reach a document the namespace holds.
  • the_full_driver_retains_every_family_it_serves is the per-family analogue of retains_writes, so "write-only" cannot recur silently in the driver itself.

Every new assertion passes against tinycortex unchanged (full_provider_conformance, 35 tests). That is what makes each one a contract rule rather than one driver's behaviour — and it is why the fixes are in the driver, not in the assertions.

Deliberate non-assertions

Three things are not pinned, each for a reason worth keeping:

  • Row order in list_documents. The engine orders by updated_at DESC and two writes can land in the same tick. An order assertion would be a flake wearing a contract's clothes. The driver still sorts that way, with the key as a tiebreak, so a host's list does not jump between reads.
  • Which entity kinds a driver accepts. EntityMatch::kind is an open vocabulary on the response side on purpose; a driver that grows a kind must not start failing this suite. Only the request-side rejection is checked, using a string no engine can adopt.
  • Ranking in recall_documents. Freshness is updated_at, which any storing driver has; how priority weighs against it is a scoring model this driver has no business inventing. priority breaks ties and score stays 0.0 rather than looking like a signal a caller could sort on.

Verification

fmt / clippy / build / test (workspace)     0
module workspace build + clippy              0
tinymemory-api doctests                      0
reference_drivers                            7 passed
full_provider_conformance (real engine)     35 passed

tinymemory-conformance's dependencies are unchanged — anyhow, async-trait, serde_json, tinymemory-api. It still cannot reach an engine.

Downstream, openhuman#6161's memory:: suite goes from 22 failures to 2, and both survivors were verified to fail identically on that repo's main (bound_driver_status_reports_id_class_contract_and_capabilities and search_entities_rpc_rejects_unknown_entity_kind, both waiting out the module-load grace) — pre-existing, unrelated to this change.

Test plan

  • cargo fmt --all --check
  • cargo clippy --workspace --all-targets -- -D warnings (the pre-existing tinymemory-documents unwrap_used errors on main are untouched by this branch)
  • cargo test --workspace
  • cargo test -p tinymemory-tinycortex --test full_provider_conformance
  • cargo build/clippy --manifest-path crates/tinymemory-module/Cargo.toml

Refs #147

Four families accepted writes and answered as if none had happened:
`put_document` stored while `list_documents` returned `[]`, `put_tool_rule`
stored while `tool_rules` returned `[]`, `set_goals` stored while `goals`
returned the default, and `put_relation` stored while `relations` returned
`[]`. `delete_tool_rule` answered `false` for a rule that was there.

This is the same defect tinyhumansai#150 fixed for the entry tier, in the families tinyhumansai#150
did not check. The shape is worth naming because it is the one this driver
keeps reproducing: a family advertised through `as_*()`, its writes accepted,
its reads empty — which passes every structural check the suite makes and
fails only when a host asks for the row back.

Found downstream. tinyhumansai/openhuman#6161 binds this driver in place of
its in-process engine, and 22 of that host's handler tests fail on the round
trip — `doc_put` then `doc_list`, `put_tool_rule` then `tool_rules`. They are
host tests, not engine tests, so the answer is a driver that stores rather
than tests that go.

`the_full_driver_retains_every_family_it_serves` is the guard, and it is a
probe rather than a suite assertion for the same reason `retains_writes` is:
`assert_provider` gates optional families on `as_*()`, and a driver that
advertises one and discards its writes satisfies every shape check it makes.
Nothing in the suite can see this class. One probe per storing family can.

Scope held deliberately: `relations` filters on namespace, subject and
predicate — the contract's own selectors — and nothing ranks or scores.
`delete_tool_rule` requires the id *and* the tool name to match, because the
pair is what the caller asserted even though the id alone is unique.

Refs tinyhumansai#147
`MemoryDocuments::list_documents` answers a `serde_json::Value`, so no type
makes two drivers agree on what is inside it, and the suite never called the
method at all. Both halves of that were load-bearing: the reference driver
returned `{"documents": [{"document_id": …, "content": …}]}` and the engine
returns `{"count": N, "documents": [{"documentId": …, "sourceType": …}]}`, and
both passed the whole suite, because the suite asked nothing.

A host reading the envelope gets `missing field \`documentId\`` from one driver
and rows from the other. That is the untyped-payload version of the same defect
as the previous commit — a family that looks served from the outside and is not
the thing the caller needs — and it is found the same way, downstream.

The engine's shape is the contract, so the reference driver moves to it:
camelCase keys, `createdAt` / `updatedAt` from a real clock rather than 0.0, the
`taint`, the `count`, and `updated_at DESC` ordering.

`assert_documents_round_trip` now pins it, and pins what the contract actually
promises rather than what one engine happens to emit: the two envelope fields,
`count` agreeing with the array's length, the nine per-row keys, and the row
matching the namespace and key that were written.

Row **order** is deliberately not asserted. The engine orders by `updated_at
DESC` and two writes can land in the same tick, so an ordering assertion would
be a flake wearing a contract's clothes. The reference driver still sorts that
way — a driver whose order is arbitrary would make a host's list jump between
reads — with the key as a tiebreak so the order is total rather than merely
stable.

Refs tinyhumansai#147
… says

`MemoryRetrieval::search_entities` documents `Invalid` for an unrecognised kind
in `kinds`, and its module docs give the reason: "silently matching nothing
would look identical to a genuine empty result". The reference driver answered
`Ok(vec![])` to any filter at all, and the suite never asked.

A driver with an empty entity index is where this is least visible and most
dangerous — every query answers no hits whether the filter was a typo or not —
so the driver that answers nothing is precisely the one that has to check.

Found downstream, in the shape the docs predict: a host passed the kind
`"not-a-kind"`, got `[]` rather than an error, and reported "no such entity".

The suite now asserts it above the `retains_writes` gate, because this is a
shape rule and applies to a driver that retains nothing just as much as to one
that retains everything.

Only the request side is checked, and that asymmetry is the contract's:
`EntityMatch::kind` in a *response* is an open vocabulary on purpose — a closed
enum would turn a newly-emitted kind into a deserialization failure instead of
an unfamiliar label — so nothing here asserts which kinds a driver accepts. A
driver that grows a new one must not start failing this suite. The probe uses a
string no engine can ever adopt rather than a plausible-looking kind.

`tinycortex` already conformed; the assertion passes against it unchanged,
which is what makes it a contract rule rather than one driver's behaviour.

Refs tinyhumansai#147
…pes down

`delete_document` is the second and last method in the contract answering an
untyped `serde_json::Value`, and it had the same defect as the first: the
engine returns `{deleted, namespace, documentId}`, the reference driver
returned `{deleted}` alone, and nothing anywhere compared them. A host decoding
the envelope got `missing field \`namespace\`` from one driver and a row from
the other.

Two untyped payloads exist in the whole contract. Both are now pinned, so the
class is closed rather than one instance of it fixed.

The shapes are also written into the trait's own doc comments. They existed
only inside the engine's SQL layer, one call away from the trait a driver
author actually reads — which is why writing a second driver produced a
different shape without anyone doing anything wrong. The doc comment says what
the type cannot, and the suite is the executable half of it.

The `namespace` echo is documented as the namespace *as the driver stores it*,
not as the caller passed it: TinyCortex sanitises, so returning the caller's
string would be a lie for the one driver whose behaviour prompted the field.

Deleting an already-absent document is asserted to report `deleted: false`
rather than to fail, which is what the method's existing error note promises
and what nothing checked.

Refs tinyhumansai#147
`recall_documents` returned an empty context unconditionally. For a namespace
holding documents that is the write-only shape once more, reached through a
third reader: `put_document` accepted the write, `list_documents` (after the
first commit here) showed it, and this said the namespace was empty. The
contract's "an empty namespace returns empty context" carries the converse, and
nothing checked it.

Freshness is the whole of the ranking, and deliberately so. The contract calls
this "the namespace's freshness and priority ranking"; freshness is
`updated_at`, which any driver storing documents already has, whereas how
priority *weighs against* it is a scoring model this driver has no business
inventing. So `priority` breaks ties and nothing more, and `score` stays 0.0
rather than a number that would look like a signal a caller could sort on.

`context_text` is assembled from the hits, because that is what the field's own
doc comment says it is. Rendering it independently is how the two come to
disagree.

The suite assertion tolerates `Unsupported` — the method's error note says a
provider predating this optional operation answers exactly that — and does not
assert ranking, only that a document the namespace holds is reachable. The
engine passes it unchanged, which is what makes it a contract rule.

That is the fourth reader of the same defect, and they were all found the same
way: a host bound this driver in place of its engine and its own handler tests
went red. Three of the four were invisible to a suite that had every structural
check and no reader.

Refs tinyhumansai#147
@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 5f2369fd-2831-4731-aef4-bd51af8d4ab5


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

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

tinysweeper found nothing blocking. Approving.

             $0.0219 · 145,218 in / 13,548 out · 34,647 cached (24%) · openrouter/openai/text-embedding-3-small, deepseek/deepseek-v4-flash, z-ai/glm-5.2 · 693 embedded
critique:    $0.0096 · 55,222 in  / 8,343 out  · 11,439 cached (21%) · deepseek/deepseek-v4-flash, z-ai/glm-5.2
security:    $0.0060 · 55,036 in  / 1,596 out  · 12,722 cached (23%) · deepseek/deepseek-v4-flash, z-ai/glm-5.2
tests:       $0.0018 · 21,137 in  / 141 out    · 0 cached (0%)       · deepseek/deepseek-v4-flash
description: $0.0045 · 13,823 in  / 3,468 out  · 10,486 cached (76%) · z-ai/glm-5.2

@tinysweeper

tinysweeper Bot commented Sep 9, 2026

Copy link
Copy Markdown

How this change flows

5 changed behaviours across 9 relationships. 5 surrounding behaviours are shown (60 graph nodes walked). 42 further behaviours left out to keep the diagram readable.

flowchart LR
  n0["Call<br/>changed"]:::changed
  n1["RecordingProvider<br/>changed"]:::changed
  n2["assert_documents_round_trip<br/>changed"]:::changed
  n3["assert_provider<br/>changed"]:::changed
  n4["the_full_driver_retains_writes<br/>changed"]:::changed
  n5["ns"]:::impacted
  n6["assert"]:::impacted
  n7["Result"]:::impacted
  n8["record"]:::impacted
  n9["recall"]:::impacted
  n1 -->|uses| n0
  n2 -->|calls| n5
  n2 -->|calls| n6
  n3 -->|calls| n2
  n4 -->|calls| n6
  n8 -->|uses| n0
  n9 -->|uses| n0
  n9 -->|uses| n7
  n9 -->|calls| n8
  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
Loading

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 0.1.0

@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Sep 9, 2026
@YellowSnnowmann
YellowSnnowmann merged commit 5c55431 into tinyhumansai:main Sep 9, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant