feat(memory): validity windows on memory_units (valid_to + /invalidate endpoint)

[nikolay-bratanov]

Closes #1391.

Summary

Adds a nullable valid_to TIMESTAMPTZ column to memory_units, a partial index limited to valid_to IS NULL rows, and a POST /memories/{memory_id}/invalidate endpoint so that superseded facts can be soft-retired without losing their timeline. Recall queries skip rows whose valid_to has elapsed; GET /memories/{id} and GET /memories/{id}/history still return them, preserving the audit trail.

What's included

• Migration a2b3c4d5e6f7_add_valid_to_to_memory_units.py — ALTER TABLE memory_units ADD COLUMN valid_to TIMESTAMPTZ NULL plus partial index idx_memory_units_active (bank_id, fact_type) WHERE valid_to IS NULL. Both IF NOT EXISTS so the migration is idempotent. Down-migration drops both.
• Model MemoryUnit.valid_to and the corresponding Index in __table_args__.
• Recall filter in engine/search/retrieval.py — a validity_clause = "AND (valid_to IS NULL OR valid_to > {dialect.current_timestamp()})" is composed once and injected into the four WHERE blocks via the existing extra_where parameter on build_semantic_arm / build_bm25_arm (semantic UNION arms, BM25 UNION arms, date-range entry-points CTE, graph-spreading neighbour join). The Oracle-fallback semantic-only path also gets the filter. No dialect-level changes — portability comes from dialect.current_timestamp().
• Engine MemoryEngine.invalidate_memory_unit(bank_id, memory_id, *, valid_to=None, reason=None, request_context) — single UPDATE with explicit $3::timestamptz cast (avoids AmbiguousParameterError when valid_to is also referenced inside JSONB metadata in the same statement). Default valid_to is UTC now(). Operation goes through validate_bank_write like the existing clear_observations_for_memory.
• Endpoint POST /v1/default/banks/{bank_id}/memories/{memory_id}/invalidate with InvalidateMemoryRequest / InvalidateMemoryResponse Pydantic models. Returns 404 when the memory unit isn't found, 400 on a malformed valid_to, and propagates OperationValidationError + @audited(...) bookkeeping like sibling memory endpoints.
• get_memory_unit also returns valid_to so clients can see invalidation status without hitting /history.
• Tests in tests/test_invalidate_memory.py covering the engine method (default-now, not-found, malformed-uuid) and the dialect-level guarantee that semantic and BM25 arms include the validity filter.

Diff size: +438 / −4 across 6 files (4 modified + migration + test).

Why

See #1391 for the full design rationale. Short version: when a fact in Hindsight becomes stale (a server is decommissioned, a person changes role, a default value changes), today's options are delete (audit trail gone) or leave it (recall returns it next to the corrected fact, and the LLM has to disambiguate at recall time). Other learning-oriented memory systems (e.g. MemPalace's triples/attributes tables) make valid_from / valid_to first-class. This PR adds the same to Hindsight in the smallest possible shape: one column + one index + one endpoint, no schema breaking changes.

Validation

I've been running the equivalent of this patch against a managed Hindsight install (~1.4k memory units, ~720 entities) for several days. End to end:

• Insert a memory → recall returns it.
• POST /memories/{id}/invalidate → recall stops returning it.
• GET /memories/{id}/history → still shows it.
• recall over a 91-result window after invalidation → invalidated row is filtered out; ordering of the rest is unchanged.

The local version did the work via text-replace on the installed retrieval.py and out-of-band DDL; this PR does it cleanly through a real alembic migration and the SQL builder.

Test plan

• pytest hindsight-api-slim/tests/test_invalidate_memory.py
• Existing recall integration tests should be unaffected — please confirm CI passes.
• Optional: integration test asserting that an invalidated row drops out of recall on a real Postgres (happy to add — wanted to keep this PR small).

Notes for reviewers

• Naming: valid_to matches the convention in MemPalace's schema and pairs naturally with eventual valid_from if we want to make that explicit (today it's implicit in mentioned_at / event_date). Open to superseded_at / retired_at if you have a strong preference.
• HTTP verb: POST .../invalidate — non-idempotent w.r.t. metadata (invalidation_reason overwrites). PATCH /memories/{id} with a partial body would also be reasonable; happy to switch.
• Worker-side awareness: this PR does not yet teach consolidation / reflect workers to skip valid_to IS NOT NULL rows. If you'd like that in the same PR, let me know — straightforward but increases scope.
• as_of recall: a natural follow-up — recall with as_of: <ISO> to "see what was active at that point". Out of scope here.

🤖 Generated with Claude Code

[yonefive71]

Second production datapoint in support of this PR.

Running Hindsight v0.13 inside a Hermes Agent install. Posting current SQL counts from the live bank (not estimates):

bank `hermes`
  documents:     163
  memory_units:  17,569
  chunks:        3,001

duplicate MU analysis (GROUP BY text):
  exact-duplicate clusters (cnt > 1):  455
  MU rows inside those clusters:       1,280
  top cluster size:                    22 byte-identical copies

fact_type distribution:
  experience:   7,775
  observation:  4,937
  world:        4,857

The top-10 clusters by size are all the same handful of Hindsight self-canon sentences ("Hindsight is the working-memory layer between llm-wiki and MemPalace…", "Hindsight explicitly excludes Minerva brand/product canon…", the nightly-dedupe-script summary) getting re-extracted on essentially every session that touches the topic. ~5% of the bank is byte-identical repeats, before we even talk about paraphrase clusters.

Why I think soft-invalidate is the right shape (not hard DELETE). I tried writing a doc-level paraphrase-cluster pruner last week. Dry-run found two clusters at Jaccard ≥0.65: one was 10 distinct sessions from a deck-authoring arc linked only by shared vocabulary (false positive — deleting would erase 4 days of session continuity), the other was 3 auto-curator pings (legitimate). Hard DELETE at doc level can't tell those apart without manual review of every cluster. Per-MU soft-invalidate cleanly resolves both cases: the duplicate facts die, the session transcripts that contain them stay alive, audit trail intact via /history.

This also matches how I already use MemPalace's valid_from/valid_to for the durable-canon layer — same mental model across both stores would be a real win.

Adoption. If this merges I'd run it immediately:

1. First sweep is trivial: GROUP BY text HAVING count(*) > 1 against the live bank yields 455 clusters / 1,280 rows. Keep oldest (or highest proof_count) per cluster, invalidate the rest with reason="exact-text-duplicate:<text-hash>". ~825 MUs reclaimed with zero ambiguity.
2. Wire POST /memories/{id}/invalidate into the existing nightly dedupe script as a new Tier F (MU-level paraphrase cluster collapse via cosine ≥0.92).
3. Promote any cluster that hits ≥5 paraphrase variants to a single canonical MU and invalidate the rest with reason="paraphrase-cluster-collapse:<cluster-hash>" for audit grep-ability.
4. Drop the doc-level Tier E shingled-Jaccard pass that currently can't act safely.

On the closed PRs (#794 / #796) and #1228. I read the thread. Understand the security gate on validate_bank_write — the approach in this PR (going through the engine method that does validate, via a soft-update rather than DELETE) sidesteps the auth-bypass concern entirely while still giving callers the surface they need. Treating invalidation as a write-operation on existing data is also more honest about what's happening than DELETE was.

One small ask, no objection if you'd rather defer. The submitter mentions worker-side awareness (consolidation/reflect skipping valid_to IS NOT NULL rows) as "happy to add if you want in this PR." For the paraphrase-cluster-collapse use case where I want the winning MU to be the source of truth going forward, having reflect not re-extract from invalidated rows matters. If it's not scope-creep, I'd vote to include; if it is, a follow-up issue is fine.

Happy to provide more diagnostic data (anonymized recall blocks, before/after token counts, the exact dup-cluster query plan) if useful. I'm planning to run this PR's branch locally against the production bank while we wait on merge — happy to report back if anything surfaces.

Thanks for the project, and to @nikolay-bratanov for the well-scoped PR.

[yonefive71]

Quick heads-up from running the branch locally against current main — there's a revision id collision that'll block merge:

$ ls hindsight_api/alembic/versions/ | grep a2b3c4d5e6f7
a2b3c4d5e6f7_add_text_signals_column.py     ← landed on main since this PR opened
a2b3c4d5e6f7_add_valid_to_to_memory_units.py ← this PR

$ pytest tests/test_alembic_dag.py::test_single_head
UserWarning: Revision a2b3c4d5e6f7 is present more than once
FAILED — Alembic has 3 heads (['a2b3c4d5e6f7', 'aa2b3c4d5e6f', 'o1a2b3c4d5e6'])

Two separate things going on:

1. Hard collision (your PR vs. main): the a2b3c4d5e6f7 revision id is reused. Renumbering this PR's migration to a fresh id and pointing down_revision at whichever head it should chain off of (likely the new tip on main) should clear it.
2. Three heads (normal merge-time churn): unrelated to this PR — there are also aa2b3c4d5e6f and o1a2b3c4d5e6 open. Maintainer can alembic merge heads at merge.

Everything else looked good locally:

• The 5 tests in test_invalidate_memory.py pass after fixing one tiny import typo (PostgresDialect → PostgreSQLDialect); see diff at the bottom of this comment.
• Full test_db_abstraction.py + test_invalidate_memory.py runs 85/85 green.
• Migration SQL is clean (ADD COLUMN IF NOT EXISTS, partial index on WHERE valid_to IS NULL, downgrade is symmetric).

Tiny test-file fix I had to apply locally so the suite runs:

- from hindsight_api.engine.sql.postgresql import PostgresDialect
+ from hindsight_api.engine.sql.postgresql import PostgreSQLDialect as PostgresDialect

The class is named PostgreSQLDialect in the rest of the codebase; both test_db_abstraction.py and the engine internals use that spelling.

Happy to open a small follow-up PR against your branch with the rename + revision-id bump if it'd save you a round-trip. Otherwise no rush — just wanted the collision on the record so it doesn't bite at merge.

[nicoloboschi]

please reply to these questions before working on the pr - closing for now #1391 (comment)