feat(evaluator): add agent-eval SDK domain model value types - #339
Merged
Conversation
Contributor
|
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:
📝 WalkthroughWalkthroughAdds three evidence value types ( ChangesEvidence Value Types
Agent Eval Type Layer
Sequence Diagram(s)sequenceDiagram
participant Caller
participant AgentTaskRunner
participant AgentEvalTrial
participant CandidateEvidence
participant LocalFilesystemEvidence
Caller->>AgentTaskRunner: run_tasks(tasks, config)
AgentTaskRunner-->>Caller: Sequence[AgentEvalTrial]
Caller->>AgentEvalTrial: access trial.evidence
AgentEvalTrial-->>Caller: CandidateEvidence | None
Caller->>CandidateEvidence: filesystem(name)
CandidateEvidence->>LocalFilesystemEvidence: materialize via _local_filesystem_ref(ref)
CandidateEvidence-->>Caller: LocalFilesystemEvidence (cached)
Caller->>LocalFilesystemEvidence: read_text(relative_path)
LocalFilesystemEvidence-->>Caller: str (file contents)
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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.
🧹 Nitpick comments (3)
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/types.py (1)
103-103: ⚡ Quick winReplace quoted forward refs with concrete type annotations.
At Line 103, Line 145, and Line 163, string-based annotations are used (
"AgentEvalTask","AgentEvalAttempt","AgentEvalDiagnostic"). Use concrete annotations directly.As per coding guidelines, "
**/*.py: Always prefer concrete type hints over string-based ones in Python code; do not import types under TYPE_CHECKING, instead import types as regular imports when possible."Also applies to: 145-145, 163-163
🤖 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/src/nemo_evaluator_sdk/agent_eval/types.py` at line 103, Replace the quoted string-based type annotations with concrete type hints in three methods within the AgentEvalTask class. Specifically, change the return type annotation of _validate_metric_references method (at line 103) from "AgentEvalTask" to AgentEvalTask, update the return type at line 145 from "AgentEvalAttempt" to AgentEvalAttempt, and update the return type at line 163 from "AgentEvalDiagnostic" to AgentEvalDiagnostic. Ensure that the necessary types are imported as regular imports at the top of the file rather than under TYPE_CHECKING, so that these concrete type annotations can be used directly.Source: Coding guidelines
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/evidence.py (2)
34-48: ⚡ Quick winAsync methods perform synchronous filesystem I/O.
exists,read_text, anditer_pathsare declaredasyncbut execute blockingPath.exists(),Path.read_text(), and directory iteration synchronously. This blocks the event loop when called.If async signatures are intentional for future-proofing, consider wrapping I/O in
asyncio.to_thread()or documenting the sync behavior.Option: Use asyncio.to_thread for non-blocking I/O
+import asyncio + class LocalFilesystemEvidence: ... async def exists(self, relative_path: str | Path = ".") -> bool: """Return whether a path exists under the evidence root.""" - return self.path(relative_path).exists() + p = self.path(relative_path) + return await asyncio.to_thread(p.exists) async def read_text(self, relative_path: str | Path, *, encoding: str = "utf-8") -> str: """Read a text file under the evidence root.""" - return self.path(relative_path).read_text(encoding=encoding) + p = self.path(relative_path) + return await asyncio.to_thread(p.read_text, encoding=encoding)🤖 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/src/nemo_evaluator_sdk/values/evidence.py` around lines 34 - 48, The async methods exists, read_text, and iter_paths all perform blocking filesystem I/O operations synchronously, which blocks the event loop. Wrap the blocking I/O calls in asyncio.to_thread() to make them non-blocking: in exists, wrap the self.path(relative_path).exists() call, in read_text wrap the self.path(relative_path).read_text(encoding=encoding) call, and in iter_paths wrap the base.is_file() check and the directory iteration logic (base.rglob or base.iterdir) to ensure the event loop is not blocked during execution.
122-130: Windows drive paths will be rejected as unknown scheme.urlparse("C:\\path")yieldsscheme="c", triggering the error on line 128–129. Consider detecting Windows drive letters if local Windows paths need to be supported, though the function appears designed for URI refs and POSIX paths (no Windows paths documented or tested).🤖 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/src/nemo_evaluator_sdk/values/evidence.py` around lines 122 - 130, The _local_filesystem_ref function will incorrectly reject valid Windows absolute paths because urlparse interprets Windows drive letters (e.g., "C") as URL schemes. To fix this, add explicit detection for Windows drive letter patterns (a single letter followed by a colon, like "C:") before the urlparse scheme check. If a Windows drive letter is detected, treat it as a local filesystem path and proceed to return Path(ref) directly, bypassing the scheme validation that would incorrectly flag it as an unsupported scheme. Alternatively, if Windows paths are not intended to be supported, document this limitation clearly in the function's docstring.
🤖 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/src/nemo_evaluator_sdk/agent_eval/types.py`:
- Line 103: Replace the quoted string-based type annotations with concrete type
hints in three methods within the AgentEvalTask class. Specifically, change the
return type annotation of _validate_metric_references method (at line 103) from
"AgentEvalTask" to AgentEvalTask, update the return type at line 145 from
"AgentEvalAttempt" to AgentEvalAttempt, and update the return type at line 163
from "AgentEvalDiagnostic" to AgentEvalDiagnostic. Ensure that the necessary
types are imported as regular imports at the top of the file rather than under
TYPE_CHECKING, so that these concrete type annotations can be used directly.
In `@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/evidence.py`:
- Around line 34-48: The async methods exists, read_text, and iter_paths all
perform blocking filesystem I/O operations synchronously, which blocks the event
loop. Wrap the blocking I/O calls in asyncio.to_thread() to make them
non-blocking: in exists, wrap the self.path(relative_path).exists() call, in
read_text wrap the self.path(relative_path).read_text(encoding=encoding) call,
and in iter_paths wrap the base.is_file() check and the directory iteration
logic (base.rglob or base.iterdir) to ensure the event loop is not blocked
during execution.
- Around line 122-130: The _local_filesystem_ref function will incorrectly
reject valid Windows absolute paths because urlparse interprets Windows drive
letters (e.g., "C") as URL schemes. To fix this, add explicit detection for
Windows drive letter patterns (a single letter followed by a colon, like "C:")
before the urlparse scheme check. If a Windows drive letter is detected, treat
it as a local filesystem path and proceed to return Path(ref) directly,
bypassing the scheme validation that would incorrectly flag it as an unsupported
scheme. Alternatively, if Windows paths are not intended to be supported,
document this limitation clearly in the function's docstring.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 47c61447-bc80-44b5-ad53-2779742e8b2b
⛔ Files ignored due to path filters (5)
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/types.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/metrics/protocol.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/values/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/values/evidence.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/values/protocol.pyis excluded by!sdk/**
📒 Files selected for processing (5)
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/types.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/protocol.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/__init__.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/evidence.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/protocol.py
Contributor
|
arpitsardhana
force-pushed
the
aalgo-272-domain-model/arpsingh
branch
2 times, most recently
from
June 15, 2026 21:03
21d4a7e to
ab2609e
Compare
ngoncharenko
approved these changes
Jun 15, 2026
arpitsardhana
force-pushed
the
aalgo-272-domain-model/arpsingh
branch
from
June 15, 2026 23:40
ab2609e to
5a4f1de
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/types.py (1)
6-6: ⚡ Quick winCoding guideline violation: string-based annotations enabled.
from __future__ import annotationsconverts all type hints to strings at runtime, conflicting with the guideline preferring concrete type hints. If forward references are needed (e.g.,AgentEvalTaskin its own validator), use quoted strings only where necessary instead of enabling it file-wide.🤖 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/src/nemo_evaluator_sdk/agent_eval/types.py` at line 6, Remove the `from __future__ import annotations` import statement at the top of the file in packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/types.py, as it converts all type hints to strings at runtime conflicting with the concrete type hints guideline. Instead, use quoted string annotations only where necessary for forward references, such as when the AgentEvalTask class references itself within validators or other type definitions that would otherwise cause circular dependency issues.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.
Nitpick comments:
In `@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/types.py`:
- Line 6: Remove the `from __future__ import annotations` import statement at
the top of the file in
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/types.py, as it
converts all type hints to strings at runtime conflicting with the concrete type
hints guideline. Instead, use quoted string annotations only where necessary for
forward references, such as when the AgentEvalTask class references itself
within validators or other type definitions that would otherwise cause circular
dependency issues.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 971fd35a-5000-44a2-a447-0a7851337fa0
⛔ Files ignored due to path filters (4)
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/types.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/values/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/values/evidence.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/values/protocol.pyis excluded by!sdk/**
📒 Files selected for processing (5)
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/.README.md.swppackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/types.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/__init__.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/evidence.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/protocol.py
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/protocol.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/init.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/evidence.py
arpitsardhana
force-pushed
the
aalgo-272-domain-model/arpsingh
branch
from
June 16, 2026 18:13
5a4f1de to
2fe5894
Compare
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 `@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/results.py`:
- Around line 109-113: The code inconsistently handles the PARTIAL status across
different parts of the pipeline. At lines 109-113, only COMPLETED status is
accepted for numeric values, but at lines 156-161, non-FAILED records (including
PARTIAL) are counted as scored, and at lines 204-208, PARTIAL is also dropped.
Update the condition at line 109 that checks `if score.status ==
AgentEvalScoreStatus.COMPLETED:` to also include PARTIAL status alongside
COMPLETED, so that numeric values are collected consistently. Then verify that
lines 156-161 and 204-208 are consistent with this change to ensure coverage and
aggregate stats agree on the same records.
In `@packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/trials.py`:
- Around line 10-20: There is a bidirectional import cycle where tasks.py
imports AgentTaskRunner from trials.py while trials.py imports
AgentEvalRunConfig and AgentEvalTask from tasks.py under TYPE_CHECKING. Create a
new shared module (e.g., agent_eval/models.py) and move the AgentEvalTask and
AgentEvalRunConfig classes there. Update trials.py to remove the TYPE_CHECKING
import block (lines 15-19) and instead import AgentEvalTask and
AgentEvalRunConfig concretely from the new models module. Update tasks.py
similarly if it also uses TYPE_CHECKING for these imports, so both files can
import concretely without creating circular dependencies.
🪄 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: 0b37dcf3-9bcd-4e93-be29-ab32c104e7be
⛔ Files ignored due to path filters (7)
sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/results.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/scores.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/tasks.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/agent_eval/trials.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/values/__init__.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/values/evidence.pyis excluded by!sdk/**sdk/python/nemo-platform/src/nemo_platform/beta/evaluator/values/protocol.pyis excluded by!sdk/**
📒 Files selected for processing (7)
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/results.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/scores.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/tasks.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/agent_eval/trials.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/__init__.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/evidence.pypackages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/protocol.py
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/init.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/protocol.py
- packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/evidence.py
arpitsardhana
force-pushed
the
aalgo-272-domain-model/arpsingh
branch
2 times, most recently
from
June 16, 2026 19:55
3d1af27 to
9587f48
Compare
SandyChapman
approved these changes
Jun 16, 2026
arpitsardhana
force-pushed
the
aalgo-272-domain-model/arpsingh
branch
from
June 16, 2026 23:04
9587f48 to
c4ff314
Compare
Introduce the standalone agent-eval domain model described in the NeMo Evaluator Agent Evaluation design: AgentEvalTask, SemanticView, ViewSignal, AgentOutput, AgentEvalAttempt (trial), AgentEvalTaskResult, and summary/coverage value types, plus the shared EvidenceDescriptor, CandidateEvidence, and LocalFilesystemEvidence types. Extend CandidateOutput with a candidate.evidence field and the metric protocol to carry evidence. Mirrored into the vendored nemo_platform beta SDK. Signed-off-by: Arpit Singh (SW-CLOUD) <arpsingh@nvidia.com>
arpitsardhana
force-pushed
the
aalgo-272-domain-model/arpsingh
branch
from
June 16, 2026 23:20
c4ff314 to
1d5f4d0
Compare
arpitsardhana
enabled auto-merge
June 16, 2026 23:31
This was referenced Jun 17, 2026
This was referenced Jun 24, 2026
This was referenced Jul 3, 2026
Merged
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.
Introduce the standalone agent-eval domain model described in the NeMo Evaluator Agent Evaluation design:
CandidateEvidence, and LocalFilesystemEvidence types. Extend CandidateOutput with a candidate.evidence field and the metric protocol to carry evidence. Mirrored into the vendored nemo_platform beta SDK.
erDiagram AgentEvalRunResult ||--o{ AgentEvalTask : "tasks" AgentEvalRunResult ||--o{ AgentEvalAttempt : "attempts" AgentEvalRunResult ||--o{ AgentEvalTaskResult : "results" AgentEvalRunResult ||--|| AgentEvalSummary : "summary" AgentEvalTask ||--o{ Metric : "metrics (ordered)" AgentEvalTask ||--o{ SemanticView : "views" SemanticView ||--|{ ViewSignal : "signals (>=1)" ViewSignal }o--|| Metric : "metric+output ref" AgentEvalAttempt }o--|| AgentEvalTask : "task_id (FK)" AgentEvalAttempt ||--o| AgentOutput : "output" AgentEvalAttempt ||--o| CandidateEvidence : "evidence" CandidateEvidence ||--o{ EvidenceDescriptor : "descriptors" CandidateEvidence ||--o{ LocalFilesystemEvidence : "fs cache (lazy)" CandidateOutput ||--o| CandidateEvidence : "evidence (extension)" AgentEvalTaskResult }o--|| AgentEvalRunResult : "run_id (FK)" AgentEvalTaskResult }o--|| AgentEvalTask : "task_id (FK)" AgentEvalTaskResult }o--|| AgentEvalAttempt : "attempt_id (FK)" AgentEvalTaskResult }o--|| Metric : "metric_type (FK)" AgentEvalTaskResult ||--o{ MetricOutput : "outputs" AgentEvalTaskResult ||--o{ AgentEvalDiagnostic : "diagnostics" AgentEvalSummary ||--o{ AgentEvalMetricOutputCoverage : "metric_coverage" MetricInput ||--|| DatasetRow : "row" MetricInput ||--|| CandidateOutput : "candidate" AgentEvalTask { string id PK string intent dict inputs list metrics dict views dict metadata } SemanticView { enum reducer "single|all|any|mean|weighted_mean" list signals } ViewSignal { string metric string output float weight } AgentOutput { string text any response dict metadata } AgentEvalAttempt { string id PK string task_id FK enum status "completed|failed|partial" AgentOutput output CandidateEvidence evidence dict metadata } EvidenceDescriptor { string kind string ref "ref or data required" string format "parser hint e.g. atif" any data dict metadata } CandidateEvidence { dict descriptors dict metadata } LocalFilesystemEvidence { Path root } AgentEvalTaskResult { string id PK string run_id FK string task_id FK string attempt_id FK string metric_type FK enum status list outputs list diagnostics dict metadata } MetricOutput { string name any value } AgentEvalDiagnostic { enum severity "error|warning|info" string message string source dict details } AgentEvalMetricOutputCoverage { int total int scored int failed int missing } AgentEvalSummary { float overall_score dict metric_scores dict metric_coverage dict semantic_view_scores int task_count int attempt_count int result_count } AgentEvalRunResult { string run_id PK list tasks list attempts list results AgentEvalSummary summary dict benchmark Path output_dir Path dashboard_path } CandidateOutput { string output_text any response any trajectory "deprecated" CandidateEvidence evidence dict metadata } MetricInput { DatasetRow row CandidateOutput candidate } DatasetRow { int row_index dict data }Summary by CodeRabbit