fix(api): improve media response reliability

docs/api/methods.md

@@ -756,6 +756,7 @@ All parameters are optional. When called with no parameters, returns root entrie
 | relativePath | string   | No       | Relative path from root directory. Present on `media` entries and logical single-game container `directory` entries on zip-as-directory platforms. |
 | tags         | object[] | No       | Tags attached to the media. Each object has `tag` (string) and `type` (string). Present on `media` entries and logical single-game container `directory` entries on zip-as-directory platforms. |
 | disambiguatingTags | object[] | No | Subset of `tags` whose values differ across same-named siblings of this title, ordered by display importance. Same object shape as `tags`. Omitted when the title has nothing to disambiguate. |
+| hasCover     | boolean  | Yes      | Whether media-level or title-level image properties are available. Meaningful for media-capable entries; clients can skip image requests when false. |
 
 ##### Browse pagination object
 
@@ -794,6 +795,7 @@ All parameters are optional. When called with no parameters, returns root entrie
         "path": "/roms/SNES",
         "type": "root",
         "fileCount": 150,
+        "hasCover": false,
         "systemId": "SNES",
         "systemIds": ["SNES"]
       }
@@ -832,14 +834,16 @@ All parameters are optional. When called with no parameters, returns root entrie
         "name": "RPGs",
         "path": "/roms/SNES/RPGs",
         "type": "directory",
