fix(temporal): audit set_valid_to closes with an entity_history snapshot (#373)

set_valid_to enforced tighten-only semantics but wrote no entity_history
snapshot, so a close was invisible to transaction-time reconstruction:
as_of/bitemporal_at at a tx instant BEFORE the close reported the close
anyway (the live row's valid_to was the only record and unversioned),
and the #372 audited-re-assert snapshot then conflated the close into
the whole pre-extension recorded window.

Mirror the #371/#372 audited stamp in set_valid_to: an effective
close/tighten snapshots the pre-change row (invalidated = now with the
anti-zero-width bump, superseded_by = self), advances the live row's
recorded_at, links supersedes, and pins a still-NULL valid_from to the
old effective opening so pre-v9 rows don't shift. The tighten-only
guards are untouched; a no-op call (earlier stored close kept) writes
no snapshot. mimir_supersede funnels through set_valid_to and inherits
the audit for free.

The verbatim snapshot INSERT shared with remember_impl is factored into
snapshot_live_row_to_history.

Tests: set_valid_to_close_is_audited_and_noop_is_not (exactly one
snapshot; bitemporal_at at a pre-close tx instant reconstructs the fact
OPEN, current knowledge shows the close; same-value and would-extend
no-ops add nothing) and supersede_close_inherits_the_audit_snapshot.
254 passed, 2 ignored; temporal gate 100% (20/20).

Closes #373

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: tcconnally

CHANGELOG.md

@@ -46,6 +46,13 @@ All notable changes to Perseus Vault (formerly Mimir/Mneme) are documented here.
   count 53 → 55.
 
 ### Fixed
+- Audited `set_valid_to` closes (#373): closing/tightening a fact's valid
+  period (directly or via `mimir_supersede`) now snapshots the pre-close
+  version to `entity_history` and advances the live row's transaction time —
+  previously a close was invisible to `mimir_as_of`/`mimir_bitemporal`
+  reconstruction, which reported the close even at transaction instants
+  before it happened. Tighten-only acceptance semantics are unchanged, and a
+  no-op call (an earlier stored close is kept) writes no snapshot.
 - Bi-temporal audit gap (#371): an identical-body re-remember that moves the
   bounds of an already-CLOSED valid period (e.g. re-extending past a
   `mimir_supersede`/`set_valid_to` close) now snapshots the pre-change version

src/db.rs

@@ -1848,23 +1848,7 @@ impl Database {
             // re-assert (#371) — same body, but the pre-change valid period
             // must stay reconstructable.
             if content_changed || audit_period_change {
-                tx.execute(
-                    "INSERT INTO entity_history
-                     (history_id, id, category, key, body_json, status, type, tags,
-                      decay_score, retrieval_count, layer, topic_path, archived,
-                      archive_reason, links, verified, source, always_on, certainty,
-                      workspace_hash, agent_id, visibility, valid_from_unix_ms,
-                      valid_to_unix_ms, recorded_at_unix_ms, invalidated_at_unix_ms,
-                      supersedes, superseded_by, created_at_unix_ms, last_accessed_unix_ms)
-                     SELECT ?1, id, category, key, body_json, status, type, tags,
-                      decay_score, retrieval_count, layer, topic_path, archived,
-                      archive_reason, links, verified, source, always_on, certainty,
-                      workspace_hash, agent_id, visibility, valid_from_unix_ms,
-                      valid_to_unix_ms, recorded_at_unix_ms, ?2,
-                      supersedes, ?3, created_at_unix_ms, last_accessed_unix_ms
-                     FROM entities WHERE id = ?3",
-                    params![history_id, now, id],
-                )?;
+                Self::snapshot_live_row_to_history(&tx, &history_id, now, &id)?;
             }
 
             tx.execute(
@@ -3614,10 +3598,43 @@ impl Database {
         Ok(())
     }
 
+    /// Snapshot the current live row of `id` into `entity_history`, retired at
+    /// transaction time `invalidated_at` and linked back to the live id via
+    /// `superseded_by`. All other columns (incl. the prior recorded_at) are
+    /// copied verbatim, so the version was live during
+    /// [recorded_at, invalidated_at). Shared by the remember supersession /
+    /// audited-re-assert path (#371) and the audited set_valid_to close (#373);
+    /// the caller owns the transaction and the follow-up stamp of the live row.
+    fn snapshot_live_row_to_history(
+        conn: &rusqlite::Connection,
+        history_id: &str,
+        invalidated_at: i64,
+        id: &str,
+    ) -> Result<(), rusqlite::Error> {
+        conn.execute(
+            "INSERT INTO entity_history
+             (history_id, id, category, key, body_json, status, type, tags,
+              decay_score, retrieval_count, layer, topic_path, archived,
+              archive_reason, links, verified, source, always_on, certainty,
+              workspace_hash, agent_id, visibility, valid_from_unix_ms,
+              valid_to_unix_ms, recorded_at_unix_ms, invalidated_at_unix_ms,
+              supersedes, superseded_by, created_at_unix_ms, last_accessed_unix_ms)
+             SELECT ?1, id, category, key, body_json, status, type, tags,
+              decay_score, retrieval_count, layer, topic_path, archived,
+              archive_reason, links, verified, source, always_on, certainty,
+              workspace_hash, agent_id, visibility, valid_from_unix_ms,
+              valid_to_unix_ms, recorded_at_unix_ms, ?2,
+              supersedes, ?3, created_at_unix_ms, last_accessed_unix_ms
+             FROM entities WHERE id = ?3",
+            params![history_id, invalidated_at, id],
+        )?;
+        Ok(())
+    }
+
     /// Close an entity's application-time period (#363): record when the fact
     /// stopped being true in the world. Used by mimir_supersede — superseding
     /// a fact ends its validity (at transaction time unless the caller says
-    /// when). Does not touch the transaction axis.
+    /// when).
     ///
     /// Conservative by construction (#363 review):
     ///   * Refuses `valid_to <= valid_from` — an inverted period would shadow
@@ -3628,16 +3645,23 @@ impl Database {
     ///     revive it). Tightening (an earlier close) is allowed; when the
     ///     stored close is already at-or-before the requested one, it is kept.
     ///
+    /// AUDITED (#373, mirroring the #371/#372 audited re-assert): an effective
+    /// close/tighten snapshots the pre-change row to entity_history and
+    /// advances the live row's transaction time, so as_of/bitemporal_at at a
+    /// tx instant before the close still reconstruct the fact as open. A
+    /// no-op call (stored close kept) writes NO snapshot and touches nothing.
+    ///
     /// Returns the EFFECTIVE close instant (the requested one, or the earlier
     /// stored close that was kept).
     pub fn set_valid_to(&self, id: &str, valid_to: i64) -> Result<i64, Box<dyn std::error::Error>> {
         let conn = self.conn()?;
-        let (eff_from, cur_to): (i64, Option<i64>) = conn.query_row(
+        let (eff_from, cur_to, old_rec): (i64, Option<i64>, i64) = conn.query_row(
             "SELECT COALESCE(valid_from_unix_ms, recorded_at_unix_ms, created_at_unix_ms), \
-                    valid_to_unix_ms \
+                    valid_to_unix_ms, \
+                    COALESCE(recorded_at_unix_ms, created_at_unix_ms) \
              FROM entities WHERE id = ?1",
             params![id],
-            |r| Ok((r.get(0)?, r.get(1)?)),
+            |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?)),
         )?;
         if valid_to <= eff_from {
             return Err(format!(
@@ -3649,14 +3673,32 @@ impl Database {
         if let Some(cur) = cur_to {
             if cur <= valid_to {
                 // Already closed at or before the requested instant: keep the
-                // earlier close (never extend validity).
+                // earlier close (never extend validity). No-op — no snapshot.
                 return Ok(cur);
             }
         }
-        conn.execute(
-            "UPDATE entities SET valid_to_unix_ms = ?1 WHERE id = ?2",
-            params![valid_to, id],
+        // #373: audited close. Snapshot the pre-close version (invalidated at
+        // `now`, bumped strictly past the old recorded_at so the history
+        // window is never zero-width — same guarantee as remember's), advance
+        // the live row's recorded_at and link supersedes, and pin a
+        // still-NULL valid_from (pre-v9 rows a legacy binary may still write)
+        // to the OLD effective opening — readers derive effective valid_from
+        // via COALESCE(valid_from, recorded_at, …), so advancing recorded_at
+        // over a NULL valid_from would silently shift the opening to `now`.
+        let now = now_ms().max(old_rec + 1);
+        let history_id = format!(
+            "hist-{}",
+            uuid::Uuid::new_v4().to_string().replace('-', "")[..16].to_string()
+        );
+        let tx = conn.unchecked_transaction()?;
+        Self::snapshot_live_row_to_history(&tx, &history_id, now, id)?;
+        tx.execute(
+            "UPDATE entities SET valid_to_unix_ms = ?1, recorded_at_unix_ms = ?2,
+                supersedes = ?3, valid_from_unix_ms = COALESCE(valid_from_unix_ms, ?4)
+             WHERE id = ?5",
+            params![valid_to, now, history_id, eff_from, id],
         )?;
+        tx.commit()?;
         Ok(valid_to)
     }
 

src/tools.rs

@@ -3228,6 +3228,147 @@ mod tests {
         let _ = std::fs::remove_file(&path);
     }
 
+    #[test]
+    fn set_valid_to_close_is_audited_and_noop_is_not() {
+        // #373: set_valid_to previously wrote no entity_history snapshot, so a
+        // close was invisible to transaction-time reconstruction — queries at
+        // a tx instant BEFORE the close reported the close anyway. An
+        // effective close must snapshot the pre-close (open) version; a no-op
+        // (stored close kept) must not.
+        use std::thread::sleep;
+        use std::time::Duration;
+        let (db, path) = temp_db();
+
+        handle_remember(
+            &db,
+            json!({"category": "facts", "key": "svt", "body_json": "{\"note\":\"open fact\"}"}),
+        )
+        .expect("open fact");
+        let ent = db.get_entity("facts", "svt").unwrap().unwrap();
+        assert!(db.history_versions("facts", "svt").unwrap().is_empty());
+
+        sleep(Duration::from_millis(5));
+        let tx_open = now_ms(); // while the fact was still believed open
+        sleep(Duration::from_millis(5));
+
+        let closed = db.set_valid_to(&ent.id, now_ms()).expect("close");
+        assert_eq!(
+            db.history_versions("facts", "svt").unwrap().len(),
+            1,
+            "an effective close must write exactly one snapshot"
+        );
+
+        // As of tx_open the fact reconstructs OPEN (the pre-close snapshot
+        // answers, valid_to unbounded)…
+        let r = handle_bitemporal(
+            &db,
+            json!({"category": "facts", "key": "svt",
+                   "tx_at_unix_ms": tx_open, "valid_at_unix_ms": closed + 60_000}),
+        )
+        .expect("bitemporal pre-close knowledge");
+        let v: Value = serde_json::from_str(&r).unwrap();
+        assert_eq!(v["found"], json!(true), "pre-close knowledge must not show the close: {r}");
+        assert_eq!(v["valid_to_unix_ms"], Value::Null, "{r}");
+        assert_eq!(v["is_live_version"], json!(false), "{r}");
+        // …while current knowledge shows the close: same instant unanswerable,
+        // an in-period instant reports valid_to = closed.
+        let r = handle_bitemporal(
+            &db,
+            json!({"category": "facts", "key": "svt",
+                   "tx_at_unix_ms": now_ms(), "valid_at_unix_ms": closed + 60_000}),
+        )
+        .expect("bitemporal post-close knowledge");
+        let v: Value = serde_json::from_str(&r).unwrap();
+        assert_eq!(v["found"], json!(false), "{r}");
+        let r = handle_bitemporal(
+            &db,
+            json!({"category": "facts", "key": "svt",
+                   "tx_at_unix_ms": now_ms(), "valid_at_unix_ms": closed - 1}),
+        )
+        .expect("bitemporal in-period cell");
+        let v: Value = serde_json::from_str(&r).unwrap();
+        assert_eq!(v["found"], json!(true), "{r}");
+        assert_eq!(v["valid_to_unix_ms"], json!(closed), "{r}");
+
+        // No-op calls (same value, or a later one — the earlier close is
+        // kept) write NO snapshot.
+        assert_eq!(db.set_valid_to(&ent.id, closed).expect("same-value no-op"), closed);
+        assert_eq!(
+            db.set_valid_to(&ent.id, closed + 10_000).expect("would-extend no-op"),
+            closed
+        );
+        assert_eq!(
+            db.history_versions("facts", "svt").unwrap().len(),
+            1,
+            "no-op set_valid_to must not snapshot"
+        );
+
+        let _ = std::fs::remove_file(&path);
+    }
+
+    #[test]
+    fn supersede_close_inherits_the_audit_snapshot() {
+        // #373: mimir_supersede funnels through set_valid_to, so closing the
+        // old fact's period now snapshots it — the pre-supersede open version
+        // stays reconstructable at earlier transaction instants.
+        use std::thread::sleep;
+        use std::time::Duration;
+        let (db, path) = temp_db();
+
+        handle_remember(
+            &db,
+            json!({"category": "facts", "key": "sup-old", "body_json": "{\"note\":\"old\"}"}),
+        )
+        .expect("old");
+        handle_remember(
+            &db,
+            json!({"category": "facts", "key": "sup-new", "body_json": "{\"note\":\"new\"}"}),
+        )
+        .expect("new");
+        assert!(db.history_versions("facts", "sup-old").unwrap().is_empty());
+
+        sleep(Duration::from_millis(5));
+        let tx_open = now_ms();
+        sleep(Duration::from_millis(5));
+
+        let r = handle_supersede(
+            &db,
+            json!({"from_category": "facts", "from_key": "sup-old",
+                   "to_category": "facts", "to_key": "sup-new"}),
+        )
+        .expect("supersede");
+        let v: Value = serde_json::from_str(&r).unwrap();
+        let closed_at = v["from_valid_to_unix_ms"].as_i64().expect("close instant");
+
+        assert_eq!(
+            db.history_versions("facts", "sup-old").unwrap().len(),
+            1,
+            "supersede's close must inherit the audit snapshot"
+        );
+        // Pre-supersede knowledge still believes the old fact open at an
+        // instant the close later excluded.
+        let r = handle_bitemporal(
+            &db,
+            json!({"category": "facts", "key": "sup-old",
+                   "tx_at_unix_ms": tx_open, "valid_at_unix_ms": closed_at + 60_000}),
+        )
+        .expect("bitemporal pre-supersede knowledge");
+        let v: Value = serde_json::from_str(&r).unwrap();
+        assert_eq!(v["found"], json!(true), "{r}");
+        assert_eq!(v["valid_to_unix_ms"], Value::Null, "{r}");
+        // Current knowledge: closed.
+        let r = handle_bitemporal(
+            &db,
+            json!({"category": "facts", "key": "sup-old",
+                   "tx_at_unix_ms": now_ms(), "valid_at_unix_ms": closed_at + 60_000}),
+        )
+        .expect("bitemporal post-supersede knowledge");
+        let v: Value = serde_json::from_str(&r).unwrap();
+        assert_eq!(v["found"], json!(false), "{r}");
+
+        let _ = std::fs::remove_file(&path);
+    }
+
     #[test]
     fn bitemporal_tool_reports_both_axes() {
         use std::thread::sleep;