Skip to content

fix(scaled-evals): keep plugin out of default --all-packages installs - #2341

Merged
ngoncharenko merged 1 commit into
mainfrom
scaled-evals-opt-in/ngoncharenko
Sep 23, 2026
Merged

ngoncharenko merged 1 commit into
mainfrom
scaled-evals-opt-in/ngoncharenko

Conversation

@ngoncharenko

@ngoncharenko ngoncharenko commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes NMP-245.

What is broken

  • make bootstrap and flox activation both run uv sync --all-packages, which installs every workspace member, including the opt-in scaled-evals plugin.
  • Once installed, its service and scaled-evals-jobs controller are discovered and started by default.
  • The controller requires credentials_encryption_key, which has no default, so it crashes and /health/ready returns 503.

Why this is the fix

  • --all-packages only installs workspace members. Making scaled-evals a path dependency instead of a member keeps it out of every --all-packages install path: Makefile, flox on-activate, Dockerfile.agentic-base, and docs.
  • It stays installable on request through the existing scaled-evals dependency group, which the CI lint and test jobs already use.
  • No runtime or discovery changes.

UX before / after

  • Before: nemo setup --auto --start-services → readiness returns 503, Unhealthy controllers: scaled-evals-jobs.
  • After: the same command → 200 {"status":"ready"}, and scaled-evals doesn't load.
  • Opting in: uv sync --frozen --all-packages --group scaled-evals.

Changes

  • pyproject.toml: drop plugins/_temporary-scaled-evals from workspace members and change its source to { path = ..., editable = true }.
  • plugins/_temporary-scaled-evals/pyproject.toml: nemo-helix and nemo-helix-plugin sources change from workspace = true to relative paths, because uv rejects workspace sources in non-members.
  • uv.lock: the plugin moves out of the member list. Resolved versions are unchanged.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with documentation updates
  • Documentation only
  • Contributor tooling or automation
  • CI, build, or test infrastructure

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification: packaging-only change. The existing scaled-evals CI job (--group scaled-evals) still runs the plugin tests.
  • Tests not applicable — justification:
  • Documentation updated for user-visible behavior
  • Documentation not applicable — justification: the default setup docs are unchanged; this restores their expected behavior.

Verification

  • Pull request title follows the repository's Conventional Commit format
  • Every commit includes an appropriate Signed-off-by: trailer
  • uv run pre-commit run -a passes, or any blocked checks are identified below
  • Targeted tests pass, or tests are marked not applicable above
  • No secrets, API keys, or credentials are included

Targeted validation:

  • uv lock --check → passes.
  • uv sync --frozen --all-packages → uninstalls nemo-scaled-evals-plugin. uv pip show nemo-scaled-evals-plugin reports it isn't found. uv sync --frozen --all-packages --check → "Would make no changes".
  • uv sync --frozen --all-packages --group scaled-evals → installs the plugin as editable.
  • uv sync --frozen --no-dev --package nemo-scaled-evals-plugin --dry-run → still resolves, so the plugin's compose Dockerfile is unaffected.
  • nemo services run with default services → /health/ready returns 200 {"status":"ready"}, and the logs don't mention scaled-evals.
  • uv run --frozen --group scaled-evals pytest plugins/_temporary-scaled-evals/tests → 976 passed, 2 skipped, 1 failed. The failure, test_benchmark_import_api.py::test_resumed_import_attaches_opaque_image_metadata_idempotently, also fails without this change (same installed environment).
  • uv run pre-commit run -a → all hooks pass.

Known limitation: the local pre-commit ty hook doesn't request --group scaled-evals. Contributors editing plugin files should run uv sync --group scaled-evals first. The CI lint-python-types.sh already includes the group.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • The scaled-evals plugin is now installed through an opt-in editable path dependency and is excluded from workspace-wide installs. CI explicitly includes the scaled-evals dependency group.
    • Using the plugin requires external configuration. Related evaluation packages also use editable local paths.

Signed-off-by: Nick Goncharenko <ngoncharenko@nvidia.com>
@github-actions github-actions Bot added the fix label Sep 23, 2026
@ngoncharenko
ngoncharenko requested review from arpitsardhana and mckornfield and removed request for mckornfield September 23, 2026 21:19
@ngoncharenko ngoncharenko self-assigned this Sep 23, 2026
@coderabbitai

coderabbitai Bot commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: NVIDIA-NeMo/nemo-helix/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 2c8e5b2e-899b-49eb-b4ee-d061ac2bd503

📥 Commits

Reviewing files that changed from the base of the PR and between 151e23a and dc4624e.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • plugins/_temporary-scaled-evals/pyproject.toml
  • pyproject.toml

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Walkthrough

Walkthrough

The scaled-evals plugin now uses editable local paths for its dependencies and is no longer a workspace member. Root project comments describe the opt-in dependency group and its CI requirements.

Changes

Scaled-evals dependency setup

Layer / File(s) Summary
Configure editable path dependencies
pyproject.toml, plugins/_temporary-scaled-evals/pyproject.toml
The root project resolves the plugin from an editable path and removes it from workspace members. The plugin project resolves both Nemo Helix dependencies from editable paths. Comments describe the opt-in group and CI requirements.

Suggested reviewers: mckornfield

Merge Risk: ⚪ Minimal · up to dc462

Scaled-evals remains opt-in while CI and dedicated checks can install it explicitly; no actionable merge-blocking risk is established.

🚥 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 The title clearly and concisely describes the main packaging change: keeping the scaled-evals plugin out of default --all-packages installs.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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

@ngoncharenko
ngoncharenko added this pull request to the merge queue Sep 23, 2026
@github-actions

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 48870/60805 80.4% 64.7%
Integration Tests 31478/57777 54.5% 26.1%

Merged via the queue into main with commit 5ce062e Sep 23, 2026
78 checks passed
@ngoncharenko
ngoncharenko deleted the scaled-evals-opt-in/ngoncharenko branch September 23, 2026 22:03
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