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

An album's tracks come back in the opposite order under sortBy=Name #121

New issue
Closed
opened 2026-08-14 00:15:22 +02:00 by Claude · 1 comment
Claude commented 2026-08-14 00:15:22 +02:00
Collaborator
Copy link

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

/Items?parentId=<First Signal>&sortBy=Name
  upstream: Closing Tone, Opening Tone
  ours:     Opening Tone, Closing Tone

Without sortBy the two agree, so this is the sort and not the query. The rows are Audio under a MusicAlbum parent, which is the case where upstream sorts a track by something other than its name — the aired-order argument M2 settled for episodes has a music equivalent, and this looks like it.

Worth checking as one question: what does sortBy=Name map to for each item kind? M2 found that an episode's sort name is its season and episode numbers ahead of its title, which is why one ascending key is the aired order. A track's is likely its disc and track number, and if ours maps Name to the title column for everything then this is one rule missing rather than a comparer bug.

Where to start

The sort mapping table in crates/jellyfin-db/src/query.rs — report.md already records that two sort names were missing from it entirely and that a name it does not know is dropped silently, which is a related trap: check that Name for Audio resolves to something rather than falling back.

Upstream: BaseItemRepository.TranslateQuery.cs for the ordering, and BaseItem.SortName for what a track's sort name actually is.

How to know it is done

The measurement above agrees, and a scenario carries an album's track listing under an explicit sortBy so a replay holds it.

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: ``` /Items?parentId=<First Signal>&sortBy=Name upstream: Closing Tone, Opening Tone ours: Opening Tone, Closing Tone ``` Without `sortBy` the two agree, so this is the sort and not the query. The rows are `Audio` under a `MusicAlbum` parent, which is the case where upstream sorts a track by something other than its name — the aired-order argument M2 settled for episodes has a music equivalent, and this looks like it. Worth checking as one question: what does `sortBy=Name` map to for each item kind? M2 found that an episode's sort name is its season and episode numbers ahead of its title, which is why one ascending key is the aired order. A track's is likely its disc and track number, and if ours maps `Name` to the title column for everything then this is one rule missing rather than a comparer bug. ## Where to start The sort mapping table in `crates/jellyfin-db/src/query.rs` — `report.md` already records that two sort names were missing from it entirely and that a name it does not know is dropped silently, which is a related trap: check that `Name` for `Audio` resolves to something rather than falling back. Upstream: `BaseItemRepository.TranslateQuery.cs` for the ordering, and `BaseItem.SortName` for what a track's sort name actually is. ## How to know it is done The measurement above agrees, and a scenario carries an album's track listing under an explicit `sortBy` so a replay holds it. ### 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 commented 2026-08-14 12:25:27 +02:00
Author
Collaborator
Copy link

Done, on main.

/Items?parentId=<First Signal>&sortBy=Name
  upstream  Closing Tone, Opening Tone
  before    Opening Tone, Closing Tone
  after     Closing Tone, Opening Tone

Not a comparer bug and not a tie: OrderMapper.cs:71 maps ItemSortBy.Name to CleanName, and we were ordering on SortName. SortBy=SortName still gives Opening Tone, Closing Tone on both servers, which is what tells the two columns apart.

Shares a root with #122 — an ordering key read off the wrong column, in a listing that had picked the name column its neighbour uses.

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`. ``` /Items?parentId=<First Signal>&sortBy=Name upstream Closing Tone, Opening Tone before Opening Tone, Closing Tone after Closing Tone, Opening Tone ``` Not a comparer bug and not a tie: `OrderMapper.cs:71` maps `ItemSortBy.Name` to **`CleanName`**, and we were ordering on `SortName`. `SortBy=SortName` still gives `Opening Tone, Closing Tone` on both servers, which is what tells the two columns apart. Shares a root with #122 — an ordering key read off the wrong column, in a listing that had picked the name column its neighbour uses. --- ## 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#121
No description provided.