Python: Reduce Azure chat client import overhead#4744
Python: Reduce Azure chat client import overhead#4744eavanvalkenburg merged 2 commits intomicrosoft:mainfrom
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull request overview
Reduces Python import-time overhead for the Azure OpenAI chat client by avoiding eager imports of heavier OpenAI runtime types and the broader agent_framework.openai package surface, while keeping runtime parsing behavior consistent.
Changes:
- Move
AsyncAzureOpenAIimport behindTYPE_CHECKINGto avoid import-time cost. - Stop eagerly importing OpenAI
Choice/ChunkChoiceruntime types and update_parse_text_from_openaito use attribute-based access (getattr). - Import
OpenAIChatOptionsdirectly fromagent_framework.openai._chat_clientrather than the heavieragent_framework.openainamespace.
You can also share your feedback on Copilot code review. Take the survey.
eavanvalkenburg
left a comment
There was a problem hiding this comment.
Automated Code Review
Reviewers: 3 | Confidence: 89%
✓ Correctness
The diff makes two changes: (1) moves openai type imports behind TYPE_CHECKING or removes them, and (2) replaces the isinstance-based dispatch in
_parse_text_from_openaiwith a getattr-based duck-typing approach. Both changes are logically correct. Thefrom __future__ import annotationsimport ensuresAsyncAzureOpenAIcan safely move to TYPE_CHECKING. The getattr fallback frommessagetodeltacorrectly mirrors the original isinstance logic sinceChoicehasmessageandChunkChoicehasdelta(and not the other). The re-export path change forOpenAIChatOptionsis valid as it's defined in_chat_client.pyand also re-exported from__init__.py. No correctness issues found.
✗ Test Coverage
The diff changes
_parse_text_from_openaiinAzureOpenAIChatClientfrom isinstance-based dispatch to duck-typing viagetattr. Existing tests cover the non-streaming path (viatest_azure_on_your_data*) and streaming withNonedelta (viatest_streaming_with_none_delta), but all exercise the method indirectly through the fullget_responseflow. There is no direct unit test for the Azure override of_parse_text_from_openai, and notably the refusal branch inside this override has zero test coverage—the existingtest_parse_text_with_refusalonly tests the baseRawOpenAIChatClient. Adding targeted tests for the Azure-specific method would catch any regressions from the isinstance-to-getattr refactor.
✗ Design Approach
The diff makes three changes: (1) moves
AsyncAzureOpenAIto theTYPE_CHECKINGblock (correct), (2) consolidatesOpenAIChatOptionsimport from the publicagent_framework.openaipackage into the privateagent_framework.openai._chat_clientmodule, and (3) replaces the typedchoice: Choice | ChunkChoiceoverride signature withchoice: Anyand substitutesisinstance(choice, Choice)discrimination withgetattrduck-typing. Thegetattrapproach at runtime is fine given the imports are being removed, but widening the signature toAnyis unnecessary and the wrong tradeoff: since the file already hasfrom __future__ import annotations(PEP 563), addingChoiceandChunkChoiceto theTYPE_CHECKINGblock would let the type annotation remain fully typed without any runtime import cost. The import ofOpenAIChatOptionsfrom a private module (_chat_client) instead of the public package API is also a leaky abstraction, though less severe.
Flagged Issues
- The Azure-specific
_parse_text_from_openaioverride changed dispatch logic (isinstance → getattr fallback) but has no direct unit test. The refusal branch (if hasattr(message, 'refusal') and message.refusal) in this override is completely untested. A regression in the getattr fallback (e.g., a ChunkChoice that unexpectedly has amessageattribute) or the refusal path would go undetected. - The override signature
_parse_text_from_openai(self, choice: Any)unnecessarily discards the type contract established by the base class (Choice | ChunkChoice). Becausefrom __future__ import annotationsis already present in this file, annotations are never evaluated at runtime —ChoiceandChunkChoiceonly need to be added to theTYPE_CHECKINGblock (not imported unconditionally) to keep the typed signature. UsingAnyhere silences the type checker and breaks the override contract without any actual benefit.
Suggestions
- Add a direct unit test for
AzureOpenAIChatClient._parse_text_from_openaicovering: (1) a Choice-like object with onlymessage, (2) a ChunkChoice-like object with onlydelta, (3) an object with bothmessage=Noneanddelta(verifying fallback), (4) the refusal path, and (5) an object with neither attribute (returns None). OpenAIChatOptionsis part of the public API ofagent_framework.openai(listed in__all__). The diff changes its import to come from the internalagent_framework.openai._chat_clientmodule, bypassing the public surface. Import it from the public package (from agent_framework.openai import OpenAIChatOptions) to avoid coupling to implementation internals.
Automated review by eavanvalkenburg's agents
…i tests - Move Choice and ChunkChoice imports under TYPE_CHECKING to avoid runtime import cost (from __future__ annotations is already present) - Restore proper typed signature (Choice | ChunkChoice) instead of Any - Add direct unit tests for _parse_text_from_openai covering: - Choice with message content - ChunkChoice with delta content - Refusal branch for both Choice and ChunkChoice - No content/no refusal returning None - None delta (async content filtering) returning None Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Motivation and Context
Latency benchmarking on current
mainshowedimport_azure_openai_chat_clientregressed relative to the latest comparable historical baseline. Profiling pointed atagent_framework.azure._chat_clienteagerly importing heavier OpenAI runtime modules and top-level package surfaces during module import. This change narrows that import path and helps address the latency work tracked in #2752.Description
AsyncAzureOpenAIbehindTYPE_CHECKINGso it is not imported at module load timeChoiceandChunkChoiceruntime typesOpenAIChatOptionsdirectly fromagent_framework.openai._chat_clientinstead of the heavieragent_framework.openaipackage surface_parse_text_from_openaito readmessageanddeltaviagetattr, preserving the existing parsing behavior without those runtime type importsThis keeps the change isolated to import-time behavior and avoids changing the public Azure chat client API surface. The change was exercised with the non-integration Azure chat client test suite.
Contribution Checklist