feat(studio): move agent eval over to evaluator endpoints#775
Open
nv-odrulea wants to merge 1 commit into
Open
feat(studio): move agent eval over to evaluator endpoints#775nv-odrulea wants to merge 1 commit into
nv-odrulea wants to merge 1 commit into
Conversation
Signed-off-by: Octavian Drulea <odrulea@nvidia.com>
Contributor
📝 WalkthroughWalkthroughChangesAgent evaluation workflow
Upload picker timestamps
Phishing-agent token limits
Sequence Diagram(s)sequenceDiagram
participant SubmitEvaluationModal
participant EvaluationAPI
participant Evaluator
participant Fileset
SubmitEvaluationModal->>EvaluationAPI: build and submit evaluation job
EvaluationAPI->>Evaluator: create agent-evaluate job
EvaluationAPI->>Evaluator: fetch status and aggregate results
EvaluationAPI->>Fileset: download task result JSONL
EvaluationAPI-->>SubmitEvaluationModal: return evaluation data
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 16
🧹 Nitpick comments (1)
web/packages/studio/src/routes/agents/AgentEvaluationsRoute/AGENTS.md (1)
1-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftSplit explanation from API reference and add the required page framing.
This page mixes conceptual explanation with endpoint, payload, result, and legacy reference specifications. Keep one Diataxis quadrant, move reference material to a linked page, list prerequisites first, and add Next Steps.
As per coding guidelines, each page must fit one Diataxis quadrant, list prerequisites at the top, and end with Next Steps.
Also applies to: 70-110, 229-307
🤖 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 `@web/packages/studio/src/routes/agents/AgentEvaluationsRoute/AGENTS.md` around lines 1 - 5, Restructure the Agent Evaluation documentation to keep this page focused on conceptual explanation: add a prerequisites section near the top, move endpoint, payload, result, and legacy specifications into a separate linked API reference page, and finish with a Next Steps section. Apply the same separation and framing to the referenced sections while preserving the existing evaluation guidance.Source: Coding guidelines
🤖 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
`@web/packages/common/src/components/UploadModal/Context/useUploadModalReducer.ts`:
- Line 55: Update the RESET handling in useUploadModalReducer so the reset state
preserves the existing showUpdatedAt option alongside acceptableFileTypes and
allowNewDataset. Ensure consumers that enabled updated-at timestamps retain that
setting after RESET.
In `@web/packages/studio/public/sample-agents/calculator/eval-config.json`:
- Around line 58-59: Remove the embedded judge model and max_concurrent_tasks
bindings from the reusable evaluation configs. In
web/packages/studio/public/sample-agents/calculator/eval-config.json at lines
58-59 and 88, and
web/packages/studio/public/sample-agents/email-phishing-analyzer/eval-config.json
at lines 167-168 and 198, delete those fields while preserving the surrounding
configuration so submission can inject the selected judge model and effective
concurrency.
In
`@web/packages/studio/public/sample-agents/email-phishing-analyzer/eval-config-dataset.json`:
- Around line 2-10: Update
web/packages/studio/public/sample-agents/email-phishing-analyzer/eval-config-dataset.json
lines 2-10 to remove dataset_file and prompt_template, replacing them with
inline tasks[] entries compatible with agent-evaluate. Update
web/packages/studio/src/api/evaluation/eval-config-fileset.ts lines 27-68 to
replace the seeded react-eval.yml with an agent-evaluate-compatible
configuration using inline tasks[] and an inline metric; do not reference a
dataset, Fileset, or Taskset.
In
`@web/packages/studio/public/sample-agents/email-phishing-analyzer/eval-spec.wip.json`:
- Around line 12-50: Update the eval spec’s metrics configuration, including all
remaining task metric blocks, to use a single root inline metric with type
“llm-judge” instead of per-task “answer_accuracy” metrics. Remove embedded judge
model, target, and concurrency settings from the spec; rely on submission-time
injection for the judge model, agent target, and effective max_concurrent_tasks.
In `@web/packages/studio/src/api/evaluation/agent-evaluations.ts`:
- Around line 236-260: Update the trial-to-score join in the surrounding mapping
logic so scores are matched by trial_id rather than task_id alone. Either return
separate details for each trial or, if retaining one selected trial per task,
filter taskScores to that trial before building scores and diagnostics, ensuring
responseText and attached scores come from the same trial.
- Around line 42-46: Update agentNameForJob to remove only the current job
workspace prefix from the returned agent name before useAgentPanel compares it
with the bare name. Preserve the existing target validation and return null
behavior, and avoid stripping unrelated portions of the agent name.
In `@web/packages/studio/src/api/evaluation/eval-config-fileset.ts`:
- Around line 124-127: Update the catch block in the fileset creation flow to
re-throw every creation error except an explicit HTTP 409 conflict; retain
cancellation propagation and only ignore the confirmed concurrent-creation
conflict, rather than treating 403 or 5xx responses as successful.
In
`@web/packages/studio/src/routes/agents/AgentEvaluationsRoute/AgentEvaluationDetailRoute.tsx`:
- Around line 194-202: Replace empty-string fallbacks passed to RelativeTime
with conditional rendering: in
web/packages/studio/src/routes/agents/AgentEvaluationsRoute/AgentEvaluationDetailRoute.tsx
lines 194-202, render each KVPair fallback when created_at or updated_at is
absent; in
web/packages/studio/src/routes/agents/AgentEvaluationsRoute/AgentEvaluationsListRoute.tsx
line 213 and
web/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/AgentDetailsContent.tsx
line 212, render — instead of RelativeTime when created_at is absent.
- Around line 89-100: Update the result and bundle queries in
AgentEvaluationDetailRoute to use a bounded refetch interval whenever their
respective data is null, allowing newly available artifacts to be discovered
after a terminal job. Stop polling once non-null data is returned, while
preserving the existing query gating and fetch behavior.
In `@web/packages/studio/src/routes/agents/AgentEvaluationsRoute/AGENTS.md`:
- Around line 38-53: Update all JSON examples in AGENTS.md, including the shared
metric and task examples, to be valid executable JSON by removing comments and
non-JSON placeholders; use concrete representative values where needed rather
than changing the fences to jsonc. Apply this consistently to the referenced
example sections.
- Line 3: Replace every hardcoded “Studio” product-name reference in AGENTS.md,
including the introductory text and the additional referenced sections, with the
repository’s configured Sphinx product-name substitution. Preserve the existing
wording and formatting aside from using the substitution consistently.
In
`@web/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/AgentEvalScoresPanel.tsx`:
- Around line 23-40: Update the score rendering in AgentEvalScoresPanel’s
scores.map callback to branch on each score’s score_type. For rubric scores,
render the rubric_distribution and mode_category data in addition to the shared
numeric statistics, while preserving the existing numeric-score rendering. Add
or update a rubric-focused test covering these fields.
In
`@web/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/SubmitEvaluationModal.tsx`:
- Around line 118-131: Update the MODE_DEFAULT branch in SubmitEvaluationModal
so name collisions are rejected rather than returning the newly selected sample
content after ensureEvalConfigFileset preserves an existing eval-config.json.
Validate that the fileset name is available before submission and surface the
existing validation/error path, or retrieve and return the persisted file
content when the name already exists; ensure the submitted content always
matches the referenced fileset.
- Around line 189-208: Update SubmitEvaluationModal’s submission and validation
flow to derive the metric type from the config being submitted, including
fileset mode, rather than relying only on the preview query. Keep the judge
selector visible and required for llm-judge metrics, block submission while the
metric type/config is unresolved, and inject the selected judge model into the
submitted config at submission time.
In
`@web/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/submitEvaluationSpec.ts`:
- Around line 120-129: Update buildAgentEvalRequestBody so
spec.max_concurrent_tasks is injected at submission with an effective value of
1, rather than reading config.max_concurrent_tasks or
DEFAULT_MAX_CONCURRENT_TASKS from the reusable EvalConfig. Keep the remaining
request construction unchanged.
- Around line 132-145: Update parseEvalConfig to fully validate the parsed
EvalConfig before returning it: reject malformed task entries, invalid inline
payload structures, unsupported metric types, and numeric options outside their
allowed bounds. Ensure invalid metric payloads and task definitions fail at this
JSON boundary rather than reaching request construction, while preserving the
existing required non-empty tasks and metric checks.
---
Nitpick comments:
In `@web/packages/studio/src/routes/agents/AgentEvaluationsRoute/AGENTS.md`:
- Around line 1-5: Restructure the Agent Evaluation documentation to keep this
page focused on conceptual explanation: add a prerequisites section near the
top, move endpoint, payload, result, and legacy specifications into a separate
linked API reference page, and finish with a Next Steps section. Apply the same
separation and framing to the referenced sections while preserving the existing
evaluation guidance.
🪄 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: 2f7fd330-bbcc-4d83-9d1b-e2b8426f41a5
📒 Files selected for processing (44)
plugins/nemo-agents/examples/email-phishing-analyzer/src/nat_email_phishing_analyzer/email-phishing-agent.ymlweb/packages/common/src/components/DatasetFileSelect/ControlledDatasetFileSelect.tsxweb/packages/common/src/components/DatasetFileSelect/DatasetFileSelect.tsxweb/packages/common/src/components/UploadModal/Context/useUploadModalReducer.tsweb/packages/common/src/components/UploadModal/DatasetUploader/Select.tsxweb/packages/common/src/components/UploadModal/InlineUploadPicker.tsxweb/packages/common/src/components/UploadModal/types.tsweb/packages/studio/public/sample-agents/calculator/eval-config.jsonweb/packages/studio/public/sample-agents/email-phishing-analyzer/agent.ymlweb/packages/studio/public/sample-agents/email-phishing-analyzer/eval-config-dataset.jsonweb/packages/studio/public/sample-agents/email-phishing-analyzer/eval-config.jsonweb/packages/studio/public/sample-agents/email-phishing-analyzer/eval-spec.wip.jsonweb/packages/studio/src/api/evaluation/agent-evaluations.test.tsweb/packages/studio/src/api/evaluation/agent-evaluations.tsweb/packages/studio/src/api/evaluation/eval-config-fileset.tsweb/packages/studio/src/components/agents/AgentBlockingInput/EvalConfigBlockingInput.test.tsxweb/packages/studio/src/components/agents/AgentBlockingInput/EvalConfigBlockingInput.tsxweb/packages/studio/src/components/evaluation/JudgeModelSelect.tsxweb/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/AgentDetailsContent.tsxweb/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/index.test.tsxweb/packages/studio/src/components/sidePanels/AgentPanels/AgentPanel/useAgentPanel.tsweb/packages/studio/src/constants/sampleAgents.test.tsweb/packages/studio/src/constants/sampleAgents.tsweb/packages/studio/src/mocks/handlers.tsweb/packages/studio/src/routes/agents/AgentEvaluationsRoute/AGENTS.mdweb/packages/studio/src/routes/agents/AgentEvaluationsRoute/AgentEvaluationDetailRoute.tsxweb/packages/studio/src/routes/agents/AgentEvaluationsRoute/AgentEvaluationsListRoute.tsxweb/packages/studio/src/routes/agents/AgentEvaluationsRoute/api.test.tsweb/packages/studio/src/routes/agents/AgentEvaluationsRoute/api.tsweb/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/AgentEvalScoresPanel.test.tsxweb/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/AgentEvalScoresPanel.tsxweb/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/AgentEvalTaskResultsPanel.tsxweb/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/EvalConfigFilesPanel.tsxweb/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/EvaluatorOutputPanel.tsxweb/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/EvaluatorReasoning.tsxweb/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/SubmitEvaluationModal.tsxweb/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/WorkflowOutputPanel.tsxweb/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/submitEvaluationSpec.test.tsweb/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/submitEvaluationSpec.tsweb/packages/studio/src/routes/agents/AgentSuggestionsRoute/api.tsweb/packages/studio/src/routes/agents/AgentSuggestionsRoute/constants.tsweb/packages/studio/src/routes/agents/AgentSuggestionsRoute/useOptimizerSuggestions.test.tsxweb/packages/studio/src/routes/agents/AgentSuggestionsRoute/useOptimizerSuggestions.tsweb/packages/studio/src/routes/agents/AgentSuggestionsRoute/utils.ts
💤 Files with no reviewable changes (8)
- web/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/EvaluatorReasoning.tsx
- web/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/EvalConfigFilesPanel.tsx
- web/packages/studio/src/routes/agents/AgentEvaluationsRoute/api.test.ts
- web/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/EvaluatorOutputPanel.tsx
- web/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/WorkflowOutputPanel.tsx
- web/packages/studio/src/routes/agents/AgentSuggestionsRoute/constants.ts
- web/packages/studio/src/routes/agents/AgentSuggestionsRoute/api.ts
- web/packages/studio/src/routes/agents/AgentEvaluationsRoute/api.ts
| datasetLabel?: string; | ||
| /** Auto-select the first root-level accepted file on fileset selection. */ | ||
| autoSelectFirstAcceptable?: boolean; | ||
| showUpdatedAt?: boolean; |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
showUpdatedAt is lost on RESET.
The RESET action preserves acceptableFileTypes, allowNewDataset, etc., but omits showUpdatedAt. After a reset, the dataset picker options will silently drop the updated-at timestamps — a visual regression for consumers that opt in via showUpdatedAt.
🛡️ Proposed fix
case 'RESET':
// Preserve configuration props that were passed as props to UploadModal
return {
...initialState,
acceptableFileTypes: state.acceptableFileTypes,
acceptableFileSize: state.acceptableFileSize,
allowMultipleFileSelection: state.allowMultipleFileSelection,
invalidFileMode: state.invalidFileMode,
allowNewDataset: state.allowNewDataset,
+ showUpdatedAt: state.showUpdatedAt,
};Also applies to: 101-110
🤖 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
`@web/packages/common/src/components/UploadModal/Context/useUploadModalReducer.ts`
at line 55, Update the RESET handling in useUploadModalReducer so the reset
state preserves the existing showUpdatedAt option alongside acceptableFileTypes
and allowNewDataset. Ensure consumers that enabled updated-at timestamps retain
that setting after RESET.
Comment on lines
+58
to
+59
| "model": "default/nvidia-nemotron-3-super-120b-a12b", | ||
| "prompt_template": { |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remove submission-time bindings from reusable configs.
These files pin the judge model and concurrency, preventing submission from reliably applying the selected judge and effective concurrency.
web/packages/studio/public/sample-agents/calculator/eval-config.json#L58-L59: remove the embedded judge model.web/packages/studio/public/sample-agents/calculator/eval-config.json#L88-L88: removemax_concurrent_tasks.web/packages/studio/public/sample-agents/email-phishing-analyzer/eval-config.json#L167-L168: remove the embedded judge model.web/packages/studio/public/sample-agents/email-phishing-analyzer/eval-config.json#L198-L198: removemax_concurrent_tasks.
Based on learnings: “Inject the agent target, judge model, and effective max_concurrent_tasks at submission time rather than storing them in reusable evaluation configs.”
📍 Affects 2 files
web/packages/studio/public/sample-agents/calculator/eval-config.json#L58-L59(this comment)web/packages/studio/public/sample-agents/calculator/eval-config.json#L88-L88web/packages/studio/public/sample-agents/email-phishing-analyzer/eval-config.json#L167-L168web/packages/studio/public/sample-agents/email-phishing-analyzer/eval-config.json#L198-L198
🤖 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 `@web/packages/studio/public/sample-agents/calculator/eval-config.json` around
lines 58 - 59, Remove the embedded judge model and max_concurrent_tasks bindings
from the reusable evaluation configs. In
web/packages/studio/public/sample-agents/calculator/eval-config.json at lines
58-59 and 88, and
web/packages/studio/public/sample-agents/email-phishing-analyzer/eval-config.json
at lines 167-168 and 198, delete those fields while preserving the surrounding
configuration so submission can inject the selected judge model and effective
concurrency.
Source: Learnings
Comment on lines
+2
to
+10
| "dataset_file": "smaller_test.csv", | ||
| "prompt_template": { | ||
| "messages": [ | ||
| { | ||
| "role": "user", | ||
| "content": "{{ item.body }}" | ||
| } | ||
| ] | ||
| }, |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Use the agent-evaluate config shape for sample fallbacks.
Both samples use legacy dataset-driven evaluation configuration, which agent-evaluate does not support.
web/packages/studio/public/sample-agents/email-phishing-analyzer/eval-config-dataset.json#L2-L10: replacedataset_fileand the prompt template with inlinetasks[].web/packages/studio/src/api/evaluation/eval-config-fileset.ts#L27-L68: replace the seededreact-eval.ymlwith an agent-evaluate-compatible config containing inline tasks and an inline metric.
Based on learnings: “Use inline tasks[] for agent-evaluate jobs; do not configure a dataset, Fileset, or Taskset reference because those references are not implemented.”
📍 Affects 2 files
web/packages/studio/public/sample-agents/email-phishing-analyzer/eval-config-dataset.json#L2-L10(this comment)web/packages/studio/src/api/evaluation/eval-config-fileset.ts#L27-L68
🤖 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
`@web/packages/studio/public/sample-agents/email-phishing-analyzer/eval-config-dataset.json`
around lines 2 - 10, Update
web/packages/studio/public/sample-agents/email-phishing-analyzer/eval-config-dataset.json
lines 2-10 to remove dataset_file and prompt_template, replacing them with
inline tasks[] entries compatible with agent-evaluate. Update
web/packages/studio/src/api/evaluation/eval-config-fileset.ts lines 27-68 to
replace the seeded react-eval.yml with an agent-evaluate-compatible
configuration using inline tasks[] and an inline metric; do not reference a
dataset, Fileset, or Taskset.
Source: Learnings
Comment on lines
+12
to
+50
| "metrics": [ | ||
| { | ||
| "bundle_kind": "metric-bundle", | ||
| "bundle_format_version": "v1", | ||
| "metric_type": "answer_accuracy", | ||
| "metadata": { | ||
| "description": null, | ||
| "labels": {} | ||
| }, | ||
| "outputs": [ | ||
| { | ||
| "name": "answer_accuracy", | ||
| "description": null, | ||
| "value_json_schema": { | ||
| "description": "Continuous numeric metric value.", | ||
| "title": "ContinuousScore", | ||
| "type": "number" | ||
| } | ||
| } | ||
| ], | ||
| "secrets": {}, | ||
| "payload": { | ||
| "kind": "inline", | ||
| "metric": { | ||
| "type": "answer_accuracy", | ||
| "judge_model": { | ||
| "url": "http://localhost:8080/apis/inference-gateway/v2/workspaces/default/openai/-/v1", | ||
| "name": "nvidia-nemotron-3-super-120b-a12b", | ||
| "format": "nim" | ||
| }, | ||
| "inference": {}, | ||
| "input_template": { | ||
| "user_input": "{{ item.inputs.instruction }}", | ||
| "response": "{{ sample.output_text }}", | ||
| "reference": "{{ item.reference.label }}" | ||
| } | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Migrate or remove this legacy spec.
answer_accuracy per-task metrics and embedded judge/target settings do not match the agent-evaluate contract. Use one root inline llm-judge metric; inject the judge, target, and concurrency during submission. Apply this to the remaining task metric blocks too.
Based on learnings: “Define llm-judge metrics with the hyphenated type name” and “Inject the agent target, judge model, and effective max_concurrent_tasks at submission time.”
🤖 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
`@web/packages/studio/public/sample-agents/email-phishing-analyzer/eval-spec.wip.json`
around lines 12 - 50, Update the eval spec’s metrics configuration, including
all remaining task metric blocks, to use a single root inline metric with type
“llm-judge” instead of per-task “answer_accuracy” metrics. Remove embedded judge
model, target, and concurrency settings from the spec; rely on submission-time
injection for the judge model, agent target, and effective max_concurrent_tasks.
Source: Learnings
Comment on lines
+42
to
+46
| export const agentNameForJob = (job: AgentEvalJob): string | null => { | ||
| const target = job.spec.target as { kind?: string; agent?: { name?: string } } | null | undefined; | ||
| if (!target || target.kind !== 'agent') return null; | ||
| const name = target.agent?.name; | ||
| return typeof name === 'string' && name.length > 0 ? name : null; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve workspace-prefixed agent matching.
This returns workspace/agent, but useAgentPanel now compares it directly with the bare agent name, hiding matching evaluations. Strip only the current job workspace prefix.
Proposed fix
const name = target.agent?.name;
- return typeof name === 'string' && name.length > 0 ? name : null;
+ if (typeof name !== 'string' || name.length === 0) return null;
+ const prefix = job.workspace ? `${job.workspace}/` : '';
+ return prefix && name.startsWith(prefix) ? name.slice(prefix.length) : name;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const agentNameForJob = (job: AgentEvalJob): string | null => { | |
| const target = job.spec.target as { kind?: string; agent?: { name?: string } } | null | undefined; | |
| if (!target || target.kind !== 'agent') return null; | |
| const name = target.agent?.name; | |
| return typeof name === 'string' && name.length > 0 ? name : null; | |
| export const agentNameForJob = (job: AgentEvalJob): string | null => { | |
| const target = job.spec.target as { kind?: string; agent?: { name?: string } } | null | undefined; | |
| if (!target || target.kind !== 'agent') return null; | |
| const name = target.agent?.name; | |
| if (typeof name !== 'string' || name.length === 0) return null; | |
| const prefix = job.workspace ? `${job.workspace}/` : ''; | |
| return prefix && name.startsWith(prefix) ? name.slice(prefix.length) : name; |
🤖 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 `@web/packages/studio/src/api/evaluation/agent-evaluations.ts` around lines 42
- 46, Update agentNameForJob to remove only the current job workspace prefix
from the returned agent name before useAgentPanel compares it with the bare
name. Preserve the existing target validation and return null behavior, and
avoid stripping unrelated portions of the agent name.
Comment on lines
+23
to
+40
| {scores.map((s) => { | ||
| const total = s.count + s.nan_count; | ||
| return ( | ||
| <Flex key={s.name} justify="between" align="center" gap="density-md" wrap="wrap"> | ||
| <Stack gap="density-xs" className="min-w-0"> | ||
| <Text kind="body/semibold/md" className="truncate"> | ||
| {s.name} | ||
| </Text> | ||
| <Text kind="body/regular/sm" color="secondary"> | ||
| {s.count}/{total} scored | ||
| {typeof s.min === 'number' && typeof s.max === 'number' | ||
| ? ` · range ${formatScore(s.min)}–${formatScore(s.max)}` | ||
| : ''} | ||
| </Text> | ||
| </Stack> | ||
| <Badge kind="solid" color={scoreColor(s.mean)}> | ||
| {formatScore(s.mean)} | ||
| </Badge> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C4 'AggregateRangeScore|AggregateRubricScore' webRepository: NVIDIA-NeMo/nemo-platform
Length of output: 2010
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== AgentEvalScoresPanel.tsx ==\n'
sed -n '1,220p' web/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/AgentEvalScoresPanel.tsx
printf '\n== agent-evaluations.ts ==\n'
sed -n '1,220p' web/packages/studio/src/api/evaluation/agent-evaluations.ts
printf '\n== Search for score_type usage ==\n'
rg -n -C3 'score_type|AggregateRangeScore|AggregateRubricScore|scoreColor\\(' web/packages/studio/srcRepository: NVIDIA-NeMo/nemo-platform
Length of output: 9939
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== score_type references ==\n'
rg -n -C2 'score_type|scoreType|AggregateRangeScore|AggregateRubricScore' web/packages/studio/src web/packages/studio/src/api web/packages/studio/src/routes
printf '\n== SDK schema files ==\n'
fd -a 'schema.*' web/packages/studio/src/node_modules web/packages/studio/src 2>/dev/null | head -n 50 || true
printf '\n== generated evaluator schema hits ==\n'
rg -n -C3 'interface AggregateRangeScore|interface AggregateRubricScore|type AggregateRangeScore|type AggregateRubricScore|score_type' web/packages/studio/srcRepository: NVIDIA-NeMo/nemo-platform
Length of output: 22984
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== score_type references ==\n'
rg -n -C2 'score_type|scoreType|AggregateRangeScore|AggregateRubricScore' web/packages/studio/src web/packages/studio/src/api web/packages/studio/src/routes || true
printf '\n== generated evaluator schema hits ==\n'
rg -n -C3 'interface AggregateRangeScore|interface AggregateRubricScore|type AggregateRangeScore|type AggregateRubricScore|score_type' web/packages/studio/src || trueRepository: NVIDIA-NeMo/nemo-platform
Length of output: 21609
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C3 'AggregateRangeScore|AggregateRubricScore|score_type|scoreType' .Repository: NVIDIA-NeMo/nemo-platform
Length of output: 50383
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C3 'AggregateRangeScore|AggregateRubricScore|score_type|scoreType' web packages .Repository: NVIDIA-NeMo/nemo-platform
Length of output: 50384
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Union/type references ==\n'
rg -n -C3 'AggregateRangeScore|AggregateRubricScore|score_type|scoreType' .
printf '\n== Files that define the evaluator schema ==\n'
fd -a 'evaluator' . | sed -n '1,120p'Repository: NVIDIA-NeMo/nemo-platform
Length of output: 50383
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== AgentEvalScoresPanel.test.tsx ==\n'
sed -n '1,220p' web/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/AgentEvalScoresPanel.test.tsx
printf '\n== evalScores.ts ==\n'
sed -n '1,220p' web/packages/studio/src/routes/agents/AgentEvaluationsRoute/evalScores.ts
printf '\n== Aggregate score schema excerpt ==\n'
sed -n '2248,2378p' plugins/nemo-evaluator/openapi/openapi.yamlRepository: NVIDIA-NeMo/nemo-platform
Length of output: 5681
Render rubric distributions for rubric scores. score_type: 'rubric' still has the shared numeric stats, but this path drops rubric_distribution and mode_category. Branch on score_type and add a rubric test.
🤖 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
`@web/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/AgentEvalScoresPanel.tsx`
around lines 23 - 40, Update the score rendering in AgentEvalScoresPanel’s
scores.map callback to branch on each score’s score_type. For rubric scores,
render the rubric_distribution and mode_category data in addition to the shared
numeric statistics, while preserving the existing numeric-score rendering. Add
or update a rubric-focused test covering these fields.
Comment on lines
+118
to
+131
| if (formData.mode === MODE_DEFAULT) { | ||
| const example = getEvaluationSampleAgent(formData.exampleKey); | ||
| const content = await fetchSampleText(example.evalConfigPath); | ||
| const files: EvalSeedFile[] = [ | ||
| { path: EVAL_CONFIG_FILENAME, content, type: 'application/json' }, | ||
| ]; | ||
| await ensureEvalConfigFileset( | ||
| workspace, | ||
| formData.newName.trim(), | ||
| new AbortController().signal, | ||
| files, | ||
| 'Agent Evaluation Config' | ||
| ); | ||
| return content; |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Reject existing fileset names instead of silently submitting different content.
ensureEvalConfigFileset never overwrites an existing eval-config.json, but this function still returns the newly selected example text. A name collision therefore submits one config while the referenced fileset retains another. Require a new name or submit the persisted file content.
🤖 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
`@web/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/SubmitEvaluationModal.tsx`
around lines 118 - 131, Update the MODE_DEFAULT branch in SubmitEvaluationModal
so name collisions are rejected rather than returning the newly selected sample
content after ensureEvalConfigFileset preserves an existing eval-config.json.
Validate that the fileset name is available before submission and surface the
existing validation/error path, or retrieve and return the persisted file
content when the name already exists; ensure the submitted content always
matches the referenced fileset.
Comment on lines
+189
to
+208
| // Fetch and parse the selected example config early to detect metric type and default model. | ||
| const { data: exampleConfig } = useQuery({ | ||
| queryKey: ['eval-config-preview', exampleKey], | ||
| queryFn: async () => { | ||
| const response = await fetch( | ||
| `${PLATFORM_BASE_URL}/apis/agents/v2/workspaces/${workspace}/agents` | ||
| ); | ||
| if (!response.ok) throw new Error('Failed to fetch agents'); | ||
| const json = (await response.json()) as { data: Agent[] }; | ||
| return json.data; | ||
| const example = getEvaluationSampleAgent(exampleKey); | ||
| if (!example) return null; | ||
| const text = await fetchSampleText(example.evalConfigPath); | ||
| return parseEvalConfig(text); | ||
| }, | ||
| enabled: open && !agentProp, | ||
| enabled: open && mode === MODE_DEFAULT && !!exampleKey, | ||
| staleTime: Infinity, | ||
| }); | ||
|
|
||
| // Prior eval jobs — the source for the "existing eval config" dropdown. | ||
| const { data: jobs = [], isLoading: isJobsLoading } = useQuery({ | ||
| queryKey: ['agent-eval-jobs', workspace], | ||
| queryFn: ({ signal }) => fetchAgentEvalJobs(workspace, signal), | ||
| enabled: open, | ||
| }); | ||
| const isLlmJudge = mode === MODE_DEFAULT && exampleConfig?.metric.metric_type === 'llm-judge'; | ||
| isLlmJudgeRef.current = isLlmJudge; | ||
|
|
||
| const defaultModelRef = | ||
| isLlmJudge && typeof exampleConfig?.metric.payload.metric.model === 'string' | ||
| ? exampleConfig.metric.payload.metric.model | ||
| : undefined; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Determine judge requirements from the config being submitted.
isLlmJudge is always false in fileset mode and remains false while the example preview loads. Users can therefore submit an llm-judge metric with an empty model. Parse the selected config before validation, expose/require the judge selector, and block submission while its metric type is unresolved.
Based on learnings, the judge model must be injected at submission time for llm-judge metrics.
Also applies to: 273-279, 334-379
🤖 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
`@web/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/SubmitEvaluationModal.tsx`
around lines 189 - 208, Update SubmitEvaluationModal’s submission and validation
flow to derive the metric type from the config being submitted, including
fileset mode, rather than relying only on the preview query. Keep the judge
selector visible and required for llm-judge metrics, block submission while the
metric type/config is unresolved, and inject the selected judge model into the
submitted config at submission time.
Source: Learnings
Comment on lines
+120
to
129
| export const buildAgentEvalRequestBody = (config: EvalConfig, selections: SubmitSelections) => { | ||
| const judge = buildJudgeModel(selections.workspace, selections.judgeModel); | ||
| return { | ||
| agent: formData.agent, | ||
| evalConfig: evalConfigName, | ||
| evalConfigFileset: formData.newName.trim(), | ||
| seedSources: [ | ||
| { | ||
| path: evalConfigName, | ||
| assetPath: example.evalConfigPath, | ||
| type: contentTypeForFile(example.evalConfigPath), | ||
| }, | ||
| { | ||
| path: fileNameOf(example.evalDataPath), | ||
| assetPath: example.evalDataPath, | ||
| type: contentTypeForFile(example.evalDataPath), | ||
| }, | ||
| ], | ||
| ...(selections.filesetName ? { description: selections.filesetName } : {}), | ||
| spec: { | ||
| tasks: fanMetricOntoTasks(config, judge), | ||
| target: buildAgentTarget(selections.workspace, selections.agent), | ||
| max_concurrent_tasks: config.max_concurrent_tasks ?? DEFAULT_MAX_CONCURRENT_TASKS, | ||
| }, | ||
| }; |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not let reusable configs control submission concurrency.
Line 127 honors arbitrary stored values, including unsafe concurrency. Inject the effective value at submission—defaulting to 1—instead of reading it from the reusable config.
Based on learnings, max_concurrent_tasks must be injected at submission and default to 1, not stored in reusable evaluation configs.
🤖 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
`@web/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/submitEvaluationSpec.ts`
around lines 120 - 129, Update buildAgentEvalRequestBody so
spec.max_concurrent_tasks is injected at submission with an effective value of
1, rather than reading config.max_concurrent_tasks or
DEFAULT_MAX_CONCURRENT_TASKS from the reusable EvalConfig. Keep the remaining
request construction unchanged.
Source: Learnings
Comment on lines
+132
to
+145
| /** Parse an eval-config.json blob, validating the minimal required shape. */ | ||
| export const parseEvalConfig = (text: string): EvalConfig => { | ||
| const parsed = JSON.parse(text) as Partial<EvalConfig>; | ||
| if (!Array.isArray(parsed.tasks) || parsed.tasks.length === 0) { | ||
| throw new Error('eval-config.json must contain a non-empty "tasks" array'); | ||
| } | ||
| if (!parsed.metric || typeof parsed.metric !== 'object') { | ||
| throw new Error('eval-config.json must contain a "metric"'); | ||
| } | ||
| return { | ||
| tasks: parsed.tasks, | ||
| metric: parsed.metric, | ||
| max_concurrent_tasks: parsed.max_concurrent_tasks, | ||
| }; |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Validate the complete config shape before returning it.
{ "tasks": ["bad"], "metric": {} } passes here, then request construction dereferences metric.payload.metric or submits malformed tasks. Validate task fields, inline payload structure, metric type, and numeric bounds at this JSON boundary.
Suggested direction
- const parsed = JSON.parse(text) as Partial<EvalConfig>;
+ const parsed: unknown = JSON.parse(text);
+ const result = evalConfigSchema.safeParse(parsed);
+ if (!result.success) {
+ throw new Error('eval-config.json has an invalid structure');
+ }
+ return result.data;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /** Parse an eval-config.json blob, validating the minimal required shape. */ | |
| export const parseEvalConfig = (text: string): EvalConfig => { | |
| const parsed = JSON.parse(text) as Partial<EvalConfig>; | |
| if (!Array.isArray(parsed.tasks) || parsed.tasks.length === 0) { | |
| throw new Error('eval-config.json must contain a non-empty "tasks" array'); | |
| } | |
| if (!parsed.metric || typeof parsed.metric !== 'object') { | |
| throw new Error('eval-config.json must contain a "metric"'); | |
| } | |
| return { | |
| tasks: parsed.tasks, | |
| metric: parsed.metric, | |
| max_concurrent_tasks: parsed.max_concurrent_tasks, | |
| }; | |
| /** Parse an eval-config.json blob, validating the minimal required shape. */ | |
| export const parseEvalConfig = (text: string): EvalConfig => { | |
| const parsed: unknown = JSON.parse(text); | |
| const result = evalConfigSchema.safeParse(parsed); | |
| if (!result.success) { | |
| throw new Error('eval-config.json has an invalid structure'); | |
| } | |
| return result.data; | |
| }; |
🤖 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
`@web/packages/studio/src/routes/agents/AgentEvaluationsRoute/components/submitEvaluationSpec.ts`
around lines 132 - 145, Update parseEvalConfig to fully validate the parsed
EvalConfig before returning it: reject malformed task entries, invalid inline
payload structures, unsupported metric types, and numeric options outside their
allowed bounds. Ensure invalid metric payloads and task definitions fail at this
JSON boundary rather than reaching request construction, while preserving the
existing required non-empty tasks and metric checks.
Contributor
|
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
1 participant
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 by CodeRabbit
New Features
Improvements