Apps/ImageApi
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

knowledge: agent self-correction with audit + per-persona gate + revert

Browse Source 
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) <noreply@anthropic.com>
This commit is contained in:
Cameron Cordes
2026-05-10 20:56:56 -04:00
parent 86c331571d
commit fd4dd89bbb
10 changed files with 515 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

139
src/database/knowledge_dao.rs
View File

@@ -210,11 +210,17 @@ pub trait KnowledgeDao: Sync + Send {
filter: FactFilter,
) -> Result<(Vec<EntityFact>, 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<Option<EntityFact>, 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<Option<EntityFact>, 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<Option<EntityFact>, DbError>;
// --- Photo links ---
@@ -1069,46 +1092,72 @@ impl KnowledgeDao for SqliteKnowledgeDao {
cx: &opentelemetry::Context,
fact_id: i32,
patch: FactPatch,
audit: Option<(&str, &str)>,
) -> Result<Option<EntityFact>, 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<Option<EntityFact>, 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::<Option<EntityFact>, 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<Option<EntityFact>, 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<EntityFact> = entity_facts
.filter(id.eq(fact_id))
.first::<EntityFact>(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::<Option<i32>>(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::<Option<i64>>(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::<EntityFact>(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();