Skip to content

Commit a76636e

Browse files
tcconnallyclaude
andcommitted
fix(temporal): review findings — pure-read temporal recall + hardened valid-period writes (#363)
Finding 1 (#356-class value inversion): handle_recall applied the valid-time filter AFTER db.recall(), whose fts5 path (and dense/hybrid with reinforce) reinforces every matched row — so `recall {query, valid_at:T}` bumped retrieval_count/decay/layer on entities the filter then hid, making invisible entities immortal under repeated temporal queries. The inner recall is now forced to a pure read whenever a valid-time filter is present, and the deferred side-effects are applied to the SURVIVING hits only (fts5 always, dense/hybrid only with reinforce:true), mirroring the expansion path. Unfiltered calls keep the original behavior byte-for-byte. Finding 2 (inverted/extended valid periods): - mimir_correct now rejects valid_to <= valid_from (same error shape as remember) — an inverted period would shadow older versions in bitemporal_at while never matching itself, making the fact unanswerable. - mimir_supersede validates an explicit valid_to against the old fact's stored period BEFORE any mutation: rejects valid_to <= valid_from and rejects extending an already-closed period (tighten-only). - db.set_valid_to is now conservative by construction: errors on inversion, never extends an already-bounded valid_to (keeps the earlier close and reports it), allows tightening. The default supersede close is bumped strictly past valid_from so a same-millisecond supersede stays coherent. Review nits: valid_op is validated against the closed enum (unknown strings rejected instead of silently meaning overlaps); the misplaced entity_from_row doc comment is restored (it had attached to TemporalVersion). Tests: 203 -> 208 — hidden-entity non-reinforcement, unfiltered-recall behavior identity (side-effects + result set), unknown valid_op rejection, inverted-period rejection on correct, and the full supersede matrix (invert-reject pre-mutation, extend-reject, default keeps earlier close, tighten allowed). Temporal gate still 100% (20/20, identical signature). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 6205151 commit a76636e

3 files changed

Lines changed: 329 additions & 8 deletions

File tree

src/db.rs

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3482,13 +3482,46 @@ impl Database {
34823482
/// stopped being true in the world. Used by mimir_supersede — superseding
34833483
/// a fact ends its validity (at transaction time unless the caller says
34843484
/// when). Does not touch the transaction axis.
3485-
pub fn set_valid_to(&self, id: &str, valid_to: i64) -> Result<(), Box<dyn std::error::Error>> {
3485+
///
3486+
/// Conservative by construction (#363 review):
3487+
/// * Refuses `valid_to <= valid_from` — an inverted period would shadow
3488+
/// older versions in `bitemporal_at` while never matching itself,
3489+
/// making the fact unanswerable.
3490+
/// * Never EXTENDS an already-bounded valid_to — a fact that already
3491+
/// ended stays ended (a default-now supersede must not retroactively
3492+
/// revive it). Tightening (an earlier close) is allowed; when the
3493+
/// stored close is already at-or-before the requested one, it is kept.
3494+
///
3495+
/// Returns the EFFECTIVE close instant (the requested one, or the earlier
3496+
/// stored close that was kept).
3497+
pub fn set_valid_to(&self, id: &str, valid_to: i64) -> Result<i64, Box<dyn std::error::Error>> {
34863498
let conn = self.conn()?;
3499+
let (eff_from, cur_to): (i64, Option<i64>) = conn.query_row(
3500+
"SELECT COALESCE(valid_from_unix_ms, recorded_at_unix_ms, created_at_unix_ms), \
3501+
valid_to_unix_ms \
3502+
FROM entities WHERE id = ?1",
3503+
params![id],
3504+
|r| Ok((r.get(0)?, r.get(1)?)),
3505+
)?;
3506+
if valid_to <= eff_from {
3507+
return Err(format!(
3508+
"valid_to ({valid_to}) must be greater than the fact's valid_from ({eff_from}) — \
3509+
refusing to invert the valid period"
3510+
)
3511+
.into());
3512+
}
3513+
if let Some(cur) = cur_to {
3514+
if cur <= valid_to {
3515+
// Already closed at or before the requested instant: keep the
3516+
// earlier close (never extend validity).
3517+
return Ok(cur);
3518+
}
3519+
}
34873520
conn.execute(
34883521
"UPDATE entities SET valid_to_unix_ms = ?1 WHERE id = ?2",
34893522
params![valid_to, id],
34903523
)?;
3491-
Ok(())
3524+
Ok(valid_to)
34923525
}
34933526

34943527
/// Find entities with identical (category, key) and merge/archive duplicates, keeping the newest.
@@ -6827,7 +6860,6 @@ pub(crate) fn sanitize_prompt_field(s: &str) -> String {
68276860
s.replace('<', "&lt;").replace('>', "&gt;")
68286861
}
68296862

6830-
/// Extract an Entity from a SQLite row, decrypting body_json if encryption is enabled.
68316863
/// One version of a fact with its bi-temporal coordinates (#363): the entity
68326864
/// content plus its application-time (valid_from/valid_to) and transaction-time
68336865
/// (recorded_at/invalidated_at) periods. Returned by `valid_at`/`bitemporal_at`
@@ -6886,6 +6918,7 @@ pub fn valid_period_contains_instant(row_from: i64, row_to: Option<i64>, t: i64)
68866918
row_from <= t && row_to.map_or(true, |rt| t < rt)
68876919
}
68886920

6921+
/// Extract an Entity from a SQLite row, decrypting body_json if encryption is enabled.
68896922
fn entity_from_row(
68906923
row: &rusqlite::Row,
68916924
encryption: Option<&EncryptionManager>,

src/mcp.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3119,7 +3119,7 @@ fn list_tools(id: Option<Value>) -> JsonRpcResponse {
31193119
},
31203120
"valid_to_unix_ms": {
31213121
"type": "integer",
3122-
"description": "When the OLD fact stopped being true in the world (#363, unix ms). Defaults to transaction time (now). Closes the old entity's application-time period so mimir_valid_at stops returning it from that instant on."
3122+
"description": "When the OLD fact stopped being true in the world (#363, unix ms). Defaults to transaction time (now). Closes the old entity's application-time period so mimir_valid_at stops returning it from that instant on. Must be after the fact's valid_from, and may only TIGHTEN an already-closed period (a fact that ended cannot be retroactively extended); violations are rejected before any mutation."
31233123
}
31243124
},
31253125
"required": [

0 commit comments

Comments
 (0)