Skip to content

feat(memory): reversible curation — edit / invalidate memory units - #1976

Merged
nicoloboschi merged 1 commit into
mainfrom
feat/memory-curation
Jun 10, 2026
Merged

feat(memory): reversible curation — edit / invalidate memory units#1976
nicoloboschi merged 1 commit into
mainfrom
feat/memory-curation

Conversation

@nicoloboschi

@nicoloboschi nicoloboschi commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds reversible memory curation — edit and invalidate individual memory units — implementing the design in #1951.

Closes #1951. Supersedes the bare valid_to approach of #1878/#1395 (and answers the long-standing #1391/#1094 "let me fix/remove a bad memory" requests).

Endpoint

PATCH /v1/default/banks/{bank_id}/memories/{memory_id} — one endpoint, three reversible operations:

  • edit ({text, reason?}): correct the fact text, re-embed, drop derived observations + links, re-consolidate; previous text saved to history.
  • invalidate ({state:"invalidated", reason?}): retire the fact — excluded from recall/consolidation/graph. Reversible.
  • revert ({state:"valid"}): bring it back, restore its entity associations, re-consolidate.

Only world/experience facts can be curated — observations are derived and regenerate from their sources.

Design: invalidation moves the row to a separate table

Rather than flag rows in place, invalidation moves a fact out of memory_units into a sibling invalidated_memory_units archive. The recall hot-path stays pristine — no state predicate on any read path, and there's zero chance an invalidated row leaks into results because it simply isn't in the table the queries read. (We evaluated an in-place state column; the team chose the separate table for the structural no-leak guarantee and a clean hot-path, accepting a two-table schema and a heavier — but rare — revert.)

  • invalidate: snapshot the unit's entity ids → INSERT … SELECT into the archive → DELETE from memory_units (the cascade prunes its links + entity associations; dependent observations are re-derived). The embedding travels with the archived row.
  • revert: move the row back, restore the entity associations from the snapshot, and re-consolidate. Links are rebuilt by graph maintenance. Lossless.
  • edit: in place on the live row.

Why not just retain a contradicting fact? Because consolidation only reconciles in-stream contradictions (e.g. "likes BMW" → later "likes Toyota") — it can't help when a fact is simply wrong, or stale with nothing to contradict it (a decommissioned server, a tool that was fixed). Those are exactly what edit/invalidate are for. Systematic extraction problems should still be fixed at the mission level and reprocessed.

Schema

Migration c9a1b2d3e4f5 creates invalidated_memory_units via LIKE memory_units INCLUDING DEFAULTS (so a row round-trips verbatim) plus invalidation_reason, invalidated_at, and an entity_ids snapshot; FK to documents ON DELETE CASCADE. Oracle baseline gets the table. memory_units is unchanged — no new column, no new index, no read-path filter. delete_bank and backup/restore (BACKUP_TABLES) cover the new table.

Surfacing / control plane / clients

  • get_memory_unit and /memories/list report state/invalidation_reason/invalidated_at; list reads the archive for ?state=invalidated (default lists live facts).
  • Control plane: PATCH proxy route, lib/api.ts updateMemory() + list state filter, memory-detail-panel state badge + Invalidate/Restore button, i18n keys.
  • Regenerated Python/TS/Go SDKs + OpenAPI (PATCH only; no schema surface change since); Rust regenerates at build time.
  • New docs page developer/curation.md.

Test plan

  • test_memory_curation.py (engine): invalidate moves to archive (links + derived observations pruned, surviving sources reset, embedding archived), revert moves back + restores entity associations, edit + history, observation/invalidated-edit guards, list state filter, recall exclusion.
  • test_curation_http.py (HTTP): PATCH invalidate → GET → revert; 404 on missing; 422 on empty body.
  • Regression: observation-invalidation, graph-maintenance, recall, backup/restore guard (test_backup_tables_covers_entire_schema) all pass.
  • ruff + ty + lint clean; single alembic head; migration-shape lint passes.
  • Rebased onto current origin/main (clean; read-path files reconcile exactly, generated SDKs/OpenAPI regenerated).
  • @pytest.mark.hs_llm_core consolidation tests need a real LLM key (pass in CI).

🤖 Generated with Claude Code

@nicoloboschi nicoloboschi changed the title feat(memory): reversible curation — edit / invalidate / purge memory units feat(memory): reversible curation — edit / invalidate memory units Jun 4, 2026
@nicoloboschi
nicoloboschi force-pushed the feat/memory-curation branch from 342f0e1 to 09023fb Compare June 4, 2026 12:54
@nicoloboschi
nicoloboschi force-pushed the feat/memory-curation branch 2 times, most recently from d40c249 to b7aebca Compare June 8, 2026 14:48
@nicoloboschi
nicoloboschi marked this pull request as draft June 8, 2026 14:52
@nicoloboschi
nicoloboschi force-pushed the feat/memory-curation branch from 865d07b to 022d882 Compare June 10, 2026 14:28
@nicoloboschi
nicoloboschi marked this pull request as ready for review June 10, 2026 14:28
@nicoloboschi
nicoloboschi force-pushed the feat/memory-curation branch from 022d882 to 4568565 Compare June 10, 2026 14:46
Edit (text/context/dates/fact_type/entities), invalidate (move to a separate
invalidated_memory_units archive, reversible), and revert raw memory units via
PATCH /memories/{id}. Tracks user edits with edited_at. Control-plane UI, docs
(Memories API page), and multi-language examples included. RFC #1951.
@nicoloboschi
nicoloboschi force-pushed the feat/memory-curation branch from 4568565 to 78d4940 Compare June 10, 2026 15:09
@nicoloboschi
nicoloboschi merged commit de22b60 into main Jun 10, 2026
87 of 89 checks passed
nicoloboschi added a commit that referenced this pull request Jun 16, 2026
…#2215)

* fix(mcp): give update_memory/invalidate_memory non-empty descriptions

update_memory and invalidate_memory (added in #1976) used an f-string as
their docstring:

    f"""{_EDIT_DOC}
    Args:
        ...
    """

An f-string is an expression, not a string literal, so Python never assigns
it to the function's __doc__ (it stays None). FastMCP derives a tool's
description from __doc__, so both tools — and their bank_id variants — were
registered with an empty description.

Amazon Bedrock's Converse API rejects any toolSpec whose description is an
empty string, so every Bedrock request that advertised these tools failed
mid-stream (surfacing to clients as a generic 'internal error occurred while
processing the stream'). Providers that tolerate empty descriptions were
unaffected, which is why this only showed up on Bedrock.

Fix: pass the shared doc constant explicitly via @mcp.tool(description=...),
matching how retain/recall already register, and keep a plain-literal
docstring for the Args section. Add a regression test asserting every
registered tool exposes a non-empty description (both registration paths).

* test(mcp): statically reject @mcp.tool definitions without a description

AST-parse mcp_tools.py and fail if any @mcp.tool-decorated function
lacks both a description= kwarg and a real string-literal docstring
(an f-string docstring leaves __doc__ None). Complements the runtime
description test by also covering flag-gated tools and pointing at the
offending line; needs no engine mocking.

* chore(lint): enable ruff B021 (f-string used as docstring)

Catches the f-string-docstring footgun repo-wide at lint time — the
root cause of the empty update_memory/invalidate_memory descriptions.
Clean across hindsight-api-slim; tests/** are excluded from lint so the
static test guards that surface instead.

---------

Co-authored-by: Nicolò Boschi <boschi1997@gmail.com>
nicoloboschi added a commit that referenced this pull request Jun 16, 2026
…2232)

Add a vitest guard that walks every src .ts/.tsx file, resolves each
useTranslations("ns") binding, and asserts every static t("key") /
t.rich("key") reference maps to a leaf key in en.json. This closes the
gap between the two existing i18n checks: messages.test.ts only compares
locale catalogs against each other (a key missing from *every* catalog,
en included, passes parity), and find-untranslated.ts does the inverse
(flags strings *not* wrapped in t()). Neither walked from a t() call
site back to the catalog, so a missing key only surfaced as a runtime
next-intl error in the browser.

Runs under the existing `npm test` step in the build-control-plane CI
job, so no workflow change is needed.

The guard immediately surfaced 14 keys referenced by the curation
feature (#1976) but missing from all 10 catalogs (filterActive,
filterInvalidated, invalidatedHint, invalidatedFactsTitle, and the
memoryDetailPanel curation*/editField* set). Add translations for all
locales so the suite is green. Supersedes #2226, which patched only
filterActive.
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.

RFC: memory curation — reversible edit & invalidate with cascade

1 participant