feat(evaluator-sdk): per-task workspace + held-out overlay verifier for Fabric agent-eval - #573
Conversation
c358bce to
fbd0c05
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughFabric now stages per-task workspaces, seeds them from ChangesFabric Runtime Workspace Staging
Verifier Overlay Files Support
Sequence Diagram(s)sequenceDiagram
participant RunTask
participant Seeder
participant Profile
participant Harness
participant Trial
RunTask->>Seeder: seed inputs['files'] into workspace_dir
Seeder-->>RunTask: seeded_files
RunTask->>Profile: build environment.workspace overlay
RunTask->>Harness: run with _fabric_input(task, seeded_files)
Harness-->>RunTask: result
RunTask->>Trial: build trial(result, workspace_dir)
sequenceDiagram
participant Caller
participant RunVerifier
participant Sandbox
participant WriteOverlay
Caller->>RunVerifier: run_verifier(overlay_files, cwd)
RunVerifier->>Sandbox: copy evidence snapshot
RunVerifier->>WriteOverlay: write overlay_files into sandbox
WriteOverlay-->>RunVerifier: ValueError if path escapes root
RunVerifier->>Sandbox: execute verifier command
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/runtime.py (1)
168-183: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winSeed handling is outside the task timeout.
asyncio.wait_for(...)only boundsclient.run;seed_workspace(...)can still block first, so a slow handler can push this task pastself._timeout_s. Wrap seeding in the timeout too, or enforce timeouts in every handler.🤖 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/runtime.py` around lines 168 - 183, The task timeout currently only wraps client.run in runtime.py’s fabric executor, so seed_workspace can still run unbounded before the timeout starts. Move the seeding work into the same timeout window as the rest of the task execution by applying asyncio.wait_for around the seed_workspace call (or otherwise ensuring the handler path is timed), then continue building seeded_files and calling _fabric_input and client.run as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/runtime.py`:
- Around line 168-183: The task timeout currently only wraps client.run in
runtime.py’s fabric executor, so seed_workspace can still run unbounded before
the timeout starts. Move the seeding work into the same timeout window as the
rest of the task execution by applying asyncio.wait_for around the
seed_workspace call (or otherwise ensuring the handler path is timed), then
continue building seeded_files and calling _fabric_input and client.run as
before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a543acb2-f790-4123-ba7d-01d20b676e21
⛔ Files ignored due to path filters (2)
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/runtime.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/values/evidence.pyis excluded by!sdk/**
📒 Files selected for processing (4)
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/runtime.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/evidence.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_evidence.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_runtime.py
|
19724e2 to
d72f36b
Compare
d72f36b to
ba2761e
Compare
…pace FabricAgentRuntime was trajectory-only: it never seeded inputs['files'] and exposed no workspace filesystem evidence, so workspace-scoring metrics (file checks, pytest runs) that work against the Codex runtime could not score a Fabric trial. Since Fabric drives the harness on the host filesystem in a dir we choose, the agent's edited files are recoverable directly. Now every task runs in its own fresh per-task workspace: inputs['files'] are seeded into it (a no-op when there are none), the harness runs in it through an `environment.workspace` profile overlay (same mechanism as the trajectory overlay), and the final tree is exposed as a `workspace` filesystem evidence descriptor — identical key/kind to the Codex runtime, so existing workspace-reading metrics work unchanged. Seeding runs inside the per-task guarded block so a bad seed fails only that task. The harness input lists staged files by name instead of dumping their contents inline. Staging is unconditional rather than a flag: a uniform per-task workspace is the right default (matches the Codex runtime) and maps cleanly onto a per-task container volume later. Any environment.workspace set in the supplied config is overridden per task. Fabric is now a superset of the Codex runtime: workspace evidence AND the ATIF trajectory. Signed-off-by: Sandy Chapman <schapman@nvidia.com>
…_files
Add an `overlay_files` param to `LocalFilesystemEvidence.run_verifier`: a
{relative_path: contents} map of trusted files written over the throwaway copy
after it is made, before the command runs. This is how a grader supplies
held-out artifacts — a canonical test suite, a reference implementation — that
must not live in, and cannot be edited through, the agent's own workspace.
Paths escaping the copy are rejected; the stored evidence is never mutated.
Pairs with the per-task Fabric workspace: seed only the agent-facing files into
the workspace, keep graders' ground truth in the task's `reference`, and overlay
it at scoring time.
Signed-off-by: Sandy Chapman <schapman@nvidia.com>
…ssMetric AgentPhaseSuccessMetric scores a trial by reading candidate.metadata["agent_ok"] (an explicit bool), which the Codex runtime sets but FabricAgentRuntime did not. So agent_phase_success was False for every Fabric trial regardless of the run, sinking any task whose view ANDs it in (e.g. a fix-a-bug task) even when the agent succeeded and its tests passed. Mirror the Codex runtime: set agent_ok=True on a successful trial and agent_ok=False on a failed one. Adds a regression test that scores AgentPhaseSuccessMetric over a Fabric trial through the evaluator's own _trial_sample path. Signed-off-by: Sandy Chapman <schapman@nvidia.com>
ba2761e to
4e98fe9
Compare
|
@coderabbitai Re: the "seed handling is outside the task timeout" note in
If we ever need to bound seed/download time, the right place is a timeout inside the fileset handler itself (your secondary suggestion), not a |
|
Tip For best results, initiate chat on the files or code changes.
|
…or Fabric agent-eval (#573) * feat(evaluator-sdk): run each Fabric agent-eval task in its own workspace FabricAgentRuntime was trajectory-only: it never seeded inputs['files'] and exposed no workspace filesystem evidence, so workspace-scoring metrics (file checks, pytest runs) that work against the Codex runtime could not score a Fabric trial. Since Fabric drives the harness on the host filesystem in a dir we choose, the agent's edited files are recoverable directly. Now every task runs in its own fresh per-task workspace: inputs['files'] are seeded into it (a no-op when there are none), the harness runs in it through an `environment.workspace` profile overlay (same mechanism as the trajectory overlay), and the final tree is exposed as a `workspace` filesystem evidence descriptor — identical key/kind to the Codex runtime, so existing workspace-reading metrics work unchanged. Seeding runs inside the per-task guarded block so a bad seed fails only that task. The harness input lists staged files by name instead of dumping their contents inline. Staging is unconditional rather than a flag: a uniform per-task workspace is the right default (matches the Codex runtime) and maps cleanly onto a per-task container volume later. Any environment.workspace set in the supplied config is overridden per task. Fabric is now a superset of the Codex runtime: workspace evidence AND the ATIF trajectory. Signed-off-by: Sandy Chapman <schapman@nvidia.com> * feat(evaluator-sdk): stage held-out files in run_verifier via overlay_files Add an `overlay_files` param to `LocalFilesystemEvidence.run_verifier`: a {relative_path: contents} map of trusted files written over the throwaway copy after it is made, before the command runs. This is how a grader supplies held-out artifacts — a canonical test suite, a reference implementation — that must not live in, and cannot be edited through, the agent's own workspace. Paths escaping the copy are rejected; the stored evidence is never mutated. Pairs with the per-task Fabric workspace: seed only the agent-facing files into the workspace, keep graders' ground truth in the task's `reference`, and overlay it at scoring time. Signed-off-by: Sandy Chapman <schapman@nvidia.com> * fix(evaluator-sdk): set agent_ok on Fabric trials for AgentPhaseSuccessMetric AgentPhaseSuccessMetric scores a trial by reading candidate.metadata["agent_ok"] (an explicit bool), which the Codex runtime sets but FabricAgentRuntime did not. So agent_phase_success was False for every Fabric trial regardless of the run, sinking any task whose view ANDs it in (e.g. a fix-a-bug task) even when the agent succeeded and its tests passed. Mirror the Codex runtime: set agent_ok=True on a successful trial and agent_ok=False on a failed one. Adds a regression test that scores AgentPhaseSuccessMetric over a Fabric trial through the evaluator's own _trial_sample path. Signed-off-by: Sandy Chapman <schapman@nvidia.com> --------- Signed-off-by: Sandy Chapman <schapman@nvidia.com>
Two related changes that let Fabric run workspace-based coding-agent evaluations and grade them against held-out ground truth.
1. Per-task workspace in
FabricAgentRuntimeFabricAgentRuntimewas trajectory-only — it never seededinputs['files']and exposed noworkspacefilesystem evidence, so the workspace-scoring metrics that work against the Codex runtime couldn't score a Fabric trial. Since Fabric drives the harness on the host filesystem in a dir we choose (no container), the agent's edited files are recoverable directly.Now every task runs in its own fresh per-task workspace:
inputs['files']are seeded into it (a no-op when there are none), the harness runs in it via anenvironment.workspaceprofile overlay, and the final tree is exposed as aworkspacefilesystemEvidenceDescriptor— identical key/kind to the Codex runtime, so existing workspace-reading metrics work unchanged. Staging is unconditional (not a flag): a uniform per-task workspace matches the Codex runtime and maps cleanly onto a per-task container volume later. A config-suppliedenvironment.workspaceis overridden per task.2.
overlay_filesonLocalFilesystemEvidence.run_verifierA
{relative_path: contents}map of trusted files written over the throwaway verifier copy before the command runs — the reusable "load workspace + overlay held-out files + run verifier" primitive. This is how a grader supplies held-out artifacts (a canonical test suite, a reference implementation) that must not live in, and cannot be edited through, the agent's own workspace. Paths escaping the copy are rejected; stored evidence is never mutated.Together: seed only agent-facing files into the workspace, keep graders' ground truth in the task's
reference, overlay it at scoring time.Testing
pytest packages/nemo_evaluator_sdk/tests/agent_eval/→ 109 passed, 1 skipped (gated live test).run_verifieroverlay: held-out file appears in the copy, overlay wins over the agent's version, nested paths work, evidence untouched, path-escape rejected.ruff/ruff format/ CItyclean;make vendorsynced.Stack
main← #561 (Taskset) ← #573 ← #565 (notebook). Base retargets tomainas #561 merges.🤖 Generated with Claude Code
Summary by CodeRabbit
agent_okfor both successful and failed trials.