Skip to content

fix: return empty list for non-positive top_k in cosine_topk#423

Open
koriyoshi2041 wants to merge 1 commit into
NevaMind-AI:mainfrom
koriyoshi2041:fix/cosine-topk-nonpositive-k
Open

fix: return empty list for non-positive top_k in cosine_topk#423
koriyoshi2041 wants to merge 1 commit into
NevaMind-AI:mainfrom
koriyoshi2041:fix/cosine-topk-nonpositive-k

Conversation

@koriyoshi2041
Copy link
Copy Markdown

cosine_topk returns the entire corpus when top_k is 0, and n-1 results when top_k is negative, instead of returning nothing.

src/memu/database/inmemory/vector.py:

actual_k = min(k, n)
if actual_k == n:
    topk_indices = np.argsort(scores)[::-1]
else:
    topk_indices = np.argpartition(scores, -actual_k)[-actual_k:]   # [-0:] == [0:] -> all
    ...

For k == 0, actual_k is 0, and the else branch slices [-0:] which is [0:] — the whole array. For k == -1 it slices [1:] (n-1 items).

top_k reaches cosine_topk unvalidated from retrieve_config.{category,item,resource}.top_k, so a user setting a tier's top_k to 0 to disable it silently gets the full corpus back rather than an empty result — an over-retrieval bug that also inflates token usage downstream.

Fix: return [] for k <= 0. cosine_topk_salience is guarded the same way — scored[:0] already returned [], but scored[:-1] (for k = -1) incorrectly dropped the last entry, so both now behave consistently.

Added tests/test_vector.py covering k=0/k=-1 (empty) and a positive-k ordering case for both functions. pytest tests/test_vector.py passes; ruff clean.

cosine_topk selected results with argpartition using actual_k = min(k, n).
For k == 0 this evaluated the [-0:] slice and returned the entire corpus
instead of nothing, and k < 0 returned n-1 results. top_k flows unvalidated
from the retrieve config, so a tier set to top_k=0 silently retrieved every
memory. Return [] for k <= 0, and guard cosine_topk_salience the same way
(where k < 0 likewise sliced off the wrong entries).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant