-
Notifications
You must be signed in to change notification settings - Fork 70
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
EdgeDB AI provider #1121
EdgeDB AI provider #1121
Conversation
Instead of using the async generator and converting the text back into a byte stream, just pipe the response from the RAG request directly to the response in `streamRag`.
Ensures that the chunks are properly parsed and emitted as they come in, splitting multiple events that arrive together in one chunk, and combining partial events from multiple chunks into a single emitted value.
I'd like @deepbuzin to also look at this |
Here are the examples of how we can use this with the Vercel SDK: |
Yeah it looks like there is some expectation that we return some warnings.
We can skip this stuff for now.
Looks like the Mistral provider uses
I think for now we support just the API that our AI extension exposes and we treat it as if we're a really restricted LLM that has built in RAG. Developers will be configuring the AI extension itself to take advantage of features in the underlying model, but we'll need to expose it directly from the extension in order for this provider to take advantage of it. |
debug( | ||
` - CLI found in PATH at: ${location} (resolved to: ${actualLocation})`, | ||
); | ||
if (locations) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small nit: maybe just treat this as an early return instead of wrapping this whole section in an if
?
if (locations == null) {
debug(" - No CLI found in PATH.");
return null;
}
I'm annoyed that we aren't catching this in our tests 🫤
RAG provider should be fine. But I think we need to extend capabilities of the AI binding. It should support other parameters besides the query and also function calling. |
I created a new cleaner PR for this: #1129. |
I think we need a call to discuss what we want to support with the provider and extension (or maybe I'm just not aligned with the goals and provider should just be built to support current capabilities of the extension).
Examples of how we can use this with the Vercel SDK:
the completion route,
the chat route
This PR has the basic support for completion and chat that we also support with edgedb-js ai lib. I made it to work with the AI ext we have and its current capabilities. And it can be used as this but definitely needs improvements in order to have fully compatible interfaces with Vercel
LanguageModelV1
provider.TODO: I need to add README and update some identifiers and interface names throughout the provider since we are not really a language model.
QUESTIONS:
We don't support any of LLM settings
maxTokens,
temperature,
topP,
topK,
frequencyPenalty,
presencePenalty,
stopSequences,
responseFormat,
seed etc,
so I don't know do we wan't to add support for these or should I return an error from the provider if user tries to set any of these.
Do we want to support setting for
safe_prompt
?Do we support structured outputs?
Do we have support for tools?
In the output our RAG returns it doesn't return:
and some other things that are used/returned from the Vercel
LanguageModelV1
provider. So I return 0 for these 2 values for now from the provider.