fix llamaindex CompletionResponse serialize error#906
Closed
nightosong wants to merge 1 commit into
Closed
Conversation
Contributor
There was a problem hiding this comment.
Disclaimer: Experimental PR review
PR Summary
This pull request addresses a serialization issue with LlamaIndex's CompletionResponse object in the EventSerializer class, resolving an 'Invalid request data' error when using langfuse with llamaindex.
- Modified
EventSerializer.default()inlangfuse/serializer.pyto handle CompletionResponse objects by converting them to strings - Added "CompletionResponse" to the condition checking for LlamaIndex objects that require special handling
- This change ensures proper serialization of CompletionResponse objects, preventing TypeError exceptions
- The fix maintains consistency with how StreamingResponse objects are already handled in the serializer
- Users working with LlamaIndex and langfuse should no longer encounter the 'Invalid request data' error related to CompletionResponse serialization
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
Contributor
There was a problem hiding this comment.
Disclaimer: Experimental PR review
PR Summary
This pull request addresses a serialization issue with LlamaIndex's CompletionResponse object in the EventSerializer class of the langfuse-python library.
- Modified
EventSerializer.default()to handle CompletionResponse objects by converting them to strings - The fix resolves the 'Invalid request data' error when using langfuse with llamaindex
- While effective, this solution may not be optimal long-term as it loses the structured data of CompletionResponse objects
- Consider exploring a more robust serialization method that preserves CompletionResponse structure in future updates
- Recommend adding tests in
/tests/test_llama_index.pyto verify the serialization of CompletionResponse objects
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
3 tasks
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.
While I use langfuse with llamaindex, I get an error output:
Then I debug the task_manager file and find this code
data = json.dumps(kwargs, cls=EventSerializer).In my case, the output object in body is type of
<class 'llama_index.core.base.llms.types.CompletionResponse'>.When it executes this line of
default()in EventSerializer, a TypeError will be thrown.TypeError:
'MockValSer' object cannot be converted to 'SchemaSerializer'(pydantic 2.8.2)I just realized that this issue is caused by the type from
llamaindex.Would there be any risks if I handle all of the CompletionResponse here?