From ab3a1e6cd6e86a32db10377a34c5c832f39dde42 Mon Sep 17 00:00:00 2001 From: Ghost Scripter Date: Thu, 3 Sep 2026 00:48:29 +0530 Subject: [PATCH] State that recall's limit applies to the backend's ordering A driver that receives more candidates than `limit` and reduces them itself must keep the order the backend returned and take a prefix. The contract said ordering was backend-defined but never said an implementation may not impose its own, and the Cortex driver read that gap the wrong way: it folded, sorted by key, then truncated, so recall returned whichever keys sorted first and discarded the engine's best hits. Fixed there in #128; this is the rule it should have been able to read. The recall fixture now stores distinct content per key. Identical content leaves a backend free to order ties however it likes, and an arbitrary order is one nothing downstream can assert about. The assertion's docs say plainly that it does *not* check this rule, and why: the suite cannot know how an engine ranks these rows, and a driver that sorts before truncating is self-consistent, so no black-box comparison of two limits separates it from a correct one. That check belongs to each driver that folds before returning, and the doc points at the one in tinymemory-remote as the shape to copy. Better an explicit gap than a fixture that reads as coverage and is not. --- crates/tinymemory-api/src/traits.rs | 11 +++++++++ .../tinymemory-conformance/src/suite/mod.rs | 24 +++++++++++++++++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/crates/tinymemory-api/src/traits.rs b/crates/tinymemory-api/src/traits.rs index 04967b30..0f517077 100644 --- a/crates/tinymemory-api/src/traits.rs +++ b/crates/tinymemory-api/src/traits.rs @@ -88,6 +88,17 @@ pub trait Memory: Send + Sync { /// yield `Ok(vec![])`, not an error. Result ordering is backend-defined /// (typically most-relevant first) but callers must not assume a stable /// order across backends. + /// + /// **`limit` applies to the backend's ordering, never to one the + /// implementation imposes.** An implementation that receives more + /// candidates than `limit` and reduces them itself must keep the order the + /// backend returned them in and take a prefix of it. Sorting — by key, by + /// namespace, by anything — and *then* truncating silently discards the + /// backend's best hits and returns whichever rows happen to sort first, + /// which is the one thing a caller asking for the top `n` cannot detect. + /// This bites the append-only backends hardest, because they fold or + /// deduplicate before returning and the fold is where an order gets + /// imposed. async fn recall( &self, query: &str, diff --git a/crates/tinymemory-conformance/src/suite/mod.rs b/crates/tinymemory-conformance/src/suite/mod.rs index 5c8650bc..09c2f285 100644 --- a/crates/tinymemory-conformance/src/suite/mod.rs +++ b/crates/tinymemory-conformance/src/suite/mod.rs @@ -429,19 +429,39 @@ pub async fn assert_taint_is_preserved(provider: &dyn MemoryProvider) { /// `recall` honours its limit and its namespace filter. /// +/// # What this deliberately does not check +/// +/// That the limit was applied to the *backend's* ordering rather than one the +/// driver imposed. Checking it here is not possible: the suite cannot know how +/// a given engine ranks these rows, and a driver that sorts before truncating +/// is self-consistent, so no black-box comparison of two limits distinguishes +/// it. A driver that folds or deduplicates before returning — every append-only +/// backend does — needs its own test for this; see +/// `recall_keeps_the_engine_ranking_when_it_truncates` in `tinymemory-remote` +/// for the shape, and [`tinymemory_api::traits::Memory::recall`] for the rule. +/// +/// The rows below carry *distinct* content for a related reason: identical +/// content leaves a backend free to order ties however it likes, and an +/// arbitrary order is one nothing downstream can assert about. +/// /// # Panics /// /// Panics when recall exceeds the limit or crosses a namespace. pub async fn assert_recall_respects_limit_and_namespace(provider: &dyn MemoryProvider) { let who = provider.driver_id(); let (mine, theirs) = (ns(provider, "recall-a"), ns(provider, "recall-b")); + let rows = [ + ("r1", "needle filed in the archive room"), + ("r2", "needle left beside the mooring rope"), + ("r3", "needle found under the zinc roof"), + ]; let keys = ["r1", "r2", "r3"]; - for key in keys { + for (key, content) in rows { provider .store( &mine, key, - "shared needle text", + content, MemoryCategory::Core, None, MemoryTaint::Internal,