Skip to content

core[patch]: revert change to stream type hint #31501

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 5, 2025
Merged

Conversation

ccurme
Copy link
Collaborator

@ccurme ccurme commented Jun 5, 2025

#31286 included an update to the return type for BaseChatModel.(a)stream, from Iterator[BaseMessageChunk] to Iterator[BaseMessage].

This change is correct, because when streaming is disabled, the stream methods return an iterator of BaseMessage, and the inheritance is such that an BaseMessage is not a BaseMessageChunk (but the reverse is true).

However, LangChain includes a pattern throughout its docs of summing BaseMessageChunks to accumulate a chat model stream. This pattern is implemented in tests for most integration packages and appears in application code. So #31286 introduces mypy errors throughout the ecosystem (or maybe more accurately, it reveals that this pattern does not account for use of the .stream method when streaming is disabled).

Here we revert just the change to the stream return type to unblock things. A fix for this should address docs + integration packages (or if we elect to just force people to update code, be explicit about that).

@ccurme ccurme requested a review from eyurtsev June 5, 2025 15:12
Copy link

vercel bot commented Jun 5, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Jun 5, 2025 3:13pm

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. Ɑ: core Related to langchain-core 🤖:nit Small modifications/deletions, fixes, deps or improvements to existing code or docs labels Jun 5, 2025
Copy link

codspeed-hq bot commented Jun 5, 2025

CodSpeed Walltime Performance Report

Merging #31501 will not alter performance

Comparing cc/stream_types (a5c7a4d) with master (b149cce)

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

Summary

✅ 13 untouched benchmarks

@dosubot dosubot bot added the lgtm PR looks good. Use to confirm that a PR is ready for merging. label Jun 5, 2025
Copy link

codspeed-hq bot commented Jun 5, 2025

CodSpeed Instrumentation Performance Report

Merging #31501 will not alter performance

Comparing cc/stream_types (a5c7a4d) with master (b149cce)

Summary

✅ 13 untouched benchmarks

@ccurme ccurme merged commit 741bb1f into master Jun 5, 2025
65 checks passed
@ccurme ccurme deleted the cc/stream_types branch June 5, 2025 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ɑ: core Related to langchain-core lgtm PR looks good. Use to confirm that a PR is ready for merging. 🤖:nit Small modifications/deletions, fixes, deps or improvements to existing code or docs size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants