Video edits keep the original: custom_title/custom_description in the info json's wrolpi section

Editing a title or description no longer overwrites yt-dlp's keys.  The
values go into the info json's `wrolpi` section (custom_title,
custom_description); the modeler prefers them when deriving the Video, so
a refresh keeps the edit while the original title and description stay in
the file.  An empty custom description means none.

Both re-download paths carry the wrolpi section over: the metadata-only
refresh merges it into the fresh info json, and a full re-download
snapshots it before yt-dlp overwrites the file and restores it (then
re-validates) in finalize_download.  FileGroup.get_wrolpi_json reads it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

modules/videos/downloader.py

@@ -887,6 +887,7 @@ class PreparedVideo:
     destination: Optional[pathlib.Path] = None
     info_json: Optional[dict] = None
     location: Optional[str] = None             # existing video, for error reports
+    wrolpi: dict = field(default_factory=dict)  # existing video's wrolpi section, to carry over
 
 
 @dataclass
@@ -907,6 +908,7 @@ class ExecutedVideo:
     tag_names: List[str]
     settings: dict
     location: Optional[str]                    # for error reporting
+    wrolpi: dict = field(default_factory=dict)  # existing video's wrolpi section, to carry over
 
 
 class VideoDownloader(Downloader, ABC):
@@ -954,6 +956,8 @@ class VideoDownloader(Downloader, ABC):
         # can still tell the user where the previous attempt landed.
         existing = Video.get_by_url(session, url)
         location = existing.location if existing else None
+        # yt-dlp rewrites the whole info json; the user's edits (its wrolpi section) must survive.
+        wrolpi = existing.file_group.get_wrolpi_json() if existing else dict()
 
         return PreparedVideo(
             url=url,
@@ -962,6 +966,7 @@ class VideoDownloader(Downloader, ABC):
             destination=download.destination,
             info_json=download.info_json,
             location=location,
+            wrolpi=wrolpi,
         )
 
     async def execute_download(self, prepared: PreparedVideo, ctx: DownloadContext,
@@ -1201,6 +1206,7 @@ class VideoDownloader(Downloader, ABC):
                 tag_names=prepared.tag_names,
                 settings=settings,
                 location=location,
+                wrolpi=prepared.wrolpi,
             )
 
         except BotBlockedDownloadError:
@@ -1238,6 +1244,11 @@ class VideoDownloader(Downloader, ABC):
         video.channel_id = executed.channel_id
         if video.info_json_path and (new_info_json := video.clean_info_json()):
             video.replace_info_json(new_info_json, clean=False)
+        if executed.wrolpi and video.info_json_path:
+            # The re-downloaded info json lost the user's edits; put them back, then derive the
+            # title/description again so the custom ones win (from_paths validated without them).
+            video.file_group.update_wrolpi_json(executed.wrolpi)
+            video.validate(session)
 
         if parent_download_url := executed.settings.get('parent_download_url'):
             try:
@@ -1368,9 +1379,12 @@ class VideoDownloader(Downloader, ABC):
         settings = download.settings or dict()
 
         with get_db_session(commit=True) as session:
-            # Find the existing video, replace its info json.
+            # Find the existing video, replace its info json; keep the user's edits (wrolpi section).
             video = Video.get_by_url(session, url)
+            wrolpi = video.file_group.get_wrolpi_json()
             video.replace_info_json(info_json)
+            if wrolpi:
+                video.file_group.update_wrolpi_json(wrolpi)
 
             if parent_download_url := settings.get('parent_download_url'):
                 try:

modules/videos/lib.py

@@ -58,7 +58,9 @@ def extract_video_info_json(video: Video) -> VideoInfoJSON:
     """
     video_info_json = VideoInfoJSON()
     if info_json := video.get_info_json():
-        title = info_json.get('fulltitle') or info_json.get('title')
+        # A title the user set (Edit > Edit) is kept apart from yt-dlp's keys, and wins.
+        wrolpi_section = info_json.get('wrolpi') or dict()
+        title = wrolpi_section.get('custom_title') or info_json.get('fulltitle') or info_json.get('title')
         video_info_json.title = html.unescape(title) if title else None
 
         upload_date = dates.strpdate(i) if (i := info_json.get('upload_date')) else None

modules/videos/models.py

@@ -141,10 +141,16 @@ class Video(ModelHelper, Base):
 
     def get_video_description(self) -> Optional[str]:
         """
