Skip to content
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

fix: Add support for litellm Message type #1308

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nate-mar
Copy link
Contributor

@nate-mar nate-mar commented Feb 25, 2025

Add support for litellm Message type

@nate-mar nate-mar requested a review from a team as a code owner February 25, 2025 03:55
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Feb 25, 2025
@nate-mar nate-mar changed the title feat: Add support for litellm Message type fix: Add support for litellm Message type Feb 25, 2025
@nate-mar nate-mar marked this pull request as draft February 25, 2025 04:01
@nate-mar nate-mar force-pushed the handle-litellm-message-type-for-input-message branch from 7746e54 to 8d59d77 Compare February 25, 2025 04:45
@nate-mar nate-mar marked this pull request as ready for review February 25, 2025 04:48
@nate-mar nate-mar force-pushed the handle-litellm-message-type-for-input-message branch from 8d59d77 to 7efda1e Compare February 25, 2025 04:52
Add support for litellm Message type
@nate-mar nate-mar force-pushed the handle-litellm-message-type-for-input-message branch from 7efda1e to 208ef0f Compare February 25, 2025 05:12
@mikeldking
Copy link
Contributor

resolves #1303

Comment on lines +32 to +34
from litellm.types.utils import (
Message as LitellmMessage,
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why split the import here? it seems identical to the import above?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, you're right.

Comment on lines +137 to +138
if messages_as_dicts:
_set_span_attribute(span, SpanAttributes.INPUT_VALUE, json.dumps(messages_as_dicts))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not blocking but this makes the input a duplicate of the messages - bloating the payload. I sorta question whether we should continue to do this...

In any case if set to a JSON dumps I would set the MIME_TYPE here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it does. Happy to discontinue here, though not sure if anyone is specifically relying on it right now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just going to move forward with this PR for now. Let's table whether we discontinue this practice across other instruments as well and discuss it later.

I'll add the MIME_TYPE.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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