MediaCards paged handlers: 1-based paging + delete-or-keep (count drift fixed in #690/#758) #832

New Issue

2026-08-26T08:20:35+02:00

timothy commented

2026-08-26 08:20:35 +02:00

Found by the cold review of PR for #690/#758, while deriving the paged-handler population by SHAPE
rather than by the GetPaged*Handler name. These three are the same mechanism as #690/#758 — the
count and the page are computed from different predicates — but they sit in MediaCards, where the
count and the page are two separate repository methods rather than two expressions in one handler,
so the #690/#758 fix (one IQueryable, count it, page it) does not directly apply.

They were deliberately not fixed in that PR and are recorded here instead of being left
unrecorded, per an issue's file list is not the population.

The three sites

Handler	Count	Page	Drift
`GetTelevisionSeasonCardsHandler:26,35`	`TelevisionRepository.GetSeasonCount` = `Seasons.CountAsync(s => s.ShowId == showId)` (`TelevisionRepository.cs:134`)	`GetPagedSeasons` (`:142`) pages `Seasons.Where(s => showIds.Contains(s.ShowId))` over every show sharing the same Title+Year	A show present in two libraries with 3 seasons each: count 3, page 6
`GetTelevisionEpisodeCardsHandler:38,47`	`Episodes.CountAsync(e => e.SeasonId == id)` (`:179`)	`EpisodeMetadata.Filter(em => em.Episode.SeasonId == id)` (`:187`)	An episode whose metadata row is missing (scanner failure) makes count > pageable rows — the "permanently empty page" symptom
`GetMusicVideoCardsHandler:33,36`	raw Dapper `SELECT COUNT(*) FROM MusicVideo WHERE ArtistId = @ArtistId` (`MusicVideoRepository.cs:171`)	pages `MusicVideoMetadata` (`:179`)	Same metadata-row asymmetry, and a second predicate language on top

Why this is not urgent

All three are currently unreachable. grep 'new GetTelevisionSeasonCards(\|new GetTelevisionEpisodeCards(\|new GetMusicVideoCards('
over the git-tracked *.cs set returns zero hits outside their own Queries/ folders — they are
leftovers from the removed Blazor UI (blazor.ui-removed). Nothing in the SPA, the controllers or
ErsatzTV.Mcp reaches them. So this is dead code carrying a latent defect, not a live bug.

They also use 1-based pageNumber (Skip((pageNumber - 1) * pageSize)), against the repo-wide
0-based contract in api.paging-zero-based — a second reason not to fold them into a fix PR
targeting live handlers.

Options

Delete them — the likeliest right answer if nothing intends to revive media-card browsing;
library browsing is served by GetLibraryBrowseItems, which counts correctly.
Fix them in place — make each repository expose one filtered query that both the count and
the page derive from, and move to 0-based paging.

Decide which before doing either; do not "fix" dead code into permanence by reflex.

Done-when

Each of the three sites is either deleted or made to count the same predicate it pages
If kept, pageNumber is 0-based per api.paging-zero-based
api.paged-count-matches-page-query's deferred-sites paragraph updated to record the outcome
Adversarial review passed

Found by the cold review of PR for #690/#758, while deriving the paged-handler population by SHAPE rather than by the `GetPaged*Handler` name. These three are the same mechanism as #690/#758 — the count and the page are computed from different predicates — but they sit in `MediaCards`, where the count and the page are two separate *repository methods* rather than two expressions in one handler, so the #690/#758 fix (one `IQueryable`, count it, page it) does not directly apply. They were **deliberately not fixed** in that PR and are recorded here instead of being left unrecorded, per `an issue's file list is not the population`. ## The three sites | Handler | Count | Page | Drift | |---|---|---|---| | `GetTelevisionSeasonCardsHandler:26,35` | `TelevisionRepository.GetSeasonCount` = `Seasons.CountAsync(s => s.ShowId == showId)` (`TelevisionRepository.cs:134`) | `GetPagedSeasons` (`:142`) pages `Seasons.Where(s => showIds.Contains(s.ShowId))` over **every show sharing the same Title+Year** | A show present in two libraries with 3 seasons each: count 3, page 6 | | `GetTelevisionEpisodeCardsHandler:38,47` | `Episodes.CountAsync(e => e.SeasonId == id)` (`:179`) | `EpisodeMetadata.Filter(em => em.Episode.SeasonId == id)` (`:187`) | An episode whose metadata row is missing (scanner failure) makes count > pageable rows — the "permanently empty page" symptom | | `GetMusicVideoCardsHandler:33,36` | raw Dapper `SELECT COUNT(*) FROM MusicVideo WHERE ArtistId = @ArtistId` (`MusicVideoRepository.cs:171`) | pages `MusicVideoMetadata` (`:179`) | Same metadata-row asymmetry, and a second predicate language on top | ## Why this is not urgent **All three are currently unreachable.** `grep 'new GetTelevisionSeasonCards(\|new GetTelevisionEpisodeCards(\|new GetMusicVideoCards('` over the git-tracked `*.cs` set returns zero hits outside their own `Queries/` folders — they are leftovers from the removed Blazor UI (`blazor.ui-removed`). Nothing in the SPA, the controllers or `ErsatzTV.Mcp` reaches them. So this is dead code carrying a latent defect, not a live bug. They also use **1-based `pageNumber`** (`Skip((pageNumber - 1) * pageSize)`), against the repo-wide 0-based contract in `api.paging-zero-based` — a second reason not to fold them into a fix PR targeting live handlers. ## Options 1. **Delete them** — the likeliest right answer if nothing intends to revive media-card browsing; library browsing is served by `GetLibraryBrowseItems`, which counts correctly. 2. **Fix them in place** — make each repository expose one filtered query that both the count and the page derive from, and move to 0-based paging. Decide which before doing either; do not "fix" dead code into permanence by reflex. ## Done-when - [ ] Each of the three sites is either deleted or made to count the same predicate it pages - [ ] If kept, `pageNumber` is 0-based per `api.paging-zero-based` - [ ] `api.paged-count-matches-page-query`'s deferred-sites paragraph updated to record the outcome - [ ] Adversarial review passed

timothy added the api bug priority: low labels 2026-08-26 08:20:42 +02:00

timothy commented

2026-08-26 08:37:47 +02:00

Scope narrowed — the count/page drift is FIXED, not deferred. The PR for #690/#758 fixes all
three sites after both cold reviews independently rated the deferral MEDIUM, and after checking the
fact that decided it: each of the six repository methods has exactly one caller (the dead handler),
so there was no blast radius.

Fixed there:

GetSeasonCount expands to the same Title+Year show set GetPagedSeasons pages.
GetEpisodeCount / GetMusicVideoCount count the metadata table their pages are taken from.
Pinned by ErsatzTV.Tests/Application/Paging/MediaCardsCountMatchesPageTests.cs, with a mutation
proof (restoring all three pre-fix counts reddens all four tests).

What is still open here

1-based pageNumber. All three page Skip((pageNumber - 1) * pageSize), against the
repo-wide 0-based contract in api.paging-zero-based. Deliberately not changed alongside a
count-correctness fix.
Delete or keep. All three handlers still have zero call sites outside their own Queries/
folder — leftovers from blazor.ui-removed. If nothing intends to revive media-card browsing,
deleting them (and the six repository methods, which have no other caller) is likely better than
migrating their paging.
An include chain can filter more narrowly than the count — found while writing the tests, and
the reason "count the table the page reads" is necessary but not sufficient here.
GetPagedEpisodes's chain walks Episode -> Season -> Show -> ShowMetadata, and a missing Show
row drops every row, so the page can be narrower than even the corrected metadata count. Not
closed, not claimed closed. If these handlers are kept, this needs its own answer; if they are
deleted, it dies with them.

Done-when (revised)

Decide delete vs keep for the three MediaCards handlers and their six repository methods
If kept: pageNumber is 0-based per api.paging-zero-based, and the include-chain narrowing
in (3) is either fixed or recorded as accepted with its reason
api.paged-count-matches-page-query's MediaCards paragraph updated to record the outcome
Adversarial review passed

**Scope narrowed — the count/page drift is FIXED, not deferred.** The PR for #690/#758 fixes all three sites after both cold reviews independently rated the deferral MEDIUM, and after checking the fact that decided it: each of the six repository methods has exactly one caller (the dead handler), so there was no blast radius. Fixed there: - `GetSeasonCount` expands to the same Title+Year show set `GetPagedSeasons` pages. - `GetEpisodeCount` / `GetMusicVideoCount` count the metadata table their pages are taken from. - Pinned by `ErsatzTV.Tests/Application/Paging/MediaCardsCountMatchesPageTests.cs`, with a mutation proof (restoring all three pre-fix counts reddens all four tests). ## What is still open here 1. **1-based `pageNumber`.** All three page `Skip((pageNumber - 1) * pageSize)`, against the repo-wide 0-based contract in `api.paging-zero-based`. Deliberately not changed alongside a count-correctness fix. 2. **Delete or keep.** All three handlers still have zero call sites outside their own `Queries/` folder — leftovers from `blazor.ui-removed`. If nothing intends to revive media-card browsing, deleting them (and the six repository methods, which have no other caller) is likely better than migrating their paging. 3. **An include chain can filter more narrowly than the count** — found while writing the tests, and the reason "count the table the page reads" is necessary but not sufficient here. `GetPagedEpisodes`'s chain walks `Episode -> Season -> Show -> ShowMetadata`, and a missing `Show` row drops every row, so the page can be narrower than even the corrected metadata count. Not closed, not claimed closed. If these handlers are kept, this needs its own answer; if they are deleted, it dies with them. ## Done-when (revised) - [ ] Decide delete vs keep for the three MediaCards handlers and their six repository methods - [ ] If kept: `pageNumber` is 0-based per `api.paging-zero-based`, and the include-chain narrowing in (3) is either fixed or recorded as accepted with its reason - [ ] `api.paged-count-matches-page-query`'s MediaCards paragraph updated to record the outcome - [ ] Adversarial review passed

timothy changed title from ~~MediaCards paged handlers count a different predicate than they page (same shape as #690/#758, dead code)~~ to MediaCards paged handlers: 1-based paging + delete-or-keep (count drift fixed in #690/#758)

2026-08-26 08:37:48 +02:00

timothy referenced this issue from a commit

2026-08-26 08:59:49 +02:00

fix(690,758): answer cold review — derive the population by shape, not by name

timothy referenced this issue from a commit

2026-08-26 08:59:49 +02:00

fix(690,758): fix the three MediaCards sites instead of deferring them

timothy referenced this issue from a commit

2026-08-26 08:59:49 +02:00

docs(690,758): correct three measured details from round 2

timothy referenced this issue from a commit

2026-08-26 08:59:49 +02:00