diff --git a/README.md b/README.md index 47691029..c00b76e0 100644 --- a/README.md +++ b/README.md @@ -214,6 +214,41 @@ Capabilities are asked **once, at bind time, and cached**: a host filters its RP surface and its agent-tool list from the answer, so a set that changed afterwards would not be noticed. +## The section surface + +Namespaces follow a `
:` convention — `conversation:thread-8f21`, +`learning:rust-async`, `document:handbook` — so "conversational memory", +"document memory" and "learnings" mean the same thing to every host and every +engine. `tinymemory::sections` makes that convention a typed surface instead of +a string every caller concatenates by hand: + +```rust +use tinymemory::sections::Sections; + +let sections = Sections::new(provider.as_ref()); + +sections.conversations().put("thread-8f21", "turn-1", text, category, None, taint).await?; +let topics = sections.learnings().scopes().await?; +let hits = sections.recall().across_section(&MemorySection::Learning, "async", 10, &opts, None).await?; +``` + +`conversations()`, `learnings()` and `documents()` are the three sections a host +writes to routinely; `section()` reaches the other four and `Custom`. Every call +composes the **mandatory** families only, so the whole surface works on every +driver — nothing to negotiate, and no capability-absent path. On a driver that +retains nothing, every call succeeds and returns empty. + +`across_section` is a fan-out: one namespace enumeration plus one recall per +scope, capped, reporting what it searched and whether the cap bit. It is not an +unfinished optimisation — `OwnedRecallOpts::namespace` is an exact match, and +leaving it unset means the `global` namespace on the embedded engine but *every* +namespace on the reference driver, so there is no cross-namespace recall to build +a single call on. See [`docs/specs/memory-section-api.md`](docs/specs/memory-section-api.md). + +Handing the layer a **file** is a different path: `DocumentIntake` sniffs the +format, converts it, and picks the capability family. The section surface is for +text you already hold. + ## What lives here, and what deliberately does not | Here | In the host | diff --git a/crates/tinymemory/examples/tinycortex.rs b/crates/tinymemory/examples/tinycortex.rs index 68f1e5d2..64ddfdc9 100644 --- a/crates/tinymemory/examples/tinycortex.rs +++ b/crates/tinymemory/examples/tinycortex.rs @@ -1,4 +1,5 @@ -//! The embedded engine, end to end: admit, construct, audit, store, recall. +//! The embedded engine, end to end: admit, construct, audit, store, recall, +//! and the same store read back through the section surface. //! //! Run with: //! @@ -19,7 +20,9 @@ use std::sync::Arc; use tinymemory::api::provider::{audit_provider, MemoryProvider}; use tinymemory::api::recall::OwnedRecallOpts; use tinymemory::api::types::{MemoryCategory, MemoryTaint}; +use tinymemory::namespace::MemorySection; use tinymemory::registry::{ConfigLabels, DriverRegistry, TINYCORTEX_DRIVER_ID}; +use tinymemory::sections::Sections; use tinymemory::tinycortex::{provider, InMemoryMemoryStore}; #[tokio::main] @@ -59,5 +62,44 @@ async fn main() -> Result<(), Box> { let hits = provider.recall("hello", 8, &opts, None).await?; println!("recall found {} entr(y/ies)", hits.len()); assert!(!hits.is_empty(), "the stored entry must be recallable"); + + // 5. The same engine through the section surface: the caller names a + // scope, never a namespace, and asks the whole section one question. + let sections = Sections::new(provider.as_ref()); + for (scope, note) in [ + ("rust-async", "pinning is not unpinning"), + ("rust-macros", "hygiene is per-expansion"), + ] { + let namespace = sections + .learnings() + .put( + scope, + "note", + note, + MemoryCategory::Core, + None, + MemoryTaint::Internal, + ) + .await?; + println!("learning stored in '{namespace}'"); + } + + let found = sections + .recall() + .across_section( + &MemorySection::Learning, + "is", + 8, + &OwnedRecallOpts::default(), + None, + ) + .await?; + println!( + "section recall searched {} namespace(s) and found {} hit(s)", + found.namespaces_searched, + found.hits.len() + ); + assert_eq!(found.namespaces_searched, 2, "both scopes must be searched"); + assert!(!found.hits.is_empty(), "the section recall must find them"); Ok(()) } diff --git a/crates/tinymemory/src/lib.rs b/crates/tinymemory/src/lib.rs index e8035da9..05dcdebb 100644 --- a/crates/tinymemory/src/lib.rs +++ b/crates/tinymemory/src/lib.rs @@ -19,6 +19,10 @@ //! rather than re-deriving the same four subtleties. //! - **[`registry`]** — driver admission. Which driver ids exist, what class //! each binds as, and the fail-closed rule for out-of-process drivers. +//! - **[`sections`]** — typed surfaces for the sections the namespace +//! convention names: conversations, learnings, documents, and a +//! section-aware recall. Composes the mandatory families only, so it works +//! on every driver. //! - **Engine adapters** — one crate per engine under `crates/`, each //! implementing [`provider::MemoryProvider`] over a concrete engine, and //! each selected by the feature named after it. @@ -138,12 +142,18 @@ pub use tinymemory_conformance as conformance; pub mod registry; +// Typed surfaces for the sections the namespace convention names — +// conversations, learnings, documents — plus a section-aware recall. Documented +// by its own `//!` docs; an outer doc comment here as well would merge the two +// and resolve the module's intra-doc links in this file's scope instead. +pub mod sections; + // The contract, re-exported wholesale. Listed module by module rather than as a // glob so the crate's own surface is visible in one place and rustdoc links // resolve — and so adding a module to the contract is a deliberate act here too. pub use tinymemory_api::{ - capabilities, chunks, error, goals, health, null, provider, recall, tool_memory, traits, tree, - types, + capabilities, chunks, error, goals, health, namespace, null, provider, recall, tool_memory, + traits, tree, types, }; pub use tinymemory_api::{is_compatible, CONTRACT_VERSION}; diff --git a/crates/tinymemory/src/sections/README.md b/crates/tinymemory/src/sections/README.md new file mode 100644 index 00000000..daa4a4f0 --- /dev/null +++ b/crates/tinymemory/src/sections/README.md @@ -0,0 +1,105 @@ +# `sections` + +Typed surfaces over the `
:` namespace convention +(`crates/tinymemory-bus/src/namespace.rs`): `Sections`, `SectionView`, and +`SectionRecall`. Nothing here is a new capability — every call composes +`MemoryCore` and `MemoryRecall`, which every driver implements as supertraits — +this module only stops a caller from hand-concatenating the `conversation:` / +`learning:` / `document:` prefix, where a typo silently produces a different, +valid namespace instead of an error. + +## Design + +```text +Sections::new(provider) + ├── conversations() ─┐ + ├── learnings() ├─ SectionView put / get / forget / list + ├── documents() │ scopes / list_section + ├── section(custom) ─┘ + └── recall() ── SectionRecall in_scope / across_section +``` + +- `Sections` is the entry point: one named accessor per routine section + (`conversations`, `learnings`, `documents`) plus `section(&MemorySection)` for + the rest of the vocabulary (`entity:`, `profile:`, `tool:`, `source:`, and + `Custom`) and `recall()` for the cross-cutting query surface. +- `SectionView` addresses one section by scope — `put` / `get` / `forget` / + `list` take the bare scope (`"thread-8f21"`), never the prefixed namespace — + and enumerates it with `scopes()` / `list_section()`. +- `SectionRecall` answers two different questions, deliberately kept apart + because they cost different amounts: `in_scope` is one provider call; + `across_section` fans out to one call per namespace in the section. + +Every handle borrows `&dyn MemoryProvider` (see `view.rs`, `recall.rs`): cheap +to construct, holds no state between calls, and cannot outlive the provider — +so a caller builds one where it is needed instead of threading it through a +struct. + +`MemorySection` is normalised through `MemorySection::from_prefix` in +`SectionView::new`, so `Custom("conversation")` and `MemorySection::Conversation` +are the same view rather than two. Storing the caller's spelling verbatim would +let a write land under `conversation:` while a `scopes()` call — which compares +against this normalised field — reported the section as empty. + +## Public surface + +- `Sections::{new, conversations, learnings, documents, section, recall}` +- `SectionView::{put, get, forget, list, scopes, list_section}` +- `SectionRecall::{in_scope, across_section}` +- `SectionScope`, `SectionHits` — the value types `scopes()` / recall return +- `MAX_SECTION_NAMESPACES` — the fan-out cap `across_section` enforces +- `NAMESPACE_FILTER_CONFLICT`, `CROSS_SESSION_SECTION_CONFLICT`, + `CROSS_SESSION_FAN_OUT_CONFLICT` — the exact `MemoryError::Invalid` messages + the recall refusals carry, exposed so a caller's test can assert against the + same string it sees + +## Operational constraints + +**`across_section` is a fan-out, not a filtered call.** `OwnedRecallOpts::namespace` +is exact-match, and `namespace: None` means the literal `global` namespace on +the embedded engine but *every* namespace on the reference driver +(`crates/tinymemory-conformance/src/reference/mod.rs`). A single unfiltered call +plus post-filtering would return nothing in production, so `across_section` +enumerates `scopes()` and issues one exact-namespace recall per scope instead, +capped at `MAX_SECTION_NAMESPACES` and reported through `SectionHits::truncated` +when the cap bites. Each namespace is asked for the full `limit`, never a +share of it — a share would let one scope's best hit lose to another's worst. + +**`cross_session` and `session_id` are refused outside the conversation +section, and refused on `across_section` unconditionally.** The bundled +`UnifiedMemory` driver's `cross_session` recall option surfaces episodic +*conversational* rows from other sessions; its `session_id` option +independently appends that session's episodic rows. Both relabel every such +row with whichever namespace the call was pinned to, regardless of the +option's own defaults. Honouring either on a `learning:` or `document:` +section would therefore return conversational content mislabeled as that +section's own hits, so `in_scope` rejects both with +`CROSS_SESSION_SECTION_CONFLICT` — checked against the section's *normalised* +form, so `Custom("conversation")` counts as `MemorySection::Conversation` — +unless `section == MemorySection::Conversation`. + +`across_section` rejects both unconditionally, with +`CROSS_SESSION_FAN_OUT_CONFLICT`, including on the conversation section. This +is not merely the same hazard: the driver's episodic augmentation runs once, +independent of the pinned namespace, so the fan-out would repeat the exact +same rows once per scope in the merged result, crowding genuine hits out of +`limit` — and it is also redundant even where it would not repeat, since +`across_section` already visits every conversation scope on its own. A caller +who wants cross-session or session-scoped recall uses `in_scope` instead, +which issues exactly one call. + +**Visit order is by entry count descending, not recency.** `SectionScope::last_updated` +is optional and no bundled driver currently populates it, so `scopes()` cannot +order by recency today. This is deliberate and raised as an open question in +`docs/specs/memory-section-api.md`, not an oversight. + +**This is not the document intake path.** `Sections::documents` writes through +`MemoryCore`, for text a caller already holds. Handing the memory layer a +*file* — sniffing its format, converting it to markdown, then choosing between +`MemoryIngest`, `MemoryDocuments`, and `MemoryCore` — is `DocumentIntake`'s job +in the `documents` module, which is the right entry point for an upload. + +**The `namespace: None` divergence between drivers is out of scope here.** The +embedded engine and the reference driver disagree on what an unfiltered recall +means, as noted above; fixing that divergence needs its own spec and is +deliberately not attempted by this module. diff --git a/crates/tinymemory/src/sections/mod.rs b/crates/tinymemory/src/sections/mod.rs new file mode 100644 index 00000000..8af04ccc --- /dev/null +++ b/crates/tinymemory/src/sections/mod.rs @@ -0,0 +1,183 @@ +//! Typed surfaces for the sections the namespace convention names: +//! conversations, learnings, and documents — plus a recall that can span one. +//! +//! Namespaces are the contract's only partitioning primitive and they cross it +//! as a bare `&str`. `MemorySection` gives the string a shape — +//! `
:`, so `conversation:thread-8f21` and `learning:rust-async` +//! mean the same thing to every host and every engine — but nothing made using +//! it easier than concatenating the prefix by hand, where a typo produces a +//! valid, silently wrong namespace instead of an error. +//! +//! This module is that missing ergonomics layer, and nothing more: +//! +//! ```text +//! Sections::new(provider) +//! ├── conversations() ─┐ +//! ├── learnings() ├─ SectionView put / get / forget / list +//! ├── documents() │ scopes / list_section +//! ├── section(custom) ─┘ +//! └── recall() ── SectionRecall in_scope / across_section +//! ``` +//! +//! ## Mandatory families only +//! +//! Every call here composes `MemoryCore` and `MemoryRecall`, which every driver +//! implements as supertraits. So the whole surface works on *any* provider — +//! there is no capability to negotiate, no accessor that can return `None`, and +//! no "unsupported" path to handle. On a driver that retains nothing, every call +//! succeeds and returns empty. +//! +//! ## This is not the document intake path +//! +//! [`Sections::documents`] writes through `MemoryCore`, so it is for text you +//! already hold. Handing the memory layer a *file* — sniffing its format, +//! converting it to markdown, then choosing between `MemoryIngest`, +//! `MemoryDocuments` and `MemoryCore` — is `DocumentIntake`'s job in the +//! `documents` module, and it is the right entry point for an upload. Reaching +//! for this one instead would build a second, worse intake. +//! +//! ## Borrowed, not owned +//! +//! Every handle holds a `&dyn MemoryProvider`. They are cheap to create and +//! discard, hold no state between calls, and cannot outlive the provider — so a +//! caller makes one where it is needed rather than threading it through a +//! struct. +//! +//! ## Example +//! +//! ``` +//! use std::sync::Arc; +//! +//! use tinymemory::namespace::MemorySection; +//! use tinymemory::provider::MemoryProvider; +//! use tinymemory::sections::Sections; +//! use tinymemory::types::{MemoryCategory, MemoryTaint}; +//! use tinymemory_conformance::InMemoryProvider; +//! +//! let provider: Arc = Arc::new(InMemoryProvider::new()); +//! let runtime = tokio::runtime::Runtime::new()?; +//! +//! runtime.block_on(async { +//! let sections = Sections::new(provider.as_ref()); +//! +//! // The caller names a scope; the handle owns the prefix. +//! let namespace = sections +//! .conversations() +//! .put( +//! "thread-8f21", +//! "turn-1", +//! "we agreed to ship on the 14th", +//! MemoryCategory::Core, +//! None, +//! MemoryTaint::Internal, +//! ) +//! .await?; +//! assert_eq!(namespace.as_str(), "conversation:thread-8f21"); +//! +//! // Discover the scopes a section holds, without knowing the convention. +//! let scopes = sections.conversations().scopes().await?; +//! assert_eq!(scopes.len(), 1); +//! assert_eq!(scopes[0].scope(), "thread-8f21"); +//! +//! // Ask the whole section one question. +//! let found = sections +//! .recall() +//! .across_section(&MemorySection::Conversation, "ship", 10, &Default::default(), None) +//! .await?; +//! assert_eq!(found.namespaces_searched, 1); +//! assert!(!found.truncated); +//! +//! Ok::<(), tinymemory::error::MemoryError>(()) +//! })?; +//! # Ok::<(), Box>(()) +//! ``` + +use std::fmt; + +use tinymemory_api::namespace::MemorySection; +use tinymemory_api::provider::MemoryProvider; + +// Private, with the whole surface re-exported below: one public path per item, +// as `registry` does with its own submodules. +mod recall; +mod types; +mod view; + +pub use recall::SectionRecall; +pub use types::{ + SectionHits, SectionScope, CROSS_SESSION_FAN_OUT_CONFLICT, CROSS_SESSION_SECTION_CONFLICT, + MAX_SECTION_NAMESPACES, NAMESPACE_FILTER_CONFLICT, +}; +pub use view::SectionView; + +/// The entry point: a provider, viewed one section at a time. +/// +/// A borrowing handle, so build one where you need it rather than storing it. +#[derive(Clone, Copy)] +pub struct Sections<'a> { + provider: &'a dyn MemoryProvider, +} + +impl fmt::Debug for Sections<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Sections") + .field("driver_id", &self.provider.driver_id()) + .finish() + } +} + +impl<'a> Sections<'a> { + /// Bind `provider`. + #[must_use] + pub fn new(provider: &'a dyn MemoryProvider) -> Self { + Self { provider } + } + + /// Turn-by-turn conversational memory — the `conversation:` section. + #[must_use] + pub fn conversations(&self) -> SectionView<'a> { + self.section(&MemorySection::Conversation) + } + + /// Durable conclusions the agent drew and expects to reuse — the + /// `learning:` section. + #[must_use] + pub fn learnings(&self) -> SectionView<'a> { + self.section(&MemorySection::Learning) + } + + /// Whole documents and the collections they sit in — the `document:` + /// section, informally "the brain". + /// + /// For text you already hold. An upload belongs to `DocumentIntake`; see + /// the module docs. + #[must_use] + pub fn documents(&self) -> SectionView<'a> { + self.section(&MemorySection::Document) + } + + /// Any section, including the four this type has no named accessor for + /// (`entity:`, `profile:`, `tool:`, `source:`) and + /// [`MemorySection::Custom`]. + /// + /// The named accessors are the three sections a host writes to routinely; + /// this is the same view over the rest of the vocabulary, so nothing is + /// second-class. + /// + /// Taken by reference, as every section argument in this module is, so a + /// caller never has to remember which side wants which. + #[must_use] + pub fn section(&self, section: &MemorySection) -> SectionView<'a> { + SectionView::new(self.provider, section) + } + + /// Section-aware recall. + #[must_use] + pub fn recall(&self) -> SectionRecall<'a> { + SectionRecall::new(self.provider) + } +} + +#[cfg(test)] +#[path = "test.rs"] +mod test; diff --git a/crates/tinymemory/src/sections/recall.rs b/crates/tinymemory/src/sections/recall.rs new file mode 100644 index 00000000..b95fe2e7 --- /dev/null +++ b/crates/tinymemory/src/sections/recall.rs @@ -0,0 +1,233 @@ +//! [`SectionRecall`] — ranked retrieval within one scope, or across a section. + +use std::fmt; + +use tinymemory_api::error::MemoryError; +use tinymemory_api::namespace::MemorySection; +use tinymemory_api::provider::types::SourceScope; +use tinymemory_api::provider::MemoryProvider; +use tinymemory_api::recall::OwnedRecallOpts; + +use super::types::{ + merge_hits, SectionHits, CROSS_SESSION_FAN_OUT_CONFLICT, CROSS_SESSION_SECTION_CONFLICT, + MAX_SECTION_NAMESPACES, NAMESPACE_FILTER_CONFLICT, +}; +use super::view::SectionView; + +/// A borrowing handle for section-aware recall. +/// +/// Two questions, deliberately separate because they cost different amounts: +/// [`Self::in_scope`] is one provider call, and [`Self::across_section`] is one +/// per namespace in the section. +#[derive(Clone, Copy)] +pub struct SectionRecall<'a> { + provider: &'a dyn MemoryProvider, +} + +impl fmt::Debug for SectionRecall<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("SectionRecall") + .field("driver_id", &self.provider.driver_id()) + .finish() + } +} + +/// Refuse options that already pin a namespace. +fn reject_namespace_filter(opts: &OwnedRecallOpts) -> Result<(), MemoryError> { + if opts.namespace.is_some() { + return Err(MemoryError::Invalid(NAMESPACE_FILTER_CONFLICT.to_string())); + } + Ok(()) +} + +/// Whether `opts` carries either of the two options whose bundled-driver +/// behaviour ignores the pinned namespace: `cross_session`, or a `session_id` +/// (which triggers the driver's own session-scoped episodic augmentation, not +/// a filter — see [`CROSS_SESSION_SECTION_CONFLICT`]). +fn requests_episodic_augmentation(opts: &OwnedRecallOpts) -> bool { + opts.cross_session || opts.session_id.is_some() +} + +/// Refuse `cross_session` or a `session_id` on any section other than +/// [`MemorySection::Conversation`] — checked against the **normalised** +/// section, so `Custom("conversation")` is not falsely rejected. +/// +/// See [`CROSS_SESSION_SECTION_CONFLICT`] for why: the bundled driver's +/// `cross_session` and `session_id` options both surface episodic +/// conversational rows independently of the pinned namespace, and relabel +/// them with whatever namespace the call was pinned to — so honouring either +/// on a document or learning section would return conversational content +/// mislabeled as that section's own hits. +fn reject_episodic_augmentation_outside_conversation( + section: &MemorySection, + opts: &OwnedRecallOpts, +) -> Result<(), MemoryError> { + let normalized = MemorySection::from_prefix(section.as_str()); + if requests_episodic_augmentation(opts) && !matches!(normalized, MemorySection::Conversation) { + return Err(MemoryError::Invalid( + CROSS_SESSION_SECTION_CONFLICT.to_string(), + )); + } + Ok(()) +} + +/// Refuse `cross_session` or a `session_id` in [`SectionRecall::across_section`] +/// unconditionally, regardless of section. +/// +/// See [`CROSS_SESSION_FAN_OUT_CONFLICT`] for why: the driver's episodic +/// augmentation for either option runs once, independent of the pinned +/// namespace, so the fan-out would append the same extra rows once per scope — +/// including for [`MemorySection::Conversation`], where +/// [`reject_episodic_augmentation_outside_conversation`] alone would let it +/// through. +fn reject_episodic_augmentation_fan_out(opts: &OwnedRecallOpts) -> Result<(), MemoryError> { + if requests_episodic_augmentation(opts) { + return Err(MemoryError::Invalid( + CROSS_SESSION_FAN_OUT_CONFLICT.to_string(), + )); + } + Ok(()) +} + +/// `opts` with `namespace` pinned to `namespace`. +fn pinned_to(opts: &OwnedRecallOpts, namespace: &str) -> OwnedRecallOpts { + let mut pinned = opts.clone(); + pinned.namespace = Some(namespace.to_string()); + pinned +} + +impl<'a> SectionRecall<'a> { + /// Bind `provider`. + #[must_use] + pub fn new(provider: &'a dyn MemoryProvider) -> Self { + Self { provider } + } + + /// Recall within one scope of one section — a single provider call. + /// + /// Hits keep the order the driver returned them in: for one namespace that + /// order *is* the driver's ranking, and re-sorting it here would discard + /// whatever the engine knows and this façade does not. + /// + /// `sources` is the contract's per-turn source allowlist, passed straight + /// through. It is named `sources` rather than `scope` because `scope` here + /// means the namespace scope, and the two are unrelated. + /// + /// Note that a driver composed from the mandatory families **refuses** a + /// `Some(sources)` recall outright — it cannot apply the predicate + /// internally, and applying it afterwards would be wrong — so on those + /// drivers only `None` succeeds. That refusal is the driver's, passed + /// through unchanged rather than pre-empted here, so a driver that does + /// implement source scoping is not held back by this façade. + /// + /// # Errors + /// + /// [`MemoryError::Invalid`] in three cases: carrying + /// [`NAMESPACE_FILTER_CONFLICT`] when `opts` already pins a namespace, + /// carrying [`CROSS_SESSION_SECTION_CONFLICT`] when `opts.cross_session` or + /// `opts.session_id` is set on any section other than + /// [`MemorySection::Conversation`] (checked against the section's + /// normalised form, so `Custom("conversation")` counts as + /// [`MemorySection::Conversation`] here too), and carrying the namespace + /// validator's own message when the section and scope cannot form a valid + /// namespace. Otherwise whatever the backend returns. + pub async fn in_scope( + &self, + section: &MemorySection, + scope: &str, + query: &str, + limit: usize, + opts: &OwnedRecallOpts, + sources: Option<&SourceScope>, + ) -> Result { + reject_namespace_filter(opts)?; + reject_episodic_augmentation_outside_conversation(section, opts)?; + let namespace = SectionView::new(self.provider, section).namespace(scope)?; + let hits = self + .provider + .recall(query, limit, &pinned_to(opts, namespace.as_str()), sources) + .await?; + Ok(SectionHits { + hits, + namespaces_searched: 1, + truncated: false, + }) + } + + /// Recall across every scope in a section. + /// + /// **Costs one namespace enumeration plus one recall per scope in the + /// section**, up to [`MAX_SECTION_NAMESPACES`]. A caller who cannot afford + /// that should use [`Self::in_scope`]. + /// + /// The fan-out is not an optimisation to be replaced later by a single call + /// with a section filter — the contract has no cross-namespace recall to + /// build one on. `OwnedRecallOpts::namespace` is an exact match, and leaving + /// it `None` means the `global` namespace on the embedded engine while + /// meaning *every* namespace on the reference driver, so filtering the + /// results of one unfiltered call would be correct in tests and empty in + /// production. Asking each namespace by name is the only honest way to do + /// this, and it is what the contract's own `list_everything` does for the + /// same reason. + /// + /// Guarantees: + /// + /// - Namespaces are visited in [`SectionView::scopes`] order — entry count + /// descending, ties by namespace — so which ones the cap drops is + /// deterministic. + /// - Each namespace is asked for the full `limit`, never a share of it: a + /// share would let one scope's best hit lose to another's worst. + /// - Hits merge by score descending, absent scores last, ties by namespace + /// then key, and are then truncated to `limit`. + /// - [`SectionHits::truncated`] means *namespaces were skipped*. Hits + /// reaching `limit` is ordinary and is not reported as truncation. + /// - A section with no namespaces yields `Ok` with no hits and + /// `namespaces_searched: 0`, never an error. + /// + /// One caveat, stated rather than papered over: the scores being ranked come + /// from separate calls. They are comparable in practice on every bundled + /// driver, since it is one engine answering one query, but the contract does + /// not guarantee that a score means the same thing across two calls. + /// + /// # Errors + /// + /// [`MemoryError::Invalid`] carrying [`NAMESPACE_FILTER_CONFLICT`] when + /// `opts` already pins a namespace, or carrying + /// [`CROSS_SESSION_FAN_OUT_CONFLICT`] when `opts.cross_session` or + /// `opts.session_id` is set at all — on *every* section, including + /// [`MemorySection::Conversation`] — because the driver's episodic + /// augmentation for either option runs once, independent of the pinned + /// namespace, and would otherwise repeat once per scope in the fan-out, + /// crowding genuine hits out of `limit`; use [`Self::in_scope`] for a + /// cross-session or session-scoped query instead. Otherwise whatever the + /// backend returns from the enumeration or from any one recall — a section + /// recall fails as a whole rather than reporting a partial answer as a + /// success. + pub async fn across_section( + &self, + section: &MemorySection, + query: &str, + limit: usize, + opts: &OwnedRecallOpts, + sources: Option<&SourceScope>, + ) -> Result { + reject_namespace_filter(opts)?; + reject_episodic_augmentation_fan_out(opts)?; + let scopes = SectionView::new(self.provider, section).scopes().await?; + let truncated = scopes.len() > MAX_SECTION_NAMESPACES; + + let mut gathered = Vec::new(); + let mut namespaces_searched = 0; + for scope in scopes.into_iter().take(MAX_SECTION_NAMESPACES) { + let pinned = pinned_to(opts, scope.namespace.as_str()); + gathered.extend(self.provider.recall(query, limit, &pinned, sources).await?); + namespaces_searched += 1; + } + + Ok(SectionHits { + hits: merge_hits(gathered, limit), + namespaces_searched, + truncated, + }) + } +} diff --git a/crates/tinymemory/src/sections/test.rs b/crates/tinymemory/src/sections/test.rs new file mode 100644 index 00000000..0e527716 --- /dev/null +++ b/crates/tinymemory/src/sections/test.rs @@ -0,0 +1,1048 @@ +//! Unit tests for the section surface. +//! +//! Two doubles, for two different jobs. [`ScoredMemory`] is a `Memory` backend +//! whose entries carry scores the test chose, wrapped through +//! [`MemoryTraitProvider`] — needed because nothing in the contract lets a +//! caller *store* a score, and the fan-out's whole job is ranking by one. +//! `NullMemoryProvider` covers the other end: a driver that retains nothing, +//! where every call must still succeed. + +// A failing assertion in a test *is* a panic; the crate-wide `unwrap_used` / +// `expect_used` / `panic` lints exist to keep the library from panicking. +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use std::collections::BTreeMap; +use std::sync::{Arc, Mutex}; + +use async_trait::async_trait; +use tinymemory_api::error::MemoryError; +use tinymemory_api::namespace::MemorySection; +use tinymemory_api::null::NullMemoryProvider; +use tinymemory_api::provider::types::SourceScope; +use tinymemory_api::recall::OwnedRecallOpts; +use tinymemory_api::traits::Memory; +use tinymemory_api::types::{ + MemoryCategory, MemoryEntry, MemoryTaint, NamespaceSummary, RecallOpts, +}; + +use crate::mandatory::MemoryTraitProvider; + +use super::types::merge_hits; +use super::{ + Sections, CROSS_SESSION_FAN_OUT_CONFLICT, CROSS_SESSION_SECTION_CONFLICT, + MAX_SECTION_NAMESPACES, NAMESPACE_FILTER_CONFLICT, +}; + +/// Build an entry directly, so a test can set the `score` no API accepts. +fn entry(namespace: &str, key: &str, content: &str, score: Option) -> MemoryEntry { + MemoryEntry { + id: format!("{namespace}/{key}"), + key: key.to_string(), + content: content.to_string(), + namespace: Some(namespace.to_string()), + category: MemoryCategory::Core, + timestamp: "2026-01-01T00:00:00Z".to_string(), + session_id: None, + score, + taint: MemoryTaint::Internal, + } +} + +/// A `BTreeMap`-backed `Memory` whose recall honours an exact namespace filter +/// and returns the scores the test seeded. +#[derive(Default)] +struct ScoredMemory { + entries: Mutex>, +} + +impl ScoredMemory { + fn provider() -> (Arc, MemoryTraitProvider) { + let memory = Arc::new(Self::default()); + let provider = MemoryTraitProvider::new(memory.clone(), "scored-double"); + (memory, provider) + } + + /// Insert an entry with a chosen score, bypassing the score-less `store`. + fn seed(&self, namespace: &str, key: &str, content: &str, score: Option) { + self.entries.lock().unwrap().insert( + (namespace.to_string(), key.to_string()), + entry(namespace, key, content, score), + ); + } + + fn rows(&self) -> Vec { + self.entries.lock().unwrap().values().cloned().collect() + } +} + +#[async_trait] +impl Memory for ScoredMemory { + fn name(&self) -> &'static str { + "scored-double" + } + + async fn store( + &self, + namespace: &str, + key: &str, + content: &str, + category: MemoryCategory, + session_id: Option<&str>, + ) -> anyhow::Result<()> { + self.store_with_taint( + namespace, + key, + content, + category, + session_id, + MemoryTaint::Internal, + ) + .await + } + + async fn store_with_taint( + &self, + namespace: &str, + key: &str, + content: &str, + category: MemoryCategory, + session_id: Option<&str>, + taint: MemoryTaint, + ) -> anyhow::Result<()> { + let mut row = entry(namespace, key, content, None); + row.category = category; + row.session_id = session_id.map(str::to_string); + row.taint = taint; + self.entries + .lock() + .unwrap() + .insert((namespace.to_string(), key.to_string()), row); + Ok(()) + } + + async fn recall( + &self, + query: &str, + limit: usize, + opts: RecallOpts<'_>, + ) -> anyhow::Result> { + // An exact namespace match, as the contract specifies. A `None` + // namespace matches nothing here on purpose: the fan-out must never + // depend on what `None` means, because the bundled drivers disagree. + Ok(self + .rows() + .into_iter() + .filter(|row| row.namespace.as_deref() == opts.namespace) + .filter(|row| query.is_empty() || row.content.contains(query)) + .take(limit) + .collect()) + } + + async fn get(&self, namespace: &str, key: &str) -> anyhow::Result> { + Ok(self + .entries + .lock() + .unwrap() + .get(&(namespace.to_string(), key.to_string())) + .cloned()) + } + + async fn list( + &self, + namespace: Option<&str>, + category: Option<&MemoryCategory>, + session_id: Option<&str>, + ) -> anyhow::Result> { + Ok(self + .rows() + .into_iter() + .filter(|row| namespace.is_none_or(|ns| row.namespace.as_deref() == Some(ns))) + .filter(|row| category.is_none_or(|cat| &row.category == cat)) + .filter(|row| session_id.is_none_or(|sid| row.session_id.as_deref() == Some(sid))) + .collect()) + } + + async fn forget(&self, namespace: &str, key: &str) -> anyhow::Result { + Ok(self + .entries + .lock() + .unwrap() + .remove(&(namespace.to_string(), key.to_string())) + .is_some()) + } + + async fn namespace_summaries(&self) -> anyhow::Result> { + let mut counts: BTreeMap = BTreeMap::new(); + for row in self.rows() { + if let Some(namespace) = row.namespace { + *counts.entry(namespace).or_default() += 1; + } + } + Ok(counts + .into_iter() + .map(|(namespace, count)| NamespaceSummary { + namespace, + count, + last_updated: None, + }) + .collect()) + } + + async fn count(&self) -> anyhow::Result { + Ok(self.entries.lock().unwrap().len()) + } + + async fn health_check(&self) -> bool { + true + } +} + +// ---------------------------------------------------------------- merge_hits + +#[test] +fn merge_orders_by_score_descending() { + let merged = merge_hits( + vec![ + entry("learning:a", "low", "x", Some(0.1)), + entry("learning:b", "high", "x", Some(0.9)), + entry("learning:c", "mid", "x", Some(0.5)), + ], + 10, + ); + let keys: Vec<&str> = merged.iter().map(|e| e.key.as_str()).collect(); + assert_eq!(keys, ["high", "mid", "low"]); +} + +#[test] +fn merge_sorts_absent_scores_last() { + let merged = merge_hits( + vec![ + entry("learning:a", "unscored", "x", None), + entry("learning:b", "scored", "x", Some(0.01)), + ], + 10, + ); + let keys: Vec<&str> = merged.iter().map(|e| e.key.as_str()).collect(); + assert_eq!(keys, ["scored", "unscored"]); +} + +#[test] +fn merge_breaks_ties_by_namespace_then_key() { + let merged = merge_hits( + vec![ + entry("learning:b", "second", "x", Some(0.5)), + entry("learning:a", "zebra", "x", Some(0.5)), + entry("learning:a", "alpha", "x", Some(0.5)), + ], + 10, + ); + let pairs: Vec<(&str, &str)> = merged + .iter() + .map(|e| (e.namespace.as_deref().unwrap(), e.key.as_str())) + .collect(); + assert_eq!( + pairs, + [ + ("learning:a", "alpha"), + ("learning:a", "zebra"), + ("learning:b", "second"), + ] + ); +} + +#[test] +fn merge_truncates_after_ranking_not_before() { + let merged = merge_hits( + vec![ + entry("learning:a", "low", "x", Some(0.1)), + entry("learning:b", "high", "x", Some(0.9)), + ], + 1, + ); + assert_eq!(merged.len(), 1); + assert_eq!(merged[0].key, "high"); +} + +// --------------------------------------------------------------- SectionView + +#[tokio::test] +async fn put_writes_under_the_section_prefix() { + let (memory, provider) = ScoredMemory::provider(); + let namespace = Sections::new(&provider) + .conversations() + .put( + "thread-8f21", + "turn-1", + "hello", + MemoryCategory::Core, + None, + MemoryTaint::Internal, + ) + .await + .unwrap(); + + assert_eq!(namespace.as_str(), "conversation:thread-8f21"); + assert_eq!( + memory.rows()[0].namespace.as_deref(), + Some("conversation:thread-8f21") + ); +} + +#[tokio::test] +async fn get_reads_back_what_put_wrote() { + let (_memory, provider) = ScoredMemory::provider(); + let sections = Sections::new(&provider); + sections + .learnings() + .put( + "rust-async", + "pin", + "pin is not unpin", + MemoryCategory::Core, + None, + MemoryTaint::Internal, + ) + .await + .unwrap(); + + let found = sections.learnings().get("rust-async", "pin").await.unwrap(); + assert_eq!(found.unwrap().content, "pin is not unpin"); +} + +#[tokio::test] +async fn each_named_section_is_isolated_from_the_others() { + let (_memory, provider) = ScoredMemory::provider(); + let sections = Sections::new(&provider); + sections + .documents() + .put( + "handbook", + "k", + "doc", + MemoryCategory::Core, + None, + MemoryTaint::Internal, + ) + .await + .unwrap(); + + // Same scope and key, a different section: not visible. + assert!(sections + .conversations() + .get("handbook", "k") + .await + .unwrap() + .is_none()); + assert!(sections + .learnings() + .get("handbook", "k") + .await + .unwrap() + .is_none()); +} + +#[tokio::test] +async fn forget_is_idempotent() { + let (_memory, provider) = ScoredMemory::provider(); + let sections = Sections::new(&provider); + sections + .documents() + .put( + "handbook", + "k", + "doc", + MemoryCategory::Core, + None, + MemoryTaint::Internal, + ) + .await + .unwrap(); + + assert!(sections.documents().forget("handbook", "k").await.unwrap()); + assert!(!sections.documents().forget("handbook", "k").await.unwrap()); +} + +#[tokio::test] +async fn an_empty_scope_is_rejected_without_storing() { + let (memory, provider) = ScoredMemory::provider(); + let err = Sections::new(&provider) + .conversations() + .put( + "", + "turn-1", + "hello", + MemoryCategory::Core, + None, + MemoryTaint::Internal, + ) + .await + .expect_err("an empty scope cannot form a namespace"); + + assert!(matches!(err, MemoryError::Invalid(_)), "got {err:?}"); + assert!( + memory.rows().is_empty(), + "a rejected put must store nothing" + ); +} + +#[tokio::test] +async fn an_overlong_scope_is_rejected() { + let (_memory, provider) = ScoredMemory::provider(); + let err = Sections::new(&provider) + .learnings() + .namespace(&"x".repeat(500)) + .expect_err("an overlong scope cannot form a namespace"); + assert!(matches!(err, MemoryError::Invalid(_)), "got {err:?}"); +} + +#[tokio::test] +async fn scopes_lists_only_this_sections_namespaces() { + let (memory, provider) = ScoredMemory::provider(); + memory.seed("conversation:one", "a", "x", None); + memory.seed("conversation:two", "b", "x", None); + memory.seed("learning:rust", "c", "x", None); + memory.seed("research-notes", "d", "x", None); // unsectioned, legacy + memory.seed("ops:deploys", "e", "x", None); // a Custom section + + let sections = Sections::new(&provider); + let mut conversations: Vec = sections + .conversations() + .scopes() + .await + .unwrap() + .iter() + .map(|s| s.scope().to_string()) + .collect(); + conversations.sort(); + assert_eq!(conversations, ["one", "two"]); + + let learnings = sections.learnings().scopes().await.unwrap(); + assert_eq!(learnings.len(), 1); + assert_eq!(learnings[0].scope(), "rust"); + + // A custom section is reachable, and is never confused for a known one. + let ops = sections + .section(&MemorySection::Custom("ops".to_string())) + .scopes() + .await + .unwrap(); + assert_eq!(ops.len(), 1); + assert_eq!(ops[0].scope(), "deploys"); +} + +#[tokio::test] +async fn scopes_orders_by_entry_count_descending() { + let (memory, provider) = ScoredMemory::provider(); + memory.seed("learning:small", "a", "x", None); + memory.seed("learning:big", "b", "x", None); + memory.seed("learning:big", "c", "x", None); + + let scopes = Sections::new(&provider).learnings().scopes().await.unwrap(); + let ordered: Vec<&str> = scopes.iter().map(super::SectionScope::scope).collect(); + assert_eq!(ordered, ["big", "small"]); +} + +#[tokio::test] +async fn list_section_spans_every_scope_in_the_section() { + let (memory, provider) = ScoredMemory::provider(); + memory.seed("learning:a", "one", "x", None); + memory.seed("learning:b", "two", "x", None); + memory.seed("conversation:c", "three", "x", None); + + let entries = Sections::new(&provider) + .learnings() + .list_section(None, None) + .await + .unwrap(); + let keys: Vec<&str> = entries.iter().map(|e| e.key.as_str()).collect(); + assert_eq!(keys, ["one", "two"], "ordered by namespace then key"); +} + +// ------------------------------------------------------------- SectionRecall + +fn opts() -> OwnedRecallOpts { + OwnedRecallOpts::default() +} + +#[tokio::test] +async fn in_scope_recall_is_confined_to_one_namespace() { + let (memory, provider) = ScoredMemory::provider(); + memory.seed("learning:rust", "a", "shipping async", Some(0.9)); + memory.seed("learning:go", "b", "shipping async", Some(0.9)); + + let found = Sections::new(&provider) + .recall() + .in_scope( + &MemorySection::Learning, + "rust", + "shipping", + 10, + &opts(), + None, + ) + .await + .unwrap(); + + assert_eq!(found.namespaces_searched, 1); + assert_eq!(found.hits.len(), 1); + assert_eq!(found.hits[0].namespace.as_deref(), Some("learning:rust")); +} + +#[tokio::test] +async fn in_scope_rejects_recall_options_that_pin_a_namespace() { + let (_memory, provider) = ScoredMemory::provider(); + let pinned = OwnedRecallOpts { + namespace: Some("learning:elsewhere".to_string()), + ..OwnedRecallOpts::default() + }; + + let err = Sections::new(&provider) + .recall() + .in_scope(&MemorySection::Learning, "rust", "q", 10, &pinned, None) + .await + .expect_err("a pinned namespace conflicts with the section"); + + match err { + MemoryError::Invalid(message) => assert_eq!(message, NAMESPACE_FILTER_CONFLICT), + other => panic!("expected Invalid, got {other:?}"), + } +} + +#[tokio::test] +async fn in_scope_rejects_cross_session_outside_the_conversation_section() { + let (_memory, provider) = ScoredMemory::provider(); + let cross_session = OwnedRecallOpts { + cross_session: true, + ..OwnedRecallOpts::default() + }; + + let err = Sections::new(&provider) + .recall() + .in_scope( + &MemorySection::Learning, + "rust", + "q", + 10, + &cross_session, + None, + ) + .await + .expect_err("cross_session only means something for conversations"); + + match err { + MemoryError::Invalid(message) => { + assert_eq!(message, CROSS_SESSION_SECTION_CONFLICT); + } + other => panic!("expected Invalid, got {other:?}"), + } +} + +#[tokio::test] +async fn in_scope_allows_cross_session_on_the_conversation_section() { + let (memory, provider) = ScoredMemory::provider(); + memory.seed("conversation:chat-a", "one", "hello", Some(0.5)); + let cross_session = OwnedRecallOpts { + cross_session: true, + ..OwnedRecallOpts::default() + }; + + let found = Sections::new(&provider) + .recall() + .in_scope( + &MemorySection::Conversation, + "chat-a", + "hello", + 10, + &cross_session, + None, + ) + .await + .unwrap(); + + assert_eq!(found.hits.len(), 1); +} + +#[tokio::test] +async fn in_scope_rejects_a_custom_alias_of_the_conversation_section_with_cross_session() { + // `Custom("conversation")` and `MemorySection::Conversation` are the same + // view (see `a_custom_section_spelling_a_known_prefix_is_the_same_view`), + // so the cross_session guard must normalise before checking — it must + // *not* reject this the way it rejects a genuinely different section. + let (memory, provider) = ScoredMemory::provider(); + memory.seed("conversation:chat-a", "one", "hello", Some(0.5)); + let cross_session = OwnedRecallOpts { + cross_session: true, + ..OwnedRecallOpts::default() + }; + + let found = Sections::new(&provider) + .recall() + .in_scope( + &MemorySection::Custom("conversation".to_string()), + "chat-a", + "hello", + 10, + &cross_session, + None, + ) + .await + .expect("a custom alias of Conversation must be treated as Conversation"); + + assert_eq!(found.hits.len(), 1); +} + +#[tokio::test] +async fn in_scope_rejects_session_id_outside_the_conversation_section() { + let (_memory, provider) = ScoredMemory::provider(); + let session_scoped = OwnedRecallOpts { + session_id: Some("session-a".to_string()), + ..OwnedRecallOpts::default() + }; + + let err = Sections::new(&provider) + .recall() + .in_scope( + &MemorySection::Document, + "brief", + "q", + 10, + &session_scoped, + None, + ) + .await + .expect_err("session_id triggers the same episodic augmentation as cross_session"); + + match err { + MemoryError::Invalid(message) => { + assert_eq!(message, CROSS_SESSION_SECTION_CONFLICT); + } + other => panic!("expected Invalid, got {other:?}"), + } +} + +#[tokio::test] +async fn across_section_rejects_cross_session_even_on_the_conversation_section() { + // Unlike `in_scope`, `across_section` refuses cross_session on *every* + // section — including Conversation — because the fan-out would repeat the + // driver's cross-session rows once per scope. + let (_memory, provider) = ScoredMemory::provider(); + let cross_session = OwnedRecallOpts { + cross_session: true, + ..OwnedRecallOpts::default() + }; + + for section in [MemorySection::Document, MemorySection::Conversation] { + let err = Sections::new(&provider) + .recall() + .across_section(§ion, "q", 10, &cross_session, None) + .await + .expect_err("across_section never allows cross_session, even for conversations"); + + match err { + MemoryError::Invalid(message) => { + assert_eq!(message, CROSS_SESSION_FAN_OUT_CONFLICT); + } + other => panic!("expected Invalid, got {other:?}"), + } + } +} + +#[tokio::test] +async fn across_section_rejects_session_id_on_every_section() { + let (_memory, provider) = ScoredMemory::provider(); + let session_scoped = OwnedRecallOpts { + session_id: Some("session-a".to_string()), + ..OwnedRecallOpts::default() + }; + + let err = Sections::new(&provider) + .recall() + .across_section(&MemorySection::Conversation, "q", 10, &session_scoped, None) + .await + .expect_err("across_section never allows session_id, even for conversations"); + + match err { + MemoryError::Invalid(message) => { + assert_eq!(message, CROSS_SESSION_FAN_OUT_CONFLICT); + } + other => panic!("expected Invalid, got {other:?}"), + } +} + +#[tokio::test] +async fn across_section_merges_every_scope_and_ranks_by_score() { + let (memory, provider) = ScoredMemory::provider(); + memory.seed("learning:rust", "mid", "async", Some(0.5)); + memory.seed("learning:go", "high", "async", Some(0.9)); + memory.seed("learning:zig", "low", "async", Some(0.1)); + memory.seed("conversation:chat", "other", "async", Some(1.0)); + + let found = Sections::new(&provider) + .recall() + .across_section(&MemorySection::Learning, "async", 10, &opts(), None) + .await + .unwrap(); + + assert_eq!(found.namespaces_searched, 3); + assert!(!found.truncated); + let keys: Vec<&str> = found.hits.iter().map(|e| e.key.as_str()).collect(); + assert_eq!( + keys, + ["high", "mid", "low"], + "ranked across namespaces, and never crossing into another section" + ); +} + +#[tokio::test] +async fn across_section_truncates_the_hits_to_the_limit() { + let (memory, provider) = ScoredMemory::provider(); + memory.seed("learning:a", "high", "async", Some(0.9)); + memory.seed("learning:b", "low", "async", Some(0.1)); + + let found = Sections::new(&provider) + .recall() + .across_section(&MemorySection::Learning, "async", 1, &opts(), None) + .await + .unwrap(); + + assert_eq!(found.hits.len(), 1); + assert_eq!(found.hits[0].key, "high", "the limit keeps the best hit"); + assert!( + !found.truncated, + "reaching the hit limit is not namespace truncation" + ); +} + +#[tokio::test] +async fn across_section_reports_truncation_past_the_namespace_cap() { + let (memory, provider) = ScoredMemory::provider(); + for index in 0..=MAX_SECTION_NAMESPACES { + memory.seed( + &format!("learning:topic-{index:03}"), + "k", + "async", + Some(0.5), + ); + } + + let found = Sections::new(&provider) + .recall() + .across_section(&MemorySection::Learning, "async", 1000, &opts(), None) + .await + .unwrap(); + + assert_eq!(found.namespaces_searched, MAX_SECTION_NAMESPACES); + assert!(found.truncated, "one namespace was skipped"); +} + +#[tokio::test] +async fn across_section_rejects_recall_options_that_pin_a_namespace() { + let (_memory, provider) = ScoredMemory::provider(); + let pinned = OwnedRecallOpts { + namespace: Some("learning:elsewhere".to_string()), + ..OwnedRecallOpts::default() + }; + + let err = Sections::new(&provider) + .recall() + .across_section(&MemorySection::Learning, "q", 10, &pinned, None) + .await + .expect_err("a pinned namespace conflicts with the section"); + + assert!(matches!(err, MemoryError::Invalid(_)), "got {err:?}"); +} + +#[tokio::test] +async fn across_section_on_an_empty_section_is_ok_and_empty() { + let (_memory, provider) = ScoredMemory::provider(); + let found = Sections::new(&provider) + .recall() + .across_section(&MemorySection::Learning, "async", 10, &opts(), None) + .await + .unwrap(); + + assert!(found.hits.is_empty()); + assert_eq!(found.namespaces_searched, 0); + assert!(!found.truncated); +} + +// ------------------------------------------------- works on any provider + +#[tokio::test] +async fn the_whole_surface_succeeds_on_a_provider_that_retains_nothing() { + let provider = NullMemoryProvider::new(); + let sections = Sections::new(&provider); + + for view in [ + sections.conversations(), + sections.learnings(), + sections.documents(), + ] { + view.put( + "scope", + "key", + "content", + MemoryCategory::Core, + None, + MemoryTaint::Internal, + ) + .await + .expect("put must succeed"); + assert!(view.get("scope", "key").await.expect("get").is_none()); + assert!(!view.forget("scope", "key").await.expect("forget")); + assert!(view + .list("scope", None, None) + .await + .expect("list") + .is_empty()); + assert!(view.scopes().await.expect("scopes").is_empty()); + assert!(view + .list_section(None, None) + .await + .expect("list_section") + .is_empty()); + } + + let found = sections + .recall() + .across_section(&MemorySection::Conversation, "q", 10, &opts(), None) + .await + .expect("across_section must succeed"); + assert!(found.hits.is_empty()); + assert_eq!(found.namespaces_searched, 0); +} + +// ------------------------------------------------ section normalisation + +#[tokio::test] +async fn a_custom_section_spelling_a_known_prefix_is_the_same_view() { + let (memory, provider) = ScoredMemory::provider(); + let sections = Sections::new(&provider); + let aliased = MemorySection::Custom("conversation".to_string()); + + // A write through the aliased spelling lands in the real section... + let namespace = sections + .section(&aliased) + .put( + "thread-1", + "turn-1", + "hello", + MemoryCategory::Core, + None, + MemoryTaint::Internal, + ) + .await + .unwrap(); + assert_eq!(namespace.as_str(), "conversation:thread-1"); + assert_eq!( + memory.rows()[0].namespace.as_deref(), + Some("conversation:thread-1") + ); + + // ...and every enumerating path sees it, through either spelling. Before + // the section was normalised at construction, these two disagreed: the + // write landed in `conversation:` while the aliased view reported nothing. + assert_eq!( + sections.section(&aliased).scopes().await.unwrap(), + sections.conversations().scopes().await.unwrap() + ); + assert_eq!(sections.section(&aliased).scopes().await.unwrap().len(), 1); + assert_eq!( + sections.conversations().section(), + &MemorySection::Conversation + ); +} + +#[tokio::test] +async fn an_invalid_custom_prefix_errors_rather_than_reporting_an_empty_section() { + let (_memory, provider) = ScoredMemory::provider(); + let sections = Sections::new(&provider); + let bad = MemorySection::Custom("Bad Name".to_string()); + + // The addressed path rejects it... + assert!(matches!( + sections + .section(&bad) + .get("scope", "key") + .await + .expect_err("an invalid prefix cannot form a namespace"), + MemoryError::Invalid(_) + )); + + // ...and so must every enumerating path, rather than answering "empty". + for outcome in [ + sections.section(&bad).scopes().await.err(), + sections.section(&bad).list_section(None, None).await.err(), + Sections::new(&provider) + .recall() + .across_section(&bad, "q", 10, &opts(), None) + .await + .err(), + ] { + assert!( + matches!(outcome, Some(MemoryError::Invalid(_))), + "an unusable section must not look empty: {outcome:?}" + ); + } +} + +// ------------------------------------------------ pathological scores + +#[test] +fn merge_sorts_non_finite_scores_with_the_absent_ones() { + let merged = merge_hits( + vec![ + entry("learning:a", "nan", "x", Some(f64::NAN)), + entry("learning:b", "real", "x", Some(0.2)), + entry("learning:c", "infinite", "x", Some(f64::INFINITY)), + entry("learning:d", "absent", "x", None), + ], + 10, + ); + let keys: Vec<&str> = merged.iter().map(|e| e.key.as_str()).collect(); + assert_eq!( + keys[0], "real", + "a real score must outrank every non-finite one, got {keys:?}" + ); + assert_eq!(merged.len(), 4, "nothing is dropped, only ranked"); +} + +// ------------------------------------- the per-namespace limit is the full one + +#[tokio::test] +async fn across_section_asks_each_namespace_for_the_full_limit() { + let (memory, provider) = ScoredMemory::provider(); + // Three strong hits in one namespace, one weak hit in another. A per- + // namespace share of the limit (3 / 2 = 1) would return the weak hit; + // the full limit ranks it out. + memory.seed("learning:deep", "a", "async", Some(0.9)); + memory.seed("learning:deep", "b", "async", Some(0.8)); + memory.seed("learning:deep", "c", "async", Some(0.7)); + memory.seed("learning:shallow", "z", "async", Some(0.1)); + + let found = Sections::new(&provider) + .recall() + .across_section(&MemorySection::Learning, "async", 3, &opts(), None) + .await + .unwrap(); + + let keys: Vec<&str> = found.hits.iter().map(|e| e.key.as_str()).collect(); + assert_eq!( + keys, + ["a", "b", "c"], + "a share of the limit would have let the weak hit in" + ); +} + +// ------------------------------------------------ remaining failure paths + +#[tokio::test] +async fn in_scope_rejects_a_scope_that_cannot_form_a_namespace() { + let (_memory, provider) = ScoredMemory::provider(); + let err = Sections::new(&provider) + .recall() + .in_scope(&MemorySection::Learning, "", "q", 10, &opts(), None) + .await + .expect_err("an empty scope cannot form a namespace"); + + match err { + MemoryError::Invalid(message) => assert_ne!( + message, NAMESPACE_FILTER_CONFLICT, + "an invalid scope must not be reported as a filter conflict" + ), + other => panic!("expected Invalid, got {other:?}"), + } +} + +#[tokio::test] +async fn a_source_scoped_recall_propagates_the_drivers_refusal() { + let (_memory, provider) = ScoredMemory::provider(); + let sources = SourceScope::default(); + + // A mandatory-composed driver cannot apply the predicate internally, so it + // refuses. The façade passes that through rather than pre-empting it. + let err = Sections::new(&provider) + .recall() + .in_scope( + &MemorySection::Learning, + "rust", + "q", + 10, + &opts(), + Some(&sources), + ) + .await + .expect_err("the driver refuses a scoped recall"); + assert!(matches!(err, MemoryError::Invalid(_)), "got {err:?}"); +} + +#[tokio::test] +async fn scopes_drops_a_namespace_the_convention_cannot_parse() { + let (memory, provider) = ScoredMemory::provider(); + memory.seed("learning:good", "a", "x", None); + memory.seed("learning:has a space", "b", "x", None); + memory.seed(&format!("learning:{}", "x".repeat(300)), "c", "x", None); + + let scopes = Sections::new(&provider).learnings().scopes().await.unwrap(); + let names: Vec<&str> = scopes.iter().map(super::SectionScope::scope).collect(); + assert_eq!( + names, + ["good"], + "one malformed name must not fail the whole section" + ); +} + +#[tokio::test] +async fn list_filters_by_category_and_session() { + let (_memory, provider) = ScoredMemory::provider(); + let sections = Sections::new(&provider); + sections + .learnings() + .put( + "rust", + "core-a", + "x", + MemoryCategory::Core, + Some("session-1"), + MemoryTaint::Internal, + ) + .await + .unwrap(); + sections + .learnings() + .put( + "rust", + "core-b", + "x", + MemoryCategory::Core, + Some("session-2"), + MemoryTaint::Internal, + ) + .await + .unwrap(); + + assert_eq!( + sections + .learnings() + .list("rust", None, None) + .await + .unwrap() + .len(), + 2 + ); + assert_eq!( + sections + .learnings() + .list("rust", Some(&MemoryCategory::Core), Some("session-1")) + .await + .unwrap() + .len(), + 1 + ); + assert!(sections + .learnings() + .list("rust", None, Some("session-3")) + .await + .unwrap() + .is_empty()); +} diff --git a/crates/tinymemory/src/sections/types.rs b/crates/tinymemory/src/sections/types.rs new file mode 100644 index 00000000..2297b35e --- /dev/null +++ b/crates/tinymemory/src/sections/types.rs @@ -0,0 +1,162 @@ +//! The value types the section surface returns, and the two constants that +//! bound and explain its one non-obvious behaviour. +//! +//! Kept apart from [`SectionView`](super::SectionView) and +//! [`SectionRecall`](super::SectionRecall) because they are what a caller +//! *stores* — a [`SectionScope`] outlives the view that produced it, whereas +//! the handles borrow the provider and cannot. + +use tinymemory_api::namespace::Namespace; +use tinymemory_api::types::MemoryEntry; + +/// How many namespaces [`SectionRecall::across_section`] visits before it stops. +/// +/// A section-wide recall costs one provider call per namespace in the section, +/// so an unbounded fan-out would let a store that has accumulated thousands of +/// conversations turn one call into thousands. The cap trades completeness for a +/// predictable ceiling and reports when it bit, through +/// [`SectionHits::truncated`] — rather than silently returning a partial answer +/// that looks complete. +/// +/// [`SectionRecall::across_section`]: super::SectionRecall::across_section +pub const MAX_SECTION_NAMESPACES: usize = 64; + +/// The message carried by the [`MemoryError::Invalid`] that +/// [`SectionRecall`](super::SectionRecall) returns when the caller's recall +/// options already pin a namespace. +/// +/// A section recall derives the namespace itself — from the section and, for +/// [`in_scope`](super::SectionRecall::in_scope), from the scope. Honouring a +/// caller's `namespace` too would mean either ignoring one of the two filters or +/// intersecting them into an empty result, so the conflict is refused instead. +/// +/// Exposed as a constant, following the precedent of the contract's own +/// `SCOPE_UNAPPLIED`, so a caller's test asserts the same string the caller sees. +/// +/// [`MemoryError::Invalid`]: tinymemory_api::error::MemoryError::Invalid +pub const NAMESPACE_FILTER_CONFLICT: &str = + "recall options must not set a namespace: the section surface derives it"; + +/// The message carried by the [`MemoryError::Invalid`] that +/// [`SectionRecall::in_scope`](super::SectionRecall::in_scope) returns when the +/// caller asks for `cross_session` or `session_id`-scoped recall on a section +/// other than +/// [`Conversation`](tinymemory_api::namespace::MemorySection::Conversation). +/// +/// The bundled `UnifiedMemory` driver's `cross_session` and `session_id` +/// options both append *episodic conversational* rows independently of the +/// pinned namespace, then relabel every such row with whichever namespace the +/// call was pinned to (`crates/tinymemory-core/src/store/memory_trait.rs`) — +/// it has no concept of "cross-session" or "session-scoped" for documents or +/// learnings. Honouring either option on a non-conversation section would +/// therefore return conversational content mislabeled as document or learning +/// hits. Refused outright rather than silently misrepresented. +/// +/// The section this checks against is the **normalised** one — the same one +/// [`SectionView::new`](super::SectionView::new) derives through +/// [`MemorySection::from_prefix`](tinymemory_api::namespace::MemorySection::from_prefix) +/// — so `Custom("conversation")` is treated exactly like +/// [`Conversation`](tinymemory_api::namespace::MemorySection::Conversation), +/// matching every other method on this surface. +/// +/// [`MemoryError::Invalid`]: tinymemory_api::error::MemoryError::Invalid +/// [`MemorySection`]: tinymemory_api::namespace::MemorySection +pub const CROSS_SESSION_SECTION_CONFLICT: &str = + "cross-session and session-scoped recall are only meaningful for the conversation section"; + +/// The message carried by the [`MemoryError::Invalid`] that +/// [`SectionRecall::across_section`](super::SectionRecall::across_section) +/// returns when the caller sets `cross_session` or `session_id`, on *any* +/// section including [`Conversation`](tinymemory_api::namespace::MemorySection::Conversation). +/// +/// Unlike [`CROSS_SESSION_SECTION_CONFLICT`], this is refused unconditionally, +/// because the problem is the fan-out itself rather than the section: the +/// bundled driver's cross-session and session-scoped augmentation runs once, +/// independent of the pinned namespace, and `across_section` issues one call +/// per scope — so the exact same extra rows would be appended once per scope, +/// repeating in the merged result and crowding out genuine hits before +/// `limit` truncates them. A caller who wants cross-session or session-scoped +/// recall should use [`SectionRecall::in_scope`](super::SectionRecall::in_scope) +/// instead, which issues exactly one call. +/// +/// [`MemoryError::Invalid`]: tinymemory_api::error::MemoryError::Invalid +pub const CROSS_SESSION_FAN_OUT_CONFLICT: &str = + "cross-session and session-scoped recall are not supported by across_section: use in_scope instead"; + +/// One namespace within a section, as [`SectionView::scopes`] reports it. +/// +/// [`SectionView::scopes`]: super::SectionView::scopes +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SectionScope { + /// The parsed namespace, section prefix included. + pub namespace: Namespace, + /// How many entries it holds. + pub entries: usize, + /// RFC 3339 timestamp of its most recent update, when the driver tracks one. + pub last_updated: Option, +} + +impl SectionScope { + /// The scope — the part after the section prefix. + /// + /// This is the string every [`SectionView`](super::SectionView) method + /// takes, so a scope discovered here can be passed straight back in. + #[must_use] + pub fn scope(&self) -> &str { + self.namespace.scope() + } +} + +/// What a section-wide recall found, and how much of the section it saw. +/// +/// The two non-hit fields exist so a caller can tell "the section holds nothing +/// matching" from "the fan-out stopped early", which a bare `Vec` cannot express. +#[derive(Debug, Clone, Default)] +pub struct SectionHits { + /// The merged hits, most relevant first. + pub hits: Vec, + /// How many namespaces were actually searched. + pub namespaces_searched: usize, + /// Whether [`MAX_SECTION_NAMESPACES`] stopped the fan-out short. + /// + /// This says namespaces were *skipped*. It never means the hits themselves + /// were truncated to the caller's limit, which is expected and ordinary. + pub truncated: bool, +} + +/// The score a hit sorts on, with absent and non-finite scores ordering last. +/// +/// Absent scores map to negative infinity rather than zero: a driver that scores +/// nothing would otherwise have its hits outrank genuinely poor matches. +/// +/// `NaN` and the infinities are folded in with them. `f64::total_cmp` would +/// order them deterministically on its own, but it ranks `+NaN` *above* `+inf` — +/// so one `NaN` from a misbehaving driver would quietly outrank every real hit. +/// Treating a score that is not a finite number as no score at all is the same +/// judgement, applied consistently. +fn sort_score(entry: &MemoryEntry) -> f64 { + match entry.score { + Some(score) if score.is_finite() => score, + _ => f64::NEG_INFINITY, + } +} + +/// Merge hits gathered from several namespaces into one ranked, bounded list. +/// +/// Ordering is score descending, absent scores last, ties broken by namespace +/// then key — total and deterministic, so a fixed store always yields the same +/// answer. It is total because `(namespace, key)` is the store's primary key, so +/// two distinct entries can never compare equal and the sort's stability is +/// never load-bearing. `total_cmp` is used rather than `partial_cmp` because +/// `partial_cmp` returns `None` for `NaN` and would poison the comparator; +/// `sort_score` has already folded the non-finite cases in with absent scores. +pub(super) fn merge_hits(mut hits: Vec, limit: usize) -> Vec { + hits.sort_by(|a, b| { + sort_score(b) + .total_cmp(&sort_score(a)) + .then_with(|| a.namespace.cmp(&b.namespace)) + .then_with(|| a.key.cmp(&b.key)) + }); + hits.truncate(limit); + hits +} diff --git a/crates/tinymemory/src/sections/view.rs b/crates/tinymemory/src/sections/view.rs new file mode 100644 index 00000000..e5604219 --- /dev/null +++ b/crates/tinymemory/src/sections/view.rs @@ -0,0 +1,248 @@ +//! [`SectionView`] — one section's slice of a provider, addressed by scope. + +use std::fmt; + +use tinymemory_api::error::MemoryError; +use tinymemory_api::namespace::{MemorySection, Namespace}; +use tinymemory_api::provider::MemoryProvider; +use tinymemory_api::types::{MemoryCategory, MemoryEntry, MemoryTaint}; + +use super::types::SectionScope; + +/// A borrowing handle onto one [`MemorySection`] of a provider. +/// +/// Every method takes the **scope** — `"thread-8f21"`, not +/// `"conversation:thread-8f21"` — and builds the namespace itself, so a caller +/// never spells the convention out and a scope that cannot form a valid +/// namespace is refused before anything is written. +/// +/// The handle borrows rather than owning an `Arc`, matching `DocumentIntake`: +/// it is cheap to make, cheap to drop, and cannot outlive the provider it reads. +#[derive(Clone)] +pub struct SectionView<'a> { + provider: &'a dyn MemoryProvider, + section: MemorySection, +} + +impl fmt::Debug for SectionView<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("SectionView") + .field("section", &self.section.as_str()) + .field("driver_id", &self.provider.driver_id()) + .finish() + } +} + +impl<'a> SectionView<'a> { + /// Bind `section` of `provider`. + /// + /// The section is **normalised** through [`MemorySection::from_prefix`], so + /// `Custom("conversation")` becomes [`MemorySection::Conversation`] and the + /// two name one view rather than two. `Namespace::new` normalises the same + /// way and for the same reason; storing the caller's spelling verbatim would + /// mean a write landed in `conversation:` while [`Self::scopes`] — which + /// compares against this field — reported the section as empty. + /// + /// [`Sections::section`]: super::Sections::section + #[must_use] + pub fn new(provider: &'a dyn MemoryProvider, section: &MemorySection) -> Self { + Self { + provider, + section: MemorySection::from_prefix(section.as_str()), + } + } + + /// Fail when this view's section cannot form a namespace at all. + /// + /// The addressed methods get this for free, because each builds the + /// namespace it needs. The enumerating ones build none, so without this + /// check a section whose prefix fails validation would report an *empty* + /// section instead of an error — and [`SectionRecall::across_section`] and + /// [`SectionRecall::in_scope`] would disagree about the same section. + /// + /// [`SectionRecall::across_section`]: super::SectionRecall::across_section + /// [`SectionRecall::in_scope`]: super::SectionRecall::in_scope + fn validate_section(&self) -> Result<(), MemoryError> { + self.namespace("probe").map(|_| ()) + } + + /// The section this view is bound to. + #[must_use] + pub fn section(&self) -> &MemorySection { + &self.section + } + + /// The namespace `scope` names within this section. + /// + /// Useful on its own for a caller that needs the string a write *would* + /// touch — an audit line, a log field — without performing the write. + /// + /// # Errors + /// + /// [`MemoryError::Invalid`] when the section and scope cannot form a valid + /// namespace: an empty scope, a disallowed character, or a rendered name + /// over the convention's length limit. + pub fn namespace(&self, scope: &str) -> Result { + Namespace::new(self.section.clone(), scope) + } + + /// Store one entry under `scope`, returning the namespace it landed in. + /// + /// The parameters after `key` mirror `MemoryCore::store` exactly, so what + /// this adds over the raw call is visible: the namespace, and nothing else. + /// + /// Returns the [`Namespace`] rather than `()` so a caller that wants to log + /// or audit where the entry went does not have to re-derive it. + /// + /// # Errors + /// + /// [`MemoryError::Invalid`] for a scope [`Self::namespace`] rejects — in + /// which case **nothing is stored** — otherwise whatever the backend returns. + pub async fn put( + &self, + scope: &str, + key: &str, + content: &str, + category: MemoryCategory, + session_id: Option<&str>, + taint: MemoryTaint, + ) -> Result { + let namespace = self.namespace(scope)?; + self.provider + .store( + namespace.as_str(), + key, + content, + category, + session_id, + taint, + ) + .await?; + Ok(namespace) + } + + /// Read one entry back by `(scope, key)`. + /// + /// # Errors + /// + /// As [`Self::put`]. A missing entry is `Ok(None)`, not an error. + pub async fn get(&self, scope: &str, key: &str) -> Result, MemoryError> { + let namespace = self.namespace(scope)?; + self.provider.get(namespace.as_str(), key).await + } + + /// Delete one entry, reporting whether it existed. + /// + /// # Errors + /// + /// As [`Self::put`]. Forgetting an absent entry is `Ok(false)`. + pub async fn forget(&self, scope: &str, key: &str) -> Result { + let namespace = self.namespace(scope)?; + self.provider.forget(namespace.as_str(), key).await + } + + /// List the entries in one scope. + /// + /// # Errors + /// + /// As [`Self::put`]. + pub async fn list( + &self, + scope: &str, + category: Option<&MemoryCategory>, + session_id: Option<&str>, + ) -> Result, MemoryError> { + let namespace = self.namespace(scope)?; + self.provider + .list(Some(namespace.as_str()), category, session_id) + .await + } + + /// Every scope this section currently holds. + /// + /// Ordered by entry count descending, ties by namespace ascending — the same + /// order [`across_section`](super::SectionRecall::across_section) visits + /// them in, so the first + /// [`MAX_SECTION_NAMESPACES`](super::MAX_SECTION_NAMESPACES) rows here are + /// exactly the ones a section-wide recall would search. + /// + /// Size, not recency, and deliberately: [`SectionScope::last_updated`] is + /// optional and no bundled driver populates it, so ordering on it would be + /// ordering on `None` and the cap would stop being deterministic. A caller + /// who wants recency has the rows here and can sort them itself. + /// + /// Namespaces belonging to another section, and unsectioned namespaces left + /// over from before the convention existed, are excluded. So are namespaces + /// the convention cannot parse at all: a driver may hold names this + /// vocabulary does not admit, and refusing to list a section because one + /// unrelated name is malformed would be the wrong failure. + /// + /// # Errors + /// + /// [`MemoryError::Invalid`] when this view's section cannot form a valid + /// namespace, so that an unusable section is not mistaken for an empty one. + /// Otherwise whatever the backend returns from its namespace enumeration. + pub async fn scopes(&self) -> Result, MemoryError> { + self.validate_section()?; + let mut scopes: Vec = self + .provider + .namespaces() + .await? + .into_iter() + .filter_map(|summary| { + let namespace = Namespace::parse(&summary.namespace).ok()?; + if namespace.section() != Some(&self.section) { + return None; + } + Some(SectionScope { + namespace, + entries: summary.count, + last_updated: summary.last_updated, + }) + }) + .collect(); + scopes.sort_by(|a, b| { + b.entries + .cmp(&a.entries) + .then_with(|| a.namespace.cmp(&b.namespace)) + }); + Ok(scopes) + } + + /// List the entries across every scope of the section this view can see. + /// + /// "Can see" is the caveat [`Self::scopes`] documents: a namespace the + /// driver does not report, or one this convention cannot parse, is not here. + /// + /// Costs one namespace enumeration plus one `list` per scope, and unlike + /// [`across_section`](super::SectionRecall::across_section) it is **not capped** — a caller asking + /// to list a section gets all of it. Use [`Self::scopes`] and [`Self::list`] + /// to page through a large section under your own control. + /// + /// Ordered by namespace then key, so the result is deterministic even though + /// the per-scope order the driver returns is not specified. + /// + /// # Errors + /// + /// As [`Self::scopes`], plus whatever any per-scope `list` returns. + pub async fn list_section( + &self, + category: Option<&MemoryCategory>, + session_id: Option<&str>, + ) -> Result, MemoryError> { + let mut entries = Vec::new(); + for scope in self.scopes().await? { + entries.extend( + self.provider + .list(Some(scope.namespace.as_str()), category, session_id) + .await?, + ); + } + entries.sort_by(|a, b| { + a.namespace + .cmp(&b.namespace) + .then_with(|| a.key.cmp(&b.key)) + }); + Ok(entries) + } +} diff --git a/crates/tinymemory/tests/sections.rs b/crates/tinymemory/tests/sections.rs new file mode 100644 index 00000000..ac7b5d71 --- /dev/null +++ b/crates/tinymemory/tests/sections.rs @@ -0,0 +1,262 @@ +//! The section surface, exercised through the public API only. +//! +//! The unit tests use a double that can seed scores; this suite deliberately +//! cannot, and asserts only what a real caller can observe. `InMemoryProvider` +//! is the conformance crate's reference driver — a driver that actually retains +//! — and `NullMemoryProvider` is one that retains nothing, which is where the +//! "works on every driver" claim is machine-checked rather than asserted. + +// A failing assertion in a test *is* a panic; the crate-wide `expect_used` / +// `unwrap_used` / `panic` lints exist to keep the library from panicking, not +// the tests. Same allowance, and same reasoning, as `src/registry/test.rs`. +#![allow(clippy::expect_used, clippy::unwrap_used, clippy::panic)] + +use std::sync::Arc; + +use tinymemory::error::MemoryError; +use tinymemory::namespace::MemorySection; +use tinymemory::null::NullMemoryProvider; +use tinymemory::provider::MemoryProvider; +use tinymemory::recall::OwnedRecallOpts; +use tinymemory::sections::{Sections, NAMESPACE_FILTER_CONFLICT}; +use tinymemory::types::{MemoryCategory, MemoryTaint}; +use tinymemory_conformance::InMemoryProvider; + +fn retaining() -> Arc { + Arc::new(InMemoryProvider::new()) +} + +#[tokio::test] +async fn a_conversation_round_trips_through_the_section_surface() { + let provider = retaining(); + let sections = Sections::new(provider.as_ref()); + + let namespace = sections + .conversations() + .put( + "thread-8f21", + "turn-1", + "we agreed to ship on the 14th", + MemoryCategory::Core, + None, + MemoryTaint::Internal, + ) + .await + .expect("put"); + + assert_eq!(namespace.as_str(), "conversation:thread-8f21"); + + let entry = sections + .conversations() + .get("thread-8f21", "turn-1") + .await + .expect("get") + .expect("the entry must be there"); + assert_eq!(entry.content, "we agreed to ship on the 14th"); + + let scopes = sections.conversations().scopes().await.expect("scopes"); + assert_eq!(scopes.len(), 1); + assert_eq!(scopes[0].scope(), "thread-8f21"); + assert_eq!(scopes[0].entries, 1); + + assert!(sections + .conversations() + .forget("thread-8f21", "turn-1") + .await + .expect("forget")); + assert!(sections + .conversations() + .scopes() + .await + .expect("scopes") + .is_empty()); +} + +#[tokio::test] +async fn the_three_sections_do_not_see_each_others_entries() { + let provider = retaining(); + let sections = Sections::new(provider.as_ref()); + + for view in [ + sections.conversations(), + sections.learnings(), + sections.documents(), + ] { + view.put( + "shared-scope", + "shared-key", + "content", + MemoryCategory::Core, + None, + MemoryTaint::Internal, + ) + .await + .expect("put"); + } + + // Three sections, three namespaces, one entry each — not one shared row. + for view in [ + sections.conversations(), + sections.learnings(), + sections.documents(), + ] { + let scopes = view.scopes().await.expect("scopes"); + assert_eq!(scopes.len(), 1, "section {:?}", view.section()); + assert_eq!(scopes[0].entries, 1); + } +} + +#[tokio::test] +async fn a_section_recall_reaches_every_scope_in_that_section_only() { + let provider = retaining(); + let sections = Sections::new(provider.as_ref()); + + for scope in ["rust-async", "rust-macros"] { + sections + .learnings() + .put( + scope, + "note", + "borrow checker notes", + MemoryCategory::Core, + None, + MemoryTaint::Internal, + ) + .await + .expect("put"); + } + sections + .conversations() + .put( + "thread-1", + "note", + "borrow checker notes", + MemoryCategory::Core, + None, + MemoryTaint::Internal, + ) + .await + .expect("put"); + + let found = sections + .recall() + .across_section( + &MemorySection::Learning, + "borrow checker", + 10, + &OwnedRecallOpts::default(), + None, + ) + .await + .expect("across_section"); + + assert_eq!(found.namespaces_searched, 2); + assert!(!found.truncated); + assert_eq!(found.hits.len(), 2); + for hit in &found.hits { + let namespace = hit.namespace.as_deref().expect("a hit carries a namespace"); + assert!( + namespace.starts_with("learning:"), + "leaked out of the section: {namespace}" + ); + } +} + +#[tokio::test] +async fn recall_options_may_not_pin_a_namespace() { + let provider = retaining(); + let pinned = OwnedRecallOpts { + namespace: Some("learning:elsewhere".to_string()), + ..OwnedRecallOpts::default() + }; + + let err = Sections::new(provider.as_ref()) + .recall() + .across_section(&MemorySection::Learning, "q", 10, &pinned, None) + .await + .expect_err("a pinned namespace conflicts with the section"); + + match err { + MemoryError::Invalid(message) => assert_eq!(message, NAMESPACE_FILTER_CONFLICT), + other => panic!("expected Invalid, got {other:?}"), + } +} + +#[tokio::test] +async fn a_custom_section_is_a_first_class_citizen() { + let provider = retaining(); + let sections = Sections::new(provider.as_ref()); + let ops = MemorySection::Custom("ops".to_string()); + + let namespace = sections + .section(&ops) + .put( + "deploys", + "2026-01-01", + "rolled back", + MemoryCategory::Core, + None, + MemoryTaint::Internal, + ) + .await + .expect("put"); + assert_eq!(namespace.as_str(), "ops:deploys"); + + let scopes = sections.section(&ops).scopes().await.expect("scopes"); + assert_eq!(scopes.len(), 1); + assert_eq!(scopes[0].scope(), "deploys"); + + // …and it is not mistaken for one of the named sections. + assert!(sections + .documents() + .scopes() + .await + .expect("scopes") + .is_empty()); +} + +#[tokio::test] +async fn every_call_succeeds_on_a_driver_that_retains_nothing() { + let provider: Arc = Arc::new(NullMemoryProvider::new()); + let sections = Sections::new(provider.as_ref()); + + sections + .documents() + .put( + "handbook", + "k", + "content", + MemoryCategory::Core, + None, + MemoryTaint::Internal, + ) + .await + .expect("put must succeed even where nothing is retained"); + + assert!(sections + .documents() + .get("handbook", "k") + .await + .expect("get") + .is_none()); + assert!(sections + .documents() + .list_section(None, None) + .await + .expect("list_section") + .is_empty()); + + let found = sections + .recall() + .in_scope( + &MemorySection::Document, + "handbook", + "q", + 10, + &OwnedRecallOpts::default(), + None, + ) + .await + .expect("in_scope"); + assert!(found.hits.is_empty()); +} diff --git a/docs/plans/memory-section-api.md b/docs/plans/memory-section-api.md new file mode 100644 index 00000000..d75d01f6 --- /dev/null +++ b/docs/plans/memory-section-api.md @@ -0,0 +1,90 @@ +# Implementation plan: the Section API + +Specification: [`../specs/memory-section-api.md`](../specs/memory-section-api.md). + +## Goal + +Add `crates/tinymemory/src/sections/` — borrowing handles that give the +`conversation:`, `learning:` and `document:` sections a typed surface, plus a +section-wide recall built by fanning out over `namespaces()`. + +## Non-goals for implementation + +No HTTP. No change to any trait, driver, capability set or error enum. No +`section` field on `OwnedRecallOpts`. No edit under `vendor/`. + +## Assumptions + +- The façade crate is the home, not `tinymemory-api`: the contract crate carries + no `[lints]` table and is held byte-identical to its `tinycortex-api` origin, + so code required to document `# Errors` belongs where the lints run. +- `tinymemory_conformance::InMemoryProvider` is a public, retaining provider and + is already an unconditional dev-dependency of the façade. It is the behavioural + test double; `NullMemoryProvider` covers "retains nothing". +- `tokio` with `macros` and `rt-multi-thread` is already a dev-dependency, so the + doctest can be fully runnable. + +## Tasks + +Each task lands its tests first. + +1. **`src/sections/types.rs`** — `SectionScope`, `SectionHits`, + `MAX_SECTION_NAMESPACES`, `NAMESPACE_FILTER_CONFLICT`, and the private merge. + Tests: merge orders by score descending; absent scores sort last; ties break by + `(namespace, key)`. +2. **`src/sections/view.rs`, reads and writes** — `SectionView::{new, section, + namespace, put, get, forget, list}`. Tests: `put` writes under the section + prefix; `get` reads back what `put` wrote; `forget` is idempotent; an invalid + scope and an empty scope are both rejected without storing. +3. **`src/sections/view.rs`, section-wide** — `scopes`, `list_section`. Tests: + only this section's namespaces are listed; unsectioned namespaces are excluded; + a `Custom` section is never mistaken for a known one; both are empty on a + provider that retains nothing. +4. **`src/sections/recall.rs`, `in_scope`** — one exact-namespace recall. Tests: + confined to one namespace; `opts.namespace: Some(_)` returns + `MemoryError::Invalid` carrying `NAMESPACE_FILTER_CONFLICT`. +5. **`src/sections/recall.rs`, `across_section`** — the fan-out. Tests: merges + hits from every scope in the section; never returns another section's hit; + orders by score descending; reports `namespaces_searched`; sets `truncated` + only past the namespace cap; an empty store is `Ok` and empty. +6. **`src/sections/mod.rs`** — `Sections`, the module `//!` docs, and a runnable + doctest over `InMemoryProvider`. Wires `#[cfg(test)] mod test;`. +7. **`src/lib.rs`** — `pub mod sections;`, a bullet in the crate docs, and + `namespace` added to the contract re-export list, which omits it today. +8. **`tests/sections.rs`** — public-API-only regression: a round trip on + `InMemoryProvider`, and the same script on `NullMemoryProvider` asserting every + call is `Ok` and empty. +9. **Docs** — a subsection in the root `README.md` after `## The contract`, and + the section handles named in `examples/tinycortex.rs`. + +## Verification + +Focused, while iterating: + +```sh +cargo test -p tinymemory sections +cargo test --doc -p tinymemory +``` + +Full, before opening the pull request: + +```sh +cargo fmt --all -- --check +cargo clippy --all-targets --all-features -- -D warnings +cargo build --all-targets --all-features +cargo test --all-features +RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-features +cargo run -p tinymemory --features tinycortex --example tinycortex +``` + +## Completion checklist + +- [x] 1 `types.rs` and its tests +- [x] 2 `SectionView` reads and writes +- [x] 3 `scopes` and `list_section` +- [x] 4 `SectionRecall::in_scope` +- [x] 5 `SectionRecall::across_section` +- [x] 6 `Sections`, module docs, doctest +- [x] 7 `lib.rs` exports, including `namespace` +- [x] 8 integration tests +- [x] 9 README and example diff --git a/docs/specs/memory-section-api.md b/docs/specs/memory-section-api.md new file mode 100644 index 00000000..d739d3d0 --- /dev/null +++ b/docs/specs/memory-section-api.md @@ -0,0 +1,203 @@ +# The Section API: Conversations, Learnings, Documents, and Recall + +**Status:** Implemented +**Owner:** TinyMemory maintainers + +A typed surface for the three content sections the namespace convention already +names, plus a recall that can span one of them. + +## Problem + +`docs/specs/graph-view-and-document-intake.md` §3 established the +`
:` namespace convention and `MemorySection` implements it. What +it did not do is give anyone a reason to use it. Every namespace still crosses +the contract as a bare `&str`, so: + +1. **Callers concatenate prefixes by hand.** `"conversation:" + thread_id` is + written at every call site that wants conversational memory, and a typo + produces a valid, silently-wrong namespace rather than an error. The + convention is documented and then left to discipline. + +2. **There is no way to ask a section-wide question.** "Everything the agent has + learned" spans every `learning:*` namespace, and the contract offers no way to + express it. `MemoryCore::list` takes one exact namespace or none; + `MemoryRecall::recall` takes one exact namespace. + +3. **`namespace: None` means two different things on two bundled drivers.** It is + documented as falling back to `GLOBAL_NAMESPACE` (`recall.rs`), and the + embedded engine implements exactly that (`memory_trait.rs`) — but the + reference driver treats it as *all* namespaces + (`tinymemory-conformance/src/reference/mod.rs`). The conformance suite only + ever asserts the `Some` case, so nothing catches the divergence. + +The third is why the obvious implementation of the second does not work. "Recall +with no namespace filter, then keep the hits whose namespace is in the section" +returns everything on the reference driver and only the `global` namespace on +TinyCortex — correct in tests, empty in production. + +## Goals + +- One typed surface per section, working on **any** `MemoryProvider` through the + mandatory three families alone. +- A section-wide recall whose cost, ordering, and truncation are stated rather + than implied. +- No trait signature change, no new capability, no new error variant, and no + change to any driver. + +## Non-goals + +- **HTTP routes or a server crate.** This is a Rust surface. A host that wants + `/v1/conversations` builds it over this. +- **A `section` filter on `OwnedRecallOpts`.** Three blockers: field parity with + the borrowed `RecallOpts` is enforced by two exhaustive destructures and a + test, so it is two structs; eighteen construction sites, most of them struct + literals without `..Default::default()`; and `RecallOpts` literals exist in + `vendor/tinycortex`, which this repository must not edit. Above all, a filter + field is a promise every driver must implement, and one that ignored it would + silently return wrong results — the failure `audit_provider` exists to prevent. +- **Routing to optional capability families.** `documents()` here writes through + `MemoryCore`. Handing the layer a *file* is `DocumentIntake`'s job, and it + already routes between `MemoryIngest`, `MemoryDocuments` and `MemoryCore`. +- **Fixing the `namespace: None` divergence.** It is real and it needs a + conformance assertion plus a contract sentence. That is its own change; this + design is built to not depend on it. +- **An optional `query` on recall.** CortexDB's recall returns a filtered slice + when the query is absent. Here that is `list_section`, because the contract + already says an empty query yields `Ok(vec![])`. + +## Proposed behavior + +### 1. `Sections`, the entry point + +```rust +let sections = Sections::new(provider.as_ref()); + +sections.conversations().put("thread-8f21", "turn-3", text, category, None, taint).await?; +let learned = sections.learnings().scopes().await?; +let hits = sections.recall().across_section(&MemorySection::Learning, "async", 10, &opts, None).await?; +``` + +`Sections`, `SectionView` and `SectionRecall` are borrowing handles, not owners — +the same shape as `DocumentIntake`. They hold `&dyn MemoryProvider` and allocate +nothing but the namespace strings they must build anyway. + +`conversations()`, `learnings()` and `documents()` are named accessors returning +the same `SectionView` bound to a different `MemorySection`; `section()` reaches +the other four sections and `Custom`. One parameterised type rather than three +newtypes, because `MemorySection` is a closed vocabulary precisely so it can be a +value. + +### 2. `SectionView` — reads and writes within a section + +Every method takes the **scope** (`"thread-8f21"`), never the full namespace; the +handle applies the prefix through the existing `Namespace` constructors, so an +invalid scope is a `MemoryError::Invalid` before anything is written. + +| Method | Maps onto | +| --- | --- | +| `put` | `MemoryCore::store`, returning the `Namespace` it wrote | +| `get` / `forget` | `MemoryCore::{get, forget}` | +| `list` | `MemoryCore::list` for one scope | +| `list_section` | `MemoryCore::list` fanned out over the section | +| `scopes` | `MemoryCore::namespaces`, filtered to the section | + +`put` mirrors `MemoryCore::store`'s parameter order exactly, so the façade is +visibly thin. + +### 3. `SectionRecall` — `in_scope` and `across_section` + +`in_scope` is one `MemoryRecall::recall` against one exact namespace. + +`across_section` enumerates `namespaces()`, keeps the section's, recalls each with +an exact namespace, and merges. This is the same strategy the contract already +uses for `list(None, ..)` in `mandatory/mod.rs`, adopted for the same reason: a +naive delegation returns one namespace and calls it "everything". + +It promises, and its rustdoc states: + +- **Cost is `1 + N` provider calls**, `N` capped at `MAX_SECTION_NAMESPACES`. +- **Visit order** is by entry count descending, ties by namespace ascending, so + which namespaces the cap drops is deterministic. +- **Each namespace is asked for the full `limit`**, never a share of it: a share + would let one namespace's best hit lose to another's worst. +- **Merge order** is score descending, absent scores last, ties by + `(namespace, key)` ascending — then truncate to `limit`. Total, because + `(namespace, key)` is the store's primary key. Scores that are not finite + numbers rank with the absent ones rather than above every real hit. +- **Visit order is by size, not recency**, because `last_updated` is optional + and no bundled driver populates it; ordering on it would be ordering on + `None` and would cost the cap its determinism. +- **`truncated` means namespaces were skipped**, never that hits exceeded `limit`. +- **`opts.namespace` must be `None`.** `Some` is `MemoryError::Invalid` carrying + `NAMESPACE_FILTER_CONFLICT`, rather than a silent override of the caller's filter. +- **`opts.cross_session` and `opts.session_id` are refused outside the + conversation section**, with `CROSS_SESSION_SECTION_CONFLICT`, checked + against the section's *normalised* form so `Custom("conversation")` is + treated as `MemorySection::Conversation` here too. The bundled driver's + cross-session path surfaces *episodic* rows from other sessions, and its + `session_id` path independently appends that session's episodic rows; both + relabel every such row with whichever namespace the call pinned, so + honouring either on `learning:` or `document:` would return conversational + content presented as a learning or a document. +- **`opts.cross_session` and `opts.session_id` are refused on + `across_section` unconditionally**, including on the conversation section, + with `CROSS_SESSION_FAN_OUT_CONFLICT`. The driver's episodic augmentation + for either option runs once, independent of the pinned namespace, so the + fan-out would repeat the same rows once per scope, crowding genuine hits + out of `limit` before the fan-out over conversation scopes adds anything — + `across_section` already visits every conversation scope on its own. A + caller who wants cross-session or session-scoped recall uses `in_scope` + instead, which issues exactly one call. + +Scores come from separate calls to one driver with one query. They are comparable +in practice on every bundled driver; the contract does not guarantee it, and the +documentation says so rather than pretending otherwise. + +## Invariants and constraints + +- A `SectionView` never reads or writes a namespace outside its own section. +- A section is normalised at construction, so `Custom("conversation")` and + `Conversation` name one view and not two. Without this a write lands in + `conversation:` while the aliased view reports the section empty — the same + hazard `Namespace::new` normalises to prevent, one layer up. +- An unusable section is an error, never an empty one: if a section's prefix + fails validation, the enumerating calls fail rather than reporting no scopes, + so they agree with the addressed calls about the same section. +- `put` then `get` on the same `(scope, key)` round-trips on any retaining driver. +- Every call succeeds on a driver that retains nothing, returning empty rather + than an error — the surface has no capability-absent path. +- What a section returns belongs to that section. A recall option that would + make the driver surface another section's content under this section's + namespace is refused, not filtered afterwards. +- Results are deterministic given a fixed store, on every ordering the API exposes. +- An invalid scope fails before any write, so a rejected call stores nothing. +- No driver, trait signature, capability set, or error enum changes. + +## Acceptance criteria + +- The full surface works against `NullMemoryProvider`, returning `Ok` and empty. +- `cross_session` and `session_id` recall are each refused on every section + but `conversation:` at `in_scope`, and refused on `across_section` + unconditionally, including on `conversation:`. +- A round trip works against `InMemoryProvider` through the public API only. +- `across_section` returns no hit belonging to another section, orders by score + descending, reports `namespaces_searched`, and sets `truncated` only when the + namespace cap skipped one. +- `across_section` with `opts.namespace: Some(_)` returns `MemoryError::Invalid`. +- The four contract commands pass, and rustdoc builds with `-D warnings`. + +## Open questions + +- **One `SectionView` or three newtypes?** Newtypes would let + `conversations().append()` and `documents().put()` diverge in vocabulary. The + parameterised type is chosen for now; adding newtypes later is purely additive. +- **Positional `put`, or an `IntakeRequest`-style request struct?** Positional + mirrors `MemoryCore::store` and is thin; a struct would survive parameter growth. +- **Should an all-sections `everywhere()` exist?** Only as a fan-out over every + namespace. It cannot be built on `namespace: None` while that means two things. +- **Should the conformance suite pin the `namespace: None` semantics?** Yes — in + its own change. +- **Should `across_section` visit by recency rather than size?** For + `conversation:` recency is usually what a caller means, and a host with more + than `MAX_SECTION_NAMESPACES` conversations currently searches the largest + rather than the latest. It needs drivers to populate `last_updated` first.