From b94cd9ed641a25a8ce4996e64e41c9a9a2774703 Mon Sep 17 00:00:00 2001 From: Disha714 Date: Wed, 5 Aug 2026 21:37:10 +0530 Subject: [PATCH] fix: materialise Hub adapter configs persistently Resolve Hugging Face snapshot blob symlinks through a persistent repository-and-revision local directory. This keeps from_model_directory strict while preserving repeat-download and offline behaviour. Add regression coverage for cache-style symlinks and revision-isolated materialisation directories. Assisted-by: Claude Code Assisted-by: Codex Signed-off-by: Disha714 Signed-off-by: Nigel Jones --- mellea/backends/adapters/adapter.py | 84 +++++++++++-- .../test_adapters/test_embedded_adapter.py | 111 ++++++++++++++++-- 2 files changed, 178 insertions(+), 17 deletions(-) diff --git a/mellea/backends/adapters/adapter.py b/mellea/backends/adapters/adapter.py index a173aaaa57..1de9c63ce5 100644 --- a/mellea/backends/adapters/adapter.py +++ b/mellea/backends/adapters/adapter.py @@ -16,8 +16,11 @@ import abc import contextlib +import hashlib import pathlib import re +import shutil +import tempfile import time import warnings from collections.abc import Callable @@ -1096,8 +1099,20 @@ def from_hub( ) -> list["EmbeddedIntrinsicAdapter"]: """Load embedded adapters from a Granite Switch model on Hugging Face Hub. - Downloads `adapter_index.json` and the `io_configs/` directory, then - delegates to :meth:`from_model_directory`. + Downloads `adapter_index.json` and the `io_configs/` directory into a + persistent self-contained local directory, then delegates to + `from_model_directory`. + + `huggingface_hub.snapshot_download`'s default cache-backed snapshot + directory populates `io_configs/` with symlinks that resolve into a + sibling `blobs/` directory *outside* the snapshot root. That breaks the + contract `from_model_directory` expects (a self-contained model + directory) and trips its path-escape check. To satisfy that contract, + the downloaded snapshot is materialised under the Hugging Face cache + into a self-contained directory keyed by its immutable revision, so + `io_configs/` contains real files rather than symlinks escaping the + directory. This preserves standard Hugging Face Hub cache reuse and + offline loading while preventing stale files from a mutable revision. Args: repo_id (str): Hugging Face Hub repository ID @@ -1118,10 +1133,11 @@ def from_hub( `adapter_index.json` (wrong repo/revision, not a Granite Switch model, or a stale cache). ValueError: If no adapters are found (delegated from - :meth:`from_model_directory`). + `from_model_directory`). """ try: import huggingface_hub + from huggingface_hub.constants import HF_HUB_CACHE from huggingface_hub.errors import GatedRepoError, RepositoryNotFoundError except ImportError as e: raise ImportError( @@ -1130,11 +1146,13 @@ def from_hub( ) from e try: - local_root = huggingface_hub.snapshot_download( - repo_id=repo_id, - allow_patterns=["adapter_index.json", "io_configs/**"], - cache_dir=cache_dir, - revision=revision, + snapshot_root = pathlib.Path( + huggingface_hub.snapshot_download( + repo_id=repo_id, + allow_patterns=["adapter_index.json", "io_configs/**"], + cache_dir=cache_dir, + revision=revision, + ) ) except (GatedRepoError, RepositoryNotFoundError) as e: auth_hint = ( @@ -1146,7 +1164,57 @@ def from_hub( ) raise PermissionError(auth_hint) from e + cache_root = pathlib.Path(cache_dir or HF_HUB_CACHE) + cache_key = hashlib.sha256( + f"{repo_id}\0{snapshot_root.name}".encode() + ).hexdigest() + local_root = cache_root / "mellea" / "embedded-adapter-configs" / cache_key + try: + if not local_root.is_dir(): + local_root.parent.mkdir(parents=True, exist_ok=True) + temporary_dir = pathlib.Path( + tempfile.mkdtemp(dir=local_root.parent, prefix=f"{cache_key}-") + ) + try: + import json as _json + + index_path = snapshot_root / "adapter_index.json" + with open(index_path, encoding="utf-8") as f: + index = _json.load(f) + shutil.copyfile(index_path, temporary_dir / "adapter_index.json") + + snapshot_cache_root = snapshot_root.parent.parent.resolve() + for entry in index.get("adapters", []): + io_config_rel = entry.get("io_config") + if io_config_rel is None: + continue + io_config_path = (snapshot_root / io_config_rel).resolve( + strict=True + ) + if not io_config_path.is_relative_to(snapshot_cache_root): + raise ValueError( + f"io_config path '{io_config_rel}' escapes " + "the downloaded Hugging Face snapshot" + ) + destination = temporary_dir / io_config_rel + destination.parent.mkdir(parents=True, exist_ok=True) + shutil.copyfile(io_config_path, destination) + + adapters = EmbeddedIntrinsicAdapter.from_model_directory( + temporary_dir, intrinsic_name=intrinsic_name + ) + try: + temporary_dir.replace(local_root) + except OSError: + if not local_root.is_dir(): + raise + else: + return adapters + finally: + if temporary_dir.exists(): + shutil.rmtree(temporary_dir) + return EmbeddedIntrinsicAdapter.from_model_directory( local_root, intrinsic_name=intrinsic_name ) diff --git a/test/backends/test_adapters/test_embedded_adapter.py b/test/backends/test_adapters/test_embedded_adapter.py index a70e8352f9..f1cf563d89 100644 --- a/test/backends/test_adapters/test_embedded_adapter.py +++ b/test/backends/test_adapters/test_embedded_adapter.py @@ -79,6 +79,14 @@ def model_dir(tmp_path): return tmp_path +@pytest.fixture +def hub_cache_dir(tmp_path, monkeypatch): + """Redirect the default Hugging Face cache outside the mocked snapshot.""" + cache_dir = tmp_path.parent / f"{tmp_path.name}-hub-cache" + monkeypatch.setattr("huggingface_hub.constants.HF_HUB_CACHE", str(cache_dir)) + return cache_dir + + # ---- EmbeddedIntrinsicAdapter.__init__ ---- @@ -295,40 +303,123 @@ def test_adapter_name_key(self, tmp_path): class TestFromHub: def test_downloads_and_delegates(self, model_dir): """from_hub calls snapshot_download then delegates to from_model_directory.""" + cache_dir = model_dir.parent / "cache" with patch( "huggingface_hub.snapshot_download", return_value=str(model_dir) ) as mock_dl: adapters = EmbeddedIntrinsicAdapter.from_hub( "ibm-granite/granite-switch-micro", revision="test-rev", - cache_dir="/tmp/test-cache", + cache_dir=str(cache_dir), ) mock_dl.assert_called_once_with( repo_id="ibm-granite/granite-switch-micro", allow_patterns=["adapter_index.json", "io_configs/**"], - cache_dir="/tmp/test-cache", + cache_dir=str(cache_dir), revision="test-rev", ) assert len(adapters) == 2 def test_filter_single_intrinsic(self, model_dir): + cache_dir = model_dir.parent / "cache" with patch( "huggingface_hub.snapshot_download", return_value=str(model_dir) ) as mock_dl: adapters = EmbeddedIntrinsicAdapter.from_hub( - "ibm-granite/granite-switch-micro", intrinsic_name="citations" + "ibm-granite/granite-switch-micro", + cache_dir=str(cache_dir), + intrinsic_name="citations", ) mock_dl.assert_called_once_with( repo_id="ibm-granite/granite-switch-micro", allow_patterns=["adapter_index.json", "io_configs/**"], - cache_dir=None, + cache_dir=str(cache_dir), revision="main", ) assert len(adapters) == 1 assert adapters[0].intrinsic_name == "citations" + def test_from_hub_materialises_hub_snapshot(self, model_dir, tmp_path): + """from_hub materialises Hub blob symlinks into its persistent local directory.""" + source_files = [ + model_dir / "adapter_index.json", + *model_dir.glob("io_configs/*/io.yaml"), + ] + + def snapshot_download(**_): + snapshot_dir = tmp_path / "snapshots" / "revision" + blob_dir = tmp_path / "blobs" + for source in source_files: + destination = snapshot_dir / source.relative_to(model_dir) + destination.parent.mkdir(parents=True, exist_ok=True) + destination.write_bytes(source.read_bytes()) + + for io_config in snapshot_dir.glob("io_configs/*/io.yaml"): + blob = blob_dir / io_config.parent.name + blob.parent.mkdir(parents=True, exist_ok=True) + blob.write_bytes(io_config.read_bytes()) + io_config.unlink() + io_config.symlink_to(os.path.relpath(blob, io_config.parent)) + (snapshot_dir / "model.safetensors").write_bytes(b"weights") + + return str(snapshot_dir) + + with patch("huggingface_hub.snapshot_download", side_effect=snapshot_download): + adapters = EmbeddedIntrinsicAdapter.from_hub( + "ibm-granite/granite-switch-micro", cache_dir=str(tmp_path / "cache") + ) + + assert {adapter.intrinsic_name for adapter in adapters} == { + "answerability", + "citations", + } + materialized_dir = next( + (tmp_path / "cache" / "mellea" / "embedded-adapter-configs").iterdir() + ) + assert not (materialized_dir / "model.safetensors").exists() + + def test_from_hub_materialises_each_snapshot_revision(self, model_dir, tmp_path): + """from_hub keeps materialised configs isolated by immutable snapshot revision.""" + source_files = [ + model_dir / "adapter_index.json", + *model_dir.glob("io_configs/*/io.yaml"), + ] + + def make_snapshot(name): + snapshot_dir = tmp_path / "snapshots" / name + for source in source_files: + destination = snapshot_dir / source.relative_to(model_dir) + destination.parent.mkdir(parents=True, exist_ok=True) + destination.write_bytes(source.read_bytes()) + return snapshot_dir + + main_snapshot = make_snapshot("main-commit") + v2_snapshot = make_snapshot("v2-commit") + cache_dir = tmp_path / "cache" + with patch( + "huggingface_hub.snapshot_download", + side_effect=[str(main_snapshot), str(main_snapshot), str(v2_snapshot)], + ) as mock_dl: + EmbeddedIntrinsicAdapter.from_hub( + "ibm-granite/granite-switch-micro", cache_dir=str(cache_dir) + ) + EmbeddedIntrinsicAdapter.from_hub( + "ibm-granite/granite-switch-micro", cache_dir=str(cache_dir) + ) + EmbeddedIntrinsicAdapter.from_hub( + "ibm-granite/granite-switch-micro", + cache_dir=str(cache_dir), + revision="v2", + ) + + assert mock_dl.call_count == 3 + materialized_dirs = list( + (cache_dir / "mellea" / "embedded-adapter-configs").iterdir() + ) + assert len(materialized_dirs) == 2 + def test_missing_huggingface_hub_raises(self): with patch.dict("sys.modules", {"huggingface_hub": None}): with pytest.raises(ImportError, match="huggingface_hub is required"): @@ -360,7 +451,9 @@ class _FakeResponse: # Original HF error is chained for debugging. assert exc.value.__cause__ is hf_error - def test_missing_index_after_download_raises_clear_file_not_found(self, tmp_path): + def test_missing_index_after_download_raises_clear_file_not_found( + self, tmp_path, hub_cache_dir + ): """A snapshot without adapter_index.json raises a repo-scoped FileNotFoundError. snapshot_download can return a path that lacks the index (wrong @@ -397,7 +490,7 @@ def test_local_directory_with_filter(self, model_dir): assert len(adapters) == 1 assert adapters[0].intrinsic_name == "answerability" - def test_hub_repo_id(self, model_dir): + def test_hub_repo_id(self, model_dir, hub_cache_dir): """Non-local string routes to from_hub.""" with patch( "huggingface_hub.snapshot_download", return_value=str(model_dir) @@ -471,7 +564,7 @@ def test_list_adapters(self, backend): def test_base_model_name(self, backend): assert backend.base_model_name == "granite-switch" - def test_register_embedded_adapter_model(self, backend, model_dir): + def test_register_embedded_adapter_model(self, backend, model_dir, hub_cache_dir): with patch("huggingface_hub.snapshot_download", return_value=str(model_dir)): names = backend.register_embedded_adapter_model( "ibm-granite/granite-switch-micro" @@ -500,7 +593,7 @@ def test_register_overwrites_existing(self, backend): == 20 ) - def test_embedded_adapters_flag_loads_from_model_id(self, model_dir): + def test_embedded_adapters_flag_loads_from_model_id(self, model_dir, hub_cache_dir): """embedded_adapters=True auto-registers adapters using model_id as source.""" from mellea.backends.openai import OpenAIBackend @@ -533,7 +626,7 @@ def test_adapter_source_used_for_loading(self, model_dir): assert len(backend._added_adapters) == 2 assert backend._model_id == "granite-switch" - def test_adapter_source_defaults_to_model_id(self, model_dir): + def test_adapter_source_defaults_to_model_id(self, model_dir, hub_cache_dir): """Without adapter_source, model_id is used (existing behavior).""" from mellea.backends.openai import OpenAIBackend