fleet-memory/hindsight-api-slim/tests/test_migration_backsweep.py
Nicolò Boschi f09ad9deac
fix(migration): backsweep orphaned observation memory units (#584)
* fix(migration): backsweep orphaned observation memory units

Delete observation rows whose every source_memory_id points to a
deleted memory unit, left behind before PR #580 fixed the chunk FK
cascade and before delete_document() called
_delete_stale_observations_for_memories.

Closes #572 (data cleanup for pre-existing installs).

* fix(migration): broaden backsweep to cover all fact types and bank-level orphans

- Pass 1: delete any memory_units row (all fact_types) whose bank_id no
  longer exists in banks — catches orphans from bank deletions that
  predate a FK cascade between the two tables.
- Pass 2: delete observation rows whose every source_memory_id points to
  a deleted memory unit, regardless of document_id/chunk_id anchors.

* test(migration): verify backsweep removes orphans and preserves legit rows

Adds a focused migration test that:
- Starts a fresh pg0 instance at revision f6g7h8i9j0k1
- Seeds orphaned rows for both backsweep passes (ghost-bank + all-dead-sources)
- Seeds legitimate rows that must survive
- Applies the backsweep migration to head
- Asserts the expected rows are deleted/preserved
2026-03-16 14:06:33 +01:00

154 lines
5.8 KiB
Python

"""Tests for migration g7h8i9j0k1l2 (backsweep orphaned memory_units).
Uses a dedicated pg0 instance (port 5562) so the test can control exactly
which migrations have run before inserting the orphan seed data.
"""
import asyncio
import uuid
from pathlib import Path
import pytest
from alembic import command
from alembic.config import Config
from sqlalchemy import create_engine, text
# ---------------------------------------------------------------------------
# Helpers
# ---------------------------------------------------------------------------
_SCRIPT_LOCATION = str(Path(__file__).parent.parent / "hindsight_api" / "alembic")
def _alembic_cfg(db_url: str) -> Config:
cfg = Config()
cfg.set_main_option("script_location", _SCRIPT_LOCATION)
cfg.set_main_option("sqlalchemy.url", db_url)
cfg.set_main_option("prepend_sys_path", ".")
cfg.set_main_option("path_separator", "os")
return cfg
def _upgrade(db_url: str, revision: str) -> None:
command.upgrade(_alembic_cfg(db_url), revision)
# ---------------------------------------------------------------------------
# Fixture: fresh database at the revision just before the backsweep
# ---------------------------------------------------------------------------
@pytest.fixture(scope="module")
def pre_backsweep_db_url():
"""
Spin up a dedicated pg0 instance and run all migrations up to (but not
including) the backsweep revision so each test can seed orphan data and
then apply the backsweep itself.
"""
from hindsight_api.pg0 import EmbeddedPostgres
pg0 = EmbeddedPostgres(name="hindsight-backsweep-test", port=5562)
loop = asyncio.new_event_loop()
try:
url = loop.run_until_complete(pg0.ensure_running())
finally:
loop.close()
# Migrate up to the revision just before the backsweep.
_upgrade(url, "f6g7h8i9j0k1")
return url
# ---------------------------------------------------------------------------
# The test
# ---------------------------------------------------------------------------
def test_backsweep_removes_orphans_and_preserves_legit_rows(pre_backsweep_db_url):
"""
Seed four kinds of rows then apply the backsweep migration and verify:
Rows that MUST be deleted
─────────────────────────
A. Any fact_type, bank_id missing from banks
→ Pass 1 deletes these regardless of fact_type or source links.
B. observation, bank exists, but ALL source_memory_ids are gone
→ Pass 2 deletes these.
Rows that MUST survive
──────────────────────
C. observation, bank exists, at least ONE source_memory_id still live
→ Pass 2 must not touch these.
D. Non-observation (world), bank exists, no sources (not relevant)
→ Pass 1 must not touch these (bank exists).
"""
db_url = pre_backsweep_db_url
engine = create_engine(db_url)
alive_bank = f"bank_{uuid.uuid4().hex[:8]}"
ghost_bank = f"bank_{uuid.uuid4().hex[:8]}" # never inserted into banks
# UUIDs for memory units
id_pass1_world = uuid.uuid4() # A: world unit, ghost bank
id_pass1_obs = uuid.uuid4() # A: observation, ghost bank
id_pass2_obs = uuid.uuid4() # B: observation, all sources gone
id_keep_obs = uuid.uuid4() # C: observation with one live source
id_keep_world = uuid.uuid4() # D: world unit, alive bank
id_live_source = uuid.uuid4() # live source for C
with engine.connect() as conn:
# --- banks ---
conn.execute(text("INSERT INTO banks (bank_id) VALUES (:b)"), {"b": alive_bank})
# --- seed memory_units ---
def insert_mu(uid, bank, fact_type, sources=None):
src_arr = "{" + ",".join(str(s) for s in (sources or [])) + "}"
conn.execute(
text(
"""
INSERT INTO memory_units
(id, bank_id, text, fact_type, source_memory_ids)
VALUES
(:id, :bank, :text, :ft, CAST(:src AS uuid[]))
"""
),
{"id": uid, "bank": bank, "text": "test", "ft": fact_type, "src": src_arr},
)
# A: ghost-bank rows (Pass 1 targets)
insert_mu(id_pass1_world, ghost_bank, "world")
insert_mu(id_pass1_obs, ghost_bank, "observation", sources=[uuid.uuid4()])
# B: observation with all-dead sources (Pass 2 target)
insert_mu(id_pass2_obs, alive_bank, "observation", sources=[uuid.uuid4(), uuid.uuid4()])
# C: observation with one live source (must survive)
insert_mu(id_live_source, alive_bank, "world")
insert_mu(id_keep_obs, alive_bank, "observation", sources=[id_live_source, uuid.uuid4()])
# D: world unit in alive bank (must survive)
insert_mu(id_keep_world, alive_bank, "world")
conn.commit()
# --- apply the backsweep ---
_upgrade(db_url, "g7h8i9j0k1l2")
# --- verify ---
with engine.connect() as conn:
def exists(uid):
return conn.execute(
text("SELECT 1 FROM memory_units WHERE id = :id"), {"id": uid}
).fetchone() is not None
# Must be gone
assert not exists(id_pass1_world), "Pass 1: world unit with ghost bank should be deleted"
assert not exists(id_pass1_obs), "Pass 1: observation with ghost bank should be deleted"
assert not exists(id_pass2_obs), "Pass 2: observation with all-dead sources should be deleted"
# Must survive
assert exists(id_keep_obs), "observation with a live source must not be deleted"
assert exists(id_keep_world), "world unit in alive bank must not be deleted"
assert exists(id_live_source), "live source memory unit must not be deleted"
engine.dispose()