-        Get the Video description from the file system.
+        Get the Video description from the file system.  A description the user set (Edit > Edit)
+        lives in the info json's `wrolpi` section and wins; an empty one means "none".
         """
-        if (info_json := self.get_info_json()) and (description := info_json.get('description')):
-            return description
+        info_json = self.get_info_json()
+        if not info_json:
+            return None
+        wrolpi_section = info_json.get('wrolpi') or dict()
+        if 'custom_description' in wrolpi_section:
+            return wrolpi_section['custom_description'] or None
+        return info_json.get('description') or None
 
     def get_description(self) -> Optional[str]:
         """The indexed description (`FileGroup.c_text`), falling back to the info json on disk.

modules/videos/test/test_downloader.py

@@ -1789,3 +1789,76 @@ async def test_enforce_codecs():
         with pytest.raises(UnrecoverableDownloadError):
             await enforce_codecs(video_path, video_paths, effective)
         mock_transcode.assert_not_called()
+
+
+@pytest.mark.asyncio
+async def test_metadata_refresh_keeps_wrolpi_section(test_session, test_directory, video_factory):
+    """Refreshing a Video's metadata replaces yt-dlp's info json; the wrolpi section (the user's
+    custom title and description) is carried over and still wins."""
+    source_id = 'video-source-id'
+    url = 'https://example.com/the-video'
+    video = video_factory(with_info_json={'id': source_id, 'title': 'Orig',
+                                          'wrolpi': {'custom_title': 'Mine', 'custom_description': 'my words'}},
+                          source_id=source_id)
+    video.file_group.url = url
+    test_session.commit()
+    assert video.file_group.title == 'Mine'
+
+    download = Download(url=url, info_json={'id': source_id}, settings={})
+    with mock.patch('modules.videos.downloader.download_video_info_json') as mock_download:
+        mock_download.return_value = {'id': source_id, 'title': 'Orig v2', 'description': 'theirs', 'duration': 9}
+        result = await VideoDownloader.download_info_json(download, video.video_path)
+    assert result.success is True, result
+
+    test_session.expire_all()
+    video = Video.find_by_file_group_id(test_session, video.file_group_id)
+    info_json = video.get_info_json()
+    assert info_json['title'] == 'Orig v2', 'yt-dlp data was refreshed'
+    assert info_json['wrolpi'] == {'custom_title': 'Mine', 'custom_description': 'my words'}
+    assert video.file_group.title == 'Mine'
+    assert video.get_description() == 'my words'
+
+
+@pytest.mark.asyncio
+async def test_redownload_keeps_wrolpi_section(test_session, test_directory, mock_video_extract_info, simple_channel,
+                                               await_switches, video_download_manager, mock_video_process_runner,
+                                               image_file):
+    """A full re-download has yt-dlp overwrite the info json; the wrolpi section is carried over."""
+    simple_channel.source_id = example_video_json['channel_id']
+    simple_channel.directory = test_directory / 'videos/channel name'
+    simple_channel.directory.mkdir(parents=True)
+    url = 'https://www.youtube.com/watch?v=31jPEBiAC3c'
+
+    video_path = simple_channel.directory / 'a video.mp4'
+    shutil.copy(PROJECT_DIR / 'test/big_buck_bunny_720p_1mb.mp4', video_path)
+    info_json_path = video_path.with_suffix('.info.json')
+    info_json_path.write_text(json.dumps({'title': 'Orig', 'webpage_url': url, 'wrolpi': {'custom_title': 'Mine'}}))
+    from modules.videos.normalize_video_url import normalize_video_url
+    existing = Video.from_paths(test_session, video_path, info_json_path)
+    existing.validate(test_session)
+    test_session.commit()
+    assert existing.file_group.title == 'Mine'
+    assert Video.get_by_url(test_session, normalize_video_url(url)) is not None, 'lookup by normalized url'
+    assert existing.file_group.get_wrolpi_json() == {'custom_title': 'Mine'}
+
+    from wrolpi.cmd import CommandResult
+
+    async def yt_dlp_overwrites_info_json(*args, **kwargs):
+        # yt-dlp rewrites the whole info json; the wrolpi section is gone from disk.
+        info_json_path.write_text(json.dumps({'title': 'Orig v2', 'webpage_url': url, 'duration': 5}))
+        return CommandResult(return_code=0, cancelled=False, stdout=b'', stderr=b'', elapsed=0)
+
+    mock_video_process_runner.side_effect = yt_dlp_overwrites_info_json
+    with mock.patch('modules.videos.downloader.prepare_video_filename') as mock_prepare_filename:
+        mock_video_extract_info.return_value = example_video_json
+        mock_prepare_filename.return_value = (video_path, {'id': 'foo'})
+        video_download_manager.create_download(test_session, url, video_downloader.name)
+        await video_download_manager.wait_for_all_downloads()
+        mock_video_process_runner.assert_called_once()
+
+    test_session.expire_all()
+    video: Video = test_session.query(Video).one()
+    info_json = video.get_info_json()
+    assert info_json['title'] == 'Orig v2'
+    assert info_json['wrolpi'] == {'custom_title': 'Mine'}
+    assert video.file_group.title == 'Mine'

