fix(topics): label a topic by its most-used spelling, not min()

packages/db/src/accounts.js

@@ -1,4 +1,5 @@
 import { newId, nowIso } from './client.js';
+import { topicLabelSql } from './topicLabel.js';
 
 /**
  * Everything the accounts layer reads and writes.
@@ -518,7 +519,7 @@ export async function followedTopics(db, userId, limit = 200) {
                  -- The topic's own spelling, so a page can say "AI" rather than
                  -- "ai". Left join: a topic whose feeds have all died still has
                  -- a follow, and the slug is a serviceable fallback.
-                 (select min(k.keyword) from feed_keywords k where k.slug = tf.slug) as keyword
+                 ${topicLabelSql('tf.slug')} as keyword
           from topic_follows tf
           where tf.user_id = ?
           order by tf.created_at desc

packages/db/src/alerts.js

@@ -1,5 +1,6 @@
 import { newId, nowIso } from './client.js';
 import { normalizeSegment } from './accounts.js';
+import { topicLabelSql } from './topicLabel.js';
 
 /**
  * Alerts: the reads and writes behind being told about a new post.
@@ -196,7 +197,7 @@ export async function alertingFollows(db, userId) {
     }),
     db.execute({
       sql: `select tf.slug, tf.segment,
-                   (select min(k.keyword) from feed_keywords k where k.slug = tf.slug) as keyword
+                   ${topicLabelSql('tf.slug')} as keyword
             from topic_follows tf
             where tf.user_id = ? and tf.alerts = 1
             order by tf.created_at desc`,
@@ -628,7 +629,7 @@ export async function newItemsFromAlertedFeeds(db, userId, cursor, limit = 50) {
 export async function alertedTopics(db, userId, limit = 50) {
   const { rows } = await db.execute({
     sql: `select tf.slug, tf.segment,
-                 (select min(k.keyword) from feed_keywords k where k.slug = tf.slug) as keyword
+                 ${topicLabelSql('tf.slug')} as keyword
           from topic_follows tf
           where tf.user_id = ? and tf.alerts = 1
           order by tf.created_at desc

packages/db/src/queries.js

@@ -1,6 +1,7 @@
 import { clusterKey, dedupeItems, topicSlug } from '@rssamplifier/feed';
 
 import { newId, nowIso } from './client.js';
+import { topicLabelSql } from './topicLabel.js';
 
 /**
  * Every query the app needs, in one place.
@@ -1092,12 +1093,10 @@ export async function keywordsForFeed(db, feedId, limit = 12) {
 export async function topicBySlug(db, slug) {
   const { rows } = await db.execute({
     sql: `select k.slug,
-                 -- Any spelling will do as the display name: extraction
-                 -- lowercases its keywords and categories are lowercased before
-                 -- they are stored, so the rows under one slug differ only in
-                 -- ways the slug already erased. Same rule as the rollup, so
-                 -- the index and the page always agree on the title.
-                 min(k.keyword) as keyword,
+                 -- The most-used spelling, not any spelling: see topicLabel.js.
+                 -- Same rule as the rollup, so the index and the page always
+                 -- agree on the title.
+                 ${topicLabelSql('k.slug')} as keyword,
                  count(*) as feed_count
           from feed_keywords k
           join feeds f on f.id = k.feed_id and f.status <> 'dead'
@@ -1454,6 +1453,11 @@ export async function countTopics(db, minFeeds = 2, query = null) {
  * blog's vocabulary, they are the overwhelming majority of the rows, and a page
  * listing one feed is not a topic page.
  *
+ * The displayed keyword is the most-used spelling of the slug, chosen by the
+ * shared rule in `topicLabel.js` rather than by this query — the topic page,
+ * the alert queries and the followed-topics list all have to agree with it, and
+ * they did not when each picked its own.
+ *
  * @param {Client} db
  * @param {number} [minFeeds]
  * @returns {Promise<number>} topics in the rollup
@@ -1467,7 +1471,7 @@ export async function refreshTopics(db, minFeeds = 2) {
       {
         sql: `insert into topics (slug, keyword, feed_count, refreshed_at)
               select k.slug,
-                     min(k.keyword),
+                     ${topicLabelSql('k.slug')},
                      count(distinct k.feed_id),
                      ?
               from feed_keywords k

packages/db/src/topicLabel.js

@@ -0,0 +1,54 @@
+/**
+ * The one place that decides what a topic is *called*.
+ *
+ * Many raw category strings slug down to the same topic, so something has to
+ * pick the label from among them. Five queries used to pick it independently
+ * with `min(keyword)` — the rollup, the topic page, both alert queries and the
+ * followed-topics list — and all five were wrong in the same way.
+ *
+ * `min()` is a *lexicographic* minimum, and in ASCII `!`(0x21) `"`(0x22)
+ * `*`(0x2A) `.`(0x2E) `/`(0x2F) `[`(0x5B) and `_`(0x5F) all sort before
+ * lowercase letters. So whenever one publisher wrote a malformed `<category>`
+ * tag, their spelling won the label for everybody. Measured on prod:
+ *
+ *   slug `news` displayed as `! news`    — 1 feed, against 10,310 spelling it `news`
+ *   slug `en`   displayed as `["en"]`    — 1 feed, against 13,130
+ *   slug `ai`   displayed as `"ai"`      — 1 feed, against 11,051
+ *   slug `post` displayed as `_posts`    — 1 feed, against 11,774
+ *   slug `life` displayed as `/life`     — 4 feeds, against 8,985
+ *
+ * The old comment on `topicBySlug` explained the choice as "any spelling will
+ * do … the rows under one slug differ only in ways the slug already erased".
+ * That is the premise that made this invisible, and it is false in exactly one
+ * direction: `topicSlug` strips punctuation, `feed_keywords.keyword` keeps it,
+ * so the surviving differences are precisely the ugly ones.
+ *
+ * Ranking by `count(distinct feed_id)` lets the directory's own usage decide.
+ * It needs no stoplist, it cannot be captured by a single feed, and it improves
+ * on its own as feeds are added. The tie-breaks handle what a count cannot:
+ * shortest first, so `ai` beats `ai,` and `ai:` at equal counts, then
+ * lexicographic so the result is deterministic.
+ *
+ * This is a display concern only. `topicSlug` already normalised these away, so
+ * URLs, grouping and counts were right the whole time — only the label on top
+ * of them was wrong.
+ */
+
+/**
+ * A correlated scalar subquery returning the most-used spelling of one slug.
+ *
+ * `slugExpr` is interpolated as SQL, so it must be a column reference the
+ * caller controls (`k.slug`, `tf.slug`) — never a value, and never anything
+ * derived from user input. Values still go through bound parameters.
+ *
+ * @param {string} slugExpr SQL expression naming the slug to label
+ * @returns {string} SQL scalar subquery
+ */
+export function topicLabelSql(slugExpr) {
+  return `(select lk.keyword
+             from feed_keywords lk
+            where lk.slug = ${slugExpr}
+            group by lk.keyword
+            order by count(distinct lk.feed_id) desc, length(lk.keyword) asc, lk.keyword asc
+            limit 1)`;
+}

