Skip to content

Commit 5e7d351

Browse files
SandyChapmanclaude
andcommitted
refactor(evaluator)!: make runner and agent-eval metrics built-in
Storing a Gym or Harbor taskset meant bundling its reward metric with `CloudpickleMetricBundlePackager()` -- the opt-in the docs correctly frame as "shipping custom code" -- for a metric the platform owns. Same for the three agent-eval metrics. None of them is custom: their entire state is JSON-able scalars, so they belong in `MetricVariants` alongside the other 23 built-ins. Promotes five: `GymRewardMetric`, `HarborRewardMetric`, `AgentPhaseSuccessMetric`, `EvidencePresenceMetric`, `SkillUsedMetric`. Each gains a `MetricType` member and subclasses `MetricBase` directly, carrying its own discriminator and fields. No separate config class. The `values/` config + `metrics/` runtime split exists to keep heavy runtime deps out of the config layer -- `metrics/bleu.py` imports sacrebleu at module scope, so `values.metrics.BLEU` stays importable without it. These five add no deps over their config, every config class in that module has exactly one consumer (its own runtime subclass), and no values-level union requires them to be co-located, so the split would be indirection that buys nothing. The type strings are unchanged from what the metrics already emitted (`gym_reward`, `harbor_reward`, ...), so nothing moves on the wire; what changes is that they now bundle inline and rehydrate without executing pickled code. BREAKING: `type` is now a fixed discriminator, so the `metric_type` override is gone. `Field(discriminator="type")` cannot express a per-caller type string. The override was used in exactly one place repo-wide -- a test fixture -- and cost every caller the cloudpickle opt-in. The two reward metrics could not simply subclass `MetricBase` where they lived. `MetricBase` drags the dataset-schema stack (jinja2, jsonschema), and `harbor_runtime` is on the optimizer's light import path, guarded by `test_agent_eval_import_does_not_pull_the_execution_stack`. Defining them there turned that test red. They now live in `metrics/runner_rewards.py` on the heavy side, and `harbor_runtime` re-exports `HarborRewardMetric` through a module `__getattr__` with a `TYPE_CHECKING` declaration, so `from ...harbor_runtime import HarborRewardMetric` still works and still type-checks while the light path stays light. The two default-metric construction sites import locally for the same reason. `gym/results.py` gets no such shim. It declares no `__all__`, never published `GymRewardMetric` as part of its surface, and nothing imports the metric from that path -- the deep-path imports are all private helpers. The public path is `from ...runtimes.gym import GymRewardMetric`, which the package `__init__` serves from the canonical module. The skill's curated metric list deliberately does not gain these five. That page is about choosing a scorer for your data, and none of them is a choice -- they arrive with the runner or the harness. `test_skill_examples` records the reasoning next to the existing `tunable-rag-evaluator` exemption. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Sandy Chapman <schapman@nvidia.com>
1 parent 965a736 commit 5e7d351

18 files changed

Lines changed: 384 additions & 224 deletions

File tree

packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/metrics.py

Lines changed: 37 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@
1919
import json
2020
import logging
2121
from collections.abc import Mapping
22-
from typing import Any
22+
from typing import Any, ClassVar, Literal
2323

24-
from nemo_evaluator_sdk.agent_eval.trials import EVIDENCE_FINAL_STATE
24+
from nemo_evaluator_sdk.agent_eval.trials import EVIDENCE_FINAL_STATE, EVIDENCE_TRACE
25+
from nemo_evaluator_sdk.enums import MetricType
2526
from nemo_evaluator_sdk.metrics.protocol import (
2627
CandidateOutput,
2728
MetricInput,
@@ -30,8 +31,8 @@
3031
MetricResult,
3132
)
3233
from nemo_evaluator_sdk.values.atif import Trajectory
33-
from nemo_evaluator_sdk.values.evidence import EVIDENCE_TRACE
34-
from pydantic import BaseModel, ConfigDict, ValidationError
34+
from nemo_evaluator_sdk.values.metrics import MetricBase
35+
from pydantic import BaseModel, ConfigDict, Field, ValidationError
3536

3637
logger = logging.getLogger(__name__)
3738

