From 3fe7f6d27a005a0a75644c1d1d42cef8c8750c5f Mon Sep 17 00:00:00 2001 From: Umi4Life Date: Tue, 16 Jun 2026 17:08:25 +0000 Subject: [PATCH] fix: preserve fallback switch notice on successful fallback Co-Authored-By: Claude Opus 4.8 --- agent/chat_completion_helpers.py | 11 +++ agent/conversation_loop.py | 7 +- run_agent.py | 27 +++++++ tests/run_agent/test_retry_status_buffer.py | 80 +++++++++++++++++++++ 4 files changed, 123 insertions(+), 2 deletions(-) diff --git a/agent/chat_completion_helpers.py b/agent/chat_completion_helpers.py index 9be0a7fed52..c3df449bdfb 100644 --- a/agent/chat_completion_helpers.py +++ b/agent/chat_completion_helpers.py @@ -1399,6 +1399,7 @@ def try_activate_fallback(agent, reason: "FailoverReason | None" = None) -> bool fb_api_mode = "bedrock_converse" old_model = agent.model + old_provider = agent.provider # Clear the per-config context_length override so the fallback # model's actual context window is resolved instead of inheriting @@ -1544,6 +1545,16 @@ def try_activate_fallback(agent, reason: "FailoverReason | None" = None) -> bool f"🔄 Primary model failed — switching to fallback: " f"{fb_model} via {fb_provider}" ) + # The buffered line above is dropped on successful recovery, but a + # provider/model switch is a durable state change operators must see + # even when the fallback succeeds. Record a one-shot notice that the + # success path surfaces exactly once via _emit_pending_fallback_notice + # (see run_agent.py); it is discarded on terminal failure since the + # buffered line is flushed instead. See fallback-observability fix. + agent._pending_fallback_notice = ( + f"🔄 Switched to fallback model: {old_model} via {old_provider} " + f"→ {fb_model} via {fb_provider}" + ) logger.info( "Fallback activated: %s → %s (%s)", old_model, fb_model, fb_provider, diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index cbcb8561701..4295bdad602 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -5062,8 +5062,11 @@ def run_conversation( # Reset retry counter/signature on successful content agent._empty_content_retries = 0 agent._thinking_prefill_retries = 0 - # Successful content reached — drop any buffered retry - # status from earlier failed attempts in this turn. + # Successful content reached — surface the one-shot fallback + # switch notice (if a fallback activated this turn) before + # dropping the noisy retry buffer, so a provider/model switch + # stays visible even when the fallback succeeds. + agent._emit_pending_fallback_notice() agent._clear_status_buffer() from agent.agent_runtime_helpers import ( diff --git a/run_agent.py b/run_agent.py index 6fb7e0903e5..61e8657b9a8 100644 --- a/run_agent.py +++ b/run_agent.py @@ -981,6 +981,29 @@ class AIAgent: except Exception: pass + def _emit_pending_fallback_notice(self) -> None: + """Surface the one-shot fallback-switch notice on successful recovery. + + A provider/model switch is a durable state change operators must see, + unlike transient retry chatter that ``_clear_status_buffer`` drops. + ``try_activate_fallback`` records the switch in + ``self._pending_fallback_notice``; this emits it exactly once via + ``_emit_status`` and then clears it, so a successful fallback still + produces one visible notice. On terminal failure the buffered switch + line is flushed instead (and this notice discarded) — see + ``_flush_status_buffer`` — so the user always sees the switch once. + """ + try: + notice = getattr(self, "_pending_fallback_notice", None) + if notice: + # Clear before emitting so a (swallowed) callback error can't + # leave the notice set for a stale re-emit on a later turn. + self._pending_fallback_notice = None + self._emit_status(notice) + except Exception: + # Never break the conversation loop on a notice hiccup. + pass + def _flush_status_buffer(self) -> None: """Emit buffered retry messages — call on terminal failure. @@ -988,6 +1011,10 @@ class AIAgent: was tried before the turn gave up. """ try: + # The buffered trace already carries the fallback switch line, so + # drop any one-shot fallback notice to avoid a stale duplicate + # leaking into a later successful turn. + self._pending_fallback_notice = None buf = getattr(self, "_retry_status_buffer", None) if not buf: return diff --git a/tests/run_agent/test_retry_status_buffer.py b/tests/run_agent/test_retry_status_buffer.py index 221c10c7596..bf116f177b7 100644 --- a/tests/run_agent/test_retry_status_buffer.py +++ b/tests/run_agent/test_retry_status_buffer.py @@ -135,6 +135,86 @@ def test_mixed_kinds_replay_through_correct_channels(): assert warns == ["warn-1"] +def test_pending_fallback_notice_emitted_once_on_success(): + """On successful recovery the one-shot fallback notice is surfaced even + though the noisy retry buffer is dropped.""" + agent = _make_bare_agent() + emitted = [] + agent._emit_status = lambda msg: emitted.append(msg) + + # Simulate try_activate_fallback: buffer the noisy switch line AND record + # the durable one-shot notice. + agent._buffer_status("🔄 Primary model failed — switching to fallback: m2 via p2") + agent._pending_fallback_notice = "🔄 Switched to fallback model: m1 via p1 → m2 via p2" + + # Success path order: emit pending notice, then drop the buffer. + agent._emit_pending_fallback_notice() + agent._clear_status_buffer() + + # The durable notice was shown exactly once; the buffered retry noise was + # silently dropped. + assert emitted == ["🔄 Switched to fallback model: m1 via p1 → m2 via p2"] + assert agent._retry_status_buffer == [] + # Notice is cleared so it cannot re-emit on a later turn. + assert agent._pending_fallback_notice is None + + # A second success path with no new fallback emits nothing. + agent._emit_pending_fallback_notice() + assert emitted == ["🔄 Switched to fallback model: m1 via p1 → m2 via p2"] + + +def test_pending_fallback_notice_noop_when_unset(): + """No fallback this turn → no notice emitted on the success path.""" + agent = _make_bare_agent() + emitted = [] + agent._emit_status = lambda msg: emitted.append(msg) + + # No _pending_fallback_notice attribute set at all. + agent._emit_pending_fallback_notice() + assert emitted == [] + + +def test_flush_discards_pending_fallback_notice(): + """On terminal failure the flushed buffer already carries the switch line, + so the one-shot notice is discarded to avoid a stale duplicate later.""" + agent = _make_bare_agent() + emitted = [] + agent._emit_status = lambda msg: emitted.append(msg) + + agent._buffer_status("🔄 Primary model failed — switching to fallback: m2 via p2") + agent._pending_fallback_notice = "🔄 Switched to fallback model: m1 via p1 → m2 via p2" + + # Terminal failure flushes the buffered trace... + agent._flush_status_buffer() + assert emitted == ["🔄 Primary model failed — switching to fallback: m2 via p2"] + # ...and discards the pending notice so it won't re-emit on a later turn. + assert agent._pending_fallback_notice is None + + emitted.clear() + agent._emit_pending_fallback_notice() + assert emitted == [] + + +def test_pending_fallback_notice_survives_emit_callback_error(): + """A failing status callback must not leave the notice set for a stale + re-emit, and must not raise.""" + agent = _make_bare_agent() + seen = [] + + def boom(msg): + seen.append(msg) + raise RuntimeError("simulated callback failure") + + agent._emit_status = boom + agent._pending_fallback_notice = "🔄 Switched to fallback model: m1 via p1 → m2 via p2" + + # Should not raise. + agent._emit_pending_fallback_notice() + # Attempt was made and the notice is cleared regardless. + assert seen == ["🔄 Switched to fallback model: m1 via p1 → m2 via p2"] + assert agent._pending_fallback_notice is None + + def test_flush_swallows_callback_exceptions(): agent = _make_bare_agent() seen = []