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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 62 additions & 14 deletions docs/dev/adapter_observability.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,19 @@ Epic #929 Phase 2, issue #1140. Covers three things landed together in the
same PR: the narrowed `AdapterMixin` verb contract, the shared
`resolve_model_options` helper, and the `AdapterFunctionMetricsPlugin` skeleton.

Issue #1141 built on top of this: `LocalFileBinding` (PEFT/aLoRA reality) now
has a real `prepare`/`activate`/`deactivate`/`release` lifecycle and a real
`from_catalog()` constructor, `adapter_scope()` really activates/deactivates
weights instead of being a no-op, and the span/metric plumbing described below
actually fires. The sections that were written in future tense against #1141
are updated in place rather than left as a historical record — see each
section for what #1141 changed. `EmbeddedBinding` (#1142, Granite Switch
reality) is still unimplemented. The existing `IntrinsicAdapter` /
`resolve_adapter()` / `_generate_from_intrinsic` production hot path is
**not** rewired onto this machinery yet — it still uses its own inline
`set_adapter()` calls and doesn't open these spans or fire these hooks. That
cutover is issue 4.1's job.

## AdapterMixin verb contract

`AdapterMixin` (`mellea/backends/adapters/adapter.py`) exposes **seven**
Expand Down Expand Up @@ -45,9 +58,15 @@ a backend overrides only the verb matching its own adapter reality.
implements this yet; the verb name is defined for when that reality is
built.

`resolve_adapter()` and `adapter_scope()` are unchanged Phase 1 scaffolding
and out of scope for this issue — their real wiring into
`WeightsBinding.activate()`/`deactivate()` belongs to #1141/#1142.
`resolve_adapter()` is unchanged Phase 1 scaffolding — it still only knows
about `IntrinsicAdapter`/`LocalHFAdapter` and is not used to look up
`LocalFileBinding`/`Adapter` instances. `adapter_scope()` is no longer
scaffolding: as of #1141 it really calls `adapter.weights.activate()` before
the `with` body and `adapter.weights.deactivate()` after (in a `finally`, so
deactivation runs even if the body raises), wrapping both in the span/metric
plumbing described below. Wiring `EmbeddedBinding.activate()`/`deactivate()`
for the Granite Switch reality is #1142's job; `adapter_scope()` itself
doesn't change again for that.

## resolve_model_options

Expand Down Expand Up @@ -83,15 +102,34 @@ default — the same class of bug PR #972 fixed elsewhere.
`schema_error` (i.e. an `AdapterSchemaMismatchError`), acting as a
schema-drift detector.

No production code fires these hooks yet — this is a skeleton, unit-tested
against synthetic payloads only (`test/telemetry/test_metrics_plugins.py`).
Real wiring from `prepare`/`activate`/`generate`/`parse`/`deactivate` is
expected to go in with #1141 (LocalFileBinding) and #1142 (EmbeddedBinding).
As of #1141, `LocalFileBinding.prepare()` and `AdapterMixin.adapter_scope()`'s
`activate`/`deactivate` phases fire `ADAPTER_FUNCTION_PHASE_COMPLETE`, and
`adapter_scope()` fires `ADAPTER_FUNCTION_INVOCATION_COMPLETE` when the parent
scope closes — both through the standard `has_plugins()`-then-`invoke_hook()`
idiom, so the metrics plugin now receives real payloads whenever
`LocalFileBinding` is prepared and activated/deactivated through
`adapter_scope()`. `release()` opens and closes its own
`adapter_function.release` span but does **not** fire a phase-complete metric:
`AdapterFunctionPhaseCompletePayload.phase`'s `Literal` (`prepare` | `activate`
| `generate` | `parse` | `deactivate`) has no `"release"` value, so there's
nothing for `LocalFileBinding.release()` to report against — this is the
existing #1140 contract, not a #1141 oversight. `generate` and `parse` never
fire in this issue either: nothing in production calls `io_contract.parse()`
yet, since the `IntrinsicAdapter` hot path isn't wired onto this machinery
(see the note at the top of this doc). Closing that gap, and wiring the
equivalent hooks for `EmbeddedBinding`, is issue 4.1's and #1142's job
respectively. `test/telemetry/test_metrics_plugins.py` still exercises the
plugin against synthetic payloads directly; it isn't yet exercised through a
real invocation end-to-end.

## Span tree (structure)

Span *emission* ships with the Bindings (#1141/#1142) — no span code lands in
this PR. What this issue fixes is the *shape*, so the traces align with the
Span *emission* for `LocalFileBinding` ships with #1141, via
`start_adapter_function_span`/`finish_adapter_function_span_success`/
`finish_adapter_function_span_error` and
`start_adapter_function_phase_span`/`finish_adapter_function_phase_span` in
`mellea/telemetry/tracing.py`. `EmbeddedBinding` (#1142) still has no span
emission. What #1140 fixed was the *shape*, so the traces align with the
metrics and follow Mellea's existing tracing conventions rather than a bespoke
scheme. Spans are opened through the `start_*_span` helper family in
`mellea/telemetry/tracing.py` (mirroring `start_backend_span` /
Expand All @@ -108,9 +146,13 @@ An invocation opens one parent span with a child span per lifecycle phase:
`mellea.adapter_function.outcome`, mirroring the
`mellea.adapter_function.invocations` counter.
- **Children** (one per phase: `prepare`, `activate`, `generate`, `parse`,
`deactivate`) — each carries `mellea.adapter_function.phase` and
corresponds one-to-one with a `mellea.adapter_function.phase_duration`
histogram sample of the same phase.
`deactivate`, plus `release` which only ever gets a span, never a metric —
see the metrics section above) — each carries `mellea.adapter_function.phase`
and, except for `release`, corresponds one-to-one with a
`mellea.adapter_function.phase_duration` histogram sample of the same phase.
As of #1141, only `prepare`, `activate`, `deactivate`, and `release`
actually open a span for `LocalFileBinding`; `generate`/`parse` are
structurally supported but nothing calls them yet (see above).

Note the deliberate split, consistent with the rest of Mellea: **metric labels
are bare** (`name`, `phase`, `revision`, …) while **span attributes are
Expand All @@ -127,8 +169,14 @@ already use (it also honours `OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT
**off by default**, so traces never capture PII or proprietary content unless
explicitly opted in. When unset or falsey, the phase spans carry metadata only;
when set truthy, they additionally attach the adapter's input/output content.
The adapter-function spans **reuse this gate rather than introducing a new one**;
content attributes are attached when the Bindings (#1141/#1142) emit spans.
The adapter-function spans **reuse this gate rather than introducing a new one**
by design, but as of #1141 no content attributes are attached yet — the
`start_adapter_function_span`/`start_adapter_function_phase_span` helpers only
set the metadata attributes listed above. There's no adapter input/output
content to attach until `generate`/`parse` actually fire, which doesn't happen
in this issue (see above). Wiring `MELLEA_TRACES_CONTENT`-gated content
attributes is deferred to whichever issue first makes `generate`/`parse` fire
in production — expected to be issue 4.1.

(#1140's acceptance criteria named this `MELLEA_TRACE_CONTENT`; the real,
already-implemented variable is `MELLEA_TRACES_CONTENT` — see
Expand Down
235 changes: 222 additions & 13 deletions mellea/backends/adapters/_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,25 @@

import abc
import json
import time
import uuid
import warnings
from dataclasses import dataclass
from typing import Literal
from typing import TYPE_CHECKING, ClassVar, Literal

from ...core import Component
from ...helpers.event_loop_helper import _run_async_in_thread
from ...plugins.manager import has_plugins, invoke_hook
from ...plugins.types import HookType
from ...telemetry.tracing import (
finish_adapter_function_phase_span,
start_adapter_function_phase_span,
)
from .capabilities import KNOWN_CAPABILITIES
from .catalog import AdapterType, fetch_intrinsic_metadata

if TYPE_CHECKING:
from .adapter import AdapterMixin

_PHASE_2_NOT_IMPLEMENTED = (
"{cls} is a Phase 0 stub; implementation lands in Epic #929 Phase 2."
Expand Down Expand Up @@ -193,8 +206,14 @@ class WeightsBinding(abc.ABC):

Concrete implementations are expected to document any deviations from this
contract (e.g. servers that prepare-and-activate atomically).

Attributes:
binding_type (ClassVar[str]): Weight-binding reality identifier used in
adapter-function telemetry (e.g. `"local_file"`).
"""

binding_type: ClassVar[str] = "unknown"

@abc.abstractmethod
def prepare(self) -> None:
"""Prepare the weights for activation (e.g. download or stage them)."""
Expand All @@ -217,32 +236,220 @@ def release(self) -> None:


class LocalFileBinding(WeightsBinding):
"""Stub binding for locally stored adapter weights."""
"""Weights binding for the LocalFile/PEFT reality (Epic #929 Phase 2).

def prepare(self) -> None:
raise NotImplementedError(
_PHASE_2_NOT_IMPLEMENTED.format(cls="LocalFileBinding")
Downloads LoRA/aLoRA adapter weights from a Hugging Face Hub repository and
loads them into a PEFT-capable backend (e.g.
:class:`~mellea.backends.huggingface.LocalHFBackend`) via the
:class:`~mellea.backends.adapters.adapter.AdapterMixin` verb contract.

`prepare()` is session-scoped: call `bind_backend()` once, then `prepare()`.
`activate()`/`deactivate()` are call-scoped, typically driven by
:meth:`~mellea.backends.adapters.adapter.AdapterMixin.adapter_scope`.
`release()` is terminal.

Attributes:
name (str): Adapter function name (e.g. `"answerability"`).
adapter_type (AdapterType): The LoRA variant.
repo_id (str): Hugging Face Hub repository containing the adapter weights.
revision (str): Git revision (branch, tag, or commit SHA) to download.
backend (AdapterMixin | None): Backend this binding is registered
with, set by `prepare()` and cleared by `release()`.
path (str | None): Local filesystem path to the downloaded adapter
weights, set by `prepare()` and cleared by `release()`.
"""

binding_type: ClassVar[str] = "local_file"

def __init__(
self,
name: str = "",
adapter_type: AdapterType = AdapterType.LORA,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Identity.adapter_type is Literal["lora","alora"]. Should this match that?

I may also be misremembering, but an Adapter is defined by an identity, an io contract, and a weights binding. Why does the weights binding defined here redefine a field in the identity portion of the adapter?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same question from the other end: LocalFileBinding carries adapter_type because it needs name + type to build the qualified_name label the verbs key on. Take the label away and the copy has nothing left to do.

Written up in #1486 — better settled there.

repo_id: str = "",
revision: str = "main",
) -> None:
"""Constructs a LocalFileBinding.

Args:
name: Adapter function name (e.g. `"answerability"`).
adapter_type: The LoRA variant.
repo_id: Hugging Face Hub repository containing the adapter weights.
revision: Git revision (branch, tag, or commit SHA) to download.
"""
self.name = name
self.adapter_type = adapter_type
self.repo_id = repo_id
self.revision = revision
self.backend: AdapterMixin | None = None
self.path: str | None = None
self._staged_backend: AdapterMixin | None = None

@property
def qualified_name(self) -> str:
"""Backend-facing adapter identifier, e.g. `"answerability_lora"`."""
return f"{self.name}_{self.adapter_type.value}"

def get_local_hf_path(self, base_model_name: str) -> str:
"""Downloads (or reuses a cached copy of) the adapter weights.

Args:
base_model_name: Base model the adapter is being loaded against.

Returns:
Filesystem path to the local copy of the adapter weights.
"""
from ...formatters.granite import intrinsics

return str(
intrinsics.obtain_lora(
self.name,
base_model_name,
self.repo_id,
revision=self.revision,
alora=self.adapter_type is AdapterType.ALORA,
)
)

def activate(self) -> None:
raise NotImplementedError(
_PHASE_2_NOT_IMPLEMENTED.format(cls="LocalFileBinding")
@classmethod
def from_catalog(cls, name: str) -> "LocalFileBinding":
"""Builds a `LocalFileBinding` from the adapter function catalog.

Args:
name: Adapter function name registered in the catalog.

Returns:
A `LocalFileBinding` configured with the catalog's pinned
`repo_id`, `revision`, and first-listed adapter type.

Raises:
ValueError: `name` is not a registered adapter function.
"""
metadata = fetch_intrinsic_metadata(name)
return cls(
name=name,
adapter_type=metadata.adapter_types[0],
repo_id=metadata.repo_id,
revision=metadata.revision,
)

def bind_backend(self, backend: "AdapterMixin") -> None:
"""Stages the backend that `prepare()` will register this binding with.

Args:
backend: The backend to register with on the next `prepare()` call.
"""
self._staged_backend = backend

def prepare(self) -> None:
"""Downloads the adapter weights and loads them into the staged backend.

Idempotent: a no-op once already prepared.

Raises:
RuntimeError: `bind_backend()` was not called first.
"""
if self.backend is not None:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

prepare()'s idempotency guard relies on add_adapter having set self.backend. But LocalHFBackend.add_adapter has two early-return warning paths (already-added-to-this-backend; duplicate qualified name) that return without setting .backend. If either fires, prepare() continues to load_peft_adapter(...) while self.backend stays None, and a later activate() then wrongly raises "requires prepare() to be called first" despite prepare() having run. Narrow (duplicate registration) and mostly self-limiting, and the fake-backend unit tests can't catch it because their add_adapter double unconditionally sets .backend. Flagging as an observation, not a required change.

return
if self._staged_backend is None:
raise RuntimeError(
"LocalFileBinding.prepare() requires bind_backend() to be called first."
)

call_id = uuid.uuid4().hex
started_at = time.monotonic()
start_adapter_function_phase_span(call_id, "prepare")
try:
self._staged_backend.add_adapter(self)
self._staged_backend.load_peft_adapter(self.qualified_name)
except BaseException as exc:
finish_adapter_function_phase_span(call_id, "prepare", exception=exc)
raise
finish_adapter_function_phase_span(call_id, "prepare")
self._fire_phase_complete("prepare", time.monotonic() - started_at)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ajbozarth, can you please weigh in on the telemetry changes in this PR? I believe we've moved to all telemetry being done through hooks. If that's the case, I think we should potentially re-evaluate here and move to the same approach.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree, I'll post a deep dive review later this afternoon for @planetf1 to address tomorrow


def activate(self) -> None:
"""Loads the adapter weights into the backend for generation.

Raises:
RuntimeError: `prepare()` was not called first.
"""
if self.backend is None:
raise RuntimeError(
"LocalFileBinding.activate() requires prepare() to be called first."
)
with self.backend._adapter_activation_lock():
self.backend.activate_peft_adapter(self.qualified_name)

def deactivate(self) -> None:
raise NotImplementedError(
_PHASE_2_NOT_IMPLEMENTED.format(cls="LocalFileBinding")
)
"""Unloads the adapter weights from the backend.

Raises:
RuntimeError: `prepare()` was not called first.
"""
if self.backend is None:
raise RuntimeError(
"LocalFileBinding.deactivate() requires prepare() to be called first."
)
with self.backend._adapter_activation_lock():
self.backend.deactivate_peft_adapter(self.qualified_name)
Comment on lines +370 to +394

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I left a comment in the backend itself, but I think this is duplicate code. Doesn't the backend already handle requesting a lock during the activation / deactivation of adapters?


def release(self) -> None:
raise NotImplementedError(
_PHASE_2_NOT_IMPLEMENTED.format(cls="LocalFileBinding")
"""Unloads the adapter from the backend and releases all resources.

Idempotent: a no-op if never prepared, or already released.
"""
if self.backend is None:
return

call_id = uuid.uuid4().hex
start_adapter_function_phase_span(call_id, "release")
try:
self.backend.unload_peft_adapter(self.qualified_name)
except BaseException as exc:
finish_adapter_function_phase_span(call_id, "release", exception=exc)
raise
finish_adapter_function_phase_span(call_id, "release")

self.backend = None
self.path = None
self._staged_backend = None

def _fire_phase_complete(self, phase: str, duration_s: float) -> None:
"""Fires `adapter_function_phase_complete` for a phase this binding owns.

Only `"prepare"` is fired from here: `"activate"`/`"deactivate"` are
owned by `AdapterMixin.adapter_scope`, and `"release"` has no phase
metric in the `AdapterFunctionPhaseCompletePayload` contract (Epic #929
Phase 1, issue #1140).

Args:
phase: Lifecycle phase name; must be a valid
`AdapterFunctionPhaseCompletePayload.phase` value.
duration_s: Wall-clock duration of the phase, in seconds.
"""
if not has_plugins(HookType.ADAPTER_FUNCTION_PHASE_COMPLETE):
return

from ...plugins.hooks.adapter_function import (
AdapterFunctionPhaseCompletePayload,
)

payload = AdapterFunctionPhaseCompletePayload(
name=self.name, phase=phase, duration_ms=duration_s * 1000.0
)
hook_coro = invoke_hook(HookType.ADAPTER_FUNCTION_PHASE_COMPLETE, payload)
try:
_run_async_in_thread(hook_coro)
except BaseException:
hook_coro.close()
raise


class EmbeddedBinding(WeightsBinding):
"""Stub binding for weights embedded in a model artifact."""

binding_type: ClassVar[str] = "embedded"

def prepare(self) -> None:
raise NotImplementedError(
_PHASE_2_NOT_IMPLEMENTED.format(cls="EmbeddedBinding")
Expand All @@ -267,6 +474,8 @@ def release(self) -> None:
class ServerMediatedBinding(WeightsBinding):
"""Stub binding for server-managed adapter weights."""

binding_type: ClassVar[str] = "server_mediated"

def prepare(self) -> None:
raise NotImplementedError(
_PHASE_2_NOT_IMPLEMENTED.format(cls="ServerMediatedBinding")
Expand Down
Loading
Loading