From 9263d4f16399ac046cf893971c7fb4c5f53c8124 Mon Sep 17 00:00:00 2001 From: er-s-an Date: Fri, 9 Oct 2026 09:35:36 +0800 Subject: [PATCH 1/3] fix(python): fold negative filter literals before vector-store parsers Fixes #14571. Python parses -5 as UnaryOp(USub, Constant(5)), and most connector _lambda_parser implementations reject unary +/-, so filters like lambda x: x.price > -5 raised NotImplementedError. Fold unary +/- of numeric constants in VectorSearch._build_filter so connectors see a plain Constant. --- python/semantic_kernel/data/vector.py | 25 ++++++- .../test_vector_negative_filter_literals.py | 75 +++++++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 python/tests/unit/data/test_vector_negative_filter_literals.py diff --git a/python/semantic_kernel/data/vector.py b/python/semantic_kernel/data/vector.py index 964e9d92bec7..542bff1a0fae 100644 --- a/python/semantic_kernel/data/vector.py +++ b/python/semantic_kernel/data/vector.py @@ -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, UAdd, USub, UnaryOp, copy_location, expr, parse from collections.abc import AsyncIterable, Callable, Mapping, Sequence from copy import deepcopy from dataclasses import dataclass @@ -714,6 +714,26 @@ class GetFilteredRecordOptions: order_by: Mapping[str, bool] | None = None +class _FoldUnaryNumericConstants(NodeTransformer): + """Fold ``-5`` / ``+5`` AST nodes into numeric 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 +/- of an int/float constant; leave other unary ops alone.""" + self.generic_visit(node) + if isinstance(node.operand, Constant) and isinstance(node.operand.value, (int, float)): + if isinstance(node.op, USub): + return copy_location(Constant(-node.operand.value), node) + if isinstance(node.op, UAdd): + return copy_location(Constant(+node.operand.value), node) + return node + + class LambdaVisitor(NodeVisitor, Generic[TFilters]): """Visitor class to visit the AST nodes.""" @@ -1987,9 +2007,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 -5 / +5 into constants 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: diff --git a/python/tests/unit/data/test_vector_negative_filter_literals.py b/python/tests/unit/data/test_vector_negative_filter_literals.py new file mode 100644 index 000000000000..b506b3e3f098 --- /dev/null +++ b/python/tests/unit/data/test_vector_negative_filter_literals.py @@ -0,0 +1,75 @@ +# Copyright (c) Microsoft. All rights reserved. + +"""Regression: negative numeric literals in vector-store filter lambdas (#14571).""" + +from ast import Constant, USub, UnaryOp, parse + +from unittest.mock import MagicMock + +import pytest +from chromadb.api import ClientAPI +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"), + ] + ) + + +def test_fold_unary_numeric_constants_negates_literal(): + tree = parse("lambda x: x.price > -5") + 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 == -5 + assert not isinstance(right, UnaryOp) + + +def test_chroma_build_filter_accepts_negative_literal(numeric_definition): + collection = ChromaCollection( + collection_name="temps", + record_type=dict, + definition=numeric_definition, + client=MagicMock(spec=ClientAPI), + ) + parsed = collection._build_filter("lambda x: x.price > -5") + assert parsed == {"price": {"$gt": -5}} + + +def test_chroma_build_filter_accepts_positive_unary_literal(numeric_definition): + collection = ChromaCollection( + collection_name="temps", + record_type=dict, + definition=numeric_definition, + client=MagicMock(spec=ClientAPI), + ) + parsed = collection._build_filter("lambda x: x.price > +5") + assert parsed == {"price": {"$gt": 5}} + + +def test_qdrant_build_filter_accepts_negative_literal(numeric_definition, monkeypatch): + monkeypatch.setenv("QDRANT_LOCATION", "http://localhost:6333") + collection = QdrantCollection( + record_type=dict, + collection_name="temps", + definition=numeric_definition, + env_file_path="test.env", + ) + parsed = collection._build_filter("lambda x: x.price > -5") + assert parsed == FieldCondition(key="price", range=Range(gt=-5)) From 9365f2e2eb52e00239bc2838203e582d2d9c5086 Mon Sep 17 00:00:00 2001 From: er-s-an <92594860+er-s-an@users.noreply.github.com> Date: Fri, 9 Oct 2026 16:54:08 +0800 Subject: [PATCH 2/3] fix(python): preserve unsupported unary vector filters Limit constant folding to unary minus of exact int/float literals. Preserve boolean and unary-plus rejection, and cover negative integer and float filters without a live Qdrant client. Prepared with AI assistance; reviewed independently. Focused and related unit suites: 133 passed. Full repository and live-service tests were not run. --- python/semantic_kernel/data/vector.py | 19 ++++---- .../test_vector_negative_filter_literals.py | 43 ++++++++++++------- 2 files changed, 37 insertions(+), 25 deletions(-) diff --git a/python/semantic_kernel/data/vector.py b/python/semantic_kernel/data/vector.py index 542bff1a0fae..51f018eb7b25 100644 --- a/python/semantic_kernel/data/vector.py +++ b/python/semantic_kernel/data/vector.py @@ -5,7 +5,7 @@ import operator import sys from abc import abstractmethod -from ast import AST, Constant, Lambda, NodeTransformer, NodeVisitor, UAdd, USub, UnaryOp, copy_location, 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 @@ -715,7 +715,7 @@ class GetFilteredRecordOptions: class _FoldUnaryNumericConstants(NodeTransformer): - """Fold ``-5`` / ``+5`` AST nodes into numeric constants before filter parsers run. + """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 @@ -724,13 +724,14 @@ class _FoldUnaryNumericConstants(NodeTransformer): """ def visit_UnaryOp(self, node: UnaryOp) -> AST: - """Fold unary +/- of an int/float constant; leave other unary ops alone.""" + """Fold unary minus of an int/float constant; leave other unary ops alone.""" self.generic_visit(node) - if isinstance(node.operand, Constant) and isinstance(node.operand.value, (int, float)): - if isinstance(node.op, USub): - return copy_location(Constant(-node.operand.value), node) - if isinstance(node.op, UAdd): - return copy_location(Constant(+node.operand.value), node) + if ( + isinstance(node.op, USub) + and isinstance(node.operand, Constant) + and type(node.operand.value) in (int, float) + ): + return copy_location(Constant(-node.operand.value), node) return node @@ -2011,7 +2012,7 @@ def _build_filter(self, search_filter: OptionalOneOrMany[Callable | str] | None) for filter_ in filters: # parse lambda expression with AST tree = parse(filter_ if isinstance(filter_, str) else getsource(filter_).strip()) - # Fold -5 / +5 into constants so connector parsers see a numeric Constant (#14571). + # Fold negative literals so connector parsers see a numeric Constant (#14571). tree = folder.visit(tree) visitor.visit(tree) created_filters = visitor.output_filters diff --git a/python/tests/unit/data/test_vector_negative_filter_literals.py b/python/tests/unit/data/test_vector_negative_filter_literals.py index b506b3e3f098..7dcba10e25e0 100644 --- a/python/tests/unit/data/test_vector_negative_filter_literals.py +++ b/python/tests/unit/data/test_vector_negative_filter_literals.py @@ -2,12 +2,12 @@ """Regression: negative numeric literals in vector-store filter lambdas (#14571).""" -from ast import Constant, USub, UnaryOp, parse - +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 @@ -30,46 +30,57 @@ def numeric_definition() -> VectorStoreCollectionDefinition: ) -def test_fold_unary_numeric_constants_negates_literal(): - tree = parse("lambda x: x.price > -5") +@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 == -5 + assert right.value == expected assert not isinstance(right, UnaryOp) -def test_chroma_build_filter_accepts_negative_literal(numeric_definition): +@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("lambda x: x.price > -5") - assert parsed == {"price": {"$gt": -5}} + parsed = collection._build_filter(f"lambda x: x.price > {literal}") + assert parsed == {"price": {"$gt": expected}} -def test_chroma_build_filter_accepts_positive_unary_literal(numeric_definition): +@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), ) - parsed = collection._build_filter("lambda x: x.price > +5") - assert parsed == {"price": {"$gt": 5}} + with pytest.raises(NotImplementedError, match="Unary"): + collection._build_filter(f"lambda x: x.price > {expression}") -def test_qdrant_build_filter_accepts_negative_literal(numeric_definition, monkeypatch): - monkeypatch.setenv("QDRANT_LOCATION", "http://localhost:6333") +@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, - env_file_path="test.env", + client=MagicMock(spec=AsyncQdrantClient), ) - parsed = collection._build_filter("lambda x: x.price > -5") - assert parsed == FieldCondition(key="price", range=Range(gt=-5)) + parsed = collection._build_filter(f"lambda x: x.price > {literal}") + assert parsed == FieldCondition(key="price", range=Range(gt=expected)) From 624d553723583c8e572529f8445509a03bb6a13e Mon Sep 17 00:00:00 2001 From: er-s-an <92594860+er-s-an@users.noreply.github.com> Date: Sat, 10 Oct 2026 16:32:47 +0800 Subject: [PATCH 3/3] fix(python): address negative filter review regressions Narrow numeric constants with isinstance while keeping bool excluded. Replace the obsolete Postgres negative-integer rejection with successful negative bound, equality and float parameter cases. Prepared with AI assistance and independently reviewed. Fresh validation: 232 data/Postgres/Chroma/Qdrant unit tests passed; mypy passed for all 556 semantic_kernel source files; Ruff lint and format checks passed for both changed files. Full repository tests and live-service integration tests were not run. --- python/semantic_kernel/data/vector.py | 3 ++- python/tests/unit/connectors/memory/test_postgres_store.py | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/python/semantic_kernel/data/vector.py b/python/semantic_kernel/data/vector.py index 51f018eb7b25..151269b27cac 100644 --- a/python/semantic_kernel/data/vector.py +++ b/python/semantic_kernel/data/vector.py @@ -729,7 +729,8 @@ def visit_UnaryOp(self, node: UnaryOp) -> AST: if ( isinstance(node.op, USub) and isinstance(node.operand, Constant) - and type(node.operand.value) in (int, float) + and isinstance(node.operand.value, (int, float)) + and not isinstance(node.operand.value, bool) ): return copy_location(Constant(-node.operand.value), node) return node diff --git a/python/tests/unit/connectors/memory/test_postgres_store.py b/python/tests/unit/connectors/memory/test_postgres_store.py index 0ddf51ef2b72..fb292b97a31f 100644 --- a/python/tests/unit/connectors/memory/test_postgres_store.py +++ b/python/tests/unit/connectors/memory/test_postgres_store.py @@ -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]), @@ -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),