mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-15 14:22:43 +00:00
refactor(salvage): scope #40632 to the two live copy-on-write sites
Trim the salvaged commit to its two still-valid conversions:
- ChatCompletionsTransport.convert_messages (copy-on-write sanitize)
- QwenProfile.prepare_messages (copy-on-write normalize + cache_control)
Dropped from the original PR:
- agent/prompt_caching.py selective-copy: superseded by #57229 which
already rewrites apply_anthropic_cache_control on current main.
- Gemma extra_content narrowing (_model_consumes_thought_signature
'gemini or gemma' -> 'gemini' only) + its two tests: unrelated
behavior change reverting deliberate e8c3ac2f5; belongs in its own
PR with its own justification if pursued.
Conflict resolution: preserved main's newer timestamp-stripping
(#47868) inside the copy-on-write path.
This commit is contained in:
parent
724ab9098d
commit
63ddd022a2
5 changed files with 8 additions and 116 deletions
|
|
@ -8,6 +8,7 @@ single session.
|
|||
Pure functions -- no class state, no AIAgent dependency.
|
||||
"""
|
||||
|
||||
import copy
|
||||
from typing import Any, Dict, List
|
||||
|
||||
|
||||
|
|
@ -80,41 +81,6 @@ def _build_marker(ttl: str) -> Dict[str, str]:
|
|||
return marker
|
||||
|
||||
|
||||
def _copy_message_for_api_mutation(msg: Dict[str, Any]) -> Dict[str, Any]:
|
||||
"""Copy message containers that later API-retry recovery may mutate."""
|
||||
copied = dict(msg)
|
||||
|
||||
content = msg.get("content")
|
||||
if isinstance(content, list) and content:
|
||||
copied_content = []
|
||||
for part in content:
|
||||
if isinstance(part, dict):
|
||||
copied_part = dict(part)
|
||||
image_url = part.get("image_url")
|
||||
if isinstance(image_url, dict):
|
||||
copied_part["image_url"] = dict(image_url)
|
||||
copied_content.append(copied_part)
|
||||
else:
|
||||
copied_content.append(part)
|
||||
copied["content"] = copied_content
|
||||
|
||||
tool_calls = msg.get("tool_calls")
|
||||
if isinstance(tool_calls, list) and tool_calls:
|
||||
copied_tool_calls = []
|
||||
for tool_call in tool_calls:
|
||||
if isinstance(tool_call, dict):
|
||||
copied_tool_call = dict(tool_call)
|
||||
function = tool_call.get("function")
|
||||
if isinstance(function, dict):
|
||||
copied_tool_call["function"] = dict(function)
|
||||
copied_tool_calls.append(copied_tool_call)
|
||||
else:
|
||||
copied_tool_calls.append(tool_call)
|
||||
copied["tool_calls"] = copied_tool_calls
|
||||
|
||||
return copied
|
||||
|
||||
|
||||
def apply_anthropic_cache_control(
|
||||
api_messages: List[Dict[str, Any]],
|
||||
cache_ttl: str = "5m",
|
||||
|
|
@ -126,17 +92,11 @@ def apply_anthropic_cache_control(
|
|||
messages, all at the same TTL.
|
||||
|
||||
Returns:
|
||||
Copy of messages with cache_control breakpoints injected. Message
|
||||
containers are copied so later API-retry recovery can mutate the
|
||||
returned request copy without touching canonical history.
|
||||
Deep copy of messages with cache_control breakpoints injected.
|
||||
"""
|
||||
if not api_messages:
|
||||
return []
|
||||
|
||||
messages = [
|
||||
_copy_message_for_api_mutation(msg) if isinstance(msg, dict) else msg
|
||||
for msg in api_messages
|
||||
]
|
||||
messages = copy.deepcopy(api_messages)
|
||||
if not messages:
|
||||
return messages
|
||||
|
||||
marker = _build_marker(cache_ttl)
|
||||
|
||||
|
|
|
|||
|
|
@ -99,7 +99,7 @@ def _is_gemini_openai_compat_base_url(base_url: Any) -> bool:
|
|||
|
||||
|
||||
def _model_consumes_thought_signature(model: Any) -> bool:
|
||||
"""True when the outgoing model is a Gemini model that requires
|
||||
"""True when the outgoing model is a Gemini family model that requires
|
||||
``extra_content`` (thought_signature) to be replayed on tool calls.
|
||||
|
||||
Gemini 3 thinking models attach ``extra_content`` to each tool call and
|
||||
|
|
@ -111,7 +111,7 @@ def _model_consumes_thought_signature(model: Any) -> bool:
|
|||
``extra_content`` from earlier in a mixed-provider session.
|
||||
"""
|
||||
m = str(model or "").lower()
|
||||
return "gemini" in m
|
||||
return "gemini" in m or "gemma" in m
|
||||
|
||||
|
||||
class ChatCompletionsTransport(ProviderTransport):
|
||||
|
|
@ -135,7 +135,7 @@ class ChatCompletionsTransport(ProviderTransport):
|
|||
``codex_message_items`` on the message, ``call_id`` /
|
||||
``response_item_id`` on ``tool_calls`` entries.
|
||||
- ``extra_content`` on ``tool_calls`` (Gemini thought_signature) —
|
||||
stripped unless the outgoing ``model`` is itself Gemini.
|
||||
stripped unless the outgoing ``model`` is itself Gemini-family.
|
||||
Gemini 3 thinking models attach it for replay, but strict providers
|
||||
(Fireworks, Mistral) reject any payload containing it with
|
||||
``Extra inputs are not permitted, field: 'messages[N].tool_calls[M].extra_content'``.
|
||||
|
|
|
|||
|
|
@ -131,54 +131,6 @@ class TestApplyAnthropicCacheControl:
|
|||
# Original should be unmodified
|
||||
assert "cache_control" not in msgs[0].get("content", "")
|
||||
|
||||
def test_request_copy_protects_unmarked_nested_content(self):
|
||||
msgs = [
|
||||
{"role": "system", "content": "System"},
|
||||
{"role": "user", "content": [{"type": "text", "text": "msg1"}]},
|
||||
{"role": "assistant", "content": [{"type": "text", "text": "msg2"}]},
|
||||
{"role": "user", "content": [{"type": "text", "text": "msg3"}]},
|
||||
{"role": "assistant", "content": [{"type": "text", "text": "msg4"}]},
|
||||
]
|
||||
|
||||
result = apply_anthropic_cache_control(msgs)
|
||||
|
||||
assert result is not msgs
|
||||
assert result[0] is not msgs[0]
|
||||
assert result[1] is not msgs[1]
|
||||
assert result[1]["content"] is not msgs[1]["content"]
|
||||
assert result[1]["content"][0] is not msgs[1]["content"][0]
|
||||
assert result[2] is not msgs[2]
|
||||
assert result[2]["content"] is not msgs[2]["content"]
|
||||
assert result[2]["content"][0] is not msgs[2]["content"][0]
|
||||
assert "cache_control" not in msgs[2]["content"][0]
|
||||
assert result[2]["content"][0]["cache_control"] == MARKER
|
||||
|
||||
result[1]["content"][0]["text"] = "mutated request copy"
|
||||
assert msgs[1]["content"][0]["text"] == "msg1"
|
||||
|
||||
def test_request_copy_protects_nested_image_url_retry_mutation(self):
|
||||
image_url = {"url": "data:image/png;base64,original"}
|
||||
msgs = [
|
||||
{"role": "system", "content": "System"},
|
||||
{
|
||||
"role": "user",
|
||||
"content": [
|
||||
{"type": "text", "text": "see image"},
|
||||
{"type": "image_url", "image_url": image_url},
|
||||
],
|
||||
},
|
||||
]
|
||||
|
||||
result = apply_anthropic_cache_control(msgs)
|
||||
|
||||
assert result[1]["content"][1] is not msgs[1]["content"][1]
|
||||
assert result[1]["content"][1]["image_url"] is not image_url
|
||||
|
||||
result[1]["content"][1]["image_url"]["url"] = "data:image/png;base64,shrunk"
|
||||
assert msgs[1]["content"][1]["image_url"]["url"] == (
|
||||
"data:image/png;base64,original"
|
||||
)
|
||||
|
||||
def test_system_message_gets_marker(self):
|
||||
msgs = [
|
||||
{"role": "system", "content": "You are helpful"},
|
||||
|
|
|
|||
|
|
@ -84,16 +84,6 @@ class TestChatCompletionsBasic:
|
|||
"google": {"thought_signature": "SIG_123"}
|
||||
}, model
|
||||
|
||||
def test_convert_messages_strips_extra_content_for_gemma(self, transport):
|
||||
"""Gemma can be served by Google, but it is not Gemini and rejects
|
||||
Gemini-only request fields. Stale thought signatures from a prior
|
||||
Gemini turn must be stripped when switching to Gemma.
|
||||
"""
|
||||
msgs = self._msg_with_extra_content()
|
||||
result = transport.convert_messages(msgs, model="google/gemma-4-31b-it")
|
||||
assert "extra_content" not in result[0]["tool_calls"][0]
|
||||
assert "extra_content" in msgs[0]["tool_calls"][0]
|
||||
|
||||
def test_convert_messages_strips_tool_name(self, transport):
|
||||
"""Internal `tool_name` (used for FTS indexing in the SQLite store) is
|
||||
not part of the OpenAI Chat Completions schema. Strict providers like
|
||||
|
|
|
|||
|
|
@ -297,16 +297,6 @@ class TestBuildApiKwargsOpenRouter:
|
|||
# call_id/response_item_id still stripped regardless of model
|
||||
assert "call_id" not in result["tool_calls"][0]
|
||||
|
||||
def test_sanitize_tool_calls_strips_extra_content_for_gemma(self, monkeypatch):
|
||||
"""Gemma is served by Google but does not consume Gemini thought signatures."""
|
||||
agent = _make_agent(monkeypatch, "openrouter")
|
||||
api_msg = self._api_msg_with_extra_content()
|
||||
result = agent._sanitize_tool_calls_for_strict_api(
|
||||
api_msg, model="google/gemma-4-31b-it"
|
||||
)
|
||||
assert "extra_content" not in result["tool_calls"][0]
|
||||
assert "call_id" not in result["tool_calls"][0]
|
||||
|
||||
|
||||
class TestDeveloperRoleSwap:
|
||||
"""GPT-5 and Codex models should get 'developer' instead of 'system' role."""
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue