Discovery final review fixes: lean publish, robust scoring, cheap prune (BL-0051)
- Cache prune counts directory entries first and only reads files when over the cap; tests for cross-kind prune, schema-mismatch and corrupt entries. - Scoring tolerates malformed manga.json and cached records (non-list tags, object or null relations, non-object items); a tag without a rank no longer passes the top-tag filter; authors without an integer AniList id are dropped. - discovery.json publishes only candidates some feed shows; examined/clipped still describe the pool. Tests that asserted the published count equals the pool size now check the pool via candidatePool() and the published set against the feeds. - The post-harvest rescore failure test now breaks discovery.json right before the publish (via a scraper stand-in) instead of relying on tags: 5, and a following successful harvest clears the stale lastError. - Consent text names everything sent to AniList; paused label mentions admin harvests; the UI test asserts the XSS seed instead of skipping it. - Slot-mix test proves the fresh/outside reservations matter. - Docs: privacy note, fresh fallback reason, per-period cap, dry-run wording, published-candidate rule; plan/spec hygiene and spec status. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
613db96355
commit
cd6a30a76b
18 files changed
+250
-65
No files matched your search
@@ -60,3 +60,14 @@ test('top tags weigh anchors, skip low rank and adult tags', () => {
|
||||
assert.deepEqual(topTagsFor(anchors, recordOf, { mature: true }), [
|
||||
{ name: 'Action', kind: 'genre' }, { name: 'Ecchi', kind: 'tag' }, { name: 'Alpha', kind: 'tag' }]);
|
||||
});
|
||||
|
||||
test('top tags drop tags without a rank and survive malformed records', () => {
|
||||
const records = {
|
||||
1: { tags: [{ name: 'NoRank', kind: 'tag', isAdult: false }, { name: 'NullRank', rank: null, kind: 'tag', isAdult: false },
|
||||
null, 7, { name: 'Kept', rank: 60, kind: 'tag', isAdult: false }] },
|
||||
2: { tags: 5 },
|
||||
3: { tags: { name: 'Object', rank: 90 } },
|
||||
};
|
||||
const anchors = [{ anilistId: 1, weight: 5 }, { anilistId: 2, weight: 3 }, { anilistId: 3, weight: 3 }];
|
||||
assert.deepEqual(topTagsFor(anchors, id => records[id]), [{ name: 'Kept', kind: 'tag' }]);
|
||||
});
|
||||
@@ -34,6 +34,14 @@ test('normalizeMedia maps fields and drops novels', () => {
|
||||
assert.equal(normalizeMedia({ ...media, status: 'HIATUS' }).status, 'unknown');
|
||||
});
|
||||
|
||||
test('normalizeMedia drops authors without an integer staff id', () => {
|
||||
const media = fixture('title').data.Media;
|
||||
const edge = (id, full) => ({ role: 'Story', node: { id, name: { full } } });
|
||||
const record = normalizeMedia({ ...media, staff: { edges: [edge(5, 'Has Id'), edge(undefined, 'No Id'),
|
||||
edge('6', 'String Id'), edge(1.5, 'Float Id'), edge(null, 'Null Id')] } });
|
||||
assert.deepEqual(record.authors, [{ staffId: 5, name: 'Has Id', role: 'Story' }]);
|
||||
});
|
||||
|
||||
test('title returns a record and sends the fixed headers', async () => {
|
||||
const { client, calls } = stub(() => json(fixture('title')));
|
||||
const record = await client.title(101);
|
||||
|
||||
@@ -49,6 +49,48 @@ test('prune keeps the newest maxFiles', (t) => {
|
||||
assert.notEqual(cache.get('title', 5), null);
|
||||
});
|
||||
|
||||
test('prune removes the oldest entries across kinds', (t) => {
|
||||
const { clock, cache } = setup(t, { maxFiles: 3 });
|
||||
const kinds = ['title', 'recs', 'creator', 'fresh', 'tagTop'];
|
||||
kinds.forEach((kind, i) => { clock.t += 1000; cache.put(kind, i, { i }); });
|
||||
assert.equal(cache.stats().files, 3);
|
||||
assert.equal(cache.get('title', 0), null);
|
||||
assert.equal(cache.get('recs', 1), null);
|
||||
for (const [i, kind] of kinds.entries()) if (i >= 2) assert.notEqual(cache.get(kind, i), null, kind);
|
||||
});
|
||||
|
||||
test('a write under the cap does not read other entries', (t) => {
|
||||
const { dir, cache } = setup(t, { maxFiles: 10 });
|
||||
cache.put('recs', 7, {});
|
||||
const file = path.join(dir, 'recs', fs.readdirSync(path.join(dir, 'recs'))[0]);
|
||||
fs.writeFileSync(file, '{broken');
|
||||
cache.put('title', 1, {});
|
||||
assert.equal(fs.readFileSync(file, 'utf8'), '{broken');
|
||||
});
|
||||
|
||||
test('over the cap, corrupt and schema-mismatch entries go before valid ones', (t) => {
|
||||
const { dir, clock, cache } = setup(t, { maxFiles: 2 });
|
||||
cache.put('recs', 1, {});
|
||||
cache.put('recs', 2, {});
|
||||
const [first, second] = fs.readdirSync(path.join(dir, 'recs')).map(name => path.join(dir, 'recs', name));
|
||||
fs.writeFileSync(first, '{broken');
|
||||
fs.writeFileSync(second, JSON.stringify({ schema: 2, kind: 'recs', key: 2, fetchedAt: new Date(clock.t).toISOString(), data: {} }));
|
||||
clock.t += 1000;
|
||||
cache.put('title', 3, {});
|
||||
assert.equal(fs.existsSync(first), false);
|
||||
assert.equal(fs.existsSync(second), false);
|
||||
assert.notEqual(cache.get('title', 3), null);
|
||||
});
|
||||
|
||||
test('schema-mismatch entry is deleted like a corrupt one', (t) => {
|
||||
const { dir, clock, cache } = setup(t);
|
||||
cache.put('recs', 7, {});
|
||||
const file = path.join(dir, 'recs', fs.readdirSync(path.join(dir, 'recs'))[0]);
|
||||
fs.writeFileSync(file, JSON.stringify({ schema: 2, kind: 'recs', key: 7, fetchedAt: new Date(clock.t).toISOString(), data: {} }));
|
||||
assert.equal(cache.get('recs', 7), null);
|
||||
assert.equal(fs.existsSync(file), false);
|
||||
});
|
||||
|
||||
test('corrupt entry is deleted', (t) => {
|
||||
const { dir, cache } = setup(t);
|
||||
cache.put('recs', 7, {});
|
||||
|
||||
@@ -50,7 +50,7 @@ const sha = s => crypto.createHash('sha256').update(s).digest('hex');
|
||||
const USERS = { boss: { admin: true, discovery: false }, alice: { discovery: true }, bob: { discovery: true }, carol: { discovery: false } };
|
||||
|
||||
// A disposable reader with ready-made accounts and sessions; nightly harvests are off unless the store says otherwise.
|
||||
async function startReader({ anilist, discovery = {}, mangaMeta = null } = {}) {
|
||||
async function startReader({ anilist, discovery = {}, mangaMeta = null, scraperUrl = 'http://127.0.0.1:9' } = {}) {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'mc-discovery-server-'));
|
||||
const library = path.join(root, 'library'), data = path.join(root, 'data');
|
||||
fs.mkdirSync(path.join(library, 'Example Manga'), { recursive: true });
|
||||
@@ -80,7 +80,7 @@ async function startReader({ anilist, discovery = {}, mangaMeta = null } = {}) {
|
||||
const port = await freePort(), output = [];
|
||||
const child = spawn(process.execPath, ['server.js'], { cwd: path.resolve(__dirname, '..'), windowsHide: true,
|
||||
env: { ...process.env, LIBRARY: library, DATA: data, PORT: String(port), SCAN_INTERVAL: '0', FULL_SCAN_INTERVAL: '0',
|
||||
SCRAPER_URL: 'http://127.0.0.1:9', SCRAPER_TOKEN: 'fixture-token', SCRAPER_TOKEN_FILE: '', BACKUP: '',
|
||||
SCRAPER_URL: scraperUrl, SCRAPER_TOKEN: 'fixture-token', SCRAPER_TOKEN_FILE: '', BACKUP: '',
|
||||
DISCOVERY_ANILIST_URL: anilist ? anilist.url : 'http://127.0.0.1:9/', DISCOVERY_HARVEST_AT: '' } });
|
||||
child.stdout.on('data', d => output.push(String(d)));
|
||||
child.stderr.on('data', d => output.push(String(d)));
|
||||
@@ -216,22 +216,55 @@ test('admin harvest is admin-only and single-flight', { timeout: 60000 }, async
|
||||
} finally { await reader.stop(); await anilist.close(); }
|
||||
});
|
||||
|
||||
// A scraper stand-in. The rescore asks it for the queue after the harvest's final store patch and
|
||||
// right before publishing, so onState can break discovery.json at exactly that point.
|
||||
async function startScraper(onState) {
|
||||
const server = http.createServer((req, res) => {
|
||||
if (req.url.startsWith('/api/state')) onState();
|
||||
res.writeHead(200, { 'content-type': 'application/json' });
|
||||
res.end(JSON.stringify({ queue: [] }));
|
||||
});
|
||||
await new Promise(resolve => server.listen(0, '127.0.0.1', resolve));
|
||||
return { url: `http://127.0.0.1:${server.address().port}`, close: () => new Promise(resolve => server.close(resolve)) };
|
||||
}
|
||||
|
||||
test('a failed rescore after a harvest reaches the admin as lastError', { timeout: 60000 }, async () => {
|
||||
const anilist = await startAniList();
|
||||
// A manga.json whose tags is not a list makes scoring throw; it stands in for any scoring fault.
|
||||
const reader = await startReader({ anilist, mangaMeta: { title: 'Broken Tags', tags: 5 } });
|
||||
let reader, armed = false, saved = null;
|
||||
const scraper = await startScraper(() => {
|
||||
if (!armed) return;
|
||||
armed = false;
|
||||
const file = path.join(reader.data, 'discovery.json');
|
||||
saved = fs.readFileSync(file, 'utf8');
|
||||
fs.writeFileSync(file, '{broken'); // the publish now refuses to overwrite it
|
||||
});
|
||||
// A malformed manga.json (tags not a list, relations an object) must not be what breaks scoring.
|
||||
reader = await startReader({ anilist, scraperUrl: scraper.url,
|
||||
mangaMeta: { title: 'Broken Tags', tags: 5, 'x-anilist': { id: 101, relations: {} } } });
|
||||
try {
|
||||
await until(async () => (await reader.get('boss', '/api/library')).body?.mangas?.length === 1, 'the library scan');
|
||||
assert.equal((await reader.post('boss', '/api/admin/discovery/harvest')).status, 200);
|
||||
const done = await harvestDone(reader);
|
||||
assert.equal(done.harvest.kind, 'admin');
|
||||
armed = true;
|
||||
const first = await reader.post('boss', '/api/admin/discovery/harvest');
|
||||
assert.equal(first.status, 200);
|
||||
await until(async () => saved && reader.output.join('').includes(`${first.body.harvestId} follow-up failed`), 'the failed rescore');
|
||||
fs.writeFileSync(path.join(reader.data, 'discovery.json'), saved);
|
||||
const admin = await until(async () => {
|
||||
const { body } = await reader.get('boss', '/api/admin/discovery');
|
||||
return body.lastError ? body : null;
|
||||
}, 'the rescore error');
|
||||
assert.match(admin.lastError, /^Re-scoring after harvest [0-9a-f]{16} failed: /);
|
||||
assert.match(admin.lastError, /^Re-scoring after harvest [0-9a-f]{16} failed: Discovery store unavailable/);
|
||||
assert.equal(admin.currentRun, null);
|
||||
} finally { await reader.stop(); await anilist.close(); }
|
||||
assert.equal(admin.harvest.kind, 'admin');
|
||||
|
||||
// The next harvest's successful rescore clears the stale error.
|
||||
const second = await reader.post('boss', '/api/admin/discovery/harvest');
|
||||
assert.equal(second.status, 200, JSON.stringify(second.body));
|
||||
const cleared = await until(async () => {
|
||||
const { body } = await reader.get('boss', '/api/admin/discovery');
|
||||
return body.harvest?.id === second.body.harvestId && !body.currentHarvest && body.lastError === null ? body : null;
|
||||
}, 'the error to clear');
|
||||
assert.ok(cleared.candidateCount > 0);
|
||||
} finally { await reader.stop(); await anilist.close(); await scraper.close(); }
|
||||
});
|
||||
|
||||
test('settings validation', { timeout: 30000 }, async () => {
|
||||
|
||||
+69
-7
@@ -4,7 +4,7 @@ const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const os = require('node:os');
|
||||
const path = require('node:path');
|
||||
const { generate, generateAsync, buildCandidates, feedForUser, mixSlots, mergeUserFeed, MAX_CANDIDATES, SLOTS } = require('../discovery/engine');
|
||||
const { generate, generateAsync, buildCandidates, candidatePool, feedForUser, mixSlots, mergeUserFeed, MAX_CANDIDATES, SLOTS } = require('../discovery/engine');
|
||||
const { DiscoveryStore } = require('../discovery/store');
|
||||
const { DiscoveryCache } = require('../discovery/cache');
|
||||
|
||||
@@ -16,12 +16,17 @@ test('discovery is bounded, diversified, opt-in, evidence-linked and user-specif
|
||||
off: { name: 'off', settings: {} } };
|
||||
const db = { users, greatestRatings: { alice: { [greatest.entries[0].id]: 'flame' },
|
||||
bob: { [greatest.entries[20].id]: 'flame' } }, discoveryFeedback: { alice: {} }, progress: {} };
|
||||
const result = generate({ db, library: [], greatest, cacheDir: path.join(os.tmpdir(), 'nonexistent-discovery-cache') });
|
||||
assert.equal(Object.keys(result.candidates).length, Math.min(2000, result.examined));
|
||||
const cacheDir = path.join(os.tmpdir(), 'nonexistent-discovery-cache');
|
||||
const result = generate({ db, library: [], greatest, cacheDir });
|
||||
// Only candidates a feed shows are published; the pool itself stays bounded at 2000.
|
||||
const shown = new Set(Object.values(result.feeds).flatMap(feed => (feed.items || []).map(item => item.candidateId)));
|
||||
assert.deepEqual(Object.keys(result.candidates).sort(), [...shown].sort());
|
||||
const pool = candidatePool({ greatest, library: [], cacheDir });
|
||||
assert.equal(pool.candidates.length, Math.min(2000, result.examined));
|
||||
assert.ok(result.feeds.alice.items.length > 0 && result.feeds.alice.items.length <= 20);
|
||||
assert.ok(result.feeds.bob.items.length > 0);
|
||||
assert.equal(result.feeds.off.disabled, true);
|
||||
assert.ok(new Set(Object.values(result.candidates).map(c => c.period)).size >= 10, 'An early catalog page must not crowd out later eras');
|
||||
assert.ok(new Set(pool.candidates.map(c => c.period)).size >= 10, 'An early catalog page must not crowd out later eras');
|
||||
assert.ok(result.feeds.alice.items.every(item => item.evidenceIds.every(id => item.evidence.some(e => e.id === id))));
|
||||
assert.ok(result.feeds.alice.items.some(item => item.evidence.some(e => e.kind === 'personal')));
|
||||
assert.ok(!result.feeds.bob.items.some(item => item.evidence.some(e => e.label?.includes(greatest.entries[0].title) && e.kind === 'personal')));
|
||||
@@ -144,10 +149,48 @@ test('engine: empty harvest cache equals Phase A pool', t => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'mc-discovery-empty-'));
|
||||
t.after(() => fs.rmSync(dir, { recursive: true, force: true }));
|
||||
const db = { users: {}, greatestRatings: {}, discoveryFeedback: {}, progress: {} };
|
||||
const without = generate({ db, library: [], greatest, cacheDir: NO_SCRAPER_CACHE });
|
||||
const withEmpty = generate({ db, library: [], greatest, cacheDir: NO_SCRAPER_CACHE, harvestCache: new DiscoveryCache(dir) });
|
||||
const without = candidatePool({ greatest, library: [], cacheDir: NO_SCRAPER_CACHE });
|
||||
const withEmpty = candidatePool({ greatest, library: [], cacheDir: NO_SCRAPER_CACHE, harvestCache: new DiscoveryCache(dir) });
|
||||
assert.equal(MAX_CANDIDATES, 2000);
|
||||
assert.deepEqual(Object.keys(withEmpty.candidates).sort(), Object.keys(without.candidates).sort());
|
||||
assert.deepEqual(withEmpty.candidates.map(c => c.id).sort(), without.candidates.map(c => c.id).sort());
|
||||
const users = { alice: { name: 'alice', settings: { discoveryEnabled: true } } };
|
||||
const run = harvestCache => generate({ db: { ...db, users }, library: [], greatest, cacheDir: NO_SCRAPER_CACHE, harvestCache });
|
||||
assert.deepEqual(Object.keys(run(new DiscoveryCache(dir)).candidates).sort(), Object.keys(run(null).candidates).sort());
|
||||
});
|
||||
|
||||
test('published candidates are only those referenced by a feed', async t => {
|
||||
const world = fixtureWorld(t);
|
||||
world.db.users.bob = { name: 'bob', settings: { discoveryEnabled: true } };
|
||||
world.db.users.off = { name: 'off', settings: {} };
|
||||
const args = { db: world.db, library: world.library, greatest: world.greatest, cacheDir: NO_SCRAPER_CACHE,
|
||||
harvestCache: world.harvestCache, queuedTitles: world.queuedTitles, now: NOW };
|
||||
const referenced = result => new Set(Object.values(result.feeds).flatMap(feed => (feed.items || []).map(item => item.candidateId)));
|
||||
for (const result of [generate(args), await generateAsync(args), await generateAsync({ ...args, onlyUser: 'alice' })]) {
|
||||
const shown = referenced(result);
|
||||
assert.ok(shown.size > 0);
|
||||
for (const id of Object.keys(result.candidates)) assert.ok(shown.has(id), `${id} is published but not shown`);
|
||||
for (const id of shown) assert.ok(result.candidates[id], `${id} is shown but not published`);
|
||||
assert.ok(result.examined > Object.keys(result.candidates).length, 'examined still counts the whole pool');
|
||||
assert.equal(result.clipped, 0);
|
||||
}
|
||||
});
|
||||
|
||||
test('malformed library metadata and cache records score without throwing', t => {
|
||||
const world = fixtureWorld(t, { discoveryReadingActivity: true });
|
||||
world.library.push(
|
||||
{ id: 'm2', title: 'Bad Tags', meta: { tags: 5, 'x-anilist': { id: 220, relations: {} } } },
|
||||
{ id: 'm3', title: 'Bad Relations', meta: { tags: [7, null, { name: 'Drama' }, 'Drama'],
|
||||
'x-anilist': { id: 221, relations: [null, 3, 'x', { id: 222, format: 'MANGA', type: 'SEQUEL', title: 'Sequel Title' }] } } },
|
||||
{ id: 'm4', title: 'Bad Identity', meta: { tags: { a: 1 }, 'x-anilist': 5 } });
|
||||
world.db.progress = { alice: { m2: { c1: { done: true } }, m3: { c1: { done: true } } } };
|
||||
world.harvestCache.put('title', 220, seedRecord(220, { authors: [null, 5, AUTHOR], tags: [null, 'Drama', { name: 'Drama', kind: 'genre' }] }));
|
||||
world.harvestCache.put('recs', 220, [null, 5, { ...seed(230), rating: 7 }]);
|
||||
const args = { db: world.db, library: world.library, greatest: world.greatest, cacheDir: NO_SCRAPER_CACHE,
|
||||
harvestCache: world.harvestCache, queuedTitles: world.queuedTitles, now: NOW };
|
||||
const result = generate(args);
|
||||
const ids = result.feeds.alice.items.map(item => item.candidateId);
|
||||
assert.ok(ids.includes('anilist:222'), JSON.stringify(ids));
|
||||
assert.ok(ids.includes('anilist:230'), JSON.stringify(ids));
|
||||
});
|
||||
|
||||
test('harvested records join the pool and fill Greatest candidates', t => {
|
||||
@@ -262,6 +305,25 @@ test('slot mix', () => {
|
||||
assert.ok(full.filter(r => r.group === '1980s').length <= SLOTS.perPeriod);
|
||||
});
|
||||
|
||||
test('slot mix reservations let low-scored fresh and outside rows past a full feed', () => {
|
||||
const row = (id, score, group, extra = {}) => ({ candidateId: `anilist:${id}`, score, group, tags: ['Drama'],
|
||||
scoreParts: { fresh: 0 }, ...extra });
|
||||
// 32 higher-scored rows that the caps alone would allow: 8 per seed kind, 4 per period.
|
||||
const rows = [];
|
||||
const groups = ['recs', 'creator', 'tagTop', '1980s', '1990s'];
|
||||
const caps = { recs: 8, creator: 8, tagTop: 8, '1980s': 4, '1990s': 4 };
|
||||
let id = 5000;
|
||||
for (const group of groups) for (let i = 0; i < caps[group]; i++) rows.push(row(id++, 100 - rows.length, group));
|
||||
assert.ok(rows.length > 20);
|
||||
rows.push(row(2001, 1, 'fresh', { scoreParts: { fresh: 1 } }), row(2002, 0.9, 'fresh', { scoreParts: { fresh: 1 } }));
|
||||
rows.push(row(3001, 0.2, '2000s', { tags: ['Sports'] }), row(3002, 0.1, '2000s', { tags: [] }));
|
||||
const picked = mixSlots(rows, { topTags: [{ name: 'Drama', kind: 'genre' }] });
|
||||
assert.equal(picked.length, 20);
|
||||
for (const low of ['anilist:2001', 'anilist:2002', 'anilist:3001', 'anilist:3002'])
|
||||
assert.ok(picked.some(r => r.candidateId === low), `${low} needs its reserved slot`);
|
||||
for (const group of groups) assert.ok(picked.filter(r => r.group === group).length <= caps[group], group);
|
||||
});
|
||||
|
||||
test('mergeUserFeed keeps other users\' referenced candidates', () => {
|
||||
const prior = { candidates: { a: { id: 'a' }, b: { id: 'b' }, c: { id: 'c' } },
|
||||
feeds: { alice: { items: [{ candidateId: 'a' }] }, bob: { items: [{ candidateId: 'b' }] }, off: { items: [], disabled: true } } };
|
||||
|
||||
Reference in new issue
Block a user