feat(evaluator): add NeMo Fabric agent-eval runner with ATIF trajectory capture - #564
Conversation
📝 WalkthroughWalkthroughAdds a Fabric-backed agent runtime that runs agent-eval tasks through ChangesFabric Agent Runtime
Possible sequence sequenceDiagram
participant Job as AgentEvalJob
participant Runtime as FabricAgentRuntime
participant Client as FabricClient
participant Relay as nemo_relay.observability
participant FS as evidence directory
Job->>Runtime: run_tasks(tasks, config)
Runtime->>Runtime: build profiles
Runtime->>Relay: create trajectory profile
loop each task
Runtime->>FS: create task evidence dir
Runtime->>Client: run(input, request_id)
Client-->>Runtime: RunResult / timeout / exception
Runtime->>FS: write fabric_result.json or error.json
end
Runtime-->>Job: AgentEvalTrial list
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.
🧹 Nitpick comments (1)
plugins/nemo-evaluator/tests/test_agent_evaluate.py (1)
197-210: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTest reaches into private attrs (
_model,_work_root).Couples the test to implementation internals rather than the public contract; consistent with no bigger issue but slightly fragile.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/nemo-evaluator/tests/test_agent_evaluate.py` around lines 197 - 210, The test is asserting private internals of FabricAgentRuntime, making it fragile. Update test_resolve_target_builds_fabric_runtime_from_runner_target to validate the public behavior of AgentEvalJob._resolve_target and FabricAgentRuntime instead of _model and _work_root; use the runtime’s exposed API or observable outcomes to confirm the model and work root are set correctly, while keeping the existing assertions about prompt_template and params.
🤖 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 `@plugins/nemo-evaluator/tests/test_agent_evaluate.py`:
- Around line 197-210: The test is asserting private internals of
FabricAgentRuntime, making it fragile. Update
test_resolve_target_builds_fabric_runtime_from_runner_target to validate the
public behavior of AgentEvalJob._resolve_target and FabricAgentRuntime instead
of _model and _work_root; use the runtime’s exposed API or observable outcomes
to confirm the model and work root are set correctly, while keeping the existing
assertions about prompt_template and params.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 822cb06d-2006-4c85-ace9-cbd87253b142
⛔ Files ignored due to path filters (1)
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/runtime.pyis excluded by!sdk/**
📒 Files selected for processing (8)
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/runtime.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_integration.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_runtime.pyplugins/nemo-evaluator/openapi/openapi.yamlplugins/nemo-evaluator/src/nemo_evaluator/jobs/agent_evaluate.pyplugins/nemo-evaluator/src/nemo_evaluator/jobs/agent_spec.pyplugins/nemo-evaluator/tests/test_agent_evaluate.pyscript/dev-install-fabric.sh
|
arpitsardhana
left a comment
There was a problem hiding this comment.
It would be nice to add Fabric Profile example for different harness to get sense of different runtimes structure
…ry capture Add a `fabric` agent-eval runner kind alongside `codex`: FabricAgentRuntime drives an agent harness through the NeMo Fabric SDK, maps each RunResult to an AgentEvalTrial, and captures the agent trajectory as ATIF (NeMo Relay file exporter) into trial evidence under the standard trace key so graders can consume it. - FabricRunnerTarget(kind="fabric") + widened AgentRunnerTarget union + _resolve_target wiring - ATIF trajectory capture -> EVIDENCE_TRACE(format=atif); telemetry uri/trace_id preserved - unit tests + hermetic CI integration test + gated live fabric->codex e2e - regenerated plugin openapi + vendored SDK mirror Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Sandy Chapman <schapman@nvidia.com>
0102d00 to
229a2fe
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_runtime.py (1)
173-173: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRepeated
FabricAgentRuntime(config=_CONFIG, work_root=tmp_path / "fabric")construction.Same call repeated across 8 tests. A
runtimefixture would cut boilerplate.Also applies to: 214-214, 235-235, 254-254, 279-279, 295-295, 308-308, 323-323
🤖 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_fabric_runtime.py` at line 173, The same FabricAgentRuntime construction is repeated across multiple tests, so factor it into a shared pytest fixture in test_fabric_runtime.py. Create a fixture that builds FabricAgentRuntime with _CONFIG, model, and the tmp_path-derived work_root, then update the affected test functions to accept and use that runtime fixture instead of duplicating the setup.
🤖 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/tests/agent_eval/test_fabric_runtime.py`:
- Line 173: The same FabricAgentRuntime construction is repeated across multiple
tests, so factor it into a shared pytest fixture in test_fabric_runtime.py.
Create a fixture that builds FabricAgentRuntime with _CONFIG, model, and the
tmp_path-derived work_root, then update the affected test functions to accept
and use that runtime fixture instead of duplicating the setup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 46239b4b-58ea-4ad5-9a35-2cec62dc37e9
⛔ Files ignored due to path filters (1)
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/runtimes/fabric/runtime.pyis excluded by!sdk/**
📒 Files selected for processing (8)
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/runtime.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_integration.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_runtime.pyplugins/nemo-evaluator/openapi/openapi.yamlplugins/nemo-evaluator/src/nemo_evaluator/jobs/agent_evaluate.pyplugins/nemo-evaluator/src/nemo_evaluator/jobs/agent_spec.pyplugins/nemo-evaluator/tests/test_agent_evaluate.pyscript/dev-install-fabric.sh
🚧 Files skipped from review as they are similar to previous changes (5)
- plugins/nemo-evaluator/src/nemo_evaluator/jobs/agent_spec.py
- script/dev-install-fabric.sh
- plugins/nemo-evaluator/src/nemo_evaluator/jobs/agent_evaluate.py
- plugins/nemo-evaluator/openapi/openapi.yaml
- plugins/nemo-evaluator/tests/test_agent_evaluate.py
|
@coderabbitai The |
|
Tip For best results, initiate chat on the files or code changes.
|
Add examples/fabric_harness_runtimes.py showing Codex-CLI (subprocess, transport=cli) vs Hermes-SDK (in-library, transport=library) Fabric config shapes and building FabricAgentRuntime per harness, so callers can see how the structure differs (adapter_id, runtime.transport, harness.settings). The FabricAgentRuntime docstring now points at it. Addresses review feedback on #564. Signed-off-by: Sandy Chapman <schapman@nvidia.com>
…ry capture (#564) Add a `fabric` agent-eval runner kind alongside `codex`: FabricAgentRuntime drives an agent harness through the NeMo Fabric SDK, maps each RunResult to an AgentEvalTrial, and captures the agent trajectory as ATIF (NeMo Relay file exporter) into trial evidence under the standard trace key so graders can consume it. - FabricRunnerTarget(kind="fabric") + widened AgentRunnerTarget union + _resolve_target wiring - ATIF trajectory capture -> EVIDENCE_TRACE(format=atif); telemetry uri/trace_id preserved - unit tests + hermetic CI integration test + gated live fabric->codex e2e - regenerated plugin openapi + vendored SDK mirror Signed-off-by: Sandy Chapman <schapman@nvidia.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Add examples/fabric_harness_runtimes.py showing Codex-CLI (subprocess, transport=cli) vs Hermes-SDK (in-library, transport=library) Fabric config shapes and building FabricAgentRuntime per harness, so callers can see how the structure differs (adapter_id, runtime.transport, harness.settings). The FabricAgentRuntime docstring now points at it. Addresses review feedback on #564. Signed-off-by: Sandy Chapman <schapman@nvidia.com>
) Add examples/fabric_harness_runtimes.py showing Codex-CLI (subprocess, transport=cli) vs Hermes-SDK (in-library, transport=library) Fabric config shapes and building FabricAgentRuntime per harness, so callers can see how the structure differs (adapter_id, runtime.transport, harness.settings). The FabricAgentRuntime docstring now points at it. Addresses review feedback on NVIDIA-NeMo#564. Signed-off-by: Sandy Chapman <schapman@nvidia.com>
What
Adds a
fabricagent-eval runner kind alongsidecodex.FabricAgentRuntimedrives an agent harness through the NeMo Fabric SDK, maps eachRunResultto anAgentEvalTrial, and captures the agent trajectory as ATIF (via NeMo Relay's file exporter) into trial evidence under the standardtracekey so graders can consume it.FabricRunnerTarget(kind="fabric")+ widenedAgentRunnerTargetunion +_resolve_targetwiringRunResult/FabricClienttypes (TYPE_CHECKING + lazy import;nemo_fabricis an optional native dep)EVIDENCE_TRACE(format=atif);telemetry.uri/trace_idpreservedfabric→codexe2eopenapi.yaml+ vendored SDK mirrorVerification
ruff/tyclean.fabric→codex→Relaye2e ran green locally (real 330 KB ATIF captured + scored).Notes / follow-ups
nemo-fabric, thecodexCLI, and thenemo-relaygateway binary (not pip-installable). Adapter discovery also relies on a build-time path orconfig_root/adapters, so a deployed image must shipadapters/(or we add abase_dirknob). Not resolved here.run()+submit()live integration tests, coupled with the adapter-shipping/base_dirdecision; plus an ATIF-vs-SDK-TraceHandleschema-compat check.🤖 Generated with Claude Code
Summary by CodeRabbit