Skip to content

ASE-702: Make analyst/experimentalist/eval-author CLI agents-only - #1052

Open
aleckhoury wants to merge 2 commits into
mainfrom
ASE-702-canonical-agents-cli/akhoury
Open

ASE-702: Make analyst/experimentalist/eval-author CLI agents-only#1052
aleckhoury wants to merge 2 commits into
mainfrom
ASE-702-canonical-agents-cli/akhoury

Conversation

@aleckhoury

@aleckhoury aleckhoury commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Drop legacy top-level CLI aliases (nemo experimentalist, nemo eval-author, nemo insights analyze|doctor) so these agents live only under nemo agents …
  • Update plugin READMEs, AGENTS.md, Studio Insight→experiment command, and tests to the canonical paths
  • Keep nemo insights analysis / analyze-job and leave eval-author placeholder verbs unimplemented

Fixes ASE-702.

Test plan

  • uv run nemo agents --help shows analyst / experimentalist / eval-author under Platform agents
  • uv run nemo insights --help has analysis and analyze-job, not analyze/doctor
  • uv run nemo experimentalist --help / nemo eval-author --help fail (no such command)
  • Targeted pytest: insights test_cli_profile, eval-author test_cli, experimentalist CLI tests
  • Studio InsightOpenModal unit test expects nemo agents experimentalist run

Summary by CodeRabbit

  • Changed

    • Standardized Analyst, Experimentalist, and Eval Author commands under the nemo agents namespace.
    • Removed deprecated top-level command aliases.
    • Updated optimizer experiment execution to use the Experimentalist command.
  • Documentation

    • Refreshed command examples, workflows, testbed guidance, and credential behavior documentation.
  • Tests

    • Updated CLI, end-to-end, profile, and optimizer checks to validate the new command paths and confirm legacy aliases are unavailable.

Drop legacy top-level aliases and document only `nemo agents …` paths so the optimizer plugins match the platform agent CLI naming.

Signed-off-by: Alec Khoury <akhoury@nvidia.com>
@aleckhoury
aleckhoury requested review from a team as code owners August 3, 2026 21:50
@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The CLI plugins now use nemo agents paths as their canonical interface. Legacy top-level registrations and usage are removed. Documentation, tests, Analyst integration, and the Studio optimizer command are updated accordingly.

Changes

CLI registration and command migration

Layer / File(s) Summary
Eval Author registration and validation
plugins/nemo-eval-author/...
Eval Author remains registered under nemo.cli.agents. The legacy nemo.cli entry is removed. Documentation and tests use nemo agents eval-author.
Experimentalist command migration
plugins/nemo-experimentalist/...
Experimentalist uses nemo agents experimentalist <verb>. The top-level registration and command examples are removed or updated.
Analyst command migration
plugins/nemo-insights/...
Analyst owns the doctor and run commands. Insights retains the periodic analysis surface. Tests, examples, and documentation use the nested command path.
Studio Experimentalist command integration
web/packages/studio/src/routes/optimizer/InsightOpenModal/...
The optimizer command builder and test now use nemo agents experimentalist run.

Possibly related PRs

Suggested labels: feat

Suggested reviewers: stefan-kickoff, mikeknep, ryana

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 3.70% 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 The title clearly and concisely describes the main change: restricting the analyst, experimentalist, and eval-author CLIs to the agents-only namespace.
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 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ASE-702-canonical-agents-cli/akhoury

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

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 30234/38242 79.1% 63.7%
Integration Tests 17879/36911 48.4% 20.9%

Drop entry-point and fake AgentsCLI mount coverage; keep verb placeholders and a single ctx.command_path check.

Signed-off-by: Alec Khoury <akhoury@nvidia.com>

@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.

🧹 Nitpick comments (1)
plugins/nemo-eval-author/tests/test_cli.py (1)

4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Restore coverage for the real Eval Author command registration.

The new test creates a fresh Typer app and registers a synthetic probe. It never loads the Eval Author CLI or the real nemo root. It can pass when the nemo.cli.agents entry point is missing, eval-author remains top-level, or the nested path is incorrect. Keep this unit test for _not_implemented, but add a metadata or root-CLI test for nemo agents eval-author ... and a rejection check for nemo eval-author ....

Based on the stated nested-agent registration objective and the listed removal of entry-point and legacy-path tests.

Also applies to: 27-27, 44-59

🤖 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 `@plugins/nemo-eval-author/tests/test_cli.py` at line 4, Extend the tests
beyond the synthetic Typer app and `_not_implemented` probe to load the real
Eval Author registration through the `nemo` root CLI and `nemo.cli.agents`.
Verify `eval-author` is available under the nested `nemo agents` command path,
and add a rejection check confirming the legacy top-level `nemo eval-author`
path is not accepted.
🤖 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 `@plugins/nemo-eval-author/tests/test_cli.py`:
- Line 4: Extend the tests beyond the synthetic Typer app and `_not_implemented`
probe to load the real Eval Author registration through the `nemo` root CLI and
`nemo.cli.agents`. Verify `eval-author` is available under the nested `nemo
agents` command path, and add a rejection check confirming the legacy top-level
`nemo eval-author` path is not accepted.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ffffcbcb-29f0-4b70-b2ce-730ef958c028

📥 Commits

Reviewing files that changed from the base of the PR and between bbde436 and d66a2df.

📒 Files selected for processing (1)
  • plugins/nemo-eval-author/tests/test_cli.py

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.

1 participant