fix(repair): coerce empty metadata to sentinel during rebuild

mempalace/backends/chroma.py

@@ -873,19 +873,40 @@ def _write_lock(self):
     # Writes
     # ------------------------------------------------------------------
 
+    @staticmethod
+    def _sanitize_metadatas_for_chromadb(metadatas):
+        """chromadb 1.5.x rejects None and empty-dict entries in the metadatas
+        list (ValueError: Expected metadata to be a non-empty dict, got 0
+        metadata attributes in add). Coerce any such entry to a sentinel so
+        the write succeeds. Operators can later locate coerced drawers via
+        ``where={"_repaired_empty_meta": True}``.
+
+        This is the chokepoint catch-all: even if a caller's own sanitizer
+        misses a case (or skips for performance), reaching the chromadb
+        client always goes through here first.
+        """
+        if metadatas is None:
+            return None
+        return [
+            m if (isinstance(m, dict) and len(m) > 0) else {"_repaired_empty_meta": True}
+            for m in metadatas
+        ]
+
     def add(self, *, documents, ids, metadatas=None, embeddings=None):
         kwargs: dict[str, Any] = {"documents": documents, "ids": ids}
-        if metadatas is not None:
-            kwargs["metadatas"] = metadatas
+        sanitized = self._sanitize_metadatas_for_chromadb(metadatas)
+        if sanitized is not None:
+            kwargs["metadatas"] = sanitized
         if embeddings is not None:
             kwargs["embeddings"] = embeddings
         with self._write_lock():
             self._collection.add(**kwargs)
 
     def upsert(self, *, documents, ids, metadatas=None, embeddings=None):
         kwargs: dict[str, Any] = {"documents": documents, "ids": ids}
-        if metadatas is not None:
-            kwargs["metadatas"] = metadatas
+        sanitized = self._sanitize_metadatas_for_chromadb(metadatas)
+        if sanitized is not None:
+            kwargs["metadatas"] = sanitized
         if embeddings is not None:
             kwargs["embeddings"] = embeddings
         with self._write_lock():

mempalace/repair.py

@@ -139,7 +139,17 @@ def _extract_drawers(col, total: int, batch_size: int):
             break
         all_ids.extend(batch["ids"])
         all_docs.extend(batch["documents"])
-        all_metas.extend(batch["metadatas"])
+        # chromadb 1.5.x's upsert validates that every metadatas[i] is a
+        # non-empty dict (chromadb/api/types.py:validate_metadata). Drawers
+        # extracted from sqlite ground truth can come back with None or {}
+        # for sparse historical writes — coerce those to a sentinel so the
+        # rebuild upsert can complete instead of raising ValueError ~80%
+        # through a multi-hour run. See #1458 for full context.
+        sanitized_metas = [
+            m if (isinstance(m, dict) and len(m) > 0) else {"_repaired_empty_meta": True}
+            for m in batch["metadatas"]
+        ]
+        all_metas.extend(sanitized_metas)
         offset += len(batch["ids"])
     return all_ids, all_docs, all_metas
 
@@ -806,11 +816,14 @@ def _flush() -> int:
         for emb_id, doc, meta in extract_via_sqlite(source_palace, collection_name):
             ids.append(emb_id)
             docs.append(doc or "")
-            # chromadb 1.5.x rejects None entries in the metadatas list
-            # but accepts empty dicts. Mempalace drawers always carry at
-            # least wing/room, so this branch is defensive — corruption
-            # in embedding_metadata could yield an emb_id with no rows.
-            metas.append(meta if meta else {})
+            # chromadb 1.5.x rejects both None and empty-dict entries in
+            # the metadatas list (ValueError: Expected metadata to be a
+            # non-empty dict). Mempalace drawers always carry at least
+            # wing/room, so this branch is defensive — corruption in
+            # embedding_metadata could yield an emb_id with no rows.
+            # Coerce to a sentinel that satisfies validation and is
+            # discoverable later via `where={"_repaired_empty_meta": True}`.
+            metas.append(meta if (meta and len(meta) > 0) else {"_repaired_empty_meta": True})
             if len(ids) >= batch_size:
                 _flush()
         _flush()

tests/test_repair.py

@@ -76,6 +76,94 @@ def test_paginate_ids_offset_exception_fallback():
     assert "id1" in ids
 
 
