Fix: strip reasoningContent from messages before sending to Bedrock to avoid ValidationException #652
Conversation
…o avoid ValidationException
There was a problem hiding this comment.
Hi thank you for the contribution.
We would like to get this fix in there are a few considerations before we can
- Is this simply a bedrock bug? I have initiated conversations internally regarding this. I am hoping to have the answer quickly.
- Should we filter incoming or outgoing? It seems that this PR filters the existing messages. I am more biased toward preventing this from entering the Messages array/Session Management eagerly rather than filtering when we call back again. There is definitely a valid use case for accessing these reasoning text blocks, but until we receive a request for that, it seems strange to pollute the messages array with content that is invalid. In my opinion, it is a one way decision for filtering on send, and a two way for filtering on receive, but I may switch no this.
- Unit tests are needed
Regardless, I will follow up regarding point 1, since that is most important and may resolve this issue for us.
|
Hi, I did a quick look at the issue across GitHub https://github.com/search?q=%22User+messages+cannot+contain+reasoning+content.+Please+remove+the+reasoning+content+and+try+again.%22&type=issues It seems there may be a bug with DeepSeek specifically langchain-ai/langchain-aws#408 where the signature is not being returned. To test this we should test with another reasoning model. The fix may be that we simply drop reasoningContent if the response is invalid aka signature is not present. |
|
Hey @dbschmigelski, thanks for your review! I tried with Bedrock Claude 3.7 Sonnet and it's working fine, I mainly use Bedrock and unfortunately I currently don't have access to the Claude 4 models so cannot test them from my side. Regarding preventing the reasoning content from entering the messages array/session management, I thought that it would be good to have a log of the reasoning content generated when using S3SessionManager to understand the agent's thought process for traceability, however if you deem that it is more suitable to avoid filtering and stop it from reaching the messages array I completely agree with your decision as there is no immediate valid use case for it. Let me know what the next steps should be, |
Will do, currently speaking internally with the Bedrock team to receive guidance on this. Apologies for the delay. We won't let this sit for too long, but I want to avoid making a change here, it getting fixed in a couple days, then us dealing with backwards compatibility issues forever. As for Will let you know when we have guidance. |
|
Understood, thank you for your response! Also, I got access to the Claude 4 models on Bedrock and tried all 3 of them which all seem to be working fine, not getting the error that I get with DeepSeek R1. Seems like this is specific to only DeepSeek. |
Fix: strip reasoningContent from messages before sending to Bedrock to avoid ValidationException
Description
This is a PR in a response to bug I found #651 which updates the Bedrock model class in the Strands Agents SDK to strip reasoning content before calling the converse_stream operation to avoid the validation exception.
Related Issues
#651
Documentation PR
Type of Change
Bug fix
Testing
How have you tested the change? Verify that the changes do not break functionality or introduce warnings in consuming repositories: agents-docs, agents-tools, agents-cli
I locally installed my version of the lib with changes I made and tested it on my code which I was getting the error for and it started working with no ValidationException error.
hatch run prepareChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.