diff --git a/CHANGELOG.md b/CHANGELOG.md index f25e224..f5562aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,14 @@ the `mimir-mem` crate, and the on-disk schema move together. copy of it. ### Fixed +- **A stale grounding is now visible on the recall line.** It rendered only + in `full_record`, i.e. `get` — so an agent that recalls, reads the ranked + lines and acts on the top hits never saw it, which is every agent. The + compact line now carries `stale-link` in the bracket, the same one-word + treatment `unsure` gets, on affected hits only. Resolved for the whole + page in one query (`grounding::stale_ids`) rather than per hit, since + recall is a hot path and the marker is usually absent; a test asserts the + batch lookup and the per-node one can never disagree. - **`mimir remember --link ` now resolves symbol names.** Both the CLI and MCP advertise "a code symbol or node", but the CLI only ever called `resolve_ref`, which resolves ids — so linking a memory to diff --git a/crates/mimir-cli/src/commands.rs b/crates/mimir-cli/src/commands.rs index d8b0fa6..f36b7b6 100644 --- a/crates/mimir-cli/src/commands.rs +++ b/crates/mimir-cli/src/commands.rs @@ -1181,6 +1181,12 @@ pub fn recall( println!("no results"); return Ok(()); } + // One lookup for the whole page; see `grounding::stale_ids`. + let stale = mimir_core::grounding::stale_ids( + &mimir.conn, + &hits.iter().map(|h| h.node.id).collect::>(), + ) + .unwrap_or_default(); for hit in &hits { if json { let mut value = node_json(&hit.node, &projects); @@ -1192,11 +1198,12 @@ pub fn recall( } else { println!( "{}", - line_q( + line_with_grounding( &hit.node, &projects, mimir.config.output.snippet_chars, - Some(&query.text) + Some(&query.text), + stale.contains(&hit.node.id) ) ); } @@ -2090,12 +2097,27 @@ fn line_q( projects: &HashMap, snippet_chars: usize, query: Option<&str>, +) -> String { + line_with_grounding(node, projects, snippet_chars, query, false) +} + +/// [`line_for_query`] plus the stale-grounding marker. Separate so the many +/// callers that render a single echoed node (remember, mark, list …) don't +/// have to run a grounding query they'd almost always get `false` from; +/// only the recall paths, which already have the whole hit set in hand, +/// pay for the one batch lookup. +fn line_with_grounding( + node: &Node, + projects: &HashMap, + snippet_chars: usize, + query: Option<&str>, + stale: bool, ) -> String { let project = node .project_id .and_then(|id| projects.get(&id)) .map(String::as_str); - mimir_core::format::agent_line_for_query(node, project, snippet_chars, query) + mimir_core::format::agent_line_for_query(node, project, snippet_chars, query, stale) } fn print_full(node: &Node, mimir: &Mimir, projects: &HashMap) -> Result<()> { diff --git a/crates/mimir-cli/src/mcp.rs b/crates/mimir-cli/src/mcp.rs index c91bc2b..4c3a594 100644 --- a/crates/mimir-cli/src/mcp.rs +++ b/crates/mimir-cli/src/mcp.rs @@ -275,6 +275,8 @@ impl MimirServer { .collect(); store::record_shown(&m.conn, query_hash.as_bytes(), &shown).map_err(engine_err)?; let projects = store::project_titles(&m.conn).map_err(engine_err)?; + let ids: Vec = hits.iter().map(|h| h.node.id).collect(); + let stale = mimir_core::grounding::stale_ids(&m.conn, &ids).unwrap_or_default(); let mut out = Vec::new(); for hit in &hits { let project = hit @@ -287,6 +289,7 @@ impl MimirServer { project, m.config.output.snippet_chars, Some(&query.text), + stale.contains(&hit.node.id), )); } Ok(out.join("\n")) diff --git a/crates/mimir-core/src/format.rs b/crates/mimir-core/src/format.rs index d008d37..d6373c9 100644 --- a/crates/mimir-core/src/format.rs +++ b/crates/mimir-core/src/format.rs @@ -196,7 +196,7 @@ fn match_window(text: &str, stems: &[String], snippet_chars: usize) -> Option, snippet_chars: usize) -> String { - agent_line_for_query(node, project, snippet_chars, None) + agent_line_for_query(node, project, snippet_chars, None, false) } /// [`agent_line`], but with the search query in hand so the snippet can be @@ -219,6 +219,7 @@ pub fn agent_line_for_query( project: Option<&str>, snippet_chars: usize, query: Option<&str>, + stale_grounding: bool, ) -> String { let id = short_uid(node.kind, &node.uid); let tag = node.subkind.as_deref().unwrap_or(node.kind.as_str()); @@ -239,7 +240,14 @@ pub fn agent_line_for_query( Some(crate::model::MemoryConfidence::Unsure) => " unsure", _ => "", }; - let mut line = format!("{id} [{tag}{scope} {date}{uses}{doubt}] {title}"); + // Same reasoning as `doubt`, and the reason this is worth a parameter + // rather than a lookup: recall is where an agent decides what to act + // on, and until now a falsified grounding was only visible to whoever + // ran `get` on that exact hit afterwards — which, in a recall loop, + // is nobody. Costs a word on the hits that have it and nothing on the + // rest. + let stale = if stale_grounding { " stale-link" } else { "" }; + let mut line = format!("{id} [{tag}{scope} {date}{uses}{doubt}{stale}] {title}"); if let Some(body) = node.body.as_deref() { let flat = collapse_ws(body); let stems = query.map(query_stems).unwrap_or_default(); @@ -491,7 +499,7 @@ mod tests { "head truncation should miss the match: {blind}" ); - let aware = agent_line_for_query(&node, None, 120, Some("prompt cache breakpoints")); + let aware = agent_line_for_query(&node, None, 120, Some("prompt cache breakpoints"), false); assert!( aware.contains("breakpoints"), "snippet should be centred on the match: {aware}" @@ -503,7 +511,7 @@ mod tests { fn query_aware_snippet_falls_back_when_nothing_matches() { let node = breadcrumb_chunk(); assert_eq!( - agent_line_for_query(&node, None, 120, Some("kubernetes ingress")), + agent_line_for_query(&node, None, 120, Some("kubernetes ingress"), false), agent_line(&node, None, 120), "a query with no lexical match must not change the line" ); @@ -514,7 +522,7 @@ mod tests { let node = breadcrumb_chunk(); // "compress" is inside the head window, which already shows it. assert_eq!( - agent_line_for_query(&node, None, 120, Some("compress")), + agent_line_for_query(&node, None, 120, Some("compress"), false), agent_line(&node, None, 120), ); } diff --git a/crates/mimir-core/src/grounding.rs b/crates/mimir-core/src/grounding.rs index a1414d6..c600697 100644 --- a/crates/mimir-core/src/grounding.rs +++ b/crates/mimir-core/src/grounding.rs @@ -133,6 +133,42 @@ pub fn tally(conn: &Connection) -> Result<(usize, usize, usize)> { Ok((grounded, stale, ungrounded)) } +/// Which of `ids` have a falsified grounding, in one query. +/// +/// The per-node [`grounding`] call is fine for `get`, which looks at one +/// record; recall renders a whole page of hits and is explicitly a hot +/// path, so asking per hit would add a query per result to buy a word that +/// is usually absent. One `IN` scan instead, over ids we already have. +/// +/// Ids are `i64` straight from the row, so interpolating them cannot +/// inject; there is no user-supplied text anywhere in this statement. +pub fn stale_ids(conn: &Connection, ids: &[i64]) -> Result> { + if ids.is_empty() { + return Ok(std::collections::HashSet::new()); + } + let kinds = GROUNDING_KINDS + .iter() + .map(|k| format!("'{}'", k.as_str())) + .collect::>() + .join(","); + let list = ids + .iter() + .map(|i| i.to_string()) + .collect::>() + .join(","); + let mut stmt = conn.prepare(&format!( + "SELECT m.id + FROM node m + JOIN edge e ON (e.src = m.id OR e.dst = m.id) + JOIN node n ON n.id = CASE WHEN e.src = m.id THEN e.dst ELSE e.src END + WHERE m.id IN ({list}) AND n.kind IN ({kinds}) + GROUP BY m.id + HAVING MAX(CASE WHEN n.deleted_at IS NULL THEN 1 ELSE 0 END) = 0" + ))?; + let rows = stmt.query_map([], |r| r.get::<_, i64>(0))?; + Ok(rows.collect::>()?) +} + /// Every live memory whose grounding has been falsified, newest first. pub fn stale_memories( conn: &Connection, @@ -179,6 +215,37 @@ mod tests { store::insert_node(conn, n).unwrap().id } + /// The batch lookup must agree with the per-node one, or recall and + /// `get` would disagree about the same memory — the exact split-brain + /// that made a `supersedes` edge read like a retirement for six weeks. + #[test] + fn stale_ids_agrees_with_per_node_grounding() { + let conn = db::open_in_memory().unwrap(); + let ok = memory(&conn, "chunker splits on symbol boundaries"); + let live = artifact(&conn, Kind::Symbol, "chunk_source"); + store::link(&conn, ok, live, Rel::About, 1.0).unwrap(); + + let broken = memory(&conn, "the old pruner ran on every write"); + let gone = artifact(&conn, Kind::Symbol, "prune_on_write"); + store::link(&conn, broken, gone, Rel::About, 1.0).unwrap(); + store::soft_delete(&conn, gone).unwrap(); + + let bare = memory(&conn, "prefer ripgrep over find on this box"); + + let ids = vec![ok, broken, bare]; + let batch = stale_ids(&conn, &ids).unwrap(); + assert_eq!(batch.len(), 1); + assert!(batch.contains(&broken)); + for id in ids { + assert_eq!( + batch.contains(&id), + grounding(&conn, id).unwrap().is_stale(), + "batch and per-node disagree on {id}" + ); + } + assert!(stale_ids(&conn, &[]).unwrap().is_empty()); + } + #[test] fn unlinked_memory_is_ungrounded() { let conn = db::open_in_memory().unwrap();