Preserve the section delimiter in namespace enumeration

crates/tinymemory-conformance/src/lib.rs

@@ -45,7 +45,8 @@ pub use reference::{InMemoryProvider, REFERENCE_DRIVER_ID};
 pub use suite::{
     assert_awkward_content_round_trips, assert_capability_audit, assert_export_cursor_terminates,
     assert_export_import_round_trip, assert_forget_is_idempotent, assert_kv_round_trip,
-    assert_list_filters_narrow, assert_namespaces_are_isolated, assert_provider,
+    assert_list_filters_narrow, assert_namespaces_are_isolated,
+    assert_namespaces_preserve_their_section, assert_provider,
     assert_recall_respects_limit_and_namespace, assert_store_get_round_trip,
     assert_taint_is_preserved, assert_upsert_replaces_rather_than_duplicates,
 };

crates/tinymemory-conformance/src/suite/mod.rs

@@ -25,6 +25,7 @@ use std::sync::Arc;
 
 use tinymemory_api::capabilities::Capability;
 use tinymemory_api::error::MemoryError;
+use tinymemory_api::namespace::Namespace;
 use tinymemory_api::provider::{audit_provider, ExportRecord, MemoryProvider, SourceScope};
 use tinymemory_api::recall::OwnedRecallOpts;
 use tinymemory_api::types::{MemoryCategory, MemoryTaint};
@@ -61,6 +62,7 @@ pub async fn assert_provider(provider: Arc<dyn MemoryProvider>) {
     assert_list_filters_narrow(p).await;
     assert_taint_is_preserved(p).await;
     assert_recall_respects_limit_and_namespace(p).await;
+    assert_namespaces_preserve_their_section(p).await;
     assert_recall_respects_source_scope(p).await;
     assert_export_import_round_trip(p).await;
     assert_awkward_content_round_trips(p).await;
@@ -489,6 +491,70 @@ pub async fn assert_recall_respects_limit_and_namespace(provider: &dyn MemoryPro
     cleanup(provider, &theirs, &["other"]).await;
 }
 
+/// `namespaces()` reports a sectioned namespace back under the same
+/// [`tinymemory_api::namespace::MemorySection`] the caller wrote it in.
+///
+/// This is the regression the unified SQLite store's own storage-address
+/// sanitiser taught us to check for: a driver whose on-disk address collapses
+/// `:` to `_` (a real filesystem constraint) must still report the *logical*
+/// namespace back through `namespaces()`, or `conversation:thread-8f21`
+/// silently re-addresses out of the `conversation` section and every caller
+/// enumerating a section's scopes sees nothing, even though the write itself
+/// succeeded.
+///
+/// # Panics
+///
+/// Panics when no reported namespace parses to the same section as the one
+/// that was written.
+pub async fn assert_namespaces_preserve_their_section(provider: &dyn MemoryProvider) {
+    let who = provider.driver_id();
+    let namespace = format!("conversation:{}", ns(provider, "section-thread"));
+    let written = Namespace::parse(&namespace)
+        .unwrap_or_else(|e| panic!("{who}: test fixture `{namespace}` failed to parse: {e}"));
+
+    provider
+        .store(
+            &namespace,
+            "k",
+            "content",
+            MemoryCategory::Core,
+            None,
+            MemoryTaint::Internal,
+        )
+        .await
+        .unwrap_or_else(|e| panic!("{who}: store failed: {e}"));
+
+    let summaries = provider
+        .namespaces()
+        .await
+        .unwrap_or_else(|e| panic!("{who}: namespaces() failed: {e}"));
+
+    let matching_scope = summaries.iter().find_map(|summary| {
+        Namespace::parse(&summary.namespace)
+            .ok()
+            .filter(|parsed| parsed.scope() == written.scope())
+    });
+
+    match matching_scope {
+        Some(parsed) => assert_eq!(
+            parsed.section(),
+            written.section(),
+            "{who}: wrote `{namespace}` under section {:?}, but namespaces() reported \
+             its scope back under section {:?} instead — a driver must not silently \
+             re-address a sectioned namespace out of its section",
+            written.section(),
+            parsed.section(),
+        ),
+        None => panic!(
+            "{who}: namespaces() did not report any namespace with scope `{}` after \
+             storing `{namespace}`; got {summaries:?}",
+            written.scope()
+        ),
+    }
+
+    cleanup(provider, &namespace, &["k"]).await;
+}
+
 /// A present, empty source scope fails closed.
 ///
 /// # Panics

crates/tinymemory-core/src/store/memory_trait.rs

@@ -18,6 +18,7 @@ use rusqlite::{params, Connection, OptionalExtension};
 use serde_json::json;
 
 use crate::store::namespace_store::fts5;
+use crate::store::safety::LOGICAL_NAMESPACE_FILTER_SQL;
 use crate::store::types::{NamespaceDocumentInput, GLOBAL_NAMESPACE};
 use crate::traits::{
     Memory, MemoryCategory, MemoryEntry, MemoryTaint, NamespaceSummary, RecallOpts,
@@ -300,13 +301,24 @@ impl UnifiedMemory {
 // be `'static` and cannot borrow the store.
 
 /// One `memory_docs` row as `get` selects it:
-/// `(document_id, key, content, updated_at, category, taint, session_id)`.
-type MemoryDocRow = (String, String, String, f64, String, String, Option<String>);
+/// `(document_id, key, content, updated_at, category, taint, session_id,
+/// logical_namespace)`.
+type MemoryDocRow = (
+    String,
+    String,
+    String,
+    f64,
+    String,
+    String,
+    Option<String>,
+    Option<String>,
+);
 
 impl UnifiedMemory {
     fn get_blocking(
         conn: &Arc<Mutex<Connection>>,
         ns: &str,
+        logical: &str,
         key: &str,
     ) -> anyhow::Result<Option<MemoryEntry>> {
         let conn = conn.lock();
@@ -315,11 +327,18 @@ impl UnifiedMemory {
         // readers disagree about one record. The contract's round-trip
         // assertion catches exactly that (`tinymemory_conformance`), and it was
         // invisible until #18 §A3 let this store be bound as a driver at all.
+        //
+        // `logical_namespace` is selected too so the returned `MemoryEntry`
+        // reports the row's own logical name rather than the physical address
+        // this method happens to have been called with — see `list_blocking`'s
+        // doc comment for why that distinction matters.
         let row: Option<MemoryDocRow> = conn
             .query_row(
-                "SELECT document_id, key, content, updated_at, category, taint, session_id
-                 FROM memory_docs WHERE namespace = ?1 AND key = ?2 LIMIT 1",
-                params![ns, key],
+                &format!(
+                    "SELECT document_id, key, content, updated_at, category, taint, session_id, logical_namespace
+                     FROM memory_docs WHERE namespace = ?1 AND key = ?3 AND {LOGICAL_NAMESPACE_FILTER_SQL} LIMIT 1"
+                ),
+                params![ns, logical, key],
                 |row| {
                     Ok((
                         row.get(0)?,
@@ -329,43 +348,60 @@ impl UnifiedMemory {
                         row.get(4)?,
                         row.get(5)?,
                         row.get(6)?,
+                        row.get(7)?,
                     ))
                 },
             )
             .optional()?;
         Ok(row.map(
-            |(id, key, content, updated_at, category, taint_str, session_id)| MemoryEntry {
-                id,
-                key,
-                content,
-                namespace: Some(ns.to_string()),
-                category: memory_category_from_stored(&category),
-                timestamp: timestamp_to_rfc3339(updated_at),
-                session_id,
-                score: None,
-                taint: crate::MemoryTaint::from_db_str(&taint_str),
+            |(id, key, content, updated_at, category, taint_str, session_id, row_logical)| {
+                MemoryEntry {
+                    id,
+                    key,
+                    content,
+                    namespace: Some(row_logical.unwrap_or_else(|| ns.to_string())),
+                    category: memory_category_from_stored(&category),
+                    timestamp: timestamp_to_rfc3339(updated_at),
+                    session_id,
+                    score: None,
+                    taint: crate::MemoryTaint::from_db_str(&taint_str),
+                }
             },
         ))
     }
 
+    /// List every row addressed to one namespace, physical **and** logical.
+    ///
+    /// A caller lists `learning:rust`; `ns` is the sanitized `learning_rust`
+    /// storage address, and `logical` is `learning:rust` itself. Filtering on
+    /// physical address alone would also return `learning_rust`'s own rows
+    /// (a distinct logical namespace that happens to sanitize identically),
+    /// mislabelling them as belonging to the section that was listed —
+    /// exactly the incompleteness `logical_namespace` exists to close. Each
+    /// returned entry's `namespace` is the row's *own* logical name (falling
+    /// back to the physical address only for pre-migration NULL rows), never
+    /// the caller's query namespace, so a row that genuinely came from the
+    /// aliased NULL-logical legacy address is still labelled honestly.
     fn list_blocking(
         conn: &Arc<Mutex<Connection>>,
         ns: &str,
+        logical: &str,
         category: Option<&MemoryCategory>,
         session_id: Option<&str>,
     ) -> anyhow::Result<Vec<MemoryEntry>> {
         let conn = conn.lock();
-        let mut stmt = conn.prepare(
-            "SELECT document_id, key, content, category, session_id, updated_at, taint
-             FROM memory_docs WHERE namespace = ?1 ORDER BY updated_at DESC",
-        )?;
-        let rows = stmt.query_map(params![ns], |row| {
+        let mut stmt = conn.prepare(&format!(
+            "SELECT document_id, key, content, category, session_id, updated_at, taint, logical_namespace
+             FROM memory_docs WHERE namespace = ?1 AND {LOGICAL_NAMESPACE_FILTER_SQL} ORDER BY updated_at DESC"
+        ))?;
+        let rows = stmt.query_map(params![ns, logical], |row| {
             let stored_category: String = row.get(3)?;
+            let row_logical: Option<String> = row.get(7)?;
             Ok(MemoryEntry {
                 id: row.get(0)?,
                 key: row.get(1)?,
                 content: row.get(2)?,
-                namespace: Some(ns.to_string()),
+                namespace: Some(row_logical.unwrap_or_else(|| ns.to_string())),
                 category: memory_category_from_stored(&stored_category),
                 session_id: row.get(4)?,
                 timestamp: timestamp_to_rfc3339(row.get(5)?),
@@ -386,13 +422,16 @@ impl UnifiedMemory {
     fn forget_lookup_blocking(
         conn: &Arc<Mutex<Connection>>,
         ns: &str,
+        logical: &str,
         key: &str,
     ) -> anyhow::Result<Option<String>> {
         let conn = conn.lock();
         Ok(conn
             .query_row(
-                "SELECT document_id FROM memory_docs WHERE namespace = ?1 AND key = ?2 LIMIT 1",
-                params![ns, key],
+                &format!(
+                    "SELECT document_id FROM memory_docs WHERE namespace = ?1 AND key = ?3 AND {LOGICAL_NAMESPACE_FILTER_SQL} LIMIT 1"
+                ),
+                params![ns, logical, key],
                 |row| row.get(0),
             )
             .optional()?)
@@ -402,11 +441,43 @@ impl UnifiedMemory {
         conn: &Arc<Mutex<Connection>>,
     ) -> anyhow::Result<Vec<NamespaceSummary>> {
         let conn = conn.lock();
+        // `COALESCE(logical_namespace, namespace)` is the entire backfill
+        // story, deliberately: rows written before the `logical_namespace`
+        // column existed have it NULL and fall back to exactly today's
+        // sanitized value. A sanitized `_` cannot be reconstructed into
+        // whatever delimiter it replaced (a scope may legitimately contain
+        // `_`), so guessing would silently mislabel unrelated namespaces —
+        // NULL rows simply keep reporting their sanitized address.
+        //
+        // `GROUP BY COALESCE(logical_namespace, namespace)` — the logical
+        // name, not the raw storage address — so two distinct logical names
+        // that happen to sanitize to the same physical address
+        // (`conversation:x` and `conversation_x` both sanitize to
+        // `conversation_x`) get two separate summaries with their own counts.
+        //
+        // This used to group by `namespace` (the address) instead, on the
+        // reasoning that every addressed call already merged aliased rows
+        // into one physical namespace, so grouping by logical name would
+        // split one merged namespace's rows across two summaries with two
+        // partial counts. That reasoning no longer holds: `list`/`get`/
+        // `forget` now filter on `logical_namespace` too (see
+        // `LOGICAL_NAMESPACE_FILTER_SQL`), so an addressed call for one
+        // logical name only ever returns that name's own rows. Grouping
+        // summaries by address here, while reads are scoped by logical name,
+        // would report one summary for both aliases while `list` on that
+        // reported name only ever returns half its count — and would hide
+        // the other alias from enumeration entirely, exactly the leak this
+        // fixes.
+        //
+        // Legacy rows with `logical_namespace IS NULL` still group by their
+        // physical address (`COALESCE` falls through to `namespace`), which
+        // matches what `list`'s `OR logical_namespace IS NULL` arm returns
+        // for that address.
         let mut stmt = conn.prepare(
-            "SELECT namespace, COUNT(*) AS n, MAX(updated_at) AS last
+            "SELECT COALESCE(logical_namespace, namespace) AS ns, COUNT(*) AS n, MAX(updated_at) AS last
              FROM memory_docs
-             GROUP BY namespace
-             ORDER BY namespace",
+             GROUP BY COALESCE(logical_namespace, namespace)
+             ORDER BY ns",
         )?;
         let rows = stmt.query_map([], |row| {
             let ns: String = row.get(0)?;
@@ -529,8 +600,9 @@ impl Memory for UnifiedMemory {
         limit: usize,
         min_vector_similarity: f64,
     ) -> anyhow::Result<Vec<(String, String)>> {
+        let (ns, logical) = UnifiedMemory::namespace_address_forms(namespace);
         let hits = self
-            .query_namespace_hits(namespace, query, limit as u32)
+            .query_namespace_hits(&ns, &logical, query, limit as u32)
             .await
             .map_err(anyhow::Error::msg)?;
         Ok(hits
@@ -548,9 +620,16 @@ impl Memory for UnifiedMemory {
         // changed anything — the caller then reads the row as absent and stores
         // it again, which is the retry loop behind #5164.
         let ns = UnifiedMemory::sanitize_namespace(namespace);
+        // The same delimiter-preserving logical name the write path bound
+        // into `logical_namespace` (`canonical_logical_namespace`), so `get`
+        // addresses the row by both its physical and logical identity — see
+        // `LOGICAL_NAMESPACE_FILTER_SQL`'s doc comment for why the physical
+        // address alone is not enough.
+        let logical =
+            crate::store::safety::canonical_logical_namespace(namespace, GLOBAL_NAMESPACE);
         let key = crate::store::safety::canonical_document_key(key);
         let conn = Arc::clone(&self.conn);
-        tokio::task::spawn_blocking(move || Self::get_blocking(&conn, &ns, &key))
+        tokio::task::spawn_blocking(move || Self::get_blocking(&conn, &ns, &logical, &key))
             .await
             .context("join Memory::get")?
     }
@@ -561,12 +640,21 @@ impl Memory for UnifiedMemory {
         category: Option<&MemoryCategory>,
         session_id: Option<&str>,
     ) -> anyhow::Result<Vec<MemoryEntry>> {
-        let ns = UnifiedMemory::sanitize_namespace(normalize_namespace(namespace));
+        let normalized = normalize_namespace(namespace);
+        let ns = UnifiedMemory::sanitize_namespace(normalized);
+        let logical =
+            crate::store::safety::canonical_logical_namespace(normalized, GLOBAL_NAMESPACE);
         let category = category.cloned();
         let session_id = session_id.map(str::to_owned);
         let conn = Arc::clone(&self.conn);
         tokio::task::spawn_blocking(move || {
-            Self::list_blocking(&conn, &ns, category.as_ref(), session_id.as_deref())
+            Self::list_blocking(
+                &conn,
+                &ns,
+                &logical,
+                category.as_ref(),
+                session_id.as_deref(),
+            )
         })
         .await
         .context("join Memory::list")?
@@ -577,13 +665,18 @@ impl Memory for UnifiedMemory {
         // addresses the raw caller identifiers can never delete a row whose
         // namespace or key was canonicalized on the way in.
         let ns = UnifiedMemory::sanitize_namespace(namespace);
+        let logical =
+            crate::store::safety::canonical_logical_namespace(namespace, GLOBAL_NAMESPACE);
         let key = crate::store::safety::canonical_document_key(key);
         let row: Option<String> = {
             let conn = Arc::clone(&self.conn);
             let ns = ns.clone();
-            tokio::task::spawn_blocking(move || Self::forget_lookup_blocking(&conn, &ns, &key))
-                .await
-                .context("join Memory::forget")??
+            let logical = logical.clone();
+            tokio::task::spawn_blocking(move || {
+                Self::forget_lookup_blocking(&conn, &ns, &logical, &key)
+            })
+            .await
+            .context("join Memory::forget")??
         };
         let Some(document_id) = row else {
             return Ok(false);