[fhgffy]

Motivation and Context

Fixes #14547. Dictionary and dataclass records with a key storage_name fail during in-memory upsert because their serialized records no longer contain the Python key name.

Description

Track the key field name when converting each record to a dictionary, before its model type is lost. Default dictionary/dataclass serialization uses storage names; default Pydantic serialization uses logical names or configured serialization aliases. Index the serialized value without changing record fields or values.

Preserve logical-name-first lookup for opaque to_dict, model_dump, and model_serializer callbacks, with a storage-name fallback when the logical key is absent. A custom serialize callback produces a store-specific record and uses storage names. A callback returning None still falls back to the normal dictionary conversion.

In-memory string filters use the fields produced by the record's serializer. Default dict/dataclass serialization uses storage_name (for example, x.stored_content), while default Pydantic model_dump() keeps logical names (x.content); configured aliases or custom serialization may change those names. This PR preserves those existing field mappings and corrects the key used for indexing and upsert's returned IDs. It does not normalize the filter view across model types.

Regressions cover single/batch upsert, get, search and delete, both directions of field-name collision, aliases, and custom serialization. Six additional cases catch transparent custom callbacks that were incorrectly treated as storage-named output in the previous revision.

Validation: 155 in-memory/data tests pass; package mypy (555 files), scoped native pre-commit hooks, UTF-8/LF checks and wheel build pass. The wheel contains the exact connector source tested. Independent checks also cover serialized key transformations, derived record types, callbacks returning multiple dictionaries, vector generation, and partial serialization fallback. The build tool reports its existing pre-release-mode deprecation warning. Full unit-suite collection previously stopped with 41 optional-provider/backend import errors; the full suite and cloud integration tests were not run.

Contribution Checklist

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

[Copilot]

Copilot review overview

🟡 Changes recommended

Storage-first key selection can incorrectly use an unrelated Pydantic field when its logical name matches the key storage name.

Review effort: Balanced
Findings: 1

Open (1)

• Prefer logical field over colliding storage name in model serialization · New

What changed in this PR

Fixes in-memory upserts when key fields use storage names.

Changes:

• Resolves keys from serialized storage or Python field names.
• Adds single and batch regressions across supported model types.

File	Description
python/​semantic_kernel/​connectors/​in_memory.py	Updates key extraction during upsert.
python/​tests/​unit/​connectors/​memory/​test_in_memory.py	Tests upsert, retrieval, search, and deletion with storage names.

---

💡 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. This does not clear the merge gate. I ran this against upstream main and the PR head (python 3.13, pydantic 2.13.5, semantic-kernel python/ from the checkout). I ran tests/unit/connectors/memory/test_in_memory.py on the PR head (79 passed). I did not run the rest of the unit suite.

The bug is real, but only for non-pydantic records. With a definition where the key field has storage_name="stored_id":

• main, dict records and dataclass records: collection.upsert([...]) raises VectorStoreOperationException: Error upserting record(s) into collection 't': 'id'. The cause is _inner_upsert doing record[self._key_field_name] on a dict that was already serialized under the storage name.
• main, pydantic records: upsert already works, since model_dump() keeps logical names and the key is stored under id.
• PR head, dict records: upsert returns ['a', 'b'], inner_storage holds {'stored_id': 'a', 'stored_content': 'hello', 'vector': [...]}, get("a") returns the record, and delete("a") removes it.

I did not get a clean dataclass run on the head: my own throwaway dataclass had a required vector field and get() without include_vectors failed to rebuild it, which is my test model, not the PR. The PR's dataclass parametrization passes.

One consequence worth stating in the PR. After this change the in-memory store keeps dict and dataclass records under storage names and pydantic records under logical names, for the same definition. String filters run against the stored record, so on the PR head:

• dict: lambda x: x.stored_content == 'hello' matches, lambda x: x.content == 'hello' raises Attribute 'content' is not available in filter expression
• pydantic: lambda x: x.content == 'hello' matches, lambda x: x.stored_content == ... raises

That is not new to the PR for the non-key fields on pydantic models, but it was unreachable for dict and dataclass records with a renamed key before, because upsert failed. Either a docs note or normalizing the filter's view of the record would avoid surprising users.

Design concern, not a bug I found: _serialize_data_model_to_dict decides the key name with type(record).model_dump is BaseModel.model_dump, type(record).__pydantic_decorators__.model_serializers, and "serialize_by_alias" in ConfigDict.__annotations__. These lean on pydantic internals and on a config key that only exists in newer pydantic, to cover cases where a storage name equals another field's logical name. The test matrix is thorough, but 244 changed lines is a lot of surface for an in-memory connector. If the collision cases are rare enough, the existing fallback in _inner_upsert (try storage name, then logical name) might be all that is needed, with the collision cases documented as unsupported. That is a call for the maintainers; I did not try to break the heuristic with a case the tests miss.

[fhgffy]

Thanks for checking this. I confirmed the filter-name distinction and added a note to the description: filters see the serializer output, which this PR preserves. I kept the key-name tracking because storage-first lookup regresses Pydantic records that work on the baseline. With the key stored as content, the ordinary content value becomes the returned ID, and deleting by the real ID leaves the record behind. I'd keep that compatibility covered if we simplify the implementation further.