fix(brief): exclude feel-good/lifestyle content from the digest pool#3748
Merged
Conversation
Sibling to classifyOpinion (PR #3690). Stamps/filters editorially-unfit feel-good and lifestyle content out of the brief pool. Design (round-2 ce-doc-review): - HARD-NEWS VETO (R3a) runs FIRST; overrides every classification path (STRONG URL, STRONG headline prefix, CORROBORATING). Expanded with morphology variants so natural news prose vetoes correctly ("strike kills six" — not requiring exact "airstrike" / "killed"). - CORROBORATING threshold raised from 2 to 3 distinct group names (adv-R2-003 — prevents reachable hard-news FPs on 2-token stacks). - Distinct-token identity is the alternation-group LABEL, not raw match text (adv-R2-002 — collapses reunite/reunited/reuniting into reunite_group so RSS-typical inflection echoes count once). - /local/, /photos/, /travel/, /style/ are CORROBORATING only — major outlets file legitimate hard news under those segments (adv-R2-001). - URL match uses new URL().pathname (try/catch), not raw .includes() — closes the tracking-param injection vector (?utm=/local/promo). - "restored" and "meet the" excluded from token list (FPs on art-restitution news and diplomacy headlines). See docs/plans/2026-05-17-001-fix-feelgood-lifestyle-filter-plan.md for full design rationale + Veterans-warplanes anchor case (May 17 0802 brief, card #4). 36 tests; biome clean.
Sibling to the isOpinion stamp PR #3690 added. parseRssXml computes classifyFeelGood({title, link, description}) and tags every ParsedItem; buildStoryTrackHsetFields persists 'isFeelGood' as '1'|'0' on the Redis HSET row so buildDigest's read-path filter can exclude feel-good content without re-classifying. Mirrors the four isOpinion sites exactly: - Import at :17 (sibling to opinion-classifier import) - ParsedItem field at :163 (sibling to isOpinion) - parseRssXml stamp at :478 (sibling to classifyOpinion call) - buildStoryTrackHsetFields persist at :911 (sibling to isOpinion field) Tests: 2 new (parseRssXml carries the flag + Veterans anchor; HSET persistence + legacy-residue → '0'). Existing baseItem fixture gains isFeelGood: false default (matches PR #3690's isOpinion pattern).
Sibling to the buildDigest opinion filter PR #3690 added. Trusts the ingest stamp (isFeelGood === "1") AND re-classifies stamp-missing residue rows from persisted title/link/description so the filter is effective immediately on rollout, not only after the 48h TTL window. Mirrors the four opinion filter sites exactly: - Import classifyFeelGood at :34 (sibling to opinion-classifier import) - droppedFeelGood counter at :490 (sibling to droppedOpinion) - Filter block immediately after opinion block (:522-549), BEFORE derivePhase / matchesSensitivity — earlier filtering keeps downstream telemetry clean - Conditional log at :567-573 mirrors the opinion log byte-for-byte: "[digest] buildDigest feel-good filter dropped N feel-good/lifestyle item(s) from the pool (variant=… lang=… sensitivity=…)". Per FEAS-001, no invented stamped/residue parenthetical (opinion mirror has none). M6 / adv-005 — counter asymmetry documented in inline comment: a row matched by BOTH classifiers (columnist-nostalgia essay; op-ed with tribute framing) increments only droppedOpinion (opinion `continue`s first). droppedFeelGood is therefore "rows the feel-good filter dropped *after* opinion passed on them this run," not "all feel-good content seen." Applies stamped/residue/mixed paths equally. See plan Operational Notes for the operator-runbook version. Per-attempt [digest] brief filter drops log is intentionally unchanged — it does not carry dropped_opinion= today and must not gain dropped_feelgood= either (C1). 14 source-textual tests in greenfield tests/digest-buildDigest-feelgood-filter.test.mjs + no regressions across 218 tests in brief-from-digest-stories / brief-llm / seed-envelope-parity sweep.
…ge (U4) Without this COPY the Railway cron crashes at startup with ERR_MODULE_NOT_FOUND on the new '../server/_shared/feelgood-classifier.js' import that U3 added to scripts/seed-digest-notifications.mjs. Same failure mode the transitive-import-closure test caught for the opinion-classifier in PR #3690 — that test still passes here by construction.
…good filter) PNG cold-render (font + resvg-wasm init) measures ~10s on a warm machine and longer under concurrent test load. The node:test default per-test timeout (5s in tsx --test) false-fails these three tests in the pre-push full-suite sweep while they pass in isolation. Bumping to 30s removes the false failure without masking real regressions. Pre-existing flake, unrelated to this PR's feel-good filter — surfaced because adding new tests/*.test.mjs files flipped the pre-push hook into RUN_ALL mode.
Contributor
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Greptile SummaryAdds a
Confidence Score: 4/5Safe to merge. The feel-good filter is conservative by design and the HARD_NEWS_VETO runs first to protect hard-news stories from false-positive exclusion. The new classifier, stamp, and filter wiring are all well-implemented with no logic bugs found. The one open item is opinion-classifier.js line 93, which still uses raw string matching on the full link URL — a known gap this PR explicitly documents but does not close. Until that backport lands, a tracking parameter such as ?utm=/opinion/promo can cause false opinion-stamps at ingest. The feel-good classifier itself does not share this flaw. server/_shared/opinion-classifier.js line 93 — the URL matching logic should be updated to use parsed pathname, as done in the new feel-good classifier. Important Files Changed
Reviews (1): Last reviewed commit: "test(carousel): bump PNG-render per-test..." | Re-trigger Greptile |
…ult leakage PR-review finding: pre-PR cached ParseResults in rss:feed:v3 contain ParsedItems without isFeelGood. If a cache hit returned one during the 1h healthy-cache rollout window, buildStoryTrackHsetFields would write `'isFeelGood', undefined ? '1' : '0'` → '0' onto the fresh story:track:v1 row. buildDigest's stampMissing check (`typeof !== 'string' || length === 0`) treats '0' as a genuine "not feel-good" verdict and skips the residue catch — feel-good content could silently slip through for up to one hour post-deploy. Fix: bump the cache prefix v3→v4 so every pre-PR ParseResult is invalidated on deploy. The first post-deploy cron tick is a cold read for every feed; fresh parseRssXml runs stamp isFeelGood correctly before buildStoryTrackHsetFields ever sees the item. Mirrors the v2→v3 precedent on this same cache for the same class of parsed-cache-shape change. Same latent bug exists in PR #3690's isOpinion path (1h leakage window after that PR shipped, masked by the TTL). A separate backport can address it; out of scope for this PR. New test: tests/news-story-track-description-persistence.test.mts asserts the v4 cache prefix is present in fetchAndParseRss source — locks the cutover so a refactor cannot silently revert to v3. Updated comment on the existing missing-field test to clarify that the '0' fallback is buildStoryTrackHsetFields' own behavior; the cache-leakage failure mode is closed by the v4 prefix bump above.
Owner
Author
|
Finding confirmed and fixed in Verified the mechanism: pre-PR Chose Option A (cache prefix bump v3→v4). Reasoning:
Option B (preserve missing as empty in Test: new lock-in at Updated the existing 86 tests green across the touched surfaces (classifier + ingest + persistence + buildDigest filter + neighbours). |
4 tasks
koala73
added a commit
that referenced
this pull request
May 17, 2026
…#3748) (#3750) PR #3748 added the same fix to feelgood-classifier.js and explicitly documented the latent gap in opinion-classifier.js:93 as a follow-up backport. Greptile's review of #3748 also flagged this. Closing the gap now. Mechanism (same as adv-002 in #3748): - Pre-fix: `lowerLink.includes('/opinion/')` on the raw link string returns true for any URL whose query string OR fragment contains "/opinion/" — including legitimate aggregator tracking params like `?utm_campaign=/opinion/promo` or news.google.com style redirects whose original `?url=...` carries an /opinion/ section. Hard news with that tracking shape would silently classify as opinion and drop from the brief. - Fix: `safePathname(link)` runs `new URL(link).pathname.toLowerCase()` inside try/catch and returns '' on malformed URLs. Matching is then done against the parsed pathname only — query strings and fragments cannot spoof a section match. Mirrors the helper in feelgood-classifier.js byte-for-byte. Two sites in classifyOpinion shared the bug: - STRONG #1 URL match (line 93) - CORROBORATING URL match (line 103) for /analysis/ + /analyses/ Both now run against the parsed pathname. 6 new regression tests prove the injection vector is closed (tracking param, URL fragment, malformed URL defensive) AND that the fix does not over-correct (genuine /opinion/ + genuine /analysis/ with tracking params still classify correctly). All 12 original tests still pass.
This was referenced May 17, 2026
Merged
koala73
added a commit
that referenced
this pull request
May 19, 2026
…9 regression) The 2026-05-19 Pro brief shipped "How nuclear war would impact the global food system" from Bulletin of Atomic Scientists as CRITICAL story #6, sitting alongside breaking news about the Iran-Israel war and the WHO Ebola declaration. The brief promises event-driven intelligence — a Bulletin analysis essay is not an event. The existing classifier missed it on all three current signals: - STRONG #1 (URL section /opinion/): Bulletin URLs have no opinion- style path because the WHOLE SITE is commentary, no hard-news section to distinguish from. - STRONG #2 (headline prefix "Opinion:"): hard-news-shaped title. - CORROBORATING (quote-wrap + columnist framing): description reads like a news lede. The signal IS the publisher. Add STRONG #3: a hand-curated allowlist of commentary-only publishers, matched by hostname (suffix-anchored to permit `newsletter.<host>` / `m.<host>` while rejecting typo-domains like `evilthebulletin.org`). Initial list (per docs/plans/2026-05-19-001 U1): - thebulletin.org — Bulletin of Atomic Scientists - project-syndicate.org — op-eds from world leaders / academics - foreignaffairs.com — CFR's analysis quarterly - foreignpolicy.com — Foreign Policy magazine - warontherocks.com — defense analysis blog Maintenance commitment: quarterly review against `droppedOpinion` telemetry to catch (a) new commentary publishers to add, (b) listed publishers that launched a hard-news section. Rollback path is remove-from-Set, never per-URL exceptions (cruft). Pattern mirrors the existing safePathname/STRONG_URL_SEGMENTS shape; new safeHostname helper parses URL().hostname so the match is on the parsed host (not raw .includes() on the link string — closes the tracking-param injection vector documented in PR #3748). 7 new test cases cover the May 19 regression, the 4 other publishers, subdomain matching, typo-domain rejection, tracking-param / fragment spoofing, malformed URLs, and the "allowlisted host PLUS hard-news content → still opinion" rule. This is PR-1 of the 4-PR Phase 1 wave from docs/plans/2026-05-19-001-fix-brief-content-quality-regressions-plan.md.
koala73
added a commit
that referenced
this pull request
May 19, 2026
…9 regression) (#3835) The 2026-05-19 Pro brief shipped "How nuclear war would impact the global food system" from Bulletin of Atomic Scientists as CRITICAL story #6, sitting alongside breaking news about the Iran-Israel war and the WHO Ebola declaration. The brief promises event-driven intelligence — a Bulletin analysis essay is not an event. The existing classifier missed it on all three current signals: - STRONG #1 (URL section /opinion/): Bulletin URLs have no opinion- style path because the WHOLE SITE is commentary, no hard-news section to distinguish from. - STRONG #2 (headline prefix "Opinion:"): hard-news-shaped title. - CORROBORATING (quote-wrap + columnist framing): description reads like a news lede. The signal IS the publisher. Add STRONG #3: a hand-curated allowlist of commentary-only publishers, matched by hostname (suffix-anchored to permit `newsletter.<host>` / `m.<host>` while rejecting typo-domains like `evilthebulletin.org`). Initial list (per docs/plans/2026-05-19-001 U1): - thebulletin.org — Bulletin of Atomic Scientists - project-syndicate.org — op-eds from world leaders / academics - foreignaffairs.com — CFR's analysis quarterly - foreignpolicy.com — Foreign Policy magazine - warontherocks.com — defense analysis blog Maintenance commitment: quarterly review against `droppedOpinion` telemetry to catch (a) new commentary publishers to add, (b) listed publishers that launched a hard-news section. Rollback path is remove-from-Set, never per-URL exceptions (cruft). Pattern mirrors the existing safePathname/STRONG_URL_SEGMENTS shape; new safeHostname helper parses URL().hostname so the match is on the parsed host (not raw .includes() on the link string — closes the tracking-param injection vector documented in PR #3748). 7 new test cases cover the May 19 regression, the 4 other publishers, subdomain matching, typo-domain rejection, tracking-param / fragment spoofing, malformed URLs, and the "allowlisted host PLUS hard-news content → still opinion" rule. This is PR-1 of the 4-PR Phase 1 wave from docs/plans/2026-05-19-001-fix-brief-content-quality-regressions-plan.md.
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
Sibling fix to PR #3690 (opinion-classifier). Excludes editorially-unfit feel-good / lifestyle content from the brief pool via the same architectural pattern: shared classifier + ingest stamp +
buildDigestread-time filter + Dockerfile COPY.Triggering incident: the May 17 0802 brief shipped "Veterans reunite with their vintage war planes" — a feel-good local human-interest piece from Peru, Illinois (population 9,800) — at card #4 (HIGH severity), sitting in a brief otherwise covering WHO's Ebola declaration, a Rwandan genocide-suspect death, Israeli airstrikes in Lebanon, US-Cuba escalation, and Iran's war-readiness warnings. Editorially out of place in a "WorldMonitor Intelligence Brief."
The upstream importance/severity classifier is the correct long-term fix surface, but rebuilding that model is out of scope. This PR is the pragmatic structural backstop — same posture as PR #3690.
Design (post round-2 multi-persona document review)
Five reviewers (coherence, feasibility, product-lens, scope-guardian, adversarial) ran across two rounds. Round 2 surfaced four P1 design issues that materially changed the implementation from the round-1 plan:
kill/kills/killed/killing,strike/strikes/struck,attack/attacks/attacked,bomb/bombed/bombing,massacre,casualties,militants,dead/died,wounded,evacuat) so natural news prose vetoes correctly ("strike kills six"rather than requiring exact\bairstrike\b+\bkilled\b).reunite/reunited/reunitingintoreunite_groupso RSS-typical inflection echoes count once. Without this, the hostages-ceasefire defense would have leaked./local/,/photos/,/photo/,/travel/,/style/are CORROBORATING only (M5 + adv-R2-001) — major outlets file legitimate hard news under all five (BBC travel advisories, FT/Bloomberg business-of-style, regional papers' breaking local, Reuters/AP wire-photo desks). As STRONG, they would silently drop those hard-news classes.new URL().pathname(try/catch), not raw.includes()(adv-R2-002 / C3) — closes the tracking-param injection vector (?utm=/local/promo). The same gap exists inserver/_shared/opinion-classifier.js:93today; recommended as a sibling follow-up backport.restoredandmeet theexcluded from CORROBORATING token list — FPs on art-restitution news (Klimt) and diplomacy headlines (US officials meet the Russian delegation).See
docs/plans/2026-05-17-001-fix-feelgood-lifestyle-filter-plan.mdfor the full design rationale.Implementation Units
server/_shared/feelgood-classifier.{js,d.ts}tests/feelgood-classifier.test.mjsserver/worldmonitor/news/v1/list-feed-digest.ts(4 sites)scripts/seed-digest-notifications.mjs(4 sites)tests/digest-buildDigest-feelgood-filter.test.mjsDockerfile.digest-notificationstests/dockerfile-digest-notifications-imports.test.mjspasses by constructionTelemetry shape (FEAS-001): the conditional
buildDigestlog mirrors opinion byte-for-byte ([digest] buildDigest feel-good filter dropped N feel-good/lifestyle item(s) from the pool (variant=… lang=… sensitivity=…)). The per-attempt[digest] brief filter dropslog line at:1654-1668does NOT carrydropped_feelgood=— same as it does NOT carrydropped_opinion=today.Counter asymmetry (M6 / adv-005): a row matched by BOTH classifiers (columnist-nostalgia essay; op-ed with tribute framing) increments only
droppedOpinion— opinioncontinues first.droppedFeelGood = Nshould be read as "rows the feel-good filter dropped after opinion passed on them this run," not "all feel-good content seen." Documented in an inline comment AND in the Operational Notes for the on-call.Test plan
tests/feelgood-classifier.test.mjscovering threshold-3, alternation-group dedup, morphology-expanded veto,/local///photos/CORROBORATING demotion, URL pathname injection vector, defensive inputs.tests/news-rss-description-extract.test.mts+tests/news-story-track-description-persistence.test.mtscovering ParsedItem shape + HSET persistence + residue →'0'.tests/digest-buildDigest-feelgood-filter.test.mjscovering filter wiring, ordering (after opinion, before phase/sensitivity), residue catch, log shape (FEAS-001), per-attempt-log negative-space (T21), M6 asymmetry.tests/dockerfile-digest-notifications-imports.test.mjstransitive-import-closure test green.brief-from-digest-stories/brief-llm/seed-envelope-parity— no regressions.main()inseed-digest-notifications.mjs— unchanged.)npx tsc --noEmiton modified TypeScript file: clean.[digest] buildDigest feel-good filter dropped Nlog line appearing on most cron ticks.Companion fix
d727ebe0e test(carousel): bump PNG-render per-test timeout— orthogonal to this PR's logic. PNG cold-render takes ~10s but the default per-test timeout (5s intsx --test) false-fails in the pre-push full-suite sweep while it passes in isolation. Surfaced because adding newtests/*.test.mjsfiles flips the pre-push hook intoRUN_ALLmode.🤖 Generated with Claude Code