Skip to content

feat(scoring): add NEL-NMP integration contracts (Approach 3)#924

Closed
wprazuch wants to merge 1 commit into
wprazuch/sdk-onboarding-approach1from
wprazuch/sdk-approach3-contracts
Closed

feat(scoring): add NEL-NMP integration contracts (Approach 3)#924
wprazuch wants to merge 1 commit into
wprazuch/sdk-onboarding-approach1from
wprazuch/sdk-approach3-contracts

Conversation

@wprazuch
Copy link
Copy Markdown
Contributor

Defines the abstract interface between NEL and downstream metric providers (e.g., NMP's nemo_evaluator_sdk) in a new module: src/nemo_evaluator/scoring/contracts.py (~250 LOC).

Provides:

  • Metric protocols (runtime_checkable, structural):
    • Metric -- core object-style metric contract (type, metric, compute_scores, score_names)
    • CorpusMetric -- corpus-level aggregation
    • MetricWithSecrets -- secret resolution protocol
    • MetricWithPreflight -- one-time preflight setup
  • Pydantic result types (no heavy deps -- no pyarrow, no pandas):
    • MetricScore, MetricResult
    • ScoreStats, RubricScoreValue, RubricScoreStat
  • SecretRefLike Protocol (abstract secret reference)
  • SecretResolver type alias
  • metric_as_scorer() helper: bridges object-style Metric to NEL's function-style (ScorerInput) -> dict scorer protocol

Design rationale (Approach 3):

  • NMP's concrete metric implementations (BLEU, ROUGE, F1, RAGAS, LLMJudge, etc.) stay in nemo_evaluator_sdk with their external deps (sacrebleu, rouge_score, openai, ragas).
  • NEL owns only the contract surface. NMP SDK will be updated in a follow-up PR to depend on NEL for these types, dropping its own copies and becoming thinner.
  • Dependency direction is strictly one-way: SDK -> NEL. NEL never imports from any provider.
  • Contracts are dep-light: pure Pydantic models + typing.Protocol, no pyarrow/pandas/openai/etc. leaking into NEL.

Verified:

  • 14 new unit tests pass
  • SDK's ExactMatchMetric structurally satisfies the Metric Protocol with zero code changes (isinstance check passes)
  • All 162 SDK + contracts tests pass together

This commit is stacked on wprazuch/sdk-onboarding-approach1 so the contracts land alongside the verbatim SDK copy for easier review. The follow-up plan is:

  1. Merge Approach 1 (verbatim copy) to dev/0.3.0
  2. Merge this PR (contracts) to dev/0.3.0
  3. Open a matching PR on NMP's nemo_evaluator_sdk to depend on NEL's contracts and drop its redundant abstraction copies

Defines the abstract interface between NEL and downstream metric
providers (e.g., NMP's nemo_evaluator_sdk) in a new module:
src/nemo_evaluator/scoring/contracts.py (~250 LOC).

Provides:
- Metric protocols (runtime_checkable, structural):
  * Metric -- core object-style metric contract (type, metric,
    compute_scores, score_names)
  * CorpusMetric -- corpus-level aggregation
  * MetricWithSecrets -- secret resolution protocol
  * MetricWithPreflight -- one-time preflight setup
- Pydantic result types (no heavy deps -- no pyarrow, no pandas):
  * MetricScore, MetricResult
  * ScoreStats, RubricScoreValue, RubricScoreStat
- SecretRefLike Protocol (abstract secret reference)
- SecretResolver type alias
- metric_as_scorer() helper: bridges object-style Metric to NEL's
  function-style (ScorerInput) -> dict scorer protocol

Design rationale (Approach 3):
- NMP's concrete metric implementations (BLEU, ROUGE, F1, RAGAS,
  LLMJudge, etc.) stay in nemo_evaluator_sdk with their external deps
  (sacrebleu, rouge_score, openai, ragas).
- NEL owns only the contract surface. NMP SDK will be updated in a
  follow-up PR to depend on NEL for these types, dropping its own
  copies and becoming thinner.
- Dependency direction is strictly one-way: SDK -> NEL. NEL never
  imports from any provider.
- Contracts are dep-light: pure Pydantic models + typing.Protocol,
  no pyarrow/pandas/openai/etc. leaking into NEL.

Verified:
- 14 new unit tests pass
- SDK's ExactMatchMetric structurally satisfies the Metric Protocol
  with zero code changes (isinstance check passes)
- All 162 SDK + contracts tests pass together

This commit is stacked on wprazuch/sdk-onboarding-approach1 so the
contracts land alongside the verbatim SDK copy for easier review. The
follow-up plan is:
1. Merge Approach 1 (verbatim copy) to dev/0.3.0
2. Merge this PR (contracts) to dev/0.3.0
3. Open a matching PR on NMP's nemo_evaluator_sdk to depend on NEL's
   contracts and drop its redundant abstraction copies

Signed-off-by: Wojciech Prazuch <wprazuch@nvidia.com>
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented Apr 22, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 22, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f012f1f2-717a-43af-a126-3941c9a00fb2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch wprazuch/sdk-approach3-contracts

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

@github-actions github-actions Bot added the tests label Apr 22, 2026
@wprazuch wprazuch closed this May 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant