Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions .github/workflows/ci_python.yml
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,38 @@ jobs:
- name: Build wheels
run: just wheels

- name: Test installed adapter discovery
run: |
set -euo pipefail
uv venv .wheel-test-venv
if [[ -x .wheel-test-venv/bin/python ]]; then
test_python="$PWD/.wheel-test-venv/bin/python"
else
test_python="$PWD/.wheel-test-venv/Scripts/python.exe"
fi
uv pip install \
--python "$test_python" \
--find-links dist \
"nemo-fabric[hermes]"

source_adapters="$GITHUB_WORKSPACE/adapters"
staged_source_adapters="$RUNNER_TEMP/nemo-fabric-source-adapters"
mv "$source_adapters" "$staged_source_adapters"
trap 'mv "$staged_source_adapters" "$source_adapters"' EXIT

smoke_dir="$(mktemp -d)"
cd "$smoke_dir"
"$test_python" - <<'PY'
from nemo_fabric import Fabric, FabricConfig, HarnessConfig, MetadataConfig

config = FabricConfig(
metadata=MetadataConfig(name="installed-adapter-smoke"),
harness=HarnessConfig(adapter_id="nvidia.fabric.hermes"),
)
plan = Fabric().plan(config)
assert plan.adapter.adapter_id == "nvidia.fabric.hermes"
PY

Comment on lines +156 to +187

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Run the smoke script with Bash.

The matrix includes windows-2022, where run defaults to PowerShell. Lines 158-186 use Bash syntax, so that wheel job fails before testing. Set shell: bash on this step.

Proposed fix
       - name: Test installed adapter discovery
+        shell: bash
         run: |
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Test installed adapter discovery
run: |
set -euo pipefail
uv venv .wheel-test-venv
if [[ -x .wheel-test-venv/bin/python ]]; then
test_python="$PWD/.wheel-test-venv/bin/python"
else
test_python="$PWD/.wheel-test-venv/Scripts/python.exe"
fi
uv pip install \
--python "$test_python" \
--find-links dist \
"nemo-fabric[hermes]"
source_adapters="$GITHUB_WORKSPACE/adapters"
staged_source_adapters="$RUNNER_TEMP/nemo-fabric-source-adapters"
mv "$source_adapters" "$staged_source_adapters"
trap 'mv "$staged_source_adapters" "$source_adapters"' EXIT
smoke_dir="$(mktemp -d)"
cd "$smoke_dir"
"$test_python" - <<'PY'
from nemo_fabric import Fabric, FabricConfig, HarnessConfig, MetadataConfig
config = FabricConfig(
metadata=MetadataConfig(name="installed-adapter-smoke"),
harness=HarnessConfig(adapter_id="nvidia.fabric.hermes"),
)
plan = Fabric().plan(config)
assert plan.adapter.adapter_id == "nvidia.fabric.hermes"
PY
- name: Test installed adapter discovery
shell: bash
run: |
set -euo pipefail
uv venv .wheel-test-venv
if [[ -x .wheel-test-venv/bin/python ]]; then
test_python="$PWD/.wheel-test-venv/bin/python"
else
test_python="$PWD/.wheel-test-venv/Scripts/python.exe"
fi
uv pip install \
--python "$test_python" \
--find-links dist \
"nemo-fabric[hermes]"
source_adapters="$GITHUB_WORKSPACE/adapters"
staged_source_adapters="$RUNNER_TEMP/nemo-fabric-source-adapters"
mv "$source_adapters" "$staged_source_adapters"
trap 'mv "$staged_source_adapters" "$source_adapters"' EXIT
smoke_dir="$(mktemp -d)"
cd "$smoke_dir"
"$test_python" - <<'PY'
from nemo_fabric import Fabric, FabricConfig, HarnessConfig, MetadataConfig
config = FabricConfig(
metadata=MetadataConfig(name="installed-adapter-smoke"),
harness=HarnessConfig(adapter_id="nvidia.fabric.hermes"),
)
plan = Fabric().plan(config)
assert plan.adapter.adapter_id == "nvidia.fabric.hermes"
PY
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci_python.yml around lines 156 - 187, Set shell: bash on
the “Test installed adapter discovery” workflow step so its Bash-specific
commands run correctly on the windows-2022 matrix job, while preserving the
existing smoke test commands.

- name: Upload wheels
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
with:
Expand Down
64 changes: 57 additions & 7 deletions crates/fabric-core/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,10 +164,17 @@ struct AdapterRegistry {
}

impl AdapterRegistry {
fn from_config(_config: &FabricConfig, base_dir: &Path) -> Result<Self> {
fn from_config(
_config: &FabricConfig,
base_dir: &Path,
adapter_descriptors: &[PathBuf],
) -> Result<Self> {
let mut registry = Self::default();
registry.register_repository_directory(&repository_adapter_dir())?;
registry.register_local_directory(&base_dir.join("adapters"))?;
for path in adapter_descriptors {
registry.register_descriptor_file(path, AdapterDescriptorSource::Local)?;
}
Ok(registry)
}

Expand Down Expand Up @@ -204,12 +211,20 @@ impl AdapterRegistry {
if !is_adapter_descriptor_file(&path) {
continue;
}
let descriptor = load_adapter_descriptor(&path)?;
self.register_descriptor(path, source, descriptor)?;
self.register_descriptor_file(&path, source)?;
}
Ok(())
}

fn register_descriptor_file(
&mut self,
path: &Path,
source: AdapterDescriptorSource,
) -> Result<()> {
let descriptor = load_adapter_descriptor(path)?;
self.register_descriptor(path.to_path_buf(), source, descriptor)
}

fn register_descriptor(
&mut self,
path: PathBuf,
Expand Down Expand Up @@ -989,6 +1004,16 @@ fn validate_config(config: &FabricConfig) -> Result<()> {
pub fn resolve_run_plan_from_config(
config: FabricConfig,
context: ResolveContext,
) -> Result<RunPlan> {
resolve_run_plan_from_config_with_adapter_descriptors(config, context, &[])
}

/// Resolve a typed Fabric config with caller-registered adapter descriptors.
#[doc(hidden)]
pub fn resolve_run_plan_from_config_with_adapter_descriptors(
config: FabricConfig,
context: ResolveContext,
adapter_descriptors: &[PathBuf],
) -> Result<RunPlan> {
validate_config(&config)?;
let supplied_base_dir = context.base_dir;
Expand All @@ -998,7 +1023,7 @@ pub fn resolve_run_plan_from_config(
path: supplied_base_dir,
source,
})?;
resolve_run_plan(config, base_dir)
resolve_run_plan(config, base_dir, adapter_descriptors)
}

fn read_json<T>(path: &Path) -> Result<T>
Expand All @@ -1015,8 +1040,12 @@ where
})
}

