Repository navigation
feat(model-server): add OpenAI-compatible /v1/embeddings endpoint - #1964
Conversation
Model Servers previously exposed only chat_completions() and responses(), so memory- and retrieval-based agents had to call an embedding provider directly instead of routing through Gym. Add a POST /v1/embeddings route on the base model server using OpenAI's CreateEmbeddingResponse schema. The base default returns 501 (policy-only servers advertise the route but reject the call); openai_model and inference_provider override it as a passthrough to the provider's /v1/embeddings, covering OpenAI, NVIDIA's hosted endpoint, and any OpenAI-compatible provider. NeMoGymAsyncOpenAI gains create_embeddings(). encoding_format is forced to "float" on the upstream request: the response schema is List[float] and cannot represent base64, and OpenAI SDK clients default to encoding_format="base64" on the wire, which would otherwise fail response validation with a 500. Signed-off-by: khazic <khazzz1c@gmail.com>
|
hello? |
|
🌿 Preview your docs: https://nvidia-preview-pr-1964-5944df350bc4.docs.buildwithfern.com/nemo/gym |
|
All CI checks are green on the latest commit: DCO, Lint, copyright-check, secrets-detector, the full Server suite (shards 0 through 7), and the Test job all pass. The lone The branch is a little behind |
…erver-embeddings Signed-off-by: khazic <khazzz1c@gmail.com>
1e384df to
f43649f
Compare
yaoyu-33
left a comment
There was a problem hiding this comment.
Thanks for the PR. The route, the 501 default, and the passthrough adapters follow the existing messages() / chat_completions() patterns, and the new tests pass at this head (test_base_responses_api_model.py, plus the new openai_model and inference_provider embeddings tests).
Process
- GitHub reports this branch as conflicting with
main; could you rebase or mergemainand re-run CI on the new head? - Could you add a short real-provider smoke result to the PR body (for example the
curlfrom the Usage section against OpenAI/NVIDIA or a vLLM--task embedserver, showing status and vector length)? Every embeddings test here mocks_client.create_embeddings, so no run covers a real provider response.
Observation (not blocking)
/v1/embeddingsis registered only on the plain route, so embeddings calls are not picked up by model-call capture or the rollout-path prefixes that/v1/chat/completionshas. A line in the docs saying so would save a surprise later.
Inline comments below: one design question on the forced float / strict response validation, one test gap, one docstring note.
| # Force float output: this endpoint's response schema is CreateEmbeddingResponse | ||
| # (embedding: List[float]) and cannot represent base64. OpenAI SDK clients default to | ||
| # encoding_format="base64" on the wire, which would otherwise fail response validation. | ||
| body_dict["encoding_format"] = "float" |
There was a problem hiding this comment.
Forcing encoding_format="float" and then validating with the strict CreateEmbeddingResponse has two side effects: a client that explicitly asks for base64 gets float arrays back, and a provider that omits usage, model or data[].object (some OpenAI-compatible servers do) fails validation and comes back as a 500 even though the upstream call succeeded. Would you consider returning the provider's JSON as-is (no forced format, no re-validation)? That keeps the passthrough honest for both cases. If you prefer to keep the typed contract, please document that base64 is not supported. The same applies to inference_provider/app.py:125.
There was a problem hiding this comment.
Went with the passthrough in 1100e05 on both servers: no forced encoding_format and no re-validation, so base64 and providers that omit usage both work. openai_model now also goes through _call_upstream.
| assert "reasoning" not in sent_types | ||
| assert "message" in sent_types | ||
|
|
||
| async def test_embeddings(self) -> None: |
There was a problem hiding this comment.
openai_model/tests/test_app.py:292
Two cases are untested: (1) openai_model has no extra_body merge test like the one added for inference_provider (request value wins over config for the same key, and self.config.extra_body is not mutated), and (2) NeMoGymAsyncOpenAI.create_embeddings (openai_utils.py:953) is never exercised because every test mocks it, so the {base_url}/embeddings URL and JSON body are not checked. A test that patches _request/_raise_for_status/get_response_json and asserts the URL and body would cover it.
| openai_response_dict = await self._client.create_chat_completion(**body_dict) | ||
| return NeMoGymChatCompletion.model_validate(openai_response_dict) | ||
|
|
||
| async def embeddings(self, body: dict = Body()) -> CreateEmbeddingResponse: |
There was a problem hiding this comment.
AGENTS.md asks for useful docstrings on public APIs. The base embeddings() has one, but this override, the one in inference_provider and create_embeddings do not. Something like:
| async def embeddings(self, body: dict = Body()) -> CreateEmbeddingResponse: | |
| async def embeddings(self, body: dict = Body()) -> CreateEmbeddingResponse: | |
| """Pass an OpenAI-compatible embeddings request through to the provider; the configured model replaces the request's model.""" |
There was a problem hiding this comment.
Thanks, your docstrings are kept as is.
…ddings, add docstrings Merge main; add openai_model extra_body precedence test, a direct create_embeddings URL/body test, and docstrings for the new public methods. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
Forward the embeddings request (only `model` is replaced) and return the provider's JSON as-is, so `encoding_format: base64` works and providers that omit optional fields no longer turn a successful call into a 500. openai_model now routes embeddings through `_call_upstream`, so the upstream retry policy and concurrency slots apply as they do for chat and responses. `/v1/embeddings` is declared non-generating, so a call under a rollout's token-capture prefix does not mark that rollout incomplete. Document the passthrough, the `extra_body` precedence and the capture scope. Signed-off-by: khazic <khazzz1c@gmail.com>
|
Thanks for the review and for 3d8bbb6. 1100e05 goes on top of it:
The branch merges cleanly with main. CI needs a new |
|
/ok to test 1100e05 |
|
/ok to test 5944df3 |
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/40334275023 |
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/40344070863 |
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/40346226624 |
What does this PR do ?
Adds an OpenAI-compatible
POST /v1/embeddingsendpoint to the Model Server, so memory- and retrieval-based agents can route embedding calls through Gym instead of calling a provider directly.nemo_gym/base_responses_api_model.py): registers the/v1/embeddingsroute and adds a non-abstractembeddings()default that returns501. This mirrors the existingmessages()precedent (uniform route surface: every model server advertises the same endpoints; unsupported ones fail at call time). The route is declared non-generating, so an embeddings call under a rollout's token-capture prefix does not mark that rollout incomplete.openai_modelandinference_provider: overrideembeddings()as a passthrough to the provider's/v1/embeddings. A single OpenAI-compatible adapter covers OpenAI, NVIDIA's hosted endpoint (integrate.api.nvidia.com/v1), and any other OpenAI-compatible provider (including vLLM served as an embedding model).openai_modelsends the call through_call_upstream, so its upstream retry policy and concurrency slots apply as they do for chat and responses.NeMoGymAsyncOpenAIgainscreate_embeddings(), alongsidecreate_chat_completion/create_response/create_tokenize, all going through the global aiohttp client.The passthrough merges the server's
extra_bodyunder the request (a field the request sets wins), replacesmodelwith the configured one, and returns the provider's JSON unchanged. Soencoding_format: base64(the OpenAI SDK's default on the wire) reaches the provider and comes back as base64, and a provider that omits optional fields such asusagestill returns200.Embeddings calls are not recorded by model-call capture, which covers only
/v1/responses,/v1/chat/completionsand/v1/messages; the docs say so.An embedding model is usually distinct from the policy model, so it wires as a separate Model Server instance referenced via
ModelServerRef, matching the existing one-server-one-model pattern (no schema change).Issues
Closes #1943
Usage
Tests
tests/unit_tests/test_base_responses_api_model.py: base default returns501.responses_api_models/openai_model/tests/test_app.pyandresponses_api_models/inference_provider/tests/test_app.py: the provider JSON comes back unchanged (base64 vectors, nousage), the configured model replaces the request's,encoding_formatis forwarded as sent, request fields win overextra_bodyand the config is not mutated, andopenai_modelretries throughupstream_retry_policy.tests/unit_tests/test_openai_utils.py:create_embeddingsposts to{base_url}/embeddingswith the kwargs as the JSON body.tests/unit_tests/test_token_id_capture.py:POST /ng-rollout/<id>/training-token-capture/v1/embeddingsleaves the rollout complete (fails without the non-generating declaration).The affected test files pass (731 tests);
ruff check/ruff format(v0.9.9) clean.Real-provider smoke
Both model servers ran in their own process and pointed at a vLLM 0.28 server hosting
Qwen/Qwen3-Embedding-0.6B(vllm serve ... --runner pooling --convert embed). Each was queried withcurl(float and base64) and with the OpenAI Python SDK, which sendsencoding_format=base64and decodes the reply itself.Smoke output
Rollout evidence
N/A: this adds a model-server route and does not change any environment, agent or verifier behavior.