packages/db/test/topic-label.test.js

@@ -0,0 +1,66 @@
+/**
+ * The label a topic wears is the spelling most feeds actually use.
+ *
+ * Regression test for the `min(keyword)` rollup: a lexicographic minimum let
+ * one publisher's malformed `<category>` tag rename a topic carried by
+ * thousands of blogs, because `!`, `"`, `/`, `[` and `_` all sort before
+ * lowercase letters in ASCII.
+ */
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import { mkdtemp, rm } from 'node:fs/promises';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+
+import { connect } from '../src/client.js';
+import { migrate } from '../src/migrate.js';
+import * as q from '../src/queries.js';
+
+/** Add `n` feeds that all spell the same slug the same way. */
+async function feedsSpelling(db, slug, keyword, n, tag) {
+  for (let i = 0; i < n; i += 1) {
+    const feed = await q.insertFeed(db, {
+      slug: `${tag}-${i}`,
+      feed_url: `https://${tag}${i}.example/feed.xml`,
+      title: `${tag} ${i}`,
+      kind: 'blog',
+    });
+    await q.replaceFeedKeywords(db, feed.id, [
+      { slug, keyword, words: 1, count: 3, source: 'category' },
+    ]);
+  }
+}
+
+test('a punctuation-prefixed spelling from one feed cannot rename a topic', async () => {
+  const dir = await mkdtemp(join(tmpdir(), 'rssamp-topiclabel-'));
+  const db = connect({ url: `file:${join(dir, 'label.db')}` });
+  await migrate(db);
+
+  // The real prod shape, scaled down: many feeds say "news", exactly one says
+  // "! news", and both slug to `news`.
+  await feedsSpelling(db, 'news', 'news', 5, 'plain');
+  await feedsSpelling(db, 'news', '! news', 1, 'punct');
+
+  await q.refreshTopics(db);
+
+  const topic = await q.topicBySlug(db, 'news');
+  assert.equal(topic.keyword, 'news', 'the majority spelling wins the label');
+  assert.equal(topic.feedCount, 6, 'every spelling still counts toward the topic');
+
+  await rm(dir, { recursive: true, force: true });
+});
+
+test('trailing punctuation loses to the bare word on a tie-break', async () => {
+  const dir = await mkdtemp(join(tmpdir(), 'rssamp-topictie-'));
+  const db = connect({ url: `file:${join(dir, 'tie.db')}` });
+  await migrate(db);
+
+  // Equal feed counts, so the count cannot decide: shortest must.
+  await feedsSpelling(db, 'ai', 'ai:', 2, 'colon');
+  await feedsSpelling(db, 'ai', 'ai', 2, 'bare');
+
+  await q.refreshTopics(db);
+  assert.equal((await q.topicBySlug(db, 'ai')).keyword, 'ai');
+
+  await rm(dir, { recursive: true, force: true });
+});