ancestors() cannot give the order it was documented to give #161

New issue

Closed

opened 2026-08-20 21:46:42 +02:00 by Claude · 2 comments

Claude commented

2026-08-20 21:46:42 +02:00

Collaborator

ItemReader::ancestors promised nearest first. ANCESTORS in
crates/ephyra-db/src/items/statements.rs recovers it from the shape of the
table:

ORDER BY COUNT(Above.ParentItemId) DESC, AncestorIds.ParentItemId ASC

Against a real upstream database that does not order the chain. Upstream files
a CollectionFolder under the UserRootFolder alone and the directory it
points at under the AggregateFolder alone, so for anything inside a library
both carry exactly one ancestor of their own and the tie falls to whichever id
sorts lower. Run over reference/snapshots/default, for a film:

8155D293-… | 1 | MediaBrowser.Controller.Entities.Folder
C8CA023C-… | 1 | MediaBrowser.Controller.Entities.CollectionFolder
F27CAA37-… | 0 | MediaBrowser.Controller.Entities.AggregateFolder

The directory comes first here by hash and not by depth. The comments now say
that instead of promising an order (6684877), which is the honest half; this
is the other half.

Why nothing is broken today

Every caller names the type it wants rather than taking the first:
latest.rs::nearest_ancestor filters for MusicAlbum or PhotoAlbum,
collections.rs::library_folders filters for a collection folder, and
views.rs uses .any. The next order-sensitive caller is the one that
inherits the problem.

Where to start

The chain's order is in the parent pointers, not in AncestorIds — upstream
walks GetParents() for it and denormalizes the set without the order. So the
question is whether a caller that genuinely needs the chain gets a walk of
ParentId (one lookup per level, on a path that is a single query today), or
whether naming the type is the contract and ancestors should return a set
rather than a sequence and say so in its own signature.

Filed rather than built because no caller wants an order yet, and picking the
walk before one does buys a per-level query nobody reads.

House rules

Tests run under cargo nextest; upstream's source is the checkout in
upstream/ at the pin. The reference on :8096 is read-only.

`ItemReader::ancestors` promised nearest first. `ANCESTORS` in `crates/ephyra-db/src/items/statements.rs` recovers it from the shape of the table: ```sql ORDER BY COUNT(Above.ParentItemId) DESC, AncestorIds.ParentItemId ASC ``` Against a real upstream database that does not order the chain. Upstream files a `CollectionFolder` under the `UserRootFolder` alone and the directory it points at under the `AggregateFolder` alone, so for anything inside a library both carry exactly one ancestor of their own and the tie falls to whichever id sorts lower. Run over `reference/snapshots/default`, for a film: ``` 8155D293-… | 1 | MediaBrowser.Controller.Entities.Folder C8CA023C-… | 1 | MediaBrowser.Controller.Entities.CollectionFolder F27CAA37-… | 0 | MediaBrowser.Controller.Entities.AggregateFolder ``` The directory comes first here by hash and not by depth. The comments now say that instead of promising an order (`6684877`), which is the honest half; this is the other half. ## Why nothing is broken today Every caller names the type it wants rather than taking the first: `latest.rs::nearest_ancestor` filters for `MusicAlbum` or `PhotoAlbum`, `collections.rs::library_folders` filters for a collection folder, and `views.rs` uses `.any`. The next order-sensitive caller is the one that inherits the problem. ## Where to start The chain's order is in the parent pointers, not in `AncestorIds` — upstream walks `GetParents()` for it and denormalizes the set without the order. So the question is whether a caller that genuinely needs the chain gets a walk of `ParentId` (one lookup per level, on a path that is a single query today), or whether naming the type is the contract and `ancestors` should return a set rather than a sequence and say so in its own signature. Filed rather than built because no caller wants an order yet, and picking the walk before one does buys a per-level query nobody reads. ### House rules Tests run under `cargo nextest`; upstream's source is the checkout in `upstream/` **at the pin**. The reference on `:8096` is read-only.

Claude added the

correctness

labels

2026-08-20 21:46:46 +02:00

Claude commented

2026-08-20 22:04:00 +02:00

Author

Collaborator

Fixed in 8db3232, and the answer turned out to be both halves rather than a
choice between them: the two questions are different questions.

Measured against reference/snapshots/default, for a film:

parent chain (ParentId):  Folder → AggregateFolder
AncestorIds:              Folder, CollectionFolder, AggregateFolder

The CollectionFolder is not in the chain at all — a library is not above the
directory on disk, it covers it. So AncestorIds is two structures in one
table and there is no nearest in it to recover; the heuristic was answering by
whichever id sorted lower.

ancestors stays what the membership callers want and now says it is a set,
ordered by id so two reads of one database agree.
parent_chain is new: a recursive walk of ParentId, nearest first, in one
query. That is what upstream's GetParents() walks, and FindParent<T> and
Photo.AlbumEntity both take the first match from it.

latest.rs::container_of asks the second one now. Checked that the album is
still reached: a track's chain on the reference is MusicAlbum → MusicArtist → Folder → root, so the album is at depth 0 rather than found by luck — and the
nested-photo-album case the set could never answer now has an answer.

what_an_item_is_filed_under_comes_back_as_a_set and
the_parents_come_back_nearest_first replace ancestors_come_back_nearest_first,
which asserted the promise this issue is about. Both fail if the ordering is
reversed.

Fixed in `8db3232`, and the answer turned out to be both halves rather than a choice between them: the two questions are different questions. Measured against `reference/snapshots/default`, for a film: ``` parent chain (ParentId): Folder → AggregateFolder AncestorIds: Folder, CollectionFolder, AggregateFolder ``` The `CollectionFolder` is not in the chain at all — a library is not above the directory on disk, it covers it. So `AncestorIds` is two structures in one table and there is no nearest in it to recover; the heuristic was answering by whichever id sorted lower. - `ancestors` stays what the membership callers want and now says it is a set, ordered by id so two reads of one database agree. - `parent_chain` is new: a recursive walk of `ParentId`, nearest first, in one query. That is what upstream's `GetParents()` walks, and `FindParent<T>` and `Photo.AlbumEntity` both take the first match from it. `latest.rs::container_of` asks the second one now. Checked that the album is still reached: a track's chain on the reference is `MusicAlbum → MusicArtist → Folder → root`, so the album is at depth 0 rather than found by luck — and the nested-photo-album case the set could never answer now has an answer. `what_an_item_is_filed_under_comes_back_as_a_set` and `the_parents_come_back_nearest_first` replace `ancestors_come_back_nearest_first`, which asserted the promise this issue is about. Both fail if the ordering is reversed.

Claude closed this issue

2026-08-20 22:04:00 +02:00

Claude commented

2026-08-20 22:21:23 +02:00

Author

Collaborator

A review of the fix caught that PARENT_CHAIN had no bound, fixed in
4fb618d.

UNION ALL carries an incrementing depth, so nothing dedups the recursion
away: a row that is its own parent, or a pair that name each other, is a walk
that never comes back — a pinned pool connection and growing memory, where
upstream's lazy GetParents() enumerable would return on the first match. The
foreign key on the parent column says the row exists, not that it is above this
one, so neither shape is refused by the schema.

Bounded at 64 climbs, with a_parent_that_points_at_itself_does_not_climb_forever
over a fixture row pointed at itself.

The route at /Items/{id}/Ancestors walks the same column in Rust
(detail.rs::ancestors, while let Some(id) = next) and is unbounded for the
same reason. It predates this work and is left alone here.

A review of the fix caught that `PARENT_CHAIN` had no bound, fixed in `4fb618d`. `UNION ALL` carries an incrementing depth, so nothing dedups the recursion away: a row that is its own parent, or a pair that name each other, is a walk that never comes back — a pinned pool connection and growing memory, where upstream's lazy `GetParents()` enumerable would return on the first match. The foreign key on the parent column says the row exists, not that it is above this one, so neither shape is refused by the schema. Bounded at 64 climbs, with `a_parent_that_points_at_itself_does_not_climb_forever` over a fixture row pointed at itself. The route at `/Items/{id}/Ancestors` walks the same column in Rust (`detail.rs::ancestors`, `while let Some(id) = next`) and is unbounded for the same reason. It predates this work and is left alone here.