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

Refactor ChatBedrock to improve support for providers #224

Open
3coins opened this issue Oct 3, 2024 · 7 comments
Open

Refactor ChatBedrock to improve support for providers #224

3coins opened this issue Oct 3, 2024 · 7 comments

Comments

@3coins
Copy link
Collaborator

3coins commented Oct 3, 2024

Problem

Bedrock supports several providers (AI21, Anthropic, Amazon, Mistral, Meta etc.), each of which has a slightly different request/response structure. The current ChatBedrock class currently accommodates these providers by incorporating several conditional blocks which have become unsustainable as more providers, models and features have been introduced in Bedrock. This has also made it difficult and error prone to debug and add bug fixes over time.

Solution

Refactor the ChatBedrock class to individual provider classes, for example ChatBedrockAnthropic, and provide a mechanism in ChatBedrock to switch to these, so the users of don't have to create these classes separately.

@3coins 3coins changed the title Refactor ChatBedrock to improve debugging and support for providers Refactor ChatBedrock to improve support for providers Oct 3, 2024
@Forma-Lin
Copy link

Hello, I'm a student from University of Toronto. Me and my team from class are looking for ways we can contribute to langchain. Is this issue intended to solve the different promts model providers accept? How's the expected result different with ChatBedrockConverse class? It looks like also a solution to this issue. Thank you so much for your time reading this.

@cab938
Copy link

cab938 commented Oct 20, 2024

I'm not @3coins but I'm happy to share my frustrations @Forma-Lin . For instance, I use llama models on bedrock, but right now the langchain_aws module doesn't include support for many (any?) of the features within the llama 3.1+ models with respect to tool calling or vision. It would be ideal imo to have the ability to use various models with bedrock and get model-specific support functions.

@Forma-Lin
Copy link

Hi @cab938, thank you so much for the answer. I totally get it now. Do you consider this class refactoring as a reasonable task for four fourth-year university students to complete in a month?

@cab938
Copy link

cab938 commented Nov 1, 2024

Hi @Forma-Lin yes, I do, but I'll note that I'm not a langchain-aws committer so I can't approve any patches or the like. I don't know what the actual process is for creating langchain-aws PRs and getting them approved.

The way I would break this problem down is to think about decoupling the underlying model based on the model ID passed into bedrock, and then creating some top level classes for major models. Sometimes there is no significant difference between two models (e.g. the 70b llama 3.1 and the 405b) with respect to their vocabulary, and sometimes there is (e.g. llama 3.2 90b and llama 3.1 70b). I think you could probably build out a great version just thinking about the different llama models which exist.

1 week planning architecture, 2 weeks writing code and tests, last week actually generating PRs and ensuring docs are good. Across four senior undergraduates in CS this seems doable to me.

@Forma-Lin
Copy link

Hi @cab938 Thank you for the advice! I'm trying to contact @3coins to take this issue.

@3coins
Copy link
Collaborator Author

3coins commented Nov 6, 2024

@Forma-Lin
Thanks for offering help with this. The immediate place to help might be look at the tasks in v0.2.7 milestone. I am discussing long term viability of maintaining the invoke API in ChatBedrock with the LangChain team, and will have more concrete tasks by end of week. In the mean time, it will be helpful to add tests for ChatBedrockConverse and fix the gaps. For example we are missing integration tests for meta models, there are some existing gaps with mistral and anthropic etc.

@3coins
Copy link
Collaborator Author

3coins commented Nov 6, 2024

@cab938
As an open source project, any community member can contribute by either opening a PR or helping review an existing PR, opening issues to report problems or discuss solutions. All of these don't require any privileged access. This repo like LangChain follows a fork and pull-request process, as documented in the contribution guide.

If you have any other questions that is stopping you from contributing, don't hesitate to ask. I can also help assign specific issues or PRs or tag you on them, if you are interested in providing review/feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants