Skip to content

feat: author and evaluate insight suite metrics - #903

Merged
ryana merged 5 commits into
mainfrom
ase-622-insight-suite-metrics/rangilly
Jul 27, 2026
Merged

feat: author and evaluate insight suite metrics#903
ryana merged 5 commits into
mainfrom
ase-622-insight-suite-metrics/rangilly

Conversation

@ryana

@ryana ryana commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • author and validate root-cause verifier metrics on the experiment-local Eval Author Insight Suite
  • evaluate the baseline and new candidates on that suite, persist aggregate/trial evidence, and mirror the Insight split into Platform experiments
  • keep validation as the ranking/Pareto input and leave the user's train and validation datasets unchanged
  • adapt the original work to the monorepo Experimentalist package, environment names, shell lifecycle, SPDX, typing, and test layout

Stack

Linear

Source

Reissues NVIDIA-dev/NeMo-Optimizer#91 after the Experimentalist plugin moved into this monorepo in #896.

Validation

  • uv run --frozen --group experimentalist pytest -q plugins/nemo-experimentalist/tests/test_eval_author_agent.py plugins/nemo-experimentalist/tests/test_eval_author_materialization.py plugins/nemo-experimentalist/tests/test_experiment_mirror.py plugins/nemo-experimentalist/tests/experimentalist/test_loop_insight_suite.py plugins/nemo-experimentalist/tests/experimentalist/test_eval_author_repair_e2e.py — 37 passed, 2 credential-gated skips
  • uv run --frozen ruff check plugins/nemo-experimentalist
  • uv run --frozen ruff format --check plugins/nemo-experimentalist
  • uv run --frozen ty check on changed production files and new tests
  • SKIP=studio-lint-staged uv run pre-commit run -a — all applicable hooks passed; Studio was skipped because this is a Python-only change and the optional Node 22.18/pnpm environment is not installed

Summary by CodeRabbit

  • New Features
    • Added end-to-end Insight Suite evaluation for baseline and newly generated candidates, persisting per-candidate Insight reward scores, details, suite identity, and metric keys under a dedicated Insight split.
    • Introduced local, deterministic Insight Suite materialization with finalized provenance and added Insight Suite promotion recommendations plus dedicated metrics tables in optimization reports.
  • Bug Fixes
    • Improved verifier validation/repair retries and ensured recovery keeps the last working locally materialized Insight Suite.
    • Tightened reporting/caching logic so Insight metrics are isolated and only reused when suite identity matches.
  • Documentation
    • Updated the Eval Author guide to reflect local staging/validation and finalized persistence.
  • Tests
    • Expanded e2e and loop/reporting tests for Insight evaluation, repair flows, caching, and rendering.

Reissues NVIDIA-dev/NeMo-Optimizer#91 in the nemo-platform monorepo.

Signed-off-by: Ryan Angilly <rangilly@nvidia.com>
@ryana
ryana requested review from a team as code owners July 27, 2026 16:12
@ryana
ryana requested a review from aleckhoury July 27, 2026 16:12
@github-actions github-actions Bot added the feat label Jul 27, 2026
Reissues NVIDIA-dev/NeMo-Optimizer#95 in the nemo-platform monorepo.

Signed-off-by: Ryan Angilly <rangilly@nvidia.com>
@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

Insight suites are staged, validated, finalized as local content-addressed artifacts, evaluated on baseline and generated candidates, persisted as an insight split, and included in optimization reporting and experiment mirroring.

Changes

Insight suite evaluation

Layer / File(s) Summary
Local suite and reward contracts
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/entities.py, plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/*, plugins/nemo-experimentalist/tests/test_eval_author_materialization.py
Candidates store Insight rewards and provenance; Eval Author returns finalized local datasets with deterministic identities and hashes.
Insight metric authoring and validation
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/agent.py, plugins/nemo-experimentalist/tests/test_eval_author_agent.py, plugins/nemo-experimentalist/tests/experimentalist/test_eval_author_repair_e2e.py
Verifier metrics are authored on the materialized suite, validated, and repaired with feedback while train and validation datasets remain unchanged.
Optimizer evaluation and mirror projection
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/loop.py, plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/experiment_mirror.py, plugins/nemo-experimentalist/tests/experimentalist/test_loop_insight_suite.py, plugins/nemo-experimentalist/tests/test_experiment_mirror.py
The optimizer evaluates and persists Insight metrics, reuses compatible cached results, cleans artifacts, and projects the insight split.
Insight validation and reporting
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/insight_promotion.py, plugins/nemo-experimentalist/tests/experimentalist/test_loop_reporting.py
Runtime metrics are validated and provenance-stamped; deterministic comparison and promotion sections are generated from repeated baseline-to-winner evidence.
Persistence and typed backend support
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/experimentalist_backend.py, plugins/nemo-experimentalist/tests/test_experimentalist_backend.py
Entity loading uses typed generics, and generated optimization reports are preserved instead of overwritten by compact summaries.

Sequence Diagram(s)

sequenceDiagram
  participant EvolutionaryOptimizer
  participant EvalAuthor
  participant InsightSuite
  participant Candidate
  participant ExperimentMirror
  EvolutionaryOptimizer->>EvalAuthor: obtain finalized insight dataset
  EvalAuthor->>InsightSuite: author and validate verifier metrics
  EvolutionaryOptimizer->>InsightSuite: evaluate baseline and candidate agents
  InsightSuite-->>EvolutionaryOptimizer: return metrics and trial results
  EvolutionaryOptimizer->>Candidate: persist insight rewards and provenance
  EvolutionaryOptimizer->>ExperimentMirror: project the insight split
Loading

Possibly related PRs

Suggested reviewers: ngoncharenko, svvarom, arpitsardhana

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 19.47% 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 PR’s main change: authoring and evaluating Insight Suite metrics.
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-622-insight-suite-metrics/rangilly

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

🤖 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-experimentalist/tests/test_experiment_mirror.py`:
- Line 11: Remove the leftover “# VERIFY-2” comment from the nemo_platform
import in the test module, leaving the imported symbols unchanged.
🪄 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: ae42fb4d-2a9f-4dd5-9bd5-6531ecf8a033

📥 Commits

Reviewing files that changed from the base of the PR and between dae9035 and 379c3ba.

📒 Files selected for processing (12)
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/entities.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/README.md
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/agent.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/materialization.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/models.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/loop.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/experiment_mirror.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/test_eval_author_agent.py
  • plugins/nemo-experimentalist/tests/test_eval_author_materialization.py
  • plugins/nemo-experimentalist/tests/test_experiment_mirror.py

Comment thread plugins/nemo-experimentalist/tests/test_experiment_mirror.py Outdated
Reissues NVIDIA-dev/NeMo-Optimizer#96 for the Experimentalist plugin.\n\nLinear: ASE-625, ASE-699

Signed-off-by: Ryan Angilly <rangilly@nvidia.com>
Reissues NVIDIA-dev/NeMo-Optimizer#97 for the Experimentalist plugin.\n\nLinear: ASE-684, ASE-699

Signed-off-by: Ryan Angilly <rangilly@nvidia.com>
@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 27236/34958 77.9% 62.2%
Integration Tests 16000/33670 47.5% 19.9%

@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

Caution

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

⚠️ Outside diff range comments (1)
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/loop.py (1)

1032-1032: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use the full insight dataset id here. HarborDataset.from_path() names insight jobs insight-<digest>, so "-insight" leaves agent-* insight result dirs behind on rollback.

🤖 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/src/nemo_experimentalist_plugin/experimentalist/components/loop.py`
at line 1032, Update the suffix iteration in the rollback cleanup logic to use
the full insight dataset identifier generated by HarborDataset.from_path(),
matching the insight-<digest> naming convention. Preserve the existing train and
validation suffix handling while ensuring agent-* insight result directories are
also removed.
🧹 Nitpick comments (4)
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/insight_promotion.py (2)

170-170: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Annotate the accumulatordict[str, list[float]] avoids an inferred partially-unknown type under strict type checking.

-    values = {metric_name: [] for metric_name in required_metrics}
+    values: dict[str, list[float]] = {metric_name: [] for metric_name in required_metrics}
🤖 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/src/nemo_experimentalist_plugin/experimentalist/components/insight_promotion.py`
at line 170, Annotate the values accumulator in the surrounding
insight-promotion logic as dict[str, list[float]], while preserving the existing
required_metrics-based initialization and accumulation behavior.

219-224: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Dead check. _task_metric_values already rejects any non-completed trial, so completed_attempts != total_attempts can never hold here; only the not total_attempts branch is reachable.

🤖 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/src/nemo_experimentalist_plugin/experimentalist/components/insight_promotion.py`
around lines 219 - 224, In the surrounding trial-validation logic, remove the
unreachable completed_attempts calculation and comparison from the
total-attempts guard. Retain only the not total_attempts check that can still
return None, while preserving the existing _task_metric_values validation.
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/loop.py (1)

1426-1428: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Redundant metadata write. Line 1428 is subsumed by the final assignment at Lines 1449-1450; drop it.

Also applies to: 1449-1450

🤖 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/src/nemo_experimentalist_plugin/experimentalist/components/loop.py`
around lines 1426 - 1428, Remove the immediate
dataset.metadata["insight_metric_keys"] assignment from the expected_metric_keys
initialization branch, while retaining the final assignment that already writes
the metric keys after processing.
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/materialization.py (1)

98-119: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Drop persisted per-file hash dicts from manifest.json.

Each task entry stores a full "files" map (and verifier.files) of every relative path → hash, but _content_provenance always recomputes these fresh from disk on resolve/finalize and never reads the stored values back — this is write-only data that inflates manifest.json with the size/count of files in each task (including environment/ build context).

♻️ Keep only the digests, not the full file listing
         tasks.append(
             {
                 **task_entry,
                 "content_hash": content_hash,
                 "verifier": {
                     "path": verifier_path,
                     "content_hash": verifier_hash,
-                    "files": verifier_files,
                 },
-                "files": files,
             }
         )

Also applies to: 404-419

🤖 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/src/nemo_experimentalist_plugin/eval_author/materialization.py`
around lines 98 - 119, Remove the persisted per-file hash dictionaries from task
manifest entries: omit the top-level "files" field and the verifier "files"
field in the task construction around _file_hashes, while retaining the computed
content_hash and verifier.content_hash digests and existing provenance
calculations. Apply the same change to the corresponding manifest construction
around the additional referenced section.
🤖 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-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/loop.py`:
- Around line 1406-1416: Metric-key comparisons currently depend on input
ordering, causing equivalent sets to be treated inconsistently. In loop.py lines
1406-1416, sort both candidate.insight_metric_keys and dataset_metric_keys
before creating cached_metric_key_sets; in insight_promotion.py lines 194-201,
sort candidate.insight_metric_keys before creating metric_key_sets. Preserve the
existing disagreement and promotion logic after normalizing the keys.
- Around line 1849-1866: Guard the insight-report rendering in the finalize flow
so ValueError from write_insight_comparison_section or
write_insight_promotion_section cannot abort a completed run. Add defensive
handling around the insight sections, matching the existing write_final_report
protection, while preserving result persistence and winner handling when insight
metadata is missing or incompatible.

---

Outside diff comments:
In
`@plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/loop.py`:
- Line 1032: Update the suffix iteration in the rollback cleanup logic to use
the full insight dataset identifier generated by HarborDataset.from_path(),
matching the insight-<digest> naming convention. Preserve the existing train and
validation suffix handling while ensuring agent-* insight result directories are
also removed.

---

Nitpick comments:
In
`@plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/materialization.py`:
- Around line 98-119: Remove the persisted per-file hash dictionaries from task
manifest entries: omit the top-level "files" field and the verifier "files"
field in the task construction around _file_hashes, while retaining the computed
content_hash and verifier.content_hash digests and existing provenance
calculations. Apply the same change to the corresponding manifest construction
around the additional referenced section.

In
`@plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/insight_promotion.py`:
- Line 170: Annotate the values accumulator in the surrounding insight-promotion
logic as dict[str, list[float]], while preserving the existing
required_metrics-based initialization and accumulation behavior.
- Around line 219-224: In the surrounding trial-validation logic, remove the
unreachable completed_attempts calculation and comparison from the
total-attempts guard. Retain only the not total_attempts check that can still
return None, while preserving the existing _task_metric_values validation.

In
`@plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/loop.py`:
- Around line 1426-1428: Remove the immediate
dataset.metadata["insight_metric_keys"] assignment from the expected_metric_keys
initialization branch, while retaining the final assignment that already writes
the metric keys after processing.
🪄 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: 2d1dd4f9-4db7-43e0-8a4a-5ed5e59933da

📥 Commits

Reviewing files that changed from the base of the PR and between 03a71af and cbe92c2.

📒 Files selected for processing (13)
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/entities.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/README.md
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/agent.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/materialization.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/models.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/experimentalist_backend.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_eval_author_agent.py
  • plugins/nemo-experimentalist/tests/test_eval_author_materialization.py
  • plugins/nemo-experimentalist/tests/test_experimentalist_backend.py
🚧 Files skipped from review as they are similar to previous changes (3)
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/README.md
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/agent.py
  • plugins/nemo-experimentalist/tests/test_eval_author_agent.py

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

I was going to leave more specific line by line reviews, but it may be better to pass this overview to your agent. Really just some areas we can simplify the hashing and copying, then a couple actual bugs:

Keep
Deterministic suite/scorer identities derived from task and verifier content.
Candidate rewards bound to the suite identity.
Resume invalidation when the authored suite changes.
Runtime metric validation: consistent keys, finite [0,1] values, and complete trials.
Conservative baseline-to-winner promotion qualification.
Deterministic Insight comparison reporting.
Simplify
The nemo-optimizer-insight-suite:// URI, resolver, and second copy under artifacts//insight-suite/ appear unnecessary at this stage.

The URI only resolves relative to the same local experiment directory. The suite is not uploaded to Filesets, included in candidate publication, or otherwise made portable. Therefore, this provides content integrity but not actual durability.

For now, I suggest:

Keep one suite at: eval-and-optimize/eval_author//insight-suite/
Compute and persist its suite/scorer identities in place.
Return the local HarborDataset with those identities in its metadata.
Cache and invalidate candidate results using the suite identity alone.
Remove the custom URI, resolver, duplicate artifact copy, and insight_suite_artifact_ref fields.
Report the local task path plus suite/task/verifier hashes in advisory promotion output.
Durable publication should happen later, only when a user explicitly promotes a task into canonical validation.

Two additional fixes
Harbor defaults to n_attempts=1, but promotion qualification requires at least two attempts. As written, suggestions cannot qualify under the default configuration. Insight-suite evaluation should use max(configured_n_attempts, 2) while leaving train and validation unchanged.
Deterministic comparison/promotion rendering currently risks failing an otherwise successful run during finalization. Evaluator and invalid-metric failures should remain fatal, but report enrichment should warn and omit the affected section rather than fail the optimization.
This retains the meaningful ASE-684 correctness guarantees while removing premature durability machinery and making promotion suggestions functional by default.

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

Approving, with plan to address comments and test e2e once merged

@ryana
ryana force-pushed the ase-622-insight-suite-metrics/rangilly branch from cbe92c2 to 9cd6ede Compare July 27, 2026 18:59
@ryana

ryana commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

@aleckhoury Addressed the review feedback in 9cd6ede:

  • kept a single experiment-local Insight suite and removed the custom URI, resolver, duplicate artifact copy, and artifact-ref fields
  • retained deterministic suite/scorer/task/verifier identities and identity-based resume invalidation
  • made Insight evaluation use at least two attempts without changing train/validation attempts
  • normalized metric-key ordering, removed digest-named rollback leftovers, and isolated Insight report enrichment failures from successful finalization
  • removed the remaining valid CodeRabbit redundancies and added focused regressions
  • rebuilt the branch as the four signed feature commits plus one signed review-fix commit; DCO is green

Validation: 567 Experimentalist tests passed (2 opt-in tests skipped), Ruff lint/format passed, and targeted ty passed. All applicable pre-commit hooks passed; the local Studio lint hook could not start because pnpm 10.32.1 is not installed, and no Studio files changed.

@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-experimentalist/src/nemo_experimentalist_plugin/eval_author/README.md (1)

42-75: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Split lifecycle explanation from return-contract reference.

Move the EvalAuthorResult field contract to a reference page and cross-link it; keep this section focused on the materialization lifecycle.

As per coding guidelines, “Each documentation page should fit ONE Diataxis quadrant; do not mix tutorials with reference tables or how-tos with architecture explanations; use cross-links instead.”

🤖 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/src/nemo_experimentalist_plugin/eval_author/README.md`
around lines 42 - 75, Move the return-contract details for
EvalAuthorResult.insight_suite and EvalAuthorResult.insight_suite_identity out
of the Materialized Insight Suite lifecycle section into an appropriate
reference page, then add a cross-link here. Keep this section focused solely on
suite materialization, validation, identity persistence, and resume lifecycle
behavior.

Source: Coding guidelines

🤖 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-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/insight_promotion.py`:
- Around line 196-199: Update the candidate processing in the insight promotion
flow to build a single suite_candidates collection filtered by
provenance.identity. Reuse it for metric-key extraction, trial evidence, and
candidate_count so stale candidates from other suites cannot affect scoring or
promotion suggestions.

---

Outside diff comments:
In
`@plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/README.md`:
- Around line 42-75: Move the return-contract details for
EvalAuthorResult.insight_suite and EvalAuthorResult.insight_suite_identity out
of the Materialized Insight Suite lifecycle section into an appropriate
reference page, then add a cross-link here. Keep this section focused solely on
suite materialization, validation, identity persistence, and resume lifecycle
behavior.
🪄 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: 9a99ee50-b414-4596-9844-96b0b8ad11fa

📥 Commits

Reviewing files that changed from the base of the PR and between cbe92c2 and 9cd6ede.

📒 Files selected for processing (12)
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/entities.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/README.md
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/agent.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/materialization.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/models.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/tests/experimentalist/test_loop_insight_suite.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_loop_reporting.py
  • plugins/nemo-experimentalist/tests/test_eval_author_agent.py
  • plugins/nemo-experimentalist/tests/test_eval_author_materialization.py
  • plugins/nemo-experimentalist/tests/test_experiment_mirror.py
💤 Files with no reviewable changes (1)
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/entities.py
🚧 Files skipped from review as they are similar to previous changes (6)
  • plugins/nemo-experimentalist/tests/test_experiment_mirror.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_loop_reporting.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/models.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/agent.py
  • plugins/nemo-experimentalist/tests/test_eval_author_agent.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/loop.py

Signed-off-by: Ryan Angilly <rangilly@nvidia.com>
@ryana
ryana force-pushed the ase-622-insight-suite-metrics/rangilly branch from 9cd6ede to 06fbf9e Compare July 27, 2026 19:17
@ryana

ryana commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the second CodeRabbit review round in 06fbf9ead:

  • Promotion evidence now consistently excludes candidates whose suite identity does not match the finalized suite; a direct helper regression verifies candidate counts, attempts, and profiles remain suite-local.
  • Moved the EvalAuthorResult return contract into eval_author/REFERENCE.md and cross-linked it from the materialization lifecycle section.

Validation: 568 passed, 2 opt-in skipped; Ruff lint/format and targeted ty pass. Repository pre-commit passes every hook except the local Studio hook because the configured pnpm 10.32.1 binary is absent (ENOENT); no Studio files changed.

@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

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

⚠️ Outside diff range comments (1)
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/REFERENCE.md (1)

6-24: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add the required reference-page sections and examples.

This page needs a prerequisites section at the top, tabbed Python SDK and CLI examples, and a Next Steps section with cross-links at the end. The current field table is useful, but does not satisfy those required documentation elements.

As per coding guidelines, documentation pages must list prerequisites first, provide Python SDK and CLI examples in tab sets, and end with cross-linked 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-experimentalist/src/nemo_experimentalist_plugin/eval_author/REFERENCE.md`
around lines 6 - 24, Add the required documentation structure to the Eval Author
Python Reference page: place a Prerequisites section first, add tabbed examples
for both the Python SDK and CLI, and finish with a Next Steps section containing
cross-links to relevant documentation. Preserve the existing EvalAuthorResult
field table and explanatory content.

Source: Coding guidelines

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

Outside diff comments:
In
`@plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/REFERENCE.md`:
- Around line 6-24: Add the required documentation structure to the Eval Author
Python Reference page: place a Prerequisites section first, add tabbed examples
for both the Python SDK and CLI, and finish with a Next Steps section containing
cross-links to relevant documentation. Preserve the existing EvalAuthorResult
field table and explanatory content.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 70858717-06fa-4b33-b8c6-2353bc4fecb5

📥 Commits

Reviewing files that changed from the base of the PR and between 9cd6ede and 06fbf9e.

📒 Files selected for processing (13)
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/entities.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/README.md
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/REFERENCE.md
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/agent.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/materialization.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/models.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/tests/experimentalist/test_loop_insight_suite.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_loop_reporting.py
  • plugins/nemo-experimentalist/tests/test_eval_author_agent.py
  • plugins/nemo-experimentalist/tests/test_eval_author_materialization.py
  • plugins/nemo-experimentalist/tests/test_experiment_mirror.py
💤 Files with no reviewable changes (1)
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/entities.py
🚧 Files skipped from review as they are similar to previous changes (11)
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/README.md
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/models.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_loop_reporting.py
  • plugins/nemo-experimentalist/tests/test_experiment_mirror.py
  • plugins/nemo-experimentalist/tests/test_eval_author_materialization.py
  • plugins/nemo-experimentalist/tests/experimentalist/test_loop_insight_suite.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/materialization.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/eval_author/agent.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/insight_promotion.py
  • plugins/nemo-experimentalist/tests/test_eval_author_agent.py
  • plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/loop.py

@ryana
ryana added this pull request to the merge queue Jul 27, 2026
Merged via the queue into main with commit 1b3782c Jul 27, 2026
108 of 110 checks passed
@ryana
ryana deleted the ase-622-insight-suite-metrics/rangilly branch July 27, 2026 21:37
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