Cold review of 572737a29. Findings taken; two are corrections to my own claims.
CORRECTION: "a regression this branch INTRODUCED" was wrong, and I verified the
reviewer's counter-claim against origin/main before accepting it. That handler
already included SongMetadata AND already routed Song there, so
GET /api/v1/playlists/{id}/items was ALREADY a live 500 for a null-Artists song.
This branch only made the same throw reachable on a second path. The record said
so twice; both are fixed, because a wrong explanation outlives a wrong line.
SWEEP: fixing one site left the mirror standing -- Playouts/Mapper.GetDisplayTitle
had the identical unguarded join on a path that also eager-loads SongMetadata, so
it too was live, feeding the playout guide, troubleshooting, media-item info and
channel states. Guarded, with a unit test; reverting it reproduces
ArgumentNullException. LibraryBrowseItemMapper already wrote `Artists ?? []`, so
the nullability was known in-tree and these sites were simply unswept. Filed #691
for the remaining SongVideoGenerator dereferences on the playback path.
Also: documented that the shared matrix is the RERUN predicate used as a superset
for playlists (the playlist write path rejects RemoteStream today); noted the
third, inert consumer ReplacePlaylistItemsHandler; added the new mechanisms to the
record's `mechanics:` field; and trimmed the record under the 60-line prose
ceiling -- it was tipping the corpus p90 above the ceiling and reddening the
calibration test in scripts/tests, which passes again.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
timothyandClaude Opus 5
34eee753b2