docs: harden examples for end-to-end use - #113
Conversation
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
|
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:
WalkthroughThe changes update code-review agent configuration and documentation, strengthen notebook variation failure and Relay-trace validation, and align Harbor examples with revised harness settings, authentication guidance, generated-file handling, and Relay CLI version 0.6.0. ChangesCode-review agent examples
Notebook variations
Harbor examples
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Notebook
participant HarnessAdapter
participant RelayTelemetry
participant ATOFTrace
Notebook->>HarnessAdapter: run configured harness variants
HarnessAdapter-->>Notebook: return statuses and failures
Notebook->>RelayTelemetry: run traced configuration
RelayTelemetry-->>Notebook: return Relay telemetry
Notebook->>ATOFTrace: read and parse events.atof.jsonl
ATOFTrace-->>Notebook: return validated trace events
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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: 4
🤖 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 `@examples/code_review_agent/README.md`:
- Line 25: Update the Python version range in the README installation
instructions from a hyphenated range to an en dash, changing “3.11-3.13” to
“3.11–3.13” while leaving the surrounding text unchanged.
In `@tests/integrations/test_harbor_runner.py`:
- Around line 343-352: Add an assertion in
test_swebench_bootstrap_pins_a_supported_relay_cli verifying that the
SWEBENCH_PREPARE script enforces the x86_64 architecture requirement, alongside
the existing Linux guard assertion. Match the script’s actual
architecture-condition text so the test detects removal of this contract.
In `@tests/python/test_code_review_example.py`:
- Around line 35-37: Extend the test around deepagents_config() and the loop
over base and deepagents to assert that the Deep Agents configuration and
harness are not identical objects to base’s corresponding values. Use identity
assertions that specifically validate the deep-copy independence contract while
preserving the existing completeness checks.
In `@tests/python/test_notebook_examples.py`:
- Around line 76-81: Extend the assertions in the notebook example source test
to verify the required ATOF trace existence and JSON-validity checks, not only
the existing empty-trace message. Anchor the additions to the current assertions
in test_notebook_examples.py and assert the source contains the relevant
missing-trace and invalid-JSON failure handling so removing either regression
check causes the test to fail.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: b910fb9c-1856-4b66-91c9-1f7e6e11ec1f
📒 Files selected for processing (16)
examples/README.mdexamples/code_review_agent/README.mdexamples/code_review_agent/config.pyexamples/code_review_agent/skills/code-review/README.mdexamples/code_review_agent/skills/code-review/SKILL.mdexamples/harbor/calculator/.gitignoreexamples/harbor/calculator/README.mdexamples/harbor/prepare_swebench.shexamples/harbor/swebench/README.mdexamples/harbor/swebench/sample-artifacts/README.mdexamples/notebooks/02_variations.ipynbexamples/notebooks/README.mdtests/integrations/test_harbor_runner.pytests/python/test_code_review_example.pytests/python/test_native_sdk.pytests/python/test_notebook_examples.py
💤 Files with no reviewable changes (2)
- examples/code_review_agent/skills/code-review/README.md
- examples/code_review_agent/config.py
📜 Review details
⏰ Context from checks skipped due to timeout. (7)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Pre-commit
🧰 Additional context used
📓 Path-based instructions (25)
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Update documentation and examples in the same branch as the public API change.
Files:
examples/harbor/swebench/sample-artifacts/README.mdexamples/code_review_agent/skills/code-review/SKILL.mdexamples/harbor/swebench/README.mdexamples/README.mdexamples/harbor/calculator/README.mdexamples/notebooks/README.mdexamples/code_review_agent/README.md
{README.md,docs/**/*.{md,mdx,yml},examples/**/*.{md,mdx,yml}}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep package names, repository references, and build commands current in documentation and examples.
Files:
examples/harbor/swebench/sample-artifacts/README.mdexamples/code_review_agent/skills/code-review/SKILL.mdexamples/harbor/swebench/README.mdexamples/README.mdexamples/harbor/calculator/README.mdexamples/notebooks/README.mdexamples/code_review_agent/README.md
{docs/**/*.{md,mdx,yml},examples/**/*.{md,mdx,yml}}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Update relevant getting-started, reference, adapter, and example documentation when the corresponding examples or adapters change.
Files:
examples/harbor/swebench/sample-artifacts/README.mdexamples/code_review_agent/skills/code-review/SKILL.mdexamples/harbor/swebench/README.mdexamples/README.mdexamples/harbor/calculator/README.mdexamples/notebooks/README.mdexamples/code_review_agent/README.md
**/*
📄 CodeRabbit inference engine (.agents/skills/karpathy-guidelines/SKILL.md)
**/*: Before implementing, explicitly state assumptions, surface ambiguity and tradeoffs, present multiple interpretations when relevant, and ask for clarification rather than silently deciding or proceeding when requirements are unclear.
Prefer the minimum code needed to solve the requested problem: avoid speculative features, unnecessary abstractions, unrequested flexibility, and handling of impossible scenarios; simplify overcomplicated solutions.
When editing existing code, make surgical changes only: do not modify unrelated code, comments, formatting, or pre-existing dead code; match the existing style, and remove only unused imports, variables, or functions introduced by your changes.
Define verifiable success criteria for each task, such as writing regression tests for bugs and invalid-input tests for validation, then verify the implementation against those criteria. For multi-step work, state a brief plan with a verification check for each step.
**/*: Always spellNVIDIAin all caps; do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun, because the name begins with an “en” sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company; use trademark symbols with product names only when required by the document type or legal guidance.
Verify official capitalization, spacing, hyphenation, and spelling for NVIDIA and third-party product names; do not rewrite official product names for grammar or title-case rules.
Precede NVIDIA product names withNVIDIAon first mention when natural and accurate, and link the first mention when the destination helps the reader.
On first use, include the company name and full model qualifier when it helps identify the model; preserve official capitalization and punctuation, and use shorter family names only after establishing the full name.
For learning-oriented and developer content, do not force trademark symbols unless explicitly required; for press, ...
Files:
examples/harbor/swebench/sample-artifacts/README.mdexamples/code_review_agent/skills/code-review/SKILL.mdtests/python/test_native_sdk.pyexamples/harbor/swebench/README.mdexamples/harbor/prepare_swebench.shexamples/README.mdtests/python/test_notebook_examples.pyexamples/harbor/calculator/README.mdtests/integrations/test_harbor_runner.pyexamples/notebooks/README.mdexamples/code_review_agent/README.mdtests/python/test_code_review_example.pyexamples/notebooks/02_variations.ipynb
**/*.{md,mdx,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,mdx,rst}: For NeMo Fabric documentation, verify technical claims against the current repository, public API, or documented command before reviewing style.
Always spellNVIDIAin all caps; do not useNvidia,nvidia, orNV.
Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text; avoid raw URLs and weak anchors such ashereorread more.
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative, parallel steps; split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English while preserving necessary technical precision.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce, and preferrefer tooverseewhen directing readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical documentation.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values; use numerals for 10 or greater and commas in thousands.
Do not add trademark symbols to learning-oriented documentation unless the source, platform, or legal guidance explicitly requires them.
Do not replace precise technical terms with simpler words when doing so would lose precision.
Do not flag passive voice when the actor is unknown or the action is the important part.
Do not rewrite API names, package names, command flags, or code literals for style.
**/*.{md,mdx,rst}: Use consistent title case for technical-document headings and table headers; avoid quotation marks, ampersands, and exclamation marks in headings, while preserving official product, event, research, and whitepaper title ...
Files:
examples/harbor/swebench/sample-artifacts/README.mdexamples/code_review_agent/skills/code-review/SKILL.mdexamples/harbor/swebench/README.mdexamples/README.mdexamples/harbor/calculator/README.mdexamples/notebooks/README.mdexamples/code_review_agent/README.md
**/*.{md,rst,txt,adoc}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-language-mechanics.md)
**/*.{md,rst,txt,adoc}: For technical documentation, use professional, active, conversational, engaging, precise, and plain-English prose. Prefer active voice, present tense, short sentences, and scannable paragraphs. Avoid casual or imprecise language, swearing, threats, insults, jokes, puns, culture-specific idioms, marketing exaggeration, and unsupported third-party comparisons.
Usecanfor possibility and reservemayfor permission; useafterfor temporal order; userefer tofor cross-references; prefer short direct sentences and specific verbs; avoid unnecessarypleasein technical documentation.
Prefer active voice when the actor matters. Passive voice is acceptable when the actor is unknown or irrelevant, when the action or result is the focus, or in programmer documentation.
Use natural contractions in conversational technical prose, but do not force them in formal legal copy, API references, or generated text.
Prefer simpler English over Latinisms: usefor exampleorsuch asinstead ofe.g.,and so oninstead ofetc.,that isinstead ofi.e.,compared toinstead ofvs., andby,through, orusinginstead ofvia. Use industry-standard terms such as in silico, in vitro, and in vivo when appropriate, and italicize them in running text.
Usethatwithout commas for essential clauses, andwhichwith commas for nonessential clauses.
Format dates and times clearly: spell out months in body text; use forms such asJune 12, 2025; avoid numeric or ordinal dates; capitalize days; use 12-hour time when appropriate; include a space beforea.m.orp.m.; useETandPTfor needed time zones; avoid24/7; and preferfrom 12:30 to 1:00 p.m.for prose ranges.
Format numbers consistently: spell out zero through nine in body text, use numerals for 10 or greater and for technical values, use commas in thousands, do not begin a sentence with a numeral, spell out ordinals, and use numerals consistently within a category wh...
Files:
examples/harbor/swebench/sample-artifacts/README.mdexamples/code_review_agent/skills/code-review/SKILL.mdexamples/harbor/swebench/README.mdexamples/README.mdexamples/harbor/calculator/README.mdexamples/notebooks/README.mdexamples/code_review_agent/README.md
**/README.md
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
Update relevant package, crate, adapter, and integration README files when public behavior or entry-point documentation changes.
Files:
examples/harbor/swebench/sample-artifacts/README.mdexamples/harbor/swebench/README.mdexamples/README.mdexamples/harbor/calculator/README.mdexamples/notebooks/README.mdexamples/code_review_agent/README.md
**/*.{md,mdx}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
**/*.{md,mdx}: Use the full product nameNVIDIA NeMo Fabricon its first usage, typically in the title or H1; useNeMo Fabricthereafter.
Usefabricby itself only when referring to the CLI tool, and surround those references with backticks.
CapitalizeNVIDIAcorrectly in public documentation.
Format commands, code elements, expressions, file names, paths, and filenames as inline code where needed.
Use title case consistently for headings in technical documentation.
Introduce code blocks, tables, and lists with complete lead-in sentences.
Use descriptive anchor text instead of raw URLs or generic link text such ashere.
Prefer active voice, present tense, short sentences, and plain English.
Use consistent terminology for the same concept throughout a document.
Write procedures as imperative, parallel, easy-to-scan steps, and split long sequences into smaller tasks.
Useafterinstead ofoncewhen expressing temporal sequence.
Usecaninstead ofmaywhen the intended meaning is possibility rather than permission.
Avoid ambiguous numeric dates and ordinal dates in body text.
For learning-oriented documentation, do not force trademark symbols unless the source document explicitly requires them.
Introduce examples' code blocks with full sentences and ensure examples match current APIs and build commands.For documentation-site changes, run
just docsto regenerate Python and Rust API references and validate Fern configuration.
Files:
examples/harbor/swebench/sample-artifacts/README.mdexamples/code_review_agent/skills/code-review/SKILL.mdexamples/harbor/swebench/README.mdexamples/README.mdexamples/harbor/calculator/README.mdexamples/notebooks/README.mdexamples/code_review_agent/README.md
**/{README.md,*.md,*.mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant SDK, API, adapter, example, integration, and embedded documentation when public behavior or the corresponding surface changes.
Files:
examples/harbor/swebench/sample-artifacts/README.mdexamples/code_review_agent/skills/code-review/SKILL.mdexamples/harbor/swebench/README.mdexamples/README.mdexamples/harbor/calculator/README.mdexamples/notebooks/README.mdexamples/code_review_agent/README.md
**/*.{html,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
HTML and Markdown files must use the specified HTML comment form for SPDX license headers.
Files:
examples/harbor/swebench/sample-artifacts/README.mdexamples/code_review_agent/skills/code-review/SKILL.mdexamples/harbor/swebench/README.mdexamples/README.mdexamples/harbor/calculator/README.mdexamples/notebooks/README.mdexamples/code_review_agent/README.md
{adapters/**,examples/**}
⚙️ CodeRabbit configuration file
{adapters/**,examples/**}: Review adapter and example changes for command correctness, config/schema consistency, artifact handling, and compatibility with the public NeMo Fabric contracts.
Files:
examples/harbor/swebench/sample-artifacts/README.mdexamples/code_review_agent/skills/code-review/SKILL.mdexamples/harbor/swebench/README.mdexamples/harbor/prepare_swebench.shexamples/README.mdexamples/harbor/calculator/README.mdexamples/notebooks/README.mdexamples/code_review_agent/README.mdexamples/notebooks/02_variations.ipynb
{*.md,**/*.md,**/*.mdx,**/*.ipynb}
⚙️ CodeRabbit configuration file
{*.md,**/*.md,**/*.mdx,**/*.ipynb}: Enforce the product name in user-facing prose: use "NVIDIA NeMo Fabric" on first use and "NeMo Fabric" thereafter. Flag standalone capitalized "Fabric" when it refers to the product. Do not flag the lowercasefabricCLI command, package/import/crate names, code identifiers, API symbols, configuration keys, file paths, or unrelated generic uses of the word.
Files:
examples/harbor/swebench/sample-artifacts/README.mdexamples/code_review_agent/skills/code-review/SKILL.mdexamples/harbor/swebench/README.mdexamples/README.mdexamples/harbor/calculator/README.mdexamples/notebooks/README.mdexamples/code_review_agent/README.mdexamples/notebooks/02_variations.ipynb
**/SKILL.md
⚙️ CodeRabbit configuration file
**/SKILL.md: Do not flag SKILL.md files for missing SPDX headers. Skill entrypoints intentionally start with YAML frontmatter instead.
Verify that every SKILL.md keeps valid YAML frontmatter with at least name and description fields before the Markdown body.
Files:
examples/code_review_agent/skills/code-review/SKILL.md
**/*.{rs,py,pyi,json,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Determine and update every affected public surface, including the CLI, PyO3 bindings, Python SDK, type stubs, schemas, and adapter contract, so they remain in parity.
Files:
tests/python/test_native_sdk.pytests/python/test_notebook_examples.pytests/integrations/test_harbor_runner.pytests/python/test_code_review_example.py
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.
Files:
tests/python/test_native_sdk.pytests/python/test_notebook_examples.pytests/integrations/test_harbor_runner.pytests/python/test_code_review_example.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Python code or a Python-facing adapter changes, run
just test-python.Use type annotations for public Python APIs and keep native Python binding declarations synchronized with their Rust implementations.
Files:
tests/python/test_native_sdk.pytests/python/test_notebook_examples.pytests/integrations/test_harbor_runner.pytests/python/test_code_review_example.py
**/*.{rs,py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*.{rs,py,pyi}: If public configuration types change, confirm schema snapshot tests injust test-rustpass and review generated schema diffs.
For schema or public contract changes, run both language suites and review changes underschemas/and generated API references.
**/*.{rs,py,pyi}: Usesnake_casefor Rust and Python functions and variables; usePascalCasefor Rust types and Python classes.
Keep native Python binding declarations synchronized with their Rust implementations when public contracts change.
Files:
tests/python/test_native_sdk.pytests/python/test_notebook_examples.pytests/integrations/test_harbor_runner.pytests/python/test_code_review_example.py
**/*.{py,pyi,rs}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
For Python SDK or PyO3 binding changes, use
python-tests, run focused pytest tests first, thenjust test-python; rebuild withjust build-pythonwhen native code or packaging changes.
Files:
tests/python/test_native_sdk.pytests/python/test_notebook_examples.pytests/integrations/test_harbor_runner.pytests/python/test_code_review_example.py
tests/**/*.{rs,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the corresponding Rust crate or the relevant area under
tests/.
Files:
tests/python/test_native_sdk.pytests/python/test_notebook_examples.pytests/integrations/test_harbor_runner.pytests/python/test_code_review_example.py
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Use Pytest to run Python tests.
Do not add@pytest.mark.asyncioto tests; async tests are automatically detected and run by the async runner.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, useunittest.mock.MagicMockorunittest.mock.AsyncMock, using thespecargument when necessary, rather than defining a new class.
Prefix mocked class names withmock, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; place fixtures needed by multiple test files inconftest.py.
Define fixtures using@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and a function named<fixture_name>_fixture; specifyscopeonly when it is notfunction.
Preferpytest.mark.parametrizeover separate tests for different input types.
Use@pytest.mark.usefixtureswhen a fixture is needed but its return value is unused or it does not return a value.
Use the autouserestore_environ_fixturefromtests/conftest.pyto restore environment variables; modify variables withos.environand do not usemonkeypatch.setenv.
Avoid defensive programming in tests; access expected data directly so missing data raises a clear failure, such as usingresults["data"]instead ofresults.get("data").
Files:
tests/python/test_native_sdk.pytests/python/test_notebook_examples.pytests/integrations/test_harbor_runner.pytests/python/test_code_review_example.py
**/*.{rs,py,toml}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
When editing version helpers, verify every
nemo-fabric-*workspace package through Cargo metadata and reject a static version inpython/pyproject.toml.
Files:
tests/python/test_native_sdk.pytests/python/test_notebook_examples.pytests/integrations/test_harbor_runner.pytests/python/test_code_review_example.py
**/*.{toml,rs,py}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
Avoid blind repository-wide replacement of version-like strings; distinguish package-version references from examples and unrelated dependency versions.
Files:
tests/python/test_native_sdk.pytests/python/test_notebook_examples.pytests/integrations/test_harbor_runner.pytests/python/test_code_review_example.py
{tests/**,python/tests/**}
⚙️ CodeRabbit configuration file
{tests/**,python/tests/**}: Tests should cover the behavior promised by the changed API surface, including error paths, lifecycle cleanup, and SDK/native parity where relevant.
Files:
tests/python/test_native_sdk.pytests/python/test_notebook_examples.pytests/integrations/test_harbor_runner.pytests/python/test_code_review_example.py
**/*.{toml,yml,yaml,sh,bash}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
TOML, YAML, and shell files must use the specified
#-comment SPDX license header.
Files:
examples/harbor/prepare_swebench.sh
examples/README.md
📄 CodeRabbit inference engine (AGENTS.md)
Update
examples/README.mdwhen public behavior, examples, or supported bindings change.
Files:
examples/README.md
🪛 LanguageTool
examples/code_review_agent/README.md
[uncategorized] ~115-~115: The official name of this software platform is spelled with a capital “H”.
Context: ...R. Set GITHUB_MCP_URLbefore runninggithub_config`; the default smoke does not con...
(GITHUB)
🪛 Ruff (0.15.21)
examples/notebooks/02_variations.ipynb
[error] 157-157: await statement outside of a function
(F704)
[error] 157-157: await should be used within an async function
(PLE1142)
🔇 Additional comments (11)
examples/harbor/calculator/.gitignore (1)
1-6: LGTM!examples/harbor/calculator/README.md (1)
88-88: LGTM!Also applies to: 113-113, 138-140, 177-179
examples/harbor/prepare_swebench.sh (1)
22-25: LGTM!Also applies to: 50-57
examples/harbor/swebench/README.md (1)
45-45: LGTM!examples/harbor/swebench/sample-artifacts/README.md (1)
23-28: LGTM!examples/notebooks/02_variations.ipynb (1)
377-377: 📐 Maintainability & Code QualityNo change needed for top-level
await. The notebook passes Ruff as configured, so there’s no linting issue to address.> Likely an incorrect or invalid review comment.tests/python/test_code_review_example.py (1)
17-17: LGTM!Also applies to: 56-62, 80-80, 95-121
tests/python/test_native_sdk.py (1)
81-81: LGTM!examples/code_review_agent/skills/code-review/SKILL.md (1)
1-15: LGTM!examples/README.md (1)
32-33: LGTM!examples/code_review_agent/README.md (1)
24-24: LGTM!Also applies to: 26-36, 77-81, 103-116
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
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 (1)
examples/notebooks/02_variations.ipynb (1)
273-279: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not show prerequisite instructions after an attempted run fails.
blocker()already returnedNone, so this branch has passed the visible prerequisite checks. Printing “To run it, provide…” is misleading for failures caused by authentication, adapter startup, or runtime errors.Proposed fix
except Exception as error: print(f" run failed ({type(error).__name__}: {oneline(error, 120)}).") - print(f" To run it, provide: {harness['needs']}") + print(" Attempted run failed; continuing with remaining variants.") run_failures.append(f"{harness['name']}: {type(error).__name__}: {error}")🤖 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 `@examples/notebooks/02_variations.ipynb` around lines 273 - 279, Remove the prerequisite guidance print from the exception handler around the harness run, leaving the failure summary and run_failures append in place. Since blocker() has already passed, the except branch should report only the actual runtime error and not print harness['needs'] instructions.
🤖 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 `@examples/notebooks/02_variations.ipynb`:
- Around line 273-279: Remove the prerequisite guidance print from the exception
handler around the harness run, leaving the failure summary and run_failures
append in place. Since blocker() has already passed, the except branch should
report only the actual runtime error and not print harness['needs']
instructions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: cd126e81-eec9-4ba8-b5d6-08aaf005d6a2
📒 Files selected for processing (2)
examples/notebooks/02_variations.ipynbtests/python/test_notebook_examples.py
📜 Review details
⏰ Context from checks skipped due to timeout. (17)
- GitHub Check: Test (Python 3.11, linux-amd64)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.11, linux-arm64)
- GitHub Check: Test (Python 3.13, linux-arm64)
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.13, linux-amd64)
- GitHub Check: Test (Python 3.12, linux-amd64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Test (Python 3.14, linux-arm64)
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Test (Python 3.14, macos-arm64)
- GitHub Check: Test (Python 3.12, linux-arm64)
- GitHub Check: Test (Python 3.14, linux-amd64)
- GitHub Check: Pre-commit
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{rs,py,pyi,json,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Determine and update every affected public surface, including the CLI, PyO3 bindings, Python SDK, type stubs, schemas, and adapter contract, so they remain in parity.
Files:
tests/python/test_notebook_examples.py
**/*
📄 CodeRabbit inference engine (.agents/skills/karpathy-guidelines/SKILL.md)
**/*: Before implementing, explicitly state assumptions, surface ambiguity and tradeoffs, present multiple interpretations when relevant, and ask for clarification rather than silently deciding or proceeding when requirements are unclear.
Prefer the minimum code needed to solve the requested problem: avoid speculative features, unnecessary abstractions, unrequested flexibility, and handling of impossible scenarios; simplify overcomplicated solutions.
When editing existing code, make surgical changes only: do not modify unrelated code, comments, formatting, or pre-existing dead code; match the existing style, and remove only unused imports, variables, or functions introduced by your changes.
Define verifiable success criteria for each task, such as writing regression tests for bugs and invalid-input tests for validation, then verify the implementation against those criteria. For multi-step work, state a brief plan with a verification check for each step.
**/*: Always spellNVIDIAin all caps; do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun, because the name begins with an “en” sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company; use trademark symbols with product names only when required by the document type or legal guidance.
Verify official capitalization, spacing, hyphenation, and spelling for NVIDIA and third-party product names; do not rewrite official product names for grammar or title-case rules.
Precede NVIDIA product names withNVIDIAon first mention when natural and accurate, and link the first mention when the destination helps the reader.
On first use, include the company name and full model qualifier when it helps identify the model; preserve official capitalization and punctuation, and use shorter family names only after establishing the full name.
For learning-oriented and developer content, do not force trademark symbols unless explicitly required; for press, ...
Files:
tests/python/test_notebook_examples.pyexamples/notebooks/02_variations.ipynb
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.
Files:
tests/python/test_notebook_examples.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Python code or a Python-facing adapter changes, run
just test-python.Use type annotations for public Python APIs and keep native Python binding declarations synchronized with their Rust implementations.
Files:
tests/python/test_notebook_examples.py
**/*.{rs,py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*.{rs,py,pyi}: If public configuration types change, confirm schema snapshot tests injust test-rustpass and review generated schema diffs.
For schema or public contract changes, run both language suites and review changes underschemas/and generated API references.
**/*.{rs,py,pyi}: Usesnake_casefor Rust and Python functions and variables; usePascalCasefor Rust types and Python classes.
Keep native Python binding declarations synchronized with their Rust implementations when public contracts change.
Files:
tests/python/test_notebook_examples.py
**/*.{py,pyi,rs}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
For Python SDK or PyO3 binding changes, use
python-tests, run focused pytest tests first, thenjust test-python; rebuild withjust build-pythonwhen native code or packaging changes.
Files:
tests/python/test_notebook_examples.py
tests/**/*.{rs,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the corresponding Rust crate or the relevant area under
tests/.
Files:
tests/python/test_notebook_examples.py
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Use Pytest to run Python tests.
Do not add@pytest.mark.asyncioto tests; async tests are automatically detected and run by the async runner.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, useunittest.mock.MagicMockorunittest.mock.AsyncMock, using thespecargument when necessary, rather than defining a new class.
Prefix mocked class names withmock, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; place fixtures needed by multiple test files inconftest.py.
Define fixtures using@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and a function named<fixture_name>_fixture; specifyscopeonly when it is notfunction.
Preferpytest.mark.parametrizeover separate tests for different input types.
Use@pytest.mark.usefixtureswhen a fixture is needed but its return value is unused or it does not return a value.
Use the autouserestore_environ_fixturefromtests/conftest.pyto restore environment variables; modify variables withos.environand do not usemonkeypatch.setenv.
Avoid defensive programming in tests; access expected data directly so missing data raises a clear failure, such as usingresults["data"]instead ofresults.get("data").
Files:
tests/python/test_notebook_examples.py
**/*.{rs,py,toml}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
When editing version helpers, verify every
nemo-fabric-*workspace package through Cargo metadata and reject a static version inpython/pyproject.toml.
Files:
tests/python/test_notebook_examples.py
**/*.{toml,rs,py}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
Avoid blind repository-wide replacement of version-like strings; distinguish package-version references from examples and unrelated dependency versions.
Files:
tests/python/test_notebook_examples.py
{tests/**,python/tests/**}
⚙️ CodeRabbit configuration file
{tests/**,python/tests/**}: Tests should cover the behavior promised by the changed API surface, including error paths, lifecycle cleanup, and SDK/native parity where relevant.
Files:
tests/python/test_notebook_examples.py
{adapters/**,examples/**}
⚙️ CodeRabbit configuration file
{adapters/**,examples/**}: Review adapter and example changes for command correctness, config/schema consistency, artifact handling, and compatibility with the public NeMo Fabric contracts.
Files:
examples/notebooks/02_variations.ipynb
{*.md,**/*.md,**/*.mdx,**/*.ipynb}
⚙️ CodeRabbit configuration file
{*.md,**/*.md,**/*.mdx,**/*.ipynb}: Enforce the product name in user-facing prose: use "NVIDIA NeMo Fabric" on first use and "NeMo Fabric" thereafter. Flag standalone capitalized "Fabric" when it refers to the product. Do not flag the lowercasefabricCLI command, package/import/crate names, code identifiers, API symbols, configuration keys, file paths, or unrelated generic uses of the word.
Files:
examples/notebooks/02_variations.ipynb
🪛 Ruff (0.15.21)
examples/notebooks/02_variations.ipynb
[warning] 241-241: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (4)
tests/python/test_notebook_examples.py (2)
76-83: Still missing regression assertions for missing and invalid ATOF traces.The test covers empty traces but would still pass if the notebook removed the “no ATOF trace” check or stopped validating JSON lines.
Proposed additions
assert "returned no Relay telemetry reference" in source + assert "produced no ATOF trace" in source assert "produced an empty ATOF trace" in source + assert "json.loads(line)" in sourceAs per path instructions, “Tests should cover the behavior promised by the changed API surface, including error paths, lifecycle cleanup, and SDK/native parity where relevant.”
Source: Path instructions
66-81: LGTM!examples/notebooks/02_variations.ipynb (2)
34-37: LGTM!Also applies to: 72-72, 117-123, 156-158, 177-181, 238-242, 254-254, 291-293, 316-326
391-411: 🩺 Stability & AvailabilityNo change needed.
> Likely an incorrect or invalid review comment.
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
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 (1)
tests/python/test_notebook_examples.py (1)
66-87: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCover the opt-in GitHub MCP contract.
This regression test checks the code-review skill and Relay behavior, but it never verifies that
GITHUB_MCP_URLconditionally adds the GitHub MCP server with the intended transport and exposure. Removing that branch would still leave the test green despite the documented capability contract. Add focused source assertions for the environment guard and explicit MCP configuration.As per path instructions, tests should cover behavior promised by the changed API surface.
🤖 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 `@tests/python/test_notebook_examples.py` around lines 66 - 87, Extend test_variations_notebook_uses_runnable_capabilities_and_checks_relay_status with source assertions covering the GITHUB_MCP_URL opt-in branch: verify the environment guard, GitHub MCP server configuration, intended transport, and exposure settings. Keep the assertions focused on the documented capability contract alongside the existing code-review and Relay checks.Source: Path instructions
🤖 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 `@tests/python/test_notebook_examples.py`:
- Around line 66-87: Extend
test_variations_notebook_uses_runnable_capabilities_and_checks_relay_status with
source assertions covering the GITHUB_MCP_URL opt-in branch: verify the
environment guard, GitHub MCP server configuration, intended transport, and
exposure settings. Keep the assertions focused on the documented capability
contract alongside the existing code-review and Relay checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: ecd26dbe-3a91-4a82-9473-d32893f04fa5
📒 Files selected for processing (6)
examples/code_review_agent/README.mdexamples/notebooks/02_variations.ipynbexamples/notebooks/README.mdtests/integrations/test_harbor_runner.pytests/python/test_code_review_example.pytests/python/test_notebook_examples.py
📜 Review details
⏰ Context from checks skipped due to timeout. (9)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.14, macos-arm64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Pre-commit
🧰 Additional context used
📓 Path-based instructions (23)
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Update documentation and examples in the same branch as the public API change.
Files:
examples/notebooks/README.mdexamples/code_review_agent/README.md
{README.md,docs/**/*.{md,mdx,yml},examples/**/*.{md,mdx,yml}}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep package names, repository references, and build commands current in documentation and examples.
Files:
examples/notebooks/README.mdexamples/code_review_agent/README.md
{docs/**/*.{md,mdx,yml},examples/**/*.{md,mdx,yml}}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Update relevant getting-started, reference, adapter, and example documentation when the corresponding examples or adapters change.
Files:
examples/notebooks/README.mdexamples/code_review_agent/README.md
**/*
📄 CodeRabbit inference engine (.agents/skills/karpathy-guidelines/SKILL.md)
**/*: Before implementing, explicitly state assumptions, surface ambiguity and tradeoffs, present multiple interpretations when relevant, and ask for clarification rather than silently deciding or proceeding when requirements are unclear.
Prefer the minimum code needed to solve the requested problem: avoid speculative features, unnecessary abstractions, unrequested flexibility, and handling of impossible scenarios; simplify overcomplicated solutions.
When editing existing code, make surgical changes only: do not modify unrelated code, comments, formatting, or pre-existing dead code; match the existing style, and remove only unused imports, variables, or functions introduced by your changes.
Define verifiable success criteria for each task, such as writing regression tests for bugs and invalid-input tests for validation, then verify the implementation against those criteria. For multi-step work, state a brief plan with a verification check for each step.
**/*: Always spellNVIDIAin all caps; do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun, because the name begins with an “en” sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company; use trademark symbols with product names only when required by the document type or legal guidance.
Verify official capitalization, spacing, hyphenation, and spelling for NVIDIA and third-party product names; do not rewrite official product names for grammar or title-case rules.
Precede NVIDIA product names withNVIDIAon first mention when natural and accurate, and link the first mention when the destination helps the reader.
On first use, include the company name and full model qualifier when it helps identify the model; preserve official capitalization and punctuation, and use shorter family names only after establishing the full name.
For learning-oriented and developer content, do not force trademark symbols unless explicitly required; for press, ...
Files:
examples/notebooks/README.mdexamples/code_review_agent/README.mdtests/python/test_notebook_examples.pytests/integrations/test_harbor_runner.pytests/python/test_code_review_example.pyexamples/notebooks/02_variations.ipynb
**/*.{md,mdx,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,mdx,rst}: For NeMo Fabric documentation, verify technical claims against the current repository, public API, or documented command before reviewing style.
Always spellNVIDIAin all caps; do not useNvidia,nvidia, orNV.
Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text; avoid raw URLs and weak anchors such ashereorread more.
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative, parallel steps; split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English while preserving necessary technical precision.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce, and preferrefer tooverseewhen directing readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical documentation.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values; use numerals for 10 or greater and commas in thousands.
Do not add trademark symbols to learning-oriented documentation unless the source, platform, or legal guidance explicitly requires them.
Do not replace precise technical terms with simpler words when doing so would lose precision.
Do not flag passive voice when the actor is unknown or the action is the important part.
Do not rewrite API names, package names, command flags, or code literals for style.
**/*.{md,mdx,rst}: Use consistent title case for technical-document headings and table headers; avoid quotation marks, ampersands, and exclamation marks in headings, while preserving official product, event, research, and whitepaper title ...
Files:
examples/notebooks/README.mdexamples/code_review_agent/README.md
**/*.{md,rst,txt,adoc}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-language-mechanics.md)
**/*.{md,rst,txt,adoc}: For technical documentation, use professional, active, conversational, engaging, precise, and plain-English prose. Prefer active voice, present tense, short sentences, and scannable paragraphs. Avoid casual or imprecise language, swearing, threats, insults, jokes, puns, culture-specific idioms, marketing exaggeration, and unsupported third-party comparisons.
Usecanfor possibility and reservemayfor permission; useafterfor temporal order; userefer tofor cross-references; prefer short direct sentences and specific verbs; avoid unnecessarypleasein technical documentation.
Prefer active voice when the actor matters. Passive voice is acceptable when the actor is unknown or irrelevant, when the action or result is the focus, or in programmer documentation.
Use natural contractions in conversational technical prose, but do not force them in formal legal copy, API references, or generated text.
Prefer simpler English over Latinisms: usefor exampleorsuch asinstead ofe.g.,and so oninstead ofetc.,that isinstead ofi.e.,compared toinstead ofvs., andby,through, orusinginstead ofvia. Use industry-standard terms such as in silico, in vitro, and in vivo when appropriate, and italicize them in running text.
Usethatwithout commas for essential clauses, andwhichwith commas for nonessential clauses.
Format dates and times clearly: spell out months in body text; use forms such asJune 12, 2025; avoid numeric or ordinal dates; capitalize days; use 12-hour time when appropriate; include a space beforea.m.orp.m.; useETandPTfor needed time zones; avoid24/7; and preferfrom 12:30 to 1:00 p.m.for prose ranges.
Format numbers consistently: spell out zero through nine in body text, use numerals for 10 or greater and for technical values, use commas in thousands, do not begin a sentence with a numeral, spell out ordinals, and use numerals consistently within a category wh...
Files:
examples/notebooks/README.mdexamples/code_review_agent/README.md
**/README.md
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
Update relevant package, crate, adapter, and integration README files when public behavior or entry-point documentation changes.
Files:
examples/notebooks/README.mdexamples/code_review_agent/README.md
**/*.{md,mdx}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
**/*.{md,mdx}: Use the full product nameNVIDIA NeMo Fabricon its first usage, typically in the title or H1; useNeMo Fabricthereafter.
Usefabricby itself only when referring to the CLI tool, and surround those references with backticks.
CapitalizeNVIDIAcorrectly in public documentation.
Format commands, code elements, expressions, file names, paths, and filenames as inline code where needed.
Use title case consistently for headings in technical documentation.
Introduce code blocks, tables, and lists with complete lead-in sentences.
Use descriptive anchor text instead of raw URLs or generic link text such ashere.
Prefer active voice, present tense, short sentences, and plain English.
Use consistent terminology for the same concept throughout a document.
Write procedures as imperative, parallel, easy-to-scan steps, and split long sequences into smaller tasks.
Useafterinstead ofoncewhen expressing temporal sequence.
Usecaninstead ofmaywhen the intended meaning is possibility rather than permission.
Avoid ambiguous numeric dates and ordinal dates in body text.
For learning-oriented documentation, do not force trademark symbols unless the source document explicitly requires them.
Introduce examples' code blocks with full sentences and ensure examples match current APIs and build commands.Update corresponding documentation when public behavior, adapters, examples, or workspace structure changes.
Files:
examples/notebooks/README.mdexamples/code_review_agent/README.md
**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
HTML/Markdown source files must use the specified HTML-comment SPDX header format.
Files:
examples/notebooks/README.mdexamples/code_review_agent/README.md
**/*.{py,toml,lock,json,md,yml,yaml}
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
Keep package wiring, descriptors, dependencies, installation, catalogs, CI enumerations, documentation, examples, fixtures, and generated artifacts consistent with the adapter implementation.
Files:
examples/notebooks/README.mdexamples/code_review_agent/README.mdtests/python/test_notebook_examples.pytests/integrations/test_harbor_runner.pytests/python/test_code_review_example.py
{adapters/**,examples/**}
⚙️ CodeRabbit configuration file
{adapters/**,examples/**}: Review adapter and example changes for command correctness, config/schema consistency, artifact handling, and compatibility with the public NeMo Fabric contracts.
Files:
examples/notebooks/README.mdexamples/code_review_agent/README.mdexamples/notebooks/02_variations.ipynb
{*.md,**/*.md,**/*.mdx,**/*.ipynb}
⚙️ CodeRabbit configuration file
{*.md,**/*.md,**/*.mdx,**/*.ipynb}: Enforce the product name in user-facing prose: use "NVIDIA NeMo Fabric" on first use and "NeMo Fabric" thereafter. Flag standalone capitalized "Fabric" when it refers to the product. Do not flag the lowercasefabricCLI command, package/import/crate names, code identifiers, API symbols, configuration keys, file paths, or unrelated generic uses of the word.
Files:
examples/notebooks/README.mdexamples/code_review_agent/README.mdexamples/notebooks/02_variations.ipynb
**/*.{rs,py,pyi,json,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Determine and update every affected public surface, including the CLI, PyO3 bindings, Python SDK, type stubs, schemas, and adapter contract, so they remain in parity.
Files:
tests/python/test_notebook_examples.pytests/integrations/test_harbor_runner.pytests/python/test_code_review_example.py
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.
**/*.{rs,py}: Usesnake_casefor functions and variables; usePascalCasefor Rust types and Python classes.
Run tests for every affected language surface; changes to the Rust core or public schemas require both Rust and Python test suites.
Keep native Python binding declarations synchronized with their Rust implementations when public contracts change.
Files:
tests/python/test_notebook_examples.pytests/integrations/test_harbor_runner.pytests/python/test_code_review_example.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Python code or a Python-facing adapter changes, run
just test-python.
Files:
tests/python/test_notebook_examples.pytests/integrations/test_harbor_runner.pytests/python/test_code_review_example.py
**/*.{rs,py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*.{rs,py,pyi}: If public configuration types change, confirm schema snapshot tests injust test-rustpass and review generated schema diffs.
For schema or public contract changes, run both language suites and review changes underschemas/and generated API references.
Files:
tests/python/test_notebook_examples.pytests/integrations/test_harbor_runner.pytests/python/test_code_review_example.py
**/*.{py,pyi,rs}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
For Python SDK or PyO3 binding changes, use
python-tests, run focused pytest tests first, thenjust test-python; rebuild withjust build-pythonwhen native code or packaging changes.
Files:
tests/python/test_notebook_examples.pytests/integrations/test_harbor_runner.pytests/python/test_code_review_example.py
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Use Pytest to run Python tests.
Do not add@pytest.mark.asyncioto tests; async tests are automatically detected and run by the async runner.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, useunittest.mock.MagicMockorunittest.mock.AsyncMock, using thespecargument when necessary, rather than defining a new class.
Prefix mocked class names withmock, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; place fixtures needed by multiple test files inconftest.py.
Define fixtures using@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and a function named<fixture_name>_fixture; specifyscopeonly when it is notfunction.
Preferpytest.mark.parametrizeover separate tests for different input types.
Use@pytest.mark.usefixtureswhen a fixture is needed but its return value is unused or it does not return a value.
Use the autouserestore_environ_fixturefromtests/conftest.pyto restore environment variables; modify variables withos.environand do not usemonkeypatch.setenv.
Avoid defensive programming in tests; access expected data directly so missing data raises a clear failure, such as usingresults["data"]instead ofresults.get("data").
Files:
tests/python/test_notebook_examples.pytests/integrations/test_harbor_runner.pytests/python/test_code_review_example.py
**/*.{rs,py,toml}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
When editing version helpers, verify every
nemo-fabric-*workspace package through Cargo metadata and reject a static version inpython/pyproject.toml.
Files:
tests/python/test_notebook_examples.pytests/integrations/test_harbor_runner.pytests/python/test_code_review_example.py
**/*.{toml,rs,py}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
Avoid blind repository-wide replacement of version-like strings; distinguish package-version references from examples and unrelated dependency versions.
Files:
tests/python/test_notebook_examples.pytests/integrations/test_harbor_runner.pytests/python/test_code_review_example.py
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: Use type annotations for public Python APIs and keep native binding declarations synchronized with their Rust implementations.
Python files must use the specified Python#SPDX header format.
Files:
tests/python/test_notebook_examples.pytests/integrations/test_harbor_runner.pytests/python/test_code_review_example.py
**/tests/**/*.{rs,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the corresponding Rust crate or relevant
tests/area.
Files:
tests/python/test_notebook_examples.pytests/integrations/test_harbor_runner.pytests/python/test_code_review_example.py
{tests/**,python/tests/**}
⚙️ CodeRabbit configuration file
{tests/**,python/tests/**}: Tests should cover the behavior promised by the changed API surface, including error paths, lifecycle cleanup, and SDK/native parity where relevant.
Files:
tests/python/test_notebook_examples.pytests/integrations/test_harbor_runner.pytests/python/test_code_review_example.py
🔇 Additional comments (6)
tests/python/test_code_review_example.py (1)
17-17: LGTM!Also applies to: 35-37, 56-64, 82-82
examples/code_review_agent/README.md (1)
24-36: LGTM!Also applies to: 77-81, 103-116
tests/integrations/test_harbor_runner.py (1)
353-353: LGTM!examples/notebooks/02_variations.ipynb (2)
34-37: LGTM!Also applies to: 72-72, 117-123, 156-158, 177-181, 238-242, 254-254, 272-279, 291-293, 316-326, 359-390, 413-414
391-411: 🎯 Functional CorrectnessDo not assume every Relay artifact is JSONL.
The code validates JSON only for
events.atof.jsonl, but the final dump callsjson.loadson every file underrelay_dir. If Relay emits metadata or another non-JSON artifact, a valid ATOF trace will still fail. Restrict parsing to known JSONL files, or verify that the pinned Relay version guarantees JSONL for every emitted file.examples/notebooks/README.md (1)
22-36: LGTM!Also applies to: 38-44
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
|
Fern docs preview: https://nvidia-preview-pull-request-113.docs.buildwithfern.com/nemo/fabric |
There was a problem hiding this comment.
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)
tests/python/test_notebook_examples.py (1)
33-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
adapter_python_available's BASE_DIR-relative and not-found branches are untested.
_variation_harness_definitions()'s exec namespace omitsBASE_DIR, so the relative-path branch ofadapter_python_available(which anchors toBASE_DIR) can't even be exercised without aNameError. Only the single-partshutil.whichbranch is covered bytest_variations_notebook_accepts_adapter_command_on_path; the "not found" negative case is untested entirely.♻️ Suggested additions
namespace = { "base_config": base_config, "HarnessConfig": HarnessConfig, "ModelConfig": ModelConfig, "os": os, "Path": Path, "shutil": shutil, + "BASE_DIR": BASE_DIR, "HERMES_PY": sys.executable, "FABRIC_PY": sys.executable, "INSTRUCTION": "Test instruction.", "WORKSPACE": "./repos/my-service", }Add a case such as
blocker({"python": "/does/not/exist"}) == "adapter interpreter not found".As per path instructions, "Tests should cover the behavior promised by the changed API surface, including error paths, lifecycle cleanup, and SDK/native parity where relevant."
🤖 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 `@tests/python/test_notebook_examples.py` around lines 33 - 48, Update _variation_harness_definitions() to include BASE_DIR in the notebook exec namespace so adapter_python_available can exercise its BASE_DIR-relative path branch, then extend the variation tests with a missing-interpreter case such as /does/not/exist and assert the blocker returns "adapter interpreter not found".Source: Path instructions
🤖 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 `@examples/notebooks/02_variations.ipynb`:
- Around line 402-422: Update the full Relay trace dump loop to iterate over the
already validated atof_paths collection instead of traces, while preserving the
existing event parsing and printing behavior. Keep traces for the file-count
listing, but ensure only ATOF trace files are passed to json.loads in the dump
section.
---
Outside diff comments:
In `@tests/python/test_notebook_examples.py`:
- Around line 33-48: Update _variation_harness_definitions() to include BASE_DIR
in the notebook exec namespace so adapter_python_available can exercise its
BASE_DIR-relative path branch, then extend the variation tests with a
missing-interpreter case such as /does/not/exist and assert the blocker returns
"adapter interpreter not found".
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: d7d23e09-889d-4d66-99a7-02cb04b3d513
📒 Files selected for processing (8)
README.mdexamples/code_review_agent/README.mdexamples/code_review_agent/__init__.pyexamples/code_review_agent/config.pyexamples/notebooks/02_variations.ipynbexamples/notebooks/README.mdtests/python/test_code_review_example.pytests/python/test_notebook_examples.py
📜 Review details
⏰ Context from checks skipped due to timeout. (10)
- GitHub Check: Preview docs
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.13, linux-arm64)
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Pre-commit
🧰 Additional context used
📓 Path-based instructions (26)
**/*.{rs,py,pyi,json,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Determine and update every affected public surface, including the CLI, PyO3 bindings, Python SDK, type stubs, schemas, and adapter contract, so they remain in parity.
Files:
examples/code_review_agent/__init__.pytests/python/test_code_review_example.pytests/python/test_notebook_examples.pyexamples/code_review_agent/config.py
**/*
📄 CodeRabbit inference engine (.agents/skills/karpathy-guidelines/SKILL.md)
**/*: Before implementing, explicitly state assumptions, surface ambiguity and tradeoffs, present multiple interpretations when relevant, and ask for clarification rather than silently deciding or proceeding when requirements are unclear.
Prefer the minimum code needed to solve the requested problem: avoid speculative features, unnecessary abstractions, unrequested flexibility, and handling of impossible scenarios; simplify overcomplicated solutions.
When editing existing code, make surgical changes only: do not modify unrelated code, comments, formatting, or pre-existing dead code; match the existing style, and remove only unused imports, variables, or functions introduced by your changes.
Define verifiable success criteria for each task, such as writing regression tests for bugs and invalid-input tests for validation, then verify the implementation against those criteria. For multi-step work, state a brief plan with a verification check for each step.
**/*: Always spellNVIDIAin all caps; do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun, because the name begins with an “en” sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company; use trademark symbols with product names only when required by the document type or legal guidance.
Verify official capitalization, spacing, hyphenation, and spelling for NVIDIA and third-party product names; do not rewrite official product names for grammar or title-case rules.
Precede NVIDIA product names withNVIDIAon first mention when natural and accurate, and link the first mention when the destination helps the reader.
On first use, include the company name and full model qualifier when it helps identify the model; preserve official capitalization and punctuation, and use shorter family names only after establishing the full name.
For learning-oriented and developer content, do not force trademark symbols unless explicitly required; for press, ...
Files:
examples/code_review_agent/__init__.pyREADME.mdexamples/code_review_agent/README.mdtests/python/test_code_review_example.pytests/python/test_notebook_examples.pyexamples/notebooks/README.mdexamples/code_review_agent/config.pyexamples/notebooks/02_variations.ipynb
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.
**/*.{rs,py}: Usesnake_casefor functions and variables; usePascalCasefor Rust types and Python classes.
Run tests for every affected language surface; changes to the Rust core or public schemas require both Rust and Python test suites.
Keep native Python binding declarations synchronized with their Rust implementations when public contracts change.
Files:
examples/code_review_agent/__init__.pytests/python/test_code_review_example.pytests/python/test_notebook_examples.pyexamples/code_review_agent/config.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Python code or a Python-facing adapter changes, run
just test-python.
Files:
examples/code_review_agent/__init__.pytests/python/test_code_review_example.pytests/python/test_notebook_examples.pyexamples/code_review_agent/config.py
**/*.{rs,py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*.{rs,py,pyi}: If public configuration types change, confirm schema snapshot tests injust test-rustpass and review generated schema diffs.
For schema or public contract changes, run both language suites and review changes underschemas/and generated API references.
Files:
examples/code_review_agent/__init__.pytests/python/test_code_review_example.pytests/python/test_notebook_examples.pyexamples/code_review_agent/config.py
**/*.{py,pyi,rs}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
For Python SDK or PyO3 binding changes, use
python-tests, run focused pytest tests first, thenjust test-python; rebuild withjust build-pythonwhen native code or packaging changes.
Files:
examples/code_review_agent/__init__.pytests/python/test_code_review_example.pytests/python/test_notebook_examples.pyexamples/code_review_agent/config.py
**/*.{rs,py,toml}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
When editing version helpers, verify every
nemo-fabric-*workspace package through Cargo metadata and reject a static version inpython/pyproject.toml.
Files:
examples/code_review_agent/__init__.pytests/python/test_code_review_example.pytests/python/test_notebook_examples.pyexamples/code_review_agent/config.py
**/*.{toml,rs,py}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
Avoid blind repository-wide replacement of version-like strings; distinguish package-version references from examples and unrelated dependency versions.
Files:
examples/code_review_agent/__init__.pytests/python/test_code_review_example.pytests/python/test_notebook_examples.pyexamples/code_review_agent/config.py
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: Use type annotations for public Python APIs and keep native binding declarations synchronized with their Rust implementations.
Python files must use the specified Python#SPDX header format.
Files:
examples/code_review_agent/__init__.pytests/python/test_code_review_example.pytests/python/test_notebook_examples.pyexamples/code_review_agent/config.py
**/*.{py,toml,lock,json,md,yml,yaml}
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
Keep package wiring, descriptors, dependencies, installation, catalogs, CI enumerations, documentation, examples, fixtures, and generated artifacts consistent with the adapter implementation.
Files:
examples/code_review_agent/__init__.pyREADME.mdexamples/code_review_agent/README.mdtests/python/test_code_review_example.pytests/python/test_notebook_examples.pyexamples/notebooks/README.mdexamples/code_review_agent/config.py
{adapters/**,examples/**}
⚙️ CodeRabbit configuration file
{adapters/**,examples/**}: Review adapter and example changes for command correctness, config/schema consistency, artifact handling, and compatibility with the public NeMo Fabric contracts.
Files:
examples/code_review_agent/__init__.pyexamples/code_review_agent/README.mdexamples/notebooks/README.mdexamples/code_review_agent/config.pyexamples/notebooks/02_variations.ipynb
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Update documentation and examples in the same branch as the public API change.
Files:
README.mdexamples/code_review_agent/README.mdexamples/notebooks/README.md
{README.md,docs/**/*.{md,mdx,yml},examples/**/*.{md,mdx,yml}}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep package names, repository references, and build commands current in documentation and examples.
Files:
README.mdexamples/code_review_agent/README.mdexamples/notebooks/README.md
{README.md,docs/index.yml}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Update
README.mdordocs/index.ymlwhen documentation entry points or example reading paths change.
Files:
README.md
**/*.{md,mdx,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,mdx,rst}: For NeMo Fabric documentation, verify technical claims against the current repository, public API, or documented command before reviewing style.
Always spellNVIDIAin all caps; do not useNvidia,nvidia, orNV.
Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text; avoid raw URLs and weak anchors such ashereorread more.
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative, parallel steps; split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English while preserving necessary technical precision.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce, and preferrefer tooverseewhen directing readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical documentation.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values; use numerals for 10 or greater and commas in thousands.
Do not add trademark symbols to learning-oriented documentation unless the source, platform, or legal guidance explicitly requires them.
Do not replace precise technical terms with simpler words when doing so would lose precision.
Do not flag passive voice when the actor is unknown or the action is the important part.
Do not rewrite API names, package names, command flags, or code literals for style.
**/*.{md,mdx,rst}: Use consistent title case for technical-document headings and table headers; avoid quotation marks, ampersands, and exclamation marks in headings, while preserving official product, event, research, and whitepaper title ...
Files:
README.mdexamples/code_review_agent/README.mdexamples/notebooks/README.md
**/*.{md,rst,txt,adoc}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-language-mechanics.md)
**/*.{md,rst,txt,adoc}: For technical documentation, use professional, active, conversational, engaging, precise, and plain-English prose. Prefer active voice, present tense, short sentences, and scannable paragraphs. Avoid casual or imprecise language, swearing, threats, insults, jokes, puns, culture-specific idioms, marketing exaggeration, and unsupported third-party comparisons.
Usecanfor possibility and reservemayfor permission; useafterfor temporal order; userefer tofor cross-references; prefer short direct sentences and specific verbs; avoid unnecessarypleasein technical documentation.
Prefer active voice when the actor matters. Passive voice is acceptable when the actor is unknown or irrelevant, when the action or result is the focus, or in programmer documentation.
Use natural contractions in conversational technical prose, but do not force them in formal legal copy, API references, or generated text.
Prefer simpler English over Latinisms: usefor exampleorsuch asinstead ofe.g.,and so oninstead ofetc.,that isinstead ofi.e.,compared toinstead ofvs., andby,through, orusinginstead ofvia. Use industry-standard terms such as in silico, in vitro, and in vivo when appropriate, and italicize them in running text.
Usethatwithout commas for essential clauses, andwhichwith commas for nonessential clauses.
Format dates and times clearly: spell out months in body text; use forms such asJune 12, 2025; avoid numeric or ordinal dates; capitalize days; use 12-hour time when appropriate; include a space beforea.m.orp.m.; useETandPTfor needed time zones; avoid24/7; and preferfrom 12:30 to 1:00 p.m.for prose ranges.
Format numbers consistently: spell out zero through nine in body text, use numerals for 10 or greater and for technical values, use commas in thousands, do not begin a sentence with a numeral, spell out ordinals, and use numerals consistently within a category wh...
Files:
README.mdexamples/code_review_agent/README.mdexamples/notebooks/README.md
README.md
📄 CodeRabbit inference engine (AGENTS.md)
Update the root
README.mdwhen public behavior, thenemo-fabricpackage, examples, or supported bindings change.Update the top-level
README.mdwhen changed behavior affects an entry point users would naturally consult.
Files:
README.md
**/README.md
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
Update relevant package, crate, adapter, and integration README files when public behavior or entry-point documentation changes.
Files:
README.mdexamples/code_review_agent/README.mdexamples/notebooks/README.md
**/*.{md,mdx}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
**/*.{md,mdx}: Use the full product nameNVIDIA NeMo Fabricon its first usage, typically in the title or H1; useNeMo Fabricthereafter.
Usefabricby itself only when referring to the CLI tool, and surround those references with backticks.
CapitalizeNVIDIAcorrectly in public documentation.
Format commands, code elements, expressions, file names, paths, and filenames as inline code where needed.
Use title case consistently for headings in technical documentation.
Introduce code blocks, tables, and lists with complete lead-in sentences.
Use descriptive anchor text instead of raw URLs or generic link text such ashere.
Prefer active voice, present tense, short sentences, and plain English.
Use consistent terminology for the same concept throughout a document.
Write procedures as imperative, parallel, easy-to-scan steps, and split long sequences into smaller tasks.
Useafterinstead ofoncewhen expressing temporal sequence.
Usecaninstead ofmaywhen the intended meaning is possibility rather than permission.
Avoid ambiguous numeric dates and ordinal dates in body text.
For learning-oriented documentation, do not force trademark symbols unless the source document explicitly requires them.
Introduce examples' code blocks with full sentences and ensure examples match current APIs and build commands.Update corresponding documentation when public behavior, adapters, examples, or workspace structure changes.
Files:
README.mdexamples/code_review_agent/README.mdexamples/notebooks/README.md
**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
HTML/Markdown source files must use the specified HTML-comment SPDX header format.
Files:
README.mdexamples/code_review_agent/README.mdexamples/notebooks/README.md
{docs/**,README.md,AGENTS.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,AGENTS.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency with generated schemas.
Files:
README.md
{*.md,**/*.md,**/*.mdx,**/*.ipynb}
⚙️ CodeRabbit configuration file
{*.md,**/*.md,**/*.mdx,**/*.ipynb}: Enforce the product name in user-facing prose: use "NVIDIA NeMo Fabric" on first use and "NeMo Fabric" thereafter. Flag standalone capitalized "Fabric" when it refers to the product. Do not flag the lowercasefabricCLI command, package/import/crate names, code identifiers, API symbols, configuration keys, file paths, or unrelated generic uses of the word.
Files:
README.mdexamples/code_review_agent/README.mdexamples/notebooks/README.mdexamples/notebooks/02_variations.ipynb
{docs/**/*.{md,mdx,yml},examples/**/*.{md,mdx,yml}}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Update relevant getting-started, reference, adapter, and example documentation when the corresponding examples or adapters change.
Files:
examples/code_review_agent/README.mdexamples/notebooks/README.md
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Use Pytest to run Python tests.
Do not add@pytest.mark.asyncioto tests; async tests are automatically detected and run by the async runner.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, useunittest.mock.MagicMockorunittest.mock.AsyncMock, using thespecargument when necessary, rather than defining a new class.
Prefix mocked class names withmock, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; place fixtures needed by multiple test files inconftest.py.
Define fixtures using@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and a function named<fixture_name>_fixture; specifyscopeonly when it is notfunction.
Preferpytest.mark.parametrizeover separate tests for different input types.
Use@pytest.mark.usefixtureswhen a fixture is needed but its return value is unused or it does not return a value.
Use the autouserestore_environ_fixturefromtests/conftest.pyto restore environment variables; modify variables withos.environand do not usemonkeypatch.setenv.
Avoid defensive programming in tests; access expected data directly so missing data raises a clear failure, such as usingresults["data"]instead ofresults.get("data").
Files:
tests/python/test_code_review_example.pytests/python/test_notebook_examples.py
**/tests/**/*.{rs,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the corresponding Rust crate or relevant
tests/area.
Files:
tests/python/test_code_review_example.pytests/python/test_notebook_examples.py
{tests/**,python/tests/**}
⚙️ CodeRabbit configuration file
{tests/**,python/tests/**}: Tests should cover the behavior promised by the changed API surface, including error paths, lifecycle cleanup, and SDK/native parity where relevant.
Files:
tests/python/test_code_review_example.pytests/python/test_notebook_examples.py
🪛 ast-grep (0.44.1)
tests/python/test_notebook_examples.py
[error] 45-45: The use of exec can be insecure
Context: exec(compile(source, str(VARIATIONS_NOTEBOOK), "exec"), namespace)
Note: [CWE-94] Improper Control of Generation of Code ('Code Injection').
(no-exec)
[warning] 45-45: The use of compile can be insecure
Context: compile(source, str(VARIATIONS_NOTEBOOK), "exec")
Note: [CWE-94] Improper Control of Generation of Code ('Code Injection').
(no-compile)
[info] 116-116: use of eval can be insecure
Context: eval(code, namespace)
Note: [CWE-94] Improper Control of Generation of Code ('Code Injection').
(no-eval-python)
[warning] 108-113: The use of compile can be insecure
Context: compile(
source,
str(VARIATIONS_NOTEBOOK),
"exec",
flags=ast.PyCF_ALLOW_TOP_LEVEL_AWAIT,
)
Note: [CWE-94] Improper Control of Generation of Code ('Code Injection').
(no-compile)
🪛 LanguageTool
examples/code_review_agent/README.md
[uncategorized] ~117-~117: The official name of this software platform is spelled with a capital “H”.
Context: ...R. Set GITHUB_MCP_URLbefore runninggithub_config`; it maps the server into the se...
(GITHUB)
🔇 Additional comments (10)
examples/code_review_agent/config.py (2)
60-61: LGTM!Also applies to: 118-119, 144-145, 182-183
204-214: 🎯 Functional CorrectnessNo compatibility alias is needed here.
with_github_mcpis the only exported helper in this tree, and the README/tests already import it; there is nowith_fabric_managed_github_mcpsymbol to preserve.> Likely an incorrect or invalid review comment.tests/python/test_code_review_example.py (1)
17-19: LGTM!Also applies to: 35-37, 56-70, 82-85, 97-110, 112-126
examples/code_review_agent/README.md (1)
24-36: LGTM!Also applies to: 54-55, 69-69, 78-86, 105-119
README.md (1)
129-130: LGTM!examples/notebooks/02_variations.ipynb (2)
117-123: LGTM!Also applies to: 169-188, 246-252, 264-264, 301-303, 326-336, 369-369
222-243: LGTM!examples/notebooks/README.md (1)
21-44: LGTM!tests/python/test_notebook_examples.py (2)
75-121: LGTM!
123-147: LGTM!
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
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 (1)
examples/notebooks/02_variations.ipynb (1)
222-229: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject non-executable interpreter paths.
Path.is_file()lets a touched placeholder pass as available, so the notebook can proceed with an interpreter path that cannot run. Require executable access for explicit paths, and make the test fixture executable instead of usingtouch().Proposed fix
- return path.is_file() + return path.is_file() and os.access(path, os.X_OK)🤖 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 `@examples/notebooks/02_variations.ipynb` around lines 222 - 229, The adapter_python_available function must reject explicit interpreter paths that are not executable; replace the regular-file check with an executable-access check while preserving command lookup and relative-path resolution. In tests/python/test_notebook_examples.py lines 79-87, update the fixture setup to create the placeholder interpreter with executable permissions instead of using touch().Source: Path instructions
🤖 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 `@examples/notebooks/02_variations.ipynb`:
- Around line 222-229: The adapter_python_available function must reject
explicit interpreter paths that are not executable; replace the regular-file
check with an executable-access check while preserving command lookup and
relative-path resolution. In tests/python/test_notebook_examples.py lines 79-87,
update the fixture setup to create the placeholder interpreter with executable
permissions instead of using touch().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 2c3d446e-02dd-40d0-8278-8c4695f55c9b
📒 Files selected for processing (2)
examples/notebooks/02_variations.ipynbtests/python/test_notebook_examples.py
📜 Review details
⏰ Context from checks skipped due to timeout. (8)
- GitHub Check: Preview docs
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Pre-commit
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{rs,py,pyi,json,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Determine and update every affected public surface, including the CLI, PyO3 bindings, Python SDK, type stubs, schemas, and adapter contract, so they remain in parity.
Files:
tests/python/test_notebook_examples.py
**/*
📄 CodeRabbit inference engine (.agents/skills/karpathy-guidelines/SKILL.md)
**/*: Before implementing, explicitly state assumptions, surface ambiguity and tradeoffs, present multiple interpretations when relevant, and ask for clarification rather than silently deciding or proceeding when requirements are unclear.
Prefer the minimum code needed to solve the requested problem: avoid speculative features, unnecessary abstractions, unrequested flexibility, and handling of impossible scenarios; simplify overcomplicated solutions.
When editing existing code, make surgical changes only: do not modify unrelated code, comments, formatting, or pre-existing dead code; match the existing style, and remove only unused imports, variables, or functions introduced by your changes.
Define verifiable success criteria for each task, such as writing regression tests for bugs and invalid-input tests for validation, then verify the implementation against those criteria. For multi-step work, state a brief plan with a verification check for each step.
**/*: Always spellNVIDIAin all caps; do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun, because the name begins with an “en” sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company; use trademark symbols with product names only when required by the document type or legal guidance.
Verify official capitalization, spacing, hyphenation, and spelling for NVIDIA and third-party product names; do not rewrite official product names for grammar or title-case rules.
Precede NVIDIA product names withNVIDIAon first mention when natural and accurate, and link the first mention when the destination helps the reader.
On first use, include the company name and full model qualifier when it helps identify the model; preserve official capitalization and punctuation, and use shorter family names only after establishing the full name.
For learning-oriented and developer content, do not force trademark symbols unless explicitly required; for press, ...
Files:
tests/python/test_notebook_examples.pyexamples/notebooks/02_variations.ipynb
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.
**/*.{rs,py}: Usesnake_casefor functions and variables; usePascalCasefor Rust types and Python classes.
Run tests for every affected language surface; changes to the Rust core or public schemas require both Rust and Python test suites.
Keep native Python binding declarations synchronized with their Rust implementations when public contracts change.
Files:
tests/python/test_notebook_examples.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Python code or a Python-facing adapter changes, run
just test-python.
Files:
tests/python/test_notebook_examples.py
**/*.{rs,py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*.{rs,py,pyi}: If public configuration types change, confirm schema snapshot tests injust test-rustpass and review generated schema diffs.
For schema or public contract changes, run both language suites and review changes underschemas/and generated API references.
Files:
tests/python/test_notebook_examples.py
**/*.{py,pyi,rs}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
For Python SDK or PyO3 binding changes, use
python-tests, run focused pytest tests first, thenjust test-python; rebuild withjust build-pythonwhen native code or packaging changes.
Files:
tests/python/test_notebook_examples.py
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Use Pytest to run Python tests.
Do not add@pytest.mark.asyncioto tests; async tests are automatically detected and run by the async runner.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, useunittest.mock.MagicMockorunittest.mock.AsyncMock, using thespecargument when necessary, rather than defining a new class.
Prefix mocked class names withmock, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; place fixtures needed by multiple test files inconftest.py.
Define fixtures using@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and a function named<fixture_name>_fixture; specifyscopeonly when it is notfunction.
Preferpytest.mark.parametrizeover separate tests for different input types.
Use@pytest.mark.usefixtureswhen a fixture is needed but its return value is unused or it does not return a value.
Use the autouserestore_environ_fixturefromtests/conftest.pyto restore environment variables; modify variables withos.environand do not usemonkeypatch.setenv.
Avoid defensive programming in tests; access expected data directly so missing data raises a clear failure, such as usingresults["data"]instead ofresults.get("data").
Files:
tests/python/test_notebook_examples.py
**/*.{rs,py,toml}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
When editing version helpers, verify every
nemo-fabric-*workspace package through Cargo metadata and reject a static version inpython/pyproject.toml.
Files:
tests/python/test_notebook_examples.py
**/*.{toml,rs,py}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
Avoid blind repository-wide replacement of version-like strings; distinguish package-version references from examples and unrelated dependency versions.
Files:
tests/python/test_notebook_examples.py
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: Use type annotations for public Python APIs and keep native binding declarations synchronized with their Rust implementations.
Python files must use the specified Python#SPDX header format.
Files:
tests/python/test_notebook_examples.py
**/tests/**/*.{rs,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the corresponding Rust crate or relevant
tests/area.
Files:
tests/python/test_notebook_examples.py
**/*.{py,toml,lock,json,md,yml,yaml}
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
Keep package wiring, descriptors, dependencies, installation, catalogs, CI enumerations, documentation, examples, fixtures, and generated artifacts consistent with the adapter implementation.
Files:
tests/python/test_notebook_examples.py
{tests/**,python/tests/**}
⚙️ CodeRabbit configuration file
{tests/**,python/tests/**}: Tests should cover the behavior promised by the changed API surface, including error paths, lifecycle cleanup, and SDK/native parity where relevant.
Files:
tests/python/test_notebook_examples.py
{adapters/**,examples/**}
⚙️ CodeRabbit configuration file
{adapters/**,examples/**}: Review adapter and example changes for command correctness, config/schema consistency, artifact handling, and compatibility with the public NeMo Fabric contracts.
Files:
examples/notebooks/02_variations.ipynb
{*.md,**/*.md,**/*.mdx,**/*.ipynb}
⚙️ CodeRabbit configuration file
{*.md,**/*.md,**/*.mdx,**/*.ipynb}: Enforce the product name in user-facing prose: use "NVIDIA NeMo Fabric" on first use and "NeMo Fabric" thereafter. Flag standalone capitalized "Fabric" when it refers to the product. Do not flag the lowercasefabricCLI command, package/import/crate names, code identifiers, API symbols, configuration keys, file paths, or unrelated generic uses of the word.
Files:
examples/notebooks/02_variations.ipynb
🪛 Ruff (0.15.21)
tests/python/test_notebook_examples.py
[warning] 26-26: Missing return type annotation for private function _variation_harness_definitions
(ANN202)
🔇 Additional comments (3)
examples/notebooks/02_variations.ipynb (1)
34-37: LGTM!Also applies to: 72-72, 117-123, 156-158, 177-181, 248-252, 264-264, 282-289, 301-303, 326-336, 368-369, 379-411, 417-428
tests/python/test_notebook_examples.py (2)
26-26: 📐 Maintainability & Code QualityResolve the reported ANN202 diagnostic.
Ruff reports a missing return annotation on this changed helper. Add a precise return type or an intentional local suppression, then validate the Python suite. As per coding guidelines, “If Python code or a Python-facing adapter changes, run
just test-python.”Sources: Coding guidelines, Linters/SAST tools
8-17: LGTM!Also applies to: 35-52, 131-156
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
dagardner-nv
left a comment
dagardner-nv
left a comment
There was a problem hiding this comment.
Approved with comments
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
|
/merge |
2 participants
Overview
Hardens the runnable examples after an end-to-end audit of the documentation refactor in #103. The changes remove implicit or invalid prerequisites, route the optional GitHub MCP server through maintained harnesses, make headless notebook failures visible, align Relay setup with the supported 0.6 CLI, and document concrete success criteria. There are no public API, dependency, lockfile, or breaking changes.
Details
SKILL.md, route the optional GitHub MCP server through each harness's native MCP support, and document adapter-specific Python selection without leaking Hermes settings into other harnesses.PATH, collect both harness and Relay failures, and require successful Relay status plus nonempty, parseable ATOF output before a headless run can pass..tmp/hermes-venv.Validation
Repository validation:
just build-alljust test-python— 405 passed, 44 skippeduv run --no-sync pre-commit run --all-files --show-diff-on-failurejust docs— completed with the expected unauthenticated Fern redirect warning onlybash -n examples/harbor/prepare_swebench.shgit diff --checkLive E2E coverage:
succeeded, and reward1.0.Environment-bound gaps are explicit rather than reported as passes:
ANTHROPIC_API_KEY, which was not available.GitHub validation for the final review-fix commit:
Linked Refactor Follow-Ups
The latest audited #103 head was
003f380. These items are specific to files changed on that branch and are intentionally not folded into this non-overlapping patch:docs/about-nemo-fabric/overview.mdxand notebook 01 still link to the removed#quick-start-hermes-agentanchor. The equivalent links in the shared example READMEs are corrected here..venv-hermes, while notebook 01 auto-detects.tmp/hermes-venv;.venv-hermesis not ignored. The canonical path is unclear and should be made consistent on docs: Reafactor theREADME.md#103.docs/getting-started/install.mdxusesjust build-all no_uv=true, which Just parses as a missing recipe. The valid override form isjust --set no_uv true build-all.adapters/claude/README.mdcontains a stray closing code fence after the Relay CLI paragraph.doctor()proves the machine and harness are ready. The current implementation checks the resolved plan and declared preflight requirements without starting the harness, so the intended guarantee should be clarified or the prose narrowed.Where should the reviewer start?
Start with
examples/code_review_agent/config.pyand its plan assertions intests/python/test_code_review_example.py, then review the prerequisite and failure-aggregation cells inexamples/notebooks/02_variations.ipynb. The Harbor host/version contract starts inexamples/harbor/prepare_swebench.sh.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Relates to docs: Reafactor the
README.md#103I confirm this contribution is my own work, or I have the right to submit it under this project's license.
I searched existing issues and open pull requests, and this does not duplicate existing work.
Summary by CodeRabbit
New Features
Documentation
ADAPTER_PYTHON.Bug Fixes
Tests