From b80fa25374a223cfbe4582cec21f92d9226c81a9 Mon Sep 17 00:00:00 2001 From: Chris Bongers Date: Tue, 25 Aug 2026 15:54:33 +0200 Subject: [PATCH 1/7] feat(shared): expert-round iteration on the /articles ad template MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Internal ads leave the template: PostEngagements gains hideInternalAd (drops AdAsComment) and PostWidgets gains hideAdWidget (drops PostSidebarAdWidget), both opt-in so every other surface is unchanged. - The article body carries an MPU per ~250 words of visible text via splitContentForAds, which only cuts where a top-level block closes — never inside a paragraph, list or code block — and folds a trailing sliver into the previous chunk so a page cannot end on an ad. - An MPU sits directly above the comment section, and the thread interleave becomes an MPU every 7 comments (was a native every 5 that never served for want of its layoutKey). - The rail's mid-column sticky (which slid over the widgets below it) is in flow again; a fixed 300x600 closes the rail as the page's ONLY sticky unit — compliant as a publisher sticky: 300px wide, desktop only, one per viewport, nothing after it to cover. - Every unit carries the policy-permitted "Advertisements" label, small and gray, top-right, inside the collapsing wrapper. - Units request 50px before entering the viewport and render centered on a lighter background block. - The cover-side MPU (slot 3) retires, with 4, 5 and 6; the map now reuses live Display units for the new placements (first-party events split per slot; dedicated units are a TODO for clean AdSense rows). - PostAnsweredQuestions renders after the body, as on the post page — it self-hides for logged-in users, matching this page's audience. Co-Authored-By: Claude Fable 5 --- .../src/components/post/PostEngagements.tsx | 8 +- .../src/components/post/PostWidgets.tsx | 13 +- .../post/arbitrage/ArbitragePostContent.tsx | 131 ++++++++++-------- .../src/components/post/arbitrage/slots.ts | 89 ++++++------ .../post/arbitrage/splitContentForAds.spec.ts | 70 ++++++++++ .../post/arbitrage/splitContentForAds.ts | 78 +++++++++++ .../features/monetization/ProgrammaticAd.tsx | 20 +-- 7 files changed, 290 insertions(+), 119 deletions(-) create mode 100644 packages/shared/src/components/post/arbitrage/splitContentForAds.spec.ts create mode 100644 packages/shared/src/components/post/arbitrage/splitContentForAds.ts diff --git a/packages/shared/src/components/post/PostEngagements.tsx b/packages/shared/src/components/post/PostEngagements.tsx index d43e49dc37..3aa08de19a 100644 --- a/packages/shared/src/components/post/PostEngagements.tsx +++ b/packages/shared/src/components/post/PostEngagements.tsx @@ -52,6 +52,11 @@ interface PostEngagementsProps { onCopyLinkClick?: (post?: Post) => void; /** Ad templates break a long thread up — see PostComments. */ interleaveEvery?: number; + /** + * Drops the internal AdAsComment. The programmatic template carries its own + * comment-thread units, and two ad systems in one thread double the density. + */ + hideInternalAd?: boolean; renderInterleaved?: (occurrence: number) => ReactNode; } @@ -60,6 +65,7 @@ function PostEngagements({ onCopyLinkClick, logOrigin, shouldOnboardAuthor, + hideInternalAd, interleaveEvery, renderInterleaved, }: PostEngagementsProps): ReactElement { @@ -172,7 +178,7 @@ function PostEngagements({ shouldHandleCommentQuery CommentInputOrModal={CommentInputOrModal} /> - {!isPlus && } + {!isPlus && !hideInternalAd && } & getRailAd?: (position: PostWidgetPosition) => ReactNode; /** Rendered last, below the footer links. */ trailing?: ReactNode; + /** Drops the internal sidebar ad — for templates carrying their own. */ + hideAdWidget?: boolean; }; /** @@ -100,6 +102,7 @@ export function PostWidgets({ hideToc = false, getRailAd, trailing, + hideAdWidget, }: PostWidgetsProps): ReactElement { const { tokenRefreshed } = useContext(AuthContext); const { source } = post; @@ -158,10 +161,12 @@ export function PostWidgets({ /> ), )} - + {!hideAdWidget && ( + + )} {withAd( PostWidgetPosition.Share, diff --git a/packages/shared/src/components/post/arbitrage/ArbitragePostContent.tsx b/packages/shared/src/components/post/arbitrage/ArbitragePostContent.tsx index ce1a201ea2..c36cfc96d7 100644 --- a/packages/shared/src/components/post/arbitrage/ArbitragePostContent.tsx +++ b/packages/shared/src/components/post/arbitrage/ArbitragePostContent.tsx @@ -19,8 +19,11 @@ import { TruncateText } from '../../utilities'; import Markdown from '../../Markdown'; import { ArbitrageAdFormat, ArbitrageAdSlot } from './ArbitrageAdSlot'; import { ArbitrageTopLeaderboard } from './ArbitrageTopLeaderboard'; +import { PostAnsweredQuestions } from '../PostAnsweredQuestions'; +import { splitContentForAds } from './splitContentForAds'; import { ARBITRAGE_SLOT, + BODY_CHARS_PER_AD, COMMENTS_PER_INTERLEAVED_AD, TOP_LEADERBOARD_STICKY_MS, } from './slots'; @@ -42,48 +45,27 @@ import PostEngagements from '../PostEngagements'; * inventory included. Only the first rail unit keeps its phone placement; the * rest are desktop-only, which brings the phone run well under the cap. */ -const RAIL_AD: Record< - PostWidgetPosition, - { - slot: number; - format: ArbitrageAdFormat; - className?: string; - hideOnPhone?: boolean; - } +const RAIL_AD: Partial< + Record< + PostWidgetPosition, + { + slot: number; + format: ArbitrageAdFormat; + className?: string; + hideOnPhone?: boolean; + } + > > = { [PostWidgetPosition.Source]: { slot: ARBITRAGE_SLOT.railAfterSource, format: ArbitrageAdFormat.MediumRectangle, }, - [PostWidgetPosition.Creator]: { - slot: ARBITRAGE_SLOT.railAfterCreator, - format: ArbitrageAdFormat.MediumRectangle, - hideOnPhone: true, - }, - [PostWidgetPosition.Share]: { - slot: ARBITRAGE_SLOT.railAfterShare, - format: ArbitrageAdFormat.MediumRectangle, - hideOnPhone: true, - }, - [PostWidgetPosition.Highlights]: { - slot: ARBITRAGE_SLOT.railAfterHighlights, - format: ArbitrageAdFormat.MediumRectangle, - hideOnPhone: true, - }, - // Between "You might like" and the discussions, and the only unit that - // stays with the visitor: it pins under the fixed chrome and rides the rest - // of the scroll. Sticky is bounded by the containing block, which must be - // the rail itself — stretched to the article column's height — for the unit - // to have the whole page to travel; FurtherReading flattens to `contents` - // around it for exactly that reason, and any wrapper that generates a box - // here would cut the travel to that box. z-1 puts it over the widgets that - // scroll underneath, and the background keeps them from showing through the - // space the creative does not fill. + // In flow, not sticky: a sticky unit mid-rail slides over the widgets + // below it, and the rail's one sticky lives at its very end (slot 19), + // where nothing follows for it to cover. [PostWidgetPosition.SimilarPosts]: { slot: ARBITRAGE_SLOT.railBetweenFurtherReading, format: ArbitrageAdFormat.MediumRectangle, - className: - 'laptop:sticky laptop:top-[calc(var(--sticky-header-offset)+1rem)] laptop:z-1 laptop:bg-background-default', hideOnPhone: true, }, }; @@ -210,24 +192,7 @@ export function ArbitragePostContent({ )} - {/* MPU 1 beside the tags, date and cover rather than above them, so the - first ad shares the fold with real page furniture instead of - standing alone. The slot is first in the DOM because a phone stacks - the column and the brief puts the unit above the article, not below - it; from laptop `order-last` moves it to the right of the group. - - The two halves are deliberately near equal — 336 for the unit - against 385 for the article's, out of the column's 745 — so the ad - reads as the cover's counterpart rather than as a tower beside it. - items-end puts their bottom edges on the same line. */} -
- - +
- {!!post.contentHtml && ( - globalThis?.document?.body} - /> - )} + {/* One MPU per BODY_CHARS_PER_AD of visible text, only ever between + top-level blocks — splitContentForAds cannot cut a paragraph, list + or code block in half. */} + {!!post.contentHtml && + splitContentForAds(post.contentHtml, BODY_CHARS_PER_AD).map( + (chunk, index, chunks) => ( + // eslint-disable-next-line react/no-array-index-key + + globalThis?.document?.body} + /> + {index < chunks.length - 1 && ( + + )} + + ), + )} + + {/* Same block the post page shows: the questions that likely brought + an anonymous visitor here (the component self-hides for logged-in + users and question-less posts). */} + + + {/* The production engagement block verbatim — counts, actions, share, sort control, composer and thread — so everything from here to the @@ -292,11 +283,12 @@ export function ArbitragePostContent({ post={post} onCopyLinkClick={onCopyPostLink} logOrigin={Origin.ArticlePage} + hideInternalAd interleaveEvery={COMMENTS_PER_INTERLEAVED_AD} renderInterleaved={() => ( )} /> @@ -311,9 +303,14 @@ export function ArbitragePostContent({ className="!gap-2 pb-8 pt-4 tablet:border-l tablet:border-border-subtlest-tertiary" hideSignupWidget hideToc + hideAdWidget getRailAd={(position) => { const spec = RAIL_AD[position]; + if (!spec) { + return null; + } + return ( ); }} + // The page's only sticky unit, closing the rail: last in the column, + // so pinning under the fixed chrome can never slide it over content — + // the overlap the mid-rail sticky produced. Compliant as a publisher + // sticky at exactly 300px wide, desktop only, one per viewport. + trailing={ + + } /> ); diff --git a/packages/shared/src/components/post/arbitrage/slots.ts b/packages/shared/src/components/post/arbitrage/slots.ts index 0a80901731..8874755805 100644 --- a/packages/shared/src/components/post/arbitrage/slots.ts +++ b/packages/shared/src/components/post/arbitrage/slots.ts @@ -10,27 +10,27 @@ import type { AdsenseSlots } from '../../../features/monetization/adsense'; export const ARBITRAGE_SLOT = { /** Leaderboard above the article. Sticks while scrolling, then releases. */ topLeaderboard: 2, - /** Medium rectangle beside the tags, date and cover image. */ - inlineMpu1: 3, - /** Rail unit after the author card. */ - railAfterCreator: 4, - /** Rail unit after the share bar. */ - railAfterShare: 5, - /** Rail unit after the highlights widget. */ - railAfterHighlights: 6, - /** Native unit, repeated through a long comment thread. */ - commentNative: 7, + /** MPU repeated through a long comment thread. */ + commentMpu: 7, /** "MPU 1" in the brief: first rail unit, under the source card. */ railAfterSource: 11, - /** Sticky rail unit after the further reading widget. */ + /** Second rail unit, after the further reading widget. */ railBetweenFurtherReading: 12, + /** MPU repeated through the article body, one per BODY_CHARS_PER_AD. */ + inBodyMpu: 17, + /** MPU directly above the comment section. */ + aboveCommentsMpu: 18, + /** Half page closing the rail — the page's only sticky unit. */ + railBottomSticky: 19, } as const; /* - * Slot numbers 1, 8, 9, 10 and 13 are retired rather than reused: the sidebar - * unit, the two closing multiplex grids, the half-page rail tower and the - * custom floating leaderboard were all dropped, and their AdSense reporting - * rows stay readable only while no other placement inherits the number. + * Slot numbers 1, 3, 4, 5, 6, 8, 9, 10 and 13 are retired rather than reused: + * the sidebar unit, the MPU beside the cover, the three extra rail units, the + * two closing multiplex grids, the half-page rail tower and the custom + * floating leaderboard were all dropped, and their AdSense reporting rows + * stay readable only while no other placement inherits the number. 15 and 16 + * belong to the organic post page below. * * The bottom leaderboard is Google's Anchor format now, not a slot in this * map: a publisher-implemented sticky is capped at 300px wide and desktop @@ -56,10 +56,18 @@ export const ARBITRAGE_SLOT = { export const TOP_LEADERBOARD_STICKY_MS = 10_000; /** - * A long thread gets a native unit after every this many comments. Short - * threads never reach the interval, so they stay entirely ad-free. + * A long thread gets an MPU after every this many comments (the expert brief + * says 6-8). Short threads never reach the interval, so they stay ad-free. */ -export const COMMENTS_PER_INTERLEAVED_AD = 5; +export const COMMENTS_PER_INTERLEAVED_AD = 7; + +/** + * Visible characters of article body between in-body MPUs — ~250 words, the + * partner brief's cadence. Content-proportional by construction: a phone + * renders this many characters far taller than the 250px unit, so the + * density cap holds at any article length. + */ +export const BODY_CHARS_PER_AD = 1500; /** * The AdSense units behind each slot, keyed by slot number. Deliberately in @@ -75,38 +83,29 @@ export const COMMENTS_PER_INTERLEAVED_AD = 5; */ export const READ_ADSENSE_SLOTS: AdsenseSlots = { [ARBITRAGE_SLOT.topLeaderboard]: { id: '9942870945', type: 'display' }, - // read_s03 (9651332107) is an in-article unit, so it is fluid: it ignored - // both the shape and an explicit 300x250 on the and kept answering the - // placement beside the cover with a card twice the cover's height. Pointing - // it at a responsive Display unit is what actually binds the shape — at the - // cost of blending its reporting with the rail unit it borrows. - // TODO(chris): create a dedicated read_s03 Display unit and swap the id back - // to get per-placement RPM. - [ARBITRAGE_SLOT.inlineMpu1]: { id: '6921226982', type: 'display' }, - // TODO(chris): create the three new rail units (suggested names - // read_s04_rail_creator, read_s05_rail_share, read_s06_rail_highlights) as - // Display 300x250. They stay collapsed until their ids are filled in. - [ARBITRAGE_SLOT.railAfterCreator]: { id: '', type: 'display' }, - [ARBITRAGE_SLOT.railAfterShare]: { id: '', type: 'display' }, - [ARBITRAGE_SLOT.railAfterHighlights]: { id: '', type: 'display' }, - // TODO(chris): layoutKey from the read_s07_comment_native "Get code" snippet - // (data-ad-layout-key). The slot stays collapsed until it is filled in. - // - // PRECONDITIONS on filling this in — this comment is the gate, since the - // workflow is "ship reviewed once, switch on by editing this map": - // 1. Ad label: DONE in code — ProgrammaticAd renders the policy-permitted - // "Advertisements" caption above every inFeed unit, so an unlabeled - // native between comments cannot ship by omission. - // 2. Re-measure phone ad density on a long thread with the interval live. - // The ~27% figure was measured with this slot inert, it is the only - // repeating slot on the page, and Chrome's Better Ads filter applies to - // the whole domain, direct-sold inventory included. - [ARBITRAGE_SLOT.commentNative]: { id: '', type: 'inFeed', layoutKey: '' }, + // The three MPU placements below share existing Display units while the + // dedicated ones don't exist: Google's per-unit reporting blends them, but + // our first-party events split by slot number, so per-placement RPM stays + // queryable in ClickHouse. + // TODO(chris): create dedicated Display units (read_s17_in_body, + // read_s18_above_comments) and swap the ids for clean AdSense-side rows. + [ARBITRAGE_SLOT.commentMpu]: { id: '6921226982', type: 'display' }, [ARBITRAGE_SLOT.railAfterSource]: { id: '5249052667', type: 'display' }, [ARBITRAGE_SLOT.railBetweenFurtherReading]: { id: '6921226982', type: 'display', }, + [ARBITRAGE_SLOT.inBodyMpu]: { id: '6921226982', type: 'display' }, + [ARBITRAGE_SLOT.aboveCommentsMpu]: { id: '5249052667', type: 'display' }, + // read_s10's fixed 300x600, back as the rail's closing unit. Compliant as a + // publisher sticky: 300px wide, desktop only, and the page's ONLY sticky — + // AdSense allows exactly one per viewport. + [ARBITRAGE_SLOT.railBottomSticky]: { + id: '4307400883', + type: 'display', + width: 300, + height: 600, + }, }; /** diff --git a/packages/shared/src/components/post/arbitrage/splitContentForAds.spec.ts b/packages/shared/src/components/post/arbitrage/splitContentForAds.spec.ts new file mode 100644 index 0000000000..6dfbd65ef6 --- /dev/null +++ b/packages/shared/src/components/post/arbitrage/splitContentForAds.spec.ts @@ -0,0 +1,70 @@ +import { splitContentForAds } from './splitContentForAds'; + +const para = (chars: number, label: string): string => + `

${label.repeat(Math.ceil(chars / label.length)).slice(0, chars)}

`; + +describe('splitContentForAds', () => { + it('returns short content as a single chunk', () => { + const html = para(100, 'a'); + expect(splitContentForAds(html, 300)).toEqual([html]); + }); + + it('splits only at top-level block boundaries', () => { + const first = para(300, 'a'); + const second = para(300, 'b'); + const chunks = splitContentForAds(first + second, 250); + + expect(chunks).toEqual([first, second]); + }); + + it('never cuts inside a nested structure', () => { + const list = `
  • ${'x'.repeat(300)}
  • ${'y'.repeat( + 300, + )}
`; + const after = para(300, 'z'); + const chunks = splitContentForAds(list + after, 250); + + // The list crosses the threshold internally but closes as one unit. + expect(chunks).toEqual([list, after]); + }); + + it('treats code blocks as unsplittable units', () => { + const code = `
${'if (x) {\n}\n'.repeat(40)}
`; + const after = para(300, 'a'); + const chunks = splitContentForAds(code + after, 250); + + expect(chunks).toHaveLength(2); + expect(chunks[0]).toBe(code); + }); + + it('does not let void elements corrupt the depth count', () => { + const withImages = `

${'a'.repeat(150)}
${'b'.repeat( + 150, + )}

`; + const after = para(300, 'c'); + + expect(splitContentForAds(withImages + after, 250)).toEqual([ + withImages, + after, + ]); + }); + + it('merges a trailing sliver into the previous chunk', () => { + const first = para(300, 'a'); + const sliver = para(40, 'b'); + const chunks = splitContentForAds(first + sliver, 250); + + // An ad before one stray line reads as the page ending on an ad. + expect(chunks).toEqual([first + sliver]); + }); + + it('keeps every byte of the input across the chunks', () => { + const html = + `${para(400, 'a')}
${'q'.repeat(300)}
` + + `

Heading

${para(400, 'b')}`; + const chunks = splitContentForAds(html, 250); + + expect(chunks.join('')).toBe(html); + expect(chunks.length).toBeGreaterThan(1); + }); +}); diff --git a/packages/shared/src/components/post/arbitrage/splitContentForAds.ts b/packages/shared/src/components/post/arbitrage/splitContentForAds.ts new file mode 100644 index 0000000000..0cd9f0b48c --- /dev/null +++ b/packages/shared/src/components/post/arbitrage/splitContentForAds.ts @@ -0,0 +1,78 @@ +const TAG_RE = /<\/?([a-zA-Z][\w-]*)(?:[^>'"]|"[^"]*"|'[^']*')*?\/?>/g; + +// Elements that never take a closing tag, so an opening token must not +// increase the nesting depth. +const VOID_ELEMENTS = new Set([ + 'area', + 'base', + 'br', + 'col', + 'embed', + 'hr', + 'img', + 'input', + 'link', + 'meta', + 'source', + 'track', + 'wbr', +]); + +const visibleLength = (text: string): number => + text + .replace(/&[#\w]+;/g, 'x') + .replace(/\s+/g, ' ') + .trim().length; + +/** + * Splits rendered article HTML into chunks of at least `minChars` of visible + * text, cutting only where a top-level block element closes — an ad between + * chunks can never land inside a paragraph, list, blockquote or code block. + * A short tail is merged into the chunk before it, so the article never ends + * on an ad followed by a stray line. + */ +export function splitContentForAds(html: string, minChars: number): string[] { + const chunks: string[] = []; + let depth = 0; + let chunkStart = 0; + let cursor = 0; + let visible = 0; + + TAG_RE.lastIndex = 0; + let match = TAG_RE.exec(html); + while (match) { + const [token, rawName] = match; + visible += visibleLength(html.slice(cursor, match.index)); + cursor = match.index + token.length; + + const name = rawName.toLowerCase(); + if (token.startsWith('= minChars) { + chunks.push(html.slice(chunkStart, cursor)); + chunkStart = cursor; + visible = 0; + } + } else if (!VOID_ELEMENTS.has(name) && !token.endsWith('/>')) { + depth += 1; + } + + match = TAG_RE.exec(html); + } + + const tail = html.slice(chunkStart); + if (tail.trim()) { + chunks.push(tail); + } + + // The last chunk earns its preceding ad only when it carries real content. + if (chunks.length > 1) { + const last = chunks[chunks.length - 1]; + if (visibleLength(last.replace(TAG_RE, ' ')) < minChars / 2) { + chunks[chunks.length - 2] += last; + chunks.pop(); + } + } + + return chunks; +} diff --git a/packages/shared/src/features/monetization/ProgrammaticAd.tsx b/packages/shared/src/features/monetization/ProgrammaticAd.tsx index c9981fae43..e1c13e27c4 100644 --- a/packages/shared/src/features/monetization/ProgrammaticAd.tsx +++ b/packages/shared/src/features/monetization/ProgrammaticAd.tsx @@ -321,7 +321,9 @@ export function ProgrammaticAd({ observer.disconnect(); setIsRequested(true); }, - { rootMargin: '600px' }, + // Expert-tuned: request just as the slot approaches rather than a + // viewport ahead — viewability over prefetch. + { rootMargin: '50px' }, ); observer.observe(element); @@ -483,7 +485,9 @@ export function ProgrammaticAd({
- {/* A fluid native unit is the "confusable with site content" shape - AdSense prohibits shipping unlabeled — "Advertisements" is one of - the two label strings its policy permits. Inside the wrapper, so an - unfilled slot's collapse takes the label down with it. */} - {isRequested && config.type === 'inFeed' && ( - + {/* Every unit is labeled so none can be confused with site content — + "Advertisements" is one of the two label strings AdSense permits + (a bare "Advertisement" is not). Inside the wrapper, so an unfilled + slot's collapse takes the label down with it. */} + {isRequested && ( + Advertisements )} From 983b413f81c29a30d70daebc74d513b3b571c97b Mon Sep 17 00:00:00 2001 From: Chris Bongers Date: Tue, 25 Aug 2026 16:19:26 +0200 Subject: [PATCH 2/7] fix(shared): resolve the iteration review and fold in the product calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review blockers: - Wrapper padding is vertical-only — border-box horizontal padding shrank every unit below its IAB width cap, visibly misaligning the fixed 300x600 rail closer. - Every FORMAT_SPEC minHeight now reserves the label row and padding, so a box never grows under the reader when its request lands in-viewport. - rootMargin lands on 250px: the expert's 50px put both the auction and the unfilled collapse inside the visible area. - The phone-density gate is honored, not deleted: the comment MPU is phone-hidden in the template until a long-thread measurement clears it, and the in-body cadence's phone math is written at the map. Product calls folded in: - The TLDR carries the same MPU cadence as a hosted body — for a scraped article it is the only main content, split at sentence/word boundaries by splitTextForAds. - The comment interval counts every comment, replies included, 8 flat. - The ad label stays lowercase-styled "Advertisements" (the policy string; no CSS uppercase). Also: splits memoised on their inputs, HTML comments can no longer skew the splitter's depth count, per-occurrence and per-section analytics on repeated placements via logExtra, and the stale five-unit-rail docs are rewritten. Co-Authored-By: Claude Fable 5 --- .../src/components/post/PostComments.tsx | 31 +++-- .../post/arbitrage/ArbitrageAdSlot.tsx | 13 ++- .../post/arbitrage/ArbitragePostContent.tsx | 107 +++++++++++------- .../src/components/post/arbitrage/slots.ts | 15 ++- .../post/arbitrage/splitContentForAds.spec.ts | 28 ++++- .../post/arbitrage/splitContentForAds.ts | 37 +++++- .../features/monetization/ProgrammaticAd.tsx | 40 +++++-- 7 files changed, 201 insertions(+), 70 deletions(-) diff --git a/packages/shared/src/components/post/PostComments.tsx b/packages/shared/src/components/post/PostComments.tsx index ed0b686cec..ebab246af7 100644 --- a/packages/shared/src/components/post/PostComments.tsx +++ b/packages/shared/src/components/post/PostComments.tsx @@ -46,10 +46,12 @@ interface PostCommentsProps { canReply?: MainCommentProps['canReply']; onReplyBlocked?: MainCommentProps['onReplyBlocked']; /** - * Renders between top-level comments, every `interleaveEvery` of them. Used - * by the ad template to break a long thread up; never after the last comment, - * where whatever follows the thread already sits. Both props are required - * together, and without them the list keeps its original markup. + * Renders after the top-level comment at which the running total of + * comments — replies included, every comment counts — crosses a multiple + * of `interleaveEvery`. Used by the ad template to break a long thread up; + * never after the last top-level comment, where whatever follows the + * thread already sits. Both props are required together, and without them + * the list keeps its original markup. */ interleaveEvery?: number; renderInterleaved?: (occurrence: number) => ReactNode; @@ -134,15 +136,28 @@ export function PostComments({ } ref={container} > - {comments!.postComments.edges.map((e, index) => { - const isLast = index === comments!.postComments.edges.length - 1; + {comments!.postComments.edges.map((e, index, edges) => { + const isLast = index === edges.length - 1; + // Replies count too: the interval is over everything the reader + // scrolls past, not just top-level rows, and the boundary can only + // sit after a top-level block — never inside one. + const countThrough = (through: number): number => + edges + .slice(0, through) + .reduce( + (total, edge) => + total + 1 + (edge.node.children?.edges?.length ?? 0), + 0, + ); + const seen = countThrough(index + 1); // Never after the last comment, where whatever follows the thread // already sits. const shouldInterleave = !!interleaveEvery && !!renderInterleaved && !isLast && - (index + 1) % interleaveEvery === 0; + Math.floor(seen / interleaveEvery) > + Math.floor(countThrough(index) / interleaveEvery); // Always the Fragment, even rows that interleave nothing: the type at // a given key must not flip as the boundary moves (a new comment @@ -174,7 +189,7 @@ export function PostComments({ onReplyBlocked={onReplyBlocked} /> {shouldInterleave && - renderInterleaved((index + 1) / interleaveEvery)} + renderInterleaved(Math.floor(seen / interleaveEvery))} ); })} diff --git a/packages/shared/src/components/post/arbitrage/ArbitrageAdSlot.tsx b/packages/shared/src/components/post/arbitrage/ArbitrageAdSlot.tsx index 3d490b1808..5d1072c676 100644 --- a/packages/shared/src/components/post/arbitrage/ArbitrageAdSlot.tsx +++ b/packages/shared/src/components/post/arbitrage/ArbitrageAdSlot.tsx @@ -28,11 +28,10 @@ export interface ArbitrageAdSlotProps { /** Marks slots wired to a declared 30-60s in-view refresh once on Ad Manager. */ refreshes?: boolean; /** - * Drops the slot below the tablet breakpoint. The Better Ads Standards cap - * mobile ad density at 30% of page height, and a scraped post carries little - * body text to dilute it — running every slot on a phone measured 56%, which - * is what gets a site's ads filtered by Chrome. The unit is hidden rather - * than skipped so it also never requests: the ad only pushes on intersection, + * Drops the slot below the tablet breakpoint — the Better Ads Standards cap + * mobile ad density at 30% of page height, and Chrome's filter for a + * violation applies to the whole domain. The unit is hidden rather than + * skipped so it also never requests: the ad only pushes on intersection, * and a display:none box never intersects. */ hideOnPhone?: boolean; @@ -49,6 +48,8 @@ export interface ArbitrageAdSlotProps { * arrived, so eager pushes ride its very first processing pass. */ eager?: boolean; + /** Per-instance extra for repeated placements — see ProgrammaticAd. */ + logExtra?: Record; } function MappedAdSlot({ @@ -58,6 +59,7 @@ function MappedAdSlot({ refreshes, hideOnPhone, eager, + logExtra, slots, surface, allowPlaceholder = false, @@ -88,6 +90,7 @@ function MappedAdSlot({ refreshes={refreshes} hideOnPhone={hideOnPhone} eager={eager} + logExtra={logExtra} /> ); } diff --git a/packages/shared/src/components/post/arbitrage/ArbitragePostContent.tsx b/packages/shared/src/components/post/arbitrage/ArbitragePostContent.tsx index c36cfc96d7..8d09d2ef58 100644 --- a/packages/shared/src/components/post/arbitrage/ArbitragePostContent.tsx +++ b/packages/shared/src/components/post/arbitrage/ArbitragePostContent.tsx @@ -1,5 +1,5 @@ import type { ReactElement } from 'react'; -import React from 'react'; +import React, { useMemo } from 'react'; import classNames from 'classnames'; import type { Post } from '../../../graphql/posts'; import { isVideoPost } from '../../../graphql/posts'; @@ -20,7 +20,7 @@ import Markdown from '../../Markdown'; import { ArbitrageAdFormat, ArbitrageAdSlot } from './ArbitrageAdSlot'; import { ArbitrageTopLeaderboard } from './ArbitrageTopLeaderboard'; import { PostAnsweredQuestions } from '../PostAnsweredQuestions'; -import { splitContentForAds } from './splitContentForAds'; +import { splitContentForAds, splitTextForAds } from './splitContentForAds'; import { ARBITRAGE_SLOT, BODY_CHARS_PER_AD, @@ -33,17 +33,10 @@ import { PostWidgets, PostWidgetPosition } from '../PostWidgets'; import PostEngagements from '../PostEngagements'; /** - * One slot per real rail widget, in render order. The rail is the page's only - * column with no article in it, so every widget there earns a unit; the two - * that are already commercial (the house ad widget and the sponsored tools - * card) have no position and so get none. - * - * Below laptop the rail stacks under the article rather than beside it, so - * an unfiltered run lands every unit on a phone too — measured at roughly 40% - * of page height against the Better Ads Standards' 30% mobile cap, and - * Chrome's ad filter for a violation applies to the whole domain, direct-sold - * inventory included. Only the first rail unit keeps its phone placement; the - * rest are desktop-only, which brings the phone run well under the cap. + * The rail carries two in-flow units between its widgets, and the closing + * sticky half page arrives separately via PostWidgets' `trailing`. Only the + * first keeps a phone placement: below laptop the rail stacks under the + * article, where the body and comment cadences already carry the density. */ const RAIL_AD: Partial< Record< @@ -102,6 +95,22 @@ export function ArbitragePostContent({ post, }); const leaderboardReleased = useTimedRelease(TOP_LEADERBOARD_STICKY_MS); + // Memoised: the splits re-scan the whole text, and this component + // re-renders on comment sorting, hover state and auth resolution. The TLDR + // is main content here — for a scraped article it is the only content — so + // it carries the same MPU cadence as a hosted body. + const summaryParts = useMemo( + () => + post.summary ? splitTextForAds(post.summary, BODY_CHARS_PER_AD) : [], + [post.summary], + ); + const bodyChunks = useMemo( + () => + post.contentHtml + ? splitContentForAds(post.contentHtml, BODY_CHARS_PER_AD) + : [], + [post.contentHtml], + ); return ( @@ -184,13 +193,22 @@ export function ArbitragePostContent({ /> )} - {!!post.summary && ( -
-

- {post.summary} -

-
- )} + {summaryParts.map((part, index, parts) => ( + // eslint-disable-next-line react/no-array-index-key + +
+

{part}

+
+ {index < parts.length - 1 && ( + + )} + + ))}
@@ -243,26 +261,26 @@ export function ArbitragePostContent({ {/* One MPU per BODY_CHARS_PER_AD of visible text, only ever between top-level blocks — splitContentForAds cannot cut a paragraph, list - or code block in half. */} - {!!post.contentHtml && - splitContentForAds(post.contentHtml, BODY_CHARS_PER_AD).map( - (chunk, index, chunks) => ( - // eslint-disable-next-line react/no-array-index-key - - globalThis?.document?.body} - /> - {index < chunks.length - 1 && ( - - )} - - ), - )} + or code block in half. Section and occurrence ride the events so + analytics can tell the first in-body unit from the sixth. */} + {bodyChunks.map((chunk, index, chunks) => ( + // eslint-disable-next-line react/no-array-index-key + + globalThis?.document?.body} + /> + {index < chunks.length - 1 && ( + + )} + + ))} {/* Same block the post page shows: the questions that likely brought an anonymous visitor here (the component self-hides for logged-in @@ -278,17 +296,22 @@ export function ArbitragePostContent({ {/* The production engagement block verbatim — counts, actions, share, sort control, composer and thread — so everything from here to the end of the discussion matches the live post page exactly. The only - addition is a native unit every few comments in a long thread. */} + addition is an MPU as a long thread grows. */} ( + renderInterleaved={(occurrence) => ( + // Phone-hidden until the density precondition in slots.ts is + // satisfied: a repeating unit, and the phone figure was measured + // without it. )} /> diff --git a/packages/shared/src/components/post/arbitrage/slots.ts b/packages/shared/src/components/post/arbitrage/slots.ts index 8874755805..062093904e 100644 --- a/packages/shared/src/components/post/arbitrage/slots.ts +++ b/packages/shared/src/components/post/arbitrage/slots.ts @@ -56,10 +56,11 @@ export const ARBITRAGE_SLOT = { export const TOP_LEADERBOARD_STICKY_MS = 10_000; /** - * A long thread gets an MPU after every this many comments (the expert brief - * says 6-8). Short threads never reach the interval, so they stay ad-free. + * A long thread gets an MPU each time this many comments have gone by — + * replies included, every comment counts, 8 flat (product call, Aug 25). + * Short threads never reach the interval, so they stay ad-free. */ -export const COMMENTS_PER_INTERLEAVED_AD = 7; +export const COMMENTS_PER_INTERLEAVED_AD = 8; /** * Visible characters of article body between in-body MPUs — ~250 words, the @@ -89,6 +90,14 @@ export const READ_ADSENSE_SLOTS: AdsenseSlots = { // queryable in ClickHouse. // TODO(chris): create dedicated Display units (read_s17_in_body, // read_s18_above_comments) and swap the ids for clean AdSense-side rows. + // + // Phone density, the written gate the previous map kept slot 7 behind: + // - The comment MPU stays hideOnPhone in the template until a long-thread + // phone measurement with the interval live says otherwise. + // - The in-body MPUs are phone-visible by construction, not by measurement: + // one 250px unit per BODY_CHARS_PER_AD (1500) visible characters is + // ~1100px of rendered text per unit at phone width — under 20% added + // density at any article length, against the Better Ads 30% cap. [ARBITRAGE_SLOT.commentMpu]: { id: '6921226982', type: 'display' }, [ARBITRAGE_SLOT.railAfterSource]: { id: '5249052667', type: 'display' }, [ARBITRAGE_SLOT.railBetweenFurtherReading]: { diff --git a/packages/shared/src/components/post/arbitrage/splitContentForAds.spec.ts b/packages/shared/src/components/post/arbitrage/splitContentForAds.spec.ts index 6dfbd65ef6..dece97e54b 100644 --- a/packages/shared/src/components/post/arbitrage/splitContentForAds.spec.ts +++ b/packages/shared/src/components/post/arbitrage/splitContentForAds.spec.ts @@ -1,4 +1,4 @@ -import { splitContentForAds } from './splitContentForAds'; +import { splitContentForAds, splitTextForAds } from './splitContentForAds'; const para = (chars: number, label: string): string => `

${label.repeat(Math.ceil(chars / label.length)).slice(0, chars)}

`; @@ -67,4 +67,30 @@ describe('splitContentForAds', () => { expect(chunks.join('')).toBe(html); expect(chunks.length).toBeGreaterThan(1); }); + it('ignores tags inside HTML comments when balancing depth', () => { + const html = `${para(300, 'a')}${para(300, 'b')}`; + const chunks = splitContentForAds(html, 250); + + expect(chunks.join('')).toBe(html); + expect(chunks.length).toBe(2); + }); +}); + +describe('splitTextForAds', () => { + it('keeps a short TLDR whole', () => { + expect(splitTextForAds('short summary.', 250)).toEqual(['short summary.']); + }); + + it('breaks a long TLDR at a sentence end past the threshold', () => { + const first = `${'a'.repeat(260)}.`; + const second = 'b'.repeat(300); + const parts = splitTextForAds(`${first} ${second}`, 250); + + expect(parts).toEqual([first, second]); + }); + + it('never ends on a sliver', () => { + const text = `${'a'.repeat(260)}. ${'b'.repeat(30)}`; + expect(splitTextForAds(text, 250)).toHaveLength(1); + }); }); diff --git a/packages/shared/src/components/post/arbitrage/splitContentForAds.ts b/packages/shared/src/components/post/arbitrage/splitContentForAds.ts index 0cd9f0b48c..40deef1710 100644 --- a/packages/shared/src/components/post/arbitrage/splitContentForAds.ts +++ b/packages/shared/src/components/post/arbitrage/splitContentForAds.ts @@ -1,4 +1,7 @@ -const TAG_RE = /<\/?([a-zA-Z][\w-]*)(?:[^>'"]|"[^"]*"|'[^']*')*?\/?>/g; +// Comments first, so a tag inside `` can never touch the +// depth count; the alternation consumes the whole comment as one token. +const TAG_RE = + /|<\/?([a-zA-Z][\w-]*)(?:[^>'"]|"[^"]*"|'[^']*')*?\/?>/g; // Elements that never take a closing tag, so an opening token must not // increase the nesting depth. @@ -45,6 +48,12 @@ export function splitContentForAds(html: string, minChars: number): string[] { visible += visibleLength(html.slice(cursor, match.index)); cursor = match.index + token.length; + if (token.startsWith('