refactor(backends)!: narrow AdapterMixin verbs, centralize option resolution, add IntrinsicMetricsPlugin skeleton

[planetf1]

Pull Request

Issue

Fixes #1140

Summary

What this unlocks: it turns Mellea's ad-hoc, per-backend adapter handling into a uniform, reality-aware lifecycle — the clean verb surface, shared option-merge, and observability contract that the upcoming Binding types (#1141/#1142) plug into. This is the Phase 2 seam that makes the rest of Epic #929's first-class adapter/intrinsic support buildable.

What it does (Epic #929, Phase 2): narrows the AdapterMixin verb surface so each backend exposes only the adapter operations that are real for its deployment ("adapter reality"), centralises the model-option merge shared across backends, and adds an observability skeleton (metrics + hooks) for adapter-function invocations. There is no behavioural change beyond the declared breaking renames, and the metrics skeleton has no production callers yet.

What to look at (review focus)

1. add_adapter contract — adapter.py / huggingface.py / openai.py. The key design call, and the resolution of the concern raised in review: the base advertised Adapter | _AdapterCore but each backend narrowed it, so a statically valid call could fail at runtime. Fix = accept the full union, reject unsupported realities at runtime with a clear TypeError. Please sanity-check this against the generic AdapterMixin[A] alternative (rationale in Details & notes).
2. Breaking renames + silent list_adapters() shift — the ! surface. Confirm the migration table matches expectations; note list_adapters() changes meaning (registered vs loaded) without changing its signature.
3. AC deviation (4 vs 7 verbs) — a call worth a reviewer's eye: the issue says four verbs, this ships seven, because Phase 1's resolve_adapter() depends on two of the universal verbs staying on the mixin. See Deviations from refactor(backends): AdapterMixin verb rename/narrow + resolve_model_options + IntrinsicMetricsPlugin (Epic #929 Phase 2) #1140 below for the full list of intentional departures.
4. Metrics + hooks are a skeleton — no production caller fires them yet (that's feat(backends): LocalFileBinding implements verbs (PEFT/aLoRA path) + from_catalog() + OTel spans (Epic #929 Phase 2) #1141/feat(backends): EmbeddedBinding implements verbs (Granite Switch path) + OTel spans (Epic #929 Phase 2) #1142), so review the contract/shape, not live behaviour.

How this fits Epic #929

The epic replaces ad-hoc adapter handling with an explicit lifecycle:

• Phase 0 (feat(intrinsics): introduce Adapter/Identity/IOContract/WeightsBinding scaffolding (Epic #929 Phase 0) #1134) — catalogue and scaffolding.
• Phase 1 (refactor(intrinsics): internal migration with shims — old classes inherit from Adapter; call_intrinsic rewritten (Epic #929 Phase 1) #1136, refactor(intrinsics): rag.py whole-file migration to new Adapter types (Epic #929 Phase 1) #1137) — compatibility shims, the call_intrinsic rewrite, and the RAG intrinsic migration.
• Phase 2 (this PR, refactor(backends): AdapterMixin verb rename/narrow + resolve_model_options + IntrinsicMetricsPlugin (Epic #929 Phase 2) #1140) — narrow the backend verb surface per adapter reality and add the observability skeleton, so the upcoming Binding types can plug into the reality-specific verbs: render_controls(name, active) will back EmbeddedBinding.activate()/deactivate() (feat(backends): EmbeddedBinding implements verbs (Granite Switch path) + OTel spans (Epic #929 Phase 2) #1142), and load_peft_adapter/unload_peft_adapter back the LocalFile/PEFT binding (feat(backends): LocalFileBinding implements verbs (PEFT/aLoRA path) + from_catalog() + OTel spans (Epic #929 Phase 2) #1141).

Key changes

1. AdapterMixin → 7 verbs, split by responsibility:
   • Universal (abstract — every backend must implement): base_model_name, add_adapter, list_adapters.
   • Reality-specific (concrete default that raises NotImplementedError; only the relevant backend overrides): load_peft_adapter/unload_peft_adapter (LocalFile/PEFT), render_controls(name, active) (Embedded/Granite Switch), set_request_adapter (ServerMediated — not yet implemented by any backend).
   • LocalHFBackend.list_adapters() now reports registered adapters rather than only loaded ones, matching OpenAIBackend's existing contract.
2. resolve_model_options (mellea/backends/_options.py, new) — centralises the backend_defaults < helper_defaults < call_options precedence merge previously duplicated in each backend's _simplify_and_merge. Behaviour-preserving; adopted by both backends and call_intrinsic.
3. IntrinsicMetricsPlugin skeleton — three OTel metrics (mellea.adapter_function.invocations, mellea.adapter_function.phase_duration, mellea.adapter_function.parse_failures) via two new hook types. Unit-tested against synthetic payloads plus an in-memory OTel exporter; no production call site fires these hooks yet (wired in feat(backends): LocalFileBinding implements verbs (PEFT/aLoRA path) + from_catalog() + OTel spans (Epic #929 Phase 2) #1141/feat(backends): EmbeddedBinding implements verbs (Granite Switch path) + OTel spans (Epic #929 Phase 2) #1142). (Metric names use the glossary's canonical adapter_function; the code symbols stay Intrinsic* — see Deviations. A further rename of these specific symbols, e.g. IntrinsicMetricsPlugin → AdapterFunctionMetricsPlugin, is under discussion in review and will follow as a separate commit once agreed.)

See docs/dev/adapter_observability.md for the full verb contract, option-resolution precedence, span-tree structure, MELLEA_TRACE_CONTENT content-capture gate, and metric/label reference.

Breaking changes (refactor(backends)!)

Removing the old verb names outright — rather than keeping deprecated aliases — is a deliberate design choice: no first-party code calls them (verified across the core repo, docs/examples/, the mellea-examples repo, and mellea-website), and aliases would re-introduce the reality-agnostic names this narrowing is meant to retire. Whether backend-API breaks should follow a formal deprecation window in future is raised for reviewer discussion (Mellea uses DeprecationWarning shims elsewhere, e.g. Watsonx and create_bedrock_mantle_backend).

Backend (adapter reality)	Old call	New call
LocalHFBackend (LocalFile/PEFT)	backend.load_adapter(name)	backend.load_peft_adapter(name)
LocalHFBackend	backend.unload_adapter(name)	backend.unload_peft_adapter(name)
OpenAIBackend (Embedded/Switch)	backend.load_adapter(name)	backend.render_controls(name, active=True)
OpenAIBackend	backend.unload_adapter(name)	backend.render_controls(name, active=False)

A removed verb now raises AttributeError. A reality-specific verb called on a backend whose reality doesn't support it raises a descriptive NotImplementedError naming the backend — a deliberate improvement over a bare AttributeError. add_adapter now raises a clear TypeError when handed an adapter type the backend doesn't support (previously a mismatched type failed less clearly) — a runtime-contract tightening, not a new restriction.

list_adapters() changes silently: same signature (() -> list[str]), but it now returns registered rather than only loaded adapters. First-party callers are unaffected (the two docs/examples/ uses are "is it registered?" guards the new meaning makes more correct); external callers relying on the old "loaded" meaning should review their usage.

Process note: the migration mapping currently lives here and in the dev-internal docs/dev/adapter_observability.md. Where end-user breaking changes should surface (changelog, release notes) is an open question to settle separately — the repo's auto-generated changelog has no "Breaking Changes" category today. Tracked in #1436.

Details & notes

• add_adapter contract (honesty). The mixin advertises the full Adapter | _AdapterCore union. These are two disjoint adapter hierarchies co-existing during the Epic Epic: Fix Adapter Function Lifecycle & Consistency in Mellea #929 old→new migration (the legacy abc.ABC Adapter/LocalHFAdapter on one side, the core dataclass Adapter/EmbeddedIntrinsicAdapter on the other — verified: neither is a subclass of the other). Each backend accepts that full union and raises a clear TypeError for the adapter realities it doesn't implement — mirroring how the reality-specific verbs raise NotImplementedError. This keeps the advertised contract truthful and removes the # type: ignore[override] the earlier narrowing required.
  • Why runtime rejection, not a generic AdapterMixin[A]? A generic mixin parameterised per backend would give compile-time safety and also drop the type-ignores, but it's more invasive (touches class definitions and resolve_adapter), and the eventual old→new Adapter hierarchy unification would likely rework it. We chose the smaller, pattern-consistent runtime-rejection fix here and flag AdapterMixin[A] as the type-safe direction for that later hierarchy-consolidation work.

Deviations from #1140 (all deliberate)

Every difference from the issue as written is intentional and traceable:

• 7 verbs, not "exactly four." The issue itself sanctions relocating the universal verbs "with documented justification"; Phase 1 (feat(intrinsics): shim old adapters onto Adapter dataclass; call_intrinsic rewritten (Epic #929 Phase 1) #1269) added resolve_adapter(), which depends on base_model_name/add_adapter staying on the mixin. Corrected in a comment on refactor(backends): AdapterMixin verb rename/narrow + resolve_model_options + IntrinsicMetricsPlugin (Epic #929 Phase 2) #1140 and documented in adapter_observability.md.
• Plugin lives in mellea/telemetry/metrics_plugins.py, not the issue's mellea/core/plugins/intrinsic_metrics.py — co-located with the sibling metrics plugins (TokenMetricsPlugin, LatencyMetricsPlugin, …) whose registration pattern it mirrors.
• Metric names are mellea.adapter_function.* (e.g. …phase_duration, unit s), not the issue's mellea.intrinsic.* / phase_duration_ms. Two deliberate alignments: (a) the glossary makes "adapter function" the canonical term for user-facing surfaces (metric/label/span names, prose), while the Python symbols stay Intrinsic* — the glossary's "current implementation name" — matching the existing call_intrinsic/Intrinsic/components/intrinsic code; (b) durations use the OTel base unit s, matching mellea.llm.request.duration/ttfb. Done now because the metrics have no producer/dashboard yet and no separate issue tracks the terminology. The payload still carries ms and converts at the recording boundary. (Under review discussion: whether the Intrinsic* code symbols introduced in this PR specifically — since they're unreleased — should be renamed to AdapterFunction* now rather than waiting for the coordinated future-phase rename.)
• Content-capture gate is the existing MELLEA_TRACES_CONTENT (plural, already implemented), not the MELLEA_TRACE_CONTENT named in refactor(backends): AdapterMixin verb rename/narrow + resolve_model_options + IntrinsicMetricsPlugin (Epic #929 Phase 2) #1140's AC — that variable doesn't exist. The intrinsic spans reuse the existing gate (which also honours OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT) rather than introducing a new one.
• Span names/attributes follow Mellea's existing tracing conventions (start_*_span helpers, mellea.* attributes) rather than a bespoke scheme; the doc documents the structure, emission lands with feat(backends): LocalFileBinding implements verbs (PEFT/aLoRA path) + from_catalog() + OTel spans (Epic #929 Phase 2) #1141/feat(backends): EmbeddedBinding implements verbs (Granite Switch path) + OTel spans (Epic #929 Phase 2) #1142.

Other notes

• render_controls(name, active: bool) signature is a design judgement (the issue names the verb, not its shape); expected to map onto feat(backends): EmbeddedBinding implements verbs (Granite Switch path) + OTel spans (Epic #929 Phase 2) #1142's activate()/deactivate().

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]

CI on this PR is currently red at the Install Ollama step — this is not caused by anything in this change. The quality workflow on main pins OLLAMA_VERSION=0.32.2-rc0, and Ollama has since removed that release-candidate tag (promoted to the v0.32.2 release), so install.sh 404s and the step fails on every PR and merge-queue run.

Fix is up in #1425 (bumps the pin to 0.32.2; tracked by #1424). Once that merges, a re-run — or a rebase onto main — will take this PR's CI green, with no change needed here.

[jakelorocco]

It seems like these "reality-specific" verbs shouldn't live at the AdapterMixin level but should live as functions on distinct subclasses?

[jakelorocco]

I'm actually okay with leaving it as is for now. I think we should see how users interact with these interfaces, but it may eventually make sense to add subclassing to the AdapterMixin type so that users get better type completion, etc...

[planetf1]

This was the main thing I wanted a view on - there's a note in the PR description ("Why not a generic AdapterMixin[A]?") with the tradeoff. The generic/subclassed version would give the better type completion you mention, but it's more invasive and the old/new Adapter hierarchy is still being unified, so I think we'd end up redoing it once that settles.

I could make the change here (and change it again later if needed), or open an issue and add it to the epic to do at the end. Or we leave it as is?

[jakelorocco]

I'm okay with postponing for now. I think we can see if the type completion / typical use cases are fine with this approach and if not, iterate.

[jakelorocco]

This seems to lock us into the hf backend supporting only peft adapters. I wonder if we should return these back to being generic (ie load_adapter or activate_adapter) and then have each adapter subclass implement these functions if necessary?

For instance, if we want to support both embedded adapters and peft adapters, we need the implementation to live somewhere. I guess either the backend can do the if/else or the adapter can be aware of the backend. I suppose it's cleaner for the backend to make these decisions.

[planetf1]

Agreed, the backend making the decision is what's here. It's not locked to peft though - a backend can support both, so when we add embedded adapters to HF (#1018) it'd just implement render_controls alongside the peft verbs. The mixin only raises for the verbs a given backend doesn't implement.

[jakelorocco]

Is there a reason the phase can be "" here? The payload above forces it's outcome literal to not be "".

[planetf1]

Fair point - I only just added this in the previous commit, so no need to preserve the "". The default was unused anyway. Went through the other fields and came up with:

• phase - dropped the "", now required
• name (both) and duration_ms - same dead default, made required since a completion event always has them
• outcome - required too. "success" felt like a dangerous default, a forgotten emit would just look like it succeeded - and the caller always knows the real outcome anyway, so no default needed
• revision - left optional (None = genuinely unpinned), but the label now says "unpinned" rather than "unknown" so it's not muddled with a revision we couldn't work out
• error - left as None, since that's just "no error", and it's the one default that's actually used

[ajbozarth]

Didn't find time to review today, will take a look tomorrow

[ajbozarth]

Some feedback from Claude.

Action items inline. I'll do a separate deeper pass on the metrics/hooks side — the telemetry notes below are shape-level only for now.

[ajbozarth]

Follow-up from my telemetry deep dive — requesting changes. The public metric/plugin naming (inline below) is the highest-stakes item, since renaming after merge breaks the API. Also a couple of missing-test items inline.

Two more in-this-PR items that land on files the diff doesn't touch (so no inline anchor):

• mellea/telemetry/__init__.py — the three new record_intrinsic_* functions aren't re-exported from the package. All sibling record_* functions are in the package __all__; these are only in metrics.py's __all__. Add them — the emission wiring will likely import them.
• test/telemetry/test_metrics.py::test_metric_instruments_lazy_initialization — asserts every instrument global starts None and becomes non-None after its record_* call, but the three new _intrinsic_* globals aren't in it. Extend it to cover them, matching the pattern for every other instrument.

Follow-up items — out of scope for this PR, should become acceptance criteria on the emission-wiring follow-ups (#1141 / #1142):

1. binding_type needs a canonical source. The label values ("local_file" / "embedded" / "server_mediated") currently exist only in docstrings and the payload's "unknown" default. Without a single source of truth (e.g. a property on WeightsBinding), each emission site will hand-write the string and they'll drift. adapter_type (maps to the AdapterType enum) and revision (maps to IntrinsicsCatalogEntry.revision) are already grounded — binding_type is the one that isn't.
2. End-to-end hook-firing test. The unit tests drive the plugin handlers directly; none go through invoke_hook. The follow-up that adds emission should add the call-site test in test/plugins/test_hook_call_sites.py proving the hook actually fires with the expected payload.
3. Justify or drop parse_failures. It's fully derivable from invocations{outcome="schema_error"}, so it's a redundant series unless a pre-aggregated schema-drift alarm is specifically wanted.
4. Phase list is duplicated in four places — the payload Literal, the two record_* docstrings, and adapter_observability.md — with no enforcement they stay in sync. Worth noting the Literal is the source of truth when emission adds phases.

[ajbozarth]

Telemetry naming should move off the deprecated Intrinsic* term. Keeping the hooks as intrinsic_* is defensible if they'll be emitted from functions still named that (call_intrinsic etc.) — matching the emitting code. But that reasoning doesn't extend to the telemetry surface, and this is public API: IntrinsicMetricsPlugin, the registration name "intrinsic_metrics", and the IntrinsicInvocationCompletePayload / IntrinsicPhaseCompletePayload payload types should be AdapterFunction* / "adapter_function_metrics". The metric/label names already made this move (mellea.adapter_function.*); the public Python symbols should follow. Renaming a public plugin/payload after consumers depend on it is a breaking change — cheapest to fix now.

[planetf1]

Agreed, and after digging into the glossary more carefully, here's the fuller picture — and the scope should be a bit wider than either of us first proposed.

• The actual documented policy (docs/docs/reference/glossary.md) is: string surfaces (metric names, labels, prose) use adapter_function now; Python symbols stay Intrinsic* until a coordinated, shim-backed rename in a later Epic Epic: Fix Adapter Function Lifecycle & Consistency in Mellea #929 phase. So "new code doesn't need to follow the old-symbol rule" isn't really the right justification — by that policy, these symbols would normally stay Intrinsic* too.
• The justification that does hold: none of this has shipped yet. Renaming now costs nothing and avoids ever releasing the deprecated name or needing a future shim for it — that's a narrower, cleaner reason than "it's new code," and it only applies because nothing depends on these names today.
• Scope needs to be complete, not partial. Beyond IntrinsicMetricsPlugin/"intrinsic_metrics"/the two payload types, this also catches record_intrinsic_invocation, record_intrinsic_parse_failure, record_intrinsic_phase_duration (all public, in metrics.py.__all__), the HookType enum constants (INTRINSIC_INVOCATION_COMPLETE/INTRINSIC_PHASE_COMPLETE), and the hooks/__init__.py re-exports. Renaming the plugin but leaving these would just move the inconsistency rather than fix it.
• Hook name strings too — deciding now rather than deferring to feat(backends): LocalFileBinding implements verbs (PEFT/aLoRA path) + from_catalog() + OTel spans (Epic #929 Phase 2) #1141/feat(backends): EmbeddedBinding implements verbs (Granite Switch path) + OTel spans (Epic #929 Phase 2) #1142. There's no live emitter either way, so there's nothing to defer for, and leaving it open just risks a different author defaulting back to intrinsic_* out of habit.

Will do the full rename across all of the above once we're agreed on the approach.

[ajbozarth]

Agreed — let's do the full rename, all in this PR since it's all net-new and nothing's shipped. Here's the complete breakdown I'd expect so we're aligned on scope before you start.

Convention: Intrinsic* → AdapterFunction* for symbols, intrinsic_* → adapter_function_* for strings. This matches the metric names that already went that way (mellea.adapter_function.*).

1. Hook string values (the wire contract — the reason to do this now):

• "intrinsic_invocation_complete" → "adapter_function_invocation_complete"
• "intrinsic_phase_complete" → "adapter_function_phase_complete"
• plugin registration name "intrinsic_metrics" → "adapter_function_metrics"

2. HookType enum constants (mellea/plugins/types.py, both the definitions at 74-75 and the registry uses at 184/188):

• INTRINSIC_INVOCATION_COMPLETE → ADAPTER_FUNCTION_INVOCATION_COMPLETE
• INTRINSIC_PHASE_COMPLETE → ADAPTER_FUNCTION_PHASE_COMPLETE

3. Public payload types (mellea/plugins/hooks/, defined in intrinsic.py, re-exported from hooks/__init__.py __all__):

• IntrinsicInvocationCompletePayload → AdapterFunctionInvocationCompletePayload
• IntrinsicPhaseCompletePayload → AdapterFunctionPhaseCompletePayload
• module file hooks/intrinsic.py → hooks/adapter_function.py (and its imports in types.py / metrics_plugins.py)

4. Plugin class + handler methods (mellea/telemetry/metrics_plugins.py):

• IntrinsicMetricsPlugin → AdapterFunctionMetricsPlugin (+ the entry in _METRICS_PLUGIN_CLASSES and the module-docstring bullet)
• handler methods record_intrinsic_invocation / record_intrinsic_phase → record_adapter_function_invocation / record_adapter_function_phase

5. Public record functions (mellea/telemetry/metrics.py, exported from telemetry/__init__.py __all__):

• record_intrinsic_invocation → record_adapter_function_invocation
• record_intrinsic_phase_duration → record_adapter_function_phase_duration
• record_intrinsic_parse_failure → record_adapter_function_parse_failure

6. Private module globals + accessors (metrics.py, consistency only — not public):

• _intrinsic_*_counter / _histogram → _adapter_function_* (+ the matching _get_* accessors, and the resets in test/telemetry/conftest.py's reset_metrics_state)

Downstream: the three telemetry test files (test_metrics.py, test_metrics_intrinsic.py, test_metrics_plugins.py) update to the new names.

Explicitly out of scope (stays Intrinsic*, owned by the coordinated shim-backed rename in #1136): the Intrinsic class, IntrinsicAdapter, components/intrinsic/, call_intrinsic. Also unchanged: the mellea.adapter_function.* metric name strings (already correct).

One knock-on we're accepting: the telemetry surface becomes AdapterFunction* while the emitter code stays intrinsic-named until #1136 — a short-lived, self-correcting mismatch, which I'm fine with. If that all matches your plan, go ahead.

[ajbozarth]

@planetf1 thanks for the fixes, I resolved all the items you fixed/gave good pushback on and left a detailed plan comment on the naming item you left open. This LGTM now pending that naming refactor for the telemetry surface