Python: [BREAKING] update context provider APIs, middleware, and per-service-call history persistence#4992
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Python Agent Framework context-provider surface area by renaming the public provider base classes, enabling context providers to add invocation-scoped middleware, and adding an opt-in mode to simulate service-stored history semantics per model call (to better match service-managed storage behavior).
Changes:
- Rename provider base APIs to
ContextProvider/HistoryProvider, keepingBaseContextProvider/BaseHistoryProvideras deprecated aliases. - Allow providers to add invocation-scoped chat/function middleware via
SessionContext, and forward that middleware into downstream execution (while rejecting provider-added agent middleware). - Add
simulate_service_stored_historysupport and implement per-model-call history-provider simulation, including orchestration handoff clone preservation and expanded test coverage.
Reviewed changes
Copilot reviewed 40 out of 40 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| python/samples/05-end-to-end/hosted_agents/README.md | Updates sample docs to reference ContextProvider. |
| python/samples/05-end-to-end/hosted_agents/agent_with_text_search_rag/main.py | Switches sample provider inheritance/imports to ContextProvider. |
| python/samples/02-agents/conversations/custom_history_provider.py | Switches sample history provider inheritance/imports to HistoryProvider. |
| python/samples/02-agents/context_providers/simple_context_provider.py | Switches sample provider inheritance/imports to ContextProvider. |
| python/samples/02-agents/chat_client/simulate_service_stored_history.py | Adds a new sample demonstrating simulated service-stored history behavior. |
| python/samples/02-agents/chat_client/README.md | Documents the new history-simulation sample and prerequisites. |
| python/samples/01-get-started/04_memory.py | Updates get-started memory sample to ContextProvider. |
| python/packages/redis/tests/test_providers.py | Updates Redis provider tests to reference HistoryProvider naming. |
| python/packages/redis/agent_framework_redis/_history_provider.py | Renames Redis history provider base class usage to HistoryProvider. |
| python/packages/redis/agent_framework_redis/_context_provider.py | Renames Redis context provider base class usage to ContextProvider. |
| python/packages/orchestrations/tests/test_handoff.py | Adds coverage for simulated history behavior preservation in handoff clones; updates provider naming. |
| python/packages/orchestrations/agent_framework_orchestrations/_handoff.py | Preserves simulate_service_stored_history when cloning agents; cleans up middleware import. |
| python/packages/openai/agent_framework_openai/_assistant_provider.py | Updates assistant provider API typing to ContextProvider. |
| python/packages/mem0/agent_framework_mem0/_context_provider.py | Updates Mem0 provider to ContextProvider. |
| python/packages/github_copilot/agent_framework_github_copilot/_agent.py | Updates agent wrapper typing to ContextProvider. |
| python/packages/foundry/agent_framework_foundry/_memory_provider.py | Updates Foundry memory provider to ContextProvider. |
| python/packages/foundry/agent_framework_foundry/_agent.py | Updates Foundry agent APIs to accept ContextProvider. |
| python/packages/core/tests/core/test_sessions.py | Adds tests for provider-added middleware and deprecated alias behavior; updates naming. |
| python/packages/core/tests/core/test_middleware_with_agent.py | Adds tests for provider-added middleware forwarding and rejection of provider-added agent middleware. |
| python/packages/core/tests/core/test_agents.py | Adds tests for per-call history simulation + response-id behavior; updates naming. |
| python/packages/core/AGENTS.md | Updates documentation to ContextProvider / HistoryProvider. |
| python/packages/core/agent_framework/observability.py | Refactors tracing wrapper to share logic via _trace_agent_invocation while preserving overloads. |
| python/packages/core/agent_framework/_workflows/_agent.py | Updates workflow agent typing/checks to ContextProvider / HistoryProvider. |
| python/packages/core/agent_framework/_tools.py | Clears simulated conversation id sentinel before returning final responses. |
| python/packages/core/agent_framework/_skills.py | Updates SkillsProvider base class to ContextProvider. |
| python/packages/core/agent_framework/_sessions.py | Introduces ContextProvider/HistoryProvider, provider-added middleware support, and history-simulation chat middleware + deprecated aliases. |
| python/packages/core/agent_framework/_compaction.py | Updates CompactionProvider base class to ContextProvider. |
| python/packages/core/agent_framework/_clients.py | Adds simulate_service_stored_history option to BaseChatClient.as_agent(...). |
| python/packages/core/agent_framework/_agents.py | Implements provider-added middleware forwarding and per-model-call history simulation wiring + response-id suppression behavior. |
| python/packages/core/agent_framework/init.py | Re-exports ContextProvider and HistoryProvider (and keeps deprecated aliases). |
| python/packages/copilotstudio/agent_framework_copilotstudio/_agent.py | Updates agent wrapper typing to ContextProvider. |
| python/packages/claude/agent_framework_claude/_agent.py | Updates agent wrapper typing to ContextProvider. |
| python/packages/azure-cosmos/agent_framework_azure_cosmos/_history_provider.py | Updates Cosmos history provider base class to HistoryProvider. |
| python/packages/azure-ai/agent_framework_azure_ai/_project_provider.py | Updates Azure AI project provider APIs to ContextProvider. |
| python/packages/azure-ai/agent_framework_azure_ai/_client.py | Updates Azure AI client as_agent typing to ContextProvider. |
| python/packages/azure-ai/agent_framework_azure_ai/_chat_client.py | Updates Azure AI chat client as_agent typing to ContextProvider. |
| python/packages/azure-ai/agent_framework_azure_ai/_agent_provider.py | Updates Azure AI agent provider APIs to ContextProvider. |
| python/packages/azure-ai-search/agent_framework_azure_ai_search/_context_provider.py | Updates Azure AI Search provider base class to ContextProvider. |
| python/packages/a2a/tests/test_a2a_agent.py | Updates A2A tests to use ContextProvider. |
| python/packages/a2a/agent_framework_a2a/_agent.py | Updates A2A agent history-provider type checks to HistoryProvider. |
TaoChenOSU
left a comment
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 76%
✓ Correctness
✗ Security Reliability
This PR renames BaseContextProvider→ContextProvider and BaseHistoryProvider→HistoryProvider (with deprecated aliases), introduces per-service-call history persistence via a new PerServiceCalHistoryPersistingMiddleware, and allows context providers to inject chat/function middleware. The major security/reliability concern is a class name typo ('PerServiceCalHistoryPersistingMiddleware' instead of 'PerServiceCallHistoryPersistingMiddleware') that appears in both the class definition and public API surface. There is also a reliability issue in the streaming path of the middleware where with_result_hook is called without reassigning to context.result—while with_result_hook mutates in-place, the lambda closure captures service_call_context by reference in a way that may cause issues with concurrent streaming. Additionally, the deprecated BaseContextProvider/BaseHistoryProvider aliases use
@deprecatedwhich emits warnings on class definition (Python 3.13+) or instantiation (typing_extensions), but the init.py import of these names with type: ignore suppression could mask other issues.
✗ Test Coverage
This PR introduces the
require_per_service_call_history_persistencefeature withPerServiceCalHistoryPersistingMiddleware, renamesBaseContextProvider/BaseHistoryProvidertoContextProvider/HistoryProvider(with deprecated aliases), addsSessionContext.extend_middleware()/get_middleware(), and refactors Agent.run() internals. Test coverage for the new feature is good at the integration level (non-streaming, streaming, service-stores-by-default bypass, response_id suppression), but several important units and edge cases lack tests: thePerServiceCalHistoryPersistingMiddlewareclass has no direct unit tests, the helper functions_split_service_call_messages,_response_contains_follow_up_request,is_local_history_conversation_id, and_clear_local_history_conversation_idhave no direct tests, and the_resolve_per_service_call_history_providersmethod's error path (raisingAgentInvalidRequestExceptionwhenconversation_idis set) is exercised only indirectly viatest_per_service_call_persistence_rejects_real_service_conversation_idwhich actually tests a different error (ChatClientInvalidResponseException from the middleware, not AgentInvalidRequestException from _resolve). There is also a typo in the class name (PerServiceCalHistoryPersistingMiddleware— missing second 'l' in 'Call').
✗ Design Approach
The PR successfully renames BaseContextProvider/BaseHistoryProvider to ContextProvider/HistoryProvider with proper deprecation shims, and the observability refactoring is clean. The main design concern is the
require_per_service_call_history_persistencefeature's sentinel mechanism:LOCAL_HISTORY_CONVERSATION_IDis injected intoresponse.conversation_idbyPerServiceCalHistoryPersistingMiddleware, then must be detected and cleared by_tools.py's function loop and filtered in three separate places in_agents.py. Overloading a semantic public field (conversation_id) as an inter-layer control signal creates a hidden cross-cutting contract that is fragile and hard to maintain — a dedicated out-of-band flag onChatResponse(e.g.local_continuation: bool) would isolate the concern to the middleware layer without leaking into the function invocation loop. There is also a typo in the middleware class name ('Cal' vs 'Call') and agetattr-based coupling fromBaseAgent._run_after_providersto a subclass attribute.
Flagged Issues
- Class name typo: 'PerServiceCalHistoryPersistingMiddleware' should be 'PerServiceCallHistoryPersistingMiddleware' (missing 'l' in 'Call'). This typo propagates through the public API surface—class definition in _sessions.py, import in _agents.py, and init.py exports. Once released as stable API, the misspelling becomes a permanent breaking-change liability.
- LOCAL_HISTORY_CONVERSATION_ID sentinel overloads response.conversation_id as an inter-layer control signal. The middleware writes it, _tools.py must clear it at two function-loop exit points, and _agents.py must filter it in _finalize_response, _post_hook, and _propagate_conversation_id. Any future code touching conversation_id must also know about this sentinel. A dedicated flag on ChatResponse (e.g.
use_local_continuation: bool = False) would confine the concern to the middleware layer without bleeding into the function invocation loop. - PerServiceCalHistoryPersistingMiddleware has no unit tests—its process(), _prepare_service_call_context(), _persist_service_call_response(), _strip_local_conversation_id(), and _finalize_response() methods are only exercised indirectly through high-level Agent.run() integration tests, making it hard to verify edge-case correctness (e.g., streaming finalization path, ValueError branches for wrong result types, real conversation_id rejection).
- The _resolve_per_service_call_history_providers error path that raises AgentInvalidRequestException when conversation_id is set while require_per_service_call_history_persistence is True has no test. The existing test_per_service_call_persistence_rejects_real_service_conversation_id tests a different code path (ChatClientInvalidResponseException from the middleware, not AgentInvalidRequestException from _resolve).
Suggestions
- Consider adding a guard in _prepare_service_call_context to handle the case where self._session is None or has been invalidated, since the middleware stores a reference to the session at construction time but the session could theoretically be replaced between middleware instantiation and execution.
- The _strip_local_conversation_id method creates a new dict from context.options on every call even when no mutation is needed. Consider checking before copying.
- Add unit tests for the helper functions is_local_history_conversation_id, _response_contains_follow_up_request, _split_service_call_messages, and _clear_local_history_conversation_id—these are pure functions with simple contracts that are easy to test and important to the feature's correctness.
- Add a test that verifies the deprecated BaseHistoryProvider alias emits DeprecationWarning on instantiation. The diff adds test_base_context_provider_warns_and_is_compatible for BaseContextProvider, but BaseHistoryProvider's warning is not tested (test_base_provider_aliases_preserve_subtyping only checks isubclass).
- BaseAgent._run_after_providers uses
getattr(self, 'require_per_service_call_history_persistence', False)to access an attribute defined only on Agent/ChatAgent. Either move the attribute to BaseAgent with a default of False, or extract the skip logic into a protected hook that subclasses can override. - _split_service_call_messages relies on the
_attributionkey in message.additional_properties as an implicit contract for separating context messages from input messages. This convention is load-bearing across modules and should at minimum be defined as a named constant.
Automated review by TaoChenOSU's agents
e6c099e to
34b4468
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
c3e4a27 to
e2fe01c
Compare
Motivation and Context
This PR bundles the three related Python context-provider updates that were planned and implemented together:
ContextProvider/HistoryProvidernaming)The history-persistence work follows the related .NET design work in #4762 and #4974, plus the follow-up rename/reframing in #4993, while adapting it to Python's agent/runtime shape.
Closes #4970.
Description
ContextProviderandHistoryProvider, while keepingBaseContextProviderandBaseHistoryProvideras deprecated compatibility aliases for now.ChatMiddlewareandFunctionMiddlewarethroughSessionContext, forward that middleware into downstream client/tool execution, and explicitly reject provider-addedAgentMiddleware.require_per_service_call_history_persistencetoAgent,BaseChatClient.as_agent(...), and Foundry wrappers, and treat that rename fromsimulate_service_stored_historyas a breaking public API change.storebehavior.samples/02-agents/chat_client/require_per_service_call_history_persistence.pyand update its README so it demonstrates the key termination edge: without per-service-call persistence the synthesized tool result is written to local history, while with per-service-call persistence it is not, matching service-side storage behavior.Contribution Checklist