fix(memory): refresh content-derived fields on compress, restore, and refine

[RobertSigmundsson]

Summary

• The compression engine's compress/decompress/recover paths and instruction refinement now refresh content_hash and the embedding when they replace a neuron's content, instead of re-saving the old ones next to the new text - the same class fix(mcp): smem_edit refreshes content_hash and the embedding on a content change #166 fixed for smem_edit, on five of the six call sites it left standing (the sixth is a server route, noted below).
• Refreshing is batched: one brain lookup and one embed_batch per compression step, not one per neuron. Compression walks every neuron of a fiber under a time budget, so a per-neuron round-trip was not an acceptable way to fix this.
• The shared helper moves to utils/content_refresh.py so both layers can use it without a new cross-layer import.
• Adds twenty-four behavioural tests, including one that fails if the per-neuron round-trip ever comes back, one for a provider that returns fewer vectors than it was given, and ones proving graph-only tombstones are paired by none of the consumers of content-derived fields - not the dedup census, not the save-time dedup pipeline, not semantic discovery, not interference detection, and never as recall anchors.

Rebased onto v3.8.0, and what that changed

This branch was rebuilt on ae8e8743 (v3.8.0). One file conflicted, and the resolution is worth
describing because taking either side wholesale would have been wrong.

#188 landed after this work was written. It kept _content_refreshed local to
mcp/lifecycle_handler.py and taught it a third derived field: metadata["_structure"], which
recall reads back to answer with the memory's fields. This branch does the opposite - it removes
that local function and moves it to utils/content_refresh.py, so the nine other call sites can
share it.

Resolving in this branch's favour alone would have silently dropped #188: the handler would
delegate to a helper that knew nothing about _structure, and an edited memory would keep
describing text that no longer exists - the exact defect #188 fixed. Nothing would have failed
loudly; detect_structure would simply have stopped being called on the edit path.

So the shared helper absorbs it. contents_refreshed now re-derives _structure for every
neuron whose content changes, and removes the key when the new content has no structure at
all - preserving #188's deliberate choice of removal over overwrite, because an overwrite-on-hit
would leave recall surfacing fields that exist nowhere.

#188's own regression tests are kept and repointed at the new location (two direct imports and
two AST probes). They pass unchanged in substance, including
test_editing_structure_away_drops_it_entirely - which is the proof that the move did not cost
the fix.

Why

#166 fixed smem_edit's content-update paths but left the same pattern everywhere else that overwrites a neuron's content: engine/compression.py's compress/decompress/recover, and mcp/instruction_handler.py's refine handler. Each did dc_replace(neuron, content=new) straight into update_neuron, which writes content_hash from the object and the vector from metadata["_embedding"] - so the old fingerprint and the old vector were re-saved against the new content.

Compression makes the effect automatic rather than opt-in. A neuron reaching the GRAPH_ONLY tier has its content replaced with the placeholder "[graph-only]" while its vector and hash keep describing the full text that was there before - the memory stays retrievable by content that has since been deleted. Tier 1-3 compression has the same problem against the compressed excerpt. Decompression and snapshot recovery restore the original text without checking the derived fields match it, so drift from an intervening compress cycle survives the restore silently.

Worth flagging for anyone testing this: for the restore paths the obvious test - restore the original, assert the hash matches - passes on the unfixed code. The field was never touched since creation and you are restoring exactly the text it was derived from, so it agrees by coincidence. Two of the tests below seed a drifted hash and vector first, so a post-restore match can only come from an actual refresh.

Batching, and why the vector is refreshed rather than cleared

The obvious cheap fix - clear embedding_vec and let reindex --missing-only repair it - does not work, and fails in a way worth recording. embedding_vec carries an HNSW index with a fixed dimension (ensure_schema defines it on every connect, defaulting the dimension when unset, so any schema-initialised deployment has it), and under that index writing an empty array is rejected:

UPDATE neuron:a MERGE { content: "[graph-only]", embedding_vec: [] };
-- Incorrect vector dimension (0). Expected a vector of 4 dimension.

The error takes the whole statement with it: the row keeps its old content as well as its old vector, so the compression step silently does nothing. (embedding_vec: NONE is accepted, but update_neuron cannot express it - a None in metadata["_embedding"] means "leave alone", so clearing would need a storage-layer change.) Refreshing the vector is therefore the only in-scope fix.

That makes the cost question real, because compression is not an interactive edit: it runs from a background consolidation pass, in loops over every neuron of a fiber, and compress_fiber already treats database round-trips as its dominant cost while compress_all defers fibers when it exhausts its time budget. Embedding one neuron at a time would have added a provider round-trip per neuron to a path that makes none today.

