fix: document pre-filter tradeoffs, improve test assertions and coverage

Addresses Copilot review feedback on MemPalace#660.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: jphein

mempalace/layers.py

@@ -82,7 +82,13 @@ class Layer1:
 
     MAX_DRAWERS = 15  # at most 15 moments in wake-up
     MAX_CHARS = 3200  # hard cap on total L1 text (~800 tokens)
-    MAX_SCAN = 2000  # don't scan more than this for L1 generation
+    # MAX_SCAN caps how many drawers we read for L1 generation.  This prevents
+    # O(n) full-table scans on large palaces (100K+ drawers) which are too slow
+    # for wake-up.  The tradeoff: the top-15 selection is approximate for very
+    # large datasets — drawers beyond MAX_SCAN are never considered.  This is
+    # acceptable because L1 is a best-effort summary, and L3 deep search covers
+    # the full corpus when precision matters.
+    MAX_SCAN = 2000
 
     def __init__(self, palace_path: str = None, wing: str = None):
         cfg = MempalaceConfig()
@@ -97,7 +103,12 @@ def _fetch_drawers(self, col) -> tuple:
         """
         _BATCH = 500
 
-        # Fast path: only fetch drawers with importance >= 3
+        # Fast path: only fetch drawers with importance >= 3.
+        # This is an optimization that catches the common case — importance is
+        # the primary signal in generate()'s scoring.  generate() also considers
+        # emotional_weight and weight, but those are rarely set without a
+        # corresponding importance value.  The fallback full-scan below ensures
+        # nothing is missed when the fast path returns too few results.
         importance_filter = {"importance": {"$gte": 3}}
         if self.wing:
             where = {"$and": [{"wing": self.wing}, importance_filter]}

tests/test_layers.py

@@ -73,11 +73,12 @@ def test_layer0_default_path():
 def _mock_chromadb_for_layer(docs, metas, monkeypatch=None):
     """Return a mock collection whose get() returns docs/metas.
 
-    Layer1._fetch_drawers() has two phases: a fast-path (importance pre-filter)
-    and a fallback full-scan.  For small test datasets (< 500 items), each phase
-    makes exactly one col.get() call before breaking (len < _BATCH).  We
-    provide two identical responses: one consumed by the fast path and one by
-    the fallback.
+    Layer1._fetch_drawers() has two phases: a fast-path (importance >= 3
+    pre-filter) and a fallback full-scan.  For small test datasets (< 500
+    items), each phase makes exactly one col.get() call before breaking
+    (len < _BATCH).  We provide two identical responses: the first is consumed
+    by the fast path, and the second is only consumed when the fast path
+    doesn't return enough results (< MAX_DRAWERS) and the fallback executes.
     """
     mock_col = MagicMock()
     mock_col.get.side_effect = [
@@ -153,6 +154,7 @@ def test_layer1_with_wing_filter():
     where = call_kwargs.get("where", {})
     assert "$and" in where
     assert {"wing": "project_x"} in where["$and"]
+    assert {"importance": {"$gte": 3}} in where["$and"]
 
 
 def test_layer1_truncates_long_snippets():
@@ -210,6 +212,61 @@ def test_layer1_importance_from_various_keys():
     assert "ESSENTIAL STORY" in result
 
 
+def test_layer1_pagination_stops_at_max_scan():
+    """_fetch_drawers() paginates in _BATCH (500) chunks and stops at MAX_SCAN."""
+    _BATCH = 500
+    # Build a full batch of 500 docs (first page) and a partial second page
+    batch_docs = [f"doc{i}" for i in range(_BATCH)]
+    batch_metas = [{"room": "r", "importance": 5} for _ in range(_BATCH)]
+    partial_docs = [f"doc{i}" for i in range(100)]
+    partial_metas = [{"room": "r", "importance": 5} for _ in range(100)]
+
+    mock_col = MagicMock()
+    # Fast path: page 1 (full batch) -> page 2 (partial, triggers break)
+    mock_col.get.side_effect = [
+        {"documents": batch_docs, "metadatas": batch_metas},
+        {"documents": partial_docs, "metadatas": partial_metas},
+    ]
+
+    with (
+        patch("mempalace.layers.MempalaceConfig") as mock_cfg,
+        patch("mempalace.layers._get_collection", return_value=mock_col),
+    ):
+        mock_cfg.return_value.palace_path = "/fake"
+        layer = Layer1(palace_path="/fake")
+        result = layer.generate()
+
+    # Fast path got 600 results (>= MAX_DRAWERS=15), so no fallback needed.
+    # Two get() calls: first batch of 500, second batch of 100 (< _BATCH -> break).
+    assert mock_col.get.call_count == 2
+    assert "ESSENTIAL STORY" in result
+
+
+def test_layer1_pagination_caps_at_max_scan():
+    """_fetch_drawers() stops reading once MAX_SCAN is reached, even mid-pagination."""
+    _BATCH = 500
+    batch_docs = [f"doc{i}" for i in range(_BATCH)]
+    batch_metas = [{"room": "r", "importance": 5} for _ in range(_BATCH)]
+
+    mock_col = MagicMock()
+    # Return full batches every time — the loop should stop after hitting MAX_SCAN
+    mock_col.get.return_value = {"documents": batch_docs, "metadatas": batch_metas}
+
+    with (
+        patch("mempalace.layers.MempalaceConfig") as mock_cfg,
+        patch("mempalace.layers._get_collection", return_value=mock_col),
+    ):
+        mock_cfg.return_value.palace_path = "/fake"
+        layer = Layer1(palace_path="/fake")
+        layer.MAX_SCAN = 1200  # Should stop after 3 pages (500+500+500 >= 1200)
+        result = layer.generate()
+
+    # Fast path: 3 pages to reach >= MAX_SCAN, then stops.
+    # No fallback since 1500 >= MAX_DRAWERS.
+    assert mock_col.get.call_count == 3
+    assert "ESSENTIAL STORY" in result
+
+
 def test_layer1_batch_exception_breaks():
     """If the fast-path raises, fallback scan still returns results gracefully."""
     mock_col = MagicMock()