-        "fileCount": 42
+        "fileCount": 42,
+        "hasCover": false
       },
       {
         "mediaId": 42,
         "name": "Super Mario World",
         "path": "/roms/SNES/Super Mario World.sfc",
         "type": "media",
         "systemId": "SNES",
+        "hasCover": true,
         "zapScript": "@SNES/Super Mario World",
         "relativePath": "Super Mario World.sfc",
         "tags": [
@@ -853,6 +857,7 @@ All parameters are optional. When called with no parameters, returns root entrie
         "path": "/roms/SNES/The Legend of Zelda - A Link to the Past.sfc",
         "type": "media",
         "systemId": "SNES",
+        "hasCover": false,
         "zapScript": "@SNES/The Legend of Zelda - A Link to the Past",
         "relativePath": "The Legend of Zelda - A Link to the Past.sfc",
         "tags": [

pkg/api/methods/media.go

@@ -1015,6 +1015,9 @@ func HandleMediaSearch(env requests.RequestEnv) (any, error) { //nolint:gocritic
 
 	coverStarted := time.Now()
 	coverStatuses := make(map[int64]bool)
+	// Unknown must remain true in the response so clients do not suppress a
+	// valid image request merely because optional enrichment timed out.
+	coverStatusesKnown := false
 	if len(searchResults) > 0 {
 		coverRefs := make([]database.MediaCoverRef, len(searchResults))
 		for i := range searchResults {
@@ -1030,6 +1033,7 @@ func HandleMediaSearch(env requests.RequestEnv) (any, error) { //nolint:gocritic
 			log.Debug().Err(coverErr).Msg("could not enrich media search cover status")
 		} else {
 			coverStatuses = resolvedCoverStatuses
+			coverStatusesKnown = true
 		}
 	}
 	coverDuration := time.Since(coverStarted)
@@ -1073,10 +1077,14 @@ func HandleMediaSearch(env requests.RequestEnv) (any, error) { //nolint:gocritic
 		}
 		relativePathDuration += time.Since(stageStarted)
 
+		hasCover := true
+		if coverStatusesKnown {
+			hasCover = coverStatuses[result.MediaID]
+		}
 		results = append(results, models.SearchResultMedia{
 			MediaID:            result.MediaID,
 			RelPath:            relPath,
-			HasCover:           coverStatuses[result.MediaID],
+			HasCover:           hasCover,
 			System:             resultSystem,
 			Name:               result.Name,
 			Path:               result.Path,

pkg/api/methods/media_browse.go

@@ -395,15 +395,25 @@ func dedupeSystemRootEntries(entries []models.BrowseEntry) []models.BrowseEntry
 		return entries
 	}
 
-	filtered := make([]models.BrowseEntry, 0, len(entries))
-	for i := range entries {
-		if systemRootEntryCoveredByDescendant(entries, i) {
-			continue
+	// Route candidates can contain several ancestor levels. One pass removes an
+	// intermediate route, but a grandparent evaluated against the original set
+	// double-counts both that intermediate subtree and its leaf routes. Repeat on
+	// the reduced set until stable so every covered ancestor is removed while
+	// preserving parents with genuinely unmatched direct media.
+	current := entries
+	for {
+		filtered := make([]models.BrowseEntry, 0, len(current))
+		for i := range current {
+			if systemRootEntryCoveredByDescendant(current, i) {
+				continue
+			}
+			filtered = append(filtered, current[i])
 		}
-		filtered = append(filtered, entries[i])
+		if len(filtered) == len(current) {
+			return filtered
+		}
+		current = filtered
 	}
-
-	return filtered
 }
 
 func systemRootEntryCoveredByDescendant(entries []models.BrowseEntry, parentIdx int) bool {

pkg/api/methods/media_browse_test.go

@@ -1057,6 +1057,19 @@ func TestDedupeSystemRootEntries(t *testing.T) {
 			},
 			want: []string{path("media", "fat", "games", "NES"), path("media", "fat", "games", "NES Hacks")},
 		},
+		{
+			name: "grandparent absorbed by deeper descendants",
+			entries: []models.BrowseEntry{
+				{Path: path("media", "fat", "games", "MegaDrive"), FileCount: count(3)},
+				{Path: path("media", "fat", "games", "Genesis"), FileCount: count(8387)},
+				{Path: path("media", "fat", "games"), FileCount: count(8390)},
+				{Path: path("media", "fat"), FileCount: count(8390)},
+			},
+			want: []string{
+				path("media", "fat", "games", "MegaDrive"),
+				path("media", "fat", "games", "Genesis"),
+			},
+		},
 		{
 			name: "parent retained when descendants do not cover count",
 			entries: []models.BrowseEntry{

pkg/api/methods/media_history.go

@@ -106,6 +106,9 @@ func HandleMediaHistory(env requests.RequestEnv) (any, error) { //nolint:gocriti
 	enrichStarted := time.Now()
 	mediaIDs := make(map[mediaPathRef]int64)
 	coverStatuses := make(map[int64]bool)
+	// Unknown must remain true in the response so clients do not suppress a
+	// valid image request merely because optional enrichment timed out.
+	coverStatusesKnown := false
 	enrichCtx, cancelEnrichment := optionalDBEnrichmentContext(env.Context)
 	defer cancelEnrichment()
 
@@ -132,15 +135,17 @@ func HandleMediaHistory(env requests.RequestEnv) (any, error) { //nolint:gocriti
 			})
 		}
 
-		resolvedCoverStatuses := make(map[int64]bool)
-		if len(coverRefs) > 0 {
-			resolvedCoverStatuses, enrichErr = env.Database.MediaDB.GetMediaCoverStatus(enrichCtx, coverRefs)
-		}
-		if enrichErr != nil {
-			log.Debug().Err(enrichErr).Msg("could not enrich media history cover status")
+		mediaIDs = resolvedMediaIDs
+		if len(coverRefs) == 0 {
+			coverStatusesKnown = true
 		} else {
-			mediaIDs = resolvedMediaIDs
-			coverStatuses = resolvedCoverStatuses
+			resolvedCoverStatuses, coverErr := env.Database.MediaDB.GetMediaCoverStatus(enrichCtx, coverRefs)
+			if coverErr != nil {
+				log.Debug().Err(coverErr).Msg("could not enrich media history cover status")
+			} else {
+				coverStatuses = resolvedCoverStatuses
+				coverStatusesKnown = true
+			}
 		}
 	}
 	enrichElapsed := time.Since(enrichStarted)
@@ -158,11 +163,15 @@ func HandleMediaHistory(env requests.RequestEnv) (any, error) { //nolint:gocriti
 			endedAt = &formatted
 		}
 		mediaID := mediaIDs[ref]
+		hasCover := true
+		if coverStatusesKnown {
+			hasCover = coverStatuses[mediaID]
+		}
 
 		responseEntries = append(responseEntries, models.MediaHistoryResponseEntry{
 			MediaID:    mediaID,
 			RelPath:    mediaResponseRelativePath(&env, entry.SystemID, entry.MediaPath),
-			HasCover:   coverStatuses[mediaID],
+			HasCover:   hasCover,
 			SystemID:   entry.SystemID,
 			SystemName: entry.SystemName,
 			MediaName:  entry.MediaName,

pkg/api/methods/media_history_test.go

@@ -203,8 +203,9 @@ func TestHandleMediaHistory_EnrichmentFailuresAreNonFatal(t *testing.T) {
 	}
 
 	tests := []struct {
-		setup func(*testing.T, *helpers.MockMediaDBI)
-		name  string
+		setup           func(*testing.T, *helpers.MockMediaDBI)
+		name            string
+		expectedMediaID int64
 	}{
 		{
 			name: "media identity lookup",
@@ -214,7 +215,8 @@ func TestHandleMediaHistory_EnrichmentFailuresAreNonFatal(t *testing.T) {
 			},
 		},
 		{
-			name: "cover lookup",
+			name:            "cover lookup",
+			expectedMediaID: 42,
 			setup: func(t *testing.T, mockMediaDB *helpers.MockMediaDBI) {
 				t.Helper()
 				mockMediaDB.On("FindMediaIDsByPaths", mock.Anything, []string{mediaPath}).
@@ -252,8 +254,8 @@ func TestHandleMediaHistory_EnrichmentFailuresAreNonFatal(t *testing.T) {
 			response, ok := result.(models.MediaHistoryResponse)
 			require.True(t, ok)
 			require.Len(t, response.Entries, 1)
-			assert.Zero(t, response.Entries[0].MediaID)
-			assert.False(t, response.Entries[0].HasCover)
+			assert.Equal(t, tt.expectedMediaID, response.Entries[0].MediaID)
+			assert.True(t, response.Entries[0].HasCover)
 			mockUserDB.AssertExpectations(t)
 			mockMediaDB.AssertExpectations(t)
 		})

pkg/api/methods/media_image.go

@@ -38,6 +38,7 @@ import (
 	"sort"
 	"strings"
 	"sync/atomic"
+	"time"
 
 	"github.com/KarpelesLab/gowebp"
 	"github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models"
@@ -899,11 +900,29 @@ func validateMediaImageDelivery(delivery string, ref mediaRefParam) error {
 
 // HandleMediaImage returns a single best-match image inline or as a transient,
 // Core-owned cached thumbnail path when explicitly requested.
-func HandleMediaImage(env requests.RequestEnv) (any, error) { //nolint:gocritic // single-use parameter in API handler
+//
+//nolint:gocritic // RequestEnv is copied once at the API handler boundary.
+func HandleMediaImage(env requests.RequestEnv) (result any, resultErr error) {
+	started := time.Now()
+	deliveryForLog := ""
+	maxSizeForLog := 0
+	hasMediaID := false
+	defer func() {
+		log.Debug().
+			Dur("duration", time.Since(started)).
+			Str("delivery", deliveryForLog).
+			Int("maxSize", maxSizeForLog).
+			Bool("mediaId", hasMediaID).
+			Bool("ok", resultErr == nil).
+			Msg("media.image handler timing")
+	}()
+
 	ref, delivery, err := parseMediaImageRequest(env.Params)
 	if err != nil {
 		return nil, err
 	}
+	deliveryForLog = delivery
+	hasMediaID = ref.MediaID != nil
 	if deliveryErr := validateMediaImageDelivery(delivery, ref); deliveryErr != nil {
 		return nil, deliveryErr
 	}
@@ -915,6 +934,7 @@ func HandleMediaImage(env requests.RequestEnv) (any, error) { //nolint:gocritic
 	if ref.MaxSize != nil {
 		snapped := snapThumbMaxSize(*ref.MaxSize)
 		ref.MaxSize = &snapped
+		maxSizeForLog = int(snapped)
 	}
 	prefs := imagePrefs(nil, ref.ImageTypes)
 

pkg/api/methods/media_image_test.go

@@ -32,6 +32,7 @@ import (
 	"image/png"
 	"os"
 	"path/filepath"
+	"strings"
 	"syscall"
 	"testing"
 	"time"
@@ -44,6 +45,8 @@ import (
 	"github.com/ZaparooProject/zaparoo-core/v2/pkg/service/state"
 	testhelpers "github.com/ZaparooProject/zaparoo-core/v2/pkg/testing/helpers"
 	"github.com/ZaparooProject/zaparoo-core/v2/pkg/testing/mocks"
+	"github.com/rs/zerolog"
+	"github.com/rs/zerolog/log"
 	"github.com/spf13/afero"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
@@ -73,6 +76,29 @@ func makeMediaImageEnv(
 	}
 }
 
+func captureHandlerLogEvent(t *testing.T, message string, run func()) map[string]any {
+	t.Helper()
+	var buf bytes.Buffer
+	originalLogger := log.Logger
+	log.Logger = zerolog.New(&buf).Level(zerolog.DebugLevel)
+	defer func() { log.Logger = originalLogger }()
+
+	run()
+	for line := range strings.SplitSeq(strings.TrimSpace(buf.String()), "\n") {
+		var event map[string]any
+		decoder := json.NewDecoder(strings.NewReader(line))
+		decoder.UseNumber()
+		if err := decoder.Decode(&event); err != nil {
+			continue
+		}
+		if event["message"] == message {
+			return event
+		}
+	}
+	t.Fatalf("log event %q not found in %q", message, buf.String())
+	return nil
+}
+
 func makeMediaFullRow(mediaDBID, titleDBID int64) *database.MediaFullRow {
 	return &database.MediaFullRow{
 		Media:  database.Media{DBID: mediaDBID, Path: filepath.Join("games", fmt.Sprintf("test-%d.rom", mediaDBID))},
@@ -648,6 +674,56 @@ func TestImageHasTransparency(t *testing.T) {
 	}
 }
 
+func TestHandleMediaImage_TimingLog(t *testing.T) {
+	// Not parallel: swaps process-wide logger.
+	mediaImageNoImages.clear()
+	t.Cleanup(mediaImageNoImages.clear)
+
+	t.Run("success", func(t *testing.T) {
+		var imageData bytes.Buffer
+		require.NoError(t, png.Encode(&imageData, image.NewRGBA(image.Rect(0, 0, 1, 1))))
+
+		mockDB := testhelpers.NewMockMediaDBI()
+		row := makeMediaFullRow(9200, 9210)
+		mockDB.On("GetMediaWithTitleAndSystemByIDs", mock.Anything, []int64{row.DBID}).
+			Return(map[int64]database.MediaFullRow{row.DBID: *row}, nil)
+		mockDB.On("GetMediaProperties", mock.Anything, row.DBID).
+			Return([]database.MediaProperty{}, nil)
+		mockDB.On("GetMediaTitleProperties", mock.Anything, row.Title.DBID).
+			Return([]database.MediaProperty{
+				{TypeTag: "property:image-boxart", ContentType: "image/png", Binary: imageData.Bytes()},
+			}, nil)
+
+		env := makeMediaImageEnv(t, mockDB, json.RawMessage(
+			`{"mediaId":9200,"delivery":"inline","maxSize":300}`))
+		event := captureHandlerLogEvent(t, "media.image handler timing", func() {
+			_, err := HandleMediaImage(env)
+			require.NoError(t, err)
+		})
+
+		assert.Equal(t, true, event["ok"])
+		assert.Equal(t, mediaImageDeliveryInline, event["delivery"])
+		assert.Equal(t, true, event["mediaId"])
+		assert.Equal(t, json.Number("512"), event["maxSize"])
+		assert.Contains(t, event, "duration")
+		mockDB.AssertExpectations(t)
+	})
+
+	t.Run("validation error", func(t *testing.T) {
+		env := makeMediaImageEnv(t, testhelpers.NewMockMediaDBI(), json.RawMessage(
+			`{"mediaId":9201,"delivery":"localPath"}`))
+		event := captureHandlerLogEvent(t, "media.image handler timing", func() {
+			_, err := HandleMediaImage(env)
+			require.Error(t, err)
+		})
+
+		assert.Equal(t, false, event["ok"])
+		assert.Equal(t, mediaImageDeliveryPath, event["delivery"])
+		assert.Equal(t, true, event["mediaId"])
+		assert.Contains(t, event, "duration")
+	})
+}
+
 func TestHandleMediaImage_MaxSizeResizesAndCachesThumbnail(t *testing.T) {
 	// Not parallel: installs the process-wide thumb cache pointer.
 	fs := afero.NewMemMapFs()