feat(benchmark): pre-bake hermes agent into dataset images for closed-book runs - #350
Open
gburachas wants to merge 3 commits into
Open
feat(benchmark): pre-bake hermes agent into dataset images for closed-book runs#350gburachas wants to merge 3 commits into
gburachas wants to merge 3 commits into
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
Comment on lines
+195
to
+206
| # HERMES_VERSION must name an immutable ref. A moving ref would let two builds | ||
| # record the same version string while installing different code, which is worse | ||
| # than not recording it: the manifest would assert a reproducibility it does not | ||
| # have. The installer script is fetched from the same ref for the same reason — | ||
| # pinning the agent but running whatever installer main has today reintroduces | ||
| # exactly the drift the pin exists to prevent. | ||
| hermes_version = pins["HERMES_VERSION"] | ||
| if hermes_version in ("main", "master", "HEAD"): | ||
| raise SystemExit( | ||
| f"HERMES_VERSION={hermes_version!r} is a moving ref and cannot be recorded " | ||
| "as a reproducible pin; use a tag or commit SHA" | ||
| ) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Use a full commit SHA for the Hermes install source.
A Git tag can be deleted or repointed. The current validation also accepts other moving branch names. The same manifest value can therefore install different Hermes code.
benchmark/prepare_harbor_dataset.py#L195-L206: require a full commit SHA, or resolve the configured ref to a commit SHA before generating the Docker layer and manifest.benchmark/agent-versions.env#L8-L13: replacev2026.8.3with the resolved full commit SHA.tests/test_prepare_harbor_dataset.py#L331-L346: update the fixture to use a full SHA and reject tag and branch references.
📍 Affects 3 files
benchmark/prepare_harbor_dataset.py#L195-L206(this comment)benchmark/agent-versions.env#L8-L13tests/test_prepare_harbor_dataset.py#L331-L346
🤖 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` around lines 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.
Comment on lines
+201
to
+202
| hermes_version = pins["HERMES_VERSION"] | ||
| if hermes_version in ("main", "master", "HEAD"): |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Require HERMES_VERSION before indexing pins.
Line 201 raises KeyError when HERMES_VERSION is absent. prepare_dataset validates only the four older pins at Line 496. Add HERMES_VERSION to required so preparation fails with the existing clear validation error.
Proposed fix
- required = {"CLAUDE_CODE_VERSION", "CODEX_VERSION", "OPENCODE_VERSION", "NODE_VERSION"}
+ required = {
+ "CLAUDE_CODE_VERSION",
+ "CODEX_VERSION",
+ "HERMES_VERSION",
+ "NODE_VERSION",
+ "OPENCODE_VERSION",
+ }🤖 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` around lines 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.
Comment on lines
+258
to
+262
| elif command -v apk >/dev/null 2>&1; then \\ | ||
| apk add --no-cache git ripgrep xz; \\ | ||
| fi; \\ | ||
| curl -fsSL https://raw.githubusercontent.com/NousResearch/hermes-agent/{hermes_version}/scripts/install.sh \\ | ||
| | bash -s -- --skip-setup --branch {hermes_version}; \\ |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Install Bash in the Alpine branch.
The APK branch installs git, ripgrep, and xz, but Line 262 pipes the installer into bash. Alpine does not include Bash by default. Hermes image builds on Alpine fail before installation.
Proposed fix
- apk add --no-cache git ripgrep xz; \
+ apk add --no-cache bash git ripgrep xz; \📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| elif command -v apk >/dev/null 2>&1; then \\ | |
| apk add --no-cache git ripgrep xz; \\ | |
| fi; \\ | |
| curl -fsSL https://raw.githubusercontent.com/NousResearch/hermes-agent/{hermes_version}/scripts/install.sh \\ | |
| | bash -s -- --skip-setup --branch {hermes_version}; \\ | |
| elif command -v apk >/dev/null 2>&1; then \\ | |
| apk add --no-cache bash git ripgrep xz; \\ | |
| fi; \\ | |
| curl -fsSL https://raw.githubusercontent.com/NousResearch/hermes-agent/{hermes_version}/scripts/install.sh \\ | |
| | bash -s -- --skip-setup --branch {hermes_version}; \\ |
🤖 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` around lines 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.
Contributor
|
@gburachas Can you please address coderabbit comments first. On high level looks good, please address the comments, I will approve |
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
3 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 #259.
What
Bakes the Hermes agent into the dataset image at prepare time, pinned to an immutable ref, 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— fetch the installer from the pinned reftests/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 to an immutable ref rather than a branch is the other half: an agent that silently changes between runs makes two runs incomparable while both appear valid.
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— 14 passeduv run pytest tests/ -m "not integration"— 873 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