[Jeevan Babu Gotru (jeevan1098)]

Fixes #14547

Problem

InMemoryCollection.upsert() raised VectorStoreOperationException: 'id' when a dict or dataclass record's key field had a storage_name different from its Python field name. The record serializer produces the storage name, but _inner_upsert() indexed the serialized record using the Python name.

Fix

_inner_upsert now reads the key by the key field's storage name first, falling back to the Python field name for record types that serialize with it (e.g. Pydantic models via model_dump()).

Tests

Added regression tests for dict, dataclass, and Pydantic records with a key storage_name alias. Full test_in_memory.py suite: 46 passed.

[Copilot]

🟡 Changes recommended

A confirmed indentation error prevents importing the connector and collecting its tests.

1 open finding

• Fix unexpected indentation before async def · New

What changed in this PR

Addresses #14547 by supporting key storage-name aliases in Python’s in-memory vector store.

Changes:

• Looks up serialized keys by storage name, falling back to the Python field name.
• Adds regression tests for dictionary, dataclass, and Pydantic records.

File	Description
python/​tests/​unit/​connectors/​memory/​test_in_memory.py	Adds key-alias regression tests.
python/​semantic_kernel/​connectors/​in_memory.py	Updates upsert key lookup to support storage aliases.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[PRABHU KIRAN VANDRANKI (VANDRANKI)]

Community review. I am not a maintainer, so this does not clear the merge gate.

First, the branch as pushed does not import. In python/semantic_kernel/connectors/in_memory.py the diff changes the _inner_upsert definition line from 4 spaces to 8 spaces of indentation while leaving its body at 8, right after the @override decorator at 4 spaces (diff line + async def _inner_upsert(...)). On the PR head:

$ python -c "import semantic_kernel.connectors.in_memory"
  File ".../semantic_kernel/connectors/in_memory.py", line 632
    async def _inner_upsert(self, records: Sequence[Any], **kwargs: Any) -> Sequence[TKey]:
IndentationError: unexpected indent

pytest tests/unit/connectors/memory/test_in_memory.py stops at collection with that error, so every test in the file errors, including the new ones.

I then tried the idea itself. I read the full diff, de-indented line 632 by 4 spaces in a scratch copy of the PR head (python/ in a venv, Python 3.13, nothing else changed), and ran the file:

• Fixed PR head: 46 passed.
• in_memory.py from origin/main with the PR's tests: test_upsert_with_key_storage_name_dict and test_upsert_with_key_storage_name_dataclass fail, the Pydantic test and the other 43 pass. So the storage-name problem is real for dict and dataclass records, and the Pydantic test is a guard that passes on main already.

So the logic is fine (use record[key_storage_name] when present and fall back to record[key_name]), and the fix to push is the one indentation change. I did not check whether ruff or the repo's pre-commit hooks flag anything else in the test file, and I did not run other connector tests or CI.

[Jeevan Babu Gotru (jeevan1098)]

@microsoft-github-policy-service agree

[Eduard van Valkenburg (eavanvalkenburg)]

The storage-name fallback introduces a Pydantic key collision; see the inline reproduction. CI also reports that Ruff modifies the new test imports, so please apply the configured hooks.

[Eduard van Valkenburg (eavanvalkenburg)]

Preferring the storage name can use a data-field value as the key for Pydantic records. With a key field id (storage_name="stored_id") and a data field stored_id (storage_name="payload"), Pydantic serialization keeps the logical names. I reproduced upsert(Record(id="actual-key", stored_id="payload-value", vector=[1.0, 0.0])) returning "payload-value"; get("actual-key") then returns None. The old Pydantic path used id correctly. Please choose the serialized key according to the record serializer's contract rather than a storage-first fallback, and add this overlapping-name regression.

[Jeevan Babu Gotru (jeevan1098)]

Thanks Eduard van Valkenburg (@eavanvalkenburg) — fixed both points.

Key choice: _inner_upsert no longer prefers the storage name. It now picks the key according to the record serializer's contract — Pydantic models serialize with model_dump() (Python field names), while dict/dataclass records are keyed by storage name — with the other name only as a fallback when the expected one is absent. Your overlapping-name reproduction now returns actual-key, and get("actual-key") returns the record.

I also made the matching change on the read side in the same file: InMemory's _deserialize_store_models_to_dicts now renames field name -> storage name in one pass for Pydantic records, because the shared Pydantic deserializer expects storage-keyed dicts and had the same overlap collision (it popped the data field's stored_id value into id). Without it, get() failed validation on your repro even with the upsert key fixed.

Added test_upsert_pydantic_key_storage_name_overlapping_data_field with your exact field layout (fails without the fix). Full test file: 47 passed. Also applied the Ruff import ordering/formatting to the test file, so the pre-commit hook should be clean now.

AI-assisted contribution, verified locally as described above.

[Jeevan Babu Gotru (jeevan1098)]

Update after merging current main into this branch: while this PR was open, upstream #14548 (merged Oct 26) landed the serializer-contract fix for the upsert side — _serialize_data_model_to_dict now tracks the key field name on _InMemoryRecord, and #14547 is closed. I have dropped my _inner_upsert rework in favor of that implementation, and dropped my original three storage-name tests, which #14548's parametrized tests now cover.

What remains in this PR is the read side of your reproduction, which still fails on current main: upsert returns actual-key, but get("actual-key") raises a deserialization error because the shared Pydantic deserializer pops the data field's stored_id value into id. The remaining change is the one-pass field-name -> storage-name rename in InMemory's _deserialize_store_models_to_dicts for Pydantic records, plus the regression test test_upsert_pydantic_key_storage_name_overlapping_data_field, which asserts the full upsert + get round-trip with your field layout. Full test file on the merged tree: 82 passed, Ruff check and format clean.

Happy to close this instead if you would rather handle the read side separately — just say the word.

AI-assisted contribution, verified locally as described above.