Repository: ROpdebee/mb-userscripts
Pull request #535 (2022-07-28 17:29:05)

fix(ecau): improve VGMdb caption mapping
* More whitespace trimming
* Support colons to separate type from comment
* Map disc matrix/runout types
* Map packaging top/bottom types
* Improved mapping of digipak/slipcase faces
* More edge cases
* Include medium type for discs with sequence numbers

https://community.metabrainz.org/t/ropdebees-userscripts-support-thread/551947/94?u=ropdebee

codecov[bot]: # [Codecov](https://codecov.io/gh/ROpdebee/mb-userscripts/pull/535?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Ruben+Opdebeeck) Report
> Merging [#535](https://codecov.io/gh/ROpdebee/mb-userscripts/pull/535?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Ruben+Opdebeeck) (d0e38e8) into [main](https://codecov.io/gh/ROpdebee/mb-userscripts/commit/a5bfdb439ed2bcc631af35b2b03a7f5a3cce2696?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Ruben+Opdebeeck) (a5bfdb4) will **decrease** coverage by `0.22%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##             main     #535      +/-   ##
==========================================
- Coverage   99.77%   99.55%   -0.23%     
==========================================
  Files          57       57              
  Lines        1308     1335      +27     
  Branches      204      209       +5     
==========================================
+ Hits         1305     1329      +24     
- Misses          3        6       +3     
```


| [Impacted Files](https://codecov.io/gh/ROpdebee/mb-userscripts/pull/535?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Ruben+Opdebeeck) | Coverage Δ | |
|---|---|---|
| [...c/mb\_enhanced\_cover\_art\_uploads/providers/vgmdb.ts](https://codecov.io/gh/ROpdebee/mb-userscripts/pull/535/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Ruben+Opdebeeck#diff-c3JjL21iX2VuaGFuY2VkX2NvdmVyX2FydF91cGxvYWRzL3Byb3ZpZGVycy92Z21kYi50cw==) | `100.00% <100.00%> (ø)` | |
| [src/lib/util/array.ts](https://codecov.io/gh/ROpdebee/mb-userscripts/pull/535/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Ruben+Opdebeeck#diff-c3JjL2xpYi91dGlsL2FycmF5LnRz) | `83.33% <0.00%> (-16.67%)` | :arrow_down: |
| [src/mb\_enhanced\_cover\_art\_uploads/fetch.ts](https://codecov.io/gh/ROpdebee/mb-userscripts/pull/535/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Ruben+Opdebeeck#diff-c3JjL21iX2VuaGFuY2VkX2NvdmVyX2FydF91cGxvYWRzL2ZldGNoLnRz) | `100.00% <0.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/ROpdebee/mb-userscripts/pull/535?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Ruben+Opdebeeck).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Ruben+Opdebeeck)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/ROpdebee/mb-userscripts/pull/535?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Ruben+Opdebeeck). Last update [a5bfdb4...d0e38e8](https://codecov.io/gh/ROpdebee/mb-userscripts/pull/535?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Ruben+Opdebeeck). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Ruben+Opdebeeck).

ROpdebee: #yolo once more.

Patch:
diff --git a/src/lib/MB/CoverArt.ts b/src/lib/MB/CoverArt.ts
index fd4597ed..04633123 100644
--- a/src/lib/MB/CoverArt.ts
+++ b/src/lib/MB/CoverArt.ts
@@ -13,4 +13,7 @@ export enum ArtworkTypeIDs {
     Track = 7,
     Tray = 9,
     Watermark = 13,
+    Matrix = 15,  // Matrix/Runout
+    Top = 48,
+    Bottom = 49,
 }
diff --git a/src/mb_enhanced_cover_art_uploads/providers/vgmdb.ts b/src/mb_enhanced_cover_art_uploads/providers/vgmdb.ts
index 5fad019f..3bd989bb 100644
--- a/src/mb_enhanced_cover_art_uploads/providers/vgmdb.ts
+++ b/src/mb_enhanced_cover_art_uploads/providers/vgmdb.ts
@@ -25,56 +25,104 @@ interface AlbumMetadata {
 // type, list of types, or type with additional information
 type MappedArtwork = ArtworkTypeIDs | ArtworkTypeIDs[] | { type: ArtworkTypeIDs | ArtworkTypeIDs[]; comment: string };
 
-export /* for tests */ function mapJacketType(caption: string): MappedArtwork {
-    if (!caption) {
+function mapPackagingType(packaging: string, caption: string): MappedArtwork {
+    if (!caption && packaging === 'Jacket') {
+        // Assume jacket is front, back, and spine.
         return {
             type: [ArtworkTypeIDs.Front, ArtworkTypeIDs.Back, ArtworkTypeIDs.Spine],
             comment: '',
         };
+    } else if (!caption) {
+        // Slipcase, digipak, box is likely just front
+        return {
+            type: [ArtworkTypeIDs.Front],
+            comment: packaging,
+        };
     }
 
-    const types = [];
+    const types = new Set<ArtworkTypeIDs>();
     const keywords = caption.split(/,|\s|and|&/i);
-    const faceKeywords = ['front', 'back', 'spine'];
-    const [hasFront, hasBack, hasSpine] = faceKeywords
-        .map((faceKw) => keywords
-            // Case-insensitive .includes()
-            .some((kw) => kw.toLowerCase() === faceKw.toLowerCase()));
-
-    if (hasFront) types.push(ArtworkTypeIDs.Front);
-    if (hasBack) types.push(ArtworkTypeIDs.Back);
+    const typeKeywordsToTypes = [
+        ['front', ArtworkTypeIDs.Front],
+        ['back', ArtworkTypeIDs.Back],
+        ['spine', ArtworkTypeIDs.Spine],
+        ['top', ArtworkTypeIDs.Top],
+        ['bottom', ArtworkTypeIDs.Bottom],
+        ['interior', ArtworkTypeIDs.Tray],
+        ['inside', ArtworkTypeIDs.Tray],
+    ] as const;
+    for (const [typeKeyword, type] of typeKeywordsToTypes) {
+        if (keywords.some((kw) => kw.toLowerCase() === typeKeyword)) {
+            types.add(type);
+        }
+    }
+
     // Assuming if the front and back are included, the spine is as well.
-    if (hasSpine || (hasFront && hasBack)) types.push(ArtworkTypeIDs.Spine);
+    if (types.has(ArtworkTypeIDs.Front) && types.has(ArtworkTypeIDs.Back)) {
+        types.add(ArtworkTypeIDs.Spine);
+    }
 
-    // Copy anything other than 'front', 'back', or 'spine' to the comment
+    // Copy anything other than the type keywords to the comment
+    const typeKeywords = new Set<string>(typeKeywordsToTypes.map(([typeKeyword]) => typeKeyword));
     const otherKeywords = keywords
-        .filter((kw) => !faceKeywords.includes(kw.toLowerCase()));
+        .filter((kw) => !typeKeywords.has(kw.toLowerCase()));
+
+    // Also include packaging name in comment if it's not a jacket.
+    if (packaging !== 'Jacket') otherKeywords.unshift(packaging);
     const comment = otherKeywords.join(' ').trim();
+
     return {
-        type: types,
+        type: types.size > 0 ? [...types] : [ArtworkTypeIDs.Other],
         comment,
     };
 }
 
+function mapDiscType(mediumType: string, caption: string): MappedArtwork {
+    const keywords = caption.split(/,|\s/).filter(Boolean);
+    const commentParts = [];
+    let type = ArtworkTypeIDs.Medium;
+    for (const keyword of keywords) {
+        // As a regular expression because it might be surrounded in parentheses
+        if (/reverse|back/i.test(keyword)) {
+            type = ArtworkTypeIDs.Matrix;
+        } else {
+            commentParts.push(keyword);
+        }
+    }
+
+    // If the comment starts with a sequence number, add the medium type
+    if (commentParts.length > 0 && /^\d+/.test(commentParts[0])) {
+        commentParts.unshift(mediumType);
+    }
+
+    return {
+        type,
+        comment: commentParts.join(' '),
+    };
+}
+
 // Keys: First word of the VGMdb caption (mostly structured), lower-cased
 // Values: Either MappedArtwork or a callable taking the remainder of the caption and returning MappedArtwork
 const __CAPTION_TYPE_MAPPING: Record<string, MappedArtwork | ((caption: string) => MappedArtwork)> = {
     front: ArtworkTypeIDs.Front,
     booklet: ArtworkTypeIDs.Booklet,
-    jacket: mapJacketType, // DVD jacket
-    disc: ArtworkTypeIDs.Medium,
+    jacket: mapPackagingType.bind(undefined, 'Jacket'), // DVD jacket
+    disc: mapDiscType.bind(undefined, 'Disc'),
+    cd: mapDiscType.bind(undefined, 'CD'),
     cassette: ArtworkTypeIDs.Medium,
     vinyl: ArtworkTypeIDs.Medium,
+    dvd: mapDiscType.bind(undefined, 'DVD'),
     tray: ArtworkTypeIDs.Tray,
     back: ArtworkTypeIDs.Back,
     obi: ArtworkTypeIDs.Obi,
-    box: { type: ArtworkTypeIDs.Other, comment: 'Box' },
+    box: mapPackagingType.bind(undefined, 'Box'),
     card: { type: ArtworkTypeIDs.Other, comment: 'Card' },
     sticker: ArtworkTypeIDs.Sticker,
-    slipcase: { type: ArtworkTypeIDs.Other, comment: 'Slipcase' },
-    digipack: { type: ArtworkTypeIDs.Other, comment: 'Digipak' },
+    slipcase: mapPackagingType.bind(undefined, 'Slipcase'),
+    digipack: mapPackagingType.bind(undefined, 'Digipak'),
     insert: { type: ArtworkTypeIDs.Other, comment: 'Insert' }, // Or poster?
-    case: { type: ArtworkTypeIDs.Other, comment: 'Case' },
+    inside: ArtworkTypeIDs.Tray,
+    case: mapPackagingType.bind(undefined, 'Case'),
     contents: ArtworkTypeIDs.Raw,
 };
 
@@ -131,15 +179,30 @@ const NSFW_PLACEHOLDER_URL = '/db/img/album-nsfw-medium.gif';
 
 function cleanupCaption(captionRest: string): string {
     return captionRest
-        // Remove superfluous spaces
-        .trim()
         // Remove parentheses, braces and brackets, but only if they wrap the
         // whole caption
         .replace(/^\((.+)\)$/, '$1')
         .replace(/^\[(.+)]$/, '$1')
         .replace(/^{(.+)}$/, '$1')
-        // Remove leading dash, possibly used to split type from comment
-        .replace(/^[-–]\s*/, '');
+        // Remove leading dash or colon, possibly used to split type from comment
+        .replace(/^[-–:]\s*/, '');
+}
+
+function convertCaption(caption: string): { types?: ArtworkTypeIDs[]; comment: string } {
+    LOGGER.debug(`Found caption “${caption}”`);
+    const [captionType, ...captionRestParts] = caption.trim().split(/\b/);
+    const captionRest = cleanupCaption(captionRestParts.join('').trim());
+    const mapper = CAPTION_TYPE_MAPPING[captionType.toLowerCase()];
+
+    if (!mapper) {
+        LOGGER.debug(`Could not map “${captionType}” to any known cover art types`);
+        LOGGER.debug(`Setting cover art comment to “${caption}”`);
+        return { comment: caption };
+    }
+
+    const mappedResult = mapper(captionRest);
+    LOGGER.debug(`Mapped caption to types ${mappedResult.types} and comment “${mappedResult.comment}”`);
+    return mappedResult;
 }
 
 export function convertCaptions(cover: { url: string; caption: string }): CoverArt {
@@ -147,14 +210,10 @@ export function convertCaptions(cover: { url: string; caption: string }): CoverA
     if (!cover.caption) {
         return { url };
     }
-    const [captionType, ...captionRestParts] = cover.caption.split(' ');
-    const captionRest = cleanupCaption(captionRestParts.join(' '));
-    const mapper = CAPTION_TYPE_MAPPING[captionType.toLowerCase()];
 
-    if (!mapper) return { url, comment: cover.caption };
     return {
         url,
-        ...mapper(captionRest),
+        ...convertCaption(cover.caption),
     };
 }
 


Test patch:
diff --git a/tests/unit/mb_enhanced_cover_art_uploads/providers/vgmdb.test.ts b/tests/unit/mb_enhanced_cover_art_uploads/providers/vgmdb.test.ts
index 310fff83..320e9d75 100644
--- a/tests/unit/mb_enhanced_cover_art_uploads/providers/vgmdb.test.ts
+++ b/tests/unit/mb_enhanced_cover_art_uploads/providers/vgmdb.test.ts
@@ -1,5 +1,5 @@
 import { ArtworkTypeIDs } from '@lib/MB/CoverArt';
-import { convertCaptions, mapJacketType, VGMdbProvider } from '@src/mb_enhanced_cover_art_uploads/providers/vgmdb';
+import { convertCaptions, VGMdbProvider } from '@src/mb_enhanced_cover_art_uploads/providers/vgmdb';
 import { setupPolly } from '@test-utils/pollyjs';
 import { itBehavesLike } from '@test-utils/shared_behaviour';
 
@@ -29,76 +29,45 @@ describe('vgmdb provider', () => {
         itBehavesLike(urlMatchingSpec, { provider, supportedUrls, unsupportedUrls });
     });
 
-    describe('mapping jacket types', () => {
-        const simpleJacketCases: Array<[string, ArtworkTypeIDs]> = [
-            ['Front', ArtworkTypeIDs.Front],
-            ['Back', ArtworkTypeIDs.Back],
-            ['Spine', ArtworkTypeIDs.Spine],
-        ];
-
-        it.each(simpleJacketCases)('should map simple jacket type with %s', (caption, expected) => {
-            expect(mapJacketType(caption)).toStrictEqual({
-                type: [expected],
-                comment: '',
-            });
-        });
-
-        it('should map to full jacket when no caption is present', () => {
-            expect(mapJacketType('')).toStrictEqual({
-                type: [
-                    ArtworkTypeIDs.Front,
-                    ArtworkTypeIDs.Back,
-                    ArtworkTypeIDs.Spine,
-                ],
-                comment: '',
-            });
-        });
-
-        it('should include spine when front and back are present', () => {
-            expect(mapJacketType('Front, Back')).toStrictEqual({
-                type: [
-                    ArtworkTypeIDs.Front,
-                    ArtworkTypeIDs.Back,
-                    ArtworkTypeIDs.Spine,
-                ],
-                comment: '',
-            });
-        });
-
-        it('should retain other comments', () => {
-            expect(mapJacketType('Front and Back colorised')).toStrictEqual({
-                type: [
-                    ArtworkTypeIDs.Front,
-                    ArtworkTypeIDs.Back,
-                    ArtworkTypeIDs.Spine,
-                ],
-                comment: 'colorised',
-            });
-        });
-    });
-
     describe('caption type mapping', () => {
         const mappingCases: Array<[string, ArtworkTypeIDs[], string]> = [
             ['Front', [ArtworkTypeIDs.Front], ''],
             ['Back', [ArtworkTypeIDs.Back], ''],
+            ['Jacket', [ArtworkTypeIDs.Front, ArtworkTypeIDs.Back, ArtworkTypeIDs.Spine], ''],
             ['Jacket Front', [ArtworkTypeIDs.Front], ''],
             ['Jacket Front & Back', [ArtworkTypeIDs.Front, ArtworkTypeIDs.Back, ArtworkTypeIDs.Spine], ''],
-            ['Jacket Front & Back', [ArtworkTypeIDs.Front, ArtworkTypeIDs.Back, ArtworkTypeIDs.Spine], ''],
-            ['Disc 1', [ArtworkTypeIDs.Medium], '1'],
+            ['Jacket Front, Back', [ArtworkTypeIDs.Front, ArtworkTypeIDs.Back, ArtworkTypeIDs.Spine], ''],
+            ['Jacket Front & Back colorised', [ArtworkTypeIDs.Front, ArtworkTypeIDs.Back, ArtworkTypeIDs.Spine], 'colorised'],
+            ['Disc', [ArtworkTypeIDs.Medium], ''],
+            ['Disc 1', [ArtworkTypeIDs.Medium], 'Disc 1'],
+            ['DVD', [ArtworkTypeIDs.Medium], ''],
+            ['Disc (reverse)', [ArtworkTypeIDs.Matrix], ''],
+            ['Disc (Back)', [ArtworkTypeIDs.Matrix], ''],
+            ['Disc 1 (Back)', [ArtworkTypeIDs.Matrix], 'Disc 1'],
             ['Cassette Front', [ArtworkTypeIDs.Medium], 'Front'],
             ['Vinyl A-side', [ArtworkTypeIDs.Medium], 'A-side'],
             ['Tray', [ArtworkTypeIDs.Tray], ''],
             ['Back', [ArtworkTypeIDs.Back], ''],
             ['Obi', [ArtworkTypeIDs.Obi], ''],
-            ['Box', [ArtworkTypeIDs.Other], 'Box'],
-            ['Box Front', [ArtworkTypeIDs.Other], 'Box Front'],
+            ['Obi Front', [ArtworkTypeIDs.Obi], 'Front'],
+            ['Box', [ArtworkTypeIDs.Front], 'Box'],
+            ['Box Front', [ArtworkTypeIDs.Front], 'Box'],
             ['Card', [ArtworkTypeIDs.Other], 'Card'],
+            ['Card Front', [ArtworkTypeIDs.Other], 'Card Front'],
             ['Sticker', [ArtworkTypeIDs.Sticker], ''],
-            ['Slipcase', [ArtworkTypeIDs.Other], 'Slipcase'],
+            ['Slipcase', [ArtworkTypeIDs.Front], 'Slipcase'],
+            ['Slipcase Front', [ArtworkTypeIDs.Front], 'Slipcase'],
+            ['Slipcase Bottom', [ArtworkTypeIDs.Bottom], 'Slipcase'],
             ['Digipack Outer Left', [ArtworkTypeIDs.Other], 'Digipak Outer Left'],
+            ['Digipack Front & Back', [ArtworkTypeIDs.Front, ArtworkTypeIDs.Back, ArtworkTypeIDs.Spine], 'Digipak'],
+            ['Digipack Interior', [ArtworkTypeIDs.Tray], 'Digipak'],
             ['Insert', [ArtworkTypeIDs.Other], 'Insert'],
-            ['Case', [ArtworkTypeIDs.Other], 'Case'],
+            ['Case', [ArtworkTypeIDs.Front], 'Case'],
+            ['Case: Back', [ArtworkTypeIDs.Back], 'Case'],
+            ['Case: Inside', [ArtworkTypeIDs.Tray], 'Case'],
             ['Contents', [ArtworkTypeIDs.Raw], ''],
+            [' Booklet Front & Back', [ArtworkTypeIDs.Booklet], 'Front & Back'],
+            ['Booklet: Interview', [ArtworkTypeIDs.Booklet], 'Interview'],
         ];
 
         it.each(mappingCases)('should map %s to the correct type', (caption, expectedTypes, expectedComment) => {