refactor(backends): release() leaves the adapter registered; is a binding reusable after release? (Epic #929)

[planetf1]

Parent epic: #929
Phase: 2 (follow-up to #1141 / PR #1454)
Depends on: nothing
Blocks: nothing

Problem

LocalFileBinding.release() does not fully deregister the adapter, so its
qualified_name stays claimed for the backend's lifetime and no later binding can
register under it.

release() calls AdapterMixin.unload_peft_adapter, which removes the entry from
LocalHFBackend._loaded_adapters — but nothing removes it from _added_adapters,
the registered set that add_adapter populates and that add_adapter itself
checks for duplicates. There is no inverse verb for add_adapter on the mixin, so
the binding has nothing to call.

Observed, with a fake backend reproducing LocalHFBackend.add_adapter's guards:

after prepare : backend set = True | _added_adapters = ['answerability_lora']
after release : backend = None | _staged_backend = None | _added_adapters STILL = ['answerability_lora']
re-prepare without bind_backend  -> RuntimeError: requires bind_backend() to be called first
re-prepare after bind_backend    -> RuntimeError: Backend refused to register 'answerability_lora'

Two consequences:

1. The name is burned. A different LocalFileBinding for the same capability
   can never be registered on that backend after the first one is released, even
   though the first is fully torn down from the model's point of view.
2. release() overstates itself. Its docstring said "releases all resources"
   and feat(backends): LocalFileBinding implements verbs (PEFT/aLoRA path) + from_catalog() (Epic #929 Phase 2) #1141's acceptance criterion says "release() cleanly unregisters". Neither
   was accurate. PR feat(backends): LocalFileBinding implements verbs (PEFT/aLoRA path) + from_catalog() (Epic #929 Phase 2) #1454 corrects the docstring to say exactly what is and is not
   released, and points here.

The actual question: should re-registration be supported at all?

This issue is a question first, not an agreed fix. The merged WeightsBinding
contract (mellea/backends/adapters/_core.py, from #1134) says:

release() — terminal; releases all resources. The binding is not reusable
after release().

If "terminal" is meant strictly, then a released binding never being revivable is
correct, and the only defects are the wording and the confusing error. But
#1141's own text says the opposite — "prepare once per session (or explicit
release()+prepare() cycle)" — which the merged ABC forbids. That contradiction
has been flagged on #1141 and needs settling here.

So, in order:

1. Decide the contract. Is a binding reusable after release()? The ABC says
   no; feat(backends): LocalFileBinding implements verbs (PEFT/aLoRA path) + from_catalog() (Epic #929 Phase 2) #1141 said yes. One of them is wrong.
2. Only then decide the mechanism. If bindings stay terminal, the fix may be
   nothing more than what PR feat(backends): LocalFileBinding implements verbs (PEFT/aLoRA path) + from_catalog() (Epic #929 Phase 2) #1454 already did (accurate docstring, clearer error)
   plus a decision about whether a fresh binding should be able to claim a
   released name — which is the case that actually bites.
3. If a released name must become re-claimable, add_adapter needs an inverse.
   That is a change to the mixin verb contract, which feat(backends): EmbeddedBinding implements apply_activation (Granite Switch path); remove render_controls + set_request_adapter (Epic #929 Phase 2) #1142 and add support for granite-switch / embedded adapters on our huggingface backend #1018 also build
   on, so it should not be done incidentally.

Why it matters for #1465

#1465 routes intrinsic generation through adapter_scope. If that work ends up
preparing a binding per call rather than once per session, it hits this
immediately — the second call cannot register. Worth resolving before #1465 picks a
lifetime for its bindings, even if the resolution is "terminal is correct, prepare
once".

Scope

• Settle whether WeightsBinding.release() is strictly terminal, and reconcile the
  ABC docstring with feat(backends): LocalFileBinding implements verbs (PEFT/aLoRA path) + from_catalog() (Epic #929 Phase 2) #1141's text so they no longer contradict.
• Decide whether a released qualified_name should become re-claimable by a fresh
  binding, and if so add the inverse of add_adapter to the mixin contract.
• Whatever is decided, cover it with a test: either that re-registration after
  release works, or that it fails with a clear, accurate error.

Not in scope

Changing unload_peft_adapter's own semantics — it correctly manages the loaded
set. The gap is the absent registered-set counterpart.

References

• PR feat(backends): LocalFileBinding implements verbs (PEFT/aLoRA path) + from_catalog() (Epic #929 Phase 2) #1454 — added release(), corrected its docstring and the misleading
  "Backend refused to register" message to point here
• feat(backends): LocalFileBinding implements verbs (PEFT/aLoRA path) + from_catalog() (Epic #929 Phase 2) #1141 — acceptance criterion "release() cleanly unregisters"; its
  "release()+prepare() cycle" line contradicts the merged ABC
• Found while verifying a third-party review of PR feat(backends): LocalFileBinding implements verbs (PEFT/aLoRA path) + from_catalog() (Epic #929 Phase 2) #1454

[planetf1]

Additional findings, from finishing PR #1454

Four things a future implementer will want, none of which were in the original write-up.

1. The leak is visible through the public API, not just internal state

LocalHFBackend.list_adapters() is return list(self._added_adapters.keys()). Since
release() never clears _added_adapters, a released adapter is still reported by
list_adapters() — indefinitely. So this is not merely internal bookkeeping drift;
it is the documented public accessor giving an answer that contradicts "released".

Note list_adapters()'s current contract is deliberate and correct as written — its
docstring says "all adapters that have been registered via add_adapter, whether or
not they have also been loaded", and #1140 changed it to read _added_adapters
precisely so LocalHFBackend and OpenAIBackend agreed. So the fix is not "make
list_adapters ignore released adapters"; it is deciding whether release() should
deregister at all. If it should, list_adapters() follows for free.

2. The behaviour is already pinned by a test — treat that as the tripwire

test/backends/test_adapters/test_local_file_e2e.py (end of the lifecycle test)
already encodes it, with a comment:

binding.release()
assert binding.backend is None
# list_adapters() reports everything ever registered via add_adapter,
# regardless of load state — release() only reverses the load, so check
# the loaded-adapters bookkeeping directly instead.
assert binding.qualified_name not in backend._loaded_adapters

That assertion deliberately reaches for _loaded_adapters because
list_adapters() would fail. So the limitation was understood during implementation;
what was missing was it being reflected in release()'s docstring and #1141's
acceptance criterion (both now corrected).

Useful consequence: any change to this contract must update that test and delete that
comment, which makes the decision impossible to make silently. Worth keeping as the
canary rather than loosening it.

3. release() also retains the object, not just the name

add_adapter does self._added_adapters[adapter.qualified_name] = adapter — a strong
reference to the binding. So after release() the backend still holds the binding
alive for its own lifetime. Minor next to the name collision, but it is the other half
of why "releases all resources" was inaccurate, and it matters for a long-lived backend
cycling through many adapters.

4. Which bindings the contract question actually touches

Scoping the blast radius, so this does not get treated as wider than it is:

Binding	Has the four verbs?	Affected by this decision?
LocalFileBinding	yes, real	yes — the only live case
EmbeddedBinding	no — #1142 criterion: "No prepare/activate/deactivate/release methods added to EmbeddedBinding"	no
ServerMediatedBinding	yes, but all four are NotImplementedError stubs	eventually — 3.1, no issue filed yet

So in practice: #1465 needs the answer, a future 3.1 will inherit it, and #1142 is
unaffected.

5. The ABC already sanctions documenting a deviation

WeightsBinding's docstring says: "Concrete implementations are expected to document
any deviations from this contract (e.g. servers that prepare-and-activate atomically)."
PR #1454 did exactly that, so the current state is contract-conformant rather than a
violation. The open question is narrower than "fix the bug": should this deviation
persist, or should release() become a true inverse of add_adapter? If the answer
is "persist", this issue closes with no code change beyond what #1454 already shipped.