docs: add Experimentalist PR walkthrough - #1041
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe changes document repository-backed Experimentalist execution, enable candidate publication and archiving, normalize provenance and candidate links, preserve Git-managed candidate files, update storage serialization, and add upload-only Harbor trace processing. ChangesExperimentalist workflows
Sequence Diagram(s)sequenceDiagram
participant User
participant record_tau_airline_traces.py
participant HarborOutput
participant Intake
User->>record_tau_airline_traces.py: Select --upload-dir
record_tau_airline_traces.py->>HarborOutput: Resolve and validate completed trials
record_tau_airline_traces.py->>Intake: Upload trial traces
Intake-->>record_tau_airline_traces.py: Confirm trace visibility
record_tau_airline_traces.py-->>User: Write upload summary
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@docs/get-started/example-agent.mdx`:
- Line 151: Update the section heading to say “create a draft PR or MR” instead
of “create a PR,” preserving the existing reference to Experimentalist.
- Around line 168-169: Update the branch-selection description in the
Experimentalist instructions so it states that the `@main` suffix selects the
source ref to optimize, while pr_base_branch: main selects the target branch for
the draft PR or MR.
- Around line 83-88: Remove the populated .env file from the copied agent
directory before the git -C "$TAU3_AGENT_DIR" add . command, while preserving
the existing initialization and commit flow.
🪄 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: 3d91e9b6-bf4c-4741-a658-868de7b587af
📒 Files selected for processing (1)
docs/get-started/example-agent.mdx
|
599d0ef to
6602778
Compare
Signed-off-by: Gaia Di Lorenzo <gdilorenzo@ethz.ch>
6602778 to
97a885e
Compare
Signed-off-by: Gaia Di Lorenzo <gdilorenzo@ethz.ch>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/experiment_mirror.py (1)
167-169: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep ancestor lookup split-aware.
This loop supports arbitrary reward channels, but
_parent_experiment_idalways retrieves(candidate.ancestor, "train"). If a candidate uses onlyvalidationor a custom channel, its ancestor link is omitted even when the matching Experiment exists. Passsplitinto_parent_experiment_idand retrieve the ancestor Experiment for that split.Proposed fix
- parent = await self._parent_experiment_id(candidate, gname) + parent = await self._parent_experiment_id(candidate, gname, split) - async def _parent_experiment_id(self, candidate: Candidate, gname: str) -> str | None: + async def _parent_experiment_id(self, candidate: Candidate, gname: str, split: str) -> str | None: if not candidate.ancestor: return None - cached = self._experiment_ids.get((candidate.ancestor, "train")) + cached = self._experiment_ids.get((candidate.ancestor, split)) if cached is not None: return cached try: exp = await self._client.evaluations.retrieve( - experiment_name(gname, candidate.ancestor, "train"), workspace=self._workspace + experiment_name(gname, candidate.ancestor, split), workspace=self._workspace )🤖 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/experiment_mirror.py` around lines 167 - 169, Update the reward-channel loop in the experiment mirroring flow to pass each `split` into `_parent_experiment_id`, and change that method to look up the ancestor Experiment using the supplied split instead of hardcoding `"train"`. Preserve support for validation and custom reward channels while keeping the existing ancestor candidate lookup behavior.plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/config.py (1)
54-54: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAdd
extra='forbid'to configuration models.Pydantic v2
BaseModelsilently ignores unknown keys. A typo likestroage: {publish_winner: false}instead ofstorage: {publish_winner: false}causes the entire key to be discarded, leavingpublish_winner=Trueby default. The run then publishes a winner branch and PR/MR despite the operator's intent to disable publication. AddConfigDict(extra='forbid')toEvolutionaryOptimizerConfig,CandidateStorageConfig, andAgentSourceConfigto reject unknown keys and surface configuration errors.🤖 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/config.py` at line 54, Add ConfigDict(extra='forbid') to the Pydantic model configuration for EvolutionaryOptimizerConfig, CandidateStorageConfig, and AgentSourceConfig, ensuring unknown configuration keys raise validation errors instead of being ignored.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/config.py`:
- Line 54: Add ConfigDict(extra='forbid') to the Pydantic model configuration
for EvolutionaryOptimizerConfig, CandidateStorageConfig, and AgentSourceConfig,
ensuring unknown configuration keys raise validation errors instead of being
ignored.
In
`@plugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/experiment_mirror.py`:
- Around line 167-169: Update the reward-channel loop in the experiment
mirroring flow to pass each `split` into `_parent_experiment_id`, and change
that method to look up the ancestor Experiment using the supplied split instead
of hardcoding `"train"`. Preserve support for validation and custom reward
channels while keeping the existing ancestor candidate lookup behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3db119ba-c939-405b-b25d-eed1178055db
📒 Files selected for processing (9)
docs/get-started/example-agent.mdxplugins/nemo-experimentalist/examples/tau3-nooa-agent/experimentalist-smoke.yamlplugins/nemo-experimentalist/examples/tau3-nooa-agent/record_tau_airline_traces.pyplugins/nemo-experimentalist/src/nemo_experimentalist_plugin/config.pyplugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/repository.pyplugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/experiment_mirror.pyplugins/nemo-experimentalist/tests/experimentalist/test_repository.pyplugins/nemo-experimentalist/tests/test_experiment_mirror_mapping.pyplugins/nemo-experimentalist/tests/test_resolve.py
Signed-off-by: Gaia Di Lorenzo <gdilorenzo@ethz.ch>
There was a problem hiding this comment.
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/examples/tau3-nooa-agent/.gitignore`:
- Around line 1-2: Add the required NVIDIA copyright SPDX header and Apache-2.0
license identifier at the top of the .gitignore file, before the existing
credential-ignore entry.
🪄 Autofix
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: c393858e-e084-4a98-98ee-1670d6a8bced
📒 Files selected for processing (3)
plugins/nemo-experimentalist/examples/tau3-nooa-agent/.gitignoreplugins/nemo-experimentalist/src/nemo_experimentalist_plugin/experimentalist/components/repository.pyplugins/nemo-experimentalist/tests/experimentalist/test_repository.py
| # Local inference credentials | ||
| .env |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the required SPDX header.
Line 1 lacks the NVIDIA copyright SPDX header and the Apache-2.0 identifier.
Proposed fix
+# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+# SPDX-License-Identifier: Apache-2.0
# Local inference credentials
.env📝 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.
| # Local inference credentials | |
| .env | |
| # SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | |
| # SPDX-License-Identifier: Apache-2.0 | |
| # Local inference credentials | |
| .env |
🤖 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/examples/tau3-nooa-agent/.gitignore` around
lines 1 - 2, Add the required NVIDIA copyright SPDX header and Apache-2.0
license identifier at the top of the .gitignore file, before the existing
credential-ignore entry.
Source: Coding guidelines
Signed-off-by: Gaia Di Lorenzo <gdilorenzo@ethz.ch>
Preserve explicitly configured storage flags while keeping implicit remote persistence quiet for local sources, and apply required formatting. Signed-off-by: Alec Khoury <akhoury@nvidia.com>
There was a problem hiding this comment.
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/src/nemo_experimentalist_plugin/cli.py`:
- Line 279: Update the shared looks_like_git() predicate to identify and reject
local filesystem paths before checking whether the normalized source ends with
“.git”. Ensure paths such as /tmp/agent.git are classified as local while
genuine remote Git sources retain the existing suffix-based behavior, including
both call sites using this predicate.
🪄 Autofix
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: d3801c4d-867c-4121-a3d9-4098b374d8c1
📒 Files selected for processing (4)
plugins/nemo-experimentalist/examples/tau3-nooa-agent/record_tau_airline_traces.pyplugins/nemo-experimentalist/src/nemo_experimentalist_plugin/cli.pyplugins/nemo-experimentalist/src/nemo_experimentalist_plugin/resolve.pyplugins/nemo-experimentalist/tests/test_experiment_mirror_mapping.py
🚧 Files skipped from review as they are similar to previous changes (2)
- plugins/nemo-experimentalist/tests/test_experiment_mirror_mapping.py
- plugins/nemo-experimentalist/examples/tau3-nooa-agent/record_tau_airline_traces.py
| task_template=plan.task_template, | ||
| agent_source=plan.agent, | ||
| storage=plan.config.storage.model_dump(), | ||
| storage=plan.config.storage.model_dump(exclude_unset=not looks_like_git(str(plan.agent))), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not classify local .git paths as remote Git sources.
looks_like_git() returns true when the normalized source ends with .git. A local agent directory such as /tmp/agent.git is therefore treated as a Git source. exclude_unset=False then retains implicit persistence defaults and can trigger remote-persistence checks for a local agent.
Update the shared predicate to reject filesystem paths before applying the .git suffix rule.
Also applies to: 422-422
🤖 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/cli.py` at line
279, Update the shared looks_like_git() predicate to identify and reject local
filesystem paths before checking whether the normalized source ends with “.git”.
Ensure paths such as /tmp/agent.git are classified as local while genuine remote
Git sources retain the existing suffix-based behavior, including both call sites
using this predicate.
2 participants
Summary
.env, generatedmetadata.json, and__pycache__/Validation
uv run --frozen pytest plugins/nemo-experimentalist/tests/test_resolve.py -q(65 passed)uv run --frozen pytest plugins/nemo-experimentalist/tests/experimentalist/test_repository.py -q(73 passed)Linear: https://linear.app/nvidia/issue/ASE-772/enable-example-agent-walk-trough-with-pr-creation
Summary by CodeRabbit
New Features
Documentation
Bug Fixes