diff --git a/src/echo_memory/cli/main.py b/src/echo_memory/cli/main.py index c5d4246..de9d09e 100644 --- a/src/echo_memory/cli/main.py +++ b/src/echo_memory/cli/main.py @@ -13,7 +13,7 @@ from pathlib import Path from echo_memory.audit.get_audit_log import get_fact_history -from echo_memory.cli import adopt, health, initdb, reattribute_cmd, stop_gate +from echo_memory.cli import adopt, health, initdb, reattribute_cmd, stop_gate, unmerge from echo_memory.cli import analyse as analyse_cmd from echo_memory.cli import dashboard as dashboard_cmd from echo_memory.cli import hooks as hooks_cmd @@ -102,6 +102,26 @@ def _add_project_parsers(sub) -> None: ) reattr.add_argument("--session", metavar="ID", help="session whose facts to reattribute") reattr.add_argument("--project", metavar="NAME", help="project to attribute them to") + reattr.add_argument( + "--agent", action="store_true", + help=( + "work on authorship instead of project: recover the agent_id of facts " + "whose session evidences one. Never guesses - a session with no " + "attributed fact, or two, is reported as unrecoverable" + ), + ) + + un = sub.add_parser( + "unmerge", + help="take back an alias a node absorbed from a different entity", + ) + un.add_argument( + "--list", action="store_true", dest="list_aliases", + help="show every node answering to another node's name", + ) + un.add_argument("--node", metavar="ID", help="the node holding the wrong alias") + un.add_argument("--alias", metavar="NAME", help="the alias to take back") + un.add_argument("--session-id", metavar="ID", help="session to record in the audit log") notice = sub.add_parser( "notice", help="queue a memory file for ingestion (called by the capture hook)" @@ -374,6 +394,7 @@ def _add_trial_parser(sub) -> None: _PROJECT_COMMANDS = { "dashboard": dashboard_cmd.run, "reattribute": reattribute_cmd.run, + "unmerge": unmerge.run, "notice": queue_cmd.run_notice, "pending": queue_cmd.run_pending, } diff --git a/src/echo_memory/cli/reattribute.py b/src/echo_memory/cli/reattribute.py index 7804254..e6d3ef1 100644 --- a/src/echo_memory/cli/reattribute.py +++ b/src/echo_memory/cli/reattribute.py @@ -14,9 +14,14 @@ import json from echo_memory.infra.db import GRAPH_NAME as GRAPH +from echo_memory.infra.project import UNKNOWN as UNATTRIBUTED from echo_memory.infra.project import normalize +class ReattributionError(Exception): + pass + + def sessions_by_project(conn, group_id: str) -> list[dict]: """Every session that has written to this scope, with its current project attribution and fact count, so the operator can see what needs saying.""" @@ -72,3 +77,124 @@ def render_sessions(scope: str, sessions: list[dict]) -> str: f" echo-memory --scope {scope} reattribute --session --project ", ] return "\n".join(lines) + "\n" + + +# --- agent attribution, recovered rather than asserted ------------------------- +# +# Project attribution above is operator knowledge: only a human knows which +# project a session belonged to, so a human says so. Agent attribution is not +# like that. Guessing it is precisely what migration 0011 refused to do when it +# backfilled absent agent_ids to 'unknown' instead of to 'claude-code', and that +# refusal was right: a fact claiming an author it cannot support is worse than +# one admitting it has none. +# +# There is one case where it does not have to be a guess. A session is one +# tool's conversation, so if some facts from a session carry a real agent_id and +# others carry none, the others were written by that same tool - evidence from +# inside the store, not recollection from outside it. Where a session offers no +# such evidence, or offers two different agents, this refuses rather than +# picking one. + + +def agent_evidence(conn, group_id: str) -> list[dict]: + """Per session: how many facts lack an author, and what the rest of that + session says about who wrote them.""" + rows = conn.execute( + f"""SELECT * FROM cypher('{GRAPH}', $$ + MATCH ()-[e:FACT {{group_id: $gid}}]->() + RETURN e.provenance.session_id, e.agent_id, count(e) + $$, %s) AS (session_id agtype, agent_id agtype, n agtype)""", + (json.dumps({"gid": group_id}),), + ).fetchall() + + by_session: dict[str, dict[str, int]] = {} + for session, agent, n in rows: + # An absent agent_id and the literal 'unknown' are the same condition + # wearing two hats: AGE drops a property whose value is null at CREATE, + # so a writer that passed None left no key, and migration 0011 turned + # what it could find into 'unknown'. + key = str(agent).strip('"') if agent is not None else UNATTRIBUTED + by_session.setdefault(str(session).strip('"'), {})[key] = int(str(n)) + + evidence = [] + for session, tally in by_session.items(): + missing = tally.get(UNATTRIBUTED, 0) + if not missing: + continue + attributed = {a: n for a, n in tally.items() if a != UNATTRIBUTED} + evidence.append({ + "session_id": session, + "missing": missing, + "attributed": attributed, + # One agent and only one. Two would mean the session id is shared + # by two tools, and then nothing here can say which wrote what. + "recoverable_as": next(iter(attributed)) if len(attributed) == 1 else None, + }) + return sorted(evidence, key=lambda e: (e["recoverable_as"] is None, e["session_id"])) + + +def reattribute_agent(conn, group_id: str, session_id: str) -> int: + """Give one session's unattributed facts the author the rest of that + session already evidences. Refuses when the session does not evidence one, + because the alternative is inventing provenance.""" + match = next( + (e for e in agent_evidence(conn, group_id) if e["session_id"] == session_id), None + ) + if match is None: + raise ReattributionError(f"session {session_id} has no unattributed facts") + if match["recoverable_as"] is None: + found = ", ".join(sorted(match["attributed"])) or "nothing" + raise ReattributionError( + f"session {session_id} does not evidence one author ({found}), so its " + f"{match['missing']} unattributed fact(s) stay unattributed. They predate " + "attribution and there is nothing in the store that can recover them." + ) + + agent = match["recoverable_as"] + rows = conn.execute( + f"""SELECT * FROM cypher('{GRAPH}', $$ + MATCH ()-[e:FACT {{group_id: $gid}}]->() + WHERE e.provenance.session_id = $sid + AND (e.agent_id = $unknown OR e.agent_id IS NULL) + SET e.agent_id = $agent + RETURN id(e) + $$, %s) AS (edge_id agtype)""", + (json.dumps( + {"gid": group_id, "sid": session_id, "agent": agent, "unknown": UNATTRIBUTED} + ),), + ).fetchall() + return len(rows) + + +def render_agent_evidence(scope: str, evidence: list[dict]) -> str: + if not evidence: + return f"Every fact in {scope} records who wrote it.\n" + + lines = [f"Facts in {scope} with no recorded author:", ""] + for e in evidence: + if e["recoverable_as"]: + lines.append( + f" {e['missing']:>3} fact(s) session {e['session_id']} " + f"-> {e['recoverable_as']} (the rest of the session says so)" + ) + else: + found = ", ".join(f"{a} x{n}" for a, n in sorted(e["attributed"].items())) + lines.append( + f"! {e['missing']:>3} fact(s) session {e['session_id']} " + f"-> unrecoverable ({found or 'no attributed fact in this session'})" + ) + + if any(e["recoverable_as"] for e in evidence): + lines += [ + "", + "Recover the evidenced ones with:", + f" echo-memory --scope {scope} reattribute --agent --session ", + ] + if any(not e["recoverable_as"] for e in evidence): + lines += [ + "", + "The rest stay unattributed. Nothing in the store says who wrote them, and", + "a fact claiming an author it cannot support is worse than one admitting", + "it has none.", + ] + return "\n".join(lines) + "\n" diff --git a/src/echo_memory/cli/reattribute_cmd.py b/src/echo_memory/cli/reattribute_cmd.py index 38ee4f2..486f411 100644 --- a/src/echo_memory/cli/reattribute_cmd.py +++ b/src/echo_memory/cli/reattribute_cmd.py @@ -1,11 +1,37 @@ """echo-memory reattribute: the command surface for pointing historical facts -at the project they came from. The queries live in reattribute.py.""" +at the project they came from, and for recovering the author of facts written +before agent_id was recorded. The queries live in reattribute.py.""" -from echo_memory.cli.reattribute import reattribute, render_sessions, sessions_by_project +from echo_memory.cli.reattribute import ( + ReattributionError, + agent_evidence, + reattribute, + reattribute_agent, + render_agent_evidence, + render_sessions, + sessions_by_project, +) + + +def _run_agent(args, group_id, conn) -> int: + if not args.session: + print(render_agent_evidence(args.scope, agent_evidence(conn, group_id)), end="") + # Listing is the helpful response to `--agent` alone, and it is also + # not a completed instruction. Same convention as the project half. + return 0 if args.list_sessions else 1 + try: + changed = reattribute_agent(conn, group_id, args.session) + except ReattributionError as e: + print(f"error: {e}") + return 1 + print(f"Recovered the author of {changed} fact(s) from session {args.session}.") + return 0 def run(args, config, conn) -> int: group_id = config.group_id(args.scope) + if getattr(args, "agent", False): + return _run_agent(args, group_id, conn) if args.list_sessions or not (args.session and args.project): print(render_sessions(args.scope, sessions_by_project(conn, group_id)), end="") # Without both --session and --project there is nothing to do, so diff --git a/src/echo_memory/cli/trial.py b/src/echo_memory/cli/trial.py index bf0d19f..c6b2d78 100644 --- a/src/echo_memory/cli/trial.py +++ b/src/echo_memory/cli/trial.py @@ -59,23 +59,27 @@ def render_criterion_six(report: dict, indent: str = " ", show_hint: bool = Tru f"(started {trial['started_on']}, {trial['days_left']} left)" ) - saves_note = "" + uncounted = [] if counts["same_tool_saves"]: - saves_note = ( - f" (+{counts['same_tool_saves']} same-tool, which the criterion doesn't count)" - ) + uncounted.append(f"{counts['same_tool_saves']} same-tool") + if counts.get("unattributed_saves"): + uncounted.append(f"{counts['unattributed_saves']} with no recorded author") + saves_note = ( + f" (+{', '.join(uncounted)}, which the criterion doesn't count)" if uncounted else "" + ) lines.append( f"{indent}[{'x' if met['saves'] else ' '}] {counts['cross_tool_saves']}" f"/{observations.REQUIRED_SAVES} recall saves to a different tool{saves_note}" ) - # An unmet bar has to name its cause. A reader who sees 3/3 saves and an - # unticked box would otherwise assume a display bug rather than a store - # that cannot yet evidence what the bar measures. + # Reported, not blocking. A save is excluded when its own evidence is + # missing (see observations.counts); facts elsewhere in the store that lost + # their author are a health problem worth seeing and say nothing about + # whether these particular saves happened. if report.get("unattributed_facts"): lines.append( f"{indent} ! {report['unattributed_facts']} fact(s) still carry " - f"agent_id '{UNKNOWN_PROJECT}', so a cross-tool save cannot be evidenced " - "- they predate attribution and cannot be recovered" + f"agent_id '{UNKNOWN_PROJECT}' and cannot evidence a save - recover the " + "ones whose session says who wrote them with `reattribute --agent`" ) lines.append( f"{indent}[{'x' if met['duplicates'] else ' '}] {counts['duplicates']} " diff --git a/src/echo_memory/cli/unmerge.py b/src/echo_memory/cli/unmerge.py new file mode 100644 index 0000000..3033aec --- /dev/null +++ b/src/echo_memory/cli/unmerge.py @@ -0,0 +1,156 @@ +"""echo-memory unmerge: take back an alias a node should never have absorbed. + +A confirmed `resolved_to` does two things, and the second is easy to miss. It +points this episode's facts at the named node, and it appends the mention to +that node's aliases - so the node now answers to both names. That is what makes +a misdirected resolution a merge rather than a misfiled fact: two distinct +entities become one node, which is criterion 6's third bar word for word. + +Both of this store's confirmed bad merges are still live for exactly that +reason. On 2026-09-02 the misdirected edges were found and deleted, and the +aliases were not, so 'node_embedding table' has answered to 'Eigon billing +profile' and 'AGE graphid column type' to 'Eigon GST tax rule' ever since. +_exact_match checks aliases, so every later mention of those names has been one +retired node away from resolving onto an embedding table's lesson. + +Deleting the edges was the visible half of the cleanup and doing it by hand is +why the other half was missed. This is the other half, as a command, so the +next person does not have to know that aliases exist to finish the job. +""" + +import json + +from echo_memory.infra.db import GRAPH_NAME as GRAPH + + +class UnmergeError(Exception): + pass + + +def foreign_aliases(conn, group_id: str) -> list[dict]: + """Nodes carrying an alias that is another node's name. + + The signature of an absorbed entity: the same string exists in this scope + both as a node in its own right and as somebody else's alias. That is not + proof - a genuine duplicate pair can look the same on the way to being + merged properly - which is why this lists and does not act. + """ + rows = conn.execute( + f"""SELECT * FROM cypher('{GRAPH}', $$ + MATCH (n:Node {{group_id: $gid}}) + RETURN id(n), n.name, n.aliases + $$, %s) AS (node_id agtype, name agtype, aliases agtype)""", + (json.dumps({"gid": group_id}),), + ).fetchall() + + nodes = [] + for node_id, name, aliases in rows: + parsed = json.loads(str(aliases)) if aliases is not None else [] + nodes.append((str(node_id), str(name).strip('"'), [str(a) for a in parsed or []])) + + names = {name.lower(): node_id for node_id, name, _ in nodes} + found = [] + for node_id, name, aliases in nodes: + for alias in aliases: + owner = names.get(alias.lower()) + # An alias equal to the node's own name is how a node records + # itself; only an alias owned by a DIFFERENT node is a merge. + if owner is not None and owner != node_id: + found.append({ + "node_id": node_id, "name": name, + "alias": alias, "alias_owner_id": owner, + }) + return sorted(found, key=lambda f: f["name"]) + + +def unmerge(conn, group_id: str, session_id: str, node_id: str, alias: str) -> dict: + """Remove one alias from one node, and say so in the audit log. + + Audited as `entity_resolved` because that is the mutation being undone and + the log's job is to let someone reconstruct how an entity's identity got to + where it is. A repair that leaves no trace is how the first half of this + cleanup came to look complete. + """ + try: + wanted = int(node_id) + except (TypeError, ValueError) as e: + raise UnmergeError(f"{node_id!r} is not a node id") from e + + row = conn.execute( + f"""SELECT * FROM cypher('{GRAPH}', $$ + MATCH (n:Node) WHERE id(n) = $nid AND n.group_id = $gid + RETURN n.name, n.aliases + $$, %s) AS (name agtype, aliases agtype)""", + (json.dumps({"nid": wanted, "gid": group_id}),), + ).fetchone() + if row is None: + raise UnmergeError(f"node {node_id} is not a node in this scope") + + name = str(row[0]).strip('"') + aliases = [str(a) for a in (json.loads(str(row[1])) if row[1] is not None else [])] + remaining = [a for a in aliases if a.lower() != alias.lower()] + if len(remaining) == len(aliases): + raise UnmergeError(f"node {node_id} ({name}) has no alias {alias!r}") + if alias.lower() == name.lower(): + # Removing a node's own name from its aliases would leave it unable to + # match itself, which is a different and worse kind of broken. + raise UnmergeError(f"{alias!r} is node {node_id}'s own name, not an absorbed alias") + + conn.execute( + f"""SELECT * FROM cypher('{GRAPH}', $$ + MATCH (n:Node) WHERE id(n) = $nid AND n.group_id = $gid + SET n.aliases = $aliases RETURN id(n) + $$, %s) AS (i agtype)""", + (json.dumps({"nid": wanted, "gid": group_id, "aliases": remaining}),), + ).fetchall() + + conn.execute( + """INSERT INTO public.audit_entry + (group_id, session_id, mutation_type, affected_node_id, summary, + resolution_detail) + VALUES (%s, %s, 'entity_resolved', %s::text::graphid, %s, %s)""", + ( + group_id, session_id, node_id, + f"unmerged alias {alias!r} from {name!r}", + f"alias {alias!r} removed: it names a different entity in this scope", + ), + ) + return {"node_id": node_id, "name": name, "alias": alias, "aliases_left": remaining} + + +def render_foreign_aliases(scope: str, found: list[dict]) -> str: + if not found: + return f"No node in {scope} answers to another node's name.\n" + + lines = [f"Nodes in {scope} carrying another node's name as an alias:", ""] + for f in found: + lines.append( + f" {f['name']} [{f['node_id']}] answers to {f['alias']!r} " + f"which is node {f['alias_owner_id']}" + ) + lines += [ + "", + "Each of these is two entities sharing one node. _exact_match checks", + "aliases, so a mention of the alias can resolve onto the wrong node.", + "Confirm each one is wrong before undoing it - a genuine duplicate pair", + "on its way to being merged properly looks the same from here:", + f" echo-memory --scope {scope} unmerge --node --alias ", + ] + return "\n".join(lines) + "\n" + + +def run(args, config, conn) -> int: + group_id = config.group_id(args.scope) + if not (args.node and args.alias): + print(render_foreign_aliases(args.scope, foreign_aliases(conn, group_id)), end="") + return 0 if args.list_aliases else 1 + try: + result = unmerge(conn, group_id, args.session_id or "cli", args.node, args.alias) + except UnmergeError as e: + print(f"error: {e}") + return 1 + print( + f"Removed alias {result['alias']!r} from {result['name']!r} " + f"({len(result['aliases_left'])} alias(es) left)." + ) + return 0 diff --git a/src/echo_memory/ingestion/resolution.py b/src/echo_memory/ingestion/resolution.py index b6381b0..54bcc86 100644 --- a/src/echo_memory/ingestion/resolution.py +++ b/src/echo_memory/ingestion/resolution.py @@ -155,24 +155,68 @@ def _fuzzy_candidates(conn, group_id: str, embedding: list[float], limit: int = ] -def _node_in_group(conn, group_id: str, node_id: str) -> bool: - """Whether node_id names a Node this group owns. - - Both halves matter. Existence alone would still let one tenant graft an - edge onto another's entity; group alone cannot be checked without the - lookup.""" +def _node_identity(conn, group_id: str, node_id: str) -> tuple[str, list[str]] | None: + """The name and aliases of a Node this group owns, or None. + + Both halves of the ownership check matter. Existence alone would still let + one tenant graft an edge onto another's entity; group alone cannot be + checked without the lookup. The name comes back with it because the caller + then has to decide whether this node has anything to do with the mention - + see _referent_similarity.""" try: wanted = int(node_id) except (TypeError, ValueError): - return False + return None row = conn.execute( f"""SELECT * FROM cypher('{GRAPH}', $$ MATCH (n:Node) WHERE id(n) = $nid AND n.group_id = $gid - RETURN id(n) - $$, %s) AS (node_id agtype)""", + RETURN n.name, n.aliases + $$, %s) AS (name agtype, aliases agtype)""", (json.dumps({"nid": wanted, "gid": group_id}),), ).fetchone() - return row is not None + if row is None: + return None + aliases = json.loads(str(row[1])) if row[1] is not None else [] + return str(row[0]).strip('"'), [str(a) for a in aliases or []] + + +def _node_in_group(conn, group_id: str, node_id: str) -> bool: + return _node_identity(conn, group_id, node_id) is not None + + +def _referent_similarity(conn, group_id: str, node_id: str, mention: str, embedder) -> float: + """How alike the mention and the named node are, on the same scale the + candidate list is ranked by. + + Against the node's stored embedding where there is one, so the number is + literally the one _fuzzy_candidates would have produced. A node written + before embeddings existed, or one whose embedding failed, falls back to + embedding its name - a slightly different number for the same question, + which beats refusing to check at all.""" + try: + wanted = int(node_id) + except (TypeError, ValueError): + return 0.0 + + query = embedder.embed(mention) + # node_id is AGE's graphid, which has no equality operator against bigint - + # the same type mismatch that once turned a targeted DELETE into a full + # one. Compared as text, exactly as _fuzzy_candidates selects it. + row = conn.execute( + """SELECT -(ne.embedding <#> %s::vector) + FROM public.node_embedding ne + WHERE ne.node_id::text = %s AND ne.group_id = %s""", + (query, str(wanted), group_id), + ).fetchone() + if row is not None: + return float(row[0]) + + identity = _node_identity(conn, group_id, node_id) + if identity is None: + return 0.0 + stored = embedder.embed(identity[0]) + norm = (sum(q * q for q in query) ** 0.5) * (sum(v * v for v in stored) ** 0.5) + return float(sum(q * v for q, v in zip(query, stored, strict=True)) / norm) if norm else 0.0 def resolve_entities( @@ -200,20 +244,14 @@ def resolve_entities( # on trust - no check that it existed, was a Node, or belonged # to the caller. # - # That produced the two bad merges this store has confirmed. An - # id recalled from memory rather than read from the graph - # pointed at 'node_embedding table' and 'AGE graphid column - # type', so three facts about Indian tax compliance were - # attached to an embedding table's lesson. Nothing objected; - # the facts simply landed somewhere else. - # - # In the hosted service the same hole is a cross-tenant write. - # Demonstrated before this fix: one account passed another - # account's node id and successfully attached an edge to it, - # because _create_edge matches nodes by id alone with no group - # filter. The edge carried the writer's own group_id while - # pointing at somebody else's entity. - if not _node_in_group(conn, group_id, resolved_to): + # In the hosted service that hole is a cross-tenant write. + # Demonstrated before this check existed: one account passed + # another account's node id and successfully attached an edge + # to it, because _create_edge matches nodes by id alone with no + # group filter. The edge carried the writer's own group_id + # while pointing at somebody else's entity. + identity = _node_identity(conn, group_id, resolved_to) + if identity is None: raise ResolutionError( f"entity_resolutions[{name!r}] points at node " f"{resolved_to!r}, which is not a node in this scope. " @@ -221,6 +259,46 @@ def resolve_entities( "candidates, or \"new\" - never one remembered from " "an earlier turn." ) + + # Owning the node is not the same as it being the right node, + # and the incident this store recorded was the second kind. + # Ids recalled from memory rather than read from the graph + # pointed at 'node_embedding table' and 'AGE graphid column + # type' - both nodes the caller owned, in a different project - + # so three facts about Indian tax compliance were attached to + # an embedding table's lesson. Nothing objected; the facts + # simply landed somewhere else. A scope check alone would have + # let every one of them through. + # + # The test is the one the server can actually justify: would it + # ever have offered this node as a candidate for this mention? + # Candidates are drawn above low_threshold, so a node below it + # cannot be an answer to a question this server asked, and an + # id that did not come from a candidate list came from + # somewhere that cannot be trusted with an entity's identity. + # + # Measured on the real embedder before choosing the bar: the + # incident's own pairs score 0.030 and 0.041, while the hardest + # legitimate confirmation on record ("AGE" / "Apache AGE") + # scores 0.497. An exact name or alias match skips the check + # entirely, since that is the one case where the id is + # redundant rather than doubtful. + node_name, aliases = identity + known = {node_name.lower(), *(a.lower() for a in aliases)} + if name.lower() not in known: + similarity = _referent_similarity( + conn, group_id, resolved_to, name, embedder + ) + if similarity < low_threshold: + raise ResolutionError( + f"entity_resolutions[{name!r}] points at node " + f"{resolved_to!r}, which is named {node_name!r}. " + f"That is too unlike {name!r} (similarity " + f"{similarity:.3f}, below {low_threshold}) for this " + "server to have offered it as a candidate, so the id " + "did not come from one. Pass an id from this call's " + "own ambiguous_entities candidates, or \"new\"." + ) outcome.resolved[name] = resolved_to outcome.audit_events.append( { diff --git a/src/echo_memory/trial/check.py b/src/echo_memory/trial/check.py index 3e52642..52431d6 100644 --- a/src/echo_memory/trial/check.py +++ b/src/echo_memory/trial/check.py @@ -270,15 +270,22 @@ def build_report( "n_unreviewed": sum(len(s["unreviewed_resolutions"]) for s in open_items.values()), "n_suppressed_pairs": sum(s["suppressed_pairs"] for s in open_items.values()), "met": { - # A store holding facts whose author was never recorded cannot - # evidence a CROSS-tool save: 'unknown' compares unequal to every - # real agent id, so those facts satisfy `written_by != recalled_by` - # for the wrong reason. Migration 0007 backfills them; until it has - # run, the bar is reported as unmet rather than as met-by-accident. - "saves": ( - tallies["cross_tool_saves"] >= observations.REQUIRED_SAVES - and unattributed == 0 - ), + # The guard this used to carry was right about the danger and wrong + # about the scope. A fact whose author was never recorded cannot + # evidence a CROSS-tool save, because 'unknown' compares unequal to + # every real agent id and would satisfy `written_by != recalled_by` + # for the wrong reason - so observations.counts now excludes such a + # save, by its own evidence, one save at a time. + # + # Requiring the whole store to be attributed instead refused two + # saves that name real tools at both ends over twenty-seven + # unrelated facts from sessions that hold nothing attributable. The + # check's own message called those unrecoverable, which made this a + # bar nothing could ever clear - and an unclearable bar is not a + # gate, it is a wall. The count is still reported, because a store + # losing authorship is worth seeing; it just no longer invalidates + # evidence that stands on its own. + "saves": tallies["cross_tool_saves"] >= observations.REQUIRED_SAVES, "duplicates": tallies["duplicates"] <= observations.MAX_DUPLICATES, "bad_merges": tallies["bad_merges"] <= observations.MAX_BAD_MERGES, }, diff --git a/src/echo_memory/trial/observations.py b/src/echo_memory/trial/observations.py index b725248..d64cb7d 100644 --- a/src/echo_memory/trial/observations.py +++ b/src/echo_memory/trial/observations.py @@ -8,6 +8,7 @@ from datetime import date +from echo_memory.infra.project import UNKNOWN as UNATTRIBUTED from echo_memory.ingestion.write_episode import MAX_STRING_LEN RECALL_SAVE = "recall_save" @@ -153,25 +154,43 @@ def counts(conn, group_ids: list[str]) -> dict: """Criterion 6's tallies. Recall saves are split cross-tool vs same-tool: only the cross-tool ones count toward the bar (the criterion says "to a different tool"), but a same-tool save is still real evidence recall works - and is worth seeing rather than silently dropping.""" + and is worth seeing rather than silently dropping. + + A save whose cited fact has no recorded author does not count either way. + 'unknown' compares unequal to every real agent id, so counting it as + cross-tool would satisfy `written_by <> recalled_by` for exactly the wrong + reason - the two sides differ because one is missing, not because two tools + were involved. Checked here, per save, against the save's own evidence. The + gate used to check it globally instead, refusing every save in a store that + held any unattributed fact anywhere; that blocked two saves whose both ends + name real tools over twenty-seven unrelated facts that cannot be recovered, + which is a bar nothing could ever clear.""" rows = conn.execute( """SELECT kind, count(*) FILTER ( WHERE written_by IS NOT NULL AND recalled_by IS NOT NULL AND written_by <> recalled_by + AND written_by <> %s AND recalled_by <> %s ) AS cross_tool, + count(*) FILTER ( + WHERE written_by = %s OR recalled_by = %s + ) AS unattributed, count(*) AS total FROM public.trial_observation WHERE group_id = ANY(%s) GROUP BY kind""", - (group_ids,), + (UNATTRIBUTED, UNATTRIBUTED, UNATTRIBUTED, UNATTRIBUTED, group_ids), ).fetchall() - by_kind = {kind: {"cross_tool": cross_tool, "total": total} for kind, cross_tool, total in rows} + by_kind = { + kind: {"cross_tool": cross_tool, "unattributed": unattributed, "total": total} + for kind, cross_tool, unattributed, total in rows + } - saves = by_kind.get(RECALL_SAVE, {"cross_tool": 0, "total": 0}) + saves = by_kind.get(RECALL_SAVE, {"cross_tool": 0, "unattributed": 0, "total": 0}) return { "cross_tool_saves": saves["cross_tool"], - "same_tool_saves": saves["total"] - saves["cross_tool"], + "unattributed_saves": saves["unattributed"], + "same_tool_saves": saves["total"] - saves["cross_tool"] - saves["unattributed"], "duplicates": by_kind.get(DUPLICATE_NODE, {}).get("total", 0), "bad_merges": by_kind.get(BAD_MERGE, {}).get("total", 0), "dismissed_pairs": by_kind.get(NOT_DUPLICATE, {}).get("total", 0), diff --git a/tests/integration/test_agent_reattribution.py b/tests/integration/test_agent_reattribution.py new file mode 100644 index 0000000..a21c96e --- /dev/null +++ b/tests/integration/test_agent_reattribution.py @@ -0,0 +1,144 @@ +"""Recovering the author of a fact, only where the store can evidence one. + +Thirty facts in the author's own store carry no agent_id. They were written by +a long-lived MCP server that had imported the package before agent_id shipped +and kept doing so for eleven days, and migration 0011 turned what it could find +into 'unknown' rather than into 'claude-code' - deliberately, because a fact +claiming an author it cannot support is worse than one admitting it has none. + +That refusal was right and it was also total, which left the store permanently +unable to evidence anything about authorship. There is one case where it does +not have to be a guess: a session is one tool's conversation, so a session with +some attributed facts and some unattributed ones has already said who wrote the +rest. Three of the thirty are recoverable that way and twenty-seven are not, and +the difference is what these tests pin. +""" + +from __future__ import annotations + +import json + +import pytest +from fake_embedder import REFERENCE, VectorEmbedder + +from echo_memory.cli.reattribute import ( + ReattributionError, + agent_evidence, + reattribute_agent, +) +from echo_memory.infra.db import GRAPH_NAME as GRAPH +from echo_memory.infra.db import connect +from echo_memory.ingestion.write_episode import write_episode + +GROUP = "user:ayush:shared" +OTHER_GROUP = "user:someone-else:shared" + + +def _embedder(*facts): + return VectorEmbedder({name: REFERENCE for name in (*facts, "Thing", "Other Thing", "Third Thing")}) + + +def _write(conn, group, session, fact, agent, name="Thing"): + write_episode( + conn, group, session, + [{"name": name, "type": "thing"}], + [{"source": name, "target": name, "relation_type": "is", + "fact": fact, "confidence": "extracted"}], + {name: {"resolved_to": "new"}}, + _embedder(fact), agent_id=agent, + ) + + +def _strip_author(conn, fact): + """What the stale server did: no agent_id property at all. AGE drops a + property whose value is null at CREATE, so this is absence, not null.""" + conn.execute( + f"""SELECT * FROM cypher('{GRAPH}', $$ + MATCH ()-[e:FACT]->() WHERE e.fact = $fact + REMOVE e.agent_id RETURN id(e) + $$, %s) AS (i agtype)""", + (json.dumps({"fact": fact}),), + ).fetchall() + + +def _author_of(conn, fact): + row = conn.execute( + f"""SELECT * FROM cypher('{GRAPH}', $$ + MATCH ()-[e:FACT]->() WHERE e.fact = $fact RETURN e.agent_id + $$, %s) AS (a agtype)""", + (json.dumps({"fact": fact}),), + ).fetchone() + return str(row[0]).strip('"') if row and row[0] is not None else None + + +def test_a_session_that_evidences_its_author_is_recoverable(migrated_db): + """The real case: 19 attributed facts and 3 unattributed ones in one + session. The session has already said who wrote them.""" + with connect(migrated_db) as conn: + _write(conn, GROUP, "s-mixed", "the attributed one", "claude-code") + _write(conn, GROUP, "s-mixed", "the stale one", "claude-code", name="Other Thing") + _strip_author(conn, "the stale one") + + evidence = agent_evidence(conn, GROUP) + assert len(evidence) == 1 + assert evidence[0]["missing"] == 1 + assert evidence[0]["recoverable_as"] == "claude-code" + + assert reattribute_agent(conn, GROUP, "s-mixed") == 1 + assert _author_of(conn, "the stale one") == "claude-code" + + +def test_a_session_with_no_attributed_fact_stays_unattributed(migrated_db): + """Twenty-seven of the thirty are this. Nothing in the store says who wrote + them, so nothing here may say either.""" + with connect(migrated_db) as conn: + _write(conn, GROUP, "s-dark", "nobody knows who wrote this", "claude-code") + _strip_author(conn, "nobody knows who wrote this") + + evidence = agent_evidence(conn, GROUP) + assert evidence[0]["recoverable_as"] is None + + with pytest.raises(ReattributionError) as e: + reattribute_agent(conn, GROUP, "s-dark") + + assert "nothing in the store that can recover them" in str(e.value) + assert _author_of(conn, "nobody knows who wrote this") is None + + +def test_a_session_claimed_by_two_tools_is_refused(migrated_db): + """One session, two agent ids, means the session id is shared and nothing + can say which tool wrote the unattributed fact. Picking the more frequent + one would be a guess wearing a majority vote.""" + with connect(migrated_db) as conn: + _write(conn, GROUP, "s-shared", "written by one", "claude-code") + _write(conn, GROUP, "s-shared", "written by another", "codex", name="Other Thing") + _write(conn, GROUP, "s-shared", "written by nobody", "cursor", name="Third Thing") + _strip_author(conn, "written by nobody") + + assert agent_evidence(conn, GROUP)[0]["recoverable_as"] is None + with pytest.raises(ReattributionError) as e: + reattribute_agent(conn, GROUP, "s-shared") + + assert "claude-code" in str(e.value) and "codex" in str(e.value) + + +def test_recovery_does_not_reach_across_scopes(migrated_db): + """Same session id in two groups is not the same session. Reading one + group's authorship to fix another's is the cross-tenant read this project + has already closed once.""" + with connect(migrated_db) as conn: + _write(conn, OTHER_GROUP, "s-same-id", "someone else's attributed fact", "codex") + _write(conn, GROUP, "s-same-id", "our unattributed fact", "claude-code") + _strip_author(conn, "our unattributed fact") + + assert agent_evidence(conn, GROUP)[0]["recoverable_as"] is None + with pytest.raises(ReattributionError): + reattribute_agent(conn, GROUP, "s-same-id") + + assert _author_of(conn, "our unattributed fact") is None + + +def test_a_fully_attributed_store_has_nothing_to_recover(migrated_db): + with connect(migrated_db) as conn: + _write(conn, GROUP, "s-clean", "everything is fine", "claude-code") + assert agent_evidence(conn, GROUP) == [] diff --git a/tests/integration/test_resolved_to_is_checked.py b/tests/integration/test_resolved_to_is_checked.py index 52380da..f9218cd 100644 --- a/tests/integration/test_resolved_to_is_checked.py +++ b/tests/integration/test_resolved_to_is_checked.py @@ -23,7 +23,7 @@ from __future__ import annotations -from fake_embedder import REFERENCE, VectorEmbedder +from fake_embedder import REFERENCE, VectorEmbedder, unit_vector_at_angle from echo_memory.infra.db import GRAPH_NAME as GRAPH from echo_memory.infra.db import connect @@ -138,3 +138,123 @@ def test_a_rejected_resolution_writes_nothing_at_all(migrated_db): $$) AS (i agtype)""" ).fetchall() assert rows == [], "the rejected episode left a node behind" + + +# --- the id was owned, and still wrong --------------------------------------- +# +# Everything above checks that the caller owns the node. The incident this +# store actually recorded passed that check: both guessed ids belonged to the +# author, in a different project. Owning a node is not the same as it being +# the right node, and a scope check alone would have let every one of those +# facts through. + +GUESSED = "node_embedding table" +MENTION = "Eigon billing profile" +LESSON = "AGE stores a graphid, not a bigint" +TAX = "Eigon invoices carry a GSTIN and a place of supply" + +# 0.04 is what the incident's own pair measures on the real embedder +# ('Eigon billing profile' vs 'node_embedding table'); 0.50 is the hardest +# legitimate confirmation on record ('AGE' vs 'Apache AGE', 0.497). The bar +# sits between them. +UNRELATED = unit_vector_at_angle(0.04) +DISTANT_BUT_REAL = unit_vector_at_angle(0.50) + + +def _incident_embedder(): + return VectorEmbedder({ + GUESSED: REFERENCE, LESSON: REFERENCE, + MENTION: UNRELATED, TAX: UNRELATED, + }) + + +def _seed_guessed_node(conn): + write_episode( + conn, ALICE, "s-lesson", + [{"name": GUESSED, "type": "table"}], + [{"source": GUESSED, "target": GUESSED, "relation_type": "is", + "fact": LESSON, "confidence": "extracted"}], + {GUESSED: {"resolved_to": "new"}}, + _incident_embedder(), agent_id="claude-code", + ) + row = conn.execute( + f"""SELECT * FROM cypher('{GRAPH}', $$ + MATCH (n:Node) WHERE n.name = '{GUESSED}' RETURN id(n) + $$) AS (i agtype)""" + ).fetchone() + return str(row[0]) + + +def _write_tax_fact_at(conn, node_id, embedder=None): + return write_episode( + conn, ALICE, "s-tax", + [{"name": MENTION, "type": "profile"}], + [{"source": MENTION, "target": MENTION, "relation_type": "is", + "fact": TAX, "confidence": "extracted"}], + {MENTION: {"resolved_to": node_id}}, + embedder or _incident_embedder(), agent_id="claude-code", + ) + + +def test_an_owned_but_unrelated_node_is_refused(migrated_db): + """The incident, reproduced. Before this check the tax fact attached + itself to the embedding table's lesson and nothing objected.""" + with connect(migrated_db) as conn: + guessed = _seed_guessed_node(conn) + result = _write_tax_fact_at(conn, guessed) + + assert "error" in result, result + assert not result.get("edges_created") + + landed = conn.execute( + f"""SELECT * FROM cypher('{GRAPH}', $$ + MATCH (a)-[e:FACT]->(b) WHERE id(b) = {int(guessed)} + RETURN e.fact + $$) AS (f agtype)""" + ).fetchall() + + facts = {str(f[0]).strip('"') for f in landed} + assert facts == {LESSON}, f"a tax fact reached the embedding table: {facts}" + + +def test_the_refusal_names_the_node_it_actually_points_at(migrated_db): + """A caller who guessed an id cannot tell what they hit without being told. + Naming it turns an opaque rejection into a one-step correction.""" + with connect(migrated_db) as conn: + guessed = _seed_guessed_node(conn) + error = _write_tax_fact_at(conn, guessed)["error"] + + assert GUESSED in error + assert "candidates" in error + + +def test_a_distant_but_offered_candidate_still_resolves(migrated_db): + """The bar is 'would this server ever have offered it', not 'are these + obviously the same thing'. Real duplicates score low - 'AGE' against + 'Apache AGE' is 0.497 - and refusing those would break the round-trip the + check exists to protect.""" + embedder = VectorEmbedder({ + GUESSED: REFERENCE, LESSON: REFERENCE, + MENTION: DISTANT_BUT_REAL, TAX: DISTANT_BUT_REAL, + }) + with connect(migrated_db) as conn: + guessed = _seed_guessed_node(conn) + result = _write_tax_fact_at(conn, guessed, embedder=embedder) + + assert result.get("edges_created"), result + + +def test_an_exact_name_match_skips_the_similarity_check(migrated_db): + """Passing the id of a node whose name IS the mention is redundant, not + doubtful, and must not depend on an embedding lookup to be allowed.""" + with connect(migrated_db) as conn: + guessed = _seed_guessed_node(conn) + result = write_episode( + conn, ALICE, "s-same-name", + [{"name": GUESSED, "type": "table"}], + [{"source": GUESSED, "target": GUESSED, "relation_type": "also_is", + "fact": LESSON, "confidence": "extracted"}], + {GUESSED: {"resolved_to": guessed}}, + _incident_embedder(), agent_id="claude-code", + ) + assert result.get("edges_created"), result diff --git a/tests/integration/test_trial.py b/tests/integration/test_trial.py index 1a6d07a..ccd1794 100644 --- a/tests/integration/test_trial.py +++ b/tests/integration/test_trial.py @@ -3,6 +3,7 @@ scans that decide what a human still has to look at, and criterion 6's tallies. See conftest.py for the migrated_db fixture and DB-reachability skip.""" +import json from datetime import date import pytest @@ -11,6 +12,7 @@ from echo_memory import server from echo_memory.cli.main import main from echo_memory.infra.config import Config +from echo_memory.infra.db import GRAPH_NAME as GRAPH from echo_memory.infra.db import connect from echo_memory.trial import check, observations @@ -91,6 +93,52 @@ def test_only_cross_tool_saves_count_toward_the_bar(migrated_db): assert counts["same_tool_saves"] == 1 +def test_a_save_whose_author_was_never_recorded_counts_for_neither(migrated_db): + """'unknown' is unequal to every real agent id, so counting it as + cross-tool would satisfy `written_by != recalled_by` because one side is + missing rather than because two tools were involved.""" + config = _seed(migrated_db) + conn = connect(migrated_db) + group_id = config.group_id("shared") + + observations.record(conn, group_id, observations.RECALL_SAVE, "a fact with no author", + written_by="unknown", recalled_by="cursor") + + counts = observations.counts(conn, [group_id]) + assert counts["cross_tool_saves"] == 0 + assert counts["same_tool_saves"] == 0 + assert counts["unattributed_saves"] == 1 + + +def test_unattributed_facts_elsewhere_do_not_invalidate_a_real_save(migrated_db): + """The bar used to require the whole store to be attributed, which refused + saves naming real tools at both ends over unrelated facts the check itself + called unrecoverable - a bar nothing could clear. The evidence for a save + is that save's own two agent ids.""" + config = _seed(migrated_db) + conn = connect(migrated_db) + group_id = config.group_id("shared") + observations.start_trial(conn, date(2026, 8, 21)) + + # A fact somewhere else in the store that lost its author, exactly as the + # stale MCP server left thirty of them. + conn.execute( + f"""SELECT * FROM cypher('{GRAPH}', $$ + MATCH ()-[e:FACT {{group_id: $gid}}]->() + SET e.agent_id = 'unknown' RETURN id(e) LIMIT 1 + $$, %s) AS (i agtype)""", + (json.dumps({"gid": group_id}),), + ).fetchall() + + for i in range(3): + observations.record(conn, group_id, observations.RECALL_SAVE, f"real save {i}", + written_by="claude-code", recalled_by="codex") + + report = check.build_report(conn, config, today=date(2026, 8, 23)) + assert report["unattributed_facts"] >= 1, "the test did not create the condition" + assert report["met"]["saves"], "three evidenced saves were refused by unrelated facts" + + def test_an_observation_needs_a_note(migrated_db): conn = connect(migrated_db) with pytest.raises(observations.TrialError): diff --git a/tests/integration/test_unmerge.py b/tests/integration/test_unmerge.py new file mode 100644 index 0000000..900e5ad --- /dev/null +++ b/tests/integration/test_unmerge.py @@ -0,0 +1,169 @@ +"""Taking back an alias one entity absorbed from another. + +A confirmed `resolved_to` points the episode's facts at the named node AND +appends the mention to that node's aliases. The second half is what makes a +misdirected resolution a merge rather than a misfiled fact: the node now +answers to both names, which is criterion 6's "two distinct entities +incorrectly merged into one node" word for word. + +Both of this store's confirmed bad merges were still live when this was +written, eight days after being found. The 2026-09-02 cleanup deleted the +misdirected edges, which was the visible half, and left the aliases - so +'node_embedding table' still answered to 'Eigon billing profile'. _exact_match +checks aliases, so the damage was not historical: the next mention of that name +could land on an embedding table's lesson. +""" + +from __future__ import annotations + +import json + +import pytest +from fake_embedder import REFERENCE, VectorEmbedder + +from echo_memory.cli.unmerge import UnmergeError, foreign_aliases, unmerge +from echo_memory.infra.db import GRAPH_NAME as GRAPH +from echo_memory.infra.db import connect +from echo_memory.ingestion.resolution import _exact_match +from echo_memory.ingestion.write_episode import write_episode + +GROUP = "user:ayush:shared" +OTHER_GROUP = "user:someone-else:shared" + +TABLE = "node_embedding table" +BILLING = "Eigon billing profile" + + +def _embedder(): + return VectorEmbedder({ + TABLE: REFERENCE, BILLING: REFERENCE, + "a lesson about the embedding table": REFERENCE, + "a fact about Indian tax compliance": REFERENCE, + }) + + +def _node(conn, group, name, fact): + write_episode( + conn, group, "s1", + [{"name": name, "type": "thing"}], + [{"source": name, "target": name, "relation_type": "is", + "fact": fact, "confidence": "extracted"}], + {name: {"resolved_to": "new"}}, + _embedder(), agent_id="claude-code", + ) + row = conn.execute( + f"""SELECT * FROM cypher('{GRAPH}', $$ + MATCH (n:Node {{group_id: $gid}}) WHERE n.name = $name RETURN id(n) + $$, %s) AS (i agtype)""", + (json.dumps({"gid": group, "name": name}),), + ).fetchone() + return str(row[0]) + + +def _absorb(conn, node_id, alias): + """What a confirmed resolved_to does to the node it points at.""" + conn.execute( + f"""SELECT * FROM cypher('{GRAPH}', $$ + MATCH (n:Node) WHERE id(n) = $nid + SET n.aliases = $aliases RETURN id(n) + $$, %s) AS (i agtype)""", + (json.dumps({"nid": int(node_id), "aliases": [alias]}),), + ).fetchall() + + +def _merged_store(conn): + table = _node(conn, GROUP, TABLE, "a lesson about the embedding table") + billing = _node(conn, GROUP, BILLING, "a fact about Indian tax compliance") + _absorb(conn, table, BILLING) + return table, billing + + +def test_a_node_answering_to_another_nodes_name_is_found(migrated_db): + with connect(migrated_db) as conn: + table, billing = _merged_store(conn) + found = foreign_aliases(conn, GROUP) + + assert len(found) == 1 + assert found[0]["node_id"] == table + assert found[0]["alias"] == BILLING + assert found[0]["alias_owner_id"] == billing + + +def test_a_nodes_own_name_among_its_aliases_is_not_a_merge(migrated_db): + """Every node records itself that way. Reporting it would bury the two real + ones under twenty that are fine.""" + with connect(migrated_db) as conn: + table = _node(conn, GROUP, TABLE, "a lesson about the embedding table") + _absorb(conn, table, TABLE) + assert foreign_aliases(conn, GROUP) == [] + + +def test_unmerging_stops_the_alias_resolving_onto_the_wrong_node(migrated_db): + """The assertion that matters. Deleting the misdirected edges left this + behaviour untouched for eight days: _exact_match checks aliases, so the + next mention of the absorbed name could still land on the wrong entity.""" + with connect(migrated_db) as conn: + table, billing = _merged_store(conn) + + # Retire the rightful owner, so the alias is the only match left and + # the landmine is visible rather than masked by the node's own name. + conn.execute( + f"""SELECT * FROM cypher('{GRAPH}', $$ + MATCH (n:Node) WHERE id(n) = $nid SET n.name = 'renamed' RETURN id(n) + $$, %s) AS (i agtype)""", + (json.dumps({"nid": int(billing)}),), + ).fetchall() + + before = _exact_match(conn, GROUP, BILLING) + assert before is not None and before[0] == table, "the merge was not reproduced" + + unmerge(conn, GROUP, "s-repair", table, BILLING) + + assert _exact_match(conn, GROUP, BILLING) is None + + +def test_the_repair_is_in_the_audit_log(migrated_db): + """A repair that leaves no trace is how the first half of this cleanup came + to look complete.""" + with connect(migrated_db) as conn: + table, _ = _merged_store(conn) + unmerge(conn, GROUP, "s-repair", table, BILLING) + + rows = conn.execute( + """SELECT mutation_type::text, summary, resolution_detail + FROM public.audit_entry WHERE session_id = 's-repair'""", + ).fetchall() + + assert len(rows) == 1 + assert rows[0][0] == "entity_resolved" + assert BILLING in rows[0][1] and TABLE in rows[0][1] + + +def test_another_scopes_node_cannot_be_edited(migrated_db): + with connect(migrated_db) as conn: + theirs = _node(conn, OTHER_GROUP, TABLE, "a lesson about the embedding table") + _absorb(conn, theirs, BILLING) + + with pytest.raises(UnmergeError) as e: + unmerge(conn, GROUP, "s-repair", theirs, BILLING) + + assert "not a node in this scope" in str(e.value) + + +def test_an_alias_that_is_not_there_is_refused(migrated_db): + with connect(migrated_db) as conn: + table, _ = _merged_store(conn) + with pytest.raises(UnmergeError): + unmerge(conn, GROUP, "s-repair", table, "something else entirely") + + +def test_a_node_cannot_be_stripped_of_its_own_name(migrated_db): + """It would leave the node unable to match itself, which is a different and + worse kind of broken than answering to one name too many.""" + with connect(migrated_db) as conn: + table = _node(conn, GROUP, TABLE, "a lesson about the embedding table") + _absorb(conn, table, TABLE) + with pytest.raises(UnmergeError) as e: + unmerge(conn, GROUP, "s-repair", table, TABLE) + + assert "own name" in str(e.value)