From 5eddedf691c17c5ad473ebd9c6c8a42ce0cc4462 Mon Sep 17 00:00:00 2001 From: KeilerHirsch Date: Tue, 28 Jul 2026 15:30:13 +0200 Subject: [PATCH] fix(repair): stop dropping drawers with zero embedding_metadata rows extract_via_sqlite drove its extraction with an INNER JOIN starting at embedding_metadata. Any embedding with zero rows there (a sparse historical write with no chroma:document and no other key -- the same condition _extract_drawers already sanitizes for the collection-layer rebuild path, see #1458) never appeared in the join result and was silently dropped by mempalace repair --mode from-sqlite, with no count mismatch and no warning. Drive the query from embeddings (LEFT JOIN embedding_metadata) instead, defaulting to an empty metadata dict when a row has none. Order by e.id rather than em.id since em.id is NULL for unmatched rows. Regression test seeds a real chromadb collection, then strips one drawer's metadata rows directly via SQLite (current chromadb validates against empty metadata on write, so this only reproduces on rows already sitting in an older palace) and asserts the drawer still comes back through the SQLite bypass. --- mempalace/repair.py | 19 ++++++++++++++++--- tests/test_repair.py | 45 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/mempalace/repair.py b/mempalace/repair.py index 9bb8d97296..516ca28b05 100644 --- a/mempalace/repair.py +++ b/mempalace/repair.py @@ -1260,6 +1260,13 @@ def extract_via_sqlite(palace_path: str, collection_name: str) -> Iterator[tuple returned as the document; this matches how chromadb itself stores ``add(documents=...)``. + Driven from ``embeddings`` (LEFT JOIN ``embedding_metadata``), not + the other way around: an embedding with zero ``embedding_metadata`` + rows — a sparse historical write with no ``chroma:document`` and no + other key, the same condition ``_extract_drawers`` already sanitizes + for the collection-layer path, see #1458 — must still be yielded + with an empty metadata dict, not silently excluded by the join. + Silent on missing palace, missing ``chroma.sqlite3``, or unknown collection name — yields nothing. Callers that need to distinguish "empty collection" from "collection not present" should query @@ -1289,15 +1296,21 @@ def extract_via_sqlite(palace_path: str, collection_name: str) -> Iterator[tuple """ SELECT e.embedding_id, em.key, em.string_value, em.int_value, em.float_value, em.bool_value - FROM embedding_metadata em - JOIN embeddings e ON em.id = e.id + FROM embeddings e + LEFT JOIN embedding_metadata em ON em.id = e.id WHERE e.segment_id = ? - ORDER BY em.id + ORDER BY e.id """, (segment_id,), ): if emb_id not in per_id: order.append(emb_id) + if key is None: + # LEFT JOIN unmatched row: this embedding has zero + # embedding_metadata rows. `order`/`per_id` already + # account for it via the defaultdict below; nothing to + # merge for this row. + continue if sv is not None: per_id[emb_id][key] = sv elif iv is not None: diff --git a/tests/test_repair.py b/tests/test_repair.py index 69198de603..2c08664f3e 100644 --- a/tests/test_repair.py +++ b/tests/test_repair.py @@ -1750,6 +1750,51 @@ def test_extract_via_sqlite_missing_palace_yields_nothing(tmp_path): assert list(repair.extract_via_sqlite(str(empty), "mempalace_drawers")) == [] +def test_extract_via_sqlite_yields_rows_with_zero_metadata_rows(tmp_path): + """A drawer whose embedding has ZERO rows in ``embedding_metadata`` + (no ``chroma:document``, no other key) must still be yielded, not + silently dropped. + + This is the same "sparse historical write" condition ``_extract_drawers`` + already sanitizes for the collection-layer rebuild path (see the + ``sanitized_metas`` comment above, and #1458) — current chromadb + validates against empty metadata on write, so this only happens to + rows already sitting in an older palace. Modeled here by seeding a + real chromadb collection, then stripping one drawer's metadata rows + directly via SQLite, mirroring how such a row actually looks on disk. + + An INNER JOIN driven from ``embedding_metadata`` can never see a row + with zero metadata rows: it must be driven from ``embeddings`` instead. + """ + rows = [ + ("drawer_ok", "normal document", {"wing": "w"}), + ("drawer_sparse", "will lose all its metadata rows", {"wing": "w"}), + ] + _seed_palace(tmp_path, "mempalace_drawers", rows) + + sqlite_path = os.path.join(str(tmp_path), "chroma.sqlite3") + conn = sqlite3.connect(sqlite_path) + try: + emb_row = conn.execute( + "SELECT id FROM embeddings WHERE embedding_id = ?", ("drawer_sparse",) + ).fetchone() + assert emb_row is not None, "seed helper didn't create the expected embedding row" + conn.execute("DELETE FROM embedding_metadata WHERE id = ?", (emb_row[0],)) + conn.commit() + finally: + conn.close() + + extracted = list(repair.extract_via_sqlite(str(tmp_path), "mempalace_drawers")) + ids = {emb_id for emb_id, _doc, _meta in extracted} + + assert "drawer_sparse" in ids, ( + "extract_via_sqlite silently dropped a drawer with zero " + "embedding_metadata rows — the INNER JOIN starting at " + "embedding_metadata structurally excludes it" + ) + assert "drawer_ok" in ids + + def test_rebuild_from_sqlite_roundtrips_via_real_chromadb(tmp_path): """End-to-end: seed source palace, rebuild into a fresh dest, then open dest with a fresh ChromaBackend and verify ``count()`` and