alex/ephyra
Watch
1
0
Fork
You've already forked ephyra
 0
Code Issues 2 Pull requests Projects Releases Packages 1 Wiki Activity Actions

A recursive listing over a box set comes back in a different order #133

New issue
Closed
opened 2026-08-14 02:55:12 +02:00 by Claude · 1 comment
Claude commented 2026-08-14 02:55:12 +02:00
Collaborator
Copy link
/Items?parentId={box set}&recursive=true
  upstream: Test Pattern S01E01, Alien, Night Signal
  ours:     sort-name order

Measured during #111 on a box set holding one row from each of three libraries. Identical before and after that change, so it belongs to neither it nor #91 — it looks like the set's own stored order against the query's ordering.

Upstream keeps what a collection was given in LinkedChildren, and M9 settled that a playlist's contents are that table rather than AncestorIds. So the likely rule is that a set's contents come back in the order the set holds them unless the request asks otherwise, which is the same shape as ids returning rows in the order they were named (fixed in #91 from Folder.GetItems:996-1008).

No scenario covers it, and #130 currently makes it awkward to measure: this server's startup scan deletes box-set rows, so a pair has to be arranged with no libraries configured.

Where to start

crates/jellyfin-http/src/routes/items.rs's linked-contents branch, and whatever crates/jellyfin-db does with LinkedChildren ordering. Upstream: Folder.GetChildren and the BoxSet/Playlist overrides.

Check the playlist case in the same pass — a playlist is the same table and a user-ordered one is far more likely to be noticed out of order than a box set is.

How to know it is done

The listing above agrees, for a box set and for a playlist, and a scenario carries both.

House rules

Tests run under cargo nextest; upstream's source is the checkout in upstream/ at the pin; generated artifacts are regenerated, never edited — a failing corpus case is the port disagreeing with upstream, not the corpus being wrong. Comments say what the code does, not how it got there. The shared reference on :8096 is read-only; anything needing a write wants its own container from the pinned image.

``` /Items?parentId={box set}&recursive=true upstream: Test Pattern S01E01, Alien, Night Signal ours: sort-name order ``` Measured during #111 on a box set holding one row from each of three libraries. Identical before and after that change, so it belongs to neither it nor #91 — it looks like the set's own stored order against the query's ordering. Upstream keeps what a collection was given in `LinkedChildren`, and M9 settled that a playlist's contents are that table rather than `AncestorIds`. So the likely rule is that a set's contents come back in the order the set holds them unless the request asks otherwise, which is the same shape as `ids` returning rows in the order they were named (fixed in #91 from `Folder.GetItems:996-1008`). No scenario covers it, and #130 currently makes it awkward to measure: this server's startup scan deletes box-set rows, so a pair has to be arranged with no libraries configured. ## Where to start `crates/jellyfin-http/src/routes/items.rs`'s linked-contents branch, and whatever `crates/jellyfin-db` does with `LinkedChildren` ordering. Upstream: `Folder.GetChildren` and the `BoxSet`/`Playlist` overrides. Check the playlist case in the same pass — a playlist is the same table and a user-ordered one is far more likely to be noticed out of order than a box set is. ## How to know it is done The listing above agrees, for a box set and for a playlist, and a scenario carries both. ### House rules Tests run under `cargo nextest`; upstream's source is the checkout in `upstream/` **at the pin**; generated artifacts are regenerated, never edited — a failing corpus case is the port disagreeing with upstream, not the corpus being wrong. Comments say what the code does, not how it got there. The shared reference on `:8096` is read-only; anything needing a write wants its own container from the pinned image.
Claude commented 2026-08-14 12:25:27 +02:00
Author
Collaborator
Copy link

Done, on main. Four cases, not one.

box set,  direct     upstream premiere date     ours was link order
box set,  recursive  upstream sort name         ours was link order
playlist, direct     upstream link order        ours link order  (agreed)
playlist, recursive  upstream sort name         ours was link order

BoxSet.cs:26 defaults DisplayOrder to PremiereDate and :118 falls back to it; a recursive listing goes through QueryRecursive → GetItemsResult, whose default is OrderBy(SortName); Playlist.GetChildren keeps link order. We answered link order for all four.

The committed fixture cannot tell two of these apart — its three rows order identically under premiere date and sort name — so the reviewer built a box set whose two orders differ and measured on that. All four hold.

It also forced a real fix in the shared sort table: upstream's PremiereDate coalesces with ProductionYear, on both the SQL path (OrderMapper.cs:76) and the in-memory one (PremiereDateComparer.GetDate), where ours read the bare column. The fixture's rows carry no premiere date at all and still order 2020 → 2021 → 2022.

A collection's stored DisplayOrder is read now rather than assumed: Default leaves the set in its own order, a name we map sorts by it, and anything else — including the empty value every set made before upstream began writing the field carries — falls back to the premiere date.

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`. Four cases, not one. ``` box set, direct upstream premiere date ours was link order box set, recursive upstream sort name ours was link order playlist, direct upstream link order ours link order (agreed) playlist, recursive upstream sort name ours was link order ``` `BoxSet.cs:26` defaults `DisplayOrder` to `PremiereDate` and `:118` falls back to it; a recursive listing goes through `QueryRecursive` → `GetItemsResult`, whose default is `OrderBy(SortName)`; `Playlist.GetChildren` keeps link order. We answered link order for all four. **The committed fixture cannot tell two of these apart** — its three rows order identically under premiere date and sort name — so the reviewer built a box set whose two orders differ and measured on that. All four hold. It also forced a real fix in the shared sort table: upstream's `PremiereDate` **coalesces with `ProductionYear`**, on both the SQL path (`OrderMapper.cs:76`) and the in-memory one (`PremiereDateComparer.GetDate`), where ours read the bare column. The fixture's rows carry no premiere date at all and still order 2020 → 2021 → 2022. A collection's stored `DisplayOrder` is read now rather than assumed: `Default` leaves the set in its own order, a name we map sorts by it, and anything else — including the empty value every set made before upstream began writing the field carries — falls back to the premiere date. --- ## 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`.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
No results found.
Labels
Clear labels   
correctness
Diverges from upstream

  
encoding
Transcoding, ffmpeg, and the arg strings

  
http
Route behaviour

  
jobs
Scheduled tasks

  
operations
Installing a server and keeping one running

  
p1
Do this first

  
p2
After the p1s

  
p3
Real, and not urgent

  
performance
Speed or resource use

  
providers
Metadata from somewhere other than the file

  
scanner
The library scan

  
security
Who is allowed to see what

  
testing
Test coverage and harness

No labels
correctness
encoding
http
jobs
operations
p1
p2
p3
performance
providers
scanner
security
testing
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
alex/ephyra#133
No description provided.