Repository navigation
Conversation
FallbackAdapter.aclose() only unsubscribed from metrics, leaving the background probes that FallbackLLMStream._try_recovery() starts for unavailable providers running. A probe could outlive the adapter: its provider request kept going, and on completion it flipped _status back to available and emitted llm_availability_changed after close. Set a closed flag before sweeping, cancel and await every recovery task, and refuse to start new probes once closed, matching the STT and TTS fallback adapters. Add regression tests covering an in-flight probe at aclose() and a stream that fails after aclose().
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(llm): cancel recovery probes when the fallback adapter closes
What breaks
FallbackAdapter.aclose()only unsubscribes the metrics collector. Recovery probes — the background tasksFallbackLLMStream._try_recovery()detaches into_LLMStatus.recovering_task— keep running after close: they can flipavailable=Trueand emitllm_availability_changedafter the adapter is closed, mutate state post-shutdown, and a stream that fails afteraclose()can spawn a brand-new probe (the probe machinery has no closed check).Fix
_closedflag, set first inaclose().aclose()thenaio.cancel_and_waits every in-flightrecovering_task, then unsubscribes metrics._try_recovery()returns immediately when_closed, so no probe can start after close.Mirrors the existing TTS/STT fallback-adapter close style. Pre-close behavior is unchanged (the fallback/retry paths are untouched; the full
test_llm_fallback.pysuite sees identical results apart from the two new tests).Tests
tests/test_llm_fallback.py(deterministic event gating, no sleeps):test_aclose_cancels_in_flight_recovery_probe— a recovery probe is in flight whenaclose()runs; asserts it is finished whenaclose()returns.test_no_recovery_probe_after_close— a gated failure that would start a probe after close; asserts no probe is created.On base: both fail —
assert ... <Task pending ... _recover_llm_task() ...>(a probe outlivedaclose()). With the fix: 2 passed, repeated 20× without flakes (also under-W error::RuntimeWarning);test_llm_fallback.py106 passed, LLM+STT+TTS fallback suites 130, coverage spans 16; ruff clean. Independent review verified cancellation cannot hang (probes are unshielded,CancelledErrorpropagates through the stream's unwind) and that_closedis set before the sweep so no probe can slip through.