fix(agent-eval): stop leaking task.intent to the agent under evaluation - #605
Merged
Conversation
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:
📝 WalkthroughWalkthroughPrompt construction in Codex, Docker sandbox, and Fabric runtimes now excludes ChangesIntent-free prompt construction
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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.
🧹 Nitpick comments (1)
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/codex/runtime.py (1)
389-416: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winIntent removed correctly; consider deduping vs.
fabric_input.Logic here (instruction derivation,
body_inputsfiltering, seed-file listing) is nearly identical to the newfabric_input/_instructionhelpers infabric/_common.py. That module's own docstring argues for centralizing this exact logic to prevent security-relevant drift between runtimes — the same argument applies here sincedefault_codex_promptisn't sharing that implementation.Consider extracting a shared prompt-framing helper (e.g. instruction resolution + body-input filtering) usable by both codex and fabric runtimes, so future changes to intent-exclusion logic can't accidentally apply to one runtime and not the other.
🤖 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 389 - 416, The prompt framing logic in default_codex_prompt duplicates the instruction resolution, body_inputs filtering, and seed-file handling already being centralized in fabric/_common.py. Refactor this function to call a shared helper used by both the codex and fabric runtimes for deriving the instruction and filtering inputs, and keep only codex-specific wording here so the intent-exclusion behavior stays consistent across runtimes.
🤖 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.
Nitpick comments:
In
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/codex/runtime.py`:
- Around line 389-416: The prompt framing logic in default_codex_prompt
duplicates the instruction resolution, body_inputs filtering, and seed-file
handling already being centralized in fabric/_common.py. Refactor this function
to call a shared helper used by both the codex and fabric runtimes for deriving
the instruction and filtering inputs, and keep only codex-specific wording here
so the intent-exclusion behavior stays consistent across runtimes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 8c8c88d4-7bdc-4c84-99e2-f694a811be0a
📒 Files selected for processing (7)
packages/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/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/_common.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/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_fabric_runtime.py
SandyChapman
force-pushed
the
claude/gallant-noether-ac62cb
branch
from
July 8, 2026 15:15
9b09903 to
580b4c5
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/_common.py (1)
17-17: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
from __future__ import annotationsstringifies type hints.Conflicts with guideline to prefer concrete over string-based type hints. Drop it since annotations here (
str | None,Mapping,Sequence) work fine without it.As per coding guidelines, "In Python code, prefer concrete type hints over string-based type hints, and do not import those types only under
TYPE_CHECKING; import them normally when possible."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/_common.py` at line 17, Remove the future annotations import from the module so type hints stay concrete rather than stringified. The affected symbols are the module-level annotations that use str | None, Mapping, and Sequence in the same file; verify they still resolve normally without from __future__ import annotations and keep the imports for those types as regular runtime imports rather than TYPE_CHECKING-only.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.
Nitpick comments:
In
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/_common.py`:
- Line 17: Remove the future annotations import from the module so type hints
stay concrete rather than stringified. The affected symbols are the module-level
annotations that use str | None, Mapping, and Sequence in the same file; verify
they still resolve normally without from __future__ import annotations and keep
the imports for those types as regular runtime imports rather than
TYPE_CHECKING-only.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a53fc711-4e90-455d-91a7-532c29687fc5
📒 Files selected for processing (7)
packages/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/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/_common.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/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_fabric_runtime.py
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/nemo_evaluator_sdk/tests/agent_eval/test_codex_runtime.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/codex/runtime.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/docker_sandbox.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_docker_sandbox_runtime.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_runtime.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/runtime.py
Contributor
|
SandyChapman
force-pushed
the
claude/gallant-noether-ac62cb
branch
from
July 8, 2026 15:28
580b4c5 to
741ddb6
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/_common.py (1)
62-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: redundant branching.
Can collapse to one return:
♻️ Simplify
- for key in _INSTRUCTION_KEYS: - value = inputs.get(key) - if isinstance(value, str) and value: - return value - if value: - return str(value) - return None + for key in _INSTRUCTION_KEYS: + value = inputs.get(key) + if value: + return value if isinstance(value, str) else str(value) + return None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/_common.py` around lines 62 - 69, The _instruction helper has redundant branching when reading from _INSTRUCTION_KEYS; simplify the logic in _instruction by using a single return path for non-empty values instead of separate string and truthy checks. Keep the same behavior for string and non-string values, but collapse the conditional flow so the value is handled in one place.
🤖 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.
Nitpick comments:
In
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/_common.py`:
- Around line 62-69: The _instruction helper has redundant branching when
reading from _INSTRUCTION_KEYS; simplify the logic in _instruction by using a
single return path for non-empty values instead of separate string and truthy
checks. Keep the same behavior for string and non-string values, but collapse
the conditional flow so the value is handled in one place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c517fb11-cd33-4e4a-85d9-fbf854cf1cbf
⛔ Files ignored due to path filters (4)
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/codex/runtime.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/docker_sandbox.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/_common.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/runtime.pyis excluded by!sdk/**
📒 Files selected for processing (7)
packages/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/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/_common.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/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_fabric_runtime.py
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/codex/runtime.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_docker_sandbox_runtime.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/docker_sandbox.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_runtime.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_codex_runtime.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/runtime.py
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/_common.py (1)
51-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify branching.
- for key in _INSTRUCTION_KEYS: - value = inputs.get(key) - if isinstance(value, str) and value: - return value - if value: - return str(value) - return None + for key in _INSTRUCTION_KEYS: + value = inputs.get(key) + if value: + return value if isinstance(value, str) else str(value) + return None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/_common.py` around lines 51 - 58, The _instruction helper has redundant branching when checking values from _INSTRUCTION_KEYS. Simplify the logic in _instruction by collapsing the string and truthy checks into a single path so it returns the first non-empty value consistently, while preserving the existing behavior of returning the original string when possible and stringifying other truthy values.
🤖 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.
Nitpick comments:
In
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/_common.py`:
- Around line 51-58: The _instruction helper has redundant branching when
checking values from _INSTRUCTION_KEYS. Simplify the logic in _instruction by
collapsing the string and truthy checks into a single path so it returns the
first non-empty value consistently, while preserving the existing behavior of
returning the original string when possible and stringifying other truthy
values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c25ce1cd-d118-42a3-bf48-70b568cc5098
📒 Files selected for processing (5)
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/codex/runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/_common.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/runtime.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_codex_runtime.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_runtime.py
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/runtime.py
- packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_runtime.py
SandyChapman
force-pushed
the
claude/gallant-noether-ac62cb
branch
2 times, most recently
from
July 9, 2026 19:41
0173310 to
a53beca
Compare
ngoncharenko
approved these changes
Jul 14, 2026
The prompt handed to the agent under evaluation is now exactly the task's natural-language instruction (inputs["instruction"]) with no runtime-added framing. intent stays eval-side only (a reward-hacking hole if leaked). - Add AgentEvalTask.agent_prompt(): resolves inputs["instruction"], raising when it is absent so an instruction-less task fails-and-skips. - Route the fabric, codex, and docker-sandbox runtimes through it and delete the duplicated prompt resolvers; drop the prompt/intent fallbacks (_task_row no longer falls back to task.intent). - Also stop persisting grader-only fields (intent, reference) into the codex evidence dir: the docker variant mounts it into the sandbox (danger-full-access), so task.json is redacted to keep the held-out ground truth out of the agent's reach. Signed-off-by: Sandy Chapman <schapman@nvidia.com>
SandyChapman
force-pushed
the
claude/gallant-noether-ac62cb
branch
from
July 14, 2026 12:26
a53beca to
44610da
Compare
SandyChapman
enabled auto-merge
July 14, 2026 12:29
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The agent-eval runtimes framed the harness prompt with
task.intent— the eval-side description of the desired behavior (what the grader checks for). Handing that to the agent under evaluation is a reward-hacking hole: the agent can read the grader's intent and target it directly. Same class of concern as the held-out-referencefield work (AGENT-EVAL).Three runtimes leaked it:
FabricAgentRuntime—f"Task id: {id}\nIntent: {intent}\nInputs: {inputs}\n"default_codex_prompt— an explicitIntent: {task.intent}linedocker_sandbox._task_prompt—intentas the last-resort fallback, written into the agent'sinstruction.mdChange
Unify the framing so every runtime builds the prompt from
task.inputsonly:runtimes/fabric/_common.pywith a shared, intent-freefabric_input: task id → instruction (inputs["instruction"|"prompt"]) → remaininginputscontent → seed files listed by name (not dumped inline). The hostFabricAgentRuntimenow calls it. Built as the shared home so the AALGO-321 container runtime (PR feat(evaluator-sdk): sandboxed containerized Fabric runner (AALGO-321) #604) reuses it instead of carrying its own copy.default_codex_promptdrops theIntent:line, lifts the instruction from inputs, keeps its workspace-edit trailer.docker_sandbox._task_promptno longer falls back totask.intent; a task with no instruction in inputs yields an empty prompt rather than leaking intent.Tests
Updated
test_fabric_runtime.py,test_codex_runtime.py, andtest_docker_sandbox_runtime.pyto lock in the intent-free contract across all three runtimes (assert instruction is surfaced, intent text andIntent:label are absent).pytest(all three suites) → 34 passedruff check/ruff format→ cleanty→ no new diagnosticsRelates to AALGO-321 fabric-runtime dedup (PR #604).
🤖 Generated with Claude Code
Summary by CodeRabbit
instruction/prompt(withinstructiontaking priority) and to omit any evaluation intent details.Inputs:JSON from remaining task inputs only, excluding instruction-related fields and the seeded-files key.