fix: guard None metadata/doc in tool_check_duplicate and Layer1/Layer2#1030
Merged
igorls merged 1 commit intoMemPalace:developfrom May 6, 2026
Merged
Conversation
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**.
eldar702
requested review from
bensig,
igorls and
milla-jovovich
as code owners
This was referenced May 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Chroma 1.5.x can return
Noneinside themetadatas/documentslists of a query/get result for partially-flushed rows. The codebase
already has a systemic None-guard pattern (merged #999, #1013 and in
flight #1019), but three call sites are still unguarded and reproduce
the same
AttributeError: 'NoneType' object has no attribute 'get'crash family:
1.
mcp_server.tool_check_duplicate—mcp_server.py:487-488```python
meta = results["metadatas"][0][i] # can be None
doc = results["documents"][0][i] # can be None
...
"wing": meta.get("wing", "?"), # AttributeError
```
The broad
except Exceptionwrapper (line 504) swallows the realcause and returns an uninformative
{\"error\": \"Duplicate check failed\"}to the MCP client.2.
layers.Layer1.generate—layers.py:126```python
for doc, meta in zip(docs, metas):
...
for key in ("importance", "emotional_weight", "weight"):
val = meta.get(key) # AttributeError on None
```
A single None metadata blows up the whole
L1_essentialwake-uprender.
3.
layers.Layer2.retrieve—layers.py:224Same pattern, same crash path, for the on-demand render.
Change
Apply the same
meta = meta or {}/doc = doc or \"\"idiom themerged guards in
searcher.pyalready use. Three-line additions, nobehaviour change on well-formed results.
Test plan
test_check_duplicate_handles_none_metadatacol.queryto returnNoneat one metadata + one document slot; asserts the call does not crash and the sentinel-rendered entry haswing/room == \"?\"and empty content.test_layer1_handles_none_metadataLayer1.generate()over a docs/metas pair where one metadata isNone; asserts render succeeds and contains the well-formed memory.test_layer1_handles_none_documenttest_layer2_handles_none_metadataLayer2.retrieve()over a list with aNonemetadata; asserts L2 render emits the header without crashing.All four new tests pass; full
test_layers.pyandtest_mcp_server.py::TestWriteToolssuites pass unchanged.
Relationship to other open PRs
searcher.pyloops. This PR extends the sameguard to the three call sites fix(searcher): guard against None metadata/doc in search result loops #1019 does not touch.
tool_check_duplicatenegative-similarity clampingbut leaves the None-metadata path unguarded — they're orthogonal and
can merge in either order.
Layer3.search_raw) or merged fix(searcher): guard against None metadata in CLI print path #999.Why it doesn't introduce new issues
The
or {}/or \"\"pattern is exactly what the existingmerged None-guard PRs used. It is a strict subset of behaviour change:
on well-formed results the coercion is a no-op; on
Noneitsubstitutes an empty sentinel that downstream
.get(...)andstring-slicing tolerate. Crashes on these paths become graceful
renders — no data is silently lost because the underlying
doccontent is already missing at the backend level.