From 27c3347faff0757619d2b145cae610226445b4ca Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Thu, 24 Sep 2026 16:40:16 +0530 Subject: [PATCH] refactor(memory): resolve a journey fingerprint by text alone The occurrence hint in _resolve_fingerprint (_local_index_hint, "the occurrence the user clicked") came from the contributor's dedupe=False mutate, which this salvage deliberately dropped: MemoryStore._mutate collapses byte-identical entries and _apply goes back to the first match with entries.index(text), so the hint could never pick an occurrence. Its only live effect was a spurious "stale" refusal for a duplicated card after an earlier entry was removed, although the text was still there. Match by fingerprint -> first entry with that text, delete _local_index_hint (and with it the second copy of the profile-index formula), and read _memory_cards() only on the legacy index-id path, so a fingerprinted id no longer re-reads and hashes both files for a hint. Legacy ids still resolve by position. The render lookup goes back to a single memory_node_id key: every render_frames caller builds the graph fresh, and build_learning_graph always sets the fingerprint. The desktop star-map keeps its dual key (imported graphs). The memory_fingerprint docstring said a memory add prepends; it appends; what shifts a card is an earlier entry removed. --- agent/learning_graph.py | 7 +++--- agent/learning_graph_render.py | 11 ++------- agent/learning_mutations.py | 42 ++++++++++++---------------------- 3 files changed, 21 insertions(+), 39 deletions(-) diff --git a/agent/learning_graph.py b/agent/learning_graph.py index 15eabc8a4a..a333e54610 100644 --- a/agent/learning_graph.py +++ b/agent/learning_graph.py @@ -130,9 +130,10 @@ def density_stats(nodes: dict[str, SkillNode], edges: list[tuple[str, str]]) -> def memory_fingerprint(entry: str) -> str: """Short stable digest of a memory entry's TEXT, carried in the node id. - A journey card is identified by what it says, not by where it sat: the list can be - prepended to (an agent ``memory_tool`` add mid-turn) between the graph being drawn and the - user submitting an edit, and a bare index then names somebody else's card (#119668). + A journey card is identified by what it says, not by where it sat: an earlier entry can be + removed (an agent ``memory_tool`` remove mid-turn, a Journey delete without a refetch) + between the graph being drawn and the user submitting an edit, and a bare index then names + somebody else's card (#119668). Cards and the mutation path both read entries through ``MemoryStore._read_file``, so the same entry digests the same on both sides (a BOM'd file included). """ diff --git a/agent/learning_graph_render.py b/agent/learning_graph_render.py index 4ee7120016..8b8f1d69a5 100644 --- a/agent/learning_graph_render.py +++ b/agent/learning_graph_render.py @@ -261,15 +261,8 @@ def _build_chart_buckets(nodes: list[dict[str, Any]], rec: dict[str, Any], max_r def _bucket_rows(buckets: list[_ChartBucket], payload: dict[str, Any]) -> list[dict[str, Any]]: cmap = category_color_map(payload) - # Keyed by BOTH id shapes: a node id carries the card's fingerprint (agent.learning_graph), - # while a payload from an older graph — or one rendered from a cached response — has none. - memory_lookup: dict[str, dict[str, Any]] = {} - for idx, card in enumerate(payload.get("memory", []) or []): - if not isinstance(card, dict): - continue - memory_lookup[f"memory:{card.get('source')}:{idx}"] = card - if card.get("fingerprint"): - memory_lookup[memory_node_id(card, idx)] = card + memory_lookup = {memory_node_id(card, idx): card + for idx, card in enumerate(payload.get("memory", []) or []) if isinstance(card, dict)} def node_row(node: dict[str, Any]) -> dict[str, Any]: card, memory = _node_card(node), memory_lookup.get(_node_id(node)) diff --git a/agent/learning_mutations.py b/agent/learning_mutations.py index ea82b48334..efc1ae5ddc 100644 --- a/agent/learning_mutations.py +++ b/agent/learning_mutations.py @@ -37,38 +37,26 @@ def _parse_memory_id(node_id: str) -> tuple[str, int, str]: raise ValueError(f"bad memory node id: {node_id!r}") from exc -def _local_index_hint(gidx: int, cards: list, source: str) -> int: - """Where the id says the entry sits in ITS file, or -1 when the index names no such card.""" - if not 0 <= gidx < len(cards) or cards[gidx].get("source") != source: - return -1 - return gidx if source == "memory" else gidx - sum(1 for c in cards if c.get("source") == "memory") +def _resolve_fingerprint(chunks: list[str], fingerprint: str) -> int | None: + """Index of the first entry in *chunks* whose text carries *fingerprint*, or None when gone. - -def _resolve_fingerprint(chunks: list[str], fingerprint: str, hint: int) -> int | None: - """Index of the clicked card in *chunks*, or None when its text is gone or ambiguous. - - The hinted position wins while it still carries that text — identical entries are separate - cards, and that is the occurrence the user clicked. Otherwise the text names the entry, which - is what survives a list that shifted under the user (another writer prepending an entry - between the graph being drawn and the edit being submitted). Several copies and a moved - position cannot be told apart, so the caller refuses instead of editing an arbitrary one. + The text names the entry, so a list that shifted under the user (an earlier entry removed + between the graph being drawn and the edit being submitted) still resolves to the card they + clicked. Identical entries are one entry to the memory store (it collapses byte-identical + copies on every mutation), so the first match is the entry. """ from agent.learning_graph import memory_fingerprint - matches = [i for i, chunk in enumerate(chunks) if memory_fingerprint(chunk) == fingerprint] - if hint in matches: - return hint - return matches[0] if len(matches) == 1 else None + return next((i for i, chunk in enumerate(chunks) if memory_fingerprint(chunk) == fingerprint), None) def _locate_memory(node_id: str) -> tuple[Path, list[str], int]: """Resolve a memory node id to (file, all §-delimited entries, local index). - Entries come from ``MemoryStore._read_file`` — the memory tool's own parser — - so journey indices stay aligned with what the graph renders; a profile card's - local index is its global index minus the MEMORY.md card count. Read-only view: - mutations resolve the id again INSIDE ``_mutate_memory``'s lock.""" + Entries come from ``MemoryStore._read_file`` — the memory tool's own parser. A + fingerprinted id resolves by the entry's text; a legacy id by position (a profile + card's local index is its global index minus the MEMORY.md card count). Read-only + view: mutations resolve the id again INSIDE ``_mutate_memory``'s lock.""" from hermes_constants import get_hermes_home - from agent.learning_graph import _memory_cards from tools.memory_tool import MemoryStore source, gidx, fingerprint = _parse_memory_id(node_id) @@ -76,14 +64,14 @@ def _locate_memory(node_id: str) -> tuple[Path, list[str], int]: if not path.exists(): raise ValueError(f"{path.name} not found") chunks = MemoryStore._read_file(path) - cards = _memory_cards() if fingerprint: - # The id names the card's TEXT, so a list that shifted since the graph was drawn still - # resolves to the entry the user clicked instead of whatever now sits at that index. - local = _resolve_fingerprint(chunks, fingerprint, _local_index_hint(gidx, cards, source)) + local = _resolve_fingerprint(chunks, fingerprint) if local is None: raise ValueError("memory node id is stale — refresh the graph") return path, chunks, local + from agent.learning_graph import _memory_cards + + cards = _memory_cards() if not 0 <= gidx < len(cards): raise IndexError(f"memory index {gidx} out of range") if cards[gidx].get("source") != source: