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

[ENH] - add support for Cohere assistants #307

Merged

Conversation

smokestacklightnin
Copy link
Contributor

@smokestacklightnin smokestacklightnin commented Feb 2, 2024

This addresses Issue #227

As always, comments and change requests are welcome and encouraged

@smokestacklightnin smokestacklightnin force-pushed the cohere-assistant/basic-functionality branch from a81385f to 8bd016f Compare February 3, 2024 00:25
@smokestacklightnin
Copy link
Contributor Author

smokestacklightnin commented Feb 7, 2024

Quick status update

The code seems to receive the JSON stream http responses without error, but is not parsing them properly. No error is reported in the terminal console when running the UI, but the UI response says that something went wrong. I believe this has to do with the type of streamedchatresponse. I'll do more work to try to fix this.

Also, mypy complains when I run it after adapting AsyncIteratorReader from the google assistant code. I tried running mypy on that file too, and got similar complaints. Does anyone have any input? I haven't pushed anything up yet because pre-commit won't pass if mypy complains

@pmeier
Copy link
Member

pmeier commented Feb 8, 2024

@smokestacklightnin I took the liberty and had a look to unblock you. I've pushed bfc868b that implements the streaming. While Cohere also uses a streaming HTTP request, in contrast to Google they don't stream JSON, but rather JSONL. Meaning, each line is valid JSON. Thus, we don't need an extra library to resolve the stream, but rather just ingest the stream line by line and convert to JSON ourselves.

@smokestacklightnin
Copy link
Contributor Author

smokestacklightnin commented Feb 8, 2024

@smokestacklightnin I took the liberty and had a look to unblock you. I've pushed bfc868b that implements the streaming. While Cohere also uses a streaming HTTP request, in contrast to Google they don't stream JSON, but rather JSONL. Meaning, each line is valid JSON. Thus, we don't need an extra library to resolve the stream, but rather just ingest the stream line by line and convert to JSON ourselves.

@pmeier Thanks for the help! I'll continue working on this

@smokestacklightnin
Copy link
Contributor Author

smokestacklightnin commented Feb 8, 2024

I added a preamble_override, which seems to be the last substantial change that is needed. I will then add links in the code and the FAQ documentation.

I tested it, and it seems to be working nominally correctly

@smokestacklightnin smokestacklightnin marked this pull request as ready for review February 8, 2024 19:47
Copy link
Member

@pmeier pmeier left a comment

Choose a reason for hiding this comment

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

Only a few minor comments left. Otherwise this is good to go. Thanks @smokestacklightnin!

@smokestacklightnin
Copy link
Contributor Author

Only a few minor comments left. Otherwise this is good to go. Thanks @smokestacklightnin!

Everything is done. Glad to contribute!

@pmeier pmeier merged commit dc39e01 into Quansight:main Feb 9, 2024
10 checks passed
@smokestacklightnin smokestacklightnin deleted the cohere-assistant/basic-functionality branch January 6, 2025 05:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants