fix(tv_shows): correct PACK vs EPISODIC classification model

The Phase 4 walker + rescan logic classified seasons by parser output (does the filename carry Exx?), but PACK vs EPISODIC is a structural distinction: * PACK = season folder with N flat SxxEyy videos directly inside * EPISODIC = season folder with N subfolders, each holding one video Changes: * walker.py: descends two levels under show_root and classifies each season folder by FS structure. SeasonFolder now carries mode: ReleaseMode | None. Mixed layouts (flat + subfolders) and EPISODIC subfolders with >1 video log a warning and report mode=None. * rescan.py: trusts walker.mode; drops the bogus 'single un- numbered video → PACK with empty episodes' branch. A season with no parseable episodes is now skipped with a warning. * Tests rewritten against the real model: PACK with flat numbered files, EPISODIC with one-video-per-subfolder, malformed mixed layout skipped, single-un-numbered-file skipped. Suite: 1237 → 1245 passing.
2026-05-25 21:37:34 +02:00
parent fe9857aaed
commit 97dc799a26
4 changed files with 416 additions and 120 deletions
@@ -85,31 +85,41 @@ def _default_info() -> MediaInfo:
 def _make_foundation_library(
    root: Path,
    *,
-    episodic_episodes: tuple[int, ...] = (1, 2, 3),
-    include_pack_s2: bool = True,
+    pack_episodes: tuple[int, ...] = (1, 2, 3),
+    episodic_episodes: tuple[int, ...] = (1, 2),
 ) -> Path:
    """Build a fake Foundation show folder under ``root``.

    Layout::

        root/Foundation/
-            Foundation.S01.1080p.x265-ELiTE/        ← EPISODIC
-                Foundation.S01E01.1080p.x265-ELiTE.mkv
+            Foundation.S01.1080p.x265-ELiTE/                ← PACK
+                Foundation.S01E01.1080p.x265-ELiTE.mkv      (flat)
                Foundation.S01E02.1080p.x265-ELiTE.mkv
                Foundation.S01E03.1080p.x265-ELiTE.mkv
-            Foundation.S02.1080p.x265-ELiTE/        ← PACK
-                Foundation.S02.1080p.x265-ELiTE.mkv
+            Foundation.S02/                                  ← EPISODIC
+                Foundation.S02E01.1080p.x265-ELiTE/         (subfolder)
+                    Foundation.S02E01.1080p.x265-ELiTE.mkv
+                Foundation.S02E02.1080p.x265-ELiTE/
+                    Foundation.S02E02.1080p.x265-ELiTE.mkv
+
+    ``pack_episodes`` and ``episodic_episodes`` can each be set to
+    empty tuples to omit the corresponding season.
    """
    show_root = root / "Foundation"
    show_root.mkdir()
-    s1 = show_root / "Foundation.S01.1080p.x265-ELiTE"
-    s1.mkdir()
-    for n in episodic_episodes:
-        (s1 / f"Foundation.S01E{n:02d}.1080p.x265-ELiTE.mkv").write_bytes(b"")
-    if include_pack_s2:
-        s2 = show_root / "Foundation.S02.1080p.x265-ELiTE"
+    if pack_episodes:
+        s1 = show_root / "Foundation.S01.1080p.x265-ELiTE"
+        s1.mkdir()
+        for n in pack_episodes:
+            (s1 / f"Foundation.S01E{n:02d}.1080p.x265-ELiTE.mkv").write_bytes(b"")
+    if episodic_episodes:
+        s2 = show_root / "Foundation.S02"
        s2.mkdir()
-        (s2 / "Foundation.S02.1080p.x265-ELiTE.mkv").write_bytes(b"")
+        for n in episodic_episodes:
+            sub = s2 / f"Foundation.S02E{n:02d}.1080p.x265-ELiTE"
+            sub.mkdir()
+            (sub / f"Foundation.S02E{n:02d}.1080p.x265-ELiTE.mkv").write_bytes(b"")
    return show_root


@@ -127,7 +137,7 @@ def _library_with_show(tmp_path: Path) -> tuple[Path, Path]:


 class TestHappyPath:
-    def test_builds_release_with_episodic_and_pack_seasons(self, tmp_path):
+    def test_builds_release_with_pack_and_episodic_seasons(self, tmp_path):
        library, show_root = _library_with_show(tmp_path)
        repo = DotAlfredSeriesReleaseRepository(library)
        prober = _StubProber()
@@ -150,9 +160,9 @@ class TestHappyPath:
        s2 = release.get_season(SeasonNumber(2))
        assert s1 is not None and s2 is not None

-        # S01 EPISODIC: three single-episode releases, each carrying
-        # its own TrackProfile.
-        assert s1.mode is ReleaseMode.EPISODIC
+        # S01 PACK: three single-episode releases, each carrying its
+        # own TrackProfile.
+        assert s1.mode is ReleaseMode.PACK
        assert s1.episode_count() == 3
        for ep in s1.episodes:
            assert ep.episodes.is_single()
@@ -160,12 +170,12 @@ class TestHappyPath:
            assert ep.tracks.audio_tracks[0].language == "eng"
            assert len(ep.tracks.subtitle_tracks) == 1

-        # S02 PACK: empty episodes tuple. Phase 5 (TMDB sync) will
-        # populate the slot map once episode_count is known.
-        assert s2.mode is ReleaseMode.PACK
-        assert s2.episodes == ()
+        # S02 EPISODIC: two single-episode releases, each in its own
+        # subfolder.
+        assert s2.mode is ReleaseMode.EPISODIC
+        assert s2.episode_count() == 2

-    def test_episode_paths_are_relative_to_show_root(self, tmp_path):
+    def test_pack_episode_paths_relative_and_flat(self, tmp_path):
        library, show_root = _library_with_show(tmp_path)
        repo = DotAlfredSeriesReleaseRepository(library)
        release = rescan_show(
@@ -181,11 +191,33 @@ class TestHappyPath:
        assert s1 is not None
        for ep in s1.episodes:
            path_str = str(ep.file_path)
-            # Must NOT be absolute and must start with the season folder.
            assert not Path(path_str).is_absolute()
+            # PACK files sit directly inside the season folder.
            assert path_str.startswith("Foundation.S01.1080p.x265-ELiTE/")
+            assert path_str.count("/") == 1

-    def test_season_folder_name_recorded(self, tmp_path):
+    def test_episodic_episode_paths_descend_into_subfolders(self, tmp_path):
+        library, show_root = _library_with_show(tmp_path)
+        repo = DotAlfredSeriesReleaseRepository(library)
+        release = rescan_show(
+            show_root,
+            tmdb_id=_TMDB_ID,
+            imdb_id=_IMDB_ID,
+            series_repo=repo,
+            scanner=_SCANNER,
+            prober=_StubProber(),
+            kb=_KB,
+        )
+        s2 = release.get_season(SeasonNumber(2))
+        assert s2 is not None
+        for ep in s2.episodes:
+            path_str = str(ep.file_path)
+            assert not Path(path_str).is_absolute()
+            # EPISODIC: season/episode_subdir/video.mkv → 2 separators.
+            assert path_str.startswith("Foundation.S02/")
+            assert path_str.count("/") == 2
+
+    def test_season_folder_names_recorded(self, tmp_path):
        library, show_root = _library_with_show(tmp_path)
        repo = DotAlfredSeriesReleaseRepository(library)
        release = rescan_show(
@@ -201,7 +233,7 @@ class TestHappyPath:
        s2 = release.get_season(SeasonNumber(2))
        assert s1 is not None and s2 is not None
        assert s1.folder == "Foundation.S01.1080p.x265-ELiTE"
-        assert s2.folder == "Foundation.S02.1080p.x265-ELiTE"
+        assert s2.folder == "Foundation.S02"

    def test_persists_sidecar_on_disk(self, tmp_path):
        library, show_root = _library_with_show(tmp_path)
@@ -216,7 +248,6 @@ class TestHappyPath:
            kb=_KB,
        )
        assert (show_root / SIDECAR_FILENAME).is_file()
-        # Round-trip via the repo.
        recovered = repo.find_by_tmdb_id(_TMDB_ID)
        assert recovered is not None
        assert len(recovered.seasons) == 2
@@ -234,8 +265,8 @@ class TestHappyPath:
            prober=prober,
            kb=_KB,
        )
-        # 3 episodes + 1 pack video = 4 probes.
-        assert len(prober.calls) == 4
+        # 3 PACK files + 2 EPISODIC files = 5 probes.
+        assert len(prober.calls) == 5

    def test_imdb_id_optional(self, tmp_path):
        library, show_root = _library_with_show(tmp_path)
