Skip to content

Commit 2774526

Browse files
authored
Python: Initialize key mapping for a custom FAISS index (#14569)
### Motivation and Context I found that `FaissCollection.ensure_collection_exists(index=...)` accepts a trained custom index, but the first `upsert()` raises `VectorStoreOperationException` caused by `KeyError('vector')`. This prevents record insertion through the documented single-index setup path, including custom HNSW indexes. The single-index branch returns before initializing the vector field's key map. Both the automatic and `indexes={...}` branches already initialize that map. The existing custom-index test covers collection creation without inserting a record. ### Description I added the missing `setdefault` before the early return. It initializes a new mapping and preserves an existing mapping when collection setup runs again. The supplied index object, training validation, native search behavior, default setup and dictionary setup remain unchanged. The change adds no file operations or persisted-format changes. The new test uses real flat L2, flat inner-product and HNSW indexes across both custom-index entry points. It checks insertion, retrieval, ordered search, repeated setup after insertion, and collection deletion/recreation. The `ensure_collection_exists` docstring also clarifies that supplied indexes for a new collection must contain no vectors. Insert records through the collection so native rows and key mappings stay aligned. Pre-populated external indexes remain unsupported; this documentation change adds no rejection or runtime behavior. After the documentation update in `3879d30662a7`, the FAISS and in-memory modules pass all 64 tests together. Applicable changed-file formatting/syntax hooks and whitespace checks pass. Current-head hosted CI still awaits maintainer approval. Validation on Windows, Python 3.11.9 and faiss-cpu 1.13.2: - On pristine `main`, the three single-index regression cases fail with the missing-map error; the three dictionary-route controls pass. - `pytest tests/unit/connectors/memory/test_faiss.py`: 21 passed. - `pytest tests/unit/connectors/memory/test_in_memory.py`: 43 passed on base and patch. - Twenty-four additional compatibility cases pass on base and patch, covering direct/store callers, automatic/dictionary setup, a supplied existing map, L2/IP metrics, renamed vector fields, storage aliases, integer keys and repeated operations. - The repository's pinned Ruff/format, end-of-file, line-ending, debug-statement, syntax and pyupgrade hooks passed on both changed files. `git diff --check` passed. I did not run the complete Python suite, GPU FAISS or non-Python builds locally. The FAISS unit module emits existing SWIG deprecation warnings. ### Contribution Checklist - [ ] The code builds clean without any errors or warnings - [x] The PR follows the [SK Contribution Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md) and the [pre-submission formatting script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts) raises no violations - [ ] All unit tests pass, and I have added new tests where possible - [ ] I didn't break anyone 😄 The local validation above covers the affected connector and its inherited in-memory behavior. The full-build and complete-suite assertions remain unchecked. AI assistance disclosure: AI-assisted development tools were used during investigation and implementation. The reported tests were run against the final diff.
1 parent 12361d9 commit 2774526

2 files changed

Lines changed: 46 additions & 0 deletions

File tree

‎python/semantic_kernel/connectors/faiss.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ def _create_indexes(self, index: faiss.Index | None = None, indexes: dict[str, f
122122
if not index.is_trained:
123123
raise VectorStoreInitializationException("Index must be trained before using.")
124124
self.indexes[self.definition.vector_fields[0].name] = index
125+
self.indexes_key_map.setdefault(self.definition.vector_fields[0].name, {})
125126
return
126127
for vector_field in self.definition.vector_fields:
127128
if indexes and vector_field.name in indexes:
@@ -153,6 +154,9 @@ async def ensure_collection_exists(
153154
This includes indexes that need training, or GPU-based indexes, since you would also
154155
need to build the faiss package for use with GPU's yourself.
155156
157+
For a new collection, supplied indexes must contain no vectors. Insert records
158+
through this collection so the vectors, records, and key mappings stay aligned.
159+
156160
Args:
157161
index: The index to use, this can be used when there is only one vector field.
158162
indexes: A dictionary of indexes, the key is the name of the vector field.

‎python/tests/unit/connectors/memory/test_faiss.py‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,48 @@ async def test_ensure_collection_exists_custom_dict(store, data_model_def):
102102
await collection.ensure_collection_deleted()
103103

104104

105+
@mark.parametrize("use_index_dict", [False, True], ids=["single-index", "index-dict"])
106+
@mark.parametrize("index_kind", ["flat-l2", "flat-ip", "hnsw"])
107+
async def test_custom_index_upsert_and_search(store, data_model_def, use_index_dict, index_kind):
108+
collection = store.get_collection(collection_name="test", record_type=dict, definition=data_model_def)
109+
records = [
110+
{"id": "first", "content": "first record", "vector": [1.0, 0.0, 0.0, 0.0, 0.0]},
111+
{"id": "second", "content": "second record", "vector": [-1.0, 0.0, 0.0, 0.0, 0.0]},
112+
]
113+
114+
for _ in range(2):
115+
if index_kind == "flat-l2":
116+
index = faiss.IndexFlatL2(5)
117+
elif index_kind == "flat-ip":
118+
index = faiss.IndexFlatIP(5)
119+
else:
120+
index = faiss.IndexHNSWFlat(5, 4)
121+
kwargs = {"indexes": {"vector": index}} if use_index_dict else {"index": index}
122+
await collection.ensure_collection_exists(**kwargs)
123+
assert collection.indexes["vector"] is index
124+
125+
assert await collection.upsert(records[0]) == "first"
126+
key_map = collection.indexes_key_map["vector"]
127+
await collection.ensure_collection_exists(**kwargs)
128+
assert collection.indexes_key_map["vector"] is key_map
129+
assert key_map == {"first": 0}
130+
assert await collection.upsert(records[1:]) == ["second"]
131+
assert key_map == {"first": 0, "second": 1}
132+
133+
results = await collection.search(
134+
vector=records[0]["vector"], top=2, include_total_count=True, include_vectors=True
135+
)
136+
assert results.total_count == 2
137+
assert [result.record async for result in results.results] == records
138+
assert await collection.get("first", include_vectors=True) == records[0]
139+
assert index.ntotal == 2
140+
141+
await collection.ensure_collection_deleted()
142+
assert collection.indexes == {}
143+
assert collection.indexes_key_map == {}
144+
assert collection.inner_storage == {}
145+
146+
105147
async def test_upsert(faiss_collection):
106148
await faiss_collection.ensure_collection_exists()
107149
record = {"id": "testid", "content": "test content", "vector": [0.1, 0.2, 0.3, 0.4, 0.5]}

0 commit comments

Comments
 (0)