So the helper takes a batch: hashes are recomputed locally for everything, and the neurons that actually carry a vector are embedded together in a single embed_batch behind a single brain lookup - the same shape encoder.py already uses when embedding on create. Each distinct text is embedded once and its vector fanned out, so a GRAPH_ONLY fiber costs one embedding rather than one per neuron of the identical placeholder; compress_all fetches the brain once for the whole pass rather than once per fiber. A fiber of any size costs two round-trips, not two per neuron.

The single-neuron entry point is a thin wrapper over the batch one, so the edit and refine paths keep #166's functional behaviour: the edit succeeds without a provider, the old vector is kept and a warning names smem reindex --all on provider failure, and the timeout branch stays separate. Two cosmetic deltas from #166, called out for anyone filtering logs: the warning now comes from the surreal_memory.utils.content_refresh logger rather than surreal_memory.mcp.lifecycle_handler, and its text no longer begins with "smem_edit" (the helper serves several tools now).

A tombstone must carry no fingerprint on any axis

One class of consequence only shows up once the derived fields are actually correct: every neuron compressed to GRAPH_ONLY ends up with the same content, "[graph-only]", so anything derived from that content is a single constant shared brain-wide. Each downstream consumer that compares derived fields then pairs unrelated tombstones with each other:

• Hash axis. The consolidation census compares anchors by SimHash Hamming distance; identical fingerprints read as distance 0 and persist a false duplicate of edge. So the placeholder is written with content_hash = 0 - the codebase's existing "no meaningful fingerprint" value, which the census, the dedup pipeline and the context optimizer already skip - and the census and the save-time dedup pipeline additionally refuse to fingerprint placeholder content at all. Those content guards are what cover pre-existing tombstones: rows written before this change carry the SimHash of their deleted original text, a fiber parked at GRAPH_ONLY never re-enters the compression path (the tier early-return), and reindex repairs vectors, not hashes. Without them, a genuine memory that near-duplicates the deleted text would be aliased to a tombstone by the census - or, worse, a brand-new save would be canonicalised onto one: the dedup candidate search reaches tombstones (the FTS analyzer tokenises the placeholder into graph/only), tier-1 would match the stale hash, and the new fiber would anchor to "[graph-only]" with its actual text surviving only as an alias neuron. Interference detection gets the same placeholder skip: its recompute-on-missing-hash fallback exists for real content that lost its hash, and would otherwise resurrect exactly the shared constant the sentinel suppresses - one save landing in the interference window would CONTRADICTS-link to every tag-overlapping tombstone at once.
• Vector axis. Two tombstones embed to the identical vector - cosine 1.0 between memories with nothing in common. Both consumers of stored vectors now skip placeholder-content neurons: semantic discovery (which runs inside automatic consolidation and would persist the pair as a SIMILAR_TO edge) and recall's embedding anchor selection (where a brain's worth of identical tombstone vectors would otherwise tie with each other and crowd genuine memories out of the top-k anchor slots for any query near that region). The stale fields were accidentally hiding all of these interactions, since they still described the different original texts.