@@ -274,12 +305,35 @@ class TestEdgeCases:
        assert release.seasons == ()
        assert (show_root / SIDECAR_FILENAME).is_file()

-    def test_season_folder_without_videos_is_skipped(self, tmp_path, caplog):
+    def test_empty_season_folder_is_skipped(self, tmp_path):
        library = tmp_path / "library"
        library.mkdir()
        show_root = library / "Foundation"
        show_root.mkdir()
-        (show_root / "Foundation.S01.WEB").mkdir()  # empty season folder
+        (show_root / "Foundation.S01.WEB").mkdir()  # empty
+        repo = DotAlfredSeriesReleaseRepository(library)
+        release = rescan_show(
+            show_root,
+            tmdb_id=_TMDB_ID,
+            series_repo=repo,
+            scanner=_SCANNER,
+            prober=_StubProber(),
+            kb=_KB,
+        )
+        assert release.seasons == ()
+
+    def test_malformed_mixed_season_is_skipped(self, tmp_path, caplog):
+        library = tmp_path / "library"
+        library.mkdir()
+        show_root = library / "Foundation"
+        show_root.mkdir()
+        season = show_root / "Foundation.S01.Mixed"
+        season.mkdir()
+        # Flat video + subfolder → malformed.
+        (season / "Foundation.S01E01.mkv").write_bytes(b"")
+        sub = season / "Foundation.S01E02-RG"
+        sub.mkdir()
+        (sub / "Foundation.S01E02-RG.mkv").write_bytes(b"")
        repo = DotAlfredSeriesReleaseRepository(library)
        with caplog.at_level("WARNING"):
            release = rescan_show(
@@ -291,16 +345,47 @@ class TestEdgeCases:
                kb=_KB,
            )
        assert release.seasons == ()
-        assert any("no video file" in r.message for r in caplog.records)
+        assert any(
+            "mixes flat videos and subfolders" in r.message
+            for r in caplog.records
+        )
+
+    def test_single_unnumbered_pack_file_is_skipped(self, tmp_path, caplog):
+        # A season folder with a single video whose name has no Exx
+        # marker: the season number is parsed but the episode is not,
+        # so the file is skipped and (no other parseable files →)
+        # the whole season is skipped.
+        library = tmp_path / "library"
+        library.mkdir()
+        show_root = library / "Foundation"
+        show_root.mkdir()
+        season = show_root / "Foundation.S01.Complete"
+        season.mkdir()
+        (season / "Foundation.S01.Complete.1080p.mkv").write_bytes(b"")
+        repo = DotAlfredSeriesReleaseRepository(library)
+        with caplog.at_level("WARNING"):
+            release = rescan_show(
+                show_root,
+                tmdb_id=_TMDB_ID,
+                series_repo=repo,
+                scanner=_SCANNER,
+                prober=_StubProber(),
+                kb=_KB,
+            )
+        assert release.seasons == ()
+        assert any(
+            "no parseable episodes" in r.message or
+            "no episode number parsed" in r.message
+            for r in caplog.records
+        )

    def test_prober_returning_none_still_produces_episodes(self, tmp_path):
        library = tmp_path / "library"
        library.mkdir()
        show_root = _make_foundation_library(
-            library, episodic_episodes=(1,), include_pack_s2=False
+            library, pack_episodes=(1,), episodic_episodes=()
        )
        repo = DotAlfredSeriesReleaseRepository(library)
-        # Prober returns None — inspect_release skips enrichment, tracks empty.
        release = rescan_show(
            show_root,
            tmdb_id=_TMDB_ID,
@@ -3,6 +3,7 @@
 from __future__ import annotations

 from alfred.application.tv_shows.walker import walk_show
+from alfred.domain.releases.value_objects import ReleaseMode
 from alfred.infrastructure.filesystem.scanner import PathlibFilesystemScanner
 from alfred.infrastructure.knowledge.release_kb import YamlReleaseKnowledge

@@ -10,10 +11,15 @@ _KB = YamlReleaseKnowledge()
 _SCANNER = PathlibFilesystemScanner()


-def _episode(dir_path, name: str) -> None:
+def _file(dir_path, name: str) -> None:
    (dir_path / name).write_bytes(b"")


+# ════════════════════════════════════════════════════════════════════════════
+# Season-folder detection (by Sxx token in folder name)
+# ════════════════════════════════════════════════════════════════════════════
+
+
 class TestSeasonFolderDetection:
    def test_keeps_folders_with_season_token(self, tmp_path):
        show = tmp_path / "Foundation"
@@ -66,54 +72,162 @@ class TestSeasonFolderDetection:
        ]


-class TestVideoFileCollection:
-    def test_keeps_only_video_extensions(self, tmp_path):
+# ════════════════════════════════════════════════════════════════════════════
+# PACK classification (flat numbered videos inside season folder)
+# ════════════════════════════════════════════════════════════════════════════
+
+
+class TestPackClassification:
+    def test_flat_videos_yield_pack(self, tmp_path):
+        show = tmp_path / "Foundation"
+        show.mkdir()
+        season = show / "Foundation.S01.1080p.WEB-DL.x265-GROUP"
+        season.mkdir()
+        _file(season, "Foundation.S01E01.1080p.WEB-DL.x265-GROUP.mkv")
+        _file(season, "Foundation.S01E02.1080p.WEB-DL.x265-GROUP.mkv")
+        tree = walk_show(show, scanner=_SCANNER, kb=_KB)
+        (folder,) = tree.season_folders
+        assert folder.mode is ReleaseMode.PACK
+        assert [p.name for p in folder.video_files] == [
+            "Foundation.S01E01.1080p.WEB-DL.x265-GROUP.mkv",
+            "Foundation.S01E02.1080p.WEB-DL.x265-GROUP.mkv",
+        ]
+
+    def test_pack_keeps_only_video_extensions(self, tmp_path):
        show = tmp_path / "Foundation"
        show.mkdir()
        season = show / "Foundation.S01.WEB"
        season.mkdir()
-        _episode(season, "Foundation.S01E01.mkv")
-        _episode(season, "Foundation.S01E02.mp4")
-        _episode(season, "Foundation.S01E01.eng.srt")  # not a video
-        _episode(season, "info.nfo")  # not a video
+        _file(season, "Foundation.S01E01.mkv")
+        _file(season, "Foundation.S01E02.mp4")
+        _file(season, "Foundation.S01E01.eng.srt")  # not a video
+        _file(season, "info.nfo")  # not a video
        tree = walk_show(show, scanner=_SCANNER, kb=_KB)
        (folder,) = tree.season_folders
+        assert folder.mode is ReleaseMode.PACK
        suffixes = sorted(p.suffix for p in folder.video_files)
        assert suffixes == [".mkv", ".mp4"]

-    def test_empty_season_folder_surfaces(self, tmp_path):
+
+# ════════════════════════════════════════════════════════════════════════════
+# EPISODIC classification (subfolders, one video each)
+# ════════════════════════════════════════════════════════════════════════════
+
+
+class TestEpisodicClassification:
+    def test_subfolders_yield_episodic(self, tmp_path):
+        show = tmp_path / "Foundation"
+        show.mkdir()
+        season = show / "Foundation.S02"
+        season.mkdir()
+        for n in (1, 2):
+            sub = season / f"Foundation.S02E{n:02d}.1080p.x265-GROUP"
+            sub.mkdir()
+            _file(sub, f"Foundation.S02E{n:02d}.1080p.x265-GROUP.mkv")
+        tree = walk_show(show, scanner=_SCANNER, kb=_KB)
+        (folder,) = tree.season_folders
+        assert folder.mode is ReleaseMode.EPISODIC
+        assert [p.name for p in folder.video_files] == [
+            "Foundation.S02E01.1080p.x265-GROUP.mkv",
+            "Foundation.S02E02.1080p.x265-GROUP.mkv",
+        ]
+
+    def test_episodic_paths_descend_into_subfolders(self, tmp_path):
+        show = tmp_path / "Foundation"
+        show.mkdir()
+        season = show / "Foundation.S02"
+        season.mkdir()
+        sub = season / "Foundation.S02E01-RG"
+        sub.mkdir()
+        _file(sub, "Foundation.S02E01-RG.mkv")
+        tree = walk_show(show, scanner=_SCANNER, kb=_KB)
+        (folder,) = tree.season_folders
+        assert folder.video_files == (sub / "Foundation.S02E01-RG.mkv",)
+
+    def test_episodic_skips_empty_subfolder(self, tmp_path, caplog):
+        show = tmp_path / "Foundation"
+        show.mkdir()
+        season = show / "Foundation.S02"
+        season.mkdir()
+        (season / "Foundation.S02E01-RG").mkdir()  # empty subfolder
+        good_sub = season / "Foundation.S02E02-RG"
+        good_sub.mkdir()
+        _file(good_sub, "Foundation.S02E02-RG.mkv")
+        with caplog.at_level("WARNING"):
+            tree = walk_show(show, scanner=_SCANNER, kb=_KB)
+        (folder,) = tree.season_folders
+        assert folder.mode is ReleaseMode.EPISODIC
+        assert len(folder.video_files) == 1
+        assert any("no video" in r.message for r in caplog.records)
+
+    def test_episodic_with_two_videos_in_subfolder_skips_season(
+        self, tmp_path, caplog
+    ):
+        show = tmp_path / "Foundation"
+        show.mkdir()
+        season = show / "Foundation.S02"
+        season.mkdir()
+        sub = season / "Foundation.S02E01-RG"
+        sub.mkdir()
+        _file(sub, "Foundation.S02E01-RG.mkv")
+        _file(sub, "Foundation.S02E01-RG-extras.mkv")  # second video!
+        with caplog.at_level("WARNING"):
+            tree = walk_show(show, scanner=_SCANNER, kb=_KB)
+        (folder,) = tree.season_folders
+        assert folder.mode is None
+        assert folder.video_files == ()
+        assert any("malformed" in r.message for r in caplog.records)
+
+
+# ════════════════════════════════════════════════════════════════════════════
+# Malformed and empty seasons
+# ════════════════════════════════════════════════════════════════════════════
+
+
+class TestMalformedAndEmpty:
+    def test_mixed_flat_and_subfolders_is_malformed(self, tmp_path, caplog):
+        show = tmp_path / "Foundation"
+        show.mkdir()
+        season = show / "Foundation.S01.Mixed"
+        season.mkdir()
+        _file(season, "Foundation.S01E01.mkv")  # flat video
+        sub = season / "Foundation.S01E02-RG"  # AND a subfolder
+        sub.mkdir()
+        _file(sub, "Foundation.S01E02-RG.mkv")
+        with caplog.at_level("WARNING"):
+            tree = walk_show(show, scanner=_SCANNER, kb=_KB)
+        (folder,) = tree.season_folders
+        assert folder.mode is None
+        assert folder.video_files == ()
+        assert any("mixes flat videos and subfolders" in r.message for r in caplog.records)
+
+    def test_empty_season_folder_yields_unknown_mode(self, tmp_path):
        show = tmp_path / "Foundation"
        show.mkdir()
        (show / "Foundation.S01.WEB").mkdir()
        tree = walk_show(show, scanner=_SCANNER, kb=_KB)
        (folder,) = tree.season_folders
+        assert folder.mode is None
        assert folder.video_files == ()

-    def test_single_pack_video_is_kept(self, tmp_path):
-        show = tmp_path / "Foundation"
-        show.mkdir()
-        season = show / "Foundation.S01.Complete.WEB"
-        season.mkdir()
-        _episode(season, "Foundation.S01.Complete.WEB.mkv")
-        tree = walk_show(show, scanner=_SCANNER, kb=_KB)
-        (folder,) = tree.season_folders
-        assert [p.name for p in folder.video_files] == [
-            "Foundation.S01.Complete.WEB.mkv"
-        ]
-
-    def test_does_not_recurse_into_sub_subfolders(self, tmp_path):
+    def test_season_with_only_non_video_files_is_unknown(self, tmp_path):
+        # No videos, no subfolders → unknown. The .nfo doesn't count.
        show = tmp_path / "Foundation"
        show.mkdir()
        season = show / "Foundation.S01.WEB"
        season.mkdir()
-        nested = season / "Extras"
-        nested.mkdir()
-        _episode(nested, "Foundation.S01E01.mkv")
+        _file(season, "info.nfo")
        tree = walk_show(show, scanner=_SCANNER, kb=_KB)
        (folder,) = tree.season_folders
+        assert folder.mode is None
        assert folder.video_files == ()


+# ════════════════════════════════════════════════════════════════════════════
+# Edge cases on the show root itself
+# ════════════════════════════════════════════════════════════════════════════
+
+
 class TestEdgeCases:
    def test_empty_show_root(self, tmp_path):
        show = tmp_path / "Empty"