diff --git a/changelog/13019.bugfix.md b/changelog/13019.bugfix.md new file mode 100644 index 000000000000..9ce820d61dbe --- /dev/null +++ b/changelog/13019.bugfix.md @@ -0,0 +1 @@ +Changed the ordering of returned events to order by ID (previously timestamp) in SQL Tracker Store diff --git a/rasa/core/exporter.py b/rasa/core/exporter.py index 58c567dbdbe2..2961b6b6fc94 100644 --- a/rasa/core/exporter.py +++ b/rasa/core/exporter.py @@ -160,7 +160,7 @@ def _validate_all_requested_ids_exist( self, conversation_ids_in_tracker_store: Set[Text] ) -> None: """Warn user if `self.requested_conversation_ids` contains IDs not found in - `conversation_ids_in_tracker_store` + `conversation_ids_in_tracker_store`. Args: conversation_ids_in_tracker_store: Set of conversation IDs contained in @@ -241,6 +241,12 @@ async def _fetch_events_within_time_range(self) -> AsyncIterator[Dict[Text, Any] continue events = self._get_events_for_conversation_id(_events, conversation_id) + + # the order of events was changed after ATO-2192 + # more context: https://github.com/RasaHQ/rasa/pull/13019 + # we should sort the events by timestamp to keep the order + events.sort(key=lambda x: x["timestamp"]) + # the conversation IDs are needed in the event publishing for event in events: if ( diff --git a/rasa/core/tracker_store.py b/rasa/core/tracker_store.py index 93632cd3d779..a91f9f5c6b87 100644 --- a/rasa/core/tracker_store.py +++ b/rasa/core/tracker_store.py @@ -1291,6 +1291,9 @@ def _event_query( self, session: "Session", sender_id: Text, fetch_events_from_all_sessions: bool ) -> "Query": """Provide the query to retrieve the conversation events for a specific sender. + The events are ordered by ID to ensure correct sequence of events. + As `timestamp` is not guaranteed to be unique and low-precision (float), it + cannot be used to order the events. Args: session: Current database session. @@ -1325,7 +1328,7 @@ def _event_query( ) ) - return event_query.order_by(self.SQLEvent.timestamp) + return event_query.order_by(self.SQLEvent.id) async def save(self, tracker: DialogueStateTracker) -> None: """Update database with events from the current conversation.""" diff --git a/tests/core/test_exporter.py b/tests/core/test_exporter.py index b74849c6bb33..0ea78f4b397f 100644 --- a/tests/core/test_exporter.py +++ b/tests/core/test_exporter.py @@ -73,9 +73,9 @@ async def test_fetch_events_within_time_range(): conversation_ids = ["some-id", "another-id"] # prepare events from different senders and different timestamps - event_1 = random_user_uttered_event(3) - event_2 = random_user_uttered_event(2) - event_3 = random_user_uttered_event(1) + event_1 = random_user_uttered_event(1) + event_2 = random_user_uttered_event(3) + event_3 = random_user_uttered_event(2) events = {conversation_ids[0]: [event_1, event_2], conversation_ids[1]: [event_3]} def _get_tracker(conversation_id: Text) -> DialogueStateTracker: diff --git a/tests/core/test_tracker_stores.py b/tests/core/test_tracker_stores.py index fb4a891b097e..4a05f370e5ff 100644 --- a/tests/core/test_tracker_stores.py +++ b/tests/core/test_tracker_stores.py @@ -614,6 +614,36 @@ async def test_sql_additional_events_with_session_start(domain: Domain): assert isinstance(additional_events[0], UserUttered) +async def test_tracker_store_retrieve_ordered_by_id( + domain: Domain, +): + tracker_store_kwargs = {"host": "sqlite:///"} + tracker_store = SQLTrackerStore(domain, **tracker_store_kwargs) + events = [ + SessionStarted(timestamp=1), + UserUttered("Hola", {"name": "greet"}, timestamp=2), + BotUttered("Hi", timestamp=2), + UserUttered("How are you?", {"name": "greet"}, timestamp=2), + BotUttered("I am good, whats up", timestamp=2), + UserUttered("Ciao", {"name": "greet"}, timestamp=2), + BotUttered("Bye", timestamp=2), + ] + sender_id = "test_sql_tracker_store_events_order" + tracker = DialogueStateTracker.from_events(sender_id, events) + await tracker_store.save(tracker) + + # Save other tracker to ensure that we don't run into problems with other senders + other_tracker = DialogueStateTracker.from_events("other-sender", [SessionStarted()]) + await tracker_store.save(other_tracker) + + # Retrieve tracker with events since latest SessionStarted + tracker = await tracker_store.retrieve(sender_id) + + assert len(tracker.events) == 7 + # assert the order of events is same as the order in which they were added + assert all((event == tracker.events[i] for i, event in enumerate(events))) + + @pytest.mark.parametrize( "tracker_store_type,tracker_store_kwargs", [(MockedMongoTrackerStore, {}), (SQLTrackerStore, {"host": "sqlite:///"})],