feat: validate adapter model providers - #118
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:
WalkthroughClaude and Codex descriptors now publish ChangesAdapter model-provider compatibility
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant FabricConfig
participant AdapterDescriptor
participant FabricCore
participant PlanOrDoctor
FabricConfig->>AdapterDescriptor: select adapter and model roles
AdapterDescriptor->>FabricCore: provide model_schema
FabricCore->>PlanOrDoctor: validate normalized model entries
PlanOrDoctor-->>FabricConfig: return plan or doctor diagnostics
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Fern docs preview: https://nvidia-preview-pull-request-118.docs.buildwithfern.com/nemo/fabric |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py (1)
328-334: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
state_dir()is not scoped by runtime ID, unlike Hermes's equivalent fix.This function's own removal message for
harness.settings.state_dirclaims state is "derived from the Fabric artifact root and runtime ID," and the README says the same, but the returned path never includes the runtime ID. Runtimes sharing an artifact root/base_dir will share the same.fabric/deepagentsdirectory, colliding on LangGraph checkpointer/session state across concurrent Fabric runtimes. Hermes's equivalent code in this same PR (adapters/hermes/src/nemo_fabric_adapters/hermes/adapter.py:264-269) correctly wraps its home directory withcommon_utils.runtime_state_directory(...).🐛 Proposed fix
def state_dir(payload: dict[str, Any]) -> Path: base_dir = Path(common_utils.base_dir(payload)).resolve() artifacts = common_utils.runtime_context(payload).get("artifacts") or {} root = artifacts.get("root") or os.environ.get("FABRIC_ARTIFACTS") - if root: - return Path(str(root)).resolve() / ".fabric" / "deepagents" - return base_dir / "artifacts" / "deepagents" / ".fabric" + if root: + base = Path(str(root)).resolve() / ".fabric" / "deepagents" + else: + base = base_dir / "artifacts" / "deepagents" / ".fabric" + return common_utils.runtime_state_directory(base, payload)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py` around lines 328 - 334, Update state_dir() to scope the resolved Fabric artifact or base directory through common_utils.runtime_state_directory(...), matching Hermes's equivalent implementation, before appending the deepagents state path. Ensure the returned path includes the current runtime ID while preserving the existing artifact-root and fallback-directory selection.
🤖 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/hermes/src/nemo_fabric_adapters/hermes/adapter.py`:
- Around line 47-51: Update the Hermes adapter’s terminal backend handling so
the configured terminal_backend is honored throughout, including TERMINAL_ENV
and build_hermes_config()["terminal"]["backend"], or revise
REMOVED_SETTING_FIELDS to explicitly document that only local execution is
supported. Keep validation and runtime configuration consistent so accepted
settings are not silently overridden.
In `@crates/fabric-core/src/runtime.rs`:
- Around line 577-581: The AdapterCompatibility error path in resolve_run_plan
currently reports route.name directly for unsupported MCP routes; map the field
through the existing CapabilityKind-to-field logic used by
validate_capability_plan_compatibility so MCP routes emit the canonical
mcp.servers.{name} path. Preserve the existing adapter_id and reason handling.
In `@docs/reference/api/python-library-reference/nemo_fabric.models.md`:
- Around line 1364-1365: Regenerate the API reference documentation so the
ToolsetConfig heading has the required blank lines before and after it,
resolving markdownlint MD022. Do not edit the generated file under
docs/reference/api/ directly; update the source or generation inputs and run the
established reference-generation process.
In
`@docs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-adapterdescriptorsource.mdx`:
- Line 5: Regenerate the generated API reference output rather than editing it
directly: update the relevant source or generator for the position changes, then
run just docs. Apply the regeneration to
docs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-adapterdescriptorsource.mdx:5-5
and
docs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-adapterkind.mdx:5-5.
In
`@docs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterconfigsupport.mdx`:
- Line 20: Update the Rust documentation source descriptions to use the
product-qualified naming: in
docs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterconfigsupport.mdx:20,
use “NVIDIA NeMo Fabric” for the first mention; in
docs/reference/api/rust-library-reference/nemo-fabric-core/config/index.mdx:58,
use “NeMo Fabric” thereafter. Regenerate the API references after updating the
source.
In
`@docs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterdescriptor.mdx`:
- Line 5: Do not edit the generated AdapterDescriptor API reference directly;
apply the position ordering change in the source documentation or generator that
produces it, then regenerate the affected docs/reference/api output.
In `@examples/harbor/README.md`:
- Around line 80-88: Update the FabricConfig field column in the README table to
use the exact paths models.default.base_url, system_prompt, max_turns,
runtime.timeout_seconds, environment.env, tools.toolsets.enabled,
tools.toolsets.blocked, and harness.settings; remove the duplicate
fabric_blocked_tools row while preserving the corresponding descriptions and
command syntax.
In `@schemas/agent.schema.json`:
- Around line 993-1001: Update the Rust source contract for max_turns to require
values greater than zero by setting its minimum to 1, then regenerate
schemas/agent.schema.json so the snapshot matches. Add or update the
schema-parity assertion to verify max_turns rejects 0 and accepts valid positive
values.
In `@schemas/run-plan.schema.json`:
- Around line 1631-1637: Update the Rust/Python contract for timeout_seconds to
reject negative values while preserving nullability, then regenerate the
schemas/run-plan.schema.json snapshot. Add invalid-input coverage demonstrating
that negative invocation timeouts are rejected, using the existing
timeout_seconds contract and validation tests.
---
Outside diff comments:
In `@adapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.py`:
- Around line 328-334: Update state_dir() to scope the resolved Fabric artifact
or base directory through common_utils.runtime_state_directory(...), matching
Hermes's equivalent implementation, before appending the deepagents state path.
Ensure the returned path includes the current runtime ID while preserving the
existing artifact-root and fallback-directory selection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 0c7e7cf5-75a4-4ac5-ab0c-593c7e93f734
📒 Files selected for processing (144)
README.mdadapters/README.mdadapters/claude/README.mdadapters/claude/fabric-adapter.jsonadapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/codex/README.mdadapters/codex/fabric-adapter.jsonadapters/codex/src/nemo_fabric_adapters/codex/adapter.pyadapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/deepagents/README.mdadapters/deepagents/fabric-adapter.jsonadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pyadapters/hermes/README.mdadapters/hermes/fabric-adapter.jsonadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pycrates/fabric-cli/assets/adapters/claude/fabric-adapter.jsoncrates/fabric-cli/assets/adapters/codex/fabric-adapter.jsoncrates/fabric-cli/assets/adapters/deepagents/fabric-adapter.jsoncrates/fabric-cli/assets/adapters/hermes/fabric-adapter.jsoncrates/fabric-cli/src/examples.rscrates/fabric-cli/src/presets.rscrates/fabric-cli/src/scaffold.rscrates/fabric-cli/templates/python/main.py.tmplcrates/fabric-cli/templates/rust/main.rs.tmplcrates/fabric-core/src/config.rscrates/fabric-core/src/error.rscrates/fabric-core/src/lib.rscrates/fabric-core/src/runtime.rsdocs/getting-started/quickstart.mdxdocs/integrations/claude.mdxdocs/integrations/codex.mdxdocs/reference/api/python-library-reference/index.mddocs/reference/api/python-library-reference/nemo_fabric.client.mddocs/reference/api/python-library-reference/nemo_fabric.models.mddocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-adapterconfigfield.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-adapterdescriptorsource.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-adapterkind.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-adaptersettingtype.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-capabilitykind.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-capabilitytarget.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-controllocation.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-environmentownership.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-mcpexposure.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatifstorageconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatofendpointfieldnamepolicy.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatofendpointtransport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatofmode.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayotlptransport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayunsupportedbehavior.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-resolutionstrategy.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-telemetryprovider.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/fn-load-adapter-descriptor.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/fn-resolve-run-plan-from-config.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterconfigsupport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterdescriptor.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adaptermodelconfigsupport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterrequirements.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adaptersettingschema.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adaptertelemetryprovidersupport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adaptertelemetrysupport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-capabilityplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-capabilityroute.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-capabilitytargetplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-environmentconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-environmentplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-fabricconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-harnessconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-mcpconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-mcpserverconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-mcpserverplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-metadataconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-modelconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayatifconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayatofconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayatofendpointconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relaycomponentconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayconfigpolicy.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayobservabilityconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayotlpconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-resolvecontext.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-resolvedadapterdescriptor.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-runplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-runtimecapabilities.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-runtimeconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-skillconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-telemetryconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-telemetryplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-telemetryproviderconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-toolsconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-toolsetconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-toolsplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/enum-doctorstatus.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/fn-doctor-plan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/struct-doctorcheck.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/struct-doctorreport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/error/enum-fabricerror.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/error/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/error/type-result.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/fn-version.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/runtime/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/runtime/struct-environmenthandle.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/schema/index.mdxdocs/sdk/python.mdxexamples/code_review_agent/config.pyexamples/harbor/README.mdexamples/harbor/calculator/README.mdexamples/harbor/swebench/README.mdexamples/harbor/swebench/adapters/claude/fabric-adapter.jsonexamples/harbor/swebench/adapters/hermes/fabric-adapter.jsonexamples/notebooks/01_quickstart.ipynbpython/src/nemo_fabric/__init__.pypython/src/nemo_fabric/integrations/harbor/fabric_agent.pypython/src/nemo_fabric/models.pypython/src/nemo_fabric/types.pyschemas/adapter-descriptor.schema.jsonschemas/adapter-invocation.schema.jsonschemas/agent.schema.jsonschemas/environment-handle.schema.jsonschemas/run-plan.schema.jsonschemas/runtime-context.schema.jsonschemas/runtime-handle.schema.jsonskills/nemo-fabric-integrate/SKILL.mdskills/nemo-fabric-integrate/references/config-mapping.mdtests/_utils/configs.pytests/adapters/test_adapaters_common_utils.pytests/adapters/test_claude_adapter.pytests/adapters/test_codex_adapter.pytests/adapters/test_deepagents.pytests/adapters/test_hermes_adapter.pytests/e2e/test_claude.pytests/e2e/test_codex.pytests/e2e/test_deepagents.pytests/e2e/test_hermes_config_mapping.pytests/e2e/test_hermes_e2e.pytests/fixtures/hermes-shim-agent/adapters/hermes-shim/fabric-adapter.jsontests/fixtures/hermes-shim-agent/adapters/hermes-shim/src/nemo_fabric_test_adapters/hermes_shim/adapter.pytests/integrations/test_harbor_runner.pytests/python/test_native_sdk.pytests/python/test_sdk_contract.pytests/python/test_typed_config.py
7d8db50 to
08fa625
Compare
08fa625 to
de79c72
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
docs/reference/api/python-library-reference/nemo_fabric.client.md (1)
4-4: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse the full product name on first use.
The new description says “with NeMo Fabric” without the required first-use “NVIDIA” qualifier. Update the source docstring or generator to emit “with NVIDIA NeMo Fabric,” then regenerate this page. As per coding guidelines, user-facing prose must use “NVIDIA NeMo Fabric” on first use and “NeMo Fabric” thereafter. As per path instructions, generated API references must be regenerated rather than edited directly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/reference/api/python-library-reference/nemo_fabric.client.md` at line 4, Update the source docstring or generator responsible for the API page description to use “with NVIDIA NeMo Fabric” on first use, then regenerate the generated reference page rather than editing it directly; retain “NeMo Fabric” for subsequent mentions.Sources: Coding guidelines, Path instructions
docs/sdk/python.mdx (1)
178-182: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse
NeMo Fabricfor the product name.The new definition says “Fabric owns the behavior,” using standalone capitalized “Fabric” for the product. Change it to “NeMo Fabric owns the behavior.” As per path instructions, user-facing prose must use “NVIDIA NeMo Fabric” on first use and “NeMo Fabric” thereafter, and must not use standalone capitalized “Fabric” for the product.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/sdk/python.mdx` around lines 178 - 182, Update the definition containing “Fabric owns the behavior” to use “NeMo Fabric owns the behavior,” preserving the rest of the documentation text and terminology.Source: Path instructions
docs/reference/api/python-library-reference/nemo_fabric.models.md (3)
1385-1445: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument the
ToolsetConfigfields and semantics.The new class section exposes inherited helpers but does not document
enabledorblocked, including the importantNoneversus empty-list behavior and overlap validation. Add this to the source model documentation, then regenerate the API reference.As per path instructions, files under
docs/reference/api/are generated output and must not be edited directly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/reference/api/python-library-reference/nemo_fabric.models.md` around lines 1385 - 1445, Document the enabled and blocked fields and their semantics in the source definition of ToolsetConfig, including the distinction between None and empty lists and the validation behavior for overlapping entries. Then regenerate the API reference so the generated ToolsetConfig section reflects this documentation; do not edit docs/reference/api files directly.Source: Path instructions
4-4: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse
NVIDIA NeMo Fabricon first use across generated API references.
docs/reference/api/python-library-reference/nemo_fabric.models.md#L4-L4: update the new frontmatter description to use the full product name.docs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-adapterconfigfield.mdx#L4-L4: update the MDX description to use the full product name.docs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-adapterconfigfield.mdx#L28-L28: update the generated body description consistently.As per path instructions, update the source documentation and regenerate these files rather than editing generated output directly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/reference/api/python-library-reference/nemo_fabric.models.md` at line 4, Update the source documentation descriptions to use “NVIDIA NeMo Fabric” on first use, then regenerate the affected outputs. Apply this to docs/reference/api/python-library-reference/nemo_fabric.models.md lines 4-4, docs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-adapterconfigfield.mdx lines 4-4, and the generated body description at lines 28-28; do not edit generated files directly.Source: Path instructions
1586-1595: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDocument
configure_toolsetsas keyword-only.The source API defines
configure_toolsets(*, enabled=..., blocked=...), but this generated signature omits*and implies positional arguments are accepted. Update the generator/source documentation and regenerate the reference withjust docs.As per path instructions, files under
docs/reference/api/are generated output and must not be edited directly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/reference/api/python-library-reference/nemo_fabric.models.md` around lines 1586 - 1595, Update the source or documentation generator definition for configure_toolsets so enabled and blocked are documented as keyword-only by including the keyword-only marker in the generated signature. Do not edit the generated file under docs/reference/api; regenerate the reference documentation with just docs and verify the signature matches the source API.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@adapters/README.md`:
- Around line 101-105: The documentation overstates model-setting validation as
covering every configured role; update the corresponding guidance to specify
validation of the selected model role’s settings, preserving the existing
selected-role behavior. Apply this wording correction in adapters/README.md
lines 101-105 and docs/sdk/python.mdx lines 217-221; no core resolver change is
required.
In `@tests/python/test_typed_config.py`:
- Around line 164-175: Extend the tests in
test_plan_rejects_undeclared_model_setting_without_exposing_value with an async
Fabric().doctor counterpart using the same undeclared regionn value, and assert
the resulting FabricConfigError diagnostic contains
models.default.settings.regionn but not secret-setting-value. Mark the new test
async and await Fabric().doctor(config) to cover the doctor error path.
---
Outside diff comments:
In `@docs/reference/api/python-library-reference/nemo_fabric.client.md`:
- Line 4: Update the source docstring or generator responsible for the API page
description to use “with NVIDIA NeMo Fabric” on first use, then regenerate the
generated reference page rather than editing it directly; retain “NeMo Fabric”
for subsequent mentions.
In `@docs/reference/api/python-library-reference/nemo_fabric.models.md`:
- Around line 1385-1445: Document the enabled and blocked fields and their
semantics in the source definition of ToolsetConfig, including the distinction
between None and empty lists and the validation behavior for overlapping
entries. Then regenerate the API reference so the generated ToolsetConfig
section reflects this documentation; do not edit docs/reference/api files
directly.
- Line 4: Update the source documentation descriptions to use “NVIDIA NeMo
Fabric” on first use, then regenerate the affected outputs. Apply this to
docs/reference/api/python-library-reference/nemo_fabric.models.md lines 4-4,
docs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-adapterconfigfield.mdx
lines 4-4, and the generated body description at lines 28-28; do not edit
generated files directly.
- Around line 1586-1595: Update the source or documentation generator definition
for configure_toolsets so enabled and blocked are documented as keyword-only by
including the keyword-only marker in the generated signature. Do not edit the
generated file under docs/reference/api; regenerate the reference documentation
with just docs and verify the signature matches the source API.
In `@docs/sdk/python.mdx`:
- Around line 178-182: Update the definition containing “Fabric owns the
behavior” to use “NeMo Fabric owns the behavior,” preserving the rest of the
documentation text and terminology.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 33357197-5440-4e66-a37c-34481fd1587e
📒 Files selected for processing (100)
adapters/README.mdadapters/claude/fabric-adapter.jsonadapters/codex/fabric-adapter.jsonadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pycrates/fabric-cli/assets/adapters/claude/fabric-adapter.jsoncrates/fabric-cli/assets/adapters/codex/fabric-adapter.jsoncrates/fabric-cli/src/examples.rscrates/fabric-core/src/config.rscrates/fabric-core/src/lib.rscrates/fabric-core/src/runtime.rscrates/fabric-core/src/schema.rsdocs/reference/api/python-library-reference/nemo_fabric.client.mddocs/reference/api/python-library-reference/nemo_fabric.errors.mddocs/reference/api/python-library-reference/nemo_fabric.models.mddocs/reference/api/python-library-reference/nemo_fabric.runtime.mddocs/reference/api/python-library-reference/nemo_fabric.types.mddocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-adapterconfigfield.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-adaptersettingtype.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-capabilitykind.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-capabilitytarget.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-controllocation.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-environmentownership.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-mcpexposure.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatifstorageconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatofendpointfieldnamepolicy.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatofendpointtransport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatofmode.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayotlptransport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayunsupportedbehavior.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-resolutionstrategy.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-telemetryprovider.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/fn-load-adapter-descriptor.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/fn-resolve-run-plan-from-config.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterconfigsupport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adaptermodelconfigsupport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterrequirements.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adaptersettingschema.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adaptertelemetryprovidersupport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adaptertelemetrysupport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-capabilityplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-capabilityroute.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-capabilitytargetplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-environmentconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-environmentplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-fabricconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-harnessconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-mcpconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-mcpserverconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-mcpserverplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-metadataconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-modelconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayatifconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayatofconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayatofendpointconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relaycomponentconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayconfigpolicy.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayobservabilityconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayotlpconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-resolvecontext.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-resolvedadapterdescriptor.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-runplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-runtimecapabilities.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-runtimeconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-skillconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-telemetryconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-telemetryplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-telemetryproviderconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-toolsconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-toolsetconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-toolsplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/enum-doctorstatus.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/fn-doctor-plan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/struct-doctorcheck.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/struct-doctorreport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/error/enum-fabricerror.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/error/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/error/type-result.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/fn-version.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/runtime/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/schema/index.mdxdocs/sdk/python.mdxexamples/harbor/README.mdexamples/harbor/swebench/adapters/claude/fabric-adapter.jsonpython/src/nemo_fabric/models.pyschemas/adapter-descriptor.schema.jsonschemas/agent.schema.jsonschemas/run-plan.schema.jsonscripts/generate_api_docs.shskills/nemo-fabric-integrate/SKILL.mdskills/nemo-fabric-integrate/references/config-mapping.mdtests/adapters/test_claude_adapter.pytests/adapters/test_codex_adapter.pytests/adapters/test_hermes_adapter.pytests/e2e/test_codex.pytests/python/test_sdk_contract.pytests/python/test_typed_config.py
de79c72 to
d088452
Compare
d088452 to
bc43fc6
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@adapters/common/src/nemo_fabric_adapters/common/utils.py`:
- Around line 142-144: Update timeout_seconds() to reject boolean and other
non-numeric timeout_seconds values before coercion, while preserving the default
when the value is absent. Return a validated float only for supported numeric
values, and raise the established configuration error used by the adapters so
invalid settings produce a clean fail-closed diagnostic; keep max_turns()
behavior as the validation model and ensure downstream boolean guards remain
meaningful.
In `@adapters/hermes/README.md`:
- Around line 34-51: Update the Hermes adapter documentation around
build_hermes_config to remove the claim that runtime.timeout_seconds controls
invocation timeouts, since the adapter does not consume it. Document the
supported harness.settings.terminal_timeout configuration instead, matching the
existing Hermes configuration behavior.
In `@docs/integrations/claude.mdx`:
- Around line 11-14: Update the documentation text around the environment
configuration and SDK pin so the configuration key `environment.env` and package
specification `claude-agent-sdk==0.2.120` use inline code formatting, preserving
the surrounding prose.
In `@docs/reference/api/python-library-reference/nemo_fabric.client.md`:
- Line 4: The first-use descriptions in
docs/reference/api/python-library-reference/nemo_fabric.client.md:4 and
docs/reference/api/python-library-reference/nemo_fabric.models.md:4 must use
“NVIDIA NeMo Fabric” instead of “NeMo Fabric”; update the source or generator
for both API references, then regenerate the files.
In `@docs/reference/api/python-library-reference/nemo_fabric.models.md`:
- Around line 1647-1656: Update the source or documentation generator for
configure_toolsets so its rendered signature preserves the keyword-only marker
after self, matching the source definition in models.py. Do not edit the
generated reference page directly; rerun just docs and verify the output shows
enabled and blocked as keyword-only parameters.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 3fd3b73f-50ed-47f0-9e33-d68d4ff28f9a
📒 Files selected for processing (149)
README.mdadapters/README.mdadapters/claude/README.mdadapters/claude/fabric-adapter.jsonadapters/claude/src/nemo_fabric_adapters/claude/adapter.pyadapters/codex/README.mdadapters/codex/fabric-adapter.jsonadapters/codex/src/nemo_fabric_adapters/codex/adapter.pyadapters/common/src/nemo_fabric_adapters/common/utils.pyadapters/deepagents/README.mdadapters/deepagents/fabric-adapter.jsonadapters/deepagents/src/nemo_fabric_adapters/deepagents/adapter.pyadapters/hermes/README.mdadapters/hermes/fabric-adapter.jsonadapters/hermes/src/nemo_fabric_adapters/hermes/adapter.pycrates/fabric-cli/assets/adapters/claude/fabric-adapter.jsoncrates/fabric-cli/assets/adapters/codex/fabric-adapter.jsoncrates/fabric-cli/assets/adapters/deepagents/fabric-adapter.jsoncrates/fabric-cli/assets/adapters/hermes/fabric-adapter.jsoncrates/fabric-cli/src/examples.rscrates/fabric-cli/src/presets.rscrates/fabric-cli/src/scaffold.rscrates/fabric-cli/templates/python/main.py.tmplcrates/fabric-cli/templates/rust/main.rs.tmplcrates/fabric-core/src/config.rscrates/fabric-core/src/error.rscrates/fabric-core/src/lib.rscrates/fabric-core/src/runtime.rscrates/fabric-core/src/schema.rsdocs/getting-started/quickstart.mdxdocs/integrations/claude.mdxdocs/integrations/codex.mdxdocs/reference/api/python-library-reference/index.mddocs/reference/api/python-library-reference/nemo_fabric.client.mddocs/reference/api/python-library-reference/nemo_fabric.errors.mddocs/reference/api/python-library-reference/nemo_fabric.models.mddocs/reference/api/python-library-reference/nemo_fabric.runtime.mddocs/reference/api/python-library-reference/nemo_fabric.types.mddocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-adapterconfigfield.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-adapterdescriptorsource.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-adapterkind.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-adaptersettingtype.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-capabilitykind.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-capabilitytarget.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-controllocation.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-environmentownership.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-mcpexposure.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatifstorageconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatofmode.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatofsinkconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatofstreamfieldnamepolicy.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayatofstreamtransport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayotlptransport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-relayunsupportedbehavior.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-resolutionstrategy.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/enum-telemetryprovider.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/fn-load-adapter-descriptor.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/fn-resolve-run-plan-from-config.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterconfigsupport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterdescriptor.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adaptermodelconfigsupport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterrequirements.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adaptersettingschema.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adaptertelemetryprovidersupport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adaptertelemetrysupport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-capabilityplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-capabilityroute.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-capabilitytargetplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-environmentconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-environmentplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-fabricconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-harnessconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-mcpconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-mcpserverconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-mcpserverplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-metadataconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-modelconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayatifconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayatofconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relaycomponentconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayconfigpolicy.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayobservabilityconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-relayotlpconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-resolvecontext.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-resolvedadapterdescriptor.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-runplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-runtimecapabilities.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-runtimeconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-skillconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-telemetryconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-telemetryplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-telemetryproviderconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-toolsconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-toolsetconfig.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-toolsplan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/enum-doctorstatus.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/fn-doctor-plan.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/struct-doctorcheck.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/doctor/struct-doctorreport.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/error/enum-fabricerror.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/error/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/error/type-result.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/fn-version.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/runtime/index.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/runtime/struct-environmenthandle.mdxdocs/reference/api/rust-library-reference/nemo-fabric-core/schema/index.mdxdocs/sdk/python.mdxexamples/code_review_agent/config.pyexamples/harbor/README.mdexamples/harbor/calculator/README.mdexamples/harbor/swebench/README.mdexamples/harbor/swebench/adapters/claude/fabric-adapter.jsonexamples/harbor/swebench/adapters/hermes/fabric-adapter.jsonexamples/notebooks/01_quickstart.ipynbpython/src/nemo_fabric/__init__.pypython/src/nemo_fabric/integrations/harbor/fabric_agent.pypython/src/nemo_fabric/models.pypython/src/nemo_fabric/types.pyschemas/adapter-descriptor.schema.jsonschemas/adapter-invocation.schema.jsonschemas/agent.schema.jsonschemas/environment-handle.schema.jsonschemas/run-plan.schema.jsonschemas/runtime-context.schema.jsonschemas/runtime-handle.schema.jsonscripts/generate_api_docs.shskills/nemo-fabric-integrate/SKILL.mdskills/nemo-fabric-integrate/references/config-mapping.mdtests/_utils/configs.pytests/adapters/test_adapaters_common_utils.pytests/adapters/test_claude_adapter.pytests/adapters/test_codex_adapter.pytests/adapters/test_deepagents.pytests/adapters/test_hermes_adapter.pytests/e2e/test_claude.pytests/e2e/test_codex.pytests/e2e/test_deepagents.pytests/e2e/test_hermes_config_mapping.pytests/e2e/test_hermes_e2e.pytests/fixtures/hermes-shim-agent/adapters/hermes-shim/fabric-adapter.jsontests/fixtures/hermes-shim-agent/adapters/hermes-shim/src/nemo_fabric_test_adapters/hermes_shim/adapter.pytests/integrations/test_harbor_runner.pytests/python/test_native_sdk.pytests/python/test_sdk_contract.pytests/python/test_typed_config.py
#### Overview Prevent the `claude` and `codex` CLI presets and example variants from constructing or planning a configuration unless `NVIDIA_FRONTIER_BASE_URL` contains a non-empty endpoint. These presets require NVIDIA Frontier and must not inherit a public NVIDIA Catalog default from downstream normalization. `Preset::config()` is now the authoritative fallible construction path, and staging and maintained examples propagate its validation error. This intentionally changes that public experimentation-CLI method from returning `FabricConfig` to returning `Result<FabricConfig, String>`; there are no core schema or Python SDK API changes. Existing CLI documentation already describes the Frontier endpoint as explicit and without a default. #### Details - Reuse each preset's existing `required_env` metadata instead of adding a second configuration-environment field. - Validate the Frontier endpoint before direct config construction, preset staging, planning, doctoring, running, or maintained-example generation. - Keep `NVIDIA_API_KEY` as a runtime credential requirement rather than a planning requirement. - Leave the Catalog-native `hermes` and `deepagents` presets unchanged. - Keep one integration test that invokes the compiled CLI for both affected presets, rejects a missing endpoint, and parses the emitted plan to prove the exact Frontier URL was preserved. - Leave legacy nested endpoint handling unchanged; PR #118 already owns its migration validation. #### Validation - `cargo fmt --all -- --check` - `just --fmt --check` - `cargo check --workspace --locked` - `just test-rust` - `git diff --check` - `cargo build -p nemo-fabric-cli --locked`, followed by a compiled-binary matrix covering Claude/Codex preset and example plan/init failures and successes plus Hermes/DeepAgents Catalog defaults - `uv run --no-sync python -` E2E: compiled CLI plan → `FabricConfig.from_mapping()` → public `Fabric.run()` → Claude adapter → mock Claude CLI; child received `https://frontier-e2e.invalid`, and Catalog was absent - `uv run --no-sync pytest -q tests/adapters/test_codex_adapter.py -k 'nvidia_provider'` (5 passed, 45 deselected) A full Codex child-process E2E was not run because the repository has no deterministic Codex CLI fixture. The focused adapter tests cover its NVIDIA endpoint mapping and missing-endpoint rejection. `just test-python` was not rerun because the final diff contains no Python, SDK, binding, or adapter changes. #### Where should the reviewer start? Start with the fallible `Preset::config()` path in `crates/fabric-cli/src/presets.rs`, then review the actual CLI boundary test in `crates/fabric-cli/tests/frontier_presets.rs`. #### Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to) - Relates to #118 - [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** * Improved configuration validation for Claude and Codex presets. * The NVIDIA Frontier endpoint must now be set when required; missing or blank values produce a clear error. * Configured Frontier endpoint settings are now preserved in generated plans. * Configuration failures are reported cleanly instead of being silently assumed valid. * **Tests** * Added regression coverage for missing and configured Frontier endpoint scenarios. Authors: - Zhongxuan (Daniel) Wang (https://github.com/zhongxuanwang-nv) Approvers: - Ajay Thorve (https://github.com/AjayThorve) URL: #120
bc43fc6 to
201399e
Compare
AjayThorve
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/fabric-core/src/config.rs (1)
374-389: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftImplement the typed descriptor model-support contract.
config.modelsis absent fromAdapterConfigSupport; becauseextensionsis flattened, descriptor model/provider-setting declarations are silently ignored rather than validated. This omits the PR’s provider-specific settings contract and its fail-closed behavior.
crates/fabric-core/src/config.rs#L374-L389: add typed model-provider and setting-schema fields, then validate providers, required settings, and JSON types during planning.schemas/adapter-descriptor.schema.json#L53-L80: regenerate the snapshot with the typedmodelscontract.schemas/run-plan.schema.json#L53-L80: regenerate the corresponding run-plan snapshot.As per path instructions, schemas are generated public contract snapshots that must stay aligned with Rust changes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/fabric-core/src/config.rs` around lines 374 - 389, Implement the typed model-provider and setting-schema contract in AdapterConfigSupport, including validation during planning for supported providers, required settings, and JSON value types; do not let these declarations fall through extensions. Update crates/fabric-core/src/config.rs at lines 374-389, then regenerate the aligned public contract snapshots in schemas/adapter-descriptor.schema.json at lines 53-80 and schemas/run-plan.schema.json at lines 53-80.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/fabric-core/src/config.rs`:
- Around line 2396-2424: Cover and document native Nvidia provider support: in
crates/fabric-core/src/config.rs lines 2396-2424, add Claude and Codex cases
using the native nvidia provider without connection fields; in
tests/python/test_typed_config.py lines 177-221, add a successful SDK planning
case for nvidia without base_url or api_key_env; and in docs/sdk/python.mdx
lines 203-207, update the compatibility matrix to list anthropic and nvidia for
Claude and openai and nvidia for Codex.
---
Outside diff comments:
In `@crates/fabric-core/src/config.rs`:
- Around line 374-389: Implement the typed model-provider and setting-schema
contract in AdapterConfigSupport, including validation during planning for
supported providers, required settings, and JSON value types; do not let these
declarations fall through extensions. Update crates/fabric-core/src/config.rs at
lines 374-389, then regenerate the aligned public contract snapshots in
schemas/adapter-descriptor.schema.json at lines 53-80 and
schemas/run-plan.schema.json at lines 53-80.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 3b96eeb0-2908-4fc6-b9c5-208e9e1820d7
📒 Files selected for processing (16)
adapters/README.mdadapters/claude/fabric-adapter.jsonadapters/codex/fabric-adapter.jsoncrates/fabric-cli/assets/adapters/claude/fabric-adapter.jsoncrates/fabric-cli/assets/adapters/codex/fabric-adapter.jsoncrates/fabric-core/src/config.rsdocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterconfigsupport.mdxdocs/sdk/python.mdxexamples/harbor/swebench/adapters/claude/fabric-adapter.jsonschemas/adapter-descriptor.schema.jsonschemas/run-plan.schema.jsonskills/integrations/consumer/nemo-fabric-integrate/references/config-mapping.mdtests/adapters/test_claude_adapter.pytests/adapters/test_codex_adapter.pytests/python/test_native_sdk.pytests/python/test_typed_config.py
📜 Review details
⏰ Context from checks skipped due to timeout. (16)
- GitHub Check: Preview docs
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.11, linux-amd64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.12, linux-amd64)
- GitHub Check: Test (Python 3.13, linux-arm64)
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Test (Python 3.13, linux-amd64)
- GitHub Check: Test (Python 3.11, linux-arm64)
- GitHub Check: Test (Python 3.12, linux-arm64)
- GitHub Check: Test (Python 3.14, linux-amd64)
- GitHub Check: Pre-commit
🧰 Additional context used
📓 Path-based instructions (40)
**/*.{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:
skills/integrations/consumer/nemo-fabric-integrate/references/config-mapping.mdadapters/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:
skills/integrations/consumer/nemo-fabric-integrate/references/config-mapping.mddocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterconfigsupport.mdxdocs/sdk/python.mdxcrates/fabric-cli/assets/adapters/codex/fabric-adapter.jsonadapters/claude/fabric-adapter.jsonadapters/codex/fabric-adapter.jsoncrates/fabric-cli/assets/adapters/claude/fabric-adapter.jsontests/python/test_typed_config.pyadapters/README.mdschemas/run-plan.schema.jsonexamples/harbor/swebench/adapters/claude/fabric-adapter.jsonschemas/adapter-descriptor.schema.jsontests/python/test_native_sdk.pytests/adapters/test_claude_adapter.pytests/adapters/test_codex_adapter.pycrates/fabric-core/src/config.rs
**/*.{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:
skills/integrations/consumer/nemo-fabric-integrate/references/config-mapping.mddocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterconfigsupport.mdxdocs/sdk/python.mdxadapters/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:
skills/integrations/consumer/nemo-fabric-integrate/references/config-mapping.mdadapters/README.md
skills/**
📄 CodeRabbit inference engine (AGENTS.md)
skills/**: Keep consumer skills underskills/self-contained and dependent only on supported public Python SDK contracts and published documentation; do not add repository-internal contribution guidance.
Keep consumer skills in parity with the public SDK guide, model, and type details when the Python/Rust binding contract changes.
Files:
skills/integrations/consumer/nemo-fabric-integrate/references/config-mapping.md
**/*.{md,mdx}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
**/*.{md,mdx}: Use the full product nameNVIDIA NeMo Fabricon its first usage, typically in the title or H1; useNeMo Fabricthereafter.
Usefabricby itself only when referring to the CLI tool, and surround those references with backticks.
CapitalizeNVIDIAcorrectly in public documentation.
Format commands, code elements, expressions, file names, paths, and filenames as inline code where needed.
Use title case consistently for headings in technical documentation.
Introduce code blocks, tables, and lists with complete lead-in sentences.
Use descriptive anchor text instead of raw URLs or generic link text such ashere.
Prefer active voice, present tense, short sentences, and plain English.
Use consistent terminology for the same concept throughout a document.
Write procedures as imperative, parallel, easy-to-scan steps, and split long sequences into smaller tasks.
Useafterinstead ofoncewhen expressing temporal sequence.
Usecaninstead ofmaywhen the intended meaning is 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.
Introduce examples' code blocks with full sentences and ensure examples match current APIs and build commands.For docs site changes, run
just docsto regenerate Python and Rust API references and validate the Fern configuration.
Files:
skills/integrations/consumer/nemo-fabric-integrate/references/config-mapping.mddocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterconfigsupport.mdxdocs/sdk/python.mdxadapters/README.md
**/*.{rs,py,html,md,mdx,toml,yml,yaml,sh,bash}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
All Rust, Python, HTML, Markdown, MDX, TOML, YAML, and shell source files must include the project SPDX copyright and Apache-2.0 license headers using the comment syntax appropriate to each file type.
Files:
skills/integrations/consumer/nemo-fabric-integrate/references/config-mapping.mddocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterconfigsupport.mdxdocs/sdk/python.mdxtests/python/test_typed_config.pyadapters/README.mdtests/python/test_native_sdk.pytests/adapters/test_claude_adapter.pytests/adapters/test_codex_adapter.pycrates/fabric-core/src/config.rs
**/{README.md,*.md,*.mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant documentation when changes affect public behavior, adapters, examples, or workspace structure.
Files:
skills/integrations/consumer/nemo-fabric-integrate/references/config-mapping.mddocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterconfigsupport.mdxdocs/sdk/python.mdxadapters/README.md
**/*.md
📄 CodeRabbit inference engine (.agents/skills/README.md)
Documentation and examples must be updated consistently with changes to public behavior and reviewed for NVIDIA technical-writing style.
Files:
skills/integrations/consumer/nemo-fabric-integrate/references/config-mapping.mdadapters/README.md
{*.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:
skills/integrations/consumer/nemo-fabric-integrate/references/config-mapping.mddocs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterconfigsupport.mdxdocs/sdk/python.mdxadapters/README.md
{README.md,docs/**/*.{md,mdx,yml},examples/**/*.{md,mdx,yml}}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep package names, repository references, and build commands current in documentation and examples.
Files:
docs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterconfigsupport.mdxdocs/sdk/python.mdx
{docs/**/*.{md,mdx,yml},examples/**/*.{md,mdx,yml}}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Update relevant getting-started, reference, adapter, and example documentation when the corresponding examples or adapters change.
Files:
docs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterconfigsupport.mdxdocs/sdk/python.mdx
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
In MDX files, use JSX comment delimiters (
{/*and*/}) for top-of-file comments, including SPDX headers; do not use HTML comments.Use
{/* ... */}delimiters for top-of-file MDX SPDX comments, not HTML comment delimiters.
Files:
docs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterconfigsupport.mdxdocs/sdk/python.mdx
docs/**/*.{md,mdx,yml}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Run
just docswhen the documentation site changes.
Files:
docs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterconfigsupport.mdxdocs/sdk/python.mdx
docs/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Update Fern documentation under
docs/when public behavior, thenemo-fabricpackage, examples, or supported bindings change.
Files:
docs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterconfigsupport.mdxdocs/sdk/python.mdx
docs/reference/api/**
📄 CodeRabbit inference engine (AGENTS.md)
Regenerate or update generated API references under
docs/reference/api/when the public API changes.Treat all files under
docs/reference/api/as generated output and do not modify them directly.
Files:
docs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterconfigsupport.mdx
{docs/**,README.md,AGENTS.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,AGENTS.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency with generated schemas.
For links between files under docs/, require paths relative to the source file with the target file's .mdx extension so they work in both Fern builds and repository browsers. Flag Fern site-root links such as NeMo Fabric overview; use the repository-relative equivalent, such as NeMo Fabric overview.
Files:
docs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterconfigsupport.mdxdocs/sdk/python.mdx
docs/sdk/python.mdx
📄 CodeRabbit inference engine (AGENTS.md)
Keep
docs/sdk/python.mdxcurrent when the public Python API changes.
Files:
docs/sdk/python.mdx
**/*.{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:
crates/fabric-cli/assets/adapters/codex/fabric-adapter.jsonadapters/claude/fabric-adapter.jsonadapters/codex/fabric-adapter.jsoncrates/fabric-cli/assets/adapters/claude/fabric-adapter.jsontests/python/test_typed_config.pyschemas/run-plan.schema.jsonexamples/harbor/swebench/adapters/claude/fabric-adapter.jsonschemas/adapter-descriptor.schema.jsontests/python/test_native_sdk.pytests/adapters/test_claude_adapter.pytests/adapters/test_codex_adapter.pycrates/fabric-core/src/config.rs
**/*.{json,jsonschema}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Public contract changes must keep checked-in JSON Schema snapshots synchronized.
Files:
crates/fabric-cli/assets/adapters/codex/fabric-adapter.jsonadapters/claude/fabric-adapter.jsonadapters/codex/fabric-adapter.jsoncrates/fabric-cli/assets/adapters/claude/fabric-adapter.jsonschemas/run-plan.schema.jsonexamples/harbor/swebench/adapters/claude/fabric-adapter.jsonschemas/adapter-descriptor.schema.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/codex/fabric-adapter.jsonadapters/README.mdexamples/harbor/swebench/adapters/claude/fabric-adapter.json
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.
**/*.{rs,py}: When changing the Rust core or public schemas, run both the Rust and Python test suites.
When adding functionality, include tests in the corresponding Rust crate or the relevant area undertests/.
Files:
tests/python/test_typed_config.pytests/python/test_native_sdk.pytests/adapters/test_claude_adapter.pytests/adapters/test_codex_adapter.pycrates/fabric-core/src/config.rs
**/*.{py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Python code or a Python-facing adapter changes, run
just test-python.
Files:
tests/python/test_typed_config.pytests/python/test_native_sdk.pytests/adapters/test_claude_adapter.pytests/adapters/test_codex_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/python/test_typed_config.pytests/python/test_native_sdk.pytests/adapters/test_claude_adapter.pytests/adapters/test_codex_adapter.pycrates/fabric-core/src/config.rs
**/*.{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.
Files:
tests/python/test_typed_config.pytests/python/test_native_sdk.pytests/adapters/test_claude_adapter.pytests/adapters/test_codex_adapter.pycrates/fabric-core/src/config.rs
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 and run by the async runner.
Do not add-> Nonereturn type annotations to test functions.
When mocking a class, useunittest.mock.MagicMockorunittest.mock.AsyncMock, using thespecargument when necessary, rather than defining a new class.
Prefix mocked class names withmock, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; place fixtures needed by multiple test files inconftest.py.
Define fixtures using@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and a function named<fixture_name>_fixture; specifyscopeonly when it is notfunction.
Preferpytest.mark.parametrizeover separate tests for different input types.
Use@pytest.mark.usefixtureswhen a fixture is needed but its return value is unused or it does not return a value.
Use the autouserestore_environ_fixturefromtests/conftest.pyto restore environment variables; modify variables withos.environand do not usemonkeypatch.setenv.
Avoid defensive programming in tests; access expected data directly so missing data raises a clear failure, such as usingresults["data"]instead ofresults.get("data").
Files:
tests/python/test_typed_config.pytests/python/test_native_sdk.pytests/adapters/test_claude_adapter.pytests/adapters/test_codex_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/python/test_typed_config.pytests/python/test_native_sdk.pytests/adapters/test_claude_adapter.pytests/adapters/test_codex_adapter.pycrates/fabric-core/src/config.rs
**/*.{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/python/test_typed_config.pytests/python/test_native_sdk.pytests/adapters/test_claude_adapter.pytests/adapters/test_codex_adapter.pycrates/fabric-core/src/config.rs
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use type annotations for public Python APIs.
Files:
tests/python/test_typed_config.pytests/python/test_native_sdk.pytests/adapters/test_claude_adapter.pytests/adapters/test_codex_adapter.py
**/*.{py,rs}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{py,rs}: Keep native Python binding declarations synchronized with their Rust implementations.
Usesnake_casefor functions and variables; usePascalCasefor Rust types and Python classes.
Files:
tests/python/test_typed_config.pytests/python/test_native_sdk.pytests/adapters/test_claude_adapter.pytests/adapters/test_codex_adapter.pycrates/fabric-core/src/config.rs
{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/python/test_typed_config.pytests/python/test_native_sdk.pytests/adapters/test_claude_adapter.pytests/adapters/test_codex_adapter.py
**/README.md
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
Update relevant package, crate, adapter, and integration README files when public behavior or entry-point documentation changes.
Files:
adapters/README.md
schemas/**/*
⚙️ CodeRabbit configuration file
schemas/**/*: Schemas are generated public contract snapshots. Check that schema diffs correspond to intentional Rust type changes and are covered by core tests.
Files:
schemas/run-plan.schema.jsonschemas/adapter-descriptor.schema.json
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.pytests/adapters/test_codex_adapter.py
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Implement new runtime or binding behavior in the shared Rust core first.
**/*.rs: Format Rust code withcargo fmt --all; Rust formatting checks must pass withcargo fmt --all -- --check.
Rust workspace changes must passcargo check --workspace --locked.
Files:
crates/fabric-core/src/config.rs
**/*.{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.
Files:
crates/fabric-core/src/config.rs
crates/fabric-core/**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes under
crates/fabric-coremust run both the Rust and Python test suites.
Files:
crates/fabric-core/src/config.rs
crates/fabric-core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
For Rust API reference changes, update Rust documentation comments under
crates/fabric-core/instead of editing generated reference output.If
crates/fabric-corechanges in a way exposed through Python, run both the Rust and Python suites.
Files:
crates/fabric-core/src/config.rs
**/*.{rs,rmeta}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Rust code changes, run
cargo fmt --all -- --checkandjust test-rust.
Files:
crates/fabric-core/src/config.rs
crates/fabric-core/src/**/*.rs
⚙️ CodeRabbit configuration file
crates/fabric-core/src/**/*.rs: Review the Rust core for runtime lifecycle correctness, handle validation, capability routing accuracy, schema stability, and error semantics.
Public API changes should match committed schemas, tests, and documentation.
Files:
crates/fabric-core/src/config.rs
🧠 Learnings (2)
📚 Learning: 2026-07-24T16:07:22.255Z
Learnt from: AjayThorve
Repo: NVIDIA/NeMo-Fabric PR: 118
File: docs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterdescriptor.mdx:5-5
Timestamp: 2026-07-24T16:07:22.255Z
Learning: In this repo, files generated under `docs/reference/api/**` are NVIDIA NeMo Fabric API reference output. When reviewing changes to these generated pages, do not treat sidebar `position`/ordering updates as direct manual edits—these can be regenerated by running `just docs` after adding public types. Only flag substantive content changes that are not explained by generation.
Applied to files:
docs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterconfigsupport.mdx
📚 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:
crates/fabric-cli/assets/adapters/codex/fabric-adapter.jsonadapters/claude/fabric-adapter.jsonadapters/codex/fabric-adapter.jsoncrates/fabric-cli/assets/adapters/claude/fabric-adapter.jsonexamples/harbor/swebench/adapters/claude/fabric-adapter.json
🔇 Additional comments (11)
tests/python/test_native_sdk.py (1)
68-80: LGTM!docs/reference/api/rust-library-reference/nemo-fabric-core/config/struct-adapterconfigsupport.mdx (1)
12-26: LGTM!skills/integrations/consumer/nemo-fabric-integrate/references/config-mapping.md (1)
36-38: LGTM!adapters/claude/fabric-adapter.json (1)
19-20: LGTM!adapters/codex/fabric-adapter.json (1)
16-17: LGTM!crates/fabric-cli/assets/adapters/claude/fabric-adapter.json (1)
19-20: LGTM!crates/fabric-cli/assets/adapters/codex/fabric-adapter.json (1)
16-17: LGTM!examples/harbor/swebench/adapters/claude/fabric-adapter.json (1)
19-20: LGTM!tests/adapters/test_claude_adapter.py (1)
90-102: LGTM!Also applies to: 129-153, 384-389, 407-416, 434-434, 458-467, 1050-1050
tests/adapters/test_codex_adapter.py (1)
78-78: LGTM!Also applies to: 183-183, 213-213, 402-432, 590-633, 666-683, 755-755, 859-897, 919-919, 943-951, 964-978, 986-989, 1000-1006, 1061-1061
adapters/README.md (1)
27-28: LGTM!Also applies to: 46-111
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
201399e to
7688ab9
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
1 participant
Overview
Add fail-fast model/provider compatibility checks to planning and
doctor(...)on top of the typedAgentConfigand descriptor-owned schema contract now onmain.This refresh replaces the earlier
config.native_model_providerslist with an optional descriptor-ownedmodel_schema, reusing the existing self-contained JSON Schema validation path. Claude declaresanthropicas its native provider and Codex declaresopenai. Other provider identifiers remain supported when the model supplies bothbase_urlandapi_key_env, and the endpoint implements the protocol expected by the adapter.The schema is applied to every configured model role, not only the selected/default role. It also closes
ModelConfig.settingsfor Claude and Codex, so undeclared model-scoped settings fail planning and appear as field-specificdoctor(...)checks. An adapter that omitsmodel_schemapreserves its existing dynamic-provider behavior, including Deep Agents and Hermes.This is an additive descriptor change with intentionally stricter preflight behavior. It preserves the contract ownership boundary: Fabric validates static model shape and adapter compatibility before startup, while adapters retain environment-dependent checks such as credential validity, endpoint reachability, and model availability. NVIDIA provider defaults remain outside FABRIC-64 and are tracked separately by FABRIC-93.
Details
AdapterDescriptor::model_schemaand validate it as a self-contained object schema when descriptors are loaded.ModelConfigagainst the selected descriptor while leaving separately validated extensions out of the model-schema payload.base_urlplusapi_key_envfor explicit custom-provider paths.ModelConfig.settingsbefore adapter startup.No dependencies or lockfiles change.
Validation
cargo fmt --all -- --check— passed.cargo check -p fabric-python --locked— passed.just test-rust— 96 workspace tests passed, plus doc-tests.just test-python— 712 passed, 16 skipped.cargo run -p nemo-fabric-core --example generate-schemas --locked -- schemas— generated snapshots match the Rust contract.just docs— API generation and Fern validation passed; authenticated redirect validation was unavailable locally.uv run --with pre-commit pre-commit run --all-files— passed, including attribution and license-diff hooks.git diff --check— passed.mainintegration check (9f0881e6) — conflict-free; schema regeneration produced no diff, andjust test-rust,just test-python, andcargo check -p fabric-python --lockedpassed on the merged tree (96 Rust tests; 719 Python tests passed, 16 skipped).Where should the reviewer start?
Start with
AdapterDescriptor::model_schemaandadapter_config_compatibility_issues(...)incrates/fabric-core/src/config.rs. Then review the Claude/Codex descriptor schemas and the planning/doctor cases intests/python/test_typed_config.py.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Closes FABRIC-64
Relates to feat: define typed adapter contract and publish authoring docs #186
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
New Features
doctorbefore adapter startup.Documentation