chore: Create E2E tests to test skill and model variation - #210
Conversation
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@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 change adds shared ATOF assertions, skill fixtures, CI Relay tooling, and Windows command-quoting updates. Relay end-to-end tests validate skill and model selection for Claude, Codex, DeepAgents, and Hermes. ChangesRelay selection validation
Windows command quoting
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 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 `@tests/e2e/test_claude.py`:
- Around line 270-280: Scenario setup requests currently block async tests
through synchronous requests.post calls. In tests/e2e/test_claude.py:270-280,
tests/e2e/test_codex.py:59-72, tests/e2e/test_deepagents.py:211-221, and
tests/e2e/test_hermes_e2e.py:228-238, move all eight scenario and MCP setup
calls outside the event loop using asyncio.to_thread or an asynchronous HTTP
client, while preserving response validation via raise_for_status.
In `@tests/e2e/test_hermes_e2e.py`:
- Around line 215-225: Update the skill setup in assert_atof_skill_selection so
the no-skill case clears every inherited skill path, not just default_skill.
After creating the config and before the skill-specific branches, remove or
reset all configured skill paths; preserve adding alternate_skill for the
alternate case and default_skill for the default case.
In `@tests/fixtures/alternate/SKILL.md`:
- Line 6: Update the heading in SKILL.md from “Alternate test skill” to title
case: “Alternate Test Skill”.
In `@tests/fixtures/default/SKILL.md`:
- Line 6: Update the heading in the default test skill document from sentence
case to title case, preserving the existing heading text otherwise.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 0cb26eb9-36fd-46df-af47-4dae2f8e77eb
📒 Files selected for processing (8)
tests/_utils/utils.pytests/conftest.pytests/e2e/test_claude.pytests/e2e/test_codex.pytests/e2e/test_deepagents.pytests/e2e/test_hermes_e2e.pytests/fixtures/alternate/SKILL.mdtests/fixtures/default/SKILL.md
📜 Review details
⏰ Context from checks skipped due to timeout. (17)
- GitHub Check: Test (Python 3.12, linux-arm64)
- GitHub Check: Test (Python 3.11, linux-amd64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.11, linux-arm64)
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.13, linux-amd64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Test (Python 3.14, linux-amd64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.12, linux-amd64)
- GitHub Check: Test (Python 3.14, linux-arm64)
- GitHub Check: Test (Python 3.13, linux-arm64)
- GitHub Check: Test (Python 3.14, macos-arm64)
- GitHub Check: Pre-commit
🧰 Additional context used
📓 Path-based instructions (24)
**/*.{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/e2e/test_claude.pytests/e2e/test_deepagents.pytests/e2e/test_hermes_e2e.pytests/e2e/test_codex.pytests/conftest.pytests/_utils/utils.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/e2e/test_claude.pytests/fixtures/default/SKILL.mdtests/fixtures/alternate/SKILL.mdtests/e2e/test_deepagents.pytests/e2e/test_hermes_e2e.pytests/e2e/test_codex.pytests/conftest.pytests/_utils/utils.py
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.Keep package names, import paths, and module names internally consistent, including the editable maturin build producing
nemo_fabric._nativeand native artifacts being placed underpython/src/nemo_fabricas expected by consumers.
Files:
tests/e2e/test_claude.pytests/e2e/test_deepagents.pytests/e2e/test_hermes_e2e.pytests/e2e/test_codex.pytests/conftest.pytests/_utils/utils.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 binding declarations synchronized with their Rust implementations.
Files:
tests/e2e/test_claude.pytests/e2e/test_deepagents.pytests/e2e/test_hermes_e2e.pytests/e2e/test_codex.pytests/conftest.pytests/_utils/utils.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.
Run tests for every affected language surface. Changes touching the Rust core or public schemas require both Rust and Python test suites.
Use the existing style in the Python SDK, adapters, examples, and tests, and maintain synchronization between native Python binding declarations and Rust implementations.
If a change touches the Rust core or public schemas, run bothjust test-rustandjust test-python; otherwise run the test targets for every affected language surface.
Files:
tests/e2e/test_claude.pytests/e2e/test_deepagents.pytests/e2e/test_hermes_e2e.pytests/e2e/test_codex.pytests/conftest.pytests/_utils/utils.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/e2e/test_claude.pytests/e2e/test_deepagents.pytests/e2e/test_hermes_e2e.pytests/e2e/test_codex.pytests/conftest.pytests/_utils/utils.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/e2e/test_claude.pytests/e2e/test_deepagents.pytests/e2e/test_hermes_e2e.pytests/e2e/test_codex.pytests/conftest.pytests/_utils/utils.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/e2e/test_claude.pytests/e2e/test_deepagents.pytests/e2e/test_hermes_e2e.pytests/e2e/test_codex.pytests/conftest.pytests/_utils/utils.py
**/*.{md,mdx,yml,py,rs,sh}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
Keep documentation aligned with current NeMo Fabric behavior, repository layout, entry points, commands, package names, APIs, bindings, and support claims.
Files:
tests/e2e/test_claude.pytests/fixtures/default/SKILL.mdtests/fixtures/alternate/SKILL.mdtests/e2e/test_deepagents.pytests/e2e/test_hermes_e2e.pytests/e2e/test_codex.pytests/conftest.pytests/_utils/utils.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 by the async runner.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, useunittest.mock.MagicMockorAsyncMock, using thespecargument when necessary, rather than defining a new class.
Prefix mocked class names withmock, notfake.
Prefer pytest fixtures over helper methods.
If a fixture is needed in multiple test files, define it once inconftest.pyrather than repeating it.
Define fixtures using@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and a<fixture_name>_fixturefunction; specifyscopeonly when it is notfunction.
Preferpytest.mark.parametrizeover separate tests for different input types.
Use@pytest.mark.usefixtureswhen a fixture is needed but its returned value is unused or it returns no value.
Avoid defensive programming in tests; access expected values directly so missing data raises a clear failure, such as usingresults["data"]instead ofresults.get("data").
When adapter installation metadata changes, packaging metadata tests must directly assert that the root project depends unconditionally on the exact-versionnemo-fabric-runtimedistribution.
Packaging metadata tests must verify that each root harness extra delegates to the matching version of the leaf adapter'sharnessextra.
Packaging metadata tests must verify that bare leaf dependencies remain adapter-owned and that the rootadapter-testsdependency group installs each leaf through itsharnessextra.
Packaging metadata tests must verify that every leaf providesfull; only adapters importing NeMo Relay Python APIs providerelay, while adapters using an external Relay executable havefullequal toharness.
Files:
tests/e2e/test_claude.pytests/e2e/test_deepagents.pytests/e2e/test_hermes_e2e.pytests/e2e/test_codex.pytests/conftest.pytests/_utils/utils.py
tests/**/*.{rs,py,pyi}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the corresponding Rust crate or in the relevant area under
tests/.
Files:
tests/e2e/test_claude.pytests/e2e/test_deepagents.pytests/e2e/test_hermes_e2e.pytests/e2e/test_codex.pytests/conftest.pytests/_utils/utils.py
**/*.{rs,py,pyi,json}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Public contract changes must keep checked-in JSON Schema snapshots and native Python binding declarations synchronized.
Files:
tests/e2e/test_claude.pytests/e2e/test_deepagents.pytests/e2e/test_hermes_e2e.pytests/e2e/test_codex.pytests/conftest.pytests/_utils/utils.py
**/*.{rs,py,html,md,mdx,toml,yaml,yml,sh,bash}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
All source files must include the appropriate SPDX copyright and Apache-2.0 license headers using the comment syntax for their file type; MDX files must use a JSX comment.
Files:
tests/e2e/test_claude.pytests/fixtures/default/SKILL.mdtests/fixtures/alternate/SKILL.mdtests/e2e/test_deepagents.pytests/e2e/test_hermes_e2e.pytests/e2e/test_codex.pytests/conftest.pytests/_utils/utils.py
**/*.{md,mdx,rst,yml,yaml,py,sh}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
**/*.{md,mdx,rst,yml,yaml,py,sh}: Keep package names, repository references, and build commands current.
Ensure example commands match current package names and paths.
Files:
tests/e2e/test_claude.pytests/fixtures/default/SKILL.mdtests/fixtures/alternate/SKILL.mdtests/e2e/test_deepagents.pytests/e2e/test_hermes_e2e.pytests/e2e/test_codex.pytests/conftest.pytests/_utils/utils.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/e2e/test_claude.pytests/fixtures/default/SKILL.mdtests/fixtures/alternate/SKILL.mdtests/e2e/test_deepagents.pytests/e2e/test_hermes_e2e.pytests/e2e/test_codex.pytests/conftest.pytests/_utils/utils.py
**/*.{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:
tests/fixtures/default/SKILL.mdtests/fixtures/alternate/SKILL.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:
tests/fixtures/default/SKILL.mdtests/fixtures/alternate/SKILL.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:
tests/fixtures/default/SKILL.mdtests/fixtures/alternate/SKILL.md
**/*.{md,mdx}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
**/*.{md,mdx}: Use the full product nameNVIDIA NeMo Fabricon first use, typically in the title and H1; useNeMo Fabricthereafter. Usefabricalone only for the CLI tool and surround it with backticks.
Treat incorrect or stale commands, package names, paths, APIs, support claims, procedures, examples, terminology, or public behavior documentation as blocking issues.
CapitalizeNVIDIAcorrectly and format code, commands, paths, and filenames as inline code where needed.
Use title case for technical-documentation headings.
Introduce code blocks, tables, and lists with complete lead-in sentences; ensure examples match current APIs and build commands.
Use descriptive anchor text, avoid raw URLs and generic labels such ashere, and use repository-relative.mdxpaths for links withindocs/.
Prefer active voice, present tense, short sentences, plain English, consistent terminology, and imperative, parallel, scannable procedures.
Useafterinstead ofoncewhen expressing temporal sequence, and usecanrather thanmaywhen describing 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.
When reporting documentation-review findings, lead withMust fix,Should fix, andNice to havecategories; include file path, line reference, current problem, rationale, and a concrete rewrite or direction.
**/*.{md,mdx}: When public behavior, adapters, examples, or workspace structure changes, update the corresponding documentation in the same branch.
For docs site changes, runjust docsto regenerate Python and Rust API references and validate Fern configuration.Keep release-process and release-history policy in
RELEASING.md, not in user-facing documentation or a duplicateCHANGELOG.md.
Files:
tests/fixtures/default/SKILL.mdtests/fixtures/alternate/SKILL.md
**/*.{md,mdx,yml,yaml}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
**/*.{md,mdx,yml,yaml}: Update entry-point documentation, includingREADME.mdordocs/index.yml, when examples or reading paths change.
Update relevant getting-started, reference, entry-point, and example or adapter README documentation when examples or adapters change.
Files:
tests/fixtures/default/SKILL.mdtests/fixtures/alternate/SKILL.md
**/*.{md,mdx,rst,yml,yaml}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Run
just docswhen the documentation site changes.
Files:
tests/fixtures/default/SKILL.mdtests/fixtures/alternate/SKILL.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:
tests/fixtures/default/SKILL.mdtests/fixtures/alternate/SKILL.md
**/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:
tests/fixtures/default/SKILL.mdtests/fixtures/alternate/SKILL.md
tests/conftest.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
Use the existing autouse
restore_environ_fixturefromtests/conftest.py; modify environment variables directly throughos.environinstead of usingmonkeypatch.setenv.
Files:
tests/conftest.py
🪛 ast-grep (0.45.1)
tests/e2e/test_claude.py
[warning] 269-278: Request-controlled URL passed to requests; validate against an allowlist to prevent SSRF.
Context: requests.post(
f"{api_server}/_scenario",
json={
"tool_call": {
"name": "Skill",
"arguments": {"skill": skill},
}
},
timeout=5,
)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(ssrf-requests)
tests/e2e/test_deepagents.py
[warning] 210-219: Request-controlled URL passed to requests; validate against an allowlist to prevent SSRF.
Context: requests.post(
f"{api_server}/_scenario",
json={
"tool_call": {
"name": "read_file",
"arguments": {"file_path": f"/{skill_file}"},
}
},
timeout=5,
)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(ssrf-requests)
tests/e2e/test_hermes_e2e.py
[warning] 227-236: Request-controlled URL passed to requests; validate against an allowlist to prevent SSRF.
Context: requests.post(
f"{api_server}/_scenario",
json={
"tool_call": {
"name": "skill_view",
"arguments": {"name": skill},
}
},
timeout=5,
)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(ssrf-requests)
tests/e2e/test_codex.py
[warning] 58-70: Request-controlled URL passed to requests; validate against an allowlist to prevent SSRF.
Context: requests.post(
f"{api_server}/_scenario",
json={
"tool_call": {
"name": "exec_command",
"arguments": {
"cmd": f"cat {selected_skill / 'SKILL.md'}",
"workdir": str(tmp_path),
},
}
},
timeout=5,
)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(ssrf-requests)
tests/_utils/utils.py
[info] 26-26: use jsonify instead of json.dumps for JSON output
Context: json.dumps(tool_records)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Ruff (0.16.1)
tests/e2e/test_claude.py
[warning] 270-270: Async functions should not call blocking HTTP methods
(ASYNC210)
tests/e2e/test_deepagents.py
[warning] 211-211: Async functions should not call blocking HTTP methods
(ASYNC210)
tests/e2e/test_hermes_e2e.py
[warning] 228-228: Async functions should not call blocking HTTP methods
(ASYNC210)
tests/e2e/test_codex.py
[warning] 27-27: Missing return type annotation for private function _mock_codex_config
(ANN202)
[warning] 59-59: Async functions should not call blocking HTTP methods
(ASYNC210)
🔇 Additional comments (6)
tests/_utils/utils.py (1)
10-18: LGTM!Also applies to: 21-55, 71-80
tests/conftest.py (1)
54-76: LGTM!Also applies to: 88-88, 97-97, 113-122
tests/e2e/test_claude.py (1)
16-20: LGTM!Also applies to: 292-305
tests/e2e/test_codex.py (1)
20-38: LGTM!Also applies to: 84-95
tests/e2e/test_deepagents.py (1)
20-20: LGTM!Also applies to: 233-257
tests/e2e/test_hermes_e2e.py (1)
18-18: LGTM!Also applies to: 250-269
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@adapters/common/src/nemo_fabric_adapters/common/relay_hooks.py`:
- Line 75: Update the parameterized test expectation for render_relay_hooks to
normalize the expected executable path with forward slashes when constructing
the command, matching the win32 conversion in render_relay_hooks. Run the
focused test on Windows, then run just test-python.
In `@tests/adapters/test_adapters_common_relay_hooks.py`:
- Around line 87-95: Update the executable path in the relay hook test around
render_relay_hooks to include a space in at least one Windows path component,
and adjust the expected command to use the corresponding forward-slash path.
Preserve the quoted executable format so the test fails if render_relay_hooks
omits quoting.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: c9dbe77d-d130-4fea-9624-aaebb76eef95
📒 Files selected for processing (2)
adapters/common/src/nemo_fabric_adapters/common/relay_hooks.pytests/adapters/test_adapters_common_relay_hooks.py
📜 Review details
⏰ Context from checks skipped due to timeout. (19)
- GitHub Check: Test (Python 3.11, linux-arm64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.14, linux-arm64)
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.14, linux-amd64)
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.12, linux-arm64)
- GitHub Check: Test (Python 3.12, linux-amd64)
- GitHub Check: Test (Python 3.13, linux-amd64)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.13, linux-arm64)
- GitHub Check: Test (Python 3.14, macos-arm64)
- GitHub Check: Test (Python 3.11, linux-amd64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (x86_64)
- GitHub Check: Test (arm64)
- GitHub Check: Pre-commit
🧰 Additional context used
📓 Path-based instructions (22)
**/*.{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:
adapters/common/src/nemo_fabric_adapters/common/relay_hooks.pytests/adapters/test_adapters_common_relay_hooks.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:
adapters/common/src/nemo_fabric_adapters/common/relay_hooks.pytests/adapters/test_adapters_common_relay_hooks.py
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.Keep package names, import paths, and module names internally consistent, including the editable maturin build producing
nemo_fabric._nativeand native artifacts being placed underpython/src/nemo_fabricas expected by consumers.
Files:
adapters/common/src/nemo_fabric_adapters/common/relay_hooks.pytests/adapters/test_adapters_common_relay_hooks.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 binding declarations synchronized with their Rust implementations.
Files:
adapters/common/src/nemo_fabric_adapters/common/relay_hooks.pytests/adapters/test_adapters_common_relay_hooks.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.
Run tests for every affected language surface. Changes touching the Rust core or public schemas require both Rust and Python test suites.
Use the existing style in the Python SDK, adapters, examples, and tests, and maintain synchronization between native Python binding declarations and Rust implementations.
If a change touches the Rust core or public schemas, run bothjust test-rustandjust test-python; otherwise run the test targets for every affected language surface.
Files:
adapters/common/src/nemo_fabric_adapters/common/relay_hooks.pytests/adapters/test_adapters_common_relay_hooks.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:
adapters/common/src/nemo_fabric_adapters/common/relay_hooks.pytests/adapters/test_adapters_common_relay_hooks.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:
adapters/common/src/nemo_fabric_adapters/common/relay_hooks.pytests/adapters/test_adapters_common_relay_hooks.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:
adapters/common/src/nemo_fabric_adapters/common/relay_hooks.pytests/adapters/test_adapters_common_relay_hooks.py
**/*.{md,mdx,yml,py,rs,sh}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
Keep documentation aligned with current NeMo Fabric behavior, repository layout, entry points, commands, package names, APIs, bindings, and support claims.
Files:
adapters/common/src/nemo_fabric_adapters/common/relay_hooks.pytests/adapters/test_adapters_common_relay_hooks.py
**/*.{rs,py,pyi,json}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Public contract changes must keep checked-in JSON Schema snapshots and native Python binding declarations synchronized.
Files:
adapters/common/src/nemo_fabric_adapters/common/relay_hooks.pytests/adapters/test_adapters_common_relay_hooks.py
**/*.{rs,py,html,md,mdx,toml,yaml,yml,sh,bash}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
All source files must include the appropriate SPDX copyright and Apache-2.0 license headers using the comment syntax for their file type; MDX files must use a JSX comment.
Files:
adapters/common/src/nemo_fabric_adapters/common/relay_hooks.pytests/adapters/test_adapters_common_relay_hooks.py
adapters/*/
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
Place each first-party adapter under
adapters/<name>/withLICENSE -> ../../LICENSE,README.md,fabric-adapter.json, language-native package and lock files, a source entry point, and focused tests.
Files:
adapters/common/src/nemo_fabric_adapters/common/relay_hooks.py
{adapters/*/fabric-adapter.json,adapters/*/**,tests/adapters/**,docs/**,catalogs/**,share/nemo-fabric/adapters/**}
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
Keep descriptor claims, implementation, focused tests, public documentation, catalog entries, and packaged metadata synchronized, starting with the narrowest truthful capability set.
Files:
adapters/common/src/nemo_fabric_adapters/common/relay_hooks.pytests/adapters/test_adapters_common_relay_hooks.py
adapters/*/**
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
adapters/*/**: Use the publicnemo-fabric-build-adapterskill for adapter-contract semantics, descriptor design, configuration mapping, lifecycle behavior, and conformance evidence.
Use the closest shared first-party host pattern with the same target boundary, consultingadapters/common/and the closest matching adapter.
Files:
adapters/common/src/nemo_fabric_adapters/common/relay_hooks.py
**/*.{md,mdx,rst,yml,yaml,py,sh}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
**/*.{md,mdx,rst,yml,yaml,py,sh}: Keep package names, repository references, and build commands current.
Ensure example commands match current package names and paths.
Files:
adapters/common/src/nemo_fabric_adapters/common/relay_hooks.pytests/adapters/test_adapters_common_relay_hooks.py
{docs,examples,adapters}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-code-freeze/SKILL.md)
Update appropriate current-version installation, package, and configuration examples under
docs,examples, andadaptersfrom the old version to<next-version>, while preserving release notes, changelogs, generated output, and third-party attribution references.
Files:
adapters/common/src/nemo_fabric_adapters/common/relay_hooks.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:
adapters/common/src/nemo_fabric_adapters/common/relay_hooks.py
tests/adapters/**/*.py
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
tests/adapters/**/*.py: If an adapter or integration changes, run its focused tests.
For adapter behavior changes, run focused adapter tests undertests/adapters, then runjust test-python.
Files:
tests/adapters/test_adapters_common_relay_hooks.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 by the async runner.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, useunittest.mock.MagicMockorAsyncMock, using thespecargument when necessary, rather than defining a new class.
Prefix mocked class names withmock, notfake.
Prefer pytest fixtures over helper methods.
If a fixture is needed in multiple test files, define it once inconftest.pyrather than repeating it.
Define fixtures using@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and a<fixture_name>_fixturefunction; specifyscopeonly when it is notfunction.
Preferpytest.mark.parametrizeover separate tests for different input types.
Use@pytest.mark.usefixtureswhen a fixture is needed but its returned value is unused or it returns no value.
Avoid defensive programming in tests; access expected values directly so missing data raises a clear failure, such as usingresults["data"]instead ofresults.get("data").
When adapter installation metadata changes, packaging metadata tests must directly assert that the root project depends unconditionally on the exact-versionnemo-fabric-runtimedistribution.
Packaging metadata tests must verify that each root harness extra delegates to the matching version of the leaf adapter'sharnessextra.
Packaging metadata tests must verify that bare leaf dependencies remain adapter-owned and that the rootadapter-testsdependency group installs each leaf through itsharnessextra.
Packaging metadata tests must verify that every leaf providesfull; only adapters importing NeMo Relay Python APIs providerelay, while adapters using an external Relay executable havefullequal toharness.
Files:
tests/adapters/test_adapters_common_relay_hooks.py
tests/**/*.{rs,py,pyi}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the corresponding Rust crate or in the relevant area under
tests/.
Files:
tests/adapters/test_adapters_common_relay_hooks.py
tests/adapters/test_*.py
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
tests/adapters/test_*.py: Include a subprocess test of the packaged entry point, exact descriptor assertions for every claimed capability, and a credential-free fixture exercisingplan,doctor, andrun.
Keep credentialed live-target tests opt-in and provide deterministic CI coverage.
Files:
tests/adapters/test_adapters_common_relay_hooks.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/adapters/test_adapters_common_relay_hooks.py
🔇 Additional comments (1)
adapters/common/src/nemo_fabric_adapters/common/relay_hooks.py (1)
8-9: LGTM!
|
/merge |
2 participants
Overview
For each we assert the change is reflected in the ATOF traces
Where should the reviewer start?
test_skill_selectionandtest_model_selectiontests in:tests/e2e/test_claude.pytests/e2e/test_codex.pytests/e2e/test_deepagents.pytests/e2e/test_hermes_e2e.pyRelated Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Closes FABRIC-183
I 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