modules/videos/test/test_video_edit.py

@@ -1,16 +1,20 @@
-"""Editing a Video's details from the video page."""
+"""Editing a Video's details from the video page.
+
+Edits never touch yt-dlp's own keys: they are written to the `wrolpi` section of the info json
+(`custom_title`, `custom_description`), which the modeler prefers when it derives the Video."""
 import json
 
 import pytest
 
+from modules.videos.lib import extract_video_info_json
 from modules.videos.models import Video
 from modules.videos.video.lib import update_video
 from wrolpi.errors import ValidationError
 
 
 def test_update_video_title(test_session, test_directory, video_factory):
-    """The new title is written to the info json (title and fulltitle) and the Video re-validated,
-    so its title and search text change too.  Other info json keys are kept."""
+    """The new title lands in the wrolpi section; yt-dlp's title/fulltitle stay as they were.  The
+    Video is re-validated, so its title and search text change."""
     video = video_factory(with_info_json={'title': 'Old Title', 'fulltitle': 'Old Title', 'duration': 7,
                                           'formats': ['kept, not cleaned']})
     test_session.commit()
@@ -24,12 +28,13 @@ def test_update_video_title(test_session, test_directory, video_factory):
     assert video.file_group.a_text == 'New Title'
     assert video.file_group.length == 7, 'Other details from the info json are unchanged'
     info_json = json.loads(video.info_json_path.read_text())
-    assert info_json['title'] == 'New Title' and info_json['fulltitle'] == 'New Title'
+    assert info_json['title'] == 'Old Title' and info_json['fulltitle'] == 'Old Title', 'The original is kept'
+    assert info_json['wrolpi']['custom_title'] == 'New Title'
     assert info_json['formats'] == ['kept, not cleaned']
 
 
 def test_update_video_title_creates_info_json(test_session, test_directory, video_factory):
-    """A Video without an info json gets one holding the title."""
+    """A Video without an info json gets one holding only the wrolpi section."""
     video = video_factory()
     test_session.commit()
     assert video.info_json_path is None
