feat: add external NAT reference adapter - #175
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:
WalkthroughAdds a source-only NVIDIA NeMo Agent Toolkit adapter. The adapter translates Fabric configuration, filters MCP tools, manages NAT workflow sessions, normalizes lifecycle errors, and provides calculator and email-phishing examples with comprehensive tests. ChangesNeMo Agent Toolkit adapter
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant FabricRunner
participant NatRuntime
participant NATConfig
participant NATWorkflow
participant NATSession
FabricRunner->>NatRuntime: start(payload)
NatRuntime->>NATConfig: build and validate configuration
NatRuntime->>NATWorkflow: create workflow
NatRuntime->>NATSession: create shared session
FabricRunner->>NatRuntime: invoke(payload)
NatRuntime->>NATSession: propagate session context
NatRuntime->>NATWorkflow: execute input
NATWorkflow-->>NatRuntime: workflow result
NatRuntime-->>FabricRunner: serialized result
FabricRunner->>NatRuntime: stop()
NatRuntime->>NATWorkflow: close resources
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 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 `@external/nat/fabric-adapter.json`:
- Around line 56-65: Update the config.accepts list in the fabric adapter
manifest to contain only the top-level capability roots consumed by
resolve_capability_plan: models, instructions, tools, and mcp. Remove the dotted
entries such as models.base_url, instructions.system, tools.enabled,
tools.blocked, and mcp.tool_filters.
In `@external/nat/README.md`:
- Around line 108-109: Update the editable-install command in the README to wrap
the `/path/to/` placeholder in angle brackets, clearly distinguishing it from
the actual project path while preserving the rest of the command.
In `@external/nat/src/nemo_fabric_adapters/nat/adapter.py`:
- Around line 522-528: Remove the redundant suppressed-membership check and its
continue from the identity-in-groups branch, leaving selected_groups and
selected_refs handling unchanged; the earlier suppressed filter already excludes
those identities.
- Around line 676-693: Update invoke to call _session_kwargs before its broad
try/except block, alongside the existing non-mapping request validation, so
ValueError for malformed request.context propagates as a caller-input error.
Convert that validation error into _failure_output using the existing
nat_invalid_request code and preserve the specific validation message; leave
LifecycleError handling inside the workflow try block.
- Around line 566-597: Update `_block_group_member` to mutate
`config["workflow"]["tool_names"]` in place instead of rebinding it, preserving
the alias held by `_apply_blocked_tools` for subsequent blocked identities. Add
a regression test covering a blocked group member followed by a sibling
function, and verify both are removed from the final workflow configuration.
In `@tests/adapters/test_external_nat_adapter.py`:
- Around line 81-147: Add an opt-in integration test that uses
pytest.importorskip("nat") to import the real NAT package, constructs a typed
Config from a Fabric payload, and asserts validation succeeds. Keep the existing
mock_nat_fixture unit coverage unchanged; use the real
nat.data_models.config.Config API to provide contract coverage without making
NAT a required dependency.
- Around line 543-564: Extend the NatRuntime tests around
test_runtime_reuses_one_builder_across_invocations_and_cleans_up to cover all
guard branches: assert a second start raises nat_runtime_already_started,
invoking before start raises nat_runtime_not_started, invoking with a different
runtime_id raises nat_runtime_mismatch, and passing a non-mapping request raises
nat_invalid_request. Reuse make_payload and mock_nat, and parameterize the
invoke-side cases if appropriate.
- Around line 687-708: Add focused tests for the stdio branch in
nat_mcp_server_config: verify a valid command is split into command and args,
environment variables in the command are expanded via os.path.expandvars,
unbalanced quotes raise LifecycleError with code nat_invalid_mcp_server, and
whitespace-only URLs also raise the same error for an empty command. Keep the
existing HTTP alias and unsupported-transport tests unchanged.
- Around line 21-25: Scope both global-state changes in
tests/adapters/test_external_nat_adapter.py: move NAT_ADAPTER_SOURCE sys.path
setup into pytest configuration or tests/adapters/conftest.py and remove the
now-unneeded noqa on the adapter import; in the test at lines 217-249, add a
pytest.MonkeyPatch parameter and replace direct NVIDIA_API_KEY assignment with
monkeypatch.setenv. The existing mock_nat fixture needs no change.
- Around line 21-25: Move the external NAT source path setup from the
module-level sys.path.insert in test_external_nat_adapter.py into pytest
configuration via tool.pytest.ini_options.pythonpath or tests/conftest.py.
Remove the local path manipulation and the associated noqa: E402 from the
nemo_fabric_adapters.nat import adapter statement, preserving the test’s ability
to import the source-only adapter.
- Around line 168-180: Convert the module-level invocation_payload helper into a
pytest factory fixture, following the existing make_payload pattern. Update each
of the four async tests that currently calls invocation_payload to request the
fixture and invoke the returned factory, preserving its defaults and payload
construction behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: d4f7e97e-8479-4d70-8449-1acd2307e5ff
📒 Files selected for processing (8)
external/README.mdexternal/nat/README.mdexternal/nat/examples/calculator.pyexternal/nat/examples/email_phishing.pyexternal/nat/fabric-adapter.jsonexternal/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.pytests/adapters/test_external_nat_adapter.py
📜 Review details
⏰ Context from checks skipped due to timeout. (18)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.12, linux-amd64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.13, linux-arm64)
- GitHub Check: Test (Python 3.11, linux-amd64)
- GitHub Check: Test (Python 3.13, linux-amd64)
- GitHub Check: Test (Python 3.12, linux-arm64)
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Test (Python 3.14, linux-arm64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.14, macos-arm64)
- GitHub Check: Test (Python 3.14, linux-amd64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Test (Python 3.11, linux-arm64)
- GitHub Check: Pre-commit
- GitHub Check: Test (arm64)
🧰 Additional context used
📓 Path-based instructions (21)
**/*.{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:
external/README.mdexternal/nat/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:
external/README.mdexternal/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/nat/fabric-adapter.jsontests/adapters/test_external_nat_adapter.pyexternal/nat/README.mdexternal/nat/examples/calculator.pyexternal/nat/examples/email_phishing.pyexternal/nat/src/nemo_fabric_adapters/nat/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:
external/README.mdexternal/nat/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:
external/README.mdexternal/nat/README.md
**/*.{md,mdx,yml,py,rs,sh}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
Keep documentation aligned with current NeMo Fabric behavior, repository layout, entry points, commands, package names, APIs, bindings, and support claims.
Files:
external/README.mdexternal/nat/src/nemo_fabric_adapters/nat/__init__.pytests/adapters/test_external_nat_adapter.pyexternal/nat/README.mdexternal/nat/examples/calculator.pyexternal/nat/examples/email_phishing.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.py
**/*.{md,mdx}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
**/*.{md,mdx}: Use the full product nameNVIDIA NeMo Fabricon first use, typically in the title and H1; useNeMo Fabricthereafter. Usefabricalone only for the CLI tool and surround it with backticks.
Treat incorrect or stale commands, package names, paths, APIs, support claims, procedures, examples, terminology, or public behavior documentation as blocking issues.
CapitalizeNVIDIAcorrectly and format code, commands, paths, and filenames as inline code where needed.
Use title case for technical-documentation headings.
Introduce code blocks, tables, and lists with complete lead-in sentences; ensure examples match current APIs and build commands.
Use descriptive anchor text, avoid raw URLs and generic labels such ashere, and use repository-relative.mdxpaths for links withindocs/.
Prefer active voice, present tense, short sentences, plain English, consistent terminology, and imperative, parallel, scannable procedures.
Useafterinstead ofoncewhen expressing temporal sequence, and usecanrather thanmaywhen describing possibility rather than permission.
Avoid ambiguous numeric dates and ordinal dates in body text.
For learning-oriented documentation, do not force trademark symbols unless the source document explicitly requires them.
When reporting documentation-review findings, lead withMust fix,Should fix, andNice to havecategories; include file path, line reference, current problem, rationale, and a concrete rewrite or direction.
Files:
external/README.mdexternal/nat/README.md
**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant SDK, API reference, adapter, example, integration, and support documentation when the corresponding public surface changes.
Files:
external/README.mdexternal/nat/README.md
**/*.{html,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
HTML and Markdown files must use the specified SPDX HTML-comment header.
Files:
external/README.mdexternal/nat/README.md
{*.md,**/*.md,**/*.mdx,**/*.ipynb}
⚙️ CodeRabbit configuration file
{*.md,**/*.md,**/*.mdx,**/*.ipynb}: Enforce the product name in user-facing prose: use "NVIDIA NeMo Fabric" on first use and "NeMo Fabric" thereafter. Flag standalone capitalized "Fabric" when it refers to the product. Do not flag the lowercasefabricCLI command, package/import/crate names, code identifiers, API symbols, configuration keys, file paths, or unrelated generic uses of the word.
Files:
external/README.mdexternal/nat/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:
external/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/nat/fabric-adapter.jsontests/adapters/test_external_nat_adapter.pyexternal/nat/examples/calculator.pyexternal/nat/examples/email_phishing.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.py
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.Use
snake_casefor functions and variables; usePascalCasefor Rust types and Python classes.
Files:
external/nat/src/nemo_fabric_adapters/nat/__init__.pytests/adapters/test_external_nat_adapter.pyexternal/nat/examples/calculator.pyexternal/nat/examples/email_phishing.pyexternal/nat/src/nemo_fabric_adapters/nat/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.In Python SDK, adapters, examples, and tests, follow the existing style, use type annotations for public APIs, and keep native binding declarations synchronized with their Rust implementations.
Files:
external/nat/src/nemo_fabric_adapters/nat/__init__.pytests/adapters/test_external_nat_adapter.pyexternal/nat/examples/calculator.pyexternal/nat/examples/email_phishing.pyexternal/nat/src/nemo_fabric_adapters/nat/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:
external/nat/src/nemo_fabric_adapters/nat/__init__.pytests/adapters/test_external_nat_adapter.pyexternal/nat/examples/calculator.pyexternal/nat/examples/email_phishing.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.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.Public contract changes must keep native Python binding declarations synchronized with their Rust implementations.
Files:
external/nat/src/nemo_fabric_adapters/nat/__init__.pytests/adapters/test_external_nat_adapter.pyexternal/nat/examples/calculator.pyexternal/nat/examples/email_phishing.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.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:
external/nat/src/nemo_fabric_adapters/nat/__init__.pytests/adapters/test_external_nat_adapter.pyexternal/nat/examples/calculator.pyexternal/nat/examples/email_phishing.pyexternal/nat/src/nemo_fabric_adapters/nat/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:
external/nat/src/nemo_fabric_adapters/nat/__init__.pytests/adapters/test_external_nat_adapter.pyexternal/nat/examples/calculator.pyexternal/nat/examples/email_phishing.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.py
**/*.{json,jsonschema}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Public contract changes must keep checked-in JSON Schema snapshots synchronized.
Files:
external/nat/fabric-adapter.json
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_external_nat_adapter.py
tests/**/*.{rs,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the corresponding Rust crate or the relevant area under
tests/.
Files:
tests/adapters/test_external_nat_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 by the async runner.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, useunittest.mock.MagicMockorAsyncMock, using thespecargument when necessary, rather than defining a new class.
Prefix mocked class names withmock, notfake.
Prefer pytest fixtures over helper methods.
If a fixture is needed in multiple test files, define it once inconftest.pyrather than repeating it.
Define fixtures using@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and a<fixture_name>_fixturefunction; specifyscopeonly when it is notfunction.
Preferpytest.mark.parametrizeover separate tests for different input types.
Use@pytest.mark.usefixtureswhen a fixture is needed but its returned value is unused or it returns no value.
Avoid defensive programming in tests; access expected values directly so missing data raises a clear failure, such as usingresults["data"]instead ofresults.get("data").
When adapter installation metadata changes, packaging metadata tests must directly assert that the root project depends unconditionally on the exact-versionnemo-fabric-runtimedistribution.
Packaging metadata tests must verify that each root harness extra delegates to the matching version of the leaf adapter'sharnessextra.
Packaging metadata tests must verify that bare leaf dependencies remain adapter-owned and that the rootadapter-testsdependency group installs each leaf through itsharnessextra.
Packaging metadata tests must verify that every leaf providesfull; only adapters importing NeMo Relay Python APIs providerelay, while adapters using an external Relay executable havefullequal toharness.
Files:
tests/adapters/test_external_nat_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_external_nat_adapter.py
🧠 Learnings (1)
📚 Learning: 2026-06-29T22:34:52.407Z
Learnt from: AjayThorve
Repo: NVIDIA/NeMo-Fabric PR: 27
File: adapters/codex-cli/fabric-adapter.json:13-15
Timestamp: 2026-06-29T22:34:52.407Z
Learning: In NeMo-Fabric adapter manifest files (e.g., `*/fabric-adapter.json`), keep `config.accepts` limited to the top-level Fabric capability sections that `resolve_capability_plan` consumes (such as `models`, `tools`, `mcp`, `skills`, `telemetry`). Do not add adapter-owned `harness.settings` keys to `config.accepts`; `harness.settings` should remain adapter-owned and be passed through unchanged.
Applied to files:
external/nat/fabric-adapter.json
🪛 ast-grep (0.45.0)
tests/adapters/test_external_nat_adapter.py
[info] 647-647: use jsonify instead of json.dumps for JSON output
Context: json.dumps(result)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 670-670: use jsonify instead of json.dumps for JSON output
Context: json.dumps(result)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
external/nat/examples/calculator.py
[info] 81-81: use jsonify instead of json.dumps for JSON output
Context: json.dumps(output.to_mapping(), indent=2)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
external/nat/examples/email_phishing.py
[info] 84-84: use jsonify instead of json.dumps for JSON output
Context: json.dumps(output.to_mapping(), indent=2)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Ruff (0.16.1)
tests/adapters/test_external_nat_adapter.py
[warning] 170-170: Dynamically typed expressions (typing.Any) are disallowed in input_value
(ANN401)
external/nat/src/nemo_fabric_adapters/nat/adapter.py
[warning] 52-52: Dynamically typed expressions (typing.Any) are disallowed in **metadata
(ANN401)
[warning] 66-66: Dynamically typed expressions (typing.Any) are disallowed in value
(ANN401)
[warning] 123-123: Too many branches (13 > 12)
(PLR0912)
[warning] 236-236: Dynamically typed expressions (typing.Any) are disallowed in value
(ANN401)
[warning] 253-253: Dynamically typed expressions (typing.Any) are disallowed in server
(ANN401)
[warning] 496-496: Too many branches (15 > 12)
(PLR0912)
[warning] 653-653: Dynamically typed expressions (typing.Any) are disallowed in build_nat_config
(ANN401)
[warning] 679-679: Prefer TypeError exception for invalid type
(TRY004)
[warning] 679-679: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 691-691: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 696-696: Dynamically typed expressions (typing.Any) are disallowed in response
(ANN401)
[warning] 729-729: Do not catch blind exception: Exception
(BLE001)
[warning] 730-733: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
[warning] 805-809: Use a single with statement with multiple contexts instead of nested with statements
(SIM117)
[warning] 813-813: Do not catch blind exception: Exception
(BLE001)
[warning] 814-817: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
[warning] 828-831: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
🔇 Additional comments (23)
external/README.md (1)
1-19: LGTM!external/nat/src/nemo_fabric_adapters/nat/__init__.py (1)
1-4: LGTM!external/nat/examples/calculator.py (1)
1-86: LGTM!external/nat/examples/email_phishing.py (1)
1-89: LGTM!external/nat/src/nemo_fabric_adapters/nat/adapter.py (13)
1-43: LGTM!
52-113: LGTM!
116-192: LGTM!
207-250: LGTM!
253-304: LGTM!
307-377: LGTM!
380-428: LGTM!
431-487: LGTM!
600-650: LGTM!
696-733: LGTM!
736-822: LGTM!Also applies to: 838-858
193-201: 🔒 Security & PrivacyReachability path
● Entry external/nat/examples/calculator.py:68 main │ ▼ ● Sink external/nat/src/nemo_fabric_adapters/nat/adapter.pyKeep API keys out of lifecycle diagnostics. Ensure
nat_config_translation_failednever prints its chained cause to stderr; the cause may containllm["api_key"]. Redact secret-valued fields before validation if traceback output is enabled.
823-835: 🩺 Stability & AvailabilityKeep the existing exception handling.
PydanticSerializationErrorsubclassesValueErrorinpydantic-core2.46.4, so the current catch tuple already handles it.> Likely an incorrect or invalid review comment.tests/adapters/test_external_nat_adapter.py (6)
28-78: LGTM!
183-214: LGTM!
280-322: LGTM!
325-540: LGTM!
566-672: LGTM!
675-684: LGTM!
AnuradhaKaruppiah
left a comment
AnuradhaKaruppiah
left a comment
There was a problem hiding this comment.
Partial review only (read the PR to get an idea of the examples)
6fc7bd0 to
48c5924
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
Fern docs preview: https://nvidia-preview-pull-request-175.docs.buildwithfern.com/nemo/fabric |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
external/nat/fabric-adapter.json (1)
84-93: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
config.acceptsstill declares dotted paths instead of capability roots.
resolve_capability_planconsumes top-level capability sections. The dotted entriesmodels.base_url,models.temperature,instructions.system,tools.enabled,tools.blocked, andmcp.tool_filtersdo not declare theinstructionsandtoolssections that the adapter actually reads throughcommon_utils.system_instructionandcommon_utils.tools_config.Replace the list with the four capability roots. Update the matching assertion in
tests/adapters/test_external_nat_adapter.pyat Lines 209-218, which currently locks in the dotted list.Proposed fix
"config": { "accepts": [ "models", - "models.base_url", - "models.temperature", - "instructions.system", - "tools.enabled", - "tools.blocked", - "mcp", - "mcp.tool_filters" + "instructions", + "tools", + "mcp" ] },Based on learnings: keep
config.acceptslimited to the top-level Fabric capability sections thatresolve_capability_planconsumes; do not add adapter-owned or dotted keys.🤖 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 `@external/nat/fabric-adapter.json` around lines 84 - 93, Update config.accepts in the external NAT fabric adapter to contain only the four top-level capability roots consumed by resolve_capability_plan: models, instructions, tools, and mcp; remove all dotted entries. Update the corresponding assertion in test_external_nat_adapter.py to expect the same four roots.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 `@crates/fabric-core/src/config.rs`:
- Around line 1676-1708: Update the runtime startup validation in
validate_workflow/start_runtime to reapply base configuration checks before or
alongside adapter workflow-schema validation, rejecting whitespace-only
workflow.entrypoint.kind and workflow.entrypoint.ref values even under
permissive schemas. Add a regression test that mutates a valid planned
configuration with blank entrypoint values and verifies start_runtime fails.
---
Duplicate comments:
In `@external/nat/fabric-adapter.json`:
- Around line 84-93: Update config.accepts in the external NAT fabric adapter to
contain only the four top-level capability roots consumed by
resolve_capability_plan: models, instructions, tools, and mcp; remove all dotted
entries. Update the corresponding assertion in test_external_nat_adapter.py to
expect the same four roots.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 09b02b0c-c7e0-483a-852b-6b774024a73a
📒 Files selected for processing (60)
.agents/skills/contribute-adapter/SKILL.mdREADME.mdadapters/README.mdcrates/fabric-cli/src/presets.rscrates/fabric-cli/templates/rust/main.rs.tmplcrates/fabric-core/src/config.rscrates/fabric-core/src/error.rscrates/fabric-core/src/lib.rscrates/fabric-core/src/runtime.rscrates/fabric-core/src/schema.rsdocs/reference/api/python-library-reference/index.mddocs/reference/api/python-library-reference/nemo_fabric.models.mddocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-capabilitykind.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-capabilitytarget.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatifstorageconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatofmode.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatofsinkconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatofstreamfieldnamepolicy.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatofstreamtransport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayotlptransport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayunsupportedbehavior.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/fn-load-adapter-descriptor.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/fn-resolve-run-plan-from-config.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterdescriptor.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-fabricconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-workflowconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-workflowentrypointconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/enum-doctorstatus.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/fn-doctor-plan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/struct-doctorcheck.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/struct-doctorreport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/error/enum-fabricerror.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/error/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/error/type-result.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/fn-version.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/runtime/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/schema/index.mdxdocs/sdk/python.mdxexternal/README.mdexternal/nat/README.mdexternal/nat/examples/calculator.pyexternal/nat/examples/email_phishing.pyexternal/nat/fabric-adapter.jsonexternal/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.pypython/src/nemo_fabric/__init__.pypython/src/nemo_fabric/models.pypython/src/nemo_fabric/types.pyschemas/SCHEMA.mdschemas/adapter-descriptor.schema.jsonschemas/agent.schema.jsonschemas/run-plan.schema.jsonskills/integrations/consumer/nemo-fabric-integrate/SKILL.mdskills/integrations/consumer/nemo-fabric-integrate/references/config-mapping.mdtests/adapters/test_external_nat_adapter.pytests/python/test_sdk_contract.pytests/python/test_workflow_validation.py
48c5924 to
89ab2dd
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@external/nat/README.md`:
- Line 24: Update the user-facing product references in README.md to use “NeMo
Fabric” instead of standalone “Fabric,” including the model-role wording on the
table row and the MCP/server policy/rejection statements identified in the
review. Preserve lowercase `fabric` references when they denote the CLI tool,
and keep the existing first-use “NVIDIA NeMo Fabric” wording unchanged.
In `@tests/python/test_workflow_validation.py`:
- Around line 89-137: Add a test alongside
test_workflow_schema_reports_exact_invalid_setting_path that starts the runtime
rather than only calling Fabric().plan, using a workflow with an invalid
workflow.settings.llm_name value and asserting FabricConfigError exposes the
exact workflow.settings.llm_name path. Ensure the runtime setup is cleaned up
appropriately while verifying descriptor workflow_schema validation is enforced
again at startup.
- Around line 112-120: Refactor
test_workflow_schema_controls_whether_workflow_is_required to use
pytest.mark.parametrize for the required and optional schema shapes, expected
validation outcome, and workflow value. Ensure each parameterized case
independently writes its descriptor and verifies either FabricConfigError or a
successful plan with config.workflow set to None.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: da414147-1f62-451c-bd78-5fb3c4ce7083
📒 Files selected for processing (60)
.agents/skills/contribute-adapter/SKILL.mdREADME.mdadapters/README.mdcrates/fabric-cli/src/presets.rscrates/fabric-cli/templates/rust/main.rs.tmplcrates/fabric-core/src/config.rscrates/fabric-core/src/error.rscrates/fabric-core/src/lib.rscrates/fabric-core/src/runtime.rscrates/fabric-core/src/schema.rsdocs/reference/api/python-library-reference/index.mddocs/reference/api/python-library-reference/nemo_fabric.models.mddocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-capabilitykind.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-capabilitytarget.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatifstorageconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatofmode.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatofsinkconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatofstreamfieldnamepolicy.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatofstreamtransport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayotlptransport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayunsupportedbehavior.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/fn-load-adapter-descriptor.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/fn-resolve-run-plan-from-config.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterdescriptor.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-fabricconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-workflowconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-workflowentrypointconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/enum-doctorstatus.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/fn-doctor-plan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/struct-doctorcheck.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/struct-doctorreport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/error/enum-fabricerror.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/error/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/error/type-result.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/fn-version.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/runtime/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/schema/index.mdxdocs/sdk/python.mdxexternal/README.mdexternal/nat/README.mdexternal/nat/examples/calculator.pyexternal/nat/examples/email_phishing.pyexternal/nat/fabric-adapter.jsonexternal/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.pypython/src/nemo_fabric/__init__.pypython/src/nemo_fabric/models.pypython/src/nemo_fabric/types.pyschemas/SCHEMA.mdschemas/adapter-descriptor.schema.jsonschemas/agent.schema.jsonschemas/run-plan.schema.jsonskills/integrations/consumer/nemo-fabric-integrate/SKILL.mdskills/integrations/consumer/nemo-fabric-integrate/references/config-mapping.mdtests/adapters/test_external_nat_adapter.pytests/python/test_sdk_contract.pytests/python/test_workflow_validation.py
#### Overview Add an optional, typed `FabricConfig.workflow` contract for adapters that expose selectable executables or custom agents. A workflow contains an adapter-defined `entrypoint.kind`, `entrypoint.ref`, and immutable construction `settings`. Adapters opt in with a descriptor-owned `workflow_schema`. NeMo Fabric validates the complete workflow during planning and again before runtime startup. `workflow_schema` is the single support and shape claim; adapters without it reject configured workflows instead of silently ignoring them. This is the prerequisite contract for the external NAT reference adapter in #175. No bundled adapter opts into workflows in this PR. #### Details - Add Rust and Python authoring/plan-snapshot types for workflow configuration. - Add `workflow_schema` to `fabric-adapter.json` descriptors and generated schemas. - Preserve additive extension fields at the workflow and entrypoint levels. - Support adapter defaults only when the descriptor schema explicitly accepts `null`; schema defaults are not applied. - Report descriptor context and canonical `workflow...` paths on validation failures. - Update generated API references, SDK guidance, adapter guidance, and the exported consumer integration skill. #### Breaking Changes Existing serialized JSON and Python configurations remain valid because `workflow` and `workflow_schema` are optional. Downstream Rust code that constructs `FabricConfig` or `AdapterDescriptor` with struct literals must initialize the new optional field (normally `None`). #### Validation - `RUSTUP_TOOLCHAIN=1.94.0 just test-rust` — 55 core tests plus CLI, scaffold, and doc tests passed. - `RUSTUP_TOOLCHAIN=1.94.0 just test-python` — 602 passed, 15 skipped. - `just build-python` - `cargo check -p fabric-python --locked` - `RUSTUP_TOOLCHAIN=1.94.0 just docs` - `uv run pre-commit run --all-files --show-diff-on-failure` - `cargo fmt --all -- --check` - `git diff --check` #### Where should the reviewer start? Start with `WorkflowConfig`, `WorkflowEntrypointConfig`, and `validate_workflow` in `crates/fabric-core/src/config.rs`, then review the Python parity in `python/src/nemo_fabric/models.py` and the fail-closed planning coverage in `tests/python/test_workflow_validation.py`. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) Relates to #175 - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. ## Summary by CodeRabbit - **New Features** - Added optional workflow configuration for selecting adapter-provided executables and entry points. - Added workflow settings support with adapter-defined schema validation. - Added workflow configuration models to the Python and Rust APIs. - Added clear validation errors for unsupported or invalid workflow settings. - **Documentation** - Updated quick-start, SDK, schema, adapter, and API references with workflow configuration guidance. - **Tests** - Added coverage for serialization, round-tripping, valid workflows, missing schemas, and invalid configuration. Authors: - Ajay Thorve (https://github.com/AjayThorve) Approvers: - Anuradha Karuppiah (https://github.com/AnuradhaKaruppiah) URL: #176
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
89ab2dd to
16c360c
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
external/nat/fabric-adapter.json (1)
84-93: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse capability roots in
config.accepts.
resolve_capability_planconsumes top-level capability sections. The dotted entries such asmodels.base_urlandinstructions.systemdo not declare support for theinstructionsandtoolssections. The README documents both mappings at Lines 25 and 32, and the adapter implements them.Replace the dotted entries with the capability roots.
Proposed fix
"config": { "accepts": [ "models", - "models.base_url", - "models.temperature", - "instructions.system", - "tools.enabled", - "tools.blocked", - "mcp", - "mcp.tool_filters" + "instructions", + "tools", + "mcp" ] },Update the matching assertion in
tests/adapters/test_external_nat_adapter.pyat Lines 209-218.Based on learnings: "keep
config.acceptslimited to the top-level Fabric capability sections thatresolve_capability_planconsumes".🤖 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 `@external/nat/fabric-adapter.json` around lines 84 - 93, Update the config.accepts list in the Fabric adapter to use only the top-level capability roots consumed by resolve_capability_plan, replacing dotted entries such as models.base_url and instructions.system with their corresponding roots. Adjust the matching assertion in test_external_nat_adapter.py to expect the revised capability list, while preserving the existing supported sections.Source: Learnings
tests/adapters/test_external_nat_adapter.py (1)
22-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the source path into pytest configuration.
sys.path.insertat Line 24 runs at import time and persists for the whole pytest process. It also forces the out-of-order import at Line 26 and the# noqa: E402. Addexternal/nat/srcto[tool.pytest.ini_options].pythonpath, or set it intests/adapters/conftest.py, then remove Lines 22-24 and thenoqa.🤖 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_external_nat_adapter.py` around lines 22 - 26, Move the NAT adapter source-path setup from the test module into pytest configuration by adding external/nat/src to tool.pytest.ini_options.pythonpath or configuring it in tests/adapters/conftest.py. Remove ROOT, NAT_ADAPTER_SOURCE, and sys.path.insert from the test, then import nemo_fabric_adapters.nat.adapter normally without the noqa E402 suppression.
🤖 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 `@external/README.md`:
- Around line 17-19: Add a complete lead-in sentence immediately before the
single-row harness/adapter/reference table in the README, briefly stating what
the table lists; keep the existing table content unchanged.
In `@tests/adapters/test_external_nat_adapter.py`:
- Around line 860-878: Add tests for the untested normalized lifecycle errors:
configure mock_nat["sessions"].shutdown to fail, then verify runtime.stop raises
LifecycleError with code nat_runtime_stop_failed, clears runtime state, and
becomes a no-op on the second call; configure
mock_nat["config_type"].model_validate to raise a non-LifecycleError exception,
then verify build_nat_config raises nat_config_translation_failed without
exposing the original message.
---
Duplicate comments:
In `@external/nat/fabric-adapter.json`:
- Around line 84-93: Update the config.accepts list in the Fabric adapter to use
only the top-level capability roots consumed by resolve_capability_plan,
replacing dotted entries such as models.base_url and instructions.system with
their corresponding roots. Adjust the matching assertion in
test_external_nat_adapter.py to expect the revised capability list, while
preserving the existing supported sections.
In `@tests/adapters/test_external_nat_adapter.py`:
- Around line 22-26: Move the NAT adapter source-path setup from the test module
into pytest configuration by adding external/nat/src to
tool.pytest.ini_options.pythonpath or configuring it in
tests/adapters/conftest.py. Remove ROOT, NAT_ADAPTER_SOURCE, and sys.path.insert
from the test, then import nemo_fabric_adapters.nat.adapter normally without the
noqa E402 suppression.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 9571a73a-d287-4c97-aae2-46ca97fa29a7
📒 Files selected for processing (9)
external/README.mdexternal/nat/README.mdexternal/nat/examples/calculator.pyexternal/nat/examples/calculator_mcp.pyexternal/nat/examples/email_phishing.pyexternal/nat/fabric-adapter.jsonexternal/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.pytests/adapters/test_external_nat_adapter.py
📜 Review details
⏰ Context from checks skipped due to timeout. (17)
- GitHub Check: Test (Python 3.14, linux-arm64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.11, linux-arm64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.14, linux-amd64)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.13, linux-arm64)
- GitHub Check: Test (Python 3.12, linux-arm64)
- GitHub Check: Test (Python 3.13, linux-amd64)
- GitHub Check: Test (Python 3.11, linux-amd64)
- GitHub Check: Test (Python 3.14, macos-arm64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Test (Python 3.12, linux-amd64)
- GitHub Check: Pre-commit
🧰 Additional context used
📓 Path-based instructions (22)
**/*.{rs,py,pyi,json,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Determine and update every affected public surface, including the CLI, PyO3 bindings, Python SDK, type stubs, schemas, and adapter contract, so they remain in parity.
Files:
external/nat/examples/calculator_mcp.pyexternal/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/nat/examples/calculator.pytests/adapters/test_external_nat_adapter.pyexternal/nat/fabric-adapter.jsonexternal/nat/src/nemo_fabric_adapters/nat/adapter.pyexternal/nat/examples/email_phishing.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:
external/nat/examples/calculator_mcp.pyexternal/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/README.mdexternal/nat/README.mdexternal/nat/examples/calculator.pytests/adapters/test_external_nat_adapter.pyexternal/nat/fabric-adapter.jsonexternal/nat/src/nemo_fabric_adapters/nat/adapter.pyexternal/nat/examples/email_phishing.py
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.Use
snake_casefor functions and variables; usePascalCasefor Rust types and Python classes.
Files:
external/nat/examples/calculator_mcp.pyexternal/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/nat/examples/calculator.pytests/adapters/test_external_nat_adapter.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.pyexternal/nat/examples/email_phishing.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.In Python SDK, adapters, examples, and tests, follow the existing style, use type annotations for public APIs, and keep native binding declarations synchronized with their Rust implementations.
Files:
external/nat/examples/calculator_mcp.pyexternal/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/nat/examples/calculator.pytests/adapters/test_external_nat_adapter.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.pyexternal/nat/examples/email_phishing.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:
external/nat/examples/calculator_mcp.pyexternal/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/nat/examples/calculator.pytests/adapters/test_external_nat_adapter.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.pyexternal/nat/examples/email_phishing.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.Public contract changes must keep native Python binding declarations synchronized with their Rust implementations.
Files:
external/nat/examples/calculator_mcp.pyexternal/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/nat/examples/calculator.pytests/adapters/test_external_nat_adapter.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.pyexternal/nat/examples/email_phishing.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:
external/nat/examples/calculator_mcp.pyexternal/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/nat/examples/calculator.pytests/adapters/test_external_nat_adapter.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.pyexternal/nat/examples/email_phishing.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:
external/nat/examples/calculator_mcp.pyexternal/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/nat/examples/calculator.pytests/adapters/test_external_nat_adapter.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.pyexternal/nat/examples/email_phishing.py
**/*.{md,mdx,yml,py,rs,sh}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
Keep documentation aligned with current NeMo Fabric behavior, repository layout, entry points, commands, package names, APIs, bindings, and support claims.
Files:
external/nat/examples/calculator_mcp.pyexternal/nat/src/nemo_fabric_adapters/nat/__init__.pyexternal/README.mdexternal/nat/README.mdexternal/nat/examples/calculator.pytests/adapters/test_external_nat_adapter.pyexternal/nat/src/nemo_fabric_adapters/nat/adapter.pyexternal/nat/examples/email_phishing.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:
external/README.mdexternal/nat/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:
external/README.mdexternal/nat/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:
external/README.mdexternal/nat/README.md
**/*.{md,mdx}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
**/*.{md,mdx}: Use the full product nameNVIDIA NeMo Fabricon first use, typically in the title and H1; useNeMo Fabricthereafter. Usefabricalone only for the CLI tool and surround it with backticks.
Treat incorrect or stale commands, package names, paths, APIs, support claims, procedures, examples, terminology, or public behavior documentation as blocking issues.
CapitalizeNVIDIAcorrectly and format code, commands, paths, and filenames as inline code where needed.
Use title case for technical-documentation headings.
Introduce code blocks, tables, and lists with complete lead-in sentences; ensure examples match current APIs and build commands.
Use descriptive anchor text, avoid raw URLs and generic labels such ashere, and use repository-relative.mdxpaths for links withindocs/.
Prefer active voice, present tense, short sentences, plain English, consistent terminology, and imperative, parallel, scannable procedures.
Useafterinstead ofoncewhen expressing temporal sequence, and usecanrather thanmaywhen describing possibility rather than permission.
Avoid ambiguous numeric dates and ordinal dates in body text.
For learning-oriented documentation, do not force trademark symbols unless the source document explicitly requires them.
When reporting documentation-review findings, lead withMust fix,Should fix, andNice to havecategories; include file path, line reference, current problem, rationale, and a concrete rewrite or direction.
Files:
external/README.mdexternal/nat/README.md
**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant SDK, API reference, adapter, example, integration, and support documentation when the corresponding public surface changes.
Files:
external/README.mdexternal/nat/README.md
**/*.{html,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
HTML and Markdown files must use the specified SPDX HTML-comment header.
Files:
external/README.mdexternal/nat/README.md
{*.md,**/*.md,**/*.mdx,**/*.ipynb}
⚙️ CodeRabbit configuration file
{*.md,**/*.md,**/*.mdx,**/*.ipynb}: Enforce the product name in user-facing prose: use "NVIDIA NeMo Fabric" on first use and "NeMo Fabric" thereafter. Flag standalone capitalized "Fabric" when it refers to the product. Do not flag the lowercasefabricCLI command, package/import/crate names, code identifiers, API symbols, configuration keys, file paths, or unrelated generic uses of the word.
Files:
external/README.mdexternal/nat/README.md
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_external_nat_adapter.py
tests/**/*.{rs,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the corresponding Rust crate or the relevant area under
tests/.
Files:
tests/adapters/test_external_nat_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 by the async runner.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, useunittest.mock.MagicMockorAsyncMock, using thespecargument when necessary, rather than defining a new class.
Prefix mocked class names withmock, notfake.
Prefer pytest fixtures over helper methods.
If a fixture is needed in multiple test files, define it once inconftest.pyrather than repeating it.
Define fixtures using@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and a<fixture_name>_fixturefunction; specifyscopeonly when it is notfunction.
Preferpytest.mark.parametrizeover separate tests for different input types.
Use@pytest.mark.usefixtureswhen a fixture is needed but its returned value is unused or it returns no value.
Avoid defensive programming in tests; access expected values directly so missing data raises a clear failure, such as usingresults["data"]instead ofresults.get("data").
When adapter installation metadata changes, packaging metadata tests must directly assert that the root project depends unconditionally on the exact-versionnemo-fabric-runtimedistribution.
Packaging metadata tests must verify that each root harness extra delegates to the matching version of the leaf adapter'sharnessextra.
Packaging metadata tests must verify that bare leaf dependencies remain adapter-owned and that the rootadapter-testsdependency group installs each leaf through itsharnessextra.
Packaging metadata tests must verify that every leaf providesfull; only adapters importing NeMo Relay Python APIs providerelay, while adapters using an external Relay executable havefullequal toharness.
Files:
tests/adapters/test_external_nat_adapter.py
tests/adapters/test_*.py
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
tests/adapters/test_*.py: Add focused tests for descriptor shape and exact capabilities, positive normalized-surface mappings, rejection of unsupported values and unenforceable policies, result normalization without secret leakage, one-shot execution, continuation, runtime isolation, and the packaged subprocess entry point.
Provide a credential-free fixture coveringplan,doctor, andrun; keep credential-dependent live-harness tests opt-in while retaining a deterministic CI end-to-end path.
Files:
tests/adapters/test_external_nat_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_external_nat_adapter.py
**/*.{json,jsonschema}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Public contract changes must keep checked-in JSON Schema snapshots synchronized.
Files:
external/nat/fabric-adapter.json
🧠 Learnings (1)
📚 Learning: 2026-06-29T22:34:52.407Z
Learnt from: AjayThorve
Repo: NVIDIA/NeMo-Fabric PR: 27
File: adapters/codex-cli/fabric-adapter.json:13-15
Timestamp: 2026-06-29T22:34:52.407Z
Learning: In NeMo-Fabric adapter manifest files (e.g., `*/fabric-adapter.json`), keep `config.accepts` limited to the top-level Fabric capability sections that `resolve_capability_plan` consumes (such as `models`, `tools`, `mcp`, `skills`, `telemetry`). Do not add adapter-owned `harness.settings` keys to `config.accepts`; `harness.settings` should remain adapter-owned and be passed through unchanged.
Applied to files:
external/nat/fabric-adapter.json
🪛 ast-grep (0.45.0)
external/nat/examples/calculator.py
[info] 85-85: use jsonify instead of json.dumps for JSON output
Context: json.dumps(output.to_mapping(), indent=2)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
tests/adapters/test_external_nat_adapter.py
[info] 904-904: use jsonify instead of json.dumps for JSON output
Context: json.dumps(result)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 928-928: use jsonify instead of json.dumps for JSON output
Context: json.dumps(result)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
external/nat/examples/email_phishing.py
[info] 90-90: use jsonify instead of json.dumps for JSON output
Context: json.dumps(output.to_mapping(), indent=2)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Ruff (0.16.1)
external/nat/examples/calculator_mcp.py
[warning] 39-39: Avoid specifying long messages outside the exception class
(TRY003)
tests/adapters/test_external_nat_adapter.py
[warning] 33-33: Dynamically typed expressions (typing.Any) are disallowed in **settings
(ANN401)
[warning] 180-180: Dynamically typed expressions (typing.Any) are disallowed in input_value
(ANN401)
[warning] 182-182: Dynamically typed expressions (typing.Any) are disallowed in context
(ANN401)
[warning] 183-183: Dynamically typed expressions (typing.Any) are disallowed in raw_request
(ANN401)
external/nat/src/nemo_fabric_adapters/nat/adapter.py
[warning] 59-59: Dynamically typed expressions (typing.Any) are disallowed in **metadata
(ANN401)
[warning] 73-73: Dynamically typed expressions (typing.Any) are disallowed in value
(ANN401)
[warning] 176-176: Too many branches (13 > 12)
(PLR0912)
[warning] 288-288: Dynamically typed expressions (typing.Any) are disallowed in value
(ANN401)
[warning] 305-305: Dynamically typed expressions (typing.Any) are disallowed in server
(ANN401)
[warning] 548-548: Too many branches (14 > 12)
(PLR0912)
[warning] 702-702: Dynamically typed expressions (typing.Any) are disallowed in build_nat_config
(ANN401)
[warning] 730-730: Prefer TypeError exception for invalid type
(TRY004)
[warning] 730-730: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 742-744: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 749-749: Dynamically typed expressions (typing.Any) are disallowed in response
(ANN401)
[warning] 782-782: Do not catch blind exception: Exception
(BLE001)
[warning] 783-786: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
[warning] 862-866: Use a single with statement with multiple contexts instead of nested with statements
(SIM117)
[warning] 870-870: Do not catch blind exception: Exception
(BLE001)
[warning] 871-874: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
[warning] 885-888: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
🔇 Additional comments (20)
external/nat/examples/calculator.py (1)
1-4: LGTM!Also applies to: 27-69, 72-90
external/nat/examples/calculator_mcp.py (1)
1-44: LGTM!external/nat/examples/email_phishing.py (1)
1-4: LGTM!Also applies to: 26-71, 74-95
external/nat/README.md (1)
6-118: LGTM!external/nat/src/nemo_fabric_adapters/nat/__init__.py (1)
1-4: LGTM!external/nat/src/nemo_fabric_adapters/nat/adapter.py (15)
59-80: LGTM!
83-137: LGTM!
140-166: LGTM!
169-257: LGTM!
264-285: LGTM!
288-302: LGTM!
305-356: LGTM!
359-429: LGTM!
432-480: LGTM!
483-539: LGTM!
542-603: LGTM!
606-646: LGTM!
649-722: LGTM!
725-774: LGTM!
777-911: LGTM!
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
There was a problem hiding this comment.
Thanks Ajay!! For the env and arg thing, I also opened another PR -178 to address this if you also think this is an issue, I think this PR is mergeable imo
|
/merge |
#### Overview Fix NAT stdio MCP configuration so the structured `args` and `env` fields in the capability plan are forwarded to NAT instead of being silently dropped. Document the resulting command, argument, and environment mapping in the NAT adapter README. The NAT adapter is now on `main` through #175; this draft contains the focused fix, regression coverage, and its adapter documentation. - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details - Appends structured MCP arguments to any legacy arguments parsed from the stdio command. - Preserves configured process environment values in the NAT MCP server mapping. - Documents argument ordering, environment forwarding, and URL-only variable expansion. - Extends the stdio MCP regression test to cover inline arguments, structured arguments, and environment variables together. #### Validation - `uv run --no-sync pytest -q tests/adapters/test_external_nat_adapter.py` — 52 passed, 2 skipped after rebasing. - `pre-commit run --files external/nat/README.md`. - `just test-python` — full Python suite passed before rebasing; the rebase introduced no code conflicts. - `uv run --with ruff ruff check ...`. - `git diff --check`. #### Where should the reviewer start? Start with `nat_mcp_server_config` in `external/nat/src/nemo_fabric_adapters/nat/adapter.py`; the README directly below the MCP routing section and the regression coverage in `tests/adapters/test_external_nat_adapter.py` explain and test the behavior. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to #175 ## Summary by CodeRabbit * **New Features** * Added NeMo Fabric integration for configuring and running NAT workflows. * Supports models, tools, MCP servers, function groups, system instructions, and tool policies. * Added asynchronous workflow execution with persistent runtime and session management. * MCP stdio servers now support command arguments and environment variables. * **Reliability** * Added validation for configurations, runtime requests, invocations, and workflow results. * Errors are reported in a structured, non-retryable format with sensitive values redacted. * **Documentation** * Documented MCP stdio URL expansion, argument handling, and environment forwarding. Authors: - Zhongxuan (Daniel) Wang (https://github.com/zhongxuanwang-nv) Approvers: - Ajay Thorve (https://github.com/AjayThorve) URL: #178
3 participants
Overview
Adds a source-only NeMo Agent Toolkit (NAT) adapter under
external/as the first third-party reference implementation of the public NeMo Fabric adapter contract.The adapter translates typed
FabricConfiginput into an in-memory NATConfig, loads installed NAT components, and owns one persistentWorkflowBuilderandSessionManageracrossstart, repeatedinvoke, andstop. This PR intentionally does not add wheel metadata, bundled catalog entries, or installed-adapter discovery wiring.The typed workflow contract is already available on
mainthrough #176. This PR now contains only the external NAT reference adapter, its examples, and focused tests.Details
nvidia.fabric.natdescriptor and generic Python lifecycle runner.workflow.entrypointand mapsworkflow.settingsinto the selected NAT component; no NAT workflow YAML orconfig_filepath is used.workflow_schemaas the single workflow support and validation claim.harness.settingsis limited to registered NATfunctionsandfunction_groups.custom/react_agentis not mutated.allowed_toolsandblocked_toolsinto NATincludeorexcludepolicy. An explicit empty allowlist fails closed by omitting the generated group because NAT interpretsinclude=[]as unfiltered.tools.enabledandtools.blockedacross NAT functions, function groups, and<group>__<member>selectors.nat.componentsregistrations.Validation
uv run --no-sync pytest -q tests/adapters/test_external_nat_adapter.py— 52 passed, 2 optional real-NAT tests skipped when NAT is not installed.just test-python— 652 passed, 17 skipped.add,subtract,multiply, anddivide, and returned42.0formultiply(21, 2).Configvalidation passed for bothreact_agentandnat.plugins.langchain.agent.react_agent/react_agent.git diff --checkpassed.Live model invocation was not exercised because it requires credentials.
Where should the reviewer start?
Start with
_nat_workflow,_is_react_agent, andbuild_nat_config_mappinginexternal/nat/src/nemo_fabric_adapters/nat/adapter.py. Then reviewworkflow_schemainexternal/nat/fabric-adapter.jsonand the root-workflow/example coverage intests/adapters/test_external_nat_adapter.py.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Relates to FABRIC-117
Relates to feat: add typed workflow configuration #176
I confirm this contribution is my own work, or I have the right to submit it under this project's license.
I searched existing issues and open pull requests, and this does not duplicate existing work.