fn resolve_run_plan(config: FabricConfig, base_dir: PathBuf) -> Result<RunPlan> {
let adapter_descriptor = resolve_adapter_descriptor(&config, &base_dir)?;
fn resolve_run_plan(
config: FabricConfig,
base_dir: PathBuf,
adapter_descriptors: &[PathBuf],
) -> Result<RunPlan> {
let adapter_descriptor = resolve_adapter_descriptor(&config, &base_dir, adapter_descriptors)?;
let descriptor = adapter_descriptor
.as_ref()
.map(|adapter| &adapter.descriptor);
Expand All @@ -1042,9 +1071,10 @@ fn resolve_run_plan(config: FabricConfig, base_dir: PathBuf) -> Result<RunPlan>
fn resolve_adapter_descriptor(
config: &FabricConfig,
base_dir: &Path,
adapter_descriptors: &[PathBuf],
) -> Result<Option<ResolvedAdapterDescriptor>> {
let adapter_id = &config.harness.adapter_id;
let registry = AdapterRegistry::from_config(config, base_dir)?;
let registry = AdapterRegistry::from_config(config, base_dir, adapter_descriptors)?;
let Some(entry) = registry.get(adapter_id) else {
return Err(FabricError::UnknownAdapter {
adapter_id: adapter_id.clone(),
Expand Down Expand Up @@ -1712,4 +1742,24 @@ mod tests {
assert_eq!(descriptor.contract_version, ADAPTER_CONTRACT_VERSION);
assert_eq!(descriptor.adapter_kind, AdapterKind::Python);
}

#[test]
fn resolves_caller_registered_adapter_descriptor() {
let descriptor_path = repository_root()
.join("tests/fixtures/hermes-shim-agent/adapters/hermes-shim/fabric-adapter.json");
let plan = resolve_run_plan_from_config_with_adapter_descriptors(
typed_config("test.fabric.hermes_shim"),
ResolveContext::new("/tmp/fabric-base"),
std::slice::from_ref(&descriptor_path),
)
.expect("registered adapter descriptor");
let resolved = plan.adapter_descriptor.expect("adapter descriptor");

assert_eq!(resolved.descriptor.adapter_id, "test.fabric.hermes_shim");
assert_eq!(resolved.source, AdapterDescriptorSource::Local);
assert_eq!(
resolved.path,
descriptor_path.canonicalize().expect("descriptor path")
);
}
}
42 changes: 33 additions & 9 deletions crates/fabric-python/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@

use std::path::PathBuf;

use nemo_fabric_core::config::resolve_run_plan_from_config_with_adapter_descriptors;
use nemo_fabric_core::{
FabricConfig, ResolveContext, RunPlan, RunRequest, RuntimeHandle, doctor_plan,
resolve_run_plan_from_config, run_plan,
FabricConfig, ResolveContext, RunPlan, RunRequest, RuntimeHandle, doctor_plan, run_plan,
};
use pyo3::exceptions::PyRuntimeError;
use pyo3::prelude::*;
Expand All @@ -20,33 +20,39 @@ fn version() -> PyResult<String> {

/// Resolve typed config JSON into a runnable plan and return JSON.
#[pyfunction]
#[pyo3(signature = (config_json, base_dir=None))]
fn plan_config(py: Python<'_>, config_json: String, base_dir: Option<String>) -> PyResult<String> {
#[pyo3(signature = (config_json, base_dir=None, adapter_descriptors=None))]
fn plan_config(
py: Python<'_>,
config_json: String,
base_dir: Option<String>,
adapter_descriptors: Option<Vec<String>>,
) -> PyResult<String> {
let config = parse_config(config_json)?;
let plan = py
.detach(|| resolve_run_plan_from_config(config, resolve_context(base_dir)))
.detach(|| resolve_config(config, base_dir, adapter_descriptors))
.map_err(to_py_error)?;
to_json(&plan)
}

/// Diagnose typed config JSON without installing or running it.
#[pyfunction]
#[pyo3(signature = (config_json, base_dir=None))]
#[pyo3(signature = (config_json, base_dir=None, adapter_descriptors=None))]
fn doctor_config(
py: Python<'_>,
config_json: String,
base_dir: Option<String>,
adapter_descriptors: Option<Vec<String>>,
) -> PyResult<String> {
let config = parse_config(config_json)?;
let plan = py
.detach(|| resolve_run_plan_from_config(config, resolve_context(base_dir)))
.detach(|| resolve_config(config, base_dir, adapter_descriptors))
.map_err(to_py_error)?;
to_json(&doctor_plan(&plan))
}

/// Run typed config JSON through its Fabric adapter and return JSON.
#[pyfunction]
#[pyo3(signature = (config_json, base_dir=None, input_text=None, input_file=None, request_json=None, request_file=None))]
#[pyo3(signature = (config_json, base_dir=None, input_text=None, input_file=None, request_json=None, request_file=None, adapter_descriptors=None))]
fn run_config(
py: Python<'_>,
config_json: String,
Expand All @@ -55,10 +61,11 @@ fn run_config(
input_file: Option<String>,
request_json: Option<String>,
request_file: Option<String>,
adapter_descriptors: Option<Vec<String>>,
) -> PyResult<String> {
let config = parse_config(config_json)?;
let plan = py
.detach(|| resolve_run_plan_from_config(config, resolve_context(base_dir)))
.detach(|| resolve_config(config, base_dir, adapter_descriptors))
.map_err(to_py_error)?;
let request = match (request_file, request_json, input_file, input_text) {
(Some(path), None, None, None) => std::fs::read_to_string(PathBuf::from(&path))
Expand Down Expand Up @@ -150,6 +157,23 @@ fn resolve_context(base_dir: Option<String>) -> ResolveContext {
ResolveContext::new(base_dir.unwrap_or_else(|| ".".to_string()))
}

fn resolve_config(
config: FabricConfig,
base_dir: Option<String>,
adapter_descriptors: Option<Vec<String>>,
) -> nemo_fabric_core::Result<RunPlan> {
let adapter_descriptors = adapter_descriptors
.unwrap_or_default()
.into_iter()
.map(PathBuf::from)
.collect::<Vec<_>>();
resolve_run_plan_from_config_with_adapter_descriptors(
config,
resolve_context(base_dir),
&adapter_descriptors,
)
}

fn parse_config(contents: String) -> PyResult<FabricConfig> {
serde_json::from_str(&contents).map_err(|error| PyRuntimeError::new_err(error.to_string()))
}
Expand Down
3 changes: 3 additions & 0 deletions python/src/nemo_fabric/_native.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ def version() -> str: ...
def plan_config(
config_json: str,
base_dir: str | None = None,
adapter_descriptors: list[str] | None = None,
) -> str: ...
def doctor_config(
config_json: str,
base_dir: str | None = None,
adapter_descriptors: list[str] | None = None,
) -> str: ...
def run_config(
config_json: str,
Expand All @@ -17,6 +19,7 @@ def run_config(
input_file: str | None = None,
request_json: str | None = None,
request_file: str | None = None,
adapter_descriptors: list[str] | None = None,
) -> str: ...
def start_runtime(plan_json: str) -> str: ...
def invoke_runtime(
Expand Down
16 changes: 16 additions & 0 deletions python/src/nemo_fabric/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@

import asyncio
import importlib
import importlib.metadata
import json
import os
from collections.abc import Mapping
from pathlib import Path
from typing import Any
from nemo_fabric.errors import (
FabricConfigError,
Expand Down Expand Up @@ -86,6 +88,7 @@ def plan(
raw = native.plan_config(
_config_json(config),
_base_dir_arg(base_dir),
_installed_adapter_descriptor_paths(),
)
return RunPlan.from_mapping(json.loads(raw))
except FabricError:
Expand Down Expand Up @@ -124,6 +127,7 @@ def diagnose() -> DoctorReport:
raw = native.doctor_config(
_config_json(config),
_base_dir_arg(base_dir),
_installed_adapter_descriptor_paths(),
)
return DoctorReport.from_mapping(json.loads(raw))

Expand Down Expand Up @@ -271,3 +275,15 @@ def _config_json(config: FabricConfig) -> str:

def _base_dir_arg(base_dir: str | os.PathLike[str] | None) -> str | None:
return None if base_dir is None else os.fspath(base_dir)


def _installed_adapter_descriptor_paths() -> list[str]:
paths: set[str] = set()
for distribution in importlib.metadata.distributions():
for file in distribution.files or ():
if file.name != "fabric-adapter.json":
continue
path = Path(distribution.locate_file(file))
if path.is_file():
paths.add(os.fspath(path.resolve()))
return sorted(paths)
4 changes: 3 additions & 1 deletion tests/python/test_runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,9 @@ async def _wait_for(event: threading.Event, timeout: float = 2.0) -> bool:
def mock_native_fixture() -> MagicMock:
mock_native = MagicMock()
mock_native.requests = []
mock_native.plan_config.side_effect = lambda config_json, base_dir: json.dumps(_plan())
mock_native.plan_config.side_effect = (
lambda config_json, base_dir, adapter_descriptors: json.dumps(_plan())
)
Comment on lines +98 to +100

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Avoid the new Ruff violations.

Line 99 reports ARG005 for all callback parameters. Prefix them with _ while retaining the fixed three-argument mock signature.

Proposed fix
-        lambda config_json, base_dir, adapter_descriptors: json.dumps(_plan())
+        lambda _config_json, _base_dir, _adapter_descriptors: json.dumps(_plan())
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
mock_native.plan_config.side_effect = (
lambda config_json, base_dir, adapter_descriptors: json.dumps(_plan())
)
mock_native.plan_config.side_effect = (
lambda _config_json, _base_dir, _adapter_descriptors: json.dumps(_plan())
)
🧰 Tools
🪛 ast-grep (0.44.1)

[info] 98-98: use jsonify instead of json.dumps for JSON output
Context: json.dumps(_plan())
Note: [CWE-116] Improper Encoding or Escaping of Output.

(use-jsonify)


[info] 100-100: use jsonify instead of json.dumps for JSON output
Context: json.dumps(_runtime())
Note: [CWE-116] Improper Encoding or Escaping of Output.

(use-jsonify)

🪛 Ruff (0.15.21)

[warning] 99-99: Unused lambda argument: config_json

(ARG005)


[warning] 99-99: Unused lambda argument: base_dir

(ARG005)


[warning] 99-99: Unused lambda argument: adapter_descriptors

(ARG005)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/python/test_runtime.py` around lines 98 - 100, Update the lambda
assigned to mock_native.plan_config.side_effect so all three callback parameters
use underscore-prefixed names to satisfy Ruff ARG005, while retaining the
required three-argument signature and existing return behavior.

Source: Linters/SAST tools

mock_native.start_runtime.return_value = json.dumps(_runtime())

def invoke(plan_json: str, runtime_json: str, request_json: str) -> str:
Expand Down
Loading
Loading