Run the conformance suite against the TinyCortex driver (#18 §E1) - #39
Merged
YellowSnnowmann merged 4 commits intoAug 18, 2026
Conversation
Issue tinyhumansai#18's acceptance criterion 5. The suite ran against the in-memory reference driver and the null driver — both written alongside it, so passing proved the assertions were self-consistent and not much else. The premise the whole issue rests on, that an engine other than TinyCortex can satisfy the contract, had never been exercised. Each adapter now runs the full `assert_provider` over a real TCP socket against a double that speaks its own HTTP shapes and retains what it is sent. That retention is the point: `failure_test`'s doubles only have to misbehave, while these have to work, because the suite writes and reads back. All three pass — eleven assertions each, including taint preservation, upsert identity, namespace isolation, and export/import round trip. Each adapter is paired with a second test asserting its double genuinely retains, and that pairing earned itself immediately. `cognee_upholds_the_ contract` passed while `the_cognee_double_actually_retains` failed: the suite returns early when a driver does not retain, so it had run four assertions and skipped the seven that matter, and reported success. Without the probe this would have been reported as "Cognee passes". The cause was the double, not the adapter. Cognee's data listing has to carry a `name` ending `.tinymemory[.json]` — the adapter skips anything else, because Cognee's own text loader strips the extension — and the listing returned only `id`, so every record was filtered out before the fetch. What this proves is narrower than "the hosted engines uphold the contract", and the module docs say so: nobody here can prove that about someone else's service. It is that *the adapter* does, given a backend answering its own documented shapes. A violation on the adapter's side of the wire — a dropped taint, a non-terminating export cursor, an upsert that duplicates — is caught. `retains_writes` is exported from the conformance crate for this: a caller standing up its own backend double needs it, for exactly the reason above. Not covered here: the TinyCortex adapter. It needs `require_embedding_host()`, a process-global, so driving it means installing host seams — which makes the test order-dependent unless it is isolated in its own target. Criterion 5 names it alongside the three, so it remains open. Refs tinyhumansai#18 (§E1, acceptance criterion 5)
Completes issue tinyhumansai#18's acceptance criterion 5. With the three hosted adapters already covered, this is the last driver the criterion names. `crate::provider` needs only a `tinycortex::memory::Memory` backend, so the suite runs against the engine's own `InMemoryMemoryStore` with no host seams. That is also the sharper test: it is the engine's simplest backend, so anything the suite catches is the adapter's behaviour rather than the storage engine's. It failed on the first run, which is the point of running it: tinycortex: store of `empty` failed: memory content cannot be empty The reference driver, the null driver and all three hosted adapters accept empty content. TinyCortex refuses it. The contract settles which is right — `MemoryCore::store` documents `MemoryError::Invalid` "for caller input the driver rejects" — so refusing is conformant and the *suite* was over-asserting. It required every content shape to round trip, which the contract never promised. `assert_awkward_content_round_trips` now allows a driver to refuse a shape, and still requires that a shape it *accepts* comes back unmangled. A guard keeps that from becoming vacuous: a driver that refused all four shapes fails, because it would otherwise pass having stored nothing. That correction surfaced a second finding, left open deliberately. The refusal arrives as `MemoryError::Other`, not `Invalid`: DIAG variant = Other(memory content cannot be empty) The engine's typed error is flattened through `anyhow` before the mandatory composition sees it, so a validation refusal is indistinguishable from a backend failure. Recovering it would need downcasting or string matching, and the real fix is §A4 — one error type across the contract. The suite says so where the assertion is, so the tightening to require `Invalid` has an obvious home rather than being rediscovered. Not weakened to get green: the reference driver accepts empty content and is still held to round-tripping it faithfully, as are the three hosted adapters. Refs tinyhumansai#18 (§E1, acceptance criterion 5)
|
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
added
the
priority: p2
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
| submodules: recursive | ||
| persist-credentials: false | ||
|
|
||
| - uses: dtolnay/rust-toolchain@stable |
There was a problem hiding this comment.
dtolnay/rust-toolchain is pinned to stable, which is mutable
A tag or branch can be repointed by whoever owns dtolnay, and the new code runs with this workflow's secrets. Pin to a full commit SHA and let Dependabot bump it.
[RULE] unpinned-action ·
| with: | ||
| components: llvm-tools-preview | ||
|
|
||
| - uses: Swatinem/rust-cache@v2 |
There was a problem hiding this comment.
Swatinem/rust-cache is pinned to v2, which is mutable
A tag or branch can be repointed by whoever owns Swatinem, and the new code runs with this workflow's secrets. Pin to a full commit SHA and let Dependabot bump it.
[RULE] unpinned-action ·
|
|
||
| - uses: Swatinem/rust-cache@v2 | ||
|
|
||
| - uses: taiki-e/install-action@v2 |
There was a problem hiding this comment.
taiki-e/install-action is pinned to v2, which is mutable
A tag or branch can be repointed by whoever owns taiki-e, and the new code runs with this workflow's secrets. Pin to a full commit SHA and let Dependabot bump it.
[RULE] unpinned-action ·
tinycortex#149 landed as a squash (8401346b), discarding the branch head this pin pointed at; 34cbb6c is diverged from tinycortex main rather than an ancestor of it. The merged commit is also the one that deletes the 33 duplicated files under api/src/, which is the state this stack depends on.
How this change flows3 changed behaviours across 10 relationships. 6 surrounding behaviours are shown (60 graph nodes walked). 29 further behaviours left out to keep the diagram readable. flowchart LR
n0["cognee_provider<br/>changed"]:::changed
n1["provider<br/>changed"]:::changed
n2["assert_awkward_content_round_trips<br/>changed"]:::changed
n3["MemoryProvider"]:::impacted
n4["assert_provider"]:::impacted
n5["ns"]:::impacted
n6["cleanup"]:::impacted
n7["CogneeMemory"]:::impacted
n8["Memory"]:::impacted
n0 -->|uses| n7
n1 -->|uses| n8
n2 -->|uses| n3
n2 -->|calls| n5
n2 -->|calls| n6
n4 -->|calls| n2
n4 -->|uses| n3
n5 -->|uses| n3
n6 -->|uses| n3
n7 -->|implements| 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
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
priority: p3
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
1 participant
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
Completes issue #18's acceptance criterion 5. #38 covered the three hosted adapters; this is the last driver the criterion names.
crate::providerneeds only atinycortex::memory::Memorybackend, so the suite runs against the engine's ownInMemoryMemoryStore— no host seams, no workspace. That is also the sharper test: it is the engine's simplest backend, so anything the suite catches is the adapter's behaviour rather than the storage engine's.It failed on the first run
Which is the point of running it.
The reference driver, the null driver, and all three hosted adapters accept empty content. TinyCortex refuses it.
The contract settles which is right.
MemoryCore::storedocuments:So refusing is conformant, and the suite was over-asserting.
assert_awkward_content_round_tripsrequired every content shape to round-trip — something the contract never promised. It was written against two drivers I also wrote; TinyCortex is the first real engine to disagree, and it was right.The corrected assertion
A driver may refuse a shape. A shape it accepts must still come back unmangled.
And a guard so that does not become vacuous:
This is not a weakening to get green. The reference driver accepts empty content and is still held to round-tripping it faithfully, as are all three hosted adapters. What changed is that the suite now asserts the contract's actual rule rather than an assumption nobody had stated.
A second finding, deliberately left open
The refusal arrives as the wrong variant:
The contract reserves
Invalidfor rejected caller input, so a validation refusal presenting asOtheris indistinguishable from a backend failure. That is exactly §1.7's complaint and exactly what §A4 exists to fix.I did not paper over it. The engine's typed error is flattened through
anyhowbefore the mandatory composition sees it, so recovering it needs downcasting or string matching — neither is a fix worth shipping, and the real one is §A4's single error type. The suite carries a comment at the assertion saying that when §A4 lands this should tighten to requireMemoryError::Invalid, so a genuine backend failure stops passing here.Criterion 5 status
Not covered:
engine::TinycortexProvider, the eighteen-family provider from §C3. It needs aMemoryClient, which needs the host's process-global seams, and a test installing a process global is order-dependent — ruled out byAGENTS.md. It needs its own integration target owning the global for the whole binary. The module docs record that.Public API changes
None. Tests, plus one corrected assertion in the conformance suite.
Validation
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-featuresRUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-featurescargo test -p tinymemory --no-default-features --test null_provider./scripts/ci/dependency-budget.shcargo test --libRelated
Part of #18 (§E1). Closes acceptance criterion 5 for every driver it names.