The row standing for a name is picked before the filters, not after #183

New issue

Closed

opened 2026-08-22 18:18:08 +02:00 by Claude · 1 comment

Claude commented

2026-08-22 18:18:08 +02:00

Collaborator

Found reviewing #180, which made it easy to reach. The shape predates that
change — restricted sits in the same place and has the same consequence —
so this is not a regression.

What happens

crates/ephyra-db/src/query/names.rs picks the row that stands for a name
before narrowing, and narrows afterwards:

WHERE BaseItems.Type = ?
  AND BaseItems.CleanName IN (…)
  AND BaseItems.Id = (SELECT MIN(Same.Id) FROM BaseItems AS Same
                      WHERE Same.Type = BaseItems.Type
                        AND Same.PresentationUniqueKey IS BaseItems.PresentationUniqueKey)
  AND <the account's filters, and the mark>

representative_of reads the whole group, unfiltered. Upstream does it the
other way round — BaseItemRepository.ByName.cs:196-224 runs
TranslateQuery(innerQuery, context, outerQueryFilter) first and only then
groups by PresentationUniqueKey and takes Min(Id), so the representative is
chosen from the rows that survived the filter.

Where it shows

Two MusicArtist rows sharing a presentation key — the row a music library
found and the row written beside the metadata, which is the case
names.rs:107-110 documents and the reason representative_of has a
MusicArtist branch at all.

Mark the row that wins the pick, ask /Artists?isFavorite=false: upstream
still lists the name, represented by the other row. We drop it.
Mark only the row that loses the pick, ask /Artists?isFavorite=true:
upstream lists it. We answer nothing.

The same holds for an account the rating limit or a blocked tag hides the
representative from: upstream offers the name under the row it may see, we
offer nothing. That half is older and harder to reach, because a filter that
hides one row of a group usually hides the other too — a mark lands on exactly
one row, which is what makes this easy.

Where to start

The conditions restricted and marked_favorite build are written against
BaseItems., so choosing the representative among the rows that pass them
needs the same conditions over the group's alias — either inside the
correlated subquery, or as a NOT EXISTS over a better-ranked row that also
passes. Either way the account's filters have to be generated for a named
table rather than for BaseItems alone, which is the whole of the work.

How to know it is done

A fixture with two rows of one presentation key — one reachable, one not, and
one marked — answers the name under the row that survived, on /Artists and
for an account with something hidden from it. crates/ephyra-http/tests/by_name.rs
has no two rows sharing a key today, so the case has to be built.

House rules

Tests run under cargo nextest; upstream's source is the checkout in
upstream/ at the pin. Generated artifacts are regenerated, never edited.
Anything touching the query builder or authorization is reviewed.

Found reviewing #180, which made it easy to reach. The shape predates that change — `restricted` sits in the same place and has the same consequence — so this is not a regression. ## What happens `crates/ephyra-db/src/query/names.rs` picks the row that stands for a name before narrowing, and narrows afterwards: ```sql WHERE BaseItems.Type = ? AND BaseItems.CleanName IN (…) AND BaseItems.Id = (SELECT MIN(Same.Id) FROM BaseItems AS Same WHERE Same.Type = BaseItems.Type AND Same.PresentationUniqueKey IS BaseItems.PresentationUniqueKey) AND <the account's filters, and the mark> ``` `representative_of` reads the whole group, unfiltered. Upstream does it the other way round — `BaseItemRepository.ByName.cs:196-224` runs `TranslateQuery(innerQuery, context, outerQueryFilter)` first and only then groups by `PresentationUniqueKey` and takes `Min(Id)`, so the representative is chosen from **the rows that survived the filter**. ## Where it shows Two `MusicArtist` rows sharing a presentation key — the row a music library found and the row written beside the metadata, which is the case `names.rs:107-110` documents and the reason `representative_of` has a `MusicArtist` branch at all. - Mark the row that wins the pick, ask `/Artists?isFavorite=false`: upstream still lists the name, represented by the other row. We drop it. - Mark only the row that loses the pick, ask `/Artists?isFavorite=true`: upstream lists it. We answer nothing. The same holds for an account the rating limit or a blocked tag hides the representative from: upstream offers the name under the row it may see, we offer nothing. That half is older and harder to reach, because a filter that hides one row of a group usually hides the other too — a mark lands on exactly one row, which is what makes this easy. ## Where to start The conditions `restricted` and `marked_favorite` build are written against `BaseItems.`, so choosing the representative among the rows that pass them needs the same conditions over the group's alias — either inside the correlated subquery, or as a `NOT EXISTS` over a better-ranked row that also passes. Either way the account's filters have to be generated for a named table rather than for `BaseItems` alone, which is the whole of the work. ## How to know it is done A fixture with two rows of one presentation key — one reachable, one not, and one marked — answers the name under the row that survived, on `/Artists` and for an account with something hidden from it. `crates/ephyra-http/tests/by_name.rs` has no two rows sharing a key today, so the case has to be built. ### House rules Tests run under `cargo nextest`; upstream's source is the checkout in `upstream/` **at the pin**. Generated artifacts are regenerated, never edited. Anything touching the query builder or authorization is reviewed.

Claude added the

label

2026-08-22 18:18:35 +02:00

Claude referenced this issue

2026-08-22 18:18:35 +02:00

IsFavorite and IsLiked are ignored by every by-name listing, so Favorites shows the whole library #180

Claude commented

2026-08-22 19:48:45 +02:00

Author

Collaborator

Fixed in e538a53, with the review's nits in fe81cdf.

The narrowing moves into a Kept term and the group is taken over that, so the
account's own conditions stay written against BaseItems where they were and
the row that stands for a name is chosen from what survived — which is
upstream's order in GetItemValues.

crates/ephyra-http/tests/by_name.rs grows the case: a second MusicArtist
row sharing the first's presentation key and folded name, with an id that wins
the pick, and the mark on the row that loses it. It answers the name; without
the change it answers nothing.

The review executed the emitted SQL rather than reading it. Binding order holds
in the maximal shape — a restricted account with a rating limit, blocked tags,
an allowlist, a blocked unrated kind and a blocked library, an ancestor_id,
two top_parent_ids, isFavorite=true and a params-carrying sort: 43
placeholders, 43 parameters, each in its slot. The answer set can only grow and
only where intended: Kept is a subset of what the old query considered and
both representative rules are stable under a subset containing the old winner,
so a plain listing returns the same names and the same representative ids —
measured over 300 names, 30 of them two-row groups.

Two things worth recording. It also restores upstream's answer for a group
whose lowest-id row carries a CleanName outside the value set, which is the
same bug in another dress. And it costs 3–8% on the listing, because the
account's predicates are re-evaluated inside the correlated group — measured
over 4,000 films and 2,000 tracks.

Fixed in `e538a53`, with the review's nits in `fe81cdf`. The narrowing moves into a `Kept` term and the group is taken over that, so the account's own conditions stay written against `BaseItems` where they were and the row that stands for a name is chosen from what survived — which is upstream's order in `GetItemValues`. `crates/ephyra-http/tests/by_name.rs` grows the case: a second `MusicArtist` row sharing the first's presentation key and folded name, with an id that wins the pick, and the mark on the row that loses it. It answers the name; without the change it answers nothing. The review executed the emitted SQL rather than reading it. Binding order holds in the maximal shape — a restricted account with a rating limit, blocked tags, an allowlist, a blocked unrated kind and a blocked library, an `ancestor_id`, two `top_parent_ids`, `isFavorite=true` and a params-carrying sort: 43 placeholders, 43 parameters, each in its slot. The answer set can only grow and only where intended: `Kept` is a subset of what the old query considered and both representative rules are stable under a subset containing the old winner, so a plain listing returns the same names *and* the same representative ids — measured over 300 names, 30 of them two-row groups. Two things worth recording. It also restores upstream's answer for a group whose lowest-id row carries a `CleanName` outside the value set, which is the same bug in another dress. And it costs 3–8% on the listing, because the account's predicates are re-evaluated inside the correlated group — measured over 4,000 films and 2,000 tracks.

Claude closed this issue

2026-08-22 19:48:45 +02:00