PR Review: fix: guard against empty ChromaDB query results in search paths

Executive Summary

Affected Areas: mempalace/searcher.py (search, search_memories), mempalace/layers.py (Layer3.search, Layer3.search_raw)

Business Impact: Prevents user-facing crashes when searching an empty or freshly initialized palace, or when ChromaDB filters exclude all results.

Flow Changes: None for the success path. The empty-result path now returns gracefully instead of crashing with IndexError.

Ratings

PR Health

• Has clear description
• References ticket/issue (IndexError when ChromaDB query returns empty results #195)
• Appropriate size (55 additions, 7 deletions across 3 files)
• Has relevant tests

The Bug

ChromaDB 0.6.x returns {documents: [[]]} (empty inner list) for zero-result queries. The existing if not docs: guard (after docs = results["documents"][0]) handled this correctly.

ChromaDB 1.x returns {documents: []} (empty outer list). The pre-fix code did results["documents"][0] before any check, causing IndexError: list index out of range.

The Fix

Guard pattern applied consistently across all 4 paths:

if not results.get("documents") or not results["documents"][0]:
    return <appropriate empty response>

This guard handles:

• {documents: []} — empty outer (ChromaDB 1.x) → not [] is True → early return
• {documents: [[]]} — empty inner (ChromaDB 0.6.x) → passes first check, not [] on [0] → early return
• Missing documents key (theoretical) → .get() returns None → early return

The old if not docs: check after [0] access is correctly removed as redundant.

Medium Priority Issues

🎨 #1: Missing test for Layer3.search()

Location: tests/test_searcher.py | Confidence: ⚠️ MED

Tests cover search_memories and Layer3.search_raw but not Layer3.search(), which also received the guard. Since Layer3.search() returns a string (not sys.exit), it's trivially testable with the same mock pattern:

def test_layer3_search_handles_empty_outer_list(monkeypatch, tmp_path):
    """Issue #195: Layer3.search also affected by the empty-outer shape."""
    _mock_empty_chromadb(monkeypatch, "mempalace.layers")
    result = Layer3(palace_path=str(tmp_path)).search("anything")
    assert result == "No results found."

Skipping searcher.search() is understandable since it uses sys.exit(1) on errors, making it harder to test without capturing process exit.

Verification Notes

• Test mock targets are correct: Both searcher.py and layers.py import chromadb directly on the main branch, so monkeypatch.setattr(f"{module}.chromadb.PersistentClient", ...) resolves correctly.
• Return values are consistent: Each guarded path returns the same type/shape as the function's normal empty-result behavior (string for Layer3.search, list for search_raw, dict for search_memories, None for CLI search).
• No regression risk: The guard is pure short-circuit logic that only activates on the previously-crashing input shape.

Created by Octocode MCP https://octocode.ai 🔍🐙

Hey @dr-robert-li — I've taken a look and ran it through CLI. Thanks for the fix! There are three open PRs addressing #195 (empty ChromaDB query results) and going with #197 as the most focused diff. Closing this one as superseded — the bug is still getting fixed, just via the other PR. Really appreciate the contribution. 💜

Reopening — this was closed too quickly and your work deserves proper review. Standing by for merge once we clear the current security baseline. Thank you for the contribution.