Repository navigation
Conversation
…enerating orphan GUID _session_partition_key previously generated a fresh random GUID for every call when session_id was None/empty. This made save_messages(None, ...) write to partition A while get_messages(None) / clear(None) read partition B/C, silently orphaning the stored data. This also diverged from the Redis scoped and VectorStore providers, which reject an empty session_id. Align Cosmos with those providers by raising ValueError for a missing session_id, and update the regression test accordingly.
lisry-1
requested review from
SergeyMenshykh,
Eduard van Valkenburg (eavanvalkenburg) and
Roger Barreto (rogerbarreto)
as code owners
October 31, 2026 07:19
lisry-1
temporarily deployed
to
github-app-auth
October 31, 2026 07:19 — with 
GitHub Actions
Inactive
GitHub Actions
Inactive
lisry-1
requested review from
Darren Cohen (dargilco),
Jose Alvarez (jpalvarezl),
sophia-ramsey and
westey (westey-m)
as code owners
October 31, 2026 07:19
lisry-1
temporarily deployed
to
github-app-auth
October 31, 2026 07:20 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
agent-framework-automation
Bot
added
the
python
github-actions
Bot
changed the title
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The regression test omits the empty-string session ID case, so it could pass while leaving part of the missing-session-ID contract unprotected.
1 open issue
What changed in this PR
This PR prevents unreachable conversation history in the Python Cosmos provider by rejecting missing session IDs instead of generating a new random partition key on each call.
Changes:
- Raise
ValueErrorforNoneor empty session IDs. - Update the regression test to verify that
Noneraises without querying Cosmos.
| File | Description |
|---|---|
python/packages/azure-cosmos/tests/test_cosmos_history_provider.py |
Replace the GUID-generation test with a validation-error test. |
python/packages/azure-cosmos/agent_framework_azure_cosmos/_history_provider.py |
Reject empty session IDs and preserve valid partition keys. |
Review effort: Balanced
| ) -> None: | ||
| mock_container.query_items.return_value = _to_async_iter([]) | ||
|
|
||
| async def test_none_session_id_raises_value_error(self, mock_container: MagicMock) -> None: |
Parameterize the missing-session-id test to cover both None and the empty string, matching the non-empty guard in _session_partition_key.
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CosmosHistoryProvider._session_partition_keygenerated a fresh random GUID on every call whensession_idwasNone/empty. Becausesave_messages,get_messages, andcleareach call it independently, saving to an empty session id wrote the data to partition A while later reads/clears targeted partitions B and C. The stored messages became silently unreachable (orphaned) and could never be listed or cleared via that session id.This also diverged from the other providers:
session_id=Nonebehavior"default"ValueErrorChange
Align Cosmos with the Redis scoped and VectorStore providers by raising
ValueErrorfor a missing session id, and update the regression test accordingly.