Skip to content

feat(benchmarks): remove the dataset-level agent pin - #3209

Open
adil-a wants to merge 1 commit into
mainfrom
data-prep/drop-agent-pin
Open

adil-a wants to merge 1 commit into
mainfrom
data-prep/drop-agent-pin

Conversation

@adil-a

@adil-a adil-a commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Problem

#2724 added an optional agent: key on dataset entries — a "pin" naming which agent runs a dataset when several agents reference the same resources server. Marta's review on #2724 (#2724 (comment)) argued the key should not exist: the only motivating case (bigcodebench) is a composition artifact, and an ambiguous config should be an error to fix, not something to paper over per dataset.

She was right, and the migration bore it out: after #2724 and #2732 there is not a single agent: pin in the repo, and the scaffold never emits one. The key is dead weight with a real cost — a second place, besides the config's own agent → resources-server wiring, where the harness can be chosen.

Change

The agent: key on dataset entries is removed. Routing has exactly two rules, both derived from the config's structure:

  1. A dataset declared on an agent block runs on that agent.
  2. A dataset declared on a resources server runs on the unique agent referencing it.

Zero or several referencing agents is a config error at load time. The error names the three supported ways to choose a harness, none of them per-dataset:

2 agents reference this resources server (['agent_a', 'agent_b']), so the rows have no single
harness. Pass +agent_map={my_rs: <agent>} to pick one at run time, add a `fan_out` entry to
run all of them, or declare the dataset on the agent.

A leftover agent: key in a dataset entry is rejected when the config loads (not silently ignored), with the same pointers:

dataset entries do not take an `agent:` key. Rows are dispatched to the agent that references the
declaring resources server; when several do, use `fan_out`, pass `+agent_map={<rs>: <agent>}`, or
declare the dataset on the agent.

What goes away:

  • BenchmarkDatasetConfig.agent and dataset_agent_pins(); resolve_dataset_agent() loses its pin parameter and the two pin-specific errors.
  • The pin plumbing in its four callers (benchmark discovery, gym eval prepare, rollout dispatch, environment validation), including validation's "conflicting pins on one instance" check.
  • The metrics-sidecar carve-out that excluded the pin from dataset dumps.

Behavior

Identical for every config on main and on #2732 — none uses the key. The only capability lost is a static, per-dataset default harness for a resources server shared by several agents; that case now fails fast and points at agent_map, fan_out, or agent-side declaration. genrm_compare, the one shared-RS config with RS-declared data, already uses fan_out.

Verification

  • Full core unit suite green locally.
  • Pin tests replaced: the discovery == collate == dispatch agreement test is kept without the pin; three new tests cover the rejection on DatasetConfig, BenchmarkDatasetConfig, and through real benchmark discovery.
  • grep for agent: in dataset entries across benchmarks/, resources_servers/, environments/, responses_api_agents/ on main and on chore(data): apply the dataset decoupling migration (agent_ref out of data files) #2732: zero hits.

Relationship to other PRs

Follows up on #2724 (merged). Independent of #2732 (configs and data only); whichever lands second rebases trivially. Closes the loop on Marta's #2724 review comment.

🤖 Generated with Claude Code

Datasets route to the agent bound to the declaring resources server (or
to the declaring agent). #2724 added an optional `agent:` key on dataset
entries as an override for a resources server shared by several agents.
Review on #2724 argued that such a config should be an error to fix, not
something to paper over per dataset — and after #2724/#2732 no config
uses the key.

- Drop `BenchmarkDatasetConfig.agent`, `dataset_agent_pins()`, and the
  `pin` parameter of `resolve_dataset_agent()` with its two pin errors.
- Callers (benchmark discovery, eval prepare, rollout dispatch,
  environment validation) lose the pin plumbing; the metrics sidecar no
  longer needs to exclude the key.
- A leftover `agent:` key fails config load with a pointer to the
  supported ways to choose a harness (agent_map, fan_out, agent-side
  declaration); the ambiguous-RS error names the same three.
- Tests: pin tests replaced by rejection tests on both dataset models
  and through real discovery; the discovery == collate == dispatch
  agreement test is kept without the pin.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Adil Asif <adasif@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Sep 10, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@adil-a

adil-a commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test b089c1f

@yaoyu-33 yaoyu-33 added area:env-infra Shared environment framework, lifecycle, registry, scaffolding, and validation feature New capabilities, enhancements, or enablement work needs-review PR is ready for code review and waiting on a reviewer labels Sep 10, 2026
@github-actions github-actions Bot added the sla:triage-overdue Review assignment is over the one-business-day SLA label Sep 11, 2026
@yaoyu-33 yaoyu-33 added the complexity:high Cross-component or shared-contract change with a large review surface label Sep 15, 2026

This branch was previously deployed

1 inactive deployment
public — b089c1f6 Deployed Sep 10, 2026 by copy-pr-bot[bot] via release / finalize / notify #2745
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:env-infra Shared environment framework, lifecycle, registry, scaffolding, and validation complexity:high Cross-component or shared-contract change with a large review surface feature New capabilities, enhancements, or enablement work needs-review PR is ready for code review and waiting on a reviewer sla:triage-overdue Review assignment is over the one-business-day SLA

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants