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

❗ NOTICE: stepfunctions-tasks: BedrockInvokeModel outputPath no longer passes output to next state #31302

Closed
1 task done
clareliguori opened this issue Sep 3, 2024 · 4 comments · Fixed by #31305, #31308 or softwaremill/tapir#4137 · May be fixed by NOUIY/aws-solutions-constructs#128 or NOUIY/aws-solutions-constructs#129
Assignees
Labels
@aws-cdk/aws-stepfunctions-tasks bug This issue is a bug. management/tracking Issues that track a subject or multiple issues p0 potential-regression Marking this issue as a potential regression to be checked by team member

Comments

@clareliguori
Copy link
Member

clareliguori commented Sep 3, 2024

Please add your +1 👍 to let us know you have encountered this

Status: IN-PROGRESS

Overview:

Previously, the BedrockInvokeModel outputPath parameter could be used to select a portion of the model output to pass to the next state. This behavior aligns with how the parameter is documented "JSONPath expression to select select a portion of the state output to pass to the next state."
https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_stepfunctions_tasks.BedrockInvokeModel.html#outputpath

Now, the outputPath parameter is assumed to be an S3Uri where the model output should be saved. The parameter can no longer be used to select model output and pass it to the next state.

This behavior appears to have changed in f5dd73b

Complete Error Message:

Workaround:

Solution:

Revert existing code for encoding the path under S3Uri, and possibly introduce another variable in BedrockTaskProps instead of extending the existing TaskStateBaseProps.

Related Issues:

Regression Issue

  • Select this option if this issue appears to be a regression.

Last Known Working CDK Version

2.150.0

Expected Behavior

In a BedrockInvokeModel task, set outputPath:

# Extract the response from the model
output_path="$.Body.content[0].text",

The Step Functions workflow definition is synthesized as:

"OutputPath": "$.Body.content[0].text",

When I execute the deployed Step Functions state machine, the model text output is passed as the output of the task.

"Here is a one sentence summary of ...."

Current Behavior

The outputPath parameter is synthesized as:

"Output": {
   "S3Uri": "$.Body.content[0].text"
}

When I execute the deployed Step Functions state machine, I get a validation exception:

The value for the S3Uri field is not recognized as a valid URI: $.Body.content[0].text

Reproduction Steps

This is my stack:
https://github.com/aws-samples/amazon-bedrock-serverless-prompt-chaining/blob/main/techniques/stacks/model_invocation.py

Possible Solution

I suggest reverting the change above, so that outputPath works as documented and expected. The input and output parameters were intended to distinguish between state that should be passed between states (inputPath/outputPath) vs destinations where input and output should be sourced from (input/output).

Additional Information/Context

No response

CDK CLI Version

2.155.0

Framework Version

No response

Node.js Version

v20.17.0

OS

Linux

Language

Python

Language Version

Python 3.12.3

Other information

No response

@clareliguori clareliguori added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 3, 2024
@github-actions github-actions bot added potential-regression Marking this issue as a potential regression to be checked by team member @aws-cdk/aws-stepfunctions-tasks labels Sep 3, 2024
@pahud pahud self-assigned this Sep 3, 2024
@pahud pahud added the investigating This issue is being investigated and/or work is in progress to resolve the issue. label Sep 3, 2024
@ashishdhingra ashishdhingra removed the needs-triage This issue or PR still needs to be triaged. label Sep 3, 2024
@pahud pahud added p0 and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Sep 3, 2024
@pahud pahud removed their assignment Sep 3, 2024
@pahud
Copy link
Contributor

pahud commented Sep 3, 2024

Hi @clareliguori

Thanks for the report. We've confirmed this a regression. We'll take actions immediately.

@Leo10Gama Leo10Gama changed the title stepfunctions-tasks: BedrockInvokeModel outputPath no longer passes output to next state ❗ NOTICE: stepfunctions-tasks: BedrockInvokeModel outputPath no longer passes output to next state Sep 3, 2024
@Leo10Gama Leo10Gama added the management/tracking Issues that track a subject or multiple issues label Sep 3, 2024
@Leo10Gama Leo10Gama pinned this issue Sep 3, 2024
mergify bot pushed a commit to cdklabs/aws-cdk-notices that referenced this issue Sep 3, 2024
@mergify mergify bot closed this as completed in #31308 Sep 3, 2024
@mergify mergify bot closed this as completed in 842ee15 Sep 3, 2024
Copy link

github-actions bot commented Sep 3, 2024

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

1 similar comment
Copy link

github-actions bot commented Sep 3, 2024

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 3, 2024
Leo10Gama added a commit that referenced this issue Sep 3, 2024
…kInvokeModel to use JsonPath" (#31308)

Reverts #30298 to resolve #31302

BREAKING CHANGE: Reverting the original PR will come with the following breaking changes
* **stepfunctions-tasks:** The `BedrockInvokeModel.outputPath` parameter will no longer be an S3Uri
mergify bot pushed a commit that referenced this issue Sep 5, 2024
#31305)

### Issue # (if applicable)

Closes #31302.

### Reason for this change

PR#30298 introduced a breaking change in the stepfunctions-tasks which was encoding the input/output path from the `TaskStateBaseProps` to be used to input a S3 URI, which is not always the case and customer might be using the JSON string as it is.

### Description of changes

To keep the functionality from the original ask of issue #29229 ,added another props to specify S3 URI as an input to the stepfunctions-tasks.

Introduced a feature flag to keep the existing behaviour for the customers and not introduce a breaking change.

### Description of how you validated changes

Updated integ test and deployed in account

Integ Test Results:
`aws stepfunctions start-execution --state-machine-arn <deployed state machine arn> `: should return execution arn

