A film alone in a disc folder takes the wrong folder's name #206

New issue

Closed

opened 2026-09-12 17:17:55 +02:00 by Claude · 1 comment

Claude commented

2026-09-12 17:17:55 +02:00

Collaborator

A film alone in a disc folder is named after the grandparent here and after the disc folder upstream. The port's rule has no basis in upstream's code, and a test asserts the port's answer as though it were upstream's.

The case

/movies/I, Robot (2004)/CD 01/I Robot (2004).mkv

Upstream: MovieResolver.FindMovie on CD 01 finds result.Items.Count == 1, no photos and no subfolders, with collectionType == movies, and so takes MovieResolver.cs:484-487:

movie.Name = Path.GetFileName(movie.ContainingFolderPath);

ContainingFolderPath for a non-stacked VideoFile is GetDirectoryName(Path) = …/CD 01, so the row is named CD 01. There is no disc exception anywhere on that path — AllExtrasTypesFolderNames is checked, disc-folder names are not.

Ours: crates/ephyra-app/src/scan/tree/movies.rs:244 and :428 suppress the folder name when is_a_disc, giving I Robot.

Why this is worse than a plain divergence

crates/ephyra-app/tests/scan.rs:651 asserts it, with the comment "the disc does not name it either" — so the port's own invention is written down as upstream's behaviour. A later reader has no way to tell the assertion from a verified one. Whichever way it is settled, the test's comment has to stop claiming upstream says this.

It is possible the port's answer is the nicer one for a user. That is not the question the harness answers; if it is kept deliberately it belongs in DECISIONS.md (see DECISIONS.md:1150-1155), not in a test comment.

Verifying

This was found by reading MovieResolver.cs, Video.cs and BaseItem.cs and probing the port — not by a real upstream scan, because the tree the answer key covers contains no such case. cargo xtask reference gen-library-kinds now stands up its own container; adding movies/I, Robot (2004)/CD 01/… to its tree settles it in one run.

Found reviewing #194.

A film alone in a disc folder is named after the **grandparent** here and after the **disc folder** upstream. The port's rule has no basis in upstream's code, and a test asserts the port's answer as though it were upstream's. ## The case `/movies/I, Robot (2004)/CD 01/I Robot (2004).mkv` Upstream: `MovieResolver.FindMovie` on `CD 01` finds `result.Items.Count == 1`, no photos and no subfolders, with `collectionType == movies`, and so takes `MovieResolver.cs:484-487`: ```csharp movie.Name = Path.GetFileName(movie.ContainingFolderPath); ``` `ContainingFolderPath` for a non-stacked `VideoFile` is `GetDirectoryName(Path)` = `…/CD 01`, so the row is named **`CD 01`**. There is no disc exception anywhere on that path — `AllExtrasTypesFolderNames` is checked, disc-folder names are not. Ours: `crates/ephyra-app/src/scan/tree/movies.rs:244` and `:428` suppress the folder name when `is_a_disc`, giving `I Robot`. ## Why this is worse than a plain divergence `crates/ephyra-app/tests/scan.rs:651` asserts it, with the comment "the disc does not name it either" — so the port's own invention is written down as upstream's behaviour. A later reader has no way to tell the assertion from a verified one. Whichever way it is settled, the test's comment has to stop claiming upstream says this. It is possible the port's answer is the nicer one for a user. That is not the question the harness answers; if it is kept deliberately it belongs in `DECISIONS.md` (see `DECISIONS.md:1150-1155`), not in a test comment. ## Verifying This was found by reading `MovieResolver.cs`, `Video.cs` and `BaseItem.cs` and probing the port — **not** by a real upstream scan, because the tree the answer key covers contains no such case. `cargo xtask reference gen-library-kinds` now stands up its own container; adding `movies/I, Robot (2004)/CD 01/…` to its tree settles it in one run. Found reviewing #194.

Claude referenced this issue

2026-09-12 17:34:17 +02:00

A library that is not movies, tvshows or music is configured, listed and never scanned #194

Claude referenced this issue

2026-09-12 17:36:02 +02:00

