fix(agent): keep system-prompt model identity in sync across provider failover - #43872
fix(agent): keep system-prompt model identity in sync across provider failover#43872IamSanchoPanza wants to merge 2 commits into
Conversation
cca7bb5 to
ed363c3
Compare
|
Verification: reviewed the diff — this is a clean, well-structured fix. Checked:
No issues found. |
| @@ -1030,6 +1030,35 @@ def build_assistant_message(agent, assistant_message, finish_reason: str) -> dic | |||
|
|
|||
|
|
|||
|
|
|||
| def rewrite_prompt_model_identity(agent, model: str, provider: str) -> None: | |||
There was a problem hiding this comment.
Core of the fix. Two invariants here are load-bearing: (1) only the last occurrence of each line is rewritten — earlier matches can be user content from memory snapshots or context files; (2) the rewrite is never persisted to the session DB, so the stored prompt keeps the primary's labels and the prompt is byte-identical again after restore_primary_runtime — upstream prefix caches still hit. The round-trip is pinned by test_round_trip_restores_byte_identical_prompt.
| @@ -368,6 +368,32 @@ def _get_continuation_prompt(is_partial_stub: bool, dropped_tools: Optional[List | |||
| ) | |||
|
|
|||
|
|
|||
| def _sync_failover_system_message(agent, api_messages, active_system_prompt): | |||
There was a problem hiding this comment.
Why the in-flight sync is required (not just nice-to-have): the current call block's api_messages were built before the failover, and on gateway turns the primary is restored between messages — so while a usage limit lasts, every turn re-fails-over mid-turn. Without this, the stale identity ships on every single gateway message. Simply invalidating _cached_system_prompt also doesn't work: continuing sessions restore the stored prompt verbatim from the session DB, resurrecting the stale line.
… failover The session-stable system prompt embeds "Model:"/"Provider:" identity lines, but try_activate_fallback swaps the runtime without touching them — so while a fallback is active the agent misreports what it is. Reproduced on a Codex plan-limit 429: every gateway turn failed over to a local gemma4:e2b-mlx, which answered "I am gpt-5.4-mini" when asked. - rewrite_prompt_model_identity (chat_completion_helpers): rewrite the identity lines on the cached prompt when a fallback activates, and back to the primary's in restore_primary_runtime. Only the LAST occurrence of each line is touched (earlier matches can be user content from memory snapshots), and the rewrite is deliberately not persisted to the session DB — the stored prompt keeps the primary's labels, so after restoration the prompt is byte-identical again and upstream prefix caches still hit. - _sync_failover_system_message (conversation_loop): patch the in-flight api_messages at every failover site. Without this the current call block ships the stale identity — and on gateway turns that is every turn, because the primary is restored between messages and re-fails-over mid-turn while the limit lasts. Invalidating the cached prompt instead would not work: continuing sessions restore the stored prompt verbatim from the session DB, resurrecting the stale identity line. Verified end-to-end against a live usage-limited primary: call 0 goes out with the primary's identity and 429s; call 1 goes to the fallback with the rewritten lines, and the model self-reports correctly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ed363c3 to
239ab3b
Compare
|
Hi maintainers — this is my first PR to the repo, so the GitHub Actions runs appear to be waiting on maintainer approval ( I’ve completed the PR checklist, kept the change focused to |
Author email lacked a numeric-id prefix so the noreply auto-extraction misses it; map it explicitly for PR #43872 salvage.
|
Merged via #49318 — your fix landed on Your branch was ~915 commits behind Verified live against real OpenRouter failover, not just unit tests: with the fix the fallback call ships the corrected Thanks for the clean writeup and the live repro — made this easy to validate. |
Author email lacked a numeric-id prefix so the noreply auto-extraction misses it; map it explicitly for PR NousResearch#43872 salvage.
Author email lacked a numeric-id prefix so the noreply auto-extraction misses it; map it explicitly for PR NousResearch#43872 salvage.
Author email lacked a numeric-id prefix so the noreply auto-extraction misses it; map it explicitly for PR NousResearch#43872 salvage.
Author email lacked a numeric-id prefix so the noreply auto-extraction misses it; map it explicitly for PR NousResearch#43872 salvage.
Author email lacked a numeric-id prefix so the noreply auto-extraction misses it; map it explicitly for PR NousResearch#43872 salvage.
Author email lacked a numeric-id prefix so the noreply auto-extraction misses it; map it explicitly for PR NousResearch#43872 salvage.
Where to look first
The diff is 182 lines but the core logic is ~30. Suggested reading order:
rewrite_prompt_model_identity, chat_completion_helpers.py L1033: swaps the identity lines, last occurrence only, never persisted. The two design constraints in its docstring are the heart of the PR._sync_failover_system_message, conversation_loop.py L371: patches the request already mid-retry; without it the fix lands a turn too late (never, on gateway turns).try_activate_fallbackafter the runtime swap, one inrestore_primary_runtimerewriting back, and the same 2-line sync at the seven failover branches inconversation_loop.py(repetition is deliberate — the sync needsrun_conversation's locals, see inline comments).What does this PR do?
Fixes the agent misreporting its own identity while a fallback provider is active. The session-stable system prompt embeds
Model:/Provider:lines, buttry_activate_fallbackswaps the runtime without touching them — so the model that is actually answering reads (and repeats) the primary's name.Reproduced live on a Codex plan-limit 429: every gateway turn failed over to a local
gemma4:e2b-mlx, which answered "I am gpt-5.4-mini" when asked what model it was. Confusing for users, and it actively undermines trust in fallback behavior.The fix rewrites the identity lines on the cached prompt when a fallback activates (and back when the primary is restored), and syncs the in-flight
api_messagesat every failover site in the conversation loop. Two deliberate design constraints:The in-flight sync matters more than it looks: on gateway turns the primary is restored between messages, so while a usage limit lasts, every turn re-fails-over mid-turn — without the sync, the stale identity ships on every single message.
Related Issue
No existing issue — bug discovered and verified directly; happy to file one if preferred.
Type of Change
Changes Made
agent/chat_completion_helpers.py— newrewrite_prompt_model_identity(); called fromtry_activate_fallbackafter the runtime swapagent/agent_runtime_helpers.py—restore_primary_runtimerewrites the lines back to the primary'sagent/conversation_loop.py— new_sync_failover_system_message(); called at all seven failover sites so the current call block ships the corrected prompttests/agent/test_failover_identity.py— 9 new tests: line swap, last-occurrence-only, byte-identical round-trip, ephemeral append, and no-op guardsHow to Test
fallback_providerspytest tests/agent/test_failover_identity.py -vfor the unit coverageVerified end-to-end against a live usage-limited primary by instrumenting
_build_api_kwargs:Checklist
Code
scripts/run_tests.sh tests/— 30,040 passed; 33 environment-dependent failures (live local gateway/launchd state) reproduce identically on pristineorigin/main, verified by running the failing subset on both branches: identical failure sets, zero failures introduced by this changeDocumentation & Housekeeping
cli-config.yaml.example— N/A (no config keys changed)CONTRIBUTING.md/AGENTS.md— N/A