-
Notifications
You must be signed in to change notification settings - Fork 46.8k
fix(agent): keep system-prompt model identity in sync across provider failover #43872
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -366,6 +366,85 @@ def _get_continuation_prompt(is_partial_stub: bool, dropped_tools: Optional[List | |
| "length limit. Continue exactly where you left off. Do not " | ||
| "restart or repeat prior text. Finish the answer directly.]" | ||
| ) | ||
| def _stored_prompt_matches_runtime(agent, prompt: str) -> bool: | ||
| """Return False when the persisted Model/Provider lines are stale.""" | ||
|
|
||
| def line_value(label: str) -> str: | ||
| prefix = f"{label}:" | ||
| value = "" | ||
| for line in prompt.splitlines(): | ||
| if line.startswith(prefix): | ||
| value = line[len(prefix):].strip() | ||
| return value | ||
|
|
||
| stored_model = line_value("Model") | ||
| current_model = str(getattr(agent, "model", "") or "").strip() | ||
| if stored_model and current_model and stored_model != current_model: | ||
| return False | ||
|
|
||
| stored_provider = line_value("Provider") | ||
| current_provider = str(getattr(agent, "provider", "") or "").strip() | ||
| if stored_provider and current_provider and stored_provider != current_provider: | ||
| return False | ||
|
|
||
| return True | ||
|
|
||
|
|
||
| _CONTENT_POLICY_RECOVERY_HINT = ( | ||
| "Try rephrasing the request, narrowing the context, or " | ||
| "adding a fallback provider with `hermes fallback add`." | ||
| ) | ||
|
|
||
|
|
||
| def _content_policy_blocked_result( | ||
| messages: List[Dict], | ||
| api_call_count: int, | ||
| *, | ||
| final_response: str, | ||
| error_detail: str, | ||
| ) -> Dict[str, Any]: | ||
| """Build the terminal turn result for a content-policy block. | ||
|
|
||
| A content-policy refusal is deterministic for the unchanged prompt, so the | ||
| turn ends here (no retry). Both the HTTP-200 refusal handler and the | ||
| exception-path handler return the identical shape — a failed, non-completed | ||
| turn carrying the user-facing message and a ``content_policy_blocked:`` | ||
| prefixed error — so they funnel through this one builder. | ||
| """ | ||
| return { | ||
| "final_response": final_response, | ||
| "messages": messages, | ||
| "api_calls": api_call_count, | ||
| "completed": False, | ||
| "failed": True, | ||
| "error": f"content_policy_blocked: {error_detail}", | ||
| } | ||
|
|
||
|
|
||
| def _sync_failover_system_message(agent, api_messages, active_system_prompt): | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why the in-flight sync is required (not just nice-to-have): the current call block's |
||
| """Refresh the in-flight system message after a provider failover. | ||
|
|
||
| ``try_activate_fallback`` rewrites the ``Model:``/``Provider:`` identity | ||
| lines on ``agent._cached_system_prompt`` (see | ||
| ``rewrite_prompt_model_identity``) so the agent reports the model that is | ||
| actually answering. But the current call block's ``api_messages`` were | ||
| built from the pre-failover prompt, and the retry loop rebuilds | ||
| ``api_kwargs`` from that list each iteration — without this sync the | ||
| whole turn (and every gateway turn, since fallback re-activates per | ||
| message while the primary is down) ships the stale identity. | ||
|
|
||
| Mutates ``api_messages[0]`` in place and returns the prompt to use as | ||
| ``active_system_prompt`` for subsequent call-block rebuilds. | ||
| """ | ||
| sp = getattr(agent, "_cached_system_prompt", None) | ||
| if not isinstance(sp, str) or not sp: | ||
| return active_system_prompt | ||
| if api_messages and api_messages[0].get("role") == "system": | ||
| effective = sp | ||
| if agent.ephemeral_system_prompt: | ||
| effective = (effective + "\n\n" + agent.ephemeral_system_prompt).strip() | ||
| api_messages[0]["content"] = effective | ||
| return sp | ||
|
|
||
|
|
||
| def run_conversation( | ||
|
|
@@ -831,6 +910,8 @@ def run_conversation( | |
| ) | ||
| agent._buffer_status(f"⏳ {_nous_msg}") | ||
| if agent._try_activate_fallback(): | ||
| active_system_prompt = _sync_failover_system_message( | ||
| agent, api_messages, active_system_prompt) | ||
| retry_count = 0 | ||
| compression_attempts = 0 | ||
| _retry.primary_recovery_attempted = False | ||
|
|
@@ -1156,6 +1237,8 @@ def _perform_api_call(next_api_kwargs): | |
| if agent._fallback_index < len(agent._fallback_chain): | ||
| agent._buffer_status("⚠️ Empty/malformed response — switching to fallback...") | ||
| if agent._try_activate_fallback(): | ||
| active_system_prompt = _sync_failover_system_message( | ||
| agent, api_messages, active_system_prompt) | ||
| retry_count = 0 | ||
| compression_attempts = 0 | ||
| _retry.primary_recovery_attempted = False | ||
|
|
@@ -1227,6 +1310,8 @@ def _perform_api_call(next_api_kwargs): | |
| if agent._has_pending_fallback(): | ||
| agent._buffer_status(f"⚠️ Max retries ({max_retries}) for invalid responses — trying fallback...") | ||
| if agent._try_activate_fallback(): | ||
| active_system_prompt = _sync_failover_system_message( | ||
| agent, api_messages, active_system_prompt) | ||
| retry_count = 0 | ||
| compression_attempts = 0 | ||
| _retry.primary_recovery_attempted = False | ||
|
|
@@ -2570,6 +2655,8 @@ def _perform_api_call(next_api_kwargs): | |
| else: | ||
| agent._buffer_status("⚠️ Rate limited — switching to fallback provider...") | ||
| if agent._try_activate_fallback(reason=classified.reason): | ||
| active_system_prompt = _sync_failover_system_message( | ||
| agent, api_messages, active_system_prompt) | ||
| retry_count = 0 | ||
| compression_attempts = 0 | ||
| _retry.primary_recovery_attempted = False | ||
|
|
@@ -2970,6 +3057,8 @@ def _perform_api_call(next_api_kwargs): | |
| else: | ||
| agent._buffer_status(f"⚠️ Non-retryable error (HTTP {status_code}) — trying fallback...") | ||
| if agent._try_activate_fallback(): | ||
| active_system_prompt = _sync_failover_system_message( | ||
| agent, api_messages, active_system_prompt) | ||
| retry_count = 0 | ||
| compression_attempts = 0 | ||
| _retry.primary_recovery_attempted = False | ||
|
|
@@ -3114,6 +3203,8 @@ def _perform_api_call(next_api_kwargs): | |
| if agent._has_pending_fallback(): | ||
| agent._buffer_status(f"⚠️ Max retries ({max_retries}) exhausted — trying fallback...") | ||
| if agent._try_activate_fallback(): | ||
| active_system_prompt = _sync_failover_system_message( | ||
| agent, api_messages, active_system_prompt) | ||
| retry_count = 0 | ||
| compression_attempts = 0 | ||
| _retry.primary_recovery_attempted = False | ||
|
|
@@ -4038,6 +4129,8 @@ def _perform_api_call(next_api_kwargs): | |
| "switching to fallback provider..." | ||
| ) | ||
| if agent._try_activate_fallback(): | ||
| active_system_prompt = _sync_failover_system_message( | ||
| agent, api_messages, active_system_prompt) | ||
| agent._empty_content_retries = 0 | ||
| agent._buffer_status( | ||
| f"↻ Switched to fallback: {agent.model} " | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,104 @@ | ||
| """Tests for system-prompt model-identity sync across provider failover. | ||
|
|
||
| The system prompt is session-stable and embeds ``Model:``/``Provider:`` | ||
| identity lines. When ``try_activate_fallback`` swaps the runtime, the | ||
| prompt must be rewritten in place (and synced into the in-flight | ||
| ``api_messages``) or the agent reports the primary model's name while a | ||
| fallback model is answering — e.g. a local gemma fallback claiming to be | ||
| gpt-5.4-mini after a Codex usage-limit 429. | ||
| """ | ||
|
|
||
| from types import SimpleNamespace | ||
|
|
||
| from agent.chat_completion_helpers import rewrite_prompt_model_identity | ||
| from agent.conversation_loop import _sync_failover_system_message | ||
|
|
||
|
|
||
| _PROMPT = ( | ||
| "You are a helpful assistant.\n" | ||
| "\n" | ||
| "Memory note at line start:\n" | ||
| "Model: decoy-from-memory\n" | ||
| "\n" | ||
| "Conversation started: Wednesday, June 10, 2026\n" | ||
| "Model: gpt-5.4-mini\n" | ||
| "Provider: openai-codex" | ||
| ) | ||
|
|
||
|
|
||
| def _agent(prompt=_PROMPT, ephemeral=None): | ||
| return SimpleNamespace( | ||
| _cached_system_prompt=prompt, | ||
| ephemeral_system_prompt=ephemeral, | ||
| ) | ||
|
|
||
|
|
||
| class TestRewritePromptModelIdentity: | ||
| def test_swaps_identity_lines_to_fallback_runtime(self): | ||
| agent = _agent() | ||
| rewrite_prompt_model_identity(agent, "gemma4:e2b-mlx", "custom") | ||
| assert "Model: gemma4:e2b-mlx" in agent._cached_system_prompt | ||
| assert "Provider: custom" in agent._cached_system_prompt | ||
| assert "Model: gpt-5.4-mini" not in agent._cached_system_prompt | ||
| assert "Provider: openai-codex" not in agent._cached_system_prompt | ||
|
|
||
| def test_only_last_occurrence_is_rewritten(self): | ||
| agent = _agent() | ||
| rewrite_prompt_model_identity(agent, "gemma4:e2b-mlx", "custom") | ||
| # Earlier matching lines may be user content (memory snapshots, | ||
| # context files) and must survive untouched. | ||
| assert "Model: decoy-from-memory" in agent._cached_system_prompt | ||
|
|
||
| def test_round_trip_restores_byte_identical_prompt(self): | ||
| # restore_primary_runtime rewrites the lines back; the result must | ||
| # match the stored prompt byte-for-byte so the primary's prefix | ||
| # cache still hits after restoration. | ||
| agent = _agent() | ||
| rewrite_prompt_model_identity(agent, "gemma4:e2b-mlx", "custom") | ||
| rewrite_prompt_model_identity(agent, "gpt-5.4-mini", "openai-codex") | ||
| assert agent._cached_system_prompt == _PROMPT | ||
|
|
||
| def test_noop_when_prompt_missing_or_empty(self): | ||
| for prompt in (None, ""): | ||
| agent = _agent(prompt=prompt) | ||
| rewrite_prompt_model_identity(agent, "m", "p") | ||
| assert agent._cached_system_prompt == prompt | ||
|
|
||
| def test_empty_values_leave_lines_unchanged(self): | ||
| agent = _agent() | ||
| rewrite_prompt_model_identity(agent, "", "") | ||
| assert agent._cached_system_prompt == _PROMPT | ||
|
|
||
|
|
||
| class TestSyncFailoverSystemMessage: | ||
| def test_patches_in_flight_system_message(self): | ||
| agent = _agent() | ||
| rewrite_prompt_model_identity(agent, "gemma4:e2b-mlx", "custom") | ||
| api_messages = [ | ||
| {"role": "system", "content": _PROMPT}, | ||
| {"role": "user", "content": "what model are you?"}, | ||
| ] | ||
| result = _sync_failover_system_message(agent, api_messages, _PROMPT) | ||
| assert "Model: gemma4:e2b-mlx" in api_messages[0]["content"] | ||
| assert result == agent._cached_system_prompt | ||
|
|
||
| def test_appends_ephemeral_system_prompt(self): | ||
| agent = _agent(ephemeral="Stay terse.") | ||
| api_messages = [{"role": "system", "content": _PROMPT}] | ||
| _sync_failover_system_message(agent, api_messages, _PROMPT) | ||
| assert api_messages[0]["content"].endswith("Stay terse.") | ||
|
|
||
| def test_noop_without_cached_prompt(self): | ||
| agent = _agent(prompt=None) | ||
| api_messages = [{"role": "system", "content": "original"}] | ||
| result = _sync_failover_system_message(agent, api_messages, "active") | ||
| assert api_messages[0]["content"] == "original" | ||
| assert result == "active" | ||
|
|
||
| def test_noop_when_first_message_is_not_system(self): | ||
| agent = _agent() | ||
| api_messages = [{"role": "user", "content": "hi"}] | ||
| result = _sync_failover_system_message(agent, api_messages, "active") | ||
| assert api_messages == [{"role": "user", "content": "hi"}] | ||
| # Still returns the cached prompt for subsequent call-block rebuilds. | ||
| assert result == agent._cached_system_prompt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 bytest_round_trip_restores_byte_identical_prompt.