fix(brief): post-LLM hallucination validator with shadow-mode rollout#3836
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryAdds a heuristic post-LLM hallucination validator (
Confidence Score: 4/5Safe to merge as-is; shadow mode is the default so no user-visible output changes can result from the heuristic edge cases identified. The validator logic is sound for the target regression and all 41 tests pass. Three heuristic issues affect only monitoring noise or future false-positive rates once enforce mode is enabled: specifically, 'Defense' and 'Foreign' in TITLE_PREFIX_STOP can silently drop the first word of institution names like 'Defense Department', producing a sequence that won't normalize to the expected acronym. In shadow mode this causes only spurious console.warn lines, but it would cause unnecessary headline-fallbacks once enforce is flipped on. shared/brief-llm-core.js — the TITLE_PREFIX_STOP list and the dead multi-word DEMONYM_NORMALIZE entries are worth revisiting before flipping to enforce mode. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[callLLM returns llmResult] --> B[validateNoHallucinatedProperNouns\nsummary, headline]
B --> C{malformed input?}
C -->|yes| D[return ok:true\nship LLM output]
C -->|no| E[extractProperNounSequences\nfor summary + headline]
E --> F[normalizeSequence\nacronym + demonym tables]
F --> G{all summarySeqs\nfound in headline?}
G -->|yes| H[return ok:true]
G -->|no| I[return ok:false\nhallucinated: seq]
H --> J[worldBrief = llmResult.text\nnormal path]
I --> K{BRIEF_VALIDATOR_MODE}
K -->|enforce| L[worldBrief = topHeadline\nbrief_hallucination ENFORCE]
K -->|shadow default| M[worldBrief = llmResult.text\nbriefhallucination SHADOW log]
Reviews (1): Last reviewed commit: "fix(brief): post-LLM hallucination valid..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Hallucination validator rollout mode (PR-2 of brief-content-quality | ||
| // regressions). `shadow` = log violations to Sentry but ship the LLM | ||
| // output unchanged (default, safe). `enforce` = on violation, replace | ||
| // the LLM summary with the source headline. Flip via Railway env after | ||
| // the 7-day shadow window confirms <5% violation rate. |
There was a problem hiding this comment.
The inline comment says shadow = log violations to Sentry, but the implementation uses console.warn and the PR description explicitly states violations are "logged to console as [brief_hallucination SHADOW]." The discrepancy could mislead a future engineer who reads this comment and assumes Sentry events exist for querying during the 7-day monitoring window — only to find nothing in the Sentry dashboard.
| 'Dr', 'Dr.', 'Mr', 'Mr.', 'Ms', 'Ms.', 'Mrs', 'Mrs.', | ||
| 'Acting', 'Interim', 'Former', 'Ex', | ||
| 'Chairman', 'Chairwoman', 'Chair', 'Speaker', | ||
| 'CEO', 'Secretary', 'Defense', 'Foreign', |
There was a problem hiding this comment.
'Defense' / 'Justice' / 'Foreign' in TITLE_PREFIX_STOP produce false positives for government-department names
When "Defense Department", "Justice Department", or "Foreign Ministry" appears at the start of a proper-noun sequence (e.g. after a sentence-start ambiguous word is filtered), the first word is consumed by the title-prefix rule (current.length === 0), leaving only ['department'] / ['ministry'] as the extracted sequence. Those tokens don't normalize to 'dod', 'doj', etc., so if the headline uses the acronym form the validator flags the summary as hallucinated. Removing them from the stop list keeps person-title suppression working (since 'Secretary' is also in the list and handles "Defense/Foreign/Justice Secretary") while letting the institution-name path stay intact.
| 'CEO', 'Secretary', 'Defense', 'Foreign', | |
| 'CEO', 'Secretary', |
| // every output. This validator catches inventions mechanically: it | ||
| // extracts proper-noun sequences from the LLM summary and verifies | ||
| // each appears in the source headline (after acronym + demonym | ||
| // normalization). When a sequence in the summary has no matching | ||
| // contiguous subsequence in the headline, the summary is rejected. | ||
| // | ||
| // The caller decides what to do with a rejection. In `seed-insights.mjs` |
There was a problem hiding this comment.
DEMONYM_NORMALIZE are unreachable dead code
DEMONYM_TO_NATION contains multi-word keys 'South Korean', 'South Koreans', 'North Korean', 'North Koreans'. These are stored in DEMONYM_NORMALIZE as 'south korean' → 'south korea' etc. However, normalizeToken performs only single-token Map lookups, and normalizeSequence's multi-word greedy loop queries only ACRONYM_NORMALIZE, never DEMONYM_NORMALIZE. The multi-word entries are therefore never retrieved. The single-word entries 'korean' → 'korea' handle the intent correctly, so there is no functional regression — but the dead entries add noise and could confuse a future contributor.
| for (const token of tokens) { | ||
| // Strip trailing punctuation and possessive 's / ’s so | ||
| // "Beirut's" → "beirut" and "U.S." → "U.S" (handled below). | ||
| let stripped = token.replace(/[.,;:'’]+$/g, ''); | ||
| stripped = stripped.replace(/['’]s$/i, ''); | ||
| const tokenForLookup = stripped || token; | ||
| const isTitlePrefix = TITLE_PREFIX_STOP.has(stripped); |
There was a problem hiding this comment.
includes check
containsSubsequence(headlineSeq, summarySeq) already handles the single-element-needle case: it iterates headlineSeq checking headlineSeq[i] === summarySeq[0], which is logically identical to headlineSeq.includes(summarySeq[0]). The secondary branch if (summarySeq.length === 1 && headlineSeq.includes(...)) can therefore never produce a different result from the containsSubsequence call above. The comment describes it as "the reverse" check but it is not — it is the same check.
d845f15 to
4e0f063
Compare
|
P1 confirmed and fixed in You're right — Fix: added a
The "requires ≥2 letter-dot pairs to commit" prevents over-collapsing — single sentence-final initials don't match. Adjacent fix surfaced during testing: single-char capitalized tokens ( New tests (5):
47/47 tests pass (was 41; +6). Mirror parity test ( |
The 2026-05-19 Pro brief shipped "Lebanese President Michel Aoun
pledged..." against a source headline that contained NO name — just
"Lebanese president vows to 'do the impossible'..." Current Lebanese
president is Joseph Aoun; Michel Aoun left office in 2022. The LLM
invented "Michel Aoun" despite the prompt explicitly forbidding
invention.
Prompts that say "do not invent proper nouns" are not enforceable on
every output. This validator catches inventions mechanically: extracts
proper-noun sequences from the LLM summary, normalizes via acronym
and demonym tables (WHO ↔ World Health Organization, Israeli ↔ Israel,
US ↔ United States, etc.), and verifies each summary sequence has a
matching contiguous subsequence in the source headline. When a summary
sequence has no headline match, the summary is rejected.
Extractor rules pinned per the plan (docs/plans/2026-05-19-001 U2):
- TITLE_PREFIX_STOP — "former President Trump" extracts as ['trump'],
consuming the title-prefix without registering it.
- PROPER_NOUN_JOINER — "Democratic Republic of Congo" stays one
sequence, bridged by lowercase joiners ('of', 'the', 'and', 'for', …).
- All-caps acronym rule — 2–6-char ALL CAPS tokens (WHO, UN, NATO,
IDF, …) register as proper nouns even at sentence start.
- SENTENCE_START_AMBIGUOUS — explicit stop list of common
sentence-starters ('The', 'It', 'Breaking', 'Meanwhile', …) that
LOOK proper-noun-like but aren't. Real proper nouns at sentence
start ('Trump said...', 'Israel announced...') pass through.
- Possessive 's stripped ("Beirut's" → "beirut").
- ACRONYM_NORMALIZE — 24 acronym↔expansion groups, bidirectional.
- DEMONYM_NORMALIZE — 50+ demonym↔nation mappings.
Shadow-mode rollout: BRIEF_VALIDATOR_MODE=shadow (default) logs
violations to console but ships the LLM output unchanged. After 7 days
of shadow data, the false-positive rate is reviewed; if <5% the env
flips to BRIEF_VALIDATOR_MODE=enforce and violations replace the LLM
summary with the source headline (R1 of the plan: "falls back to a
safe summary").
Defensive against malformed inputs (undefined, null, empty string,
non-string, HTML-encoded, 10KB long) — all return { ok: true } without
throwing, so the validator can never crash brief generation.
41 unit tests cover: the captured May 19 hallucination + 3 synthesized
variants (LLM non-determinism guard), title-prefix consume,
acronym↔expansion both directions, demonym normalization, multi-word
joiners, sentence-start handling, possessive-s, malformed inputs, and
the explicit out-of-scope case (headline-level errors not fact-checked).
This is PR-2 of the 4-PR Phase 1 wave from
docs/plans/2026-05-19-001-fix-brief-content-quality-regressions-plan.md.
4e0f063 to
cb066b7
Compare
|
P1 confirmed and fixed in You're right — this is the same root-dir escape class as PR #3818's hotfix to the simulation/forecast workers. Even though Fixes shipped:
Verified:
Recipe match: skill |
…identity (#3872) * ci(digest): add digest-image build job + scheduled feed-validation workflow PR CI gains a docker-build smoke for Dockerfile.digest-notifications that catches cross-directory-import + native-dep breakage the static BFS test in tests/dockerfile-digest-notifications-imports.test.mjs cannot see. Gated on path changes under scripts/, shared/, server/_shared/, api/, the Dockerfile itself, or the root package manifest. Feed-registry validation moves OFF pull_request CI (SSRF surface — a hostile PR could rewrite src/config/feeds.ts to make runners hit arbitrary URLs) and INTO a new feed-validation.yml that runs on push-to-main, every-6h schedule, and workflow_dispatch. The new test:feeds:ci script passes --ci to scripts/validate-rss-feeds.mjs which enforces https-only, allowlist-membership (via api/_rss-allowed-domain-match.js, the new extracted predicate the Edge rss-proxy.js also consumes), and per-hop redirect re-checks. Plan: docs/plans/2026-05-23-001-fix-digest-pipeline-review-actions-plan.md * test(railway): registry as single source of truth for service entrypoints Replaces the hand-maintained ENTRY_POINTS array in tests/scripts-railway-nixpacks-no-escape-import.test.mts (4 entries, already known to drift per PR #3836 retrospective) with a derived list from scripts/railway-services.json. The Dockerfile.digest-notifications BFS test reads the same registry filtered to deployMode === "dockerfile". A new coverage test (tests/railway-services-registry-coverage.test.mts) audits every Dockerfile.* CMD line and every runbook "Start command:" entry against the registry — if a deployment artifact references a script that isn't registered, the test fails naming the file. Self- fixtures pin both regex shapes so a future simplification can't silently stop matching. The registry surfaces 17 nixpacks services (up from 4) and 4 Dockerfile- deployed services. The runbook header now points new-service authors at the registry as the first stop. Plan: docs/plans/2026-05-23-001-fix-digest-pipeline-review-actions-plan.md * fix(rss): exclude missing-date items from freshness via effectivePubDateMs The legacy parseFeedDateOrNow silently substituted Date.now() for any missing/invalid feed pubDate, which produced false freshness across every ranking and recency gate. The fix replaces it with parseFeedDate (returns { date, missing }) and a new effectivePubDateMs helper that returns 0 for items flagged pubDateMissing — so they sort last in newest-first comparators and fail any positive-duration recency gate (Date.now() - 0 = huge), while keeping the synthesized pubDate in place for display consumers. Routed through the helper: - rss.ts AI-candidate top-N selection + min-heap insert (lines 296, 352-369) - daily-market-brief.ts headline-pool sort (line 217) - breaking-news-alerts.ts isRecent recency gate + best-alert ranking (lines 145, 260) - data-loader.ts staleItems sort, hero/digest sorts, time-range filter (6 sites) - panel-layout.ts time-range filter (caught by the new guardrail audit) - velocity.ts trend-midpoint partition (lines 58-59) - analysis-core.ts cluster sort + WINDOW_MS recency gate (lines 292, 440) Cache prefix bumped feed: → feed:v2: at rss.ts:23, 44-65 because pre-v2 serialized items lack pubDateMissing; old entries are left to TTL out. Two new tests: - tests/feed-date.test.mts — parser shapes + helper ranking behavior - tests/feed-date-ranking-uses-effective.test.mts — static audit that walks src/services/ + src/app/ and fails if any .pubDate.getTime() inside a ranking/recency call site bypasses the helper, with a documented allow-list for metadata/identity uses and self-fixtures pinning both sort-comparator and recency-gate regex shapes Plan: docs/plans/2026-05-23-001-fix-digest-pipeline-review-actions-plan.md * fix(rss): use link-keyed identity in AI classify queue dedupe Previously canQueueAiClassification(title) collapsed every article sharing a wire headline into one dedupe slot, so only the first got an AI threat upgrade and the rest stayed at keyword confidence for the 30-minute window. The new signature canQueueAiClassification({ link, title }) keys on the normalized link when present (lowercased host, fragments dropped, utm_*/fbclid/gclid stripped) and falls back to title when link is empty or malformed. Behavioral surface is narrower than it might look: the caller in rss.ts only invokes the queue for items where threat.source === 'keyword' — items already keyword-classified that are eligible for an AI confidence upgrade. A dropped enqueue does NOT drop the article from the feed; it just leaves its threat label at keyword confidence. Also adds __resetAiClassifyQueueForTests() following the project's __…ForTests convention (insights-loader.ts:104) so the new test file can drive the module-scope dedupe map across scenarios deterministically. Import switched from '@/config' barrel to '@/config/variant' leaf to avoid pulling @/utils/proxy.ts (and its import.meta.env.DEV usage) into the node:test runner. Plan: docs/plans/2026-05-23-001-fix-digest-pipeline-review-actions-plan.md * fix(rss): close pubDate-getTime guardrail bypass + harden effectivePubDateMs Code-review caught a real regex bypass: src/app/country-intel.ts:297 ran `new Date(b.pubDate).getTime() - new Date(a.pubDate).getTime()` over NewsItem[] from ctx.allNews — a freshness-ranking comparator that the U3 guardrail regex /\.pubDate\s*\.getTime\(\)/ did not match. The country-brief panel's news ordering would have surfaced missing-date items at the top despite the U3 fix. Three changes: 1. src/app/country-intel.ts:297 now routes through effectivePubDateMs. 2. PUBDATE_GETTIME_RE broadened to also match `item.pubDate?.getTime()` (optional chain, idiomatic in TS 4+) and `new Date(item.pubDate). getTime()` (the bypass the country-intel violation used). Self-fixtures pin all three shapes plus a negative-control fixture confirming the regex does NOT catch `new Date(item.endDate).getTime()`. Paren-wrapped, aliased, and destructured reads remain residual risk — documented in the regex docblock. 3. effectivePubDateMs now guards NaN/Infinity on every input branch (Date with Invalid value, NaN/Infinity number, unparseable string). Sort comparators on NaN have unspecified behavior per the JS spec; the previous numeric branch returned the value verbatim. Tests cover NaN, ±Infinity, and Invalid Date. Also minor cleanups from review: - Inverted JSDoc on isRecent clarified (`0 >= (large positive) = false`). - Misleading migration-fallback comment in rss.ts (claimed "older builds stored feeds as feed:v2:") corrected to describe the actual scope: language-scope migration carried forward from the pre-v2 era. Plan: docs/plans/2026-05-23-001-fix-digest-pipeline-review-actions-plan.md * fix(rss): apply 9 P2/observation findings from code review (LFG batch) F2 — Cache prefix bump for happy-all-items: data-loader.ts:3200 HAPPY_ITEMS_CACHE_KEY bumped 'happy-all-items' → 'happy-all-items:v2' to match the feed: → feed:v2: invalidation. Pre-v2 entries serialize NewsItem without pubDateMissing; under the new helper they would fraudulently claim freshness inside the 24h gate window. Bumping forces refetch; pre-v2 entries are left to TTL out naturally. F3 — Velocity excludes missing-date items from trend math: velocity.ts pre-filters items.filter(i => !i.pubDateMissing) before the midpoint partition. Without this, clustering.ts:116 (allow-listed raw aggregation) would compute firstSeen/lastUpdated from synthesized stamps, then velocity would treat every missing-date item as 'older' via effectivePubDateMs=0, flipping cluster trend to 'falling' for any cluster with several missing-date items. F4/F5 — CI workflow timeouts: feed-validation.yml job gains timeout-minutes: 15 (cap on hung npm ci or stalled feed registry). digest-image job in test.yml gains timeout-minutes: 20 (cap on docker build infra flakes). F6 — Per-hop fetch timeout in --ci validator: validate-rss-feeds.mjs --ci redirect handler now creates a fresh AbortController + setTimeout per hop instead of one budget across all hops. "Timeout (15s)" in the report now means a single 15s network call, not 17s+ chained. F7 — MAX_HOPS off-by-one in --ci validator: Loop bound changed from `hop <= MAX_HOPS` to `hop < MAX_HOPS` so the documented "max 3 hops" intent matches the actual iteration count. Final-iteration redirect now throws 'Too many redirects' consistently. F8 — Dublin Core <dc:date> fallback in RSS parser: rss.ts:267-274 adds dc:date as a final fallback for the RSS branch (after pubDate). ArXiv RSS feeds — "ArXiv AI" and "ArXiv ML" in the feed registry — ship dc:date with no <pubDate>; without this fallback every ArXiv item would be marked pubDateMissing=true and demoted in every freshness ranking. Prior precedent: PR #3417 dc:date strict-drop incident. Observation 1 — Asymmetric tie-break in breaking-news-alerts.ts: Captures a bestEffectiveMs local so the tie-break comparison is symmetric (helper on both sides). Today isRecent excludes missing- date items upstream so the comparison is safe; the local makes the guarantee defense-in-depth against a future refactor that removes the upstream gate. Observation 2 — Document NaN/Infinity exclusion in time-range filters: data-loader.ts filterItemsByTimeRange and panel-layout.ts time-range filter previously wrapped raw pubDate.getTime() with Number.isFinite and fell through to `true` for non-finite, INCLUDING corrupt-stamp items in narrow windows. The helper now sanitizes NaN/Infinity to 0 uniformly with pubDateMissing — corrupt-stamp items are excluded. This is intentional (untrustworthy timestamps shouldn't claim freshness) but undocumented in U3; comments at both sites now call out the behavior shift. Plan: docs/plans/2026-05-23-001-fix-digest-pipeline-review-actions-plan.md
1 participant
Symptom
The 2026-05-19 0801 Pro brief shipped "Lebanese President Michel Aoun pledged..." against a source headline that contained no name ("Lebanese president vows to 'do the impossible' to end war with Israel..."). Current Lebanese president is Joseph Aoun; Michel Aoun left office in 2022. The LLM invented "Michel Aoun" despite the prompt explicitly forbidding invention. One of 5 distinct content-quality defects on the same brief; this PR addresses defect #1.
Root cause
scripts/_insights-brief.mjs:30-39briefSystemPromptinstructs the LLM "Do not invent proper nouns (people, organizations, countries) that are not in the headline." The instruction was correct. The LLM violated it anyway. Prompts that say "do not invent" aren't mechanically enforceable on every output.Fix
Add
validateNoHallucinatedProperNouns(summary, headline)toshared/brief-llm-core.js. Extract proper-noun sequences from both texts, normalize via acronym + demonym tables, and verify each summary sequence has a matching contiguous subsequence in the headline. When a summary sequence has no headline match, the summary is rejected.Extractor rules pinned (per the plan, after
/ce-doc-reviewflagged these as load-bearing — see origin plan U2 "Approach (H2 fix)"):TITLE_PREFIX_STOP— "former President Trump" extracts as['trump'](consume the title-prefix, don't register it).PROPER_NOUN_JOINER— "Democratic Republic of Congo" stays one sequence, bridged by lowercase joiners (of,the,and,for, …).SENTENCE_START_AMBIGUOUS— explicit stop list of common sentence-starters ("The", "It", "Breaking", "Meanwhile", …) that LOOK proper-noun-like but aren't. Real proper nouns at sentence start ("Trump said...", "Israel announced...") pass through.'sstripped ("Beirut's" → "beirut").ACRONYM_NORMALIZE— 24 acronym↔expansion groups, bidirectional (WHO ↔ World Health Organization,US ↔ United States,EU ↔ European Union, …).DEMONYM_NORMALIZE— 50+ demonym↔nation mappings (Israeli ↔ Israel,Iranian ↔ Iran,Cuban ↔ Cuba, …).Shadow-mode rollout (env-flag controlled):
BRIEF_VALIDATOR_MODE=shadow(DEFAULT): violations logged to console as[brief_hallucination SHADOW]but the LLM output ships unchanged. Lets us measure false-positive rate in production without breaking anything.BRIEF_VALIDATOR_MODE=enforce: violations replace the LLM summary with the source headline (R1 of the plan: "falls back to a safe summary (headline-grounded template)"). Flip via Railway env after the 7-day shadow window confirms <5% false-positive rate.Defensive against malformed inputs (undefined, null, empty string, non-string, HTML-encoded, 10KB long): all return
{ ok: true }without throwing. The validator can never crash brief generation.Test plan
WHO ↔ World Health Organization,US ↔ United States✓Thenot registered ✓npx tsx --test tests/brief-llm-core.test.mjs)tests/scripts-shared-mirror.test.mjs) —shared/brief-llm-core.jsandscripts/shared/brief-llm-core.jsare byte-for-byte equalnpm run typecheckcleanbiome lintclean on changed files[brief_hallucination SHADOW]log frequency for 7 days. False-positive rate ≥5% → expand acronym/demonym lists before flipping enforce. ≤5% → flipBRIEF_VALIDATOR_MODE=enforce.Origin
PR-2 of the 4-PR Phase 1 wave from
docs/plans/2026-05-19-001-fix-brief-content-quality-regressions-plan.md.Out of scope (documented in plan Scope Boundaries)