Skip to content

Fix dynamic content types like audio and images #34

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.

Sign up for GitHub

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

Open
davidberenstein1957 wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Fix dynamic content types like audio and images #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3cb0664
fix: Add message conversion to JSON
davidberenstein1957 Nov 27, 2024
1741d92
tests: Remove dependency on DatasetsStore
davidberenstein1957 Nov 27, 2024
220113a
Merge branch 'main' into fix/dynamic-content-types
davidberenstein1957 Nov 27, 2024
2673f53
Merge branch 'main' into fix/dynamic-content-types
davidberenstein1957 Dec 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 1 addition & 7 deletions examples/openai_function_calling_example.py → ...ervers/openai_function_calling_example.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
from observers.observers import wrap_openai
from observers.stores import DatasetsStore
from openai import OpenAI

store = DatasetsStore(
repo_name="gpt-4o-function-calling-traces",
every=5, # sync every 5 minutes
)

openai_client = OpenAI()

tools = [
Expand Down Expand Up @@ -42,7 +36,7 @@
]


client = wrap_openai(openai_client, store=store)
client = wrap_openai(openai_client)

response = client.chat.completions.create(
model="gpt-4o",
Expand Down
8 changes: 1 addition & 7 deletions examples/vision_example.py → examples/observers/openai_vision_example.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,8 @@
from observers.observers import wrap_openai
from observers.stores import DatasetsStore
from openai import OpenAI

store = DatasetsStore(
repo_name="gpt-4o-mini-vision-traces",
every=5, # sync every 5 minutes
)

openai_client = OpenAI()
client = wrap_openai(openai_client, store=store)
client = wrap_openai(openai_client)

response = client.chat.completions.create(
model="gpt-4o-mini",
Expand Down
6 changes: 0 additions & 6 deletions src/observers/observers/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,6 @@
from argilla import Argilla


@dataclass
class Message:
role: Literal["system", "user", "assistant", "function"]
content: str


@dataclass
class Record(ABC):
"""
Expand Down
17 changes: 13 additions & 4 deletions src/observers/observers/models/openai.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
from dataclasses import dataclass, field
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Union

from observers.observers.base import Message, Record
from openai.types.chat.chat_completion_message_param import ChatCompletionMessageParam

from observers.observers.base import Record
from observers.stores.duckdb import DuckDBStore

if TYPE_CHECKING:
Expand All @@ -22,7 +24,7 @@ class OpenAIResponseRecord(Record):

model: str = None
timestamp: str = field(default_factory=lambda: datetime.datetime.now().isoformat())
messages: List[Message] = None
messages: List[ChatCompletionMessageParam] = None
assistant_message: Optional[str] = None
completion_tokens: Optional[int] = None
prompt_tokens: Optional[int] = None
Expand Down Expand Up @@ -82,7 +84,7 @@ def duckdb_schema(self):
id VARCHAR PRIMARY KEY,
model VARCHAR,
timestamp TIMESTAMP,
messages STRUCT(role VARCHAR, content VARCHAR)[],

@davidberenstein1957 davidberenstein1957 Nov 27, 2024 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@cfahlgren1, currently other modalities, like the openai_vision_example.py, do not work with the current message schema.

I am not too familiar with DuckDB but can you have a look at this schema definiton? The input is a bit dynaminc and I am not sure how to best tackle this besides converting to JSON (losing the nice array queryability etc).

tests/integration/observers/test_observers_examples.py hold some of the potential types we could be expecting and it should work for testing them.

[
      {
        role: "user",
        content: [
          {
            type: "text",
            text: "What's in this image?",
          },
          {
            type: "image_url",
            image_url: {
              url: imageUrl,
            },
          },
        ],
      },
]

@cfahlgren1 cfahlgren1 Nov 27, 2024

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Yeah very good point. Ollama has this format for images for their client library for vision

"messages": [
    {
      "role": "user",
      "content": "what is in this image?",
      "images": ["<base64-encoded image data>"]
    }

@cfahlgren1 cfahlgren1 Nov 27, 2024 •
edited
Loading

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

So for it to be a DuckDB Struct each row would have to have the same amount of keys.

The key difference is that DuckDB STRUCTs require the same keys in each row of a STRUCT column

We may have to make it a JSON type

@davidberenstein1957 davidberenstein1957 Nov 27, 2024 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@cfahlgren1 Or we can rewrite the message structure to follow the following structure? I think they can be used interchangeably according to the typing. Then we avoid losing the queryability of the arrays. WDYT?
{"type": "text", "text": "hello", "image_url": None}
{"type": "text", "text": None, "image_url": "datauri"}

@davidberenstein1957 davidberenstein1957 Nov 27, 2024

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

These are the message formats. e.g.

ChatCompletionSystemMessageParam(content=""),
ChatCompletionUserMessageParam(
    content=ChatCompletionContentPartTextParam(text="test")
),
ChatCompletionAssistantMessageParam(content=""),
ChatCompletionUserMessageParam(
    content=ChatCompletionContentPartImageParam(image_url="image")
),
ChatCompletionAssistantMessageParam(content=""),
ChatCompletionUserMessageParam(
    content=ChatCompletionContentPartInputAudioParam(
        input_audio=InputAudio(data="audio", format="wav")
    )
),
ChatCompletionFunctionMessageParam(content=""),
ChatCompletionToolMessageParam(content="")

# the following are equal, more or less

ChatCompletionUserMessageParam(
    content=ChatCompletionContentPartTextParam(text="test")
) 
ChatCompletionUserMessageParam(
    content="test"
)

@davidberenstein1957 davidberenstein1957 Nov 27, 2024

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, that would be the down side, but querying etc would be easier I think.

@cfahlgren1 cfahlgren1 Nov 27, 2024

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Yeah I think that is fine and gets most of the benefits of having it in a sql store like that. Obviously when being send to HuggingFace we won't need to send image url etc if there are none.

@cfahlgren1 cfahlgren1 Nov 27, 2024

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The DuckDB union could be valuable here, but not sure if it would make anything more complicated

@cfahlgren1 cfahlgren1 Dec 1, 2024

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

One the schema changes topic, how do we want to make things backwards compatible or for upgrading existing duckdb instances to work with new version?

One idea is some type of migrations to update columns etc? But don't want to add to much complexity 🤔

@cfahlgren1 cfahlgren1 Dec 1, 2024

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We will have the same problem with SQLite store. It may be better to just store as JSON. It adds a step (CASTING to STRUCT) for querying locally but I think it's a worthy trade off in favor of not introducing a lot of complexity or maintenence

messages JSON,
assistant_message TEXT,
completion_tokens INTEGER,
prompt_tokens INTEGER,
Expand Down Expand Up @@ -177,7 +179,14 @@ def table_name(self):

@property
def json_fields(self):
return ["tool_calls", "function_call", "tags", "properties", "raw_response"]
return [
"tool_calls",
"function_call",
"tags",
"properties",
"raw_response",
"messages",
]

@property
def image_fields(self):
Expand Down
35 changes: 34 additions & 1 deletion tests/integration/observers/test_observers_examples.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,21 @@
ChatCompletionMessage,
)
from openai.types.chat.chat_completion import Choice, CompletionUsage
from openai.types.chat.chat_completion_content_part_input_audio_param import (
InputAudio,
)
from openai.types.chat.chat_completion_content_part_param import (
ChatCompletionContentPartImageParam,
ChatCompletionContentPartInputAudioParam,
ChatCompletionContentPartTextParam,
)
from openai.types.chat.chat_completion_message_param import (
ChatCompletionAssistantMessageParam,
ChatCompletionFunctionMessageParam,
ChatCompletionSystemMessageParam,
ChatCompletionToolMessageParam,
ChatCompletionUserMessageParam,
)


def get_example_files():
Expand All @@ -29,6 +44,24 @@ def mock_clients():
def get_fake_return():
return ChatCompletion(
id=str(uuid.uuid4()),
messages=[
ChatCompletionSystemMessageParam(content=""),
ChatCompletionUserMessageParam(
content=ChatCompletionContentPartTextParam(text="test")
),
ChatCompletionAssistantMessageParam(content=""),
ChatCompletionUserMessageParam(
content=ChatCompletionContentPartImageParam(image_url="image")
),
ChatCompletionAssistantMessageParam(content=""),
ChatCompletionUserMessageParam(
content=ChatCompletionContentPartInputAudioParam(
input_audio=InputAudio(data="audio", format="wav")
)
),
ChatCompletionFunctionMessageParam(content=""),
ChatCompletionToolMessageParam(content=""),
],
choices=[
Choice(
message=ChatCompletionMessage(
Expand All @@ -37,7 +70,7 @@ def get_fake_return():
finish_reason="stop",
index=0,
logprobs=None,
)
),
],
model="gpt-4o",
usage=CompletionUsage(
Expand Down