fix: address code review feedback for after_timestamp feature

- Add _make_timestamp_filter() helper to eliminate code duplication
- Convert datetime filters to ISO strings once (O(1) vs O(n) parsing)
- Add unit tests for normalize_datetime_to_server_timezone()
- Add unit tests for _make_timestamp_filter() helper
- Add behavioral tests that verify actual filtering (not just mock calls)
- Add warning log when after_timestamp provided without resend_all
- Add Query description for after_timestamp in OpenAPI docs

Co-authored-by: openhands <openhands@all-hands.dev>

Author: jpshackelford

openhands-agent-server/openhands/agent_server/sockets.py

@@ -94,7 +94,16 @@ async def events_socket(
     websocket: WebSocket,
     session_api_key: Annotated[str | None, Query(alias="session_api_key")] = None,
     resend_all: Annotated[bool, Query()] = False,
-    after_timestamp: Annotated[datetime | None, Query()] = None,
+    after_timestamp: Annotated[
+        datetime | None,
+        Query(
+            description=(
+                "Filter events to timestamps >= this value when resend_all=True. "
+                "Accepts ISO 8601 format. Timezone-aware datetimes are converted "
+                "to server local time; naive datetimes assumed in server timezone."
+            )
+        ),
+    ] = None,
 ):
     """WebSocket endpoint for conversation events.
 
@@ -130,6 +139,13 @@ async def events_socket(
         else None
     )
 
+    # Warn if after_timestamp is provided without resend_all
+    if after_timestamp and not resend_all:
+        logger.warning(
+            f"after_timestamp provided without resend_all=True, "
+            f"will be ignored: {conversation_id}"
+        )
+
     try:
         # Resend existing events if requested
         if resend_all:

tests/agent_server/test_event_router.py

@@ -1,5 +1,6 @@
 """Tests for event_router.py endpoints."""
 
+from datetime import UTC, datetime, timezone
 from pathlib import Path
 from typing import cast
 from unittest.mock import AsyncMock, MagicMock
@@ -10,14 +11,76 @@
 from fastapi.testclient import TestClient
 
 from openhands.agent_server.dependencies import get_event_service
-from openhands.agent_server.event_router import event_router
+from openhands.agent_server.event_router import (
+    event_router,
+    normalize_datetime_to_server_timezone,
+)
 from openhands.agent_server.event_service import EventService
 from openhands.agent_server.models import SendMessageRequest
 from openhands.sdk import Message
 from openhands.sdk.event.llm_convertible.message import MessageEvent
 from openhands.sdk.llm.message import ImageContent, TextContent
 
 
+class TestNormalizeDatetimeToServerTimezone:
+    """Unit tests for normalize_datetime_to_server_timezone function."""
+
+    def test_naive_datetime_passthrough(self):
+        """Naive datetimes should be returned unchanged."""
+        naive_dt = datetime(2025, 1, 15, 10, 30, 0)
+        result = normalize_datetime_to_server_timezone(naive_dt)
+
+        assert result == naive_dt
+        assert result.tzinfo is None
+
+    def test_utc_datetime_converted_to_naive(self):
+        """UTC datetime should be converted to server local time and made naive."""
+        utc_dt = datetime(2025, 1, 15, 10, 30, 0, tzinfo=UTC)
+        result = normalize_datetime_to_server_timezone(utc_dt)
+
+        # Result should be naive
+        assert result.tzinfo is None
+        # Result should represent the same instant converted to local time
+        expected = utc_dt.astimezone(None).replace(tzinfo=None)
+        assert result == expected
+
+    def test_non_utc_timezone_converted_to_naive(self):
+        """Non-UTC timezone-aware datetime should be converted to server local time."""
+        # Create a timezone-aware datetime in UTC
+        aware_dt = datetime(2025, 6, 15, 18, 30, 0, tzinfo=UTC)
+
+        result = normalize_datetime_to_server_timezone(aware_dt)
+
+        # Result should be naive
+        assert result.tzinfo is None
+        # Result should represent the same instant in server local time
+        expected = aware_dt.astimezone(None).replace(tzinfo=None)
+        assert result == expected
+
+    def test_preserves_microseconds(self):
+        """Microseconds should be preserved through conversion."""
+        utc_dt = datetime(2025, 1, 15, 10, 30, 0, 123456, tzinfo=UTC)
+        result = normalize_datetime_to_server_timezone(utc_dt)
+
+        assert result.microsecond == 123456
+
+    def test_different_fixed_offset_timezone(self):
+        """Test with a specific fixed offset timezone."""
+        # UTC+5:30 (like India Standard Time)
+        from datetime import timedelta
+
+        ist = timezone(timedelta(hours=5, minutes=30))
+        ist_dt = datetime(2025, 1, 15, 16, 0, 0, tzinfo=ist)  # 4:00 PM IST
+
+        result = normalize_datetime_to_server_timezone(ist_dt)
+
+        # Should be naive
+        assert result.tzinfo is None
+        # Should be the same instant converted to server local time
+        expected = ist_dt.astimezone(None).replace(tzinfo=None)
+        assert result == expected
+
+
 @pytest.fixture
 def client():
     """Create a test client for the FastAPI app without authentication."""

tests/agent_server/test_event_router_websocket.py

@@ -647,3 +647,162 @@ async def test_resend_all_without_after_timestamp(
         mock_event_service.search_events.assert_called_once_with(
             page_id=None, timestamp__gte=None
         )
+
+
+class TestAfterTimestampFilteringBehavioral:
+    """Behavioral tests that verify actual filtering works with real events.
+
+    These tests use real EventService instances with mock conversations
+    to verify that timestamp filtering actually produces correct results,
+    not just that the right methods were called.
+    """
+
+    @pytest.fixture
+    def event_service_with_timestamped_events(self):
+        """Create a real EventService with timestamped events for testing."""
+        from pathlib import Path
+
+        from openhands.agent_server.event_service import EventService
+        from openhands.agent_server.models import StoredConversation
+        from openhands.sdk import LLM, Agent
+        from openhands.sdk.security.confirmation_policy import NeverConfirm
+        from openhands.sdk.workspace import LocalWorkspace
+
+        stored = StoredConversation(
+            id=uuid4(),
+            agent=Agent(llm=LLM(model="gpt-4", usage_id="test-llm"), tools=[]),
+            workspace=LocalWorkspace(working_dir="workspace/project"),
+            confirmation_policy=NeverConfirm(),
+            initial_message=None,
+            metrics=None,
+        )
+        service = EventService(
+            stored=stored, conversations_dir=Path("test_conversation_dir")
+        )
+
+        # Create mock conversation with timestamped events
+        from unittest.mock import MagicMock
+
+        from openhands.sdk import Conversation
+        from openhands.sdk.conversation.state import ConversationState
+
+        conversation = MagicMock(spec=Conversation)
+        state = MagicMock(spec=ConversationState)
+
+        # Events with specific timestamps spanning 10:00 to 14:00
+        timestamps = [
+            "2025-01-01T10:00:00.000000",
+            "2025-01-01T11:00:00.000000",
+            "2025-01-01T12:00:00.000000",
+            "2025-01-01T13:00:00.000000",
+            "2025-01-01T14:00:00.000000",
+        ]
+
+        events = []
+        for index, timestamp in enumerate(timestamps, 1):
+            event = MessageEvent(
+                id=f"event{index}",
+                source="user",
+                llm_message=Message(
+                    role="user", content=[TextContent(text=f"Message {index}")]
+                ),
+                timestamp=timestamp,
+            )
+            events.append(event)
+
+        state.events = events
+        state.__enter__ = MagicMock(return_value=state)
+        state.__exit__ = MagicMock(return_value=None)
+        conversation._state = state
+
+        service._conversation = conversation
+        return service
+
+    @pytest.mark.asyncio
+    async def test_timestamp_filter_returns_correct_events(
+        self, event_service_with_timestamped_events
+    ):
+        """Test that timestamp filtering returns only events >= the filter time."""
+        from datetime import datetime
+
+        service = event_service_with_timestamped_events
+
+        # Filter for events >= 12:00:00 (should return events 3, 4, 5)
+        filter_time = datetime(2025, 1, 1, 12, 0, 0)
+        result = await service.search_events(timestamp__gte=filter_time)
+
+        # Should return exactly 3 events
+        assert len(result.items) == 3
+
+        # Verify the correct events were returned
+        returned_ids = [event.id for event in result.items]
+        assert "event3" in returned_ids
+        assert "event4" in returned_ids
+        assert "event5" in returned_ids
+
+        # Events 1 and 2 should NOT be returned
+        assert "event1" not in returned_ids
+        assert "event2" not in returned_ids
+
+    @pytest.mark.asyncio
+    async def test_timestamp_filter_boundary_condition(
+        self, event_service_with_timestamped_events
+    ):
+        """Test that filter boundary is inclusive (>=)."""
+        from datetime import datetime
+
+        service = event_service_with_timestamped_events
+
+        # Filter for events >= exactly 12:00:00 (event3's timestamp)
+        filter_time = datetime(2025, 1, 1, 12, 0, 0)
+        result = await service.search_events(timestamp__gte=filter_time)
+
+        # Event3 at exactly 12:00:00 should be included
+        returned_ids = [event.id for event in result.items]
+        assert "event3" in returned_ids
+
+    @pytest.mark.asyncio
+    async def test_timestamp_filter_no_matches(
+        self, event_service_with_timestamped_events
+    ):
+        """Test that filter returns empty when no events match."""
+        from datetime import datetime
+
+        service = event_service_with_timestamped_events
+
+        # Filter for events >= 15:00:00 (no events exist after 14:00)
+        filter_time = datetime(2025, 1, 1, 15, 0, 0)
+        result = await service.search_events(timestamp__gte=filter_time)
+
+        assert len(result.items) == 0
+
+    @pytest.mark.asyncio
+    async def test_timestamp_filter_all_events_match(
+        self, event_service_with_timestamped_events
+    ):
+        """Test that all events are returned when filter is before all events."""
+        from datetime import datetime
+
+        service = event_service_with_timestamped_events
+
+        # Filter for events >= 09:00:00 (before all events)
+        filter_time = datetime(2025, 1, 1, 9, 0, 0)
+        result = await service.search_events(timestamp__gte=filter_time)
+
+        # Should return all 5 events
+        assert len(result.items) == 5
+
+    @pytest.mark.asyncio
+    async def test_count_events_with_timestamp_filter(
+        self, event_service_with_timestamped_events
+    ):
+        """Test that count_events also respects timestamp filtering."""
+        from datetime import datetime
+
+        service = event_service_with_timestamped_events
+
+        # Count events >= 12:00:00 (should be 3)
+        filter_time = datetime(2025, 1, 1, 12, 0, 0)
+        count = await service.count_events(timestamp__gte=filter_time)
+
+        assert count == 3

tests/agent_server/test_event_service.py

@@ -8,7 +8,7 @@
 
 import pytest
 
-from openhands.agent_server.event_service import EventService
+from openhands.agent_server.event_service import EventService, _make_timestamp_filter
 from openhands.agent_server.models import (
     ConfirmationResponseRequest,
     EventPage,
@@ -29,6 +29,72 @@
 from openhands.sdk.workspace import LocalWorkspace
 
 
+class TestMakeTimestampFilter:
+    """Unit tests for _make_timestamp_filter helper function."""
+
+    def test_no_filters_returns_none(self):
+        """When no filters provided, should return None."""
+        result = _make_timestamp_filter(None, None)
+        assert result is None
+
+    def test_gte_filter_only(self):
+        """Test filtering with only timestamp__gte."""
+        filter_time = datetime(2025, 1, 1, 12, 0, 0)
+        filter_fn = _make_timestamp_filter(filter_time, None)
+
+        assert filter_fn is not None
+        # Event at 11:00 should be excluded
+        assert filter_fn("2025-01-01T11:00:00.000000") is False
+        # Event at exactly 12:00 should be included
+        assert filter_fn("2025-01-01T12:00:00.000000") is True
+        # Event at 13:00 should be included
+        assert filter_fn("2025-01-01T13:00:00.000000") is True
+
+    def test_lt_filter_only(self):
+        """Test filtering with only timestamp__lt."""
+        filter_time = datetime(2025, 1, 1, 12, 0, 0)
+        filter_fn = _make_timestamp_filter(None, filter_time)
+
+        assert filter_fn is not None
+        # Event at 11:00 should be included
+        assert filter_fn("2025-01-01T11:00:00.000000") is True
+        # Event at exactly 12:00 should be excluded
+        assert filter_fn("2025-01-01T12:00:00.000000") is False
+        # Event at 13:00 should be excluded
+        assert filter_fn("2025-01-01T13:00:00.000000") is False
+
+    def test_range_filter(self):
+        """Test filtering with both gte and lt (range filter)."""
+        gte_time = datetime(2025, 1, 1, 11, 0, 0)
+        lt_time = datetime(2025, 1, 1, 13, 0, 0)
+        filter_fn = _make_timestamp_filter(gte_time, lt_time)
+
+        assert filter_fn is not None
+        # Event at 10:00 should be excluded (before range)
+        assert filter_fn("2025-01-01T10:00:00.000000") is False
+        # Event at 11:00 should be included (start of range, inclusive)
+        assert filter_fn("2025-01-01T11:00:00.000000") is True
+        # Event at 12:00 should be included (middle of range)
+        assert filter_fn("2025-01-01T12:00:00.000000") is True
+        # Event at 13:00 should be excluded (end of range, exclusive)
+        assert filter_fn("2025-01-01T13:00:00.000000") is False
+        # Event at 14:00 should be excluded (after range)
+        assert filter_fn("2025-01-01T14:00:00.000000") is False
+
+    def test_microseconds_handled_correctly(self):
+        """Test that microseconds are handled in string comparison."""
+        filter_time = datetime(2025, 1, 1, 12, 0, 0, 500000)  # 12:00:00.500000
+        filter_fn = _make_timestamp_filter(filter_time, None)
+
+        assert filter_fn is not None
+        # Event at 12:00:00.400000 should be excluded
+        assert filter_fn("2025-01-01T12:00:00.400000") is False
+        # Event at 12:00:00.500000 should be included (exact match)
+        assert filter_fn("2025-01-01T12:00:00.500000") is True
+        # Event at 12:00:00.600000 should be included
+        assert filter_fn("2025-01-01T12:00:00.600000") is True
+
+
 @pytest.fixture
 def sample_stored_conversation():
     """Create a sample StoredConversation for testing."""