refactor(evaluator)!: collapse the dataset entrypoints into evaluate_dataset by SandyChapman · Pull Request #1202 · NVIDIA-NeMo/nemo-platform

SandyChapman · 2026-08-10T15:24:56Z

Summary

The backend contract carried two dataset entrypoints that differed only in metric arity and return type — evaluate took one metric and returned an EvaluationResult, evaluate_benchmark took a list and returned a BenchmarkEvaluationResult. One metric is the degenerate case of several, so this collapses them into a single evaluate_dataset(metrics=[...]).

This is deliberately a subtraction: 210 insertions against 781 deletions. It also frees the name evaluate, which a follow-up will use for the taskset entrypoint — nothing claims it here, so evaluate_dataset is the only method on the contract after this lands.

Related Issue

None.

Changes

EvaluationBackend / SyncEvaluationBackend: evaluate and evaluate_benchmark replaced by evaluate_dataset. Same for LocalBackend and the sync adapter.
Evaluator.run / run_sync: the three single-metric overloads on each are gone, leaving the three that discriminate on config and target. Return type narrows to BenchmarkEvaluationResult.
BenchmarkEvaluationResult.__str__, matching EvaluationResult. Without it, collapsing to one result type would have quietly regressed print(result) to a pydantic dump for every caller.
The plugin's evaluate job drops metrics if len(metrics) > 1 else metrics[0], which existed only to reach the singular overload.
A backend's flavour is now decided by one method, so the async/sync/mixed three-way collapses to two — a lone method is either a coroutine function or it is not. _MixedBackend and its test go with it.
Call sites updated to metrics=[...] across tests, examples, skill references and the walkthrough notebook.

Type of Change

Code change with documentation updates
Code change (feature, bug fix, or refactor)
Documentation only
Contributor tooling or automation
CI, build, or test infrastructure

Quality Gates

Tests added or updated for changed behavior
Existing tests cover changed behavior — justification:
Tests not applicable — justification:
Documentation updated for user-visible behavior
Documentation not applicable — justification:

Existing tests were retargeted rather than duplicated: the backend fakes now carry one dataset result instead of two, and test_api.py asserts the multi-metric shape, whose row columns are metric-qualified (output.exact-match.exact-match) and whose preview is per-metric. Two tests were removed because their subjects no longer exist — the single-vs-sequence dispatch fallthrough, and the mixed sync/async client.

Verification

Pull request title follows the repository's Conventional Commit format
Every commit includes an appropriate Signed-off-by: trailer
uv run pre-commit run -a passes, or any blocked checks are identified below
Targeted tests pass, or tests are marked not applicable above
No secrets, API keys, or credentials are included

Command	Result
`uv run --frozen pytest packages/nemo_evaluator_sdk/tests plugins/nemo-evaluator/tests docs/evaluator -q`	2225 passed, 24 skipped
`uv run --frozen pytest e2e -q`	collected clean; 238 skipped without a live platform
`uv run ruff check` / `ruff format --check`	passed
`bash tools/lint/run-ty-check.sh`	passed
`make vendor`	vendored copy matches source
`uv run pre-commit run -a`	all hooks pass except the two below

Two hooks fail on this machine only, neither related to the diff:

Run uv lock with platform uv requires uv 0.9.14 exactly and exits on the version check before reading a file. Check for uv.lock drift passes and no pyproject.toml changed.
Run UI lint-staged fails with mise ERROR No version is set for shim: pnpm in a fresh worktree. This PR changes nothing under web/.

Notes for reviewers

Two things surfaced only once this was split out of a larger branch:

is_metric / is_metric_sequence are not dead yet. The plugin's bundle_metrics_for_spec still dispatches on single-vs-sequence, so they survive until the plugin's own API goes plural.
Dropping _is_sync_backend cost the TypeGuard narrowing on the final branch, so it stays, with an unreachable else that exists so the checker can narrow.

Summary by CodeRabbit

New Features
- Evaluation now consistently accepts one or more metrics and returns a unified benchmark result.
- Added concise result summaries with previews of evaluated rows.
- Exposed the benchmark result type through the SDK’s public API.
Documentation
- Updated SDK, plugin, walkthrough, and execution examples to use collection-based metrics.
Tests
- Expanded coverage for multi-metric evaluation, synchronous and asynchronous execution, result formatting, and backend validation.

