Consolidating StreamingResponse definitions and slight fixes#327
Consolidating StreamingResponse definitions and slight fixes#327rodrigobr-msft wants to merge 3 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR consolidates duplicated streaming-response helpers (StreamingResponse, Citation, CitationUtil) by removing the framework-specific copies (FastAPI + aiohttp) and centralizing the implementation under hosting-core, with minor behavioral tweaks to the core implementation.
Changes:
- Removed duplicate
streaming/implementations frommicrosoft-agents-hosting-fastapiandmicrosoft-agents-hosting-aiohttp. - Updated
TurnContext.streaming_responseto construct a coreStreamingResponse. - Updated core
StreamingResponsedefaults/behavior (e.g., channel detection and stream-id initialization for some channels).
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| libraries/microsoft-agents-hosting-fastapi/microsoft_agents/hosting/fastapi/app/streaming/streaming_response.py | Removed duplicate StreamingResponse implementation (consolidation). |
| libraries/microsoft-agents-hosting-fastapi/microsoft_agents/hosting/fastapi/app/streaming/citation_util.py | Removed duplicate citation utilities (consolidation). |
| libraries/microsoft-agents-hosting-fastapi/microsoft_agents/hosting/fastapi/app/streaming/citation.py | Removed duplicate Citation model (consolidation). |
| libraries/microsoft-agents-hosting-fastapi/microsoft_agents/hosting/fastapi/app/streaming/init.py | Removed streaming re-export package (consolidation). |
| libraries/microsoft-agents-hosting-fastapi/microsoft_agents/hosting/fastapi/app/init.py | Removed app package re-exports (consolidation). |
| libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/turn_context.py | Switched TurnContext streaming_response wiring to core StreamingResponse (currently broken). |
| libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/streaming/streaming_response.py | Centralized StreamingResponse implementation and adjusted defaults/IDs (currently introduces circular import). |
| libraries/microsoft-agents-hosting-aiohttp/microsoft_agents/hosting/aiohttp/app/streaming/streaming_response.py | Removed duplicate StreamingResponse implementation (consolidation). |
| libraries/microsoft-agents-hosting-aiohttp/microsoft_agents/hosting/aiohttp/app/streaming/citation_util.py | Removed duplicate citation utilities (consolidation). |
| libraries/microsoft-agents-hosting-aiohttp/microsoft_agents/hosting/aiohttp/app/streaming/citation.py | Removed duplicate Citation model (consolidation). |
| libraries/microsoft-agents-hosting-aiohttp/microsoft_agents/hosting/aiohttp/app/streaming/init.py | Removed streaming re-export package (consolidation). |
| libraries/microsoft-agents-hosting-aiohttp/microsoft_agents/hosting/aiohttp/app/init.py | Removed app package re-exports (consolidation). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/turn_context.py
Show resolved
Hide resolved
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/turn_context.py
Outdated
Show resolved
Hide resolved
...rosoft-agents-hosting-core/microsoft_agents/hosting/core/app/streaming/streaming_response.py
Outdated
Show resolved
Hide resolved
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/turn_context.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self._cancelled = True | ||
| else: | ||
| logger.error( | ||
| f"Error occurred when sending activity while streaming: {err}" | ||
| f"Error occurred when sending activity while streaming: {type(err).__name__}" | ||
| ) | ||
| raise | ||
| finally: |
There was a problem hiding this comment.
The comparison on line 328 self._context.activity.channel_id == Channels.ms_teams should be changed to compare the channel property instead: self._context.activity.channel_id.channel == Channels.ms_teams (assuming channel_id is not None). This is inconsistent with the pattern used in _set_defaults (lines 234-238) where the channel property is extracted first. While the current comparison may work because ChannelId inherits from str, it's comparing the full channel ID string rather than just the channel part, which could lead to false negatives if a sub-channel is present.
| import uuid | ||
| import asyncio | ||
| import logging | ||
| from typing import List, Optional, Callable, Literal, TYPE_CHECKING |
There was a problem hiding this comment.
The TYPE_CHECKING import on line 7 is imported but never used. The TurnContext type is only used in string form as a forward reference ("TurnContext" on lines 40 and 232), which doesn't require the TYPE_CHECKING guard. Either remove the unused import or add a proper TYPE_CHECKING guard block if you want to import TurnContext for type checking purposes.
| from typing import List, Optional, Callable, Literal, TYPE_CHECKING | |
| from typing import List, Optional, Callable, Literal |
| import asyncio | ||
| import logging | ||
| from typing import List, Optional, Callable, Literal, TYPE_CHECKING | ||
| from dataclasses import dataclass |
There was a problem hiding this comment.
The dataclass import on line 8 is imported but never used in this file. Consider removing this unused import.
| from dataclasses import dataclass |
This pull request refactors the streaming response logic to improve channel handling and error reporting, and ensures that citation utilities are only used from the core library going forward.
Citation code removal and consolidation:
citation.py,citation_util.py, andstreaming_response.py) and their exports fromaiohttpandfastapihosting libraries, including their__init__.pyfiles. All citation handling is now centralized in thecorelibrary. [1] [2] [3] [4]Streaming response refactor and improvements:
StreamingResponsein thecorelibrary to improve channel detection, set streaming defaults based on channel, and generate stream IDs for webchat and direct line channels. [1] [2]Nonevalues and improved chunk queuing logic. [1] [2] [3]Core library integration:
TurnContextto importStreamingResponsefrom the core library instead of the hosting libraries, ensuring citation utilities are only used from the core.