Expand Relay plugin configuration

[dagardner-nv]

• The Hermes Relay plugin supports configuring both ATOF and ATIF Relay plugins from environment variables.
• While other Relay plugins, notably OTel and OpenInference, require a Relay TOML configuration file or a PluginConfig
• Add a new write_relay_plugins_toml helper method for hermes_cli
• Add new _relay_api_plugin_config helper method for hermes_sdk
• Add a new adapters/common/src/nemo_fabric_adapters/common/utils.py module for methods like write_relay_plugins_toml which are not Hermes specific and could be of use to other adapters.
• Split tests/test_adapaters_hermes_common.py tests into those for the common/hermes and common/utils tests
• Always use the hermes chat --query <prompt> command in the hermes_cli adapter rather that the -z flag, the reason is that with -z the session is not finalized on completion the way it is with hermes chat, this conforms with behavior of the hermes_sdk adapter.
• Remove require_yaml parameter, since JSON is a subset of YAML, writing JSON to a YAML file is always valid we should not raise exceptions on a missing PyYaml library.
• Move the pyyaml dependency to be a dependency of the hermes extra (currently only need to write Hermes config files)
• Add a dependency for tomli-w to the relay extra

Summary by CodeRabbit

Summary

• New Features

  • Added Hermes Relay/OpenInference and Relay/OpenTelemetry example profiles for oneshot tracing and artifact generation.
  • When relay is enabled, Hermes config generation is standardized and may emit a Relay plugins TOML for plugin setup.

• Bug Fixes

  • Updated Hermes CLI argument handling to consistently use chat --quiet --query <prompt> (including session continuation).

• Refactor

  • Unified shared Hermes/relay config and payload normalization across the Hermes CLI and SDK adapters.

• Tests

  • Updated and expanded unit, smoke, and end-to-end tests to match the new shared relay/config behavior and CLI argument expectations.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 699d0740-5624-44a7-94af-aa626903cfec

📥 Commits

Reviewing files that changed from the base of the PR and between 7ad7f1f and f33fa44.

📒 Files selected for processing (1)

• adapters/common/src/nemo_fabric_adapters/common/utils.py

🚧 Files skipped from review as they are similar to previous changes (1)

• adapters/common/src/nemo_fabric_adapters/common/utils.py

---

📝 Walkthrough

Walkthrough

Shared payload/config and relay helper logic moved into common.utils, Hermes config writing and relay wiring use those helpers, the Hermes CLI/SDK adapters consume the new utilities, and new relay profiles plus updated tests were added.

Changes

Hermes shared utilities and relay profiles

Layer / File(s)	Summary
Payload, YAML, and list helpers adapters/common/src/nemo_fabric_adapters/common/utils.py, tests/test_adapaters_common_utils.py, tests/test_adapaters_common_hermes.py	Shared payload accessors, normalize_list, and dump_yaml are added in common.utils, with utility tests added and duplicate Hermes-local coverage removed.
Relay plugin helpers adapters/common/src/nemo_fabric_adapters/common/utils.py, pyproject.toml, tests/test_adapaters_common_hermes.py, tests/test_adapaters_common_utils.py	Relay plugin loading, output normalization, artifact collection, and TOML writing move into common.utils, and the Hermes/relay extras add pyyaml and tomli-w.
Hermes common refactor adapters/common/src/nemo_fabric_adapters/common/hermes.py, tests/smoke_hermes_config_mapping.py	hermes.py now builds configs and relay settings through common.utils, writes configs with the shared YAML helper, and the config smoke loader imports the split modules from source.
Hermes CLI flow adapters/hermes-cli/src/nemo_fabric_adapters/hermes_cli/adapter.py, tests/fixtures/hermes-cli-agent/bin/fake-hermes.py, tests/test_hermes_cli.py, tests/smoke_hermes_cli.py, examples/code-review-agent/profiles/hermes-relay-cli-openinference.yaml	The Hermes CLI adapter now builds chat --quiet --query commands, uses common.utils for payload and relay helpers, and the fake CLI plus E2E tests and CLI profile were updated to the new arguments and records.
Hermes SDK flow adapters/hermes-sdk/src/nemo_fabric_adapters/hermes_sdk/adapter.py, tests/test_hermes_sdk_adapter.py, examples/code-review-agent/profiles/hermes-relay-openinference.yaml	The Hermes SDK adapter now uses common.utils for toolset normalization, config-root resolution, config writing, relay artifact collection, and async execution, with the SDK profile and test updated accordingly.
OpenInference relay profiles examples/code-review-agent/profiles/hermes-cli-relay-otel.yaml, examples/code-review-agent/profiles/hermes-relay-otel.yaml	New SDK and CLI OpenInference profiles set harness runtime, telemetry, and artifact outputs.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• NVIDIA/NeMo-Fabric#15: Refactors the same Hermes/Relay helper surface moved into common.utils, including config writing and relay plugin handling.
• NVIDIA/NeMo-Fabric#18: Touches the Hermes CLI E2E and relay artifact expectations in tests/test_hermes_cli.py.
• NVIDIA/NeMo-Fabric#21: Updates the Hermes session/runtime plumbing around runtime_session_id() and related adapter flow.

Suggested reviewers

• AjayThorve

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 3.51% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title matches the main change: adding support to write a full Relay plugin TOML configuration.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (1)

tests/test_adapaters_common_utils.py (1)

86-160: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add direct coverage for write_relay_plugins_toml().

The tests cover loading and artifact collection, but not the new TOML writer or the FABRIC_RELAY_CONFIG_PATH → relay-plugins.toml path contract.

🤖 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/test_adapaters_common_utils.py` around lines 86 - 160, Add direct test
coverage for write_relay_plugins_toml() and the
FABRIC_RELAY_CONFIG_PATH-to-relay-plugins.toml contract. Extend the common_utils
tests to exercise the TOML writer with a config_root/effective_config payload,
then assert the generated relay-plugins.toml is written in the expected location
and contains the normalized observability config. Use the existing
load_relay_plugin_config() and collect_relay_artifacts() coverage as a guide,
and reference write_relay_plugins_toml() plus FABRIC_RELAY_CONFIG_PATH to locate
the behavior.

🤖 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/hermes.py`:
- Around line 201-205: The relay_model_name helper still assumes the selected
entry from common_utils.models_payload is a dict, so a non-dict model config can
crash downstream adapters. Update relay_model_name to mirror the shared guard by
validating the value returned from models.get(...) before calling .get on it,
and fall back to "unknown" when the model config is not a dict or does not
contain a usable model field.

In `@adapters/common/src/nemo_fabric_adapters/common/utils.py`:
- Around line 162-166: _guard the selected model config in _relay_model_name
before calling .get(): the value retrieved from models_payload(payload) may not
be a mapping, so normalize it to an empty dict when it is not a dict, then keep
the existing fallback chain using settings_payload, models_payload, and the
returned model_config. Ensure _relay_model_name never assumes the selected model
entry supports .get(), matching the safer behavior already used by
selected_model_config._
- Around line 130-141: The artifact collection loop currently uses broad globs
in the section_name-based scan, which can pick up unrelated files from a shared
output_directory. Update the logic in this utility to derive the search pattern
from each section’s configured filename or filename_template instead of
hardcoding "*.jsonl" and "*.json", and keep the existing
enabled/config/directory checks so only the intended artifacts are appended.

In `@tests/fixtures/hermes-cli-agent/bin/fake-hermes.py`:
- Around line 17-20: The fake Hermes CLI contract is too loose in the fixture’s
argument handling: the current `--query` check in fake-hermes.py no longer
enforces the required `chat` subcommand and can still crash when `--query` has
no following value. Update the argument validation around the existing `args`
parsing so it first requires `chat`, then verifies `--query` is present with a
value before reading the prompt, and return a clean error for invalid
invocations instead of raising `IndexError`.

---

Nitpick comments:
In `@tests/test_adapaters_common_utils.py`:
- Around line 86-160: Add direct test coverage for write_relay_plugins_toml()
and the FABRIC_RELAY_CONFIG_PATH-to-relay-plugins.toml contract. Extend the
common_utils tests to exercise the TOML writer with a
config_root/effective_config payload, then assert the generated
relay-plugins.toml is written in the expected location and contains the
normalized observability config. Use the existing load_relay_plugin_config() and
collect_relay_artifacts() coverage as a guide, and reference
write_relay_plugins_toml() plus FABRIC_RELAY_CONFIG_PATH to locate the behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 33e2b98f-06b2-4c9b-b267-8e7c093247f2

📥 Commits

Reviewing files that changed from the base of the PR and between 4b0488a and db1eebb.

📒 Files selected for processing (11)

• adapters/common/src/nemo_fabric_adapters/common/hermes.py
• adapters/common/src/nemo_fabric_adapters/common/utils.py
• adapters/hermes-cli/src/nemo_fabric_adapters/hermes_cli/adapter.py
• adapters/hermes-sdk/src/nemo_fabric_adapters/hermes_sdk/adapter.py
• examples/code-review-agent/profiles/hermes-relay-cli-openinference.yaml
• examples/code-review-agent/profiles/hermes-relay-openinference.yaml
• pyproject.toml
• tests/fixtures/hermes-cli-agent/bin/fake-hermes.py
• tests/test_adapaters_common_hermes.py
• tests/test_adapaters_common_utils.py
• tests/test_hermes_cli.py

[AjayThorve]

@dagardner-nv I think we should split the Relay configuration path by adapter boundary.

For hermes-cli, generating a per-invocation Relay plugin TOML is the correct path. Fabric is launching Hermes as a separate process, so the Relay config has to cross a process boundary through process-visible state: env, CLI args, or a file. Relay’s own plugin config discovery is file-based (plugins.toml layering), so TOML is a reasonable bridge there.

For hermes-sdk, though, we are already in the same Python process before Hermes plugin discovery / agent construction. In that path we should avoid writing relay-plugins.toml and avoid mutating process-global HERMES_NEMO_RELAY_* variables. Relay exposes an in-process API for this:

from nemo_relay import plugin
from nemo_relay.observability import (
    AtofConfig,
    AtifConfig,
    ComponentSpec,
    ObservabilityConfig,
    OtlpConfig,
)

plugin_config = plugin.PluginConfig(
    components=[
        ComponentSpec(
            ObservabilityConfig(
                atof=AtofConfig(...),
                atif=AtifConfig(...),
                openinference=OtlpConfig(...),
            )
        )
    ]
)

async with plugin.plugin(plugin_config):
    # run Hermes SDK conversation
    ...

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

adapters/hermes-sdk/src/nemo_fabric_adapters/hermes_sdk/adapter.py (1)

120-120: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Reaching across module boundary into a private helper.

common_utils._relay_api_plugin_config(...) is consumed here from another module despite its leading underscore marking it as private to common.utils. Either promote it to a public name (relay_api_plugin_config) or wrap it behind a public helper so the SDK adapter isn't coupled to an internal symbol that may change without notice.

🤖 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-sdk/src/nemo_fabric_adapters/hermes_sdk/adapter.py` at line
120, The adapter is calling a private helper from another module, which couples
Hermes SDK’s adapter logic to an internal symbol. Update the adapter to use a
public API instead of common_utils._relay_api_plugin_config, either by
renaming/promoting that helper to relay_api_plugin_config in common.utils or by
introducing a public wrapper that the adapter can call. Keep the change
localized around the relay_api_config assignment in the adapter.py flow so the
SDK depends only on supported symbols.

🤖 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 179-187: The Relay plugin configuration path skips validation
before activation, so malformed config can reach plugin setup and fail late. In
the flow that builds `relay_api_config` via `_relay_api_plugin_config(...)` and
then enters `async with plugin.plugin(relay_api_config)`, insert a
`plugin.validate(...)` call on `relay_api_config` first and handle failures
before activation, keeping the fix localized to the Relay initialization path.

In `@adapters/hermes-sdk/src/nemo_fabric_adapters/hermes_sdk/adapter.py`:
- Around line 119-128: The relay-enabled branch in adapter.py runs the
synchronous _invoke_hermes() directly inside the async with plugin.plugin(...)
context, which can block the relay event loop and delay span/metric draining.
Update the relay execution path in the adapter logic around relay_enabled,
relay_api_config, and plugin.plugin(...) so Hermes runs off the event loop (for
example via a worker thread) or so the relay context only wraps non-blocking
work, while preserving the current result, enabled_toolsets, relay_artifacts,
and adapter_stdout behavior.

---

Nitpick comments:
In `@adapters/hermes-sdk/src/nemo_fabric_adapters/hermes_sdk/adapter.py`:
- Line 120: The adapter is calling a private helper from another module, which
couples Hermes SDK’s adapter logic to an internal symbol. Update the adapter to
use a public API instead of common_utils._relay_api_plugin_config, either by
renaming/promoting that helper to relay_api_plugin_config in common.utils or by
introducing a public wrapper that the adapter can call. Keep the change
localized around the relay_api_config assignment in the adapter.py flow so the
SDK depends only on supported symbols.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 95ac7cb7-b52a-4e2b-a56f-267d6a36451a

📥 Commits

Reviewing files that changed from the base of the PR and between e3c98a6 and 7ad7f1f.

📒 Files selected for processing (3)

• adapters/common/src/nemo_fabric_adapters/common/utils.py
• adapters/hermes-sdk/src/nemo_fabric_adapters/hermes_sdk/adapter.py
• tests/test_hermes_sdk_adapter.py

[AjayThorve]

This split looks right to me now: CLI keeps serialized TOML/env because it launches Hermes out-of-process, and SDK uses the in-process Relay plugin API with scoped cleanup.

Non-blocking follow-up: it would still be useful to add an SDK regression test that exercises a Relay-enabled run followed by a Relay-disabled run in the same Python process, but I don’t think that needs to block this PR.