A5-zEnXPmmPWTJx
{
    "executionArn": "arn:aws:states:us-east-1:XXXX:execution:StateMachine2E01A3A5-zEnXPmmPWTJx:516f2e60-9507-46cb-95e6-4d9453429b08",
    "startDate": "2024-09-04T15:43:33.200000-07:00"
}

`aws stepfunctions describe-execution --execution-arn <exection-arn generated before> `: should return status as SUCCEEDED



### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@shikha372
Copy link
Contributor

This issue has been fixed now in latest release (v2.156.0) under feature flag useNewS3UriParametersForBedrockInvokeModelTask which is enabled by default. New properties s3InputUri , s3OutputUri have been added under BedrockInvokeModelProps which can be used to populate S3 URI field dynamically the from previous runtime. To maintain the existing behaviour, you can set this feature flag to false but we recommend using the newly added properties.

Please let us know if you are still facing the issue.

pahud pushed a commit to pahud/aws-cdk that referenced this issue Sep 9, 2024
…kInvokeModel to use JsonPath" (aws#31308)

Reverts aws#30298 to resolve aws#31302

BREAKING CHANGE: Reverting the original PR will come with the following breaking changes
* **stepfunctions-tasks:** The `BedrockInvokeModel.outputPath` parameter will no longer be an S3Uri
pahud pushed a commit to pahud/aws-cdk that referenced this issue Sep 9, 2024
aws#31305)

### Issue # (if applicable)

Closes aws#31302.

### Reason for this change

PR#30298 introduced a breaking change in the stepfunctions-tasks which was encoding the input/output path from the `TaskStateBaseProps` to be used to input a S3 URI, which is not always the case and customer might be using the JSON string as it is.

### Description of changes

To keep the functionality from the original ask of issue aws#29229 ,added another props to specify S3 URI as an input to the stepfunctions-tasks.

Introduced a feature flag to keep the existing behaviour for the customers and not introduce a breaking change.

### Description of how you validated changes

Updated integ test and deployed in account

Integ Test Results:
`aws stepfunctions start-execution --state-machine-arn <deployed state machine arn> `: should return execution arn

A5-zEnXPmmPWTJx
{
    "executionArn": "arn:aws:states:us-east-1:XXXX:execution:StateMachine2E01A3A5-zEnXPmmPWTJx:516f2e60-9507-46cb-95e6-4d9453429b08",
    "startDate": "2024-09-04T15:43:33.200000-07:00"
}

`aws stepfunctions describe-execution --execution-arn <exection-arn generated before> `: should return status as SUCCEEDED



### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
xazhao pushed a commit to xazhao/aws-cdk that referenced this issue Sep 12, 2024
…kInvokeModel to use JsonPath" (aws#31308)

Reverts aws#30298 to resolve aws#31302

BREAKING CHANGE: Reverting the original PR will come with the following breaking changes
* **stepfunctions-tasks:** The `BedrockInvokeModel.outputPath` parameter will no longer be an S3Uri
xazhao pushed a commit to xazhao/aws-cdk that referenced this issue Sep 12, 2024
aws#31305)

### Issue # (if applicable)

Closes aws#31302.

### Reason for this change

PR#30298 introduced a breaking change in the stepfunctions-tasks which was encoding the input/output path from the `TaskStateBaseProps` to be used to input a S3 URI, which is not always the case and customer might be using the JSON string as it is.

### Description of changes

To keep the functionality from the original ask of issue aws#29229 ,added another props to specify S3 URI as an input to the stepfunctions-tasks.

Introduced a feature flag to keep the existing behaviour for the customers and not introduce a breaking change.

### Description of how you validated changes

Updated integ test and deployed in account

Integ Test Results:
`aws stepfunctions start-execution --state-machine-arn <deployed state machine arn> `: should return execution arn

A5-zEnXPmmPWTJx
{
    "executionArn": "arn:aws:states:us-east-1:XXXX:execution:StateMachine2E01A3A5-zEnXPmmPWTJx:516f2e60-9507-46cb-95e6-4d9453429b08",
    "startDate": "2024-09-04T15:43:33.200000-07:00"
}

`aws stepfunctions describe-execution --execution-arn <exection-arn generated before> `: should return status as SUCCEEDED



### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
GavinZZ pushed a commit that referenced this issue Sep 12, 2024
…kInvokeModel to use JsonPath" (#31308)

Reverts #30298 to resolve #31302

BREAKING CHANGE: Reverting the original PR will come with the following breaking changes
* **stepfunctions-tasks:** The `BedrockInvokeModel.outputPath` parameter will no longer be an S3Uri
GavinZZ pushed a commit that referenced this issue Sep 12, 2024
#31305)

### Issue # (if applicable)

Closes #31302.

### Reason for this change

PR#30298 introduced a breaking change in the stepfunctions-tasks which was encoding the input/output path from the `TaskStateBaseProps` to be used to input a S3 URI, which is not always the case and customer might be using the JSON string as it is.

### Description of changes

To keep the functionality from the original ask of issue #29229 ,added another props to specify S3 URI as an input to the stepfunctions-tasks.

Introduced a feature flag to keep the existing behaviour for the customers and not introduce a breaking change.

### Description of how you validated changes

Updated integ test and deployed in account

Integ Test Results:
`aws stepfunctions start-execution --state-machine-arn <deployed state machine arn> `: should return execution arn

A5-zEnXPmmPWTJx
{
    "executionArn": "arn:aws:states:us-east-1:XXXX:execution:StateMachine2E01A3A5-zEnXPmmPWTJx:516f2e60-9507-46cb-95e6-4d9453429b08",
    "startDate": "2024-09-04T15:43:33.200000-07:00"
}

`aws stepfunctions describe-execution --execution-arn <exection-arn generated before> `: should return status as SUCCEEDED



### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@shikha372 shikha372 unpinned this issue Oct 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.