fix(scraper): follow the MiSTer artwork pack format - #1429
Merged
Conversation
Conform the mister-docs scraper to the published MiSTer Artwork Pack format (chipster6502/MiSTer_artwork_pack, PACK_FORMAT.md). Arcade packs file games under MAME parent setnames, which live inside each MRA rather than in its filename, so nothing arcade resolved. For systems with an arcade source the scraper now reads <setname> from installed MRAs and matches on it; on a MiSTer with 2,971 MRAs the Arcade pack went from 343 matches (single-word titles only) to 2,658. Resolution follows the format's order: catalogued name, arcade setname, a trailing (setname) tag that is itself a pack key, then the bare title, which is only allowed when it identifies exactly one key in the pack and one title in the library. Images the index does not mention resolve by exact name only, which is also what a pack shipped without an index gets instead of being ignored. Synopsis files are globbed and chosen by media.default_langs, then English, then the first available language, rather than hardcoding synopsis_en.tsv. Games gameinfo.tsv lists without an image still receive year, genre, developer, players and description. Title metadata is first-wins so a demo or regional variant that resolves to the same title does not replace the representative dump's details. Shared-catalogue fallbacks follow the format's table: Game Boy and Game Boy Color try each other, Super Game Boy reads both, FDS falls back to NES but never the reverse, and the general SG-1000 to ColecoVision and Neo Geo Pocket Color to Neo Geo Pocket fallbacks are blocked because the pack catalogues those separately. Duplicate rows in gameinfo or synopsis are skipped instead of discarding the file, and /media/usb6 and /media/usb7 are probed for docs. A large pack takes minutes per step, and the scraper reported nothing until a step finished. It now reports the step size as soon as the pack is loaded and advances after every committed write batch, and logs a per-step line with load, scan, match, cleanup and write durations.
The four property-by-ID getters share query builders that joined MediaProperties or MediaTitleProperties to Tags, TagTypes and MediaBlobs with plain JOINs. Scrapers fill the property tables after indexing has run ANALYZE, so the planner usually has no statistics for them, and for a lookup of many IDs it drove the query from TagTypes outward, probing the property index once per tag per requested ID. On a MiSTer the mister-docs force-mode cleanup, which fetches the properties of every media row in a system, spent 567s of a 637s Arcade step (2,971 IDs) inside that one query; the same lookup takes 4.7s with the join order fixed. The getters also serve media.meta and media.image, which were exposed to the same plan. CROSS JOIN fixes SQLite's nesting order so every lookup starts with the property table's own index whatever the statistics say. A test pins the plan on a fresh, un-analysed database.
|
Warning Review limit reachedNext included review available in 27 minutes. View limit detailsLimit details: You’ve used all 2 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (14)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
wizzomafizzo
added a commit
that referenced
this pull request
Sep 5, 2026
- The first v2.17.2 tag build ended in `startup_failure` with no jobs run. GitHub rejected build.yml with: "Error calling workflow 'lint-and-test.yml'. The workflow is requesting 'security-events: write', but is only allowed 'security-events: none'." - lint-and-test.yml declares `security-events: write` at workflow level for its govulncheck SARIF upload. The release gate job added in #1409 calls it granting only `contents`, `pull-requests` and `packages` read, and GitHub validates a called workflow's declared permissions against the caller's grant before any job starts, whether or not the step that uses them will run. - Grant `security-events: write` on the gate job. Nothing in a gate run uses it, because the `release_gate` input already skips the SARIF upload. - With the gate running, its Windows job failed on `TestScrapeLoop_AccumulatesSourceLoadFailures` from #1429. `scrapeLoop` formats each failed source path with `%q`, which escapes the backslashes in a Windows path, while the test searched the error for the raw path. The assertion now compares against `strconv.Quote` of the path, which is what `%q` produces. - With the gate passing and every target built, Windows signing was rejected: SignPath accepts an artifact only when every job upstream of it ran in the "GitHub Actions" runner group, and the gate's Linux job runs on `ubicloud-standard-4`. Before #1409 the build job had no dependency on lint-and-test.yml, so that runner was never in a release's chain. The gate now runs its Linux job on `ubuntu-latest`; pull requests and pushes to main keep the Ubicloud runner. - The gate had never run on a real tag before this: #1409 landed after v2.17.1, and the weekly build-check workflow, which calls build.yml the same way, has been failing at startup on main since 2026-08-31 for a reason not yet looked at. - All three commits are cherry-picked from `release/v2.17.2`, where the v2.17.2 tag points at the last one.
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
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.
mister-docsscraper (introduced in feat(scraper): import MiSTer installed docs metadata #1308) to the published MiSTer Artwork Pack format, PACK_FORMAT.md.<setname>from installed MRAs; on a MiSTer with 2,971 MRAs the Arcade pack went from 343 matches to 2,658.(setname)tag that is itself a pack key, then the bare title only when it identifies exactly one key in the pack and one title in the library. Images the index does not mention resolve by exact name, which is also what a pack without an index gets instead of being ignored.synopsis_*.tsvis globbed and chosen bymedia.default_langs, then English, then the first available language, instead of hardcodingsynopsis_en.tsv. Gamesgameinfo.tsvlists without an image still receive their metadata, and title metadata is first-wins so a demo or regional variant does not replace the representative dump's details.gameinfo/synopsisrows are skipped rather than discarding the file, and/media/usb6and/media/usb7are probed.mediadbjoined with plainJOINs, and without statistics on the scraper-filled property tables SQLite planned a many-ID lookup fromTagTypesoutward, probing the property index once per tag per requested ID. The force-mode cleanup spent 567s of a 637s Arcade step in that query; withCROSS JOINfixing the nesting order it takes 4.7s. The same getters servemedia.metaandmedia.image. A test pins the plan on an un-analysed database.docs/scraper.mddescribes the new discovery and matching rules.