/Library/MediaFolders ignores isHidden #123

New issue

Closed

opened 2026-08-14 00:15:43 +02:00 by Claude · 1 comment

Claude commented

2026-08-14 00:15:43 +02:00

Collaborator

Measured on the live pair during #91's review:

/Library/MediaFolders?isHidden=false    upstream: 5 rows    ours: 6
/Library/MediaFolders                   upstream: 6 rows    ours: 6

So the row set is right and the parameter is not read. The extra row under isHidden=false is the playlists folder, which upstream hides there.

This was first reported as "we list the playlists folder and upstream does not", which is not what is happening — without the parameter upstream lists it too. crates/jellyfin-http/src/routes/library.rs:232 appends it deliberately with a comment saying upstream does, and that comment is correct; what is missing is the filter.

Same class as the unbound /Items filters, one route over.

Where to start

crates/jellyfin-http/src/routes/library.rs around :232. Upstream is LibraryController.GetMediaFolders, whose isHidden maps onto the folder's own hidden flag — check what it reads that flag off, since a CollectionFolder's is not the same question as a user's view being hidden.

How to know it is done

Both spellings of the request agree with upstream, and a scenario carries the parameterised one.

House rules

Tests run under cargo nextest; upstream's source is the checkout in upstream/
at the pin (v12.0-rc5, 6d501ba4); generated files are regenerated rather
than edited; comments say what the code does, not how it got there. The
reference pair is cargo xtask reference up, a run is
cargo run -p jellyfin-harness -- replay --upstream URL --ours URL --scenario FILE --out DIR,
and crates/jellyfin-harness/README.md says what a diff means. A server started
off current main binds UDP 7359 for discovery — set [discovery] enabled = false
in a scratch config if another one is already running.

Measured on the live pair during #91's review: ``` /Library/MediaFolders?isHidden=false upstream: 5 rows ours: 6 /Library/MediaFolders upstream: 6 rows ours: 6 ``` So the row set is right and the **parameter is not read**. The extra row under `isHidden=false` is the playlists folder, which upstream hides there. This was first reported as "we list the playlists folder and upstream does not", which is not what is happening — without the parameter upstream lists it too. `crates/jellyfin-http/src/routes/library.rs:232` appends it deliberately with a comment saying upstream does, and that comment is correct; what is missing is the filter. Same class as the unbound `/Items` filters, one route over. ## Where to start `crates/jellyfin-http/src/routes/library.rs` around `:232`. Upstream is `LibraryController.GetMediaFolders`, whose `isHidden` maps onto the folder's own hidden flag — check what it reads that flag off, since a `CollectionFolder`'s is not the same question as a user's view being hidden. ## How to know it is done Both spellings of the request agree with upstream, and a scenario carries the parameterised one. ### House rules Tests run under `cargo nextest`; upstream's source is the checkout in `upstream/` **at the pin** (`v12.0-rc5`, `6d501ba4`); generated files are regenerated rather than edited; comments say what the code does, not how it got there. The reference pair is `cargo xtask reference up`, a run is `cargo run -p jellyfin-harness -- replay --upstream URL --ours URL --scenario FILE --out DIR`, and `crates/jellyfin-harness/README.md` says what a diff means. A server started off current `main` binds UDP 7359 for discovery — set `[discovery] enabled = false` in a scratch config if another one is already running.

Claude added the

correctness

http

labels

2026-08-14 00:15:43 +02:00

Claude referenced this issue

2026-08-14 00:38:51 +02:00

/Items: six id filters are accepted and never applied, genreIds matches nothing, and a parentless listing returns the whole library #91

Claude commented

2026-08-14 12:25:26 +02:00

Author

Collaborator

Done, on main.

/Library/MediaFolders                upstream 5   ours 5
/Library/MediaFolders?isHidden=false upstream 4   ours was 5, now 4
/Library/MediaFolders?isHidden=true  upstream 1   ours was 0, now 1

The row set was always right and the parameter was simply not read — which is why the first report of this ("we list the playlists folder and upstream does not") was wrong: without the parameter upstream lists it too.

IsHidden is overridden to true in PlaylistsFolder alone; BaseItem's own is virtual … => false. So the one folder a media-folder listing calls hidden is the playlists folder, and isHidden=true returning exactly it is the check.

The batch this closed in

Seven issues went out together because they looked like one defect class. They turned out to be three roots:

#120 and #124 — /Items' Params had been grown one client at a time instead of held against upstream's controller signature, and /UserItems/Resume shared it.
#121 and #122 — an ordering key read off the wrong column: sortBy=Name maps to CleanName upstream, not SortName, and a by-name search orders by relevance with no sort-name tiebreak, so ties fall out in id order.
#133 — its own, and it forced a real fix in the shared sort table: upstream's PremiereDate coalesces with ProductionYear where ours read the bare column.

A reviewer re-derived the parameter audit mechanically — extracting every [FromQuery] name from GetItems and doing set arithmetic against what we bind — and got nothing bound that upstream does not take and nothing in the register that is not an upstream parameter. This class is closed rather than narrowed, which matters because it had been found three times.

Verified on a restored pair: browsing 34 → 0, items 33 requests at 0, clients 56 → 3, portable 115 → 6. JELLYFIN_RS_REQUIRE_FIXTURES=1 cargo nextest run --workspace: 2228 passed, 0 skipped. Clippy clean. No ignore-list entry was added or touched.

Two things the review caught that the batch had not

Five of the ten new scenario entries could not fail. The default snapshot has no genres, studios or tags, so five items.json entries compared two empty pages — reverting the whole fix would still have replayed clean. They moved into editing.json, after the save that gives one film a genre, a tag, a certificate and a studio and then takes them away, so each selects one row out of nineteen. That is the fifth time this session the same failure has turned up in a different layer, after M5's vacuous scenarios, M8's bundle.json, #111's all-refusals visibility.json and #137's silently-skipping tests.

Reading the reference can write to it. Studios=, Artists= and Albums= resolve through upstream's GetStudio/GetArtist, which call CreateItemByName — so a read files a BaseItems row. That is how a stray Brandywine Productions got into the shared fixture mid-session and made portable.json disagree by 149. Genres, Tags and OfficialRatings narrow through ItemValues and are safe. Both halves are now written into crates/jellyfin-harness/README.md.

Done, on `main`. ``` /Library/MediaFolders upstream 5 ours 5 /Library/MediaFolders?isHidden=false upstream 4 ours was 5, now 4 /Library/MediaFolders?isHidden=true upstream 1 ours was 0, now 1 ``` The row set was always right and the parameter was simply not read — which is why the first report of this ("we list the playlists folder and upstream does not") was wrong: without the parameter upstream lists it too. `IsHidden` is overridden to `true` in `PlaylistsFolder` alone; `BaseItem`'s own is `virtual … => false`. So the one folder a media-folder listing calls hidden is the playlists folder, and `isHidden=true` returning exactly it is the check. --- ## The batch this closed in Seven issues went out together because they looked like one defect class. They turned out to be **three roots**: - **#120 and #124** — `/Items`' `Params` had been grown one client at a time instead of held against upstream's controller signature, and `/UserItems/Resume` shared it. - **#121 and #122** — an ordering key read off the wrong column: `sortBy=Name` maps to `CleanName` upstream, not `SortName`, and a by-name search orders by relevance with no sort-name tiebreak, so ties fall out in id order. - **#133** — its own, and it forced a real fix in the shared sort table: upstream's `PremiereDate` coalesces with `ProductionYear` where ours read the bare column. A reviewer re-derived the parameter audit mechanically — extracting every `[FromQuery]` name from `GetItems` and doing set arithmetic against what we bind — and got nothing bound that upstream does not take and nothing in the register that is not an upstream parameter. **This class is closed rather than narrowed**, which matters because it had been found three times. Verified on a restored pair: `browsing` **34 → 0**, `items` 33 requests at 0, `clients` **56 → 3**, `portable` 115 → 6. `JELLYFIN_RS_REQUIRE_FIXTURES=1 cargo nextest run --workspace`: **2228 passed, 0 skipped**. Clippy clean. No ignore-list entry was added or touched. ### Two things the review caught that the batch had not **Five of the ten new scenario entries could not fail.** The `default` snapshot has no genres, studios or tags, so five `items.json` entries compared two empty pages — reverting the whole fix would still have replayed clean. They moved into `editing.json`, after the save that gives one film a genre, a tag, a certificate and a studio and then takes them away, so each selects one row out of nineteen. That is the fifth time this session the same failure has turned up in a different layer, after M5's vacuous scenarios, M8's `bundle.json`, #111's all-refusals `visibility.json` and #137's silently-skipping tests. **Reading the reference can write to it.** `Studios=`, `Artists=` and `Albums=` resolve through upstream's `GetStudio`/`GetArtist`, which call `CreateItemByName` — so a *read* files a `BaseItems` row. That is how a stray `Brandywine Productions` got into the shared fixture mid-session and made `portable.json` disagree by 149. `Genres`, `Tags` and `OfficialRatings` narrow through `ItemValues` and are safe. Both halves are now written into `crates/jellyfin-harness/README.md`.

Claude closed this issue

2026-08-14 12:25:26 +02:00