[AngeloDanducci]

Pull Request

Issue

Fixes #1621

Description

Removes deprecated adapter shims

Testing

• Tests added to the respective file if code was changed
• New code has 100% coverage if code was added
• Ensure existing tests and github automation passes (a maintainer will kick off the github automation when the rest of the PR is populated)

Attribution

• AI coding assistants used

---

Adding a new component, requirement, sampling strategy, or tool?

If your PR adds or modifies one of the types below, check the matching box. A checklist of type-specific review items will be posted as a comment.

• Component
• Requirement
• Sampling Strategy
• Tool

NOTE: Please ensure you have an issue that has been acknowledged by a core contributor and routed you to open a pull request against this repository. Otherwise, please open an issue before continuing with this pull request.

[planetf1]

Shim removal itself is clean — traced the dispatch paths across all three backends, no regressions found. One thing before merge, flagged inline. Also curious whether #1621 wants a dedicated migration note or if the auto-generated changelog covers it — not blocking.

[jakelorocco]

The actual changes look good. A few test issues. Can you please do a full run of all the disparate huggingface tests? (or just run a nightly on this branch)

[jakelorocco]

Three failures:

FAILED test/backends/test_huggingface.py::test_adapters - KeyError: 'requirement-check_alora'
FAILED test/backends/test_huggingface.py::test_constraint_lora_with_requirement - AssertionError: assert 'requirement_check' in 'The answer should mention that there is a b in the middle of one of the strings but not the other.'
FAILED test/backends/test_huggingface.py::test_constraint_lora_override_does_not_override_alora - AssertionError: assert 'requirement_check' in 'None'

[jakelorocco]

Do these AdapterMixin tests need to get kept? They look like they aren't quite the shim tests described by the file?

[jakelorocco]

We may have previously missed this; config isn't used in this function.

[jakelorocco]

The new error handling in LocalFileBinding.activate() in mellea/backends/adapters/_core.py:719-731 causes unexpected behavior here. When you unload an adapter, it's model.peft_config gets removed. The fact that the error is now caught and this returns None, means the generation ends up happening without the adapter weights.

I think we just need unload_peft_adapter to reset the bindings _loaded field.

For example, the below script takes the old requirements adapter that didn't tokenize correctly. It should always raise an activation error. Instead:

1. call 1: raised AloraActivationError (guard ran)
2. after unload_peft_adapter(): in model.peft_config: False, binding._loaded: True
   call 2: returned score=0.9975  <-- guard did not run
   WARNING: aLoRA adapter 'requirement-check_alora' is active but its declared
   invocation sequence [27, 71226, 29] does not occur in the prompt, so
   generation used base-model weights only.
3. call 3: raised AloraActivationError (guard ran)
REPRODUCED: guard -> score -> guard

Claude coded reproduction script:

"""Repro for PR #1700: the aLoRA activation guard is skipped right after an unload.

`LocalFileBinding.activate()` now catches `ValueError` and reloads the weights.
That reload runs inside `adapter_scope`, after `_check_alora_activation` has
already looked for the adapter in `model.peft_config`, found nothing, and
returned. So the first call after `unload_peft_adapter()` generates with
base-model weights and returns a score instead of raising
`AloraActivationError`.

The adapter is pinned to a granitelib-core revision whose requirement-check
io.yaml writes `<requirements>:`. That tokenizes as [27, 71226, 27916], so the
aLoRA's invocation sequence [27, 71226, 29] never appears in the prompt and the
guard should raise on every call.

Run from the PR #1700 scratch workspace:

    HF_HUB_OFFLINE=1 uv run python scratchpad/repro_alora_guard_skip.py

Exits 1 when the bug reproduces, 0 otherwise.
"""

import logging
import sys

from mellea.backends import model_ids
from mellea.backends.adapters._core import (
    Adapter,
    AloraActivationError,
    Identity,
    LocalFileBinding,
)
from mellea.backends.adapters.catalog import AdapterType, fetch_intrinsic_metadata
from mellea.backends.adapters.io_contracts import get_io_contract
from mellea.backends.huggingface import LocalHFBackend
from mellea.core.utils import MelleaLogger
from mellea.stdlib.components import Message
from mellea.stdlib.components.intrinsic import core
from mellea.stdlib.context import ChatContext

NAME = "requirement-check"
QUALIFIED_NAME = f"{NAME}_alora"
MISMATCHED_REVISION = "d0a2a96a4cd07e96f0fe7ca29a42bfe088299d43"
REQUIREMENT = "The response must be a single sentence."


class _CollectWarnings(logging.Handler):
    def __init__(self) -> None:
        super().__init__(logging.WARNING)
        self.messages: list[str] = []

    def emit(self, record: logging.LogRecord) -> None:
        self.messages.append(record.getMessage())


def _call(backend: LocalHFBackend, ctx: ChatContext, label: str) -> str:
    try:
        score = core.requirement_check(ctx, backend, REQUIREMENT)
    except AloraActivationError:
        print(f"  {label}: raised AloraActivationError (guard ran)")
        return "guard"
    except Exception as e:
        print(f"  {label}: raised {type(e).__name__}: {e}")
        return "error"
    print(f"  {label}: returned score={score!r}  <-- guard did not run")
    return "score"


def main() -> int:
    warnings = _CollectWarnings()
    MelleaLogger.get_logger().addHandler(warnings)

    print("Loading granite-4.1-3b ...")
    backend = LocalHFBackend(model_id=model_ids.IBM_GRANITE_4_1_3B)

    metadata = fetch_intrinsic_metadata(NAME)
    binding = LocalFileBinding(
        name=NAME,
        adapter_type=AdapterType.ALORA,
        repo_id=metadata.repo_id,
        revision=MISMATCHED_REVISION,
    )
    backend.add_adapter(
        Adapter(
            identity=Identity(
                name=NAME, adapter_type="alora", capability=metadata.effective_capability
            ),
            io_contract=get_io_contract(NAME),
            weights=binding,
        )
    )

    ctx = (
        ChatContext()
        .add(Message("user", "Describe the ocean in one sentence."))
        .add(Message("assistant", "The ocean covers about 71% of Earth's surface."))
    )

    print("\n1. Weights loaded by add_adapter():")
    first = _call(backend, ctx, "call 1")

    backend.unload_peft_adapter(QUALIFIED_NAME)
    peft_config = getattr(backend._model, "peft_config", {}) or {}
    print("\n2. After backend.unload_peft_adapter():")
    print(f"  in model.peft_config: {QUALIFIED_NAME in peft_config}")
    print(f"  binding._loaded:      {binding._loaded}")
    second = _call(backend, ctx, "call 2")

    print("\n3. Next call (weights reloaded during call 2):")
    third = _call(backend, ctx, "call 3")

    base_model_warnings = [m for m in warnings.messages if "base-model weights only" in m]
    if base_model_warnings:
        print("\nWarning logged during generation:")
        for m in base_model_warnings:
            print(f"  {m}")

    reproduced = (first, second, third) == ("guard", "score", "guard")
    print(f"\n{'REPRODUCED' if reproduced else 'NOT REPRODUCED'}: {first} -> {second} -> {third}")
    return 1 if reproduced else 0


if __name__ == "__main__":
    sys.exit(main())

[planetf1]

Ran the repro from this thread against granite-4.1-3b with the pinned adapter. It reproduces at 7b72102 (guard, score, guard) and not at 5c93aa4, where call 2 now raises AloraActivationError. So the reload-before-guard fix works.

Nothing pins it yet though: replace the reload with pass and the unit tests still pass. A small test would cover it: aLoRA in _added_adapters but not _loaded_adapters, assert load_peft_adapter runs before _check_alora_activation.

[jakelorocco]

I think the exception block needs to set _loaded to untrue. I think that will fix the below issue as well.

[planetf1]

Traced both suggested fixes and neither closes it:

• Setting _loaded = False in this except block is too late: _check_alora_activation has already returned for this call (it bails at huggingface.py:534 when the name is missing from peft_config).
• Resetting it in unload_peft_adapter makes the next call raise RuntimeError: the generation path never re-prepares (huggingface.py:1021, lock-order), and resolve_adapter returns a found adapter without re-preparing (adapter.py:991). That brings back the test_constraint_lora_* failures this reload was added to fix.

The weights need to be back before the guard runs. In _generate_from_intrinsic, when the aLoRA qualified name is missing from _loaded_adapters but still in _added_adapters, reload it under _generation_lock ahead of _check_alora_activation; this except then only covers a genuine race. Two side notes: the _alora_qualified_name_for_activation_check docstring (huggingface.py:457) still says registration-time prepare() makes the check safe, and the reload bills a full PEFT load into the phase="activate" metric (adapter.py:1144), which a pre-guard reload removes.

[planetf1]

The removal itself is clean: dispatch, re-exports and the source sweep all check out at this head, and the resolve/locking port is thorough. Before merge:

• Reload resilience (blocking). The new reload in LocalFileBinding.activate() interacts badly with the aLoRA guard, as discussed in the threads on huggingface.py:534 and _core.py:723. Details and a suggested ordering are in my reply there. There is also a second, rarer failure on the reload itself (inline).
• Docs still describe the shims as live. Three pages need the same sweep the code got: docs/docs/advanced/lora-and-alora-adapters.md:264 (the "deprecated IntrinsicAdapter shim" bullet, plus the "one of the three paths" wording below it), docs/docs/advanced/intrinsics.md:379 ("remain functional for now"), and docs/docs/observability/metrics.md:308 ("deprecated shim classes alike").
• Migration note. #1621 lists a changelog entry with a migration note as in scope, and the chore: title files this under "Other Changes" (.github/release.yml maps * there). There's precedent for refactor(backends)!: (#1422). A short mapping would cover most users: shim constructors to resolve_adapter / composed Adapter + LocalFileBinding; from_hub/from_source to register_embedded_adapter_model; CustomIntrinsicAdapter to the tutorial 07 pattern; plus a line that subclass add_adapter implementations now receive a composed Adapter (with config= on the embedded path).
• Test coverage lost in the port (inline). The adapter_scope TypeError guard and the openai.py embedded-weights guard have no test left, and three intrinsic tests moved to the embedded stub, so the local-file branch lost logits and closure-release coverage.
• HF qualitative run. I can't see a nightly or qualitative run on this branch at this head, only the standard CI. Worth the run jakelorocco asked for, since the test_constraint_lora_* fixes rest on the new reload path.

[planetf1]

Traced both suggested fixes and neither closes it:

• Setting _loaded = False in this except block is too late: _check_alora_activation has already returned for this call (it bails at huggingface.py:534 when the name is missing from peft_config).
• Resetting it in unload_peft_adapter makes the next call raise RuntimeError: the generation path never re-prepares (huggingface.py:1021, lock-order), and resolve_adapter returns a found adapter without re-preparing (adapter.py:991). That brings back the test_constraint_lora_* failures this reload was added to fix.

The weights need to be back before the guard runs. In _generate_from_intrinsic, when the aLoRA qualified name is missing from _loaded_adapters but still in _added_adapters, reload it under _generation_lock ahead of _check_alora_activation; this except then only covers a genuine race. Two side notes: the _alora_qualified_name_for_activation_check docstring (huggingface.py:457) still says registration-time prepare() makes the check safe, and the reload bills a full PEFT load into the phase="activate" metric (adapter.py:1144), which a pre-guard reload removes.

[planetf1]

Separate failure mode on the reload itself, worth fixing alongside the guard-ordering point in the thread above.

If load_peft_adapter raises here (say the adapter cache was evicted after unload_peft_adapter()), PEFT has already injected the modules and registered the name in peft_config before it reads the checkpoint. The exception propagates with the name half-registered and _loaded still True, so the next activate() succeeds through set_adapter without retrying the load. From then on every call generates with zero-effect LoRA modules: base-model output under the adapter's name, with no error.

Reproduced offline with transformers 5.10.2 / peft 0.19.1: load_adapter pointed at a dir whose checkpoint file was removed raises FileNotFoundError but leaves the name in peft_config; set_adapter then succeeds and the logits equal the base model's (max diff 0.0). It needs an unload plus a failed reload, so it's unlikely, but the wrong results would persist silently.

Suggest catching the failure here and calling backend.unload_peft_adapter(self.qualified_name) before re-raising. That method already has best-effort cleanup for exactly this injected-but-unloaded state (huggingface.py:3257-3274), so the next call hits the ValueError again and retries the load cleanly. I'd leave _loaded alone, for the reason given in the thread above. The Raises: section (_core.py:704) could also say what the retry propagates.

[planetf1]

Nit, and rare: this reload has the same gap as the one in activate(). If load_peft_adapter fails partway (e.g. checkpoint evicted after an unload), the name stays in peft_config but not in _loaded_adapters. The next call's load_peft_adapter then swallows PEFT's "already exists" error (huggingface.py:3241-3246) and generation runs on modules without the checkpoint weights. Reproduced at 5c93aa4.

It needs an unload plus a failed reload, so fine to leave if you'd rather not widen this PR. If you do fix it, doing the cleanup inside load_peft_adapter would cover both sites.

[planetf1]

Re-checked at 5c93aa4. The earlier feedback is addressed. I ran jakelorocco's aLoRA repro against granite-4.1-3b: it reproduces at 7b72102 and not here, so the reload fix works.

Happy to approve once the HF tests have had a full run (including the test_constraint_lora_* ones). test/scripts/run_tests_with_ollama_and_vllm.sh does it, and PHASES=hf limits it to the HF phase. If you don't have a GPU host, a maintainer can run it (no on-demand nightly yet, #734).

Non-blocking: a unit test for the reload-before-guard ordering would be good.