feat(evaluator): refresh evaluator plugin skills - #1055
Conversation
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
| settings={"sandbox": "read-only"}, | ||
| ), | ||
| models={"default": {"provider": "openai", "model": "gpt-5.4"}}, | ||
| models={"default": ModelConfig(provider="openai", model="gpt-5.4")}, |
There was a problem hiding this comment.
updated examples - ty marked typing issues
| # Fabric builds a Rust/pyo3 extension from source.) | ||
| # | ||
| # A live codex run additionally needs the `codex` CLI + `codex login` auth. | ||
| # See plugins/nemo-evaluator/docs/design/fabric-runner-integration.md. |
There was a problem hiding this comment.
no such doc, so linked to skills for reference
📝 WalkthroughWalkthroughThe PR updates evaluator specifications, plugin workflows, documentation, validation tests, and Fabric integration. It distinguishes local SDK execution from durable platform jobs and adds task-driven agent-evaluation guidance. ChangesEvaluator workflows
Sequence Diagram(s)sequenceDiagram
participant EvaluatorCLI
participant EvaluatorSDK
participant EvaluatorService
participant EvaluatorJob
participant ResultArtifacts
EvaluatorCLI->>EvaluatorService: Submit evaluator or agent-evaluate specification
EvaluatorSDK->>EvaluatorService: Submit evaluator job
EvaluatorService->>EvaluatorJob: Create and run job
EvaluatorJob->>ResultArtifacts: Persist results and artifacts
EvaluatorCLI->>ResultArtifacts: Retrieve completed outputs
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/nemo-evaluator/tests/test_evaluate_job.py (1)
71-74: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCover
fabric_agent_eval.jsonin the agent-evaluation compiler test.
test_checked_durable_fabric_job_is_a_valid_agent_eval_specvalidates onlyAgentEvalInputSpec. The compiler test uses a separate synthetic spec. Load this fixture, convert it throughAgentEvalJob.to_spec, and compile it withAgentEvalJob.compile.🤖 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/tests/test_evaluate_job.py` around lines 71 - 74, Update the agent-evaluation compiler test around EXAMPLE_SPEC_PATHS and test_checked_durable_fabric_job_is_a_valid_agent_eval_spec to include the fabric_agent_eval.json fixture. Load that fixture, convert it with AgentEvalJob.to_spec, and compile the resulting spec using AgentEvalJob.compile instead of relying on the separate synthetic spec.
🤖 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 `@docs/evaluator/agent-eval/index.mdx`:
- Line 16: Hyphenate the compound modifier as “platform-durable” in both
execution-mode notes: update docs/evaluator/agent-eval/index.mdx lines 16-16 to
“Local and platform-durable interfaces are available.” and
docs/evaluator/index.mdx lines 123-123 to “Agent evaluation has local and
platform-durable interfaces.”
- Around line 17-18: Update the local SDK usage note in
docs/evaluator/agent-eval/index.mdx at lines 17-18 to document await
AgentEvaluator().run(tasks=..., target=...). Apply the same await addition to
the corresponding note in docs/evaluator/index.mdx at lines 124-125; no other
changes are needed.
In `@packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_integration.py`:
- Around line 248-254: Update the requires_live_fabric skip reason to reference
the documented installation command, adding --frozen and --inexact to the
existing uv sync invocation while preserving the rest of the guidance.
In `@plugins/nemo-evaluator/README.md`:
- Around line 74-100: Correct the “Plugin SDK Execution” documentation around
the NeMoPlatform evaluator calls: either add a separate
client.evaluator.run(...) example demonstrating local execution, or revise the
heading and description to state that client.evaluator.submit(...) is durable
execution only.
In `@script/dev-install-fabric.sh`:
- Around line 20-21: Update the comment around the package-scoped command to say
it restores the locked project dependencies rather than returning the
environment to a locked state, and mention using an exact sync when a clean
environment is required.
In `@skills/nemo-evaluator-plugin/references/agent-evaluation.md`:
- Line 8: Update the sentence introducing AgentEvaluator to use correct grammar,
stating that it provides lightweight in-process evaluation without requiring a
running nemo-platform.
In `@skills/nemo-evaluator-plugin/references/execution.md`:
- Line 11: Add the required blank lines around the headings at lines 11, 63, and
196, and before and after the fenced code blocks at lines 69, 77, 89, 130, 137,
141, 158, and 170 in the markdown document, without changing its content.
In `@skills/nemo-evaluator-plugin/references/metric-selection.md`:
- Around line 31-36: Update the metric discovery commands in
skills/nemo-evaluator-plugin/references/metric-selection.md:31-36 to prefix both
nemo invocations with uv run, ensuring repository checkouts use the project CLI.
Apply the same execution-context fix to the secret commands in
skills/nemo-evaluator-plugin/references/api-auth.md:15-28 by prefixing them with
uv run, or explicitly labeling them as installed-platform commands.
In `@skills/nemo-evaluator-plugin/references/resources.md`:
- Around line 21-56: Update the snippet before its first client.evaluator usage
to either initialize client with NeMoPlatform or explicitly state that an
existing client is required; ensure the resulting example is self-contained and
runnable.
In `@skills/nemo-evaluator-plugin/SKILL.md`:
- Around line 38-53: Update the “CLI Interface” prerequisites in SKILL.md to
require setting NMP_BASE_URL to http://localhost:8080, checking whether the
local platform instance is already running, and verifying
$NMP_BASE_URL/health/ready before any CLI commands. Instruct users to stop and
follow SETUP.md if the readiness check fails.
---
Outside diff comments:
In `@plugins/nemo-evaluator/tests/test_evaluate_job.py`:
- Around line 71-74: Update the agent-evaluation compiler test around
EXAMPLE_SPEC_PATHS and
test_checked_durable_fabric_job_is_a_valid_agent_eval_spec to include the
fabric_agent_eval.json fixture. Load that fixture, convert it with
AgentEvalJob.to_spec, and compile the resulting spec using AgentEvalJob.compile
instead of relying on the separate synthetic spec.
🪄 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: 5e46cf47-c6af-43ee-b959-4333635877ad
📒 Files selected for processing (25)
docs/evaluator/agent-eval/index.mdxdocs/evaluator/index.mdxpackages/nemo_evaluator_sdk/examples/fabric_harness_runtimes.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_integration.pyplugins/nemo-evaluator/README.mdplugins/nemo-evaluator/src/nemo_evaluator/cli.pyplugins/nemo-evaluator/src/nemo_evaluator/docs/index.mdplugins/nemo-evaluator/tests/test_evaluate_job.pyplugins/nemo-evaluator/tests/test_skill_examples.pyscript/dev-install-fabric.shskills/nemo-evaluator-plugin/SKILL.mdskills/nemo-evaluator-plugin/assets/examples/plugin_sdk_examples.pyskills/nemo-evaluator-plugin/assets/specs/exact_match_benchmark.jsonskills/nemo-evaluator-plugin/assets/specs/exact_match_metric.jsonskills/nemo-evaluator-plugin/assets/specs/fabric_agent_eval.jsonskills/nemo-evaluator-plugin/assets/specs/llm_as_judge.jsonskills/nemo-evaluator-plugin/references/agent-evaluation.mdskills/nemo-evaluator-plugin/references/api-auth.mdskills/nemo-evaluator-plugin/references/evaluation-shapes.mdskills/nemo-evaluator-plugin/references/execution.mdskills/nemo-evaluator-plugin/references/llm-judge.mdskills/nemo-evaluator-plugin/references/metric-selection.mdskills/nemo-evaluator-plugin/references/resources.mdskills/nemo-evaluator-plugin/references/troubleshooting.mdskills/nemo-evaluator-plugin/scripts/generate_example_specs.py
💤 Files with no reviewable changes (1)
- skills/nemo-evaluator-plugin/assets/specs/exact_match_benchmark.json
|
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
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)
skills/nemo-evaluator-plugin/references/troubleshooting.md (1)
3-4: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd prerequisites before the first command.
Add a
Prerequisitessection beforeInspect the installed contracts. State that thenemo evaluatorCLI must be available and that repository checkouts must run commands throughuv run.As per coding guidelines, documentation pages must list prerequisites at the top.
🤖 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 `@skills/nemo-evaluator-plugin/references/troubleshooting.md` around lines 3 - 4, Add a Prerequisites section at the top of the troubleshooting document, before “Inspect the installed contracts,” stating that the nemo evaluator CLI must be available and that repository checkouts must invoke commands through uv run. Preserve the existing command instructions and CLI wording.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.
Outside diff comments:
In `@skills/nemo-evaluator-plugin/references/troubleshooting.md`:
- Around line 3-4: Add a Prerequisites section at the top of the troubleshooting
document, before “Inspect the installed contracts,” stating that the nemo
evaluator CLI must be available and that repository checkouts must invoke
commands through uv run. Preserve the existing command instructions and CLI
wording.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: d0e6398e-b4c8-45c8-a412-fc81c9c5a842
📒 Files selected for processing (15)
.cursor/hooks/state/continual-learning.jsondocs/evaluator/agent-eval/index.mdxdocs/evaluator/index.mdxpackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_integration.pyplugins/nemo-evaluator/README.mdplugins/nemo-evaluator/tests/test_agent_evaluate.pyplugins/nemo-evaluator/tests/test_evaluate_job.pyplugins/nemo-evaluator/tests/test_skill_examples.pyscript/dev-install-fabric.shskills/nemo-evaluator-plugin/SKILL.mdskills/nemo-evaluator-plugin/references/agent-evaluation.mdskills/nemo-evaluator-plugin/references/execution.mdskills/nemo-evaluator-plugin/references/metric-selection.mdskills/nemo-evaluator-plugin/references/resources.mdskills/nemo-evaluator-plugin/references/troubleshooting.md
🚧 Files skipped from review as they are similar to previous changes (12)
- docs/evaluator/agent-eval/index.mdx
- script/dev-install-fabric.sh
- skills/nemo-evaluator-plugin/references/metric-selection.md
- skills/nemo-evaluator-plugin/references/resources.md
- plugins/nemo-evaluator/tests/test_skill_examples.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_integration.py
- skills/nemo-evaluator-plugin/references/agent-evaluation.md
- plugins/nemo-evaluator/README.md
- plugins/nemo-evaluator/tests/test_evaluate_job.py
- skills/nemo-evaluator-plugin/SKILL.md
- docs/evaluator/index.mdx
- skills/nemo-evaluator-plugin/references/execution.md
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
|
/nvskills-ci |
|
/nvskills-ci |
| command group is `nemo evaluator`; the legacy generated `nemo evaluation` | ||
| surface is not used for new workflows. |
There was a problem hiding this comment.
the legacy generated
nemo evaluationsurface is not used for new workflows.
Not sure we need to call this out.
| Durable target variants are: | ||
|
|
||
| ## Python Examples | ||
| - `ModelTarget` | ||
| - `AgentTarget` | ||
| - `CodexRunnerTarget` | ||
| - `FabricRunnerTarget` | ||
| - `HarborRunnerTarget` |
There was a problem hiding this comment.
Although I think this is how this works, I think we need to revisit our local->remote execution APIs again. I don't think it was good that we didn't follow the dataset-based evals and allow injecting a Backend to the AgentEvaluator instance.
Something I'd like to discuss this week.
| ### SDK Execution | ||
| Plugin SDK execution is not supported for task-driven evaluation. Use the standalone Python SDK instead, which is available for local execution. |
There was a problem hiding this comment.
We need to address this gap tbh. This shouldn't be true and we need a way to take our "locally defined agent eval" and port it to the backend.
| @@ -1,93 +1,156 @@ | |||
| # Evaluator Plugin Reference | |||
|
|
|||
| The evaluator plugin is a first-party for evaluator functionality. It keeps the plugin identity separate from the legacy `/apis/evaluation` service while proving the basic surfaces needed for SDK-backed jobs. | |||
There was a problem hiding this comment.
should we remove this doc? What's the purpose of this doc compared to README.md and SKILL.md? We keep repeating api walkthrough in 3 places
Summary
.run()from SKILLS since it's going awayIn Progress
Pass nv-carps pipeline
Exploring feasibility of using nmp container as an agent runtime in the eval
Linear: AALGO-450
Filled out a few stories to resolve some identified gaps: https://linear.app/nvidia/project/evaluator-plugin-agenteval-support-29a12b9a6a95/overview
Summary by CodeRabbit
New Features
Documentation
Tests