@@ -40,13 +45,14 @@ def test_update_video_title_creates_info_json(test_session, test_directory, vide
     video = Video.find_by_file_group_id(test_session, video.file_group_id)
     assert video.file_group.title == 'Brand New'
     assert video.info_json_path and video.info_json_path.is_file()
-    assert json.loads(video.info_json_path.read_text())['title'] == 'Brand New'
+    info_json = json.loads(video.info_json_path.read_text())
+    assert info_json == {'wrolpi': {'custom_title': 'Brand New'}}
     assert video.info_json_path.name in [i['path'] for i in video.file_group.files], 'The new file is tracked'
 
 
 def test_update_video_description(test_session, test_directory, video_factory):
-    """The description is written to the info json and indexed; the title is untouched when not
-    given.  An empty description clears it."""
+    """The description lands in the wrolpi section and is indexed; yt-dlp's description stays.
+    An empty custom description clears the shown description."""
     video = video_factory(with_info_json={'title': 'Kept', 'description': 'old words'})
     test_session.commit()
     assert video.file_group.c_text == 'old words'
@@ -58,12 +64,15 @@ def test_update_video_description(test_session, test_directory, video_factory):
     assert video.file_group.title == 'Kept'
     assert video.file_group.c_text == 'new words\nsecond line'
     assert video.get_description() == 'new words\nsecond line'
-    assert json.loads(video.info_json_path.read_text())['description'] == 'new words\nsecond line'
+    info_json = json.loads(video.info_json_path.read_text())
+    assert info_json['description'] == 'old words', 'The original is kept'
+    assert info_json['wrolpi']['custom_description'] == 'new words\nsecond line'
 
     update_video(video.file_group_id, description='')
     test_session.expire_all()
     video = Video.find_by_file_group_id(test_session, video.file_group_id)
     assert video.get_description() is None
+    assert json.loads(video.info_json_path.read_text())['description'] == 'old words'
     assert video.file_group.title == 'Kept'
 
 
@@ -83,6 +92,44 @@ def test_update_video_title_empty(test_session, video_factory):
     assert Video.find_by_file_group_id(test_session, video.file_group_id).file_group.title == 'Old'
 
 
+def test_extract_video_info_json_prefers_custom_title(test_session, video_factory):
+    """The modeler's reader prefers the wrolpi custom title over fulltitle/title."""
+    video = video_factory(with_info_json={'title': 'Orig', 'fulltitle': 'Orig Full',
+                                          'wrolpi': {'custom_title': 'Mine & yours'}})
+    assert extract_video_info_json(video).title == 'Mine & yours', 'HTML entities are unescaped as for any title'
+    video2 = video_factory(with_info_json={'title': 'Orig', 'fulltitle': 'Orig Full', 'wrolpi': {}})
+    assert extract_video_info_json(video2).title == 'Orig Full'
+
+
+@pytest.mark.asyncio
+async def test_custom_title_survives_refresh(test_session, test_directory, async_client, video_factory):
+    """A file refresh re-models the Video from its files; the custom title must win again."""
+    from wrolpi.files.worker import file_worker
+
+    video = video_factory(with_info_json={'title': 'Original'})
+    test_session.commit()
+    update_video(video.file_group_id, title='Custom', description='custom words')
+    video_path = video.video_path
+    file_group_id = video.file_group_id
+
+    # Force a full re-model, as a refresh of a modified file does.
+    test_session.expire_all()
+    video = Video.find_by_file_group_id(test_session, file_group_id)
+    video.file_group.indexed = False
+    video.file_group.title = None
+    video.file_group.c_text = None
+    test_session.commit()
+
+    job_id = file_worker.queue_refresh([video_path], send_events=False)
+    await file_worker.wait_for_job(job_id)
+
+    test_session.expire_all()
+    video = Video.find_by_file_group_id(test_session, file_group_id)
+    assert video.file_group.title == 'Custom'
+    assert video.get_description() == 'custom words'
+    assert json.loads(video.info_json_path.read_text())['title'] == 'Original'
+
+
 @pytest.mark.asyncio
 async def test_video_update_api(async_client, test_session, video_factory, wrol_mode_fixture):
     video = video_factory(with_info_json={'title': 'Old'})
@@ -133,3 +180,33 @@ async def test_update_video_wrol_mode(test_session, video_factory, wrol_mode_fix
 
     test_session.expire_all()
     assert Video.find_by_file_group_id(test_session, video.file_group_id).file_group.title == 'Old'
+
+
+@pytest.mark.asyncio
+async def test_edit_creates_info_json_that_survives_refresh(test_session, test_directory, async_client,
+                                                            video_factory):
+    """A Video with no info json is edited (the edit creates one), then its file is refreshed: the
+    new sidecar must stay in the FileGroup and the custom title/description must still win."""
+    from wrolpi.files.worker import file_worker
+
+    video = video_factory()
+    test_session.commit()
+    video_path = video.video_path
+    file_group_id = video.file_group_id
+
+    update_video(file_group_id, title='Custom', description='my words')
+    test_session.expire_all()
+    video = Video.find_by_file_group_id(test_session, file_group_id)
+    assert video.info_json_path and video.info_json_path.is_file()
+    assert video.get_description() == 'my words'
+
+    job_id = file_worker.queue_refresh([video_path], send_events=False)
+    await file_worker.wait_for_job(job_id)
+
+    test_session.expire_all()
+    video = Video.find_by_file_group_id(test_session, file_group_id)
+    assert video.info_json_path is not None, 'The refresh forgot the info json'
+    names = {i['path'] for i in video.file_group.files}
+    assert video_path.name in names and video.info_json_path.name in names, names
+    assert video.file_group.title == 'Custom'
+    assert video.get_description() == 'my words'

modules/videos/video/lib.py

@@ -56,8 +56,9 @@ def update_video(file_group_id: int, title: str = None, description: str = None)
     Video's .info.json (created if missing) and the Video is validated again from it.  Only the
     fields given (not None) are changed; an empty description clears it.
 
-    A later re-download of the Video's metadata will overwrite the info json, and this edit with
-    it.
+    Edits go into the info json's `wrolpi` section (`custom_title`, `custom_description`), never
+    over yt-dlp's own keys: the original title and description stay in the file, and a metadata
+    re-download carries the section over.
 
     @raise UnknownVideo: if the Video can not be found
     @raise ValidationError: when nothing is given, or the title is empty
@@ -70,17 +71,19 @@ def update_video(file_group_id: int, title: str = None, description: str = None)
         if not title:
             raise ValidationError('Video title cannot be empty')
 
+    changes = dict()
+    if title is not None:
+        changes['custom_title'] = title
+    if description is not None:
+        changes['custom_description'] = description
+
     with get_db_session(commit=True) as session:
         video = Video.find_by_file_group_id(session, file_group_id)
-        info_json = video.get_info_json() or dict()
-        if title is not None:
-            # `extract_video_info_json` prefers fulltitle over title; keep the two in agreement.
-            info_json['title'] = title
-            info_json['fulltitle'] = title
-        if description is not None:
-            info_json['description'] = description
-        # Keep the file as it was, other than the edited fields.
-        video.replace_info_json(info_json, clean=False)
+        if video.info_json_path is None or not video.info_json_path.is_file():
+            # No info json yet: create one holding only the wrolpi section.
+            video.replace_info_json({'wrolpi': changes}, clean=False)
+        else:
+            video.file_group.update_wrolpi_json(changes)
         # Re-derive the title (and search text) from the file just written.
         video.validate(session)
         video.flush()

wrolpi/files/models.py

@@ -432,6 +432,19 @@ class FileGroup(ModelHelper, Base):
         if json_path.name not in tracked_paths:
             self.append_files(json_path)
 
+    def get_wrolpi_json(self) -> dict:
+        """The `wrolpi` section of this FileGroup's metadata JSON: WROLPi-owned keys (a user's
+        custom title, the parent download) that yt-dlp/readability never write.  Empty if none."""
+        json_path = self.metadata_json_path
+        if not json_path or not json_path.is_file():
+            return dict()
+        try:
+            contents = json.loads(json_path.read_text())
+        except (OSError, ValueError):
+            return dict()
+        section = contents.get('wrolpi') if isinstance(contents, dict) else None
+        return dict(section) if isinstance(section, dict) else dict()
+
     def update_wrolpi_json(self, data: dict):
         """Read this FileGroup's metadata JSON, merge `data` into its `wrolpi` section, and write back.