From d952e68ec45f17a11da8d05b92e70ef6c9b40720 Mon Sep 17 00:00:00 2001 From: Gunnar Holwerda <gunnarholwerda@gmail.com> Date: Sun, 23 Feb 2025 19:56:21 -0800 Subject: [PATCH] Fix refine synthesizer empty source nodes behavior (#1677) --- ...synthesizer_empty_source_nodes_behavior.md | 5 ++ .../core/src/response-synthesizers/factory.ts | 9 ++- .../tests/memory/chat-memory-buffer.test.ts | 4 +- .../compact-and-refine.test.ts | 73 ++++++++++++++----- 4 files changed, 71 insertions(+), 20 deletions(-) create mode 100644 .changeset/fix_refine_synthesizer_empty_source_nodes_behavior.md diff --git a/.changeset/fix_refine_synthesizer_empty_source_nodes_behavior.md b/.changeset/fix_refine_synthesizer_empty_source_nodes_behavior.md new file mode 100644 index 000000000..a8fb91361 --- /dev/null +++ b/.changeset/fix_refine_synthesizer_empty_source_nodes_behavior.md @@ -0,0 +1,5 @@ +--- +"@llamaindex/core": patch +--- + +Refine synthesizer will now return an empty string as the response if an empty array of source nodes were provided. Before it would throw an internal error converting undefined to ReadableStream. diff --git a/packages/core/src/response-synthesizers/factory.ts b/packages/core/src/response-synthesizers/factory.ts index b513c2348..0e6ae1753 100644 --- a/packages/core/src/response-synthesizers/factory.ts +++ b/packages/core/src/response-synthesizers/factory.ts @@ -116,7 +116,14 @@ class Refine extends BaseSynthesizer { } } - // fixme: no source nodes provided, cannot fix right now due to lack of context + if (response === undefined) { + response = stream + ? (async function* () { + yield ""; + })() + : ""; + } + if (typeof response === "string") { return EngineResponse.fromResponse(response, false, nodes); } else { diff --git a/packages/core/tests/memory/chat-memory-buffer.test.ts b/packages/core/tests/memory/chat-memory-buffer.test.ts index 92f13c59b..51a7a0c37 100644 --- a/packages/core/tests/memory/chat-memory-buffer.test.ts +++ b/packages/core/tests/memory/chat-memory-buffer.test.ts @@ -66,9 +66,9 @@ describe("ChatMemoryBuffer", () => { expect(result).toEqual([...inputMessages, ...storedMessages]); }); - test("getMessages throws error when initial token count exceeds limit", () => { + test("getMessages throws error when initial token count exceeds limit", async () => { const buffer = new ChatMemoryBuffer({ tokenLimit: 10 }); - expect(async () => buffer.getMessages(undefined, 20)).rejects.toThrow( + await expect(async () => buffer.getMessages(undefined, 20)).rejects.toThrow( "Initial token count exceeds token limit", ); }); diff --git a/packages/core/tests/response-synthesizers/compact-and-refine.test.ts b/packages/core/tests/response-synthesizers/compact-and-refine.test.ts index fa3cad252..8bc20e9f8 100644 --- a/packages/core/tests/response-synthesizers/compact-and-refine.test.ts +++ b/packages/core/tests/response-synthesizers/compact-and-refine.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, test, vi } from "vitest"; +import { beforeEach, describe, expect, test, vi } from "vitest"; import type { LLMMetadata } from "../../llms/dist/index.js"; import { getResponseSynthesizer } from "../../response-synthesizers/dist/index.js"; import { Document } from "../../schema/dist/index.js"; @@ -10,26 +10,69 @@ const mockLllm = () => ({ return response; } - function* gen() { - // yield a few times to make sure each chunk has the sourceNodes - yield response; - yield response; - yield response; - } - - return gen(); + return { + [Symbol.asyncIterator]: function* gen() { + // yield a few times to make sure each chunk has the sourceNodes + yield response; + yield response; + yield response; + }, + }; }), chat: vi.fn(), metadata: {} as unknown as LLMMetadata, }); +describe("refine response synthesizer", () => { + let synthesizer: ReturnType<typeof getResponseSynthesizer<"refine">>; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const isAsyncIterable = (obj: any): boolean => + obj[Symbol.asyncIterator] !== undefined; + + beforeEach(() => { + synthesizer = getResponseSynthesizer("refine", { + llm: mockLllm(), + }); + }); + + describe("getResponse", () => { + test("should return async iterable of EngineResponse when stream is true and sourceNodes are empty", async () => { + const response = await synthesizer.getResponse( + "unimportant query", + [], + true, + ); + + expect(isAsyncIterable(response)).toBe(true); + for await (const chunk of response) { + expect(chunk.message.content).toEqual(""); + } + }); + + test("should return non async iterable when stream is false and sourceNodes are empty", async () => { + const response = await synthesizer.getResponse( + "unimportant query", + [], + false, + ); + + expect(isAsyncIterable(response)).toBe(false); + expect(response.message.content).toEqual(""); + }); + }); +}); + describe("compact and refine response synthesizer", () => { + let synthesizer: ReturnType<typeof getResponseSynthesizer<"compact">>; + + beforeEach(() => { + synthesizer = getResponseSynthesizer("compact", { + llm: mockLllm(), + }); + }); + describe("synthesize", () => { test("should return original sourceNodes with response when stream = false", async () => { - const synthesizer = getResponseSynthesizer("compact", { - llm: mockLllm(), - }); - const sourceNode = { node: new Document({}), score: 1 }; const response = await synthesizer.synthesize( @@ -44,10 +87,6 @@ describe("compact and refine response synthesizer", () => { }); test("should return original sourceNodes with response when stream = true", async () => { - const synthesizer = getResponseSynthesizer("compact", { - llm: mockLllm(), - }); - const sourceNode = { node: new Document({}), score: 1 }; const response = await synthesizer.synthesize( -- GitLab