fix: guard Layer3.search_raw against None doc/meta from ChromaDB (#1011)#1013
fix: guard Layer3.search_raw against None doc/meta from ChromaDB (#1011)#1013
Conversation
igorls
left a comment
There was a problem hiding this comment.
Tested locally (py 3.12, clean worktree on pr-1013):
Reproduction confirms fix. Wrote a small script that patches _get_collection to return the partial-flush shape ChromaDB emits (valid hit + None meta + None doc):
from unittest.mock import patch
from mempalace.layers import Layer3
class FakeCol:
def query(self, **_):
return {
"documents": [["real doc text", None]],
"metadatas": [[{"wing": "w1", "room": "r1", "source_file": "/a/b.md"}, None]],
"distances": [[0.1, 0.42]],
}
with patch("mempalace.layers._get_collection", return_value=FakeCol()):
hits = Layer3(palace).search_raw("anything", n_results=2)- Without the fix (checked out
develop'slayers.py):AttributeError: 'NoneType' object has no attribute 'get'atlayers.py:333— exactly matches issue #1011. - With the fix (pr-1013
layers.py): 2 hits, second falls through cleanly withwing="unknown",room="unknown",source_file="?",text="".
Full test suite: 973 passed. The two-line guard exactly mirrors #999's pattern in searcher.py, so the code-review surface is trivial.
Worth a small follow-up (not this PR): a unit test that pins the None-meta behavior so neither this nor the searcher.py sibling regresses. The reproduction above is essentially that test.
LGTM.
Code reviewFound 2 issues:
Lines 280 to 288 in 66f33a6
mempalace/mempalace/backends/chroma.py Lines 1 to 20 in 66f33a6 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
Same class of bug as #1007: ChromaDB's query() can return None in the documents and metadatas arrays when a drawer's HNSW vector entry exists but its metadata/document rows haven't been materialized. The code in Layer3.search_raw (mempalace/layers.py) calls meta.get("wing", ...), meta.get("room", ...), meta.get("source_file", ...) directly without null safety, so it raises: AttributeError: 'NoneType' object has no attribute 'get' Two-line defensive coercion matching the pattern in #1009 / PR #999 for searcher.py: meta = meta or {}, doc = doc or "". The hit still appears with its real distance; source/wing/room fall back to their fallback values where the metadata row is missing. Frequently hit on chromadb 1.5.x (root cause #1006). Even after the chromadb floor lands (#1010), partial-state results remain possible during interrupted mines and schema upgrade boundaries, so the guard is worth having on its own. Fixes #1011.
66f33a6 to
49e9e04
Compare
|
Amended to also guard Remaining finding (adapter-level consolidation per @jphein on #999) is a maintainer design call — not pulling that into this PR. |
…uard Merges MemPalace#990 (RFC 002 spec), MemPalace#1014 (BaseSourceAdapter/PalaceContext scaffolding), MemPalace#1013 (Layer3.search_raw None guard), MemPalace#1012 (docs), MemPalace#1010 (chromadb >=1.5.4), and MemPalace#998 (sweeper/tandem transcript safety net). Fork changes preserved: - quarantine_stale_hnsw() in chroma.py (guards HNSW/sqlite drift segfault) - get-then-create instead of get_or_create (guards ChromaDB 1.5.x metadata segfault) - paginated status() loop (guards SQLite variable limit on large palaces) - searcher hits-loop, BM25 fallback, _count_in_scope - .jsonl exempt from JUNK_FILE_SIZE cap (Claude Code transcripts can be large) - _validate_where() + operator constants taken from upstream Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Chroma 1.5.x can return ``None`` inside the ``metadatas`` / ``documents`` lists of a query/get result for partially-flushed rows. The codebase already has a systemic None-guard pattern (merged MemPalace#999, MemPalace#1013, MemPalace#1019) but three call sites were still unguarded: * ``mcp_server.tool_check_duplicate`` (``mcp_server.py:487-488``) — ``meta = results["metadatas"][0][i]`` followed by ``meta.get(...)`` raises ``AttributeError: 'NoneType' object has no attribute 'get'``. The broad ``except Exception`` wrapper (line 504) swallows it and returns an uninformative ``"Duplicate check failed"``. * ``layers.Layer1.generate`` (``layers.py:126``) — iterates ``zip(docs, metas)`` and calls ``meta.get(key)`` in the importance loop. A single None metadata blows up the entire wake-up render. * ``layers.Layer2.retrieve`` (``layers.py:224``) — same pattern, same crash path for the on-demand render. Apply the same ``meta = meta or {}`` / ``doc = doc or ""`` idiom used by the merged guards in the search path. Three-line additions, no behaviour change on well-formed results. Tests added: * ``test_check_duplicate_handles_none_metadata`` — mocks the collection query to return ``None`` for one metadata and document, asserts the call does not crash and the sentinel-rendered entry has wing/room "?" and empty content. * ``test_layer1_handles_none_metadata`` / ``_handles_none_document`` * ``test_layer2_handles_none_metadata`` Relationship to other open PRs: * **MemPalace#1019** guarded ``searcher.py`` loops. This PR extends the same guard to the three call sites MemPalace#1019 did not touch. * **MemPalace#979** fixed ``tool_check_duplicate`` negative similarity but left the None-metadata path unguarded. * Does not overlap **MemPalace#1013** (``Layer3.search_raw``) or **MemPalace#999**.
Version bumps across pyproject.toml, mempalace/version.py, README badge, uv.lock, and plugin manifests (.claude-plugin/*, .codex-plugin/*). CHANGELOG aligned with main (post-3.3.1) and a new [3.3.2] section added covering the 11 PRs merged on develop since v3.3.1 — silent-transcript-drop fix + tandem sweeper (#998), None-metadata guards (#999, #1013), chromadb ≥1.5.4 for Py 3.13/3.14 (#1010), Windows Unicode (#681), HNSW quarantine recovery (#1000), PID stacking guard (#1023), doc-path cleanup (#996, #1012), and RFC 001/002 internal scaffolding (#995, #1014, #990).
…ndary
ChromaDB 1.5.x can return None inside the `metadatas` list even when
the write stored a dict. Three prior PRs added per-site `meta = meta
or {}` guards to compensate:
- MemPalace#999 — guards in searcher.py (CLI + API + closet-boost),
miner.status(), and 4 mcp_server.py handlers
- MemPalace#1013 — guard in Layer3.search_raw()
- Various other paths picked up the same defensive pattern
organically (e.g. palace_graph, layers.py L1 pre-filter)
That scatter works but violates the typed contract QueryResult and
GetResult declare: both advertise `metadatas: list[dict]`, never
`list[Optional[dict]]`. Every call site that forgot the guard is a
latent `AttributeError: 'NoneType' object has no attribute 'get'`.
This PR moves the coercion to the single place the type contract is
produced — ChromaCollection.query() and ChromaCollection.get() in
backends/chroma.py. Each individual None dict gets coerced to {} at
the same boundary where the outer list is already normalized via
_none_list_to_empty(). Downstream callers receive a guaranteed
list[dict] matching the declared return shape.
## Per-site guards
Leaving the existing guards in place as defensive belt-and-suspenders.
They become redundant after this boundary coercion but removing them
risks regressing any call path that might bypass the typed wrappers.
A follow-up cleanup PR can remove them once we're confident nothing
else reaches chromadb directly.
## Tests
Six new tests in tests/test_backends.py cover the boundary's behavior
on the known None-metadata shapes:
- query(): mixed None / dict in a single batch
- query(): all-None inner list
- query(): None dicts across multiple queries in one call
- get(): mixed None / dict
- get(): padding regression (short metas list → {} pads, never None)
- get(): metadatas not requested → empty list passthrough
Full test_backends.py suite: 35 passed (6 new).
Full repo suite: 1072 passed.
Ruff format + check clean.
Summary
Fixes #1011.
Layer3.search_raw()inmempalace/layers.pyhas the same unguardedmeta.get(...)pattern that #1007 / PR #999 fix insearcher.py. Applies the same two-line defensive coercion at the top of the results loop.Change
One file, defensive guard added at the top of the
search_rawresults loop:Test plan
Nonemetadata in ChromaDB query results (any 1.5.x install per bug: freshpip install mempalacesilently broken on chromadb 1.5.x — writes queued but never flush, search returns ghosts #1006)Layer3.search_raw("query")completes and returns resultsRelated
mempalace searchcrashes withAttributeError: 'NoneType' object has no attribute 'get'on partial-state results (searcher.py:286) #1007, PR fix(searcher): guard against None metadata in CLI print path #999 — same class of bug insearcher.pypip install mempalacesilently broken on chromadb 1.5.x — writes queued but never flush, search returns ghosts #1006, fix: upgrade chromadb to >=1.5.4 for Python 3.13/3.14 compatibility + fix 1.5.x queue-stall (closes #1006) #1010 — chromadb 1.5.x queue-stall root cause and fix