diff --git a/packages/ingest/src/crawl.js b/packages/ingest/src/crawl.js index afcb569..4fd693f 100644 --- a/packages/ingest/src/crawl.js +++ b/packages/ingest/src/crawl.js @@ -322,7 +322,6 @@ export async function crawlFeed(db, feed, opts = {}) { // same set without the read-after-write that used to force a second // transaction. let storedItems = []; - let existingTopics = 1; let hasAuthors = true; /** @type {Array<{ slug: string, keyword: string, words: number, count: number, source: string }>} */ let storedTopics = []; @@ -335,14 +334,32 @@ export async function crawlFeed(db, feed, opts = {}) { q.feedKeywordRows(db, id).catch(() => []), authors.feedHasAuthors(db, id).catch(() => true), ]); - existingTopics = storedTopics.length; } - // Topics, when the feed has published something or has none yet. + // Topics, re-derived on every crawl. + // + // This used to be gated on `publishedSomethingNew || existingTopics === 0`, + // which asked the wrong question. Topics come from `topicsFrom(feed, items)` + // -- the *channel's* own categories, title and description as well as the + // items -- so a publisher who retags their feed, renames it, or rewrites its + // description has changed its topics without publishing anything at all. The + // old guard could not see that, and a feed that went quiet was pinned to + // whatever it was about on the last day it posted. `feeds.category` has + // always been re-derived on every successful crawl for exactly this reason + // (`upsertFeed` writes it unconditionally); topics now agree with it. + // + // This is affordable because of the diff below, not in spite of it. The three + // reads it needs are already issued above whenever auxiliary writes are on, + // so re-deriving adds no round trip -- `topicsFrom` is pure computation and + // `keywordDiffStatements` returns an empty array when the extracted set + // matches what is stored, which for a quiet feed is every time. The cost that + // forced `CRAWL_AUXILIARY_WRITES=0` is *first* crawls, where every topic is a + // genuine insert and no diff can help; that is a property of the backlog and + // is unchanged by this. let topics = 0; /** @type {Array<{ sql: string, args: unknown[] }>} */ let topicStatements = []; - if (AUXILIARY_WRITES && (publishedSomethingNew || existingTopics === 0)) { + if (AUXILIARY_WRITES) { try { const extracted = topicsFrom(resolved.feed, storedItems); // A diff rather than a replace. Most re-crawls extract the topics the diff --git a/packages/ingest/test/recrawl.test.js b/packages/ingest/test/recrawl.test.js index 13a3edf..0a892ab 100644 --- a/packages/ingest/test/recrawl.test.js +++ b/packages/ingest/test/recrawl.test.js @@ -6,7 +6,7 @@ import { join } from 'node:path'; import { connect, migrate, newId, q } from '@rssamplifier/db'; -import { crawlFeed, nextIntervalMinutes } from '../src/crawl.js'; +import { crawlFeed, nextIntervalMinutes, topicsFrom } from '../src/crawl.js'; /** * What a re-crawl reports, and what the crawler does about it. @@ -519,3 +519,63 @@ test('a feed that fills up after being empty is noticed at once', async () => { const after = await q.feedBySlug(db, 'quiet'); assert.equal(JSON.parse(after.change_log).length, 2, 'and it is recorded as a change'); }); + +test('a feed that is retagged without publishing still has its topics revised', async () => { + // The case the old guard could not see. + // + // Topics were recomputed only when the feed had published something new, or + // had none yet. But `topicsFrom` reads the *channel's* own categories, title + // and description as well as its items, so a publisher can change what their + // feed is about without posting: retag it, rename it, rewrite the standfirst. + // For a blog that then goes quiet, the old condition was never true again and + // the directory filed it under whatever it happened to be about on the last + // day it posted. + // + // `feeds.category` never had this problem -- `upsertFeed` re-derives it on + // every successful crawl. This asserts the topics now agree with it. + const retagged = async () => ({ + ok: true, + feed: { ...DOCUMENT.feed, categories: ['astronomy'] }, + }); + + const feed = await seed(); + await crawlFeed(db, feed, { resolve }); + + const before = (await q.feedKeywordRows(db, feed.id)).map((r) => String(r.slug)); + assert.ok(before.includes('writing'), `the first crawl files the tag it was given: ${before}`); + + // The same five posts, same dates, nothing new to store -- only the channel's + // tag has changed. This is the crawl the old code skipped. + const res = await crawlFeed(db, await q.feedBySlug(db, 'quiet'), { resolve: retagged }); + assert.equal(res.newItems, 0, 'nothing new was published'); + + const after = (await q.feedKeywordRows(db, feed.id)).map((r) => String(r.slug)); + assert.ok(after.includes('astronomy'), `the new tag is filed: ${after}`); +}); + +test('a quiet feed that has not changed writes no topic rows at all', async () => { + // The other half of the trade, and the reason re-deriving every crawl is + // affordable: `keywordDiffStatements` compares the extracted set against the + // stored one and emits nothing when they match. Re-deriving costs computation + // on a document already parsed and reads already issued -- not writes. + // + // The diff is the witness. `feed_keywords` carries no timestamp, so asserting + // on the rows cannot distinguish "rewritten to the same values" from "left + // alone" -- which is the exact indistinguishability that hid this cost in the + // first place. Asking `keywordDiffStatements` what it would emit is the + // claim itself, not a proxy for it. + const feed = await seed(); + await crawlFeed(db, feed, { resolve }); + + const stored = await q.feedKeywordRows(db, feed.id); + assert.ok(stored.length > 0, 'the first crawl filed some topics'); + + // Exactly what the crawl recomputes on the next pass: the same document + // against the items now stored for it. + const again = topicsFrom(DOCUMENT.feed, await q.itemsForKeywords(db, feed.id)); + assert.deepEqual( + q.keywordDiffStatements(feed.id, again, stored), + [], + 'an unchanged re-crawl emits no topic write at all', + ); +});