fix(nemo-agents): support IGW for Fabric agents and install deps by default - #1003
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:
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 (1)
📝 WalkthroughWalkthroughAdds Fabric adapter dependencies and multi-harness examples, introduces top-level instructions and model ChangesFabric agent configuration
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/examples/nemo-agent-config/README.md`:
- Around line 27-29: Update the README description of instructions.system to
call it the shared system prompt field or location, not a path, while preserving
the existing guidance about inline content and adapter-specific settings.
- Around line 3-4: Add a prerequisites section at the beginning of the README
before the command list, covering the required runtime, CLI setup, credentials,
and any optional services needed by the nemo-agents-spec-v1 configurations.
Clearly distinguish required prerequisites from optional service dependencies,
while preserving the existing command instructions.
- Around line 35-64: Add a Python SDK tab alongside the existing CLI workflow in
the README, documenting an equivalent agent create, deploy, and invoke sequence
using the required SDK commands and configuration. Test the documented Python
flow end to end and ensure it matches the CLI example’s agent, deployment, and
input behavior.
- Around line 52-59: Update the README’s Relay verification commands to use the
same configuration file and deployment name established by the create/deploy
flow, specifically agent.yaml and fabric-agent-deployment. Replace the unrelated
hard-coded artifact path with a path derived from or matching that deployment,
and apply the same alignment to the additionally referenced section.
In `@plugins/nemo-agents/pyproject.toml`:
- Line 23: Update the nemo-fabric dependency declaration in pyproject.toml so
the deepagents extra is not included in the default installation. Move
deepagents to a separate opt-in dependency extra while preserving the existing
claude, codex, and relay requirements and version bounds.
🪄 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: bce4982f-09b7-4ead-bab1-f3b7b0c7f3e9
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
packages/nemo_platform/pyproject.tomlplugins/nemo-agents/README.mdplugins/nemo-agents/examples/nemo-agent-config/README.mdplugins/nemo-agents/examples/nemo-agent-config/agent-relay-intake.yamlplugins/nemo-agents/examples/nemo-agent-config/agent-relay.yamlplugins/nemo-agents/examples/nemo-agent-config/agent.yamlplugins/nemo-agents/pyproject.tomlplugins/nemo-agents/src/nemo_agents_plugin/agent_config.pyplugins/nemo-agents/src/nemo_agents_plugin/fabric/translator.pyplugins/nemo-agents/src/nemo_agents_plugin/utils.pyplugins/nemo-agents/tests/unit/test_agent_config.pyplugins/nemo-agents/tests/unit/test_deployments_api.pyplugins/nemo-agents/tests/unit/test_fabric_translator.pyplugins/nemo-agents/tests/unit/test_utils.pythird_party/licenses.jsonlthird_party/osv-licenses.jsonthird_party/requirements-main.txt
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>
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
cebdaa4 to
897f998
Compare
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@third_party/requirements-main.txt`:
- Around line 1688-1690: Update the nemo-fabric-adapters-hermes requirement
marker to include the existing Python <3.14 constraint, matching the
corresponding source manifests and uv.lock while preserving the current platform
conditions and hash.
🪄 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: b5a1bb40-f0b1-4214-b5ae-e1c4fd19c907
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
packages/nemo_platform/pyproject.tomlplugins/nemo-agents/README.mdplugins/nemo-agents/examples/nemo-agent-config/README.mdplugins/nemo-agents/examples/nemo-agent-config/agent-relay-intake.yamlplugins/nemo-agents/examples/nemo-agent-config/agent-relay.yamlplugins/nemo-agents/examples/nemo-agent-config/agent.yamlplugins/nemo-agents/pyproject.tomlplugins/nemo-agents/src/nemo_agents_plugin/agent_config.pyplugins/nemo-agents/src/nemo_agents_plugin/fabric/translator.pyplugins/nemo-agents/src/nemo_agents_plugin/runner/deployments_backend.pyplugins/nemo-agents/src/nemo_agents_plugin/utils.pyplugins/nemo-agents/tests/unit/test_agent_config.pyplugins/nemo-agents/tests/unit/test_deployments_api.pyplugins/nemo-agents/tests/unit/test_fabric_translator.pyplugins/nemo-agents/tests/unit/test_runner_deployments.pyplugins/nemo-agents/tests/unit/test_utils.pythird_party/licenses.jsonlthird_party/osv-licenses.jsonthird_party/requirements-main.txttools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/license/overrides.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- plugins/nemo-agents/README.md
- third_party/licenses.jsonl
- plugins/nemo-agents/examples/nemo-agent-config/README.md
AjayThorve
left a comment
There was a problem hiding this comment.
aligned with new fabric changes
Signed-off-by: Manjesh Mogallapalli <mmogallapall@nvidia.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/nemo-agents/tests/unit/test_utils.py (1)
189-204: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAssert that the original nested endpoint is preserved.
The test only checks that
base_urlwas not added to the original top level. It would still pass if translation removedconfig["models"]["default"]["settings"]["base_url"]in place.assert fabric_config.models["default"].base_url == "http://legacy:8080/v1" assert "base_url" not in result["models"]["default"]["settings"] + assert config["models"]["default"]["settings"]["base_url"] == "http://legacy:8080/v1"🤖 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_utils.py` around lines 189 - 204, Update test_promotes_legacy_settings_endpoint to assert that config["models"]["default"]["settings"]["base_url"] remains "http://legacy:8080/v1" after inject_fabric_gateway_url, while preserving the existing assertions that the result promotes the endpoint and the original top level is unchanged.
🤖 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.
Outside diff comments:
In `@plugins/nemo-agents/tests/unit/test_utils.py`:
- Around line 189-204: Update test_promotes_legacy_settings_endpoint to assert
that config["models"]["default"]["settings"]["base_url"] remains
"http://legacy:8080/v1" after inject_fabric_gateway_url, while preserving the
existing assertions that the result promotes the endpoint and the original top
level is unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 944cf31a-b7ba-47f8-b246-8d08b92f49ea
📒 Files selected for processing (5)
plugins/nemo-agents/examples/nemo-agent-config/README.mdplugins/nemo-agents/src/nemo_agents_plugin/fabric/translator.pyplugins/nemo-agents/src/nemo_agents_plugin/utils.pyplugins/nemo-agents/tests/unit/test_fabric_translator.pyplugins/nemo-agents/tests/unit/test_utils.py
Summary
This MR finishes the Fabric-backed agent wiring for the
nemo-agentsplugin:nemo-agentsinstall so Fabric-backed agents have the required runtime packages for subprocess, Docker, and Kubernetes deployments.hermes-agentstill has dependency conflicts with the Platform environment.base_urlinto FabricModelConfig.base_url.instructions.systemsupport for shared Fabric system instructions.base_urlvalues for Fabric-backed Docker/Kubernetes deployments, including the current top-level Fabricmodel.base_urlshape and legacysettings.base_url.Validation
Unit coverage added/updated for:
settings.base_urlpromotioninstructions.systemVerified Fabric planning for all example harnesses:
Verified end-to-end create/deploy/invoke flows locally:
.venv-hermesandADAPTER_PYTHONNotes
Hermes remains intentionally documented as a separate setup path. For subprocess deployments,
ADAPTER_PYTHONmust be exported before Platform starts so the Platform service can launch the Hermes adapter with the separate interpreter.Fabric dependencies were previously optional, which worked for local development paths but left non-local deployments without the packages needed to start Fabric-backed agents.
Summary by CodeRabbit