Skip to content

feat(evaluator): add metric type name alias - #68

Merged
SandyChapman merged 2 commits into
mainfrom
metric-type-name/schapman
May 28, 2026
Merged

feat(evaluator): add metric type name alias#68
SandyChapman merged 2 commits into
mainfrom
metric-type-name/schapman

Conversation

@SandyChapman

@SandyChapman SandyChapman commented May 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a shared non-empty MetricTypeName alias for evaluator SDK metric identifiers
  • use it in Metric, MetricDescriptor, and inline metric values
  • cover empty descriptor type validation in the metric contract tests

Testing

  • uv run --frozen ruff check packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/protocol.py packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/metrics.py packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/init.py packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/init.py packages/nemo_evaluator_sdk/tests/metrics/test_metric_contract.py
  • uv run --frozen ruff format --check packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/protocol.py packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/metrics.py packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/init.py packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/init.py packages/nemo_evaluator_sdk/tests/metrics/test_metric_contract.py
  • uv run --frozen --extra cpu ty check packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/protocol.py packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/metrics.py packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/init.py packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/init.py packages/nemo_evaluator_sdk/tests/metrics/test_metric_contract.py
  • uv run --frozen pytest packages/nemo_evaluator_sdk/tests/metrics/test_metric_contract.py -q

Summary by CodeRabbit

  • New Features

    • MetricTypeName is now available in the public API, providing a unified type definition for all metric type identifiers.
  • Bug Fixes

    • Metric descriptors and metrics now consistently validate and enforce non-empty type identifiers across the entire SDK with standardized constraints.

Review Change Stack

Signed-off-by: Sandy Chapman <schapman@nvidia.com>
@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: caa4794e-a111-42fc-83b7-32afa79318d5

📥 Commits

Reviewing files that changed from the base of the PR and between c87f685 and cb80057.

📒 Files selected for processing (5)
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/__init__.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/protocol.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/__init__.py
  • packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/metrics.py
  • packages/nemo_evaluator_sdk/tests/metrics/test_metric_contract.py

📝 Walkthrough

Walkthrough

The PR introduces MetricTypeName, a non-empty string constraint, and applies it throughout the metrics SDK. The type replaces plain str in metric type fields across protocol and value layers, with automatic validation via Pydantic's annotation constraints. The type is exported as public API.

Changes

MetricTypeName constraint and propagation

Layer / File(s) Summary
MetricTypeName type definition and protocol contract
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/metrics/protocol.py, packages/nemo_evaluator_sdk/tests/metrics/test_metric_contract.py
MetricTypeName is introduced as Annotated[str, StringConstraints(min_length=1)]. MetricDescriptor.type and Metric.type are updated to use it, eliminating the previous manual field validator. A new test verifies empty metric types are rejected.
Values layer metric type field alignment
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/metrics.py
MetricBase.type field is changed from str to MetricTypeName, applying the shared constraint consistently.
Public API exports
packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/__init__.py, packages/nemo_evaluator_sdk/src/nemo_evaluator_sdk/values/__init__.py
MetricTypeName is imported and added to __all__ in both root and values package initializers, exposing it as public API.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title directly describes the main change: introducing a shared MetricTypeName alias used across metric protocol, descriptor, and inline values.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch metric-type-name/schapman

Comment @coderabbitai help to get the list of available commands and usage tips.

@SandyChapman SandyChapman changed the title feat(evaluator-sdk): add metric type name alias feat(evaluator): add metric type name alias May 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Documentation preview is ready

Preview: https://nvidia-nemo.github.io/nemo-platform/pr-preview/pr-68/pr-68/

Built from cb80057 in workflow run.

This preview is deployed from this PR branch, updates when docs changes are pushed, and will be removed when the PR closes.

@SandyChapman
SandyChapman marked this pull request as ready for review May 27, 2026 12:18
@SandyChapman
SandyChapman requested review from a team as code owners May 27, 2026 12:18
@github-actions

github-actions Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 18244/24191 75.4% 61.9%
Integration Tests 11667/22973 50.8% 25.9%

Signed-off-by: Sandy Chapman <schapman@nvidia.com>
@SandyChapman
SandyChapman added this pull request to the merge queue May 28, 2026
Merged via the queue into main with commit 252b42d May 28, 2026
23 of 24 checks passed
aray12 pushed a commit that referenced this pull request May 28, 2026
* feat(evaluator-sdk): add metric type name alias

Signed-off-by: Sandy Chapman <schapman@nvidia.com>

* fix(evaluator-sdk): sync vendored metric type alias

Signed-off-by: Sandy Chapman <schapman@nvidia.com>

---------

Signed-off-by: Sandy Chapman <schapman@nvidia.com>
Signed-off-by: Alex Ray <alray@nvidia.com>
@SandyChapman
SandyChapman deleted the metric-type-name/schapman branch June 2, 2026 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants