docs/plans/2026-05-21-musicbrainz-provider.md

@@ -75,13 +75,13 @@ Implement a MusicBrainz API provider with strict 1 request/second rate limiting,
 - [x] run tests - must pass before next task
 
 ### Task 4: Implement database integration and sync orchestration
-- [ ] create `internal/musicbrainz/sync.go` with SyncArtistDiscography function
-- [ ] implement upsert logic: INSERT OR REPLACE into external_releases table
-- [ ] add cached_at column to external_releases table via migration
-- [ ] implement context.Context support for cancellation
-- [ ] write tests for database upsert operations
-- [ ] write integration tests with in-memory SQLite
-- [ ] run tests - must pass before next task
+- [x] create `internal/musicbrainz/sync.go` with SyncArtistDiscography function
+- [x] implement upsert logic: INSERT OR REPLACE into external_releases table
+- [x] add cached_at column to external_releases table via migration (done in Task 3 as migration 005)
+- [x] implement context.Context support for cancellation
+- [x] write tests for database upsert operations
+- [x] write integration tests with in-memory SQLite
+- [x] run tests - must pass before next task
 
 ### Task 5: Wire up provider in application entry point
 - [ ] update `cmd/naviwatcher/main.go` to initialize MusicBrainz client

internal/musicbrainz/sync.go

@@ -0,0 +1,130 @@
+package musicbrainz
+
+import (
+	"context"
+	"fmt"
+	"time"
+
+	"naviwatcher/internal/database"
+)
+
+// SyncArtistDiscography synchronizes an artist's discography from MusicBrainz
+// into the local external_releases table. It follows this flow:
+//  1. Check if cached data exists and is within TTL.
+//  2. If cache hit, return the cached releases immediately.
+//  3. If cache miss or expired, fetch release groups from MusicBrainz API.
+//  4. Apply status and type filtering.
+//  5. Upsert each filtered release group into external_releases with current timestamp.
+//  6. Return the list of external releases.
+//
+// Context cancellation is checked before the API call and between each upsert
+// to allow graceful interruption.
+func SyncArtistDiscography(
+	ctx context.Context,
+	client *MusicBrainzClient,
+	db *database.DB,
+	artistMBID string,
+	ttl time.Duration,
+) ([]database.ExternalRelease, error) {
+	// Check context before starting.
+	if err := ctx.Err(); err != nil {
+		return nil, fmt.Errorf("sync artist discography: %w", err)
+	}
+
+	// Step 1: Check cache.
+	cached, err := GetCachedReleases(db, artistMBID, ttl)
+	if err != nil {
+		return nil, fmt.Errorf("sync artist discography: cache check failed: %w", err)
+	}
+
+	// Step 2: If we have cached data, return it.
+	if cached.CacheHitCount > 0 {
+		return database.GetExternalReleasesByArtistWithCache(db, artistMBID, ttl)
+	}
+
+	// Step 3: Cache miss — fetch from MusicBrainz API.
+	groups, err := client.GetArtistReleaseGroups(ctx, artistMBID)
+	if err != nil {
+		return nil, fmt.Errorf("sync artist discography: fetch release groups: %w", err)
+	}
+
+	// Step 4: Apply filtering.
+	filtered := FilterReleaseGroups(groups)
+
+	// Step 5: Upsert each release group into the database.
+	now := time.Now()
+	var releases []database.ExternalRelease
+	for _, rg := range filtered {
+		// Check context cancellation between each upsert.
+		if err := ctx.Err(); err != nil {
+			return nil, fmt.Errorf("sync artist discography: %w", err)
+		}
+
+		ext := rg.ToExternalRelease()
+		ext.CachedAt = now
+
+		if err := database.SaveExternalRelease(db, ext); err != nil {
+			return nil, fmt.Errorf("sync artist discography: save release %s: %w", rg.ID, err)
+		}
+
+		releases = append(releases, *ext)
+	}
+
+	return releases, nil
+}
+
+// SyncArtistDiscographyWithFilter works like SyncArtistDiscography but applies
+// per-artist type filtering preferences in addition to the base filters.
+func SyncArtistDiscographyWithFilter(
+	ctx context.Context,
+	client *MusicBrainzClient,
+	db *database.DB,
+	artistMBID string,
+	ttl time.Duration,
+	artistFilter *ArtistTypeFilter,
+) ([]database.ExternalRelease, error) {
+	// Check context before starting.
+	if err := ctx.Err(); err != nil {
+		return nil, fmt.Errorf("sync artist discography with filter: %w", err)
+	}
+
+	// Step 1: Check cache.
+	cached, err := GetCachedReleases(db, artistMBID, ttl)
+	if err != nil {
+		return nil, fmt.Errorf("sync artist discography with filter: cache check failed: %w", err)
+	}
+
+	// Step 2: If we have cached data, return it.
+	if cached.CacheHitCount > 0 {
+		return database.GetExternalReleasesByArtistWithCache(db, artistMBID, ttl)
+	}
+
+	// Step 3: Cache miss — fetch from MusicBrainz API.
+	groups, err := client.GetArtistReleaseGroups(ctx, artistMBID)
+	if err != nil {
+		return nil, fmt.Errorf("sync artist discography with filter: fetch release groups: %w", err)
+	}
+
+	// Step 4: Apply filtering with artist-specific type preferences.
+	filtered := FilterReleaseGroupsWithArtistFilter(groups, artistFilter)
+
+	// Step 5: Upsert each release group into the database.
+	now := time.Now()
+	var releases []database.ExternalRelease
+	for _, rg := range filtered {
+		if err := ctx.Err(); err != nil {
+			return nil, fmt.Errorf("sync artist discography with filter: %w", err)
+		}
+
+		ext := rg.ToExternalRelease()
+		ext.CachedAt = now
+
+		if err := database.SaveExternalRelease(db, ext); err != nil {
+			return nil, fmt.Errorf("sync artist discography with filter: save release %s: %w", rg.ID, err)
+		}
+
+		releases = append(releases, *ext)
+	}
+
+	return releases, nil
+}

