Merge branch 'main' into feat/persistent-importance-column

Author: tcconnally

docs/retention.md

@@ -104,13 +104,22 @@ Deletion is explicit and two-step:
 - **`purge`** — permanently delete entities that are **already archived**.
   Supports `dry_run`. This is the only way memory leaves the database.
 
-## Known limitation
+## Semantic recall and reinforcement
 
-Retrieval reinforcement currently fires only on the keyword (`fts5`) recall
-path; the default hybrid/dense path is side-effect-free to keep recall
-byte-deterministic over a frozen DB (#247, see
+By default, retrieval reinforcement fires only on the keyword (`fts5`) recall
+path; the hybrid/dense paths are side-effect-free so recall over a frozen DB
+stays byte-deterministic (#247, see
 `deterministic-recall-and-provenance.md`). A memory that is only ever found
-via hybrid recall therefore still decays as if unused. Whether to trade that
-determinism for reinforcement-on-use is an open product decision; until it is
-made, mark load-bearing memories `verified` (floor) or `always_on`
-(unconditional injection) rather than relying on usage to keep them alive.
+semantically therefore decays as if unused — unless you opt in:
+
+- **`reinforce: true`** on `mimir_recall` with `mode: 'dense'`/`'hybrid'`
+  applies the standard side-effects (retrieval-count bump, recency reset,
+  +0.25 decay boost, layer promotion) to the returned hits. This trades
+  byte-determinism of *subsequent* recalls for "used memories resist decay" —
+  the recall that carries the flag still returns the same ranking it would
+  have without it.
+- Alternatively, mark load-bearing memories `verified` (decay floor) or
+  `always_on` (unconditional injection) and keep semantic recall pure.
+
+`skip_side_effects` always wins over `reinforce`: a caller that asked for a
+pure read never mutates.

src/db.rs

@@ -1820,6 +1820,7 @@ impl Database {
                     let dense_results = self.dense_search(query_vec, params.limit as usize)?;
                     let mut out: Vec<Entity> = dense_results.into_iter().map(|(e, _)| e).collect();
                     Self::retain_layer(&mut out, params);
+                    self.reinforce_if_requested(params, &out)?;
                     return Ok(out);
                 }
 
@@ -1907,6 +1908,7 @@ impl Database {
                 };
                 let mut out: Vec<Entity> = fused.into_iter().map(|(e, _)| e).collect();
                 Self::retain_layer(&mut out, params);
+                self.reinforce_if_requested(params, &out)?;
                 return Ok(out);
             }
             // Empty query: nothing to embed, fall through to FTS5
@@ -1917,6 +1919,26 @@ impl Database {
         Ok(results)
     }
 
+    /// Opt-in reinforcement for the semantic (Dense/Hybrid) recall paths.
+    /// Applies the standard recall side-effects (retrieval-count bump,
+    /// recency, decay boost, layer promotion) to the returned hits when the
+    /// caller set `reinforce` — and only then. The default stays
+    /// side-effect-free so repeated semantic recalls over a frozen DB remain
+    /// byte-deterministic (#247). `skip_side_effects` always wins: a caller
+    /// that asked for a pure read never mutates, whatever else is set.
+    fn reinforce_if_requested(
+        &self,
+        params: &RecallParams,
+        hits: &[Entity],
+    ) -> Result<(), Box<dyn std::error::Error>> {
+        if !params.reinforce || params.skip_side_effects || hits.is_empty() {
+            return Ok(());
+        }
+        let ids: Vec<String> = hits.iter().map(|e| e.id.clone()).collect();
+        self.apply_recall_side_effects(&ids)?;
+        Ok(())
+    }
+
     /// Core FTS5 + LIKE keyword search (extracted for reuse by recall and hybrid).
     fn fts5_search(
         &self,
@@ -8606,6 +8628,74 @@ mod tests {
         let _ = fs::remove_file(&path);
     }
 
+    #[test]
+    fn hybrid_recall_reinforce_flag_bumps_returned_hits_opt_in_only() {
+        // Opt-in counterpart to hybrid_recall_is_read_only_and_idempotent:
+        // reinforce=true applies the standard side-effects to the returned
+        // hits; skip_side_effects still wins over it.
+        let (db, path) = temp_db();
+        let blob = |v: &[f32]| -> Vec<u8> { v.iter().flat_map(|f| f.to_le_bytes()).collect() };
+        db.conn()
+            .unwrap()
+            .execute(
+                "INSERT INTO entities (id, category, key, body_json, type, status,
+                    retrieval_count, last_accessed_unix_ms, created_at_unix_ms,
+                    decay_score, layer, embedding, archived)
+                 VALUES ('e-r', 'insight', 'reinf', '{\"note\":\"espresso ritual\"}',
+                         'insight', 'active', 0, 0, 0, 0.5, 'buffer', ?1, 0)",
+                params![blob(&[1.0, 0.0, 0.0])],
+            )
+            .unwrap();
+        db.conn()
+            .unwrap()
+            .execute(
+                "INSERT INTO entities_fts (rowid, body_json)
+                 VALUES ((SELECT rowid FROM entities WHERE id = 'e-r'), '{\"note\":\"espresso ritual\"}')",
+                [],
+            )
+            .unwrap();
+
+        // skip_side_effects wins over reinforce: still a pure read.
+        let pure = RecallParams {
+            query: "espresso".to_string(),
+            mode: crate::models::SearchMode::Hybrid,
+            embedding: Some(vec![1.0, 0.0, 0.0]),
+            limit: 10,
+            reinforce: true,
+            skip_side_effects: true,
+            ..RecallParams::default()
+        };
+        db.recall(&pure).unwrap();
+        let rc: i64 = db
+            .conn()
+            .unwrap()
+            .query_row("SELECT retrieval_count FROM entities WHERE id = 'e-r'", [], |r| r.get(0))
+            .unwrap();
+        assert_eq!(rc, 0, "skip_side_effects must override reinforce");
+
+        // reinforce alone: returned hit gets the standard side-effects.
+        let reinforcing = RecallParams {
+            skip_side_effects: false,
+            ..pure
+        };
+        let hits = db.recall(&reinforcing).unwrap();
+        assert!(hits.iter().any(|e| e.id == "e-r"), "hit expected");
+        let (rc2, la2, ds2): (i64, i64, f64) = db
+            .conn()
+            .unwrap()
+            .query_row(
+                "SELECT retrieval_count, last_accessed_unix_ms, decay_score FROM entities WHERE id = 'e-r'",
+                [],
+                |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?)),
+            )
+            .unwrap();
+        assert_eq!(rc2, 1, "reinforce must bump retrieval_count");
+        assert!(la2 > 0, "reinforce must touch last_accessed");
+        assert!(ds2 > 0.5, "reinforce must boost decay_score, got {ds2}");
+
+        let _ = fs::remove_file(&path);
+    }
+
     #[test]
     fn hybrid_over_fetches_arms_before_fusion() {
         // Each hybrid arm is fetched at a candidate pool LARGER than `limit`, then
@@ -8840,6 +8930,7 @@ mod tests {
             agent_id: None,
             visibility: None,
             layer: None,
+            reinforce: false,
             },
         )
         .unwrap();
