feat(nemo-agents): adding agent.yaml to FabricConfig translation - #701
Merged
mmogallapalli merged 10 commits intoJul 16, 2026
Conversation
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Contributor
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds a strict ChangesAgent configuration and Fabric execution
Sequence Diagram(s)sequenceDiagram
participant AgentConfigLoader
participant translate_agent_config
participant FabricConfig
participant validate_fabric_config
AgentConfigLoader->>translate_agent_config: validated AgentConfig
translate_agent_config->>FabricConfig: translated runtime settings
translate_agent_config->>validate_fabric_config: FabricConfig and base directory
validate_fabric_config->>FabricConfig: plan and doctor execution
FabricConfig-->>validate_fabric_config: plan and doctor report
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
plugins/nemo-agents/tests/unit/test_fabric_validation.py (1)
73-145: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace hardcoded
"/tmp/agent"with thetmp_pathfixture.Static analysis flags this pattern 9x (CWE-377). No real filesystem access happens here, but
tmp_pathis already the idiomatic pytest fixture for this and avoids repeat flags.♻️ Example fix
class TestValidateFabricConfig: - async def test_returns_plan_and_doctor_report(self, fake_nemo_fabric: None) -> None: + async def test_returns_plan_and_doctor_report(self, fake_nemo_fabric: None, tmp_path: Path) -> None: ... - result = await validate_fabric_config(fabric_config, base_dir=Path("/tmp/agent"), fabric=fabric) + result = await validate_fabric_config(fabric_config, base_dir=tmp_path, fabric=fabric)(apply the same substitution at each remaining
Path("/tmp/agent")occurrence)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/nemo-agents/tests/unit/test_fabric_validation.py` around lines 73 - 145, Replace every hardcoded Path("/tmp/agent") in the affected tests around validate_fabric_config with pytest’s tmp_path fixture, updating each test signature to accept tmp_path and passing it directly as base_dir. Preserve the existing assertions and validation behavior while applying the substitution consistently to all occurrences.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 `@plugins/nemo-agents/src/nemo_agents_plugin/fabric/validation.py`:
- Around line 54-62: Update the async validation flow containing
fabric_client.plan and fabric_client.doctor to run the synchronous plan call off
the event loop via the project’s async thread-offloading mechanism, while
preserving FabricConfigError translation to FabricValidationError. Wrap the
doctor await in the established async timeout mechanism with the intended
validation timeout, and convert timeout failures into FabricValidationError
without allowing the validation call to hang indefinitely.
In `@plugins/nemo-agents/src/nemo_agents_plugin/schema.py`:
- Around line 45-46: Update the checked-in OpenAPI artifact for
CreateAgentRequest.config to describe it as a format-neutral agent configuration
dictionary, matching the schema.py source model rather than the NAT
workflow-specific description. Regenerate the OpenAPI schema using the project’s
standard process, or make the corresponding artifact-only update if regeneration
is unavailable.
---
Nitpick comments:
In `@plugins/nemo-agents/tests/unit/test_fabric_validation.py`:
- Around line 73-145: Replace every hardcoded Path("/tmp/agent") in the affected
tests around validate_fabric_config with pytest’s tmp_path fixture, updating
each test signature to accept tmp_path and passing it directly as base_dir.
Preserve the existing assertions and validation behavior while applying the
substitution consistently to all occurrences.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3f5dbf16-0722-47b2-80b8-468b7791796a
📒 Files selected for processing (14)
plugins/nemo-agents/examples/nemo-agent-config/agent.yamlplugins/nemo-agents/openapi/openapi.yamlplugins/nemo-agents/pyproject.tomlplugins/nemo-agents/src/nemo_agents_plugin/agent_config.pyplugins/nemo-agents/src/nemo_agents_plugin/entities.pyplugins/nemo-agents/src/nemo_agents_plugin/fabric/README.mdplugins/nemo-agents/src/nemo_agents_plugin/fabric/__init__.pyplugins/nemo-agents/src/nemo_agents_plugin/fabric/translator.pyplugins/nemo-agents/src/nemo_agents_plugin/fabric/validation.pyplugins/nemo-agents/src/nemo_agents_plugin/schema.pyplugins/nemo-agents/tests/unit/test_agent_config.pyplugins/nemo-agents/tests/unit/test_entities.pyplugins/nemo-agents/tests/unit/test_fabric_translator.pyplugins/nemo-agents/tests/unit/test_fabric_validation.py
Contributor
|
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
Contributor
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 `@plugins/nemo-agents/tests/unit/test_fabric_validation.py`:
- Line 48: Update the fake doctor report initialization to preserve any
explicitly supplied value, including falsey reports such as {}, by checking
whether doctor_report is not None before falling back to the default
_FakeDoctorReport. Keep the existing default behavior only when no report is
provided.
- Around line 115-120: Update validate_fabric_config so the
fabric_client.doctor(...) await is enforced with
FABRIC_VALIDATION_TIMEOUT_SECONDS via asyncio.wait_for or an equivalent timeout
mechanism. Catch the resulting timeout and raise FabricValidationError with the
expected “Fabric doctor timed out after …s” message, while preserving normal
doctor validation behavior.
- Around line 94-101: Update validate_fabric_config to execute the synchronous
fabric_client.plan call via asyncio.to_thread or an equivalent thread offload,
and await its result. Preserve the existing plan arguments and validation
behavior while ensuring plan runs outside the event loop thread.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 58df58ca-fe8b-41ab-a4b8-adf2ef5ae5ff
📒 Files selected for processing (2)
plugins/nemo-agents/openapi/openapi.yamlplugins/nemo-agents/tests/unit/test_fabric_validation.py
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/nemo-agents/openapi/openapi.yaml
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
mikeknep
approved these changes
Jul 16, 2026
mikeknep
left a comment
mikeknep
left a comment
Contributor
There was a problem hiding this comment.
Couple small nits and one suggestion that might be overkill and/or unwanted. Overall LGTM!
mmogallapalli
enabled auto-merge
July 16, 2026 21:12
mmogallapalli
deleted the
mmogallapall/aircore-896-define-platform-owned-fabric-agent-config-and-fabricconfig
branch
July 16, 2026 21:25
soluwalana
pushed a commit
that referenced
this pull request
Jul 17, 2026
* updating descriptions and adding constants Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com> * adding config format, translation, and validation Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com> * adding dep todos Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com> * adding config loading Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com> * cleanup and examples Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com> * lint Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com> * addressing feedback Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com> * addressing feedback Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com> * lint Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com> * feedback Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com> --------- Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
This was referenced Jul 17, 2026
Merged
This was referenced Jul 23, 2026
This was referenced Jul 30, 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.
Summary
This PR defines the initial Platform-owned config contract for Fabric-backed NeMo Agents as part of AIRCORE-896.
The main boundary established here is:
NeMo Platform owns the persisted
agent.yaml/nemo-agents-spec-v1shape. Fabric remains an execution dependency, and the plugin translates the Platform-owned config into a typed in-memoryFabricConfigbefore using Fabric SDK planning or execution APIs.Changes
nemo-agents-spec-v1and canonicalagent.yamlconstants/file refs.AgentConfigPydantic models for the Platform-owned agent config shape.agent.yaml.AgentConfigtoFabricConfig.plugins/nemo-agents/examples/nemo-agent-config/agent.yaml.Agent.configis no longer NAT-specific.Out Of Scope
Those are covered by follow-up AIRCORE-895 tickets.
Validation
Passed:
Result:
52 passedSummary by CodeRabbit
New Features
nemo-agents-spec-v1agent configuration format.Documentation