dspy
dspy copied to clipboard
Refactor Bedrock class to support Claude-3 model and improve error handling
This PR refactors the Bedrock class to support the Claude-3 model. The main changes include:
-
Added support for the Claude-3 model which uses a different API format with messages instead of a single prompt. The use_messages flag is set based on the model name to determine the API format to use.
-
Refactored the _create_body method to handle the different API formats for Claude-3 vs other models. It now constructs the appropriate request body based on the use_messages flag.
-
Updated the _call_model method to handle the different response formats from Claude-3 vs other models. It extracts the completion text based on the expected response structure.
@cta2106, I encountered the same "extraneous key [n] is not permitted, please reformat your input and try again." error as you did with Claude 3. To resolve it, I adjusted the code in aws_lm.py by placing del kwargs["n"] right after the check if "n" in kwargs.keys():. I think this adjustment should address issue #680.
Thanks @cta2106 @aazizisoufiane! Should we merge both #680 and #720 in that case? the changes make sense to me but I just want to confirm there are no behavior conflicts from merging either PR.
@arnavsinghvi11 I've tested all Claude models on bedrock, for me there is no conflict.
Thanks folks for the contribution here, can you please just lint the code with ruff config that we have to pass the checks?
Hi @cta2106,
I ran ruff check . --fix-only on my local for aws_lm.py. It seems the linting request is about bedrock.py. Could you check this on your side? Let me know if I can help.
Thanks!
Certainement Soufiane
@cta2106 looks good to merge after you run ruff check . --fix-only and push!
Hey guys, please review PR https://github.com/stanfordnlp/dspy/pull/772 that tackles this issue with a much better redesign. Almost done...
@drawal1 is this PR good to close as well with #795 merged?
yea. pretty sure this can be closed