Conversation
Collaborator
🤖 Open Code ReviewTarget: PR #2388 ✅ OpenCodeReview: Review complete: 0 finding(s) across 2 selected item(s). Generated by cloud-assistant via Open Code Review. |
Collaborator
|
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
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.
Description
With a Qdrant collection configured for Euclidean distance,
GeneralTextMemory.search()returns its matches in the wrong order. For example, a query with distances0.0,0.7071, and2.0comes back with the farthest match first, even though Qdrant returned the nearest one first. This also reverses the order within a limited top-k result.The memory layer currently re-sorts every score descending. That works for similarity scores, but Euclidean scores are distances. This change keeps the backend's relevance order when converting vector results into memory items. It leaves the scores, selected memories, and public API unchanged.
The regression stores three memories in a real embedded Qdrant collection and searches them using fixed embeddings. It covers Euclidean, cosine, and dot-product collections, plus a smaller top-k. Only the LLM and embedding providers are mocked; the memory and vector-store paths run normally.
Fixes #2387
Type of change
How Has This Been Tested?
poetry run pytest tests/memories/textual/ tests/vec_dbs/test_qdrant.py -q: 76 passed.make format: passed.Validated on macOS 15.5 / Python 3.12.14 with qdrant-client 1.19.1. The tests use embedded Qdrant; a remote Qdrant server, Milvus, and the full project/API suite were not run.
Checklist
Documentation: no public API or configuration change; a separate documentation PR is not needed. No specific reviewer has been requested.
Reviewer Checklist