Skip to content

feat(evaluator): promote generic agentic runtimes into the Agent-Eval SDK - #383

Merged
arpitsardhana merged 1 commit into
mainfrom
aalgo-258-nat-runner/arpsingh
Jun 23, 2026
Merged

feat(evaluator): promote generic agentic runtimes into the Agent-Eval SDK#383
arpitsardhana merged 1 commit into
mainfrom
aalgo-258-nat-runner/arpsingh

Conversation

@arpitsardhana

@arpitsardhana arpitsardhana commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Promotes a small set of generic agent-evaluation primitives into the Agent-Eval SDK, and ships a runnable run_agent_eval example that drives the full build env → run agent → verify → score → gate → persist flow end to end. The SDK stays deliberately lean — it owns generating trials and scoring them; CI policy (the pass/fail gate), the pipeline wrapper, the Harbor-style verifier, the run layout, and all Docker/build glue live in the example.

Agent-Eval SDK (nemo_evaluator_sdk/agent_eval/)

Three focused additions, no speculative surface:

  • runtimes/environment.py — a pluggable agent-execution seam that sits below AgentTaskRunner: AgentEnvironmentProvider / AgentEnvironmentHandle protocols, an AbstractEnvironmentHandle that routes run_agent / run_verifier through a single run(spec, role), the EnvRunSpec / EnvCommandResult value types, and a Docker reference implementation (DockerEnvironmentProvider / DockerEnvironmentHandle, stdlib-subprocess docker run with secret redaction).
  • metrics.py — reusable scorers plus the measurement contract: AgentPhaseSuccessMetric (reads the agent-phase outcome) and EvidencePresenceMetric (scores over candidate.evidence on disk rather than a stamped reward — the SDK's metric-over-evidence value-add), and TrialMeasurements, the typed projection of the token/runtime/reward keys producers write onto trial.metadata.
  • trials.py (extended) — adds the AgentTrialSerde protocol (the offline counterpart to AgentTaskRunner, so prior runs can be re-scored), and the runtime-agnostic trial-shaping helpers resolve_trial_status and standard_evidence_descriptors (the documented evidence-key builder).
  • Vendored SDK copies (beta/evaluator/agent_eval/) regenerated to match.

run_agent_eval example (nemo_evaluator_sdk/examples/run_agent_eval/)

A self-contained example that drives agent-eval tasks through the SDK with two paths:

  • Toy path (default) — mini_agent.py runs as a host subprocess via workflow_runtime.py (an AgentTaskRunner + TrialJsonSerde + example tasks/metric), so it runs end to end with no external infrastructure.
  • Real path (--agentic-task <name>) — runs an actual tests/agentic-use task through Docker with two backends:
    • workflow (platform_runtime.py) — task-local nat run of workflow.yml, with BUILD (image) / AGENT (nat run) / VERIFY (pytest) phases.
    • aut (aut_runtime.py) — create → seed inference providers → deploy → health-check → invoke a deployed agent-under-test.
  • Example-local CI/backend glue: build_spec.py (environment.yamlBuildPlan, with a Dockerfile escape hatch), layout.py (run-dir scaffold), verify.py (Harbor reward.txt convention), gating.py (evaluate_gate / GateThresholdsgate.json), pipeline.py (online run + offline rescore + gate wrapper), and usage.py (token/runtime extraction onto each trial).
  • run_agent_eval.py is the CLI harness (online run, offline rescore, gate); ships a task-capable aut_agent.workspace.example.yml matched to workspace-basic-mcp (text-based ReAct so it works across whichever backend the gateway routes to).

Tests (tests/agent_eval/ mirrors the SDK package 1:1)

  • test_environment.py, test_metrics.py, and test_trials.py (incl. the trial-shaping helpers) cover the new SDK code; test_import_hygiene.py keeps agent_eval/ free of NeMo-Platform imports.
  • Tests that exercised example-local code rather than the SDK were removed from the SDK suite, including the now example-coupled test_profbench.py.

Test plan

  • uv run pytest packages/nemo_evaluator_sdk/tests/agent_eval/ → green (55 passed)

  • uv run ruff check / ruff format --check / uv run ty check on the new code → clean

  • make vendor → SDK mirror in sync (no drift)

  • run_agent_eval toy path (default mini_agent, no external infra) end to end, plus offline rescore of the same bundle:

    tasks: 2  trials: 2
    agent_phase_success.agent_phase_success: 2/2 true
    evidence_presence.evidence_present: 2/2 true
    output_contains.output_contains: 2/2 true
    gate.json → gate_passed: true, pass_rate=1.000
    
  • run_agent_eval real path (AUT backend) verified end to end on workspace-basic-mcp:

    agent_phase_success: 1/1 true
    agentic_use_verifier_reward.verifier_reward: mean=1.000
    runtime_sec: 87.9
    gate.json → gate_passed: true, pass_rate=1.000
    

@github-actions github-actions Bot added the feat label Jun 20, 2026
@arpitsardhana
arpitsardhana marked this pull request as ready for review June 20, 2026 21:20
@arpitsardhana
arpitsardhana requested review from a team as code owners June 20, 2026 21:20
@arpitsardhana
arpitsardhana force-pushed the aalgo-258-nat-runner/arpsingh branch from dd5d412 to e01b23b Compare June 22, 2026 06:36
@arpitsardhana

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@coderabbitai

coderabbitai Bot commented Jun 22, 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

Adds a complete run_agent_eval example package to nemo_evaluator_sdk with two runtime backends (subprocess workflow and AUT/platform Docker), a gating module with regression checks, token-usage extraction, verifier phase support, Docker environment abstractions, a build-spec toolchain, and a runnable CLI entrypoint. Core SDK (trials.py, metrics.py) gains trial serde, status helpers, and reusable metrics.

Agent-Eval SDK Pipeline and Runtime Stack

Layer / File(s) Summary
Core trial contracts: measurements, serde, metrics, trial helpers
src/.../agent_eval/trials.py, src/.../agent_eval/metrics.py, tests/agent_eval/test_trials.py, tests/agent_eval/test_metrics.py
AgentTrialSerde protocol, resolve_trial_status, standard_evidence_descriptors, AgentPhaseSuccessMetric, EvidencePresenceMetric, and TrialMeasurements with bool-safe coercions. Tests validate all helpers and metric scoring.
Gating, summarization, and regression checks
examples/run_agent_eval/gating.py
GateThresholds/GateCheck/GateReport dataclasses, summarize_run aggregation, run_gate_checks enforcing pass-rate/token/runtime thresholds and baseline regression, reward extraction, and write_gate_report/load_baseline_summary I/O.
AgentEvalPipeline: online and offline evaluation
examples/run_agent_eval/pipeline.py
PipelineConfig and AgentEvalPipeline wrapping AgentEvaluator: run_tasks (with prepare_task hook and extra_metrics injection), score_trials offline path, and conditional gate reporting.
Docker runtime primitives
src/.../agent_eval/runtimes/environment.py, examples/run_agent_eval/layout.py, examples/run_agent_eval/verify.py, examples/run_agent_eval/build_spec.py, tests/agent_eval/test_environment.py, tests/agent_eval/test_import_hygiene.py
EnvRunSpec/EnvCommandResult/AgentEnvironmentHandle/AgentEnvironmentProvider with DockerEnvironmentHandle/DockerEnvironmentProvider; RunLayout filesystem helpers; VerifierOutcome collection; BuildSpec/BuildPlan/execute_build_plan toolchain with injection-safety guards. Tests cover routing, timeouts, tag injection, and import hygiene.
Token-usage extraction
examples/run_agent_eval/usage.py
TokenMetrics TypedDict, iter_agent_log_json_payloads, agent_log_has_workflow_error, and extract_usage_metrics aggregating prompt/completion/cache tokens and duration across multiple key-shape layouts.
WorkflowAgentRuntime and example tasks
examples/run_agent_eval/workflow_runtime.py, examples/run_agent_eval/mini_agent.py
WorkflowAgentRuntime running tasks via subprocess with placeholder substitution and timeout; TrialJsonSerde/load_stored_trials for offline loading; OutputContainsMetric; example_tasks/tasks_by_id; mini_agent.py toy CLI agent.
NatWorkflowRuntime (platform backend)
examples/run_agent_eval/platform_runtime.py
NatWorkflowConfig/AgenticRunLayout, VerifierRewardMetric, agentic_task_from_dir, workflow YAML rewriting, container env/mount assembly, pytest verifier spec, trial construction from on-disk artifacts, and run_agent_then_verify orchestration.
NatAutRuntime (AUT backend)
examples/run_agent_eval/aut_runtime.py, examples/run_agent_eval/aut_agent.workspace.example.yml
AutConfig dataclass, build_aut_agent_cmd bash -c script for agent create/deploy/invoke/undeploy lifecycle, prepare_aut_config_for_runtime YAML rewriting, and NatAutRuntime per-task env/mount assembly. Includes example AUT YAML for workspace-basic-mcp.
CLI entrypoint and documentation
examples/run_agent_eval/run_agent_eval.py, examples/run_agent_eval/README.md, examples/run_agent_eval/.gitignore
run_online/run_agentic_task/rescore async functions, backend routing between NatWorkflowRuntime and NatAutRuntime, result/measurement printing, full CLI argument wiring, README with execution flow and commands, and .gitignore for output directory.

Suggested reviewers

  • ngoncharenko
  • SandyChapman
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 31.85% 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
Title check ✅ Passed The title clearly and concisely identifies the main change: promoting generic agentic runtimes into the SDK. It accurately reflects the comprehensive additions documented in the PR summary.
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch aalgo-258-nat-runner/arpsingh

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: 10

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/examples/run_agent_eval/README.md (1)

1-227: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift

Split this page by Diataxis and move prerequisites to the top.

This page mixes tutorial/how-to/explanation/reference, places prerequisites late, and omits a “Next Steps” section. Restructure into single-quadrant pages with cross-links.

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", "Always list prerequisites at the top of documentation pages before other content", and "Include 'Next Steps' section at the end with cross-links to related documentation content."

🤖 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/examples/run_agent_eval/README.md` around lines 1
- 227, The README.md mixes tutorial content (Run it), how-to guides (Plugging in
a real workflow, Running a real tests/agentic-use task), explanation (What it
demonstrates, Execution flow), and reference material (Files table) without
clear separation, and places prerequisites late in the document. Reorganize the
content by moving the prerequisites section to the very top before "Run it",
create distinct sections aligned to Diataxis quadrants (Tutorial for "Run it",
How-To for the real workflow sections, Explanation for "What it demonstrates"
and execution flow, Reference for "Files" and configuration tables), and add a
"Next Steps" section at the end with cross-links to related documentation. Each
reorganized section should stand independently and reference other sections via
cross-links rather than embedding mixed content types together.

Source: Coding guidelines

🧹 Nitpick comments (5)
packages/nemo_evaluator_sdk/examples/run_agent_eval/aut_runtime.py (2)

169-169: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Import inside function body.

inject_gateway_url is imported inside prepare_aut_config_for_runtime. Move to module-level for consistency with coding guidelines (prefer concrete imports, not deferred).

Proposed fix
 from pathlib import Path
 
 import yaml
+from nemo_agents_plugin.utils import inject_gateway_url
 from nemo_evaluator_sdk.agent_eval.runtimes.environment import AgentEnvironmentProvider, EnvRunSpec

Then remove line 169.

🤖 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/examples/run_agent_eval/aut_runtime.py` at line
169, The import statement for `inject_gateway_url` from
`nemo_agents_plugin.utils` is currently inside the
`prepare_aut_config_for_runtime` function body at line 169. Move this import to
the module-level imports at the top of the file with the other import
statements, then remove the import statement from inside the function body.

Source: Coding guidelines


72-75: 🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff

API key interpolation may break on special characters.

The inline Python script at line 73 replaces ${NVIDIA_API_KEY} and ${ANTHROPIC_API_KEY} placeholders via simple string replacement. If an API key contains quotes or backslashes, the resulting YAML could be malformed.

Consider using proper YAML serialization instead of text replacement.

🤖 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/examples/run_agent_eval/aut_runtime.py` around
lines 72 - 75, The API key interpolation in the inline Python script uses simple
string replacement with the replace() method for NVIDIA_API_KEY and
ANTHROPIC_API_KEY placeholders. This approach is fragile and will break if API
keys contain special characters like quotes or backslashes, potentially creating
malformed YAML. Instead of using str.replace() to substitute these placeholders,
load the YAML file using a YAML library like PyYAML, update the parsed
dictionary with the actual API key values, and then serialize it back to YAML
format. This ensures proper escaping and quoting of special characters in the
API keys.
packages/nemo_evaluator_sdk/examples/run_agent_eval/platform_runtime.py (2)

253-264: 🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff

Brittle string replacement for workflow rewriting.

text.replace("http://localhost:8080", ...) and model name replacement assume exact string matches. If the source YAML has different formatting (e.g., quotes, trailing slashes), replacement silently fails.

Consider parsing YAML first, modifying the data structure, then serializing—which you already do later for the tracing config (line 266+).

🤖 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/examples/run_agent_eval/platform_runtime.py`
around lines 253 - 264, The current implementation uses brittle string
replacements (for "http://localhost:8080", model name, and MCP client function
groups) that will silently fail if the YAML has different formatting like quotes
or spacing. Replace these string-based replacements with a proper YAML parsing
approach: parse the workflow_path content using a YAML parser to convert it to a
Python dictionary, modify the relevant fields in the data structure
(nmp_base_url under appropriate keys, model_name field, and function_groups vs
functions), then serialize the modified structure back to YAML text. This
approach is more robust and maintainable, and aligns with how the tracing config
is already handled later in the function.

546-554: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

Sequential task execution limits throughput.

run_tasks processes tasks sequentially without parallelism. Unlike WorkflowAgentRuntime which uses asyncio.Semaphore, this awaits each task serially. If intentional (e.g., Docker resource constraints), consider documenting it; otherwise, add concurrency control.

🤖 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/examples/run_agent_eval/platform_runtime.py`
around lines 546 - 554, The run_tasks method processes tasks sequentially using
a simple for loop with await, which limits throughput. Refactor the task
execution to use concurrent processing with asyncio.Semaphore (similar to
WorkflowAgentRuntime) to allow multiple tasks to run in parallel while still
respecting resource constraints. Replace the sequential loop that awaits
self._run_task for each task with either asyncio.gather or asyncio.TaskGroup
along with a Semaphore to control concurrency. Alternatively, if sequential
execution is intentional due to constraints like Docker resource limitations,
add a docstring comment to the run_tasks method documenting this design
decision.
packages/nemo_evaluator_sdk/tests/agent_eval/test_import_hygiene.py (1)

20-20: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Scan all namespace roots, not just the first path entry.

Line 20 only checks one agent_eval.__path__ entry. In split namespace installs, this can miss offenders.

Suggested fix
-AGENT_EVAL_ROOT = Path(next(iter(agent_eval.__path__))).resolve()
+AGENT_EVAL_ROOTS = [Path(p).resolve() for p in agent_eval.__path__]
@@
-    for path in sorted(AGENT_EVAL_ROOT.rglob("*.py")):
-        text = path.read_text(encoding="utf-8")
-        for match in _FORBIDDEN.finditer(text):
-            line_no = text.count("\n", 0, match.start()) + 1
-            offenders.append(f"{path.relative_to(AGENT_EVAL_ROOT)}:{line_no}: {match.group(0).strip()}")
+    for root in AGENT_EVAL_ROOTS:
+        for path in sorted(root.rglob("*.py")):
+            text = path.read_text(encoding="utf-8")
+            for match in _FORBIDDEN.finditer(text):
+                line_no = text.count("\n", 0, match.start()) + 1
+                offenders.append(f"{path.relative_to(root)}:{line_no}: {match.group(0).strip()}")

Also applies to: 32-37

🤖 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_import_hygiene.py` at line
20, The current implementation using next(iter(agent_eval.__path__)) only
examines the first path entry in the namespace, which causes missed violations
in split namespace installations. Instead of extracting a single
AGENT_EVAL_ROOT, refactor the code to iterate through all entries in
agent_eval.__path__ and check each namespace root for import hygiene violations.
Apply this same pattern to both the initial AGENT_EVAL_ROOT assignment on line
20 and the related code section mentioned in lines 32-37.
🤖 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/examples/run_agent_eval/README.md`:
- Around line 53-61: The markdown file has two linting issues that need to be
fixed. First, add a language identifier (such as json or text) after the opening
backticks for the fenced code blocks at lines 53 and 178 that show the agent
evaluation output and configuration examples respectively. Second, locate the
blockquote section around line 194 and remove the blank line that appears within
or between blockquote elements, as markdown linting requires proper blockquote
spacing without isolated blank lines.

In `@packages/nemo_evaluator_sdk/examples/run_agent_eval/run_agent_eval.py`:
- Around line 107-110: The validation for requiring --aut-agent-name when
--backend aut is specified is happening at runtime in the conditional block with
NatAutRuntime instantiation, causing an unhandled ValueError. Move this
validation to the argument parser setup using a custom validation method or
action that checks the mutually dependent arguments (backend and aut_agent_name)
during parse time rather than after, so that argparse can handle the error with
a proper controlled exit message instead of a traceback.

In
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/common_metrics.py`:
- Around line 40-41: The bool() coercion on the metadata.get("agent_ok") value
at line 40 treats any non-empty string (including "false") as True, which
incorrectly marks failed trials as passed. Replace the truthy coercion with
proper boolean conversion that correctly handles string representations of
booleans. Instead of bool(input.candidate.metadata.get("agent_ok")), implement
logic that checks if the value is explicitly True or a string "true"
(case-insensitive), returning False for any other value including the string
"false" or absence of the key.

In `@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/gating.py`:
- Around line 422-429: The `_aggregate_provenance` function attempts to add
provenance field values directly to sets using `.add(value)`, which will raise a
TypeError when the value is an unhashable type like a list or dict. Modify the
code where `observed[field_name].add(value)` is called to handle unhashable
values by either converting them to hashable equivalents (e.g., converting lists
to tuples, dicts to frozensets or their string representations) or wrapping the
add operation in a try-except block to gracefully skip unhashable values. This
ensures the aggregation can complete successfully without aborting summarization
and gate report generation.

In
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/build_spec.py`:
- Around line 169-170: The code at the dependency injection point in the RUN pip
install command is vulnerable to shell command injection because the dependency
strings from spec.python_dependencies are joined and directly interpolated into
a shell command without proper escaping. To fix this, properly escape or quote
each individual dependency value from spec.python_dependencies before joining
them into the deps string, ensuring that any shell metacharacters in the
dependency names cannot be interpreted as commands during Docker image build.
Alternatively, use the exec form of the RUN instruction with a JSON array syntax
to bypass shell interpretation entirely, passing each dependency as a separate
list element.
- Around line 90-98: The python_dependencies parsing in the BuildSpec creation
does not validate that python_deps is actually a list before converting it with
list(). When python_deps is a string like "pytest", list(python_deps) converts
it to individual characters instead of a list containing the single dependency.
Add validation to ensure python_deps is a list before using it in the BuildSpec
constructor. If python_deps is not a list (such as when it's a string), either
raise a validation error to reject the invalid YAML structure or handle the
string case appropriately to produce correct dependency lists. This ensures only
properly formatted dependencies are accepted.

In
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/docker.py`:
- Around line 18-32: The redact_cmd_for_logging function only redacts values in
the KEY=value format but misses sensitive flag patterns like --token secret or
--password secret where the secret value is a separate argument. Enhance the
function to detect sensitive flags (tokens containing KEY, TOKEN, SECRET, or
PASSWORD markers) even without the equals sign, and when such a flag is found,
redact the following token in the command sequence. This will require iterating
through the cmd sequence with index access so you can both identify sensitive
flags and mark the subsequent argument as redacted.

In
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/environment.py`:
- Around line 27-29: The default_image_tag function uses the raw task_id
parameter directly without sanitization, but Docker image tags have character
restrictions and invalid characters will cause image build and runtime failures.
Sanitize the task_id before constructing the tag by converting it to lowercase,
replacing or removing invalid Docker tag characters (such as spaces and special
characters that are not alphanumeric, hyphens, underscores, or periods),
ensuring the resulting tag conforms to Docker naming conventions.

In
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/generalized_agent.py`:
- Around line 124-136: The subprocess created by `self._process_factory` is not
explicitly killed when the `asyncio.wait_for` call times out, which can leave
zombie processes running. Add a finally block after the try-except to ensure the
subprocess is properly terminated by calling `process.kill()` regardless of
whether the operation succeeds or fails. Alternatively, add explicit cleanup in
the exception handler to terminate the process before returning from the
`_failed_trial` call, ensuring the process object has been created before
attempting to kill it.

In
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/verify.py`:
- Around line 51-63: The code creates an inconsistency where the reward value
(read from reward_path file) can contradict the passed status (derived from the
ok variable). When reward.txt contains "1" but ok is False, the metadata becomes
internally inconsistent with passed=False but reward=1. To fix this, ensure that
the reward and passed values are always consistent by having the reward value
determine or validate the passed status, rather than allowing them to diverge.
Apply this consistency check throughout the verification logic, including all
code sections mentioned (lines 51-63 and also applies to 76-84) where this
pattern occurs.

---

Outside diff comments:
In `@packages/nemo_evaluator_sdk/examples/run_agent_eval/README.md`:
- Around line 1-227: The README.md mixes tutorial content (Run it), how-to
guides (Plugging in a real workflow, Running a real tests/agentic-use task),
explanation (What it demonstrates, Execution flow), and reference material
(Files table) without clear separation, and places prerequisites late in the
document. Reorganize the content by moving the prerequisites section to the very
top before "Run it", create distinct sections aligned to Diataxis quadrants
(Tutorial for "Run it", How-To for the real workflow sections, Explanation for
"What it demonstrates" and execution flow, Reference for "Files" and
configuration tables), and add a "Next Steps" section at the end with
cross-links to related documentation. Each reorganized section should stand
independently and reference other sections via cross-links rather than embedding
mixed content types together.

---

Nitpick comments:
In `@packages/nemo_evaluator_sdk/examples/run_agent_eval/aut_runtime.py`:
- Line 169: The import statement for `inject_gateway_url` from
`nemo_agents_plugin.utils` is currently inside the
`prepare_aut_config_for_runtime` function body at line 169. Move this import to
the module-level imports at the top of the file with the other import
statements, then remove the import statement from inside the function body.
- Around line 72-75: The API key interpolation in the inline Python script uses
simple string replacement with the replace() method for NVIDIA_API_KEY and
ANTHROPIC_API_KEY placeholders. This approach is fragile and will break if API
keys contain special characters like quotes or backslashes, potentially creating
malformed YAML. Instead of using str.replace() to substitute these placeholders,
load the YAML file using a YAML library like PyYAML, update the parsed
dictionary with the actual API key values, and then serialize it back to YAML
format. This ensures proper escaping and quoting of special characters in the
API keys.

In `@packages/nemo_evaluator_sdk/examples/run_agent_eval/platform_runtime.py`:
- Around line 253-264: The current implementation uses brittle string
replacements (for "http://localhost:8080", model name, and MCP client function
groups) that will silently fail if the YAML has different formatting like quotes
or spacing. Replace these string-based replacements with a proper YAML parsing
approach: parse the workflow_path content using a YAML parser to convert it to a
Python dictionary, modify the relevant fields in the data structure
(nmp_base_url under appropriate keys, model_name field, and function_groups vs
functions), then serialize the modified structure back to YAML text. This
approach is more robust and maintainable, and aligns with how the tracing config
is already handled later in the function.
- Around line 546-554: The run_tasks method processes tasks sequentially using a
simple for loop with await, which limits throughput. Refactor the task execution
to use concurrent processing with asyncio.Semaphore (similar to
WorkflowAgentRuntime) to allow multiple tasks to run in parallel while still
respecting resource constraints. Replace the sequential loop that awaits
self._run_task for each task with either asyncio.gather or asyncio.TaskGroup
along with a Semaphore to control concurrency. Alternatively, if sequential
execution is intentional due to constraints like Docker resource limitations,
add a docstring comment to the run_tasks method documenting this design
decision.

In `@packages/nemo_evaluator_sdk/tests/agent_eval/test_import_hygiene.py`:
- Line 20: The current implementation using next(iter(agent_eval.__path__)) only
examines the first path entry in the namespace, which causes missed violations
in split namespace installations. Instead of extracting a single
AGENT_EVAL_ROOT, refactor the code to iterate through all entries in
agent_eval.__path__ and check each namespace root for import hygiene violations.
Apply this same pattern to both the initial AGENT_EVAL_ROOT assignment on line
20 and the related code section mentioned in lines 32-37.
🪄 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: 99bd2b0a-e2dd-4984-9347-897b60511c0e

📥 Commits

Reviewing files that changed from the base of the PR and between 68f9512 and 3ba0a99.

⛔ Files ignored due to path filters (12)
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/common_metrics.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/gating.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/measurements.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/pipeline.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/build_spec.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/docker.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/environment.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/generalized_agent.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/layout.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/verify.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/trial_artifacts.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/trials.py is excluded by !sdk/**
📒 Files selected for processing (30)
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/.gitignore
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/README.md
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/aut_agent.workspace.example.yml
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/aut_runtime.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/mini_agent.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/platform_runtime.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/run_agent_eval.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/usage.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/workflow_runtime.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/common_metrics.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/gating.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/measurements.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/pipeline.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/build_spec.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/docker.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/environment.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/generalized_agent.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/layout.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/verify.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/trial_artifacts.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/trials.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_common_metrics.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_environment.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_gating.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_generalized_agent.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_import_hygiene.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_measurements.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_pipeline.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_trial_artifacts.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_verify.py

Comment thread packages/nemo_evaluator_sdk/examples/run_agent_eval/README.md Outdated
Comment thread packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/common_metrics.py Outdated
Comment thread packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/gating.py Outdated
Comment thread packages/nemo_evaluator_sdk/examples/run_agent_eval/build_spec.py
Comment thread packages/nemo_evaluator_sdk/examples/run_agent_eval/build_spec.py Outdated
Comment thread packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/docker.py Outdated
Comment thread packages/nemo_evaluator_sdk/examples/run_agent_eval/verify.py
Comment thread packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/trial_artifacts.py Outdated
Comment thread packages/nemo_evaluator_sdk/examples/run_agent_eval/gating.py
Comment thread packages/nemo_evaluator_sdk/examples/run_agent_eval/gating.py
Comment thread packages/nemo_evaluator_sdk/examples/run_agent_eval/pipeline.py
Comment thread packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/measurements.py Outdated
Comment thread packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/verify.py Outdated
Comment thread packages/nemo_evaluator_sdk/examples/run_agent_eval/layout.py
Comment thread packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/gating.py Outdated
Base automatically changed from aalgo-272-profbench/arpsingh to main June 22, 2026 19:41
@arpitsardhana
arpitsardhana force-pushed the aalgo-258-nat-runner/arpsingh branch from 9d6381d to 6ec07e2 Compare June 22, 2026 20:42

@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

🤖 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/examples/run_agent_eval/workflow_runtime.py`:
- Around line 98-107: The subprocess created by create_subprocess_exec at line
99 is not being terminated when asyncio.wait_for times out at line 104, allowing
orphaned processes to continue running. Modify the exception handling to
specifically catch asyncio.TimeoutError before the general Exception handler,
and when a timeout occurs, call process.kill() on the subprocess object to
ensure it is terminated before returning the failed trial from line 107. This
ensures proper cleanup of the subprocess on timeout, similar to the fix
implemented in generalized_agent.py.
🪄 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: 9afab548-cd2c-4b8d-b87c-e004cf2981b2

📥 Commits

Reviewing files that changed from the base of the PR and between 3ba0a99 and 6ec07e2.

⛔ Files ignored due to path filters (12)
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/common_metrics.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/gating.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/measurements.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/pipeline.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/build_spec.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/docker.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/environment.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/generalized_agent.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/layout.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/verify.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/trial_artifacts.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/trials.py is excluded by !sdk/**
📒 Files selected for processing (30)
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/.gitignore
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/README.md
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/aut_agent.workspace.example.yml
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/aut_runtime.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/mini_agent.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/platform_runtime.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/run_agent_eval.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/usage.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/workflow_runtime.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/common_metrics.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/gating.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/measurements.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/pipeline.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/build_spec.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/docker.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/environment.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/generalized_agent.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/layout.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/verify.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/trial_artifacts.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/trials.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_common_metrics.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_environment.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_gating.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_generalized_agent.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_import_hygiene.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_measurements.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_pipeline.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_trial_artifacts.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_verify.py
✅ Files skipped from review due to trivial changes (6)
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/README.md
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/aut_agent.workspace.example.yml
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_common_metrics.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_trial_artifacts.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/.gitignore
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/pipeline.py
🚧 Files skipped from review as they are similar to previous changes (17)
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_import_hygiene.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/trial_artifacts.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/measurements.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_verify.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/layout.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_generalized_agent.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/mini_agent.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/common_metrics.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/trials.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_gating.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/environment.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_measurements.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/verify.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/usage.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_environment.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/build_spec.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/run_agent_eval.py

@github-actions

github-actions Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 20898/27461 76.1% 61.1%
Integration Tests 12133/26230 46.3% 19.8%

@arpitsardhana
arpitsardhana force-pushed the aalgo-258-nat-runner/arpsingh branch from 19af066 to 4825d87 Compare June 23, 2026 00:36

@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

🤖 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/examples/run_agent_eval/build_spec.py`:
- Around line 178-180: The comment at line 178-179 provides incorrect rationale
for why `<` and `>` are excluded from `_DOCKERFILE_UNSAFE_CHARS`. The default
RUN form does process shell redirections via `/bin/sh -c`, so these characters
ARE unsafe and must be included in the set. Fix this by adding `<` and `>` to
the character set in `_DOCKERFILE_UNSAFE_CHARS` (change `set('\n\r"`$;&|')` to
include them), and remove or replace the incorrect comment that claims they are
intentionally allowed. The comment should reflect that these characters are
unsafe because they enable shell redirections that break version pins and enable
file overwrites, so they must be quoted instead.
🪄 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: fe2d00eb-a5e5-4ffe-ba9d-447c0dc72e22

📥 Commits

Reviewing files that changed from the base of the PR and between 19af066 and 4825d87.

⛔ Files ignored due to path filters (2)
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/measurements.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/trial_artifacts.py is excluded by !sdk/**
📒 Files selected for processing (16)
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/README.md
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/build_spec.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/gating.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/layout.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/pipeline.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/platform_runtime.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/run_agent_eval.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/verify.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/workflow_runtime.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/measurements.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/trial_artifacts.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_environment.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_gating.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_measurements.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_pipeline.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_verify.py
💤 Files with no reviewable changes (1)
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/layout.py
✅ Files skipped from review due to trivial changes (1)
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/README.md
🚧 Files skipped from review as they are similar to previous changes (13)
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_measurements.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_verify.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/trial_artifacts.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/verify.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/measurements.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_pipeline.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/gating.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_environment.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_gating.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/pipeline.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/run_agent_eval.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/platform_runtime.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/workflow_runtime.py

Comment thread packages/nemo_evaluator_sdk/examples/run_agent_eval/build_spec.py
@arpitsardhana
arpitsardhana force-pushed the aalgo-258-nat-runner/arpsingh branch from 4825d87 to 592f1e5 Compare June 23, 2026 01:36

@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 (3)
packages/nemo_evaluator_sdk/examples/run_agent_eval/workflow_runtime.py (1)

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

Subprocess still orphaned on timeout.

asyncio.wait_for timeout leaves the create_subprocess_exec child running; nothing kills it before returning the failed trial.

Proposed fix
+        process = None
         try:
             process = await asyncio.create_subprocess_exec(
                 *command,
                 stdout=subprocess.PIPE,
                 stderr=subprocess.PIPE,
             )
             stdout, stderr = await asyncio.wait_for(process.communicate(), timeout=self.config.timeout_s)
         except Exception as exc:  # noqa: BLE001 - any launch/timeout failure is a trial-production failure.
+            if process is not None:
+                try:
+                    process.kill()
+                    await process.wait()
+                except Exception:
+                    pass
             return self._failed_trial(task, layout.run_dir, exc)
🤖 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/examples/run_agent_eval/workflow_runtime.py`
around lines 104 - 112, The subprocess created by asyncio.create_subprocess_exec
is not being terminated when asyncio.wait_for times out, leaving the child
process orphaned. In the exception handler that catches the timeout failure and
calls self._failed_trial, add a call to terminate and/or kill the process object
(using process.kill() and potentially awaiting process.wait() to ensure it
exits) before returning the failed trial, so the subprocess is properly cleaned
up regardless of whether the timeout or another exception occurs.
packages/nemo_evaluator_sdk/examples/run_agent_eval/build_spec.py (2)

89-90: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Fail fast when environment.dependencies is not a mapping.

Line 89 and Line 90 silently drop dependencies when dependencies is not a dict. That should raise a validation error instead of continuing with an incomplete build spec.

Proposed fix
-    dependencies = data.get("dependencies") or {}
-    python_deps = dependencies.get("python") if isinstance(dependencies, dict) else None
+    dependencies_raw = data.get("dependencies")
+    if dependencies_raw is None:
+        python_deps = None
+    elif isinstance(dependencies_raw, dict):
+        python_deps = dependencies_raw.get("python")
+    else:
+        raise ValueError(f"Invalid build spec in {task_dir}: 'dependencies' must be a mapping")
🤖 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/examples/run_agent_eval/build_spec.py` around
lines 89 - 90, The code at lines 89-90 silently handles the case where
dependencies is not a dictionary by setting python_deps to None, which causes
incomplete build specs to proceed without error. Instead of checking
isinstance(dependencies, dict) and continuing with None, add validation that
raises an error when dependencies is not a dictionary or mapping type.
Specifically, after retrieving dependencies from data at line 89, validate that
if dependencies is not None and not empty, it must be a dict, and raise an
appropriate validation error if this constraint is violated to fail fast rather
than silently dropping the dependencies.

179-181: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Quote dependency tokens before generating RUN pip install.

Line 207 feeds raw tokens into shell-form RUN at Line 208. numpy<2 is parsed as shell redirection, so valid pins break and redirection side effects are possible.

Proposed fix
+import shlex
@@
-# Note: '<'/'>' are intentionally allowed — pip version pins (``numpy<2``,
-# ``pytest>=8``) use them and a Dockerfile ``RUN`` arg doesn't shell-redirect.
+# Note: RUN shell form is interpreted by a shell; dependency tokens must be quoted.
 _DOCKERFILE_UNSAFE_CHARS = set('\n\r"`$;&|')
@@
     if spec.python_dependencies:
-        deps = " ".join(spec.python_dependencies)
+        deps = " ".join(shlex.quote(dep) for dep in spec.python_dependencies)
         lines.append(f"RUN pip install --no-cache-dir {deps}")
In Dockerfile shell-form RUN, is `pip install numpy<2` parsed using shell redirection semantics, and is quoting each dependency token (e.g., via shlex.quote) the correct mitigation?

Also applies to: 207-208

🤖 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/examples/run_agent_eval/build_spec.py` around
lines 179 - 181, The RUN pip install command being generated at line 208 uses
raw dependency tokens from line 207 without shell quoting, which causes shell
interpretation of special characters like '<' and '>' as redirection operators
rather than part of version specifiers (e.g., numpy<2 gets parsed as
redirection). Apply shlex.quote to each dependency token before building the RUN
command string to properly escape shell special characters and prevent shell
redirection side effects.
🤖 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/trials.py`:
- Around line 159-163: The ATIF format detection on line 162 uses
trace_name.startswith("atif") which is too restrictive and misclassifies files
like "trace.atif.json" as JSON. Instead of checking if the trace_name starts
with "atif", check if the name contains or ends with ".atif" extension (e.g.,
using endswith(".atif") or checking if ".atif" is in the filename) to properly
detect ATIF traces regardless of their complete filename pattern.

---

Duplicate comments:
In `@packages/nemo_evaluator_sdk/examples/run_agent_eval/build_spec.py`:
- Around line 89-90: The code at lines 89-90 silently handles the case where
dependencies is not a dictionary by setting python_deps to None, which causes
incomplete build specs to proceed without error. Instead of checking
isinstance(dependencies, dict) and continuing with None, add validation that
raises an error when dependencies is not a dictionary or mapping type.
Specifically, after retrieving dependencies from data at line 89, validate that
if dependencies is not None and not empty, it must be a dict, and raise an
appropriate validation error if this constraint is violated to fail fast rather
than silently dropping the dependencies.
- Around line 179-181: The RUN pip install command being generated at line 208
uses raw dependency tokens from line 207 without shell quoting, which causes
shell interpretation of special characters like '<' and '>' as redirection
operators rather than part of version specifiers (e.g., numpy<2 gets parsed as
redirection). Apply shlex.quote to each dependency token before building the RUN
command string to properly escape shell special characters and prevent shell
redirection side effects.

In `@packages/nemo_evaluator_sdk/examples/run_agent_eval/workflow_runtime.py`:
- Around line 104-112: The subprocess created by asyncio.create_subprocess_exec
is not being terminated when asyncio.wait_for times out, leaving the child
process orphaned. In the exception handler that catches the timeout failure and
calls self._failed_trial, add a call to terminate and/or kill the process object
(using process.kill() and potentially awaiting process.wait() to ensure it
exits) before returning the failed trial, so the subprocess is properly cleaned
up regardless of whether the timeout or another exception occurs.
🪄 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: 273b6d09-cca3-400f-b84b-3dfb227c2337

📥 Commits

Reviewing files that changed from the base of the PR and between 4825d87 and 592f1e5.

⛔ Files ignored due to path filters (3)
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/metrics.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/environment.py is excluded by !sdk/**
  • sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/trials.py is excluded by !sdk/**
📒 Files selected for processing (22)
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/.gitignore
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/README.md
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/aut_agent.workspace.example.yml
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/aut_runtime.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/build_spec.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/gating.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/layout.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/mini_agent.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/pipeline.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/platform_runtime.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/run_agent_eval.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/usage.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/verify.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/workflow_runtime.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/metrics.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/environment.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/trials.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_environment.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_import_hygiene.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_metrics.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_profbench.py
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_trials.py
💤 Files with no reviewable changes (1)
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_profbench.py
✅ Files skipped from review due to trivial changes (3)
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/.gitignore
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/README.md
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/verify.py
🚧 Files skipped from review as they are similar to previous changes (7)
  • packages/nemo_evaluator_sdk/tests/agent_eval/test_import_hygiene.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/mini_agent.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/pipeline.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/aut_agent.workspace.example.yml
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/run_agent_eval.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/layout.py
  • packages/nemo_evaluator_sdk/examples/run_agent_eval/usage.py

Comment thread packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/trials.py Outdated
@arpitsardhana
arpitsardhana force-pushed the aalgo-258-nat-runner/arpsingh branch from 592f1e5 to fe07759 Compare June 23, 2026 05:39

@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.

lgtm! thanks for the edits

@arpitsardhana
arpitsardhana added this pull request to the merge queue Jun 23, 2026
@arpitsardhana
arpitsardhana removed this pull request from the merge queue due to a manual request Jun 23, 2026
…xample

Add a lean, trials-based agent-evaluation core under
nemo_evaluator_sdk.agent_eval and a self-contained run_agent_eval example
that drives it.

SDK core owns the invariant spine only: AgentEvaluator.run generates or
imports trials, scores each trial x metric, builds a summary, and persists
the run bundle (trials.jsonl, scores, report.html). Supporting modules cover
trial/evidence/target primitives (trials.py), reusable metrics and trial
measurements (metrics.py), results/scores models, a pluggable agent execution
environment (runtimes/environment.py), persistence, and the dashboard.

The example owns task definitions, the runtime/target adapters (toy host
subprocess, Docker-backed nat run workflow, and a deployed agent-under-test),
evidence layout, the deterministic gate policy, build specs, and CLI/printing.
It supports an online path, a real tests/agentic-use BUILD -> AGENT -> VERIFY
path, and offline rescoring of a prior run's trials.

Includes import-hygiene guardrail tests and aligned unit tests covering the
SDK modules.

Signed-off-by: Arpit Singh (SW-CLOUD) <arpsingh@nvidia.com>
@arpitsardhana
arpitsardhana force-pushed the aalgo-258-nat-runner/arpsingh branch from fe07759 to f2082b8 Compare June 23, 2026 19:45
@arpitsardhana
arpitsardhana enabled auto-merge June 23, 2026 19:55
@arpitsardhana
arpitsardhana added this pull request to the merge queue Jun 23, 2026
Merged via the queue into main with commit 41e97a0 Jun 23, 2026
51 checks passed
@arpitsardhana
arpitsardhana deleted the aalgo-258-nat-runner/arpsingh branch June 23, 2026 20:09
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.

2 participants