Skip to content

fix(mcp): smem_edit refreshes content_hash and the embedding on a content change - #166

Merged
acidkill merged 1 commit into
acidkill:mainfrom
RobertSigmundsson:fix/edit-refreshes-hash-and-embedding
Aug 15, 2026
Merged

fix(mcp): smem_edit refreshes content_hash and the embedding on a content change#166
acidkill merged 1 commit into
acidkill:mainfrom
RobertSigmundsson:fix/edit-refreshes-hash-and-embedding

Conversation

@RobertSigmundsson

Copy link
Copy Markdown
Contributor

Summary

  • A content edit through smem_edit now recomputes content_hash and, when the neuron already carries an embedding, re-embeds the new text through the same provider path the write path uses.
  • If the provider is unavailable, the edit still succeeds but logs a warning naming smem reindex --all — the stale vector becomes loud and repairable instead of silent and invisible.
  • Adds three regression tests: hash refresh, vector refresh, and the provider-unavailable fallback.

Why

Both edit paths did dc_replace(neuron, content=new_content) and handed the result to update_neuron. That writes content_hash from the object and the vector from metadata["_embedding"] — and since _row_to_neuron surfaces the stored vector into that key on read, the old embedding was not merely left behind. It was actively re-saved against the new text, on every content edit.

Three things drift at once, and each is quiet in its own way:

  • Semantic recall returns the memory for what it used to say. The worst case we measured is also the most natural use of an edit: an entry corrected specifically to say it is outdated was still retrieved as if current, because its vector still described the pre-edit text.
  • reindex --missing-only (the default) cannot see the damage. The vector field is never empty, so the only repair is a blind --all re-embed of the whole brain.
  • Near-duplicate detection compares fingerprints of text that no longer exists, since content_hash is a stored field, not a computed one.

Measured on a production brain of ~15k neurons (full re-embed and cosine compare of every neuron): 7 edited memories had drifted, worst cosine 0.75 against a flat 1.0000 for every untouched neuron — and all 7 carried an explicit correction marker in their text, i.e. they were exactly the memories someone had cared enough to fix.

Design choices, called out so they are decisions rather than surprises:

  • The embedding is refreshed only when one already exists. Installs without an embedding provider have no vector to go stale, and the edit path gains no new dependency on one.
  • On provider failure the edit keeps the old vector and warns, mirroring the write path's fail-soft philosophy (a slow or absent provider must not turn a successful edit into an error). If you would rather clear the vector so --missing-only picks it up, that is a one-line change in the helper — I chose the warning because clearing silently degrades recall for that memory, and the log line names the repair either way.
  • The same stale-derived-data pattern exists in mcp/instruction_handler.py and in engine/compression.py's compress/restore paths; this PR deliberately fixes only the user-facing edit tool. Happy to follow up on the others if you want them handled the same way — and a reindex --stale mode (re-embed-and-compare, which is how we found this) might be worth having regardless.
  • metadata["_structure"] has the same problem on content edits. I have left it out of this PR and will raise it separately, because it needs a product decision (recompute vs drop) rather than a mechanical refresh.

Test plan

  • pytest tests/ -m "not stress" -n auto passes locally — 7020 passed, 142 skipped, 1 xfailed (baseline on main is 7017; the delta is this PR's three tests).
  • ruff check src/ tests/ clean; ruff format --check clean.
  • mypy src/ --ignore-missing-imports clean — no issues in 353 source files.
  • All three new tests were proven to fail on main's lifecycle_handler.py: the saved neuron kept the old content_hash, kept the old vector, and no warning was emitted — the exact silent behaviour this PR removes.
  • Diff touches mcp/ and tests/ only; no server routes or dashboard assets.

Verified by

@RobertSigmundsson

…tent change

A content edit replaced the text and nothing else. Both edit paths did
dc_replace(neuron, content=new_content) and handed the result straight to
update_neuron — which writes content_hash from the object and the vector from
metadata["_embedding"]. Since the storage read surfaces the STORED vector into
that key, the old embedding was not merely left behind: it was actively
re-saved against the new text, every time.

The effect compounds quietly. The memory stays retrievable by what it USED to
say — the worst case being an entry edited specifically to say it is outdated,
still returned by semantic recall as if current, because its vector describes
the pre-edit text. And because the vector field is never empty, `reindex
--missing-only` (the default) cannot see the damage; only a blind `--all`
re-embed of the whole brain repairs it. content_hash drifted the same way, so
near-duplicate detection compared fingerprints of text that no longer exists.

A content edit now refreshes both derived fields. content_hash is recomputed
unconditionally (a pure function of the content). The embedding is recomputed
only when the neuron already carries one, through the same provider path and
bounded wait the write path uses; if the provider is unavailable the edit
still succeeds, but the stale vector is reported with a warning naming
`reindex --all` — loud and repairable instead of silent and invisible.

@acidkill acidkill left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified locally on top of this branch:

  • The bug is real and I traced the full chain: _row_to_neuron surfaces the stored embedding_vec into metadata["_embedding"] on read, the old edit path passed it through dc_replace untouched, and update_neuron pops that key straight back into embedding_vec — so every content edit actively re-saved the OLD vector against the NEW text, and reindex --missing-only could never see it. content_hash likewise persisted the object's stale SimHash.
  • The fix matches the write path: _create_provider(..., task_type="RETRIEVAL_DOCUMENT") + embed_batch([embedding_text()]) under _inline_embed_timeout() is the same shape encoder uses.
  • Fail-soft on provider unavailability is the right call for an edit tool, and the warning names the repair command.
  • All three new tests fail on main's handler and pass with the fix (verified by reverting just the src file); full module 15/15; ruff/format clean; mypy only the pre-registered `google.genai] env error; Integration rerun green (earlier fail was the pre-#172 signin flake).

Follow-ups worth their own issues when you get to them (as the PR body already flags): the same stale-derived-data pattern in mcp/instruction_handler.py and engine/compression.py, metadata["_structure"] on content edits, and a reindex --stale mode.

@acidkill
acidkill merged commit 1495888 into acidkill:main Aug 15, 2026
17 of 18 checks passed
acidkill added a commit that referenced this pull request Aug 27, 2026
…188)

