diff --git a/.gitignore b/.gitignore index fd8dcc737..3fc289cba 100644 --- a/.gitignore +++ b/.gitignore @@ -77,6 +77,7 @@ api-cache.json verbose-mode.json skills-lock.json tmp/ +.context/ # Local planning documents (not for public repo) docs/plans/ diff --git a/.markdownlint-cli2.jsonc b/.markdownlint-cli2.jsonc index ba7fccb5e..8fe0887e5 100644 --- a/.markdownlint-cli2.jsonc +++ b/.markdownlint-cli2.jsonc @@ -6,5 +6,5 @@ "MD022": true, "MD032": true }, - "ignores": ["node_modules/**", "dist/**", "src-tauri/target/**", ".planning/**", "DMCA-TAKEDOWN-NOTICE.md"] + "ignores": ["node_modules/**", "dist/**", "src-tauri/target/**", ".planning/**", ".context/**", "DMCA-TAKEDOWN-NOTICE.md"] } diff --git a/scripts/lib/brief-compose.mjs b/scripts/lib/brief-compose.mjs index 605252511..52c637666 100644 --- a/scripts/lib/brief-compose.mjs +++ b/scripts/lib/brief-compose.mjs @@ -32,8 +32,15 @@ function compareRules(a, b) { const aFull = a.variant === 'full' ? 0 : 1; const bFull = b.variant === 'full' ? 0 : 1; if (aFull !== bFull) return aFull - bFull; - const aRank = SENSITIVITY_RANK[a.sensitivity ?? 'all'] ?? 0; - const bRank = SENSITIVITY_RANK[b.sensitivity ?? 'all'] ?? 0; + // Default missing sensitivity to 'high' (NOT 'all') so the rank + // matches what compose/buildDigest/cache/log actually treat the + // rule as. Otherwise a legacy undefined-sensitivity rule would be + // ranked as the most-permissive 'all' and tried first, but compose + // would then apply a 'high' filter — shipping a narrow brief while + // an explicit 'all' rule for the same user is never tried. + // See PR #3387 review (P2). + const aRank = SENSITIVITY_RANK[a.sensitivity ?? 'high'] ?? 0; + const bRank = SENSITIVITY_RANK[b.sensitivity ?? 'high'] ?? 0; if (aRank !== bRank) return aRank - bRank; return (a.updatedAt ?? 0) - (b.updatedAt ?? 0); } @@ -161,7 +168,10 @@ const MAX_STORIES_PER_USER = 12; * @param {{ nowMs: number }} [opts] */ export function composeBriefForRule(rule, insights, { nowMs = Date.now() } = {}) { - const sensitivity = rule.sensitivity ?? 'all'; + // Default to 'high' (NOT 'all') for parity with composeBriefFromDigestStories, + // buildDigest, the digestFor cache key, and the per-attempt log line. + // See PR #3387 review (P2). + const sensitivity = rule.sensitivity ?? 'high'; const tz = rule.digestTimezone ?? 'UTC'; const stories = filterTopStories({ stories: insights.topStories, @@ -272,17 +282,30 @@ function digestStoryToUpstreamTopStory(s) { * @param {object} rule — enabled alertRule row * @param {unknown[]} digestStories — output of buildDigest(rule, windowStart) * @param {{ clusters: number; multiSource: number }} insightsNumbers - * @param {{ nowMs?: number }} [opts] + * @param {{ nowMs?: number, onDrop?: import('../../shared/brief-filter.js').DropMetricsFn }} [opts] + * `onDrop` is forwarded to filterTopStories so the seeder can + * aggregate per-user filter-drop counts without this module knowing + * how they are reported. */ -export function composeBriefFromDigestStories(rule, digestStories, insightsNumbers, { nowMs = Date.now() } = {}) { +export function composeBriefFromDigestStories(rule, digestStories, insightsNumbers, { nowMs = Date.now(), onDrop } = {}) { if (!Array.isArray(digestStories) || digestStories.length === 0) return null; - const sensitivity = rule.sensitivity ?? 'all'; + // Default to 'high' (NOT 'all') for undefined sensitivity, aligning + // with buildDigest at scripts/seed-digest-notifications.mjs:392 and + // the digestFor cache key. The live cron path pre-filters the pool + // to {critical, high}, so this default is a no-op for production + // calls — but a non-prefiltered caller with undefined sensitivity + // would otherwise silently widen to {medium, low} stories while the + // operator log labels the attempt as 'high', misleading telemetry. + // See PR #3387 review (P2) and Defect 2 / Solution 1 in + // docs/plans/2026-04-24-004-fix-brief-topic-adjacency-defects-plan.md. + const sensitivity = rule.sensitivity ?? 'high'; const tz = rule.digestTimezone ?? 'UTC'; const upstreamLike = digestStories.map(digestStoryToUpstreamTopStory); const stories = filterTopStories({ stories: upstreamLike, sensitivity, maxStories: MAX_STORIES_PER_USER, + onDrop, }); if (stories.length === 0) return null; const issueDate = issueDateInTz(nowMs, tz); diff --git a/scripts/lib/brief-llm.mjs b/scripts/lib/brief-llm.mjs index 5ac2b3535..5e49503da 100644 --- a/scripts/lib/brief-llm.mjs +++ b/scripts/lib/brief-llm.mjs @@ -523,7 +523,15 @@ async function mapLimit(items, limit, fn) { export async function enrichBriefEnvelopeWithLLM(envelope, rule, deps) { if (!envelope?.data || !Array.isArray(envelope.data.stories)) return envelope; const stories = envelope.data.stories; - const sensitivity = rule?.sensitivity ?? 'all'; + // Default to 'high' (NOT 'all') so the digest prompt and cache key + // align with what the rest of the pipeline (compose, buildDigest, + // cache, log) treats undefined-sensitivity rules as. Mismatched + // defaults would (a) mislead personalization — the prompt would say + // "Reader sensitivity level: all" while the actual brief contains + // only critical/high stories — and (b) bust the cache for legacy + // rules vs explicit-'all' rules that should share entries. See PR + // #3387 review (P3). + const sensitivity = rule?.sensitivity ?? 'high'; // Per-story enrichment — whyMatters AND description in parallel // per story (two LLM calls) but bounded across stories. diff --git a/scripts/seed-digest-notifications.mjs b/scripts/seed-digest-notifications.mjs index aaab46359..4e0b6c12d 100644 --- a/scripts/seed-digest-notifications.mjs +++ b/scripts/seed-digest-notifications.mjs @@ -1244,14 +1244,20 @@ async function composeBriefsForRun(rules, nowMs) { console.warn('[digest] brief: insights read failed, using zeroed stats:', err.message); } - // Memoize buildDigest by (variant, lang, windowStart). Many users - // share a variant/lang, so this saves ZRANGE + HGETALL round-trips - // across the per-user loop. Scoped to this cron run — no cross-run - // memoization needed (Redis is authoritative). + // Memoize buildDigest by (variant, lang, sensitivity, windowStart). + // Many users share a variant/lang, so this saves ZRANGE + HGETALL + // round-trips across the per-user loop. Scoped to this cron run — + // no cross-run memoization needed (Redis is authoritative). + // + // Sensitivity is part of the key because buildDigest filters by + // rule.sensitivity BEFORE dedup — without it, a stricter user + // inherits a looser populator's pool (the earlier populator "wins" + // and decides which severity tiers enter the pool, so stricter + // users get a pool that contains severities they never wanted). const windowStart = nowMs - BRIEF_STORY_WINDOW_MS; const digestCache = new Map(); async function digestFor(candidate) { - const key = `${candidate.variant ?? 'full'}:${candidate.lang ?? 'en'}:${windowStart}`; + const key = `${candidate.variant ?? 'full'}:${candidate.lang ?? 'en'}:${candidate.sensitivity ?? 'high'}:${windowStart}`; if (digestCache.has(key)) return digestCache.get(key); const stories = await buildDigest(candidate, windowStart); digestCache.set(key, stories ?? []); @@ -1298,12 +1304,56 @@ async function composeAndStoreBriefForUser(userId, candidates, insightsNumbers, for (const candidate of candidates) { const digestStories = await digestFor(candidate); if (!digestStories || digestStories.length === 0) continue; + const dropStats = { severity: 0, headline: 0, url: 0, shape: 0, cap: 0, in: digestStories.length }; const composed = composeBriefFromDigestStories( candidate, digestStories, insightsNumbers, - { nowMs }, + { + nowMs, + onDrop: (ev) => { dropStats[ev.reason] = (dropStats[ev.reason] ?? 0) + 1; }, + }, ); + + // Per-attempt filter-drop line. Emits one structured row for every + // candidate whose digest pool was non-empty, tagged with that + // candidate's own sensitivity and variant. See Solution 0 in + // docs/plans/2026-04-24-004-fix-brief-topic-adjacency-defects-plan.md + // for why this log exists (deciding whether Solution 3 is warranted). + // + // Emitting per attempt — not per user — because: + // - A user can have multiple rules with different sensitivities; + // a single-row-per-user log would have to either pick one + // sensitivity arbitrarily or label as 'mixed', hiding drops + // from the non-winning candidates. + // - An earlier candidate wiped out by post-group filtering (the + // exact signal Sol-0 targets) is invisible if only the winner + // is logged. Every attempt emits its own row so the fallback + // chain is visible. + // + // Outcomes per row: + // outcome=shipped — this candidate's envelope shipped; loop breaks. + // outcome=rejected — composed was null (every story filtered out); + // loop continues to the next candidate. + // + // A user whose every row is `outcome=rejected` is a wipeout — + // operators detect it by grouping rows by user and checking for + // absence of `outcome=shipped` within the tick. + const out = composed?.data?.stories?.length ?? 0; + console.log( + `[digest] brief filter drops user=${userId} ` + + `sensitivity=${candidate.sensitivity ?? 'high'} ` + + `variant=${candidate.variant ?? 'full'} ` + + `outcome=${composed ? 'shipped' : 'rejected'} ` + + `in=${dropStats.in} ` + + `dropped_severity=${dropStats.severity} ` + + `dropped_url=${dropStats.url} ` + + `dropped_headline=${dropStats.headline} ` + + `dropped_shape=${dropStats.shape} ` + + `dropped_cap=${dropStats.cap} ` + + `out=${out}`, + ); + if (composed) { envelope = composed; chosenVariant = candidate.variant; @@ -1311,6 +1361,7 @@ async function composeAndStoreBriefForUser(userId, candidates, insightsNumbers, break; } } + if (!envelope) return null; // Phase 3b — LLM enrichment. Substitutes the stubbed whyMatters / diff --git a/shared/brief-filter.d.ts b/shared/brief-filter.d.ts index e8ccd12f4..f524fe4b9 100644 --- a/shared/brief-filter.d.ts +++ b/shared/brief-filter.d.ts @@ -22,15 +22,38 @@ export function normaliseThreatLevel(upstream: string): BriefThreatLevel | null; export type AlertSensitivity = 'all' | 'high' | 'critical'; +/** + * Optional drop-metrics callback. Called synchronously once per + * dropped story. `severity` is present when threatLevel parsed but + * failed the sensitivity gate, or when a later gate (headline/url) + * dropped a story that had already passed the severity check. + * + * `cap` fires once per story skipped after `maxStories` has been + * reached — neither severity nor field metadata is included since + * the loop short-circuits without parsing the remaining stories. + */ +export type DropMetricsFn = (event: { + reason: 'severity' | 'headline' | 'url' | 'shape' | 'cap'; + severity?: string; + sourceUrl?: string; +}) => void; + /** * Filters the upstream `topStories` array against a user's * `alertRules.sensitivity` setting and caps at `maxStories`. Stories * with an unknown upstream severity are dropped. + * + * When `onDrop` is provided, it is invoked synchronously for each + * dropped story with the drop reason and available metadata. The + * callback runs before the `continue` that skips the story — callers + * can use it to aggregate per-user drop counters without altering + * filter behaviour. */ export function filterTopStories(input: { stories: UpstreamTopStory[]; sensitivity: AlertSensitivity; maxStories?: number; + onDrop?: DropMetricsFn; }): BriefStory[]; /** diff --git a/shared/brief-filter.js b/shared/brief-filter.js index b7d38c591..5fe0f736d 100644 --- a/shared/brief-filter.js +++ b/shared/brief-filter.js @@ -94,24 +94,56 @@ function clip(v, cap) { } /** - * @param {{ stories: UpstreamTopStory[]; sensitivity: AlertSensitivity; maxStories?: number }} input + * @typedef {(event: { reason: 'severity'|'headline'|'url'|'shape'|'cap', severity?: string, sourceUrl?: string }) => void} DropMetricsFn + */ + +/** + * @param {{ stories: UpstreamTopStory[]; sensitivity: AlertSensitivity; maxStories?: number; onDrop?: DropMetricsFn }} input * @returns {BriefStory[]} */ -export function filterTopStories({ stories, sensitivity, maxStories = 12 }) { +export function filterTopStories({ stories, sensitivity, maxStories = 12, onDrop }) { if (!Array.isArray(stories)) return []; const allowed = ALLOWED_LEVELS_BY_SENSITIVITY[sensitivity]; if (!allowed) return []; + // Per Solution 0 of the topic-adjacency plan: when the caller passes + // onDrop, we emit one event per filter drop so the seeder can + // aggregate counts and log per-tick drop rates. onDrop is optional + // and synchronous — any throw is the caller's problem (tested above). + const emit = typeof onDrop === 'function' ? onDrop : null; + /** @type {BriefStory[]} */ const out = []; - for (const raw of stories) { - if (out.length >= maxStories) break; - if (!raw || typeof raw !== 'object') continue; + for (let i = 0; i < stories.length; i++) { + const raw = stories[i]; + if (out.length >= maxStories) { + // Cap-truncation: remaining stories are not evaluated. Emit one + // event per skipped story so operators can reconcile in vs out + // counts (`in - out - sum(dropped_severity|headline|url|shape) + // == dropped_cap`). Without this, cap-truncated stories are + // invisible to Sol-0 telemetry and Sol-3's gating signal is + // undercounted by up to (DIGEST_MAX_ITEMS - MAX_STORIES_PER_USER) + // per user per tick. + if (emit) { + for (let j = i; j < stories.length; j++) emit({ reason: 'cap' }); + } + break; + } + if (!raw || typeof raw !== 'object') { + if (emit) emit({ reason: 'shape' }); + continue; + } const threatLevel = normaliseThreatLevel(raw.threatLevel); - if (!threatLevel || !allowed.has(threatLevel)) continue; + if (!threatLevel || !allowed.has(threatLevel)) { + if (emit) emit({ reason: 'severity', severity: threatLevel ?? undefined }); + continue; + } const headline = clip(asTrimmedString(raw.primaryTitle), MAX_HEADLINE_LEN); - if (!headline) continue; + if (!headline) { + if (emit) emit({ reason: 'headline', severity: threatLevel }); + continue; + } // v2: every surfaced story must have a working outgoing link so // the magazine can wrap the source line in a UTM anchor. A story @@ -121,7 +153,10 @@ export function filterTopStories({ stories, sensitivity, maxStories = 12 }) { // populated on every ingested item; the check exists so one bad // row can't slip through. const sourceUrl = normaliseSourceUrl(raw.primaryLink); - if (!sourceUrl) continue; + if (!sourceUrl) { + if (emit) emit({ reason: 'url', severity: threatLevel, sourceUrl: typeof raw.primaryLink === 'string' ? raw.primaryLink : undefined }); + continue; + } const description = clip( asTrimmedString(raw.description) || headline, diff --git a/tests/brief-composer-rule-dedup.test.mjs b/tests/brief-composer-rule-dedup.test.mjs index 723e2a088..131a97f07 100644 --- a/tests/brief-composer-rule-dedup.test.mjs +++ b/tests/brief-composer-rule-dedup.test.mjs @@ -88,6 +88,57 @@ describe('dedupeRulesByUser', () => { assert.equal(out1[0].updatedAt, 1_000); assert.equal(out2[0].updatedAt, 1_000); }); + + describe('undefined sensitivity ranks as "high" (NOT "all")', () => { + // PR #3387 review (P2): the rank function used to default to 'all', + // which would place a legacy undefined-sensitivity rule FIRST in + // the candidate order — but composeBriefFromDigestStories now + // applies a 'high' filter to undefined-sensitivity rules. Result: + // an explicit 'all' rule for the same user would never be tried, + // and the user would silently receive a narrower brief. Rank must + // match what compose actually applies. + function ruleWithoutSensitivity(overrides = {}) { + const r = rule(overrides); + delete r.sensitivity; + return r; + } + + it('explicit "all" rule beats undefined-sensitivity rule of same variant + age', () => { + const explicitAll = rule({ variant: 'full', sensitivity: 'all', updatedAt: 1_000 }); + const undefSens = ruleWithoutSensitivity({ variant: 'full', updatedAt: 1_000 }); + // Both arrival orders must produce the same winner. + const out1 = dedupeRulesByUser([explicitAll, undefSens]); + const out2 = dedupeRulesByUser([undefSens, explicitAll]); + assert.equal(out1[0].sensitivity, 'all'); + assert.equal(out2[0].sensitivity, 'all'); + }); + + it('undefined-sensitivity rule ties with explicit "high" (decided by updatedAt)', () => { + // Both should rank as 'high' → tiebreak by updatedAt → newer (older?) + // matches existing semantics: earlier updatedAt wins per the + // "stable under input reordering" test above. + const undefSens = ruleWithoutSensitivity({ variant: 'full', updatedAt: 1_000 }); + const explicitHigh = rule({ variant: 'full', sensitivity: 'high', updatedAt: 2_000 }); + const out1 = dedupeRulesByUser([undefSens, explicitHigh]); + const out2 = dedupeRulesByUser([explicitHigh, undefSens]); + // Earlier updatedAt wins → undefined rule (1_000 < 2_000). + assert.equal(out1[0].updatedAt, 1_000); + assert.equal(out2[0].updatedAt, 1_000); + }); + + it('candidate order in groupEligibleRulesByUser respects new ranking', () => { + // groupEligibleRulesByUser sorts candidates so the most-permissive + // (and most-preferred) is tried first by composeAndStoreBriefForUser. + // After the rank-default fix, undefined-sensitivity should sit + // BELOW explicit 'all' in the try order. + const explicitAll = rule({ variant: 'full', sensitivity: 'all', updatedAt: 1_000 }); + const undefSens = ruleWithoutSensitivity({ variant: 'full', updatedAt: 2_000 }); + const grouped = groupEligibleRulesByUser([undefSens, explicitAll]); + const candidates = grouped.get('user_abc'); + assert.equal(candidates[0].sensitivity, 'all', 'explicit "all" should be tried first'); + assert.equal(candidates[1].sensitivity, undefined, 'undefined sensitivity should come second'); + }); + }); }); describe('aiDigestEnabled default parity', () => { diff --git a/tests/brief-filter.test.mjs b/tests/brief-filter.test.mjs index e197050f0..b07702656 100644 --- a/tests/brief-filter.test.mjs +++ b/tests/brief-filter.test.mjs @@ -230,6 +230,168 @@ describe('assembleStubbedBriefEnvelope', () => { }); }); +describe('filterTopStories — onDrop metrics', () => { + const sensitivity = 'high'; + + it('does not invoke onDrop when every story passes', () => { + const calls = []; + const stories = [upstreamStory(), upstreamStory({ primaryTitle: 'Another' })]; + filterTopStories({ stories, sensitivity, onDrop: (ev) => calls.push(ev) }); + assert.equal(calls.length, 0); + }); + + it('fires onDrop with reason=severity when sensitivity excludes the level', () => { + const calls = []; + filterTopStories({ + stories: [upstreamStory({ threatLevel: 'low' })], + sensitivity, + onDrop: (ev) => calls.push(ev), + }); + assert.equal(calls.length, 1); + assert.equal(calls[0].reason, 'severity'); + assert.equal(calls[0].severity, 'low'); + }); + + it('fires onDrop with reason=headline when primaryTitle is empty', () => { + const calls = []; + filterTopStories({ + stories: [upstreamStory({ primaryTitle: '' })], + sensitivity, + onDrop: (ev) => calls.push(ev), + }); + assert.equal(calls.length, 1); + assert.equal(calls[0].reason, 'headline'); + assert.equal(calls[0].severity, 'high'); + }); + + it('fires onDrop with reason=url when primaryLink is invalid', () => { + const calls = []; + filterTopStories({ + stories: [upstreamStory({ primaryLink: 'ftp://bad' })], + sensitivity, + onDrop: (ev) => calls.push(ev), + }); + assert.equal(calls.length, 1); + assert.equal(calls[0].reason, 'url'); + assert.equal(calls[0].severity, 'high'); + assert.equal(calls[0].sourceUrl, 'ftp://bad'); + }); + + it('fires onDrop with reason=shape for non-object input', () => { + const calls = []; + filterTopStories({ + stories: [null, 'not an object', upstreamStory()], + sensitivity, + onDrop: (ev) => calls.push(ev), + }); + assert.equal(calls.length, 2); + assert.equal(calls[0].reason, 'shape'); + assert.equal(calls[1].reason, 'shape'); + }); + + it('output is byte-identical whether onDrop is supplied or not', () => { + // Regression guard: the metrics hook must not alter filter behaviour. + const stories = [ + upstreamStory({ threatLevel: 'low' }), + upstreamStory(), + upstreamStory({ primaryLink: 'ftp://bad' }), + upstreamStory({ primaryTitle: '' }), + upstreamStory({ primaryTitle: 'Second valid' }), + ]; + const without = filterTopStories({ stories, sensitivity }); + const with_ = filterTopStories({ stories, sensitivity, onDrop: () => {} }); + assert.deepEqual(without, with_); + }); + + it('distinct reasons are counted separately across a mixed batch', () => { + // Matches the seeder's per-user aggregation pattern. + const tally = { severity: 0, headline: 0, url: 0, shape: 0, cap: 0 }; + filterTopStories({ + stories: [ + upstreamStory({ threatLevel: 'low' }), // severity + upstreamStory({ threatLevel: 'medium' }), // severity + upstreamStory({ primaryTitle: '' }), // headline + upstreamStory({ primaryLink: 'ftp://bad' }), // url + null, // shape + upstreamStory(), // kept + ], + sensitivity, + onDrop: (ev) => { tally[ev.reason]++; }, + }); + assert.equal(tally.severity, 2); + assert.equal(tally.headline, 1); + assert.equal(tally.url, 1); + assert.equal(tally.shape, 1); + assert.equal(tally.cap, 0); + }); + + it('fires onDrop with reason=cap once per story skipped after maxStories', () => { + // Without this, cap-truncated stories are invisible to telemetry + // and `in - out - sum(other_drops)` does not reconcile. + const calls = []; + filterTopStories({ + stories: [ + upstreamStory({ primaryTitle: 'A' }), + upstreamStory({ primaryTitle: 'B' }), + upstreamStory({ primaryTitle: 'C' }), + upstreamStory({ primaryTitle: 'D' }), + upstreamStory({ primaryTitle: 'E' }), + ], + sensitivity, + maxStories: 2, + onDrop: (ev) => calls.push(ev), + }); + assert.equal(calls.length, 3, 'should emit one cap event per story past maxStories'); + for (const ev of calls) assert.equal(ev.reason, 'cap'); + }); + + it('cap events do NOT count earlier severity/headline/url drops twice', () => { + // The cap-emit loop runs from the break point onward — earlier + // valid stories that pushed `out` to maxStories are not re-emitted, + // and earlier-dropped stories are accounted under their own reason. + const tally = { severity: 0, headline: 0, url: 0, shape: 0, cap: 0 }; + filterTopStories({ + stories: [ + upstreamStory({ primaryTitle: 'A' }), // kept + upstreamStory({ threatLevel: 'low' }), // severity (not cap) + upstreamStory({ primaryTitle: 'B' }), // kept (out reaches 2) + upstreamStory({ primaryTitle: 'C' }), // cap + upstreamStory({ primaryLink: 'ftp://bad' }), // cap (loop short-circuits past url check) + ], + sensitivity, + maxStories: 2, + onDrop: (ev) => { tally[ev.reason]++; }, + }); + assert.equal(tally.severity, 1); + assert.equal(tally.cap, 2); + assert.equal(tally.url, 0, 'url drop should NOT fire after cap break'); + }); + + it('reconciliation invariant: in === out + sum(dropped_*) across all reasons', () => { + // Locks in the operator-facing invariant that motivated adding `cap`. + const tally = { severity: 0, headline: 0, url: 0, shape: 0, cap: 0 }; + const stories = [ + upstreamStory({ primaryTitle: 'A' }), + upstreamStory({ primaryTitle: 'B' }), + upstreamStory({ threatLevel: 'low' }), + upstreamStory({ primaryTitle: '' }), + upstreamStory({ primaryLink: 'ftp://bad' }), + null, + upstreamStory({ primaryTitle: 'C' }), + upstreamStory({ primaryTitle: 'D' }), + upstreamStory({ primaryTitle: 'E' }), + ]; + const out = filterTopStories({ + stories, + sensitivity, + maxStories: 3, + onDrop: (ev) => { tally[ev.reason]++; }, + }); + const totalDrops = tally.severity + tally.headline + tally.url + tally.shape + tally.cap; + assert.equal(stories.length, out.length + totalDrops); + }); +}); + describe('issueDateInTz', () => { // 2026-04-18T00:30:00Z — midnight UTC + 30min. Tokyo (+9) is // already mid-morning on the 18th; LA (-7) is late on the 17th. diff --git a/tests/brief-from-digest-stories.test.mjs b/tests/brief-from-digest-stories.test.mjs index 4c53e2d7d..1628bbc9c 100644 --- a/tests/brief-from-digest-stories.test.mjs +++ b/tests/brief-from-digest-stories.test.mjs @@ -312,4 +312,64 @@ describe('composeBriefFromDigestStories — continued', () => { assert.ok(env); assert.equal(env.data.stories[0].description, 'Iran threatens to close Strait of Hormuz'); }); + + describe('undefined sensitivity defaults to "high" (NOT "all")', () => { + // PR #3387 review (P2): the previous `?? 'all'` default would + // silently widen to {medium, low} for any non-prefiltered caller + // with undefined sensitivity, while operator telemetry labeled the + // attempt as 'high' (matching buildDigest's default). The two + // defaults must agree to keep the per-attempt log accurate and to + // prevent unintended severity widening through this entry point. + function ruleWithoutSensitivity() { + const r = rule(); + delete r.sensitivity; + return r; + } + + it('admits critical and high stories when sensitivity is undefined', () => { + const env = composeBriefFromDigestStories( + ruleWithoutSensitivity(), + [ + digestStory({ hash: 'a', title: 'Critical event', severity: 'critical' }), + digestStory({ hash: 'b', title: 'High event', severity: 'high' }), + ], + { clusters: 0, multiSource: 0 }, + { nowMs: NOW }, + ); + assert.ok(env); + assert.equal(env.data.stories.length, 2); + }); + + it('drops medium and low stories when sensitivity is undefined', () => { + const env = composeBriefFromDigestStories( + ruleWithoutSensitivity(), + [ + digestStory({ hash: 'a', title: 'Medium event', severity: 'medium' }), + digestStory({ hash: 'b', title: 'Low event', severity: 'low' }), + ], + { clusters: 0, multiSource: 0 }, + { nowMs: NOW }, + ); + // No critical/high stories survive → composer returns null per + // the empty-survivor contract (caller falls back to next variant). + assert.equal(env, null); + }); + + it('emits onDrop reason=severity for medium/low when sensitivity is undefined', () => { + // Locks in alignment with the per-attempt telemetry: if compose + // were to default to 'all' again, medium/low would NOT fire a + // severity drop and the log would silently misreport the filter. + const tally = { severity: 0, headline: 0, url: 0, shape: 0, cap: 0 }; + composeBriefFromDigestStories( + ruleWithoutSensitivity(), + [ + digestStory({ hash: 'a', title: 'Medium', severity: 'medium' }), + digestStory({ hash: 'b', title: 'Low', severity: 'low' }), + ], + { clusters: 0, multiSource: 0 }, + { nowMs: NOW, onDrop: (ev) => { tally[ev.reason]++; } }, + ); + assert.equal(tally.severity, 2); + }); + }); }); diff --git a/tests/digest-cache-key-sensitivity.test.mjs b/tests/digest-cache-key-sensitivity.test.mjs new file mode 100644 index 000000000..0b9ea2d63 --- /dev/null +++ b/tests/digest-cache-key-sensitivity.test.mjs @@ -0,0 +1,75 @@ +/** + * Regression test for the `digestFor` memoization key in + * scripts/seed-digest-notifications.mjs. + * + * buildDigest filters by rule.sensitivity BEFORE dedup (line 392). + * The digestFor cache used to key by (variant, lang, windowStart), + * which meant stricter-sensitivity users in a shared bucket inherited + * the looser populator's pool — producing the wrong story set AND + * defeating the topic-grouping adjacency intent once post-group + * sensitivity re-filtering kicked in. + * + * Guard on the cache-key string itself: if a future refactor drops + * sensitivity from the key, this test fails. + * + * Follows the same static-shape pattern as + * tests/digest-score-floor.test.mjs — the cron script has a top-level + * env-exit block that makes runtime imports fragile. + * + * Run: node --test tests/digest-cache-key-sensitivity.test.mjs + */ + +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import { resolve, dirname } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const src = readFileSync( + resolve(__dirname, '../scripts/seed-digest-notifications.mjs'), + 'utf-8', +); + +describe('digestFor cache key includes sensitivity', () => { + it('memoization key interpolates candidate.sensitivity', () => { + // The key must include sensitivity alongside variant+lang+windowStart + // so stricter users do not inherit a looser populator's pool. + assert.match( + src, + /const\s+key\s*=\s*`\$\{candidate\.variant[^`]*?\$\{candidate\.sensitivity[^`]*?\$\{windowStart\}`/, + 'digestFor cache key must interpolate candidate.sensitivity', + ); + }); + + it('defaults missing sensitivity to "high" (matches buildDigest default)', () => { + // buildDigest uses `rule.sensitivity ?? 'high'` at line 392. + // The cache key must use the same default or a stricter-populator + // (explicit 'critical') would collide with a default-populator + // (undefined → buildDigest treats as 'high', cache would treat as + // something else). + // + // Anchor the match to the cache-key template-literal context so it + // cannot be satisfied by an unrelated `chosenCandidate.sensitivity + // ?? 'high'` elsewhere in the file (e.g. the new operator log line). + assert.match( + src, + /\$\{candidate\.sensitivity\s*\?\?\s*'high'\}\s*:\s*\$\{windowStart\}/, + 'cache key default for sensitivity must be "high" to align with buildDigest default, anchored inside the cache-key template literal', + ); + }); + + it('key construction lives inside digestFor closure', () => { + // Sanity: ensure the key construction is not pulled out into a + // separate helper whose shape this test can no longer see. + const digestForBlock = src.match( + /async\s+function\s+digestFor\s*\(candidate\)\s*\{[\s\S]*?\n\s*\}/, + ); + assert.ok(digestForBlock, 'digestFor function block should exist'); + assert.match( + digestForBlock[0], + /candidate\.sensitivity/, + 'sensitivity must be referenced inside digestFor', + ); + }); +});