The same reasoning covers two smaller hash consumers: entity reuse at encode time (its near-duplicate fallback could bind a brand-new entity reference to a placeholder neuron - a persisted wrong link) and novelty scoring (a tombstone's stale hash would make a new memory resembling the deleted text look familiar, storing it with deflated surprise). Both now skip placeholder content.

When a legacy tombstone does meet the compression path again - a partially recovered fiber, reset to FULL while its snapshot-less neurons kept the placeholder - the pass normalises its stale hash to the sentinel, without touching its vector and without a provider call. But that path is opportunistic, not a remediation: the guards above are what make legacy fingerprints inert. These seven guarded sites (census, save-time dedup, interference, discovery, recall anchors, entity reuse, novelty scoring) are, to the best of my enumeration, every consumer whose comparison against a tombstone fingerprint would persist a wrong outcome; the storage-level KNN search has no callers in the recall path. Two places still see legacy fingerprints, disclosed rather than stretched into this diff: sync's insert dedup compares exact hashes inside the storage query (a re-synced copy of text identical to a tombstone's deleted original is treated as already present - pre-existing behaviour, and arguably the intended dedup of the same aged-out memory; the sharp edge is a tombstone whose snapshot is missing, where the peer's copy is the only surviving full text and is still dropped), and the recall context optimizer's in-flight duplicate filter (no persisted state). A one-shot migration stamping the sentinel onto legacy rows (content = '[graph-only]' AND content_hash != 0) would retire both and is a natural follow-up.

One instance deliberately left out

server/routes/memory.py's neuron update route has the same bug: it builds replace(neuron, **updates) with a new content and hands it to update_neuron, so a content edit through the HTTP API re-saves the old hash and vector exactly as the paths here did. I have left it out rather than quietly widen the diff - it is a different surface with its own test story (routes, dashboard), and this PR is already the compression-and-refine change. It is the last instance I can find: I enumerated every update_neuron call in src/, and the rest are metadata-only stamps (conflict_detection, supersession, reconsolidation, conflict_handler, integration/mapper.py), vector writes already derived from current content (encoder, doc_trainer), or peer-state replication (sync_engine). Happy to follow up with the route in a separate PR if you would rather have it handled the same way.

Where the helper lives

mcp/ already imports engine/ lazily at the call site, but engine/ does not import mcp/; reusing the helper from engine/compression.py would have added a backwards engine → mcp edge. It is content-and-storage logic, not MCP-protocol logic, so it moved to utils/content_refresh.py - the home simhash already has, for the same reason. Its imports of the embedding provider stay function-local, exactly as before, so nothing changes about when that code runs.

Test plan

• pytest -m "not stress" -n auto passes locally - 7103 passed, 146 skipped, 1 xfailed (baseline on main is 7079 passed with the same skip/xfail counts; the delta is exactly this PR's twenty-four new tests).
• ruff check src/ tests/ clean; ruff format --check src/ tests/ clean.
• mypy src/ --ignore-missing-imports clean - no issues in 354 source files.
• The new tests were proven to fail against main: each compression site (GRAPH_ONLY compress, tier-1 compress, decompress, snapshot recovery) and the refine site saved a neuron carrying the old content_hash and vector next to the new content; the provider-unavailable tests saw no warning logged.
• A three-neuron fiber is compressed with the provider factory patched, asserting it is constructed once and handed all three texts in one embed_batch - so a per-neuron round-trip cannot creep back in unnoticed. Separate tests cover a fiber mixing vectored and vectorless neurons, an install with no vectors never reaching for an embedder, and the bounded-wait branch.
• A provider returning fewer vectors than texts is rejected before anything is assigned, and warns: assigning positionally over a short list would leave the tail neurons holding old vectors with no error and no warning - the same silent-stale failure this PR removes. A companion test restores a three-neuron fiber through an echo provider and asserts each neuron receives the vector derived from its own text, so a misaligned batch assignment cannot pass either.
• Two graph-only tombstones go through the consolidation dedup census with no ALIAS edge and through semantic discovery with no SIMILAR_TO edge, while genuine similar neurons in the same brain still link. A legacy tombstone carrying the SimHash of its deleted text is neither aliased to a genuine memory whose content matches that deleted text nor allowed to canonicalise a new save in the dedup pipeline, and when a partially recovered fiber re-enters compression, the stale hash is normalised to the sentinel without a provider call.
• The empty-vector rejection quoted above was reproduced against an ephemeral in-memory SurrealDB with the project's own schema and HNSW index definition.
• Diff touches engine/, mcp/, utils/ (the new shared helper) and tests/ only; no server routes or dashboard assets.

Verified by

@RobertSigmundsson

---

A note of thanks that is also context: #188 landed the day before this branch was rebuilt, and it
is a good fix - it is precisely why the conflict here had to be resolved as a merge rather than
a cherry-pick, and why the shared helper now carries the _structure handling instead of dropping
it. Your regression tests for it are kept and repointed at the new location; they pass unchanged
in substance.

Thanks as well for closing #174, #175 and #176. This PR is the remaining half of that same class -
the call sites outside the MCP layer that _content_refreshed never reached.

[acidkill]

Reviewed against main@ae8e8743 across four passes — general correctness, silent-failure hunting, Python/type review, and a dedicated test-quality audit. No correctness defects found, and nothing blocking. Four follow-up items below, each with a note on whether it is introduced here or pre-existing, because that distinction changes what is owed.

What was verified, not just read

• Batch alignment is correct. The dedup-then-fan-out (unique_index / texts / targets) pairs every neuron with the vector derived from its own text; zip(..., strict=True) and the len(vectors) != len(unique_texts) pre-assignment check together close the misalignment and short-response holes. test_batched_recover_pairs_each_neuron_with_its_own_text_vector and test_short_provider_output_warns_instead_of_silently_keeping_old_vectors are both causally dependent on that logic.
• The dc_replace-then-mutate-the-shared-dict pattern is sound, not a latent bug: Neuron is a frozen dataclass with no __post_init__, and dataclasses.replace does not copy metadata, so the later metas[i]["_embedding"] = ... genuinely reaches the returned neuron.
• #188 survived the move intact. _structure is re-derived and removed (not overwritten) when the new content has no structure, and test_editing_structure_away_drops_it_entirely still asserts exactly what it did before. The repointed AST probes now search contents_refreshed, which is where the logic actually lives — a correct re-point, not a weakening.
• No pre-existing test was weakened or deleted. Every hunk in the test files is either a pure addition or one of two mechanical repoints (the logger name in caplog.at_level, and the import paths).
• The provider path mirrors encoder.py — same embed_batch + _inline_embed_timeout + TimeoutError-before-Exception shape — so it inherits the write path's established behaviour rather than inventing a second convention.
• Restore-path tests genuinely seed drift. You flagged in the description that the naive version of these tests passes on unfixed code; the tests do the right thing and overwrite content, hash and vector with a mismatched triple before restoring, so the post-restore assertion can only come from a real refresh.

Follow-ups

1. "[graph-only]" is now a magic string across eight files — worth a named constant. (Introduced here.) On main the literal appears twice, in one file. This PR takes it to fourteen occurrences across eight. All seven new guards compare against a bare literal with nothing importing a shared symbol, so changing the placeholder text in compression.py would type-check, lint clean, and silently disable every guard — reintroducing in one commit the exact failure modes this PR documents. A GRAPH_ONLY_PLACEHOLDER constant imported at each site would make that change fail loudly instead.

2. The new brain pre-fetch in run() swallows its exception without logging. (Introduced here.) compression.py around the try: brain = await self._storage.get_brain(...) / except Exception: brain = None block is the only unlogged except in a file where all ten others log. It is correctly fail-soft — the helper re-fetches per fiber — but two consequences are invisible: the one-lookup-per-pass optimisation silently degrades to N lookups with no way to find out why the pass got slower, and if the storage problem persists, the warning the operator does see comes from content_refresh and blames the embedding provider for what is actually a brain-fetch failure. A logger.warning(..., exc_info=True) before the fallback would fix both.

3. The legacy-tombstone repair normalises the hash but leaves a stale _structure. (Pre-existing on main; this PR reduces it rather than causing it.) The refreshed.extend(dc_replace(n, content_hash=0) ...) branch bypasses contents_refreshed, so a tombstone written before this change keeps metadata["_structure"] describing its deleted original text — which the recall handler echoes as item["structure"]. To be explicit about attribution: pre-PR GRAPH_ONLY compression did a bare dc_replace(neuron, content="[graph-only]") and never touched metadata, so this staleness already exists today; after this PR newly compressed tombstones have it correctly removed, and only rows written earlier are affected. Popping _structure in that branch is a pure local computation, so it would keep the "a repeat pass stays provider-free" property you designed for. Reasonable either here or in the one-shot migration you already propose.

4. test_graph_only_tombstone_anchors_are_never_aliased passes on unfixed code. Both seeded anchors carry content_hash=0, which is caught by the content_hash is None or content_hash == 0 skip that predates this PR, so the loop never reaches the new content guard. It is not wrong — it pins a pre-existing invariant that this PR's design now leans on — but the new guard's actual regression cover comes from its sibling test_legacy_stale_hash_tombstone_is_never_aliased, which uses a non-zero legacy hash and is correctly targeted. Worth a docstring tweak so a future reader does not mistake the redundant one for the load-bearing one.

Also noted, not asked for: contents_refreshed runs roughly 95 executable lines against the project's <50 guideline, and smem_edit/smem_refine still return a clean {"status": "edited"} when re-embedding failed, with the warning reaching only the server log and never the calling agent. The second is inherited from #166, not introduced here, but it is the same shape of invisibility this PR exists to remove, so it may be worth a warning field in the tool response at some point.

On the two judgement calls you flagged

Leaving server/routes/memory.py out was right — different surface, different test story, and the enumeration backing the claim that it is the last instance checks out. Same for deferring the neuron_snapshots column normalisation in #192.

The conflict resolution against #188 is the part worth calling out: taking either side wholesale would have been wrong, and the failure mode you avoided — a helper that silently stops calling detect_structure on the edit path, with nothing failing loudly — is precisely the kind that survives review. Absorbing it into the shared helper and keeping the regression tests pointed at the new location was the right call.

Approving. None of the four items above changes behaviour today; they are worth a follow-up rather than another round here.