Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion python/semantic_kernel/data/vector.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import operator
import sys
from abc import abstractmethod
from ast import AST, Lambda, NodeVisitor, expr, parse
from ast import AST, Constant, Lambda, NodeTransformer, NodeVisitor, UnaryOp, USub, copy_location, expr, parse
from collections.abc import AsyncIterable, Callable, Mapping, Sequence
from copy import deepcopy
from dataclasses import dataclass
Expand Down Expand Up @@ -714,6 +714,28 @@ class GetFilteredRecordOptions:
order_by: Mapping[str, bool] | None = None


class _FoldUnaryNumericConstants(NodeTransformer):
"""Fold negative numeric literals into constants before filter parsers run.

Python parses negative literals as ``UnaryOp(USub, Constant(n))``. Most vector-store
``_lambda_parser`` implementations reject unary ``+/-`` wholesale, so filters such as
``lambda x: x.price > -5`` fail even though a negative bound is otherwise valid.
Folding the unary into a constant here keeps connector parsers unchanged (#14571).
"""

def visit_UnaryOp(self, node: UnaryOp) -> AST:
"""Fold unary minus of an int/float constant; leave other unary ops alone."""
self.generic_visit(node)
if (
isinstance(node.op, USub)
and isinstance(node.operand, Constant)
and isinstance(node.operand.value, (int, float))
and not isinstance(node.operand.value, bool)
):
return copy_location(Constant(-node.operand.value), node)
return node


class LambdaVisitor(NodeVisitor, Generic[TFilters]):
"""Visitor class to visit the AST nodes."""

Expand Down Expand Up @@ -1987,9 +2009,12 @@ def _build_filter(self, search_filter: OptionalOneOrMany[Callable | str] | None)
created_filters: list[Any] = []

visitor = LambdaVisitor(self._lambda_parser)
folder = _FoldUnaryNumericConstants()
for filter_ in filters:
# parse lambda expression with AST
tree = parse(filter_ if isinstance(filter_, str) else getsource(filter_).strip())
# Fold negative literals so connector parsers see a numeric Constant (#14571).
tree = folder.visit(tree)
visitor.visit(tree)
created_filters = visitor.output_filters
if len(created_filters) == 0:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,9 @@ class FilterRecord:
("x.id == 1", '"id" = %s', [1]),
("x.id != 1", '"id" <> %s', [1]),
("x.id > 1", '"id" > %s', [1]),
("x.id > -1", '"id" > %s', [-1]),
("x.id == -1", '"id" = %s', [-1]),
("x.id == -1.5", '"id" = %s', [-1.5]),
("x.id >= 1", '"id" >= %s', [1]),
("x.id < 1", '"id" < %s', [1]),
("x.id <= 1", '"id" <= %s', [1]),
Expand Down Expand Up @@ -636,7 +639,6 @@ def test_filter_identifier_quoting(filter_collection):
("1j != None", VectorStoreOperationException),
("x.id is None", NotImplementedError),
("x.id is not None", NotImplementedError),
("x.id == -1", NotImplementedError),
("x.id == +1", NotImplementedError),
("x.id == ~1", NotImplementedError),
("x.tenant.startswith('a')", NotImplementedError),
Expand Down
86 changes: 86 additions & 0 deletions python/tests/unit/data/test_vector_negative_filter_literals.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
# Copyright (c) Microsoft. All rights reserved.

"""Regression: negative numeric literals in vector-store filter lambdas (#14571)."""

from ast import Constant, UnaryOp, dump, parse
from unittest.mock import MagicMock

import pytest
from chromadb.api import ClientAPI
from qdrant_client.async_qdrant_client import AsyncQdrantClient
from qdrant_client.models import FieldCondition, Range

from semantic_kernel.connectors.chroma import ChromaCollection
from semantic_kernel.connectors.qdrant import QdrantCollection
from semantic_kernel.data.vector import (
VectorStoreCollectionDefinition,
VectorStoreField,
_FoldUnaryNumericConstants,
)


@pytest.fixture
def numeric_definition() -> VectorStoreCollectionDefinition:
return VectorStoreCollectionDefinition(
fields=[
VectorStoreField("key", name="id", type="str"),
VectorStoreField("data", name="price", type="float"),
VectorStoreField("vector", name="vector", dimensions=2, type="float"),
]
)


@pytest.mark.parametrize("literal,expected", [("-5", -5), ("-5.5", -5.5)])
def test_fold_unary_numeric_constants_negates_literal(literal, expected):
tree = parse(f"lambda x: x.price > {literal}")
folded = _FoldUnaryNumericConstants().visit(tree)
# Body of the lambda should now compare against Constant(-5), not UnaryOp(USub, ...).
compare = folded.body[0].value.body
right = compare.comparators[0]
assert isinstance(right, Constant)
assert right.value == expected
assert not isinstance(right, UnaryOp)


@pytest.mark.parametrize("expression", ["-True", "-False", "+True", "+False", "+5", "+5.5", "~5", "-x.price"])
def test_fold_unary_numeric_constants_preserves_other_unary_expressions(expression):
tree = parse(f"lambda x: x.price > {expression}")
original = dump(tree)
folded = _FoldUnaryNumericConstants().visit(tree)
assert dump(folded) == original


@pytest.mark.parametrize("literal,expected", [("-5", -5), ("-5.5", -5.5)])
def test_chroma_build_filter_accepts_negative_literal(numeric_definition, literal, expected):
collection = ChromaCollection(
collection_name="temps",
record_type=dict,
definition=numeric_definition,
client=MagicMock(spec=ClientAPI),
)
parsed = collection._build_filter(f"lambda x: x.price > {literal}")
assert parsed == {"price": {"$gt": expected}}


@pytest.mark.parametrize("expression", ["-True", "-False", "+True", "+False", "+5", "+5.5", "~5", "-x.price"])
def test_chroma_build_filter_rejects_other_unary_expressions(numeric_definition, expression):
collection = ChromaCollection(
collection_name="temps",
record_type=dict,
definition=numeric_definition,
client=MagicMock(spec=ClientAPI),
)
with pytest.raises(NotImplementedError, match="Unary"):
collection._build_filter(f"lambda x: x.price > {expression}")


@pytest.mark.parametrize("literal,expected", [("-5", -5), ("-5.5", -5.5)])
def test_qdrant_build_filter_accepts_negative_literal(numeric_definition, literal, expected):
collection = QdrantCollection(
record_type=dict,
collection_name="temps",
definition=numeric_definition,
client=MagicMock(spec=AsyncQdrantClient),
)
parsed = collection._build_filter(f"lambda x: x.price > {literal}")
assert parsed == FieldCondition(key="price", range=Range(gt=expected))
Loading