From f54cf18adcf48b627e862a7863f82ddeb93f28c6 Mon Sep 17 00:00:00 2001 From: blue Date: Fri, 30 Oct 2026 19:35:46 +0800 Subject: [PATCH 1/3] Python: fix(faiss): resolve the serialized key name on upsert FaissCollection._inner_upsert reads the record key by the declared key field name. Records arrive already serialized, and the key name differs by serializer: the dict path writes the key's storage_name, while a pydantic model_dump() or a custom to_dict() may write the logical name. The dict shape therefore failed every upsert with KeyError, and simply switching to the storage name would have broken the pydantic shape that works today. Mirror InMemoryCollection: use the key name the serialization recorded when it is available, otherwise fall back between the storage name and the logical name. Add regression tests for the dict, pydantic and custom to_dict shapes with a key field that declares storage_name. --- python/semantic_kernel/connectors/faiss.py | 11 ++- .../unit/connectors/memory/test_faiss.py | 71 ++++++++++++++++++- 2 files changed, 80 insertions(+), 2 deletions(-) diff --git a/python/semantic_kernel/connectors/faiss.py b/python/semantic_kernel/connectors/faiss.py index 1f4c92e70226..f0f84ac1f202 100644 --- a/python/semantic_kernel/connectors/faiss.py +++ b/python/semantic_kernel/connectors/faiss.py @@ -180,7 +180,16 @@ async def _inner_upsert(self, records: Sequence[Any], **kwargs: Any) -> Sequence self.indexes[vector_field.name].add(vectors) # type: ignore start = len(self.indexes_key_map[vector_field.name]) for i, record in enumerate(records): - key = record[self.definition.key_field.name] + # Records are serialized in whichever shape the data model and + # definition produce: logical names from a pydantic model_dump(), + # storage names from the dict path, or a custom to_dict(). Use the + # key name the serialization actually recorded, like InMemory does. + key_field_name = getattr(record, "key_field_name", self._key_field_storage_name) + if key_field_name not in record: + key_field_name = ( + self._key_field_storage_name if self._key_field_storage_name in record else self._key_field_name + ) + key = record[key_field_name] self.indexes_key_map[vector_field.name][key] = start + i return await super()._inner_upsert(records, **kwargs) diff --git a/python/tests/unit/connectors/memory/test_faiss.py b/python/tests/unit/connectors/memory/test_faiss.py index f0f4e7375ffc..08880c529c39 100644 --- a/python/tests/unit/connectors/memory/test_faiss.py +++ b/python/tests/unit/connectors/memory/test_faiss.py @@ -1,10 +1,18 @@ # Copyright (c) Microsoft. All rights reserved. +from typing import Annotated + import faiss +from pydantic import BaseModel from pytest import fixture, mark, raises from semantic_kernel.connectors.faiss import FaissCollection, FaissStore -from semantic_kernel.data.vector import DistanceFunction, VectorStoreCollectionDefinition, VectorStoreField +from semantic_kernel.data.vector import ( + DistanceFunction, + VectorStoreCollectionDefinition, + VectorStoreField, + vectorstoremodel, +) from semantic_kernel.exceptions import VectorStoreInitializationException @@ -144,6 +152,67 @@ async def test_custom_index_upsert_and_search(store, data_model_def, use_index_d assert collection.inner_storage == {} +@fixture(scope="function") +def aliased_definition(data_model_def) -> VectorStoreCollectionDefinition: + """Only the key field declares a storage_name. + + This mirrors the InMemory test for the same behaviour and keeps the data and + vector fields on their logical names, so the record reaches _inner_upsert + under whichever serialization the record type produces. + """ + data_model_def.key_field.storage_name = "stored_id" + return data_model_def + + +@vectorstoremodel +class AliasedModel(BaseModel): + id: Annotated[str, VectorStoreField("key", storage_name="stored_id")] + content: Annotated[str, VectorStoreField("data")] = "test content" + vector: Annotated[ + list[float] | None, + VectorStoreField( + "vector", + dimensions=5, + index_kind="flat", + distance_function="dot_prod", + type="float", + ), + ] = None + + +@mark.parametrize("model_kind", ["dict", "pydantic"], ids=["dict", "pydantic"]) +async def test_upsert_with_key_storage_name(aliased_definition, model_kind): + """A key field with a storage_name must upsert whichever shape it serializes to.""" + model_type = {"dict": dict, "pydantic": AliasedModel}[model_kind] + collection = FaissCollection(record_type=model_type, definition=aliased_definition, collection_name="test-alias") + record = model_type(id="testid", content="test content", vector=[1.0, 0.0, 0.0, 0.0, 0.0]) + await collection.ensure_collection_exists() + + assert await collection.upsert(record) == "testid" + assert collection.indexes_key_map["vector"] == {"testid": 0} + assert set(collection.inner_storage) == {"testid"} + + results = await collection.search(vector=[1.0, 0.0, 0.0, 0.0, 0.0], top=1, include_vectors=True) + assert [result.record async for result in results.results] == [record] + await collection.ensure_collection_deleted() + + +async def test_upsert_with_custom_to_dict_logical_names(aliased_definition): + """A custom to_dict that emits the logical key name must still resolve.""" + aliased_definition.to_dict = lambda record, **kwargs: { + "id": record["id"], + "content": record["content"], + "vector": record["vector"], + } + collection = FaissCollection(record_type=dict, definition=aliased_definition, collection_name="test-alias-logical") + record = {"id": "testid", "content": "test content", "vector": [1.0, 0.0, 0.0, 0.0, 0.0]} + await collection.ensure_collection_exists() + + assert await collection.upsert(record) == "testid" + assert collection.indexes_key_map["vector"] == {"testid": 0} + await collection.ensure_collection_deleted() + + async def test_upsert(faiss_collection): await faiss_collection.ensure_collection_exists() record = {"id": "testid", "content": "test content", "vector": [0.1, 0.2, 0.3, 0.4, 0.5]} From d3004d77a411de7b16fbf18ca69e453966cbbba2 Mon Sep 17 00:00:00 2001 From: blue Date: Fri, 30 Oct 2026 21:01:08 +0800 Subject: [PATCH 2/3] Python: fix(faiss): resolve the serialized vector name on upsert The vector read in _inner_upsert assumed the field's storage_name is always present, but that is only true for the dict path: a pydantic model_dump() and a custom to_dict() keep the logical name, so a vector field with storage_name produced a one-element list and numpy raised "not enough values to unpack". Read the vector under either name, mirroring the key lookup in the previous commit, and cover the pydantic-model shape with a regression test. --- python/semantic_kernel/connectors/faiss.py | 14 ++++- .../unit/connectors/memory/test_faiss.py | 55 ++++++++++++++++++- 2 files changed, 67 insertions(+), 2 deletions(-) diff --git a/python/semantic_kernel/connectors/faiss.py b/python/semantic_kernel/connectors/faiss.py index f0f84ac1f202..0b3916f43814 100644 --- a/python/semantic_kernel/connectors/faiss.py +++ b/python/semantic_kernel/connectors/faiss.py @@ -64,6 +64,18 @@ def _create_index(field: VectorStoreField) -> faiss.Index: raise VectorStoreInitializationException(f"Index with {field.index_kind} is not supported.") +def _vector_value(record: dict[str, Any], vector_field: VectorStoreField) -> Any: + """Read a vector from a serialized record under either its storage or logical name. + + The dict path serializes with the field's storage_name, while a pydantic + model_dump() and a custom to_dict() may keep the logical name. + """ + storage_name = vector_field.storage_name or vector_field.name + if storage_name in record: + return record[storage_name] + return record.get(vector_field.name) + + class FaissCollection(InMemoryCollection[TKey, TModel], Generic[TKey, TModel]): """Create a Faiss collection. @@ -168,7 +180,7 @@ async def ensure_collection_exists( async def _inner_upsert(self, records: Sequence[Any], **kwargs: Any) -> Sequence[TKey]: """Upsert records.""" for vector_field in self.definition.vector_fields: - vectors_to_add = [record.get(vector_field.storage_name or vector_field.name) for record in records] + vectors_to_add = [_vector_value(record, vector_field) for record in records] vectors = np.array(vectors_to_add, dtype=np.float32) if not self.indexes[vector_field.name].is_trained: raise VectorStoreOperationException( diff --git a/python/tests/unit/connectors/memory/test_faiss.py b/python/tests/unit/connectors/memory/test_faiss.py index 08880c529c39..215a9323f874 100644 --- a/python/tests/unit/connectors/memory/test_faiss.py +++ b/python/tests/unit/connectors/memory/test_faiss.py @@ -4,7 +4,7 @@ import faiss from pydantic import BaseModel -from pytest import fixture, mark, raises +from pytest import approx, fixture, mark, raises from semantic_kernel.connectors.faiss import FaissCollection, FaissStore from semantic_kernel.data.vector import ( @@ -197,6 +197,59 @@ async def test_upsert_with_key_storage_name(aliased_definition, model_kind): await collection.ensure_collection_deleted() +@vectorstoremodel +class VectorAliasedModel(BaseModel): + id: Annotated[str, VectorStoreField("key", type="str")] + content: Annotated[str, VectorStoreField("data", type="str")] = "test content" + embedding: Annotated[ + list[float] | None, + VectorStoreField( + "vector", + storage_name="vec", + dimensions=5, + index_kind="flat", + distance_function="dot_prod", + type="float", + ), + ] = None + + +def vector_aliased_definition() -> VectorStoreCollectionDefinition: + return VectorStoreCollectionDefinition( + fields=[ + VectorStoreField("key", name="id", type="str"), + VectorStoreField("data", name="content", type="str"), + VectorStoreField( + "vector", + name="embedding", + storage_name="vec", + dimensions=5, + index_kind="flat", + distance_function="dot_prod", + type="float", + ), + ] + ) + + +async def test_upsert_with_vector_storage_name_pydantic_model(): + """A pydantic model_dump keeps the logical vector name; the vector must still resolve.""" + collection = FaissCollection( + record_type=VectorAliasedModel, definition=vector_aliased_definition(), collection_name="test-vec-alias" + ) + record = VectorAliasedModel(id="testid", embedding=[1.0, 0.0, 0.0, 0.0, 0.0]) + await collection.ensure_collection_exists() + + assert await collection.upsert(record) == "testid" + assert collection.indexes["embedding"].ntotal == 1 + # Assert the indexed vector itself, so a wrong-but-valid vector cannot pass. + assert collection.indexes["embedding"].reconstruct(0) == approx([1.0, 0.0, 0.0, 0.0, 0.0]) + + results = await collection.search(vector=[1.0, 0.0, 0.0, 0.0, 0.0], top=1, include_vectors=True) + assert [result.record async for result in results.results] == [record] + await collection.ensure_collection_deleted() + + async def test_upsert_with_custom_to_dict_logical_names(aliased_definition): """A custom to_dict that emits the logical key name must still resolve.""" aliased_definition.to_dict = lambda record, **kwargs: { From bd161f195a2c73c6e5cd09046382b44a4f658c5b Mon Sep 17 00:00:00 2001 From: blue Date: Fri, 30 Oct 2026 21:34:19 +0800 Subject: [PATCH 3/3] Python: fix(faiss): return no results when the index is empty _inner_search asked faiss for min(top, ntotal) neighbours without checking ntotal, so searching a collection that was just created, or one whose last record was deleted, failed with faiss' bare "assert k > 0" wrapped in a VectorSearchExecutionException. The in-memory collections return an empty result in the same situation. Return an empty result before touching faiss, and cover both the fresh-collection and the deleted-last-record paths. --- python/semantic_kernel/connectors/faiss.py | 15 +++++++-- .../unit/connectors/memory/test_faiss.py | 33 ++++++++++++++++++- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/python/semantic_kernel/connectors/faiss.py b/python/semantic_kernel/connectors/faiss.py index 0b3916f43814..a3845361e1db 100644 --- a/python/semantic_kernel/connectors/faiss.py +++ b/python/semantic_kernel/connectors/faiss.py @@ -249,9 +249,18 @@ async def _inner_search( # first we create the vector to search with np_vector = np.array(vector, dtype=np.float32).reshape(1, -1) # then do the actual vector search - distances, indexes = self.indexes[field.name].search( - np_vector, min(options.top, self.indexes[field.name].ntotal) - ) # type: ignore[call-arg] + ntotal = self.indexes[field.name].ntotal + if ntotal == 0: + # An empty (or fully deleted) index has nothing to search: faiss + # asserts k > 0. Resolve the filter and build the empty result from + # the same helper as the populated path so an invalid filter still + # raises, as it does for a populated index and for InMemory. + filtered_records = self._get_filtered_records(options) + return KernelSearchResults( + results=self._get_vector_search_results_from_results(return_list, options), + total_count=0 if options and options.include_total_count else None, + ) + distances, indexes = self.indexes[field.name].search(np_vector, min(options.top, ntotal)) # type: ignore[call-arg] # since Faiss indexes do not contain the full records, # we get the filtered records, this is a dict of the records that match the search filters # and use that to get the actual records diff --git a/python/tests/unit/connectors/memory/test_faiss.py b/python/tests/unit/connectors/memory/test_faiss.py index 215a9323f874..ad28b0edacdb 100644 --- a/python/tests/unit/connectors/memory/test_faiss.py +++ b/python/tests/unit/connectors/memory/test_faiss.py @@ -13,7 +13,7 @@ VectorStoreField, vectorstoremodel, ) -from semantic_kernel.exceptions import VectorStoreInitializationException +from semantic_kernel.exceptions import VectorStoreInitializationException, VectorStoreOperationException @fixture(scope="function") @@ -275,6 +275,37 @@ async def test_upsert(faiss_collection): await faiss_collection.ensure_collection_deleted() +async def test_search_on_empty_collection_returns_no_results(faiss_collection): + """An empty index has nothing to search; faiss would otherwise assert k > 0.""" + await faiss_collection.ensure_collection_exists() + + results = await faiss_collection.search(vector=[0.1, 0.2, 0.3, 0.4, 0.5], top=3, include_total_count=True) + assert [result async for result in results.results] == [] + assert results.total_count == 0 + await faiss_collection.ensure_collection_deleted() + + +async def test_search_on_empty_collection_still_validates_the_filter(faiss_collection): + """An empty index returns early, but an invalid filter must still raise.""" + await faiss_collection.ensure_collection_exists() + + with raises(VectorStoreOperationException): + await faiss_collection.search(vector=[0.1, 0.2, 0.3, 0.4, 0.5], top=3, filter="not valid (") + await faiss_collection.ensure_collection_deleted() + + +async def test_search_after_deleting_every_record_returns_no_results(faiss_collection): + """Deleting the last record empties the index; a later search must not raise.""" + await faiss_collection.ensure_collection_exists() + record = {"id": "testid", "content": "test content", "vector": [0.1, 0.2, 0.3, 0.4, 0.5]} + await faiss_collection.upsert(record) + await faiss_collection.delete("testid") + + results = await faiss_collection.search(vector=record["vector"], top=3) + assert [result async for result in results.results] == [] + await faiss_collection.ensure_collection_deleted() + + async def test_get(faiss_collection): await faiss_collection.ensure_collection_exists() record = {"id": "testid", "content": "test content", "vector": [0.1, 0.2, 0.3, 0.4, 0.5]}