-
Notifications
You must be signed in to change notification settings - Fork 375
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: chat engine based on agents #909
Conversation
🦋 Changeset detectedLatest commit: 996b181 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
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 |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
as |
@marcusschiesser it felt safer, one reason was the output reasponse/streams of ChatEngines and Agents aren't compatible, so this engine is a way to standardise it to match the other engines. I also couldn't traverse exactly what happens, but the Agents seem to keep a history for internal usage, would help to clarify what the history on the Agent does Are you thinking that the |
@parhammmm @himself65 i guess we should then better unify the output reasponse/streams of ChatEngines and Agents? |
@marcusschiesser I'm not confident I can do it myself safely but I can take a stab at it and sync with @himself65 for notes/review. Would it be possible in the meantime to release |
@marcusschiesser what do you think? |
@himself65 parhammmm is proposing the new agent chat engine for two reasons (instead of just using the
About 1. I think it would be good to use |
@marcusschiesser thanks for taking a look. part of the fix for 2 is actually in this PR and can be used to match the other engines here: https://github.com/run-llama/LlamaIndexTS/pull/909/files#diff-e722a872695a5840308e8f2f844e6a3d6f25969880fe3e670ae5797ac38a9724R289 What do you think of doing the two patches for 1 & 2 in two different PRs? The history bug is the most important and it would help unlock making a gpt style chat, which is what I'm actually using Thanks again. |
@parhammmm great, yes 1 & 2 should be one PR each. If you're blocked by 2, would you like to send a PR for it by changing the |
@marcusschiesser sure thing will get to it shortly |
@himself65 and @parhammmm i started a draft PR how to solve problem 1: #930 - please leave your comments |
@marcusschiesser @himself65 I'm not deep enough into LITS to be able to give good feed back on 930, but I pushed the history fix to this PR #933 And as a work around for the agent's responses I'm doing the following on our side, which translates to adding a utility that handles the merging of the two types (just incase it's helpful) import { Response } from "llamaindex";
import { ReadableStream } from "node:stream/web";
type LlamaindexAgentResponse = { response: { delta: string } };
type LlamaindexResponse = Response | LlamaindexAgentResponse;
...
// agent or chat engine
const response: AsyncIterable<Response> | ReadableStream<LlamaindexAgentResponse> = await agent.chat({ ... })
...
// value is from reading the response stream
const result =
typeof value.response === "string"
? value.response
: value.response.delta; |
@marcusschiesser this one is the other engine we talked about.
Includes a fix for chat history in agents