feat(api): speed up Core media browsing - #1239
Merged
Merged
Conversation
Let clients request Core-owned cached thumbnail paths while preserving inline delivery as the default. Validate returned cache artifacts and fall back to inline data when materialization fails. Inspired by Giancarlo Erra's MiSTer artwork performance work and measurements in ZaparooProject/zaparoo-frontend#360. Ref ZaparooProject/zaparoo-frontend#360 Co-authored-by: Giancarlo Erra <giancarlo@widescreen.studio>
Let clients request newest-session-per-media history directly from Core while preserving existing paginated history behavior. Group by system and media path before cursor filtering so older sessions cannot reappear on later pages. Inspired by Giancarlo Erra's recents performance work and measurements in ZaparooProject/zaparoo-frontend#360. Ref ZaparooProject/zaparoo-frontend#360 Co-authored-by: Giancarlo Erra <giancarlo@widescreen.studio>
Correlate queue, handler, response, marshal, payload, encryption, and HTTP timing logs by request ID. This makes target-device API latency attributable without changing response contracts. Motivated by MiSTer performance measurements from ZaparooProject/zaparoo-frontend#360. Ref ZaparooProject/zaparoo-frontend#360
Build an exact in-memory index of media and title artwork properties while retaining a bounded title-first SQL fallback during cold start. Invalidate the index on image-property and database lifecycle changes so Core can serve cover status without exposing its schema. Inspired by Giancarlo Erra's MiSTer artwork and browse performance work and measurements in ZaparooProject/zaparoo-frontend#360. Ref ZaparooProject/zaparoo-frontend#360 Co-authored-by: Giancarlo Erra <giancarlo@widescreen.studio>
Return an always-present hasCover flag from media.search and media.history using Core's batched media/title artwork lookup. This lets clients plan artwork requests without reading Core-owned database tables. Inspired by Giancarlo Erra's artwork, browse, and recents performance work in ZaparooProject/zaparoo-frontend#360. Ref ZaparooProject/zaparoo-frontend#360 Co-authored-by: Giancarlo Erra <giancarlo@widescreen.studio>
Measure semaphore wait, database search, cover lookup, response construction, system metadata, ZapScript, and relative-path stages independently. This makes remaining target-device search latency attributable without changing API behavior. Motivated by MiSTer performance measurements from ZaparooProject/zaparoo-frontend#360. Ref ZaparooProject/zaparoo-frontend#360
Carry title IDs through search results and derive disambiguating ZapScript tags from media-scope rows already returned by the full tag query. Keep a small-page tag preflight while avoiding a redundant query for larger tagged result sets. Developed while attributing MiSTer search latency reported through ZaparooProject/zaparoo-frontend#360. Ref ZaparooProject/zaparoo-frontend#360
Update existing scoped and multi-variant search mocks for the MediaTitleDBID column and shared tag lookup introduced by search enrichment.
Stream default-order media rows by DBID and filter large cached title candidate sets in memory, stopping once the page fills. Cache scoped media bounds, support up to four systems, and fall back to grouped SQL when a bounded window is sparse or the request is unsupported. Developed from target-device latency analysis prompted by ZaparooProject/zaparoo-frontend#360. Ref ZaparooProject/zaparoo-frontend#360
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughThe API now reports cover availability, supports distinct media history and local-path image delivery, and adds transport timing diagnostics. The database adds cover-status caching, large-candidate search paths, title-aware lookups, and distinct history pagination. ChangesMedia API and database enhancements
Estimated code review effort: 5 (Critical) | ~120 minutes Mergeability Score: 🟡 Moderate · up to The PR adds request and search timing behavior, but focused tests for the new transport and queue diagnostics are still missing, leaving a bounded risk of unnoticed regressions. Merge should wait for those tests or explicit owner acceptance. Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (3)
pkg/api/methods/media_image.go (1)
861-884: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueReuse the resolved path instead of scanning the cache twice.
cachedMediaImageResponsecallslookupPath, and the inline branch then callsread, which repeats the same format scan and stat calls. Read the bytes from the already-resolvedpathinstead.afero.ReadFilereturning an error still yields a cache miss, so the deletion-between-lookup-and-read behavior is unchanged.♻️ Proposed refactor
if localPath { if response, ok := localMediaImageResponse(cache, path, contentType, typeTag); ok { return response, true } } - data, contentType, found := cache.read(ref, system, typeTag, maxSize) - if !found { + //nolint:gosec // lookupPath only returns controlled cache paths. + data, err := afero.ReadFile(cache.fs, path) + if err != nil { return models.MediaImageResponse{}, false } return inlineMediaImageResponse(data, contentType, sourcePath, typeTag), true🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/api/methods/media_image.go` around lines 861 - 884, Update cachedMediaImageResponse to read inline response bytes directly from the already-resolved path returned by cache.lookupPath, rather than calling cache.read and repeating the cache scan. Use afero.ReadFile with the cache filesystem, return a cache miss when reading fails, and preserve the existing localMediaImageResponse path and inlineMediaImageResponse behavior.pkg/database/database.go (1)
251-254: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument the key semantics of
GetMediaCoverStatusandMediaCoverRef.The returned map is keyed by
MediaDBID, and a media entry is reported as covered when either the media-level or title-level image property exists. Neighboring declarations in this file carry doc comments. Add a short comment so callers do not have to readmediadbto learn the key and the fallback rule. Also state whether absent keys mean "no cover".Also applies to: 1048-1048
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/database/database.go` around lines 251 - 254, Add concise Go doc comments for GetMediaCoverStatus and MediaCoverRef describing that results are keyed by MediaDBID, coverage is true when either the media-level or title-level image exists, and absent keys mean no cover. Follow the surrounding declaration comment style.pkg/database/mediadb/mediadb.go (1)
2486-2520: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winA bounds-query error fails the search instead of using the grouped SQL path.
If
db.getMediaSearchBoundsreturns an error,streamErris set,strategystays empty, and control reachesif streamErr != nil && strategy == "", which returns the error. The grouped SQL path below does not need the bounds and would still serve the request. Treat a bounds lookup failure as "streaming unavailable" and fall through, so a transient error on the bounds query does not failmedia.search.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/database/mediadb/mediadb.go` around lines 2486 - 2520, Update the system-scoped branch around getMediaSearchBounds so a bounds lookup error disables the streaming strategy without leaving streamErr set for the final failure check. Preserve the grouped SQL fallback by allowing execution to continue with strategy empty, while retaining existing error propagation for failures from the grouped search itself.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/api/methods.md`:
- Line 560: Update both media.search response examples’ results objects to
include the required hasCover field, using a representative boolean value such
as false, consistent with the SearchResultMedia.HasCover JSON output and the
documented table.
In `@pkg/api/methods/media_history.go`:
- Around line 107-136: Update the media-history enrichment flow around
resolveMediaPathIDs and GetMediaCoverStatus to use the existing
optionalDBEnrichmentContext timeout and treat all lookup failures as non-fatal.
Preserve the history response by retaining zero-value media IDs and cover
statuses (MediaID 0 and HasCover false) when either media.db operation fails,
rather than returning an error from the handler; keep successful enrichment
unchanged.
Apply the same fix in `@pkg/api/methods/media.go` around lines 1026 - 1029: Covers
the equivalent whole-request failure in media.search when cover-status
enrichment fails.
Apply the same fix in `@pkg/api/methods/media_history_test.go` around lines 157 -
194: Covers the missing regression test for history enrichment failures.
In `@pkg/api/methods/media_image_test.go`:
- Around line 176-178: Update the symlink setup in the test around
cache.isSafeLocalPath to detect os.Symlink errors indicating unsupported
symlinks or insufficient privileges, skip the test in those cases, and continue
failing on unexpected errors. Keep the existing symlink safety assertion
unchanged when creation succeeds.
In `@pkg/api/server.go`:
- Around line 469-489: Add focused tests for the new transport diagnostics: in
pkg/api/server.go ranges 469-489, 501-514, 529-542, 1338-1350, 1373-1385, and
1482-1531, verify WebSocket success/failure and plaintext/encrypted HTTP
result/error timing fields; in pkg/api/ws_dispatcher.go ranges 60-87 and
208-410, verify zero/nonzero queue timestamps and request, response, and pong
queue metadata. Use the relevant logging and dispatcher symbols to assert
emitted fields and error behavior.
In `@pkg/database/mediadb/media_search_scope_test.go`:
- Around line 310-313: Use require.Equal for the result-count comparison before
each indexed comparison loop in the affected tests, including the matching
pattern around the second comparison. Keep the per-item assert.Equal checks
unchanged so mismatched lengths fail cleanly before expected[i] can panic.
In `@pkg/database/mediadb/mediadb.go`:
- Around line 278-317: Refactor getMediaSearchBounds so the SQL MIN/MAX query
never runs while mediaSearchBoundsMu is held and concurrent misses are coalesced
per systemDBID rather than globally. Keep cache reads and writes under the
mutex, and before storing query results re-check the relevant invalidation
generation or dirty state so results from an invalidated query are not cached;
preserve existing return and error behavior.
In `@pkg/database/mediadb/sql_browse.go`:
- Around line 1307-1345: Update queryImagePropertyEntityIDs to split entityIDs
into chunks of sqliteMaxParams minus len(imageTagIDs) before querying, while
preserving the existing scope-specific SQL and argument ordering. Execute each
chunk, merge all returned IDs into one result map, and retain early returns and
scope validation. Add a regression test covering a maximum-sized page with one
image tag.
In `@pkg/database/userdb/media_history.go`:
- Around line 499-518: Add a database migration creating a composite index on
MediaHistory covering SystemID, MediaPath, and DBID, so the LatestMedia GROUP BY
query can avoid scanning and temporary grouping structures. Follow existing
migration conventions and ensure the migration is safely applied and reversible
if supported.
---
Nitpick comments:
In `@pkg/api/methods/media_image.go`:
- Around line 861-884: Update cachedMediaImageResponse to read inline response
bytes directly from the already-resolved path returned by cache.lookupPath,
rather than calling cache.read and repeating the cache scan. Use afero.ReadFile
with the cache filesystem, return a cache miss when reading fails, and preserve
the existing localMediaImageResponse path and inlineMediaImageResponse behavior.
In `@pkg/database/database.go`:
- Around line 251-254: Add concise Go doc comments for GetMediaCoverStatus and
MediaCoverRef describing that results are keyed by MediaDBID, coverage is true
when either the media-level or title-level image exists, and absent keys mean no
cover. Follow the surrounding declaration comment style.
In `@pkg/database/mediadb/mediadb.go`:
- Around line 2486-2520: Update the system-scoped branch around
getMediaSearchBounds so a bounds lookup error disables the streaming strategy
without leaving streamErr set for the final failure check. Preserve the grouped
SQL fallback by allowing execution to continue with strategy empty, while
retaining existing error propagation for failures from the grouped search
itself.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1e26308f-ea5e-4e78-928a-ba770a094278
📒 Files selected for processing (32)
docs/api/methods.mdpkg/api/methods/media.gopkg/api/methods/media_history.gopkg/api/methods/media_history_test.gopkg/api/methods/media_image.gopkg/api/methods/media_image_test.gopkg/api/methods/media_response_helpers.gopkg/api/methods/media_scrape_test.gopkg/api/methods/media_search_test.gopkg/api/models/params.gopkg/api/models/responses.gopkg/api/request_priority.gopkg/api/request_priority_test.gopkg/api/server.gopkg/api/ws_dispatcher.gopkg/database/database.gopkg/database/mediadb/disambiguation_test.gopkg/database/mediadb/media_search_scope_test.gopkg/database/mediadb/mediadb.gopkg/database/mediadb/multi_variant_search_test.gopkg/database/mediadb/sql_browse.gopkg/database/mediadb/sql_browse_test.gopkg/database/mediadb/sql_scraper.gopkg/database/mediadb/sql_scraper_test.gopkg/database/mediadb/sql_search.gopkg/database/mediadb/sql_search_test.gopkg/database/mediadb/sql_test.gopkg/database/userdb/media_history.gopkg/database/userdb/media_history_property_test.gopkg/testing/helpers/db_mocks.gopkg/ui/tui/mock_api_client_test.gopkg/ui/tui/settings_service.go
Comment on lines
+469
to
+489
| func logWebSocketTransportTiming( | ||
| id models.RPCID, | ||
| responseType string, | ||
| encrypted bool, | ||
| responseBytes int, | ||
| marshalDuration time.Duration, | ||
| writeDuration time.Duration, | ||
| writeErr error, | ||
| ) { | ||
| event := log.Debug(). | ||
| Str("requestId", requestIDForLog(id)). | ||
| Str("responseType", responseType). | ||
| Bool("encrypted", encrypted). | ||
| Int("responseBytes", responseBytes). | ||
| Dur("marshalDuration", marshalDuration). | ||
| Dur("writeDuration", writeDuration) | ||
| if writeErr != nil { | ||
| event = event.Err(writeErr) | ||
| } | ||
| event.Msg("websocket response transport timing") | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add focused tests for the new transport diagnostics.
The changed tests cover request metadata and request-ID truncation. They do not cover the new WebSocket and HTTP timing fields or the dispatcher queue-duration fields.
pkg/api/server.go#L469-L489: Test the WebSocket timing event for successful and failed writes.pkg/api/server.go#L501-L514: Test plaintext result-response timing.pkg/api/server.go#L529-L542: Test plaintext error-response timing.pkg/api/server.go#L1338-L1350: Test encrypted result-response timing.pkg/api/server.go#L1373-L1385: Test encrypted error-response timing.pkg/api/server.go#L1482-L1531: Test HTTP result and error response timing.pkg/api/ws_dispatcher.go#L60-L87: Test zero and nonzero queue timestamps.pkg/api/ws_dispatcher.go#L208-L410: Test request, response, and pong queue metadata.
As per coding guidelines: “Write tests for all new code — see TESTING.md and pkg/testing/README.md.”
📍 Affects 2 files
pkg/api/server.go#L469-L489(this comment)pkg/api/server.go#L501-L514pkg/api/server.go#L529-L542pkg/api/server.go#L1338-L1350pkg/api/server.go#L1373-L1385pkg/api/server.go#L1482-L1531pkg/api/ws_dispatcher.go#L60-L87pkg/api/ws_dispatcher.go#L208-L410
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/api/server.go` around lines 469 - 489, Add focused tests for the new
transport diagnostics: in pkg/api/server.go ranges 469-489, 501-514, 529-542,
1338-1350, 1373-1385, and 1482-1531, verify WebSocket success/failure and
plaintext/encrypted HTTP result/error timing fields; in pkg/api/ws_dispatcher.go
ranges 60-87 and 208-410, verify zero/nonzero queue timestamps and request,
response, and pong queue metadata. Use the relevant logging and dispatcher
symbols to assert emitted fields and error behavior.
Source: Coding guidelines
Comment on lines
+499
to
+518
| // Group before applying the cursor. Filtering raw history rows first would | ||
| // let an older session for a media identity reappear on a later page. | ||
| //nolint:gosec // latestWhere contains only fixed SQL and placeholders. | ||
| query := fmt.Sprintf(` | ||
| WITH LatestMedia AS ( | ||
| SELECT MAX(DBID) AS DBID | ||
| FROM MediaHistory | ||
| %s | ||
| GROUP BY SystemID, MediaPath | ||
| ) | ||
| SELECT | ||
| history.DBID, history.StartTime, history.EndTime, | ||
| history.SystemID, history.SystemName, history.MediaPath, | ||
| history.MediaName, history.LauncherID, history.PlayTime | ||
| FROM MediaHistory AS history | ||
| INNER JOIN LatestMedia AS latest ON latest.DBID = history.DBID | ||
| WHERE history.DBID < ? | ||
| ORDER BY history.DBID DESC | ||
| LIMIT ?; | ||
| `, latestWhere) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect MediaHistory schema and indexes in the userdb migrations/schema.
rg -nP --type=go -C3 'CREATE (UNIQUE )?INDEX.*MediaHistory|CREATE TABLE MediaHistory'Repository: ZaparooProject/zaparoo-core
Length of output: 165
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files | rg -i '(^|/)(database|db|migration|migrations|schema)(/|$)|userdb|\.sql$' | head -200
printf '%s\n' '--- MediaHistory references ---'
rg -n -C3 'MediaHistory|media_history' .
printf '%s\n' '--- relevant index/table declarations ---'
rg -n -i -C3 'create[[:space:]]+(unique[[:space:]]+)?index|create[[:space:]]+table|index[[:space:]]+.*(systemid|mediapath|dbid)' . | rg -i 'MediaHistory|SystemID|MediaPath|DBID|userdb|database|migration|schema' | head -300Repository: ZaparooProject/zaparoo-core
Length of output: 50384
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- database files ---'
git ls-files pkg/database | sed -n '1,240p'
printf '%s\n' '--- exact table/index statements ---'
rg -n -i -C2 --glob '*.go' --glob '*.sql' --glob '*.json' \
'(CREATE[[:space:]]+TABLE|CREATE[[:space:]]+(UNIQUE[[:space:]]+)?INDEX|MediaHistory)' \
pkg/database | head -400
printf '%s\n' '--- query symbols ---'
rg -n -C8 'func .*GetMediaHistory|latestWhere|distinctMedia|LatestMedia' pkg/database/userdbRepository: ZaparooProject/zaparoo-core
Length of output: 50383
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- userdb files ---'
git ls-files pkg/database/userdb
printf '%s\n' '--- userdb schema/index declarations ---'
rg -n -i -C3 --glob '*.go' --glob '*.sql' \
'(CREATE[[:space:]]+TABLE|CREATE[[:space:]]+(UNIQUE[[:space:]]+)?INDEX|MediaHistory)' \
pkg/database/userdb
printf '%s\n' '--- distinct query ---'
sed -n '430,535p' pkg/database/userdb/media_history.go
printf '%s\n' '--- distinct test schema and assertions ---'
sed -n '264,334p' pkg/database/userdb/media_history_property_test.go
printf '%s\n' '--- migration loading references ---'
rg -n -C4 'userdb/migrations|migrations.*userdb|goose|UserDB' pkg/database/migrations.go pkg/database/userdb pkg/database/conn.goRepository: ZaparooProject/zaparoo-core
Length of output: 50384
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- migration filenames ---'
git ls-files | rg -i '(^|/)(migrations?|schema)(/|$)|migration.*\.(sql|go)$|(^|/)sql_.*\.go$' | rg -i 'user|database|migration|schema' | sed -n '1,300p'
printf '%s\n' '--- possible embedded migration declarations ---'
rg -l -i 'MediaHistory|CREATE TABLE.*History|CREATE INDEX.*History' --glob '*.sql' --glob '*.go' . | sort
printf '%s\n' '--- index declarations mentioning relevant columns ---'
rg -n -i --glob '*.sql' --glob '*.go' \
'CREATE[[:space:]]+(UNIQUE[[:space:]]+)?INDEX[^;\n]*(MediaHistory|SystemID|MediaPath|DBID)|INDEX[^;\n]*ON[[:space:]]+MediaHistory' \
. | sed -n '1,240p'
printf '%s\n' '--- relevant schema loader symbols ---'
rg -n -C3 'migrationFiles|migrationDir|embed\.FS|userdb' pkg/database --glob '*.go' | rg -i 'migration|embed|userdb' | sed -n '1,240p'Repository: ZaparooProject/zaparoo-core
Length of output: 32335
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import sqlite3
query = """
WITH LatestMedia AS (
SELECT MAX(DBID) AS DBID
FROM MediaHistory
{where}
GROUP BY SystemID, MediaPath
)
SELECT
history.DBID, history.StartTime, history.EndTime,
history.SystemID, history.SystemName, history.MediaPath,
history.MediaName, history.LauncherID, history.PlayTime
FROM MediaHistory AS history
INNER JOIN LatestMedia AS latest ON latest.DBID = history.DBID
WHERE history.DBID < ?
ORDER BY history.DBID DESC
LIMIT ?;
"""
def explain(extra_index, where):
db = sqlite3.connect(":memory:")
db.executescript("""
CREATE TABLE MediaHistory (
DBID INTEGER PRIMARY KEY AUTOINCREMENT,
StartTime INTEGER NOT NULL,
EndTime INTEGER,
SystemID TEXT NOT NULL,
SystemName TEXT NOT NULL,
MediaPath TEXT NOT NULL,
MediaName TEXT NOT NULL,
LauncherID TEXT NOT NULL,
PlayTime INTEGER NOT NULL,
BootUUID TEXT,
UpdatedAt INTEGER,
DeviceID TEXT,
ProfileID TEXT
);
CREATE INDEX idx_media_history_start_time ON MediaHistory (StartTime);
CREATE INDEX idx_media_history_open ON MediaHistory (EndTime) WHERE EndTime IS NULL;
CREATE INDEX idx_media_history_boot ON MediaHistory (BootUUID);
CREATE INDEX idx_media_history_updated ON MediaHistory (UpdatedAt);
CREATE INDEX idx_media_history_device ON MediaHistory (DeviceID) WHERE DeviceID IS NOT NULL;
CREATE INDEX idx_media_history_profile ON MediaHistory (ProfileID) WHERE ProfileID IS NOT NULL;
""")
if extra_index:
db.execute(
"CREATE INDEX idx_media_history_distinct ON "
"MediaHistory (SystemID, MediaPath, DBID)"
)
args = ("NES", 9223372036854775807, 25) if where else (9223372036854775807, 25)
plan = db.execute(
"EXPLAIN QUERY PLAN " + query.format(where="WHERE SystemID = ?" if where else ""),
args,
).fetchall()
db.close()
return plan
for where_name, where in (("unfiltered", False), ("system-filtered", True)):
for index_name, extra_index in (("production indexes", False), ("plus grouping index", True)):
print(f"--- {where_name}; {index_name} ---")
for row in explain(extra_index, where):
print(" | ".join(map(str, row)))
PYRepository: ZaparooProject/zaparoo-core
Length of output: 1320
Add an index for distinct media grouping. LatestMedia scans all matching rows before applying history.DBID < ?; SQLite uses a temporary B-tree for GROUP BY. Add a migration with (SystemID, MediaPath, DBID) to support this query.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/database/userdb/media_history.go` around lines 499 - 518, Add a database
migration creating a composite index on MediaHistory covering SystemID,
MediaPath, and DBID, so the LatestMedia GROUP BY query can avoid scanning and
temporary grouping structures. Follow existing migration conventions and ensure
the migration is safely applied and reversible if supported.
This was referenced Aug 14, 2026
This was referenced Aug 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
media.imagelocal-path delivery for validated Core-owned cached thumbnails while keeping inline delivery as the default and fallbackThis keeps Core as schema and data owner while addressing the MiSTer browsing bottlenecks demonstrated in Zaparoo frontend PR #360. Relevant feature commits credit Giancarlo Erra as co-author.
MiSTer results
Measured on a 244,623-item MiSTer library:
No database migration or persistent flattened cover cache is added. Sparse, sorted, filtered, and unsupported search scopes retain established SQL paths.
Ref ZaparooProject/zaparoo-frontend#360
Summary by CodeRabbit
New Features
Documentation