@@ -8973,6 +9064,7 @@ mod tests {
                     agent_id: None,
                     visibility: None,
                     layer: None,
+                    reinforce: false,
                 }) {
                     Ok(_) => {},
                     Err(e) => {
@@ -9036,6 +9128,7 @@ mod tests {
             agent_id: None,
             visibility: None,
             layer: None,
+            reinforce: false,
         };
 
         // Scope to "alpha" — should only see ent_a
@@ -9081,6 +9174,7 @@ mod tests {
             agent_id: None,
             visibility: None,
             layer: None,
+            reinforce: false,
         };
         let results = db.recall(&params).unwrap();
         let found = results.iter().find(|e| e.key == "key1").expect("entity recalled");

src/mcp.rs

@@ -392,6 +392,11 @@ fn list_tools(id: Option<Value>) -> JsonRpcResponse {
           "default": false,
           "description": "Add a normalized confidence score (0.0-1.0) to each result, rolled up from rank, trust (verified/certainty), and decay. Presentation-only; does not change ranking."
         },
+        "reinforce": {
+          "type": "boolean",
+          "default": false,
+          "description": "Opt-in reinforcement for mode='dense'/'hybrid': bump retrieval_count/last_accessed/decay on the returned hits so semantically-used memories resist decay and promote through layers. Default false keeps semantic recall side-effect-free and byte-deterministic over a frozen DB. No effect on mode='fts5', which already reinforces."
+        },
         "expansion": {
           "type": "object",
           "properties": {

src/models.rs

@@ -235,6 +235,15 @@ pub struct RecallParams {
     #[allow(dead_code)]
     pub visibility: Option<String>,
     pub layer: Option<String>,
+    /// Opt-in reinforcement for Dense/Hybrid recall. The semantic paths are
+    /// side-effect-free by default so repeated recalls over a frozen DB stay
+    /// byte-deterministic (#247) — the cost is that memories only ever found
+    /// semantically decay as if unused. When true, the returned hits receive
+    /// the same retrieval_count/last_accessed/decay-boost side-effects the
+    /// fts5 path applies, trading determinism for "used memories resist
+    /// decay". Ignored when skip_side_effects is set. No effect on the fts5
+    /// path, which already reinforces unless skip_side_effects.
+    pub reinforce: bool,
 }
 
 /// Search mode for recall: FTS5 keyword, dense vector, or hybrid fusion.
@@ -299,6 +308,7 @@ impl Default for RecallParams {
             agent_id: None,
             visibility: None,
             layer: None,
+            reinforce: false,
         }
     }
 }

src/tools.rs

@@ -172,6 +172,11 @@ pub struct RecallArgs {
     /// existing callers and snapshot tests are unaffected; ranking is unchanged.
     #[serde(default, deserialize_with = "null_as_default")]
     pub include_confidence: bool,
+    /// Opt-in reinforcement for dense/hybrid recall: bump retrieval stats on
+    /// the returned hits so semantically-used memories resist decay. Default
+    /// false — the semantic paths stay byte-deterministic (#247).
+    #[serde(default, deserialize_with = "null_as_default")]
+    pub reinforce: bool,
 }
 
 /// #287: presentation-layer confidence rollup over signals Mneme already has.
@@ -516,6 +521,7 @@ pub fn handle_recall(db: &Database, args: Value) -> Result<String, String> {
         agent_id: a.agent_id.clone(),
         visibility: None,
         layer: a.layer.as_deref().filter(|s| !s.is_empty()).map(canonical_layer),
+        reinforce: a.reinforce,
     };
 
     let entities = db
@@ -646,6 +652,9 @@ fn handle_recall_with_expansion(db: &Database, a: &RecallArgs) -> Result<String,
             agent_id: a.agent_id.clone(),
             visibility: None,
             layer: a.layer.as_deref().filter(|s| !s.is_empty()).map(canonical_layer),
+            // Fts5-only path: reinforcement is handled by the batched
+            // side-effect below, not the per-variant recalls.
+            reinforce: false,
         };
 
         if let Ok(entities) = db.recall(&params) {