Skip to content

Python: await Task and Future sources in ResponseStream - #8331

Open
Sun Haoyuan (Shy7777) wants to merge 1 commit into
microsoft:mainfrom
Shy7777:fix/response-stream-awaitable-sources
Open

Python: await Task and Future sources in ResponseStream#8331
Sun Haoyuan (Shy7777) wants to merge 1 commit into
microsoft:mainfrom
Shy7777:fix/response-stream-awaitable-sources

Conversation

@Shy7777

Copy link
Copy Markdown

Motivation & Context

Scheduling stream initialization with asyncio.create_task() currently breaks ResponseStream, even though its input accepts an awaitable. Task and Future sources are treated as iterators instead of being awaited, so callers get an AttributeError during iteration or RuntimeError: Inner stream not available during finalization.

Description & Review Guide

  • What are the major changes? Use the existing isawaitable() check when resolving a stream source. Extend the constructor and from_awaitable() tests to cover coroutine, Task, and Future inputs.
  • What is the impact of these changes? Pre-scheduled stream initialization now works through the existing API. Async iterable sources still take precedence, and coroutine handling is unchanged. The four new Task/Future cases failed before the fix; all 312 tests in test_types.py pass with it. Package build, package lint, and the repository pre-commit hooks pass, as do all five configured type checkers on the changed test file and strict Pyright on the changed source file. Full-package Pyright is not green in the minimal local environment: optional provider packages are absent. The full provider test suites were not run.
  • What do you want reviewers to focus on? Stream-source resolution and preserving the behavior of values that are both awaitable and async iterable.

Related Issue

Fixes #8330

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@agent-framework-automation agent-framework-automation Bot added the python Usage: [Issues, PRs], Target: Python label Sep 12, 2026
@Shy7777

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@Ricky-7-Yan Ricky-7-Yan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed current head aab491a. Using inspect.isawaitable() is the correct contract-level check here: it includes coroutine objects, asyncio.Task, and Future, while the unchanged __aiter__ branch preserves async-iterable precedence for hybrid sources. The parameterized tests exercise all three awaitable forms through both the constructor and from_awaitable(), including iteration and finalization. I found no blocking issue in the scoped diff.

The repository's full Python workflows have not run on this external branch yet, so this approval is based on the current code/tests plus the author's reported focused 312-test and type-checker runs, not a claim that upstream CI is complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Bug]: ResponseStream rejects Task and Future stream sources

3 participants