fix: discover installed adapter descriptors#108
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe change adds interpreter-specific adapter descriptor discovery, threads discovered directories through core run-plan resolution, preserves agent-local overrides, documents scan locations, and adds Rust and Python coverage. ChangesAdapter descriptor discovery
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant FabricPython
participant sysconfig
participant FabricCore
participant AdapterRegistry
FabricPython->>sysconfig: query selected interpreter data path
sysconfig-->>FabricPython: return data directory
FabricPython->>FabricCore: resolve plan with adapter directory
FabricCore->>AdapterRegistry: register installed descriptors
FabricCore->>AdapterRegistry: register base_dir/adapters
AdapterRegistry-->>FabricCore: resolve descriptor with local precedence
FabricCore-->>FabricPython: return run plan
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Fern docs preview: https://nvidia-preview-pull-request-108.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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/fabric-core/src/config.rs`:
- Around line 1741-1814: Update
resolves_additional_adapter_directory_before_agent_local_override to use a scope
guard for the temporary root directory, ensuring remove_dir_all(root) runs
during normal completion and unwinding after any assertion panic. Remove the
cleanup-only-at-the-end approach while preserving the existing test setup and
assertions.
In `@crates/fabric-python/src/lib.rs`:
- Around line 26-34: Extract the repeated context resolution and detached plan
construction into a shared helper, such as resolve_plan, accepting py,
config_json, and base_dir and returning PyResult<RunPlan>. Update plan_config,
doctor_config, and run_config to call this helper while preserving their
existing inputs and error propagation.
In `@tests/python/test_installed_adapter_discovery.py`:
- Around line 42-59: Extract the duplicated sysconfig.get_path monkeypatching
from both tests into a pytest fixture named patch_sysconfig_data, with typed
nested wrapper parameters and return values. Update each test to use this
fixture, while preserving the existing data-root behavior and assertions; keep
_config and _write_descriptor usage unchanged unless existing fixture
conventions provide replacements.
🪄 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: e711384d-b314-4df8-b75f-a8a0a8e32752
📒 Files selected for processing (5)
adapters/README.mdcrates/fabric-core/src/config.rscrates/fabric-core/src/lib.rscrates/fabric-python/src/lib.rstests/python/test_installed_adapter_discovery.py
📜 Review details
⏰ Context from checks skipped due to timeout. (7)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Pre-commit
⚠️ CI failures not shown inline (6)
GitHub Actions: Rust / Test (x86_64): Discover installed adapter descriptors
Conclusion: failure
ring("harness"), String("runtime")], "type": String("object")}, "HarnessConfig": Object {"additionalProperties": Bool(true), "description": String("Harness selection."), "properties": Object {"adapter_id": Object {"description": String("Adapter implementation id."), "type": String("string")}, "resolution": Object {"anyOf": Array [Object {"$ref": String("`#/`$defs/ResolutionStrategy")}, Object {"type": String("null")}], "description": String("Selected install or availability strategy.")}, "settings": Object {"additionalProperties": Bool(true), "description": String("Harness-specific settings."), "type": String("object")}}, "required": Array [String("adapter_id")], "type": String("object")}, "McpConfig": Object {"additionalProperties": Bool(true), "description": String("MCP capability configuration."), "properties": Object {"servers": Object {"additionalProperties": Object {"$ref": String("`#/`$defs/McpServerConfig")}, "description": String("Named MCP servers."), "type": String("object")}}, "type": String("object")}, "McpExposure": Object {"description": String("MCP exposure strategy."), "oneOf": Array [Object {"const": String("harness_native"), "description": String("Map into harness-native MCP config through the selected adapter."), "type": String("string")}, Object {"const": String("fabric_managed"), "description": String("Fabric manages MCP and exposes basic tools/actions."), "type": String("string")}]}, "McpServerConfig": Object {"additionalProperties": Bool(true), "description": String("MCP server configuration."), "properties": Object {"exposure": Object {"$ref": String("`#/`$defs/McpExposure"), "description": String("How Fabric exposes the MCP capability to the harness.")}, "transport": Object {"description": String("MCP transport."), "type": String("string")}, "url": Object {"description": String("MCP server URL or process command, depending on transport."), "type": String("string")}}, "required": Array [String("transport"), String("url"), String("exposure")], "...
GitHub Actions: Rust / 0_Test (x86_64).txt: Discover installed adapter descriptors
Conclusion: failure
##[group]Run bail() {
�[36;1mbail() {�[0m
�[36;1m printf '::error::install-action: %s\n' "$*"�[0m
GitHub Actions: Rust / Test (x86_64): Discover installed adapter descriptors
Conclusion: failure
##[group]Run bail() {
�[36;1mbail() {�[0m
�[36;1m printf '::error::install-action: %s\n' "$*"�[0m
GitHub Actions: Rust / Test (arm64): Discover installed adapter descriptors
Conclusion: failure
ring("harness"), String("runtime")], "type": String("object")}, "HarnessConfig": Object {"additionalProperties": Bool(true), "description": String("Harness selection."), "properties": Object {"adapter_id": Object {"description": String("Adapter implementation id."), "type": String("string")}, "resolution": Object {"anyOf": Array [Object {"$ref": String("`#/`$defs/ResolutionStrategy")}, Object {"type": String("null")}], "description": String("Selected install or availability strategy.")}, "settings": Object {"additionalProperties": Bool(true), "description": String("Harness-specific settings."), "type": String("object")}}, "required": Array [String("adapter_id")], "type": String("object")}, "McpConfig": Object {"additionalProperties": Bool(true), "description": String("MCP capability configuration."), "properties": Object {"servers": Object {"additionalProperties": Object {"$ref": String("`#/`$defs/McpServerConfig")}, "description": String("Named MCP servers."), "type": String("object")}}, "type": String("object")}, "McpExposure": Object {"description": String("MCP exposure strategy."), "oneOf": Array [Object {"const": String("harness_native"), "description": String("Map into harness-native MCP config through the selected adapter."), "type": String("string")}, Object {"const": String("fabric_managed"), "description": String("Fabric manages MCP and exposes basic tools/actions."), "type": String("string")}]}, "McpServerConfig": Object {"additionalProperties": Bool(true), "description": String("MCP server configuration."), "properties": Object {"exposure": Object {"$ref": String("`#/`$defs/McpExposure"), "description": String("How Fabric exposes the MCP capability to the harness.")}, "transport": Object {"description": String("MCP transport."), "type": String("string")}, "url": Object {"description": String("MCP server URL or process command, depending on transport."), "type": String("string")}}, "required": Array [String("transport"), String("url"), String("exposure")], "...
GitHub Actions: Rust / Test (arm64): Discover installed adapter descriptors
Conclusion: failure
##[group]Run bail() {
�[36;1mbail() {�[0m
�[36;1m printf '::error::install-action: %s\n' "$*"�[0m
GitHub Actions: Rust / 1_Test (arm64).txt: Discover installed adapter descriptors
Conclusion: failure
##[group]Run bail() {
�[36;1mbail() {�[0m
�[36;1m printf '::error::install-action: %s\n' "$*"�[0m
🧰 Additional context used
📓 Path-based instructions (28)
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Update documentation and examples in the same branch as the public API change.
Files:
adapters/README.md
**/*
📄 CodeRabbit inference engine (.agents/skills/karpathy-guidelines/SKILL.md)
**/*: Before implementing, explicitly state assumptions, surface ambiguity and tradeoffs, present multiple interpretations when relevant, and ask for clarification rather than silently deciding or proceeding when requirements are unclear.
Prefer the minimum code needed to solve the requested problem: avoid speculative features, unnecessary abstractions, unrequested flexibility, and handling of impossible scenarios; simplify overcomplicated solutions.
When editing existing code, make surgical changes only: do not modify unrelated code, comments, formatting, or pre-existing dead code; match the existing style, and remove only unused imports, variables, or functions introduced by your changes.
Define verifiable success criteria for each task, such as writing regression tests for bugs and invalid-input tests for validation, then verify the implementation against those criteria. For multi-step work, state a brief plan with a verification check for each step.
**/*: Always spellNVIDIAin all caps; do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun, because the name begins with an “en” sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company; use trademark symbols with product names only when required by the document type or legal guidance.
Verify official capitalization, spacing, hyphenation, and spelling for NVIDIA and third-party product names; do not rewrite official product names for grammar or title-case rules.
Precede NVIDIA product names withNVIDIAon first mention when natural and accurate, and link the first mention when the destination helps the reader.
On first use, include the company name and full model qualifier when it helps identify the model; preserve official capitalization and punctuation, and use shorter family names only after establishing the full name.
For learning-oriented and developer content, do not force trademark symbols unless explicitly required; for press, ...
Files:
adapters/README.mdcrates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-python/src/lib.rscrates/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:
adapters/README.md
**/*.{md,rst,txt,adoc}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-language-mechanics.md)
**/*.{md,rst,txt,adoc}: For technical documentation, use professional, active, conversational, engaging, precise, and plain-English prose. Prefer active voice, present tense, short sentences, and scannable paragraphs. Avoid casual or imprecise language, swearing, threats, insults, jokes, puns, culture-specific idioms, marketing exaggeration, and unsupported third-party comparisons.
Usecanfor possibility and reservemayfor permission; useafterfor temporal order; userefer tofor cross-references; prefer short direct sentences and specific verbs; avoid unnecessarypleasein technical documentation.
Prefer active voice when the actor matters. Passive voice is acceptable when the actor is unknown or irrelevant, when the action or result is the focus, or in programmer documentation.
Use natural contractions in conversational technical prose, but do not force them in formal legal copy, API references, or generated text.
Prefer simpler English over Latinisms: usefor exampleorsuch asinstead ofe.g.,and so oninstead ofetc.,that isinstead ofi.e.,compared toinstead ofvs., andby,through, orusinginstead ofvia. Use industry-standard terms such as in silico, in vitro, and in vivo when appropriate, and italicize them in running text.
Usethatwithout commas for essential clauses, andwhichwith commas for nonessential clauses.
Format dates and times clearly: spell out months in body text; use forms such asJune 12, 2025; avoid numeric or ordinal dates; capitalize days; use 12-hour time when appropriate; include a space beforea.m.orp.m.; useETandPTfor needed time zones; avoid24/7; and preferfrom 12:30 to 1:00 p.m.for prose ranges.
Format numbers consistently: spell out zero through nine in body text, use numerals for 10 or greater and for technical values, use commas in thousands, do not begin a sentence with a numeral, spell out ordinals, and use numerals consistently within a category wh...
Files:
adapters/README.md
**/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
**/*.{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 documentation-site changes, run
just docsto regenerate Python and Rust API references and validate Fern configuration.
Files:
adapters/README.md
**/{README.md,*.md,*.mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant SDK, API, adapter, example, integration, and embedded documentation when public behavior or the corresponding surface changes.
Files:
adapters/README.md
**/*.{html,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
HTML and Markdown files must use the specified HTML comment form for SPDX license headers.
Files:
adapters/README.md
{adapters/**,examples/**}
⚙️ CodeRabbit configuration file
{adapters/**,examples/**}: Review adapter and example changes for command correctness, config/schema consistency, artifact handling, and compatibility with the public NeMo Fabric contracts.
Files:
adapters/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:
adapters/README.md
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Implement new runtime or binding behavior in the shared Rust core first.
Files:
crates/fabric-core/src/lib.rscrates/fabric-python/src/lib.rscrates/fabric-core/src/config.rs
**/*.{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-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-python/src/lib.rscrates/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.
Use Rust stable tooling; format Rust code with
cargo fmt --all, check formatting withcargo fmt --all -- --check, and compile withcargo check --workspace --locked.
Files:
crates/fabric-core/src/lib.rscrates/fabric-python/src/lib.rscrates/fabric-core/src/config.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.
Files:
crates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-python/src/lib.rscrates/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/lib.rscrates/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/lib.rscrates/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/lib.rscrates/fabric-python/src/lib.rscrates/fabric-core/src/config.rs
**/*.{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.
**/*.{rs,py,pyi}: Usesnake_casefor Rust and Python functions and variables; usePascalCasefor Rust types and Python classes.
Keep native Python binding declarations synchronized with their Rust implementations when public contracts change.
Files:
crates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-python/src/lib.rscrates/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:
crates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-python/src/lib.rscrates/fabric-core/src/config.rs
**/*.{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:
crates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-python/src/lib.rscrates/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:
crates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-python/src/lib.rscrates/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/lib.rscrates/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.Use type annotations for public Python APIs and keep native Python binding declarations synchronized with their Rust implementations.
Files:
tests/python/test_installed_adapter_discovery.py
tests/**/*.{rs,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the corresponding Rust crate or the relevant area under
tests/.
Files:
tests/python/test_installed_adapter_discovery.py
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Use Pytest to run Python tests.
Do not add@pytest.mark.asyncioto tests; async tests are automatically detected 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_installed_adapter_discovery.py
{tests/**,python/tests/**}
⚙️ CodeRabbit configuration file
{tests/**,python/tests/**}: Tests should cover the behavior promised by the changed API surface, including error paths, lifecycle cleanup, and SDK/native parity where relevant.
Files:
tests/python/test_installed_adapter_discovery.py
crates/fabric-python/**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If the PyO3 bridge or package metadata changes, run
just build-pythonandcargo check -p fabric-python --locked.
Files:
crates/fabric-python/src/lib.rs
crates/fabric-python/**/*
⚙️ CodeRabbit configuration file
crates/fabric-python/**/*: Treat native binding changes as public API changes. Check JSON/type conversion, error propagation, GIL/thread behavior, and parity with the Python SDK.
Files:
crates/fabric-python/src/lib.rs
🪛 ast-grep (0.44.1)
tests/python/test_installed_adapter_discovery.py
[info] 19-27: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{
"contract_version": "fabric.adapter/v1alpha1",
"adapter_id": "test.fabric.installed",
"harness": "installed-test",
"adapter_kind": "python",
"runner": {"module": module},
}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 69-77: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{
"contract_version": "fabric.adapter/v1alpha1",
"adapter_id": "test.fabric.installed",
"harness": "installed-test",
"adapter_kind": "python",
"runner": {"module": "local.adapter"},
}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Ruff (0.15.21)
tests/python/test_installed_adapter_discovery.py
[warning] 49-49: Missing return type annotation for private function get_path
(ANN202)
[warning] 49-49: Missing type annotation for *args
(ANN002)
[warning] 49-49: Missing type annotation for **kwargs
(ANN003)
[warning] 82-82: Missing return type annotation for private function get_path
(ANN202)
[warning] 82-82: Missing type annotation for *args
(ANN002)
[warning] 82-82: Missing type annotation for **kwargs
(ANN003)
🔇 Additional comments (4)
crates/fabric-core/src/config.rs (1)
133-138: LGTM!Also applies to: 167-179, 996-1023, 1039-1044, 1067-1087
crates/fabric-core/src/lib.rs (1)
20-20: LGTM!adapters/README.md (1)
16-30: LGTM!crates/fabric-python/src/lib.rs (1)
170-188: 🩺 Stability & AvailabilityNo issue:
sysconfig.get_path("data")is a standard CPython path lookup. This doesn’t introduce a new failure path forplan/doctor/run.> Likely an incorrect or invalid review comment.
c16d828 to
db0058a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@adapters/README.md`:
- Around line 16-30: Add the repository-standard SPDX copyright and Apache-2.0
license header as HTML comments at the beginning of the README, before the
“Descriptor Discovery” heading. Follow the exact header format used by other
Markdown files in the repository.
In `@tests/python/test_installed_adapter_discovery.py`:
- Around line 56-59: Extend the regression test around Fabric().plan to assert
that the serialized adapter_descriptor provenance has source set to local, in
addition to verifying the descriptor path. Use the existing
plan["adapter_descriptor"] structure so the test protects the documented RunPlan
contract.
🪄 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: 27d297c7-9721-4889-ba72-d65c9d281964
📒 Files selected for processing (5)
adapters/README.mdcrates/fabric-core/src/config.rscrates/fabric-core/src/lib.rscrates/fabric-python/src/lib.rstests/python/test_installed_adapter_discovery.py
📜 Review details
⏰ Context from checks skipped due to timeout. (7)
- GitHub Check: Preview docs
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.12, linux-arm64)
- GitHub Check: Pre-commit
🧰 Additional context used
📓 Path-based instructions (28)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Implement new runtime or binding behavior in the shared Rust core first.
Files:
crates/fabric-core/src/lib.rscrates/fabric-python/src/lib.rscrates/fabric-core/src/config.rs
**/*.{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-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-python/src/lib.rscrates/fabric-core/src/config.rs
**/*
📄 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:
crates/fabric-core/src/lib.rsadapters/README.mdtests/python/test_installed_adapter_discovery.pycrates/fabric-python/src/lib.rscrates/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.
Use Rust stable tooling; format Rust code with
cargo fmt --all, check formatting withcargo fmt --all -- --check, and compile withcargo check --workspace --locked.
Files:
crates/fabric-core/src/lib.rscrates/fabric-python/src/lib.rscrates/fabric-core/src/config.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.
Files:
crates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-python/src/lib.rscrates/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/lib.rscrates/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/lib.rscrates/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/lib.rscrates/fabric-python/src/lib.rscrates/fabric-core/src/config.rs
**/*.{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.
**/*.{rs,py,pyi}: Usesnake_casefor Rust and Python functions and variables; usePascalCasefor Rust types and Python classes.
Keep native Python binding declarations synchronized with their Rust implementations when public contracts change.
Files:
crates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-python/src/lib.rscrates/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:
crates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-python/src/lib.rscrates/fabric-core/src/config.rs
**/*.{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:
crates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-python/src/lib.rscrates/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:
crates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-python/src/lib.rscrates/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/lib.rscrates/fabric-core/src/config.rs
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Update documentation and examples in the same branch as the public API change.
Files:
adapters/README.md
**/*.{md,mdx,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,mdx,rst}: For NeMo Fabric documentation, verify technical claims against the current repository, public API, or documented command before reviewing style.
Always spellNVIDIAin all caps; do not useNvidia,nvidia, orNV.
Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text; avoid raw URLs and weak anchors such ashereorread more.
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative, parallel steps; split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English while preserving necessary technical precision.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce, and preferrefer tooverseewhen directing readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical documentation.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values; use numerals for 10 or greater and commas in thousands.
Do not add trademark symbols to learning-oriented documentation unless the source, platform, or legal guidance explicitly requires them.
Do not replace precise technical terms with simpler words when doing so would lose precision.
Do not flag passive voice when the actor is unknown or the action is the important part.
Do not rewrite API names, package names, command flags, or code literals for style.
**/*.{md,mdx,rst}: Use consistent title case for technical-document headings and table headers; avoid quotation marks, ampersands, and exclamation marks in headings, while preserving official product, event, research, and whitepaper title ...
Files:
adapters/README.md
**/*.{md,rst,txt,adoc}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-language-mechanics.md)
**/*.{md,rst,txt,adoc}: For technical documentation, use professional, active, conversational, engaging, precise, and plain-English prose. Prefer active voice, present tense, short sentences, and scannable paragraphs. Avoid casual or imprecise language, swearing, threats, insults, jokes, puns, culture-specific idioms, marketing exaggeration, and unsupported third-party comparisons.
Usecanfor possibility and reservemayfor permission; useafterfor temporal order; userefer tofor cross-references; prefer short direct sentences and specific verbs; avoid unnecessarypleasein technical documentation.
Prefer active voice when the actor matters. Passive voice is acceptable when the actor is unknown or irrelevant, when the action or result is the focus, or in programmer documentation.
Use natural contractions in conversational technical prose, but do not force them in formal legal copy, API references, or generated text.
Prefer simpler English over Latinisms: usefor exampleorsuch asinstead ofe.g.,and so oninstead ofetc.,that isinstead ofi.e.,compared toinstead ofvs., andby,through, orusinginstead ofvia. Use industry-standard terms such as in silico, in vitro, and in vivo when appropriate, and italicize them in running text.
Usethatwithout commas for essential clauses, andwhichwith commas for nonessential clauses.
Format dates and times clearly: spell out months in body text; use forms such asJune 12, 2025; avoid numeric or ordinal dates; capitalize days; use 12-hour time when appropriate; include a space beforea.m.orp.m.; useETandPTfor needed time zones; avoid24/7; and preferfrom 12:30 to 1:00 p.m.for prose ranges.
Format numbers consistently: spell out zero through nine in body text, use numerals for 10 or greater and for technical values, use commas in thousands, do not begin a sentence with a numeral, spell out ordinals, and use numerals consistently within a category wh...
Files:
adapters/README.md
**/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
**/*.{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 documentation-site changes, run
just docsto regenerate Python and Rust API references and validate Fern configuration.
Files:
adapters/README.md
**/{README.md,*.md,*.mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant SDK, API, adapter, example, integration, and embedded documentation when public behavior or the corresponding surface changes.
Files:
adapters/README.md
**/*.{html,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
HTML and Markdown files must use the specified HTML comment form for SPDX license headers.
Files:
adapters/README.md
{adapters/**,examples/**}
⚙️ CodeRabbit configuration file
{adapters/**,examples/**}: Review adapter and example changes for command correctness, config/schema consistency, artifact handling, and compatibility with the public NeMo Fabric contracts.
Files:
adapters/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:
adapters/README.md
**/*.{py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Python code or a Python-facing adapter changes, run
just test-python.Use type annotations for public Python APIs and keep native Python binding declarations synchronized with their Rust implementations.
Files:
tests/python/test_installed_adapter_discovery.py
tests/**/*.{rs,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the corresponding Rust crate or the relevant area under
tests/.
Files:
tests/python/test_installed_adapter_discovery.py
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Use Pytest to run Python tests.
Do not add@pytest.mark.asyncioto tests; async tests are automatically detected 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_installed_adapter_discovery.py
{tests/**,python/tests/**}
⚙️ CodeRabbit configuration file
{tests/**,python/tests/**}: Tests should cover the behavior promised by the changed API surface, including error paths, lifecycle cleanup, and SDK/native parity where relevant.
Files:
tests/python/test_installed_adapter_discovery.py
crates/fabric-python/**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If the PyO3 bridge or package metadata changes, run
just build-pythonandcargo check -p fabric-python --locked.
Files:
crates/fabric-python/src/lib.rs
crates/fabric-python/**/*
⚙️ CodeRabbit configuration file
crates/fabric-python/**/*: Treat native binding changes as public API changes. Check JSON/type conversion, error propagation, GIL/thread behavior, and parity with the Python SDK.
Files:
crates/fabric-python/src/lib.rs
🪛 ast-grep (0.44.1)
tests/python/test_installed_adapter_discovery.py
[info] 19-27: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{
"contract_version": "fabric.adapter/v1alpha1",
"adapter_id": "test.fabric.installed",
"harness": "installed-test",
"adapter_kind": "python",
"runner": {"module": module},
}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 69-77: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{
"contract_version": "fabric.adapter/v1alpha1",
"adapter_id": "test.fabric.installed",
"harness": "installed-test",
"adapter_kind": "python",
"runner": {"module": "local.adapter"},
}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Ruff (0.15.21)
tests/python/test_installed_adapter_discovery.py
[warning] 49-49: Missing return type annotation for private function get_path
(ANN202)
[warning] 49-49: Missing type annotation for *args
(ANN002)
[warning] 49-49: Missing type annotation for **kwargs
(ANN003)
[warning] 82-82: Missing return type annotation for private function get_path
(ANN202)
[warning] 82-82: Missing type annotation for *args
(ANN002)
[warning] 82-82: Missing type annotation for **kwargs
(ANN003)
🔇 Additional comments (11)
crates/fabric-core/src/config.rs (4)
1743-1813: Existing panic-safe cleanup finding remains.An assertion failure skips
remove_dir_allat Line 1813, leaving the test directory behind. This was already reported on this range.
136-137: LGTM!
167-178: LGTM!
999-1022: LGTM!Also applies to: 1039-1044, 1070-1073
crates/fabric-python/src/lib.rs (3)
26-34: Existing duplicated plan-resolution setup remains.The repeated context-resolution and detached planner calls were already reported on these paths.
Also applies to: 48-56, 74-82
10-10: LGTM!
170-188: LGTM!tests/python/test_installed_adapter_discovery.py (3)
42-54: Existing duplicatedsysconfig.get_pathpatch finding remains.The repeated untyped monkeypatch wrappers were already reported on these ranges.
Also applies to: 61-87
1-39: LGTM!
89-92: LGTM!crates/fabric-core/src/lib.rs (1)
20-20: LGTM!
db0058a to
bd38be6
Compare
AnuradhaKaruppiah
changed the title
bd38be6 to
6572033
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/fabric-python/src/lib.rs`:
- Around line 181-191: Update query_python_data_path to execute the
ADAPTER_PYTHON subprocess with a bounded timeout, and terminate the child
process when the deadline expires before returning an error. Preserve the
existing successful output and error propagation behavior used by the data_path
resolution block.
🪄 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: b8510ea7-880a-4c0a-bc42-97c167bc71b3
📒 Files selected for processing (5)
adapters/README.mdcrates/fabric-core/src/config.rscrates/fabric-core/src/lib.rscrates/fabric-python/src/lib.rstests/python/test_installed_adapter_discovery.py
📜 Review details
⏰ Context from checks skipped due to timeout. (8)
- GitHub Check: Preview docs
- GitHub Check: Test (Python 3.13, macos-arm64)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Pre-commit
🧰 Additional context used
📓 Path-based instructions (28)
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Update documentation and examples in the same branch as the public API change.
Files:
adapters/README.md
**/*
📄 CodeRabbit inference engine (.agents/skills/karpathy-guidelines/SKILL.md)
**/*: Before implementing, explicitly state assumptions, surface ambiguity and tradeoffs, present multiple interpretations when relevant, and ask for clarification rather than silently deciding or proceeding when requirements are unclear.
Prefer the minimum code needed to solve the requested problem: avoid speculative features, unnecessary abstractions, unrequested flexibility, and handling of impossible scenarios; simplify overcomplicated solutions.
When editing existing code, make surgical changes only: do not modify unrelated code, comments, formatting, or pre-existing dead code; match the existing style, and remove only unused imports, variables, or functions introduced by your changes.
Define verifiable success criteria for each task, such as writing regression tests for bugs and invalid-input tests for validation, then verify the implementation against those criteria. For multi-step work, state a brief plan with a verification check for each step.
**/*: Always spellNVIDIAin all caps; do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun, because the name begins with an “en” sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company; use trademark symbols with product names only when required by the document type or legal guidance.
Verify official capitalization, spacing, hyphenation, and spelling for NVIDIA and third-party product names; do not rewrite official product names for grammar or title-case rules.
Precede NVIDIA product names withNVIDIAon first mention when natural and accurate, and link the first mention when the destination helps the reader.
On first use, include the company name and full model qualifier when it helps identify the model; preserve official capitalization and punctuation, and use shorter family names only after establishing the full name.
For learning-oriented and developer content, do not force trademark symbols unless explicitly required; for press, ...
Files:
adapters/README.mdcrates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-core/src/config.rscrates/fabric-python/src/lib.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:
adapters/README.md
**/*.{md,rst,txt,adoc}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-language-mechanics.md)
**/*.{md,rst,txt,adoc}: For technical documentation, use professional, active, conversational, engaging, precise, and plain-English prose. Prefer active voice, present tense, short sentences, and scannable paragraphs. Avoid casual or imprecise language, swearing, threats, insults, jokes, puns, culture-specific idioms, marketing exaggeration, and unsupported third-party comparisons.
Usecanfor possibility and reservemayfor permission; useafterfor temporal order; userefer tofor cross-references; prefer short direct sentences and specific verbs; avoid unnecessarypleasein technical documentation.
Prefer active voice when the actor matters. Passive voice is acceptable when the actor is unknown or irrelevant, when the action or result is the focus, or in programmer documentation.
Use natural contractions in conversational technical prose, but do not force them in formal legal copy, API references, or generated text.
Prefer simpler English over Latinisms: usefor exampleorsuch asinstead ofe.g.,and so oninstead ofetc.,that isinstead ofi.e.,compared toinstead ofvs., andby,through, orusinginstead ofvia. Use industry-standard terms such as in silico, in vitro, and in vivo when appropriate, and italicize them in running text.
Usethatwithout commas for essential clauses, andwhichwith commas for nonessential clauses.
Format dates and times clearly: spell out months in body text; use forms such asJune 12, 2025; avoid numeric or ordinal dates; capitalize days; use 12-hour time when appropriate; include a space beforea.m.orp.m.; useETandPTfor needed time zones; avoid24/7; and preferfrom 12:30 to 1:00 p.m.for prose ranges.
Format numbers consistently: spell out zero through nine in body text, use numerals for 10 or greater and for technical values, use commas in thousands, do not begin a sentence with a numeral, spell out ordinals, and use numerals consistently within a category wh...
Files:
adapters/README.md
**/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
**/*.{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 documentation-site changes, run
just docsto regenerate Python and Rust API references and validate Fern configuration.
Files:
adapters/README.md
**/{README.md,*.md,*.mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant SDK, API, adapter, example, integration, and embedded documentation when public behavior or the corresponding surface changes.
Files:
adapters/README.md
**/*.{html,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
HTML and Markdown files must use the specified HTML comment form for SPDX license headers.
Files:
adapters/README.md
{adapters/**,examples/**}
⚙️ CodeRabbit configuration file
{adapters/**,examples/**}: Review adapter and example changes for command correctness, config/schema consistency, artifact handling, and compatibility with the public NeMo Fabric contracts.
Files:
adapters/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:
adapters/README.md
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Implement new runtime or binding behavior in the shared Rust core first.
Files:
crates/fabric-core/src/lib.rscrates/fabric-core/src/config.rscrates/fabric-python/src/lib.rs
**/*.{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-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-core/src/config.rscrates/fabric-python/src/lib.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.
Use Rust stable tooling; format Rust code with
cargo fmt --all, check formatting withcargo fmt --all -- --check, and compile withcargo check --workspace --locked.
Files:
crates/fabric-core/src/lib.rscrates/fabric-core/src/config.rscrates/fabric-python/src/lib.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.
Files:
crates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-core/src/config.rscrates/fabric-python/src/lib.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/lib.rscrates/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/lib.rscrates/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/lib.rscrates/fabric-core/src/config.rscrates/fabric-python/src/lib.rs
**/*.{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.
**/*.{rs,py,pyi}: Usesnake_casefor Rust and Python functions and variables; usePascalCasefor Rust types and Python classes.
Keep native Python binding declarations synchronized with their Rust implementations when public contracts change.
Files:
crates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-core/src/config.rscrates/fabric-python/src/lib.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:
crates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-core/src/config.rscrates/fabric-python/src/lib.rs
**/*.{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:
crates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-core/src/config.rscrates/fabric-python/src/lib.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:
crates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-core/src/config.rscrates/fabric-python/src/lib.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/lib.rscrates/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.Use type annotations for public Python APIs and keep native Python binding declarations synchronized with their Rust implementations.
Files:
tests/python/test_installed_adapter_discovery.py
tests/**/*.{rs,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the corresponding Rust crate or the relevant area under
tests/.
Files:
tests/python/test_installed_adapter_discovery.py
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Use Pytest to run Python tests.
Do not add@pytest.mark.asyncioto tests; async tests are automatically detected 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_installed_adapter_discovery.py
{tests/**,python/tests/**}
⚙️ CodeRabbit configuration file
{tests/**,python/tests/**}: Tests should cover the behavior promised by the changed API surface, including error paths, lifecycle cleanup, and SDK/native parity where relevant.
Files:
tests/python/test_installed_adapter_discovery.py
crates/fabric-python/**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If the PyO3 bridge or package metadata changes, run
just build-pythonandcargo check -p fabric-python --locked.
Files:
crates/fabric-python/src/lib.rs
crates/fabric-python/**/*
⚙️ CodeRabbit configuration file
crates/fabric-python/**/*: Treat native binding changes as public API changes. Check JSON/type conversion, error propagation, GIL/thread behavior, and parity with the Python SDK.
Files:
crates/fabric-python/src/lib.rs
🪛 ast-grep (0.44.1)
tests/python/test_installed_adapter_discovery.py
[error] 132-139: Command coming from incoming request
Context: subprocess.check_output(
[
adapter_python,
"-c",
"import sysconfig; print(sysconfig.get_path('data'), end='')",
],
text=True,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
[info] 28-36: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{
"contract_version": "fabric.adapter/v1alpha1",
"adapter_id": adapter_id,
"harness": "installed-test",
"adapter_kind": "python",
"runner": {"module": module},
}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 83-91: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{
"contract_version": "fabric.adapter/v1alpha1",
"adapter_id": "test.fabric.installed",
"harness": "installed-test",
"adapter_kind": "python",
"runner": {"module": "local.adapter"},
}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[error] 132-139: Avoid command injection
Context: subprocess.check_output(
[
adapter_python,
"-c",
"import sysconfig; print(sysconfig.get_path('data'), end='')",
],
text=True,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(command-injection-python)
🪛 Ruff (0.15.21)
tests/python/test_installed_adapter_discovery.py
[warning] 52-52: Missing return type annotation for private function _clear_adapter_python
Add return type annotation: None
(ANN202)
[warning] 63-63: Missing return type annotation for private function get_path
(ANN202)
[warning] 63-63: Missing type annotation for *args
(ANN002)
[warning] 63-63: Missing type annotation for **kwargs
(ANN003)
[warning] 96-96: Missing return type annotation for private function get_path
(ANN202)
[warning] 96-96: Missing type annotation for *args
(ANN002)
[warning] 96-96: Missing type annotation for **kwargs
(ANN003)
[warning] 120-120: Missing return type annotation for private function get_path
(ANN202)
[warning] 120-120: Missing type annotation for *args
(ANN002)
[warning] 120-120: Missing type annotation for **kwargs
(ANN003)
[error] 133-133: subprocess call: check for execution of untrusted input
(S603)
🔇 Additional comments (11)
crates/fabric-core/src/config.rs (3)
1741-1814: 🩺 Stability & AvailabilityTemp directory still leaks on assertion panic.
Same issue as previously flagged:
remove_dir_all(root)only runs at the end; a failedassert_eq!skips cleanup and leaves debris under the system temp dir.
167-179: LGTM!
1780-1811: 🗄️ Data Integrity & Integration | ⚡ Quick winAssert resolved
sourcefor both installed and local descriptors.The test verifies
path/runner.modulebut never checksadapter_descriptor.source. Per the run-plan schema, both installed-directory and agent-local descriptors must reportsource: "local"— this test is the natural place to lock that contract in.Proposed additions
assert_eq!( plan.adapter_descriptor .as_ref() .map(|adapter| adapter.path.as_path()), Some(expected_installed_descriptor.as_path()) ); + assert_eq!( + plan.adapter_descriptor.as_ref().map(|adapter| adapter.source), + Some(AdapterDescriptorSource::Local) + );As per path instructions, "run-plan.adapter_descriptor may be null, but when present it must include: source, path, root, descriptor" and "the resolved provenance maps to 'local' for both discovered-installed adapters and local overrides."
Source: Path instructions
crates/fabric-python/src/lib.rs (3)
32-40: Same resolve-then-detach sequence repeated three times.
plan_config,doctor_config, andrun_configeach repeat the identicalresolve_context+resolve_run_plan_from_config_with_adapter_directories+detachsequence; this predates the PR but is now extended withadapter_directories. Consider a sharedfn resolve_plan(py, config_json, base_dir) -> PyResult<RunPlan>helper.Also applies to: 54-62, 80-88
176-201: 🚀 Performance & Scalability | ⚡ Quick winCache the resolved adapter data path instead of re-deriving it on every call.
resolve_contextre-runs thesysconfig.get_path("data")lookup (or spawns a subprocess forADAPTER_PYTHON) on everyplan_config/doctor_config/run_configcall. The interpreter data path is invariant for the process lifetime; caching it (e.g., behind astd::sync::OnceLock) avoids repeated Python calls or process spawns on a hot path.
203-210: 🎯 Functional Correctness | ⚡ Quick winRelative multi-component
ADAPTER_PYTHONpath joined tobase_diris untested.The
elsebranch (join withbase_dir) isn't covered by any of the three new tests — they only exercise the absolute-path case. Add a regression test with a relative, multi-componentADAPTER_PYTHONvalue to lock in the join behavior.Source: Path instructions
tests/python/test_installed_adapter_discovery.py (3)
118-125: Duplicatedsysconfig.get_pathpatch, now a third instance.Same nested
get_pathwrapper is duplicated a third time; extracting a fixture (as previously suggested for the first two occurrences) would also cover this one.
70-73: Still missingsourceassertion on the resolved descriptor.As previously flagged, this test verifies only the path, not that the installed descriptor serializes with
source: "local"per the documentedRunPlancontract.
51-53: 📐 Maintainability & Code Quality | ⚡ Quick winUse
os.environ+ the autouserestore_environ_fixture, notmonkeypatch.setenv/delenv; also fix fixture naming.Two guideline violations here:
_clear_adapter_python(51-53) usesmonkeypatch.delenv, and the new test at 143 usesmonkeypatch.setenv. Repo convention is to modify env vars viaos.environdirectly and rely on the autouserestore_environ_fixturefor teardown.- The fixture itself isn't named per convention: it should be declared as
@pytest.fixture(name="clear_adapter_python")with a function namedclear_adapter_python_fixture.Proposed fix
-@pytest.fixture(autouse=True) -def _clear_adapter_python(monkeypatch): - monkeypatch.delenv("ADAPTER_PYTHON", raising=False) +@pytest.fixture(name="clear_adapter_python", autouse=True) +def clear_adapter_python_fixture(): + os.environ.pop("ADAPTER_PYTHON", None)- monkeypatch.setenv("ADAPTER_PYTHON", str(adapter_python)) + os.environ["ADAPTER_PYTHON"] = str(adapter_python)As per coding guidelines, "Use the autouse
restore_environ_fixturefromtests/conftest.pyto restore environment variables; modify variables withos.environand do not usemonkeypatch.setenv," and "Define fixtures using@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and a function named<fixture_name>_fixture."Also applies to: 143-143
Source: Coding guidelines
crates/fabric-core/src/lib.rs (1)
20-20: LGTM!adapters/README.md (1)
16-31: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/python/test_installed_adapter_discovery.py`:
- Around line 51-53: Update _clear_adapter_python and the test at line 143 to
modify ADAPTER_PYTHON through os.environ directly, replacing monkeypatch.delenv
and monkeypatch.setenv. Rely on the autouse restore_environ_fixture for cleanup
and do not add a separate monkeypatch-based restoration path.
🪄 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: 900762bf-bba4-49cd-a806-ee94b78da9c8
📒 Files selected for processing (6)
adapters/README.mdcrates/fabric-core/src/config.rscrates/fabric-core/src/lib.rscrates/fabric-python/src/lib.rstests/python/test_installed_adapter_discovery.pytests/python/test_native_sdk.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (28)
**/*.{rs,py,pyi,json,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Determine and update every affected public surface, including the CLI, PyO3 bindings, Python SDK, type stubs, schemas, and adapter contract, so they remain in parity.
Files:
tests/python/test_native_sdk.pycrates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-core/src/config.rscrates/fabric-python/src/lib.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/karpathy-guidelines/SKILL.md)
**/*: Before implementing, explicitly state assumptions, surface ambiguity and tradeoffs, present multiple interpretations when relevant, and ask for clarification rather than silently deciding or proceeding when requirements are unclear.
Prefer the minimum code needed to solve the requested problem: avoid speculative features, unnecessary abstractions, unrequested flexibility, and handling of impossible scenarios; simplify overcomplicated solutions.
When editing existing code, make surgical changes only: do not modify unrelated code, comments, formatting, or pre-existing dead code; match the existing style, and remove only unused imports, variables, or functions introduced by your changes.
Define verifiable success criteria for each task, such as writing regression tests for bugs and invalid-input tests for validation, then verify the implementation against those criteria. For multi-step work, state a brief plan with a verification check for each step.
**/*: Always spellNVIDIAin all caps; do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun, because the name begins with an “en” sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company; use trademark symbols with product names only when required by the document type or legal guidance.
Verify official capitalization, spacing, hyphenation, and spelling for NVIDIA and third-party product names; do not rewrite official product names for grammar or title-case rules.
Precede NVIDIA product names withNVIDIAon first mention when natural and accurate, and link the first mention when the destination helps the reader.
On first use, include the company name and full model qualifier when it helps identify the model; preserve official capitalization and punctuation, and use shorter family names only after establishing the full name.
For learning-oriented and developer content, do not force trademark symbols unless explicitly required; for press, ...
Files:
tests/python/test_native_sdk.pyadapters/README.mdcrates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-core/src/config.rscrates/fabric-python/src/lib.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.
Files:
tests/python/test_native_sdk.pycrates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-core/src/config.rscrates/fabric-python/src/lib.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.Use type annotations for public Python APIs and keep native Python binding declarations synchronized with their Rust implementations.
Files:
tests/python/test_native_sdk.pytests/python/test_installed_adapter_discovery.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.
**/*.{rs,py,pyi}: Usesnake_casefor Rust and Python functions and variables; usePascalCasefor Rust types and Python classes.
Keep native Python binding declarations synchronized with their Rust implementations when public contracts change.
Files:
tests/python/test_native_sdk.pycrates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-core/src/config.rscrates/fabric-python/src/lib.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_native_sdk.pycrates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-core/src/config.rscrates/fabric-python/src/lib.rs
tests/**/*.{rs,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the corresponding Rust crate or the relevant area under
tests/.
Files:
tests/python/test_native_sdk.pytests/python/test_installed_adapter_discovery.py
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Use Pytest to run Python tests.
Do not add@pytest.mark.asyncioto tests; async tests are automatically detected 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_native_sdk.pytests/python/test_installed_adapter_discovery.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_native_sdk.pycrates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-core/src/config.rscrates/fabric-python/src/lib.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_native_sdk.pycrates/fabric-core/src/lib.rstests/python/test_installed_adapter_discovery.pycrates/fabric-core/src/config.rscrates/fabric-python/src/lib.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_native_sdk.pytests/python/test_installed_adapter_discovery.py
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Update documentation and examples in the same branch as the public API change.
Files:
adapters/README.md
**/*.{md,mdx,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,mdx,rst}: For NeMo Fabric documentation, verify technical claims against the current repository, public API, or documented command before reviewing style.
Always spellNVIDIAin all caps; do not useNvidia,nvidia, orNV.
Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text; avoid raw URLs and weak anchors such ashereorread more.
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative, parallel steps; split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English while preserving necessary technical precision.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce, and preferrefer tooverseewhen directing readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical documentation.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values; use numerals for 10 or greater and commas in thousands.
Do not add trademark symbols to learning-oriented documentation unless the source, platform, or legal guidance explicitly requires them.
Do not replace precise technical terms with simpler words when doing so would lose precision.
Do not flag passive voice when the actor is unknown or the action is the important part.
Do not rewrite API names, package names, command flags, or code literals for style.
**/*.{md,mdx,rst}: Use consistent title case for technical-document headings and table headers; avoid quotation marks, ampersands, and exclamation marks in headings, while preserving official product, event, research, and whitepaper title ...
Files:
adapters/README.md
**/*.{md,rst,txt,adoc}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-language-mechanics.md)
**/*.{md,rst,txt,adoc}: For technical documentation, use professional, active, conversational, engaging, precise, and plain-English prose. Prefer active voice, present tense, short sentences, and scannable paragraphs. Avoid casual or imprecise language, swearing, threats, insults, jokes, puns, culture-specific idioms, marketing exaggeration, and unsupported third-party comparisons.
Usecanfor possibility and reservemayfor permission; useafterfor temporal order; userefer tofor cross-references; prefer short direct sentences and specific verbs; avoid unnecessarypleasein technical documentation.
Prefer active voice when the actor matters. Passive voice is acceptable when the actor is unknown or irrelevant, when the action or result is the focus, or in programmer documentation.
Use natural contractions in conversational technical prose, but do not force them in formal legal copy, API references, or generated text.
Prefer simpler English over Latinisms: usefor exampleorsuch asinstead ofe.g.,and so oninstead ofetc.,that isinstead ofi.e.,compared toinstead ofvs., andby,through, orusinginstead ofvia. Use industry-standard terms such as in silico, in vitro, and in vivo when appropriate, and italicize them in running text.
Usethatwithout commas for essential clauses, andwhichwith commas for nonessential clauses.
Format dates and times clearly: spell out months in body text; use forms such asJune 12, 2025; avoid numeric or ordinal dates; capitalize days; use 12-hour time when appropriate; include a space beforea.m.orp.m.; useETandPTfor needed time zones; avoid24/7; and preferfrom 12:30 to 1:00 p.m.for prose ranges.
Format numbers consistently: spell out zero through nine in body text, use numerals for 10 or greater and for technical values, use commas in thousands, do not begin a sentence with a numeral, spell out ordinals, and use numerals consistently within a category wh...
Files:
adapters/README.md
**/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
**/*.{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 documentation-site changes, run
just docsto regenerate Python and Rust API references and validate Fern configuration.
Files:
adapters/README.md
**/{README.md,*.md,*.mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant SDK, API, adapter, example, integration, and embedded documentation when public behavior or the corresponding surface changes.
Files:
adapters/README.md
**/*.{html,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
HTML and Markdown files must use the specified HTML comment form for SPDX license headers.
Files:
adapters/README.md
{adapters/**,examples/**}
⚙️ CodeRabbit configuration file
{adapters/**,examples/**}: Review adapter and example changes for command correctness, config/schema consistency, artifact handling, and compatibility with the public NeMo Fabric contracts.
Files:
adapters/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:
adapters/README.md
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Implement new runtime or binding behavior in the shared Rust core first.
Files:
crates/fabric-core/src/lib.rscrates/fabric-core/src/config.rscrates/fabric-python/src/lib.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.
Use Rust stable tooling; format Rust code with
cargo fmt --all, check formatting withcargo fmt --all -- --check, and compile withcargo check --workspace --locked.
Files:
crates/fabric-core/src/lib.rscrates/fabric-core/src/config.rscrates/fabric-python/src/lib.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/lib.rscrates/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/lib.rscrates/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/lib.rscrates/fabric-core/src/config.rscrates/fabric-python/src/lib.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/lib.rscrates/fabric-core/src/config.rs
crates/fabric-python/**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If the PyO3 bridge or package metadata changes, run
just build-pythonandcargo check -p fabric-python --locked.
Files:
crates/fabric-python/src/lib.rs
crates/fabric-python/**/*
⚙️ CodeRabbit configuration file
crates/fabric-python/**/*: Treat native binding changes as public API changes. Check JSON/type conversion, error propagation, GIL/thread behavior, and parity with the Python SDK.
Files:
crates/fabric-python/src/lib.rs
🪛 ast-grep (0.44.1)
tests/python/test_installed_adapter_discovery.py
[info] 28-36: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{
"contract_version": "fabric.adapter/v1alpha1",
"adapter_id": adapter_id,
"harness": "installed-test",
"adapter_kind": "python",
"runner": {"module": module},
}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 83-91: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{
"contract_version": "fabric.adapter/v1alpha1",
"adapter_id": "test.fabric.installed",
"harness": "installed-test",
"adapter_kind": "python",
"runner": {"module": "local.adapter"},
}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[error] 132-139: Avoid command injection
Context: subprocess.check_output(
[
adapter_python,
"-c",
"import sysconfig; print(sysconfig.get_path('data'), end='')",
],
text=True,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(command-injection-python)
[error] 132-139: Command coming from incoming request
Context: subprocess.check_output(
[
adapter_python,
"-c",
"import sysconfig; print(sysconfig.get_path('data'), end='')",
],
text=True,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🪛 Ruff (0.15.21)
tests/python/test_installed_adapter_discovery.py
[warning] 52-52: Missing return type annotation for private function _clear_adapter_python
Add return type annotation: None
(ANN202)
[warning] 63-63: Missing return type annotation for private function get_path
(ANN202)
[warning] 63-63: Missing type annotation for *args
(ANN002)
[warning] 63-63: Missing type annotation for **kwargs
(ANN003)
[warning] 96-96: Missing return type annotation for private function get_path
(ANN202)
[warning] 96-96: Missing type annotation for *args
(ANN002)
[warning] 96-96: Missing type annotation for **kwargs
(ANN003)
[warning] 120-120: Missing return type annotation for private function get_path
(ANN202)
[warning] 120-120: Missing type annotation for *args
(ANN002)
[warning] 120-120: Missing type annotation for **kwargs
(ANN003)
[error] 133-133: subprocess call: check for execution of untrusted input
(S603)
🔇 Additional comments (11)
crates/fabric-core/src/config.rs (2)
1741-1814: Temp directory leak if an assertion fails mid-test (unresolved from prior review).
std::fs::remove_dir_all(root)still only runs at the very end (line 1813); any panic from the earlierassert_eq!calls skips cleanup and leaves the directory under the system temp dir.
133-137: LGTM!Also applies to: 167-179, 996-1023, 1039-1044, 1067-1087
crates/fabric-python/src/lib.rs (4)
32-40: Same resolve-then-detach pattern repeated three times (unresolved from prior review).
plan_config,doctor_config(54-62), andrun_config(80-88) still repeat the identicalresolve_context+resolve_run_plan_from_config_with_adapter_directories+detachsequence.Also applies to: 54-62, 80-88
212-243: UnboundedADAPTER_PYTHONprobe (unresolved from prior review).
query_python_data_pathstill usesCommand::output()with no deadline, so a hung or slow interpreter stalls everyplan_config/doctor_config/run_configcall wheneverADAPTER_PYTHONis set.
6-20: LGTM!
203-210: 🎯 Functional CorrectnessConfirm intended base_dir-relative resolution for multi-component
ADAPTER_PYTHONpaths.
resolve_adapter_pythonjoins relative, multi-componentADAPTER_PYTHONvalues againstbase_dirrather than the process CWD (e.g.ADAPTER_PYTHON=venv/bin/pythonresolves relative to the agent directory, not the shell's working directory). This isn't documented inadapters/README.md's Descriptor Discovery section and isn't covered by the new tests, which only exercise absolute paths.adapters/README.md (1)
16-31: Missing SPDX license header (unresolved from prior review).This README still appears to lack the repository-standard SPDX copyright/Apache-2.0 header as HTML comments at the top of the file.
tests/python/test_installed_adapter_discovery.py (2)
56-73: Duplicatedsysconfig.get_pathpatch; prefer a fixture over ad-hoc helpers (unresolved from prior review).Each test still defines an identical nested
get_pathwrapper to monkeypatchsysconfig.get_path, and uses free helper functions instead of fixtures.Also applies to: 75-107, 109-157
70-73: Assert installed-descriptor provenance (unresolved from prior review).The documentation promises installed descriptors serialize as
source: local, but this test still verifies only the path.crates/fabric-core/src/lib.rs (1)
20-20: LGTM!tests/python/test_native_sdk.py (1)
20-20: LGTM!Also applies to: 43-54
6572033 to
a70637b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/python/test_installed_adapter_discovery.py`:
- Around line 92-95: Remove the -> None return annotations from
test_plan_discovers_adapter_from_python_data_directory and the three other test
functions at the referenced locations, while leaving their parameters and test
logic unchanged.
- Around line 53-76: Rename the fixture functions `_clear_adapter_python` and
`patch_sysconfig_data` to `_clear_adapter_python_fixture` and
`patch_sysconfig_data_fixture`, and declare each with
`@pytest.fixture(name="...")` using its existing public fixture name. Preserve
the current autouse behavior and patching logic.
🪄 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: 65866249-4080-4321-b9dd-a0d2fd8ef47d
📒 Files selected for processing (6)
adapters/README.mdcrates/fabric-core/src/config.rscrates/fabric-core/src/lib.rscrates/fabric-python/src/lib.rstests/python/test_installed_adapter_discovery.pytests/python/test_native_sdk.py
📜 Review details
⏰ Context from checks skipped due to timeout. (10)
- GitHub Check: Preview docs
- GitHub Check: Test (Python 3.14, windows-amd64)
- GitHub Check: Test (Python 3.13, windows-amd64)
- GitHub Check: Test (Python 3.11, windows-amd64)
- GitHub Check: Test (Python 3.11, macos-arm64)
- GitHub Check: Test (Python 3.12, macos-arm64)
- GitHub Check: Test (Python 3.14, macos-arm64)
- GitHub Check: Test (Python 3.12, linux-amd64)
- GitHub Check: Test (Python 3.12, windows-amd64)
- GitHub Check: Pre-commit
🧰 Additional context used
📓 Path-based instructions (28)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Implement new runtime or binding behavior in the shared Rust core first.
Files:
crates/fabric-core/src/lib.rscrates/fabric-core/src/config.rscrates/fabric-python/src/lib.rs
**/*.{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-core/src/lib.rstests/python/test_native_sdk.pycrates/fabric-core/src/config.rstests/python/test_installed_adapter_discovery.pycrates/fabric-python/src/lib.rs
**/*
📄 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:
crates/fabric-core/src/lib.rsadapters/README.mdtests/python/test_native_sdk.pycrates/fabric-core/src/config.rstests/python/test_installed_adapter_discovery.pycrates/fabric-python/src/lib.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.
Use Rust stable tooling; format Rust code with
cargo fmt --all, check formatting withcargo fmt --all -- --check, and compile withcargo check --workspace --locked.
Files:
crates/fabric-core/src/lib.rscrates/fabric-core/src/config.rscrates/fabric-python/src/lib.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
For native binding changes, run
cargo check -p fabric-python --locked.
Files:
crates/fabric-core/src/lib.rstests/python/test_native_sdk.pycrates/fabric-core/src/config.rstests/python/test_installed_adapter_discovery.pycrates/fabric-python/src/lib.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/lib.rscrates/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/lib.rscrates/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/lib.rscrates/fabric-core/src/config.rscrates/fabric-python/src/lib.rs
**/*.{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.
**/*.{rs,py,pyi}: Usesnake_casefor Rust and Python functions and variables; usePascalCasefor Rust types and Python classes.
Keep native Python binding declarations synchronized with their Rust implementations when public contracts change.
Files:
crates/fabric-core/src/lib.rstests/python/test_native_sdk.pycrates/fabric-core/src/config.rstests/python/test_installed_adapter_discovery.pycrates/fabric-python/src/lib.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:
crates/fabric-core/src/lib.rstests/python/test_native_sdk.pycrates/fabric-core/src/config.rstests/python/test_installed_adapter_discovery.pycrates/fabric-python/src/lib.rs
**/*.{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:
crates/fabric-core/src/lib.rstests/python/test_native_sdk.pycrates/fabric-core/src/config.rstests/python/test_installed_adapter_discovery.pycrates/fabric-python/src/lib.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:
crates/fabric-core/src/lib.rstests/python/test_native_sdk.pycrates/fabric-core/src/config.rstests/python/test_installed_adapter_discovery.pycrates/fabric-python/src/lib.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/lib.rscrates/fabric-core/src/config.rs
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)
Update documentation and examples in the same branch as the public API change.
Files:
adapters/README.md
**/*.{md,mdx,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,mdx,rst}: For NeMo Fabric documentation, verify technical claims against the current repository, public API, or documented command before reviewing style.
Always spellNVIDIAin all caps; do not useNvidia,nvidia, orNV.
Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text; avoid raw URLs and weak anchors such ashereorread more.
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative, parallel steps; split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English while preserving necessary technical precision.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce, and preferrefer tooverseewhen directing readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical documentation.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values; use numerals for 10 or greater and commas in thousands.
Do not add trademark symbols to learning-oriented documentation unless the source, platform, or legal guidance explicitly requires them.
Do not replace precise technical terms with simpler words when doing so would lose precision.
Do not flag passive voice when the actor is unknown or the action is the important part.
Do not rewrite API names, package names, command flags, or code literals for style.
**/*.{md,mdx,rst}: Use consistent title case for technical-document headings and table headers; avoid quotation marks, ampersands, and exclamation marks in headings, while preserving official product, event, research, and whitepaper title ...
Files:
adapters/README.md
**/*.{md,rst,txt,adoc}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-language-mechanics.md)
**/*.{md,rst,txt,adoc}: For technical documentation, use professional, active, conversational, engaging, precise, and plain-English prose. Prefer active voice, present tense, short sentences, and scannable paragraphs. Avoid casual or imprecise language, swearing, threats, insults, jokes, puns, culture-specific idioms, marketing exaggeration, and unsupported third-party comparisons.
Usecanfor possibility and reservemayfor permission; useafterfor temporal order; userefer tofor cross-references; prefer short direct sentences and specific verbs; avoid unnecessarypleasein technical documentation.
Prefer active voice when the actor matters. Passive voice is acceptable when the actor is unknown or irrelevant, when the action or result is the focus, or in programmer documentation.
Use natural contractions in conversational technical prose, but do not force them in formal legal copy, API references, or generated text.
Prefer simpler English over Latinisms: usefor exampleorsuch asinstead ofe.g.,and so oninstead ofetc.,that isinstead ofi.e.,compared toinstead ofvs., andby,through, orusinginstead ofvia. Use industry-standard terms such as in silico, in vitro, and in vivo when appropriate, and italicize them in running text.
Usethatwithout commas for essential clauses, andwhichwith commas for nonessential clauses.
Format dates and times clearly: spell out months in body text; use forms such asJune 12, 2025; avoid numeric or ordinal dates; capitalize days; use 12-hour time when appropriate; include a space beforea.m.orp.m.; useETandPTfor needed time zones; avoid24/7; and preferfrom 12:30 to 1:00 p.m.for prose ranges.
Format numbers consistently: spell out zero through nine in body text, use numerals for 10 or greater and for technical values, use commas in thousands, do not begin a sentence with a numeral, spell out ordinals, and use numerals consistently within a category wh...
Files:
adapters/README.md
**/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
**/*.{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 documentation-site changes, run
just docsto regenerate Python and Rust API references and validate Fern configuration.
Files:
adapters/README.md
**/{README.md,*.md,*.mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update relevant SDK, API, adapter, example, integration, and embedded documentation when public behavior or the corresponding surface changes.
Files:
adapters/README.md
**/*.{html,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
HTML and Markdown files must use the specified HTML comment form for SPDX license headers.
Files:
adapters/README.md
{adapters/**,examples/**}
⚙️ CodeRabbit configuration file
{adapters/**,examples/**}: Review adapter and example changes for command correctness, config/schema consistency, artifact handling, and compatibility with the public NeMo Fabric contracts.
Files:
adapters/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:
adapters/README.md
**/*.{py,pyi}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Python code or a Python-facing adapter changes, run
just test-python.Use type annotations for public Python APIs and keep native Python binding declarations synchronized with their Rust implementations.
Files:
tests/python/test_native_sdk.pytests/python/test_installed_adapter_discovery.py
tests/**/*.{rs,py}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
When adding functionality, include tests in the corresponding Rust crate or the relevant area under
tests/.
Files:
tests/python/test_native_sdk.pytests/python/test_installed_adapter_discovery.py
tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)
tests/**/*.py: Use Pytest to run Python tests.
Do not add@pytest.mark.asyncioto tests; async tests are automatically detected 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_native_sdk.pytests/python/test_installed_adapter_discovery.py
{tests/**,python/tests/**}
⚙️ CodeRabbit configuration file
{tests/**,python/tests/**}: Tests should cover the behavior promised by the changed API surface, including error paths, lifecycle cleanup, and SDK/native parity where relevant.
Files:
tests/python/test_native_sdk.pytests/python/test_installed_adapter_discovery.py
crates/fabric-python/**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If the PyO3 bridge or package metadata changes, run
just build-pythonandcargo check -p fabric-python --locked.
Files:
crates/fabric-python/src/lib.rs
crates/fabric-python/**/*
⚙️ CodeRabbit configuration file
crates/fabric-python/**/*: Treat native binding changes as public API changes. Check JSON/type conversion, error propagation, GIL/thread behavior, and parity with the Python SDK.
Files:
crates/fabric-python/src/lib.rs
🪛 ast-grep (0.44.1)
tests/python/test_installed_adapter_discovery.py
[error] 80-87: Avoid command injection
Context: subprocess.check_output(
[
python,
"-c",
f"import sysconfig; print(sysconfig.get_path({name!r}), end='')",
],
text=True,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(command-injection-python)
[info] 30-38: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{
"contract_version": "fabric.adapter/v1alpha1",
"adapter_id": adapter_id,
"harness": "installed-test",
"adapter_kind": "python",
"runner": {"module": module},
}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[info] 115-123: use jsonify instead of json.dumps for JSON output
Context: json.dumps(
{
"contract_version": "fabric.adapter/v1alpha1",
"adapter_id": "test.fabric.installed",
"harness": "installed-test",
"adapter_kind": "python",
"runner": {"module": "local.adapter"},
}
)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
[error] 80-87: Command coming from incoming request
Context: subprocess.check_output(
[
python,
"-c",
f"import sysconfig; print(sysconfig.get_path({name!r}), end='')",
],
text=True,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🪛 Ruff (0.15.21)
tests/python/test_installed_adapter_discovery.py
[error] 81-81: subprocess call: check for execution of untrusted input
(S603)
🔇 Additional comments (7)
crates/fabric-core/src/config.rs (2)
167-176: LGTM!Also applies to: 999-1022, 1039-1044, 1070-1073
1741-1821: LGTM!crates/fabric-core/src/lib.rs (1)
20-20: LGTM!crates/fabric-python/src/lib.rs (2)
180-281:resolve_context,resolve_adapter_python, and the boundedquery_python_data_pathprobe look correct: relative multi-componentADAPTER_PYTHONresolves againstbase_dir, bare names defer toPATH, and the poll loop kills/reaps the child on timeout or error. LGTM!
36-92: LGTM!adapters/README.md (1)
16-34: LGTM!tests/python/test_native_sdk.py (1)
20-20: LGTM!Also applies to: 43-53
Signed-off-by: Anuradha Karuppiah <26330987+AnuradhaKaruppiah@users.noreply.github.com>
a70637b to
12d7d1a
Compare
zhongxuanwang-nv
left a comment
zhongxuanwang-nv
left a comment
There was a problem hiding this comment.
Thanks Anuradha! Just one potential nitty thing I found
Signed-off-by: Anuradha Karuppiah <26330987+AnuradhaKaruppiah@users.noreply.github.com>
|
/merge |
2 participants
Overview
Adapter wheels already install
fabric-adapter.jsonunder their Python datadirectory, but Fabric only searched repository and agent-local descriptors. A
package-installed adapter could therefore be importable while Fabric reported
it as unknown.
This PR adds stopgap installed-metadata discovery without adding a
FabricConfigfield or defining the final adapter-registry contract. WhenADAPTER_PYTHONis set, its interpreter is the sole installed-metadata source;otherwise Fabric uses the current Python interpreter. Agent-local
<base_dir>/adaptersdescriptors remain the highest-precedence override.There are no breaking configuration changes or new dependencies.
Details
<sysconfig data>/share/nemo-fabric/adaptersfromADAPTER_PYTHONwhen it is set and nonempty, otherwise from the current Fabric Python
interpreter.
interpreter selected to execute adapter code.
ADAPTER_PYTHONinstead of silently fallingback to the current environment.
ADAPTER_PYTHONmetadata probe to five seconds and terminate thechild process on timeout.
metadata, then agent-local
<base_dir>/adaptersoverrides.registry, including relative interpreter path resolution.
split Fabric/adapter Python environments, and probe timeouts.
Validation
cargo fmt --all -- --checkcargo check -p fabric-python --lockedjust test-rust— 39 tests passedjust no_uv=true build-pythonnemo-fabric[codex]==0.1.0a1smoke resolved the Codex descriptorfrom the
ADAPTER_PYTHONenvironment.The full local Python suite was attempted but stalled in the existing
tests/adapters/test_adapters_common_lifecycle.pysection in this environment;GitHub CI provides the complete suite run.
Where should the reviewer start?
Start in
crates/fabric-python/src/lib.rsatresolve_contextandquery_python_data_path, then reviewtest_adapter_python_data_directory_replaces_current_data_directoryintests/python/test_installed_adapter_discovery.py. The main design decision isthat
ADAPTER_PYTHON, when present, replaces rather than augmentscurrent-interpreter installed metadata. The probe is bounded so a hung
interpreter cannot block planning indefinitely.
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Relates to: none
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.