Repository navigation
refactor(backends): route intrinsic generation through adapter_scope (Epic #929 Phase 2) #1465
Description
Parent epic: #929
Phase: 2
Depends on: #1141
Blocks: #1466 (generate / parse spans only)
Problem
Adapter-function generation does not run inside adapter_scope. LocalFileBinding
can now prepare / activate / deactivate / release a PEFT adapter, and
adapter_scope() demonstrably flips the real model's active adapter set — but the
model call in _generate_from_intrinsic still happens outside that scope, and the
standard generation path deactivates adapters first
(_generate_with_adapter_lock). So the lifecycle verbs and the generation they
exist to serve are not yet connected.
This was #1141's acceptance criterion 66 — "E2E adapter call (prepare →
activate → generate → deactivate → release) passes" — and it is not excluded by
#1141's out-of-scope list. It was deferred during implementation of PR #1454, and
the deferral is disclosed only in a test docstring
(test/backends/test_adapters/test_local_file_e2e.py) which refers to a follow-up
issue that was never created. This issue is that follow-up.
It is also not what the epic informally calls "4.1". 4.1 is #1144 (shim
removal plus the intrinsics_and_adapters.md rewrite), whose scope does not
mention generation, activation, scoping or locking. Without a separate issue this
work falls through the gap between the two.
Known constraint: the activation lock is not reentrant
_adapter_activation_lock returns _generation_lock, which is not reentrant.
Once generation runs inside adapter_scope and activate() re-acquires that
lock, it deadlocks. This was raised in PR #1454 review. Separately, the lock is
weaker than its docstring implies today — it does not serialise everything the
docstring claims — so the fix should reconcile behaviour and documentation rather
than only unblocking the deadlock.
Any solution needs to keep the current guarantee that a caller cannot observe
another caller's adapter active, which is what the existing
deactivate-before-generate behaviour buys.
Scope
- Route the model call in
_generate_from_intrinsicthroughadapter_scope, so
the prepared and activated adapter is the one used for generation. - Resolve the activation/generation lock reentrancy, and align the docstring with
actual behaviour. - Extend the existing e2e coverage so the full lifecycle is exercised with
generation inside the scope, and remove the deferral note from
test_local_file_e2e.py.
Out of scope
EmbeddedBinding(feat(backends): EmbeddedBinding implements apply_activation (Granite Switch path); remove render_controls + set_request_adapter (Epic #929 Phase 2) #1142) andServerMediatedBinding.- Span emission for
generate/parse— that is blocked on this issue and
tracked in feat(telemetry): add complete adapter-function lifecycle observability #1466.
Acceptance criteria
-
_generate_from_intrinsicperforms its model call insideadapter_scope - Generation demonstrably uses the activated adapter, asserted rather than
smoke-tested - No deadlock:
activate()insideadapter_scopewith generation nested
completes, with a regression test that would deadlock under the current lock -
_adapter_activation_lock/_generation_lockdocstrings match observed
behaviour - A concurrent-callers test shows one caller cannot generate against another
caller's adapter -
test_local_file_e2e.pycovers prepare → activate → generate → deactivate →
release, and its deferral note is gone -
uv run pytest test/ -m "not qualitative"passes; the e2e test passes on
GPU hardware
References
- feat(backends): LocalFileBinding implements verbs (PEFT/aLoRA path) + from_catalog() (Epic #929 Phase 2) #1141 acceptance criterion 66 — the criterion this issue carries
- PR feat(backends): LocalFileBinding implements verbs (PEFT/aLoRA path) + from_catalog() (Epic #929 Phase 2) #1454 — where the deferral happened, and the review comment identifying the
deadlock - refactor(backends): AdapterMixin verb rename/narrow + resolve_model_options + IntrinsicMetricsPlugin (Epic #929 Phase 2) #1140 — introduced
adapter_scopeand the verb set - refactor(intrinsics): remove deprecation shims; rewrite intrinsics_and_adapters.md; write 3 tutorials (Epic #929 Phase 4) #1144 — "4.1"; explicitly not this work
Activity
feat(backends): implement LocalFileBinding verbs and from_catalog()
fix(backends): classify schema mismatches in adapter_scope; correct s…
planetf1 commentedon Aug 11, 2026
Heads-up from PR #1454's verification pass: #1528 is worth reading before this issue picks a lifetime for its bindings.
LocalFileBinding.release()unloads the weights but does not deregister —unload_peft_adapterclears the backend's loaded set, while_added_adapters(the registered set thatadd_adapterchecks for duplicates) keeps its entry, and there is no inverse verb to call. So a releasedqualified_namestays claimed for the backend's lifetime and no later binding can register under it.That bites here specifically if this cutover ends up preparing a binding per call rather than once per session — the second call cannot register. #1528 frames the prior question: the merged
WeightsBindingABC saysrelease()is terminal and the binding is not reusable, while #1141's text promised arelease()+prepare()cycle. Worth settling which is intended before choosing a binding lifetime here, even if the answer is "terminal is correct, prepare once".planetf1 commentedon Aug 11, 2026
Heads up: #1528 is deciding whether a
WeightsBindingis reusable afterrelease(). Worth a quick check before this issue fixes a binding lifetime, per #1528's own note ("worth resolving before #1465 picks a lifetime for its bindings").docs(backends): fix RST cross-reference roles and stale-status docstr…
4 remaining items
docs(backends): fix RST cross-reference roles and stale-status docstr…
test(backends): pin adapter_scope's new raise on shim-backed adapters
fix(backends): lock PEFT load/unload in prepare()/release(); document…
feat(backends): LocalFileBinding implements verbs (PEFT/aLoRA path) +…
fix(backends): fix telemetry mislabelling in the intrinsic adapter_sc…
fix(backends): address review findings on the adapter registry lifecycle
docs(backends): address final re-review nits across the docstrings
planetf1 commentedon Aug 20, 2026
Design note from the PR #1558 review (adapter_function lifecycle spans, now ready-state) — two things to settle in #1465's design before adding the
generate/parsephases:1. Phase-complete is success-only, by contract — and that contract is about to meet the highest-failure-cost phase.
AdapterFunctionPhaseCompletePayloadhas nooutcome/errorfields: a phase that raised fires phase-start but never phase-complete, and the failure is attributed only at the span level —finish_adapter_function_spandefensively closes any still-openadapter_function.<phase>child with the invocation's exception and ERROR status. That is accurate today because the lifecycle phases are sequential (at most one child open at a time), and it will stay accurate for a failedgeneratephase the same way. The gap is for plugin consumers that don't use tracing:AdapterFunctionMetricsPluginrecordsphase_durationonly on phase-complete, so a phase that raised yields no histogram sample and no plugin-visible event — the span is the only record of it. If that matters for generation (arguably nowhere else — it's where failures are most expensive to attribute), #1465 needs either a failure-carrying phase event, or outcome/error symmetry withinvocation_complete, and that's a contract decision that should be made once, before thegeneratephase is built on the current one.2. The attribution path it will flow through has a known robustness gap. The dangling-child cleanup that performs that span-level attribution runs without isolation between span operations (#1577, filed from the same review; unguarded in the new completion path, plus a pre-existing handler-dispatch routing exposure shared by all plugin-closed span families). Hardening that before #1465 adds more phases through it is free leverage — one fix covers the two new phases for free.
No action requested on this issue — recording so the design discussion has the context.
fix(backends): release() deregisters via a new remove_adapter() inver…
fix(backends): route intrinsic generation through adapter_scope, fix …
fix(backends): serialize resolve_adapter() registration under the act…