Python: [BREAKING] Scope Redis history storage keys - #8236
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
The breaking persisted-key format and migration implications warrant final human review despite strong unit coverage.
Pull request overview
Introduces scoped Redis conversation-history keys to isolate applications, tenants, agents, providers, and sessions while preserving explicit legacy compatibility.
Changes:
- Adds deterministic scoped keys and validation.
- Retains legacy key formatting for migrations.
- Updates tests, documentation, and samples.
File summaries
| File | Description |
|---|---|
python/packages/redis/agent_framework_redis/_history_provider.py |
Implements scoped and legacy keys. |
python/packages/redis/tests/test_providers.py |
Tests scoping, validation, and compatibility. |
python/packages/redis/README.md |
Documents isolation and migration. |
python/packages/redis/AGENTS.md |
Updates package guidance. |
python/samples/02-agents/conversations/redis_history_provider.py |
Adds stable scope identifiers. |
python/samples/02-agents/context_providers/redis/azure_redis_conversation.py |
Updates Azure Redis configuration. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: No findings
Scope: full PR (1 commit(s)): 223503a20fc2
Model: gpt-5.6-sol-fast
Overview
The PR introduces versioned, independently encoded Redis history boundaries while retaining the exact historical key format behind explicit legacy mode. Constructor and operation guards reject missing scoped identifiers, tests cover boundary isolation and legacy compatibility, and all changed production callers provide stable application and session identities. No PR-introduced Critical, High, or Medium defect remained after reconciliation.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.
223503a to
edbe8a2
Compare
Jose Alvarez (jpalvarezl)
left a comment
There was a problem hiding this comment.
Looks good! Just left a couple of questions and flagged something I might be misunderstanding.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
edbe8a2 to
db4ec85
Compare
Motivation & Context
Redis-backed conversation history needs stable storage boundaries across applications, providers, tenants, agents, and sessions, while existing deployments need an explicit compatibility path for historical keys.
Description & Review Guide
Related Issue
Related to #7471
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and the title prefix in sync automatically.