aws[minor]: Add ChatModel that uses Bedrock.converse API

[baskaryan]

Decisions to discuss:

• supports bedrock converse, anthropic, and openai format inputs
• outputs anthropic format messages

[rsgrewal-aws]

I feel this class should be merged with the ChatBedrock Class with a flag being passed in at creation time to indicate which api to switch too. The reason being converse will not work in a chain unless we have prompt tenmplate, retrievers .... all having a similiar api. It may be better to continue to use invoke in the chain but internally in the ChatBedrock class fork and switch to converse api

[baskaryan]

The reason being converse will not work in a chain unless we have prompt tenmplate, retrievers...

Not sure i understand, why wouldn't it work with other components? its still implementing the BaseChatModel interface

[rsgrewal-aws]

The reason being say I define a chain like Prompt | llm | outputparse And if I try and call it by doing chain.converse(..) that will not work so I have to invoke it using chain.invoke(..) however in LLM (ChatBedrock ) class we can fork and call converse api Secondly from design we encapsulate the converse api since that is native and will proprietary to Bedrock only. SageMaker for example will not support converse api From: Bagatur ***@***.***> Reply-To: langchain-ai/langchain-aws ***@***.***> Date: Thursday, June 13, 2024 at 3:24 PM To: langchain-ai/langchain-aws ***@***.***> Cc: "Grewal, Rupinder" ***@***.***>, Comment ***@***.***> Subject: Re: [langchain-ai/langchain-aws] aws[minor]: Add ChatModel that uses Bedrock.converse API (PR #74) The reason being converse will not work in a chain unless we have prompt tenmplate, retrievers... Not sure i understand, why wouldn't it work with other components? its still implementing the BaseChatModel interface — Reply to this email directly, view it on GitHub<#74 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AYMBZRRLRUDCUJXNGE5JMSDZHILWPAVCNFSM6AAAAABJJGOL7CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRWHA4TANZXGY>. You are receiving this because you commented.Message ID: ***@***.***>

[baskaryan]

The reason being say I define a chain like Prompt | llm | outputparse And if I try and call it by doing chain.converse(..) that will not work so I have to invoke it using chain.invoke(..) however in LLM (ChatBedrock ) class we can fork and call converse api Secondly from design we encapsulate the converse api since that is native and will proprietary to Bedrock only. SageMaker for example will not support converse api From: Bagatur @.> Reply-To: langchain-ai/langchain-aws @.> Date: Thursday, June 13, 2024 at 3:24 PM To: langchain-ai/langchain-aws @.> Cc: "Grewal, Rupinder" @.>, Comment @.> Subject: Re: [langchain-ai/langchain-aws] aws[minor]: Add ChatModel that uses Bedrock.converse API (PR #74) The reason being converse will not work in a chain unless we have prompt tenmplate, retrievers... Not sure i understand, why wouldn't it work with other components? its still implementing the BaseChatModel interface — Reply to this email directly, view it on GitHub<#74 (comment)>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AYMBZRRLRUDCUJXNGE5JMSDZHILWPAVCNFSM6AAAAABJJGOL7CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRWHA4TANZXGY. You are receiving this because you commented.Message ID: @.>

to clarify, this is using the bedrock client's converse API but the way you use the resulting langchain chat model is still via invoke. so your chain would continue to work

[rsgrewal-aws]

Right but it would not be using the converse api. Our goal is to have the ChatBedrock use and switch to the converse api internally since that will be the dominant way of running the workloads. Hence I feel a new class by itself will not be so useful if I cannot plug it into any chain From: Bagatur ***@***.***> Reply-To: langchain-ai/langchain-aws ***@***.***> Date: Thursday, June 13, 2024 at 5:52 PM To: langchain-ai/langchain-aws ***@***.***> Cc: "Grewal, Rupinder" ***@***.***>, Comment ***@***.***> Subject: Re: [langchain-ai/langchain-aws] aws[minor]: Add ChatModel that uses Bedrock.converse API (PR #74) The reason being say I define a chain like Prompt | llm | outputparse And if I try and call it by doing chain.converse(..) that will not work so I have to invoke it using chain.invoke(..) however in LLM (ChatBedrock ) class we can fork and call converse api Secondly from design we encapsulate the converse api since that is native and will proprietary to Bedrock only. SageMaker for example will not support converse api From: Bagatur @.> Reply-To: langchain-ai/langchain-aws @.> Date: Thursday, June 13, 2024 at 3:24 PM To: langchain-ai/langchain-aws @.> Cc: "Grewal, Rupinder" @.>, Comment @.> Subject: Re: [langchain-ai/langchain-aws] aws[minor]: Add ChatModel that uses Bedrock.converse API (PR #74<#74>) The reason being converse will not work in a chain unless we have prompt tenmplate, retrievers... Not sure i understand, why wouldn't it work with other components? its still implementing the BaseChatModel interface — Reply to this email directly, view it on GitHub<#74 (comment)<#74 (comment)>>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AYMBZRRLRUDCUJXNGE5JMSDZHILWPAVCNFSM6AAAAABJJGOL7CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRWHA4TANZXGY. You are receiving this because you commented.Message ID: @.> to clarify, this is using the bedrock client's converse API but the way you use the result langchain chat model is still via invoke. so your chain would continue to work — Reply to this email directly, view it on GitHub<#74 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AYMBZRUBGOQ2J4NOABAFVGTZHI5B7AVCNFSM6AAAAABJJGOL7CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRXGAZDANRWG4>. You are receiving this because you commented.Message ID: ***@***.***>

[userlerueda]

@baskaryan is there a way for me to test this (other than copying the code locally)? I would really like to give it a try.

[userlerueda]

@baskaryan is there a way for me to test this (other than copying the code locally)? I would really like to give it a try.

Nevermind, I got it, leaving it here for reference (using poetry):

poetry add git+https://github.com/langchain-ai/langchain-aws@bagatur/bedrock_converse#subdirectory=libs/aws

[userlerueda]

Here are my findings after playing with the ChatBedrockConverse model:

• We should keep model_id instead of model (remove the alias) so that is in line with BedrockBase
• Found two fields that are optional but that had no default value (added them to Fixes a problem with tools not working with the new converse endpoint using anthropic models #76)
• Found some fields that might have been typos (e.g., toolUseID vs toolUseId) boto3 docs
• Found an issue where bedrock returns the input values for the tool as string instead of a JSON (dict), added a function to attempt to convert it and on JSON decode error, return the original value.
• Found another issue where the current _snake_to_camel_keys function will attempt to convert the input values for the tool. The problem seems to be that we are calling the _format_tools twice. An easy way to fix it is to add a property to the ChatBedrockConverse so we keep the formatted_tools, that way we only format them once. I have done that and marked the property as excluded from dumping to avoid any compatibility problems.

With these changes, I was able to run the https://python.langchain.com/v0.1/docs/use_cases/tool_use/multiple_tools/ example with bedrock, using anthropic.

{
  "input": "Take 3 to the fifth power and multiply that by the sum of twelve and three, then square the whole result",
  "conversation_id": null,
  "chat_history": [],
  "output": [
    {
      "type": "text",
      "text": "So the final result of taking 3 to the 5th power (243), multiplying by 12 + 3 (15) to get 3645, and then squaring that is 13286025.",
      "index": 0
    }
  ]
}

[austinmw]

From a user perspective, will it be confusing to switch class name yet again, to ChatBedrockConverse? If the converse API will be the primary API going forward, can ChatBedrock not just switch to using that?

BedrockChat -> ChatBedrock -> ChatBedrockConverse is getting a little crazy IMO 😅

[baskaryan]

From a user perspective, will it be confusing to switch class name yet again, to ChatBedrockConverse? If the converse API will be the primary API going forward, can ChatBedrock not just switch to using that?

BedrockChat -> ChatBedrock -> ChatBedrockConverse is getting a little crazy IMO 😅

idea is to eventually replace ChatBedrock with ChatBedrockConverse. issue is converse doesn't currently support guardrails or custom models afaik, so it'd be breaking to replace now. but for all regular functionality converse provides a more reliable/universal api (eg we can implement tool calling for all models in one go) so it seems worth exposing now.

very open to different names (eg ChatBedrockV2). also this is being marked as beta and we can make clear in the docs that this is a preview of what's to come, and folks should only switch to it if they are having issues with ChatBedrock or want to get ahead of the curve

[rsgrewal-aws]

We cannot change the name as that is 1/ breaking change and will cause confusion for existing customers 2/ We need to follow the naming convention of LangChain which is ChatXYZ.... like ChatAnthropic etc. So ChatBedrock will need to stay 3/ Converse API is a internal to Bedrock and not a feature of LangChain and hence it has to be encapsulated in the ChatBedrock class

[anjanvb]

I echo @austinmw's and @rsgrewal-aws position on this. Would it make more sense to have an optional parameter for ChatBedrock defaulting to false, called enable_converse_api or something along the lines?

@3coins thoughts?

[rsgrewal-aws]

also there is another PR -- #76 may be we can review that

[3coins]

@austinmw @anjanvb
Having this new class provides option to users who want to migrate to the converse API (tool support) early while also allowing users who want to use guardrails/custom models to keep using the existing bedrock classes. Existing Bedrock classes are already hard to maintain and test with all the conditional logic, and any additional code to support converse in the same class will make it worse.

[austinmw]

Understood. Would have been ideal IMO to turn on converse functionality in ChatBedrock directly with an optional parameter, but if that makes the class logic too complicated to maintain then I guess a new beta class will have to do.

[rsgrewal-aws]

i would suggest to then add the builder pattern to compose this class at run time and inject the objects which provide or do not provide the functionality. But changing the Class name and pivoting to a new class has impacts and consequences wider and we will tend to loose customer trust . This will be the 3rd class we will be introducing for Bedrock

[baskaryan]

think we could maybe get the best of both worlds, added support for

ChatBedrock(beta_use_converse_api=True)

55710e4

which delegates logic to ChatBedrockConverse. what do folks think @3coins @austinmw @anjanvb?

[3coins]

@baskaryan
Looks good to me!

@rsgrewal-aws @austinmw
Are you folks ok with this change? Would you like to test the changes and provide your feedback here?

[ccurme]

LGTM up to streaming tool calls (discussed offline):

https://smith.langchain.com/public/f05cf920-a2a6-46f1-aeaf-dbd4859c4d8d/r

tool call IDs are null in aggregated output

[ccurme]

(nit) could we keep the standard tests in test_standard.py? will be easier to find.

[anjanvb]

@baskaryan Looks good to me!

@rsgrewal-aws @austinmw Are you folks ok with this change? Would you like to test the changes and provide your feedback here?

I'm good with this approach @3coins @baskaryan

[rsgrewal-aws]

Awesome -- good with this for now - this will help a lot. I think we can may be start to refactor this class and do the builder / composite pattern to start to reduce the complexity and reduce all the if-then statements

[userlerueda]

Updated the other PR to submit just the new changes for the stuff I found during testing with the anthropic claude model using the new bedrock converse endpoint.