A library that is not movies, tvshows or music is configured, listed and never scanned #194

Claude commented

2026-09-12 20:05:01 +02:00

Author

Collaborator

Landed on main: 2a5a91ec, 5a2ff728, e0653be6.

Settled in upstream's favour, and confirmed by a real scan rather than argued: movies/I, Robot (2004)/CD 01/I Robot (2004).mp4 is a Movie named CD 01, year 2004, under a Folder I, Robot (2004). The is_a_disc suppression is gone. tests/scan.rs no longer claims upstream says otherwise — a_film_alone_in_a_disc_folder_takes_that_folders_name asserts upstream's answer. No DECISIONS.md entry: the port now matches.

One correction to the reasoning in 2a5a91ec, from review: VIDEO_TS, stream and playlist folders can reach films() (a VIDEO_TS holding a plain .mkv and no .vob is not a rip). Removing the suppression is still right there — FindMovie names that film VIDEO_TS too.

Verified

Against a real upstream scan. The fixture tree gained the cases these issues asked for — a stack at a library root (movies/Recital - cd1/cd2), a film alone in a disc folder (movies/I, Robot (2004)/CD 01/…), and musicvideos/Versions/ renamed to Anthem/ so its files really start with the folder name. gen-library-kinds was regenerated from a live upstream container and every library_kinds test agrees with it.
Review round: no defects in the new write path; filed_in_order was probed against two levels of ownership, a diamond, a cycle and an owner outside the batch. Mutation-checked: each change reverted alone turns its own tests red.
In the web client, over a tree carrying all three shapes: the films library lists exactly Anthem, CD 01 · 2004, Concert and Loose Film · 1999 — no second Anthem from the owned cut and no stack part — which is the same predicate upstream hides them on (BaseItemRepository.TranslateQuery.cs:810-814).

On an existing install, an incremental scan decides a row is unchanged from its file (path, mtime, size), so rows filed before this change keep their old value until the file is touched or a full refresh runs. New rows are right immediately.

Landed on `main`: `2a5a91ec`, `5a2ff728`, `e0653be6`. Settled in upstream's favour, and confirmed by a real scan rather than argued: `movies/I, Robot (2004)/CD 01/I Robot (2004).mp4` is a `Movie` named **`CD 01`**, year 2004, under a `Folder` `I, Robot (2004)`. The `is_a_disc` suppression is gone. `tests/scan.rs` no longer claims upstream says otherwise — `a_film_alone_in_a_disc_folder_takes_that_folders_name` asserts upstream's answer. No `DECISIONS.md` entry: the port now matches. One correction to the reasoning in `2a5a91ec`, from review: `VIDEO_TS`, `stream` and `playlist` folders *can* reach `films()` (a `VIDEO_TS` holding a plain `.mkv` and no `.vob` is not a rip). Removing the suppression is still right there — `FindMovie` names that film `VIDEO_TS` too. ## Verified - **Against a real upstream scan.** The fixture tree gained the cases these issues asked for — a stack at a library root (`movies/Recital - cd1/cd2`), a film alone in a disc folder (`movies/I, Robot (2004)/CD 01/…`), and `musicvideos/Versions/` renamed to `Anthem/` so its files really start with the folder name. `gen-library-kinds` was regenerated from a live upstream container and every `library_kinds` test agrees with it. - **Review round**: no defects in the new write path; `filed_in_order` was probed against two levels of ownership, a diamond, a cycle and an owner outside the batch. Mutation-checked: each change reverted alone turns its own tests red. - **In the web client**, over a tree carrying all three shapes: the films library lists exactly `Anthem`, `CD 01 · 2004`, `Concert` and `Loose Film · 1999` — no second `Anthem` from the owned cut and no stack part — which is the same predicate upstream hides them on (`BaseItemRepository.TranslateQuery.cs:810-814`). **On an existing install**, an incremental scan decides a row is unchanged from its file (path, mtime, size), so rows filed before this change keep their old value until the file is touched or a full refresh runs. New rows are right immediately.

Claude closed this issue

2026-09-12 20:05:01 +02:00