feat(evaluator-sdk): allow a prebuilt image for FabricContainerRuntime - #784
Conversation
|
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:
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 (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough
ChangesFabric image override
Sequence Diagram(s)sequenceDiagram
participant Caller
participant FabricContainerRuntime
participant ensure_fabric_image
participant SandboxProvider
Caller->>FabricContainerRuntime: provide image override or none
alt image override supplied
FabricContainerRuntime->>SandboxProvider: create SandboxSpec with supplied image
else no override
FabricContainerRuntime->>ensure_fabric_image: provision default image
FabricContainerRuntime->>SandboxProvider: create SandboxSpec with default image
end
SandboxProvider-->>FabricContainerRuntime: return sandbox result
FabricContainerRuntime->>FabricContainerRuntime: _to_trial adds selected image metadata
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.
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/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/container_runtime.py`:
- Around line 109-112: The _run_task failure path currently calls _failed_trial
without the selected image metadata. Reuse the base metadata containing
self._image when constructing failed trials, including failures before _to_trial
during startup, seeding, or download, and add a regression test covering an
early failure that verifies trial.metadata["image"].
🪄 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: ad991084-7d3d-488a-9cae-6455c3b11237
📒 Files selected for processing (2)
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/runtimes/fabric/container_runtime.pypackages/nemo_evaluator_sdk/tests/agent_eval/test_fabric_container_runtime.py
|
521ee45 to
79cde46
Compare
Add an optional keyword-only `image` param so a caller can supply a prebuilt tag (extra tooling, pinned base) instead of the build-if-missing stock image; the trial runs inside it. Backward-compatible (None = current behavior). Trials that fail before _to_trial now also carry the runtime + image metadata. Covered by new tests. Signed-off-by: Sandy Chapman <schapman@nvidia.com>
79cde46 to
908a2b5
Compare
What
FabricContainerRuntimealways provisioned the harness-agnostic Fabric image (build-if-missing). This adds an optional keyword-onlyimageparameter so a caller can supply a prebuilt image tag when the sandbox needs extra tooling — e.g. a document-processing image (pandoc/libreoffice), a pinned base, or an image from an air-gapped registry.Behavior
imagedefaults toNone; when unset, behavior is unchanged (ensure_fabric_image()build-if-missing on first run).Tests
Two new cases in
test_fabric_container_runtime.py(existing fake-provider harness):SandboxSpecand short-circuitsensure_fabric_image;pytestgreen (17/17 in the file);ruff checkandruff format --checkclean.Summary by CodeRabbit
trialmetadata.trialmetadata.