From 0a1adf9962b48d494af7f77cb2059cba767c7b4d Mon Sep 17 00:00:00 2001
From: jp <jp@jphein.com>
Date: Mon, 11 May 2026 14:17:07 -0700
Subject: [PATCH] test(repair): unit coverage for empty-metadata sanitization
 in _extract_drawers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Addresses @Copilot's review feedback on #1459. Five tests:

  - test_extract_drawers_preserves_valid_metadata: non-empty dict
    passes through unchanged (regression guard against breaking happy
    path).
  - test_extract_drawers_sanitizes_none_metadata: None entries
    coerce to {"_repaired_empty_meta": True} (the core fix).
  - test_extract_drawers_sanitizes_empty_dict_metadata: empty dict
    {} entries also coerce to the sentinel (chromadb 1.5.x rejects
    both shapes equally).
  - test_extract_drawers_sanitization_preserves_alignment: critical
    invariant — ids[i] / documents[i] / metadatas[i] stay in
    lockstep through the sanitizer; mis-pairing would silently
    corrupt rebuilds.
  - test_extract_drawers_multiple_batches: pagination boundary
    correctness (sanitizer applied per-batch, no drops/duplicates).

Verified passing locally against mempalace fork main + chromadb
1.5.8 in the palace-daemon venv (5 passed, 67 deselected in 1.88s).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 tests/test_repair.py | 88 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/tests/test_repair.py b/tests/test_repair.py
index 9507c5d..144d907 100644
--- a/tests/test_repair.py
+++ b/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 ───────────────────────────────────────────────────────