From 3d7e60ca7d4e58edbdfa59b9c06b6e07d58c108e Mon Sep 17 00:00:00 2001 From: Elie Habib Date: Sat, 11 Apr 2026 17:10:06 +0400 Subject: [PATCH] fix(digest): never skip AI summary when userPreferences are missing (#2939) * fix(digest): never skip AI summary when userPreferences are missing Users who enabled the AI executive summary toggle on their notification rule still received digest emails without the summary. The Railway log pinpointed it: [digest] No preferences for user_... skipping AI summary [digest] Email delivered to ... Root cause chain: convex/http.ts:591 /relay/user-preferences returns literal null when no userPreferences row exists for (userId, variant). scripts/lib/user-context.cjs fetchUserPreferences forwards that as { data: null, error: false }. scripts/seed-digest-notifications.mjs:458 generateAISummary bails with return null. The AI-summary toggle lives on the alertRules table. userPreferences is a SEPARATE table (the SPA app-settings blob: watchlist, airports, panels). A user can have an alertRule (with aiDigestEnabled: true) without having ever saved userPreferences, or only under a different variant. Missing prefs must NOT silently disable the feature the user explicitly enabled. The correct behavior is to degrade to a non-personalized summary. Fix: remove the early return in generateAISummary. Call extractUserContext(null), which already returns a safe empty context, and formatUserProfile(ctx, 'full') returns "Variant: full" alone. The LLM then generates a generic daily brief instead of nothing. An info log still reports the missing-prefs case for observability. Regression coverage: tests/user-context.test.mjs (new, 10 cases) locks in that extractUserContext(null|undefined|{}|"") returns the empty shape and formatUserProfile(emptyCtx, variant) returns exactly "Variant: {variant}". Any future refactor that reintroduces the null-bail will fail the tests. Note: the same log also shows the rule fired at 13:01 Dubai instead of 8 AM / 8 PM. That is a separate issue in isDue or rule-save flow and needs more log data to diagnose; not included here. * fix(digest): distinguish transient prefs fetch failure from missing row Addresses Greptile P2 review feedback on PR #2939. fetchUserPreferences returns { data, error } where: error: true = transient fetch failure (network, non-OK HTTP, env missing) error: false = the (userId, variant) row genuinely does not exist The previous log treated both cases identically as "No stored preferences", which was misleading when the real cause was an unreachable Convex endpoint. Behavior is unchanged (both still degrade to a non-personalized summary), only the log line differentiates them so transient fetch failures are visible in observability. --- scripts/seed-digest-notifications.mjs | 19 ++++- tests/user-context.test.mjs | 103 ++++++++++++++++++++++++++ 2 files changed, 119 insertions(+), 3 deletions(-) create mode 100644 tests/user-context.test.mjs diff --git a/scripts/seed-digest-notifications.mjs b/scripts/seed-digest-notifications.mjs index 0877fb9a1..ba16740f1 100644 --- a/scripts/seed-digest-notifications.mjs +++ b/scripts/seed-digest-notifications.mjs @@ -454,10 +454,23 @@ async function generateAISummary(stories, rule) { if (!AI_DIGEST_ENABLED) return null; if (!stories || stories.length === 0) return null; - const { data: prefs } = await fetchUserPreferences(rule.userId, rule.variant ?? 'full'); + // rule.aiDigestEnabled (from alertRules) is the user's explicit opt-in for + // AI summaries. userPreferences is a SEPARATE table (SPA app settings blob: + // watchlist, airports, panels). A user can have alertRules without having + // ever saved userPreferences — or under a different variant. Missing prefs + // must NOT silently disable the feature the user just enabled; degrade to + // a non-personalized summary instead. + // error: true = transient fetch failure (network, non-OK HTTP, env missing) + // error: false = the (userId, variant) row genuinely does not exist + // Both cases degrade to a non-personalized summary, but log them distinctly + // so transient fetch failures are visible in observability. + const { data: prefs, error: prefsFetchError } = await fetchUserPreferences(rule.userId, rule.variant ?? 'full'); if (!prefs) { - console.log(`[digest] No preferences for ${rule.userId} — skipping AI summary`); - return null; + console.log( + prefsFetchError + ? `[digest] Prefs fetch failed for ${rule.userId} — generating non-personalized AI summary` + : `[digest] No stored preferences for ${rule.userId} — generating non-personalized AI summary`, + ); } const ctx = extractUserContext(prefs); const profile = formatUserProfile(ctx, rule.variant ?? 'full'); diff --git a/tests/user-context.test.mjs b/tests/user-context.test.mjs new file mode 100644 index 000000000..37cec4688 --- /dev/null +++ b/tests/user-context.test.mjs @@ -0,0 +1,103 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { createRequire } from 'node:module'; + +const require = createRequire(import.meta.url); +const { extractUserContext, formatUserProfile } = require('../scripts/lib/user-context.cjs'); + +// These helpers feed the digest AI executive summary prompt. The digest +// cron fetches userPreferences via a Convex relay endpoint that returns +// literal `null` when the (userId, variant) row doesn't exist. A user can +// have alertRules (with aiDigestEnabled: true) but no userPreferences +// document — for example, a notification rule enabled before the user +// ever synced the SPA, or under a different variant. Missing preferences +// MUST NOT silently disable the AI summary; we degrade to a generic +// "Variant: full" profile and still call the LLM. +// +// These tests lock in that contract so `extractUserContext(null)` and +// `formatUserProfile(ctx, variant)` remain null-safe. + +describe('extractUserContext null-safety', () => { + const empty = { + tickers: [], + airports: [], + airlines: [], + frameworkName: null, + enabledPanels: [], + disabledFeeds: [], + }; + + it('returns empty context for null', () => { + assert.deepEqual(extractUserContext(null), empty); + }); + + it('returns empty context for undefined', () => { + assert.deepEqual(extractUserContext(undefined), empty); + }); + + it('returns empty context for empty object', () => { + assert.deepEqual(extractUserContext({}), empty); + }); + + it('returns empty context for non-object (string)', () => { + assert.deepEqual(extractUserContext('not an object'), empty); + }); + + it('extracts tickers from wm-market-watchlist-v1', () => { + const ctx = extractUserContext({ + 'wm-market-watchlist-v1': [ + { symbol: 'AAPL' }, + { symbol: 'TSLA' }, + { notASymbol: true }, + ], + }); + assert.deepEqual(ctx.tickers, ['AAPL', 'TSLA']); + }); + + it('extracts airports and airlines from aviation:watchlist:v1', () => { + const ctx = extractUserContext({ + 'aviation:watchlist:v1': { + airports: ['JFK', 'LHR'], + airlines: ['UAL', 'BA'], + }, + }); + assert.deepEqual(ctx.airports, ['JFK', 'LHR']); + assert.deepEqual(ctx.airlines, ['UAL', 'BA']); + }); +}); + +describe('formatUserProfile null-safety', () => { + it('handles an entirely empty context (no crash, no empty lines)', () => { + const emptyCtx = extractUserContext(null); + const profile = formatUserProfile(emptyCtx, 'full'); + assert.equal(profile, 'Variant: full'); + }); + + it('handles empty context for every variant value', () => { + for (const variant of ['full', 'tech', 'finance', 'commodity', 'happy']) { + const profile = formatUserProfile(extractUserContext(null), variant); + assert.equal(profile, `Variant: ${variant}`); + } + }); + + it('includes watchlist entries when present', () => { + const ctx = extractUserContext({ + 'wm-market-watchlist-v1': [{ symbol: 'AAPL' }, { symbol: 'TSLA' }], + }); + const profile = formatUserProfile(ctx, 'finance'); + assert.match(profile, /^Variant: finance$/m); + assert.match(profile, /^Watches: AAPL, TSLA$/m); + }); + + it('always includes the Variant line even with rich context', () => { + const ctx = extractUserContext({ + 'wm-market-watchlist-v1': [{ symbol: 'AAPL' }], + 'aviation:watchlist:v1': { airports: ['JFK'], airlines: ['UAL'] }, + }); + const profile = formatUserProfile(ctx, 'full'); + assert.match(profile, /^Variant: full$/m); + assert.match(profile, /Watches: AAPL/); + assert.match(profile, /Monitors airports: JFK/); + assert.match(profile, /Monitors airlines: UAL/); + }); +});