Fixes #3544 based on the data-type of message

[paneru-rajan]

Fixes the error while ollama_chat use langfuse

The value of response_obj["choices"][0]["message"] is Message object and dict

Added a conditional to use .json only iff it is Message Object

Relevant issues

Fixes #3544

Type

🐛 Bug Fix

Testing

Error Replicates with

litellm.success_callback = ["langfuse"]
resp = completion(
    model="ollama_chat/llama3:instruct",
    messages=[
        {"role": "user", "content": "Hello"},
    ],
    base_url='http://localhost:11434'
)

print(resp["choices"][0]["message"])

After fix it will work for both: ollama_chat/llama3:instruct and ollama/llama3:instruct

[vercel]

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

Name	Status	Preview	Comments	Updated (UTC)
litellm	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	May 10, 2024 0:45am

[krrishdholakia]

we should assume the response will be of type ModelResponse and it's subsequent pydantic objects - e.g. Messages. (since that's the type we enforce on .completion())

The correct fix here is to fix the ollama file to return the correct pydantic object.

@paneru-rajan let me know if you want to close this and open another pr, or rework the existing pr to fix the actual issue.

[paneru-rajan]

we should assume the response will be of type ModelResponse and it's subsequent pydantic objects - e.g. Messages. (since that's the type we enforce on .completion())

The correct fix here is to fix the ollama file to return the correct pydantic object.

@paneru-rajan let me know if you want to close this and open another pr, or rework the existing pr to fix the actual issue.

Thank you, I update this PR with the proper fix, that commit was just a patch so it makes sense not to go that route.

[paneru-rajan]

Updated: Preserving the Pydantic Message Object

model_response["choices"][0]["message"] = response_json["message"]
This statement replaces the Pydantic Message Object and initialize it with the dict

We need to make sure message is always litellm.Message object

As a fix, based on the code of ollama.py file, i've just updated the content instead of entire object for both sync and async functions

Relevant issues

Fixes #3544

Type

🐛 Bug Fix

Testing

• Checked completion with ollama_chat/llama3:instruct
• Checked acompletion with ollama_chat/llama3:instruct

Looks good to me, handing over to you @krrishdholakia for review.

[Viktor2k]

@paneru-rajan, do you mind assigning @krrishdholakia as the reviewer? Would be so nice if we could get this through🤞🏼

[krrishdholakia]

this doesn't fix the problem of the message being a dict object instead of pydantic.

@paneru-rajan can you share a test you wrote to see if this fixed the problem, and that it's passing?

[paneru-rajan]

I tested, model_response["choices"][0]["message"] is a pydantic Message Object.

As before we were replacing it with dict, which is what was the issue. Since i am just assigning the content which is a string, which does not override the existing Message Object.

[paneru-rajan]

Following image shows assert doesn't raise an exception.

[paneru-rajan]

@paneru-rajan, do you mind assigning @krrishdholakia as the reviewer? Would be so nice if we could get this through🤞🏼

It seemed Github does not allow us to add reviewer. I tried adding @krrishdholakia as reviewer and then left the comment instead because of that.

[krrishdholakia]

LGTM! thanks for sharing the screenshot @paneru-rajan that helped