From bfdfe436f0b16ad1fb2e34b847aeb0ce6049167d Mon Sep 17 00:00:00 2001 From: Alex Castillo Date: Sat, 31 Oct 2026 19:57:25 -0400 Subject: [PATCH 1/2] Python: fix FAISS delete leaving stale vectors after removing several records remove_ids shifts every later vector down by one position, but the key map kept the old positions, so later deletes removed the wrong vectors. Update the remaining positions after each removal. --- python/semantic_kernel/connectors/faiss.py | 8 +++-- .../unit/connectors/memory/test_faiss.py | 31 +++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/python/semantic_kernel/connectors/faiss.py b/python/semantic_kernel/connectors/faiss.py index 1f4c92e70226..46ce016afbe1 100644 --- a/python/semantic_kernel/connectors/faiss.py +++ b/python/semantic_kernel/connectors/faiss.py @@ -189,9 +189,13 @@ async def _inner_delete(self, keys: Sequence[TKey], **kwargs: Any) -> None: for key in keys: for vector_field in self.definition.vector_field_names: if key in self.indexes_key_map[vector_field]: - vector_index = self.indexes_key_map[vector_field][key] + key_map = self.indexes_key_map[vector_field] + vector_index = key_map.pop(key) self.indexes[vector_field].remove_ids(np.array([vector_index])) - self.indexes_key_map[vector_field].pop(key, None) + # removing a vector shifts every later vector down by one position + for other_key, position in key_map.items(): + if position > vector_index: + key_map[other_key] = position - 1 await super()._inner_delete(keys, **kwargs) @override diff --git a/python/tests/unit/connectors/memory/test_faiss.py b/python/tests/unit/connectors/memory/test_faiss.py index f0f4e7375ffc..4f9eff10fc97 100644 --- a/python/tests/unit/connectors/memory/test_faiss.py +++ b/python/tests/unit/connectors/memory/test_faiss.py @@ -179,6 +179,37 @@ async def test_delete(faiss_collection): await faiss_collection.ensure_collection_deleted() +async def test_delete_multiple_keeps_index_in_sync(faiss_collection): + await faiss_collection.ensure_collection_exists() + records = [ + {"id": "a", "content": "a", "vector": [1.0, 0.0, 0.0, 0.0, 0.0]}, + {"id": "b", "content": "b", "vector": [0.0, 1.0, 0.0, 0.0, 0.0]}, + {"id": "c", "content": "c", "vector": [0.0, 0.0, 1.0, 0.0, 0.0]}, + ] + await faiss_collection.upsert(records) + await faiss_collection.delete(["a", "b", "c"]) + assert faiss_collection.indexes["vector"].ntotal == 0 + assert faiss_collection.indexes_key_map["vector"] == {} + await faiss_collection.ensure_collection_deleted() + + +async def test_delete_updates_remaining_positions(faiss_collection): + await faiss_collection.ensure_collection_exists() + records = [ + {"id": "a", "content": "a", "vector": [1.0, 0.0, 0.0, 0.0, 0.0]}, + {"id": "b", "content": "b", "vector": [0.0, 1.0, 0.0, 0.0, 0.0]}, + {"id": "c", "content": "c", "vector": [0.0, 0.0, 1.0, 0.0, 0.0]}, + ] + await faiss_collection.upsert(records) + await faiss_collection.delete("a") + assert faiss_collection.indexes["vector"].ntotal == 2 + assert faiss_collection.indexes_key_map["vector"] == {"b": 0, "c": 1} + await faiss_collection.delete("c") + assert faiss_collection.indexes["vector"].ntotal == 1 + assert faiss_collection.indexes_key_map["vector"] == {"b": 0} + await faiss_collection.ensure_collection_deleted() + + async def test_collection_exists(faiss_collection): assert await faiss_collection.collection_exists() is False await faiss_collection.ensure_collection_exists() From 4de28a8653f127ca84a1532caaa95b0fcd19ae42 Mon Sep 17 00:00:00 2001 From: Alex Castillo Date: Sun, 1 Nov 2026 02:09:37 -0500 Subject: [PATCH 2/2] Python: keep FAISS key map consistent on delete; reject delete for non-flat indexes Remove the vector from the index before dropping the key mapping. Only flat indexes compact ids on removal, which the key map relies on (positions in search, start offset in upsert), so deleting from other indexes (e.g. IVF, HNSW) is rejected before anything is mutated. --- python/semantic_kernel/connectors/faiss.py | 14 ++++++-- .../unit/connectors/memory/test_faiss.py | 32 ++++++++++++++++++- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/python/semantic_kernel/connectors/faiss.py b/python/semantic_kernel/connectors/faiss.py index 46ce016afbe1..d80f3125749d 100644 --- a/python/semantic_kernel/connectors/faiss.py +++ b/python/semantic_kernel/connectors/faiss.py @@ -186,13 +186,23 @@ async def _inner_upsert(self, records: Sequence[Any], **kwargs: Any) -> Sequence @override async def _inner_delete(self, keys: Sequence[TKey], **kwargs: Any) -> None: + for vector_field in self.definition.vector_field_names: + if not isinstance(self.indexes[vector_field], faiss.IndexFlat) and any( + key in self.indexes_key_map[vector_field] for key in keys + ): + raise VectorStoreOperationException( + f"Deleting records is only supported for flat indexes, " + f"not for {type(self.indexes[vector_field])} (vector field '{vector_field}')." + ) for key in keys: for vector_field in self.definition.vector_field_names: if key in self.indexes_key_map[vector_field]: key_map = self.indexes_key_map[vector_field] - vector_index = key_map.pop(key) + vector_index = key_map[key] + # remove from the index first, so the mapping is kept if the index fails self.indexes[vector_field].remove_ids(np.array([vector_index])) - # removing a vector shifts every later vector down by one position + del key_map[key] + # flat indexes shift every later vector down by one position for other_key, position in key_map.items(): if position > vector_index: key_map[other_key] = position - 1 diff --git a/python/tests/unit/connectors/memory/test_faiss.py b/python/tests/unit/connectors/memory/test_faiss.py index 4f9eff10fc97..1d6f3acf5b16 100644 --- a/python/tests/unit/connectors/memory/test_faiss.py +++ b/python/tests/unit/connectors/memory/test_faiss.py @@ -1,11 +1,12 @@ # Copyright (c) Microsoft. All rights reserved. import faiss +import numpy as np 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.exceptions import VectorStoreInitializationException +from semantic_kernel.exceptions import VectorStoreInitializationException, VectorStoreOperationException @fixture(scope="function") @@ -247,3 +248,32 @@ async def test_ensure_collection_exists_and_search(faiss_collection, dist): assert res.record == record1 if idx == 0 else record2 idx += 1 await faiss_collection.ensure_collection_deleted() + + +async def test_delete_keeps_key_map_when_index_cannot_remove(store, data_model_def): + collection = store.get_collection(collection_name="test", record_type=dict, definition=data_model_def) + await collection.ensure_collection_exists(index=faiss.IndexHNSWFlat(5, 4)) + await collection.upsert({"id": "a", "content": "a", "vector": [1.0, 0.0, 0.0, 0.0, 0.0]}) + with raises(VectorStoreOperationException): + await collection.delete("a") + assert collection.indexes_key_map["vector"] == {"a": 0} + assert collection.indexes["vector"].ntotal == 1 + await collection.ensure_collection_deleted() + + +async def test_delete_rejected_for_index_that_preserves_ids(store, data_model_def): + collection = store.get_collection(collection_name="test", record_type=dict, definition=data_model_def) + index = faiss.IndexIVFFlat(faiss.IndexFlatL2(5), 5, 1) + index.train(np.random.default_rng(0).random((10, 5), dtype=np.float32)) + await collection.ensure_collection_exists(index=index) + records = [ + {"id": "a", "content": "a", "vector": [1.0, 0.0, 0.0, 0.0, 0.0]}, + {"id": "b", "content": "b", "vector": [0.0, 1.0, 0.0, 0.0, 0.0]}, + ] + await collection.upsert(records) + with raises(VectorStoreOperationException, match="flat indexes"): + await collection.delete("a") + assert index.ntotal == 2 + assert collection.indexes_key_map["vector"] == {"a": 0, "b": 1} + assert await collection.get("a") is not None + await collection.ensure_collection_deleted()