feat(evaluator): agent-evaluate wire publish_to_intake() to API - #1148
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:
📝 WalkthroughWalkthroughChangesAgent-evaluation specifications now support optional Intake publication and expanded aggregate score schemas. Jobs publish persisted results with stable run timestamps, structured outcomes, and configurable failure behavior. Tests cover validation, failures, timestamp propagation, and idempotent republishing. Agent evaluation publication
Sequence Diagram(s)sequenceDiagram
participant AgentEvalJob
participant publish_agent_eval_result
participant Workspace
participant Intake
AgentEvalJob->>publish_agent_eval_result: publish persisted result
publish_agent_eval_result->>Workspace: verify evaluation_id
Workspace-->>publish_agent_eval_result: evaluation exists
publish_agent_eval_result->>Intake: submit result and metadata
Intake-->>publish_agent_eval_result: return publication outcome
publish_agent_eval_result-->>AgentEvalJob: return outcome or raise required failure
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 `@plugins/nemo-evaluator/src/nemo_evaluator/jobs/publication.py`:
- Around line 149-168: Update the exception handling around the _publish call in
publish_agent_eval_result to catch unexpected Exception instances and route them
through fail, preserving the existing required=False behavior. Keep the specific
PublishError, NotFoundError, and NeMoPlatformError handlers unchanged, and
include the unexpected exception details in the fallback failure result.
🪄 Autofix
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: d5cb7e44-96b6-467b-94c3-bd8172e59a51
📒 Files selected for processing (10)
plugins/nemo-evaluator/openapi/openapi.yamlplugins/nemo-evaluator/src/nemo_evaluator/intake/mapping.pyplugins/nemo-evaluator/src/nemo_evaluator/intake/publish.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/publication.pyplugins/nemo-evaluator/tests/intake/test_mapping.pyplugins/nemo-evaluator/tests/intake/test_publish.pyplugins/nemo-evaluator/tests/integration/test_publish_to_intake.pyplugins/nemo-evaluator/tests/jobs/test_publication.py
77edb93 to
d90c4d4
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 3897-3901: The agent_name field in the OpenAPI schema must reject
empty strings. Update the corresponding IntakePublicationSpec validation and the
agent_spec identity check to enforce a non-empty value, preserve valid-name
behavior, then regenerate the OpenAPI schema.
- Around line 2678-2683: Version the aggregate-score response contract so
existing v2 clients retain the required count behavior while the
nullable/omitted count form is exposed only through a newer version. Apply the
same compatibility strategy to both the aggregate-score count schema at
plugins/nemo-evaluator/openapi/openapi.yaml:2678-2683 and the rubric-score count
schema at plugins/nemo-evaluator/openapi/openapi.yaml:2761-2766.
In `@plugins/nemo-evaluator/tests/jobs/test_publication.py`:
- Around line 398-407: The bridge test should exercise loop affinity: update
_FakeEvaluator.run_sync to actually run its coroutine on an event loop, and
update _FakeClient to bind to the first loop used and reject calls from any
different loop. Preserve the existing AgentEvalJob().run invocation so
regressions to asyncio.run or reuse of a closed-loop SDK client fail.
🪄 Autofix
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: d8dac1b9-0c46-4f5a-be1b-4d15998e524c
📒 Files selected for processing (10)
plugins/nemo-evaluator/openapi/openapi.yamlplugins/nemo-evaluator/src/nemo_evaluator/intake/mapping.pyplugins/nemo-evaluator/src/nemo_evaluator/intake/publish.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/publication.pyplugins/nemo-evaluator/tests/intake/test_mapping.pyplugins/nemo-evaluator/tests/intake/test_publish.pyplugins/nemo-evaluator/tests/integration/test_publish_to_intake.pyplugins/nemo-evaluator/tests/jobs/test_publication.py
🚧 Files skipped from review as they are similar to previous changes (8)
- plugins/nemo-evaluator/src/nemo_evaluator/intake/publish.py
- plugins/nemo-evaluator/src/nemo_evaluator/intake/mapping.py
- plugins/nemo-evaluator/src/nemo_evaluator/jobs/agent_evaluate.py
- plugins/nemo-evaluator/tests/intake/test_mapping.py
- plugins/nemo-evaluator/tests/integration/test_publish_to_intake.py
- plugins/nemo-evaluator/src/nemo_evaluator/jobs/agent_spec.py
- plugins/nemo-evaluator/src/nemo_evaluator/jobs/publication.py
- plugins/nemo-evaluator/tests/intake/test_publish.py
|
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)
plugins/nemo-evaluator/openapi/openapi.yaml (1)
2678-2683: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRegenerate the OpenAPI schema with nullable aggregate fields.
AggregateScoreBasedefinescount,sample_std_dev, andsample_varianceas nullable, and runtime code emitsnull. Update all three aggregate schemas to acceptnull, then refresh the generated OpenAPI file.🤖 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/openapi/openapi.yaml` around lines 2678 - 2683, Update the OpenAPI definitions for AggregateScoreBase and the other two aggregate schemas so count, sample_std_dev, and sample_variance accept null in addition to integer/number values, matching the nullable fields and runtime output. Regenerate the openapi.yaml artifact after updating the source schema rather than editing only the generated entry.
🤖 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 `@plugins/nemo-evaluator/openapi/openapi.yaml`:
- Around line 2678-2683: Update the OpenAPI definitions for AggregateScoreBase
and the other two aggregate schemas so count, sample_std_dev, and
sample_variance accept null in addition to integer/number values, matching the
nullable fields and runtime output. Regenerate the openapi.yaml artifact after
updating the source schema rather than editing only the generated entry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 51a0f1e0-6e9f-46b3-8021-e9a63105e0a3
📒 Files selected for processing (3)
plugins/nemo-evaluator/openapi/openapi.yamlplugins/nemo-evaluator/src/nemo_evaluator/jobs/agent_spec.pyplugins/nemo-evaluator/tests/jobs/test_publication.py
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/nemo-evaluator/src/nemo_evaluator/jobs/agent_spec.py
SandyChapman
left a comment
There was a problem hiding this comment.
thanks for adding this! I left a couple comments, but overall this lgtm!
Signed-off-by: Octavian Drulea <odrulea@nvidia.com>
Signed-off-by: Octavian Drulea <odrulea@nvidia.com>
Mechanical regeneration, unrelated to this branch's feature work. #1065 added `median`, `sample_std_dev` and `sample_variance` to `AggregateScore` in `nemo_evaluator_sdk` and reworded the variance/std_dev descriptions, but did not regenerate the evaluator plugin spec that re-exports those types. Nothing was set up to catch it: the openapi pre-commit hook is `stages: [manual]` and its `files:` pattern does not cover `packages/nemo_evaluator_sdk/`, and CI's `tools/lint/lint-openapi.sh` only diffs the platform specs, never the per-plugin ones under `plugins/*/openapi/`. Regenerating here rather than leaving it for whoever next touches this file. No publication/intake schema changes are in this diff. Signed-off-by: Octavian Drulea <odrulea@nvidia.com>
Signed-off-by: Octavian Drulea <odrulea@nvidia.com>
Signed-off-by: Octavian Drulea <odrulea@nvidia.com>
Signed-off-by: Octavian Drulea <odrulea@nvidia.com>
677ef46 to
9180e45
Compare
Summary
Agent-evaluation jobs can now publish their results to Intake, so eval runs show up under
Experiments and are associated with a specific agent. Previously
publish_to_intakeexisted butnothing called it except tests — there was no API to request publication, so Studio couldn't
trigger it.
Related Issue
https://linear.app/nvidia/issue/ASTD-384/agent-experiments-evaluator-publish-to-intake-from-api
Changes
Adds an opt-in
publication.intakeblock to the agent-evaluate job spec:{ "publication": { "intake": { "evaluation_id": "email-security-baseline", "agent_name": "email-security-analyst", "required": true } "intake": { "evaluation_id": "email-security-baseline", "agent_name": "email-security-analyst", "required": true } } }Absent → no publish, zero Intake calls. Present → the worker publishes after the run, under an
Evaluation that must already exist (it
Also fixes: publishing was not idemp
trial_to_atif_ingestemitted a step with no timestamp, so Intake fell back to a per-requestingest clock — and
spansis aReplacingMergeTreekeyed onstart_time. Re-publishing wroterows that never collapse. Measured agaise, one re-published trial:
traces.listreturned1either way, which is why this would not have been noticed. Fix is onefield: stamp
result.metadata.started_aton the step.publish_to_intakenow refuses to publishwithout it rather than writing uncollap
This matters because the recovery storye-publish" — without idempotency
that instruction corrupts the data inst
Notable decisions
requireddefaults to true. A run that asked to publish shouldn't report success withnothing in Experiments. The bundle isn runs, so a failed job still
leaves results intact to re-publish.
required: falseopts out.It's the only step that can fail the job.
,HarborRunnerTarget). Otherwiseagent_name` is required and rejected at submit with a 422 — not after a 20-minute eval.PublicationOutcome, typed with the sharedPlatformJobStatus(
COMPLETED/ERROR), not the publisher's internalPublishReport— which names theEvaluation
experiment_id, contradiche API accepts.Testing
double-publish idempotency test.
nemo services runwithout
--port, so it always bound the 8080 default and couldn't run alongside a local devplatform. Now derives the port from
NMP_BASE_URL.Follow-ups
nmp-v7j— recovery endpoint to re-pUntil it ships, recovery after afailed publication means re-running t
nmp-046— Studio submit wiring. Splg with in-flight modal changes(
nmp-bjm). Noteweb/packages/sdk/generated/is gitignored, so run`pnpm --filter @nemo/sdk gen:evaluato
Not in this PR
Trajectories published today are single; real
steps[]fromtrial.evidenceis deferred to D2 / AAscores plus a stub trace.This reverses AALGO-290's stance that py* optional (no feature flag).
The explicit opt-in spec field preserve = no publish.
Summary by CodeRabbit
New Features
Bug Fixes