Repository navigation
Conversation
Pull request dashboard statusWaiting on the author · refreshed 2026-10-29 21:27 UTC Two things need attention:
Status above doesn't look right?
|
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The localized cleanup changes have focused synchronous and asynchronous regression coverage, with no unresolved blocking findings.
No open issues 🎉
What changed in this PR
This PR fixes early stream cleanup in LangChain’s agent announcement wrappers so closing a stream releases the underlying iterator and finalizes telemetry.
Changes:
- Forward synchronous and asynchronous cleanup while preserving first-step announcement scoping and synchronous generator delegation.
- Add regression tests for cleanup, context restoration, and agent-span finalization.
- Document the fix in a changelog fragment.
| File | Description |
|---|---|
| instrumentation/opentelemetry-instrumentation-genai-langchain/tests/test_stream_context.py | Tests early agent-stream closure and telemetry finalization. |
| instrumentation/opentelemetry-instrumentation-genai-langchain/tests/test_agent_classification_corpus.py | Tests cleanup and announcement withdrawal on closure, errors, and exhaustion. |
| instrumentation/opentelemetry-instrumentation-genai-langchain/src/opentelemetry/instrumentation/genai/langchain/agent_context.py | Forwards stream cleanup while preserving announcement scope and delegation. |
| instrumentation/opentelemetry-instrumentation-genai-langchain/.changelog/953.fixed | Records the stream-cleanup fix. |
Review effort: Balanced
lmolkova
reviewed
Oct 22, 2026
Comment on lines
+629
to
+632
| assert agent_span.status.status_code == StatusCode.ERROR | ||
| assert ( | ||
| agent_span.attributes[error_attributes.ERROR_TYPE] == "GeneratorExit" | ||
| ) |
Member
There was a problem hiding this comment.
An early close by the caller isn't an error, so the agent span shouldn't get error.type=GeneratorExit or an error status. To match OpenAI chat today, could we set gen_ai.response.finish_reasons=["error"] instead? We'll revisit the value once semconv decides, tracked in #963.
Comment on lines
+161
to
+178
| delegating = False | ||
| try: | ||
| first = next(iterator) | ||
| except StopIteration: | ||
| return | ||
| entry = _push(name) | ||
| try: | ||
| first = next(iterator) | ||
| except StopIteration: | ||
| return | ||
| finally: | ||
| _pop(entry) | ||
| yield first | ||
| # yield from forwards close itself once delegation has started. | ||
| delegating = True | ||
| yield from iterator | ||
| finally: | ||
| _pop(entry) | ||
| yield first | ||
| yield from iterator | ||
| if not delegating: | ||
| close = getattr(iterator, "close", None) | ||
| if close is not None: | ||
| close() |
Member
There was a problem hiding this comment.
nit: Pregel.stream is typed as Iterator, but it's a generator. Narrowing with isinstance avoids getattr and the delegating flag (close() on a finished generator is a no-op). Needs Generator from collections.abc.
Suggested change
| delegating = False | |
| try: | |
| first = next(iterator) | |
| except StopIteration: | |
| return | |
| entry = _push(name) | |
| try: | |
| first = next(iterator) | |
| except StopIteration: | |
| return | |
| finally: | |
| _pop(entry) | |
| yield first | |
| # yield from forwards close itself once delegation has started. | |
| delegating = True | |
| yield from iterator | |
| finally: | |
| _pop(entry) | |
| yield first | |
| yield from iterator | |
| if not delegating: | |
| close = getattr(iterator, "close", None) | |
| if close is not None: | |
| close() | |
| try: | |
| entry = _push(name) | |
| try: | |
| first = next(iterator) | |
| except StopIteration: | |
| return | |
| finally: | |
| _pop(entry) | |
| yield first | |
| yield from iterator | |
| finally: | |
| if isinstance(iterator, Generator): | |
| iterator.close() |
Comment on lines
+197
to
+200
| finally: | ||
| _pop(entry) | ||
| yield first | ||
| async for chunk in iterator: | ||
| yield chunk | ||
| close = getattr(iterator, "aclose", None) | ||
| if close is not None: | ||
| await close() |
Member
There was a problem hiding this comment.
nit: same here, needs AsyncGenerator from collections.abc:
Suggested change
| finally: | |
| _pop(entry) | |
| yield first | |
| async for chunk in iterator: | |
| yield chunk | |
| close = getattr(iterator, "aclose", None) | |
| if close is not None: | |
| await close() | |
| finally: | |
| if isinstance(iterator, AsyncGenerator): | |
| await iterator.aclose() |
rads-1996
force-pushed
the
langchain-agent-stream-cleanup
branch
from
October 23, 2026 15:36
6620c4b to
1b14b4e
Compare
|
Hi @rads-1996 — just a friendly reminder that this pull request is waiting on you. The dashboard status comment has the open items and is kept current.
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Forward
close()andaclose()through LangChain’s agent announcement wrappers so early stream closure releases the underlying stream and finalizes telemetry. Preserve first-step announcement scoping and synchronous generator delegation.Type of change
Please delete options that are not relevant.
How has this been tested?
Please describe the tests that you ran to verify your changes. Provide
instructions so we can reproduce. List any relevant details for your test
configuration.
Checklist
See CONTRIBUTING.md
for the style guide, changelog guidance, and more.