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

@@ -300,8 +300,18 @@ 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(
@@ -315,9 +325,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. This is purely a
+        // labelling improvement: the row is still addressed by the physical
+        // `namespace` column alone (`WHERE namespace = ?1`), so two logical
+        // namespaces that sanitize to the same physical address are still
+        // one namespace here, same as before `logical_namespace` existed.
         let row: Option<MemoryDocRow> = conn
             .query_row(
-                "SELECT document_id, key, content, updated_at, category, taint, session_id
+                "SELECT document_id, key, content, updated_at, category, taint, session_id, logical_namespace
                  FROM memory_docs WHERE namespace = ?1 AND key = ?2 LIMIT 1",
                 params![ns, key],
                 |row| {
@@ -329,25 +348,41 @@ 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 physical namespace.
+    ///
+    /// Addressed by the physical `namespace` column only (`WHERE namespace =
+    /// ?1`) — exactly as before `logical_namespace` existed. Two logical
+    /// namespaces that sanitize to the same physical address (`a:b_c` and
+    /// `a_b:c` both sanitize to `a_b_c`) are still one namespace for this
+    /// call, and `sanitize_namespace` has always collapsed them that way; this
+    /// is pre-existing behaviour, not something this column changes. What
+    /// `logical_namespace` adds is purely the label: each returned entry's
+    /// `namespace` is the row's *own* logical name (falling back to the
+    /// physical address for pre-migration NULL rows) instead of the raw
+    /// sanitized address, so a sectioned namespace still reports its `:`
+    /// spelling back to a caller enumerating it.
     fn list_blocking(
         conn: &Arc<Mutex<Connection>>,
         ns: &str,
@@ -356,16 +391,17 @@ impl UnifiedMemory {
     ) -> anyhow::Result<Vec<MemoryEntry>> {
         let conn = conn.lock();
         let mut stmt = conn.prepare(
-            "SELECT document_id, key, content, category, session_id, updated_at, taint
+            "SELECT document_id, key, content, category, session_id, updated_at, taint, logical_namespace
              FROM memory_docs WHERE namespace = ?1 ORDER BY updated_at DESC",
         )?;
         let rows = stmt.query_map(params![ns], |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)?),
@@ -402,11 +438,35 @@ 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 namespace` (the storage address), not the logical name:
+        // every OTHER operation on this store — `get`, `list`, `forget`,
+        // `recall`, `clear_namespace` — addresses a row by its physical
+        // `namespace` column alone, so two logical names that sanitize to the
+        // same address (`conversation:x` and `conversation_x` both sanitize
+        // to `conversation_x`) are already treated as one namespace
+        // everywhere else. Grouping summaries by the logical name instead
+        // would report two summaries with two partial counts for what every
+        // other call still treats, and returns, as a single merged
+        // namespace — `list` on either reported name would return BOTH
+        // aliases' rows, double the count either summary claims. Grouping by
+        // the address keeps one summary per physical namespace with an
+        // accurate count; `MIN(logical_namespace)` (aggregate `MIN` ignores
+        // `NULL`) just picks a single, deterministic logical representative
+        // to report it under, so a sectioned namespace still enumerates
+        // under its `:` spelling instead of the sanitized `_` form.
         let mut stmt = conn.prepare(
-            "SELECT namespace, COUNT(*) AS n, MAX(updated_at) AS last
+            "SELECT COALESCE(MIN(logical_namespace), namespace) AS ns, COUNT(*) AS n, MAX(updated_at) AS last
              FROM memory_docs
              GROUP BY namespace
-             ORDER BY namespace",
+             ORDER BY ns",
         )?;
         let rows = stmt.query_map([], |row| {
             let ns: String = row.get(0)?;