fix: merge same-key operator dicts in AND metadata filters (#4853)
This commit is contained in:
+24
-8
@@ -1271,6 +1271,14 @@ class Memory(MemoryBase):
|
|||||||
raise ValueError(f"Unsupported metadata filter operator: {operator}")
|
raise ValueError(f"Unsupported metadata filter operator: {operator}")
|
||||||
return result
|
return result
|
||||||
|
|
||||||
|
def merge_filters(target: Dict[str, Any], source: Dict[str, Any]) -> None:
|
||||||
|
"""Merge source into target, deep-merging nested operator dicts for the same key."""
|
||||||
|
for key, value in source.items():
|
||||||
|
if key in target and isinstance(target[key], dict) and isinstance(value, dict):
|
||||||
|
target[key].update(value)
|
||||||
|
else:
|
||||||
|
target[key] = value
|
||||||
|
|
||||||
for key, value in metadata_filters.items():
|
for key, value in metadata_filters.items():
|
||||||
if key == "AND":
|
if key == "AND":
|
||||||
# Logical AND: combine multiple conditions
|
# Logical AND: combine multiple conditions
|
||||||
@@ -1278,7 +1286,7 @@ class Memory(MemoryBase):
|
|||||||
raise ValueError("AND operator requires a list of conditions")
|
raise ValueError("AND operator requires a list of conditions")
|
||||||
for condition in value:
|
for condition in value:
|
||||||
for sub_key, sub_value in condition.items():
|
for sub_key, sub_value in condition.items():
|
||||||
processed_filters.update(process_condition(sub_key, sub_value))
|
merge_filters(processed_filters, process_condition(sub_key, sub_value))
|
||||||
elif key == "OR":
|
elif key == "OR":
|
||||||
# Logical OR: Pass through to vector store for implementation-specific handling
|
# Logical OR: Pass through to vector store for implementation-specific handling
|
||||||
if not isinstance(value, list) or not value:
|
if not isinstance(value, list) or not value:
|
||||||
@@ -1288,7 +1296,7 @@ class Memory(MemoryBase):
|
|||||||
for condition in value:
|
for condition in value:
|
||||||
or_condition = {}
|
or_condition = {}
|
||||||
for sub_key, sub_value in condition.items():
|
for sub_key, sub_value in condition.items():
|
||||||
or_condition.update(process_condition(sub_key, sub_value))
|
merge_filters(or_condition, process_condition(sub_key, sub_value))
|
||||||
processed_filters["$or"].append(or_condition)
|
processed_filters["$or"].append(or_condition)
|
||||||
elif key == "NOT":
|
elif key == "NOT":
|
||||||
# Logical NOT: Pass through to vector store for implementation-specific handling
|
# Logical NOT: Pass through to vector store for implementation-specific handling
|
||||||
@@ -1298,10 +1306,10 @@ class Memory(MemoryBase):
|
|||||||
for condition in value:
|
for condition in value:
|
||||||
not_condition = {}
|
not_condition = {}
|
||||||
for sub_key, sub_value in condition.items():
|
for sub_key, sub_value in condition.items():
|
||||||
not_condition.update(process_condition(sub_key, sub_value))
|
merge_filters(not_condition, process_condition(sub_key, sub_value))
|
||||||
processed_filters["$not"].append(not_condition)
|
processed_filters["$not"].append(not_condition)
|
||||||
else:
|
else:
|
||||||
processed_filters.update(process_condition(key, value))
|
merge_filters(processed_filters, process_condition(key, value))
|
||||||
|
|
||||||
return processed_filters
|
return processed_filters
|
||||||
|
|
||||||
@@ -2682,6 +2690,14 @@ class AsyncMemory(MemoryBase):
|
|||||||
raise ValueError(f"Unsupported metadata filter operator: {operator}")
|
raise ValueError(f"Unsupported metadata filter operator: {operator}")
|
||||||
return result
|
return result
|
||||||
|
|
||||||
|
def merge_filters(target: Dict[str, Any], source: Dict[str, Any]) -> None:
|
||||||
|
"""Merge source into target, deep-merging nested operator dicts for the same key."""
|
||||||
|
for key, value in source.items():
|
||||||
|
if key in target and isinstance(target[key], dict) and isinstance(value, dict):
|
||||||
|
target[key].update(value)
|
||||||
|
else:
|
||||||
|
target[key] = value
|
||||||
|
|
||||||
for key, value in metadata_filters.items():
|
for key, value in metadata_filters.items():
|
||||||
if key == "AND":
|
if key == "AND":
|
||||||
# Logical AND: combine multiple conditions
|
# Logical AND: combine multiple conditions
|
||||||
@@ -2689,7 +2705,7 @@ class AsyncMemory(MemoryBase):
|
|||||||
raise ValueError("AND operator requires a list of conditions")
|
raise ValueError("AND operator requires a list of conditions")
|
||||||
for condition in value:
|
for condition in value:
|
||||||
for sub_key, sub_value in condition.items():
|
for sub_key, sub_value in condition.items():
|
||||||
processed_filters.update(process_condition(sub_key, sub_value))
|
merge_filters(processed_filters, process_condition(sub_key, sub_value))
|
||||||
elif key == "OR":
|
elif key == "OR":
|
||||||
# Logical OR: Pass through to vector store for implementation-specific handling
|
# Logical OR: Pass through to vector store for implementation-specific handling
|
||||||
if not isinstance(value, list) or not value:
|
if not isinstance(value, list) or not value:
|
||||||
@@ -2699,7 +2715,7 @@ class AsyncMemory(MemoryBase):
|
|||||||
for condition in value:
|
for condition in value:
|
||||||
or_condition = {}
|
or_condition = {}
|
||||||
for sub_key, sub_value in condition.items():
|
for sub_key, sub_value in condition.items():
|
||||||
or_condition.update(process_condition(sub_key, sub_value))
|
merge_filters(or_condition, process_condition(sub_key, sub_value))
|
||||||
processed_filters["$or"].append(or_condition)
|
processed_filters["$or"].append(or_condition)
|
||||||
elif key == "NOT":
|
elif key == "NOT":
|
||||||
# Logical NOT: Pass through to vector store for implementation-specific handling
|
# Logical NOT: Pass through to vector store for implementation-specific handling
|
||||||
@@ -2709,10 +2725,10 @@ class AsyncMemory(MemoryBase):
|
|||||||
for condition in value:
|
for condition in value:
|
||||||
not_condition = {}
|
not_condition = {}
|
||||||
for sub_key, sub_value in condition.items():
|
for sub_key, sub_value in condition.items():
|
||||||
not_condition.update(process_condition(sub_key, sub_value))
|
merge_filters(not_condition, process_condition(sub_key, sub_value))
|
||||||
processed_filters["$not"].append(not_condition)
|
processed_filters["$not"].append(not_condition)
|
||||||
else:
|
else:
|
||||||
processed_filters.update(process_condition(key, value))
|
merge_filters(processed_filters, process_condition(key, value))
|
||||||
|
|
||||||
return processed_filters
|
return processed_filters
|
||||||
|
|
||||||
|
|||||||
@@ -760,6 +760,38 @@ class TestProcessMetadataFiltersMerge:
|
|||||||
"score": {"gt": 0.5, "lt": 0.9},
|
"score": {"gt": 0.5, "lt": 0.9},
|
||||||
}
|
}
|
||||||
|
|
||||||
|
def test_and_same_key_different_operators_merged(self, mock_sqlite, mock_llm_factory, mock_vector_factory, mock_embedder_factory):
|
||||||
|
"""AND with same key in separate conditions must merge operators (issue #4850)."""
|
||||||
|
memory = self._make_memory(mock_sqlite, mock_llm_factory, mock_vector_factory, mock_embedder_factory)
|
||||||
|
result = memory._process_metadata_filters({
|
||||||
|
"AND": [{"price": {"gt": 10}}, {"price": {"lt": 20}}]
|
||||||
|
})
|
||||||
|
assert result == {"price": {"gt": 10, "lt": 20}}
|
||||||
|
|
||||||
|
def test_and_same_key_three_operators_merged(self, mock_sqlite, mock_llm_factory, mock_vector_factory, mock_embedder_factory):
|
||||||
|
"""AND with three conditions on the same key must merge all operators."""
|
||||||
|
memory = self._make_memory(mock_sqlite, mock_llm_factory, mock_vector_factory, mock_embedder_factory)
|
||||||
|
result = memory._process_metadata_filters({
|
||||||
|
"AND": [{"price": {"gte": 5}}, {"price": {"lte": 100}}, {"price": {"ne": 50}}]
|
||||||
|
})
|
||||||
|
assert result == {"price": {"gte": 5, "lte": 100, "ne": 50}}
|
||||||
|
|
||||||
|
def test_and_mixed_keys_with_same_key_overlap(self, mock_sqlite, mock_llm_factory, mock_vector_factory, mock_embedder_factory):
|
||||||
|
"""AND with a mix of same-key and different-key conditions."""
|
||||||
|
memory = self._make_memory(mock_sqlite, mock_llm_factory, mock_vector_factory, mock_embedder_factory)
|
||||||
|
result = memory._process_metadata_filters({
|
||||||
|
"AND": [{"price": {"gt": 10}}, {"category": "electronics"}, {"price": {"lt": 20}}]
|
||||||
|
})
|
||||||
|
assert result == {"price": {"gt": 10, "lt": 20}, "category": "electronics"}
|
||||||
|
|
||||||
|
def test_and_simple_equality_no_merge(self, mock_sqlite, mock_llm_factory, mock_vector_factory, mock_embedder_factory):
|
||||||
|
"""AND with simple equality values on the same key — last value wins."""
|
||||||
|
memory = self._make_memory(mock_sqlite, mock_llm_factory, mock_vector_factory, mock_embedder_factory)
|
||||||
|
result = memory._process_metadata_filters({
|
||||||
|
"AND": [{"status": "active"}, {"status": "pending"}]
|
||||||
|
})
|
||||||
|
assert result == {"status": "pending"}
|
||||||
|
|
||||||
|
|
||||||
# --- Issue #3040: reset() should clean up graph database ---
|
# --- Issue #3040: reset() should clean up graph database ---
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user