From 589f103bf6dc42f09be8a49b31423443745b3f52 Mon Sep 17 00:00:00 2001 From: Shanu Date: Thu, 20 Aug 2026 21:47:30 +0530 Subject: [PATCH 1/5] api: the error taxonomy learns its transport and backend classes (#18 A4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MemoryError gains Unauthorized, Unreachable, Timeout, Unavailable and Backend, and goes #[non_exhaustive] so the next class is an upgrade for downstream hosts, not a breakage. The wire grows their names; an older host degrades an unknown name to Other via the existing fallback, never to Invalid. engine_error stops flattening everything: it downcasts a typed MemoryError riding the anyhow payload, which makes every Memory-backed provider typed with zero signature churn on the deliberately-anyhow trait. The conformance suite tightens the assertion its own comment said was waiting: a store refusal must now BE Invalid — and the first run of the tightened suite caught a real offender, the tinycortex adapter's empty-content refusal arriving as opaque prose from the vendored engine. That adapter now enforces the same documented rule at its own boundary, typed (a mirror of the stated contract, not message sniffing). The failure suite upgrades from "an error comes back at all" to "and it is the right class": 401 downcasts to Unauthorized, 500 to Backend. The integration README stops documenting api.cognee.ai, a host that resolves and answers nothing — the tenant URL is the only Cognee Cloud address that exists. --- adapters/remote/src/failure_test.rs | 26 ++++++++++++++-------- adapters/tinycortex/src/memory.rs | 19 +++++++++++++++++ api/src/error.rs | 32 ++++++++++++++++++++++++++++ api/src/mandatory/mod.rs | 10 ++++++++- api/src/mandatory/provider.rs | 6 ++++++ api/src/traits.rs | 13 +++++++++++ api/src/wire.rs | 27 ++++++++++++++++++++++- api/src/wire_tests.rs | 28 ++++++++++++++++++++++++ conformance/src/suite/mod.rs | 22 ++++++++++--------- integration/remote-engines/README.md | 9 +++++--- 10 files changed, 168 insertions(+), 24 deletions(-) diff --git a/adapters/remote/src/failure_test.rs b/adapters/remote/src/failure_test.rs index d1a2a667..87711b4c 100644 --- a/adapters/remote/src/failure_test.rs +++ b/adapters/remote/src/failure_test.rs @@ -5,10 +5,10 @@ //! drive a backend that answers correctly, which is the half that was never in //! doubt. //! -//! The assertion that matters is not which error comes back — the contract's -//! error type is still `anyhow` under `MemoryError::Other` here, and §A4 is what -//! makes "unsupported" distinguishable from "failed". It is that a failure comes -//! back **at all**. +//! Since §A4 landed, the assertion is two-fold: a failure comes back **at +//! all**, and — where the class is knowable — it comes back **typed**: a 401 +//! downcasts to `Unauthorized`, a 500 to `Backend`, a dead port to +//! `Unreachable`, so a caller can act on the class instead of parsing prose. //! //! A read that answers `Ok(None)` when the backend returned 500 is saying "this //! memory does not exist" when the truth is "I could not ask". A caller cannot @@ -26,6 +26,7 @@ use axum::http::StatusCode; use axum::routing::{any, get}; use axum::Router; +use tinymemory_api::error::MemoryError; use tinymemory_api::provider::MemoryProvider; use tinymemory_api::recall::RecallOpts; use tinymemory_api::traits::Memory; @@ -94,10 +95,13 @@ async fn a_backend_failure_on_write_is_reported_rather_than_swallowed() { MemoryTaint::Internal, ) .await; + let error = result.expect_err("a 500 on write must surface"); assert!( - result.is_err(), - "{name}: a 500 on write must not report success — a caller that \ - believes the write landed has no reason to retry it" + matches!( + error.downcast_ref::(), + Some(MemoryError::Backend(_)) + ), + "{name}: a 500 must arrive typed as Backend, got: {error}" ); } } @@ -133,9 +137,13 @@ async fn an_unauthorized_backend_is_not_reported_as_an_empty_store() { let endpoint = failing(StatusCode::UNAUTHORIZED).await; for (name, memory) in adapters(&endpoint) { let listed = memory.list(None, None, None).await; + let error = listed.expect_err("a 401 must not present as an empty result set"); assert!( - listed.is_err(), - "{name}: a 401 must not present as an empty result set" + matches!( + error.downcast_ref::(), + Some(MemoryError::Unauthorized(_)) + ), + "{name}: a 401 must arrive typed as Unauthorized, got: {error}" ); let recalled = memory.recall("anything", 10, RecallOpts::default()).await; diff --git a/adapters/tinycortex/src/memory.rs b/adapters/tinycortex/src/memory.rs index 32359736..8bd055e5 100644 --- a/adapters/tinycortex/src/memory.rs +++ b/adapters/tinycortex/src/memory.rs @@ -11,6 +11,7 @@ use std::sync::Arc; use async_trait::async_trait; +use tinymemory_api::error::MemoryError; use tinymemory_api::recall::OwnedRecallOpts; use tinymemory_api::traits::Memory; use tinymemory_api::types::{ @@ -60,6 +61,18 @@ impl Memory for TinycortexMemory { category: MemoryCategory, session_id: Option<&str>, ) -> anyhow::Result<()> { + // §A4: the engine refuses empty content with an opaque anyhow + // (vendor/tinycortex, outside this repo's reach), which the mandatory + // composition can only flatten to `Other` — indistinguishable from a + // backend failure. Enforce the same documented rule HERE, typed, so + // the refusal arrives as the `Invalid` the conformance suite now + // requires. Not message-sniffing: this mirrors the engine's stated + // contract, it does not parse its prose. + if content.trim().is_empty() { + return Err(anyhow::Error::new(MemoryError::Invalid( + "memory content cannot be empty".to_string(), + ))); + } self.inner .store(namespace, key, content, category, session_id) .await @@ -79,6 +92,12 @@ impl Memory for TinycortexMemory { session_id: Option<&str>, taint: MemoryTaint, ) -> anyhow::Result<()> { + // Same typed refusal as `store` — see the comment there. + if content.trim().is_empty() { + return Err(anyhow::Error::new(MemoryError::Invalid( + "memory content cannot be empty".to_string(), + ))); + } self.inner .store_with_taint(namespace, key, content, category, session_id, taint) .await diff --git a/api/src/error.rs b/api/src/error.rs index c9e64056..b7e76a02 100644 --- a/api/src/error.rs +++ b/api/src/error.rs @@ -22,7 +22,14 @@ use thiserror::Error; use crate::capabilities::Capability; /// Errors surfaced by the memory engine. +/// +/// `#[non_exhaustive]` (issue #18 §A4): downstream hosts must keep a wildcard +/// arm, so the *next* class this enum learns to name is an upgrade for them, +/// not a breakage. Inside this workspace `wire::wire_name` still matches +/// exhaustively — a new variant is a compile error there, never a silent +/// fallthrough onto `OTHER`. #[derive(Debug, Error)] +#[non_exhaustive] pub enum MemoryError { /// A requested record / source / node was not found. #[error("not found: {0}")] @@ -73,6 +80,31 @@ pub enum MemoryError { /// driver reported. capability: String, }, + /// The backend rejected the configured credential, or required one that + /// was never configured (HTTP 401 / 403). Retrying cannot help; fixing + /// the key can. The message names the host and the auth scheme's hint, + /// never the credential itself. + #[error("unauthorized: {0}")] + Unauthorized(String), + /// The backend could not be reached at all — connection refused, DNS + /// resolution failed, or the TLS handshake broke. The request never + /// arrived, so nothing was applied. + #[error("unreachable: {0}")] + Unreachable(String), + /// The call exceeded its deadline. Whether the backend applied the work + /// is unknown — which is why write paths must never blindly retry this. + #[error("timed out: {0}")] + Timeout(String), + /// The backend answered that it cannot serve right now (HTTP 429, 502, + /// 503, 504): the retryable class, distinct from [`MemoryError::Backend`] + /// so a retry policy can key on it without parsing prose. + #[error("unavailable: {0}")] + Unavailable(String), + /// The backend answered, and the answer was a failure this contract has + /// no more specific name for (an unexpected 4xx/5xx with its status and + /// bounded body in the message). + #[error("backend failed: {0}")] + Backend(String), /// Catch-all wrapping an opaque lower-level error. #[error(transparent)] Other(#[from] anyhow::Error), diff --git a/api/src/mandatory/mod.rs b/api/src/mandatory/mod.rs index 578fc9da..f427268c 100644 --- a/api/src/mandatory/mod.rs +++ b/api/src/mandatory/mod.rs @@ -77,7 +77,15 @@ pub const SCOPE_UNAPPLIED: &str = /// actually known. #[must_use] pub fn engine_error(error: anyhow::Error) -> MemoryError { - MemoryError::Other(error) + // §A4: an adapter that already knows the failure's class attaches a typed + // MemoryError as the anyhow payload (adapters/remote/src/common.rs does + // for transport and HTTP-status failures). Recover it here instead of + // flattening everything to Other — this one line is what makes + // "unauthorized" distinguishable from "unreachable" across every + // `Memory`-backed provider without touching the trait's signatures. + error + .downcast::() + .unwrap_or_else(MemoryError::Other) } /// `MemoryCore::list` for the all-namespaces case. diff --git a/api/src/mandatory/provider.rs b/api/src/mandatory/provider.rs index 5eee27f8..2c4f46ed 100644 --- a/api/src/mandatory/provider.rs +++ b/api/src/mandatory/provider.rs @@ -188,6 +188,12 @@ impl MemoryProvider for MemoryTraitProvider { } async fn health(&self) -> MemoryHealth { + // A backend that can say more than a boolean does (issue #18 §U4): + // the remote adapters report the typed probe outcome — credential + // rejected, unreachable, throttled — instead of one frozen string. + if let Some(health) = self.memory.health_probe().await { + return health; + } if self.memory.health_check().await { MemoryHealth::Ready } else { diff --git a/api/src/traits.rs b/api/src/traits.rs index c96e0b9b..bf488658 100644 --- a/api/src/traits.rs +++ b/api/src/traits.rs @@ -154,4 +154,17 @@ pub trait Memory: Send + Sync { /// rather than `Err`, so it is safe to call from a liveness probe without /// error-handling boilerplate. async fn health_check(&self) -> bool; + + /// Rich health, when the backend can say more than a boolean (issue #18 + /// follow-up U4). + /// + /// `None` — the default every existing implementation inherits — means + /// "this backend only knows the boolean"; callers fall back to + /// [`Memory::health_check`]. `Some(health)` carries the typed answer: + /// `Down`/`Degraded` with a reason naming the failure class (credential + /// rejected, unreachable, throttled), never a credential or a payload — + /// the reason string reaches operator-facing status surfaces. + async fn health_probe(&self) -> Option { + None + } } diff --git a/api/src/wire.rs b/api/src/wire.rs index 0cba4efe..02a6e61f 100644 --- a/api/src/wire.rs +++ b/api/src/wire.rs @@ -66,6 +66,16 @@ pub const SERDE: &str = "ai.tinyhumans.tinymemory.Error.Serde"; pub const UNSUPPORTED: &str = "ai.tinyhumans.tinymemory.Error.Unsupported"; /// An opaque lower-level failure. pub const OTHER: &str = "ai.tinyhumans.tinymemory.Error.Other"; +/// The backend rejected or required a credential (§A4). +pub const UNAUTHORIZED: &str = "ai.tinyhumans.tinymemory.Error.Unauthorized"; +/// The backend could not be reached (connect / DNS / TLS) (§A4). +pub const UNREACHABLE: &str = "ai.tinyhumans.tinymemory.Error.Unreachable"; +/// The call exceeded its deadline (§A4). +pub const TIMEOUT: &str = "ai.tinyhumans.tinymemory.Error.Timeout"; +/// The backend answered that it cannot serve right now (§A4). +pub const UNAVAILABLE: &str = "ai.tinyhumans.tinymemory.Error.Unavailable"; +/// The backend answered with an otherwise-unclassified failure (§A4). +pub const BACKEND: &str = "ai.tinyhumans.tinymemory.Error.Backend"; /// The wire name for `error`. /// @@ -82,6 +92,11 @@ pub fn wire_name(error: &MemoryError) -> &'static str { MemoryError::Io(_) => IO, MemoryError::Serde(_) => SERDE, MemoryError::Unsupported { .. } => UNSUPPORTED, + MemoryError::Unauthorized(_) => UNAUTHORIZED, + MemoryError::Unreachable(_) => UNREACHABLE, + MemoryError::Timeout(_) => TIMEOUT, + MemoryError::Unavailable(_) => UNAVAILABLE, + MemoryError::Backend(_) => BACKEND, MemoryError::Other(_) => OTHER, } } @@ -97,7 +112,12 @@ pub fn wire_message(error: &MemoryError) -> String { MemoryError::NotFound(message) | MemoryError::Invalid(message) | MemoryError::BudgetExceeded(message) - | MemoryError::PathEscape(message) => message.clone(), + | MemoryError::PathEscape(message) + | MemoryError::Unauthorized(message) + | MemoryError::Unreachable(message) + | MemoryError::Timeout(message) + | MemoryError::Unavailable(message) + | MemoryError::Backend(message) => message.clone(), MemoryError::Unsupported { capability } => capability.clone(), // No inner string to lift: these carry a foreign error type, so the // rendered form is all there is. @@ -124,6 +144,11 @@ pub fn from_wire(name: &str, message: &str) -> MemoryError { IO => MemoryError::Other(anyhow::anyhow!("io error: {message}")), SERDE => MemoryError::Other(anyhow::anyhow!("serde error: {message}")), UNSUPPORTED => MemoryError::unsupported_raw(message), + UNAUTHORIZED => MemoryError::Unauthorized(message.to_string()), + UNREACHABLE => MemoryError::Unreachable(message.to_string()), + TIMEOUT => MemoryError::Timeout(message.to_string()), + UNAVAILABLE => MemoryError::Unavailable(message.to_string()), + BACKEND => MemoryError::Backend(message.to_string()), _ => MemoryError::Other(anyhow::anyhow!("{message}")), } } diff --git a/api/src/wire_tests.rs b/api/src/wire_tests.rs index 0653dc6d..22f00855 100644 --- a/api/src/wire_tests.rs +++ b/api/src/wire_tests.rs @@ -102,3 +102,31 @@ fn an_unknown_capability_name_off_the_wire_survives() { other => panic!("expected Unsupported, got {other:?}"), } } + +/// §A4: the five typed classes round-trip the wire with their messages, and +/// an OLD host that has never heard the new names degrades them to `Other` +/// (the `_ =>` fallback) rather than mislabeling them. +#[test] +fn a4_variants_round_trip_and_degrade_gracefully() { + let cases = [ + MemoryError::Unauthorized("401 from host".into()), + MemoryError::Unreachable("dns failed".into()), + MemoryError::Timeout("deadline".into()), + MemoryError::Unavailable("503".into()), + MemoryError::Backend("500 detail".into()), + ]; + for error in cases { + let name = wire_name(&error); + let message = wire_message(&error); + let rebuilt = from_wire(name, &message); + assert_eq!(wire_name(&rebuilt), name, "round trip changed the class"); + assert_eq!( + wire_message(&rebuilt), + message, + "round trip changed the message" + ); + } + // The unknown-name fallback IS the compatibility story. + let degraded = from_wire("ai.tinyhumans.tinymemory.Error.SomethingNewer", "detail"); + assert!(matches!(degraded, MemoryError::Other(_))); +} diff --git a/conformance/src/suite/mod.rs b/conformance/src/suite/mod.rs index 64754dd1..ff271916 100644 --- a/conformance/src/suite/mod.rs +++ b/conformance/src/suite/mod.rs @@ -24,6 +24,7 @@ use std::sync::Arc; use tinymemory_api::capabilities::Capability; +use tinymemory_api::error::MemoryError; use tinymemory_api::provider::{audit_provider, ExportRecord, MemoryProvider}; use tinymemory_api::recall::OwnedRecallOpts; use tinymemory_api::types::{MemoryCategory, MemoryTaint}; @@ -608,14 +609,12 @@ pub async fn assert_awkward_content_round_trips(provider: &dyn MemoryProvider) { // engine uses that to refuse empty content. What a driver may *not* do // is accept a value and hand back something else. // - // The refusal is not yet required to be `Invalid` specifically. The - // engine's own error is flattened through `anyhow` before the mandatory - // composition sees it, so a validation refusal currently arrives as - // `Other` and is indistinguishable from a backend failure. That is the - // gap §A4 closes; when it does, this should tighten to require - // `MemoryError::Invalid` so a genuine backend failure stops passing - // here. - if provider + // §A4 landed: a refusal must now be `Invalid` — the caller's input + // was rejected — never a transport/backend class wearing a refusal's + // clothes. A driver that answers `Unauthorized` or `Backend` here is + // not refusing the shape, it is failing, and failure must fail the + // suite instead of reading as a documented refusal. + match provider .store( &ns, key, @@ -625,9 +624,12 @@ pub async fn assert_awkward_content_round_trips(provider: &dyn MemoryProvider) { MemoryTaint::Internal, ) .await - .is_err() { - continue; + Ok(()) => {} + Err(MemoryError::Invalid(_)) => continue, + Err(other) => panic!( + "{who}: refusing `{key}` must be Invalid (a validation refusal); got: {other}" + ), } accepted.push(key); if let Some(got) = provider diff --git a/integration/remote-engines/README.md b/integration/remote-engines/README.md index c7600001..3b8256af 100644 --- a/integration/remote-engines/README.md +++ b/integration/remote-engines/README.md @@ -33,11 +33,14 @@ cargo run -p tinymemory-remote --example conformance -- \ supermemory https://api.supermemory.ai "$SUPERMEMORY_API_KEY" cargo run -p tinymemory-remote --example conformance -- \ - cognee-api https://api.cognee.ai "$COGNEE_API_KEY" + cognee-api "https://tenant-.aws.cognee.ai" "$COGNEE_API_KEY" ``` -For tenant-specific Cognee deployments, replace the shared endpoint with the -tenant URL issued by Cognee. The command writes a unique conformance namespace, +Cognee Cloud has **no shared endpoint** — `api.cognee.ai` resolves in DNS but +nothing listens there (see the constructor note in +`adapters/remote/src/cognee.rs`), which is why this crate exports no default +Cognee endpoint constant. The tenant URL printed beside your API key on the +Cognee dashboard is the only address that exists; substitute it above. The command writes a unique conformance namespace, verifies Core, Recall, and Portability, and deletes its test record before exiting. From b9c30d40f13892016c8987475698f3778ef46cc5 Mon Sep 17 00:00:00 2001 From: Shanu Date: Thu, 20 Aug 2026 21:47:30 +0530 Subject: [PATCH 2/5] adapters/remote: typed classification, read retries, deep health, honest min_score MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The transport classifier returns a real enum instead of prose; transport_error and status_error mint the A4 variants as the anyhow payload (401/403 Unauthorized with the credential hint, 404 NotFound, 429 and the gateway trio Unavailable, other non-2xx Backend; DNS, TLS and connect Unreachable; an unclassifiable transport failure stays Other rather than overclaiming). Log prose is unchanged. U5: the two read paths (json, text — every call on them is a list, search or raw fetch) retry up to three times with 250ms doubling backoff, gated on the typed transient classes, never on message substrings; writes and multipart stay un-retried because a timed-out write's fate is unknown. The client gains a 10s connect deadline beside the 60s default, and each adapter a with_request_timeout builder. U4: Dialect::health answers typed instead of bool. Supermemory probes the authenticated container-tags list — the root page answered 200 to a wrong key against a live server, so bad credentials looked healthy until the first real call. Mem0's fallback reports BOTH failures instead of a blind ||. Memory grows a defaulted health_probe (None for every existing impl); the mandatory provider prefers it, mapping Unavailable to the contract's first-ever constructed Degraded and everything else failed to Down with the probe's own reason. U6: an unscored hit no longer clears a min_score threshold — the old is_none_or made the filter silently inert against score-less backends. Cognee, whose API takes no threshold, over-fetches (capped) when one is set so client-side filtering can still fill the caller's limit. --- Cargo.lock | 8 + adapters/remote/Cargo.toml | 3 + adapters/remote/src/cognee.rs | 39 ++- adapters/remote/src/common.rs | 345 +++++++++++++++++++----- adapters/remote/src/mem0.rs | 36 ++- adapters/remote/src/supermemory.rs | 31 ++- adapters/remote/src/supermemory_test.rs | 5 +- vendor/tinycortex | 2 +- 8 files changed, 394 insertions(+), 75 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b8a40a68..84706073 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1869,7 +1869,15 @@ dependencies = [ name = "tinycortex-api" version = "0.1.1" dependencies = [ + "anyhow", + "async-trait", + "chrono", + "serde", + "serde_json", + "sha2 0.10.9", + "thiserror 2.0.20", "tinymemory-api", + "uuid", ] [[package]] diff --git a/adapters/remote/Cargo.toml b/adapters/remote/Cargo.toml index 18ec568e..ea30747d 100644 --- a/adapters/remote/Cargo.toml +++ b/adapters/remote/Cargo.toml @@ -20,6 +20,9 @@ anyhow = "1" # cap rather than buffered whole, because the endpoint is operator-supplied # and a broken or hostile one must not be able to OOM the host. reqwest = { version = "0.12", default-features = false, features = ["json", "multipart", "rustls-tls", "stream"] } +# Only the timer, for the read-retry backoff (issue #18 §U5) — reqwest already +# requires a tokio runtime, so this adds no new runtime assumption. +tokio = { version = "1", default-features = false, features = ["time"] } # Remote records are translated through a private, lossless envelope. serde = { version = "1", features = ["derive"] } # Streaming a capped body needs a Stream combinator. diff --git a/adapters/remote/src/cognee.rs b/adapters/remote/src/cognee.rs index 4bcede41..1e385658 100644 --- a/adapters/remote/src/cognee.rs +++ b/adapters/remote/src/cognee.rs @@ -20,6 +20,26 @@ pub struct CogneeMemory { } impl CogneeMemory { + /// Rebuilds the HTTP transport with a different per-request deadline + /// (issue #18 follow-up U5). The default is 60s with a 10s connect + /// deadline — right for interactive calls; a bulk migration or a tight + /// liveness probe may want its own budget. + /// + /// # Errors + /// + /// Fails only if the underlying HTTP client cannot be rebuilt — a + /// configuration-time failure, before any request is made. + pub fn with_request_timeout(mut self, timeout: std::time::Duration) -> anyhow::Result { + let client = self + .inner + .dialect_mut() + .client + .clone() + .with_timeout(timeout)?; + self.inner.dialect_mut().client = client; + Ok(self) + } + /// Connect to a self-hosted Cognee server. /// /// `access_token` is sent as a bearer token. Local deployments with @@ -343,6 +363,17 @@ impl Dialect for CogneeDialect { opts: RecallOpts<'_>, ) -> anyhow::Result> { let datasets = opts.namespace.map(Self::dataset_name); + // Cognee's recall API takes no score threshold, so `min_score` is + // enforced client-side by the shared pass in `common.rs` (issue #18 + // §U6). Over-fetch when a threshold is set — with `top_k == limit`, + // every hit the filter drops is a slot the caller asked for and + // cannot be backfilled. Capped: an aggressive threshold is not a + // license to pull the whole store. + let top_k = if opts.min_score.is_some() { + limit.saturating_mul(3).min(limit.saturating_add(50)) + } else { + limit + }; let response: Value = self .client .json( @@ -352,7 +383,7 @@ impl Dialect for CogneeDialect { "query": query, "search_type": "CHUNKS", "datasets": datasets.map(|name| vec![name]), - "top_k": limit, + "top_k": top_k, "only_context": true, "session_id": opts.session_id })), @@ -402,9 +433,9 @@ impl Dialect for CogneeDialect { Ok(true) } - /// Checks Cognee's aggregate health endpoint. - async fn health(&self) -> bool { - self.client.healthy("health").await + /// Probes Cognee's aggregate health endpoint, typed. + async fn health(&self) -> anyhow::Result<()> { + self.client.probe("health").await } } diff --git a/adapters/remote/src/common.rs b/adapters/remote/src/common.rs index a9aa53d7..43c07464 100644 --- a/adapters/remote/src/common.rs +++ b/adapters/remote/src/common.rs @@ -8,6 +8,7 @@ use reqwest::header::{HeaderValue, AUTHORIZATION}; use reqwest::{Method, RequestBuilder, StatusCode, Url}; use serde::{de::DeserializeOwned, Deserialize, Serialize}; use sha2::{Digest, Sha256}; +use tinymemory_api::error::MemoryError; use tinymemory_api::traits::Memory; use tinymemory_api::types::{ MemoryCategory, MemoryEntry, MemoryTaint, NamespaceSummary, RecallOpts, @@ -155,14 +156,31 @@ impl HttpClient { endpoint.set_path(&path); } Ok(Self { - inner: reqwest::Client::builder() - .timeout(std::time::Duration::from_secs(60)) - .build()?, + inner: Self::build_inner(std::time::Duration::from_secs(60))?, endpoint, auth, }) } + /// One place builds the reqwest client, so the two timeouts stay paired: + /// the per-request deadline, and a connect deadline that keeps a + /// black-holed endpoint from consuming the whole request budget before + /// the first byte. + fn build_inner(timeout: std::time::Duration) -> anyhow::Result { + Ok(reqwest::Client::builder() + .timeout(timeout) + .connect_timeout(std::time::Duration::from_secs(10).min(timeout)) + .build()?) + } + + /// Rebuilds this client with a different per-request deadline (issue #18 + /// follow-up U5). The 60s default suits interactive calls; a bulk + /// migration or a health probe may want its own budget. + pub(crate) fn with_timeout(mut self, timeout: std::time::Duration) -> anyhow::Result { + self.inner = Self::build_inner(timeout)?; + Ok(self) + } + /// Resolves a relative API path and attaches the configured authentication. fn request(&self, method: Method, path: &str) -> anyhow::Result { let url = self @@ -208,11 +226,22 @@ impl HttpClient { parts.join(": ") }; let class = classify_transport(error.is_timeout(), error.is_connect(), &chain); - if chain.is_empty() { - anyhow::anyhow!("memory API request to {host}: {class}") + let described = class.describe(); + let message = if chain.is_empty() { + format!("memory API request to {host}: {described}") } else { - anyhow::anyhow!("memory API request to {host}: {class} ({chain})") - } + format!("memory API request to {host}: {described} ({chain})") + }; + // §A4: the typed error rides as the anyhow payload, so + // `tinymemory_api::mandatory::engine_error` can downcast it back out + // at the contract boundary instead of flattening it into `Other`. + anyhow::Error::new(match class { + TransportClass::Timeout => MemoryError::Timeout(message), + TransportClass::Dns | TransportClass::Tls | TransportClass::Connect => { + MemoryError::Unreachable(message) + } + TransportClass::Other => return anyhow::anyhow!("{message}"), + }) } /// The error for a non-success status, written for the operator reading a @@ -238,7 +267,9 @@ impl HttpClient { } format!(" — {shown}") }; - match status.as_u16() { + // §A4: every bucket mints a typed [`MemoryError`] carried as the + // anyhow payload — same prose as before, now matchable downstream. + anyhow::Error::new(match status.as_u16() { 401 | 403 => { let hint = match &self.auth { Auth::ApiKey(_) | Auth::Token(_) => "check the API key", @@ -247,12 +278,66 @@ impl HttpClient { "the endpoint requires credentials this client was not configured with" } }; - anyhow::anyhow!( + MemoryError::Unauthorized(format!( "memory API {path} on {host}: the configured credential was rejected \ (HTTP {status}) — {hint}{detail}" - ) + )) + } + 404 => MemoryError::NotFound(format!( + "memory API {path} on {host} returned HTTP 404{detail}" + )), + // The answered-but-cannot-serve class: rate limiting and the + // gateway trio. Distinct from `Backend` so a retry policy can key + // on it without parsing prose. + 429 | 502 | 503 | 504 => MemoryError::Unavailable(format!( + "memory API {path} on {host} returned HTTP {status}{detail}" + )), + _ => MemoryError::Backend(format!( + "memory API {path} on {host} returned HTTP {status}{detail}" + )), + }) + } + + /// Whether an error is worth one more attempt on a READ path: the typed + /// transient classes only (issue #18 follow-up U5). Keyed on the §A4 + /// variants rather than message substrings — the fragility the + /// composio sync client's needle-matching retry shows the cost of. + /// `Unauthorized`, `Invalid`, `NotFound`, `Backend` never retry: the + /// answer will not change. + fn retryable(error: &anyhow::Error) -> bool { + matches!( + error.downcast_ref::(), + Some( + MemoryError::Timeout(_) | MemoryError::Unreachable(_) | MemoryError::Unavailable(_) + ) + ) + } + + /// Runs a read-path attempt up to three times with 250ms·2ⁿ backoff. + /// + /// READ paths only — `json` and `text` below, whose calls are all list, + /// search and raw-fetch operations across the three adapters. The write + /// paths (`empty`, `multipart`) are deliberately not routed through + /// here: a `Timeout` on a write leaves whether the backend applied it + /// unknown, and Cognee's multipart upsert and Mem0's add are not + /// idempotent. + async fn with_read_retry(&self, attempt: F) -> anyhow::Result + where + F: Fn() -> Fut, + Fut: std::future::Future>, + { + const MAX_ATTEMPTS: u32 = 3; + let mut tried = 0; + loop { + tried += 1; + match attempt().await { + Ok(value) => return Ok(value), + Err(error) if tried < MAX_ATTEMPTS && Self::retryable(&error) => { + let backoff = std::time::Duration::from_millis(250) * 2_u32.pow(tried - 1); + tokio::time::sleep(backoff).await; + } + Err(error) => return Err(error), } - _ => anyhow::anyhow!("memory API {path} on {host} returned HTTP {status}{detail}"), } } @@ -262,38 +347,44 @@ impl HttpClient { path: &str, body: Option<&serde_json::Value>, ) -> anyhow::Result { - let mut request = self.request(method, path)?; - if let Some(body) = body { - request = request.json(body); - } - let response = request - .send() - .await - .map_err(|error| self.transport_error(error))?; - let status = response.status(); - if !status.is_success() { - let body = response.text().await.unwrap_or_default(); - return Err(self.status_error(path, status, &body)); - } - let body = read_capped(response, path).await?; - serde_json::from_slice(&body) - .with_context(|| format!("memory API {path} returned invalid JSON")) + self.with_read_retry(|| async { + let mut request = self.request(method.clone(), path)?; + if let Some(body) = body { + request = request.json(body); + } + let response = request + .send() + .await + .map_err(|error| self.transport_error(error))?; + let status = response.status(); + if !status.is_success() { + let body = response.text().await.unwrap_or_default(); + return Err(self.status_error(path, status, &body)); + } + let body = read_capped(response, path).await?; + serde_json::from_slice(&body) + .with_context(|| format!("memory API {path} returned invalid JSON")) + }) + .await } /// Sends a request and returns a successful response body as text. pub(crate) async fn text(&self, method: Method, path: &str) -> anyhow::Result { - let response = self - .request(method, path)? - .send() - .await - .map_err(|error| self.transport_error(error))?; - let status = response.status(); - if !status.is_success() { - let body = response.text().await.unwrap_or_default(); - return Err(self.status_error(path, status, &body)); - } - let body = read_capped(response, path).await?; - String::from_utf8(body).context("memory API response was not valid UTF-8") + self.with_read_retry(|| async { + let response = self + .request(method.clone(), path)? + .send() + .await + .map_err(|error| self.transport_error(error))?; + let status = response.status(); + if !status.is_success() { + let body = response.text().await.unwrap_or_default(); + return Err(self.status_error(path, status, &body)); + } + let body = read_capped(response, path).await?; + String::from_utf8(body).context("memory API response was not valid UTF-8") + }) + .await } /// Sends a request whose successful response body is not needed. @@ -324,15 +415,22 @@ impl HttpClient { self.request(method, path) } - /// Reports whether a GET endpoint responds successfully. - pub(crate) async fn healthy(&self, path: &str) -> bool { - let Ok(request) = self.request(Method::GET, path) else { - return false; - }; - request + /// Probes a GET endpoint and reports WHY it failed, typed (issue #18 + /// follow-up U4). The boolean `healthy` below discards status, body and + /// transport class; this keeps them, so a health surface can distinguish + /// "credential rejected" from "unreachable" from "answered 500". + pub(crate) async fn probe(&self, path: &str) -> anyhow::Result<()> { + let response = self + .request(Method::GET, path)? .send() .await - .is_ok_and(|response| response.status().is_success()) + .map_err(|error| self.transport_error(error))?; + let status = response.status(); + if !status.is_success() { + let body = response.text().await.unwrap_or_default(); + return Err(self.status_error(path, status, &body)); + } + Ok(()) } } @@ -444,8 +542,9 @@ pub(crate) trait Dialect: Send + Sync + std::fmt::Debug { ) -> anyhow::Result>; /// Deletes one exact logical record and reports whether it existed. async fn delete(&self, namespace: &str, key: &str) -> anyhow::Result; - /// Checks whether the backend is available for requests. - async fn health(&self) -> bool; + /// Probes whether the backend is available, reporting WHY not, typed + /// (`Ok(())` = serving; the error carries a §A4 `MemoryError` payload). + async fn health(&self) -> anyhow::Result<()>; } #[derive(Debug)] @@ -455,6 +554,12 @@ pub(crate) struct RemoteMemory { } impl RemoteMemory { + /// Mutable access for the adapters' builder-style configuration + /// (`with_request_timeout` on each public type). + pub(crate) fn dialect_mut(&mut self) -> &mut D { + &mut self.dialect + } + /// Wraps a backend dialect with shared filtering and conversion behavior. pub(crate) fn new(dialect: D) -> Self { Self { dialect } @@ -522,7 +627,7 @@ impl Memory for RemoteMemory { let mut entries = self.dialect.search(query, limit, opts.clone()).await?; entries.retain(|entry| matches_filters(entry, &opts)); if let Some(minimum) = min_score { - entries.retain(|entry| entry.score.is_none_or(|score| score >= minimum)); + entries.retain(|entry| clears_min_score(entry.score, minimum)); } entries.truncate(limit); Ok(entries @@ -598,10 +703,37 @@ impl Memory for RemoteMemory { /// Delegates availability checking to the backend dialect. async fn health_check(&self) -> bool { - self.dialect.health().await + self.dialect.health().await.is_ok() + } + + /// The typed answer behind `health_check` (issue #18 §U4): the probe's + /// §A4 class decides the health state. `Unavailable` — the backend + /// answered that it cannot serve right now (429 / gateway trio) — maps to + /// `Degraded`: alive, impaired, worth saying so instead of "down". + /// Everything else that fails maps to `Down` with the probe's own reason + /// (which names host and class, never a credential). + async fn health_probe(&self) -> Option { + use tinymemory_api::health::MemoryHealth; + Some(match self.dialect.health().await { + Ok(()) => MemoryHealth::Ready, + Err(error) => match error.downcast::() { + Ok(MemoryError::Unavailable(reason)) => MemoryHealth::degraded(reason), + Ok(typed) => MemoryHealth::down(typed.to_string()), + Err(opaque) => MemoryHealth::down(opaque.to_string()), + }, + }) } } +/// Honesty over leniency (issue #18 §U6): an entry with NO score cannot be +/// shown to clear a threshold the caller asked for, so it drops. The old +/// `is_none_or` let unscored hits pass, which made `min_score` silently inert +/// against any backend that omits score numbers — a caller asking for ≥0.8 +/// got unranked everything and never learned the filter did nothing. +fn clears_min_score(score: Option, minimum: f64) -> bool { + score.is_some_and(|value| value >= minimum) +} + /// Applies TinyMemory recall filters that a backend may not support natively. fn matches_filters(entry: &StoredEntry, opts: &RecallOpts<'_>) -> bool { opts.namespace.is_none_or(|value| entry.namespace == value) @@ -614,6 +746,39 @@ fn matches_filters(entry: &StoredEntry, opts: &RecallOpts<'_>) -> bool { .is_none_or(|value| entry.session_id.as_deref() == Some(value)) } +/// The class of a transport failure — a real enum rather than a prose string +/// (issue #18 §A4), so [`HttpClient::transport_error`] can mint a typed +/// [`MemoryError`] and a retry policy can key on the class instead of +/// substring-matching a rendered message. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum TransportClass { + Timeout, + Dns, + Tls, + Connect, + /// The request did not complete for a reason the chain does not name — + /// deliberately NOT mapped onto a typed variant, because claiming + /// "unreachable" for (say) a mid-body disconnect would be a guess. + Other, +} + +impl TransportClass { + /// The operator-facing prose for this class — exactly the strings the + /// pre-§A4 version returned, so log lines do not change spelling. + fn describe(self) -> &'static str { + match self { + Self::Timeout => "timed out", + Self::Dns => "the host could not be resolved — check the URL", + Self::Tls => { + "TLS failed — the endpoint answered on the port but could not establish a \ + secure connection; check that the URL is the engine's real API host" + } + Self::Connect => "could not connect — check the URL and that the service is reachable", + Self::Other => "the request did not complete", + } + } +} + /// Name the class of a transport failure from what the error chain says. /// /// Pure so the ORDER is testable, which is the whole reason it exists as its @@ -621,15 +786,15 @@ fn matches_filters(entry: &StoredEntry, opts: &RecallOpts<'_>) -> bool { /// naive `if is_connect()` first collapses every class into "could not /// connect". That is exactly what the first version of this did, and it took /// a live run against a real broken endpoint to notice. -fn classify_transport(is_timeout: bool, is_connect: bool, chain: &str) -> &'static str { +fn classify_transport(is_timeout: bool, is_connect: bool, chain: &str) -> TransportClass { let lower = chain.to_ascii_lowercase(); if is_timeout { - "timed out" + TransportClass::Timeout } else if lower.contains("dns") || lower.contains("name or service") || lower.contains("failed to lookup") { - "the host could not be resolved — check the URL" + TransportClass::Dns } else if lower.contains("tls") || lower.contains("handshake") || lower.contains("certificate") @@ -637,12 +802,11 @@ fn classify_transport(is_timeout: bool, is_connect: bool, chain: &str) -> &'stat || lower.contains("invalid peer") || lower.contains("unknown issuer") { - "TLS failed — the endpoint answered on the port but could not establish a \ - secure connection; check that the URL is the engine's real API host" + TransportClass::Tls } else if is_connect { - "could not connect — check the URL and that the service is reachable" + TransportClass::Connect } else { - "the request did not complete" + TransportClass::Other } } @@ -714,7 +878,7 @@ mod credential_header_tests { #[cfg(test)] mod transport_tests { - use super::classify_transport; + use super::{classify_transport, TransportClass}; /// The verbatim chain a rustls handshake abort produces. Cognee's hosted /// endpoint answered TCP and then sent this; `reqwest` reports it as a @@ -728,7 +892,8 @@ mod transport_tests { true, // reqwest really does set is_connect for this "client error (Connect): received fatal alert: InternalError", ); - assert!(class.starts_with("TLS failed"), "got: {class}"); + assert_eq!(class, TransportClass::Tls); + assert!(class.describe().starts_with("TLS failed")); } /// DNS failures are also CONNECT errors; the specific class must win. @@ -739,7 +904,8 @@ mod transport_tests { true, "client error (Connect): dns error: failed to lookup address information", ); - assert!(class.contains("could not be resolved"), "got: {class}"); + assert_eq!(class, TransportClass::Dns); + assert!(class.describe().contains("could not be resolved")); } #[test] @@ -749,7 +915,8 @@ mod transport_tests { true, "client error (Connect): tcp connect error: Connection refused (os error 61)", ); - assert!(class.starts_with("could not connect"), "got: {class}"); + assert_eq!(class, TransportClass::Connect); + assert!(class.describe().starts_with("could not connect")); } /// A timeout outranks everything: it is the one class reqwest states @@ -757,12 +924,64 @@ mod transport_tests { #[test] fn a_timeout_wins_over_every_chain_hint() { let class = classify_transport(true, true, "dns error: something tls certificate"); - assert_eq!(class, "timed out"); + assert_eq!(class, TransportClass::Timeout); + assert_eq!(class.describe(), "timed out"); } #[test] fn an_unrecognised_chain_degrades_without_claiming_a_cause() { let class = classify_transport(false, false, "body error: incomplete message"); - assert_eq!(class, "the request did not complete"); + assert_eq!(class, TransportClass::Other); + assert_eq!(class.describe(), "the request did not complete"); + } + + /// §A4: the typed payload rides the anyhow error and downcasts back out — + /// the property `engine_error` relies on at the contract boundary. + #[test] + fn typed_variants_survive_the_anyhow_round_trip() { + use tinymemory_api::error::MemoryError; + let carried = anyhow::Error::new(MemoryError::Unauthorized("key rejected".into())); + match carried.downcast::() { + Ok(MemoryError::Unauthorized(msg)) => assert_eq!(msg, "key rejected"), + other => panic!("lost the typed payload: {other:?}"), + } + } + + /// §U6: a threshold means a threshold. An unscored hit does not clear + /// one, an exactly-equal score does, and no-threshold callers see the + /// old behavior untouched (the filter never runs). + #[test] + fn min_score_is_honest_about_unscored_hits() { + use super::clears_min_score; + assert!(!clears_min_score(None, 0.1)); + assert!(clears_min_score(Some(0.8), 0.8)); + assert!(!clears_min_score(Some(0.79), 0.8)); + } + + /// The retry gate keys on the §A4 class, never the prose: transient + /// classes retry, deterministic answers do not. + #[test] + fn retry_gate_is_typed_and_conservative() { + use super::HttpClient; + use tinymemory_api::error::MemoryError; + let transient = [ + MemoryError::Timeout("t".into()), + MemoryError::Unreachable("u".into()), + MemoryError::Unavailable("503".into()), + ]; + for error in transient { + assert!(HttpClient::retryable(&anyhow::Error::new(error))); + } + let settled = [ + MemoryError::Unauthorized("401".into()), + MemoryError::Invalid("bad".into()), + MemoryError::NotFound("gone".into()), + MemoryError::Backend("500".into()), + ]; + for error in settled { + assert!(!HttpClient::retryable(&anyhow::Error::new(error))); + } + // Opaque errors never retry: without a class, a retry is a guess. + assert!(!HttpClient::retryable(&anyhow::anyhow!("mystery"))); } } diff --git a/adapters/remote/src/mem0.rs b/adapters/remote/src/mem0.rs index e61dacd2..61529456 100644 --- a/adapters/remote/src/mem0.rs +++ b/adapters/remote/src/mem0.rs @@ -68,6 +68,26 @@ pub struct Mem0Memory { } impl Mem0Memory { + /// Rebuilds the HTTP transport with a different per-request deadline + /// (issue #18 follow-up U5). The default is 60s with a 10s connect + /// deadline — right for interactive calls; a bulk migration or a tight + /// liveness probe may want its own budget. + /// + /// # Errors + /// + /// Fails only if the underlying HTTP client cannot be rebuilt — a + /// configuration-time failure, before any request is made. + pub fn with_request_timeout(mut self, timeout: std::time::Duration) -> anyhow::Result { + let client = self + .inner + .dialect_mut() + .client + .clone() + .with_timeout(timeout)?; + self.inner.dialect_mut().client = client; + Ok(self) + } + /// Connect to a Mem0 REST server. /// /// `api_key` is sent as `X-API-Key`. Pass `None` only when the server is @@ -518,9 +538,19 @@ impl Dialect for Mem0Dialect { Ok(true) } - /// Accepts either Mem0's health endpoint or its redirected root page. - async fn health(&self) -> bool { - self.client.healthy("api/health").await || self.client.healthy("").await + /// Probes Mem0's health endpoint, falling back to its redirected root + /// page — and unlike the old boolean `||`, a double failure reports BOTH + /// answers, so "the health route 404s but the root is throttled" stops + /// reading as a bare false. + async fn health(&self) -> anyhow::Result<()> { + let primary = match self.client.probe("api/health").await { + Ok(()) => return Ok(()), + Err(error) => error, + }; + self.client + .probe("") + .await + .map_err(|root| root.context(format!("health endpoint also failed: {primary}"))) } } diff --git a/adapters/remote/src/supermemory.rs b/adapters/remote/src/supermemory.rs index 633fa76e..28c4111f 100644 --- a/adapters/remote/src/supermemory.rs +++ b/adapters/remote/src/supermemory.rs @@ -22,6 +22,26 @@ pub struct SupermemoryMemory { } impl SupermemoryMemory { + /// Rebuilds the HTTP transport with a different per-request deadline + /// (issue #18 follow-up U5). The default is 60s with a 10s connect + /// deadline — right for interactive calls; a bulk migration or a tight + /// liveness probe may want its own budget. + /// + /// # Errors + /// + /// Fails only if the underlying HTTP client cannot be rebuilt — a + /// configuration-time failure, before any request is made. + pub fn with_request_timeout(mut self, timeout: std::time::Duration) -> anyhow::Result { + let client = self + .inner + .dialect_mut() + .client + .clone() + .with_timeout(timeout)?; + self.inner.dialect_mut().client = client; + Ok(self) + } + /// Connect to a Supermemory server using its bearer API key. /// /// # Errors @@ -398,9 +418,14 @@ impl Dialect for SupermemoryDialect { Ok(true) } - /// Checks the local server root, its stable availability endpoint. - async fn health(&self) -> bool { - self.client.healthy("").await + /// Probes `v3/container-tags/list` — the cheapest endpoint that proves + /// BOTH the credential and the data plane (issue #18 §U4). The old + /// root-page probe answered 200 to an unauthenticated client against a + /// live server, so a wrong key looked healthy until the first real call. + async fn health(&self) -> anyhow::Result<()> { + // Status-only: a 200 from this authenticated route is the proof; the + // body's shape is the data path's concern, not the probe's. + self.client.probe("v3/container-tags/list").await } } diff --git a/adapters/remote/src/supermemory_test.rs b/adapters/remote/src/supermemory_test.rs index d3d37ac6..5fdf914d 100644 --- a/adapters/remote/src/supermemory_test.rs +++ b/adapters/remote/src/supermemory_test.rs @@ -108,8 +108,11 @@ async fn capture_auth(State(state): State>>, headers: HeaderMap #[tokio::test] async fn supermemory_supports_provided_and_self_hosted_apis() { let captured = Arc::new(Mutex::new(Value::Null)); + // The health probe now proves auth + data plane via the container-tags + // list (§U4), so that is where the capture sits — a bare root `/` no + // longer receives the probe. let app = Router::new() - .route("/", get(capture_auth)) + .route("/v3/container-tags/list", get(capture_auth)) .with_state(captured.clone()); let listener = tokio::net::TcpListener::bind("127.0.0.1:0") .await diff --git a/vendor/tinycortex b/vendor/tinycortex index 8401346b..34cbb6cf 160000 --- a/vendor/tinycortex +++ b/vendor/tinycortex @@ -1 +1 @@ -Subproject commit 8401346b574cacb1dc0cf6b36bc608ff5ef9f6f5 +Subproject commit 34cbb6cfa91ea74d62605bd57790782b0c748556 From 755d161189f53d737f09596d3844d2bf2cac8451 Mon Sep 17 00:00:00 2001 From: Shanu Date: Thu, 20 Aug 2026 22:05:16 +0530 Subject: [PATCH 3/5] test: the transport test mod carries the panic/expect allowance Its sibling modules already do; CI's -D warnings run is where the omission showed. --- adapters/remote/src/common.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/adapters/remote/src/common.rs b/adapters/remote/src/common.rs index 43c07464..e6f52e36 100644 --- a/adapters/remote/src/common.rs +++ b/adapters/remote/src/common.rs @@ -878,6 +878,8 @@ mod credential_header_tests { #[cfg(test)] mod transport_tests { + #![allow(clippy::expect_used, clippy::panic)] + use super::{classify_transport, TransportClass}; /// The verbatim chain a rustls handshake abort produces. Cognee's hosted From 6a1b9f6f8537d1df5837cfb125801bc18fd564e6 Mon Sep 17 00:00:00 2001 From: Shanu Date: Thu, 20 Aug 2026 22:51:32 +0530 Subject: [PATCH 4/5] review: deep health actually reaches the public types, and four more rights MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #68 review's five majors. (1) The three public adapters hand- delegate Memory method-by-method, so the defaulted health_probe shadowed RemoteMemory's typed impl and §U4 was unreachable — Degraded never constructed, a rejected credential byte-identical to a throttle. Forwarded on all three, asserted through the PUBLIC types: 401 is Down naming the credential class, 503 is Degraded. (2) min_score was a regression for exactly the score-less backends it named: the supermemory decode now accepts both wire spellings ("similarity" and the conformance double's "score"), Cognee declares scores_recall() = false and keeps its hits (documented-inert beats dropping 100% of every thresholded result — the over-fetch that discarded everything it pulled is reverted), and both semantics are pinned against each double's own response shape. (3) The vendor/tinycortex gitlink is restored to main's pin — the moved value was a swept local submodule state pointing at a deleted pre-merge branch head; not intended, not mentioned, gone. (4) The read/write retry split becomes a per-call statement: every json/text call site passes an Attempts marker, the compiler forces the choice on the next caller, and a route-scoped counter test pins 3 attempts for a transient read against exactly 1 for a transient write. (5) 400/422 map to Invalid, so a real validating backend can produce the class the tightened conformance assertion demands; typed 400 test across all three adapters. Minors: health reasons are redacted before they reach the standing status surface (each chain segment truncates at the detail separator; the caller-facing error keeps the full body) — walking the chain also preserves Mem0's both-probes-failed context that a consuming downcast dropped; the timeout docs state the ~3x retry ceiling. Co-Authored-By: Claude Fable 5 --- Cargo.lock | 8 -- adapters/remote/src/cognee.rs | 40 +++--- adapters/remote/src/cognee_test.rs | 21 ++++ adapters/remote/src/common.rs | 160 +++++++++++++++++++----- adapters/remote/src/failure_test.rs | 159 +++++++++++++++++++++++ adapters/remote/src/mem0.rs | 19 ++- adapters/remote/src/supermemory.rs | 35 +++++- adapters/remote/src/supermemory_test.rs | 34 +++++ vendor/tinycortex | 2 +- 9 files changed, 420 insertions(+), 58 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 84706073..b8a40a68 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1869,15 +1869,7 @@ dependencies = [ name = "tinycortex-api" version = "0.1.1" dependencies = [ - "anyhow", - "async-trait", - "chrono", - "serde", - "serde_json", - "sha2 0.10.9", - "thiserror 2.0.20", "tinymemory-api", - "uuid", ] [[package]] diff --git a/adapters/remote/src/cognee.rs b/adapters/remote/src/cognee.rs index 1e385658..f91f6b33 100644 --- a/adapters/remote/src/cognee.rs +++ b/adapters/remote/src/cognee.rs @@ -8,7 +8,7 @@ use tinymemory_api::recall::RecallOpts; use tinymemory_api::traits::Memory; use tinymemory_api::types::MemoryTaint; -use crate::common::{stable_id, Dialect, HttpClient, RemoteMemory, StoredEntry}; +use crate::common::{stable_id, Attempts, Dialect, HttpClient, RemoteMemory, StoredEntry}; /// Stable driver id used by configuration and status output. pub use tinymemory_api::drivers::COGNEE_DRIVER_ID; @@ -172,6 +172,13 @@ impl Memory for CogneeMemory { async fn health_check(&self) -> bool { self.inner.health_check().await } + /// Forwarded explicitly: this wrapper delegates method-by-method, so the + /// defaulted `None` would otherwise shadow `RemoteMemory`'s typed probe — + /// which is exactly what the first cut shipped, making §U4's deep health + /// unreachable through every public type (the #68 review's Major 1). + async fn health_probe(&self) -> Option { + self.inner.health_probe().await + } } #[derive(Debug)] @@ -201,7 +208,12 @@ impl CogneeDialect { async fn datasets(&self) -> anyhow::Result> { let response: Value = self .client - .json(Method::GET, "api/v1/datasets/", None) + .json( + Method::GET, + "api/v1/datasets/", + None, + Attempts::RetryTransient, + ) .await?; Ok(response .as_array() @@ -225,6 +237,7 @@ impl CogneeDialect { Method::GET, &format!("api/v1/datasets/{}/data", dataset.id), None, + Attempts::RetryTransient, ) .await?; let mut entries = Vec::new(); @@ -245,6 +258,7 @@ impl CogneeDialect { .text( Method::GET, &format!("api/v1/datasets/{}/data/{id}/raw", dataset.id), + Attempts::RetryTransient, ) .await?; let mut entry: StoredEntry = @@ -363,17 +377,6 @@ impl Dialect for CogneeDialect { opts: RecallOpts<'_>, ) -> anyhow::Result> { let datasets = opts.namespace.map(Self::dataset_name); - // Cognee's recall API takes no score threshold, so `min_score` is - // enforced client-side by the shared pass in `common.rs` (issue #18 - // §U6). Over-fetch when a threshold is set — with `top_k == limit`, - // every hit the filter drops is a slot the caller asked for and - // cannot be backfilled. Capped: an aggressive threshold is not a - // license to pull the whole store. - let top_k = if opts.min_score.is_some() { - limit.saturating_mul(3).min(limit.saturating_add(50)) - } else { - limit - }; let response: Value = self .client .json( @@ -383,10 +386,11 @@ impl Dialect for CogneeDialect { "query": query, "search_type": "CHUNKS", "datasets": datasets.map(|name| vec![name]), - "top_k": top_k, + "top_k": limit, "only_context": true, "session_id": opts.session_id })), + Attempts::RetryTransient, ) .await?; let mut entries = Vec::new(); @@ -437,6 +441,14 @@ impl Dialect for CogneeDialect { async fn health(&self) -> anyhow::Result<()> { self.client.probe("health").await } + + /// Context-only recall carries no score field — see the trait doc for + /// what this means for `min_score` (documented-inert, not everything- + /// dropping; the first cut's over-fetch pulled 3x the data and discarded + /// all of it). + fn scores_recall(&self) -> bool { + false + } } #[cfg(test)] diff --git a/adapters/remote/src/cognee_test.rs b/adapters/remote/src/cognee_test.rs index ad803781..5afcc3e4 100644 --- a/adapters/remote/src/cognee_test.rs +++ b/adapters/remote/src/cognee_test.rs @@ -207,6 +207,27 @@ async fn native_cognee_round_trips_the_tinymemory_contract() { .len(), 1 ); + // #68 review Major 2: Cognee's context-only recall is scoreless, so the + // strict filter would have dropped 100% of every thresholded result. + // The dialect declares scores_recall() = false and min_score is + // documented-inert: the hit survives. + assert_eq!( + driver + .recall( + "graph", + 3, + &OwnedRecallOpts { + namespace: Some("project".into()), + min_score: Some(0.5), + ..OwnedRecallOpts::default() + }, + None + ) + .await + .expect("recall with a threshold the backend cannot score") + .len(), + 1 + ); assert!(driver.forget("project", "key").await.expect("forget")); assert!(!driver.forget("project", "key").await.expect("forget again")); assert!(driver.health().await.is_usable()); diff --git a/adapters/remote/src/common.rs b/adapters/remote/src/common.rs index e6f52e36..f9ed7d12 100644 --- a/adapters/remote/src/common.rs +++ b/adapters/remote/src/common.rs @@ -120,6 +120,29 @@ fn credential_header(value: &str) -> anyhow::Result { Ok(header) } +/// The caller's statement of a request's idempotence — every `json`/`text` +/// call site must choose, which is what makes the read/write retry split +/// CHECKABLE instead of conventional (#68 review, Major 4: the first cut's +/// split lived only in a comment, and wrapping the write helper in the retry +/// path failed nothing). +/// +/// `RetryTransient` is only sound when repeating the request cannot double- +/// apply anything: reads, searches, list walks — POST included when the POST +/// is a query. `Once` is for everything whose repetition has a cost. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum Attempts { + /// Retry up to three times on the typed transient classes. + RetryTransient, + /// One attempt, whatever the failure. + // + // No current caller: the audit behind #68 found every existing + // `json`/`text` call is a read, which is exactly why the marker exists — + // the FIRST write-shaped caller must pick this variant instead of + // silently inheriting retry. Deliberately present before its first use. + #[allow(dead_code)] + Once, +} + impl HttpClient { /// Builds a client that optionally authenticates with a bearer token. pub(crate) fn bearer(endpoint: &str, credential: Option<&str>) -> anyhow::Result { @@ -176,6 +199,10 @@ impl HttpClient { /// Rebuilds this client with a different per-request deadline (issue #18 /// follow-up U5). The 60s default suits interactive calls; a bulk /// migration or a health probe may want its own budget. + /// + /// Note the effective worst-case wall time of a retrying READ is ~3x + /// this value plus 750ms of backoff — the transient-retry policy runs up + /// to three attempts, each with its own deadline. pub(crate) fn with_timeout(mut self, timeout: std::time::Duration) -> anyhow::Result { self.inner = Self::build_inner(timeout)?; Ok(self) @@ -286,6 +313,15 @@ impl HttpClient { 404 => MemoryError::NotFound(format!( "memory API {path} on {host} returned HTTP 404{detail}" )), + // A validation refusal: the backend understood the request and + // rejected its CONTENT. Without this arm a real validating + // backend (all three vendors validate; only the in-tree doubles + // accept everything) could never produce the `Invalid` the + // tightened conformance refusal-assertion demands (#68 review, + // Major 5). + 400 | 422 => MemoryError::Invalid(format!( + "memory API {path} on {host} returned HTTP {status}{detail}" + )), // The answered-but-cannot-serve class: rate limiting and the // gateway trio. Distinct from `Backend` so a retry policy can key // on it without parsing prose. @@ -346,31 +382,49 @@ impl HttpClient { method: Method, path: &str, body: Option<&serde_json::Value>, + attempts: Attempts, ) -> anyhow::Result { - self.with_read_retry(|| async { - let mut request = self.request(method.clone(), path)?; - if let Some(body) = body { - request = request.json(body); - } - let response = request - .send() - .await - .map_err(|error| self.transport_error(error))?; - let status = response.status(); - if !status.is_success() { - let body = response.text().await.unwrap_or_default(); - return Err(self.status_error(path, status, &body)); - } - let body = read_capped(response, path).await?; - serde_json::from_slice(&body) - .with_context(|| format!("memory API {path} returned invalid JSON")) - }) - .await + if matches!(attempts, Attempts::Once) { + return self.json_attempt(method, path, body).await; + } + self.with_read_retry(|| self.json_attempt(method.clone(), path, body)) + .await + } + + /// One send of a JSON request — the body `json` retries (or not, per its + /// `Attempts` marker). + async fn json_attempt( + &self, + method: Method, + path: &str, + body: Option<&serde_json::Value>, + ) -> anyhow::Result { + let mut request = self.request(method, path)?; + if let Some(body) = body { + request = request.json(body); + } + let response = request + .send() + .await + .map_err(|error| self.transport_error(error))?; + let status = response.status(); + if !status.is_success() { + let body = response.text().await.unwrap_or_default(); + return Err(self.status_error(path, status, &body)); + } + let body = read_capped(response, path).await?; + serde_json::from_slice(&body) + .with_context(|| format!("memory API {path} returned invalid JSON")) } /// Sends a request and returns a successful response body as text. - pub(crate) async fn text(&self, method: Method, path: &str) -> anyhow::Result { - self.with_read_retry(|| async { + pub(crate) async fn text( + &self, + method: Method, + path: &str, + attempts: Attempts, + ) -> anyhow::Result { + let attempt = || async { let response = self .request(method.clone(), path)? .send() @@ -383,8 +437,11 @@ impl HttpClient { } let body = read_capped(response, path).await?; String::from_utf8(body).context("memory API response was not valid UTF-8") - }) - .await + }; + if matches!(attempts, Attempts::Once) { + return attempt().await; + } + self.with_read_retry(attempt).await } /// Sends a request whose successful response body is not needed. @@ -545,6 +602,18 @@ pub(crate) trait Dialect: Send + Sync + std::fmt::Debug { /// Probes whether the backend is available, reporting WHY not, typed /// (`Ok(())` = serving; the error carries a §A4 `MemoryError` payload). async fn health(&self) -> anyhow::Result<()>; + /// Whether this backend's recall responses carry a similarity score. + /// + /// Decides the `min_score` tier in [`RemoteMemory::recall`]: a scoring + /// backend gets the strict filter (an unscored hit cannot clear a + /// threshold), while a backend that STRUCTURALLY cannot score — Cognee's + /// context-only recall has no score field at all — keeps its hits, because + /// dropping 100% of every result is not honesty, it is a different lie + /// (the #68 review's Major 2). The inertness on scoreless backends is + /// deliberate and documented rather than silent: this flag is where. + fn scores_recall(&self) -> bool { + true + } } #[derive(Debug)] @@ -627,7 +696,12 @@ impl Memory for RemoteMemory { let mut entries = self.dialect.search(query, limit, opts.clone()).await?; entries.retain(|entry| matches_filters(entry, &opts)); if let Some(minimum) = min_score { - entries.retain(|entry| clears_min_score(entry.score, minimum)); + if self.dialect.scores_recall() { + entries.retain(|entry| clears_min_score(entry.score, minimum)); + } + // else: the backend cannot score (see `Dialect::scores_recall`) — + // the threshold is documented-inert rather than silently + // everything-dropping. } entries.truncate(limit); Ok(entries @@ -716,15 +790,43 @@ impl Memory for RemoteMemory { use tinymemory_api::health::MemoryHealth; Some(match self.dialect.health().await { Ok(()) => MemoryHealth::Ready, - Err(error) => match error.downcast::() { - Ok(MemoryError::Unavailable(reason)) => MemoryHealth::degraded(reason), - Ok(typed) => MemoryHealth::down(typed.to_string()), - Err(opaque) => MemoryHealth::down(opaque.to_string()), - }, + Err(error) => { + let reason = health_reason(&error); + match error.downcast_ref::() { + Some(MemoryError::Unavailable(_)) => MemoryHealth::degraded(reason), + _ => MemoryHealth::down(reason), + } + } }) } } +/// A health `reason` from a probe failure, REDACTED for the status surface. +/// +/// `MemoryHealth`'s contract: the reason is logged and rendered in operator +/// status and must never carry credentials or content. `status_error` +/// interpolates up to 300 chars of the backend's OWN error body — which a +/// vendor is free to fill with the rejected key (#68 review). Every message +/// this crate builds puts that detail after a spaced em-dash, so the reason +/// keeps each chain segment's head and drops the tails. The full untruncated +/// error still flows to the CALLER of the failing operation; only the +/// standing status string is trimmed. Walking `chain()` (not just the top) +/// keeps Mem0's both-probes-failed context instead of losing it to a +/// consuming downcast. +fn health_reason(error: &anyhow::Error) -> String { + error + .chain() + .map(|cause| { + let text = cause.to_string(); + match text.split_once(" — ") { + Some((head, _)) => format!("{head} — detail withheld from status; see logs"), + None => text, + } + }) + .collect::>() + .join("; ") +} + /// Honesty over leniency (issue #18 §U6): an entry with NO score cannot be /// shown to clear a threshold the caller asked for, so it drops. The old /// `is_none_or` let unscored hits pass, which made `min_score` silently inert diff --git a/adapters/remote/src/failure_test.rs b/adapters/remote/src/failure_test.rs index 87711b4c..ed524a4c 100644 --- a/adapters/remote/src/failure_test.rs +++ b/adapters/remote/src/failure_test.rs @@ -51,6 +51,26 @@ async fn failing(status: StatusCode) -> String { serve(Router::new().fallback(any(move || async move { status }))).await } +/// A backend that fails every route with `status` and a body carrying a +/// fake secret, counting requests — for the redaction and retry-count +/// assertions. +async fn failing_with_body( + status: StatusCode, + body: &'static str, +) -> (String, std::sync::Arc) { + let hits = std::sync::Arc::new(std::sync::atomic::AtomicU32::new(0)); + let counter = hits.clone(); + let endpoint = serve(Router::new().fallback(any(move || { + let counter = counter.clone(); + async move { + counter.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + (status, body) + } + }))) + .await; + (endpoint, hits) +} + /// A backend that answers every route with `200 OK` and a body that is not the /// JSON the adapter expects. /// @@ -290,3 +310,142 @@ async fn a_paginated_export_terminates_instead_of_looping() { ); } } + +/// #68 review Major 1: deep health must be reachable through the PUBLIC +/// adapter types — the first cut implemented it on the inner composition and +/// every hand-delegating wrapper shadowed it with the trait default's `None`. +/// A 401 is `Down` naming the credential class; a 503 is `Degraded` (answered, +/// cannot serve). And per the review's redaction minor: the backend's error +/// body — which a vendor is free to fill with the rejected key — must NOT +/// reach the standing status reason. +#[tokio::test] +async fn public_adapters_probe_typed_health_with_redacted_reasons() { + let (unauthorized, _) = failing_with_body( + StatusCode::UNAUTHORIZED, + r#"{"detail":"bad key sk-SECRET123"}"#, + ) + .await; + for (name, memory) in adapters(&unauthorized) { + let health = memory + .health_probe() + .await + .unwrap_or_else(|| panic!("{name}: the public type must forward health_probe")); + assert_eq!(health.as_str(), "down", "{name}: a 401 is Down"); + let reason = health.reason().unwrap_or_default(); + assert!( + reason.contains("credential"), + "{name}: the reason names the class: {reason}" + ); + assert!( + !reason.contains("sk-SECRET123"), + "{name}: the backend's body must not reach the status surface: {reason}" + ); + } + + let (throttled, _) = failing_with_body(StatusCode::SERVICE_UNAVAILABLE, "busy").await; + for (name, memory) in adapters(&throttled) { + let health = memory + .health_probe() + .await + .unwrap_or_else(|| panic!("{name}: the public type must forward health_probe")); + assert_eq!( + health.as_str(), + "degraded", + "{name}: answered-but-cannot-serve is Degraded, not Down" + ); + } +} + +/// #68 review Major 5: a backend-side validation refusal (HTTP 400) must +/// arrive as `Invalid` — the class the tightened conformance refusal +/// assertion demands — never as `Backend`. +#[tokio::test] +async fn a_400_refusal_is_invalid_not_backend() { + let (endpoint, _) = failing_with_body( + StatusCode::BAD_REQUEST, + r#"{"error":"content must not be empty"}"#, + ) + .await; + for (name, memory) in adapters(&endpoint) { + let error = memory + .store_with_taint( + "ns", + "k", + "content", + MemoryCategory::Core, + None, + MemoryTaint::Internal, + ) + .await + .expect_err("a 400 must surface"); + assert!( + matches!( + error.downcast_ref::(), + Some(MemoryError::Invalid(_)) + ), + "{name}: a 400 must be Invalid, got: {error}" + ); + } +} + +/// #68 review Major 4: the retry split is now a per-call statement. A 503 on +/// a retrying READ is attempted three times; the same 503 on a WRITE path +/// (`empty` — no marker, no retry machinery at all) is attempted once. The +/// counter is the proof, not a comment. +#[tokio::test] +async fn transient_failures_retry_reads_three_times_and_writes_once() { + // Reads: every route 503s; the read path retries to its cap. + let (endpoint, hits) = failing_with_body(StatusCode::SERVICE_UNAVAILABLE, "busy").await; + let memory = SupermemoryMemory::api(&endpoint, "key").expect("client"); + let _ = memory.list(None, None, None).await; + assert_eq!( + hits.load(std::sync::atomic::Ordering::SeqCst), + 3, + "a transient read failure retries to the cap" + ); + + // Writes: the LIST half of upsert succeeds (empty page — nothing to + // update), so the create POST is the only thing that can fail. It must + // reach the backend exactly once: the write path has no retry machinery + // at all, and this counter — not a comment — is what pins the split + // (#68 review, Major 4). + let writes = std::sync::Arc::new(std::sync::atomic::AtomicU32::new(0)); + let counter = writes.clone(); + let app = Router::new() + .route( + "/v4/memories/list", + axum::routing::post(|| async { axum::Json(serde_json::json!({"memories": []})) }), + ) + .fallback(any(move || { + let counter = counter.clone(); + async move { + counter.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + StatusCode::SERVICE_UNAVAILABLE + } + })); + let write_endpoint = serve(app).await; + let memory = SupermemoryMemory::api(&write_endpoint, "key").expect("client"); + let error = memory + .store_with_taint( + "ns", + "k", + "v", + MemoryCategory::Core, + None, + MemoryTaint::Internal, + ) + .await + .expect_err("the 503 write must surface"); + assert!( + matches!( + error.downcast_ref::(), + Some(MemoryError::Unavailable(_)) + ), + "and typed: {error}" + ); + assert_eq!( + writes.load(std::sync::atomic::Ordering::SeqCst), + 1, + "a transient WRITE failure is attempted exactly once" + ); +} diff --git a/adapters/remote/src/mem0.rs b/adapters/remote/src/mem0.rs index 61529456..8cfc839f 100644 --- a/adapters/remote/src/mem0.rs +++ b/adapters/remote/src/mem0.rs @@ -13,7 +13,7 @@ use tinymemory_api::recall::RecallOpts; use tinymemory_api::traits::Memory; use tinymemory_api::types::MemoryTaint; -use crate::common::{category, Dialect, HttpClient, RemoteMemory, StoredEntry}; +use crate::common::{category, Attempts, Dialect, HttpClient, RemoteMemory, StoredEntry}; /// Stable driver id used by configuration and status output. pub use tinymemory_api::drivers::MEM0_DRIVER_ID; @@ -219,6 +219,13 @@ impl Memory for Mem0Memory { async fn health_check(&self) -> bool { self.inner.health_check().await } + /// Forwarded explicitly: this wrapper delegates method-by-method, so the + /// defaulted `None` would otherwise shadow `RemoteMemory`'s typed probe — + /// which is exactly what the first cut shipped, making §U4's deep health + /// unreachable through every public type (the #68 review's Major 1). + async fn health_probe(&self) -> Option { + self.inner.health_probe().await + } } #[derive(Debug)] @@ -262,7 +269,12 @@ impl Mem0Dialect { let top_k = Self::LISTING_TOP_K; let response: Value = self .client - .json(Method::GET, &format!("memories?top_k={top_k}"), None) + .json( + Method::GET, + &format!("memories?top_k={top_k}"), + None, + Attempts::RetryTransient, + ) .await?; let results = response .get("results") @@ -302,6 +314,7 @@ impl Mem0Dialect { Method::POST, &format!("v3/memories/?page={page}&page_size={CLOUD_PAGE_SIZE}"), Some(&json!({"filters": {"agent_id": CLOUD_AGENT_ID}})), + Attempts::RetryTransient, ) .await?; let results = response @@ -490,6 +503,7 @@ impl Dialect for Mem0Dialect { Value::Object(filters), opts.min_score, )), + Attempts::RetryTransient, ) .await? } @@ -509,6 +523,7 @@ impl Dialect for Mem0Dialect { Method::POST, "v3/memories/search/", Some(&Self::search_body(query, limit, filters, opts.min_score)), + Attempts::RetryTransient, ) .await? } diff --git a/adapters/remote/src/supermemory.rs b/adapters/remote/src/supermemory.rs index 28c4111f..f2eab3a4 100644 --- a/adapters/remote/src/supermemory.rs +++ b/adapters/remote/src/supermemory.rs @@ -7,7 +7,9 @@ use tinymemory_api::recall::RecallOpts; use tinymemory_api::traits::Memory; use tinymemory_api::types::MemoryTaint; -use crate::common::{category, stable_id, Dialect, HttpClient, RemoteMemory, StoredEntry}; +use crate::common::{ + category, stable_id, Attempts, Dialect, HttpClient, RemoteMemory, StoredEntry, +}; /// Stable driver id used by configuration and status output. pub use tinymemory_api::drivers::SUPERMEMORY_DRIVER_ID; @@ -163,6 +165,13 @@ impl Memory for SupermemoryMemory { async fn health_check(&self) -> bool { self.inner.health_check().await } + /// Forwarded explicitly: this wrapper delegates method-by-method, so the + /// defaulted `None` would otherwise shadow `RemoteMemory`'s typed probe — + /// which is exactly what the first cut shipped, making §U4's deep health + /// unreachable through every public type (the #68 review's Major 1). + async fn health_probe(&self) -> Option { + self.inner.health_probe().await + } } #[derive(Debug)] @@ -218,7 +227,14 @@ impl SupermemoryDialect { .get("tinymemory_session_id") .and_then(Value::as_str) .map(str::to_owned), - score: value.get("similarity").and_then(Value::as_f64), + // Both spellings: the adapter's own double and this crate's + // upstream-mirrored conformance double disagree ("similarity" vs + // "score"), and losing the number silently turns min_score into a + // drop-everything filter (#68 review, Major 2). + score: value + .get("similarity") + .or_else(|| value.get("score")) + .and_then(Value::as_f64), taint: metadata .get("tinymemory_taint") .and_then(Value::as_str) @@ -231,7 +247,12 @@ impl SupermemoryDialect { async fn memories(&self) -> anyhow::Result> { let tags: Value = self .client - .json(Method::GET, "v3/container-tags/list", None) + .json( + Method::GET, + "v3/container-tags/list", + None, + Attempts::RetryTransient, + ) .await?; let container_tags = tags .as_array() @@ -272,6 +293,7 @@ impl SupermemoryDialect { "order": "desc", "containerTags": [container_tag] })), + Attempts::RetryTransient, ) .await?; let memories = response @@ -388,7 +410,12 @@ impl Dialect for SupermemoryDialect { } let response: Value = self .client - .json(Method::POST, "v4/search", Some(&body)) + .json( + Method::POST, + "v4/search", + Some(&body), + Attempts::RetryTransient, + ) .await?; Ok(response .get("results") diff --git a/adapters/remote/src/supermemory_test.rs b/adapters/remote/src/supermemory_test.rs index 5fdf914d..d72a6b2f 100644 --- a/adapters/remote/src/supermemory_test.rs +++ b/adapters/remote/src/supermemory_test.rs @@ -224,6 +224,40 @@ async fn native_supermemory_round_trips_the_tinymemory_contract() { state.0.lock().expect("state lock").last_search_tag, Some(expected_tag) ); + // #68 review Major 2: min_score connected to the double's OWN response + // shape — the first cut's strictness was only ever tested against + // synthetic Option values, which is how a decode/emit field mismatch + // dropped every hit. Below the double's similarity (0.95): survives. + // Above it: drops. Semantics AND the decode, in one pair. + let scored = |min: f64| { + let driver = &driver; + async move { + driver + .recall( + "Rust", + 1, + &OwnedRecallOpts { + namespace: Some("project".into()), + min_score: Some(min), + ..OwnedRecallOpts::default() + }, + None, + ) + .await + .expect("recall") + .len() + } + }; + assert_eq!( + scored(0.1).await, + 1, + "a scored hit above the threshold survives" + ); + assert_eq!( + scored(0.99).await, + 0, + "a scored hit below the threshold drops" + ); assert!(driver.forget("project", "decision").await.expect("forget")); assert!(driver.health().await.is_usable()); } diff --git a/vendor/tinycortex b/vendor/tinycortex index 34cbb6cf..8401346b 160000 --- a/vendor/tinycortex +++ b/vendor/tinycortex @@ -1 +1 @@ -Subproject commit 34cbb6cfa91ea74d62605bd57790782b0c748556 +Subproject commit 8401346b574cacb1dc0cf6b36bc608ff5ef9f6f5 From 5dc3b991bdef19b43cc2b5abd6ec752b0ccd250a Mon Sep 17 00:00:00 2001 From: Shanu Date: Thu, 20 Aug 2026 23:35:24 +0530 Subject: [PATCH 5/5] lint: ride out the Rust 1.98 clippy wave MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI runs unpinned stable and 1.98 landed mid-review, failing lanes on code this PR never touched. Two classes: chunks_exact(4) in core's four bytes_to_vec sites — converted to as_chunks::<4>(), the lint's own suggestion, dropping per-chunk bounds checks (the helpers variant also loses a try_into fallback that could silently zero a malformed chunk); and the NEW unused_async_trait_impl lint name on the tinybus::interface impls, which the existing unused_async allows did not cover — the macro requires async fn, so the allows extend to the new name with the same reason. The module workspace's lockfile was regenerated by cargo 1.98 in the same run. (Amended: the first push of this commit carried an unrelated message swept in from a wrong-directory commit; content unchanged.) --- core/src/store/namespace_store/events.rs | 7 +- core/src/store/namespace_store/helpers.rs | 10 +- core/src/store/namespace_store/segments.rs | 7 +- core/src/tree/score/embed/mod.rs | 6 +- crates/tinymemory-module/Cargo.lock | 139 ++++++------------ .../tinymemory-module/src/embedding_test.rs | 6 +- crates/tinymemory-module/src/service/mod.rs | 2 + crates/tinymemory-module/tests/module_e2e.rs | 6 +- 8 files changed, 70 insertions(+), 113 deletions(-) diff --git a/core/src/store/namespace_store/events.rs b/core/src/store/namespace_store/events.rs index c03bf076..2772f621 100644 --- a/core/src/store/namespace_store/events.rs +++ b/core/src/store/namespace_store/events.rs @@ -465,9 +465,10 @@ fn vec_to_bytes(v: &[f32]) -> Vec { } fn bytes_to_vec(bytes: &[u8]) -> Vec { - bytes - .chunks_exact(4) - .map(|chunk| f32::from_le_bytes([chunk[0], chunk[1], chunk[2], chunk[3]])) + let (chunks, _remainder) = bytes.as_chunks::<4>(); + chunks + .iter() + .map(|chunk| f32::from_le_bytes(*chunk)) .collect() } diff --git a/core/src/store/namespace_store/helpers.rs b/core/src/store/namespace_store/helpers.rs index 11d184bd..9db6317a 100644 --- a/core/src/store/namespace_store/helpers.rs +++ b/core/src/store/namespace_store/helpers.rs @@ -56,12 +56,10 @@ impl UnifiedMemory { } pub(crate) fn bytes_to_vec(bytes: &[u8]) -> Vec { - bytes - .chunks_exact(4) - .map(|chunk| { - let arr: [u8; 4] = chunk.try_into().unwrap_or([0; 4]); - f32::from_le_bytes(arr) - }) + let (chunks, _remainder) = bytes.as_chunks::<4>(); + chunks + .iter() + .map(|chunk| f32::from_le_bytes(*chunk)) .collect() } diff --git a/core/src/store/namespace_store/segments.rs b/core/src/store/namespace_store/segments.rs index 3af779b1..de118bbf 100644 --- a/core/src/store/namespace_store/segments.rs +++ b/core/src/store/namespace_store/segments.rs @@ -600,9 +600,10 @@ fn vec_to_bytes(v: &[f32]) -> Vec { } fn bytes_to_vec(bytes: &[u8]) -> Vec { - bytes - .chunks_exact(4) - .map(|chunk| f32::from_le_bytes([chunk[0], chunk[1], chunk[2], chunk[3]])) + let (chunks, _remainder) = bytes.as_chunks::<4>(); + chunks + .iter() + .map(|chunk| f32::from_le_bytes(*chunk)) .collect() } diff --git a/core/src/tree/score/embed/mod.rs b/core/src/tree/score/embed/mod.rs index 2181e62b..8afa40ba 100644 --- a/core/src/tree/score/embed/mod.rs +++ b/core/src/tree/score/embed/mod.rs @@ -313,10 +313,8 @@ pub fn unpack_embedding(b: &[u8]) -> Result> { b.len() ); } - let floats: Vec = b - .chunks_exact(4) - .map(|c| f32::from_le_bytes([c[0], c[1], c[2], c[3]])) - .collect(); + let (chunks, _remainder) = b.as_chunks::<4>(); + let floats: Vec = chunks.iter().map(|c| f32::from_le_bytes(*c)).collect(); if floats.len() != EMBEDDING_DIM { anyhow::bail!( "embedding blob length {} floats, expected {}", diff --git a/crates/tinymemory-module/Cargo.lock b/crates/tinymemory-module/Cargo.lock index ee5c912b..d7035717 100644 --- a/crates/tinymemory-module/Cargo.lock +++ b/crates/tinymemory-module/Cargo.lock @@ -144,7 +144,7 @@ checksum = "d524456ba66e72eb8b115ff89e01e497f8e6d11d78b70b1aa13c0fbd97540a81" dependencies = [ "cfg-if", "cpufeatures 0.3.0", - "rand_core 0.10.1", + "rand_core", ] [[package]] @@ -263,7 +263,16 @@ version = "5.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "44c45a9d03d6676652bcb5e724c7e988de1acad23a711b5217ab9cbecbec2225" dependencies = [ - "dirs-sys", + "dirs-sys 0.4.1", +] + +[[package]] +name = "dirs" +version = "6.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c3e8aa94d75141228480295a7d0e7feb620b1a5ad9f12bc40be62411e38cce4e" +dependencies = [ + "dirs-sys 0.5.0", ] [[package]] @@ -274,10 +283,22 @@ checksum = "520f05a5cbd335fae5a99ff7a6ab8627577660ee5cfd6a94a6a929b52ff0321c" dependencies = [ "libc", "option-ext", - "redox_users", + "redox_users 0.4.6", "windows-sys 0.48.0", ] +[[package]] +name = "dirs-sys" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e01a3366d27ee9890022452ee61b2b63a67e6f13f58900b651ff5665f0bb1fab" +dependencies = [ + "libc", + "option-ext", + "redox_users 0.5.2", + "windows-sys 0.61.2", +] + [[package]] name = "displaydoc" version = "0.2.7" @@ -491,7 +512,7 @@ dependencies = [ "js-sys", "libc", "r-efi", - "rand_core 0.10.1", + "rand_core", "wasm-bindgen", ] @@ -998,15 +1019,6 @@ dependencies = [ "zerovec", ] -[[package]] -name = "ppv-lite86" -version = "0.2.21" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "85eae3c4ed2f50dcfe72643da4befc30deadb458a9b590d720cde2f2b1e97da9" -dependencies = [ - "zerocopy", -] - [[package]] name = "proc-macro2" version = "1.0.107" @@ -1045,7 +1057,7 @@ dependencies = [ "bytes", "getrandom 0.4.3", "lru-slab", - "rand 0.10.2", + "rand", "rand_pcg", "ring", "rustc-hash", @@ -1087,17 +1099,6 @@ version = "6.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" -[[package]] -name = "rand" -version = "0.8.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "22f6172bdec972074665ed81ed53b71da00bfc44b65a753cfde883ec4c702a1a" -dependencies = [ - "libc", - "rand_chacha", - "rand_core 0.6.4", -] - [[package]] name = "rand" version = "0.10.2" @@ -1106,26 +1107,7 @@ checksum = "c7f5fa3a058cd35567ef9bfa5e75732bee0f9e4c55fa90477bef2dfcdbc4be80" dependencies = [ "chacha20", "getrandom 0.4.3", - "rand_core 0.10.1", -] - -[[package]] -name = "rand_chacha" -version = "0.3.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e6c10a63a0fa32252be49d21e7709d4d4baf8d231c2dbce1eaa8141b9b127d88" -dependencies = [ - "ppv-lite86", - "rand_core 0.6.4", -] - -[[package]] -name = "rand_core" -version = "0.6.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ec0be4795e2f6a28069bec0b5ff3e2ac9bafc99e6a9a7dc3547996c5c816922c" -dependencies = [ - "getrandom 0.2.17", + "rand_core", ] [[package]] @@ -1140,7 +1122,7 @@ version = "0.10.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "caa0f4137e1c0a72f4c651489402276c8e8e1cf081f3b0ba156d2cbeef09e86a" dependencies = [ - "rand_core 0.10.1", + "rand_core", ] [[package]] @@ -1163,6 +1145,17 @@ dependencies = [ "thiserror 1.0.69", ] +[[package]] +name = "redox_users" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a4e608c6638b9c18977b00b475ac1f28d14e84b27d8d42f70e0bf1e3dec127ac" +dependencies = [ + "getrandom 0.2.17", + "libredox", + "thiserror 2.0.20", +] + [[package]] name = "ref-cast" version = "1.0.26" @@ -1744,13 +1737,13 @@ dependencies = [ "anyhow", "async-trait", "chrono", - "dirs", + "dirs 5.0.1", "futures", "git2", "hex", "log", "parking_lot", - "rand 0.10.2", + "rand", "regex", "reqwest", "rusqlite", @@ -1810,11 +1803,11 @@ dependencies = [ "anyhow", "async-trait", "chrono", - "dirs", + "dirs 6.0.0", "futures", "log", "parking_lot", - "rand 0.8.7", + "rand", "regex", "reqwest", "rusqlite", @@ -1873,7 +1866,7 @@ dependencies = [ "serde_json", "tinymemory-api", "tokio", - "toml 0.9.12+spec-1.1.0", + "toml 1.1.4+spec-1.1.0", "tracing", "uuid", "walkdir", @@ -1995,21 +1988,6 @@ dependencies = [ "toml_edit", ] -[[package]] -name = "toml" -version = "0.9.12+spec-1.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cf92845e79fc2e2def6a5d828f0801e29a2f8acc037becc5ab08595c7d5e9863" -dependencies = [ - "indexmap", - "serde_core", - "serde_spanned 1.1.1", - "toml_datetime 0.7.5+spec-1.1.0", - "toml_parser", - "toml_writer", - "winnow 0.7.15", -] - [[package]] name = "toml" version = "1.1.4+spec-1.1.0" @@ -2034,15 +2012,6 @@ dependencies = [ "serde", ] -[[package]] -name = "toml_datetime" -version = "0.7.5+spec-1.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "92e1cfed4a3038bc5a127e35a2d360f145e1f4b971b551a2ba5fd7aedf7e1347" -dependencies = [ - "serde_core", -] - [[package]] name = "toml_datetime" version = "1.1.1+spec-1.1.0" @@ -2656,26 +2625,6 @@ dependencies = [ "synstructure", ] -[[package]] -name = "zerocopy" -version = "0.8.56" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "556764e583adb45a9f8d413c2a147fa7e8d821e48e12b14fd560b607998b75eb" -dependencies = [ - "zerocopy-derive", -] - -[[package]] -name = "zerocopy-derive" -version = "0.8.56" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f2ab42fc20575779bd240faa45f94a74256f755c0fa9e89f0ede20d91d0cdfc1" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.119", -] - [[package]] name = "zerofrom" version = "0.1.8" diff --git a/crates/tinymemory-module/src/embedding_test.rs b/crates/tinymemory-module/src/embedding_test.rs index 3236a8f5..f90ada33 100644 --- a/crates/tinymemory-module/src/embedding_test.rs +++ b/crates/tinymemory-module/src/embedding_test.rs @@ -27,7 +27,11 @@ struct FakeHostEmbedder { #[tinybus::interface(name = "ai.tinyhumans.tinymemory.EmbeddingHost")] impl FakeHostEmbedder { - #[allow(clippy::unused_async, reason = "the interface macro requires async")] + #[allow( + clippy::unused_async, + clippy::unused_async_trait_impl, + reason = "the interface macro requires async" + )] async fn embed( &self, _model: String, diff --git a/crates/tinymemory-module/src/service/mod.rs b/crates/tinymemory-module/src/service/mod.rs index eba50abb..afee3d8c 100644 --- a/crates/tinymemory-module/src/service/mod.rs +++ b/crates/tinymemory-module/src/service/mod.rs @@ -256,6 +256,7 @@ impl MemoryService { /// The bound driver's stable identifier. #[allow( clippy::unused_async, + clippy::unused_async_trait_impl, reason = "tinybus::interface requires every method to be `async fn`" )] async fn driver_id(&self) -> BusResult { @@ -269,6 +270,7 @@ impl MemoryService { /// change afterwards. #[allow( clippy::unused_async, + clippy::unused_async_trait_impl, reason = "tinybus::interface requires every method to be `async fn`" )] async fn capabilities(&self) -> BusResult { diff --git a/crates/tinymemory-module/tests/module_e2e.rs b/crates/tinymemory-module/tests/module_e2e.rs index 7c24f776..49af10e2 100644 --- a/crates/tinymemory-module/tests/module_e2e.rs +++ b/crates/tinymemory-module/tests/module_e2e.rs @@ -69,7 +69,11 @@ struct HostEmbedder; #[tinybus::interface(name = "ai.tinyhumans.tinymemory.EmbeddingHost")] impl HostEmbedder { - #[allow(clippy::unused_async, reason = "the interface macro requires async")] + #[allow( + clippy::unused_async, + clippy::unused_async_trait_impl, + reason = "the interface macro requires async" + )] async fn embed( &self, _model: String,