diff --git a/extension/extract.js b/extension/extract.js index 3f9644e..509c11b 100644 --- a/extension/extract.js +++ b/extension/extract.js @@ -26,10 +26,13 @@ // 1. A quoted string ("Reek of Putrefaction" or 'Scum') — a strong "this is a title" cue, // but only when it reads like a title (short, no sentence-ending punctuation mid-span), // so prose quotes ("We hold these truths…", "lol") don't become candidates. - // 2. An album cue phrase ("the album Scum", "Scum LP/EP") naming a title-shaped run: the - // capture starts at a capitalized/quoted token (so a cue followed by a verb/article — - // "the album by Carcass", "the album is great" — yields no candidate) and the LP/EP - // branch is anchored to 1-4 capitalized words instead of grabbing the sentence prefix. + // 2. An album cue phrase ("the album Scum", "Scum LP/EP") naming a title-shaped run. The WEAK + // cue (album|record|ep|lp) can't separate a stylized lowercase title from prose, so after it + // only a capital/digit-led run OR a SINGLE lowercase token is admitted (a multi-word + // lowercase run after a weak cue is rejected — it leaks prose). A multi-word lowercase title + // is admitted ONLY after a STRONG explicit cue ("album called good kid maad city"). A cue + // followed by a verb/article ("the album by Carcass", "the album is great") yields no + // candidate, and the LP/EP branch is anchored to a 1-4-word title, not the sentence prefix. // Group 1 of each holds the candidate; the `d` flag gives its position for in-place wrapping. const QUOTED_RE = /["“]([^"”]{2,80})["”]|['‘]([^'’]{2,80})['’]/gd; // A single word inside a title run (Title-Case/ALL-CAPS — or, for a digit-led run, a number). @@ -40,18 +43,34 @@ // is excluded. The lead also admits a DIGIT, so number-led titles ("1989 LP", "36 Chambers") // survive; it does NOT admit a bare lowercase lead (keeps "their new LP" from leaking "new"). const TITLE_RUN = `["“'‘]?[A-Z0-9]${WORD}(?:[ ]+(?:${TOKEN}|of|the|and|&|in|on|to|for|a|an)){0,3}["”'’]?`; - // LOWER_RUN — a fully lowercase-styled title (lead + inner words all lowercase/digit), bounded to - // 1-4 words, for stylized titles after an explicit album cue ("good kid maad city", "untitled"). - // A verb/article lead ("the album by Carcass", "the album is great") is captured here but then - // dropped by ALBUM_LEAD_STOP below, so this run never weakens the precision guard. + // DIGIT_RUN — a DIGIT-led run whose inner words may be lowercase ("36 chambers", "808s and + // heartbreak", "1989 deluxe"). The leading digit IS the precision signal a title was meant (a + // bare lowercase lead is not), so unlike TITLE_RUN this run absorbs trailing lowercase tokens — + // bounded to 1-4 words so it never grabs the sentence tail. Tried BEFORE TITLE_RUN in the + // alternation so a digit lead captures its full continuation, not just the bare number. + const DIGIT_RUN = `["“'‘]?[0-9]${WORD}(?:[ ]+[A-Za-z0-9'’.&\\-]+){0,3}["”'’]?`; + // LOWER_RUN — a fully lowercase-styled MULTI-word title (lead + inner words all lowercase/digit), + // bounded to 1-4 words. The weak cue (album|record|ep|lp) cannot tell a stylized lowercase title + // ("good kid maad city") from prose ("makes me cry") — the lowercase-prose space dwarfs the rare + // real case — so a multi-word lowercase run is admitted ONLY after a STRONG explicit cue + // (called|titled|named). After the weak cue, only a single lowercase token (LOWER_ONE) is allowed. const LOWER_RUN = `[a-z0-9]${WORD}(?:[ ]+[a-z0-9]${WORD}){0,3}`; + // LOWER_ONE — a SINGLE lowercase token admitted after the WEAK cue ("the album untitled"). The + // negative lookahead `(?![ ]+[a-z])` requires it to be the WHOLE title: a second lowercase word + // means prose ("makes me cry", "changed everything"), which is rejected outright — the lever that + // closes the LOWER_RUN precision leak while keeping a one-word stylized title like "untitled". + const LOWER_ONE = `[a-z]${WORD}(?![ ]+[a-z])`; const ALBUM_CUE_RE = new RegExp( - // "the album X" — bounded to a 1-4-token run (TITLE_RUN for a capital/digit-led title, LOWER_RUN - // for a lowercase-styled one), so "the album Money Store dropped" yields "Money Store", not the - // sentence tail. A verb/article lead is dropped by ALBUM_LEAD_STOP below. - `\\b(?:the )?(?:album|record|ep|lp)\\s+(?:called\\s+|titled\\s+|named\\s+)?(${TITLE_RUN}|${LOWER_RUN})\\b` + - // "X LP/EP" — anchored title run (no left-anchor-less prefix grab). - `|\\b(${TITLE_RUN})\\s+(?:LP|EP)\\b`, + // STRONG cue: "album called/titled/named X" — the explicit naming verb licenses a multi-word + // lowercase title (LOWER_RUN), the ONLY place a bare lowercase run is admitted. Digit- and + // capital-led runs work here too. DIGIT_RUN precedes TITLE_RUN so a digit lead absorbs its + // lowercase continuation instead of [A-Z0-9] matching the bare number and stopping. + `\\b(?:the )?(?:album|record|ep|lp)\\s+(?:called|titled|named)\\s+(${DIGIT_RUN}|${TITLE_RUN}|${LOWER_RUN})\\b` + + // WEAK cue: "the album X" — a capital/digit-led title (DIGIT_RUN|TITLE_RUN) or at most a + // SINGLE lowercase token (LOWER_ONE). No multi-word lowercase run here: it would leak prose. + `|\\b(?:the )?(?:album|record|ep|lp)\\s+(${DIGIT_RUN}|${TITLE_RUN}|${LOWER_ONE})\\b` + + // "X LP/EP" — anchored title run (no left-anchor-less prefix grab); digit-led too. + `|\\b(${DIGIT_RUN}|${TITLE_RUN})\\s+(?:LP|EP)\\b`, "gd" ); // First word of an album-cue capture that, despite a leading capital, is not a title. @@ -144,6 +163,10 @@ const out = []; const seen = new Set(); // dedupe identical (index,length) spans across the two signals const push = (name, index, length) => { + // Position-mapping invariant: the wrapper links a candidate via text.substr(index, length), + // so the emitted span must reproduce the name exactly. Drop any candidate that violates it + // (a defensive guard over the quote/cue index-shift arithmetic) rather than mis-link the DOM. + if (text.substr(index, length) !== name) return; const key = index + ":" + length; if (seen.has(key)) return; seen.add(key); @@ -170,7 +193,7 @@ } ALBUM_CUE_RE.lastIndex = 0; while ((m = ALBUM_CUE_RE.exec(text))) { - const gi = m.indices[1] || m.indices[2]; // "album X" group or "X LP/EP" group + const gi = m.indices[1] || m.indices[2] || m.indices[3]; // strong-cue / weak-cue / "X LP/EP" group if (!gi) continue; // Trim a leading opening-quote the title run may have captured, shifting the index so // the emitted span still maps back to the source text; then strip trailing punct/quotes. diff --git a/extension/extract.test.cjs b/extension/extract.test.cjs index e06cb6b..89fb805 100644 --- a/extension/extract.test.cjs +++ b/extension/extract.test.cjs @@ -112,10 +112,12 @@ test("albumMatchesIn: recall — number-led titles via an explicit album cue", ( assert.ok(albumMatchesIn("the album 36 Chambers").map((m) => m.name).includes("36 Chambers")); }); -test("albumMatchesIn: recall — lowercase-styled titles after a cue", () => { - // good kid maad city is fully lowercase — a stylized title, not prose; the cue admits it. +test("albumMatchesIn: recall — a lowercase-styled MULTI-word title needs a STRONG cue", () => { + // good kid maad city is fully lowercase. The WEAK cue (album|record|ep|lp) can't tell a stylized + // title from prose, so it emits NOTHING; a STRONG explicit cue (called|titled|named) licenses it. + assert.equal(albumMatchesIn("the album good kid maad city").length, 0); // weak cue → nothing assert.ok( - albumMatchesIn("the album good kid maad city").map((m) => m.name).includes("good kid maad city") + albumMatchesIn("an album called good kid maad city").map((m) => m.name).includes("good kid maad city") ); }); @@ -173,6 +175,93 @@ test("albumMatchesIn: every rebalance candidate maps back to the source text", ( } }); +// --- Convergence: the prior LOWER_RUN both leaked lowercase prose and truncated digit-led titles. +// The stable design: weak cue (album|record|ep|lp) admits a capital/digit-led run OR a SINGLE +// lowercase token only; a MULTI-word lowercase title needs a STRONG cue (called|titled|named); a +// digit lead absorbs its full lowercase continuation. These assert BOTH directions — the precision +// floor (album path emits NOTHING on non-stop-set lowercase leads, the bug the old test missed) AND +// the recovered recall (digit continuation, single-token, strong-cue). --- + +test("albumMatchesIn: precision — weak-cue lowercase leads OUTSIDE the stop-set emit nothing", () => { + // The shipped leak: LOWER_RUN was guarded only by a 6-word stop-set, so any OTHER lowercase lead + // ('changed', 'dropped', 'makes', 'really', 'sounds') after a weak cue leaked a prose fragment. + for (const text of [ + "the album i bought yesterday", + "this record changed everything for me", + "their ep dropped on bandcamp last night", + "the album makes me cry", + "the album really grew on me", + "the album sounds great", + ]) { + assert.equal(albumMatchesIn(text).length, 0, `expected no album candidate from: ${text}`); + } + // The old stop-set precision cases must still hold too. + assert.equal(albumMatchesIn("the album by Carcass").length, 0); // verb/article left edge + assert.equal(albumMatchesIn("I think their new LP is amazing").length, 0); // lowercase filler before LP + assert.equal(albumMatchesIn('"We hold these truths to be self-evident."').length, 0); // prose quote +}); + +test("albumMatchesIn: recall — a digit-led title captures its full lowercase continuation", () => { + // The shipped bug: TITLE_RUN's [A-Z0-9] lead matched the bare digit and stopped, so '36 chambers' + // truncated to '36'. The digit-led run now absorbs trailing lowercase tokens (bounded to 4 words). + assert.ok(albumMatchesIn("the album 36 chambers").map((m) => m.name).includes("36 chambers")); + assert.ok(albumMatchesIn("the album 1989 deluxe").map((m) => m.name).includes("1989 deluxe")); + assert.ok( + albumMatchesIn("the album 808s and heartbreak").map((m) => m.name).includes("808s and heartbreak") + ); + // Prior digit recall cases (bare number, "X LP", capital-led mixed) still pass. + assert.ok(albumMatchesIn("the album 1989").map((m) => m.name).includes("1989")); + assert.ok(albumMatchesIn("1989 LP").map((m) => m.name).includes("1989")); + assert.ok(albumMatchesIn("the album 36 Chambers").map((m) => m.name).includes("36 Chambers")); +}); + +test("albumMatchesIn: recall — weak cue admits a SINGLE lowercase token (whole-title only)", () => { + // 'untitled' is the entire title — a lone lowercase token survives; a second lowercase word would + // be prose and is rejected (covered by the precision test above). + assert.ok(albumMatchesIn("the album untitled").map((m) => m.name).includes("untitled")); + assert.ok(albumMatchesIn("the album untitled.").map((m) => m.name).includes("untitled")); +}); + +test("albumMatchesIn: recall — a MULTI-word lowercase title needs a STRONG cue (called/titled/named)", () => { + // The ONLY place a bare multi-word lowercase run is admitted: an explicit naming verb. + assert.ok( + albumMatchesIn("an album called good kid maad city").map((m) => m.name).includes("good kid maad city") + ); + assert.ok(albumMatchesIn("the record titled good kid maad city").map((m) => m.name).includes("good kid maad city")); + assert.ok(albumMatchesIn("the album named good kid maad city").map((m) => m.name).includes("good kid maad city")); + // The same multi-word lowercase run WITHOUT a strong cue emits nothing (the precision invariant). + assert.equal(albumMatchesIn("the album good kid maad city").length, 0); +}); + +test("albumMatchesIn: convergence cases all map back to the source text", () => { + // Position-mapping invariant across the new precision/recall cases (digit continuation, single + // token, strong cue) — text.substr(index, length) === name. + const texts = [ + "the album 36 chambers", + "the album 1989 deluxe", + "the album 808s and heartbreak", + "the album untitled", + "the album untitled.", + "an album called good kid maad city", + "the record titled good kid maad city", + "the album Reign in Blood", // capital-led unchanged + "Reign in Blood LP", + "the album Money Store dropped", + ]; + for (const text of texts) { + for (const m of albumMatchesIn(text)) assert.equal(text.substr(m.index, m.length), m.name); + } +}); + +test("albumMatchesIn: no ReDoS — a pathological multi-KB input returns promptly", () => { + // Bounded runs ({0,3}, single linear lookahead) must stay linear. A worst-case repeat of the + // cue + many tokens must not blow up; assert it completes well under a generous wall-clock bound. + const evil = "the album " + "a ".repeat(20000) + "!".repeat(20000); + const t0 = Date.now(); + albumMatchesIn(evil); + assert.ok(Date.now() - t0 < 1000, "albumMatchesIn took too long on a pathological input"); +}); + test("isSafeHttpUrl accepts http/https and rejects javascript/data/garbage/empty", () => { // accepted: the only schemes a resolver link may inject as an href assert.ok(isSafeHttpUrl("https://open.spotify.com/artist/abc"));