Skip to content

refactor(sse): type the rerank response adapter's options parameter - #8528

Merged
diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.49from
backryun:chore/ts7-types-rerank-handler
Jul 26, 2026
Merged

diegosouzapw merged 1 commit into
cheaper-inference:release/v3.8.49from
backryun:chore/ts7-types-rerank-handler

Conversation

@backryun

Copy link
Copy Markdown
Contributor

Part of #8484. Type-only, no toolchain changes. First slice into open-sse/handlers — see the scope note at the bottom, because that area turned out not to be what the tracking issue said it was.

One untyped parameter, 12 diagnostics

export function transformResponseFromProvider(providerConfig, data, options = {}) {

No annotation, so TS infers options as {} from the default value. Every read of it fails:

rerank.ts: TS2339: Property 'top_n' does not exist on type '{}'.              x6
           TS2339: Property 'documents' does not exist on type '{}'.          x4
           TS2339: Property 'return_documents' does not exist on type '{}'.   x2

Six of those are top_n alone, because the DeepInfra and Voyage branches each spell it three times in one expression.

Fix

Declared the shape the JSDoc on handleRerank already documents:

type RerankDocument = string | { text?: string };

interface RerankResponseOptions {
  documents?: RerankDocument[];
  return_documents?: boolean;
  top_n?: number;
}

All fields optional — the parameter defaults to {}, and the existing suites call it with subsets ({ documents: ["a", "b"] }).

RerankDocument is a union rather than string[] because the Cohere-compatible API accepts both forms and both adapters already branch on it:

const text = typeof doc === "string" ? doc : doc?.text || "";

providerConfig and data are left unannotated. Only options produced errors — it is the one parameter with a default value, which is what handed TS the {} — so only options is touched.

Verification

280 → 268, zero new errors, on a line-number-agnostic diff of the full tsc error set (with the worktree path normalized out — TS embeds absolute paths in some messages, which otherwise reads as phantom regressions).

  • npm run typecheck:core — clean
  • eslint on both changed files — clean
  • 43/43 across the 6 affected suites (rerank, rerank-providers-5332, rerank-voyage-7809, rerank-openrouter-6574, rerank-proxy-pinning-7350, media-cost-headers-handlers)

Tests — the union had a real hole

Existing coverage of these three options is genuinely good: top_n truncation, descending sort, return_documents: false omitting text, and Voyage's index remapping are all asserted on both adapters. My first read was that there was nothing worth adding.

That was wrong, and the distinction matters. The { text } document form is tested — but only through the request adapter:

  • #5332 deepinfra request adapter → {queries,documents} (string + {text})
  • #7809 voyage request adapter handles {text} object documents

Every response-adapter test passes plain strings. So typeof doc === "string" ? doc : doc?.text on the response side had no coverage — and that branch is precisely what makes RerankDocument a union rather than string. Nothing would have caught a later "simplification" to documents?: string[].

tests/unit/rerank-object-documents-response-path.test.ts, 5 tests:

test pins
deepinfra, { text } documents text resolved from objects, ranking order preserved
deepinfra, mixed array ["plain string", { text: "object form" }] both resolve
deepinfra, {} with no text falls back to "", never undefined
voyage, { text } documents same through the second adapter
voyage, empty { text: "" } filtered index 1 maps back to original index 2

As with the other slices in this campaign these are preservation guards, not TDD red/green — there is no bug. I verified that by reverting rerank.ts to the parent commit and re-running: 5/5 pass there too.

Scope note on open-sse/handlers

#8484 previously described this area as "121 diagnostics — chatCore/stream, standalone, last, heaviest validation". Measured, that is wrong: only 39 are in chatCore.ts. The other 82 are peripheral handlers with no dependency on it — rerank.ts (12), designerWeb.ts (10), audioTranscription.ts (8), responsesHandler.ts (5), and so on. They are sliceable now rather than blocked behind the chatCore work.

They do not share a root cause with each other, though — audioTranscription is mostly TS2769 overloads, imageGeneration is TS2554 arity plus a Buffer → BlobPart mismatch. rerank.ts is the only clean single-cause cluster in the group, which is why it goes first. The rest slice per-file. Issue body updated with the breakdown.

`transformResponseFromProvider(providerConfig, data, options = {})` annotated
nothing, so TS inferred `options` as `{}` from its default value and rejected
every read of it: `top_n` x6, `documents` x4, `return_documents` x2 across the
DeepInfra and Voyage adapters. All 12 of the file's diagnostics, one cause.

Declared `RerankResponseOptions` from the JSDoc that already documents these
fields on handleRerank, with `documents` as `Array<string | { text?: string }>`
— the two forms the Cohere-compatible API accepts and the adapters already
branch on. `providerConfig` and `data` stay unannotated; only `options` was
producing errors and only `options` is touched.

280 -> 268, zero new, on a line-number-agnostic diff of the full tsc error set.

Tests: `{ text }` documents were only ever exercised through the *request*
adapter (cheaper-inference#5332, cheaper-inference#7809) — every response-adapter test passed plain strings, so
the `typeof doc === "string" ? doc : doc?.text` branch on the response side was
uncovered, and that branch is exactly what gives the declared type its union.
Added rerank-object-documents-response-path.test.ts (5 tests): object-form
documents resolved through both adapters, a mixed string/object array, the
`{}`-without-text fallback, and Voyage's index remap across an empty `{ text: "" }`
document. They pass against the parent commit's rerank.ts too — no behavior
change. 43/43 across the 6 rerank/media-cost suites.
@backryun
backryun force-pushed the chore/ts7-types-rerank-handler branch from 9c56d19 to 50cf723 Compare July 25, 2026 21:08
@diegosouzapw
diegosouzapw merged commit 2cf462c into cheaper-inference:release/v3.8.49 Jul 26, 2026
5 checks passed
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Thanks @backryun — merged into release/v3.8.49 via the local merge-train (validated as one combined tree: full test:unit + test:vitest 274/274 on the 32-core box, tip d4b9ce6016). Your commit keeps its authorship. 🚀

@backryun
backryun deleted the chore/ts7-types-rerank-handler branch July 26, 2026 07:02
@diegosouzapw diegosouzapw mentioned this pull request Jul 28, 2026
HouMinXi pushed a commit to HouMinXi/OmniRoute that referenced this pull request Aug 2, 2026
…heaper-inference#8528)

`transformResponseFromProvider(providerConfig, data, options = {})` annotated
nothing, so TS inferred `options` as `{}` from its default value and rejected
every read of it: `top_n` x6, `documents` x4, `return_documents` x2 across the
DeepInfra and Voyage adapters. All 12 of the file's diagnostics, one cause.

Declared `RerankResponseOptions` from the JSDoc that already documents these
fields on handleRerank, with `documents` as `Array<string | { text?: string }>`
— the two forms the Cohere-compatible API accepts and the adapters already
branch on. `providerConfig` and `data` stay unannotated; only `options` was
producing errors and only `options` is touched.

280 -> 268, zero new, on a line-number-agnostic diff of the full tsc error set.

Tests: `{ text }` documents were only ever exercised through the *request*
adapter (cheaper-inference#5332, cheaper-inference#7809) — every response-adapter test passed plain strings, so
the `typeof doc === "string" ? doc : doc?.text` branch on the response side was
uncovered, and that branch is exactly what gives the declared type its union.
Added rerank-object-documents-response-path.test.ts (5 tests): object-form
documents resolved through both adapters, a mixed string/object array, the
`{}`-without-text fallback, and Voyage's index remap across an empty `{ text: "" }`
document. They pass against the parent commit's rerank.ts too — no behavior
change. 43/43 across the 6 rerank/media-cost suites.

Co-authored-by: backryun <busan011@ormbiz.co.kr>
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…heaper-inference#8528)

`transformResponseFromProvider(providerConfig, data, options = {})` annotated
nothing, so TS inferred `options` as `{}` from its default value and rejected
every read of it: `top_n` x6, `documents` x4, `return_documents` x2 across the
DeepInfra and Voyage adapters. All 12 of the file's diagnostics, one cause.

Declared `RerankResponseOptions` from the JSDoc that already documents these
fields on handleRerank, with `documents` as `Array<string | { text?: string }>`
— the two forms the Cohere-compatible API accepts and the adapters already
branch on. `providerConfig` and `data` stay unannotated; only `options` was
producing errors and only `options` is touched.

280 -> 268, zero new, on a line-number-agnostic diff of the full tsc error set.

Tests: `{ text }` documents were only ever exercised through the *request*
adapter (cheaper-inference#5332, cheaper-inference#7809) — every response-adapter test passed plain strings, so
the `typeof doc === "string" ? doc : doc?.text` branch on the response side was
uncovered, and that branch is exactly what gives the declared type its union.
Added rerank-object-documents-response-path.test.ts (5 tests): object-form
documents resolved through both adapters, a mixed string/object array, the
`{}`-without-text fallback, and Voyage's index remap across an empty `{ text: "" }`
document. They pass against the parent commit's rerank.ts too — no behavior
change. 43/43 across the 6 rerank/media-cost suites.

Co-authored-by: backryun <busan011@ormbiz.co.kr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants