feat(nemo-agents): add NAT Fabric adapter#888
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:
📝 WalkthroughWalkthroughAdds a NeMo Fabric NAT adapter with configuration translation, lifecycle execution, model and capability mapping, packaged discovery, NAT workflow examples, and unit/integration coverage. ChangesNAT Fabric adapter
Sequence Diagram(s)sequenceDiagram
participant Platform
participant translate_agent_config
participant NatRuntime
participant NATWorkflow
Platform->>translate_agent_config: translate NAT agent.yaml
translate_agent_config-->>Platform: return FabricConfig
Platform->>NatRuntime: start runtime
NatRuntime->>NATWorkflow: load workflow and create session
Platform->>NatRuntime: invoke request
NatRuntime->>NATWorkflow: run workflow
NATWorkflow-->>NatRuntime: return result
NatRuntime-->>Platform: return normalized response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugins/nemo-agents/examples/nemo-agent-config/README.md (1)
1-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winComplete the README’s required usage structure.
Add a top-level prerequisites section, pair CLI examples with Python SDK examples in tab-sets, and add a
Next Stepssection with cross-links.As per coding guidelines, documentation pages must list prerequisites first, provide Python SDK and CLI examples in tab-sets, and include a
Next Stepssection.Also applies to: 14-48, 68-72
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/nemo-agents/examples/nemo-agent-config/README.md` around lines 1 - 12, Update the README’s documentation structure by adding a top-level Prerequisites section first, then organize each CLI example with its corresponding Python SDK example in tab-sets. Add a Next Steps section with links to relevant follow-up documentation, while preserving the existing Fabric installation and example content.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.
Nitpick comments:
In `@plugins/nemo-agents/examples/nemo-agent-config/README.md`:
- Around line 1-12: Update the README’s documentation structure by adding a
top-level Prerequisites section first, then organize each CLI example with its
corresponding Python SDK example in tab-sets. Add a Next Steps section with
links to relevant follow-up documentation, while preserving the existing Fabric
installation and example content.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7516a4d7-1ce5-4b00-aed4-bb32872db075
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
plugins/nemo-agents/examples/nemo-agent-config/README.mdplugins/nemo-agents/examples/nemo-agent-config/nat-calculator/agent.yamlplugins/nemo-agents/examples/nemo-agent-config/nat-calculator/workflow.ymlplugins/nemo-agents/examples/nemo-agent-config/nat-email-phishing/agent.yamlplugins/nemo-agents/examples/nemo-agent-config/nat-email-phishing/workflow.ymlplugins/nemo-agents/pyproject.tomlplugins/nemo-agents/src/nemo_agents_plugin/fabric/adapters/nat/adapter.pyplugins/nemo-agents/src/nemo_agents_plugin/fabric/adapters/nat/fabric-adapter.jsonplugins/nemo-agents/src/nemo_agents_plugin/fabric/translator.pyplugins/nemo-agents/tests/integration/test_fabric_nat_adapter.pyplugins/nemo-agents/tests/unit/test_fabric_nat_adapter.pyplugins/nemo-agents/tests/unit/test_fabric_nat_adapter_packaging.pyplugins/nemo-agents/tests/unit/test_fabric_translator.py
da5f87b to
ad55914
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
plugins/nemo-agents/tests/unit/test_fabric_nat_adapter.py (2)
17-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrop redundant string-quoted forward refs.
from __future__ import annotationsalready defers annotation evaluation; quoting"_FakeRunner"(line 22) and"_FakeSessions"(line 40) is unnecessary.As per coding guidelines, "prefer concrete type hints over string-based type hints... import them normally when possible."
♻️ Proposed fix
- async def __aenter__(self) -> "_FakeRunner": + async def __aenter__(self) -> _FakeRunner:- def __init__(self, sessions: "_FakeSessions") -> None: + def __init__(self, sessions: _FakeSessions) -> None:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/nemo-agents/tests/unit/test_fabric_nat_adapter.py` around lines 17 - 56, Remove the redundant string quotes from the _FakeRunner return annotation on _FakeSession.__aenter__ and the _FakeSessions parameter annotation in _FakeSession.__init__, relying on the module’s postponed annotation evaluation while preserving the existing types.Source: Coding guidelines
197-238: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing test for already-started guard.
NatRuntime.startraisesnat_runtime_already_startedwhen called on an already-running instance (per adapter.py'sstart), but no test here exercises callingstart()twice.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/nemo-agents/tests/unit/test_fabric_nat_adapter.py` around lines 197 - 238, Add a unit test covering the already-started guard in NatRuntime.start: successfully start one runtime instance, call start again with the same payload, and assert LifecycleError code nat_runtime_already_started. Ensure cleanup with runtime.stop() so the test remains safe and isolated.
🤖 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.
Nitpick comments:
In `@plugins/nemo-agents/tests/unit/test_fabric_nat_adapter.py`:
- Around line 17-56: Remove the redundant string quotes from the _FakeRunner
return annotation on _FakeSession.__aenter__ and the _FakeSessions parameter
annotation in _FakeSession.__init__, relying on the module’s postponed
annotation evaluation while preserving the existing types.
- Around line 197-238: Add a unit test covering the already-started guard in
NatRuntime.start: successfully start one runtime instance, call start again with
the same payload, and assert LifecycleError code nat_runtime_already_started.
Ensure cleanup with runtime.stop() so the test remains safe and isolated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c72500f0-7be9-4c67-9142-4ba9a9454a3c
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
plugins/nemo-agents/examples/nemo-agent-config/README.mdplugins/nemo-agents/examples/nemo-agent-config/agent.yamlplugins/nemo-agents/examples/nemo-agent-config/nat-calculator/agent.yamlplugins/nemo-agents/examples/nemo-agent-config/nat-calculator/workflow.ymlplugins/nemo-agents/examples/nemo-agent-config/nat-email-phishing/agent.yamlplugins/nemo-agents/examples/nemo-agent-config/nat-email-phishing/workflow.ymlplugins/nemo-agents/pyproject.tomlplugins/nemo-agents/src/nemo_agents_plugin/fabric/adapters/nat/adapter.pyplugins/nemo-agents/src/nemo_agents_plugin/fabric/adapters/nat/fabric-adapter.jsonplugins/nemo-agents/src/nemo_agents_plugin/fabric/translator.pyplugins/nemo-agents/tests/integration/test_fabric_nat_adapter.pyplugins/nemo-agents/tests/unit/test_fabric_nat_adapter.pyplugins/nemo-agents/tests/unit/test_fabric_nat_adapter_packaging.pyplugins/nemo-agents/tests/unit/test_fabric_translator.py
🚧 Files skipped from review as they are similar to previous changes (11)
- plugins/nemo-agents/examples/nemo-agent-config/nat-calculator/agent.yaml
- plugins/nemo-agents/tests/integration/test_fabric_nat_adapter.py
- plugins/nemo-agents/src/nemo_agents_plugin/fabric/adapters/nat/fabric-adapter.json
- plugins/nemo-agents/examples/nemo-agent-config/nat-email-phishing/agent.yaml
- plugins/nemo-agents/examples/nemo-agent-config/nat-email-phishing/workflow.yml
- plugins/nemo-agents/examples/nemo-agent-config/nat-calculator/workflow.yml
- plugins/nemo-agents/pyproject.toml
- plugins/nemo-agents/tests/unit/test_fabric_nat_adapter_packaging.py
- plugins/nemo-agents/examples/nemo-agent-config/README.md
- plugins/nemo-agents/src/nemo_agents_plugin/fabric/translator.py
- plugins/nemo-agents/src/nemo_agents_plugin/fabric/adapters/nat/adapter.py
| @@ -0,0 +1,26 @@ | |||
| function_groups: | |||
There was a problem hiding this comment.
Can we drop the separate workflow.yml requirement and make the NAT configuration part of agent.yaml?
|
|
||
| runtime_id = _runtime_id(payload) | ||
| validate_supported_fabric_config(payload) | ||
| config_file = resolve_config_file(payload) |
There was a problem hiding this comment.
I don’t think workflow.yml or harness.settings.config_file should be part of the NAT adapter contract.
- agent.yaml is the public Platform agent schema and is already translated into a normalized FabricConfig.
- The NAT adapter should consume that FabricConfig, translate its models, harness settings, MCP servers, tools, prompts, environment, and other supported capabilities into a typed NAT Config, and then launch it with WorkflowBuilder.from_config() and SessionManager.create().
Requiring a native NAT workflow file creates a second source of truth and makes FabricConfig an overlay rather than the adapter’s authoritative input. It also introduces llm_map solely to reconcile aliases between the two configurations.
I recommend removing - harness.settings.config_file,harness.settings.llm_map,the example workflow.yml files
the file-resolution and containment machinery
The adapter should instead implement an explicit FabricConfig => NAT Config translation. If information required to construct the NAT workflow is missing from the public agent.yaml/FabricConfig contract, we address that in the public schema or barf (rather than passing native NAT configuration through harness.settings).
This may mean the initial adapter supports only the normalized subset expressible by the public schema. That is preferable to establishing a parallel, NAT-specific configuration contract.
Signed-off-by: Ajay Thorve <[email protected]>
Signed-off-by: Ajay Thorve <[email protected]>
Signed-off-by: Ajay Thorve <[email protected]>
Signed-off-by: Ajay Thorve <[email protected]>
Signed-off-by: Ajay Thorve <[email protected]>
Signed-off-by: Ajay Thorve <[email protected]>
Build on the shared Platform-to-Fabric config translation from NVIDIA-NeMo#890 and apply NAT-native MCP and blocked-tool mappings before the workflow runtime starts. Co-authored-by: Manjesh Mogallapalli <[email protected]> Signed-off-by: Ajay Thorve <[email protected]>
Signed-off-by: Ajay Thorve <[email protected]>
Signed-off-by: Ajay Thorve <[email protected]>
Signed-off-by: Ajay Thorve <[email protected]>
ad55914 to
ab817e0
Compare
2 participants
Overview
Add a Platform-owned, third-party NeMo Agent Toolkit adapter for NeMo Fabric.
agent.yamlis the only agent configuration source: Platform translates itinto a normalized
FabricConfig, and the adapter translates that config into atyped in-memory NAT
Config.This replaces #885, which GitHub closed automatically when its stacked base
#865 merged. The branch is based directly on
mainand incorporates the sharedagent.yamlcapability translation introduced by #890.The change:
nvidia.nemo.platform.natmetadata in the wheel data directorydiscovered by NeMo Fabric after fix: discover installed adapter descriptors NVIDIA/NeMo-Fabric#108;
workflow.yml,harness.settings.config_file,llm_map, and thefile-resolution/containment path;
reactandcurrent_timezoneworkflows;calculator,current_datetime, andemail_phishing_analyzertool names to NAT functions/function groups insidethe adapter rather than exposing native NAT configuration in
agent.yaml;defaultLLM,including provider, model name, temperature, credential environment variable,
and explicit provider settings;
start_runtime, launches it withWorkflowBuilder.from_config()andSessionManager.create(), invokes itthrough NAT's session runner, and shuts it down during
stop;harness_nativeMCP servers to NATmcp_clientfunction groups andenforces
tools.blockedbefore building the workflow;skills.pathsrather than silently ignoring them;agent.yamlthat selects Codex, Hermes, or NAT throughdefault_harness, plus self-contained calculator and email-phishingagent.yamlexamples.NAT serve, streaming, durable Platform sessions, Platform telemetry
normalization, Fabric-managed MCP exposure, runtime skills, arbitrary NAT
component configuration, and multi-LLM NAT workflows remain out of scope.
sequenceDiagram participant Platform participant Fabric participant Adapter as "NAT adapter host" participant NAT Platform->>Fabric: translated agent.yaml + input Fabric->>Adapter: start(runtime context, FabricConfig) Adapter->>Adapter: map workflow, tools, model, MCP, blocked tools Adapter->>NAT: validate typed Config Adapter->>NAT: WorkflowBuilder.from_config() + SessionManager.create() Fabric->>Adapter: invoke(request) Adapter->>NAT: session.run(input, RUN_OR_SERVE) NAT-->>Adapter: workflow result Adapter-->>Fabric: normalized result Fabric->>Adapter: stop(runtime_id) Adapter->>NAT: shutdown sessions, builder, componentsWhere should the reviewer start?
Start with
plugins/nemo-agents/src/nemo_agents_plugin/agent_config.pyfor the narrowPlatform NAT settings contract, then review
plugins/nemo-agents/src/nemo_agents_plugin/fabric/translator.pyandplugins/nemo-agents/src/nemo_agents_plugin/fabric/adapters/nat/adapter.pyforthe Platform -> Fabric -> typed NAT translation. The calculator and phishing
examples under
plugins/nemo-agents/examples/nemo-agent-config/show thecomplete one-file authoring contract.
Validation
-> installed NAT subprocess boundary without a workflow file
WorkflowBuilder/SessionManagerstart-stop with their installed NATcomponents
uv lock --check,ruff check,ruff format --check,git diff --check,and all pre-commit hooks passed
nemo-fabric[runtime]==0.1.0a20260724loaded the installedPlatform descriptor
Live calculator/phishing inference was not run because this shell did not expose
an NVIDIA API credential. Runtime construction used a non-secret placeholder
credential and did not make an external model request.
Related Issues