From afd2d993e632637683ab62403f4c5e9b38e04ca5 Mon Sep 17 00:00:00 2001 From: Paco Cartones <253313177+pacocartones@users.noreply.github.com> Date: Fri, 11 Sep 2026 22:42:21 +0200 Subject: [PATCH] fix(rerank): clamp Voyage top_k and honor NVIDIA return_documents (#12523) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both defects are real adapter bugs: `top_k` computed from the unfiltered array after the adapter drops empty strings makes Voyage reject a request that is valid under the Cohere-style contract this endpoint exposes. Good that the NVIDIA `return_documents` half rides along rather than waiting. --- Validated in one consolidated worktree cut from `release/v3.8.51`, boarded with the rest of this batch — zero conflicts between the 19 PRs. - `typecheck:core` clean; `check:dashboard-typecheck` OK (206 pre-existing, all within the frozen baseline); `check:changelog-integrity` OK - complexity 2802 / baseline 3218 and cognitive-complexity 1267 / baseline 1437 — both under baseline - 226 of 228 focused assertions green across the batch's 23 test files. The 2 remaining belong to #12551, which is held separately. Two batch-owned defects were found and fixed in flight, both pure base drift: `173_xp_action_counts.sql` collided with `173_call_logs_video_content_removed.sql` (renumbered to 176 on #12651 — it aborted every DB open, which is what 53 of the first run's failures were), and the feature-flag catalog was missing the `SERVER_OWNED_TOOL_LOOP_ENABLED` row the base gained after #12552 was written. ⚠️ base-red inherited: #12732 — `Docs Gates`, `Merge integrity`, `No new ESLint warnings`, `Unit Tests fast-path` and `Fast Quality Gates` reproduce on the pure tip (provider count 356 vs the 358 the modules define, SKILL.md drift, and `open-sse/utils/stream.ts` at 3115 > frozen 3098, which this batch does not touch). Thanks @pacocartones — the `file:line` citations and the explicit out-of-scope notes on every one of these made a 19-PR batch reviewable in one pass. --- .../12523-rerank-topk-and-return-documents.md | 1 + open-sse/handlers/rerank.ts | 9 +++- tests/unit/rerank-providers-5332.test.ts | 34 ++++++++++++++ tests/unit/rerank-voyage-7809.test.ts | 44 +++++++++++++++++++ 4 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 changelog.d/fixes/12523-rerank-topk-and-return-documents.md 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); +});