fix(mem0): pass top_k to SDK in OSSBackend.get_all for accurate count - #52973
Closed
luxuguang-leo wants to merge 1 commit into
Closed
luxuguang-leo wants to merge 1 commit into
luxuguang-leo wants to merge 1 commit into
Conversation
OSSBackend.get_all() calls Memory.get_all(filters=filters) without passing top_k. The Mem0 SDK defaults top_k to 20, so the response is always truncated to at most 20 results. Since OSSBackend computes count = len(all_results), the count is wrong whenever there are more than 20 stored memories. Fix: pass top_k=10000 to the SDK call so the full result set is returned for accurate counting and client-side pagination. Test: 4 new tests in TestOSSBackendGetAll: - test_passes_top_k_to_sdk: verifies the parameter is forwarded - test_count_reflects_total_not_sdk_default: 50 memories, count=50 - test_client_side_pagination: page=2, size=10 returns correct slice - test_pagination_beyond_total_returns_empty: out-of-range page Fixes: NousResearch#52921
Contributor
Author
|
Quick check-in. This PR's approach was independently confirmed today by two other contributors — #58323 (bvisible, running in production) and #58410 (nankingjing) were both triaged as duplicates of this one. The check suites are stuck at |
Contributor
Author
|
This PR is obsoleted by commit 9a32272 (kshitijk4poor), which pruned Related:
Closing as no longer needed — the dead code path this PR fixed no longer exists on main. |
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.
Problem
OSSBackend.get_all()callsMemory.get_all(filters=filters)without passingtop_k. The Mem0 SDK defaultstop_kto 20 (source: mem0ai/mem0/memory/main.py), so the response is always truncated to at most 20 results.Since
OSSBackendcomputescount = len(all_results)from the raw SDK response, the reportedcountis wrong whenever there are more than 20 stored memories — it caps at 20.Root Cause
In
plugins/memory/mem0/_backend.py,OSSBackend.get_all():The Mem0 SDK's
Memory.get_all()signature is:The
PlatformBackenddoes not have this problem — it usesMemoryClient.get_all(filters, page, page_size)which handles pagination server-side and returns an accuratecount.Fix
Pass
top_k=10000toself._memory.get_all()so the full result set is returned.OSSBackendalready applies client-side pagination (page/page_size slicing) on the full set, socount = len(all_results)is correct again.Testing
4 new tests in
TestOSSBackendGetAll:test_passes_top_k_to_sdktop_k=10000is forwarded to the SDKtest_count_reflects_total_not_sdk_defaultcount=50(not 20)test_client_side_paginationcount=50test_pagination_beyond_total_returns_empty[]with correct countAll 46 mem0 tests pass.
Fixes #52921