+# ── _extract_drawers ──────────────────────────────────────────────────
+
+
+def test_extract_drawers_preserves_valid_metadata():
+    """Non-empty dict metadata passes through unchanged."""
+    col = MagicMock()
+    col.get.return_value = {
+        "ids": ["id1", "id2"],
+        "documents": ["doc1", "doc2"],
+        "metadatas": [{"wing": "a", "room": "1"}, {"wing": "b", "room": "2"}],
+    }
+    all_ids, all_docs, all_metas = repair._extract_drawers(col, total=2, batch_size=2)
+    assert all_ids == ["id1", "id2"]
+    assert all_docs == ["doc1", "doc2"]
+    assert all_metas == [{"wing": "a", "room": "1"}, {"wing": "b", "room": "2"}]
+
+
+def test_extract_drawers_sanitizes_none_metadata():
+    """None entries in metadatas are coerced to the sentinel dict.
+
+    chromadb 1.5.x's `validate_metadata` raises `ValueError: Expected metadata
+    to be a non-empty dict, got 0 metadata attributes in add.` if it sees a
+    None entry; the sanitizer keeps the rebuild upsert from crashing.
+    """
+    col = MagicMock()
+    col.get.return_value = {
+        "ids": ["id1", "id2", "id3"],
+        "documents": ["doc1", "doc2", "doc3"],
+        "metadatas": [{"wing": "a"}, None, {"wing": "c"}],
+    }
+    _, _, all_metas = repair._extract_drawers(col, total=3, batch_size=3)
+    assert all_metas[0] == {"wing": "a"}
+    assert all_metas[1] == {"_repaired_empty_meta": True}
+    assert all_metas[2] == {"wing": "c"}
+
+
+def test_extract_drawers_sanitizes_empty_dict_metadata():
+    """Empty dict {} entries are coerced to the sentinel dict.
+
+    chromadb 1.5.x rejects `{}` the same way it rejects `None`. The comment
+    in the previous code path mistakenly assumed otherwise.
+    """
+    col = MagicMock()
+    col.get.return_value = {
+        "ids": ["id1", "id2"],
+        "documents": ["doc1", "doc2"],
+        "metadatas": [{}, {"wing": "b"}],
+    }
+    _, _, all_metas = repair._extract_drawers(col, total=2, batch_size=2)
+    assert all_metas[0] == {"_repaired_empty_meta": True}
+    assert all_metas[1] == {"wing": "b"}
+
+
+def test_extract_drawers_sanitization_preserves_alignment():
+    """Sanitized output keeps the same length and ordering as input.
+
+    Critical invariant: ids[i] / documents[i] / metadatas[i] must stay in
+    lockstep through the sanitizer; otherwise the rebuild upsert mis-pairs
+    documents with metadata.
+    """
+    col = MagicMock()
+    col.get.return_value = {
+        "ids": ["id1", "id2", "id3", "id4"],
+        "documents": ["d1", "d2", "d3", "d4"],
+        "metadatas": [None, {"k": "v"}, {}, None],
+    }
+    all_ids, all_docs, all_metas = repair._extract_drawers(col, total=4, batch_size=4)
+    assert len(all_ids) == len(all_docs) == len(all_metas) == 4
+    assert all_ids == ["id1", "id2", "id3", "id4"]
+    assert all_metas[0] == {"_repaired_empty_meta": True}
+    assert all_metas[1] == {"k": "v"}
+    assert all_metas[2] == {"_repaired_empty_meta": True}
+    assert all_metas[3] == {"_repaired_empty_meta": True}
+
+
+def test_extract_drawers_multiple_batches():
+    """Pagination handles batch boundaries without losing/duplicating rows."""
+    col = MagicMock()
+    col.get.side_effect = [
+        {"ids": ["id1", "id2"], "documents": ["d1", "d2"], "metadatas": [{"a": 1}, None]},
+        {"ids": ["id3"], "documents": ["d3"], "metadatas": [{}]},
+        {"ids": [], "documents": [], "metadatas": []},
+    ]
+    all_ids, all_docs, all_metas = repair._extract_drawers(col, total=3, batch_size=2)
+    assert all_ids == ["id1", "id2", "id3"]
+    assert all_metas == [{"a": 1}, {"_repaired_empty_meta": True}, {"_repaired_empty_meta": True}]
+
+
 # ── scan_palace ───────────────────────────────────────────────────────