refactor(core): one commit step and one embedding step for remember and contradiction (#1514, part 1)

CHANGELOG.md

@@ -20,6 +20,7 @@
 
 ### Changed
 
+- **`remember` with `detect_contradiction` writes like plain `remember` (#1514, part 1)** - the two now share one embedding step and one commit step. The contradiction path used to skip the memory type check (an unknown type was stored), embed JSON content as raw text instead of the flattened text, try the embedding once, and fail the whole write when the embedding failed. It now rejects an unknown type, embeds the flattened text, retries like `remember`, and on a persistent embedding failure stores the memory FTS5-only with a warning (the contradiction check is skipped without a vector, as the duplicate check is). Import and consolidation move onto the same commit step in later parts, once uteke-cloud#242 is answered.
 - Internal: the redaction flag policy is its own module, #1520 (round 2 of the architecture deepening, #1523): `redaction_flags::resolve` decides which `redacted` / `redactions` flags a memory carries after an update as one pure function with a test per rule, and `update_memory_detailed` loses its inline five-arm `match`. The three flag helpers moved with it and are still reachable as `uteke_core::privacy::{with,without,carry}_redaction_flags`. No behaviour change.
 - Internal: errors are classified once, part of #1517 (round 2 of the architecture deepening, #1523): `Error::NotFound` and `Error::class()` (`BadRequest`, `NotFound`, `SecretDetected`, `Internal`), and one `core_error` mapping in the server instead of seven hand-written `Validation => 400, else 500` arms. The two handlers that matched `contains("not found")` (`POST /doc/move`, `DELETE /room/delete`) now read the class. HTTP statuses do not change: a missing memory on update or a missing room on rename still answer 400 until the contract with uteke-cloud moves them. The one visible difference is the text of the `POST /doc/move` 404, which no longer starts with `Validation error:`.
 - Internal: `ort` 2.0.0-rc.12/rc.13 to rc.14 (replaces the dependabot PR, which cannot build): rc.14 builds sessions from an explicit `Environment`, so the one process-wide environment is now created in `embed::ort_init` and handed to `Session::builder`. The bundled ONNX Runtime stays at 1.24.4 (`api-18`). Checked with the real model: `remember` writes the embedding and `recall` finds it.

crates/uteke-core/src/consolidate.rs

@@ -71,6 +71,8 @@ impl crate::Uteke {
         contradiction_threshold: f32,
     ) -> Result<(RememberOutcome, ContradictionResult), Error> {
         crate::validate_input(content, tags)?;
+        let memory_type = memory_type.unwrap_or("fact");
+        Self::validate_memory_type(memory_type)?;
         let ns = namespace.unwrap_or(DEFAULT_NAMESPACE);
         // Privacy filter (#1464), before the content is embedded or stored.
         let prepared = self
@@ -82,18 +84,23 @@ impl crate::Uteke {
         let tags: Vec<&str> = tag_owned.iter().map(String::as_str).collect();
         let tags = tags.as_slice();
 
-        // Embed first to check for contradictions before persisting
-        self.ensure_embedder()?;
-        let embedding = self
-            .embedder
-            .lock()
-            .map_err(|_| Error::lock("embedder lock during remember_with_contradiction"))?
-            .as_ref()
-            .expect("embedder ensured above")
-            .embed(content)?;
-
-        // Check for contradictions (read-only)
-        let contradiction = if check_contradiction {
+        // Embed first to check for contradictions before persisting: the same
+        // embedding step as `remember` (JSON flattened, retried; no vector
+        // means FTS5-only with a warning).
+        let content_type = crate::memory::crud::detect_content_type(content);
+        let (embedding, generation_error) = self.embed_for_write(content, content_type)?;
+
+        // Check for contradictions (read-only). Without a vector there is
+        // nothing to compare, like the duplicate check of `remember`.
+        let skipped = check_contradiction && embedding.is_empty();
+        // The caller asked for the check: say it did not happen.
+        let generation_error = if skipped {
+            generation_error
+                .or_else(|| Some("contradiction check skipped: no embedding was produced".into()))
+        } else {
+            generation_error
+        };
+        let contradiction = if check_contradiction && !skipped {
             self.check_contradiction(content, &embedding, ns, contradiction_threshold)?
         } else {
             ContradictionResult {
@@ -103,19 +110,16 @@ impl crate::Uteke {
             }
         };
 
-        // Detect content type for consolidated memory
-        let content_type = crate::memory::crud::detect_content_type(content);
-
         // Use remember_precomputed to avoid double-embedding
         let mut outcome = self.remember_precomputed_detailed(
             content,
             tags,
             metadata,
             Some(ns),
-            memory_type.unwrap_or("fact"),
+            memory_type,
             content_type,
             &embedding,
-            None,
+            generation_error,
         )?;
         let id = outcome.id.clone();
         self.record_redaction(&id, &prepared.redactions);

crates/uteke-core/src/lib.rs

@@ -43,6 +43,8 @@ mod privacy_field_tests;
 #[cfg(test)]
 mod privacy_write_tests;
 mod redaction_flags;
+#[cfg(test)]
+mod write_path_tests;
 
 pub use forget_request::{
     ForgetErrorKind, ForgetInput, ForgetPolicy, ForgetRequest, ForgetRequestError, FullIdForm,

crates/uteke-core/src/operations.rs

@@ -295,34 +295,25 @@ impl crate::Uteke {
         );
     }
 
-    /// Embed-then-store with honest embedding reporting (#1273).
-    fn remember_embed_detailed(
-        &self,
-        content: &str,
-        tags: &[&str],
-        metadata: Option<serde_json::Value>,
-        namespace: Option<&str>,
-        memory_type: &str,
-    ) -> Result<RememberOutcome, Error> {
-        crate::validate_input(content, tags)?;
-        // Validate memory_type against known variants.
+    /// Reject a memory type this version does not know.
+    pub(crate) fn validate_memory_type(memory_type: &str) -> Result<(), Error> {
         crate::memory::types::MemoryType::from_str_opt(memory_type).ok_or_else(|| {
             Error::Validation(format!(
                 "Unknown memory type '{memory_type}'. Valid types: fact, procedure, preference, decision, context, note, insight, reference, event"
             ))
         })?;
-        // Privacy filter (#1464): before the content is embedded, indexed or
-        // stored, so a secret never reaches the vector, FTS5 or the row.
-        let prepared = self
-            .privacy_config
-            .prepare_with_tags(content, tags, metadata)?;
-        let content = prepared.content.as_str();
-        let metadata = prepared.metadata.clone();
-        let tag_owned = prepared.tags.clone();
-        let tags: Vec<&str> = tag_owned.iter().map(String::as_str).collect();
-        let tags = tags.as_slice();
-        // Detect JSON content and use flattened text for embedding
-        let content_type = crate::memory::crud::detect_content_type(content);
+        Ok(())
+    }
+
+    /// Embed the text of a memory about to be written: JSON content is
+    /// flattened first, the embedding is retried (#621), and a backend that
+    /// is not configured or keeps failing gives an EMPTY embedding (the row is
+    /// stored FTS5-only, #1166) with the reason in the second field (#1273).
+    pub(crate) fn embed_for_write(
+        &self,
+        content: &str,
+        content_type: &str,
+    ) -> Result<(Vec<f32>, Option<String>), Error> {
         let embed_text = if content_type == "json" {
             crate::memory::crud::flatten_json_for_embedding(content)
         } else {
@@ -336,10 +327,7 @@ impl crate::Uteke {
         // supplied via the injected-embedding path or `uteke repair`.
         // #1273: embedding GENERATION failure is also a store-and-warn case —
         // the row is kept (SQLite + FTS5) and the outcome carries the reason.
-        let (embedding, generation_error): (Vec<f32>, Option<String>) = if self
-            .embedder_backend
-            .is_empty()
-        {
+        Ok(if self.embedder_backend.is_empty() {
             tracing::debug!("No embedding backend configured; storing memory FTS5-only (#1166)");
             (Vec::new(), None)
         } else {
@@ -359,7 +347,32 @@ impl crate::Uteke {
                     )
                 }
             }
-        };
+        })
+    }
+
+    /// Embed-then-store with honest embedding reporting (#1273).
+    fn remember_embed_detailed(
+        &self,
+        content: &str,
+        tags: &[&str],
+        metadata: Option<serde_json::Value>,
+        namespace: Option<&str>,
+        memory_type: &str,
+    ) -> Result<RememberOutcome, Error> {
+        crate::validate_input(content, tags)?;
+        Self::validate_memory_type(memory_type)?;
+        // Privacy filter (#1464): before the content is embedded, indexed or
+        // stored, so a secret never reaches the vector, FTS5 or the row.
+        let prepared = self
+            .privacy_config
+            .prepare_with_tags(content, tags, metadata)?;
+        let content = prepared.content.as_str();
+        let metadata = prepared.metadata.clone();
+        let tag_owned = prepared.tags.clone();
+        let tags: Vec<&str> = tag_owned.iter().map(String::as_str).collect();
+        let tags = tags.as_slice();
+        let content_type = crate::memory::crud::detect_content_type(content);
+        let (embedding, generation_error) = self.embed_for_write(content, content_type)?;
 
         // Dedup check: if an existing memory has cosine >= 0.95, return it
         // instead of creating a duplicate (#442 enhancement).
@@ -545,13 +558,31 @@ impl crate::Uteke {
             author: None,
         };
 
+        self.commit_memory(&memory, embedding_error)
+    }
+
+    /// Everything that happens once a memory has been built and embedded:
+    /// the SQLite row, the source hash, the `Created` event, edge wiring,
+    /// recall-cache invalidation, the vector, cosine links. The memory
+    /// carries its own embedding (empty: stored FTS5-only).
+    pub(crate) fn commit_memory(
+        &self,
+        memory: &Memory,
+        embedding_error: Option<String>,
+    ) -> Result<RememberOutcome, Error> {
+        let id = memory.id.clone();
+        let content = memory.content.as_str();
+        let embedding = memory.embedding.as_slice();
+        let tag_refs: Vec<&str> = memory.tags.iter().map(String::as_str).collect();
+        let tags = tag_refs.as_slice();
+
         // SQLite is the source of truth and is written first, WITHOUT holding the
         // index write lock: the timeline / provenance / edge wiring below issue
         // many queries and would otherwise block every concurrent recall for
         // their whole duration. The vector is mirrored afterwards under a short
         // write lock; if that fails the row stays FTS5-only and the outcome says
         // so (#1273), `uteke repair` can backfill.
-        self.store.insert(&memory)?;
+        self.store.insert(memory)?;
 
         // Provenance: record the content hash at write time (#1172 Fase 1).
         // Best-effort — audits recompute this to detect post-write tampering.

crates/uteke-core/src/write_path_tests.rs

@@ -0,0 +1,159 @@
+//! The steps shared by every way of writing a memory (#1514): `remember` and
+//! `remember_with_contradiction` embed the same text, retry the same way,
+//! validate the type, and leave the same side effects.
+
+use std::sync::{Arc, Mutex};
+
+use crate::embed::Embedder;
+use crate::test_support::open_keyword;
+use crate::{Error, Uteke};
+
+/// Records every text it is asked to embed and can fail the first `fail` calls.
+struct Recording {
+    seen: Arc<Mutex<Vec<String>>>,
+    fail: Mutex<usize>,
+}
+
+impl Embedder for Recording {
+    fn embed(&self, text: &str) -> Result<Vec<f32>, Error> {
+        self.seen.lock().unwrap().push(text.to_string());
+        let mut fail = self.fail.lock().unwrap();
+        if *fail > 0 {
+            *fail -= 1;
+            return Err(Error::embed_msg("transient embedding failure"));
+        }
+        let mut v = vec![0.0_f32; 8];
+        v[text.len() % 8] = 1.0;
+        Ok(v)
+    }
+    fn dims(&self) -> usize {
+        8
+    }
+    fn max_seq_len(&self) -> usize {
+        128
+    }
+    fn name(&self) -> &str {
+        "recording-test-embedder"
+    }
+}
+
+fn store_with(fail: usize) -> (Uteke, Arc<Mutex<Vec<String>>>) {
+    let seen = Arc::new(Mutex::new(Vec::new()));
+    let (_db, store) = Uteke::open_store(":memory:").expect("open_store");
+    let u = Uteke::finish_open_full(
+        store,
+        Some(Box::new(Recording {
+            seen: seen.clone(),
+            fail: Mutex::new(fail),
+        })),
+        "test-recording".to_string(),
+        crate::TierConfig::default(),
+        crate::RecallConfig::default(),
+        crate::EmbeddingSettings::default(),
+        crate::graph_rerank::GraphRerankConfig::default(),
+        None,
+    )
+    .expect("finish_open_full");
+    (u, seen)
+}
+
+const JSON_CONTENT: &str = r#"{"user_name":"Bob","tags":["x","y"],"n":{"deep":1}}"#;
+
+fn contradiction(u: &Uteke, content: &str, ns: &str) -> Result<crate::RememberOutcome, Error> {
+    u.remember_with_contradiction(content, &[], None, Some(ns), None, true, 0.65)
+        .map(|(o, _)| o)
+}
+
+#[test]
+fn the_contradiction_path_rejects_an_unknown_type_like_remember() {
+    let u = open_keyword();
+    let err = u
+        .remember_with_contradiction("a memory", &[], None, None, Some("bogus"), true, 0.65)
+        .unwrap_err();
+    assert!(matches!(err, Error::Validation(_)), "{err}");
+    let err = u
+        .remember_typed_detailed("a memory", &[], None, None, "bogus")
+        .unwrap_err();
+    assert!(matches!(err, Error::Validation(_)), "{err}");
+    assert!(u.list(None, 10, 0, None).unwrap().is_empty());
+}
+
+#[test]
+fn both_paths_embed_the_same_text_for_json_content() {
+    let (u, seen) = store_with(0);
+    u.remember_detailed(JSON_CONTENT, &[], None, Some("a"))
+        .unwrap();
+    contradiction(&u, JSON_CONTENT, "b").unwrap();
+    let seen = seen.lock().unwrap().clone();
+    let flat = crate::memory::crud::flatten_json_for_embedding(JSON_CONTENT);
+    assert_ne!(flat, JSON_CONTENT, "the flattening must change this input");
+    assert_eq!(seen, vec![flat.clone(), flat], "{seen:?}");
+}
+
+#[test]
+fn the_contradiction_path_retries_a_transient_embedding_failure() {
+    let (u, seen) = store_with(1);
+    let o = contradiction(&u, "retry me", "b").unwrap();
+    assert!(o.embedding_written, "{:?}", o.warning);
+    assert_eq!(seen.lock().unwrap().len(), 2, "one failure, one success");
+}
+
+#[test]
+fn a_failing_embedder_stores_fts_only_with_a_warning_instead_of_failing_the_write() {
+    // Three failures exhaust the retries.
+    let (u, _) = store_with(3);
+    let o = contradiction(&u, "never embedded", "b").unwrap();
+    assert!(!o.embedding_written);
+    assert!(
+        o.warning
+            .as_deref()
+            .unwrap()
+            .contains("embedding generation failed")
+    );
+    // The row is stored and keyword-searchable.
+    let rows = u.list(None, 10, 0, None).unwrap();
+    assert_eq!(rows.len(), 1);
+    assert_eq!(rows[0].content, "never embedded");
+}
+
+#[test]
+fn both_paths_leave_the_same_side_effects() {
+    let (u, _) = store_with(0);
+    let a = u
+        .remember_detailed("alpha note", &[], None, Some("a"))
+        .unwrap()
+        .id;
+    let b = contradiction(&u, "beta note", "b").unwrap().id;
+    for id in [&a, &b] {
+        let events = u.timeline(id, 20).unwrap();
+        assert!(
+            events.iter().any(|e| e.event_type == "created"),
+            "{id}: {events:?}"
+        );
+        let m = u.get_by_id(id).unwrap().unwrap();
+        assert!(!m.embedding.is_empty(), "{id}: vector stored");
+    }
+}
+
+#[test]
+fn without_an_embedder_a_requested_contradiction_check_says_it_was_skipped() {
+    let u = Uteke::open_with_backend(":memory:", None).expect("open without embedder");
+    let (o, c) = u
+        .remember_with_contradiction("no vectors here", &[], None, None, None, true, 0.65)
+        .unwrap();
+    assert!(!o.embedding_written);
+    assert!(!c.contradicted);
+    assert!(
+        o.warning
+            .as_deref()
+            .unwrap()
+            .contains("contradiction check skipped"),
+        "{:?}",
+        o.warning
+    );
+    // Not asked for: no warning (same as plain remember without an embedder).
+    let (o, _) = u
+        .remember_with_contradiction("still no vectors", &[], None, None, None, false, 0.65)
+        .unwrap();
+    assert_eq!(o.warning, None);
+}