feat(evaluator)!: typed run metadata and a required runner identity contract - #1013
Open
SandyChapman wants to merge 2 commits into
Open
feat(evaluator)!: typed run metadata and a required runner identity contract#1013SandyChapman wants to merge 2 commits into
SandyChapman wants to merge 2 commits into
Conversation
SandyChapman
force-pushed
the
aalgo-451-run-metadata/schapman
branch
from
July 31, 2026 17:06
92370ff to
74a1ad5
Compare
SandyChapman
marked this pull request as ready for review
July 31, 2026 17:06
Contributor
📝 WalkthroughWalkthroughThe SDK replaces untyped benchmark metadata with typed run metadata. It records labels, runner provenance, target identity, timing, duration, and SDK version. Persisted metadata now uses ChangesRun metadata migration
Sequence Diagram(s)sequenceDiagram
participant EvaluationPipeline
participant AgentEvalEvaluator
participant AgentTaskRunner
participant RunPersistence
EvaluationPipeline->>AgentEvalEvaluator: submit labels
AgentEvalEvaluator->>AgentTaskRunner: request runner_info()
AgentTaskRunner-->>AgentEvalEvaluator: return runner provenance
AgentEvalEvaluator->>RunPersistence: persist RunMetadata
RunPersistence-->>EvaluationPipeline: expose metadata.json
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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
🤖 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/agent_eval/runtimes/docker_sandbox.py`:
- Around line 113-119: Update runner_info() in
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/docker_sandbox.py:113-119
to persist effective instructions. In
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/callable_runtime.py:56-65,
persist a module-qualified callable identity; in
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/codex/runtime.py:79-89,
persist codex_bin and custom prompt-builder identity; in
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/container_runtime.py:180-189,
use self._provider.name and persist image, adapter, skills, and curated
non-secret Fabric configuration; in
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/runtime.py:163-169,
persist the harness adapter, skills, and curated non-secret Fabric
configuration. Add persistence tests covering each configuration variation.
In
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/gym_runtime.py`:
- Around line 335-353: Add the missing result-shaping configuration fields to
the metadata so different runs can be distinguished. In
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/gym_runtime.py
at lines 335-353, extend the config dictionary in the RunnerInfo returned by
runner_info to include bind_resources_server, env_overrides, and reward_key,
which all affect Gym command generation or rollout adaptation. In
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.py
at lines 203-214, persist n_attempts and the effective resolved job_dir value
(computing job_dir early rather than deferring resolution to run_tasks so
metadata records the actual path instead of null), since these settings shape
native mode behavior and results.
In `@packages/nemo_evaluator_sdk/tests/agent_eval/test_run_metadata.py`:
- Around line 45-56: Extend test_every_shipped_runner_reports_a_stable_name to
import and instantiate the shipped Fabric, Fabric Container, Gym, and Harbor
runner classes, then assert each runner_info().name matches its curated stable
identity alongside the existing Callable, Codex, and Docker Sandbox assertions.
🪄 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: c07abc81-6426-4933-a40a-56fe5dc5a24d
⛔ Files ignored due to path filters (12)
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/evaluator.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/persistence.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/results.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/callable_runtime.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/codex/runtime.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/docker_sandbox.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/container_runtime.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/runtime.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/gym_runtime.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/harbor_runtime.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/tasks.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/trials.pyis excluded by!sdk/**
📒 Files selected for processing (22)
packages/nemo_evaluator_sdk/examples/codex_docker/example.pypackages/nemo_evaluator_sdk/examples/profbench/README.mdpackages/nemo_evaluator_sdk/examples/profbench/runner.pypackages/nemo_evaluator_sdk/examples/run_agent_eval/gating.pypackages/nemo_evaluator_sdk/examples/run_agent_eval/pipeline.pypackages/nemo_evaluator_sdk/examples/run_agent_eval/run_agent_eval.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/evaluator.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/persistence.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/results.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/callable_runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/codex/runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/docker_sandbox.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/container_runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/gym_runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/tasks.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/trials.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_codex_docker_example.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_evaluator.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_persistence.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_run_metadata.py
💤 Files with no reviewable changes (1)
- packages/nemo_evaluator_sdk/tests/agent_eval/test_persistence.py
Contributor
|
…ontract
Replace the untyped `benchmark: dict[str, Any]` bag on AgentEvalResult with
typed run provenance, and require runners to identify themselves.
A finished run could not answer "what produced this, with what settings, and
how long did it take?" — there was no timing, no target identity, and no SDK
version. Callers improvised provenance inside the benchmark dict with no
convention ({"name": ...} vs {"benchmark": ...}, plus mode/backend/task/
score_source), and the auto-derived value shape-shifted between str and
list[str] depending on how many benchmarks the tasks declared.
- AgentEvalResult.benchmark -> metadata: RunMetadata, carrying typed labels,
target, started_at/finished_at/duration_sec, and sdk_version.
- runner_info() -> RunnerInfo is now part of the AgentTaskRunner protocol.
Identity is universal rather than an optional capability, so every shipped
runner implements it with a curated name (gym, harbor, docker_sandbox,
callable, codex_cli, codex_docker_cli, fabric, fabric_container) plus the
settings that shape its results. Credentials are deliberately excluded.
- Drop `benchmark` entirely: nothing computed on it, it duplicated what labels
already express, and it invited misuse as a run name.
- Bundle artifact benchmark.json -> metadata.json.
BREAKING CHANGE: AgentTaskRunner now requires runner_info(). Because the
protocol is runtime_checkable and isinstance-dispatched, a runner without it
no longer matches and raises NotImplementedError: unsupported agent-eval
target type.
Signed-off-by: Sandy Chapman <schapman@nvidia.com>
SandyChapman
force-pushed
the
aalgo-451-run-metadata/schapman
branch
from
July 31, 2026 17:31
74a1ad5 to
23d4202
Compare
…he plugin Addresses CI and review feedback on the run-metadata change. - Every runner now records the settings that actually shape its results, so two runs that behaved differently cannot record identical provenance: gym gains bind_resources_server/env_overrides/reward_key; harbor gains n_attempts plus the configured job location; docker_sandbox its effective instructions; codex its codex_bin and prompt builder; fabric its adapter and skills. - FabricContainerRuntime recorded str(provider), a memory-address repr, so identical runs produced differing metadata. Use the provider's declared name. - Callable identities are module-qualified; a bare __qualname__ is ambiguous across modules. - The evaluator plugin forwarded AgentEvalSpec.benchmark into a parameter that no longer exists. Rename the spec field to labels to match. AgentEvalSpec is plugin-internal and absent from the OpenAPI surface, so this is not an API-visible change. Harbor's concrete job directory is deliberately not resolved early: the native job name defaults to a run-time timestamp, so the configured jobs_dir/job_name are recorded rather than a fabricated path. Tests cover all eight shipped runners, asserting both the curated name and the presence of each runner's result-shaping configuration. Signed-off-by: Sandy Chapman <schapman@nvidia.com>
SandyChapman
force-pushed
the
aalgo-451-run-metadata/schapman
branch
from
July 31, 2026 19:04
23d4202 to
5c409f4
Compare
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
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/runtime.py`:
- Around line 169-181: Update runner_info() to report the effective Fabric model
from the same configuration source used by _compose_config(), including when
self._model is None and the model is supplied through self._config. Preserve the
existing explicit self._model behavior, or reject config-only model settings
consistently if that is the established contract.
🪄 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: 2ddfc9c0-c0fa-4c6c-abb8-b019203d4a26
⛔ Files ignored due to path filters (8)
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/callable_runtime.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/codex/runtime.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/docker_sandbox.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/container_runtime.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/runtime.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/gym_runtime.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/harbor_runtime.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/trials.pyis excluded by!sdk/**
📒 Files selected for processing (11)
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/callable_runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/codex/runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/docker_sandbox.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/container_runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/gym_runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/trials.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_run_metadata.pyplugins/nemo-evaluator/src/nemo_evaluator/jobs/agent_evaluate.pyplugins/nemo-evaluator/src/nemo_evaluator/jobs/agent_spec.py
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/gym_runtime.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/callable_runtime.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/docker_sandbox.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/container_runtime.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/codex/runtime.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_run_metadata.py
- plugins/nemo-evaluator/src/nemo_evaluator/jobs/agent_evaluate.py
- plugins/nemo-evaluator/src/nemo_evaluator/jobs/agent_spec.py
Comment on lines
+169
to
+181
| def runner_info(self) -> RunnerInfo: | ||
| """Identify this runner and the Fabric settings that shape its results.""" | ||
| return RunnerInfo( | ||
| name=self._runtime_name, | ||
| kind="runner", | ||
| config={ | ||
| "model": self._model, | ||
| "timeout_s": self._timeout_s, | ||
| "adapter_id": self._adapter_id(), | ||
| "skills": [skill.name for skill in self._skill_set.skills], | ||
| }, | ||
| ) | ||
|
|
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Record the effective Fabric model.
When self._model is None and the model is configured in self._config, runner_info() records "model": None. _compose_config() preserves and uses that configured model. Runs with different models can therefore receive identical runner provenance. Derive the reported model from the same effective configuration source, or reject config-only model settings.
🤖 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/agent_eval/runtimes/fabric/runtime.py`
around lines 169 - 181, Update runner_info() to report the effective Fabric
model from the same configuration source used by _compose_config(), including
when self._model is None and the model is supplied through self._config.
Preserve the existing explicit self._model behavior, or reject config-only model
settings consistently if that is the established contract.
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.
Closes AALGO-451.
Problem
A finished run couldn't answer "what produced this, with what settings, and how long did it take?" — no timing, no target identity, no SDK version.
Callers improvised provenance inside the untyped
benchmark: dict[str, Any]bag with no convention —{"name": ...}in one example,{"benchmark": ...}in another, plusmode/backend/task/score_source. The auto-derived value also shape-shifted betweenstrandlist[str]depending on how many benchmarks the tasks declared.Change
AgentEvalResult.benchmark→metadata: RunMetadata— typedlabels,target,started_at/finished_at/duration_sec,sdk_version.runner_info() -> RunnerInfois now part ofAgentTaskRunner. Identity is universal, not an optional capability, so every shipped runner implements it with a curated name (gym,harbor,docker_sandbox,callable,codex_cli,codex_docker_cli,fabric,fabric_container) plus the settings that shape its results. Credentials are deliberately excluded —FabricContainerRuntimereports its provider but never_secrets.benchmarkdropped entirely (field, config parameter, and task-metadata derivation). Nothing computed on it, it duplicated whatlabelsalready expresses, and it invited misuse as a run name — one in-tree example was using it for exactly that.benchmark.json→metadata.json.Example output from a live Gym run:
{ "target": { "name": "gym", "kind": "runner", "config": { "resources_server": "mcqa", "agent": "simple_agent", "model_type": "inference_provider", "num_repeats": 2 } }, "started_at": "2026-07-31T11:46:40Z", "finished_at": "2026-07-31T14:14:08Z", "duration_sec": 8848.3, "sdk_version": "0.0.0", "labels": {} }Breaking change
AgentTaskRunnernow requiresrunner_info(). The protocol is@runtime_checkableand isinstance-dispatched, so a runner lacking it no longer matches and raisesNotImplementedError: unsupported agent-eval target type. All in-tree runners and test doubles are updated.Verification
main(23 ragas + 1 git_patch + 1 fabric live-integration, all pre-existing); 1287 passing vs main's 1283 — the +4 are new provenance tests.tyat the pre-existing 18 diagnostics.make vendor.Draft for review. A follow-up PR carries the AALGO-434 aggregation work (pass@k, runner-provided aggregations, sample/population stats), which builds on this.
Summary by CodeRabbit
metadata.json.