coderabbitai · 2026-08-10T15:33:26Z

📝 Walkthrough

Walkthrough

The SDK now requires metric sequences for Evaluator.run and run_sync. Backends use evaluate_dataset and return BenchmarkEvaluationResult. Jobs, tests, examples, documentation, and skill metadata now use the unified contract.

Changes

Unified evaluation API

Layer / File(s)	Summary
Backend dataset contract `packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/execution/backends/`, `packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/`, `packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/__init__.py`	Backend protocols and `LocalBackend` now expose `evaluate_dataset` for multiple metrics and return `BenchmarkEvaluationResult`. The result type is publicly exported and has a summary string representation.
Evaluator orchestration `packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/execution/evaluator.py`	`Evaluator.run` and `run_sync` now require metric sequences, use `evaluate_dataset`, and return benchmark results. Sync backends use the unified adapter contract.
Integration and validation `packages/nemo_evaluator_sdk/tests/execution/`, `packages/nemo_evaluator_sdk/tests/metrics/`, `packages/nemo_evaluator_sdk/tests/test_api.py`, `plugins/nemo-evaluator/*`	Tests and evaluator jobs pass metric lists and validate benchmark-result output.
Examples and usage documentation `packages/nemo_evaluator_sdk/examples/`, `packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/execution/README.md`, `packages/nemo_platform_ext/.../sdk-execution.md`, `skills/nemo-evaluator-plugin/`	Examples and references use single-element metric lists. The walkthrough uses `BenchmarkEvaluationResult` and passes glob paths directly.

Possibly related PRs

NVIDIA-NeMo/nemo-platform#1228: Both PRs modify NeMo Evaluator execution interfaces.

