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.
This commit is contained in:
@@ -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).
|
||||
"""
|
||||
|
||||
@@ -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))
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user