From fd4dd89bbbe16ff3f00d7e104523d21abd91ea94 Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Sun, 10 May 2026 20:56:56 -0400 Subject: [PATCH] knowledge: agent self-correction with audit + per-persona gate + revert MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bundles three coupled changes so agent-side mutations stay auditable and reversible: 1. Audit columns on entity_facts — `last_modified_by_model` / `last_modified_by_backend` / `last_modified_at`. Stamped on every mutation path (update_fact, supersede_fact, manual PATCH, manual supersede, the new revert). NULL on rows never touched since creation. Partial index on `last_modified_at WHERE NOT NULL` keeps the "show me recent edits" feed fast without bloating from legacy rows. 2. Per-persona gate `personas.allow_agent_corrections` (BOOLEAN, default 0). Defense in depth at two layers: - build_tool_definitions: when off, `update_fact` and `supersede_fact` aren't in the catalog at all, so even a hallucinated tool call by the model fails fast. - tool_update_fact / tool_supersede_fact: re-checks the persona flag at call time and returns an explicit "corrections disabled" error if it's somehow off (e.g. flag flipped mid- loop). ToolGateOpts grows the flag; current_gate_opts splits into `current_gate_opts` (no persona context, defaults closed) + `current_gate_opts_for_persona` for chat callers that have a persona id. Both call sites in insight_chat are updated. 3. Revert action — new DAO method `revert_supersession` + `POST /knowledge/facts/{id}/restore`. Flips status back to 'active', clears `superseded_by`, clears `valid_until` (we don't track whether it was hand-set vs auto-stamped, so the safe reset is to drop it — user can re-bound after). Stamps `last_modified_*` so the revert itself is attributable. Manual paths (PATCH / supersede via HTTP, plus restore) stamp the audit columns with `("manual", "manual")`. Agent paths stamp the loop-time chat model and backend (mirroring the existing created_by_* convention). FactDetail in the HTTP response now carries the audit triple alongside the existing provenance. Apollo wires the new field set in the matching commit. PersonaView / UpdatePersonaRequest grow `allowAgentCorrections`; the PersonaPatch + InsertPersona + bulk_import paths thread it. 317 lib tests pass, including unchanged update_fact / supersede DAO tests (now passing audit=None — None means "no provenance context to attribute", legacy semantics). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../down.sql | 5 + .../up.sql | 30 +++ src/ai/insight_chat.rs | 15 +- src/ai/insight_generator.rs | 246 ++++++++++++++++++ src/database/knowledge_dao.rs | 139 +++++++++- src/database/models.rs | 17 ++ src/database/persona_dao.rs | 13 + src/database/schema.rs | 4 + src/knowledge.rs | 44 +++- src/personas.rs | 9 + 10 files changed, 515 insertions(+), 7 deletions(-) create mode 100644 migrations/2026-05-10-000500_entity_facts_audit_and_agent_gate/down.sql create mode 100644 migrations/2026-05-10-000500_entity_facts_audit_and_agent_gate/up.sql diff --git a/migrations/2026-05-10-000500_entity_facts_audit_and_agent_gate/down.sql b/migrations/2026-05-10-000500_entity_facts_audit_and_agent_gate/down.sql new file mode 100644 index 0000000..0d83de6 --- /dev/null +++ b/migrations/2026-05-10-000500_entity_facts_audit_and_agent_gate/down.sql @@ -0,0 +1,5 @@ +ALTER TABLE personas DROP COLUMN allow_agent_corrections; +DROP INDEX IF EXISTS idx_entity_facts_last_modified_at; +ALTER TABLE entity_facts DROP COLUMN last_modified_at; +ALTER TABLE entity_facts DROP COLUMN last_modified_by_backend; +ALTER TABLE entity_facts DROP COLUMN last_modified_by_model; diff --git a/migrations/2026-05-10-000500_entity_facts_audit_and_agent_gate/up.sql b/migrations/2026-05-10-000500_entity_facts_audit_and_agent_gate/up.sql new file mode 100644 index 0000000..1951131 --- /dev/null +++ b/migrations/2026-05-10-000500_entity_facts_audit_and_agent_gate/up.sql @@ -0,0 +1,30 @@ +-- Three coupled changes for agent self-correction safety: +-- +-- 1. `entity_facts.last_modified_by_*` + `last_modified_at` track who +-- most recently mutated each fact. `created_by_*` from migration +-- 2026-05-10-000300 records who first wrote the row; this records +-- who last *changed* it. Separate columns so the create vs update +-- audit is independently grep-able ("show me every fact gpt-5 +-- altered last week" stays a single index scan). +-- +-- 2. `personas.allow_agent_corrections` is the gate for the new +-- agent-side `update_fact` / `supersede_fact` tools. Default OFF — +-- a fresh persona's agent can create but can't alter or replace. +-- Operator opts in per-persona after the model has earned trust, +-- typically via the strict-mode flow (curate, then ratchet up +-- agent autonomy as confidence rises). Parallel in shape to +-- `reviewed_only_facts` from 2026-05-10-000400; they compose. +-- +-- 3. Index on `last_modified_at` (partial, NOT NULL) for the +-- audit-feed reads in the curation UI ("show recent agent edits +-- sorted newest first"). + +ALTER TABLE entity_facts ADD COLUMN last_modified_by_model TEXT; +ALTER TABLE entity_facts ADD COLUMN last_modified_by_backend TEXT; +ALTER TABLE entity_facts ADD COLUMN last_modified_at BIGINT; + +CREATE INDEX idx_entity_facts_last_modified_at + ON entity_facts(last_modified_at) + WHERE last_modified_at IS NOT NULL; + +ALTER TABLE personas ADD COLUMN allow_agent_corrections BOOLEAN NOT NULL DEFAULT 0; diff --git a/src/ai/insight_chat.rs b/src/ai/insight_chat.rs index 98bd59b..adb2da4 100644 --- a/src/ai/insight_chat.rs +++ b/src/ai/insight_chat.rs @@ -405,7 +405,10 @@ impl InsightChatService { // and probes the per-table presence flags. Pass `offer_describe_tool` // directly — the `!is_hybrid && local_first_user_has_image` decision // is the chat-path's vision predicate. - let gate_opts = self.generator.current_gate_opts(offer_describe_tool); + let gate_opts = self.generator.current_gate_opts_for_persona( + offer_describe_tool, + Some((req.user_id, &active_persona)), + ); let tools = InsightGenerator::build_tool_definitions(gate_opts); // Image base64 only needed when describe_photo is on the menu. Load @@ -837,7 +840,10 @@ impl InsightChatService { .map(|imgs| !imgs.is_empty()) .unwrap_or(false); let offer_describe_tool = !is_hybrid && local_first_user_has_image; - let gate_opts = self.generator.current_gate_opts(offer_describe_tool); + let gate_opts = self.generator.current_gate_opts_for_persona( + offer_describe_tool, + Some((req.user_id, &active_persona)), + ); let tools = InsightGenerator::build_tool_definitions(gate_opts); let image_base64: Option = if offer_describe_tool { @@ -1026,7 +1032,10 @@ impl InsightChatService { // the chat model can re-look at the photo on demand. Hybrid: // already inlined, no tool needed. let offer_describe_tool = !is_hybrid && image_base64.is_some(); - let gate_opts = self.generator.current_gate_opts(offer_describe_tool); + let gate_opts = self.generator.current_gate_opts_for_persona( + offer_describe_tool, + Some((req.user_id, &active_persona)), + ); let tools = InsightGenerator::build_tool_definitions(gate_opts); // System message = persona + photo context block. Photo context diff --git a/src/ai/insight_generator.rs b/src/ai/insight_generator.rs index 362c508..af3abbc 100644 --- a/src/ai/insight_generator.rs +++ b/src/ai/insight_generator.rs @@ -108,6 +108,12 @@ pub struct ToolGateOpts { pub calendar_present: bool, pub location_history_present: bool, pub faces_present: bool, + /// Per-persona toggle from migration 2026-05-10-000500. When + /// false the agent's update_fact / supersede_fact tools aren't + /// in the catalog at all — defense-in-depth so a hallucinated + /// tool name still 404s, and the agent doesn't waste iterations + /// trying corrections it isn't allowed to do. + pub allow_agent_corrections: bool, } impl InsightGenerator { @@ -164,6 +170,20 @@ impl InsightGenerator { /// supplied by the caller because it depends on the model selected /// for this turn, not on persistent state. pub fn current_gate_opts(&self, has_vision: bool) -> ToolGateOpts { + self.current_gate_opts_for_persona(has_vision, None) + } + + /// Same as `current_gate_opts` but resolves the per-persona + /// `allow_agent_corrections` flag too. Pass `Some((user_id, + /// persona_id))` when generating in a persona context (every chat + /// turn does); pass `None` for callers that don't have one yet + /// (cold paths, populate_knowledge bin), which defaults the gate + /// to closed — the conservative posture. + pub fn current_gate_opts_for_persona( + &self, + has_vision: bool, + persona: Option<(i32, &str)>, + ) -> ToolGateOpts { let cx = opentelemetry::Context::new(); let calendar_present = { let mut dao = self @@ -190,6 +210,16 @@ impl InsightGenerator { let mut dao = self.face_dao.lock().expect("Unable to lock FaceDao"); dao.has_any_faces(&cx).unwrap_or(false) }; + let allow_agent_corrections = persona + .and_then(|(uid, pid)| { + let mut pdao = self + .persona_dao + .lock() + .expect("Unable to lock PersonaDao"); + pdao.get_persona(&cx, uid, pid).ok().flatten() + }) + .map(|p| p.allow_agent_corrections) + .unwrap_or(false); ToolGateOpts { has_vision, apollo_enabled: self.apollo_enabled(), @@ -197,6 +227,7 @@ impl InsightGenerator { calendar_present, location_history_present, faces_present, + allow_agent_corrections, } } @@ -1592,6 +1623,14 @@ Return ONLY the summary, nothing else."#, ) .await } + "update_fact" => { + self.tool_update_fact(arguments, user_id, persona_id, model, backend, cx) + .await + } + "supersede_fact" => { + self.tool_supersede_fact(arguments, user_id, persona_id, model, backend, cx) + .await + } "get_current_datetime" => Self::tool_get_current_datetime(), unknown => format!("Unknown tool: {}", unknown), }; @@ -2761,6 +2800,12 @@ Return ONLY the summary, nothing else."#, superseded_by: None, created_by_model: Some(model.to_string()), created_by_backend: Some(backend.to_string()), + // Initial write — no modification yet; last_modified_* + // intentionally NULL so the audit feed only shows real + // post-creation changes. + last_modified_by_model: None, + last_modified_by_backend: None, + last_modified_at: None, }; let mut kdao = self @@ -2800,6 +2845,151 @@ Return ONLY the summary, nothing else."#, ) } + /// Tool: update_fact — patch a fact's mutable fields. Gated by the + /// active persona's `allow_agent_corrections` flag at the schema / + /// catalog layer (build_tool_definitions); rechecked here as a + /// defense in depth in case a hallucinated tool call slips + /// through. + async fn tool_update_fact( + &self, + args: &serde_json::Value, + user_id: i32, + persona_id: &str, + model: &str, + backend: &str, + cx: &opentelemetry::Context, + ) -> String { + use crate::database::FactPatch; + + // Defense-in-depth gate check. + let allowed = { + let mut pdao = self.persona_dao.lock().expect("Unable to lock PersonaDao"); + pdao.get_persona(cx, user_id, persona_id) + .ok() + .flatten() + .map(|p| p.allow_agent_corrections) + .unwrap_or(false) + }; + if !allowed { + return "Error: agent corrections are disabled for this persona. Ask the operator to flip allow_agent_corrections.".to_string(); + } + + let fact_id = match args.get("fact_id").and_then(|v| v.as_i64()) { + Some(id) => id as i32, + None => return "Error: missing required parameter 'fact_id'".to_string(), + }; + + // Build the patch from any fields present on `args`. valid_* + // are tri-state — JSON null → Some(None) → clear back to NULL, + // omitted → None → leave alone, value → Some(Some(value)) → + // set. The match-on-presence pattern below mirrors the HTTP + // PATCH path's serde-helper behaviour. + let parse_optional_i64 = + |v: Option<&serde_json::Value>| -> Option> { + v.map(|val| { + if val.is_null() { + None + } else { + val.as_i64() + } + }) + }; + + let patch = FactPatch { + predicate: args + .get("predicate") + .and_then(|v| v.as_str()) + .map(|s| s.to_string()), + object_value: args + .get("object_value") + .and_then(|v| v.as_str()) + .map(|s| s.to_string()), + status: args + .get("status") + .and_then(|v| v.as_str()) + .filter(|s| matches!(*s, "active" | "reviewed" | "rejected")) + .map(|s| s.to_string()), + confidence: args + .get("confidence") + .and_then(|v| v.as_f64()) + .map(|f| f.clamp(0.0, 0.95) as f32), + valid_from: parse_optional_i64(args.get("valid_from")), + valid_until: parse_optional_i64(args.get("valid_until")), + }; + + log::info!("tool_update_fact: fact_id={}", fact_id); + + let mut kdao = self + .knowledge_dao + .lock() + .expect("Unable to lock KnowledgeDao"); + + match kdao.update_fact(cx, fact_id, patch, Some((model, backend))) { + Ok(Some(f)) => format!( + "Updated fact ID:{} (status={}, confidence={:.2})", + f.id, f.status, f.confidence + ), + Ok(None) => format!("Error: fact ID:{} not found", fact_id), + Err(e) => format!("Error updating fact: {:?}", e), + } + } + + /// Tool: supersede_fact — replace one fact with another. Same + /// gating as update_fact. + async fn tool_supersede_fact( + &self, + args: &serde_json::Value, + user_id: i32, + persona_id: &str, + model: &str, + backend: &str, + cx: &opentelemetry::Context, + ) -> String { + let allowed = { + let mut pdao = self.persona_dao.lock().expect("Unable to lock PersonaDao"); + pdao.get_persona(cx, user_id, persona_id) + .ok() + .flatten() + .map(|p| p.allow_agent_corrections) + .unwrap_or(false) + }; + if !allowed { + return "Error: agent corrections are disabled for this persona.".to_string(); + } + + let old_fact_id = match args.get("old_fact_id").and_then(|v| v.as_i64()) { + Some(id) => id as i32, + None => return "Error: missing required parameter 'old_fact_id'".to_string(), + }; + let new_fact_id = match args.get("new_fact_id").and_then(|v| v.as_i64()) { + Some(id) => id as i32, + None => return "Error: missing required parameter 'new_fact_id'".to_string(), + }; + if old_fact_id == new_fact_id { + return "Error: old_fact_id and new_fact_id must differ".to_string(); + } + + log::info!( + "tool_supersede_fact: old={}, new={}", + old_fact_id, + new_fact_id + ); + + let mut kdao = self + .knowledge_dao + .lock() + .expect("Unable to lock KnowledgeDao"); + + match kdao.supersede_fact(cx, old_fact_id, new_fact_id, Some((model, backend))) { + Ok(Some(f)) => format!( + "Superseded fact ID:{} (now status={}, valid_until={:?})", + f.id, f.status, f.valid_until + ), + Ok(None) => "Error: old or new fact not found".to_string(), + Err(e) => format!("Error superseding fact: {:?}", e), + } + } + /// Tool: get_current_datetime — returns the current local date and time fn tool_get_current_datetime() -> String { let now = Local::now(); @@ -3049,6 +3239,55 @@ Return ONLY the summary, nothing else."#, }), )); + // Self-correction tools — only exposed when the active persona + // has allow_agent_corrections=true. Gating happens here AND in + // the tool method itself (the runtime check is the load-bearing + // one; this just keeps the tool out of the model's catalog so + // it doesn't waste iterations trying calls it can't make). + if opts.allow_agent_corrections { + tools.push(Tool::function( + "update_fact", + "Correct an existing fact in the knowledge memory. Use sparingly — only when you have \ + stronger evidence than the original write justified (e.g. a clearer photo, a \ + contradicting timestamp on a related fact, or explicit user correction). Common \ + patches: tighten `valid_from` / `valid_until` to a known interval, downgrade \ + `confidence` after seeing contradicting evidence, or flip `status` to 'reviewed' if \ + you've verified the fact independently. Cannot change subject / object — supersede \ + instead. Pass `fact_id` plus any subset of patchable fields.", + serde_json::json!({ + "type": "object", + "required": ["fact_id"], + "properties": { + "fact_id": { "type": "integer", "description": "ID of the fact to patch (from recall_facts_for_photo or list)." }, + "predicate": { "type": "string", "description": "New predicate string. Rare." }, + "object_value": { "type": "string", "description": "New free-text object. Use for typed-fact corrections." }, + "status": { "type": "string", "description": "'active' | 'reviewed' | 'rejected'. 'superseded' is for the supersede_fact tool." }, + "confidence": { "type": "number", "description": "0.0–0.95. Lower when you've seen contradicting evidence." }, + "valid_from": { "type": "integer", "description": "Unix-seconds lower bound on when the fact began being true. Null clears." }, + "valid_until": { "type": "integer", "description": "Unix-seconds upper bound on when the fact stopped being true. Null clears." } + } + }), + )); + + tools.push(Tool::function( + "supersede_fact", + "Mark an old fact as replaced by a newer one. Use when the new fact contradicts the \ + old AND the contradiction is a *time-bounded change* (relationship changed, address \ + changed, role changed), not a correction of a mistake — for mistakes, set the old \ + fact's status to 'rejected' via update_fact. Atomically: flips old.status to \ + 'superseded', points old.superseded_by at the new fact, and stamps old.valid_until \ + from new.valid_from (when not already set) so the two intervals are disjoint.", + serde_json::json!({ + "type": "object", + "required": ["old_fact_id", "new_fact_id"], + "properties": { + "old_fact_id": { "type": "integer", "description": "The fact being replaced." }, + "new_fact_id": { "type": "integer", "description": "The fact that replaces it. Must already exist (use store_fact first if you're recording a new one)." } + } + }), + )); + } + tools.push(Tool::function( "get_current_datetime", "Get the current date and time. Useful when reasoning about how long ago a photo was taken.", @@ -4008,6 +4247,7 @@ mod tests { calendar_present: false, location_history_present: false, faces_present: false, + allow_agent_corrections: false, }; let tools = InsightGenerator::build_tool_definitions(opts); let names: Vec<&str> = tools.iter().map(|t| t.function.name.as_str()).collect(); @@ -4030,6 +4270,9 @@ mod tests { assert!(!names.contains(&"get_calendar_events")); assert!(!names.contains(&"get_location_history")); assert!(!names.contains(&"get_faces_in_photo")); + // Agent-correction tools are absent without the gate. + assert!(!names.contains(&"update_fact")); + assert!(!names.contains(&"supersede_fact")); } #[test] @@ -4041,6 +4284,7 @@ mod tests { calendar_present: true, location_history_present: true, faces_present: true, + allow_agent_corrections: true, }; let tools = InsightGenerator::build_tool_definitions(opts); let names: Vec<&str> = tools.iter().map(|t| t.function.name.as_str()).collect(); @@ -4050,6 +4294,8 @@ mod tests { assert!(names.contains(&"get_calendar_events")); assert!(names.contains(&"get_location_history")); assert!(names.contains(&"get_faces_in_photo")); + assert!(names.contains(&"update_fact")); + assert!(names.contains(&"supersede_fact")); } fn place(name: &str, description: &str) -> ApolloPlace { diff --git a/src/database/knowledge_dao.rs b/src/database/knowledge_dao.rs index 68b3da4..64c3674 100644 --- a/src/database/knowledge_dao.rs +++ b/src/database/knowledge_dao.rs @@ -210,11 +210,17 @@ pub trait KnowledgeDao: Sync + Send { filter: FactFilter, ) -> Result<(Vec, i64), DbError>; + /// Update a fact. `audit` stamps the row's `last_modified_*` + /// columns — None = legacy internal callers without provenance + /// context; HTTP passes `Some(("manual", "manual"))`; the agent + /// passes its loop-time model + backend so the audit trail can + /// distinguish human edits from agent corrections. fn update_fact( &mut self, cx: &opentelemetry::Context, id: i32, patch: FactPatch, + audit: Option<(&str, &str)>, ) -> Result, DbError>; fn update_facts_insight_id( @@ -232,6 +238,7 @@ pub trait KnowledgeDao: Sync + Send { /// - sets old.status = 'superseded' /// - stamps old.valid_until = new.valid_from (if not already /// set; otherwise leaves it) + /// - stamps old.last_modified_* from `audit` /// /// Returns the updated old fact. Errors if either id is missing. fn supersede_fact( @@ -239,6 +246,22 @@ pub trait KnowledgeDao: Sync + Send { cx: &opentelemetry::Context, old_id: i32, new_id: i32, + audit: Option<(&str, &str)>, + ) -> Result, DbError>; + + /// Undo a supersession: clear `superseded_by`, flip status back to + /// 'active', clear `valid_until` (we don't know if it was auto- + /// stamped by the supersede or hand-set, so the conservative reset + /// is to clear it — user can re-bound after). Stamps the audit + /// columns so the revert is itself attributable. + /// + /// Returns the restored fact. Errors if the fact doesn't exist or + /// wasn't superseded in the first place (no-op semantics). + fn revert_supersession( + &mut self, + cx: &opentelemetry::Context, + fact_id: i32, + audit: Option<(&str, &str)>, ) -> Result, DbError>; // --- Photo links --- @@ -1069,46 +1092,72 @@ impl KnowledgeDao for SqliteKnowledgeDao { cx: &opentelemetry::Context, fact_id: i32, patch: FactPatch, + audit: Option<(&str, &str)>, ) -> Result, DbError> { trace_db_call(cx, "update", "update_fact", |_span| { use schema::entity_facts::dsl::*; let mut conn = self.connection.lock().expect("KnowledgeDao lock"); + let mut touched = false; if let Some(ref new_predicate) = patch.predicate { diesel::update(entity_facts.filter(id.eq(fact_id))) .set(predicate.eq(new_predicate)) .execute(conn.deref_mut()) .map_err(|e| anyhow::anyhow!("Update error: {}", e))?; + touched = true; } if let Some(ref new_value) = patch.object_value { diesel::update(entity_facts.filter(id.eq(fact_id))) .set(object_value.eq(new_value)) .execute(conn.deref_mut()) .map_err(|e| anyhow::anyhow!("Update error: {}", e))?; + touched = true; } if let Some(ref new_status) = patch.status { diesel::update(entity_facts.filter(id.eq(fact_id))) .set(status.eq(new_status)) .execute(conn.deref_mut()) .map_err(|e| anyhow::anyhow!("Update error: {}", e))?; + touched = true; } if let Some(new_confidence) = patch.confidence { diesel::update(entity_facts.filter(id.eq(fact_id))) .set(confidence.eq(new_confidence)) .execute(conn.deref_mut()) .map_err(|e| anyhow::anyhow!("Update error: {}", e))?; + touched = true; } if let Some(new_from) = patch.valid_from { diesel::update(entity_facts.filter(id.eq(fact_id))) .set(valid_from.eq(new_from)) .execute(conn.deref_mut()) .map_err(|e| anyhow::anyhow!("Update error: {}", e))?; + touched = true; } if let Some(new_until) = patch.valid_until { diesel::update(entity_facts.filter(id.eq(fact_id))) .set(valid_until.eq(new_until)) .execute(conn.deref_mut()) .map_err(|e| anyhow::anyhow!("Update error: {}", e))?; + touched = true; + } + + // Only stamp the audit columns if we actually changed + // something — empty patches stay quiet. + if touched { + let now = chrono::Utc::now().timestamp(); + let (model_str, backend_str) = match audit { + Some((m, b)) => (Some(m.to_string()), Some(b.to_string())), + None => (None, None), + }; + diesel::update(entity_facts.filter(id.eq(fact_id))) + .set(( + last_modified_by_model.eq(model_str), + last_modified_by_backend.eq(backend_str), + last_modified_at.eq(Some(now)), + )) + .execute(conn.deref_mut()) + .map_err(|e| anyhow::anyhow!("Audit-stamp error: {}", e))?; } entity_facts @@ -1171,6 +1220,7 @@ impl KnowledgeDao for SqliteKnowledgeDao { cx: &opentelemetry::Context, old_id: i32, new_id: i32, + audit: Option<(&str, &str)>, ) -> Result, DbError> { trace_db_call(cx, "update", "supersede_fact", |_span| { use schema::entity_facts::dsl::*; @@ -1182,6 +1232,12 @@ impl KnowledgeDao for SqliteKnowledgeDao { )); } + let now = chrono::Utc::now().timestamp(); + let (audit_model, audit_backend) = match audit { + Some((m, b)) => (Some(m.to_string()), Some(b.to_string())), + None => (None, None), + }; + conn.transaction::, diesel::result::Error, _>( |conn| { // Pull the new fact's valid_from so we can close @@ -1216,6 +1272,9 @@ impl KnowledgeDao for SqliteKnowledgeDao { status.eq("superseded"), superseded_by.eq(Some(new_id)), valid_until.eq(target_valid_until), + last_modified_by_model.eq(audit_model.clone()), + last_modified_by_backend.eq(audit_backend.clone()), + last_modified_at.eq(Some(now)), )) .execute(conn)?; @@ -1238,6 +1297,68 @@ impl KnowledgeDao for SqliteKnowledgeDao { }) } + fn revert_supersession( + &mut self, + cx: &opentelemetry::Context, + fact_id: i32, + audit: Option<(&str, &str)>, + ) -> Result, DbError> { + trace_db_call(cx, "update", "revert_supersession", |_span| { + use schema::entity_facts::dsl::*; + let mut conn = self.connection.lock().expect("KnowledgeDao lock"); + + // Verify the fact exists and was in fact superseded — + // reverting an already-active fact is a no-op and the + // handler can 404 / 409 on the None. + let existing: Option = entity_facts + .filter(id.eq(fact_id)) + .first::(conn.deref_mut()) + .optional() + .map_err(|e| anyhow::anyhow!("Query error: {}", e))?; + let Some(row) = existing else { + return Ok(None); + }; + if row.status != "superseded" && row.superseded_by.is_none() { + // Not superseded — nothing to revert. Returning the + // current row is friendlier than 404 here; the + // handler decides what status to return. + return Ok(Some(row)); + } + + let now = chrono::Utc::now().timestamp(); + let (audit_model, audit_backend) = match audit { + Some((m, b)) => (Some(m.to_string()), Some(b.to_string())), + None => (None, None), + }; + + diesel::update(entity_facts.filter(id.eq(fact_id))) + .set(( + status.eq("active"), + superseded_by.eq::>(None), + // Clear the auto-stamped valid_until. If the user + // had hand-set it pre-supersede we don't have a + // way to know — accepting the loss as the cost of + // a clean revert. Curator can re-bound after. + valid_until.eq::>(None), + last_modified_by_model.eq(audit_model), + last_modified_by_backend.eq(audit_backend), + last_modified_at.eq(Some(now)), + )) + .execute(conn.deref_mut()) + .map_err(|e| anyhow::anyhow!("Revert error: {}", e))?; + + entity_facts + .filter(id.eq(fact_id)) + .first::(conn.deref_mut()) + .optional() + .map_err(|e| anyhow::anyhow!("Query error: {}", e)) + }) + .map_err(|e| { + log::warn!("revert_supersession({}) failed: {}", fact_id, e); + DbError::new(DbErrorKind::UpdateError) + }) + } + // ----------------------------------------------------------------------- // Photo link operations // ----------------------------------------------------------------------- @@ -1421,6 +1542,7 @@ mod tests { created_at: 0, updated_at: 0, reviewed_only_facts: false, + allow_agent_corrections: false, }) .execute(c.deref_mut()) .unwrap(); @@ -1473,6 +1595,9 @@ mod tests { superseded_by: None, created_by_model: None, created_by_backend: None, + last_modified_by_model: None, + last_modified_by_backend: None, + last_modified_at: None, }, ) .unwrap(); @@ -1696,6 +1821,9 @@ mod tests { superseded_by: None, created_by_model: None, created_by_backend: None, + last_modified_by_model: None, + last_modified_by_backend: None, + last_modified_at: None, }, ); assert!( @@ -1747,11 +1875,12 @@ mod tests { valid_from: Some(Some(1640995200)), // 2022-01-01 valid_until: None, }, + None, ) .unwrap(); let updated = dao - .supersede_fact(&cx, old.id, new.id) + .supersede_fact(&cx, old.id, new.id, None) .unwrap() .expect("supersede returned None"); @@ -1777,7 +1906,7 @@ mod tests { let old = add_fact(&mut dao, cameron.id, "lives_in", "NYC", alice, "default"); let new = add_fact(&mut dao, cameron.id, "lives_in", "SF", alice, "default"); - dao.supersede_fact(&cx, old.id, new.id).unwrap().unwrap(); + dao.supersede_fact(&cx, old.id, new.id, None).unwrap().unwrap(); dao.delete_fact(&cx, new.id).unwrap(); let rehydrated = dao @@ -1846,6 +1975,7 @@ mod tests { valid_from: Some(Some(1577836800)), // 2020-01-01 valid_until: Some(Some(1640995200)), // 2022-01-01 }, + None, ) .unwrap() .unwrap(); @@ -1865,6 +1995,7 @@ mod tests { valid_from: None, valid_until: None, }, + None, ) .unwrap() .unwrap(); @@ -1884,6 +2015,7 @@ mod tests { valid_from: None, valid_until: Some(None), }, + None, ) .unwrap() .unwrap(); @@ -1930,6 +2062,9 @@ mod tests { superseded_by: None, created_by_model: None, created_by_backend: None, + last_modified_by_model: None, + last_modified_by_backend: None, + last_modified_at: None, }, ) .unwrap(); diff --git a/src/database/models.rs b/src/database/models.rs index a22e8fd..b5e1c1e 100644 --- a/src/database/models.rs +++ b/src/database/models.rs @@ -268,6 +268,14 @@ pub struct InsertEntityFact { /// `created_by_backend` is "local" / "hybrid" / "manual" / NULL. pub created_by_model: Option, pub created_by_backend: Option, + /// Audit trail for mutations after creation — see migration + /// 2026-05-10-000500. `last_modified_*` stamp on any update + /// (status flip, valid-time edit, supersede, manual PATCH); + /// `last_modified_at` is unix seconds. NULL on rows that have + /// never been touched since creation. + pub last_modified_by_model: Option, + pub last_modified_by_backend: Option, + pub last_modified_at: Option, } #[derive(Serialize, Queryable, Clone, Debug)] @@ -289,6 +297,9 @@ pub struct EntityFact { pub superseded_by: Option, pub created_by_model: Option, pub created_by_backend: Option, + pub last_modified_by_model: Option, + pub last_modified_by_backend: Option, + pub last_modified_at: Option, } #[derive(Insertable)] @@ -328,6 +339,11 @@ pub struct InsertPersona<'a> { /// 'reviewed' (human-verified). Default false. See migration /// 2026-05-10-000400. pub reviewed_only_facts: bool, + /// Gate for the agent's update_fact / supersede_fact tools. + /// Default false — fresh personas let the agent create but not + /// alter or replace. Operator opts in once a model has earned + /// trust. See migration 2026-05-10-000500. + pub allow_agent_corrections: bool, } #[derive(Serialize, Queryable, Clone, Debug)] @@ -342,6 +358,7 @@ pub struct Persona { pub created_at: i64, pub updated_at: i64, pub reviewed_only_facts: bool, + pub allow_agent_corrections: bool, } #[derive(Insertable)] diff --git a/src/database/persona_dao.rs b/src/database/persona_dao.rs index 72eb187..4924244 100644 --- a/src/database/persona_dao.rs +++ b/src/database/persona_dao.rs @@ -18,6 +18,7 @@ pub struct PersonaPatch { pub system_prompt: Option, pub include_all_memories: Option, pub reviewed_only_facts: Option, + pub allow_agent_corrections: Option, } /// One row of a bulk migration upload. Fields named to match the JSON @@ -166,6 +167,7 @@ impl PersonaDao for SqlitePersonaDao { created_at: now, updated_at: now, reviewed_only_facts: false, + allow_agent_corrections: false, }) .execute(conn.deref_mut()) .map_err(|e| anyhow::anyhow!("Insert error: {}", e))?; @@ -222,6 +224,15 @@ impl PersonaDao for SqlitePersonaDao { .execute(conn.deref_mut()) .map_err(|e| anyhow::anyhow!("Update reviewed_only_facts error: {}", e))?; } + if let Some(new_allow_corrections) = patch.allow_agent_corrections { + diesel::update(personas.filter(user_id.eq(uid)).filter(persona_id.eq(pid))) + .set(( + allow_agent_corrections.eq(new_allow_corrections), + updated_at.eq(now), + )) + .execute(conn.deref_mut()) + .map_err(|e| anyhow::anyhow!("Update allow_agent_corrections error: {}", e))?; + } personas .filter(user_id.eq(uid)) @@ -396,6 +407,7 @@ mod tests { system_prompt: Some("new prompt".into()), include_all_memories: None, reviewed_only_facts: None, + allow_agent_corrections: None, }, ) .unwrap() @@ -425,6 +437,7 @@ mod tests { system_prompt: None, include_all_memories: Some(true), reviewed_only_facts: None, + allow_agent_corrections: None, }, ) .unwrap() diff --git a/src/database/schema.rs b/src/database/schema.rs index 34ea3f5..d001d80 100644 --- a/src/database/schema.rs +++ b/src/database/schema.rs @@ -64,6 +64,9 @@ diesel::table! { superseded_by -> Nullable, created_by_model -> Nullable, created_by_backend -> Nullable, + last_modified_by_model -> Nullable, + last_modified_by_backend -> Nullable, + last_modified_at -> Nullable, } } @@ -178,6 +181,7 @@ diesel::table! { created_at -> BigInt, updated_at -> BigInt, reviewed_only_facts -> Bool, + allow_agent_corrections -> Bool, } } diff --git a/src/knowledge.rs b/src/knowledge.rs index 92aaa2c..91fbd13 100644 --- a/src/knowledge.rs +++ b/src/knowledge.rs @@ -123,6 +123,12 @@ pub struct FactDetail { /// rows. `created_by_backend` is "local" / "hybrid" / "manual". pub created_by_model: Option, pub created_by_backend: Option, + /// Audit trail — see migration 2026-05-10-000500. Set on any + /// post-creation mutation. NULL on rows that have never been + /// touched after they were first written. + pub last_modified_by_model: Option, + pub last_modified_by_backend: Option, + pub last_modified_at: Option, /// Set when another active fact has the same subject+predicate, /// a different object, AND their valid-time intervals overlap. /// Detected at read time by the get_entity handler grouping @@ -314,6 +320,10 @@ where web::resource("/facts/{id}/supersede") .route(web::post().to(supersede_fact::)), ) + .service( + web::resource("/facts/{id}/restore") + .route(web::post().to(restore_fact::)), + ) .service(web::resource("/recent").route(web::get().to(get_recent::))), ) } @@ -438,6 +448,9 @@ async fn get_entity( superseded_by: f.superseded_by, created_by_model: f.created_by_model, created_by_backend: f.created_by_backend, + last_modified_by_model: f.last_modified_by_model, + last_modified_by_backend: f.last_modified_by_backend, + last_modified_at: f.last_modified_at, in_conflict: false, }); } @@ -779,6 +792,9 @@ async fn create_fact( // from agent-generated ones in the audit view. created_by_model: None, created_by_backend: Some("manual".to_string()), + last_modified_by_model: None, + last_modified_by_backend: None, + last_modified_at: None, }; match dao.upsert_fact(&cx, insert) { @@ -815,7 +831,10 @@ async fn patch_fact( }; let mut dao = dao.lock().expect("Unable to lock KnowledgeDao"); - match dao.update_fact(&cx, fact_id, patch) { + // Manual PATCH from the curation UI — provenance stamped as + // "manual" so the audit feed can distinguish human edits from + // agent corrections. + match dao.update_fact(&cx, fact_id, patch, Some(("manual", "manual"))) { Ok(Some(fact)) => HttpResponse::Ok().json(fact), Ok(None) => HttpResponse::NotFound().json(serde_json::json!({"error": "Fact not found"})), Err(e) => { @@ -855,7 +874,9 @@ async fn supersede_fact( .json(serde_json::json!({"error": "old_id and by_fact_id must differ"})); } let mut dao = dao.lock().expect("Unable to lock KnowledgeDao"); - match dao.supersede_fact(&cx, old_id, body.by_fact_id) { + // Manual supersede from the curation UI — same stamping rule as + // the PATCH path. + match dao.supersede_fact(&cx, old_id, body.by_fact_id, Some(("manual", "manual"))) { Ok(Some(fact)) => HttpResponse::Ok().json(fact), Ok(None) => HttpResponse::NotFound() .json(serde_json::json!({"error": "Old or new fact not found"})), @@ -866,6 +887,25 @@ async fn supersede_fact( } } +async fn restore_fact( + _claims: Claims, + id: web::Path, + dao: web::Data>, +) -> impl Responder { + let cx = opentelemetry::Context::current(); + let fact_id = id.into_inner(); + let mut dao = dao.lock().expect("Unable to lock KnowledgeDao"); + match dao.revert_supersession(&cx, fact_id, Some(("manual", "manual"))) { + Ok(Some(fact)) => HttpResponse::Ok().json(fact), + Ok(None) => HttpResponse::NotFound() + .json(serde_json::json!({"error": "Fact not found"})), + Err(e) => { + log::error!("restore_fact error: {:?}", e); + HttpResponse::InternalServerError().json(serde_json::json!({"error": "Database error"})) + } + } +} + async fn get_recent( req: HttpRequest, claims: Claims, diff --git a/src/personas.rs b/src/personas.rs index c4ecf53..e5997de 100644 --- a/src/personas.rs +++ b/src/personas.rs @@ -40,6 +40,11 @@ pub struct PersonaView { /// 2026-05-10-000400. #[serde(rename = "reviewedOnlyFacts")] pub reviewed_only_facts: bool, + /// Gate for the agent's update_fact / supersede_fact tools. + /// Default false — fresh personas let the agent create but not + /// alter. See migration 2026-05-10-000500. + #[serde(rename = "allowAgentCorrections")] + pub allow_agent_corrections: bool, } impl From for PersonaView { @@ -53,6 +58,7 @@ impl From for PersonaView { created_at: p.created_at, updated_at: p.updated_at, reviewed_only_facts: p.reviewed_only_facts, + allow_agent_corrections: p.allow_agent_corrections, } } } @@ -80,6 +86,8 @@ pub struct UpdatePersonaRequest { pub include_all_memories: Option, #[serde(default, rename = "reviewedOnlyFacts")] pub reviewed_only_facts: Option, + #[serde(default, rename = "allowAgentCorrections")] + pub allow_agent_corrections: Option, } #[derive(Deserialize)] @@ -258,6 +266,7 @@ async fn update_persona( system_prompt: body.system_prompt.clone(), include_all_memories: body.include_all_memories, reviewed_only_facts: body.reviewed_only_facts, + allow_agent_corrections: body.allow_agent_corrections, }; match dao.update_persona(&cx, uid, &pid, patch) { Ok(Some(p)) => HttpResponse::Ok().json(PersonaView::from(p)),