feat(evaluator): aggregate agent-eval results natively and import Gym's own - #1065
Open
SandyChapman wants to merge 2 commits into
Open
feat(evaluator): aggregate agent-eval results natively and import Gym's own#1065SandyChapman wants to merge 2 commits into
SandyChapman wants to merge 2 commits into
Conversation
SandyChapman
force-pushed
the
aalgo-434-gym-aggregations/schapman
branch
from
August 4, 2026 14:36
81903b3 to
7120a96
Compare
SandyChapman
force-pushed
the
aalgo-434-gym-aggregations/schapman
branch
from
August 4, 2026 16:00
7120a96 to
dc95306
Compare
SandyChapman
marked this pull request as ready for review
August 4, 2026 16:13
…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.
Provenance that omits a result-shaping setting is worse than none, because two
runs that behaved differently would record identical metadata. So each runner
surfaces its own knobs: gym its bind_resources_server/env_overrides/reward_key;
harbor its n_attempts and configured job location; docker_sandbox its effective
instructions; codex its codex_bin and prompt builder; fabric its adapter,
skills, effective model, and capture_trajectory. Fabric's model is resolved the
way _compose_config resolves it — an explicit argument wins, otherwise the
config's default — since a model supplied only through config is what actually
runs. capture_trajectory is recorded because with it off no relay/ATIF exporter
runs and the trial carries no trajectory evidence.
Two identity values were unstable across identical runs and are now derived
from declared names instead: FabricContainerRuntime recorded str(provider), a
memory-address repr, and callable identities used a bare __qualname__, which is
ambiguous across modules.
The evaluator plugin forwarded AgentEvalSpec.benchmark into a parameter that no
longer exists; the spec field is renamed 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.
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
August 4, 2026 16:28
3d4a2ec to
de3d021
Compare
…'s own Agent-eval reported a mean and little else, and a runner's own aggregations were discarded entirely — so a Gym run's headline numbers (pass@k, per-environment metrics) were visible in Gym's output files but not in the eval result. Native aggregation: - pass@k per task via the unbiased estimator (Chen et al. 2021), for every score-like metric output. A pass is full credit, deliberately not configurable. - Percentiles on every aggregate, reusing the deterministic-metric helper (now public as `compute_percentiles`), which also gives each metric a median. - Both standard-deviation conventions, named explicitly: `std_dev`/`variance` stay population (divide by n), and `sample_std_dev`/`sample_variance` are new. Gym computes its spread with pandas (ddof=1), so naming both lets the two sets of numbers coexist without either changing meaning. Runner aggregations: - `RunAggregationsProvider` lets a runner surface run-level numbers it computed itself. The opaque payload passes through onto `AgentEvalResult.runner_provided_aggregations` (and its own bundle file); the subset that maps onto typed aggregates is merged into `summary.scores`. - Gym's flattened `key_metrics` are imported under `runner.gym.<metric>`. A full mean/max/min/std family is re-assembled into one range score; anything else becomes an `AggregateScalarScore`. The full-family requirement matters because a resources-server may define a metric literally named `mean` — re-assembling on a partial match would rename a real metric into a statistic of a distribution that never existed. `reward` is skipped as redundant with the natively-computed `gym_reward.reward`; nothing else numeric is dropped, which a synthetic custom-environment payload pins as an invariant. `AggregateScalarScore` is a new variant for a single reported figure with no underlying sample, so a reader can tell "this is the whole story" from "this summarizes count samples" instead of seeing a range score with a count of 1. The dashboard and the new `inspect_results.py` example both render it by value. Signed-off-by: Sandy Chapman <schapman@nvidia.com>
SandyChapman
force-pushed
the
aalgo-434-gym-aggregations/schapman
branch
from
August 4, 2026 16:35
dc95306 to
03d1d26
Compare
SandyChapman
force-pushed
the
aalgo-451-run-metadata/schapman
branch
from
August 4, 2026 16:59
de3d021 to
ee5af9c
Compare
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
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-434.
Why
Agent-eval reported a mean and little else, and a runner's own aggregations were discarded entirely — so a Gym run's headline numbers (pass@k, per-environment metrics) were visible in Gym's output files but nowhere in the eval result.
Native aggregation
compute_percentiles) so both paths report distributions the same way.medianadded toAggregateScoreBase, populated natively fromp50, so the field means the same thing whether a score was computed here or imported from a backend that reports a median without a full distribution.std_dev/varianceremain population (÷n);sample_std_dev/sample_varianceare new (÷n−1,Nonewhen n<2). Gym computes its spread with pandas (ddof=1), so naming both lets the two sets of numbers coexist without either silently changing meaning. No existing value changes.Importing Gym's own numbers
RunAggregationsProvideris a single method —run_aggregate_scores()— returning typed aggregates that merge intosummary.scores. There is no parallel opaque passthrough: Gym already writes its rawrollouts_aggregate_metrics.jsoninside the run's work dir, so a second copy on the result would duplicate a file already in the bundle and re-introduce the untypeddict[str, Any]bag that #1013 removes.Names are
runner.gym.<metric>— namespaced by runner, not agent, since each run uses a single agent and what a reader needs to know is which backend produced the number. Falls back torunner.gym.<agent>.<metric>only if a run produced several. Gym'srewardis skipped: the SDK already scores it natively asgym_reward.rewardfrom the same rollouts.The part most worth reviewing
Imports read
agent_metrics, notkey_metrics. This matters:key_metricsis a subset ofagent_metricschosen by the resources-server, and Gym's defaultget_key_metricskeeps only themean/*entries — so max/min/median/std never appear there, and sourcing from it would degrade every distribution into a lonerunner.gym.mean/<name>scalar.test_imports_read_agent_metrics_not_the_key_metrics_subsetpins this.Gym flattens each distribution into
<stat>/<metric>keys. Itsdescribe_dataframeemitsmean/max/min/median/std/histogramtogether, andprepare_for_serializationstripshistogrambefore writing — so the family in the file is exactly those five, and re-assembly requires all five. That guard matters because 36 of Gym's ~97 resources-servers overridecompute_metrics, emitting keys in their own shapes (arena_elo/score,easy/pass@1/accuracy), and a server is free to define a metric literally namedmean. A partial match stays as standalone scalars rather than being renamed into a statistic of a distribution that never existed.test_nothing_numeric_is_dropped_from_a_custom_environment_payloadpins the invariant: importing is a renaming, never a filter. Onlyreward(redundant by construction) and non-numeric values may disappear.New
AggregateScalarScoreA single pre-computed value with no underlying sample. Distinct from
AggregateRangeScoreso a reader can tell "this is the whole story" from "this summarizescountsamples", instead of seeing a range score with a suspiciouscountof 1.countis nowint | None, and imported aggregates carryNone— Gym reports statistics without the n behind them, and0would assert that nothing was evaluated (besides being a division hazard). The dashboard and example renderNoneas an em dash while still showing a genuine0.Also
examples/gym/inspect_results.py— companion torun_gym_eval.py, showing how to reach each kind of result, with lift-and-paste accessors.Known scope boundary
Gym's
group_level_metrics(its per-task aggregation) is not imported.summary.scoresis run-level, so it has no home there yet — the SDK computes per-task groupings internally for pass@k but doesn't surface that granularity. Native per-task rollups plus importing Gym's into them is tracked separately; the raw data remains in Gym's own file inside the bundle.Verification
tyat the branch base's baseline (151);make vendorrun with no drift.