Skip to content

chore(packaging): remove legacy evaluator wiring - #230

Merged
SandyChapman merged 2 commits into
mainfrom
remove-evaluator-package-wiring/schapman
Jun 11, 2026
Merged

chore(packaging): remove legacy evaluator wiring#230
SandyChapman merged 2 commits into
mainfrom
remove-evaluator-package-wiring/schapman

Conversation

@SandyChapman

@SandyChapman SandyChapman commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • remove nmp-evaluator from root dependencies, groups, sources, and workspace members
  • remove the legacy evaluator-service bundle entry from nemo-platform packaging
  • refresh uv.lock and third_party license metadata after dropping evaluator-only dependencies

Validation

  • uv lock --check
  • make check-licenses
  • bash tools/lint/lint-sdk-vendored.sh
  • tools/lint/lint-all.sh outside sandbox: 10/12 passed before commit; expected local pre-commit failures were lint-licenses and lint-sdk-vendored due generated files not yet committed, both passed after regeneration/commit

Stacked on #227 (platform-seed disentangle).

Summary by CodeRabbit

  • Chores
    • Removed the evaluator service from the platform distribution and workspace wiring.
    • Regenerated dependency and package manifests and updated third‑party requirement exports.
    • Adjusted bundled dependency lists and plugin/runtime dependency constraints.
  • Security
    • Updated aggregated vulnerability records and license summary data.
  • Tests
    • Removed evaluator tests from automated discovery.
  • Documentation
    • Simplified evaluator configuration docs and updated config generation to reflect the new config model.
  • Refactor
    • Simplified evaluator plugin configuration and updated its greeting behavior.

@SandyChapman
SandyChapman marked this pull request as ready for review June 9, 2026 15:27
@SandyChapman
SandyChapman requested review from a team as code owners June 9, 2026 15:27
Base automatically changed from remove-evaluator-platform-seed/schapman to main June 9, 2026 19:14
@SandyChapman
SandyChapman force-pushed the remove-evaluator-package-wiring/schapman branch from 74be2bd to 13787b7 Compare June 9, 2026 19:17
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 18990/25181 75.4% 61.3%
Integration Tests 10975/23953 45.8% 20.2%

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Removes the evaluator service from workspace and platform packaging, drops evaluator tests from discovery, updates docs/config import for evaluator, prunes related third-party license and OSV entries, and regenerates the pinned requirements export.

Changes

Evaluator Service Removal from Platform

Layer / File(s) Summary
Remove evaluator from workspace configuration
pyproject.toml
Deletes nmp-evaluator from root project dependencies, dev and container dependency groups, removes it from uv.sources workspace mappings, removes services/evaluator from uv workspace members, and adds ./services/evaluator/ to type-exclude.
Remove evaluator from platform package configuration
packages/nemo_platform/pyproject.toml
Removes evaluator-service optional-dependency extra, removes it from the aggregated services extra list, removes nmp-evaluator from [tool.bundle-package], and adjusts generated bundled dependency constraints for nemo-evaluator-sdk and nemo-safe-synthesizer-plugin.
Docs and config generation update
docs/set-up/config-reference.md, script/generate_config_docs.py
Deletes the evaluator config section from the global config reference and updates the config-docs generator to import EvaluatorConfig from nemo_evaluator.config and include it in CONFIG_CLASSES.
Evaluator plugin code: config and hello service
plugins/nemo-evaluator/src/nemo_evaluator/config.py, plugins/nemo-evaluator/src/nemo_evaluator/service.py
Simplifies EvaluatorConfig (removes greeting_style) and changes /hello/{name} to return HelloResponse(message=say_hello(name)) by importing say_hello from nemo_evaluator.core.
Remove evaluator tests from discovery
pytest.ini
Removes services/evaluator/tests from testpaths so pytest no longer discovers those tests.
SDK and plugin dependency updates
packages/nemo_evaluator_sdk/pyproject.toml, plugins/nemo-safe-synthesizer/pyproject.toml
Adds ragas==0.3.5, langchain-openai>=1.1.14, langchain-nvidia-ai-endpoints>=1.0.0,<2.0.0 to nemo_evaluator_sdk and pins datasets>=3.3.1,<=4.3.0 for safe-synthesizer.
Prune third-party license entries
third_party/licenses.jsonl
Removes the python-box license JSONL record (list reflowed).
Update OSV vulnerability records and license summary
third_party/osv-licenses.json
Removes several package entries from results.packages, adds pip vuln PYSEC-2026-196, updates/duplicates pyarrow vulnerability records and groups, and adjusts experimental license summary counts.
Regenerate requirements export
third_party/requirements-main.txt
Regenerates export from the updated dependency graph; removes transitive pins (datasets, instructor, scikit-network, smmap) and updates many # via provider annotations.

Possibly related PRs

Suggested reviewers

  • tylersbray
  • mckornfield
  • arpitsardhana
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title clearly describes the main change: removing legacy evaluator wiring from the package configuration and dependencies.
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 unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch remove-evaluator-package-wiring/schapman

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

@SandyChapman
SandyChapman force-pushed the remove-evaluator-package-wiring/schapman branch 2 times, most recently from dc6221b to 49e0bb1 Compare June 9, 2026 19:52
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Documentation preview is ready

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

Built from 6c4a945 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 force-pushed the remove-evaluator-package-wiring/schapman branch 2 times, most recently from 063caa5 to 6c4a945 Compare June 9, 2026 20:18

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/pyproject.toml`:
- Around line 28-30: Update the dependency specification for langchain-openai to
add an upper bound consistent with the other LangChain dependency: change
"langchain-openai>=1.1.14" to "langchain-openai>=1.1.14,<2.0.0" in the
pyproject.toml dependencies block (look for the existing langchain-openai
entry), then regenerate your lockfile / reinstall dependencies (poetry lock or
pip/poetry install) so the constraint takes effect.

In `@plugins/nemo-safe-synthesizer/pyproject.toml`:
- Line 9: The pinned upper bound on the datasets dependency
("datasets>=3.3.1,<=4.3.0") is overly restrictive; either remove or relax the
<=4.3.0 cap so downstreams can use newer releases (e.g., change to
"datasets>=3.3.1" or broaden to "datasets>=3.3.1,<5.0.0"), or if there is a real
incompatibility with >=4.4.0, document the exact breaking change and add a
comment in pyproject.toml and the plugin README explaining why the upper bound
is required; update the dependency spec in pyproject.toml and add the
documentation note where the plugin's installation/requirements are described.
🪄 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: 02a0c79f-5a62-4847-8ba4-52dab814aa8a

📥 Commits

Reviewing files that changed from the base of the PR and between 063caa5 and 6c4a945.

⛔ Files ignored due to path filters (2)
  • sdk/python/nemo-platform/pyproject.toml is excluded by !sdk/**
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (12)
  • docs/set-up/config-reference.md
  • packages/nemo_evaluator_sdk/pyproject.toml
  • packages/nemo_platform/pyproject.toml
  • plugins/nemo-evaluator/src/nemo_evaluator/config.py
  • plugins/nemo-evaluator/src/nemo_evaluator/service.py
  • plugins/nemo-safe-synthesizer/pyproject.toml
  • pyproject.toml
  • pytest.ini
  • script/generate_config_docs.py
  • third_party/licenses.jsonl
  • third_party/osv-licenses.json
  • third_party/requirements-main.txt
💤 Files with no reviewable changes (4)
  • pytest.ini
  • script/generate_config_docs.py
  • third_party/licenses.jsonl
  • docs/set-up/config-reference.md
✅ Files skipped from review due to trivial changes (1)
  • third_party/requirements-main.txt
🚧 Files skipped from review as they are similar to previous changes (4)
  • plugins/nemo-evaluator/src/nemo_evaluator/service.py
  • plugins/nemo-evaluator/src/nemo_evaluator/config.py
  • pyproject.toml
  • third_party/osv-licenses.json

Comment thread packages/nemo_evaluator_sdk/pyproject.toml
Comment thread plugins/nemo-safe-synthesizer/pyproject.toml
@SandyChapman
SandyChapman force-pushed the remove-evaluator-package-wiring/schapman branch from 6c4a945 to 25a3625 Compare June 10, 2026 11:30
@github-actions

Copy link
Copy Markdown
Contributor

@SandyChapman
SandyChapman force-pushed the remove-evaluator-package-wiring/schapman branch from 25a3625 to 91518fb Compare June 10, 2026 12:05

@mckornfield mckornfield left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

love me some red

@SandyChapman
SandyChapman added this pull request to the merge queue Jun 10, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jun 10, 2026
@github-actions github-actions Bot added the chore label Jun 11, 2026
Signed-off-by: Sandy Chapman <schapman@nvidia.com>
@SandyChapman
SandyChapman force-pushed the remove-evaluator-package-wiring/schapman branch 2 times, most recently from ab3ccfc to cfb4ff7 Compare June 11, 2026 17:16
Signed-off-by: Sandy Chapman <schapman@nvidia.com>
@SandyChapman
SandyChapman force-pushed the remove-evaluator-package-wiring/schapman branch from cfb4ff7 to 442a474 Compare June 11, 2026 17:33
@SandyChapman
SandyChapman added this pull request to the merge queue Jun 11, 2026
Merged via the queue into main with commit 3367eea Jun 11, 2026
44 checks passed
SandyChapman added a commit that referenced this pull request Jun 17, 2026
…refs

Add metric persistence to the evaluator plugin so reusable metrics can be
stored on the platform and referenced from eval jobs. Restores the metrics
surface removed in #230, aligned to its workspace/name conventions.

- Entity + storage: MetricBundleEntity (entity_type "metric_bundle") in the
  entity store; the executable cloudpickle bundle is uploaded to the Files
  service (one fileset per metric), with the entity holding bundle_ref + digest.
- CRUD API (name-in-path): POST/GET/DELETE
  /apis/evaluator/v2/workspaces/{workspace}/metrics/{name} + list. Metrics are
  immutable (no update).
- SDK: client.evaluator.metrics (create/retrieve/list/delete).
- Job integration: EvaluateInputSpec.metrics accepts inline MetricInline and/or
  MetricRef (workspace/name); refs resolve from the entity store + Files during
  spec resolution.
- Explicit CloudpickleMetricPayload schema (discriminated on kind); regenerated
  openapi.yaml.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Sandy Chapman <schapman@nvidia.com>
SandyChapman added a commit that referenced this pull request Jun 17, 2026
…refs

Add metric persistence to the evaluator plugin so reusable metrics can be
stored on the platform and referenced from eval jobs. Restores the metrics
surface removed in #230, aligned to its workspace/name conventions.

- Entity + storage: MetricBundleEntity (entity_type "metric_bundle") in the
  entity store; the executable cloudpickle bundle is uploaded to the Files
  service (one fileset per metric), with the entity holding bundle_ref + digest.
- CRUD API (name-in-path): POST/GET/DELETE
  /apis/evaluator/v2/workspaces/{workspace}/metrics/{name} + list. Metrics are
  immutable (no update).
- SDK: client.evaluator.metrics (create/retrieve/list/delete).
- Job integration: EvaluateInputSpec.metrics accepts inline MetricInline and/or
  MetricRef (workspace/name); refs resolve from the entity store + Files during
  spec resolution.
- Explicit CloudpickleMetricPayload schema (discriminated on kind); regenerated
  openapi.yaml.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Sandy Chapman <schapman@nvidia.com>
SandyChapman added a commit that referenced this pull request Jun 22, 2026
…refs

Add metric persistence to the evaluator plugin so reusable metrics can be
stored on the platform and referenced from eval jobs. Restores the metrics
surface removed in #230, aligned to its workspace/name conventions.

- Entity + storage: MetricBundleEntity (entity_type "metric_bundle") in the
  entity store; the executable cloudpickle bundle is uploaded to the Files
  service (one fileset per metric), with the entity holding bundle_ref + digest.
- CRUD API (name-in-path): POST/GET/DELETE
  /apis/evaluator/v2/workspaces/{workspace}/metrics/{name} + list. Metrics are
  immutable (no update).
- SDK: client.evaluator.metrics (create/retrieve/list/delete).
- Job integration: EvaluateInputSpec.metrics accepts inline MetricInline and/or
  MetricRef (workspace/name); refs resolve from the entity store + Files during
  spec resolution.
- Explicit CloudpickleMetricPayload schema (discriminated on kind); regenerated
  openapi.yaml.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Sandy Chapman <schapman@nvidia.com>
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.

2 participants