From 1c5b04aef4d45620c27301679768e882ca348d4a Mon Sep 17 00:00:00 2001 From: Nigel Jones Date: Fri, 25 Sep 2026 21:39:50 +0100 Subject: [PATCH 1/7] fix(backends): activate aLoRA adapters on the LocalHFBackend generate path aLoRA intrinsic functions called through LocalHFBackend silently ran the base model: PEFT computes and injects alora_offsets only from inside its PeftModel wrapper's generate()/forward() overrides, and the backend loads adapters via the bare model.load_adapter() path, so the aLoRA variant layers never received offsets and masked the adapter off for every token. generate_with_transformers() now wraps the model.generate() call in _alora_activation_context(): a no-op unless exactly one active adapter declares alora_invocation_tokens, in which case it computes offsets with PEFT's public calculate_alora_offsets and registers PEFT's own 2-line pre-forward hooks on every LoraLayer for the duration of the call. The hook is required because transformers' model code does not propagate kwargs to the projection Linear calls. Handles both bare PeftAdapterMixin models (active_adapters as method) and PeftModel wrappers (property; idempotent there). Also repairs, with a warning, the published requirement-check io.yaml instruction that does not tokenise to its own declared invocation sequence (issue #1679): verification-driven and self-terminating, so a correctly republished adapter file is left untouched in either fix direction. The repair block is intentionally removable if we prefer Mellea not to touch published prompt text. Tests: 8 mechanism unit tests on a tiny random model (no download), 8 repair unit tests on a fake tokenizer mimicking the Granite BPE merge of the '>' and ':' tokens, and 2 qualitative e2e differential tests (requirement-check on the as-published adapter, uncertainty) asserting adapter-on vs adapter-off moves the score by more than 0.5 through the real backend path. Assisted-by: pi Signed-off-by: Nigel Jones --- mellea/backends/huggingface.py | 116 +++++ mellea/formatters/granite/base/util.py | 88 +++- test/backends/test_alora_invocation_repair.py | 136 ++++++ .../base/test_base_alora_activation.py | 402 ++++++++++++++++++ 4 files changed, 741 insertions(+), 1 deletion(-) create mode 100644 test/backends/test_alora_invocation_repair.py create mode 100644 test/formatters/granite/base/test_base_alora_activation.py diff --git a/mellea/backends/huggingface.py b/mellea/backends/huggingface.py index 910c166f1d..b759a6bb5e 100644 --- a/mellea/backends/huggingface.py +++ b/mellea/backends/huggingface.py @@ -1063,6 +1063,53 @@ def _obtain_local_file_io_yaml_config(self, binding: LocalFileBinding) -> dict: ) return loaded + def _repair_composed_alora_instruction( + self, io_yaml_config: dict, qualified_name: str + ) -> None: + """Repair an aLoRA io.yaml instruction that cannot activate its own adapter. + + Some published adapters (issue #1679: `requirement-check`, all + granite-4.1 slots) ship an instruction whose text does not tokenise to + the `alora_invocation_tokens` declared in the adapter's own config, + so the adapter can never activate no matter what the loading path + does. This loads a locally repaired instruction instead, with a loud + warning, so the capability works end to end until the publisher + republishes a corrected file. The repair itself is + verification-driven and self-terminating (see + `_alora_invocation_repair`): a healthy file -- in either direction a + publisher might fix it -- is returned untouched. + + Called at the composed-adapter commit point in `add_adapter`, once + `binding.prepare()` has loaded the PEFT config the declared sequence + is read from. + + Args: + io_yaml_config: The freshly loaded `io.yaml` mapping for this + adapter; mutated in place only when a repair applies. + qualified_name: The PEFT adapter name under which `prepare()` + loaded the weights (`binding.qualified_name`). + """ + peft_config = self._model.peft_config.get(qualified_name) + invocation_tokens = getattr(peft_config, "alora_invocation_tokens", None) + if not invocation_tokens: + return + instruction = io_yaml_config.get("instruction") + if not isinstance(instruction, str) or not instruction: + return + repaired = _alora_invocation_repair( + self._tokenizer, instruction, invocation_tokens + ) + if repaired is None: + return + MelleaLogger.get_logger().warning( + f"Adapter {qualified_name!r}: the published io.yaml instruction does " + "not tokenise to the adapter's declared aLoRA invocation sequence, so " + "the adapter could never activate as published. Loaded a locally " + "repaired instruction instead (issue #1679). Ask the adapter " + "publisher to republish a corrected io.yaml." + ) + io_yaml_config["instruction"] = repaired + async def _generate_from_intrinsic( self, action: Intrinsic, @@ -2954,6 +3001,7 @@ def add_adapter(self, adapter: AdapterInput, *, config: dict | None = None) -> N # commit, releasing (terminal, weights unloaded) the exact # binding this is about to publish as registered. with binding.hold_prepared(self), self._adapter_activation_lock(): + self._repair_composed_alora_instruction(io_yaml_config, key) self._composed_adapter_configs[key] = io_yaml_config self._composed_adapters[key] = adapter return @@ -3462,3 +3510,71 @@ def deactivate(self) -> None: def release(self) -> None: return + + +def _token_sequence_present( + tokenizer: PreTrainedTokenizerBase, text: str, token_ids: Sequence[int] +) -> bool: + """Whether `token_ids` occurs as a contiguous run in `tokenizer.encode(text)`. + + Args: + tokenizer: The tokenizer to encode with. + text: The text to search. + token_ids: The token run to look for. + + Returns: + True if the run occurs at least once in the encoded text. + """ + if not token_ids: + return False + tokens = tokenizer.encode(text, add_special_tokens=False) + seq = list(token_ids) + n = len(seq) + return any(tokens[i : i + n] == seq for i in range(len(tokens) - n + 1)) + + +def _alora_invocation_repair( + tokenizer: PreTrainedTokenizerBase, + instruction: str, + invocation_tokens: Sequence[int], +) -> str | None: + """Repair an aLoRA io.yaml instruction that cannot activate its own adapter. + + An aLoRA adapter activates only when its declared `alora_invocation_tokens` + occur, after tokenisation, in the assembled prompt. Some published + adapters (issue #1679: `requirement-check`, all granite-4.1 slots) ship an + instruction whose text does not tokenise to the declared sequence, because + the Granite tokeniser merges `>` with a following `:` into one token, so + the instruction's `:` never yields the declared + `` run. + + The repair is deliberately verification-driven and self-terminating: it + returns a changed instruction only when the declared sequence is absent + from the tokenised instruction, the decoded invocation text followed by a + single colon is present in it, and dropping that colon makes the declared + sequence present. A correctly republished file passes the first check and + is left untouched, in either direction a publisher might fix it (the + instruction text changed, or the declared tokens changed to match the + existing text). + + Args: + tokenizer: The base model's tokenizer. + instruction: The io.yaml `instruction` template text. + invocation_tokens: The adapter's declared `alora_invocation_tokens`. + + Returns: + The repaired instruction, or `None` when the instruction is already + consistent with the declared sequence or no repair is possible. + """ + if _token_sequence_present(tokenizer, instruction, invocation_tokens): + return None + invocation_text = cast( + str, tokenizer.decode(list(invocation_tokens), skip_special_tokens=False) + ) + broken = invocation_text + ":" + if broken not in instruction: + return None + repaired = instruction.replace(broken, invocation_text, 1) + if _token_sequence_present(tokenizer, repaired, invocation_tokens): + return repaired + return None diff --git a/mellea/formatters/granite/base/util.py b/mellea/formatters/granite/base/util.py index 94a9243b84..8d10502f96 100644 --- a/mellea/formatters/granite/base/util.py +++ b/mellea/formatters/granite/base/util.py @@ -6,6 +6,7 @@ # Standard from __future__ import annotations +import contextlib import itertools import json import os @@ -378,6 +379,90 @@ def chat_completion_request_to_transformers_inputs( return generate_input, other_input +@contextlib.contextmanager +def _alora_activation_context(model: PreTrainedModel, input_tokens: Any) -> Any: + """Context manager that makes aLoRA weights actually apply during `generate()`. + + PEFT computes the per-token aLoRA activation mask (`alora_offsets`) only + from inside its `PeftModel` wrapper's own `generate()`/`forward()` + overrides. A model that loads adapters via transformers' native + `PeftAdapterMixin.load_adapter()` (as `LocalHFBackend` does) is never + wrapped in `PeftModel`, so without this the aLoRA weights silently never + apply and generation degrades to base-model behaviour. This mirrors what + `PeftModel` does internally (`_enable_peft_forward_hooks` in + `peft/tuners/lora/model.py`): compute the offsets with PEFT's public + `calculate_alora_offsets`, then inject them into every LoRA layer's + forward via temporary pre-forward hooks. The hook is required because + transformers' model code does not generally propagate `**kwargs` down to + the projection `nn.Linear` calls, so passing `alora_offsets` to + `generate()` alone is not enough. + + Yields immediately (no-op) unless exactly one adapter is active and its + PEFT config declares `alora_invocation_tokens`. Hooks are removed on + exit, including on error. Safe to call on any model; the PEFT imports + happen lazily so this module keeps working without the `hf` extra. + + Args: + model: Hugging Face model object (or a proxy that forwards attribute + access to one). + input_tokens: Prompt token-id tensor on the model's device, as + passed to `generate(input_ids=...)`. + + Yields: + None. Active for the duration of the `with` block. + """ + peft_config = getattr(model, "peft_config", None) + if not peft_config: + yield + return + try: + active_adapters = model.active_adapters + # `active_adapters` is a method on transformers' native + # PeftAdapterMixin but a property on peft's PeftModel wrapper. + active = cast( + "list[str]", + active_adapters() if callable(active_adapters) else active_adapters, + ) + except ValueError: + active = [] + if len(active) != 1: + yield + return + adapter_name = active[0] + invocation = getattr(peft_config.get(adapter_name), "alora_invocation_tokens", None) + if invocation is None: + yield + return + if input_tokens is None or not hasattr(input_tokens, "device"): + yield + return + + # Third Party (lazy: peft is an optional `hf`-extra dependency) + from peft.tuners.lora import ( + layer as _peft_lora_layer, + variants as _peft_lora_variants, + ) + + offsets = _peft_lora_variants.calculate_alora_offsets( + peft_config, adapter_name, input_tokens + ) + + def _inject_offsets(module: Any, args: Any, kwargs: Any) -> Any: + kwargs["alora_offsets"] = offsets + return args, kwargs + + handles = [ + module.register_forward_pre_hook(_inject_offsets, with_kwargs=True) + for module in model.modules() + if isinstance(module, _peft_lora_layer.LoraLayer) + ] + try: + yield + finally: + for handle in handles: + handle.remove() + + def generate_with_transformers( tokenizer: PreTrainedTokenizerBase, model: PreTrainedModel, @@ -412,7 +497,8 @@ def generate_with_transformers( generate_input = generate_input.copy() del generate_input["input_tokens"] - generate_result = model.generate(input_ids=input_tokens, **generate_input) # type: ignore[operator] + with _alora_activation_context(model, input_tokens): + generate_result = model.generate(input_ids=input_tokens, **generate_input) # type: ignore[operator] # Result is a a 2D tensor of shape (num responses, prompt + max generated tokens) # containing tokens, plus a tuple of tensors of shape diff --git a/test/backends/test_alora_invocation_repair.py b/test/backends/test_alora_invocation_repair.py new file mode 100644 index 0000000000..040df54384 --- /dev/null +++ b/test/backends/test_alora_invocation_repair.py @@ -0,0 +1,136 @@ +# Copyright IBM Corp. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Unit tests for the aLoRA io.yaml invocation-sequence repair (issue #1679). + +The repair under test (`_alora_invocation_repair`) handles published adapters +whose io.yaml instruction text does not tokenise to the invocation sequence +declared in the adapter's own config — specifically the `requirement-check` +aLoRA, where the instruction starts `:` but the declared +sequence is `` (no colon) and the Granite tokeniser merges `>:` +into a single token, so the sequence can never appear. + +The fake tokenizer below mimics exactly that BPE quirk at word granularity: +`>:` is one token, a standalone `>` is a different one. +""" + +# Standard +import re +from collections.abc import Sequence + +# Third Party +import pytest + +pytest.importorskip( + "transformers", reason="transformers not installed — install mellea[hf]" +) + +# First Party +from mellea.backends.huggingface import ( + _alora_invocation_repair, + _token_sequence_present, +) + +_TOKEN_RE = re.compile(r">:|\S+|\s+") + + +class _MergingTokenizer: + """Word-level tokenizer where `>:` merges into one token, like the Granite BPE quirk.""" + + def __init__(self, seed_text: str): + self._ids: dict[str, int] = {} + self._by_id: dict[int, str] = {} + for m in _TOKEN_RE.finditer(seed_text): + self._register(m.group()) + + def _register(self, token: str) -> None: + if token not in self._ids: + self._ids[token] = len(self._ids) + 1 + self._by_id[self._ids[token]] = token + + def encode(self, text: str, add_special_tokens: bool = True) -> list[int]: + out = [] + for m in _TOKEN_RE.finditer(text): + self._register(m.group()) + out.append(self._ids[m.group()]) + return out + + def decode(self, ids: Sequence[int], skip_special_tokens: bool = True) -> str: + return "".join(self._by_id[i] for i in ids) + + +def _tokenizer() -> _MergingTokenizer: + # Seed so both "" and ":" are encodable. + return _MergingTokenizer(" : x y") + + +def _invocation_no_colon() -> list[int]: + return _tokenizer().encode("") + + +def _invocation_with_colon() -> list[int]: + return _tokenizer().encode(":") + + +class TestTokenSequencePresent: + def test_present(self): + tok = _tokenizer() + assert _token_sequence_present( + tok, "a b", tok.encode("") + ) + + def test_absent_when_colon_merges(self): + tok = _tokenizer() + assert not _token_sequence_present( + tok, "a : b", tok.encode("") + ) + + def test_empty_ids(self): + assert not _token_sequence_present(_tokenizer(), "anything", []) + + +class TestAloraInvocationRepair: + def test_healthy_instruction_unchanged(self): + tok = _tokenizer() + instruction = " {requirement}\nEvaluate." + assert ( + _alora_invocation_repair(tok, instruction, _invocation_no_colon()) is None + ) + + def test_repairs_colon_mismatch(self): + tok = _tokenizer() + instruction = ": {requirement}\nEvaluate." + repaired = _alora_invocation_repair(tok, instruction, _invocation_no_colon()) + assert repaired == " {requirement}\nEvaluate." + assert _token_sequence_present(tok, repaired, _invocation_no_colon()) + + def test_self_terminates_when_publisher_fixes_tokens_instead(self): + """If the publisher re-declares the invocation tokens to match the + existing `:` text (the other valid fix direction), the + file is healthy and must be left untouched.""" + tok = _tokenizer() + instruction = ": {requirement}\nEvaluate." + assert ( + _alora_invocation_repair(tok, instruction, _invocation_with_colon()) is None + ) + + def test_invocation_not_from_instruction_is_untouched(self): + """Adapters whose invocation sequence is supplied by the chat template + (e.g. role markers) never match instruction text; no repair applies.""" + tok = _tokenizer() + instruction = ": {requirement}\nEvaluate." + assert ( + _alora_invocation_repair(tok, instruction, tok.encode("<|start_of_role|>")) + is None + ) + + def test_unrepairable_mismatch_returns_none(self): + """Declared sequence is `:` (with colon) but the text + only ever carries `` with the colon elsewhere: the + `broken`-form (`:` + `:`) appears nowhere, so no repair + applies and the text is left untouched for an upstream fix.""" + tok = _tokenizer() + instruction = "x y: {requirement}\nEvaluate." + assert ( + _alora_invocation_repair(tok, instruction, _invocation_with_colon()) is None + ) diff --git a/test/formatters/granite/base/test_base_alora_activation.py b/test/formatters/granite/base/test_base_alora_activation.py new file mode 100644 index 0000000000..7f052697ab --- /dev/null +++ b/test/formatters/granite/base/test_base_alora_activation.py @@ -0,0 +1,402 @@ +# Copyright IBM Corp. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Tests for aLoRA activation through the bare-model (`load_adapter`) path. + +Background: PEFT only computes and injects aLoRA activation masks +(`alora_offsets`) from inside its `PeftModel` wrapper's `generate()`/ +`forward()` overrides. Models that load adapters via transformers' native +`PeftAdapterMixin.load_adapter()` — which is what `LocalHFBackend` does — +are never wrapped in `PeftModel`, so the aLoRA weights silently never apply +and generation degrades to base-model behaviour. +`util._alora_activation_context` fixes this by mirroring `PeftModel`'s own +mechanism (offset computation + temporary per-layer pre-forward hooks). + +These tests use a tiny randomly-initialised model (no download) and assert +on whether the aLoRA variant layer actually receives `alora_offsets` during +`generate()` — the exact property that was missing before the fix. +""" + +# Standard +from unittest.mock import MagicMock + +# Third Party +import pytest + +torch = pytest.importorskip("torch", reason="torch not installed — install mellea[hf]") +pytest.importorskip( + "transformers", reason="transformers not installed — install mellea[hf]" +) +peft = pytest.importorskip("peft", reason="peft not installed — install mellea[hf]") +# First Party +import peft.tuners.lora.variants as peft_variants +from peft import LoraConfig +from transformers import LlamaConfig, LlamaForCausalLM + +from mellea.backends.huggingface import LocalHFBackend +from mellea.formatters.granite.base.util import ( + _alora_activation_context, + generate_with_transformers, +) + +INVOCATION = [1, 2, 3] + + +def _tiny_model(adapter_config: LoraConfig | None = None) -> LlamaForCausalLM: + config = LlamaConfig( + vocab_size=64, + hidden_size=32, + intermediate_size=64, + num_hidden_layers=2, + num_attention_heads=2, + num_key_value_heads=2, + max_position_embeddings=128, + ) + model = LlamaForCausalLM(config).eval() + if adapter_config is not None: + model.add_adapter(adapter_config, "rc") + model.set_adapter("rc") + return model + + +def _alora_config() -> LoraConfig: + return LoraConfig( + r=4, + lora_alpha=8, + target_modules=["q_proj", "v_proj"], + task_type="CAUSAL_LM", + alora_invocation_tokens=INVOCATION, + ) + + +def _lora_config() -> LoraConfig: + return LoraConfig( + r=4, lora_alpha=8, target_modules=["q_proj", "v_proj"], task_type="CAUSAL_LM" + ) + + +class _VariantSpy: + """Count ALoraLinearVariant.forward calls and record the alora_offsets each received.""" + + def __init__(self): + self.n_calls = 0 + self.offsets_seen: list = [] + self._orig = peft_variants.ALoraLinearVariant.forward + + def __enter__(self): + def spy(module, active_adapter, x, result, **kwargs): + self.n_calls += 1 + self.offsets_seen.append(kwargs.get("alora_offsets")) + return self._orig(module, active_adapter, x, result, **kwargs) + + peft_variants.ALoraLinearVariant.forward = staticmethod(spy) + return self + + def __exit__(self, *exc): + # Restore as a staticmethod: class-attribute access to a staticmethod + # returns the bare function, so a plain reassignment would turn it + # into an instance method and break every later aLoRA forward. + peft_variants.ALoraLinearVariant.forward = staticmethod(self._orig) + return False + + +class TestAloraActivationContext: + def test_no_offsets_reach_variant_without_context(self): + """The regression itself: bare model + aLoRA adapter, no context -> + the aLoRA variant is called but never receives alora_offsets, so it + masks to base-model behaviour.""" + model = _tiny_model(_alora_config()) + input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) + with _VariantSpy() as spy: + with torch.no_grad(): + model.generate(input_ids=input_ids, max_new_tokens=2) + assert spy.n_calls > 0, "aLoRA variant layer should be exercised at all" + assert spy.offsets_seen == [None] * spy.n_calls + + def test_context_delivers_expected_offsets(self): + model = _tiny_model(_alora_config()) + # Invocation sequence starts at index 1 of a 6-token prompt; PEFT + # counts the offset back from the end, so it is 6 - 1 = 5. + input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) + with _VariantSpy() as spy: + with torch.no_grad(), _alora_activation_context(model, input_ids): + model.generate(input_ids=input_ids, max_new_tokens=2) + assert spy.n_calls > 0 + assert spy.offsets_seen[0] == [5] + assert all(o == [5] for o in spy.offsets_seen) + + def test_context_with_missing_invocation_passes_none_offset(self): + """Invocation sequence absent -> offsets are [None] (peft semantics: + adapter inactive for that row), which the variant masks to base.""" + model = _tiny_model(_alora_config()) + input_ids = torch.tensor([[10, 11, 12, 40, 50]]) + with _VariantSpy() as spy: + with torch.no_grad(), _alora_activation_context(model, input_ids): + model.generate(input_ids=input_ids, max_new_tokens=2) + assert spy.offsets_seen and all(o == [None] for o in spy.offsets_seen) + + def test_hooks_removed_after_context_exit(self): + model = _tiny_model(_alora_config()) + input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) + with torch.no_grad(), _alora_activation_context(model, input_ids): + pass + n_hooks = sum( + len(getattr(m, "_forward_pre_hooks", {})) for m in model.modules() + ) + assert n_hooks == 0, "pre-forward hooks must not outlive the context" + + def test_hooks_removed_on_error(self): + model = _tiny_model(_alora_config()) + input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) + with pytest.raises(RuntimeError, match="boom"): + with torch.no_grad(), _alora_activation_context(model, input_ids): + raise RuntimeError("boom") + n_hooks = sum( + len(getattr(m, "_forward_pre_hooks", {})) for m in model.modules() + ) + assert n_hooks == 0 + + def test_no_adapter_is_noop(self): + model = _tiny_model() + input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) + with torch.no_grad(), _alora_activation_context(model, input_ids): + model.generate(input_ids=input_ids, max_new_tokens=2) # must not raise + + def test_plain_lora_adapter_is_noop(self): + model = _tiny_model(_lora_config()) + input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) + with torch.no_grad(), _alora_activation_context(model, input_ids): + model.generate(input_ids=input_ids, max_new_tokens=2) + n_hooks = sum( + len(getattr(m, "_forward_pre_hooks", {})) for m in model.modules() + ) + assert n_hooks == 0 + + def test_generate_with_transformers_uses_context(self): + """Wiring check: the production call path must deliver offsets to the + variant, not just the standalone context manager.""" + model = _tiny_model(_alora_config()) + input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) + tokenizer = MagicMock() + tokenizer.eos_token_id = 9999 # outside vocab: never in generated tokens + tokenizer.decode.side_effect = lambda *a, **k: "x" + tokenizer.batch_decode.side_effect = lambda seqs: ["x"] * len(seqs) + with _VariantSpy() as spy: + generate_with_transformers( + tokenizer, + model, + generate_input={ + "input_tokens": input_ids, + "max_new_tokens": 2, + "do_sample": False, + "return_dict_in_generate": True, + }, + other_input={}, + ) + assert spy.n_calls > 0 + assert spy.offsets_seen[0] == [5] + + +@pytest.mark.huggingface +@pytest.mark.e2e +@pytest.mark.qualitative +@pytest.mark.slow +class TestAloraDifferentialEndToEnd: + """The aLoRA weights must measurably change the score versus no adapter. + + Guards against the silent-degradation failure mode where aLoRA + generation falls back to base-model behaviour (issue #1679): before the + fix, adapter-on and adapter-off scores were byte-identical on every item. + The cardiff probe item is one where granite-4.1-3b's base model is + confidently wrong (says "yes" to a bullet-points check on a plain + sentence) and the published requirement-check aLoRA is correct ("no") — + scores measured 0.047 vs 0.999 in the #1679 diagnostic eval. + """ + + def test_requirement_check_adapter_moves_score(self, gh_run): + # Skip the expensive model download + inference on CI, following the + # gh_run pattern used by the other huggingface e2e tests in this suite. + if gh_run == 1: + pytest.xfail("Model download + inference not run on CI") + + from mellea.backends.adapters._core import Adapter, Identity, LocalFileBinding + from mellea.backends.adapters.catalog import ( + AdapterType, + fetch_intrinsic_metadata, + ) + from mellea.backends.adapters.io_contracts import get_io_contract + from mellea.stdlib.components import Message + from mellea.stdlib.components.intrinsic import core + from mellea.stdlib.context import ChatContext + + # NOTE: the published requirement-check io.yaml instruction does not + # tokenise to the adapter's declared invocation sequence (issue + # #1679). LocalHFBackend repairs it at load time with a warning + # (`_repair_composed_alora_instruction`) until the publisher + # republishes a corrected file, so this test runs on the as-published + # adapter with no local workaround. + md = fetch_intrinsic_metadata("requirement-check") + backend = LocalHFBackend(model_id="ibm-granite/granite-4.1-3b") + backend.add_adapter( + Adapter( + identity=Identity( + name="requirement-check", + adapter_type="alora", + capability="requirement_check", + ), + io_contract=get_io_contract("requirement-check"), + weights=LocalFileBinding( + name="requirement-check", + adapter_type=AdapterType.ALORA, + repo_id=md.repo_id, + revision=md.revision, + ), + ) + ) + ctx = ( + ChatContext() + .add(Message("user", "Write one sentence about Cardiff.")) + .add(Message("assistant", "Cardiff is the capital city of Wales.")) + ) + requirement = "The response uses bullet points." + + # The model-level PEFT adapter name as registered by load_adapter. + qualified = next(iter(backend._model.peft_config)) + + # Adapter ON: the intrinsic path activates the adapter itself + # around the generate call; this is the real production flow. + score_on = core.requirement_check(ctx, backend, requirement) + + # Adapter OFF: the intrinsic scope re-activates the adapter for + # every call, so the only way to get a genuine no-adapter score + # is to clear the model's active adapters at the generate level + # (the same set_adapter([]) override the diagnostic harness used; + # disable_adapters() is a documented no-op on this model class). + orig_generate = backend._model.generate + + def off_generate(*a, **kw): + backend._model.set_adapter([]) + try: + return orig_generate(*a, **kw) + finally: + backend._model.set_adapter(qualified) + + backend._model.generate = off_generate + try: + score_off = core.requirement_check(ctx, backend, requirement) + finally: + backend._model.generate = orig_generate + + # The adapter must say "no" (low score); the base model says "yes". + assert score_on < 0.5, ( + f"aLoRA active: expected 'no' (score < 0.5), got {score_on}" + ) + assert score_off > 0.5, ( + f"base model: expected 'yes' (score > 0.5), got {score_off}" + ) + assert score_off - score_on > 0.5, ( + f"adapter contribution too small to be a real effect: " + f"on={score_on}, off={score_off}" + ) + + +@pytest.mark.huggingface +@pytest.mark.e2e +@pytest.mark.qualitative +@pytest.mark.slow +class TestUncertaintyAloraDifferentialEndToEnd: + """Second aLoRA capability (uncertainty): confirms the activation fix is + general, not requirement-check-specific. + + The published uncertainty aLoRA io.yaml has no tokenisation mismatch, so + this test needs no instruction patching — it exercises the fix on the + as-published adapter files. Measured on granite-4.1-3b in the #1679 + diagnostic: base model is underconfident (~0.06) on both a right and a + wrong last response, while the adapter gives ~0.95 (right) and ~0.73 + (wrong) — a wide differential either way. + """ + + def test_check_certainty_adapter_moves_score(self, gh_run): + if gh_run == 1: + pytest.xfail("Model download + inference not run on CI") + + from mellea.backends.adapters._core import Adapter, Identity, LocalFileBinding + from mellea.backends.adapters.catalog import ( + AdapterType, + fetch_intrinsic_metadata, + ) + from mellea.backends.adapters.io_contracts import get_io_contract + from mellea.stdlib.components import Message + from mellea.stdlib.components.intrinsic import core + from mellea.stdlib.context import ChatContext + + md = fetch_intrinsic_metadata("uncertainty") + backend = LocalHFBackend(model_id="ibm-granite/granite-4.1-3b") + backend.add_adapter( + Adapter( + identity=Identity( + name="uncertainty", adapter_type="alora", capability="uncertainty" + ), + io_contract=get_io_contract("uncertainty"), + weights=LocalFileBinding( + name="uncertainty", + adapter_type=AdapterType.ALORA, + repo_id=md.repo_id, + revision=md.revision, + ), + ) + ) + qualified = next(iter(backend._model.peft_config)) + + def score_with_adapter_off(user: str, assistant: str) -> float: + ctx = ( + ChatContext() + .add(Message("user", user)) + .add(Message("assistant", assistant)) + ) + orig_generate = backend._model.generate + + def off_generate(*a, **kw): + backend._model.set_adapter([]) + try: + return orig_generate(*a, **kw) + finally: + backend._model.set_adapter(qualified) + + backend._model.generate = off_generate + try: + return core.check_certainty(ctx, backend) + finally: + backend._model.generate = orig_generate + + user = "What is the capital of France?" + score_on_right = core.check_certainty( + ChatContext() + .add(Message("user", user)) + .add(Message("assistant", "The capital of France is Paris.")), + backend, + ) + score_off_right = score_with_adapter_off( + user, "The capital of France is Paris." + ) + + score_on_wrong = core.check_certainty( + ChatContext() + .add(Message("user", user)) + .add(Message("assistant", "The capital of France is Madrid.")), + backend, + ) + score_off_wrong = score_with_adapter_off( + user, "The capital of France is Madrid." + ) + + # The adapter must move the certainty score by a wide margin on both + # cases; the base 3b model sits near 0.06 in both. + assert score_on_right - score_off_right > 0.5, ( + f"right answer: adapter contribution too small: " + f"on={score_on_right}, off={score_off_right}" + ) + assert score_on_wrong - score_off_wrong > 0.5, ( + f"wrong answer: adapter contribution too small: " + f"on={score_on_wrong}, off={score_off_wrong}" + ) From 8b4216f5661a56e34e1348ce186e59b5d6acdd6f Mon Sep 17 00:00:00 2001 From: Nigel Jones Date: Mon, 28 Sep 2026 10:09:41 +0100 Subject: [PATCH 2/7] fix(backends): repair aLoRA instruction on the shim path and harden activation edges Review follow-ups for the LocalHFBackend aLoRA activation fix (#1679). - Apply the io.yaml invocation repair on the deprecated IntrinsicAdapter path too. That shim loads its weights per generate call, after its config has been rendered into the prompt, so the declared sequence is read from the downloaded adapter_config.json at registration. The repair now returns a copy instead of mutating, since a shim's config can be the caller's dict. - Refuse beam search for an activating aLoRA, as PEFT's own path does, and repeat offsets per row for num_return_sequences > 1; both previously failed with an opaque IndexError inside the variant layer. - When the declared invocation sequence is absent from the prompt, register no hooks and log a WARNING once per adapter instead of staying silent. - Treat an empty alora_invocation_tokens as a no-op, and tolerate only the "No adapter loaded" ValueError from active_adapters(). - Tighten types, strengthen the no-op and teardown tests to assert hook counts inside the context, relabel the upstream-canary test, add require_gpu to the e2e classes, and drop their dead gh_run xfail. Assisted-by: Claude Code Signed-off-by: Nigel Jones --- mellea/backends/huggingface.py | 76 +++++-- mellea/formatters/granite/base/util.py | 92 +++++++-- test/backends/test_alora_invocation_repair.py | 182 +++++++++++++++++ .../base/test_base_alora_activation.py | 186 +++++++++++++----- 4 files changed, 457 insertions(+), 79 deletions(-) diff --git a/mellea/backends/huggingface.py b/mellea/backends/huggingface.py index b759a6bb5e..82c76fa7aa 100644 --- a/mellea/backends/huggingface.py +++ b/mellea/backends/huggingface.py @@ -1063,9 +1063,12 @@ def _obtain_local_file_io_yaml_config(self, binding: LocalFileBinding) -> dict: ) return loaded - def _repair_composed_alora_instruction( - self, io_yaml_config: dict, qualified_name: str - ) -> None: + def _repair_alora_instruction( + self, + io_yaml_config: dict, + invocation_tokens: Sequence[int] | None, + adapter_name: str, + ) -> dict: """Repair an aLoRA io.yaml instruction that cannot activate its own adapter. Some published adapters (issue #1679: `requirement-check`, all @@ -1079,36 +1082,42 @@ def _repair_composed_alora_instruction( `_alora_invocation_repair`): a healthy file -- in either direction a publisher might fix it -- is returned untouched. - Called at the composed-adapter commit point in `add_adapter`, once - `binding.prepare()` has loaded the PEFT config the declared sequence - is read from. + Called from both `add_adapter` registration paths: the composed-adapter + commit point, with the declared sequence read from the PEFT config + `binding.prepare()` loaded, and the `IntrinsicAdapter` shim path, with + it read from the downloaded `adapter_config.json` (that shim loads its + weights per generate call, after its config has already been rendered + into the prompt). Args: - io_yaml_config: The freshly loaded `io.yaml` mapping for this - adapter; mutated in place only when a repair applies. - qualified_name: The PEFT adapter name under which `prepare()` - loaded the weights (`binding.qualified_name`). + io_yaml_config: The adapter's `io.yaml` mapping. Never mutated: a + shim's config can be the caller's own `config_dict`. + invocation_tokens: The adapter's declared + `alora_invocation_tokens`, or `None` for a non-aLoRA adapter. + adapter_name: The adapter's qualified name, for the warning. + + Returns: + `io_yaml_config` itself when no repair applies, otherwise a shallow + copy carrying the repaired instruction. """ - peft_config = self._model.peft_config.get(qualified_name) - invocation_tokens = getattr(peft_config, "alora_invocation_tokens", None) if not invocation_tokens: - return + return io_yaml_config instruction = io_yaml_config.get("instruction") if not isinstance(instruction, str) or not instruction: - return + return io_yaml_config repaired = _alora_invocation_repair( self._tokenizer, instruction, invocation_tokens ) if repaired is None: - return + return io_yaml_config MelleaLogger.get_logger().warning( - f"Adapter {qualified_name!r}: the published io.yaml instruction does " + f"Adapter {adapter_name!r}: the published io.yaml instruction does " "not tokenise to the adapter's declared aLoRA invocation sequence, so " "the adapter could never activate as published. Loaded a locally " "repaired instruction instead (issue #1679). Ask the adapter " "publisher to republish a corrected io.yaml." ) - io_yaml_config["instruction"] = repaired + return {**io_yaml_config, "instruction": repaired} async def _generate_from_intrinsic( self, @@ -3001,7 +3010,12 @@ def add_adapter(self, adapter: AdapterInput, *, config: dict | None = None) -> N # commit, releasing (terminal, weights unloaded) the exact # binding this is about to publish as registered. with binding.hold_prepared(self), self._adapter_activation_lock(): - self._repair_composed_alora_instruction(io_yaml_config, key) + peft_config = self._model.peft_config.get(key) + io_yaml_config = self._repair_alora_instruction( + io_yaml_config, + getattr(peft_config, "alora_invocation_tokens", None), + key, + ) self._composed_adapter_configs[key] = io_yaml_config self._composed_adapters[key] = adapter return @@ -3043,6 +3057,12 @@ def add_adapter(self, adapter: AdapterInput, *, config: dict | None = None) -> N return adapter.path = adapter.get_local_hf_path(self.base_model_name) + if isinstance(adapter, IntrinsicAdapter): + adapter.config = self._repair_alora_instruction( + adapter.config, + _read_alora_invocation_tokens(adapter.path), + adapter.qualified_name, + ) adapter.backend = self self._added_adapters[adapter.qualified_name] = adapter @@ -3533,6 +3553,26 @@ def _token_sequence_present( return any(tokens[i : i + n] == seq for i in range(len(tokens) - n + 1)) +def _read_alora_invocation_tokens(adapter_dir: str) -> list[int] | None: + """Read the declared `alora_invocation_tokens` from a downloaded adapter. + + Args: + adapter_dir: Local directory holding the adapter's + `adapter_config.json`. + + Returns: + The declared invocation token ids, or `None` when the directory has + no `adapter_config.json` or the config declares none (a plain LoRA). + """ + config_path = pathlib.Path(adapter_dir) / "adapter_config.json" + if not config_path.is_file(): + return None + tokens = json.loads(config_path.read_text(encoding="utf-8")).get( + "alora_invocation_tokens" + ) + return list(tokens) if tokens else None + + def _alora_invocation_repair( tokenizer: PreTrainedTokenizerBase, instruction: str, diff --git a/mellea/formatters/granite/base/util.py b/mellea/formatters/granite/base/util.py index 8d10502f96..fe5df1747f 100644 --- a/mellea/formatters/granite/base/util.py +++ b/mellea/formatters/granite/base/util.py @@ -11,6 +11,7 @@ import json import os import uuid +from collections.abc import Iterator, Mapping from typing import TYPE_CHECKING, Any, cast # Third Party @@ -379,8 +380,27 @@ def chat_completion_request_to_transformers_inputs( return generate_input, other_input +# aLoRA adapter names already warned about for a missing invocation sequence, +# so a mispackaged adapter logs once per process rather than once per call. +_ALORA_MISSING_INVOCATION_WARNED: set[str] = set() + + +def _generation_setting( + model: PreTrainedModel, generate_kwargs: Mapping[str, Any], name: str +) -> int: + """Effective integer `generate()` setting: call kwarg, else model default, else 1.""" + value = generate_kwargs.get(name) + if value is None: + value = getattr(getattr(model, "generation_config", None), name, None) + return value if isinstance(value, int) else 1 + + @contextlib.contextmanager -def _alora_activation_context(model: PreTrainedModel, input_tokens: Any) -> Any: +def _alora_activation_context( + model: PreTrainedModel, + input_tokens: torch.Tensor | None, + generate_kwargs: Mapping[str, Any] | None = None, +) -> Iterator[None]: """Context manager that makes aLoRA weights actually apply during `generate()`. PEFT computes the per-token aLoRA activation mask (`alora_offsets`) only @@ -390,26 +410,40 @@ def _alora_activation_context(model: PreTrainedModel, input_tokens: Any) -> Any: wrapped in `PeftModel`, so without this the aLoRA weights silently never apply and generation degrades to base-model behaviour. This mirrors what `PeftModel` does internally (`_enable_peft_forward_hooks` in - `peft/tuners/lora/model.py`): compute the offsets with PEFT's public - `calculate_alora_offsets`, then inject them into every LoRA layer's - forward via temporary pre-forward hooks. The hook is required because - transformers' model code does not generally propagate `**kwargs` down to - the projection `nn.Linear` calls, so passing `alora_offsets` to - `generate()` alone is not enough. + `peft/tuners/lora/model.py`): compute the offsets with PEFT's + `calculate_alora_offsets` helper (internal to + `peft.tuners.lora.variants`, not part of PEFT's exported API), then inject + them into every LoRA layer's forward via temporary pre-forward hooks. The + hook is required because transformers' model code does not generally + propagate `**kwargs` down to the projection `nn.Linear` calls, so passing + `alora_offsets` to `generate()` alone is not enough. Yields immediately (no-op) unless exactly one adapter is active and its - PEFT config declares `alora_invocation_tokens`. Hooks are removed on - exit, including on error. Safe to call on any model; the PEFT imports - happen lazily so this module keeps working without the `hf` extra. + PEFT config declares a non-empty `alora_invocation_tokens`. When the + declared sequence does not occur in `input_tokens`, the adapter cannot + activate: no hooks are registered (so generation uses the base-model + weights, as PEFT would) and a WARNING naming the adapter is logged once + per process. Offsets are repeated per returned sequence, since + `generate()` expands the prompt batch for `num_return_sequences`. Hooks + are removed on exit, including on error. Safe to call on any model; the + PEFT imports happen lazily so this module keeps working without the `hf` + extra. Args: model: Hugging Face model object (or a proxy that forwards attribute access to one). input_tokens: Prompt token-id tensor on the model's device, as passed to `generate(input_ids=...)`. + generate_kwargs: The other keyword arguments the caller is about to + pass to `generate()`, read for `num_beams` and + `num_return_sequences`. Yields: None. Active for the duration of the `with` block. + + Raises: + ValueError: Beam search (`num_beams > 1`) was requested while an aLoRA + adapter can activate; PEFT does not support that combination. """ peft_config = getattr(model, "peft_config", None) if not peft_config: @@ -423,14 +457,19 @@ def _alora_activation_context(model: PreTrainedModel, input_tokens: Any) -> Any: "list[str]", active_adapters() if callable(active_adapters) else active_adapters, ) - except ValueError: + except ValueError as e: + # transformers raises + # `ValueError("No adapter loaded. Please load an adapter first.")` + # when nothing is loaded; anything else is a real fault. + if "No adapter loaded" not in str(e): + raise active = [] if len(active) != 1: yield return adapter_name = active[0] invocation = getattr(peft_config.get(adapter_name), "alora_invocation_tokens", None) - if invocation is None: + if not invocation: yield return if input_tokens is None or not hasattr(input_tokens, "device"): @@ -446,8 +485,33 @@ def _alora_activation_context(model: PreTrainedModel, input_tokens: Any) -> Any: offsets = _peft_lora_variants.calculate_alora_offsets( peft_config, adapter_name, input_tokens ) + if all(offset is None for offset in offsets): + if adapter_name not in _ALORA_MISSING_INVOCATION_WARNED: + _ALORA_MISSING_INVOCATION_WARNED.add(adapter_name) + MelleaLogger.get_logger().warning( + f"aLoRA adapter {adapter_name!r} is active but its declared " + f"invocation sequence {list(invocation)} does not occur in the " + "prompt, so generation used base-model weights only. Check that " + "the adapter's io.yaml instruction or chat template produces " + "that token sequence." + ) + yield + return - def _inject_offsets(module: Any, args: Any, kwargs: Any) -> Any: + generate_kwargs = generate_kwargs or {} + if _generation_setting(model, generate_kwargs, "num_beams") > 1: + # Same refusal as PeftModel's own path (`_enable_peft_forward_hooks`). + raise ValueError( + f"Beam search is not supported for aLoRA adapters (adapter {adapter_name!r})." + ) + # generate() expands each prompt row `num_return_sequences` times via + # `repeat_interleave`, so repeat each row's offset the same way. + n_sequences = _generation_setting(model, generate_kwargs, "num_return_sequences") + offsets = [offset for offset in offsets for _ in range(n_sequences)] + + def _inject_offsets( + module: torch.nn.Module, args: tuple[Any, ...], kwargs: dict[str, Any] + ) -> tuple[tuple[Any, ...], dict[str, Any]]: kwargs["alora_offsets"] = offsets return args, kwargs @@ -497,7 +561,7 @@ def generate_with_transformers( generate_input = generate_input.copy() del generate_input["input_tokens"] - with _alora_activation_context(model, input_tokens): + with _alora_activation_context(model, input_tokens, generate_input): generate_result = model.generate(input_ids=input_tokens, **generate_input) # type: ignore[operator] # Result is a a 2D tensor of shape (num responses, prompt + max generated tokens) diff --git a/test/backends/test_alora_invocation_repair.py b/test/backends/test_alora_invocation_repair.py index 040df54384..b546b3820f 100644 --- a/test/backends/test_alora_invocation_repair.py +++ b/test/backends/test_alora_invocation_repair.py @@ -15,8 +15,12 @@ """ # Standard +import json +import logging import re from collections.abc import Sequence +from types import SimpleNamespace +from unittest.mock import MagicMock, mock_open, patch # Third Party import pytest @@ -24,10 +28,16 @@ pytest.importorskip( "transformers", reason="transformers not installed — install mellea[hf]" ) +torch = pytest.importorskip("torch", reason="torch not installed — install mellea[hf]") # First Party +from mellea.backends.adapters import AdapterType, IntrinsicAdapter +from mellea.backends.adapters._core import Adapter, Identity, LocalFileBinding +from mellea.backends.adapters.io_contracts import get_io_contract from mellea.backends.huggingface import ( + LocalHFBackend, _alora_invocation_repair, + _read_alora_invocation_tokens, _token_sequence_present, ) @@ -134,3 +144,175 @@ def test_unrepairable_mismatch_returns_none(self): assert ( _alora_invocation_repair(tok, instruction, _invocation_with_colon()) is None ) + + def test_colon_present_but_removal_does_not_restore_returns_none(self): + """The colon form occurs, but dropping it still does not yield the + declared run (here `:x` -> `x`, a different + token): the third check refuses rather than rewrite blindly.""" + tok = _tokenizer() + instruction = ":x {requirement}\nEvaluate." + assert ( + _alora_invocation_repair(tok, instruction, _invocation_no_colon()) is None + ) + + +class TestReadAloraInvocationTokens: + def test_reads_declared_tokens(self, tmp_path): + (tmp_path / "adapter_config.json").write_text( + json.dumps({"alora_invocation_tokens": [27, 71226, 29]}) + ) + assert _read_alora_invocation_tokens(str(tmp_path)) == [27, 71226, 29] + + def test_plain_lora_config_returns_none(self, tmp_path): + (tmp_path / "adapter_config.json").write_text(json.dumps({"r": 8})) + assert _read_alora_invocation_tokens(str(tmp_path)) is None + + def test_missing_config_file_returns_none(self, tmp_path): + assert _read_alora_invocation_tokens(str(tmp_path)) is None + + +_BROKEN_CONFIG = {"instruction": ": {requirement}\nEvaluate."} +_REPAIRED_INSTRUCTION = " {requirement}\nEvaluate." + + +def _stub_backend(tokenizer: _MergingTokenizer) -> LocalHFBackend: + """A real `LocalHFBackend` over mock model/tokenizer (no download), with + the fake merging tokenizer swapped in for the repair to use.""" + mock_tok = MagicMock(eos_token_id=0, vocab_size=32000) + mock_tok._tokenizer = MagicMock() + mock_tok._tokenizer.get_vocab_size.return_value = 32000 + mock_tok.__len__ = MagicMock(return_value=32000) + with ( + patch("mellea.backends.huggingface.llguidance") as mock_llg, + patch("mellea.backends.huggingface.set_seed"), + ): + mock_llg.hf.from_tokenizer.return_value = MagicMock(vocab_size=32000) + backend = LocalHFBackend( + model_id="ibm-granite/granite-4.1-3b", + custom_config=(mock_tok, MagicMock(vocab_size=32000), torch.device("cpu")), + ) + backend._tokenizer = tokenizer # type: ignore[assignment] + return backend + + +class TestRepairAloraInstruction: + def test_repair_returns_repaired_copy_and_warns(self, caplog): + backend = _stub_backend(_tokenizer()) + config = dict(_BROKEN_CONFIG) + with caplog.at_level(logging.WARNING, logger="mellea"): + result = backend._repair_alora_instruction( + config, _invocation_no_colon(), "requirement-check_alora" + ) + assert result["instruction"] == _REPAIRED_INSTRUCTION + assert result is not config + assert config == _BROKEN_CONFIG, "caller's dict must not be mutated" + assert any( + "'requirement-check_alora'" in r.message and "#1679" in r.message + for r in caplog.records + ) + + def test_healthy_config_returned_unchanged(self, caplog): + backend = _stub_backend(_tokenizer()) + config = {"instruction": _REPAIRED_INSTRUCTION} + with caplog.at_level(logging.WARNING, logger="mellea"): + result = backend._repair_alora_instruction( + config, _invocation_no_colon(), "requirement-check_alora" + ) + assert result is config + assert not caplog.records + + @pytest.mark.parametrize("tokens", [None, []]) + def test_no_declared_invocation_returns_config_unchanged(self, tokens): + backend = _stub_backend(_tokenizer()) + config = dict(_BROKEN_CONFIG) + assert backend._repair_alora_instruction(config, tokens, "x_lora") is config + + def test_no_instruction_returns_config_unchanged(self): + backend = _stub_backend(_tokenizer()) + config = {"parameters": {}} + assert ( + backend._repair_alora_instruction(config, _invocation_no_colon(), "x") + is config + ) + + +class TestRepairWiring: + """`add_adapter` must apply the repair on both registration paths: the + deprecated `IntrinsicAdapter` shim (weights load per generate call, after + the rewriter has read the config) and the composed `Adapter`.""" + + def test_intrinsic_adapter_shim_path_repairs_config(self, tmp_path): + tok = _tokenizer() + backend = _stub_backend(tok) + (tmp_path / "adapter_config.json").write_text( + json.dumps({"alora_invocation_tokens": _invocation_no_colon()}) + ) + caller_config = dict(_BROKEN_CONFIG) + with pytest.warns(DeprecationWarning): + adapter = IntrinsicAdapter( + "requirement-check", + adapter_type=AdapterType.ALORA, + config_dict=caller_config, + base_model_name=backend.base_model_name, + ) + adapter.get_local_hf_path = lambda base_model_name: str(tmp_path) # type: ignore[method-assign] + + backend.add_adapter(adapter) + + assert adapter.config["instruction"] == _REPAIRED_INSTRUCTION + assert caller_config == _BROKEN_CONFIG, "caller's dict must not be mutated" + _, config = backend._intrinsic_adapter_name_and_config(adapter) + assert config["instruction"] == _REPAIRED_INSTRUCTION + + def test_intrinsic_adapter_shim_lora_left_untouched(self, tmp_path): + backend = _stub_backend(_tokenizer()) + (tmp_path / "adapter_config.json").write_text(json.dumps({"r": 8})) + with pytest.warns(DeprecationWarning): + adapter = IntrinsicAdapter( + "requirement-check", + adapter_type=AdapterType.LORA, + config_dict=dict(_BROKEN_CONFIG), + base_model_name=backend.base_model_name, + ) + adapter.get_local_hf_path = lambda base_model_name: str(tmp_path) # type: ignore[method-assign] + + backend.add_adapter(adapter) + + assert adapter.config == _BROKEN_CONFIG + + def test_composed_adapter_path_repairs_config(self): + tok = _tokenizer() + backend = _stub_backend(tok) + key = "requirement-check_alora" + backend._model.peft_config = { + key: SimpleNamespace(alora_invocation_tokens=_invocation_no_colon()) + } + binding = LocalFileBinding( + name="requirement-check", + adapter_type=AdapterType.ALORA, + repo_id="fake/repo", + ) + binding.get_local_hf_path = lambda base_model_name: "/fake/path" # type: ignore[method-assign] + composed = Adapter( + identity=Identity( + name="requirement-check", + adapter_type="alora", + capability="requirement_check", + ), + io_contract=get_io_contract("requirement-check"), + weights=binding, + ) + with ( + patch( + "mellea.formatters.granite.intrinsics.obtain_io_yaml", + return_value="/fake/adapter.yaml", + ), + patch("builtins.open", mock_open(read_data="key: value")), + patch("yaml.safe_load", return_value=dict(_BROKEN_CONFIG)), + ): + backend.add_adapter(composed) + + assert ( + backend._composed_adapter_configs[key]["instruction"] + == _REPAIRED_INSTRUCTION + ) diff --git a/test/formatters/granite/base/test_base_alora_activation.py b/test/formatters/granite/base/test_base_alora_activation.py index 7f052697ab..d784f63521 100644 --- a/test/formatters/granite/base/test_base_alora_activation.py +++ b/test/formatters/granite/base/test_base_alora_activation.py @@ -18,6 +18,7 @@ """ # Standard +import logging from unittest.mock import MagicMock # Third Party @@ -28,20 +29,33 @@ "transformers", reason="transformers not installed — install mellea[hf]" ) peft = pytest.importorskip("peft", reason="peft not installed — install mellea[hf]") -# First Party import peft.tuners.lora.variants as peft_variants from peft import LoraConfig from transformers import LlamaConfig, LlamaForCausalLM +# First Party +import mellea.formatters.granite.base.util as granite_util from mellea.backends.huggingface import LocalHFBackend from mellea.formatters.granite.base.util import ( _alora_activation_context, generate_with_transformers, ) +from test.predicates import require_gpu INVOCATION = [1, 2, 3] +def _n_pre_hooks(model: LlamaForCausalLM) -> int: + return sum(len(getattr(m, "_forward_pre_hooks", {})) for m in model.modules()) + + +@pytest.fixture(autouse=True) +def _reset_missing_invocation_warnings(monkeypatch: pytest.MonkeyPatch) -> None: + # The missing-invocation WARNING is once per adapter name per process; + # isolate each test from the others' warnings. + monkeypatch.setattr(granite_util, "_ALORA_MISSING_INVOCATION_WARNED", set()) + + def _tiny_model(adapter_config: LoraConfig | None = None) -> LlamaForCausalLM: config = LlamaConfig( vocab_size=64, @@ -102,9 +116,12 @@ def __exit__(self, *exc): class TestAloraActivationContext: def test_no_offsets_reach_variant_without_context(self): - """The regression itself: bare model + aLoRA adapter, no context -> - the aLoRA variant is called but never receives alora_offsets, so it - masks to base-model behaviour.""" + """Canary on upstream PEFT behaviour, not the regression guard: bare + model + aLoRA adapter, no context -> the aLoRA variant is called but + never receives alora_offsets, so it masks to base-model behaviour. + Passes with or without the fix; if it starts failing, PEFT has begun + supplying offsets on the bare-model path and the context may be + redundant. The guard is `test_generate_with_transformers_uses_context`.""" model = _tiny_model(_alora_config()) input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) with _VariantSpy() as spy: @@ -125,82 +142,164 @@ def test_context_delivers_expected_offsets(self): assert spy.offsets_seen[0] == [5] assert all(o == [5] for o in spy.offsets_seen) - def test_context_with_missing_invocation_passes_none_offset(self): - """Invocation sequence absent -> offsets are [None] (peft semantics: - adapter inactive for that row), which the variant masks to base.""" + def test_missing_invocation_warns_once_and_registers_no_hooks(self, caplog): + """Invocation sequence absent -> the adapter cannot activate. The + context registers no hooks (the variant sees `alora_offsets=None`, + PEFT's base-model mask) and logs one WARNING naming the adapter, not + one per call.""" model = _tiny_model(_alora_config()) input_ids = torch.tensor([[10, 11, 12, 40, 50]]) + with caplog.at_level(logging.WARNING, logger="mellea"): + for _ in range(2): + with _VariantSpy() as spy: + with torch.no_grad(), _alora_activation_context(model, input_ids): + assert _n_pre_hooks(model) == 0 + model.generate(input_ids=input_ids, max_new_tokens=2) + assert spy.offsets_seen and all(o is None for o in spy.offsets_seen) + warned = [r for r in caplog.records if "invocation sequence" in r.message] + assert len(warned) == 1 + assert "'rc'" in warned[0].message + + def test_num_return_sequences_repeats_offsets_per_row(self): + """`generate()` expands the prompt to one row per returned sequence; + the offsets must be expanded to match, or the variant's mask no longer + lines up with the batch and indexing fails.""" + model = _tiny_model(_alora_config()) + input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) + kwargs = {"num_return_sequences": 2, "do_sample": True} with _VariantSpy() as spy: - with torch.no_grad(), _alora_activation_context(model, input_ids): - model.generate(input_ids=input_ids, max_new_tokens=2) - assert spy.offsets_seen and all(o == [None] for o in spy.offsets_seen) + with torch.no_grad(), _alora_activation_context(model, input_ids, kwargs): + model.generate(input_ids=input_ids, max_new_tokens=2, **kwargs) + assert spy.offsets_seen and all(o == [5, 5] for o in spy.offsets_seen) + + def test_beam_search_raises_clear_error(self): + """PEFT's own `PeftModel` path rejects beam search for aLoRA; so must + this one, rather than failing later with an opaque IndexError.""" + model = _tiny_model(_alora_config()) + input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) + with pytest.raises(ValueError, match="Beam search"): + with _alora_activation_context(model, input_ids, {"num_beams": 2}): + pass + assert _n_pre_hooks(model) == 0 def test_hooks_removed_after_context_exit(self): model = _tiny_model(_alora_config()) input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) with torch.no_grad(), _alora_activation_context(model, input_ids): - pass - n_hooks = sum( - len(getattr(m, "_forward_pre_hooks", {})) for m in model.modules() + assert _n_pre_hooks(model) > 0 + assert _n_pre_hooks(model) == 0, ( + "pre-forward hooks must not outlive the context" ) - assert n_hooks == 0, "pre-forward hooks must not outlive the context" def test_hooks_removed_on_error(self): model = _tiny_model(_alora_config()) input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) with pytest.raises(RuntimeError, match="boom"): with torch.no_grad(), _alora_activation_context(model, input_ids): + assert _n_pre_hooks(model) > 0 raise RuntimeError("boom") - n_hooks = sum( - len(getattr(m, "_forward_pre_hooks", {})) for m in model.modules() - ) - assert n_hooks == 0 + assert _n_pre_hooks(model) == 0 def test_no_adapter_is_noop(self): model = _tiny_model() input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) with torch.no_grad(), _alora_activation_context(model, input_ids): + assert _n_pre_hooks(model) == 0 model.generate(input_ids=input_ids, max_new_tokens=2) # must not raise def test_plain_lora_adapter_is_noop(self): model = _tiny_model(_lora_config()) input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) with torch.no_grad(), _alora_activation_context(model, input_ids): + assert _n_pre_hooks(model) == 0 model.generate(input_ids=input_ids, max_new_tokens=2) - n_hooks = sum( - len(getattr(m, "_forward_pre_hooks", {})) for m in model.modules() + + def test_empty_invocation_tokens_is_noop(self): + """An aLoRA config declaring `alora_invocation_tokens=[]` can never + activate; treat it like a plain adapter rather than handing PEFT an + empty sequence to search for.""" + model = _tiny_model( + LoraConfig( + r=4, + lora_alpha=8, + target_modules=["q_proj", "v_proj"], + task_type="CAUSAL_LM", + alora_invocation_tokens=[], + ) ) - assert n_hooks == 0 + input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) + with torch.no_grad(), _alora_activation_context(model, input_ids): + assert _n_pre_hooks(model) == 0 + model.generate(input_ids=input_ids, max_new_tokens=2) + + def test_only_no_adapter_loaded_error_is_tolerated(self): + """transformers raises `ValueError("No adapter loaded...")` from + `active_adapters()` on a model with no adapter; that means no-op. Any + other ValueError is a real fault and must propagate.""" + + class _Model: + peft_config = {"rc": object()} + + def __init__(self, exc: ValueError): + self._exc = exc + + def active_adapters(self) -> list[str]: + raise self._exc - def test_generate_with_transformers_uses_context(self): - """Wiring check: the production call path must deliver offsets to the - variant, not just the standalone context manager.""" - model = _tiny_model(_alora_config()) input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) + no_adapter = ValueError("No adapter loaded. Please load an adapter first.") + with _alora_activation_context(_Model(no_adapter), input_ids): + pass + with pytest.raises(ValueError, match="boom"): + with _alora_activation_context(_Model(ValueError("boom")), input_ids): + pass + + @staticmethod + def _generate_via_production_path(model, input_ids, **generate_kwargs): tokenizer = MagicMock() tokenizer.eos_token_id = 9999 # outside vocab: never in generated tokens tokenizer.decode.side_effect = lambda *a, **k: "x" tokenizer.batch_decode.side_effect = lambda seqs: ["x"] * len(seqs) + generate_with_transformers( + tokenizer, + model, + generate_input={ + "input_tokens": input_ids, + "max_new_tokens": 2, + "return_dict_in_generate": True, + **generate_kwargs, + }, + other_input={}, + ) + + def test_generate_with_transformers_uses_context(self): + """The regression guard: the production call path must deliver + offsets to the variant. Fails when the context wiring is removed.""" + model = _tiny_model(_alora_config()) + input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) with _VariantSpy() as spy: - generate_with_transformers( - tokenizer, - model, - generate_input={ - "input_tokens": input_ids, - "max_new_tokens": 2, - "do_sample": False, - "return_dict_in_generate": True, - }, - other_input={}, - ) + self._generate_via_production_path(model, input_ids, do_sample=False) assert spy.n_calls > 0 assert spy.offsets_seen[0] == [5] + def test_generate_with_transformers_passes_generation_kwargs(self): + """Wiring check: the production path hands its generate kwargs to the + context, so a multi-sequence request expands the offsets instead of + crashing inside the variant.""" + model = _tiny_model(_alora_config()) + input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) + with _VariantSpy() as spy: + self._generate_via_production_path( + model, input_ids, do_sample=True, num_return_sequences=2 + ) + assert spy.offsets_seen[0] == [5, 5] + @pytest.mark.huggingface @pytest.mark.e2e @pytest.mark.qualitative @pytest.mark.slow +@require_gpu(min_vram_gb=20) class TestAloraDifferentialEndToEnd: """The aLoRA weights must measurably change the score versus no adapter. @@ -213,12 +312,7 @@ class TestAloraDifferentialEndToEnd: scores measured 0.047 vs 0.999 in the #1679 diagnostic eval. """ - def test_requirement_check_adapter_moves_score(self, gh_run): - # Skip the expensive model download + inference on CI, following the - # gh_run pattern used by the other huggingface e2e tests in this suite. - if gh_run == 1: - pytest.xfail("Model download + inference not run on CI") - + def test_requirement_check_adapter_moves_score(self): from mellea.backends.adapters._core import Adapter, Identity, LocalFileBinding from mellea.backends.adapters.catalog import ( AdapterType, @@ -232,7 +326,7 @@ def test_requirement_check_adapter_moves_score(self, gh_run): # NOTE: the published requirement-check io.yaml instruction does not # tokenise to the adapter's declared invocation sequence (issue # #1679). LocalHFBackend repairs it at load time with a warning - # (`_repair_composed_alora_instruction`) until the publisher + # (`_repair_alora_instruction`) until the publisher # republishes a corrected file, so this test runs on the as-published # adapter with no local workaround. md = fetch_intrinsic_metadata("requirement-check") @@ -304,6 +398,7 @@ def off_generate(*a, **kw): @pytest.mark.e2e @pytest.mark.qualitative @pytest.mark.slow +@require_gpu(min_vram_gb=20) class TestUncertaintyAloraDifferentialEndToEnd: """Second aLoRA capability (uncertainty): confirms the activation fix is general, not requirement-check-specific. @@ -316,10 +411,7 @@ class TestUncertaintyAloraDifferentialEndToEnd: (wrong) — a wide differential either way. """ - def test_check_certainty_adapter_moves_score(self, gh_run): - if gh_run == 1: - pytest.xfail("Model download + inference not run on CI") - + def test_check_certainty_adapter_moves_score(self): from mellea.backends.adapters._core import Adapter, Identity, LocalFileBinding from mellea.backends.adapters.catalog import ( AdapterType, From 8ea404e471d19543c41063c91358fb3bddead1fb Mon Sep 17 00:00:00 2001 From: Nigel Jones Date: Mon, 28 Sep 2026 10:47:01 +0100 Subject: [PATCH 3/7] test(backends): trim redundant aLoRA activation and repair unit tests Drop unit tests that duplicate another test or pin a trivial early return, keeping every guard that catches a regression in the fix: - activation: remove the context-level num_return_sequences test (the production-path test covers the same expansion), merge the two hook teardown tests and the three no-op tests, and drop the ValueError narrowing test. - repair: remove the direct tests of the small helpers and of the method's early returns, which the repair-logic and add_adapter wiring tests already exercise; reuse test_huggingface_unit's _make_backend instead of a copy, and assert the repair warning in the shim wiring test. Assisted-by: Claude Code Signed-off-by: Nigel Jones --- test/backends/test_alora_invocation_repair.py | 104 ++---------------- .../base/test_base_alora_activation.py | 81 ++------------ 2 files changed, 23 insertions(+), 162 deletions(-) diff --git a/test/backends/test_alora_invocation_repair.py b/test/backends/test_alora_invocation_repair.py index b546b3820f..139d9a2707 100644 --- a/test/backends/test_alora_invocation_repair.py +++ b/test/backends/test_alora_invocation_repair.py @@ -20,7 +20,7 @@ import re from collections.abc import Sequence from types import SimpleNamespace -from unittest.mock import MagicMock, mock_open, patch +from unittest.mock import mock_open, patch # Third Party import pytest @@ -37,9 +37,9 @@ from mellea.backends.huggingface import ( LocalHFBackend, _alora_invocation_repair, - _read_alora_invocation_tokens, _token_sequence_present, ) +from test.backends.test_huggingface_unit import _make_backend _TOKEN_RE = re.compile(r">:|\S+|\s+") @@ -82,23 +82,6 @@ def _invocation_with_colon() -> list[int]: return _tokenizer().encode(":") -class TestTokenSequencePresent: - def test_present(self): - tok = _tokenizer() - assert _token_sequence_present( - tok, "a b", tok.encode("") - ) - - def test_absent_when_colon_merges(self): - tok = _tokenizer() - assert not _token_sequence_present( - tok, "a : b", tok.encode("") - ) - - def test_empty_ids(self): - assert not _token_sequence_present(_tokenizer(), "anything", []) - - class TestAloraInvocationRepair: def test_healthy_instruction_unchanged(self): tok = _tokenizer() @@ -156,92 +139,24 @@ def test_colon_present_but_removal_does_not_restore_returns_none(self): ) -class TestReadAloraInvocationTokens: - def test_reads_declared_tokens(self, tmp_path): - (tmp_path / "adapter_config.json").write_text( - json.dumps({"alora_invocation_tokens": [27, 71226, 29]}) - ) - assert _read_alora_invocation_tokens(str(tmp_path)) == [27, 71226, 29] - - def test_plain_lora_config_returns_none(self, tmp_path): - (tmp_path / "adapter_config.json").write_text(json.dumps({"r": 8})) - assert _read_alora_invocation_tokens(str(tmp_path)) is None - - def test_missing_config_file_returns_none(self, tmp_path): - assert _read_alora_invocation_tokens(str(tmp_path)) is None - - _BROKEN_CONFIG = {"instruction": ": {requirement}\nEvaluate."} _REPAIRED_INSTRUCTION = " {requirement}\nEvaluate." def _stub_backend(tokenizer: _MergingTokenizer) -> LocalHFBackend: - """A real `LocalHFBackend` over mock model/tokenizer (no download), with - the fake merging tokenizer swapped in for the repair to use.""" - mock_tok = MagicMock(eos_token_id=0, vocab_size=32000) - mock_tok._tokenizer = MagicMock() - mock_tok._tokenizer.get_vocab_size.return_value = 32000 - mock_tok.__len__ = MagicMock(return_value=32000) - with ( - patch("mellea.backends.huggingface.llguidance") as mock_llg, - patch("mellea.backends.huggingface.set_seed"), - ): - mock_llg.hf.from_tokenizer.return_value = MagicMock(vocab_size=32000) - backend = LocalHFBackend( - model_id="ibm-granite/granite-4.1-3b", - custom_config=(mock_tok, MagicMock(vocab_size=32000), torch.device("cpu")), - ) + """`_make_backend` (mock weights, no download) with the fake merging + tokenizer swapped in for the repair to use.""" + backend = _make_backend() backend._tokenizer = tokenizer # type: ignore[assignment] return backend -class TestRepairAloraInstruction: - def test_repair_returns_repaired_copy_and_warns(self, caplog): - backend = _stub_backend(_tokenizer()) - config = dict(_BROKEN_CONFIG) - with caplog.at_level(logging.WARNING, logger="mellea"): - result = backend._repair_alora_instruction( - config, _invocation_no_colon(), "requirement-check_alora" - ) - assert result["instruction"] == _REPAIRED_INSTRUCTION - assert result is not config - assert config == _BROKEN_CONFIG, "caller's dict must not be mutated" - assert any( - "'requirement-check_alora'" in r.message and "#1679" in r.message - for r in caplog.records - ) - - def test_healthy_config_returned_unchanged(self, caplog): - backend = _stub_backend(_tokenizer()) - config = {"instruction": _REPAIRED_INSTRUCTION} - with caplog.at_level(logging.WARNING, logger="mellea"): - result = backend._repair_alora_instruction( - config, _invocation_no_colon(), "requirement-check_alora" - ) - assert result is config - assert not caplog.records - - @pytest.mark.parametrize("tokens", [None, []]) - def test_no_declared_invocation_returns_config_unchanged(self, tokens): - backend = _stub_backend(_tokenizer()) - config = dict(_BROKEN_CONFIG) - assert backend._repair_alora_instruction(config, tokens, "x_lora") is config - - def test_no_instruction_returns_config_unchanged(self): - backend = _stub_backend(_tokenizer()) - config = {"parameters": {}} - assert ( - backend._repair_alora_instruction(config, _invocation_no_colon(), "x") - is config - ) - - class TestRepairWiring: """`add_adapter` must apply the repair on both registration paths: the deprecated `IntrinsicAdapter` shim (weights load per generate call, after the rewriter has read the config) and the composed `Adapter`.""" - def test_intrinsic_adapter_shim_path_repairs_config(self, tmp_path): + def test_intrinsic_adapter_shim_path_repairs_config(self, tmp_path, caplog): tok = _tokenizer() backend = _stub_backend(tok) (tmp_path / "adapter_config.json").write_text( @@ -257,10 +172,15 @@ def test_intrinsic_adapter_shim_path_repairs_config(self, tmp_path): ) adapter.get_local_hf_path = lambda base_model_name: str(tmp_path) # type: ignore[method-assign] - backend.add_adapter(adapter) + with caplog.at_level(logging.WARNING, logger="mellea"): + backend.add_adapter(adapter) assert adapter.config["instruction"] == _REPAIRED_INSTRUCTION assert caller_config == _BROKEN_CONFIG, "caller's dict must not be mutated" + assert any( + "'requirement-check_alora'" in r.message and "#1679" in r.message + for r in caplog.records + ) _, config = backend._intrinsic_adapter_name_and_config(adapter) assert config["instruction"] == _REPAIRED_INSTRUCTION diff --git a/test/formatters/granite/base/test_base_alora_activation.py b/test/formatters/granite/base/test_base_alora_activation.py index d784f63521..281132a7c9 100644 --- a/test/formatters/granite/base/test_base_alora_activation.py +++ b/test/formatters/granite/base/test_base_alora_activation.py @@ -73,13 +73,13 @@ def _tiny_model(adapter_config: LoraConfig | None = None) -> LlamaForCausalLM: return model -def _alora_config() -> LoraConfig: +def _alora_config(invocation: list[int] = INVOCATION) -> LoraConfig: return LoraConfig( r=4, lora_alpha=8, target_modules=["q_proj", "v_proj"], task_type="CAUSAL_LM", - alora_invocation_tokens=INVOCATION, + alora_invocation_tokens=invocation, ) @@ -160,18 +160,6 @@ def test_missing_invocation_warns_once_and_registers_no_hooks(self, caplog): assert len(warned) == 1 assert "'rc'" in warned[0].message - def test_num_return_sequences_repeats_offsets_per_row(self): - """`generate()` expands the prompt to one row per returned sequence; - the offsets must be expanded to match, or the variant's mask no longer - lines up with the batch and indexing fails.""" - model = _tiny_model(_alora_config()) - input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) - kwargs = {"num_return_sequences": 2, "do_sample": True} - with _VariantSpy() as spy: - with torch.no_grad(), _alora_activation_context(model, input_ids, kwargs): - model.generate(input_ids=input_ids, max_new_tokens=2, **kwargs) - assert spy.offsets_seen and all(o == [5, 5] for o in spy.offsets_seen) - def test_beam_search_raises_clear_error(self): """PEFT's own `PeftModel` path rejects beam search for aLoRA; so must this one, rather than failing later with an opaque IndexError.""" @@ -182,7 +170,7 @@ def test_beam_search_raises_clear_error(self): pass assert _n_pre_hooks(model) == 0 - def test_hooks_removed_after_context_exit(self): + def test_hooks_live_only_inside_context(self): model = _tiny_model(_alora_config()) input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) with torch.no_grad(), _alora_activation_context(model, input_ids): @@ -190,70 +178,23 @@ def test_hooks_removed_after_context_exit(self): assert _n_pre_hooks(model) == 0, ( "pre-forward hooks must not outlive the context" ) - - def test_hooks_removed_on_error(self): - model = _tiny_model(_alora_config()) - input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) with pytest.raises(RuntimeError, match="boom"): with torch.no_grad(), _alora_activation_context(model, input_ids): - assert _n_pre_hooks(model) > 0 raise RuntimeError("boom") - assert _n_pre_hooks(model) == 0 + assert _n_pre_hooks(model) == 0, "hooks must be removed on error too" - def test_no_adapter_is_noop(self): - model = _tiny_model() - input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) - with torch.no_grad(), _alora_activation_context(model, input_ids): - assert _n_pre_hooks(model) == 0 - model.generate(input_ids=input_ids, max_new_tokens=2) # must not raise - - def test_plain_lora_adapter_is_noop(self): - model = _tiny_model(_lora_config()) - input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) - with torch.no_grad(), _alora_activation_context(model, input_ids): - assert _n_pre_hooks(model) == 0 - model.generate(input_ids=input_ids, max_new_tokens=2) - - def test_empty_invocation_tokens_is_noop(self): - """An aLoRA config declaring `alora_invocation_tokens=[]` can never - activate; treat it like a plain adapter rather than handing PEFT an - empty sequence to search for.""" - model = _tiny_model( - LoraConfig( - r=4, - lora_alpha=8, - target_modules=["q_proj", "v_proj"], - task_type="CAUSAL_LM", - alora_invocation_tokens=[], - ) - ) + @pytest.mark.parametrize( + "adapter_config", + [None, _lora_config(), _alora_config(invocation=[])], + ids=["no-adapter", "plain-lora", "empty-invocation"], + ) + def test_non_activating_model_is_noop(self, adapter_config): + model = _tiny_model(adapter_config) input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) with torch.no_grad(), _alora_activation_context(model, input_ids): assert _n_pre_hooks(model) == 0 model.generate(input_ids=input_ids, max_new_tokens=2) - def test_only_no_adapter_loaded_error_is_tolerated(self): - """transformers raises `ValueError("No adapter loaded...")` from - `active_adapters()` on a model with no adapter; that means no-op. Any - other ValueError is a real fault and must propagate.""" - - class _Model: - peft_config = {"rc": object()} - - def __init__(self, exc: ValueError): - self._exc = exc - - def active_adapters(self) -> list[str]: - raise self._exc - - input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) - no_adapter = ValueError("No adapter loaded. Please load an adapter first.") - with _alora_activation_context(_Model(no_adapter), input_ids): - pass - with pytest.raises(ValueError, match="boom"): - with _alora_activation_context(_Model(ValueError("boom")), input_ids): - pass - @staticmethod def _generate_via_production_path(model, input_ids, **generate_kwargs): tokenizer = MagicMock() From c63d22e906235e7233eca1ea1426985739f393a6 Mon Sep 17 00:00:00 2001 From: Nigel Jones Date: Tue, 29 Sep 2026 09:50:32 +0100 Subject: [PATCH 4/7] fix(formatters): honour generation_config when guarding aLoRA beam search The aLoRA activation context read num_beams and num_return_sequences only from flat generate() kwargs and the model default, so a caller passing generation_config=GenerationConfig(num_beams=...) through model_options bypassed the beam-search refusal and the per-row offset expansion, and hit an opaque IndexError in the variant layer. Resolve settings in generate()'s own order: flat kwarg, then a generation_config kwarg, then the model default (unset GenerationConfig fields are None, so each level falls through). Reported in review by @jakelorocco, whose tests are added as generation-config cases of the existing beam-search and multi-sequence tests. Assisted-by: Claude Code Signed-off-by: Nigel Jones --- mellea/formatters/granite/base/util.py | 9 ++++- .../base/test_base_alora_activation.py | 36 +++++++++++++------ 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/mellea/formatters/granite/base/util.py b/mellea/formatters/granite/base/util.py index fe5df1747f..e1c704cb4e 100644 --- a/mellea/formatters/granite/base/util.py +++ b/mellea/formatters/granite/base/util.py @@ -388,8 +388,15 @@ def chat_completion_request_to_transformers_inputs( def _generation_setting( model: PreTrainedModel, generate_kwargs: Mapping[str, Any], name: str ) -> int: - """Effective integer `generate()` setting: call kwarg, else model default, else 1.""" + """Effective integer `generate()` setting, resolved in `generate()`'s order. + + A flat kwarg wins, then a `generation_config=` kwarg, then the model's own + `generation_config`; 1 if none sets it. Unset `GenerationConfig` fields + are `None`, so each level falls through to the next. + """ value = generate_kwargs.get(name) + if value is None: + value = getattr(generate_kwargs.get("generation_config"), name, None) if value is None: value = getattr(getattr(model, "generation_config", None), name, None) return value if isinstance(value, int) else 1 diff --git a/test/formatters/granite/base/test_base_alora_activation.py b/test/formatters/granite/base/test_base_alora_activation.py index 281132a7c9..995bc8f6e7 100644 --- a/test/formatters/granite/base/test_base_alora_activation.py +++ b/test/formatters/granite/base/test_base_alora_activation.py @@ -31,7 +31,7 @@ peft = pytest.importorskip("peft", reason="peft not installed — install mellea[hf]") import peft.tuners.lora.variants as peft_variants from peft import LoraConfig -from transformers import LlamaConfig, LlamaForCausalLM +from transformers import GenerationConfig, LlamaConfig, LlamaForCausalLM # First Party import mellea.formatters.granite.base.util as granite_util @@ -160,13 +160,19 @@ def test_missing_invocation_warns_once_and_registers_no_hooks(self, caplog): assert len(warned) == 1 assert "'rc'" in warned[0].message - def test_beam_search_raises_clear_error(self): + @pytest.mark.parametrize( + "generate_kwargs", + [{"num_beams": 2}, {"generation_config": GenerationConfig(num_beams=2)}], + ids=["kwarg", "generation-config"], + ) + def test_beam_search_raises_clear_error(self, generate_kwargs): """PEFT's own `PeftModel` path rejects beam search for aLoRA; so must - this one, rather than failing later with an opaque IndexError.""" + this one, however the setting is passed, rather than failing later + with an opaque IndexError.""" model = _tiny_model(_alora_config()) input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) with pytest.raises(ValueError, match="Beam search"): - with _alora_activation_context(model, input_ids, {"num_beams": 2}): + with _alora_activation_context(model, input_ids, generate_kwargs): pass assert _n_pre_hooks(model) == 0 @@ -223,16 +229,26 @@ def test_generate_with_transformers_uses_context(self): assert spy.n_calls > 0 assert spy.offsets_seen[0] == [5] - def test_generate_with_transformers_passes_generation_kwargs(self): + @pytest.mark.parametrize( + "generate_kwargs", + [ + {"do_sample": True, "num_return_sequences": 2}, + { + "generation_config": GenerationConfig( + do_sample=True, num_return_sequences=2 + ) + }, + ], + ids=["kwarg", "generation-config"], + ) + def test_generate_with_transformers_passes_generation_kwargs(self, generate_kwargs): """Wiring check: the production path hands its generate kwargs to the - context, so a multi-sequence request expands the offsets instead of - crashing inside the variant.""" + context, so a multi-sequence request (flat kwarg or `generation_config` + object) expands the offsets instead of crashing inside the variant.""" model = _tiny_model(_alora_config()) input_ids = torch.tensor([[10, 1, 2, 3, 40, 50]]) with _VariantSpy() as spy: - self._generate_via_production_path( - model, input_ids, do_sample=True, num_return_sequences=2 - ) + self._generate_via_production_path(model, input_ids, **generate_kwargs) assert spy.offsets_seen[0] == [5, 5] From b36de71d887e4fb4246a610d6a951240cf981173 Mon Sep 17 00:00:00 2001 From: Nigel Jones Date: Tue, 29 Sep 2026 09:50:38 +0100 Subject: [PATCH 5/7] refactor(adapters): move the aLoRA io.yaml repair helpers into their own module _token_sequence_present, _read_alora_invocation_tokens and _alora_invocation_repair have no dependency on the backend, so move them from huggingface.py into mellea/backends/adapters/_alora_repair.py. The module docstring records that it is the removable #1679 workaround, so retiring it is a one-file delete plus LocalHFBackend._repair_alora_instruction and its two call sites. Suggested in review by @jakelorocco. Assisted-by: Claude Code Signed-off-by: Nigel Jones --- mellea/backends/adapters/_alora_repair.py | 110 ++++++++++++++++++ mellea/backends/huggingface.py | 92 +-------------- test/backends/test_alora_invocation_repair.py | 8 +- 3 files changed, 118 insertions(+), 92 deletions(-) create mode 100644 mellea/backends/adapters/_alora_repair.py diff --git a/mellea/backends/adapters/_alora_repair.py b/mellea/backends/adapters/_alora_repair.py new file mode 100644 index 0000000000..9cd734ea8c --- /dev/null +++ b/mellea/backends/adapters/_alora_repair.py @@ -0,0 +1,110 @@ +# Copyright IBM Corp. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Repair for published aLoRA io.yaml files that cannot activate their adapter. + +A removable workaround for issue #1679 (the published `requirement-check` +aLoRA, all granite-4.1 slots): its io.yaml instruction does not tokenise to the +adapter's declared `alora_invocation_tokens`. `LocalHFBackend.add_adapter` +applies it through `LocalHFBackend._repair_alora_instruction`. Once the +publisher republishes the adapter and the catalogue pin is bumped, delete this +module, that method, and its two call sites in `add_adapter`. +""" + +from __future__ import annotations + +import json +import pathlib +from collections.abc import Sequence +from typing import TYPE_CHECKING, cast + +if TYPE_CHECKING: + from transformers.tokenization_utils_base import PreTrainedTokenizerBase + + +def _token_sequence_present( + tokenizer: PreTrainedTokenizerBase, text: str, token_ids: Sequence[int] +) -> bool: + """Whether `token_ids` occurs as a contiguous run in `tokenizer.encode(text)`. + + Args: + tokenizer: The tokenizer to encode with. + text: The text to search. + token_ids: The token run to look for. + + Returns: + True if the run occurs at least once in the encoded text. + """ + if not token_ids: + return False + tokens = tokenizer.encode(text, add_special_tokens=False) + seq = list(token_ids) + n = len(seq) + return any(tokens[i : i + n] == seq for i in range(len(tokens) - n + 1)) + + +def _read_alora_invocation_tokens(adapter_dir: str) -> list[int] | None: + """Read the declared `alora_invocation_tokens` from a downloaded adapter. + + Args: + adapter_dir: Local directory holding the adapter's + `adapter_config.json`. + + Returns: + The declared invocation token ids, or `None` when the directory has + no `adapter_config.json` or the config declares none (a plain LoRA). + """ + config_path = pathlib.Path(adapter_dir) / "adapter_config.json" + if not config_path.is_file(): + return None + tokens = json.loads(config_path.read_text(encoding="utf-8")).get( + "alora_invocation_tokens" + ) + return list(tokens) if tokens else None + + +def _alora_invocation_repair( + tokenizer: PreTrainedTokenizerBase, + instruction: str, + invocation_tokens: Sequence[int], +) -> str | None: + """Repair an aLoRA io.yaml instruction that cannot activate its own adapter. + + An aLoRA adapter activates only when its declared `alora_invocation_tokens` + occur, after tokenisation, in the assembled prompt. Some published + adapters (issue #1679: `requirement-check`, all granite-4.1 slots) ship an + instruction whose text does not tokenise to the declared sequence, because + the Granite tokeniser merges `>` with a following `:` into one token, so + the instruction's `:` never yields the declared + `` run. + + The repair is deliberately verification-driven and self-terminating: it + returns a changed instruction only when the declared sequence is absent + from the tokenised instruction, the decoded invocation text followed by a + single colon is present in it, and dropping that colon makes the declared + sequence present. A correctly republished file passes the first check and + is left untouched, in either direction a publisher might fix it (the + instruction text changed, or the declared tokens changed to match the + existing text). + + Args: + tokenizer: The base model's tokenizer. + instruction: The io.yaml `instruction` template text. + invocation_tokens: The adapter's declared `alora_invocation_tokens`. + + Returns: + The repaired instruction, or `None` when the instruction is already + consistent with the declared sequence or no repair is possible. + """ + if _token_sequence_present(tokenizer, instruction, invocation_tokens): + return None + invocation_text = cast( + str, tokenizer.decode(list(invocation_tokens), skip_special_tokens=False) + ) + broken = invocation_text + ":" + if broken not in instruction: + return None + repaired = instruction.replace(broken, invocation_text, 1) + if _token_sequence_present(tokenizer, repaired, invocation_tokens): + return repaired + return None diff --git a/mellea/backends/huggingface.py b/mellea/backends/huggingface.py index 82c76fa7aa..e4334a76ed 100644 --- a/mellea/backends/huggingface.py +++ b/mellea/backends/huggingface.py @@ -97,6 +97,10 @@ IntrinsicAdapter, LocalHFAdapter, ) +from .adapters._alora_repair import ( + _alora_invocation_repair, + _read_alora_invocation_tokens, +) from .adapters._core import ( Adapter as _AdapterCore, IOContract, @@ -3530,91 +3534,3 @@ def deactivate(self) -> None: def release(self) -> None: return - - -def _token_sequence_present( - tokenizer: PreTrainedTokenizerBase, text: str, token_ids: Sequence[int] -) -> bool: - """Whether `token_ids` occurs as a contiguous run in `tokenizer.encode(text)`. - - Args: - tokenizer: The tokenizer to encode with. - text: The text to search. - token_ids: The token run to look for. - - Returns: - True if the run occurs at least once in the encoded text. - """ - if not token_ids: - return False - tokens = tokenizer.encode(text, add_special_tokens=False) - seq = list(token_ids) - n = len(seq) - return any(tokens[i : i + n] == seq for i in range(len(tokens) - n + 1)) - - -def _read_alora_invocation_tokens(adapter_dir: str) -> list[int] | None: - """Read the declared `alora_invocation_tokens` from a downloaded adapter. - - Args: - adapter_dir: Local directory holding the adapter's - `adapter_config.json`. - - Returns: - The declared invocation token ids, or `None` when the directory has - no `adapter_config.json` or the config declares none (a plain LoRA). - """ - config_path = pathlib.Path(adapter_dir) / "adapter_config.json" - if not config_path.is_file(): - return None - tokens = json.loads(config_path.read_text(encoding="utf-8")).get( - "alora_invocation_tokens" - ) - return list(tokens) if tokens else None - - -def _alora_invocation_repair( - tokenizer: PreTrainedTokenizerBase, - instruction: str, - invocation_tokens: Sequence[int], -) -> str | None: - """Repair an aLoRA io.yaml instruction that cannot activate its own adapter. - - An aLoRA adapter activates only when its declared `alora_invocation_tokens` - occur, after tokenisation, in the assembled prompt. Some published - adapters (issue #1679: `requirement-check`, all granite-4.1 slots) ship an - instruction whose text does not tokenise to the declared sequence, because - the Granite tokeniser merges `>` with a following `:` into one token, so - the instruction's `:` never yields the declared - `` run. - - The repair is deliberately verification-driven and self-terminating: it - returns a changed instruction only when the declared sequence is absent - from the tokenised instruction, the decoded invocation text followed by a - single colon is present in it, and dropping that colon makes the declared - sequence present. A correctly republished file passes the first check and - is left untouched, in either direction a publisher might fix it (the - instruction text changed, or the declared tokens changed to match the - existing text). - - Args: - tokenizer: The base model's tokenizer. - instruction: The io.yaml `instruction` template text. - invocation_tokens: The adapter's declared `alora_invocation_tokens`. - - Returns: - The repaired instruction, or `None` when the instruction is already - consistent with the declared sequence or no repair is possible. - """ - if _token_sequence_present(tokenizer, instruction, invocation_tokens): - return None - invocation_text = cast( - str, tokenizer.decode(list(invocation_tokens), skip_special_tokens=False) - ) - broken = invocation_text + ":" - if broken not in instruction: - return None - repaired = instruction.replace(broken, invocation_text, 1) - if _token_sequence_present(tokenizer, repaired, invocation_tokens): - return repaired - return None diff --git a/test/backends/test_alora_invocation_repair.py b/test/backends/test_alora_invocation_repair.py index 139d9a2707..5a76c3eb7c 100644 --- a/test/backends/test_alora_invocation_repair.py +++ b/test/backends/test_alora_invocation_repair.py @@ -32,13 +32,13 @@ # First Party from mellea.backends.adapters import AdapterType, IntrinsicAdapter -from mellea.backends.adapters._core import Adapter, Identity, LocalFileBinding -from mellea.backends.adapters.io_contracts import get_io_contract -from mellea.backends.huggingface import ( - LocalHFBackend, +from mellea.backends.adapters._alora_repair import ( _alora_invocation_repair, _token_sequence_present, ) +from mellea.backends.adapters._core import Adapter, Identity, LocalFileBinding +from mellea.backends.adapters.io_contracts import get_io_contract +from mellea.backends.huggingface import LocalHFBackend from test.backends.test_huggingface_unit import _make_backend _TOKEN_RE = re.compile(r">:|\S+|\s+") From 9afa0b8b0521d3b8a7ac95c336d27d9859cfb75b Mon Sep 17 00:00:00 2001 From: Nigel Jones Date: Tue, 29 Sep 2026 10:58:54 +0100 Subject: [PATCH 6/7] fix(backends): run the composed aLoRA repair before prepare() and tighten tests Second-round review follow-ups for the #1679 aLoRA work. - Apply the composed-Adapter io.yaml repair before binding.prepare(), reading the declared invocation sequence from the downloaded adapter_config.json as the IntrinsicAdapter path already does. The post-prepare commit block goes back to its two dict assignments, so a failure in the repair leaves nothing registered. - Document the ValueError that generate_with_transformers can now raise for beam search with an active aLoRA, and list the attributes the logits capture proxy must now forward. - Make the fake tokenizer reproduce the real three-token invocation ([<, requirements, >]) and the >: merge, so the multi-token match is exercised without a model download. - Move the repair tests to test/backends/test_adapters/test_alora_repair.py to mirror the new module, and build the e2e backends through a fixture that uses hf_skip() and cleanup_gpu_backend(). Assisted-by: Claude Code Signed-off-by: Nigel Jones --- mellea/backends/huggingface.py | 31 ++++--- mellea/formatters/granite/base/util.py | 5 ++ .../test_alora_repair.py} | 28 +++--- .../base/test_base_alora_activation.py | 90 +++++++++---------- 4 files changed, 80 insertions(+), 74 deletions(-) rename test/backends/{test_alora_invocation_repair.py => test_adapters/test_alora_repair.py} (89%) diff --git a/mellea/backends/huggingface.py b/mellea/backends/huggingface.py index e4334a76ed..a4653c527f 100644 --- a/mellea/backends/huggingface.py +++ b/mellea/backends/huggingface.py @@ -1086,12 +1086,12 @@ def _repair_alora_instruction( `_alora_invocation_repair`): a healthy file -- in either direction a publisher might fix it -- is returned untouched. - Called from both `add_adapter` registration paths: the composed-adapter - commit point, with the declared sequence read from the PEFT config - `binding.prepare()` loaded, and the `IntrinsicAdapter` shim path, with - it read from the downloaded `adapter_config.json` (that shim loads its - weights per generate call, after its config has already been rendered - into the prompt). + Called from both `add_adapter` registration paths, with the declared + sequence read from the downloaded `adapter_config.json`: the + composed-adapter path before `binding.prepare()`, so a failure leaves + nothing registered, and the `IntrinsicAdapter` shim path at + registration, because that shim loads its weights per generate call, + after its config has already been rendered into the prompt. Args: io_yaml_config: The adapter's `io.yaml` mapping. Never mutated: a @@ -1343,7 +1343,8 @@ async def _generate_from_intrinsic( # Two load-bearing assumptions: (1) __getattr__ falls through for attributes # accessed by generate_with_transformers (model.device, model.vocab_size, - # model.generation_config). (2) chat_completion_request_to_transformers_inputs + # model.generation_config, and for aLoRA activation model.peft_config, + # model.active_adapters, model.modules()). (2) chat_completion_request_to_transformers_inputs # always sets return_dict_in_generate=True, so .generate() always returns a # GenerateDecoderOnlyOutput — if that ever changes, the cell stays None and # logits silently won't be populated. @@ -2984,6 +2985,16 @@ def add_adapter(self, adapter: AdapterInput, *, config: dict | None = None) -> N # this ordering only has to cover the fetch this method adds # ahead of it. io_yaml_config = self._obtain_local_file_io_yaml_config(binding) + # Before prepare() for the same reason: the declared invocation + # sequence is read from the downloaded adapter_config.json, so a + # failure here also leaves nothing registered. + io_yaml_config = self._repair_alora_instruction( + io_yaml_config, + _read_alora_invocation_tokens( + binding.get_local_hf_path(self.base_model_name) + ), + key, + ) # bind_backend() itself raises if binding.backend is already # a *different* backend, and no-ops if it's already self — # calling it unconditionally (rather than guarding on @@ -3014,12 +3025,6 @@ def add_adapter(self, adapter: AdapterInput, *, config: dict | None = None) -> N # commit, releasing (terminal, weights unloaded) the exact # binding this is about to publish as registered. with binding.hold_prepared(self), self._adapter_activation_lock(): - peft_config = self._model.peft_config.get(key) - io_yaml_config = self._repair_alora_instruction( - io_yaml_config, - getattr(peft_config, "alora_invocation_tokens", None), - key, - ) self._composed_adapter_configs[key] = io_yaml_config self._composed_adapters[key] = adapter return diff --git a/mellea/formatters/granite/base/util.py b/mellea/formatters/granite/base/util.py index e1c704cb4e..d7e6264710 100644 --- a/mellea/formatters/granite/base/util.py +++ b/mellea/formatters/granite/base/util.py @@ -559,6 +559,11 @@ def generate_with_transformers( Returns: A chat completion response in OpenAI format. + + Raises: + ValueError: Beam search (`num_beams > 1`) was requested while an aLoRA + adapter is active and its invocation sequence is in the prompt; + PEFT does not support that combination. """ with import_optional("torch"): # Third Party diff --git a/test/backends/test_alora_invocation_repair.py b/test/backends/test_adapters/test_alora_repair.py similarity index 89% rename from test/backends/test_alora_invocation_repair.py rename to test/backends/test_adapters/test_alora_repair.py index 5a76c3eb7c..c91ea9efb9 100644 --- a/test/backends/test_alora_invocation_repair.py +++ b/test/backends/test_adapters/test_alora_repair.py @@ -10,8 +10,10 @@ sequence is `` (no colon) and the Granite tokeniser merges `>:` into a single token, so the sequence can never appear. -The fake tokenizer below mimics exactly that BPE quirk at word granularity: -`>:` is one token, a standalone `>` is a different one. +The fake tokenizer below reproduces the real token shape: `<`, `requirements` +and `>` are separate tokens (the declared `[27, 71226, 29]` run), and `>` +merges with a following `:` (or `=`) into one token, so the broken text +shares the first two tokens of the run and differs only at the last. """ # Standard @@ -19,7 +21,6 @@ import logging import re from collections.abc import Sequence -from types import SimpleNamespace from unittest.mock import mock_open, patch # Third Party @@ -41,11 +42,11 @@ from mellea.backends.huggingface import LocalHFBackend from test.backends.test_huggingface_unit import _make_backend -_TOKEN_RE = re.compile(r">:|\S+|\s+") +_TOKEN_RE = re.compile(r"<|>:|>=|>|[^\s<>]+|\s+") class _MergingTokenizer: - """Word-level tokenizer where `>:` merges into one token, like the Granite BPE quirk.""" + """Tokenizer that splits `<`, `>` and words, merging `>:` and `>=` like Granite's BPE.""" def __init__(self, seed_text: str): self._ids: dict[str, int] = {} @@ -130,10 +131,11 @@ def test_unrepairable_mismatch_returns_none(self): def test_colon_present_but_removal_does_not_restore_returns_none(self): """The colon form occurs, but dropping it still does not yield the - declared run (here `:x` -> `x`, a different - token): the third check refuses rather than rewrite blindly.""" + declared run (here `:=` -> `=`, where `>=` + merges into one token): the third check refuses rather than rewrite + blindly.""" tok = _tokenizer() - instruction = ":x {requirement}\nEvaluate." + instruction = ":= {requirement}\nEvaluate." assert ( _alora_invocation_repair(tok, instruction, _invocation_no_colon()) is None ) @@ -200,19 +202,19 @@ def test_intrinsic_adapter_shim_lora_left_untouched(self, tmp_path): assert adapter.config == _BROKEN_CONFIG - def test_composed_adapter_path_repairs_config(self): + def test_composed_adapter_path_repairs_config(self, tmp_path): tok = _tokenizer() backend = _stub_backend(tok) key = "requirement-check_alora" - backend._model.peft_config = { - key: SimpleNamespace(alora_invocation_tokens=_invocation_no_colon()) - } + (tmp_path / "adapter_config.json").write_text( + json.dumps({"alora_invocation_tokens": _invocation_no_colon()}) + ) binding = LocalFileBinding( name="requirement-check", adapter_type=AdapterType.ALORA, repo_id="fake/repo", ) - binding.get_local_hf_path = lambda base_model_name: "/fake/path" # type: ignore[method-assign] + binding.get_local_hf_path = lambda base_model_name: str(tmp_path) # type: ignore[method-assign] composed = Adapter( identity=Identity( name="requirement-check", diff --git a/test/formatters/granite/base/test_base_alora_activation.py b/test/formatters/granite/base/test_base_alora_activation.py index 995bc8f6e7..a4e7165c92 100644 --- a/test/formatters/granite/base/test_base_alora_activation.py +++ b/test/formatters/granite/base/test_base_alora_activation.py @@ -40,6 +40,7 @@ _alora_activation_context, generate_with_transformers, ) +from test.conftest import cleanup_gpu_backend, hf_skip from test.predicates import require_gpu INVOCATION = [1, 2, 3] @@ -252,6 +253,43 @@ def test_generate_with_transformers_passes_generation_kwargs(self, generate_kwar assert spy.offsets_seen[0] == [5, 5] +@pytest.fixture +def alora_backend(): + """Factory for a granite-4.1-3b `LocalHFBackend` with one catalogue aLoRA + registered as a composed `Adapter`. Hub errors skip the test, and every + backend built is freed afterwards.""" + from mellea.backends.adapters._core import Adapter, Identity, LocalFileBinding + from mellea.backends.adapters.catalog import AdapterType, fetch_intrinsic_metadata + from mellea.backends.adapters.io_contracts import get_io_contract + + backends: list[LocalHFBackend] = [] + + def _make(name: str, capability: str) -> LocalHFBackend: + md = fetch_intrinsic_metadata(name) + with hf_skip(): + backend = LocalHFBackend(model_id="ibm-granite/granite-4.1-3b") + backends.append(backend) + backend.add_adapter( + Adapter( + identity=Identity( + name=name, adapter_type="alora", capability=capability + ), + io_contract=get_io_contract(name), + weights=LocalFileBinding( + name=name, + adapter_type=AdapterType.ALORA, + repo_id=md.repo_id, + revision=md.revision, + ), + ) + ) + return backend + + yield _make + for backend in backends: + cleanup_gpu_backend(backend, "test_base_alora_activation") + + @pytest.mark.huggingface @pytest.mark.e2e @pytest.mark.qualitative @@ -269,13 +307,7 @@ class TestAloraDifferentialEndToEnd: scores measured 0.047 vs 0.999 in the #1679 diagnostic eval. """ - def test_requirement_check_adapter_moves_score(self): - from mellea.backends.adapters._core import Adapter, Identity, LocalFileBinding - from mellea.backends.adapters.catalog import ( - AdapterType, - fetch_intrinsic_metadata, - ) - from mellea.backends.adapters.io_contracts import get_io_contract + def test_requirement_check_adapter_moves_score(self, alora_backend): from mellea.stdlib.components import Message from mellea.stdlib.components.intrinsic import core from mellea.stdlib.context import ChatContext @@ -286,24 +318,7 @@ def test_requirement_check_adapter_moves_score(self): # (`_repair_alora_instruction`) until the publisher # republishes a corrected file, so this test runs on the as-published # adapter with no local workaround. - md = fetch_intrinsic_metadata("requirement-check") - backend = LocalHFBackend(model_id="ibm-granite/granite-4.1-3b") - backend.add_adapter( - Adapter( - identity=Identity( - name="requirement-check", - adapter_type="alora", - capability="requirement_check", - ), - io_contract=get_io_contract("requirement-check"), - weights=LocalFileBinding( - name="requirement-check", - adapter_type=AdapterType.ALORA, - repo_id=md.repo_id, - revision=md.revision, - ), - ) - ) + backend = alora_backend("requirement-check", "requirement_check") ctx = ( ChatContext() .add(Message("user", "Write one sentence about Cardiff.")) @@ -368,33 +383,12 @@ class TestUncertaintyAloraDifferentialEndToEnd: (wrong) — a wide differential either way. """ - def test_check_certainty_adapter_moves_score(self): - from mellea.backends.adapters._core import Adapter, Identity, LocalFileBinding - from mellea.backends.adapters.catalog import ( - AdapterType, - fetch_intrinsic_metadata, - ) - from mellea.backends.adapters.io_contracts import get_io_contract + def test_check_certainty_adapter_moves_score(self, alora_backend): from mellea.stdlib.components import Message from mellea.stdlib.components.intrinsic import core from mellea.stdlib.context import ChatContext - md = fetch_intrinsic_metadata("uncertainty") - backend = LocalHFBackend(model_id="ibm-granite/granite-4.1-3b") - backend.add_adapter( - Adapter( - identity=Identity( - name="uncertainty", adapter_type="alora", capability="uncertainty" - ), - io_contract=get_io_contract("uncertainty"), - weights=LocalFileBinding( - name="uncertainty", - adapter_type=AdapterType.ALORA, - repo_id=md.repo_id, - revision=md.revision, - ), - ) - ) + backend = alora_backend("uncertainty", "uncertainty") qualified = next(iter(backend._model.peft_config)) def score_with_adapter_off(user: str, assistant: str) -> float: From a1cdbb0935207c51e4372f03f327f7a2d1b298df Mon Sep 17 00:00:00 2001 From: Nigel Jones Date: Wed, 30 Sep 2026 08:59:42 +0100 Subject: [PATCH 7/7] refactor(backends): drop the aLoRA io.yaml repair workaround The published requirement-check aLoRA io.yaml is being corrected upstream, so remove the load-time instruction repair rather than carry it in Mellea. The activation fix is unaffected: an adapter whose invocation sequence is missing from the prompt stays off and logs a warning. Mark the requirement-check differential e2e test as a strict xfail until the adapter is republished and the catalogue pin is bumped. Assisted-by: Claude Code Signed-off-by: Nigel Jones --- mellea/backends/adapters/_alora_repair.py | 110 -------- mellea/backends/huggingface.py | 76 ------ .../test_adapters/test_alora_repair.py | 240 ------------------ .../base/test_base_alora_activation.py | 16 +- 4 files changed, 8 insertions(+), 434 deletions(-) delete mode 100644 mellea/backends/adapters/_alora_repair.py delete mode 100644 test/backends/test_adapters/test_alora_repair.py diff --git a/mellea/backends/adapters/_alora_repair.py b/mellea/backends/adapters/_alora_repair.py deleted file mode 100644 index 9cd734ea8c..0000000000 --- a/mellea/backends/adapters/_alora_repair.py +++ /dev/null @@ -1,110 +0,0 @@ -# Copyright IBM Corp. All Rights Reserved. -# SPDX-License-Identifier: Apache-2.0 - -"""Repair for published aLoRA io.yaml files that cannot activate their adapter. - -A removable workaround for issue #1679 (the published `requirement-check` -aLoRA, all granite-4.1 slots): its io.yaml instruction does not tokenise to the -adapter's declared `alora_invocation_tokens`. `LocalHFBackend.add_adapter` -applies it through `LocalHFBackend._repair_alora_instruction`. Once the -publisher republishes the adapter and the catalogue pin is bumped, delete this -module, that method, and its two call sites in `add_adapter`. -""" - -from __future__ import annotations - -import json -import pathlib -from collections.abc import Sequence -from typing import TYPE_CHECKING, cast - -if TYPE_CHECKING: - from transformers.tokenization_utils_base import PreTrainedTokenizerBase - - -def _token_sequence_present( - tokenizer: PreTrainedTokenizerBase, text: str, token_ids: Sequence[int] -) -> bool: - """Whether `token_ids` occurs as a contiguous run in `tokenizer.encode(text)`. - - Args: - tokenizer: The tokenizer to encode with. - text: The text to search. - token_ids: The token run to look for. - - Returns: - True if the run occurs at least once in the encoded text. - """ - if not token_ids: - return False - tokens = tokenizer.encode(text, add_special_tokens=False) - seq = list(token_ids) - n = len(seq) - return any(tokens[i : i + n] == seq for i in range(len(tokens) - n + 1)) - - -def _read_alora_invocation_tokens(adapter_dir: str) -> list[int] | None: - """Read the declared `alora_invocation_tokens` from a downloaded adapter. - - Args: - adapter_dir: Local directory holding the adapter's - `adapter_config.json`. - - Returns: - The declared invocation token ids, or `None` when the directory has - no `adapter_config.json` or the config declares none (a plain LoRA). - """ - config_path = pathlib.Path(adapter_dir) / "adapter_config.json" - if not config_path.is_file(): - return None - tokens = json.loads(config_path.read_text(encoding="utf-8")).get( - "alora_invocation_tokens" - ) - return list(tokens) if tokens else None - - -def _alora_invocation_repair( - tokenizer: PreTrainedTokenizerBase, - instruction: str, - invocation_tokens: Sequence[int], -) -> str | None: - """Repair an aLoRA io.yaml instruction that cannot activate its own adapter. - - An aLoRA adapter activates only when its declared `alora_invocation_tokens` - occur, after tokenisation, in the assembled prompt. Some published - adapters (issue #1679: `requirement-check`, all granite-4.1 slots) ship an - instruction whose text does not tokenise to the declared sequence, because - the Granite tokeniser merges `>` with a following `:` into one token, so - the instruction's `:` never yields the declared - `` run. - - The repair is deliberately verification-driven and self-terminating: it - returns a changed instruction only when the declared sequence is absent - from the tokenised instruction, the decoded invocation text followed by a - single colon is present in it, and dropping that colon makes the declared - sequence present. A correctly republished file passes the first check and - is left untouched, in either direction a publisher might fix it (the - instruction text changed, or the declared tokens changed to match the - existing text). - - Args: - tokenizer: The base model's tokenizer. - instruction: The io.yaml `instruction` template text. - invocation_tokens: The adapter's declared `alora_invocation_tokens`. - - Returns: - The repaired instruction, or `None` when the instruction is already - consistent with the declared sequence or no repair is possible. - """ - if _token_sequence_present(tokenizer, instruction, invocation_tokens): - return None - invocation_text = cast( - str, tokenizer.decode(list(invocation_tokens), skip_special_tokens=False) - ) - broken = invocation_text + ":" - if broken not in instruction: - return None - repaired = instruction.replace(broken, invocation_text, 1) - if _token_sequence_present(tokenizer, repaired, invocation_tokens): - return repaired - return None diff --git a/mellea/backends/huggingface.py b/mellea/backends/huggingface.py index a4653c527f..119f4e34b6 100644 --- a/mellea/backends/huggingface.py +++ b/mellea/backends/huggingface.py @@ -97,10 +97,6 @@ IntrinsicAdapter, LocalHFAdapter, ) -from .adapters._alora_repair import ( - _alora_invocation_repair, - _read_alora_invocation_tokens, -) from .adapters._core import ( Adapter as _AdapterCore, IOContract, @@ -1067,62 +1063,6 @@ def _obtain_local_file_io_yaml_config(self, binding: LocalFileBinding) -> dict: ) return loaded - def _repair_alora_instruction( - self, - io_yaml_config: dict, - invocation_tokens: Sequence[int] | None, - adapter_name: str, - ) -> dict: - """Repair an aLoRA io.yaml instruction that cannot activate its own adapter. - - Some published adapters (issue #1679: `requirement-check`, all - granite-4.1 slots) ship an instruction whose text does not tokenise to - the `alora_invocation_tokens` declared in the adapter's own config, - so the adapter can never activate no matter what the loading path - does. This loads a locally repaired instruction instead, with a loud - warning, so the capability works end to end until the publisher - republishes a corrected file. The repair itself is - verification-driven and self-terminating (see - `_alora_invocation_repair`): a healthy file -- in either direction a - publisher might fix it -- is returned untouched. - - Called from both `add_adapter` registration paths, with the declared - sequence read from the downloaded `adapter_config.json`: the - composed-adapter path before `binding.prepare()`, so a failure leaves - nothing registered, and the `IntrinsicAdapter` shim path at - registration, because that shim loads its weights per generate call, - after its config has already been rendered into the prompt. - - Args: - io_yaml_config: The adapter's `io.yaml` mapping. Never mutated: a - shim's config can be the caller's own `config_dict`. - invocation_tokens: The adapter's declared - `alora_invocation_tokens`, or `None` for a non-aLoRA adapter. - adapter_name: The adapter's qualified name, for the warning. - - Returns: - `io_yaml_config` itself when no repair applies, otherwise a shallow - copy carrying the repaired instruction. - """ - if not invocation_tokens: - return io_yaml_config - instruction = io_yaml_config.get("instruction") - if not isinstance(instruction, str) or not instruction: - return io_yaml_config - repaired = _alora_invocation_repair( - self._tokenizer, instruction, invocation_tokens - ) - if repaired is None: - return io_yaml_config - MelleaLogger.get_logger().warning( - f"Adapter {adapter_name!r}: the published io.yaml instruction does " - "not tokenise to the adapter's declared aLoRA invocation sequence, so " - "the adapter could never activate as published. Loaded a locally " - "repaired instruction instead (issue #1679). Ask the adapter " - "publisher to republish a corrected io.yaml." - ) - return {**io_yaml_config, "instruction": repaired} - async def _generate_from_intrinsic( self, action: Intrinsic, @@ -2985,16 +2925,6 @@ def add_adapter(self, adapter: AdapterInput, *, config: dict | None = None) -> N # this ordering only has to cover the fetch this method adds # ahead of it. io_yaml_config = self._obtain_local_file_io_yaml_config(binding) - # Before prepare() for the same reason: the declared invocation - # sequence is read from the downloaded adapter_config.json, so a - # failure here also leaves nothing registered. - io_yaml_config = self._repair_alora_instruction( - io_yaml_config, - _read_alora_invocation_tokens( - binding.get_local_hf_path(self.base_model_name) - ), - key, - ) # bind_backend() itself raises if binding.backend is already # a *different* backend, and no-ops if it's already self — # calling it unconditionally (rather than guarding on @@ -3066,12 +2996,6 @@ def add_adapter(self, adapter: AdapterInput, *, config: dict | None = None) -> N return adapter.path = adapter.get_local_hf_path(self.base_model_name) - if isinstance(adapter, IntrinsicAdapter): - adapter.config = self._repair_alora_instruction( - adapter.config, - _read_alora_invocation_tokens(adapter.path), - adapter.qualified_name, - ) adapter.backend = self self._added_adapters[adapter.qualified_name] = adapter diff --git a/test/backends/test_adapters/test_alora_repair.py b/test/backends/test_adapters/test_alora_repair.py deleted file mode 100644 index c91ea9efb9..0000000000 --- a/test/backends/test_adapters/test_alora_repair.py +++ /dev/null @@ -1,240 +0,0 @@ -# Copyright IBM Corp. All Rights Reserved. -# SPDX-License-Identifier: Apache-2.0 - -"""Unit tests for the aLoRA io.yaml invocation-sequence repair (issue #1679). - -The repair under test (`_alora_invocation_repair`) handles published adapters -whose io.yaml instruction text does not tokenise to the invocation sequence -declared in the adapter's own config — specifically the `requirement-check` -aLoRA, where the instruction starts `:` but the declared -sequence is `` (no colon) and the Granite tokeniser merges `>:` -into a single token, so the sequence can never appear. - -The fake tokenizer below reproduces the real token shape: `<`, `requirements` -and `>` are separate tokens (the declared `[27, 71226, 29]` run), and `>` -merges with a following `:` (or `=`) into one token, so the broken text -shares the first two tokens of the run and differs only at the last. -""" - -# Standard -import json -import logging -import re -from collections.abc import Sequence -from unittest.mock import mock_open, patch - -# Third Party -import pytest - -pytest.importorskip( - "transformers", reason="transformers not installed — install mellea[hf]" -) -torch = pytest.importorskip("torch", reason="torch not installed — install mellea[hf]") - -# First Party -from mellea.backends.adapters import AdapterType, IntrinsicAdapter -from mellea.backends.adapters._alora_repair import ( - _alora_invocation_repair, - _token_sequence_present, -) -from mellea.backends.adapters._core import Adapter, Identity, LocalFileBinding -from mellea.backends.adapters.io_contracts import get_io_contract -from mellea.backends.huggingface import LocalHFBackend -from test.backends.test_huggingface_unit import _make_backend - -_TOKEN_RE = re.compile(r"<|>:|>=|>|[^\s<>]+|\s+") - - -class _MergingTokenizer: - """Tokenizer that splits `<`, `>` and words, merging `>:` and `>=` like Granite's BPE.""" - - def __init__(self, seed_text: str): - self._ids: dict[str, int] = {} - self._by_id: dict[int, str] = {} - for m in _TOKEN_RE.finditer(seed_text): - self._register(m.group()) - - def _register(self, token: str) -> None: - if token not in self._ids: - self._ids[token] = len(self._ids) + 1 - self._by_id[self._ids[token]] = token - - def encode(self, text: str, add_special_tokens: bool = True) -> list[int]: - out = [] - for m in _TOKEN_RE.finditer(text): - self._register(m.group()) - out.append(self._ids[m.group()]) - return out - - def decode(self, ids: Sequence[int], skip_special_tokens: bool = True) -> str: - return "".join(self._by_id[i] for i in ids) - - -def _tokenizer() -> _MergingTokenizer: - # Seed so both "" and ":" are encodable. - return _MergingTokenizer(" : x y") - - -def _invocation_no_colon() -> list[int]: - return _tokenizer().encode("") - - -def _invocation_with_colon() -> list[int]: - return _tokenizer().encode(":") - - -class TestAloraInvocationRepair: - def test_healthy_instruction_unchanged(self): - tok = _tokenizer() - instruction = " {requirement}\nEvaluate." - assert ( - _alora_invocation_repair(tok, instruction, _invocation_no_colon()) is None - ) - - def test_repairs_colon_mismatch(self): - tok = _tokenizer() - instruction = ": {requirement}\nEvaluate." - repaired = _alora_invocation_repair(tok, instruction, _invocation_no_colon()) - assert repaired == " {requirement}\nEvaluate." - assert _token_sequence_present(tok, repaired, _invocation_no_colon()) - - def test_self_terminates_when_publisher_fixes_tokens_instead(self): - """If the publisher re-declares the invocation tokens to match the - existing `:` text (the other valid fix direction), the - file is healthy and must be left untouched.""" - tok = _tokenizer() - instruction = ": {requirement}\nEvaluate." - assert ( - _alora_invocation_repair(tok, instruction, _invocation_with_colon()) is None - ) - - def test_invocation_not_from_instruction_is_untouched(self): - """Adapters whose invocation sequence is supplied by the chat template - (e.g. role markers) never match instruction text; no repair applies.""" - tok = _tokenizer() - instruction = ": {requirement}\nEvaluate." - assert ( - _alora_invocation_repair(tok, instruction, tok.encode("<|start_of_role|>")) - is None - ) - - def test_unrepairable_mismatch_returns_none(self): - """Declared sequence is `:` (with colon) but the text - only ever carries `` with the colon elsewhere: the - `broken`-form (`:` + `:`) appears nowhere, so no repair - applies and the text is left untouched for an upstream fix.""" - tok = _tokenizer() - instruction = "x y: {requirement}\nEvaluate." - assert ( - _alora_invocation_repair(tok, instruction, _invocation_with_colon()) is None - ) - - def test_colon_present_but_removal_does_not_restore_returns_none(self): - """The colon form occurs, but dropping it still does not yield the - declared run (here `:=` -> `=`, where `>=` - merges into one token): the third check refuses rather than rewrite - blindly.""" - tok = _tokenizer() - instruction = ":= {requirement}\nEvaluate." - assert ( - _alora_invocation_repair(tok, instruction, _invocation_no_colon()) is None - ) - - -_BROKEN_CONFIG = {"instruction": ": {requirement}\nEvaluate."} -_REPAIRED_INSTRUCTION = " {requirement}\nEvaluate." - - -def _stub_backend(tokenizer: _MergingTokenizer) -> LocalHFBackend: - """`_make_backend` (mock weights, no download) with the fake merging - tokenizer swapped in for the repair to use.""" - backend = _make_backend() - backend._tokenizer = tokenizer # type: ignore[assignment] - return backend - - -class TestRepairWiring: - """`add_adapter` must apply the repair on both registration paths: the - deprecated `IntrinsicAdapter` shim (weights load per generate call, after - the rewriter has read the config) and the composed `Adapter`.""" - - def test_intrinsic_adapter_shim_path_repairs_config(self, tmp_path, caplog): - tok = _tokenizer() - backend = _stub_backend(tok) - (tmp_path / "adapter_config.json").write_text( - json.dumps({"alora_invocation_tokens": _invocation_no_colon()}) - ) - caller_config = dict(_BROKEN_CONFIG) - with pytest.warns(DeprecationWarning): - adapter = IntrinsicAdapter( - "requirement-check", - adapter_type=AdapterType.ALORA, - config_dict=caller_config, - base_model_name=backend.base_model_name, - ) - adapter.get_local_hf_path = lambda base_model_name: str(tmp_path) # type: ignore[method-assign] - - with caplog.at_level(logging.WARNING, logger="mellea"): - backend.add_adapter(adapter) - - assert adapter.config["instruction"] == _REPAIRED_INSTRUCTION - assert caller_config == _BROKEN_CONFIG, "caller's dict must not be mutated" - assert any( - "'requirement-check_alora'" in r.message and "#1679" in r.message - for r in caplog.records - ) - _, config = backend._intrinsic_adapter_name_and_config(adapter) - assert config["instruction"] == _REPAIRED_INSTRUCTION - - def test_intrinsic_adapter_shim_lora_left_untouched(self, tmp_path): - backend = _stub_backend(_tokenizer()) - (tmp_path / "adapter_config.json").write_text(json.dumps({"r": 8})) - with pytest.warns(DeprecationWarning): - adapter = IntrinsicAdapter( - "requirement-check", - adapter_type=AdapterType.LORA, - config_dict=dict(_BROKEN_CONFIG), - base_model_name=backend.base_model_name, - ) - adapter.get_local_hf_path = lambda base_model_name: str(tmp_path) # type: ignore[method-assign] - - backend.add_adapter(adapter) - - assert adapter.config == _BROKEN_CONFIG - - def test_composed_adapter_path_repairs_config(self, tmp_path): - tok = _tokenizer() - backend = _stub_backend(tok) - key = "requirement-check_alora" - (tmp_path / "adapter_config.json").write_text( - json.dumps({"alora_invocation_tokens": _invocation_no_colon()}) - ) - binding = LocalFileBinding( - name="requirement-check", - adapter_type=AdapterType.ALORA, - repo_id="fake/repo", - ) - binding.get_local_hf_path = lambda base_model_name: str(tmp_path) # type: ignore[method-assign] - composed = Adapter( - identity=Identity( - name="requirement-check", - adapter_type="alora", - capability="requirement_check", - ), - io_contract=get_io_contract("requirement-check"), - weights=binding, - ) - with ( - patch( - "mellea.formatters.granite.intrinsics.obtain_io_yaml", - return_value="/fake/adapter.yaml", - ), - patch("builtins.open", mock_open(read_data="key: value")), - patch("yaml.safe_load", return_value=dict(_BROKEN_CONFIG)), - ): - backend.add_adapter(composed) - - assert ( - backend._composed_adapter_configs[key]["instruction"] - == _REPAIRED_INSTRUCTION - ) diff --git a/test/formatters/granite/base/test_base_alora_activation.py b/test/formatters/granite/base/test_base_alora_activation.py index a4e7165c92..26822fbc6c 100644 --- a/test/formatters/granite/base/test_base_alora_activation.py +++ b/test/formatters/granite/base/test_base_alora_activation.py @@ -307,17 +307,18 @@ class TestAloraDifferentialEndToEnd: scores measured 0.047 vs 0.999 in the #1679 diagnostic eval. """ + @pytest.mark.xfail( + strict=True, + raises=AssertionError, + reason="the published requirement-check aLoRA io.yaml instruction does " + "not tokenise to its declared invocation sequence, so the adapter " + "cannot activate until the adapter is republished", + ) def test_requirement_check_adapter_moves_score(self, alora_backend): from mellea.stdlib.components import Message from mellea.stdlib.components.intrinsic import core from mellea.stdlib.context import ChatContext - # NOTE: the published requirement-check io.yaml instruction does not - # tokenise to the adapter's declared invocation sequence (issue - # #1679). LocalHFBackend repairs it at load time with a warning - # (`_repair_alora_instruction`) until the publisher - # republishes a corrected file, so this test runs on the as-published - # adapter with no local workaround. backend = alora_backend("requirement-check", "requirement_check") ctx = ( ChatContext() @@ -376,8 +377,7 @@ class TestUncertaintyAloraDifferentialEndToEnd: general, not requirement-check-specific. The published uncertainty aLoRA io.yaml has no tokenisation mismatch, so - this test needs no instruction patching — it exercises the fix on the - as-published adapter files. Measured on granite-4.1-3b in the #1679 + this test exercises the fix on the as-published adapter files. Measured on granite-4.1-3b in the #1679 diagnostic: base model is underconfident (~0.06) on both a right and a wrong last response, while the adapter gives ~0.95 (right) and ~0.73 (wrong) — a wide differential either way.