fix(streaming): drop empty tool_calls arrays before API send (#5737)

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 <nesquena-hermes@users.noreply.github.com>
This commit is contained in:
Swan-Sandnesquena-hermes committed 2026-07-07 04:43:46 +00:00
1 parent ad7d0abf44
commit ccd62f9b9d
2 files changed
+87 -1

No files matched your search

+12
View File
@@ -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
+75 -1
View File
@@ -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)"