From ccd62f9b9d14194fdcda12c7e1b1fb14cf4c7fc1 Mon Sep 17 00:00:00 2001 From: Swan-S <139963757@qq.com> Date: Tue, 7 Jul 2026 04:35:35 +0000 Subject: [PATCH 1/2] fix(streaming): drop empty tool_calls arrays before API send (#5737) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Strict providers (DeepSeek v4, newer OpenAI) reject an assistant message carrying tool_calls: [] with HTTP 400. _sanitize_messages_for_api already strips orphaned tool results, but not an assistant message that literally stores an empty tool_calls list. Drop the empty key on ALL THREE sanitized-dict sites that must agree: _sanitize_messages_for_api, _api_safe_message_positions, and the _safe_projection inside _restore_reasoning_metadata (so a tool_calls: [] row projects identically on both sides of the metadata-carry-forward alignment and doesn't lose its reasoning/id/timestamp every turn — Fable gate follow-up, folded in). Added mutation-checked regression tests: empty tool_calls dropped + populated chain preserved (both sanitizer paths), and metadata carry-forward survives a tool_calls: [] row. Co-authored-by: nesquena-hermes --- api/streaming.py | 12 ++++ tests/test_context_history_sanitization.py | 76 +++++++++++++++++++++- 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/api/streaming.py b/api/streaming.py index 4bc49fe0e..0fd7a8290 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -4025,6 +4025,10 @@ def _sanitize_messages_for_api( # Orphaned tool result — skip to avoid 400 from strict providers. continue sanitized = {k: v for k, v in msg.items() if k in _API_SAFE_MSG_KEYS} + # Drop empty tool_calls — strict providers (DeepSeek, newer OpenAI) + # reject tool_calls: [] with HTTP 400 even when no orphaned calls exist. + if 'tool_calls' in sanitized and not sanitized['tool_calls']: + del sanitized['tool_calls'] # Provider-aware reasoning_content stripping from model-facing history. # Historical assistant reasoning_content is stripped only when the user # explicitly requests strip mode or auto mode identifies a local/generic @@ -4136,6 +4140,8 @@ def _api_safe_message_positions(messages): if not tid or tid not in valid_tool_call_ids: continue sanitized = {k: v for k, v in msg.items() if k in _API_SAFE_MSG_KEYS} + if 'tool_calls' in sanitized and not sanitized['tool_calls']: + del sanitized['tool_calls'] if is_recovered: sanitized['_recovered'] = True # temporary marker — stripped before return if 'content' in sanitized: @@ -4315,6 +4321,12 @@ def _restore_reasoning_metadata(previous_messages, updated_messages): if not isinstance(msg, dict): return None projected = {k: v for k, v in msg.items() if k in _API_SAFE_MSG_KEYS and msg.get('role')} + # Mirror the empty-tool_calls drop applied by _api_safe_message_positions + # (#5737) so this projection matches the API-safe positions it's aligned + # against — otherwise a row stored with tool_calls: [] projects + # differently here than in prev_safe and loses its metadata carry-forward. + if 'tool_calls' in projected and not projected['tool_calls']: + del projected['tool_calls'] if 'content' in projected: projected['content'] = _strip_oob_blocks(projected['content']) return projected diff --git a/tests/test_context_history_sanitization.py b/tests/test_context_history_sanitization.py index 750f76e74..aadfd352b 100644 --- a/tests/test_context_history_sanitization.py +++ b/tests/test_context_history_sanitization.py @@ -6,7 +6,11 @@ import threading from types import SimpleNamespace from unittest.mock import patch -from api.streaming import _sanitize_messages_for_api, _strip_oob_blocks +from api.streaming import ( + _restore_reasoning_metadata, + _sanitize_messages_for_api, + _strip_oob_blocks, +) OOB_BLOCK = ( @@ -99,6 +103,45 @@ def test_sanitize_messages_for_api_preserves_tool_chains(): assert not _contains_oob(sanitized) +def test_sanitize_messages_drops_empty_tool_calls_array(): + """#5737: strict providers (DeepSeek v4, newer OpenAI) reject an assistant + message carrying `tool_calls: []` with HTTP 400. A stored assistant message + can literally have an empty list (not None, not missing), which the orphan + linker doesn't catch. The sanitizer must drop the empty key while leaving a + populated tool_calls chain intact.""" + messages = [ + {"role": "user", "content": "hi"}, + # Empty tool_calls: [] must be dropped (would 400 on strict providers). + {"role": "assistant", "content": "thinking out loud", "tool_calls": []}, + {"role": "user", "content": "go on"}, + # Populated tool_calls (+ its tool result) must be preserved untouched. + { + "role": "assistant", + "content": "", + "tool_calls": [ + {"type": "function", "id": "call-9", + "function": {"name": "terminal", "arguments": "{}"}}, + ], + }, + {"role": "tool", "tool_call_id": "call-9", "content": "ok"}, + ] + + sanitized = _sanitize_messages_for_api(messages) + + # The empty-tool_calls assistant message survives, but WITHOUT the key. + empty_asst = next(m for m in sanitized if m.get("content") == "thinking out loud") + assert empty_asst["role"] == "assistant" + assert "tool_calls" not in empty_asst, "empty tool_calls: [] must be dropped (strict-provider 400)" + # The populated chain is preserved. + populated = next(m for m in sanitized if m.get("role") == "assistant" and m.get("tool_calls")) + assert populated["tool_calls"][0]["id"] == "call-9" + assert any(m.get("tool_call_id") == "call-9" for m in sanitized) + # No message that reaches the API carries an empty tool_calls array. + for m in sanitized: + assert not ("tool_calls" in m and not m["tool_calls"]), \ + "no message may ship an empty tool_calls array" + + def test_gateway_conversation_history_no_oob(): from api.config import STREAM_PARTIAL_TEXT, STREAM_REASONING_TEXT from api.gateway_chat import _STREAM_RUN_IDS, _run_gateway_runs_api_streaming @@ -261,3 +304,34 @@ def test_strip_oob_blocks_incomplete_block_preserved(): # Incomplete block should remain untouched assert "[OUT-OF-BAND USER MESSAGE" in cleaned + + +def test_restore_metadata_aligns_row_with_empty_tool_calls(): + """#5737 third site: _restore_reasoning_metadata aligns previous vs updated + rows by their API-safe projection. A row stored with tool_calls: [] must + project identically on both sides (empty key dropped) so its reasoning / + stable id / timestamp still carry forward — otherwise the projections diverge + and the row silently loses its metadata on every turn.""" + previous = [ + {"role": "user", "content": "q"}, + { + "role": "assistant", + "content": "answer", + "tool_calls": [], # empty — the strict-provider case + "reasoning": "prior reasoning", + "id": "msg-42", + "timestamp": 1234, + }, + ] + # The agent echoes the row back WITHOUT our metadata (and without tool_calls). + updated = [ + {"role": "user", "content": "q"}, + {"role": "assistant", "content": "answer"}, + ] + + restored = _restore_reasoning_metadata(previous, updated) + + asst = restored[1] + assert asst["reasoning"] == "prior reasoning", "reasoning must carry forward across the empty-tool_calls row" + assert asst["id"] == "msg-42", "stable id must carry forward" + assert asst["timestamp"] == 1234, "timestamp must carry forward (row must not re-mint / drift)" From 1a2dcc7231922ace109044fee3e6f102c7174b7b Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Tue, 7 Jul 2026 04:52:55 +0000 Subject: [PATCH 2/2] docs(changelog): #5737 drop empty tool_calls before API send --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa50883a1..b738d9bc0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ### Fixed +- **Turns no longer fail with an HTTP 400 from strict providers on empty tool-call history.** Some stored assistant messages carry an empty `tool_calls: []` array, which DeepSeek v4 and newer OpenAI models reject outright ("empty array. Expected an array with minimum length 1"). The model-facing history sanitizer now drops an empty `tool_calls` key on every path that builds the API payload, so a session that accumulated such a message can still send. Populated tool-call chains and their linked results are untouched. Thanks @Swanzb. (#5737) + - **Switching profiles no longer occasionally sticks a session to the wrong provider.** When you switched profiles or tabs while the model dropdown was mid-rebuild, the app could read the *previous* profile's selected model and stamp its provider (e.g. `ollama`) onto a model that doesn't belong to it. That wrong provider got saved to the session and re-sent on every turn, bricking it with a "Provider 'X' is set… but no API key was found" error for a provider the session never used. The provider is now resolved from the dropdown option whose value actually matches the model being sent, so the mismatch can't happen. Credit @b3nw for the root-cause trace. (#5567) - **Clicking a session link that belongs to another profile now switches to that profile instead of failing.** A valid `session://` deep link pointing at a session owned by a different Hermes profile used to look identical to a deleted session — the UI showed "Session not available in web UI" and self-healed away. Now the WebUI recognizes a valid-but-wrong-profile session, offers to switch to the owning profile, and retries the load once; truly missing/deleted sessions still self-heal as before. No foreign-profile transcript is ever returned — the mismatch response carries only the owning profile name. Thanks @harcek. (#5419)