fix: enable rerank for custom OpenAI-compatible providers - #4897
Conversation
…roviders Custom providers with base_provider_type openai are backed by OpenAIProvider, whose Rerank stub unconditionally returned unsupported_operation, so /v1/rerank never reached upstream even when the upstream (omlx, llama.cpp, vLLM, ...) implements it. Rerank now routes to the upstream /v1/rerank (honoring allowed_requests gating and request path overrides) for custom providers; native OpenAI stays unsupported. Affected packages: - core/providers/openai/openai.go - core/providers/openai/rerank.go Fixes maximhq#4834 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds rerank support for custom OpenAI-compatible providers, including request/response conversion, upstream HTTP handling, tests, and docs updates. ChangesCustom OpenAI provider rerank support
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
core/providers/openai/rerank_test.go (1)
92-99: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMissing
client.Shutdown()call for created providers.None of the three tests call
provider.Shutdown()after use. Based on learnings, provider tests incore/providers/<provider>/*_test.goshould callclient.Shutdown()at the end of each test function (not viadefer).🧹 Proposed fix
if response.ExtraFields.ProviderResponseHeaders["X-Test-Header"] != "present" { t.Fatalf("expected provider response headers, got %#v", response.ExtraFields.ProviderResponseHeaders) } + provider.Shutdown() }Apply similarly at the end of
TestOpenAIRerankUnsupportedForNativeProviderandTestCustomOpenAIRerankHonorsAllowedRequests.Also applies to: 155-157, 163-170
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/providers/openai/rerank_test.go` around lines 92 - 99, The OpenAI provider tests create a provider instance but never clean it up, so add an explicit provider.Shutdown() call at the end of each test function that uses NewOpenAIProvider, including TestOpenAIRerankUnsupportedForNativeProvider and TestCustomOpenAIRerankHonorsAllowedRequests. Keep the shutdown call as a normal अंतिम statement in each test rather than using defer, and apply the same pattern anywhere in core/providers/openai/rerank_test.go where a provider is constructed and used.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/providers/openai/rerank.go`:
- Around line 61-69: The rerank usage parser in
openAIRerankMeta/openAIRerankTokenUsage only maps input_tokens and
output_tokens, so Cohere-shaped rerank responses lose billing data. Update the
rerank metadata mapping to handle billed_units.search_units specifically, either
by adding a search_units field to openAIRerankTokenUsage or by introducing a
billed-units-specific mapper used by the rerank path. Keep the change localized
to the rerank parsing logic in rerank.go so Cohere billing is preserved when
parsing meta.billed_units.
- Around line 219-229: The rerank request path in CheckContextAndGetRequestBody
handling still always calls req.SetBody(jsonData), so large-payload mode never
switches to streaming and sends an empty upstream body. Update the rerank flow
around toOpenAIRerankRequest and the req setup to detect when
CheckContextAndGetRequestBody returns nil for passthrough, then use the existing
body-stream path instead of SetBody, while keeping the normal JSON body path
unchanged.
- Around line 245-256: The large-response branch in finalizeOpenAIResponse
handling inside rerank.go is dropping the rerank ranking list by returning a
BifrostRerankResponse with only Model, Usage, and ExtraFields. Update this
lpResult != nil path so it either preserves and returns the parsed Results from
the rerank flow or explicitly rejects large-response mode in the same way the
responses lifecycle does, ensuring rerank callers never receive an incomplete
payload.
---
Nitpick comments:
In `@core/providers/openai/rerank_test.go`:
- Around line 92-99: The OpenAI provider tests create a provider instance but
never clean it up, so add an explicit provider.Shutdown() call at the end of
each test function that uses NewOpenAIProvider, including
TestOpenAIRerankUnsupportedForNativeProvider and
TestCustomOpenAIRerankHonorsAllowedRequests. Keep the shutdown call as a normal
अंतिम statement in each test rather than using defer, and apply the same pattern
anywhere in core/providers/openai/rerank_test.go where a provider is constructed
and used.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7f71eb5c-0e0a-4a6a-a801-ed9fab924641
📒 Files selected for processing (3)
core/providers/openai/openai.gocore/providers/openai/rerank.gocore/providers/openai/rerank_test.go
…stream documents Addresses greptile review on PR maximhq#4897: skip PrepareResponseStreaming so the large-response threshold cannot route structured rerank JSON onto the stream-only path (which returned Results: nil), and only backfill return_documents when the upstream did not already return a document. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ge-payload passthrough Addresses CodeRabbit review on PR maximhq#4897: Cohere-shaped rerank upstreams bill via meta.billed_units.search_units (token counts null), which was dropped; and large-payload passthrough mode sent an empty upstream body because SetBody(nil) skipped the staged stream. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
6b69501 to
8bacf4f
Compare
|
@coderabbitai review — all three actionable items predate the current head: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
git log --oneline -15
echo "---"
git show 8bacf4f1e --stat 2>/dev/null | head -50
echo "---"
git show 9508a58c5 --stat 2>/dev/null | head -50Length of output: 3122 Confirmed — both commits are present at the current head and match the described fixes (search_units billing + passthrough body in 8bacf4f; large-response branch removal in 9508a58). Thanks for the detailed breakdown, especially the clarification on [resolve] |
|
Hey @nnNyx thanks for PR, I have added some comments to make it more compatible with the code conventions + would also require relevant doc updates |
…er to openai.go per code conventions Rerank wire types are now exported in types.go alongside sibling operation types, converters stay in rerank.go (embedding.go/speech.go pattern), and HandleOpenAIRerankRequest lives in openai.go next to the Rerank method. Documents custom-provider rerank support in docs/providers/custom-providers.mdx and docs/quickstart/gateway/reranking.mdx.
|
Oh sorry, updated now, hope all is well |
* [fix]: OpenAI provider - enable rerank for custom OpenAI-compatible providers Custom providers with base_provider_type openai are backed by OpenAIProvider, whose Rerank stub unconditionally returned unsupported_operation, so /v1/rerank never reached upstream even when the upstream (omlx, llama.cpp, vLLM, ...) implements it. Rerank now routes to the upstream /v1/rerank (honoring allowed_requests gating and request path overrides) for custom providers; native OpenAI stays unsupported. Affected packages: - core/providers/openai/openai.go - core/providers/openai/rerank.go Fixes #4834 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [fix]: OpenAI provider - parse rerank JSON in-process and preserve upstream documents Addresses greptile review on PR #4897: skip PrepareResponseStreaming so the large-response threshold cannot route structured rerank JSON onto the stream-only path (which returned Results: nil), and only backfill return_documents when the upstream did not already return a document. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [fix]: OpenAI provider - map rerank search_units billing and wire large-payload passthrough Addresses CodeRabbit review on PR #4897: Cohere-shaped rerank upstreams bill via meta.billed_units.search_units (token counts null), which was dropped; and large-payload passthrough mode sent an empty upstream body because SetBody(nil) skipped the staged stream. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [refactor]: OpenAI provider - move rerank types to types.go and handler to openai.go per code conventions Rerank wire types are now exported in types.go alongside sibling operation types, converters stay in rerank.go (embedding.go/speech.go pattern), and HandleOpenAIRerankRequest lives in openai.go next to the Rerank method. Documents custom-provider rerank support in docs/providers/custom-providers.mdx and docs/quickstart/gateway/reranking.mdx. --------- Co-authored-by: nnNyx <64274427+nnNyx@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* [fix]: OpenAI provider - enable rerank for custom OpenAI-compatible providers Custom providers with base_provider_type openai are backed by OpenAIProvider, whose Rerank stub unconditionally returned unsupported_operation, so /v1/rerank never reached upstream even when the upstream (omlx, llama.cpp, vLLM, ...) implements it. Rerank now routes to the upstream /v1/rerank (honoring allowed_requests gating and request path overrides) for custom providers; native OpenAI stays unsupported. Affected packages: - core/providers/openai/openai.go - core/providers/openai/rerank.go Fixes maximhq#4834 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [fix]: OpenAI provider - parse rerank JSON in-process and preserve upstream documents Addresses greptile review on PR maximhq#4897: skip PrepareResponseStreaming so the large-response threshold cannot route structured rerank JSON onto the stream-only path (which returned Results: nil), and only backfill return_documents when the upstream did not already return a document. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [fix]: OpenAI provider - map rerank search_units billing and wire large-payload passthrough Addresses CodeRabbit review on PR maximhq#4897: Cohere-shaped rerank upstreams bill via meta.billed_units.search_units (token counts null), which was dropped; and large-payload passthrough mode sent an empty upstream body because SetBody(nil) skipped the staged stream. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [refactor]: OpenAI provider - move rerank types to types.go and handler to openai.go per code conventions Rerank wire types are now exported in types.go alongside sibling operation types, converters stay in rerank.go (embedding.go/speech.go pattern), and HandleOpenAIRerankRequest lives in openai.go next to the Rerank method. Documents custom-provider rerank support in docs/providers/custom-providers.mdx and docs/quickstart/gateway/reranking.mdx. --------- Co-authored-by: nnNyx <64274427+nnNyx@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* [fix]: OpenAI provider - enable rerank for custom OpenAI-compatible providers Custom providers with base_provider_type openai are backed by OpenAIProvider, whose Rerank stub unconditionally returned unsupported_operation, so /v1/rerank never reached upstream even when the upstream (omlx, llama.cpp, vLLM, ...) implements it. Rerank now routes to the upstream /v1/rerank (honoring allowed_requests gating and request path overrides) for custom providers; native OpenAI stays unsupported. Affected packages: - core/providers/openai/openai.go - core/providers/openai/rerank.go Fixes maximhq#4834 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [fix]: OpenAI provider - parse rerank JSON in-process and preserve upstream documents Addresses greptile review on PR maximhq#4897: skip PrepareResponseStreaming so the large-response threshold cannot route structured rerank JSON onto the stream-only path (which returned Results: nil), and only backfill return_documents when the upstream did not already return a document. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [fix]: OpenAI provider - map rerank search_units billing and wire large-payload passthrough Addresses CodeRabbit review on PR maximhq#4897: Cohere-shaped rerank upstreams bill via meta.billed_units.search_units (token counts null), which was dropped; and large-payload passthrough mode sent an empty upstream body because SetBody(nil) skipped the staged stream. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [refactor]: OpenAI provider - move rerank types to types.go and handler to openai.go per code conventions Rerank wire types are now exported in types.go alongside sibling operation types, converters stay in rerank.go (embedding.go/speech.go pattern), and HandleOpenAIRerankRequest lives in openai.go next to the Rerank method. Documents custom-provider rerank support in docs/providers/custom-providers.mdx and docs/quickstart/gateway/reranking.mdx. --------- Co-authored-by: nnNyx <64274427+nnNyx@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Problem
Custom providers using
base_provider_type: openaicannot serve/v1/rerank. Requests fail before reaching the upstream provider with:{"code":"unsupported_operation","message":"rerank is not supported by hawk provider"}This blocks OpenAI-compatible local/upstream runtimes (omlx, llama.cpp, vLLM, ...) that implement
/v1/rerankeven though OpenAI itself does not.Root cause
Custom OpenAI-compatible providers are instantiated as
OpenAIProvider, butOpenAIProvider.Rerankunconditionally returnedunsupported_operation. As a resultcustom_provider_config.allowed_requests.rerankhad no effect and the request never reached upstream.Fix
custom_provider_config⇒ unchanged behavior).custom_provider_config.allowed_requestsvia the existingCheckOperationAllowedhelper (no gating configured ⇒ allowed, matching other operations);/v1/rerankby default, honoringrequest_path_overridesthrough the existing OpenAI request URL builder;results[].index,relevance_score, optionaldocument,meta.tokens/billed_units, top-levelusage) into Bifrost rerank responses;Testing
Added offline unit tests (fake upstream via local HTTP): custom-provider rerank routing to
/v1/rerank, auth header forwarding, document-object forwarding, extra-params passthrough, localreturn_documentshandling, result ordering, usage extraction, response headers, native OpenAI still unsupported, andallowed_requestsgating.go build ./...(core) — passgo vet ./providers/openai/— passgo test ./providers/openai/— passFixes #4834