Skip to content

[BUG] smem_edit replaces content without recomputing metadata._structure, so recall returns both #176

Description

@RobertSigmundsson

What happens

smem_edit can replace a memory's content:

# src/surreal_memory/mcp/lifecycle_handler.py
async def _edit(self, args: dict[str, Any]) -> dict[str, Any]:
    """Edit an existing memory's type, content, priority, or tier."""
    ...
    new_content = args.get("content")

Since #166 a content edit also refreshes the two fields derived from the content that were being left stale — content_hash, and the embedding vector when one exists. It still does not touch metadata["_structure"], which is likewise derived from the content when the memory is first written:

# src/surreal_memory/engine/pipeline_steps.py
result = detect_structure(ctx.content)
if result.is_structured:
    ctx.metadata["_structure"] = {...}

Recall does read it back:

# src/surreal_memory/mcp/recall_handler.py
structure = anchor.metadata.get("_structure") if anchor.metadata else None

So after an edit, the content says one thing and the cached structure describes the content as it was before. grep -rn '_structure' src/surreal_memory/mcp/ shows the read but no write — repo-wide the only writer is the pipeline step above.

The mechanism is worth stating precisely: _content_refreshed copies the neuron's metadata and sets only _embedding in it, so _structure is carried across and re-saved verbatim against the new content. It is not overlooked at write time; it is actively preserved.

Why it matters

The two are read together on the way out, which is what makes this awkward: a single recall can return the new content alongside structured fields extracted from the old one. That does not look like a stale cache to whoever is reading it — it looks like the memory itself is internally contradictory, and there is no marker saying which half is current.

The narrower the edit, the worse it reads: fixing one wrong value in a structured note leaves the structure asserting the value you just corrected.

Suggestion

Re-run detect_structure when content changes, and either update metadata["_structure"] or drop it so recall falls back to the unstructured path. Dropping it is the smaller change and fails safe; recomputing preserves the feature.

I have not sent a patch because the choice between those two is a product decision, and because I could not tell from the outside whether structure detection is cheap enough to run in the edit path or whether you would rather mark the memory for re-extraction. Happy to send whichever you prefer.

Related

_structure turned out to be one of three content-derived fields the edit path left stale — the other two are content_hash and the embedding vector, and those were the expensive ones (a memory edited to say it is outdated stayed retrievable by its pre-edit meaning, and reindex --missing-only could not see it because the vector field is never empty). Those two are fixed by #166, now merged, which also lists the same pattern in instruction_handler and compression. This issue covers the remaining field, since it needs a product decision rather than a mechanical refresh.

Found on main at 4b133af1 (v3.5.1).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions