From e09ccbaac956b82b5167a4a6c63ddd56624b8a02 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Fri, 7 Aug 2026 19:53:02 -0700 Subject: [PATCH] Release exp-v0.52.174: regression coverage for empty tool_calls drop on mirror path (#6803) (#6844) * test(streaming): cover empty tool_calls drop in _api_safe_message_positions (#6796) * Release exp-v0.52.174: regression coverage for empty tool_calls drop on mirror path (#6803) --------- Co-authored-by: Webtecnica Bot Co-authored-by: nesquena-hermes --- CHANGELOG.md | 2 ++ tests/test_context_history_sanitization.py | 41 ++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2951168..56f424d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ - **Internal: clarified the transcript-virtualization boot comment (opt-in default retained).** The comment above the boot-time `_virtualizeTranscript` apply now records that the #4346 Phase B footer-jitter fix resolved the original scroll-up flicker root cause, while virtualization stays opt-in (default OFF) until battle-tested further. Zero behavior change — the apply predicate, settings-load fallback, checkbox, and server default all remain opt-in/OFF and in agreement. Thanks @webtecnica. (#6318, #6155) +- **Internal: added regression coverage for the empty-`tool_calls` drop on the metadata-restoration mirror path.** `_api_safe_message_positions()` must apply the same `tool_calls: []` drop as the main API sanitizer (#5737) so metadata restoration never treats a `tool_calls: []` row as a distinct message; a new test pins that empty arrays lose the key while populated tool-call chains (and their tool results) survive untouched. Test-only, no production change. Thanks @webtecnica. (#6803, #6796) + - **The busy-time send behavior is now called "Default message mode," and new installs default to Steer.** The Settings → Preferences control formerly labeled "Busy input mode" is renamed to "Default message mode," and a fresh install now defaults to **Steer** (inject a mid-turn correction without interrupting) instead of Queue. Your existing choice is preserved — if you ever saved settings, your current mode (Queue/Interrupt/Steer) is migrated as-is and unchanged; only never-configured installs pick up the new Steer default. The saved preference still survives a reload or a brief server outage (the localStorage mirror from the previous release is intact). Thanks @rodboev. (#5162, #5145) ### Added diff --git a/tests/test_context_history_sanitization.py b/tests/test_context_history_sanitization.py index f412e425..a65b20fa 100644 --- a/tests/test_context_history_sanitization.py +++ b/tests/test_context_history_sanitization.py @@ -7,6 +7,7 @@ from types import SimpleNamespace from unittest.mock import patch from api.streaming import ( + _api_safe_message_positions, _compact_image_parts_for_persistence, _compact_session_image_parts_for_persistence, _restore_reasoning_metadata, @@ -309,6 +310,46 @@ def test_sanitize_messages_drops_empty_tool_calls_array(): "no message may ship an empty tool_calls array" +def test_api_safe_message_positions_drops_empty_tool_calls_array(): + """#5737 mirror path: _api_safe_message_positions must apply the same + empty-tool_calls drop as _sanitize_messages_for_api, so metadata + restoration (which aligns rows against these positions) never treats a + tool_calls: [] row as a different message. Empty array -> key removed; + populated tool_calls chain -> preserved.""" + 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"}, + ] + + positions = _api_safe_message_positions(messages) + + sanitized = [msg for _idx, msg in positions] + # 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" + # 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 row that reaches the API carries an empty tool_calls array. + for _idx, msg in positions: + assert not ("tool_calls" in msg and not msg["tool_calls"]), \ + "no API-safe position 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