fix: guard against empty ChromaDB query results in search paths

[dr-robert-li]

What does this PR do?

Guards searcher.search, searcher.search_memories, layers.Layer3.search, and layers.Layer3.search_raw against empty ChromaDB query results. ChromaDB 1.x can return {documents: []} (empty outer list) for queries against a fresh collection or filter that excludes everything; the pre-fix code did results['documents'][0] and crashed with IndexError.

ChromaDB 0.6.x returns {documents: [[]]} (empty inner) which the existing if not docs check already handled, so this only manifests on 1.x — but the guard hardens both shapes for free.

Fixes #195.

How to test

python -m pytest tests/test_searcher.py::test_search_memories_handles_empty_outer_list                  tests/test_searcher.py::test_layer3_search_raw_handles_empty_outer_list -v

Tests inject the empty-outer shape via mock so they exercise the guard regardless of installed ChromaDB version. Without the fix they raise IndexError: list index out of range.

Checklist

• Tests pass (python -m pytest tests/ -v) — 119 passed, 0 failed
• No hardcoded paths
• Linter passes (ruff check .)

[bgauryy]

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

Executive Summary

Aspect	Value
PR Goal	Guard 4 search paths against IndexError when ChromaDB 1.x returns {documents: []} instead of {documents: [[]]}
Files Changed	3
Risk Level	🟢 LOW - purely defensive guards, no behavior change on success path
Review Effort	1 - small, focused fix with clear pattern
Recommendation	✅ APPROVE

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

Aspect	Score
Correctness	5/5
Security	5/5
Performance	5/5
Maintainability	4/5

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 🔍🐙

[milla-jovovich]

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. 💜

[bensig]

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.

[web3guru888]

Good defensive fix. The guard against {documents: []} (empty outer list) is necessary for ChromaDB 1.x compatibility, and the approach of checking before [0] indexing is correct.

Coverage is comprehensive — all 4 search paths are guarded:

• searcher.search() — CLI search
• searcher.search_memories() — library/MCP search
• layers.Layer3.search() — memory stack search
• layers.Layer3.search_raw() — raw result search

The guard logic is consistent across all paths:

if not results.get("documents") or not results["documents"][0]:

This handles both {documents: []} (1.x) and {documents: [[]]} (0.6.x) in one check, which is nice.

One subtle note: The PR moves the guard before the docs = results["documents"][0] assignment and removes the old if not docs check that came after the assignment. This is correct — the old check was dead code for 1.x since it would crash before reaching it. The new placement prevents the crash entirely.

Test approach is good — using mocks to inject the empty-outer shape means the tests work regardless of installed ChromaDB version. This is the right way to test version-dependent behavior.

We pin chromadb >=0.6.0,<0.7.0 in our integration so we haven't hit this specific shape, but the guard is forward-compatible and low-risk. Clean fix.

🔭 Reviewed as part of the MemPalace-AGI integration project — autonomous research with perfect memory. Community interaction updates are posted regularly on the dashboard.

[igorls]

Hi, thanks for the contribution.

This PR has merge conflicts with develop, and the branch has not been updated in over 7 days, which puts it before our most recent release. The conflicts are likely against work that landed in that release.

Could you rebase onto develop so we can take another look?

If this change is no longer relevant, feel free to close the PR.

(This message is part of a periodic backlog pass, sent to all open PRs that match this state.)

[igorls]

Thanks for this, and apologies for the slow response. The empty-result guard in the search path landed as a direct commit (fix(searcher): guard against empty ChromaDB query results), closing issue #195. Closing this as superseded — good catch on the IndexError.