feat(evaluator): add Runtimes and ProfBench agent-eval examples#380
Merged
Conversation
arpitsardhana
changed the title
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds three agent-eval runtimes ( ChangesSDK Agent Runtimes
ProfBench Rubric-Based Agent-Eval Example
Possibly Related PRs
Suggested Reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
packages/nemo_evaluator_sdk/examples/profbench/README.md (1)
1-277: ⚖️ Poor tradeoffREADME mixes multiple Diataxis quadrants and needs prerequisite/Next Steps sections.
The content spans TUTORIAL (usage), HOW-TO (commands), EXPLANATION (domain model, runner comparison), and REFERENCE (output layout). Per coding guidelines, each page should focus on one quadrant and use cross-links for related content.
Currently, prerequisites (OPENAI_API_KEY, NVIDIA_API_KEY, extras) are scattered throughout. They should be listed at the top. Add a "Next Steps" section at the end linking to
class_diagram.mdandlive_llm_judge_sequence.md.Refactoring to separate pages (e.g., Quick Start → How-to; Architecture → Explanation; Runner Comparison → Reference) would improve clarity, but this is deferred as a longer-term improvement.
📋 Suggested immediate improvements (minimal effort)
Add this at the top (after line 1):
## Prerequisites - Python 3.10+ - `uv` package manager - For `--agent codex`: Local Codex CLI and `~/.codex/auth.json` - For live judge/candidate: `NVIDIA_API_KEY` or configured model API key - For SDK Docker runtime: `OPENAI_API_KEY` (OpenAI Platform) and `--extra agent-runtimes` - Docker (if using `--runtime docker`)Add this at the end (line 277+):
## Next Steps - See [class_diagram.md](class_diagram.md) for an overview of ProfBench types and how they relate to SDK domain model types. - See [live_llm_judge_sequence.md](live_llm_judge_sequence.md) for detailed sequence/flowchart diagrams explaining live judge and live candidate execution paths.🤖 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/profbench/README.md` around lines 1 - 277, The README currently lacks clear prerequisite information and navigation to related documentation. Add a new Prerequisites section immediately after the main title that lists required tools and environment variables (Python 3.10+, uv package manager, Codex CLI for agent usage, API keys like OPENAI_API_KEY and NVIDIA_API_KEY, Docker, and the agent-runtimes extra). Then add a Next Steps section at the very end of the document that provides cross-links to class_diagram.md and live_llm_judge_sequence.md to help readers navigate the broader documentation structure. This consolidates scattered prerequisite information and establishes clear entry points for further learning.Source: Coding guidelines
packages/nemo_evaluator_sdk/tests/agent_eval/test_docker_sandbox_runtime.py (1)
80-107: ⚡ Quick winUse concrete annotations instead of quoted forward refs in test fakes.
Proposed fix
+from __future__ import annotations ... - async def create(self, *, manifest: _FakeManifest, options: _FakeDockerOptions) -> "_FakeSandbox": + async def create(self, *, manifest: _FakeManifest, options: _FakeDockerOptions) -> _FakeSandbox: ... - async def delete(self, session: "_FakeSandbox") -> "_FakeSandbox": + async def delete(self, session: _FakeSandbox) -> _FakeSandbox: ... - async def __aenter__(self) -> "_FakeSandbox": + async def __aenter__(self) -> _FakeSandbox:As per coding guidelines, "Always prefer concrete type hints over string-based ones in Python code."
🤖 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_docker_sandbox_runtime.py` around lines 80 - 107, Replace all quoted forward reference type annotations with concrete type hints in the test fake classes. Specifically, in the _FakeSandboxFactory class methods (create and delete) and the _FakeSandbox class methods (__aenter__), remove the quotes from "_FakeSandbox" annotations so they read as concrete types instead. If self-reference issues arise in the _FakeSandbox class definition, add "from __future__ import annotations" at the top of the file to enable postponed annotation evaluation, which allows you to use concrete type hints without quotes.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 `@packages/nemo_evaluator_sdk/examples/profbench/profbench.py`:
- Around line 584-586: The code in the `if source_text.startswith(("http://",
"https://"))` check allows both HTTP and HTTPS protocols, but the noqa comment
references only "trusted HTTPS URL". To fix this security issue, either restrict
the URL scheme to HTTPS only by changing the startswith tuple to only include
"https://" if HTTPS is required, or update the noqa comment to accurately
reflect that both HTTP and HTTPS protocols are supported if backward
compatibility is needed.
In
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/codex/runtime.py`:
- Around line 176-181: The _evidence_dir method uses _safe_path_name(task.id) to
create directory names, but different task IDs can normalize to the same
sanitized name, causing collisions where different tasks overwrite each other's
evidence. Fix this by ensuring unique directory paths that incorporate the task
index (the index parameter) alongside the sanitized task ID, so that even if two
task IDs sanitize to the same name, they will still get separate directories.
Apply the same fix to the other location mentioned at lines 390-391 that also
uses _safe_path_name for path generation.
- Around line 123-145: The final_output EvidenceDescriptor in the descriptors
dictionary is being added unconditionally, but when final_output_path does not
exist (which happens when Codex exits successfully without writing to
final_output.txt), the evidence descriptor references a non-existent file path
even though output_text falls back to stdout_text. Fix this by conditionally
including the "final_output" descriptor in the descriptors dictionary only when
final_output_path.exists() is true, ensuring the evidence descriptor only
references files that actually exist.
- Around line 97-108: Store the process object returned from
self._process_factory before the asyncio.wait_for() call so it can be accessed
in the exception handler. In the except block, check if the exception is
asyncio.TimeoutError and explicitly call process.kill() or process.terminate()
to terminate the timed-out subprocess before returning the failed trial. This
ensures that subprocesses spawned by the process_factory are properly cleaned up
when timeout occurs, preventing orphaned processes from continuing to run after
a failed trial is returned.
In
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/docker_sandbox.py`:
- Around line 364-365: The _new_runtime_run_id() function generates run IDs
using only second-resolution timestamps, which can cause collisions if multiple
runs start within the same second. Modify the function to include finer-grained
time resolution (such as microseconds from datetime.now(UTC).microsecond) or
append a random component (such as a short UUID or random string) to the
timestamp to ensure each run ID is globally unique, preventing output
directories from merging across simultaneous runs.
- Around line 269-275: The _evidence_dir method constructs task_name using
_safe_path_name which applies sanitization and truncation, potentially mapping
different task IDs to identical folder names and causing artifact collisions.
Modify the task_name construction to incorporate the task index to ensure each
task gets a unique directory path even when sanitization produces collisions.
The task_name assignment should combine the sanitized name with the index
parameter to guarantee uniqueness per task within a run.
In `@packages/nemo_evaluator_sdk/tests/agent_eval/test_profbench.py`:
- Line 25: Add the import statement `from __future__ import annotations` at the
top of the file to match the repo's standardization. Once added, update the
return type annotation on the `__enter__` method from the quoted string
`"_FakeUrlopenResponse"` to the unquoted type `_FakeUrlopenResponse`, since the
future annotations import allows concrete type hints without string quotes.
---
Nitpick comments:
In `@packages/nemo_evaluator_sdk/examples/profbench/README.md`:
- Around line 1-277: The README currently lacks clear prerequisite information
and navigation to related documentation. Add a new Prerequisites section
immediately after the main title that lists required tools and environment
variables (Python 3.10+, uv package manager, Codex CLI for agent usage, API keys
like OPENAI_API_KEY and NVIDIA_API_KEY, Docker, and the agent-runtimes extra).
Then add a Next Steps section at the very end of the document that provides
cross-links to class_diagram.md and live_llm_judge_sequence.md to help readers
navigate the broader documentation structure. This consolidates scattered
prerequisite information and establishes clear entry points for further
learning.
In `@packages/nemo_evaluator_sdk/tests/agent_eval/test_docker_sandbox_runtime.py`:
- Around line 80-107: Replace all quoted forward reference type annotations with
concrete type hints in the test fake classes. Specifically, in the
_FakeSandboxFactory class methods (create and delete) and the _FakeSandbox class
methods (__aenter__), remove the quotes from "_FakeSandbox" annotations so they
read as concrete types instead. If self-reference issues arise in the
_FakeSandbox class definition, add "from __future__ import annotations" at the
top of the file to enable postponed annotation evaluation, which allows you to
use concrete type hints without quotes.
🪄 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: 4cc5574e-bd97-41ae-8261-d9809d5fd586
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
packages/nemo_evaluator_sdk/examples/profbench/README.mdpackages/nemo_evaluator_sdk/examples/profbench/class_diagram.mdpackages/nemo_evaluator_sdk/examples/profbench/dashboard.pypackages/nemo_evaluator_sdk/examples/profbench/live_llm_judge_sequence.mdpackages/nemo_evaluator_sdk/examples/profbench/profbench.pypackages/nemo_evaluator_sdk/examples/profbench/runner.pypackages/nemo_evaluator_sdk/pyproject.tomlpackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/callable_runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/codex/runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/docker_sandbox.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_callable_runtime.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_codex_runtime.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_docker_sandbox_runtime.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_evidence.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_profbench.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_tasks.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_trials.py
arpitsardhana
force-pushed
the
aalgo-272-profbench/arpsingh
branch
from
June 17, 2026 19:35
4b10a1c to
8492498
Compare
Contributor
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (6)
packages/nemo_evaluator_sdk/tests/agent_eval/test_profbench.py (1)
4-9:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse concrete return type annotation for
__enter__.Line 25 uses a string annotation. Add
from __future__ import annotationsand annotate with_FakeUrlopenResponsedirectly.Proposed fix
+from __future__ import annotations @@ - def __enter__(self) -> "_FakeUrlopenResponse": + def __enter__(self) -> _FakeUrlopenResponse: return selfAs per coding guidelines, “Always prefer concrete type hints over string-based ones in Python code; do not import types under TYPE_CHECKING, instead import types as regular imports when possible.”
Also applies to: 25-26
🤖 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_profbench.py` around lines 4 - 9, Add `from __future__ import annotations` to the imports at the top of the file, then update the `__enter__` method's return type annotation (around line 25-26) to use the concrete type `_FakeUrlopenResponse` directly instead of a string-based annotation. This aligns with coding guidelines that prefer concrete type hints over string-based ones.Source: Coding guidelines
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/docker_sandbox.py (2)
364-365:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake auto-generated run IDs collision-resistant.
Second-resolution timestamp IDs can collide for runs started in the same second and merge outputs.
Proposed fix
+from uuid import uuid4 @@ def _new_runtime_run_id() -> str: - return f"agent-runtime-{datetime.now(UTC).strftime('%Y%m%d%H%M%S')}" + timestamp = datetime.now(UTC).strftime("%Y%m%d%H%M%S%f") + return f"agent-runtime-{timestamp}-{uuid4().hex[:8]}"🤖 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/docker_sandbox.py` around lines 364 - 365, The _new_runtime_run_id() function generates IDs using only second-resolution timestamps, which can cause collisions when multiple runs start within the same second. Enhance the ID generation logic to include microsecond-level precision or add a random component (such as a UUID or random hex string) to ensure uniqueness. This will prevent output files from merging when runs are initiated in quick succession.
269-275:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTask evidence paths can collide after sanitization/truncation.
Line 274 can produce identical
task_namefor different IDs; artifacts can overwrite each other within the same run.Proposed fix
- task_name = _safe_path_name(task.id) or f"task-{index}" + safe_task_id = _safe_path_name(task.id) + task_name = f"{index:06d}-{safe_task_id}" if safe_task_id else f"task-{index:06d}"🤖 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/docker_sandbox.py` around lines 269 - 275, The task_name construction in the _evidence_dir method can produce identical directory names for different task IDs because _safe_path_name may truncate or sanitize different IDs to the same result, and the index fallback only applies when the sanitized name is empty. To fix this, ensure the task_name incorporates both the sanitized task ID and the index to guarantee uniqueness for all tasks within a run, preventing artifact collisions even when different task IDs sanitize to identical values.packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/codex/runtime.py (3)
96-108:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTerminate timed-out Codex subprocesses before returning FAILED trials.
On timeout at Line 103, the child process can keep running because the exception path at Line 107 does not kill/wait it. That leaks processes and can exhaust resources.
Proposed fix
+import contextlib @@ - try: - process = await self._process_factory( + process: Any | None = None + try: + process = await self._process_factory( *command, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, ) stdout, stderr = await asyncio.wait_for( process.communicate(prompt.encode("utf-8")), timeout=self._timeout_s, ) + except asyncio.TimeoutError as exc: + if process is not None and process.returncode is None: + process.kill() + with contextlib.suppress(Exception): + await process.wait() + return _failed_codex_trial(task, evidence_dir, exc, runtime_name=self._runtime_name) except Exception as exc: return _failed_codex_trial(task, evidence_dir, exc, runtime_name=self._runtime_name)🤖 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/codex/runtime.py` around lines 96 - 108, When a timeout or other exception occurs during the asyncio.wait_for call on the process.communicate invocation, the child process created by self._process_factory continues running and is never cleaned up, causing resource leaks. In the except block that handles exceptions and calls _failed_codex_trial, terminate the child process by calling its termination method (such as kill() or terminate()), and optionally await the process to ensure it is fully cleaned up before returning the failed trial result.
123-145:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnsure
final_outputevidence always references an existing file.Line 144 always publishes
final_output, but Line 123 can fall back to stdout without creatingfinal_output.txt. That breaks evidence-file contract for successful trials.Proposed fix
- output_text = final_output_path.read_text(encoding="utf-8") if final_output_path.exists() else stdout_text + if final_output_path.exists(): + output_text = final_output_path.read_text(encoding="utf-8") + else: + output_text = stdout_text + final_output_path.write_text(output_text, encoding="utf-8")🤖 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/codex/runtime.py` around lines 123 - 145, The code at line 123 conditionally falls back to stdout_text when final_output_path does not exist, but the descriptors dictionary at line 144 always unconditionally includes the final_output EvidenceDescriptor referencing final_output_path, which may not exist. Fix this by conditionally including the final_output descriptor in the descriptors dictionary only when final_output_path actually exists. You can achieve this by checking final_output_path.exists() before adding the final_output key to the descriptors dictionary, ensuring the evidence-file contract is maintained for all trials.
176-181:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid task evidence directory collisions from sanitized/truncated IDs.
Line 180 and
_safe_path_name(Line 390) can map distinct task IDs to the same directory. Prefix with index (or hash) to make per-task evidence paths unique.Proposed fix
- return Path(root) / (_safe_path_name(task.id) or f"task-{index}") + safe_task_id = _safe_path_name(task.id) + task_dir = f"{index:06d}-{safe_task_id}" if safe_task_id else f"task-{index:06d}" + return Path(root) / task_dirAlso applies to: 390-391
🤖 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/codex/runtime.py` around lines 176 - 181, The _evidence_dir method constructs evidence directory paths that can collide when different task IDs get sanitized to the same directory name by _safe_path_name. To fix this, modify the return statement in _evidence_dir to prefix the directory path with the index parameter, ensuring each task gets a unique evidence directory even when their sanitized IDs are identical. For example, prefix the path component using the index before applying the sanitized task ID or fallback task identifier.
🧹 Nitpick comments (3)
packages/nemo_evaluator_sdk/tests/agent_eval/test_codex_runtime.py (1)
117-219: ⚡ Quick winAdd regressions for timeout cleanup and colliding task IDs.
Current tests don’t assert timeout subprocess termination or unique evidence dirs when different task IDs sanitize to the same name. Add both to prevent recurrence of runtime data-loss/leak bugs.
🤖 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_codex_runtime.py` around lines 117 - 219, Add two new regression test functions to this file to prevent recurrence of runtime data-loss and cleanup bugs. First, add a test similar to test_codex_cli_agent_runtime_uses_local_codex_command_and_writes_evidence but that exercises timeout behavior, verifying that when a task execution times out the subprocess is properly terminated and cleaned up. Second, add a test that runs multiple tasks with different IDs that sanitize to the same directory name and verifies that each task gets a unique evidence directory, preventing evidence files from being overwritten or corrupted. Both tests should use the same FakeProcess and fake_process_factory pattern as the existing tests.packages/nemo_evaluator_sdk/examples/profbench/README.md (1)
163-277: 🏗️ Heavy liftSplit this page into one Diataxis quadrant and cross-link the rest.
This section mixes HOW-TO flow with EXPLANATION and REFERENCE material in one page. Keep this README task-focused and move domain model/scoring internals to separate docs, then add a
Next Stepscross-link block at the end.
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,” 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/profbench/README.md` around lines 163 - 277, The README is mixing multiple Diataxis quadrants together by including Domain Model (explanation), Comparison table (reference), and Scoring Logic (reference) content alongside the HOW-TO content about the three runner examples. Refactor the page to keep only the task-focused HOW-TO material: keep the three run_profbench examples and their descriptions, then move the Domain Model section, Comparison table, and Scoring Logic section into separate documentation files (e.g., a domain-model.md reference doc and a scoring-logic.md reference doc). Finally, add a "Next Steps" section at the end of the README with cross-links to these new documentation pages and any other related content to guide readers on where to learn more.Source: Coding guidelines
packages/nemo_evaluator_sdk/examples/profbench/profbench.py (1)
6-6: ⚡ Quick winRemove
from __future__ import annotationsand use concreteSelfannotations.Replace line 6's deferred annotation import and the four string-based return annotations on lines 72, 123, and 183 with concrete
Selftype hints. Python 3.11+ (your minimum target) supportstyping.Selfnatively.Proposed fix
-from __future__ import annotations - import json import os import re from collections.abc import Awaitable from pathlib import Path -from typing import Any, Protocol +from typing import Any, Protocol, Self @@ - def _atif_requires_line(self) -> "EvidenceLocator": + def _atif_requires_line(self) -> Self: @@ - def _validate_evidence(self) -> "ScoreDeduction": + def _validate_evidence(self) -> Self: @@ - ) -> "ProfBenchCriterion": + ) -> Self:Per coding guidelines,
**/*.py: prefer concrete type hints over string-based ones; do not import types 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 `@packages/nemo_evaluator_sdk/examples/profbench/profbench.py` at line 6, Remove the `from __future__ import annotations` import statement from the top of the file and replace it with a concrete import of `Self` from the `typing` module. Then locate and replace all string-based return annotations (particularly those returning `"Self"`) with the concrete `Self` type hint. This applies to the return type annotations on the methods around lines 72, 123, and 183. Since Python 3.11+ natively supports `typing.Self`, using concrete type hints instead of deferred string-based annotations aligns with the project's coding guidelines.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 `@packages/nemo_evaluator_sdk/examples/profbench/profbench.py`:
- Around line 545-549: The code uses _coerce_bool() to coerce fulfilment values
from rubrics, which causes missing values to become False and unknown non-empty
strings to become True, leading to incorrect scoring instead of failing fast.
Remove the _coerce_bool() calls wrapping
row["rubrics"][index].get(f"{model_id}_fulfilment") in the dictionary
comprehension at lines 545-549, and apply the same fix to the similar code block
at lines 573-577. Additionally, either remove the _coerce_bool() function
definition at lines 719-728 if it is no longer used elsewhere, or refactor it to
properly validate expected fulfilment label values instead of using generic
bool() coercion.
- Around line 184-190: The code in the Rubric.from_raw method currently uses
PROFBENCH_WEIGHT_POINTS.get(weight_name, 1.0) which silently defaults unknown
weights to 1.0 point, corrupting benchmark scores. Replace the .get() call with
direct dictionary access or explicit validation to reject unknown
criterion_weight values. Check if weight_name exists in PROFBENCH_WEIGHT_POINTS
and raise a ValueError or appropriate exception when an unknown or misspelled
weight is encountered, ensuring only valid weights are accepted when assigning
the points attribute.
In `@packages/nemo_evaluator_sdk/examples/profbench/README.md`:
- Around line 1-50: The README file in
packages/nemo_evaluator_sdk/examples/profbench/ starts with run commands without
listing required setup steps. Add a Prerequisites section immediately after the
main title "# ProfBench Agent-Eval Example" and before the "Run the example from
the repository root:" line. This section should document all environment setup,
required tooling, authentication credentials, dependencies, and runtime
prerequisites that users need to complete before executing the example commands.
- Around line 6-24: The fenced code blocks in the README violate MD031 markdown
linting rules due to missing blank lines around them. Add a blank line between
the "- Local" list item and the opening code fence (```bash), add a blank line
after the closing fence of the first code block, add a blank line between the "-
Docker" list item and its opening code fence, and add a blank line after the
closing fence of the second code block. This ensures proper spacing around all
fenced code blocks in the markdown list.
In
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/docker_sandbox.py`:
- Around line 153-154: The `await client.delete(sandbox)` call in the finally
block (line 154) can raise an exception that will override the actual trial
result if sandbox cleanup fails. Wrap this call in a try-except block to catch
and handle any exceptions that occur during sandbox deletion, logging them if
needed, so that exceptions during cleanup do not propagate and mask the original
task outcome.
- Around line 162-165: The workspace_dir obtained from task.inputs is being
passed directly to Path(workspace_dir) without validation, creating a directory
traversal vulnerability where untrusted inputs could specify paths like
"../../../etc" or absolute paths. After retrieving workspace_dir with
task.inputs.get("workspace_dir"), validate it to ensure the resolved path stays
within expected boundaries and does not contain directory traversal sequences.
Use Path.resolve() to get the absolute path and verify it is within allowed
directories before passing it to sdk.LocalDir(src=Path(workspace_dir)).
---
Duplicate comments:
In
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/codex/runtime.py`:
- Around line 96-108: When a timeout or other exception occurs during the
asyncio.wait_for call on the process.communicate invocation, the child process
created by self._process_factory continues running and is never cleaned up,
causing resource leaks. In the except block that handles exceptions and calls
_failed_codex_trial, terminate the child process by calling its termination
method (such as kill() or terminate()), and optionally await the process to
ensure it is fully cleaned up before returning the failed trial result.
- Around line 123-145: The code at line 123 conditionally falls back to
stdout_text when final_output_path does not exist, but the descriptors
dictionary at line 144 always unconditionally includes the final_output
EvidenceDescriptor referencing final_output_path, which may not exist. Fix this
by conditionally including the final_output descriptor in the descriptors
dictionary only when final_output_path actually exists. You can achieve this by
checking final_output_path.exists() before adding the final_output key to the
descriptors dictionary, ensuring the evidence-file contract is maintained for
all trials.
- Around line 176-181: The _evidence_dir method constructs evidence directory
paths that can collide when different task IDs get sanitized to the same
directory name by _safe_path_name. To fix this, modify the return statement in
_evidence_dir to prefix the directory path with the index parameter, ensuring
each task gets a unique evidence directory even when their sanitized IDs are
identical. For example, prefix the path component using the index before
applying the sanitized task ID or fallback task identifier.
In
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/docker_sandbox.py`:
- Around line 364-365: The _new_runtime_run_id() function generates IDs using
only second-resolution timestamps, which can cause collisions when multiple runs
start within the same second. Enhance the ID generation logic to include
microsecond-level precision or add a random component (such as a UUID or random
hex string) to ensure uniqueness. This will prevent output files from merging
when runs are initiated in quick succession.
- Around line 269-275: The task_name construction in the _evidence_dir method
can produce identical directory names for different task IDs because
_safe_path_name may truncate or sanitize different IDs to the same result, and
the index fallback only applies when the sanitized name is empty. To fix this,
ensure the task_name incorporates both the sanitized task ID and the index to
guarantee uniqueness for all tasks within a run, preventing artifact collisions
even when different task IDs sanitize to identical values.
In `@packages/nemo_evaluator_sdk/tests/agent_eval/test_profbench.py`:
- Around line 4-9: Add `from __future__ import annotations` to the imports at
the top of the file, then update the `__enter__` method's return type annotation
(around line 25-26) to use the concrete type `_FakeUrlopenResponse` directly
instead of a string-based annotation. This aligns with coding guidelines that
prefer concrete type hints over string-based ones.
---
Nitpick comments:
In `@packages/nemo_evaluator_sdk/examples/profbench/profbench.py`:
- Line 6: Remove the `from __future__ import annotations` import statement from
the top of the file and replace it with a concrete import of `Self` from the
`typing` module. Then locate and replace all string-based return annotations
(particularly those returning `"Self"`) with the concrete `Self` type hint. This
applies to the return type annotations on the methods around lines 72, 123, and
183. Since Python 3.11+ natively supports `typing.Self`, using concrete type
hints instead of deferred string-based annotations aligns with the project's
coding guidelines.
In `@packages/nemo_evaluator_sdk/examples/profbench/README.md`:
- Around line 163-277: The README is mixing multiple Diataxis quadrants together
by including Domain Model (explanation), Comparison table (reference), and
Scoring Logic (reference) content alongside the HOW-TO content about the three
runner examples. Refactor the page to keep only the task-focused HOW-TO
material: keep the three run_profbench examples and their descriptions, then
move the Domain Model section, Comparison table, and Scoring Logic section into
separate documentation files (e.g., a domain-model.md reference doc and a
scoring-logic.md reference doc). Finally, add a "Next Steps" section at the end
of the README with cross-links to these new documentation pages and any other
related content to guide readers on where to learn more.
In `@packages/nemo_evaluator_sdk/tests/agent_eval/test_codex_runtime.py`:
- Around line 117-219: Add two new regression test functions to this file to
prevent recurrence of runtime data-loss and cleanup bugs. First, add a test
similar to
test_codex_cli_agent_runtime_uses_local_codex_command_and_writes_evidence but
that exercises timeout behavior, verifying that when a task execution times out
the subprocess is properly terminated and cleaned up. Second, add a test that
runs multiple tasks with different IDs that sanitize to the same directory name
and verifies that each task gets a unique evidence directory, preventing
evidence files from being overwritten or corrupted. Both tests should use the
same FakeProcess and fake_process_factory pattern as the existing tests.
🪄 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: 27b126d8-a1b6-48ea-b3f9-35d29cd92a6e
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
packages/nemo_evaluator_sdk/examples/profbench/README.mdpackages/nemo_evaluator_sdk/examples/profbench/class_diagram.mdpackages/nemo_evaluator_sdk/examples/profbench/dashboard.pypackages/nemo_evaluator_sdk/examples/profbench/live_llm_judge_sequence.mdpackages/nemo_evaluator_sdk/examples/profbench/profbench.pypackages/nemo_evaluator_sdk/examples/profbench/runner.pypackages/nemo_evaluator_sdk/pyproject.tomlpackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/callable_runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/codex/runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/docker_sandbox.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_callable_runtime.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_codex_runtime.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_docker_sandbox_runtime.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_evidence.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_profbench.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_tasks.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_trials.py
✅ Files skipped from review due to trivial changes (2)
- packages/nemo_evaluator_sdk/examples/profbench/class_diagram.md
- packages/nemo_evaluator_sdk/examples/profbench/live_llm_judge_sequence.md
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/nemo_evaluator_sdk/pyproject.toml
- packages/nemo_evaluator_sdk/tests/agent_eval/test_evidence.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_callable_runtime.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_trials.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/callable_runtime.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_tasks.py
- packages/nemo_evaluator_sdk/examples/profbench/runner.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_docker_sandbox_runtime.py
arpitsardhana
force-pushed
the
aalgo-272-profbench/arpsingh
branch
from
June 17, 2026 20:28
8492498 to
c540026
Compare
arpitsardhana
force-pushed
the
aalgo-272-profbench/arpsingh
branch
2 times, most recently
from
June 19, 2026 22:54
9d8dda8 to
207e171
Compare
Cover the standalone agent-eval domain model split out of the engine work: task metric-descriptor serialization, duplicate-metric and view-signal validation (test_tasks.py); trial mapping-shaped evidence coercion, descriptor serialization, and completed-trial output requirement (test_trials.py); and candidate evidence preservation through build_metric_input plus lazy/cached local filesystem access (test_evidence.py). Signed-off-by: Arpit Singh (SW-CLOUD) <arpsingh@nvidia.com>
arpitsardhana
force-pushed
the
aalgo-272-profbench/arpsingh
branch
from
June 20, 2026 07:05
207e171 to
e795e67
Compare
…dbox) Add the bare-minimum AgentTaskRunner implementations the AgentEvaluator can drive via run(target=runner): - CallableAgentTaskRunner: dependency-light reference runtime that delegates each task to a user-supplied async callable. - CodexCliAgentRuntime / CodexDockerCliAgentRuntime: Codex CLI candidate generation on the host or inside Docker, with resolve_codex_target() routing. - DockerSandboxAgentRuntime: OpenAI Agents SDK SandboxAgent per task, gated behind the optional nemo-evaluator-sdk[agent-runtimes] extra. Signed-off-by: Arpit Singh (SW-CLOUD) <arpsingh@nvidia.com>
Add the ProfBench example for the agent-eval SDK: dataset loading, rubric metric with traceable point deductions, a Yes/No LLM judge, a ProfBench-aware HTML dashboard, and a CLI runner with baseline, live-judge, and live-candidate modes (including codex-backed candidate generation). Includes example docs (README, class and sequence diagrams) and unit tests. Stacked on the agent-eval runtimes commit (callable/codex/docker sandbox). Signed-off-by: Arpit Singh (SW-CLOUD) <arpsingh@nvidia.com>
arpitsardhana
force-pushed
the
aalgo-272-profbench/arpsingh
branch
from
June 20, 2026 08:31
e795e67 to
68f9512
Compare
SandyChapman
approved these changes
Jun 22, 2026
This was referenced Jul 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the ProfBench example for the agent-eval SDK, along with the agent-eval runtimes it depends on. This branch is stacked locally on top of two unmerged ancestors, so this PR (based on
main) currently includes three commits:test(evaluator): add agent-eval SDK domain-model unit tests— unit tests for the agent-eval domain model (tasks, trials, evidence).feat(evaluator): add agent-eval runtimes (callable, codex, docker sandbox)—CallableAgentTaskRunner,CodexCliAgentRuntime/CodexDockerCliAgentRuntime, andDockerSandboxAgentRuntime, plus theagent-runtimesoptional extra (openai-agents[docker]).feat(evaluator): add ProfBench agent-eval example— the ProfBench example itself.ProfBench example contents
profbench.py— dataset loading, a rubric metric with traceable point deductions, and a Yes/No LLM judge.dashboard.py— a ProfBench-aware HTML report built on theAgentEvalResult/AgentEvalTaskScoremodel.runner.py— CLI with baseline, live-judge, and live-candidate modes (including codex-backed candidate generation).README.md,class_diagram.md,live_llm_judge_sequence.md— example docs and diagrams.tests/agent_eval/test_profbench.py— unit tests for the example.Test plan
uv run ruff check(profbench files) — passesuv run ruff format --check— cleanuv run --frozen ty check(profbench files) — passesuv run --frozen pytest tests/agent_eval/test_profbench.py— 20 passedNotes
uv.lockis updated for theagent-runtimesextra; verify it's reconciled against the latestmainbefore merge.Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores
agent-runtimesdependency group with OpenAI Agents SDK Docker support