@@ -45,19 +46,18 @@
4546
)
4647

4748

48-
class AgentPhaseSuccessMetric:
49+
class AgentPhaseSuccessMetric(MetricBase):
4950
"""Emit ``True`` when the agent phase exited successfully, else ``False``.
5051
51-
The metric ``type`` is overridable via the ``metric_type`` class attribute so
52-
callers can namespace it; the output name stays ``agent_phase_success`` (which
53-
gating reads as a reward signal — ``True``/``False`` coerces to ``1.0``/``0.0``).
54-
"""
52+
The output name stays ``agent_phase_success`` (which gating reads as a reward
53+
signal — ``True``/``False`` coerces to ``1.0``/``0.0``).
5554
56-
metric_type: str = "agent_phase_success"
55+
A built-in metric type, so it bundles inline and needs no cloudpickle opt-in to be
56+
stored on a task. ``type`` is therefore a fixed discriminator and no longer
57+
overridable per caller.
58+
"""
5759

58-
@property
59-
def type(self) -> str:
60-
return self.metric_type
60+
type: Literal[MetricType.AGENT_PHASE_SUCCESS] = MetricType.AGENT_PHASE_SUCCESS
6161

6262
def output_spec(self) -> list[MetricOutputSpec]:
6363
return [MetricOutputSpec.boolean("agent_phase_success")]
@@ -70,51 +70,43 @@ async def compute_scores(self, input: MetricInput) -> MetricResult:
7070
return MetricResult(outputs=[MetricOutput(name="agent_phase_success", value=agent_ok)])
7171

7272

73-
class EvidencePresenceMetric:
73+
class EvidencePresenceMetric(MetricBase):
7474
"""Emit ``True`` when a named filesystem evidence directory exists (and is non-empty).
7575
7676
Reads ``candidate.evidence`` directly — the canonical metric-over-evidence
7777
pattern — so the result reflects what the agent actually produced on disk,
7878
not a reward stamped into metadata by a verifier.
7979
"""
8080

81-
def __init__(
82-
self,
83-
*,
84-
evidence_name: str = EVIDENCE_FINAL_STATE,
85-
output_name: str = "evidence_present",
86-
require_non_empty: bool = True,
87-
) -> None:
88-
self._evidence_name = evidence_name
89-
self._output_name = output_name
90-
self._require_non_empty = require_non_empty
91-
92-
@property
93-
def type(self) -> str:
94-
return "evidence_presence"
81+
type: Literal[MetricType.EVIDENCE_PRESENCE] = MetricType.EVIDENCE_PRESENCE
82+
evidence_name: str = Field(default=EVIDENCE_FINAL_STATE, description="Evidence directory to look for.")
83+
output_name: str = Field(default="evidence_present", description="Name of the emitted boolean score.")
84+
require_non_empty: bool = Field(
85+
default=True, description="Require the evidence directory to be non-empty, not merely present."
86+
)
9587

9688
def output_spec(self) -> list[MetricOutputSpec]:
97-
return [MetricOutputSpec.boolean(self._output_name)]
89+
return [MetricOutputSpec.boolean(self.output_name)]
9890

9991
async def compute_scores(self, input: MetricInput) -> MetricResult:
10092
present = False
10193
evidence = input.candidate.evidence
102-
if evidence is not None and evidence.get(self._evidence_name) is not None:
94+
if evidence is not None and evidence.get(self.evidence_name) is not None:
10395
try:
104-
handle = await evidence.filesystem(self._evidence_name)
96+
handle = await evidence.filesystem(self.evidence_name)
10597
if await handle.exists():
106-
present = bool(await handle.iter_paths(recursive=True)) if self._require_non_empty else True
98+
present = bool(await handle.iter_paths(recursive=True)) if self.require_non_empty else True
10799
except (KeyError, ValueError) as exc:
108100
logger.warning(
109101
"EvidencePresenceMetric scored False: could not resolve evidence %r for output %r: %s",
110-
self._evidence_name,
111-
self._output_name,
102+
self.evidence_name,
103+
self.output_name,
112104
exc,
113105
)
114-
return MetricResult(outputs=[MetricOutput(name=self._output_name, value=present)])
106+
return MetricResult(outputs=[MetricOutput(name=self.output_name, value=present)])
115107

116108

117-
class SkillUsedMetric:
109+
class SkillUsedMetric(MetricBase):
118110
"""Emit ``skill_present`` and ``skill_used`` so an eval can flag a failure to use an injected skill.
119111
120112
* ``skill_present`` — ``True`` when one or more skills were injected into the trial. Reads
@@ -134,18 +126,13 @@ class SkillUsedMetric:
134126
With no skill present, both outputs are ``False``.
135127
"""
136128

137-
metric_type: str = "skill_used"
138-
OUTPUT_PRESENT: str = "skill_present"
139-
OUTPUT_USED: str = "skill_used"
140-
# Metadata key skill-aware runtimes stamp the provenance list under (matches the fabric runtime).
141-
_SKILLS_KEY: str = "skills"
129+
type: Literal[MetricType.SKILL_USED] = MetricType.SKILL_USED
130+
trace_evidence: str = Field(default=EVIDENCE_TRACE, description="Trace evidence to scan for skill usage.")
142131

143-
def __init__(self, *, trace_evidence: str = EVIDENCE_TRACE) -> None:
144-
self._trace_evidence = trace_evidence
145-
146-
@property
147-
def type(self) -> str:
148-
return self.metric_type
132+
OUTPUT_PRESENT: ClassVar[str] = "skill_present"
133+
OUTPUT_USED: ClassVar[str] = "skill_used"
134+
# Metadata key skill-aware runtimes stamp the provenance list under (matches the fabric runtime).
135+
_SKILLS_KEY: ClassVar[str] = "skills"
149136

150137
def output_spec(self) -> list[MetricOutputSpec]:
151138
return [
@@ -175,15 +162,15 @@ async def _any_skill_used(self, candidate: CandidateOutput, provenances: list[Ma
175162
if not locations:
176163
return False
177164
evidence = candidate.evidence
178-
if evidence is None or evidence.get(self._trace_evidence) is None:
165+
if evidence is None or evidence.get(self.trace_evidence) is None:
179166
return False
180167
try:
181-
trajectory = await (await evidence.trace(self._trace_evidence)).trace()
168+
trajectory = await (await evidence.trace(self.trace_evidence)).trace()
182169
except (KeyError, ValueError, ValidationError, OSError) as exc:
183170
# Best-effort: a missing/malformed/invalid trajectory must score skill_used=False, not raise.
184171
# ValidationError covers Trajectory.model_validate; OSError covers the underlying file read.
185172
logger.warning(
186-
"SkillUsedMetric scored skill_used=False: could not read trace %r: %s", self._trace_evidence, exc
173+
"SkillUsedMetric scored skill_used=False: could not read trace %r: %s", self.trace_evidence, exc
187174
)
188175
return False
189176
return any(_trajectory_references(trajectory, loc) for loc in locations)

packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/gym/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@
7272

7373
from nemo_evaluator_sdk.agent_eval.runtimes.gym.config import DEFAULT_REWARD_KEY, GymRuntimeConfig
7474
from nemo_evaluator_sdk.agent_eval.runtimes.gym.dataset import discover_gym_tasks
75-
from nemo_evaluator_sdk.agent_eval.runtimes.gym.results import GymRewardMetric
7675
from nemo_evaluator_sdk.agent_eval.runtimes.gym.runtime import GymAgentTaskRunner
76+
from nemo_evaluator_sdk.metrics.runner_rewards import GymRewardMetric
7777

7878
__all__ = [
7979
"DEFAULT_REWARD_KEY",

packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/gym/dataset.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
NG_TASK_INDEX,
2323
_read_jsonl,
2424
)
25-
from nemo_evaluator_sdk.agent_eval.runtimes.gym.results import GymRewardMetric
2625
from nemo_evaluator_sdk.agent_eval.tasks import AgentEvalTask
2726

2827
logger = logging.getLogger(__name__)
@@ -94,6 +93,13 @@ def _render_instruction(responses_create_params: Mapping[str, Any]) -> str:
9493
return "\n\n".join(part for part in parts if part).strip()
9594

9695

96+
def _default_gym_metric() -> object:
97+
"""The default reward metric, imported lazily (see ``metrics.runner_rewards``)."""
98+
from nemo_evaluator_sdk.metrics.runner_rewards import GymRewardMetric
99+
100+
return GymRewardMetric()
101+
102+
97103
def discover_gym_tasks(dataset: str | Path, *, metrics: Sequence[Any] | None = None) -> list[AgentEvalTask]:
98104
"""Build one :class:`AgentEvalTask` per distinct row in a Gym dataset (jsonl).
99105
@@ -156,7 +162,7 @@ def discover_gym_tasks(dataset: str | Path, *, metrics: Sequence[Any] | None = N
156162
**({"instruction": instruction} if instruction else {}),
157163
"gym_row": params,
158164
},
159-
metrics=list(metrics) if metrics is not None else [GymRewardMetric()],
165+
metrics=list(metrics) if metrics is not None else [_default_gym_metric()],
160166
metadata={
161167
"gym_dataset_path": str(dataset),
162168
# Everything except responses_create_params, which already lives in inputs['gym_row'].

packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/gym/results.py

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
)
2727
from nemo_evaluator_sdk.agent_eval.tasks import AgentEvalTask
2828
from nemo_evaluator_sdk.agent_eval.trials import AgentEvalTrial, AgentEvalTrialStatus, AgentOutput
29-
from nemo_evaluator_sdk.metrics.protocol import MetricInput, MetricOutput, MetricOutputSpec, MetricResult
3029
from nemo_evaluator_sdk.values.evidence import CandidateEvidence, EvidenceDescriptor
3130
from nemo_evaluator_sdk.values.results import AggregateRangeScore, AggregateScalarScore, AggregateScore
3231

@@ -43,33 +42,6 @@
4342
#: rows that survives a round-trip: Gym mutates ``responses_create_params`` (even the prompt) and
4443
#: copies only a fixed allowlist of row keys onto the result, so no field we invent comes back. Gym
4544
#: *honors* a caller-supplied ``_ng_task_index``, which is what makes the join here deterministic.
46-
class GymRewardMetric:
47-
"""Score the Gym verifier reward stamped onto trial metadata.
48-
49-
The Gym analogue of :class:`HarborRewardMetric`: reads the per-trial ``reward``
50-
off the candidate metadata (populated by :class:`GymAgentTaskRunner`); a trial
51-
with no reward is left **unscored** (``None`` → ``nan``), excluded from the mean
52-
and surfaced as ``nan_count`` rather than counted as a spurious ``0.0``. Gym owns
53-
the scoring — this metric only surfaces it (Evaluator does not re-derive the reward).
54-
"""
55-
56-
def __init__(self, *, output_name: str = "reward", metric_type: str = "gym_reward") -> None:
57-
self._output_name = output_name
58-
self._metric_type = metric_type
59-
60-
@property
61-
def type(self) -> str:
62-
return self._metric_type
63-
64-
def output_spec(self) -> list[MetricOutputSpec]:
65-
return [MetricOutputSpec.continuous_score(self._output_name)]
66-
67-
async def compute_scores(self, input: MetricInput) -> MetricResult:
68-
reward = input.candidate.metadata.get("reward")
69-
value = float(reward) if reward is not None else None
70-
return MetricResult(outputs=[MetricOutput(name=self._output_name, value=value)])
71-
72-
7345
def _agent_never_ran(record: Mapping[str, Any]) -> bool:
7446
"""True when a result record shows the agent produced nothing *and* never called the model.
7547

packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.py

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@
3131

3232
from __future__ import annotations
3333

34+
from typing import TYPE_CHECKING
35+
36+
if TYPE_CHECKING:
37+
from nemo_evaluator_sdk.metrics.runner_rewards import HarborRewardMetric
38+
3439
import contextlib
3540
import hashlib
3641
import importlib.machinery
@@ -60,7 +65,7 @@
6065
TrialError,
6166
standard_evidence_descriptors,
6267
)
63-
from nemo_evaluator_sdk.metrics.protocol import Metric, MetricInput, MetricOutput, MetricOutputSpec, MetricResult
68+
from nemo_evaluator_sdk.metrics.protocol import Metric
6469
from nemo_evaluator_sdk.values.evidence import CandidateEvidence
6570
from pydantic import BaseModel, ConfigDict, Field, model_validator
6671

@@ -174,32 +179,6 @@ def _agent_dir_needs_import_path(self) -> HarborRuntimeConfig:
174179
return self
175180

176181

177-
class HarborRewardMetric:
178-
"""Score the verifier reward Harbor stamped onto trial metadata.
179-
180-
Reads ``reward`` from the candidate metadata (populated by
181-
:func:`build_trials_from_job_dir`); a trial with no verifier reward scores
182-
``0.0``. This is the Harbor analogue of the example ``VerifierRewardMetric``
183-
— a reward-off-metadata scorer.
184-
"""
185-
186-
def __init__(self, *, output_name: str = "reward", metric_type: str = "harbor_reward") -> None:
187-
self._output_name = output_name
188-
self._metric_type = metric_type
189-
190-
@property
191-
def type(self) -> str:
192-
return self._metric_type
193-
194-
def output_spec(self) -> list[MetricOutputSpec]:
195-
return [MetricOutputSpec.continuous_score(self._output_name)]
196-
197-
async def compute_scores(self, input: MetricInput) -> MetricResult:
198-
reward = input.candidate.metadata.get("reward")
199-
value = float(reward) if reward is not None else 0.0
200-
return MetricResult(outputs=[MetricOutput(name=self._output_name, value=value)])
201-
202-
203182
def _effective_harbor_agent(config: HarborRuntimeConfig | None) -> str | None:
204183
"""The agent a run will actually use, mirroring ``run_job``'s resolution order.
205184
@@ -1365,7 +1344,7 @@ def discover_harbor_tasks(dataset_path: str | Path) -> list[AgentEvalTask]:
13651344
# lives in `inputs["instruction"]`.
13661345
intent=task_name,
13671346
inputs={"instruction": instruction},
1368-
metrics=[HarborRewardMetric()],
1347+
metrics=[_harbor_reward_metric()],
13691348
metadata={"harbor_dataset_path": str(dataset_path), "harbor_task_dir": str(task_dir)},
13701349
)
13711350
)
@@ -1493,3 +1472,24 @@ def reward_payload_from_result(
14931472
"run_harbor_eval",
14941473
"scoped_harbor_agent_import",
14951474
]
1475+
1476+
1477+
def _harbor_reward_metric() -> "HarborRewardMetric":
1478+
"""Build the default reward metric, importing it lazily to keep this module light."""
1479+
from nemo_evaluator_sdk.metrics.runner_rewards import HarborRewardMetric
1480+
1481+
return HarborRewardMetric()
1482+
1483+
1484+
def __getattr__(name: str) -> object:
1485+
"""Re-export ``HarborRewardMetric`` without importing the metric stack at module scope.
1486+
1487+
Defining it here would pull ``MetricBase`` and the dataset-schema machinery (jinja2,
1488+
jsonschema) onto the optimizer's light import path. See
1489+
``nemo_evaluator_sdk.metrics.runner_rewards``.
1490+
"""
1491+
if name == "HarborRewardMetric":
1492+
from nemo_evaluator_sdk.metrics.runner_rewards import HarborRewardMetric
1493+
1494+
return HarborRewardMetric
1495+
raise AttributeError(f"module {__name__!r} has no attribute {name!r}")

packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/enums.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,15 @@ class MetricType(str, Enum):
3636
NOISE_SENSITIVITY = "noise_sensitivity"
3737
TUNABLE_RAG_EVALUATOR = "tunable-rag-evaluator"
3838

39+
# Runner-owned rewards: the runner scores, these surface it.
40+
GYM_REWARD = "gym_reward"
41+
HARBOR_REWARD = "harbor_reward"
42+
43+
# Agent-eval scoring over trial metadata and evidence.
44+
AGENT_PHASE_SUCCESS = "agent_phase_success"
45+
EVIDENCE_PRESENCE = "evidence_presence"
46+
SKILL_USED = "skill_used"
47+
3948
SYSTEM = "system"
4049

4150

0 commit comments

Comments
 (0)