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

fix(ai-constructs): support no input graphql query tools #1907

Merged
merged 6 commits into from
Aug 22, 2024

Conversation

atierian
Copy link
Member

@atierian atierian commented Aug 22, 2024

Problem

  • the toolConfiguration.graphqlRequestInputDescriptor.selectionSet in the conversation handler event payload should be a string to support nested selection sets. e.g.
{ 
  items {
    id
    name
    activeCart {
      customerId
      id
      items {
        name
        price
      }
    }
    orderHistory {
      items {
        customerId
        id
        items {
          name
          price
        }
      }
    }
  }
}
  • input-less GraphQL queries are valid tools, but currently don't work because we wrap all generated input strings in parentheses even if they're empty. This results in a tool definition like this:
toolDefinition: {
  name: 'foo',
  description: 'foo description',
  inputSchema: {
    json: {},
  },
  graphqlRequestInputDescriptor: {
    queryName: 'foo',
    selectionSet: 'bar baz
    propertyTypes: {},
  },
}

generating this query:

query ToolQuery() {
  foo() {
     bar baz
  }
}

which is invalid GraphQL. It should be generating this:

- query ToolQuery() {
-  foo() {
+ query ToolQuery {
+ foo {
     bar baz
  }
}

Issue number, if available:

Changes

  • toolConfiguration.graphqlRequestInputDescriptor.selectionSet is now a string instead of string[].
  • input-less GraphQL query tools now generate queries without enclosing ().
  • GraphQL types in propertyTypes will include ! if they're required so the lambda doesn't have to handle appending the ! based on the required array in the json schema.

Corresponding docs PR, if applicable:

Validation

Checklist

  • If this PR includes a functional change to the runtime behavior of the code, I have added or updated automated test coverage for this change.
  • If this PR requires a change to the Project Architecture README, I have included that update in this PR.
  • If this PR requires a docs update, I have linked to that docs PR above.
  • If this PR modifies E2E tests, makes changes to resource provisioning, or makes SDK calls, I have run the PR checks with the run-e2e label set.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@atierian atierian requested review from a team as code owners August 22, 2024 20:02
Copy link

changeset-bot bot commented Aug 22, 2024

🦋 Changeset detected

Latest commit: 6fe941b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@aws-amplify/ai-constructs Patch
@aws-amplify/backend-ai Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Member

@sobolk sobolk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

I'll enable e2e tests on this PR to see if they pass.

// (No @packageDocumentation comment for this package)

```
## API Report File for "@aws-amplify/ai-constructs"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you check why this file shows large diff and regenerate this is such a way that it doesnt?

This might be happening if you have this configured https://docs.github.com/en/get-started/getting-started-with-git/configuring-git-to-handle-line-endings .

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the reference. Addressed in f242254

@sobolk sobolk added the run-e2e Label that will include e2e tests in PR checks workflow label Aug 22, 2024
@sobolk sobolk merged commit bee711e into feature/conversation-handler Aug 22, 2024
56 of 57 checks passed
@sobolk sobolk deleted the conversation-tool-query branch August 22, 2024 23:59
sobolk added a commit that referenced this pull request Sep 4, 2024
… conversation handler (#1948)

* Add `ai-constructs` and default implementation of conversation handler (#1822)

* add backend-ai package

* commonjs

* default handler - wip

* fix bedrock version

* fix that

* e2e test

* update response type

* update response type

* make code testable

* some PR feedback

* rename construct package

* top level try catch

* construct unit tests

* lint

* more tests

* ehhh

* adapter test

* more tests

* addressing TODOs

* readme

* undo that

* Add Bedrock tools to conversation handler (#1830)

* add tools

* changeset

* add e2e test for programmatic tool

* add e2e test for data tool

* lint

* adapter tests

* refactor event tools

* refactor event tools

* gql tool test

* provider test

* factory test

* detect all duplicates

* pr feedback.

* Merge main to conversation handler (#1846)

* update cdk error mapper to catch all deployment errors (#1826)

* update cdk error mapper to catch all deployment errors

* add package-loc

* prevent ctrl+c handling for pnpm (#1818)

* feat: Multi bucket support (#1742)

* feat: init multi-bucket support

* refactor: BackendOutput type

* update package-lock

* temp

* feat: update schema and client_config

* feat: increment schema and client_config versions to 1.1

* rename to buckets and remove friendlyName

* chore: add changeset

* chore: update API.md

* fix: remove duplicated code

* fix: construct test

* feat: validate isDefault

* chore: remove as any

* test: fix client_config_contributor

* chore: update package-lock

* fix: getOutput

* refactor: backend_output_client

* refactor: DefaultBackendOutputClient

* revert changes on addBackendOutputEntry, use appendToBackendOutputList

* update JSdocs for construct

* make storageOutput more specific

* add factoryCounter and hasDefault

* use factoryCounter as postfix

* update changeset

* fix: buckets output schema

* feat: add name to buckets

* feat: throw no isdefault error before deploy

* chore: remove post-deploy error

* refactor: use factoryCounter for policy

* refactor: pass buckets to appendToBackendOutputList

* remove StorageOutputPayloadToStore

* fix construct test

* revert BackendOutput type

* chore: update changeset

* fix: client_config_contributer_v1 and test

* fix: pin api-extractor to 7.40.0

* chore: update API.md

* refactor: use global factoryCounter for policy

* refactor: use addBackendOutputEntry for default

* test: fix storage construct test

* this works

* move DeepPartial

* remove StorageBucketsPayload

* refactor: use Aspects to validate storage

* fix: add buckets to metadata

* fix: factory unit test

* fix: metadata_output_storage_strategy unit test

* fix: not export AmplifyStorage

* refactor: addOrUpdateMetadata

* update package-lock

* update package-lock

* Update packages/backend-storage/src/factory.ts

Co-authored-by: Amplifiyer <[email protected]>

* Update packages/backend-storage/src/factory.ts

Co-authored-by: Amplifiyer <[email protected]>

* refactor: remove redundant code in StorageClientConfigContributor

* test: add unit tests for construct and factory

* fix: one bucket no default case

* fix: unit test

* test log currentCodebaseOutputs and npmOutputs

* fix: convert bucketName, storageRegion into bucket_name, aws_region

* fix: client_config_contributor_v1.test

* refactor: use Aspect to replact factory static

* fix: buckets keys

* fix: unit test

* fix MultipleDefaultBucketError and add more unit tests

* remove policyCount static

* Update packages/backend-storage/src/factory.ts

Co-authored-by: Amplifiyer <[email protected]>

* refactor StorageValidator

* Refactor: storeOutput to factory

* remove comment

* Update packages/backend-storage/src/factory.ts

Co-authored-by: Kamil Sobol <[email protected]>

* Update packages/backend-storage/src/factory.ts

Co-authored-by: Kamil Sobol <[email protected]>

* Update packages/backend-storage/src/factory.ts

Co-authored-by: Kamil Sobol <[email protected]>

* Update packages/backend-storage/src/construct.ts

Co-authored-by: Kamil Sobol <[email protected]>

* Revert "test log currentCodebaseOutputs and npmOutputs"

This reverts commit 4ce8db5.

* rename defaultStorageFound

* remove firstStorage from getInstance

* rename buckets to storage, and fix unit tests

* use isStorageProcessed

* fix construct test

* fix StackMetadataBackendOutputStorageStrategy unit test

* fix a typo to make lint happy

* Update packages/backend-storage/src/factory.ts

Co-authored-by: Amplifiyer <[email protected]>

* Update packages/client-config/src/client-config-contributor/client_config_contributor_v1.ts

Co-authored-by: Edward Foyle <[email protected]>

* fix type error in client_config_contributer_v1

* refactor: use early return

* test: fix test name

* refactor: move Aspects to another file

* refactor: remove this.node

* Update packages/backend-storage/src/storage_outputs_aspect.ts

Co-authored-by: Amplifiyer <[email protected]>

* add isMatch to match outputs

* add unit test for aspects

---------

Co-authored-by: Kamil Sobol <[email protected]>
Co-authored-by: Amplifiyer <[email protected]>
Co-authored-by: Kamil Sobol <[email protected]>
Co-authored-by: Edward Foyle <[email protected]>

* chore(deps): bump fast-xml-parser, @aws-sdk/client-amplify, @aws-sdk/client-cloudformation, @aws-sdk/client-cognito-identity-provider, @aws-sdk/client-dynamodb, @aws-sdk/client-iam, @aws-sdk/client-s3 and @aws-sdk/client-ssm (#1836)

Bumps [fast-xml-parser](https://github.com/NaturalIntelligence/fast-xml-parser) to 4.4.1 and updates ancestor dependencies [fast-xml-parser](https://github.com/NaturalIntelligence/fast-xml-parser), [@aws-sdk/client-amplify](https://github.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-amplify), [@aws-sdk/client-cloudformation](https://github.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-cloudformation), [@aws-sdk/client-cognito-identity-provider](https://github.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-cognito-identity-provider), [@aws-sdk/client-dynamodb](https://github.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-dynamodb), [@aws-sdk/client-iam](https://github.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-iam), [@aws-sdk/client-s3](https://github.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-s3) and [@aws-sdk/client-ssm](https://github.com/aws/aws-sdk-js-v3/tree/HEAD/clients/client-ssm). These dependencies need to be updated together.


Updates `fast-xml-parser` from 4.1.2 to 4.4.1
- [Release notes](https://github.com/NaturalIntelligence/fast-xml-parser/releases)
- [Changelog](https://github.com/NaturalIntelligence/fast-xml-parser/blob/master/CHANGELOG.md)
- [Commits](NaturalIntelligence/fast-xml-parser@v4.1.2...v4.4.1)

Updates `@aws-sdk/client-amplify` from 3.614.0 to 3.624.0
- [Release notes](https://github.com/aws/aws-sdk-js-v3/releases)
- [Changelog](https://github.com/aws/aws-sdk-js-v3/blob/main/clients/client-amplify/CHANGELOG.md)
- [Commits](https://github.com/aws/aws-sdk-js-v3/commits/v3.624.0/clients/client-amplify)

Updates `@aws-sdk/client-cloudformation` from 3.614.0 to 3.624.0
- [Release notes](https://github.com/aws/aws-sdk-js-v3/releases)
- [Changelog](https://github.com/aws/aws-sdk-js-v3/blob/main/clients/client-cloudformation/CHANGELOG.md)
- [Commits](https://github.com/aws/aws-sdk-js-v3/commits/v3.624.0/clients/client-cloudformation)

Updates `@aws-sdk/client-cognito-identity-provider` from 3.614.0 to 3.625.0
- [Release notes](https://github.com/aws/aws-sdk-js-v3/releases)
- [Changelog](https://github.com/aws/aws-sdk-js-v3/blob/main/clients/client-cognito-identity-provider/CHANGELOG.md)
- [Commits](https://github.com/aws/aws-sdk-js-v3/commits/v3.625.0/clients/client-cognito-identity-provider)

Updates `@aws-sdk/client-dynamodb` from 3.614.0 to 3.624.0
- [Release notes](https://github.com/aws/aws-sdk-js-v3/releases)
- [Changelog](https://github.com/aws/aws-sdk-js-v3/blob/main/clients/client-dynamodb/CHANGELOG.md)
- [Commits](https://github.com/aws/aws-sdk-js-v3/commits/v3.624.0/clients/client-dynamodb)

Updates `@aws-sdk/client-iam` from 3.614.0 to 3.624.0
- [Release notes](https://github.com/aws/aws-sdk-js-v3/releases)
- [Changelog](https://github.com/aws/aws-sdk-js-v3/blob/main/clients/client-iam/CHANGELOG.md)
- [Commits](https://github.com/aws/aws-sdk-js-v3/commits/v3.624.0/clients/client-iam)

Updates `@aws-sdk/client-s3` from 3.614.0 to 3.626.0
- [Release notes](https://github.com/aws/aws-sdk-js-v3/releases)
- [Changelog](https://github.com/aws/aws-sdk-js-v3/blob/main/clients/client-s3/CHANGELOG.md)
- [Commits](https://github.com/aws/aws-sdk-js-v3/commits/v3.626.0/clients/client-s3)

Updates `@aws-sdk/client-ssm` from 3.614.0 to 3.624.0
- [Release notes](https://github.com/aws/aws-sdk-js-v3/releases)
- [Changelog](https://github.com/aws/aws-sdk-js-v3/blob/main/clients/client-ssm/CHANGELOG.md)
- [Commits](https://github.com/aws/aws-sdk-js-v3/commits/v3.624.0/clients/client-ssm)

---
updated-dependencies:
- dependency-name: fast-xml-parser
  dependency-type: indirect
- dependency-name: "@aws-sdk/client-amplify"
  dependency-type: direct:production
- dependency-name: "@aws-sdk/client-cloudformation"
  dependency-type: direct:production
- dependency-name: "@aws-sdk/client-cognito-identity-provider"
  dependency-type: direct:development
- dependency-name: "@aws-sdk/client-dynamodb"
  dependency-type: direct:development
- dependency-name: "@aws-sdk/client-iam"
  dependency-type: direct:development
- dependency-name: "@aws-sdk/client-s3"
  dependency-type: direct:production
- dependency-name: "@aws-sdk/client-ssm"
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Revert "feat: Multi bucket support" (#1837)

* Revert "feat: Multi bucket support (#1742)"

This reverts commit d8b43d2.

* add changeset

* upgrade AWS SDK packages to latest (#1839)

* Split sandbox e2e tests (#1841)

* Handle nested namespaces in api check (#1842)

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Amplifiyer <[email protected]>
Co-authored-by: Roshane Pascual <[email protected]>
Co-authored-by: MJ Zhang <[email protected]>
Co-authored-by: Edward Foyle <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Make conversation response selection set dynamic (#1848)

* Make conversation selection set dynamic

* grrr

* UI tools (#1853)

* add client tools to schema

* api

* logic and unit tests

* e2e

* pr feedback

* Add `defineConversationHandlerFunction` and `@aws-amplify/backend-ai` package. (#1858)

* Add backend-ai

* factory

* this works

* changeset

* api

* fix that

* boop

* add unit test

* readme

* some pr feedback

* Serialize tool us blocks (#1866)

* stringify tool input

* stringify tool input

* Adjust API.md generation in backend-ai for api check to work. (#1869)

* Adjust API.md generation in backend-ai for api check to work.

* changeset

* Add main entry point. (#1870)

* fix e2e test

* Add ability to pass inference configuration (#1878)

* Add ability to pass inference configuration

* Add ability to pass inference configuration

* fix(ai-constructs): support no input graphql query tools (#1907)

* fix(ai-constructs): no input graphql query tools

* add changeset

* empty

* fix api diff

* fix tool query assertion

* lets try this again

* Api adjustments for conversation handler (#1938)

* accept models from schema def

* api updates

* api updates

* Squash changeset on feature/conversation-handler branch and fix api check (#1949)

* squash changeset

* fix api check

* grrr

* handle usage of subnamespaced types in api check (#1952)

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Amplifiyer <[email protected]>
Co-authored-by: Roshane Pascual <[email protected]>
Co-authored-by: MJ Zhang <[email protected]>
Co-authored-by: Edward Foyle <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Ian Saultz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-e2e Label that will include e2e tests in PR checks workflow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants