Fix: set cosine distance metadata on all collection creation sites#807
Conversation
ChromaDB defaults HNSW index to L2 (Euclidean) distance, but
MemPalace scoring uses 1-distance which requires cosine (range 0-2).
Add metadata={"hnsw:space": "cosine"} to the 4 production and 3 test
call sites that were missing it.
Closes MemPalace#218
sha2fiddy
requested review from
bensig,
igorls and
milla-jovovich
as code owners
|
@sha2fiddy thank so much for catching this! The fix is correct. All 4 missing sites verified: backends/chroma.py:88, cli.py:243 (repair), cli.py:389 (compressed), CI needs a small cleanup. The lint failures are just formatting — looks like a newer local ruff reformatted things the uvx --from 'ruff>=0.4.0,<0.5' ruff format . Ideally the final diff is just the metadata={"hnsw:space": "cosine"} additions with no extra reformatting — keeps it One thing to call out for users: Since hnsw:space is set at collection-creation time, existing palaces on L2 won't Once the formatting is cleaned up, this is good to go. We'll close #304 as superseded after this lands. Thank you from Milla and Lu ✨ |
|
@milla-jovovich Fixed the lint — reformatted with pinned ruff 0.4.x. The test failures are pre-existing on develop (version.py out of sync with pyproject.toml), not from this PR. Added an upgrade note to the PR description since |
3 participants
Summary
metadata={"hnsw:space": "cosine"}to all 4 productioncreate_collection/get_or_create_collectioncall sites that were missing it (backends/chroma.py,cli.pyx2,migrate.py)conftest.py,test_mcp_server.py,test_miner.py)ChromaBackend.get_collection(create=True)produces a cosine-distance collectionCloses #218. Supersedes stale branch
feature/fix-cosine-distance(170 files divergent, pre-backend-refactor).Upgrading existing palaces
Existing palaces created before this fix use L2 (Euclidean) distance and will not self-heal from this merge. The
hnsw:spacesetting is baked in at collection-creation time. After upgrading, runmempalace repairto rebuild the collection with cosine distance.Context
ChromaDB defaults HNSW index to L2 (Euclidean) distance. MemPalace scoring computes
1 - distance, which only produces valid similarity scores under cosine distance (range 0–2). Under L2, distances are unbounded, producing negative scores. Onlymcp_server.pyandrepair.pywere passing the cosine metadata.Note on
metadata=vsconfiguration=This PR uses the legacy
metadata={"hnsw:space": "cosine"}syntax intentionally. ChromaDB 0.6.3+ introduced a newerconfiguration=API, but it requires aCollectionConfigurationInternalobject — passing a plain dict doesn't work in 0.6.x. Since the repo pinschromadb>=0.5.0,<0.7, the legacymetadata=approach is the only one that works across the full supported range. When the chromadb pin is bumped (e.g. PR #664), these calls should be migrated to theconfiguration=API.Test plan
pytest tests/ -x -v— 685 passed (2 pre-existing version-mismatch failures unrelated to this change)ruff check mempalace/ tests/— all checks passed (formatted with CI-pinned ruff 0.4.x)create_collection/get_or_create_collectioncall (excluding benchmarks) now hasmetadata=or is read-onlyget_collectiontest_chroma_backend_creates_collection_with_cosine_distanceverifies metadata via rawchromadb.PersistentClientmempalace repairconfirmed to rebuild collections withmetadata={"hnsw:space": "cosine"}(cli.py:247)