internal/musicbrainz/sync_test.go

@@ -0,0 +1,798 @@
+package musicbrainz
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	"golang.org/x/time/rate"
+	"naviwatcher/internal/config"
+	"naviwatcher/internal/database"
+)
+
+// mbReleaseGroupXML is a helper to build a single release-group XML element.
+func mbReleaseGroupXML(id, title, rgType, status, artistID, artistName, releaseDate string) string {
+	statusAttr := ""
+	if status != "" {
+		statusAttr = ` status="` + status + `"`
+	}
+	return `<release-group id="` + id + `" type="` + rgType + `"` + statusAttr + `>` +
+		`<title>` + title + `</title>` +
+		`<artist-credit><name-credit><artist id="` + artistID + `">` +
+		`<name>` + artistName + `</name>` +
+		`</artist></name-credit></artist-credit>` +
+		`<first-release-date>` + releaseDate + `</first-release-date>` +
+		`</release-group>`
+}
+
+// mbReleaseGroupListResponse builds a full MusicBrainz XML response for a release-group list.
+func mbReleaseGroupListResponse(groups string, count int) string {
+	return `<?xml version="1.0" encoding="UTF-8"?>
+<metadata xmlns="http://musicbrainz.org/ns/mmd-2.0#">
+  <release-group-list count="` + itoa(count) + `">` +
+		groups +
+		`</release-group-list>
+</metadata>`
+}
+
+// itoa converts an int to a string without importing strconv.
+func itoa(n int) string {
+	if n == 0 {
+		return "0"
+	}
+	var buf [20]byte
+	i := len(buf)
+	for n > 0 {
+		i--
+		buf[i] = byte('0' + n%10)
+		n /= 10
+	}
+	return string(buf[i:])
+}
+
+// newTestMBServer creates a mock MusicBrainz HTTP server.
+func newTestMBServer(handler http.HandlerFunc) *httptest.Server {
+	return httptest.NewServer(handler)
+}
+
+// newTestDB creates an in-memory SQLite database with all migrations applied.
+func newTestDB(t *testing.T) *database.DB {
+	t.Helper()
+	db, err := database.New(":memory:")
+	if err != nil {
+		t.Fatalf("database.New() error: %v", err)
+	}
+	return db
+}
+
+// newTestClient creates a MusicBrainzClient pointing at the given test server
+// with a relaxed rate limiter (100 req/sec) for fast test execution.
+func newTestClient(serverURL string) *MusicBrainzClient {
+	cfg := config.MusicBrainzConfig{
+		UserAgent: "test-agent/1.0",
+	}
+	return &MusicBrainzClient{
+		httpClient: &http.Client{},
+		userAgent:  cfg.UserAgent,
+		baseURL:    serverURL + "/ws/2",
+		rateLimiter: rate.NewLimiter(rate.Limit(100), 100),
+	}
+}
+
+// seedArtist inserts a minimal artist_settings row so foreign key constraints pass.
+func seedArtist(t *testing.T, db *database.DB, id, name string) {
+	t.Helper()
+	if err := database.SaveArtistSettings(db, &database.ArtistSettings{
+		ID:        id,
+		Name:      name,
+		Monitored: true,
+	}); err != nil {
+		t.Fatalf("seedArtist(%s) error: %v", id, err)
+	}
+}
+
+// -----------------------------------------------------------------------
+// Test: SyncArtistDiscography fetches from API and upserts on cache miss
+// -----------------------------------------------------------------------
+
+func TestSyncArtistDiscography_CacheMiss_FetchesAndUpserts(t *testing.T) {
+	artistMBID := "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"
+	artistName := "Test Artist"
+
+	server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/xml")
+		resp := mbReleaseGroupListResponse(
+			mbReleaseGroupXML("rg1", "First Album", "Album", "", artistMBID, artistName, "2020-01-01")+
+				mbReleaseGroupXML("rg2", "Second Album", "Album", "", artistMBID, artistName, "2022-06-15")+
+				mbReleaseGroupXML("rg3", "A Single", "Single", "", artistMBID, artistName, "2021-03-10"),
+			3,
+		)
+		w.Write([]byte(resp))
+	})
+	defer server.Close()
+
+	db := newTestDB(t)
+	defer db.Close()
+	seedArtist(t, db, artistMBID, artistName)
+
+	client := newTestClient(server.URL)
+	ctx := context.Background()
+	ttl := 24 * time.Hour
+
+	releases, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl)
+	if err != nil {
+		t.Fatalf("SyncArtistDiscography() error: %v", err)
+	}
+
+	if len(releases) != 3 {
+		t.Fatalf("expected 3 releases, got %d", len(releases))
+	}
+
+	// Verify each release has CachedAt set.
+	for _, r := range releases {
+		if r.CachedAt.IsZero() {
+			t.Errorf("release %s: CachedAt should be set, got zero", r.RGID)
+		}
+		if r.ArtistID != artistMBID {
+			t.Errorf("release %s: expected ArtistID %q, got %q", r.RGID, artistMBID, r.ArtistID)
+		}
+	}
+
+	// Verify data was persisted in the database.
+	stored, err := database.GetExternalReleasesByArtist(db, artistMBID)
+	if err != nil {
+		t.Fatalf("GetExternalReleasesByArtist() error: %v", err)
+	}
+	if len(stored) != 3 {
+		t.Errorf("expected 3 stored releases, got %d", len(stored))
+	}
+}
+
+// -----------------------------------------------------------------------
+// Test: SyncArtistDiscography returns cached data on cache hit
+// -----------------------------------------------------------------------
+
+func TestSyncArtistDiscography_CacheHit_ReturnsCached(t *testing.T) {
+	artistMBID := "bbbbbbbb-cccc-dddd-eeee-ffffffffffff"
+
+	db := newTestDB(t)
+	defer db.Close()
+	seedArtist(t, db, artistMBID, "Cached Artist")
+
+	// Pre-populate the cache with one release.
+	now := time.Now()
+	if err := database.SaveExternalRelease(db, &database.ExternalRelease{
+		RGID:        "rg-cached",
+		ArtistID:    artistMBID,
+		Title:       "Cached Album",
+		Type:        "Album",
+		ReleaseDate: "2019-05-01",
+		CachedAt:    now,
+	}); err != nil {
+		t.Fatalf("SaveExternalRelease() error: %v", err)
+	}
+
+	// Server that would be called on cache miss — should NOT be called.
+	serverCalled := false
+	server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) {
+		serverCalled = true
+		w.Header().Set("Content-Type", "application/xml")
+		w.Write([]byte(mbReleaseGroupListResponse("", 0)))
+	})
+	defer server.Close()
+
+	client := newTestClient(server.URL)
+	ctx := context.Background()
+	ttl := 24 * time.Hour
+
+	releases, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl)
+	if err != nil {
+		t.Fatalf("SyncArtistDiscography() error: %v", err)
+	}
+
+	if serverCalled {
+		t.Error("expected cache hit but server was called (cache miss)")
+	}
+
+	if len(releases) != 1 {
+		t.Fatalf("expected 1 cached release, got %d", len(releases))
+	}
+
+	if releases[0].RGID != "rg-cached" {
+		t.Errorf("expected RGID 'rg-cached', got %q", releases[0].RGID)
+	}
+	if releases[0].Title != "Cached Album" {
+		t.Errorf("expected Title 'Cached Album', got %q", releases[0].Title)
+	}
+}
+
+// -----------------------------------------------------------------------
+// Test: SyncArtistDiscography applies filtering (excluded statuses)
+// -----------------------------------------------------------------------
+
+func TestSyncArtistDiscography_FiltersExcludedStatuses(t *testing.T) {
+	artistMBID := "cccccccc-dddd-eeee-ffff-000000000000"
+
+	server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/xml")
+		// Include a Bootleg and a Promotion that should be filtered out.
+		resp := mbReleaseGroupListResponse(
+			mbReleaseGroupXML("rg-legit", "Legit Album", "Album", "", artistMBID, "Artist", "2020-01-01")+
+				mbReleaseGroupXML("rg-bootleg", "Bootleg Album", "Album", "Bootleg", artistMBID, "Artist", "2020-02-01")+
+				mbReleaseGroupXML("rg-promo", "Promo Album", "Album", "Promotion", artistMBID, "Artist", "2020-03-01")+
+				mbReleaseGroupXML("rg-pseudo", "Pseudo Album", "Album", "Pseudo-Release", artistMBID, "Artist", "2020-04-01"),
+			4,
+		)
+		w.Write([]byte(resp))
+	})
+	defer server.Close()
+
+	db := newTestDB(t)
+	defer db.Close()
+	seedArtist(t, db, artistMBID, "Filter Artist")
+
+	client := newTestClient(server.URL)
+	ctx := context.Background()
+	ttl := 24 * time.Hour
+
+	releases, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl)
+	if err != nil {
+		t.Fatalf("SyncArtistDiscography() error: %v", err)
+	}
+
+	// Only the legit album should remain after filtering.
+	if len(releases) != 1 {
+		t.Fatalf("expected 1 release after filtering, got %d", len(releases))
+	}
+	if releases[0].RGID != "rg-legit" {
+		t.Errorf("expected RGID 'rg-legit', got %q", releases[0].RGID)
+	}
+}
+
+// -----------------------------------------------------------------------
+// Test: SyncArtistDiscography applies type filtering
+// -----------------------------------------------------------------------
+
+func TestSyncArtistDiscography_FiltersExcludedTypes(t *testing.T) {
+	artistMBID := "dddddddd-eeee-ffff-0000-111111111111"
+
+	server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/xml")
+		resp := mbReleaseGroupListResponse(
+			mbReleaseGroupXML("rg-album", "An Album", "Album", "", artistMBID, "Artist", "2020-01-01")+
+				mbReleaseGroupXML("rg-single", "A Single", "Single", "", artistMBID, "Artist", "2020-02-01")+
+				mbReleaseGroupXML("rg-ep", "An EP", "EP", "", artistMBID, "Artist", "2020-03-01")+
+				mbReleaseGroupXML("rg-comp", "A Compilation", "Compilation", "", artistMBID, "Artist", "2020-04-01")+
+				mbReleaseGroupXML("rg-soundtrack", "A Soundtrack", "Soundtrack", "", artistMBID, "Artist", "2020-05-01"),
+			5,
+		)
+		w.Write([]byte(resp))
+	})
+	defer server.Close()
+
+	db := newTestDB(t)
+	defer db.Close()
+	seedArtist(t, db, artistMBID, "Type Filter Artist")
+
+	client := newTestClient(server.URL)
+	ctx := context.Background()
+	ttl := 24 * time.Hour
+
+	releases, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl)
+	if err != nil {
+		t.Fatalf("SyncArtistDiscography() error: %v", err)
+	}
+
+	// Soundtrack should be excluded (not in includedTypes).
+	if len(releases) != 4 {
+		t.Fatalf("expected 4 releases after type filtering, got %d", len(releases))
+	}
+
+	rgIDs := make(map[string]bool)
+	for _, r := range releases {
+		rgIDs[r.RGID] = true
+	}
+	if rgIDs["rg-soundtrack"] {
+		t.Error("Soundtrack type should have been filtered out")
+	}
+}
+
+// -----------------------------------------------------------------------
+// Test: SyncArtistDiscography with context cancellation
+// -----------------------------------------------------------------------
+
+func TestSyncArtistDiscography_ContextCancellation(t *testing.T) {
+	artistMBID := "eeeeeeee-ffff-0000-1111-222222222222"
+
+	server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/xml")
+		resp := mbReleaseGroupListResponse(
+			mbReleaseGroupXML("rg1", "Album One", "Album", "", artistMBID, "Artist", "2020-01-01"),
+			1,
+		)
+		w.Write([]byte(resp))
+	})
+	defer server.Close()
+
+	db := newTestDB(t)
+	defer db.Close()
+	seedArtist(t, db, artistMBID, "Cancel Artist")
+
+	client := newTestClient(server.URL)
+	ttl := 24 * time.Hour
+
+	// Create a context that is already cancelled.
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+
+	_, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl)
+	if err == nil {
+		t.Fatal("SyncArtistDiscography() expected error for cancelled context, got nil")
+	}
+}
+
+// -----------------------------------------------------------------------
+// Test: SyncArtistDiscography upsert is idempotent (re-sync replaces)
+// -----------------------------------------------------------------------
+
+func TestSyncArtistDiscography_IdempotentUpsert(t *testing.T) {
+	artistMBID := "ffffffff-0000-1111-2222-333333333333"
+
+	server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/xml")
+		resp := mbReleaseGroupListResponse(
+			mbReleaseGroupXML("rg1", "Album One", "Album", "", artistMBID, "Artist", "2020-01-01")+
+				mbReleaseGroupXML("rg2", "Album Two", "Album", "", artistMBID, "Artist", "2021-01-01"),
+			2,
+		)
+		w.Write([]byte(resp))
+	})
+	defer server.Close()
+
+	db := newTestDB(t)
+	defer db.Close()
+	seedArtist(t, db, artistMBID, "Idempotent Artist")
+
+	client := newTestClient(server.URL)
+	ctx := context.Background()
+	ttl := 24 * time.Hour
+
+	// First sync.
+	releases1, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl)
+	if err != nil {
+		t.Fatalf("first SyncArtistDiscography() error: %v", err)
+	}
+	if len(releases1) != 2 {
+		t.Fatalf("expected 2 releases after first sync, got %d", len(releases1))
+	}
+
+	// Second sync should use cache (server would error if called again).
+	// To verify the cache path, we use a very short TTL so cache expires.
+	releases2, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl)
+	if err != nil {
+		t.Fatalf("second SyncArtistDiscography() error: %v", err)
+	}
+	if len(releases2) != 2 {
+		t.Fatalf("expected 2 releases after second sync, got %d", len(releases2))
+	}
+
+	// Verify no duplicates in the database.
+	stored, err := database.GetExternalReleasesByArtist(db, artistMBID)
+	if err != nil {
+		t.Fatalf("GetExternalReleasesByArtist() error: %v", err)
+	}
+	if len(stored) != 2 {
+		t.Errorf("expected 2 stored releases (no duplicates), got %d", len(stored))
+	}
+}
+
+// -----------------------------------------------------------------------
+// Test: SyncArtistDiscographyWithFilter applies per-artist type filter
+// -----------------------------------------------------------------------
+
+func TestSyncArtistDiscographyWithFilter_ArtistTypeFilter(t *testing.T) {
+	artistMBID := "11111111-2222-3333-4444-555555555555"
+
+	server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/xml")
+		resp := mbReleaseGroupListResponse(
+			mbReleaseGroupXML("rg-album", "An Album", "Album", "", artistMBID, "Artist", "2020-01-01")+
+				mbReleaseGroupXML("rg-single", "A Single", "Single", "", artistMBID, "Artist", "2020-02-01")+
+				mbReleaseGroupXML("rg-ep", "An EP", "EP", "", artistMBID, "Artist", "2020-03-01")+
+				mbReleaseGroupXML("rg-comp", "A Compilation", "Compilation", "", artistMBID, "Artist", "2020-04-01"),
+			4,
+		)
+		w.Write([]byte(resp))
+	})
+	defer server.Close()
+
+	db := newTestDB(t)
+	defer db.Close()
+	seedArtist(t, db, artistMBID, "Filter Artist")
+
+	client := newTestClient(server.URL)
+	ctx := context.Background()
+	ttl := 24 * time.Hour
+
+	// Filter that excludes Singles and Compilations.
+	filter := &ArtistTypeFilter{
+		ArtistID:            artistMBID,
+		IncludeSingles:      false,
+		IncludeCompilations: false,
+		IncludeEP:           true,
+	}
+
+	releases, err := SyncArtistDiscographyWithFilter(ctx, client, db, artistMBID, ttl, filter)
+	if err != nil {
+		t.Fatalf("SyncArtistDiscographyWithFilter() error: %v", err)
+	}
+
+	// Should only have Album and EP.
+	if len(releases) != 2 {
+		t.Fatalf("expected 2 releases with artist filter, got %d", len(releases))
+	}
+
+	rgIDs := make(map[string]bool)
+	for _, r := range releases {
+		rgIDs[r.RGID] = true
+	}
+	if !rgIDs["rg-album"] {
+		t.Error("expected rg-album in filtered results")
+	}
+	if !rgIDs["rg-ep"] {
+		t.Error("expected rg-ep in filtered results")
+	}
+	if rgIDs["rg-single"] {
+		t.Error("rg-single should have been filtered out")
+	}
+	if rgIDs["rg-comp"] {
+		t.Error("rg-comp should have been filtered out")
+	}
+}
+
+// -----------------------------------------------------------------------
+// Test: SyncArtistDiscographyWithFilter cache hit
+// -----------------------------------------------------------------------
+
+func TestSyncArtistDiscographyWithFilter_CacheHit(t *testing.T) {
+	artistMBID := "22222222-3333-4444-5555-666666666666"
+
+	db := newTestDB(t)
+	defer db.Close()
+	seedArtist(t, db, artistMBID, "Cache Hit Filter Artist")
+
+	// Pre-populate cache.
+	if err := database.SaveExternalRelease(db, &database.ExternalRelease{
+		RGID:     "rg-old",
+		ArtistID: artistMBID,
+		Title:    "Old Cached",
+		Type:     "Album",
+		CachedAt: time.Now(),
+	}); err != nil {
+		t.Fatalf("SaveExternalRelease() error: %v", err)
+	}
+
+	serverCalled := false
+	server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) {
+		serverCalled = true
+		w.Header().Set("Content-Type", "application/xml")
+		w.Write([]byte(mbReleaseGroupListResponse("", 0)))
+	})
+	defer server.Close()
+
+	client := newTestClient(server.URL)
+	ctx := context.Background()
+	ttl := 24 * time.Hour
+	filter := DefaultArtistTypeFilter(artistMBID)
+
+	releases, err := SyncArtistDiscographyWithFilter(ctx, client, db, artistMBID, ttl, filter)
+	if err != nil {
+		t.Fatalf("SyncArtistDiscographyWithFilter() error: %v", err)
+	}
+
+	if serverCalled {
+		t.Error("expected cache hit but server was called")
+	}
+	if len(releases) != 1 {
+		t.Fatalf("expected 1 cached release, got %d", len(releases))
+	}
+}
+
+// -----------------------------------------------------------------------
+// Test: SyncArtistDiscography with empty response (no release groups)
+// -----------------------------------------------------------------------
+
+func TestSyncArtistDiscography_EmptyResponse(t *testing.T) {
+	artistMBID := "33333333-4444-5555-6666-777777777777"
+
+	server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/xml")
+		w.Write([]byte(mbReleaseGroupListResponse("", 0)))
+	})
+	defer server.Close()
+
+	db := newTestDB(t)
+	defer db.Close()
+	seedArtist(t, db, artistMBID, "Empty Artist")
+
+	client := newTestClient(server.URL)
+	ctx := context.Background()
+	ttl := 24 * time.Hour
+
+	releases, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl)
+	if err != nil {
+		t.Fatalf("SyncArtistDiscography() error: %v", err)
+	}
+
+	if len(releases) != 0 {
+		t.Fatalf("expected 0 releases for empty response, got %d", len(releases))
+	}
+
+	// Verify nothing in DB.
+	stored, err := database.GetExternalReleasesByArtist(db, artistMBID)
+	if err != nil {
+		t.Fatalf("GetExternalReleasesByArtist() error: %v", err)
+	}
+	if len(stored) != 0 {
+		t.Errorf("expected 0 stored releases, got %d", len(stored))
+	}
+}
+
+// -----------------------------------------------------------------------
+// Test: SyncArtistDiscography API error propagation
+// -----------------------------------------------------------------------
+
+func TestSyncArtistDiscography_APIError(t *testing.T) {
+	artistMBID := "44444444-5555-6666-7777-888888888888"
+
+	server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError)
+		w.Write([]byte("internal server error"))
+	})
+	defer server.Close()
+
+	db := newTestDB(t)
+	defer db.Close()
+	seedArtist(t, db, artistMBID, "Error Artist")
+
+	client := newTestClient(server.URL)
+	ctx := context.Background()
+	ttl := 24 * time.Hour
+
+	_, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl)
+	if err == nil {
+		t.Fatal("SyncArtistDiscography() expected error for API failure, got nil")
+	}
+}
+
+// -----------------------------------------------------------------------
+// Test: SyncArtistDiscographyWithFilter context cancellation
+// -----------------------------------------------------------------------
+
+func TestSyncArtistDiscographyWithFilter_ContextCancellation(t *testing.T) {
+	artistMBID := "55555555-6666-7777-8888-999999999999"
+
+	server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/xml")
+		w.Write([]byte(mbReleaseGroupListResponse(
+			mbReleaseGroupXML("rg1", "Album", "Album", "", artistMBID, "Artist", "2020-01-01"),
+			1,
+		)))
+	})
+	defer server.Close()
+
+	db := newTestDB(t)
+	defer db.Close()
+	seedArtist(t, db, artistMBID, "Cancel Filter Artist")
+
+	client := newTestClient(server.URL)
+	ttl := 24 * time.Hour
+	filter := DefaultArtistTypeFilter(artistMBID)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+
+	_, err := SyncArtistDiscographyWithFilter(ctx, client, db, artistMBID, ttl, filter)
+	if err == nil {
+		t.Fatal("SyncArtistDiscographyWithFilter() expected error for cancelled context, got nil")
+	}
+}
+
+// -----------------------------------------------------------------------
+// Test: Verify XML parsing integration — ensure the full pipeline works
+// with real XML structure matching MusicBrainz responses.
+// -----------------------------------------------------------------------
+
+func TestSyncArtistDiscography_FullXMLPipeline(t *testing.T) {
+	artistMBID := "66666666-7777-8888-9999-000000000000"
+
+	// Build a realistic MusicBrainz XML response.
+	xmlBody := `<?xml version="1.0" encoding="UTF-8"?>
+<metadata>
+  <release-group-list count="2">
+    <release-group id="rg-real-1" type="Album">
+      <title>Real Album One</title>
+      <artist-credit>
+        <name-credit>
+          <artist id="` + artistMBID + `">
+            <name>Real Artist</name>
+          </artist>
+        </name-credit>
+      </artist-credit>
+      <first-release-date>2019-03-15</first-release-date>
+    </release-group>
+    <release-group id="rg-real-2" type="Single">
+      <title>Real Single Two</title>
+      <artist-credit>
+        <name-credit>
+          <artist id="` + artistMBID + `">
+            <name>Real Artist</name>
+          </artist>
+        </name-credit>
+      </artist-credit>
+      <first-release-date>2020-07-20</first-release-date>
+    </release-group>
+  </release-group-list>
+</metadata>`
+
+	server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/xml")
+		w.Write([]byte(xmlBody))
+	})
+	defer server.Close()
+
+	db := newTestDB(t)
+	defer db.Close()
+	seedArtist(t, db, artistMBID, "Real Artist")
+
+	client := newTestClient(server.URL)
+	ctx := context.Background()
+	ttl := 24 * time.Hour
+
+	releases, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl)
+	if err != nil {
+		t.Fatalf("SyncArtistDiscography() error: %v", err)
+	}
+
+	if len(releases) != 2 {
+		t.Fatalf("expected 2 releases, got %d", len(releases))
+	}
+
+	// Verify the parsed data.
+	byID := make(map[string]database.ExternalRelease)
+	for _, r := range releases {
+		byID[r.RGID] = r
+	}
+
+	rg1, ok := byID["rg-real-1"]
+	if !ok {
+		t.Fatal("expected rg-real-1 in results")
+	}
+	if rg1.Title != "Real Album One" {
+		t.Errorf("rg-real-1 title = %q, want %q", rg1.Title, "Real Album One")
+	}
+	if rg1.Type != "Album" {
+		t.Errorf("rg-real-1 type = %q, want %q", rg1.Type, "Album")
+	}
+	if rg1.ReleaseDate != "2019-03-15" {
+		t.Errorf("rg-real-1 release_date = %q, want %q", rg1.ReleaseDate, "2019-03-15")
+	}
+
+	rg2, ok := byID["rg-real-2"]
+	if !ok {
+		t.Fatal("expected rg-real-2 in results")
+	}
+	if rg2.Type != "Single" {
+		t.Errorf("rg-real-2 type = %q, want %q", rg2.Type, "Single")
+	}
+}
+
+// -----------------------------------------------------------------------
+// Test: Verify CachedAt timestamps are consistent across a sync batch
+// -----------------------------------------------------------------------
+
+func TestSyncArtistDiscography_ConsistentCachedAtTimestamp(t *testing.T) {
+	artistMBID := "77777777-8888-9999-0000-111111111111"
+
+	server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/xml")
+		resp := mbReleaseGroupListResponse(
+			mbReleaseGroupXML("rg-ts-1", "Album A", "Album", "", artistMBID, "Artist", "2020-01-01")+
+				mbReleaseGroupXML("rg-ts-2", "Album B", "Album", "", artistMBID, "Artist", "2021-01-01")+
+				mbReleaseGroupXML("rg-ts-3", "Album C", "Album", "", artistMBID, "Artist", "2022-01-01"),
+			3,
+		)
+		w.Write([]byte(resp))
+	})
+	defer server.Close()
+
+	db := newTestDB(t)
+	defer db.Close()
+	seedArtist(t, db, artistMBID, "Timestamp Artist")
+
+	client := newTestClient(server.URL)
+	ctx := context.Background()
+	ttl := 24 * time.Hour
+
+	releases, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl)
+	if err != nil {
+		t.Fatalf("SyncArtistDiscography() error: %v", err)
+	}
+
+	// All releases in a batch should have the same CachedAt timestamp.
+	if len(releases) != 3 {
+		t.Fatalf("expected 3 releases, got %d", len(releases))
+	}
+
+	first := releases[0].CachedAt
+	for _, r := range releases[1:] {
+		if !r.CachedAt.Equal(first) {
+			t.Errorf("CachedAt mismatch: %v vs %v for release %s", first, r.CachedAt, r.RGID)
+		}
+	}
+}
+
+// -----------------------------------------------------------------------
+// Test: Verify XML unmarshalling edge case — release-group with no type attr
+// -----------------------------------------------------------------------
+
+func TestSyncArtistDiscography_XMLNoTypeAttribute(t *testing.T) {
+	artistMBID := "88888888-9999-0000-1111-222222222222"
+
+	xmlBody := `<?xml version="1.0" encoding="UTF-8"?>
+<metadata>
+  <release-group-list count="2">
+    <release-group id="rg-notype" type="">
+      <title>No Type Album</title>
+      <artist-credit>
+        <name-credit>
+          <artist id="` + artistMBID + `">
+            <name>Artist</name>
+          </artist>
+        </name-credit>
+      </artist-credit>
+      <first-release-date>2020-01-01</first-release-date>
+    </release-group>
+    <release-group id="rg-withtype" type="Album">
+      <title>Typed Album</title>
+      <artist-credit>
+        <name-credit>
+          <artist id="` + artistMBID + `">
+            <name>Artist</name>
+          </artist>
+        </name-credit>
+      </artist-credit>
+      <first-release-date>2021-01-01</first-release-date>
+    </release-group>
+  </release-group-list>
+</metadata>`
+
+	server := newTestMBServer(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/xml")
+		w.Write([]byte(xmlBody))
+	})
+	defer server.Close()
+
+	db := newTestDB(t)
+	defer db.Close()
+	seedArtist(t, db, artistMBID, "No Type Artist")
+
+	client := newTestClient(server.URL)
+	ctx := context.Background()
+	ttl := 24 * time.Hour
+
+	releases, err := SyncArtistDiscography(ctx, client, db, artistMBID, ttl)
+	if err != nil {
+		t.Fatalf("SyncArtistDiscography() error: %v", err)
+	}
+
+	// The release-group with no type should be filtered out (empty string is not in includedTypes).
+	if len(releases) != 1 {
+		t.Fatalf("expected 1 release (empty type filtered), got %d", len(releases))
+	}
+	if releases[0].RGID != "rg-withtype" {
+		t.Errorf("expected rg-withtype, got %s", releases[0].RGID)
+	}
+}