Skip to content

Python: Fix VolatileMemoryStore ignoring with_embeddings=False in get_batch and get_nearest_matches - #14524

Open
PRABHU KIRAN VANDRANKI (VANDRANKI) wants to merge 1 commit into
microsoft:mainfrom
VANDRANKI:fix/volatile-memory-with-embeddings
Open

PRABHU KIRAN VANDRANKI (VANDRANKI) wants to merge 1 commit into
microsoft:mainfrom
VANDRANKI:fix/volatile-memory-with-embeddings

Conversation

@VANDRANKI

Copy link
Copy Markdown

Motivation and Context

VolatileMemoryStore.get_batch and get_nearest_matches documented with_embeddings=False as "do not include embeddings", but the code rebound a loop variable to a deepcopy and dropped it, so the original records (with embeddings) were returned. Fixes #14522.

Description

Build the list of deep copies first and clear the embedding on the copies, so the stored records keep their embeddings. Added tests/unit/memory/test_volatile_memory_store_embeddings.py (4 tests). Two of them fail on main and all pass with the change.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • The PR follows the SK Contribution Guidelines (ruff and mypy are clean on the changed files)
  • All unit tests pass, and I have added new tests where possible
  • I didn't break anyone

Copilot AI balanced review requested due to automatic review settings October 1, 2026 16:57
@semantic-kernel-automation semantic-kernel-automation Bot added the python Pull requests for the Python Semantic Kernel label Oct 1, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The focused fix correctly addresses the discarded-copy bug and includes appropriate regression coverage.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes VolatileMemoryStore to honor with_embeddings=False without mutating stored records.

Changes:

  • Deep-copies batch and nearest-match results before removing embeddings.
  • Adds unit tests for both embedding modes and storage preservation.
File Description
python/​semantic_kernel/​memory/​volatile_memory_store.py Corrects embedding removal on returned copies.
python/​tests/​unit/​memory/​test_volatile_memory_store_embeddings.py Tests embedding inclusion, exclusion, and persistence.

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

This branch was successfully deployed

1 active deployment
github-app-auth — 8939b9d8 Deployed Oct 1, 2026 by VANDRANKI via team_check #572
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Pull requests for the Python Semantic Kernel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: VolatileMemoryStore.get_batch and get_nearest_matches ignore with_embeddings=False (deepcopy result is discarded)

2 participants