fix(brief): restore email multi-section format + de-vapidify lead prompt#3411
Conversation
Two regressions from the canonical-synthesis refactor (PR #3396) shipped in the 2026-04-25 evening tick: 1. Email Executive Summary became a 1-paragraph pull-quote. Pre-refactor, generateAISummary returned a 5-paragraph blob (lead + 3-5 bullets + "Signals to watch:") that filled the email's editorial slot. Post-refactor, the canonical synthesis returns structured fields ({lead, threads, signals}) and `injectEmailSummary` only consumed `.lead` — leaving threads and signals visible in the magazine but absent from email. Reader saw "1 paragraph instead of 5". 2. The lead paragraph itself was vapid editorial filler. The new system prompt's "addresses the reader directly" instruction invited gemini-2.5-flash to produce phrases like "the global stage is buzzing with developments" and "navigating the evolving landscape" instead of naming actors and events. Pre-refactor Brain B prompt's "Lead with the single most impactful development for this user" forced concrete naming — that semantic was lost. Fixes: scripts/lib/email-summary-html.mjs (new): - Extracts injectEmailSummary so it can be unit-tested without the cron's env-checking side effects. - Now accepts a structured {lead, threads, signals} object (or a string for legacy / L3 stub) and renders all three sections in the email body — restoring the pre-refactor 5-paragraph richness: * Lead paragraph (top of block) * Threads as "<b>Tag</b> — teaser" lines * "Signals to watch:" header + bulleted signals - All thread/signal text HTML-escaped (defense-in-depth). scripts/seed-digest-notifications.mjs: - Imports injectEmailSummary from the new lib module; removed inline definition. - Send loop captures the FULL synthesis (briefSynthesis) in addition to the string projection (briefLead). Email gets the full structured object; non-email channels (Telegram, Slack, Discord, webhook) keep the single-string lead since their formats favour brevity. - Removed unused markdownToEmailHtml import (now in the helper). scripts/lib/brief-llm.mjs: - DIGEST_PROSE_SYSTEM_BASE prompt rewritten: * Lead instruction: "FIRST sentence MUST name the single most impactful development by its specific actor and event" (e.g. "Pentagon chief Hegseth declared..."). Concrete naming is now load-bearing, not optional editorial flourish. * Threads + signals each carry a "name SPECIFIC event/actor" instruction with positive examples. * BANNED phrasing list: "the global stage", "buzzing with developments", "intricate shifts", "evolving landscape", "navigating", "discerning reader", "continues to simmer", "shape the coming months", "strategic importance" — these were the exact phrases the model produced last evening. - Cache key bumped brief:llm:digest:v3 → v4. v3 rows still in TTL would otherwise serve stale vapid leads for 4h post-deploy. Same precedent as the v2→v3 bump. Tests added (tests/email-summary-html.test.mjs, +12): - null/undefined/empty/empty-lead → slot stripped - string input → lead-only render (legacy + L3 stub path) - structured input → lead + threads + signals all rendered - malformed thread entries skipped without rejecting the block - HTML-escapes hostile thread tags / teasers / signals - malformed signal entries skipped - empty-html input passthrough Tests updated (tests/brief-llm.test.mjs): - Cache key prefix references bumped v3 → v4. Test results: - tests/email-summary-html.test.mjs: 12/12 (new file) - tests/brief-llm.test.mjs: 77/77 - Full data suite: 7121/7121 - typecheck + typecheck:api: clean User-facing impact: tomorrow morning's brief will again show the specific named events ("Pentagon chief Hegseth declared...", "Navy Secretary John Phelan fired...") in 5 paragraphs instead of one paragraph of editorial filler.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Greptile SummaryThis PR fixes two regressions from PR #3396: the email body was truncated to only the lead paragraph (threads and signals were never injected), and the LLM prompt invited vapid editorial framing instead of named actors and events. The fix extracts a new Confidence Score: 4/5Safe to merge; only P2 findings remain and the core regression fixes are correct. All findings are P2. The logic is sound: briefSynthesis is correctly captured and passed to the new helper, markdownToEmailHtml HTML-escapes the lead before markdown processing, threads and signals escape their content, and cache-key versioning is properly documented. The one actionable issue—an orphaned "Signals to watch:" header when all signal entries are malformed—is a defensive-coding gap unlikely to fire against real LLM output. scripts/lib/email-summary-html.mjs — signals block header emission logic; tests/email-summary-html.test.mjs — missing all-malformed-signals test case. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[runSynthesisWithFallback] --> B{synthesis ok?}
B -- yes --> C[briefSynthesis = full object]
B -- no --> D[briefSynthesis = null]
C --> E[briefLead = synthesis.lead string]
D --> F[briefLead = null]
E --> G[buildChannelBodies\nTelegram / Slack / Discord\nuses string lead only]
C --> H[injectEmailSummary\nhtmlRaw + briefSynthesis]
D --> H
H --> I{summary type?}
I -- null or no lead --> J[strip slot placeholder]
I -- string legacy --> K[lead block only]
I -- full object --> L[lead + threads + signals\nmulti-section HTML]
L --> M[Email: Executive Summary\n+ Thread lines\n+ Signals to watch]
Reviews (1): Last reviewed commit: "fix(brief): restore email multi-section ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const signalsHtml = payload.signals.length > 0 | ||
| ? `<div style="font-size:13px;line-height:1.7;color:#ccc;margin:14px 0 0 0;"><b style="color:#f2ede4;">Signals to watch:</b></div>` + | ||
| payload.signals.map((s) => { | ||
| const text = escape(typeof s === 'string' ? s : ''); | ||
| if (!text) return ''; | ||
| return `<div style="font-size:13px;line-height:1.7;color:#ccc;margin:4px 0 0 0;">• ${text}</div>`; | ||
| }).filter(Boolean).join('') | ||
| : ''; |
There was a problem hiding this comment.
The "Signals to watch:" header <div> is unconditionally included in the ternary's true branch. If payload.signals is non-empty but every entry is a non-string or empty string, the .filter(Boolean).join('') step produces '', so the rendered block contains the header with no bullets beneath it.
This is a live edge — the same code explicitly guards against non-string entries (typeof s === 'string' ? s : ''), and the matching test only covers the case where at least one valid signal exists. Extracting the filtered items first makes the header conditional on there being something to show:
const signalsHtml = (() => {
if (payload.signals.length === 0) return '';
const items = payload.signals
.map((s) => {
const text = escape(typeof s === 'string' ? s : '');
if (!text) return '';
return `<div style="font-size:13px;line-height:1.7;color:#ccc;margin:4px 0 0 0;">• ${text}</div>`;
})
.filter(Boolean);
if (items.length === 0) return '';
return `<div style="font-size:13px;line-height:1.7;color:#ccc;margin:14px 0 0 0;"><b style="color:#f2ede4;">Signals to watch:</b></div>` +
items.join('');
})();| const out = injectEmailSummary(TEMPLATE, { | ||
| lead: 'A long-enough lead text for the validator floor.', | ||
| threads: [], | ||
| signals: ['Valid signal.', '', 42, null], | ||
| }); | ||
| assert.ok(out.includes('• Valid signal')); | ||
| // Header still emitted because at least one signal is valid. | ||
| assert.ok(out.includes('Signals to watch:')); | ||
| // Only one bullet — the malformed entries are skipped. | ||
| const bullets = (out.match(/• /g) ?? []).length; | ||
| assert.equal(bullets, 1); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The skips empty / non-string signal entries test always has at least one valid signal, so the case where payload.signals is non-empty but every entry is invalid (e.g., [null, 42, '']) is untested. That edge is the exact condition that triggers the orphaned-header issue in email-summary-html.mjs. A one-liner addition covers it:
it('emits no signals block when all signal entries are non-string or empty', () => {
const out = injectEmailSummary(TEMPLATE, {
lead: 'A long-enough lead text for the validator floor.',
threads: [],
signals: [null, 42, ''],
});
assert.ok(!out.includes('Signals to watch:'));
});
1 participant
Summary
Two regressions from the canonical-synthesis refactor (PR #3396) shipped in the 2026-04-25 evening tick. Reader noticed the change immediately:
Old (afternoon, pre-refactor):
New (evening, post-refactor):
Two distinct problems shipping together:
injectEmailSummaryonly consumed.leadfrom the structured synthesis. Threads + signals existed in the envelope but never made it to email.Fixes
scripts/lib/email-summary-html.mjs(new)seed-digest-notifications.mjsso the HTML assembly can be unit-tested without the cron's env-checking side effects.{lead, threads, signals}object (or a string for legacy / L3 stub).<b>Tag</b> — teaserlinesscripts/seed-digest-notifications.mjsinjectEmailSummaryfrom the new lib module; removed inline definition.briefSynthesis) in addition to the string projection (briefLead). Email gets the full structured object; non-email channels (Telegram/Slack/Discord/webhook) keep the single-string lead since their formats favour brevity.markdownToEmailHtmlimport (now in the helper).scripts/lib/brief-llm.mjs— prompt rewritebrief:llm:digest:v3→v4. v3 rows still in TTL would otherwise serve stale vapid leads for 4h post-deploy. Same precedent as the v2→v3 bump.Test plan
tests/email-summary-html.test.mjs: 12/12 new (null/empty handling, string fallback, structured rendering, malformed-thread skip, HTML-escape, malformed-signal skip)tests/brief-llm.test.mjs: 77/77 (cache prefix references updated to v4)npm run test:data: 7121/7121npm run typecheck+npm run typecheck:api: cleannode -c scripts/seed-digest-notifications.mjs: parsesPost-deploy validation
Tomorrow morning's brief (first tick after deploy + cache TTL expires within 4h):
digest.leadis unchanged (still the pull-quote)Failure signals:
briefSynthesisis being passed (notbriefLead) toinjectEmailSummaryRollback: revert this PR. Branch is one commit; revert is mechanical. Cache key v4 means v3 rows will resurface on revert — acceptable cost (4h TTL for stale-on-revert rows).
Companion
Parent: PR #3396 (canonical synthesis brain), PR #3401 (Greptile P1+P4). This is the third follow-up — the user-visible regression patch.