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.
GitHub Actions
Inactive
GitHub Actions
Inactive
agent-framework-automation
Bot
added
the
python
github-actions
Bot
changed the title
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.
GitHub Actions
Inactive
|
Thanks for the contribution, lisry-1. The code review is clean, but this changes the session-ID behavior contract, so the PR needs the required Related Issue answer before approval. Please link an agreed issue with a closing keyword, or document a maintainer-approved exception. Missing AI Assistance alone is not a blocker. |
GitHub Actions
Inactive
|
Thanks Eduard van Valkenburg (@eavanvalkenburg). I've opened #9975 to track this bug and linked it in the PR's Related Issue section ( |
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:
| Provider |
session_id=Nonebehavior ||---|---|
| File / Redis (legacy) | maps to stable
"default"|| InMemory | stores in session state |
| Redis (scoped) / VectorStore | raises
ValueError|| Cosmos | random GUID every call (orphans data) |
Change
Align Cosmos with the Redis scoped and VectorStore providers by raising
ValueErrorfor a missing session id, and update the regression test accordingly.