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.

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

Jump to bottom

Add ChatMessage as a first class cog type #2186

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Add ChatMessage as a first class cog type #2186

wants to merge 9 commits into from
+239 −11

Conversation

8W9aG
Copy link
Contributor

@8W9aG 8W9aG commented Mar 6, 2025

8W9aG added 2 commits March 6, 2025 13:43
@8W9aG
Add CommonChatSchema types
7f9f7ac 
* Add typed dictionaries which conform to OpenAI’s
protocol specification as first class objects due
to the ubiquity of their use.
@8W9aG
Add integration test for chat messages
f3c1593
@8W9aG 8W9aG requested review from fofr and andreasjansson March 6, 2025 18:47
erbridge
erbridge reviewed Mar 7, 2025
View reviewed changes
@andreasjansson
Copy link
Member

Niceee!

Is there a reason to use CommonChatSchemaChatMessage instead of just calling it ChatMessage?

How do you test this with cog predict? Does cog predict -i messages='[{"role": "user", "content": "hello"}]' work?

@8W9aG
Copy link
Contributor Author

8W9aG commented Mar 7, 2025

Niceee!

Is there a reason to use CommonChatSchemaChatMessage instead of just calling it ChatMessage?

How do you test this with cog predict? Does cog predict -i messages='[{"role": "user", "content": "hello"}]' work?

Reason for the namespace CommonChatSchema was to namespace all the sub objects such as Audio and Function, however I can change the root to ChatMessage if it suits since its a union anyway.

I couldn't get cog predict to work on this but I can get it to recognise an API call, would you like me to extend the support to cog predict?

@andreasjansson
Copy link
Member

Yes I think ChatMessage is cleaner. And it would be great if we could get it working with cog predict so these functions can be tested locally.

8W9aG added 4 commits March 7, 2025 15:28
@8W9aG
Change CommonChatSchemaChatMessage to ChatMessage
076c478
@8W9aG
Support chat message in cog predict
631ac1f 
* Use the schema to parse the expected format of
the key, and if it is in our anticipated formats
encode it with JSON and send that via API to the
cog service.
@8W9aG
Merge branch 'main' into sackfield/add-first-class-chat-message-type
502d92b
@8W9aG
Fix no messages within Input schema
fda86d4 
* This is valid in OpenAPI
@8W9aG
Copy link
Contributor Author

8W9aG commented Mar 10, 2025

Yes I think ChatMessage is cleaner. And it would be great if we could get it working with cog predict so these functions can be tested locally.

I believe this now complete

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erbridge erbridge erbridge left review comments

@fofr fofr Awaiting requested review from fofr

@andreasjansson andreasjansson Awaiting requested review from andreasjansson

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@8W9aG @andreasjansson @erbridge