fix(searcher): address Dialectician review on MemPalace#1005

- _count_in_scope: use col.count() for unfiltered scopes (O(1) vs 28
  paginated round trips on 137K drawers when wing/room filter is absent)
- allow_fallback gate: fire BM25 fallback when max_distance is permissive
  (<=0.0) OR when a vector error occurred (warnings non-empty). Fixes the
  MCP path: tool_search passes max_distance=1.5, which the old gate read
  as "strict" and silently skipped fallback — so agents on a drifted palace
  saw 0 hits instead of BM25 coverage + a warning
- Add explanatory comment on vector_hit_count pre-mutation capture
- Update test mocks with explicit count.return_value (required now that
  _count_in_scope calls col.count() for unfiltered scopes)

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

Author: jphein

mempalace/searcher.py

@@ -306,27 +306,24 @@ def search(query: str, palace_path: str, wing: str = None, room: str = None, n_r
 
 
 def _count_in_scope(drawers_col, where: dict) -> int | None:
-    """Return the total number of drawers matching ``where`` via paginated
-    ``col.get(include=[], ...)`` calls.
+    """Return the total number of drawers matching ``where``.
 
-    ChromaDB's ``Collection.count()`` does not accept a ``where`` filter, so
-    for a sqlite-authoritative per-scope count we paginate ``get()`` with
-    ``include=[]`` (ids are always returned by Chroma, even when include is
-    empty) and sum the batch sizes. Pagination keeps each query well under
-    the #950 "too many SQL variables" limit.
+    When ``where`` is empty (unfiltered scope), uses ``Collection.count()``
+    which is O(1). Otherwise paginates ``get(include=[])`` — ChromaDB's
+    ``count()`` does not accept a ``where`` filter. Pagination keeps each
+    query well under the #950 "too many SQL variables" limit.
 
     Returns ``None`` if the count could not be computed (e.g., filter
     planner error).
     """
-    PAGE = 5000
-    offset = 0
-    total = 0
     try:
+        if not where:
+            return drawers_col.count()
+        PAGE = 5000
+        offset = 0
+        total = 0
         while True:
-            kwargs: dict = {"limit": PAGE, "offset": offset, "include": []}
-            if where:
-                kwargs["where"] = where
-            batch = drawers_col.get(**kwargs)
+            batch = drawers_col.get(limit=PAGE, offset=offset, include=[], where=where)
             batch_ids = batch.get("ids") or []
             if not batch_ids:
                 break
@@ -652,18 +649,25 @@ def search_memories(
     vector_hit_count = len(hits)
     vector_underdelivered = vector_hit_count < n_results
 
-    # Sqlite-authoritative scope count + top-up fallback. When max_distance
-    # is set, the caller wants strict similarity — don't pad with BM25-only
-    # hits that have no vector distance. Scope count still runs so callers
-    # can see what they're missing.
+    # Capture vector hit count before BM25 may extend hits. The scope warning
+    # must fire whenever vector underdelivered — even when BM25 fills the
+    # request to n_results — because vector is still the degraded layer.
+    # BM25 fallback is a reliability mechanism: it fires when the distance
+    # threshold is permissive (max_distance=0.0 means "no filtering") OR
+    # when a vector error occurred (warnings non-empty at this point). This
+    # ensures MCP callers on a drifted palace get fallback coverage even
+    # though tool_search passes max_distance=1.5, without firing fallback
+    # when a strict distance filter legitimately eliminates all results on
+    # a working HNSW index.
+    allow_fallback = (max_distance <= 0.0) or bool(warnings)
     available_in_scope, fallback_warnings = _sqlite_fallback_and_scope(
         drawers_col,
         query,
         where,
         hits,
         n_results=n_results,
         vector_underdelivered=vector_underdelivered,
-        allow_fallback=(max_distance <= 0.0),
+        allow_fallback=allow_fallback,
     )
     warnings.extend(fallback_warnings)
 

tests/test_searcher.py

@@ -62,6 +62,7 @@ def test_created_at_contains_filed_at(self, palace_path, seeded_collection):
     def test_created_at_fallback_when_filed_at_missing(self):
         """created_at defaults to 'unknown' when filed_at is absent."""
         mock_col = MagicMock()
+        mock_col.count.return_value = 1
         mock_col.query.return_value = {
             "ids": [["drawer_no_date"]],
             "documents": [["Some text without a date"]],
@@ -82,6 +83,7 @@ def test_search_memories_query_error_degrades_to_warning(self):
         would be worse than a crash because it makes the palace look empty
         when the data is actually there."""
         mock_col = MagicMock()
+        mock_col.count.return_value = 0
         mock_col.query.side_effect = RuntimeError("query failed")
         # col.get is also called (for the sqlite fallback and pool count);
         # return an empty pool so the fallback finds nothing to promote.
@@ -226,6 +228,7 @@ def test_search_query_error_degrades_to_warning(self, capsys):
         fallback. The warning is printed so the user sees why the palace
         is returning fewer results than expected."""
         mock_col = MagicMock()
+        mock_col.count.return_value = 0
         mock_col.query.side_effect = RuntimeError("boom")
         mock_col.get.return_value = {"documents": [], "metadatas": [], "ids": []}