feat: validate adapter model compatibility#118
Conversation
WalkthroughThe PR introduces typed normalized configuration fields, separate tool and toolset policies, adapter compatibility validation, runtime environment propagation, and updated Claude, Codex, Deep Agents, and Hermes mappings. SDK models, CLI scaffolding, schemas, tests, examples, and documentation are updated accordingly. ChangesTyped configuration and compatibility
Adapter normalization
SDK, CLI, and validation
Documentation and API references
Estimated code review effort: 5 (Critical) | ~120 minutes 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 |
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
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
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
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
de79c72 to
d088452
Compare
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
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
| def timeout_seconds(payload: dict[str, Any], *, default: float) -> float: | ||
| value = (fabric_config(payload).get("runtime") or {}).get("timeout_seconds") | ||
| return float(default if value is None else value) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
timeout_seconds() silently accepts a boolean config value where max_turns() (two functions above) explicitly rejects one.
def max_turns(payload: dict[str, Any]) -> int | None:
value = fabric_config(payload).get("max_turns")
return value if isinstance(value, int) and not isinstance(value, bool) else None
def timeout_seconds(payload: dict[str, Any], *, default: float) -> float:
value = (fabric_config(payload).get("runtime") or {}).get("timeout_seconds")
return float(default if value is None else value)float(True) is 1.0, so a config value of runtime.timeout_seconds: true (a plausible typo/misconfiguration) silently becomes a 1-second timeout instead of failing validation — directly undermining the fail-closed diagnostics goal this PR is built around. It also neutralizes the downstream isinstance(value, bool) guards in adapters/claude/.../adapter.py and adapters/codex/.../adapter.py, which now check a value that's already been coerced to float by this function and can therefore never be a bool again. A non-numeric type (e.g. a list/dict) would also raise an unstructured TypeError/ValueError here instead of a clean adapter-specific configuration error.
🐛 Proposed fix
def timeout_seconds(payload: dict[str, Any], *, default: float) -> float:
value = (fabric_config(payload).get("runtime") or {}).get("timeout_seconds")
- return float(default if value is None else value)
+ if value is None:
+ return float(default)
+ if isinstance(value, bool) or not isinstance(value, (int, float)):
+ raise ValueError(
+ f"runtime.timeout_seconds must be a number, got {type(value).__name__}"
+ )
+ return float(value)🤖 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/common/src/nemo_fabric_adapters/common/utils.py` around lines 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.
| - selected model provider, model name, base URL, and temperature through | ||
| `models`; | ||
| - top-level `system_prompt` and `max_turns`; | ||
| - workspace and explicit environment variables through `environment`; | ||
| - invocation timeout through `runtime.timeout_seconds`; | ||
| - NeMo Fabric skills as external skill directories for Hermes Agent; | ||
| - NeMo Fabric MCP servers as Hermes Agent MCP server config; | ||
| - `tools.blocked` as disabled toolsets for Hermes Agent, unioned with | ||
| `harness.settings.disabled_toolsets`; | ||
| - `tools.toolsets.enabled` and `tools.toolsets.blocked` as Hermes toolset | ||
| selection and blocking policy; | ||
| - optional NeMo Relay telemetry plugin configuration. | ||
|
|
||
| `hermes_home` configures a base directory. The adapter creates a child under | ||
| `runtimes/<runtime_id>` so invocations in one NeMo Fabric runtime share Hermes Agent state | ||
| without sharing config or the session database with another runtime. | ||
| Per-tool `tools.blocked` is not accepted by Hermes because Hermes exposes | ||
| toolset-level policy at this boundary. Keep Hermes-specific controls such as | ||
| terminal timeout, reasoning configuration, and plugin configuration in | ||
| `harness.settings`. The adapter derives Hermes state from the Fabric artifact | ||
| root and creates a child under `runtimes/<runtime_id>`, so invocations in one | ||
| NeMo Fabric runtime share state without sharing config or the session database | ||
| with another runtime. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remove or implement the runtime.timeout_seconds mapping claim.
build_hermes_config does not consume runtime.timeout_seconds; Hermes currently uses harness.settings.terminal_timeout. The README promises a control that the adapter does not map.
As per coding guidelines, “For NeMo Fabric documentation, verify technical claims against the current repository, public API, or documented command before reviewing style.”
🤖 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/hermes/README.md` around lines 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.
Sources: Coding guidelines, Path instructions
| variables plus values explicitly configured in `environment.env`. | ||
|
|
||
| The adapter pins `claude-agent-sdk==0.2.120`. The SDK owns its compatible Claude | ||
| Code runtime unless `harness.settings.cli_path` explicitly selects another | ||
| executable. | ||
| The adapter pins `claude-agent-sdk==0.2.120`. The SDK owns and selects its | ||
| compatible Claude Code runtime. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Format configuration fields and package pins as code.
Wrap environment.env and claude-agent-sdk==0.2.120 in backticks. The documentation rules require code elements and package names to use inline code formatting.
As per coding guidelines, code elements, package names, and paths must use inline code formatting.
🤖 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/integrations/claude.mdx` around lines 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.
Source: Coding guidelines
| title: "Client" | ||
| slug: "/reference/api/python-library-reference/client" | ||
| description: "Resolve, plan, diagnose, and run agents with Fabric." | ||
| description: "Resolve, plan, diagnose, and run agents with NeMo Fabric." |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the full product name in first-use descriptions.
Both changed descriptions should use NVIDIA NeMo Fabric on first use, then reserve NeMo Fabric for subsequent references.
docs/reference/api/python-library-reference/nemo_fabric.client.md#L4-L4: change the description to useNVIDIA NeMo Fabric.docs/reference/api/python-library-reference/nemo_fabric.models.md#L4-L4: change the description to useNVIDIA NeMo Fabric.
As per coding guidelines, first use must be “NVIDIA NeMo Fabric”; as per path instructions, regenerate these API references through the source or generator.
📍 Affects 2 files
docs/reference/api/python-library-reference/nemo_fabric.client.md#L4-L4(this comment)docs/reference/api/python-library-reference/nemo_fabric.models.md#L4-L4
🤖 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,
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.
Sources: Coding guidelines, Path instructions
| ### <kbd>method</kbd> `configure_toolsets` | ||
|
|
||
| ```python | ||
| configure_toolsets( | ||
| enabled: 'Sequence[str] | None' = None, | ||
| blocked: 'Sequence[str]' = () | ||
| ) → Self | ||
| ``` | ||
|
|
||
| Set adapter-native toolset selection and blocking policy. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve the keyword-only configure_toolsets contract.
The source method in python/src/nemo_fabric/models.py defines configure_toolsets(*, enabled, blocked), but this page renders no *. Users following the documentation can pass positional arguments and receive TypeError; preserve the keyword-only marker in the generator output and rerun just docs.
As per path instructions, update the source or documentation generator instead of patching 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` around
lines 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.
Source: Path instructions
Overview
Extend adapter descriptors so
Fabric.plan(...)andFabric.doctor(...)canvalidate normalized model configuration before starting an adapter.
This PR is stacked on #117 and should merge after it. Once #117 lands, this
branch will be rebased so the final diff contains only the FABRIC-64 descriptor
extension.
The descriptor can now declare:
models.<role>.providervalues; andmodels.<role>.settingswith required flags anddescriptions.
Claude declares
anthropicandnvidia; Codex declaresopenaiandnvidia.Adapters without a provider allowlist retain dynamic-provider behavior. No
first-party adapter currently declares provider-specific setting keys.
Details
Planning and doctor now share the same fail-closed diagnostics for:
Diagnostics identify the canonical field path without including secret setting
values.
The complete normalized compatibility matrix remains:
FabricConfigfieldschema_versionmetadata.name,.descriptionharness.adapter_id,.resolutionharness.settingsmodels.<role>.provideranthropic,nvidiaopenai,nvidiamodels.<role>.modelmodels.<role>.api_key_envmodels.<role>.base_urlmodels.<role>.temperaturemodels.<role>.settings.<key>system_promptmax_turnsruntime.input_schema,.output_schemaruntime.artifacts,.timeout_secondsenvironment.provider,.control_location,.ownershipenvironment.workspace,.artifacts,.envenvironment.connection,.metadata,.settingstools.blockedtools.toolsets.enabled,.blockedskills.pathsmcp.servers.<name>.transport,.urlwithharness_nativeexposuremcp.servers.<name>.exposure = "fabric_managed"telemetry.providers.relaytelemetry.providers.nativetelemetry.providers.<provider>.configrelay.project,.output_dir,.observabilityrelay.components,.policyextensionson typed config objectsBreaking behavior: Claude and Codex provider mismatches, and undeclared
models.<role>.settingskeys, now fail before adapter startup instead ofreaching adapter-native validation or being ignored.
Validation
RUSTUP_TOOLCHAIN=1.94.0 just test-rust— 54 tests passed, plus doc tests.RUSTUP_TOOLCHAIN=1.94.0 cargo check -p fabric-python --locked— passed.RUSTUP_TOOLCHAIN=1.94.0 ... just docs— Fern and generated API referencechecks passed; redirect validation was skipped because this machine is not
authenticated to Fern.
Where should the reviewer start?
Start with
AdapterModelConfigSupport,AdapterSettingSchema, andvalidate_model_compatibilityincrates/fabric-core/src/config.rs. Then reviewthe Claude and Codex
fabric-adapter.jsondeclarations and the sharedplan/doctor tests in
tests/python/test_typed_config.py.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Depends on feat: normalize portable adapter configuration #117
Closes FABRIC-64
Relates to FABRIC-93
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
Bug Fixes
Documentation