Suggested reviewers: arpitsardhana, ngoncharenko

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 30.38% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main breaking change: consolidating dataset evaluation entrypoints into evaluate_dataset.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch evaluator-collapse-dataset-entrypoints/schapman

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

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/nemo_evaluator_sdk/examples/high_level_evaluate_walkthrough.ipynb`:
- Line 463: Update the directory_result call in high_level_evaluate_walkthrough
so evaluator.run_sync uses only its supported arguments; remove the unsupported
pattern keyword and pass the dataset directory through the accepted dataset path
or glob representation.

In
`@packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-evaluator/references/sdk-execution.md`:
- Line 84: Update the Evaluator API documentation around the metrics=[metric]
example and the descriptions of Evaluator.run and Evaluator.run_sync to require
a metric sequence, not a bare metric, and state that both methods return
BenchmarkEvaluationResult. Remove references to the deprecated single-metric
overload and alternate result types while preserving the existing usage example
structure.

🪄 Autofix

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: a3d12cfb-6c1e-400e-a288-05b7eee51c09

📥 Commits

Reviewing files that changed from the base of the PR and between 1c2e9d6 and 4883dc7.

⛔ Files ignored due to path filters (5)

sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/execution/backends/base.py is excluded by !sdk/**
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/execution/backends/local/backend.py is excluded by !sdk/**
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/execution/evaluator.py is excluded by !sdk/**
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/values/multi_metric_results.py is excluded by !sdk/**
sdk/python/nemo-platform/src/nemo_platform/skills/nemo-evaluator/references/sdk-execution.md is excluded by !sdk/**

📒 Files selected for processing (23)

packages/nemo_evaluator_sdk/examples/examples.py
packages/nemo_evaluator_sdk/examples/high_level_evaluate_walkthrough.ipynb
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/execution/README.md
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/execution/backends/base.py
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/execution/backends/local/backend.py
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/execution/evaluator.py
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/multi_metric_results.py
packages/nemo_evaluator_sdk/tests/execution/backends/local/test_backend.py
packages/nemo_evaluator_sdk/tests/execution/test_evaluator.py
packages/nemo_evaluator_sdk/tests/execution/test_metric_execution.py
packages/nemo_evaluator_sdk/tests/execution/test_resolvers.py
packages/nemo_evaluator_sdk/tests/metrics/test_bleu.py
packages/nemo_evaluator_sdk/tests/metrics/test_f1.py
packages/nemo_evaluator_sdk/tests/metrics/test_number_check.py
packages/nemo_evaluator_sdk/tests/metrics/test_rouge.py
packages/nemo_evaluator_sdk/tests/metrics/test_string_check.py
packages/nemo_evaluator_sdk/tests/metrics/test_tool_calling.py
packages/nemo_evaluator_sdk/tests/test_api.py
packages/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-evaluator/references/sdk-execution.md
plugins/nemo-evaluator/src/nemo_evaluator/jobs/evaluate.py
plugins/nemo-evaluator/tests/test_evaluate_job.py
skills/nemo-evaluator-plugin/assets/examples/plugin_sdk_examples.py
skills/nemo-evaluator-plugin/references/execution.md

github-actions · 2026-08-10T15:43:57Z

Suite	Lines Covered	Line Rate	Branch Rate
Unit Tests	31993/40684	78.6%	63.5%
Integration Tests	18567/38610	48.1%	20.8%

…dataset The backend contract carried two dataset entrypoints that differed only in metric arity and return type: `evaluate` took one metric and returned an `EvaluationResult`, `evaluate_benchmark` took a list and returned a `BenchmarkEvaluationResult`. One metric is the degenerate case of several, so the two collapse into a single `evaluate_dataset(metrics=[...])`. `Evaluator.run`/`run_sync` lose their single-metric overloads — three each — leaving the three that discriminate on config and target, where the choice actually matters. Callers pass a list. `BenchmarkEvaluationResult` gains the `__str__` that `EvaluationResult` already had. Without it, collapsing to one result type would have quietly regressed `print(result)` to a pydantic dump for every caller. Two smaller things fall out of having one destination rather than two. The plugin's evaluate job no longer unwraps a one-metric list to hit the singular overload. And a backend's flavour is now decided by a single method, so the async/sync/mixed three-way collapses to two: a lone method is either a coroutine function or it is not, and there is no mixed client left to reject. This frees the name `evaluate` for the taskset entrypoint, which follows separately; nothing claims it here. BREAKING CHANGE: `EvaluationBackend.evaluate` and `evaluate_benchmark` are replaced by `evaluate_dataset`, which takes a metric list and returns a `BenchmarkEvaluationResult`. `Evaluator.run` and `run_sync` no longer accept a bare metric. Signed-off-by: Sandy Chapman <schapman@nvidia.com>

…eft behind The collapse removed the single-metric overloads but left seven `run_sync` calls in the high-level walkthrough passing a bare metric. They fail as `AttributeError: 'tuple' object has no attribute 'output_spec'`, which says nothing about the real problem. The earlier sweep matched `metrics=[` and these pass named variables, so none of them matched. `BenchmarkEvaluationResult` is now what every dataset run returns, but it was only reachable from `nemo_evaluator_sdk.values.multi_metric_results` — callers could not name the type they are handed. It is exported from the package root and from `values` alongside `EvaluationResult`. The walkthrough's directory example passed `pattern=`, which no `Evaluator` method has ever accepted, on this branch or before it. The loader already handles a glob path when no pattern is given, so the example points `dataset` at `part-*.jsonl` and keeps excluding the file it is meant to exclude. The skill reference still promised a bare metric and one of two result types. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Sandy Chapman <schapman@nvidia.com>

SandyChapman · 2026-08-12T15:03:23Z

/nvskills-ci

coderabbitai · 2026-08-12T15:30:01Z

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

SandyChapman · 2026-08-12T16:56:39Z

/nvskills-ci

Signed-off-by: nvskills-svc-account <svc-nvskills-signing@nvidia.com>

coderabbitai

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 `@skills/nemo-evaluator-plugin/skill-card.md`:
- Line 2: Update the generator or Sphinx substitution configuration that
produces the skill-card description so the NeMo Evaluator product name comes
from a defined substitution rather than hardcoded text, then regenerate the
affected skill-card files, including the referenced 12-12 variant. Do not edit
the generated markdown manually.
- Around line 38-39: Update the generator or its markdownlint configuration so
generated skill cards include blank lines after the headings corresponding to
lines 38, 53, and 81, plus before the table at line 72; then regenerate
skills/nemo-evaluator-plugin/skill-card.md and verify the markdownlint warnings
are resolved without manually editing the generated file.

🪄 Autofix

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: a4da49de-df7d-4d41-b38a-656c17421c5d

📥 Commits

Reviewing files that changed from the base of the PR and between acfad8e and e40f142.

📒 Files selected for processing (3)

skills/nemo-evaluator-plugin/BENCHMARK.md
skills/nemo-evaluator-plugin/skill-card.md
skills/nemo-evaluator-plugin/skill.oms.sig

github-actions Bot added breaking breaking change (!-marked title) refactor labels Aug 10, 2026

SandyChapman commented Aug 10, 2026

View reviewed changes

Comment thread packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/execution/evaluator.py

SandyChapman commented Aug 10, 2026

View reviewed changes

Comment thread packages/nemo_evaluator_sdk/tests/execution/backends/local/test_backend.py

SandyChapman marked this pull request as ready for review August 10, 2026 15:27

SandyChapman requested review from a team as code owners August 10, 2026 15:27

SandyChapman requested review from arpitsardhana and ngoncharenko August 10, 2026 15:28

coderabbitai Bot reviewed Aug 10, 2026

View reviewed changes

Comment thread packages/nemo_evaluator_sdk/examples/high_level_evaluate_walkthrough.ipynb Outdated

Comment thread ...es/nemo_platform_ext/src/nemo_platform_ext/skills/nemo-evaluator/references/sdk-execution.md

This was referenced Aug 10, 2026

test(evaluator): run plugin skill evals in NeMo Platform #1063

Open

fix(evaluator): prefer sync SDK for FilesetRef dataset download #1211

Merged

ngoncharenko approved these changes Aug 11, 2026

View reviewed changes

SandyChapman and others added 2 commits August 12, 2026 11:44

SandyChapman force-pushed the evaluator-collapse-dataset-entrypoints/schapman branch from fb45f3f to acfad8e Compare August 12, 2026 15:29

Attach NVSkills validation signatures

e40f142

Signed-off-by: nvskills-svc-account <svc-nvskills-signing@nvidia.com>

coderabbitai Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread skills/nemo-evaluator-plugin/skill-card.md

Comment thread skills/nemo-evaluator-plugin/skill-card.md

SandyChapman enabled auto-merge August 12, 2026 18:41

SandyChapman added this pull request to the merge queue Aug 12, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 12, 2026

SandyChapman added this pull request to the merge queue Aug 12, 2026

coderabbitai Bot mentioned this pull request Aug 12, 2026

feat(evaluator): model evaluate wire publish_to_intake() to API #1152

Merged

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 12, 2026

SandyChapman mentioned this pull request Aug 12, 2026

refactor(evaluator)!: drop the plugin's local execution path #1262

Merged

SandyChapman added this pull request to the merge queue Aug 13, 2026

Merged via the queue into main with commit 65c6f81 Aug 13, 2026
105 of 106 checks passed

SandyChapman deleted the evaluator-collapse-dataset-entrypoints/schapman branch August 13, 2026 11:02

Navigation Menu

Uh oh!

refactor(evaluator)!: collapse the dataset entrypoints into evaluate_dataset - #1202

refactor(evaluator)!: collapse the dataset entrypoints into evaluate_dataset#1202
SandyChapman merged 3 commits into
mainfrom
evaluator-collapse-dataset-entrypoints/schapman

SandyChapman commented Aug 10, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot commented Aug 10, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 10, 2026 •

edited

Loading

Uh oh!

SandyChapman commented Aug 12, 2026

Uh oh!

coderabbitai Bot commented Aug 12, 2026

Uh oh!

SandyChapman commented Aug 12, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

SandyChapman commented Aug 10, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Related Issue

Changes

Type of Change

Quality Gates

Verification

Notes for reviewers

Summary by CodeRabbit

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SandyChapman commented Aug 12, 2026

Uh oh!

coderabbitai Bot commented Aug 12, 2026

Uh oh!

SandyChapman commented Aug 12, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SandyChapman commented Aug 10, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 10, 2026 •

edited

Loading

github-actions Bot commented Aug 10, 2026 •

edited

Loading