Skip to content

Commit a583e32

Browse files
ether-btcCharon
andauthored
fix: hybrid retrieval graceful degradation with Promise.allSettled (#581)
* fix: hybrid retrieval graceful degradation with Promise.allSettled Replace Promise.all with Promise.allSettled in hybridRetrieval() so that if either vector or BM25 search throws, the other search's results are still used instead of failing the entire query. Behavior: - One search fails → other search results returned, failed side logged - Both fail → throws with failureStage=hybrid.parallelSearch - Diagnostics correctly reflect 0 for failed search counts Fixes Bug 3 from community code audit. * Fix both-backend-fail guard: check rejection status, not array length Resolves reviewer rwmjhb's F1 and MR1 feedback: **F1: Fix both-fail guard** - Changed from checking `results.length === 0` to checking `settledResults.status === "rejected"` for both backends - Empty result sets are now correctly distinguished from failures - Error message enriched with rejection reasons for debugging **MR1: Update existing tests + add new test coverage** - Updated 2 existing tests in query-expander.test.mjs to reflect graceful-degradation behavior (no longer throws on partial failures) - Added new test file: retriever-graceful-degradation.test.mjs with 7 comprehensive tests covering: - Both backends fail → throws error - One fails, one succeeds → uses successful backend results - Both succeed with empty results → returns empty array - Empty results vs. both-fail distinction **SE-approved implementation** (3 conditions verified): 1. Fallback behavior confirmed: fuseResults() handles partial failures 2. Type safety: Promise.allSettled pattern maintained 3. Error enrichment: rejection reasons included in error message All 20 tests passing (7 new + 13 existing). --------- Co-authored-by: Charon <charon@openclaw.ai>
1 parent 619d703 commit a583e32

File tree

4 files changed

+405
-33
lines changed

4 files changed

+405
-33
lines changed

package-lock.json

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/retriever.ts

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -920,24 +920,56 @@ export class MemoryRetriever {
920920

921921
trace?.startStage("parallel_search", []);
922922
failureStage = "hybrid.parallelSearch";
923-
const [vectorResults, bm25Results] = await Promise.all([
923+
const settledResults = await Promise.allSettled([
924924
this.runVectorSearch(
925925
queryVector,
926926
candidatePoolSize,
927927
scopeFilter,
928928
category,
929-
).catch((error) => {
930-
throw attachFailureStage(error, "hybrid.vectorSearch");
931-
}),
929+
),
932930
this.runBM25Search(
933931
bm25Query,
934932
candidatePoolSize,
935933
scopeFilter,
936934
category,
937-
).catch((error) => {
938-
throw attachFailureStage(error, "hybrid.bm25Search");
939-
}),
935+
),
940936
]);
937+
938+
const vectorResult_ = settledResults[0];
939+
const bm25Result_ = settledResults[1];
940+
941+
let vectorResults: RetrievalResult[];
942+
let bm25Results: RetrievalResult[];
943+
944+
if (vectorResult_.status === "rejected") {
945+
const error = attachFailureStage(vectorResult_.reason, "hybrid.vectorSearch");
946+
console.warn(`[Retriever] vector search failed: ${error.message}`);
947+
vectorResults = [];
948+
} else {
949+
vectorResults = vectorResult_.value;
950+
}
951+
952+
if (bm25Result_.status === "rejected") {
953+
const error = attachFailureStage(bm25Result_.reason, "hybrid.bm25Search");
954+
console.warn(`[Retriever] bm25 search failed: ${error.message}`);
955+
bm25Results = [];
956+
} else {
957+
bm25Results = bm25Result_.value;
958+
}
959+
960+
// Check if BOTH backends failed (rejected), not just empty results
961+
// Empty result sets are valid; only throw when both promises reject
962+
const bothFailed =
963+
vectorResult_.status === "rejected" && bm25Result_.status === "rejected";
964+
965+
if (bothFailed) {
966+
const vectorError = vectorResult_.reason?.message || "unknown";
967+
const bm25Error = bm25Result_.reason?.message || "unknown";
968+
throw attachFailureStage(
969+
new Error(`both vector and BM25 search failed: ${vectorError}, ${bm25Error}`),
970+
"hybrid.parallelSearch",
971+
);
972+
}
941973
if (diagnostics) {
942974
diagnostics.vectorResultCount = vectorResults.length;
943975
diagnostics.bm25ResultCount = bm25Results.length;

test/query-expander.test.mjs

Lines changed: 44 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ describe("retriever BM25 query expansion gating", () => {
255255
});
256256

257257
it("distinguishes vector-search failures inside the hybrid parallel stage", async () => {
258-
const { retriever } = createRetrieverHarness(
258+
const { retriever, bm25Queries } = createRetrieverHarness(
259259
{},
260260
{
261261
async vectorSearch() {
@@ -264,51 +264,70 @@ describe("retriever BM25 query expansion gating", () => {
264264
},
265265
);
266266

267-
await assert.rejects(
268-
retriever.retrieve({
269-
query: "普通查询",
270-
limit: 1,
271-
source: "manual",
272-
}),
273-
/simulated vector search failure/,
274-
);
267+
// With graceful degradation, BM25 results should be returned even though vector failed
268+
const results = await retriever.retrieve({
269+
query: "普通查询",
270+
limit: 1,
271+
source: "manual",
272+
});
273+
274+
// Results from BM25 should be returned
275+
assert.ok(results.length > 0);
276+
assert.ok(bm25Queries.length > 0);
275277

278+
// Overall retrieval did not fail (graceful degradation)
276279
assert.equal(
277280
retriever.getLastDiagnostics()?.failureStage,
278-
"hybrid.vectorSearch",
281+
undefined,
279282
);
283+
// Vector result count should be 0 (failed)
280284
assert.equal(
281-
retriever.getLastDiagnostics()?.errorMessage,
282-
"simulated vector search failure",
285+
retriever.getLastDiagnostics()?.vectorResultCount,
286+
0,
287+
);
288+
// BM25 result count should be > 0 (succeeded)
289+
assert.ok(
290+
(retriever.getLastDiagnostics()?.bm25ResultCount ?? 0) > 0,
283291
);
284292
});
285293

286294
it("distinguishes bm25-search failures inside the hybrid parallel stage", async () => {
287-
const { retriever } = createRetrieverHarness(
295+
const { retriever, bm25Queries } = createRetrieverHarness(
288296
{},
289297
{
290-
async bm25Search() {
298+
async bm25Search(query) {
299+
bm25Queries.push(query);
291300
throw new Error("simulated bm25 search failure");
292301
},
293302
},
294303
);
295304

296-
await assert.rejects(
297-
retriever.retrieve({
298-
query: "普通查询",
299-
limit: 1,
300-
source: "manual",
301-
}),
302-
/simulated bm25 search failure/,
303-
);
305+
// With graceful degradation, vector results should be returned even though BM25 failed
306+
const results = await retriever.retrieve({
307+
query: "普通查询",
308+
limit: 1,
309+
source: "manual",
310+
});
304311

312+
// Results from vector should be returned (empty by default in harness)
313+
assert.equal(results.length, 0);
314+
// BM25 was attempted (even though it failed)
315+
assert.equal(bm25Queries.length, 1);
316+
317+
// Overall retrieval did not fail (graceful degradation)
305318
assert.equal(
306319
retriever.getLastDiagnostics()?.failureStage,
307-
"hybrid.bm25Search",
320+
undefined,
321+
);
322+
// Vector result count should be 0 (empty by default in harness)
323+
assert.equal(
324+
retriever.getLastDiagnostics()?.vectorResultCount,
325+
0,
308326
);
327+
// BM25 result count should be 0 (failed)
309328
assert.equal(
310-
retriever.getLastDiagnostics()?.errorMessage,
311-
"simulated bm25 search failure",
329+
retriever.getLastDiagnostics()?.bm25ResultCount,
330+
0,
312331
);
313332
});
314333
});

0 commit comments

Comments
 (0)