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

Cohere: Add CohereClient and CohereChatModel to support Chat #917

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

Cohere: Add CohereClient and CohereChatModel to support Chat #917

wants to merge 13 commits into from
+902 −27

Conversation

daixi98
Copy link
Contributor

@daixi98 daixi98 commented Apr 11, 2024 •
edited

Context

As the first step of issue #891 , this PR creates a Java client to invoke Cohere Chat API, and a CohereChatModel: https://docs.cohere.com/reference/chat

Change

  • Enhanced the CohereClient to support Chat API.
  • The updated CohereClient can ask Cohere to generate texts from existing documents, using tools, or connectors.
  • Added a CohereChatModel.

Checklist

Before submitting this PR, please check the following points:

  • I have added unit and integration tests for my change
  • All unit and integration tests in the module I have added/changed are green
  • All unit and integration tests in the core and main modules are green
  • I have added/updated the documentation
  • I have added an example in the examples repo (only for "big" features)
  • I have added my new module in the BOM (only when a new module is added)

Checklist for adding new embedding store integration

  • I have added a {NameOfIntegration}EmbeddingStoreIT that extends from either EmbeddingStoreIT or EmbeddingStoreWithFilteringIT

@daixi98 daixi98 marked this pull request as draft April 11, 2024 06:31
@daixi98 daixi98 marked this pull request as ready for review April 11, 2024 22:52
@langchain4j langchain4j added the P2 High priority label Apr 23, 2024
@langchain4j
Copy link
Owner

Hi @daixi98 thanks a lot! Do you plan to implement CohereChatModel in scope of this PR?

@daixi98
Copy link
Contributor Author

daixi98 commented May 6, 2024

Hey @langchain4j , my plan was to implement the CohereChatModel after the integration with these Cohere endpoints is done. Also open to any new inputs, thanks!

@langchain4j
Copy link
Owner

@daixi98 but that is not related to this change, right?

@daixi98
Copy link
Contributor Author

daixi98 commented May 6, 2024 •
edited

@langchain4j Correct, this change is independent, and can be merged before the integration. I can also start implementing the CohereChatModel if we cannot do the integration recently.

langchain4j
langchain4j reviewed May 7, 2024
View reviewed changes
langchain4j-cohere/src/main/java/dev/langchain4j/model/cohere/ChatHistory.java Outdated
import lombok.NonNull;

@Builder
public class ChatHistory {
Copy link
Owner

Choose a reason for hiding this comment

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

Minor: we are now changing the way classes are organized in the modules. Could you please use the following structure (inside the internal package)?

langchain4j-cohere/src/main/java/dev/langchain4j/model/cohere/CohereChatRequest.java Outdated

String model;

boolean stream;
Copy link
Owner

Choose a reason for hiding this comment

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

Is there a reason primitives are used in this class?

langchain4j-cohere/src/main/java/dev/langchain4j/model/cohere/CohereChatResponse.java Outdated

List<Map<String, String>> documents;

boolean isSearchRequired;
Copy link
Owner

Choose a reason for hiding this comment

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

Is there a reason primitives are used in this class?

langchain4j-cohere/src/main/java/dev/langchain4j/model/cohere/Connector.java Outdated

String userAccessToken;

boolean continueOnFailure;
Copy link
Owner

Choose a reason for hiding this comment

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

Is there a reason primitives are used in this class?

langchain4j-cohere/src/main/java/dev/langchain4j/model/cohere/ParameterDefinition.java Outdated
@NonNull
String type;

boolean required;
Copy link
Owner

Choose a reason for hiding this comment

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

Is there a reason primitives are used in this class?

langchain4j-cohere/src/main/java/dev/langchain4j/model/cohere/SearchResult.java Outdated

String errorMessage;

boolean continueOnFailure;
Copy link
Owner

Choose a reason for hiding this comment

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

Is there a reason primitives are used in this class?

@langchain4j
Copy link
Owner

@daixi98 if CohereChatModel can be implemented without blockers, please proceed, thank you!

@daixi98 daixi98 changed the title Cohere: Update CohereClient to support Chat Cohere: Update CohereClient and CohereChatModel to support Chat May 8, 2024
@daixi98 daixi98 changed the title Cohere: Update CohereClient and CohereChatModel to support Chat Cohere: Add CohereClient and CohereChatModel to support Chat May 8, 2024
@daixi98
Copy link
Contributor Author

daixi98 commented May 8, 2024 •
edited

@langchain4j PR updated with the CohereChatModel.

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

@langchain4j langchain4j langchain4j left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
P2 High priority
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@daixi98 @langchain4j