Skip to content

Python: Fix in-memory UUID key lookup and deletion - #9061

Open
Zhibo Lin (LE0-Lin) wants to merge 1 commit into
microsoft:mainfrom
LE0-Lin:fix/in-memory-uuid-keys
Open

Zhibo Lin (LE0-Lin) wants to merge 1 commit into
microsoft:mainfrom
LE0-Lin:fix/in-memory-uuid-keys

Conversation

@LE0-Lin

Copy link
Copy Markdown

Motivation & Context

UUID-keyed records can be inserted into InMemoryCollection, but querying with the model's UUID returns no record and deleting with it silently leaves the record stored. Shared serialization stores the UUID as a string; the lookup operations need to use the same representation.

Description & Review Guide

  • What are the major changes? Normalize UUID keys to strings in in-memory get and delete. Add four regression cases covering native UUIDs and existing string calls, ordered retrieval, missing keys, and deletion isolation.
  • What is the impact of these changes? Direct UUID CRUD retrieves and deletes the inserted records. Existing serialized keys and JSON CRUD tool behavior remain compatible. No public signatures, dependencies, or scoring/filter behavior change.
  • What do you want reviewers to focus on? Consistent UUID identity across serialization and CRUD, including compatibility with normalized string keys.

On unchanged production code the native UUID cases fail and the string controls pass (2 failed / 2 passed). With the patch, test_in_memory.py and test_vectors.py pass together (419 passed), including the existing UUID JSON-tool test. Ruff 0.16.10 lint/format, strict source Pyright and the five upstream test-typing configurations pass on the changed files. The full workspace and remote integrations have not been run.

Codex assisted reproduction, implementation, tests, and PR preparation. No independent human execution or review is claimed. This draft is ready for review of the small fix; upstream CI and maintainer acceptance remain pending.

Related Issue

Fixes #9060

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Signed-off-by: Zhibo Lin <147509942+LE0-Lin@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 5, 2026 15:24

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 implementation aligns lookup keys with stored serialization and includes adequate regression coverage.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes UUID-keyed record retrieval and deletion in the Python in-memory vector store.

Changes:

  • Normalizes native UUID lookup and deletion keys to serialized strings.
  • Adds regression tests for UUID/string keys, ordering, missing keys, and deletion isolation.
File Description
python/​packages/​core/​agent_framework/​_in_memory.py Normalizes UUID keys during get and delete operations.
python/​packages/​core/​tests/​core/​test_in_memory.py Adds UUID retrieval and deletion regression coverage.

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

@LE0-Lin

Zhibo Lin (LE0-Lin) commented Oct 6, 2026 •

Copy link
Copy Markdown
Author

Codex-assisted CI follow-up for head 5b233c5e4d7f8b09522e796183959d94a73dd38f: the only failed test in the Windows/Python 3.12 job is packages/ag-ui/tests/ag_ui/test_endpoint.py::test_endpoint_exactly_full_unstarted_stream_releases_detached_capacity (line 1711). It receives HTTP 200 but an empty response body, so the RUN_FINISHED assertion fails. The test sets _DETACHED_READER_START_TIMEOUT_SECONDS to 0.01 seconds.

The job reports 1 failed, 18,084 passed, 37 skipped, 2 xfailed. The other nine completed OS/Python test jobs passed; Python 3.15/Linux was cancelled. Quality, coverage, and the executed integration checks passed. merge-gatekeeper explicitly fails because of this Windows/Python 3.12 check.

This failure is in the AG-UI endpoint test, outside the two changed in-memory UUID files. The short timeout suggests a possible scheduling race, but I have not confirmed the cause through a rerun. Could a maintainer rerun the failed Windows/Python 3.12 and cancelled Linux/Python 3.15 test jobs, then refresh merge-gatekeeper if those pass? The gate also treats cancelled checks as errors. I have read-only upstream permissions and have kept the UUID patch unchanged.

This branch was previously deployed

1 inactive deployment
github-app-auth — 5b233c5e Deployed Oct 5, 2026 by LE0-Lin via team_check #6065
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Bug]: InMemoryCollection cannot get or delete records by native UUID keys

2 participants