feat(evaluator): Harbor runner target + agent-eval job list & create-serialization fixes - #723
Merged
SandyChapman merged 3 commits intoJul 21, 2026
Conversation
Contributor
|
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 selected for processing (1)
📝 WalkthroughWalkthroughAdds Harbor-backed agent evaluation targets and runtime wiring, separates agent-evaluate job records, adds Harbor execution coverage, and validates JSON-mode serialization of byte-containing job specifications. ChangesHarbor evaluation integration
Created job serialization
Sequence Diagram(s)sequenceDiagram
participant AgentEvalJob
participant HarborAgentTaskRunner
participant HarborRuntimeConfig
participant LocalJobResults
AgentEvalJob->>HarborAgentTaskRunner: resolve HarborRunnerTarget
AgentEvalJob->>HarborRuntimeConfig: configure Harbor runtime
HarborAgentTaskRunner->>LocalJobResults: persist evaluation artifacts and reward scores
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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: 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 `@plugins/nemo-evaluator/src/nemo_evaluator/service.py`:
- Around line 52-55: Update the agent-evaluate setup around add_job_routes and
AGENT_EVAL_JOB_SOURCE to include a migration or re-sourcing path for existing
agent-evaluate records currently tagged nemo-evaluator. Ensure legacy records
are moved to the dedicated agent-evaluate source before relying on the new
source partition, while preserving EvaluateJob’s derived nemo-evaluator source.
🪄 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: 70cf17da-2486-411d-bce3-0377229079a5
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
packages/nemo_evaluator_sdk/pyproject.tomlpackages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/api_factory.pypackages/nemo_platform_plugin/tests/test_jobs_filter.pyplugins/nemo-evaluator/src/nemo_evaluator/jobs/agent_evaluate.pyplugins/nemo-evaluator/src/nemo_evaluator/jobs/agent_spec.pyplugins/nemo-evaluator/src/nemo_evaluator/jobs/result_persistence.pyplugins/nemo-evaluator/src/nemo_evaluator/service.pyplugins/nemo-evaluator/tests/integration/test_agent_evaluate_job.pyplugins/nemo-evaluator/tests/integration/test_harbor_plugin_run.pyplugins/nemo-evaluator/tests/test_agent_evaluate.pyplugins/nemo-evaluator/tests/test_result_persistence.py
Contributor
|
ngoncharenko
approved these changes
Jul 20, 2026
arpitsardhana
approved these changes
Jul 20, 2026
…create serialization Add HarborRunnerTarget to the agent-eval runner-target union and wire it through _resolve_target (native HarborAgentTaskRunner, jobs_dir injected from job storage) and result-entity trait extraction. Declare a marker-gated `harbor` extra on nemo-evaluator-sdk (`harbor>=0.16.1; python_version >= '3.12'`) so it installs on 3.12 without raising the 3.11 workspace floor; harbor stays lazily imported. Fix a 500 on the agent-evaluate/evaluate job list endpoints in a mixed workspace: the evaluator plugin owns two job types that shared one `source` tag, so each collection's list returned the other type's records and 500'd rendering the foreign spec. Give AgentEvalJob a distinct source via `add_job_routes(service_name=...)`; EvaluateJob keeps its derived source (existing records still match). Fix plugin job create for bytes-bearing specs (inline cloudpickle metric bundles): the factory coerced the transformed spec with model_dump() (python mode), leaving raw bytes the typed client couldn't JSON-encode (invalid utf-8 on the pickle marker). Dump mode="json" so nested payload models base64-encode and round-trip. Tests: Harbor resolve/compile/run_local unit coverage plus a real Docker-backed plugin e2e via NemoJobScheduler().run_local (gated on harbor + docker); mixed-job list partitioning against a live platform; and a create-path bytes-serialization regression driving the real POST /jobs route. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Sandy Chapman <schapman@nvidia.com>
The HarborRunnerTarget added to the agent-eval Target union flows into the evaluator plugin's OpenAPI spec (consumed by the web SDK generation check). Regenerated via 'make refresh-openapi'; no web SDK TS diff results. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Sandy Chapman <schapman@nvidia.com>
SandyChapman
force-pushed
the
harbor-runner-target-and-evaluator-fixes/schapman
branch
from
July 20, 2026 20:18
e87ad66 to
5afb9ce
Compare
SandyChapman
enabled auto-merge
July 20, 2026 20:18
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@plugins/nemo-evaluator/openapi/openapi.yaml`:
- Around line 3152-3196: Update the OpenAPI schema properties agent_name,
agent_import_path, agent_model_name, and trace_dir to accept both string and
null values, matching their str | None definitions in agent_spec.py. Preserve
their existing descriptions, defaults, and other validation settings while
adding the nullable type alternative.
In `@plugins/nemo-evaluator/tests/integration/test_harbor_plugin_run.py`:
- Around line 43-46: Update _docker_available to pass a short timeout to the
subprocess.run Docker probe, and catch subprocess.TimeoutExpired to return
False. Preserve the existing unavailable result for missing Docker or nonzero
docker info exit codes.
🪄 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: b511b381-b580-4a74-91e5-144a7446e5d3
📒 Files selected for processing (12)
packages/nemo_evaluator_sdk/pyproject.tomlpackages/nemo_platform_plugin/src/nemo_platform_plugin/jobs/api_factory.pypackages/nemo_platform_plugin/tests/test_jobs_filter.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/src/nemo_evaluator/jobs/result_persistence.pyplugins/nemo-evaluator/src/nemo_evaluator/service.pyplugins/nemo-evaluator/tests/integration/test_agent_evaluate_job.pyplugins/nemo-evaluator/tests/integration/test_harbor_plugin_run.pyplugins/nemo-evaluator/tests/test_agent_evaluate.pyplugins/nemo-evaluator/tests/test_result_persistence.py
🚧 Files skipped from review as they are similar to previous changes (7)
- plugins/nemo-evaluator/src/nemo_evaluator/service.py
- plugins/nemo-evaluator/src/nemo_evaluator/jobs/agent_evaluate.py
- packages/nemo_evaluator_sdk/pyproject.toml
- plugins/nemo-evaluator/src/nemo_evaluator/jobs/agent_spec.py
- plugins/nemo-evaluator/src/nemo_evaluator/jobs/result_persistence.py
- plugins/nemo-evaluator/tests/test_agent_evaluate.py
- packages/nemo_platform_plugin/tests/test_jobs_filter.py
A wedged Docker daemon could make the 'docker info' skip probe hang until the test-level timeout. Pass a short timeout and treat TimeoutExpired as Docker unavailable so the test skips cleanly instead of blocking. Signed-off-by: Sandy Chapman <schapman@nvidia.com>
SandyChapman
deleted the
harbor-runner-target-and-evaluator-fixes/schapman
branch
July 21, 2026 11:18
This was referenced Jul 28, 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
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.
Summary
Three related changes to the evaluator plugin's agent-eval path:
1. Harbor runner target (feature)
HarborRunnerTargetto the agent-eval runner-target union and wires it through_resolve_target(nativeHarborAgentTaskRunner,jobs_dirinjected from job storage) and result-entity trait extraction (also fills in the previously-missing Fabric trait branch).harborextra onnemo-evaluator-sdk:harbor = ["harbor>=0.16.1; python_version >= '3.12'"]. Harbor requires Python ≥3.12; the marker scopes it to the 3.12 slice of the universal lock sopip install nemo-evaluator-sdk[harbor]works on 3.12 while the workspace floor stays 3.11 (no floor bump).harborstays lazily imported, so importing the SDK / resolving a Harbor target is valid on 3.11 — only executing a run needs the extra.2. Mixed-job list-endpoint 500 (bug fix — reported by Studio)
The evaluator plugin owns two job types (
evaluate+agent-evaluate) that shared onesourcetag. The list endpoints filter bysource, so in a workspace with both, each collection returned the other type's records and 500'd rendering the foreign spec (dozens of validation errors). Fix: giveAgentEvalJoba distinct source viaadd_job_routes(service_name=AGENT_EVAL_JOB_SOURCE).EvaluateJobkeeps its derivednemo-evaluatorsource so existing records still match.3. Plugin job create serialization for bytes-bearing specs (latent bug fix)
The shared job factory coerced the transformed spec with
model_dump()(python mode), leaving rawbytes(e.g. an inline cloudpickle metric bundle) that the typed jobs client couldn't JSON-encode —PydanticSerializationError: invalid utf-8on the\x80pickle marker. Fix: dumpmode="json"so nested payload models'ser_json_bytes="base64"runs and the blob round-trips. This is in the sharedapi_factory, so it fixes create for any plugin/job type whose spec carries bytes. (Latent because the opt-in E2E suite that exercises it isn't in CI.)Testing
run_local(faked evaluator) across target types; harbor/fabric result traits; distinct-source list-filter forwarding;nemo-evaluator-sdksource distinctness; create-path bytes-serialization regression driving the realPOST /jobsroute.NemoJobScheduler().run_local(...)(gated onharbor+ Docker; oracle solves hello-world, reward 1.0 persisted).uv lock --checkclean; ruff + ty clean for changed files.Follow-up
run_local/ subprocess backend) but not inside thecpu-taskscontainer; direction is to route Harbor at a remote/k8s sandbox rather than plugin-side DinD.🤖 Generated with Claude Code
Summary by CodeRabbit