feat(benchmark): pre-bake hermes agent into dataset images for closed-book runs - #350
Conversation
…-book runs Closed-book Harbor runs cannot curl-install agents at task time. Bake the NousResearch hermes-agent into the dataset image install layer (pinned via HERMES_VERSION in benchmark/agent-versions.env) and add an install-skip probe to the harbor hermes agent patch so the runtime install() short-circuits when hermes is already present. The agent patch also threads a custom OpenAI-compatible base_url (e.g. a Switchyard gateway) into hermes' config.yaml, which hermes requires for a custom chat-model endpoint (OPENAI_BASE_URL alone is ignored for the chat model). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Subhashree Radhakrishnan <subhashreer@nvidia.com>
Signed-off-by: Giedrius Burachas <gburachas@nvidia.com>
…r there Signed-off-by: Giedrius Burachas <gburachas@nvidia.com>
WalkthroughThe change pins Hermes to ChangesHermes dataset runtime
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
benchmark/prepare_harbor_dataset.py (1)
210-210: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWrap the changed Python source lines.
Lines 210 and 261 exceed 100 characters. Extract the prebaked-version value and split the installer command across adjacent string literals.
As per coding guidelines: "Keep lines within 100 characters."
Also applies to: 261-262
🤖 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 `@benchmark/prepare_harbor_dataset.py` at line 210, In the dataset preparation command construction, extract the prebaked agent version value into a local variable and reuse it in the environment assignment. Wrap the long Python source lines, including the installer command around the referenced command-building logic, by splitting adjacent string literals while preserving the generated command exactly.Source: Coding guidelines
🤖 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 `@benchmark/prepare_harbor_dataset.py`:
- Around line 195-206: Require HERMES_VERSION in the prepare_harbor_dataset
validation flow to be a full commit SHA, or resolve configured refs to one
before generating the Docker layer and manifest; reject tags and branch
references. In benchmark/prepare_harbor_dataset.py lines 195-206, update the
HERMES_VERSION validation accordingly; in benchmark/agent-versions.env lines
8-13, replace v2026.8.3 with the resolved full SHA; in
tests/test_prepare_harbor_dataset.py lines 331-346, use a full-SHA fixture and
assert that tag and branch references are rejected.
- Around line 258-262: Update the Alpine package installation branch in the
generated setup command to include the bash package alongside git, ripgrep, and
xz, ensuring the subsequent installer pipeline to bash succeeds on Alpine.
- Around line 201-202: Add "HERMES_VERSION" to the required pin names validated
by prepare_dataset before the pins["HERMES_VERSION"] access, preserving the
existing validation error behavior for missing pins.
---
Nitpick comments:
In `@benchmark/prepare_harbor_dataset.py`:
- Line 210: In the dataset preparation command construction, extract the
prebaked agent version value into a local variable and reuse it in the
environment assignment. Wrap the long Python source lines, including the
installer command around the referenced command-building logic, by splitting
adjacent string literals while preserving the generated command exactly.
🪄 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: af463a69-0464-4e0c-9a01-b8bff70b1b94
📒 Files selected for processing (4)
benchmark/agent-versions.envbenchmark/patches/harbor-agent-patches.diffbenchmark/prepare_harbor_dataset.pytests/test_prepare_harbor_dataset.py
|
@gburachas Can you please address coderabbit comments first. On high level looks good, please address the comments, I will approve |
…ng-pin gaps Signed-off-by: Giedrius Burachas <gburachas@nvidia.com>
|
@ayushag-nv all three CodeRabbit findings are addressed in f4052d9 — this is ready for your review.
One thing worth your attention rather than a rubber stamp. The SHA suggestion could not be applied as written: the value was also passed to the installer's
Validation
Scope is unchanged: |
Closes #259.
What
Bakes the Hermes agent into the dataset image at prepare time, pinned to a full commit SHA, so closed-book Terminal-Bench runs do not fetch it at task time.
benchmark/agent-versions.env— the pin, in one placebenchmark/prepare_harbor_dataset.py— install the agent during dataset prep and record the pin in the manifestbenchmark/patches/harbor-agent-patches.diff— skip the runtime install when the agent is already presenttests/test_prepare_harbor_dataset.py— assert the pin reaches the manifestWhy
Closed-book runs deny network egress to the task container. An agent installed at task time therefore fails, and the failure looks like a task failure rather than a harness problem — it shows up as a score, not an error. Pre-baking moves the fetch into dataset preparation, where the network is available and a failure is loud.
Pinning is the other half: an agent that silently changes between runs makes two runs incomparable while both appear valid.
The pin is a full commit SHA, not a tag. A tag reads as immutable but can be deleted or repointed, so two builds could record the same string in the manifest while installing different code — the manifest would then assert a reproducibility it does not have. Requiring one shape is complete by construction, where a deny-list of moving names (
main,master, ...) never can be, since any branch name passes it. The installer script is fetched from the same commit for the same reason.The pin is applied with the installer's
--commit, not--branch.--branchreachesgit clone --branch, which accepts only branch and tag names and rejects a SHA outright.--force-commitis required alongside it: without it the installer skips the pin whenever the commit is an ancestor of the freshly clonedHEAD, logs a warning, and leaves the image on the tip ofmain— the drift this pin exists to prevent, arriving as a warning rather than a build failure.test_the_hermes_pin_is_applied_by_commit_and_forcedpins that flag combination so it cannot be quietly dropped.Scope
Benchmark tooling only. No Rust, no library or server behaviour, no public API. Kept minimal per @ayushag-nv's note on #259.
How tested
uv run pytest tests/test_prepare_harbor_dataset.py -v— 17 passeduv run pytest tests/ -m "not integration"— 876 passed, 9 skippeduv run ruff check ..pyfilesRebased onto current
mainwith no conflicts.Note: the first commit is authored by @SubhashreeR, carrying their original sign-off.
Summary by CodeRabbit
New Features
Bug Fixes
Tests