feat(nemo-evaluator): add REST endpoints for metric catalog, schema, and sync evaluate - #509
Open
marcusds wants to merge 5 commits into
Open
feat(nemo-evaluator): add REST endpoints for metric catalog, schema, and sync evaluate#509marcusds wants to merge 5 commits into
marcusds wants to merge 5 commits into
Conversation
marcusds
force-pushed
the
add-evaluator-rest-apis/mschwab
branch
from
June 29, 2026 21:04
041a24b to
6afdba8
Compare
Contributor
|
marcusds
force-pushed
the
add-evaluator-rest-apis/mschwab
branch
6 times, most recently
from
July 6, 2026 18:35
870945c to
b317702
Compare
…and sync evaluate
Bring the evaluator plugin's REST surface to parity with its CLI. Three
capabilities were CLI-only and are now reachable over HTTP:
- GET /v2/metric-types and /v2/metric-types/{metric_type} — built-in metric
catalog and per-type JSON schema (mirrors `nemo evaluator metric-types`).
- GET /v2/evaluate/schema — evaluate input spec schema (mirrors
`nemo evaluator evaluate explain`).
- POST /v2/workspaces/{workspace}/evaluate — run a bounded evaluation
synchronously and return the result inline (mirrors `nemo evaluator
evaluate run`).
To keep the CLI and REST from drifting, the metric-type catalog introspection
moves out of cli.py into a shared metric_catalog module that both consume.
The Evaluator().run_sync target dispatch is factored out of EvaluateJob.run
into a reusable run_evaluation() shared by the job and the new endpoint.
The synchronous endpoint is deliberately bounded and guarded: inline
(built-in) metrics only — cloudpickle bundles are rejected (422) so arbitrary
code is never unpickled in the long-lived API process; inline dataset rows
only, capped at 10; execution runs in a threadpool under a 60s timeout.
Adds the evaluator.evaluate.exec permission for the sync route and read
scopes for the read-only catalog routes.
Regenerates the plugin OpenAPI spec and documents the endpoints in the plugin
reference and the nemo-evaluator-plugin skill.
Hardening from adversarial review (codex + multi-persona): the synchronous
endpoint runs in the long-lived API process, where the SDK's local backend
resolves secrets from os.getenv — so a request-supplied model URL + secret was
an SSRF + env-secret exfiltration vector. The endpoint is now scoped to what is
safe in-process: offline only (no online target); models must be platform
ModelRefs (resolved to the inference gateway with no secret); and cloudpickle
bundles, network (remote) metric types, request-supplied secrets, and inline
model definitions are all rejected with 422. LLM-judge and other model-backed
metrics remain supported via ModelRefs. The resolved-model inference call
carries the caller's request-scoped headers (principal + trace) so it runs as
the caller, not an elevated/cached service principal. Execution runs on a
dedicated bounded thread pool with backpressure (503 when full; a timed-out run
keeps its slot until the blocking call returns, so the pool can't oversubscribe
or starve the main request pool); hydration/resolution and SDK EvaluationError
failures map to 422 (was 500); all failure paths log with sanitized client
messages. The service now declares its `models` and `inference-gateway`
dependencies. The two shared helpers used by both the job and the route are now
public (`to_runtime_bundle`, `unresolved_model_refs`).
Also fixes three pre-existing ty diagnostics in test_evaluate_job.py (use the
ModelFormat enum; convert MetricInline via to_runtime_bundle before
unbundle_metric) surfaced by the type-check hook now that the file is touched.
Signed-off-by: mschwab <mschwab@nvidia.com>
…ract Availability and correctness fixes for the synchronous evaluate endpoint: - Release capacity slots via a thread-safe semaphore in the worker's done callback instead of the submitting request's event loop, which leaked slots permanently (503 until restart) when the loop closed first - Run evaluations on daemon threads instead of a module ThreadPoolExecutor so a stuck eval cannot block interpreter shutdown at SIGTERM - Bound each metric's inference calls to the sync budget (wrapped inference fn for judge metrics, request_timeout/max_retries extras for RAGAS) so a detached worker frees its slot near the request timeout, not 600s+ - Replace wait_for with asyncio.wait so a worker-internal TimeoutError is not misreported as the sync budget expiring - Scope 422s to request validation (now carrying the underlying message) plus worker EvaluationErrors; internal worker bugs surface as 500s - Cap the metrics list at MAX_SYNC_METRICS (10) to bound per-request work - Flip the payload guard to an inline allow-list (fail closed for future payload kinds) and scan for inline models recursively Forward Model.default_headers into RAGAS judge/embeddings clients so the run-as-the-caller identity design also holds for the RAGAS metric family. Drop models/inference-gateway from evaluator startup dependencies; they blocked startup up to 120s each when deployed-but-unhealthy. Catalog contract: typed MetricTypeList/MetricTypeEntry response envelope, and /evaluate/schema now describes the sync request body while the new /evaluate/jobs/schema serves the durable-job input spec. Tests: SSRF guard pinned with a real RemoteMetric bundle, plus coverage for 503 backpressure, 504 timeout, slot release across event loops, daemon workers, caps, and error mapping. Signed-off-by: mschwab <mschwab@nvidia.com>
…backpressure Address review findings on the synchronous evaluate route: - RAGAS metrics build their judge client from caller-supplied inference params (extra=allow), which could smuggle transport/auth kwargs (base_url, default_headers, ...) that redirect the judge call (SSRF) or replace the forwarded caller identity. Make the resolved model's transport/auth authoritative in _get_llm_judge, and on the sync path strip RAGAS inference to an allowlist of generation params. - Clamp explicit RAGAS request_timeout/max_retries into the sync budget (was fill-if-absent), so an explicit 3600s/99-retry request cannot hold a worker slot for hours after the 60s response. - Acquire the capacity slot before model resolution so backpressure gates the remote model/provider lookups instead of letting requests fan out unbounded while full. - Give the 422/503/504 responses a typed EvaluateSyncError body so generated clients receive typed error detail. - Correct the evaluator skill's REST table: /evaluate/schema returns the sync request body; /evaluate/jobs/schema is the evaluate-explain equivalent. Regenerate the evaluator plugin OpenAPI spec. Signed-off-by: mschwab <mschwab@nvidia.com>
Collapse multi-line comment blocks to terse one-liners across the sync evaluate route, RAGAS transport fix, and service deps; no behavior change. Signed-off-by: mschwab <mschwab@nvidia.com>
The RAGAS default_headers forwarding and transport-override guard were committed to the nemo_evaluator_sdk source but never vendored into sdk/python, leaving lint-sdk-vendored and lint-cli red. Re-vendor the evaluator SDK so the mirror matches the source. Signed-off-by: mschwab <mschwab@nvidia.com>
marcusds
force-pushed
the
add-evaluator-rest-apis/mschwab
branch
from
July 7, 2026 17:07
40ef372 to
90186a1
Compare
marcusds
marked this pull request as ready for review
July 7, 2026 17:35
Contributor
📝 WalkthroughWalkthroughAdds a bounded synchronous evaluate REST endpoint, a metric catalog module for discovering built-in metric types/schemas, v2 catalog routes, and OpenAPI/docs updates. Also fixes RAGAS header propagation/parameter precedence, refactors CLI and jobs/evaluate to use the new catalog and dispatch helper. ChangesRAGAS header propagation fix
Synchronous evaluate API and metric catalog
Sequence Diagram(s)sequenceDiagram
participant Client
participant EvaluateSyncRoute
participant ModelResolver
participant WorkerThread
participant SDKEvaluator
Client->>EvaluateSyncRoute: POST /evaluate (inline metrics, dataset)
EvaluateSyncRoute->>EvaluateSyncRoute: validate allow-list, reject inline models/secrets
EvaluateSyncRoute->>EvaluateSyncRoute: acquire capacity semaphore
alt semaphore unavailable
EvaluateSyncRoute-->>Client: 503
else acquired
EvaluateSyncRoute->>ModelResolver: resolve ModelRef(s) with forwarded headers
ModelResolver-->>EvaluateSyncRoute: resolved model config
EvaluateSyncRoute->>WorkerThread: submit bounded evaluation (daemon thread)
WorkerThread->>SDKEvaluator: run_evaluation(metrics, dataset, target)
SDKEvaluator-->>WorkerThread: EvaluationResult / error
EvaluateSyncRoute->>EvaluateSyncRoute: asyncio.wait(timeout)
alt completed in time
WorkerThread-->>EvaluateSyncRoute: result
EvaluateSyncRoute-->>Client: 200 EvaluationResult
else timeout exceeded
EvaluateSyncRoute-->>Client: 504
end
end
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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: 3
🧹 Nitpick comments (1)
plugins/nemo-evaluator/src/nemo_evaluator/api/v2/evaluate.py (1)
61-62: 🔒 Security & Privacy | 🔵 Trivial | 🏗️ Heavy liftDeny-list for network-backed metrics can miss future types.
_NETWORK_BACKED_METRIC_TYPESis a hardcoded deny-list, while the payload-kind check right above it is explicitly designed as an allow-list that "fails closed" for unknown kinds. A new SDK metric type that calls an external URL won't be blocked here unless someone remembers to add it to this frozenset — the opposite of the fail-closed posture the rest of the route aims for.Consider deriving this from metric metadata (e.g., a
calls_external_endpointflag surfaced viametric_catalog) rather than a manually maintained list, or at minimum add a test that fails when a newMetricTypeis added without a corresponding classification.Also applies to: 244-251
🤖 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 `@plugins/nemo-evaluator/src/nemo_evaluator/api/v2/evaluate.py` around lines 61 - 62, The network-backed metric filter in evaluate.py is a manually maintained deny-list, so new MetricType values that call external URLs can slip through unless updated by hand. Update the evaluation path around _NETWORK_BACKED_METRIC_TYPES and the related payload-kind check to derive this classification from metric metadata (for example via metric_catalog using a calls_external_endpoint-style flag) instead of hardcoding MetricType.REMOTE and MetricType.NEMO_AGENT_TOOLKIT_REMOTE, and add a regression test that fails when a new MetricType is introduced without an explicit external-endpoint classification.
🤖 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/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/ragas/base.py`:
- Around line 304-312: The merge in the Ragas base path still lets
caller-provided default_headers through from inference params, so explicitly
strip default_headers before building chat_params. Update the merge logic in the
same section that combines self._inference_params and self._llm_model so only
resolved-model transport/auth settings are retained, matching the existing
handling for base_url, api_key, and model.
In `@plugins/nemo-evaluator/src/nemo_evaluator/api/v2/catalog.py`:
- Line 11: The deprecated Starlette status constant is being pulled in through
EvaluateSyncRequest’s upstream module, so update the responses mapping in
evaluate.py to use HTTP_422_UNPROCESSABLE_CONTENT instead of
HTTP_422_UNPROCESSABLE_ENTITY. Make the change in the place where the API
response codes are defined so catalog.py no longer indirectly imports the
deprecated constant.
In `@plugins/nemo-evaluator/src/nemo_evaluator/api/v2/evaluate.py`:
- Around line 83-93: The recursive inline-model check in _has_inline_model
currently misses pydantic v2 extra fields because it only walks
vars(value).values(), so nested BaseModel instances can hide a Model inside
model_extra and bypass the SSRF guard. Update _has_inline_model to include both
the model’s normal fields and its model_extra contents when recursing through
BaseModel values, while preserving the existing handling for dicts and
iterables.
---
Nitpick comments:
In `@plugins/nemo-evaluator/src/nemo_evaluator/api/v2/evaluate.py`:
- Around line 61-62: The network-backed metric filter in evaluate.py is a
manually maintained deny-list, so new MetricType values that call external URLs
can slip through unless updated by hand. Update the evaluation path around
_NETWORK_BACKED_METRIC_TYPES and the related payload-kind check to derive this
classification from metric metadata (for example via metric_catalog using a
calls_external_endpoint-style flag) instead of hardcoding MetricType.REMOTE and
MetricType.NEMO_AGENT_TOOLKIT_REMOTE, and add a regression test that fails when
a new MetricType is introduced without an explicit external-endpoint
classification.
🪄 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: Enterprise
Run ID: 62cf4f1b-74a0-4b8b-8c9f-2bae3efea23c
⛔ Files ignored due to path filters (1)
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/metrics/ragas/base.pyis excluded by!sdk/**
📒 Files selected for processing (14)
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/ragas/base.pypackages/nemo_evaluator_sdk/tests/metrics/ragas/test_ragas_headers.pyplugins/nemo-evaluator/openapi/openapi.yamlplugins/nemo-evaluator/src/nemo_evaluator/api/v2/catalog.pyplugins/nemo-evaluator/src/nemo_evaluator/api/v2/evaluate.pyplugins/nemo-evaluator/src/nemo_evaluator/cli.pyplugins/nemo-evaluator/src/nemo_evaluator/docs/index.mdplugins/nemo-evaluator/src/nemo_evaluator/jobs/evaluate.pyplugins/nemo-evaluator/src/nemo_evaluator/metric_catalog.pyplugins/nemo-evaluator/src/nemo_evaluator/service.pyplugins/nemo-evaluator/tests/api/v2/test_catalog_routes.pyplugins/nemo-evaluator/tests/api/v2/test_evaluate_sync_route.pyplugins/nemo-evaluator/tests/test_evaluate_job.pyskills/nemo-evaluator-plugin/SKILL.md
Comment on lines
+304
to
312
| chat_params: dict[str, Any] = {} | ||
| if self._inference_params: | ||
| chat_params.update(self._inference_params) | ||
| # Applied last: transport/auth from the resolved model must win over inference params, | ||
| # else a request could redirect the judge call (SSRF) or replace the forwarded identity. | ||
| chat_params.update(self._llm_model) | ||
|
|
||
| # Filter out None values | ||
| chat_params = {k: v for k, v in chat_params.items() if v is not None} |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep outline packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/params.py --match InferenceParams --view expanded
rg -nP -C3 '(model_config|extra\s*=|default_headers|base_url)' packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/params.pyRepository: NVIDIA-NeMo/nemo-platform
Length of output: 1209
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the model construction and the inference params shape.
ast-grep outline packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/ragas/base.py --view expanded
printf '\n--- params.py ---\n'
cat -n packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/params.py | sed -n '1,120p'Repository: NVIDIA-NeMo/nemo-platform
Length of output: 7826
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '155,190p' packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/ragas/base.py | cat -n
printf '\n---\n'
sed -n '299,316p' packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/ragas/base.py | cat -nRepository: NVIDIA-NeMo/nemo-platform
Length of output: 3285
Drop default_headers from inference params before merging. InferenceParams allows extras, so caller-supplied default_headers still reach ChatOpenAI when the resolved model has none. base_url, api_key, and model are already overridden; default_headers needs the same explicit guard.
🤖 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 `@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/ragas/base.py`
around lines 304 - 312, The merge in the Ragas base path still lets
caller-provided default_headers through from inference params, so explicitly
strip default_headers before building chat_params. Update the merge logic in the
same section that combines self._inference_params and self._llm_model so only
resolved-model transport/auth settings are retained, matching the existing
handling for base_url, api_key, and model.
| from typing import Any | ||
|
|
||
| from fastapi import APIRouter, HTTPException, status | ||
| from nemo_evaluator.api.v2.evaluate import EvaluateSyncRequest |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Deprecated Starlette constant used upstream.
CI flags HTTP_422_UNPROCESSABLE_ENTITY as deprecated (surfaces via this import of evaluate.py). Replace with HTTP_422_UNPROCESSABLE_CONTENT in evaluate.py where the responses dict is defined.
rg -n 'HTTP_422_UNPROCESSABLE_ENTITY' plugins/nemo-evaluator/src/nemo_evaluator/api/v2/evaluate.py🧰 Tools
🪛 GitHub Actions: CI / 42_Lint all.txt
[warning] 11-11: StarletteDeprecationWarning: 'HTTP_422_UNPROCESSABLE_ENTITY' is deprecated. Use 'HTTP_422_UNPROCESSABLE_CONTENT' instead.
🪛 GitHub Actions: CI / Lint all
[warning] 11-11: StarletteDeprecationWarning: 'HTTP_422_UNPROCESSABLE_ENTITY' is deprecated. Use 'HTTP_422_UNPROCESSABLE_CONTENT' instead.
🤖 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 `@plugins/nemo-evaluator/src/nemo_evaluator/api/v2/catalog.py` at line 11, The
deprecated Starlette status constant is being pulled in through
EvaluateSyncRequest’s upstream module, so update the responses mapping in
evaluate.py to use HTTP_422_UNPROCESSABLE_CONTENT instead of
HTTP_422_UNPROCESSABLE_ENTITY. Make the change in the place where the API
response codes are defined so catalog.py no longer indirectly imports the
deprecated constant.
Source: Pipeline failures
Comment on lines
+83
to
+93
| def _has_inline_model(value: object) -> bool: | ||
| """True if an inline Model (vs a platform ModelRef) appears anywhere in a metric's fields.""" | ||
| if isinstance(value, Model): | ||
| return True | ||
| if isinstance(value, BaseModel): | ||
| return any(_has_inline_model(field_value) for field_value in vars(value).values()) | ||
| if isinstance(value, dict): | ||
| return any(_has_inline_model(item) for item in value.values()) | ||
| if isinstance(value, (list, tuple, set, frozenset)): | ||
| return any(_has_inline_model(item) for item in value) | ||
| return False |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how extra="allow" fields are exposed on pydantic v2 BaseModel instances.
python3 - <<'EOF'
from pydantic import BaseModel, ConfigDict
class M(BaseModel):
model_config = ConfigDict(extra="allow")
a: int = 1
m = M(a=1, b={"nested": "value"})
print("vars():", vars(m))
print("model_extra:", m.model_extra)
EOFRepository: NVIDIA-NeMo/nemo-platform
Length of output: 220
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file and the referenced model types / call sites.
git ls-files plugins/nemo-evaluator/src/nemo_evaluator/api/v2/evaluate.py \
plugins/nemo-evaluator/src/nemo_evaluator/api/v2 \
| sed 's#^`#FILE` #'
echo
echo "== evaluate.py outline =="
ast-grep outline plugins/nemo-evaluator/src/nemo_evaluator/api/v2/evaluate.py --view expanded || true
echo
echo "== evaluate.py relevant slice =="
sed -n '1,220p' plugins/nemo-evaluator/src/nemo_evaluator/api/v2/evaluate.py | cat -n
echo
echo "== search for BaseModel / model_extra / InferenceParams / ModelRef / Model usage =="
rg -n --hidden --glob '!.git' \
-e 'model_extra' \
-e 'InferenceParams' \
-e 'ModelRef' \
-e 'class Model' \
-e 'additionalProperties' \
plugins/nemo-evaluator/src/nemo_evaluatorRepository: NVIDIA-NeMo/nemo-platform
Length of output: 16863
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the metric schema and the inline-model guard call site.
echo "== metrics.py outline =="
ast-grep outline plugins/nemo-evaluator/src/nemo_evaluator/api/v2/metrics.py --view expanded || true
echo
echo "== metrics.py relevant slice =="
sed -n '1,260p' plugins/nemo-evaluator/src/nemo_evaluator/api/v2/metrics.py | cat -n
echo
echo "== evaluate.py call site around the guard =="
sed -n '240,330p' plugins/nemo-evaluator/src/nemo_evaluator/api/v2/evaluate.py | cat -n
echo
echo "== search for InferenceParams definition / config =="
rg -n --hidden --glob '!.git' \
-e 'class InferenceParams' \
-e 'model_config = ConfigDict\\(extra="allow"\\)' \
-e 'extra="allow"' \
plugins/nemo-evaluator/src/nemo_evaluator_sdk plugins/nemo-evaluator/src/nemo_evaluatorRepository: NVIDIA-NeMo/nemo-platform
Length of output: 15258
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the request/metric schemas that feed _has_inline_model.
echo "== schema definitions for MetricInline / Metric / related models =="
ast-grep outline plugins/nemo-evaluator/src/nemo_evaluator/api/schemas.py --view expanded || true
echo
echo "== relevant slices from schemas.py =="
sed -n '1,320p' plugins/nemo-evaluator/src/nemo_evaluator/api/schemas.py | cat -n
echo
echo "== locate Model / ModelRef-bearing fields in metric schemas =="
rg -n --hidden --glob '!.git' \
-e 'MetricInline' \
-e 'MetricWithModels' \
-e 'ModelRef' \
-e 'InferenceParams' \
plugins/nemo-evaluator/src/nemo_evaluator/api/schemas.py plugins/nemo-evaluator/src/nemo_evaluator/shared plugins/nemo-evaluator/src/nemo_evaluator/jobsRepository: NVIDIA-NeMo/nemo-platform
Length of output: 30893
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import inspect
from pydantic import BaseModel, ConfigDict
from nemo_evaluator_sdk.values.params import InferenceParams
from nemo_evaluator_sdk.metrics.protocol import MetricWithModels
from nemo_evaluator.api.schemas import MetricInline
from nemo_evaluator.jobs.metric_resolution import to_runtime_bundle
from nemo_evaluator.shared.metric_bundles.bundles import unbundle_metric
print("pydantic:", __import__("pydantic").__version__)
print("InferenceParams:", InferenceParams)
print("InferenceParams model_config.extra:", getattr(InferenceParams, "model_config", {}).get("extra"))
print("InferenceParams fields:", list(getattr(InferenceParams, "model_fields", {}).keys()))
print("MetricWithModels:", MetricWithModels)
print("MetricInline:", MetricInline)
print("to_runtime_bundle source:", inspect.getsource(to_runtime_bundle).splitlines()[0:20])
print("unbundle_metric source:", inspect.getsource(unbundle_metric).splitlines()[0:25])
class M(BaseModel):
model_config = ConfigDict(extra="allow")
a: int = 1
m = M(a=1, b={"nested": "value"})
print("vars(M):", vars(m))
print("model_dump:", m.model_dump())
print("model_extra:", m.model_extra)
PYRepository: NVIDIA-NeMo/nemo-platform
Length of output: 294
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== shared.metric_bundles.bundles.py outline =="
ast-grep outline plugins/nemo-evaluator/src/nemo_evaluator/shared/metric_bundles/bundles.py --view expanded || true
echo
echo "== shared.metric_bundles.bundles.py relevant slice =="
sed -n '1,260p' plugins/nemo-evaluator/src/nemo_evaluator/shared/metric_bundles/bundles.py | cat -n
echo
echo "== metric_resolution.py relevant slice =="
sed -n '1,180p' plugins/nemo-evaluator/src/nemo_evaluator/jobs/metric_resolution.py | cat -n
echo
echo "== search for model_config extra=\"allow\" in evaluator code =="
rg -n --hidden --glob '!.git' 'model_config = ConfigDict\(extra="allow"' plugins/nemo-evaluator/src/nemo_evaluatorRepository: NVIDIA-NeMo/nemo-platform
Length of output: 19667
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,240p' plugins/nemo-evaluator/src/nemo_evaluator/shared/metric_bundles/bundles.py | cat -nRepository: NVIDIA-NeMo/nemo-platform
Length of output: 10572
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,180p' plugins/nemo-evaluator/src/nemo_evaluator/jobs/metric_resolution.py | cat -n
echo
echo "== metric protocol references =="
rg -n --hidden --glob '!.git' 'class MetricWithModels|class .*Metric|inference:' plugins/nemo-evaluator/src/nemo_evaluator_sdk plugins/nemo-evaluator/src/nemo_evaluator | head -n 80Repository: NVIDIA-NeMo/nemo-platform
Length of output: 8998
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== search repo docs/code for InferenceParams mentions =="
rg -n --hidden --glob '!.git' 'InferenceParams|request_timeout|max_retries|max_completion_tokens' plugins/nemo-evaluator/src/nemo_evaluator plugins/nemo-evaluator/docs plugins/nemo-evaluator/README* || trueRepository: NVIDIA-NeMo/nemo-platform
Length of output: 1613
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pydantic import BaseModel, ConfigDict
class Inline:
pass
class Child(BaseModel):
model_config = ConfigDict(extra="allow")
x: int = 1
class Parent(BaseModel):
child: Child
def has_inline_model(value):
if isinstance(value, Inline):
return True
if isinstance(value, BaseModel):
return any(has_inline_model(v) for v in vars(value).values())
if isinstance(value, dict):
return any(has_inline_model(v) for v in value.values())
if isinstance(value, (list, tuple, set, frozenset)):
return any(has_inline_model(v) for v in value)
return False
c = Child(x=1, smuggled=Inline())
p = Parent(child=c)
print("vars(child):", vars(c))
print("model_extra:", c.model_extra)
print("has_inline_model(child):", has_inline_model(c))
print("has_inline_model(parent):", has_inline_model(p))
print("walked_values(child):", list(vars(c).values()))
PYRepository: NVIDIA-NeMo/nemo-platform
Length of output: 344
Include model_extra in the recursion. vars(value).values() skips pydantic v2 extra="allow" fields, so a nested BaseModel can hide an inline Model in model_extra and bypass this SSRF check.
🤖 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 `@plugins/nemo-evaluator/src/nemo_evaluator/api/v2/evaluate.py` around lines 83
- 93, The recursive inline-model check in _has_inline_model currently misses
pydantic v2 extra fields because it only walks vars(value).values(), so nested
BaseModel instances can hide a Model inside model_extra and bypass the SSRF
guard. Update _has_inline_model to include both the model’s normal fields and
its model_extra contents when recursing through BaseModel values, while
preserving the existing handling for dicts and iterables.
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
1 participant
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.
Summary
Brings the
nemo-evaluatorplugin's REST surface to parity with its CLI. After thelegacy evaluator service was removed and replaced by the plugin, several CLI capabilities had
no HTTP equivalent — so a UI or any non-Python client couldn't reach them. This adds those
endpoints, with a synchronous evaluate route that is carefully bounded to what is safe inside
the long-lived API process.
GET /v2/metric-typesnemo evaluator metric-typesGET /v2/metric-types/{metric_type}nemo evaluator metric-types <name>GET /v2/evaluate/schemaGET /v2/evaluate/jobs/schemanemo evaluator evaluate explainPOST /v2/workspaces/{workspace}/evaluatenemo evaluator evaluate runChanges
metric_catalog.py(new) — metric-type introspection moves out ofcli.pyso the CLIand the new REST routes share one source of truth.
jobs/evaluate.py— factored theEvaluator().run_synctarget dispatch out ofEvaluateJob.runinto a reusablerun_evaluation(...)(behavior-preserving), shared by thejob and the sync route.
api/v2/catalog.py(new) — read-only discovery routes, mounted at/v2. TypedMetricTypeList/MetricTypeEntryresponse envelope so generated clients get typed accessors.api/v2/evaluate.py(new) — the synchronous evaluate route (see hardening below).service.py— registers the routers and authz (evaluator.metric_typesread scopes;new
evaluator.evaluate.execwrite permission).metrics/ragas/base.py— RAGAS judge/embeddings clients now carry the resolved model'sdefault_headers(so run-as-caller identity forwarding works for the RAGAS family), and theresolved model's transport/auth is authoritative over caller-supplied inference params.
plugins/nemo-evaluator/openapi/openapi.yaml; documented the endpoints in theplugin reference and the
nemo-evaluator-pluginskill.Security / limits on the synchronous endpoint
It executes the SDK evaluator inside the long-lived API process, so it is deliberately bounded.
Rejected with
422:payload.kind == "inline"), socloudpickle bundles and any future payload kind fail closed; arbitrary code is never executed
in the API process. Ship those as a durable job.
remote,nemo-agent-toolkit-remote) — they call auser-supplied URL (SSRF surface).
environment would be an exfiltration vector.
ModelRef(
workspace/model), scanned recursively; inline models carry an arbitrary URL (SSRF). Refsresolve to the inference gateway with no secret, and the in-process call carries the
caller's request-scoped headers, so it runs as the caller (not an elevated service
principal). For RAGAS, caller-supplied
inferenceparams are reduced to a generation-paramallow-list so they can't override the resolved model's transport/auth.
FilesetRefdatasets — submit those as a job.Capacity and lifecycle:
MAX_SYNC_ROWS(10); metrics list capped atMAX_SYNC_METRICS(10)._SYNC_EVAL_MAX_WORKERS, 4). At capacity theendpoint returns
503immediately (no queueing); the slot is reserved before model resolutionso backpressure gates downstream lookups.
under a 60s wall-clock timeout →
504. Each metric's model calls are bounded to the samebudget (retries capped), so a detached worker frees its slot near the timeout rather than
holding it for the upstream's full default timeout.
422/503/504responses carry a typedEvaluateSyncErrorbody with actionable detail.modelsandinference-gatewayare intentionally not evaluator startup dependencies (theywould block startup when deployed-but-unhealthy); the sync route degrades at request time with an
actionable
422instead.Testing
tests/api/v2/test_catalog_routes.py— catalog/schema parity, 404, typed responseenvelope, and the job-vs-sync schema split.
tests/api/v2/test_evaluate_sync_route.py— happy-path offline eval; all guard rejections(SSRF guard pinned with a real
RemoteMetricbundle, not a mutated type); the full capacitylifecycle (503 backpressure, 504 timeout, slot release across event loops, daemon workers);
metrics cap; actionable-422 and worker-bug-500 error mapping; RAGAS transport strip + timeout
clamp; typed error bodies.
packages/nemo_evaluator_sdk/tests/metrics/ragas/test_ragas_headers.py(new) — RAGASforwards caller headers to judge/embeddings clients, and caller
inferenceparams cannotoverride the resolved model's transport/identity.
pytest plugins/nemo-evaluator/tests/→ 470 passed (integration deselected); relevant SDKRAGAS tests pass;
ruff+tyclean.Notes
gen:evaluator) regenerates cleanly; its output is gitignored, so no diff here.SKILL.mdedit makesskill.oms.sigstale — run/nvskills-ciso the signing botrefreshes it (required by branch protection before merge).
/apis/evaluator/*paths; the plugin ownsits SDK and surfaces to web clients via orval.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes