feat(backends): LocalFileBinding implements verbs (PEFT/aLoRA path) + from_catalog() (Epic #929 Phase 2)

[planetf1]

Pull Request

Issue

Fixes #1141

Summary

LocalFileBinding — the weights binding for locally-downloaded LoRA/aLoRA adapters — was four NotImplementedError stubs. This PR makes it real: working prepare/activate/deactivate/release verbs, a from_catalog() constructor, and AdapterMixin.adapter_scope() wired to actually activate/deactivate weights and fire metric hooks, instead of being a Phase 1 no-op.

Two smaller, independent bugs came along for the ride because they blocked writing correct tests for the above: IntrinsicAdapter was silently resolving revision="main" instead of forwarding the catalogue's pinned SHA, and LocalFileBinding's own revision default did the same thing.

Spans and the production generation cutover are explicitly not in this PR — see "Where this fits" below for exactly where they land instead.

Where this fits — Epic #929, Phase 2

This is #1141. Two things are deliberately not here, each with its own issue and acceptance criteria — don't file findings against their absence:

• refactor(backends): route intrinsic generation through adapter_scope (Epic #929 Phase 2) #1465 — routes real generation through adapter_scope(). Three problems converge on this one issue:
  • adapter_scope() now activates real weights, so with backend.adapter_scope(backend.resolve_adapter(name)): goes from a no-op to NotImplementedError for every adapter resolve_adapter() currently returns (they carry a Phase-1 shim binding). Deliberate, pinned by test_adapter_scope_raises_for_a_shim_backed_adapter, not a silent regression — nothing today calls adapter_scope() outside tests.
  • adapter_scope() is not atomic: _adapter_activation_lock() is held only inside each activate()/deactivate() verb call, not across the with body, so two concurrent adapter_scope() calls on one backend can interleave. Confirmed with a real two-thread test (test_adapter_scope_is_not_atomic_across_concurrent_calls) rather than left as a theoretical concern. Latent today — nothing calls it concurrently — but real.
  • Widening the lock to fix the second point was tried in this PR and reverted: making _generation_lock reentrant and holding it across the whole scope deadlocks the instant the body does real async generation, because that work runs on mellea's shared event-loop thread, not the calling thread — same-thread reentrancy doesn't help across threads. Confirmed by running test_local_file_e2e.py, which hung. refactor(backends): route intrinsic generation through adapter_scope (Epic #929 Phase 2) #1465's own acceptance criteria already require solving the reentrancy and the atomicity together with the threading model, so that's where this belongs rather than a partial fix here.
• docs(telemetry): document the hook/plugin span-production rule and enforce it (Epic #929) #1464 / feat(telemetry): add complete adapter-function lifecycle observability #1466 — span production. docs(telemetry): document the hook/plugin span-production rule and enforce it (Epic #929) #1464 documents the hook→plugin rule and enforces it in CI; feat(telemetry): add complete adapter-function lifecycle observability #1466 adds the missing start hooks and emits the adapter-function spans from a plugin, once docs(telemetry): document the hook/plugin span-production rule and enforce it (Epic #929) #1464 lands. See "Telemetry" below for why spans aren't in this PR at all.

Also referenced, narrower in scope:

• refactor(intrinsics): resolve the adapter's output contract from the adapter, not a parallel argument (Epic #929 Phase 2) #1516 — the ten module-level Adapter constants in rag.py/guardian.py carry unconfigured placeholder bindings; a stricter identity/weights adapter_type cross-check can't be enforced until those get real bindings.
• refactor(backends): release() leaves the adapter registered; is a binding reusable after release? (Epic #929) #1528 — whether a release()d binding's qualified_name should ever become re-claimable (today it doesn't; release() is terminal by contract).

What changed

Area	Change
LocalFileBinding	prepare/activate/deactivate/release implemented; from_catalog(name) constructor; release() is enforced-terminal (bind_backend/prepare raise if called after release()); prepare() retries only the failed step after a load failure instead of silently no-op'ing forever; prepare()/release() now hold the backend's exclusivity lock around the PEFT load/unload calls, matching activate/deactivate
AdapterMixin	New verbs activate_peft_adapter/deactivate_peft_adapter (extracted from _generate_with_adapter_lock's inline PEFT calls — behaviour-preserving); _adapter_activation_lock() for the exclusivity these verbs document as required
AdapterMixin.adapter_scope()	Real activate → yield → deactivate, with metric hooks (ADAPTER_FUNCTION_PHASE_COMPLETE, ADAPTER_FUNCTION_INVOCATION_COMPLETE) instead of a no-op; a hook-dispatch failure can never strand the adapter active or mask the real outcome (including the invocation-complete hook itself); revision in telemetry is the resolved catalogue pin, not the raw unresolved None
Telemetry	Metric hooks only — see "Telemetry" below
IntrinsicAdapter	Forwards the catalogue's pinned revision to obtain_io_yaml/obtain_lora instead of implicitly resolving "main"
mellea/helpers/event_loop_helper.py	_run_async_in_thread's shared coroutine-close handling covers all 13 real call sites instead of 3, including its own internal wrapper coroutine (not just the caller's); narrowed to except Exception (not BaseException) to avoid a cross-thread race on KeyboardInterrupt

Telemetry: what changed and why

The original branch called start_adapter_function_span(...) directly from mellea/backends/adapters/. Wrong mechanism — since #1181, span production belongs to plugins subscribing to hooks, and library code never opens a span itself. The mechanism was more wrong than the diff made it look:

• Until refactor(telemetry)!: move backend tracing onto plugin/hook pattern #1181, calling the span helpers directly was the pattern — all five backends had a bare span = start_generate_span(...).
• refactor(telemetry)!: move backend tracing onto plugin/hook pattern #1181 moved that onto hooks and plugins correctly, but never updated docs/dev/adapter_observability.md, which still named start_backend_span/start_action_span as the model to mirror. This PR's first draft followed that stale doc.
• Even compliant, it couldn't have worked: ADAPTER_FUNCTION_INVOCATION_COMPLETE/ADAPTER_FUNCTION_PHASE_COMPLETE are the only HookType family with no pre/start sibling for a plugin to open a span on.

So: all inline span code is removed (mellea/telemetry/tracing.py is back to its upstream/main state), metric hooks are the only telemetry here, the doc now states the hook/plugin rule and says plainly there's no span coverage of this lifecycle until #1466, and tests assert hooks/payloads rather than span names.

Deviations from the literal issue spec

• Span naming: the issue said intrinsic.call/intrinsic.prepare; the convention that actually merged (refactor(backends): AdapterMixin verb rename/narrow + resolve_model_options + IntrinsicMetricsPlugin (Epic #929 Phase 2) #1140/refactor(backends)!: narrow AdapterMixin verbs, centralize option resolution, add AdapterFunctionMetricsPlugin skeleton #1422) is adapter_function/adapter_function.<phase>. Moot here since spans are gone, but feat(backends): LocalFileBinding implements verbs (PEFT/aLoRA path) + from_catalog() (Epic #929 Phase 2) #1141 and feat(telemetry): add complete adapter-function lifecycle observability #1466 are corrected so the next implementer doesn't copy the invented names.
• The content-capture gate is MELLEA_TRACES_CONTENT, not the issue's MELLEA_TRACE_CONTENT.
• The issue's acceptance criteria named IntrinsicMetricsPlugin; the real class is AdapterFunctionMetricsPlugin.
• docs/docs/advanced/intrinsics.md and docs/examples/intrinsics/ are untouched — documenting LocalFileBinding.from_catalog(...) as the construction pattern would document a path nothing in production can reach yet (deferred to refactor(backends): route intrinsic generation through adapter_scope (Epic #929 Phase 2) #1465/refactor(intrinsics): remove deprecation shims; rewrite intrinsics_and_adapters.md; write 3 tutorials (Epic #929 Phase 4) #1144, recorded on feat(backends): LocalFileBinding implements verbs (PEFT/aLoRA path) + from_catalog() (Epic #929 Phase 2) #1141).

Testing

uv run pytest test/ -m "not qualitative" — 3931 passed, 21 skipped (hardware-gated), 0 failures. ruff format/ruff check/mypy clean. test/backends/test_adapters/test_local_file_e2e.py (GPU-gated) passes against a real model, including through the lock changes in this PR.

Pre-existing failures unrelated to this diff: test/package/test_dependency_isolation.py's test_hooks/test_telemetry/test_telemetry_plugins_register reproduce identically on a clean upstream/main checkout.

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

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.

[AngeloDanducci]

A few non-blocking notes on top of @jakelorocco's threads, which already cover the main design questions plus the adapter.py:373 guard and double-backticks nits. I've replied on the 373 thread with a related second site, and left two inline notes below: a contradictory adapter_type between identity and binding in the two new tests, and a prepare() idempotency edge. For context, the diff looks correct relative to #1141, the scope boundary (production hot path deferred to 4.1) is documented honestly, and the revision-forwarding fix has good regression coverage.

[ajbozarth]

Clean scoping, the 4.1 boundary is well-documented, and the revision-forwarding fix is a good catch with regression coverage. One blocking item on the span telemetry — two equally-fine ways to resolve it below — plus two small independent notes.

Blocker: adapter-function spans are emitted inline instead of through the tracing-plugin pattern

Spans here are opened/closed by direct start_*_span/finish_*_span calls in adapter_scope/_run_adapter_phase/prepare/release. Everywhere else in Mellea, spans are emitted by a *TracingPlugin in mellea/telemetry/tracing_plugins.py subscribing to lifecycle hooks, and core code never imports the span helpers. Inline emission is only used where code is synchronous and can't fire paired hooks (the session lifecycle spans); adapter_scope is async-capable (the e2e test awaits generate_from_context inside it), so it falls under the plugin pattern.

The underlying reason it had to be inline: the ADAPTER_FUNCTION_*_COMPLETE hooks are completion-only. This is the one hook family in HookType without a pre/start sibling — every other family (generation, component, tool, streaming, sampling, validation, session) has paired members. A span needs an opener to anchor on, so completion-only hooks can feed a metric but can't drive span open/close, which forced the inline workaround. (The metrics half of this PR is fine — it consumes those hooks correctly, the same as the other *MetricsPlugins.)

Two ways to resolve, both fine by me:

Option A — do it here, properly. Add pre/post (or start/end) hook points to the adapter-function contract and move span emission into an AdapterFunctionTracingPlugin. Metrics then read the post payload (which already carries duration/outcome) instead of a separate *_COMPLETE event. Two gaps to close while you're in there:

• The phase enum advertises generate and parse, but nothing fires them (no span, no hook, no call site). Their real production sites are the model call in _generate_from_intrinsic (huggingface.py:575) and action.parse(result) (huggingface.py:1776) — place the hook sites there so the contract doesn't ship dead phases.
• adapter_scope's phase and invocation hooks are currently unasserted (only prepare's is tested) — the new plugin + paired hooks should get coverage like the existing pairs have.

Option B — pull span telemetry out, follow up separately. Keep the LocalFileBinding lifecycle, from_catalog, verb extraction, and the revision fix (all self-standing), drop the inline spans, and open a follow-up issue to add adapter-function tracing through the plugin pattern. Reasonable if the hook-contract redesign is more than you want in this PR — and it lines up naturally with the 4.1 cutover, which is what would fire generate/parse anyway. Whether the metrics half stays or goes with it is your call.

Small note (file not in the diff, so can't inline it)

mellea/telemetry/metrics_plugins.py — the AdapterFunctionMetricsPlugin docstring still says "No production call site fires these hooks yet." This PR adds them; please update or remove it, depending on which option above you take.

[planetf1]

@ajbozarth thanks — you're right, and the mechanism is more wrong than the diff makes it look.

Summary

The spans in this PR are produced the wrong way — inline in mellea/backends/
rather than from a tracing plugin. I'm removing them here instead of fixing them
in place, keeping the metric hooks, and fixing the LocalFileBinding defaults,
which is the real blocker. The root cause predates this PR, so I've filed #1464,
#1465 and #1466 under #929.

How we got here

Until #1181 in June, calling the span helpers directly from library code was
the pattern here — all five backends had a bare span = start_generate_span(...),
with the machinery in mellea/telemetry/backend_instrumentation.py. #1181 moved
that onto hooks and plugins, correctly, and deleted the module.

What it didn't do was write the new rule down. docs/docs/observability/tracing.md
still contains no mention of "plugin" — it lists which spans exist, never how one
gets produced — and there's no import contract, so restoring the old pattern is
invisible to CI. The one in-code template you'll find is mellea/stdlib/session.py,
which imports the helpers directly and is a deliberate exception. Six weeks later
docs/dev/adapter_observability.md was written describing the pre-#1181 approach,
pointing at start_backend_span / start_action_span as the model to mirror. I
followed it.

The bit I'd most like fixed: even knowing the plugin rule, I couldn't have
complied. ADAPTER_FUNCTION_INVOCATION_COMPLETE and
ADAPTER_FUNCTION_PHASE_COMPLETE are the only hook family with no pre/start
sibling, so there's no event at which a plugin could open these spans. The doc
wasn't just stale — it described the only approach the current hooks allow.

In this PR

• Spans removed — around 320 lines. Metric hooks stay.
• LocalFileBinding defaults fixed; the ten bare descriptors all defaulted to
  revision="main".
• feat(backends): LocalFileBinding implements verbs (PEFT/aLoRA path) + from_catalog() (Epic #929 Phase 2) #1141's criteria amended on the issue.

Follow-ups under #929

• docs(telemetry): document the hook/plugin span-production rule and enforce it (Epic #929) #1464 — hook/plugin rule documented and enforced. No dependencies; before feat(backends): EmbeddedBinding implements apply_activation (Granite Switch path); remove render_controls + set_request_adapter (Epic #929 Phase 2) #1142.
• refactor(backends): route intrinsic generation through adapter_scope (Epic #929 Phase 2) #1465 — generation routed through adapter_scope. Needs this PR, and carries
  your lock reentrancy point. Not "4.1" — that's refactor(intrinsics): remove deprecation shims; rewrite intrinsics_and_adapters.md; write 3 tutorials (Epic #929 Phase 4) #1144.
• feat(telemetry): add complete adapter-function lifecycle observability #1466 — spans from a plugin. Needs docs(telemetry): document the hook/plugin span-production rule and enforce it (Epic #929) #1464 and refactor(backends): route intrinsic generation through adapter_scope (Epic #929 Phase 2) #1465. I'll take it.

One correction before anyone builds on it: the parse site you pointed at,
huggingface.py:1776, is in _generate_from_raw, not the adapter path.
_generate_from_intrinsic has no action.parse call — it goes via the
IOContract. Your generate site is right.

Shout if you'd sequence any of this differently.

[planetf1]

Thanks for the comments so far. Will wait to see if the proposed approach is in the right direction on otel (@ajbozarth ) - i read this first, and more generally on the abstraction (@jakelorocco ) - thread - (since this is more than a in-pr review comment to be handled) before addressing the various other detailed comments which need changes & closing off conversation threads etc.

[planetf1]

Moved to draft pending the outcome of some design discussions — see #1486.

[planetf1]

Progress here stalled on the API-shape question in #1486 — I've posted a summary there of
where it's got to.

@ajbozarth — none of your review depends on that outcome, so I'll work through your points
in parallel rather than waiting on it.

[planetf1]

Unblocked and ready for review. #1486 is closed as resolved, which was what this was
waiting on, and the review points are worked through.

What changed since you last looked

• Rebased onto current main (was 27 commits behind).
• Spans removed — around 320 lines. mellea/telemetry/tracing.py is back to its
  upstream/main state, the helpers are unimported from _core.py and adapter.py, and
  test/telemetry/test_tracing_adapter_function.py is deleted. Metric hooks stay and are
  now the only telemetry here. docs/dev/adapter_observability.md is corrected to stop
  teaching the pre-refactor(telemetry)!: move backend tracing onto plugin/hook pattern #1181 pattern and to say plainly that there's no span coverage of this
  lifecycle until feat(telemetry): add complete adapter-function lifecycle observability #1466. Tests assert the fired hook payloads instead of span names and
  span status.
• LocalFileBinding.revision default fixed — was "main", so all ten bare
  LocalFileBinding() in mellea/ silently opted into tracking-latest, defeating fix(intrinsics): pin catalogue entries to HF revision SHAs + deduplicate requirement_check entries (Epic #929 Phase 0) #1135's
  pinning. Now None, meaning "use the catalogue's pinned revision", resolved lazily so
  it survives a re-pin.
• Review fixes: both hook-fire sites guarded, the two contradictory adapter_type
  test identities aligned, prepare() now fails loudly when add_adapter refuses
  registration instead of leaving a half-prepared binding, TODO(#1465) at the lock, and
  the four PR-introduced double-backtick sites swept.
• PR body and commit message rewritten to match the branch — the previous ones
  described the spans as a shipped feature, which stopped being true on 2026-07-29 and was
  never corrected.

One thing I tried and backed out, flagged rather than buried: the Adapter.__post_init__
cross-check from #1486 that would have caught @AngeloDanducci's adapter_type bug at
construction. It broke collection of eight test modules — the ten module-level Adapter
constants in rag.py/guardian.py pair an alora identity with a deliberately
unconfigured LocalFileBinding() defaulting to LoRA, and since every catalogue entry
supports both types those are placeholders, not conflicts. The check was firing on "not
configured yet". Recorded on #1516 with a NOTE(#1516) at the site; the invariant is
right, the timing isn't.

Verification — all run on the rebased head:

uv run pytest test/ -m "not qualitative"   3909 passed, 21 skipped, 4 xpassed
uv run mypy .                              Success: no issues found in 709 source files
uv run ruff format . / ruff check .        clean
pre-commit (via commit -s)                 all hooks passed

Four threads left open on purpose, all @jakelorocco's: the lock-duplication question
(both sites), the reality-specific-verbs question, and the adapter_type-duplication
question. I answered those rather than changing code, so they're yours to close or push
back on — I didn't want to resolve my own answers.

Sequencing note: #1465 is the follow-up that routes generation through adapter_scope,
and it carries @ajbozarth's lock-reentrancy point as a named constraint. It is not
"4.1" — that's #1144 (shim removal plus docs rewrite), whose scope never mentioned
generation or locking, so this would have fallen through the gap between the two.

[planetf1]

@ajbozarth — apologies, I answered your inline threads and never replied to this review
body. I'd been reading the PR through the thread list, so your summary and its two asks
sat unaddressed for a fortnight. Both are done now.

Your blocker — I took Option B. Inline spans are gone: mellea/telemetry/tracing.py
is back to byte-identical with main (the 112 added lines removed), the five helpers are
unimported from _core.py and adapter.py, and test/telemetry/test_tracing_adapter_function.py
is deleted. The LocalFileBinding lifecycle, from_catalog, verb extraction and the
revision fix stay, as you suggested. The metrics half stays too — it consumes the
completion-only hooks correctly, and it's the only observability this PR now has.

Your diagnosis of why it had to be inline was the useful part, and I've written it into
the follow-ups rather than leaving it in a review thread: the ADAPTER_FUNCTION_* family
being the one HookType family without a pre/start sibling is recorded on #1466 as
the root cause and its first work item, not a footnote. #1464 covers the rule itself —
documenting it in docs/docs/observability/tracing.md (which today contains no occurrence
of the word "plugin") and enforcing it in CI so the next occurrence fails a check instead
of needing you to catch it. #1464 blocks #1466 for exactly the reason you'd expect: the
rewritten guidance is what #1466 should be built from.

Your small note — done. AdapterFunctionMetricsPlugin's docstring no longer claims
"No production call site fires these hooks yet". It now says which phases fire and which
don't, and is explicit about a distinction I think matters: prepare, activate and
deactivate have real firing sites, but no internal Mellea path reaches them either —
they're on the public Adapter/LocalFileBinding surface, so today they fire only for a
caller driving that surface directly. generate/parse need #1465.

Two of your Option A points landed anyway, since they weren't really Option-specific:

• "adapter_scope's phase and invocation hooks are currently unasserted (only prepare's
  is tested)." Now asserted. test_local_file_integration.py checks the phase sequence is
  exactly ["activate", "deactivate"] and that the invocation payload carries the right
  name/binding_type/adapter_type and outcome == "success"; a second test asserts
  outcome == "error" with the original exception on the raising path. Two more in
  test_adapter_scope.py cover outcome classification.
• "The phase enum advertises generate and parse, but nothing fires them ... so the
  contract doesn't ship dead phases." Deferred to refactor(backends): route intrinsic generation through adapter_scope (Epic #929 Phase 2) #1465/feat(telemetry): add complete adapter-function lifecycle observability #1466 rather than fixed here,
  since the firing sites you pointed at are on the generation path this PR deliberately
  doesn't touch — but I found the problem is worse than you described and have put the full
  picture on feat(telemetry): add complete adapter-function lifecycle observability #1466. After the span removal release fires nothing (no span, no hook) and
  isn't in the Literal either, so the mismatch runs in both directions: three of six
  phases are currently unobservable. feat(telemetry): add complete adapter-function lifecycle observability #1466 now carries a table of exactly which, and has to
  reconcile both directions.

While tracing the same "contract ships something dead" pattern I found a third instance and
fixed it here, because unlike the others it's reachable today: adapter_scope declared
outcome: Literal["success", "error"], so it could never emit "schema_error" — and
mellea.adapter_function.parse_failures increments only on schema_error. That counter
was structurally dead. adapter_scope now classifies AdapterSchemaMismatchError
separately, which matters now rather than after #1465: adapter_scope is public, so a
caller can already parse inside the scope. #1141's parse_failures criterion was otherwise
unmeetable.

One correction to the review, for the record: the parse site you pointed at
(huggingface.py:1776) is in _generate_from_raw, not the adapter path —
_generate_from_intrinsic has no action.parse call, it goes via the IOContract. Your
generate site was right.

Verification on the current head (5ee47240):

uv run pytest test/ -m "not qualitative"   3911 passed, 21 skipped, 4 xpassed
uv run mypy .                              Success: no issues found in 709 source files
uv run ruff format . / ruff check .        clean
pre-commit                                 all hooks passed

Pushed as a separate commit on top rather than a force-push, so you can diff exactly
what moved since your review.

[planetf1]

Hi @jakelorocco and @ajbozarth — the earlier approvals predate a small set of lifecycle hardening changes. These isolate phase-hook failures, clarify retry/error behaviour, reject invalid binding transitions, and serialise binding prepare/release to prevent a race. The changes are covered by focused regression tests and the full non-qualitative suite. Would you mind taking a quick fresh look at the updated lifecycle code before merge?

[ajbozarth]

Some feedback from Claude

LGTM. Verified locally: 276 adapter/event-loop/hf unit tests pass, ruff + mypy clean, and every behavioral claim in the description has a matching test (terminal release, retryable prepare, the non-atomic-scope limitation via a real two-thread test). Lock ordering (_lifecycle_lock → _generation_lock) is consistent with no reverse acquisition, and the except Exception narrowing in event_loop_helper is correctly reasoned.

One optional nit inline. Non-blocking.

[jakelorocco]

lgtm