Repository navigation
feat(models): add Qwen3 Embedding 8B and BGE-M3 via DeepInfra - #3188
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughDeepInfra embedding routing and model metadata now support rerank capabilities. Gateway schemas and output validation recognize ChangesDeepInfra rerank support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant GatewayEmbeddings
participant DeepInfra
Client->>GatewayEmbeddings: Submit embedding request
GatewayEmbeddings->>DeepInfra: POST /embeddings with input and model
DeepInfra-->>GatewayEmbeddings: Return embedding response
GatewayEmbeddings-->>Client: Return embedding response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
db559e8 to
9df12d7
Compare
|
Ran the e2e suite against this branch ( 1. Blocker: the gateway no longer compilesAdding
2.
|
b6a6cbb to
8c69131
Compare
|
Re-ran the round on the updated branch (8c69131) — all three earlier issues are resolved and the scoped e2e suite is green.
One heads-up, not a blocker: during a first run the embeddings test timed out because DeepInfra was returning |
|
One last thing before merge: the |
8c69131 to
ff5747c
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
I have tried separately with DeepInfra's API key the Qwen3 8B embedded model, and it's also often buggy and not working. I have switched to BAAI/bge-m3 that always worked as in 99.99%. I think I will add that instead. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/gateway/src/embeddings/embeddings.ts (1)
899-916: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid duplicating the embedding request-body builder.
This branch duplicates the
input,model,encoding_format,dimensions, anduserconstruction that follows at Lines 918-931. Keep the provider-specific difference limited to URL selection and build the body once, preventing the two paths from drifting.As per coding guidelines, apply DRY principles for code reuse.
🤖 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 `@apps/gateway/src/embeddings/embeddings.ts` around lines 899 - 916, Update the isDeepInfra branch in the embeddings request flow to set only the provider-specific upstreamUrl, then remove its duplicated requestBody construction. Reuse the shared builder after the branch so input, model, encoding_format, dimensions, and user are assembled once for both paths.Source: Coding guidelines
🤖 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 `@packages/models/src/models/alibaba.ts`:
- Around line 2937-2959: Update the Qwen3 Reranker 8B model entry in the Alibaba
model definitions to include the appropriate deactivatedAt value, keeping it
inactive until the /v1/rerank endpoint is available. Preserve the existing model
metadata and provider configuration.
---
Nitpick comments:
In `@apps/gateway/src/embeddings/embeddings.ts`:
- Around line 899-916: Update the isDeepInfra branch in the embeddings request
flow to set only the provider-specific upstreamUrl, then remove its duplicated
requestBody construction. Reuse the shared builder after the branch so input,
model, encoding_format, dimensions, and user are assembled once for both paths.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8957e085-493e-4746-97bb-2f94edd49613
📒 Files selected for processing (7)
apps/gateway/src/chat-helpers.e2e.tsapps/gateway/src/embeddings/embeddings.tsapps/gateway/src/lib/validate-model-output.tsapps/gateway/src/models/models.tspackages/models/src/model-metadata.spec.tspackages/models/src/models.tspackages/models/src/models/alibaba.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@packages/models/src/models/baai.ts`:
- Line 17: Update the outputPrice value in the model definition to use the
per-token zero notation "0e-6" instead of "0", preserving the existing string
type and surrounding configuration.
- Line 8: Update the BAAI model description in the model metadata to remove the
claim that it supports the `dimensions` parameter, while preserving the
remaining capabilities and description.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0122ad4f-2555-40c7-bd83-0d8820c28a62
📒 Files selected for processing (2)
packages/models/src/models.tspackages/models/src/models/baai.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/models/src/models.ts
4fb3a3d to
a1d7546
Compare
|
One structural request before this merges: please either drop As merged, the reranker would be a catalog entry with zero working code path: calling it on chat completions 400s pointing at
Both directions are fine by us:
Which way do you want to go? |
|
I am choosing option 1. thx for comment |
09f7b77 to
5a5bdad
Compare
40f04b0 to
3c28199
Compare
|
Re-verified the updated branch (3c28199, rerank removed, BGE-M3 added) — everything is green. Both embedding models return valid vectors end-to-end through the gateway's |
3c28199 to
b5a2010
Compare
Add two embedding model definitions served through the DeepInfra provider and fix the embeddings upstream URL for DeepInfra. - Add a DeepInfra provider mapping to the existing Qwen3 Embedding 8B model (32K context, 4096-dim output, MRL support, $0.010/1M tokens) - Add BGE-M3 as a new BAAI family entry (8K context, 1024-dim output, dense/sparse/multi-vector retrieval, $0.010/1M tokens) - Fix the DeepInfra embeddings upstream URL to use /embeddings instead of /v1/embeddings so it does not duplicate the /v1/openai segment already present in DeepInfra's base URL Co-Authored-By: Claude <noreply@anthropic.com>
c5317a5 to
4bc15ee
Compare
Summary
Adds two embedding model definitions served through the DeepInfra provider:
Qwen3 Embedding 8B
/v1/embeddingsendpointBGE-M3
DeepInfra embeddings path fix
https://api.deepinfra.com/v1/openai, so the embeddings handler was producing…/v1/openai/v1/embeddingswhich 404s. This PR adds a DeepInfra-specific branch that uses/embeddingsdirectly.Files changed
packages/models/src/models/alibaba.tspackages/models/src/models/baai.tspackages/models/src/models.tsapps/gateway/src/embeddings/embeddings.ts🤖 Generated with Claude Code