feat(hermes): run Relay-enabled sessions through the gateway - #124
feat(hermes): run Relay-enabled sessions through the gateway#124bbednarski9 wants to merge 9 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughHermes Relay mode now runs through invocation-scoped ChangesHermes Relay CLI execution
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant HermesRuntime
participant HermesRelayRunner
participant nemo-relay
participant HermesCLI
HermesRuntime->>HermesRelayRunner: invoke(prompt, invocation_id)
HermesRelayRunner->>nemo-relay: start with scoped config and plugins
nemo-relay->>HermesCLI: execute Hermes command
HermesCLI-->>nemo-relay: produce stdout and stderr
nemo-relay-->>HermesRelayRunner: return status and artifacts
HermesRelayRunner-->>HermesRuntime: return HermesRelayResult
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
76a1ca4 to
21795d2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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/utils.py`:
- Around line 316-321: Update the observability validation loop around
config.get("version", 2) to handle malformed version values without allowing int
conversion TypeError or ValueError to escape. Treat any non-integer or invalid
version as failing the required version-2 contract and raise the existing
descriptive ValueError, while preserving acceptance of version 2.
In `@adapters/hermes/README.md`:
- Around line 55-84: Update the prose in the Relay documentation to identify the
product as “NVIDIA NeMo Fabric” on its first occurrence and “NeMo Fabric”
thereafter, replacing standalone capitalized “Fabric” references in the affected
passages while preserving unrelated uses such as “Fabric writes” only when they
refer to the product.
In `@adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py`:
- Around line 366-369: Update the child working-directory resolution near
environment_payload and build_hermes_config so it uses the Hermes configuration
precedence: environment.workspace, then settings.workspace, then root. Remove
settings["cwd"] from this adapter path-resolution fallback, while preserving the
existing relative-to-root handling for non-absolute paths.
In `@adapters/hermes/src/nemo_fabric_adapters/hermes/relay_cli.py`:
- Line 395: Wrap the invocation_dir creation in the relay invocation path around
invocation_dir.mkdir, including FileExistsError and PermissionError, and
re-raise them as HermesRelayError while preserving the original exception as the
cause. Keep successful directory creation unchanged and ensure all failures in
this path follow the existing HermesRelayError contract.
- Around line 195-200: Update the environment construction near
env.update(configured) so the inherited credential is mapped to OPENAI_API_KEY
only when configured does not already provide an explicit OPENAI_API_KEY value.
Preserve the existing inherited api_key_env lookup and child-environment mapping
while honoring operator configuration.
- Around line 330-343: Remove the preexec_fn argument and the related
_linux_parent_death_signal selection from the subprocess launch flow in
relay_cli.py. Preserve start_new_session=True and the existing _stop_process()
shutdown behavior; do not invoke preexec_fn from this post-asyncio.to_thread
startup path.
In `@tests/adapters/test_hermes_relay_cli.py`:
- Around line 63-77: Make the affected Hermes CLI tests portable by gating the
shebang/chmod-based version, runner, and cancellation tests with a POSIX-only
skip condition, or replace their fake executable invocation with sys.executable
and a Windows-compatible .cmd shim for the version case. Update the tests around
hermes_cli_version and the runner/cancellation helpers, including the additional
affected ranges, and avoid relying on POSIX process-group SIGINT behavior on
Windows.
- Around line 15-18: Guard the Hermes imports in test_hermes_relay_cli.py,
including adapter and relay_cli, using the same conditional import pattern as
the existing Hermes test suite so collection succeeds when the Hermes extra is
unavailable on Python 3.14.
- Around line 395-400: The shared _launch helper should be converted into a
pytest factory fixture. Define it as a fixture using the project’s fixture
naming convention, with a _launch_fixture function and the fixture name set to
_launch; preserve its relay_executable, env, and setup behavior, then update the
six tests to request and invoke the fixture instead of calling the helper
directly.
🪄 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: d0d1b945-a474-4578-9f83-8dc171e4581f
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
adapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/hermes/README.mdadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pyadapters/hermes/src/nemo_fabric_adapters/hermes/relay_cli.pypyproject.tomltests/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.py
| env.update(configured) | ||
| if api_key_env in inherited: | ||
| # Hermes is intentionally forced through its OpenAI-compatible custom | ||
| # provider so Relay can own the gateway. Keep the source credential | ||
| # available for plugin references and map it only in the child. | ||
| env["OPENAI_API_KEY"] = inherited[api_key_env] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Explicit harness.settings.env["OPENAI_API_KEY"] is silently overwritten.
env.update(configured) runs first, then the credential mapping unconditionally reassigns OPENAI_API_KEY, so an operator-configured value is discarded without warning. Prefer honoring the explicit setting.
🔧 Proposed fix
- if api_key_env in inherited:
+ if api_key_env in inherited and "OPENAI_API_KEY" not in configured:
# Hermes is intentionally forced through its OpenAI-compatible custom
# provider so Relay can own the gateway. Keep the source credential
# available for plugin references and map it only in the child.
env["OPENAI_API_KEY"] = inherited[api_key_env]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| env.update(configured) | |
| if api_key_env in inherited: | |
| # Hermes is intentionally forced through its OpenAI-compatible custom | |
| # provider so Relay can own the gateway. Keep the source credential | |
| # available for plugin references and map it only in the child. | |
| env["OPENAI_API_KEY"] = inherited[api_key_env] | |
| env.update(configured) | |
| if api_key_env in inherited and "OPENAI_API_KEY" not in configured: | |
| # Hermes is intentionally forced through its OpenAI-compatible custom | |
| # provider so Relay can own the gateway. Keep the source credential | |
| # available for plugin references and map it only in the child. | |
| env["OPENAI_API_KEY"] = inherited[api_key_env] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@adapters/hermes/src/nemo_fabric_adapters/hermes/relay_cli.py` around lines
195 - 200, Update the environment construction near env.update(configured) so
the inherited credential is mapped to OPENAI_API_KEY only when configured does
not already provide an explicit OPENAI_API_KEY value. Preserve the existing
inherited api_key_env lookup and child-environment mapping while honoring
operator configuration.
| def _launch( | ||
| tmp_path: Path, | ||
| *, | ||
| relay_executable: Path | None = None, | ||
| env: dict[str, str] | None = None, | ||
| ) -> relay_cli.HermesRelayLaunch: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Convert _launch into a factory fixture.
It's shared by six tests and carries non-trivial setup. As per coding guidelines, "Prefer pytest fixtures over helper methods" and fixtures are defined as @pytest.fixture(name="<fixture_name>") with a <fixture_name>_fixture function.
♻️ Sketch
-def _launch(
- tmp_path: Path,
- *,
- relay_executable: Path | None = None,
- env: dict[str, str] | None = None,
-) -> relay_cli.HermesRelayLaunch:
+@pytest.fixture(name="make_launch")
+def make_launch_fixture():
+ def _make(
+ tmp_path: Path,
+ *,
+ relay_executable: Path | None = None,
+ env: dict[str, str] | None = None,
+ ) -> relay_cli.HermesRelayLaunch:
+ ...
+
+ return _make🤖 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/adapters/test_hermes_relay_cli.py` around lines 395 - 400, The shared
_launch helper should be converted into a pytest factory fixture. Define it as a
fixture using the project’s fixture naming convention, with a _launch_fixture
function and the fixture name set to _launch; preserve its relay_executable,
env, and setup behavior, then update the six tests to request and invoke the
fixture instead of calling the helper directly.
Source: Coding guidelines
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py (1)
496-518: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRedact the prompt from captured Relay output.
The prompt is passed as a CLI argument, but
result.stdoutis returned as bothresponseandadapter_stdout.quiet_responseonly removes the pinned startup diagnostic, so an echoed prompt—including accidental credentials—can escape through the adapter response. Apply shared prompt redaction to captured output and add a regression test.🤖 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 `@adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py` around lines 496 - 518, Update the Relay invocation handling around _relay_runner.invoke and quiet_response to apply the shared prompt-redaction utility to captured stdout before returning it as response or adapter_stdout, ensuring echoed CLI prompts and credentials are removed while preserving normal output. Add a regression test covering prompt text echoed in Relay stdout and verify it is absent from both returned fields.tests/adapters/test_hermes_relay_cli.py (1)
131-138: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
os.environwith the repository’s restoration fixture.The test guidelines require the autouse
restore_environ_fixture; they explicitly prohibitmonkeypatch.setenvandmonkeypatch.delenv. Replace these mutations withos.environ[...] = ...andos.environ.pop(..., None).
As per coding guidelines, environment changes in tests must useos.environwith the autouse restoration fixture.Also applies to: 349-372
🤖 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/adapters/test_hermes_relay_cli.py` around lines 131 - 138, Update test_child_environment_forwards_only_explicit_credentials and the additionally affected environment setup/cleanup to use direct os.environ assignments and os.environ.pop(..., None) instead of monkeypatch.setenv or monkeypatch.delenv. Rely on the repository’s autouse restore_environ_fixture for restoration, while preserving the existing credential and unrelated-secret values.Source: Coding guidelines
♻️ Duplicate comments (4)
adapters/hermes/README.md (1)
55-70: 📐 Maintainability & Code Quality | 🟡 MinorUse
NeMo Fabricinstead of standaloneFabricin prose.These lines use
Fabric,Fabric's, andpersistent Fabric runtimefor the product. UseNeMo Fabricafter the existing first full usage.
As per path instructions, user-facing prose must useNVIDIA NeMo Fabricon first use andNeMo Fabricthereafter.Also applies to: 86-88
🤖 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 `@adapters/hermes/README.md` around lines 55 - 70, Update the user-facing prose in the Hermes README to use “NVIDIA NeMo Fabric” on the first product reference, then “NeMo Fabric” for subsequent references, including possessive and “persistent” usages; do not use standalone “Fabric” for the product.Source: Path instructions
tests/adapters/test_hermes_relay_cli.py (2)
91-116: 🩺 Stability & Availability | 🟠 MajorKeep POSIX-only fake-process tests out of Windows runs.
These tests use shebang-based executables,
chmod, POSIX process groups, and SIGINT cancellation. Gate them with a POSIX-only marker or invoke helpers throughsys.executablewith a Windows-compatible version shim.Also applies to: 166-226, 266-316, 318-347, 485-556
🤖 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/adapters/test_hermes_relay_cli.py` around lines 91 - 116, Restrict the fake-process tests in test_hermes_cli_version_contract and the additionally referenced test blocks to POSIX platforms, since they create shebang executables, change executable permissions, and exercise POSIX process behavior. Apply the project’s existing POSIX-only marker or equivalent platform guard consistently to those tests, while leaving non-process-specific tests unchanged.
432-483: 📐 Maintainability & Code Quality | 🔵 TrivialConvert
_launchinto a pytest factory fixture.This shared setup helper should follow the repository rule to prefer fixtures, using a fixture factory with the required
<fixture_name>_fixturenaming convention.🤖 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/adapters/test_hermes_relay_cli.py` around lines 432 - 483, Convert the `_launch` helper into a pytest fixture factory named with the required `<fixture_name>_fixture` convention, returning a callable that accepts the current `tmp_path`, `relay_executable`, and `env` overrides and produces the same HermesRelayLaunch setup. Update its callers to request the fixture and invoke the returned factory while preserving all existing defaults and configuration.Source: Coding guidelines
adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py (1)
382-384: 🎯 Functional Correctness | 🟡 MinorUse the Hermes configuration’s working-directory precedence.
build_hermes_configusesenvironment.workspace, thensettings.workspace, but Relay launch usessettings["cwd"]first. When both are set, Hermes terminal commands and the child process run in different directories. Removesettings["cwd"]and preserve root-relative resolution.
Based on learnings, adapter paths are config-root-relative andharness.settings.cwdis unsupported here.🤖 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 `@adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py` around lines 382 - 384, Update the working-directory resolution in the Relay launch path to use the Hermes precedence of environment.workspace, then settings.workspace, then root; remove settings["cwd"] from the fallback chain. Preserve resolving any relative selected directory against root so terminal commands and the child process share the same config-root-relative working directory.Source: Learnings
🤖 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/hermes/src/nemo_fabric_adapters/hermes/adapter.py`:
- Around line 49-53: Resolve the TRY003 warning in the relay validation branch
by introducing a small ValueError subclass or using the project’s existing
exception type, with the stable message defined on that exception. Update the
raise in the relay_enabled check to use the new exception while preserving the
exact current error text for callers and tests.
In `@tests/adapters/test_hermes_relay_cli.py`:
- Around line 21-34: Update test_validate_hermes_relay_plugin_mode to replace
the boolean relay_enabled and raises parameters with concrete telemetry
configuration and expected-error values, and adjust the parameterized cases and
assertions accordingly. Remove the boolean-typed positional test parameters
while preserving the existing validation scenarios and outcomes.
---
Outside diff comments:
In `@adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py`:
- Around line 496-518: Update the Relay invocation handling around
_relay_runner.invoke and quiet_response to apply the shared prompt-redaction
utility to captured stdout before returning it as response or adapter_stdout,
ensuring echoed CLI prompts and credentials are removed while preserving normal
output. Add a regression test covering prompt text echoed in Relay stdout and
verify it is absent from both returned fields.
In `@tests/adapters/test_hermes_relay_cli.py`:
- Around line 131-138: Update
test_child_environment_forwards_only_explicit_credentials and the additionally
affected environment setup/cleanup to use direct os.environ assignments and
os.environ.pop(..., None) instead of monkeypatch.setenv or monkeypatch.delenv.
Rely on the repository’s autouse restore_environ_fixture for restoration, while
preserving the existing credential and unrelated-secret values.
---
Duplicate comments:
In `@adapters/hermes/README.md`:
- Around line 55-70: Update the user-facing prose in the Hermes README to use
“NVIDIA NeMo Fabric” on the first product reference, then “NeMo Fabric” for
subsequent references, including possessive and “persistent” usages; do not use
standalone “Fabric” for the product.
In `@adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py`:
- Around line 382-384: Update the working-directory resolution in the Relay
launch path to use the Hermes precedence of environment.workspace, then
settings.workspace, then root; remove settings["cwd"] from the fallback chain.
Preserve resolving any relative selected directory against root so terminal
commands and the child process share the same config-root-relative working
directory.
In `@tests/adapters/test_hermes_relay_cli.py`:
- Around line 91-116: Restrict the fake-process tests in
test_hermes_cli_version_contract and the additionally referenced test blocks to
POSIX platforms, since they create shebang executables, change executable
permissions, and exercise POSIX process behavior. Apply the project’s existing
POSIX-only marker or equivalent platform guard consistently to those tests,
while leaving non-process-specific tests unchanged.
- Around line 432-483: Convert the `_launch` helper into a pytest fixture
factory named with the required `<fixture_name>_fixture` convention, returning a
callable that accepts the current `tmp_path`, `relay_executable`, and `env`
overrides and produces the same HermesRelayLaunch setup. Update its callers to
request the fixture and invoke the returned factory while preserving all
existing defaults and configuration.
🪄 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: 38a75bbe-5034-4648-aa43-44055527da49
📒 Files selected for processing (3)
adapters/hermes/README.mdadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pytests/adapters/test_hermes_relay_cli.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (24)
**/*.{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:
adapters/hermes/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:
adapters/hermes/README.mdtests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
**/*.{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:
adapters/hermes/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:
adapters/hermes/README.md
adapters/*/README.md
📄 CodeRabbit inference engine (AGENTS.md)
Update adapter README files when public behavior, examples, or supported bindings change.
Files:
adapters/hermes/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:
adapters/hermes/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 docs site changes, run
just docsto regenerate Python and Rust API references and validate the Fern configuration.
Files:
adapters/hermes/README.md
**/*.{rs,py,html,md,mdx,toml,yml,yaml,sh,bash}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
All Rust, Python, HTML, Markdown, MDX, TOML, YAML, and shell source files must include the project SPDX copyright and Apache-2.0 license headers using the comment syntax appropriate to each file type.
Files:
adapters/hermes/README.mdtests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
**/{README.md,*.md,*.mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant documentation when changes affect public behavior, adapters, examples, or workspace structure.
Files:
adapters/hermes/README.md
**/*.md
📄 CodeRabbit inference engine (.agents/skills/README.md)
Documentation and examples must be updated consistently with changes to public behavior and reviewed for NVIDIA technical-writing style.
Files:
adapters/hermes/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:
adapters/hermes/README.mdadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
{*.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:
adapters/hermes/README.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/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.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}: When changing the Rust core or public schemas, run both the Rust and Python test suites.
When adding functionality, include tests in the corresponding Rust crate or the relevant area undertests/.
Files:
tests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.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/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.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/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.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_hermes_relay_cli.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/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.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/adapters/test_hermes_relay_cli.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/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.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/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use type annotations for public Python APIs.
Files:
tests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
**/*.{py,rs}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{py,rs}: Keep native Python binding declarations synchronized with their Rust implementations.
Usesnake_casefor functions and variables; usePascalCasefor Rust types and Python classes.
Files:
tests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.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_hermes_relay_cli.py
🧠 Learnings (1)
📚 Learning: 2026-07-09T22:28:51.689Z
Learnt from: AjayThorve
Repo: NVIDIA/NeMo-Fabric PR: 43
File: adapters/claude-sdk/src/nemo_fabric_adapters/claude_sdk/adapter.py:164-168
Timestamp: 2026-07-09T22:28:51.689Z
Learning: In the NeMo-Fabric adapters, treat path values used in Fabric adapter configuration (including logic like `_resolve_path` in adapter.py) as config-root-relative. Do not apply `Path.expanduser()` (or otherwise apply `~`/home or shell-style expansion), because it will make the resolved paths normalize inconsistently across adapters. Also, do not rely on or add any resolution behavior that uses `harness.settings.cwd` as an override point for these adapter paths—`harness.settings.cwd` is explicitly unsupported in this adapter context.
Applied to files:
adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
🪛 Ruff (0.15.21)
tests/adapters/test_hermes_relay_cli.py
[warning] 31-31: Boolean-typed positional argument in function definition
(FBT001)
[warning] 33-33: Boolean-typed positional argument in function definition
(FBT001)
adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
[warning] 50-53: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (4)
tests/adapters/test_hermes_relay_cli.py (2)
1-20: 🩺 Stability & AvailabilityVerify Hermes imports are safe when the optional extra is unavailable.
The previous collection failure occurred because this module imported Hermes modules unconditionally on Python 3.14. Confirm the current imports use
pytest.importorskipfor bothadapterandrelay_cli; otherwise the whole test module still fails during collection.rg -n 'importorskip|from nemo_fabric_adapters\.hermes|import nemo_fabric_adapters\.hermes' tests/adapters/test_hermes_relay_cli.py
35-56: LGTM!Also applies to: 58-88, 117-129, 228-265, 558-573
adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py (1)
34-34: LGTM!Also applies to: 43-48, 64-68, 124-127, 139-146, 219-221, 231-232, 243-254, 267-288, 343-371, 385-415, 417-423, 443-450, 485-495, 519-547, 549-586, 623-646
adapters/hermes/README.md (1)
8-43: LGTM!Also applies to: 45-53, 72-84, 92-112
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/adapters/test_hermes_relay_cli.py (1)
163-170: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
restore_environ_fixtureinstead ofmonkeypatch.setenv.Replace
setenv/delenvwithos.environupdates; retainmonkeypatchonly for object patching.
As per coding guidelines, test environment changes must useos.environwith the autouse restoration fixture.Also applies to: 381-404
🤖 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/adapters/test_hermes_relay_cli.py` around lines 163 - 170, Update test_child_environment_forwards_only_explicit_credentials and the related environment setup at the referenced later block to modify os.environ directly instead of using monkeypatch.setenv or monkeypatch.delenv. Retain monkeypatch only for object patching, relying on the autouse restore_environ_fixture to restore environment changes.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/adapters/test_hermes_adapter.py`:
- Around line 61-64: Restore lifecycle regression tests alongside
test_build_hermes_config_maps_fabric_config_to_hermes_config, covering
HermesRuntime.stop() invoking the runner, resetting runtime state, and
propagating cleanup failures. Include assertions for finalization ordering and
stop-failure behavior, using the existing test fixtures and mocking patterns.
- Line 64: Replace the hard-coded “/fabric-root” values with platform-native
roots derived from tmp_path across the adapter tests: use tmp_path in the main
config-mapping, unset-iterations, null-iterations, and gateway-plugin tests in
tests/adapters/test_hermes_adapter.py (sites 64-64, 154-154, 170-170, and
310-310), and derive the workspace-precedence matrix from tmp_path in
tests/adapters/test_hermes_relay_cli.py (44-47). Update expected paths
consistently from the same root.
---
Outside diff comments:
In `@tests/adapters/test_hermes_relay_cli.py`:
- Around line 163-170: Update
test_child_environment_forwards_only_explicit_credentials and the related
environment setup at the referenced later block to modify os.environ directly
instead of using monkeypatch.setenv or monkeypatch.delenv. Retain monkeypatch
only for object patching, relying on the autouse restore_environ_fixture to
restore environment changes.
🪄 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: 106fd894-0a83-4439-a823-98889bdb49b8
📒 Files selected for processing (3)
adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pytests/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.py
📜 Review details
🧰 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/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.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/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.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}: When changing the Rust core or public schemas, run both the Rust and Python test suites.
When adding functionality, include tests in the corresponding Rust crate or the relevant area undertests/.
Files:
tests/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.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/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.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/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.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_hermes_adapter.pytests/adapters/test_hermes_relay_cli.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/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.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/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.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/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.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/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
**/*.{rs,py,html,md,mdx,toml,yml,yaml,sh,bash}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
All Rust, Python, HTML, Markdown, MDX, TOML, YAML, and shell source files must include the project SPDX copyright and Apache-2.0 license headers using the comment syntax appropriate to each file type.
Files:
tests/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use type annotations for public Python APIs.
Files:
tests/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
**/*.{py,rs}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{py,rs}: Keep native Python binding declarations synchronized with their Rust implementations.
Usesnake_casefor functions and variables; usePascalCasefor Rust types and Python classes.
Files:
tests/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.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_hermes_adapter.pytests/adapters/test_hermes_relay_cli.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/hermes/src/nemo_fabric_adapters/hermes/adapter.py
🧠 Learnings (1)
📚 Learning: 2026-07-09T22:28:51.689Z
Learnt from: AjayThorve
Repo: NVIDIA/NeMo-Fabric PR: 43
File: adapters/claude-sdk/src/nemo_fabric_adapters/claude_sdk/adapter.py:164-168
Timestamp: 2026-07-09T22:28:51.689Z
Learning: In the NeMo-Fabric adapters, treat path values used in Fabric adapter configuration (including logic like `_resolve_path` in adapter.py) as config-root-relative. Do not apply `Path.expanduser()` (or otherwise apply `~`/home or shell-style expansion), because it will make the resolved paths normalize inconsistently across adapters. Also, do not rely on or add any resolution behavior that uses `harness.settings.cwd` as an override point for these adapter paths—`harness.settings.cwd` is explicitly unsupported in this adapter context.
Applied to files:
adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
🔇 Additional comments (5)
tests/adapters/test_hermes_relay_cli.py (4)
15-18: Duplicate: guard optional Hermes imports during collection.The previous Python 3.14 collection failure remains relevant: missing Hermes extras must skip this module rather than abort collection. Preserve the
pytest.importorskip-style guard.Source: Pipeline failures
62-66: Duplicate: remove boolean control parameters from the parametrized test.This is the previously reported FBT001 finding; parameterize concrete telemetry and expected-error cases instead of
relay_enabledandraisesbooleans.Source: Linters/SAST tools
123-147: Duplicate: keep fake CLI tests portable across Windows.The prior Windows failures covered shebang/chmod-based executables and POSIX process-group
SIGINT. Keep these tests POSIX-gated or invoke helpers throughsys.executablewith a Windows-compatible shim.Also applies to: 198-347, 350-380, 517-589
Source: Pipeline failures
464-514: Duplicate: make_launcha pytest factory fixture.The shared helper should follow the repository’s fixture convention so setup is injected consistently across tests.
As per coding guidelines, prefer pytest fixtures over helper methods.Source: Coding guidelines
adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py (1)
43-53: LGTM!Also applies to: 64-72, 75-139, 147-154, 227-229, 239-262, 275-296, 351-420, 422-488, 490-552, 554-602
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/utils.py`:
- Around line 320-329: Update the observability config validation around the
existing config/version check to reject any non-dict config, including lists,
strings, and None, before reading version. Preserve the current version-two
validation for dictionaries and raise the same ValueError for all invalid
configurations.
In `@tests/adapters/test_adapaters_common_utils.py`:
- Around line 354-375: Expand the parametrized version cases in
test_relay_cli_plugin_config_rejects_malformed_observability_versions to include
non-required integers such as 1 and 3, while preserving the existing malformed
values and expected ValueError assertion.
🪄 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: 823ff7b1-933f-45de-8202-2ff8aa3701c7
📒 Files selected for processing (6)
adapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/hermes/README.mdadapters/hermes/src/nemo_fabric_adapters/hermes/relay_cli.pytests/adapters/test_adapaters_common_utils.pytests/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.py
📜 Review details
🧰 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/adapters/test_adapaters_common_utils.pyadapters/common/src/nemo_fabric_adapters/common/utils.pytests/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/relay_cli.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/adapters/test_adapaters_common_utils.pyadapters/hermes/README.mdadapters/common/src/nemo_fabric_adapters/common/utils.pytests/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/relay_cli.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}: When changing the Rust core or public schemas, run both the Rust and Python test suites.
When adding functionality, include tests in the corresponding Rust crate or the relevant area undertests/.
Files:
tests/adapters/test_adapaters_common_utils.pyadapters/common/src/nemo_fabric_adapters/common/utils.pytests/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/relay_cli.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/adapters/test_adapaters_common_utils.pyadapters/common/src/nemo_fabric_adapters/common/utils.pytests/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/relay_cli.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/adapters/test_adapaters_common_utils.pyadapters/common/src/nemo_fabric_adapters/common/utils.pytests/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/relay_cli.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_adapaters_common_utils.pytests/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.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/adapters/test_adapaters_common_utils.pyadapters/common/src/nemo_fabric_adapters/common/utils.pytests/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/relay_cli.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/adapters/test_adapaters_common_utils.pytests/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.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/adapters/test_adapaters_common_utils.pyadapters/common/src/nemo_fabric_adapters/common/utils.pytests/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/relay_cli.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/adapters/test_adapaters_common_utils.pyadapters/common/src/nemo_fabric_adapters/common/utils.pytests/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/relay_cli.py
**/*.{rs,py,html,md,mdx,toml,yml,yaml,sh,bash}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
All Rust, Python, HTML, Markdown, MDX, TOML, YAML, and shell source files must include the project SPDX copyright and Apache-2.0 license headers using the comment syntax appropriate to each file type.
Files:
tests/adapters/test_adapaters_common_utils.pyadapters/hermes/README.mdadapters/common/src/nemo_fabric_adapters/common/utils.pytests/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/relay_cli.py
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use type annotations for public Python APIs.
Files:
tests/adapters/test_adapaters_common_utils.pyadapters/common/src/nemo_fabric_adapters/common/utils.pytests/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/relay_cli.py
**/*.{py,rs}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{py,rs}: Keep native Python binding declarations synchronized with their Rust implementations.
Usesnake_casefor functions and variables; usePascalCasefor Rust types and Python classes.
Files:
tests/adapters/test_adapaters_common_utils.pyadapters/common/src/nemo_fabric_adapters/common/utils.pytests/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.pyadapters/hermes/src/nemo_fabric_adapters/hermes/relay_cli.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_adapaters_common_utils.pytests/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.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:
adapters/hermes/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:
adapters/hermes/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:
adapters/hermes/README.md
adapters/*/README.md
📄 CodeRabbit inference engine (AGENTS.md)
Update adapter README files when public behavior, examples, or supported bindings change.
Files:
adapters/hermes/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:
adapters/hermes/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 docs site changes, run
just docsto regenerate Python and Rust API references and validate the Fern configuration.
Files:
adapters/hermes/README.md
**/{README.md,*.md,*.mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant documentation when changes affect public behavior, adapters, examples, or workspace structure.
Files:
adapters/hermes/README.md
**/*.md
📄 CodeRabbit inference engine (.agents/skills/README.md)
Documentation and examples must be updated consistently with changes to public behavior and reviewed for NVIDIA technical-writing style.
Files:
adapters/hermes/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:
adapters/hermes/README.mdadapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/hermes/src/nemo_fabric_adapters/hermes/relay_cli.py
{*.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:
adapters/hermes/README.md
🪛 Ruff (0.15.21)
adapters/common/src/nemo_fabric_adapters/common/utils.py
[warning] 327-329: Avoid specifying long messages outside the exception class
(TRY003)
adapters/hermes/src/nemo_fabric_adapters/hermes/relay_cli.py
[warning] 391-393: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (4)
adapters/hermes/src/nemo_fabric_adapters/hermes/relay_cli.py (1)
388-393: LGTM!tests/adapters/test_hermes_relay_cli.py (1)
15-26: LGTM!Also applies to: 29-73, 155-155, 222-230, 233-233, 272-272, 297-297, 336-336, 389-389
adapters/hermes/README.md (1)
8-11: 📐 Maintainability & Code QualityValidate the Relay Command and Generated Documentation.
Run
just docsand verify the documentednemo-relay run --config <config.toml> --agent hermes -- <hermes chat args>contract against the supported CLI before merge. As per coding guidelines, documentation changes must runjust docswhen practical and verify documented commands.Also applies to: 55-59, 60-60, 69-70, 86-86
Source: Coding guidelines
tests/adapters/test_hermes_adapter.py (1)
15-15: LGTM!Also applies to: 62-99, 101-107, 159-159, 192-198, 210-216, 354-356
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com> # Conflicts: # adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
262b318 to
e1fa2e0
Compare
Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
1 participant
Summary
Run Relay-enabled Hermes sessions through the public NeMo Relay CLI/gateway
boundary. When Fabric Relay telemetry is disabled, run Hermes directly through
its Python SDK without Fabric-managed Relay telemetry.
This is the first of three stacked follow-ups to #107 and #114. The later
layers add sink-aware artifact compatibility and invocation-scoped dynamic
plugins.
Why
The Terminal-Bench 2.0 evaluation exposed several brittle integration
boundaries in the earlier Hermes path:
127.0.0.1:9endpoint instead of the configured upstream;teardown failed afterward;
Relay/gateway/Hermes process tree;
environment contracts were implicit.
Using the CLI/gateway boundary gives Relay ownership of gateway startup and the
wrapped Hermes process while keeping that execution detail internal to the
Fabric adapter.
Changes
Relay; keep the existing direct Hermes path otherwise.
credential, explicitly configured values, and environment variables
referenced by Relay plugin configuration.
provider-token work.
configuration before launch.
public native-versus-gateway mode selector.
environment.workspace, thenharness.settings.workspace, then the Fabricroot; relative values remain rooted under the Fabric invocation.
timeout, and descendant cleanup.
error occurs.
observability/nemo_relayplugin from the gatewaypath to avoid duplicate model-event observation.
observability/nemo_relayplugin whenFabric Relay telemetry is disabled, rather than silently running a partial,
unmanaged Relay integration.
Sink-aware artifact containment is intentionally left to the next stacked PR.
Invocation-scoped dynamic plugin registration is left to the third PR.
Evaluation evidence
gateway route and passed validation, routing attribution, and Phoenix upload.
Its reward of zero was a valid benchmark nonpass, not an integration failure.
completed-response preservation contract.
2,700-second hang was not reproduced.
are deferred to the next PR.
The PR does not claim to fix evaluation-harness failures, Docker outages,
upstream Hermes stdout/SQLite deadlocks, Relay binary packaging, or Phoenix
exporter scalability.
Validation
Focused suites:
tests/adapters/test_adapaters_common_utils.pytests/adapters/test_hermes_adapter.pytests/adapters/test_hermes_relay_cli.pyuv lock --checkalso passes.Summary by CodeRabbit