Skip to content

fix(search): recover from Chroma ID lookup divergence - #1463

Open
mrbob-git wants to merge 2 commits into
MemPalace:developfrom
mrbob-git:codex/fix-1398-search-id-fallback
Open

fix(search): recover from Chroma ID lookup divergence#1463
mrbob-git wants to merge 2 commits into
MemPalace:developfrom
mrbob-git:codex/fix-1398-search-id-fallback

Conversation

@mrbob-git

@mrbob-git mrbob-git commented May 11, 2026

Copy link
Copy Markdown

What does this PR do?

Fixes #1398.

When Chroma raises Error finding id during MCP search, the drawer search now falls back to the existing SQLite BM25 path instead of returning an error. Closet and neighbor lookups with the same error degrade gracefully to the drawer/base result and log the reduced ranking quality.

How to test

  • uv run python -m pytest tests/ -v
  • uv run pytest tests/test_searcher.py -v
  • uv run pytest tests/test_mcp_server.py::TestSearchTool::test_search_retries_once_on_hnsw_flush_transient tests/test_mcp_server.py::TestSearchTool::test_search_returns_second_error_if_retry_also_fails -v
  • uv run ruff check .
  • uv run ruff format --check mempalace/searcher.py tests/test_searcher.py

Checklist

  • Tests pass (python -m pytest tests/ -v)
  • No hardcoded paths
  • Linter passes (ruff check .)

When Chroma metadata gets ahead of its HNSW segment, MCP search can fail even though the SQLite-backed lexical path can still answer scoped queries. Route that transient drawer-query failure through the existing BM25 SQLite fallback and log lower-risk closet/neighbor divergence as ranking degradation.

Tested: uv run pytest tests/test_searcher.py -v; uv run pytest tests/test_mcp_server.py::TestSearchTool::test_search_retries_once_on_hnsw_flush_transient tests/test_mcp_server.py::TestSearchTool::test_search_returns_second_error_if_retry_also_fails -v; uv run ruff check .; uv run ruff format --check mempalace/searcher.py tests/test_searcher.py

Co-authored-by: OmX <omx@oh-my-codex.dev>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a mechanism to handle transient Chroma ID lookup errors by falling back to BM25 search via SQLite. It adds helper functions to detect these errors, perform recovery, and provide conditional logging, refactoring the search_memories function to be more resilient. New tests verify the recovery logic and ensure that failures in specific search components do not cause the entire search to fail. Feedback was provided regarding the fragility of the _log_chroma_id_lookup_or_debug helper's string formatting requirements.

Comment thread mempalace/searcher.py Outdated
@jphein

jphein commented May 11, 2026

Copy link
Copy Markdown
Collaborator

+1. This is the narrow case of the same architectural move as #1005 (when vector underdelivers, fall through to sqlite BM25) — bounded to Error finding id specifically, which is the right granularity for the chromadb 1.5.x failure mode #1398 describes.

Operator-experience data point: on a ~160K-drawer production palace this week we've been working through HNSW degradation cases including a capacity-divergence event. The exception-vs-empty-result split between this PR and #1005 means narrow catches don't fight broader catches — if Error finding id becomes the inner exception type, #1005's outer fallback catches the rest. They compose.

Graceful degrade for closet/neighbor lookups with the log surface is also the right call — those layers are enrichment, not primary signal. A partially-ranked result with a logged warning beats a hard failure.

Thanks for the fallback semantics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/mcp MCP server and tools area/search Search and retrieval bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP mempalace_search returns 'Error finding id' after bulk-add until WAL flushes; CLI works

2 participants