fix(brief): single canonical synthesis brain — eliminate email/brief lead divergence#3396
Merged
Conversation
Extends generateDigestProse to be the single source of truth for
brief executive-summary synthesis (canonicalises what was previously
split between brief-llm's generateDigestProse and seed-digest-
notifications.mjs's generateAISummary). Ports Brain B's prompt
features into buildDigestPrompt:
- ctx={profile, greeting, isPublic} parameter (back-compat: 4-arg
callers behave like today)
- per-story severity uppercased + short-hash prefix [h:XXXX] so the
model can emit rankedStoryHashes for stable re-ranking
- profile lines + greeting opener appear only when ctx.isPublic !== true
validateDigestProseShape gains optional rankedStoryHashes (≥4-char
strings, capped to MAX_STORIES_PER_USER × 2). v2-shaped rows still
pass — field defaults to [].
hashDigestInput v3:
- material includes profile-SHA, greeting bucket, isPublic flag,
per-story hash
- isPublic=true substitutes literal 'public' for userId in the cache
key so all share-URL readers of the same (date, sensitivity, pool)
hit ONE cache row (no PII in public cache key)
Adds generateDigestProsePublic(stories, sensitivity, deps) wrapper —
no userId param by design — for the share-URL surface.
Cache prefix bumped brief:llm:digest:v2 → v3. v2 rows expire on TTL.
Per the v1→v2 precedent (see hashDigestInput comment), one-tick cost
on rollout is acceptable for cache-key correctness.
Tests: 72/72 passing in tests/brief-llm.test.mjs (8 new for the v3
behaviors), full data suite 6952/6952.
Plan: docs/plans/2026-04-25-002-fix-brief-email-two-brain-divergence-plan.md
Step 1, Codex-approved (5 rounds).
Bumps BRIEF_ENVELOPE_VERSION 2 → 3. Adds optional BriefDigest.publicLead — non-personalised executive lead generated by generateDigestProsePublic (already in this branch from the previous commit) for the public share-URL surface. Personalised `lead` is the canonical synthesis for authenticated channels; publicLead is its profile-stripped sibling so api/brief/public/* never serves user-specific content (watched assets/regions). SUPPORTED_ENVELOPE_VERSIONS = [1, 2, 3] keeps v1 + v2 envelopes in the 7-day TTL window readable through the rollout — the composer only ever writes the current version, but readers must tolerate older shapes that haven't expired yet. Same rollout pattern used at the v1 → v2 bump. Renderer changes (server/_shared/brief-render.js): - ALLOWED_DIGEST_KEYS gains 'publicLead' (closed-key-set still enforced; v2 envelopes pass because publicLead === undefined is the v2 shape). - assertBriefEnvelope: new isNonEmptyString check on publicLead when present. Type contract enforced; absence is OK. Tests (tests/brief-magazine-render.test.mjs): - New describe block "v3 publicLead field": v3 envelope renders; malformed publicLead rejected; v2 envelope still passes; ad-hoc digest keys (e.g. synthesisLevel) still rejected — confirming the closed-key-set defense holds for the cron-local-only fields the orchestrator must NOT persist. - BRIEF_ENVELOPE_VERSION pin updated 2 → 3 with rollout-rationale comment. Test results: 182 brief-related tests pass; full data suite 6956/6956. Plan: docs/plans/2026-04-25-002-fix-brief-email-two-brain-divergence-plan.md Step 2, Codex Round-3 Medium #2.
Plumbs the canonical synthesis output (lead, threads, signals, publicLead, rankedStoryHashes from generateDigestProse) through the pure composer so the orchestration layer can hand pre-resolved data into envelope.digest. Composer stays sync / no I/O — Codex Round-2 High #2 honored. Changes: scripts/lib/brief-compose.mjs: - digestStoryToUpstreamTopStory now emits `hash` (the digest story's stable identifier, falls back to titleHash when absent). Without this, rankedStoryHashes from the LLM has nothing to match against. - composeBriefFromDigestStories accepts opts.synthesis = {lead, threads, signals, rankedStoryHashes?, publicLead?}. When passed, splices into envelope.digest after the stub is built. Partial synthesis (e.g. only `lead` populated) keeps stub defaults for the other fields — graceful degradation when L2 fallback fires. shared/brief-filter.js: - filterTopStories accepts optional rankedStoryHashes. New helper applyRankedOrder re-orders stories by short-hash prefix match BEFORE the cap is applied, so the model's editorial judgment of importance survives MAX_STORIES_PER_USER. Stable for ties; stories not in the ranking come after in original order. Empty/missing ranking is a no-op (legacy callers unchanged). shared/brief-filter.d.ts: - filterTopStories signature gains rankedStoryHashes?: string[]. - UpstreamTopStory gains hash?: unknown (carried through from digestStoryToUpstreamTopStory). Tests added (tests/brief-from-digest-stories.test.mjs): - synthesis substitutes lead/threads/signals/publicLead. - legacy 4-arg callers (no synthesis) keep stub lead. - partial synthesis (only lead) keeps stub threads/signals. - rankedStoryHashes re-orders pool before cap. - short-hash prefix match (model emits 8 chars; story carries full). - unranked stories go after in original order. Test results: 33/33 in brief-from-digest-stories; 182/182 across all brief tests; full data suite 6956/6956. Plan: docs/plans/2026-04-25-002-fix-brief-email-two-brain-divergence-plan.md Step 3, Codex Round-2 Low + Round-2 High #2.
Restructures the digest cron's per-user compose + send loops to produce ONE canonical synthesis per user per issueSlot — the lead text every channel (email HTML, plain-text, Telegram, Slack, Discord, webhook) and the magazine show is byte-identical. This eliminates the "two-brain" divergence that was producing different exec summaries on different surfaces (observed 2026-04-25 0802). Architecture: composeBriefsForRun (orchestration): - Pre-annotates every eligible rule with lastSentAt + isDue once, before the per-user pass. Same getLastSentAt helper the send loop uses so compose + send agree on lastSentAt for every rule. composeAndStoreBriefForUser (per-user): - Two-pass winner walk: try DUE rules first (sortedDue), fall back to ALL eligible rules (sortedAll) for compose-only ticks. Preserves today's dashboard refresh contract for weekly / twice_daily users on non-due ticks (Codex Round-4 High #1). - Within each pass, walk by compareRules priority and pick the FIRST candidate with a non-empty pool — mirrors today's behavior at scripts/seed-digest-notifications.mjs:1044 and prevents the "highest-priority but empty pool" edge case (Codex Round-4 Medium #2). - Three-level synthesis fallback chain: L1: generateDigestProse(fullPool, ctx={profile,greeting,!public}) L2: generateDigestProse(envelope-sized slice, ctx={}) L3: stub from assembleStubbedBriefEnvelope Distinct log lines per fallback level so ops can quantify failure-mode distribution. - Generates publicLead in parallel via generateDigestProsePublic (no userId param; cache-shared across all share-URL readers). - Splices synthesis into envelope via composer's optional `synthesis` arg (Step 3); rankedStoryHashes re-orders the pool BEFORE the cap so editorial importance survives MAX_STORIES. - synthesisLevel stored in the cron-local briefByUser entry — NOT persisted in the envelope (renderer's assertNoExtraKeys would reject; Codex Round-2 Medium #5). Send loop: - Reads lastSentAt via shared getLastSentAt helper (single source of truth with compose flow). - briefLead = brief?.envelope?.data?.digest?.lead — the canonical lead. Passed to buildChannelBodies (text/Telegram/Slack/Discord), injectEmailSummary (HTML email), and sendWebhook (webhook payload's `summary` field). All-channel parity (Codex Round-1 Medium #6). - Subject ternary reads cron-local synthesisLevel: 1 or 2 → "Intelligence Brief", 3 → "Digest" (preserves today's UX for fallback paths; Codex Round-1 Missing #5). Removed: - generateAISummary() — the second LLM call that produced the divergent email lead. ~85 lines. - AI_SUMMARY_CACHE_TTL constant — no longer referenced. The digest:ai-summary:v1:* cache rows expire on their existing 1h TTL (no cleanup pass). Helpers added: - getLastSentAt(rule) — extracted Upstash GET for digest:last-sent so compose + send both call one source of truth. - buildSynthesisCtx(rule, nowMs) — formats profile + greeting for the canonical synthesis call. Preserves all today's prefs-fetch failure-mode behavior. Composer: - compareRules now exported from scripts/lib/brief-compose.mjs so the cron can sort each pass identically to groupEligibleRulesByUser. Test results: full data suite 6962/6962 (was 6956 pre-Step 4; +6 new compose-synthesis tests from Step 3). Plan: docs/plans/2026-04-25-002-fix-brief-email-two-brain-divergence-plan.md Steps 4 + 4b. Codex-approved (5 rounds).
…sed lead Public-share render path (api/brief/public/[hash].ts → renderer publicMode=true) MUST NEVER serve the personalised digest.lead because that string can carry profile context — watched assets, saved-region names, etc. — written by generateDigestProse with ctx.profile populated. Previously: redactForPublic redacted user.name and stories.whyMatters but passed digest.lead through unchanged. Codex Round-2 High (security finding). Now (v3 envelope contract): - redactForPublic substitutes digest.lead = digest.publicLead when the v3 envelope carries one (generated by generateDigestProsePublic with profile=null, cache-shared across all public readers). - When publicLead is absent (v2 envelope still in TTL window OR v3 envelope where publicLead generation failed), redactForPublic sets digest.lead to empty string. - renderDigestGreeting: when lead is empty, OMIT the <blockquote> pull-quote entirely. Page still renders complete (greeting + horizontal rule), just without the italic lead block. - NEVER falls back to the original personalised lead. assertBriefEnvelope still validates publicLead's contract (when present, must be a non-empty string) BEFORE redactForPublic runs, so a malformed publicLead throws before any leak risk. Tests added (tests/brief-magazine-render.test.mjs): - v3 envelope renders publicLead in pull-quote, personalised lead text never appears. - v2 envelope (no publicLead) omits pull-quote; rest of page intact. - empty-string publicLead rejected by validator (defensive). - private render still uses personalised lead. Test results: 68 brief-magazine-render tests pass; full data suite remains green from prior commit. Plan: docs/plans/2026-04-25-002-fix-brief-email-two-brain-divergence-plan.md Step 5, Codex Round-2 High (security).
Adds the parity-contract observability line and supplementary
acceptance tests for the canonical synthesis path.
Parity log (per send, after successful delivery):
[digest] brief lead parity user=<id> rule=<v>:<s>:<lang>
synthesis_level=<1|2|3> exec_len=<n> brief_lead_len=<n>
channels_equal=<bool> public_lead_len=<n>
When channels_equal=false an extra WARN line fires —
"PARITY REGRESSION user=… — email lead != envelope lead." Sentry's
existing console-breadcrumb hook lifts this without an explicit
captureMessage call. Plan acceptance criterion A5.
Tests added (tests/brief-llm.test.mjs, +9):
- generateDigestProsePublic: two distinct callers with identical
(sensitivity, story-pool) hit the SAME cache row (per Codex
Round-2 Medium #4 — "no PII in public cache key").
- public + private writes never collide on cache key (defensive).
- greeting bucket change re-keys the personalised cache (Brain B
parity).
- profile change re-keys the personalised cache.
- v3 cache prefix used (no v2 writes).
Test results: 77/77 in brief-llm; full data suite 6971/6971
(was 6962 pre-Step-7; +9 new public-cache tests).
Plan: docs/plans/2026-04-25-002-fix-brief-email-two-brain-divergence-plan.md
Steps 6 (partial) + 7. Acceptance A5, A6.g, A6.f.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
Greptile SummaryThis PR eliminates the "two-brain" divergence between email and magazine lead text by promoting
Confidence Score: 3/5Safe to merge after fixing the false-positive parity WARN for opt-out users — the core synthesis path and security fail-safes are correct. One P1 found: the parity regression warn fires on every tick for every opted-out user (aiDigestEnabled === false), poisoning the Sentry alert that the PR itself relies on for post-deploy monitoring. The architectural changes, the public-share fail-safe, and the three-level fallback chain are all sound and well-tested. Score held below 4 due to the P1. scripts/seed-digest-notifications.mjs — parity check logic and two-pass winner walk deduplication Important Files Changed
Sequence DiagramsequenceDiagram
participant Cron as Cron tick
participant Compose as composeAndStoreBriefForUser
participant LLM as generateDigestProse (LLM)
participant PubLLM as generateDigestProsePublic
participant Composer as composeBriefFromDigestStories
participant Redis as Redis (Upstash)
participant Send as main() send loop
Cron->>Compose: annotated candidates (due+all)
Compose->>Compose: two-pass winner walk (sortedDue → sortedAll)
Compose->>LLM: L1 — full pool + ctx (profile, greeting)
LLM-->>Compose: synthesis {lead, threads, signals, rankedStoryHashes}
alt L1 fails
Compose->>LLM: L2 — capped slice, empty ctx
LLM-->>Compose: synthesis (degraded)
alt L2 also fails
Compose-->>Compose: L3 stub (synthesisLevel=3)
end
end
Compose->>PubLLM: full pool, no profile/greeting (isPublic=true)
PubLLM-->>Compose: publicLead (non-personalised)
Compose->>Composer: synthesis + publicLead injected
Composer-->>Compose: envelope v3 (digest.lead + publicLead)
Compose->>Redis: SETEX brief:userId:issueSlot
Compose-->>Cron: {envelope, magazineUrl, synthesisLevel}
Cron->>Send: briefByUser map
Send->>Send: briefLead = envelope.data.digest.lead
Send->>Send: all channels (email/Telegram/Slack/Discord/webhook) use briefLead
Send->>Send: parity log (channels_equal = briefLead === envLead)
Reviews (1): Last reviewed commit: "feat(digest): brief lead parity log + ex..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+1749
to
+1755
| // the magazine's digest.lead, the channel-body lead, and the | ||
| // webhook's `summary` field MUST all be the same string. Plan | ||
| // acceptance criterion A5. Log on every send so ops can grep for | ||
| // `channels_equal=false` in Railway logs without manually opening | ||
| // the email + the magazine to compare. | ||
| const envLead = brief?.envelope?.data?.digest?.lead ?? ''; | ||
| const channelsEqual = briefLead === envLead; |
Contributor
There was a problem hiding this comment.
When rule.aiDigestEnabled === false, briefLead is set to null (no summary in channel bodies), but envLead still reads the envelope's synthesized or stub lead. The comparison null !== '<stub lead>' triggers the PARITY REGRESSION console.warn on every tick for every opted-out user, flooding the Sentry surface and making the regression alert useless.
The parity log should be guarded by the same condition that governs briefLead:
if (AI_DIGEST_ENABLED && rule.aiDigestEnabled !== false) {
const envLead = brief?.envelope?.data?.digest?.lead ?? '';
const channelsEqual = briefLead === envLead;
// ... parity log + warn ...
}
Comment on lines
+1315
to
+1340
| * canonical, 2 = degraded, 3 = stub) — drives the email subject-line | ||
| * ternary and the parity log. | ||
| * | ||
| * @param {string} userId | ||
| * @param {Array<{ rule: object; lastSentAt: number | null; due: boolean }>} annotated | ||
| * @param {{ clusters: number; multiSource: number }} insightsNumbers | ||
| * @param {(rule: object) => Promise<unknown[]>} digestFor | ||
| * @param {number} nowMs | ||
| */ | ||
| async function composeAndStoreBriefForUser(userId, candidates, insightsNumbers, digestFor, nowMs) { | ||
| let envelope = null; | ||
| let chosenVariant = null; | ||
| let chosenCandidate = null; | ||
| 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, | ||
| onDrop: (ev) => { dropStats[ev.reason] = (dropStats[ev.reason] ?? 0) + 1; }, | ||
| }, | ||
| ); | ||
| async function composeAndStoreBriefForUser(userId, annotated, insightsNumbers, digestFor, nowMs) { | ||
| // Two-pass walk: prefer DUE rules first (so the synthesis pool + | ||
| // ctx come from a rule that's about to send), fall back to ANY | ||
| // eligible rule (compose-only tick — keeps the dashboard brief | ||
| // refreshed for weekly/twice_daily users on non-due ticks). Within | ||
| // each pass, `compareRules` priority order. Pick first candidate | ||
| // with non-empty pool — mirrors today's behavior at the legacy | ||
| // composeAndStoreBriefForUser walk. | ||
| // Codex Round-3 High #1 + Round-4 High #1 + Round-4 Medium #2. | ||
| const sortedDue = annotated.filter((a) => a.due).sort((a, b) => compareRules(a.rule, b.rule)); | ||
| const sortedAll = [...annotated].sort((a, b) => compareRules(a.rule, b.rule)); | ||
|
|
||
| let winner = null; | ||
| let winnerStories = null; | ||
| for (const cand of [...sortedDue, ...sortedAll]) { | ||
| if (winner) break; | ||
| const stories = await digestFor(cand.rule); |
Contributor
There was a problem hiding this comment.
[...sortedDue, ...sortedAll] contains every due rule twice — once in each sub-array. When a due candidate's pool is empty the outcome=empty-pool log fires on the first pass, then digestFor(cand.rule) is called again for the same rule during the sortedAll pass. This doubles upstream API calls for that candidate and emits duplicate outcome=empty-pool log lines per tick, which confuses operators counting empty-pool events.
A Set of already-visited rule identifiers checked at the top of the for body (before the digestFor call) is sufficient to deduplicate both the fetch and the log line.
Comment on lines
1745
to
+1757
| console.log( | ||
| `[digest] Sent ${stories.length} stories to ${rule.userId} (${rule.variant}, ${rule.digestMode})`, | ||
| ); | ||
| // Parity contract observability — the email's exec block string, | ||
| // the magazine's digest.lead, the channel-body lead, and the | ||
| // webhook's `summary` field MUST all be the same string. Plan | ||
| // acceptance criterion A5. Log on every send so ops can grep for | ||
| // `channels_equal=false` in Railway logs without manually opening | ||
| // the email + the magazine to compare. | ||
| const envLead = brief?.envelope?.data?.digest?.lead ?? ''; | ||
| const channelsEqual = briefLead === envLead; | ||
| const publicLead = brief?.envelope?.data?.digest?.publicLead ?? ''; | ||
| console.log( |
Contributor
There was a problem hiding this comment.
For opted-in users, briefLead and envLead both read from brief?.envelope?.data?.digest?.lead. Since they're the same source, channelsEqual is structurally always true for this path — it only fires false for the opt-out and compose-fail edge cases (which are false positives, see separate P1 comment). The check therefore provides no safety net against a future regression where an email channel is accidentally wired to a different lead source.
If the intent is to verify that all channel bodies use the canonical lead, you'd need to capture the actual string passed to each channel sender and compare it to envLead. Alternatively, logging exec_len vs brief_lead_len and flagging length mismatches is a simpler approach that avoids the opt-out false-positive entirely.
Comment on lines
+337
to
+353
| 'rankedStoryHashes: at least the top 3 stories by editorial importance, ' + | ||
| 'using the short hash from each story line (the value inside [h:...]). ' + | ||
| 'Lead with the single most impactful development. Lead under 250 words.'; | ||
|
|
||
| /** | ||
| * Compute a coarse greeting bucket for cache-key stability. | ||
| * Greeting strings can vary in punctuation/capitalisation across | ||
| * locales; the bucket collapses them to one of three slots so the | ||
| * cache key only changes when the time-of-day window changes. | ||
| * | ||
| * @param {string|null|undefined} greeting | ||
| * @returns {'morning' | 'afternoon' | 'evening' | ''} | ||
| */ | ||
| export function greetingBucket(greeting) { | ||
| if (typeof greeting !== 'string') return ''; | ||
| const g = greeting.toLowerCase(); | ||
| if (g.includes('morning')) return 'morning'; |
Contributor
There was a problem hiding this comment.
greetingBucket returns '' for unrecognised greetings, silently collapsing cache-key stability
When a greeting string doesn't match 'morning', 'afternoon', 'evening', or 'night' (e.g. a locale-specific greeting or an empty string after locale changes), greetingBucket returns ''. That's a valid bucket, but it collapses all unrecognised greetings into a single slot — a user whose greeting swaps between a known and unknown value will get different cache keys, behaving like a cache miss even though the temporal bucket didn't change. Adding a comment noting this intentional behaviour would prevent future confusion.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
…surface
Two findings from human review of the canonical-synthesis PR:
1. Public-share redaction leaked personalised signals + threads.
The new prompt explicitly personalises both `lead` and `signals`
("personalise lead and signals"), but redactForPublic only
substituted `lead` — leaving `signals` and `threads` intact.
Public renderer's hasSignals gate would emit the signals page
whenever `digest.signals.length > 0`, exposing watched-asset /
region phrasing to anonymous readers. Same privacy bug class
the original PR was meant to close, just on different fields.
2. Multi-rule users got cross-pool lead/storyList mismatch.
composeAndStoreBriefForUser picks ONE winning rule for the
canonical envelope. The send loop then injected that ONE
`briefLead` into every due rule's channel body — even though
each rule's storyList came from its own (per-rule) digest pool.
Multi-rule users (e.g. `full` + `finance`) ended up with email
bodies leading on geopolitics while listing finance stories.
Cross-rule editorial mismatch reintroduced after the cross-
surface fix.
Fix 1 — public signals + threads:
- Envelope shape: BriefDigest gains `publicSignals?: string[]` +
`publicThreads?: BriefThread[]` (sibling fields to publicLead).
Renderer's ALLOWED_DIGEST_KEYS extended; assertBriefEnvelope
validates them when present.
- generateDigestProsePublic already returned a full prose object
(lead + signals + threads) — orchestration now captures all
three instead of just `.lead`. Composer splices each into its
envelope slot.
- redactForPublic substitutes:
digest.lead ← publicLead (or empty → omits pull-quote)
digest.signals ← publicSignals (or empty → omits signals page)
digest.threads ← publicThreads (or category-derived stub via
new derivePublicThreadsStub helper — never
falls back to the personalised threads)
- New tests cover all three substitutions + their fail-safes.
Fix 2 — per-rule synthesis in send loop:
- Each due rule independently calls runSynthesisWithFallback over
ITS OWN pool + ctx. Channel body lead is internally consistent
with the storyList (both from the same pool).
- Cache absorbs the cost: when this is the winner rule, the
synthesis hits the cache row written during the compose pass
(same userId/sensitivity/pool/ctx) — no extra LLM call. Only
multi-rule users with non-overlapping pools incur additional
LLM calls.
- magazineUrl still points at the winner's envelope (single brief
per user per slot — `(userId, issueSlot)` URL contract). Channel
lead vs magazine lead may differ for non-winner rule sends;
documented as acceptable trade-off (URL/key shape change to
support per-rule magazines is out of scope for this PR).
- Parity log refined: adds `winner_match=<bool>` field. The
PARITY REGRESSION warning now fires only when winner_match=true
AND the channel lead differs from the envelope lead (the actual
contract regression). Non-winner sends with legitimately
different leads no longer spam the alert.
Test results:
- tests/brief-magazine-render.test.mjs: 75/75 (+7 new for public
signals/threads + validator + private-mode-ignores-public-fields)
- Full data suite: 6995/6995 (was 6988; +7 net)
- typecheck + typecheck:api: clean
Plan: docs/plans/2026-04-25-002-fix-brief-email-two-brain-divergence-plan.md
Addresses 2 review findings on PR #3396 not anticipated in the
5-round Codex review.
10 tasks
Address two residual risks in PR #3396 (single-canonical-brain refactor): Risk 1 — canonical lead synthesized from a fixed 24h pool while the send loop ships stories from `lastSentAt ?? 24h`. For weekly users that meant a 24h-pool lead bolted onto a 7d email body — the same cross-surface divergence the refactor was meant to eliminate, just in a different shape. Twice-daily users hit a 12h-vs-24h variant. Fix: extract the window formula to `digestWindowStartMs(lastSentAt, nowMs, defaultLookbackMs)` in digest-orchestration-helpers.mjs and call it from BOTH the compose path's digestFor closure AND the send loop. The compose path now derives windowStart per-candidate from `cand.lastSentAt`, identical to what the send loop will use for that rule. Removed the now-unused BRIEF_STORY_WINDOW_MS constant. Side-effect: digestFor now receives the full annotated candidate (`cand`) instead of just the rule, so it can reach `cand.lastSentAt`. Backwards-compatible at the helper level — pickWinningCandidateWithPool forwards `cand` instead of `cand.rule`. Cache memo hit rate drops since lastSentAt varies per-rule, but correctness > a few extra Upstash GETs. Risk 2 — pickWinningCandidateWithPool returned the first candidate with a non-empty raw pool as winner. If composeBriefFromDigestStories then dropped every story (URL/headline/shape filters), the caller bailed without trying lower-priority candidates. Pre-PR behaviour was to keep walking. This regressed multi-rule users whose top-priority rule's pool happens to be entirely filter-rejected. Fix: optional `tryCompose(cand, stories)` callback on pickWinningCandidateWithPool. When provided, the helper calls it after the non-empty pool check; falsy return → log filter-rejected and walk to the next candidate; truthy → returns `{winner, stories, composeResult}` so the caller can reuse the result. Without the callback, legacy semantics preserved (existing tests + callers unaffected). Caller composeAndStoreBriefForUser passes a no-synthesis compose call as tryCompose — cheap pure-JS, no I/O. Synthesis only runs once after the winner is locked in, so the perf cost is one extra compose per filter-rejected candidate, no extra LLM round-trips. Tests: - 10 new cases in tests/digest-orchestration-helpers.test.mjs covering: digestFor receiving full candidate; tryCompose fall-through to lower-priority; all-rejected returns null; composeResult forwarded; legacy semantics without tryCompose; digestWindowStartMs lastSentAt-vs-default branches; weekly + twice-daily window parity assertions; epoch-zero ?? guard. - Updated tests/digest-cache-key-sensitivity.test.mjs static-shape regex to match the new `cand.rule.sensitivity` cache-key shape (intent unchanged: cache key MUST include sensitivity). Stacked on PR #3396 — targets feat/brief-two-brain-divergence.
Owner
Author
|
Both residual risks raised in review are now addressed in this PR via commit 5d10cee (rebased on top of c6f9227): Risk 1 — canonical lead vs send-channel window divergence Extracted Side effect: Risk 2 — filter-rejected top-priority candidate suppresses brief Added optional Test coverage (+10 new cases in
Full |
3 tasks
koala73
added a commit
that referenced
this pull request
Apr 25, 2026
P1 — False-positive PARITY REGRESSION for AI-digest opt-out users
(scripts/seed-digest-notifications.mjs)
When rule.aiDigestEnabled === false, briefLead is intentionally
null (no summary in channel bodies), but envLead still reads the
envelope's stub lead. The string comparison null !== '<stub lead>'
fired channels_equal=false on every tick for every opted-out user
— flooding the parity log with noise and risking the PARITY
REGRESSION alert becoming useless.
The WARN was already gated by `briefLead && envLead` (so no Sentry
flood), but the LOG line still misled operators counting
channels_equal=false. Gate the entire parity-log block on the same
condition that governs briefLead population:
if (AI_DIGEST_ENABLED && rule.aiDigestEnabled !== false) {
// … parity log + warn …
}
Opt-out users now produce no parity-log line at all (correct —
there's no canonical synthesis to compare against).
P4 — greetingBucket '' fallback semantics
(scripts/lib/brief-llm.mjs)
Doc-only — Greptile flagged that unrecognised greetings collapse
to '' (a single bucket). Added a comment clarifying this is
intentional: '' is a stable fourth bucket, not a sentinel for
"missing data". A user whose greeting flips between recognised
and unrecognised values gets different cache keys, which is
correct (those produce visibly different leads).
Other Greptile findings (no code change — replied via PR comment):
- P2 (double-fetch in [...sortedDue, ...sortedAll]): already
addressed in helper extraction commit df35630 of PR #3396 —
see `seen` Set dedupe at scripts/lib/digest-orchestration-helpers.mjs:103.
- P2 (parity check no-op for opted-in): outdated as written —
after 5d10cee's per-rule synthesis, briefLead is per-rule and
envLead is winner-rule's envelope.lead. They diverge for
non-winner rules (legitimate); agree for winner rules (cache-
shared via generateDigestProse). The check still serves its
documented purpose for cache-drift detection.
Stacked on the merged PR #3396; opens as a follow-up since the
parent branch is now closed.
Test results: 7012/7012 (was 7006 pre-rebase onto post-merge main).
5 tasks
koala73
added a commit
that referenced
this pull request
Apr 25, 2026
…mpt (#3411) * fix(brief): restore email multi-section format + de-vapidify lead prompt 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. * fix(brief): rename escape → htmlEscape (biome noShadowRestrictedNames) * fix(brief): omit Signals to watch header when all bullets dropped (Greptile P2)
6 tasks
koala73
added a commit
that referenced
this pull request
May 12, 2026
…eads (#3667) * fix(brief): grounding gate on digest synthesis — block hallucinated leads The 2026-05-12 0801 send shipped a "President Biden announced a new executive order targeting cryptocurrency mixers and privacy coins" lead to users whose actual story pool was Trump-era geopolitics (Iran ceasefire, Israeli strikes in Lebanon, Sudan drones, Cuba rhetoric, Russia/Ukraine, EU sanctions). The magazine envelope rendered the correct grounded lead; the email Executive Summary block rendered a fully fabricated narrative with four threads all about the imaginary Biden EO. Shape was valid — content was a hallucination from training- data priors. Root cause: validateDigestProseShape only checks shape (lengths, types, array presence). Any well-formed JSON whose lead names entities absent from the input pool was happily cached for 4h and re-served on every cache hit until the row TTL'd out. The canonical-brain promise from PR #3396 ("compose-phase synthesis is reused on send-phase cache read") only guarantees same-output-per-cache-hit; it does not guard against the LLM committing to a fabricated row in the first place. This adds checkLeadGrounding(synthesis, stories): extract proper-noun tokens (capitalised, length ≥4) from story headlines; require ≥2 distinct hits in the lead + thread teasers (relaxed to 1 when the corpus has <4 anchor tokens). Length cap of 4 deliberately filters short-form acronyms (US/EU/UN/RSF) which are too generic to discriminate. Plumbed through validateDigestProseShape (new optional stories arg, back-compat preserved) → parseDigestProse → generateDigestProse cache-hit path. Cache key bumped v4 → v5 so existing v4 rows (which may carry shape-valid hallucinations) are evicted on rollout; 4h paid-for-once cost matching the established pattern at this function. When a cached row fails the grounding gate, the cron's existing 3-level fallback chain falls through to L2 (capped pool, no profile/greeting) and ultimately L3 (stub with "Digest" subject). A user gets either a re-rolled grounded lead or a degraded subject line — never a hallucinated headline. Test coverage: the verbatim May 12 hallucinated lead + the verbatim 2026-05-12 story headlines as fixtures, asserting the validator rejects. Plus the actual magazine lead from the same day as positive control, plus skip-on-empty-stories back-compat, plus single-story threshold relaxation, plus short-acronym filtering, plus a generateDigestProse cache-hit re-LLM regression test. 86/86 brief-llm tests pass. 179/179 brief-related tests pass. typecheck clean. biome lint clean. * fix(brief): address PR #3667 review — lead must ground independently + token-set matching Two real bugs caught on review of the grounding validator. #1 — Combined haystack let a hallucinated lead pass when teasers happened to mention real entities. Before: lead + threads[].teaser were joined into one string and any ≥2 anchor hits across the combination passed. So a synthesis with a fabricated "President Biden announced..." lead and grounded teasers like "Trump rejected Tehran's response" would accept — even though the visible top-of-email lead stayed fabricated. After: lead must independently hit ≥1 anchor. Combined check (≥2 hits, or ≥1 for sparse corpora) still applies on top of that. A hallucinated lead with grounded teasers now correctly rejects. #2 — haystack.includes(tok) accepted unrelated entities via substring match. Before: anchor "iran" hit on "tirana", "oman" on "romania", "india" on "indiana". Any anchor that happens to appear as a substring of an unrelated word in the synthesis prose counted. After: both sides tokenise on the same delimiter regex into Sets and match by membership. Story-side keeps the proper-noun capitalisation filter; synthesis-side does not (so possessive forms / sentence-medial mentions still count as anchor hits). Both regressions covered by new golden tests: - hallucinated-lead-grounded-threads: rejects with the new lead independence requirement, accepts when the lead is also grounded - substring-trap-corpus (Iran/Oman/India + Tirana/Romania/Indiana): rejects under token-set matching, accepts under real word matches Tests: 88/88 brief-llm, 8604/8604 full suite, typecheck clean, biome clean. * fix(brief): address PR #3667 review round 2 — stopword filter + Unicode delimiters Two more bugs in the grounding validator caught on second review. #1 (HIGH) — Generic capitalised words leaked into the anchor set. Before: any capitalised word ≥4 chars from a headline became an anchor. So "President Trump signed Iran sanctions" added "president" to storyTokens. A hallucinated lead like "President Biden announced..." passed the lead-anchor check via the shared "president" token, and a teaser mentioning Iran satisfied the combined threshold — recreating the exact failure mode this PR is trying to block. After: GROUNDING_ANCHOR_STOPWORDS filters honorifics ("President", "Senator", "Minister"), generic role labels ("Officials", "Members", "Forces"), quasi-adjectives ("Senior", "Federal", "Former"), sentence-start filler ("After", "Following", "Despite"), and calendar words. Specific entity names (Iran, Trump, Israel, EU) are deliberately NOT on the list. "May" is also omitted (Theresa May, May Day, May the month all collide). #2 (MEDIUM) — Unicode apostrophes weren't delimiters. Before: GROUNDING_TOKEN_DELIMS only included ASCII apostrophe. Reuters/AP/Guardian headlines use U+2019 ("China's", "Iran's", "DPRK's"). The regex didn't split on U+2019, so "China's" became a single token "china's" that no normal lead saying "China" could ever match — a false negative that would reject genuinely grounded leads. After: U+2018, U+2019, U+201C, U+201D, U+00B4 added alongside ASCII counterparts. Both regressions covered by new golden tests: - "President Trump signed..." headlines + "President Biden announced..." hallucinated lead must REJECT - U+2019 headlines + ASCII-quote grounded lead must ACCEPT Tests: 90/90 brief-llm, 8606/8606 full suite, typecheck clean, biome clean. * fix(brief): address PR #3667 review round 3 — bigram-leading title stopwords Round 2 added "President" to the anchor stopword list. Round 3 review caught that other common bigram titles still leak through. Before: "Prime Minister Netanyahu says Iran threats continue" added "prime" to anchors. A hallucinated "Prime Minister Trudeau announced cryptocurrency restrictions..." passed the lead-anchor check via the shared "prime" token. A teaser mentioning Iran satisfied the combined threshold. Same shape worked for Chief Justice / Cardinal X / Chancellor X / Speaker X / Ambassador X. After: GROUNDING_ANCHOR_STOPWORDS extended with bigram-leading titles: prime, chief, premier, chancellor, speaker, ambassador, envoy, commissioner, attorney, cardinal, archbishop, monsignor, reverend, pastor, bishop, lord, lady, dame, congressman/woman/person, representative, delegate, baron(ess). Regression test covers Prime Minister / Chief Justice / Cardinal ride-along leads, plus a counter-control naming the actual entity (Netanyahu) which still passes. Tests: 91/91 brief-llm, 8607/8607 full suite, typecheck clean, biome clean. * fix(brief): observability for digest synthesis failures (PR #3667 review #3) Greptile review caught: when generateDigestProse returns null, ops can't tell whether the LLM threw, returned no content, returned malformed JSON, or returned a shape-valid hallucination that the grounding gate rejected. All four classes look identical in logs — just "L1 returned null" — so a sustained model regression looks identical to an infra blip during on-call triage. Add two distinct console.warn lines: - "LLM call threw" — provider/network failure (catches the actual error message for triage) - "ungrounded or malformed output" — provider returned but parsing or grounding rejected (logs text length so 0 vs ~900-char distinguishes "no content" from "model drift/hallucination") Cost note documented inline: we deliberately do NOT cache the failure with a sentinel under the v5 key. At temperature 0.4 the next cron tick may roll a grounded output for the same prompt; caching the failure would block legitimate retries. The L1→L2→L3 fallback in runSynthesisWithFallback handles user-visible degradation; these logs handle ops visibility. Tests: 91/91 brief-llm pass, typecheck clean, biome clean. * fix(brief): PR #3667 review round 4 cleanup — extract grounding helpers + fixture comment Two reviewer-flagged cleanups: - P2: extract `extractAnchors` / `tokensetOf` from inside checkLeadGrounding to file-level `extractAnchorTokens` / `groundingTokenSet`. Avoids closure re-instantiation per call, separates the two helpers cleanly, and makes them individually inspectable / unit-testable. Behaviour unchanged — same callers, same inputs, same outputs. - P3: add a load-bearing comment on the `story()` test factory documenting that the default headline ("Iran threatens to close Strait of Hormuz...") is what grounds every `validJson` lead used by `generateDigestProse` / `generateDigestProsePublic` cache-shape tests. If a future contributor changes the default headline so it no longer mentions Iran/Hormuz, those tests would silently reject via the v5 grounding gate with cascading "expected truthy, got null" failures whose root cause is invisible. Comment names the invariant + the escape hatch (override headline + update validJson). Tests: 91/91 brief-llm pass, typecheck clean, biome clean. * fix(brief): PR #3667 review round 5 — JSDoc attachment, blind-spot warn, stopword heuristic Three reviewer findings: #1 — JSDoc block was orphaned by the round-4 helper extraction. The big "Cheap content-grounding check..." doc sat at line 519, BEFORE extractAnchorTokens and groundingTokenSet — JSDoc parsers attach a doc block to the NEXT declaration, so the rich docs were describing the wrong function. Moved directly above checkLeadGrounding where they belong. #2 — Lowercase-headline blind spot. If a feed ever produces all- lowercase or all-≤3-char headlines, extractAnchorTokens returns empty for every story, storyTokens.size === 0, and the gate silently returns true (skip). Added a console.warn gated on stories.length >= 3 so synthetic single-headline test corpora don't spam logs but real production feed regressions surface in cron output. #3 — Stopword maintenance heuristic. Added a comment to GROUNDING_ANCHOR_STOPWORDS describing the detection rule: dump a week of real headlines, tokenise with stopwords disabled, count frequencies, inspect any token appearing in >~10% of headlines that isn't a known proper noun. The Prime/Chief/Cardinal gaps caught on rounds 2-3 would have surfaced from such a frequency audit. Captures the maintenance burden as an actionable signal rather than guesswork. Tests: 91/91 brief-llm pass, typecheck clean, biome clean. The new console.warn calls are intentionally surface in test output when triggered (reviewer round 5 #4 — informational, no action).
Merged
7 tasks
koala73
added a commit
that referenced
this pull request
May 14, 2026
… parity regression (#3685) * fix(brief): single canonical synthesis — kill the compose/enrich/send parity regression The May 14 0802 brief tripped `PARITY REGRESSION` for a real user: the email channel lead (403 chars) and the magazine envelope lead (373 chars) were different syntheses. The canonical-brain promise from PR #3396 ("one synthesis, every channel byte-identical") had regressed — there were THREE synthesis call sites, not one: 1. Compose pass — runSynthesisWithFallback (seed-digest:1530), ctx-aware, spliced into the envelope. Its output never survived. 2. enrichBriefEnvelopeWithLLM — generateDigestProse (brief-llm.mjs) with a 4-arg NO-ctx call that OVERWROTE envelope.data.digest. The comment claimed "this pass only fills per-story rationales"; it lied — it re-synthesised the whole digest prose. 3. Send pass — runSynthesisWithFallback (seed-digest:1949) against a different `stories` pool, for the channel bodies. So compose built a ctx-aware synthesis, enrich threw it away and re-rolled context-free, and send rolled a third time. At temperature 0.4 the three rolls diverge. Fix — collapse to ONE synthesis, produced by the compose pass: - enrichBriefEnvelopeWithLLM gains `opts.skipDigestProse`. When set, it does ONLY per-story whyMatters/description enrichment and leaves envelope.data.digest untouched — no second generateDigestProse call. The compose path passes skipDigestProse:true (the canonical synthesis is already spliced in). Call site 2 is gone. - composeAndStoreBriefForUser now returns the `synthesis` object on its briefByUser entry (alongside the existing synthesisLevel). - The send pass no longer calls runSynthesisWithFallback. It reads briefSynthesis / briefLead / synthesisLevel from the compose-pass result on the briefByUser entry. Call site 3 is gone. - L3 / opt-out gate preserved: briefSynthesis/briefLead are only set when AI_DIGEST_ENABLED && rule.aiDigestEnabled !== false && synthesisLevel !== 3. The persisted envelope always carries a digest.lead (the L3 stub included), so reading from the briefByUser entry — NOT the envelope — is what keeps L3/opt-out users from a fake "Executive Summary". - Parity log: channels_equal is now `n/a` (not a misleading `false`) when there is no channel synthesis (L3 / opt-out / compose-miss), so those users stop tripping a false PARITY REGRESSION every tick. The remaining PARITY REGRESSION branch fires only on a genuine post-compose envelope mutation — which after this change should be unreachable for L1/L2. Legitimate non-violations preserved: generateDigestProsePublic stays (separate public/share synthesis), and the L1→L2 fallback inside runSynthesisWithFallback may still call generateDigestProse twice within one canonical attempt. Tests: 2 new enrichBriefEnvelopeWithLLM cases — skipDigestProse:true suppresses the generateDigestProse call + leaves digest untouched + still enriches whyMatters; default (no opts) still synthesises (back-compat). 93/93 brief-llm tests, 8702/8702 full unit suite, typecheck clean. Plan: docs/plans/2026-05-14-001-fix-brief-pipeline-parity-grounding-opinion-plan.md (F1, Phase 1). Codex-reviewed (3 rounds, APPROVED). Phase 1.5 (F5 root cause) finding: a harness proved orderBriefCandidates cannot produce the May 14 interleaved severity walk under any topic assignment — H2 (comparator bug) ruled out, H1 confirmed. The comparator is untouched; F5 gets a post-deploy verification step. * fix(brief): compose-miss no longer trips a false winner_match PARITY REGRESSION Review catch on PR #3685. The Phase-1 change fixed the channels_equal false-positive for L3/opt-out, but compose-miss still tripped the OTHER alarm: when briefByUser has no entry, `brief` is undefined, so the parity block computed winnerVariant='' → winnerMatch=false → fired `PARITY REGRESSION winner_match=false` every compose-miss tick. Split the parity block: when `!brief` (compose-miss), emit an informational parity line with winner_match=n/a channels_equal=n/a reason=compose-miss and skip BOTH alarms — the compose-miss is already logged separately via `[digest] compose-miss user=…`. The winner_match alarm now only fires when `brief` exists, which is the genuine bug case (canonical-rule filter bypass / chosenVariant drift). Updated the stale "unreachable in practice" comment to note it is reached only with a present `brief`. Pre-existing bug (compose-miss always set winnerVariant=''), but this PR owns the parity-log block and Phase 1 step 5 of the plan explicitly scopes "no compose-pass result → n/a, not a false regression" — compose-miss is the same family as L3/opt-out. typecheck clean, 143/143 brief-llm + digest-orchestration tests pass.
koala73
added a commit
that referenced
this pull request
May 14, 2026
…+ restore the grounding gate (#3688) * fix(brief): single canonical synthesis — kill the compose/enrich/send parity regression The May 14 0802 brief tripped `PARITY REGRESSION` for a real user: the email channel lead (403 chars) and the magazine envelope lead (373 chars) were different syntheses. The canonical-brain promise from PR #3396 ("one synthesis, every channel byte-identical") had regressed — there were THREE synthesis call sites, not one: 1. Compose pass — runSynthesisWithFallback (seed-digest:1530), ctx-aware, spliced into the envelope. Its output never survived. 2. enrichBriefEnvelopeWithLLM — generateDigestProse (brief-llm.mjs) with a 4-arg NO-ctx call that OVERWROTE envelope.data.digest. The comment claimed "this pass only fills per-story rationales"; it lied — it re-synthesised the whole digest prose. 3. Send pass — runSynthesisWithFallback (seed-digest:1949) against a different `stories` pool, for the channel bodies. So compose built a ctx-aware synthesis, enrich threw it away and re-rolled context-free, and send rolled a third time. At temperature 0.4 the three rolls diverge. Fix — collapse to ONE synthesis, produced by the compose pass: - enrichBriefEnvelopeWithLLM gains `opts.skipDigestProse`. When set, it does ONLY per-story whyMatters/description enrichment and leaves envelope.data.digest untouched — no second generateDigestProse call. The compose path passes skipDigestProse:true (the canonical synthesis is already spliced in). Call site 2 is gone. - composeAndStoreBriefForUser now returns the `synthesis` object on its briefByUser entry (alongside the existing synthesisLevel). - The send pass no longer calls runSynthesisWithFallback. It reads briefSynthesis / briefLead / synthesisLevel from the compose-pass result on the briefByUser entry. Call site 3 is gone. - L3 / opt-out gate preserved: briefSynthesis/briefLead are only set when AI_DIGEST_ENABLED && rule.aiDigestEnabled !== false && synthesisLevel !== 3. The persisted envelope always carries a digest.lead (the L3 stub included), so reading from the briefByUser entry — NOT the envelope — is what keeps L3/opt-out users from a fake "Executive Summary". - Parity log: channels_equal is now `n/a` (not a misleading `false`) when there is no channel synthesis (L3 / opt-out / compose-miss), so those users stop tripping a false PARITY REGRESSION every tick. The remaining PARITY REGRESSION branch fires only on a genuine post-compose envelope mutation — which after this change should be unreachable for L1/L2. Legitimate non-violations preserved: generateDigestProsePublic stays (separate public/share synthesis), and the L1→L2 fallback inside runSynthesisWithFallback may still call generateDigestProse twice within one canonical attempt. Tests: 2 new enrichBriefEnvelopeWithLLM cases — skipDigestProse:true suppresses the generateDigestProse call + leaves digest untouched + still enriches whyMatters; default (no opts) still synthesises (back-compat). 93/93 brief-llm tests, 8702/8702 full unit suite, typecheck clean. Plan: docs/plans/2026-05-14-001-fix-brief-pipeline-parity-grounding-opinion-plan.md (F1, Phase 1). Codex-reviewed (3 rounds, APPROVED). Phase 1.5 (F5 root cause) finding: a harness proved orderBriefCandidates cannot produce the May 14 interleaved severity walk under any topic assignment — H2 (comparator bug) ruled out, H1 confirmed. The comparator is untouched; F5 gets a post-deploy verification step. * fix(brief): compose-miss no longer trips a false winner_match PARITY REGRESSION Review catch on PR #3685. The Phase-1 change fixed the channels_equal false-positive for L3/opt-out, but compose-miss still tripped the OTHER alarm: when briefByUser has no entry, `brief` is undefined, so the parity block computed winnerVariant='' → winnerMatch=false → fired `PARITY REGRESSION winner_match=false` every compose-miss tick. Split the parity block: when `!brief` (compose-miss), emit an informational parity line with winner_match=n/a channels_equal=n/a reason=compose-miss and skip BOTH alarms — the compose-miss is already logged separately via `[digest] compose-miss user=…`. The winner_match alarm now only fires when `brief` exists, which is the genuine bug case (canonical-rule filter bypass / chosenVariant drift). Updated the stale "unreachable in practice" comment to note it is reached only with a present `brief`. Pre-existing bug (compose-miss always set winnerVariant=''), but this PR owns the parity-log block and Phase 1 step 5 of the plan explicitly scopes "no compose-pass result → n/a, not a false regression" — compose-miss is the same family as L3/opt-out. typecheck clean, 143/143 brief-llm + digest-orchestration tests pass. * fix(brief): synthesis-boundary adapter — un-starve the digest prompt + restore the grounding gate PR B of the brief-pipeline plan (Phase 2, F2 + F8). Stacked on PR A (#3685). ESCALATED FINDING. The plan scoped F2 as "grounding gate inert." A direct probe proved it is far worse: the synthesis prompt receives ZERO story content. `buildDigest` pushes stories shaped { hash, title, link, severity, currentScore, mentionCount, phase, sources, description }. `digestFor` → `buildDigest` with no transform, so `runSynthesisWithFallback` gets that raw shape. But `buildDigestPrompt`, `checkLeadGrounding`, and `hashDigestInput` all read { headline, threatLevel, source, category, country } — fields the raw shape does not have. The title→headline / severity→threatLevel mapping only happens inside `digestStoryToUpstreamTopStory`, which runs at COMPOSE time, AFTER synthesis. Probe — feeding the verbatim buildDigest shape into buildDigestPrompt: 01. [h:abc12345] [] undefined — undefined · undefined · undefined 02. [h:def67890] [] undefined — undefined · undefined · undefined Every story line is "undefined". The model gets the sensitivity level, the greeting, and 12 lines of "[h:hash] [] undefined" — no story content. It confabulates the entire lead/threads/signals from training-data priors. This — not F1 alone — is the direct cause of the May 12 Biden+crypto hallucination and the May 14 "Ukraine attacks Russian energy" confabulation. checkLeadGrounding seeing an empty headline and skipping is the SAME mismatch, downstream. Fix — one synthesis-boundary adapter: - `digestStoryToSynthesisShape` (brief-compose.mjs) maps the raw buildDigest story → { headline, threatLevel, source, category, country, hash }. The headline gets the same prefix/suffix cleanup the magazine headline gets (so the lead grounds against the same text the reader sees). category/country default to General/Global (matching digestStoryToUpstreamTopStory + filterTopStories). - F8: every free-text field (headline, source, category, country) is run through `sanitizeForPrompt` — the digest-prose prompt carries the reader's profile context, so an unsanitised hostile RSS <title> was a prompt-injection vector. threatLevel (enum) and hash (digest) are not sanitised. - Wired at the single synthesis boundary in composeAndStoreBriefForUser: winnerStories → synthesisStories once, passed to BOTH runSynthesisWithFallback (L1 + L2-slice inherit) AND generateDigestProsePublic. composeBriefFromDigestStories keeps the raw winnerStories — digestStoryToUpstreamTopStory expects it. Tests: - 6 digestStoryToSynthesisShape unit tests (mapping, headline cleanup, source fallback, explicit category/country, F8 sanitization, missing-input safety) - 4 integration tests through the FULL live-path raw→adapted shape: buildDigestPrompt produces real content not "undefined"; checkLeadGrounding RUNS (non-empty storyTokens) and both accepts a grounded lead and rejects a fabricated one; the May 12 Biden+crypto hallucination is rejected; a hostile <title> is sanitized before the prompt. 97/97 brief-llm, 76/76 brief-from-digest-stories, 8712/8712 full unit suite, typecheck clean, biome clean (2 pre-existing complexity warnings unchanged). Note: cache key prefix not bumped — the synthesis cache material (headline/threatLevel/category/country/source) goes from all-empty to real values, so old v5 rows naturally miss; one tick of re-synthesis on rollout, same cost as any shape fix. * fix(brief): address PR #3688 review — headline sanitizer + empty-source guard Greptile P2 review of digestStoryToSynthesisShape: 1. Headline sanitizer. The adapter ran the headline through the full `sanitizeForPrompt`, which strips SEMANTIC injection phrases — it would mangle a legitimate headline whose subject IS such a phrase ("Senator urges Trump to ignore all previous instructions on tariffs" → "Senator urges Trump to on tariffs"), silently degrading the synthesis this PR restores. The codebase already has `sanitizeHeadline` (structural-delimiters-only) for exactly this. Switched the headline field to it. Structural-only sanitisation left one gap: a multi-line hostile `<title>` could still break the per-story prompt line into a line-start "assistant:" role turn. Closed by normalising the title to a single line up front (a headline is one line by definition). Net: structural injection (newline role-turn, model-delimiter tokens) is neutralised; a phrase quoted as a news SUBJECT survives. 2. Empty-source guard. `sources: ['']` (and whitespace-only) passed the `typeof === 'string'` guard and produced a blank `source` field — a prompt line rendered "… — General · Global · " with a trailing empty attribution. Added a `.trim().length > 0` check so it falls back to "Multiple wires". Non-headline free-text fields (source/category/country) keep the full `sanitizeForPrompt` — they are metadata, not headlines. Tests updated: the F8 headline test now asserts the corrected behaviour (semantic phrase preserved, structural injection neutralised) and the source-fallback test covers empty/whitespace entries. 418 brief-compose/llm tests pass; biome clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Eliminates the email/brief "two-brain" lead divergence by promoting
generateDigestProse()to the single canonical synthesis. One LLM call per user produces the lead text every channel reads (email HTML, plain-text email, Telegram, Slack, Discord, webhook) AND the magazine'sdigest.lead.User-visible failure that triggered this work (2026-04-25 0802 brief):
digest.lead: "The global landscape is increasingly dominated by the escalating conflict between the US and Iran…"Same
?t=token, sameissueDateslot — two contradictory editorial narratives. Architectural cause: two independent LLM calls (generateAISummaryover the 20-story digest pool for the email;generateDigestProseover the 12-story envelope for the magazine) producing different leads from different inputs.Plan reviewed iteratively by Codex (gpt-5.4) across 5 rounds, final verdict APPROVED. Plan lives at
docs/plans/2026-04-25-002-fix-brief-email-two-brain-divergence-plan.md(gitignored — workflow convention).Architecture
Single canonical synthesis path:
generateDigestProse(userId, fullPool, sensitivity, deps, ctx)— extended to acceptctx={profile, greeting, isPublic}. Ports Brain B's prompt features intobuildDigestPrompt:[CRITICAL]/[HIGH]/[MEDIUM]severity prefixes, profile lines, greeting opener.generateDigestProsePublic(stories, sensitivity, deps)— no-userId wrapper for the share-URL surface. Substitutes'public'for the userId slot in the cache key, so all anonymous share-URL readers of the same(date, sensitivity, story-pool)hit ONE cache row.brief:llm:digest:v2→v3(rollout cost: one tick of misses, same pattern as v1→v2).Per-user orchestration (composeAndStoreBriefForUser):
sortedDue), fall back to ALL eligible rules (sortedAll) for compose-only ticks. Preserves today's dashboard refresh contract for weekly/twice_daily users on non-due ticks.compareRulespriority and pick the FIRST candidate with a non-empty pool — mirrors today's behavior, fixes the highest-priority-but-empty-pool edge case.synthesisLevellives in the cron-localbriefByUsermap, NOT in the persisted envelope (renderer'sassertNoExtraKeyswould reject).All-channel parity:
briefByUser.get(rule.userId).envelope.data.digest.lead.generateAISummaryanddigest:ai-summary:v1:*cache rows removed (TTL-expire naturally).[digest] brief lead parity user=… channels_equal=<bool>log line per send.channels_equal=falsetriggers a loud WARN that Sentry's console-breadcrumb hook lifts.Envelope v3:
BRIEF_ENVELOPE_VERSION2 → 3.SUPPORTED_ENVELOPE_VERSIONS = [1, 2, 3]keeps v1+v2 envelopes in TTL window readable through the rollout.BriefDigest.publicLead?: string— non-personalised parallel synthesis for the share-URL surface. Renderer'sassertNoExtraKeysdigest-allowed-keys list extended; new validator ensurespublicLeadis a non-empty string when present.Public-share fail-safe (security):
redactForPublicsubstitutesdigest.leadwithdigest.publicLeadwhen present, OR with empty string when absent.renderDigestGreetingomits the<blockquote>whenleadis empty.Composer (pure, sync — no I/O added):
composeBriefFromDigestStoriesaccepts optionalsynthesis = { lead, threads, signals, rankedStoryHashes?, publicLead? }. Splices into envelope.digest after stub assembly.digestStoryToUpstreamTopStoryplumbshashthrough to the upstream-like shape sofilterTopStoriescan re-order byrankedStoryHashesBEFORE the cap. Editorial importance survivesMAX_STORIES_PER_USER.Codex review history (5 rounds)
Files changed
scripts/lib/brief-llm.mjs— canonical synthesis prompt + v3 cache +generateDigestProsePublic+greetingBuckethelperscripts/lib/brief-compose.mjs—compareRulesexported;digestStoryToUpstreamTopStoryemitshash; composer acceptssynthesisarg + splicesscripts/seed-digest-notifications.mjs— orchestration restructure (2-pass walk, 3-level fallback),getLastSentAt+buildSynthesisCtxhelpers, all-channel rewiring, parity log; removesgenerateAISummary(~85 LOC)shared/brief-envelope.{js,d.ts}—BRIEF_ENVELOPE_VERSION2→3;BriefDigest.publicLead?shared/brief-filter.{js,d.ts}—filterTopStoriesacceptsrankedStoryHashes;applyRankedOrderhelper;UpstreamTopStory.hashtypedefserver/_shared/brief-render.js—ALLOWED_DIGEST_KEYSincludespublicLead;redactForPublicsubstitutes safe lead;renderDigestGreetingomits empty pull-quotetests/brief-llm.test.mjs(+17 new),tests/brief-from-digest-stories.test.mjs(+6 new),tests/brief-magazine-render.test.mjs(+8 new)Test plan
npm run typecheckcleannpm run typecheck:apicleanbiome lintclean for all changed files (only pre-existingmain()complexity warning, also flagged by PR fix(digest): brief filter-drop instrumentation + cache-key correctness #3387 as unchanged)npm run test:data— 6971/6971 passing (+15 new tests vs main)briefByUser.get(rule.userId).envelope.data.digest.leadPost-Deploy Monitoring & Validation
Log queries (Railway
scripts-cron-digest-notifications):[digest] brief lead parity user=— one row per send, containschannels_equal,synthesis_level,exec_len,brief_lead_len[digest] PARITY REGRESSION— fires whenchannels_equal=false. Should NEVER fire post-deploy.[digest] synthesis level=2_degraded— L1 fallback to L2 (LLM transient failure). Sparse acceptable.[digest] synthesis level=3_stub— L2 also failed, stub shipped. Should be rare; investigate if sustained.Expected healthy signals (within 1 cron tick after deploy):
channels_equal=truefor ~all sendssynthesis_leveldistribution mostly1public_lead_len > 0for ~all sends (the publicLead generation rarely fails since it shares cache with personalised when pool is identical)Failure signals & rollback:
channels_equal=false→ revert this PR; the canonical-synthesis contract has regressed.synthesis_level=3_stub→ LLM provider outage (not specific to this PR). Check OpenRouter status./api/brieflogs → renderer rejects the new shape;SUPPORTED_ENVELOPE_VERSIONSshould keep v2 readable so failures are limited to fresh envelopes.Validation window: First cron tick after deploy. Manual verification:
Confirm:
digest.lead(open both, compare)synthesis_levelmostly1Rollback: revert this PR. Branch is 6 atomic commits with clean message + per-commit test pass; revert is mechanical.
Companion
Plan file:
docs/plans/2026-04-25-002-fix-brief-email-two-brain-divergence-plan.md(local;docs/plans/is gitignored per repo convention).Independently of this PR:
2026-04-24-004-fix-brief-topic-adjacency-defects-plan.md, Solutions 2-3 still pending) is orthogonal to this work — this PR ensures all surfaces show the same fragmented output, not N different fragmented outputs.