fix(pgvector, server): add rich filter operators and fix /search 502 (#5263)
This commit is contained in:
@@ -16,7 +16,6 @@ pytest.importorskip("fastapi", reason="fastapi not installed")
|
||||
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fixtures
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -306,9 +305,9 @@ class TestExistingParamsUnchanged:
|
||||
})
|
||||
assert resp.status_code == 200
|
||||
_, kwargs = mock_memory.search.call_args
|
||||
assert kwargs["user_id"] == "u1"
|
||||
assert kwargs["agent_id"] == "a1"
|
||||
assert kwargs["filters"] == {"category": "food"}
|
||||
assert kwargs["filters"]["user_id"] == "u1"
|
||||
assert kwargs["filters"]["agent_id"] == "a1"
|
||||
assert kwargs["filters"]["category"] == "food"
|
||||
|
||||
def test_add_metadata_still_forwarded(self, client, mock_memory):
|
||||
resp = client.post("/memories", json={
|
||||
@@ -465,11 +464,14 @@ class TestCallSignatureMatch:
|
||||
"top_k": 10, "threshold": 0.5,
|
||||
})
|
||||
assert resp.status_code == 200
|
||||
# The handler passes query= as a keyword arg, so it appears in kwargs too
|
||||
_, kwargs = mock_memory.search.call_args
|
||||
valid_params = {"query", "user_id", "agent_id", "run_id", "top_k", "filters", "threshold", "rerank"}
|
||||
valid_params = {"query", "top_k", "filters", "threshold", "rerank"}
|
||||
for key in kwargs:
|
||||
assert key in valid_params, f"Unexpected kwarg '{key}' forwarded to Memory.search()"
|
||||
assert kwargs["filters"]["user_id"] == "u1"
|
||||
assert kwargs["filters"]["agent_id"] == "a1"
|
||||
assert kwargs["filters"]["run_id"] == "r1"
|
||||
assert kwargs["filters"]["k"] == "v"
|
||||
|
||||
def test_add_kwargs_are_valid(self, client, mock_memory):
|
||||
"""All kwargs forwarded to Memory.add() must be in its signature."""
|
||||
@@ -588,3 +590,69 @@ class TestGetMemories:
|
||||
# 3. Verify the core logic: the param was mapped to the filters dict!
|
||||
_, kwargs = mock_memory.get_all.call_args
|
||||
assert kwargs["filters"] == {"user_id": "test_routing_user"}
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
# SearchRequest: entity IDs mapped into filters (fix for server 502)
|
||||
# ===========================================================================
|
||||
|
||||
class TestSearchEntityIdMapping:
|
||||
"""Verify that POST /search maps top-level user_id / agent_id / run_id
|
||||
into the filters dict instead of forwarding them as kwargs, which would
|
||||
cause Memory.search() to raise ValueError in v3."""
|
||||
|
||||
def test_user_id_mapped_to_filters(self, client, mock_memory):
|
||||
resp = client.post("/search", json={"query": "food", "user_id": "u1"})
|
||||
assert resp.status_code == 200
|
||||
_, kwargs = mock_memory.search.call_args
|
||||
assert "user_id" not in kwargs
|
||||
assert kwargs["filters"]["user_id"] == "u1"
|
||||
|
||||
def test_agent_id_mapped_to_filters(self, client, mock_memory):
|
||||
resp = client.post("/search", json={"query": "food", "agent_id": "a1"})
|
||||
assert resp.status_code == 200
|
||||
_, kwargs = mock_memory.search.call_args
|
||||
assert "agent_id" not in kwargs
|
||||
assert kwargs["filters"]["agent_id"] == "a1"
|
||||
|
||||
def test_run_id_mapped_to_filters(self, client, mock_memory):
|
||||
resp = client.post("/search", json={"query": "food", "run_id": "r1"})
|
||||
assert resp.status_code == 200
|
||||
_, kwargs = mock_memory.search.call_args
|
||||
assert "run_id" not in kwargs
|
||||
assert kwargs["filters"]["run_id"] == "r1"
|
||||
|
||||
def test_all_entity_ids_mapped(self, client, mock_memory):
|
||||
resp = client.post("/search", json={
|
||||
"query": "food", "user_id": "u1", "agent_id": "a1", "run_id": "r1",
|
||||
})
|
||||
assert resp.status_code == 200
|
||||
_, kwargs = mock_memory.search.call_args
|
||||
assert kwargs["filters"] == {"user_id": "u1", "agent_id": "a1", "run_id": "r1"}
|
||||
|
||||
def test_entity_ids_merged_with_explicit_filters(self, client, mock_memory):
|
||||
resp = client.post("/search", json={
|
||||
"query": "food",
|
||||
"user_id": "u1",
|
||||
"filters": {"category": "food"},
|
||||
})
|
||||
assert resp.status_code == 200
|
||||
_, kwargs = mock_memory.search.call_args
|
||||
assert kwargs["filters"]["user_id"] == "u1"
|
||||
assert kwargs["filters"]["category"] == "food"
|
||||
|
||||
def test_no_entity_ids_no_filters(self, client, mock_memory):
|
||||
resp = client.post("/search", json={"query": "food"})
|
||||
assert resp.status_code == 200
|
||||
_, kwargs = mock_memory.search.call_args
|
||||
assert kwargs["filters"] == {}
|
||||
|
||||
def test_only_filters_no_entity_ids(self, client, mock_memory):
|
||||
resp = client.post("/search", json={
|
||||
"query": "food",
|
||||
"filters": {"user_id": "u1", "category": "food"},
|
||||
})
|
||||
assert resp.status_code == 200
|
||||
_, kwargs = mock_memory.search.call_args
|
||||
assert kwargs["filters"]["user_id"] == "u1"
|
||||
assert kwargs["filters"]["category"] == "food"
|
||||
|
||||
@@ -4,7 +4,7 @@ import unittest
|
||||
import uuid
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from mem0.vector_stores.pgvector import PGVector
|
||||
from mem0.vector_stores.pgvector import PGVector, _build_filter_conditions
|
||||
|
||||
|
||||
class TestPGVector(unittest.TestCase):
|
||||
@@ -2233,3 +2233,158 @@ class TestPGVector(unittest.TestCase):
|
||||
def tearDown(self):
|
||||
"""Clean up after each test."""
|
||||
pass
|
||||
|
||||
|
||||
class TestBuildFilterConditions(unittest.TestCase):
|
||||
"""Tests for the _build_filter_conditions helper that translates filter dicts to SQL."""
|
||||
|
||||
def test_none_filters(self):
|
||||
conditions, params = _build_filter_conditions(None)
|
||||
self.assertEqual(conditions, [])
|
||||
self.assertEqual(params, [])
|
||||
|
||||
def test_empty_filters(self):
|
||||
conditions, params = _build_filter_conditions({})
|
||||
self.assertEqual(conditions, [])
|
||||
self.assertEqual(params, [])
|
||||
|
||||
def test_simple_equality(self):
|
||||
conditions, params = _build_filter_conditions({"user_id": "alice"})
|
||||
self.assertEqual(len(conditions), 1)
|
||||
self.assertIn("payload->>%s = %s", conditions[0])
|
||||
self.assertEqual(params, ["user_id", "alice"])
|
||||
|
||||
def test_multiple_equalities(self):
|
||||
conditions, params = _build_filter_conditions({"user_id": "alice", "agent_id": "bot1"})
|
||||
self.assertEqual(len(conditions), 2)
|
||||
self.assertEqual(params, ["user_id", "alice", "agent_id", "bot1"])
|
||||
|
||||
def test_eq_operator(self):
|
||||
conditions, params = _build_filter_conditions({"status": {"eq": "active"}})
|
||||
self.assertEqual(len(conditions), 1)
|
||||
self.assertIn("payload->>%s = %s", conditions[0])
|
||||
self.assertEqual(params, ["status", "active"])
|
||||
|
||||
def test_ne_operator(self):
|
||||
conditions, params = _build_filter_conditions({"status": {"ne": "deleted"}})
|
||||
self.assertEqual(len(conditions), 1)
|
||||
self.assertIn("payload->>%s != %s", conditions[0])
|
||||
self.assertEqual(params, ["status", "deleted"])
|
||||
|
||||
def test_gt_operator(self):
|
||||
conditions, params = _build_filter_conditions({"price": {"gt": 100}})
|
||||
self.assertEqual(len(conditions), 1)
|
||||
self.assertIn("(payload->>%s)::numeric > %s", conditions[0])
|
||||
self.assertEqual(params, ["price", 100.0])
|
||||
|
||||
def test_gte_operator(self):
|
||||
conditions, params = _build_filter_conditions({"price": {"gte": 100}})
|
||||
self.assertEqual(len(conditions), 1)
|
||||
self.assertIn("(payload->>%s)::numeric >= %s", conditions[0])
|
||||
self.assertEqual(params, ["price", 100.0])
|
||||
|
||||
def test_lt_operator(self):
|
||||
conditions, params = _build_filter_conditions({"price": {"lt": 50}})
|
||||
self.assertEqual(len(conditions), 1)
|
||||
self.assertIn("(payload->>%s)::numeric < %s", conditions[0])
|
||||
self.assertEqual(params, ["price", 50.0])
|
||||
|
||||
def test_lte_operator(self):
|
||||
conditions, params = _build_filter_conditions({"price": {"lte": 50}})
|
||||
self.assertEqual(len(conditions), 1)
|
||||
self.assertIn("(payload->>%s)::numeric <= %s", conditions[0])
|
||||
self.assertEqual(params, ["price", 50.0])
|
||||
|
||||
def test_range_combination(self):
|
||||
conditions, params = _build_filter_conditions({"score": {"gte": 1, "lte": 10}})
|
||||
self.assertEqual(len(conditions), 2)
|
||||
self.assertIn("(payload->>%s)::numeric >= %s", conditions[0])
|
||||
self.assertIn("(payload->>%s)::numeric <= %s", conditions[1])
|
||||
self.assertEqual(params, ["score", 1.0, "score", 10.0])
|
||||
|
||||
def test_in_operator(self):
|
||||
conditions, params = _build_filter_conditions({"status": {"in": ["active", "pending"]}})
|
||||
self.assertEqual(len(conditions), 1)
|
||||
self.assertIn("payload->>%s = ANY(%s)", conditions[0])
|
||||
self.assertEqual(params, ["status", ["active", "pending"]])
|
||||
|
||||
def test_nin_operator(self):
|
||||
conditions, params = _build_filter_conditions({"status": {"nin": ["deleted", "archived"]}})
|
||||
self.assertEqual(len(conditions), 1)
|
||||
self.assertIn("NOT (payload->>%s = ANY(%s))", conditions[0])
|
||||
self.assertEqual(params, ["status", ["deleted", "archived"]])
|
||||
|
||||
def test_contains_operator(self):
|
||||
conditions, params = _build_filter_conditions({"name": {"contains": "alice"}})
|
||||
self.assertEqual(len(conditions), 1)
|
||||
self.assertIn("payload->>%s LIKE %s", conditions[0])
|
||||
self.assertEqual(params, ["name", "%alice%"])
|
||||
|
||||
def test_icontains_operator(self):
|
||||
conditions, params = _build_filter_conditions({"name": {"icontains": "Alice"}})
|
||||
self.assertEqual(len(conditions), 1)
|
||||
self.assertIn("payload->>%s ILIKE %s", conditions[0])
|
||||
self.assertEqual(params, ["name", "%Alice%"])
|
||||
|
||||
def test_wildcard(self):
|
||||
conditions, params = _build_filter_conditions({"metadata_key": "*"})
|
||||
self.assertEqual(len(conditions), 1)
|
||||
self.assertIn("payload ? %s", conditions[0])
|
||||
self.assertEqual(params, ["metadata_key"])
|
||||
|
||||
def test_list_shorthand(self):
|
||||
conditions, params = _build_filter_conditions({"tags": ["a", "b", "c"]})
|
||||
self.assertEqual(len(conditions), 1)
|
||||
self.assertIn("payload->>%s = ANY(%s)", conditions[0])
|
||||
self.assertEqual(params, ["tags", ["a", "b", "c"]])
|
||||
|
||||
def test_or_operator(self):
|
||||
conditions, params = _build_filter_conditions({
|
||||
"$or": [
|
||||
{"user_id": "alice"},
|
||||
{"user_id": "bob"},
|
||||
]
|
||||
})
|
||||
self.assertEqual(len(conditions), 1)
|
||||
self.assertIn(" OR ", conditions[0])
|
||||
self.assertTrue(conditions[0].startswith("("))
|
||||
self.assertEqual(params, ["user_id", "alice", "user_id", "bob"])
|
||||
|
||||
def test_not_operator(self):
|
||||
conditions, params = _build_filter_conditions({
|
||||
"$not": [
|
||||
{"status": "deleted"},
|
||||
]
|
||||
})
|
||||
self.assertEqual(len(conditions), 1)
|
||||
self.assertTrue(conditions[0].startswith("NOT"))
|
||||
self.assertEqual(params, ["status", "deleted"])
|
||||
|
||||
def test_or_with_operators(self):
|
||||
conditions, params = _build_filter_conditions({
|
||||
"$or": [
|
||||
{"price": {"gt": 100}},
|
||||
{"price": {"lt": 10}},
|
||||
]
|
||||
})
|
||||
self.assertEqual(len(conditions), 1)
|
||||
self.assertIn(" OR ", conditions[0])
|
||||
self.assertEqual(params, ["price", 100.0, "price", 10.0])
|
||||
|
||||
def test_mixed_simple_and_operator_filters(self):
|
||||
conditions, params = _build_filter_conditions({
|
||||
"user_id": "alice",
|
||||
"score": {"gte": 5},
|
||||
})
|
||||
self.assertEqual(len(conditions), 2)
|
||||
self.assertIn("payload->>%s = %s", conditions[0])
|
||||
self.assertIn("(payload->>%s)::numeric >= %s", conditions[1])
|
||||
|
||||
def test_unsupported_operator_raises(self):
|
||||
with self.assertRaises(ValueError) as ctx:
|
||||
_build_filter_conditions({"x": {"badop": 1}})
|
||||
self.assertIn("Unsupported filter operator", str(ctx.exception))
|
||||
|
||||
def test_in_with_numeric_values(self):
|
||||
conditions, params = _build_filter_conditions({"priority": {"in": [1, 2, 3]}})
|
||||
self.assertEqual(params, ["priority", ["1", "2", "3"]])
|
||||
|
||||
Reference in New Issue
Block a user