Fix/check duplicate negative similarity#979
Fix/check duplicate negative similarity#979shafdev wants to merge 6 commits intoMemPalace:developfrom
Conversation
Bumps version across pyproject.toml, mempalace/version.py, README badge, and uv.lock. Finalizes the 3.3.0 CHANGELOG section (was still labeled 'Unreleased') and adds a 3.3.1 section covering the multi-language entity-detection infra and the five new locales landed since 2026-04-13. Highlights: - Multi-language entity detection infra (MemPalace#911) + script-aware word boundaries for combining-mark scripts (MemPalace#932) + BCP 47 case-insensitive locale resolution (MemPalace#928) + i18n patterns wired into miner/palace/ entity_registry (MemPalace#931) - Five new fully-supported locales: pt-br (MemPalace#156), ru (MemPalace#760), it (MemPalace#907), hi (MemPalace#773), id (MemPalace#778) - UTF-8 encoding fix on read_text() calls for non-UTF-8 Windows locales (MemPalace#946) - KnowledgeGraph lock correctness (MemPalace#884, MemPalace#887) - Various smaller fixes and improvements
Advisor caught: initial boundary (962776c..develop) skipped PRs that landed on develop after v3.3.0 tag but before the sync-back merge. Adds entries for MemPalace#871 MEMPAL_VERBOSE, MemPalace#811 research() local-only default, MemPalace#866 init .gitignore, MemPalace#864 MCP stdout redirect, MemPalace#863 precompact hook, MemPalace#865 searcher empty results, MemPalace#831 cold-start palace, MemPalace#862 init help, MemPalace#815 Slack provenance, MemPalace#840 save hook auto-mine. Also drops the awkward caveat on MemPalace#846 created_at — it's post-v3.3.0.
version-guard workflow checks five sources must agree: mempalace/version.py, pyproject.toml, .claude-plugin/marketplace.json, .claude-plugin/plugin.json, .codex-plugin/plugin.json. Initial release commit missed the three plugin manifests.
|
Hi, Layer3.search() now clamps similarity to >= 0, but Layer3.search_raw() still returns round(1 - dist, 3), which can be negative for cosine distances > 1.0. This leaves an inconsistent Layer3 API where raw callers can still receive negative similarity values despite the PR’s stated fix for Layer3. Severity: action required | Category: correctness How to fix: Clamp similarity in search_raw Agent prompt to fix - you can give this to your LLM of choice:
Qodo code review - free for open-source. |
945c07e to
acf537d
Compare
…nd Layer3 With hnsw:space=cosine, ChromaDB distances are in [0, 2]. For maximally dissimilar vectors the distance can exceed 1.0, making 1 - dist negative. searcher.py already guards this with max(0.0, 1 - dist) at line 285; tool_check_duplicate (mcp_server.py) and Layer3.search() (layers.py) were missing the same clamp. Fix: apply max(0.0, 1 - dist) in both locations so similarity is always in [0.0, 1.0]. Regression tests: - test_check_duplicate_similarity_never_negative - test_layer3_search_similarity_never_negative Fixes MemPalace#978
acf537d to
8a1703b
Compare
|
The Qodo reviewer's The scope issue is real: the files to strip before merging are |
``search_memories`` computes ``effective_dist = dist - boost`` where ``boost`` can be as large as ``CLOSET_RANK_BOOSTS[0] == 0.40`` for a rank-0 closet hit. When the raw drawer distance is small — any near-exact match — the subtraction goes negative. Two downstream effects: 1. Line 418 returns ``round(max(0.0, 1 - effective_dist), 3)`` as ``similarity``. With ``effective_dist = -0.30`` that yields ``similarity = 1.30``, outside the documented ``[0, 1]`` range. The ``max(0.0, ...)`` only prevents negative similarities; it does not cap above 1. 2. Line 427 stores ``_sort_key: effective_dist`` and line 435 sorts ``scored`` ascending by that key. A negative key drops *below* the rest, so the strongest hybrid matches end up sorting after weaker ones — ranking inversion under the exact conditions hybrid retrieval is supposed to serve best. Clamp ``effective_dist`` to the valid cosine-distance range ``[0, 2]``. The boost still wins (closet-backed hit still ranks first), it just no longer flips the order. Test added: mock drawer_col (base dist 0.08 / 0.35 for two sources) + closet_col (rank-0 closet for the 0.08 source) → assert all hits have ``0 <= similarity <= 1`` and ``0 <= effective_distance <= 2``, and that the closet-boosted source still ranks first. Relationship to other PRs: * **MemPalace#988** clamps the output ``similarity`` alone. That does not fix the sort-key inversion or the invalid ``effective_distance`` in the returned dict. This PR clamps at the arithmetic source so both downstream users of the value stay in range. * Orthogonal to **MemPalace#979** (``tool_check_duplicate`` negative similarity).
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**.
|
Thanks @ramv-mle! Closing as duplicate — PR #988 lands the same fix at +2/-2 (just the two missing |
``search_memories`` computes ``effective_dist = dist - boost`` where ``boost`` can be as large as ``CLOSET_RANK_BOOSTS[0] == 0.40`` for a rank-0 closet hit. When the raw drawer distance is small — any near-exact match — the subtraction goes negative. Two downstream effects: 1. Line 418 returns ``round(max(0.0, 1 - effective_dist), 3)`` as ``similarity``. With ``effective_dist = -0.30`` that yields ``similarity = 1.30``, outside the documented ``[0, 1]`` range. The ``max(0.0, ...)`` only prevents negative similarities; it does not cap above 1. 2. Line 427 stores ``_sort_key: effective_dist`` and line 435 sorts ``scored`` ascending by that key. A negative key drops *below* the rest, so the strongest hybrid matches end up sorting after weaker ones — ranking inversion under the exact conditions hybrid retrieval is supposed to serve best. Clamp ``effective_dist`` to the valid cosine-distance range ``[0, 2]``. The boost still wins (closet-backed hit still ranks first), it just no longer flips the order. Test added: mock drawer_col (base dist 0.08 / 0.35 for two sources) + closet_col (rank-0 closet for the 0.08 source) → assert all hits have ``0 <= similarity <= 1`` and ``0 <= effective_distance <= 2``, and that the closet-boosted source still ranks first. Relationship to other PRs: * **MemPalace#988** clamps the output ``similarity`` alone. That does not fix the sort-key inversion or the invalid ``effective_distance`` in the returned dict. This PR clamps at the arithmetic source so both downstream users of the value stay in range. * Orthogonal to **MemPalace#979** (``tool_check_duplicate`` negative similarity).
fix: clamp similarity scores to 0.0 minimum in tool_check_duplicate and Layer3
Fixes #978
What does this PR do?
Fixes a silent correctness bug in two places where cosine distance was converted to similarity without clamping:
mempalace/mcp_server.py—tool_check_duplicate()mempalace/layers.py—Layer3.search()Both used
round(1 - dist, 3). Withhnsw:space=cosine, ChromaDB distances are in[0, 2]— not[0, 1]. For maximally dissimilar vectors the distance slightly exceeds1.0, making1 - distnegative (e.g.dist=1.0035→similarity=-0.004).Before:
After:
searcher.pyline 285 already uses the correctmax(0.0, ...)pattern — this PR brings the two remaining sites into line with it.How to test
Run the new regression tests:
test_check_duplicate_similarity_never_negativequeries with unrelated content (threshold=0.0) and asserts all returned similarity scores are>= 0.0.test_layer3_search_similarity_never_negativemocks a ChromaDB distance of1.5(beyond orthogonal) and asserts the rendered output containssim=0.0and notsim=-.You can also reproduce the original bug directly:
Checklist
python -m pytest tests/ -v)ruff check .)