fix: clean up legacy source ID media

Author: Suncuss

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## [Unreleased]
 
+- Fixed detection deletion responses reporting the generated filename instead of the legacy media file actually removed
+- Fixed automatic storage cleanup skipping audio and spectrogram files created during the source-ID filename transition, allowing upgraded stations to reclaim those legacy recordings
 - Improved native updates on Raspberry Pis by stabilizing Docker base layers between releases and removing the backend compiler toolchain from the runtime image, reducing both routine downloads and slow SD-card extraction
 - Fixed failed update checks exposing the exact installed commit to anonymous visitors through detailed GitHub error URLs; diagnostics remain available to signed-in owners and in logs
 - Fixed authentication and settings temporary files being readable by other local users while sensitive content was still being written; private files now start owner-only

backend/core/db.py

@@ -1355,8 +1355,8 @@ def get_cleanup_scan_batch(self, after_timestamp=None, after_id=None,
 
         Returns:
             List of dicts with id, common_name, confidence, timestamp,
-            extra (raw JSON string); fewer than ``limit`` rows signals
-            the end of the table.
+            extra (raw JSON string), and audio_source; fewer than ``limit``
+            rows signals the end of the table.
         """
         where = _NO_FILTERS
         params = []
@@ -1365,7 +1365,7 @@ def get_cleanup_scan_batch(self, after_timestamp=None, after_id=None,
             params = [after_timestamp, after_timestamp, after_id]
 
         query = f"""
-        SELECT id, common_name, confidence, timestamp, extra
+        SELECT id, common_name, confidence, timestamp, extra, audio_source
         FROM detections
         WHERE {where}
         ORDER BY timestamp ASC, id ASC

backend/core/routes/detections.py

@@ -331,12 +331,9 @@ def delete_detection(detection_id):
     # Clean up associated files using shared utility
     delete_result = delete_detection_files(detection)
 
-    # Build files_deleted list for response
-    files_deleted = []
-    if delete_result['deleted_audio']:
-        files_deleted.append(detection['audio_filename'])
-    if delete_result['deleted_spectrogram']:
-        files_deleted.append(detection['spectrogram_filename'])
+    # Report the resolved names that were actually removed. These can differ
+    # from the canonical DB filenames for source-ID and colon-pattern files.
+    files_deleted = delete_result['deleted_filenames']
 
     logger.info("Detection deleted with files", extra={
         'detection_id': detection_id,

backend/core/storage_manager.py

@@ -98,8 +98,15 @@ def _resolve_path_with_legacy_fallback(filename, directory):
     return path  # Return original path even if it doesn't exist
 
 
-def _detection_filenames(detection):
-    """Build the dash-pattern filenames for a detection record."""
+def _detection_filename_candidates(detection):
+    """Build ordered dash-pattern filename candidates for a detection.
+
+    Source labels are frozen into ``extra`` for current files. During the
+    brief transition to multi-source recording, files instead used the raw
+    ``audio_source`` id (for example, ``source_0``) and those rows have no
+    saved label. Some imported legacy rows can still be unsuffixed despite
+    carrying an audio source, so keep that as the final fallback.
+    """
     extra = detection.get('extra', {})
     if isinstance(extra, str):
         # Only the source_label matters here, and most rows don't have one —
@@ -112,14 +119,36 @@ def _detection_filenames(detection):
         else:
             extra = {}
     source_label = extra.get('source_label')
-    return build_detection_filenames(
-        detection['common_name'],
-        detection['confidence'],
-        detection['timestamp'],
-        audio_source=source_label or None
+    if source_label:
+        source_suffixes = (source_label,)
+    elif detection.get('audio_source'):
+        source_suffixes = (detection['audio_source'], None)
+    else:
+        source_suffixes = (None,)
+
+    return tuple(
+        build_detection_filenames(
+            detection['common_name'],
+            detection['confidence'],
+            detection['timestamp'],
+            audio_source=source_suffix,
+        )
+        for source_suffix in source_suffixes
     )
 
 
+def _resolve_detection_path(filename_candidates, key, directory):
+    """Return the first existing path for one detection media type."""
+    first_path = None
+    for filenames in filename_candidates:
+        path = _resolve_path_with_legacy_fallback(filenames[key], directory)
+        if first_path is None:
+            first_path = path
+        if os.path.exists(path):
+            return path
+    return first_path
+
+
 def get_detection_files(detection):
     """Get full file paths for a detection record.
 
@@ -132,11 +161,13 @@ def get_detection_files(detection):
     Returns:
         dict with audio_path and spectrogram_path
     """
-    filenames = _detection_filenames(detection)
+    filename_candidates = _detection_filename_candidates(detection)
 
     return {
-        'audio_path': _resolve_path_with_legacy_fallback(filenames['audio_filename'], EXTRACTED_AUDIO_DIR),
-        'spectrogram_path': _resolve_path_with_legacy_fallback(filenames['spectrogram_filename'], SPECTROGRAM_DIR)
+        'audio_path': _resolve_detection_path(
+            filename_candidates, 'audio_filename', EXTRACTED_AUDIO_DIR),
+        'spectrogram_path': _resolve_detection_path(
+            filename_candidates, 'spectrogram_filename', SPECTROGRAM_DIR),
     }
 
 
@@ -166,9 +197,11 @@ def scan(directory):
 def _has_files_on_disk(detection, audio_names, spectrogram_names):
     """Whether any of the detection's files (dash or legacy pattern) exist
     in the normalized directory snapshots from _disk_filename_sets()."""
-    filenames = _detection_filenames(detection)
-    return (filenames['audio_filename'] in audio_names
-            or filenames['spectrogram_filename'] in spectrogram_names)
+    return any(
+        filenames['audio_filename'] in audio_names
+        or filenames['spectrogram_filename'] in spectrogram_names
+        for filenames in _detection_filename_candidates(detection)
+    )
 
 
 def delete_detection_files(detection):
@@ -178,12 +211,14 @@ def delete_detection_files(detection):
         detection: dict with common_name, confidence, timestamp
 
     Returns:
-        dict with deleted_audio, deleted_spectrogram, bytes_freed
+        dict with deleted_audio, deleted_spectrogram, deleted_filenames,
+        and bytes_freed
     """
     paths = get_detection_files(detection)
     result = {
         'deleted_audio': False,
         'deleted_spectrogram': False,
+        'deleted_filenames': [],
         'bytes_freed': 0
     }
 
@@ -194,6 +229,7 @@ def delete_detection_files(detection):
             size = os.path.getsize(audio_path)
             os.remove(audio_path)
             result['deleted_audio'] = True
+            result['deleted_filenames'].append(os.path.basename(audio_path))
             result['bytes_freed'] += size
         except OSError as e:
             logger.warning("Failed to delete audio file", extra={
@@ -208,6 +244,8 @@ def delete_detection_files(detection):
             size = os.path.getsize(spectrogram_path)
             os.remove(spectrogram_path)
             result['deleted_spectrogram'] = True
+            result['deleted_filenames'].append(
+                os.path.basename(spectrogram_path))
             result['bytes_freed'] += size
         except OSError as e:
             logger.warning("Failed to delete spectrogram file", extra={

backend/tests/api/test_detections_api.py

@@ -611,6 +611,58 @@ def test_delete_detection_removes_files(self, api_client, real_db_manager):
             assert not os.path.exists(audio_file)
             assert not os.path.exists(spectrogram_file)
 
+    def test_delete_reports_resolved_legacy_source_filenames(
+            self, api_client, real_db_manager):
+        """The response names the source-ID files it actually removes."""
+        with tempfile.TemporaryDirectory() as tmpdir:
+            audio_dir = os.path.join(tmpdir, 'audio')
+            spectrogram_dir = os.path.join(tmpdir, 'spectrograms')
+            os.makedirs(audio_dir)
+            os.makedirs(spectrogram_dir)
+
+            timestamp = '2024-01-15T10:30:00'
+            detection_id = real_db_manager.insert_detection({
+                'timestamp': timestamp,
+                'group_timestamp': timestamp,
+                'common_name': 'American Robin',
+                'scientific_name': 'Turdus migratorius',
+                'confidence': 0.8500,
+                'latitude': 40.7128,
+                'longitude': -74.0060,
+                'cutoff': 0.5,
+                'sensitivity': 0.75,
+                'overlap': 0.25,
+                'extra': {},
+                'audio_source': 'source_0',
+            })
+
+            from core.utils import build_detection_filenames
+
+            filenames = build_detection_filenames(
+                'American Robin', 0.8500, timestamp,
+                audio_source='source_0')
+            audio_file = os.path.join(
+                audio_dir, filenames['audio_filename'])
+            spectrogram_file = os.path.join(
+                spectrogram_dir, filenames['spectrogram_filename'])
+            for path in (audio_file, spectrogram_file):
+                with open(path, 'w') as f:
+                    f.write('media data')
+
+            with patch('core.auth.is_authenticated', return_value=True), \
+                 patch('core.storage_manager.EXTRACTED_AUDIO_DIR', audio_dir), \
+                 patch('core.storage_manager.SPECTROGRAM_DIR', spectrogram_dir):
+                response = api_client.delete(
+                    f'/api/detections/{detection_id}')
+
+            assert response.status_code == 200
+            assert response.get_json()['files_deleted'] == [
+                filenames['audio_filename'],
+                filenames['spectrogram_filename'],
+            ]
+            assert not os.path.exists(audio_file)
+            assert not os.path.exists(spectrogram_file)
+
 
 class TestDetectionsDatabaseMethods:
     """Tests for the underlying database methods."""

backend/tests/test_storage_manager.py

@@ -248,8 +248,8 @@ def test_keyset_walk_covers_every_row_once(self, populated_db_for_cleanup):
         keys = [(d['timestamp'], d['id']) for d in seen]
         assert keys == sorted(keys)
 
-    def test_returns_extra_column(self, test_db_manager):
-        """Scan rows include extra for filename reconstruction."""
+    def test_returns_filename_metadata(self, test_db_manager):
+        """Scan rows include source metadata for filename reconstruction."""
         import json
 
         base_time = datetime(2024, 1, 15, 10, 0, 0)
@@ -269,6 +269,7 @@ def test_returns_extra_column(self, test_db_manager):
             if isinstance(extra, str):
                 extra = json.loads(extra)
             assert extra.get('source_label') == 'Backyard_Mic'
+            assert detection['audio_source'] == 'alsa_input.usb-test'
 
 
 class TestGetDiskUsage:
@@ -328,13 +329,64 @@ def test_constructs_paths_with_source_label(self):
                     'confidence': 0.85,
                     'timestamp': '2024-01-15T10:30:00',
                     'extra': '{"source_label": "Backyard_Mic"}',
+                    'audio_source': 'source_0',
                 }
 
                 paths = get_detection_files(detection)
 
                 assert paths['audio_path'].endswith('_Backyard_Mic.mp3')
                 assert paths['spectrogram_path'].endswith('_Backyard_Mic.webp')
 
+    def test_falls_back_to_legacy_source_id(self, storage_dirs):
+        """Rows from the source-ID transition resolve their suffixed files."""
+        audio_dir, spectrogram_dir = storage_dirs
+
+        from core.utils import build_detection_filenames
+
+        names = build_detection_filenames(
+            'Test Bird', 0.85, '2024-01-15T10:30:00',
+            audio_source='source_0')
+        legacy_audio = os.path.join(audio_dir, names['audio_filename'])
+        legacy_spectrogram = os.path.join(
+            spectrogram_dir, names['spectrogram_filename'])
+        with open(legacy_audio, 'w') as f:
+            f.write('audio')
+        with open(legacy_spectrogram, 'w') as f:
+            f.write('spectrogram')
+
+        from core.storage_manager import get_detection_files
+
+        paths = get_detection_files({
+            'common_name': 'Test Bird',
+            'confidence': 0.85,
+            'timestamp': '2024-01-15T10:30:00',
+            'extra': '{}',
+            'audio_source': 'source_0',
+        })
+
+        assert paths['audio_path'] == legacy_audio
+        assert paths['spectrogram_path'] == legacy_spectrogram
+
+    def test_source_id_row_can_fall_back_to_unsuffixed_file(self, storage_dirs):
+        """Imported rows with a source id can still own unsuffixed files."""
+        audio_dir, _ = storage_dirs
+        unsuffixed_audio = os.path.join(
+            audio_dir, 'Test_Bird_85_2024-01-15-birdnet-10-30-00.mp3')
+        with open(unsuffixed_audio, 'w') as f:
+            f.write('audio')
+
+        from core.storage_manager import get_detection_files
+
+        paths = get_detection_files({
+            'common_name': 'Test Bird',
+            'confidence': 0.85,
+            'timestamp': '2024-01-15T10:30:00',
+            'extra': '{}',
+            'audio_source': 'source_0',
+        })
+
+        assert paths['audio_path'] == unsuffixed_audio
+
     def test_fallback_to_legacy_colon_pattern(self, storage_dirs):
         """Should fall back to legacy colon-pattern files if dash-pattern not found."""
         audio_dir, spectrogram_dir = storage_dirs
@@ -453,6 +505,7 @@ def test_deletes_existing_files(self, storage_dirs):
 
             assert result['deleted_audio']
             assert result['deleted_spectrogram']
+            assert result['deleted_filenames'] == ['test.mp3', 'test.webp']
             assert result['bytes_freed'] == 1500
             assert not os.path.exists(audio_file)
             assert not os.path.exists(spectrogram_file)
@@ -475,6 +528,7 @@ def test_handles_missing_files_gracefully(self):
 
                         assert not result['deleted_audio']
                         assert not result['deleted_spectrogram']
+                        assert result['deleted_filenames'] == []
                         assert result['bytes_freed'] == 0
 
 
@@ -574,6 +628,39 @@ def test_cleanup_resolves_multi_source_filenames(self, test_db_manager, storage_
         assert result['skipped_missing'] == 0
         assert result['files_deleted'] == 10  # 70 - 60 protected
 
+    def test_cleanup_deletes_legacy_source_id_files(self, test_db_manager,
+                                                    storage_dirs):
+        """Cleanup finds files created before source-label filenames."""
+        audio_dir, spectrogram_dir = storage_dirs
+
+        from core.utils import build_detection_filenames
+
+        for i in range(3):
+            timestamp = f'2024-01-15T10:0{i}:00'
+            detection = make_detection(
+                timestamp, extra={}, audio_source='source_0')
+            test_db_manager.insert_detection(detection)
+            names = build_detection_filenames(
+                'Test Bird', 0.85, timestamp, audio_source='source_0')
+            for name, directory in (
+                    (names['audio_filename'], audio_dir),
+                    (names['spectrogram_filename'], spectrogram_dir)):
+                with open(os.path.join(directory, name), 'wb') as f:
+                    f.write(b'x' * 100)
+
+        with patch('core.storage_manager.get_disk_usage',
+                   return_value=mock_disk_usage_needing(10 * 1024**3)):
+            from core.storage_manager import cleanup_storage
+
+            result = cleanup_storage(
+                test_db_manager, target_percent=80,
+                keep_per_species=0, keep_recent_per_species=0)
+
+        assert result['skipped_missing'] == 0
+        assert result['files_deleted'] == 3
+        assert len(os.listdir(audio_dir)) == 0
+        assert len(os.listdir(spectrogram_dir)) == 0
+
     def test_cleanup_deletes_legacy_colon_files(self, test_db_manager, storage_dirs):
         """Rows whose files still use the legacy colon pattern are found via
         the normalized directory snapshot and deleted through the legacy