diff --git a/package-lock.json b/package-lock.json index 96bfabe4..de165655 100644 --- a/package-lock.json +++ b/package-lock.json @@ -233,7 +233,6 @@ "resolved": "https://registry.npmjs.org/apache-arrow/-/apache-arrow-18.1.0.tgz", "integrity": "sha512-v/ShMp57iBnBp4lDgV8Jx3d3Q5/Hac25FWmQ98eMahUiHPXcvwIMKJD0hBIgclm/FCG+LwPkAKtkRO1O/W0YGg==", "license": "Apache-2.0", - "peer": true, "dependencies": { "@swc/helpers": "^0.5.11", "@types/command-line-args": "^5.2.3", diff --git a/src/retriever.ts b/src/retriever.ts index 769c248b..a1a53a2e 100644 --- a/src/retriever.ts +++ b/src/retriever.ts @@ -916,24 +916,56 @@ export class MemoryRetriever { trace?.startStage("parallel_search", []); failureStage = "hybrid.parallelSearch"; - const [vectorResults, bm25Results] = await Promise.all([ + const settledResults = await Promise.allSettled([ this.runVectorSearch( queryVector, candidatePoolSize, scopeFilter, category, - ).catch((error) => { - throw attachFailureStage(error, "hybrid.vectorSearch"); - }), + ), this.runBM25Search( bm25Query, candidatePoolSize, scopeFilter, category, - ).catch((error) => { - throw attachFailureStage(error, "hybrid.bm25Search"); - }), + ), ]); + + const vectorResult_ = settledResults[0]; + const bm25Result_ = settledResults[1]; + + let vectorResults: RetrievalResult[]; + let bm25Results: RetrievalResult[]; + + if (vectorResult_.status === "rejected") { + const error = attachFailureStage(vectorResult_.reason, "hybrid.vectorSearch"); + console.warn(`[Retriever] vector search failed: ${error.message}`); + vectorResults = []; + } else { + vectorResults = vectorResult_.value; + } + + if (bm25Result_.status === "rejected") { + const error = attachFailureStage(bm25Result_.reason, "hybrid.bm25Search"); + console.warn(`[Retriever] bm25 search failed: ${error.message}`); + bm25Results = []; + } else { + bm25Results = bm25Result_.value; + } + + // Check if BOTH backends failed (rejected), not just empty results + // Empty result sets are valid; only throw when both promises reject + const bothFailed = + vectorResult_.status === "rejected" && bm25Result_.status === "rejected"; + + if (bothFailed) { + const vectorError = vectorResult_.reason?.message || "unknown"; + const bm25Error = bm25Result_.reason?.message || "unknown"; + throw attachFailureStage( + new Error(`both vector and BM25 search failed: ${vectorError}, ${bm25Error}`), + "hybrid.parallelSearch", + ); + } if (diagnostics) { diagnostics.vectorResultCount = vectorResults.length; diagnostics.bm25ResultCount = bm25Results.length; diff --git a/test/query-expander.test.mjs b/test/query-expander.test.mjs index 6035281d..1ad83a2e 100644 --- a/test/query-expander.test.mjs +++ b/test/query-expander.test.mjs @@ -255,7 +255,7 @@ describe("retriever BM25 query expansion gating", () => { }); it("distinguishes vector-search failures inside the hybrid parallel stage", async () => { - const { retriever } = createRetrieverHarness( + const { retriever, bm25Queries } = createRetrieverHarness( {}, { async vectorSearch() { @@ -264,51 +264,70 @@ describe("retriever BM25 query expansion gating", () => { }, ); - await assert.rejects( - retriever.retrieve({ - query: "普通查询", - limit: 1, - source: "manual", - }), - /simulated vector search failure/, - ); + // With graceful degradation, BM25 results should be returned even though vector failed + const results = await retriever.retrieve({ + query: "普通查询", + limit: 1, + source: "manual", + }); + + // Results from BM25 should be returned + assert.ok(results.length > 0); + assert.ok(bm25Queries.length > 0); + // Overall retrieval did not fail (graceful degradation) assert.equal( retriever.getLastDiagnostics()?.failureStage, - "hybrid.vectorSearch", + undefined, ); + // Vector result count should be 0 (failed) assert.equal( - retriever.getLastDiagnostics()?.errorMessage, - "simulated vector search failure", + retriever.getLastDiagnostics()?.vectorResultCount, + 0, + ); + // BM25 result count should be > 0 (succeeded) + assert.ok( + (retriever.getLastDiagnostics()?.bm25ResultCount ?? 0) > 0, ); }); it("distinguishes bm25-search failures inside the hybrid parallel stage", async () => { - const { retriever } = createRetrieverHarness( + const { retriever, bm25Queries } = createRetrieverHarness( {}, { - async bm25Search() { + async bm25Search(query) { + bm25Queries.push(query); throw new Error("simulated bm25 search failure"); }, }, ); - await assert.rejects( - retriever.retrieve({ - query: "普通查询", - limit: 1, - source: "manual", - }), - /simulated bm25 search failure/, - ); + // With graceful degradation, vector results should be returned even though BM25 failed + const results = await retriever.retrieve({ + query: "普通查询", + limit: 1, + source: "manual", + }); + // Results from vector should be returned (empty by default in harness) + assert.equal(results.length, 0); + // BM25 was attempted (even though it failed) + assert.equal(bm25Queries.length, 1); + + // Overall retrieval did not fail (graceful degradation) assert.equal( retriever.getLastDiagnostics()?.failureStage, - "hybrid.bm25Search", + undefined, + ); + // Vector result count should be 0 (empty by default in harness) + assert.equal( + retriever.getLastDiagnostics()?.vectorResultCount, + 0, ); + // BM25 result count should be 0 (failed) assert.equal( - retriever.getLastDiagnostics()?.errorMessage, - "simulated bm25 search failure", + retriever.getLastDiagnostics()?.bm25ResultCount, + 0, ); }); }); diff --git a/test/retriever-graceful-degradation.test.mjs b/test/retriever-graceful-degradation.test.mjs new file mode 100644 index 00000000..9994224b --- /dev/null +++ b/test/retriever-graceful-degradation.test.mjs @@ -0,0 +1,322 @@ +import assert from "node:assert"; +import { describe, it } from "node:test"; +import jitiFactory from "jiti"; + +const jiti = jitiFactory(import.meta.url, { + interopDefault: true, +}); + +const { createRetriever } = jiti("../src/retriever.ts"); + +function buildResult(id = "memory-1", text = "test result") { + return { + entry: { + id, + text, + vector: [0.1, 0.2, 0.3], + category: "other", + scope: "global", + importance: 0.7, + timestamp: 1700000000000, + metadata: "{}", + }, + score: 0.9, + }; +} + +function createRetrieverHarness( + config = {}, + storeOverrides = {}, + embedderOverrides = {}, +) { + const bm25Queries = []; + const embeddedQueries = []; + + const retriever = createRetriever( + { + hasFtsSupport: true, + async vectorSearch() { + return []; + }, + async bm25Search(query) { + bm25Queries.push(query); + return [buildResult()]; + }, + async hasId() { + return true; + }, + async get() { + return null; + }, + async upsert() { + return []; + }, + async delete() { + return; + }, + ...storeOverrides, + }, + { + async embedQuery(query) { + embeddedQueries.push(query); + return [0.1, 0.2, 0.3]; + }, + ...embedderOverrides, + }, + config, + ); + + return { retriever, bm25Queries, embeddedQueries }; +} + +describe("Retriever Graceful Degradation (Promise.allSettled)", () => { + it("throws when both vector and BM25 search reject", async () => { + const { retriever } = createRetrieverHarness( + {}, + { + async vectorSearch() { + throw new Error("vector failed"); + }, + async bm25Search() { + throw new Error("bm25 failed"); + }, + }, + ); + + await assert.rejects( + retriever.retrieve({ query: "test", limit: 1, source: "manual" }), + /both vector and BM25 search failed.*vector failed.*bm25 failed/, + ); + + assert.equal( + retriever.getLastDiagnostics()?.failureStage, + "hybrid.parallelSearch", + ); + assert.equal( + retriever.getLastDiagnostics()?.vectorResultCount, + 0, + ); + assert.equal( + retriever.getLastDiagnostics()?.bm25ResultCount, + 0, + ); + }); + + it("uses vector-only results when BM25 fails", async () => { + const { retriever, bm25Queries } = createRetrieverHarness( + {}, + { + async vectorSearch() { + return [buildResult()]; + }, + async bm25Search(query) { + bm25Queries.push(query); + throw new Error("bm25 failed"); + }, + }, + ); + + const results = await retriever.retrieve({ + query: "test", + limit: 1, + source: "manual", + }); + + assert.equal(results.length, 1); + assert.equal(bm25Queries.length, 1); // BM25 was attempted and failed + assert.equal( + retriever.getLastDiagnostics()?.vectorResultCount, + 1, + ); + assert.equal( + retriever.getLastDiagnostics()?.bm25ResultCount, + 0, + ); + assert.equal( + retriever.getLastDiagnostics()?.failureStage, + null, // No overall failure + ); + }); + + it("uses bm25-only results when vector fails", async () => { + const { retriever, bm25Queries } = createRetrieverHarness( + {}, + { + async vectorSearch() { + throw new Error("vector failed"); + }, + async bm25Search(query) { + bm25Queries.push(query); + return [buildResult()]; + }, + }, + ); + + const results = await retriever.retrieve({ + query: "test", + limit: 1, + source: "manual", + }); + + assert.equal(results.length, 1); + assert.equal(bm25Queries.length, 1); + assert.equal( + retriever.getLastDiagnostics()?.vectorResultCount, + 0, + ); + assert.equal( + retriever.getLastDiagnostics()?.bm25ResultCount, + 1, + ); + assert.equal( + retriever.getLastDiagnostics()?.failureStage, + null, // No overall failure + ); + }); + + it("returns empty results when both backends succeed with no matches", async () => { + const { retriever, bm25Queries } = createRetrieverHarness( + {}, + { + async vectorSearch() { + return []; // ✅ Success, but empty + }, + async bm25Search(query) { + bm25Queries.push(query); + return []; // ✅ Success, but empty + }, + }, + ); + + const results = await retriever.retrieve({ + query: "test", + limit: 1, + source: "manual", + }); + + // Empty result set is valid — should not throw + assert.equal(results.length, 0); + assert.equal( + retriever.getLastDiagnostics()?.vectorResultCount, + 0, + ); + assert.equal( + retriever.getLastDiagnostics()?.bm25ResultCount, + 0, + ); + assert.equal( + retriever.getLastDiagnostics()?.failureStage, + null, // No failure + ); + }); + + it("uses vector-only results when BM25 fails and vector returns empty", async () => { + const { retriever, bm25Queries } = createRetrieverHarness( + {}, + { + async vectorSearch() { + return []; // ✅ Success, but empty + }, + async bm25Search(query) { + bm25Queries.push(query); + throw new Error("bm25 failed"); + }, + }, + ); + + const results = await retriever.retrieve({ + query: "test", + limit: 1, + source: "manual", + }); + + // Empty result from successful vector search is valid + assert.equal(results.length, 0); + assert.equal(bm25Queries.length, 1); + assert.equal( + retriever.getLastDiagnostics()?.vectorResultCount, + 0, + ); + assert.equal( + retriever.getLastDiagnostics()?.bm25ResultCount, + 0, + ); + assert.equal( + retriever.getLastDiagnostics()?.failureStage, + null, // No failure + ); + }); + + it("uses bm25-only results when vector fails and bm25 returns empty", async () => { + const { retriever, bm25Queries } = createRetrieverHarness( + {}, + { + async vectorSearch() { + throw new Error("vector failed"); + }, + async bm25Search(query) { + bm25Queries.push(query); + return []; // ✅ Success, but empty + }, + }, + ); + + const results = await retriever.retrieve({ + query: "test", + limit: 1, + source: "manual", + }); + + // Empty result from successful BM25 search is valid + assert.equal(results.length, 0); + assert.equal(bm25Queries.length, 1); + assert.equal( + retriever.getLastDiagnostics()?.vectorResultCount, + 0, + ); + assert.equal( + retriever.getLastDiagnostics()?.bm25ResultCount, + 0, + ); + assert.equal( + retriever.getLastDiagnostics()?.failureStage, + null, // No failure + ); + }); + + it("normalizes empty results as distinct from both-fail errors", async () => { + const { retriever } = createRetrieverHarness( + {}, + { + async vectorSearch() { + return []; // ✅ Success, empty results + }, + async bm25Search(query) { + return []; // ✅ Success, empty results + }, + }, + ); + + const results = await retriever.retrieve({ + query: "nonexistent", + limit: 10, + source: "manual", + }); + + // This was the bug: empty results were treated as both-fail + // Now empty results should succeed (valid search outcome) + assert.equal(results.length, 0); + assert.equal( + retriever.getLastDiagnostics()?.failureStage, + null, + ); + assert.equal( + retriever.getLastDiagnostics()?.vectorResultCount, + 0, + ); + assert.equal( + retriever.getLastDiagnostics()?.bm25ResultCount, + 0, + ); + }); +});