Fix: langchain async and sync batch calls #518
Closed
Conversation
This file contains 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
* test: Add test cases for langchain syn and async methods * fix: move the assertion on streaming outside exception block * test: add ainvoke test on llm model * test: removed streaming=true from non-streaming cases * test: adding the streaming parameter in cases where it makes sense
…:noble-varghese/langfuse-python into noble-varghese/langchain-streaming-tests
…:langfuse/langfuse-python into noble-varghese/langchain-streaming-tests
…:noble-varghese/langfuse-python into noble-varghese/langchain-streaming-tests
Once the runs are generated in the current scenario when the callback is declared as a single declaration and re-used, then the runs basically starts collecting the run traces indefinitely untill it goes OOM. To avoid this situtaion we remove the run details after the end of the chain and at the end of the run invocation itself. This ensures the details are persistent till the end of the loop and is removed once the traces are pushed to langfuse.
maxdeichmann
reviewed
Apr 3, 2024
| @@ -743,6 +743,9 @@ def _update_trace(self, run_id: str, parent_run_id: Optional[str], output: any): | |||
| ): | |||
| self.trace = self.trace.update(output=output) | |||
|
|
|||
| # Remove the run details after updating the trace. | |||
There was a problem hiding this comment.
Can we quickly rename the function? E.g. _update_trace_and_remove_state(...) to make this more readable?
There was a problem hiding this comment.
If the tests complete, i think we are good to go here. Can you add assertions to e.g. https://github.com/noble-varghese/langfuse-python/blob/c1aee1422c48c7cabb4fc4febf7407fca11ccfa4/tests/test_langchain.py#L886 to check that the runs map is empty?
There was a problem hiding this comment.
Fixed on Issue #527 and added assertions as well.
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
Test Langchain's synchronous, asynchronous streaming, batch, and invoke methods, and resolve the asynchronous issue encountered with Langchain batch calls.
Key Modifications: