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

feat: Handle multiple user messages in a single request #26

Merged
merged 5 commits into from
Oct 9, 2024

Conversation

ysekiy
Copy link
Contributor

@ysekiy ysekiy commented Jun 26, 2024

Issue #25 : Error when including multiple user messages in a single request to /chat/completions API

Description of changes:

This pull request adds a new method _reframe_multi_payloard to handle multiple user messages in a single request to the /chat/completions API. The method converts the OpenAI format messages to the Bedrock conversational API format by aggregating all user messages into a single message with an array of contents.

The method performs the following steps:

  1. Aggregates all user messages and assistant messages separately into two lists.
  2. Creates a new output message list.
  3. Adds the first user message with the aggregated user contents to the output list.
  4. Adds the first assistant message with the aggregated assistant contents to the output list.

This change allows the application to handle multiple user messages in a single request, which was previously causing an error.

@ysekiy ysekiy changed the title feat: add _reframe_multi_payloard method to handle multiple user mess… feat: Handle multiple user messages in a single request Jun 27, 2024
@daixba
Copy link
Contributor

daixba commented Jul 1, 2024

The PR tried to merge all the user messages and assistant messages into one. I am afraid that changes the order of the messages (in chat history) which may have impacts to the AI response.

A user message can only be merged with the previous one if the previous one is with the same role.

@ysekiy
Copy link
Contributor Author

ysekiy commented Jul 2, 2024

@daixba
Thank you for your comment.

I refactored the implementation to not change the order.

Changed to accept OpenAI-compliant input

  • Allow user and assistant to make multi-turn utterances
  • Allow assistant to make multiple utterances

@rstrahan
Copy link

Hi @ysekiy - Great to see progress on the fix for Issue #25 - I'm keen to try it as I've hit the same problem when trying to use the bedrock-access-gateway with cursor.sh (which sends multiple messages).
Wondering what it your timeline (approx) for releasing the fix? If soon, I'll wait.. otherwise I'll try your branch. Tx!

@ysekiy
Copy link
Contributor Author

ysekiy commented Aug 18, 2024

@rstrahan
Hi Bob, I'm not sure about the timeline for when the fix will be released. I'm using my own ECR image with the following setup:

environment

  • Cloud9 (t2.large - intel cpu architecture)
  • us-west-2 region

step1 : clone

git clone https://github.com/aws-samples/bedrock-access-gateway.git
git fetch origin pull/26/head:fix
git switch fix

step2:

You can add bedrock-access-gateway/scripts/push-to-ecr-fargate.sh (following script) to push your ECR repository.

# Make sure you have created the Repo in AWS ECR in every regions you want to push to before executing this script.
# Usage:
#    cd scripts
#    chmod +x push-to-ecr.sh
#    ./push-to-ecr.sh


# Define variables
IMAGE_NAME="bedrock-proxy-api-ecs"
TAG="latest"
AWS_REGIONS=("us-west-2") # List of AWS regions
#AWS_REGIONS=("us-east-1" "us-west-2" "eu-central-1" "ap-southeast-1" "ap-northeast-1") # List of AWS regions

# Build Docker image
docker build --no-cache -t $IMAGE_NAME:$TAG --platform linux/x86_64 -f ../src/Dockerfile_ecs ../src/

# Loop through each AWS region
for REGION in "${AWS_REGIONS[@]}"
do
    # Get the account ID for the current region
    ACCOUNT_ID=$(aws sts get-caller-identity --region $REGION --query Account --output text)

    # Create repository URI
    REPOSITORY_URI="${ACCOUNT_ID}.dkr.ecr.${REGION}.amazonaws.com/${IMAGE_NAME}"

    # Log in to ECR
    aws ecr get-login-password --region $REGION | docker login --username AWS --password-stdin $REPOSITORY_URI

    # Tag the image for the current region
    docker tag $IMAGE_NAME:$TAG $REPOSITORY_URI:$TAG

    # Push the image to ECR
    docker push $REPOSITORY_URI:$TAG
    echo "Pushed $IMAGE_NAME:$TAG to $REPOSITORY_URI"
done

step3:

create your own docker image and push it

cd script
./push-to-ecr-fargate.sh

step4:

cd {PROJECT_ROOT}
## change account id
sed -i -e "s/366590864501/{YOUR ACCOUNT ID}/g" deployment/BedrockProxyFargate.template
## change ECR computing architecture
sed -i -e "s/ARM64/X86_64/" deployment/BedrockProxyFargate.template

You can push your CloudFormation template to your AWS account with BedrockProxyFargate.template

@ysekiy
Copy link
Contributor Author

ysekiy commented Aug 18, 2024

@daixba
Thank you for your comment on the PR.
I've made the modifications based on your suggestions. This PR seems to have garnered interest from others as well.
Could you please merge this PR?

@deadblue22
Copy link

@ysekiy
I have tried it, and it works for cursor with bedrock claude 3.5.
Thank you!

@zxkane zxkane linked an issue Oct 8, 2024 that may be closed by this pull request
2 tasks
@zxkane zxkane requested a review from daixba October 8, 2024 15:12
@zxkane zxkane merged commit c655f50 into aws-samples:main Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when including multiple user messages in a single request
5 participants