docs: the history phrases the id sweep missed, restated as present-tense facts or traps (#6277)
release / govulncheck (push) Successful in 19s
release / web (push) Successful in 1m14s
release / go (push) Successful in 1m43s
release / integration (push) Successful in 5m18s
release / android (push) Successful in 5m33s
release / Build signed APK (releases and dev) (push) Successful in 5m40s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 25s
release / Verify release artifacts (tag releases only) (push) Skipped

A wider pass for "previous", "before the fix", "we shipped" and the
like. The chi double-subtree, uppercase Vorbis key and stopPropagation
comments now describe the trap they guard; commit hashes, CI run
numbers and a deleted file name are gone; uuidString's comment says
what it returns.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-23 12:43:11 -04:00
co-authored by Claude Opus 5.5
parent 10d50dccbd
commit 9f588b6d61
12 changed files with 32 additions and 39 deletions
+2 -2
View File
@@ -34,8 +34,8 @@ func TestClientIP(t *testing.T) {
},
{
// The common case: one TLS-terminating proxy. Note RemoteAddr is
// PUBLIC here — a proxy on its own host — which the previous
// private-range heuristic got wrong.
// PUBLIC here — a proxy on its own host — which a private-range
// heuristic would get wrong.
name: "one proxy on a public address yields the client",
hops: 1,
remoteAddr: "203.0.113.200:40000",
+1 -1
View File
@@ -362,7 +362,7 @@ func (e *Enricher) RetryAlbum(ctx context.Context, albumID pgtype.UUID) error {
return e.EnrichAlbum(ctx, albumID)
}
// uuidString carries over from the previous enricher.
// uuidString renders a UUID as text for logs, or "<nil>" when it is not set.
func uuidString(u pgtype.UUID) string {
if !u.Valid {
return "<nil>"
+1 -1
View File
@@ -334,7 +334,7 @@ func TestExtractGenres_EndToEnd(t *testing.T) {
if err != nil {
t.Fatalf("tag.ReadFrom: %v", err)
}
// Confirm the upstream behaviour this fix exists for is still present —
// Confirm the upstream behaviour this reader works around is still present —
// if dhowden ever fixes it, this test tells us the workaround can go.
if welded := meta.Genre(); welded != "Alternative RockRock" {
t.Logf("note: dhowden/tag no longer welds multi-values (got %q)", welded)
+3 -4
View File
@@ -97,10 +97,9 @@ func TestExtractMBIDs_MissingTagsReturnEmpty(t *testing.T) {
}
func TestExtractMBIDs_VorbisUppercaseKeysMiss(t *testing.T) {
// Regression guard: real dhowden never produces uppercase Vorbis
// keys. If a future change starts looking up "MUSICBRAINZ_ALBUMID"
// directly, this test must keep returning empty so we don't
// silently re-introduce the v0 bug where every album skipped.
// Real dhowden never produces uppercase Vorbis keys. Looking up
// "MUSICBRAINZ_ALBUMID" directly would find nothing, and every album
// would be skipped; this test keeps that lookup returning empty.
m := stubMeta{
format: tag.VORBIS,
raw: map[string]interface{}{
+2 -2
View File
@@ -168,8 +168,8 @@ func TestCrossReleaseLinkNeedsAutoResolve_Integration(t *testing.T) {
// A merge keeps the removed copy's link to the song on another release,
// whichever copy's id names the song. That copy has no song_id of its own,
// and before the fix its link was lost whenever the random ids made it the
// key (the flaky failure in run 8891).
// and its link must survive whichever way the random ids fall, including when
// they make it the key.
func TestMergeInheritsSongLink_Integration(t *testing.T) {
for _, removeAlbumCut := range []bool{true, false} {
pool := newPool(t)
+1 -1
View File
@@ -170,7 +170,7 @@ func TestExtractGenres_FLACRepeatedFields(t *testing.T) {
if err != nil {
t.Fatalf("tag.ReadFrom: %v", err)
}
// Confirm the upstream behaviour this fix exists for is still present.
// Confirm the upstream behaviour this reader works around is still present.
if got := meta.Genre(); got != "Instrumental Hip Hop" {
t.Logf("note: dhowden/tag no longer keeps only the last GENRE (got %q)", got)
}
+2 -3
View File
@@ -221,9 +221,8 @@ func (s *Scheduler) reconcile(ctx context.Context) {
}
// runStartupCatchUp fires BuildSystemPlaylists once for every active
// user at startup, regardless of scheduled time. Matches the
// behavior of the previous system_cron.go startup runOnce so users
// whose scheduled fire was missed during downtime get caught up.
// user at startup, regardless of scheduled time, so users whose
// scheduled fire was missed during downtime get caught up.
func (s *Scheduler) runStartupCatchUp(ctx context.Context, users []dbq.ListActiveUsersWithTimezonesRow) {
for _, u := range users {
s.rebuildUserDaily(ctx, u.ID, time.Now())
+7 -9
View File
@@ -150,13 +150,12 @@ func TestRouter_RestPathNotSwallowedBySPA(t *testing.T) {
}
}
// TestRouter_AdminSubtreeNotShadowed is a regression test for a real bug we
// shipped in 06a1fe1 and didn't catch until M6a: r.Route("/api/admin", ...)
// in server.go was creating a second chi subtree at the same prefix as the
// nested admin Route inside api.Mount. chi.Walk shows both subtrees as
// registered, but runtime routing dispatch only matches one — every admin
// endpoint registered by api.Mount (summary, quarantine, etc.)
// silently 404'd for authenticated callers in production.
// TestRouter_AdminSubtreeNotShadowed guards the trap of a second
// r.Route("/api/admin", ...) in server.go: it creates a chi subtree at the
// same prefix as the nested admin Route inside api.Mount. chi.Walk shows both
// subtrees as registered, but runtime routing dispatch only matches one, so
// every admin endpoint registered by api.Mount (summary, quarantine, etc.)
// would silently 404 for authenticated callers.
//
// chi.Walk enumeration would NOT have caught this (both branches are in
// the tree). The only reliable check is to make a real authenticated
@@ -214,8 +213,7 @@ func TestRouter_AdminSubtreeNotShadowed(t *testing.T) {
// Each path is registered by a different package: admin/summary from
// api.Mount and admin/scan from server.Router. If chi runtime dispatch
// only routes one /api/admin subtree, one of these returns 404 — that
// IS the bug we shipped in 06a1fe1.
// only routes one /api/admin subtree, one of these returns 404.
cases := []struct {
method string
path string
+2 -2
View File
@@ -3,8 +3,8 @@
// part goes through library.DeleteTrackFile — the one path that deletes a
// track file.
//
// The web track-kebab entry that called this was removed in f7278f24; the
// endpoint was kept deliberately for a safer admin surface to rebind.
// No client calls the endpoint today; it is kept for an admin surface to
// rebind.
package tracks
import (
@@ -70,15 +70,12 @@ describe('DiscoverResultCard', () => {
expect(onRequest).not.toHaveBeenCalled();
});
// NOTE: previous versions of this file asserted .badge-row { min-height: 22px
// } and .actions { margin-top: auto } via getComputedStyle, which only worked
// because the component shipped inline style="" attributes — a layering
// violation. Stylesheet introspection (document.styleSheets) was tried as a
// replacement, but vitest's @testing-library/svelte renderer does not inject
// the component's scoped <style> block into jsdom (styleSheets.length === 0).
// Asserting "CSS exists in CSS" has weak value anyway, so the layout
// discipline (reserved badge slot + bottom-anchored actions) is enforced
// visually at the consumer page rather than as a unit test here.
// The layout discipline (reserved badge slot + bottom-anchored actions) is
// not asserted here: vitest's @testing-library/svelte renderer does not
// inject the component's scoped <style> block into jsdom
// (styleSheets.length === 0), so getComputedStyle sees nothing, and
// asserting "CSS exists in CSS" has weak value anyway. It is checked
// visually at the consumer page.
test('renders <img> when imageUrl is set', () => {
const { container } = render(DiscoverResultCard, {
@@ -210,10 +210,10 @@ describe('SuggestionFeed snooze (#2375)', () => {
expect(screen.getByText(/back in 14 days/i)).toBeInTheDocument();
});
// Pins the days→months seam. The singular branch was originally dead code
// here too: the threshold (45) sat above the divisor (30), so no day count
// could round to one month without hitting the days branch first. Kept in
// lockstep with SuggestionSnoozeRefTest on Android.
// Pins the days→months seam: with a threshold above the divisor (45 vs 30),
// no day count could round to one month without hitting the days branch
// first, and the singular branch would be dead code. Kept in lockstep with
// SuggestionSnoozeRefTest on Android.
test('the days-to-months boundary sits at 30 days, so "a month" is reachable', () => {
setSuggestions([]);
setSnoozes([
+1 -1
View File
@@ -2,7 +2,7 @@
* A shuffled copy of `items` (Fisher–Yates). The input is not touched.
*
* The one copy for every Shuffle control: the album and playlist headers
* and the artist card's play button each had their own before this.
* and the artist card's play button.
*/
export function shuffled<T>(items: readonly T[]): T[] {
const arr = items.slice();