Skip to content

Require closable async iterators - #473

Merged
elimoss merged 3 commits into
mainfrom
closable-async-generators
Jul 21, 2026
Merged

Require closable async iterators#473
elimoss merged 3 commits into
mainfrom
closable-async-generators

Conversation

@elimoss

@elimoss elimoss commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

In a few places we are passing around async iterators that may or may not implement aclose. tighten this to specify that they do and add unconditional call to aclose after exit or cancellation. this allows more reliable stream interruption handling and lifecycle management in preparation for #471

Copilot AI review requested due to automatic review settings July 20, 2026 17:11

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.

Pull request overview

This PR tightens streaming interfaces in lmi to require closable async generators and adds explicit aclose() calls so upstream provider streams can be interrupted and cleaned up reliably (in preparation for PR #471’s streaming behavior).

Changes:

  • Narrow streaming type contracts from generic AsyncIterable/AsyncIterator to AsyncGenerator where closure is required.
  • Ensure committed/limited streaming wrappers close their upstream generators via aclose().
  • Add tests asserting upstream streams are closed when wrappers are closed.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
packages/lmi/src/lmi/llms.py Tightens streaming return types to AsyncGenerator and adds upstream aclose() in committed and limiter wrappers.
packages/lmi/src/lmi/cost_tracker.py Adds TrackedStreamWrapper.aclose() delegating to the wrapped provider stream.
packages/lmi/tests/test_dispatch.py Updates generator typing and adds a regression test ensuring committed streams close their source.
packages/lmi/tests/test_cost_tracking.py Adds a test ensuring TrackedStreamWrapper.aclose() closes the wrapped stream.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/lmi/src/lmi/llms.py
Comment thread packages/lmi/src/lmi/llms.py Outdated
Comment thread packages/lmi/src/lmi/llms.py
Comment thread packages/lmi/src/lmi/llms.py
@elimoss
elimoss requested a review from sidnarayanan July 20, 2026 17:26
Comment thread packages/lmi/src/lmi/llms.py Outdated
@elimoss
elimoss force-pushed the closable-async-generators branch from 1a90dfd to e2195da Compare July 21, 2026 22:24
@elimoss elimoss changed the title Require closable async generators Require closable async iterators Jul 21, 2026
@elimoss
elimoss merged commit e887332 into main Jul 21, 2026
7 checks passed
@elimoss
elimoss deleted the closable-async-generators branch July 21, 2026 23:57
Eddie-MG added a commit that referenced this pull request Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants