Skip to content

refactor(v2): give the metadata provider taxonomy one home - #4263

Merged
gantoine merged 4 commits into
rommapp:masterfrom
sdornan:claude/compassionate-chatelet-c7bcc7
Aug 22, 2026
Merged

refactor(v2): give the metadata provider taxonomy one home#4263
gantoine merged 4 commits into
rommapp:masterfrom
sdornan:claude/compassionate-chatelet-c7bcc7

Conversation

@sdornan

@sdornan sdornan commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Description

The v2 frontend declared the "general catalogs vs specialised sources vs hash proxies" provider taxonomy four separate times, each hand-maintained with its own shape:

Surface What it declared
useScanProviders/index.ts GENERAL_PROVIDER_KEYS / SPECIFIC_PROVIDER_KEYS (module-private key sets)
Auth/SetupStepMetadata.vue catalogs / specialised / proxies (key, name, logo, locale keys, requiresKey, disabled)
Settings/MetadataSources.vue catalogs / specialised / proxies (name, subtitle, value, logo, website, docsUrl, ...)
Scan/ScanInfoDialog.vue generalProviders / specificProviders / proxies (id, name, logo, locale keys)

This cost real bugs. When Steam was added in #4241 it landed in the specialised group in three surfaces while the composable treated it as general; two were fixed in 3064de63b and the third only turned up later in 0560c974c, because the first sweep found two of the three stragglers. An unclassified provider is worse than misclassified: it silently disappears from the scan selects entirely.

What changed

  • New frontend/src/v2/utils/metadataProviderGroups.ts holds the one map from provider slug (the same slug the backend MetadataSource enum and the heartbeat store use) to "catalog" | "specialised" | "proxy", plus the shared section order and the MetadataProviderKey type.
  • All four surfaces read from it. The three view surfaces now derive their sections by filtering one flat provider list, instead of declaring three lists, so a provider physically cannot be filed differently per surface. That also collapses three near-identical markup blocks per surface into one loop: net -244 lines across the four files.
  • The row key field is now typed key: MetadataProviderKey in all three components (was key: string / value / id), so an unknown provider slug is a compile error.
  • HASH_MATCHER_KEYS and the HashMatcherKey union in the composable are derived from the proxy group, so a third proxy becomes a compile error at the switch rather than a silent miss.
  • Per-surface presentation data (logos, setup.* / settings.* locale keys, websites, docs URLs, heartbeat wiring, section icons) deliberately stayed where it is used. Only the grouping moved.

Rendered providers, their order and every per-provider field value are unchanged (verified by diffing the extracted entries before/after). Rows and sections gained data-provider / data-group attributes as the test hook; no CSS keys off them.

Tests

frontend/src/v2/utils/metadataProviderGroups.test.ts is the contract test the taxonomy had been missing (only MetadataSources.vue had any grouping coverage before):

  • every provider the heartbeat exposes is classified, and every provider in the ROM-match registry is classified;
  • the scan selects split all 12 providers by the taxonomy (gamelist / libretro included);
  • the three reference surfaces render identical provider-to-group maps, read off the mounted DOM rather than matched on display names (the old approach had to special-case "SteamGridDB" containing "Steam");
  • the section order partitions the taxonomy.

I mutation-tested both guards: dropping sgdb from ScanInfoDialog fails the cross-surface assertion, and adding an unclassified steam to the heartbeat store fails the classification assertion. Those are the two exact failure modes from #4241.

gamelist and libretro are still absent from the three reference surfaces (they would need new locale keys, logos and website/docs entries). Out of scope here; the taxonomy classifies them, so the scan selects behave as before.

Verification

npm run typecheck clean, npm run test 746 passing / 68 files (was 741), trunk fmt && trunk check clean. Note the suite needs the .nvmrc Node 24 — under Node 26 every test file fails in vitest.setup.ts because Node's gated built-in localStorage collides with happy-dom's. That is pre-existing on master, not from this change.

I have not opened the setup wizard / settings / scan dialog in a browser in both themes. The change preserves rendering and the new test asserts the rendered grouping for all three surfaces, but a visual pass before merge is worthwhile.

AI assistance

This PR was written primarily by Claude Code (Opus 5), including the code, tests and this description, under my review and direction.

Checklist
Please check all that apply.

  • I've tested the changes locally
  • I've updated relevant comments
  • I've assigned reviewers for this PR
  • I've added unit tests that cover the changes

Screenshots (if applicable)

None. Pure refactor with no intended visual change.

🤖 Generated with Claude Code

The "general catalogs vs specialised sources vs hash proxies" split was
declared four separate times, each with its own shape: the scan
composable's key sets, and the provider arrays in the setup wizard's
metadata step, the settings view and the scan info dialog. Adding Steam
in rommapp#4241 filed it as specialised in three of them while the composable
treated it as general, and the fix took two passes because the first
sweep only found two of the three stragglers.

Provider slug to group now lives in one map, and each surface derives
its sections from it instead of declaring them, so the three near
identical markup blocks per surface collapse into one loop. Per-surface
presentation data (logos, locale keys, websites, docs URLs, heartbeat
wiring, section icons) stays where it is used. The hash matcher key
list and union are derived from the proxy group too.

Rendered providers, their order and their fields are unchanged; rows
and sections gained data-provider / data-group attributes as test
hooks.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 21, 2026 23:39
@greptile-apps

greptile-apps Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR centralizes v2 metadata-provider grouping into one typed taxonomy and updates scan selection, setup, settings, and scan-reference surfaces to derive their sections from it.

  • Adds a shared provider-group map, group order, key types, and lookup helpers.
  • Refactors three provider views to filter flat presentation lists through the shared taxonomy.
  • Derives scan provider and proxy classification from the taxonomy and adds cross-surface contract tests.

Confidence Score: 4/5

The PR appears safe to merge, with one non-blocking gap in the intended compile-time protection for adding future proxy providers.

Current provider membership, rendering, selection behavior, and payload construction remain equivalent, but a future proxy can widen the derived key type without forcing the hard-coded hash-matcher branches to be updated.

Files Needing Attention: frontend/src/v2/utils/metadataProviderGroups.ts, frontend/src/v2/composables/useScanProviders/index.ts

Important Files Changed

Filename Overview
frontend/src/v2/utils/metadataProviderGroups.ts Introduces the canonical provider taxonomy and derived types, though the advertised compile-time exhaustiveness for future proxies is not realized by consumers.
frontend/src/v2/composables/useScanProviders/index.ts Replaces duplicate provider sets with taxonomy lookups while preserving current selection and payload behavior; proxy handling remains hard-coded for two providers.
frontend/src/v2/components/Auth/SetupStepMetadata.vue Consolidates three explicit provider sections into taxonomy-derived groups while preserving provider data, ordering, probing, and status rendering.
frontend/src/v2/components/Scan/ScanInfoDialog.vue Derives provider reference sections from the shared taxonomy with the existing provider content and order preserved.
frontend/src/v2/views/Settings/MetadataSources.vue Refactors metadata-source cards into taxonomy-derived sections while retaining status, links, heartbeat wiring, and presentation fields.
frontend/src/v2/utils/metadataProviderGroups.test.ts Adds classification, partitioning, scan-select, and rendered cross-surface consistency coverage.

Fix all with Greploop Fix All in Claude Code

Prompt To Fix All With AI
### Issue 1
frontend/src/v2/utils/metadataProviderGroups.ts:27-34
**Proxy exhaustiveness is not enforced**

Adding another `proxy` widens `MetadataProviderKeyIn<"proxy">`, but the hash-matcher branches treat every non-Hasheous key as Playmatch and payload construction still handles only the current two providers. Type checking therefore does not force those branches to be updated, allowing a future proxy to share Playmatch state or be omitted from scan payloads.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "refactor(v2): give the metadata provider..." | Re-trigger Greptile

Comment thread frontend/src/v2/utils/metadataProviderGroups.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request centralizes the v2 metadata-provider taxonomy and updates all related surfaces to derive their groupings from it.

Changes:

  • Adds shared provider groups, ordering, and typed keys.
  • Refactors scan, setup, settings, and scan-info surfaces.
  • Adds classification and cross-surface contract tests.

Review findings:

  • useScanProviders/index.ts: Moderate (3 votes) — hash-matcher handling is not exhaustive for new proxy providers.
  • metadataProviderGroups.ts: Moderate (3 votes) — inherited properties may be accepted as provider keys.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Summary
frontend/src/v2/views/Settings/MetadataSources.vue Derives settings sections from the shared taxonomy.
frontend/src/v2/utils/metadataProviderGroups.ts Defines shared taxonomy utilities.
frontend/src/v2/utils/metadataProviderGroups.test.ts Tests classification and cross-surface consistency.
frontend/src/v2/composables/useScanProviders/index.ts Uses taxonomy-driven scan filtering and hash matching.
frontend/src/v2/components/Scan/ScanInfoDialog.vue Derives provider sections from the shared taxonomy.
frontend/src/v2/components/Auth/SetupStepMetadata.vue Derives setup sections from the shared taxonomy.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread frontend/src/v2/composables/useScanProviders/index.ts Outdated
Comment thread frontend/src/v2/utils/metadataProviderGroups.ts Outdated
sdornan and others added 3 commits August 21, 2026 18:44
MetadataProviderKeyIn claimed a consumer would get a compile error when
a group grew a member, but the hash-matcher code branched on the key:
setHashMatcher treated every non-Hasheous key as Playmatch, so a third
proxy would silently share Playmatch's stored flag and be missing from
the switch list.

Both now hang off a Record keyed by HashMatcherKey, so adding a proxy to
the map fails to compile until it declares its own stored flag and its
name/logo/gate. providerKeysInGroup returns the narrowed union to make
that possible, and the catalog filter asks the taxonomy for the group
instead of matching against the proxy key list.

Payload wiring stays hand-written: which slot a matcher fills is set by
its backend gate, not by the taxonomy.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`in` walks the prototype chain, so metadataProviderGroup("toString")
returned the inherited function rather than undefined, breaking the
declared return type. Object.hasOwn keeps the lookup to the map's own
keys.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Each of the three provider surfaces had hand-rolled the same section
labels and the same order-and-partition block. The setup wizard's step
and the scan info dialog held byte-identical `GROUP_LABELS` maps, so
those move to `SETUP_GROUP_LABELS`, and the partition becomes
`groupProviders()` with a unit test. The settings view keeps its own
labels, since its `settings.*` keys and section icons differ.

Trims the comments that explained the taxonomy instead of recording what
the code cannot say.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gantoine
gantoine merged commit 6a25a21 into rommapp:master Aug 22, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants