Skip to content

feat(experimentalist): wire up nemo-evaluator-sdk using HarborRunner - #955

Open
ngoncharenko wants to merge 11 commits into
mainfrom
ngoncharenko/aalgo-312-wire-eval-into-optimizer
Open

feat(experimentalist): wire up nemo-evaluator-sdk using HarborRunner#955
ngoncharenko wants to merge 11 commits into
mainfrom
ngoncharenko/aalgo-312-wire-eval-into-optimizer

Conversation

@ngoncharenko

@ngoncharenko ngoncharenko commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Main changes

  • Add the NeMo Evaluator SDK's HarborAgentTaskRunner as a second evaluator
  • Two evaluator types: EvaluatorType = harbor_native | harbor_evaluator. The default stays harbor_native until harbor_evaluator reaches full parity. harbor is still accepted as a deprecated alias for harbor_native and warns once.

Linear

Review pointers

  • Majority of the files changed are harbor test examples and e2e test yaml configs. They are created to test the experimentalist at different levels of complexity
  • evaluator/harbor_evaluator.py — the new evaluator
  • evaluator/harbor.pyharbor_native + trials_from_job_dir extracted for sharing; HarborEvaluator._run widened to match the base-class contract
  • docs/architecture.md §4 - the two namespaces, and how to add a third evaluator

What changes

The loop gains a second evaluator_type. Both run Harbor underneath — what moves is who owns the orchestration:

evaluator_type Owns the run
harbor_evaluator The SDK's HarborAgentTaskRunner builds the JobConfig, owns the success-aware job-dir cache, and scopes the agent import.
harbor_native (default) The plugin builds Harbor's JobConfig and drives Job itself — the original path, kept as the A/B baseline. It stays the default until harbor_evaluator reaches full parity.

Both read results back through the same on-disk adapter, so the trials the loop sees are equivalent. The SDK's own trial model is deliberately not the contract, but not because it is lossy — metadata["reward_details"] does carry every verifier reward. The job directory is the shared source of truth for both evaluator types, and that sharing is what keeps them equivalent.

Design notes worth reviewing

Entry and exit are both shared.

Two evaluators run Harbor differently, but must mean the same thing by “evaluate this candidate”:

                    resolve_harbor_run_inputs()     ← ENTRY (shared)
                              │
              ┌───────────────┴───────────────┐
              ▼                               ▼
     `harbor_native`                  `harbor_evaluator`
     (plugin builds Job)              (SDK builds Job)
              │                               │
              └───────────────┬───────────────┘
                              ▼
                    trials_from_job_dir()           ← EXIT (shared)
                              │
                              ▼
                    TrialResult[] for the loop

Sharing resolve_harbor_run_inputs forces both to agree on agent path, dataset path, job dir, and preflight before either starts Harbor.
Sharing only the results side would let the two drift on what "the same run" means, and the A/B tests would not catch it — they compare results, not inputs.

Cache staleness is the SDK's job now.

Old (broken) cache key:

"reuse job dir if job_name matches AND enough successful trials exist"

Example failure:

Run 1: eval candidate-v1 → jobs/agent-0-train/ has 2 green trials → cached
Run 2: edit agent.py (candidate-v2), same job_name
       Old logic: "2 successes already → skip Harbor, re-adapt Day-1 results"
       → you score v1 code while thinking you scored v2

Same trap if you change a task’s test.sh, or a result-affecting option (timeouts, etc.).

New: SDK writes a stamp file in the job dir:

jobs/agent-0-train/
  .nemo-eval-harbor-cache.json    ← digests of agent + tasks + options
  <trial>/result.json

Rerun only reuses results if the stamp still matches. Plugin does not invent that logic — it only passes force_rerun=True when the user wants a wipe.

Experimentalist plugin decides when to force a wipe; SDK decides whether previous results are still valid for today’s inputs. Per-task resume (only re-run missing trials) is also SDK-owned (AALGO-427), not the plugin.

Two task-name namespaces

Harbor's local-dataset task_names filter matches the task directory name (sum-two); result.json records [task].name from task.toml (hello/sum-two), which is what the SDK's tasks and cache are keyed by. Mapping is by resolved directory, never name similarity. Passing full names to the Harbor filter — the naive reading of the SDK — makes Harbor match nothing and raise.

Same task has two different strings, used in different places:

dataset/train/
  sum-two/                 ← folder name = short id
    task.toml              ← [task] name = "hello/sum-two"  (full id)
Who Uses Example
Harbor’s task_names filter folder name "sum-two"
result.json / SDK tasks / SDK cache [task].name from toml "hello/sum-two"

Config is extra="forbid".

Every HarborRunnerConfig field maps 1:1 onto HarborRuntimeConfig. Options with no unambiguous SDK equivalent (plain Harbor's retry model) are rejected, not ignored. agent_dir is absent by design — always derived from the candidate, so a config cannot aim a run at different code.

aggregate_results filters

aggregate_results filters on status == "completed".** Equivalent to the old != "failed" while TrialStatus has two values, but it opts statuses in rather than opting "failed" out, so a third status is not silently averaged.

Testing

  • Unit — factory/config, the short↔full name mapping (subsets, duplicates, namespaced names), execution (job config, cache hit/miss, force_rerun, concurrent candidates), and result parity between the two evaluators.
  • Live, against real Docker (test_evaluator_harbor_ab_e2e.py, 4 tests) — runs the bundled example through both evaluators and asserts they produce equivalent trials, plus cache reuse and force_rerun behaviour. Both report {"reward": 0.5, "format_ok": 1.0}.
  • The SDK's own bundled oracle Harbor E2E passes.
  • A full nemo experimentalist run through the CLI completes on harbor_evaluator: winner=agent-0, validation_reward={'reward': 0.5, 'format_ok': 1.0}.

611 passed, 2 skipped (both skips are pre-existing Eval Author canaries that need RUN_EVAL_AUTHOR_* env vars). Ruff clean; ty diagnostics one below the pre-change baseline.

Reproducing the A/B

Needs Docker; no model API key — the evaluator seam makes no LLM calls.

uv run plugins/nemo-experimentalist/docs/e2e/run-eval-only.py --evaluator-type harbor
uv run plugins/nemo-experimentalist/docs/e2e/run-eval-only.py --evaluator-type harbor_evaluator

Both print the same aggregate. Full walkthrough — prerequisites, artifact layout, cache behaviour, config reference, and the CLI configs — is in examples/hello-harbor-agent/README.md.

Note docs/e2e/experiment-eval-only.yaml pins evaluator_type: harbor_native explicitly. It is the plain-Harbor arm of the A/B; pinning both arms keeps the comparison readable and independent of whatever the default happens to be.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added SDK-backed Harbor evaluation alongside native evaluation.
    • Added caching, partial-run resumption, and faster reruns.
    • Added runnable greeting, arithmetic, and airline-agent examples with local workflows.
    • Added debugging, evaluation-only, and fast integration configurations.
  • Documentation

    • Added architecture, onboarding, setup, troubleshooting, and end-to-end guides.
    • Clarified Python, Docker, credentials, installation, and local execution requirements.
  • Bug Fixes

    • Improved trial aggregation, validation, task mapping, and evaluator error handling.
    • Preserved compatibility with legacy evaluator configuration values.

@ngoncharenko
ngoncharenko requested review from a team as code owners July 28, 2026 20:21
@github-actions github-actions Bot added the feat label Jul 28, 2026
@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 29654/37644 78.8% 63.5%
Integration Tests 17481/36362 48.1% 20.6%

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR adds SDK-backed Harbor evaluation, content-addressed native Harbor caching, runnable Hello and Tau2 examples, experiment configurations, architecture and E2E documentation, and regression tests.

Changes

Experimentalist evaluator and examples

Layer / File(s) Summary
Evaluator contracts and Harbor implementations
plugins/nemo-experimentalist/src/.../evaluator/*, plugins/nemo-eval-author/...
Adds harbor_native and harbor_evaluator, deprecated alias normalization, shared Harbor input and result handling, SDK task mapping, configuration wiring, and evaluator parity coverage.
Native Harbor cache and resume flow
packages/nemo_evaluator_sdk/.../harbor_runtime.py, packages/nemo_evaluator_sdk/tests/agent_eval/*
Adds content-addressed cache stamps, deterministic scoped imports, source-change safeguards, partial-result reuse, selective resume retries, and end-to-end resume coverage.
Hello Harbor example
plugins/nemo-experimentalist/examples/hello-harbor-agent/*, plugins/nemo-experimentalist/tests/experimentalist/test_hello_example_baseline.py
Adds a deterministic greeting agent, Harbor wrapper, OTLP tracing, optimizer profile, task fixtures, exact-output verifiers, and baseline validation.
Tau2 airline example
plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/*
Adds the CodeAct airline agent, pinned Tau2 runtime containers, MCP orchestration, serialized runtime state, Harbor execution, trace collection, and task policies.
Documentation and experiment entry points
plugins/nemo-experimentalist/README.md, plugins/nemo-experimentalist/docs/*, plugins/nemo-experimentalist/examples/README.md, plugins/nemo-experimentalist/pyproject.toml
Adds architecture and E2E guidance, minimal experiment configurations, an evaluator-only CLI, example documentation, dependency wiring, and pytest markers.

Sequence Diagram(s)

sequenceDiagram
    participant CLI as run-eval-only.py
    participant Factory as evaluator/factory.py
    participant Evaluator as HarborRunnerEvaluator
    participant SDK as HarborAgentTaskRunner
    participant Runtime as harbor_runtime.py

    CLI->>Factory: build evaluator
    Factory->>Evaluator: create HarborRunnerConfig
    CLI->>Evaluator: run dataset and agent
    Evaluator->>Evaluator: map dataset task names
    Evaluator->>SDK: execute Harbor tasks
    SDK->>Runtime: resolve cache and scoped import
    Runtime-->>SDK: reuse or rerun job
    SDK-->>Evaluator: return trial results
    Evaluator-->>CLI: return aggregate metrics
Loading

Possibly related PRs

Suggested labels: docs

Suggested reviewers: arpitsardhana, aleckhoury

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.82% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the primary change: integrating the NeMo Evaluator SDK through HarborRunner for Experimentalist.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ngoncharenko/aalgo-312-wire-eval-into-optimizer

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 16

🧹 Nitpick comments (3)
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor_agent_task_runner.py (1)

51-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

TYPE_CHECKING-only import violates path guideline.

HarborAgentTaskRunner, HarborRuntimeConfig, AgentEvalTask are imported only under TYPE_CHECKING for annotation purposes. The lazy-import rationale (documented in the module docstring) is sound, but this specific pattern is exactly what the path guideline prohibits. Consider a structural Protocol for the SDK surface actually used (mirroring HarborJobOptions in harbor.py), which preserves the lazy-import contract without a TYPE_CHECKING-only import.

As per path instructions, plugins/nemo-experimentalist/**/*.py: "Use concrete type hints rather than string-based type hints, and do not hide imports under TYPE_CHECKING."

Also applies to: 195-217

🤖 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
`@plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor_agent_task_runner.py`
around lines 51 - 59, Replace the TYPE_CHECKING-only SDK imports in the Harbor
task runner with local structural Protocols describing the
HarborAgentTaskRunner, HarborRuntimeConfig, and AgentEvalTask members actually
used. Update the annotations and related code in the task-runner flow to use
these Protocols while preserving lazy SDK loading and concrete type hints
without hidden imports.

Source: Path instructions

plugins/nemo-experimentalist/README.md (1)

127-129: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a final Next Steps section to each documentation page.

Each page provides useful links or verification commands but ends without the required cross-linked Next Steps section.

  • plugins/nemo-experimentalist/README.md#L127-L129: turn the example and architecture links into the final Next Steps section.
  • plugins/nemo-experimentalist/docs/architecture.md#L703-L707: add links to the runnable examples and evaluator documentation after the verification example.
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md#L212-L218: add links to the architecture guide and evaluator tests after the Docker verification instructions.
🤖 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 `@plugins/nemo-experimentalist/README.md` around lines 127 - 129, Append a
final “Next Steps” section to all three documentation sites: in
plugins/nemo-experimentalist/README.md lines 127-129, convert the existing
example and architecture links into that section; in
plugins/nemo-experimentalist/docs/architecture.md lines 703-707, add links to
the runnable examples and evaluator documentation after the verification
example; and in
plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md lines
212-218, add links to the architecture guide and evaluator tests after the
Docker verification instructions.

Source: Coding guidelines

plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/Dockerfile (1)

10-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Git not purged after clone, unlike runtime-server/Dockerfile.

runtime-server/Dockerfile purges git post-clone; this Dockerfile doesn't, leaving unnecessary packages in the final image.

♻️ Align with runtime-server/Dockerfile
 RUN apt-get update && apt-get install -y --no-install-recommends \
     git \
     && git clone --depth=1 "${TAU2_BENCH_REPO}" "${TAU2_BENCH_ROOT}" \
     && pip install --no-cache-dir uv "${TAU2_BENCH_ROOT}[knowledge]" \
+    && apt-get purge -y --auto-remove git \
     && rm -rf /var/lib/apt/lists/*
🤖 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
`@plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/Dockerfile`
around lines 10 - 14, Update the package-install chain in the Dockerfile to
remove git immediately after cloning and installing the benchmark dependencies,
matching the cleanup behavior in runtime-server/Dockerfile while retaining the
existing apt lists cleanup.
🤖 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 `@plugins/nemo-experimentalist/docs/architecture.md`:
- Around line 308-309: Update the failed-trial aggregation statement in the
architecture documentation to accurately describe the evaluator contract: failed
trials are excluded from the mean, so they do not directly lower it but reduce
the number of contributing trials. Remove the contradictory claim that a crash
drags the aggregate down.
- Around line 690-700: Make the evaluator example runnable by importing Path,
defining an async main function, and invoking it through asyncio.run(main())
instead of awaiting at module scope. Before evaluator.run, materialize the
baseline agent using the same approach as docs/e2e/run-eval-only.py so the
referenced agent-0 path exists.

In `@plugins/nemo-experimentalist/docs/e2e/experiment-debug-round.yaml`:
- Line 5: Update the runnable config comments to use platform-root-relative
paths: in plugins/nemo-experimentalist/docs/e2e/experiment-debug-round.yaml
lines 5-5, plugins/nemo-experimentalist/docs/e2e/experiment-eval-only.yaml lines
5-5, and plugins/nemo-experimentalist/docs/e2e/experiment-fast.yaml lines 2-2,
replace the docs/e2e path with the full plugins/nemo-experimentalist/docs/e2e
path.

In `@plugins/nemo-experimentalist/docs/e2e/experiment-fast.yaml`:
- Around line 1-3: Add the standard NVIDIA copyright and Apache-2.0 SPDX header
to plugins/nemo-experimentalist/docs/e2e/experiment-fast.yaml lines 1-3 using
YAML comments, and add the corresponding Markdown-comment header before the task
text in
plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/instruction.md
line 1.

In
`@plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/environment/Dockerfile`:
- Around line 7-11: Update both Dockerfiles at
plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/environment/Dockerfile
lines 7-11 and
plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/environment/Dockerfile
lines 7-11: create a dedicated non-root user, assign that user ownership of
/app/artifacts and /app/traces, and set USER to run each image as that user.

In
`@plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/instruction.md`:
- Around line 1-9: Add SPDX headers to the Markdown fixtures in
plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/instruction.md
and
plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/instruction.md.
Keep the exact-output fixture
plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/tests/expected.txt
unchanged, or strip its header in test.sh before comparison.

In
`@plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/README.md`:
- Around line 4-16: Update the task-template README to add a prerequisites
section before the existing explanation, then append a “Next Steps” section
containing links to the relevant follow-up documentation. Ensure the page ends
with navigation and preserve the existing task-template details.

In
`@plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/test.sh`:
- Around line 21-32: Update both verifier scripts at
plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/test.sh
lines 21-32 and
plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/tests/test.sh
lines 21-32: remove first-line-only extraction, compare the full CR-normalized
output against the full expected content, and reject outputs containing any
additional lines or content.

In
`@plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/environment/Dockerfile`:
- Around line 7-11: Create a dedicated non-root user in the Dockerfile, grant
that user ownership of /app and its artifacts and traces directories, then set
the Docker USER directive before execution so the agent-controlled task does not
run as root.

In
`@plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/expected.txt`:
- Line 1: Resolve SPDX compliance for all three fixtures using one
repository-approved metadata or exception mechanism: in
plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/expected.txt
lines 1-1 and
plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/tests/expected.txt
lines 1-1, preserve the exact compared payloads while satisfying or explicitly
exempting the SPDX rule; in
plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/instruction.md
lines 1-1, add compliant NVIDIA copyright and Apache-2.0 metadata without
changing prompt semantics.

In
`@plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/test.sh`:
- Around line 21-38: Update the test script’s output validation to fail closed:
explicitly verify that reading both `/tests/expected.txt` and `$OUTPUT`
succeeds, normalize complete file contents without `head -n 1`, and compare the
complete normalized values. Preserve zero reward and unsuccessful formatting
when either read fails or the contents differ.

In
`@plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/environment/Dockerfile`:
- Around line 7-11: Update the Dockerfile after creating /app/artifacts and
/app/traces to add a dedicated unprivileged user, grant that user ownership of
both directories, and set the USER directive before the container executes the
agent. Preserve the existing /app working directory and artifact/trace paths.

In
`@plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/instruction.md`:
- Around line 1-7: Update the instruction document by adding a top-level
Markdown heading and changing the fenced code block to declare the text
language, while preserving the existing output path and exact required line.

In `@plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md`:
- Around line 119-130: Update the A/B evaluator commands in the README to pass
distinct --experiment-dir values to run-eval-only.py, ensuring the Harbor and
harbor_agent_task_runner runs use separate experiment directories and cannot
reuse each other’s cached job results.

In `@plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/AGENT-SPEC.md`:
- Line 1: Prepend the required NVIDIA copyright SPDX header and Apache-2.0
license identifier to all six affected files:
plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/AGENT-SPEC.md (1-1),
dataset/template/task_template/environment/docker-compose.yaml (1-1),
environment/Dockerfile (1-1), environment/runtime-server/Dockerfile (1-1),
dataset/template/task_template/instruction.md (1-1), and
dataset/template/task_template/task.toml (1-1). Use the repository’s format for
each file type, placing comments before existing content and before FROM or
<instructions> where applicable.

In `@plugins/nemo-experimentalist/pyproject.toml`:
- Around line 14-16: Update the nemo-evaluator-sdk dependency declaration in
pyproject.toml to install only for Python 3.12 and newer, while preserving the
existing ungated harbor dependency and the evaluator_type behavior. Use the
project’s dependency marker syntax to exclude nemo-evaluator-sdk from Python
3.11 environments.

---

Nitpick comments:
In
`@plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/Dockerfile`:
- Around line 10-14: Update the package-install chain in the Dockerfile to
remove git immediately after cloning and installing the benchmark dependencies,
matching the cleanup behavior in runtime-server/Dockerfile while retaining the
existing apt lists cleanup.

In `@plugins/nemo-experimentalist/README.md`:
- Around line 127-129: Append a final “Next Steps” section to all three
documentation sites: in plugins/nemo-experimentalist/README.md lines 127-129,
convert the existing example and architecture links into that section; in
plugins/nemo-experimentalist/docs/architecture.md lines 703-707, add links to
the runnable examples and evaluator documentation after the verification
example; and in
plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md lines
212-218, add links to the architecture guide and evaluator tests after the
Docker verification instructions.

In
`@plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor_agent_task_runner.py`:
- Around line 51-59: Replace the TYPE_CHECKING-only SDK imports in the Harbor
task runner with local structural Protocols describing the
HarborAgentTaskRunner, HarborRuntimeConfig, and AgentEvalTask members actually
used. Update the annotations and related code in the task-runner flow to use
these Protocols while preserving lazy SDK loading and concrete type hints
without hidden imports.
🪄 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: 03aa3d96-1fdb-48f5-aab9-0ba78ef0204c

📥 Commits

Reviewing files that changed from the base of the PR and between 1739e3a and 2e591d6.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (69)
  • plugins/nemo-experimentalist/README.md
  • plugins/nemo-experimentalist/docs/architecture.md
  • plugins/nemo-experimentalist/docs/e2e/experiment-debug-round.yaml
  • plugins/nemo-experimentalist/docs/e2e/experiment-eval-only-sdk.yaml
  • plugins/nemo-experimentalist/docs/e2e/experiment-eval-only.yaml
  • plugins/nemo-experimentalist/docs/e2e/experiment-fast.yaml
  • plugins/nemo-experimentalist/docs/e2e/run-eval-only.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/AGENT-SPEC.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/agent.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/harbor_wrapper.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/main.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/optimizer.yaml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/tracing.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/AGENT-SPEC.md
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/agent.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/docker-compose.yaml
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/Dockerfile
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/server.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/task_config.json
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/instruction.md
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/task.toml
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/tests/test.sh
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/harbor_wrapper.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/main.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/optimizer.yaml
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/pyproject.toml
  • plugins/nemo-experimentalist/pyproject.toml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/config.yaml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/run.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/base.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/factory.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor_agent_task_runner.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/deps.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/run.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/resolve.py
  • plugins/nemo-experimentalist/tests/experimentalist/conftest.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor_ab_e2e.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor_agent_task_runner.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_hello_example_baseline.py
  • plugins/nemo-experimentalist/tests/test_eval_author_run.py

Comment thread plugins/nemo-experimentalist/docs/architecture.md Outdated
Comment thread plugins/nemo-experimentalist/docs/architecture.md Outdated
Comment thread plugins/nemo-experimentalist/docs/e2e/experiment-debug-round.yaml Outdated
Comment thread plugins/nemo-experimentalist/docs/e2e/experiment-fast.yaml Outdated
Comment thread plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md
Comment thread plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/AGENT-SPEC.md Outdated
Comment thread plugins/nemo-experimentalist/pyproject.toml Outdated
@ngoncharenko
ngoncharenko force-pushed the ngoncharenko/aalgo-312-wire-eval-into-optimizer branch from ba6355e to 1f903e8 Compare July 28, 2026 22:08

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

♻️ Duplicate comments (2)
plugins/nemo-experimentalist/pyproject.toml (1)

16-16: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Gate nemo-evaluator-sdk for Python 3.12+.

This ungated dependency breaks supported Python 3.11 installs before users can select the plain Harbor fallback. Mirror the Harbor marker.

-    "nemo-evaluator-sdk",
+    "nemo-evaluator-sdk; python_full_version >= '3.12'",
🤖 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 `@plugins/nemo-experimentalist/pyproject.toml` at line 16, Update the
nemo-evaluator-sdk dependency entry in pyproject.toml to include the same
Python-version environment marker used by the Harbor dependency, restricting it
to Python 3.12 and newer while preserving the existing dependency declaration.

Source: Learnings

plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/environment/Dockerfile (1)

7-11: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Run all Harbor task images as an unprivileged user.

  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/environment/Dockerfile#L7-L11: create a user, chown /app and its writable directories, and set USER.
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/environment/Dockerfile#L7-L11: apply the same non-root setup.
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/environment/Dockerfile#L7-L11: apply the same non-root setup.
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/environment/Dockerfile#L7-L11: apply the same non-root setup.
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/environment/Dockerfile#L7-L11: apply the same non-root setup.
    Based on learnings, Harbor uploads the agent into /app and currently executes it as root; validate upload, permissions, startup, and execution end to end for every image.
🤖 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
`@plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/environment/Dockerfile`
around lines 7 - 11, The five listed Dockerfiles must run Harbor tasks as an
unprivileged user: create a dedicated user, chown /app and its writable
artifacts and traces directories, then set USER to that account. Apply the same
setup in the task-template, train/greet-world, train/sum-two,
validation/greet-universe, and validation/sum-three Dockerfiles, and verify
upload, permissions, startup, and execution end to end for every image.

Sources: Learnings, Linters/SAST tools

🧹 Nitpick comments (3)
plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/README.md (1)

11-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Make this a focused how-to and move prerequisites first.

Move the component table to a linked reference page, then place Docker, registry access, network access, and INFERENCE_API_KEY under Prerequisites before the commands. As per coding guidelines, each page must fit one Diataxis quadrant and list prerequisites at the top.

🤖 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 `@plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/README.md` around
lines 11 - 54, Restructure the README into a focused how-to: add a Prerequisites
section near the top listing Docker, dataset-registry access, task-container
network access, and a valid INFERENCE_API_KEY before the setup and run commands.
Remove the inline “What it contains” component table and replace it with a link
to the appropriate reference page, keeping the remaining execution instructions
focused on running the example.

Source: Coding guidelines

plugins/nemo-experimentalist/docs/architecture.md (1)

4-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Split the mixed documentation pages by Diataxis type.

  • plugins/nemo-experimentalist/docs/architecture.md#L4-L19: retain architecture explanation here; move run/debug procedures and reference material to linked pages.
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md#L4-L16: retain the example how-to here; move evaluator, cache, and configuration reference material to linked pages.
    As per coding guidelines, each documentation page should fit one Diataxis quadrant.
🤖 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 `@plugins/nemo-experimentalist/docs/architecture.md` around lines 4 - 19, Split
the mixed documentation by Diataxis type: in
plugins/nemo-experimentalist/docs/architecture.md lines 4-19, retain only the
architecture explanation and link run/debug procedures and reference material to
dedicated pages; in
plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md lines 4-16,
retain the example how-to and move evaluator, cache, and configuration reference
content to linked pages.

Source: Coding guidelines

plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/AGENT-SPEC.md (1)

1-167: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Policy text duplicated verbatim in instruction.md.

The full airline policy here is copy-pasted into dataset/template/task_template/instruction.md. Two independent copies of a ~180-line policy will drift; consider generating the <policy> block in instruction.md from this file (or vice versa) at dataset-prep time.

🤖 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 `@plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/AGENT-SPEC.md`
around lines 1 - 167, Remove the duplicated airline policy from
dataset/template/task_template/instruction.md and establish a single source of
truth using the policy in AGENT-SPEC.md, generating or synchronizing the
instruction.md <policy> block during dataset preparation. Ensure the generated
block remains verbatim and update the preparation flow rather than maintaining
two independently edited copies.
🤖 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 `@plugins/nemo-experimentalist/docs/architecture.md`:
- Around line 584-593: Move the Prerequisites block in
plugins/nemo-experimentalist/docs/architecture.md (lines 584-593) before the
architecture overview. Also move the corresponding prerequisites block in
plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md (lines
111-117) before the example layout and behavior sections, keeping each block’s
existing content unchanged.

In
`@plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/test.sh`:
- Around line 37-39: Replace the tr -d '\r' normalization in the EXPECTED and
ACTUAL assignments of all three test.sh
files—plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/test.sh
lines 37-39,
plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/test.sh
lines 37-39, and
plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/tests/test.sh
lines 37-39—with CRLF-only normalization that removes carriage returns only when
paired with line feeds, preserving standalone carriage returns.

In `@plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md`:
- Around line 172-174: Update the force-rerun command in the README to include
the explicit --experiment-dir tmp/eval-only-sdk argument, ensuring it refreshes
the SDK evaluator arm and remains distinct from the other evaluator experiment
directory.

In
`@plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/docker-compose.yaml`:
- Line 1: Add the required NVIDIA copyright and Apache-2.0 SPDX license
identifiers at the top of the docker-compose.yaml file, before the existing
services declaration, following the repository’s standard header format.

In `@plugins/nemo-experimentalist/README.md`:
- Around line 115-125: Use distinct experiment directories in both onboarding
examples: update the plain-Harbor command in
plugins/nemo-experimentalist/README.md lines 115-125 to use
tmp/exp-hello-eval-plain, and update the SDK runner command in
plugins/nemo-experimentalist/docs/e2e/README.md lines 58-64 to use
tmp/exp-hello-eval-sdk.

In
`@plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor_agent_task_runner.py`:
- Around line 175-176: Update the task execution flow around _cache_fingerprint
and _cache_is_stale to acquire a per-job-directory lock before rechecking cache
state. Keep the lock held through the cache recheck, run_tasks invocation, and
fingerprint write, so concurrent processes cannot run or write the same trial
directory; release it afterward.

---

Duplicate comments:
In
`@plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/environment/Dockerfile`:
- Around line 7-11: The five listed Dockerfiles must run Harbor tasks as an
unprivileged user: create a dedicated user, chown /app and its writable
artifacts and traces directories, then set USER to that account. Apply the same
setup in the task-template, train/greet-world, train/sum-two,
validation/greet-universe, and validation/sum-three Dockerfiles, and verify
upload, permissions, startup, and execution end to end for every image.

In `@plugins/nemo-experimentalist/pyproject.toml`:
- Line 16: Update the nemo-evaluator-sdk dependency entry in pyproject.toml to
include the same Python-version environment marker used by the Harbor
dependency, restricting it to Python 3.12 and newer while preserving the
existing dependency declaration.

---

Nitpick comments:
In `@plugins/nemo-experimentalist/docs/architecture.md`:
- Around line 4-19: Split the mixed documentation by Diataxis type: in
plugins/nemo-experimentalist/docs/architecture.md lines 4-19, retain only the
architecture explanation and link run/debug procedures and reference material to
dedicated pages; in
plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md lines 4-16,
retain the example how-to and move evaluator, cache, and configuration reference
content to linked pages.

In `@plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/AGENT-SPEC.md`:
- Around line 1-167: Remove the duplicated airline policy from
dataset/template/task_template/instruction.md and establish a single source of
truth using the policy in AGENT-SPEC.md, generating or synchronizing the
instruction.md <policy> block during dataset preparation. Ensure the generated
block remains verbatim and update the preparation flow rather than maintaining
two independently edited copies.

In `@plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/README.md`:
- Around line 11-54: Restructure the README into a focused how-to: add a
Prerequisites section near the top listing Docker, dataset-registry access,
task-container network access, and a valid INFERENCE_API_KEY before the setup
and run commands. Remove the inline “What it contains” component table and
replace it with a link to the appropriate reference page, keeping the remaining
execution instructions focused on running the example.
🪄 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: f53b7e03-88f4-40e6-883c-c36ee933b2f5

📥 Commits

Reviewing files that changed from the base of the PR and between 2e591d6 and 1f903e8.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (72)
  • plugins/nemo-experimentalist/README.md
  • plugins/nemo-experimentalist/docs/architecture.md
  • plugins/nemo-experimentalist/docs/e2e/README.md
  • plugins/nemo-experimentalist/docs/e2e/experiment-debug-round.yaml
  • plugins/nemo-experimentalist/docs/e2e/experiment-eval-only-sdk.yaml
  • plugins/nemo-experimentalist/docs/e2e/experiment-eval-only.yaml
  • plugins/nemo-experimentalist/docs/e2e/experiment-fast.yaml
  • plugins/nemo-experimentalist/docs/e2e/run-eval-only.py
  • plugins/nemo-experimentalist/examples/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/AGENT-SPEC.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/agent.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/harbor_wrapper.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/main.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/optimizer.yaml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/tracing.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/AGENT-SPEC.md
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/README.md
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/agent.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/docker-compose.yaml
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/Dockerfile
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/server.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/task_config.json
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/instruction.md
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/task.toml
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/tests/test.sh
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/harbor_wrapper.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/main.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/optimizer.yaml
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/pyproject.toml
  • plugins/nemo-experimentalist/pyproject.toml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/config.yaml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/run.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/base.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/factory.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor_agent_task_runner.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/deps.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/run.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/resolve.py
  • plugins/nemo-experimentalist/tests/experimentalist/conftest.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor_ab_e2e.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor_agent_task_runner.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_hello_example_baseline.py
  • plugins/nemo-experimentalist/tests/test_eval_author_run.py
🚧 Files skipped from review as they are similar to previous changes (37)
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/task.toml
  • plugins/nemo-experimentalist/docs/e2e/experiment-eval-only.yaml
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/main.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/expected.txt
  • plugins/nemo-experimentalist/docs/e2e/experiment-eval-only-sdk.yaml
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/optimizer.yaml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/tests/test.sh
  • plugins/nemo-experimentalist/docs/e2e/experiment-fast.yaml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/AGENT-SPEC.md
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/resolve.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/task.toml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/run.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/config.yaml
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/pyproject.toml
  • plugins/nemo-experimentalist/tests/experimentalist/conftest.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/run.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/deps.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/main.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/optimizer.yaml
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/task_config.json
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/task.toml
  • plugins/nemo-experimentalist/tests/test_eval_author_run.py
  • plugins/nemo-experimentalist/docs/e2e/experiment-debug-round.yaml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/agent.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/base.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_hello_example_baseline.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/harbor_wrapper.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/task.toml

Comment thread plugins/nemo-experimentalist/docs/architecture.md Outdated
Comment thread plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md Outdated
Comment thread plugins/nemo-experimentalist/README.md
ngoncharenko added a commit that referenced this pull request Jul 28, 2026
…ions

Round-2 review fixes on #955.

Verifier (all 5 copies of tests/test.sh):
- Normalize CRLF at end-of-line only. `tr -d '\r'` deleted every carriage
  return, so `sum=4<CR>2` collapsed into a passing `sum=42` — the same
  reward-hacking class as the trailing-output hole fixed earlier. The agent
  under test is code the optimizer actively reward-maximizes, so a lax
  verifier is a live surface, not a hypothetical one.
- Mirror the rule in test_hello_example_baseline.py, which had the identical
  hole via `.replace("\r", "")`, and assert the shell and Python rules stay in
  lockstep; that pairing has now drifted twice.

Docs:
- Give every evaluator arm its own --experiment-dir. README.md (plain Harbor)
  and docs/e2e/README.md (SDK runner) both pointed at tmp/exp-hello-eval; the
  two evaluators disagree about an existing job dir, so sharing one makes the
  result order-dependent.
- Point the --force-rerun example at tmp/eval-only-sdk rather than the default.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
ngoncharenko added a commit that referenced this pull request Jul 28, 2026
…ions

Round-2 review fixes on #955.

Verifier (all 5 copies of tests/test.sh):
- Normalize CRLF at end-of-line only. `tr -d '\r'` deleted every carriage
  return, so `sum=4<CR>2` collapsed into a passing `sum=42` — the same
  reward-hacking class as the trailing-output hole fixed earlier. The agent
  under test is code the optimizer actively reward-maximizes, so a lax
  verifier is a live surface, not a hypothetical one.
- Mirror the rule in test_hello_example_baseline.py, which had the identical
  hole via `.replace("\r", "")`, and assert the shell and Python rules stay in
  lockstep; that pairing has now drifted twice.

Docs:
- Give every evaluator arm its own --experiment-dir. README.md (plain Harbor)
  and docs/e2e/README.md (SDK runner) both pointed at tmp/exp-hello-eval; the
  two evaluators disagree about an existing job dir, so sharing one makes the
  result order-dependent.
- Point the --force-rerun example at tmp/eval-only-sdk rather than the default.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
@ngoncharenko
ngoncharenko force-pushed the ngoncharenko/aalgo-312-wire-eval-into-optimizer branch from 2e23b4b to ca7e2ad Compare July 28, 2026 23:40
@ngoncharenko ngoncharenko changed the title feat(experimentalist): wire up nemo-evaluator-sdk with HarborRunner feat(experimentalist): wire up nemo-evaluator-sdk using HarborRunner Jul 28, 2026
ngoncharenko added a commit that referenced this pull request Jul 28, 2026
…ions

Round-2 review fixes on #955.

Verifier (all 5 copies of tests/test.sh):
- Normalize CRLF at end-of-line only. `tr -d '\r'` deleted every carriage
  return, so `sum=4<CR>2` collapsed into a passing `sum=42` — the same
  reward-hacking class as the trailing-output hole fixed earlier. The agent
  under test is code the optimizer actively reward-maximizes, so a lax
  verifier is a live surface, not a hypothetical one.
- Mirror the rule in test_hello_example_baseline.py, which had the identical
  hole via `.replace("\r", "")`, and assert the shell and Python rules stay in
  lockstep; that pairing has now drifted twice.

Docs:
- Give every evaluator arm its own --experiment-dir. README.md (plain Harbor)
  and docs/e2e/README.md (SDK runner) both pointed at tmp/exp-hello-eval; the
  two evaluators disagree about an existing job dir, so sharing one makes the
  result order-dependent.
- Point the --force-rerun example at tmp/eval-only-sdk rather than the default.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
@ngoncharenko
ngoncharenko force-pushed the ngoncharenko/aalgo-312-wire-eval-into-optimizer branch from ca7e2ad to 04528b1 Compare July 28, 2026 23:43

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/server.py (1)

4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Drop from __future__ import annotations; hints are already concrete for this 3.12 target.

This import stringifies all annotations in the file, conflicting with the path guideline to use concrete type hints. The file already uses native int | None / dict[str, Any] syntax that needs no postponed evaluation on Python 3.12.

As per path instructions for plugins/nemo-experimentalist/**/*.py: "Use concrete type hints rather than string-based type hints, and do not hide imports under TYPE_CHECKING."

🤖 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
`@plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/server.py`
at line 4, Remove the from __future__ import annotations directive from
server.py, leaving the existing concrete Python 3.12 annotations and imports
unchanged.

Source: Path instructions

plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/Dockerfile (1)

5-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Unpinned tau2-bench git ref in both task Dockerfiles. Both images clone tau2-bench with --depth=1 and no commit/tag pin, so rebuilds can silently pick up a different upstream source and dependency versions, breaking eval-image reproducibility.

  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/Dockerfile#L5-L14: add a TAU2_BENCH_REF build arg, git checkout it after clone, pin the uv version, and purge git after install (matching the sibling Dockerfile).
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/Dockerfile#L5-L16: same TAU2_BENCH_REF pin, and cap fastmcp to a known-good range instead of an open-ended >=3.0.
🤖 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
`@plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/Dockerfile`
around lines 5 - 14, Pin tau2-bench and dependency versions in both Dockerfiles:
in
plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/Dockerfile
lines 5-14, add TAU2_BENCH_REF, checkout that ref after cloning, pin uv, and
remove git after installation; in
plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/Dockerfile
lines 5-16, add and checkout TAU2_BENCH_REF and constrain fastmcp to the
known-good version range.
🤖 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 `@plugins/nemo-experimentalist/docs/architecture.md`:
- Around line 308-312: Update the aggregation contract in the trial-status
documentation to state that only trials with status == "completed" are averaged,
replacing the broader status != "failed" wording. Preserve the existing
explanation that failed trials remain in the denominator and all-failed rounds
aggregate to {}.

---

Nitpick comments:
In
`@plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/Dockerfile`:
- Around line 5-14: Pin tau2-bench and dependency versions in both Dockerfiles:
in
plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/Dockerfile
lines 5-14, add TAU2_BENCH_REF, checkout that ref after cloning, pin uv, and
remove git after installation; in
plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/Dockerfile
lines 5-16, add and checkout TAU2_BENCH_REF and constrain fastmcp to the
known-good version range.

In
`@plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/server.py`:
- Line 4: Remove the from __future__ import annotations directive from
server.py, leaving the existing concrete Python 3.12 annotations and imports
unchanged.
🪄 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: 001c2536-569b-47cb-8d78-1958cce08ea6

📥 Commits

Reviewing files that changed from the base of the PR and between 2e23b4b and 04528b1.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (72)
  • plugins/nemo-experimentalist/README.md
  • plugins/nemo-experimentalist/docs/architecture.md
  • plugins/nemo-experimentalist/docs/e2e/README.md
  • plugins/nemo-experimentalist/docs/e2e/experiment-debug-round.yaml
  • plugins/nemo-experimentalist/docs/e2e/experiment-eval-only-sdk.yaml
  • plugins/nemo-experimentalist/docs/e2e/experiment-eval-only.yaml
  • plugins/nemo-experimentalist/docs/e2e/experiment-fast.yaml
  • plugins/nemo-experimentalist/docs/e2e/run-eval-only.py
  • plugins/nemo-experimentalist/examples/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/AGENT-SPEC.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/agent.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/harbor_wrapper.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/main.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/optimizer.yaml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/tracing.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/AGENT-SPEC.md
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/README.md
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/agent.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/docker-compose.yaml
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/Dockerfile
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/server.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/task_config.json
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/instruction.md
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/task.toml
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/tests/test.sh
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/harbor_wrapper.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/main.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/optimizer.yaml
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/pyproject.toml
  • plugins/nemo-experimentalist/pyproject.toml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/config.yaml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/run.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/base.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/factory.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor_agent_task_runner.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/deps.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/run.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/resolve.py
  • plugins/nemo-experimentalist/tests/experimentalist/conftest.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor_ab_e2e.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor_agent_task_runner.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_hello_example_baseline.py
  • plugins/nemo-experimentalist/tests/test_eval_author_run.py
🚧 Files skipped from review as they are similar to previous changes (43)
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/AGENT-SPEC.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/tests/expected.txt
  • plugins/nemo-experimentalist/docs/e2e/experiment-fast.yaml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/task.toml
  • plugins/nemo-experimentalist/docs/e2e/experiment-eval-only-sdk.yaml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/run.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/main.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/optimizer.yaml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/optimizer.yaml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/config.yaml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/task.toml
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/pyproject.toml
  • plugins/nemo-experimentalist/tests/experimentalist/conftest.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/expected.txt
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/run.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/task_config.json
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/test.sh
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/docker-compose.yaml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/main.py
  • plugins/nemo-experimentalist/docs/e2e/experiment-eval-only.yaml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/agent.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/task.toml
  • plugins/nemo-experimentalist/pyproject.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/test.sh
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/README.md
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/base.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/deps.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_hello_example_baseline.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/harbor_wrapper.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/tests/test.sh
  • plugins/nemo-experimentalist/docs/e2e/experiment-debug-round.yaml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/resolve.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/tests/test.sh
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/factory.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py

Comment thread plugins/nemo-experimentalist/docs/architecture.md Outdated
@ngoncharenko ngoncharenko self-assigned this Jul 29, 2026
@ngoncharenko
ngoncharenko force-pushed the ngoncharenko/aalgo-312-wire-eval-into-optimizer branch from 04528b1 to dc158c9 Compare July 29, 2026 01:50

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (7)
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor_agent_task_runner.py (2)

112-112: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use default_factory=list.

Matches HarborRuntimeConfig and avoids a shared literal default.

♻️ Proposed change
-    artifacts: list[str] = Field(default=[], description="Additional Harbor artifact sources to collect per trial.")
+    artifacts: list[str] = Field(
+        default_factory=list, description="Additional Harbor artifact sources to collect per trial."
+    )
🤖 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
`@plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor_agent_task_runner.py`
at line 112, Update the artifacts field in the relevant task-runner
configuration model to use default_factory=list instead of a mutable list
literal default, matching HarborRuntimeConfig and ensuring each instance
receives its own list.

51-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

TYPE_CHECKING-only SDK imports conflict with the repo guideline.

The docstring rationale is sound (a broken SDK install must not break the plain evaluator), so this is likely the right call — but it deviates from the stated rule. Consider annotating with NamedTuple field types resolved via Any/protocols, or documenting the exemption so the next reader doesn't "fix" it.

As per coding guidelines: "prefer concrete type hints over string-based type hints, and do not import those types only under TYPE_CHECKING; import them normally when possible."

🤖 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
`@plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor_agent_task_runner.py`
around lines 51 - 59, Document the intentional TYPE_CHECKING-only imports in the
module or nearby code, explicitly recording the Harbor SDK lazy-import
requirement as an exemption to the normal concrete-type import guideline.
Preserve the existing HarborAgentTaskRunner, HarborRuntimeConfig, and
AgentEvalTask annotations and lazy-import behavior.

Source: Coding guidelines

packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.py (1)

489-501: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Use _safe_resolve here too.

Path.resolve() on a vanishing symlink chain can raise OSError (the reason _safe_resolve exists). Lines 489/500 and 588 call .resolve() directly, so a mid-walk race turns a best-effort cache check into a failed run.

♻️ Proposed change
-    dataset_root = dataset_path.resolve()
+    dataset_root = _safe_resolve(dataset_path)
@@
-            candidate_resolved = candidate.resolve()
+            candidate_resolved = _safe_resolve(candidate)
🤖 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
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.py`
around lines 489 - 501, Replace the direct Path.resolve() calls in the
task-directory cache validation and the related path handling around the runtime
logic with the existing _safe_resolve helper. Ensure vanishing symlink chains or
other OSError cases remain best-effort and continue through normal re-discovery
rather than failing the run.
packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime.py (1)

319-321: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Drop async/asyncio marks from these three tests.

Nothing is awaited in test_changed_inputs_invalidate_the_cache, test_cosmetic_options_do_not_evict_the_cache, or test_task_subset_of_a_cached_run_still_hits — they only call sync helpers.

♻️ Proposed change
-@pytest.mark.asyncio
 `@pytest.mark.parametrize`("mutation", ["agent", "task", "option"])
-async def test_changed_inputs_invalidate_the_cache(tmp_path: Path, mutation: str) -> None:
+def test_changed_inputs_invalidate_the_cache(tmp_path: Path, mutation: str) -> None:

Also applies to: 344-345, 357-358

🤖 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 `@packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime.py` around
lines 319 - 321, Remove the async declaration and `@pytest.mark.asyncio` decorator
from test_changed_inputs_invalidate_the_cache,
test_cosmetic_options_do_not_evict_the_cache, and
test_task_subset_of_a_cached_run_still_hits, since each test uses only
synchronous helpers and does not await anything.
plugins/nemo-experimentalist/docs/architecture.md (2)

582-593: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Keep this page in one Diataxis quadrant.

This is an EXPLANATION page, but it embeds a HOW-TO and ends without Next Steps. Move run/debug instructions to a linked HOW-TO and add cross-links at the end.

🤖 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 `@plugins/nemo-experimentalist/docs/architecture.md` around lines 582 - 593,
Keep architecture.md focused on explanation by removing the embedded “Running
and debugging it” prerequisites and command instructions, then move them into a
linked HOW-TO document. Add an appropriate cross-link to that HOW-TO and a “Next
Steps” section at the end of the architecture page, linking to relevant
follow-up documentation.

Source: Coding guidelines


582-593: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Apply the Diataxis structure rules consistently.

  • plugins/nemo-experimentalist/docs/architecture.md#L582-L593: keep the page as EXPLANATION, move run/debug instructions to a linked HOW-TO, and add Next Steps.
  • plugins/nemo-experimentalist/examples/README.md#L4-L14: identify the page as REFERENCE and add prerequisites plus Next Steps.
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md#L96-L117: keep one TUTORIAL/HOW-TO scope and move reference material to linked pages.
🤖 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 `@plugins/nemo-experimentalist/docs/architecture.md` around lines 582 - 593,
Apply Diataxis structure consistently: in
plugins/nemo-experimentalist/docs/architecture.md lines 582-593, keep the page
explanatory by moving run/debug commands to a linked HOW-TO and adding a Next
Steps section; in plugins/nemo-experimentalist/examples/README.md lines 4-14,
label the page as REFERENCE, add prerequisites, and add Next Steps; in
plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md lines 96-117,
retain a single TUTORIAL/HOW-TO scope and move reference content to linked
pages.

Source: Coding guidelines

plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/Dockerfile (1)

11-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Pin fastmcp to an exact version instead of >=3.0.

FastMCP's own docs call out fastmcp>=3.0.0 as bad practice, since minor versions can ship breaking changes; they recommend exact pins for reproducible builds.

♻️ Proposed fix
-    && pip install --no-cache-dir uv "${TAU2_BENCH_ROOT}[knowledge]" "fastmcp>=3.0" \
+    && pip install --no-cache-dir uv "${TAU2_BENCH_ROOT}[knowledge]" "fastmcp==3.4.4" \
🤖 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
`@plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/Dockerfile`
around lines 11 - 16, Update the fastmcp dependency in the Dockerfile install
command to use an exact version pin rather than the open-ended “>=3.0”
constraint, while leaving the other package installations unchanged.
🤖 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
`@plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor_agent_task_runner.py`:
- Around line 355-382: Update test_errored_cached_job_is_rerun to use the
stamped, complete cached_job_dir fixture instead of manually constructing an
unstamped job_dir with _write_trial. Before invoking HarborRunnerEvaluator._run,
modify the cached sum-three trial’s result.json to add exception_info while
preserving the cache stamp, then assert fake_job.calls confirms only that
errored trial is rerun.

---

Nitpick comments:
In
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.py`:
- Around line 489-501: Replace the direct Path.resolve() calls in the
task-directory cache validation and the related path handling around the runtime
logic with the existing _safe_resolve helper. Ensure vanishing symlink chains or
other OSError cases remain best-effort and continue through normal re-discovery
rather than failing the run.

In `@packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime.py`:
- Around line 319-321: Remove the async declaration and `@pytest.mark.asyncio`
decorator from test_changed_inputs_invalidate_the_cache,
test_cosmetic_options_do_not_evict_the_cache, and
test_task_subset_of_a_cached_run_still_hits, since each test uses only
synchronous helpers and does not await anything.

In `@plugins/nemo-experimentalist/docs/architecture.md`:
- Around line 582-593: Keep architecture.md focused on explanation by removing
the embedded “Running and debugging it” prerequisites and command instructions,
then move them into a linked HOW-TO document. Add an appropriate cross-link to
that HOW-TO and a “Next Steps” section at the end of the architecture page,
linking to relevant follow-up documentation.
- Around line 582-593: Apply Diataxis structure consistently: in
plugins/nemo-experimentalist/docs/architecture.md lines 582-593, keep the page
explanatory by moving run/debug commands to a linked HOW-TO and adding a Next
Steps section; in plugins/nemo-experimentalist/examples/README.md lines 4-14,
label the page as REFERENCE, add prerequisites, and add Next Steps; in
plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md lines 96-117,
retain a single TUTORIAL/HOW-TO scope and move reference content to linked
pages.

In
`@plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/Dockerfile`:
- Around line 11-16: Update the fastmcp dependency in the Dockerfile install
command to use an exact version pin rather than the open-ended “>=3.0”
constraint, while leaving the other package installations unchanged.

In
`@plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor_agent_task_runner.py`:
- Line 112: Update the artifacts field in the relevant task-runner configuration
model to use default_factory=list instead of a mutable list literal default,
matching HarborRuntimeConfig and ensuring each instance receives its own list.
- Around line 51-59: Document the intentional TYPE_CHECKING-only imports in the
module or nearby code, explicitly recording the Harbor SDK lazy-import
requirement as an exemption to the normal concrete-type import guideline.
Preserve the existing HarborAgentTaskRunner, HarborRuntimeConfig, and
AgentEvalTask annotations and lazy-import behavior.
🪄 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: 3b7ee668-add0-46c3-8b43-e941f15917c9

📥 Commits

Reviewing files that changed from the base of the PR and between 2e23b4b and 77d8b1b.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (74)
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime.py
  • plugins/nemo-experimentalist/README.md
  • plugins/nemo-experimentalist/docs/architecture.md
  • plugins/nemo-experimentalist/docs/e2e/README.md
  • plugins/nemo-experimentalist/docs/e2e/experiment-debug-round.yaml
  • plugins/nemo-experimentalist/docs/e2e/experiment-eval-only-sdk.yaml
  • plugins/nemo-experimentalist/docs/e2e/experiment-eval-only.yaml
  • plugins/nemo-experimentalist/docs/e2e/experiment-fast.yaml
  • plugins/nemo-experimentalist/docs/e2e/run-eval-only.py
  • plugins/nemo-experimentalist/examples/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/AGENT-SPEC.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/agent.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/harbor_wrapper.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/main.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/optimizer.yaml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/tracing.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/AGENT-SPEC.md
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/README.md
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/agent.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/docker-compose.yaml
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/Dockerfile
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/server.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/task_config.json
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/instruction.md
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/task.toml
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/tests/test.sh
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/harbor_wrapper.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/main.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/optimizer.yaml
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/pyproject.toml
  • plugins/nemo-experimentalist/pyproject.toml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/config.yaml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/run.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/base.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/factory.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor_agent_task_runner.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/deps.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/run.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/resolve.py
  • plugins/nemo-experimentalist/tests/experimentalist/conftest.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor_ab_e2e.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor_agent_task_runner.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_hello_example_baseline.py
  • plugins/nemo-experimentalist/tests/test_eval_author_run.py
🚧 Files skipped from review as they are similar to previous changes (44)
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/task.toml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/run.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/runtime-server/task_config.json
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/pyproject.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/expected.txt
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/README.md
  • plugins/nemo-experimentalist/docs/e2e/README.md
  • plugins/nemo-experimentalist/pyproject.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/task.toml
  • plugins/nemo-experimentalist/docs/e2e/experiment-fast.yaml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/config.yaml
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/main.py
  • plugins/nemo-experimentalist/docs/e2e/experiment-eval-only.yaml
  • plugins/nemo-experimentalist/tests/experimentalist/conftest.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/optimizer.yaml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/resolve.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/tests/expected.txt
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/factory.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/base.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/optimizer.yaml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/tests/test.sh
  • plugins/nemo-experimentalist/tests/test_eval_author_run.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/main.py
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/task.toml
  • plugins/nemo-experimentalist/docs/e2e/experiment-debug-round.yaml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/deps.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/task.toml
  • plugins/nemo-experimentalist/examples/tau2-nemo-oo-agent/dataset/template/task_template/environment/docker-compose.yaml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/agent.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/tests/test.sh
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/run.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_hello_example_baseline.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/test.sh
  • plugins/nemo-experimentalist/README.md
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/harbor_wrapper.py

@SandyChapman SandyChapman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've got some questions around using AgentEvaluator and duplication of models in the plugin.

Comment thread plugins/nemo-experimentalist/docs/e2e/run-eval-only.py Outdated
# The returned SDK trials are discarded: they expose only a single
# `harbor_reward.reward`, while the loop needs every verifier metric. The
# job dir is the richer contract, so both evaluators read that instead.
sdk_trials = await runner.run_tasks(list(sdk_tasks.values()))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why invoke run_tasks directly instead of using AgentEvaluator?

@ngoncharenko ngoncharenko Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need to score trials. Harbor does it. trials_from_job_dir gets all the data it needs for both native_harbor and evaluator_harbor. If we make it to score, this adds computation that is not used later to construct experimentalist's TrialResult. The source of truth for both harbor's is the job results dir content. If/when native_harbor is removed, then we can switch that to AgentEvalResult ---> TrialResult

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the point is to give a consistent API contract based on Evaluator's conventions rather than require experimentalist to build it's own data model for evaluation. E.g. Experimentalist should use AgentEvalResult and TrialResult removed. We can pick this up in standup/parking lot today.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it makes sense, but we will need to resolve the gaps first in our models as discussed above: #955 (comment) + P3 section gaps https://docs.google.com/document/d/1Fuqm2KkbSWHYuuGNw3uHwOKfRNCBO0NzULuYWnoClFg/edit?tab=t.7gayh93h0pc2#heading=h.28u730wv2tvv before we can replace the contract

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/harbor_runtime.py`:
- Around line 931-934: Update the package identity construction near
_digest_directory and _safe_identifier to include a digest of
agent_dir.resolve() alongside the content digest, so equal-content directories
with the same basename remain distinct. Preserve the existing import-root and
identifier format as appropriate, and add a test covering overlapping
installations of same-basename agent directories that verifies each imports from
its own resolved path.

In `@packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime_e2e.py`:
- Around line 123-136: Make the resume test distinguishable from a full rerun by
preserving an observable artifact in the existing job directory. Add a marker
file before the second run and assert it still exists afterward, or retain one
completed trial while deleting another result and verify the retained attempt is
unchanged. Update the test around run_harbor_eval and the resume-job setup
without changing the expected resumed trial 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: 68d2c23b-82d5-4a0a-b169-87bdcb3bb919

📥 Commits

Reviewing files that changed from the base of the PR and between 004b9ba and a25a06c.

⛔ Files ignored due to path filters (1)
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/harbor_runtime.py is excluded by !sdk/**
📒 Files selected for processing (3)
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime_e2e.py

Comment thread packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime_e2e.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
plugins/nemo-experimentalist/docs/architecture.md (1)

688-728: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Move the runnable procedure to a HOW-TO page.

This task-specific smoke-test procedure mixes HOW-TO content into the architecture EXPLANATION. Extract it to a dedicated E2E how-to and retain a short cross-link here. As per coding guidelines, “Each documentation page should fit ONE Diataxis quadrant.”

🤖 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 `@plugins/nemo-experimentalist/docs/architecture.md` around lines 688 - 728,
Move the “Verifying without credentials” runnable procedure and its code sample
from the architecture explanation into a dedicated E2E HOW-TO page, preserving
the instructions and references there. Replace the removed section in
architecture.md with a brief cross-link to the new how-to page, keeping the
architecture page focused on explanation.

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 `@plugins/nemo-experimentalist/docs/architecture.md`:
- Around line 294-296: Update the architecture documentation to remove the claim
that an unavailable SDK falls back to harbor. Document harbor_native as the A/B
baseline and harbor only as a deprecated alias, unless the factory import flow
is changed to implement a genuine fallback.

---

Nitpick comments:
In `@plugins/nemo-experimentalist/docs/architecture.md`:
- Around line 688-728: Move the “Verifying without credentials” runnable
procedure and its code sample from the architecture explanation into a dedicated
E2E HOW-TO page, preserving the instructions and references there. Replace the
removed section in architecture.md with a brief cross-link to the new how-to
page, keeping the architecture page focused on explanation.
🪄 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: b7122eb6-5abb-4a23-8448-a208c5d8f478

📥 Commits

Reviewing files that changed from the base of the PR and between a25a06c and 34776fa.

📒 Files selected for processing (21)
  • plugins/nemo-experimentalist/docs/architecture.md
  • plugins/nemo-experimentalist/docs/e2e/README.md
  • plugins/nemo-experimentalist/docs/e2e/experiment-eval-only-sdk.yaml
  • plugins/nemo-experimentalist/docs/e2e/experiment-eval-only.yaml
  • plugins/nemo-experimentalist/docs/e2e/run-eval-only.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/config.yaml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/run.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/base.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/factory.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor_evaluator.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/deps.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/resolve.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_dataset_staging.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_factory.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor_ab_e2e.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor_evaluator.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_loop_failure.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_loop_insight_suite.py
  • plugins/nemo-experimentalist/tests/test_eval_author_run.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • plugins/nemo-experimentalist/docs/e2e/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md

Comment thread plugins/nemo-experimentalist/docs/architecture.md Outdated
ngoncharenko added a commit that referenced this pull request Jul 30, 2026
Three findings from CodeRabbit on PR #955.

* Scoped agent import: `_install_agent_package` wrote `__path__` unconditionally,
  so a second scope sharing a package name silently repointed the first,
  still-open scope at a different directory. Keep the first path instead.

  CodeRabbit proposed hashing the resolved directory path into the package
  identity. Rejected: that makes the name location-dependent, so the same agent
  evaluated from a different path yields a different JobConfig and Harbor refuses
  to resume -- reintroducing AALGO-430, which this branch exists to fix. A shared
  package name implies an equal content digest, hence byte-identical trees (the
  excluded content is not importable), so the already-installed path is exactly as
  correct and keeping it is the safe resolution.

* Resume e2e: deleting the only trial result made "discarded and re-run from
  scratch" observationally identical to "resumed", so the test passed either way.
  Now runs two attempts, drops one, and asserts the surviving trial directory
  keeps its name and its result.json byte-for-byte -- Harbor suffixes trial dirs
  with a shortuuid, so a discarded job dir returns under a different name.
  Verified by mutation: forcing force_rerun=True now fails the test.

* architecture.md no longer promises `harbor` as an SDK-unavailable fallback --
  false since the SDK import moved to module scope. CodeRabbit flagged one
  paragraph; the adjacent "import optional runtimes lazily inside `_run`" guidance
  was wrong for the same reason and is corrected too. Uses `harbor_native` per the
  rename, with `harbor` named only as the deprecated alias.

Refs AALGO-430, AALGO-312.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.py (1)

299-320: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win

before stamp computed even when caching is disabled.

before = _cache_stamp(self._config, dataset_path, coverage) (and the coverage walk feeding it) runs unconditionally whenever a native job is (re)built — including when self._config.job_name is None. Only the after comparison/write is gated on job_name is not None (line 309). This directly contradicts the stated intent a few lines above ("This keeps the digest I/O off every run of callers that don't pin a job name") — every unpinned run now pays for a full agent/dataset content digest with no use for the result.

🔧 Proposed fix
-                coverage = _stamp_coverage(dataset_path, tasks, self._task_names)
-                before = _cache_stamp(self._config, dataset_path, coverage)
+                before = None
+                if self._config.job_name is not None:
+                    coverage = _stamp_coverage(dataset_path, tasks, self._task_names)
+                    before = _cache_stamp(self._config, dataset_path, coverage)
                 await run_job()
                 if self._config.job_name is not None:
                     after = _cache_stamp(self._config, dataset_path, coverage)
🤖 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
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.py`
around lines 299 - 320, Gate the cache fingerprinting flow in the native job
execution path on self._config.job_name being set: only compute coverage and
before, run the before/after comparison, and write the cache stamp for pinned
jobs. Keep unpinned jobs from performing _stamp_coverage or _cache_stamp while
preserving the existing Harbor execution behavior.
🤖 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.

Outside diff comments:
In
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.py`:
- Around line 299-320: Gate the cache fingerprinting flow in the native job
execution path on self._config.job_name being set: only compute coverage and
before, run the before/after comparison, and write the cache stamp for pinned
jobs. Keep unpinned jobs from performing _stamp_coverage or _cache_stamp while
preserving the existing Harbor execution behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e2947179-02b4-40e3-baec-c2f4fb93cf39

📥 Commits

Reviewing files that changed from the base of the PR and between 3d474f4 and 7df6bd9.

⛔ Files ignored due to path filters (1)
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/harbor_runtime.py is excluded by !sdk/**
📒 Files selected for processing (4)
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime_e2e.py
  • plugins/nemo-experimentalist/docs/architecture.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • plugins/nemo-experimentalist/docs/architecture.md

@ngoncharenko
ngoncharenko force-pushed the ngoncharenko/aalgo-312-wire-eval-into-optimizer branch from 7df6bd9 to 2ac8e19 Compare July 30, 2026 17:13

@SandyChapman SandyChapman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

based on our convo in standup the approach is:

  1. keep the existing experimentalist models; mark them deprecated (or somehow signal that we should be informed if changes are being made)
  2. merge this and close any feature gaps with the harbor runner
  3. perform UAT testing with the experimentalist team
  4. cut over to the new evaluator-harbor runner
  5. migrate experimentalist to the evaluator models

@ngoncharenko
ngoncharenko force-pushed the ngoncharenko/aalgo-312-wire-eval-into-optimizer branch from 2ac8e19 to 09ae12e Compare July 30, 2026 20:57

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 17

🧹 Nitpick comments (9)
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py (1)

791-797: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Derive job_name from the resolved path.

agent.name is taken from the unresolved argument, so Path(".") or a trailing .. yields an empty/garbage job name (and hence job dir + cache key), while agent_path right above is already normalized.

♻️ Suggested change
-        job_name=options.job_name or f"{agent.name}-{dataset.id}",
+        job_name=options.job_name or f"{agent_path.name}-{dataset.id}",
🤖 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
`@plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py`
around lines 791 - 797, Update the job_name fallback in the HarborRunInputs
construction to derive the agent identifier from the already-resolved agent_path
rather than the unresolved agent.name argument. Preserve options.job_name when
provided, and keep the existing dataset-based fallback format unchanged.
plugins/nemo-experimentalist/tests/test_deps.py (1)

59-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Pin the error to evaluator_type.

A bare ValidationError also passes if the agent URI or datasets stop validating, so the test could go green without the alias ever being rejected.

♻️ Suggested tightening
-    with pytest.raises(ValidationError):
+    with pytest.raises(ValidationError, match="harbor"):
🤖 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 `@plugins/nemo-experimentalist/tests/test_deps.py` around lines 59 - 67, Update
test_deprecated_evaluator_alias_is_rejected to assert that the ValidationError
specifically identifies evaluator_type as the failing field, while preserving
the existing invalid alias and valid agent/dataset setup so the test cannot pass
due to unrelated validation failures.
packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime.py (1)

331-332: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Drop async/asyncio from these three pure-sync tests.

test_changed_inputs_invalidate_the_cache, test_cosmetic_options_do_not_evict_the_cache, and test_task_subset_of_a_cached_run_still_hits never await; the event-loop plumbing is noise.

Also applies to: 355-356, 368-369

🤖 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 `@packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime.py` around
lines 331 - 332, Remove the async declaration from
test_changed_inputs_invalidate_the_cache,
test_cosmetic_options_do_not_evict_the_cache, and
test_task_subset_of_a_cached_run_still_hits, and remove any corresponding
asyncio/event-loop plumbing in these tests. Keep their existing synchronous
assertions and behavior unchanged.
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.py (1)

503-550: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Use _safe_resolve for the candidate/dataset paths.

resolve() here can raise OSError on a symlink that vanishes mid-check, which would kill a run that the rest of the fingerprint path deliberately degrades instead of failing.

♻️ Suggested change
-    dataset_root = dataset_path.resolve()
+    dataset_root = _safe_resolve(dataset_path)
@@
-            candidate_resolved = candidate.resolve()
+            candidate_resolved = _safe_resolve(candidate)
🤖 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
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.py`
around lines 503 - 550, Update _task_dirs_for to use the existing _safe_resolve
helper for both the stamped candidate path and the active dataset_root instead
of calling Path.resolve() directly. Preserve the current directory and
relative-to-dataset validation, and ensure transient resolution failures degrade
to the existing unresolved/uncacheable behavior rather than propagating OSError.
plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/runtime-server/server.py (2)

12-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Prefer collections.abc.Callable over the deprecated typing.Callable.

🤖 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
`@plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/runtime-server/server.py`
at line 12, Update the Callable import in server.py to use
collections.abc.Callable instead of typing.Callable, while retaining Any from
typing.

179-183: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Silent except Exception hides tool-loading failures.

If get_user_tools fails, the user simulator runs without tools and the task fails in a way that's impossible to diagnose from the state log. Log the exception at minimum.

Proposed fix
-        user_tools = None
         try:
             user_tools = self.environment.get_user_tools(include=self.task.user_tools) or None
-        except Exception:
+        except Exception:  # noqa: BLE001 - domain envs raise heterogeneous errors
+            print(f"warning: user tools unavailable for domain {self.domain}", file=sys.stderr)
             user_tools = None
🤖 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
`@plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/runtime-server/server.py`
around lines 179 - 183, Update the get_user_tools error handling in the runtime
server to log the caught exception with sufficient context before setting
user_tools to None, while preserving the existing fallback behavior.
plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/harbor_wrapper.py (1)

130-141: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Move import asyncio to module scope.

Proposed fix
-        import asyncio as _asyncio
-
         proc = None
         for _attempt in range(3):
@@
             if _attempt < 2:
-                await _asyncio.sleep(10)
+                await asyncio.sleep(10)

Add import asyncio at the top of the file.

🤖 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 `@plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/harbor_wrapper.py`
around lines 130 - 141, Move the asyncio import from inside the setup retry flow
to module scope at the top of harbor_wrapper.py, then update the retry loop to
use the module-level asyncio reference while preserving the existing sleep
behavior.
plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/AGENT-SPEC.md (1)

19-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Fix markdownlint failures. Add a blank line after the headings on lines 19, 33, 49, 154 (MD022) and end the file with a newline (MD047).

Also applies to: 33-33, 49-49, 154-154, 167-167

🤖 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 `@plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/AGENT-SPEC.md` at
line 19, Add blank lines immediately after the headings at the affected
locations in AGENT-SPEC.md, including the heading near line 167, and ensure the
file ends with a trailing newline. Preserve all existing content and structure.

Source: Linters/SAST tools

plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/agent.py (1)

20-24: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Missing INFERENCE_API_KEY fails late.

api_key=None surfaces as an opaque provider auth error mid-run. Fail fast at construction with a clear message.

🤖 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 `@plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/agent.py` around
lines 20 - 24, Validate that INFERENCE_API_KEY is present before constructing
CompletionClient, and fail immediately with a clear configuration error when it
is missing. Keep the existing model and INFERENCE_BASE_URL defaults unchanged,
and pass the validated key to CompletionClient.
🤖 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 `@plugins/nemo-experimentalist/docs/architecture.md`:
- Around line 594-598: Replace NVIDIA_INFERENCE_HUB_KEY with INFERENCE_API_KEY
in the setup commands at plugins/nemo-experimentalist/docs/architecture.md lines
594-598, plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md
lines 84-87, and lines 185-187 so all documented flows provide the credential
consumed by the CLI; alternatively document the required mapping explicitly at
each site.
- Around line 291-296: Update the evaluator-selection documentation to identify
harbor_evaluator as the default SDK-backed evaluator and harbor_native as the
A/B baseline or fallback, preserving the deprecated harbor alias behavior. Apply
this contract correction in plugins/nemo-experimentalist/docs/architecture.md
lines 291-296 and update the evaluator comparison table in
plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md lines
100-103; no runtime changes are needed.

In `@plugins/nemo-experimentalist/docs/e2e/run-eval-only.py`:
- Around line 32-35: Align the evaluator default and documentation with the
SDK-default contract: in plugins/nemo-experimentalist/docs/e2e/run-eval-only.py
lines 32-35, change the evaluator default to harbor_evaluator; in
plugins/nemo-experimentalist/docs/e2e/experiment-eval-only-sdk.yaml lines 21-22,
identify harbor_evaluator as the default SDK arm; in
plugins/nemo-experimentalist/docs/e2e/experiment-eval-only.yaml lines 22-24,
identify harbor_native as the explicit baseline; and in
plugins/nemo-experimentalist/tests/test_e2e_helper_defaults.py lines 13-20,
rename the test and expect harbor_evaluator.

In
`@plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/tests/test.sh`:
- Around line 41-47: Update the comparison logic in the test script around
EXPECTED and ACTUAL so command substitution does not remove trailing newlines.
Normalize CRLF line endings while preserving each file’s complete contents, then
compare those contents directly so outputs with missing or extra trailing
newlines are scored differently.

In `@plugins/nemo-experimentalist/examples/hello-harbor-agent/harbor_wrapper.py`:
- Around line 62-69: Update the upload loop around AGENT_DIR.iterdir() to
explicitly reject symlink entries before calling entry.is_file() or
entry.is_dir(), and ensure recursive directory uploads also reject symlinks at
every level rather than following them. Preserve the existing EXCLUDE filtering
and upload destinations for regular files and directories.

In `@plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/AGENT-SPEC.md`:
- Around line 119-121: Correct the grammar in the cabin-change pricing rule by
replacing “the user is should be refunded” with “the user should be refunded,”
while preserving the stated refund behavior.
- Line 1: Add the standard NVIDIA CORPORATION & AFFILIATES Apache-2.0 SPDX
header as an HTML comment at the beginning of AGENT-SPEC.md, matching the format
used by the directory’s README.md.

In `@plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/agent.py`:
- Line 6: Resolve the unused requests dependency in the example module: either
remove the requests import if it is not needed, or declare requests in the
example manifest so clean environments can import the module successfully.
Update the import or manifest associated with the agent example accordingly.

In
`@plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/Dockerfile`:
- Line 1: Prepend the standard NVIDIA copyright and Apache-2.0 SPDX comment
lines to
plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/Dockerfile
lines 1-1, runtime-server/Dockerfile lines 1-1, and docker-compose.yaml lines
1-1; place the headers above the existing FROM or services declarations.
- Around line 10-14: Pin the cloned tau2-bench revision in both Dockerfiles: add
a TAU2_BENCH_REV build argument, check out that revision after cloning in the
task environment Dockerfile, and apply the same argument and checkout flow in
the runtime-server Dockerfile so both images use one reproducible version.

In
`@plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/runtime-server/Dockerfile`:
- Line 14: Update the fastmcp dependency in the Dockerfile’s pip install command
to require the repository-supported baseline of fastmcp>=3.2.0 and constrain it
below version 4, preserving the existing installation extras and other
dependencies.

In
`@plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/runtime-server/server.py`:
- Line 560: Protect the shared Tau3Runtime instance and its mutable state by
adding a synchronization lock around every MCP tool entry point that accesses or
mutates runtime, including the tools near the global initialization and lines
627-632. Ensure each complete tool call, including state-file updates and
step/error accounting, executes single-flight so concurrent requests cannot
interleave.
- Around line 251-266: Update _write_state to serialize the payload to a
temporary file in STATE_LOG_PATH.parent, then atomically replace STATE_LOG_PATH
with os.replace after the write completes. Ensure the temporary file uses the
same directory and is cleaned up if writing or replacement fails.

In
`@plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/task.toml`:
- Line 1: Restore the SPDX license and copyright headers across
task_template/task.toml, task_template/instruction.md, both Dockerfiles,
task_template/docker-compose.yaml,
task_template/environment/runtime-server/task_config.json, and
task_template/tests/test.sh. For task_config.json, add the SPDX metadata as
top-level JSON properties rather than comments so json.loads remains valid; use
the repository’s standard header format for each other file.

In `@plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/optimizer.yaml`:
- Line 17: Update the registry_url in the tau2 optimizer configuration to either
reference a publicly accessible, redistributable registry or clearly document
the NVIDIA-internal access requirement in the surrounding comment. Preserve the
example’s dataset configuration while ensuring users understand or can satisfy
the registry access prerequisite.

In `@plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/pyproject.toml`:
- Around line 16-17: Update the nooa source entry in the pyproject configuration
to use an immutable commit SHA via rev instead of the movable v0.0.6 tag, and
synchronize the same revision with the main nooa pin, examples/tau3-nooa-agent
pin, and both lockfiles.

In
`@plugins/nemo-experimentalist/tests/experimentalist/test_hello_example_baseline.py`:
- Around line 109-112: Update the baseline test around `_EXPECTED_BASELINE` to
enumerate each split’s on-disk task directory and assert its task IDs exactly
match the expected IDs before constructing `rewards`. Keep the existing
reward-shape assertion, ensuring extra or missing tasks cannot pass unnoticed.

---

Nitpick comments:
In
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.py`:
- Around line 503-550: Update _task_dirs_for to use the existing _safe_resolve
helper for both the stamped candidate path and the active dataset_root instead
of calling Path.resolve() directly. Preserve the current directory and
relative-to-dataset validation, and ensure transient resolution failures degrade
to the existing unresolved/uncacheable behavior rather than propagating OSError.

In `@packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime.py`:
- Around line 331-332: Remove the async declaration from
test_changed_inputs_invalidate_the_cache,
test_cosmetic_options_do_not_evict_the_cache, and
test_task_subset_of_a_cached_run_still_hits, and remove any corresponding
asyncio/event-loop plumbing in these tests. Keep their existing synchronous
assertions and behavior unchanged.

In `@plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/AGENT-SPEC.md`:
- Line 19: Add blank lines immediately after the headings at the affected
locations in AGENT-SPEC.md, including the heading near line 167, and ensure the
file ends with a trailing newline. Preserve all existing content and structure.

In `@plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/agent.py`:
- Around line 20-24: Validate that INFERENCE_API_KEY is present before
constructing CompletionClient, and fail immediately with a clear configuration
error when it is missing. Keep the existing model and INFERENCE_BASE_URL
defaults unchanged, and pass the validated key to CompletionClient.

In
`@plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/runtime-server/server.py`:
- Line 12: Update the Callable import in server.py to use
collections.abc.Callable instead of typing.Callable, while retaining Any from
typing.
- Around line 179-183: Update the get_user_tools error handling in the runtime
server to log the caught exception with sufficient context before setting
user_tools to None, while preserving the existing fallback behavior.

In `@plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/harbor_wrapper.py`:
- Around line 130-141: Move the asyncio import from inside the setup retry flow
to module scope at the top of harbor_wrapper.py, then update the retry loop to
use the module-level asyncio reference while preserving the existing sleep
behavior.

In
`@plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py`:
- Around line 791-797: Update the job_name fallback in the HarborRunInputs
construction to derive the agent identifier from the already-resolved agent_path
rather than the unresolved agent.name argument. Preserve options.job_name when
provided, and keep the existing dataset-based fallback format unchanged.

In `@plugins/nemo-experimentalist/tests/test_deps.py`:
- Around line 59-67: Update test_deprecated_evaluator_alias_is_rejected to
assert that the ValidationError specifically identifies evaluator_type as the
failing field, while preserving the existing invalid alias and valid
agent/dataset setup so the test cannot pass due to unrelated validation
failures.
🪄 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: 2353747e-a71e-4e74-8d4e-9d0fdae8f953

📥 Commits

Reviewing files that changed from the base of the PR and between 3d474f4 and a41f3c9.

⛔ Files ignored due to path filters (1)
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/harbor_runtime.py is excluded by !sdk/**
📒 Files selected for processing (84)
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime_e2e.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/config.yaml
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/run.py
  • plugins/nemo-eval-author/tests/test_eval_author_run.py
  • plugins/nemo-experimentalist/README.md
  • plugins/nemo-experimentalist/docs/architecture.md
  • plugins/nemo-experimentalist/docs/e2e/README.md
  • plugins/nemo-experimentalist/docs/e2e/experiment-debug-round.yaml
  • plugins/nemo-experimentalist/docs/e2e/experiment-eval-only-sdk.yaml
  • plugins/nemo-experimentalist/docs/e2e/experiment-eval-only.yaml
  • plugins/nemo-experimentalist/docs/e2e/experiment-fast.yaml
  • plugins/nemo-experimentalist/docs/e2e/run-eval-only.py
  • plugins/nemo-experimentalist/examples/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/.env.example
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/AGENT-SPEC.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/agent.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/harbor_wrapper.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/main.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/optimizer.yaml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/tracing.py
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/.env.example
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/AGENT-SPEC.md
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/README.md
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/agent.py
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/docker-compose.yaml
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/runtime-server/Dockerfile
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/runtime-server/server.py
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/runtime-server/task_config.json
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/instruction.md
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/task.toml
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/tests/test.sh
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/harbor_wrapper.py
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/main.py
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/optimizer.yaml
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/pyproject.toml
  • plugins/nemo-experimentalist/pyproject.toml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/base.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/factory.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor_evaluator.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/deps.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/run.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/preflight.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/resolve.py
  • plugins/nemo-experimentalist/tests/experimentalist/conftest.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_dataset_staging_loop.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_factory.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor_ab_e2e.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor_evaluator.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_hello_example_baseline.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_loop_failure.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_loop_insight_suite.py
  • plugins/nemo-experimentalist/tests/test_deps.py
  • plugins/nemo-experimentalist/tests/test_e2e_helper_defaults.py
🚧 Files skipped from review as they are similar to previous changes (6)
  • plugins/nemo-eval-author/tests/test_eval_author_run.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/config.yaml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/.env.example
  • plugins/nemo-experimentalist/docs/e2e/README.md
  • plugins/nemo-experimentalist/tests/experimentalist/test_loop_insight_suite.py

Comment thread plugins/nemo-experimentalist/docs/architecture.md
Comment thread plugins/nemo-experimentalist/docs/architecture.md Outdated
Comment thread plugins/nemo-experimentalist/docs/e2e/run-eval-only.py Outdated
Comment thread plugins/nemo-experimentalist/examples/hello-harbor-agent/harbor_wrapper.py Outdated
Comment thread plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/optimizer.yaml Outdated
Comment thread plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/pyproject.toml Outdated
Comment thread plugins/nemo-experimentalist/tests/experimentalist/test_hello_example_baseline.py Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/Dockerfile (1)

1-1: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Still missing SPDX header.

Prior review flagged this same file for a missing SPDX header; it's still absent at line 1.

📝 Proposed fix
+# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+# SPDX-License-Identifier: Apache-2.0
+
 FROM python:3.12-slim
As per coding guidelines: "Every file must include the SPDX copyright header for NVIDIA CORPORATION & AFFILIATES and the Apache-2.0 license."
🤖 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
`@plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/Dockerfile`
at line 1, Add the standard SPDX copyright and Apache-2.0 license header at the
beginning of the Dockerfile before the existing FROM directive. Use the
repository’s established NVIDIA CORPORATION & AFFILIATES header format.

Source: Coding guidelines

🧹 Nitpick comments (1)
plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md (1)

96-217: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Separate the Diataxis content.

This section mixes HOW-TO commands, evaluator explanations, caching concepts, and configuration reference. Split these into linked pages, or retain one quadrant here and link out. As per coding guidelines, “Each documentation page should fit ONE Diataxis quadrant; do not mix tutorials with reference tables or how-tos with architecture explanations; use cross-links instead.”

🤖 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 `@plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md` around
lines 96 - 217, Separate the mixed Diataxis content around the “Running the
evaluation through the NeMo Evaluator SDK” section: keep this README focused on
one quadrant, and move the A/B commands, caching guidance, conceptual evaluator
explanation, and configuration reference into appropriately linked pages. Add
clear cross-links so readers can find the relocated how-to, explanation, and
reference content without duplicating it.

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
`@plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/runtime-server/Dockerfile`:
- Around line 6-9: Add the repository-standard SPDX copyright and license
declarations before the Dockerfile’s FROM instruction, preserving the existing
TAU2_BENCH_REV argument and surrounding comments unchanged.

---

Duplicate comments:
In
`@plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/Dockerfile`:
- Line 1: Add the standard SPDX copyright and Apache-2.0 license header at the
beginning of the Dockerfile before the existing FROM directive. Use the
repository’s established NVIDIA CORPORATION & AFFILIATES header format.

---

Nitpick comments:
In `@plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md`:
- Around line 96-217: Separate the mixed Diataxis content around the “Running
the evaluation through the NeMo Evaluator SDK” section: keep this README focused
on one quadrant, and move the A/B commands, caching guidance, conceptual
evaluator explanation, and configuration reference into appropriately linked
pages. Add clear cross-links so readers can find the relocated how-to,
explanation, and reference content without duplicating it.
🪄 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: dc6bc4c5-9221-4ce1-bec4-21c2f74061f5

📥 Commits

Reviewing files that changed from the base of the PR and between a41f3c9 and 6a7ef47.

⛔ Files ignored due to path filters (1)
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/harbor_runtime.py is excluded by !sdk/**
📒 Files selected for processing (25)
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/config.yaml
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/run.py
  • plugins/nemo-eval-author/tests/test_eval_author_run.py
  • plugins/nemo-experimentalist/docs/architecture.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/harbor_wrapper.py
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/README.md
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/agent.py
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/docker-compose.yaml
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/runtime-server/Dockerfile
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/runtime-server/server.py
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/runtime-server/task_config.json
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/task.toml
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/optimizer.yaml
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/pyproject.toml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor_evaluator.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_tau2_example_contract.py
🚧 Files skipped from review as they are similar to previous changes (8)
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/runtime-server/task_config.json
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/docker-compose.yaml
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/optimizer.yaml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py
  • plugins/nemo-experimentalist/docs/architecture.md
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/README.md
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/task.toml
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.py

ngoncharenko and others added 7 commits July 30, 2026 21:29
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
`job_name` is the cache identity: it names the Harbor job directory, which is what
the SDK's success-aware cache and Harbor's per-trial resume both key on. It was
built from the caller's spelling of the agent path while the SDK's scoped agent
import derives its package name from the *resolved* directory, so the two
identities could disagree.

Concretely: `Path(".").name` is empty, so every `--agent .` run produced job name
`-<dataset>` regardless of which directory it pointed at, and two different agents
collided on one job dir. With a symlinked agent dir the alias kept its own name
while resolving elsewhere, so flipping the link at a fixed job_name reused one job
dir for a different agent -- caught only by Harbor's config refusal rather than by
design.

`scoped_harbor_agent_import`'s docstring justified its location-sensitivity by
asserting the caller's job_name tracks the agent name too. That was only true once
both derive from the resolved path; the docstring now says so explicitly.

Migration: this changes the cache identity for pinned runs whose agent was passed
as `.` or through a symlink -- one silent re-run each. Old job dirs stay on disk.

Also in this change:

* `_task_dirs_for` uses `_safe_resolve()` for the dataset root and stamped task
  paths. Bare `resolve()` raises when a symlink vanishes mid-walk, which would
  fail a run over a best-effort cache guard that is designed to degrade.

* Align the Eval Author's `evaluator_type` default with the Experimentalist's
  (`harbor_native`). It selects only the dataset adapter -- `run_eval_author`
  never calls `build_evaluator`, and both types map to `HarborDataset` -- so this
  is inert today, but it stops being inert the day the two types get different
  Dataset classes. Guarded by a test so the two cannot drift.

* Assert the deprecated-alias ValidationError originates from `evaluator_type`,
  not merely that some field rejected the value.

Refs AALGO-312, AALGO-430.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
* All five verifier scripts now compare byte-for-byte with `cmp` after
  end-of-line CRLF normalization, instead of `[ "$ACTUAL" = "$EXPECTED" ]` over
  command substitution. Command substitution strips *every* trailing newline on
  both sides, so an agent could append blank lines and still score 1.0. The code
  under test is LLM-generated and the optimizer is actively maximizing this
  number, so a comparison that tolerates extra output is a reward-hacking
  surface -- the same class as the trailing-content hole the script already
  warned about. Missing, extra, and duplicated trailing newlines all fail now.

  Verified safe for the baseline: `main.py` writes `answer + "\n"` and every
  `expected.txt` ends in exactly one newline, so correct answers still score 1.0.

  End-of-line-only normalization is kept deliberately: `tr -d '\r'` would
  collapse `sum=4<CR>2` into `sum=42` and score a false 1.0.

* `WrappedAgent.setup` rejects symlinks before uploading anything. `upload_dir`
  follows them, so a link anywhere in a selected subtree would copy host files
  into the task container -- `.env`, an SSH key, anything the exclude list names
  but a link bypasses. The whole selection is scanned up front so a rejection
  cannot leave a half-populated /app. Covers top-level and nested links.

* `NVIDIA_INFERENCE_HUB_KEY` -> `INFERENCE_API_KEY` in the example README and
  architecture doc. The former is not read anywhere; `.env.example` and the
  credential-defaulting path in cli.py both use the latter.

Refs AALGO-312.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
…ime safe

Build reproducibility:

* Pin tau2-bench to 363133ada1936491fb5bcec33cd62c3518a99f65 in both images.
  `git clone --depth=1` cannot check out an arbitrary sha, so it silently tracked
  the default branch however the ARG was written; replaced with
  `git init` + `fetch --depth 1 <repo> <rev>` + `checkout --detach FETCH_HEAD`.

* Raise the runtime server's FastMCP floor from >=3.0 to >=3.2.0,<4. This was not
  merely unbounded: root pyproject.toml already overrides FastMCP to >=3.2.0 for
  GHSA-vv7q-7jx5-f767 (Critical) and GHSA-rww4-4w9c-7733 (High), and pip resolves
  inside the image where that override cannot reach, so the container was allowed
  to install a version the repo bans.

* Pin nooa to a commit rather than tag v0.0.6 -- tags are mutable.

* Drop the unused `requests` import, and fail fast when INFERENCE_API_KEY is
  absent instead of letting CompletionClient surface it as an auth error mid-trial,
  which reads as a model failure and costs a container run to diagnose.

* Document that the dataset registry_url is NVIDIA-internal and what external
  users need instead.

Runtime state:

* Publish the state file atomically via a same-directory temp file and
  `os.replace`. The Harbor wrapper collects it as a trial artifact while the
  conversation is still running, so a plain `write_text` let it capture a
  half-serialized document.

* Serialize MCP-exposed runtime calls behind a reentrant lock, including the
  dynamically registered domain tools. FastMCP dispatches synchronous tools on a
  thread pool and `runtime` is a module-level singleton driving one conversation,
  so concurrent calls interleaved mutations of the message history, the counters,
  and the state file. Serializing is the semantics here, not a throughput cost --
  two simultaneous turns on one conversation are already meaningless.

  The wrapper copies `__signature__` and `__annotations__` explicitly. Python 3.14
  swapped `__annotations__` for `__annotate__` in functools.WRAPPER_ASSIGNMENTS
  (PEP 649), and make_domain_tool_handler assigns `__annotations__` directly, so
  `wraps` alone produced tools whose schema had no arguments. Verified on 3.12
  (the image) and 3.14.

* Log user-tool discovery failures with domain context. A genuine tau2 API break
  was indistinguishable from "this domain has no user tools" and surfaced only as
  a quietly degraded user simulator.

Adds contract tests asserting relationships between files -- both images agree on
the revision, neither uses a shallow clone, the FastMCP floor clears the advisory
minimum, nooa is a sha -- rather than restating a pin as a literal.

Refs AALGO-312.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
… exemptions

Adds SPDX to the files that are configuration: task.toml and docker-compose.yaml
get comment headers, task_config.json gets top-level metadata fields (JSON has no
comments; Tau3Runtime indexes known keys, so extra ones are inert -- verified the
file still parses and keeps `domain`).

Deliberately NOT applied to `instruction.md` or `AGENT-SPEC.md`, which CodeRabbit
asked for. Both are upstream tau2-bench benchmark content, not source:

* Harbor reads `instruction.md` verbatim into the agent prompt, so a header would
  prepend licence boilerplate to every prompt -- the same failure mode CodeRabbit
  itself identified for the hello example's `expected.txt` fixture.
* `AGENT-SPEC.md` duplicates the same airline policy and is read by the Coder as a
  contract, so the two must stay byte-identical to each other.

For the same reason the flagged grammar ("the user is should be refunded the
difference") is left alone: the sentence appears in *both* files, so it is upstream
policy text. Correcting it would fork the benchmark and make scores incomparable
with published tau2 results, for no behavioural gain.

A README section records this so the next linter pass does not re-open it.

Refs AALGO-312.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
…aseline task set

Follow-up to the CodeRabbit pass on 6a7ef47.

* SPDX headers added to both Tau2 Dockerfiles and the task-template
  `tests/test.sh` (a 0-byte placeholder; two comments still exit 0). These were
  simply missed when the other infrastructure files were done.

* SPDX added to `AGENT-SPEC.md` after all. The earlier commit declined it on the
  grounds that the file must stay in step with `instruction.md`'s policy text --
  but the header is an HTML comment *above* the policy body, and
  `instruction.md` wraps the same policy in its own `<policy>` block, so nothing
  desynchronises. The decline was wrong; the header is additive.

  `instruction.md` itself still has none, and that part stands: Harbor reads it
  verbatim into the agent prompt, so a header would prepend licence boilerplate
  to every prompt.

* `test_every_split_keeps_one_passing_and_one_failing_task` now asserts the
  split's on-disk task directories match `_EXPECTED_BASELINE` before scoring.
  It previously iterated the expectation itself, so adding or removing a dataset
  task left every assertion passing while the split silently lost the
  one-pass/one-failure shape the README documents.

Refs AALGO-312.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
@ngoncharenko
ngoncharenko force-pushed the ngoncharenko/aalgo-312-wire-eval-into-optimizer branch from 503ca31 to 20813e3 Compare July 31, 2026 04:30

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/harbor_runtime.py`:
- Around line 613-617: Update the task digest construction in the loop over
_task_dirs_for to pass each non-None task_dir through the existing _safe_resolve
helper instead of calling task_dir.resolve() directly. Preserve the
"<unresolved>" value for missing directories and ensure resolution failures
degrade to the safe fallback without raising from cache-stamp generation.

In `@plugins/nemo-experimentalist/docs/architecture.md`:
- Around line 444-446: Update the aggregate_metrics description in the
architecture documentation to state that each key’s mean is computed only over
trials whose status is exactly "completed", replacing the broader "non-failed
trials" wording. Preserve the existing explanation of multi-key Pareto
comparison.
🪄 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: 1cdc5533-a21e-4e65-92c9-92f461400763

📥 Commits

Reviewing files that changed from the base of the PR and between 503ca31 and 20813e3.

⛔ Files ignored due to path filters (2)
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/harbor_runtime.py is excluded by !sdk/**
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (85)
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/harbor_runtime.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime_e2e.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/config.yaml
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/run.py
  • plugins/nemo-eval-author/tests/test_eval_author_run.py
  • plugins/nemo-experimentalist/README.md
  • plugins/nemo-experimentalist/docs/architecture.md
  • plugins/nemo-experimentalist/docs/e2e/README.md
  • plugins/nemo-experimentalist/docs/e2e/experiment-debug-round.yaml
  • plugins/nemo-experimentalist/docs/e2e/experiment-eval-only-sdk.yaml
  • plugins/nemo-experimentalist/docs/e2e/experiment-eval-only.yaml
  • plugins/nemo-experimentalist/docs/e2e/experiment-fast.yaml
  • plugins/nemo-experimentalist/docs/e2e/run-eval-only.py
  • plugins/nemo-experimentalist/examples/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/.env.example
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/AGENT-SPEC.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/agent.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/instruction.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/harbor_wrapper.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/main.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/optimizer.yaml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/tracing.py
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/.env.example
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/AGENT-SPEC.md
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/README.md
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/agent.py
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/Dockerfile
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/docker-compose.yaml
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/runtime-server/Dockerfile
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/runtime-server/server.py
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/runtime-server/task_config.json
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/instruction.md
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/task.toml
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/tests/test.sh
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/harbor_wrapper.py
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/main.py
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/optimizer.yaml
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/pyproject.toml
  • plugins/nemo-experimentalist/pyproject.toml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/base.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/factory.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor_evaluator.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/deps.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/run.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/preflight.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/resolve.py
  • plugins/nemo-experimentalist/tests/experimentalist/conftest.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_dataset_staging_loop.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_factory.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor_ab_e2e.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor_evaluator.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_hello_example_baseline.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_loop_failure.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_loop_insight_suite.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_tau2_example_contract.py
  • plugins/nemo-experimentalist/tests/test_deps.py
  • plugins/nemo-experimentalist/tests/test_e2e_helper_defaults.py
🚧 Files skipped from review as they are similar to previous changes (60)
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/expected.txt
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/run.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/preflight.py
  • plugins/nemo-experimentalist/pyproject.toml
  • plugins/nemo-experimentalist/tests/experimentalist/test_loop_failure.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/task.toml
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/.env.example
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/pyproject.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/README.md
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/main.py
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/runtime-server/task_config.json
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/task.toml
  • plugins/nemo-experimentalist/tests/experimentalist/test_tau2_example_contract.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_dataset_staging_loop.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/factory.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/task.toml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/resolve.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/sum-two/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/task.toml
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/test.sh
  • plugins/nemo-experimentalist/docs/e2e/experiment-fast.yaml
  • plugins/nemo-experimentalist/tests/experimentalist/conftest.py
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/optimizer.yaml
  • plugins/nemo-experimentalist/docs/e2e/experiment-eval-only.yaml
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/task-template/tests/test.sh
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/tests/expected.txt
  • plugins/nemo-experimentalist/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/main.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/agent.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/config.yaml
  • plugins/nemo-experimentalist/tests/test_deps.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/optimizer.yaml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/task.toml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/train/greet-world/tests/expected.txt
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/.env.example
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/sum-three/tests/test.sh
  • plugins/nemo-experimentalist/tests/experimentalist/test_hello_example_baseline.py
  • plugins/nemo-eval-author/tests/test_eval_author_run.py
  • plugins/nemo-experimentalist/tests/test_e2e_helper_defaults.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime_e2e.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/deps.py
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/harbor_wrapper.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/run.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_loop_insight_suite.py
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/README.md
  • plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_factory.py
  • plugins/nemo-experimentalist/examples/tau2-nooa-oo-agent/dataset/template/task_template/environment/docker-compose.yaml
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_harbor_runtime.py
  • plugins/nemo-experimentalist/docs/e2e/experiment-eval-only-sdk.yaml
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/AGENT-SPEC.md
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor.py
  • plugins/nemo-experimentalist/docs/e2e/experiment-debug-round.yaml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/base.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/harbor_evaluator.py
  • plugins/nemo-experimentalist/docs/e2e/README.md
  • plugins/nemo-experimentalist/examples/hello-harbor-agent/dataset/validation/greet-universe/tests/test.sh

Comment thread plugins/nemo-experimentalist/docs/architecture.md Outdated
architecture.md described `aggregate_metrics` as a mean "over non-failed trials",
contradicting section 4 two hundred lines earlier, which documents the actual
contract: only `status == "completed"` trials are averaged.

The wording matters beyond consistency. `base.py` uses the positive predicate
deliberately -- `!= "failed"` is equivalent only while TrialStatus is
Literal["completed", "failed"], and would silently start averaging any third
status someone adds. The doc line was the opt-out phrasing that comment warns
against, so a reader adding a status would have read it as scoreable.

Refs AALGO-312.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
…t OSError

CodeRabbit flagged a bare `task_dir.resolve()` in `_cache_stamp` and suggested
routing it through `_safe_resolve` like `_task_dirs_for` does. Investigating that
turned up a larger problem: `_safe_resolve` would not have fixed it.

On CPython 3.12 -- the floor this package targets -- `Path.resolve()` translates
ELOOP into `RuntimeError("Symlink loop from ...")`, which is not an `OSError`.
`_safe_resolve` caught only `OSError`, so a symlink loop propagated straight
through the very helper written to absorb it, killing a run over a best-effort
cache fingerprint. It raises deterministically, not as a race. Verified across
interpreters: 3.12 raises RuntimeError; 3.14 resolves a loop without raising at
all, so the hazard is live only on the supported range.

The loop is reached through `_digest_directory`'s walk, which already routes every
path through `_safe_resolve` -- so this was live regardless of the flagged line.

`_cache_stamp`'s three resolve calls now use `_safe_resolve` too. Being precise
about why: that is consistency, not a demonstrated fix. `_task_dirs_for` filters
candidates with `is_dir()`, which returns False for a loop, so a looping path
never reaches them. Reverting those three lines alone does not fail the new test;
reverting the exception set does.

Regression test uses a self-referential symlink and asserts the stamp still
fingerprints the task. Mutation-checked against CodeRabbit's exact suggestion
(`_safe_resolve` with `except OSError`), which still fails.

Refs AALGO-312.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
# Positive predicate on purpose: `!= "failed"` is equivalent while TrialStatus
# is Literal["completed", "failed"], but it would silently start averaging any
# third status someone adds. Opt statuses in, do not opt "failed" out.
completed = [r for r in results if r.status == "completed"]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • This does not change current behavior in main besides making it more defensive. There are two values in the TrialStatus: TrialStatus: TypeAlias = Literal["completed", "failed"] , so status != "failed" to status == "completed" are equivalent currently. If we add a 3rd value in the future to TrialStatus like timeout , that’s when behavior will differ. In my view, we don’t want timeout error, for example, affect the real scores aggregation and drag it down - does not seem right to me.

  • The comment in main was out of date and didn’t reflect actual code behavior:
    https://github.com/NVIDIA-NeMo/nemo-platform/blob/main/plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/evaluator/base.py#L80 - this is aggregation in main. So it it didn’t count failures towards aggregation

completed = [r for r in results if r.status != "failed"]   # failures filtered OUT
....
aggregate_metrics[metric_name] = aggregate_metrics[metric_name] / len(completed)   # NOT len(results)

…2-wire-eval-into-optimizer

Signed-off-by: Alec Khoury <akhoury@nvidia.com>
Retarget the realistic example path to Nico's Tau3 setup, keep hello as the
deterministic Harbor harness, and remove the obsolete docs/e2e configs.

Signed-off-by: Alec Khoury <akhoury@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants