[alexcastgonz (pcbeingused333)]

Fixes #14695

Motivation and Context

FaissCollection.delete with several keys leaves stale vectors in the FAISS index: the repro in #14695 ends with ntotal == 1 and an empty key map, so later searches can return vectors for records that no longer exist.

Root cause: _inner_delete removes a vector with remove_ids, which shifts every later vector in a flat index down by one position, but the key map kept the old positions. Every delete after the first then targeted the wrong position.

Description

After removing a key's vector, decrement the stored position of every key that sat after it in the same vector field's map. The change is limited to _inner_delete in python/semantic_kernel/connectors/faiss.py.

Tests added in tests/unit/connectors/memory/test_faiss.py:

• test_delete_multiple_keeps_index_in_sync: the issue's scenario; index and key map are empty after deleting all records.
• test_delete_updates_remaining_positions: positions stay correct after partial deletes.

Both fail on main and pass with the change; the rest of test_faiss.py passes, and ruff format --check / ruff check are clean on the touched files.

Contribution Checklist

• The code builds clean without any errors or warnings
• The PR follows the SK Contribution Guidelines and the pre-submission formatting script raises no violations
• All unit tests pass, and I have added new tests where possible
• I didn't break anyone 😄

This change was prepared with the help of an AI coding assistant.

[Copilot]

🟡 Changes recommended

Deletion can corrupt key mappings when index removal fails and incorrectly renumber stable-ID indexes, leaving orphaned vectors.

2 open issues

• Preserve ID mapping when index removal fails · New
• Avoid renumbering IDs in indexes that preserve them · New

What changed in this PR

This PR addresses #14695 by updating the Python FAISS connector’s key mappings after flat-index deletions, preventing subsequent deletes from targeting stale positions.

Changes:

• Decrement later mapped positions after each vector removal.
• Add regression tests for deleting all records and maintaining correct positions after partial deletes.

File	Description
python/​tests/​unit/​connectors/​memory/​test_faiss.py	Adds full and partial deletion regression tests.
python/​semantic_kernel/​connectors/​faiss.py	Updates remaining key positions after vector deletion.

_{Review effort: Balanced}

[alexcastgonz (pcbeingused333)]

Thanks, both Copilot points were valid and are addressed in the new commit:

• _inner_delete now calls remove_ids first and drops the key mapping only after it succeeds, so an index that can't remove vectors (e.g. the custom IndexHNSWFlat) no longer loses its mapping. Test: test_delete_keeps_key_map_when_index_cannot_remove.
• Only flat indexes compact ids on removal, and the key map relies on that (search resolves ids by position, upsert starts from the current count). Deleting from any other index (e.g. IVF, HNSW) now raises VectorStoreOperationException before anything is mutated, instead of leaving a broken mapping. Test: test_delete_rejected_for_index_that_preserves_ids.

Both new tests fail against the previous revision; tests/unit/connectors/memory/test_faiss.py passes (25 tests) and ruff format/ruff check are clean.

This comment was drafted with Claude Code.

[PRABHU KIRAN VANDRANKI (VANDRANKI)]

I ran this on current main (df1cb33) and on main with this diff applied (python venv, faiss 1.15.1, real FaissCollection, no mocks). The diff applied cleanly.

Main reproduces the bug and it is worse than a stale count. remove_ids shifts later vectors down, and main kept the old positions in the key map, so the next delete in the same call or a later call removes the wrong vector. With 5 records a to e:

• delete(["a", "b"]): main ends with ntotal 3 and the map {c: 2, d: 3, e: 4}. The PR ends with {c: 0, d: 1, e: 2}.
• delete("a") then delete("c"): on main a search with c's own vector returns d, and a search with d's vector returns b. On the PR the surviving records b, d, e each find themselves.
• delete of all five in one call: main leaves ntotal at 2 with an empty map. The PR leaves ntotal 0.

I then checked an invariant instead of single cases. After the deletes, for every key still in the map: the vector at its list position (the order search uses to turn a faiss position into a key) must be that record's own vector, ntotal must equal the key count, and a search with the record's vector must return that record. I used 6 records and 7 delete patterns, each with one and with two vector fields:

• main breaks the invariant in 4 of the 7: ["k0","k1"], "k0" then "k2" (a search with k3's vector returns k1), ["k1","k3","k4"] (a search for k5 raises VectorSearchExecutionException: list index out of range), and deleting all six.
• the other 3 are fine on main for data (deleting only the last record, deleting only k2, deleting k0 twice), though the last two leave stale positions in the map.
• the PR holds the invariant in all 7, with one and with two vector fields.

Then 300 random sequences of deletes of 1 to 3 keys out of 6 records: 74 end consistent on main, 300 of 300 on the PR.

Non-flat indexes. This is a behavior change worth knowing about. With an IndexIVFFlat (trained), delete("a") on main returns without an error. Afterwards a search for e's vector raises list index out of range and a search for b's vector returns key c. On the PR the same delete("a") raises VectorStoreOperationException: Deleting records is only supported for flat indexes ... before anything is removed, and searches still return the right keys (e gives e, b gives b). With IndexHNSWFlat, main already raised (a wrapped faiss error, map left alone) and the PR raises the clearer message. So users with an IVF index who call delete now get an error where they used to get a silently broken index. I think that is the right call, and it may deserve a release note.

Tests. tests/unit/connectors/memory/test_faiss.py: 21 passed on main, 25 on the PR. If I copy the PR's test file onto main, 3 of the 4 new tests fail (delete_multiple_keeps_index_in_sync, delete_updates_remaining_positions, delete_rejected_for_index_that_preserves_ids). The HNSW test passes on main because main already raises there.

Limits. I did not test upserting an existing key and then deleting it, search with filters after deletes, GPU indexes, or any other index type.

Approving.