UN-3503 [FEAT] Add NVIDIA Build and OpenRouter OpenAI-compatible LLM and embedding adapters#2004
Merged
Merged
Conversation
…adapters Both reuse the existing OpenAI Compatible adapter's validation logic with their API endpoints hard-coded, so users only pick a model and supply an API key: - NVIDIA Build -> https://integrate.api.nvidia.com/v1 - OpenRouter -> https://openrouter.ai/api/v1 Adds branded parameter subclasses (sharing one validate helper), thin adapter classes, branded JSON schemas (api_base hidden), and logos. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
|
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:
WalkthroughAdds NVIDIA Build and OpenRouter LLM adapters, OpenAI-compatible embedding adapters, shared validation helpers that inject branded api_base and normalize models, JSON schemas for each adapter, package exports, and tests validating registration and parameter behavior. ChangesBranded OpenAI-Compatible LLM Adapters
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
chandrasekharan-zipstack
requested review from
jaseemjaskp,
johnyrahul and
pk-zipstack
Contributor
|
| Filename | Overview |
|---|---|
| unstract/sdk1/src/unstract/sdk1/adapters/base1.py | Adds NvidiaBuildLLMParameters, OpenRouterLLMParameters, NvidiaBuildEmbeddingParameters, and OpenAICompatibleEmbeddingParameters; api_base fallback injection, OpenRouter reasoning idempotency, and encoding_format pinning all look correct. |
| unstract/sdk1/src/unstract/sdk1/embedding.py | Extracts _prepare_call() helper and threads input_type through all four embedding call sites; pops embed_batch_size in init so it never reaches LiteLLM. |
| unstract/sdk1/tests/test_branded_openai_adapters.py | Comprehensive test suite for new adapters, but test_compatible_embedding_omits_input_type never calls get_embedding() so the mock is never invoked and captured["model"] raises KeyError. |
| unstract/sdk1/src/unstract/sdk1/adapters/llm1/static/nvidia_build.json | New branded LLM schema with conditional reasoning fields; uses non-standard "format": "url" instead of "uri" for api_base. |
| unstract/sdk1/src/unstract/sdk1/adapters/llm1/static/openrouter.json | New branded LLM schema with reasoning toggle; same "format": "url" inconsistency as nvidia_build.json. |
| unstract/sdk1/src/unstract/sdk1/adapters/llm1/nvidia_build.py | Thin adapter shell for NVIDIA Build LLM; clean and follows existing adapter conventions exactly. |
| unstract/sdk1/src/unstract/sdk1/adapters/llm1/openrouter.py | Thin adapter shell for OpenRouter LLM; clean, consistent with other adapters. |
| unstract/sdk1/src/unstract/sdk1/adapters/embedding1/nvidia_build.py | Thin adapter shell for NVIDIA Build embedding; routes through nvidia_nim/ prefix for native LiteLLM cost resolution. |
| unstract/sdk1/src/unstract/sdk1/adapters/embedding1/openai_compatible.py | New generic OpenAI-compatible embedding adapter; routes through openai/ prefix, supports keyless gateways via _NO_AUTH_API_KEY placeholder. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph LLM_Adapters[LLM Adapters]
NB_LLM[NvidiaBuildLLMAdapter] -->|validate| VBOC[_validate_branded_openai_compatible]
VBOC -->|injects api_base, delegates| OAC_LLM[OpenAICompatibleLLMParameters.validate]
OAC_LLM -->|model prefix custom_openai| LiteLLM_C[LiteLLM custom_openai provider]
OR_LLM[OpenRouterLLMAdapter] -->|validate| OR_P[OpenRouterLLMParameters.validate]
OR_P -->|model prefix openrouter| LiteLLM_OR[LiteLLM openrouter provider]
end
subgraph Embedding_Adapters[Embedding Adapters]
NB_EMB[NvidiaBuildEmbeddingAdapter] -->|validate| NB_EP[NvidiaBuildEmbeddingParameters.validate]
NB_EP -->|model prefix nvidia_nim, encoding_format float| LiteLLM_NIM[LiteLLM nvidia_nim provider]
OAC_EMB[OpenAICompatibleEmbeddingAdapter] -->|validate| OAC_EP[OpenAICompatibleEmbeddingParameters.validate]
OAC_EP -->|model prefix openai, keyless placeholder| LiteLLM_OA[LiteLLM openai provider]
end
subgraph embedding_py[embedding.py]
NB_EMB -->|get_embedding/get_embeddings| PrepCall[_prepare_call injects input_type for nvidia_nim]
OAC_EMB -->|get_embedding/get_embeddings| PrepCall
PrepCall -->|input_type query or passage| LiteLLM_E[litellm.embedding]
end
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
unstract/sdk1/tests/test_branded_openai_adapters.py:301-311
The test constructs an `Embedding` object but never calls `get_embedding()` or `get_embeddings()`, so `litellm.embedding` is never reached, `captured` stays empty, and `captured["model"]` raises `KeyError`. The assertion `"input_type" not in captured` passes trivially on an empty dict, giving false confidence. A `get_embedding("test")` call is needed to actually exercise the mock path.
```suggestion
captured = _patch_capture_embedding(monkeypatch)
emb = emb_mod.Embedding(
adapter_id=OpenAICompatibleEmbeddingAdapter.get_id(),
adapter_metadata={
"model": "BAAI/bge-m3",
"api_base": "https://gw.example/v1",
"api_key": "k",
},
)
emb.get_embedding("test")
assert "input_type" not in captured
assert captured["model"] == "openai/BAAI/bge-m3"
```
### Issue 2 of 3
unstract/sdk1/src/unstract/sdk1/adapters/llm1/static/nvidia_build.json:27-33
`"format": "url"` is not a registered JSON Schema format keyword; the correct standard value is `"uri"` (RFC 3986). Most validators silently ignore unknown formats, so URL validation won't fire. The companion embedding schemas already use `"uri"` consistently.
```suggestion
"api_base": {
"type": "string",
"format": "uri",
"title": "API Base",
"default": "https://integrate.api.nvidia.com/v1",
"description": "NVIDIA Build endpoint. Pre-filled with the default; change only if NVIDIA moves the base URL."
},
```
### Issue 3 of 3
unstract/sdk1/src/unstract/sdk1/adapters/llm1/static/openrouter.json:27-33
`"format": "url"` is not a registered JSON Schema format keyword; the correct standard value is `"uri"`. Same inconsistency as `nvidia_build.json` — the embedding-side schemas already use `"uri"` correctly.
```suggestion
"api_base": {
"type": "string",
"format": "uri",
"title": "API Base",
"default": "https://openrouter.ai/api/v1",
"description": "OpenRouter endpoint. Pre-filled with the default; change only if OpenRouter moves the base URL."
},
```
Reviews (11): Last reviewed commit: "Merge branch 'main' into feat/branded-op..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
johnyrahul
approved these changes
Jun 1, 2026
Routes through LiteLLM's native nvidia_nim provider (custom_openai has no embedding support), which reuses the OpenAI embedding handler and resolves the integrate.api.nvidia.com endpoint automatically. Reuses OpenAIEmbeddingParameters with the model prefixed nvidia_nim/ and the endpoint pinned. Scaffolded via the adapter-ops skill. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@unstract/sdk1/src/unstract/sdk1/adapters/base1.py`:
- Around line 1353-1383: The comment should be updated to clarify that LiteLLM's
nvidia_nim provider defaults to https://integrate.api.nvidia.com/v1 and that
this adapter intentionally pins api_base to that same value; update the comment
near _NVIDIA_NIM_PROVIDER_PREFIX and the NvidiaBuildEmbeddingParameters class
(and its validate method) to state that _NVIDIA_BUILD_API_BASE equals the
provider default and validate() sets api_base to that pinned default for clarity
and to remove the implication of a runtime risk.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ea00eb6e-fad0-438c-a5b6-f086b2e9a6fd
📒 Files selected for processing (4)
unstract/sdk1/src/unstract/sdk1/adapters/base1.pyunstract/sdk1/src/unstract/sdk1/adapters/embedding1/__init__.pyunstract/sdk1/src/unstract/sdk1/adapters/embedding1/nvidia_build.pyunstract/sdk1/src/unstract/sdk1/adapters/embedding1/static/nvidia_build.json
…ing adapter, tests - Expose api_base (pre-filled with the default) in the NVIDIA Build and OpenRouter LLM adapters and the NVIDIA Build embedding adapter, so users can repoint if a provider moves its base URL. Validation honours an override and falls back to the default when blank. - Add a generic OpenAI Compatible embedding adapter. LiteLLM has no custom_openai embedding handler, so it routes via the openai/ provider with a user-supplied api_base (api_key optional, api_base required). - Add unit tests covering registration, model prefixing, api_base default/override, and routing for all new adapters. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- NvidiaBuild/OpenRouter LLM and NvidiaBuild embedding params now declare api_base as a required str defaulting to the provider URL (was str | None = None), so a directly-constructed instance is always valid even without going through validate() (greptile). - Clarify the NVIDIA embedding comment: nvidia_nim already defaults api_base to the same integrate.api.nvidia.com URL; we pin it as an overridable default, not to mitigate a runtime risk (coderabbit). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Addressed review comments in 8fa9405:
Verified: direct construction now yields valid endpoints for all three; adapter unit tests green (39 passed). |
Contributor
|
Tip For best results, initiate chat on the files or code changes. The PR description image shows the "Add LLM" provider selection dialog with NVIDIA Build and OpenRouter both visible and correctly rendering with their respective icons in the grid alongside other providers.
All green on 39 adapter tests is reassuring. 🐇✨ If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@unstract/sdk1/src/unstract/sdk1/adapters/base1.py`:
- Around line 1410-1419: The validate method currently converts a blank api_key
string to None which then gets forwarded into
OpenAICompatibleEmbeddingParameters and returned by model_dump(), causing the
client to receive api_key=None; instead, detect blank api_key in validate (the
static method) and remove it from adapter_metadata (e.g., pop "api_key") so it
is omitted entirely, or ensure you call
OpenAICompatibleEmbeddingParameters.model_dump(exclude_none=True) before
returning so no explicit null api_key is forwarded; update the validate flow
around adapter_metadata, api_key, and the final model_dump usage to omit the
api_key when blank rather than setting it to None.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d91a0760-35c7-405d-93fb-c4b1ada7665d
📒 Files selected for processing (8)
unstract/sdk1/src/unstract/sdk1/adapters/base1.pyunstract/sdk1/src/unstract/sdk1/adapters/embedding1/__init__.pyunstract/sdk1/src/unstract/sdk1/adapters/embedding1/openai_compatible.pyunstract/sdk1/src/unstract/sdk1/adapters/embedding1/static/custom_openai.jsonunstract/sdk1/src/unstract/sdk1/adapters/embedding1/static/nvidia_build.jsonunstract/sdk1/src/unstract/sdk1/adapters/llm1/static/nvidia_build.jsonunstract/sdk1/src/unstract/sdk1/adapters/llm1/static/openrouter.jsonunstract/sdk1/tests/test_branded_openai_adapters.py
🚧 Files skipped from review as they are similar to previous changes (4)
- unstract/sdk1/src/unstract/sdk1/adapters/llm1/static/openrouter.json
- unstract/sdk1/src/unstract/sdk1/adapters/embedding1/init.py
- unstract/sdk1/src/unstract/sdk1/adapters/embedding1/static/nvidia_build.json
- unstract/sdk1/src/unstract/sdk1/adapters/llm1/static/nvidia_build.json
Contributor
|
Both fixes look correct:
One unrelated issue still open in this PR (CodeRabbit's comment on Tip: You can customize Greptile's behavior for this repo with |
- OpenRouter now routes through LiteLLM's native openrouter/ provider instead of the generic custom_openai path, so per-token cost resolves for the OpenRouter models LiteLLM prices (~95 popular models) and reasoning params map natively. Dedicated OpenRouterLLMParameters forwards reasoning_effort only when reasoning is enabled and drops temperature on that path (OpenAI o-series reject non-default temperature). NVIDIA Build stays on custom_openai (LiteLLM ships no nvidia pricing either way). - Add clickable markdown doc links in NVIDIA Build and OpenRouter help text (build.nvidia.com, openrouter.ai/keys, openrouter.ai/models) and correct the OpenRouter reasoning help text for native handling. - Update tests for native OpenRouter routing + reasoning passthrough. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
unstract/sdk1/tests/test_branded_openai_adapters.py (1)
180-182:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNarrow the expected exception type to
pydantic.ValidationError.
OpenAICompatibleEmbeddingParameters.validate()instantiates a Pydantic model withapi_base: strrequired, so omittingapi_baseraisespydantic.ValidationError(not just any exception).♻️ Proposed refactor
+from pydantic import ValidationError + def test_compatible_embedding_requires_api_base() -> None: - with pytest.raises(Exception): # noqa: B017 - pydantic ValidationError + with pytest.raises(ValidationError): OpenAICompatibleEmbeddingParameters.validate({"model": "m"})🤖 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 `@unstract/sdk1/tests/test_branded_openai_adapters.py` around lines 180 - 182, The test currently expects any Exception but should assert the specific pydantic.ValidationError: update the pytest.raises in test_compatible_embedding_requires_api_base to expect pydantic.ValidationError instead of Exception (use OpenAICompatibleEmbeddingParameters.validate(...) as the call) and add/import ValidationError from pydantic so the test asserts the precise error type.
🤖 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.
Outside diff comments:
In `@unstract/sdk1/tests/test_branded_openai_adapters.py`:
- Around line 180-182: The test currently expects any Exception but should
assert the specific pydantic.ValidationError: update the pytest.raises in
test_compatible_embedding_requires_api_base to expect pydantic.ValidationError
instead of Exception (use OpenAICompatibleEmbeddingParameters.validate(...) as
the call) and add/import ValidationError from pydantic so the test asserts the
precise error type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 596c9d82-8e53-4173-8e38-f72260beefa2
📒 Files selected for processing (5)
unstract/sdk1/src/unstract/sdk1/adapters/base1.pyunstract/sdk1/src/unstract/sdk1/adapters/embedding1/static/nvidia_build.jsonunstract/sdk1/src/unstract/sdk1/adapters/llm1/static/nvidia_build.jsonunstract/sdk1/src/unstract/sdk1/adapters/llm1/static/openrouter.jsonunstract/sdk1/tests/test_branded_openai_adapters.py
✅ Files skipped from review due to trivial changes (1)
- unstract/sdk1/src/unstract/sdk1/adapters/embedding1/static/nvidia_build.json
🚧 Files skipped from review as they are similar to previous changes (3)
- unstract/sdk1/src/unstract/sdk1/adapters/llm1/static/openrouter.json
- unstract/sdk1/src/unstract/sdk1/adapters/llm1/static/nvidia_build.json
- unstract/sdk1/src/unstract/sdk1/adapters/base1.py
…uild LiteLLM sends encoding_format=null when unset; NVIDIA Build (and other strict OpenAI-compatible embedding gateways) reject null, demanding 'float' or 'base64'. Default it to 'float' in the NVIDIA Build and generic OpenAI-compatible embedding validators. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
embed_batch_size is a llama-index client-side batching hint, not an API field, yet it was model-dumped into the litellm.embedding kwargs. LiteLLM's nvidia_nim handler dumps unknown kwargs into the request body, so NVIDIA Build rejected it with a 400 (extra_forbidden). It also never drove any batching: indexing uses llama-index's own embed_batch_size default. Strip it centrally in Embedding.__init__ (fixes every provider) and drop the now-inert field from the NVIDIA Build and generic OpenAI-compatible embedding schemas. Existing adapters are unaffected (Pydantic ignores the stale key). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
NVIDIA's retrieval embedding models (nv-embedqa*, nv-embedcode) are asymmetric and require an input_type of "query" or "passage"; without it they return a 400. Thread input_type through the embedding call path — "query" for query embeddings (and the test-connection snippet), "passage" for documents — gated to nvidia_nim models so other providers, which reject the field, are unaffected. litellm forwards it via extra_body. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
embed_batch_size is stripped before the litellm call, so the schema field was a control that did nothing. Remove it from the remaining embedding schemas (openai, azure, ollama, gemini, vertexai) for consistency with the branded adapters. Also tighten the related code comments. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Trim the comments and docstrings added in this PR to single-line WHY notes, dropping provider-specific and implementation-detail narration. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
chandrasekharan-zipstack
changed the title
Deepak-Kesavan
approved these changes
Jun 2, 2026
…r reasoning reload - OpenAI-compatible embedding: blank api_key normalized to a non-empty placeholder instead of None, since the OpenAI SDK rejects a null key before reaching keyless gateways (coderabbit). - OpenRouter: infer enable_reasoning from a surviving reasoning_effort on re-validation so reloading a saved reasoning config no longer silently disables it (greptile P1). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Frontend Lint Report (Biome)✅ All checks passed! No linting or formatting issues found. |
|
Contributor
Unstract test resultsPer-group results
Critical paths
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds two new branded OpenAI-compatible LLM adapters:
https://integrate.api.nvidia.com/v1https://openrouter.ai/api/v1Both behave exactly like the existing OpenAI Compatible adapter, but with the API base preconfigured so users only pick a model and supply an API key.
Why
These providers expose an OpenAI-compatible API. A generic "OpenAI Compatible" adapter already covers them, but dedicated, branded entries (name + logo + no URL to type) give a cleaner, more discoverable UX for these popular providers.
How
base1.py:NvidiaBuildLLMParametersandOpenRouterLLMParameterssubclassOpenAICompatibleLLMParameters. A shared_validate_branded_openai_compatible()helper forces the provider's hard-codedapi_basethen delegates to the existing compatible-adapter validation — so model prefixing (custom_openai/), reasoning/extra_bodyhandling, and idempotent re-validation are all reused, not duplicated.llm1/nvidia_build.py,llm1/openrouter.py: thin adapter classes (id, metadata, provider, icon).static/nvidia_build.json,static/openrouter.json: branded schemas — copies ofcustom_openai.jsonwithapi_baseremoved (hidden/hard-coded) and provider-specific model examples.llm1/__init__.py.Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
No. Purely additive — two new adapter modules + schemas + logos and two export lines. No existing adapter, shared logic, or signature is modified; the new params subclass the existing one and only inject a hard-coded
api_base. Verified all 13 adapters still register and validate.Database Migrations
Env Config
Relevant Docs
Related Issues or PRs
Dependencies Versions
Notes on Testing
Verified locally against the worktree SDK source:
Adapterkit(13 total) and their JSON schemas parse.validate()injects the hard-codedapi_baseand prefixes the model withcustom_openai/(e.g.nvidia/nemotron-mini-4b-instruct→custom_openai/nvidia/..., basehttps://integrate.api.nvidia.com/v1).temperature, emitsreasoning_effort+max_completion_tokensviaextra_body).ruff check/ruff formatclean on all new code; pre-commit hooks passed.Screenshots
NVIDIA Build and OpenRouter adapter icons (512×512) added under
frontend/public/icons/adapter-icons/.Checklist
I have read and understood the Contribution Guidelines.
🤖 Generated with Claude Code