diff --git a/CHANGELOG.md b/CHANGELOG.md index f586118a..5f570efe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,10 +8,6 @@ and this project adheres to ## [Unreleased] -## Fixed - -- 🐛(transcription) fix silent bug in speaker assignment - ### Changed - ⬆️(frontend) upgrade @mediapipe/tasks-vision from 0.10.14 to 0.10.35 @@ -20,6 +16,11 @@ and this project adheres to - ⬆️(frontend) upgrade @tanstack/react-query from 5.101.0 to 5.101.1 - ⬆️(frontend) upgrade livekit-client from 2.19.2 to 2.20.0 +## Fixed + +- 🐛(transcription) fix silent bug in speaker assignment +- 🐛(backend) fix bug linked to recording state inconsistency + ## [1.24.0] - 2026-07-21 ### Added diff --git a/src/backend/core/recording/services/recording_events.py b/src/backend/core/recording/services/recording_events.py index 3bebac0d..3b11315f 100644 --- a/src/backend/core/recording/services/recording_events.py +++ b/src/backend/core/recording/services/recording_events.py @@ -9,6 +9,11 @@ from livekit import api from core import models, utils from core.models import Recording from core.recording.event.notification import notification_service +from core.services.room_management import ( + RoomManagement, + RoomManagementException, + RoomNotFoundException, +) logger = getLogger(__name__) @@ -39,10 +44,15 @@ class RecordingEventsService: recording_status = status_mapping.get(egress_status) if recording_status: try: - utils.update_room_metadata( + RoomManagement().update_metadata( room_name, {"recording_status": recording_status} ) - except utils.MetadataUpdateException as e: + except RoomNotFoundException: + logger.info( + "LiveKit room %s no longer exists, skipping metadata update", + room_name, + ) + except RoomManagementException as e: logger.exception("Failed to update room's metadata: %s", e) @staticmethod diff --git a/src/backend/core/recording/worker/mediator.py b/src/backend/core/recording/worker/mediator.py index c73a96fe..2de8be05 100644 --- a/src/backend/core/recording/worker/mediator.py +++ b/src/backend/core/recording/worker/mediator.py @@ -2,8 +2,12 @@ import logging -from core import utils from core.models import Recording, RecordingStatusChoices +from core.services.room_management import ( + RoomManagement, + RoomManagementException, + RoomNotFoundException, +) from .exceptions import ( RecordingStartError, @@ -64,10 +68,15 @@ class WorkerServiceMediator: mode = recording.options.get("original_mode", None) or recording.mode try: - utils.update_room_metadata( + RoomManagement().update_metadata( room_name, {"recording_mode": mode, "recording_status": "starting"} ) - except utils.MetadataUpdateException as e: + except RoomNotFoundException: + logger.info( + "LiveKit room %s no longer exists, skipping metadata update", + room_name, + ) + except RoomManagementException as e: logger.exception("Failed to update room's metadata: %s", e) logger.info( diff --git a/src/backend/core/services/livekit_events.py b/src/backend/core/services/livekit_events.py index 5852f6d4..8bdf1fd0 100644 --- a/src/backend/core/services/livekit_events.py +++ b/src/backend/core/services/livekit_events.py @@ -11,7 +11,7 @@ from django.conf import settings from livekit import api -from core import models, utils +from core import models from core.recording.services.metadata_collector import ( MetadataCollectorException, MetadataCollectorService, @@ -23,6 +23,11 @@ from core.recording.services.recording_events import ( ) from .lobby import LobbyService +from .room_management import ( + RoomManagement, + RoomManagementException, + RoomNotFoundException, +) from .telephony import TelephonyException, TelephonyService logger = getLogger(__name__) @@ -177,10 +182,15 @@ class LiveKitEventsService: try: room_name = str(recording.room.id) - utils.update_room_metadata( - room_name, {}, ["recording_mode", "recording_status"] + RoomManagement().update_metadata( + room_name, remove_keys=["recording_mode", "recording_status"] ) - except utils.MetadataUpdateException as e: + except RoomNotFoundException: + logger.info( + "LiveKit room %s no longer exists, skipping metadata update", + room_name, + ) + except RoomManagementException as e: logger.exception("Failed to update room's metadata: %s", e) if recording.options.get("metadata_collector_dispatch_id", None) is not None: diff --git a/src/backend/core/services/room_management.py b/src/backend/core/services/room_management.py index 5934f285..109d7f9d 100644 --- a/src/backend/core/services/room_management.py +++ b/src/backend/core/services/room_management.py @@ -8,6 +8,7 @@ from typing import Dict, Optional from asgiref.sync import async_to_sync from livekit.api import ( + ListRoomsRequest, TwirpError, UpdateRoomMetadataRequest, ) @@ -29,20 +30,45 @@ class RoomManagement: """Service for managing LiveKit rooms.""" @async_to_sync - async def update_metadata(self, room_name: str, metadata: Optional[Dict] = None): - """Update a LiveKit room's metadata. + async def update_metadata( + self, + room_name: str, + metadata: Optional[Dict] = None, + remove_keys: Optional[list[str]] = None, + ): + """Merge values into a LiveKit room's metadata. The `room_name` corresponds to the LiveKit room identifier (i.e. the Room model's UUID as a string). + + Raises: + RoomNotFoundException: the room does not exist in LiveKit. + RoomManagementException: the metadata update otherwise fails. """ lkapi = utils.create_livekit_client() try: + response = await lkapi.room.list_rooms(ListRoomsRequest(names=[room_name])) + + if not response.rooms: + logger.warning( + "Room %s not found in LiveKit, skipping metadata update", + room_name, + ) + raise RoomNotFoundException("Room does not exist") + + existing_metadata = json.loads(response.rooms[0].metadata or "{}") + + for key in remove_keys or []: + existing_metadata.pop(key, None) + + updated_metadata = {**existing_metadata, **(metadata or {})} + await lkapi.room.update_room_metadata( UpdateRoomMetadataRequest( room=room_name, - metadata=json.dumps(metadata) if metadata is not None else "", + metadata=json.dumps(updated_metadata), ) ) diff --git a/src/backend/core/tests/recording/worker/test_mediator.py b/src/backend/core/tests/recording/worker/test_mediator.py index 0b3fd4c3..d4c0fa1e 100644 --- a/src/backend/core/tests/recording/worker/test_mediator.py +++ b/src/backend/core/tests/recording/worker/test_mediator.py @@ -34,10 +34,8 @@ def mediator(mock_worker_service): return WorkerServiceMediator(mock_worker_service) -@mock.patch("core.utils.update_room_metadata") -def test_start_recording_success( - mock_update_room_metadata, mediator, mock_worker_service -): +@mock.patch("core.services.room_management.RoomManagement.update_metadata") +def test_start_recording_success(mock_update_metadata, mediator, mock_worker_service): """Test successful recording start""" # Setup worker_id = "test-worker-123" @@ -60,7 +58,7 @@ def test_start_recording_success( assert mock_recording.worker_id == worker_id assert mock_recording.status == RecordingStatusChoices.ACTIVE - mock_update_room_metadata.assert_called_once_with( + mock_update_metadata.assert_called_once_with( str(mock_recording.room.id), {"recording_mode": mock_recording.mode, "recording_status": "starting"}, ) @@ -69,9 +67,9 @@ def test_start_recording_success( @pytest.mark.parametrize( "error_class", [WorkerRequestError, WorkerConnectionError, WorkerResponseError] ) -@mock.patch("core.utils.update_room_metadata") +@mock.patch("core.services.room_management.RoomManagement.update_metadata") def test_mediator_start_recording_worker_errors( - mock_update_room_metadata, mediator, mock_worker_service, error_class + mock_update_metadata, mediator, mock_worker_service, error_class ): """Test handling of various worker errors during start""" # Setup @@ -89,7 +87,7 @@ def test_mediator_start_recording_worker_errors( assert mock_recording.status == RecordingStatusChoices.FAILED_TO_START assert mock_recording.worker_id is None - mock_update_room_metadata.assert_not_called() + mock_update_metadata.assert_not_called() @pytest.mark.parametrize( @@ -103,9 +101,9 @@ def test_mediator_start_recording_worker_errors( RecordingStatusChoices.ABORTED, ], ) -@mock.patch("core.utils.update_room_metadata") +@mock.patch("core.services.room_management.RoomManagement.update_metadata") def test_mediator_start_recording_from_forbidden_status( - mock_update_room_metadata, mediator, mock_worker_service, status + mock_update_metadata, mediator, mock_worker_service, status ): """Test handling of various worker errors during start""" # Setup @@ -119,7 +117,7 @@ def test_mediator_start_recording_from_forbidden_status( mock_recording.refresh_from_db() assert mock_recording.status == status - mock_update_room_metadata.assert_not_called() + mock_update_metadata.assert_not_called() def test_mediator_stop_recording_success(mediator, mock_worker_service): diff --git a/src/backend/core/tests/services/test_livekit_events.py b/src/backend/core/tests/services/test_livekit_events.py index 56d0e0e5..16adb661 100644 --- a/src/backend/core/tests/services/test_livekit_events.py +++ b/src/backend/core/tests/services/test_livekit_events.py @@ -20,8 +20,9 @@ from core.services.livekit_events import ( api, ) from core.services.lobby import LobbyService +from core.services.room_management import RoomManagementException from core.services.telephony import TelephonyException, TelephonyService -from core.utils import MetadataUpdateException, NotificationError +from core.utils import NotificationError pytestmark = pytest.mark.django_db @@ -70,9 +71,9 @@ def test_initialization( ), ) @mock.patch("core.utils.notify_participants") -@mock.patch("core.utils.update_room_metadata") +@mock.patch("core.services.room_management.RoomManagement.update_metadata") def test_handle_egress_ended_success( - mock_update_room_metadata, mock_notify, mode, notification_type, service + mock_update_metadata, mock_notify, mode, notification_type, service ): """Should successfully stop recording and notifies all participant.""" @@ -86,8 +87,8 @@ def test_handle_egress_ended_success( mock_notify.assert_called_once_with( room_name=str(recording.room.id), notification_data={"type": notification_type} ) - mock_update_room_metadata.assert_called_once_with( - str(recording.room.id), {}, ["recording_mode", "recording_status"] + mock_update_metadata.assert_called_once_with( + str(recording.room.id), remove_keys=["recording_mode", "recording_status"] ) recording.refresh_from_db() @@ -104,9 +105,9 @@ def test_handle_egress_ended_success( (EgressStatus.EGRESS_ABORTED, "aborted"), ), ) -@mock.patch("core.utils.update_room_metadata") +@mock.patch("core.services.room_management.RoomManagement.update_metadata") def test_handle_egress_updated_success( - mock_update_room_metadata, egress_status, status, service + mock_update_metadata, egress_status, status, service ): """Should successfully update room's metadata.""" @@ -117,7 +118,7 @@ def test_handle_egress_updated_success( service._handle_egress_updated(mock_data) - mock_update_room_metadata.assert_called_once_with( + mock_update_metadata.assert_called_once_with( str(recording.room.id), {"recording_status": status} ) @@ -129,9 +130,9 @@ def test_handle_egress_updated_success( EgressStatus.EGRESS_LIMIT_REACHED, ), ) -@mock.patch("core.utils.update_room_metadata") +@mock.patch("core.services.room_management.RoomManagement.update_metadata") def test_handle_egress_updated_non_handled( - mock_update_room_metadata, egress_status, service + mock_update_metadata, egress_status, service ): """Should ignore certain egress status and don't trigger metadata updates.""" @@ -142,7 +143,7 @@ def test_handle_egress_updated_non_handled( service._handle_egress_updated(mock_data) - mock_update_room_metadata.assert_not_called() + mock_update_metadata.assert_not_called() @pytest.mark.parametrize( @@ -153,9 +154,9 @@ def test_handle_egress_updated_non_handled( ), ) @mock.patch("core.utils.notify_participants") -@mock.patch("core.utils.update_room_metadata") +@mock.patch("core.services.room_management.RoomManagement.update_metadata") def test_handle_egress_ended_metadata_update_fails( - mock_update_room_metadata, mock_notify, mode, notification_type, service + mock_update_metadata, mock_notify, mode, notification_type, service ): """Should successfully stop and save recording when metadata's update fails.""" @@ -164,7 +165,7 @@ def test_handle_egress_ended_metadata_update_fails( mock_data.egress_info.egress_id = recording.worker_id mock_data.egress_info.status = EgressStatus.EGRESS_LIMIT_REACHED - mock_update_room_metadata.side_effect = MetadataUpdateException("Error notifying") + mock_update_metadata.side_effect = RoomManagementException("Error notifying") service._handle_egress_ended(mock_data) @@ -178,9 +179,9 @@ def test_handle_egress_ended_metadata_update_fails( @mock.patch("core.utils.notify_participants") -@mock.patch("core.utils.update_room_metadata") +@mock.patch("core.services.room_management.RoomManagement.update_metadata") def test_handle_egress_ended_notification_fails( - mock_update_room_metadata, mock_notify, service + mock_update_metadata, mock_notify, service ): """Should raise ActionFailedError when notification fails but still stop recording.""" @@ -200,15 +201,15 @@ def test_handle_egress_ended_notification_fails( recording.refresh_from_db() assert recording.status == "stopped" - mock_update_room_metadata.assert_called_once_with( - str(recording.room.id), {}, ["recording_mode", "recording_status"] + mock_update_metadata.assert_called_once_with( + str(recording.room.id), remove_keys=["recording_mode", "recording_status"] ) @mock.patch("core.utils.notify_participants") -@mock.patch("core.utils.update_room_metadata") +@mock.patch("core.services.room_management.RoomManagement.update_metadata") def test_handle_egress_ended_recording_not_found( - mock_update_room_metadata, mock_notify, service + mock_update_metadata, mock_notify, service ): """Should raise ActionFailedError when recording doesn't exist.""" @@ -223,16 +224,16 @@ def test_handle_egress_ended_recording_not_found( service._handle_egress_ended(mock_data) mock_notify.assert_not_called() - mock_update_room_metadata.assert_not_called() + mock_update_metadata.assert_not_called() recording.refresh_from_db() assert recording.status == "active" @mock.patch("core.utils.notify_participants") -@mock.patch("core.utils.update_room_metadata") +@mock.patch("core.services.room_management.RoomManagement.update_metadata") def test_handle_egress_ended_recording_not_active( - mock_update_room_metadata, mock_notify, service + mock_update_metadata, mock_notify, service ): """Should ignore non-active recordings.""" @@ -244,8 +245,8 @@ def test_handle_egress_ended_recording_not_active( service._handle_egress_ended(mock_data) mock_notify.assert_not_called() - mock_update_room_metadata.assert_called_once_with( - str(recording.room.id), {}, ["recording_mode", "recording_status"] + mock_update_metadata.assert_called_once_with( + str(recording.room.id), remove_keys=["recording_mode", "recording_status"] ) recording.refresh_from_db() @@ -253,9 +254,9 @@ def test_handle_egress_ended_recording_not_active( @mock.patch("core.utils.notify_participants") -@mock.patch("core.utils.update_room_metadata") +@mock.patch("core.services.room_management.RoomManagement.update_metadata") def test_handle_egress_ended_recording_not_limit_reached( - mock_update_room_metadata, mock_notify, service + mock_update_metadata, mock_notify, service ): """Should ignore egress non-limit-reached statuses.""" @@ -267,16 +268,16 @@ def test_handle_egress_ended_recording_not_limit_reached( service._handle_egress_ended(mock_data) mock_notify.assert_not_called() - mock_update_room_metadata.assert_called_once_with( - str(recording.room.id), {}, ["recording_mode", "recording_status"] + mock_update_metadata.assert_called_once_with( + str(recording.room.id), remove_keys=["recording_mode", "recording_status"] ) assert recording.status == "stopped" @mock.patch("core.services.livekit_events.MetadataCollectorService") -@mock.patch("core.utils.update_room_metadata") +@mock.patch("core.services.room_management.RoomManagement.update_metadata") def test_handle_egress_ended_calls_metadata_collector_stop_when_conditions_are_met( - mock_update_room_metadata, mock_collector_class, service, settings + mock_update_metadata, mock_collector_class, service, settings ): """Should call MetadataCollectorService.stop when it exists.""" settings.METADATA_COLLECTOR_ENABLED = True @@ -306,7 +307,7 @@ def test_handle_egress_ended_calls_metadata_collector_stop_when_conditions_are_m ], ) @mock.patch("core.services.livekit_events.MetadataCollectorService") -@mock.patch("core.utils.update_room_metadata") +@mock.patch("core.services.room_management.RoomManagement.update_metadata") def test_handle_egress_ended_does_not_call_metadata_collector_stop_when_conditions_not_met( _, mock_collector_class, metadata_enabled, options, service, settings ): # pylint: disable=too-many-arguments,too-many-positional-arguments @@ -335,7 +336,7 @@ def test_handle_egress_ended_does_not_call_metadata_collector_stop_when_conditio "notify_external_services" ) @mock.patch("core.utils.notify_participants") -@mock.patch("core.utils.update_room_metadata") +@mock.patch("core.services.room_management.RoomManagement.update_metadata") @pytest.mark.parametrize( "egress_status", [EgressStatus.EGRESS_COMPLETE, EgressStatus.EGRESS_LIMIT_REACHED], @@ -345,7 +346,7 @@ def test_handle_egress_ended_does_not_call_metadata_collector_stop_when_conditio [(True, "notification_succeeded"), (False, "saved")], ) def test_handle_egress_ended_finalizes_recording( # noqa: PLR0913 - mock_update_room_metadata, + mock_update_metadata, mock_notify, mock_notify_external_services, notify_return_value, @@ -378,7 +379,7 @@ def test_handle_egress_ended_finalizes_recording( # noqa: PLR0913 "notify_external_services" ) @mock.patch("core.utils.notify_participants") -@mock.patch("core.utils.update_room_metadata") +@mock.patch("core.services.room_management.RoomManagement.update_metadata") @pytest.mark.parametrize( "egress_status, expected_status", [ @@ -387,7 +388,7 @@ def test_handle_egress_ended_finalizes_recording( # noqa: PLR0913 ], ) def test_handle_egress_ended_does_not_finalize_when_webhooks_enabled( # noqa: PLR0913 - mock_update_room_metadata, + mock_update_metadata, mock_notify, mock_notify_external_services, egress_status, @@ -424,9 +425,9 @@ def test_handle_egress_ended_does_not_finalize_when_webhooks_enabled( # noqa: P EgressStatus.EGRESS_ABORTED, ], ) -@mock.patch("core.utils.update_room_metadata") +@mock.patch("core.services.room_management.RoomManagement.update_metadata") def test_handle_egress_ended_does_not_save_on_wrong_status( - mock_update_room_metadata, egress_status, service, settings + mock_update_metadata, egress_status, service, settings ): """Shouldn't save on invalid status.""" settings.RECORDING_STORAGE_EVENT_ENABLE = False @@ -445,9 +446,9 @@ def test_handle_egress_ended_does_not_save_on_wrong_status( @pytest.mark.parametrize( "status", ["failed_to_start", "aborted", "failed_to_stop", "saved", "initiated"] ) -@mock.patch("core.utils.update_room_metadata") +@mock.patch("core.services.room_management.RoomManagement.update_metadata") def test_handle_egress_ended_ignores_non_savable_recording( - mock_update_room_metadata, status, service, settings + mock_update_metadata, status, service, settings ): """Should handle non-savable recordings idempotently without raising. diff --git a/src/backend/core/utils.py b/src/backend/core/utils.py index 6d843e17..a9a7525c 100644 --- a/src/backend/core/utils.py +++ b/src/backend/core/utils.py @@ -31,7 +31,6 @@ from livekit.api import ( # pylint: disable=E0611 LiveKitAPI, SendDataRequest, TwirpError, - UpdateRoomMetadataRequest, VideoGrants, ) @@ -258,57 +257,6 @@ async def notify_participants(room_name: str, notification_data: dict): await lkapi.aclose() -class MetadataUpdateException(Exception): - """Room's metadata update fails.""" - - -@async_to_sync -async def update_room_metadata( - room_name: str, metadata: dict, remove_keys: Optional[list[str]] = None -): - """Update LiveKit room metadata by merging new values with existing metadata. - - Args: - room_name: Name of the room to update - metadata: Dictionary of metadata key-values to add/update - remove_keys: Optional list of keys to remove from existing metadata. - """ - - lkapi = create_livekit_client() - - try: - response = await lkapi.room.list_rooms( - ListRoomsRequest( - names=[room_name], - ) - ) - - if not response.rooms: - return - - room = response.rooms[0] - - existing_metadata = json.loads(room.metadata) if room.metadata else {} - - if remove_keys: - for key in remove_keys: - existing_metadata.pop(key, None) - - updated_metadata = {**existing_metadata, **metadata} - - await lkapi.room.update_room_metadata( - UpdateRoomMetadataRequest( - room=room_name, metadata=json.dumps(updated_metadata).encode("utf-8") - ) - ) - except TwirpError as e: - raise MetadataUpdateException( - f"Failed to update metadata for room {room_name}: {e}" - ) from e - finally: - await lkapi.aclose() - - ALPHANUMERIC_CHARSET = string.ascii_letters + string.digits diff --git a/src/frontend/src/features/rooms/livekit/hooks/useSyncLiveKitMetadata.ts b/src/frontend/src/features/rooms/livekit/hooks/useSyncLiveKitMetadata.ts index 5e92acb2..b75afb2f 100644 --- a/src/frontend/src/features/rooms/livekit/hooks/useSyncLiveKitMetadata.ts +++ b/src/frontend/src/features/rooms/livekit/hooks/useSyncLiveKitMetadata.ts @@ -13,8 +13,7 @@ import { useRoomContext } from '@livekit/components-react' import { useRoomData } from './useRoomData' /** - * Shape of the LiveKit room metadata blob pushed by the backend. - * Matches RoomManagement.update_metadata → {"configuration": room.configuration} + * The subset of LiveKit's room metadata this hook actually uses. */ type RoomLiveKitMetadata = { configuration?: RoomConfiguration