From 71721439732b0e1914f394d56456bd915b5a3f90 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Sat, 29 Aug 2026 15:13:40 +0000 Subject: [PATCH] fix(topics): label a topic by its most-used spelling, not min() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /topics was full of labels like `! news`, `["en"]`, `_posts` and `/life`. None of them were extraction bugs: `topicSlug` had normalised every one of them away correctly, and the grouping, URLs and counts were right the whole time. Only the display name was wrong. Five queries picked that name independently with `min(keyword)` — the rollup, the topic page, both alert queries and the followed-topics list. `min()` is a lexicographic minimum, and in ASCII `!` `"` `*` `.` `/` `[` and `_` all sort before lowercase letters, so a single publisher's malformed tag won the label for everybody. Measured on prod: slug `news` showed as `! news` — 1 feed, against 10,310 spelling it `news` slug `en` showed as `["en"]` — 1 feed, against 13,130 slug `post` showed as `_posts` — 1 feed, against 11,774 slug `life` showed as `/life` — 4 feeds, against 8,985 12,318 of 152,814 topics carry such a label, concentrated at the top of the directory by feed count. The rule now lives once, in topicLabel.js, and ranks by count(distinct feed_id): the directory's own usage decides, which needs no stoplist and cannot be captured by one feed. Tie-breaks are shortest-first (so `ai` beats `ai,` and `ai:`) then lexicographic for determinism. The old comment on topicBySlug is why this stayed invisible — it reasoned that "any spelling will do … the rows under one slug differ only in ways the slug already erased". The slug strips punctuation and the keyword keeps it, so the surviving differences are exactly the ugly ones. Not addressed here: topics that are junk in themselves rather than mislabelled — `#39` and `rsquo` from undecoded HTML entities, and non-English grammar words from an English-only stoplist. Those are extraction-side and need a re-crawl, not a rollup. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TGaU2kAfCZSNxH6pvW5ZZQ --- packages/db/src/accounts.js | 3 +- packages/db/src/alerts.js | 5 ++- packages/db/src/queries.js | 18 +++++--- packages/db/src/topicLabel.js | 54 +++++++++++++++++++++++ packages/db/test/topic-label.test.js | 66 ++++++++++++++++++++++++++++ 5 files changed, 136 insertions(+), 10 deletions(-) create mode 100644 packages/db/src/topicLabel.js create mode 100644 packages/db/test/topic-label.test.js diff --git a/packages/db/src/accounts.js b/packages/db/src/accounts.js index d0ed394..fb86ac0 100644 --- a/packages/db/src/accounts.js +++ b/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 diff --git a/packages/db/src/alerts.js b/packages/db/src/alerts.js index f68cd2c..5b852d0 100644 --- a/packages/db/src/alerts.js +++ b/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 diff --git a/packages/db/src/queries.js b/packages/db/src/queries.js index 9c7a007..29e64cd 100644 --- a/packages/db/src/queries.js +++ b/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} 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 diff --git a/packages/db/src/topicLabel.js b/packages/db/src/topicLabel.js new file mode 100644 index 0000000..2073c05 --- /dev/null +++ b/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 `` + * 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)`; +} diff --git a/packages/db/test/topic-label.test.js b/packages/db/test/topic-label.test.js new file mode 100644 index 0000000..a5f55fc --- /dev/null +++ b/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 `` 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 }); +});