mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-17 12:22:34 +03:00
fix(rerank): clamp Voyage top_k and honor NVIDIA return_documents (#12523)
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.
This commit is contained in:
@@ -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)
|
||||
@@ -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" },
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user