diff --git a/AGENTS.md b/AGENTS.md index 1c4f4374c..9f2185abe 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -213,6 +213,18 @@ score = core.check_certainty(context, backend) For lower-level control (custom adapters, model options), use `mfuncs.act()` with `Intrinsic` directly — see examples in `docs/examples/intrinsics/`. +### Weights binding shapes + +`Adapter.weights` normalizes each deployment's activation mechanism behind one of +two shapes — a `WeightsBinding` lifecycle for weights you stage yourself, or +`EmbeddedBinding.apply_activation` for weights already in the served model. The +post-activation shape each produces: + +| Binding | Reality | Lifecycle verbs | Caller invokes | Normalized post-activation state | +|---------|---------|------------------|-----------------|-----------------------------------| +| `LocalFileBinding` | LocalFile/PEFT | `prepare` / `activate` / `deactivate` / `release` | `activate()` / `deactivate()`, via `adapter_scope` | Backend-internal PEFT adapter state toggled; the outgoing request is untouched | +| `EmbeddedBinding` | Embedded/Granite Switch | none — weights are already in the served model | `apply_activation(request, identity)` | `request.extra_body["chat_template_kwargs"]["adapter_name"]` set; `request.api_params["model"]` removed if present | + ### Project Resources - **Canonical catalog**: `mellea/backends/adapters/catalog.py` — source of truth for adapter function names, HF repo IDs, and adapter types diff --git a/docs/docs/advanced/intrinsics.md b/docs/docs/advanced/intrinsics.md index e75b9b797..8d3ac9806 100644 --- a/docs/docs/advanced/intrinsics.md +++ b/docs/docs/advanced/intrinsics.md @@ -227,6 +227,81 @@ For OpenAI backends with Granite Switch, adapters are loaded from the model's Hugging Face repository configuration instead of the adapter function catalog. Output format is task-specific — `requirement-check` returns `{"requirement_check": {"score": }}`. +## Composable adapter construction (advanced) + +> **Advanced:** `Adapter` composes an `Identity`, an `IOContract`, and a +> weights binding into a single, inspectable object. It's scaffolding for a +> future backend-integration surface (Epic #929) — today, neither backend +> accepts a composed `Adapter` directly: `LocalHFBackend.add_adapter` takes a +> `LocalFileBinding` or the `LocalHFAdapter` shim, while +> `OpenAIBackend.add_adapter` takes only the deprecated +> `EmbeddedIntrinsicAdapter` shim, which builds an `EmbeddedBinding` +> internally. The construction below is illustrative of the binding shapes; +> write a new backend integration against the bindings themselves. + +Each weights binding models how its deployment turns an adapter on. +`LocalFileBinding` downloads and loads LoRA/aLoRA weights, so it exposes a +`prepare`/`activate`/`deactivate`/`release` lifecycle: + +```python +# Requires: mellea[hf] +from mellea.backends.adapters import Adapter, EmbeddedBinding, Identity, IOContract, LocalFileBinding +from mellea.backends.huggingface import LocalHFBackend +from mellea.backends.openai import OpenAIBackend +from mellea.core import Component + + +class AnswerabilityContract(IOContract): + def build_prompt(self, **kwargs: object) -> Component: + raise NotImplementedError # request formatting lands with #1516 + + def parse(self, raw: str) -> dict[str, object]: + import json + + return json.loads(raw) + + +# LocalFile/PEFT reality — LocalHFBackend downloads and loads the weights. +hf_backend = LocalHFBackend(model_id="ibm-granite/granite-4.1-3b") +hf_binding = LocalFileBinding.from_catalog("answerability") +hf_binding.bind_backend(hf_backend) +# hf_binding.prepare() downloads the weights and loads them into hf_backend. +# adapter_type must match the binding — from_catalog loads the first +# catalog-listed adapter type, which is LoRA for answerability. +hf_adapter = Adapter( + identity=Identity(name="answerability", adapter_type="lora"), + io_contract=AnswerabilityContract(), + weights=hf_binding, +) +``` + +`EmbeddedBinding` has no weights to manage — the adapter is already part of +the served base model — so it exposes a single method, `apply_activation`, +that edits the outgoing request instead of a lifecycle: + +```python +switch_backend = OpenAIBackend( + model_id="granite-switch", + api_key="EMPTY", + base_url="http://localhost:8000/v1", +) +switch_adapter = Adapter( + identity=Identity(name="answerability", adapter_type="alora"), + io_contract=AnswerabilityContract(), + weights=EmbeddedBinding.from_base_model(switch_backend), +) +``` + +Weights-binding support by backend today — this tracks the binding +implementations, not whether a composed `Adapter` can be registered directly: + +| Backend | `LocalFileBinding` (LocalFile/PEFT) | `EmbeddedBinding` (Embedded/Granite Switch) | `ServerMediatedBinding` | +| --- | --- | --- | --- | +| `LocalHFBackend` | ✅ shipping — `add_adapter` accepts a `LocalFileBinding` directly | 🔜 planned (#1018) | — | +| `OpenAIBackend` | — | ✅ shipping, via the deprecated `EmbeddedIntrinsicAdapter` shim above, which builds an `EmbeddedBinding` internally | — | + +`ServerMediatedBinding` has no backend implementation yet — see discussion #1486. + --- ## Guardian adapter functions diff --git a/mellea/backends/adapters/__init__.py b/mellea/backends/adapters/__init__.py index 932f3f55b..1c16445ad 100644 --- a/mellea/backends/adapters/__init__.py +++ b/mellea/backends/adapters/__init__.py @@ -6,6 +6,7 @@ from ._core import ( Adapter, AdapterSchemaMismatchError, + EmbeddedActivationRequest, EmbeddedBinding, Identity, IOContract, @@ -34,6 +35,7 @@ "AdapterMixin", "AdapterSchemaMismatchError", "AdapterType", + "EmbeddedActivationRequest", "EmbeddedBinding", "EmbeddedIntrinsicAdapter", "IOContract", diff --git a/mellea/backends/adapters/_core.py b/mellea/backends/adapters/_core.py index 25fd09fa5..fb4dba3a9 100644 --- a/mellea/backends/adapters/_core.py +++ b/mellea/backends/adapters/_core.py @@ -12,7 +12,8 @@ Also provides: - :class:`LocalFileBinding` -- :class:`EmbeddedBinding` — stub :class:`WeightsBinding` subclass +- :class:`EmbeddedBinding` — Embedded/Granite Switch binding; `apply_activation`, + no weights lifecycle (issue #1142) - :class:`ServerMediatedBinding` — stub :class:`WeightsBinding` subclass - :class:`AdapterSchemaMismatchError` - :class:`_DictContract`, :class:`_ListContract` — generic, capability-agnostic @@ -37,7 +38,7 @@ import time import warnings from dataclasses import dataclass -from typing import TYPE_CHECKING, ClassVar, Literal +from typing import TYPE_CHECKING, Any, ClassVar, Literal from ...core import Component, MelleaLogger from ...helpers.event_loop_helper import _run_async_in_thread @@ -680,31 +681,161 @@ def _fire_phase_complete(self, phase: str, duration_s: float) -> None: ) -class EmbeddedBinding(WeightsBinding): - """Stub binding for weights embedded in a model artifact.""" +@dataclass +class EmbeddedActivationRequest: + """Mutable outgoing-request state that `EmbeddedBinding.apply_activation` edits. + + Bundles the two dicts an OpenAI-compatible call site builds separately — + `extra_body` and the top-level API call kwargs — so a binding can edit + both in one call. Both are mutated in place. The caller must merge its + other edits (tool wiring, thinking mode, user options) before calling + `apply_activation`, because the binding performs the final framework edit + and the selected adapter is authoritative. + + Attributes: + extra_body (dict[str, Any]): The provider's `extra_body` payload. + `apply_activation` writes the activation field here (e.g. + `chat_template_kwargs.adapter_name` for Granite Switch). + api_params (dict[str, Any]): Top-level request kwargs (e.g. `model`). + `apply_activation` removes entries an embedded adapter's + activation would make incorrect. + """ + + extra_body: dict[str, Any] + api_params: dict[str, Any] + + +class EmbeddedBinding: + """Weights binding for the Embedded/Granite Switch reality (Epic #929 Phase 2). + + Adapter weights for this reality are already baked into the base model — + there is nothing to download, load, toggle, or unload. The only thing + that varies per call is one field on the outgoing request, so this + binding has a single method, `apply_activation`, rather than the four + `WeightsBinding` lifecycle verbs (see discussion #1486, which rescoped + issue #1142: declaring the four verbs here would mean four raises with + nothing behind them, as the previous stub did). + + Stateless across calls: `apply_activation` reads only its arguments, so + activating one adapter for a call never leaks into the request built for + the next. + + Attributes: + binding_type (ClassVar[str]): `"embedded"`. + source (str): Base model identifier this binding activates adapters + against — the backend's `base_model_name` (e.g. `granite-4.1-3b` + for a backend built against `ibm-granite/granite-4.1-3b`). + Stamped by `OpenAIBackend.add_adapter` at registration; not + otherwise used by `apply_activation`. + """ binding_type: ClassVar[str] = "embedded" - def prepare(self) -> None: - raise NotImplementedError( - _PHASE_2_NOT_IMPLEMENTED.format(cls="EmbeddedBinding") - ) + def __init__(self, source: str = "") -> None: + """Constructs an EmbeddedBinding. - def activate(self) -> None: - raise NotImplementedError( - _PHASE_2_NOT_IMPLEMENTED.format(cls="EmbeddedBinding") - ) + Args: + source: Base model identifier this binding activates adapters + against. Prefer `from_base_model` when a backend is on hand. + """ + self.source = source - def deactivate(self) -> None: - raise NotImplementedError( - _PHASE_2_NOT_IMPLEMENTED.format(cls="EmbeddedBinding") - ) + @classmethod + def from_base_model(cls, backend: "AdapterMixin") -> "EmbeddedBinding": + """Builds an EmbeddedBinding recording `backend`'s base model as the source. - def release(self) -> None: - raise NotImplementedError( - _PHASE_2_NOT_IMPLEMENTED.format(cls="EmbeddedBinding") + Args: + backend: The backend whose base model has the adapter embedded + (e.g. an `OpenAIBackend` pointed at a Granite Switch deployment). + + Returns: + An `EmbeddedBinding` with `source` set to `backend.base_model_name`. + """ + return cls(source=backend.base_model_name) + + async def apply_activation( + self, request: EmbeddedActivationRequest, identity: "Identity" + ) -> None: + """Edits `request` so the served model activates `identity`'s adapter. + + Granite Switch (the only Embedded deployment today) reads the + adapter to activate from `chat_template_kwargs["adapter_name"]` in + the chat template. The rewriter config can also set the top-level + `model` parameter to the adapter's name; for an embedded adapter the + real model is the base model already being served, so that value is + dropped here rather than sent to the API. + + Fires `adapter_function_phase_complete` (phase `"activate"`), so + Embedded calls contribute to the `mellea.adapter_function.phase_duration` + metric like every other binding. Does **not** fire + `adapter_function_invocation_complete`: unlike `LocalFileBinding`'s + verbs (driven by `adapter_scope`, which wraps the whole call and + knows the real outcome), this method only edits a request — the + actual generation and parsing happen later, asynchronously, once the + caller awaits the resulting `ModelOutputThunk`. Firing an + invocation-complete event here would have to guess an `outcome` that + this method cannot know, which is worse than not firing it: it would + report `outcome="success"` for calls that go on to fail. Wiring a + real invocation-complete signal in requires the caller (currently + `OpenAIBackend._generate_from_intrinsic`) to fire it once generation + and parsing resolve — tracked as a follow-up, not part of this method. + + This method is `async` (unlike the rest of `EmbeddedBinding`'s + surface) purely because hook dispatch (`invoke_hook`) is async; its + own work is synchronous. Its one caller, + `OpenAIBackend._generate_from_intrinsic`, is already a coroutine, so + `await`ing here — rather than bridging through + `_run_async_in_thread`, which is for calling async code from sync + code — avoids spawning a throwaway event loop and thread per call. + + Args: + request: The outgoing request state to edit; both of its dicts + are mutated in place. + identity: Identifies the adapter to activate. + """ + started_at = time.monotonic() + chat_template_kwargs = request.extra_body.pop("chat_template_kwargs", {}) or {} + chat_template_kwargs["adapter_name"] = identity.name + request.extra_body["chat_template_kwargs"] = chat_template_kwargs + request.api_params.pop("model", None) + duration_s = time.monotonic() - started_at + + await self._fire_activate_phase_complete(identity.name, duration_s) + + async def _fire_activate_phase_complete(self, name: str, duration_s: float) -> None: + """Fires `adapter_function_phase_complete` for the activate phase. + + `duration_s` is the cost of editing a dict, not of an adapter + activation in the sense `LocalFileBinding`'s real PEFT activation is — + the resulting `phase_duration` samples for `binding_type="embedded"` + are not comparable to `binding_type="local_file"` samples for the + same adapter name; the histogram carries no `binding_type` attribute + to separate them. + + Args: + name: Adapter function name, used as the metric's `name` field. + duration_s: Wall-clock duration of `apply_activation`'s request edit. + """ + if not has_plugins(HookType.ADAPTER_FUNCTION_PHASE_COMPLETE): + return + + from ...plugins.hooks.adapter_function import ( + AdapterFunctionPhaseCompletePayload, ) + try: + payload = AdapterFunctionPhaseCompletePayload( + name=name, phase="activate", duration_ms=duration_s * 1000.0 + ) + await invoke_hook(HookType.ADAPTER_FUNCTION_PHASE_COMPLETE, payload) + except Exception: + MelleaLogger.get_logger().warning( + f"adapter_function_phase_complete hook dispatch failed for {name!r} " + "during 'activate'; ignoring so it does not turn a completed " + "request edit into an operation failure.", + exc_info=True, + ) + class ServerMediatedBinding(WeightsBinding): """Stub binding for server-managed adapter weights.""" @@ -736,18 +867,22 @@ def release(self) -> None: class Adapter: """Composable adapter dataclass (Epic #929 Phase 0). - Composes an :class:`Identity`, an :class:`IOContract`, and a - :class:`WeightsBinding` into a single, inspectable object. + Composes an :class:`Identity`, an :class:`IOContract`, and a weights + binding (a :class:`WeightsBinding` or :class:`EmbeddedBinding`) into a + single, inspectable object. Attributes: identity (Identity): Name, type, and capability for this adapter. io_contract (IOContract): Prompt builder and output parser. - weights (WeightsBinding): Pluggable weights lifecycle handler. + weights (WeightsBinding | EmbeddedBinding): Pluggable weights handler — + either a `WeightsBinding` (a lifecycle to stage and switch on) or + an `EmbeddedBinding` (nothing to stage; activation edits the + outgoing request instead). """ identity: Identity io_contract: IOContract - weights: WeightsBinding + weights: WeightsBinding | EmbeddedBinding # NOTE(#1516): a construction-time cross-check that `weights.adapter_type` # agrees with `identity.adapter_type` was tried here and backed out. It is the diff --git a/mellea/backends/adapters/adapter.py b/mellea/backends/adapters/adapter.py index 528b28422..a173aaaa5 100644 --- a/mellea/backends/adapters/adapter.py +++ b/mellea/backends/adapters/adapter.py @@ -33,6 +33,7 @@ from ._core import ( Adapter as _AdapterCore, AdapterSchemaMismatchError, + EmbeddedBinding, Identity, LocalFileBinding, WeightsBinding, @@ -97,27 +98,23 @@ def get_local_hf_path(self, base_model_name: str) -> str: class _ShimWeightsBinding(WeightsBinding): - """Phase 1 placeholder; Phase 2 (see epic #929) wires in real lifecycle.""" + """Placeholder weights binding for the deprecated IntrinsicAdapter shims. + + All lifecycle verbs raise NotImplementedError; it exists only so the + shims can satisfy the Adapter protocol. + """ def prepare(self) -> None: - raise NotImplementedError( - "Phase 2 (see epic #929) — WeightsBinding not yet implemented" - ) + raise NotImplementedError("WeightsBinding not yet implemented") def activate(self) -> None: - raise NotImplementedError( - "Phase 2 (see epic #929) — WeightsBinding not yet implemented" - ) + raise NotImplementedError("WeightsBinding not yet implemented") def deactivate(self) -> None: - raise NotImplementedError( - "Phase 2 (see epic #929) — WeightsBinding not yet implemented" - ) + raise NotImplementedError("WeightsBinding not yet implemented") def release(self) -> None: - raise NotImplementedError( - "Phase 2 (see epic #929) — WeightsBinding not yet implemented" - ) + raise NotImplementedError("WeightsBinding not yet implemented") class IntrinsicAdapter(LocalHFAdapter, _AdapterCore): @@ -437,7 +434,7 @@ class AdapterMixin(Backend, abc.ABC): Three verbs are universal across every adapter reality (LocalFile/PEFT, Embedded/Granite Switch, ServerMediated): `base_model_name`, - `add_adapter`, and `list_adapters`. The remaining seven verbs are + `add_adapter`, and `list_adapters`. The remaining five verbs are reality-specific — a concrete backend overrides only the verb(s) matching its own reality; the others keep raising `NotImplementedError`. @@ -598,49 +595,6 @@ def _adapter_activation_lock( """ return contextlib.nullcontext() - def render_controls(self, adapter_qualified_name: str, active: bool) -> None: - """Render or clear the control tokens for a baked-in embedded adapter. - - Embedded/Granite Switch reality only. Weights are already baked into - the model; this only toggles the control-token rendering that - activates or deactivates the adapter's behaviour for subsequent - requests. - - Args: - adapter_qualified_name (str): The `adapter.qualified_name` of the - adapter to activate or deactivate. - active (bool): `True` to render the adapter's control tokens, - `False` to clear them. - - Raises: - NotImplementedError: If this backend's adapter reality is not - Embedded/Granite Switch. - """ - raise NotImplementedError( - f"Backend type {type(self)} does not support render_controls()." - ) - - def set_request_adapter(self, adapter_qualified_name: str) -> None: - """Select the adapter to use for the next request. - - ServerMediated reality only — for servers that accept an adapter - selection per request rather than loading/unloading weights or - toggling control tokens locally. No backend implements this reality - yet. - - Args: - adapter_qualified_name (str): The `adapter.qualified_name` of the - adapter to select. - - Raises: - NotImplementedError: Always — the ServerMediated adapter reality - has no implementation yet. - """ - raise NotImplementedError( - f"Backend type {type(self)} does not support set_request_adapter(); " - "the ServerMediated adapter reality is not implemented yet." - ) - def resolve_adapter(self, name: str) -> _AdapterCore: """Find or lazily register an adapter by capability name. @@ -799,10 +753,18 @@ def adapter_scope(self, adapter: "_AdapterCore | None"): # type: ignore[type-ar deactivates. Pre-existing, not specific to the intrinsic path this method now supports. + `AdapterFunctionMetricsPlugin` in + `mellea/telemetry/metrics_plugins.py` emits the adapter-function + metrics; their instruments and attributes are defined in + `mellea/telemetry/metrics.py`. + Args: adapter: The adapter to activate, or `None` (no-op). Raises: + TypeError: `adapter.weights` is not a `WeightsBinding` (e.g. an + `EmbeddedBinding`, which has no activate()/deactivate() to + scope — call its `apply_activation()` directly instead). BaseException: An error raised by activation, the `with` body, or deactivation. If both the body and deactivation fail, the body error remains primary and the deactivation error is chained. @@ -829,6 +791,16 @@ def adapter_scope(self, adapter: "_AdapterCore | None"): # type: ignore[type-ar binding_type = adapter.weights.binding_type adapter_type = adapter.identity.adapter_type + # adapter_scope drives the WeightsBinding lifecycle (activate/deactivate); + # a binding with no lifecycle (e.g. EmbeddedBinding) activates through its + # own apply_activation() instead (issue #1142) and never reaches this scope. + if not isinstance(adapter.weights, WeightsBinding): + raise TypeError( + f"adapter_scope() requires a WeightsBinding-backed adapter; " + f"{binding_type!r} bindings have no activate()/deactivate() to " + "scope. Call apply_activation() directly instead." + ) + outcome: Literal["success", "schema_error", "error"] = "success" exception: BaseException | None = None activated = False @@ -982,8 +954,7 @@ class EmbeddedIntrinsicAdapter(_AdapterCore): - `io_contract`: the real, declared contract for `intrinsic_name` (issue #1516); no longer a placeholder. - - `weights`: Phase 1 `_ShimWeightsBinding` placeholder; raises - `NotImplementedError` until issue #1142 replaces it. + - `weights`: a real `EmbeddedBinding`; activation runs through it. """ def __setattr__(self, name: str, value: object) -> None: @@ -1032,7 +1003,7 @@ def __init__(self, intrinsic_name: str, config: dict, technology: str = "lora"): io_contract = get_io_contract(intrinsic_name) - weights = _ShimWeightsBinding() + weights = EmbeddedBinding() _AdapterCore.__init__( self, identity=identity, io_contract=io_contract, weights=weights diff --git a/mellea/backends/huggingface.py b/mellea/backends/huggingface.py index 3d0507b78..e471cd423 100644 --- a/mellea/backends/huggingface.py +++ b/mellea/backends/huggingface.py @@ -583,11 +583,11 @@ def _generate_with_adapter_lock(self, generate_func: Callable, *args, **kwargs): adapter is deactivated before the model call, and the model's active state is asserted before and after. Adapter-active generation goes elsewhere — `_generate_intrinsic_with_adapter_scope` for intrinsics - (routed through `adapter_scope`, with its lifecycle hooks), and, once - #1018 lands, Granite Switch's embedded activation through a binding - `apply_activation` verb (not yet implemented — that is #1142's work) — - so no current path activates through this method, which is why it - takes no adapter name. + (routed through `adapter_scope`, with its lifecycle hooks). Granite + Switch's `EmbeddedBinding.apply_activation()` is implemented for the + OpenAI backend; local Hugging Face integration remains pending (#1018). + No current path activates through this method, which is why it takes no + adapter name. Args: generate_func: The synchronous generation callable to invoke. @@ -664,15 +664,14 @@ def _generate_intrinsic_with_adapter_scope( Returns: Whatever `generate_func` returns. """ + weights = _IntrinsicPeftBinding( + self, adapter.qualified_name, adapter.intrinsic_metadata.revision + ) scope_adapter = _AdapterCore( - identity=adapter.identity, - io_contract=_UnusedIOContract(), - weights=_IntrinsicPeftBinding( - self, adapter.qualified_name, adapter.intrinsic_metadata.revision - ), + identity=adapter.identity, io_contract=_UnusedIOContract(), weights=weights ) with self._generation_lock: - scope_adapter.weights.prepare() + weights.prepare() with self.adapter_scope(scope_adapter): _assert_correct_adapters(adapter.qualified_name, self._model) out = generate_func(*args, **kwargs) diff --git a/mellea/backends/openai.py b/mellea/backends/openai.py index 739f5581b..9d536f996 100644 --- a/mellea/backends/openai.py +++ b/mellea/backends/openai.py @@ -53,6 +53,7 @@ from ..stdlib.requirements import LLMaJRequirement from ..telemetry.context import generate_request_id, with_context from ._options import resolve_model_options +from .adapters import EmbeddedActivationRequest, EmbeddedBinding from .adapters.adapter import AdapterInput, AdapterMixin, EmbeddedIntrinsicAdapter from .backend import FormatterBackend from .model_options import ModelOption @@ -308,7 +309,9 @@ def add_adapter(self, adapter: AdapterInput) -> None: Accepts the full `AdapterInput` union to honour the mixin contract, but currently only `EmbeddedIntrinsicAdapter` (the Embedded/Granite Switch - reality) is supported; other realities are rejected at runtime. + reality) is supported; other realities are rejected at runtime. As a + side effect, an `EmbeddedBinding` weights handler is stamped with this + backend's `base_model_name` in its `source` field. Args: adapter (AdapterInput): The adapter to register. Must be an @@ -323,23 +326,10 @@ def add_adapter(self, adapter: AdapterInput) -> None: f"Got: {type(adapter).__name__}" ) adapter.backend = self + if isinstance(adapter.weights, EmbeddedBinding): + adapter.weights.source = self.base_model_name self._added_adapters[adapter.qualified_name] = adapter - def render_controls(self, adapter_qualified_name: str, active: bool) -> None: - """No-op for embedded adapters — weights are baked into the model. - - Args: - adapter_qualified_name (str): The `adapter.qualified_name` of the - adapter to activate or deactivate. - active (bool): `True` to activate the adapter, `False` to - deactivate it. - """ - MelleaLogger.get_logger().debug( - "render_controls is a no-op for OpenAIBackends (adapter: %s, active: %s)", - adapter_qualified_name, - active, - ) - def list_adapters(self) -> list[str]: """Return qualified names of all registered adapters. @@ -702,9 +692,14 @@ async def _generate_from_intrinsic( intrinsic output. Raises: + NotImplementedError: If the context isn't a chat context, or if + streaming is requested (intrinsic post-processing requires + the complete response). ValueError: If no embedded adapter is registered for the requested intrinsic. - TypeError: If the adapter isn't an EmbeddedIntrinsicAdapter. + TypeError: If the adapter isn't an EmbeddedIntrinsicAdapter, or its + `weights` isn't an EmbeddedBinding (only reachable if a caller + reassigns `.weights` after construction). """ if not ctx.is_chat_context: raise NotImplementedError("Intrinsics require a chat context.") @@ -782,15 +777,6 @@ async def _generate_from_intrinsic( if rewriter.parameters: api_params.update(rewriter.parameters) - # Embedded adapters activate via control tokens in the chat template. - if isinstance(adapter, EmbeddedIntrinsicAdapter): - chat_template_kwargs = extra_body.pop("chat_template_kwargs", {}) or {} - chat_template_kwargs["adapter_name"] = action.intrinsic_name - extra_body["chat_template_kwargs"] = chat_template_kwargs - # The rewriter config may set `model` to the adapter name, but - # for embedded adapters the actual model is self._model_id. - api_params.pop("model", None) - # Collect tools if tool_calls is enabled. tools: dict[str, AbstractMelleaTool] = dict() if tool_calls: @@ -807,6 +793,22 @@ async def _generate_from_intrinsic( model_options, is_chat_context=True ) user_extra_body = user_api_params.pop("extra_body", None) + if user_extra_body is not None: + protected_extra_body_keys = { + "messages", + "model", + "parallel_tool_calls", + "stream", + "stream_options", + "tool_choice", + "tools", + } + overridden_keys = protected_extra_body_keys.intersection(user_extra_body) + if overridden_keys: + raise ValueError( + "extra_body cannot override intrinsic request fields: " + + ", ".join(sorted(overridden_keys)) + ) api_params.update(user_api_params) # Map THINKING to the correct backend parameter(s). Two mechanisms: @@ -828,6 +830,23 @@ async def _generate_from_intrinsic( extra_body = self._merge_user_extra_body(extra_body, user_extra_body) + # Embedded adapters activate via control tokens in the chat template; + # the binding owns the final request edit so callers cannot override + # the adapter selected for this intrinsic. `adapter.weights` is always + # an EmbeddedBinding here — EmbeddedIntrinsicAdapter.__init__ + # constructs one unconditionally — but the shim permits attribute + # mutation, so a caller reassigning `.weights` must fail loudly here + # rather than silently skip activation and send an unactivated request. + if not isinstance(adapter.weights, EmbeddedBinding): + raise TypeError( + f"EmbeddedIntrinsicAdapter.weights must be an EmbeddedBinding; " + f"got {type(adapter.weights).__name__}. Activation cannot proceed." + ) + activation_request = EmbeddedActivationRequest( + extra_body=extra_body, api_params=api_params + ) + await adapter.weights.apply_activation(activation_request, adapter.identity) + # --- call the OpenAI-compatible API -------------------------------- # The rewriter may add instruction messages where 'role' is a default # (e.g. UserMessage with role="user"). exclude_unset would drop it, diff --git a/test/backends/test_adapters/test_adapter_mixin.py b/test/backends/test_adapters/test_adapter_mixin.py index 1fd5c1b51..b04b353c3 100644 --- a/test/backends/test_adapters/test_adapter_mixin.py +++ b/test/backends/test_adapters/test_adapter_mixin.py @@ -5,11 +5,13 @@ Verifies that: - the reality-specific verbs (`load_peft_adapter`, `unload_peft_adapter`, - `remove_adapter`, `activate_peft_adapter`, `deactivate_peft_adapter`, - `render_controls`, `set_request_adapter`) raise `NotImplementedError` by - default on the mixin + `remove_adapter`, `activate_peft_adapter`, `deactivate_peft_adapter`) + raise `NotImplementedError` by default on the mixin - each concrete backend overrides only the verb(s) matching its own adapter reality, leaving the others on the default (raising) implementation + +Embedded/Granite Switch activation is not a mixin verb — it lives on +`EmbeddedBinding.apply_activation` (issue #1142); see test_embedded_binding.py. """ from unittest.mock import MagicMock @@ -26,8 +28,6 @@ "remove_adapter", "activate_peft_adapter", "deactivate_peft_adapter", - "render_controls", - "set_request_adapter", ) @@ -36,10 +36,9 @@ def test_default_reality_specific_verb_raises_not_implemented(verb): """Each reality-specific verb raises NotImplementedError by default on the mixin.""" mock_backend = MagicMock(spec=AdapterMixin) method = getattr(AdapterMixin, verb) - args = ("some_adapter", True) if verb == "render_controls" else ("some_adapter",) with pytest.raises(NotImplementedError): - method(mock_backend, *args) + method(mock_backend, "some_adapter") def test_hf_backend_overrides_only_peft_verbs(): @@ -49,23 +48,16 @@ def test_hf_backend_overrides_only_peft_verbs(): assert "remove_adapter" in vars(LocalHFBackend) assert "activate_peft_adapter" in vars(LocalHFBackend) assert "deactivate_peft_adapter" in vars(LocalHFBackend) - assert "render_controls" not in vars(LocalHFBackend) - assert "set_request_adapter" not in vars(LocalHFBackend) -def test_openai_backend_overrides_only_render_controls(): - """OpenAIBackend (Embedded/Granite Switch reality) overrides render_controls only.""" - assert "render_controls" in vars(OpenAIBackend) +def test_openai_backend_overrides_no_reality_specific_verbs(): + """OpenAIBackend (Embedded/Granite Switch reality) activates through the + binding, not a mixin verb, so it overrides none of the PEFT verbs.""" assert "load_peft_adapter" not in vars(OpenAIBackend) assert "unload_peft_adapter" not in vars(OpenAIBackend) assert "remove_adapter" not in vars(OpenAIBackend) - assert "set_request_adapter" not in vars(OpenAIBackend) - - -def test_no_backend_implements_server_mediated_reality(): - """set_request_adapter has no concrete implementation anywhere yet.""" - assert "set_request_adapter" not in vars(LocalHFBackend) - assert "set_request_adapter" not in vars(OpenAIBackend) + assert "activate_peft_adapter" not in vars(OpenAIBackend) + assert "deactivate_peft_adapter" not in vars(OpenAIBackend) def test_default_adapter_activation_lock_is_a_noop(): diff --git a/test/backends/test_adapters/test_core_types.py b/test/backends/test_adapters/test_core_types.py index 72fe311ac..fee071244 100644 --- a/test/backends/test_adapters/test_core_types.py +++ b/test/backends/test_adapters/test_core_types.py @@ -119,10 +119,9 @@ def deactivate(self) -> None: PartialBinding() # type: ignore[abstract] -@pytest.mark.parametrize("cls", [EmbeddedBinding, ServerMediatedBinding]) @pytest.mark.parametrize("verb", ["prepare", "activate", "deactivate", "release"]) -def test_stub_binding_subclasses_raise_not_implemented(cls, verb): - binding = cls() +def test_server_mediated_binding_raises_not_implemented(verb): + binding = ServerMediatedBinding() with pytest.raises(NotImplementedError, match="Phase 0 stub"): getattr(binding, verb)() @@ -130,7 +129,15 @@ def test_stub_binding_subclasses_raise_not_implemented(cls, verb): def test_local_file_binding_not_a_phase_0_stub(): # LocalFileBinding graduated out of the stub set in Epic #929 Phase 2 # (issue #1141) — see test_local_file_binding.py for its real behavior. - assert LocalFileBinding.prepare is not EmbeddedBinding.prepare + assert LocalFileBinding.prepare is not ServerMediatedBinding.prepare + + +def test_embedded_binding_is_not_a_weights_binding(): + # EmbeddedBinding graduated out of the WeightsBinding stub set in Epic #929 + # Phase 2 (issue #1142) — it has no weights lifecycle at all, so it is not + # even a WeightsBinding subclass. See test_embedded_binding.py for its real + # behavior (`apply_activation`). + assert not isinstance(EmbeddedBinding(), WeightsBinding) def test_adapter_schema_mismatch_error_format(): diff --git a/test/backends/test_adapters/test_embedded_adapter.py b/test/backends/test_adapters/test_embedded_adapter.py index 734cddba0..a70e8352f 100644 --- a/test/backends/test_adapters/test_embedded_adapter.py +++ b/test/backends/test_adapters/test_embedded_adapter.py @@ -468,19 +468,6 @@ def test_list_adapters(self, backend): ) assert set(backend.list_adapters()) == {"answerability_alora", "citations_lora"} - def test_render_controls_is_noop(self, backend): - """render_controls succeeds silently for embedded adapters.""" - backend.add_adapter( - EmbeddedIntrinsicAdapter( - "answerability", config=_ANSWERABILITY_CONFIG, technology="alora" - ) - ) - # These should not raise. - backend.render_controls("answerability_alora", active=True) - backend.render_controls("answerability_alora", active=False) - # Adapter is still registered after activating/deactivating. - assert "answerability_alora" in backend._added_adapters - def test_base_model_name(self, backend): assert backend.base_model_name == "granite-switch" diff --git a/test/backends/test_adapters/test_embedded_binding.py b/test/backends/test_adapters/test_embedded_binding.py new file mode 100644 index 000000000..ccac55c61 --- /dev/null +++ b/test/backends/test_adapters/test_embedded_binding.py @@ -0,0 +1,160 @@ +# Copyright IBM Corp. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Unit tests for EmbeddedBinding (Epic #929 Phase 2, issue #1142). + +No real backend or network access. `apply_activation` is exercised directly +against `EmbeddedActivationRequest` instances built in-test. +""" + +from typing import Literal +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest + +from mellea.backends.adapters import ( + EmbeddedActivationRequest, + EmbeddedBinding, + Identity, +) +from mellea.plugins.types import HookType + + +def _identity( + name: str = "answerability", adapter_type: Literal["lora", "alora"] = "alora" +) -> Identity: + return Identity(name=name, adapter_type=adapter_type, capability=name) + + +async def test_apply_activation_sets_adapter_name(): + binding = EmbeddedBinding() + request = EmbeddedActivationRequest(extra_body={}, api_params={}) + + await binding.apply_activation(request, _identity("answerability")) + + assert request.extra_body["chat_template_kwargs"]["adapter_name"] == "answerability" + + +async def test_apply_activation_removes_model_param(): + # The rewriter config can set `model` to the adapter name; for an embedded + # adapter the real model is the base model already being served, so this + # must be dropped rather than sent to the API. + binding = EmbeddedBinding() + request = EmbeddedActivationRequest( + extra_body={}, api_params={"model": "answerability_alora", "seed": 1} + ) + + await binding.apply_activation(request, _identity("answerability")) + + assert "model" not in request.api_params + assert request.api_params["seed"] == 1 + + +async def test_apply_activation_preserves_existing_chat_template_kwargs(): + binding = EmbeddedBinding() + request = EmbeddedActivationRequest( + extra_body={"chat_template_kwargs": {"enable_thinking": True}}, api_params={} + ) + + await binding.apply_activation(request, _identity("citations")) + + ctk = request.extra_body["chat_template_kwargs"] + assert ctk["enable_thinking"] is True + assert ctk["adapter_name"] == "citations" + + +async def test_apply_activation_handles_explicit_none_chat_template_kwargs(): + # The `or {}` branch: an explicit None value (not a missing key) is + # normalised to a dict rather than crashing on assignment. + binding = EmbeddedBinding() + request = EmbeddedActivationRequest( + extra_body={"chat_template_kwargs": None}, api_params={} + ) + + await binding.apply_activation(request, _identity("answerability")) + + assert request.extra_body["chat_template_kwargs"]["adapter_name"] == ( + "answerability" + ) + + +def test_no_weights_verbs_on_embedded_binding(): + binding = EmbeddedBinding() + for verb in ("prepare", "activate", "deactivate", "release"): + assert not hasattr(binding, verb), f"EmbeddedBinding must not have {verb!r}" + + +async def test_multi_call_isolation(): + # EmbeddedBinding is stateless across calls: activating one adapter must not + # leak into the request built for the next call. + binding = EmbeddedBinding() + + request_one = EmbeddedActivationRequest(extra_body={}, api_params={"model": "m"}) + await binding.apply_activation(request_one, _identity("answerability")) + + request_two = EmbeddedActivationRequest(extra_body={}, api_params={"model": "m"}) + await binding.apply_activation( + request_two, _identity("citations", adapter_type="lora") + ) + + assert request_one.extra_body["chat_template_kwargs"]["adapter_name"] == ( + "answerability" + ) + assert request_two.extra_body["chat_template_kwargs"]["adapter_name"] == ( + "citations" + ) + + +def test_from_base_model_records_backend_base_model_name(): + backend = MagicMock(base_model_name="granite-switch") + binding = EmbeddedBinding.from_base_model(backend) + assert binding.source == "granite-switch" + + +async def test_apply_activation_fires_phase_complete_metric(): + # AdapterFunctionMetricsPlugin (mellea/telemetry/metrics_plugins.py) hooks + # into `adapter_function_phase_complete` to record the + # `mellea.adapter_function.phase_duration` histogram. Pin that + # apply_activation fires it correctly for the "activate" phase. + pytest.importorskip("cpex", reason="cpex not installed — install mellea[hooks]") + binding = EmbeddedBinding() + request = EmbeddedActivationRequest(extra_body={}, api_params={}) + + with ( + patch("mellea.backends.adapters._core.has_plugins", return_value=True), + patch( + "mellea.backends.adapters._core.invoke_hook", new_callable=AsyncMock + ) as mock_invoke, + ): + await binding.apply_activation(request, _identity("answerability")) + + mock_invoke.assert_awaited_once() + assert mock_invoke.call_args.args[0] is HookType.ADAPTER_FUNCTION_PHASE_COMPLETE + payload = mock_invoke.call_args.args[1] + assert payload.name == "answerability" + assert payload.phase == "activate" + + +async def test_apply_activation_does_not_fire_invocation_complete(): + # apply_activation only edits the request — the real generate+parse + # outcome isn't known yet at this point (OpenAIBackend resolves it later, + # lazily, when the caller awaits the ModelOutputThunk). Firing + # `adapter_function_invocation_complete` here would have to guess an + # `outcome`, which would misreport failed calls as "success". Pin that it + # doesn't, so nobody "fixes" this back to a hardcoded success outcome + # without solving the underlying problem (see the docstring on + # apply_activation for the follow-up this needs). + pytest.importorskip("cpex", reason="cpex not installed — install mellea[hooks]") + binding = EmbeddedBinding() + request = EmbeddedActivationRequest(extra_body={}, api_params={}) + + with ( + patch("mellea.backends.adapters._core.has_plugins", return_value=True), + patch( + "mellea.backends.adapters._core.invoke_hook", new_callable=AsyncMock + ) as mock_invoke, + ): + await binding.apply_activation(request, _identity("answerability")) + + fired_hook_types = [call.args[0] for call in mock_invoke.call_args_list] + assert HookType.ADAPTER_FUNCTION_INVOCATION_COMPLETE not in fired_hook_types diff --git a/test/backends/test_adapters/test_embedded_integration.py b/test/backends/test_adapters/test_embedded_integration.py new file mode 100644 index 000000000..dd07591f5 --- /dev/null +++ b/test/backends/test_adapters/test_embedded_integration.py @@ -0,0 +1,132 @@ +# Copyright IBM Corp. All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Integration test: OpenAIBackend activating embedded adapters through the +new EmbeddedBinding (Epic #929 Phase 2, issue #1142). + +A real `OpenAIBackend` and its adapter registration/generation path are used; +only the outer network boundary (the OpenAI async client) is mocked, per +test/README.md's definition of `integration`. No vLLM server or Granite +Switch model is required — see `test/backends/test_openai_intrinsics.py` for +the GPU-backed e2e counterpart. +""" + +from unittest.mock import AsyncMock, MagicMock, PropertyMock, patch + +import pytest +from openai.types.chat import ChatCompletion, ChatCompletionMessage +from openai.types.chat.chat_completion import Choice +from openai.types.completion_usage import CompletionUsage + +from mellea.backends.adapters import EmbeddedBinding, ServerMediatedBinding +from mellea.backends.adapters.adapter import EmbeddedIntrinsicAdapter +from mellea.backends.openai import OpenAIBackend +from mellea.stdlib import functional as mfuncs +from mellea.stdlib.components import Intrinsic, Message +from mellea.stdlib.context import ChatContext + +pytestmark = pytest.mark.integration + +_SIMPLE_CONFIG = { + "model": None, + "response_format": None, + "transformations": None, + "instruction": None, + "parameters": {"max_completion_tokens": 64}, + "sentence_boundaries": None, +} + + +def _chat_completion(content: str = '{"result": "ok"}') -> ChatCompletion: + return ChatCompletion( + id="test-embedded-integration", + created=0, + model="granite-switch", + object="chat.completion", + choices=[ + Choice( + index=0, + finish_reason="stop", + message=ChatCompletionMessage(role="assistant", content=content), + ) + ], + usage=CompletionUsage(prompt_tokens=10, completion_tokens=4, total_tokens=14), + ) + + +def _backend_with_adapter(technology: str) -> OpenAIBackend: + backend = OpenAIBackend( + model_id="granite-switch", + api_key="fake-key", + base_url="http://localhost:9999/v1", + ) + backend.add_adapter( + EmbeddedIntrinsicAdapter( + intrinsic_name="answerability", config=_SIMPLE_CONFIG, technology=technology + ) + ) + return backend + + +@pytest.mark.parametrize("technology", ["lora", "alora"]) +async def test_activation_goes_through_embedded_binding(technology): + """The registered adapter's weights are a real EmbeddedBinding, and it is + that binding's `apply_activation` — not an inline isinstance check — that + ends up writing the request the API call receives.""" + backend = _backend_with_adapter(technology) + adapter = backend._added_adapters[f"answerability_{technology}"] + assert isinstance(adapter.weights, EmbeddedBinding) + # add_adapter stamps the registration-time source (openai.py), distinct + # from the from_base_model() classmethod covered in test_embedded_binding.py. + assert adapter.weights.source == "granite-switch" + + mock_create = AsyncMock(return_value=_chat_completion()) + mock_client = MagicMock() + mock_client.chat.completions.create = mock_create + + original_apply_activation = EmbeddedBinding.apply_activation + with ( + patch.object( + OpenAIBackend, + "_async_client", + new_callable=PropertyMock, + return_value=mock_client, + ), + patch.object( + EmbeddedBinding, + "apply_activation", + autospec=True, + side_effect=original_apply_activation, + ) as mock_apply, + ): + ctx = ChatContext().add(Message("user", "What is the square root of 4?")) + mot, _ = await mfuncs.aact( + Intrinsic("answerability"), ctx, backend, strategy=None + ) + await mot.avalue() + + mock_apply.assert_called_once() + _, called_identity = mock_apply.call_args.args[1:] + assert called_identity.name == "answerability" + + call_kwargs = mock_create.call_args.kwargs + assert call_kwargs["extra_body"]["chat_template_kwargs"]["adapter_name"] == ( + "answerability" + ) + assert call_kwargs["model"] == "granite-switch" + + +async def test_reassigned_weights_fail_loudly(): + """Reassigning `.weights` off the EmbeddedBinding must fail at generation. + + The shim permits attribute mutation, so a caller reassigning `.weights` + after construction must hit the explicit TypeError rather than silently + skipping activation and sending an unactivated request (issue #1142). + """ + backend = _backend_with_adapter("alora") + adapter = backend._added_adapters["answerability_alora"] + adapter.weights = ServerMediatedBinding() + + ctx = ChatContext().add(Message("user", "What is the square root of 4?")) + with pytest.raises(TypeError, match=r"weights must be an EmbeddedBinding"): + await mfuncs.aact(Intrinsic("answerability"), ctx, backend, strategy=None) diff --git a/test/backends/test_adapters/test_shims.py b/test/backends/test_adapters/test_shims.py index b7bdfd93f..1708d590f 100644 --- a/test/backends/test_adapters/test_shims.py +++ b/test/backends/test_adapters/test_shims.py @@ -331,11 +331,35 @@ def test_adapter_scope_raises_for_a_shim_backed_adapter(): mock_backend = MagicMock(spec=AdapterMixin) adapter = _make_intrinsic_adapter("answerability") - with pytest.raises(NotImplementedError, match="Phase 2"): + with pytest.raises(NotImplementedError, match="WeightsBinding not yet implemented"): with AdapterMixin.adapter_scope(mock_backend, adapter): pytest.fail("body must not run when the shim's activate() raises") +def test_adapter_scope_rejects_an_embedded_binding(): + """adapter_scope drives the WeightsBinding lifecycle; EmbeddedBinding has none. + + Deliberate behaviour change from #1142: `adapter_scope` used to raise + `NotImplementedError` from `_ShimWeightsBinding.activate()` for an + `EmbeddedIntrinsicAdapter` and fire `invocation_complete(outcome="error")`; + it now raises `TypeError` before entering the try/finally and fires + nothing. This pins the new guard (`adapter.py`'s `isinstance(adapter.weights, + WeightsBinding)` check) rather than leaving it to surface as an + `AttributeError` from `adapter.weights.activate()` the next time someone + wires embedded generation through this scope. + """ + mock_backend = MagicMock(spec=AdapterMixin) + with warnings.catch_warnings(): + warnings.simplefilter("ignore", DeprecationWarning) + adapter = EmbeddedIntrinsicAdapter( + "answerability", config={}, technology="alora" + ) + + with pytest.raises(TypeError, match=r"have no activate\(\)/deactivate\(\)"): + with AdapterMixin.adapter_scope(mock_backend, adapter): + pytest.fail("body must not run for a binding with no lifecycle") + + def test_resolve_adapter_returns_existing_by_capability(): """resolve_adapter must return an already-registered adapter without creating a new one.""" existing = _make_intrinsic_adapter("answerability") diff --git a/test/backends/test_openai_intrinsics_unit.py b/test/backends/test_openai_intrinsics_unit.py index 3577a16dc..35a0934dd 100644 --- a/test/backends/test_openai_intrinsics_unit.py +++ b/test/backends/test_openai_intrinsics_unit.py @@ -543,6 +543,63 @@ async def test_user_extra_body_merges_into_intrinsic_extra_body(): assert call_kwargs["reasoning_effort"] == "medium" +async def test_user_extra_body_cannot_override_embedded_adapter(): + """The resolved embedded adapter overrides a caller-supplied adapter_name.""" + backend = _make_backend_with_adapter(_SIMPLE_CONFIG) + ctx = _make_context() + mock_create = AsyncMock(return_value=_simple_chat_completion()) + + mock_client = MagicMock() + mock_client.chat.completions.create = mock_create + + with patch.object( + OpenAIBackend, + "_async_client", + new_callable=PropertyMock, + return_value=mock_client, + ): + mot, _ = await mfuncs.aact( + Intrinsic("answerability"), + ctx, + backend, + strategy=None, + model_options={ + "extra_body": { + "chat_template_kwargs": {"adapter_name": "caller-selected"} + } + }, + ) + await mot.avalue() + + extra_body = mock_create.call_args.kwargs["extra_body"] + assert extra_body["chat_template_kwargs"]["adapter_name"] == "answerability" + + +@pytest.mark.parametrize( + ("field", "value"), + [("model", "other-model"), ("messages", []), ("stream", True), ("tools", [])], +) +async def test_intrinsic_extra_body_rejects_protected_request_fields( + field: str, value: object +): + """Provider-specific extra_body values cannot override intrinsic invariants.""" + backend = _make_backend_with_adapter(_SIMPLE_CONFIG) + ctx = _make_context() + + with pytest.raises( + ValueError, + match=rf"extra_body cannot override intrinsic request fields: {field}", + ): + mot, _ = await mfuncs.aact( + Intrinsic("answerability"), + ctx, + backend, + strategy=None, + model_options={"extra_body": {field: value}}, + ) + await mot.avalue() + + async def test_user_extra_body_without_thinking(): """User extra_body merges when THINKING is unset; the #1241 reproduction.""" backend = _make_backend_with_adapter(_SIMPLE_CONFIG)