-
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
refactor: unify response and agent response #930
Changes from 1 commit
50590fd
5599aa6
5bd9d82
642eb3a
8a04273
0899592
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,27 @@ | ||
import type { NodeWithScore } from "./Node.js"; | ||
import type { ChatMessage, ChatResponse } from "./llm/types.js"; | ||
|
||
/** | ||
* Response is the output of a LLM | ||
*/ | ||
export class Response { | ||
export class Response implements ChatResponse { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could we don't naming There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how about |
||
// @deprecated use 'message' instead | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we tell the user to not use |
||
response: string; | ||
sourceNodes?: NodeWithScore[]; | ||
metadata: Record<string, unknown> = {}; | ||
|
||
constructor(response: string, sourceNodes?: NodeWithScore[]) { | ||
message: ChatMessage; | ||
raw: object | null; | ||
|
||
constructor( | ||
response: string, | ||
sourceNodes?: NodeWithScore[], | ||
chatResponse?: ChatResponse, | ||
) { | ||
this.response = response; | ||
this.sourceNodes = sourceNodes || []; | ||
this.message = chatResponse?.message ?? { | ||
content: response, | ||
role: "assistant", | ||
}; | ||
this.raw = chatResponse?.raw ?? null; | ||
} | ||
|
||
protected _getFormattedSources() { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
import { ReadableStream, TransformStream, randomUUID } from "@llamaindex/env"; | ||
import { Response } from "../Response.js"; | ||
import { Settings } from "../Settings.js"; | ||
import { | ||
type ChatEngine, | ||
|
@@ -9,13 +10,8 @@ import { wrapEventCaller } from "../internal/context/EventCaller.js"; | |
import { consoleLogger, emptyLogger } from "../internal/logger.js"; | ||
import { getCallbackManager } from "../internal/settings/CallbackManager.js"; | ||
import { isAsyncIterable } from "../internal/utils.js"; | ||
import type { | ||
ChatMessage, | ||
ChatResponse, | ||
ChatResponseChunk, | ||
LLM, | ||
MessageContent, | ||
} from "../llm/index.js"; | ||
import type { ChatMessage, LLM, MessageContent } from "../llm/index.js"; | ||
import { extractText } from "../llm/utils.js"; | ||
import type { BaseToolWithCall, ToolOutput } from "../types.js"; | ||
import type { | ||
AgentTaskContext, | ||
|
@@ -101,16 +97,6 @@ export function createTaskOutputStream< | |
}); | ||
} | ||
|
||
export type AgentStreamChatResponse<Options extends object> = { | ||
response: ChatResponseChunk<Options>; | ||
sources: ToolOutput[]; | ||
}; | ||
|
||
export type AgentChatResponse<Options extends object> = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
response: ChatResponse<Options>; | ||
sources: ToolOutput[]; | ||
}; | ||
|
||
export type AgentRunnerParams< | ||
AI extends LLM, | ||
Store extends object = {}, | ||
|
@@ -210,11 +196,7 @@ export abstract class AgentRunner< | |
> | ||
? AdditionalMessageOptions | ||
: never, | ||
> implements | ||
ChatEngine< | ||
AgentChatResponse<AdditionalMessageOptions>, | ||
ReadableStream<AgentStreamChatResponse<AdditionalMessageOptions>> | ||
> | ||
> implements ChatEngine | ||
{ | ||
readonly #llm: AI; | ||
readonly #tools: | ||
|
@@ -320,47 +302,34 @@ export abstract class AgentRunner< | |
}); | ||
} | ||
|
||
async chat( | ||
params: ChatEngineParamsNonStreaming, | ||
): Promise<AgentChatResponse<AdditionalMessageOptions>>; | ||
async chat(params: ChatEngineParamsNonStreaming): Promise<Response>; | ||
async chat( | ||
params: ChatEngineParamsStreaming, | ||
): Promise<ReadableStream<AgentStreamChatResponse<AdditionalMessageOptions>>>; | ||
): Promise<ReadableStream<Response>>; | ||
@wrapEventCaller | ||
async chat( | ||
params: ChatEngineParamsNonStreaming | ChatEngineParamsStreaming, | ||
): Promise< | ||
| AgentChatResponse<AdditionalMessageOptions> | ||
| ReadableStream<AgentStreamChatResponse<AdditionalMessageOptions>> | ||
> { | ||
): Promise<Response | ReadableStream<Response>> { | ||
const task = this.createTask(params.message, !!params.stream); | ||
for await (const stepOutput of task) { | ||
// update chat history for each round | ||
this.#chatHistory = [...stepOutput.taskStep.context.store.messages]; | ||
if (stepOutput.isLast) { | ||
const { output, taskStep } = stepOutput; | ||
const { output } = stepOutput; | ||
if (isAsyncIterable(output)) { | ||
return output.pipeThrough< | ||
AgentStreamChatResponse<AdditionalMessageOptions> | ||
>( | ||
return output.pipeThrough<Response>( | ||
new TransformStream({ | ||
transform(chunk, controller) { | ||
controller.enqueue({ | ||
response: chunk, | ||
get sources() { | ||
return [...taskStep.context.store.toolOutputs]; | ||
}, | ||
}); | ||
controller.enqueue(new Response(chunk.delta)); | ||
}, | ||
}), | ||
); | ||
} else { | ||
return { | ||
response: output, | ||
get sources() { | ||
return [...taskStep.context.store.toolOutputs]; | ||
}, | ||
} satisfies AgentChatResponse<AdditionalMessageOptions>; | ||
return new Response( | ||
extractText(output.message.content), | ||
undefined, | ||
output, | ||
); | ||
} | ||
} | ||
} | ||
|
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.
Idea: we include
message
andraw
fromChatResponse
instead of addingchatResponse: ChatResponse
as an attribute toResponse
Note: we
ChatResponse
supports extension withOptions
we have to add this to this PR (after we agree on the contract)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.
another advantage is that
Reponse
behaves likeChatReponse
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.
TBD: how to deal with
chunk
s? We haveChatReponseChunk
s as type returned by LLMs but chat engines are just streamingResponse
objects.Suggestion: we add an optional
delta
attribute toResponse