feat: add LangChain Deep Agents adapter - #47
Conversation
Add a Deep Agents (deepagents) harness adapter as a standalone wheel (nemo-fabric-adapters-deepagents), supporting one-shot and multi-turn/ resumed execution keyed by the Fabric runtime_id via a persistent LangGraph SQLite checkpointer. - Map Fabric input, model config (NVIDIA OpenAI-compatible default with a generic init_chat_model hook), workspace (FilesystemBackend root), system prompt, allowed tools, and MCP servers (via langchain-mcp-adapters) onto create_deep_agent; normalize response, messages, usage, LangGraph thread id, and errors into the Fabric result. - Telemetry through relay (nemo_relay.integrations.deepagents, ATOF/ATIF artifacts) and native (telemetry.config OTLP/OpenInference export, no artifacts). - Adapter preflight covering the deepagents package and model credential, plus the descriptor requirement.env doctor check. - Mocked adapter tests and an opt-in real integration test; a deepagents_config example builder and a file-config profile. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Yuchen Zhang <yuchenz@nvidia.com>
|
📖 Fern docs preview: https://nvidia-preview-pull-request-47.docs.buildwithfern.com/nemo/fabric |
|
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 LangChain Deep Agents adapter for Fabric, including runtime execution, model and tool resolution, checkpointing, telemetry, packaging, documentation, example configuration, CI wiring, and unit/integration tests. ChangesDeep Agents Adapter
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Adapter
participant ChatModel
participant DeepAgents
participant Checkpointer
Client->>Adapter: run(payload)
Adapter->>ChatModel: build_chat_model(payload)
Adapter->>Checkpointer: load or open runtime state
Adapter->>DeepAgents: invoke_agent(agent_kwargs, user_message, thread_id)
DeepAgents-->>Adapter: streamed result and events
Adapter->>Checkpointer: save thread_id and close checkpoint
Adapter-->>Client: normalized Fabric output
Possibly related PRs
Suggested labels: 🚥 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: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
153-155: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdapter guides list is missing the new Deep Agents README link.
Core Concepts now mentions
adapters/deepagents/, but this "Adapter guides" bullet only links Hermes SDK, Hermes CLI, and Codex CLI READMEs. Since a newadapters/deepagents/README.mdis added in this PR, it should be linked here too for consistency.📝 Proposed fix
- Adapter guides: [Hermes SDK](adapters/hermes-sdk/README.md), [Hermes CLI](adapters/hermes-cli/README.md), and - [Codex CLI](adapters/codex-cli/README.md). + [Codex CLI](adapters/codex-cli/README.md), and + [LangChain Deep Agents](adapters/deepagents/README.md).🤖 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 `@README.md` around lines 153 - 155, The “Adapter guides” bullet in README is missing the new Deep Agents guide, so update that list to include the adapters/deepagents/README.md link alongside the existing Hermes SDK, Hermes CLI, and Codex CLI entries. Keep the wording and ordering consistent with the current Adapter guides section, and make sure the new Deep Agents README is referenced from the same bullet where the other adapter guide links live.Source: Path instructions
🤖 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/deepagents/pyproject.toml`:
- Around line 16-24: The dependency list in the deepagents pyproject is missing
upper bounds for langchain-mcp-adapters and langgraph-checkpoint-sqlite, unlike
the other pinned packages. Update the dependency spec in the pyproject.toml for
those two entries to include compatible upper version limits, matching the
existing pattern used by deepagents, langchain, and langgraph so future breaking
releases are not pulled in unexpectedly.
In `@adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py`:
- Around line 307-343: The checkpointer is opened before the guarded scope, so
failures in `resolve_tools`, `resolve_backend`, or `resolve_observability` can
skip `close_checkpointer` and leak the SQLite connection. Move the
`open_checkpointer` setup for `checkpointer`/`thread_id` into the same `try`
block in the deepagents adapter flow, or otherwise ensure acquisition and
cleanup are wrapped together so `finally` always runs. Keep `observability`
available where it is later reused, but make sure the lifecycle of
`checkpointer` is fully covered by the cleanup path.
- Around line 269-280: The open_checkpointer() function creates a synchronous
SqliteSaver but the runtime uses asynchronous agent.ainvoke(), causing
incompatibility with runtime_id resume functionality. Replace the import
statement to use AsyncSqliteSaver from langgraph.checkpoint.sqlite.aio instead
of SqliteSaver, update the SqliteSaver.from_conn_string() call to use
AsyncSqliteSaver.from_conn_string(), and ensure the context manager and cleanup
code (referenced around lines 372-380) are also updated to properly handle the
async checkpointer cleanup.
In `@tests/adapters/test_deepagents.py`:
- Around line 119-142: The shared test setup helpers are implemented as local
helper functions instead of pytest fixtures, which conflicts with the test
guidelines and makes reuse across test files harder. Convert _payload,
_install_fake_relay, and _install_fake_langgraph in test_deepagents.py into
fixtures (or move them to conftest.py if they must be shared with other test
modules) and update the tests to consume those fixtures rather than calling
helper methods directly.
- Around line 31-117: The test setup is using hand-rolled Fake classes instead
of the project’s required mock style. Replace `_FakeChatOpenAI`,
`_FakeFilesystemBackend`, `_FakeAgent`, `_FakeSaverCM`, `_FakeSaver`, and
`_FakeSqliteSaver` with `unittest.mock.MagicMock` or `AsyncMock` instances (use
`spec` where appropriate) inside `fake_sdks_fixture` and
`_install_fake_langgraph`, and rename any helper mock variables with a `mock`
prefix. Keep the same behavior captured in `create_deep_agent`,
`_FakeAgent.ainvoke`, and `SqliteSaver.from_conn_string`, but implement it
through mocks rather than custom classes.
- Around line 164-165: The assertions in the deepagents tests are using
defensive `.get()` lookups on expected dictionaries, which should be replaced
with direct key access so missing keys fail loudly. अपडेट the assertions in the
relevant test cases in test_deepagents.py to use direct indexing on
fake_sdks["create_kwargs"] and calls instead of .get(), keeping the checks in
the existing test functions that verify system_prompt, instructions, and call
contents.
- Around line 318-322: The _FakeMCPClient test helper has a mutable class-level
default for connections that triggers RUF012; update the class definition to
make the intent explicit by marking connections as a ClassVar, or remove the
class attribute entirely and keep the reassignment inside __init__. Preserve the
existing _FakeMCPClient behavior while ensuring the class no longer advertises a
mutable default shared across instances.
In `@tests/e2e/test_deepagents.py`:
- Around line 30-31: Remove the `-> None` return type annotation from the test
functions in `test_deepagents_oneshot_and_runtime` and `test_deepagents_doctor`
so they match the repo’s test conventions. Update the function signatures only,
keeping the existing test bodies and any calls like `_require_integration()`
unchanged.
- Around line 19-27: Convert the integration gate from the plain helper
`_require_integration` into a pytest fixture in `tests/e2e/test_deepagents.py`,
since the repo prefers fixtures over helper methods for test gating. Make the
gate run as a fixture with no return value, then attach it to the relevant tests
using `@pytest.mark.usefixtures("_require_integration")` (or equivalent fixture
wiring) instead of calling it at the top of each test. Keep the existing checks
for `RUN_FABRIC_DEEPAGENTS_INTEGRATION`, `deepagents`, `nemo_fabric._native`,
and `NVIDIA_API_KEY` inside that fixture.
---
Outside diff comments:
In `@README.md`:
- Around line 153-155: The “Adapter guides” bullet in README is missing the new
Deep Agents guide, so update that list to include the
adapters/deepagents/README.md link alongside the existing Hermes SDK, Hermes
CLI, and Codex CLI entries. Keep the wording and ordering consistent with the
current Adapter guides section, and make sure the new Deep Agents README is
referenced from the same bullet where the other adapter guide links live.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: 38698144-5f69-429a-8266-ef8c9e0624cd
⛔ Files ignored due to path filters (2)
adapters/deepagents/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
.github/workflows/ci_python.ymlREADME.mdadapters/deepagents/README.mdadapters/deepagents/fabric-adapter.jsonadapters/deepagents/pyproject.tomladapters/deepagents/src/nemo_fabric_adapters/deepagents/__init__.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pyexamples/code_review_agent/__init__.pyexamples/code_review_agent/config.pyjustfilepyproject.tomltests/adapters/test_deepagents.pytests/e2e/test_deepagents.pytests/fixtures/file-config-agent/profiles/deepagents.yaml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (18)
**/*.{py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*.{py,pyi}: If Python code or a Python-facing adapter changed, runjust test-python.
For Python SDK or PyO3 binding changes, usepython-tests, run focused pytest tests first, then runjust test-python; rebuild withjust build-pythonwhen native code or packaging changed.
Files:
examples/code_review_agent/__init__.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/__init__.pyexamples/code_review_agent/config.pytests/e2e/test_deepagents.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
For CI or packaging changes, use
maintain-ciormaintain-packaging, then run the recipes and checks whose behavior changed.
Files:
examples/code_review_agent/__init__.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/__init__.pyadapters/deepagents/fabric-adapter.jsonadapters/deepagents/README.mdtests/fixtures/file-config-agent/profiles/deepagents.yamlexamples/code_review_agent/config.pyREADME.mdadapters/deepagents/pyproject.tomlpyproject.tomljustfiletests/e2e/test_deepagents.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
{adapters/**,examples/**}
⚙️ CodeRabbit configuration file
{adapters/**,examples/**}: Review adapter and example changes for command correctness, config/schema consistency, artifact handling, and compatibility with the public Fabric contracts.
Files:
examples/code_review_agent/__init__.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/__init__.pyadapters/deepagents/fabric-adapter.jsonadapters/deepagents/README.mdexamples/code_review_agent/config.pyadapters/deepagents/pyproject.tomladapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
**/README.md
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Update relevant adapter or example
README.mdfiles when examples or adapters have changed
Files:
adapters/deepagents/README.mdREADME.md
**/*.{md,mdx}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
**/*.{md,mdx}: SpellNVIDIAin all caps; do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun becauseNVIDIAstarts with an "en" sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company.
Use trademark symbols with product names only when the document type or legal guidance requires them.
Verify official capitalization, spacing, and hyphenation for NVIDIA product names.
Precede NVIDIA product names withNVIDIAon first mention when it is natural and accurate.
Link the first mention of a product name when the destination helps the reader.
Do not rewrite product names for grammar or title-case rules.
Preserve third-party product names according to the owner's spelling.
Include the company name and full model qualifier on first use when it helps identify the model.
Preserve the official capitalization and punctuation of model names.
Use shorter family names only after the full model name is established.
For learning-oriented docs, technical blog posts, GTC sessions, tutorials, and developer guides: do not force trademark symbols unless the source, platform, or legal guidance explicitly requires them.
For learning-oriented docs, technical blog posts, GTC sessions, tutorials, and developer guides: keep the product name accurate and consistent.
For press releases, product landing pages, packaging, sales content, or legal copy: follow the current NVIDIA trademark and copyright guidance.
For press releases, product landing pages, packaging, sales content, or legal copy: attribute trademarks on first use when required.
For press releases, product landing pages, packaging, sales content, or legal copy: use the required trademark symbol for the specific product or service.
For press releases, product landing pages, packaging, sales content, or legal copy: do not invent trademark attributions; verify current legal copy.
If the platform requires legal copy, confirm the current source of truth instead...
Files:
adapters/deepagents/README.mdREADME.md
**/*.{md,mdx,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-technical-docs.md)
**/*.{md,mdx,rst}: When reviewing technical documentation, verify that commands, examples, paths, APIs, and support claims match the current repository.
Make technical documents easy to scan by fixing headings, lead-in sentences, lists, tables, and procedure shape.
Preserve exact code, command, API, package, and UI strings unless they are factually wrong.
Prefer focused findings over broad rewrites.
Use title case consistently in technical documentation headings.
Avoid quotation marks, ampersands, and exclamation marks in technical documentation headings.
Keep product, event, research, and whitepaper names in their official title case.
Use title case for table headers.
Do not force social-media sentence case into technical documentation.
Format code elements, commands, parameters, package names, and expressions in monospace.
Format directories, file names, and paths in monospace.
Use angle brackets inside monospace for variables inside paths.
Format error messages and strings with quotation marks, using code formatting when that is clearer.
Format UI buttons, menus, fields, and labels in bold.
Use angle brackets between UI labels for menu paths.
Use italics on first use for new terms, and only when the term is introduced.
Italicize publication titles.
Write keyboard shortcuts in plain text.
Use Owner/repo link text for GitHub repositories.
Introduce every code block with a complete sentence.
Do not make a code block complete the grammar of the previous sentence.
Do not continue a sentence after a code block.
Use syntax highlighting when the format supports it.
Avoid the word "snippet" unless the surrounding documentation already uses it as a term of art.
Keep inline method, function, and class references consistent with nearby docs, and omit empty parentheses in prose when no call is shown.
Use descriptive anchor text that matches the destination title when possible.
Avoid raw URLs in running text.
Avoid generic anchors such as "here," "this page," and "read more....
Files:
adapters/deepagents/README.mdREADME.md
**/*.{md,mdx,rst,txt}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*.{md,mdx,rst,txt}: If documentation or examples changed, runjust docswhen practical and verify documented commands against the current repository.
For documentation-only changes, usecontribute-docsandreview-doc-style; runjust docsfor docs-site or generated-reference changes.
Files:
adapters/deepagents/README.mdREADME.md
.github/workflows/*.{yml,yaml}
📄 CodeRabbit inference engine (.agents/skills/maintain-ci/SKILL.md)
.github/workflows/*.{yml,yaml}: Putpermissions:on each job that needs token access.
Avoid workflow-levelpermissions:unless the repository intentionally centralizes them and the inheritance tradeoff is documented.
Keep third-party actions pinned to full commit SHAs and preserve the readable version comment after the SHA.
Prefer action-native or ecosystem-native caching over genericactions/cache.
Use lockfiles or dependency manifests to drive cache invalidation.
Keep deploy and publish permissions isolated to the jobs that need them.
Read both caller and callee when a workflow usesworkflow_call.
The default minimum for checkout-based build, test, docs, and packaging jobs iscontents: read.
pull-requests: readis required for PR metadata lookup jobs.
pages: writeandid-token: writeshould be limited to Pages deployment jobs and any caller that invokes them through a reusable workflow.
For reusable workflows, the caller must grant every permission the called jobs require; the callee cannot elevate beyond what the caller provides.
Preferastral-sh/setup-uvcache support withcache-dependency-globanchored touv.lock.
PreferSwatinem/rust-cachewith explicitshared-keyandworkspacesinstead of ad hoc target-directory caching.
Avoid caching generated outputs that can hide stale behavior unless the repo already relies on them deliberately.
Each job should have the minimum permissions it needs.
Reusable workflow callers should grant only the scopes their callees require.
Every external action must be pinned to a full SHA.
Cache settings should be tied to lockfiles, manifests, or explicit tool versions.
Secrets should only be passed to the jobs that consume them.
Python, Rust, and documentation jobs should remain aligned with their lockfiles andjustfilerecipes.
Concurrency, branch filters, and documentation publish guards should still reflect repository intent.
Files:
.github/workflows/ci_python.yml
{.github/workflows/ci_python.yml,.github/workflows/ci_rust.yml}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Keep CI workflow install commands and example commands in the Python and Rust workflows consistent with the current package names and build/install flow.
Files:
.github/workflows/ci_python.yml
{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/fixtures/file-config-agent/profiles/deepagents.yamltests/e2e/test_deepagents.pytests/adapters/test_deepagents.py
{README.md,docs/index.yml,docs/**/*.{md,mdx}}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Update entry-point docs when examples or reading paths change
Files:
README.md
{README.md,docs/index.yml}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Update
README.mdordocs/index.ymlwhen entry points changed
Files:
README.md
{docs/**,README.md,AGENTS.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,AGENTS.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency with generated schemas.
Files:
README.md
**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If the PyO3 bridge or package metadata changed, run
just build-pythonandcargo check -p fabric-python --locked.
Files:
adapters/deepagents/pyproject.tomlpyproject.toml
pyproject.toml
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Keep the root
pyproject.tomlaligned with NeMo Fabric's Rust/Python packaging and maturin build configuration.
Files:
pyproject.toml
justfile
📄 CodeRabbit inference engine (.agents/skills/maintain-ci/SKILL.md)
Keep local commands aligned with the corresponding
justfilerecipes when they provide equivalent behavior.Keep
justfilebuild, test, clean, and documentation recipes aligned with the current packaging and release workflow.Use
just --fmt --checkfor Justfile and patch hygiene.
Files:
justfile
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Usepytestto run Python tests.
Do not add@pytest.mark.asyncioto test functions; async tests should be auto-detected and run by the async runner.
Do not add a-> Nonereturn type annotation to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, usingspecwhen necessary.
Name mocked classes with amockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures across test files; if a fixture is needed in multiple test files, place it inconftest.py.
When creating a fixture, use the pattern@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])followed bydef <fixture_name>_fixture() -> <return_type>:; only specifyscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating individual tests for different input types.
If a fixture is needed for a test but does not return a value or its value is unused, use@pytest.mark.usefixtures.
If you need to modify environment variables in a test, useos.environ;tests/conftest.pyprovides an autouserestore_environ_fixturethat restores the environment after each test, somonkeypatch.setenvis unnecessary.
Avoid defensive programming in tests; access expected dictionary keys directly (for example,results["data"]) instead of using.get(), so failures raise loudly and clearly.
Files:
tests/e2e/test_deepagents.pytests/adapters/test_deepagents.py
tests/adapters/**/*.py
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
tests/adapters/**/*.py: If an adapter or integration changed, run its focused tests.
For adapter behavior changes, run the focused adapter tests undertests/adapters, thenjust test-python.
Files:
tests/adapters/test_deepagents.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:
adapters/deepagents/fabric-adapter.json
🪛 ast-grep (0.44.1)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
[info] 34-34: use jsonify instead of json.dumps for JSON output
Context: json.dumps(output, sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 263-263: use jsonify instead of json.dumps for JSON output
Context: json.dumps({"runtime_id": runtime_id, "thread_id": thread_id}, indent=2)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 299-299: use jsonify instead of json.dumps for JSON output
Context: json.dumps(user_message, sort_keys=True)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Ruff (0.15.20)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
[warning] 59-62: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 68-71: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 107-107: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 112-112: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 139-139: Dynamically typed expressions (typing.Any) are disallowed in resolve_backend
(ANN401)
[warning] 254-254: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 269-269: Dynamically typed expressions (typing.Any) are disallowed in open_checkpointer
(ANN401)
[warning] 282-282: Dynamically typed expressions (typing.Any) are disallowed in checkpointer
(ANN401)
[warning] 339-339: Do not catch blind exception: Exception
(BLE001)
[warning] 372-372: Dynamically typed expressions (typing.Any) are disallowed in invoke_agent
(ANN401)
[warning] 391-391: Boolean-typed positional argument in function definition
(FBT001)
[warning] 405-405: Boolean positional value in function call
(FBT003)
[warning] 411-411: Boolean positional value in function call
(FBT003)
[warning] 425-425: Dynamically typed expressions (typing.Any) are disallowed in result_state
(ANN401)
[warning] 458-458: Dynamically typed expressions (typing.Any) are disallowed in result_state
(ANN401)
[warning] 464-464: Use a list comprehension to create a transformed list
(PERF401)
[warning] 468-468: Dynamically typed expressions (typing.Any) are disallowed in message
(ANN401)
[warning] 481-481: Dynamically typed expressions (typing.Any) are disallowed in _final_response
(ANN401)
[warning] 506-506: Dynamically typed expressions (typing.Any) are disallowed in callable_obj
(ANN401)
tests/adapters/test_deepagents.py
[warning] 32-32: Dynamically typed expressions (typing.Any) are disallowed in **kwargs
(ANN401)
[warning] 37-37: Dynamically typed expressions (typing.Any) are disallowed in **_kwargs
(ANN401)
[warning] 66-66: Dynamically typed expressions (typing.Any) are disallowed in **kwargs
(ANN401)
[warning] 90-90: Remove quotes from type annotation
Remove quotes
(UP037)
[warning] 93-93: Dynamically typed expressions (typing.Any) are disallowed in *_exc
(ANN401)
[warning] 184-184: Dynamically typed expressions (typing.Any) are disallowed in *args
(ANN401)
[warning] 184-184: Dynamically typed expressions (typing.Any) are disallowed in **kwargs
(ANN401)
[warning] 184-184: Dynamically typed expressions (typing.Any) are disallowed in fake_find_spec
(ANN401)
[warning] 198-198: Dynamically typed expressions (typing.Any) are disallowed in **_kwargs
(ANN401)
[warning] 198-198: Dynamically typed expressions (typing.Any) are disallowed in boom
(ANN401)
[warning] 199-199: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 212-212: Dynamically typed expressions (typing.Any) are disallowed in **_
(ANN401)
[warning] 219-219: Missing return type annotation for private function plugin_ctx
(ANN202)
[warning] 219-219: Dynamically typed expressions (typing.Any) are disallowed in _config
(ANN401)
[warning] 252-252: Assertion should be broken down into multiple parts
Break down assertion into multiple parts
(PT018)
[warning] 294-294: Assertion should be broken down into multiple parts
Break down assertion into multiple parts
(PT018)
[warning] 319-319: Mutable default value for class attribute
(RUF012)
🔇 Additional comments (17)
tests/adapters/test_deepagents.py (1)
145-207: LGTM! Coverage of one-shot normalization, preflight failures (missing API key, missingdeepagentspackage), error normalization, relay/native telemetry, workspace-rooted backend, MCP tool filtering, and resume/thread-id behavior is thorough and matches the adapter's described contract.Also applies to: 239-366
adapters/deepagents/src/nemo_fabric_adapters/deepagents/__init__.py (1)
1-5: LGTM!adapters/deepagents/fabric-adapter.json (2)
10-12: 🗄️ Data Integrity & Integration | ⚡ Quick winStatic
requirements.envdoesn't track configurableapi_key_env.The manifest hardcodes
NVIDIA_API_KEYas the required env var, but the README documents thatmodels.default.api_key_envis configurable per-config. If a user sets a differentapi_key_env, this manifest requirement will not reflect the actual variable the adapter needs, potentially causing incorrect preflight pass/fail behavior.Since
adapter.py's preflight logic isn't in this review scope, please confirm whether preflight checks readrequirements.envfrom this manifest or independently resolveapi_key_envfrom the runtime config.
1-9: LGTM!Also applies to: 13-19
adapters/deepagents/pyproject.toml (1)
1-15: LGTM!Also applies to: 25-35
adapters/deepagents/README.md (2)
41-56: 🎯 Functional CorrectnessRuntime/checkpointer behavior not verifiable in this cohort.
The described resume semantics (fresh
runtime_idper one-shot run, thread ID correlation, SQLite checkpointer reuse) are consistent with the PR objectives, butadapter.pyimplementing this logic isn't part of this review batch.Please confirm this matches the actual
run_deepagents/checkpointer implementation inadapter.pyonce available for review.
1-40: LGTM!Also applies to: 57-71
examples/code_review_agent/config.py (1)
161-186: LGTM!.github/workflows/ci_python.yml (1)
58-58: LGTM!README.md (1)
122-124: LGTM!justfile (1)
13-13: LGTM!Also applies to: 331-331
pyproject.toml (1)
29-32: LGTM!Also applies to: 45-48, 73-73, 104-104
examples/code_review_agent/__init__.py (1)
10-10: LGTM!Also applies to: 25-25
tests/e2e/test_deepagents.py (2)
40-60: LGTM!
63-74: LGTM!tests/fixtures/file-config-agent/profiles/deepagents.yaml (2)
1-37: LGTM!
8-13: 🗄️ Data Integrity & Integration
harness.settings.workspaceis consumed as a fallback, so this is not dead config.resolve_backend()usesenvironment.workspacefirst and falls back tocommon_utils.settings_payload(payload).get("workspace"); the duplicate value here is redundant, but active.> Likely an incorrect or invalid review comment.
- Default to NVIDIA's endpoint only for nvidia/unspecified providers; a plain openai provider now uses ChatOpenAI's own default endpoint. - Read the tools allow-list from config.tools (the routed plan only marks native.tools_configured), and resolve the MCP url/command from McpServerPlan.url (there is no command field) so stdio servers work. - Map routed native.skill_paths to Deep Agents skills, and pass harness.settings.deepagents through to create_deep_agent. - Stream the run to buffer per-step events; surface token usage and cost when the provider reports it. - Add nemo-relay to the adapter dependencies so relay and native telemetry work from the standalone wheel / documented install. - Drop the provider-specific static NVIDIA_API_KEY doctor requirement; the runtime preflight validates the configured credential and deepagents import. - Regenerate lockfiles with a current uv (restores lock revision 3, focused diff). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Yuchen Zhang <yuchenz@nvidia.com>
Address Deep Agents adapter review round 2: - Use AsyncSqliteSaver (langgraph.checkpoint.sqlite.aio) instead of the sync SqliteSaver, whose async methods raise NotImplementedError under astream, so real one-shot and multi-turn runs no longer fail. Acquire/close it async. - Enforce the config.tools allow-list across the full tool surface (Deep Agents built-ins such as write_file/execute/task and MCP tools) via a gating middleware, rather than only filtering the MCP tools list. - Acquire the async checkpointer inside the guarded try/finally, after tools, backend, and observability are resolved, so a setup failure can no longer leak the SQLite connection in the inline runtime. - Add real-stack regression tests: a real LangGraph graph + AsyncSqliteSaver driven via astream, and the allow-list gating middleware. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Yuchen Zhang <yuchenz@nvidia.com>
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/adapters/test_deepagents.py (1)
357-362: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRename the MCP test to match middleware-based enforcement.
The assertion intentionally passes both tools to Deep Agents; filtering now happens in middleware. Rename the surrounding test so it no longer claims the tool list is filtered.
🤖 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_deepagents.py` around lines 357 - 362, Rename the surrounding MCP test to describe middleware-based tool enforcement rather than filtered tool lists. Keep the assertions in the test unchanged, including passing both “read_file” and “write_file” tools to Deep Agents; update only the test name and any related wording that incorrectly claims filtering occurs before invocation.
🤖 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 `@tests/adapters/test_deepagents.py`:
- Line 394: Remove the -> None return annotations from the pytest test functions
test_allowed_tools_middleware_blocks_disallowed_tools and the test function
introduced at line 416, leaving their async definitions unannotated.
- Around line 97-108: Update _FakeAsyncSaverCM to record __aexit__ invocations,
then extend the run_deepagents tests to assert the async checkpointer context is
exited exactly once on both successful and failing execution paths. Ensure the
shared spy/counter is reset or isolated between tests and that assertions verify
cleanup even when run_deepagents raises.
- Around line 439-444: Update the _build function to require a checkpointer from
kwargs using direct indexing, assert that the retrieved checkpointer is
non-null, and pass it to graph.compile so the regression test cannot silently
run without persistence wiring.
---
Outside diff comments:
In `@tests/adapters/test_deepagents.py`:
- Around line 357-362: Rename the surrounding MCP test to describe
middleware-based tool enforcement rather than filtered tool lists. Keep the
assertions in the test unchanged, including passing both “read_file” and
“write_file” tools to Deep Agents; update only the test name and any related
wording that incorrectly claims filtering occurs before invocation.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: a42c6d40-d734-4bac-bdcc-c35f1712e500
📒 Files selected for processing (3)
adapters/deepagents/README.mdadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Build and publish docs
- GitHub Check: CodeRabbit / Review
🧰 Additional context used
📓 Path-based instructions (3)
{adapters/**,examples/**}
⚙️ CodeRabbit configuration file
{adapters/**,examples/**}: Review adapter and example changes for command correctness, config/schema consistency, artifact handling, and compatibility with the public Fabric contracts.
Files:
adapters/deepagents/README.mdadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Usepytestto run Python tests.
Do not add@pytest.mark.asyncioto test functions; async tests are detected and run automatically.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, addingspecwhen needed.
Name mocked classes with amockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not duplicate fixtures across test files; if a fixture is needed in multiple test files, define it inconftest.py.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and define the function as<fixture_name>_fixture() -> <return_type>; only passscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating separate tests for different input types.
If a fixture is needed for a test but does not return a value, or its value is unused, use@pytest.mark.usefixtures.
When modifying environment variables in a test, useos.environ;tests/conftest.pyprovides an autouserestore_environ_fixturethat restores environment variables after each test, somonkeypatch.setenvis unnecessary.
Files:
tests/adapters/test_deepagents.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_deepagents.py
🧠 Learnings (1)
📚 Learning: 2026-07-09T22:28:51.689Z
Learnt from: AjayThorve
Repo: NVIDIA/NeMo-Fabric PR: 43
File: adapters/claude-sdk/src/nemo_fabric_adapters/claude_sdk/adapter.py:164-168
Timestamp: 2026-07-09T22:28:51.689Z
Learning: In the NeMo-Fabric adapters, treat path values used in Fabric adapter configuration (including logic like `_resolve_path` in adapter.py) as config-root-relative. Do not apply `Path.expanduser()` (or otherwise apply `~`/home or shell-style expansion), because it will make the resolved paths normalize inconsistently across adapters. Also, do not rely on or add any resolution behavior that uses `harness.settings.cwd` as an override point for these adapter paths—`harness.settings.cwd` is explicitly unsupported in this adapter context.
Applied to files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
🧬 Code graph analysis (1)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py (1)
tests/adapters/test_deepagents.py (1)
run_deepagents(156-156)
🪛 Ruff (0.15.20)
tests/adapters/test_deepagents.py
[warning] 98-98: Remove quotes from type annotation
Remove quotes
(UP037)
[warning] 101-101: Dynamically typed expressions (typing.Any) are disallowed in *_exc
(ANN401)
[warning] 401-401: Dynamically typed expressions (typing.Any) are disallowed in _request
(ANN401)
[warning] 436-436: Dynamically typed expressions (typing.Any) are disallowed in _state
(ANN401)
[warning] 439-439: Dynamically typed expressions (typing.Any) are disallowed in **kwargs
(ANN401)
[warning] 439-439: Dynamically typed expressions (typing.Any) are disallowed in _build
(ANN401)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
[warning] 176-176: Dynamically typed expressions (typing.Any) are disallowed in allowed_tools_middleware
(ANN401)
[warning] 188-188: Dynamically typed expressions (typing.Any) are disallowed in request
(ANN401)
[warning] 188-188: Dynamically typed expressions (typing.Any) are disallowed in _blocked
(ANN401)
[warning] 197-197: Dynamically typed expressions (typing.Any) are disallowed in request
(ANN401)
[warning] 197-197: Dynamically typed expressions (typing.Any) are disallowed in handler
(ANN401)
[warning] 197-197: Dynamically typed expressions (typing.Any) are disallowed in awrap_tool_call
(ANN401)
[warning] 202-202: Dynamically typed expressions (typing.Any) are disallowed in request
(ANN401)
[warning] 202-202: Dynamically typed expressions (typing.Any) are disallowed in handler
(ANN401)
[warning] 202-202: Dynamically typed expressions (typing.Any) are disallowed in wrap_tool_call
(ANN401)
[warning] 303-303: Dynamically typed expressions (typing.Any) are disallowed in open_checkpointer
(ANN401)
[warning] 320-320: Dynamically typed expressions (typing.Any) are disallowed in checkpointer
(ANN401)
[warning] 330-330: Dynamically typed expressions (typing.Any) are disallowed in model
(ANN401)
🔇 Additional comments (5)
adapters/deepagents/README.md (1)
46-48: LGTM!adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py (2)
159-163: LGTM!Also applies to: 303-323, 329-349, 367-397
176-207: 🔒 Security & PrivacyNo change needed.
tests/adapters/test_deepagents.py (2)
93-108: Replace hand-rolled SDK mocks withMagicMock/AsyncMock.The async checkpointer stubs still define custom mock classes, repeating the prior finding. Use
MagicMock/AsyncMockwithspecwhere appropriate andmock_*variable names.As per path instructions, tests must use
MagicMock/AsyncMockwhen mocking classes.Source: Path instructions
111-123: Move shared LangGraph setup into fixtures.
_install_fake_langgraphand_use_real_langgraphare reusable test setup helpers, repeating the prior fixture-guideline finding. Convert them into fixtures so module-state mutation and cleanup are declared dependencies.As per coding guidelines, prefer pytest fixtures over helper methods.
Also applies to: 365-375
Source: Coding guidelines
An explicitly configured `tools: []` produced an empty set that `names or None` collapsed to `None`, which build_agent_kwargs read as "no allow-list" and so attached no gating middleware, leaving every Deep Agents built-in and MCP tool enabled. Return the set as-is (empty = deny-all) and only treat unconfigured tools as `None`. Add deny-all regression tests. Also correct the adapter README to describe the `astream` (updates/values) event-buffering runtime instead of a single `ainvoke`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Yuchen Zhang <yuchenz@nvidia.com>
- Bound langchain-mcp-adapters (<0.3.0) and langgraph-checkpoint-sqlite; the unbounded specs had already resolved to 0.3.0 / 3.1.0. Use <4.0 (not the suggested <3.0) for langgraph-checkpoint-sqlite: the 2.x line is incompatible with the langgraph core deepagents pulls (AsyncSqliteSaver fails with 'Connection object has no attribute is_alive'); lock the working 3.1.0. - Tests: use direct dict access instead of .get() in assertions; drop the `-> None` return annotations on test functions; mark _FakeMCPClient.connections ClassVar; convert the e2e integration gate to a fixture via usefixtures. - Add checkpointer-cleanup coverage: the fake async saver records __aexit__ and a test asserts the connection is closed on both success and failure paths. - Require the real-graph regression test to receive a non-null checkpointer. CodeRabbit's async-checkpointer and checkpointer-leak comments were already fixed in earlier commits. The Fake*->MagicMock and helpers->fixtures refactors are skipped: async generators / async context managers do not map cleanly to MagicMock, and CodeRabbit tagged the helper refactor a poor tradeoff. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Yuchen Zhang <yuchenz@nvidia.com>
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 `@adapters/deepagents/pyproject.toml`:
- Around line 20-25: Update the langgraph-checkpoint-sqlite dependency
constraint in pyproject.toml to exclude the incompatible 2.x major and allow
only the supported release range, keeping the inline compatibility note
consistent with the constraint.
🪄 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: ASSERTIVE
Plan: Enterprise
Run ID: cdbf7a39-be96-4a70-b928-30bb6c1bb34d
⛔ Files ignored due to path filters (2)
adapters/deepagents/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
adapters/deepagents/pyproject.tomltests/adapters/test_deepagents.pytests/e2e/test_deepagents.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: CodeRabbit / Review
🧰 Additional context used
📓 Path-based instructions (3)
{adapters/**,examples/**}
⚙️ CodeRabbit configuration file
{adapters/**,examples/**}: Review adapter and example changes for command correctness, config/schema consistency, artifact handling, and compatibility with the public Fabric contracts.
Files:
adapters/deepagents/pyproject.toml
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Usepytestto run Python tests.
Do not add@pytest.mark.asyncioto test functions; async tests are detected and run automatically.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, addingspecwhen needed.
Name mocked classes with amockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not duplicate fixtures across test files; if a fixture is needed in multiple test files, define it inconftest.py.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and define the function as<fixture_name>_fixture() -> <return_type>; only passscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating separate tests for different input types.
If a fixture is needed for a test but does not return a value, or its value is unused, use@pytest.mark.usefixtures.
When modifying environment variables in a test, useos.environ;tests/conftest.pyprovides an autouserestore_environ_fixturethat restores environment variables after each test, somonkeypatch.setenvis unnecessary.
Files:
tests/e2e/test_deepagents.pytests/adapters/test_deepagents.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/e2e/test_deepagents.pytests/adapters/test_deepagents.py
🧬 Code graph analysis (1)
tests/adapters/test_deepagents.py (1)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py (2)
run_deepagents(353-425)_allowed_tool_names(167-174)
🪛 Ruff (0.15.20)
tests/adapters/test_deepagents.py
[warning] 104-104: Dynamically typed expressions (typing.Any) are disallowed in *_exc
(ANN401)
[warning] 336-336: Dynamically typed expressions (typing.Any) are disallowed in **_kwargs
(ANN401)
[warning] 336-336: Dynamically typed expressions (typing.Any) are disallowed in _boom
(ANN401)
🔇 Additional comments (7)
tests/adapters/test_deepagents.py (6)
31-90: 📐 Maintainability & Code QualityHand-rolled Fake classes still used instead of
unittest.mock.
_FakeChatOpenAI,_FakeFilesystemBackend,_FakeAgent,_FakeSaverCM/_FakeAsyncSaverCM,_FakeSaver,_FakeSqliteSaver, and the new_FakeMCPClient(lines 352-364) are all custom classes named with aFakeprefix, still not converted toMagicMock/AsyncMockwithspec.As per path instructions,
tests/**/*.py: "When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, addingspecwhen needed. Name mocked classes with amockprefix, notfake."Also applies to: 352-364
Source: Path instructions
110-127: 📐 Maintainability & Code QualityHelper functions still not converted to fixtures.
_install_fake_langgraph(and_payload/_install_fake_relayelsewhere in the file) remain plain helper functions rather than pytest fixtures, and are not centralized inconftest.pydespite being reusable setup shared across tests.As per coding guidelines, "Prefer pytest fixtures over helper methods" and "Do not duplicate fixtures across test files; if a fixture is needed in multiple test files, define it in
conftest.py."Source: Coding guidelines
178-179:.get()→ direct indexing fix confirmed.Assertions now use direct dict indexing (
fake_sdks["create_kwargs"]["system_prompt"],calls["wrapped"], etc.) instead of.get(), resolving the prior review comment.Also applies to: 266-267, 275-275, 309-310, 318-318
480-482: Regression test now requires a non-null checkpointer.
checkpointer = kwargs["checkpointer"]+assert checkpointer is not Nonebefore compiling closes the gap flagged previously.
327-343: 🎯 Functional Correctness | ⚡ Quick winAssert the first (success) run actually succeeded before checking cleanup.
Line 331 discards the output of the first
run_deepagentscall and never asserts it succeeded (e.g.output["failed"] is False) before checkingsaver_exits == 1. If the "success" path silently failed, this test would still pass, undermining the "success and failure" cleanup coverage the test name promises.As per path instructions, tests should cover "lifecycle cleanup" for the changed API surface — this test's success-path assertion is incomplete.
🧪 Proposed fix
- await adapter.run_deepagents(_payload(tmp_path)) + success_output = await adapter.run_deepagents(_payload(tmp_path)) + assert success_output["failed"] is False assert fake_sdks["saver_exits"] == 1Source: Path instructions
345-386: 🎯 Functional Correctness | ⚡ Quick winTest name/coverage mismatch: no allow-list filtering is actually exercised.
test_mcp_servers_become_tools_filtered_by_allowedimplies MCP tools are filtered by an allow-list, but the payload never setseffective_config.config.tools; the test only verifies unfiltered pass-through of bothread_fileandwrite_file. Per the adapter design (allow-listing is enforced viaallowed_tools_middleware, not tool-list filtering — seetest_allowed_tools_recorded_as_middleware), this test's name overstates what it covers.As per path instructions, tests should cover the behavior promised by the API surface — rename to reflect actual coverage, or add an allow-list assertion (e.g. via middleware) to match the name.
Source: Path instructions
tests/e2e/test_deepagents.py (1)
19-28: Prior review comments resolved.
_require_integrationis now a proper fixture (@pytest.fixture(name="_require_integration")/_require_integration_fixture() -> None) consumed via@pytest.mark.usefixtures, and both test functions no longer carry-> Noneannotations. Both previously-flagged issues are fixed.Also applies to: 31-32, 63-64
…pter # Conflicts: # .github/workflows/ci_python.yml # README.md # justfile # pyproject.toml # uv.lock
- README: add runnable Fabric.run / start_runtime examples for one-shot and multi-turn resume; list the Deep Agents adapter in the root README adapter guides and the code-review example variant table; add a `deepagents` variant to the example entrypoint. - Tighten langgraph-checkpoint-sqlite to >=3.0,<4.0 (2.x breaks AsyncSqliteSaver) so the resolver cannot pick the known-bad major; keep the inline note aligned. - Drop the stale parenthetical claiming fabric doctor's requirement.env is the auth guard: the descriptor declares no static env requirement (auth is provider-specific and validated by the runtime preflight). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Yuchen Zhang <yuchenz@nvidia.com>
Address the two open CodeRabbit threads on the Deep Agents adapter tests: - Replace the hand-rolled Fake* classes (chat model, filesystem backend, agent, async saver, MCP client, tools, request) with unittest.mock MagicMock/AsyncMock; async-generator and async-context-manager behaviors are provided by plain functions rather than new classes. - Convert the shared setup helpers (_payload, _install_fake_relay, _install_fake_langgraph, _use_real_langgraph) into pytest fixtures (make_payload factory, fake_relay, use_real_langgraph). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Yuchen Zhang <yuchenz@nvidia.com>
- Resumed usage/cost double-counted replayed prior-turn messages because usage was aggregated from the full final LangGraph state. Aggregate only the messages emitted this turn (the astream `updates` deltas) and add a resumed-usage regression test. - Docs: list the Deep Agents adapter across the public entry points — the code-review example README (four variants), examples/README.md, the Python SDK guide variant list, and the adapters-deepagents (plus adapters-claude) install extras. - Reword the preflight docstring so it describes invocation-time checks rather than implying `fabric doctor` performs the package/credential checks (core has no adapter-doctor hook). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Yuchen Zhang <yuchenz@nvidia.com>
…pter Signed-off-by: Yuchen Zhang <yuchenz@nvidia.com>
Separate the bundled test_deepagents_oneshot_and_runtime into test_deepagents_oneshot and test_deepagents_multi_turn so the multi-turn resume coverage is discoverable and each case can fail independently. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Yuchen Zhang <yuchenz@nvidia.com>
…failures Address code-review findings on the Deep Agents adapter: - Gate delegated subagents with the config.tools allow-list so tools routed through the `task` tool cannot run ungated. - Pass virtual_mode=True to FilesystemBackend so absolute paths and `..` cannot escape the workspace root (and to not rely on the deprecated 0.5 default). - Run preflight, model construction, and resume-state load inside the guarded scope so a missing credential or absent package returns a normalized failure instead of a raw traceback. - Raise AdapterConfigError for a non-list `tools` value or a misconfigured MCP server instead of silently disabling gating / dropping the server. - Default api_key_env per provider (NVIDIA_API_KEY / OPENAI_API_KEY) and require it explicitly for other providers, so a key is never sent to the wrong endpoint. - Stream with subgraphs=True and fold subagent message deltas (deduped by id) into usage so usage/cost is accurate for delegating runs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Yuchen Zhang <yuchenz@nvidia.com>
…r-provider defaults Follow-ups to the per-provider credential change: - README no longer claims api_key_env defaults to NVIDIA_API_KEY unconditionally; it now states the per-provider defaults in one place. - Document that `openai-compatible` (and any init_chat_model backend) must set api_key_env explicitly — a deliberate behavior change that was undocumented. - Reword the preflight error so it no longer says the credential env var "is defined in the configuration" when it may have only been defaulted per provider. - Add a regression test that `openai-compatible` without api_key_env is a normalized configuration failure. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Yuchen Zhang <yuchenz@nvidia.com>
AjayThorve
left a comment
AjayThorve
left a comment
There was a problem hiding this comment.
I have two questions about the SDK extension boundary before we establish this adapter contract.
|
@yczhang-nv I think we should keep subagent support narrow for this PR:
@AnuradhaKaruppiah curious to know your thoughts |
…t subagent scope Address review of the harness.settings.deepagents boundary: - Reserve Fabric-owned keys (model, tools, backend, skills, system_prompt, middleware, checkpointer): the passthrough can no longer override them, which previously let a config silently defeat the normalized model config, MCP tool resolution, workspace confinement, and tool gating. - Restrict the passthrough to a documented, JSON-serializable allow-list (subagents, interrupt_on) and fail clearly on unknown keys instead of silently dropping them via _supported_kwargs. - Rewrite the README: it is a JSON-only passthrough, not a Python-object escape hatch; document the subagent inheritance model (delegated subagents inherit the parent's tools/skills/workspace/permissions and the config.tools policy, so they cannot broaden capabilities) and that independently-configured subagents are future work. - Add an opt-in real delegation e2e test plus unit tests for passthrough validation (reserved-key override, unknown key, supported option forwarded). Deterministic verification of delegated tool-gating remains mock-based pending a subagents SDK update; tracked as a follow-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Yuchen Zhang <yuchenz@nvidia.com>
@AjayThorve Thanks for the clarification! I updated the code in 1b7f7ad to match the scope you mentioned. Specifically,
@AnuradhaKaruppiah LMK if you have any comments on those as well |
|
/merge |
2 participants
Overview
Adds a LangChain Deep Agents (
deepagents) harness adapter as a standalone, independently installable wheel (nemo-fabric-adapters-deepagents), following the per-adapter-package layout.The adapter supports one-shot and multi-turn/resumed execution, keyed by the Fabric
runtime_idvia a persistent LangGraph SQLite checkpointer (mirroring the codex-cli adapter). It maps Fabric config ontocreate_deep_agent:models.default(NVIDIA OpenAI-compatible by default, with aninit_chat_modelhook for other providers).FilesystemBackendatenvironment.workspace.langchain-mcp-adapters, filtered by thetoolsallow-list.RunResult.nemo_relay.integrations.deepagents, ATOF/ATIF artifacts) and native (telemetry.configOTLP/OpenInference export, no artifacts).deepagentspackage is importable and the model-provider credential is set.Wiring: root
pyproject.toml,justfile(python_projects, wheels), and the Python CI extras; plus adeepagents_config()example builder and a file-config profile.Where should the reviewer start?
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py— therun_deepagentsflow (model/tools/workspace mapping,runtime_id-keyed resume, and the unified relay/native telemetry path).tests/adapters/test_deepagents.pyis the mocked adapter contract;tests/e2e/test_deepagents.pyis the opt-in real smoke.Validation:
just test-python(full suite) passes; the adapter's mocked tests cover one-shot normalization, runtime resume, relay + native telemetry, MCP/allow-list mapping, workspace rooting, and preflight failures.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
N/A — no associated GitHub issue.
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.
Summary by CodeRabbit