feat: move Claude adapter to v1alpha2 - #197
Conversation
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
|
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 Claude adapter now consumes validated ChangesClaude typed configuration migration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Lifecycle serving
participant ClaudeRuntime
participant RuntimeContext
participant Relay gateway
participant Claude SDK
Lifecycle serving->>ClaudeRuntime: Load AgentConfig.from_mapping
ClaudeRuntime->>RuntimeContext: Validate runtime context
ClaudeRuntime->>Relay gateway: Start Relay with typed configuration
ClaudeRuntime->>Claude SDK: Build options and invoke query
Claude SDK-->>ClaudeRuntime: Return result messages
ClaudeRuntime-->>Lifecycle serving: Return normalized result
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/adapters/test_claude_adapter.py`:
- Line 543: Update the pytest.raises assertion using the match argument in the
relevant test to escape the period in “SKILL.md”, so the regex requires a
literal dot while preserving the existing expected error.
🪄 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: 6f913ae5-9e45-47fe-aab4-9056d2166098
⛔ Files ignored due to path filters (2)
adapters/claude/uv.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
adapters/claude/fabric-adapter.jsonadapters/claude/pyproject.tomladapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/common/src/nemo_fabric_adapters/common/utils.pycrates/fabric-cli/assets/adapters/claude/fabric-adapter.jsontests/adapters/test_adapter_package_metadata.pytests/adapters/test_claude_adapter.py
📜 Review details
⏰ Context from checks skipped due to timeout. (15)
- GitHub Check: Preview docs
- GitHub Check: Test (Python 3.14, macos-arm64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.13, linux-arm64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.11, linux-arm64)
- GitHub Check: Test (Python 3.11, linux-amd64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Test (Python 3.14, linux-amd64)
- GitHub Check: Test (Python 3.14, linux-arm64)
- GitHub Check: Test (Python 3.12, linux-arm64)
- GitHub Check: Test (Python 3.12, linux-amd64)
- GitHub Check: Pre-commit
🧰 Additional context used
📓 Path-based instructions (28)
**/*.{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/claude/fabric-adapter.jsoncrates/fabric-cli/assets/adapters/claude/fabric-adapter.jsontests/adapters/test_adapter_package_metadata.pyadapters/common/src/nemo_fabric_adapters/common/utils.pytests/adapters/test_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
**/*
📄 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/claude/fabric-adapter.jsonadapters/claude/pyproject.tomlcrates/fabric-cli/assets/adapters/claude/fabric-adapter.jsontests/adapters/test_adapter_package_metadata.pyadapters/common/src/nemo_fabric_adapters/common/utils.pytests/adapters/test_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
**/*.{json,jsonschema}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Public contract changes must keep checked-in JSON Schema snapshots synchronized.
Files:
adapters/claude/fabric-adapter.jsoncrates/fabric-cli/assets/adapters/claude/fabric-adapter.json
adapters/*/fabric-adapter.json
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
Define a narrow, truthful adapter descriptor; keep
config.accepts,config.generates, requirements, telemetry declarations, lifecycle capabilities, and advertised capabilities synchronized with implementation and tests.
Files:
adapters/claude/fabric-adapter.json
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/claude/fabric-adapter.json
{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/claude/fabric-adapter.jsonadapters/claude/pyproject.tomladapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For any Rust change, run
just test-rustandcargo fmt --all -- --check.For Rust core, CLI, or shared runtime semantic changes, run Rust formatting and tests, and add Python tests when behavior is exposed through the SDK.
Use Rust stable tooling; format Rust code with
cargo fmt --all, verify formatting withcargo fmt --all -- --check, and compile withcargo check --workspace --locked.
Files:
adapters/claude/pyproject.toml
**/{Cargo.toml,Cargo.lock,pyproject.toml,package.json}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For new or updated dependencies, document the functional need, alternatives considered, and why the selected dependency is the narrowest fit.
Files:
adapters/claude/pyproject.toml
**/*.{toml,lock}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a manifest or lockfile changes, run the license-diff script against
origin/main, review transitive license changes, and run theattributions-rustandattributions-pythonpre-commit hooks.
Files:
adapters/claude/pyproject.toml
**/*.{yml,yaml,toml,lock}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
For CI or packaging changes, use
maintain-ciormaintain-packaging, then run recipes and checks whose behavior changed.
Files:
adapters/claude/pyproject.toml
{pyproject.toml,adapters/**/pyproject.toml}
📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)
{pyproject.toml,adapters/**/pyproject.toml}: Update the literalproject.versionin the root setuptools project and every adapterpyproject.toml.
Keep internal exact-version requirements aligned: rootnemo-fabric-* == <version>optional dependencies and each adapter'snemo-fabric-adapters-common == <version>dependency.
Files:
adapters/claude/pyproject.toml
**/*.{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/claude/pyproject.tomltests/adapters/test_adapter_package_metadata.pyadapters/common/src/nemo_fabric_adapters/common/utils.pytests/adapters/test_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/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:
adapters/claude/pyproject.tomltests/adapters/test_adapter_package_metadata.pyadapters/common/src/nemo_fabric_adapters/common/utils.pytests/adapters/test_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
**/*.{toml,yaml,yml,sh,bash}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
TOML, YAML, and shell files must use the specified
#SPDX copyright and Apache-2.0 license headers.
Files:
adapters/claude/pyproject.toml
**/{Cargo.toml,pyproject.toml}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
**/{Cargo.toml,pyproject.toml}: Keep package names, dependency declarations, import paths, module names, and workspace or Python package metadata internally consistent.
Prefer the standard library, an existing dependency, or a small local implementation before adding a new direct dependency.
When multiple dependencies satisfy the requirement, prefer a maintained OSS option with clear SPDX metadata, a smaller transitive graph, and permissive licensing such as Apache-2.0, MIT, BSD, or ISC.
Files:
adapters/claude/pyproject.toml
**/{Cargo.toml,pyproject.toml,Cargo.lock,uv.lock}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
**/{Cargo.toml,pyproject.toml,Cargo.lock,uv.lock}: For new dependencies, record the functional need, viable alternatives considered, why the selected dependency is the narrowest fit, and any unresolved licensing question.
After updating manifests or lockfiles, runuv run --no-project python scripts/licensing/license_diff.py --base-ref origin/mainand review added packages and license changes.
Keep workspace, Python package, and lockfile versions aligned wherever the packaging contract requires alignment.
Files:
adapters/claude/pyproject.toml
adapters/*/pyproject.toml
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
adapters/*/pyproject.toml: Keep leaf adapter runtime dependencies razor-thin and adapter-owned. Do not declare the wrapped harness/SDK or dependencies already declared by its supported package; provide dependency-free fallbacks for optional libraries.
Provide every installable Python leaf adapter with aharnessextra and afullextra for package-installable integrations; provide arelayextra when importing NeMo Relay Python APIs, but not when Relay is an external executable.
For packaged harnesses, declare and document aharnessversion constraint supported by upstream contracts and test evidence; do not claim a broader range than the evidence supports.
Files:
adapters/claude/pyproject.toml
adapters/*/{pyproject.toml,uv.lock}
📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)
Keep each adapter package independent and small, using Python package metadata and lock files as applicable.
Files:
adapters/claude/pyproject.toml
**/*.{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_adapter_package_metadata.pyadapters/common/src/nemo_fabric_adapters/common/utils.pytests/adapters/test_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/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_adapter_package_metadata.pyadapters/common/src/nemo_fabric_adapters/common/utils.pytests/adapters/test_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/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_adapter_package_metadata.pyadapters/common/src/nemo_fabric_adapters/common/utils.pytests/adapters/test_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/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_adapter_package_metadata.pytests/adapters/test_claude_adapter.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_adapter_package_metadata.pyadapters/common/src/nemo_fabric_adapters/common/utils.pytests/adapters/test_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
**/*.{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:
tests/adapters/test_adapter_package_metadata.pyadapters/common/src/nemo_fabric_adapters/common/utils.pytests/adapters/test_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/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_adapter_package_metadata.pytests/adapters/test_claude_adapter.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_adapter_package_metadata.pytests/adapters/test_claude_adapter.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_adapter_package_metadata.pytests/adapters/test_claude_adapter.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/common/src/nemo_fabric_adapters/common/utils.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
🧠 Learnings (2)
📚 Learning: 2026-06-29T22:34:52.407Z
Learnt from: AjayThorve
Repo: NVIDIA/NeMo-Fabric PR: 27
File: adapters/codex-cli/fabric-adapter.json:13-15
Timestamp: 2026-06-29T22:34:52.407Z
Learning: In NeMo-Fabric adapter manifest files (e.g., `*/fabric-adapter.json`), keep `config.accepts` limited to the top-level Fabric capability sections that `resolve_capability_plan` consumes (such as `models`, `tools`, `mcp`, `skills`, `telemetry`). Do not add adapter-owned `harness.settings` keys to `config.accepts`; `harness.settings` should remain adapter-owned and be passed through unchanged.
Applied to files:
adapters/claude/fabric-adapter.jsoncrates/fabric-cli/assets/adapters/claude/fabric-adapter.json
📚 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/claude/src/nemo_fabric_adapters/claude/adapter.py
🪛 Ruff (0.16.1)
tests/adapters/test_claude_adapter.py
[warning] 543-543: Pattern passed to match= contains metacharacters but is neither escaped nor raw
(RUF043)
adapters/claude/src/nemo_fabric_adapters/claude/adapter.py
[warning] 751-751: Unnecessary dict comprehension for iterable; use dict.fromkeys instead
Replace with dict.fromkeys(iterable))
(C420)
[warning] 905-907: Abstract raise to an inner function
(TRY301)
🔇 Additional comments (8)
adapters/claude/src/nemo_fabric_adapters/claude/adapter.py (2)
33-35: LGTM!Also applies to: 166-253, 257-326, 363-398, 451-477, 529-645, 675-675, 745-778, 803-813, 887-915, 930-957, 1030-1064, 1123-1123
33-35: 📐 Maintainability & Code QualityRun the required Python suite.
The stated validation includes focused tests only. Run
just test-pythonbefore merge for this Python-facing adapter change.As per coding guidelines, “If Python code or a Python-facing adapter changes, run
just test-python.”Source: Coding guidelines
adapters/claude/fabric-adapter.json (1)
44-44: LGTM!adapters/claude/pyproject.toml (1)
28-28: 📐 Maintainability & Code QualityValidate and document the new direct dependency.
Record the functional need, alternatives considered, and why
nemo-fabric-adapter-contractis the narrowest fit. Run the required license-diff script and theattributions-rustandattributions-pythonpre-commit hooks. Confirm the lockfile and package metadata remain synchronized.As per coding guidelines, “For new dependencies, record the functional need, viable alternatives considered, why the selected dependency is the narrowest fit,” and run the license-diff and attribution checks.
Also applies to: 55-55
Source: Coding guidelines
crates/fabric-cli/assets/adapters/claude/fabric-adapter.json (1)
44-44: LGTM!adapters/common/src/nemo_fabric_adapters/common/utils.py (1)
234-264: LGTM!Also applies to: 306-306
tests/adapters/test_adapter_package_metadata.py (1)
69-69: LGTM!tests/adapters/test_claude_adapter.py (1)
30-31: LGTM!Also applies to: 51-96, 170-170, 203-246, 277-277, 335-356, 380-382, 419-419, 494-496, 510-510, 523-536, 539-541, 562-657, 684-684, 743-786, 849-849, 961-1018, 1089-1198, 1242-1290, 1343-1359, 1401-1422, 1435-1435
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
|
Fern docs preview: https://nvidia-preview-pull-request-197.docs.buildwithfern.com/nemo/fabric |
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@adapters/claude/src/nemo_fabric_adapters/claude/adapter.py`:
- Around line 954-955: Update invoke() to propagate the per-invocation
request_id into the Relay correlation context before processing each request,
while continuing to use runtime_id for runtime-scoped artifact handling. Add a
two-turn test that invokes the adapter with distinct request IDs and verifies
each request is correlated with its own ID.
🪄 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: bd03b7c1-ca85-4885-8e80-837e5ebd07d2
📒 Files selected for processing (2)
adapters/claude/src/nemo_fabric_adapters/claude/adapter.pytests/adapters/test_claude_adapter.py
📜 Review details
⏰ Context from checks skipped due to timeout. (16)
- GitHub Check: Preview docs
- GitHub Check: Test (Python 3.14, linux-arm64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.14, macos-arm64)
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Test (Python 3.11, linux-arm64)
- GitHub Check: Test (Python 3.12, linux-arm64)
- GitHub Check: Test (Python 3.12, linux-amd64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Test (Python 3.14, linux-amd64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.13, linux-amd64)
- GitHub Check: Pre-commit
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{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_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
**/*
📄 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:
tests/adapters/test_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/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_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/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_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/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_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/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_claude_adapter.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_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/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_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/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_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/adapter.py
**/*.{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:
tests/adapters/test_claude_adapter.pyadapters/claude/src/nemo_fabric_adapters/claude/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_claude_adapter.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_claude_adapter.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_claude_adapter.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/claude/src/nemo_fabric_adapters/claude/adapter.py
{adapters/**,examples/**}
⚙️ CodeRabbit configuration file
{adapters/**,examples/**}: Review adapter and example changes for command correctness, config/schema consistency, artifact handling, and compatibility with the public NeMo Fabric contracts.
Files:
adapters/claude/src/nemo_fabric_adapters/claude/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/claude/src/nemo_fabric_adapters/claude/adapter.py
🔇 Additional comments (2)
tests/adapters/test_claude_adapter.py (1)
543-544: LGTM!Also applies to: 547-551
adapters/claude/src/nemo_fabric_adapters/claude/adapter.py (1)
33-33: LGTM!Also applies to: 171-178, 898-898, 919-919
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
1 participant
Overview
Move the Claude adapter to the
fabric.adapter/v1alpha2typed contract. Lifecycle startup now validatesAgentConfig; model, MCP, skills, tool policy, instructions, and runtime context use contract dataclasses while the preview invocation request stays opaque.Preserve Relay behavior with a backward-compatible typed model-name override in the shared Relay helper. No shared legacy helper is removed because Claude, Codex, Hermes, and Deep Agents still have callers.
Per-invocation Relay request-ID correlation requires SDK/Relay support and is deferred.
Validated with
pytest -q tests/integrations/test_harbor_runner.py::test_swebench_matrix_translates_harbor_inputs_to_typed_config tests/adapters/test_claude_adapter.py tests/adapters/test_adapter_package_metadata.py(67 passed).Where should the reviewer start?
Start with
adapters/claude/src/nemo_fabric_adapters/claude/adapter.py, then the descriptor and focused lifecycle tests.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Relates to FABRIC-184
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.