feat(evaluator)!: typed run metadata and a required runner identity contract - #1013
Open
SandyChapman wants to merge 1 commit into
Open
feat(evaluator)!: typed run metadata and a required runner identity contract#1013SandyChapman wants to merge 1 commit into
SandyChapman wants to merge 1 commit 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
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 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
|
SandyChapman
force-pushed
the
aalgo-451-run-metadata/schapman
branch
from
July 31, 2026 17:31
74a1ad5 to
23d4202
Compare
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
SandyChapman
force-pushed
the
aalgo-451-run-metadata/schapman
branch
from
August 4, 2026 11:28
5c409f4 to
3c4b692
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 184-196: Update the runner_info method to include
self._capture_trajectory in the RunnerInfo config so provenance distinguishes
both trajectory-capture modes. Extend test_run_metadata.py to verify runner
metadata records the flag as both true and false.
🪄 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: e24da39a-9959-4757-94a0-46ea4fb9f375
⛔ 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 (10)
- plugins/nemo-evaluator/src/nemo_evaluator/jobs/agent_spec.py
- plugins/nemo-evaluator/src/nemo_evaluator/jobs/agent_evaluate.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/trials.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/docker_sandbox.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/gym_runtime.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_run_metadata.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/callable_runtime.py
SandyChapman
force-pushed
the
aalgo-451-run-metadata/schapman
branch
2 times, most recently
from
August 4, 2026 12:27
28b937c to
3d4a2ec
Compare
This was referenced Aug 4, 2026
ngoncharenko
reviewed
Aug 4, 2026
| kind="runner", | ||
| config={ | ||
| "model": self._model, | ||
| "timeout_s": self._timeout_s, |
Contributor
There was a problem hiding this comment.
what specific timeout is it?
Contributor
Author
There was a problem hiding this comment.
Per-task wall clock for the Codex CLI subprocess — the asyncio.wait_for around a single process.communicate(), covering the agent's whole run on one task. Default 600s.
It was undocumented, which is a fair catch — I've added a docstring on DEFAULT_CODEX_TIMEOUT_S spelling this out.
SandyChapman
force-pushed
the
aalgo-451-run-metadata/schapman
branch
from
August 4, 2026 16:28
3d4a2ec to
de3d021
Compare
…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:59
de3d021 to
ee5af9c
Compare
ngoncharenko
reviewed
Aug 4, 2026
| "num_repeats": cfg.num_repeats, | ||
| "concurrency": cfg.concurrency, | ||
| "bind_resources_server": cfg.bind_resources_server, | ||
| "env_overrides": list(cfg.env_overrides), |
Contributor
There was a problem hiding this comment.
should we just in case guard or redact cfg.model.api_key kind of overrides here?
ngoncharenko
reviewed
Aug 4, 2026
| if target is None: | ||
| return RunnerInfo(name="imported", kind="imported") | ||
| if isinstance(target, Model): | ||
| return RunnerInfo(name=target.name, kind="model") |
Contributor
There was a problem hiding this comment.
same name models at different URLs would be indistinguishable. Should we provide additional metadata?
Contributor
There was a problem hiding this comment.
for models maybe we also want supply hyperparams. For agents: effort level, max_tokens
ngoncharenko
approved these changes
Aug 4, 2026
ngoncharenko
reviewed
Aug 4, 2026
| name="harbor", | ||
| kind="runner", | ||
| config={ | ||
| "agent_name": config.agent_name if config is not None else None, |
Contributor
There was a problem hiding this comment.
agent_import_path overrides agent_name, but this metadata still reports the default oracle and omits both the import path and agent_model_name. Runs using pkg_a:Agent / model-a and pkg_b:Agent / model-b therefore produce identical RunnerInfo. Please record the effective custom agent and model so these runs remain distinguishable.
Codex-generated review comment.
ngoncharenko
reviewed
Aug 4, 2026
| labels: dict[str, str] = Field( | ||
| default_factory=dict, | ||
| description="Benchmark metadata recorded alongside the run.", | ||
| description="Caller-supplied tags recorded on the run's metadata (e.g. mode, backend, scenario). " |
Contributor
There was a problem hiding this comment.
This description says the benchmark label is derived from task metadata, but that derivation was removed, so a task with metadata={"benchmark": "Example"} now produces empty run labels. The ProfBench README snippets at lines 137 and 263 also still pass the removed benchmark= argument and fail AgentEvalRunConfig validation. Please update the contract text and those snippets to use string-valued labels=.
Codex-generated review comment.
ngoncharenko
reviewed
Aug 4, 2026
| async def _agent_fn(task): # pragma: no cover - never called | ||
| return None | ||
|
|
||
| class _Provider: |
Contributor
There was a problem hiding this comment.
These _Provider test doubles define only name, so scoped ty check reports invalid-argument-type at both FabricContainerRuntime call sites: the constructor expects a SandboxProvider, but the stub does not implement that protocol. Please use a protocol-complete fake or explicitly cast this identity-only stub to SandboxProvider so the new test does not add type-check failures.
Codex-generated review comment.
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
2 participants
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.