* fix(mcp): refresh derived structure on edit, and expose min_salience

Closes #176. Closes #175.

smem_edit already refreshed content_hash and the embedding when content
changed (#166), but left metadata["_structure"] describing the previous
text. Recall reads that field back to answer with a memory's fields, so
an edited structured memory kept reporting fields the new content no
longer had.

Recomputed alongside the other derived fields — and REMOVED, not merely
overwritten, when the new content has no structure. Overwrite-on-hit
would preserve the old fields in exactly the worst case: recall would
surface fields that exist nowhere.

TransplantFilter.min_salience was validated, documented and applied, but
nothing in mcp/ ever set it, so smem_transplant always ran unfiltered
with no way to ask otherwise. Now advertised in the tool schema with its
0.0-1.0 range stated up front, and passed through by the handler. The
engine validates the range itself; the handler turns that into an
answer rather than letting it escape as a tool crash.

Both defects are the same shape — a capability that exists everywhere
except where a caller could reach it — so the tests assert reachability,
not just correctness: the schema advertises it, the handler forwards it,
and the edit path actually recomputes rather than merely being expected
to.

* docs: regenerate the MCP tool reference for min_salience

The reference is generated from the tool schemas, so exposing a new
argument leaves it stale.
acidkill pushed a commit that referenced this pull request Aug 30, 2026
… refine (#193)

Content changes outside smem_edit re-saved the old content_hash and the old
embedding vector next to the new text. #166 fixed the edit tool; the compression
engine's compress/decompress/recover paths and the instruction refine handler
kept the pattern, so a memory stayed retrievable by content that had since been
compressed away.

Refreshing is batched. Compression walks every neuron of a fiber from a
background pass under a time budget, in a path that already counts database
round-trips as its dominant cost, so the helper takes a list: hashes are
recomputed locally and the neurons carrying a vector are embedded together in
one embed_batch behind one brain lookup, mirroring how encoder.py embeds on
create. A fiber costs two round-trips regardless of its size. A provider
returning fewer vectors than texts is rejected before any of it is assigned -
assigning positionally over a short list would leave the tail neurons holding
their old vectors with no error and no warning, the same silent staleness this
change removes.

A tombstone must carry no fingerprint on any axis. Every GRAPH_ONLY neuron
shares the placeholder as content, so anything derived from it is one constant
brain-wide, and every consumer that compares derived fields pairs unrelated
tombstones: the dedup census by Hamming distance, interference detection via
its recompute-on-missing-hash fallback, semantic discovery by cosine on the
stored vector, and recall's anchor selection, where identical tombstone vectors
crowd genuine memories out of the top-k slots. The placeholder therefore
carries content_hash 0 (the established no-fingerprint value the census, the
dedup pipeline and the context optimizer already skip), and every consumer
whose comparison against a tombstone fingerprint would persist a wrong outcome
skips placeholder content: interference, discovery, recall anchor selection,
the census, the save-time dedup pipeline's candidate filter (a legacy
tombstone's stale hash could canonicalise a brand-new save onto a placeholder
anchor), entity reuse at encode time, and novelty scoring. The census content guard is what covers tombstones written before the
sentinel existed: a fiber parked at GRAPH_ONLY never re-enters compression, so
their stale hashes cannot be re-stamped - they are made inert instead. When a
partially recovered fiber does re-enter, the stale hash is normalised to the
sentinel without a provider call.

The batch helper accepts a pre-fetched brain so a whole compress_all pass pays
one brain lookup instead of one per fiber, and embeds each distinct text once,
so a GRAPH_ONLY fiber costs one embedding rather than N copies of the
placeholder.

Clearing the vector instead was not available: embedding_vec carries an HNSW
index with a fixed dimension, so an empty array is rejected outright and takes
the whole update with it, leaving content unchanged too.

The helper moves to utils/content_refresh.py; engine/ does not import mcp/, and
reusing it in place would have added a backwards cross-layer edge.

Co-authored-by: Robert Sigmundsson <230784065+RobertSigmundsson@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants