Skip to content

Commit 990e8df

Browse files
Copilotpontemonti
andcommitted
Fix send_chat_history_messages to send empty lists to API
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>
1 parent bb1f5b8 commit 990e8df

6 files changed

Lines changed: 59 additions & 48 deletions

File tree

‎libraries/microsoft-agents-a365-tooling-extensions-agentframework/microsoft_agents_a365/tooling/extensions/agentframework/services/mcp_tool_registration_service.py‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -221,10 +221,12 @@ async def send_chat_history_messages(
221221
Send chat history messages to the MCP platform for real-time threat protection.
222222
223223
This is the primary implementation method that handles message conversion
224-
and delegation to the core tooling service.
224+
and delegation to the core tooling service. Empty message lists are valid
225+
and will be sent to the API.
225226
226227
Args:
227228
chat_messages: Sequence of Agent Framework ChatMessage objects to send.
229+
Empty lists are valid and will be sent to the API.
228230
turn_context: TurnContext from the Agents SDK containing conversation info.
229231
tool_options: Optional configuration for the request. Defaults to
230232
AgentFramework-specific options if not provided.
@@ -249,11 +251,6 @@ async def send_chat_history_messages(
249251
if turn_context is None:
250252
raise ValueError("turn_context cannot be None")
251253

252-
# Handle empty messages - return success with warning
253-
if len(chat_messages) == 0:
254-
self._logger.warning("Empty message list provided to send_chat_history_messages")
255-
return OperationResult.success()
256-
257254
self._logger.info(f"Send chat history initiated with {len(chat_messages)} messages")
258255

259256
# Use default options if not provided
@@ -263,10 +260,7 @@ async def send_chat_history_messages(
263260
# Convert messages to ChatHistoryMessage format
264261
history_messages = self._convert_chat_messages_to_history(chat_messages)
265262

266-
# Check if all messages were filtered out during conversion
267-
if len(history_messages) == 0:
268-
self._logger.warning("All messages were filtered out during conversion (empty content)")
269-
return OperationResult.success()
263+
self._logger.debug(f"Converted {len(history_messages)} messages to ChatHistoryMessage format")
270264

271265
# Delegate to core service
272266
result = await self._mcp_server_configuration_service.send_chat_history(

‎libraries/microsoft-agents-a365-tooling-extensions-openai/microsoft_agents_a365/tooling/extensions/openai/mcp_tool_registration_service.py‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -340,14 +340,15 @@ async def send_chat_history_messages(
340340
341341
This method accepts a list of OpenAI TResponseInputItem messages, converts
342342
them to ChatHistoryMessage format, and sends them to the MCP platform.
343+
Empty message lists are valid and will be sent to the API.
343344
344345
Args:
345346
turn_context: TurnContext from the Agents SDK containing conversation info.
346347
Must have a valid activity with conversation.id, activity.id,
347348
and activity.text.
348349
messages: List of OpenAI TResponseInputItem messages to send. Supports
349350
UserMessage, AssistantMessage, SystemMessage, and other OpenAI
350-
message types.
351+
message types. Empty lists are valid and will be sent to the API.
351352
options: Optional ToolOptions for customization. If not provided,
352353
uses default options with orchestrator_name="OpenAI".
353354
@@ -382,11 +383,6 @@ async def send_chat_history_messages(
382383
if messages is None:
383384
raise ValueError("messages cannot be None")
384385

385-
# Handle empty list as no-op
386-
if len(messages) == 0:
387-
self._logger.info("Empty message list provided, returning success")
388-
return OperationResult.success()
389-
390386
self._logger.info(f"Sending {len(messages)} OpenAI messages as chat history")
391387

392388
# Set default options
@@ -399,10 +395,6 @@ async def send_chat_history_messages(
399395
# Convert OpenAI messages to ChatHistoryMessage format
400396
chat_history_messages = self._convert_openai_messages_to_chat_history(messages)
401397

402-
if len(chat_history_messages) == 0:
403-
self._logger.warning("No messages could be converted to chat history format")
404-
return OperationResult.success()
405-
406398
self._logger.debug(
407399
f"Converted {len(chat_history_messages)} messages to ChatHistoryMessage format"
408400
)

‎libraries/microsoft-agents-a365-tooling/microsoft_agents_a365/tooling/services/mcp_tool_server_configuration_service.py‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -569,7 +569,7 @@ async def send_chat_history(
569569
Must have a valid activity with conversation.id, activity.id, and
570570
activity.text.
571571
chat_history_messages: List of ChatHistoryMessage objects representing the chat
572-
history. Must be non-empty.
572+
history. Empty lists are valid and will be sent to the API.
573573
options: Optional ToolOptions instance containing optional parameters.
574574
575575
Returns:
@@ -578,7 +578,7 @@ async def send_chat_history(
578578
On failure, returns OperationResult.failed() with error details.
579579
580580
Raises:
581-
ValueError: If turn_context is None, chat_history_messages is None or empty,
581+
ValueError: If turn_context is None, chat_history_messages is None,
582582
turn_context.activity is None, or any of the required fields
583583
(conversation.id, activity.id, activity.text) are missing or empty.
584584
@@ -602,11 +602,6 @@ async def send_chat_history(
602602
if chat_history_messages is None:
603603
raise ValueError("chat_history_messages cannot be None")
604604

605-
# Handle empty messages - return success with warning (consistent with extension behavior)
606-
if len(chat_history_messages) == 0:
607-
self._logger.warning("Empty message list provided to send_chat_history")
608-
return OperationResult.success()
609-
610605
# Extract required information from turn context
611606
if not turn_context.activity:
612607
raise ValueError("turn_context.activity cannot be None")

‎tests/tooling/extensions/agentframework/services/test_send_chat_history.py‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -120,17 +120,19 @@ async def test_send_chat_history_from_store_validates_turn_context_none(
120120

121121
@pytest.mark.asyncio
122122
@pytest.mark.unit
123-
async def test_send_chat_history_messages_empty_messages_returns_success(
123+
async def test_send_chat_history_messages_empty_messages_calls_api(
124124
self, service, mock_turn_context
125125
):
126-
"""Test that empty message list returns success with warning log."""
126+
"""Test that empty message list is sent to the API."""
127127
# Act
128128
result = await service.send_chat_history_messages([], mock_turn_context)
129129

130130
# Assert
131131
assert result.succeeded is True
132-
# Core service should not be called for empty messages
133-
service._mcp_server_configuration_service.send_chat_history.assert_not_called()
132+
# Core service should be called with empty list
133+
service._mcp_server_configuration_service.send_chat_history.assert_called_once()
134+
call_args = service._mcp_server_configuration_service.send_chat_history.call_args
135+
assert call_args.kwargs["chat_history_messages"] == []
134136

135137
@pytest.mark.asyncio
136138
@pytest.mark.unit
@@ -492,10 +494,10 @@ async def test_send_chat_history_messages_skips_messages_with_none_role(
492494

493495
@pytest.mark.asyncio
494496
@pytest.mark.unit
495-
async def test_send_chat_history_messages_all_filtered_returns_success(
497+
async def test_send_chat_history_messages_all_filtered_calls_api_with_empty_list(
496498
self, service, mock_turn_context, mock_role
497499
):
498-
"""Test that all messages filtered out returns success without calling core (CRM-006)."""
500+
"""Test that all messages filtered out calls API with empty list (CRM-006)."""
499501
# Arrange - all messages have empty content
500502
msg1 = Mock()
501503
msg1.message_id = "msg-1"
@@ -517,8 +519,10 @@ async def test_send_chat_history_messages_all_filtered_returns_success(
517519

518520
# Assert
519521
assert result.succeeded is True
520-
# Core service should not be called when all messages are filtered out
521-
service._mcp_server_configuration_service.send_chat_history.assert_not_called()
522+
# Core service should be called with empty list when all messages are filtered out
523+
service._mcp_server_configuration_service.send_chat_history.assert_called_once()
524+
call_args = service._mcp_server_configuration_service.send_chat_history.call_args
525+
assert call_args.kwargs["chat_history_messages"] == []
522526

523527
@pytest.mark.asyncio
524528
@pytest.mark.unit

‎tests/tooling/extensions/openai/test_send_chat_history.py‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,25 @@ async def test_send_chat_history_messages_validates_messages_none(
4545
# UV-03
4646
@pytest.mark.asyncio
4747
@pytest.mark.unit
48-
async def test_send_chat_history_messages_empty_list_returns_success(
48+
async def test_send_chat_history_messages_empty_list_calls_api(
4949
self, service, mock_turn_context
5050
):
51-
"""Test that empty message list returns success (no-op)."""
52-
result = await service.send_chat_history_messages(mock_turn_context, [])
51+
"""Test that empty message list is sent to the API."""
52+
with patch.object(
53+
service.config_service,
54+
"send_chat_history",
55+
new_callable=AsyncMock,
56+
) as mock_send:
57+
mock_send.return_value = OperationResult.success()
5358

54-
assert result.succeeded is True
55-
assert len(result.errors) == 0
59+
result = await service.send_chat_history_messages(mock_turn_context, [])
60+
61+
assert result.succeeded is True
62+
assert len(result.errors) == 0
63+
# Verify the API was called with empty list
64+
mock_send.assert_called_once()
65+
call_args = mock_send.call_args
66+
assert call_args.kwargs["chat_history_messages"] == []
5667

5768
# UV-04
5869
@pytest.mark.asyncio

‎tests/tooling/services/test_send_chat_history.py‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -146,14 +146,29 @@ async def test_send_chat_history_validates_chat_history_messages(
146146
await service.send_chat_history(mock_turn_context, None)
147147

148148
@pytest.mark.asyncio
149-
async def test_send_chat_history_empty_list_returns_success(self, service, mock_turn_context):
150-
"""Test that send_chat_history returns success for empty list (CRM-008)."""
151-
# Act
152-
result = await service.send_chat_history(mock_turn_context, [])
153-
154-
# Assert - empty list should return success, not raise exception
155-
assert result.succeeded is True
156-
assert len(result.errors) == 0
149+
async def test_send_chat_history_empty_list_calls_api(self, service, mock_turn_context):
150+
"""Test that send_chat_history sends empty list to API (CRM-008)."""
151+
# Arrange
152+
mock_response = AsyncMock()
153+
mock_response.status = 200
154+
mock_response.text = AsyncMock(return_value="OK")
155+
156+
# Mock aiohttp.ClientSession
157+
with patch("aiohttp.ClientSession") as mock_session:
158+
mock_session_instance = MagicMock()
159+
mock_post = AsyncMock()
160+
mock_post.__aenter__.return_value = mock_response
161+
mock_session_instance.post.return_value = mock_post
162+
mock_session.return_value.__aenter__.return_value = mock_session_instance
163+
164+
# Act
165+
result = await service.send_chat_history(mock_turn_context, [])
166+
167+
# Assert - empty list should call API and return success
168+
assert result.succeeded is True
169+
assert len(result.errors) == 0
170+
# Verify the API was called
171+
assert mock_session_instance.post.called
157172

158173
@pytest.mark.asyncio
159174
async def test_send_chat_history_validates_activity(self, service, chat_history_messages):

0 commit comments

Comments
 (0)