Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <symbol>` 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
Expand Down
28 changes: 25 additions & 3 deletions crates/mimir-cli/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Vec<_>>(),
)
.unwrap_or_default();
for hit in &hits {
if json {
let mut value = node_json(&hit.node, &projects);
Expand All @@ -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)
)
);
}
Expand Down Expand Up @@ -2090,12 +2097,27 @@ fn line_q(
projects: &HashMap<i64, String>,
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<i64, String>,
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<i64, String>) -> Result<()> {
Expand Down
3 changes: 3 additions & 0 deletions crates/mimir-cli/src/mcp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<i64> = 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
Expand All @@ -287,6 +289,7 @@ impl MimirServer {
project,
m.config.output.snippet_chars,
Some(&query.text),
stale.contains(&hit.node.id),
));
}
Ok(out.join("\n"))
Expand Down
18 changes: 13 additions & 5 deletions crates/mimir-core/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ fn match_window(text: &str, stems: &[String], snippet_chars: usize) -> Option<St
/// One-line agent format for a node.
/// `project` is the display name of the node's project, if scoped.
pub fn agent_line(node: &Node, project: Option<&str>, 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
Expand All @@ -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());
Expand All @@ -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();
Expand Down Expand Up @@ -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}"
Expand All @@ -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"
);
Expand All @@ -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),
);
}
Expand Down
67 changes: 67 additions & 0 deletions crates/mimir-core/src/grounding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::collections::HashSet<i64>> {
if ids.is_empty() {
return Ok(std::collections::HashSet::new());
}
let kinds = GROUNDING_KINDS
.iter()
.map(|k| format!("'{}'", k.as_str()))
.collect::<Vec<_>>()
.join(",");
let list = ids
.iter()
.map(|i| i.to_string())
.collect::<Vec<_>>()
.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::<rusqlite::Result<_>>()?)
}

/// Every live memory whose grounding has been falsified, newest first.
pub fn stale_memories(
conn: &Connection,
Expand Down Expand Up @@ -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();
Expand Down