Skip to content

feat(shared): expert-round ad iteration on /articles #6528

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
rebelchris wants to merge 1 commit into from
Closed

feat(shared): expert-round ad iteration on /articles #6528

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion packages/shared/src/components/post/PostEngagements.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -60,6 +65,7 @@ function PostEngagements({
onCopyLinkClick,
logOrigin,
shouldOnboardAuthor,
hideInternalAd,
interleaveEvery,
renderInterleaved,
}: PostEngagementsProps): ReactElement {
Expand Down Expand Up @@ -172,7 +178,7 @@ function PostEngagements({
shouldHandleCommentQuery
CommentInputOrModal={CommentInputOrModal}
/>
{!isPlus && <AdAsComment postId={post.id} />}
{!isPlus && !hideInternalAd && <AdAsComment postId={post.id} />}
<PostComments
post={post}
sortBy={sortBy}
Expand Down
13 changes: 9 additions & 4 deletions packages/shared/src/components/post/PostWidgets.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ export type PostWidgetsProps = Omit<PostHeaderActionsProps, 'contextMenuId'> &
getRailAd?: (position: PostWidgetPosition) => ReactNode;
/** Rendered last, below the footer links. */
trailing?: ReactNode;
/** Drops the internal sidebar ad — for templates carrying their own. */
hideAdWidget?: boolean;
};

/**
Expand Down Expand Up @@ -100,6 +102,7 @@ export function PostWidgets({
hideToc = false,
getRailAd,
trailing,
hideAdWidget,
}: PostWidgetsProps): ReactElement {
const { tokenRefreshed } = useContext(AuthContext);
const { source } = post;
Expand Down Expand Up @@ -158,10 +161,12 @@ export function PostWidgets({
/>
),
)}
<PostSidebarAdWidget
postId={post.id}
className={{ container: cardClasses }}
/>
{!hideAdWidget && (
<PostSidebarAdWidget
postId={post.id}
className={{ container: cardClasses }}
/>
)}
<MentionedToolsWidget postTags={post.tags || []} />
{withAd(
PostWidgetPosition.Share,
Expand Down
131 changes: 70 additions & 61 deletions packages/shared/src/components/post/arbitrage/ArbitragePostContent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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,
},
};
Expand Down Expand Up @@ -210,24 +192,7 @@ export function ArbitragePostContent({
</div>
)}

{/* 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. */}
<div className="mb-6 flex flex-col gap-6 laptop:flex-row laptop:items-end">
<ArbitrageAdSlot
slot={ARBITRAGE_SLOT.inlineMpu1}
format={ArbitrageAdFormat.Rectangle}
refreshes
className="laptop:order-last"
/>

<div className="mb-6">
<div className="min-w-0 flex-1">
<PostTagList post={post} />
<PostMetadata
Expand Down Expand Up @@ -276,13 +241,39 @@ export function ArbitragePostContent({
</div>
</div>

{!!post.contentHtml && (
<Markdown
className="my-6"
content={post.contentHtml}
appendTooltipTo={() => 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(

@rebelchris rebelchris Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: splitContentForAds runs on every render of this component — the whole article body re-scanned on each comment-sort change, hover state, auth resolution, etc. Wrap it in useMemo keyed on post.contentHtml.

Also worth confirming the per-chunk cost of Markdown: each instance runs its own DOMPurify pass and mounts its own HoverCard (a dynamic(ssr: false) import) plus the mention useQuery. A long article now instantiates that 5-8 times instead of once. If mention hovercards or image-modal delegation behave differently across chunk boundaries, that would show up here first — did you check a body with mentions and images on the preview?

Reviewed by AI.

(chunk, index, chunks) => (
// eslint-disable-next-line react/no-array-index-key
<React.Fragment key={index}>
<Markdown
className="my-6"
content={chunk}
appendTooltipTo={() => globalThis?.document?.body}
/>
{index < chunks.length - 1 && (
<ArbitrageAdSlot
slot={ARBITRAGE_SLOT.inBodyMpu}

@rebelchris rebelchris Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking (analytics + polish): every in-body occurrence renders with the same slot number, so first-party events cannot distinguish the first in-body MPU from the sixth. Since the whole point of the cadence is finding where in-body inventory pays, consider passing the occurrence index through the log extra. It also means several elements share data-testid="adsense-slot-17".

Minor: this slot has no spacing class while the above-comments MPU gets my-6, and the surrounding <div className="mb-6"><div className="min-w-0 flex-1"> nesting above is left over from the removed flex row — it can collapse to one element now.

Reviewed by AI.

format={ArbitrageAdFormat.MediumRectangle}
/>
)}
</React.Fragment>
),
)}

{/* 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). */}
<PostAnsweredQuestions post={post} />

<ArbitrageAdSlot
slot={ARBITRAGE_SLOT.aboveCommentsMpu}
format={ArbitrageAdFormat.MediumRectangle}
className="my-6"
/>

{/* The production engagement block verbatim — counts, actions, share,
sort control, composer and thread — so everything from here to the
Expand All @@ -292,11 +283,12 @@ export function ArbitragePostContent({
post={post}
onCopyLinkClick={onCopyPostLink}
logOrigin={Origin.ArticlePage}

@rebelchris rebelchris Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking (doc drift): a few comments no longer match the code after this change and will mislead the next reader:

  • here: "The only addition is a native unit every few comments" — it is an MPU now.
  • the RAIL_AD docblock: "One slot per real rail widget, in render order... every widget there earns a unit" and the ~40% phone measurement, both written for the five-unit rail.
  • ArbitrageAdSlot's hideOnPhone doc still quotes the 56% figure from that layout.
  • refreshes is now only used by the top leaderboard; worth a look at whether the prop still earns its place on ArbitrageAdSlot.

Reviewed by AI.

hideInternalAd
interleaveEvery={COMMENTS_PER_INTERLEAVED_AD}
renderInterleaved={() => (
<ArbitrageAdSlot
slot={ARBITRAGE_SLOT.commentNative}
format={ArbitrageAdFormat.Native}
slot={ARBITRAGE_SLOT.commentMpu}
format={ArbitrageAdFormat.MediumRectangle}
/>
)}
/>
Expand All @@ -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 (
<ArbitrageAdSlot
slot={spec.slot}
Expand All @@ -323,6 +320,18 @@ export function ArbitragePostContent({
/>
);
}}
// 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={
<ArbitrageAdSlot
slot={ARBITRAGE_SLOT.railBottomSticky}
format={ArbitrageAdFormat.HalfPage}
className="laptop:sticky laptop:top-[calc(var(--sticky-header-offset)+1rem)] laptop:z-1"
hideOnPhone
/>
}
/>
</PostContentContainerRaw>
);
Expand Down
89 changes: 44 additions & 45 deletions packages/shared/src/components/post/arbitrage/slots.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;

@rebelchris rebelchris Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking (density gate removed, not satisfied): the version of this map being replaced kept slot 7 collapsed behind an explicit written precondition — "Re-measure phone ad density on a long thread with the interval live. The ~27% figure was measured with this slot inert... Chrome's Better Ads filter applies to the whole domain, direct-sold inventory included."

This PR fills the unit id (activating that repeating unit) and additionally adds an unbounded run of in-body MPUs with no hideOnPhone, plus the above-comments MPU. The RAIL_AD docblock in ArbitragePostContent records that the unfiltered phone run measured ~40% (and ArbitrageAdSlot's hideOnPhone doc says 56%) against the 30% cap, which is why all but one rail unit are desktop-only.

So the page now has more repeating phone inventory than when the gate was written, and the gate itself was deleted along with the comment. Please post a fresh measured phone density on a long article with a long thread before this ships, and consider a hard cap on in-body units (or hideOnPhone on all but the first) so density cannot scale with article length.

Reviewed by AI.


/**
* 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
Expand All @@ -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 <ins> 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,
},
};

/**
Expand Down
Loading
Loading