Skip to content

feat: extract Eval Author into nemo-eval-author-plugin - #932

Merged
aleckhoury merged 7 commits into
mainfrom
ase-eval-author-plugin/akhoury
Jul 29, 2026
Merged

feat: extract Eval Author into nemo-eval-author-plugin#932
aleckhoury merged 7 commits into
mainfrom
ase-eval-author-plugin/akhoury

Conversation

@aleckhoury

@aleckhoury aleckhoury commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Extract Eval Author into a thin library-only plugin plugins/nemo-eval-author/ (nemo-eval-author-plugin) that owns the agent package (eval_author/) plus an AUTHOR_* model_config.
  • Keep shared Harbor evaluator, dataset staging, trace helpers, tools, cache, and client modules in Experimentalist. Eval Author hard-depends on Experimentalist and imports those modules from there.
  • Avoid a circular package dependency: Experimentalist does not declare a reverse dep on Eval Author. Insight mode still imports EvalAuthor at runtime when both packages are installed via uv sync --group experimentalist.

Dependency model

nemo-eval-author-plugin ──(hard)──► nemo-experimentalist-plugin
        ▲                                  │
        │ runtime import (soft)            │
        └──────── insight-mode loop ───────┘
Package Declares Why
Eval Author → Experimentalist Needs evaluator / staging / traces / tools
Experimentalist (no Eval Author dep) Avoids A⇄B cycle
Root experimentalist uv group both plugins Same install UX; makes soft reverse import work

This is intentionally not the Insights packaging pattern. Insights works as Experimentalist → Insights because Analyst does not need Experimentalist internals. Eval Author does, so the package arrow flips; the product relationship (Experimentalist consumes Eval Author in insight mode) stays the same via co-install.

What lives where

Eval Author plugin owns

  • eval_author/EvalAuthor NOOA agent, materialization, models, run_eval_author
  • model_config.py — prefers AUTHOR_*, falls back to EXPERIMENTALIST_*, bridges into unset Experimentalist slots for shared helpers; accepts INFERENCE_API_KEY on the NVIDIA gateway
  • .example.env — standalone credential template (CLI not wired yet)

Experimentalist keeps

  • evaluator/ (Harbor + models + factory)
  • dataset_staging.py
  • trace_analyzer.py / trace_explorer.py
  • tools.py, cache.py, client.py, and other shared NOOA runtime helpers

Tests / CI

  • Agent / materialization / run / model_config tests live under plugins/nemo-eval-author/tests/.
  • Evaluator / staging / trace unit tests stay under Experimentalist.
  • Loop staging + EvolutionaryOptimizerConfig nesting / legacy curator rejection stay in Experimentalist.
  • Root CI on 3.11 skips Eval Author discovery the same way as Experimentalist (tests/discovery_exclusions.py); ty overrides cover Eval Author's nooa/harbor imports and agent empty bodies.

Test plan

  • uv sync --group experimentalist
  • uv run --frozen pytest -q plugins/nemo-eval-author/tests plugins/nemo-experimentalist/tests (575 passed, 2 credential-gated skips)
  • uv run ruff check / ruff format --check clean for touched plugins
  • ty CI ignores for Eval Author mirror Experimentalist
  • Root discovery excludes Eval Author when the plugin is not installed (3.11 CI)
  • Confirm Experimentalist insight-mode smoke still stages inputs and calls EvalAuthor
  • CI green on this PR after the thin-plugin + CI gate commits

Summary by CodeRabbit

  • New Features

    • Added the standalone Eval Author plugin for generating, validating, and finalizing insight suites with reproducible identities and provenance.
    • Added configurable model credentials, model tiers, run limits, validation repair attempts, and dataset outputs.
    • Added environment configuration templates and compatibility support for existing Experimentalist credentials.
    • Updated Experimentalist integration to use the new Eval Author plugin.
  • Documentation

    • Added installation, configuration, invocation, API reference, and plugin lifecycle guidance.
  • Tests

    • Expanded coverage for configuration validation, dataset staging, trace references, plugin boundaries, and end-to-end behavior.

@aleckhoury
aleckhoury requested review from a team as code owners July 27, 2026 21:31
@github-actions github-actions Bot added the feat label Jul 27, 2026
Base automatically changed from ase-622-insight-suite-metrics/rangilly to main July 27, 2026 21:37
Move Eval Author, the Harbor evaluator stack, staging, and trace analysis
into a library plugin so Experimentalist depends on it one-way like Insights,
without Eval Author depending on Experimentalist.

Signed-off-by: Alec Khoury <akhoury@nvidia.com>
@aleckhoury
aleckhoury force-pushed the ase-eval-author-plugin/akhoury branch from 2a1a837 to ea2fc80 Compare July 27, 2026 21:55
@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 28094/35888 78.3% 62.7%
Integration Tests 16327/34606 47.2% 19.6%

@coderabbitai

coderabbitai Bot commented Jul 27, 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

Changes

The PR extracts Eval Author into nemo-eval-author-plugin, adds standalone models, credential resolution, deterministic insight-suite materialization, and rewires Experimentalist imports and dependencies. It also adds workspace packaging, migration documentation, boundary checks, integration tests, and optional test discovery handling.

Eval Author plugin extraction

Layer / File(s) Summary
Package configuration and public contracts
plugins/nemo-eval-author/pyproject.toml, plugins/nemo-eval-author/README.md, plugins/nemo-eval-author/src/..., pyproject.toml, plugins/nemo-experimentalist/AGENTS.md
Defines the new plugin package, public result/configuration models, configuration templates, documentation, workspace wiring, and migration rules.
Standalone configuration and runtime entrypoints
plugins/nemo-eval-author/src/nemo_eval_author_plugin/model_config.py, .../_env_bridge.py, .../eval_author/agent.py, .../eval_author/run.py
Resolves AUTHOR credentials and model tiers, bridges environment variables during import, caches completion clients, and instantiates the extracted Eval Author agent.
Insight suite staging and finalization
plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/materialization.py
Stages template tasks, validates and promotes local suites, records analysis, computes provenance hashes, and finalizes Harbor datasets with deterministic identities.
Experimentalist namespace and dependency wiring
plugins/nemo-experimentalist/src/..., plugins/nemo-experimentalist/pyproject.toml
Routes Eval Author and shared component imports through the extracted package, relocates rationale models, and adds the plugin dependency.
Integration validation and test migration
plugins/nemo-eval-author/tests/*, plugins/nemo-experimentalist/tests/*, tests/discovery_exclusions.py
Tests credential resolution, environment bridging, package boundaries, configuration, staging isolation, trace URI parsing, and updated Eval Author integration paths.

Sequence Diagram(s)

sequenceDiagram
  participant EvalAuthorRun
  participant EnvBridge
  participant ModelConfig
  participant Experimentalist
  EvalAuthorRun->>EnvBridge: import-time AUTHOR_* bridging
  EvalAuthorRun->>ModelConfig: resolve model and credentials
  ModelConfig->>ModelConfig: create or reuse CompletionClient
  EvalAuthorRun->>Experimentalist: load shared evaluator, trace, and tool components
  Experimentalist-->>EvalAuthorRun: execute extracted Eval Author flow
Loading

Possibly related PRs

Suggested reviewers: svvarom, nicot, crookedstorm, maxdubrinsky, mckornfield

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 49.62% 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 matches the main change: extracting Eval Author into the new nemo-eval-author-plugin.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ase-eval-author-plugin/akhoury
🔧 Fix failing CI
  • Fix failing CI in branch ase-eval-author-plugin/akhoury

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

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

🧹 Nitpick comments (4)
plugins/nemo-eval-author/src/nemo_eval_author_plugin/backend.py (1)

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

Move _redact_url import to top-level and stop reaching into a private symbol.

_redact_url is imported locally, inside an except block, from a leading-underscore (private) name in repository.py. It's already imported at the top of this file alongside AgentSource, clone_agent_repo, etc. (lines 23-29) — add it there instead of a function-local import, and consider exposing it without the underscore if it's meant to be used cross-module.

♻️ Proposed fix
 from nemo_eval_author_plugin.repository import (
     AgentSource,
     clone_agent_repo,
     looks_like_git,
     split_agent_spec,
     split_git_ref,
+    _redact_url,
 )
@@
     async def _clone_git_agent(self, agent: str, dest: Path, *, clone_depth: int | None = None) -> AgentSource:
         try:
             return await asyncio.to_thread(clone_agent_repo, agent, dest, clone_depth=clone_depth)
         except subprocess.CalledProcessError:
             remote, _ = split_git_ref(split_agent_spec(agent)[0])
-            from nemo_eval_author_plugin.repository import _redact_url
-
             raise ValueError(f"failed to fetch --agent {_redact_url(remote)!r}") from None
🤖 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/src/nemo_eval_author_plugin/backend.py` around lines
98 - 106, Move the _redact_url import from the _clone_git_agent exception block
to the module-level imports alongside AgentSource and clone_agent_repo. Continue
using the existing _redact_url symbol in the error message, without changing
unrelated cloning or error-handling behavior.
plugins/nemo-eval-author/README.md (1)

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

Keep this page to one Diátaxis quadrant.

It currently mixes API reference, installation how-to, and internal TODO content. Add prerequisites before the first section, separate the API reference from setup instructions, and add a Next Steps section with cross-links.

As per coding guidelines, each documentation page must fit one Diátaxis quadrant, list prerequisites at the top, and include a Next Steps section with cross-links.

🤖 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/README.md` around lines 1 - 36, Restructure the
README into a single Diátaxis quadrant by choosing either API reference or
setup/how-to as its primary purpose, moving or removing content from the other
category. Add a prerequisites section before the first main section, retain only
relevant content for the chosen quadrant, and finish with a “Next Steps” section
containing cross-links to related documentation. Update the README structure and
headings while preserving the accurate public API or installation details.

Source: Coding guidelines

plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/REFERENCE.md (1)

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

Missing "Next Steps" cross-links.

Add a closing section linking to eval_author/README.md / usage docs.

As per coding guidelines: "Include 'Next Steps' section at the end with cross-links to related documentation content."

🤖 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/src/nemo_eval_author_plugin/eval_author/REFERENCE.md`
around lines 1 - 23, Add a closing “Next Steps” section to the EvalAuthorResult
reference document, including cross-links to eval_author/README.md and the
relevant usage documentation. Keep it as the final section after the existing
content and use repository-relative documentation links.

Source: Coding guidelines

plugins/nemo-eval-author/src/nemo_eval_author_plugin/model_config.py (1)

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

Default model names duplicated between getters and log_model_config.

Same fallback strings hardcoded in 4 places; a future default change in one getter won't be reflected in the log output.

♻️ Proposed fix
+_SMART_MODEL_DEFAULT = "openai/openai/openai/gpt-5.5"
+_MID_MODEL_DEFAULT = "openai/gcp/google/gemini-3.5-flash"
+_FAST_MODEL_DEFAULT = "openai/openai/openai/gpt-5-mini"
+
 `@functools.cache`
 def get_smart_model() -> CompletionClient:
     ...
-    name = _optional_env("EXPERIMENTALIST_SMART_MODEL_NAME", "openai/openai/openai/gpt-5.5")
+    name = _optional_env("EXPERIMENTALIST_SMART_MODEL_NAME", _SMART_MODEL_DEFAULT)

Apply similarly to the other two getters and to log_model_config.

Also applies to: 53-53, 70-70, 95-97

🤖 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/src/nemo_eval_author_plugin/model_config.py` at line
36, Centralize the default model-name values used by the getters and reuse those
symbols in log_model_config. Update the getters around
EXPERIMENTALIST_SMART_MODEL_NAME and the other two model configuration keys,
plus log_model_config, so each fallback is defined once and logging reflects any
future default change.
🤖 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 `@plugins/nemo-eval-author/pyproject.toml`:
- Around line 5-12: Resolve the Python-version mismatch between the project
metadata and util.py: either raise requires-python to exclude Python 3.11, or
make nooa available for Python 3.11 by removing its conditional dependency
marker and ensuring util.py’s unconditional import remains valid.

In `@plugins/nemo-eval-author/src/nemo_eval_author_plugin/cache.py`:
- Around line 109-117: Move the path.parent.mkdir call in the cache write flow
into the existing try block that handles tmp.write_text and tmp.replace. Ensure
OSError from cache-directory creation is caught by the existing warning log and
the optional cache write is skipped without propagating the exception.

In `@plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/run.py`:
- Line 79: Update the mode argument docstring in run_eval_author to reference
make_eval_author_backend instead of Experimentalist’s backend factory, keeping
the documentation aligned with the current backend creation contract.

In `@plugins/nemo-eval-author/src/nemo_eval_author_plugin/evaluator/models.py`:
- Around line 138-155: Update __aenter__ to call _stop_started_runtime only
after the dependency start command has actually been attempted successfully;
when self._runtime.start is None, raise ValueError without invoking the stop
command. Preserve cleanup and exception propagation for failures occurring
during or after run_dependency_command(self._runtime.start, "start").

In `@plugins/nemo-eval-author/src/nemo_eval_author_plugin/tools.py`:
- Around line 110-123: Update list_agents sorting to handle directory names
without a "-" separator before accessing the numeric suffix. Preserve natural
numeric ordering for agent-N names while safely assigning a fallback sort key to
other directories containing metadata.json.
- Line 171: Update the read_analysis_file method signature to use the concrete
str | int type hint directly instead of a quoted string, leaving the limit
annotation and method behavior unchanged.
- Around line 189-208: Update get_metadata to honor its documented fallback
contract: catch missing-file, JSON parsing, read, and Candidate validation
failures, then return a minimal Candidate with default values. Preserve the
existing successful parsing and entity_id assignment behavior for valid
metadata.

---

Nitpick comments:
In `@plugins/nemo-eval-author/README.md`:
- Around line 1-36: Restructure the README into a single Diátaxis quadrant by
choosing either API reference or setup/how-to as its primary purpose, moving or
removing content from the other category. Add a prerequisites section before the
first main section, retain only relevant content for the chosen quadrant, and
finish with a “Next Steps” section containing cross-links to related
documentation. Update the README structure and headings while preserving the
accurate public API or installation details.

In `@plugins/nemo-eval-author/src/nemo_eval_author_plugin/backend.py`:
- Around line 98-106: Move the _redact_url import from the _clone_git_agent
exception block to the module-level imports alongside AgentSource and
clone_agent_repo. Continue using the existing _redact_url symbol in the error
message, without changing unrelated cloning or error-handling behavior.

In
`@plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/REFERENCE.md`:
- Around line 1-23: Add a closing “Next Steps” section to the EvalAuthorResult
reference document, including cross-links to eval_author/README.md and the
relevant usage documentation. Keep it as the final section after the existing
content and use repository-relative documentation links.

In `@plugins/nemo-eval-author/src/nemo_eval_author_plugin/model_config.py`:
- Line 36: Centralize the default model-name values used by the getters and
reuse those symbols in log_model_config. Update the getters around
EXPERIMENTALIST_SMART_MODEL_NAME and the other two model configuration keys,
plus log_model_config, so each fallback is defined once and logging reflects any
future default change.
🪄 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: f0f898a7-d0e3-4a28-8e28-2ed1863c554e

📥 Commits

Reviewing files that changed from the base of the PR and between 1b3782c and ea2fc80.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (66)
  • plugins/README.md
  • plugins/nemo-eval-author/README.md
  • plugins/nemo-eval-author/pyproject.toml
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/backend.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/cache.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/client.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/dataset_staging.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/README.md
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/REFERENCE.md
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/agent.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/config.yaml
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/materialization.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/models.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/run.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/evaluator/__init__.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/evaluator/base.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/evaluator/factory.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/evaluator/harbor.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/evaluator/models.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/holdout_utils.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/model_config.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/py.typed
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/rationale.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/repository.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/tools.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/trace_analyzer.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/trace_explorer.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/util.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/workspace_entities.py
  • plugins/nemo-eval-author/tests/test_dataset_staging.py
  • plugins/nemo-eval-author/tests/test_eval_author_agent.py
  • plugins/nemo-eval-author/tests/test_eval_author_materialization.py
  • plugins/nemo-eval-author/tests/test_eval_author_run.py
  • plugins/nemo-eval-author/tests/test_evaluator_base.py
  • plugins/nemo-eval-author/tests/test_evaluator_factory.py
  • plugins/nemo-eval-author/tests/test_evaluator_harbor.py
  • plugins/nemo-eval-author/tests/test_trace_explorer_from_ref.py
  • plugins/nemo-experimentalist/AGENTS.md
  • plugins/nemo-experimentalist/benchmarks/run.py
  • plugins/nemo-experimentalist/pyproject.toml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/entities.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/analyzer.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/coder.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/goal_tree.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/insight_promotion.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/loop.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/rationalizer.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/trace_scorer.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/deps.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/experimentalist_backend.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/otlp.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/run.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/resolve.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_dataset_staging_loop.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_eval_author_repair_e2e.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_loop_insight_suite.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_loop_reporting.py
  • plugins/nemo-experimentalist/tests/test_deps.py
  • plugins/nemo-experimentalist/tests/test_eval_author_config.py
  • plugins/nemo-experimentalist/tests/test_experiment_cli.py
  • plugins/nemo-experimentalist/tests/test_experimentalist_analyzer.py
  • plugins/nemo-experimentalist/tests/test_experimentalist_backend.py
  • plugins/nemo-experimentalist/tests/test_experimentalist_benchmark.py
  • plugins/nemo-experimentalist/tests/test_experimentalist_run.py
  • plugins/nemo-experimentalist/tests/test_otlp.py
  • pyproject.toml

Comment thread plugins/nemo-eval-author/pyproject.toml Outdated
Comment thread plugins/nemo-eval-author/src/nemo_eval_author_plugin/cache.py Outdated
Comment thread plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/run.py Outdated
Comment thread plugins/nemo-eval-author/src/nemo_eval_author_plugin/tools.py Outdated
Comment thread plugins/nemo-eval-author/src/nemo_eval_author_plugin/tools.py Outdated
Comment thread plugins/nemo-eval-author/src/nemo_eval_author_plugin/tools.py Outdated

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

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 7

🧹 Nitpick comments (4)
plugins/nemo-eval-author/src/nemo_eval_author_plugin/backend.py (1)

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

Move _redact_url import to top-level and stop reaching into a private symbol.

_redact_url is imported locally, inside an except block, from a leading-underscore (private) name in repository.py. It's already imported at the top of this file alongside AgentSource, clone_agent_repo, etc. (lines 23-29) — add it there instead of a function-local import, and consider exposing it without the underscore if it's meant to be used cross-module.

♻️ Proposed fix
 from nemo_eval_author_plugin.repository import (
     AgentSource,
     clone_agent_repo,
     looks_like_git,
     split_agent_spec,
     split_git_ref,
+    _redact_url,
 )
@@
     async def _clone_git_agent(self, agent: str, dest: Path, *, clone_depth: int | None = None) -> AgentSource:
         try:
             return await asyncio.to_thread(clone_agent_repo, agent, dest, clone_depth=clone_depth)
         except subprocess.CalledProcessError:
             remote, _ = split_git_ref(split_agent_spec(agent)[0])
-            from nemo_eval_author_plugin.repository import _redact_url
-
             raise ValueError(f"failed to fetch --agent {_redact_url(remote)!r}") from None
🤖 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/src/nemo_eval_author_plugin/backend.py` around lines
98 - 106, Move the _redact_url import from the _clone_git_agent exception block
to the module-level imports alongside AgentSource and clone_agent_repo. Continue
using the existing _redact_url symbol in the error message, without changing
unrelated cloning or error-handling behavior.
plugins/nemo-eval-author/README.md (1)

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

Keep this page to one Diátaxis quadrant.

It currently mixes API reference, installation how-to, and internal TODO content. Add prerequisites before the first section, separate the API reference from setup instructions, and add a Next Steps section with cross-links.

As per coding guidelines, each documentation page must fit one Diátaxis quadrant, list prerequisites at the top, and include a Next Steps section with cross-links.

🤖 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/README.md` around lines 1 - 36, Restructure the
README into a single Diátaxis quadrant by choosing either API reference or
setup/how-to as its primary purpose, moving or removing content from the other
category. Add a prerequisites section before the first main section, retain only
relevant content for the chosen quadrant, and finish with a “Next Steps” section
containing cross-links to related documentation. Update the README structure and
headings while preserving the accurate public API or installation details.

Source: Coding guidelines

plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/REFERENCE.md (1)

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

Missing "Next Steps" cross-links.

Add a closing section linking to eval_author/README.md / usage docs.

As per coding guidelines: "Include 'Next Steps' section at the end with cross-links to related documentation content."

🤖 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/src/nemo_eval_author_plugin/eval_author/REFERENCE.md`
around lines 1 - 23, Add a closing “Next Steps” section to the EvalAuthorResult
reference document, including cross-links to eval_author/README.md and the
relevant usage documentation. Keep it as the final section after the existing
content and use repository-relative documentation links.

Source: Coding guidelines

plugins/nemo-eval-author/src/nemo_eval_author_plugin/model_config.py (1)

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

Default model names duplicated between getters and log_model_config.

Same fallback strings hardcoded in 4 places; a future default change in one getter won't be reflected in the log output.

♻️ Proposed fix
+_SMART_MODEL_DEFAULT = "openai/openai/openai/gpt-5.5"
+_MID_MODEL_DEFAULT = "openai/gcp/google/gemini-3.5-flash"
+_FAST_MODEL_DEFAULT = "openai/openai/openai/gpt-5-mini"
+
 `@functools.cache`
 def get_smart_model() -> CompletionClient:
     ...
-    name = _optional_env("EXPERIMENTALIST_SMART_MODEL_NAME", "openai/openai/openai/gpt-5.5")
+    name = _optional_env("EXPERIMENTALIST_SMART_MODEL_NAME", _SMART_MODEL_DEFAULT)

Apply similarly to the other two getters and to log_model_config.

Also applies to: 53-53, 70-70, 95-97

🤖 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/src/nemo_eval_author_plugin/model_config.py` at line
36, Centralize the default model-name values used by the getters and reuse those
symbols in log_model_config. Update the getters around
EXPERIMENTALIST_SMART_MODEL_NAME and the other two model configuration keys,
plus log_model_config, so each fallback is defined once and logging reflects any
future default change.
🤖 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 `@plugins/nemo-eval-author/pyproject.toml`:
- Around line 5-12: Resolve the Python-version mismatch between the project
metadata and util.py: either raise requires-python to exclude Python 3.11, or
make nooa available for Python 3.11 by removing its conditional dependency
marker and ensuring util.py’s unconditional import remains valid.

In `@plugins/nemo-eval-author/src/nemo_eval_author_plugin/cache.py`:
- Around line 109-117: Move the path.parent.mkdir call in the cache write flow
into the existing try block that handles tmp.write_text and tmp.replace. Ensure
OSError from cache-directory creation is caught by the existing warning log and
the optional cache write is skipped without propagating the exception.

In `@plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/run.py`:
- Line 79: Update the mode argument docstring in run_eval_author to reference
make_eval_author_backend instead of Experimentalist’s backend factory, keeping
the documentation aligned with the current backend creation contract.

In `@plugins/nemo-eval-author/src/nemo_eval_author_plugin/evaluator/models.py`:
- Around line 138-155: Update __aenter__ to call _stop_started_runtime only
after the dependency start command has actually been attempted successfully;
when self._runtime.start is None, raise ValueError without invoking the stop
command. Preserve cleanup and exception propagation for failures occurring
during or after run_dependency_command(self._runtime.start, "start").

In `@plugins/nemo-eval-author/src/nemo_eval_author_plugin/tools.py`:
- Around line 110-123: Update list_agents sorting to handle directory names
without a "-" separator before accessing the numeric suffix. Preserve natural
numeric ordering for agent-N names while safely assigning a fallback sort key to
other directories containing metadata.json.
- Line 171: Update the read_analysis_file method signature to use the concrete
str | int type hint directly instead of a quoted string, leaving the limit
annotation and method behavior unchanged.
- Around line 189-208: Update get_metadata to honor its documented fallback
contract: catch missing-file, JSON parsing, read, and Candidate validation
failures, then return a minimal Candidate with default values. Preserve the
existing successful parsing and entity_id assignment behavior for valid
metadata.

---

Nitpick comments:
In `@plugins/nemo-eval-author/README.md`:
- Around line 1-36: Restructure the README into a single Diátaxis quadrant by
choosing either API reference or setup/how-to as its primary purpose, moving or
removing content from the other category. Add a prerequisites section before the
first main section, retain only relevant content for the chosen quadrant, and
finish with a “Next Steps” section containing cross-links to related
documentation. Update the README structure and headings while preserving the
accurate public API or installation details.

In `@plugins/nemo-eval-author/src/nemo_eval_author_plugin/backend.py`:
- Around line 98-106: Move the _redact_url import from the _clone_git_agent
exception block to the module-level imports alongside AgentSource and
clone_agent_repo. Continue using the existing _redact_url symbol in the error
message, without changing unrelated cloning or error-handling behavior.

In
`@plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/REFERENCE.md`:
- Around line 1-23: Add a closing “Next Steps” section to the EvalAuthorResult
reference document, including cross-links to eval_author/README.md and the
relevant usage documentation. Keep it as the final section after the existing
content and use repository-relative documentation links.

In `@plugins/nemo-eval-author/src/nemo_eval_author_plugin/model_config.py`:
- Line 36: Centralize the default model-name values used by the getters and
reuse those symbols in log_model_config. Update the getters around
EXPERIMENTALIST_SMART_MODEL_NAME and the other two model configuration keys,
plus log_model_config, so each fallback is defined once and logging reflects any
future default change.
🪄 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: f0f898a7-d0e3-4a28-8e28-2ed1863c554e

📥 Commits

Reviewing files that changed from the base of the PR and between 1b3782c and ea2fc80.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (66)
  • plugins/README.md
  • plugins/nemo-eval-author/README.md
  • plugins/nemo-eval-author/pyproject.toml
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/backend.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/cache.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/client.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/dataset_staging.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/README.md
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/REFERENCE.md
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/agent.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/config.yaml
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/materialization.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/models.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/run.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/evaluator/__init__.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/evaluator/base.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/evaluator/factory.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/evaluator/harbor.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/evaluator/models.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/holdout_utils.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/model_config.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/py.typed
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/rationale.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/repository.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/tools.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/trace_analyzer.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/trace_explorer.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/util.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/workspace_entities.py
  • plugins/nemo-eval-author/tests/test_dataset_staging.py
  • plugins/nemo-eval-author/tests/test_eval_author_agent.py
  • plugins/nemo-eval-author/tests/test_eval_author_materialization.py
  • plugins/nemo-eval-author/tests/test_eval_author_run.py
  • plugins/nemo-eval-author/tests/test_evaluator_base.py
  • plugins/nemo-eval-author/tests/test_evaluator_factory.py
  • plugins/nemo-eval-author/tests/test_evaluator_harbor.py
  • plugins/nemo-eval-author/tests/test_trace_explorer_from_ref.py
  • plugins/nemo-experimentalist/AGENTS.md
  • plugins/nemo-experimentalist/benchmarks/run.py
  • plugins/nemo-experimentalist/pyproject.toml
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/entities.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/analyzer.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/coder.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/goal_tree.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/insight_promotion.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/loop.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/rationalizer.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/trace_scorer.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/deps.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/experimentalist_backend.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/otlp.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/run.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/resolve.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_dataset_staging_loop.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_eval_author_repair_e2e.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_loop_insight_suite.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_loop_reporting.py
  • plugins/nemo-experimentalist/tests/test_deps.py
  • plugins/nemo-experimentalist/tests/test_eval_author_config.py
  • plugins/nemo-experimentalist/tests/test_experiment_cli.py
  • plugins/nemo-experimentalist/tests/test_experimentalist_analyzer.py
  • plugins/nemo-experimentalist/tests/test_experimentalist_backend.py
  • plugins/nemo-experimentalist/tests/test_experimentalist_benchmark.py
  • plugins/nemo-experimentalist/tests/test_experimentalist_run.py
  • plugins/nemo-experimentalist/tests/test_otlp.py
  • pyproject.toml
🛑 Comments failed to post (1)
plugins/nemo-eval-author/src/nemo_eval_author_plugin/evaluator/models.py (1)

138-155: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Stop command runs even when start was never attempted.

If self._runtime.start is None, ValueError is raised before any command runs, but the except BaseException branch still calls _stop_started_runtime(), executing self._runtime.stop (if set) against dependencies that were never started.

🐛 Proposed fix
     async def __aenter__(self) -> DependencyRuntime | None:
         """Start dependencies and return the runtime that was entered."""
         if self._runtime is None:
             return None
+        if self._runtime.start is None:
+            raise ValueError("DependencyRuntime requires start")
         try:
-            if self._runtime.start is None:
-                raise ValueError("DependencyRuntime requires start")
             await run_dependency_command(self._runtime.start, "start")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

    async def __aenter__(self) -> DependencyRuntime | None:
        """Start dependencies and return the runtime that was entered."""
        if self._runtime is None:
            return None
        if self._runtime.start is None:
            raise ValueError("DependencyRuntime requires start")
        try:
            await run_dependency_command(self._runtime.start, "start")

            if self._runtime.readiness is not None:
                await run_dependency_command(self._runtime.readiness, "readiness")
        except BaseException:
            try:
                await self._stop_started_runtime()
            except Exception:
                pass
            raise
        return self._runtime
🤖 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/src/nemo_eval_author_plugin/evaluator/models.py`
around lines 138 - 155, Update __aenter__ to call _stop_started_runtime only
after the dependency start command has actually been attempted successfully;
when self._runtime.start is None, raise ValueError without invoking the stop
command. Preserve cleanup and exception propagation for failures occurring
during or after run_dependency_command(self._runtime.start, "start").

Own only the agent package and AUTHOR_* config in the plugin; restore
evaluator/staging/trace helpers to Experimentalist and install both via
the experimentalist uv group to avoid a circular package dependency.

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.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/materialization.py (1)

185-203: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Make task_slug static or accept self.

self.task_slug(index, trace_ref) binds self, but the method declares only (index, trace_ref). Every staging call raises TypeError before copying tasks.

Proposed fix
 class InsightSuite:
+    `@staticmethod`
     def task_slug(index: int, trace_ref: str) -> str:
🤖 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/src/nemo_eval_author_plugin/eval_author/materialization.py`
around lines 185 - 203, Update the task_slug method declaration to accept self,
or mark it as static while preserving its current call from stage. Ensure the
signature matches the invocation in InsightSuite.stage so staging proceeds
without a TypeError.
🧹 Nitpick comments (2)
plugins/nemo-eval-author/README.md (1)

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

Separate the README’s Diátaxis concerns.

This page mixes architecture explanation, API reference, and a credentials how-to. Move the API material to REFERENCE.md; keep this page as a how-to with prerequisites first, parallel Python/CLI guidance where supported, and a Next Steps section.

As per coding guidelines, each documentation page must fit one Diátaxis quadrant and include prerequisites and Next Steps.

🤖 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/README.md` around lines 1 - 45, Restructure the
README as a how-to guide: add prerequisites first, retain standalone credential
setup, provide parallel Python and CLI guidance where the CLI is supported, and
finish with a Next Steps section. Move the “Public API” imports and API details
to a new REFERENCE.md, and remove architecture/explanatory material that belongs
outside the how-to while preserving the documented standalone and
Experimentalist credential behavior.

Source: Coding guidelines

plugins/nemo-experimentalist/tests/experimentalist/test_dataset_staging.py (1)

156-156: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Retain equivalent orchestration coverage.

The deleted test covered staging train/validation/template before Eval Author execution, awaiting shell.close, and preventing generated-task side effects after failure. Port it to the current runner or verify equivalent coverage in test_dataset_staging_loop.py.

🤖 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-experimentalist/tests/experimentalist/test_dataset_staging.py`
at line 156, Restore the orchestration coverage removed from the dataset staging
tests by porting it to the current runner or confirming equivalent coverage in
test_dataset_staging_loop.py: verify train, validation, and template staging
occurs before Eval Author execution, await shell.close, and ensure
generated-task side effects do not occur after failure.
🤖 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 `@plugins/nemo-eval-author/tests/conftest.py`:
- Around line 16-19: Update the AUTHOR_API_BASE and EXPERIMENTALIST_API_BASE
defaults in the test environment setup to use HTTPS URLs with non-routable
.invalid hosts. Keep the existing API key placeholders unchanged and ensure both
endpoints prevent accidental transmission of real credentials to an external
service.

---

Outside diff comments:
In
`@plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/materialization.py`:
- Around line 185-203: Update the task_slug method declaration to accept self,
or mark it as static while preserving its current call from stage. Ensure the
signature matches the invocation in InsightSuite.stage so staging proceeds
without a TypeError.

---

Nitpick comments:
In `@plugins/nemo-eval-author/README.md`:
- Around line 1-45: Restructure the README as a how-to guide: add prerequisites
first, retain standalone credential setup, provide parallel Python and CLI
guidance where the CLI is supported, and finish with a Next Steps section. Move
the “Public API” imports and API details to a new REFERENCE.md, and remove
architecture/explanatory material that belongs outside the how-to while
preserving the documented standalone and Experimentalist credential behavior.

In `@plugins/nemo-experimentalist/tests/experimentalist/test_dataset_staging.py`:
- Line 156: Restore the orchestration coverage removed from the dataset staging
tests by porting it to the current runner or confirming equivalent coverage in
test_dataset_staging_loop.py: verify train, validation, and template staging
occurs before Eval Author execution, await shell.close, and ensure
generated-task side effects do not occur after failure.
🪄 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: 703ea3d1-98f3-4e8b-af48-131d6967557c

📥 Commits

Reviewing files that changed from the base of the PR and between ea2fc80 and f273841.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (38)
  • plugins/nemo-eval-author/.example.env
  • plugins/nemo-eval-author/README.md
  • plugins/nemo-eval-author/pyproject.toml
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/README.md
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/agent.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/materialization.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/models.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/run.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/model_config.py
  • plugins/nemo-eval-author/tests/conftest.py
  • plugins/nemo-eval-author/tests/test_eval_author_agent.py
  • plugins/nemo-eval-author/tests/test_eval_author_materialization.py
  • plugins/nemo-eval-author/tests/test_eval_author_run.py
  • plugins/nemo-eval-author/tests/test_model_config.py
  • plugins/nemo-experimentalist/AGENTS.md
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/analyzer.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/coder.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/insight_promotion.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/loop.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/rationale.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/rationalizer.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/trace_analyzer.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/trace_scorer.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/experimentalist_backend.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/run.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/resolve.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_dataset_staging.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_dataset_staging_loop.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_eval_author_repair_e2e.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_evaluator_harbor.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_loop_insight_suite.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_loop_reporting.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_trace_explorer_from_ref.py
  • plugins/nemo-experimentalist/tests/test_experiment_cli.py
  • plugins/nemo-experimentalist/tests/test_experimentalist_analyzer.py
  • plugins/nemo-experimentalist/tests/test_experimentalist_backend.py
  • plugins/nemo-experimentalist/tests/test_experimentalist_run.py
  • pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (9)
  • plugins/nemo-experimentalist/tests/test_experimentalist_analyzer.py
  • plugins/nemo-eval-author/pyproject.toml
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/README.md
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/trace_scorer.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_dataset_staging_loop.py
  • plugins/nemo-eval-author/tests/test_eval_author_agent.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_eval_author_repair_e2e.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_loop_insight_suite.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_loop_reporting.py

Comment thread plugins/nemo-eval-author/tests/conftest.py Outdated
…entalist

Eval Author ships in the 3.12-only experimentalist group, so its tests must
be skipped when root CI collects on 3.11 and its nooa/harbor imports and
agent method stubs need the same ty overrides Experimentalist already has.
Also apply ruff import sorting and formatting missed by the module move.

Signed-off-by: Alec Khoury <akhoury@nvidia.com>
A plaintext placeholder base paired with a real key from the environment
could send that key unencrypted on an unmocked call. Also document why the
plugin declares a 3.11 floor while nooa and harbor are gated to 3.12.

Signed-off-by: Alec Khoury <akhoury@nvidia.com>
Restore the repo lint config for the plugin. A local [tool.ruff] table had
made the plugin's pyproject the root ruff config for that subtree, which
disabled import sorting and reflowed the moved Eval Author modules at 88
columns, hiding that the extraction was a pure move. The E402 waiver that
table existed for is gone too, replaced by an _env_bridge module imported
for its side effect, so isort maintains the ordering the credential bridge
depends on rather than a comment asking readers not to reshuffle imports.

Only forward INFERENCE_API_KEY to the NVIDIA gateway over HTTPS, matching
nemo_experimentalist_plugin.cli. Matching on hostname alone would have sent
the key to a plain-http base in cleartext.

Declare Experimentalist's dependency on Eval Author, which insight mode
already imports at module scope. That is the arrow that survives, so pin
the modules Eval Author still borrows in tests/test_plugin_boundary.py to
keep the reverse coupling shrinking, and tag the transitional credential
fallback and env bridge TODO(eval-author-standalone).

Cover the previously untested bridge with a subprocess test that imports
the agent with EXPERIMENTALIST_* stripped, move the Eval Author e2e canary
into the plugin and fix a credential gate that could never skip, drop dead
code and the unused pyyaml dependency, and rename .example.env to
.env.example to match the rest of the repo.

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.

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 `@plugins/nemo-eval-author/.env.example`:
- Around line 5-7: Update the setup instructions in .env.example to remove the
implication that copying the file to .env enables automatic loading, and instead
direct users to export or source the variables in their shell until CLI
environment-file loading is implemented.

In `@plugins/nemo-eval-author/tests/test_eval_author_repair_e2e.py`:
- Around line 35-37: Update the test credential gate to resolve the API key
through the same model-config contract used by model_config._api_key(),
including supported INFERENCE_API_KEY configurations for the HTTPS NVIDIA
gateway. Preserve the existing base-URL resolution and placeholder checks, while
ensuring valid live-canary credentials are not skipped.
🪄 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: 96ebb518-de69-483b-9b10-e20d194c881d

📥 Commits

Reviewing files that changed from the base of the PR and between 38f2e8b and aa21cde.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (19)
  • .gitignore
  • plugins/nemo-eval-author/.env.example
  • plugins/nemo-eval-author/README.md
  • plugins/nemo-eval-author/pyproject.toml
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/_env_bridge.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/agent.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/materialization.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/eval_author/run.py
  • plugins/nemo-eval-author/src/nemo_eval_author_plugin/model_config.py
  • plugins/nemo-eval-author/tests/conftest.py
  • plugins/nemo-eval-author/tests/test_eval_author_agent.py
  • plugins/nemo-eval-author/tests/test_eval_author_materialization.py
  • plugins/nemo-eval-author/tests/test_eval_author_repair_e2e.py
  • plugins/nemo-eval-author/tests/test_eval_author_run.py
  • plugins/nemo-eval-author/tests/test_model_config.py
  • plugins/nemo-eval-author/tests/test_plugin_boundary.py
  • plugins/nemo-experimentalist/AGENTS.md
  • plugins/nemo-experimentalist/pyproject.toml
  • pyproject.toml
💤 Files with no reviewable changes (1)
  • plugins/nemo-eval-author/pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (4)
  • plugins/nemo-eval-author/tests/conftest.py
  • plugins/nemo-eval-author/README.md
  • pyproject.toml
  • plugins/nemo-experimentalist/AGENTS.md

Comment thread plugins/nemo-eval-author/.env.example
Comment thread plugins/nemo-eval-author/tests/test_eval_author_repair_e2e.py
…triple

The _completion_client docstring called its arguments a "model triple", which
made a reader decode the term before learning what the cache is keyed on. Name
the three values, and say why the base URL and key are part of the key rather
than leaving that implicit: reusing a client across endpoints or credentials
would be a correctness bug, not just a missed optimization.

Also record which alternative was rejected. The previous note asserted that the
duplicate client Experimentalist holds "is the price of Eval Author not
importing from it", which only reads as a reason if you already know an earlier
revision shared one factory and was reverted to keep this module standalone.

Signed-off-by: Alec Khoury <akhoury@nvidia.com>
main dropped Python 3.11 (requires-python is now >=3.12 and ty resolves on
3.12), which turns both conflicting hunks in the root pyproject.toml into
obsolete 3.11 accommodations rather than changes worth keeping:

- the experimentalist dependency group no longer needs
  "; python_full_version >= '3.12'", so it lists nemo-experimentalist-plugin
  and nemo-eval-author-plugin unmarked.
- the unresolved-import = "ignore" ty override existed only because nooa and
  harbor went unresolved when ty resolved on 3.11. main deleted it and this
  merge does not resurrect it; verified that neither plugin reports an
  unresolved-import diagnostic without it.

Carried the same cleanup into the two plugin manifests, matching what main
already did for Experimentalist. nemo-eval-author declared >=3.11 while
hard-depending on nemo-experimentalist-plugin, which main raised to >=3.12, so
it claimed support for a Python version where its own dependency cannot be
installed. Raised to >=3.12,<3.14, dropped the now-vacuous harbor and nooa
markers, and dropped the vacuous marker on Experimentalist's dependency back on
nemo-eval-author-plugin.

Reworded the two root test-discovery exclusion reasons, which described
themselves as 3.11 artifacts. The find_spec guard is still correct, since the
exclusion fires whenever the optional experimentalist group is not synced into
the root environment, so only the text changed. It now matches the
version-agnostic phrasing already used for insights.

uv.lock was re-resolved from main's copy rather than hand-merged; the only
delta is the added nemo-eval-author-plugin entry.

Signed-off-by: Alec Khoury <akhoury@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