diff --git a/changelog.d/fixes/12523-rerank-topk-and-return-documents.md b/changelog.d/fixes/12523-rerank-topk-and-return-documents.md new file mode 100644 index 0000000000..88b1a23726 --- /dev/null +++ b/changelog.d/fixes/12523-rerank-topk-and-return-documents.md @@ -0,0 +1 @@ +- **fix(rerank):** clamp Voyage `top_k` to the documents actually sent after empty-string filtering, and honor `return_documents: false` in the NVIDIA response adapter (#12523 — thanks @pacocartones) diff --git a/open-sse/handlers/rerank.ts b/open-sse/handlers/rerank.ts index 45ab3c2bee..3963f7d154 100644 --- a/open-sse/handlers/rerank.ts +++ b/open-sse/handlers/rerank.ts @@ -73,6 +73,10 @@ function buildAuthHeader(providerConfig, token) { // strings (whitespace-only documents are accepted and ranked upstream). We // filter out exact empty strings and track original indices implicitly via the // response adapter, which reconstructs the map from options.documents (#7809). + // `top_k` is clamped to the number of documents actually sent: the handler + // defaults `top_n` to the caller's *unfiltered* document count, so dropping an + // empty string would otherwise ask Voyage to rank more documents than it got, + // and Voyage rejects `top_k > documents.length` with HTTP 400. // `return_documents` is always forced off upstream: Voyage echoes documents as // plain strings (not Cohere's {text}), so we never rely on the echo — document // text is always synthesized locally from the caller's originals (#7811). @@ -84,7 +88,7 @@ function buildAuthHeader(providerConfig, token) { model: body.model, query: body.query, documents: docTexts, - top_k: body.top_n || docTexts.length, + top_k: Math.min(body.top_n || docTexts.length, docTexts.length), return_documents: false, }; } @@ -101,12 +105,13 @@ function buildAuthHeader(providerConfig, token) { options: RerankResponseOptions = {} ) { if (providerConfig.format === "nvidia") { + const returnDocuments = options.return_documents !== false; return { id: data.id != null ? String(data.id) : `rerank-${Date.now()}`, results: (data.rankings || []).map((r) => ({ index: r.index, relevance_score: r.logit || r.score || 0, - document: { text: r.text || "" }, + ...(returnDocuments ? { document: { text: r.text || "" } } : {}), })), meta: { api_version: { version: "2" }, diff --git a/tests/unit/rerank-providers-5332.test.ts b/tests/unit/rerank-providers-5332.test.ts index 20a0ad74ef..c7e39a1fb9 100644 --- a/tests/unit/rerank-providers-5332.test.ts +++ b/tests/unit/rerank-providers-5332.test.ts @@ -69,3 +69,37 @@ test("#5332 deepinfra response omits document text when return_documents=false", assert.equal(out.results[0].document, undefined); assert.equal(out.results[0].index, 1); }); + +// ─── NVIDIA must honor return_documents like its deepinfra/voyage siblings ── + +test("#5332 nvidia response omits document text when return_documents=false", () => { + const cfg = getRerankProvider("nvidia"); + const out = transformResponseFromProvider( + cfg, + { id: "r1", rankings: [{ index: 0, logit: 0.8, text: "a" }] }, + { documents: ["a"], return_documents: false } + ); + assert.equal(out.results[0].document, undefined); + assert.equal(out.results[0].index, 0); + assert.equal(out.results[0].relevance_score, 0.8); +}); + +test("#5332 nvidia response includes document text when return_documents is true", () => { + const cfg = getRerankProvider("nvidia"); + const out = transformResponseFromProvider( + cfg, + { id: "r1", rankings: [{ index: 1, logit: 0.4, text: "b" }] }, + { documents: ["a", "b"], return_documents: true } + ); + assert.equal(out.results[0].document.text, "b"); +}); + +test("#5332 nvidia response includes document text when return_documents is omitted", () => { + const cfg = getRerankProvider("nvidia"); + const out = transformResponseFromProvider( + cfg, + { id: "r1", rankings: [{ index: 0, logit: 0.9, text: "a" }] }, + { documents: ["a"] } + ); + assert.equal(out.results[0].document.text, "a"); +}); diff --git a/tests/unit/rerank-voyage-7809.test.ts b/tests/unit/rerank-voyage-7809.test.ts index 208fad237c..059c964d25 100644 --- a/tests/unit/rerank-voyage-7809.test.ts +++ b/tests/unit/rerank-voyage-7809.test.ts @@ -223,3 +223,47 @@ test("#7809 voyage response adapter handles empty data array", () => { const out = transformResponseFromProvider(cfg, { data: [] }, { documents: ["a", "b"] }); assert.deepEqual(out.results, []); }); + +// ─── top_k must never exceed the surviving document count ────────────────── +// The handler normalizes `top_n: top_n || documents.length` BEFORE the adapter +// runs, so a caller that omits top_n and sends an exact empty string yields +// top_k > documents.length — which Voyage rejects with HTTP 400. + +test("#7809 voyage request adapter clamps top_k to the surviving document count", () => { + const cfg = getRerankProvider("voyage-ai"); + const out = transformRequestForProvider(cfg, { + model: "rerank-2.5-lite", + query: "teste", + documents: ["a", "", "b"], + // Mirrors the handler's `top_n: top_n || documents.length` when the caller omits top_n. + top_n: 3, + return_documents: true, + }); + assert.deepEqual(out.documents, ["a", "b"]); + assert.equal(out.top_k, 2, "top_k must not exceed the number of documents actually sent"); +}); + +test("#7809 voyage request adapter clamps an explicit oversized top_n", () => { + const cfg = getRerankProvider("voyage-ai"); + const out = transformRequestForProvider(cfg, { + model: "rerank-2.5-lite", + query: "teste", + documents: ["a", "", "", "b"], + top_n: 10, + return_documents: true, + }); + assert.deepEqual(out.documents, ["a", "b"]); + assert.equal(out.top_k, 2); +}); + +test("#7809 voyage request adapter keeps a legitimate top_n below the document count", () => { + const cfg = getRerankProvider("voyage-ai"); + const out = transformRequestForProvider(cfg, { + model: "rerank-2.5-lite", + query: "teste", + documents: ["a", "b", "c"], + top_n: 2, + return_documents: true, + }); + assert.equal(out.top_k, 2); +});