fix: keep a completed Deep Agents turn completed when telemetry fails - #191
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:
WalkthroughThe DeepAgents adapter separates agent execution failures from Relay telemetry setup, teardown, and artifact-collection failures. Completed results remain successful after post-invocation telemetry faults, which are reported through degraded telemetry output. ChangesRelay telemetry lifecycle
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Invocation
participant Relay
participant Agent
participant Artifacts
participant Output
Invocation->>Relay: Set up telemetry scope
Relay->>Agent: Hand off execution
Agent-->>Invocation: Return result or invocation error
Invocation->>Relay: Tear down telemetry scope
Relay-->>Invocation: Return telemetry error when teardown fails
Invocation->>Artifacts: Collect telemetry artifacts
Artifacts-->>Invocation: Return artifacts or collection error
Invocation->>Output: Normalize result and telemetry status
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
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 `@adapters/deepagents/README.md`:
- Around line 211-217: In the telemetry failure description, replace “Once the
agent has been invoked” with “After the agent has been invoked” to follow the
temporal wording guideline while preserving the stated outcome and
error-reporting behavior.
In `@adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py`:
- Around line 612-617: Update the exception handling around telemetry lifecycle
failures in the adapter’s invocation flow so `str(exc)` is never placed in the
returned response; use an allowlisted error code or fixed safe message for
pre-invocation failures while preserving the existing invocation-failure
behavior. Add a regression test using a secret-like exception message and assert
that the lifecycle output does not contain that value.
In `@tests/adapters/test_deepagents.py`:
- Around line 449-468: Update
test_relay_setup_failure_before_the_agent_runs_stays_an_invocation_failure to
replace or stub adapter.invoke_compiled_agent with an AsyncMock before calling
invoke_once, then assert the mock was not awaited afterward. Preserve the
existing output assertions while explicitly verifying relay scope setup failure
prevents agent execution.
🪄 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: 26b8fe7f-d325-4cbd-bbf7-93b1a68a46db
📒 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. (13)
- GitHub Check: Test (Python 3.12, linux-arm64)
- GitHub Check: Test (Python 3.14, linux-arm64)
- GitHub Check: Test (Python 3.12, linux-amd64)
- GitHub Check: Test (Python 3.13, linux-amd64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.11, linux-arm64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.14, linux-amd64)
- GitHub Check: Test (Python 3.13, linux-arm64)
- GitHub Check: Test (Python 3.11, linux-amd64)
- GitHub Check: Pre-commit
🧰 Additional context used
📓 Path-based instructions (24)
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Update documentation and examples in the same branch as the public API change.
Files:
adapters/deepagents/README.md
**/*
📄 CodeRabbit inference engine (.agents/skills/karpathy-guidelines/SKILL.md)
**/*: Before implementing, explicitly state assumptions, surface ambiguity and tradeoffs, present multiple interpretations when relevant, and ask for clarification rather than silently deciding or proceeding when requirements are unclear.
Prefer the minimum code needed to solve the requested problem: avoid speculative features, unnecessary abstractions, unrequested flexibility, and handling of impossible scenarios; simplify overcomplicated solutions.
When editing existing code, make surgical changes only: do not modify unrelated code, comments, formatting, or pre-existing dead code; match the existing style, and remove only unused imports, variables, or functions introduced by your changes.
Define verifiable success criteria for each task, such as writing regression tests for bugs and invalid-input tests for validation, then verify the implementation against those criteria. For multi-step work, state a brief plan with a verification check for each step.
**/*: Always spellNVIDIAin all caps; do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun, because the name begins with an “en” sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company; use trademark symbols with product names only when required by the document type or legal guidance.
Verify official capitalization, spacing, hyphenation, and spelling for NVIDIA and third-party product names; do not rewrite official product names for grammar or title-case rules.
Precede NVIDIA product names withNVIDIAon first mention when natural and accurate, and link the first mention when the destination helps the reader.
On first use, include the company name and full model qualifier when it helps identify the model; preserve official capitalization and punctuation, and use shorter family names only after establishing the full name.
For learning-oriented and developer content, do not force trademark symbols unless explicitly required; for press, ...
Files:
adapters/deepagents/README.mdtests/adapters/test_deepagents.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
**/*.{md,mdx,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,mdx,rst}: For NeMo Fabric documentation, verify technical claims against the current repository, public API, or documented command before reviewing style.
Always spellNVIDIAin all caps; do not useNvidia,nvidia, orNV.
Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text; avoid raw URLs and weak anchors such ashereorread more.
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative, parallel steps; split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English while preserving necessary technical precision.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce, and preferrefer tooverseewhen directing readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical documentation.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values; use numerals for 10 or greater and commas in thousands.
Do not add trademark symbols to learning-oriented documentation unless the source, platform, or legal guidance explicitly requires them.
Do not replace precise technical terms with simpler words when doing so would lose precision.
Do not flag passive voice when the actor is unknown or the action is the important part.
Do not rewrite API names, package names, command flags, or code literals for style.
**/*.{md,mdx,rst}: Use consistent title case for technical-document headings and table headers; avoid quotation marks, ampersands, and exclamation marks in headings, while preserving official product, event, research, and whitepaper title ...
Files:
adapters/deepagents/README.md
**/*.{md,rst,txt,adoc}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-language-mechanics.md)
**/*.{md,rst,txt,adoc}: For technical documentation, use professional, active, conversational, engaging, precise, and plain-English prose. Prefer active voice, present tense, short sentences, and scannable paragraphs. Avoid casual or imprecise language, swearing, threats, insults, jokes, puns, culture-specific idioms, marketing exaggeration, and unsupported third-party comparisons.
Usecanfor possibility and reservemayfor permission; useafterfor temporal order; userefer tofor cross-references; prefer short direct sentences and specific verbs; avoid unnecessarypleasein technical documentation.
Prefer active voice when the actor matters. Passive voice is acceptable when the actor is unknown or irrelevant, when the action or result is the focus, or in programmer documentation.
Use natural contractions in conversational technical prose, but do not force them in formal legal copy, API references, or generated text.
Prefer simpler English over Latinisms: usefor exampleorsuch asinstead ofe.g.,and so oninstead ofetc.,that isinstead ofi.e.,compared toinstead ofvs., andby,through, orusinginstead ofvia. Use industry-standard terms such as in silico, in vitro, and in vivo when appropriate, and italicize them in running text.
Usethatwithout commas for essential clauses, andwhichwith commas for nonessential clauses.
Format dates and times clearly: spell out months in body text; use forms such asJune 12, 2025; avoid numeric or ordinal dates; capitalize days; use 12-hour time when appropriate; include a space beforea.m.orp.m.; useETandPTfor needed time zones; avoid24/7; and preferfrom 12:30 to 1:00 p.m.for prose ranges.
Format numbers consistently: spell out zero through nine in body text, use numerals for 10 or greater and for technical values, use commas in thousands, do not begin a sentence with a numeral, spell out ordinals, and use numerals consistently within a category wh...
Files:
adapters/deepagents/README.md
**/*.{md,mdx,yml,py,rs,sh}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
Keep documentation aligned with current NeMo Fabric behavior, repository layout, entry points, commands, package names, APIs, bindings, and support claims.
Files:
adapters/deepagents/README.mdtests/adapters/test_deepagents.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
**/*.{md,mdx}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
**/*.{md,mdx}: Use the full product nameNVIDIA NeMo Fabricon first use, typically in the title and H1; useNeMo Fabricthereafter. Usefabricalone only for the CLI tool and surround it with backticks.
Treat incorrect or stale commands, package names, paths, APIs, support claims, procedures, examples, terminology, or public behavior documentation as blocking issues.
CapitalizeNVIDIAcorrectly and format code, commands, paths, and filenames as inline code where needed.
Use title case for technical-documentation headings.
Introduce code blocks, tables, and lists with complete lead-in sentences; ensure examples match current APIs and build commands.
Use descriptive anchor text, avoid raw URLs and generic labels such ashere, and use repository-relative.mdxpaths for links withindocs/.
Prefer active voice, present tense, short sentences, plain English, consistent terminology, and imperative, parallel, scannable procedures.
Useafterinstead ofoncewhen expressing temporal sequence, and usecanrather thanmaywhen describing possibility rather than permission.
Avoid ambiguous numeric dates and ordinal dates in body text.
For learning-oriented documentation, do not force trademark symbols unless the source document explicitly requires them.
When reporting documentation-review findings, lead withMust fix,Should fix, andNice to havecategories; include file path, line reference, current problem, rationale, and a concrete rewrite or direction.
Files:
adapters/deepagents/README.md
**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant SDK, API reference, adapter, example, integration, and support documentation when the corresponding public surface changes.
Files:
adapters/deepagents/README.md
**/*.{html,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
HTML and Markdown files must use the specified SPDX HTML-comment header.
Files:
adapters/deepagents/README.md
adapters/*/{README.md,fabric-adapter.json}
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
Document installation, supported configuration, harness-only settings, credentials, lifecycle, telemetry, artifacts, limitations, and focused test commands; keep documentation consistent with descriptor claims.
Files:
adapters/deepagents/README.md
adapters/*/README.md
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
Provide a credential-free fixture covering
plan,doctor, andrun, plus a canonical typed SDK example and, when required, a canonical harness-native YAML fixture.
Files:
adapters/deepagents/README.md
{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 NeMo Fabric contracts.
Files:
adapters/deepagents/README.mdadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
{*.md,**/*.md,**/*.mdx,**/*.ipynb}
⚙️ CodeRabbit configuration file
{*.md,**/*.md,**/*.mdx,**/*.ipynb}: Enforce the product name in user-facing prose: use "NVIDIA NeMo Fabric" on first use and "NeMo Fabric" thereafter. Flag standalone capitalized "Fabric" when it refers to the product. Do not flag the lowercasefabricCLI command, package/import/crate names, code identifiers, API symbols, configuration keys, file paths, or unrelated generic uses of the word.
Files:
adapters/deepagents/README.md
**/*.{rs,py,pyi,json,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Determine and update every affected public surface, including the CLI, PyO3 bindings, Python SDK, type stubs, schemas, and adapter contract, so they remain in parity.
Files:
tests/adapters/test_deepagents.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.Use
snake_casefor functions and variables; usePascalCasefor Rust types and Python classes.
Files:
tests/adapters/test_deepagents.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Python code or a Python-facing adapter changes, run
just test-python.In Python SDK, adapters, examples, and tests, follow the existing style, use type annotations for public APIs, and keep native binding declarations synchronized with their Rust implementations.
Files:
tests/adapters/test_deepagents.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
**/*.{rs,py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*.{rs,py,pyi}: If public configuration types change, confirm schema snapshot tests injust test-rustpass and review generated schema diffs.
For schema or public contract changes, run both language suites and review changes underschemas/and generated API references.
Files:
tests/adapters/test_deepagents.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
tests/adapters/**/*.py
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
tests/adapters/**/*.py: If an adapter or integration changes, run its focused tests.
For adapter behavior changes, run focused adapter tests undertests/adapters, then runjust test-python.
Files:
tests/adapters/test_deepagents.py
**/*.{py,pyi,rs}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
For Python SDK or PyO3 binding changes, use
python-tests, run focused pytest tests first, thenjust test-python; rebuild withjust build-pythonwhen native code or packaging changes.Public contract changes must keep native Python binding declarations synchronized with their Rust implementations.
Files:
tests/adapters/test_deepagents.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
**/*.{rs,py,toml}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
When editing version helpers, verify every
nemo-fabric-*workspace package through Cargo metadata and reject a static version inpython/pyproject.toml.
Files:
tests/adapters/test_deepagents.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
**/*.{toml,rs,py}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
Avoid blind repository-wide replacement of version-like strings; distinguish package-version references from examples and unrelated dependency versions.
Files:
tests/adapters/test_deepagents.pyadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
tests/**/*.{rs,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the corresponding Rust crate or the relevant area under
tests/.
Files:
tests/adapters/test_deepagents.py
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Use pytest to run Python tests.
Do not add@pytest.mark.asyncioto tests; async tests are automatically detected by the async runner.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, useunittest.mock.MagicMockorAsyncMock, using thespecargument when necessary, rather than defining a new class.
Prefix mocked class names withmock, notfake.
Prefer pytest fixtures over helper methods.
If a fixture is needed in multiple test files, define it once inconftest.pyrather than repeating it.
Define fixtures using@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and a<fixture_name>_fixturefunction; specifyscopeonly when it is notfunction.
Preferpytest.mark.parametrizeover separate tests for different input types.
Use@pytest.mark.usefixtureswhen a fixture is needed but its returned value is unused or it returns no value.
Avoid defensive programming in tests; access expected values directly so missing data raises a clear failure, such as usingresults["data"]instead ofresults.get("data").
When adapter installation metadata changes, packaging metadata tests must directly assert that the root project depends unconditionally on the exact-versionnemo-fabric-runtimedistribution.
Packaging metadata tests must verify that each root harness extra delegates to the matching version of the leaf adapter'sharnessextra.
Packaging metadata tests must verify that bare leaf dependencies remain adapter-owned and that the rootadapter-testsdependency group installs each leaf through itsharnessextra.
Packaging metadata tests must verify that every leaf providesfull; only adapters importing NeMo Relay Python APIs providerelay, while adapters using an external Relay executable havefullequal toharness.
Files:
tests/adapters/test_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
adapters/*/src/**/*.py
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
adapters/*/src/**/*.py: Implement adapters using the existing Fabricpythonorprocessrunner and normalized request/result contracts; do not add a runner or one-off abstraction for a single adapter.
Treat normalizedconfig, Fabric-resolved plans, andruntime_contextas authoritative; reserveharness.settingsfor adapter-wide behavior and apply precedence in the order: normalized config, plans/context, harness settings, descriptor/default values.
Reject conflicting duplicate declarations and unsupported behavior with actionable errors naming the field and supported alternatives; never silently drop configuration.
Validate dependency versions, hooks, and credentials before harness invocation, and never expose credential values in outputs, errors, events, logs, or fixtures.
Forward only required system, selected credential, telemetry, and documented harness-specific environment variables; never forward or log unrelated environment values.
Maintain one local adapter host per Fabric runtime across orderedstart→invoke*→stopoperations. Emit one JSON lifecycle response per request on stdout and diagnostics on stderr.
Return harness-level invocation failures as successful lifecycle responses containingresponse: null,failed: true, and structurederrorfields (code,message,retryable, and optionalmetadata).
Do not emit NeMo Relay stream records on adapter stdout; return exactly one terminal lifecycle response while streaming occurs through the SDK-owned out-of-band endpoint.
Scope workspace, generated configuration, state, sessions, and artifacts to the resolved runtime context, and isolate stateful adapter instances by Fabric runtime ID.
Usestartto initialize adapter-owned harness state, retain it for continuation across repeatedinvokecalls on the same runtime, and release it instop.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.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
🪛 Ruff (0.16.1)
tests/adapters/test_deepagents.py
[warning] 416-416: Unused function argument: fake_sdks
(ARG001)
[warning] 416-416: Unused function argument: fake_relay
(ARG001)
[warning] 428-428: Missing return type annotation for private function exploding_scope
(ANN202)
[warning] 428-428: Unused function argument: name
(ARG001)
[warning] 428-428: Unused function argument: scope_type
(ARG001)
[warning] 428-428: Unused function argument: kwargs
(ARG001)
[warning] 430-432: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 450-450: Unused function argument: fake_sdks
(ARG001)
[warning] 450-450: Unused function argument: fake_relay
(ARG001)
[warning] 457-457: Missing return type annotation for private function failing_scope
(ANN202)
[warning] 457-457: Unused function argument: name
(ARG001)
[warning] 457-457: Unused function argument: scope_type
(ARG001)
[warning] 457-457: Unused function argument: kwargs
(ARG001)
[warning] 458-458: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 471-471: Unused function argument: fake_sdks
(ARG001)
[warning] 471-471: Unused function argument: fake_relay
(ARG001)
[warning] 475-475: Missing return type annotation for private function boom
Add return type annotation: Never
(ANN202)
[warning] 476-476: Avoid specifying long messages outside the exception class
(TRY003)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
[warning] 612-612: Do not catch blind exception: Exception
(BLE001)
[warning] 656-656: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (3)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py (1)
638-658: LGTM!tests/adapters/test_deepagents.py (2)
389-412: LGTM!Also applies to: 415-447
470-487: LGTM!
b4a54df to
1a0da88
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/README.md`:
- Around line 211-217: Update the telemetry behavior description to qualify the
completed-result claim: telemetry faults preserve a completed result only when
agent execution succeeds, while an agent invocation failure remains failed even
if telemetry teardown or reporting also fails. Retain the existing description
of recording and joining telemetry faults.
In `@adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py`:
- Around line 616-629: Move callback handler construction from before the try
block into the try block surrounding the telemetry and agent invocation flow in
invoke. Ensure constructor exceptions are caught by the existing telemetry
lifecycle error handling and converted into the required invocation failure
response, and add a test covering a callback handler constructor failure.
🪄 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: 250ad950-2a27-47bb-949f-d0ff175aa11a
📒 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. (17)
- GitHub Check: Pre-commit
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.11, linux-amd64)
- GitHub Check: Test (Python 3.14, linux-amd64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.13, linux-arm64)
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Test (Python 3.14, linux-arm64)
- GitHub Check: Test (Python 3.12, linux-amd64)
- GitHub Check: Test (Python 3.13, linux-amd64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Test (Python 3.14, macos-arm64)
- GitHub Check: Test (Python 3.11, linux-arm64)
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.12, linux-arm64)
🧰 Additional context used
📓 Path-based instructions (24)
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Update documentation and examples in the same branch as the public API change.
Files:
adapters/deepagents/README.md
**/*
📄 CodeRabbit inference engine (.agents/skills/karpathy-guidelines/SKILL.md)
**/*: Before implementing, explicitly state assumptions, surface ambiguity and tradeoffs, present multiple interpretations when relevant, and ask for clarification rather than silently deciding or proceeding when requirements are unclear.
Prefer the minimum code needed to solve the requested problem: avoid speculative features, unnecessary abstractions, unrequested flexibility, and handling of impossible scenarios; simplify overcomplicated solutions.
When editing existing code, make surgical changes only: do not modify unrelated code, comments, formatting, or pre-existing dead code; match the existing style, and remove only unused imports, variables, or functions introduced by your changes.
Define verifiable success criteria for each task, such as writing regression tests for bugs and invalid-input tests for validation, then verify the implementation against those criteria. For multi-step work, state a brief plan with a verification check for each step.
**/*: Always spellNVIDIAin all caps; do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun, because the name begins with an “en” sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company; use trademark symbols with product names only when required by the document type or legal guidance.
Verify official capitalization, spacing, hyphenation, and spelling for NVIDIA and third-party product names; do not rewrite official product names for grammar or title-case rules.
Precede NVIDIA product names withNVIDIAon first mention when natural and accurate, and link the first mention when the destination helps the reader.
On first use, include the company name and full model qualifier when it helps identify the model; preserve official capitalization and punctuation, and use shorter family names only after establishing the full name.
For learning-oriented and developer content, do not force trademark symbols unless explicitly required; for press, ...
Files:
adapters/deepagents/README.mdadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{md,mdx,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,mdx,rst}: For NeMo Fabric documentation, verify technical claims against the current repository, public API, or documented command before reviewing style.
Always spellNVIDIAin all caps; do not useNvidia,nvidia, orNV.
Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text; avoid raw URLs and weak anchors such ashereorread more.
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative, parallel steps; split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English while preserving necessary technical precision.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce, and preferrefer tooverseewhen directing readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical documentation.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values; use numerals for 10 or greater and commas in thousands.
Do not add trademark symbols to learning-oriented documentation unless the source, platform, or legal guidance explicitly requires them.
Do not replace precise technical terms with simpler words when doing so would lose precision.
Do not flag passive voice when the actor is unknown or the action is the important part.
Do not rewrite API names, package names, command flags, or code literals for style.
**/*.{md,mdx,rst}: Use consistent title case for technical-document headings and table headers; avoid quotation marks, ampersands, and exclamation marks in headings, while preserving official product, event, research, and whitepaper title ...
Files:
adapters/deepagents/README.md
**/*.{md,rst,txt,adoc}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-language-mechanics.md)
**/*.{md,rst,txt,adoc}: For technical documentation, use professional, active, conversational, engaging, precise, and plain-English prose. Prefer active voice, present tense, short sentences, and scannable paragraphs. Avoid casual or imprecise language, swearing, threats, insults, jokes, puns, culture-specific idioms, marketing exaggeration, and unsupported third-party comparisons.
Usecanfor possibility and reservemayfor permission; useafterfor temporal order; userefer tofor cross-references; prefer short direct sentences and specific verbs; avoid unnecessarypleasein technical documentation.
Prefer active voice when the actor matters. Passive voice is acceptable when the actor is unknown or irrelevant, when the action or result is the focus, or in programmer documentation.
Use natural contractions in conversational technical prose, but do not force them in formal legal copy, API references, or generated text.
Prefer simpler English over Latinisms: usefor exampleorsuch asinstead ofe.g.,and so oninstead ofetc.,that isinstead ofi.e.,compared toinstead ofvs., andby,through, orusinginstead ofvia. Use industry-standard terms such as in silico, in vitro, and in vivo when appropriate, and italicize them in running text.
Usethatwithout commas for essential clauses, andwhichwith commas for nonessential clauses.
Format dates and times clearly: spell out months in body text; use forms such asJune 12, 2025; avoid numeric or ordinal dates; capitalize days; use 12-hour time when appropriate; include a space beforea.m.orp.m.; useETandPTfor needed time zones; avoid24/7; and preferfrom 12:30 to 1:00 p.m.for prose ranges.
Format numbers consistently: spell out zero through nine in body text, use numerals for 10 or greater and for technical values, use commas in thousands, do not begin a sentence with a numeral, spell out ordinals, and use numerals consistently within a category wh...
Files:
adapters/deepagents/README.md
**/*.{md,mdx,yml,py,rs,sh}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
Keep documentation aligned with current NeMo Fabric behavior, repository layout, entry points, commands, package names, APIs, bindings, and support claims.
Files:
adapters/deepagents/README.mdadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{md,mdx}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
**/*.{md,mdx}: Use the full product nameNVIDIA NeMo Fabricon first use, typically in the title and H1; useNeMo Fabricthereafter. Usefabricalone only for the CLI tool and surround it with backticks.
Treat incorrect or stale commands, package names, paths, APIs, support claims, procedures, examples, terminology, or public behavior documentation as blocking issues.
CapitalizeNVIDIAcorrectly and format code, commands, paths, and filenames as inline code where needed.
Use title case for technical-documentation headings.
Introduce code blocks, tables, and lists with complete lead-in sentences; ensure examples match current APIs and build commands.
Use descriptive anchor text, avoid raw URLs and generic labels such ashere, and use repository-relative.mdxpaths for links withindocs/.
Prefer active voice, present tense, short sentences, plain English, consistent terminology, and imperative, parallel, scannable procedures.
Useafterinstead ofoncewhen expressing temporal sequence, and usecanrather thanmaywhen describing possibility rather than permission.
Avoid ambiguous numeric dates and ordinal dates in body text.
For learning-oriented documentation, do not force trademark symbols unless the source document explicitly requires them.
When reporting documentation-review findings, lead withMust fix,Should fix, andNice to havecategories; include file path, line reference, current problem, rationale, and a concrete rewrite or direction.
Files:
adapters/deepagents/README.md
**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant SDK, API reference, adapter, example, integration, and support documentation when the corresponding public surface changes.
Files:
adapters/deepagents/README.md
**/*.{html,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
HTML and Markdown files must use the specified SPDX HTML-comment header.
Files:
adapters/deepagents/README.md
adapters/*/{README.md,fabric-adapter.json}
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
Document installation, supported configuration, harness-only settings, credentials, lifecycle, telemetry, artifacts, limitations, and focused test commands; keep documentation consistent with descriptor claims.
Files:
adapters/deepagents/README.md
adapters/*/README.md
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
Provide a credential-free fixture covering
plan,doctor, andrun, plus a canonical typed SDK example and, when required, a canonical harness-native YAML fixture.
Files:
adapters/deepagents/README.md
{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 NeMo Fabric contracts.
Files:
adapters/deepagents/README.mdadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
{*.md,**/*.md,**/*.mdx,**/*.ipynb}
⚙️ CodeRabbit configuration file
{*.md,**/*.md,**/*.mdx,**/*.ipynb}: Enforce the product name in user-facing prose: use "NVIDIA NeMo Fabric" on first use and "NeMo Fabric" thereafter. Flag standalone capitalized "Fabric" when it refers to the product. Do not flag the lowercasefabricCLI command, package/import/crate names, code identifiers, API symbols, configuration keys, file paths, or unrelated generic uses of the word.
Files:
adapters/deepagents/README.md
**/*.{rs,py,pyi,json,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Determine and update every affected public surface, including the CLI, PyO3 bindings, Python SDK, type stubs, schemas, and adapter contract, so they remain in parity.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.Use
snake_casefor functions and variables; usePascalCasefor Rust types and Python classes.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Python code or a Python-facing adapter changes, run
just test-python.In Python SDK, adapters, examples, and tests, follow the existing style, use type annotations for public APIs, and keep native binding declarations synchronized with their Rust implementations.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{rs,py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*.{rs,py,pyi}: If public configuration types change, confirm schema snapshot tests injust test-rustpass and review generated schema diffs.
For schema or public contract changes, run both language suites and review changes underschemas/and generated API references.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{py,pyi,rs}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
For Python SDK or PyO3 binding changes, use
python-tests, run focused pytest tests first, thenjust test-python; rebuild withjust build-pythonwhen native code or packaging changes.Public contract changes must keep native Python binding declarations synchronized with their Rust implementations.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{rs,py,toml}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
When editing version helpers, verify every
nemo-fabric-*workspace package through Cargo metadata and reject a static version inpython/pyproject.toml.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{toml,rs,py}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
Avoid blind repository-wide replacement of version-like strings; distinguish package-version references from examples and unrelated dependency versions.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
adapters/*/src/**/*.py
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
adapters/*/src/**/*.py: Implement adapters using the existing Fabricpythonorprocessrunner and normalized request/result contracts; do not add a runner or one-off abstraction for a single adapter.
Treat normalizedconfig, Fabric-resolved plans, andruntime_contextas authoritative; reserveharness.settingsfor adapter-wide behavior and apply precedence in the order: normalized config, plans/context, harness settings, descriptor/default values.
Reject conflicting duplicate declarations and unsupported behavior with actionable errors naming the field and supported alternatives; never silently drop configuration.
Validate dependency versions, hooks, and credentials before harness invocation, and never expose credential values in outputs, errors, events, logs, or fixtures.
Forward only required system, selected credential, telemetry, and documented harness-specific environment variables; never forward or log unrelated environment values.
Maintain one local adapter host per Fabric runtime across orderedstart→invoke*→stopoperations. Emit one JSON lifecycle response per request on stdout and diagnostics on stderr.
Return harness-level invocation failures as successful lifecycle responses containingresponse: null,failed: true, and structurederrorfields (code,message,retryable, and optionalmetadata).
Do not emit NeMo Relay stream records on adapter stdout; return exactly one terminal lifecycle response while streaming occurs through the SDK-owned out-of-band endpoint.
Scope workspace, generated configuration, state, sessions, and artifacts to the resolved runtime context, and isolate stateful adapter instances by Fabric runtime ID.
Usestartto initialize adapter-owned harness state, retain it for continuation across repeatedinvokecalls on the same runtime, and release it instop.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
tests/adapters/**/*.py
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
tests/adapters/**/*.py: If an adapter or integration changes, run its focused tests.
For adapter behavior changes, run focused adapter tests undertests/adapters, then runjust test-python.
Files:
tests/adapters/test_deepagents.py
tests/**/*.{rs,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the corresponding Rust crate or the relevant area under
tests/.
Files:
tests/adapters/test_deepagents.py
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Use pytest to run Python tests.
Do not add@pytest.mark.asyncioto tests; async tests are automatically detected by the async runner.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, useunittest.mock.MagicMockorAsyncMock, using thespecargument when necessary, rather than defining a new class.
Prefix mocked class names withmock, notfake.
Prefer pytest fixtures over helper methods.
If a fixture is needed in multiple test files, define it once inconftest.pyrather than repeating it.
Define fixtures using@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and a<fixture_name>_fixturefunction; specifyscopeonly when it is notfunction.
Preferpytest.mark.parametrizeover separate tests for different input types.
Use@pytest.mark.usefixtureswhen a fixture is needed but its returned value is unused or it returns no value.
Avoid defensive programming in tests; access expected values directly so missing data raises a clear failure, such as usingresults["data"]instead ofresults.get("data").
When adapter installation metadata changes, packaging metadata tests must directly assert that the root project depends unconditionally on the exact-versionnemo-fabric-runtimedistribution.
Packaging metadata tests must verify that each root harness extra delegates to the matching version of the leaf adapter'sharnessextra.
Packaging metadata tests must verify that bare leaf dependencies remain adapter-owned and that the rootadapter-testsdependency group installs each leaf through itsharnessextra.
Packaging metadata tests must verify that every leaf providesfull; only adapters importing NeMo Relay Python APIs providerelay, while adapters using an external Relay executable havefullequal toharness.
Files:
tests/adapters/test_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
🪛 Ruff (0.16.1)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
[warning] 629-629: Do not catch blind exception: Exception
(BLE001)
[warning] 659-659: Do not catch blind exception: Exception
(BLE001)
[warning] 691-691: Do not catch blind exception: Exception
(BLE001)
tests/adapters/test_deepagents.py
[warning] 416-416: Unused function argument: fake_sdks
(ARG001)
[warning] 416-416: Unused function argument: fake_relay
(ARG001)
[warning] 428-428: Missing return type annotation for private function exploding_scope
(ANN202)
[warning] 428-428: Unused function argument: name
(ARG001)
[warning] 428-428: Unused function argument: scope_type
(ARG001)
[warning] 428-428: Unused function argument: kwargs
(ARG001)
[warning] 430-432: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 451-451: Unused function argument: fake_sdks
(ARG001)
[warning] 451-451: Unused function argument: fake_relay
(ARG001)
[warning] 462-462: Missing return type annotation for private function exploding_plugin
(ANN202)
[warning] 462-462: Unused function argument: config
(ARG001)
[warning] 464-464: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 479-479: Unused function argument: fake_sdks
(ARG001)
[warning] 479-479: Unused function argument: fake_relay
(ARG001)
[warning] 488-488: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 502-502: Unused function argument: fake_sdks
(ARG001)
[warning] 502-502: Unused function argument: fake_relay
(ARG001)
[warning] 509-509: Missing return type annotation for private function exploding_scope
(ANN202)
[warning] 509-509: Unused function argument: name
(ARG001)
[warning] 509-509: Unused function argument: scope_type
(ARG001)
[warning] 509-509: Unused function argument: kwargs
(ARG001)
[warning] 511-511: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 514-514: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 528-528: Unused function argument: fake_sdks
(ARG001)
[warning] 528-528: Unused function argument: fake_relay
(ARG001)
[warning] 535-535: Missing return type annotation for private function failing_scope
(ANN202)
[warning] 535-535: Unused function argument: name
(ARG001)
[warning] 535-535: Unused function argument: scope_type
(ARG001)
[warning] 535-535: Unused function argument: kwargs
(ARG001)
[warning] 536-536: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 552-552: Unused function argument: fake_sdks
(ARG001)
[warning] 552-552: Unused function argument: fake_relay
(ARG001)
[warning] 556-556: Missing return type annotation for private function boom
Add return type annotation: Never
(ANN202)
[warning] 557-557: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 572-572: Unused function argument: fake_sdks
(ARG001)
[warning] 572-572: Unused function argument: fake_relay
(ARG001)
[warning] 578-578: Missing return type annotation for private function boom
Add return type annotation: Never
(ANN202)
[warning] 579-579: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 582-582: Missing return type annotation for private function exploding_scope
(ANN202)
[warning] 582-582: Unused function argument: name
(ARG001)
[warning] 582-582: Unused function argument: scope_type
(ARG001)
[warning] 582-582: Unused function argument: kwargs
(ARG001)
[warning] 584-584: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (4)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py (2)
809-810: Do Not Return Raw Exception Text.
_error_text()places arbitrary exception text into lifecycle output. Exceptions can include plugin configuration or credential values. This remains unresolved from the earlier review finding.As per coding guidelines, “never expose credential values in outputs, errors, events, logs, or fixtures.”
Source: Coding guidelines
573-595: LGTM!tests/adapters/test_deepagents.py (2)
527-548: Assert That Scope Setup Blocks Agent Execution.The test checks the returned error, but it does not prove that the agent was not called. Stub
adapter.invoke_compiled_agentwithAsyncMockand assert that it was not awaited.As per path instructions, “Tests should cover the behavior promised by the changed API surface.”
Source: Path instructions
389-412: LGTM!
1a0da88 to
b33a174
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/README.md`:
- Around line 220-224: Update the documentation around the telemetry.degraded
artifact behavior to distinguish teardown failures from artifact-collection
failures: artifacts may still be referenced when teardown does not complete
cleanly, but are unavailable when collection fails. Replace the unconditional
degraded-run artifact claim while preserving the guidance to treat degraded
trajectories as untrusted.
In `@adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py`:
- Around line 612-629: Update the lifecycle handling around _relay_plugin.plugin
and _relay_scope.scope so scope-exit and plugin __aexit__ exceptions are
captured independently rather than allowing the later plugin failure to replace
the scope failure. Combine both telemetry faults with _join_faults, preserve the
existing outcome behavior, and add a test covering simultaneous failures from
both teardown operations.
🪄 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: 33c8c148-5080-4f57-a6d1-0df15494c9f4
📒 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. (17)
- GitHub Check: Test (Python 3.14, linux-amd64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.13, linux-amd64)
- GitHub Check: Test (Python 3.14, macos-arm64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.13, linux-arm64)
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.14, linux-arm64)
- GitHub Check: Test (Python 3.12, linux-arm64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.12, linux-amd64)
- GitHub Check: Test (Python 3.11, linux-amd64)
- GitHub Check: Test (Python 3.11, linux-arm64)
- GitHub Check: Pre-commit
🧰 Additional context used
📓 Path-based instructions (24)
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Update documentation and examples in the same branch as the public API change.
Files:
adapters/deepagents/README.md
**/*
📄 CodeRabbit inference engine (.agents/skills/karpathy-guidelines/SKILL.md)
**/*: Before implementing, explicitly state assumptions, surface ambiguity and tradeoffs, present multiple interpretations when relevant, and ask for clarification rather than silently deciding or proceeding when requirements are unclear.
Prefer the minimum code needed to solve the requested problem: avoid speculative features, unnecessary abstractions, unrequested flexibility, and handling of impossible scenarios; simplify overcomplicated solutions.
When editing existing code, make surgical changes only: do not modify unrelated code, comments, formatting, or pre-existing dead code; match the existing style, and remove only unused imports, variables, or functions introduced by your changes.
Define verifiable success criteria for each task, such as writing regression tests for bugs and invalid-input tests for validation, then verify the implementation against those criteria. For multi-step work, state a brief plan with a verification check for each step.
**/*: Always spellNVIDIAin all caps; do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun, because the name begins with an “en” sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company; use trademark symbols with product names only when required by the document type or legal guidance.
Verify official capitalization, spacing, hyphenation, and spelling for NVIDIA and third-party product names; do not rewrite official product names for grammar or title-case rules.
Precede NVIDIA product names withNVIDIAon first mention when natural and accurate, and link the first mention when the destination helps the reader.
On first use, include the company name and full model qualifier when it helps identify the model; preserve official capitalization and punctuation, and use shorter family names only after establishing the full name.
For learning-oriented and developer content, do not force trademark symbols unless explicitly required; for press, ...
Files:
adapters/deepagents/README.mdadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{md,mdx,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,mdx,rst}: For NeMo Fabric documentation, verify technical claims against the current repository, public API, or documented command before reviewing style.
Always spellNVIDIAin all caps; do not useNvidia,nvidia, orNV.
Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text; avoid raw URLs and weak anchors such ashereorread more.
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative, parallel steps; split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English while preserving necessary technical precision.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce, and preferrefer tooverseewhen directing readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical documentation.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values; use numerals for 10 or greater and commas in thousands.
Do not add trademark symbols to learning-oriented documentation unless the source, platform, or legal guidance explicitly requires them.
Do not replace precise technical terms with simpler words when doing so would lose precision.
Do not flag passive voice when the actor is unknown or the action is the important part.
Do not rewrite API names, package names, command flags, or code literals for style.
**/*.{md,mdx,rst}: Use consistent title case for technical-document headings and table headers; avoid quotation marks, ampersands, and exclamation marks in headings, while preserving official product, event, research, and whitepaper title ...
Files:
adapters/deepagents/README.md
**/*.{md,rst,txt,adoc}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-language-mechanics.md)
**/*.{md,rst,txt,adoc}: For technical documentation, use professional, active, conversational, engaging, precise, and plain-English prose. Prefer active voice, present tense, short sentences, and scannable paragraphs. Avoid casual or imprecise language, swearing, threats, insults, jokes, puns, culture-specific idioms, marketing exaggeration, and unsupported third-party comparisons.
Usecanfor possibility and reservemayfor permission; useafterfor temporal order; userefer tofor cross-references; prefer short direct sentences and specific verbs; avoid unnecessarypleasein technical documentation.
Prefer active voice when the actor matters. Passive voice is acceptable when the actor is unknown or irrelevant, when the action or result is the focus, or in programmer documentation.
Use natural contractions in conversational technical prose, but do not force them in formal legal copy, API references, or generated text.
Prefer simpler English over Latinisms: usefor exampleorsuch asinstead ofe.g.,and so oninstead ofetc.,that isinstead ofi.e.,compared toinstead ofvs., andby,through, orusinginstead ofvia. Use industry-standard terms such as in silico, in vitro, and in vivo when appropriate, and italicize them in running text.
Usethatwithout commas for essential clauses, andwhichwith commas for nonessential clauses.
Format dates and times clearly: spell out months in body text; use forms such asJune 12, 2025; avoid numeric or ordinal dates; capitalize days; use 12-hour time when appropriate; include a space beforea.m.orp.m.; useETandPTfor needed time zones; avoid24/7; and preferfrom 12:30 to 1:00 p.m.for prose ranges.
Format numbers consistently: spell out zero through nine in body text, use numerals for 10 or greater and for technical values, use commas in thousands, do not begin a sentence with a numeral, spell out ordinals, and use numerals consistently within a category wh...
Files:
adapters/deepagents/README.md
**/*.{md,mdx,yml,py,rs,sh}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
Keep documentation aligned with current NeMo Fabric behavior, repository layout, entry points, commands, package names, APIs, bindings, and support claims.
Files:
adapters/deepagents/README.mdadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{md,mdx}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
**/*.{md,mdx}: Use the full product nameNVIDIA NeMo Fabricon first use, typically in the title and H1; useNeMo Fabricthereafter. Usefabricalone only for the CLI tool and surround it with backticks.
Treat incorrect or stale commands, package names, paths, APIs, support claims, procedures, examples, terminology, or public behavior documentation as blocking issues.
CapitalizeNVIDIAcorrectly and format code, commands, paths, and filenames as inline code where needed.
Use title case for technical-documentation headings.
Introduce code blocks, tables, and lists with complete lead-in sentences; ensure examples match current APIs and build commands.
Use descriptive anchor text, avoid raw URLs and generic labels such ashere, and use repository-relative.mdxpaths for links withindocs/.
Prefer active voice, present tense, short sentences, plain English, consistent terminology, and imperative, parallel, scannable procedures.
Useafterinstead ofoncewhen expressing temporal sequence, and usecanrather thanmaywhen describing possibility rather than permission.
Avoid ambiguous numeric dates and ordinal dates in body text.
For learning-oriented documentation, do not force trademark symbols unless the source document explicitly requires them.
When reporting documentation-review findings, lead withMust fix,Should fix, andNice to havecategories; include file path, line reference, current problem, rationale, and a concrete rewrite or direction.
Files:
adapters/deepagents/README.md
**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant SDK, API reference, adapter, example, integration, and support documentation when the corresponding public surface changes.
Files:
adapters/deepagents/README.md
**/*.{html,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
HTML and Markdown files must use the specified SPDX HTML-comment header.
Files:
adapters/deepagents/README.md
adapters/*/{README.md,fabric-adapter.json}
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
Document installation, supported configuration, harness-only settings, credentials, lifecycle, telemetry, artifacts, limitations, and focused test commands; keep documentation consistent with descriptor claims.
Files:
adapters/deepagents/README.md
adapters/*/README.md
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
Provide a credential-free fixture covering
plan,doctor, andrun, plus a canonical typed SDK example and, when required, a canonical harness-native YAML fixture.
Files:
adapters/deepagents/README.md
{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 NeMo Fabric contracts.
Files:
adapters/deepagents/README.mdadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
{*.md,**/*.md,**/*.mdx,**/*.ipynb}
⚙️ CodeRabbit configuration file
{*.md,**/*.md,**/*.mdx,**/*.ipynb}: Enforce the product name in user-facing prose: use "NVIDIA NeMo Fabric" on first use and "NeMo Fabric" thereafter. Flag standalone capitalized "Fabric" when it refers to the product. Do not flag the lowercasefabricCLI command, package/import/crate names, code identifiers, API symbols, configuration keys, file paths, or unrelated generic uses of the word.
Files:
adapters/deepagents/README.md
**/*.{rs,py,pyi,json,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Determine and update every affected public surface, including the CLI, PyO3 bindings, Python SDK, type stubs, schemas, and adapter contract, so they remain in parity.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.Use
snake_casefor functions and variables; usePascalCasefor Rust types and Python classes.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Python code or a Python-facing adapter changes, run
just test-python.In Python SDK, adapters, examples, and tests, follow the existing style, use type annotations for public APIs, and keep native binding declarations synchronized with their Rust implementations.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{rs,py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*.{rs,py,pyi}: If public configuration types change, confirm schema snapshot tests injust test-rustpass and review generated schema diffs.
For schema or public contract changes, run both language suites and review changes underschemas/and generated API references.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{py,pyi,rs}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
For Python SDK or PyO3 binding changes, use
python-tests, run focused pytest tests first, thenjust test-python; rebuild withjust build-pythonwhen native code or packaging changes.Public contract changes must keep native Python binding declarations synchronized with their Rust implementations.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{rs,py,toml}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
When editing version helpers, verify every
nemo-fabric-*workspace package through Cargo metadata and reject a static version inpython/pyproject.toml.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{toml,rs,py}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
Avoid blind repository-wide replacement of version-like strings; distinguish package-version references from examples and unrelated dependency versions.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
adapters/*/src/**/*.py
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
adapters/*/src/**/*.py: Implement adapters using the existing Fabricpythonorprocessrunner and normalized request/result contracts; do not add a runner or one-off abstraction for a single adapter.
Treat normalizedconfig, Fabric-resolved plans, andruntime_contextas authoritative; reserveharness.settingsfor adapter-wide behavior and apply precedence in the order: normalized config, plans/context, harness settings, descriptor/default values.
Reject conflicting duplicate declarations and unsupported behavior with actionable errors naming the field and supported alternatives; never silently drop configuration.
Validate dependency versions, hooks, and credentials before harness invocation, and never expose credential values in outputs, errors, events, logs, or fixtures.
Forward only required system, selected credential, telemetry, and documented harness-specific environment variables; never forward or log unrelated environment values.
Maintain one local adapter host per Fabric runtime across orderedstart→invoke*→stopoperations. Emit one JSON lifecycle response per request on stdout and diagnostics on stderr.
Return harness-level invocation failures as successful lifecycle responses containingresponse: null,failed: true, and structurederrorfields (code,message,retryable, and optionalmetadata).
Do not emit NeMo Relay stream records on adapter stdout; return exactly one terminal lifecycle response while streaming occurs through the SDK-owned out-of-band endpoint.
Scope workspace, generated configuration, state, sessions, and artifacts to the resolved runtime context, and isolate stateful adapter instances by Fabric runtime ID.
Usestartto initialize adapter-owned harness state, retain it for continuation across repeatedinvokecalls on the same runtime, and release it instop.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
tests/adapters/**/*.py
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
tests/adapters/**/*.py: If an adapter or integration changes, run its focused tests.
For adapter behavior changes, run focused adapter tests undertests/adapters, then runjust test-python.
Files:
tests/adapters/test_deepagents.py
tests/**/*.{rs,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the corresponding Rust crate or the relevant area under
tests/.
Files:
tests/adapters/test_deepagents.py
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Use pytest to run Python tests.
Do not add@pytest.mark.asyncioto tests; async tests are automatically detected by the async runner.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, useunittest.mock.MagicMockorAsyncMock, using thespecargument when necessary, rather than defining a new class.
Prefix mocked class names withmock, notfake.
Prefer pytest fixtures over helper methods.
If a fixture is needed in multiple test files, define it once inconftest.pyrather than repeating it.
Define fixtures using@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and a<fixture_name>_fixturefunction; specifyscopeonly when it is notfunction.
Preferpytest.mark.parametrizeover separate tests for different input types.
Use@pytest.mark.usefixtureswhen a fixture is needed but its returned value is unused or it returns no value.
Avoid defensive programming in tests; access expected values directly so missing data raises a clear failure, such as usingresults["data"]instead ofresults.get("data").
When adapter installation metadata changes, packaging metadata tests must directly assert that the root project depends unconditionally on the exact-versionnemo-fabric-runtimedistribution.
Packaging metadata tests must verify that each root harness extra delegates to the matching version of the leaf adapter'sharnessextra.
Packaging metadata tests must verify that bare leaf dependencies remain adapter-owned and that the rootadapter-testsdependency group installs each leaf through itsharnessextra.
Packaging metadata tests must verify that every leaf providesfull; only adapters importing NeMo Relay Python APIs providerelay, while adapters using an external Relay executable havefullequal toharness.
Files:
tests/adapters/test_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
🪛 Ruff (0.16.1)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
[warning] 622-622: Do not catch blind exception: Exception
(BLE001)
[warning] 646-646: Do not catch blind exception: Exception
(BLE001)
[warning] 676-676: Do not catch blind exception: Exception
(BLE001)
tests/adapters/test_deepagents.py
[warning] 416-416: Unused function argument: fake_sdks
(ARG001)
[warning] 416-416: Unused function argument: fake_relay
(ARG001)
[warning] 428-428: Missing return type annotation for private function exploding_scope
(ANN202)
[warning] 428-428: Unused function argument: name
(ARG001)
[warning] 428-428: Unused function argument: scope_type
(ARG001)
[warning] 428-428: Unused function argument: kwargs
(ARG001)
[warning] 430-432: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 451-451: Unused function argument: fake_sdks
(ARG001)
[warning] 451-451: Unused function argument: fake_relay
(ARG001)
[warning] 462-462: Missing return type annotation for private function exploding_plugin
(ANN202)
[warning] 462-462: Unused function argument: config
(ARG001)
[warning] 464-464: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 479-479: Unused function argument: fake_sdks
(ARG001)
[warning] 479-479: Unused function argument: fake_relay
(ARG001)
[warning] 488-488: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 502-502: Unused function argument: fake_sdks
(ARG001)
[warning] 502-502: Unused function argument: fake_relay
(ARG001)
[warning] 509-509: Missing return type annotation for private function exploding_scope
(ANN202)
[warning] 509-509: Unused function argument: name
(ARG001)
[warning] 509-509: Unused function argument: scope_type
(ARG001)
[warning] 509-509: Unused function argument: kwargs
(ARG001)
[warning] 511-511: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 514-514: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 528-528: Unused function argument: fake_sdks
(ARG001)
[warning] 528-528: Unused function argument: fake_relay
(ARG001)
[warning] 535-535: Missing return type annotation for private function failing_scope
(ANN202)
[warning] 535-535: Unused function argument: name
(ARG001)
[warning] 535-535: Unused function argument: scope_type
(ARG001)
[warning] 535-535: Unused function argument: kwargs
(ARG001)
[warning] 536-536: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 556-556: Unused function argument: fake_sdks
(ARG001)
[warning] 556-556: Unused function argument: fake_relay
(ARG001)
[warning] 560-560: Missing return type annotation for private function boom
Add return type annotation: Never
(ANN202)
[warning] 561-561: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 576-576: Unused function argument: fake_sdks
(ARG001)
[warning] 576-576: Unused function argument: fake_relay
(ARG001)
[warning] 582-582: Missing return type annotation for private function boom
Add return type annotation: Never
(ANN202)
[warning] 583-583: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 586-586: Missing return type annotation for private function exploding_scope
(ANN202)
[warning] 586-586: Unused function argument: name
(ARG001)
[warning] 586-586: Unused function argument: scope_type
(ARG001)
[warning] 586-586: Unused function argument: kwargs
(ARG001)
[warning] 588-588: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (2)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py (2)
609-612: Move callback handler construction into the lifecycletryblock.
self._callback_handler_type()can raise before Line 611. That bypasses the required invocation failure response for telemetry setup failures. Add a regression test for a failing callback handler constructor.
790-791: Do Not Return Raw Exception Text.
_error_text()copies exception messages into both invocation and telemetry output. Exceptions can contain credential or plugin configuration values. Return an allowlisted code or fixed safe message, and add a redaction regression test.As per coding guidelines, “never expose credential values in outputs, errors, events, logs, or fixtures.”
Source: Coding guidelines
|
The 41.38% is measured over the whole file rather than the diff; 5 of the 6 functions this PR adds have docstrings. Raising the full module to 80% would be a large unrelated diff, but happy to do it as a follow-up. |
b33a174 to
324c53c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/src/nemo_fabric_adapters/deepagents/adapter.py`:
- Around line 574-577: The invocation branch currently treats native and Relay
telemetry identically, causing native-only runs to depend on Relay. Update the
adapter’s startup and invocation flow around _configure_observability(),
_invoke_with_telemetry(), and self._telemetry_provider so Relay setup and
telemetry are selected only for provider "relay", while native configurations
use the native path; add a regression test that exercises native telemetry
without nemo_relay installed.
In `@tests/adapters/test_deepagents.py`:
- Around line 510-532: Extend
test_callback_handler_construction_failure_is_a_normalized_failure to stub
adapter.invoke_compiled_agent with an AsyncMock and assert it is not awaited
when callback construction fails. Also assert the response telemetry marks
degraded as true and telemetry.error contains the callback construction failure,
while preserving the existing normalized invocation error assertions.
🪄 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: 0645c070-c0be-4700-b1c2-afdde7675fc0
📒 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. (17)
- GitHub Check: Test (Python 3.14, macos-arm64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.14, linux-amd64)
- GitHub Check: Test (Python 3.12, linux-amd64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.12, linux-arm64)
- GitHub Check: Test (Python 3.13, linux-arm64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.14, linux-arm64)
- GitHub Check: Test (Python 3.11, linux-amd64)
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Test (Python 3.11, linux-arm64)
- GitHub Check: Test (Python 3.13, linux-amd64)
- GitHub Check: Pre-commit
🧰 Additional context used
📓 Path-based instructions (24)
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Update documentation and examples in the same branch as the public API change.
Files:
adapters/deepagents/README.md
**/*
📄 CodeRabbit inference engine (.agents/skills/karpathy-guidelines/SKILL.md)
**/*: Before implementing, explicitly state assumptions, surface ambiguity and tradeoffs, present multiple interpretations when relevant, and ask for clarification rather than silently deciding or proceeding when requirements are unclear.
Prefer the minimum code needed to solve the requested problem: avoid speculative features, unnecessary abstractions, unrequested flexibility, and handling of impossible scenarios; simplify overcomplicated solutions.
When editing existing code, make surgical changes only: do not modify unrelated code, comments, formatting, or pre-existing dead code; match the existing style, and remove only unused imports, variables, or functions introduced by your changes.
Define verifiable success criteria for each task, such as writing regression tests for bugs and invalid-input tests for validation, then verify the implementation against those criteria. For multi-step work, state a brief plan with a verification check for each step.
**/*: Always spellNVIDIAin all caps; do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun, because the name begins with an “en” sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company; use trademark symbols with product names only when required by the document type or legal guidance.
Verify official capitalization, spacing, hyphenation, and spelling for NVIDIA and third-party product names; do not rewrite official product names for grammar or title-case rules.
Precede NVIDIA product names withNVIDIAon first mention when natural and accurate, and link the first mention when the destination helps the reader.
On first use, include the company name and full model qualifier when it helps identify the model; preserve official capitalization and punctuation, and use shorter family names only after establishing the full name.
For learning-oriented and developer content, do not force trademark symbols unless explicitly required; for press, ...
Files:
adapters/deepagents/README.mdadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{md,mdx,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,mdx,rst}: For NeMo Fabric documentation, verify technical claims against the current repository, public API, or documented command before reviewing style.
Always spellNVIDIAin all caps; do not useNvidia,nvidia, orNV.
Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text; avoid raw URLs and weak anchors such ashereorread more.
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative, parallel steps; split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English while preserving necessary technical precision.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce, and preferrefer tooverseewhen directing readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical documentation.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values; use numerals for 10 or greater and commas in thousands.
Do not add trademark symbols to learning-oriented documentation unless the source, platform, or legal guidance explicitly requires them.
Do not replace precise technical terms with simpler words when doing so would lose precision.
Do not flag passive voice when the actor is unknown or the action is the important part.
Do not rewrite API names, package names, command flags, or code literals for style.
**/*.{md,mdx,rst}: Use consistent title case for technical-document headings and table headers; avoid quotation marks, ampersands, and exclamation marks in headings, while preserving official product, event, research, and whitepaper title ...
Files:
adapters/deepagents/README.md
**/*.{md,rst,txt,adoc}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-language-mechanics.md)
**/*.{md,rst,txt,adoc}: For technical documentation, use professional, active, conversational, engaging, precise, and plain-English prose. Prefer active voice, present tense, short sentences, and scannable paragraphs. Avoid casual or imprecise language, swearing, threats, insults, jokes, puns, culture-specific idioms, marketing exaggeration, and unsupported third-party comparisons.
Usecanfor possibility and reservemayfor permission; useafterfor temporal order; userefer tofor cross-references; prefer short direct sentences and specific verbs; avoid unnecessarypleasein technical documentation.
Prefer active voice when the actor matters. Passive voice is acceptable when the actor is unknown or irrelevant, when the action or result is the focus, or in programmer documentation.
Use natural contractions in conversational technical prose, but do not force them in formal legal copy, API references, or generated text.
Prefer simpler English over Latinisms: usefor exampleorsuch asinstead ofe.g.,and so oninstead ofetc.,that isinstead ofi.e.,compared toinstead ofvs., andby,through, orusinginstead ofvia. Use industry-standard terms such as in silico, in vitro, and in vivo when appropriate, and italicize them in running text.
Usethatwithout commas for essential clauses, andwhichwith commas for nonessential clauses.
Format dates and times clearly: spell out months in body text; use forms such asJune 12, 2025; avoid numeric or ordinal dates; capitalize days; use 12-hour time when appropriate; include a space beforea.m.orp.m.; useETandPTfor needed time zones; avoid24/7; and preferfrom 12:30 to 1:00 p.m.for prose ranges.
Format numbers consistently: spell out zero through nine in body text, use numerals for 10 or greater and for technical values, use commas in thousands, do not begin a sentence with a numeral, spell out ordinals, and use numerals consistently within a category wh...
Files:
adapters/deepagents/README.md
**/*.{md,mdx,yml,py,rs,sh}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
Keep documentation aligned with current NeMo Fabric behavior, repository layout, entry points, commands, package names, APIs, bindings, and support claims.
Files:
adapters/deepagents/README.mdadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{md,mdx}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
**/*.{md,mdx}: Use the full product nameNVIDIA NeMo Fabricon first use, typically in the title and H1; useNeMo Fabricthereafter. Usefabricalone only for the CLI tool and surround it with backticks.
Treat incorrect or stale commands, package names, paths, APIs, support claims, procedures, examples, terminology, or public behavior documentation as blocking issues.
CapitalizeNVIDIAcorrectly and format code, commands, paths, and filenames as inline code where needed.
Use title case for technical-documentation headings.
Introduce code blocks, tables, and lists with complete lead-in sentences; ensure examples match current APIs and build commands.
Use descriptive anchor text, avoid raw URLs and generic labels such ashere, and use repository-relative.mdxpaths for links withindocs/.
Prefer active voice, present tense, short sentences, plain English, consistent terminology, and imperative, parallel, scannable procedures.
Useafterinstead ofoncewhen expressing temporal sequence, and usecanrather thanmaywhen describing possibility rather than permission.
Avoid ambiguous numeric dates and ordinal dates in body text.
For learning-oriented documentation, do not force trademark symbols unless the source document explicitly requires them.
When reporting documentation-review findings, lead withMust fix,Should fix, andNice to havecategories; include file path, line reference, current problem, rationale, and a concrete rewrite or direction.
Files:
adapters/deepagents/README.md
**/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant SDK, API reference, adapter, example, integration, and support documentation when the corresponding public surface changes.
Files:
adapters/deepagents/README.md
**/*.{html,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
HTML and Markdown files must use the specified SPDX HTML-comment header.
Files:
adapters/deepagents/README.md
adapters/*/{README.md,fabric-adapter.json}
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
Document installation, supported configuration, harness-only settings, credentials, lifecycle, telemetry, artifacts, limitations, and focused test commands; keep documentation consistent with descriptor claims.
Files:
adapters/deepagents/README.md
adapters/*/README.md
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
Provide a credential-free fixture covering
plan,doctor, andrun, plus a canonical typed SDK example and, when required, a canonical harness-native YAML fixture.
Files:
adapters/deepagents/README.md
{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 NeMo Fabric contracts.
Files:
adapters/deepagents/README.mdadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
{*.md,**/*.md,**/*.mdx,**/*.ipynb}
⚙️ CodeRabbit configuration file
{*.md,**/*.md,**/*.mdx,**/*.ipynb}: Enforce the product name in user-facing prose: use "NVIDIA NeMo Fabric" on first use and "NeMo Fabric" thereafter. Flag standalone capitalized "Fabric" when it refers to the product. Do not flag the lowercasefabricCLI command, package/import/crate names, code identifiers, API symbols, configuration keys, file paths, or unrelated generic uses of the word.
Files:
adapters/deepagents/README.md
**/*.{rs,py,pyi,json,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Determine and update every affected public surface, including the CLI, PyO3 bindings, Python SDK, type stubs, schemas, and adapter contract, so they remain in parity.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.Use
snake_casefor functions and variables; usePascalCasefor Rust types and Python classes.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Python code or a Python-facing adapter changes, run
just test-python.In Python SDK, adapters, examples, and tests, follow the existing style, use type annotations for public APIs, and keep native binding declarations synchronized with their Rust implementations.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{rs,py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*.{rs,py,pyi}: If public configuration types change, confirm schema snapshot tests injust test-rustpass and review generated schema diffs.
For schema or public contract changes, run both language suites and review changes underschemas/and generated API references.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{py,pyi,rs}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
For Python SDK or PyO3 binding changes, use
python-tests, run focused pytest tests first, thenjust test-python; rebuild withjust build-pythonwhen native code or packaging changes.Public contract changes must keep native Python binding declarations synchronized with their Rust implementations.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{rs,py,toml}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
When editing version helpers, verify every
nemo-fabric-*workspace package through Cargo metadata and reject a static version inpython/pyproject.toml.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
**/*.{toml,rs,py}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
Avoid blind repository-wide replacement of version-like strings; distinguish package-version references from examples and unrelated dependency versions.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pytests/adapters/test_deepagents.py
adapters/*/src/**/*.py
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
adapters/*/src/**/*.py: Implement adapters using the existing Fabricpythonorprocessrunner and normalized request/result contracts; do not add a runner or one-off abstraction for a single adapter.
Treat normalizedconfig, Fabric-resolved plans, andruntime_contextas authoritative; reserveharness.settingsfor adapter-wide behavior and apply precedence in the order: normalized config, plans/context, harness settings, descriptor/default values.
Reject conflicting duplicate declarations and unsupported behavior with actionable errors naming the field and supported alternatives; never silently drop configuration.
Validate dependency versions, hooks, and credentials before harness invocation, and never expose credential values in outputs, errors, events, logs, or fixtures.
Forward only required system, selected credential, telemetry, and documented harness-specific environment variables; never forward or log unrelated environment values.
Maintain one local adapter host per Fabric runtime across orderedstart→invoke*→stopoperations. Emit one JSON lifecycle response per request on stdout and diagnostics on stderr.
Return harness-level invocation failures as successful lifecycle responses containingresponse: null,failed: true, and structurederrorfields (code,message,retryable, and optionalmetadata).
Do not emit NeMo Relay stream records on adapter stdout; return exactly one terminal lifecycle response while streaming occurs through the SDK-owned out-of-band endpoint.
Scope workspace, generated configuration, state, sessions, and artifacts to the resolved runtime context, and isolate stateful adapter instances by Fabric runtime ID.
Usestartto initialize adapter-owned harness state, retain it for continuation across repeatedinvokecalls on the same runtime, and release it instop.
Files:
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
tests/adapters/**/*.py
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
tests/adapters/**/*.py: If an adapter or integration changes, run its focused tests.
For adapter behavior changes, run focused adapter tests undertests/adapters, then runjust test-python.
Files:
tests/adapters/test_deepagents.py
tests/**/*.{rs,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the corresponding Rust crate or the relevant area under
tests/.
Files:
tests/adapters/test_deepagents.py
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Use pytest to run Python tests.
Do not add@pytest.mark.asyncioto tests; async tests are automatically detected by the async runner.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, useunittest.mock.MagicMockorAsyncMock, using thespecargument when necessary, rather than defining a new class.
Prefix mocked class names withmock, notfake.
Prefer pytest fixtures over helper methods.
If a fixture is needed in multiple test files, define it once inconftest.pyrather than repeating it.
Define fixtures using@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and a<fixture_name>_fixturefunction; specifyscopeonly when it is notfunction.
Preferpytest.mark.parametrizeover separate tests for different input types.
Use@pytest.mark.usefixtureswhen a fixture is needed but its returned value is unused or it returns no value.
Avoid defensive programming in tests; access expected values directly so missing data raises a clear failure, such as usingresults["data"]instead ofresults.get("data").
When adapter installation metadata changes, packaging metadata tests must directly assert that the root project depends unconditionally on the exact-versionnemo-fabric-runtimedistribution.
Packaging metadata tests must verify that each root harness extra delegates to the matching version of the leaf adapter'sharnessextra.
Packaging metadata tests must verify that bare leaf dependencies remain adapter-owned and that the rootadapter-testsdependency group installs each leaf through itsharnessextra.
Packaging metadata tests must verify that every leaf providesfull; only adapters importing NeMo Relay Python APIs providerelay, while adapters using an external Relay executable havefullequal toharness.
Files:
tests/adapters/test_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
🪛 Ruff (0.16.1)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py
[warning] 627-627: Do not catch blind exception: Exception
(BLE001)
[warning] 629-629: Do not catch blind exception: Exception
(BLE001)
[warning] 653-653: Do not catch blind exception: Exception
(BLE001)
[warning] 683-683: Do not catch blind exception: Exception
(BLE001)
tests/adapters/test_deepagents.py
[warning] 416-416: Unused function argument: fake_sdks
(ARG001)
[warning] 416-416: Unused function argument: fake_relay
(ARG001)
[warning] 428-428: Missing return type annotation for private function exploding_scope
(ANN202)
[warning] 428-428: Unused function argument: name
(ARG001)
[warning] 428-428: Unused function argument: scope_type
(ARG001)
[warning] 428-428: Unused function argument: kwargs
(ARG001)
[warning] 430-432: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 451-451: Unused function argument: fake_sdks
(ARG001)
[warning] 451-451: Unused function argument: fake_relay
(ARG001)
[warning] 462-462: Missing return type annotation for private function exploding_plugin
(ANN202)
[warning] 462-462: Unused function argument: config
(ARG001)
[warning] 464-464: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 479-479: Unused function argument: fake_sdks
(ARG001)
[warning] 479-479: Unused function argument: fake_relay
(ARG001)
[warning] 489-489: Missing return type annotation for private function exploding_scope
(ANN202)
[warning] 489-489: Unused function argument: name
(ARG001)
[warning] 489-489: Unused function argument: scope_type
(ARG001)
[warning] 489-489: Unused function argument: kwargs
(ARG001)
[warning] 491-491: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 494-494: Missing return type annotation for private function exploding_plugin
(ANN202)
[warning] 494-494: Unused function argument: config
(ARG001)
[warning] 496-496: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 511-511: Unused function argument: fake_sdks
(ARG001)
[warning] 511-511: Unused function argument: fake_relay
(ARG001)
[warning] 519-519: Missing return type annotation for private function exploding_handler
Add return type annotation: Never
(ANN202)
[warning] 520-520: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 536-536: Unused function argument: fake_sdks
(ARG001)
[warning] 536-536: Unused function argument: fake_relay
(ARG001)
[warning] 545-545: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 559-559: Unused function argument: fake_sdks
(ARG001)
[warning] 559-559: Unused function argument: fake_relay
(ARG001)
[warning] 566-566: Missing return type annotation for private function exploding_scope
(ANN202)
[warning] 566-566: Unused function argument: name
(ARG001)
[warning] 566-566: Unused function argument: scope_type
(ARG001)
[warning] 566-566: Unused function argument: kwargs
(ARG001)
[warning] 568-568: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 571-571: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 585-585: Unused function argument: fake_sdks
(ARG001)
[warning] 585-585: Unused function argument: fake_relay
(ARG001)
[warning] 592-592: Missing return type annotation for private function failing_scope
(ANN202)
[warning] 592-592: Unused function argument: name
(ARG001)
[warning] 592-592: Unused function argument: scope_type
(ARG001)
[warning] 592-592: Unused function argument: kwargs
(ARG001)
[warning] 593-593: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 613-613: Unused function argument: fake_sdks
(ARG001)
[warning] 613-613: Unused function argument: fake_relay
(ARG001)
[warning] 617-617: Missing return type annotation for private function boom
Add return type annotation: Never
(ANN202)
[warning] 618-618: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 633-633: Unused function argument: fake_sdks
(ARG001)
[warning] 633-633: Unused function argument: fake_relay
(ARG001)
[warning] 639-639: Missing return type annotation for private function boom
Add return type annotation: Never
(ANN202)
[warning] 640-640: Avoid specifying long messages outside the exception class
(TRY003)
[warning] 643-643: Missing return type annotation for private function exploding_scope
(ANN202)
[warning] 643-643: Unused function argument: name
(ARG001)
[warning] 643-643: Unused function argument: scope_type
(ARG001)
[warning] 643-643: Unused function argument: kwargs
(ARG001)
[warning] 645-645: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (4)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py (1)
579-596: LGTM!Also applies to: 598-637, 639-685, 785-809, 862-896
tests/adapters/test_deepagents.py (2)
389-508: LGTM!
535-658: LGTM!adapters/deepagents/README.md (1)
211-227: LGTM!
324c53c to
ca23a80
Compare
AjayThorve
left a comment
There was a problem hiding this comment.
The agent-outcome/telemetry-outcome split is the right containment boundary, and the real-agent runs confirm that response and workspace effects survive the teardown fault. I am requesting changes only for cross-invocation containment: the first degraded turn currently leaves Relay state corrupted while a later turn can be reported clean.
The concurrency/root fix likely belongs in NeMo Relay, not in this adapter. Current Relay 0.7.x has task-context forking primitives from NVIDIA/NeMo-Relay#589, but its LangChain callback does not apply them at overlapping sibling-run boundaries. This Fabric PR should establish a safe post-fault invariant and link/track the Relay-side fix rather than reimplementing Relay internals here.
The full NVBug acceptance still requires the Relay fix, complete ATOF/ATIF without scope-pop errors, and the Platform test matrices. Once the persistent-runtime invariant below is addressed, I would be comfortable merging this as containment while the Relay fix proceeds.
`DeepAgentsRuntime.invoke()` wrapped the Relay plugin/scope context managers and
the agent call in one `try/except Exception`. Because a context manager's
`__exit__` runs inside that `try`, an exception raised purely while *closing* the
telemetry scope was assigned to the invocation `error`, and `normalize_output`
reported `completed=false` / `failed=true` for a turn the agent had already
finished — final response, tool calls, and workspace changes all intact.
This is reachable today. NeMo Relay keeps one process-global LIFO scope stack in
a `ContextVar`; LangGraph schedules child tasks with `copy_context()`, which
shares that same mutable stack, so concurrent chain callbacks can close out of
LIFO order and Relay's validator rejects the pop with
RuntimeError: invalid argument: scope handle is not at the top of the stack
The stranded child scope then makes the outer `deepagents-request` scope raise on
exit. Downstream (NVBug 6562846) this scored 31 otherwise-successful agent-eval
trials as adapter invocation failures. Deep Agents is where this is reachable
because it is the only adapter that installs a LangChain callback handler, whose
nested per-chain scopes are what LangGraph's concurrent tasks close out of order.
Separate the two failure domains structurally. `TurnOutcome` carries `error` (the
agent failed) and `telemetry_error` (recording the turn failed) as independent
fields. `_invoke_agent` owns the invocation domain; `_invoke_with_telemetry` owns
the telemetry one and guards nothing but the context managers, so an exception it
catches can only be a telemetry fault. Whether the agent got to run is answered by
whether an outcome exists, not by a side flag: no outcome means telemetry failed
on the way in and there is nothing to preserve, so that stays an invocation
failure and the split cannot widen into general failure suppression. Scope and
plugin teardown are caught separately, because an exception crossing the plugin's
`__aexit__` is replaced by any fault the plugin raises in turn.
Artifact collection is covered too. It walks the filesystem after the turn is
over, so letting it raise would discard a completed invocation — the same failure
mode this change exists to stop. `_telemetry_output` returns that fault instead of
raising, and faults from multiple stages are joined rather than the first winning.
Relay's scope stack outlives an invocation, so preserving the turn is not enough
on its own: a fault that leaves a scope current poisons the runtime, and a later,
properly nested turn would otherwise report itself telemetry-clean while running
under the stale scope. Compare the current scope handle against the one captured
before the turn; a mismatch quarantines the runtime. Quarantined turns keep
running and stay completed, but open no request scope, reference no artifacts of
their own, and always report degraded. The quarantine survives `stop()`/`start()`,
because restarting does not clean the process's scope stack. This is containment,
not repair: the Relay middleware attached at start still emits under the stale
scope, and the real fix belongs in NeMo Relay.
Report a fault as `telemetry.degraded: true` alongside `telemetry.error`, so a
consumer has a machine-readable signal rather than a message to parse. On the turn
the fault happened `telemetry.error` carries it verbatim; turns that inherit the
quarantine report it as `telemetry.quarantine_cause` instead, so a consumer
matching per-turn errors does not see one fault reported once per remaining turn.
Both keys are absent on a clean run.
Signed-off-by: Sandy Chapman <schapman@nvidia.com>
ca23a80 to
a1c02cf
Compare
AjayThorve
left a comment
There was a problem hiding this comment.
Validated the current head: the persistent-runtime containment now keeps poisoned runtimes degraded across ordered invokes and stop/start, and the real Relay callback regression covers the A/B/A-end/B-end leak. Focused validation passed 60 tests; just test-python passed 730 tests with 16 skipped; hosted checks are green.
Approving this as the Fabric containment fix. The root scope-stack repair remains owned by NeMo Relay. I left one non-blocking P2 inline for the faulting-turn artifact contract.
|
/merge |
`invoke()` reads the quarantine state from before the turn, but `_telemetry_output` consulted `self._telemetry_quarantine` directly. By the time it ran, `_invoke_with_telemetry` had already set that flag on the turn whose fault caused the quarantine, so that turn suppressed its own `relay_artifacts` as well. That contradicts the documented contract. A turn that opened a request scope and faulted during teardown produced its own partial trajectory, so it publishes it flagged as degraded; only turns that inherit the quarantine have none of their own and must not claim an earlier turn's. Pass the pre-invocation state in rather than re-reading the field afterwards. The existing regression test asserted only that later turns withhold artifacts, which is why this went unnoticed. It now stubs a recognisable artifact and asserts by value, because real collection over an empty evidence directory returns an empty list — a presence-only assertion passed either way. Reported by @AjayThorve in review of NVIDIA#191. Signed-off-by: Sandy Chapman <schapman@nvidia.com>
…#204) #### Overview Follow-up to #191, addressing the P2 @AjayThorve raised in review after it had merged. `invoke()` captures the quarantine state from *before* the turn, but `_telemetry_output` consulted `self._telemetry_quarantine` directly. By the time it runs, `_invoke_with_telemetry` has already set that flag on the turn whose fault caused the quarantine — so that turn suppressed its own `relay_artifacts` too, reproducing as `faulting_turn_artifacts=None`. That contradicts the contract #191 documented: a scope- or flush-degraded turn still references its partial artifacts, and only turns that *inherit* the quarantine omit them. #### Where should the reviewer start? `_telemetry_output` in `adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py`, which now takes `inherited_quarantine` as a parameter rather than re-reading the field afterwards. The distinction it encodes: a turn that opened a request scope and faulted during teardown produced its own partial trajectory, so it publishes it flagged as degraded; a turn that inherits the quarantine opened no scope and has nothing of its own, so it must not claim an earlier turn's artifacts. Then the assertion change in `test_a_dirty_scope_stack_quarantines_telemetry_for_later_turns`. The original test asserted only that later turns withhold artifacts and said nothing about the faulting turn, which is why this went unnoticed. Strengthening it exposed a second weakness worth flagging: that test does not stub `collect_relay_artifacts`, so real collection over an empty evidence directory returns `[]` — a presence-only check like `"relay_artifacts" in first` passes against an implementation that publishes an empty list forever. It now stubs a recognisable artifact and asserts by value. Both directions are mutation-checked: restoring the original `self._telemetry_quarantine` read fails the test with `KeyError: 'relay_artifacts'`, and removing the suppression entirely fails it because the inherited turn would publish the earlier turn's artifacts as its own. #### Testing - `just test-python` (full suite on this base) — 702 passed, 53 skipped. - `tests/adapters/test_deepagents.py` — 56 passed. - `ruff check` / `ruff format --check` (v0.15.21, matching `.pre-commit-config.yaml`) and the copyright-header hook pass on both changed files. No behaviour changes beyond the artifact policy, and no API changes. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to #191 - Relates to NVBug 6562846 - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. ## Summary by CodeRabbit * **Bug Fixes** * Telemetry artifacts are now published for the invocation that triggers quarantine. * Subsequent quarantined invocations no longer collect or publish telemetry artifacts. * Improved telemetry behavior when handling Relay scope quarantine conditions. Authors: - Sandy Chapman (https://github.com/SandyChapman) Approvers: - Ajay Thorve (https://github.com/AjayThorve) URL: #204
…755) #### Overview > Targets `release/0.7` so this can go into a 0.7.3 cut for Fabric 0.2 **What.** When a LangChain chain run ends, the handler records it as completed and closes finished scopes from the top of the stack down, instead of attempting a close that the stack may reject and then abandoning the scope. **Why.** Relay closes scopes strictly LIFO, but LangGraph schedules sibling chain runs as concurrent asyncio tasks that share one scope stack, so two siblings can finish in an order the stack rejects: A starts, B starts, A ends, B ends. `_pop_scope` removed the handle from `_scope_handles` *before* attempting the pop and then swallowed the rejection, so the scope stayed live on the stack with nothing left able to close it. It remained current for everything that followed, and the caller's own enclosing scope raised on exit: ``` RuntimeError: invalid argument: scope handle is not at the top of the stack ``` Downstream this reported fully successful agent work as failed (NVBug 6562846). The enclosing scope belonged to a NeMo Fabric Deep Agents invocation, whose adapter turned a teardown error into an invocation failure (since contained separately in NVIDIA/NeMo-Fabric#191) — 31 otherwise-successful agent-eval trials were scored as adapter failures in one regression run, including at parallelism 1. - [x] I confirm this contribution is my own work, or I have the right to submit it under this project's license. - [x] I searched existing issues and open pull requests, and this does not duplicate existing work. #### Details Each run's handle is tracked from `on_chain_start`. When a run ends, its prepared output, metadata and end timestamp are recorded as completed state, and finished scopes are closed from the top of the active stack down: read `scope.get_handle()`, close it when its uuid belongs to a completed run of ours, stop as soon as the top is a run still going or a scope this handler does not own. Only the current top is ever closed, so the stack is never asked to accept a close it would reject. There is no speculative pop, hence no failure to classify and no dependency on the runtime's error text. **On "drain only the owning stack".** Implemented, then removed, with evidence. A stack-identity check *cannot* be written against the public API: `set_thread_scope_stack` genuinely shares the stack (a push in the thread is visible outside it) but returns a **different Python wrapper**, and `ScopeStack` exposes no id and only identity equality. A handler completing a run on a worker thread would therefore park every completion and strand every scope — the original bug, reintroduced by the guard. Dropping it also removes a strong reference to a `ScopeStack` per retained completion. Covered by `test_a_run_completed_on_a_worker_thread_still_closes`. Cross-stack safety is therefore structural, not an ownership guard: a completion is matched on the top scope's uuid **and** name. uuid alone is not sufficient — a stack rebuilt from a `PropagationContext` re-creates a scope carrying an existing uuid, so a uuid-only match pops the stand-in, discards the completion and strands the real scope on its own stack (`test_a_propagated_stack_does_not_consume_another_stacks_completion`). The two are distinguishable because the runtime names rebuilt scopes `propagated-parent` and `propagated-root` (`scope_stack.rs`), which `test_propagated_stand_ins_keep_their_reserved_names` pins so a rename fails loudly rather than silently closing the wrong scope. Residual: a chain run named exactly one of those two reserved names would still collide. **@AjayThorve's findings.** Output is prepared when the callback fires, so later mutation cannot change emitted telemetry. Cross-stack loss is structural rather than guarded, per above. **@willkill07's second review.** Output is prepared *before* the run is dropped from the handler's map and degrades to `None` on failure — serializing walks caller data and a cyclic output raises `RecursionError`; a run removed with no completion recorded is a scope nothing can close. A completion is deleted only once the stack accepts the close, since the runtime can refuse before mutating the stack. Handler state transitions are serialized under a lock. Reading the current handle is guarded by `scope_stack_active()`, which does not create a stack, so completing a run in a context without one no longer materialises an empty stack there. #### Where should the reviewer start? `_close_completed_scopes_locked` in `python/nemo_relay/integrations/langchain/callbacks.py` — the loop that reads the top and stops as soon as it is not a completed run of ours. Note the locked/unlocked split: `_pop_scope` holds the lock and calls the locked form, so the lock is never re-acquired on the normal path. That was a real fragility — an earlier revision nested the acquisition and only worked because the lock was reentrant. Then the tests, which use a real scope stack. `test_callbacks.py` mocks `nemo_relay`, and its mock accepted any pop in any order, so it could not observe the LIFO rule this fix turns on; it now models the stack and rejects closing anything but the top. The LangGraph tests drive real compiled graphs with no stubbing: a two-branch fan-out, a nested three-branch fan-out with a subgraph, and a failing node completing through `on_chain_error`. #### Testing - `python/tests/integrations` — 64 passed, 2 skipped. - `python/tests` (excluding `plugin/` and `test_dynamic_plugin_host.py`, which need `just build-test-plugin-fixtures`) — 545 passed, 2 skipped. - Pre-commit on the changed files: copyright, ruff, ruff-format, ty all pass. - Mutation checks, each caught by a named test: closing the top regardless of ownership (13 tests fail); a non-reentrant lock; serializing output at close time; letting output serialization escape; deleting a completion before the pop succeeds; dropping the `scope_stack_active` guard; and a handler that pushes no scopes at all. - Every new test was run against the unpatched upstream handler: 19 of 21 fail, and the 2 that pass are the ones that should — the harness control and the topology test, neither of which depends on this change. Of the 19, nine fail behaviourally (the scope-stack error, or its absence); the rest reference internals that do not exist upstream, so they guard against regressions in this implementation rather than detecting the original defect. - End to end against the original symptom: built this branch as a wheel and installed it into a NeMo Fabric checkout. Fabric's own tests that assert the leak *exists* flip to failing, and its Deep Agents mitigation for this bug never engages because there is nothing left to mitigate. A wheel built from unpatched `release/0.7` was used as the control, and confirms the flip is caused by this change rather than by version skew. #### Remaining limitations - **A scope closed out of band** can never return to the top, so its completion is retained for the life of the handler. That is the trade for never guessing at error text; the entry is inert and everything around it still closes. Documented on the type and in a test. - **A sibling that never ends at all** — a cancelled task with no `on_chain_error` — still leaves its parent waiting. Unchanged by this PR, and better than an abandoned scope since the state is visible. - Untested: runs that never end retaining their `_scope_handles` entry, sync (non-async) LangGraph execution, and very wide fan-out as a performance question. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to internal NVBug ## Summary by CodeRabbit ## Bug Fixes - Improved reliability when concurrent workflow steps complete out of order. - Preserved outputs, metadata, and completion timestamps during deferred processing. - Prevented lingering workflow state after concurrent or nested executions. - Improved handling of failed workflow steps and branch cleanup. - Increased reliability for concurrent LangGraph branch execution. ## Tests - Added coverage for nested, overlapping, and out-of-order workflow lifecycles. - Added regression coverage for concurrent branches, shared handlers, worker-thread completion, and cleanup behavior. Authors: - Sandy Chapman (https://github.com/SandyChapman) Approvers: - Will Killian (https://github.com/willkill07) URL: #755
Overview
DeepAgentsRuntime.invoke()wrapped the Relay plugin/scope context managers and the agent call in a singletry/except Exception. A context manager's__exit__runs inside thattry, so an exception raised purely while closing the telemetry scope was assigned to the invocationerror, andnormalize_outputthen reportedcompleted=false/failed=truefor a turn the agent had already finished — final response, tool calls, and workspace changes all intact.This is reachable today. NeMo Relay keeps one process-global LIFO scope stack in a
ContextVar; LangGraph schedules child tasks withcopy_context(), which preserves that same mutable stack rather than cloning it, so concurrent chain callbacks can close out of LIFO order and Relay's scope validator rejects the pop with:The stranded child scope then makes the outer
deepagents-requestscope raise on exit. Downstream this scored 31 otherwise-successful NeMo Evaluator agent-eval trials as adapter invocation failures (NVBug 6562846), including at evaluator parallelism 1. Deep Agents is where this is reachable because it is the only adapter that installs a LangChain callback handler, and its nested per-chain scopes are what LangGraph's concurrent tasks close out of order.This PR separates the two failure domains structurally rather than papering over the Relay bug:
TurnOutcomecarrieserror(the agent failed, no functional result) andtelemetry_error(recording the turn failed) as independent fields._invoke_agentowns the invocation domain._invoke_with_telemetryowns the telemetry one and guards nothing but the context managers, so an exception it catches can only be a telemetry fault._telemetry_outputreturns that fault instead of raising, and faults from multiple stages are joined rather than the first one winning.telemetry.degraded: trueaccompaniestelemetry.error, so a consumer has a machine-readable signal instead of a message to parse. Artifacts are still referenced when degraded — a partial trajectory is usually worth reading — but the flag marks them as possibly truncated. Both keys are absent on a clean run, and the block is emitted for a fault even without a runtime block so the diagnostic cannot be silently dropped.The Relay-side root cause (shared scope stack across concurrent LangGraph tasks; a failed
scope.popstranding an untracked scope) is being reported separately against NeMo-Relay. This change is worth making on its own regardless: an observability fault should never rewrite the functional outcome of a completed invocation.Where should the reviewer start?
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py—TurnOutcomeand the_invoke_with_telemetry/_invoke_agentsplit. The design decision to check is theoutcome is Nonetest in_invoke_with_telemetry: it is what distinguishes "telemetry broke on the way out, after a completed turn" (recoverable) from "telemetry broke on the way in, so the agent never ran" (a real failure).The contract question worth a maintainer's opinion is the artifact policy: on a degraded run this references artifacts anyway and flags them, rather than withholding them. Withholding is the other defensible choice; happy to switch if you'd rather a degraded run expose nothing.
Then
tests/adapters/test_deepagents.py, which pins each branch: scope-teardown fault keeps the turn completed; plugin-flush fault likewise; artifact-collection fault likewise; two faults in one turn are both reported; a setup fault stays an invocation failure; a genuine agent failure still fails with a clean telemetry block; and agent failure plus teardown fault keep their own domains.Review follow-ups from this round: the completed-result claim in the README was overstated (a telemetry fault after an agent failure correctly leaves the turn failed) and is now qualified; the setup-failure test asserts
invoke_compiled_agentwas never awaited, so a regression that consumes an agent request before reporting the fault is caught.Not applied: redacting the exception text in
telemetry.error. The concern is real, but this adapter already returns rawstr(exc)in the invocationerrorand has since before this PR, and that path is the likelier credential carrier (a provider client error can echo abase_url). Redacting only the new field would be inconsistent and would remove the diagnostic the field exists for. If the exception text should be sanitized, it belongs in one place across the normalized result — happy to open that separately.Testing:
just test-python— 680 passed, 53 skipped.tests/adapters/test_deepagents.py— 49 passed (42 before, 7 new).ruff check/ruff format --check(v0.15.21, matching.pre-commit-config.yaml) and the copyright-header hook pass on all changed files.Not addressed here, flagged rather than fixed:
_invoke_agentcollapses an exception tof"{type}: {exc}"and discards the traceback, which is pre-existing behavior and this module has no logger to route it to. Separately,tests/adapters/test_claude_adapter.py::test_build_options_maps_normalized_capabilities_and_claude_settingsfails wheneverANTHROPIC_BASE_URLis set in the environment — unrelated to this PR, but it bites anyone running the suite from a shell that exports it, and amonkeypatch.delenvwould fix it.No breaking changes: the normalized result gains optional
telemetry.degraded/telemetry.errorkeys and is otherwise unchanged, andnormalize_output's newtelemetry_errorparameter is keyword-only with a default.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Relates to NVBug 6562846
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
Bug Fixes
Documentation