Python: DevUI Fix Serialization, Timestamp and Other Issues (#1584)

* refactor(devui): adopt standard OpenAI lifecycle events for agents and workflows - Replace custom workflow events with OpenAI Responses API standard lifecycle events - Add AgentStartedEvent, AgentCompletedEvent, AgentFailedEvent for clean separation - Implement ExecutorActionItem for workflow executor tracking - Convert informational events to trace events to reduce noise - Update README mapper table with comprehensive event mappings - Maintain full backward compatibility with legacy events * fix(devui): resolve timestamp overwriting and Content serialization errors - Fix tool call timestamps being overwritten on each render (#1483) - Add recursive Content serialization to handle ChatMessage and nested objects (#1548) - Implement proper MCP tool cleanup on server shutdown - Add timestamp field to function_result.complete events - Enhance credential and client resource cleanup Fixes #1483, #1548 Partial improvements for #1476
2026-06-16 21:04:09 +08:00 · 2025-10-23 11:19:20 -07:00
parent 064ee8afbe
commit 6b66a34609
21 changed files with 1859 additions and 682 deletions
@@ -13,7 +13,14 @@ import pytest
 sys.path.insert(0, str(Path(__file__).parent.parent.parent / "main"))

 # Import Agent Framework types (assuming they are always available)
-from agent_framework._types import AgentRunResponseUpdate, ErrorContent, FunctionCallContent, Role, TextContent
+from agent_framework._types import (
+    AgentRunResponseUpdate,
+    ErrorContent,
+    FunctionCallContent,
+    FunctionResultContent,
+    Role,
+    TextContent,
+)

 from agent_framework_devui._mapper import MessageMapper
 from agent_framework_devui.models._openai_custom import AgentFrameworkRequest
@@ -79,15 +86,30 @@ async def test_critical_isinstance_bug_detection(mapper: MessageMapper, test_req


 async def test_text_content_mapping(mapper: MessageMapper, test_request: AgentFrameworkRequest) -> None:
-    """Test TextContent mapping."""
+    """Test TextContent mapping with proper OpenAI event hierarchy."""
    content = create_test_content("text", text="Hello, clean test!")
    update = create_test_agent_update([content])

    events = await mapper.convert_event(update, test_request)

-    assert len(events) == 1
-    assert events[0].type == "response.output_text.delta"
-    assert events[0].delta == "Hello, clean test!"
+    # With proper OpenAI hierarchy, we expect 3 events:
+    # 1. response.output_item.added (message)
+    # 2. response.content_part.added (text part)
+    # 3. response.output_text.delta (actual text)
+    assert len(events) == 3
+
+    # Check message output item
+    assert events[0].type == "response.output_item.added"
+    assert events[0].item.type == "message"
+    assert events[0].item.role == "assistant"
+
+    # Check content part
+    assert events[1].type == "response.content_part.added"
+    assert events[1].part.type == "output_text"
+
+    # Check text delta
+    assert events[2].type == "response.output_text.delta"
+    assert events[2].delta == "Hello, clean test!"


 async def test_function_call_mapping(mapper: MessageMapper, test_request: AgentFrameworkRequest) -> None:
@@ -108,6 +130,83 @@ async def test_function_call_mapping(mapper: MessageMapper, test_request: AgentF
    assert "TestCity" in full_json


+async def test_function_result_content_with_string_result(
+    mapper: MessageMapper, test_request: AgentFrameworkRequest
+) -> None:
+    """Test FunctionResultContent with plain string result (regular tools)."""
+    content = FunctionResultContent(
+        call_id="test_call_123",
+        result="Hello, World!",  # Plain string like regular Python function tools
+    )
+    update = create_test_agent_update([content])
+
+    events = await mapper.convert_event(update, test_request)
+
+    # Should produce response.function_result.complete event
+    assert len(events) >= 1
+    result_events = [e for e in events if e.type == "response.function_result.complete"]
+    assert len(result_events) == 1
+    assert result_events[0].output == "Hello, World!"
+    assert result_events[0].call_id == "test_call_123"
+    assert result_events[0].status == "completed"
+
+
+async def test_function_result_content_with_nested_content_objects(
+    mapper: MessageMapper, test_request: AgentFrameworkRequest
+) -> None:
+    """Test FunctionResultContent with nested Content objects (MCP tools case).
+
+    This tests the issue from GitHub #1476 where MCP tools return FunctionResultContent
+    with nested TextContent objects that fail to serialize properly.
+    """
+    # This is what MCP tools return - result contains nested Content objects
+    content = FunctionResultContent(
+        call_id="mcp_call_456",
+        result=[TextContent(text="Hello from MCP!")],  # List containing TextContent object
+    )
+    update = create_test_agent_update([content])
+
+    events = await mapper.convert_event(update, test_request)
+
+    # Should successfully serialize the nested Content object
+    assert len(events) >= 1
+    result_events = [e for e in events if e.type == "response.function_result.complete"]
+    assert len(result_events) == 1
+
+    # The output should contain the text from the nested TextContent
+    # Should not have TypeError or empty output
+    assert result_events[0].output != ""
+    assert "Hello from MCP!" in result_events[0].output
+    assert result_events[0].call_id == "mcp_call_456"
+
+
+async def test_function_result_content_with_multiple_nested_content_objects(
+    mapper: MessageMapper, test_request: AgentFrameworkRequest
+) -> None:
+    """Test FunctionResultContent with multiple nested Content objects."""
+    # MCP tools can return multiple Content objects
+    content = FunctionResultContent(
+        call_id="mcp_call_789",
+        result=[
+            TextContent(text="First result"),
+            TextContent(text="Second result"),
+        ],
+    )
+    update = create_test_agent_update([content])
+
+    events = await mapper.convert_event(update, test_request)
+
+    assert len(events) >= 1
+    result_events = [e for e in events if e.type == "response.function_result.complete"]
+    assert len(result_events) == 1
+
+    # Should serialize all nested Content objects
+    output = result_events[0].output
+    assert output != ""
+    assert "First result" in output
+    assert "Second result" in output
+
+
 async def test_error_content_mapping(mapper: MessageMapper, test_request: AgentFrameworkRequest) -> None:
    """Test ErrorContent mapping."""
    content = create_test_content("error", message="Test error", code="test_code")
@@ -182,6 +281,140 @@ async def test_agent_run_response_mapping(mapper: MessageMapper, test_request: A
    assert text_events[0].delta == "Complete response from run()"


+async def test_agent_lifecycle_events(mapper: MessageMapper, test_request: AgentFrameworkRequest) -> None:
+    """Test that agent lifecycle events are properly converted to OpenAI format."""
+    from agent_framework_devui.models._openai_custom import AgentCompletedEvent, AgentFailedEvent, AgentStartedEvent
+
+    # Test AgentStartedEvent
+    start_event = AgentStartedEvent()
+    events = await mapper.convert_event(start_event, test_request)
+
+    assert len(events) == 2  # Should emit response.created and response.in_progress
+    assert events[0].type == "response.created"
+    assert events[1].type == "response.in_progress"
+    assert events[0].response.model == "test_agent"  # Should use model from request
+    assert events[0].response.status == "in_progress"
+
+    # Test AgentCompletedEvent
+    complete_event = AgentCompletedEvent()
+    events = await mapper.convert_event(complete_event, test_request)
+
+    assert len(events) == 1
+    assert events[0].type == "response.completed"
+    assert events[0].response.status == "completed"
+
+    # Test AgentFailedEvent
+    error = Exception("Test error")
+    failed_event = AgentFailedEvent(error=error)
+    events = await mapper.convert_event(failed_event, test_request)
+
+    assert len(events) == 1
+    assert events[0].type == "response.failed"
+    assert events[0].response.status == "failed"
+    assert events[0].response.error.message == "Test error"
+    assert events[0].response.error.code == "server_error"
+
+
+@pytest.mark.skip(reason="Workflow events need real classes from agent_framework.workflows")
+async def test_workflow_lifecycle_events(mapper: MessageMapper, test_request: AgentFrameworkRequest) -> None:
+    """Test that workflow lifecycle events are properly converted to OpenAI format."""
+
+    # Create mock workflow events (since we don't have access to the real ones in tests)
+    class WorkflowStartedEvent:  # noqa: B903
+        def __init__(self, workflow_id: str):
+            self.workflow_id = workflow_id
+
+    class WorkflowCompletedEvent:  # noqa: B903
+        def __init__(self, workflow_id: str):
+            self.workflow_id = workflow_id
+
+    class WorkflowFailedEvent:  # noqa: B903
+        def __init__(self, workflow_id: str, error_info: dict | None = None):
+            self.workflow_id = workflow_id
+            self.error_info = error_info
+
+    # Test WorkflowStartedEvent
+    start_event = WorkflowStartedEvent(workflow_id="test_workflow_123")
+    events = await mapper.convert_event(start_event, test_request)
+
+    assert len(events) == 2  # Should emit response.created and response.in_progress
+    assert events[0].type == "response.created"
+    assert events[1].type == "response.in_progress"
+    assert events[0].response.model == "test_agent"  # Should use model from request
+    assert events[0].response.status == "in_progress"
+
+    # Test WorkflowCompletedEvent
+    complete_event = WorkflowCompletedEvent(workflow_id="test_workflow_123")
+    events = await mapper.convert_event(complete_event, test_request)
+
+    assert len(events) == 1
+    assert events[0].type == "response.completed"
+    assert events[0].response.status == "completed"
+
+    # Test WorkflowFailedEvent with error info
+    failed_event = WorkflowFailedEvent(workflow_id="test_workflow_123", error_info={"message": "Workflow failed"})
+    events = await mapper.convert_event(failed_event, test_request)
+
+    assert len(events) == 1
+    assert events[0].type == "response.failed"
+    assert events[0].response.status == "failed"
+    assert events[0].response.error.message == "{'message': 'Workflow failed'}"
+    assert events[0].response.error.code == "server_error"
+
+
+@pytest.mark.skip(reason="Executor events need real classes from agent_framework.workflows")
+async def test_executor_action_events(mapper: MessageMapper, test_request: AgentFrameworkRequest) -> None:
+    """Test that workflow executor events are properly converted to custom output item events."""
+
+    # Create mock executor events (since we don't have access to the real ones in tests)
+    class ExecutorInvokedEvent:  # noqa: B903
+        def __init__(self, executor_id: str, executor_type: str = "test"):
+            self.executor_id = executor_id
+            self.executor_type = executor_type
+
+    class ExecutorCompletedEvent:  # noqa: B903
+        def __init__(self, executor_id: str, result: Any = None):
+            self.executor_id = executor_id
+            self.result = result
+
+    class ExecutorFailedEvent:  # noqa: B903
+        def __init__(self, executor_id: str, error: Exception | None = None):
+            self.executor_id = executor_id
+            self.error = error
+
+    # Test ExecutorInvokedEvent
+    invoked_event = ExecutorInvokedEvent(executor_id="exec_123", executor_type="test_executor")
+    events = await mapper.convert_event(invoked_event, test_request)
+
+    assert len(events) == 1
+    assert events[0].type == "response.output_item.added"
+    assert events[0].item["type"] == "executor_action"
+    assert events[0].item["executor_id"] == "exec_123"
+    assert events[0].item["status"] == "in_progress"
+
+    # Test ExecutorCompletedEvent
+    complete_event = ExecutorCompletedEvent(executor_id="exec_123", result={"data": "success"})
+    events = await mapper.convert_event(complete_event, test_request)
+
+    assert len(events) == 1
+    assert events[0].type == "response.output_item.done"
+    assert events[0].item["type"] == "executor_action"
+    assert events[0].item["executor_id"] == "exec_123"
+    assert events[0].item["status"] == "completed"
+    assert events[0].item["result"] == {"data": "success"}
+
+    # Test ExecutorFailedEvent
+    failed_event = ExecutorFailedEvent(executor_id="exec_123", error=Exception("Executor failed"))
+    events = await mapper.convert_event(failed_event, test_request)
+
+    assert len(events) == 1
+    assert events[0].type == "response.output_item.done"
+    assert events[0].item["type"] == "executor_action"
+    assert events[0].item["executor_id"] == "exec_123"
+    assert events[0].item["status"] == "failed"
+    assert "Executor failed" in str(events[0].item["error"]["message"])
+
+
 if __name__ == "__main__":
    # Simple test runner
    async def run_all_tests() -> None:
@@ -143,6 +143,104 @@ def test_select_primary_input_type_prefers_string_and_dict():
    assert fallback is int


+@pytest.mark.asyncio
+async def test_credential_cleanup() -> None:
+    """Test that async credentials are properly closed during server cleanup."""
+    from unittest.mock import AsyncMock, Mock
+
+    from agent_framework import ChatAgent
+
+    # Create mock credential with async close
+    mock_credential = AsyncMock()
+    mock_credential.close = AsyncMock()
+
+    # Create mock chat client with credential
+    mock_client = Mock()
+    mock_client.async_credential = mock_credential
+    mock_client.model_id = "test-model"
+
+    # Create agent with mock client
+    agent = ChatAgent(name="TestAgent", chat_client=mock_client, instructions="Test agent")
+
+    # Create DevUI server with agent
+    server = DevServer()
+    server._pending_entities = [agent]
+    await server._ensure_executor()
+
+    # Run cleanup
+    await server._cleanup_entities()
+
+    # Verify credential.close() was called
+    assert mock_credential.close.called, "Async credential close should have been called"
+    assert mock_credential.close.call_count == 1
+
+
+@pytest.mark.asyncio
+async def test_credential_cleanup_error_handling() -> None:
+    """Test that credential cleanup errors are handled gracefully."""
+    from unittest.mock import AsyncMock, Mock
+
+    from agent_framework import ChatAgent
+
+    # Create mock credential that raises error on close
+    mock_credential = AsyncMock()
+    mock_credential.close = AsyncMock(side_effect=Exception("Close failed"))
+
+    # Create mock chat client with credential
+    mock_client = Mock()
+    mock_client.async_credential = mock_credential
+    mock_client.model_id = "test-model"
+
+    # Create agent with mock client
+    agent = ChatAgent(name="TestAgent", chat_client=mock_client, instructions="Test agent")
+
+    # Create DevUI server with agent
+    server = DevServer()
+    server._pending_entities = [agent]
+    await server._ensure_executor()
+
+    # Run cleanup - should not raise despite credential error
+    await server._cleanup_entities()
+
+    # Verify close was attempted
+    assert mock_credential.close.called
+
+
+@pytest.mark.asyncio
+async def test_multiple_credential_attributes() -> None:
+    """Test that we check all common credential attribute names."""
+    from unittest.mock import AsyncMock, Mock
+
+    from agent_framework import ChatAgent
+
+    # Create mock credentials
+    mock_cred1 = Mock()
+    mock_cred1.close = Mock()
+    mock_cred2 = AsyncMock()
+    mock_cred2.close = AsyncMock()
+
+    # Create mock chat client with multiple credential attributes
+    mock_client = Mock()
+    mock_client.credential = mock_cred1
+    mock_client.async_credential = mock_cred2
+    mock_client.model_id = "test-model"
+
+    # Create agent with mock client
+    agent = ChatAgent(name="TestAgent", chat_client=mock_client, instructions="Test agent")
+
+    # Create DevUI server with agent
+    server = DevServer()
+    server._pending_entities = [agent]
+    await server._ensure_executor()
+
+    # Run cleanup
+    await server._cleanup_entities()
+
+    # Verify both credentials were closed
+    assert mock_cred1.close.called, "Sync credential should be closed"
+    assert mock_cred2.close.called, "Async credential should be closed"
+
+
 if __name__ == "__main__":
    # Simple test runner
    async def run_tests():