Skip to content

WIP: AWS Bedrock Support, Google Gemini Function calling, and file support, and more. #320

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

tryvin
Copy link

@tryvin tryvin commented May 19, 2025

  • AWS Bedrock
  • Google Gemini function calling, file support, and more
  • CerebrasAI support
  • Basis for OpenAI-compatible API generic providers

Explanation

Hey guys, I'm finally adding what I have been working on in other projects and bringing it up-to-date with the new Metadata and Output processor for token counts.

This also adds new providers, and an "OpenAI generic" provider is also coming.

I created the PR so you are aware of what I'm doing and prevent double work, and of course, so you can start the review while I finish the tests and more.

@tryvin
Copy link
Author

tryvin commented May 19, 2025

And I just saw #312

@OskarStark OskarStark added the enhancement New feature or request label May 19, 2025
@tryvin tryvin marked this pull request as draft May 19, 2025 20:06
@chr-hertel
Copy link
Member

Oh wow, that's a ton - thanks for bringing that forward! 🙏

I think three things would be important here:

Cheers

@tryvin
Copy link
Author

tryvin commented May 19, 2025

Hey @chr-hertel, yeah, these are things I had to do in my projects that got scattered all over the place, so I'm integrating all of them into the architecture of this project.

Now, answering your questions:

  • Let's try to extract some concerns from this PR to get it sorted more easily, e.g., extended Gemini support or events

    • Agreed, lemme try to finish extracting everything I had into this big PR, and then we can decide on what should be extracted and what shouldn't. There's still work to do here.
  • Can you take a look at these PR and tell which should go first or wait? refactor: introduce model capabilities with generic base model instead of interfaces with supportsX methods #305 refactor: switch from JsonSerializable to normalizer #301 I could imagine that this might help with stuff you needed to sort here

    • I would say 301 should go first, it's a lot of refactor,ing but it will help a lot with adding more providers and making things simple. Sadly, not every provider has the same input and output across the board.
  • And of course, what's your take on feat: implement Bedrock and model clients for Claude, Llama, and Nova #312?

    • We might want to merge things at some point here, I noticed he is using a wrapper for bedrock, in my first iteration (in one project here), I used AWS SDK directly and wrapped the response in HttpResponseInterface implementation, but this time I implemented it directly via REST calls, and only used the AWS SDK to sign the headers so I don't need to care about how to get the credentials and sign it correctly.

@tryvin
Copy link
Author

tryvin commented May 22, 2025

@chr-hertel I think it's now stable enough for me to start writing tests for it, and for us to decide how I should split this PR.

Thoughts?

@chr-hertel
Copy link
Member

Hey, i hope i got it right, but i'd propose to split it into 4 different PRs:

  1. Event Dispatching - was wondering tho why in the Toolbox' processor or if it belongs directly in the chain
  2. Gemini Improvements
  3. Cerebras Implementation

and of course

  1. AWS Bedrock Implementation - but here I'd need another thought what the better approach is - compared to feat: implement Bedrock and model clients for Claude, Llama, and Nova #312 - I personally use AWS too little to have an opinion atm

so, best to start with 1-3? wasn't able to progress with #301 this week - and weekend looks busy as well

@tryvin
Copy link
Author

tryvin commented May 22, 2025

@chr-hertel

  • Event Dispatching - was wondering tho why in the Toolbox's processor, or if it belongs directly in the chain

So, the idea is that to keep the message history correct and track all tool calls and usage tokens, I need access to the intermediary messages. The chain processor was hiding all these intermediary messages and only calling the output processor at the end with the last response that wasn't a tool call.

We can maybe change it to some "Intermediary Processor"

  • AWS Bedrock Implementation - but here I'd need another thought on what the better approach is

We are achieving similar results. I focused initially on Nova and Titan embedding models and used the raw HTTP client. Both of us are using external packages, though. I need AWS SDK for the credentials signature, and the other PR is using an external implementation for calling the service itself, and transforming the response (kinda of what I did initially the first time I interacted with bedrock and php-llm)

IMHO, it's better to use the official SDK and rely only on HTTP requests so you can keep it "uniform", but that's just my thought.

P.S.: Agreed with the Gemini and cerebras splitting. Also, there's an OpenAI-compatible bridge in this PR, don't know if you noticed.

@chr-hertel
Copy link
Member

We can maybe change it to some "Intermediary Processor"

uh, interesting - i like that 👍

it's better to use the official SDK and rely only on HTTP requests so you can keep it "uniform"

I def agree on that uniform argument, but another thought came to my mind - maybe you can confirm or challenge that:

It is quite likely that LLM Chain gets integrated into an existing application, that also has already it's AWS infrastructure layer. And with that infrastructure layer it was already decided to use the SDK or the Async lib.
I'd say it would actually be a nice feature for LLM Chain not to mess with that decision but basically offer both - PlatformFactory::createAsync and PlatformFactory::createSdk (or better naming :D) - and model classes could actually be the same i guess. is there a logic within that thought? WDYT? cc @bjalt

@tryvin
Copy link
Author

tryvin commented May 22, 2025

@chr-hertel The thing is, the SDK just allows you to invoke the model, AWS calls then "Foundation Models", so everything else is on you.

Tool calling management, message bags, and everything.

I mean, I only used the SDK because I didn't want to handle AWS signatures

@chr-hertel
Copy link
Member

gave it another thought and would go with both AWS implementations - one relies on that async package and one the sdk - like you did. still makes sense to me - would need docs and reasonable naming tho

@bjalt
Copy link
Contributor

bjalt commented May 27, 2025

@chr-hertel

We usually use async-aws since it's smaller then the official SDK. The official SDK has more services supported though. I find working with async-aws also much easier.
Both, async-aws and the official SDK, can live side by side.
Having both is probably minimizing the need to install another external library as you already stated.

First I thought the way the signer is used in this PR would hinder role based authentication and require an access key. But now I think the default CredentialProvider will probably handle the authentication chain the same way as just using a service would and thus eliminate the need for handling access keys in an AWS infrastructure. Can you confirm this @tryvin?

Maybe in the chain bundle we can set a bedrock flavor and use the correct library based on that configuration.

@tryvin
Copy link
Author

tryvin commented May 27, 2025

@bjalt The way the PR is currently set up, it would support Session Tokens (Lambdas and IAM Anywhere), as well as the usual static KEY/SECRET. Still, there's also the possibility to add a new type (array|CredentialsProvider $credentials) for it to accept your own AWS CredentialsProvider instance, instead of creating one by itself.

There's also another option, which is supporting both aws-sdk and async-aws/core for the CredentialProvider, as I only need the provider and signer; everything else is useless.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants