feat: add typed workflow configuration - #176
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (52)
WalkthroughThe change adds adapter-owned workflow configuration across Rust, Python, and JSON schemas. Planning and runtime startup validate workflows against adapter descriptors. SDK models, errors, tests, and documentation now expose the workflow contract. ChangesWorkflow configuration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant FabricConfig
participant resolve_run_plan_from_config
participant AdapterDescriptor
participant validate_workflow
participant start_runtime
FabricConfig->>resolve_run_plan_from_config: Submit workflow configuration
resolve_run_plan_from_config->>AdapterDescriptor: Resolve selected descriptor
resolve_run_plan_from_config->>validate_workflow: Validate against workflow_schema
validate_workflow-->>resolve_run_plan_from_config: Return valid workflow or InvalidWorkflow
start_runtime->>validate_workflow: Validate workflow before startup
validate_workflow-->>start_runtime: Return validation result
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Fern docs preview: https://nvidia-preview-pull-request-176.docs.buildwithfern.com/nemo/fabric |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/python/test_sdk_contract.py (1)
79-85: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReplace
Anyin the variadic keyword annotation.Ruff reports
ANN401for**settings: Any. Useobjector a concrete JSON-value type so this test helper passes the configured lint rules while still accepting the invalid integer test value.🤖 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/python/test_sdk_contract.py` around lines 79 - 85, Update the variadic keyword annotation in the test helper used by FabricConfig.from_mapping to replace Any with object or an appropriate JSON-value type; retain support for the invalid integer setting exercised by the test.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@adapters/README.md`:
- Around line 30-35: Update the “Complete FabricConfig Support” compatibility
table in adapters/README.md to add a workflow row covering workflow.entrypoint
and workflow.settings, marking both unsupported for Claude, Codex, Deep Agents,
and Hermes Agent, consistent with docs/sdk/python.mdx.
In `@crates/fabric-core/src/runtime.rs`:
- Line 544: Update start_runtime to validate the deserialized RunPlan
configuration at the runtime boundary by calling validate_config before
prepare_environment, while retaining validate_workflow for adapter-specific
checks. Ensure blank workflow.entrypoint.kind or workflow.entrypoint.ref values
are rejected before environment preparation.
In `@python/src/nemo_fabric/models.py`:
- Around line 124-130: Update WorkflowConfig serialization so an empty settings
mapping is excluded from the emitted workflow mapping, matching Rust behavior
while preserving non-empty settings. Extend the relevant Python/Rust parity test
coverage for WorkflowConfig.to_mapping() to verify both omission when settings
is empty and retention when populated.
---
Outside diff comments:
In `@tests/python/test_sdk_contract.py`:
- Around line 79-85: Update the variadic keyword annotation in the test helper
used by FabricConfig.from_mapping to replace Any with object or an appropriate
JSON-value type; retain support for the invalid integer setting exercised by the
test.
🪄 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: 409501fd-69a3-4b7c-a03c-71f1054eca62
📒 Files selected for processing (52)
.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.mdxpython/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/python/test_sdk_contract.pytests/python/test_workflow_validation.py
2 tasks
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
AjayThorve
force-pushed
the
topic/custom_agents
branch
from
August 5, 2026 22:13
5bf0401 to
9959d9f
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. |
Collaborator
Author
|
/merge |
This was referenced Aug 5, 2026
rapids-bot Bot
pushed a commit
that referenced
this pull request
Aug 6, 2026
#### 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 `FabricConfig` input into an in-memory NAT `Config`, loads installed NAT components, and owns one persistent `WorkflowBuilder` and `SessionManager` across `start`, repeated `invoke`, and `stop`. This PR intentionally does not add wheel metadata, bundled catalog entries, or installed-adapter discovery wiring. The typed workflow contract is already available on `main` through #176. This PR now contains only the external NAT reference adapter, its examples, and focused tests. #### Details - Adds the `nvidia.fabric.nat` descriptor and generic Python lifecycle runner. - Selects NAT workflows through root `workflow.entrypoint` and maps `workflow.settings` into the selected NAT component; no NAT workflow YAML or `config_file` path is used. - Uses descriptor `workflow_schema` as the single workflow support and validation claim. `harness.settings` is limited to registered NAT `functions` and `function_groups`. - Maps normalized models and ReAct system instructions. Built-in ReAct behavior is applied only to NAT's short and canonical qualified ReAct references, so a third-party `custom/react_agent` is not mutated. - Maps per-server MCP `allowed_tools` and `blocked_tools` into NAT `include` or `exclude` policy. An explicit empty allowlist fails closed by omitting the generated group because NAT interprets `include=[]` as unfiltered. - Applies root `tools.enabled` and `tools.blocked` across NAT functions, function groups, and `<group>__<member>` selectors. - Loads custom workflows, functions, and function groups through installed NAT `nat.components` registrations. - Adds typed calculator-MCP and NAT-native email-phishing examples using the same root workflow contract. The calculator example includes a source-only stdio MCP server and requires no separately managed endpoint. - Normalizes errors without exposing underlying exception text in NeMo Fabric results or stderr artifacts. #### 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. - A direct MCP stdio protocol check discovered `add`, `subtract`, `multiply`, and `divide`, and returned `42.0` for `multiply(21, 2)`. - Real NAT 1.8 typed-`Config` validation passed for both `react_agent` and `nat.plugins.langchain.agent.react_agent/react_agent`. - Both typed examples produce current NeMo Fabric plans against the external descriptor. - Changed-file pre-commit hooks and `git diff --check` passed. Live model invocation was not exercised because it requires credentials. #### Where should the reviewer start? Start with `_nat_workflow`, `_is_react_agent`, and `build_nat_config_mapping` in `external/nat/src/nemo_fabric_adapters/nat/adapter.py`. Then review `workflow_schema` in `external/nat/fabric-adapter.json` and the root-workflow/example coverage in `tests/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 #176 - [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. Authors: - Ajay Thorve (https://github.com/AjayThorve) Approvers: - Zhongxuan (Daniel) Wang (https://github.com/zhongxuanwang-nv) URL: #175
This was referenced Aug 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Add an optional, typed
FabricConfig.workflowcontract for adapters that expose selectable executables or custom agents. A workflow contains an adapter-definedentrypoint.kind,entrypoint.ref, and immutable constructionsettings.Adapters opt in with a descriptor-owned
workflow_schema. NeMo Fabric validates the complete workflow during planning and again before runtime startup.workflow_schemais 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
workflow_schematofabric-adapter.jsondescriptors and generated schemas.null; schema defaults are not applied.workflow...paths on validation failures.Breaking Changes
Existing serialized JSON and Python configurations remain valid because
workflowandworkflow_schemaare optional. Downstream Rust code that constructsFabricConfigorAdapterDescriptorwith struct literals must initialize the new optional field (normallyNone).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-pythoncargo check -p fabric-python --lockedRUSTUP_TOOLCHAIN=1.94.0 just docsuv run pre-commit run --all-files --show-diff-on-failurecargo fmt --all -- --checkgit diff --checkWhere should the reviewer start?
Start with
WorkflowConfig,WorkflowEntrypointConfig, andvalidate_workflowincrates/fabric-core/src/config.rs, then review the Python parity inpython/src/nemo_fabric/models.pyand the fail-closed planning coverage intests/python/test_workflow_validation.py.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Relates to #175
Summary by CodeRabbit
New Features
Documentation
Tests